Menu

Show posts

This section allows you to view all posts made by this member. Note that you can only see posts made in areas you currently have access to.

Show posts Menu

Messages - Nightinggale

#31
Bugs / Re: [1.0] Suggested Colony name error
June 19, 2018, 01:42:20 PM
While talking about string length restrictions, I will add that savegames can be an issue too since they copy the name. Even with the same limit, it's still an issue because with savegames you would sometimes like to add a suffix to the name, like "raid", which isn't possible to add if the default name is 3 characters shorter than the limit. I encountered this issue in B18 once and while it wasn't a huge issue, it did become a bit annoying at times.
#32
Quote from: ZorbaTHut on June 18, 2018, 02:27:48 PM
Quote from: Nightinggale on June 17, 2018, 03:25:19 PM
Obvious solution: Move the loop to a new method.

Done. I've actually function-ized a lot of things in that function, both for readability reasons and for modability reasons. Let me know if this doesn't do the job, but I think it will.
While I haven't actually coded anything yet, I see no reason why I would be prevented from doing anything I plan to do. In fact the current build appears to offer the most modder friendly design of LoadAllActiveMods() ever provided by RimWorld. Great job.

Quote from: Ulvaar on June 19, 2018, 12:49:38 PM
Excellent update on the whole, enjoying the rebalace!

Funny little bug I just encountered: wanted to try out the new randomize button for colony names, got a random name that I wanted to keep, but was too long:

https://imgur.com/Yl3dHNJ

'Fraid I didn't check exactly how long it was before clicking away - a limit to the name generator will probably do the trick.
This really should be in the bug report subforum. Apart from that, remember if touching the limit to the colony name, that name is the default name for savegames. It would be nice if the limit to savegame names is longer than colony names, allowing suffixes to your savegame names without deleting from the colony name.
#33
Quote from: SargBjornson on June 18, 2018, 02:15:43 PM
Keep us informed! I hope Tynan can patch things up
Current status is that I received feedback on my feedback. The method in question changed and is updated in the next build. I'm told to report back if it's not enough to do what I want to do. Looks like we will keep ModCheck in RW 1.0  :)

I have no ETA for the next release though, partly due to not having seen the next build yet, partly because I know I more or less will have to modify the core of ModCheck. However it looks like it requires a lot of work (time) and I will likely end up releasing a partial version with the most used operations out ASAP. That way I will likely not hold back releases of other mods while I figure out how to get everything working perfectly as well as fully optimized to the new RW 1.0 code design (which is essentially a rewrite).
#34
Quote from: Tynan on June 18, 2018, 10:40:52 AM
Quote from: Hondamusprime on June 18, 2018, 10:37:03 AM
From my experience on the unstable build it seems as if some multicore rendering has been implemented into the final build of rimworld. Is this true?

I'm afraid not, but we did optimize stuff.

From user POV, optimizations are equivalent. Multithreading is just another type of optimization.
I would call this great news. Vanilla is faster, but it still leaves idle CPU cores for mods to exploit. I think only one mod released for B18 actually use more than one core, but one means it's possible and who knows what we will see in the future. Also optimizing by using less CPU time to do the same will lower power usage while optimizing by using more cores will increase power usage. In other words RimWorld got more laptop/battery friendly.

What strikes me the most about the 1.0 changes is actually 64 bit. It's really annoying to have a game in B18, which grows to be so close to the memory limit that the game becomes unstable. It's my experience that I reach this point way before the CPU power becomes an issue.
#35
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.
#36
I have good news and bad news.

The good news is that I finished documenting each PatchOperation on the wiki. I have plans for other pages, like concepts and guides for how to use them together and stuff like that.

The bad news is that I just provided feedback to RimWorld 1.0 because it completely destroys ModCheck in such a way that it's not possible to fix. If ModCheck will make it to 1.0, RimWorld itself will have to change to become more open to ModCheck.
#37
EDIT: I decided to file this as a bug report here. While it might not strictly be the kind of bug reports asked for, it is a feature in RimWorld itself, which allowed something in B18, which is gone in 1.0.

