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

Expose an API for the Navigation mesh generator to register custom geometry providers and reduce its dependencies #5138

Closed
Zylann opened this issue Aug 10, 2022 · 1 comment
Milestone

Comments

@Zylann
Copy link

Zylann commented Aug 10, 2022

Describe the project you are working on

Terrain plugin(s) that make use of Godot servers directly

Describe the problem or limitation you are having in your project

NavigationMeshGenerator is a Godot core singleton which is used to generate a navmesh in a scene. The problem is, when you start creating nodes that use servers directly or behave in different ways (for example rendering without conventional geometry), this generator does not see them anymore. It then becomes impossible to make such nodes contribute some geometry to the navmesh with the Bake process. The only workaround is to create temporary proxy MeshInstance nodes which can then be recognized by the generator and cleaned up afterward, but it is cumbersome and uses unnecessary resources.

This problem already occurred in Godot itself:

  • GridMap is not a single MeshInstance with obvious geometry, it is a grid of many meshes. But instead of having an API to register its navigational generation part, a dependency to GridMap was added in the navigation module and it was done there.
  • HeightMapShape3D wasn't supported at all until this recent PR, but then instead of registering, it was also added directly to the navigation module.
  • CSG nodes from the csg module are a bit different from MeshInstance as well, but once again they were added to the navigation module
  • And so on...

This approach only works for core modules. It is also not modular because even if there are #ifdefs, the module still had to get modified over and over in order to account for new nodes. But the larger issue is that this obviously cannot work for third-party modules, plugins and extensions. For example, users generate their navmesh, and keep finding that my plugin isn't taken into account, because they have to first generate a temporary proxy mesh with an option of the plugin. It should be possible to have this work without this extra step.

Nodes can still add geometry later by calling into the navigation server, but then, what is a good time to do it, if not when the user triggers Bake? (it is expensive to generate especially when there is no actual geometry in the first place) How would it account for everything else in the scene? And what are all these node types doing in navigation_mesh_generator.cpp if they could do that too? They implement a given use case, and that use case should be exposed to be extended properly.

Describe the feature / enhancement and how it helps to overcome the problem or limitation

Expose an API available to modules, plugins and extensions, to register node types that contribute to navigation. At the very least, this would allow non-core custom node types to properly do their logic all in the same baking process, without having to create temporary proxies.

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

This is where every node type is checked for:
https://github.com/godotengine/godot/blob/11abffbf1282d457f966741f14aa5a5793f3c208/modules/navigation/navigation_mesh_generator.cpp#L164

The idea is to instead register a callback or a class per node type which will then be called with similar arguments (or a context object to make future changes easier and make it compatible with the script API). The logic can then be implemented next to the definition of respective node types.

If this enhancement will not be used often, can it be worked around with a few lines of script?

Workarounds I've used so far require more than a few lines of code to expose an option in the editor to create a proxy mesh, which is an extra step for users each time they need to update a scene-wide navmesh.

Is there a reason why this should be core and not an add-on in the asset library?

It affects an existing core feature, and possibly allows to remove dependencies from it.

@Zylann Zylann changed the title Expose an API for the Navigation mesh generator to remove its dependencies and allow proper customization Expose an API for the Navigation mesh generator to register geometry providers and remove its dependencies Aug 10, 2022
@Zylann Zylann changed the title Expose an API for the Navigation mesh generator to register geometry providers and remove its dependencies Expose an API for the Navigation mesh generator to register geometry providers and reduce its dependencies Aug 10, 2022
@Zylann Zylann changed the title Expose an API for the Navigation mesh generator to register geometry providers and reduce its dependencies Expose an API for the Navigation mesh generator to register custom geometry providers and reduce its dependencies Aug 10, 2022
@akien-mga akien-mga added this to the 4.3 milestone Apr 19, 2024
@akien-mga
Copy link
Member

Implemented by godotengine/godot#90876.

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

Successfully merging a pull request may close this issue.

3 participants