Skip to content

Hook Source Generator#31

Draft
Fexty12573 wants to merge 2 commits intomasterfrom
hook-generator
Draft

Hook Source Generator#31
Fexty12573 wants to merge 2 commits intomasterfrom
hook-generator

Conversation

@Fexty12573
Copy link
Owner

This PR introduces a new source generator to the project, which will live inside its own NuGet package. The source generator aims to improve how function hooking is done in plugins. Currently, the hooking API looks as follows:

private delegate float MyHookDelegate(nint obj, float x, int y, bool z);
private Hook<MyHookDelegate> _myHook = null!;

public void OnLoad()
{
    _myHook = Hook.Create<MyHookDelegate>(/* address */, (obj, x, y, z) =>
    {
        var result = _myHook.Original(obj, x, y, z);
        return z ? (result * x) : (result / x);
    });
}

There is basically zero marshalling that is performed, save for some very basic types like structs and strings. The MarshallingHook class, found in the SharpPluginLoader.Core.Experimental namespace aimed to alleviate this a little by also allowing hook functions to take in (and return) any type that inherits from SharpPluginLoader.Core.NativeWrapper. However, that class has a few issues:

  1. It uses runtime code-generation, often inefficient.
  2. That runtime generated code is very unstable (Hence, it lives in the Experimental namespace).
  3. The only additional benefit you get out of it is marshalling for NativeWrapper.
  4. The drawback is always 2 extra indirections for each hook.
  5. Runtime-generated code is very hard to debug.

Additionally, the current hooking API just requires a lot of boilerplate code.

SharpPluginLoader.HookGenerator

This new source generator aims to solve all but one of these issues completely. The new hooking API looks as follows:

[HookProvider]
public class Plugin : IPlugin
{
    [Hook(Address = /* address */)]
    public float MyHook(MtObject obj, float x, int y, bool z)
    {
        var result = _myHook.Original(obj, x, y, z);
        return z ? (obj.Get<float>(0x10) * x / result) : (x * result * 1.5f);
    }
}

The Hook attribute marks a function as a hook (duh). It is structurally very similar to the InternalCall attribute, in that you can specify either an absolute address, or an AOB with an offset.

Benefits

The source generator does a few things for you:

  1. Generate the required boilerplate code (like the delegate, hook object, etc):
  2. Automatically generates a forwarder, if necessary, to provide extensive levels of marshalling
  3. Forwarder is only generated if anything actually needs marshalling
  4. Automatic initialization on plugin load
  5. Automatic cleanup on plugin unload

Limitations

Due to how initialization is handled, there are a few limitations on where, and how, hooks can be placed.

  • Any class that contains hooks must be marked using the HookProvider attribute.
  • Any hooks that are not inside the main plugin class (the one that implements IPlugin) are required to be static.

@GreenComfyTea
Copy link

GreenComfyTea commented Mar 18, 2024

Let me express one of the concerns that I have. It applies to the previous hooking API, but I don't know if this aspect has been addressed in the new one.

Basically, automatic marshalling can lead to crashes when the game calls functions with junk parameters (idk why would it do that).

As an example - AddRequestLobbyListNumericalFilter. As I mentioned before, it gets called a lot in online sessions when you are not the host. It also gets called before entering main menu with key address = 4. This is not an accessible memory address.

image

The game crashes with AccessViolationException error regardless if I have automatic marshalling or manual.
private delegate void numericalFilter_Delegate(nint steamInterface, string key, int value, int comparison);

private delegate void numericalFilter_Delegate(nint steamInterface, nint keyAddress, int value, int comparison);

NumericalFilterHook = Hook.Create<numericalFilter2_Delegate>(numericalFilterAddress, (steamInterface, key, value, comparison) =>
{
	var key = MemoryUtil.ReadString(keyAddress);
	NumericalFilterHook!.Original(steamInterface, keyAddress, value, comparison);
});

Currently, I circumvented it by filtering out irrelevant calls before manual marshalling.

private delegate void numericalFilter_Delegate(nint steamInterface, nint keyAddress, int value, int comparison);

NumericalFilterHook = Hook.Create<numericalFilter2_Delegate>(numericalFilterAddress, (steamInterface, key, value, comparison) =>
{
	if(steamInterface != SteamMatchmakingInterface || CurrentSearchType == SearchTypes.None) 
	{
		NumericalFilterHook!.Original(steamInterface, keyAddress, value, comparison);
		return;
	}

	var key = MemoryUtil.ReadString(keyAddress);
	NumericalFilterHook!.Original(steamInterface, keyAddress, value, comparison);
});

Idk if the issue is fixable SPL-side thou.

@Fexty12573
Copy link
Owner Author

Fexty12573 commented Mar 21, 2024

The issue you're describing it not something I can fix by revamping the hooking API. What is and isn't valid data is up to the user to decide, not the API. The new API will respect nullptrs and marshal those as C# null values accordingly, but ultimately if some garbage value is passed to the function by the game, there's not much I can do about it. The user needs to handle that.
This should also only happen in ecceedingly rare circumstances so I wouldn't say it's a common issue.

The one thing that could address this is IsBadReadPtr, but that function is pretty slow and I don't want to make use of it in the framework itself. (Or in your case more accurately, IsBadStringPtr)

@GreenComfyTea
Copy link

GreenComfyTea commented Mar 21, 2024

Understandable! 👍 But mentioning it in the wiki so future modders knew would be nice.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants