pardeike / Harmony

A library for patching, replacing and decorating .NET and Mono methods during runtime
https://www.patreon.com/pardeike
MIT License
5.15k stars 485 forks source link

"ref" keyword not allowed for "__runOriginal" #617

Closed garrettluskey closed 2 months ago

garrettluskey commented 2 months ago

IMPORTANT: Do NOT use this form to send support questions. If you send things like "How do I do X" or "It does not patch Y" your submission will be closed and you will be forwarded to the official Harmony Discord channel anyway.

OFFICIAL DISCORD: https://discord.gg/xXgghXR

If you think you found a bug in Harmony please use the following to format:


Describe the bug A clear and concise description of what the bug is.

To Reproduce Steps to reproduce the behavior:

internal class ThisIsATest
{
    public void SomeMethod()
    {
        // DO SOMETHING IDK
    }
}

[HarmonyPatch(typeof(ThisIsATest), nameof(ThisIsATest.SomeMethod))]
internal class ThisIsATestPatch
{
    public static bool Prefix()
    {
        // This can be anything
        return true;
    }

    public static void Postfix(ref bool __runOriginal)
    {
        // This crashes
    }
}

Stack Trace

Message: 
HarmonyLib.HarmonyException : Patching exception in method System.Void GameInterface.Services.MobileParties.Patches.ThisIsATest::SomeMethod()
---- System.InvalidProgramException : Common Language Runtime detected an invalid program.

  Stack Trace: 
PatchClassProcessor.ReportException(Exception exception, MethodBase original)
PatchClassProcessor.Patch()
<>c.<PatchAllUncategorized>b__12_1(PatchClassProcessor patchClass)
CollectionExtensions.Do[T](IEnumerable`1 sequence, Action`1 action)
CollectionExtensions.DoIf[T](IEnumerable`1 sequence, Func`2 condition, Action`1 action)
Harmony.PatchAllUncategorized(Assembly assembly)
----- Inner Stack Trace -----
PatchFunctions.UpdateWrapper(MethodBase original, PatchInfo patchInfo)
PatchClassProcessor.ProcessPatchJob(Job job)

Expected behavior Expected no crash or better error to describe what the issue was.

Screenshots / Code If applicable, add material to help explain your problem.

Runtime environment (please complete the following information):

Additional context Add any other context about the problem here.

Error message isn't that helpful, this issue isn't very serious but could lead some people down an unnecessary rabbit hole.

I'll try to be active on this issue :)

pardeike commented 2 months ago

Have you tried with “ref”? It is of no use since you cannot and should not change the value of the injected value in this case.

garrettluskey commented 2 months ago

This issue is passing ref to the postfix causes the error described above. It appears that adding the "ref" keyword breaks harmony on patching.

pardeike commented 2 months ago

Then don’t add ref. As I said it makes no sense in this case anyway.

garrettluskey commented 2 months ago

Ok, I understand now. What I am suggesting is that the error could be more verbose when passing __runOriginal by reference. If you feel it doesn't add enough value feel free to close this issue

pardeike commented 2 months ago

Seems too esoteric an error to fix. To me it seems obvious that a Boolean you only suppose to read should not use ref at all. Since it’s actually non-trivial to add the error I rather not.