Fix dock visibility issues #84122

kitbdev · 2023-10-28T17:44:25Z

Fix dock visibility for Editor Features and distraction free mode.

fixes Editor dock rendering is broken #66834
fixes Godot crashes when changing Editor Features when certain docks are floating #84125 crash when changing Editor Features with floating docks (import, filesystem, node, or history)
fixes error when closing floating dock if there are less tabs than before
fixes loading layouts in distraction free mode being very buggy (bottom dock disappears, side docks might open and be empty)
fixes floating dock still opened even if it was disabled in Editor Features when loading layout

Updated TabContainer's move_tab_from_tab_container to preserve all tab data.

The crash fix is a workaround, ideally it wouldn't have to close all floating docks. I am working on a different pr that should fix this in the future.

There were 4 separate implementations to update the dock slots visibility:
update_dock_slots_visibility(), _update_dock_containers(), in _load_docks_from_config(), and _dock_tab_changed() (removed the lines affecting bottom docks for this one, see #39177).
I have consolidated them all in update_dock_slots_visibility().

scene/gui/tab_container.cpp

kitbdev · 2023-10-28T20:41:24Z

Since this fixes a crash, should it be for 4.2?

AThousandShips · 2023-10-28T20:46:31Z

Can you please open an issue for the crash part, I don't see it reported in the linked issue, a crash is serious enough to track actively

Especially clear steps to replicate the crash so we can confirm it is fixed

AThousandShips · 2023-10-28T20:52:28Z

But note also that we're in the late stage of the 4.2 cycle and focus right now is on bugs introduced in 4.2 and fixing them so 4.2 introduces as few new bugs as possible, and as some of this behaviour goes back to 4.0 it's lower priority right now, marked for cherry picking for 4.2 so it can be added to it for 4.2.1 if it doesn't meet the deadline

kitbdev · 2023-10-28T20:57:38Z

Made an issue for the crash: #84125
If this isn't cherrypick-able I can create a smaller pr that is just the crash fix if needed.

YeldhamDev

Can't reproduce #66834 to confirm, but everything else seems alright.

YuriSizov · 2023-10-30T11:12:11Z

I'm curious if @kitbdev could reproduce #66834, or if this is an assumed fix based on code changes.

YuriSizov · 2023-10-30T11:19:04Z

scene/gui/tab_container.cpp

 	// Get the tab properties before they get erased by the child removal.
 	String tab_title = p_from->get_tab_title(p_from_index);
 	Ref<Texture2D> tab_icon = p_from->get_tab_icon(p_from_index);
+	Ref<Texture2D> tab_button_icon = p_from->get_tab_button_icon(p_from_index);
 	bool tab_disabled = p_from->is_tab_disabled(p_from_index);
+	bool tab_hidden = p_from->is_tab_hidden(p_from_index);
 	Variant tab_metadata = p_from->get_tab_metadata(p_from_index);
+	int tab_icon_max_width = p_from->get_tab_bar()->get_tab_icon_max_width(p_from_index);

 	Control *moving_tabc = p_from->get_tab_control(p_from_index);
 	p_from->remove_child(moving_tabc);
 	add_child(moving_tabc, true);

-	set_tab_title(get_tab_count() - 1, tab_title);
-	set_tab_icon(get_tab_count() - 1, tab_icon);
-	set_tab_disabled(get_tab_count() - 1, tab_disabled);
-	set_tab_metadata(get_tab_count() - 1, tab_metadata);
-
 	if (p_to_index < 0 || p_to_index > get_tab_count() - 1) {
 		p_to_index = get_tab_count() - 1;
 	}
 	move_child(moving_tabc, get_tab_control(p_to_index)->get_index(false));
+
+	set_tab_title(p_to_index, tab_title);
+	set_tab_icon(p_to_index, tab_icon);
+	set_tab_button_icon(p_to_index, tab_button_icon);
+	set_tab_disabled(p_to_index, tab_disabled);
+	set_tab_hidden(p_to_index, tab_hidden);
+	set_tab_metadata(p_to_index, tab_metadata);
+	get_tab_bar()->set_tab_icon_max_width(p_to_index, tab_icon_max_width);
+


This isn't really maintainable. I think we should create a method in TabBar that returns a copy of the entire Tab struct for one tab and add API to TabContainer which can fetch it and apply it to one of its tabs. It doesn't need to be exposed to scripting, just public for internal use.

To be clear, this is not a requirement for this PR, but with the number of things we need to keep track of, it's a bigger concern now.

editor/editor_node.cpp

YuriSizov

Seems like a good refactoring. I'm not sure about the workaround for the crash. There are no other reports of it, it seems, and this is not a proper fix. So maybe it's not urgent to fix in 4.2 (so the entire PR can be pushed to 4.3, with maybe a cherry-pick later).

kitbdev · 2023-10-30T14:26:53Z

I added some reproduction steps on #66834

I only found the crash since I was extracting all dock logic from EditorNode (#84193) and found some issues and thought they should be applicable to 4.2, so I fixed them without the extraction.
I'm working on a different pr (open and close docks) that already has a proper fix for the crash.
So we can wait for that if we want to.

YuriSizov · 2023-10-30T14:46:15Z

Since this PR is pretty much ready (just too late for 4.2), I think that we should merge it first, and then discuss your other PR built on top of this.

kitbdev · 2023-10-30T14:54:35Z

rebased, changed workaround comment to FIXME, and used set_visible

YuriSizov · 2023-12-08T14:33:25Z

Thanks!

YuriSizov · 2024-01-24T15:12:06Z

Cherry-picked for 4.2.2.

YuriSizov · 2024-01-24T17:39:07Z

~~Cherry-picked for 4.1.4.~~

Edit: Not cherry-picked in the end, can't be done cleanly due to other dependencies (such as #79104) and because #83637 can't be cleanly cherry-picked either.

If we want these fixes in 4.1.4, we need a dedicated PR for the 4.1 branch.

kitbdev requested a review from a team as a code owner October 28, 2023 17:44

AThousandShips added bug topic:editor topic:gui labels Oct 28, 2023

AThousandShips added this to the 4.3 milestone Oct 28, 2023

YeldhamDev reviewed Oct 28, 2023

View reviewed changes

scene/gui/tab_container.cpp Outdated Show resolved Hide resolved

kitbdev force-pushed the fix-dock-visibility branch from a11d1b2 to 43ede54 Compare October 28, 2023 19:06

AThousandShips added cherrypick:4.1 Considered for cherry-picking into a future 4.1.x release cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release labels Oct 28, 2023

YeldhamDev approved these changes Oct 30, 2023

View reviewed changes

YuriSizov reviewed Oct 30, 2023

View reviewed changes

editor/editor_node.cpp Outdated Show resolved Hide resolved

YuriSizov reviewed Oct 30, 2023

View reviewed changes

editor/editor_node.cpp Outdated Show resolved Hide resolved

YuriSizov approved these changes Oct 30, 2023

View reviewed changes

kitbdev mentioned this pull request Oct 30, 2023

Extract EditorDockManager from EditorNode #84193

Merged

kitbdev force-pushed the fix-dock-visibility branch from 43ede54 to 82ecff1 Compare October 30, 2023 14:51

fix dock visibility issues

a267446

kitbdev force-pushed the fix-dock-visibility branch from 82ecff1 to a267446 Compare October 30, 2023 14:53

YuriSizov merged commit 6c86974 into godotengine:master Dec 8, 2023
15 checks passed

YuriSizov added the crash label Dec 8, 2023

YuriSizov removed the cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release label Jan 24, 2024

kitbdev deleted the fix-dock-visibility branch January 24, 2024 16:31

YuriSizov mentioned this pull request Jan 24, 2024

Fix TabBar and TabContainer dragging issues #83637

Merged

YuriSizov removed the cherrypick:4.1 Considered for cherry-picking into a future 4.1.x release label Jan 24, 2024

This was referenced Feb 2, 2024

bottom pannel can disapear #80746

Closed

Docks do not clear from editor layout when disabled in Editor Features #68843

Closed

Refactor and fix issues in Editor Dock Manager #88003

Merged

kitbdev mentioned this pull request Mar 16, 2024

History Editor comes back when disabled #89549

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix dock visibility issues #84122

Fix dock visibility issues #84122

kitbdev commented Oct 28, 2023 •

edited

Loading

kitbdev commented Oct 28, 2023

AThousandShips commented Oct 28, 2023 •

edited

Loading

AThousandShips commented Oct 28, 2023

kitbdev commented Oct 28, 2023

YeldhamDev left a comment

YuriSizov commented Oct 30, 2023

YuriSizov Oct 30, 2023 •

edited

Loading

YuriSizov Oct 30, 2023

YuriSizov left a comment

kitbdev commented Oct 30, 2023 •

edited

Loading

YuriSizov commented Oct 30, 2023

kitbdev commented Oct 30, 2023

YuriSizov commented Dec 8, 2023

YuriSizov commented Jan 24, 2024

YuriSizov commented Jan 24, 2024 •

edited

Loading

Fix dock visibility issues #84122

Fix dock visibility issues #84122

Conversation

kitbdev commented Oct 28, 2023 • edited Loading

kitbdev commented Oct 28, 2023

AThousandShips commented Oct 28, 2023 • edited Loading

AThousandShips commented Oct 28, 2023

kitbdev commented Oct 28, 2023

YeldhamDev left a comment

Choose a reason for hiding this comment

YuriSizov commented Oct 30, 2023

YuriSizov Oct 30, 2023 • edited Loading

Choose a reason for hiding this comment

YuriSizov Oct 30, 2023

Choose a reason for hiding this comment

YuriSizov left a comment

Choose a reason for hiding this comment

kitbdev commented Oct 30, 2023 • edited Loading

YuriSizov commented Oct 30, 2023

kitbdev commented Oct 30, 2023

YuriSizov commented Dec 8, 2023

YuriSizov commented Jan 24, 2024

YuriSizov commented Jan 24, 2024 • edited Loading

kitbdev commented Oct 28, 2023 •

edited

Loading

AThousandShips commented Oct 28, 2023 •

edited

Loading

YuriSizov Oct 30, 2023 •

edited

Loading

kitbdev commented Oct 30, 2023 •

edited

Loading

YuriSizov commented Jan 24, 2024 •

edited

Loading