[PATCH] Stackable auras

[fatherbushido]

Actually, auras are not stackable. But there is some side effects with capture. For example : If one have two macedonians theater (one and one captured), auras don't stack. But if one have one macedonian theater and one spartan theater (captured), auras stack.

Same with temple for example.

[fatherbushido]

As discussed on irc with elexis and sanderd17, we can use other robust keys/identifiers for auras such that auras never stack. We also add an optional flag in auras template which always allow stacking.

[fatherbushido]

rebased on 17810

[fatherbushido]

This sets wonder as stackable.

[fatherbushido]

According to our discussion :​/irclogs, there is a problematic case. If player with civ A have for example a temple with an aura wich modify the same value by adding 5 and a player with civ B have a temple with an aura wich modify the same value by adding 10, what is expected ?

The patch have a good reaction and affects units with only one of this aura (the first which applies). But this situation may not occurs since it doesn't make sense. One solution that mimo's give is the following :

• it would be nice to ensure that all auras have the same value for all civs
• differences would be introduced by civ specific technologies (now that they can apply to auras) possibly autoresearched

So as tech will apply to the player civ, we don't care if it's temple A or B which applies its aura, since the value will be the same.

[fatherbushido]

Sum up.

• what is an Aura ?

An aura is a set of modifications. An aura is a component of an entity and targets some entities.

• Can a target unit be affected by 2 same auras at the same time ?

The postulate is : no.

• How can we check this ?

There is now many auras and the set of their modifications is not one to one associated with their names. So most of the auras have the same name (typically Aura1, Aura2, or sometimes Heal). (I think at first design an aura with a defined name was expected to do a defined set of modifications). So we must define an identifier (a key) that is in one to one correspondence with the set of modifications of the aura. A natural choice is : (name + "/" + mod.value + "/" + (mod.add || 0) + "/" + (mod.multiply || 1)) as it contains the set of modifications !

And if i wan't that an Aura can be applied twice ?

Here we add a stackable flag and use an unique key for each source of the Aura (name + "/" + mod.value + "/" + this.entity)

• So what's the without patch and with the patch problem ?

• situation 1

A temple T1 of civ C1 have an aura who add 1 to Health.Max A temple T2 of civ C2 have an aura who add 1 to Health.Max

• current situation :

