godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
89.12k stars 20.2k forks source link

NavigationServer : regions removing linked NavigationMesh automatically? #75704

Closed YouveBeen closed 1 year ago

YouveBeen commented 1 year ago

Godot version

4.0.1 Stable

System information

Windows 10

Issue description

First of, sorry if it's not an issue, i'm reading the cpp for the navigation server and i don't understand why something is happening (or not happening) in my project. Here is my use of the NavigationServer3D (nodeless, i only show the relevant code):

Context: I'm generating a map randomly and divide it into chunks. (3D but top-down so only 2D chunks). I generate the starting chunks navmeshes in _ready and the other on the fly in another thread

func _ready():
    NavigationServer3D.set_debug_enabled(true)
    NavigationServer3D.map_set_active(nav_map_small, true)
    NavigationServer3D.map_set_active(nav_map_medium, true)
    for y in range(0, max_chunks):
        var line : Array[Chunk] = []
        for x in range(0, max_chunks):
            var chunk = Chunk.new()
            chunk.navigation_meshes.clear()
            chunk.navigation_regions.clear()
            chunk.coordinates = Vector2i(x, y)
            var p = map_to_world(chunk.coordinates)
            var c = Vector3.ZERO
            var navmesh = nav_mesh_template.duplicate()
            navmesh.agent_radius = 0.5
            navmesh.filter_baking_aabb = AABB(Vector3(c.x - navmesh.agent_radius, 0.0, c.y - navmesh.agent_radius), Vector3(CHUNK_SIZE + navmesh.agent_radius*2.0, 2.0, CHUNK_SIZE + navmesh.agent_radius*2.0))
            navmesh.filter_baking_aabb_offset = Vector3(p.z, 0.0, p.x)
            chunk.navigation_meshes.push_back(navmesh)
            navmesh = nav_mesh_template.duplicate()
            navmesh.agent_radius = 2.0
            navmesh.filter_baking_aabb = AABB(Vector3(c.x - navmesh.agent_radius, 0.0, c.y - navmesh.agent_radius), Vector3(CHUNK_SIZE + navmesh.agent_radius*2.0, 2.0, CHUNK_SIZE + navmesh.agent_radius*2.0))
            navmesh.filter_baking_aabb_offset = Vector3(p.z, 0.0, p.x)
            chunk.navigation_meshes.push_back(navmesh)
            var reg = NavigationServer3D.region_create()
            chunk.navigation_regions.push_back(reg)
            reg = NavigationServer3D.region_create()
            chunk.navigation_regions.push_back(reg)
            NavigationServer3D.region_set_map(chunk.navigation_regions[0], nav_map_small)
            NavigationServer3D.region_set_map(chunk.navigation_regions[1], nav_map_medium)
            NavigationServer3D.region_set_navigation_mesh(chunk.navigation_regions[0], chunk.navigation_meshes[0])
            NavigationServer3D.region_set_navigation_mesh(chunk.navigation_regions[1], chunk.navigation_meshes[1])

            #chunk.navigation_regions.push_back(reg)

            line.append(chunk)
        chunks_data.append(line)

Then i generate the map (randomly) and bake the first navigation meshes:

func bake_chunk_nav_mesh(coord : Vector2i):
    var c = map_to_world(coord)
    var i = 0
    for navmesh in chunks_data[coord.x][coord.y].navigation_meshes:
        NavigationServer3D.region_bake_navigation_mesh(navmesh, self)
        #NavigationServer3D.region_set_navigation_mesh(chunks_data[coord.x][coord.y].navigation_regions[i], navmesh)
        i += 1
    chunks_data[coord.x][coord.y].navigable = true

From there it's working fine. Now when the player move around the navmeshes are generated on the fly with the same function but in a thread. The only way for the navigation meshes generated at runtime to work is to un-comment the line with "region_set_navigation_mesh" method: but the navmesh is already linked in _ready(), and the navmeshes generated at runtime are fine without re-linking them to the region.

So i wondered if the server flush the empty meshes after the first physic frame, i went to see the source files and in nav_region.cpp and nav_map.cpp, i see the the update_polygon function first do nothing when the mesh is empty but when "sync" get called and the navigation mesh is now baked and should update the polygons. So i wonder if i missed something, or if it's the intended usage to link the navmesh after baking it?

Steps to reproduce

See code

Minimal reproduction project

See code

smix8 commented 1 year ago

