Error in BuildingAI FireArrows when a player with a tower is defeated

[Jonathan Waller]

When testing with qBot vs JuBot I encountered this error which appeared repeatedly as soon as the "You have Won" dialog appeared.

ERROR: Error in timer on entity 3114, IID 38, function FireArrows: TypeError: Engine.QueryInterface(target, IID_Position) is null ("Ranged",3108)@simulation/components/Attack.js:160 ([object Object],0)@simulation/components/BuildingAI.js:162 ([object Object])@simulation/components/Timer.js:93

I have a test scenario which allows fairly rapid reproduction of this error which I could upload if it would be helpful, but I would guess any map with a tower of a defeated player with an enemy in range should reproduce this.

[Philip Taylor]

I think the bug is that BuildingAI.prototype.FireArrows fails to check that the target entity is in range before calling cmpAttack.PerformAttack (which is documented to require a range check). Otherwise the target might die before being attacked, or might garrison inside another building so it doesn't have a valid position. In those cases OnRangeUpdate will remove the entity from the target list, but not instantly, and the building might try attacking the invalid target before that range update. FireArrows should be changed to verify each target before attacking them.

[FeXoR]

I'm not sure if I got that right but perhaps attacking a coordinate rather than an entity would help. When an entity (no matter if ranged/melee or unit/building) notices an enemy entity to attack (and is close enough to attack it) it saves the target entity and it's position. Then it starts the attack animation until the projectile goes off (ranged) or the damage would be dealt (melee). If the stored target entity "is still in the world" change the target coordinate to the actual position of the target entity, otherwise use the "old" coordinates as target of the projectile (ranged)/to deal the damage (melee). [It would be good if the units range (especially max range for melee and min range for ranged attacks) is NOT checked again at this point to avoid pointless chasing units. {It might still be good to lower the chance to hit based on the distance or to a fixed chance if the later coordinates are "out of range" to e.g. 50% or reduce the damage.}.] If an (enemy, otherwise extended friendly fire - more realistic) entities inner of a footprint (boundaries/collision radius) got hit by the coordinate actually deal the damage no matter if its the originally targeted unit or not.

This would also fix #1537 and infinite charging of 1 ranged vs 1 melee unit.

[sanderd17]

I'm not able to reproduce this anymore, and if it's still reproducible, I think #1960 would fix it as a side effect.