[LIB] Harmony v1.2.0.1

Started by Brrainz, January 13, 2017, 04:59:21 PM

Previous topic - Next topic

Nightinggale

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?
ModCheck - boost your patch loading times and include patchmods in your main mod.

Brrainz

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.

Nightinggale

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.
ModCheck - boost your patch loading times and include patchmods in your main mod.

CannibarRechter

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.
CR All Mods and Tools Download Link
CR Total Texture Overhaul : Gives RimWorld a Natural Feel
CR Moddable: make RimWorld more moddable.
CR CompFX: display dynamic effects over RimWorld objects

Nightinggale

ModCheck - boost your patch loading times and include patchmods in your main mod.

Brrainz

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.

Nightinggale

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 ???
ModCheck - boost your patch loading times and include patchmods in your main mod.

Brrainz

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.

Nightinggale

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.
ModCheck - boost your patch loading times and include patchmods in your main mod.

Brrainz

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

SpaceDorf

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.
Maxim 1   : Pillage, then burn
Maxim 37 : There is no overkill. There is only open fire and reload.
Rule 34 of Rimworld :There is a mod for that.
Avatar Made by Chickenplucker

Brrainz

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

ENJOY

Nightinggale

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?
ModCheck - boost your patch loading times and include patchmods in your main mod.

Brrainz

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

CannibarRechter

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).
CR All Mods and Tools Download Link
CR Total Texture Overhaul : Gives RimWorld a Natural Feel
CR Moddable: make RimWorld more moddable.
CR CompFX: display dynamic effects over RimWorld objects