Skip to content
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

harmony is not compatible with .net 8 . #543

Closed
BQFiruz opened this issue Aug 23, 2023 · 16 comments
Closed

harmony is not compatible with .net 8 . #543

BQFiruz opened this issue Aug 23, 2023 · 16 comments
Assignees

Comments

@BQFiruz
Copy link

BQFiruz commented Aug 23, 2023

hi
i test harmony in .net 7 and its work.
i test harmony in .net 8 and its give error.

@pardeike
Copy link
Owner

What?

@BQFiruz
Copy link
Author

BQFiruz commented Aug 23, 2023

error message is : Object reference not set to an instance of an object.
stacktracer is : at MonoMod.Utils.DMDEmitDynamicMethodGenerator._Generate(DynamicMethodDefinition dmd, Object context)

my code run in the .net 7 but if change .net version to 8 casuses raise error

@pardeike
Copy link
Owner

Not enough information. Also: is NET 8 official yet?

@BQFiruz
Copy link
Author

BQFiruz commented Aug 28, 2023

i use folowing code :

using Xunit;
using HarmonyLib;
using System.Reflection;
public class HarmonySimpleTester
{
    private static bool PrefixMethodCalled = false;
    private static object? PrefixMethodInstance = null;
    private static MethodBase? PrefixMethodOriginalMethod = null;
    [Fact]
    public void PropertySetMethodTester()
    {
        var prefix = typeof(HarmonySimpleTester).GetMethod(nameof(PrefixMethod), BindingFlags.Public | BindingFlags.Static);
        var prefixMethodHarmonyMethod = new HarmonyMethod(prefix);

        var type = typeof(TestClass);
        var idProperty = type.GetProperty("ID");
        Assert.NotNull(idProperty);

        var idSetMethod = idProperty.SetMethod;
        Assert.NotNull(idSetMethod);

        var harmony = new Harmony($"TEST{Guid.NewGuid().ToString().Replace("-", "")}");
        var patchedMethod = harmony.Patch(idSetMethod, prefixMethodHarmonyMethod);
        Assert.NotNull(patchedMethod);

        var testInstance = new TestClass();
        PrefixMethodCalled = false;
        testInstance.ID = 1;
        Assert.True(PrefixMethodCalled);
        Assert.Equal(testInstance, PrefixMethodInstance);
        Assert.Equal(idSetMethod, PrefixMethodOriginalMethod);
    }
    public static bool PrefixMethod(object __instance, MethodBase __originalMethod)
    {
        PrefixMethodCalled = true;
        PrefixMethodInstance = __instance;
        PrefixMethodOriginalMethod = __originalMethod;
        return true;
    }
    class TestClass
    {
        public int ID { get; set; }
        public string Name { get; set; } = string.Empty;
        public int? Age { get; set; }
        public string? Father { get; set; }
    }
}

its ok in .net 7 but has error in .net 8
error in .net 8 :
image
image
image

if upgrade to 2.3.0-prerelease.2 change errors :
error message is : Object reference not set to an instance of an object.
stacktracer is : at MonoMod.Utils.DMDEmitDynamicMethodGenerator._Generate(DynamicMethodDefinition dmd, Object context)

@pardeike
Copy link
Owner

Microsoft writes about NET 8:
Preview releases provide early access to features that are currently under development. These releases are generally not supported for production use.

@zabulus
Copy link

zabulus commented Sep 18, 2023

I'm currently testing using dotnet 8 GA and experiencing following exception:

System.InvalidOperationException: Cannot find returnType fieeld on DynamicMethod
   at MonoMod.Utils.DMDEmitDynamicMethodGenerator..cctor()

call stack:

static DMDEmitDynamicMethodGenerator()
DMDEmitDynamicMethodGenerator.GenerateCore()
DMDGenerator<DMDEmitDynamicMethodGenerator>.Generate()
DynamicMethodDefinition.Generate()
DynamicMethodDefinition.Generate()
MethodPatcher.CreateReplacement()
PatchFunctions.UpdateWrapper()
PatchProcessor.Patch()
Harmony.Patch()

Dependencies:

    <PackageVersion Include="MonoMod.Core" Version="1.0.1" />
    <PackageVersion Include="MonoMod.Utils" Version="25.0.3" />
    <PackageVersion Include="Lib.Harmony" Version="2.3.0-prerelease.2" />

I looks like MonoMod doesn't support dotnet 8, and according to this MonoMod/MonoMod.Common#31 dotnet 7 either.

@blakepell
Copy link

blakepell commented Nov 16, 2023

Can confirm this issue with .NET 8 RTM (both with the current release and the pre-release). I'm running a basic Patch command, nothing fancy, not much else is in the stack other than the exception message shared above. Reverting to .NET 7 fixes the issue.

@pardeike
Copy link
Owner

This issue here won't do anything good. You guys need to open one in the MonoMod project which is used by Harmony to execute the patches.

@Fasjeit
Copy link

Fasjeit commented Nov 17, 2023

The existing Dotnet 8 related issue in MonoMod - MonoMod/MonoMod#136

upd: and another one - MonoMod/MonoMod#149

@AlexBar
Copy link

AlexBar commented Nov 21, 2023

Looks like the fix there MonoMod/MonoMod#150 Waiting for it to be merged and released.

@misandrie
Copy link

Fix is merged, this can be closed

@pardeike
Copy link
Owner

I close it when I have verified and got someone else to verify it too

@karanshah-browserstack
Copy link

@pardeike Any update when can we expect this to be released with the fix Monomod provided?

@nefarius
Copy link

nefarius commented Dec 5, 2023

There hasn't been a new MonoMod release yet, just because it's merged doesn't mean it's in a release candidate state. Patience.

@pardeike
Copy link
Owner

pardeike commented Dec 5, 2023

@pardeike Any update when can we expect this to be released with the fix Monomod provided?

Right now I don’t have any ETA because Harmony has a blocking issue with building the right kind of release. On the one hand backwards compatibility is essential but previously merged types from Cecil and MonoMod.Common are no longer available or have changed. On the other hand users demand that Harmony no longer merges dependencies because of type conflicts so a thin version is necessary. Finally, for certain applications a fat Harmony in a single DLL is still the best solution.

This is a rather complicated problem that doesn’t get easier if you also have the problem that merging in the project for all architectures and .NET versions in the CI is tricky. I had someone helping me before the summer but that person has not been available since and my experience is limited.

Until we solve this in a professional way this blocks the release (and of course MonoMod.Core must first create a NET 8 release).

@misandrie
Copy link

misandrie commented Dec 17, 2023

Can confirm some degree of support for .NET 8. I have forked harmony and have retargeted it to use net8.0.

Code: https://bin.gy/curtherchi
harmony.log.txt: https://bin.gy/pardessuri
Output:

Running 8.0.0
test
True
True
PATCHED: test
False
False

I have not tested Reverse Patching as I personally had no use case for it at the current moment.
Using Mono.Cecil 0.11.5, MonoMod.Core 1.1.0-prerelease.1

In testing for a personal project I have not yet found any new issues arising from the retarget. In fact patches compiled with .NET 7 for some reason work with .NET 8.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants