Godot_3D_Navigation_Jump_Links icon indicating copy to clipboard operation
Godot_3D_Navigation_Jump_Links copied to clipboard

Physics_Frame yield is hiding a race condition.

Open MJBrune opened this issue 2 years ago • 2 comments

https://github.com/smix8/Godot_3D_Navigation_Jump_Links/blob/4c04cb555bbe71ab1dcc6d36a717b59e736355a3/addons/navigation_jump_links/scripts/JumpLinkNavigation.gd#L45 This yield is not enough. I end up getting null meshes in _navmesh_mappings. I noticed that the 4.x branch does not have this yield. Is there a reason there is a difference? In 4.x is it that the nav server is ready before _ready() is called on the JumpLinkNavigation node?

MJBrune avatar Sep 03 '22 06:09 MJBrune

A workaround I've found, add another physics_frame yield. Better yet, add another one on top for safekeeping. The better solution is to yield until we know the navigation server is ready. I can't find a good way to do that. I assume there wasn't really a good way to do this and thus yielding on physics_frame was introduced?

MJBrune avatar Sep 03 '22 06:09 MJBrune

@MJBrune Hey, sorry did not receive a notification.

The NavigationServer will sync on the physics tick in both versions. Without the sync the navigation map is empty when a request is made in the _ready() function that is why the yield or any other wait method is required.

Usually a call_deferred() (so all nodes are ready) and a single yield() (to wait for the physics tick) is enough with default physics tick rate to solve this. If not another yield may be safer but I personally never had the need for it or the mentioned issues.

That the Godot 4 version is different has more to do that I did not update for a long time. Navigation links are now a core Godot navigation feature already merged in Godot 4 and part of the recent Godot 4 beta.

smix8 avatar Sep 18 '22 19:09 smix8