Ludeon Forums

HARMONY 1.2.0.1
the right way of patching code at runtime

Hi,

Harmony is a simple dll that you embed in your mods. It will allow you to add code to any method inside RimWorld or other mods. It even works if more than one mod patches a method and it is not a mod itself which means that your users don't have to install it.

You can find out more about it on GitHub. I made it completely open source and hope that we all together can continue to evolve it. It comes with some wiki documentation and there are many mods using it on GitHub too (HugsLib has it build in i.e.). It works on PC, Mac and Linux.

The github repository has a ready made version of the lib in dll form and you can either add it to your Assemblies folder (add reference and keep "Copy" on) or merge it with a tool like ILMerge into your own dll.
Please feel free to give it a try and come back with feedback.

Harmony on nuget: Lib.Harmony (https://www.nuget.org/packages/Lib.Harmony/)

Harmony on GitHub:
https://github.com/pardeike/Harmony/releases/tag/v1.2.0.1 (https://github.com/pardeike/Harmony/releases/tag/v1.2.0.1)
https://github.com/pardeike/Harmony/wiki (https://github.com/pardeike/Harmony/wiki)

Example 1:Camera+ on GitHub:
https://github.com/pardeike/CameraPlus (https://github.com/pardeike/CameraPlus)

Example 2:SameSpot on GitHub:
https://github.com/pardeike/SameSpot (https://github.com/pardeike/SameSpot)

Quick starter snippet including a patch to load a specific game on start:
https://gist.github.com/pardeike/12c457e135a42a28e068f2aba5337221 (https://gist.github.com/pardeike/12c457e135a42a28e068f2aba5337221)

Please also help spreading the word so we all finally get to a point where we don't sabotage each other with detours that create dependencies and force the user to load mods in certain orders (Harmony defines priorities and dependencies internally). I am available for any questions in this thread. We are about to incorporate this into all the major mods and general libs like HugsLib. I will personally try to advertise Harmony in the Unity forums because as it is now, Harmony isn't RimWorld specific.

Thanks for your feedback and support.

Cheers,
Andreas Pardeike

Very good, i will check library and contribute if i can.

It will be good place to leave reference to CCL and HugsLib that ultimately have same goal:
https://ludeon.com/forums/index.php?topic=16599.0
https://ludeon.com/forums/index.php?topic=28066.0

reference to BuildProductive code already present.

Thnx RawCode,

good idea.
/Andreas

PS: a friend of mine had a access violation on his AMD 64bit running Win10 yesterday. He has the same problem with the Achtung mod which means that the AsmHelper might have a problem. I have tested on my MacBook and on an intel 64bit machine without problem.

there are number of issues with different platforms, first of all, portable and proper way to allocate RWX memory, i had suggested this before, i don't know why this was ignored, still, second try:

[DllImport("__Internal")]
static extern private unsafe void* mono_code_manager_new();

//this method will allocate memory with RWX permissions
//since you have exclusive access to that memory, no reasons to follow commit procedure
[DllImport("__Internal")]
static extern private unsafe void* mono_code_manager_reserve(void* MonoCodeManager, int size);

static private void* u_MonoCodeManager;
static private void* u_RWXM;

static CodeManipulationUtils()
{
u_MonoCodeManager = mono_code_manager_new ();
u_RWXM = mono_code_manager_reserve (u_MonoCodeManager, sizeof(void*)*256);

Console.WriteLine ("RWX memory allocation " + (int)u_RWXM);
}
this is same routine (but with commit procedure omitted) used internally by runtime itself, it's not any better then current implementation, but at least portable without much additional code.

Looks incredibly promising!

Also looks very complicated, which means it's going to require extremely good documentation and a lot of testing. I'm definitely keeping an eye on this, but before I'd like to be able to at least get an idea of what's happening in the lib before I start using it. The current documentation doesn't really help there.

Sure. I totally understand your point of view Fluffy. I have a lot of internal documentation that probably is tl;dr for many people so I have not put any effort into preparing them.

The overall concept evolved around at least 10 different approaches which, by the nature of the problem, did not pan out in some way. At one point, I even added my own csharp compiler and had dynamic source code in the project. In the end, the current state is the result of lots of lessons learned.

So until I put up more details, I don't expect anybody to put anything into a something ;-)

But since the whole point is to build trust with the community, the only way forward is to put it out in the open and lab test it and gain insides. Raw already gave me a nice replacement for the memory allocation that I am about to test with my friend.

So for now, let me sketch the basic idea behind Harmony: Instead of having global state, I use the method that is to be patched (I call it the target method) to attach information. So I allocate memory, add a jump from the jit code to that area (just like the old Detour). There, I have a new jump to the replacement method (I call it a wrapper). In that memory area, I prefix the jump with a pointer to a byte array that deserializes to the patch information. Since every mod has its own class hierarchy this is necessary. The first one to patch a targe method allocates this, everyone else that comes later simply reads the patch information, adds to it and replaces the patch information with a new version.

This solves the global state problem. Now for the wrapper method. Since I want to keep the original intact, I copy the IL codes of the target method to a copy. I then construct the wrapper to call all prefix methods, then the copy, then the postfix methods. Every time a prefix or postfix is added, the patch information is rewritten and a new wrapper is created.

All the rest is just support stuff that is necessary to make this easy for the user. The critical and "unsafe" parts here are:

- memory allocation and basic assembler handling for the initial redirect

- creating a copy of the original in CIL

- constructing a wrapper in CIL that has the same signature as the original

Since I don't rely on subclassing and instead juggle arguments around, the whole thing becomes much more safe. It was a bit tricky to get the initial order of things working but once it works it works pretty flawless. Right now, it works with any kind of method unless inlined - I even got it to work with some anonymous subclass method where RimWorld originally defines a anonymous method to be used as a Predicate. Works great. Properties work too, just patch the "g(s)et_Property" method.

So what's on the todo list:

- I want this to work for constructors

- I want to add a replacer where you not only have prefix/postfix methods but also a way to replace methodbase A with B in the original method (since all IL codes are copied this is really trivial as long as A and B have the same call signature). Here, multiple mods patching could register their replacers which would work in a chain:

original calls x.foo() and foo gets passed to replacer1, replacer2 replacer3 who all have their chance to replace it with something else and in the end x.bar() is written. This opens up great ways to instantiate subclasses instead of the original inside the target method. If one would like to go crazy, a general il code processor could be added thus allowing you to modify the original in any crazy way. Of course, totally overkill and quite dangerous.

And hopefully you guys come up with other needed stuff to implement.
/Andreas

Apropos testing. I wrote unit tests for everything I could. The problem with VS is that unit tests run in at least .Net 3.5 which screws things up - you're basically not testing the dynamic method things in the environment of the original Unity application. So if someone comes up with a good way to run those tests properly, I will make the test coverage 100% for everything.

"C:\Program Files (x86)\Mono\bin\mono" --debug "D:\User\Desktop\MonoRuntimeTest\bin\Release\MonoRuntimeTest.dll"

this "command line" used by me to execute code with specific mono runtime, in addition, mono.dll and mscorlib.dll are replaced by files provided with game, this allows to test code with "same runtime" without running game itself.
No matter how hard VS try, you always will run properly by similar command line.
OFC you will need mono version 2 installed and files swapped.

as for constructors, first runtime calls allocator, that reserve memory and setup object vtable and then calls constructor as non virtual instance method.

Constructors created by "normal" compiler always calls base class constructors, it's "CLI" feature, still you can emit code that violate this.
Constructors not any special by itself.

ok, that sounds good.

Method replacement would definitely be needed, both because it's required is some cases, and because it would make for a drop-in replacement.

I'm not entirely sure allowing multiple mods to replace the same method is a great idea, considering they are quite likely to both call base behaviour that would then be duplicated. But that's an implementation detail.

I'm very much looking forward to being able to attach what are essentially events to whatever methods I want - that opens up a world of possibilities (and sooooo many potential obscure bugs that it scares me, but with great power comes great responsibility I suppose :P)

I don't see the difference between acting on an event and i.e. tail patching a method that runs during that event.

If you see a method you patch as a node in a call graph, you can always choose a different level to add you head or tail patch so you never need to call any base methods. In the end it is up to the responsibility of the person writing the code to not leave any side effects. If your code has desired side effects and another mod has overlapping features it is always difficult to solve the conflict.

Still, not replicating the whole methods code to change a small area is a huge step forward and a structured way to learn about other mods patching the area you self want to change will improve things too.

Event based code has its downsides too. For this, a common lib like hugslib is probably the best solution.

And btw: a Harmony prefix can be defined with very high priority and then return false - effectively canceling the original method or any other prefixes that otherwise would run. Postpatches run always but I am open for discussion on this.

It's about "number of features" vs "ease of use".
"singular method replacements" vs "lack of conflicts".

priority settings is illusion of "conflictless", we can make any number of priority settings, still, as soon as two mods try to register top and singular injections, one of them (or both) will fail.

Good reflections Raw,
And I think the only possible solution is to increase transparency so that mod A and B get the tools to discover the conflict and then cooperate to solve it. Ultimately, two mods doing the same thing different will always collide.

Btw, the change to your suggested memory allocation just gave my friend a

KERNELBASE.dll caused an Access Violation (0xc0000005)
  in module KERNELBASE.dll at 0023:7464e512.

Not when patching but when the patch is then executed. :/
/Andreas

Attempting to use the example Camara+ Gives me an kernel.dll access violation at the same address. Logs and dump attached

[attachment deleted by admin due to age]

I got some great help with the cross platform stuff and will put up a new release tonight that should work on PC, Mac and Linux in 32 and 64 bits. I'll post it here and in the official Tools thread.

Alright,

thanks again RawCode for the original idea to use mono_code_manager_* but I had to add all those *nix dll refs in DllImport to make it fly even on other OSes. I tested with two guys on discord and I am now quite confident that this should work on 32/64 pc, mac & linux.

Both repositories on GitHub have been updated so feel free to test again.

The remaining question for me is: now Harmony has a direct dependency on RimWorld dll paths (at least for the linux stuff). How can I make the DllImport annotations dynamic so they find their files correctly even for other Unity applications? Any takers?

Cheers,
Andreas Pardeike

Hmm, I still can't get the Camara+ example to run. It causes an exception when activating the mod.

Non platform assembly: data-1F5EAF10 (this message is harmless)
Fallback handler could not load library C:/Program Files (x86)/Steam/steamapps/common/RimWorld/RimWorldWin_Data/Mono/data-1F5EAF10.dll
Non platform assembly: data-1F5FAD48 (this message is harmless)
Fallback handler could not load library C:/Program Files (x86)/Steam/steamapps/common/RimWorld/RimWorldWin_Data/Mono/data-1F5FAD48.dll
Caught exception while loading play data but there are active mods other than Core. Resetting mods config and trying again.
The exception was: System.TypeLoadException: Could not load type 'Harmony.HarmonyPatch' from assembly 'CameraPlus'.
  at (wrapper managed-to-native) System.MonoCustomAttrs:GetCustomAttributesInternal (System.Reflection.ICustomAttributeProvider,System.Type,bool)
  at System.MonoCustomAttrs.GetCustomAttributesBase (ICustomAttributeProvider obj, System.Type attributeType) [0x00000] in <filename unknown>:0
  at System.MonoCustomAttrs.GetCustomAttributes (ICustomAttributeProvider obj, System.Type attributeType, Boolean inherit) [0x00000] in <filename unknown>:0
  at System.MonoType.GetCustomAttributes (System.Type attributeType, Boolean inherit) [0x00000] in <filename unknown>:0
  at Verse.GenAttribute.TryGetAttribute[DefOf] (System.Reflection.MemberInfo memberInfo, RimWorld.DefOf& customAttribute) [0x00000] in <filename unknown>:0
  at Verse.GenAttribute.HasAttribute[DefOf] (System.Reflection.MemberInfo memberInfo) [0x00000] in <filename unknown>:0
  at Verse.GenTypes.<AllTypesWithAttribute`1>m__A0E[DefOf] (System.Type x) [0x00000] in <filename unknown>:0
  at System.Linq.Enumerable+<CreateWhereIterator>c__Iterator1D`1[System.Type].MoveNext () [0x00000] in <filename unknown>:0
  at RimWorld.DefOfHelper.RebindAllDefOfs (Boolean earlyTryMode) [0x00000] in <filename unknown>:0
  at Verse.PlayDataLoader.DoPlayLoad () [0x00000] in <filename unknown>:0
  at Verse.PlayDataLoader.LoadAllPlayData (Boolean recovering) [0x00000] in <filename unknown>:0


__internal tag is magical and should always work on any platform and any OS, it's reference to mono itself.
probably there is some kind of trick related to name mangling or something, sadly i have no resources to test OS other then windows 7-64.
i will check source of mono runtime once again for possible solution.

issue of Camera+ related to modload order, Harmony MUST be second mod, right after Core, all mods that use anything from Harmony must load after it.
If mod try to use classes not yet exists in class pool, loading fail and current implementation of classloader is unable to handle situation properly.

Quote from: RawCode on January 16, 2017, 12:14:26 AM
issue of Camera+ related to modload order, Harmony MUST be second mod, right after Core, all mods that use anything from Harmony must load after it.
If mod try to use classes not yet exists in class pool, loading fail and current implementation of classloader is unable to handle situation properly.

Current Mod order is Core > Harmony > CamaraPlus, and it still gives an exception. Using Win 10 x64 Insider Preview 14986

Sorry for the confusion, it was late and I was tired and got too excited. Anyway, I'll clean up things asap and test more. So my thoughts:

Regarding the magical __Internal - it does not work out of the box. Not on my mac and not on linux. Would be cool to get it work though.

But: new idea, and I think this should work great: why not make a new dummy method with enough "content" so the resulting JIT code is large enough to contain those bytes that I need as an in-between area to store the pointer to the shared information and the jump to the final method. It would work on any platform and it does not need any native calls. I tried to implement it yesterday and it worked first in the prototype but then crashed unrelated when doing anything in RW and I was desperate for sleep.

Ok, quick update (about to get to work). I suspected the GC to get into my way, so I changed the memory allocation from what is in on GitHub to:

      static DynamicMethod dynamicMethod; // WARNING, test only. Works only with 1 patch
      static Delegate theDelegate; // WARNING, test only. Works only with 1 patch
      delegate void Dummy();
      public static long GetMemory(int size)
      {
         dynamicMethod = new DynamicMethod("", typeof(void), new Type[] { });
         var il = dynamicMethod.GetILGenerator();
         il.DeclareLocal(typeof(int));
         il.Emit(OpCodes.Ldc_I4, 0);
         for (int i = 1; i <= 64; i++)
         {
            il.Emit(OpCodes.Stloc_0);
            il.Emit(OpCodes.Ldloc_0);
            il.Emit(OpCodes.Ldc_I4, i);
            il.Emit(OpCodes.Add);
         }
         il.Emit(OpCodes.Stloc_0);
         il.Emit(OpCodes.Ret);
         theDelegate = dynamicMethod.CreateDelegate(typeof(Dummy));
         return theDelegate.Method.MethodHandle.GetFunctionPointer().ToInt64();
      }

and it works (on my Mac where it prev. did crash too). But when I quit RW, I get this nasty stack trace:

Thread 0 Crashed:: MainThrd  Dispatch queue: com.apple.main-thread
0   libsystem_kernel.dylib           0x00007fffaf7dbdd6 __pthread_kill + 10
1   libsystem_pthread.dylib          0x00007fffaf8c7787 pthread_kill + 90
2   libsystem_c.dylib                0x00007fffaf741420 abort + 129
3   libmono.0.dylib                  0x00000001029e6244 dlfree + 3045
4   libmono.0.dylib                  0x00000001029e3c1c free_chunklist + 79
5   libmono.0.dylib                  0x00000001029e3bbe mono_code_manager_destroy + 34
6   libmono.0.dylib                  0x000000010285abb1 dynamic_method_info_free + 25
7   libmono.0.dylib                  0x00000001029ee2a6 g_hash_table_foreach + 81
8   libmono.0.dylib                  0x0000000102859890 mini_free_jit_domain_info + 101
9   libmono.0.dylib                  0x0000000102937ef4 mono_domain_free + 574
10  libmono.0.dylib                  0x000000010285a615 mini_cleanup + 1369
11  libmono.0.dylib                  0x00000001028a70de mono_jit_cleanup + 13
12  unity.Ludeon Studios.RimWorld by Ludeon Studios   0x0000000100c8fd09 CleanupMono() + 57
13  unity.Ludeon Studios.RimWorld by Ludeon Studios   0x0000000100c25daf PlayerCleanup(bool) + 143
14  unity.Ludeon Studios.RimWorld by Ludeon Studios   0x000000010135c883 DoQuit(bool) + 19
15  unity.Ludeon Studios.RimWorld by Ludeon Studios   0x000000010135f94d -[PlayerAppDelegate applicationShouldTerminate:] + 45
16  com.apple.AppKit                 0x00007fff97e8e420 -[NSApplication _docController:shouldTerminate:] + 71
17  com.apple.AppKit                 0x00007fff97e8e2d4 __91-[NSDocumentController(NSInternal) _closeAllDocumentsWithDelegate:shouldTerminateSelector:]_block_invoke + 567
18  com.apple.AppKit                 0x00007fff97e8def3 -[NSDocumentController(NSInternal) _closeAllDocumentsWithDelegate:shouldTerminateSelector:] + 1318
19  com.apple.AppKit                 0x00007fff97e8d9a6 -[NSDocumentController(NSInternal) __closeAllDocumentsWithDelegate:shouldTerminateSelector:] + 307
20  com.apple.AppKit                 0x00007fff97e8d5f7 -[NSApplication _shouldTerminate] + 843
21  com.apple.AppKit                 0x00007fff97e8cc96 -[NSApplication terminate:] + 773
22  libsystem_trace.dylib            0x00007fffaf8dfc3d _os_activity_initiate + 61
23  com.apple.AppKit                 0x00007fff98408c9c -[NSApplication(NSResponder) sendAction:to:from:] + 456
24  com.apple.AppKit                 0x00007fff97edce06 -[NSMenuItem _corePerformAction] + 324
25  com.apple.AppKit                 0x00007fff97edcb72 -[NSCarbonMenuImpl performActionWithHighlightingForItemAtIndex:] + 114
26  libsystem_trace.dylib            0x00007fffaf8dfc3d _os_activity_initiate + 61
27  com.apple.AppKit                 0x00007fff97edb985 -[NSMenu performKeyEquivalent:] + 367
28  com.apple.AppKit                 0x00007fff98407897 routeKeyEquivalent + 1024
29  com.apple.AppKit                 0x00007fff984054ad -[NSApplication(NSEvent) sendEvent:] + 3377
30  unity.Ludeon Studios.RimWorld by Ludeon Studios   0x000000010135d72e -[PlayerApplication sendEvent:] + 190
31  com.apple.AppKit                 0x00007fff97c80f81 -[NSApplication run] + 1002
32  com.apple.AppKit                 0x00007fff97c4b850 NSApplicationMain + 1237
33  unity.Ludeon Studios.RimWorld by Ludeon Studios   0x000000010135d9fe PlayerMain(int, char const**) + 654
34  unity.Ludeon Studios.RimWorld by Ludeon Studios   0x0000000100002034 start + 52

I have to be doing something wrong. Im getting my last crash on OSX sierra, even with the patch.

Quote from: scuba156 on January 16, 2017, 03:50:42 AM
I have to be doing something wrong. Im getting my last crash on OSX sierra, even with the patch.

All I did was quickly fetch the latest version of Harmony from GitHub (did some changes last night) and replaced the method above. Compiled in Debug and added dll to Camera+, compiled Debug, made sure both dlls were correct and tested in RimWorld from steam. Did work to launch a map and crashed when I quit RW.

So recompiling the Camara+ example with the new dll works, just replacing the harmony dll with the updated one does not. I would not of thought that it would need recompiling when it's referencing an external lib.

So now I'm up to date, it crashes after map gen and on exit too. Here's the stack trace from mine:


========== OUTPUTING STACK TRACE ==================

0x076A8B5C (mono) unity_mono_method_is_generic
0x076A8B99 (mono) unity_mono_method_is_generic
0x0768983D (mono) mono_debugger_run_finally
0x0768AA15 (mono) mono_debugger_run_finally
  ERROR: SymGetSymFromAddr64, GetLastError: 'The specified module could not be found.' (Address: 077D0066)
  ERROR: SymGetModuleInfo64, GetLastError: 'A dynamic link library (DLL) initialization routine failed.' (Address: 077D0066)
0x077D0066 ((<unknown>))
0x0438FF12 (Mono JIT Code) Verse.CameraDriver:Awake ()
0x07DDF541 (Mono JIT Code) (wrapper runtime-invoke) object:runtime_invoke_void__this__ (object,intptr,intptr,intptr)
0x07691440 (mono) mono_set_defaults
0x075FD7EE (mono) mono_runtime_invoke
0x003BD2DE (RimWorldWin) scripting_gchandle_get_target
0x0044200A (RimWorldWin) ScriptingArguments::AddString
0x00319C3E (RimWorldWin) RenderSettings::GetAmbientProbe
0x0044226C (RimWorldWin) ScriptingArguments::AddString
0x003ADF2F (RimWorldWin) IClusterRenderer::IClusterRenderer
0x003AE496 (RimWorldWin) IClusterRenderer::IClusterRenderer
0x003B0478 (RimWorldWin) IClusterRenderer::IClusterRenderer
0x003AE8BC (RimWorldWin) IClusterRenderer::IClusterRenderer
0x0045AD82 (RimWorldWin) RegisterAllowNameConversionInDerivedTypes
0x0045AF6D (RimWorldWin) RegisterAllowNameConversionInDerivedTypes
0x004405D6 (RimWorldWin) CallbackArray3<int const ,AwakeFromLoadQueue &,enum RuntimeSceneManager::LoadingMode>::Invoke
0x00440786 (RimWorldWin) CallbackArray3<int const ,AwakeFromLoadQueue &,enum RuntimeSceneManager::LoadingMode>::Invoke
0x0043F60A (RimWorldWin) AnimationCurveTpl<Quaternionf>::GetTypeString
0x0041A57F (RimWorldWin) RectT<int>::Contains
0x004B0EAB (RimWorldWin) Append
0x004B360E (RimWorldWin) PlayerWinMain
0x008AC8D8 (RimWorldWin) RectT<int>::GetBottom
0x008DBD88 (RimWorldWin) RectT<int>::GetBottom
0x76FE8694 (KERNEL32) BaseThreadInitThunk
0x771004BA (ntdll) ApiSetQueryApiSetPresence

========== END OF STACKTRACE ===========


Yes, sometimes the lib does not get copied over and you need to have it updated to have your code reference the correct version (simply replacing it does not cut it).

Good news, I replaced DynamicMethod with MethodBuilder and it seems to work fine. Just tested on my Mac so we need people to jump in and test it on Win and Linux. Tonight I can test on my Windows machine.

The git repo has no release yet but is updated to v1.0.3 so you need to build it yourself for now. I think this is good progress and I feel confident that we are getting close. A lot of nasty code has disappeared - usually a good sign :-)

Cheers
Andreas Pardeike

EDIT 1: sorry I made a small change to the IL code generator. My bad, I undo it right away....
EDIT 2: fixed

I'll test on Windows as soon as you post the new commit, I just tried it and an IL issue. Ready to recompile when its done.

EDIT: RimWorld starts fine, cant start a new game or load a game without crashing. Can exit without crashing though.


========== OUTPUTING STACK TRACE ==================

0x7682E512 (KERNELBASE) RaiseException
0x074310DD (mono) set_vprintf_func
0x074310F6 (mono) set_vprintf_func
0x0744E4D9 (mono) mono_class_from_mono_type
0x0751BFB0 (mono) mono_print_method_from_ip
0x0751C1CB (mono) mono_print_method_from_ip
0x0751DFC4 (mono) mono_jit_thread_attach
0x0751FDC1 (mono) mono_set_defaults
0x07520C2A (mono) mono_set_defaults
0x07521185 (mono) mono_set_defaults
0x075211F2 (mono) mono_set_defaults
0x0751ACA1 (mono) mono_debugger_run_finally
  ERROR: SymGetSymFromAddr64, GetLastError: 'The specified module could not be found.' (Address: 077F0066)
  ERROR: SymGetModuleInfo64, GetLastError: 'A dynamic link library (DLL) initialization routine failed.' (Address: 077F0066)