If player 1 have temple T1 and capture temple T2, units are affected by both auras (they stack) If player 1 have temple T1 and build antoher temple T1, units are affected by one aura (they don't stack)

• with patch :

auras don't stack (identifier is the same) if we add stackable flag, they stack.

• situation 2

A temple T1 of civ C1 have an aura who add 1 to Health.Max A barrack B1 of civ C1 have an aura who add 2 to Health.Max

• current situation

Auras stacks (template name of the source is part of the actual key)

• with patch

Auras stacks (modifications are different, so the identifiers / the auras are different).

• situation 3

A temple T1 of civ C1 have an aura who add 1 to Health.Max A barrack B1 of civ C1 have an aura who add 1 to Health.Max

• current situation :

If player 1 build temple T1 and barrack B1, auras stack.(template name of the source is part of the actual key) It is perhaps what we can expected but it doesn't match the postulate. Here the auras are the same, so they shouldn't stack.

• with patch :

auras don't stack (identifier/auras are the same). But in this particular case, we can expect that the man who writes the templates wants that they stack. In this case, he needs to add the stackable flag.

• situation 4

A temple T1 of civ C1 have an aura who add 1 to Health.Max A temple T2 of civ C2 have an aura who add 2 to Health.Max

• current situation

Auras stack (because the civ are differents and is in the actual identifier of auras)

• with patch

Auras stack (because the modifications / auras are differents).

Perhaps it's not what the template writer wants. But how can we expect what he wants in this case ? For this case (using specific civ modifications of an aura), the template writer can use an autoresearch tech which affects this aura (meta modifications of auras).

• sum up

The patch has the expected behavior in each case. It fixs the bug and is ok with the postulate : "a same aura don't apply twice" (an aura is a set of modifications of a definite value). If we don't want that, we use the stackable flag. So template's writer can have what he wants.

[elexis]

After a long discussion with leper and sanderd17, we decided that the only solution can be to move all auras from XML to JSON files again.

This way we can use the filename as a unique identifier instead of relying on workarounds and weird assumptions and running into edgecases.

[fatherbushido]

I m working on it.

[Itms]

This will change a lot of things and it would be dangerous to include it in A20.

[fatherbushido]

I moved back xml to json. I modified Auras.js code and reimplement meta modifications of auras by technologies. Actually i have some issue with tooltips and structure tree. Both call GetTemplateDataHelper @ globalscripts/Template.js. As auraDescription and auraName aren't in template anymore, i call this info in the JSON file. It needs to be optimized. But, this globalscripts is used

• by gui/common/tooltips.js
• by gui/structure/tree/helper.js and load.js

which read the file in different path (structure tree forget simulation/data).

see :​irc for ideas.

[sanderd17]

You need watch out, the Engine.ReadJSONFile will return null when the JSON file doesn't exist. It would be best to show at least a warning mentionning the filename instead of just having a null-reference error.

Perhaps the Engine call can be moved into a separate function of the global Templates.js, so it can be called from the simulation and have the same error handling (and later on, the global helper could hold some kind of sensible cache if it's needed, the cache doesn't have to be serialised anyway).

You also need to add the attribute=tokens to the XML, else f.e. the garrison aura of the general hero template won't be inherited by other heroes.

The rest is quite ok, but still has to be tested

For the sake of the diff, it would probably be best to include your json files in a separate zip. Else, working with a whole bunch of new files can become a bit annoying when testing the patch.

[fatherbushido]

Thanks for the comments, the hints and the advices.

I will adress them.

[fatherbushido]

json files folder to put in simulation/data/

[fatherbushido]

test it

[fatherbushido]

Ref : #3830 #3522 #3792.

• moving all to json
• adding requiredTechnology for auras (following first idea of Mimo and improvement of Sanderd17, we can modify an aura by technology using two auras, one of them has a requiredTechnology).

• it lacks only the wall aura tooltip in structure tree

[fatherbushido]

Just look at aura_patch.diff​ (and aura_data.7z). Previous patches are bullcrap.

[sanderd17]

A code review of the latest patch.

The biggest problem is that the tech-enabling of auras doesn't work. The cleaning uses the affected units to re-apply the auras to them. But auras that aren't enabled don't affect any units. So the cleaning has no effect (unless it's a global or range aura). This part will have to be redone, but it can be ripped out for now (for simplicity of the first commit).

Next to that, some small remarks:

• The "aurasTemplate" parameter needs to be documented in the javaDoc above the function (if there's JavaDoc, it should be correct).
• cmpTechnologyTemplateManager should be renamed now, though that's not for this patch (renaming files gets annoying with SVN)
• Lines 597-600 in GuiInterface.js don't have a null-check and a double if with the same arguments.

The rest of the patch looks fine.

[sanderd17]

In 17974:

Cleanup of the Auras code in preparation of stackable auras. Implements auras being enabled by techs. Refs #3792. Patch by fatherbushido

[fatherbushido]

stackable flag for last patch, wonder stackable as example

[sanderd17]

In 17982:

Implement stackable auras + fix an issue with techs, auras and ownership changes. Patch by fatherbushido. Fixes #3792

[elexis]

In 18076:

Revert a copy&paste mistake in r17974 when auras were converted from XML to JSON. Patch by fatherbushido, refs #3792.

[elexis]

In 18188:

Fix typo in r17974. Patch by fatherbushido, refs #3792.

[elexis]

In 18400:

Fix indentation of auras in r17974. Patch by fatherbushido, reported by leper, fixes #4062, refs #3792.