Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor and fix issues in Editor Dock Manager #88003

Merged
1 commit merged into from
Mar 24, 2024

Conversation

kitbdev
Copy link
Contributor

@kitbdev kitbdev commented Feb 6, 2024

Implementation Details

EditorDockManager

  • Added DockInfo struct and a HashMap<Control *, DockInfo> of all docks to be able to easily access information on docks.
    • This replaces the bottom_docks Vector.
  • Replaced _edit_current() with _update_layout() to handle all updating that needs to happen after any dock moves. Previously some actions like floating a dock wouldn't save the layout, so it wouldn't restore if a crash occurs.
  • Docks disabled with Editor features are moved out of the dock and hidden, instead of using hidden tabs. They use set_dock_enabled() which sets a flag and closes or opens the dock.
  • close_dock() will hide the dock and move it out of the tab container, so it doesn't affect anything and isn't seen in the dock menu. open_dock() will restore the dock to its previous location. These will also be useful for Allow opening and closing any editor dock godot-proposals#8206
  • Saving a closed disabled dock will still add it to a slot so we know where to restore it to after loading, just like floating windows.
  • Saving bottom docks was changed to do the same so it is more like closed and floating docks. This means that previous layouts with a bottom dock will need to be resaved.
  • Simplified and made private _update_dock_slots_visibility() because hidden tabs no longer need to be checked.
  • Added _move_dock() to handle moving the dock, to make sure it is always properly handled when removed from the bottom panel or a floating window, and making sure the title is set when moving to a TabContainer.
  • focus_dock() handles focusing on a dock no matter where it is.

Extracts the dock popup into its own class DockContextPopup. (#84193 (comment))

  • It now operates on a dock Control, rather than indexes for the dock and bottom panel item.
  • Added tooltips for the L/R move tab buttons.
  • The docks are drawn now at the appropriate position when in rtl.
  • Editor scale is now used for tab drawing.
    • Before (150%):
      image
    • After (150%):
      image
  • Size issues when buttons were added/removed are fixed.
    • Before:
      Screenshot 2024-02-05 172557
    • After:
      image
  • Docks at the bottom panel can now float directly (Allow to move FileSystem dock to bottom and drag resources across bottom docks #86765 (comment)).
  • Tabs can be right clicked to open the popup.

@kitbdev kitbdev requested review from a team as code owners February 6, 2024 01:33
@Calinou Calinou added this to the 4.3 milestone Feb 6, 2024
@akien-mga akien-mga added the crash label Feb 6, 2024
@kitbdev
Copy link
Contributor Author

kitbdev commented Feb 10, 2024

Changes:

  • Removed the layout_changed signal and instead use save_editor_layout_delayed(). This prevents unnecessarily saving multiple times in a row. This saving is only really in case of a crash, and it doesn't need to happen immediately.
  • _dock_container_update_visibility() replaces _update_dock_slots_visibility(), and it just updates the one tab container's visibility when it is needed, when a child is added or removed.
  • set_docks_visible() handle updating the dock slots visibility itself.
  • Connected dock slots to active_tab_rearranged to make sure layout is updated when dragging a tab within itself.
  • Made sure _update_layout() only happens once after docks have been moved.
  • Changed DockInfo name to title to be more accurate and better distinguish from when the Control's name is used instead.
  • Fix Bottom Dock context menu position when not in fullscreen.
  • Load layout doesn't need to update layout because the layout will be saved after it is called.
  • Rebased.

Does EditorNode::edit_current() need to be called whenever the layout changes?
I have it there because it was being called when the layout changed in some cases, but I don't know if it is actually needed.

@kitbdev kitbdev force-pushed the dock-manager-fixes branch 2 times, most recently from 2c77f3f to 069d59b Compare February 29, 2024 18:08
@kitbdev kitbdev force-pushed the dock-manager-fixes branch from 069d59b to c3a0f9c Compare March 6, 2024 00:47
@kitbdev
Copy link
Contributor Author

kitbdev commented Mar 6, 2024

Rebased to fix merge conflicts.
Shortcut is now saved in dock info instead of in the meta.
Also fixed to show translated dock title in floating window.

Copy link
Member

@KoBeWi KoBeWi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great overall.

editor/editor_dock_manager.h Outdated Show resolved Hide resolved
editor/editor_dock_manager.h Outdated Show resolved Hide resolved
editor/editor_dock_manager.cpp Outdated Show resolved Hide resolved
editor/editor_dock_manager.cpp Show resolved Hide resolved
editor/editor_dock_manager.cpp Show resolved Hide resolved
@kitbdev kitbdev force-pushed the dock-manager-fixes branch from c3a0f9c to 4d31991 Compare March 16, 2024 16:51
@kitbdev kitbdev force-pushed the dock-manager-fixes branch from 4d31991 to 0c9c84f Compare March 16, 2024 17:51
akien-mga added a commit that referenced this pull request Mar 24, 2024
Refactor and fix issues in Editor Dock Manager
@akien-mga akien-mga closed this pull request by merging all changes into godotengine:master in 06abc86 Mar 24, 2024
@akien-mga
Copy link
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants