Projectile Attachment Error with Slaughtering

[scythetwirler]

In Atlas or during a game:

Have 2+ players. Player 1, own a couple human units (or non-domesticated animals) mixed with couple sheep/goats/chickens. Huddle them together. Player 2, own ranged infantry citizen soldiers. Attack Player 1's "herd".

When player 2's unit start killing sheep, they use the slaughter animation. After they kill a sheep, if they attack a non-domesticated unit, an error pops up with skirmishers and archers. Slingers do not trigger the error, but start slinging rocks with the slaughter animation.

[leper]

In 13751:

Fix #2079.

[mimo]

Playing with r13760, I've got the following error

ERROR: Error in timer on entity 4903, IID 73, function TimerHandler: TypeError: this.order is undefined

([object Object])@simulation/components/UnitAI.js:1474 ([object Object],[object Object])@simulation/helpers/FSM.js:274 ([object Object],0)@simulation/components/UnitAI.js:2950 ([object Object])@simulation/components/Timer.js:93

This seems to be connected to the fix for #2079. Looking at the code for any reason for this.order to be undefined, I must say I'm puzzled by the fix : from my understanding, this.order and this.orderQueue[0] are identical after FindNewTargets, so the test in 1474 is always satisfied ? and in addition, whatever is done in these lines, this.order.data.attackTypeUnchanged is never defined in line 393 as we pass there during the FindNewTargets so before it is defined ? That's why the fix nonetheless works : I've the impression all the fix is equivalent to replacing the SetNextState by SetNextStateAlwaysEntering. To go with the logic of this fix, we should save the AttackType of the current attack (before the FinishOrder in line 1462) and test on it in line 393.

[leper]

Yes, the attackType should be saved before the call to FinishOrder(). I missed that call before committing the fix. You're also right about the attackTypeUnchanged never being set (at least not when it matters).

[leper]

In 13768:

Properly fix #2079.

[leper]

Thanks for noticing mimo :)

[mimo]

Thanks for fixing. I also wonder if we shouldn't add a leave method to COMBAT with

this.oldAttackType = undefined;

otherwise we could be left with some remnant of an old attack ?

[leper]

Adding one would clean up the variable, but just leaving it as it is doesn't hurt either. If we have a "wrong" attack type saved we can't be in ATTACKING so we will always run through the enter state. If we are in ATTACKING and we have a new attack order queued we don't need the value if we go to the new order through FinishOrder() and we just reduce the delay before firing again (if the target is within range). If we don't have another queued order, we set it before using it (as we do with a queued order) so we don't have the wrong attack type.

(Though judging from the needed explanation maybe adding it to leave would be a good idea. Are you sure it should go in COMBAT.leave and not in ATTACKING.leave?)

[mimo]

I don't think it can be in ATTACKING.leave because if we find a new target but are out of range, we would have to go to COMBAT.APPROACHING and then the variable would be cleaned ?

But thinking again to it, you must be right that it does not hurt to leave it as it is.