HomeWildfire Games

Ensure units do get in attacking range since that range can change over time…
Needs VerificationrP22429

Description

Ensure units do get in attacking range since that range can change over time because of elevation differences.

This is a partial revert of rP22351, which skipped the "MoveToTargetAttackingRange" in APPROACHING. I (incorrectly) assumed that the original order was still perfectly fine, but in fact the attacker's max range may have changed as that depends on the relative elevation between attack and target - and so the original order might never get us in range!
This was introduced originally in rP13626.

Add a comment to clarify this.

Further, this makes sure UnitMotion still is aware that it has a target even if it is in range from the beginning, as that could lead
to stuckiness (and did when chasing sometimes). This was done in D1984 anyways.

Fixes #5478.

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

Event Timeline

elexis raised a concern with this commit.Jul 6 2019, 10:50 AM
elexis added a subscriber: elexis.
../../../source/simulation2/components/CCmpUnitMotion.cpp: In member function ‘virtual bool CCmpUnitMotion::MoveToTargetRange(entity_id_t, entity_pos_t, entity_pos_t)’:
../../../source/simulation2/components/CCmpUnitMotion.cpp:1524:17: warning: variable ‘circleDistance’ set but not used [-Wunused-but-set-variable]
 1524 |    entity_pos_t circleDistance = (pos - CFixedVector2D(obstruction.x, obstruction.z)).Length() - circleRadius;
      |                 ^~~~~~~~~~~~~~
../../../source/simulation2/components/CCmpUnitMotion.cpp:1525:17: warning: variable ‘previousCircleDistance’ set but not used [-Wunused-but-set-variable]
 1525 |    entity_pos_t previousCircleDistance = (pos - CFixedVector2D(previousObstruction.x, previousObstruction.z)).Length() - circleRadius;
      |                 ^~~~~~~~~~~~~~~~~~~~~~
/ps/trunk/source/simulation2/components/CCmpUnitMotion.cpp
1553

circleDistance and previousCircleDistance unused now, the comment seems obsoleted too (and what more I don't know yet)

This commit now has outstanding concerns.Jul 6 2019, 10:50 AM
wraitii added inline comments.Jul 6 2019, 10:51 AM
/ps/trunk/source/simulation2/components/CCmpUnitMotion.cpp
1553

If you mean the above comment, it's not. We're still going to a circle circumscribed by the square.

elexis added inline comments.Jul 6 2019, 10:53 AM
/ps/trunk/source/simulation2/components/CCmpUnitMotion.cpp
1553

the previous "distance < maxRange" check

Which check?

wraitii added inline comments.Jul 6 2019, 11:01 AM
/ps/trunk/source/simulation2/components/CCmpUnitMotion.cpp
1553

I think this referred to L1530.
You're right though, it looks weird. Also this comment makes us take this branch always (that has no negative consequence for existing unitMotion).

Again, this is cleaned up in D1984.

wraitii requested verification of this commit.Jul 19 2019, 2:44 PM
This commit now requires verification by auditors.Jul 19 2019, 2:44 PM
Angen added a subscriber: Angen.Jul 24 2019, 10:42 AM
Angen added inline comments.
/ps/trunk/binaries/data/mods/public/simulation/components/UnitAI.js
4355

2*max*(h + max/2) = 2*max*h + 2*max*max/2 = 2*max*h + max^2

Angen added inline comments.Jul 24 2019, 10:45 AM
/ps/trunk/binaries/data/mods/public/simulation/components/UnitAI.js
4355

sorry got confused :) its ok