Aside from this I got idea how this could be unlimited number of players.
You would need to store inside PlayerData an array of m_data and work with the value based on index you get.
Lets say you store m_data as u32. So if you get index 0 - 16 you get first item from array. If you get index from 17 - 32 you get second item from array and substract 16 from index.

@Feldfeld anything against? If yes do we agree that camel traders need to have the same footprint, one or another?

some more comments

Fixed in rP23560

please split footprint changes from name changes
else I agree, footprints should not be smaller than entity

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.

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)

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.

Complete

Nice catch :)

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

Remaining
pers_stable

Remaining
iber_player_teambonus.json

I wanted to avoid copying the same function to another two classes.
That line was not intentional. I planed to remove it in next update or commit.

Following functions already checked if unitmotion exists.

moving out of queue

remove comments

Petra does not use Chasing string in her codebase -> ok.

@Feldfeld whats your opinion ?

Dunno if we need it in vanilla. It rewards players to not use their units directly?

Only citizen soldiers are able to promote so having them garrisoned for most time will hurt economy of the player. And experience gain is not so fast as in combat.

[16:18] <Freagarach> One could perhaps check the timer every time the rate is computed?
[16:19] <Stan`> could be delete this.trickleTimer
[16:20] <Freagarach> That too, and var -> let ;)

(http://irclogs.wildfiregames.com/2020-03/2020-03-21-QuakeNet-%230ad-dev.log)

@Feldfeld any objections ?

the former will stop in its tracks

Need to fix tests.

fix linter comments

Needs rework,
caption change event is triggering rebuild, so solution would be possibly override.

Comparison to number is done correctly.

cancel CHASING timer

warnings

What could be done is to have %(statusName)s: %(tooltip)s, %(effects)s, %(interval)s, %(duration)s %(stackability)s defined in template.
But duration and interval can be changed through aura or technology, what makes 4 options defined in template and 4 ifs in code what will make it easier to read.
Downside is 4 strings for every template.

better approach

nothing braking i can find just by reading the code

http://irclogs.wildfiregames.com/2020-03/2020-03-06-QuakeNet-%230ad-dev.log

16:02 < elexis> I dont know why forEach was used there other than consistency, with the function above, perhaps there is a reason, but if there is, its not clear to me
16:03 < elexis> otherwise I dont see any syntax error
16:03 < elexis> no objections that is
16:04 < elexis> (performance doesnt matter here since its only one call per click, but the difference might be relevant in areas that are called per entity per turn)
16:06 < elexis> (for (let cmpUnitAI of GetFormationUnitAIs(data.entities, player)) would be the alternative)

mainly for that, whole file is using .forEach with GetFormationUnitAIs

Minimal code.
Starting entities are garrisoned correctly.
Saving map in atlas again does not rewrite Garrison element.
There is no other map to be updated.
Single way how to define garrisoned entities in map file.

desired effect by "multiply": 0 sounds like "replace": 0

It would be nice to solve the whole ticket here. Are you up to that task?

add .
reformulate comment
use new icon

+1

It is correct to allow templates decide if friendly fire is enabled instead hardcoding it.
Changes to code are complete and correct.
All required templates have been updated.
Test are passing.

can you request review ?

windows build was fixed