Skip to content
This repository has been archived by the owner on Aug 4, 2023. It is now read-only.

AddIfNotThere ModOp #28

Open
taubenangriff opened this issue Aug 3, 2019 · 7 comments
Open

AddIfNotThere ModOp #28

taubenangriff opened this issue Aug 3, 2019 · 7 comments
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@taubenangriff
Copy link
Contributor

should exactly work like an add, but with a precheck if the added content is already there on the same level, in which case it does not add it.

Useful for making mods modular with building menu.

Example:
Mod A and Mod B should insert Buildings into the modded category 9000.
For no dependency between both mods the building category 9000 needs to be created and set up, and also added to the construction menu in both.
Problem is, if I add the category to the construction menu in both mods, it appears twice. But I only want to add it once even though both mods add buildings to the category 9000.

@xforce xforce added enhancement New feature or request good first issue Good for newcomers labels Mar 26, 2020
@taubenangriff
Copy link
Contributor Author

taubenangriff commented Aug 4, 2020

This old issue has become relevant again today - as we found out, mods from different creators are incompatible because of doubled nodes which Anno expects to be unique.

The problem is adding the same unique node with different Child Nodes from two mods. In our case <BuildingReplacements> is non-existent in the original assets and both mods do the following ModOp:

<ModOp Type = "add" GUID = '167' Path = "/Values/ConstructionAi">
    <BuildingReplacements>
        <Item />
        <Item />
    </BuildingReplacements>
</ModOp>

Which in the end results in:

<ConstructionAi>
    <BuildingReplacements>
        <Item />
        <Item />
    </BuildingReplacements>
    <BuildingReplacements>
        <Item />
        <Item />
    </BuildingReplacements>
</ConstructionAi>

Anno always uses the last one, which means only BuildingReplacements from the mod that was loaded last will be applied ingame, which can result in serious trouble with the Ai.
There is no temporary workaround for this except for an external mod which just adds this node, which is kinda iffy and not very user-friendly (i.e. people could deactivate it because they do not know what this mod does).

What I would prefer for full manual control is having a CheckPath seperate from Path. If CheckPath returns null, the content of the ModOp gets added in the specified Path. That way, all mods adding BuildingReplacements could use this structure:

<ModOp Type = "addIfNotThere" GUID = '167' CheckPath = "/Values/ConstructionAi/BuildingReplacements" Path = "/Values/ConstructionAi">
    <BuildingReplacements />
</ModOp>

<ModOp Type = "add" GUID = '167' Path = "/Values/ConstructionAi/BuildingReplacements">
     <Item />
     <Item />
</ModOp>

Optional: We could have an Argument to negate CheckPath Results to not add stuff if CheckPath returns zero.

@xforce
Copy link
Owner

xforce commented Aug 4, 2020

Yea, I am currently looking into adding this.
The extra path to check was also something I was thinking about, because otherwise trying to determine that just by the child nodes would be buggy mess I fear.

As for having certain setup steps separate, I am currently also investigating a mod description format where you can specify dependencies, that will also ensure a certain load order without having to rely on the name with the alphabetical loading like it is right now (that should have been something from the start, but oh well)

@taubenangriff
Copy link
Contributor Author

https://github.com/taubenangriff/Modinfo

I am planning to include ModDependencies in the modinfo files that will be added in the next spice it up update, in case you want to use those.

@Serpens66
Copy link

ah, this request is similiar to mine, while my suggested "merge_add" is superior, if it is possible to implement:
#65

@Sealring
Copy link

Sealring commented Sep 8, 2021

Do you guys think thank having some separate condition attrs or nodes could be more useful/flexible, as I proposed in discord?
CheckPath is not a bad idea, but it's a bit ambiguous. By having proper condition properties we could achieve a better control for other operations, not just "addIfNotThere":

IfPathExists/ IfPathNotExists/ IfGuidExists/ IfGuidNotExists

<ModOp Type="add" GUID='1337' Path="/Values/Standard/Name" IfNotExistsGuid='0000'/>

In particular, IfGuidExists could really help to interface better with other mods since you could make the choice of whether to replace or add something not just based on the existence of the subelements of the current node.

Alternatively, maybe we could introduce a more extensible system with ModCond node for conditional execution?

<ModCond>
    <If><PathExists/></If>
    <Then>
           <ModOp ... />
    </Then>
    <Else>
           <ModOp ... />
    </Else>
</ModCond>

This also could make it very interoperable with this mod config issue, if we had for example some kind of setting value check for conditional operator execution.

Edit: Looking at the structure of the XmlOperations class, maybe it could be easier to extend the current Ops with a new type like Type='If' instead of bifurcating ModCond from ModOp

<ModOp Type="if">
    <If><PathExists/></If>
    <Then>
           <ModOp ... />
    </Then>
    <Else>
           <ModOp ... />
    </Else>
</ModOp >

@jakobharder
Copy link
Contributor

jakobharder commented Nov 20, 2021

I face this issue with build menus. I add buildings to the menu and place a fallback add in case there's a mod messing up the build menu (e.g. replacing townhall with a townhall menu).

I can solve the issue with "not()" in XPath, but it has its flaws:

Option 1: Two ModOps, either of them will work. The other produces a warning.

  <ModOp Type="addNextSibling" GUID='500944,25000191' Path="/Values/ConstructionCategory/BuildingList/Item[Building='100415']">
    <Item>
      <Building>1500010021</Building>
    </Item>
  </ModOp>
  <ModOp Type="addNextSibling" GUID='500944,25000191' Path="/Values/ConstructionCategory/BuildingList[not(Item[Building='1500010021'])]/Item[last()]">
    <Item>
      <Building>1500010021</Building>
    </Item>
  </ModOp>

Option 2: Use XPath or-statement. But that doesn't work with speculative lookup, hence takes 150ms+ per ModOp.

  <ModOp Type="addNextSibling" Path="//Asset[Values/Standard/GUID='500944' or Values/Standard/GUID='25000191']/Values/ConstructionCategory/BuildingList/Item[Building='100415'] | //Asset[Values/Standard/GUID='500944' or Values/Standard/GUID='25000191']/Values/ConstructionCategory/BuildingList[not(Item[Building='100415'])]/Item[last()]">
    <Item>
      <Building>1500010021</Building>
    </Item>
  </ModOp>

I'd be super happy if we can suppress warnings for ModOps where we expect a node not to be found (and maybe also suppressing non-speculative fallback in that case). It would also cover a lot of addIfNotThere use cases.

Nevertheless, allowing or-statements for speculative look up would be also great.

@jakobharder
Copy link
Contributor

Add with two fallbacks, fast GUID lookup and no warnings (1st fallback is before, 2nd fallback is at the end)!

<ModOp Type="addNextSibling" GUID='500944' Path="/Values/ConstructionCategory/BuildingList/Item[Building='1010343' or Building='1500010186' or (not(../Item/Building='1010343') and not(../Item/Building='1500010186'))][last()]">
  <Item>
    <Building>1500010212</Building>
  </Item>
</ModOp>

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

5 participants