Harmony should support MS .NET and not only Mono. (was: Error trying to override int getter)

[pardeike]

Harmony should support MS .NET and not only Mono. (was: Error trying to override int getter)

[pardeike]

I prioritized Mono but that does not mean that with a few checks one couldn't add .NET compatibility. I think I address this in the future or let someone else do it in a pull request. I am pretty sure that it is possible to have one version of Harmony that supports both environments.

[pardeike]

Can you specify your settings? Which Mono version, OS or application? Did you use the release version of Harmony or the latest master? I agree, it should work.

It is a bit weird that your stack trace points to the creation of the DynamicMethod. At that time, nothing has been created or patched yet so it is not related to how you patch. I never had that error "Only public, static, standard supported". Sounds like your runtime is non-standard.

C:\repo\Harmony\Harmony\Tools\DynamicTools.cs:line 22

var method = new DynamicMethod(
	patchName,
	MethodAttributes.Public | (original.IsStatic ? MethodAttributes.Static : 0),
	CallingConventions.Standard,
	AccessTools.GetReturnedType(original),
	paramTypes,
	original.DeclaringType,
	true
);

You can clearly see that it is public and standard and in my environment it adding static or not does not matter. A dynamic method is by nature static with a possible instance parameter as the beginning. One thing you could to, is to test if you succeed by removing the optional "original.IsStatic" and always have "MethodAttributes.Public | MethodAttributes.Static" instead:

var method = new DynamicMethod(
	patchName,
	MethodAttributes.Public | MethodAttributes.Static,
	CallingConventions.Standard,
	AccessTools.GetReturnedType(original),
	paramTypes,
	original.DeclaringType,
	true
);

Let me know your findings,
Andreas

[llaughlin]

Well, I'm not using Mono, and that may be the problem. I'm using a unit test framework on .NET 4.5.1, but I thought it might work the same.

[pardeike]

Yes, it might even be that it works different outside of unit testing. Anyway, you should really try to do the edit I suggested and see if that works.

[llaughlin]

Hey, that seems to have worked partially! I now get an error in DynamicTools.cs:line 75. It's not finding the CreateDynMethod method on the DynamicMethod class. Inspecting it, I couldn't find that method either. It may be due to a change in the structure of DynamicMethod past .NET 3.5. I'll continue digging.

UPDATE:

Just realized that the .NET version shouldn't be changing, and that Harmony is still targeting 3.5, so it shouldn't have failed to find that method. Thoughts?

Update 2:
Pulled latest master, still seeing this behavior.

[llaughlin]

Should the line be

typeof(DynamicTools).GetMethod("CreateDynamicMethod", BindingFlags.NonPublic | BindingFlags.Instance);

instead of

typeof(DynamicMethod).GetMethod("CreateDynMethod", BindingFlags.NonPublic | BindingFlags.Instance);

?

[pardeike]

No. That line actually gets a private method inside DynamicMethod to compile it. Pretty certain it is Mono specific.

[llaughlin]

Well, that might be the problem then :P. I wonder if I can find something suitable to use outside of Mono...

[dr4cul4]

In .NET CreateDynMethod doesn't exist in DynamicMethod class and I see no similar construct. But even if you surpass this problem then you get against next wall which is native calls to mono libs. At this point I just gave up.

I rally would like to use this library, shame.

[pardeike]

In my early experiments, I had a different strategy to force creation of the dynamic method: I simply "pre-run" them by calling them. Since it's a dynamic method one could add a small IL header to the method that simply returns directly and then call into your newly build method. The trick is then to find the right start point in the resulting assembler code after that immediate return. Use that as the destination of the assembler jump from the original method and you're done.

Alternatively, one could add IL code using a static "firstTime" bool to do the same. This will remove the need to find a different start in the assembler but at the cost of an extra if-check which makes it a slower approach.

Finally, the mono calls are just for getting the jit-ed assembler code starting location. I think that this can be done in MS.NET too but in a different way.

[pardeike]

Please try the latest changes that were merged into the master and report back any findings.

[antonberezan]

@pardeike it works now for me, thanks!

[pardeike]

Excellent!

[bcmpinc]

When will this fix be released?

[pardeike]

Releases are rather sparse because they require a very good quality level. Mostly because of other applications and communities depend on a stable version.

I am working towards a v1.1 release within the coming weeks.