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.

Sign up for GitHub

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

Jump to bottom

Fix pathfinding #7522

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Fix pathfinding #7522

wants to merge 2 commits into from
+342 −293

Conversation

glebm
Copy link
Collaborator

@glebm glebm commented Nov 10, 2024 •
edited
Loading

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.

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.2514         -0.2514          2909          2178          2908          2177
BM_SinglePath_median                -0.2520         -0.2521          2908          2175          2908          2175
BM_SinglePath_stddev                +2.0994         +2.0402             3             8             3             8
BM_SinglePath_cv                    +3.1401         +3.0611             0             0             0             0
BM_Bridges_pvalue                    0.0002          0.0002      U Test, Repetitions: 10 vs 10
BM_Bridges_mean                     -0.5083         -0.5083         59105         29061         59101         29058
BM_Bridges_median                   -0.4987         -0.4987         57890         29023         57888         29020
BM_Bridges_stddev                   -0.9664         -0.9661          3369           113          3368           114
BM_Bridges_cv                       -0.9316         -0.9311             0             0             0             0
BM_NoPath_pvalue                     0.0002          0.0002      U Test, Repetitions: 10 vs 10
BM_NoPath_mean                      -0.5579         -0.5579        127510         56373        127500         56368
BM_NoPath_median                    -0.5647         -0.5647        126943         55261        126934         55256
BM_NoPath_stddev                    +2.7160         +2.7191          1130          4199          1129          4199
BM_NoPath_cv                        +7.4054         +7.4123             0             0             0             0
OVERALL_GEOMEAN                     -0.4542         -0.4542             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.

@glebm glebm marked this pull request as draft November 10, 2024 13:01
ephphatha
ephphatha reviewed Nov 10, 2024
View reviewed changes
test/path_test.cpp Outdated Show resolved Hide resolved
ephphatha
ephphatha reviewed Nov 10, 2024
View reviewed changes
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 •
edited
Loading

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

@glebm
Add a pathfinding benchmark
1b50a15 
Example run:

```
$ tools/build_and_run_benchmark.py path_benchmark

--------------------------------------------------------
Benchmark              Time             CPU   Iterations
--------------------------------------------------------
BM_SinglePath       2905 ns         2905 ns       241038
BM_Bridges         65951 ns        65948 ns         9785
BM_NoPath         131397 ns       131390 ns         5247
```
@glebm
Fix pathfinding
8c41c5b 
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ephphatha ephphatha ephphatha left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@glebm @kphoenix137 @ephphatha