Page MenuHomeWildfire Games
Differential D1984

Combine Goal computation logic from MoveToPoint and MoveToTarget
ClosedPublic
Actions

Authored by wraitii on Jun 15 2019, 9:46 PM.

Details

Reviewers
None
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Commits
rP22450: Unit Motion - combine Goal computation logic from MoveToPoint and MoveToTarget
Summary

MoveToPoint and MoveToTarget both compute a goal from a move request. They can be combined.

This allows using ComputeGoal everywhere a goal is needed, which lets us remove m_FinalGoal downstream.

NB: this commit is more dangerous than others since mistakes here could lead to stuck units easily.

Test Plan

Review the merge, compile, test in game.

Diff Detail

Event Timeline

wraitii created this revision.Jun 15 2019, 9:46 PM
wraitii mentioned this in D1902: Unit Motion - Remove m_FinalGoal.
wraitii added a child revision: D1902: Unit Motion - Remove m_FinalGoal.
wraitii added a parent revision: D1983: UnitMotion - clarify what CheckTargetMovement is doing, improve the logic..
Vulcan added a comment.Jun 15 2019, 10:30 PM

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

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

Harbormaster failed remote builds in B7956: Diff 8496!Jun 15 2019, 10:30 PM
Stan added a subscriber: Stan.Jun 15 2019, 11:52 PM
Stan added inline comments.
source/simulation2/components/CCmpUnitMotion.cpp
1193

Can be moved at line 1214, right?

wraitii mentioned this in D2035: Ensure units do get in attacking range since that range can change over time because of elevation differences.Jul 3 2019, 7:23 PM
wraitii mentioned this in rP22429: Ensure units do get in attacking range since that range can change over time….Jul 3 2019, 8:05 PM
wraitii updated this revision to Diff 8783.Jul 8 2019, 8:29 PM

Rebased for jenkins (maybe - might conflict)

Vulcan added a comment.Jul 8 2019, 10:15 PM

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

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

Harbormaster failed remote builds in B8250: Diff 8783!Jul 8 2019, 10:15 PM
wraitii updated this revision to Diff 8804.Jul 9 2019, 10:00 PM

This time it should patch and work correctly.

wraitii added a comment.Jul 9 2019, 10:01 PM

I'll probably add some tests before committing this

Vulcan added a comment.Jul 9 2019, 10:19 PM

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

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

Harbormaster completed remote builds in B8270: Diff 8804.Jul 9 2019, 10:19 PM
wraitii added inline comments.Jul 10 2019, 10:41 AM
source/simulation2/components/CCmpUnitMotion.cpp
1283

I think this bit here might be related to #5454 and I think it could safely be removed, I'll test.

wraitii added inline comments.Jul 10 2019, 11:56 AM
source/simulation2/components/CCmpUnitMotion.cpp
1283

er actually no because this is max range, not min range. Still it might be removed safely.

wraitii added a comment.Jul 10 2019, 8:29 PM

Rescinding the test comment, it's quite annoying to test a component right now, particularly when one wants to test things that aren't exposed to the interface. That _would_ require additional work which I might do later, but not right now.

source/simulation2/components/CCmpUnitMotion.cpp
1283

As said on the ticket, turned out to be completely unrelated.
The reason why this was there - I believe - is that units aren't supposed to go to building edges directly since that's likely to make the long-range pathfinder choke. However since I'm now adding the clearance, it can safely be removed.

It was added in rP7484.

wraitii updated this revision to Diff 8820.Jul 10 2019, 8:31 PM

Comment touch-ups.

Vulcan added a comment.Jul 10 2019, 8:36 PM

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

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

Harbormaster completed remote builds in B8281: Diff 8820.Jul 10 2019, 8:36 PM
This revision was not accepted when it landed; it landed in state Needs Review.Jul 10 2019, 8:42 PM
Closed by commit rP22450: Unit Motion - combine Goal computation logic from MoveToPoint and MoveToTarget (authored by wraitii). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathSize
source/
simulation2/
components/
376 lines

Diff 8496

View Options

source/simulation2/components/CCmpUnitMotion.cpp

Loading...
Wildfire Games · Phabricator