-
Notifications
You must be signed in to change notification settings - Fork 215
Native hooking improvements #404
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
Open
noopsledge
wants to merge
9
commits into
satisfactorymodding:dev
Choose a base branch
from
noopsledge:dev-native-hooking
base: dev
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.
Open
Conversation
This file contains hidden or 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
The existing code was following the Windows ABI on all platforms, which meant that on Linux the parameters were in the wrong order.
This removes all code duplication around member and non-member functions and introduces a generalized HookInvoker that can hook in other ways by swapping out the backend.
These are intended to be used in the cases where compiler optimizations have made the functions un-hookable with the normal methods, e.g. if they have been merged with other (unrelated) functions.
Archengius
previously approved these changes
Dec 8, 2025
Member
Archengius
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me as long as the changes are well tested on both Win64 and Linux
…tring This is something I wanted to do before, but now I've got an actual excuse: the next submission will be adding a new handle type, which will need to store that debug name, and it needs to be as lightweight as possible. This technically breaks binary compatibility with vtable and UFunction hooks, but they haven't reached a release yet so that should be fine. A backwards-compatible export has been provided for standard hooks.
This is a replacement for the old UNSUBSCRIBE_* macros, which have now been removed. Unsubscribing is now a lot easier as the handle will remember the parameters for you, so you don't need to worry about them when unsubscribing. This will be necessary (instead of just nice to have) after a future submission which will add new parameters to virtual hooks that users can't repeat when unsubscribing. The signature of UnregisterHook on the backends is now locked in, instead of letting the backend add custom parameters, because the handle doesn't know about backend-specific stuff. Fortunately none of the backends have custom parameters so this isn't an issue.
Hooks are grouped by the function pointer that they're hooking. However with virtual functions you can have the same function pointer resolve to different functions, depending on the SampleObjectInstance that you give it. This has been worked around by ensuring that each virtual function hook has its own HookInvoker. This is mildly less efficient if the same module hooks the same virtual function multiple times, but that's the best that we can do.
…ethodReference If the MethodReference referred to a base class, but SampleObjectInstance was of a derived type, then the appropriate adjustment wasn't being applied to the pointer when finding the vtable. This has been resolved by changing the SampleObjectInstance parameter to match the class referred to by MethodReference, instead of just using a void pointer, so that the adjustment will be done for us.
I'm going to be making some changes to that core logic and I didn't want to have to do it twice...
…nce is involved Before we could have cases where a hook thought it had a pointer to a derived class, when it fact it pointed to a base class, so the pointer was effectively garbage. Crimes have been committed to make this work in a way that's backwards compatible, apologies for the pain and suffering this has caused.
Author
|
I've updated the PR with a bunch of other bug fixes that we discussed recently. There's one more bug fix which is waiting on the change to AssemblyAnalyzer (satisfactorymodding/AssemblyAnalyzer#1) |
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.
Tests added in a PR here: satisfactorymodding/SMLFeatureTests#3