Opened 12 years ago

Closed 10 years ago

Last modified 8 years ago

#1537 closed defect (fixed)

[PATCH] Melee attack on a running target

Reported by: CoolleR Owned by: sanderd17
Priority: Must Have Milestone: Alpha 16
Component: Core engine Keywords: patch
Cc: cooller@… Patch:

Description (last modified by sanderd17)

Any attack on a running target in melee does not pass - the animation goes, but the damage is not applied. As a result, the rider can not kill him by running footman

There are other cases the patch wants to solve too, here's a list of related cases it could/should solve:

  • faster ranged unit with a minrange running away from a melee target should run far enough to have the time to fire
  • Units with a prepare time should come close enough to a slower fleeing target to be able to fire at least one projectile before starting the chase again
  • Units that want to move into a moving garrisonholder should be able to do so
  • Units that want to drop resources to a moving dropsite should be able to do so

Attachments (7)

rangedFix-example.patch (1.4 KB ) - added by wraitii 12 years ago.
chasing.diff (6.2 KB ) - added by sanderd17 10 years ago.
chasing.2.diff (15.6 KB ) - added by sanderd17 10 years ago.
chasing.3.diff (26.9 KB ) - added by sanderd17 10 years ago.
chasing.4.diff (29.8 KB ) - added by sanderd17 10 years ago.
chasing.5.diff (31.5 KB ) - added by sanderd17 10 years ago.
chasing.6.diff (11.7 KB ) - added by sanderd17 10 years ago.

Download all attachments as: .zip

Change History (28)

comment:1 by wraitii, 12 years ago

There is the same problem in some other cases, such as when a javeliner is attacked by a lion. It flees to some minimal distance, starts to shoot its javelin, and the lion catches up before the shot so the javeliner gets hit and inflicts no damage as it tries to regain some distance. It's sort of linked to the problem of two units with the same walking speed chasing each other possibly indefinitely.

The melee problem is at line 842 of UnitAI.js, it checks after the "Timer" if the attacker is still in range. Only the attacker stopped moving and the target not, so the attacker is out of range. Rinse, repeat. To fix this problem, we could either make the attacked unit stop, or allow a unit to attack while walking.

The javeliner problem is caused by the minimal range of the attack. The lion has gotten too close, so the javelineer moves away. Only the javeliner forgot to take into account that the lion moved too and that it should retreat way more. So he gets stuck in an infinite loop of dying. Fix for that would be to make the javeliner move farther away (javeliner aka any ranged unit, I mean). I might throw a patch for this, seems simple enough, while the melee one is trickier.

The infinite chase problem is because the units don't check they can outrun their target. "ShouldAbandonChase" (line 2613) doesn't check for unit speed, when it really should. I'll try fixing that one as well.

Last edited 12 years ago by wraitii (previous) (diff)

by wraitii, 12 years ago

Attachment: rangedFix-example.patch added

comment:2 by wraitii, 12 years ago

The attached patch should improve/fix the behavior of ranged units (aka the javelineer case above). I'm showing this more as a proof of concept/matter of discussion though, it may introduce weird behaviors I haven't thought of.

comment:3 by historic_bruno, 12 years ago

I was looking at this problem a few weeks ago. I think it's more complicated than using speed difference alone, because we have to consider attack timings (how long does it take to prepare an attack?) There is a preparation speed for melee and ranged units, some of them like siege are very slow - up to 5 seconds! A low level component like CmpUnitMotion probably shouldn't know about attack speeds, but it should obviously be a part of determining point of attack. I was thinking maybe altering the min and max attack range to account for this but couldn't get a satisfactory solution.

It leads to another problem in the pathfinder, I don't think a unit can continuously pursue a target? There is a fixed target which is where the target was, and every time the pursuer reaches that target, it stops and if possible calculates a path to where the target is now. During the time the unit is stopped, the target gets farther ahead, which is ridiculous and would never happen in the real world. When units have similar speeds it's especially noticeable. We need to eliminate that delay.

One issue to think about for the future is different movement speeds like running and charging, and how these will impact the calculation. For now I guess it would be OK to work with the assumption of walking.

Last edited 12 years ago by historic_bruno (previous) (diff)

comment:4 by historic_bruno, 12 years ago

Although thinking about it, the siege unit in pursuit is not a big deal because I think many or all siege units will not be mobile once we have packing and unpacking, and also their targets will often be static (structures). So that's an extreme case, but the timing concern still applies to other units.

I don't like the idea of making the target unit stop, but maybe temporarily slowing it down if the attack range check has succeeded wouldn't be a bad idea. It should likely be done not to an absolute speed but proportionally, so that an attacked and wounded cavalry unit would still have a chance to "outrun" a slow infantry unit.

comment:5 by wraitii, 12 years ago

We discussed it with Deiz on IRC, and came up with this for the "estimation of by how much you should walk farther away": (anim time + aim time) * speed diff + (some small arbitrary number)

The problem is actually two-fold: fleeing enough to fire a shot while not being hit, and when chasing, chasing enough to have time to fire before the units goes away again (which might mean being ahead of the chased unit in some cases). Thisis valid for both melee and ranged units, but ranged units anims usually take longer.

The problem with chasing is that the units assume the enemy is not moving. I'm trying to fix by having the unit go to the "estimated interceptory point", but my fix will be pretty basic.

comment:6 by Kieran P, 12 years ago

Milestone: Alpha 11Alpha 12

comment:7 by FeXoR, 12 years ago

I don't think this will always help since the (ranged) unit to attack can (will if trying to get the min dist to the attacking melee unit) move the other direction if the melee unit gets to the "estimated" position on the other side of the ranged unit. I said this quite often a long time ago but I will say it again once in a while. And now it's time, sorry!