0x077F0066 ((<unknown>))
0x1E9AA5B2 (Mono JIT Code) Verse.CameraDriver:Awake ()
0x07D6F541 (Mono JIT Code) (wrapper runtime-invoke) object:runtime_invoke_void__this__ (object,intptr,intptr,intptr)
0x07521440 (mono) mono_set_defaults
0x0748D7EE (mono) mono_runtime_invoke
0x003BD2DE (RimWorldWin) scripting_gchandle_get_target
0x0044200A (RimWorldWin) ScriptingArguments::AddString
0x00319C3E (RimWorldWin) RenderSettings::GetAmbientProbe
0x0044226C (RimWorldWin) ScriptingArguments::AddString
0x003ADF2F (RimWorldWin) IClusterRenderer::IClusterRenderer
0x003AE496 (RimWorldWin) IClusterRenderer::IClusterRenderer
0x003B0478 (RimWorldWin) IClusterRenderer::IClusterRenderer
0x003AE8BC (RimWorldWin) IClusterRenderer::IClusterRenderer
0x0045AD82 (RimWorldWin) RegisterAllowNameConversionInDerivedTypes
0x0045AF6D (RimWorldWin) RegisterAllowNameConversionInDerivedTypes
0x004405D6 (RimWorldWin) CallbackArray3<int const ,AwakeFromLoadQueue &,enum RuntimeSceneManager::LoadingMode>::Invoke
0x00440786 (RimWorldWin) CallbackArray3<int const ,AwakeFromLoadQueue &,enum RuntimeSceneManager::LoadingMode>::Invoke
0x0043F60A (RimWorldWin) AnimationCurveTpl<Quaternionf>::GetTypeString
0x0041A57F (RimWorldWin) RectT<int>::Contains
0x004B0EAB (RimWorldWin) Append
0x004B360E (RimWorldWin) PlayerWinMain
0x008AC8D8 (RimWorldWin) RectT<int>::GetBottom
0x008DBD88 (RimWorldWin) RectT<int>::GetBottom
0x76FE8694 (KERNEL32) BaseThreadInitThunk
0x771004BA (ntdll) ApiSetQueryApiSetPresence

========== END OF STACKTRACE ===========

mono_lookup_pinvoke_call
handle loading of native function.
nasty trick exists, i managed to detect issue during research of "latebind" native methods

if you execute method, that have references to unbound native method, it will crash\fail ever if you managed to fix method before process reach actual call.
reason is simple - JIT already knows that method is invalid and emit unconditional exception throw.
if you store all natives inside wrapper with "no inline" option, everything works properly, also you can modify wrapper at will.

Most simple and effective way to force JIT to allocate large memory section for method, is exception throws and exception handling.
JIT emit large blocks for each exception section.

Quote from: scuba156 on January 16, 2017, 06:54:39 AM
I'll test on Windows as soon as you post the new commit, I just tried it and an IL issue. Ready to recompile when its done.

EDIT: RimWorld starts fine, cant start a new game or load a game without crashing. Can exit without crashing though.

Code Select Expand


ERROR: SymGetModuleInfo64, GetLastError: 'A dynamic link library (DLL) initialization routine failed.' (Address: 077F0066)

This very much looks like your camera code not correctly bound to the right version of the Harmony lib. It isn't related to the bug we discuss right now. I suggest you double check that your project refers to the correct version of the lib and that it in fact is named 0harmony in the Assemblies folder to load it first. Too bad you are on Mac, otherwise I would recommend to use ILMerge to join the two dlls into a single one. That would definitely get rid of that error.

Quote from: RawCode on January 16, 2017, 08:15:55 AM
handle loading of native function.
nasty trick exists, i managed to detect issue during research of "latebind" native methods

if you execute method, that have references to unbound native method, it will crash\fail ever if you managed to fix method before process reach actual call.
reason is simple - JIT already knows that method is invalid and emit unconditional exception throw.
if you store all natives inside wrapper with "no inline" option, everything works properly, also you can modify wrapper at will.

Most simple and effective way to force JIT to allocate large memory section for method, is exception throws and exception handling.
JIT emit large blocks for each exception section.

I must admit that I am having a hard time to follow you, Raw. Especially since I don't have any invalid methods anywhere. Are we actually talking about the same topic? Maybe I failed to explain what I am actually doing here. In that case I am really sorry.

Quote from: pardeike on January 16, 2017, 08:56:16 AM
This very much looks like your camera code not correctly bound to the right version of the Harmony lib. It isn't related to the bug we discuss right now. I suggest you double check that your project refers to the correct version of the lib and that it in fact is named 0harmony in the Assemblies folder to load it first. Too bad you are on Mac, otherwise I would recommend to use ILMerge to join the two dlls into a single one. That would definitely get rid of that error.

I've double and triple checked my refrences and everything appears to be fine. I've cleaned and rebuilt multiple times. I'm using Windows as my main machine, used ILMerge to make a single dll but it still caused an exception.

Even using the updated 1.0.3 CamaraPlus dll's from your github cause the same exception.

The problem was that the JIT compiler optimized the assembler and Harmony was writing over the end of the JIT buffer. I have used LINQPad to test my CIL code in real time and think that this will solve the problem. Somehow my local PC was not so picky about memory allocation and never triggered the problem.

Harmony + CameraPlus updated on GitHub, but no release yet. CameraPlus is completely build, so a zip-download should be sufficient for testing.

Seems not to work accordingly to the guys on discord. I'm out of ideas - since it always works on any of my machines.  :-\

I tested on Win10 and OSX and get the same. Here's an OS X stacktrace:

Receiving unhandled NULL exception
Obtained 38 stack frames.
#0  0x00000101e213a5 in mono_type_get_type
#1  0x00000101decadb in type_check_context_used
#2  0x00000101deca18 in inst_check_context_used
#3  0x00000101dec9c6 in mono_generic_context_check_used
#4  0x00000101d7d8d5 in mono_method_check_context_used
#5  0x00000101d7783a in mono_magic_trampoline
#6  0x0000010a79c171 in (Unknown)
#7  0x00000115e159bf in (Unknown)
#8  0x00000101d0a012 in mono_jit_runtime_invoke
#9  0x00000101e3442a in mono_runtime_invoke
#10 0x00000100bc2a51 in ScriptingInvocation::Invoke(MonoException**)
#11 0x00000100bc2f9b in ScriptingInvocationNoArgs::InvokeChecked()
#12 0x00000100bb0ece in MonoBehaviour::CallAwake()
#13 0x00000100bb1343 in MonoBehaviour::AddToManager()
#14 0x00000100bb0e05 in MonoBehaviour::AwakeFromLoad(AwakeFromLoadMode)
#15 0x00000100bf0cb9 in AwakeFromLoadQueue::InvokePersistentManagerAwake(AwakeFromLoadQueue::Item*, unsigned int, AwakeFromLoadMode)
#16 0x00000100bc16aa in LoadSceneOperation::CompleteAwakeSequence()
#17 0x00000100bc11fc in LoadSceneOperation::PlayerLoadSceneFromThread()
#18 0x00000100bc0e82 in LoadSceneOperation::IntegrateMainThread()
#19 0x00000100bbd8cb in PreloadManager::UpdatePreloadingSingleStep(PreloadManager::UpdatePreloadingFlags, int)
#20 0x00000100bbe1da in PreloadManager::UpdatePreloading()
#21 0x00000100b51e69 in PlayerLoop(bool, bool, IHookEvent*)
#22 0x0000010115e6a1 in -[PlayerAppDelegate UpdatePlayer]
#23 0x007fffab28ef7f in __NSFireTimer
#24 0x007fffa97e5294 in __CFRUNLOOP_IS_CALLING_OUT_TO_A_TIMER_CALLBACK_FUNCTION__
#25 0x007fffa97e4f23 in __CFRunLoopDoTimer
#26 0x007fffa97e4a7a in __CFRunLoopDoTimers
#27 0x007fffa97dc5d1 in __CFRunLoopRun
#28 0x007fffa97dbb54 in CFRunLoopRunSpecific
#29 0x007fffa8d66acc in RunCurrentEventLoopInMode
#30 0x007fffa8d66901 in ReceiveNextEventCommon
#31 0x007fffa8d66736 in _BlockUntilNextEventMatchingListInModeWithFilter
#32 0x007fffa730cae4 in _DPSNextEvent
#33 0x007fffa7a8721f in -[NSApplication(NSEvent) _nextEventMatchingEventMask:untilDate:inMode:dequeue:]
#34 0x007fffa7301465 in -[NSApplication run]
#35 0x007fffa72cbd80 in NSApplicationMain
#36 0x0000010115cfde in PlayerMain(int, char const**)
#37 0x00000100002034 in start
Stacktrace:


Native stacktrace:

0   libsystem_kernel.dylib              0x00007fffbef07dd6 __pthread_kill + 10
1   libsystem_c.dylib                   0x00007fffbee6d420 abort + 129
2   RimWorldMac                         0x0000000100bb4ef4 _Z12HandleSignaliP9__siginfoPv + 36
3   libmono.0.dylib                     0x0000000101dbfece mono_chain_signal + 75
4   libmono.0.dylib                     0x0000000101d0838a mono_sigsegv_signal_handler + 210
5   libsystem_platform.dylib            0x00007fffbefe6bba _sigtramp + 26
6   ???                                 0x00000000000000b0 0x0 + 176
7   libmono.0.dylib                     0x0000000101decadb type_check_context_used + 22
8   libmono.0.dylib                     0x0000000101deca18 inst_check_context_used + 58
9   libmono.0.dylib                     0x0000000101dec9c6 mono_generic_context_check_used + 22
10  libmono.0.dylib                     0x0000000101d7d8d5 mono_method_check_context_used + 37
11  libmono.0.dylib                     0x0000000101d7783a mono_magic_trampoline + 795
12  ???                                 0x000000010a79c171 0x0 + 4470718833
13  ???                                 0x0000000115e159bf 0x0 + 4662057407
14  libmono.0.dylib                     0x0000000101d0a012 mono_jit_runtime_invoke + 1766
15  libmono.0.dylib                     0x0000000101e3442a mono_runtime_invoke + 117
16  RimWorldMac                         0x0000000100bc2a51 _ZN19ScriptingInvocation6InvokeEPP13MonoException + 49
17  RimWorldMac                         0x0000000100bc2f9b _ZN25ScriptingInvocationNoArgs13InvokeCheckedEv + 43
18  RimWorldMac                         0x0000000100bb0ece _ZN13MonoBehaviour9CallAwakeEv + 174
19  RimWorldMac                         0x0000000100bb1343 _ZN13MonoBehaviour12AddToManagerEv + 147
20  RimWorldMac                         0x0000000100bb0e05 _ZN13MonoBehaviour13AwakeFromLoadE17AwakeFromLoadMode + 549
21  RimWorldMac                         0x0000000100bf0cb9 _ZN18AwakeFromLoadQueue28InvokePersistentManagerAwakeEPNS_4ItemEj17AwakeFromLoadMode + 217
22  RimWorldMac                         0x0000000100bc16aa _ZN18LoadSceneOperation21CompleteAwakeSequenceEv + 202
23  RimWorldMac                         0x0000000100bc11fc _ZN18LoadSceneOperation25PlayerLoadSceneFromThreadEv + 636
24  RimWorldMac                         0x0000000100bc0e82 _ZN18LoadSceneOperation19IntegrateMainThreadEv + 114
25  RimWorldMac                         0x0000000100bbd8cb _ZN14PreloadManager26UpdatePreloadingSingleStepENS_21UpdatePreloadingFlagsEi + 363
26  RimWorldMac                         0x0000000100bbe1da _ZN14PreloadManager16UpdatePreloadingEv + 218
27  RimWorldMac                         0x0000000100b51e69 _Z10PlayerLoopbbP10IHookEvent + 921
28  RimWorldMac                         0x000000010115e6a1 -[PlayerAppDelegate UpdatePlayer] + 321
29  Foundation                          0x00007fffab28ef7f __NSFireTimer + 83
30  CoreFoundation                      0x00007fffa97e5294 __CFRUNLOOP_IS_CALLING_OUT_TO_A_TIMER_CALLBACK_FUNCTION__ + 20
31  CoreFoundation                      0x00007fffa97e4f23 __CFRunLoopDoTimer + 1075
32  CoreFoundation                      0x00007fffa97e4a7a __CFRunLoopDoTimers + 298
33  CoreFoundation                      0x00007fffa97dc5d1 __CFRunLoopRun + 2081
34  CoreFoundation                      0x00007fffa97dbb54 CFRunLoopRunSpecific + 420
35  HIToolbox                           0x00007fffa8d66acc RunCurrentEventLoopInMode + 240
36  HIToolbox                           0x00007fffa8d66901 ReceiveNextEventCommon + 432
37  HIToolbox                           0x00007fffa8d66736 _BlockUntilNextEventMatchingListInModeWithFilter + 71
38  AppKit                              0x00007fffa730cae4 _DPSNextEvent + 1120
39  AppKit                              0x00007fffa7a8721f -[NSApplication(NSEvent) _nextEventMatchingEventMask:untilDate:inMode:dequeue:] + 2789
40  AppKit                              0x00007fffa7301465 -[NSApplication run] + 926
41  AppKit                              0x00007fffa72cbd80 NSApplicationMain + 1237
42  RimWorldMac                         0x000000010115cfde _Z10PlayerMainiPPKc + 638
43  RimWorldMac                         0x0000000100002034 start + 52

I am starting to wonder if it is somehow build/project related. Would anybody here try this: using the latest version of both projects on GitHub, just copy all .cs files from both into a new project just like you would for your own mods and run it? Only dependencies are the usual two Rimworld dll's.

Just for reference:

I removed Mono installation from my PC, started VS 2015 Enterprise, created a new project (.NET 3.5), went to GitHub and did a "Clone or download -> Download ZIP" there, copied all the downloaded Harmony .cs files, copied the Main.cs from CameraPlus, set the unsafe flag in the project settings and build a Debug build. Then I put it into my RW,  enabled the new mod (only Core otherwise), did not even quit RW and went into an existing game and it zooms perfectly. No crash. That's all I did, nothing more.

My PC: Intel i7-3770, 16GB 64-bit x64-processor, freshly restarted.

So where is the fucking problem?

I'll do some test builds in about 10 mins. I've been using VS2017 community, but I'll give 2015 a go without mono and see if it helps.

PC specs: Win 10 x64, i7 6700k x64, 16Gb

Quote from: scuba156 on January 17, 2017, 03:09:01 AM
I'll do some test builds in about 10 mins. I've been using VS2017 community, but I'll give 2015 a go without mono and see if it helps.

PC specs: Win 10 x64, i7 6700k x64, 16Gb

At this point, I am quite sure it is somehow the logic of the code that inserts the first jump and then the second jump. It may got f*cked up after refactoring and contains a bug that just triggers on architectures different from me. It's not that the code is huge or complicated but I had no time to double check. A friend had followed the above and it still crashes so I think it's pointless for you to just replicate.

Quote from: pardeike on January 17, 2017, 03:29:11 AM

Quote from: scuba156 on January 17, 2017, 03:09:01 AM
I'll do some test builds in about 10 mins. I've been using VS2017 community, but I'll give 2015 a go without mono and see if it helps.

PC specs: Win 10 x64, i7 6700k x64, 16Gb

At this point, I am quite sure it is somehow the logic of the code that inserts the first jump and then the second jump. It may got f*cked up after refactoring and contains a bug that just triggers on architectures different from me. It's not that the code is huge or complicated but I had no time to double check. A friend had followed the above and it still crashes so I think it's pointless for you to just replicate.

Can confirm that is still crashes.

I have no experience with assembly and memory allocation so I don't believe I would be of much use prior to testing.

Brrainz myself and Fluffy debugged this last night on the modders discord and Brrainz figured out the issue with the crashes. Expect an update from him soon. He was very excited obviously but it was late when we figured it out.

Hej guys,

I had some time documenting the crash in Harmony. What I did was test a debug version of Harmony (checked in) with two mods of mine: CameraPlus and SameSpot. I used offline-RW with the debug mono.dll version to break into VS but I think the crash happens even in the steam version.

I have documented the process as close as possible with some memory debug info from Harmony from before and after and all the memory locations. Here is a gist for the report:

https://gist.github.com/pardeike/ec9ffb349379390b46d94f787bbdeb90

RawCode, skullywag, anybody with some intimate knowledge on why this happens with DynamicMethod, could you please have a look at clarify things? This is no longer related to the jumps or memory allocation or such but with DynamicMethod "changing". I do pin all managed objects to a global static array so its hard to believe that GC is the culprit here.

As a short refresher: Harmony adds a jump from the original method to an intermediate memory, and from there a jump to the wrapper (DynamicMethod) that is custom build to match the signature of the original method. That wrapper calls all patches and of course the copy of the original (also a DynamicMethod). It is that wrapper that is changing.

I think this is the last bug in Harmony.
/Andreas

Each method's life have 3 stages:

metadata record, no code at all; - before calling "compile_method, that called GetFunctionPointer() in managed;
code filled with trampolines; - before calling method at least once
actual code with actual references; - after method actually completed at least once

Dynamic methods are not any special, when method is compiled, it's still filled with trampolines and calling trampoline cause runtime to update call site.
As far as i remember, runtime always emit trampolines, ever if have compiled version of method in cache.

RuntimeHelpers.PrepareMethod(targetMethod.MethodHandle);
and all code changes related to this method, well, i have bad news for you
[MonoTODO("Currently a no-op")]
public static void PrepareMethod (RuntimeMethodHandle method)
{
}

[MonoTODO("Currently a no-op")]
public static void PrepareMethod (RuntimeMethodHandle method, RuntimeTypeHandle[] instantiation)
{
}
Fetching x86 for this method from game will give you noop function also.
All core related to usege of preparemethod is invalid for mono runtime.

Code allocated for functions, allocated from native pool, not heap, in addition, it's permanent and never deallocated, ever if dynamic method is lost, it's data will stay as long as domain hosting data alive.

To detect why and how, just "protect" dynamic method memory from writing with virtual protect, and you will see trace with method name\chain that caused modification.

Quote from: RawCode on January 19, 2017, 06:52:26 AM
To detect why and how, just "protect" dynamic method memory from writing with virtual protect, and you will see trace with method name\chain that caused modification.

That is a good idea, RawCode. One question I have though:

Assuming that the old way of Detour works by simply writing a jump from original method to replacement method, why would the Harmony code not work?

