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?

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

This is complete and correct.

Forgot,,,

One could use a technology to swap the house with the apartment in the build list? Meaning the player would have houses and can build apartments thereafter.
E.g. the player builds one house, researches the technology and can only build apartments next.

Component.

To me this adds merely a lot of duplication actually. While I am not opposed to moving values to specific templates, without the number of template_unit_fauna_* files can be greatly reduced, keeping only those that are meaningful (e.g. elephant, whale). this patch makes little sense IMHO.
E.g. there are three bears, nicely inheriting from a *bear template, which now all three have the exact same values added. I know that in reality those three bears are vastly different and should have different stats, but until they do I do no think it is necessary to duplicate those values.

In D1009#131906, @Nescio wrote:

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.

I would be interested in that as well. In the meantime one could write a bash script like used for the civ-moving. (Notice the first part of each sentence in my itemisation was a command one could use :) )

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

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

Agreed, but then there is the question what is ,,only the most important and relevant information'' ;)

See IRC on acceptance-day.

In D2985#131860, @Nescio wrote:

in the one that opens when right-clicking on an entity icon.

Hmm, maybe that is indeed enough,,,

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

@Stan did the discussion help you any further?

In D2477#103805, @Nescio wrote:

It isn't pretty, but it works.

Since a key of 0 A.D. is prettiness, I don't think this should be committed.

In D2802#119810, @borg- wrote:

As far as I know about them, they were Greek and not Iberian ships, but I must be mistaken. It would be interesting to have a historical expert here.

@genava55

In D2507#123985, @badosu wrote:

This would be ideal, unfortunately I don't have the skills to be able to figure out if this is a meta changing patch with a few games.

(Better not commit this too late in the dev-cycle then.)

Changed some JSDOC.

In D2161#131391, @Angen wrote:

Spreading this issue to melee units is not happy solution. Also I am not sure if this is actually needed for melee. Or target is 2 meters from me and I can reach in any direction, does not matter if target stays a bit higher than me or below or I cant reach it because my weapon is too short.

Notice melee units don't use parabolic range, so the only described case that would be true is that the range query would return entities which are actually out of range.

In D2993#131489, @Nescio wrote:

it's consistent to remove them.

True that.

The main reason for this differentiation above just Melee and Ranged is secondary attacks (which _will_ come and thus should be counted with). Then one can have a longer ranged melee attack, e.g. using a sarissa and one for close combat, e.g. a sword and Melee wouldn't suffice anymore.
We may just use AttackName and use the weapon there when possible (i.e. the above mentioned case) and revert to Melee and Ranged when it is either not possible (e.g. animals) or undesirable (e.g. structures).

Works as advertised.

@elexis provided the hint why the line is changed: if (template.wallSet) will always be false and thus it is not necessary to go through that?

Wonder why rP20046 changed this like this

Good question.

Rebased.

Thanks for the review @Angen :)

Noted issue is gone now :)
(Although I cannot reproduce it even with the former patch.)

Properly internationalise tooltips. (The rest accepts review.)

In D2983#131611, @vladislavbelov wrote:

Have you compiled the game?

Yes :)

Changed some comments.

Okay, forgot to mention it in the update message, but I've moved the immobility and garrisoned setting now to the "correct" locations. However, they are not correct here for turrets, since those change states, trigger leave and thus break the whole idea. (See D2379.)

Thanks for the review @bb!

I can't review since I wrote part of the code myself.

Revert the move in gather.

I'm not particularly fond of the the integer wait time, but I failed to device a better way without major changes (i.e. using a range query and just setting a single timer to go to the next waypoint).

In D2913#131562, @wraitii wrote:

Possibly final cleanup.

Did you try this in-game?

It looks much better with only one second waiting time :)

Idea and code is correct, adds ~50-150 microseconds per arrow.

• Build two sentry towers.
• Upgrade the first to a defence tower.
• Select both towers.
• Try to research sentries.
• Notice the message that we can't start production.

• Forest plant: Indeed unused, can be added later if deemed a necessity.
• Effectively nothing has changed. Complete.
• Change is nice, it clears it up nicely.

It is good read map data correctly. Stupid mapmakers should get stupid results.
Complete.

Or make it useful by using sim-information in the tooltip? Although I guess PetraAI would break if one changes classes in-game.

gui/gamesetup/Pages/GameSetupPage/GameSettings/Single/Dropdowns

• MapSize.js
• WorldPopulationCap.js

If you wield a sarissa it does matter whether your target is at exactly six meters away horizontally or six meters horizontally and two meters up (e.g. on top of an elephant).

These are attack effects, the effects an attack can have (attack types are melee and ranged etc. currently).

• Ditch temporary array
• Don't hardcode attack sound.
• Return {} in tests.

Before:

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

• Checkrefs says it is complete.
• Playtesting reveals no errors.
• UnitsDemo loads fine.
• Change is for the better.

Thanks for the review @Angen and @bb :)

Thanks for the review @Angen (and @bb) :)

• Remove redundant typeIcon in tooltip.
• Remove tooltip from attackAndResistanceStats.
• Show before health in reference page.

Point autogarrisoning to GARRISONING-state.

@Angen, I'm keeping JSDOC anyway ;)

• Code looks good.
• PetraAI vs PetraAI matches give no oddities.
• Dangerous animals don't error out.
• Entities cheer when won a fight instead of after promoting, which is a big improvement!

Could you elaborate on why this ought to be an improvement? To me sharing dropsites follows naturally from sharing information.

(Also maybe add a test for this behaviour?)

Break your head around whether it is actually translated, or hope the best

Or verify experimentally by manually inserting an entry in a .po file ;) (Which I did, and it gets translated properly.)