All the NavigationServer function region_bake_navigation_mesh() does is route to the NavigationMeshGenerator bake() and clear() functions.

In an ancient time this was necessary due to dependency issues in the Godot init change. The NavigationMeshGenerator was not ready when required by certain objects so the NavigationServer acted as a router. All 3 functions will be changed / removed should pr https://github.com/godotengine/godot/pull/70724 be merged. At that point you use the NavigationMeshGenerator directly to (re)bake navigation meshes.

The NavigationMesh resources are just data container for the vertices and polygons. They have no connection to the NavigationServer. While there are plans to change that at some point for now you can't "link" navigation meshes in the sense that they auto-update the NavigationServer internal NavRegion when the navigation mesh is changed. Hence why you need to use the region_set_navigation_mesh() again for the region after (re)baking the navigation mesh.

When you call region_set_navigation_mesh() all you are doing is placing this NavigationMesh ref in the waiting room and activating the dirty flag for the next NavigationServer sync phase. On sync the NavigationServer copies the vertices and polygons from the NavigationMesh resource to the internal NavRegion and rebuilds the navigation map. The NavigationMesh is completely meaningless for the NavigationServer after that so theoretical might as well be deleted until the next region update is required.

YouveBeen commented 1 year ago

Thank you for your explanations. This is problematic for me performance wise, as even when baked in a thread there is a slight fps drop every time the navigation map is updated. Hopefully your pr will fix this.

smix8 commented 1 year ago

Performance problems when the navigation map changes can have many issues. I don't know how detailed you navigation meshes are build or how large your world and chunks are. If you don't have performance issue while baking and you don't have any NavigationAgent in use most likely it is properly the navigation map synchronization alone after changing a navigation mesh.

In that case pr https://github.com/godotengine/godot/pull/75601 that allows to disable the costly edge connections might help soon with larger navigation maps that have many navigation mesh polygons and edges. In general try to have as few edges as possible for runtime update performance.

YouveBeen commented 1 year ago

My chunks are 64x64 "meters" I just have a few low-poly trees on each chunks (no more than 20) and i use the partition_layer method on static colliders that are in explicit group. Changing partition type doesn't really change anything.

The thing is i have currently 2 different maps for the different enemy sizes, so i bake and attach each chunk double time with different agent_radius (was planing on 3 maps originally). I bake navmeshes of 3 chunks when players change chunk, each chunk in a different thread (according to computer spec)

For numbers, if that means anything to you, i have currently 4 chunks with navmeshes and Navigation monitor numbers are : Polygons : 869 Edges : 1657 Edges Merged : 950 Edges Connected : 30 Edges Free : 707

Also using the NavigationServer3D Directly i can't see any visual debug of navmeshes, like i can do with NavigationRegion3D. "Visible Navigation" is checked and set_debug_enabled is set to true in script but nothing draw.

PS: After rereading i though about make sure no static decors was at the edge of any chunks so the navmeshes edges are a straight line. The numbers now are: Polygons : 1172 Edges : 2200 Edges Merged : 1316 Edges Connected : 0 Edges Free : 884

It's a bit better at start but as the nav_maps grow bigger the frame drop become longer. Should i detach far away chunks navmeshes and attach them back them if player goes back there? So keeping a 9x9 chunks of navmeshes at all time and drop the rest? I believe the PR you mentionning will help a lot enventually.

EDIT: Numbers of agents (from a few to none) on the map doesn't change anything

smix8 commented 1 year ago

The navigation debug in Godot is all attached to nodes. No nodes, no debug visuals. Same with physics.

I don't see too much out of the ordinary with your stats except that having hundreds of free edges is properly what costs you so much performance on navigation map changes due to the costly edge connection calculations for each free edge.

The thing is the way the NavigationServer currently does update navigation maps any tiny change triggers a full rebuild of the navigation map. So the more your navigation map grows the more unavoidable performance issues you will encounter when doing runtime changes. The only way for now is to keep the amount of navigation polygon edges as small as possible. If that does not help with performance enough the only workaround is to slice your world into chunks that each use a different navigation map and switch your agents between them.

smix8 commented 1 year ago

Closing this in favor of https://github.com/godotengine/godot/issues/68642 as the issues left here are the same navigation map sync and update performance issue with large maps already tracked in that other issue.

Feel free to comment if there is something else still unresolved and not tracked by that other issue.