Overhaul scene loading and edit state management

[KoBeWi]

This PR makes a major overhaul on how scenes are loaded in the editor and how their edit state is handled.

• load_scene() was renamed to open_scene()
• Added new load_scene() method that only loads a scene. The scene is not even added to the scene tree.
  • open_scene() calls load_method() and then switches to the loaded scene.
• When a scene is loaded (using the new method), it's edit state is loaded into EditorData, instead of directly into plugins
• When a scene is loaded, the selected nodes are loaded into EditorData, instead of directly into EditorSelection
• When scenes are being restored on load, they are only loaded using the new load_scene().
  • Together with the simplified initialization, this should make scene restoring faster and more stable.
• When a scene is closed, its edit state is saved to a file.
  • Example fixed bug: Move 2D editor view, close the scene and open it again. The camera was being reset.
• Selected nodes are now stored as paths relative to root, instead of absolute paths.
  • This makes Node selection stored in editstate files resilient to editor structure changes
  • However it will cause one-time errors when loading old projects in 4.7 (I think?)
• Removed various deferred calls. Scenes now open completely, without running extra logic "sometime later"
  • This also allowed to simplify some code, like auto_select_current_scene_file setting
• Reloading a scene using a path that does not match any open scene now results in an error.
  • Seems like this made CI fail xd

Fixes #116867
Fixes #116865
Might fix some other issues.

[KoBeWi]

Resolving the issues required some more radical changes, because the old code was an irredeemable mess (and I didn't even include all improvements, I'll probably do follow-ups). See OP for the summary of changes. Likely needs testing. I did test it and scene opening/closing/reloading/changing/soft reloading/etc is working, but maybe I missed some edge case.

[nikitalita]

I have tested the PR extensively, both manually and with a monkey tester. It fixes the issue and it seems to have significantly improved reloading stability as well.

[nikitalita]

An issue that I found in additional testing:

If we call open_scene_from_path on a scene that is already open but is not currently in focus, and then call close_scene() on it on the same frame (which is enabled by this PR now), we cause a segfault on the next frame. I can reproduce this consistently.

This is due to the deferred call to Tree::scroll_to_item in SceneTreeEditor::_update_filter_helper()

Basically:
Open the scene -> SceneTreeEditor updates selection and scrolls to the first node TreeItem -> close scene -> SceneTreeEditor clears tree, freeing that TreeItem -> next frame, the call deferred crashes because the TreeItem parameter has been freed.

Stack trace up to that deferred call from open_scene_from_path():

SceneTreeEditor::_update_filter_helper(TreeItem*, bool, TreeItem*&) (/Users/nikita/Workspace/godot-ws/godot-pw/editor/scene/scene_tree_editor.cpp:1135)
SceneTreeEditor::_update_filter(TreeItem*, bool) (/Users/nikita/Workspace/godot-ws/godot-pw/editor/scene/scene_tree_editor.cpp:995)
SceneTreeEditor::set_filter(String const&) (/Users/nikita/Workspace/godot-ws/godot-pw/editor/scene/scene_tree_editor.cpp:1732)
SceneTreeDock::set_filter(String const&) (/Users/nikita/Workspace/godot-ws/godot-pw/editor/docks/scene_tree_dock.cpp:4235)
EditorNode::_set_main_scene_state(Dictionary const&) (/Users/nikita/Workspace/godot-ws/godot-pw/editor/editor_node.cpp:4580)
EditorNode::_set_current_scene_nocheck(int, bool) (/Users/nikita/Workspace/godot-ws/godot-pw/editor/editor_node.cpp:4666)
EditorNode::_set_current_scene(int) (/Users/nikita/Workspace/godot-ws/godot-pw/editor/editor_node.cpp:4612)
EditorNode::open_scene(String const&, bool, bool, bool) (/Users/nikita/Workspace/godot-ws/godot-pw/editor/editor_node.cpp:4855)
EditorInterface::open_scene_from_path(String const&, bool) (/Users/nikita/Workspace/godot-ws/godot-pw/editor/editor_interface.cpp:715)

[KoBeWi]

I can't reliably reproduce the crash. Does wrapping the item in Variant fix it for you?

callable_mp(tree, &Tree::scroll_to_item).call_deferred(Variant(p_parent), false);

[nikitalita]

@KoBeWi Yes it does, that resolves the issue.

[nikitalita]

@YeldhamDev Can you review this PR? I have extensively tested this and we are currently using it in production without issue.

[YeldhamDev]

When I try to open my test project it crashes, with this output:

ERROR: Can't use get_node() with absolute paths from outside the active scene tree.
   at: get_node_or_null (scene/main/node.cpp:1912)
ERROR: Can't use get_node() with absolute paths from outside the active scene tree.
   at: get_node_or_null (scene/main/node.cpp:1912)
While attempting to print an error, another error was printed:
ERROR: Attempted to free an uninitialized or invalid RID
   at: free (./core/templates/rid_owner.h:352)
free(): double free detected in tcache 2

[nikitalita]

Is this on Linux? Can you post your test project?

[YeldhamDev]

@nikitalita I am, but I don't think that's related. And I tried replicating it in a clean project, but no dice. It happens on a scene that has a child scene inside, and it's not always a crash, a lot of times it's just the Can't use get_node() with absolute paths from outside the active scene tree. error.

Still, I also experienced some rare crashes doing unrelated stuff, like creating a fresh scene. I'm sorry I cannot provide more info, or a reliable way to reproduce those at least.

[YeldhamDev]

Wait, I actually managed to find a reproducible bug with this:

• Open a project with a scene that has nodes in it.
• Click the add scene button on the top tabs. Do nothing with it and close the editor.
• Open the project again, the buttons that appear on new scene creation will be shown instead of the scene's nodes, and a lot of errors in the output.

[KoBeWi]

Fixed. The editor was trying to open non-existent current_scene.

[YeldhamDev]

I managed to find a reliable way of reproducing the Can't use get_node() with absolute paths from outside the active scene tree. error:

• Instance a child scene while using an editor version from the latest master.
• Open that project using a version from this PR.

[KoBeWi]

Yeah, I already noted that the paths change may cause one-time errors. It happens only when opening project from earlier versions, so it's not that critical I think.

[YeldhamDev]

Well, besides that issue, I couldn't find any other problems with it currently, so I guess it's fine. Would be nice if that error could be silenced somehow, but that's not a showstopper.

A second review would be appreciated.

[bruvzg]

Looks good. Having one time errors messages seems fine, but it probably should be mentioned on the Upgrading from Godot 4.6 to Godot 4.7 page.

[Repiteo]

Thanks!

[KoBeWi]

Having one time errors messages seems fine

So as it turns out, it's one time per scene (and not fixed until you save it). I think it can be fixed by renaming the key for selected nodes.