Differential D1867

Decouple long and hierarchical pathfinders somewhat.
ClosedPublic
Actions

Authored by wraitii on May 8 2019, 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

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.

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.May 8 2019, 2:27 PM

source/simulation2/helpers/LongPathfinder.h
33 ↗	(On Diff #7931)	Wonder how I can test that.

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

Harbormaster completed remote builds in B7385: Diff 7931.May 8 2019, 2:55 PM

This revision was not accepted when it landed; it landed in state Needs Review.May 13 2019, 6:58 PM

Closed by commit rP22278: Decouple long and hierarchical pathfinders to an extent. (authored by wraitii). · Explain Why

This revision was automatically updated to reflect the committed changes.