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 - alkor

#1
Quote from: Spdskatr on May 27, 2017, 09:12:12 PM
So... Just putting this out there... If I were to use C# to replace the def after it was all loaded, would this way be the fastest? Compared to PatchOperations, I'm doing a targeted System.Collections.Generic.Dictionary search and then directly editing the fields with the same accuracy as a PatchOperation.
Being somewhat of a necromancer, and you probably got your answer long ago already, for posteriority's sake:

Yes, it would be the fastest approach. You are effectively operating in-memory on native objects, as opposed to in-memory representations of XML documents (which are somewhat more complex than one level deep lists or dictionaries). There is a caveat you have to remember though: patch operations are effectively preprocessing, whereas the objects you work with in C# (unless you hook into the core resolver) are ones that have already been resolved - inherited, overridden, so forth. During a patch, many defs with the same defNames can co-exist in the XML structure (each in a different mod package). After resolution there'll be at most one given def type with a given defName, so naturally its lookup will be faster.

But.

This effectively invalidates load order resolution on the XML level, because when you do that, everyone who wants to tweak your values further (be it in a public mod, or for their own personal use locally), is effectively forced by you to not only load in their modifications after your mod, you are also forcing them to write their own C# assembly and apply their changes there. So not only do they have to at the very least understand C# basics, they need to figure out your code, and then they also need to understand the lifecycle of game objects, hook calling order, and so on. Well, you get the picture :)

Also, this is (theoretically) a double-edged sword. If you wanted to remove a def from the database, it would be faster do to this in a patch than after resolution, because it prevents the removed node(s) from being resolved at all (they don't exist anymore once they arrive at the resolver for processing). If you were to do it programatically in C#, then you would be adding overhead by removing something that went through the whole patching-resolution process. Theoretically.

Practically, it depends on how heavy the mod load is, how many patches are getting applied to the values you're interested in (...). You'd have to benchmark on a case by case basis, which is not feasible. Which in turn means this pretty much falls into the micro-optimizations tantrum, and instead of worrying about performance, you should pick the most apparent, easiest to read solution. I'd argue patches are easy to read, especially for others, and even coding not-so-savvy players. As opposed to digging through, let alone decompiling, your code to find out what you are changing.
#2
Quote from: faltonico on July 20, 2017, 05:04:16 AM
I had forgotten about this question here xD
Thank you for your reply!
Somebody answered that question for me not long ago. Indeed, I don't need to copy the abstracts, but now that it is copied, as long it is the same as in Core, it doesn't do any harm, other than an additional check for me the moment i update the mod to the next alpha. Well at least that is what i understand of it. If another mod wants to change the Core def on purpose, welp....
The thing is - the "welp" part is the important part, and I was arguing that unnecessary redefinitions *are*, well - can be, harmful.

Even in your case, something as seemingly harmless is the cause for this chain of events:
1) Vanilla BuildingBase is defined as a /Defs/ThingDef, yours is defined as a /ThingDefs/ThingDef instead.
2) So, in order to patch BuildingBase, I can't just target /Defs/ThingDef[@Name = "BuildingBase"], because plenty of mods redefine the bases for no reasons, under different locations, with inconsistencies in values (the bigger your mod, the more redefinitions you included, the easier it is to overlook that a vanilla value changed and you didn't reproduce the change - plenty of popular mods out on here which suffer from this).
3) As such, I have to target */ThingDef[@Name = "BuildingBase"]. Which effectively targets not just core - in which case I could skip the glob match because I know the exact path of the def - but all loaded packages. The operation is slower in and of itself.

Now, the person who actually pays for this is the player who has to sit through an extended loading time on each game startup.

You have to remember that a mod can exist as a single entity next to core, but it can also sit next to 100 other mods. And it really isn't that loading 100 mods takes a long time - but the time grows exponentially because many mods follow bad practices, not because the game is poorly coded. The game can't make certain optimization decisions without making some assumptions (considerably) limiting our ability to mod it (with just XML anyways). And it's *great* that it doesn't, speaking as a modder and programmer, but speaking as a player - sometimes I wish it did.

Imagine, as an example, 100 mods redeclare BuildingBase (because it is "harmless").
1) During normal resolution (without or after patching), BuildingBase gets resolved 100 times (objects for the def get created, then * 10-or-so objects for its properties and attributes, then those get read in, then inherited and merged/overridden...), one after another, 100 times. All of which - completely unnecessary.

2) During patching. It becomes even more convoluted. Imagine 100 mods which redeclare BuildingBase and then a handful others simply patch BuildingBase. Now "simply patching" happens 100 times the number of mods which apply a patch, per each patch operation - instead of just once per patching mod, for the core definition they actually intended to patch. Now imagine we want to replace not just one value, but several. This cannot be done in a single operation, so if I wanted to change a single "true" to a "false" somewhere, and then a "1" to a "0.8" somewhere else, this is not 2 operations - it becomes 200.

