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 closest possible navigation path position #79004

Merged
merged 1 commit into from
Jul 9, 2023

Conversation

smix8
Copy link
Contributor

@smix8 smix8 commented Jul 3, 2023

Fixes closest possible navigation path position.

Fixes #78943.

This fixes two edge cases in the navigation mesh pathfinding related to unreachable target positions. It wouldn't affect pathfinding when the target position is reachable.

In the situation of an unreachable target position the users expect a path returned that goes from the closest point to the start position to the closest possible point reachable to the target position.

Without the fixes in this pr the path would in certain cases be either empty or end at the wrong position on an unexpected polygon.

The first edge case was that an empty path is returned as the fallback when no route is found at all. This is the case when the starting polygon has not a single connected edge, e.g. is a single, isolated triangle or other convex polygon. Since the pathfinding loop goes through connections it does not run to the code point where it would add path segments for a basic route, returning the empty path as the defined fallback. Now in this case an additional search of the start polygon faces is added.

The second edge case was if the target position is on an unreachable polygon and the start polygon happens to be the one with the closest possible position to the target position, it would still pick positions on other polygons. The reason for this is that the first polygon is never fully added to the path corridor, only the segment from the start position to the first connected edge is added. This means the start polygons entire faces are not considered in the search for the closest, possible position to the target. Only position along the line segment from start position to edge are. If another polygon would have a point closer than this segment the path would use the other polygon instead even if visually the user would see the start polygon being closer. Now in this case a full search of the start polygon faces is added.

@smix8 smix8 added bug topic:navigation cherrypick:4.0 cherrypick:4.1 Considered for cherry-picking into a future 4.1.x release labels Jul 3, 2023
@smix8 smix8 added this to the 4.x milestone Jul 3, 2023
@smix8 smix8 force-pushed the fix_closest_navpath_pos_4.x branch from 3251acf to 5792a22 Compare July 3, 2023 23:47
Fixes closest possible navigation path position.
@smix8 smix8 force-pushed the fix_closest_navpath_pos_4.x branch from 5792a22 to e5c24f7 Compare July 8, 2023 21:17
@smix8 smix8 modified the milestones: 4.x, 4.2 Jul 8, 2023
@smix8
Copy link
Contributor Author

smix8 commented Jul 9, 2023

I tested a few scenarios that I could think of with Godot Engine v4.2.dev.mono.custom_build.28e251c5c and it seems like everything works as expected.

I think we can merge this earlier in the 4.2 cycle so we have enough time before it gets a potential cherry-pick for older Godot4 versions. I don't expect much in terms of regressions because the PR only does something in "if" branches that already have no path to the target. As quoted razcore also tested it extensively and reported that it works as expected and fixes the issues.

@akien-mga akien-mga merged commit 1b8cbfe into godotengine:master Jul 9, 2023
@akien-mga
Copy link
Member

Thanks!

@smix8 smix8 deleted the fix_closest_navpath_pos_4.x branch July 9, 2023 23:22
@YuriSizov YuriSizov removed the cherrypick:4.1 Considered for cherry-picking into a future 4.1.x release label Jul 10, 2023
@YuriSizov
Copy link
Contributor

Cherry-picked for 4.1.1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Navigation recomputes incorrect longer path to unreachable target
3 participants