Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix pathfinding #7522

Merged
merged 3 commits into from
Jan 26, 2025
Merged

Fix pathfinding #7522

merged 3 commits into from
Jan 26, 2025

Conversation

glebm
Copy link
Collaborator

@glebm glebm commented Nov 10, 2024

The previous implementation didn't behave quite like A-* is supposed to.

After trying to figure out what's causing it and giving up, I've reimplemented it in a straightforward manner.
Now it seems to work a lot better.

Also increases maximum player path length to 100 steps.
We still only store the first 25 steps in the save file for vanilla compatibility.

Why draft?

  1. Need to check how much smarter monsters are with this.
  2. New demo needs to be recorded.

Benchmarking shows that the new algorithm is about twice as fast:

Comparing build-reld-path-bench/path_benchmark to build-reld/path_benchmark
Benchmark                              Time             CPU      Time Old      Time New       CPU Old       CPU New
-------------------------------------------------------------------------------------------------------------------
BM_SinglePath_pvalue                 0.0002          0.0002      U Test, Repetitions: 10 vs 10
BM_SinglePath_mean                  -0.3270         -0.3271          2917          1963          2917          1963
BM_SinglePath_median                -0.3247         -0.3247          2907          1963          2907          1963
BM_SinglePath_stddev                -0.8035         -0.8067            15             3            15             3
BM_SinglePath_cv                    -0.7080         -0.7127             0             0             0             0
BM_Bridges_pvalue                    0.0002          0.0002      U Test, Repetitions: 10 vs 10
BM_Bridges_mean                     -0.7641         -0.7641         59742         14095         59736         14094
BM_Bridges_median                   -0.7617         -0.7618         59083         14077         59077         14074
BM_Bridges_stddev                   -0.9810         -0.9810          2719            52          2719            52
BM_Bridges_cv                       -0.9194         -0.9195             0             0             0             0
BM_NoPath_pvalue                     0.0002          0.0002      U Test, Repetitions: 10 vs 10
BM_NoPath_mean                      -0.8406         -0.8406        127807         20371        127792         20368
BM_NoPath_median                    -0.8406         -0.8406        127556         20330        127540         20328
BM_NoPath_stddev                    -0.9138         -0.9141          1109            96          1111            95
BM_NoPath_cv                        -0.4592         -0.4609             0             0             0             0
OVERALL_GEOMEAN                     -0.4405         -0.4405             0             0             0             0

If we go ahead with this fix, we can consider increasing the max path length for the player in a follow-up PR.

Here is what a somewhat long path looks like:

path

We may want to tune it further to prefer diagonal steps first more reliably.

@glebm glebm marked this pull request as draft November 10, 2024 13:01
test/path_test.cpp Outdated Show resolved Hide resolved
test/path_test.cpp Outdated Show resolved Hide resolved
@glebm glebm force-pushed the path-fix branch 8 times, most recently from d8360c2 to e022d20 Compare November 11, 2024 20:10
@kphoenix137
Copy link
Collaborator

Does this PR also fix #6103 ?

@glebm
Copy link
Collaborator Author

glebm commented Nov 15, 2024

Probably not, this fixes the pathfinding algorithm but doesn't change how stuns are handled.

@glebm glebm force-pushed the path-fix branch 6 times, most recently from 5bd46c9 to e40a718 Compare January 26, 2025 12:46
glebm added 2 commits January 26, 2025 13:36
The previous implementation didn't behave quite like A-* is supposed to.

After trying to figure out what's causing it and giving up,
I've reimplemented it in a straightforward manner.
Now it seems to work a lot better.

Also increases maximum player path length to 100 steps.
We still only store the first 25 steps in the save file for vanilla
compatibility.
@glebm glebm marked this pull request as ready for review January 26, 2025 13:36
@glebm
Copy link
Collaborator Author

glebm commented Jan 26, 2025

@ephphatha @qndel @AJenbo Ready for review and playtesting!

@AJenbo AJenbo merged commit 5b66f12 into diasurgical:master Jan 26, 2025
23 checks passed
@glebm glebm deleted the path-fix branch January 26, 2025 18:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants