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

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

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
729 ↗(On Diff #7776)

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 ↗(On Diff #7808)

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.