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

[4.x] Expose NavigationServer3D to GDExtension #69881

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

DarkKilauea
Copy link
Contributor

@DarkKilauea DarkKilauea commented Dec 10, 2022

Fixes: #36091

Supersedes: #65562

Adds the ability to register multiple implementations of NavigationServer3D and allow the user to select the one they want to use in their project.

Also exposes NavigationServer3D to GDExtension to make it possible for plugins to replace the default implementation.

NavigationServer2D is left alone, as it is a wrapper around NavigationServer3D and separating it will be a lot of work. Might be something worth considering for the future if there is demand.

Things still WIP:

  • _query_path_extension has a const pointer parameter whose type is not being picked up correctly.
  • Document NavigationServer3DExtension.
  • Test compiling out the Godot Navigation module.

@DarkKilauea
Copy link
Contributor Author

Fixed the issue with the missing type for the params parameter of _query_path_extension. Also added a dummy implementation of NavigationServer3D to stop Godot from crashing outright if the navigation module is disabled during compilation. I can now confirm that Godot builds and launches without the built-in navigation module.

Currently working on fixing an issue with the generated C++ header for NavigationServer3DExtensionPathQueryResult, then we should be pretty close to being ready for final reviews and a merge.

@DarkKilauea DarkKilauea marked this pull request as ready for review December 17, 2022 07:58
@DarkKilauea DarkKilauea requested review from a team as code owners December 17, 2022 07:58
@DarkKilauea
Copy link
Contributor Author

Marking this as ready for review. I still need to add some documentation, but the code should be ready to go now.

@DarkKilauea DarkKilauea force-pushed the nav-servers-be-free branch 3 times, most recently from 8e2ee3f to 2b2538e Compare December 17, 2022 23:21
@DarkKilauea
Copy link
Contributor Author

All documentation should be up to date now.

@DarkKilauea DarkKilauea force-pushed the nav-servers-be-free branch 4 times, most recently from f402294 to 7d93753 Compare December 22, 2022 04:55
@DarkKilauea
Copy link
Contributor Author

Rebased to account for changes from #67111

@DarkKilauea DarkKilauea force-pushed the nav-servers-be-free branch 2 times, most recently from b74acbe to 5efa8bd Compare January 8, 2023 00:47
@DarkKilauea
Copy link
Contributor Author

Rebased to account for changes to copyright headers and ProjectSettings::set_custom_property_info

@DarkKilauea DarkKilauea force-pushed the nav-servers-be-free branch 2 times, most recently from 965b25b to e62b8f3 Compare January 9, 2023 06:12
@akien-mga akien-mga modified the milestones: 4.0, 4.1 Feb 9, 2023
@DarkKilauea DarkKilauea force-pushed the nav-servers-be-free branch 2 times, most recently from d8bf8c3 to 0910dbe Compare February 10, 2023 06:20
@YuriSizov YuriSizov modified the milestones: 4.1, 4.x Jun 14, 2023
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.

Crash on startup with gdnavigation module disabled
4 participants