Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
smix8 opened this issue Jul 11, 2024 · 6 comments
Open

Comments

@smix8
Copy link
Contributor

smix8 commented Jul 11, 2024

Tested versions

4.3dev

System information

Windows 10

Issue description

This is a regression from PR #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

  • Create a scene with GridMap and cells that have build-in navigation mesh.
  • Save Scene
  • Restart Editor
  • See that the debug does not display anymore.

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
Copy link
Contributor Author

smix8 commented Jul 11, 2024

@KoBeWi

@KoBeWi
Copy link
Member

KoBeWi commented Jul 11, 2024

A minimal project would be useful.

@smix8
Copy link
Contributor Author

smix8 commented Jul 11, 2024

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
Copy link
Member

KoBeWi commented Jul 11, 2024

The error comes from

g.navigation_debug_edge_connections_mesh->clear_surfaces();

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
Copy link
Contributor Author

smix8 commented Jul 11, 2024

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
Copy link
Member

KoBeWi commented Jul 11, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants