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

Allow compiling out the navigation module #73031

Merged

Conversation

DarkKilauea
Copy link
Contributor

Fixes: #36091

Alternative to #69881

This is a stripped down version of #69881 which addresses the core problem of not being able to compile Godot without the navigation module, without the complexity of supporting GDExtension nor introducing a project setting for changing the server implementation.

It leaves the existing system for registering navigation servers alone and mainly updates main.cpp to check if no server has been registered and if so, swapping it out for a dummy implementation instead of throwing an error. Since there is always a dummy implementation compiled, the linker will always find an implementation of NavigationServer3D and won't fail to link if the module is missing.

There are a few housekeeping items also in this PR, mostly swaps to equivalent types for readability, additional comments, missing override keywords, and a missing virtual keyword.

All changes are backwards compatible, with no changes to ClassDB nor any changes to the C++ API shape. We'd like to include this change in 4.0 if possible.

@DarkKilauea DarkKilauea requested review from a team as code owners February 10, 2023 07:15
@Calinou
Copy link
Member

Calinou commented Feb 10, 2023

We'd like to include this change in 4.0 if possible.

4.0 is in RC stage, so we can't afford making potentially risky changes now. That said, if this is considered a bug fix, this can be merged for 4.1 and cherry-picked to a 4.0.x patch release.

@DarkKilauea
Copy link
Contributor Author

@Calinou That's fine, just keep in mind that anyone trying to compile out the navigation module will run into issues until a fix is merged. This effectively makes the navigation module mandatory.

Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

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

Seems fine.

@akien-mga akien-mga modified the milestones: 4.1, 4.0 Feb 11, 2023
@akien-mga akien-mga merged commit 6442cb9 into godotengine:master Feb 11, 2023
@akien-mga
Copy link
Member

Thanks!

@DarkKilauea DarkKilauea deleted the allow-no-navigation-module branch February 11, 2023 17:55
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
5 participants