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

Improve AStarGrid2D performance when jumping is enabled #93319

Merged

Conversation

10Drenth
Copy link
Contributor

fixes #91367

On sparse grids, enabling jumping can now increase performance 5x, which should be more than 10x faster than before, as jumping unintentionally used to decrease performance instead.

AStarGrid_jumping_Performance.zip

@10Drenth 10Drenth requested a review from a team as a code owner June 18, 2024 18:09
@AThousandShips AThousandShips modified the milestones: 4.3, 4.4 Jun 18, 2024
@10Drenth 10Drenth force-pushed the astar_jump_performance_improvement branch from 57a882d to c128a7b Compare June 18, 2024 18:19
@AThousandShips AThousandShips changed the title Improved AStarGrid2D performance when jumping is enabled Improve AStarGrid2D performance when jumping is enabled Jun 18, 2024
@AThousandShips AThousandShips changed the title Improve AStarGrid2D performance when jumping is enabled Improve AStarGrid2D performance when jumping is enabled Jun 18, 2024
@akien-mga akien-mga requested a review from Chaosus June 18, 2024 22:03
@10Drenth 10Drenth force-pushed the astar_jump_performance_improvement branch from c128a7b to 5de9ac3 Compare June 19, 2024 05:36
@Chaosus
Copy link
Member

Chaosus commented Jun 19, 2024

I see some inconsistencies with current pathfinding:

At this commit:
изображение

At master:
изображение

You can check it up using https://github.com/theshaggydev/the-shaggy-dev-projects/tree/main/projects/godot-4/astargrid2d

@10Drenth 10Drenth force-pushed the astar_jump_performance_improvement branch from 5de9ac3 to db18fe5 Compare June 21, 2024 11:12
@10Drenth 10Drenth requested a review from Chaosus June 21, 2024 11:55
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.

I think it's a correct now, great, thanks.

@akien-mga
Copy link
Member

I notice this PR and #85965 (both ready to merge) will conflict, so one would need to be merged first and the other rebased. But I wanted to take the opportunity to get both checked in parallel, I'm not familiar with AStar implementation but I wonder if there's overlap in what those two optimizations aim to solve, and thus whether they will combine well or not.

CC @ershn @godotengine/navigation

@10Drenth
Copy link
Contributor Author

10Drenth commented Sep 2, 2024

I wonder if there's overlap in what those two optimizations aim to solve

I'd say they do exist in separate spaces, but I'd have to check whether this specific optimization is still significantly faster compared to improved general performance from the other PR

@10Drenth
Copy link
Contributor Author

10Drenth commented Sep 2, 2024

On a second look, the linked PR appears to be completely separate. I am also not seeing any conflicts with their changes

@10Drenth 10Drenth force-pushed the astar_jump_performance_improvement branch from 446a7b7 to b5b0d06 Compare September 2, 2024 18:58
@10Drenth 10Drenth requested review from a team as code owners September 2, 2024 18:58
@akien-mga akien-mga removed request for a team September 3, 2024 11:06
@10Drenth 10Drenth force-pushed the astar_jump_performance_improvement branch from 5e0bcc7 to 202e197 Compare September 5, 2024 16:13
@akien-mga akien-mga merged commit 38447c5 into godotengine:master Sep 5, 2024
20 checks passed
@akien-mga
Copy link
Member

Thanks! And congrats for your first merged Godot contribution 🎉

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.

AStarGrid2D with jumping_enabled is significantly slower than with it disabled.
6 participants