In Dyson Sphere Program 0.7.x targeting multiple methods not functional? about harmonyx HOT 7 CLOSED

Did a bit of testing, and can confirm this is an issue in latest HarmonyX with a minimal example DSPMultiplePatchTest.zip
but there is a very notable distinction to make, which can be seen by looking at the patch classes.

    [HarmonyPatch(typeof(UIEscMenu))]
    public static class PatchesFail
    {
        [HarmonyPrefix]
        [HarmonyPatch(nameof(UIEscMenu.OnButton5Click))]
        [HarmonyPatch(nameof(UIEscMenu.OnButton6Click))]
        public static void TestPrefix(MethodBase __originalMethod)
        {
            Class1.Logger.LogMessage($"This should fail for Quit Game (5) or Exit to Desktop (6): {__originalMethod.Name}");
        }
    }
    
    [HarmonyPatch]
    public static class PatchesSucceed
    {
        [HarmonyPrefix]
        [HarmonyPatch(typeof(UIEscMenu), nameof(UIEscMenu.OnButton5Click))]
        [HarmonyPatch(typeof(UIEscMenu), nameof(UIEscMenu.OnButton6Click))]
        public static void TestPrefix(MethodBase __originalMethod)
        {
            Class1.Logger.LogMessage($"This should succeed for Quit Game (5) and Exit to Desktop (6): {__originalMethod.Name}");
        }
    }

The top fails, the bottom succeeds. This leads me to believe the error is somewhere in attribute merging, but I haven't gotten to narrowing it down yet.

from harmonyx.

Going to confirm here: the multitargeting syntax @Windows10CE shows in PatchesFail was never intended for class patches.
While Harmony 2 does allow you to write class patches as shown in https://harmony.pardeike.net/articles/annotations.html#combining-annotations, it does not allow you to write multiple HarmonyPatch attributes on the same method.

The syntax shown in the PatchesSucceed is the new additional syntax for HarmonyX that was actually intended for everyone's use if you use method patching. If you specify multiple HarmonyPatch attributes that include both type and method name, HarmonyX does not merge them but instead treats them as separate targets. Those are so-called "complete" patch targets.

Nevertheless, I don't see why one couldn't allow multitargeting in class patches provided that at least one of the two holds:

• There is [HarmonyPatch(typeof(Foo))] attribute on the type
• There is [HarmonyPatch(typeof(Foo), "Name")] attribute on the method

I'll add this feature, test it and close the issue once it's fixed.

from harmonyx.

Alright, I've managed to track down where this came from, although I'm not entirely sure whether or not it's an actual bug or just poorly documented. The behavior comes from this line, where it confirms that each attribute is "complete" as defined by https://github.com/BepInEx/HarmonyX/wiki/Multitargeted-patches .

I'm not sure how this worked in earlier versions of DSP, maybe it's been broken the whole time and just didn't throw an NRE previously? Whether or not it's a bug to try and be fixed is up to @ghorsington rather than me, if it's to be left as is I'll probably add more emphasis to the multi-patch wiki page about each attribute needing to be complete on it's own, with nothing coming from attributes on the patch class.

from harmonyx.

I'm not sure how this worked in earlier versions of DSP, maybe it's been broken the whole time and just didn't throw an NRE previously?

It definitely runs in 0.6.x, I just double-checked by throwing a log statement in there and it prints as expected.

If this isn't intended behavior it's fine then, but I think the format makes more sense than having to specify the type in each line and it worked well up until now.

from harmonyx.

The top version working was unintended I believe, the assumption always was that you provide all information in single attributes when multi-targeting.

from harmonyx.

Just reporting back because I didn't realize where that format came from, but it's actually from Harmony's documentation:
https://harmony.pardeike.net/articles/annotations.html#combining-annotations

from harmonyx.

The documentation is for the per-assembly patching method which is used in stock harmony, not per-type patching which is unique to harmonyx.

from harmonyx.