Page MenuHomeWildfire Games

Hierarchical Pathfinder - Add Global Regions / Optimise MakeGoalReachable (D53 outtake)
ClosedPublic

Authored by wraitii on Apr 20 2019, 10:19 AM.

Details

Reviewers
Itms
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Commits
rP22653: Hierarchical Pathfinder - Add Global Regions / Optimise MakeGoalReachable
Summary

Improve MakeGoalReachable using global regions, by leveraging the fact that we can easily know which goal regions we can reach. Particularly for point-goals, it becomes almost instant.
This does not change hashes.

(D53 outtake)

Test Plan

Review code, maths and logic.

Diff Detail

Event Timeline

wraitii created this revision.Apr 20 2019, 10:19 AM
wraitii retitled this revision from Hierarchical Pathfinder - Optimise MakeGoalReachable to Hierarchical Pathfinder - Optimise MakeGoalReachable (D53 outtake).
wraitii edited the summary of this revision. (Show Details)

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

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

wraitii updated this revision to Diff 7902.May 4 2019, 3:19 PM
wraitii retitled this revision from Hierarchical Pathfinder - Optimise MakeGoalReachable (D53 outtake) to Hierarchical Pathfinder - Add Global Regions / Optimise MakeGoalReachable (D53 outtake).
wraitii edited the summary of this revision. (Show Details)

Spliced from D1834 - this is the global region introduction + some MakeGoalReachable changes that don't actually changes hashes (assuming D1829).

I ran some profiling, on a giant map this adds about 2ms per update it seems, which all things considered is pretty low. The impact on MakeGoalReachable seems to be exactly the opposite, so it comes out a little on top of the new code.

Furthermore, this lets us do interesting things in the future, such as sending the global regions to the AI or calling MakeGoalReachable more often.

Vulcan added a comment.May 4 2019, 3:20 PM

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

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

Stan added a subscriber: Stan.May 4 2019, 6:05 PM
Stan added inline comments.
source/simulation2/helpers/HierarchicalPathfinder.cpp
373

Missing a capital letter at the beginning of the comment.

407

Same.

624

Here too.

653

Ternary ?

662

Aren't those last two variables always equal to 0 ?

wraitii updated this revision to Diff 8003.May 13 2019, 7:10 PM
wraitii marked 5 inline comments as done.

Rebased, clean up comments.

source/simulation2/helpers/HierarchicalPathfinder.cpp
662

NavcellCenter gives them values.

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

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

wraitii updated this revision to Diff 8107.May 25 2019, 10:24 AM

This should compile / pass tests

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

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

wraitii added a subscriber: Itms.May 25 2019, 10:47 AM

@Itms Dumbfounded by this build error... Maybe the diff script is trying to be too clever and the tests are compiled against an old header-file ? I can't reproduce this, it compiles just fine on my computer.

Itms added a comment.May 25 2019, 11:55 AM

I restarted the build, let's see.

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

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

Only happens in release mode... Can't think of anything that would make it fail and it works on my system, so I would appreciate if somebody else (@Stan , @vladislavbelov) could reproduce the bug (or not)

Itms requested changes to this revision.May 26 2019, 2:57 PM

There is your issue. I haven't fully reviewed the rest yet.

source/simulation2/components/tests/test_HierPathfinder.h
76

Should be u16, not size_t (mapSize is u16, the function declaration has this as well).

source/simulation2/helpers/HierarchicalPathfinder.cpp
433

> >
We should probably have a lint for this if we keep it as a style convention.

646

These shouldn't be inline, else the linker won't find them from another file 🙂 Inlining doesn't happen in debug mode to make following the call stack easier.

source/simulation2/helpers/HierarchicalPathfinder.h
205

> >

This revision now requires changes to proceed.May 26 2019, 2:57 PM
wraitii added inline comments.May 26 2019, 3:03 PM
source/simulation2/helpers/HierarchicalPathfinder.cpp
646

Ah, you're right 👍

wraitii updated this revision to Diff 8155.May 26 2019, 3:22 PM

>> > > > and remove the inline, this should run on debug and release then.

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

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

wraitii updated this revision to Diff 9305.Mon, Aug 12, 11:57 AM

Rebased for commit.

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

Linter detected issues:
Executing section Source...

source/simulation2/components/tests/test_HierPathfinder.h
|  25| class·TestHierarchicalPathfinder·:·public·CxxTest::TestSuite
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'classTestHierarchicalPathfinder:' is invalid C code. Use --std or --language to configure the language.

source/simulation2/helpers/HierarchicalPathfinder.h
|  34| ·*·is·defined·as·a·region.
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'template<...' is invalid C code. Use --std or --language to configure the language.
Executing section JS...
Executing section cli...

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

This revision was not accepted when it landed; it landed in state Needs Review.Mon, Aug 12, 3:19 PM
This revision was automatically updated to reflect the committed changes.