-
Notifications
You must be signed in to change notification settings - Fork 16
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat/refactor: bemanitools core, logger, thread #282
Open
icex2
wants to merge
41
commits into
djhackersdev:development
Choose a base branch
from
icex2:logger-refactoring
base: development
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We want this to always be visible, so the log level must be the highest possible. Unfortunately, that's quite the hack around the existing API and how fatal behaves. The log API stopped scaling already a while ago and needs considerable refactoring to consider the various use-cases that emerged since it was first created on alpha versions of bemanitools.
Because we are using mingw, we can't just use window's dbghelp library as the symbols created are in dwarf format. Fortunately, the dwarfstack library already provides all the facilities to easily print very descriptive stacktraces, including function names, file names and line numbers, when dwarf symbols are available. This moves the incomplete exception handling portion from signal to a separate module as well to improve scoping.
Adjust inject to utilize the new feature. This also requires including the dwarfstack.dll in all distribution packages.
Expecting to improve debugability significantly. Stacktraces can be enhanced with function names, file names and line numbers when symbols are included in all exe and dll files. Furthermore, this also improves debugger usage as symbols can be imported to help navigate disassembly/decompiled code.
This has been a source of common error in the past. It is known that most, or even all, games run into various issues when not run with elevated privileges.
Use this to share helpers or other extensions to the original avs API across modules. Start with including error codes to readable strings to improve velocity on AVS API error analysis.
Taken from a private eamuse server backend which had more complete mappings.
Supports improving expressiveness of the API interface
These were previously missing and are required for various file system related tasks such as iterating directory trees, reading and writing files through the AVS file system for the upcoming launcher rework. Note that the AVS API broke with some mode flags after version 2.13.06.
Again, required for the launcher rework when dealing with property node trees.
Add all functions and their respective ordinals (and mangled names for documentation purpose) to all currently used AVS version.
This one was missing and is the actual correct version used for ddr-13. 2.13.04 was compatible, thus far, but there isn't any guarantee that they are actually 100% compatible (only konmai knows...).
Apparently forgotten to get updated to reflect the currently supported versions correctly.
Apparently also forgotten to reflect currently supported games.
Kudos to Shiz for providing the groundwork for this. Fundamentally re-think how launcher operates and bootstrapping the games is managed and configured. This brings it significantly closer to how the original bootstrap is doing the job: launcher now utilizes the data (structures) provided by the bootstrap.xml configuration file. This creates compatibility with vanilla data dumps and original stock images. Note that bemanitools does not include any code or means to run DRM'd data, only decrypted. But, this allows users to keep decrypted dumps as stock as possible which means: * No copying around of property files anymore * Keep the modules/ folder with the binaries * Have bemanitools binaries separate in the data * No need to edit/customize the original configuration files A list of key features of the "new" launcher: * Boostrap games by following the configuration provided by stock game's bootstrap.xml files * Custom launcher.xml configuration file that adds further launcher configurable features, composability of bootstrap.xml configuration(s) as well as configuration overriding/stacking of selected types of configurations, e.g. eamuse config, avs-config. The latter eliminates the need for modifying stock config files in the prop/ folder * Unified logging system: launcher and AVS logging uses the same logger, all output can now be in a single file * Original features such as various hook types still available Due to the significant architectural changes, this also breaks with any backwards compatibility to existing launcher setups. Thus, users need to migrate by re-applying the new configuration format and migrating their config parameters accordingly. Further migration instructions and updated documentation will be provided upon release. Co-authored-by: Shiz <[email protected]>
Move everything to new launcher.xml configuration files. Adjust the bootstrapping of launcher in the .bat files. Features such as copying the default props/ files to nvram are now handled by launcher. Using the PATH variable, bemanitools binaries can live in their own dedicated bemanitools/ subfolder next to props/ and modules/ now. All original binaries are expected to be kept in a modules/ folder like on stock data.
A general debugging tool. 3rd party applications such as "procmon" (same name) provide these capabilites and even more. But, they are more difficult to run with bemanitools and don't provide a unified look at the output in combination with the log output by bemanitools. Provide an initial set of system call hooks that have already supported debugging efforts. More can be added when needed later.
Integrate this as an optional dependency into launcher and load it dynamically. Thus, these can be easily loaded/enabled by developers and end-users.
icex2
force-pushed
the
logger-refactoring
branch
2 times, most recently
from
February 9, 2024 19:29
76720fe
to
79b972d
Compare
icex2
force-pushed
the
logger-refactoring
branch
from
February 19, 2024 00:02
f9dee5d
to
93ea41f
Compare
This module contains the "core" (API) of bemanitools which includes an abstraction layer for threads and logging at this time. The threads API is very close to what util/thread already was with some structural enhancements which make it easier to understand and work with the API, I hope. Some additional helpers (*-ext module) support in doing common tasks, e.g. setting up the thread API with other modules. The log(ging) part receives a major overhaul to address known limitations and issues with the util/log module: - Cleaner API layer - Separate sinks from actual logging engine - Sinks are composable - Improved and cleaner compatibility layer with AVS logging API Additional "extensions" (*-ext modules) add various helper functions for common tasks like setting up the logging engine with a file and stdout sink. The sinks also improved significantly with the file sink now supporting proper appending and log rotation. Logging to stdout/stderr supports coloring of log messages which works across logging engines. Overall, this refactored foundation is expected to support future developments and removes known limitations at the current scale of bemanitools such as: - Reducing boiler plate code across hooks - Interop of bemanitools and AVS (and setting the foundation for addressing currently missing interop, e.g. for dealing with property structures without AVS) - Addressing performance issues in the logging engine due to incorrect interop with AVS
Doesn't really reduce boiler plate but adds clarity with a more meaningful function name what the operation does.
Use these to improve error handling by allowing one to provide additional error information on property related operations.
Split files and add name spacing.
Default configuration files use this already instead of the old style specific nvram and raw nodes. Required to make overriding these work.
Boils down to: - Include headers - Reduce boiler plate with helpers - Swap out explicit usages with core API layer and ensure the right API is configured beforehand
Keep this a separate commit because this also removes inject's own logging engine and replaces it with the streamlined core API. The core API provides all the features of inject's own logging engine which also performed horribly. The entire logging operation was locked which included expensive operations that formatted the log messages and required memory allocations and copying around data. The core API's implementation at least only synchronizes the actual IO operations (though this can be improved further with an actual async logging sink, TBD)
Keep this a separate commit because this also removes launcher's own logging engine (which was a copy-paste from inject's) and replaces it with the streamlined core API. The core API provides all the features of launcher's own logging engine which also performed horribly. The entire logging operation was locked which included expensive operations that formatted the log messages and required memory allocations and copying around data. This also resulted in switching to AVS's logging engine to perform badly which likely is the/one root cause for reports of game's stuttering and even de-syncing.
Apply a simple heuristic to provide the user with more specific information regarding which vcredist package might not have been found on their system. This is a common problem as different games require different versions and different versions of windows and installations might already come with some versions already pre-installed. Also improve the readme regarding that and provide links to all versions that are required by one game or another today.
Improve the development experience by providing an additional docker container that can be started and used as an interactive development environment. It provides all the tools and a stable environment for building (identical to the build container).
Iterated implementation after the initial refactoring of launcher. It turns out, this a bit more complicated as I expected. Once you are dealing with any kind of mutation operation on property (node) data, the property API expects you to have allocated enough memory for your modifications to fit into the owning property structure. This was not considered in the old merging code and caused odd errors of data not being written at all or just partially. Improving the error handling and output, the property API communicates these errors within an error code state in the property structure that can be retrieved using property_get_error. The fixed merging and cloning functions ensure that the property structure should have enough space to house a worst case merger of adding everything from the source to the parent. Furthermore, the merging now supports custom "merging strategies". As some konmai developers decided to not stick to the actually well defined property structure format, use-cases like the new style mounttables in the avs config exist which uses xml attributes to store actual data. This requires allowing to plug in arbitrary visitor-style functions that execute a different node visiting, evaluation and merging strategy.
A custom node merge strategy to support the new style mounttables in avs configurations.
Required to support reading the legacy and new style mounttables in avs configuration files.
With the new abstraction for mounttables, this can be fixed to support all currently known styles of mounttables in avs configs.
icex2
force-pushed
the
logger-refactoring
branch
from
February 19, 2024 22:27
93ea41f
to
2d7d084
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Recommendation to reviewer(s): Review by commit, I added further details and reasoning and tried my best to scope slice and scope the changes.
The main goal of the entire changeset is to refactor two "core" APIs/modules in bemanitools, thread and log(ger). This is, more or less, simply done by moving it out of utils and into a new package named "core".
But, the changes furthermore address several issues and limitations that bemanitools has experienced over the last years.
Copy-paste from the "key commit" that adds the new core module with thread and log:
This module contains the "core" (API) of
bemanitools which includes an abstraction
layer for threads and logging at this time.
The threads API is very close to what
util/thread already was with some structural
enhancements which make it easier to understand
and work with the API, I hope. Some additional
helpers (*-ext module) support in doing common
tasks, e.g. setting up the thread API with other
modules.
The log(ging) part receives a major overhaul to
address known limitations and issues with the
util/log module:
with AVS logging API
Additional "extensions" (*-ext modules) add
various helper functions for common tasks like
setting up the logging engine with a file and stdout
sink.
The sinks also improved significantly with the file
sink now supporting proper appending and log rotation.
Logging to stdout/stderr supports coloring of log
messages which works across logging engines.
Overall, this refactored foundation is expected to
support future developments and removes known
limitations at the current scale of bemanitools such as:
for addressing currently missing interop, e.g. for
dealing with property structures without AVS)
due to incorrect interop with AVS