Prefix invocation is not skipped by returning false

[jeiea]

What I did

It was not reproduced with Harmony 2.0.1 and also with BepInEx 5.0.1.0.

public class TargetClass {
  public TargetClass() {
    Start();
  }

  void Start() {
    Debug.Log("well..");
  }
}

public class Patcher1 {
  public static void Patch() {
    var harmony = new Harmony("patcher1");
    PatchWithHarmony(harmony);
  }

  private static void PatchWithHarmony(Harmony harmony) {
    string methodName = nameof(Patcher1.SomePrefix);
    BindingFlags flags = BindingFlags.NonPublic | BindingFlags.Static;
    MethodInfo prefix = typeof(Patcher1).GetMethod(methodName, flags);

    string targetMethodName = "Start";
    BindingFlags targetFlags = BindingFlags.NonPublic | BindingFlags.Instance;
    MethodInfo start = typeof(TargetClass).GetMethod(targetMethodName, targetFlags);
    harmony.Patch(start, prefix: new HarmonyMethod(prefix));
  }

  private static bool SomePrefix() {
    Debug.Log("I'm patcher 1");
    return false;
  }
}

public class Patcher2 {
  public static void Patch() {
    var harmony = new Harmony("patcher2");
    PatchWithHarmony(harmony);
  }

  private static void PatchWithHarmony(Harmony harmony) {
    string methodName = nameof(Patcher2.SomePrefix);
    BindingFlags flags = BindingFlags.NonPublic | BindingFlags.Static;
    MethodInfo prefix = typeof(Patcher2).GetMethod(methodName, flags);

    string targetMethodName = "Start";
    BindingFlags targetFlags = BindingFlags.NonPublic | BindingFlags.Instance;
    MethodInfo start = typeof(TargetClass).GetMethod(targetMethodName, targetFlags);
    harmony.Patch(start, prefix: new HarmonyMethod(prefix) {
      before = new[] { "patcher1" },
    });
  }

  private static bool SomePrefix() {
    Debug.Log("It should block patcher1");
    return false;
  }
}

public class PatcherTester {
  public static void Test() {
    Patcher1.Patch();
    Patcher2.Patch();
    new TargetClass();
  }
}

What I expected

[Info   : Unity Log] It should block patcher1

What I got

[Info   : Unity Log] It should block patcher1
[Info   : Unity Log] I'm patcher 1

Environment

BepInEx v5.1
.NET framework 3.5
Unity 2017

[ManlyMarco]

This is the expected behavior in new versions. Prefix patches preventing other prefix patchers from running have been found to be very error prone and nondeterministic in more complex setups. This combination caused seemingly random crashes in plugins that did not expect their prefix patches to not run and was very difficult to diagnose and fix (different users could have the patches ordered differently because of different plugin load orders, so for some users it worked fine while it broke for others). It is also not reasonable to expect a plugin authors to know all other plugins that patch a particular method, therefore the decision was to make this change.

If you need to prevent a certain plugin's prefix patch from running the recommended way is to patch the patch itself.

[ghorsington]

Greetings!

As Marco already pointed out, this behaviour is intended to be fully symmetrical with how postfixes operate.

We've had a plan on exposing the __skipOriginal boolean to prefixes and postfixes instead so that prefixes themselves can decide whether skip their execution or not.

Seeing as this is closely related to that feature, I'll reopen this issue and close it again once bool __skipOriginal parameter support is in.

Related, I've made #8 which I plan on addressing this week so that all possible differences from original Harmony is known.