Page MenuHomeWildfire Games

Adapt unitMotion to edge-edge distance checks
ClosedPublic

Authored by wraitii on Jun 11 2019, 9:43 PM.

Details

Reviewers
bb
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Commits
rP22422: Adapt unitMotion to edge-edge distance checks.
Summary

D981/rP22345 changed IsInXChecks to use edge-to-edge distance instead of centre-to-edge, which broke UnitMotion's min-range movement, which assumed distance to the center.

Since units are treated square, the diagonal point may be closer to the target than the "real" clearance by a factor √2, so the delta between minimum range and maximum range should be at least (√2 - 1) * clearance to be safe in all situations (this is generally not a problem for regular units which have a clearance of 0.8, but could be one for catapults or elephants). Note that this doesn't actually apply if the min-range is 0.

NB: since the long-range pathfinder brings units to navcell centres, it can in some cases to actually bring the unit in correct range.
This is alleviated by TryGoingStraightToEntity, but moving with point targets might still fail.
This issue explains why whales will sometimes get stuck during roaming, and also why some units may fail to stop from "Moving" state.
However, D1982 makes TryGoingStraightToEntity work with point targets, and D1983 will improve logic to use the short-range pathfinder when we get close to the target and are out of waypoint, which should fix this issue - until the long-range pathfinder's MakeGoalReachable is improved in that respect.

Test Plan

This is a bit tricky to test. The unitMotion min_range buffer should probably be checked logically. Fleeing behaviour can be used to get some data, also walking away from foundations.

Diff Detail

Event Timeline

wraitii created this revision.Jun 11 2019, 9:43 PM

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/differential/1692/display/redirect

wraitii updated this revision to Diff 8595.Jun 24 2019, 5:57 PM
wraitii retitled this revision from Support infinite max range checks again in IsInXRange // Adapt unitMotion when moving to Support infinite max range checks again in IsInXRange // Adapt unitMotion to edge-edge distance checks.

Reworked slightly with better comments and no navcell fix which should not be needed. I believe the GOAL_DELTA was intended to do something slightly similar at some point, and is also removed.

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/differential/1806/display/redirect

Stan added a subscriber: Stan.Jun 24 2019, 6:28 PM
Stan added inline comments.
source/simulation2/components/CCmpObstructionManager.cpp
841 ↗(On Diff #8595)

Maybe make that a constant like INVALID_PLAYER ?

wraitii updated this revision to Diff 8658.Jun 30 2019, 6:19 PM
wraitii retitled this revision from Support infinite max range checks again in IsInXRange // Adapt unitMotion to edge-edge distance checks to Adapt unitMotion to edge-edge distance checks.
wraitii edited the summary of this revision. (Show Details)

Cleaned up of the infinite range thing which is now in D1980.
Updated summary.

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/differential/1854/display/redirect

wraitii updated this revision to Diff 8677.Jul 1 2019, 8:43 PM

There were a few things I didn't understand correctly, not it works properly.

wraitii added inline comments.Jul 1 2019, 8:45 PM
source/simulation2/components/CCmpUnitMotion.cpp
1617

We can remove GOAL_DELTA here since it's edge-to-edge and this asks the centre of the unit to be at max range (likewise above), so no need for a delta.

1622

m_Clearance here is actually required or the pathing becomes odd as units try going impossibly close to the obstruction.

This does mean max ranges must be at least TERRAIN_TILE_SIZE)/16 or the unit might never be in range.

wraitii updated this revision to Diff 8679.Jul 1 2019, 8:49 PM

Use m_Clearance instead of cmpObstruction::GetUnitRadius since the latter gets it from m_Clearance, making this kind of recursive.

Vulcan added a comment.Jul 1 2019, 8:51 PM

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/differential/1866/display/redirect

Vulcan added a comment.Jul 1 2019, 8:57 PM

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/differential/1867/display/redirect

wraitii updated this revision to Diff 8681.Jul 1 2019, 9:04 PM

This time ready to commit.

Vulcan added a comment.Jul 1 2019, 9:43 PM

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/differential/1869/display/redirect

This revision was not accepted when it landed; it landed in state Needs Review.Jul 1 2019, 9:50 PM
This revision was automatically updated to reflect the committed changes.