Patch works as expected, though it triggers a compilation warning see inline. (Missing WxUnused)

Changed war elephant cost to 300F and 200M. So their total cost is the same as in A23, but they are more accessible due to having a lower metal cost. Also adjusted their loot.

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

I think it would be nicer to have the MP savegame features, but if it isn't gonna happen soon, I believe this is the next best thing.

Feature is good, code looks good to me, and it works as expected.

(or posting that you accept it so that he can close the revision proposal since it was committed in rP23395, to avoid ambiguity with an empty review)

Please do not forget to post which parts were reviewed, which parts were tested when posting an acceptance.

In D2567#108291, @Stan wrote:

Accepting so you can close.

Accepting so you can close.

What do you mean exactly?

This is the patch I uploaded at https://code.wildfiregames.com/rP23387#40853 so it would be correct to credit that.

The patch results in

ERROR: GUI: Script path gui/pregame is not a directory

while it actually is a directory, just not one recognized by the code.

Also:
Removes the hack to Pop and Push the dialog in order to update it.
Update the gameAttributes instantly when changing an AI dropdown, so that the selection isnt lost when another client changes any game setting, preparing for #3806.

• mapType moved to common/ in rP23374
• mapFilter move is unwarranted following the decision in rP23413

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

As mentioned in D2581, it is necessary to run the AIConfig page in the same script context as the GameSetupPage if one wants exclusively that page to be in charge of the AI attributes in g_GameAttributes.settings.PlayerData, since the page can't run code if its closed, but it can if its only hidden.
Not doing it that way would mean there would be a class in the AIConfig page manifesting the AI setting logic and another class in the Gamesetup page coding the same logic.
Second reason to perform this change is as mentioned the existing coupling between the Gamesetup page and subpages, which is a pattern, and thus refactoring this page to become a subpage means the pattern becomes an example of how to implement Gamesetup subpages, not a counterexample, with the new subpages being empowered and encouraged to reuse the existing gamesetup controlers (gameSettingsControl, playerAssignmentsControl, netMessages, mapCache, mapFilters, ...).

Rebase following rP23413 and remove more hardcodings.

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

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

Fixes @Stan notes.

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

True, there are no Elephant-specific technologies, though e.g. archery tradition does apply to elephant archers.

True, I forgot about it.

Something that could be tried is introducing two technologies, the first upgrades elephant archers from basic (one archer) to advanced (two archers), the second to elite (three archers). I'm not sure it's a good idea, though. And probably beyond the scope of this patch.

It is beyond the scope of this patch, but it's an idea I've thought about. Upon each promotion the elephant has another archer firing off of its back, but even though there are 2 or 3 units firing in the animation only 1 arrow gets shot, so what could also be done is to have the upgrades give them an extra projectile, kind of like the Chukonu in AoE2.
But, overall the patch makes elephant archers more useful in the early game and them falling off as upgrades start kicking in is something that can be addressed in a future patch.

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

The crannog and colony are fundamentally centres and should thus be listed together.
And it'll only show up to those civs that actually have the template, if that's what you meant.

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

mercenary/ies → Mercenary/ies
recruit → train

I’ve left one more comment, but overall this is an amazing change for the better, great work!

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

Rebased.

In D2496#104876, @Nescio wrote:

Unit templates don't have a static obstruction defined in their templates, so I assumed their effective target size is based upon their footprint.

The related code (Attacking.js-helper; L137 onwards) shows that footprints are indeed used.

I agree that the CC ought to be placed first. Not sure though that the crannog, which is a Briton-specific structure, should be placed in the general build-list.
+1 on the Military Colony.

(The patch was not tested, see IRC)

Looks ok for me.

True, there are no Elephant-specific technologies, though e.g. archery tradition does apply to elephant archers.
Something that could be tried is introducing two technologies, the first upgrades elephant archers from basic (one archer) to advanced (two archers), the second to elite (three archers). I'm not sure it's a good idea, though. And probably beyond the scope of this patch.

Thanks! Now let's hope a team member agrees with the change and is willing to take responsibility for committing it.

Discussed with Vladislav on http://irclogs.wildfiregames.com/2020-01/2020-01-18-QuakeNet-%230ad-dev.log and with nani in a PM, concluding preference for running it in the same page.
It is necessary to run it in the same page if there is a requirement to have the setting handler exclusively in one class, and in only one page, see summary.

I've tested the patch on Windows VS2015, it works and looks good for me.

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.

• Rename the sdl2_* keys to sdl_*.
• Include used CLogger.h.
• Include useless libsdl.h but that’s part of the project guidelines.
• Attempt a workaround for MSVC2015.

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

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

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.

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

Infantry archers cost 50 food + 50 wood and have 50 health.
Camel archers cost 100 food + 50 wood and have 100 health.
One would expect the elephant archer to cost 200 food + 50 wood (it has 200 health).

200 health is not really a lot, though: worker elephants have 300, champion elephants 750, champion cavalry spearmen 300, and citizen cavalry spearmen 160. So maybe the elephant archer deserves a significant health increase?

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

Fix the copyright year.

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

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

I support this change. Also, I remember there being a Persian cavalry archer champion in the files somewhere. Wouldn't be bad to include that guy in the stable as well if possible.

Include "pregenerated.h"

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