Merge pull request #71770 from KoBeWi/better_editing_or_something

Rework EditorPlugin editing logic

doc/classes/EditorPlugin.xml

@@ -41,6 +41,7 @@
 			<param index="0" name="object" type="Variant" />
 			<description>
 				This function is used for plugins that edit specific object types (nodes or resources). It requests the editor to edit the given object.
+				[param object] can be [code]null[/code] if the plugin was editing an object, but there is no longer any selected object handled by this plugin. It can be used to cleanup editing state.
 			</description>
 		</method>
 		<method name="_enable_plugin" qualifiers="virtual">

editor/editor_inspector.cpp

@@ -3282,6 +3282,11 @@ void EditorInspector::update_tree() {
 		ped->parse_end(object);
 		_parse_added_editors(main_vbox, nullptr, ped);
 	}
+
+	if (_is_main_editor_inspector()) {
+		// Updating inspector might invalidate some editing owners.
+		EditorNode::get_singleton()->hide_unused_editors();
+	}
 }
 
 void EditorInspector::update_property(const String &p_prop) {
@@ -3306,6 +3311,9 @@ void EditorInspector::_clear() {
 	sections.clear();
 	pending.clear();
 	restart_request_props.clear();
+	if (_is_main_editor_inspector()) {
+		EditorNode::get_singleton()->hide_unused_editors(this);
+	}
 }
 
 Object *EditorInspector::get_edited_object() {
@@ -3640,6 +3648,10 @@ void EditorInspector::_edit_set(const String &p_name, const Variant &p_value, bo
 	}
 }
 
+bool EditorInspector::_is_main_editor_inspector() const {
+	return InspectorDock::get_singleton() && InspectorDock::get_inspector_singleton() == this;
+}
+
 void EditorInspector::_property_changed(const String &p_path, const Variant &p_value, const String &p_name, bool p_changing, bool p_update_all) {
 	// The "changing" variable must be true for properties that trigger events as typing occurs,
 	// like "text_changed" signal. E.g. text property of Label, Button, RichTextLabel, etc.

editor/editor_inspector.h

@@ -502,6 +502,7 @@ class EditorInspector : public ScrollContainer {
 	bool restrict_to_basic = false;
 
 	void _edit_set(const String &p_name, const Variant &p_value, bool p_refresh_all, const String &p_changed_field);
+	bool _is_main_editor_inspector() const;
 
 	void _property_changed(const String &p_path, const Variant &p_value, const String &p_name = "", bool p_changing = false, bool p_update_all = false);
 	void _multiple_properties_changed(Vector<String> p_paths, Array p_values, bool p_changing = false);

editor/editor_node.cpp

@@ -2078,51 +2078,55 @@ bool EditorNode::_is_class_editor_disabled_by_feature_profile(const StringName &
 	return false;
 }
 
-void EditorNode::edit_item(Object *p_object) {
+void EditorNode::edit_item(Object *p_object, Object *p_editing_owner) {
+	ERR_FAIL_NULL(p_editing_owner);
+
 	if (p_object && _is_class_editor_disabled_by_feature_profile(p_object->get_class())) {
 		return;
 	}
 
-	Vector<EditorPlugin *> top_plugins = editor_plugins_over->get_plugins_list();
 	Vector<EditorPlugin *> item_plugins;
 	if (p_object) {
 		item_plugins = editor_data.get_subeditors(p_object);
 	}
 
 	if (!item_plugins.is_empty()) {
-		bool same = true;
-		if (item_plugins.size() == top_plugins.size()) {
-			for (int i = 0; i < item_plugins.size(); i++) {
-				if (item_plugins[i] != top_plugins[i]) {
-					same = false;
-				}
+		ObjectID owner_id = p_editing_owner->get_instance_id();
+
+		for (EditorPlugin *plugin : active_plugins[owner_id]) {
+			if (!item_plugins.has(plugin)) {
+				plugin->make_visible(false);
+				plugin->edit(nullptr);
 			}
-		} else {
-			same = false;
 		}
 
-		if (!same) {
-			_display_top_editors(false);
-			_set_top_editors(item_plugins);
+		for (EditorPlugin *plugin : item_plugins) {
+			for (KeyValue<ObjectID, HashSet<EditorPlugin *>> &kv : active_plugins) {
+				if (kv.key != owner_id) {
+					EditorPropertyResource *epres = Object::cast_to<EditorPropertyResource>(ObjectDB::get_instance(kv.key));
+					if (epres && kv.value.has(plugin)) {
+						// If it's resource property editing the same resource type, fold it.
+						epres->fold_resource();
+					}
+					kv.value.erase(plugin);
+				}
+			}
+			active_plugins[owner_id].insert(plugin);
+			plugin->edit(p_object);
+			plugin->make_visible(true);
 		}
-		_set_editing_top_editors(p_object);
-		_display_top_editors(true);
-	} else if (!top_plugins.is_empty()) {
-		hide_top_editors();
+	} else {
+		hide_unused_editors(p_editing_owner);
 	}
 }
 
-void EditorNode::edit_item_resource(Ref<Resource> p_resource) {
-	edit_item(p_resource.ptr());
-}
-
 void EditorNode::push_item(Object *p_object, const String &p_property, bool p_inspector_only) {
 	if (!p_object) {
 		InspectorDock::get_inspector_singleton()->edit(nullptr);
 		NodeDock::get_singleton()->set_node(nullptr);
 		SceneTreeDock::get_singleton()->set_selected(nullptr);
 		InspectorDock::get_singleton()->update(nullptr);
-		_display_top_editors(false);
+		hide_unused_editors();
 		return;
 	}
 
@@ -2150,22 +2154,34 @@ void EditorNode::_save_default_environment() {
 	}
 }
 
-void EditorNode::hide_top_editors() {
-	_display_top_editors(false);
-
-	editor_plugins_over->clear();
-}
-
-void EditorNode::_display_top_editors(bool p_display) {
-	editor_plugins_over->make_visible(p_display);
-}
-
-void EditorNode::_set_top_editors(Vector<EditorPlugin *> p_editor_plugins_over) {
-	editor_plugins_over->set_plugins_list(p_editor_plugins_over);
-}
+void EditorNode::hide_unused_editors(const Object *p_editing_owner) {
+	if (p_editing_owner) {
+		const ObjectID id = p_editing_owner->get_instance_id();
+		for (EditorPlugin *plugin : active_plugins[id]) {
+			plugin->make_visible(false);
+			plugin->edit(nullptr);
+			editor_plugins_over->remove_plugin(plugin);
+		}
+		active_plugins.erase(id);
+	} else {
+		// If no editing owner is provided, this method will go over all owners and check if they are valid.
+		// This is to sweep properties that were removed from the inspector.
+		List<ObjectID> to_remove;
+		for (KeyValue<ObjectID, HashSet<EditorPlugin *>> &kv : active_plugins) {
+			if (!ObjectDB::get_instance(kv.key)) {
+				to_remove.push_back(kv.key);
+				for (EditorPlugin *plugin : kv.value) {
+					plugin->make_visible(false);
+					plugin->edit(nullptr);
+					editor_plugins_over->remove_plugin(plugin);
+				}
+			}
+		}
 
-void EditorNode::_set_editing_top_editors(Object *p_current_object) {
-	editor_plugins_over->edit(p_current_object);
+		for (const ObjectID &id : to_remove) {
+			active_plugins.erase(id);
+		}
+	}
 }
 
 static bool overrides_external_editor(Object *p_object) {
@@ -2199,7 +2215,7 @@ void EditorNode::_edit_current(bool p_skip_foreign) {
 		NodeDock::get_singleton()->set_node(nullptr);
 		InspectorDock::get_singleton()->update(nullptr);
 
-		_display_top_editors(false);
+		hide_unused_editors();
 
 		return;
 	}
@@ -2327,6 +2343,9 @@ void EditorNode::_edit_current(bool p_skip_foreign) {
 		InspectorDock::get_inspector_singleton()->set_use_folding(!disable_folding);
 	}
 
+	Object *editor_owner = is_node ? (Object *)SceneTreeDock::get_singleton() : is_resource ? (Object *)InspectorDock::get_inspector_singleton()
+																							: (Object *)this;
+
 	// Take care of the main editor plugin.
 
 	if (!inspector_only) {
@@ -2343,6 +2362,7 @@ void EditorNode::_edit_current(bool p_skip_foreign) {
 			}
 		}
 
+		ObjectID editor_owner_id = editor_owner->get_instance_id();
 		if (main_plugin && !skip_main_plugin) {
 			// Special case if use of external editor is true.
 			Resource *current_res = Object::cast_to<Resource>(current_obj);
@@ -2353,15 +2373,22 @@ void EditorNode::_edit_current(bool p_skip_foreign) {
 			}
 
 			else if (main_plugin != editor_plugin_screen && (!ScriptEditor::get_singleton() || !ScriptEditor::get_singleton()->is_visible_in_tree() || ScriptEditor::get_singleton()->can_take_away_focus())) {
+				// Unedit previous plugin.
+				editor_plugin_screen->edit(nullptr);
+				active_plugins[editor_owner_id].erase(editor_plugin_screen);
 				// Update screen main_plugin.
 				editor_select(plugin_index);
 				main_plugin->edit(current_obj);
 			} else {
 				editor_plugin_screen->edit(current_obj);
 			}
+			is_main_screen_editing = true;
+		} else if (!main_plugin && editor_plugin_screen && is_main_screen_editing) {
+			editor_plugin_screen->edit(nullptr);
+			is_main_screen_editing = false;
 		}
 
-		edit_item(current_obj);
+		edit_item(current_obj, editor_owner);
 	}
 
 	InspectorDock::get_singleton()->update(current_obj);

editor/editor_node.h

@@ -68,6 +68,7 @@ class EditorLayoutsDialog;
 class EditorLog;
 class EditorPluginList;
 class EditorQuickOpen;
+class EditorPropertyResource;
 class EditorResourcePreview;
 class EditorResourceConversionPlugin;
 class EditorRun;
@@ -293,6 +294,8 @@ class EditorNode : public Node {
 	bool _initializing_plugins = false;
 	HashMap<String, EditorPlugin *> addon_name_to_plugin;
 	LocalVector<String> pending_addons;
+	HashMap<ObjectID, HashSet<EditorPlugin *>> active_plugins;
+	bool is_main_screen_editing = false;
 
 	PanelContainer *scene_root_parent = nullptr;
 	Control *theme_base = nullptr;
@@ -592,10 +595,6 @@ class EditorNode : public Node {
 	void _inherit_request(String p_file);
 	void _instantiate_request(const Vector<String> &p_files);
 
-	void _display_top_editors(bool p_display);
-	void _set_top_editors(Vector<EditorPlugin *> p_editor_plugins_over);
-	void _set_editing_top_editors(Object *p_current_object);
-
 	void _quick_opened();
 	void _quick_run();
 	void _open_command_palette();
@@ -796,9 +795,8 @@ class EditorNode : public Node {
 	void show_about() { _menu_option_confirm(HELP_ABOUT, false); }
 
 	void push_item(Object *p_object, const String &p_property = "", bool p_inspector_only = false);
-	void edit_item(Object *p_object);
-	void edit_item_resource(Ref<Resource> p_resource);
-	void hide_top_editors();
+	void edit_item(Object *p_object, Object *p_editing_owner);
+	void hide_unused_editors(const Object *p_editing_owner = nullptr);
 
 	void select_editor_by_name(const String &p_name);
 

editor/editor_properties.cpp

@@ -3905,40 +3905,7 @@ void EditorPropertyResource::_open_editor_pressed() {
 	Ref<Resource> res = get_edited_object()->get(get_edited_property());
 	if (res.is_valid()) {
 		// May clear the editor so do it deferred.
-		callable_mp(EditorNode::get_singleton(), &EditorNode::edit_item_resource).bind(res).call_deferred();
-	}
-}
-
-void EditorPropertyResource::_fold_other_editors(Object *p_self) {
-	if (this == p_self) {
-		return;
-	}
-
-	Ref<Resource> res = get_edited_object()->get(get_edited_property());
-	if (!res.is_valid()) {
-		return;
-	}
-
-	bool use_editor = false;
-	for (int i = 0; i < EditorNode::get_editor_data().get_editor_plugin_count(); i++) {
-		EditorPlugin *ep = EditorNode::get_editor_data().get_editor_plugin(i);
-		if (ep->handles(res.ptr())) {
-			use_editor = true;
-			break;
-		}
-	}
-	if (!use_editor) {
-		return;
-	}
-
-	opened_editor = false;
-
-	bool unfolded = get_edited_object()->editor_is_section_unfolded(get_edited_property());
-	if (unfolded) {
-		// Refold.
-		resource_picker->set_toggle_pressed(false);
-		get_edited_object()->editor_set_section_unfold(get_edited_property(), false);
-		update_property();
+		callable_mp(EditorNode::get_singleton(), &EditorNode::edit_item).bind(res.ptr(), this).call_deferred();
 	}
 }
 
@@ -4091,20 +4058,17 @@ void EditorPropertyResource::update_property() {
 				sub_inspector_vbox->add_child(sub_inspector);
 				resource_picker->set_toggle_pressed(true);
 
-				bool use_editor = false;
+				Array editor_list;
 				for (int i = 0; i < EditorNode::get_editor_data().get_editor_plugin_count(); i++) {
 					EditorPlugin *ep = EditorNode::get_editor_data().get_editor_plugin(i);
 					if (ep->handles(res.ptr())) {
-						use_editor = true;
+						editor_list.push_back(ep);
 					}
 				}
 
-				if (use_editor) {
-					// Open editor directly and hide other such editors which are currently open.
+				if (!editor_list.is_empty()) {
+					// Open editor directly.
 					_open_editor_pressed();
-					if (is_inside_tree()) {
-						get_tree()->call_deferred(SNAME("call_group"), "_editor_resource_properties", "_fold_other_editors", this);
-					}
 					opened_editor = true;
 				}
 
@@ -4123,7 +4087,7 @@ void EditorPropertyResource::update_property() {
 				sub_inspector_vbox = nullptr;
 
 				if (opened_editor) {
-					EditorNode::get_singleton()->hide_top_editors();
+					EditorNode::get_singleton()->hide_unused_editors();
 					opened_editor = false;
 				}
 
@@ -4157,6 +4121,15 @@ void EditorPropertyResource::set_use_sub_inspector(bool p_enable) {
 	use_sub_inspector = p_enable;
 }
 
+void EditorPropertyResource::fold_resource() {
+	bool unfolded = get_edited_object()->editor_is_section_unfolded(get_edited_property());
+	if (unfolded) {
+		resource_picker->set_toggle_pressed(false);
+		get_edited_object()->editor_set_section_unfold(get_edited_property(), false);
+		update_property();
+	}
+}
+
 void EditorPropertyResource::_notification(int p_what) {
 	switch (p_what) {
 		case NOTIFICATION_ENTER_TREE:
@@ -4168,14 +4141,8 @@ void EditorPropertyResource::_notification(int p_what) {
 	}
 }
 
-void EditorPropertyResource::_bind_methods() {
-	ClassDB::bind_method(D_METHOD("_fold_other_editors"), &EditorPropertyResource::_fold_other_editors);
-}
-
 EditorPropertyResource::EditorPropertyResource() {
 	use_sub_inspector = bool(EDITOR_GET("interface/inspector/open_resources_in_current_inspector"));
-
-	add_to_group("_editor_resource_properties");
 }
 
 ////////////// DEFAULT PLUGIN //////////////////////

editor/editor_properties.h

@@ -834,13 +834,11 @@ class EditorPropertyResource : public EditorProperty {
 	void _sub_inspector_object_id_selected(int p_id);
 
 	void _open_editor_pressed();
-	void _fold_other_editors(Object *p_self);
 	void _update_property_bg();
 	void _update_preferred_shader();
 
 protected:
 	virtual void _set_read_only(bool p_read_only) override;
-	static void _bind_methods();
 	void _notification(int p_what);
 
 public:
@@ -852,6 +850,7 @@ class EditorPropertyResource : public EditorProperty {
 	void expand_revertable() override;
 
 	void set_use_sub_inspector(bool p_enable);
+	void fold_resource();
 
 	EditorPropertyResource();
 };