Zylann / godot_voxel

Voxel module for Godot Engine
MIT License
2.64k stars 248 forks source link

The graph editor UI is janky #486

Open Zylann opened 1 year ago

Zylann commented 1 year ago

The graph editor used to work fine for a while during Godot 4 betas, despite some workarounds I had to do, but there were changes to EditorPlugin behavior in later Godot 4 betas/RCs, which really broke it. Sometimes it even crashed. I tried to add more workarounds in 2550cce8a9cd88129f50cb99b0936309da51b5bd but Godot still logs errors, and mouse input sometimes gets missed. It became annoying to use.

The input issues were the reason I added the original workarounds, and now they seem to be partly back: https://github.com/godotengine/godot/issues/40166

I opened another issue for the new problems in Godot 4: https://github.com/godotengine/godot/issues/73650 which however won't be investigated until at least Godot 4.1.

The core of the issue is because VoxelGraphEditorPlugin relies on the Godot inspector to edit "sub-objects". The graph contains nodes, which have properties that can be edited, which can in turn be edited as they can be curves and noise resources. To achieve this, the plugin handles() several types of resources, the graph itself and nodes of the graph, and tells Godot to inspect_object() when selecting nodes or the background. This however causes all sorts of issues now (some of them bugs), and it's unclear how to properly do it.

According to https://github.com/godotengine/godot/issues/40166#issuecomment-1213573494, inspect_object() has a boolean option inspector_only, which was supposed to be a fix for some of the issues. Unfortunately it no longer solves the problem, it still triggers edit(nullptr) and make_visible(false) so when a node is selected it hides the entire graph editor. No idea if that's a bug.

I searched how the VisualShader editor handles editing sub-resources inside nodes with the inspector, without getting bothered, and it uses none of the above. Instead it directly uses InspectorDock::get_inspector_singleton()->edit(p_resource.ptr());, which is not available to scripts and extensions.

There doesn't seem to be other solutions to inspect an object without causing undesired side-effects...

Valeryn4 commented 1 year ago

just add a cross to the Node Graph. Let it call "remove_node_gui(this.name)" add "remove" on popup menu

Removing nodes from a graph does not work

Zylann commented 1 year ago

I think you're describing a new issue, it's probably not the right thread to report it (what you describe does not solve the present issue with edition behavior).

If the Delete key isn't working it has to be fixed, that's a separate problem.