Harmony icon indicating copy to clipboard operation
Harmony copied to clipboard

Generics in general are funky

Open Balint66 opened this issue 4 years ago • 3 comments

Describe the bug I have a generic class (and it's child classes) that I'd like to patch. Not every child class overrides the method that I'm patching, so I'm modifying the base generic class. However, only the last patch is executed and "alive" but no error is thrown.

To Reproduce Steps to reproduce the behavior:

  1. Create a generic class with a method (does not need to include the generic type in the method)
  2. Patch the code with a post method.
  3. No error, but if patched with multiple types, only the last one is executed

Expected behavior Every patch called with the type that was specified

Screenshots / Code The patch:

  public static class RepositoryPatch<T, M> where T : Repository<T, M> where M : NSEipix.Base.Model
	{

		public delegate void onAction(T repo);
		
		public static event onAction PostDeserialization;
		
		private static void DeserializePost(JsonRepository<T, M> __instance)
		{
			if ( PostDeserialization != null && __instance is T repo )
			{
				PostDeserialization(repo);
			}
		}

		public static void ApplyPatch(Harmony harmony)
		{
			
			var origDeser = typeof(T).GetMethod("Deserialize", BindingFlags.Instance | BindingFlags.NonPublic);

			if ( origDeser == null || origDeser.IsVirtual )
			{
				Logger.Instance.info("No specific deserializer implementation for this repository. Falling back to JsonRepository (this is normal)");
				origDeser = typeof(JsonRepository<T, M>).GetMethod(
					"Deserialize", BindingFlags.Instance | BindingFlags.NonPublic);
			}
			
			var deserPost = typeof(RepositoryPatch<T, M>).GetMethod("DeserializePost", BindingFlags.Static | BindingFlags.NonPublic);
			
			harmony.Patch(origDeser, postfix: new HarmonyMethod(deserPost));
			
			Logger.Instance.info("The repository patching <"+typeof(T).Name + ", " + typeof(M).Name + "> was done.");

		}

	}

where the patches are applyed:

            try
            {
                var harmony = new Harmony("com.modloader.nsmeadival");
                    
                MainMenuPatch.ApplyPatch(harmony); // Works fine
                RoomTypePatch.ApplyPatch(harmony);// Works fine
                RoomTypeTooltipViewPatch.ApplyPatch(harmony);//Works fine
                RoomViewPatch.ApplyPatch(harmony);// Works fine
                RepositoryPatch<CropfieldRepository, Cropfield>.ApplyPatch(harmony);//No error, doesn't get applied
                RepositoryPatch<ResearchRepository, ResearchModel>.ApplyPatch(harmony);//No error, doesn't get applied
                RepositoryPatch<RoomTypeRepository, RoomType>.ApplyPatch(harmony);// Works fine
            }
            catch (Exception e)
            {
                Logger.Instance.info("Error happened while loading patches.\n" + e);
                throw;
            }

Runtime environment (please complete the following information):

  • OS: Windows 10 Enterprise build:19042.1052
  • .NET version: v4.6.2
  • Harmony version: 2.0.4.0
  • Name of game or host application: Going Medieval

Additional context Changing the order of the method calls does affect which patch is called. Maybe related issues:

  • #391
  • #325

Balint66 avatar Jun 16 '21 10:06 Balint66

Unfortunately this will be a WONTFIX due to lack of low level documentation on the internals of the .NET JIT compiler and how it structures assembler trampolines and other arcane structures that compiling IL to assembler does.

Even if documentation would exist the sheer amount of customization for all combinations of .NET version, processor architecture and environment will make this very hard to implement.

pardeike avatar Jun 16 '21 11:06 pardeike

Well, this is truly unfortunate.

Then I'd have to use a workaround somehow. I think leaving this issue open is a good idea, so other's will see that it can not be fixed.

Thank you for your support, your time, and of course, the library.

Have a nice day!

Balint66 avatar Jun 16 '21 12:06 Balint66

Just a small comment on something that comes up from time to time: Using generics in patch methods is a really bad idea.

Just because it works in your compiler doesn’t mean that it works at runtime too. The compiled IL makes the JIT process do funky stuff to call or multiply generic methods which simply does not work when embedding a call to such a prefix/postfix inside a replacement method. And while it will look like it works for a single use case does not mean it will work for other or multiple patches.

pardeike avatar May 13 '22 22:05 pardeike