Page MenuHomeWildfire Games

Unit Motion - Remove m_FinalGoal
ClosedPublic

Authored by wraitii on May 15 2019, 11:06 PM.

Details

Reviewers
None
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Commits
rP22452: Unit Motion - Remove m_FinalGoal in favour of recomputing when necessary.
Summary

Yet another case of redundant state removal cleaning up code and letting us make things better in the future.

UnitMotion stores a target (see D1887), which isn't equivalent to a pathing goal. Several functions need to be able to compute / update a Goal from said Motion Request. Instead of doing this weirdly, we'll just recompute a goal from the motion request whenever necessary.

This makes the code more flexible since one doesn't have to be careful about whether one uses the move request data or the goal data, and it lets one update the goal more sanely. And it removes state.

This is a slightly extensive change as I merge the "goal computation" logic from MoveToTargetRange and MoveToPointRange together - needs to be reviewed.

Test Plan

Compile. Review UpdateMotionGoal. Test on the pathfinding integration test map.

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.May 15 2019, 11:06 PM

Build failure - The Moirai have given mortals hearts that can endure.

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

(this revision probably includes a few unrelated changes... I'll clean things up later)

Stan added a subscriber: Stan.May 16 2019, 12:06 PM
Stan added inline comments.
source/simulation2/components/CCmpUnitMotion.cpp
617 ↗(On Diff #8041)

Definition could be put out of the class, no ?

1269 ↗(On Diff #8041)

ternary ?

1330 ↗(On Diff #8041)

spaces between math operators.

wraitii updated this revision to Diff 8081.May 19 2019, 4:58 PM
wraitii marked 3 inline comments as done.

Updated

Stan added inline comments.May 19 2019, 5:01 PM
source/simulation2/components/CCmpUnitMotion.cpp
1269 ↗(On Diff #8041)

I guess that's a no ^^

wraitii added inline comments.May 19 2019, 5:08 PM
source/simulation2/components/CCmpUnitMotion.cpp
617 ↗(On Diff #8041)

could, but won't be called for anything else than UnitMotion, so better to keep it here.

1269 ↗(On Diff #8041)

meh, makes for a longer line.

Build failure - The Moirai have given mortals hearts that can endure.

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

wraitii updated this revision to Diff 8497.Jun 15 2019, 9:47 PM

Cleaned up of D1981, D1982, D1983 and D1984.

Build failure - The Moirai have given mortals hearts that can endure.

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

wraitii updated this revision to Diff 8822.Wed, Jul 10, 8:37 PM

Rebased for (hopefully) jenkins building it.

Build failure - The Moirai have given mortals hearts that can endure.

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

wraitii updated this revision to Diff 8825.Wed, Jul 10, 8:44 PM

Was wrong, didn't apply cleanly, this time it should run.

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

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

This revision was not accepted when it landed; it landed in state Needs Review.Wed, Jul 10, 8:58 PM
This revision was automatically updated to reflect the committed changes.