RimWorld 1.0 has completely destroyed ModCheck. The problem is the new patch loading code. It's not the fact that it changed, but more it changed in a way that I lost the ability to mod the patch loading process, meaning I can't update ModCheck to fix the problem.

The problem is Verse.LoadedModManager.LoadAllActiveMods().
foreach (PatchOperation current3 in LoadedModManager.runningMods.SelectMany((ModContentPack rm) => rm.Patches))
{
current3.Apply(xmlDocument);
}

The code itself is fine. The problem is I need to patch it with Harmony. However ModCheck calls Harmony from the mod class instantiation earlier in LoadAllActiveMods(). In order to use Harmony, it needs to be loaded before the patched method is called and as such it's not possible to mod LoadAllActiveMods.

Obvious solution: Move the loop to a new method.

Alternatively make a new method containing everything after the loop starting with: foreach (Type type in typeof(Mod).InstantiableDescendantsAndSelf())
This will give max modding freedom.


To be completely honest, I kind of like the old approach better. Now all files are merged into one and then the patches are applied one by one. In B18, all patches are applied to each file and then the patched files are merged. While it may be slower, the file by file approach provides the modname and filename of the file being patched and ModCheck reads that with Harmony. Not only can this be included in error messages, it can also be used actively by PatchOperation. I added ModCheck.FindFile, which takes two strings and compare them against the current mod and file names. It only returns true if both matches, meaning it can be used to avoid using XPATH searches on wrong files. When it applies just once, the file is significantly smaller. Animal mods often have one file for each animal, meaning Defs will contain just a single child, which should speed up XPATH searching. I have made performance tests with and without FindFile, but not against RW 1.0. Still, since XPATH is by far the slowest part, I will assume the FindFile approach is significantly faster than the RW 1.0 approach.
#38
Tools / Re: [LIB] Harmony v1.1
June 17, 2018, 08:45:29 AM
What happens if one mod use 1.0 and another 1.1? Would it make sense to make all RW 0.18 mods use harmony 1.0 while all RW 1.0 mods use Harmony 1.1?
#39
Quote from: Saebbi on June 07, 2018, 01:30:08 PMFor the rest it doesn't really matter, but best is main mod -> CE -> CE Patch
The game loops the mods several times at startup and each time it reads one part of them. The patches are actually loaded before the xml files. The list of patches is then applied to each xml file right after it has been loaded, but before they are merged into one big database.

Related to load order, the result is this:

  • Animal Collab Project
  • CE
  • Animal Collab Project CE patch
The patch file will trigger on ACP, meaning the patch is applied in step 1, not 3. In other words the patch is "loaded" before CE despite being after CE in the mod list. In other words the order of the patch mod doesn't matter at all. The only exception is if you want to control the order of which two patch mods applies changes to the same file, but I can't really think of a case where this would make a difference without the mods downright overwriting each other (like two mods changing the power usage of the same building), in which case the last one will be used.

The question is if the CE patch requires CE to be loaded. If it does then CE needs to be first. The patch seems to rely on the CE DLL and the DLL files are loaded at the very first thing, meaning this should not cause a load order issue either. If it relies on something in the CE xml files, the error log will tell you something went wrong.
#40
Releases / Re: [B18] Mending
June 05, 2018, 02:18:09 PM
How do you tell the normal and easy mode Mending mods apart while patching?

I found a steam mod called Small workbenches and I noticed that it will patch for Mending if the mod is loaded. However it assumes MendingKit to be present if a mod named Mending is loaded, meaning it causes a lot of errors when using the easy Mending mod.

