Reimporting GLTF causes "Attempt to disconnect a nonexistent connection" in inherited scene, duplicate meshes and crash

[DevLewa]

Tested versions

Tested in v4.2.1 (latest stable version)

System information

Godot v4.2.1 - Windows 10, Forward+/Vulkan GTX 1060 6GB

Issue description

Reimporting a gltf file (either by opening the .gltf in Godot and clicking "reimport" or by dragging the gltf and accompanying .bin file into godots ressource directory letting godot reimport it) causes a
Attempt to disconnect a nonexistent connection from 'test:<Node3D#1825814147267>'. Signal: 'replacing_by', callable: 'EditorNode::set_edited_scene'.
error to be printed in the output window. The asset also doesn't get reimported.

Also, if we create another scene and place the inherited scene in there and try to reimport the asset, it fails and creates a duplicate instance of the reimported asset at the center of the scene.

"level.tscn" which has "test.tscn" placed in the scene with a translation offset:

After reimporting the gltf:

Note also how the Scene view on the left side on the screen doesn't match what we actually see in the 3D preview. (it shows the nodes from the inherited scene)
This in turn can also cause Godot to crash shortly after. (somewhat random.)

I attached the reproduction sample/project as a zip.

Steps to reproduce

1. start godot
2. export a mesh as a GLTF file from blender (can be the default cube which is already placed in the scene).
3. drag the .gltf and .bin file into the res:// directory of godot
4. right click on the gltf > new inherited scene
5. save the opened scene in the res:// directory of godot
6. open the gltf in godot and click "Reimport"

Minimal reproduction project (MRP)

ReimportTest.zip

[josephknight]

Experiencing same issue. Having the inherited scene open while doing the re-import on the original model is key to replicating the bug. Before finding this post, this bug caused hours of running in circles since the window is not showing accurate data. Also occurring to .blend files because of the behind-the-scenes GLTF importer.

[Jonathan-Jay]

Experiencing the same issue with a custom importer. Even if we end the _import function early by just returning Error.Ok, or any other Error, the console error and duplicate "ghost" object still appears.

Addendum: Forgot to mention that our _import function does run properly and things get reimported, so we don't encounter that issue specifically

Some things we noticed:

• The error and duplicate only appears if the inherited scene is open in a tab view (selected or not).
• If the inherited scene is not selected, its node tree will replace the visible one of the currently selected scene after the reimport, and only change back when you change to a different scene (the reasoning we found was that it is added to the subviewport and becomes the youngest child, but when you change scene, it becomes the oldest child of the editor subviewport, and thus, its node tree isn't shown)
• Changing to the inherited scene or closing it (middle click or X button) gets rid of the duplicate
• The duplicate follows you around whenever you change scene (that aren't the inherited scene)
• The duplicate is actually the inherited scene itself (printing the node copy by doing print(get_parent().get_children()) on the root node of various scenes to verify this), based on its id, and keeps its id as you change scenes
• If you are on the inherited scene, then it'll try to add_child() itself to the subviewport. We found this out because our plugin uses a custom editor that moves the edited node to a different node, and when it is reimported, we get the error Can't add child 'NodeName' to '@SubViewport@9288', already has a parent 'World'
• The error messages appear before the import process itself. Based on the fact that the error messages are posted before our _import function's GD.print() calls (this could be a delay issue with GD.print() too, but mentioning it anyways)

A tldr of the above: if an inherited scene's original asset is reimported, while it is an open tab in the editor, the node itself gets added to the subviewport and appears in any open scene until you a) open the scene it originates from, or b) close the scene it originates from. None of this happens if the inherited scene is not open in the editor. during the import process.

This seems to happen specifically with inherited scenes, as the original import scenes don't have this problem

After checking some of the godot engine code, we found that the function on line 3546 (https://github.com/godotengine/godot/blob/master/editor/editor_node.cpp#L3546) void EditorNode::set_edited_scene(Node *p_scene) performs the 2 errors we encounter:

• attempts to disconnect a a function from a signal without any checks if it was already connected
• tries to add the node as a child to the viewport (supposedly), without checking if it's already a child of a different node

[dev-n-it]

Experiencing the duplication issue and the error message. The re-import of the gltf assets goes through though.

[Rindbee]

godot/editor/editor_node.cpp

Lines 5754 to 5764 in b2f425f

	editor_data.set_edited_scene_root(instantiated_node);
	current_edited_scene = instantiated_node;
	if (original_node->is_inside_tree()) {
	SceneTreeDock::get_singleton()->set_edited_scene(current_edited_scene);
	original_node->get_tree()->set_edited_scene_root(instantiated_node);
	}
	}
	// Replace the original node with the instantiated version.
	original_node->replace_by(instantiated_node, false);

Related to the above code. The previous code does a similar job to EditorNode::set_edited_scene() when it is the scene root, and replace_by() will automatically call EditorNode::set_edited_scene().

diff --git a/editor/editor_node.cpp b/editor/editor_node.cpp
index 4fb1a86ce2..81d08c8953 100644
--- a/editor/editor_node.cpp
+++ b/editor/editor_node.cpp
@@ -5751,13 +5751,7 @@ void EditorNode::reload_instances_with_path_in_edited_scenes(const String &p_ins
                                                instantiated_node->set_scene_inherited_state(state);
                                                instantiated_node->set_scene_file_path(String());
                                        }
-                                       editor_data.set_edited_scene_root(instantiated_node);
                                        current_edited_scene = instantiated_node;
-
-                                       if (original_node->is_inside_tree()) {
-                                               SceneTreeDock::get_singleton()->set_edited_scene(current_edited_scene);
-                                               original_node->get_tree()->set_edited_scene_root(instantiated_node);
-                                       }
                                }
 
                                // Replace the original node with the instantiated version.

It might be better to have replacing_by emitted earlier. #90479