No non-siege unit should have a min range > 0. This is more realistic AND avoids such problems. If you feel it's really needed you could use another animation in this case (as if archers would use a dagger or Javelins don't throw their weapon) but the damage does not need to wary for this (indeed other problems/strangenesses arise when the melee damage can be smaller than the ranged attack).

Melee units attacks should deal the damage VERY EARLY during the attack animation (despite the actual time the hit takes place visually) OR melee units range should only be checked when starting the attack, not when they are actually dealing the damage. Both solution solve the problem by "unlinking" visual and actual damage dealing.

Unit AI should not try to do the job of the player. So ranged units should not flee and melee units should not focus ranged units/siege engines far behind other enemy units that might be considered less optimal targets, because: That "try to be efficient" approach in a global scale is very inefficient! They simply deal less damage in both cases.

Similar problems will arise when attack priority is added so please think this through before applying it...

This issue is somehow connected to #1010.

Last edited 11 years ago by FeXoR (previous) (diff)

comment:8 by Kieran P, 11 years ago

Milestone: Alpha 12Backlog

Backlogged until a decision is reached about how best to fix this.

comment:9 by historic_bruno, 11 years ago

A different (partial) solution proposed by Michael is to move the range check for attacks to before the attack begins, especially for melee attacks. The way it works now, a prepare timer is set when the unit enters the "attacking" state, and it's not until after that the range check occurs, immediately before damage. It would require a slight rework of the attack logic because the range check would occur at two points, before the attack begins and before it repeats.

I'm not sure how it would work for ranged units, they could still fire, but we would want their projectiles to be limited to the max range, not the target's current position (which could be well beyond max range).

comment:10 by wraitii, 11 years ago

I think it's a good solution to the issue (proper fixing would mean attacking while moving, to me, but we don't have that and it's hard, while this absolutely needs to be fixed).

Basically for ranged units we check if the unit will attack at attack start, and we update the position of the shot at arrow/javelin launch. Only we still count the maximal range, ie the unit will fire too short if the enemy has moved out of range.

Seems sensible anyhow because an archer will most likely hardly go "I want to shoot that", ready his shot, then just as he's about to fire "wait no he moved a meter away, I'm out of range, let's not shoot". He'll just fail.

by sanderd17, 10 years ago

Attachment: chasing.diff added

comment:11 by sanderd17, 10 years ago

Milestone: BacklogAlpha 16

This patch allows units to attack other units that are running away.

There are two parts needed for the attack to come through:

  1. When the unit is attacking another moving unit, there's an overshoot added to the ranges, so the attacker has at least one turn time to fire the attack
  2. When the target is a moving entity, the path will be recalculated when the entity isn't in range after all. The path recalculation happens before sending messages, so there's no change in animations.

Next to that, I also made the attack timer faster in UnitAI. Because of this faster timer, the attacker faster realises the target has moved out of range, and the attacker can begin chasing again. Without the faster timer, the attack animation takes as long as defined in the attack repeatTime, and by that time, the target can be quite far already, so the animation looks silly.

by sanderd17, 10 years ago

Attachment: chasing.2.diff added

comment:12 by sanderd17, 10 years ago

New version now fixes the problems with small ranges too (garrisoning in a moving object, dropping a resource off to a walking elephant ...), as well as the problems that require more time than one turn (ranged attack with a prepare time).

by sanderd17, 10 years ago

Attachment: chasing.3.diff added

by sanderd17, 10 years ago

Attachment: chasing.4.diff added

comment:13 by sanderd17, 10 years ago

Keywords: patch review added
Summary: Melee attack on a running target[PATCH] Melee attack on a running target

It ended up to be a bigger patch than expected. But now, all range problems with moving ents should be solved, and the margins should be decided on some logical places.

by sanderd17, 10 years ago

Attachment: chasing.5.diff added

comment:14 by sanderd17, 10 years ago

Fixes for some minRange issues.

comment:15 by wraitii, 10 years ago

Seems to work from the cases I've tested, even with archers. I'd like it with leper or historic bruno could give this a look before commitment though.

comment:16 by sanderd17, 10 years ago

Description: modified (diff)

by sanderd17, 10 years ago

Attachment: chasing.6.diff added

comment:17 by sanderd17, 10 years ago

After some discussion with Philip, a really simplified patch. Instead of having the range check before performing the attack, it only happens after the attack. So there's one attack always coming through.

comment:18 by wraitii, 10 years ago

Some element sin CcmpUnitMotion.cpp: Why the change from 4 to 2 for g_GoalDelta? l1002: I would put "cmpUnitMotion && cmpUnitMotion->IsMoving()" on the same line for readability.

From a technical POV, this seems very simple, and if Philip' and you agree on that, I'll say it's likely among the best solutions. I've talked on IRC about an issue, if it's fixed then commit.

comment:19 by sanderd17, 10 years ago

Owner: set to sanderd17
Resolution: fixed
Status: newclosed

In 14449:

Fix units chasing each other. Fixes #1537

comment:20 by Itms, 9 years ago

In 17013:

Support inverted goals with the long-range pathfinder. This allows units to flee and should fix problems with ranged units too close to their targets. Fixes #3405, refs #3372.

Now that units flee it's necessary to fix the unit chasing: this commit reintroduces some code from r14449 that disappeared with the committing of the new pathfinder. Refs #1537.

Also includes some style improvements to the UnitMotion code.

comment:21 by sanderd17, 8 years ago

Keywords: review removed
Note: See TracTickets for help on using tickets.