Page MenuHomeWildfire Games

Fix some selection group names
ClosedPublic

Authored by temple on Nov 13 2017, 10:11 PM.

Details

Summary

Long Ptolemy walls have a different selection group name than the medium and short ones, which means if you double click a long wall it'll only select other long walls rather than medium and short ones too. This patch fixes that.

Seleucid walls should use the civ name.

Rome barracks champion infantry inherit from regular champ infantry, so they don't need to define the selection group name again.

Persian trireme horses should inherit the same way as barracks champions do, i.e., use the same unit and just add the required technology (and in this case change the promoted entity).

Balance change: I don't see any reason for the +3 build time for trireme horses. With the exception of women, I don't think any other unit has a different cost depending on where it's built. (Buildings can have different batch multipliers, though.)
(It seems originally at rP14268 it was regular sword cav 80f, 60m, 13 seconds versus trireme 80f, 70m, 15 seconds; and regular jav cav 100f, 40w, 12 seconds versus 100f, 40w, 15 seconds.)

Test Plan

Agree.

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

temple created this revision.Nov 13 2017, 10:11 PM
Stan added a subscriber: Stan.Nov 13 2017, 11:48 PM

Might I suggest keeping the balance change for another ticket ?

elexis added a subscriber: elexis.Nov 14 2017, 12:01 AM

(Should be fine in this case if it's just dropping of one not so relevant inconsistency and is mentioned in the commit message)

If a reviewer/committer wants to split it up, that's fine. Just need to add this to the six files:

<Cost>
  <BuildTime op="add">3</BuildTime>
</Cost>
bb accepted this revision.Nov 14 2017, 6:04 PM
bb added a subscriber: bb.

Even the logic that one can build to many training spots doesn't quite hold here, since ships are just as hard as barracks, so fine for the balance.

Some strange inconsistendy that I noticed, when building a foundation (f.e. a corral one) and double clicking that, one select all corral foundations and not the fully build ones. When doing the same with walls, one selects all walls (foundations+build ones). Probably unrelated since any civ has it.

Also unrelated triple clicking does make the difference between "barrack" and fort champs (or ship vs whatever), don't know if that is intended.

Another revision could perhaps also remove the rank from the group name for most citizens healers and dogs.

Patch looks correct and complete further.

This revision is now accepted and ready to land.Nov 14 2017, 6:04 PM
In D1033#40718, @bb wrote:

Some strange inconsistendy that I noticed, when building a foundation (f.e. a corral one) and double clicking that, one select all corral foundations and not the fully build ones. When doing the same with walls, one selects all walls (foundations+build ones). Probably unrelated since any civ has it.

Also unrelated triple clicking does make the difference between "barrack" and fort champs (or ship vs whatever), don't know if that is intended.

I noticed both of these things and guessed that it's because if there's no selection group name (or if you triple click), then it uses the template name instead. Since foundations add "foundation|" to the front of the templates, they won't match completed buildings. However, if you have a selection group name for the building, then the foundation inherits that, so they will match (unless you triple click, when it'll use the template name instead.) But I didn't look at the code to see if this was actually how it was done.

Another revision could perhaps also remove the rank from the group name for most citizens healers and dogs.

Well, you could remove the '_b' from the selection group name of every promotable unit, but it's not really hurting anything. Was there something wrong with healers and dogs in particular?

bb added a comment.Nov 14 2017, 7:48 PM

(healers and dogs have ranks too, but for another revision though)

This revision was automatically updated to reflect the committed changes.