godotengine / godot

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

GridMap editor debug visuals broken since deferred call was changed to Callable #94201

Open smix8 opened 1 month ago

smix8 commented 1 month ago

Tested versions

4.3dev

System information

Windows 10

Issue description

This is a regression from PR https://github.com/godotengine/godot/pull/86301 as reverting that PR fixes the debug display.

GridMap does not display its editor debug for cell build-in navigation meshes correctly anymore. Even toggling the debug on and off with "bake_navigation" does not change anything.

Steps to reproduce

Sometimes scene switches help but not always. Toggling the debug on and off has no effect, e.g. toggling "bake_navigation", does nothing.

Minimal reproduction project (MRP)

GridMapDebug.zip

smix8 commented 1 month ago

@KoBeWi

KoBeWi commented 1 month ago

A minimal project would be useful.

smix8 commented 1 month ago

Added MRP. How to test:

Open "TestScene". There should be GridMap navmesh debug for all the added cells but isnt. Instead the RenderingServer prints mesh RID errors.

Change scene tab to "SwitchScene" and back to "TestScene", same errors print again, so it is trying but failing.

Toggle GridMap.bake_navigation property in the inspector. Nothing happens (before this did toggle the debug just fine).

Start the "TestScene" to see the debug at runtime how it should look as it works just normal at runtime, the issue only exists inside the Editor.

KoBeWi commented 1 month ago

The error comes from https://github.com/godotengine/godot/blob/383a6e4ef285fddcb0e1cbe28a20332c7e5a815b/modules/gridmap/grid_map.cpp#L1393 which doesn't look like it would affect drawing.

Trying to revert #86301 results in too many conflicts, so it's difficult to test if it's even related. Reverting only GridMap code does not fix the bug, so the problem is somewhere else.

smix8 commented 1 month ago

I reverted just the GridMap code on current Godot master before opening this issue and it returns the debug to the working state that I remember

So it is related to the order of how deferred calls happen on the timeline with the MessageQueue push VS using a Callable, with whatever suble difference this change has for the order that things are executed.

My guess would be that likely the GridMap already has stuff that frees and deletes executed before the Callable function runs which result now in those null errors or code branches not running that check for valid RIDs. When I look at the NavigationServer it also does not create those RIDs that it normally would as if things are stuck in queue or blocked by some dirty flag already set to false too early.

KoBeWi commented 1 month ago

I reverted just the GridMap code on current Godot master before opening this issue and it returns the debug to the working state that I remember

That's not the result I'm seeing.

So it is related to the order of how deferred calls happen on the timeline with the MessageQueue push VS using a Callable, with whatever suble difference this change has for the order that things are executed.

push_call() calls push_callp(), which calls push_callablep(), the same as call_deferred() in Callable. The order should be the same.