Page MenuHomeWildfire Games

UnitMotion - Split Move() into several functions
ClosedPublic

Authored by wraitii on May 15 2019, 10:44 PM.

Details

Reviewers
None
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Commits
rP22364: UnitMotion - Split Move() into several functions
Summary

Move() is generally 4 parts:

  • Moving
  • Updating our state
  • Handling obstructed moves
  • Checking if we are at destination.

These can all be put in their own functions, clarifying logic and making it harder to make mistakes. D1893 and D1896 make it trivial.

Test Plan

Compile.

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, 10:44 PM

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

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

wraitii updated this revision to Diff 8076.May 19 2019, 4:50 PM

Updated

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

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

wraitii updated this revision to Diff 8124.May 25 2019, 4:33 PM

Rebased

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

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

Angen added a subscriber: Angen.Jun 9 2019, 7:10 PM
Angen added inline comments.
source/simulation2/components/CCmpUnitMotion.cpp
853 ↗(On Diff #8124)

please add space after pos.X,

959 ↗(On Diff #8124)

maybe use negation to return true as early return and you do not need if {} else {}

985 ↗(On Diff #8124)

negation for early return and remove if else ?

1010 ↗(On Diff #8124)

It does not looks like obstruction required handling here, we just stopped as we are close no?

wraitii updated this revision to Diff 8427.Jun 10 2019, 9:39 PM

Rebased for committing (tomorrow maybe)

wraitii added inline comments.Jun 10 2019, 9:41 PM
source/simulation2/components/CCmpUnitMotion.cpp
1010 ↗(On Diff #8124)

ClosEnoughFromDestinationToStop is more liberal than the range checks.

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

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

This revision was not accepted when it landed; it landed in state Needs Review.Jun 11 2019, 7:27 PM
This revision was automatically updated to reflect the committed changes.