Could you please split these off?

[EDIT] Also gives all siege wall segments the same cost-to-health ratio.
And clean up <BuildingAI> to define it only in structure templates with an <Attack>.

At least the latter, for I can easily review and commit that whilst this patch and the former need some opionion about balancing.

Structures: Same as BuildingAI in another patch, the sound is only necessary when a structure can attack, right?

There is a problem in this patch, mostly noticeable in MP: When changing the values as a player, the values will first change by the GUI handling. Then at the next simUpdate (new turn), the values will move back to the old ones and a few turns later, the values will move back to what they should be (since the command is only executed a few turns later). This is obviously wrong. To counter this, we can add a if (g_Observer) check. Which would be correct in the sense that observers can't change the simstate, and should just view what the current sim values are. However I also considered #2459, if the values change otherwise than playerdoing, we will be in trouble. I presume fixing this there would need some event-based gui handling. We could argue this out of scope.

Multiplayer is weird now.

• Gave template_structure_defensive_tower_bolt.xml the same sounds as template_unit_siege_boltshooter.xml.
• Removed attack sounds from maur_support_elephant.xml, spotted by @Freagarach.

You can include the few Gaia/ entries as well (donkey and horse)?

• Not changed to avoid conflict with D2953.

Below the translation field, there is a "more info" box displayed. The third item says "Context", the context string added here will be displayed there.

If I understand correctly, strings are translated on transifex, not occurrences, i.e. if a particular string is translated, all occurrences that are exactly the same have the same translation, which is typically fine for full sentences or longer texts. Although context is displayed on transifex, it's essentially meta-data, and since it does not alter the string, I fear both "Sword" (with double-edged context) and "Sword" (with single-edged context) will have the same translation. That said, I don't know the inner workings of transifex and I didn't test for this particular case.

And then it is replaced by the correct term, when wanted. If I counted correctly there are 20 different sword wielding units, of which 4 use a single-edged one. I believe avoiding duplication legitimizes settting the double-edged variant in the parent.

Did you check champions too? Those usually don't have swordsman in their file name. Anyway, if single-edged blades are indeed a small minority, I agree it's better to set Sword in the generic template parent and a different string in the relevant children. Perhaps ‘Saber’? It's a bit anachronistic, but probably clearer than e.g. ‘Backsword’.

You can include the few Gaia/ entries as well (donkey and horse)?
maur_support_elephant also has an attack node.
Idem for the infant elephant and domestic cattle.

-e 's,gaia/flora_bush_tropic,tree/bush_tropic,g' \

But since it is unused it doesn't really matter ;)

This is looking better and better :) Just one thought, for entities like animals, which have no clear weapon they use, it may be clearer to just use "melee"? E.g. the elephant uses both trunk and tusks.

Works as advertised.
Trading goods may be affected by triggers and will be updated accordingly now.

In D2995#132090, @Nescio wrote:

This is actually easy to fix, we can add a context tag in the template, which will be extracted already. So I just added the code to pipe the context through and use it for translations. In example: one can put <AttackName context="Double-edged">Sword</AttackName> in the template.

How does this work, i.e. what's the result on transifex?

Below the translation field, there is a "more info" box displayed. The third item says "Context", the context string added here will be displayed there.

On this specific topic: I also added context for all sword wielding units.

No you didn't: you set it for the generic template_unit_*_swordsman.xml shared parents, which is wrong for about half their children.

And then it is replaced by the correct term, when wanted. If I counted correctly there are 20 different sword wielding units, of which 4 use a single-edged one. I believe avoiding duplication legitimizes settting the double-edged variant in the parent.

Then another concern: if I understand correctly, the attack name is taken from the template and translated separately. However, in some languages e.g. “spear” and “attack” will give a different translation than “spear-attack”.

Again context, I will add a context for all the attackNames now (a general one in messages.json), which will be visible for all in transifex for all strings which don't have a context defined in the template.

Duplicate the parabolic code from unitai

• renamed remaining flora_tree_* files with script provided by @Freagarach
• corrected sed command
• validated afterwards

• rebased

• remove walk speed multiplier from template_unit_fauna.xml

• rebased and updated
• moved <UnitAI/NaturalBehaviour> to specific gaia/fauna_* templates and deprecated 20 generic template_unit_fauna_* templates, per @Freagarach
• renamed template_unit_fauna.xml to template_unit_animal.xml

Rebased.

This change seems good, but needs a rebase.

Works as advertised.
Please fix linter (when committing).

Try this:

• Checkrefs passes.
• Playtesting reveals no errors.
• Change is good.

Thanks for the review @bb!

Inlines.

@vladislavbelov

(For the record: D2572.)

