Add colored margin in Inspector for arrays and dictionaries.

Apply suggestions from code review

Co-Authored-By: A Thousand Ships <[email protected]>
Co-Authored-By: Tomek <[email protected]>

Author: 3 people

doc/classes/EditorSettings.xml

@@ -750,6 +750,10 @@
 			- [b]Localized:[/b] Displays the localized string for the current editor language if a translation is available for the given property. If no translation is available, falls back to [b]Capitalized[/b].
 			[b]Note:[/b] To display translated setting names in Project Settings and Editor Settings, use [member interface/editor/localize_settings] instead.
 		</member>
+		<member name="interface/inspector/delimitate_all_container_and_resources" type="bool" setter="" getter="">
+			If [code]true[/code], add a margin around Array, Dictionary, and Resource Editors that are not already colored.
+			[b]Note:[/b] If [member interface/inspector/nested_color_mode] is set to [b]Containers &amp; Resources[/b] this parameter will have no effect since those editors will already be colored
+		</member>
 		<member name="interface/inspector/disable_folding" type="bool" setter="" getter="">
 			If [code]true[/code], forces all property groups to be expanded in the Inspector dock and prevents collapsing them.
 		</member>
@@ -765,6 +769,12 @@
 		<member name="interface/inspector/max_array_dictionary_items_per_page" type="int" setter="" getter="">
 			The number of [Array] or [Dictionary] items to display on each "page" in the inspector. Higher values allow viewing more values per page, but take more time to load. This increased load time is noticeable when selecting nodes that have array or dictionary properties in the editor.
 		</member>
+		<member name="interface/inspector/nested_color_mode" type="int" setter="" getter="">
+			Control which property editors are colored when they are opened.
+			- [b]Containers &amp; Resources:[/b] Color all Array, Dictionary, and Resource Editors.
+			- [b]Resources:[/b] Color all Resource Editors.
+			- [b]External Resources:[/b] Color Resource Editors that edits an external resource.
+		</member>
 		<member name="interface/inspector/open_resources_in_current_inspector" type="bool" setter="" getter="">
 			If [code]true[/code], subresources can be edited in the current inspector view. If the resource type is defined in [member interface/inspector/resources_to_open_in_new_inspector] or if this setting is [code]false[/code], attempting to edit a subresource always opens a new inspector view.
 		</member>

editor/editor_inspector.cpp

@@ -489,6 +489,49 @@ StringName EditorProperty::_get_revert_property() const {
 	return property;
 }
 
