Differential D1897

UnitMotion - Split Move() into several functions
ClosedPublic
Actions

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

Tags

None

Subscribers

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

Harbormaster failed remote builds in B7497: Diff 8036!May 15 2019, 10:45 PM

wraitii added parent revisions: D1896: UnitMotion - move TryGoingStraight before the core of the 'Move' logic, D1893: Drop an indentation level in UnitMotion.May 15 2019, 10:46 PM

wraitii mentioned this in D1898: UnitMotion - move "possibly at destination" checks earlier in Move().May 15 2019, 10:49 PM

wraitii added a child revision: D1898: UnitMotion - move "possibly at destination" checks earlier in Move().

Updated

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

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

Harbormaster failed remote builds in B7538: Diff 8076!May 19 2019, 5:21 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

Harbormaster failed remote builds in B7586: Diff 8124!May 25 2019, 4:33 PM

Silier added a subscriber: Silier.Jun 9 2019, 7:10 PM

Silier 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 mentioned this in rP22363: UnitMotion - move TryGoingStraight before the core of the 'Move' logic.Jun 10 2019, 9:35 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

Harbormaster completed remote builds in B7887: Diff 8427.Jun 10 2019, 11:27 PM

This revision was not accepted when it landed; it landed in state Needs Review.Jun 11 2019, 7:27 PM

Closed by commit rP22364: UnitMotion - Split Move() into several functions (authored by wraitii). · Explain Why

This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Path

Size

ps/

trunk/

source/

simulation2/

components/

CCmpUnitMotion.cpp

373 lines

Diff 8430

ps/trunk/source/simulation2/components/CCmpUnitMotion.cpp

Loading...