Repository: godotengine/godot (language: cpp)

Issue:
Editor crash when trying to reimport gltf scenes that are nested in godot
### Tested versions

v4.4.1.rc1.official [daa4b058e]

### System information

Godot v4.4.1.rc1 - macOS Sequoia (15.1.0) - Multi-window, 1 monitor - Metal (Forward+) - integrated Apple M1 Max (Apple7) - Apple M1 Max (10 threads)

### Issue description

Godot crashes when trying to reimport two scenes where one scene is a child of another scene

https://github.com/user-attachments/assets/95828c6e-d05f-4bb7-bbf8-ae44be9f130b

### Steps to reproduce

Open the MRP, select both gltf files in the filesystem dock, right click and reimport

### Minimal reproduction project (MRP)

[mrp_nested_gltf_crash.zip](https://github.com/user-attachments/files/19266573/mrp_nested_gltf_crash.zip)


Discussion / hints:
Can reproduce on Windows 10 on 4.4 stable. When selecting `Outer.gltf` first, then `Inner.gltf`, and then reimport, no crash occurs.
Backtrace investigation:
Crash in "reload_instances_with_path_in_edited_scenes" around some code that was added to increase performance of reloading scenes when re-importing them.

<details>
<summary>Full Backtrace main/4.5 (ef1153b)</summary>

~~~
PS C:\repos\godot\bin> .\godot.windows.editor.dev.x86_64.console.exe "C:\Users\b\Downloads\mrp_nested_gltf_crash\new-game-project\project.godot"
Godot Engine v4.5.dev.custom_build.ef1153baf (2025-03-19 13:54:01 UTC) - https://godotengine.org
Vulkan 1.4.303 - Forward+ - Using Device #0: NVIDIA - NVIDIA GeForce GTX 1080


================================================================
CrashHandlerException: Program crashed
Engine version: Godot Engine v4.5.dev.custom_build (ef1153baf3343470b65258421a163c92455d32b3)
Dumping the backtrace. Please include this when reporting the bug to the project developer.
[0] Node::remove_child (C:\repos\godot\scene\main\node.cpp:1681)
[1] EditorNode::reload_instances_with_path_in_edited_scenes (C:\repos\godot\editor\editor_node.cpp:6434)
[2] EditorNode::_resources_reimported (C:\repos\godot\editor\editor_node.cpp:1170)
[3] call_with_variant_args_helper<EditorNode,Vector<String> const &,0> (C:\repos\godot\core\variant\binder_common.h:314)
[4] call_with_variant_args<EditorNode,Vector<String> const &> (C:\repos\godot\core\variant\binder_common.h:429)
[5] CallableCustomMethodPointer<EditorNode,void,Vector<String> const &>::call (C:\repos\godot\core\object\callable_method_pointer.h:108)
[6] Callable::callp (C:\repos\godot\core\variant\callable.cpp:58)
[7] Object::emit_signalp (C:\repos\godot\core\object\object.cpp:1246)
[8] Node::emit_signalp (C:\repos\godot\scene\main\node.cpp:4017)
[9] Object::emit_signal<Vector<String> > (C:\repos\godot\core\object\object.h:923)
[10] EditorFileSystem::reimport_files (C:\repos\godot\editor\editor_file_system.cpp:3299)
[11] ImportDock::_reimport (C:\repos\godot\editor\import_dock.cpp:689)
[12] ImportDock::_reimport_attempt (C:\repos\godot\editor\import_dock.cpp:579)
[13] ImportDock::reimport_resources (C:\repos\godot\editor\import_dock.cpp:372)
[14] FileSystemDock::_file_option (C:\repos\godot\editor\filesystem_dock.cpp:2502)
[15] FileSystemDock::_tree_rmb_option (C:\repos\godot\editor\filesystem_dock.cpp:2111)
[16] call_with_variant_args_helper<FileSystemDock,int,0> (C:\repos\godot\core\variant\binder_common.h:319)
[17] call_with_variant_args<FileSystemDock,int> (C:\repos\godot\core\variant\binder_common.h:429)
[18] CallableCustomMethodPointer<FileSystemDock,void,int>::call (C:\repos\godot\core\object\callable_method_pointer.h:108)
[19] Callable::callp (C:\repos\godot\core\variant\callable.cpp:58)
[20] Object::emit_signalp (C:\repos\godot\core\object\object.cpp:1246)
[21] Node::emit_signalp (C:\repos\godot\scene\main\node.cpp:4017)
[22] Object::emit_signal<int> (C:\repos\godot\core\object\object.h:923)
[23] PopupMenu::activate_item (C:\repos\godot\scene\gui\popup_menu.cpp:2589)
[24] PopupMenu::_input_from_window_internal (C:\repos\godot\scene\gui\popup_menu.cpp:684)
[25] PopupMenu::_input_from_window (C:\repos\godot\scene\gui\popup_menu.cpp:474)
[26] Window::_window_input (C:\repos\godot\scene\main\window.cpp:1698)
[27] call_with_variant_args_helper<Window,Ref<InputEvent> const &,0> (C:\repos\godot\core\variant\binder_common.h:314)
[28] call_with_variant_args<Window,Ref<InputEvent> const &> (C:\repos\godot\core\variant\binder_common.h:429)
[29] CallableCustomMethodPointer<Window,void,Ref<InputEvent> const &>::call (C:\repos\godot\core\object\callable_method_pointer.h:108)
[30] Callable::callp (C:\repos\godot\core\variant\callable.cpp:58)
[31] Callable::call<Ref<InputEvent> > (C:\repos\godot\core\variant\variant.h:955)
[32] DisplayServerWindows::_dispatch_input_event (C:\repos\godot\platform\windows\display_server_windows.cpp:4310)
[33] DisplayServerWindows::_dispatch_input_events (C:\repos\godot\platform\windows\display_server_windows.cpp:4281)
[34] Input::_parse_input_event_impl (C:\repos\godot\core\input\input.cpp:905)
[35] Input::flush_buffered_events (C:\repos\godot\core\input\input.cpp:1186)
[36] DisplayServerWindows::process_events (C:\repos\godot\platform\windows\display_server_windows.cpp:3694)
[37] OS_Windows::run (C:\repos\godot\platform\windows\os_windows.cpp:2075)
[38] widechar_main (C:\repos\godot\platform\windows\godot_windows.cpp:97)
[39] _main (C:\repos\godot\platform\windows\godot_windows.cpp:122)
[40] main (C:\repos\godot\platform\windows\godot_windows.cpp:136)
[41] WinMain (C:\repos\godot\platform\windows\godot_windows.cpp:150)
[42] __scrt_common_main_seh (D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:288)
[43] <couldn't map PC to fn name>
-- END OF BACKTRACE --
================================================================
~~~
</details>

Function involved in the crash:
~~~
editor\editor_node.cpp:6253
void EditorNode::reload_instances_with_path_in_edited_scenes() {
   ...
	// Remove all nodes which were added as additional elements (they will be restored later).
	for (AdditiveNodeEntry additive_node_entry : instance_modifications.addition_list) {
		Node *addition_node = additive_node_entry.node;
		addition_node->get_parent()->remove_child(addition_node); // <--- remove_child() call causes crash. "addition_node->get_parent()" is null.
	}
   ...
}
~~~

<details>
<summary>Partial solution/deep dive</summary>

Taking a first pass at a fix, adding a check to make sure the parent isn't null in the for loop above fixes the crash, but it leads to some funky behavior. Everything re-imports, but in the scene tree, the "inner"/child node asset is removed from the viewport/editor scene tree. It's still in the node asset stored file, so all you have to do is re-open the asset and everything works as expected.

However, a user could reimport a set of nested gltf files, then save the parent node scene without verifying everything is correct and lose data, which is probably worse than a crash, so I'm going to leave the crash fix code here and not make a PR (anyone who reads this is free to tackle the rest of it).

Partial Solution:
~~~
void EditorNode::reload_instances_with_path_in_edited_scenes() {
        ...
        // Remove all nodes which were added as additional elements (they will be restored later).
	for (AdditiveNodeEntry additive_node_entry : instance_modifications.addition_list) {
		Node *addition_node = additive_node_entry.node;
		if (addition_node->get_parent()) {
			addition_node->get_parent()->remove_child(addition_node);
		}
	}
        ...
}
~~~

Further down in "reload_instances_with_path_in_edited_scenes()" is a section that re-adds these "addition_node" nodes that are removed in the above for loop (editor\editor_node.cpp:6508). This is the "// ... (they will be restored later)" bit referenced in the above function.

Single stepping through this with a debugger didn't show anything useful. The "inner" node was properly in the for loop and it hit every normal case below as expected.

I'm guessing something in this area is what needs to be changed as well, but can't seem to find it.
~~~
editor\editor_node.cpp:6508
void EditorNode::reload_instances_with_path_in_edited_scenes() {
        ...
        // Attempt to re-add all the additional nodes.
	for (AdditiveNodeEntry additive_node_entry : instance_modifications.addition_list) {
		Node *parent_node = instantiated_node->get_node_or_null(additive_node_entry.parent);

		if (!parent_node) {
			parent_node = current_edited_scene;
		}

		parent_node->add_child(additive_node_entry.node);
		parent_node->move_child(additive_node_entry.node, additive_node_entry.index);
		// If the additive node's owner was the node which got replaced, update it.
		if (additive_node_entry.owner == original_node) {
			additive_node_entry.owner = instantiated_node;
		}

		additive_node_entry.node->set_owner(additive_node_entry.owner);

		// If the parent node was lost, attempt to restore the original global transform.
		{
			Node2D *node_2d = Object::cast_to<Node2D>(additive_node_entry.node);
			if (node_2d) {
				node_2d->set_transform(additive_node_entry.transform_2d);
			}

			Node3D *node_3d = Object::cast_to<Node3D>(additive_node_entry.node);
			if (node_3d) {
				node_3d->set_transform(additive_node_entry.transform_3d);
			}
		}
	}
        ...
}
~~~
A work around if we can't get this "in place" reload to work might be to first force save the scene, then detect when this smart reload fails (return false or -1 from "reload_instances_with_path_in_edited_scenes") and fall back to a dumb/slow method like just reloading the scene from the file.
</details>

@Hilderin do you mind taking a peek at this and seeing if anything sticks out to you regarding nested re-importing and the "reload_instances_with_path_in_edited_scenes" code around "// Attempt to re-add all the additional nodes."?
I can reproduce the crash in 4.4.stable and 4.3.stable, but not 4.2.stable.

Between 4.2 and 4.3, it seems like the crash starts with 4.3.rc3, the last RC before stable. This indeed points at #95225 as @brennennen identified.

In 4.3.rc2 and earlier, it doesn't crash, but prints this error:
```
ERROR: Basis [X: (0, 0, 0), Y: (0, 0, 0), Z: (0, 0, 0)] must be normalized in order to be casted to a Quaternion. Use get_rotation_quaternion() or call orthonormalized() if the Basis contains linearly independent vectors.
   at: get_quaternion (core/math/basis.cpp:725)
```
and occasionally the meshes disappear on reimport.

In both 4.3 and 4.2, the main scene fails loading at first with:
![Image](https://github.com/user-attachments/assets/164c3b10-b07a-4e2a-b3be-673b83e3d8b7)

But then double clicking it works fine.

Fix patch:
diff --git a/editor/editor_node.cpp b/editor/editor_node.cpp
index 520666faa839..0614aa55bd85 100644
--- a/editor/editor_node.cpp
+++ b/editor/editor_node.cpp
@@ -6457,6 +6457,18 @@ void EditorNode::reload_instances_with_path_in_edited_scenes() {
 
 			get_scene_editor_data_for_node(owner, original_node, scene_editor_data_table);
 
+			// The current node being reloaded may also be an additional node for another node
+			// that is in the process of being reloaded.
+			// Replacing the additional node with the new one prevents a crash where nodes
+			// in 'addition_list' are removed from the scene tree and queued for deletion.
+			for (InstanceModificationsEntry &im : scene_modifications->instance_list) {
+				for (AdditiveNodeEntry &additive_node_entry : im.addition_list) {
+					if (additive_node_entry.node == original_node) {
+						additive_node_entry.node = instantiated_node;
+					}
+				}
+			}
+
 			bool original_node_scene_instance_load_placeholder = original_node->get_scene_instance_load_placeholder();
 
 			// Delete all the remaining node children.


Test patch: