Page MenuHomeWildfire Games

Decouple long and hierarchical pathfinders somewhat.
ClosedPublic

Authored by wraitii on Wed, May 8, 1:56 PM.

Details

Reviewers
None
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Commits
rP22278: Decouple long and hierarchical pathfinders to an extent.
Summary

Following rP22253, this decouples the hierarchical pathfinder and the long pathfinder. The long pathfinder was the class owning the hierarchical pathfinder, which didn't really make sense and resulted in some interface awkwardness.

At the moment, the long pathfinder still needs to hierarchical pathfinder to compute paths (to make sure they are reachable). I think this could be decoupled too, but it was a few too many changes for this diff in my opinion.

Test Plan

Review changes.

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.Wed, May 8, 1:56 PM

Following this diff we could merge CcmpPathfinder_Common.h and CCmpPathfinder.cpp together if we wanted. Other headers should no longer need to include CCmpPathfinderCommon.

Stan added a subscriber: Stan.Wed, May 8, 2:07 PM

Can ComputePathAsync, and ComputeShoreGrid, ComputeShortPathAsync be const as well ?

source/simulation2/helpers/HierarchicalPathfinder.cpp
353 ↗(On Diff #7931)

No check for m_DebugOverlay ?

source/simulation2/helpers/LongPathfinder.h
23 ↗(On Diff #7931)

Not in alphabetical order.

26 ↗(On Diff #7931)

Why the include ?

In D1867#77202, @Stan wrote:

Can ComputePathAsync, and ComputeShoreGrid, ComputeShortPathAsync be const as well ?

ComputeShoreGrid possibly, but no to the other two as they are changing the CCmpPathfinder state (we're adding a new request onto the queue).

source/simulation2/helpers/HierarchicalPathfinder.cpp
353 ↗(On Diff #7931)

Will add.

source/simulation2/helpers/LongPathfinder.h
26 ↗(On Diff #7931)

Won't compile otherwise. There's some Overlay stuff down this class which need this.

Stan added inline comments.Wed, May 8, 2:27 PM
source/simulation2/helpers/LongPathfinder.h
33 ↗(On Diff #7931)

Wonder how I can test that.

Vulcan added a comment.Wed, May 8, 2:55 PM

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

Linter detected issues:
Executing section Source...

source/simulation2/components/CCmpAIManager.cpp
|   1| /*·Copyright·(C)·2018·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2019" year instead of "2018"

source/simulation2/helpers/LongPathfinder.h
|   1| /*·Copyright·(C)·2017·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2019" year instead of "2017"
Executing section JS...
Executing section cli...

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

This revision was not accepted when it landed; it landed in state Needs Review.Mon, May 13, 6:58 PM
This revision was automatically updated to reflect the committed changes.