Obviously the answer is to conditionally patch depending on which version is loaded, but it looks like those two mods are too similar to tell them apart. I found two differences, one is the MendingKit and the other is the different preview image. None of those can be used while patching. Wouldn't it make sense to make at least some difference, like easy mode targets 0.18.0 and normal 0.18.1 in about.xml. ModCheck is able to read that number and that way patch creators will be able to tell them apart. Alternatively renaming one of them, but that would likely be messy to do now that they have been released and the name is in savegames.
#41
Quote from: Saebbi on June 04, 2018, 01:02:03 AMI've used ModCheck before, but i didn't know the difference was that huge.
The core of the speed boost in this specific case comes from ModCheck.FindFile, which was added in 1.6. ModCheck was further optimized in 1.7, which I released yesterday. Regardless of what you once did, just the fact that I used 1.7 and you didn't would give me a better result.
#42
I started writing the wiki for 1.7, but it would be nice to get feedback on the style of writing/layout before I go through all the operations. The pages so far:
Is this something people can read and understand?
Obviously it will need some overview page at some point. This is pages for getting details on specific features.
#43
Releases / Re: [B18] ModSync RW
June 04, 2018, 06:18:17 AM
Quote from: historic_os on June 03, 2018, 10:24:33 PMp.s. with modsync ninja all updates were stored on the database, so I could check if any update between local version and latest was "save breaking" for that flag. since you don't have that version log how does the feature work now?
How about adding a list of savegame breaking versions, or even just last savegame breaking version? It will require converting the version string into Version to figure out if a version is newer than another though, meaning you can can only use 2-4 ints, not characters like b (beta) or similar. Right now all it does is comparing two strings to see if the local matches the remote.
#44
I looked at the Animal Collab Project patch mod and started playing around inside it. Specifically I looked into the trigger conditions and started to profile different solutions.
<li Class="CombatExtended.PatchOperationFindMod">
<modName>AnimalCollabProj</modName>
</li>

This is your version. It takes 10.5 ms to patch.
<li Class="ModCheck.FindFile">
<modName>AnimalCollabProj</modName>
<file>Races_Animal_Angora.xml</file>
</li>
<li Class="ModCheck.IsModLoaded">
<modName>Combat Extended</modName>
</li>

I replaced it with this and it patches in around 0.4 ms.

The difference is that CombatExtended.PatchOperationFindMod will search through the list of loaded mods each time it's called and it's always true (providing the mod is loaded). This will make the game try to patch all files, though it will only find one.

The ModCheck approach will check against the mod and file currently being patched. This is information, which is available without searching and is virtually instant. It is only true when the right file is hit, meaning the game will only try to patch one file.

ModCheck will use nearly the same time regardless of how many mods are loaded while CombatExtended.PatchOperationFindMod will take twice as long when the number of xml files doubles. In other words my example with just a few mods will not show the difference people get from loading 100+ mods.

Remember this is for just one patch. You need to multiply this with the number of patches you add. It quickly adds up. The worst I have seen in released patches was 2 seconds for each patch while I loaded 300 mods to stress test. Considering the number of patches in all those mods, RimWorld took ages to start.

ModCheck.IsModLoaded is an extra check I added. It will make sure the patch will only be applied if CE is actually loaded. This makes it safe to add this patch file to Animal Collab Project. Because IsModLoaded makes use of caching as well, in this specific case it's instant. It's so fast that it takes less time than the randomness in the digits while speed testing.

As for xpath searching read A warning to modders: xpath performance. According to my test results back then, you add 6.4% to the time spend searching the xml files for where to patch.

<Operation Class="ModCheck.Sequence">
<success>Always</success>
<patchName>ACPAngoraRabbit</patchName>

ModCheck.Sequence is a ModCheck 1.7 addition. It works just like PatchOperationSequence, except for adding patchName (and other options we don't need here). Whatever you write here will show up in the log when profiling, meaning if you have 30 patches in a mod and one is slow, you can easily tell which one it is. The time spend on each patch will be printed to the end of the log if verbose logging is on and ModCheck is loaded, either as mod or added as DLL in a mod.


I'm writing this because I assume you aren't aware of this issue, which is not surprising since it's an often overlooked problem. Other than this, you seem to know what you are doing and is doing a great job.
#45
Releases / Re: [B18] ModSync RW
June 03, 2018, 07:12:59 PM
We have such a great new tool, which brings version control without relying on a single person/server and yet nobody replies  ???

I just released ModCheck 1.7 and naturally it comes with ModSync RW support. Now we need to see other mods adding support as well.