From 01888ae7ab2d8989b6e25cb2edbe7b3d27cf4e4e Mon Sep 17 00:00:00 2001 From: Yuri Sizov Date: Mon, 4 Dec 2023 13:18:48 +0100 Subject: [PATCH] Fix theme application in various editor dialogs 99% of the time we shouldn't rely on the signal, we should use the notification instead. I left some comments in places where I couldn't quickly improve the code. --- editor/editor_about.cpp | 42 +++++++++++------------- editor/editor_about.h | 2 -- editor/editor_command_palette.cpp | 13 ++++---- editor/editor_command_palette.h | 3 +- editor/editor_quick_open.cpp | 9 +++-- editor/editor_quick_open.h | 2 -- editor/export/project_export.cpp | 12 +++---- editor/export/project_export.h | 1 - editor/plugins/path_3d_editor_plugin.cpp | 3 ++ editor/plugins/tiles/tile_map_editor.cpp | 8 +++-- editor/shader_create_dialog.cpp | 31 +++++------------ editor/shader_create_dialog.h | 1 - 12 files changed, 54 insertions(+), 73 deletions(-) diff --git a/editor/editor_about.cpp b/editor/editor_about.cpp index f6360db5692..672697bab03 100644 --- a/editor/editor_about.cpp +++ b/editor/editor_about.cpp @@ -39,29 +39,25 @@ // The metadata key used to store and retrieve the version text to copy to the clipboard. const String EditorAbout::META_TEXT_TO_COPY = "text_to_copy"; -void EditorAbout::_theme_changed() { - const Ref font = get_theme_font(SNAME("source"), EditorStringName(EditorFonts)); - const int font_size = get_theme_font_size(SNAME("source_size"), EditorStringName(EditorFonts)); - - _tpl_text->begin_bulk_theme_override(); - _tpl_text->add_theme_font_override("normal_font", font); - _tpl_text->add_theme_font_size_override("normal_font_size", font_size); - _tpl_text->add_theme_constant_override("line_separation", 4 * EDSCALE); - _tpl_text->end_bulk_theme_override(); - - _license_text->begin_bulk_theme_override(); - _license_text->add_theme_font_override("normal_font", font); - _license_text->add_theme_font_size_override("normal_font_size", font_size); - _license_text->add_theme_constant_override("line_separation", 4 * EDSCALE); - _license_text->end_bulk_theme_override(); - - _logo->set_texture(get_editor_theme_icon(SNAME("Logo"))); -} - void EditorAbout::_notification(int p_what) { switch (p_what) { - case NOTIFICATION_ENTER_TREE: { - _theme_changed(); + case NOTIFICATION_THEME_CHANGED: { + const Ref font = get_theme_font(SNAME("source"), EditorStringName(EditorFonts)); + const int font_size = get_theme_font_size(SNAME("source_size"), EditorStringName(EditorFonts)); + + _tpl_text->begin_bulk_theme_override(); + _tpl_text->add_theme_font_override("normal_font", font); + _tpl_text->add_theme_font_size_override("normal_font_size", font_size); + _tpl_text->add_theme_constant_override("line_separation", 4 * EDSCALE); + _tpl_text->end_bulk_theme_override(); + + _license_text->begin_bulk_theme_override(); + _license_text->add_theme_font_override("normal_font", font); + _license_text->add_theme_font_size_override("normal_font_size", font_size); + _license_text->add_theme_constant_override("line_separation", 4 * EDSCALE); + _license_text->end_bulk_theme_override(); + + _logo->set_texture(get_editor_theme_icon(SNAME("Logo"))); } break; } } @@ -128,12 +124,12 @@ EditorAbout::EditorAbout() { set_hide_on_ok(true); VBoxContainer *vbc = memnew(VBoxContainer); - vbc->connect("theme_changed", callable_mp(this, &EditorAbout::_theme_changed)); + add_child(vbc); + HBoxContainer *hbc = memnew(HBoxContainer); hbc->set_h_size_flags(Control::SIZE_EXPAND_FILL); hbc->set_alignment(BoxContainer::ALIGNMENT_CENTER); hbc->add_theme_constant_override("separation", 30 * EDSCALE); - add_child(vbc); vbc->add_child(hbc); _logo = memnew(TextureRect); diff --git a/editor/editor_about.h b/editor/editor_about.h index 0bc863e3c1a..22b5f3ded0d 100644 --- a/editor/editor_about.h +++ b/editor/editor_about.h @@ -64,8 +64,6 @@ private: RichTextLabel *_tpl_text = nullptr; TextureRect *_logo = nullptr; - void _theme_changed(); - protected: void _notification(int p_what); static void _bind_methods(); diff --git a/editor/editor_command_palette.cpp b/editor/editor_command_palette.cpp index da970980eb9..6626251ee67 100644 --- a/editor/editor_command_palette.cpp +++ b/editor/editor_command_palette.cpp @@ -129,8 +129,8 @@ void EditorCommandPalette::_update_command_search(const String &search_text) { section->set_text(0, item_name); section->set_selectable(0, false); section->set_selectable(1, false); - section->set_custom_bg_color(0, search_options->get_theme_color(SNAME("prop_subsection"), EditorStringName(Editor))); - section->set_custom_bg_color(1, search_options->get_theme_color(SNAME("prop_subsection"), EditorStringName(Editor))); + section->set_custom_bg_color(0, get_theme_color(SNAME("prop_subsection"), EditorStringName(Editor))); + section->set_custom_bg_color(1, get_theme_color(SNAME("prop_subsection"), EditorStringName(Editor))); sections[section_name] = section; } @@ -164,6 +164,10 @@ void EditorCommandPalette::_notification(int p_what) { was_showed = true; } } break; + + case NOTIFICATION_THEME_CHANGED: { + command_search_box->set_right_icon(get_editor_theme_icon(SNAME("Search"))); + } break; } } @@ -303,10 +307,6 @@ Ref EditorCommandPalette::add_shortcut_command(const String &p_command return p_shortcut; } -void EditorCommandPalette::_theme_changed() { - command_search_box->set_right_icon(search_options->get_editor_theme_icon(SNAME("Search"))); -} - void EditorCommandPalette::_save_history() const { Dictionary command_history; @@ -330,7 +330,6 @@ EditorCommandPalette::EditorCommandPalette() { connect("confirmed", callable_mp(this, &EditorCommandPalette::_confirmed)); VBoxContainer *vbc = memnew(VBoxContainer); - vbc->connect("theme_changed", callable_mp(this, &EditorCommandPalette::_theme_changed)); add_child(vbc); command_search_box = memnew(LineEdit); diff --git a/editor/editor_command_palette.h b/editor/editor_command_palette.h index d2f25f6ec55..01fa6896a9e 100644 --- a/editor/editor_command_palette.h +++ b/editor/editor_command_palette.h @@ -83,10 +83,9 @@ class EditorCommandPalette : public ConfirmationDialog { float _score_path(const String &p_search, const String &p_path); void _sbox_input(const Ref &p_ie); void _confirmed(); - void _update_command_keys(); void _add_command(String p_command_name, String p_key_name, Callable p_binded_action, String p_shortcut_text = "None"); - void _theme_changed(); void _save_history() const; + EditorCommandPalette(); protected: diff --git a/editor/editor_quick_open.cpp b/editor/editor_quick_open.cpp index 82313c12e2e..965d0269d6f 100644 --- a/editor/editor_quick_open.cpp +++ b/editor/editor_quick_open.cpp @@ -250,23 +250,22 @@ void EditorQuickOpen::_notification(int p_what) { } } break; + case NOTIFICATION_THEME_CHANGED: { + search_box->set_right_icon(get_editor_theme_icon(SNAME("Search"))); + } break; + case NOTIFICATION_EXIT_TREE: { disconnect("confirmed", callable_mp(this, &EditorQuickOpen::_confirmed)); } break; } } -void EditorQuickOpen::_theme_changed() { - search_box->set_right_icon(search_options->get_editor_theme_icon(SNAME("Search"))); -} - void EditorQuickOpen::_bind_methods() { ADD_SIGNAL(MethodInfo("quick_open")); } EditorQuickOpen::EditorQuickOpen() { VBoxContainer *vbc = memnew(VBoxContainer); - vbc->connect("theme_changed", callable_mp(this, &EditorQuickOpen::_theme_changed)); add_child(vbc); search_box = memnew(LineEdit); diff --git a/editor/editor_quick_open.h b/editor/editor_quick_open.h index 2b64efb151d..ec8ce0175e2 100644 --- a/editor/editor_quick_open.h +++ b/editor/editor_quick_open.h @@ -72,8 +72,6 @@ class EditorQuickOpen : public ConfirmationDialog { void _sbox_input(const Ref &p_ie); void _text_changed(const String &p_newtext); - void _theme_changed(); - protected: void _notification(int p_what); static void _bind_methods(); diff --git a/editor/export/project_export.cpp b/editor/export/project_export.cpp index 719c3114f4b..cacdf108cf9 100644 --- a/editor/export/project_export.cpp +++ b/editor/export/project_export.cpp @@ -90,11 +90,6 @@ ProjectExportTextureFormatError::ProjectExportTextureFormatError() { fix_texture_format_button->connect("pressed", callable_mp(this, &ProjectExportTextureFormatError::_on_fix_texture_format_pressed)); } -void ProjectExportDialog::_theme_changed() { - duplicate_preset->set_icon(presets->get_editor_theme_icon(SNAME("Duplicate"))); - delete_preset->set_icon(presets->get_editor_theme_icon(SNAME("Remove"))); -} - void ProjectExportDialog::_notification(int p_what) { switch (p_what) { case NOTIFICATION_VISIBILITY_CHANGED: { @@ -103,6 +98,11 @@ void ProjectExportDialog::_notification(int p_what) { } } break; + case NOTIFICATION_THEME_CHANGED: { + duplicate_preset->set_icon(presets->get_editor_theme_icon(SNAME("Duplicate"))); + delete_preset->set_icon(presets->get_editor_theme_icon(SNAME("Remove"))); + } break; + case NOTIFICATION_READY: { duplicate_preset->set_icon(presets->get_editor_theme_icon(SNAME("Duplicate"))); delete_preset->set_icon(presets->get_editor_theme_icon(SNAME("Remove"))); @@ -1161,8 +1161,8 @@ ProjectExportDialog::ProjectExportDialog() { set_clamp_to_embedder(true); VBoxContainer *main_vb = memnew(VBoxContainer); - main_vb->connect("theme_changed", callable_mp(this, &ProjectExportDialog::_theme_changed)); add_child(main_vb); + HSplitContainer *hbox = memnew(HSplitContainer); main_vb->add_child(hbox); hbox->set_v_size_flags(Control::SIZE_EXPAND_FILL); diff --git a/editor/export/project_export.h b/editor/export/project_export.h index 30f6812acbd..1a359b08dab 100644 --- a/editor/export/project_export.h +++ b/editor/export/project_export.h @@ -188,7 +188,6 @@ class ProjectExportDialog : public ConfirmationDialog { void _tab_changed(int); protected: - void _theme_changed(); void _notification(int p_what); static void _bind_methods(); diff --git a/editor/plugins/path_3d_editor_plugin.cpp b/editor/plugins/path_3d_editor_plugin.cpp index 9d66e606b09..42c080fdf27 100644 --- a/editor/plugins/path_3d_editor_plugin.cpp +++ b/editor/plugins/path_3d_editor_plugin.cpp @@ -713,6 +713,9 @@ void Path3DEditorPlugin::_notification(int p_what) { } break; case NOTIFICATION_READY: { + // FIXME: This can trigger theme updates when the nodes that we want to update are not yet available. + // The toolbar should be extracted to a dedicated control and theme updates should be handled through + // the notification. Node3DEditor::get_singleton()->connect("theme_changed", callable_mp(this, &Path3DEditorPlugin::_update_theme)); } break; } diff --git a/editor/plugins/tiles/tile_map_editor.cpp b/editor/plugins/tiles/tile_map_editor.cpp index 6bee3660cf7..a6de74a1d29 100644 --- a/editor/plugins/tiles/tile_map_editor.cpp +++ b/editor/plugins/tiles/tile_map_editor.cpp @@ -2373,7 +2373,9 @@ TileMapEditorTilesPlugin::TileMapEditorTilesPlugin() { // --- Bottom panel tiles --- tiles_bottom_panel = memnew(VBoxContainer); - tiles_bottom_panel->connect("tree_entered", callable_mp(this, &TileMapEditorTilesPlugin::_update_theme)); + // FIXME: This can trigger theme updates when the nodes that we want to update are not yet available. + // The toolbar should be extracted to a dedicated control and theme updates should be handled through + // the notification. tiles_bottom_panel->connect("theme_changed", callable_mp(this, &TileMapEditorTilesPlugin::_update_theme)); tiles_bottom_panel->connect("visibility_changed", callable_mp(this, &TileMapEditorTilesPlugin::_stop_dragging)); tiles_bottom_panel->connect("visibility_changed", callable_mp(this, &TileMapEditorTilesPlugin::_tab_changed)); @@ -3536,7 +3538,9 @@ void TileMapEditorTerrainsPlugin::edit(ObjectID p_tile_map_id, int p_tile_map_la TileMapEditorTerrainsPlugin::TileMapEditorTerrainsPlugin() { main_vbox_container = memnew(VBoxContainer); - main_vbox_container->connect("tree_entered", callable_mp(this, &TileMapEditorTerrainsPlugin::_update_theme)); + // FIXME: This can trigger theme updates when the nodes that we want to update are not yet available. + // The toolbar should be extracted to a dedicated control and theme updates should be handled through + // the notification. main_vbox_container->connect("theme_changed", callable_mp(this, &TileMapEditorTerrainsPlugin::_update_theme)); main_vbox_container->set_name(TTR("Terrains")); diff --git a/editor/shader_create_dialog.cpp b/editor/shader_create_dialog.cpp index 28776cbddc7..ca1cd642fbb 100644 --- a/editor/shader_create_dialog.cpp +++ b/editor/shader_create_dialog.cpp @@ -48,8 +48,6 @@ enum ShaderType { void ShaderCreateDialog::_notification(int p_what) { switch (p_what) { case NOTIFICATION_ENTER_TREE: { - _update_theme(); - String last_lang = EditorSettings::get_singleton()->get_project_metadata("shader_setup", "last_selected_language", ""); if (!last_lang.is_empty()) { for (int i = 0; i < type_menu->get_item_count(); i++) { @@ -68,30 +66,19 @@ void ShaderCreateDialog::_notification(int p_what) { } break; case NOTIFICATION_THEME_CHANGED: { - _update_theme(); + static const char *shader_types[3] = { "Shader", "VisualShader", "TextFile" }; + for (int i = 0; i < 3; i++) { + Ref icon = get_editor_theme_icon(shader_types[i]); + if (icon.is_valid()) { + type_menu->set_item_icon(i, icon); + } + } + + path_button->set_icon(get_editor_theme_icon(SNAME("Folder"))); } break; } } -void ShaderCreateDialog::_update_theme() { - Ref shader_icon = gc->get_editor_theme_icon(SNAME("Shader")); - if (shader_icon.is_valid()) { - type_menu->set_item_icon(0, shader_icon); - } - - Ref visual_shader_icon = gc->get_editor_theme_icon(SNAME("VisualShader")); - if (visual_shader_icon.is_valid()) { - type_menu->set_item_icon(1, visual_shader_icon); - } - - Ref include_icon = gc->get_editor_theme_icon(SNAME("TextFile")); - if (include_icon.is_valid()) { - type_menu->set_item_icon(2, include_icon); - } - - path_button->set_icon(get_editor_theme_icon(SNAME("Folder"))); -} - void ShaderCreateDialog::_update_language_info() { type_data.clear(); diff --git a/editor/shader_create_dialog.h b/editor/shader_create_dialog.h index d6d9f100201..5240842110c 100644 --- a/editor/shader_create_dialog.h +++ b/editor/shader_create_dialog.h @@ -101,7 +101,6 @@ class ShaderCreateDialog : public ConfirmationDialog { void _update_dialog(); protected: - void _update_theme(); void _notification(int p_what); static void _bind_methods();