Page MenuHomeWildfire Games

Move the Vertex Pathfinder to its own class
ClosedPublic

Authored by wraitii on Wed, May 1, 10:52 AM.

Details

Reviewers
None
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Commits
rP22253: Move the Vertex Pathfinder to its own helper class
Summary

The vertex pathfinder was still only a separate file from the Pathfinder component, unlike LongPathfinder which got its own class a long time ago.

This moves it to its own helper VertexPathfinder, which gets us ready for D14. Some struct definitions need to be moved around.
I think CCmpPathfinder_Common.h will be mergeable with CCmpPathfinder.cpp (or just renamed) then.

(D14 outtake).

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

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

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

wraitii updated this revision to Diff 7926.Wed, May 8, 12:27 PM

Cleaned up of the Async renaming as this is going to be needed for more upstream revisions and I want to commit it.

source/simulation2/components/CCmpPathfinder_Common.h
69 ↗(On Diff #7926)

Declaring the destructor here and having a default interface in the cpp file is necessary for std::unique_ptr to work, since otherwise the destructor tries to use an incomplete type and things don't work.

Bit annoying, but I think the forward declarations are worth it.

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

Linter detected issues:
Executing section Source...

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

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

source/simulation2/helpers/Pathfinding.h
|   1| /*·Copyright·(C)·2016·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2019" year instead of "2016"

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

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

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

Stan added a subscriber: Stan.Wed, May 8, 12:56 PM
Stan added inline comments.
source/simulation2/components/CCmpPathfinder.cpp
202 ↗(On Diff #7926)

Why not split this into two functions ? :)

source/simulation2/helpers/Pathfinding.h
32 ↗(On Diff #7926)

Why the async prefix, can't they be used a non async calls ?

source/simulation2/helpers/VertexPathfinder.cpp
853 ↗(On Diff #7926)

Safe to use a float here ?

865 ↗(On Diff #7926)

Can't this be made an inline function ?

887 ↗(On Diff #7926)

static_cast

935 ↗(On Diff #7926)

Edges ?

940 ↗(On Diff #7926)

This will trigger warnings for unreachable code.

source/simulation2/helpers/VertexPathfinder.h
48 ↗(On Diff #7926)

Dunno if we have a convention for multiple declaration on the same lines. it seems it's not done in the struct above though.

51 ↗(On Diff #7926)

Doxygen ?

56 ↗(On Diff #7926)

Same here

wraitii updated this revision to Diff 7929.Wed, May 8, 1:08 PM
wraitii marked 10 inline comments as done.

Fix tests failing compilation.

source/simulation2/components/CCmpPathfinder.cpp
202 ↗(On Diff #7926)

I don't want to introduce too many changes with this diff as I have another that decouples long and hierarchical pathfinder coming up. Cleanup can be done in another diff imo.

source/simulation2/helpers/Pathfinding.h
32 ↗(On Diff #7926)

See D1854

source/simulation2/helpers/VertexPathfinder.cpp
853 ↗(On Diff #7926)

debug code so yes

865 ↗(On Diff #7926)

As with most things, it could. Again, I'm only trying to move code here.

935 ↗(On Diff #7926)

probably good.

940 ↗(On Diff #7926)

Will comment out again then.

source/simulation2/helpers/VertexPathfinder.h
56 ↗(On Diff #7926)

All of the above >> Cleanup will be done later

Angen added a subscriber: Angen.Wed, May 8, 1:15 PM
Angen added inline comments.
source/simulation2/components/CCmpPathfinder.cpp
1 ↗(On Diff #7929)

2019 :)

source/simulation2/components/CCmpPathfinder_Common.h
1 ↗(On Diff #7929)

2019 :)

source/simulation2/components/ICmpPathfinder.h
1 ↗(On Diff #7929)

2019 :)

Vulcan added a comment.Wed, May 8, 1:32 PM

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

Linter detected issues:
Executing section Source...

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

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

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

source/simulation2/helpers/Pathfinding.h
|   1| /*·Copyright·(C)·2016·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2019" year instead of "2016"

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

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

This revision was not accepted when it landed; it landed in state Needs Review.Wed, May 8, 1:53 PM
This revision was automatically updated to reflect the committed changes.