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

Generics in general are funky #407

Open
Balint66 opened this issue Jun 16, 2021 · 3 comments
Open

Generics in general are funky #407

Balint66 opened this issue Jun 16, 2021 · 3 comments
Assignees
Labels

Comments

@Balint66
Copy link

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:

@pardeike
Copy link
Owner

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.

@Balint66
Copy link
Author

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!

@pardeike
Copy link
Owner

pardeike commented May 13, 2022

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.

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

No branches or pull requests

2 participants