Extract EditorDockManager from EditorNode

[kitbdev]

Moves most code about docks, dock management, and dock select popup from EditorNode to EditorDockManager.
There should be no behavioral changes in this pr.

Details

• Created new class EditorDockManager singleton to encapsulate docks and dock select logic from EditorNode
• EditorNode passes the dock slots and dock splits to EditorDockManager
• Created DockSplitContainer : SplitContainer that automatically shows if any children are visible, otherwise it hides
• EditorNode still adds the Scene, FileSystem, and other docks but it uses EditorDockManager to do it.
• Made save_docks_to_config, load_docks_from_config, update_dock_slots_visibility public in EditorDockManager to be accessible from EditorNode.
• Made close_all_floating_docks as part of the workaround for Godot crashes when changing Editor Features when certain docks are floating #84125. This can be removed after the proper fix, if wanted.
• Added signal layout_changed in EditorDockManager to trigger EditorNode::_save_editor_layout(). This could probably be changed to EditorNode::get_singleton()->save_editor_layout_delayed() if the delay is fine.
• EditorPlugin and VersionControlEditorPlugin now use EditorDockManager to add and remove docks
• Added optional name parameter to add_control_to_dock, since the default docks use a custom title on the tabs. This is not exposed to users.
• Changed some parameter names to be better (ofs -> p_offset, p_which -> p_dock_slot)

[YuriSizov]

• EditorDockManager is a MarginContainer that is the second child of main_vbox (under the title bar) and it handles the SplitContainers under that. It exposes its main_hsplit so EditorNode can handle the center panel and bottom panel.

Why is it a node at all?

There is no reason for this. It should only handle the docks themselves, and if you insert it as a node and as a node in this particular place, it automatically becomes a parent to not only docks, but also to the main editor and the bottom panel.

I think docks and dockable splits should be made into a reusable control, and a manager class can be introduced to handle them without becoming a key node in the editor layout tree.

Edit: This allows us to have a modular approach when it comes to how the layout works. This will be important if we want to make floating windows into mini editors with their own dockable areas.

[kitbdev]

I only made it a node so it can take all of the SplitContainers and dock slots from EditorNode, without needing to pass each of them to the manager.

I was going to look into doing godotengine/godot-proposals#1984 later, but I like your modular idea so I'll look into that.

[YuriSizov]

I only made it a node so it can take all of the SplitContainers and dock slots from EditorNode, without needing to pass each of them to the manager.

I have plans to completely divorce the UI part of the editor from EditorNode, there would be an EditorLayout node or something, and it can handle registering dockable areas and what not. Then this kind of layout node would also be used by floating windows.

But that shouldn't affect your PR much. I just don't think that making a dock manager a root of most UI is correct for its functionality.

Edit: By the way, feel free to tag me on RocketChat if you want to figure out a solution together!

[trollodel]

Cool. I actually wanted to do this after #62378, but I didn't find the time.
What's the status of this?

[kitbdev]

@trollodel after #84122 is merged, I'll rebase this and then I think it will be ready for review

[YuriSizov]

You can rebase this now and we can look into getting it across the finish line 🙂

[kitbdev]

Rebased and updated initial comment to be accurate to the changes. The Dock Manager is now an Object and the dock slots are passed into it from the EditorNode. The Dock Manager still handles all of the dock select Controls. Added DockSplitContainer to handle automatically showing and hiding the SplitContainer.

[KoBeWi]

This is how it looks in master, I thought it was intentional?

No, this is how it looks on master:

The space between 1st and 2nd element is smaller, but it's there. Though it makes it look more obviously like a bug.

[kitbdev]

No, this is how it looks on master:

Hmm, this is how master looks for me:

Godot v4.3.dev (26b1fd0) - Windows 10.0.22621 - Vulkan (Forward+)

Looking at _dock_select_draw(), I didn't change anything, so this is weird.

Edit:
Looks like there is only 1px of spacing, so the resolution matters a lot and it looks to be inconsistent based on where it is.
UI Scale set to 75%:

UI Scale set to 125%:

[YuriSizov]

This popup (with all of its layout) could probably be extracted into its own class, it contains plenty of code which doesn't need to be shared.

You also won't need to connect to theme_changed this way, you will be able to use the notification.

[YuriSizov]

That said, this can probably be done in a follow-up.

[YuriSizov]

Some changes are necessary, but overall this is a very clean and welcome extraction. Great job!

[YuriSizov]

Regarding the drawing of the popup the code seems identical to me. But it's not great to begin with, it doesn't account for the scale, I think. So glitches are possible, and I'm pretty sure I've seen it misbehave like that before. It needs a lot of improvements, really, but I don't think this PR breaks it.

[YuriSizov]

LGTM now, aside from the missing empty line.

[akien-mga]

Thanks!