It only writes jumps too (the extra step in between can't be the problem).

The difference is that the old Detour uses replacement methods that are from the loaded Assembly of the mod and Harmony uses a DynamicMethod that generates the method instead.

You said that DynamicMethods are not special. But that means that the jump that triggers all the things you explained in your post would do the same with a DynamicMethod. I assume that the assembler entry point of a method (dynamic or not) is always what is returned from GetFunctionPointer() and will never change.

But redirecting the assembler flow from original to replacement is different for assembly loaded methods and dynamic methods. Which means that they are not treated the same. Do you follow my logic here or is there something I am missing?

I also found these two articles:
https://www.codeproject.com/Articles/37549/CLR-Injection-Runtime-Method-Replacer
http://www.ntcore.com/Files/netint_injection.htm

EDIT: I am at work so no way to test this, but I found this on stack overflow: http://stackoverflow.com/questions/13655402/dynamicmethod-prelink :

Marshal.Prelink(MethodInfo);

which the author claims only works in mono (yay!) - could this be the solution?

no MS runtime code injection stuff works on mono and vice versa.

ICALL_EXPORT void
ves_icall_System_Runtime_InteropServices_Marshal_Prelink (MonoReflectionMethod *method)
{
MONO_ARCH_SAVE_REGS;
prelink_method (method->method);
}

static void
prelink_method (MonoMethod *method)
{
const char *exc_class, *exc_arg;
if (!(method->flags & METHOD_ATTRIBUTE_PINVOKE_IMPL))
return;
mono_lookup_pinvoke_call (method, &exc_class, &exc_arg);
if (exc_class) {
mono_raise_exception(
mono_exception_from_name_msg (mono_defaults.corlib, "System", exc_class, exc_arg ) );
}
/* create the wrapper, too? */
}

prelink works only for platform invoke (native\extern) methods and do nothing for managed methods.

probably your problem related to usage of MS based code, that does not work.

i still not ever peeked on harmony code, probably there is some obvious error related to some obvious thing like not calling native constructor or calling native constructor with invalid arguments.

I know you are sceptic but I have coded professionally for like 28 years and I triple checked the code. There might be some edge cases with strange argument types and classes but that usually results in a simple illegal opcode error. Besides the jumps and the code that executes only if you patch a method again (not the scope of this test and not executed) there is nothing assembler related in Harmony. The debug info would also show any abnormal use of the assembler routines or the statics being reused because of the library being cached in the assembly cache. I think one could even comment out the extra code that handles regenerating the il code and storing information for that. Which would mean that one could use the ordinary Detour as used by i.e. HugsLib.

I might try that tomorrow to proof that it's not any funky stuff involved. My way forward now is to read and understand mono - both .cs and .c sources. StarWars debugging: "Use the source, Luke!"

ever 48 years of experience will not shield you from undocumented runtime implementation features\bugs.

sceptic or not both method Prelink and PrepareMethod do nothing in your case, to be precise in case of mono runtime.

IL converted into native by jitter when "native constructor" is called on dynamic method, calling "finalize" or "build" is not suffice, getting function pointer before jitter is complited result in undefined behavior.

If mono can IL -> ASM then there is a way to call into mono to trigger it manually. I just have to find it.

After a whole weekend to get to the truth, this is what I know:

- detouring to dynamic methods works different

- you need to jit dynamic methods - simplest way is to have a "first time? return" statement at the beginning and then pre-run them

- detouring works for one method if replacement is dynamic method

- detouring does not work for more than one method regardless of what you do. It possibly corrupts the generic trampoline mechanism

- I need to understand trampolines way better before I continue work on Harmony

And no, RawCode, it is not a simple error. I rewrote the whole solution in different ways and simplified and reduced any chance for error and tested in a 100+ different ways. Say result every time. I can now get the whole patching work 100% reliable for a single patch but as soon as I write a small mod that does two non related patches all hell breaks loose (those two each alone work fine).

/Andreas

sad but true, copy native code into native memory and rebase relative opcodes faster, more simple and feature no overhead compared to work with dynamic methods.

Trying to run it on Linux. Latest release from CameraPlus with the latest 0harmony.dll from Harmony.

Crashes with the following after trying to generate the map.
http://pastebin.com/raw/Ewx4MCeZ

Well, to avoid direct spoilers, i created little thread with basics (my own experience) explained (somewhat).
https://ludeon.com/forums/index.php?topic=29861.0

YAY!!!

Well, I did it. I just finished rewriting the core of Harmony. Better, faster, no crashes and you can even manipulate the original method directly. Tested and with almost no assembler low level stuff (just two tiny mono methods via dllimport.

Guys, I am so happy that I finally got this working. Tomorrow, I merge my test project with Harmony and update the documentation. Stay tuned!

/Andreas 8)

PS: here is what you can do as an example:

---

using Verse;
using System;
using RimWorld;
using UnityEngine;
using System.Collections.Generic;
using System.Linq;
using System.Reflection;

namespace Test
{
[StaticConstructorOnStartup]
public static class TestMain
{
static TestMain()
{
FileLog.Reset();
FileLog.Log("DynamicPatcher test started at " + DateTime.Now);

var p1 = new DynamicPatcher(CompileTools.Method(typeof(SelectionDrawer), "Notify_Selected"));
p1.AddPrefix(CompileTools.Method(typeof(Patches), "Notify_Selected_Prefix"));
p1.Patch();

var p2 = new DynamicPatcher(CompileTools.Method(typeof(SelectionDrawerUtility), "CalculateSelectionBracketPositionsWorld", null, new Type[] { typeof(object) }));
p2.AddPostfix(CompileTools.Method(typeof(Patches), "CalculateSelectionBracketPositionsWorld_Postfix"));
p2.Patch();

var p3 = new DynamicPatcher(CompileTools.Method(typeof(FloatMenuMakerMap), "AddDraftedOrders"));
p3.AddPostfix(CompileTools.Method(typeof(Patches), "AddDraftedOrders_Postfix"));
p3.Patch();

var p4 = new DynamicPatcher(CompileTools.Method(typeof(FloatMenuMakerMap), "AddUndraftedOrders"));
p4.AddPostfix(CompileTools.Method(typeof(Patches), "AddUndraftedOrders_Postfix"));
p4.Patch();

var p5 = new DynamicPatcher(CompileTools.Method(typeof(FloatMenuMakerMap), "AddHumanlikeOrders"));
p5.AddPostfix(CompileTools.Method(typeof(Patches), "AddHumanlikeOrders_Postfix"));
p5.Patch();

var innerType = typeof(RCellFinder).GetNestedTypes(CompileTools.all).First(t => t.Name.Contains("BestOrderedGotoDestNear"));
var p6 = new DynamicPatcher(CompileTools.Method(innerType, "<>m__60C"));
var p6_m1 = CompileTools.Method(typeof(PawnDestinationManager), "DestinationIsReserved", new Type[] { typeof(IntVec3), typeof(Pawn) });
var p6_p1 = CompileTools.Method(typeof(Patches), "DestinationIsReserved_Never");
var p6_m2 = CompileTools.Method(typeof(GenGrid), "Standable");
var p6_p2 = CompileTools.Method(typeof(Patches), "Standable_Always");
p6.AddModifier(new ILCode(p6_m1), new ILCode(p6_p1));
p6.AddModifier(new ILCode(p6_m2), new ILCode(p6_p2));
p6.Patch();

var p7 = new DynamicPatcher(CompileTools.Method(typeof(MemoryThoughtHandler), "TryGainMemoryThought", new Type[] { typeof(Thought_Memory), typeof(Pawn) }));
p7.AddPostfix(CompileTools.Method(typeof(Patches), "TryGainMemoryThought_Postfix"));
p7.Patch();

var p8 = new DynamicPatcher(CompileTools.Method(typeof(Pawn_FilthTracker), "Notify_EnteredNewCell"));
p8.AddPrefix(CompileTools.Method(typeof(Patches), "Notify_EnteredNewCell_Prefix"));
p8.AddPostfix(CompileTools.Method(typeof(Patches), "Notify_EnteredNewCell_Postfix"));
p8.Patch();

FileLog.Log("Done");
FileLog.Log("");
}
}

public static class Patches
{
// Demo 1: cast letters for all selected pawns
//
public static void Notify_Selected_Prefix(object t)
{
var pawn = t as Pawn;
if (pawn != null && pawn.NameStringShort != null && pawn.NameStringShort.Length > 0)
Find.LetterStack.ReceiveLetter(new Letter(pawn.NameStringShort, "You selected this pawn", LetterType.Good));
}

// Demo 2: expand selection brackets to show them apart from the selected object
//
public static void CalculateSelectionBracketPositionsWorld_Postfix(Vector3[] bracketLocs)
{
bracketLocs[0] = bracketLocs[0] + new Vector3(-.5f, 0, -.5f);
bracketLocs[1] = bracketLocs[1] + new Vector3(+.5f, 0, -.5f);
bracketLocs[2] = bracketLocs[2] + new Vector3(+.5f, 0, +.5f);
bracketLocs[3] = bracketLocs[3] + new Vector3(-.5f, 0, +.5f);
}

// Demo 3-5: add extra menu choice into context menu
//
public static void AddDraftedOrders_Postfix(List<FloatMenuOption> opts)
{
opts.Add(new FloatMenuOption("DRAFTED", null));
}
public static void AddUndraftedOrders_Postfix(List<FloatMenuOption> opts)
{
opts.Add(new FloatMenuOption("UNDRAFTED", null));
}
public static void AddHumanlikeOrders_Postfix(List<FloatMenuOption> opts)
{
opts.Add(new FloatMenuOption("HUMANLIKE", null));
}

// Demo 6: Modifying the Predicate inside BestOrderedGotoDestNear() so a destination is never reserved and everything
//         is standable
public static bool DestinationIsReserved_Never(PawnDestinationManager p1, IntVec3 p2, Pawn p3)
{
return false;
}
public static bool Standable_Always(IntVec3 p1, Map p2)
{
return true;
}

// Demo 7: Log all old and new thoughts to error log
//
public static void TryGainMemoryThought_Postfix(MemoryThoughtHandler __instance, Thought_Memory newThought)
{
if (__instance.Memories.LastOrDefault() == newThought)
Log.Error("New thought from " + newThought.pawn.NameStringShort + ": " + newThought.LabelCap + " [" + newThought.MoodOffset() + "]");
}

// Demo 8: track changes to the filth a pawn carries
// (here, we use __state to compare before/after call to Notify_EnteredNewCell)
//
public static void Notify_EnteredNewCell_Prefix(ref object __state, Pawn_FilthTracker __instance)
{
__state = ((List<Filth>)typeof(Pawn_FilthTracker)
.GetField("carriedFilth", BindingFlags.NonPublic | BindingFlags.Instance)
.GetValue(__instance)).ToArray();
}
//
public static void Notify_EnteredNewCell_Postfix(object __state, Pawn_FilthTracker __instance)
{
var oldFilth = ((Filth[])__state).ToList();
var oldFilthSum = 0;
oldFilth.ForEach(f => oldFilthSum += f.thickness);

var newFilth = (List<Filth>)typeof(Pawn_FilthTracker)
.GetField("carriedFilth", BindingFlags.NonPublic | BindingFlags.Instance)
.GetValue(__instance);
var newFilthSum = 0;
newFilth.ForEach(f => newFilthSum += f.thickness);

if (oldFilthSum == newFilthSum) return;
var diff = "" + (newFilthSum - oldFilthSum);
if (newFilthSum - oldFilthSum > 0) diff = "+" + diff;

var pawn = (Pawn)typeof(Pawn_FilthTracker).GetField("pawn", CompileTools.all).GetValue(__instance);
Log.Warning(pawn.NameStringShort + " filth " + diff + " at " + pawn.Position.x + "," + pawn.Position.z);
}

}
}

i hope you will explain what was wrong and how things are fixed?

Quote from: RawCode on February 01, 2017, 02:20:01 AM
i hope you will explain what was wrong and how things are fixed?

Absolutely. Just polishing the code a bit

Quote from: RawCode on February 01, 2017, 02:20:01 AM
i hope you will explain what was wrong and how things are fixed?

Hi RawCode. I released a fixed version (1.0.6) of Harmony. The mistake that I made was so stupid that I am too embarrassed to tell. Maybe you find it out (hint: has NOTHING to do with low level stuff at all).

HARMONY 1.0.6 - the future of patching code at runtime

So this is it. I have release a public preview of Harmony on GitHub: https://github.com/pardeike/Harmony/releases/tag/v1.0.6

Everyone is welcome to try it out. It most certainly will find its way in the popular community libraries as well (HugsLib). However, the design is done in a way so modders can use it in their own mods without any conflict with the Harmony version that is used by other mods or by a common library like HugsLib.

The github repository has a ready made version of the lib in dll form and you can either add it to your Assemblies folder (add reference and keep "Copy" on) or merge it with a tool like ILMerge into your own dll. I strongly recommend to test things before going wild and releasing stuff to all users.

Documentation exists in the form of a wiki on github: https://github.com/pardeike/Harmony/wiki or you can download two mods I made for demonstration purpose: SameSpot (https://github.com/pardeike/SameSpot) or Camera+ (https://github.com/pardeike/CameraPlus). SameSpot has some advanced technique to modify the original method so I recommend Camera+ to begin with.

Thanks for your patience and support. Please try out this release and keep the feedback coming.

Cheers,
Andreas Pardeike

Quote from: pardeike on February 04, 2017, 06:41:29 AM

Quote from: RawCode on February 01, 2017, 02:20:01 AM
i hope you will explain what was wrong and how things are fixed?

Hi RawCode. I released a fixed version (1.0.6) of Harmony. The mistake that I made was so stupid that I am too embarrassed to tell. Maybe you find it out (hint: has NOTHING to do with low level stuff at all).

you hooked into "Invoke" method instead of actual code of dynamic methods?

good job anyway.

You are best!

Quote from: MaximStale on February 06, 2017, 12:22:53 PM
You are best!

Thank you!

I haven't had the time to try it out yet, but congratulations on getting it working. Looking forward to using it in the future.

I've started actively using this in my mods in internal builds.
Magnificent job.

Any clue how to use Harmony to handle "out" variables?

Let's say I want to modify....

Verse.Pawn_HealthTracker
public void PreApplyDamage(DamageInfo dinfo, out bool absorbed)

...
with a Prefix.
My goal is to have it return the absorbed as "true" before the main method and prevent the main method from occurring.

How could I do that?
Thanks!

Quote from: jecrell on February 10, 2017, 12:37:14 PM
Any clue how to use Harmony to handle "out" variables?

Let's say I want to modify....

Code Select Expand


Verse.Pawn_HealthTracker
public void PreApplyDamage(DamageInfo dinfo, out bool absorbed)

...
with a Prefix.
My goal is to have it return the absorbed as "true" before the main method and prevent the main method from occurring.

How could I do that?
Thanks!

Here is how you do it: the prefix can return a boolean that controls if the original is run. So define your prefix like this:


static bool Prefix(ref bool absorbed)
{
    absorbed = true;
    return false;
}


Sadly, the result is an error.


Could not execute post-long-event action. Exception: System.TypeInitializationException: An exception was thrown by the type initializer for CompDeflector.HarmonyCompDeflector ---> System.Exception: Parameter "absorbed" not found in method Void PreApplyDamage(DamageInfo, Boolean ByRef)
  at Harmony.MethodPatcher.EmitCallParameter (System.Reflection.Emit.ILGenerator il, System.Reflection.MethodBase original, System.Reflection.MethodInfo patch, System.Collections.Generic.Dictionary`2 variables, Boolean noOut) [0x00000] in <filename unknown>:0
  at Harmony.MethodPatcher+<>c__DisplayClass7_0.<AddPrefixes>b__0 (System.Reflection.MethodInfo fix) [0x00000] in <filename unknown>:0
  at System.Collections.Generic.List`1[System.Reflection.MethodInfo].ForEach (System.Action`1 action) [0x00000] in <filename unknown>:0
  at Harmony.MethodPatcher.AddPrefixes (System.Reflection.Emit.ILGenerator il, System.Reflection.MethodBase original, System.Collections.Generic.List`1 prefixes, System.Collections.Generic.Dictionary`2 variables, Label label) [0x00000] in <filename unknown>:0
  at Harmony.MethodPatcher.CreatePatchedMethod (System.Reflection.MethodBase original, System.Collections.Generic.List`1 prefixes, System.Collections.Generic.List`1 postfixes, System.Collections.Generic.List`1 processors) [0x00000] in <filename unknown>:0
  at Harmony.PatchFunctions.UpdateWrapper (System.Reflection.MethodBase original, Harmony.PatchInfo patchInfo) [0x00000] in <filename unknown>:0
  at Harmony.PatchProcessor.Patch () [0x00000] in <filename unknown>:0
  at Harmony.HarmonyInstance.Patch (System.Reflection.MethodBase original, Harmony.HarmonyMethod prefix, Harmony.HarmonyMethod postfix, Harmony.HarmonyProcessor infix) [0x00000] in <filename unknown>:0
  at CompDeflector.HarmonyCompDeflector..cctor () [0x00000] in <filename unknown>:0
  --- End of inner exception stack trace ---
  at (wrapper managed-to-native) System.Runtime.CompilerServices.RuntimeHelpers:RunClassConstructor (intptr)
  at System.Runtime.CompilerServices.RuntimeHelpers.RunClassConstructor (RuntimeTypeHandle type) [0x00000] in <filename unknown>:0
  at Verse.StaticConstructorOnStartupUtility.CallAll () [0x00000] in <filename unknown>:0
  at Verse.PlayDataLoader.<DoPlayLoad>m__6F8 () [0x00000] in <filename unknown>:0
  at Verse.LongEventHandler.ExecuteToExecuteWhenFinished () [0x00000] in <filename unknown>:0

This is what I'm trying to do~~



namespace CompDeflector
{
    [StaticConstructorOnStartup]
    static class HarmonyCompDeflector
    {
        static HarmonyCompDeflector()
        {
            HarmonyInstance harmony = HarmonyInstance.Create("rimworld.jecrell.comps.deflector");

            harmony.Patch(typeof(Pawn_HealthTracker).GetMethod("PreApplyDamage"), new HarmonyMethod(typeof(HarmonyCompDeflector).GetMethod("PreApplyDamagePostFix")), null);

        }



        //=================================== COMPDEFLECTOR

        public static bool PreApplyDamagePostFix(Pawn_HealthTracker __instance, DamageInfo dinfo, ref bool absorbed)
        {
           .......(Code snipped, let me know if you need to see it) absorbed = true;  return false;
        }


I've cut it off there, because I'm not sure you need to see the whole method.

Also, the original method's parameters etc look like....

Verse.Pawn_HealthTracker
public void PreApplyDamage(DamageInfo dinfo, out bool absorbed)

I will look into it, thanks.

little note:

ref and out are special case of syntax sugar and result in exactly same native code
(ref variable) and (& variable) provide equal results.
ref byte and byte* are also equals on postjit level

only possible difference is using ref on managed types, because using & on managed type is not allowed by default compiler (but perfectly possible with hacks)

Quote from: RawCode on February 13, 2017, 11:36:00 PM
little note:

ref and out are special case of syntax sugar and result in exactly same native code
(ref variable) and (& variable) provide equal results.
ref byte and byte* are also equals on postjit level

only possible difference is using ref on managed types, because using & on managed type is not allowed by default compiler (but perfectly possible with hacks)

Yeah, I saw that when comparing IL code for the different cases. BTW, Zhentars problem was actually a stupid bug related to using the wrong parameter index. Fixed in 1.0.7 which has no release yet but is up on master repo. Harmony now can handle any combination of normal/ref/out parameters. And compiler wise out parameters actually make a differences regarding the need to assign values to them so I had to take care of that too.

hello i am curious if this error means that linux mint cinnamon + latest mono ..is not compatable because unlike ubuntu it has some sort of unity bug? should i chase that or is this harmony code issue? (Filename: /home/builduser/buildslave/unity/build/artifacts/generated/common/runtime/UnityEngineDebugBindings.gen.cpp Line: 42)

Fallback handler could not load library /home/leaf/.local/share/Steam/steamapps/common/RimWorld/RimWorldLinux_Data/Mono/x86_64/mono.dll
Fallback handler could not load library /home/leaf/.local/share/Steam/steamapps/common/RimWorld/RimWorldLinux_Data/Mono/x86_64/libmono.dll.so
Fallback handler could not load library /home/leaf/.local/share/Steam/steamapps/common/RimWorld/RimWorldLinux_Data/Mono/x86_64/./mono.dll
Fallback handler could not load library /home/leaf/.local/share/Steam/steamapps/common/RimWorld/RimWorldLinux_Data/Mono/x86_64/./libmono.dll.so
Fallback handler could not load library /home/leaf/.local/share/Steam/steamapps/common/RimWorld/RimWorldLinux_Data/Mono/x86_64/mono.dll
Fallback handler could not load library /home/leaf/.local/share/Steam/steamapps/common/RimWorld/RimWorldLinux_Data/Mono/x86_64/mono
Fallback handler could not load library /home/leaf/.local/share/Steam/steamapps/common/RimWorld/RimWorldLinux_Data/Mono/x86_64/libmono.so
Fallback handler could not load library /home/leaf/.local/share/Steam/steamapps/common/RimWorld/RimWorldLinux_Data/Mono/x86_64/./mono
Fallback handler could not load library /home/leaf/.local/share/Steam/steamapps/common/RimWorld/RimWorldLinux_Data/Mono/x86_64/./libmono.so
Fallback handler could not load library /home/leaf/.local/share/Steam/steamapps/common/RimWorld/RimWorldLinux_Data/Mono/x86_64/mono
Fallback handler could not load library /home/leaf/.local/share/Steam/steamapps/common/RimWorld/RimWorldLinux_Data/Mono/x86_64/libmono.dll
Fallback handler could not load library /home/leaf/.local/share/Steam/steamapps/common/RimWorld/RimWorldLinux_Data/Mono/x86_64/libmono.dll.so
Fallback handler could not load library /home/leaf/.local/share/Steam/steamapps/common/RimWorld/RimWorldLinux_Data/Mono/x86_64/./libmono.dll
Fallback handler could not load library /home/leaf/.local/share/Steam/steamapps/common/RimWorld/RimWorldLinux_Data/Mono/x86_64/./libmono.dll.so
Fallback handler could not load library /home/leaf/.local/share/Steam/steamapps/common/RimWorld/RimWorldLinux_Data/Mono/x86_64/libmono.dll
Exception from long event: System.EntryPointNotFoundException: mono_domain_get
  at (wrapper managed-to-native) Harmony.ILCopying.MonoInternals:mono_domain_get_other ()
  at Harmony.ILCopying.MonoInternals.mono_domain_get () [0x00000] in <filename unknown>:0
  at Harmony.ILCopying.MonoInternals.GetCodeInfo (System.Reflection.MethodBase method, System.Int64& start) [0x00000] in <filename unknown>:0
  at Harmony.ILCopying.Memory.GetMethodStart (System.Reflection.MethodBase method) [0x00000] in <filename unknown>:0
  at Harmony.PatchFunctions.UpdateWrapper (System.Reflection.MethodBase original, Harmony.PatchInfo patchInfo) [0x00000] in <filename unknown>:0
  at Harmony.PatchProcessor.Patch () [0x00000] in <filename unknown>:0
  at Harmony.HarmonyInstance.Patch (System.Reflection.MethodBase original, Harmony.HarmonyMethod prefix, Harmony.HarmonyMethod postfix, Harmony.HarmonyProcessor infix) [0x00000] in <filename unknown>:0
  at PrisonerRansom.ReplacementCode+<>c.<.cctor>b__0_0 () [0x00000] in <filename unknown>:0
  at Verse.LongEventHandler.UpdateCurrentSynchronousEvent (System.Boolean& sceneChanged) [0x00000] in <filename unknown>:0

sorry and thank you

If you figure out how to DLLImport the mono lib on your setup, I can fix that in the code. This line is causing it:


[DllImport("__Internal", EntryPoint = "mono_domain_get")]


No idea what it should be for your Linux.

http://www.mono-project.com/docs/advanced/pinvoke/dllnotfoundexception/

^^ this is it an i think you probably know all this so anyway ill find the dll file an tell you what i can

libmono.so is proper name for linux.

and mac too, but mac have somewhat strange rules on library loading.

Quote from: RawCode on February 21, 2017, 05:00:20 AM
libmono.so is proper name for linux.

and mac too, but mac have somewhat strange rules on library loading.

There are quite a few cases where "libmono.so" as a name does not work. So the real problem is how to choose between the different names. Also, since Harmony is not RimWorld specific, names that contain specific paths are not an option.

using System;
using System.Runtime.InteropServices;

namespace rcrml
{
public unsafe class _Native
{
static public PLATFORM CURRENT_PLATFORM;
public enum PLATFORM
{
EXT,W32,M64,L64
}


//mono_domain_get

[DllImport("__Internal",
EntryPoint="mono_domain_get")]
extern static private void* mono_domain_get__EXT ();
[DllImport("mono.dll",
EntryPoint="mono_domain_get")]
extern static private void* mono_domain_get__W32 ();
[DllImport("libmono.so",
EntryPoint="mono_domain_get")]
extern static private void* mono_domain_get__M64 ();
[DllImport("libmono.so",
EntryPoint="mono_domain_get")]
extern static private void* mono_domain_get__L64 ();

//mono_jit_info_table_find

//void* equal to "native integer", it follows word size based on platform used automatically
public struct _MonoJitInfo
{
//should work on x64 due to variable size of void*
public RuntimeMethodHandle method; //should work properly because IntPtr host void* intenrnally
public void* next_jit_code_hash;
public IntPtr code_start; //same void* under wrappers
public uint  unwind_info;
public int    code_size;
public void* __rest_is_omitted;
//struct is longer actually, but rest of fields does not matter
}

[DllImport("__Internal",
EntryPoint="mono_jit_info_table_find")]
extern static private _MonoJitInfo* mono_jit_info_table_find__EXT (void* domain,void* function);
[DllImport("mono.dll",
EntryPoint="mono_jit_info_table_find")]
extern static private _MonoJitInfo* mono_jit_info_table_find__W32 (void* domain,void* function);
[DllImport("libmono.so",
EntryPoint="mono_jit_info_table_find")]
extern static private _MonoJitInfo* mono_jit_info_table_find__M64 (void* domain,void* function);
[DllImport("libmono.so",
EntryPoint="mono_jit_info_table_find")]
extern static private _MonoJitInfo* mono_jit_info_table_find__L64 (void* domain,void* function);

//mono_code_manager_new

[DllImport("__Internal",
EntryPoint="mono_code_manager_new")]
extern static private void* mono_code_manager_new__EXT ();
[DllImport("mono.dll",
EntryPoint="mono_code_manager_new")]
extern static private void* mono_code_manager_new__W32 ();
[DllImport("libmono.so",
EntryPoint="mono_code_manager_new")]
extern static private void* mono_code_manager_new__M64 ();
[DllImport("libmono.so",
EntryPoint="mono_code_manager_new")]
extern static private void* mono_code_manager_new__L64 ();

//mono_code_manager_reserve

[DllImport("__Internal",
EntryPoint="mono_code_manager_reserve")]
extern static private void* mono_code_manager_reserve__EXT (void* MonoCodeManager, int size);
[DllImport("mono.dll",
EntryPoint="mono_code_manager_reserve")]
extern static private void* mono_code_manager_reserve__W32 (void* MonoCodeManager, int size);
[DllImport("libmono.so",
EntryPoint="mono_code_manager_reserve")]
extern static private void* mono_code_manager_reserve__M64 (void* MonoCodeManager, int size);
[DllImport("libmono.so",
EntryPoint="mono_code_manager_reserve")]
extern static private void* mono_code_manager_reserve__L64 (void* MonoCodeManager, int size);


static public void* mono_code_manager_new (void* MonoCodeManager, int size)
{
if (CURRENT_PLATFORM == PLATFORM.EXT)
return mono_code_manager_reserve__EXT(MonoCodeManager, size);

return mono_code_manager_reserve__W32(MonoCodeManager, size);
}

static public void* mono_code_manager_new ()
{
if (CURRENT_PLATFORM == PLATFORM.EXT)
return mono_code_manager_new__EXT();

return mono_code_manager_new__W32();
}

static public void* mono_domain_get ()
{
if (CURRENT_PLATFORM == PLATFORM.EXT)
return mono_domain_get__EXT();

return mono_domain_get__W32();
}

static public _MonoJitInfo* mono_jit_info_table_find (void* domain,void* function)
{
if (CURRENT_PLATFORM == PLATFORM.EXT)
return mono_jit_info_table_find__EXT(domain,function);

return mono_jit_info_table_find__W32(domain,function);
}

static _Native()
{
string s = Environment.CommandLine;
//last char of command line string
s = s.Substring (s.Length - 1);

switch (s)
{
case "e": //exE
CURRENT_PLATFORM = PLATFORM.W32;
return;
case "l": //dlL
CURRENT_PLATFORM = PLATFORM.EXT;
return;

//C for mac
//4 for linux
}
}
}
}

Thnx RawCode,
I'll give that a try!

Harmony v1.0.8 released. Lots of bug fixes and now supports constructor patching too: https://github.com/pardeike/Harmony/releases/tag/v1.0.8

/Andreas

I think I'm getting the hang of this. Those documentations were superb  :D

*waves rainbows in the air*

If only more modders would see this post. There are still mods out there that use Hooks when it would have been much easier with a simple prefix/postfix.

Anyway, better not interrupt you discussion too much. I'm out O/

umm i looked everyplace i could and i found a folder named mono on my system but it did not contain libmono.dll.so or libmono.so or mono.dll.so .. i'm not sure what you need me to do but i'm willing to do anything to help =) would a directory tree picture help any? (using mono complete)

Quote from: darkitten on February 23, 2017, 07:31:40 AM
umm i looked everyplace i could and i found a folder named mono on my system but it did not contain libmono.dll.so or libmono.so or mono.dll.so .. i'm not sure what you need me to do but i'm willing to do anything to help =) would a directory tree picture help any? (using mono complete)

What about your RimWorld installation. It contains mono.

folder named mono contains folder named bin where all binary files are stored.

issue with native bindings - developer must use very specific library, that used to construct VM, using any other library cause crashing on most methods.

If two methods alter the same method, the library throw a error on startup and even freak out if a game loaded:
You can test it if you use the CleaningArea and the N.V.A mod. Tested on Windows 10


Could not execute post-long-event action. Exception: System.TypeInitializationException: An exception was thrown by the type initializer for Nandonalt_VisualAddons.HarmonyPatches ---> System.ArgumentException: Object type System.Collections.Generic.List`1[Harmony.Patch] cannot be converted to target type: Harmony.Patch[]
Parameter name: val
  at System.Reflection.MonoField.SetValue (System.Object obj, System.Object val, BindingFlags invokeAttr, System.Reflection.Binder binder, System.Globalization.CultureInfo culture) [0x00000] in <filename unknown>:0
  at System.Reflection.FieldInfo.SetValue (System.Object obj, System.Object value) [0x00000] in <filename unknown>:0
  at System.Runtime.Serialization.ObjectRecord.SetMemberValue (System.Runtime.Serialization.ObjectManager manager, System.Reflection.MemberInfo member, System.Object value) [0x00000] in <filename unknown>:0
  at System.Runtime.Serialization.FixupRecord.FixupImpl (System.Runtime.Serialization.ObjectManager manager) [0x00000] in <filename unknown>:0
  at System.Runtime.Serialization.BaseFixupRecord.DoFixup (System.Runtime.Serialization.ObjectManager manager, Boolean strict) [0x00000] in <filename unknown>:0
  at System.Runtime.Serialization.ObjectRecord.DoFixups (Boolean asContainer, System.Runtime.Serialization.ObjectManager manager, Boolean strict) [0x00000] in <filename unknown>:0
  at System.Runtime.Serialization.ObjectManager.RegisterObjectInternal (System.Object obj, System.Runtime.Serialization.ObjectRecord record) [0x00000] in <filename unknown>:0
  at System.Runtime.Serialization.ObjectManager.RegisterObject (System.Object obj, Int64 objectID, System.Runtime.Serialization.SerializationInfo info, Int64 idOfContainingObj, System.Reflection.MemberInfo member, System.Int32[] arrayIndex) [0x00000] in <filename unknown>:0
  at System.Runtime.Serialization.Formatters.Binary.ObjectReader.RegisterObject (Int64 objectId, System.Object objectInstance, System.Runtime.Serialization.SerializationInfo info, Int64 parentObjectId, System.Reflection.MemberInfo parentObjectMemeber, System.Int32[] indices) [0x00000] in <filename unknown>:0
  at System.Runtime.Serialization.Formatters.Binary.ObjectReader.ReadNextObject (System.IO.BinaryReader reader) [0x00000] in <filename unknown>:0
  at System.Runtime.Serialization.Formatters.Binary.ObjectReader.ReadObjectGraph (BinaryElement elem, System.IO.BinaryReader reader, Boolean readHeaders, System.Object& result, System.Runtime.Remoting.Messaging.Header[]& headers) [0x00000] in <filename unknown>:0
  at System.Runtime.Serialization.Formatters.Binary.BinaryFormatter.NoCheckDeserialize (System.IO.Stream serializationStream, System.Runtime.Remoting.Messaging.HeaderHandler handler) [0x00000] in <filename unknown>:0
  at System.Runtime.Serialization.Formatters.Binary.BinaryFormatter.Deserialize (System.IO.Stream serializationStream) [0x00000] in <filename unknown>:0
  at Harmony.PatchInfoSerialization.Deserialize (System.Byte[] bytes) [0x00000] in <filename unknown>:0
  at Harmony.HarmonySharedState.GetPatchInfo (System.Reflection.MethodBase method) [0x00000] in <filename unknown>:0
  at Harmony.PatchProcessor.Patch () [0x00000] in <filename unknown>:0
  at Harmony.HarmonyInstance.<PatchAll>b__5_0 (System.Type type) [0x00000] in <filename unknown>:0
  at Harmony.CollectionExtensions.Do[Type] (IEnumerable`1 sequence, System.Action`1 action) [0x00000] in <filename unknown>:0
  at Harmony.HarmonyInstance.PatchAll (System.Reflection.Assembly assembly) [0x00000] in <filename unknown>:0
  at Nandonalt_VisualAddons.HarmonyPatches..cctor () [0x00000] in <filename unknown>:0
  --- End of inner exception stack trace ---
  at (wrapper managed-to-native) System.Runtime.CompilerServices.RuntimeHelpers:RunClassConstructor (intptr)
  at System.Runtime.CompilerServices.RuntimeHelpers.RunClassConstructor (RuntimeTypeHandle type) [0x00000] in <filename unknown>:0
  at Verse.StaticConstructorOnStartupUtility.CallAll () [0x00000] in <filename unknown>:0
  at Verse.PlayDataLoader.<DoPlayLoad>m__6F8 () [0x00000] in <filename unknown>:0
  at Verse.LongEventHandler.ExecuteToExecuteWhenFinished () [0x00000] in <filename unknown>:0
Verse.Log:Error(String)
Verse.LongEventHandler:ExecuteToExecuteWhenFinished()
Verse.LongEventHandler:UpdateCurrentAsynchronousEvent()
Verse.LongEventHandler:LongEventsUpdate(Boolean&)
Verse.Root:Update()
Verse.Root_Entry:Update()


Already working on it.

I have commited a bunch of fixes to Harmony v1.0.9 (not released until fully tested). So anyone who likes to verify it can go and grab the master from https://github.com/pardeike/Harmony and test. It is important that all involved mods use that version for proper testing.

Thanks and keep the feedback coming.

After recompiling CleaningArea and N.V.A mod with the new harmony assembly it works fine now. But havnt tested it completly. Just the usecases that are used in both mods. Good job!

BTW: Keep up with the good WIKI. Very well explained. Dont let it go bad by missing explaining features.

Quote from: Hatti on March 14, 2017, 02:12:11 PM
After recompiling CleaningArea and N.V.A mod with the new harmony assembly it works fine now. But havnt tested it completly. Just the usecases that are used in both mods. Good job!

BTW: Keep up with the good WIKI. Very well explained. Dont let it go bad by missing explaining features.

Thanks! Make sure you have the very-latest, I updated the master branch a few times today. Latest update fixes Traverse bugs!

Now I will test it with my own mods and if nothing special comes up, I will release version 1.0.9 officially. I still need someone to test it on Linux. I added code from RawCode that in theory should make it work on all major platforms.

So please: if anybody could test the github version on a simple example mod and see if it runs on linux. That would be super cool!

Thanks,
Andreas

Compiled it maybe 20 minutes before the post i made.

Btw take a look at HarmonySharedState line 39. Should be "stateField == null"

Quote from: Hatti on March 14, 2017, 05:25:44 PM
Compiled it maybe 20 minutes before the post i made.

Btw take a look at HarmonySharedState line 39. Should be "stateField == null"

Excellent find!

Today I removed all platform dependent code from Harmony. I will put up a new release within this weekend. I'll update the master branch tonight. Release this weekend!

Alright. It's time. I released Harmony 1.0.9 and it is available on github:
https://github.com/pardeike/Harmony/releases/tag/v1.0.9

There is probably little to nothing you need to do beside a recompile with the new released dll. Since the internal sharing mechanism has changed, it is important that everyone updates to 1.0.9 as soon as possible.

Thanks to all the friendly people from the RimWorld community that helped me on the journey: Fluffy, Zhentar, UnlimitedHugs, erdelf, SkullyWag, DonationBox, Fumblesneeze and many more!

Andreas Pardeike

I could be wrong - but it appears that with your recent update, mods that are a mix of those who have and have not taken the update breaks drawing in the game. Im getting a ton of errors like this after Call of Cthulu updated with Harmony 1.0.9 and clearly other mods in my list use Harmony :

MissingMethodException: Method not found: 'Harmony.Traverse.GetValue'.
at (wrapper dynamic-method) Verse.PawnRenderer.RenderPawnInternal_Patch1 (object,UnityEngine.Vector3,UnityEngine.Quaternion,bool,Verse.Rot4,Verse.Rot4,Verse.RotDrawMode,bool) <0x00170>
at Verse.PawnRenderer.RenderPortait () <0x001b1>
at RimWorld.PortraitRenderer.OnPostRender () <0x00027>

i forgot how do you install harmony on rimworld?

Quote from: Robloxsina66 on March 20, 2017, 07:59:43 AM
i forgot how do you install harmony on rimworld?

To avoid a situation where mods are dependent on another mod and cannot be released because that mod is not updated quickly enough, Harmony does not work like that. Instead it is a simple library that each developer bundles with their mod. Harmony itself is not RimWorld specific and needs not to stay in sync with RimWorld changes.

Quote from: kanid99 on March 18, 2017, 12:31:52 PM
I could be wrong - but it appears that with your recent update, mods that are a mix of those who have and have not taken the update breaks drawing in the game. Im getting a ton of errors like this after Call of Cthulu updated with Harmony 1.0.9 and clearly other mods in my list use Harmony

Harmony 1.0.9 broke the original promise of being compatible across different mods and Harmony versions. It was a bug that was necessary to fix. But as of 1.0.9 the risk that this happens again is very low.

This means that we need to keep nagging everyone to update to 1.0.9 to archive stability. This is unfortunate but a side effect of introducing a complicated tool like Harmony. I still think that it serves more value than this single incident costs and so far 1.0.9 works flawless - even across different platforms.

My apologies

Quote from: pardeike on March 20, 2017, 11:31:39 AM

Quote from: Robloxsina66 on March 20, 2017, 07:59:43 AM
i forgot how do you install harmony on rimworld?

To avoid a situation where mods are dependent on another mod and cannot be released because that mod is not updated quickly enough, Harmony does not work like that. Instead it is a simple library that each developer bundles with their mod. Harmony itself is not RimWorld specific and needs not to stay in sync with RimWorld changes.

ah you gotta put the Harmony stuff in the mod then to work?

Quote from: Robloxsina66 on March 20, 2017, 07:59:43 AM
ah you gotta put the Harmony stuff in the mod then to work?

Yes

ah ok

Hi, pardeike, first thanks for creating Harmony, my small project couldn't have even started without something like Harmony around to be sure that in the future it will "gracefully collide" against other mods.

If you had the time could you check why I'm getting this totally unexpected error?:

Exception in Tick (pawn=Irgo, job=AttackMelee A=Thing_Mamuffalo337661, CurToil=2): System.Reflection.TargetParameterCountException: parameters do not match signature
at System.Reflection.MonoMethod.Invoke (object,System.Reflection.BindingFlags,System.Reflection.Binder,object[],System.Globalization.CultureInfo) <0x001c2>
at System.Reflection.MethodBase.Invoke (object,object[]) <0x00025>
at Harmony.Traverse.GetValue () <0x00100>
at Harmony.Traverse.Resolve () <0x00024>
at Harmony.Traverse.Field (string) <0x0001b>
at MeleeParry.MeleeParryPatch.Prefix (RimWorld.Verb_MeleeAttack,bool&) <0x0043f>
at (wrapper dynamic-method) RimWorld.Verb_MeleeAttack.TryCastShot_Patch1 (object) <0x0002b>
at Verse.Verb.TryCastNextBurstShot () <0x0001a>
at Verse.Verb.WarmupComplete () <0x00028>
at Verse.Verb.TryStartCastOn (Verse.LocalTargetInfo,bool,bool) <0x0050c>
at RimWorld.Pawn_MeleeVerbs.TryMeleeAttack (Verse.Thing,Verse.Verb,bool) <0x001dc>
at Verse.AI.JobDriver_AttackMelee/<MakeNewToils>c__Iterator194.<>m__664 () <0x00080>
at Verse.AI.Toils_Combat/<FollowAndMeleeAttack>c__AnonStorey44E.<>m__6B6 () <0x0022a>
at Verse.AI.JobDriver.DriverTick () <0x00322>

Verse.Log:Error(String)
Verse.AI.JobDriver:DriverTick()
Verse.AI.Pawn_JobTracker:JobTrackerTick()
Verse.Pawn:Tick()
Verse.TickList:Tick()
Verse.TickManager:DoSingleTick()
Verse.TickManager:TickManagerUpdate()
Verse.Game:Update()
Verse.Root_Play:Update()


I traced the error to the following line. I added context related to the operation but the commented line is the one causing the error (The rest of Traverse operations work flawlessly):



    [HarmonyPatch(typeof(Verb_MeleeAttack))]
    [HarmonyPatch("TryCastShot")]
    public static class MeleeParryPatch
    {

        public static bool Prefix(Verb_MeleeAttack __instance, ref bool __result)
        {
            var verbMA = Traverse.Create(__instance);
            LocalTargetInfo target = verbMA.Field("currentTarget").GetValue<LocalTargetInfo>();
            Thing thing = target.Thing;

            // The following is the line triggering the error
            bool surprise = verbMA.Field("surpriseAttack").GetValue<bool>();




Meanwhile, on a previous test, that Prefixed other method OF THE SAME CLASS, using the same ways to access THE SAME FIELD nothing failed and everything was working according to design in-game. Here is an extract of the patch for the other.



    [HarmonyPatch(typeof(Verb_MeleeAttack))]
    [HarmonyPatch("GetHitChance")]
    public static class MeleeParryPatch
    {
        public static bool Prefix(Verb_MeleeAttack __instance, ref float __result, LocalTargetInfo target)
        {
            var verbMA = Traverse.Create(__instance);
            __result = verbMA.Field("DefaultHitChance").GetValue<float>(); // Default chances if anything else fails to evaluate
            if (verbMA.Field("surpriseAttack").GetValue<bool>() || verbMA.Method("IsTargetImmobile", target).GetValue<bool>()){



For reference this are the relevant class definitions, according to ILSpy:


public class Verb_MeleeAttack : Verb
{
                private const float DefaultHitChance = 0.6f;

protected override bool TryCastShot()
{
                         (...)
}

private float GetHitChance(LocalTargetInfo target)
{
                       (...)
}


And the relevant fields on the definition of the base class Verb:


public abstract class Verb : ILoadReferenceable, IExposable
{
protected LocalTargetInfo currentTarget = null;

protected bool surpriseAttack;



Both sniplets of code aren't compiled together (I reused the same class name but disabled ALL THE CODE and cleaned the ENTIRE project of any remaining data)... I'm at the "learning what can be done" phase and all this are temporal tests on how the final solution can be really implemented (I wanted to also take the chance to practice with your Library).

I'm puzzled because ONLY the access to the "surpriseAttack" field is failing while the rest of Traverse operations work without issues... The only remarkable difference I find is the type of methods I'm patching and that the problematic one is an override and also lacks arguments, compared to the "good one" were the same traverse operations work without a hitch.

Any ideas? Extra tests I can try?

EDIT: Silly me, forgot the most important info, this is using Harmony 1.0.9 on VS 2017.

EDIT2: Replacing all Traverse calls by direct Reflections using GetField & GetMethod, made everything working back again... So there is something I must be doing wrong while using your Traverse tool.

I'll look into it. Thanks for the report, oreganor

Thanks for taking the time to look into it. I will publish my mod before the weekend (Need feedback) but will hold on the Sources until I can offer a full integration with the Traverse tool, ATM my code is a mess :).

Quote from: oreganor on March 23, 2017, 04:17:41 AM
Thanks for taking the time to look into it. I will publish my mod before the weekend (Need feedback) but will hold on the Sources until I can offer a full integration with the Traverse tool, ATM my code is a mess :).

could you isolate the problem into a separate project or file - that would help me tremendously

Sure... This evening when I reach home I will create the test project that triggers the error, pack it and send you a google drive link on a PM.

Quote from: oreganor on March 23, 2017, 05:42:26 AM
Sure... This evening when I reach home I will create the test project that triggers the error, pack it and send you a google drive link on a PM.

Awesome

Quote from: oreganor on March 23, 2017, 05:42:26 AM
Sure... This evening when I reach home I will create the test project that triggers the error, pack it and send you a google drive link on a PM.

Thank you. I have successfully tested with your test project and found two bugs in Harmony that otherwise would have not been spotted. I have updated the github master with the fix: https://github.com/pardeike/Harmony/releases/tag/v1.0.9.1

One change to your code though:

var DamageInfosToApply = Traverse.Create(vMA).Method("DamageInfosToApply", new Type[] { typeof(LocalTargetInfo) });
foreach (DamageInfo current in (DamageInfosToApply.GetValue<IEnumerable<DamageInfo>>(target))) { }

Method("DamageInfosToApply") returns null because Method() needs you to specify the method exactly. That method has one parameter of type LocalTargetInfo, which I added in the code above. Also, I move the traverse instance out of the loop code just for good measures.

Please let me know if helps you.
Andreas

Woah! All Traverse calls working flawlessly.

Thanks a lot, specially for helping at the correct syntax for those calls to private methods. Glad I could help you at squashing some bugs.

Time for a cleanup on my mod code to publish also some decent sources.

Keep up the good work, oreganor!

Hey, sorry for the noobishness, working on something pretty simple and running into issues.  This is my attempt to change the formula for skill levels.  "Test Boot" is logged at startup, but never in game "Test Patch".  I also previously tried a prefix version with no results.

namespace GeneralBalanceAdvanced
{
    [StaticConstructorOnStartup]
    class HarmonyBalanceBooter
    {
        static HarmonyBalanceBooter()
        {
            var harmony = HarmonyInstance.Create("com.greep.balance.rimworld.mod.release");
            harmony.PatchAll(Assembly.GetExecutingAssembly());
            Log.Warning("Test Boot");
        }
    }
}

namespace GeneralBalanceAdvanced
{
    [HarmonyPatch(typeof(SkillRecord))]
    [HarmonyPatch("XpRequiredToLevelUpFrom")]
   class SkillCostPatcher
    {
        public static void Postfix(SkillRecord __instance, ref float __result, ref int startingLevel)
        {
            Log.Warning("Test Patch");
            int val = startingLevel;
            __result = val * val * 30 + val * 110 + 350;
        }
    }
}

I could just be misunderstanding rimworlds code there, because this logs and does work as expected e.g.

    [HarmonyPatch(typeof(SkillRecord))]
    [HarmonyPatch("Interval")]
    public static class SkillIntervalPatcher
    {
        public static void Prefix(SkillRecord __instance)
        {
            __instance.Learn(-1f);
            Log.Warning("Test Patch 2");
        }
    }

I guess that XpRequiredToLevelUpFrom() isn't called as often as you want. Must be something like that because your code looks ok and patching works for you in general.

K, back with another strange issue. This time moving from autopatching approach as wiki explained to manual patching as wiki also explains (My little mod now Detours another method that's also detoured by another mod (Not using Harmony)... So I'm at the 1st step to see if I can make conditional patching based on the pressence of the Other Mod, to avoid the collision).

Still haven't even touched anything related to the final goal... I was just, in theory, converting my code to "manual patching" mode as wiki explains.

From this:


            //Initializing Harmony detours
            var harmony = HarmonyInstance.Create("net.oreganor.rimworld.mod.meleerebalance");
            harmony.PatchAll(Assembly.GetExecutingAssembly());


To this:

   
            //Initializing Harmony detours
            var harmony = HarmonyInstance.Create("net.oreganor.rimworld.mod.meleerebalance");

            var original = typeof(Verb_MeleeAttack).GetMethod("TryCastShot");
            var detour = typeof(VerbMeleeTryCastShotPatch).GetMethod("Prefix");
            harmony.Patch(original, new HarmonyMethod(detour), new HarmonyMethod(null));

            original = typeof(Pawn_DraftController).GetMethod("GetGizmos");
            detour = typeof(Pawn_DraftControllerGetGizmosPatch).GetMethod("Postfix");
            harmony.Patch(original, new HarmonyMethod(null), new HarmonyMethod(detour));


Which is just what wiki describes with the added null definitions according to what HarmonyInstance.Patch expects internally because I only need a fix in each Detour (one Prefix for the 1st, and a Postfix for the 2nd).

This is the error-stack I get from Rimworld when it loads:


Could not execute post-long-event action. Exception: System.TypeInitializationException: An exception was thrown by the type initializer for MeleeRebalance.MainController ---> System.ArgumentNullException: Argument cannot be null.
Parameter name: key
  at System.Collections.Generic.Dictionary`2[System.Reflection.MethodBase,System.Byte[]].TryGetValue (System.Reflection.MethodBase key, System.Byte[]& value) [0x00000] in <filename unknown>:0
  at Harmony.GeneralExtensions.GetValueSafe[MethodBase,Byte[]] (System.Collections.Generic.Dictionary`2 dictionary, System.Reflection.MethodBase key) [0x00000] in <filename unknown>:0
  at Harmony.HarmonySharedState.GetPatchInfo (System.Reflection.MethodBase method) [0x00000] in <filename unknown>:0
  at Harmony.PatchProcessor.Patch () [0x00000] in <filename unknown>:0
  at Harmony.HarmonyInstance.Patch (System.Reflection.MethodBase original, Harmony.HarmonyMethod prefix, Harmony.HarmonyMethod postfix, Harmony.HarmonyMethod transpiler) [0x00000] in <filename unknown>:0
  at MeleeRebalance.MainController..cctor () [0x00000] in <filename unknown>:0
  --- End of inner exception stack trace ---
  at (wrapper managed-to-native) System.Runtime.CompilerServices.RuntimeHelpers:RunClassConstructor (intptr)
  at System.Runtime.CompilerServices.RuntimeHelpers.RunClassConstructor (RuntimeTypeHandle type) [0x00000] in <filename unknown>:0
  at Verse.StaticConstructorOnStartupUtility.CallAll () [0x00000] in <filename unknown>:0
  at Verse.PlayDataLoader.<DoPlayLoad>m__6F8 () [0x00000] in <filename unknown>:0
  at Verse.LongEventHandler.ExecuteToExecuteWhenFinished () [0x00000] in <filename unknown>:0
Verse.Log:Error(String)
Verse.LongEventHandler:ExecuteToExecuteWhenFinished()
Verse.LongEventHandler:UpdateCurrentAsynchronousEvent()
Verse.LongEventHandler:LongEventsUpdate(Boolean&)
Verse.Root:Update()
Verse.Root_Entry:Update()



The stack is so deep, so many tests passed... That I ran out of ideas (Tested even with dummy REAL postfix & prefix methods that did some bogus actions and then return instead of the null declarations... The error was the same).

What I'm missing?

EDIT: The other Mod has been completely disabled over the course of ALL this tests. In fact I already did collision tests when patching automatically and nothing happens (My Detour is just ignored, as it should be).

EDIT2: I added the 2 manual patches as reference... Just in case. I tested each of them individually and the error was the same. The only thing I'm missing is to add specifially a dummy Transpiler... The Method prototype marks it as optional, so that would be a really exotic bug.

An update, pardeike. I found the issue, after some headbanging sessions checking the code flow differences between automatic patching and manual patching... Certainly the original method could be null as deep as to the GetValueSafe call...

...I forgot to overload with the appropiate BindingFlags for the method in question so, on the code above if I replace:


var original = typeof(Verb_MeleeAttack).GetMethod("TryCastShot");


with:


var original = typeof(Verb_MeleeAttack).GetMethod("TryCastShot", BindingFlags.Static | BindingFlags.Public);


Original method is now non-null and detouring works like a charm. So now I can start checking if I can make conditional patching working.


It would be nice if you warned specifically about this pitfall on the Wiki's example... Because amateur moders like me, just have the habit of "copy'n paste" examples without really understanding the innards of what we are doing :) . I think it would save some headaches to both the user and to yourself by not having to loose time in reports like the above.

Nice that you solved the problem. But regarding your suggested documentation change: no offense but I consider that part basic knowledge of handling MethodInfo. If you patch manually, I expect you to handle fetching MethodInfos the correct way.

QuoteBut regarding your suggested documentation change: no offense but I consider that part basic knowledge of handling MethodInfo. If you patch manually, I expect you to handle fetching MethodInfos the correct way.

None taken :)... It's your documentation and you decide which kind of users it's aimed for.

I'm probably missing something incredibly obvious, especially since nobody else seems to have reported this, but.... Whenever I load the game with any mods utilizing Harmony in the mod list, I end up with a "harmony.log.txt" file on my desktop. How do I stop that from happening?

Quote from: dburgdorf on April 15, 2017, 04:34:22 PM
I'm probably missing something incredibly obvious, especially since nobody else seems to have reported this, but.... Whenever I load the game with any mods utilizing Harmony in the mod list, I end up with a "harmony.log.txt" file on my desktop. How do I stop that from happening?

At least one of your mods uses Harmony and has the debug flag set in the initialization code. Which one I don't know.

Hi everyone, I just would like to share a quick snippet with all of you. This one serves as a mods Main.cs and includes a Harmony patch that loads a specific save game when you start Rimworld with the command line parameters "-rungame game_file_name" instead of "-quicktest". Enjoy!

https://gist.github.com/pardeike/12c457e135a42a28e068f2aba5337221

/Andreas

How do i install this mod? both Harmony, Harmony Tests and Harmony master appear red on my mods folder, am i missing something? or am i just dumb and there are no mods that use this library yet?

You should not need to install the library separately.
The general convention should be that each mod that used Harmony includes the .dll in its download and you dont have to do anything else.

Because HugsLib doesn't do detouring now, I had a look at Harmony for one of my mods.

I got all the way to releasing the mod on Workshop before I realised "whoops, my detour doesn't actually work."

I'm trying to override DefaultParmsNow in StoryTellerUtility with my own calculation, but it's not happening. As far as I am able to tell (I've been turning on 'Write Storyteller' and monitoring the result using the Debug Inspector) the game is continuing to use the vanilla calculation.

Is Harmony not suitable for this or am I just using it wrong?

Quote from: MarvinKosh on June 07, 2017, 05:13:49 PM
Because HugsLib doesn't do detouring now, I had a look at Harmony for one of my mods.

I got all the way to releasing the mod on Workshop before I realised "whoops, my detour doesn't actually work."

I'm trying to override DefaultParmsNow in StoryTellerUtility with my own calculation, but it's not happening. As far as I am able to tell (I've been turning on 'Write Storyteller' and monitoring the result using the Debug Inspector) the game is continuing to use the vanilla calculation.

Is Harmony not suitable for this or am I just using it wrong?

I can't see why it should not work with that method. Can you post your source?

Sure thing.

I've tried a few things so if it seems off it's because I've been smashing my caveman head at it. ;)

namespace CRC_Reintegrated
{

[StaticConstructorOnStartup]
public static class CRC_Loader
{

static HarmonyInstance harmony;

static void PatchSTU()
{
harmony = HarmonyInstance.Create("net.marvinkosh.rimworld.mod.combatreadinesscheck");



var original = typeof(StorytellerUtility).GetMethod("DefaultParmsNow", BindingFlags.Static | BindingFlags.Public);
var detour = typeof(MarvsStoryTellerUtility).GetMethod("ModdedParmsNow");
harmony.Patch(original, new HarmonyMethod(null), new HarmonyMethod(detour));

//harmony.PatchAll(Assembly.GetExecutingAssembly());

}


}



public static class MarvsStoryTellerUtility
{

const float PowerLimit = 1000;
const float PointsPer1000BuildingWealth = 5.5f;


public static IncidentParms ModdedParmsNow(StorytellerDef tellerDef, IncidentCategory incCat, IIncidentTarget target)
{
if (tellerDef == null)
{
tellerDef = StorytellerDefOf.Cassandra;
}

var incidentParms = new IncidentParms();
incidentParms.target = target;

var map = target as Map;

if (incCat == IncidentCategory.ThreatSmall || incCat == IncidentCategory.ThreatBig)
{

float buildingPoints = 0;

if (map != null)
{
buildingPoints = (map.wealthWatcher.WealthBuildings - 1000) / 1000 * PointsPer1000BuildingWealth;
if (buildingPoints < 0)
{
buildingPoints = 0;
}
}

float colonistPoints = 0;
float armouryPoints = 0;

if (map != null)
{
IEnumerable<Pawn> colonists = map.mapPawns.FreeColonists;
ArmouryUtility.GetColonistArmouryPoints(colonists, target, out colonistPoints, out armouryPoints);
}
else
{
var caravan = target as Caravan;
if (caravan != null)
{
IEnumerable<Pawn> caravaneers = caravan.PawnsListForReading.FindAll((Pawn dude) => dude.IsColonist && dude.HostFaction == null);
ArmouryUtility.GetColonistArmouryPoints(caravaneers, target, out colonistPoints, out armouryPoints);
}
}


//float num3 = (float)Find.MapPawns.FreeColonistsCount * PointsPerColonist;
float difficultyFactor = Find.Storyteller.difficulty.threatScale;
if (difficultyFactor < 0.65)
{
buildingPoints *= 0.5f;
}

incidentParms.points = colonistPoints;
incidentParms.points *= Find.StoryWatcher.watcherRampUp.TotalThreatPointsFactor;
incidentParms.points *= difficultyFactor;
incidentParms.points += buildingPoints;
incidentParms.points += armouryPoints;
switch (Find.StoryWatcher.statsRecord.numThreatBigs)
{
case 0:
incidentParms.points = 35;
incidentParms.raidForceOneIncap = true;
incidentParms.raidNeverFleeIndividual = true;
break;
case 1:
incidentParms.points *= 0.5f;
break;
case 2:
incidentParms.points *= 0.7f;
break;
case 3:
incidentParms.points *= 0.8f;
break;
case 4:
incidentParms.points *= 0.9f;
break;
default:
incidentParms.points *= 1;
break;
}
if (incidentParms.points < 0f)
{
incidentParms.points = 0f;
}
if (incidentParms.points > PowerLimit)
{
if (difficultyFactor > 0.65)
{
incidentParms.points = PowerLimit - 1 + Mathf.Pow(incidentParms.points + 1 - PowerLimit, 0.87f);
}
else
{

incidentParms.points = PowerLimit - 1 + Mathf.Pow(incidentParms.points + 1 - PowerLimit, 0.8f);
}
}
}
else if (incCat == IncidentCategory.CaravanTarget)
{
var caravan = incidentParms.target as Caravan;
IEnumerable<Pawn> playerPawns;
if (caravan != null)
{
playerPawns = caravan.PawnsListForReading;
}
else
{
Faction playerFaction = Faction.OfPlayer;
playerPawns = from x in ((Map)incidentParms.target).mapPawns.AllPawnsSpawned
where x.Faction == playerFaction || x.HostFaction == playerFaction
select x;
}
incidentParms.points = CaravanIncidentUtility.CalculateIncidentPoints(playerPawns);
}


return incidentParms;

}//End of ModdedParmsNow

}//End of class




}//End of namespace

First, I would log out if "original" and "detour" are not null. Because if you made a small mistake there, nothing will happen when one of them is null.

Second, your patch is a Postfix which means that it will run after the original method has run. Is that your intended behavior?

/Andreas

So a postfix patch is used if you definitely want to use the original method but do something afterwards no matter what, prefix patch is used when you may want to use the original method but do something beforehand, but there is the option to skip the original method entirely. Is that right?

It would seem like either method could work, however, since I already replace the original calculation it would probably be best to do a prefix patch and skip the original method.

I did a check for null like you suggested - it seems that there's no problem finding the original or detour methods.

I think my main problem is that I don't quite get how Harmony is supposed to decide when to skip the original method. I've been reading the wiki, and I think I'm starting to understand, but putting it into practise is probably going to require some experimentation.

Okay I think I have a more basic problem. I put an unconditional Log.Message string in there to test. The code just doesn't run at all. ;)

You are talking about replacing the original method with your own. I did not look up and compare both but I would like to point out that Harmony isn't just simple detouring. That would not improve compatibility much.

So my question is: how much of that code overlaps with the original? What are you trying to do? You can also watch the log and see if it throws any errors during startup. Maybe use the annotation syntax instead of the manual patching. And finally, you might try to add a HarmonyInstance.DEBUG = true at the very beginning and Harmony will spit out an extra debug log to your desktop.

To actually allow other mods to get at DefaultParmsNow in addition to my mod, I would need to rewrite it so that it is more easily moddable. Which actually, I have sort of started to do by putting some of the point calculations in their own utility. By default, two mods which attempt to patch it are inevitably going to fight. :)

Like I said above, the main problem is that none of the code runs, not even logging. So that sort of limits the possibilities: RimWorld could be busted, or the thing I use to write and compile code could be busted.

Tried compiling the project with something else. Same result. Eh, I guess I'll do something else and come back to it later.

Stupid question time ;)

I'm doing a tiny dll-only mod that needs to patch some default functions and thus uses Harmony (Yay for structured detours, and not everyone-roll-your-own!!)

Soooooo... What's up with the harmonycheck assembly? I've noticed Fluffy putting that into each of his mods that uses Harmony, but it's not part of the Harmony github releases. What does it do? Where does it come from? What's the licensing situation for that assembly?


Harmony and ModSettings... Harmony expects static functions for pre/postfix code. Modsettings are stored per Mod instance... Those two dont mix ;)

Is there a way to get the running mod from inside a pre/postfix method? I'm loathe to go the Assembly.GetExecutingAssembly().GetTypes().OfType<Mod>().FirstOrDefault()?.Settings... route inside the pre-/postfix handler...
Would static settings inside the HarmonyPatch owner type be the way to go? Has anybody done this already where the source is available so I can take a look?

Where do i put it? i want to play star wars mod, but an error occurs with jecstools, and the game say that this mod version is unknown, like its author.

Quote from: Rexaurius on June 21, 2017, 08:38:32 PM
Where do i put it? i want to play star wars mod, but an error occurs with jecstools, and the game say that this mod version is unknown, like its author.

If you are a mod user, you never need to install Harmony, it is incorporated into the mod. Unlike libraries like CCL, HugsLib, etc., which do need to be installed.

I had an unreleased mod working great in A16 with destructive detouring, but I just have no comprehension as to how I'd go about replacing it with a non-destructive detour.  If I understand how things have changed since the old detouring methods:

• Prefixing is either destructive detouring, which is strongly advised against, or merely hooking, which won't affect the function I want to replace.
  
• Postfixing is non-destructive detouring which doesn't affect the function I want to replace and allows it to run.
  
• Transpiling requires a Master's in Computer Science with at least five years experience in a AAA game studio. =)
  

Overriding the boolean properties I want to override is simple enough, but there's also a very big and grotesque method where I need to chop out part of the code.

How does one go about transpiling, more specifically hacking out a boolean comparison in that function?  The way to do it would be either: to nullify the code for that block, which would be slightly more efficient; or just to change the IL for the condition in the if to evaluate as nothing more than if(false).

Since there's no way to discuss this meaningfully without spilling the beans on the mod (which I have far too much history knowing it's death knell for ever releasing that mod), here's the specifics:

Easy for me: Rimworld.Planet.Caravan.ImmobilizedByMass (property) => Prefix destructive detour always to return false.
Easy for me: Rimworld.Planet.Caravan.TicksPerMove (property) => Postfix non-destructive detour to modify return value to reflect penalties to overloaded caravans.
Impossible for me: Rimworld.Dialog_FormCaravan.CheckForErrors() (method) => Transpile to remove segment of code that prevents formation of caravan if MassUsage exceeds MassCapacity.

The old fully-functional HugsLib detour I was using was simply reflecting the previous private variables and then duplicating all of the vanilla code except for a commented-out section.  But without knowing even the remotest thing about transpilation I have no understanding of how to go about doing something similar with Harmony:

class Dialog_Caravan_Detour : Dialog_FormCaravan
{
public Dialog_Caravan_Detour(Map map, bool reform = false) : base(map, reform) {
}

[DetourMethod(typeof(Dialog_FormCaravan), "CheckForErrors")]
private bool CheckForErrors(List<Pawn> pawns) {
Map map = (Map)typeof(Dialog_FormCaravan).GetField("map", BindingFlags.NonPublic|BindingFlags.Instance).GetValue(this);
bool reform = (bool)typeof(Dialog_FormCaravan).GetField("reform", BindingFlags.NonPublic|BindingFlags.Instance).GetValue(this);
List<TransferableOneWay> transferables = (List<TransferableOneWay>)typeof(Dialog_FormCaravan).GetField("transferables", BindingFlags.NonPublic|BindingFlags.Instance).GetValue(this);
int startingTile = (int)typeof(Dialog_FormCaravan).GetField("startingTile", BindingFlags.NonPublic|BindingFlags.Instance).GetValue(this);

if (!reform && startingTile < 0) {
Messages.Message("NoExitDirectionForCaravanChosen".Translate(), MessageSound.RejectInput);
return false;
}
if (!pawns.Any((Pawn x) => CaravanUtility.IsOwner(x, Faction.OfPlayer) && !x.Downed)) {
Messages.Message("CaravanMustHaveAtLeastOneColonist".Translate(), MessageSound.RejectInput);
return false;
}
/* Disabled for detour
if (!this.reform && this.MassUsage > this.MassCapacity) {
this.FlashMass();
Messages.Message("TooBigCaravanMassUsage".Translate(), MessageSound.RejectInput);
return false;
}
*/
Pawn pawn = pawns.Find((Pawn x) => !x.IsColonist && !pawns.Any((Pawn y) => y.IsColonist && y.CanReach(x, PathEndMode.Touch, Danger.Deadly, false, TraverseMode.ByPawn)));
if (pawn != null) {
Messages.Message("CaravanPawnIsUnreachable".Translate(new object[]
{
pawn.LabelShort
}).CapitalizeFirst(), pawn, MessageSound.RejectInput);
return false;
}

for (int i = 0; i < transferables.Count; i++) {
if (transferables[i].ThingDef.category == ThingCategory.Item) {
int countToTransfer = transferables[i].countToTransfer;
int num = 0;
if (countToTransfer > 0) {
for (int j = 0; j < transferables[i].things.Count; j++) {
Thing t = transferables[i].things[j];
if (!t.Spawned || pawns.Any((Pawn x) => x.IsColonist && x.CanReach(t, PathEndMode.Touch, Danger.Deadly, false, TraverseMode.ByPawn))) {
num += t.stackCount;
if (num >= countToTransfer) {
break;
}
}
}
if (num < countToTransfer) {
if (countToTransfer == 1) {
Messages.Message("CaravanItemIsUnreachableSingle".Translate(new object[] {
transferables[i].ThingDef.label
}), MessageSound.RejectInput);
}
else {
Messages.Message("CaravanItemIsUnreachableMulti".Translate(new object[] {
countToTransfer,
transferables[i].ThingDef.label
}), MessageSound.RejectInput);
}
return false;
}
}
}
}
return true;
}


I'm very tempted to bypass Harmony entirely and just do an external detour, but since there's a remarkable number of A17 mods that modify TicksToMove I'd be remiss to do so.

Using Harmony to prefix means that you can have some conditional code which skips over your function and executes the original code, or executes your function and skips the original code, or executes your function and the original code.

I'll point you at a good example when I turn on my laptop.

Here you go: https://ludeon.com/forums/index.php?topic=33556.msg342254#msg342254

Quote from: JT on June 27, 2017, 02:06:00 AM
I'm very tempted to bypass Harmony entirely and just do an external detour, but since there's a remarkable number of A17 mods that modify TicksToMove I'd be remiss to do so.

Do not give up! I really like that you came here to ask to DoTheRightThing(tm). So I give you a run down on how I would do it including all the things I use and do:

Simple Harmony Transpiler Tutorial
https://gist.github.com/pardeike/c02e29f9e030e6a016422ca8a89eefc9

Enjoy!

int* ptr = &thanks;
return ptr;
(I'm paying a pointer forward in return for the pointers.  ...And maliciously introducing memory leaks.  Man, it's been a while since I've used a non-garbage-collected language. =P)

Also thanks for the link to Zhentar's ILSpy.  I've been using the main branch ILSpy for a half decade now and was mostly infuriated by its terrible support for lambdas and yield iterators in the Rimworld core code -- but I'm far too cheap to shell out cash for Reflector or JustDecompile (the latter of which also has creepy levels of "anonymous" metrics).

I'm currently well past bedtime, but did a reasonably close skim and so far nothing in the tutorial jumps out at me as being something I'd have a great deal of difficulty with.  I tried self-learning IASM more than a few moons ago and turned into a vegetable, but IL here seems very tame by comparison.  I'll pore over it tomorrow.  I notice you gave me the cheatsheet version at the bottom but I figure it'd be nice to learn it properly. =)

[edit] Having read it in great detail to the point that I'm pretty certain I understand it, I'm very tempted to leave dead code and just replace the instruction found after the first ret, at 0078, with a jump to the instruction after the second ret, to 00ac, rather than actually culling out the instructions.  In other words, still do the ret-inal scan (teehee) but switch the nop for a jmp without the subsequent removal of opcodes between the found points.  Although supremely unlikely, this would have the advantage that if anyone else transpiles that method, then any modifications they make to that segment of opcodes would be ignored rather than rendered completely incompatible.

free(ptr);
void* memoryLeak = malloc(27934767328468762386428763874676726438762378646723647862378642786387);

It's just great seeing the community help each other like that.

tbh IL is a pretty easy language to understand. If you get C#, you'll get IL.

There are no excuses for using detours.

_{Oh also if you need any further help there's always a great community on the discord that can help you :D}

c# do not allocate unmanaged memory on it's own.

int* ptr = &thanks;
return ptr;

will not result in allocation of unmanaged memory and completed on stack, stack frames cannot leak, as soon as function unwind, stack frame is terminated.

I've got a question / possible bug report.

There's an empty virtual method in MapParent:

public virtual void PostMapGenerate()
{
}

Its overrided by a grand total of one class, Site:

public override void PostMapGenerate()
{
base.PostMapGenerate();
Map map = base.Map;
this.core.Worker.PostMapGenerate(map);
for (int i = 0; i < this.parts.Count; i++)
{
this.parts[i].Worker.PostMapGenerate(map);
}
this.anyEnemiesInitially = GenHostility.AnyHostileActiveThreat(base.Map);
}

Given the following patches:

    [HarmonyPatch(typeof(MapParent), "PostMapGenerate")]
    public static class MapParent_PostMapGenerate_Postfix
    {

        [HarmonyPostfix]
        private static void PostMapGenerate(MapParent __instance)
        {
            Log.Message("post map generate");
            //custom code...
        }
    }

    [HarmonyPatch(typeof(Site), "PostMapGenerate")]
    public static class Site_PostMapGenerate_Postfix
    {

        [HarmonyPostfix]
        private static void PostMapGenerate(Site __instance)
        {
            Log.Message("post map generate SITE");
            //custom code...
        }
    }

Only the first patch gets called (as expected) when MapParent.PostMapGenerate gets called on non-Site classes inheriting from MapParent (specifically, I've been testing by attacking faction bases). The weird part is that only the second patch (or nothing, if I comment it out) gets called when PostMapGenerate is called on an instance of Site (tested by entering ItemStashes and PreciousLumps spawned through the debug commands), even though the override calls the base function (base.PostMapGenerate(); )...

Is it supposed to work like that? Is it a consequence of the compiler being too smart and optimizing away the empty function call? Is it a bug? Am I being stupid?

calling base virtual method is not mandatory, implementation methods may call only override.
in such case unspecific code injection put on base method will not fire, because base method will not fire.

Quote from: RawCode on August 01, 2017, 10:00:10 AM
calling base virtual method is not mandatory, implementation methods may call only override.
in such case unspecific code injection put on base method will not fire, because base method will not fire.

Since the base method looks like its code size is small, another possibility is that in lower grade code the method got inlined as a compiler optimization, therefore losing its instance. Though virtual methods should not be affected by compile time optimization, only runtime...

I'm having both success and difficulty with Harmony. I think the code will be self-explanatory (when my comment says it FAILS or SUCCEEDS, it is because the print statements in the functions do or do not print). Can someone pretty please tell me why the functions aren't firing? My main desire is to actually get the POSTFIX function on PawnKindLifeStage.ResolveReferences() to fire. But I have figured out a long the way I cannot predict what makes harmony work or not work.

Help?

using Harmony;
using System;
//using System.Linq;
//using System.Text;
//using System.Diagnostics;
//using System.Collections.Generic;
using RimWorld;
using Verse;
using UnityEngine;
//using Verse.AI;
//using Verse.Sound;
namespace CR
{
    [StaticConstructorOnStartup]
    static class GraphicsPatches
    {
        static GraphicsPatches()
        {
            HarmonyInstance harmony = HarmonyInstance.Create("rimworld.CR.graphix");
         
            // FAILS: defined as public class PawnKindDef { public override void ResolveReferences() ... }
         harmony.Patch(typeof(PawnKindDef).GetMethod("ResolveReferences"), new HarmonyMethod(typeof(GraphicsPatches).GetMethod("MakeTransparentPreFix1")), null);

         // FAILS; defined as public class PawnKindLifeStage { public void ResolveReferences() ... }
            harmony.Patch(typeof(PawnKindLifeStage).GetMethod("ResolveReferences"), new HarmonyMethod(typeof(GraphicsPatches).GetMethod("MakeTransparentPreFix2")), null);
         
         // SUCCEEDS: defined as public class PawnGraphicSet { public void ResolveAllGraphics)() ... }
            harmony.Patch(typeof(PawnGraphicSet).GetMethod("ResolveAllGraphics"), new HarmonyMethod(typeof(GraphicsPatches).GetMethod("MakeTransparentPreFix3")), null);

         // FAILS; defined as public class PawnKindLifeStage { public void ResolveReferences() ... }         
            harmony.Patch(typeof(PawnKindLifeStage).GetMethod("ResolveReferences"), null, new HarmonyMethod(typeof(GraphicsPatches).GetMethod("MakeTransparentPostFix")));                             
      }
      
      public static bool MakeTransparentPreFix1(PawnKindDef __instance)
      {   
         UnityEngine.Debug.Log( string.Concat(new object[] { "CR. Transparent Prefix (PawnKindDef) CALLED" } ));      
         return true;
      }
      
      public static bool MakeTransparentPreFix2(PawnKindLifeStage __instance)
      {   
         UnityEngine.Debug.Log( string.Concat(new object[] { "CR. Transparent Prefix (PawnKindLifeStage) CALLED" } ));      
         return true;
      }
      
      public static bool MakeTransparentPreFix3(PawnGraphicSet __instance)
      {   
         UnityEngine.Debug.Log( string.Concat(new object[] { "CR. Transparent Prefix (PawnGraphicSet) CALLED" } ));      
         return true;
      }
      
      public static void MakeTransparentPostFix(PawnKindLifeStage __instance)
      {   
         if (__instance.dessicatedBodyGraphicData != null)
         {
            UnityEngine.Debug.Log( string.Concat(new object[] { "CR. Making corpse graphic transparent SUCCEEDED" } ));      
            __instance.dessicatedBodyGraphicData.shaderType = ShaderType.Transparent;
         }
         else UnityEngine.Debug.Log( string.Concat(new object[] { "CR. Making corpse graphic transparent FAILED:", __instance.label } ));   
      }
    }
}

Let's start with the most obvious. I haven't tested or looked it up myself but are you sure your naming and access rights are correct on those manual method fetching calls? If any of those return null, you won't patch successfully. So please add logging to make sure your variations of ResolveReference actually return a valid MethodInfo.

Yep. I explored that.


    static class GraphicsPatches
    {
        static GraphicsPatches()
        {
            HarmonyInstance harmony = HarmonyInstance.Create("rimworld.CR.graphix");

// FAILS; defined as public class PawnKindLifeStage { public void ResolveReferences() ... }

            MethodInfo method = typeof(Verse.PawnKindLifeStage).GetMethod("ResolveReferences");
UnityEngine.Debug.Log( string.Concat(new object[] { "CR. Initing GraphicsPatches: ", (method!=null ? method.ToString(): "NULL") } ));
            harmony.Patch( method, null, new HarmonyMethod(typeof(GraphicsPatches).GetMethod("MakeTransparentPostFix1")));
                   
}
public static void MakeTransparentPostFix1(PawnKindLifeStage __instance)
{
UnityEngine.Debug.Log( string.Concat(new object[] { "CR. Lifestage postfix called: ", __instance.label } ));
}
}
}


This code prints "Void ResolveReferences()".

The postfix as shown never executes. I have a work-around (higher up the call tree), but this is really the best place for it, so it's irking me. ;-P

I feel like I need to learn how to run rimworld in a debugger, and put a breakpoint in the function, just to prove that rimworlds is calling it at all. Any suggestions in that direction?

Anyway PawnKindLifeStage is a super simple class, and fully public. Thus:


using System;

namespace Verse
{
public class PawnKindLifeStage
{
// ... member variables

public void ResolveReferences()
{
// ... small bit of graphics load/setup code
}
}
}



No idea what you are doing but this mini mod works just fine for me:


using Harmony;
using System.Reflection;
using Verse;

namespace Test
{
public class TestMain : Mod
{
public TestMain(ModContentPack content) : base(content)
{
var harmony = HarmonyInstance.Create("net.pardeike.test");
harmony.PatchAll(Assembly.GetExecutingAssembly());
}
}

[HarmonyPatch(typeof(PawnKindLifeStage))]
[HarmonyPatch("ResolveReferences")]
static class PawnKindLifeStage_ResolveReferences_Patch
{
static void Prefix()
{
Log.Error("CALLING ResolveReferences");
}
}
}


Quote from: CannibarRechter on September 03, 2017, 06:27:03 AM
Yep. I explored that.

Code Select Expand


    static class GraphicsPatches
    {
        static GraphicsPatches()
        {
            HarmonyInstance harmony = HarmonyInstance.Create("rimworld.CR.graphix");

// FAILS; defined as public class PawnKindLifeStage { public void ResolveReferences() ... }

            MethodInfo method = typeof(Verse.PawnKindLifeStage).GetMethod("ResolveReferences");
UnityEngine.Debug.Log( string.Concat(new object[] { "CR. Initing GraphicsPatches: ", (method!=null ? method.ToString(): "NULL") } ));
            harmony.Patch( method, null, new HarmonyMethod(typeof(GraphicsPatches).GetMethod("MakeTransparentPostFix1")));
                   
}
public static void MakeTransparentPostFix1(PawnKindLifeStage __instance)
{
UnityEngine.Debug.Log( string.Concat(new object[] { "CR. Lifestage postfix called: ", __instance.label } ));
}
}
}


This code prints "Void ResolveReferences()".

The postfix as shown never executes. I have a work-around (higher up the call tree), but this is really the best place for it, so it's irking me. ;-P

I feel like I need to learn how to run rimworld in a debugger, and put a breakpoint in the function, just to prove that rimworlds is calling it at all. Any suggestions in that direction?

Anyway PawnKindLifeStage is a super simple class, and fully public. Thus:

Code Select Expand


using System;

namespace Verse
{
public class PawnKindLifeStage
{
// ... member variables

public void ResolveReferences()
{
// ... small bit of graphics load/setup code
}
}
}


Thank you. Different question. I am trying to Prefix patch a generic. Nothing complex right now, I just want the Prefix to execute, print a line, and then let the original generic do its work.

I have  this:


[HarmonyPatch(typeof(Verse.ContentFinder<>), new Type[] { typeof(Texture2D) } ) ]
[HarmonyPatch("Get")]
static class ContentFinder_Get_Patch
{
static bool Prefix( Texture2D __instance, Texture2D __result, string itemPath, bool reportFailure = true )
{
if (itemPath == "UI/Commands/Forbidden")
Log.Error( "CR. Intercepted content finder.");

return true;
}
}


So, this method is never invoked, but it borks the game. I get a series of errors like this before the game bails out and invalidates the mod (I'm thinking an internal exception happens or some such, but can't really tell):


(Filename: C:/buildslave/unity/build/artifacts/generated/common/runtime/UnityEngineDebugBindings.gen.cpp Line: 42)

Could not load UnityEngine.Texture2D at Items/Drugs/BottleBeer in any active mod or in base resources.

(Filename: C:/buildslave/unity/build/artifacts/generated/common/runtime/UnityEngineDebugBindings.gen.cpp Line: 42)

MatFrom with null sourceTex.

(Filename: C:/buildslave/unity/build/artifacts/generated/common/runtime/UnityEngineDebugBindings.gen.cpp Line: 42)

Could not load UnityEngine.Texture2D at Items/Drugs/BoxAmbrosia in any active mod or in base resources.

(Filename: C:/buildslave/unity/build/artifacts/generated/common/runtime/UnityEngineDebugBindings.gen.cpp Line: 42)

MatFrom with null sourceTex.


ContentFinder is pretty basic as a class. It looks like this:



namespace Verse
{
public static class ContentFinder<T> where T : class
{
public static T Get(string itemPath, bool reportFailure = true)
{
...


Am I missing something about how to patch generics?

Generic methods are split by the compiler into individual methods, one for each type or interface that can be used. So you cannot patch all at once since Harmony only can redirect code flow of a concrete implementation of a method. One can loop through all types you want to patch and patch them with code instead of annotations. You don't have to patch all types if you are only interested in a few cases.


MethodInfo method = typeof(Sample).GetMethod("GenericMethod");
MethodInfo generic = method.MakeGenericMethod(myType);


It's a generic class, not a generic method. I tried the manual approach here:


namespace CR
{
    [StaticConstructorOnStartup]
    public class ModdableUI : Mod
    {
        public ModdableUI( ModContentPack content ) : base( content )
        {
try
{
HarmonyInstance harmony = HarmonyInstance.Create("rimworld.CR.moddableUI");

Type genericClass = typeof( ContentFinder<Texture2D> );

UnityEngine.Debug.Log( string.Concat(new object[] { "CR: ModdableUI: ", (genericClass!=null ? genericClass.ToString(): "NULL") } ));

MethodInfo method = genericClass.GetMethod("Get");

UnityEngine.Debug.Log( string.Concat(new object[] { "CR: ModdableUI: ", (method!=null ? method.ToString(): "NULL") } ));

//harmony.Patch( method, new HarmonyMethod(typeof(GraphicsPatches).GetMethod("MakeTransparentPreFix")), null);
harmony.Patch( method, null, new HarmonyMethod(typeof(ModdableUI).GetMethod("GetPostfx")));
}
catch( Exception e )
{
UnityEngine.Debug.Log( string.Concat(new object[] { "CR: Exception: ", e } ));
}
}

static public void GetPostfix( Texture2D __instance, string itemPath, bool reportFailure )
{
// if (itemPath == "UI/Commands/Forbidden")
UnityEngine.Debug.Log( string.Concat(new object[] { "CR: Intercepted GET: ", itemPath } ));

return;
}
    }
}

What the debug statements print is here:

CR: ModdableUI: Verse.ContentFinder`1[UnityEngine.Texture2D]

CR: ModdableUI: UnityEngine.Texture2D Get(System.String, Boolean)

Those appear to be the correctly reflected generic class and method. Rimworld loses the ability to process any textures at all when this patch is applied. Which is weird, as it's a postfix patch which changes nothing, so it should have no impact.

BTW, while I went ahead and used the manual method invocation here, the Harmony website discusses generics, and gives recommendations that are similar to the stuff I tried the first time (class decorations).

Quote from: CannibarRechter on September 04, 2017, 10:39:50 AM
It's a generic class, not a generic method. I tried the manual approach here:

Code Select Expand


static public void GetPostfix( Texture2D __instance, string itemPath, bool reportFailure )
{
}


Since ContentFinder.Get is a static method, I am not sure why your postfix wants Harmony to inject "Texture2D __instance". It's static and even if it were not, Texture2D would need to be ContentFinder.

Welp, that was a silly error. I was hopeful, but the same problem continues. Here's the entire classfile now (as you can see I've tried it with the decorator method and manual method both, adjusted properly for a static method without an __instance type). The game issues several pages of can't load texture errors and then bongs out. Anyway, thank you so much for your help.

Is there a good way to debug Harmony? I suppose I could download the code and establish some traps...


using Harmony;
using System;
//using System.Linq;
//using System.Text;
//using System.Diagnostics;
//using System.Collections.Generic;
using System.Reflection;
using RimWorld;
using Verse;
using UnityEngine;
//using Verse.AI;
//using Verse.Sound;
namespace CR
{
    public class ModdableUI : Mod
    {
        public ModdableUI( ModContentPack content ) : base( content )
        {
try
{
HarmonyInstance harmony = HarmonyInstance.Create("rimworld.CR.moddableUI");
//harmony.PatchAll( Assembly.GetExecutingAssembly() );

Type genericClass = typeof( ContentFinder<Texture2D> );

UnityEngine.Debug.Log( string.Concat(new object[] { "CR: ModdableUI: ", (genericClass!=null ? genericClass.ToString(): "NULL") } ));

MethodInfo method = genericClass.GetMethod("Get");

UnityEngine.Debug.Log( string.Concat(new object[] { "CR: ModdableUI: ", (method!=null ? method.ToString(): "NULL") } ));

//harmony.Patch( method, new HarmonyMethod(typeof(GraphicsPatches).GetMethod("MakeTransparentPreFix")), null);
harmony.Patch( method, null, new HarmonyMethod(typeof(ModdableUI).GetMethod("GetPostfx")));

UnityEngine.Debug.Log( string.Concat(new object[] { "CR: Inited Moddable UI" } ));
}
catch( Exception e )
{
UnityEngine.Debug.Log( string.Concat(new object[] { "CR: Exception: ", e, "; this mod failed. We suggest you disable the mod and contact the author." } ));
}
}

static public void GetPostfix( string itemPath, bool reportFailure )
{
// if (itemPath == "UI/Commands/Forbidden")
UnityEngine.Debug.Log( string.Concat(new object[] { "CR: Intercepted GET: ", itemPath } ));

return;
}

//[HarmonyPatch(typeof(Verse.ContentFinder<Texture2D>))]
//[HarmonyPatch("Get")]
//static class ContentFinder_Get_Patch
//{
//static bool Prefix( Texture2D __instance, Texture2D __result, string itemPath, bool reportFailure )
// static void Postfix( string itemPath, bool reportFailure )
// {
// if (itemPath == "UI/Commands/Forbidden")
// UnityEngine.Debug.Log( string.Concat(new object[] { "CR. Intercepted GET: ", itemPath } ));

// return;
// }
//}
    }
}



So, if I declare ContentFinder<Texture2D>, and then GetMethod ("Get"), and then apply a null prefix (one that simply returns true, but does nothing) or a postfix to it, I get the exceptions. It took me a while to really understand what's happening: the exception is being issued by ContentFinder<Texture2D>.Get() itself. This is notable, because it suggests that Harmony is changing the behavior of Get(). If I am right, that's a Harmony bug. I believe I know where the problem occurs, if not why. Look here:


public static class ContentFinder<T> where T : class
{
public static T Get(string itemPath, bool reportFailure = true)
{
if (!UnityData.IsInMainThread)
{
Log.Error("Tried to get a resource \"" + itemPath + "\" from a different thread. All resources must be loaded in the main thread.");
return (T)((object)null);
}
T t = (T)((object)null);
foreach (ModContentPack current in LoadedModManager.RunningMods)
{
t = current.GetContentHolder<T>().Get(itemPath);
if (t != null)
{
T result = t;
return result;
}
}
// BEGIN ERROR BLOCK
if (typeof(T) == typeof(Texture2D))
{
t = (T)((object)Resources.Load<Texture2D>(GenFilePaths.ContentPath<Texture2D>() + itemPath));
}
if (typeof(T) == typeof(AudioClip))
{
t = (T)((object)Resources.Load<AudioClip>(GenFilePaths.ContentPath<AudioClip>() + itemPath));
}
// END ERROR BLOCK
if (t != null)
{
return t;
}
if (reportFailure)
{
// THIS IS THE EXCEPTION THAT IS RAISED:
Log.Error(string.Concat(new object[]
{
"Could not load ",
typeof(T),
" at ",
itemPath,
" in any active mod or in base resources."
}));
}
return (T)((object)null);
}


So my theory is that when Harmony patches a method on a generic class, something goes wrong with the original method. If you look at the error block that I annotated, I believe (although I am not 100% sure) what is happening is that those conditional checks are not returning true for whatever reason, and therefore being skipped. We are therefore reaching the reportFailure block. The other possibility is that Resources.Load() is returning null. I doubt this, though. That's a Unity method.

I've noticed another anomaly that might be helpful. I went ahead and attempted to implement an override style prefix (one that returns false). This would prevent the original possibly borked method from executing at all, and use my method instead. I've noticed that even though I specifically bound to ContentFinder<Texture2D>.Get(), the game/harmony is invoking the method
with AudioClip requests. I don't know if this is relevant information for you or not. I would think that shouldn't be able to happen. The game only looks for sound on bound generics like this: ContentFinder<AudioClip>.Get().

I think I have enough information to implement workarounds on my end at this point, but I thought I would raise the possibility of a bug in Harmony for you to look at.

This might be a limitation in how Harmony redirects the original method by inserting an assembler jump to the assembler code of an compiled dynamic method. My guess is that generic methods in generic classes have some extra logic inside the assembler code that simply cannot be replicated with a dynamic method. That, or the way to configure the dynamic method is incomplete (or simply does not support this scenario). To be honest, I have no clue how to address this.

Quote from: CannibarRechter on September 07, 2017, 07:41:28 AM
So, if I declare ContentFinder<Texture2D>, and then GetMethod ("Get"), and then apply a null prefix (one that simply returns true, but does nothing) or a postfix to it, I get the exceptions. It took me a while to really understand what's happening: the exception is being issued by ContentFinder<Texture2D>.Get() itself. This is notable, because it suggests that Harmony is changing the behavior of Get(). If I am right, that's a Harmony bug. I believe I know where the problem occurs, if not why. Look here:

Code Select Expand


public static class ContentFinder<T> where T : class
{
public static T Get(string itemPath, bool reportFailure = true)
{
if (!UnityData.IsInMainThread)
{
Log.Error("Tried to get a resource \"" + itemPath + "\" from a different thread. All resources must be loaded in the main thread.");
return (T)((object)null);
}
T t = (T)((object)null);
foreach (ModContentPack current in LoadedModManager.RunningMods)
{
t = current.GetContentHolder<T>().Get(itemPath);
if (t != null)
{
T result = t;
return result;
}
}
// BEGIN ERROR BLOCK
if (typeof(T) == typeof(Texture2D))
{
t = (T)((object)Resources.Load<Texture2D>(GenFilePaths.ContentPath<Texture2D>() + itemPath));
}
if (typeof(T) == typeof(AudioClip))
{
t = (T)((object)Resources.Load<AudioClip>(GenFilePaths.ContentPath<AudioClip>() + itemPath));
}
// END ERROR BLOCK
if (t != null)
{
return t;
}
if (reportFailure)
{
// THIS IS THE EXCEPTION THAT IS RAISED:
Log.Error(string.Concat(new object[]
{
"Could not load ",
typeof(T),
" at ",
itemPath,
" in any active mod or in base resources."
}));
}
return (T)((object)null);
}


So my theory is that when Harmony patches a method on a generic class, something goes wrong with the original method. If you look at the error block that I annotated, I believe (although I am not 100% sure) what is happening is that those conditional checks are not returning true for whatever reason, and therefore being skipped. We are therefore reaching the reportFailure block. The other possibility is that Resources.Load() is returning null. I doubt this, though. That's a Unity method.

I've noticed another anomaly that might be helpful. I went ahead and attempted to implement an override style prefix (one that returns false). This would prevent the original possibly borked method from executing at all, and use my method instead. I've noticed that even though I specifically bound to ContentFinder<Texture2D>.Get(), the game/harmony is invoking the method
with AudioClip requests. I don't know if this is relevant information for you or not. I would think that shouldn't be able to happen. The game only looks for sound on bound generics like this: ContentFinder<AudioClip>.Get().

I think I have enough information to implement workarounds on my end at this point, but I thought I would raise the possibility of a bug in Harmony for you to look at.

Okay. I'll let you know if I can get my work around working. If it works ti will assume that when invoked that the prefix method must assume that it could be EITHER version of the generic, and cannot return TRUE. I think this can be made to work; it will be dependent on me being able to determine if what is passed into the function is Texture2D or AudioClip. If I can do that, I can get what I need done, I think. It does mean that one couldn't ever safely use a Postfix method on this type of generic. That makes me a sad panda.

Unfortunately my work-around doesn't work. While the code I had  in mind executes perfectly well (I did it with 'object'), the value of __result is ignored by the game. So while I set __result to a value using code functionally similar to the original ContentFinder, the game after making the load complains about a MatFrom null. That's what the game does when it's going to set a texture to that red box, if you have seen it before, but now it's pretty much doing that to all graphics in Rimworld.

I'll see about creating an isolated non-Rimworld program that can replicate this bug for you...

@CannibarRechter:

I have analyzed your problem and it seems that there are two issues here:

1) [HarmonyPatch(typeof(ContentFinder<>), new Type[] { typeof(Texture2D) })] seems to be not working right now so you have to use a manual approach instead:


static MethodBase TargetMethod()
{
var type = typeof(ContentFinder<>);
type = type.MakeGenericType(new Type[] { typeof(Texture2D) });
var method = type.GetMethod("Get", AccessTools.all);
return method;
}


This is annoying and I will look into it.

2) The main problem is somewhere else. Right now, Harmony has no support for try-catch structures in the IL code of a method. And unfortunately, ContentFinder<T>.Get() has a try-catch structure that has code logic in its finally handler. As a result, the wrong part of the code is executed because the try-catch is not executed (in IL code, try-catch is not purely done by IL codes but also meta data that Harmony would need to maintain and insert at the correct places again).

I already have a pull request for handling try-catch metadata but my feeling is that it solves the problem only partially since I am pretty sure that it does not allow restructuring IL code with transpilers. So while pulling in this fix would most likely solve this issue, it would not work 100% in all cases.

The only workaround for you left at this point (until Harmony supports try/catch) would be to rewrite the method in a prefix that returns false.

/Andreas

I'm a little confused here. Using the most recent version of ILSpy, below is what ContentFinder<T>.Get() looks like to me. Is there some reason I am seeing a different ContentFinder than you are?


public static T Get(string itemPath, bool reportFailure = true)
{
if (!UnityData.IsInMainThread)
{
Log.Error("Tried to get a resource \"" + itemPath + "\" from a different thread. All resources must be loaded in the main thread.");
return (T)((object)null);
}
T t = (T)((object)null);
foreach (ModContentPack current in LoadedModManager.RunningMods)
{
t = current.GetContentHolder<T>().Get(itemPath);
if (t != null)
{
T result = t;
return result;
}
}
if (typeof(T) == typeof(Texture2D))
{
t = (T)((object)Resources.Load<Texture2D>(GenFilePaths.ContentPath<Texture2D>() + itemPath));
}
if (typeof(T) == typeof(AudioClip))
{
t = (T)((object)Resources.Load<AudioClip>(GenFilePaths.ContentPath<AudioClip>() + itemPath));
}
if (t != null)
{
return t;
}
if (reportFailure)
{
Log.Error(string.Concat(new object[]
{
"Could not load ",
typeof(T),
" at ",
itemPath,
" in any active mod or in base resources."
}));
}
return (T)((object)null);
}

Don't use the disassembled source code. Look at the IL code and you see that it contains try/catch meta data.

You're wrooooong ;D The decompiled code is correct - and that is because
foreach(var foo in bar) {... }
is just syntactic sugar for

IEnumerable<X> e;
try {
  e = bar.GetEnumerator();
  while (e.MoveNext()) {
    var foo = e.Current;
    ...
  }
} finally {
  e.Dispose();
}


Just passing through, wanted to say I'm happy you continued the good work, pardeike!

Quote from: Master Bucketsmith on September 14, 2017, 10:20:01 AM
Just passing through, wanted to say I'm happy you continued the good work, pardeike!

ai remember our conversation like they were yesterday. Hope all is well my friend!

Hi, pardeike,

Using Visual Studio 2017 Enterprise, when attempting to compile Transpilers.cs, it issues he following error:

Severity   Code   Description   Project   File   Line   Suppression State
Error   CS0252   Possible unintended reference comparison; to get a value comparison, cast the left hand side to type 'MethodBase'   Harmony   H:\joe.kraska\Artifacts\Mods\Harmony-master\Harmony\Transpilers.cs   13   Active

... while I can fix it in the base code, I thought I'd let you know about the issue.

Hi, pardeike, as promised I have created a standalone test program for you, attached below. Regarding your pull request for generics support, I would suggest that you make harmony modal: one mode "strict," which attempts to implement everything well, and another one that implements transpilers, but doesn't support generics. Transpilers appear to be more an edge case to me, but perhaps your other users think otherwise. Regardless, why not support both? Just support it an argument to the constructor of the HarmonyInstance.

Looking on your github site, it's unclear to me which pull request adds the try/catch metadata. I only see the 3. I was wondering if I could try it (I do not need transpilers, unless somehow their involved in basic patching, and I don't think they are).

Anyway, here is the code:


using System;
using System.Reflection;
using Harmony;

namespace ConsoleApp1
{
    class Program
    {
        static void Main(string[] args)
        {
Console.WriteLine( "------------- BASE BLOCK ------------" );
Alpha.Do();
Beta.Do();
Test<Alpha>.Do();
Test<Beta>.Do();

try
{
HarmonyInstance harmony = HarmonyInstance.Create( "testme.testme" );

MethodInfo origMethod = null;
MethodInfo prefixMethod = null;
MethodInfo postfixMethod = null;
HarmonyMethod prefix = null;
HarmonyMethod postfix = null;

//--> This block has problems;

Console.WriteLine( "------------- BLOCK WITH GENERIC ------------" );
origMethod = AccessTools.Method( typeof(Test<Beta>), "Do" );
Console.WriteLine( $"origMethod: {origMethod}" );
prefixMethod = AccessTools.Method( typeof(Patch), "Prefix" );
Console.WriteLine( $"targetMethod: {prefixMethod}" );
postfixMethod = AccessTools.Method( typeof(Patch), "Postfix" );
Console.WriteLine( $"targetMethod: {postfixMethod}" );
prefix = new HarmonyMethod( prefixMethod );
postfix = new HarmonyMethod( postfixMethod );
Console.WriteLine( $"prefix: {prefix}" );
Console.WriteLine( $"postfix: {postfix}" );
harmony.Patch( origMethod, prefix, null );
harmony.Patch( origMethod, null, postfix );


Console.WriteLine( "------------- TEST<ALPHA> ------------" );
Test<Alpha>.Do();
Console.WriteLine( "------------- TEST<BETA> ------------" );
Test<Beta>.Do();

//--> This block works:

Console.WriteLine( "------------- BLOCK NO GENERIC ------------" );
origMethod = AccessTools.Method( typeof(Alpha), "Do" );
Console.WriteLine( $"origMethod: {origMethod}" );
prefixMethod = AccessTools.Method( typeof(Patch), "Prefix" );
Console.WriteLine( $"targetMethod: {prefixMethod}" );
postfixMethod = AccessTools.Method( typeof(Patch), "Postfix" );
Console.WriteLine( $"targetMethod: {postfixMethod}" );
prefix = new HarmonyMethod( prefixMethod );
postfix = new HarmonyMethod( postfixMethod );
Console.WriteLine( $"prefix: {prefix}" );
Console.WriteLine( $"postfix: {postfix}" );
harmony.Patch( origMethod, prefix, null );
harmony.Patch( origMethod, null, postfix );

Alpha.Do();

}
catch (Exception e)
{
Console.WriteLine($"Caught exception: {e}");
}

                   
}
    }

public class Patch
{
public static bool Prefix()
{
Console.WriteLine(">>>>>>>>>>>>>> PREFIX ");
return true;
}
public static void Postfix()
{
Console.WriteLine("<<<<<<<<<<<<<< POSTFIX");
}
}


public class Test<T> // where T : class
{
public static void Do()
{
Console.WriteLine( "A." );
if (typeof(T) == typeof(Alpha))
{
Console.WriteLine( "B." );
Alpha.Do();
}
Console.WriteLine( "C." );
if (typeof(T) == typeof(Beta))
{
Console.WriteLine( "D." );
Beta.Do();
}
Console.WriteLine( "E." );
}
}

public class Alpha
{
public static void Do()
{
Console.WriteLine("Alpha");
}
}

public  class Beta
{
public static void Do()
{
Console.WriteLine("Beta");
}
}
}


I have downloaded your example and am testing it with my latest version that is based on the master but with the fixes I am working on. I can confirm that your example does not work. It simply does not have any effect when I run it in the latest .NET 4.6. My guess is that generics work different in mono and .NET core.

Regarding transpilers: there is nothing special about them. Harmony builds its new method using IL code to call prefix/postfixes and transpilers only affect the il codes that come from the original method like a filter. So there is no need to exclude them since they are not the cause of the problem.

All I can say without looking deeper into the issue is that generics work in the mono version of Harmony.

BTW, is Harmony supposed to be working on constructors right now? I tied a pattern of className plus function name = className, and it appears to hard crash rimworld entirely. The most straightforwad patch I'm working on is a Postfix on a constructor. I can probably find somewhere else to hook this, but...

Yes, Harmony can patch constructors. You need to use manual patching or TargetMethod() to return the correct MethodBase value but it should work.

Keep in mind though that constructors are not returning any values. They are called on empty new instances of a type and are suppose to fill/initialize the instance. Look at a typical IL code example. I have personally used transpilers on a constructor with success.

Okay. I see what I was doing wrong. #1, I was using MethodInfo everywhere. That's probably why you use MethodBase in your examples, as that is the common parent of both MethodInfo and ConstructorInfo. So I had to fix that up to get around a casting error. After that, you have to use a different method to get a hold of the constructor.

I was looking for a null argument constructor, so it needed a request like this:


origMethod = (MethodBase) typeof(Alpha).GetConstructor( new Type[]{} );


That's postfixable just fine.

Regarding the your comments about not returning any values, I think what you are saying is that if I wanted to run a prefix, I would initialize all the __instance data items individually, and probablly best return false. I can't imagine the fuckery if constructors are chained. LOL.

Anyway, my needs are pretty simple. I need to make a core class change a single value in the constructor, so the postfix is cleaner.

Thanks for your help.

Quote from: CannibarRechter on September 20, 2017, 01:34:14 PM

Code Select Expand


origMethod = (MethodBase) typeof(Alpha).GetConstructor( new Type[]{} );


*cough* Type.EmptyTypes *cough*

I just ran across a strange situation. What I really want to do is merely add an override method to an existing (defined) class. Since that doesn't appear possible, my next question is: is it expected to work to replace a virtual method on a base class?

The anser is: if a disassembler shows code for a method and that method does not get inlined, then Harmony can patch that method. If a class does not override a base classes method then there is nothing to patch but that base class method.

Quote from: CannibarRechter on September 27, 2017, 07:04:17 PM
I just ran across a strange situation. What I really want to do is merely add an override method to an existing (defined) class. Since that doesn't appear possible, my next question is: is it expected to work to replace a virtual method on a base class?

I have run into what I consider an odd problem. I added this piece of code:
    [HarmonyPatch(typeof(Verse.Corpse))]
    [HarmonyPatch("SpecialDisplayStats")]
    class CorpseDisplay
    {
    }
This results in "System.ArgumentException: No target method specified for class....". The odd thing is that if I replace SpecialDisplayStats with something else, it works. The class is indeed completely empty, meaning it's not the contents of my class, which cause this issue. The question is then: what does and what do I do to solve this issue?

The vanilla code in question is this:
public override IEnumerable<StatDrawEntry> SpecialDisplayStats
{
get
{
foreach (StatDrawEntry s in base.SpecialDisplayStats)
{
yield return s;
}
if (this.GetRotStage() == RotStage.Fresh)
{
yield return new StatDrawEntry(StatCategoryDefOf.Basics, "Nutrition".Translate(), FoodUtility.GetBodyPartNutrition(this.InnerPawn, this.InnerPawn.RaceProps.body.corePart).ToString("0.##"), 0);
StatDef meatAmount = StatDefOf.MeatAmount;
yield return new StatDrawEntry(meatAmount.category, meatAmount, this.InnerPawn.GetStatValue(meatAmount, true), StatRequest.For(this.InnerPawn), ToStringNumberSense.Undefined);
StatDef leatherAmount = StatDefOf.LeatherAmount;
yield return new StatDrawEntry(leatherAmount.category, leatherAmount, this.InnerPawn.GetStatValue(leatherAmount, true), StatRequest.For(this.InnerPawn), ToStringNumberSense.Undefined);
}
}
}
It's a little big to expect it to be inlined.

Another semi related question. When the vanilla function is based on IEnumerable and yield return, how do I append to the output in Postfix? I thought I could get __result to be a reference to a list, but I can't get that to work. I'm thinking it would be best to answer this question with an example on the wiki, which say reads __result, removes the first element and then appends it to the end. It might not be that useful, but it's simple and demonstrates both adding and removal/editing of the output from vanilla.

Quote from: Nightinggale on October 21, 2017, 04:48:39 PM
I have run into what I consider an odd problem. I added this piece of code:

Code Select Expand

    [HarmonyPatch(typeof(Verse.Corpse))]
    [HarmonyPatch("SpecialDisplayStats")]
    class CorpseDisplay
    {
    }
This results in "System.ArgumentException: No target method specified for class....". The odd thing is that if I replace SpecialDisplayStats with something else, it works. The class is indeed completely empty, meaning it's not the contents of my class, which cause this issue. The question is then: what does and what do I do to solve this issue?

That method isn't a method but a property. So the underlying method is actually called get_SpecialDisplayStats(). You solve this by either renaming your string (not recommended) or by using this annotation instead:
    [HarmonyPatch(typeof(Verse.Corpse))]
    [HarmonyPatch("SpecialDisplayStats", PropertyMethod.Getter)]
    class CorpseDisplay
    {
    }

Quote from: Nightinggale on October 21, 2017, 04:48:39 PM
Another semi related question. When the vanilla function is based on IEnumerable and yield return, how do I append to the output in Postfix? I thought I could get __result to be a reference to a list, but I can't get that to work. I'm thinking it would be best to answer this question with an example on the wiki, which say reads __result, removes the first element and then appends it to the end. It might not be that useful, but it's simple and demonstrates both adding and removal/editing of the output from vanilla.

If you use ILSpy (or dnSpy) to look up the actual method you will be surprised to see that it is rather short:

.method public hidebysig specialname virtual
instance class [mscorlib]System.Collections.Generic.IEnumerable`1<class RimWorld.StatDrawEntry> get_SpecialDisplayStats () cil managed
{
// Method begins at RVA 0x16b2a8
// Code size 23 (0x17)
.maxstack 3
.locals init (
[0] class Verse.Corpse/'<>c__Iterator22A'
)

IL_0000: newobj instance void Verse.Corpse/'<>c__Iterator22A'::.ctor()
IL_0005: stloc.0
IL_0006: ldloc.0
IL_0007: ldarg.0
IL_0008: stfld class Verse.Corpse Verse.Corpse/'<>c__Iterator22A'::'<>f__this'
IL_000d: ldloc.0
IL_000e: dup
IL_000f: ldc.i4.s -2
IL_0011: stfld int32 Verse.Corpse/'<>c__Iterator22A'::$PC
IL_0016: ret
} // end of method Corpse::get_SpecialDisplayStats

This is because that method internally only returns an iterator object (the class name of that in this case is called '<>c__Iterator22A') that does the job. It's that one you need to patch. The class '<>c__Iterator22A' is an inner class of the class Verse.Corpse and has, as every iterator, two main methods: MoveNext() and Reset(). It's MoveNext() you need to patch.

Alternatively, you could patch the getter and just return your own instance of your own iterator implementation with like a prefix returning false. In that iterator implementation you use reflections to iterate through the original corpses iterator and yield return all the original values. Then you just yield more after the loop and you got yourself new elements added.

Sorry that this is all very complicated but that's how C# works. I did not add this to the wiki since the wikis purpose is to explain the things Harmony adds to the mix. It requires that you learn the inner workings of C# too.

Quote from: pardeike on October 21, 2017, 05:05:00 PMThis is because that method internally only returns an iterator object (the class name of that in this case is called '<>c__Iterator22A') that does the job. It's that one you need to patch. The class '<>c__Iterator22A' is an inner class of the class Verse.Corpse and has, as every iterator, two main methods: MoveNext() and Reset(). It's MoveNext() you need to patch.

So I should add a Postfix to MoveNext(), but wouldn't that trigger each time the caller requests the next item, effectively make my method trigger and inject every second line? Alternatively it should check if it is pointing to nil and then make it point to a new list, which adds the item I want to add, but then the caller will call MoveNext() again and then it will end up pointing to nil again, which could make my code trigger again  ???

If IEnumerable return type is just an iterator to a list, I wonder if it would be doable to make a reference to __result, then use __result.MoveNext() to read all the contents to a new list, modify that list as needed and then make an iterator to the new list and store it in __result. While it looks good on paper, I'm concerned about leaking memory and what will happen to the new list once it goes out of scope before it's read by the caller. Do anybody know this or should I just try my luck and see what happens?

Performance is not a concern due to the lists in question being rather short and the low call frequency.

Quote from: pardeike on October 21, 2017, 05:05:00 PMAlternatively, you could patch the getter and just return your own instance of your own iterator implementation with like a prefix returning false. In that iterator implementation you use reflections to iterate through the original corpses iterator and yield return all the original values. Then you just yield more after the loop and you got yourself new elements added.

Having a Prefix, which stops execution sounds like it's on the path to the problems Harmony should fix, which is compatibility problems with other mods. I might as well just replace the function without using Harmony if it blocks other mods from using Postfix anyway.

Quote from: pardeike on October 21, 2017, 05:05:00 PMSorry that this is all very complicated but that's how C# works. I did not add this to the wiki since the wikis purpose is to explain the things Harmony adds to the mix. It requires that you learn the inner workings of C# too.

I both agree and disagree with what you are saying here. Harmony is brilliant and it solves a major compatibility issue. However it only works if the mods actually use it. If the difficulty is so high that the majority of modders can't figure out how to use it even if they want to, the result will be a lot of mods not using Harmony, which defeats the purpose. My experience from years of modding is that most modders are somewhat self taught and they fight to do the best they can. You can't just tell them to use better coding theory, because they would be like "what does that mean?", then ignore you and either change nothing and make a mess or give up on modding. We don't want either.

I admit I'm not a C# guru. In fact I decided to write a mod to get to know C#. However I am an engineer (master of science no less) and I do have a strong C++ background as well as knowing a bunch of other languages and the approach to write some production code to learn a new language has served we well before and I would say it has here as well. However the problems I have run into aren't C# as such. It's an issue that it enforces low level programming as well as apparently reading assembler. While I'm not lost in doing that (particularly when I know that's what I have to do), I would say that I wouldn't except modders in general to be able to do that.

In order to get Harmony to be used by all mods (xml only excluded), it will require some very simple examples to show how to use a subset in order to get people started. I might write that myself, that is if I figure out how to actually make it work myself.

Quote from: wiki[HarmonyPatch(String, PropertyMethod)] Defines the property to be patched by name

I admit it eluded me that the second argument takes PropertyMethod.Getter and PropertyMethod.Setter. I seems obvious in retrospect, but PropertyMethod is actually not mentioned other than the line I quoted.

It is easy to say that Harmony should make all patching available to even the uninformed/uneducated modders out there. But when it comes to actually solving this problem you seem to not be able to offer a solution. That's a bit like saying that everybody should be able to drive a racing car or do figure skating or any other sophisticated task, without the requirements of training, dedication or risk. That won't happen.

If you come up with any sketch of a good solution to this problem that does not involve endless hours of work, or results in endless hours of text/video/code or other educational material that basically shortcuts the skills and knowledge required to solve these kind of patching you are talking about, please let me know and I am more than happy to implement it.

In my opinion Harmony does live on the sweet spot between easy enough to adapt for 60% of patching and powerful enough to allow more advanced coders to do their job too. It is not an educational tool.

Let me know what you think.

I think results speak for themselves here. Not only do all self-respecting RimWorld modders use Harmony for things we would have detoured in the past, but the library itself has been adopted by modders for other games and even some companies.

I have done some thinking and the main problem is not with Harmony itself. If __result is say an int, it's just an int and it can easily be modded. Also it's fairly easy to hook up, particularly after you realize how to use PropertyMethod, meaning most calls are actually fairly doable.

The problem is the clash between Harmony and IEnumerable<> because it provides an iterator rather than the data itself. It makes perfect sense to do so if you think about function calls and return values in registers and patching a compiled DLL will indeed have to consider low level stuff like that.

However this knowledge does not help me with my current problem. Iterators have read only access to the data, meaning if I want to modify the data itself, I have to write code like the following:
    [HarmonyPatch(typeof(Verse.Corpse))]
    [HarmonyPatch("SpecialDisplayStats", PropertyMethod.Getter)]
    class CorpseDisplay
    {
        static void Postfix(Verse.Pawn __instance, ref IEnumerator<StatDrawEntry> __result)
        {
            IEnumerable<StatDrawEntry> NewList = new List<StatDrawEntry>();
           
            while (__result.MoveNext())
            {
                // NewList.Add(__result.Current); <-- this line crashes the game
                StatDrawEntry temp = __result.Current; // this doesn't
                NewList.Add(temp);
            }
            // todo append modded lines to NewLine

            __result = NewList.GetEnumerator(); // causes a crash
        }
    }
However this crashes the game. The first crash is fixable using a buffer, but the second one isn't. I tried making NewList static to avoid the risk of an issue of freeing the memory once it goes out of scope, but that changed nothing.

Do anybody have an idea on how to get this working?

Quote from: Dingo on October 22, 2017, 03:03:52 AM
I think results speak for themselves here. Not only do all self-respecting RimWorld modders use Harmony for things we would have detoured in the past, but the library itself has been adopted by modders for other games and even some companies.

Point well taken and once I realized the issue is specific to IEnumerable, I kind of regretted what I wrote yesterday, or at least how I wrote it. It still doesn't change the fact that IEnumerable are widely used and that people should be able to mod the contents of such lists without reinventing the wheel.

Without the time right now to go into your issue in-depth I can at least say that I might add a helper function to the already existing transpiler tools in Harmony. Something that will allow anyone to patch enumerators on a higher level. I already did that for replacing all occurrences of a call to a method A with B inside a patched method. I might just find a generic way to do what you want.

Personally, I habe successfully patched enumerators before, I just hadn't have the time to make it generic.

Quote from: Nightinggale on October 22, 2017, 01:53:14 PM
            while (__result.MoveNext())
            {
                // NewList.Add(__result.Current); <-- this line crashes the game
                StatDrawEntry temp = __result.Current; // this doesn't
                NewList.Add(temp);
            }
            // todo append modded lines to NewLine[/code]

Do anybody have an idea on how to get this working?

If what you want to do is read __result and then return a different IEnumerable, I would try adding a small method to the class which does this. Consider this and see if it works for you:

[HarmonyPatch(typeof(Verse.Corpse))]
[HarmonyPatch("SpecialDisplayStats", PropertyMethod.Getter)]
class CorpseDisplay
{
static void Postfix(Verse.Pawn __instance, ref IEnumerator<StatDrawEntry> __result)
{
IEnumerable<StatDrawEntry> NewList = this.modifiedResult(__result);

Log.Message("This is a debug message that shows I'm making a new IEnumerable!");

__result = NewList;
}

private IEnumerable<StatDrawEntry> modifiedResult(IEnumerable<StatDrawEntry> source)
{
foreach (StatDrawEntry entry in source)
{
if (myCustomBoolHere || !someOtherBoolIWantToBeFalse)
{
yield return entry; //Add source entry to my modified IEnumerable if I don't need to change it etc.
}

else
{
StatDrawEntry replacementEntry = entry;
replacementEntry.overrideReportText = "Hi, I'm a new entry"

yield return replacementEntry;
}
}
}
}

Hmmm. I didn't even know that postfix methods could change the return value. I thought only prefix methods could do that. This confuses me a lot.

In any case, your code appears to simply make a copy of the enumerator, and you say that this "crashes the game". Can you explain what the crash error is? Is there a specific exception?

I finally got it to work!!!!  :D
    [HarmonyPatch(typeof(Verse.Corpse))]
    [HarmonyPatch("SpecialDisplayStats", PropertyMethod.Getter)]
    class CorpseDisplay
    {
        static void Postfix(Verse.Corpse __instance, ref IEnumerable<StatDrawEntry> __result)
        {
            // Create a modifyable list
            List<StatDrawEntry> NewList = new List<StatDrawEntry>();

            // copy vanilla entries into the new list
            foreach (StatDrawEntry entry in __result)
            {
                // it's possible to discard entries here if needed
                NewList.Add(entry);
            }

            // custom code to modify list contents
            // add vanilla leather line just to verify that output is modified
            StatDef leatherAmount = StatDefOf.LeatherAmount;
            NewList.Add(new StatDrawEntry(leatherAmount.category, leatherAmount, __instance.InnerPawn.GetStatValue(leatherAmount, true), StatRequest.For(__instance.InnerPawn), ToStringNumberSense.Undefined));

            // convert list to IEnumerable to match the caller's expectations
            IEnumerable<StatDrawEntry> output = NewList;
           
            // make caller use the list
            __result = output;
        }
    }
Yes, this will indeed just duplicate the leather line, but it's fine for testing if __result is modified. When I started having problems with IEnumerable, I decided on focusing on the GUI first precisely because it's so easy to see results ingame.

It works and quite importantly it seems to allow multiple mods to make Postfixes for this function without conflicting.

There is still a bit weirdness, which is __result only contains 3 lines while 8 are displayed ingame. It likely has to do with SpecialDisplayStats being overloaded and perhaps multiple versions are called. Luckily my goal is to add a line, in which case this doesn't matter.

Quote from: Dingo on October 22, 2017, 06:52:23 PMIf what you want to do is read __result and then return a different IEnumerable, I would try adding a small method to the class which does this. Consider this and see if it works for you:

Thanks for trying, but I couldn't get that approach to work. It did however provoke my train of thought and it might have played a role in finally getting it right. Also thanks to pardeike. You also helped me a lot in getting this to work.

See? That wasn't too hard. And we haven't violated the multiple mods compatibility either. Great work.

Now that I got Harmony working and has a bit of experience with it, I will say that despite what I said earlier, it's absolutely great. The code I pasted earlier has worked as a template and I have made multiple mods since (only one published), making hooking up to new methods really fast and easy. Adding multiple postfixes to the same method works well and without conflicts and updating from A17 to A18 didn't even require me to recompile.

In short, great work on Harmony. I absolutely love it now. The only issue is the learning curve, but now I plan to write something on the RimWorld wiki based on my experience, which will hopefully help other people get started.

Most excellent. I am really glad you like what I created. Keep up the good work and if you find time, maybe write a tutorial. So far, I have made the Harmony wiki and a gist tutorial about Transpilers: https://gist.github.com/pardeike/c02e29f9e030e6a016422ca8a89eefc9

Cheers

Quote from: Dingo on October 22, 2017, 03:03:52 AM
I think results speak for themselves here. Not only do all self-respecting RimWorld modders use Harmony for things we would have detoured in the past, but the library itself has been adopted by modders for other games and even some companies.

calling developers of alternative technologies "non self-respecting" is not ever close to reasonable.

Harmony is wrapper over unsafe and it implements exactly same single line payload that used in early code injection.

as for iteration\yeld return - Zenthar great job backfired, as it hide actual implementation from developer below tick layer of syntax sugar.
It's possible to automate recovery and redirect injections, but this harmful in longrun as developers must have at least basic understanding of C# internals and must be aware about syntax sugar and how real CIL looks like.

I actually don't like the decorator methods used by Harmony. It invited the developer to eschew proper exception handling. If the decorator method wesn't used, and you always had to call the manual methods, the pardeike probably would have had exceptions thrown all the way through to the caller. That's the way they should be. Instead, you often end up with silent failures. It's a bit of a debugging disaster.

Quote from: RawCode on November 08, 2017, 03:55:55 AMHarmony is wrapper over unsafe and it implements exactly same single line payload that used in early code injection.

I'm not entirely sure I agree with your statement here. The old approach is to claim the method call and prevent other mods from accessing that particular method. Harmony allows multiple mods to attach to the same method. Sure it's not without danger, but if you look in the help section, there was recently a thread about using Transpillar in a method where another mod use Transpillar and on how to make the two mods compatible. The fact that it eventually ended up working as intended shows that Harmony is far superior to not using Harmony. Sure it could be considered unsafe, but at the same time allowing such "unsafe and conflict prone" code allows compatibility, which would otherwise be impossible.

I don't really agree with the approach of returning false in Prefix though. I fail to see the difference between that and just redirecting the call and claim it for your mod. On the other hand I have had great success with adding multiple Postfix to the same method and since they append to the output list rather than modifying it, conflicts are avoided without even considering conflicts, just the way it should ideally be. Each such mod will not have to even consider the existence of another mod attaching itself to the same method.

Quote from: RawCode on November 08, 2017, 03:55:55 AMas for iteration\yeld return - Zenthar great job backfired, as it hide actual implementation from developer below tick layer of syntax sugar.
It's possible to automate recovery and redirect injections, but this harmful in longrun as developers must have at least basic understanding of C# internals and must be aware about syntax sugar and how real CIL looks like.

That's actually a really good point. If I had not used Zenthar's fork of ILSpy, I might not have had the amount of problems I had. I will remember this if I run into problems in the future.

Quote from: CannibarRechter on November 08, 2017, 06:30:20 AM
I actually don't like the decorator methods used by Harmony. It invited the developer to eschew proper exception handling. If the decorator method wesn't used, and you always had to call the manual methods, the pardeike probably would have had exceptions thrown all the way through to the caller. That's the way they should be. Instead, you often end up with silent failures. It's a bit of a debugging disaster.

I'm not completely following you. When will you lose exceptions? What will prevent you from using Transpillar to just inject a method call and then deal with that method including exceptions in a method, which is not part of Transpillar?

Or are you saying that going through any Harmony method (Prefix and Postfix included) will silence the exceptions in your methods?

> Or are you saying that going through any Harmony method (Prefix and Postfix included) will silence the exceptions in your methods?

Harmony ITSELF doesn't throw exceptions when it should. I believe this is because of the attributes (decorators); these don't afford the programmer a useful place to catch exceptions if thrown. While the design is pretty and convenient, I don't believe this worth the trade.

The next version should have try-catch actually.

@Nightinggale
https://github.com/pardeike/Harmony/blob/master/Harmony/ILCopying/Memory.cs

Multiple mods that try full replacement will conflict anyway, there are no ways around, this is how things works.
Same for bytecode injections.

People must understand and know truth about system and how exactly it works in order to use it in efficient manner.
When people do not understand internals, they phone to make wrong assumption, this is bad in longrun.

As for exceptions, try checking stacktrace of your injection, all Prefix and Postfix methods are called via reflection hack from inside of gate method one by one, it's quite easy to wrap everything into try catch and process exceptions in orderly manner.

Quote from: RawCode on November 08, 2017, 09:16:14 AMMultiple mods that try full replacement will conflict anyway, there are no ways around, this is how things works.
Same for bytecode injections.

Obviously conflicts can't always be avoided and I don't think anybody with any insights at all will ever claim otherwise. My point is that you can write a Prefix and return false and you can write a Transpillar to remove the method contents entirely and just make a method call to a new method where you write the same as you would have in Prefix. Both will end up being complete replacements, but only the Transpillar will not conflict with another mod using Postfix. There are also cases where a light modification of a method with Transpillar (like changed conditions in if statement) will be compatible with other mods using Transpillar on the same method. In other words using Prefix and returning false seems to be compatibility hostile compared to other options.

It goes without saying that some mods will conflict by design. Imagine having two people giving directions for where a car should go. One says left and the other says right and then they both assume the car to do both. It's the same with mods and it only make sense to talk about compatible coding if the mods do not conflict on the concept level and I'm talking about cases where compatibility is possible if done right, but where it's possible to write code, which will conflict. I don't think it will make sense to talk about writing compatible code in any other cases. However you have to remember that while you might not have to consider being compatible with anything, the mod somebody else makes next month might have to be compatible with your mod, meaning the new mod might require you to have compatibility friendly code and not just return false in Prefix.

> Prefix and return false and you can write a Transpillar to remove the method contents entirely

Almost no modders will be implementing transpilers: they would be high end work even for a software engineering professional (retired software engineering professional here). So the practical thing for most modders to do is use postfix methods when possible, and prefix methods (with false return values) when they have to. If a few stretch themselves and learn transpilers, great, but that's not gonna happen very often, let's be honest.

Quote from: CannibarRechter on November 08, 2017, 11:21:47 AMAlmost no modders will be implementing transpilers: they would be high end work even for a software engineering professional

That's my prediction as well. People will use high level if they can get away with it. However that doesn't change the fact that Transpillar is a better option than Prefix returning false when it comes to mod compatibility.

Perhaps it would be a good idea to introduce a new hooking method, which provides a method with the same arguments and return value as the vanilla one and then the modded one is called instead. It's like detouring, except it will not conflict with mods using Prefix and Postfix. While I prefer people to not use it, it will still be better than using Prefix and returning false.

Alternatively it should be made possible to make a Prefix, which can skip vanilla only or both vanilla and other mods.

Guys, keep to the facts. A quick search on GitHub reveals over 20 pages! of search results of people writing transpiler code:

https://github.com/search?l=C%23&q=IEnumerable%3CCodeInstruction%3E&type=Code

Point well taken. What I was actually thinking of when writing my last post is the group of modders, which won't or rather aren't skilled enough to use Transpillar. I'm not really concerned with the number, but I should have pointed out that I disagree with "Almost no modders". My concern is that a number of mods relies on returning false in Prefix and that it's the least compatible approach. Another concern is the fact that some mods still use detouring. My thinking is that if a "detour" option exist in Harmony and that it support that mods can use Prefix and Postfix, then even though it isn't ideal, it will still be better than what they use today.

I'm not saying Harmony is broken. The problem is as always people are surprisingly good at figuring out how to break otherwise good solutions.

The community and the major modders had the exact same concern. Interestingly, it turned out to not be a technical problem. It's educational and that you don't solve with technology. Instead we focused on spreading knowledge and helping out those who struggle or are ignorant. This is exactly what the discord servers are for and they do a pretty good job.

Hi guys!

I have a series of doubts, and some of them may sound quite stupid. I have a little bit of background as a programmer, but my usual approach is throwing things and seeing what sticks...

So first, what I'm trying to do: I want to add to the Genetic Rim mod the possibility of controlling directly ("drafting") some of the tamed animals in the mod. Because that would be so cool. So I looked around and saw a mod called DraftAnything that hasn't been updated since a15, but the main ideas, I think, will still stand.

If I understand correctly, the best way to proceed would be to modify with Harmony the methods AddAndRemoveDynamicComponents in RimWorld.PawnComponentsUtility, GetGizmos in Verse.Pawn and CanTakeOrder in RimWorld.FloatMenuMakerMap (I'm not smart enough to figure this out, Shaun McFall did in that mod).

However, in his mod he used an "Inject" method, which if I'm interpreting this correctly just changes the vanilla method with his own (was that an old Harmony method?). This will obviously cause problems with other mods. Looking at the documentation, I infer that I should use a Postfix to add some code to the end of the methods I want to change, or a Transpiler, which either swaps code for other code, or is slang for a transexual caterpillar.

So, apart from that last idiocy, am I in the right track?

PS: the mod: https://ludeon.com/forums/index.php?topic=26830.0

Yes. You are on the right track. Although not the simplest example to start with. Remember the pseudo code on the Harmony wiki that explains how the patching works and the information flow through the prefix/postfix methods. Also: if you want to go beyond prefix/postfix stuff but still not feel for the full complexity of a complete transpiler, you should look into the readymade transpilers that come with Harmony, like the MethodReplacer that allows you to replace a method with your own but inside the method that you patch. If you're smart you can use that to insert code inside a method without an understanding of IL.

Quote from: pardeike on November 29, 2017, 02:26:30 PMyou should look into the readymade transpilers that come with Harmony, like the MethodReplacer

Is there a list of Harmony features like that anywhere? I missed the existence of this one until right now. It looks to me like it totally beats both then redirection and then Prefix+return false approaches since both supports Postfix just fine.

In fact is there any Transpiler documentation other than the wiki from the first post? It seems that the most useful I can find is working examples of code, but looking at Harmony source code, it looks like they are only scratching the surface of all the features.

Also how soon can Transpiler be applied? It looks like all dll files are loaded in Verse.LoadedModManager.LoadAllActiveMods() and obviously Harmony will not work before being loaded. However can it be used in this method after the dll files have been loaded or will it only work on methods called after harmony is loaded?

No, there is no such list. Transpilers are ultimately simple in design as all they do is take a list of codes and spit out a list of codes. The buildin transpilers all exist in the Transpilers class and the best I can offer is this transpiler tutorial gist:

https://gist.github.com/pardeike/c02e29f9e030e6a016422ca8a89eefc9

Other than that, try to learn how IL works and you will not regret it. It's a rather simple language with a stack and easy to reason about. Microsoft has good documentation on it.

Applying patches (pre- post- or transpiler) works at any time. It's just that the execution of the method you patch will be the original one until you patch it and only in that sense the timing of the patch is relevant. Harmony truely changes the action of a method at runtime.

Oh my god, I got it! I'm right now making a Bearodile run at my command around the map munching on things! Thanks a lot, Harmony seems super powerful and it is really not very difficult to use, even if you don't quite know what you are doing hehe

I have run into an issue with ModCheck and either Harmony suffers from it too or I would like to know how it's solved. The problem is when multiple mods adds the DLL. The game will load one of them and reject the rest, which is ok if all of them are updated. However if some fail to update, then the goal becomes to load the newest. However based on my experience and reading Verse.LoadedModManager.LoadAllActiveMods(), my impression is that it loads the first dll it encounters. This mean it will use the dll from the first mod in the modlist, regardless of the version number.

How do you ensure that the newest version of Harmony is used if there are multiple versions to pick from? Since ModCheck isn't needed before patching, it should be possible to use Harmony to load another DLL if needed and use reflection between dll activation and xml loading. However I'm far from certain that it's the best solution. How is Harmony dealing with this?

There is no easy way to solve this. I am working on an update to Harmony and so far, don't have any solution to the problem you describe.

Ok, then it's not just me starring myself blind on the problem. If there is no known automated correction of the problem, then maybe an automated detection could be used.

foreach (ModContentPack ModPack in LoadedModManager.runningMods)
{
foreach (Assembly AsmFile in ModPack.assemblies.loadedAssemblies)
That should loops the dll files in the order they are loaded. This allow reading AssemblyTitle, AssemblyVersion and AssemblyFileVersion and if you filter for a specific AssemblyTitle, then it should be possible to figure out if the highest number is the first. If not, it can write an error to the log and create a popup telling the user to upgrade 0Harmony.dll in a specific mod or switch mod order.

Not great, but at least better than the current situation.

foreach (ModContentPack ModPack in LoadedModManager.runningMods)

There used to be a similar problem to this in A17 for textures, where the textures/ folder was "first man wins" for a similar reason (similar code), and I reported this to the devs as a bug. They fixed it for A18.

If this bug exists for the assemblies as well, you may want to report it.

Quote from: CannibarRechter on December 02, 2017, 07:11:13 AMIf this bug exists for the assemblies as well, you may want to report it.

Done https://ludeon.com/forums/index.php?topic=37361.0 (https://ludeon.com/forums/index.php?topic=37361.0)

Overall, that would not solve the problem completely. In order to allow multiple patches to coexist on a method, Harmony has to maintain global state. I currently do it with a singleton in-memory assembly that has a static field. Works great until you have to introduce breaking changes.

While the global state is carefully designed to not contain domain specific objects (only things like String, MethodInfo and a few simple wrapper objects) the overall patch process requires that every time someone patches a method, any existing patches have to be replayed to generate the final (and new) replacement method. Like this:

Mod 1:
Original -> patch1 -> Replacement

Mod 2:
Original -> patch1 -> patch2 -> New replacement

Here, patch1 is just a method call inside the dynamically build replacement method.

This works just fine with prefixes and postfixes but transpilers are a problem. They are also chained so if two mods use transpilers, you get this:

Original -> List of CodeInstructions -> Transpiler1 -> Transpiler2 -> Replacement

Here, CodeInstruction can actually come from two different versions of Harmony and thus cannot be assigned:

// Mod1:
CodeInstruction ci1;
// Mod2:
CodeInstruction ci2 = ci1; <-- will throw a cast exception

The current solution to this is to serialize/deserialize the instructions and that just works fine too.

Now, in order to support try-catch blocks, I have to modify the Harmony type CodeInstruction so it carries, preserves and supports the information that is required. This will break the serialization or it will remove the unserializable information effectively erasing it.

One possible solution would be to patch existing Harmony methods to use the new type everywhere. That should work in theory since the extra fields do not need to be understood by the old library to successfully carry over the information during the piping. Old user code adding instructions could instantiate the new class and defaults would prevent errors. No idea if this is doable and maybe someone else could help me discuss this idea.

Quote from: pardeike on December 02, 2017, 01:25:33 PMHere, CodeInstruction can actually come from two different versions of Harmony and thus cannot be assigned:

I'm not really sure we are talking about 100% the same problem. Based on my testing, if somebody loads two mods A and B, both containing ModCheck.dll, then both mods will be using ModCheck from A. There is no conflict from using multiple mods, but if B has a newer version with more tags, bugfixes or similar, then it could cause issues because it's using the outdated version from A.

Based on this, I don't really get the issue of two versions of Harmony using transpiler on the same file ???

Sorry, I thought you were talking about the Harmony dll. Btw, dlls are not loaded again only if the dll version is the same. This is because of the system dll cache and is hard to work around - even from RimWorld.

Quote from: pardeike on December 02, 2017, 06:06:49 PM
Sorry, I thought you were talking about the Harmony dll. Btw, dlls are not loaded again only if the dll version is the same. This is because of the system dll cache and is hard to work around - even from RimWorld.

I'm talking about dll loading in RimWorld when there is more than one copy of the same dll with more than one version. Since I ran into this issue with ModCheck, I figure Harmony would have the same issue. It's not about Harmony's inner workings, but rather how RimWorld reads Harmony in the first place.

In my tests, as long as both dlls have different versions, they are loaded separately you rimworld (or any other host assembly)

Hey Pardeike,

after updating and loading my gianourmous modlist again and loading it without a hitch,
I thought I should thank the one guy who became essential for that.

You.

So thank you for making mods compatible.

Thank you for your feedback. Very kind. It is my pleasure to contribute to a otherwise hard to solve problem.

ENJOY

I have run into a timing issue regarding starting Harmony. I'm trying to use it in Verse.LoadedModManager.LoadAllActiveMods() and Verse.ModContentPack.LoadDefs().

The problem is that LoadAllActiveMods() loads the dll files and then it calls LoadDefs() to load xml files. Once all the xml files have been loaded and all patches have been applied, [StaticConstructorOnStartup] activates and start Harmony.

This puzzled me a bit because I thought [StaticConstructorOnStartup] would be called when the DLL is loaded. However I can add PatchOperations to a DLL and then call it during patching, which takes place prior to calling [StaticConstructorOnStartup]  :o

I made a PatchOperation and called that as the first patch in the first mod. I made this one init Harmony and sure enough Prefix() for LoadDefs() started to work. I can do without applying a Transpiler to LoadAllActiveMods() if I really have to, but I would really like to apply a Transpiler to LoadDefs() before it's called the first time and my current approach will make LoadDefs() start Harmony on first call. Also relying on a certain patch file to be present is not good, particularly not when this requirement is added to 3rd party mods.

Any ideas on how to activate Harmony earlier?

You need to extend the official "Mod" class. It is the preferred way to start a mod and runs much earlier.

Even so, note that StaticConstructorOnStartup is actually called *twice*, and the very early version of it is called before any mods are loaded. This is one of the reasons that many of the cores "readonly" attributes cannot be redirected (or, rather, can be, but you have to use Reflection to overwrite them instead of injecting the initial code).

Quote from: pardeike on December 11, 2017, 01:36:21 AM
You need to extend the official "Mod" class. It is the preferred way to start a mod and runs much earlier.

I'm not really sure what I'm supposed to do. It's not possible to make an extension constructor (https://stackoverflow.com/questions/4782034/is-it-possible-to-create-constructor-extension-method-how). A normal extension method is not called by vanilla. Class inheritance doesn't seem to be the answer either as no instance of the child class will be allocated, hence no constructor. The only thing left I could think of would be .cctor, but it looks like that doesn't work with RimWorld (https://ludeon.com/forums/index.php?topic=16880.0). I'm back to being out of ideas on how to execute C# code prior to LoadDefs() being called for the first time.

If there is some other way of activating Harmony earlier, then please enlighten me.

>>  You need to extend the official "Mod" class

> It's not possible to make an extension constructor.

Pretty sure this got garbled. The game has first class support for running classes you inherit from the Mod class. If I remember when I get home at night, I'll look for the pattern for you. But I doubt he meant C# style extension classes here at all.

I got it working (finally)
    public sealed class HarmonyStarter : Mod
    {
        public HarmonyStarter(ModContentPack content) : base (content)
        {
            var harmony = HarmonyInstance.Create("com.rimworld.modcheck");
            harmony.PatchAll(Assembly.GetExecutingAssembly());
        }
    }
It's so beautifully clean and simple once you see the result and once again I'm like "I spent this long on that???".

Thanks for the replies. I'm not sure I would have thought of inheriting the Mod class on my own.

Yup. You got it. That's what I was gonna post for you. ;-P

A heads up to other developers. I was having issues on Mac and Linux (Ubuntu specifically) systems where RW would crashing when my mods were enabled. I tracked the problem down to the simple act of patching Window.PreClose:

[HarmonyPatch(typeof(Window), "PreClose")]

Just patching it will causes the game to crash. More info here: https://ludeon.com/forums/index.php?topic=37269.msg385072#msg385072

My question was answered in another thread.

Quote from: Nightinggale on December 01, 2017, 02:31:54 PM
I have run into an issue with ModCheck and either Harmony suffers from it too or I would like to know how it's solved. The problem is when multiple mods adds the DLL. The game will load one of them and reject the rest, which is ok if all of them are updated. However if some fail to update, then the goal becomes to load the newest. However based on my experience and reading Verse.LoadedModManager.LoadAllActiveMods(), my impression is that it loads the first dll it encounters. This mean it will use the dll from the first mod in the modlist, regardless of the version number.

I finally managed to solve this. Even better I have a generic solution, which seems to work for all DLL files including Harmony. Because of this I have decided to ask for feedback here before making a real release. It's a standalone DLL and while it can handle multiple versions of other DLL files, it can't handle multiple different versions of itself, which is why I want to get it right in the first release.

The project url: https://github.com/Nightinggale/VersionedAssemblyLoader (https://github.com/Nightinggale/VersionedAssemblyLoader)

The approach is as follows:
It relies on mod class init, which is the first thing to run in vanilla after loading DLL files. On init, it loops all mods and stores data on each DLL file located in a folder called VersionedAssemblies. Once completed, it loads one copy of each filename and then it makes a call to init code in the newly loaded DLL file.
The following rules apply:

• Filenames are sorted alphabetically (0Harmony.dll is first)
• When multiple files have the same name, it will use the file with the highest AssemblyFileVersion
  AssemblyFileVersion is assumed to be of type Version, that is x.x, x.x.x or x.x.x.x where x is ints
• It calls title.VersionedAssemblyInit where title is AssemblyTitle
  AssemblyTitle is assumed to be a static method without parameters and is ignored if the method is missing

I think this is enough to allow it to apply to any DLL file, which is meant to be included in multiple mods while ensuring that the DLL file is only read once and only the newest version is used.

Any comments about this? Feel free to test this, but please do not start distributing until I release it as a stable release.

Quote from: Nightinggale on January 22, 2018, 01:14:00 PM
It's a standalone DLL and while it can handle multiple versions of other DLL files, it can't handle multiple different versions of itself, which is why I want to get it right in the first release.

You may be interested in this. It allows easier updates to .dll mods that get bundled with other mods. It stores which version of the assembly is the latest into a unity GameObject and only executes on that assembly. It will only need a few small changes to work for you.

It starts at the Main.cs constructor (http://"https://github.com/scuba156/DependencyChecker/blob/master/DependencyChecker/Source/Main.cs#L20"), the first instance ran will call Init() (https://github.com/scuba156/DependencyChecker/blob/master/DependencyChecker/Source/Main.cs#L54) which finds all available assemblies and determines the newest version (my code also uses GameObjects to store other data, so Init() does some other things too). Then back at the constructor, it calls Execute() if the current instance is the latest version, otherwise it skips this version and will not execute. Everything inside Execute() is then safe to change in future versions if needed, everything before it cannot be updated without ensuring it will be called since only the first instance will actually run it and that could be any version. From what I remember, RimWorld will only load one instance of an assembly that has the same name and version, no matter where it comes from, so you don't have to check for that.

Other classes you will need are GameObjectUtils.cs (https://github.com/scuba156/DependencyChecker/blob/master/DependencyChecker/Source/Utils/GameObjectUtils.cs) (you can remove StoredModIdentifiers and the ComponentModIdentifiers class) and AssemblyUtility.cs (https://github.com/scuba156/DependencyChecker/blob/master/DependencyChecker/Source/Utils/AssemblyUtility.cs).

Side note: I rediscovered this code snippet yesterday for anyone intersted. If you want to use use any translated strings for any sort of UI when you have a dll like this that gets bundled with other mods, but want to everything contained in a single dll file, then set all your language files as an EmbeddedResource and then you can use this class (https://github.com/scuba156/DependencyChecker/blob/master/DependencyChecker/Source/Utils/LanguageUtility.cs) to extract and load them when you need them. Only thing is I cannot remember if I tested it with a dll that has multiple languages, so a check might need to be added to determine the current language and only load that one, but it also might not need it, no idea. It can also be adapted to work on other file types like images.

Quote from: Kiame on December 16, 2017, 02:44:08 PM
A heads up to other developers. I was having issues on Mac and Linux (Ubuntu specifically) systems where RW would crashing when my mods were enabled. I tracked the problem down to the simple act of patching Window.PreClose:

Code Select Expand

[HarmonyPatch(typeof(Window), "PreClose")]

Just patching it will causes the game to crash. More info here: https://ludeon.com/forums/index.php?topic=37269.msg385072#msg385072

The same happens with Window.ExtraOnGUI(). Since the code I was executing wasn't all that important, my temporary solution was to manually patch it only on windows platforms (https://github.com/scuba156/ModListBackup/blob/v2.0/ModListBackup/src/Patches/Patcher.cs), leaving unix unpatched. It works, but it's situation dependent.

Quote from: scuba156 on January 22, 2018, 11:40:01 PMYou may be interested in this. It allows easier updates to .dll mods that get bundled with other mods. It stores which version of the assembly is the latest into a unity GameObject and only executes on that assembly. It will only need a few small changes to work for you.

Thanks, but after close examination of the source code as well as testing, I ended up not believing it to work for me. Say we have a DLL where version 1.1 and 1.2 are present in Assemblies and loaded in that order. Your approach is to call both in [StaticConstructorOnStartup]. First 1.1 detects that it's outdated and does nothing, then 1.2 is called and it executes. ModCheck has to work on PatchOperations, which are executed prior to [StaticConstructorOnStartup] calls. This mean all I have available is method calls from vanilla. It will start by calling 1.1 and even if I decide not to do anything, it will not call 1.2 afterwards. ModCheck has caused a number of issues from being loaded this early because not all features are available until some time after patching.

Quote from: Nightinggale on January 23, 2018, 04:20:11 AM

Quote from: scuba156 on January 22, 2018, 11:40:01 PMYou may be interested in this. It allows easier updates to .dll mods that get bundled with other mods. It stores which version of the assembly is the latest into a unity GameObject and only executes on that assembly. It will only need a few small changes to work for you.

Thanks, but after close examination of the source code as well as testing, I ended up not believing it to work for me. Say we have a DLL where version 1.1 and 1.2 are present in Assemblies and loaded in that order. Your approach is to call both in [StaticConstructorOnStartup]. First 1.1 detects that it's outdated and does nothing, then 1.2 is called and it executes. ModCheck has to work on PatchOperations, which are executed prior to [StaticConstructorOnStartup] calls. This mean all I have available is method calls from vanilla. It will start by calling 1.1 and even if I decide not to do anything, it will not call 1.2 afterwards. ModCheck has caused a number of issues from being loaded this early because not all features are available until some time after patching.

If I understand correctly, then you should be able to load the latest assembly into memory and invoke a class in there. Activator.CreateInstance(string,string) (http://"https://msdn.microsoft.com/en-us/library/d133hta4(v=vs.110).aspx") should help, there is also Activator.CreateInstance(Type) (http://"https://msdn.microsoft.com/en-us/library/wccyzw83(v=vs.110).aspx"). It has examples there that should help, also check the remarks section.

You could then store a bool on whether you have executed and use reflection to read the variable. HugsLibChecker has an example here (http://"https://github.com/UnlimitedHugs/RimworldHugsLibChecker/blob/master/Source/HugsLibChecker.cs#L61").

Quote from: scuba156 on January 23, 2018, 06:58:46 AMIf I understand correctly, then you should be able to load the latest assembly into memory and invoke a class in there. Activator.CreateInstance(string,string) (http://"https://msdn.microsoft.com/en-us/library/d133hta4(v=vs.110).aspx") should help, there is also Activator.CreateInstance(Type) (http://"https://msdn.microsoft.com/en-us/library/wccyzw83(v=vs.110).aspx"). It has examples there that should help, also check the remarks section.

I am actually using CreateInstance. However all those proposals makes me wonder if we had a case of miscommunication. I did manage to make a working solution here: https://github.com/Nightinggale/VersionedAssemblyLoader (https://github.com/Nightinggale/VersionedAssemblyLoader) (same link as before). I could go ahead and release it as it is, but since it will be a mess if multiple versions are floating around in different mods, I decided to ask if people would have requests or could spot issues before releasing anything. Harmony has the same issue if updating, which is why I decided to ask the question here. I'm not asking for a new design just for the sake of change. It should be if it's an improvement over the current implementation.

Having said that, I don't feel like I wasted time reading code for other possible solutions. Not only is it always good to consider if the chosen solution is the best, I also realized that AssemblyUtility.CurrentAssemblyVersion can be used to provide an error if it is using an outdated DLL file. Not ideal, but better than using an outdated version without acting on it.

using Verse;
using Harmony;
using System.Reflection;

namespace Prototype
{
        [StaticConstructorOnStartup]
        class Main
        {
                static Main()
                {
                        var harm = HarmonyInstance.Create("rimworld.mod.nutrition");
                        harm.PatchAll(Assembly.GetExecutingAssembly());
                }
        }
        [HarmonyPatch(typeof(IngredientValueGetter_Nutrition))]
        [HarmonyPatch("ValuePerUnitOf")]
        class Mod
        {
                [HarmonyPostfix]
                static void ValueForMeat(ref float __result, ThingDef t, IngredientValueGetter_Nutrition __instanse)
                {
                        float meat = t.IsMeat ? 2.6f : 1;
                        __result *= meat;
                }
        }
}

Where may be issue, this code just not work.

Quote from: fiziologus on February 18, 2018, 11:56:56 PM

Code Select Expand

using Verse;
using Harmony;
using System.Reflection;

namespace Prototype
{
        [StaticConstructorOnStartup]
        class Main
        {
                static Main()
                {
                        var harm = HarmonyInstance.Create("rimworld.mod.nutrition");
                        harm.PatchAll(Assembly.GetExecutingAssembly());
                }
        }
        [HarmonyPatch(typeof(IngredientValueGetter_Nutrition))]
        [HarmonyPatch("ValuePerUnitOf")]
        class Mod
        {
                [HarmonyPostfix]
                static void ValueForMeat(ref float __result, ThingDef t, IngredientValueGetter_Nutrition __instanse)
                {
                        float meat = t.IsMeat ? 2.6f : 1;
                        __result *= meat;
                }
        }
}

Where may be issue, this code just not work.

Have you verified the patch is being called? Add

Log.ErrorOnce("IngredientValueGetter_Nutrition.ValuePerUnitOf Patch Called", __instance.GetHashCode());

To the beginning of the method to make sure it is successfully overridden. If it is then it could be the reference is not getting set. I've had other issues at times where reference values were not getting persisted and being verbose fixed the problem. In this case try:

float meat = t.IsMeat ? 2.6f : 1f;
meat = meat * __result;
__result = meat;