Differential D2135

Unit Motion - Improve behaviour around obstructions and unreachable goals
ClosedPublic
Actions

Authored by wraitii on Jul 29 2019, 9:27 PM.

Details

Reviewers

None

Group Reviewers

Restricted Owners Package

(Owns No Changed Paths)

Commits

rP22609: Unit Motion - Improve behaviour around obstructions and unreachable goals

Trac Tickets

#5545
#5547

Summary

As introduced in rP22566, units now reject paths that would take them farther from the goal than their current position.

This does a few things:

use the "pretend correct path" for short goals too. This is a better fix for #5545, since the position check now works correctly without needing to add a tricky check for path-vs-path distance.
simplify HandleObstructedMove to use ComputePathToGoal more. This means we occasionally compute a long path when stuck, which avoids the "horrible" bug in #5547 which I thought I didn't have a fix for. It's also slightly less hacky.
Move some common calls into functions for convenience.
make sure the short-path-waypoint-range-relaxing introduced in rP22526 doesn't happen for the last waypoint or things went awry easily.

This won't fix all instances of unit dancing, but it should improve most of them.

Test Plan

Ordre large groups of units too clump. Order large groups of units to move around static obstructions.

Diff Detail

Repository

rP 0 A.D. Public Repository

Lint

Automatic diff as part of commit; lint not applicable.

Unit

Automatic diff as part of commit; unit tests not applicable.

Event Timeline

wraitii created this revision.Jul 29 2019, 9:27 PM

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

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/268/display/redirect

Harbormaster completed remote builds in B8659: Diff 9171.Jul 29 2019, 9:33 PM

As said by elexis on irc:

"we should also reject paths if we have a path right now and our current path takes us closer to the goal than this new path." but why is there a new path if the old one is still valid?

Need to be careful to not reject paths that would unstuck the unit though, so this is probably trickier than I though.

Change the goals f the diff slightly, tackling both #5545 and #5547.

This does a few things:

use the "pretend correct path" for short goals too. This is a better fix for #5545, since the position check now works correctly without needing to add a tricky check for path-vs-path distance.
simplify HandleObstructedMove to use ComputePathToGoal more. This means we occasionally compute a long path when stuck, which avoids the "horrible" bug in #5547 which I thought I didn't have a fix for. It's also slightly less hacky.
Move some common calls into functions for convenience.
make sure the short-path-waypoint-range-relaxing introduced in rP22526 doesn't happen for the last waypoint or things went awry easily.