Page MenuHomeWildfire Games

Melee cavalry should not preferentially target siege
ClosedPublic

Authored by causative on Apr 29 2017, 9:02 AM.

Details

Summary

The templates for melee cavalry have <PreferredClasses>Siege Human</PreferredClasses>. This means for example that if there is an enemy army with a siege engine behind it, your cavalry on an attack-move order will try to run around/through the army to hit the siege engine, instead of engaging the army. Usually this means your cavalry die. Another scenario would be if there are archers you want your cavalry to engage and have walked your cavalry up to, but there's also a siege engine way off to the side near edge of LOS, your cavalry will ignore the adjacent archers and run far away to hit the siege engine. There is basically no scenario where it is helpful to have Siege as the most preferred class for melee cavalry.

This patch updates the various templates for melee cavalry and removes Siege as a preferred class, so they only have Human as a preferred class, like most other land units have.

Test Plan

Make a scenario with a hero cavalry spearman, a hero cavalry swordsman, a champion cavalry spearman, a champion cavalry swordsman, a citizen cavalry spearman, a citizen cavalry swordsman, a few enemy humans, and an enemy siege engine off to the side. Verify that if you order your cavalry to attack-walk past the humans, or halt them near the humans, they will attack the humans instead of the siege engine.

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

causative created this revision.Apr 29 2017, 9:02 AM
causative updated this revision to Diff 1528.Apr 29 2017, 9:15 AM

swapped the classes to Human Siege instead of just having Human

Vulcan added a subscriber: Vulcan.Apr 29 2017, 10:42 AM

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!

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

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!

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

fatherbushido edited edge metadata.Apr 29 2017, 2:21 PM

Perhaps some are missing (keep it for ballista perhaps)

template_unit_hero_cavalry_spearman.xml:15: <PreferredClasses datatype="tokens">Siege Human</PreferredClasses>
template_unit_champion_cavalry_swordsman.xml:15: <PreferredClasses datatype="tokens">Siege Human</PreferredClasses>
template_unit_hero_infantry_swordsman.xml:15: <PreferredClasses datatype="tokens">Human Siege</PreferredClasses>
template_unit_champion_cavalry_spearman.xml:15: <PreferredClasses datatype="tokens">Siege Human</PreferredClasses>
template_unit_infantry_melee_swordsman.xml:13: <PreferredClasses datatype="tokens">Siege</PreferredClasses>
template_unit_cavalry_melee.xml:15: <PreferredClasses datatype="tokens">Siege Human</PreferredClasses>
template_unit_hero_cavalry_swordsman.xml:15: <PreferredClasses datatype="tokens">Siege Human</PreferredClasses>
template_unit_mechanical_siege_ballista.xml:22: <PreferredClasses datatype="tokens">Human Siege</PreferredClasses>
template_unit_champion_infantry_swordsman.xml:15: <PreferredClasses datatype="tokens">Human Siege</PreferredClasses>

elexis edited edge metadata.Apr 29 2017, 2:46 PM
elexis added a subscriber: Grugnas.

Just swap the preference order.

Should that remaining prefered class become Unit so that it focuses the closes unit, even if the unit is surrounded by rams?

Could go for Unit, but it's not consistent with other templates, and possibly that could lead to them attacking sheep? Dunno.

Hmm, so all those structures who prefer to attack Human also target elephants last? Actually elephants are human, meh
Perhaps Organic would be a less ambiguous classname to use? But then we need to remove Domestic indeed.

What if it became Human+Siege, would it consider Human and Siege units equally (i.e. attack the closest siege or human while ignoring sheeps and buildings that are closer)?

causative added a comment.EditedApr 29 2017, 5:33 PM

Perhaps some are missing (keep it for ballista perhaps)

template_unit_hero_cavalry_spearman.xml:15: <PreferredClasses datatype="tokens">Siege Human</PreferredClasses>
template_unit_champion_cavalry_swordsman.xml:15: <PreferredClasses datatype="tokens">Siege Human</PreferredClasses>
template_unit_hero_infantry_swordsman.xml:15: <PreferredClasses datatype="tokens">Human Siege</PreferredClasses>
template_unit_champion_cavalry_spearman.xml:15: <PreferredClasses datatype="tokens">Siege Human</PreferredClasses>
template_unit_infantry_melee_swordsman.xml:13: <PreferredClasses datatype="tokens">Siege</PreferredClasses>
template_unit_cavalry_melee.xml:15: <PreferredClasses datatype="tokens">Siege Human</PreferredClasses>
template_unit_hero_cavalry_swordsman.xml:15: <PreferredClasses datatype="tokens">Siege Human</PreferredClasses>
template_unit_mechanical_siege_ballista.xml:22: <PreferredClasses datatype="tokens">Human Siege</PreferredClasses>
template_unit_champion_infantry_swordsman.xml:15: <PreferredClasses datatype="tokens">Human Siege</PreferredClasses>

Keep what for ballista? What is missing? I was just making it about melee cavalry, not trying to fix PreferredClasses for every unit. I did verify that infantry swordsmen don't target siege preferentially even though their template says "Siege" - apparently they inherit "Human" from the melee infantry template with higher priority.

What if it became Human+Siege, would it consider Human and Siege units equally (i.e. attack the closest siege or human while ignoring sheeps and buildings that are closer)?

I don't know about "Human+Siege" with the addition sign. If it says "Human Siege" then it attacks the closest human, ignoring siege that are closer, unless there are no humans around, and then it attacks the closest siege, unless there is no siege around, and then it attacks the closest anything.

By the way, I think not auto-attacking siege when humans are around is not a problem, because you can just click on the siege to individually focus it down. Usually you want to micro like this because only certain units are good at killing siege, there aren't many siege around, and you want to kill them one at a time in a specific order to minimize damage to your buildings.

fatherbushido resigned from this revision.Apr 29 2017, 5:37 PM

ah yes we set Human Siege for Infantry. (we was I :p)
Keep cool :-)

Recently had a game where I had only elephants and some skirmishers facing a group of enemy units in a remote distance to the enemy city and those elephants entirely ignored the attackers.
So it's the same issue.
However for this case we can use the ctrl+q+rightclick hotkey, which should be set to a more prevalent combination (same for the idle worker hotkey/lasso, which could be set to F while almost noone uses F to follow units)

Grugnas requested changes to this revision.EditedMay 3 2017, 9:12 AM

Spearman cavalry should really prefer humans over sieges. The only units effective against a siege are swordmen, and they can easly be commanded to attack a siege instead of humans around despite their class preference.

Addressing units with most of their damage output as pierce damage to attack an high pierce armor siege is kinda pointless (notice that sieges even have higher pierce armor than buildings).

Despite the ballista crush damage, they are meant to attack units (it is even more effective since the splash damage has been introduced).

I'd suggest yo to simply remove the class Human from any cavalry template, and let templates inherit it from their unit_cavalry parent (just like happens for infantry) avoiding to specify the preferred class in any template.

This revision now requires changes to proceed.May 3 2017, 9:12 AM

elexis, I think that elephants are okay because of ctrl+q+click. That way you have the flexibility to attack units, or to by default let them hit buildings.

Grugnas, it's not possible to inherit from base cavalry templates because PreferredClasses is attached to the melee attack, which only appears on the specific melee templates. I can remove Siege from cavalry hero and champion spearmen. I also have an option to add -Siege on the non-champ spear cavalry template; not 100% sure that will work. Also note it doesn't really affect gameplay, as having Human Siege only means they will attack siege in preference to buildings if no enemy Humans are around. And maybe you'd actually want spear cavalry to do that, if there are no humans are around the siege is likely to be undefended.

causative updated this revision to Diff 1698.May 7 2017, 3:25 AM
causative edited edge metadata.

Updated so that cavalry spearmen (normal, champion, or hero) have only Human as a PreferredClass. Also updated so that cavalry swordsmen have Human Siege as PreferredClasses, which is the same as what infantry swordsmen have.

Vulcan added a comment.May 7 2017, 4:16 AM

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!

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

Grugnas accepted this revision.May 8 2017, 4:35 PM

While at it, please check Rams too. I am afraid they stop from attacking a building whenever an unit attack them while in "aggressive" stance as default and i am not sure if a siege should change target and attack its attacker.

This revision is now accepted and ready to land.May 8 2017, 4:35 PM

Rams have <PreferredClasses datatype="tokens">Gates Structure</PreferredClasses> .

elexis accepted this revision.May 16 2017, 4:43 AM
  • Implementing Human+Siege with MatchesClassList might be nicer so that it always focuses the closest thing (RestrictedClasses has that parsing implemented already in the Attack component). But it is an acceptable design decision to require the player to focus down the siege engines if he wants to
  • Might disagree with not prefering siege engines over buildings. The latter have like 10 times more HP and spear cavalry still have hack damage output currently. Also it isn't really consistent (in case someone wants to go for a consistency patch, I wouldn't raise a concern)
This revision was automatically updated to reflect the committed changes.