[A17] [PSA] Use Def inheritence in your XML's and wildcard-like xpath patching!

Started by CosmicDan, June 27, 2017, 03:14:05 PM

Previous topic - Next topic

CosmicDan

I just started modding the other day (A17b has already been out for some time) and discovered a peculiarity when working with another mod - core def's being re-declared inside the mod itself. Apparently this was how you had to do it before A17 (or possibly older). Oh, the horror! Apparently a lot of modders are not aware that they can directly reference def's from other "packages" now. The benefits of doing so are obvious.

So, I may be a brand new modder but after looking through some prominent mods, I feel it very necessary to raise awareness of two important things...

1) XML inheritance is a new fantastic thing. Don't copy-paste Core (or other mod) Def's into your own mods - you can set your ParentName attribute to any other Def with a Name attribute and it will inherit (copy) all of it's nodes as a base (as long as your mod is loaded after whatever mod the parent is from). You then only need to (re)define the nodes that you want to change, add or remove (FYI, you can remove with empty nodes e.g. <nutrition/>).

Also remember that you can inherit from *any* named def, it does NOT have to be abstract. Aside: It would be great if Ludeon made a change so that every *Def with a defName childnode could be loaded with a runtime-generated Name attribute that matches it's defName (if not already taken) - this would make inheritance even more powerful.

2) Patching and xpath criteria. I don't think a lot of people understand how the xpath stuff works, they just copy-paste other stuff and hack it with a vague understanding. As mentioned in the thread that warns about xpath performance, I feel I need to first reiterate that:

  • //ThingDef[defName="someDefName"]
    Recursively searches an unlimited number of child nodes for ThingDef nodes containing <defName>someDefName</defName>. Very slow and completely unnecessary. This is like searching a lot of text for a relatively common string.
  • */ThingDef[defName="someDefName"]
    Will search child nodes that are nested ONE level ONLY named ThingDef and containing <defName>someDefName</defName>. The * is a wildcard, meaning match any single node. Very fast and how it should be done.


Now, about the "wildcard-like xpath patching" I mention. I see a lot of mods xpathing partially, but in a round-about way. Example:

<Operation Class="PatchOperationInsert">
<xpath>//ThingDef[defName = "FueledGenerator"]/comps/li[@Class = "CompProperties_Refuelable"]/fuelFilter/thingDefs/li</xpath>
<value>
<li>Coal</li>
</value>
</Operation>


In case it wasn't obvious, the intention here is to inject an additional fuel source of Coal (sorry and no offense to the modder who did this - many modders seem to do it this way so I don't blame you) which is repeated similarly a few more times for other machines that use WoodLog as a fuel source. This is not optimal - why not just find every ThingDef that already uses WoodLog as a fuel source, and then inject upon them? There's no reason why you'd not want Coal to be a fuel source for everything! Here's how you'd do that:

<Operation Class="PatchOperationSequence">
<success>Always</success>
<operations>
<li Class="PatchOperationTest">
<!-- Only add if this is a machine that has WoodLog as a fuel source already -->
<xpath>*/ThingDef/comps/li[@Class = "CompProperties_Refuelable"]/fuelFilter/thingDefs/li[text() = "WoodLog"]</xpath>
</li>
<li Class="PatchOperationTest">
<!-- Skip if already patched -->
<xpath>*/ThingDef/comps/li[@Class = "CompProperties_Refuelable"]/fuelFilter/thingDefs/li[text() = "Coal"]</xpath>
<success>Invert</success>
</li>
<li Class="PatchOperationAdd">
<!-- Inject Coal fuel source alongside WoodLog -->
<xpath>*/ThingDef/comps/li[@Class = "CompProperties_Refuelable" and fuelFilter/thingDefs/li[text() = "WoodLog"]]/fuelFilter/thingDefs</xpath>
<value>
<li>Coal</li>
</value>
</li>
</operations>
</Operation>


Comments should make that pretty self-explanatory. Simples! To everyone who hasn't learned about XPathing, I highly suggest the W3CSchools tutorial at least (although it doesn't seem to mention text() and other node tests which is curious, info about them can be found here).

Aside: Regarding the Coal, I'm working on a mod which makes Coal and Charcoal three-time as potent a fuel source as Wood Logs, among many other things (I only just started learning C# yesterday, and coming from Java + ASM for Minecraft modding, this .NET and Harmony library is an absolutely beautiful change and I can't get over how easy this game is to mod... just look at it <3).




I hope this thread gets some exposure so we can see modders making their work as compatible and optimized as possible :)

wwWraith

May be it's better not to delete this because a lot of mods still copying the bases defs and it could be a cause of incompatibilities, their authors should have this information.
Think about it. Think around it. Perhaps you'll get some new good idea even if it would be completely different from my words.

CosmicDan

Quote from: wwWraith on June 27, 2017, 04:14:53 PM
May be it's better not to delete this because a lot of mods still copying the bases defs and it could be a cause of incompatibilities, their authors should have this information.

Really, it's that much of a problem? So maybe I should turn this into some kind of PSA thread heh...

Now, if only there was a way to do some kind of "prerequisite" XML patching, that'd be awesome (even if it only allows insertions). I just found out that you can inherit from any Def that has a Name attribute set, not just Def's that have abstract flag set! So I was thinking it'd be great if you could, say, have an XML patch run that injects Name="ElectricSmelter" to the base ElectricSmelter def, then in your mod XML you could set Parent="ElectricSmelter" to quickly make a custom smelter - override whatever tags necessary.

