[1.0] [fixed] modding patch loading is no longer possible

Started by Nightinggale, June 18, 2018, 10:19:01 AM

Previous topic - Next topic

Nightinggale

EDIT: build 1939 added full mod support to the code in question, meaning everything mentioned here can now be modded.

I'm not 100% sure where to post this, but I ended up calling it a bug because it works in B18 and it's most likely not intentional.

The issue is Verse.LoadedModManager.LoadAllActiveMods(). In B18, it loads xml def files and apply patches to each before merging. In 1.0, it load def files, merge and then patch. Apparently this is done purely for performance reasons and it will likely patch noteworthy faster when using vanilla operations only.

The problem is that it clashes big time with ModCheck. It's not about the need to update the mod, more like the whole concept is now locked out from working at all regardless of modded code.

What ModCheck does in B18 is use Harmony to patch Verse.ModContentPack.LoadDefs(), which was called for each mod from Verse.LoadedModManager.LoadAllActiveMods(). Here it calls the singleton in ModCheck to inform modname, name of file being patched as well as start and stop for applying each patch. No change to the code, just reading more data.

This is no longer possible. Verse.ModContentPack.LoadDefs() no longer handles patching. Instead patching is done inside Verse.LoadedModManager.LoadAllActiveMods(). ModCheck use an instantiation of a Mod type class to start Harmony. This is done from LoadAllActiveMods() and Harmony doesn't patch methods already in the stack, or at least I couldn't get it to work. However LoadDefs() is called after applying Harmony patching, meaning it's fully patchable with Harmony.

ModCheck aims to speed up patching and profiling has told me that XPATH searching is by far the dominant part concerning time spend patching. ModCheck solves this with ModCheck.FindFile where the patch creator adds two strings, modname and filename. ModCheck will then compare those against strings already in memory and only return true if both are true. This way it's possible to write a patch file, which only makes an XPATH search on a single file and it discards wrong files virtually instantly. Some mods are full of small files, like adding 50 animals with one animal in each file. Not only will FindFile result in only using XPATH once, it will do so on a file with just a single def.

Another major feature is profiling. ModCheck is able to measure time spend on each root PatchOperation. Downloading a bunch of mods from steam and profiling them has revealed that there are patches there, which takes a few seconds per root PatchOperation when loading a lot of mods. The ability to locate such slow patch files is the first step in getting them up to speed.

Another lost feature is log writing, particularly the 5 arguments, which are quite useful when generating error messages, both for ModCheck and for patch creators.

Now what?
If ModCheck would be rendered obsolete by an update in RimWorld, then I wouldn't mind. The problem is the vanilla change is most likely slower than proper use of ModCheck and a number of ModCheck features are lost without a replacement. For users of ModCheck, this is a major step backward.

I propose two changes:
One is to split Verse.LoadedModManager.LoadAllActiveMods() into two methods. Move everything after the foreach (Type type in typeof(Mod).InstantiableDescendantsAndSelf()) loop to a new method and end LoadAllActiveMods() by calling the new method. This way the new method will be compatible with Harmony and some functionality can be restored (particularly profiling).

The other change would be to restore patching ability to Verse.ModContentPack.LoadDefs(). Not as a replacement to the current 1.0 system, but rather it should load patches from a different folder. This way people can use the current patch folder or knowingly relying on FindFile, they can revert to the old approach. This part might be possible to add using Harmony, but I aim to make ModCheck as little intrusive as possible on vanilla patch loading and as such I prefer to just read variables and not actually change functionality in vanilla loading.
ModCheck - boost your patch loading times and include patchmods in your main mod.