Removing and re-adding Navigation and NavigationAgent to SceneTree inconsistent behavior

[TheOrioli]

Godot version

3.5.rc3

System information

Windows 10

Issue description

When removing and re-adding a Navigation and a NavigationAgent node to the SceneTree they appear to randomly stop working. The randomness comes from the inconsistence between the different setups of the NavigationAgent.

Problem cases

avoidance_enabled is false

When _enter_tree is called on the NavigationAgent, calling set_navigation again will be enough to let calls to methods such as get_next_location or is_target_reached to work.

avoidance_enabled is true

When the NavigationAgent uses avoidance, it is not enough to call set_navigation when entering tree again. The Navigation node itself must be readied again by calling request_ready before it is re-added to the tree. If both request_ready on the Navigation node and set_navigation on NavigationAgent are called than the agent will work as expected.

Workaround

Add a call to request_ready on Navigation in _exit_tree , call set_navigation on NavigationAgent's in _enter_tree after the first time the navigation is setup by keeping references to the Navigation node. This provides a consistent behavior across all agents.

Expected behavior

Manipulating Navigation related nodes in the SceneTree should not require manually keeping track of the state.

At the very least the request_ready call on the Navigation should not be necessary, I don't think any other built-in node requires "manual resetting" of it's state when exiting/entering the tree. If the set_navigation is required it should be documented in the NavigationAgent class.

Everything works as expected in Godot 4 Alpha 10. This issue is not present. Calling request_ready is unnecessary and the set_navigation call doesn't exist.

Steps to reproduce

Run the sample project, press the Remove/Add button to move the Navigation and NavigationAgent nodes from the scene tree, use the various checkbox to toggle what methods get called and if the NavigationAgent has avoidance enabled.

Minimal reproduction project

navagent_reparent_repro.zip
navagent_reparent_repro_godot4.zip

[akien-mga]

CC @smix8

This might be fixed in current 3.x as the navigation system was further changed to be more in-line with the 4.0 one. Could you check with latest CI builds to see if it works? https://github.com/godotengine/godot/actions/runs/2508552846

[TheOrioli]

Tested with the 3.x build, only NavigationAgent.set_navigation must be called.
Navigation.request_ready is not necessary in any case.

[smix8]

The NavigationAgent (re)parent fixes from Godot 4.0 were only recently added to Godot 3.5 with #61996 that should be part of 3.5 RC4 when it is released.

Godot 3.5 is a little quirky cause it has still the old Navigation node around that creates a new navigation map for all the child navigation nodes. When the NavigationAgent node (re)enters the tree it connects to the (new) parent spatial and the default navigation map for this spatial node which is the world 3d resource default navigation map. The NavigationAgent also crawls up the SceneTree and if it finds a Navigation node uses the navigation map from this node. To override this (default) map in Godot 3.5 the NavigationAgent functions set_navigation or set_navigation_node can be used.

EDIT
Need to backport #62044 to 3.5 to override the navigation map.

[smix8]

Note that the test project for 3.5 had the agent placed at the wrong position for automatic (re)join fo the navigation map.

It must be below the navigation node or else it will register to the default world navigation map which causes the agent to break cause this map is empty and has no navigation mesh. If it is desired to have the agent at another position using set_navigation is a requirement as the automatic map join only works when the navigation is a parent node.

[akien-mga]

Fixed by #62117.

[TheOrioli]

@akien-mga @smix8 I've tested the latest build. I have found a combination of methods that work great, but the API is now confusing. Please correct me if I misunderstood something, because I would really like to document this properly.

With the new navigation system, engine users have two choices:

Legacy

Use the Navigation node, with multiple child NavigationMeshInstance to describe the navigation polygons. This creates a new map in the NavigationServer, so agents need to be told about it.

The NavigationAgent must either be a child of the Navigation node, or set_navigation or set_navigation_map must be called in e.g. _ready.
Here is where the confusing API comes into play.

When calling set_navigation and passing in the Navigation node, it is a requirement to call set_navigation any time the agent exits/enters the SceneTree.

However, if calling NavigationAgent.set_navigation_map instead, the agent behaves as expected, and there is no need to call NavigationAgent.set_navigation_map on tree entry again.

I understand that the set_navigation call is kept around for API compatibility reasons, but shouldn't it then just act as an alias:
NavigationAgent.set_navigation(nav_node) -> NavigationAgent.set_navigation_map(nav_node.get_rid()) to insure consistent behavior across method calls.

Godot 4

Place NavigationMeshInstance nodes directly into the tree, without a parent Navigation node. This will create regions inside the global map.

NavigationAgent will default to the global map when not told about a separate Navigation node or a custom map RID. This means the agent is very flexible when entering/exiting the tree.

[smix8]

@TheOrioli

When NavigationAgent enters the tree again calling set_navigation() manually is only a requirement if you do not place the agent below the navigation node. This was actually the case in your 3.5 test project as the agent was child of the "NodeToRemove" node and not the Navigation node.

Godot 4.0 and 3.5 navigation work the same now but 3.5 has some old stuff kept around for compatibility with existing scenes.

What is shared with both version is that every world resource creates a default navigation map and every navigation related node like regions and agents join this map if not told otherwise. NavigationAgent.set_navigation_map() can be used to override the default map for both future pathfinding queries of the NavigationAgent node as well as avoidance rvo_agent on the NavigationServer in one go.

That said Godot 3.5 has still the old Navigation and Navigation2D nodes around that are kept for scene compatibility with old 3.x version. These nodes create a new navigation map, something that can only be done with the NavigationServer API directly in Godot 4.0. When a navigation related node is placed below such node it will join the navigation map of the navigation node and not the default map. The functions NavigationAgent.set_navigation() and NavigationAgent.set_navigation_node() are old and exclusive to 3.x because of the Navigation/Navigation2D nodes and backward compatibility. Might mark them and the entire nodes as depr soon. For Godot 3.5 I would advice to not use the old Navigation/Navigation2D nodes and the old functions at all anymore and move to the new Godot 4.0 navigation way.

[TheOrioli]

@smix8 Amazing, thank you. I'll poke around the docs on Monday and try to update them with this information to make it clearer for devs.

Final question, some useful methods require a map RID to work, e.g. NavigationServer.map_get_closest_point It seems like the best way to get the RID for the default map right now is to call the NavigationAgent.get_navigation_map for an agent you know is part of the default map or by using a NavigationMeshInstance.get_region followed by NavigationServer.region_get_map

Is there another way to get the default/world map RID?

[smix8]

@TheOrioli
The default navigation maps are part of the World2D or World3D resources of the viewport(s).

The default 2D navigation map can be optained from any Node2D inheriting node with get_world_2d().get_navigation_map().
The default 3D navigation map can be optained from any Node3D inheriting node with get_world_3d().get_navigation_map().