[BUG] softReset breaking things

[Suor]

In my Standout Enemies mod I patch skills for a necromancer this way in tactical_entity_manager.setupEntity():

// This means necromancer will be able to cast 3 spells each turn
local skills = e.getSkills().getAllSkillsOfType(gt.Const.SkillType.Active);
foreach (s in skills) {
    if (s.m.ID == "actives.raise_undead" || s.m.ID == "actives.possess_undead") {
        s.m.ActionPointCost = 2;
        s.m.FatigueCost = 6;
    }
}

Since Neecromancer has 7 AP this allows how to raise dead twice and possess, etc. However, MSU overwrites this with .softReset() at some point. Sure if I were to patch these skills before they were added then my changes will go to your "base values" and everything will work. However:

1. I only want to patch some necromancers, not all of them.
2. This code perfectly works without MSU.

By exploring the stuff I came out with a simple fix:

e.m.Skills.removeByID("actives.aimed_shot")
local quickShot = e.m.Skills.getSkillByID("actives.quick_shot");
foreach (s in skills) {
    if (s.m.ID == "actives.raise_undead" || s.m.ID == "actives.possess_undead") {
        s.b.ActionPointCost = 2;
        s.b.FatigueCost = 6;
        s.m.ActionPointCost = 2;
        s.m.FatigueCost = 6;
    }
}

And were quite happy with it until I realized that this .b thing is not part of the game and I actually broke my mod for anyone not having MSU installed.

I am not sure how common is this, but I understand cloning everything is actually a way in gamedev to enable easy instance specialization without affecting the world, so it is possible that some other mods or even game itself might broken by this.

I am also not sure how this could be fixed without breaking backwards compatibility too.

[LordMidas]

You can use MSU setBaseValue to set the base value of a field to a certain value. However, base values are only available after the skill has been added to an entity.

Btw, even without MSU, your code is not safe against the skill being removed and re-added to an entity. Let's say if, due to a submod or whatever, the raise_undead skill was removed from the necromancer and then re-added, your changes will be lost. The best way to accomplish what you are trying to accomplish is via adding a new hidden "status_effect" to the desired Necromancers and in the onAfterUpdate of that status effect you find the actives.raise_undead skill and modify its AP cost. When using MSU this modification can be done incrementally e.g. s.m.ActionPointCost -= 1 will work just fine. Because of the softReset. However, without MSU you'd need to set it to a certain value like you are doing in your examples right now.

The skills part of MSU was created to provide actually inter-mod compatible and streamlined ways to achieve what you are trying to achieve. So I'd say use MSU for your mod.

I am not sure how common is this, but I understand cloning everything is actually a way in gamedev to enable easy instance specialization without affecting the world, so it is possible that some other mods or even game itself might broken by this.

I don't quite understand what you mean by this in context of what MSU is trying to achieve here i.e. we need to be able to have a copy of the base values that the skill had when it was added to a character.

[Suor]

Only 1) this mod does not depend on msu 2) these skills are not removed and readed normally 3) it worked perfectly well for years and still works without msu.

Sure some mod might break this, but currently msu is breaking it not some mod.

I understand what msu tries to achieve, however it alters the vanilla behavior in a way that might lead to bugs.

[TaroEld]

Of course there are multiple ways around this - Midas' method, creating a new skill, checking for MSU presence before either changing the values in m or in b. Generally speaking though, I don't think it's possible to make something on the scale of MSU and not break compatibility with some stuff. The question is whether the gains are worth it, and I'd say they are.

[LordMidas]

If our additions break existing vanilla behavior in any way, we manually patch those. E.g. we recently patched out how AdditionalAccuracy from named weapons in vanilla was being affected by our softReset behavior. Previously we also fixed how it was affecting vanilla behavior regarding FatigueOnSkillUse in skills added by orc weapons. While, theoretically speaking, other things in vanilla could break too, but practically none exist right now (in our knowledge). If, and when, they exist and come into our knowledge, we can fix them on a per-problem basis. We ensure that the addition of MSU does not alter existing vanilla behavior for the user in any way.

However, compatibility with mods created before MSU (or mods created without MSU in mind) is not guaranteed. While we would prefer to implement systems that don't break such mods, it may not be possible in every case. Therefore, I'd say update your mod to require MSU, or if you don't want that, then add a check for the presence of MSU and adjust the code accordingly.