Merge pull request #49227 from pycbouh/editor-theme-freeze-changes

Prevent `Theme` resource from emitting changes during bulk operations

editor/plugins/theme_editor_plugin.cpp

@@ -756,7 +756,9 @@ void ThemeItemImportTree::_import_selected() {
 		return;
 	}
 
-	ProgressDialog::get_singleton()->add_task("import_theme_items", TTR("Importing Theme Items"), selected_items.size());
+	// Prevent changes from immediatelly being reported while the operation is still ongoing.
+	edited_theme->_freeze_change_propagation();
+	ProgressDialog::get_singleton()->add_task("import_theme_items", TTR("Importing Theme Items"), selected_items.size() + 2);
 
 	int idx = 0;
 	for (Map<ThemeItem, ItemCheckedState>::Element *E = selected_items.front(); E; E = E->next()) {
@@ -814,6 +816,12 @@ void ThemeItemImportTree::_import_selected() {
 		idx++;
 	}
 
+	// Allow changes to be reported now that the operation is finished.
+	ProgressDialog::get_singleton()->task_step("import_theme_items", TTR("Updating the editor"), idx++);
+	edited_theme->_unfreeze_and_propagate_changes();
+	// Make sure the task is not ended before the editor freezes to update the Inspector.
+	ProgressDialog::get_singleton()->task_step("import_theme_items", TTR("Finalizing"), idx++);
+
 	ProgressDialog::get_singleton()->end_task("import_theme_items");
 	emit_signal("items_imported");
 }
