Fix errors when re-importing 3D asset files

[Rindbee]

Some 3D asset files (gltf/glb) are treated as scenes and may cause some errors when importing.

Manually updating the cache in advance resulted in some bugs when EditorNode::set_edited_scene() was automatically called for the second update.

When working through different scene tabs, we need to temporarily add the scene root to the SceneTree to ensure that editor_selection->add_node() can work smoothly. This avoids the error message: ERROR: Condition "!p_node->is_inside_tree()" is true.

This also ensures that no other scenes are accidentally added to the SceneTree causing the wrong display. When there is an inherited scene tab open and it is not the current tab, the new root node is accidentally added as a child node of scene_root during replacement.

Instantiate the scene early so caches in SceneState that are cleared due to loading are rebuilt early. This avoids numerous error messages: This operation requires the node cache to have been built.

Supersedes #89428.

Fixes #87451.

[lyuma]

I'll admit I don't understand this code very well, but currently I get randomly duplicated nodes and this crash constantly and it's driving me nuts.

If this PR has even a remote chance of solving that, I'm in. It can't possibly make things worse than they are now.

[KoBeWi]

The cube is still duplicated 🤔

godot.windows.editor.dev.x86_64_iwSv4PC1m2.mp4

It's gone when you switch scene tab.

[Rindbee]

Sorry for the late reply.

It reloads scene tabs one by one, so it's better to switch scene tabs first and then process them later.

In addition, it does not seem to save scene modifications when necessary, so re-importing resources may cause the scene to lose unsaved modifications.

[KoBeWi]

Something is still wrong:

E90hwCfN2Y.mp4

The level scene gets replaced by test (only visually). It doesn't always happen though. I also had a bug where test.tscn had 2 cubes.

[Rindbee]

Two scenes appear to be open in the 3D screen at the same time. It seems that get_tree()->set_edited_scene_root() is not switched correctly.

[KoBeWi]

Now it seems to work correctly, but there is an error spam 🙃

godot.windows.editor.dev.x86_64_9hPF05HkJ5.mp4

Also the current changes make updating scenes after reimport much more expensive. Though maybe it doesn't matter unless someone opens, like, 50 scenes with instances to update.

[Rindbee]

godot/editor/editor_node.cpp

Line 5658 in 2149682

Ref<PackedScene> instance_scene_packed_scene = ResourceLoader::load(p_instance_path, "", ResourceFormatLoader::CACHE_MODE_REPLACE, &err);

Reloading a PackedScene clears the cache, which is not rebuilt until instantiate() is called.

godot/scene/resources/packed_scene.cpp

Lines 1279 to 1280 in 8e2141e

	int SceneState::find_node_by_path(const NodePath &p_node) const {
	ERR_FAIL_COND_V_MSG(node_path_cache.is_empty(), -1, "This operation requires the node cache to have been built.");

[Rindbee]

If the scene has unsaved modifications before reimporting, these modifications will not be saved after reimporting, but the unsaved mark (*) will disappear.

Should these modifications be saved before reimporting?

[KoBeWi]

IIRC reloading a scene tries to re-apply unsaved changes, so nothing should be lost. Though maybe it's different for regular instances vs imported scenes.

[Rindbee]

But it doesn't prompt to save when closing the tab.

Peek.2024-05-23.19-13.mp4

[KoBeWi]

That's because the scene does not appear unsaved. It should retain the unsaved status after soft-reload.

[KoBeWi]

Needs rebase due to conflicts >_>

[Rindbee]

The conflict has been resolved.

The latest commit in master no longer restores the property changes caused by the import, which should refresh the 3D Screen to show the latest changes, but it seems that only the inherited scene will be refreshed, not other scenes. It needs to be reloaded to refresh.

[akien-mga]

Thanks!

[Bonkahe]

I apologize for adding a comment here, not sure if I should have just posted it in the issue (bit new to issues and all of this honestly), but here goes.
I believe this commit is causing an issue, I posted an issue here: #92623

While this appears to be an intended result, this is more or less destroying my functionality, or I suppose to some degree a lot of tool functionality that depends on enter_tree, in my case rebuilding my terrain every time I reimport any resource, this includes the resources that are part of the terrain being generated, resulting in an endless loop.

I believe the implementation of this commit should be re-reviewed, as this method of fixing the errors when re-importing assets has some pretty impactful downsides.