NavigationAgent3D: Calling GetNextPathPosition during WaypointReached signal causes infinite recursion

[TyberiusInSD]

Tested versions

-Reproducible in: v4.1.stable.mono.official [9704596], v4.2.stable.official [46dc277], v4.2.2.stable.mono.official [15073af], v4.3.dev5.mono.official [89f70e9]
-Not reproducible in: v4.0.stable.mono.official [92bee43]

System information

Godot v4.2.2.stable.mono - Windows 10.0.19045 - Vulkan (Forward+) - dedicated AMD Radeon RX 6900 XT (Advanced Micro Devices, Inc.; 31.0.21001.45002) - AMD Ryzen 7 5800X 8-Core Processor (16 Threads)

Issue description

Connecting a method to the waypoint_reached signal of a NavigationAgent3D and calling navigation_agent.get_next_path_position() in said method causes an infinite recursion on the line that calls get_next_path_position().

For example, connecting the below method would cause a crash, but only if getPositionOnReach is set to true

func reached(_details: Dictionary):
	if getPositionOnReach:
		print(navigation_agent.get_next_path_position())
	else:
		print("No path position")

As a workaround, you can call "await get_tree().process_frame" before get_next_path_position() is called and it will work as expected.

This issue seems to have been introduced in v4.1.

I tested in v4.2 non-mono and it had the same error as well.

Steps to reproduce

1. Create a NavigationAgent3D class like at the bottom of this page in the docs
2. Connect a method to the navigation_agent.waypoint_reached signal which calls navigation_agent.get_next_path_position()
3. Play the scene and get the error "Stack overflow (stack size: 1024). Check for infinite recursion in your script"

In C# there are no errors; the build opens, freezes for a moment, then closes itself.

In the MRP, I have included separate scenes for C# and GDScript. The GDScript scene can be run without a .NET build as far as I can tell, but I wanted to include C# specifically since there is no error shown for the crash which made it much more difficult to debug.

Minimal reproduction project (MRP)

Navigation3D MRP.zip

[smix8]

You get an infinite recursion when you chain the function that updates the agent state and emits the signal to a function that calls the function.

The function get_next_path_position() updates all the internal agent state, including emitting all the signals when their conditions apply. If the path point is within reach it goes to the next path point.

The function also queries an entire new path when the current path end is reached so it really should only be called intentional once per update. If you function chain it like that you are calling it for infinity when your path reaches the end all the time or is not valid because it will query a new path with the same conditions.

The reason why it is not blocked to call this function more than once per update step (it was blocked originally) is because there are valid use cases (e.g. rollbacks) where users want to call it more than once.

In general creating a signal chain to the same function that triggers the signal emission is bound to give trouble. It is the same reasoning why you wouldn't chain a physics sliding on a collision signal back-to-back because you will end up sliding and colliding for infinity. The only reason why this is not regularly happening is because the PhysicServer does not process your request while the NavigationServer processes everything (except avoidance) immediately.

[TyberiusInSD]

That wasn't the case in 4.0 and isn't mentioned in the documentation from what I have seen. If it's intended behavior then the docs should at least be updated.

Also the fact that there was no error in C# makes something like this much more difficult to debug. I'm not sure why a very helpful infinite recursion error would be possible in gdscript but not C#, but that sounds like a separate issue now that I think about it.

[smix8]

In Godot 4.0 the update function was blocked after the first call on each physics step. This was removed because it blocked many valid use cases for the function and caused bugs where the once-per-step-update was consumed before it was intended by users.