Page MenuHomeWildfire Games

Move the Vertex Pathfinder to its own class
ClosedPublic

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

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.May 8 2019, 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

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.May 8 2019, 12:56 PM
Stan added inline comments.
source/simulation2/components/CCmpPathfinder.cpp
202

Why not split this into two functions ? :)

source/simulation2/helpers/Pathfinding.h
32

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

source/simulation2/helpers/VertexPathfinder.cpp
853

Safe to use a float here ?

865

Can't this be made an inline function ?

887

static_cast

935

Edges ?

940

This will trigger warnings for unreachable code.

source/simulation2/helpers/VertexPathfinder.h
48

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

Doxygen ?

56

Same here

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

Fix tests failing compilation.

source/simulation2/components/CCmpPathfinder.cpp
202

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

See D1854

source/simulation2/helpers/VertexPathfinder.cpp
853

debug code so yes

865

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

935

probably good.

940

Will comment out again then.

source/simulation2/helpers/VertexPathfinder.h
56

All of the above >> Cleanup will be done later

Silier added a subscriber: Silier.May 8 2019, 1:15 PM
Silier added inline comments.
source/simulation2/components/CCmpPathfinder.cpp
1

2019 :)

source/simulation2/components/CCmpPathfinder_Common.h
1

2019 :)

source/simulation2/components/ICmpPathfinder.h
1

2019 :)

Vulcan added a comment.May 8 2019, 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.May 8 2019, 1:53 PM
This revision was automatically updated to reflect the committed changes.