Page MenuHomeWildfire Games

[gameplay] allow training barracks champions at any barracks
ClosedPublic

Authored by Nescio on Jan 8 2020, 8:37 PM.

Details

Reviewers
ValihrAnt
Angen
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Commits
rP23715: [gameplay-a24] Allow training all barracks champions at captured barracks.
Summary

As pointed out by @Stan in D2529, it is unbalanced you can train champion cavalry only at some but not all barracks. This patch inserts the barracks champions in the production list of the shared parent, which means that players who've researched the "unlock_champion_units" technology can train their champions at any captured enemy barracks, regardless the civ who built it, thus improving balance.

Likewise, it's unnecessary to specify the "unlock_champion_units" technology in individual specific barracks rather than the generic parent, because the technology files specify the civs that can research it; cf. D1941/rP22329.

Furthermore, it moves mace_champion_infantry_a_barracks.xml to mace_champion_infantry_barracks.xml and deletes mace_champion_infantry_e_barracks.xml, so Macedon can train them at captured enemy barracks and enemies can train theirs at mace barracks.

Test Plan

Apply the patch, play-test a couple of games, agree this is an improvement.

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

Nescio created this revision.Jan 8 2020, 8:37 PM
Vulcan added a comment.Jan 8 2020, 8:39 PM

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

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/979/display/redirect

Vulcan added a comment.Jan 8 2020, 8:41 PM

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

Link to build: https://jenkins.wildfiregames.com/job/macos-differential/75/display/redirect

Vulcan added a comment.Jan 8 2020, 8:42 PM

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

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/1497/display/redirect

Nescio updated this revision to Diff 11188.Jan 26 2020, 6:59 PM
Nescio edited the summary of this revision. (Show Details)

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

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/1662/display/redirect

Nescio edited reviewers, added: Restricted Owners Package; removed: Restricted Owners Package.Mar 25 2020, 6:27 PM
Nescio removed subscribers: ValihrAnt, bb.
ValihrAnt accepted this revision as: ValihrAnt.Apr 9 2020, 7:23 PM

I think for now it's a good change. This was one of the many things I struggled with when trying to balance champions in my mod. The civilizations which can only train champions at fortresses simply can't keep up. In the future when work is done to differentiate the civilizations and make them more unique, then a different approach can be taken.

This revision is now accepted and ready to land.Apr 9 2020, 7:23 PM

I think for now it's a good change. This was one of the many things I struggled with when trying to balance champions in my mod. The civilizations which can only train champions at fortresses simply can't keep up. In the future when work is done to differentiate the civilizations and make them more unique, then a different approach can be taken.

Just to be clear: this patch does not give all civs barracks champions; that's something for a different patch, if desired.
What this patch does, is allow the civs that already have barracks champions (and the corresponding unlock technology) to train them at any barracks, regardless who built it.
Without this patch e.g. Rome can train their champions at captured Macedonian barracks, but not at captured Athenian barracks.

Just to be clear: this patch does not give all civs barracks champions; that's something for a different patch, if desired.
What this patch does, is allow the civs that already have barracks champions (and the corresponding unlock technology) to train them at any barracks, regardless who built it.
Without this patch e.g. Rome can train their champions at captured Macedonian barracks, but not at captured Athenian barracks.

My bad, I didn't read it carefully enough. Regardless, I think this is a fine change.

Angen added a subscriber: Angen.May 28 2020, 10:42 AM
Angen added inline comments.
binaries/data/mods/public/simulation/templates/structures/maur_barracks.xml
16 ↗(On Diff #11188)

this one missed

binaries/data/mods/public/simulation/templates/structures/pers_barracks.xml
15 ↗(On Diff #11188)

should not they have been added into stable template?
I mean you remove champion_cavalry_barracks from here but do not add anywhere for persians.

Nescio added inline comments.May 28 2020, 10:57 AM
binaries/data/mods/public/simulation/templates/structures/maur_barracks.xml
16 ↗(On Diff #11188)

No, this is intentional, the maiden is a non-standard champion, like the athen marine, kush temple guard, or pers kardakes.

binaries/data/mods/public/simulation/templates/structures/pers_barracks.xml
15 ↗(On Diff #11188)

pers_champion_cavalry.xml is trainable at the fortress only, pers_champion_cavalry_barracks.xml does not exist, and the pers_barracks.xml is an infantry-only structure.
For moving champions to the stable, see D2532.

Angen added inline comments.May 29 2020, 9:26 AM
binaries/data/mods/public/simulation/templates/structures/maur_barracks.xml
16 ↗(On Diff #11188)

but it is _barrack tech trainable champion aside from other mentioned
why to allow for maur half of _barrack tech champions and other not ?

Nescio added inline comments.May 29 2020, 10:21 AM
binaries/data/mods/public/simulation/templates/structures/maur_barracks.xml
16 ↗(On Diff #11188)

If you think it's better, then that line could be moved to the generic barracks template. However, shouldn't e.g.

units/{civ}_infantry_marine_archer_b
units/{civ}_champion_marine

be moved from athen_dock.xml to the generic dock template then too?

Angen added inline comments.May 29 2020, 11:04 AM
binaries/data/mods/public/simulation/templates/structures/maur_barracks.xml
16 ↗(On Diff #11188)

likely not or if yes, in another patch as this would out of scope for this one as they are not affected by barrack technology.

They probably should not be moved to generic template as they do not have suffix _dock so any civilization will be able to train own marines in docks what does not look like case right now.

Also they are affected by hellenes/special_iphicratean_reforms exclusive for athens which states "Athenian triremes can train Marines and Cretan Mercenary Archers." what looks like or tooltip is wrong or they have no place in athen_docks.

Nescio added inline comments.May 29 2020, 11:22 AM
binaries/data/mods/public/simulation/templates/structures/maur_barracks.xml
16 ↗(On Diff #11188)

The maiden guards are a bit unusual, the swords(wo)man is trainable at maur_fortress and maur_barracks, the archer at maur_hero_maurya (i.e. Chandragupta) and maur_palace (unbuildable). No other civs have maiden units, though, whereas others do have the normally named *_barracks champions.

athen is the only civ with marines.

The tooltip is at least partially correct, since they are trainable at the athen_ship_trireme.

Nescio updated this revision to Diff 12039.May 29 2020, 11:34 AM
  • Make maur maiden available at all barracks, as requested by @Angen.
Owners added a subscriber: Restricted Owners Package.May 29 2020, 11:34 AM
Angen requested changes to this revision.May 29 2020, 11:36 AM

Maurs do have two barrack infantry champions, so they need to have different file names and cannot stick with what is standard naming for other civilisations.
It is not about if only this civilization is able to train some special unit or not.
It is not about any other building that can or cannot train some unit.
It is about allowing to train any champion unit affected by this technology in any captured barracks what is not allowed for units/{civ}_champion_maiden_barracks even if it is clearly affected by <RequiredTechnology>unlock_champion_units</RequiredTechnology> and is trainable in maur_barracks.xml so this is not complete differential and that is why I cannot agree with it.

This revision now requires changes to proceed.May 29 2020, 11:36 AM
Angen accepted this revision.May 29 2020, 11:38 AM

I was too slow with writing comment :D
Ok now I can agree.
Thank you for the patch.

This revision is now accepted and ready to land.May 29 2020, 11:38 AM

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

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/2254/display/redirect

Thank you for reviewing and committing this!