search

smix8 / Godot_3D_Navigation_Jump_Links

Navigation Jumplinks to connect navmesh islands and allow AI pathfinding non-conventional ways to travel. Godot Game Engine.
MIT License
72 stars 9 forks source link

Physics_Frame yield is hiding a race condition. #4

Open MJBrune opened 2 years ago

MJBrune commented 2 years ago

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 commented 2 years ago

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?

smix8 commented 2 years ago

@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.