Bake navigation meshes by groups as alternative to NavigationMeshInstance-Children

[dertom95]

Atm to create a navigation-mesh you have two options:
a) create it manually i.e. in blender (or let blender create it...) and export it with "-navmesh" postfix
b) create Navigation-Node with an NavigationMeshInstance as child, and add all mesh-nodes that should be taken into account to the NavigationMeshInstance.

a) works as intended. (actually I just fixed blender to create navigation-meshes from multiple objects. not sure when this will make it's way into master, but this is another story ;) )
b) also works as intended (but the gizmo mesh is upside down)

Using one mesh in multiple navigation-meshes is not possible. E.g. you would want to create a navmesh for other Navmesh-settings (Agent is taller/thicker...) or you want one navmesh e.g. with a small scope(one tile with ground and obstacles) and another one with a bigger scope e.g. multiple tiles...

I created two more options (beside "navmesh-children" which is still the default):

1. "group with children": Use all nodes that are in a group specified in the "Navmesh Groupname"-Property and its children
2. "group explicit": only take the nodes that are in the specified group. children will be ignored

You can access the options on the NavigationMeshInstance using the "Bake Mode Type"-Property and the Navmesh Groupname and can also be activated using the "Bake"-button

This Pullrequest also fixes the gizmo for editor-created navmeshes.

Oh,I just read(in the contributing-txt) that I should have asked beforehand if this feature is wanted....Next time I will do this,..the feature just happend... ;)

Also look at the attached project that shows the changes in action
TestNavmeshGroups.zip

[dertom95]

I really don't know why travis-ci is not running through and what clang-format.sh wants here? The patch created would overwrite the whole navigation_mesh_generator.cpp-file making it impossible see the diff afterwards!? Any help?

[guilhermefelipecgs]

@dertom95 You need to fix the syntax for clang-format.sh to pass, for example:
What travis said:

-	void set_navmesh_groupname(const StringName& name);
+	void set_navmesh_groupname(const StringName &name);

minus (-) is your actual code
plus (+) is what are expected

What you need to do:
change this part of the code StringName& name to this StringName &name in your commit.
Do it for everything that was reported by clang-format and then, you'll need to squash all your commits in one for this PR to be approved.

[akien-mga]

Thanks a lot for your contribution!
We are now entering a strict release freeze for Godot 3.0 and will only consider major bug fixes. We won't merge new features and enhancements anymore until 3.0 is released.

Moving this PR to the 3.1 milestone, to be reviewed once the release freeze is lifted. It could eventually be cherry-picked for a future 3.0.1 maintenance release if it doesn't change the user-facing APIs and doesn't compromise the engine's stability.

[dertom95]

To see how to use the navigation mesh here a little tutorial:
https://youtu.be/0VPRg7QeI0I

[NathanWarden]

I like that, it makes a lot of sense and keeps the hierarchy from having to be dependent on being a child of the nav mesh instance node. I wouldn't mind if this was the default mode for baking.

[atllllll]

@akien-mga Can we take a second look at this? 3D navigation meshes could use some work and this seems to be an improvement.

[akien-mga]

Eventually, yes. There's a lot of PR backlog since the 3.0 release and we're doing all we can (many hours per week) to reduce the backlog will stay atop of new PRs.

[dertom95]

Just for info, just rebased on current master

[Zireael07]

Can this please get merged?

[slapin]

Will this PR be merged for 3.1?

Thanks!

[dertom95]

I actually would love to show the "navmesh groupname"-textinput only if one of the two group-based modes is selected, but I can't figure out how to implement "conditional properties" like e.g. enabling 'clear coat' in the material inspector, where the values only popup if enabled is selected.... Any hint appreciated. I already asked in irc and forum ( https://godotdevelopers.org/forum/discussion/19648/godot-engine-property-ui-show-hide-property-on-certain-condition#latest )

[akien-mga]

Why not return the enum directly to avoid having to cast it? Then it would also properly be documented as returning values from said enum.

[dertom95]

Hmm,..I did this for the UI-Mapping. Not sure how to bind an enum directly. Any sample for that?

[akien-mga]

Search for BIND_ENUM_CONSTANT in the code base.

[akien-mga]

@SaracenOne Could you help review this PR since it expands on your integration of recast?

[SaracenOne]

@akien-mga Sorry, I've been a bit busy lately with some other things. I'll try to find some time tomorrow to help your review this.

[reduz]

We were discussing with Odino that it should not be that complicated to change current navigation code to support agents of different sizes, or internally cache them, but currently Navigation internal code needs some redesign, specially to be able to support A* in threads (not currently supported), as well as proper local obstacle avoidance

[akien-mga]

As discussed with @reduz and mentioned above, we're thinking of redesigning how navigation works in the near future (3.2 probably), so this will be reviewed more in-depth at that time.

[Anutrix]

Any updates?

[slapin]

I guess no. But I do slowly work on my own implementation of navigation based on Recast/Detour/DetourCrowd libs. Currently I reimplement it navigation code as server which will be used by helper nodes. You can see current version in navigation_pr branch in my repo or look for closed navigation_pr PR. I might submit new PR as I will be satisfied if nothing funny happens.

…

On Mon, Jun 17, 2019 at 10:39 AM Anutrix ***@***.***> wrote: Any updates? — You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

[TypeOverride]

+1 (maybe some dev is seeing this. This is a need need feature off course.)

[JFonS]

The idea seems good, but the PR needs a rebase.

[dertom95]

The idea seems good, but the PR needs a rebase.

@JFonS Feel free to do it and create a PR of your own. I keep the PR just open for "nostalgic" reasons and won't support it anymore. (if you want you can rebase and do a PR on my repo ;) )

[akien-mga]

As 3.2 is in feature freeze and this PR is not yet ready for merging, I move it to the 4.0 milestone.
@JFonS If you think this should really get into 3.2, the clock is ticking :)

[JFonS]

I will probably work on it for 4.0. It's definitely not getting in 3.2 unless someone else wants to give it a spin.

[akien-mga]

Superseded by #32863.

Thanks for the contribution nevertheless, which is being improved further in that PR.