The point is the values and names are defined in the generic template_* parent templates, but the props and animations in the actor files, which are set in the specific units/* templates. If you use the actor viewer in Atlas, you'll see e.g. cart_cavalry_swordsman_gaul_b.xml uses a double-edged sword, but cart_cavalry_swordsman_iber_b.xml a single-edged sword.

This is actually easy to fix, we can add a context tag in the template, which will be extracted already. So I just added the code to pipe the context through and use it for translations. In example: one can put <AttackName context="Double-edged">Sword</AttackName> in the template.

How does this work, i.e. what's the result on transifex?

On this specific topic: I also added context for all sword wielding units.

No you didn't: you set it for the generic template_unit_*_swordsman.xml shared parents, which is wrong for about half their children.

Proofreading other fauna names does not result in more cases.

Actually there are at least two more: deer and zebra. I didn't include them because it's unclear which species they are. When correcting the specific names (D1523/rP22014) I made educated guesses, but the actors are not that clear and it could be argued they're meant to represent a different species.

There are also XML files elsewhere; the vast majority is in the art/ folder.

Correctness forces the accept

Apparently only the mime-types where changed, while the line-endings where already correct.

In D2810#132054, @bb wrote:

Observers can already see the whole map, slightly bad that observers don't have the retroMe cheat anymore, but well, meh.

D1442 for the record

Note that the reveal map cheat does not work anymore for observers after this. Is that bad?

Observers can already see the whole map, slightly bad that observers don't have the retroMe cheat anymore, but well, meh.

Then there is the problem of translating. For instance, whilst ‘sword’ can refer to both single- and double-edged blades in English, this is not true for all languages. In Chinese, jiàn is the word for double-edged swords and dāo the term for single-edged swords (and knives). Thus in those languages “sword” would be wrong in about half of the cases.

This is actually easy to fix, we can add a context tag in the template, which will be extracted already. So I just added the code to pipe the context through and use it for translations. In example: one can put <AttackName context="Double-edged">Sword</AttackName> in the template. On this specific topic: I also added context for all sword wielding units.

Back to attackName
add context support
add context for swords
improve some names

Digging back to D555, this code must be ccmpSelectable copy-pasta

The barracks already has quite a few props to make it interesting. Besides, the Persian arsenal, range, stable don't have it either.

It's important in the sense that it removes decoration as such makes buildings more historically accurate, but also more bland and less interesting to look at. So when I need to edit buildings I have to take it into account.

Yes, but that's a minor point. Besides, those don't belong on generic structures either: reliefs of Persians in ceremonial robes are found on several of the monumental structures of the Persepolis palace terrace, as well as on the tomb of Darius and some other rock inscriptions (i.e. all Wonder-quality), but to the best of my knowledge not on any of the buildings found in the lower city of Persepolis or elsewhere.

To be able to use the lower floor (also for siege wall gate) the actor needs to be edited to raise the platform, because currently the garrison heads go through the roof: https://wildfiregames.com/forum/topic/22779-0abc-mod/page/17/?tab=comments#comment-391719

Good points! I wrote and ran the script, but didn't validate it afterwards to verify the replacements were correct; I should have.
Anyway, I've now updated the sed script. Could you try and check again?

Your script needs some fixes.
E.g.

-e 's,geology_metal_alpine,alpine_small,g' \
-e 's,geology_metal_alpine_slabs,alpine_large,g' \

means that first geology_metal_alpine_slabs is replaced with alpine_small_slabs.
Also the gaia-folder is missing in the replacement strings.

I guess we need only some opinions from the balancing department.
Unless this:

In D2993#131488, @Stan wrote:

The top of the tower looks very crowded though.

is sufficient reason to hold this off?

That also removes the two characters from the walls

Anything that needs changing?

See also https://wildfiregames.com/forum/topic/24495-celtic-reference/page/8/

This (D2950) and the other (D2841) are two separate patches, with different aims, that can be reviewed independently. If and when one is committed, the other will have to be rebased, though that's unimportant for the discussion here.
This patch initially kept the population for brit and gaul structures, but that exception was later removed, per @borg-.
As for D2841, the purpose of which is to differentiate brit and gaul, it's unclear if and in what form it will end up. There was some more discussion on the forums last month, which indicate that perhaps the brit might keep the structure build time discount and the gauls would get one or more new bonuses (still to be determined). Anyway, that's outside the scope of this patch, and belongs in D2841.

gui/credits/texts/art.json?

@gameboy No sir, this patch is not ready. In the title it says [WIP] -- meaning that it is a work in progress. Moreover the status of the patch is Changes Planned meaning there is stuff that needs to be done but time (and motivation) to be found.