Page MenuHomeWildfire Games

Hierarchical pathfinder: fix an issue with regions and some touch-ups (D53 outtake)
ClosedPublic

Authored by wraitii on Apr 19 2019, 6:50 PM.

Details

Reviewers
None
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Restricted Owners Package(Owns No Changed Paths)
Commits
rP22218: Hierarchical pathfinder: fix an issue with regions and some touch-ups
Summary

This switches from using m_NumRegions to a vector of region IDs because there is actually no guarantee that our regions occupy the n lowest IDs (and in fact it's quite likely they don't).

This might actually mean that D1830 crashes on svn because I'm switching to .at() instead of []

See inline comment for a currently silently buggy use.

(D53 outtake)

Test Plan

To verify the claim above: add a printf in InitRegions

Otherwise review, run the game.

Diff Detail

Event Timeline

wraitii created this revision.Apr 19 2019, 6:50 PM

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

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

wraitii added inline comments.Apr 19 2019, 6:51 PM
source/simulation2/helpers/HierarchicalPathfinder.cpp
734

Because of the above issue, this actually very well could miss some real regions and add some garbage regions. I'm not sure what consequences it had in practice.

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

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

wraitii updated this revision to Diff 7781.Apr 20 2019, 9:56 AM

Missed a bit in my rebases.

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

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

wraitii updated this revision to Diff 7808.Apr 22 2019, 6:13 PM

Reupload after dependent commits have been merged.

I've added tests that showcase the problem: there are only 2 regions, but one has ID 4, so using m_NumRegions (which is 2 if you run that on svn) is broken.

source/simulation2/helpers/HierarchicalPathfinder.cpp
734

Seems the only consequences are "the JPS pathfinder sometimes picked a sub-optimal nearest navcell when we started on an impassable cell". Which I guess is why we never noticed, as that's a pretty rare use case.

Regardless, buggy.

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

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

This revision was not accepted when it landed; it landed in state Needs Review.Apr 24 2019, 9:02 PM
This revision was automatically updated to reflect the committed changes.