-
Notifications
You must be signed in to change notification settings - Fork 43
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
Patch does not run if specific other patch exists #71
Comments
Try putting [HarmonyDebug] attributes on your outer prefix and see what the IL comes out like. |
Thanks for your suggestion @ManlyMarco. It seems that the method is unchanged (as you'd expect with an empty prefix). If someone reported this to me I'd say they were crazy and there must be different circumstances that caused the inner method not to be called. Yet here we are, I can reproduce this behaviour with 100% consistency :( Working with IL isn't my favourite hobby (another rabbit hole!) but I suppose the next step is diving in with a transpiler, to figure out what the outer method is doing exactly? If that can be avoided I'm all for it.. The original method is a bit larger than the pseudo code example I provided, and it does some checks before calling the inner method. However, the mere existence of another patch surely wouldn't affect that.. |
Small update: with a crazy work-around I've been able to determine that the method I want to patch does run (in short: I was able to check for a value that it sets). So it really is just the patch that does not run. (not entirely coincidentally I can use this same work-around to achieve what I wrote the patch for, so for my mod the issue is solved I think) |
The plot thickens.. If I change the order in which I write the patches, both work. In this example, only the outer patch is called: // this is the outer method
[HarmonyPatch(typeof(Player), nameof(Player.PlacePiece))]
static class Player_PlacePiece
{
static void Postfix()
{
// This runs
Log.Debug("Player.PlacePiece.Postfix");
}
}
// this is the inner method
[HarmonyPatch(typeof(WearNTear), nameof(WearNTear.OnPlaced))]
static class WearNTear_OnPlaced
{
static void Postfix()
{
// This does not run
Log.Debug("WearNTear.OnPlaced.Postfix");
}
} But here, the other way around, both run: // this is the inner method
[HarmonyPatch(typeof(WearNTear), nameof(WearNTear.OnPlaced))]
static class WearNTear_OnPlaced
{
static void Postfix()
{
// This runs!
Log.Debug("WearNTear.OnPlaced.Postfix");
}
}
// this is the outer method
[HarmonyPatch(typeof(Player), nameof(Player.PlacePiece))]
static class Player_PlacePiece
{
static void Postfix()
{
// This runs
Log.Debug("Player.PlacePiece.Postfix");
}
} Is there a way I can control which patch is applied first, bearing in mind that the outer patch might come from another mod that I have no control over? |
Can you try the same but with type-wide patching instead of assembly-wide? https://github.com/BepInEx/HarmonyX/wiki/Patching-with-Harmony#attribute-based-patching-for-types |
Okay, I now run this: patcher.PatchAll(typeof(Player_PlacePiece)); // outer
patcher.PatchAll(typeof(WearNTear_OnPlaced)); // inner instead of patcher.PatchAll(); Is that what you meant? The outcome is the same - the outer patch runs, the inner patch does not. But if I swap the calls, both run: patcher.PatchAll(typeof(WearNTear_OnPlaced)); // inner
patcher.PatchAll(typeof(Player_PlacePiece)); // outer |
Thanks, @ghorsington will have to take a look at it. |
This sounds like it's just an inlining issue, if you patch the outer first, the method gets compiled before the inner patch can be applied to disable inlining for the inner method. Inlined methods effectively aren't patched, because the JIT uses the original IL to inline the method. If you patch the inner first, MonoMod disables inlining for the method permanently, meaning the inner method will no longer be inlined, and both patches will run. This issue will not be visible in IL, as the JIT is what does inlining. This is a bit of a wontfix on HarmonyX's part, it can't globally disable inlining even if it wanted to (that would kill performance anyway), and there's no way to know all methods that will need to be patched in advance. |
Thanks for the insight @Windows10CE So is there something I can do to apply the inner patch before another mod applies the outer patch? That would solve the issue for me. |
Easiest way? Get your mod loaded before the other mod, either by getting the other mod to add a SoftDependency on your mod, or having your mod get sorted higher alphabetically. Alternatively, you could write a preloader that adds the NoInlining flag to the inner method before the assembly even loads. |
The more I think about this, the less sense it makes in my head. Why does the inner method get inlined when a harmony patch exists on the outer method, but not if it doesn't? Getting other mods to add a soft dependency on my mod would be an impossible task. There are a plethora of mods that patch the outer method. It seems that in a game where you can build stuff, lots of mods are interested in the build event for some reason! :P Renaming my mod isn't great either as I would have to abandon the current mod and make a new one, risking my sanity as well as a loss in endorsements and users. Writing a preloader just because I want to know when a specific item is being built in a game seems like insane overkill, I'd like to avoid that approach at all costs. I did write a silly work-around which checks the result of the inner method after a small delay, and in that way I am able to "emulate" the circumstances under which the inner patch "would have" run. It's very ugly but I guess it'll have to do.
The only other thing for it, I suppose, would be to write "known issue" in the comment above my patch and move on with life. Edit: Working around this issue like so: var playerPlacePiecePatched =
Harmony.GetAllPatchedMethods().Where(m => m.DeclaringType.Name.Equals("Player") && m.Name.Equals("PlacePiece")).Any();
if (!playerPlacePiecePatched)
{
Log.Debug("Patching WearNTear.OnPlaced, yay!");
patcher.PatchAll(typeof(WearNTear_OnPlaced));
}
else
{
Log.Debug("Patching Piece.SetCreator, boo!");
patcher.PatchAll(typeof(Piece_SetCreator));
} Seems to get the job done when I patch the outer method myself. ( Now it doesn't matter if my mod was loaded first or not - so long as the outer method was not patched yet, patching the inner one will work. And once the inner method is patched, patching the outer method does not cause problems. |
I am also experiencing this problem. A year later. Same area of the code too. Player.PlacePiece and WearNTear.OnPlaced. |
I don't think this is going to be fixable by Harmony, at least not in the short term. The bug is created by the compiler's inlining, and so the function being patched no longer exists when the time comes for Harmony to apply its patches. Working around it seems the only way to cope with it for now. |
This sounds a bit weird to me.
BTW how does Unity Mono do JIT? |
Apologies in advance for the vague description. This is a mind boggling issue for me (I'm new to this) and I'm not even sure if Harmony is at fault. I'm hoping someone can point me in the right direction with this.
Consider this pseudo code:
If I patch
OneThing
, that works as expected. Prefix, postfix, finalizer, all work fine.Now, if I also patch
DoThings
, that works, but any patch forOneThing
will now be skipped. That is to say, the debug logs say that the method did get patched, but the code does not run. It does not matter if it is a prefix, postfix, or finalizer. None of them run.In fact, if the patch on
DoThings
is empty, like so:Obviously it will now not do anything, and yet it still breaks the other patch.
Is this expected behaviour?
The context in which I discovered this is the Unity game Valheim, and the bug (if it is one) happens in a mod I wrote, but was initially caused by another mod. Meaning, my mod had patched
OneThing
and that no longer worked because another mod had patchedDoThings
.How can I debug this and figure out what's actually going on "behind the scenes"?
The text was updated successfully, but these errors were encountered: