Don't attack invulnerable units
AbandonedPublic
Actions

Authored by temple on Nov 16 2017, 1:36 AM.

Details

Reviewers: None

Summary

Catafalques are invulnerable, so it's weird when elephants start attacking them. This patch fixes that.

Units are invulnerable for a few moments while they cheer after being promoted, so attacking units will now stop attacking them during that period, either finding a new target or going idle.

Test Plan

Agree.
See that I haven't made mistakes.

In addition to CanAttack, there's also GetBestAttackAgainst, but since capture attack is looked at before melee or ranged, there shouldn't be a problem with relics if we skip the invulnerability check. In CanAttack we check if it's an enemy and check a height difference, neither of which we do in GetBestAttackAgainst, so I guess that's precedent. On the other hand, I'm not sure why we have two functions that basically do the same thing?...

Diff Detail

Lint

Lint Skipped

Unit

Unit Tests Skipped

Event Timeline

temple created this revision.Nov 16 2017, 1:36 AM

Units are invulnerable for a few moments while they cheer after being promoted, so attacking units will now stop attacking them during that period, either finding a new target or going idle.

That sounds a lot like it will break in ways that are annoying to the player.

Unit A (currently being promoted) is attacked by 2 units. Both units stop attacking it. A (now promoted) picks the next target, and can hit it without being attacked by the 3rd unit at all. The second unit is maybe going to attack back, but only after actually being attacked. So depending on the situation unit A might make 2 kills instead of dying as it would now.

Part of the issues with catafalques is that they can be attacked. Them being invulnerable is a workaround to that specific issue.

In D1044#41021, @leper wrote:

Unit A (currently being promoted) is attacked by 2 units. Both units stop attacking it. A (now promoted) picks the next target, and can hit it without being attacked by the 3rd unit at all. The second unit is maybe going to attack back, but only after actually being attacked. So depending on the situation unit A might make 2 kills instead of dying as it would now.

That's one way to look at it. But consider one unit versus two others, and the one he's currently attacking gets promoted. Currently he'll continue attacking the invulnerable unit, doing no damage while the third unit keeps hurting him. With the patch he'll stop attacking the invulnerable unit and instead attack the other unit and actually do damage.

Your situation is probably rather rare since it's usually group vs group fighting. And I guess it's kind of a UnitAI bug (similar to things I listed in #4725). But you have a point.

Part of the issues with catafalques is that they can be attacked. Them being invulnerable is a workaround to that specific issue.

I'm open to other solutions.

I'm open to other solutions.

Remove Health and related things that make them attackable, see what breaks and why, fix those things.

In D1044#41024, @leper wrote:

I'm open to other solutions.

Remove Health and related things that make them attackable, see what breaks and why, fix those things.

Well, we still want them to be attackable, i.e. capturable.
We could make a "invulnerable"-type thing but in the health component, so then it wouldn't affect cheering units.
Another option would be to add Relic to the RestrictedClasses of every melee and ranged attack template.

After relics were committed in rP19345, leper has mentioned in the first comment of that commit that the Cost, DamageReceiver and Health component are likely unneeded. We have removed the Cost component in D334.
In case you want to take that over, D341 is to remove the unneeded health component (which would also remove the hence misleading health bar) and just need some figuring out of the UnitAI maze.
IIRC the plan was to create one or two special templates that set the undeletable and invulnerable setting and use that for the survival woman too (Sandarac and mimo had already fixed the AI to work with new special templates after leper moved the special templates hardcoding from C++ to small XML files in #2951) .

In D1044#41027, @elexis wrote:

After relics were committed in rP19345, leper has mentioned in the first comment of that commit that the Cost, DamageReceiver and Health component are likely unneeded. We have removed the Cost component in D334.
In case you want to take that over, D341 is to remove the unneeded health component (which would also remove the hence misleading health bar) and just need some figuring out of the UnitAI maze.
IIRC the plan was to create one or two special templates that set the undeletable and invulnerable setting and use that for the survival woman too (Sandarac and mimo had already fixed the AI to work with new special templates after leper moved the special templates hardcoding from C++ to small XML files in #2951) .