Diffusion 0 A.D. Public Repository rP22429

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

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

Details

Auditors

elexis

Committed

wraitii

Jul 3 2019, 8:05 PM

Differential Revision

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

Parents

rP22428: Return Gathering to A23 behaviour - Make units try to gather near the target…

Branches

Unknown

Tags

Unknown

Build Status

Buildable 8180
Build 13327: Trigger Windows Autobuild
Build 13326: Post-Commit Build	Jenkins

Event Timeline

wraitii committed rP22429: Ensure units do get in attacking range since that range can change over time….Jul 3 2019, 8:05 PM

Harbormaster completed building B8180: rP22429: Ensure units do get in attacking range since that range can change over time….Jul 3 2019, 9:27 PM

../../../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.