Also gives all siege wall segments the same cost-to-health ratio.

This is intimately linked with this patch. Currently wall towers are disproportionally more expensive than other wall segments because they have an <Attack>. If that's removed, then their cost ought to be proportional.

And clean up <BuildingAI> to define it only in structure templates with an <Attack>.

Done in D3016.

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

Right. Done in D3016.

That can take a while before it is reviewed, sadly.

That's understandable, I reallize D2953 is quite a large patch (as is D3000). I do hope it'll make it into A24, though, since the current template_unit_fauna_* situation is rather messy.

• 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’.

• 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

(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.

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

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?

Anything that needs changing?

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

This (D2950) and the other (D2848) 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 D2848, 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 D2848.

gui/credits/texts/art.json?

• redone from scratch

without [...] this patch makes little sense IMHO.

That's a fair point. This patch does mean more duplication. When writing it, I envisioned the following:

• move various values to specific fauna files (this patch)
• move <UnitAI/NaturalBehaviour> and the related <Vision/Range> to specific fauna files
• reduce the number of generic template_unit_fauna* from the current 25 to perhaps three or four that are meaningful
• differentiate animal stats (cf. D497, #3778, #3918), affecting gameplay

The first and second are prerequisites of the third and also make the fourth easier. This patch is already quite large, however, if you believe it's better, I suppose I could merge the first three and do all those things here.

@Freagarach, since this patch now is completely different from the initial version, you should just commandeer.

(svn rename is just an alias to svn mv according to its help function, whereas the rename program can do more.)

Yeah, I figured that out already.

My knowledge is limited, but I would do something along the lines of:

• find files with name matching flora_tree_*
• cut that part from the name
• svn mv the matching files to tree/ with their new name

That's what I want to do, however, there are over 70 such files and I don't really want to svn mv them one by one, so I'm hoping someone knows a clever way to move and rename all of them at once with a single command while preserving their svn file history.

TODO: flora_tree_* → tree/*

The following does what I want:

svn mv flora_tree_* tree/
cd tree/
rename 'flora_tree_' '' flora_tree_*

However, replacing rename with svn rename doesn't work on my end, and without it the svn history is lost. Any suggestions?

• redone from scratch

That varies from case to case. For attack & resistance specifically, I suppose only combat and action (e.g. healer, gatherer) stats.

Yes, the extra population is removed from the brit, gaul, and iber structures; brit and gaul structures already are built more quickly, whereas iber fortress and tower have significantly more health; see also the D2841 discussion.

In my opinion the tooltip that opens when right-clicking on a entity icon should list everything (as is already the case), but the smaller ones that pop up when hovering over an icon ought to display only the most important and relevant information, hence this patch.

• replace armor_ship_* technologies with different ship_* technologies, as discussed with @borg-
• delete special_exploration.json and vision_sibylline.json, per @borg-

Perhaps this could be moved to the tooltip of the entity icon?

Which tooltip do you mean exactly?
Loot and resource trickle are already displayed in the one that pops up when hovering over an icon in the structure tree, as well as in the one that opens when right-clicking on an entity icon.

A fireship is “a ship loaded with burning material and explosives and set adrift to ignite and blow up an enemy's ships”. It should not be confused with a warship armed with flamethrowers and Greek fire, as used by the Byzantines in the 7th C AD. Age of Empires II erroneously calls the latter a “Fire Ship”, while their “Demolition Ship” actually resembles a real fireship.

• rebased

For simplicity and consistency it might be best if the name node takes exactly the name of its parent, i.e. Capture, Melee, Ranged, or Slaughter. If and when multiple attacks are enabled and can have any name, then the translatable string can be renamed too.

Correctize

The correct verb is to correct.

You are right that elaborate trading balance doesn't belong to this patch but could be aimed for in another. I like the garrisoning idea because in reduces ship crowding and makes individual ships more meaningful/interesting as targets / for protection.

Personally I like the idea of traders boarding merchant ships too, however, unless <GarrisonGainMultiplier> is greater than 1 (currently it's 0.2), constructing more merchantmen is always more profitable than loading them with traders.

That being said, I wonder if they should be slightly nerfed in consequence, or not?

Maybe lower <DefaultArrowCount> or <MaxArrowCount>?

The primary concern I have is the names are essentially arbitrary: functionally, it's irrelevant whether something is labelled axe, spear, or sword. The names are based on visuals (the props used by the actors' animations). Moreover, one can't really expect template editors to view the attack animations of each individual entity, nor artists to check and correct the relevant simulation templates whenever they make art changes.

The garrison flag is not terribly important and could be kept, if you think that's better.
However, I believe the purpose of the garrison flag is to indicate an entity has a garrison inside. Actors of structures with visible garrison slots (e.g. long wall segments) don't have garrison flags, presumably because, well, their garrison is visible. Since this patch makes the garrsion visible, there no longer is a need for the garrison flag, and it's consistent to remove them.
Then there is also the fact that the emblem on the flag is the actor's civilization, not the owner's, but that's a separate issue.

@Stan opinion on the flag removal? (I like the flags, but I'm no artist.)

(Other siege wall actors don't have garrison flags either.)

currently 1.25:1

Currently merchant ships cost 100 metal and 0 wood. This patch proposes to keep the metal cost unchanged, adds a 200 wood cost, and increases health, resistance, and trade gain in compensation. That said, I don't feel strongly about particular values, and agree with you traders and merchant ships should have meaningful costs, since they basically generate free resources.

And, as mimo and I have tried to address in #3428 some years ago, garrisoning traders should be buffed (if this mechanic should actually be seen in game - right now it seems incredibly wasteful resource-wise and inefficient micro-wise).

Yes, I agree garrisoning traders in merchant ships is seldom worth it. However, #3428 proposes changing the mechanic, which I'm not sure is necessary, merely changing the current values could work too. But perhaps it should be abandoned altogether, constructing more merchant ships is better? It might be out of scope of this particular patch, though.

Yes, the

unlock_shared_dropsites.json
unlock_shared_los.json
unlock_spies.json

technologies are conceptually related, each of them enables a mechanic, rather than changing a value, like most technologies. However, I fail to see the need to enforce a particular order. In my opinion players ought to have the flexibility to pick and choose which ones they want to use, and ignore the others.
Moreover, there is a “Disable Spies” match setting and a mod that enables shared vision from start. Ideally all three ought to be treated similarly: { shared vision , shared dropsites , espionage } × { from game start , with technology , never }, but that's beyond the scope of this patch.

(I assume specific templates are renamed in another diff?)

Yes, this is only cleaning up the lines inside; file moves will be done in a separate patch, to keep things reviewable. Same for D2990 and D2953.

• clean up <BuildingAI> to define it only in structure templates with an <Attack>, per @Freagarach

Thank you!
Folders for other resource subtypes could make sense too, though the gaia/* files could use some clean-up first, hence D2953, D2990, D2991. Interested?

loose

Is this some kind of sociolect? I'm asking because I've also observed other team members write ‘loose’ (i.e. set free, release) instead of ‘lose’ (i.e. be deprived of, cease to have) more than once.

• rebased
• since the gui now supports more than 24 icons (rP24028), this patch is now committable

Since the alternative (D2875) has been committed (rP24028), this one (D2806) is no longer necessary.

Thank you, @Angen, for committing this! I'm sure @wowgetoffyourcellphone, @borg-, and others appreciate it too.

• fix generic name and player colour band conflict spotted by @Angen (there was indeed a 2 pixel overlap)

action buttons in midle panel could be moved a bit lower,

See the summary:

Command icons (middle panel) have their centre vertically aligned with the bottom row of the left and right selection panels.

In a previous version I put them at the bottom, but it turned out it looks better if their centres are vertically aligned.

Also player/civilisation bar is too close to entity name in brackets, for some entities it overlaps a bit.

Which ones?

• address player name colour mistake spotted by @Angen:

• correct tooltip

• change population cap from flat +50 to relative +20%, per @badosu and @borg-

• include special_hellenistic_metropolis.json
• include maur_tower_double.xml

Land traders cost (80) metal too, and are much weaker and far less profitable (see previous post).

That means you can't build anymore units if you only have a barracks left ?

Barracks didn't add to the population limit anyway. Besides, you'll quickly lose due to territory decay.

The consensus is clearly against.

Why not simply set a translate() in the gui/common/tooltips.js, as used to be the case, and is also done for resistance (e.g. line 198)? There are not that many possibilities (Capture, Melee, Ranged). Or just take and translate the node itself?
Introducing a new node means changing a lot of templates, and more importantly, the names are arbitrary. A tiger that mauls a target uses both claws and teeth, and jumps. Male elephants figthing each other lock their tusks and push forehead to forehead, to force the other to the ground; trained war elephants are able to strike humans with their trunks, stab or tear with their tusks, or crush heads under their feet. For ranged attacks, should the name be the weapon or the projectile (bow vs arrow, crossbow vs bolt, sling vs stone)? And if a certain name seems appropiate for a template (e.g. “Sword” for template_unit_infantry_swordsman.xml), it is not necessarily correct for all its children (e.g. kush_infantry_clubman_b.xml). Naming a <Melee> attack “Knife” but a <Slaughter> attack “Slaughter” seems inconsistent too. And so forth.

Thank you for the review, @bb!
Are you willing to do D2247, D2568, D2623, D2926, and D2985 too? Those are much smaller gui patches.

Cartage tech still bad. Vision is a practically useless attribute, they were excellent marine traders, perhaps a trade bonus or movement speed bonus for merchant ships?

Carthage already has quite a few trade-related (free) civ bonuses:

• Docks cost −25% wood.
• Merchant ships +25% trade gain.
• Docks and markets +0.1 [i.e. 10%] international trade bonus.
• Team bonus: allied docks and markets +0.1 [i.e. 10%] international trade bonus.

Adding yet another cart trade bonus in the special technology is probably not a good idea.
If vision is deemed “practically useless”, then I'd suggest deleting the carthaginians/special_exploration.xml and romans/vision_sibylline.json technologies.

I don't think it's necessary three equal health techs.

Good point! How about the following?

1. Fishing Boats +30% fish gather rate. (unchanged; village phase)
2. Fishing Boats +20 food capacity. (unchanged; town phase)
3. Ships −20% construction time. (city phase)
4. Ships +25% health. (city phase)
5. Ships +10% movement speed. (city phase)

Any suggestions as for appropiate technology costs?

• The change is correct.
• The proposed bireme in village phase, trireme in town phase, and quinquereme in city phase is sensible.
• It does make early gameplay more interesting.
• Especially useful for e.g. the “Migration” random map.
• The only flaw is not all civs have “biremes”; however, that's not a serious objection; not all have archers or slingers either.

• tweak costs

• rebased

• return to initial version (sorry @elexis), because it works for all resolutions, looks good, is simplest, and the dynamic alternative (11160) now has issues:
  

• move splashDetails function up, per @bb
• introduced var indent = " "; because I got fed up with counting spaces

• purge unused ForestPlant class