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 AStar2D, AStar3D, AStarGrid2D from not returning a path when the destination is disabled/solid even with allow_partial_path option #94246

Merged
merged 1 commit into from
Sep 16, 2024

Conversation

alliephante
Copy link
Contributor

@alliephante alliephante commented Jul 12, 2024

Previously all of the AStar _solve methods would terminate early if the destination point was solid/disabled. This caused confusion amongst users who were expecting the allow_partial_path option to still return a path even if the destination was solid/disabled. I fixed this by pulling that argument into _solve and only terminating early if it was also false, so a path is still returned if the user uses the option.

I noticed however while working on this that doing this significantly changes performance. I added a comment in the docs on the functions for these to warn users about this.

This will also happen whenever the end point is not reachable (aka the path is fully blocked off by solid/disabled points), however the code still executes even if allow_partial_path is false, as the pathfinding still has to run to determine if it is unreachable. I don't know if there is really a good way to fix this, as the only way to know a point is unreachable is traversing the entire graph anyway.
I'm not sure if this should also be mentioned somewhere in the documentation though.

Sample output of a microbenchmark when the destination is a solid/disabled point or unreachable

pathfinding took 15834 microseconds

Compared to when the point is reachable and not solid/disabled

pathfinding took 259 microseconds

This is with 1139 points in the graph.

Here is a sample project to test this:

astar_partial_test.zip

astar_partial_path_fix.webm

Closes #93409

@alliephante alliephante requested review from a team as code owners July 12, 2024 01:04
@Calinou Calinou added this to the 4.3 milestone Jul 12, 2024
@alliephante alliephante force-pushed the fix-astar-partial-path branch from 8cd3f8f to f13eabc Compare July 12, 2024 18:52
@akien-mga akien-mga changed the title Fix AStar2D, AStar3D, AStarGrid2D from not returning a path when the destination is disabled/solid even with allow_partial_path option Fix AStar2D, AStar3D, AStarGrid2D from not returning a path when the destination is disabled/solid even with allow_partial_path option Jul 31, 2024
@akien-mga akien-mga modified the milestones: 4.3, 4.4 Jul 31, 2024
* AStar2D, AStar3D and AStarGrid2D will now return a path when allow_partial_path is true even if the destination is a solid/disabled point.

# Conflicts:
#	core/math/a_star_grid_2d.cpp
#	core/math/a_star_grid_2d.h
@alliephante alliephante force-pushed the fix-astar-partial-path branch from f13eabc to c46b5af Compare September 13, 2024 03:32
@akien-mga akien-mga requested a review from Chaosus September 13, 2024 08:51
Copy link
Member

@Chaosus Chaosus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It makes sense, I guess

@akien-mga akien-mga merged commit 91553f5 into godotengine:master Sep 16, 2024
20 checks passed
@akien-mga
Copy link
Member

Thanks!

@AThousandShips AThousandShips added the cherrypick:4.3 Considered for cherry-picking into a future 4.3.x release label Oct 1, 2024
@ivoiv
Copy link

ivoiv commented Oct 14, 2024

@theashtronaut Thank you so much for your effort on this, it was driving me nuts having to disable all solids to calculate a path to them!

@Valar1415
Copy link

Hi! I'm fairly new to Godot and Github, could someone explain to me what's going on here? Am I viewing a suggestion for a feature that may be implemented into the Godot engine itself, or an asset feature that can be downloaded and merged with a project by anyone? If it's the latter, how do I actually merge - do I have to download Godot source code and edit it, then relaunch my project in the modified Godot version or something else?

I noticed that Godot 4.3 has "allow_partial_path" while 4,2 doesn't, but the 4.3 version still doesn't work like in the video, it returns an empty path when trying to click on a solid point.

@Zireael07
Copy link
Contributor

Zireael07 commented Nov 14, 2024

@Valar1415 This is an already merged feature. It should be in the most recent beta AFAIK.

If you want to use it from source, all you have to do is download the current Godot master and build it, and launch the project in Godot built from source. Since it's already merged you do not have to create a fork and merge into it

@Valar1415
Copy link

@Zireael07 so in my Godot 4.3 version this feature should work like in the video? Currently it doesn't and if it should, I should check that I haven't set up my function wrong.

@Zireael07
Copy link
Contributor

Godot 4.3 is from August, and this was merged in September. Try a beta version - afaict the most recent one is from October.

@Valar1415
Copy link

Gotcha, that should be it. Tyvm for the help!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug cherrypick:4.3 Considered for cherry-picking into a future 4.3.x release topic:navigation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

AStarGrid2D allow_partial_path returning no path when there should be
9 participants