Or, even easier, if the RimWorld mod API just implicitly added a Name attribute to any *Def tags that don't already have one, just copied from the defName...

...Do the dev's check this forum much? :D Maybe they already have these suggestions or plans.

EDIT: Done. For posterity's sake, here was the original thread before I re-wrote it:

Hi, wondering if someone semi-experienced in modding can help me real quick

I'm just going through the XML of the great "Powerless!" mod with the intention of making a "modpack" (a compatibility/integration mod) and I've noticed that the mod re-declares some base defs in e.g. ThingDefs_Bases.xml. Stuff like BaseMeleeWeapon, PlantFoodRawBase, quite a lot of stuff actually (it's quite daunting).

Am I correct in assuming that this does not replace the core abstracts, but rather redefines them for future use with the intention of being placed early in the load order? I.e., so any other mods that use these base classes will inherit the changes that Powerless! made instead of from the core game?

Hope that makes sense :)

If this is true, which I'm assuming it is, can anybody think of why this was done in this case? Is there a legitimate reason to not patch these base classes so that even the core stuff takes advantage of it yet mod-added stuff will (seems like an inconsistency IMO)? Or is it perhaps just something that cuproPanda hasn't had time to update to the new XML patching stuff (maybe I'll have to ask him/her if they are around)?

Thanks :D

wwWraith

Quote from: CosmicDan on June 27, 2017, 05:46:32 PM
Now, if only there was a way to do some kind of "prerequisite" XML patching, that'd be awesome (even if it only allows insertions). I just found out that you can inherit from any Def that has a Name attribute set, not just Def's that have abstract flag set! So I was thinking it'd be great if you could, say, have an XML patch run that injects Name="ElectricSmelter" to the base ElectricSmelter def, then in your mod XML you could set Parent="ElectricSmelter" to quickly make a custom smelter - override whatever tags necessary.

I think it could be done by setting Parent as attribute also in the same patch after you injected Name.
Think about it. Think around it. Perhaps you'll get some new good idea even if it would be completely different from my words.

CosmicDan

#4
Quote from: wwWraith on June 27, 2017, 08:04:25 PM
Quote from: CosmicDan on June 27, 2017, 05:46:32 PM
Now, if only there was a way to do some kind of "prerequisite" XML patching, that'd be awesome (even if it only allows insertions). I just found out that you can inherit from any Def that has a Name attribute set, not just Def's that have abstract flag set! So I was thinking it'd be great if you could, say, have an XML patch run that injects Name="ElectricSmelter" to the base ElectricSmelter def, then in your mod XML you could set Parent="ElectricSmelter" to quickly make a custom smelter - override whatever tags necessary.

I think it could be done by setting Parent as attribute also in the same patch after you injected Name.

Hmm, that's not a bad idea. Because after the patching is done, all the XML needs to be "refreshed" anyway. So theoretically that could work, but it will very likely throw config errors due to incomplete XML in the pre-patch phase of mod loading.

I'm just getting into the Harmony library at the moment though (man this is some powerful stuff, and so much easier than Java's ASM library) so I might be able to make a mod that does this anyway. Maybe it's something worthy to go into HugsLib if not the base game...

EDIT: Actually it's not quite that easy, Verse.XmlInheritance is a bit more complicated than I thought...

EDIT2: I could probably hook onto Verse.XmlInheritance#TryRegister postfix but I'm wondering if there's a downside to this (apart from a possible load time increase)...

faltonico

I don't know if this has changed recently, but as far as i know, you couldn't inherit abstracts from core, thus the need to redefine the abstract in the mod (a.k.a copy paste from core).

wwWraith

Quote from: faltonico on July 03, 2017, 10:38:20 PM
I don't know if this has changed recently, but as far as i know, you couldn't inherit abstracts from core, thus the need to redefine the abstract in the mod (a.k.a copy paste from core).

Yes, it has changed, and this thread is here to provide this info for modders.
Think about it. Think around it. Perhaps you'll get some new good idea even if it would be completely different from my words.

faltonico

Quote from: wwWraith on July 04, 2017, 01:48:17 AM
Quote from: faltonico on July 03, 2017, 10:38:20 PM
I don't know if this has changed recently, but as far as i know, you couldn't inherit abstracts from core, thus the need to redefine the abstract in the mod (a.k.a copy paste from core).

Yes, it has changed, and this thread is here to provide this info for modders.
Well that was almost taboo just an alpha ago, (i personally had some trouble with that) and i haven't heard from Tynan, ZorbatHut or any of the older modders here about that new feature, just xpath (or is it that xpath enabled inheriting from core? i am mostly ignorant when it comes to modding or coding).

Thank you for pointing this out!.

The-Eroks

Quote from: CosmicDan on June 27, 2017, 03:14:05 PM
I just started modding the other day (A17b has already been out for some time) and discovered a peculiarity when working with another mod - core def's being re-declared inside the mod itself. Apparently this was how you had to do it before A17 (or possibly older). Oh, the horror! Apparently a lot of modders are not aware that they can directly reference def's from other "packages" now. The benefits of doing so are obvious.

This is maybe my favorite thing about [A17]... so much easier to work with the core assets now  ::)