vesuvius and two_seas are missing a p (e.g. two_seas_6p.xml instead of two_seas_6.xml)

Trigger-based test map, showing rather poor behaviour all around. The patch fixes the particular issue it was made to fix, but not the fact that ships move around the map too much.
I noticed that ship behaviour is bad because they don't adjust when the unit they want to pick up moves, which is because they are following a "known bad path", so they only update the path when it ends (this is WAD but the design expected units & ships to converge on a common point, which doesn't happen).

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

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

rebased

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

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

rebased

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

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

rebased

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

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

rebased

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

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

The map preview art files naming is inconsistent and ideally ought to follow the https://trac.wildfiregames.com/wiki/ArtFileNamingConventions and be made to match the file names of the maps they belong to (rP23719). However, I won't be the one doing that.

This is a pleasant surprise! Many, many thanks, I highly appreciate it! D1042 was the one patch I was looking forward to most. When I proposed it in 2017, I hoped it would make it into A23; I'm glad it's now part of A24. It's a significant improvement, one that gives Vulcan one less reason to complain phabricator fails to build. Thanks again, you made my day!

Thanks, @Stan!
Next request: could athen and spart get a donkey, and brit and gaul a cart?
Also, a dromedary camel might be more appropiate for sele; cf. pers and ptol.

The last time I looked at this it grouped packed and unpacked siege engines together. That no longer seems to be the case.
Two other things to consider:

• Roman artillery inflicts more damage; should those be grouped separately?
• While there is only one generic parent template for bolt shooters and one for stone throwers, in the units folder (and workshop production queue) there are three each: scorpio, polybolos, oxybeles and onager, lithobolos, ballista. Perhaps those ought to be the selection groups?

I don't know, I don't have strong feelings either way.

The

And in case it wasn't clear from the code, the trigger script spawns wolves, the wolves receive Move orders making the wolves attack the players units - so basically zombie wolves. It was added to make the map less boring. The wolves are Domestic because currently only Domestic animals can follow move orders […]

This plays into two concepts:

• Controllability (see D1960)
• actually being able to move.

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

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

Investigating further:

• it seems one could patrol to a target by changing the GUI. Unlikely to have worked before, fixed by this patch.
• Guarding has some seriously custom code in Transform.js to maintain the list of guards. It seems we could clean it up a bit.
• As noted by bb, fleeing was broken before. This fixes it but plays the "panic" sound again, so I'll add a custom handler.
• Garrison approaching will request a new pickup, which seems fair. Garrisoned early-exits.
• Chasing & repairing have been noted and still work correctly.

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

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

Add tests & actually mage visible garrisoned entities have no obstruction at all because it still broke and that makes more sense than hacking around flags.

For the record, given the hack I'm now introducing in ProductionQueue.js, I won't commit this until I either:

• Get some other dev to look at this and agree that it's painful but we should go along with it
• Refactor so I can remove the hack.

Thanks for the look both of you, this was one of these things that had been annoying me for a while :)

I independently rediscovered this on D2768. ReportError seems rather useless (particularly since JSNative functions can return false to indicate failure), and it is removed in later SM versions.

Shouldn't the sele paired tech benefit from this immediately?

Thanks for the patch and review, I had forgotton about that regression of mine.

The comments without parentheses will be understood as helpful by most readers.

Needs a rebase, appears to be equivalent to rP8997

The name of Flight_demo_2 should have been adapted.

In rP22296#42528, @bb wrote:

Seems like the pmp file is missing, map doesn't load here

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

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

Seems like the pmp file is missing, map doesn't load here

In rP23710#42484, @Angen wrote:

Start game with iberians.
Update long wall to gate.
Move unit through gate.
It will never ever close back.

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

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

Are there any unintended consequences?

Welp, this broke rP23710

@Nescio thoughts ?

@Itms It seems we can now set up 're-openable diffs' which would be rather convenient here, see https://stackoverflow.com/questions/13633031/how-to-reopen-differential-review-in-phabricator

In rP23710#42505, @Nescio wrote:

Tested by: Nescio

For the record, I don't believe I actually tested D1418; that would have involved rebuilding the source code. What I did was comment that I liked the concept, and ask for its current status.

@ValihrAnt @borg- would this be okay if I only committed the cost change? Or is there a good reasoning behind the team bonus change?

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

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

I can commit it

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

@bb, interested in reviewing this one?

you can try :)

Thanks, I did, in combination with this patch, and I'm quite happy with the result: