Fexty12573 / SharpPluginLoader

A C# plugin loader for Monster Hunter World
MIT License
32 stars 2 forks source link

Hook Source Generator #31

Open Fexty12573 opened 5 months ago

Fexty12573 commented 5 months ago

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.

GreenComfyTea commented 5 months ago

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 commented 5 months ago

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 commented 5 months ago

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