And step 1), the resolution of those defs, along with its own issues, happens afterwards on top of that.

Bottom line - this happens more often than necessary, consumes time and resources of the player, so I wouldn't call it harmless.

I'm aware that this an unrealistic example, but the same principle applies to every other redeclared def, abstract or not, in every single mod out there. So while it may not exponentially stack up to 100 times for a single def, it'll still happen in a manner of "one redeclaration here, one redeclarion there", and suddenly we're performing thousands more operations than necessary.

Apologies for another lengthy message, especially since my rant isn't really directed at you, but more generally at a lack of good neighbour policies and conventions in RW mods :P I'm aware it's not due to people's bad intentions or not giving a damn, but more often than not just outdated (or lack of) information and understanding.

Quote from: faltonico on July 20, 2017, 05:04:16 AM
Regarding your other question, i really don't know, but i think it is possible =D
In the help forums you can get help if you have an idea:
https://ludeon.com/forums/index.php?board=14.0
I did not really ask a question - but,... appreciated ^^
#3
Quote from: sidfu on July 18, 2017, 12:43:12 AM
your missing

<recipeUsers>
   <li>Human</li>
</recipeUsers>

for recipes

Not really. They're correctly defined in parent defs.
#4
Quote from: faltonico on May 30, 2017, 03:41:42 PM
The basic gist of it is that... i would be overriding a base game abstract if i don't copy it exactly as it is on the core folder (anyone with more experience please correct me if i'm wrong). I don't really know if it is still necessary to copy the abstract into the mod or if there is a better way to do it, but there you have it xD
Use the new version and not the target version change only.

Somewhat belated, but no - you don't have to duplicate the abstract. As a matter of fact by duplicating it in your defs you *are* overriding the core def, even if the values are the same. The same basic resolution rule applies to non-abstract defs. Which is why load order matters and defs with collisions, eg. same type and defName, override each other.

It gets slightly more nuanced when, for example, you add (C#) a BuildingThingDef as a subclass of ThingDef, and then another mod loaded later overrides it as a ThingDef with the same defName. In this (edge) scenario, I believe yours would continue to exist in memory as a separate object in a DefDatabase of BuildingThingDefs, but the def would also exist as a ThingDef with values loaded in later by the other mod. However, it's been a while since I looked at the sauce and I only really glimpsed over it, so I may very well be entirely wrong about how the resolver handles subclassing.

tl;dr:

You don't need to redeclare defs that you are not changing. In fact - unless your mod relies on the given def being *exactly* as it is defined in vanilla at the time you copy the values, you should not redeclare them. If you really, really, really do rely on them, you should name your base def differently than vanilla -- conventionally done by prefixing them -- unless you potentially also want to affect other mods which use that base.

And if you are changing them (either core defs or defs from other mods), this should be done via patch operations. You are not, in this mod, of course - I'm just writing this up in a more general fashion :)



The gritty details:

If you leave out the abstract (BuildingBase in your case), then your ReinforcedPowerConduit will inherit the values of BuildingBase that were set at the time your mod got loaded. Meaning: if another mod that changes the abstract base *by redeclaring it* gets loaded before yours, your Conduit will inherit the changed values of BuildingBase. But if your mod gets loaded first, then it would use the non-changed vanilla values and you got a load order issue on your hands. Usually this isn't really a big deal and just causes inconsistencies in minor values, but the keyword is "usually", and as such it's not good practice. Well, since A17 anyways - before that you could not really avoid it.

Similarly, if you leave the abstract base redefinition in your mod and your mod gets loaded after the one that changes the base via redeclaring it, then your mod will effectively invalidate the other mod's changes and restore vanilla values (the values you redeclared) for all subsequently loaded mods. Unless they themselves redeclare them again, so on, so forth. Load order issue again.

There's a lot of nuances involved and it mostly depends on what the desired outcome is. But if the intention of the other mod is to change vanilla base values so that they get applied to everything inheriting from the base def, then it should do so via a patch (which can also broadly target "misbehaving" mods and apply the changes to them even if they redefine the base).


On a more curious thing (not really related to your question): it's entirely possible, via patching alone, to inject a custom abstract before other core abstract types, and have core abstracts inherit from it (effectively affecting the whole inheritance tree on that object). Heck, you can patch another mod's defs to use your base abstracts instead of its own, regardless of which one gets loaded first, with no issues if you so please.

It is also possible to inject a "Name" attribute into a non-abstract core def in order to then in your mod inherit from it, and only define the values that you want to change/override instead of redeclaring all vanilla values (or patching it).