+void EditorProperty::_update_property_bg() {
+	// This function is to be called on EditorPropertyResource, EditorPropertyArray, and EditorPropertyDictionary.
+	// Behavior is undetermined on any other EditorProperty.
+	if (!is_inside_tree()) {
+		return;
+	}
+
+	begin_bulk_theme_override();
+
+	if (bottom_editor) {
+		ColorationMode nested_color_mode = (ColorationMode)(int)EDITOR_GET("interface/inspector/nested_color_mode");
+		bool delimitate_all_container_and_resources = EDITOR_GET("interface/inspector/delimitate_all_container_and_resources");
+		int count_subinspectors = 0;
+		if (is_colored(nested_color_mode)) {
+			Node *n = this;
+			while (n) {
+				EditorProperty *ep = Object::cast_to<EditorProperty>(n);
+				if (ep && ep->is_colored(nested_color_mode)) {
+					count_subinspectors++;
+				}
+				n = n->get_parent();
+			}
+			count_subinspectors = MIN(16, count_subinspectors);
+		}
+		add_theme_style_override(SNAME("DictionaryAddItem"), get_theme_stylebox("DictionaryAddItem" + itos(count_subinspectors), EditorStringName(EditorStyles)));
+		add_theme_constant_override("v_separation", 0);
+		if (delimitate_all_container_and_resources || is_colored(nested_color_mode)) {
+			add_theme_style_override("bg_selected", get_theme_stylebox("sub_inspector_property_bg" + itos(count_subinspectors), EditorStringName(EditorStyles)));
+			add_theme_style_override("bg", get_theme_stylebox("sub_inspector_property_bg" + itos(count_subinspectors), EditorStringName(EditorStyles)));
+			add_theme_color_override("property_color", get_theme_color(SNAME("sub_inspector_property_color"), EditorStringName(EditorStyles)));
+			bottom_editor->add_theme_style_override("panel", get_theme_stylebox("sub_inspector_bg" + itos(count_subinspectors), EditorStringName(EditorStyles)));
+		} else {
+			bottom_editor->add_theme_style_override("panel", get_theme_stylebox("sub_inspector_bg_no_border", EditorStringName(EditorStyles)));
+		}
+	} else {
+		remove_theme_style_override("bg_selected");
+		remove_theme_style_override("bg");
+		remove_theme_color_override("property_color");
+	}
+	end_bulk_theme_override();
+	queue_redraw();
+}
+
 void EditorProperty::update_editor_property_status() {
 	if (property == StringName()) {
 		return; //no property, so nothing to do
@@ -3481,8 +3524,8 @@ void EditorInspector::edit(Object *p_object) {
 
 	next_object = p_object; // Some plugins need to know the next edited object when clearing the inspector.
 	if (object) {
-		_clear();
 		object->disconnect("property_list_changed", callable_mp(this, &EditorInspector::_changed_callback));
+		_clear();
 	}
 	per_array_page.clear();
 
@@ -3676,30 +3719,11 @@ void EditorInspector::set_use_wide_editors(bool p_enable) {
 	wide_editors = p_enable;
 }
 
-void EditorInspector::_update_inspector_bg() {
-	if (sub_inspector) {
-		int count_subinspectors = 0;
-		Node *n = get_parent();
-		while (n) {
-			EditorInspector *ei = Object::cast_to<EditorInspector>(n);
-			if (ei && ei->sub_inspector) {
-				count_subinspectors++;
-			}
-			n = n->get_parent();
-		}
-		count_subinspectors = MIN(15, count_subinspectors);
-		add_theme_style_override("panel", get_theme_stylebox("sub_inspector_bg" + itos(count_subinspectors), EditorStringName(Editor)));
-	} else {
-		add_theme_style_override("panel", get_theme_stylebox(SNAME("panel"), SNAME("Tree")));
-	}
-}
 void EditorInspector::set_sub_inspector(bool p_enable) {
 	sub_inspector = p_enable;
 	if (!is_inside_tree()) {
 		return;
 	}
-
-	_update_inspector_bg();
 }
 
 void EditorInspector::set_use_deletable_properties(bool p_enabled) {
@@ -4014,7 +4038,7 @@ void EditorInspector::_notification(int p_what) {
 		case NOTIFICATION_READY: {
 			EditorFeatureProfileManager::get_singleton()->connect("current_feature_profile_changed", callable_mp(this, &EditorInspector::_feature_profile_changed));
 			set_process(is_visible_in_tree());
-			_update_inspector_bg();
+			add_theme_style_override("panel", get_theme_stylebox(SNAME("panel"), SNAME("Tree")));
 		} break;
 
 		case NOTIFICATION_ENTER_TREE: {
@@ -4088,10 +4112,6 @@ void EditorInspector::_notification(int p_what) {
 		} break;
 
 		case EditorSettings::NOTIFICATION_EDITOR_SETTINGS_CHANGED: {
-			if (EditorThemeManager::is_generated_theme_outdated()) {
-				_update_inspector_bg();
-			}
-
 			bool needs_update = false;
 
 			if (use_settings_name_style && EditorSettings::get_singleton()->check_changed_settings_in_group("interface/editor/localize_settings")) {
@@ -4101,6 +4121,7 @@ void EditorInspector::_notification(int p_what) {
 					needs_update = true;
 				}
 			}
+
 			if (EditorSettings::get_singleton()->check_changed_settings_in_group("interface/inspector")) {
 				needs_update = true;
 			}

editor/editor_inspector.h

@@ -67,6 +67,12 @@ class EditorProperty : public Container {
 		MENU_OPEN_DOCUMENTATION,
 	};
 
+	enum ColorationMode {
+		COLORATION_CONTAINER_RESOURCE,
+		COLORATION_RESOURCE,
+		COLORATION_EXTERNAL,
+	};
+
 private:
 	String label;
 	int text_size;
@@ -141,6 +147,8 @@ class EditorProperty : public Container {
 	virtual Variant _get_cache_value(const StringName &p_prop, bool &r_valid) const;
 	virtual StringName _get_revert_property() const;
 
+	void _update_property_bg();
+
 public:
 	void emit_changed(const StringName &p_property, const Variant &p_value, const StringName &p_field = StringName(), bool p_changing = false);
 
@@ -177,6 +185,8 @@ class EditorProperty : public Container {
 	void set_keying(bool p_keying);
 	bool is_keying() const;
 
+	virtual bool is_colored(ColorationMode p_mode) { return false; }
+
 	void set_deletable(bool p_enable);
 	bool is_deletable() const;
 	void add_focusable(Control *p_control);
@@ -557,8 +567,6 @@ class EditorInspector : public ScrollContainer {
 
 	bool _is_property_disabled_by_feature_profile(const StringName &p_property);
 
-	void _update_inspector_bg();
-
 	ConfirmationDialog *add_meta_dialog = nullptr;
 	LineEdit *add_meta_name = nullptr;
 	OptionButton *add_meta_type = nullptr;

editor/editor_properties.cpp

@@ -48,6 +48,7 @@
 #include "editor/property_selector.h"
 #include "editor/scene_tree_dock.h"
 #include "editor/themes/editor_scale.h"
+#include "editor/themes/editor_theme_manager.h"
 #include "scene/2d/gpu_particles_2d.h"
 #include "scene/3d/fog_volume.h"
 #include "scene/3d/gpu_particles_3d.h"
@@ -3220,42 +3221,6 @@ void EditorPropertyResource::_open_editor_pressed() {
 	}
 }
 
-void EditorPropertyResource::_update_property_bg() {
-	if (!is_inside_tree()) {
-		return;
-	}
-
-	updating_theme = true;
-
-	begin_bulk_theme_override();
-	if (sub_inspector != nullptr) {
-		int count_subinspectors = 0;
-		Node *n = get_parent();
-		while (n) {
-			EditorInspector *ei = Object::cast_to<EditorInspector>(n);
-			if (ei && ei->is_sub_inspector()) {
-				count_subinspectors++;
-			}
-			n = n->get_parent();
-		}
-		count_subinspectors = MIN(15, count_subinspectors);
-
-		add_theme_color_override("property_color", get_theme_color(SNAME("sub_inspector_property_color"), EditorStringName(Editor)));
-		add_theme_style_override("bg_selected", get_theme_stylebox("sub_inspector_property_bg" + itos(count_subinspectors), EditorStringName(Editor)));
-		add_theme_style_override("bg", get_theme_stylebox("sub_inspector_property_bg" + itos(count_subinspectors), EditorStringName(Editor)));
-		add_theme_constant_override("v_separation", 0);
-	} else {
-		add_theme_color_override("property_color", get_theme_color(SNAME("property_color"), SNAME("EditorProperty")));
-		add_theme_style_override("bg_selected", get_theme_stylebox(SNAME("bg_selected"), SNAME("EditorProperty")));
-		add_theme_style_override("bg", get_theme_stylebox(SNAME("bg"), SNAME("EditorProperty")));
-		add_theme_constant_override("v_separation", get_theme_constant(SNAME("v_separation"), SNAME("EditorProperty")));
-	}
-	end_bulk_theme_override();
-
-	updating_theme = false;
-	queue_redraw();
-}
-
 void EditorPropertyResource::_update_preferred_shader() {
 	Node *parent = get_parent();
 	EditorProperty *parent_property = nullptr;
@@ -3362,12 +3327,10 @@ void EditorPropertyResource::update_property() {
 				sub_inspector->set_read_only(is_read_only());
 				sub_inspector->set_use_folding(is_using_folding());
 
-				sub_inspector_vbox = memnew(VBoxContainer);
-				sub_inspector_vbox->set_mouse_filter(MOUSE_FILTER_STOP);
-				add_child(sub_inspector_vbox);
-				set_bottom_editor(sub_inspector_vbox);
+				sub_inspector->set_mouse_filter(MOUSE_FILTER_STOP);
+				add_child(sub_inspector);
+				set_bottom_editor(sub_inspector);
 
-				sub_inspector_vbox->add_child(sub_inspector);
 				resource_picker->set_toggle_pressed(true);
 
 				Array editor_list;
@@ -3383,20 +3346,18 @@ void EditorPropertyResource::update_property() {
 					_open_editor_pressed();
 					opened_editor = true;
 				}
-
-				_update_property_bg();
 			}
 
 			if (res.ptr() != sub_inspector->get_edited_object()) {
 				sub_inspector->edit(res.ptr());
+				_update_property_bg();
 			}
 
 		} else {
 			if (sub_inspector) {
 				set_bottom_editor(nullptr);
-				memdelete(sub_inspector_vbox);
+				memdelete(sub_inspector);
 				sub_inspector = nullptr;
-				sub_inspector_vbox = nullptr;
 
 				if (opened_editor) {
 					EditorNode::get_singleton()->hide_unused_editors();
@@ -3442,10 +3403,26 @@ void EditorPropertyResource::fold_resource() {
 	}
 }
 
+bool EditorPropertyResource::is_colored(ColorationMode p_mode) {
+	switch (p_mode) {
+		case COLORATION_CONTAINER_RESOURCE:
+			return sub_inspector != nullptr;
+		case COLORATION_RESOURCE:
+			return true;
+		case COLORATION_EXTERNAL:
+			if (sub_inspector) {
+				Resource *edited_resource = Object::cast_to<Resource>(sub_inspector->get_edited_object());
+				return edited_resource && !edited_resource->is_built_in();
+			}
+			break;
+	}
+	return false;
+}
+
 void EditorPropertyResource::_notification(int p_what) {
 	switch (p_what) {
 		case NOTIFICATION_THEME_CHANGED: {
-			if (!updating_theme) {
+			if (EditorThemeManager::is_generated_theme_outdated()) {
 				_update_property_bg();
 			}
 		} break;

editor/editor_properties.h

@@ -699,8 +699,6 @@ class EditorPropertyResource : public EditorProperty {
 
 	bool use_sub_inspector = false;
 	EditorInspector *sub_inspector = nullptr;
-	VBoxContainer *sub_inspector_vbox = nullptr;
-	bool updating_theme = false;
 	bool opened_editor = false;
 
 	void _resource_selected(const Ref<Resource> &p_resource, bool p_inspect);
@@ -713,7 +711,6 @@ class EditorPropertyResource : public EditorProperty {
 	void _sub_inspector_object_id_selected(int p_id);
 
 	void _open_editor_pressed();
-	void _update_property_bg();
 	void _update_preferred_shader();
 
 protected:
@@ -731,6 +728,8 @@ class EditorPropertyResource : public EditorProperty {
 	void set_use_sub_inspector(bool p_enable);
 	void fold_resource();
 
+	virtual bool is_colored(ColorationMode p_mode) override;
+
 	EditorPropertyResource();
 };