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

In D2956#131486, @Nescio wrote:

currently 1.25:1

Currently merchant ships cost 100 metal and 0 wood.

Idea and code is correct, adds ~50-150 microseconds per fire arrows function call.

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

check for cmpplayer and add test

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

MapSize.js
WorldPopulationCap.js

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

a = class a { constructor() {console.log('constructor')} }
a = new a();
a = new a();

output

constructor
error: Uncaught TypeError: a is not a constructor

I missed in code where are m_EnabledRenderSubmit or m_EnabledInterpolate used?
Looks like they are here just for switch, they can be merged as always or are both enabled or both disabled.
Also that two ifs can be merged.

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

Is it attack effect or attack type ?

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

Before:

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.

(That's a tower)

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

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

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

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

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?

My comment above remains valid, my status is "waiting on @Imarok to update the diff" ;)

Make formations walk again.

@wraitii @Freagarach can you have another look at this ?

Thanks for the review @Angen and @bb :)

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

(The older way was made irrelevant by rP22313 which moved the "Move" call to states instead of orders.)

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

I think your previous diff had things in the wrong spot, and this one makes it obvious which.
Given the scope of things, I think you can get away with just "approaching" and "garrisoned", but you need to add some more code to "approaching" imo to not make it reliant on unit-motion.

Point autogarrisoning to GARRISONING-state.

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

@Angen, I'm keeping JSDOC anyway ;)

In D2956#128928, @borg- wrote:

Do not agree with the new cost of merchant ship, it is very expensive, and some players including @badosu were talking about removing the metal cost of the merchant units. It's really bad late game you need metal to build units that in theory are being trained to collect metal mostly.

• 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!

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

Thanks for the comments @Itms , in that case I'll carry on investigating the FPS issue and move towards committing this :)

@vladislavbelov Do you think the approach is acceptable? Indeed it works, but only by "coincidence" that CSkeletonAnimDef are long-lived. However it does bring a rather tangible improvement.

Updated, thanks for the look and thanks for reminding me I had uploaded this :p

Outpost are almost not used in 2x2 games and
3x3 or 4x4. The current changes along with nescio pacth(outpost need unit to vision)outposts are more interesting and useful, as they do not need vision technology to have a considerable vision, they also lose territory slowly.
100% is not true.

This is 100% nonsense, in every competitive MP game I play, especially the vision tech is researched ALWAYS.

Technologies were not very potent and hardly ever researched.

disabled buttons

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

@Angen not sure what you mean? Can't you call it when you display the message ?

(Also maybe add a test for this behaviour?)

In D2652#131397, @Stan wrote:

Can we support playing a sound ?

Can we support playing a sound ?

Right now,
range queries for melee units perfectly matches their range attacks,
range queries for ranged units does not because they use the same non-parabolic query as rest of units.
May be because performance or because no-one bothered to give them parabolic queries. I believe there is ticket for that. Anyway that causes ranged units to get targets which are actually out of range or not get targets that would be in parabolic range.

missed in https://code.wildfiregames.com/rP14702

Now D2998

no, thats just my english, thank you

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.

Has the ghost been cleared?

dummy
(a+b)*(a+b) != a*a + b*b

let jenkins complain about intendation

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

use correct elevation range calculation