HomeWildfire Games

Use UnitMotion to predict target position in Attack.js to prevent 'dancing'
AuditedrP24701

Description

Use UnitMotion to predict target position in Attack.js to prevent 'dancing'

Attack.js can use UnitMotion to calculate the position of the unit in the future, accounting for odd movements such
as zigzags, turnarouds and early stops (to the extent of the current order).
This improves the resilience of units against the 'dancing' trick.

The linear interpolation is kept as a failsafe and to avoid an edge case in the new prediction code.

Patch by: bb

Refs #5106

Differential Revision: https://code.wildfiregames.com/D3225

Event Timeline

bb raised a concern with this commit.Jan 19 2021, 5:19 PM
bb added a subscriber: bb.

You messed up the height. Prior to the commit the 3d position was linearly extrapolated, so we assumed a constant slope. The new calculation uses the actual position, so we better use the actual height at that position which can be different at different positions. You now assume a horizontal surface. You do need to go through the fuzz of computing the height from the position component (see earlier diff), but add it manually outside the UM.

Arrows will now land inside a mountain if the target is walking up/down a slope.

This commit now has outstanding concerns.Jan 19 2021, 5:19 PM
wraitii added a comment.EditedJan 19 2021, 5:24 PM
In rP24701#48020, @bb wrote:

You messed up the height. Prior to the commit the 3d position was linearly extrapolated, so we assumed a constant slope.

This is wrong, actually. The projectile's target height was manually reset to targetPosition.y, so the code has no new bug (see L569).
I did the change on purpose to match the current behaviour, though I'll agree it seems kinda dumb.

I'll fix it in a separate diff.

Edit: ah, I guess you mean that you fixed it in an intermediate diff & I rebroke it though, which would be correct, regardless also in SVN.

Edit2 -> https://code.wildfiregames.com/D3425

Stan added a subscriber: Stan.Jan 19 2021, 7:38 PM

3>c:\dev\trunk\source\simulation2\components\icmpunitmotion.cpp(116): warning C4373: 'CCmpUnitMotionScripted::EstimateFuturePosition' : la fonction virtuelle se substitue à 'ICmpUnitMotion::EstimateFuturePosition', les versions précédentes n'ont pas été substituées lorsque les paramètres ne différaient que par les qualificateurs const/volatile
3>c:\dev\trunk\source\simulation2\components\icmpunitmotion.h(123): note: voir la déclaration de 'ICmpUnitMotion::EstimateFuturePosition'
2>SoundGroup.cpp

/ps/trunk/source/simulation2/components/ICmpUnitMotion.cpp
116

missing const.

virtual CFixedVector2D EstimateFuturePosition(const fixed dt) const
wraitii requested verification of this commit.Jan 27 2021, 4:58 PM
This commit now requires verification by auditors.Jan 27 2021, 4:58 PM
Stan added inline comments.Apr 14 2021, 10:30 AM
/ps/trunk/binaries/data/mods/public/simulation/components/UnitMotionFlying.js
318

Typo? EstimateFuturePosition #6141

wraitii added inline comments.Apr 14 2021, 10:35 AM
/ps/trunk/binaries/data/mods/public/simulation/components/UnitMotionFlying.js
318

I think I renamed it at some point and forgot to fix this instance.

bb accepted this commit.Dec 25 2021, 6:28 PM

code reads like being fixed. Didn't test

All concerns with this commit have now been addressed.Dec 25 2021, 6:28 PM