@@ -1488,15 +1496,24 @@ void ThemeItemEditorDialog::_add_theme_item(Theme::DataType p_data_type, String
 void ThemeItemEditorDialog::_remove_data_type_items(Theme::DataType p_data_type, String p_item_type) {
 	List<StringName> names;
 
+	// Prevent changes from immediatelly being reported while the operation is still ongoing.
+	edited_theme->_freeze_change_propagation();
+
 	edited_theme->get_theme_item_list(p_data_type, p_item_type, &names);
 	for (List<StringName>::Element *E = names.front(); E; E = E->next()) {
 		edited_theme->clear_theme_item(p_data_type, E->get(), p_item_type);
 	}
+
+	// Allow changes to be reported now that the operation is finished.
+	edited_theme->_unfreeze_and_propagate_changes();
 }
 
 void ThemeItemEditorDialog::_remove_class_items() {
 	List<StringName> names;
 
+	// Prevent changes from immediatelly being reported while the operation is still ongoing.
+	edited_theme->_freeze_change_propagation();
+
 	for (int dt = 0; dt < Theme::DATA_TYPE_MAX; dt++) {
 		Theme::DataType data_type = (Theme::DataType)dt;
 
@@ -1509,12 +1526,18 @@ void ThemeItemEditorDialog::_remove_class_items() {
 		}
 	}
 
+	// Allow changes to be reported now that the operation is finished.
+	edited_theme->_unfreeze_and_propagate_changes();
+
 	_update_edit_item_tree(edited_item_type);
 }
 
 void ThemeItemEditorDialog::_remove_custom_items() {
 	List<StringName> names;
 
+	// Prevent changes from immediatelly being reported while the operation is still ongoing.
+	edited_theme->_freeze_change_propagation();
+
 	for (int dt = 0; dt < Theme::DATA_TYPE_MAX; dt++) {
 		Theme::DataType data_type = (Theme::DataType)dt;
 
@@ -1527,12 +1550,18 @@ void ThemeItemEditorDialog::_remove_custom_items() {
 		}
 	}
 
+	// Allow changes to be reported now that the operation is finished.
+	edited_theme->_unfreeze_and_propagate_changes();
+
 	_update_edit_item_tree(edited_item_type);
 }
 
 void ThemeItemEditorDialog::_remove_all_items() {
 	List<StringName> names;
 
+	// Prevent changes from immediatelly being reported while the operation is still ongoing.
+	edited_theme->_freeze_change_propagation();
+
 	for (int dt = 0; dt < Theme::DATA_TYPE_MAX; dt++) {
 		Theme::DataType data_type = (Theme::DataType)dt;
 
@@ -1543,6 +1572,9 @@ void ThemeItemEditorDialog::_remove_all_items() {
 		}
 	}
 
+	// Allow changes to be reported now that the operation is finished.
+	edited_theme->_unfreeze_and_propagate_changes();
+
 	_update_edit_item_tree(edited_item_type);
 }
 

scene/resources/theme.cpp

@@ -33,6 +33,11 @@
 #include "core/string/print_string.h"
 
 void Theme::_emit_theme_changed() {
+	if (no_change_propagation) {
+		return;
+	}
+
+	notify_property_list_changed();
 	emit_changed();
 }
 
@@ -415,8 +420,7 @@ void Theme::set_default_theme_font(const Ref<Font> &p_default_font) {
 		default_theme_font->connect("changed", callable_mp(this, &Theme::_emit_theme_changed), varray(), CONNECT_REFERENCE_COUNTED);
 	}
 
-	notify_property_list_changed();
-	emit_changed();
+	_emit_theme_changed();
 }
 
 Ref<Font> Theme::get_default_theme_font() const {
@@ -430,8 +434,7 @@ void Theme::set_default_theme_font_size(int p_font_size) {
 
 	default_theme_font_size = p_font_size;
 
-	notify_property_list_changed();
-	emit_changed();
+	_emit_theme_changed();
 }
 
 int Theme::get_default_theme_font_size() const {
@@ -478,8 +481,6 @@ void Theme::set_default_font_size(int p_font_size) {
 }
 
 void Theme::set_icon(const StringName &p_name, const StringName &p_theme_type, const Ref<Texture2D> &p_icon) {
-	bool new_value = !icon_map.has(p_theme_type) || !icon_map[p_theme_type].has(p_name);
-
 	if (icon_map[p_theme_type].has(p_name) && icon_map[p_theme_type][p_name].is_valid()) {
 		icon_map[p_theme_type][p_name]->disconnect("changed", callable_mp(this, &Theme::_emit_theme_changed));
 	}
@@ -490,10 +491,7 @@ void Theme::set_icon(const StringName &p_name, const StringName &p_theme_type, c
 		icon_map[p_theme_type][p_name]->connect("changed", callable_mp(this, &Theme::_emit_theme_changed), varray(), CONNECT_REFERENCE_COUNTED);
 	}
 
-	if (new_value) {
-		notify_property_list_changed();
-		emit_changed();
-	}
+	_emit_theme_changed();
 }
 
 Ref<Texture2D> Theme::get_icon(const StringName &p_name, const StringName &p_theme_type) const {
@@ -520,8 +518,7 @@ void Theme::rename_icon(const StringName &p_old_name, const StringName &p_name,
 	icon_map[p_theme_type][p_name] = icon_map[p_theme_type][p_old_name];
 	icon_map[p_theme_type].erase(p_old_name);
 
-	notify_property_list_changed();
-	emit_changed();
+	_emit_theme_changed();
 }
 
 void Theme::clear_icon(const StringName &p_name, const StringName &p_theme_type) {
@@ -534,8 +531,7 @@ void Theme::clear_icon(const StringName &p_name, const StringName &p_theme_type)
 
 	icon_map[p_theme_type].erase(p_name);
 
-	notify_property_list_changed();
-	emit_changed();
+	_emit_theme_changed();
 }
 
 void Theme::get_icon_list(StringName p_theme_type, List<StringName> *p_list) const {
@@ -569,8 +565,6 @@ void Theme::get_icon_type_list(List<StringName> *p_list) const {
 }
 
 void Theme::set_stylebox(const StringName &p_name, const StringName &p_theme_type, const Ref<StyleBox> &p_style) {
-	bool new_value = !style_map.has(p_theme_type) || !style_map[p_theme_type].has(p_name);
-
 	if (style_map[p_theme_type].has(p_name) && style_map[p_theme_type][p_name].is_valid()) {
 		style_map[p_theme_type][p_name]->disconnect("changed", callable_mp(this, &Theme::_emit_theme_changed));
 	}
@@ -581,10 +575,7 @@ void Theme::set_stylebox(const StringName &p_name, const StringName &p_theme_typ
 		style_map[p_theme_type][p_name]->connect("changed", callable_mp(this, &Theme::_emit_theme_changed), varray(), CONNECT_REFERENCE_COUNTED);
 	}
 
-	if (new_value) {
-		notify_property_list_changed();
-	}
-	emit_changed();
+	_emit_theme_changed();
 }
 
 Ref<StyleBox> Theme::get_stylebox(const StringName &p_name, const StringName &p_theme_type) const {
@@ -611,8 +602,7 @@ void Theme::rename_stylebox(const StringName &p_old_name, const StringName &p_na
 	style_map[p_theme_type][p_name] = style_map[p_theme_type][p_old_name];
 	style_map[p_theme_type].erase(p_old_name);
 
-	notify_property_list_changed();
-	emit_changed();
+	_emit_theme_changed();
 }
 
 void Theme::clear_stylebox(const StringName &p_name, const StringName &p_theme_type) {
@@ -625,8 +615,7 @@ void Theme::clear_stylebox(const StringName &p_name, const StringName &p_theme_t
 
 	style_map[p_theme_type].erase(p_name);
 
-	notify_property_list_changed();
-	emit_changed();
+	_emit_theme_changed();
 }
 
 void Theme::get_stylebox_list(StringName p_theme_type, List<StringName> *p_list) const {
@@ -660,8 +649,6 @@ void Theme::get_stylebox_type_list(List<StringName> *p_list) const {
 }
 
 void Theme::set_font(const StringName &p_name, const StringName &p_theme_type, const Ref<Font> &p_font) {
-	bool new_value = !font_map.has(p_theme_type) || !font_map[p_theme_type].has(p_name);
-
 	if (font_map[p_theme_type][p_name].is_valid()) {
 		font_map[p_theme_type][p_name]->disconnect("changed", callable_mp(this, &Theme::_emit_theme_changed));
 	}
@@ -672,10 +659,7 @@ void Theme::set_font(const StringName &p_name, const StringName &p_theme_type, c
 		font_map[p_theme_type][p_name]->connect("changed", callable_mp(this, &Theme::_emit_theme_changed), varray(), CONNECT_REFERENCE_COUNTED);
 	}
 
-	if (new_value) {
-		notify_property_list_changed();
-		emit_changed();
-	}
+	_emit_theme_changed();
 }
 
 Ref<Font> Theme::get_font(const StringName &p_name, const StringName &p_theme_type) const {
@@ -704,8 +688,7 @@ void Theme::rename_font(const StringName &p_old_name, const StringName &p_name,
 	font_map[p_theme_type][p_name] = font_map[p_theme_type][p_old_name];
 	font_map[p_theme_type].erase(p_old_name);
 
-	notify_property_list_changed();
-	emit_changed();
+	_emit_theme_changed();
 }
 
 void Theme::clear_font(const StringName &p_name, const StringName &p_theme_type) {
@@ -717,8 +700,8 @@ void Theme::clear_font(const StringName &p_name, const StringName &p_theme_type)
 	}
 
 	font_map[p_theme_type].erase(p_name);
-	notify_property_list_changed();
-	emit_changed();
+
+	_emit_theme_changed();
 }
 
 void Theme::get_font_list(StringName p_theme_type, List<StringName> *p_list) const {
@@ -752,14 +735,9 @@ void Theme::get_font_type_list(List<StringName> *p_list) const {
 }
 
 void Theme::set_font_size(const StringName &p_name, const StringName &p_theme_type, int p_font_size) {
-	bool new_value = !font_size_map.has(p_theme_type) || !font_size_map[p_theme_type].has(p_name);
-
 	font_size_map[p_theme_type][p_name] = p_font_size;
 
-	if (new_value) {
-		notify_property_list_changed();
-		emit_changed();
-	}
+	_emit_theme_changed();
 }
 
 int Theme::get_font_size(const StringName &p_name, const StringName &p_theme_type) const {
@@ -788,8 +766,7 @@ void Theme::rename_font_size(const StringName &p_old_name, const StringName &p_n
 	font_size_map[p_theme_type][p_name] = font_size_map[p_theme_type][p_old_name];
 	font_size_map[p_theme_type].erase(p_old_name);
 
-	notify_property_list_changed();
-	emit_changed();
+	_emit_theme_changed();
 }
 
 void Theme::clear_font_size(const StringName &p_name, const StringName &p_theme_type) {
@@ -797,8 +774,8 @@ void Theme::clear_font_size(const StringName &p_name, const StringName &p_theme_
 	ERR_FAIL_COND_MSG(!font_size_map[p_theme_type].has(p_name), "Cannot clear the font size '" + String(p_name) + "' because it does not exist.");
 
 	font_size_map[p_theme_type].erase(p_name);
-	notify_property_list_changed();
-	emit_changed();
+
+	_emit_theme_changed();
 }
 
 void Theme::get_font_size_list(StringName p_theme_type, List<StringName> *p_list) const {
@@ -832,14 +809,9 @@ void Theme::get_font_size_type_list(List<StringName> *p_list) const {
 }
 
 void Theme::set_color(const StringName &p_name, const StringName &p_theme_type, const Color &p_color) {
-	bool new_value = !color_map.has(p_theme_type) || !color_map[p_theme_type].has(p_name);
-
 	color_map[p_theme_type][p_name] = p_color;
 
-	if (new_value) {
-		notify_property_list_changed();
-		emit_changed();
-	}
+	_emit_theme_changed();
 }
 
 Color Theme::get_color(const StringName &p_name, const StringName &p_theme_type) const {
@@ -866,8 +838,7 @@ void Theme::rename_color(const StringName &p_old_name, const StringName &p_name,
 	color_map[p_theme_type][p_name] = color_map[p_theme_type][p_old_name];
 	color_map[p_theme_type].erase(p_old_name);
 
-	notify_property_list_changed();
-	emit_changed();
+	_emit_theme_changed();
 }
 
 void Theme::clear_color(const StringName &p_name, const StringName &p_theme_type) {
@@ -875,8 +846,8 @@ void Theme::clear_color(const StringName &p_name, const StringName &p_theme_type
 	ERR_FAIL_COND_MSG(!color_map[p_theme_type].has(p_name), "Cannot clear the color '" + String(p_name) + "' because it does not exist.");
 
 	color_map[p_theme_type].erase(p_name);
-	notify_property_list_changed();
-	emit_changed();
+
+	_emit_theme_changed();
 }
 
 void Theme::get_color_list(StringName p_theme_type, List<StringName> *p_list) const {
@@ -910,13 +881,9 @@ void Theme::get_color_type_list(List<StringName> *p_list) const {
 }
 
 void Theme::set_constant(const StringName &p_name, const StringName &p_theme_type, int p_constant) {
-	bool new_value = !constant_map.has(p_theme_type) || !constant_map[p_theme_type].has(p_name);
 	constant_map[p_theme_type][p_name] = p_constant;
 
-	if (new_value) {
-		notify_property_list_changed();
-		emit_changed();
-	}
+	_emit_theme_changed();
 }
 
 int Theme::get_constant(const StringName &p_name, const StringName &p_theme_type) const {
@@ -943,8 +910,7 @@ void Theme::rename_constant(const StringName &p_old_name, const StringName &p_na
 	constant_map[p_theme_type][p_name] = constant_map[p_theme_type][p_old_name];
 	constant_map[p_theme_type].erase(p_old_name);
 
-	notify_property_list_changed();
-	emit_changed();
+	_emit_theme_changed();
 }
 
 void Theme::clear_constant(const StringName &p_name, const StringName &p_theme_type) {
@@ -952,8 +918,8 @@ void Theme::clear_constant(const StringName &p_name, const StringName &p_theme_t
 	ERR_FAIL_COND_MSG(!constant_map[p_theme_type].has(p_name), "Cannot clear the constant '" + String(p_name) + "' because it does not exist.");
 
 	constant_map[p_theme_type].erase(p_name);
-	notify_property_list_changed();
-	emit_changed();
+
+	_emit_theme_changed();
 }
 
 void Theme::get_constant_list(StringName p_theme_type, List<StringName> *p_list) const {
@@ -1217,8 +1183,17 @@ void Theme::get_theme_item_type_list(DataType p_data_type, List<StringName> *p_l
 	}
 }
 
+void Theme::_freeze_change_propagation() {
+	no_change_propagation = true;
+}
+
+void Theme::_unfreeze_and_propagate_changes() {
+	no_change_propagation = false;
+	_emit_theme_changed();
+}
+
 void Theme::clear() {
-	//these need disconnecting
+	// These items need disconnecting.
 	{
 		const StringName *K = nullptr;
 		while ((K = icon_map.next(K))) {
@@ -1264,8 +1239,7 @@ void Theme::clear() {
 	color_map.clear();
 	constant_map.clear();
 
-	notify_property_list_changed();
-	emit_changed();
+	_emit_theme_changed();
 }
 
 void Theme::copy_default_theme() {
@@ -1279,6 +1253,8 @@ void Theme::copy_theme(const Ref<Theme> &p_other) {
 		return;
 	}
 
+	_freeze_change_propagation();
+
 	// These items need reconnecting, so add them normally.
 	{
 		const StringName *K = nullptr;
@@ -1315,8 +1291,7 @@ void Theme::copy_theme(const Ref<Theme> &p_other) {
 	color_map = p_other->color_map;
 	constant_map = p_other->constant_map;
 
-	notify_property_list_changed();
-	emit_changed();
+	_unfreeze_and_propagate_changes();
 }
 
 void Theme::get_type_list(List<StringName> *p_list) const {

scene/resources/theme.h

@@ -41,6 +41,11 @@ class Theme : public Resource {
 	GDCLASS(Theme, Resource);
 	RES_BASE_EXTENSION("theme");
 
+#ifdef TOOLS_ENABLED
+	friend class ThemeItemImportTree;
+	friend class ThemeItemEditorDialog;
+#endif
+
 public:
 	enum DataType {
 		DATA_TYPE_COLOR,
@@ -53,6 +58,8 @@ public:
 	};
 
 private:
+	bool no_change_propagation = false;
+
 	void _emit_theme_changed();
 
 	HashMap<StringName, HashMap<StringName, Ref<Texture2D>>> icon_map;
@@ -96,6 +103,9 @@ protected:
 
 	static void _bind_methods();
 
+	void _freeze_change_propagation();
+	void _unfreeze_and_propagate_changes();
+
 	virtual void reset_state() override;
 
 public: