-
-
Notifications
You must be signed in to change notification settings - Fork 21.2k
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
Add NavigationObstacle options to affect navigation mesh baking #89034
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Mickeon
reviewed
Mar 1, 2024
smix8
force-pushed
the
navobstacle_bake_n_carve
branch
2 times, most recently
from
March 1, 2024 14:32
35621f6
to
68b7b77
Compare
Scony
reviewed
Mar 5, 2024
Comment on lines
823
to
837
if (!projected_obstructions.is_empty()) { | ||
for (const NavigationMeshSourceGeometryData3D::ProjectedObstruction &projected_obstruction : projected_obstructions) { | ||
if (projected_obstruction.carve == false) { | ||
continue; | ||
} | ||
if (projected_obstruction.vertices.is_empty() || projected_obstruction.vertices.size() % 3 != 0) { | ||
continue; | ||
} | ||
|
||
const float *projected_obstruction_verts = projected_obstruction.vertices.ptr(); | ||
const int projected_obstruction_nverts = projected_obstruction.vertices.size() / 3; | ||
|
||
rcMarkConvexPolyArea(&ctx, projected_obstruction_verts, projected_obstruction_nverts, projected_obstruction.elevation, projected_obstruction.elevation + projected_obstruction.height, RC_NULL_AREA, *chf); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
perhaps we could extract a part common with the block before to a separate function to remove duplication as well
smix8
force-pushed
the
navobstacle_bake_n_carve
branch
from
March 7, 2024 11:48
68b7b77
to
5d74d74
Compare
smix8
force-pushed
the
navobstacle_bake_n_carve
branch
from
March 9, 2024 13:01
5d74d74
to
8483d0b
Compare
Added support for the recently added source geometry |
smix8
force-pushed
the
navobstacle_bake_n_carve
branch
3 times, most recently
from
March 11, 2024 21:04
7a8dd4c
to
2790a01
Compare
Scony
approved these changes
Mar 12, 2024
akien-mga
approved these changes
Mar 12, 2024
AThousandShips
approved these changes
Mar 13, 2024
chef kiss thank you, you beauties! May all your dreams and wishes come true! |
Adds NavigationObstacle options to affect and carve navigation mesh.
smix8
force-pushed
the
navobstacle_bake_n_carve
branch
from
March 15, 2024 00:45
2790a01
to
5d5e85f
Compare
Thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Adds obstacle options to contribute to the navigation mesh baking.
Also helps with bake performance and removing unwanted navigation mesh parts.
Both
NavigationObstacle2D
andNavigationObstacle3D
have new properties that allow the obstacle to take part in the navigation mesh baking process.NavigationObstacle2D
For 2D the obstacle acts similar to normal source geometry.
The difference is that it "cuts" from the navigation mesh in a late pass after all other geometry.
It also can be made unaffected by the agent radius offset so that it "cuts" without additional margin added.
NavigationObstacle3D
For 3D the obstacle acts similar to 2D with one major difference compared to normal source geometry.
An obstacle does not add walkable geometry like collision shapes or visual meshes do, instead it acts as a discard for parts of all other source geometry inside its shape.
This can be used to remove navigation mesh from being baked in unwanted places that should not have it, e.g. inside "solid" geometry or on top of geometry.
In turn the obstacle alone can not be used to create navigation mesh when there is no other geometry around!
An obstacle does not add, it only removes. It does so by nullifying all the geometry (voxel) cells in the baking process that are inside its parsed shape.
The reason why this "remove navmesh inside geometry" is made available only on the obstacle, and not for all "solid" collision shapes or visual meshes, is that it requires a very restricted shape to be reasonibly accurate and performant to process.
Those restrictions are not possible to put in place for the many allowed uses of physics collision shapes or visual meshes.
This PR was also totally not made after seeing user navigation mesh like this:
Tbh this feature was cooking on the backburner for a while. Seeing that stuff was just what pushed it over the finishing line.
Shout out to team scott-veggie for sharing definitely-not-jotson's live struggle.
Affect the navigation mesh baking
The shape is defined with the already existing properties for
height
and outlinevertices
that were previously used only for the avoidance.The property
affect_navigation_mesh
makes the obstacle contribute to the navigation mesh baking. It will be parsed or not-parsed like all other node objects in a navigation mesh baking process.The property
carve_navigation_mesh
makes the shape unaffected by offsets of the baking, e.g. agent radius. It will basically act as a stencil and cut into the already offset navigation mesh surface. It will still be affected by further postprocessing of the baking process like edge simplification.Performance
If you do not plan to use the by default enabled avoidance of the obstacle turn off
enabled_avoidance
to save performance.The contributed geometry is a simple, projected shape of the obstacle vertices (top-down for 3D) or a low-detail version of the radius. This means this can be used to create comparably performance cheap projected "holes" for the navigation mesh baking.
It is one of the most efficient (but limited) ways available now to affect source geometry when baking navigation mesh. You will still need to combine it with other source geometry as the obstacle only discards.
Since the obstacle has ready-made geometry 100% stored on the CPU similar to physics shapes no parse and server-stalling related performance issues will happen. E.g. they are a regular issue when using MeshInstance3D for the source geometry when baking.
Projects with bake performance issues at runtime often should not look further than if they are using a rendering mesh as source geometry somewhere. Parsing even a single one of them at runtime can completely annihilate the frame rate because all that geometry data of a rendering mesh needs to be received from the GPU blocking the rendering.
It is recommended to use physics collision and obstacles exklusive as source geometry and replace all uses of rendering meshes when baking navigation mesh with them, at least for runtime.
Limitations and Caveats
Both the bake and carve features are still limited to the (voxel) cell resolution. It is not intended as an equivalent to more detailed boolean operations like e.g. CSG Meshes that are all super-costly in comparison.
The obstacle shape is very similar in function to an AABB except that instead of a square / box shape it uses an outline. Still, it needs to stay aligned to the horizontal xz axes and no transform like rotation or scale will be used, only the positions. This was already the case for the avoidance that has similar axis restrictions so it came naturally to use the same properties.
Do not expect the contours of the obstacle to perfectly carve into the navigation mesh. There are too many steps involved in the baking process that affect the final navigation mesh result and none of them aims for perfect accuracy. They all aim for bake performance and a low-detail polygon surface with a small edge count so the pathfinding can be fast.
For more accuracy the bake cell size and the simplification threshold can be lowered but a noticeable performance hit needs to be accepted when doing so as it creates a high (voxel) cell count that needs to be processed.
For accurate outside edges when baking, e.g. chunk navigation meshes, obstacles are not a good choice. The newer
baking Rect / AABB
andborder_size
can be used. Especially in 3D the baking AABB will also help to fix the voxel span count so navigation mesh polygon edges will not change so much when things are rebaked.Prevent navigation mesh from appearing "inside" or "on top" of source geometry
There is also a more subtle feature added with this that many users have asked for over the years.
Since obstacle have a very restricted, plane projected shape that discards geometry they can be used to discard the entire internal geometry of the shape from the navigation mesh baking at comparably low cost. This prevents navigation mesh from appearing both inside as well as on top the source geometry.
So if you have certain areas around a level that should not have unwanted navigation mesh place an obstacle shape there.
I dont want to go into too much technical detail but this would be near impossible to do in a performant manner with visual meshes or collision shapes as source geometry. Both can end up unpredictable complex, or worse, transformed and skewed. That is also why this is such a common issue in nearly all game engines based on ReCast navmesh baking. ReCast has no concept of "inner" geometry.
With an obstacle shape the enclosed voxel cells are now discarded in the baking process so this "block navmesh from appearing in internal geometry" is now done automatically when an obstacle is baked. It still has an added cost but from tests it is nowhere as brutal as with all the other geometry options.
Adding obstacles to NavigationMeshSourceGeometryData
For those that bake navigation mesh mostly procedural in scripts the source geometry objects have new functions added.
The
add_projected_obstruction()
can be used to add an obstacle shape to the source geometry.The
vertices
are in global coordinates by default, just watch out what you set as the root node for the parsing should it not be at origin.The
carve
parameter defines if the obstacle is affected by offsets, e.g. the agent radius, when baked.For 3D the y-axis of the vertices is ignored. Instead, the position and shape on the y-axis is controlled with the
elevation
and the extrusionheight
.Additional background why this is added to the obstacle
One of the main reasons why this is added to the obstacle is shape restrictions but there are also other reasons why this is not just made e.g. a new node type.
The community regularly recommends to use NavigationObstacles to make agents not move somewhere unwanted.
This was and is for the most part well-meaning but wrong advice.
You can not fix any pathfinding issues with the current obstacle, the navigation mesh path will still go right through the obstacle because the navigation mesh is still there unaffected. This creates conflict with the pathfinding wanting to move inside the obstacle while the avoidance pushes back getting the agent stuck.
The attempts to fix this "wrong advice" issue with more and more documentation was more or less in vain. Nearly every new user and even more experienced long-term godot users trip over this regularly.
That the problem even exists I blame on the poor choice of the node name.
The NavigationObstacle never had any functionality outside of avoidance. In hindsight it really should have been named AvoidanceObstacle from the start because that is all it ever did and could do (before 4.1 it could not even do that really).
Now that the NavigationObstacle name can no longer easily be changed and the documentation does not help the solution is another. The obstacle gets the functionality that is the most expected from users and quickly associated due to the node name.