it shouldn't really matter whether they are on foot or ride a horse, chariot, camel, or elephant.
Well it matter for citizen-soldiers and champions.
Maybe they could start at 500 hp and use the same scaling as champions for example.

cf. rP22977

Is that in-game or at the structure tree from the main menu?

Also it would not be shown in structure tree and right clicking to trainable unit. (Side note, I think you need to put that technology somewhere to work if I am not mistaken)

Actually it works:

At first sight it looks good because it removes bunch of duplication, however it hides cost change from templates. Also it would not be shown in structure tree and right clicking to trainable unit. (Side note, I think you need to put that technology somewhere to work if I am not mistaken)

@ValihrAnt please accept if you (still) agree and have verified this patch :)

Successful build - Chance fights ever on the side of the prudent.

Successful build - Chance fights ever on the side of the prudent.

Successful build - Chance fights ever on the side of the prudent.

Thank you!

the chariot currently cannot be trained, and will be up to the balancers to include or not.

D2683

Successful build - Chance fights ever on the side of the prudent.

Successful build - Chance fights ever on the side of the prudent.

Without this patch:

• Conquest games are won by destroying all enemy structures and units
• Conquest Structures games are won by destroying all enemy structures; units are ignored
• Conquest Units games are won by destroying all enemy units; structures are ignored

Expected behaviour with this patch:

• Conquest and Conquest Structures games: foundations are ignored
• Conquest Units and non-Conquest games: unchanged

That should be easy to verify by running some test games.

Do to recent comment https://trac.wildfiregames.com/ticket/3190#comment:13 I suggest @Itms after this is commited to upgrade it straight to recent versions as done https://github.com/s0600204/0ad/tree/fmt .

Successful build - Chance fights ever on the side of the prudent.

Send the stopped message only after invalidating the target.

I agree with moving persians to the persian folders, I guess if they were indeed using more horse archers than chariots then they should use that, I agree about the icon change, the specific name change, and the chariot currently cannot be trained, and will be up to the balancers to include or not.

Strings in tests?

Successful build - Chance fights ever on the side of the prudent.

• Add test for stopping.
• Use proper prepare time when starting to heal.

I've never player Conquest only units or only structures, so no clue what is supposed to happen there.

data passed inside timer are not used so removal is ok and it eliminates object creation.
SelecAnimation takes only 3 parameters DEFINE_INTERFACE_METHOD_3("SelectAnimation", void, ICmpVisual, SelectAnimation, std::string, bool, fixed)

Did not have time to get into it yet, so pointing only style.

Successful build - Chance fights ever on the side of the prudent.

Also remove duplicate/depraced "pack" in SetAnimation.
Missed in rP21359.

Successful build - Chance fights ever on the side of the prudent.

Successful build - Chance fights ever on the side of the prudent.

Complete

Nice catch :)

maur_wall_gate.xml, pers_stable.xml

Thanks. D2222 is a similar patch for the {civ}.json data files.

Thanks for reviewing this, I appreciate it!
As for gates, the <Cost> component determines how expensive it is to repair the structure, and loot is 20% of the cost. The cost is indeed lower than building a long segment and upgrading it to a gate, but I think that's justifiable and the lesser evil.
Likewise, building a village tower and upgrading it to a town tower is more expensive than waiting and building a town tower directly.

Remaining pers_stable

Oops, missed that one, will correct, thanks for pointing that out!

Changes are complete.
Changes are good following similar cleaning patches and keeping CC for json.

Successful build - Chance fights ever on the side of the prudent.

The new patch looks to work fine, just tested it.

No errors, warnings, or unexpected AI behaviour? Could you describe what you tested and how exactly? Also maybe update this patch's summary and test plan.
At the very least one should verify it works as intended on:

• Conquest games
• Conquest Units games
• Conquest Structures games
• non-Conquest games

As I wrote earlier, I expect this to work, but didn't test it myself.

Remaining
pers_stable

Remaining
iber_player_teambonus.json

Removed const and gave the child template parameter a different name so it wont complain in gcc.

Not needed with the differing aproach in D2674.

The new patch looks to work fine, just tested it.

This is a great improvement. When I'm trying to select all my barracks or Civic Centers I don't care from whom I captured something, I care about getting those buildings in the same selection.

Successful build - Chance fights ever on the side of the prudent.

Try the patch as is (arc patch D2674).
The only unrelated change is I added full stops at the end of the comments in the xml file, but that doesn't make any difference.

This looks to be unrelated changes aswell? Should i just test the ConquestCritical -> ConquestCritical+!Foundation part?

Successful build - Chance fights ever on the side of the prudent.

Try this instead, I think it should work, but haven't tested.
@nephele, please take back (commandeer) this patch.

The only other option I see would be to introduce a new token that can be removed without a warning, The commit that introduced the warning fixed a crash when removing the token anyhow.

This is bad. The warning is there for a reason: it is meaningful and helpful. It should be kept, not removed. Instead, find a way to make an exception for foundations.

Successful build - Chance fights ever on the side of the prudent.

I'm not sure how (and if) this inheritance is properly used in the test.
Help is appreciated!

Please see D2678 for the patch to remove the warning.

Build failure - The Moirai have given mortals hearts that can endure.

Successful build - Chance fights ever on the side of the prudent.

Great idea, thanks!

This patch will currently trigger a warning I will adress in my next diff.

It can only be committed if it works without errors, warnings, or problems for the AI (Petra).

Technically isn't a siege tower a "structure"?

It is an object constructed from several parts, so yes, that means it qualifies as structure, and it has a roof and walls, so it qualifies as a building too.
However, in 0 A.D. all siege entities are considered to be units (inherit from template_unit.xml, have the “Unit” class, and can move), not structures (inherit from template_structures.xml, have the “Structure” class, and can't move).
Moreover, having most siege actors in the units folder (e.g. Roman ram) but some in the structures folder (e.g. Persian ram) is both inconsistent and confusing.

Build failure - The Moirai have given mortals hearts that can endure.

Successful build - Chance fights ever on the side of the prudent.

Build failure - The Moirai have given mortals hearts that can endure.

Technically isn't a siege tower a "structure"?

Build failure - The Moirai have given mortals hearts that can endure.

only brit now

Successful build - Chance fights ever on the side of the prudent.