Fix unexpected starting units on random maps
ClosedPublic
Actions

Authored by elexis on Jun 16 2017, 1:39 PM.

Details

Reviewers

Grugnas
fatherbushido

Commits

rP19788: Fix starting units on random and skirmish maps.

Summary

Iberians and Macedonians start with a cavalry unit that can only be trained later at a barracks. Not a big deal, but it is consistent with all other units and when hunting,
one assumes that doubleclicking would select all units of that type, but that one cavalry is not being selected for that reason.

Seleucids are the only civ that place 4 infantry units of the same type. Not a big deal either, yet inconsistent with the rest and
there are no units placed at the right hand side of the civic center but 4 in front, whereas all other civs have 2 units at the front and 2 units at the right hand side.

Test Plan

Start a game with all these civs to see that the changed template names are correct.
If you don't trust me that it's complete, test each civ individiually.
Can be done by starting two singleplayergames with 8 players and changing the perspective with Alt+D.

Diff Detail

Repository

rP 0 A.D. Public Repository

Lint

Automatic diff as part of commit; lint not applicable.

Unit

Automatic diff as part of commit; unit tests not applicable.

Event Timeline

elexis created this revision.Jun 16 2017, 1:39 PM

Harbormaster completed remote builds in B2307: Diff 2581.Jun 16 2017, 1:39 PM

EDIT: wrong comment at the wrong place

In D647#26183, @fatherbushido wrote:

EDIT: wrong comment at the wrong place

Uploading a patch for review is asking, no? Even if it was Grugnas uploading and not me ;)

In D647#26185, @elexis wrote:

In D647#26183, @fatherbushido wrote:

EDIT: wrong comment at the wrong place

Uploading a patch for review is asking, no? Even if it was Grugnas uploading and not me ;)

wrong paste

Executing section Default...
Executing section Source...
Executing section JS...
Executing section XML GUI...
Executing section Python...
Executing section Perl...

http://jw:8080/job/phabricator_lint/201/ for more details.

The patch content seems clear enough.

Seleucids are the only civ that place 4 infantry units of the same type. Not a big deal either, yet inconsistent with the rest and
there are no units placed at the right hand side of the civic center but 4 in front, whereas all other civs have 2 units at the front and 2 units at the right hand side.

This isn't really a big deal but it is reasonable.

Iberians and Macedonians start with a cavalry unit that can only be trained later at a barracks

It is reasonable. Units trainable later in the game shouldn't be available at start

This revision is now accepted and ready to land.Jun 16 2017, 2:38 PM

Phormio mentioned this in D649: Fix wrong starting units for all maps in accordance with D647.Jun 16 2017, 2:41 PM

I asked for that sometimes ago too but got a negative answer iirc. (Those leftovers by the time become features :p)

For both it seems leftovers (r10389 r14239 r14438 r15713 r16504 r11450)

From the design docs point of view.

seleucids had only spearmen at cc
iberians didn't have any basic javelin cav
mace had both kind of cav

So for seleucids and iberians, it seems the root were here, but design have evolved since.

(

It is reasonable. Units trainable later in the game shouldn't be available at start

I can't agree. See special starting units (speaking of that, that a thing not finished). (I won't speek of skirmish and scenario map).
)

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Checking XML files...

http://jw:8080/job/phabricator/1569/ for more details.

Harbormaster completed remote builds in B2307: Diff 2581.Jun 16 2017, 3:19 PM

fatherbushido accepted this revision.Jun 16 2017, 3:23 PM

Thanks for the reviews and looking up of the related template edit history

elexis retitled this revision from Fix wrong starting units to Fix unexpected starting units on random maps.Jun 16 2017, 6:50 PM

Closed by commit rP19788: Fix starting units on random and skirmish maps. (authored by elexis). · Explain WhyJun 16 2017, 6:51 PM

This revision was automatically updated to reflect the committed changes.

elexis mentioned this in rP19789: Apply previous commit to scenario maps..Jun 16 2017, 8:51 PM