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

Extract EditorMainScreen from EditorNode #96389

Merged
merged 1 commit into from
Sep 11, 2024

Conversation

kitbdev
Copy link
Contributor

@kitbdev kitbdev commented Aug 31, 2024

Extracts all functionality related to the main screen from EditorNode.
Behavior should not change.

Details

In _get_main_scene_state(), removed the main_tab key. I don't think it has been used since 081a236.
In _set_main_scene_state(), removed the editor_index key check. Only used between 334a818 and #11075 (less than a month during 3.0 dev).
The state in only actually saved and used in _save_central_editor_layout_to_config() and _load_central_editor_layout_from_config(). _get/set_main_scene_state() is just used for switching between 2D/3D when opening a scene or changing selection to match the node.

Removed distraction_free->move_to_front(); from EditorNode::add_editor_plugin since it isn't needed. The distraction free button is already after everything else in the SceneTabs, and if it was needed it should be somewhere else.

set_visible_editor() and editor_select() did the same thing. They are now EditorMainScreen::select().
Renamed editor_plugin_screen -> selected_plugin, and removed some other editor_main prefixes.

The scene_root_parent is now the editor_main_screen. It is not actually the parent of the scene root, and this way I don't need to add a new control to hold the vbox.
I could make the editor_main_screen actually be the vbox instead of being parent to it, and the scene_root_parent can be called main_screen_parent, if that is better.

Also removed unused forward declarations in EditorNode.

@kitbdev kitbdev requested review from a team as code owners August 31, 2024 15:17
@AThousandShips AThousandShips added this to the 4.x milestone Aug 31, 2024
@kitbdev kitbdev changed the title Extract editor main screen Extract EditorMainScreen from EditorNode Aug 31, 2024
editor/editor_node.cpp Outdated Show resolved Hide resolved
editor/editor_node.cpp Outdated Show resolved Hide resolved
@KoBeWi

This comment was marked as resolved.

@kitbdev kitbdev force-pushed the extract-main-screen branch 2 times, most recently from 57df8fd to ccc04d3 Compare September 9, 2024 01:46
@kitbdev
Copy link
Contributor Author

kitbdev commented Sep 9, 2024

Fixed crash, it was because I used get_plugin_index which can return -1 but I didn't check for it. Also added ERR_FAIL_INDEX_V so it won't crash if it happens again.

Only plugins with a main screen are now added as children to the main screen control, otherwise they use gui_base.

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 fine.
I noticed some potential improvements that could be done in a follow-up, e.g. merging buttons and editor_table by using a struct.

editor/editor_main_screen.cpp Outdated Show resolved Hide resolved
editor/editor_main_screen.cpp Outdated Show resolved Hide resolved
editor/editor_main_screen.h Outdated Show resolved Hide resolved
@akien-mga akien-mga modified the milestones: 4.x, 4.4 Sep 10, 2024
@akien-mga akien-mga merged commit b6906b9 into godotengine:master Sep 11, 2024
20 checks passed
@akien-mga
Copy link
Member

Thanks!

@kitbdev kitbdev deleted the extract-main-screen branch September 11, 2024 15:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants