From 86bec25c510c77aa79407938a19ad19ac2da29b0 Mon Sep 17 00:00:00 2001
From: Hristo Stamenov <capitane71@gmail.com>
Date: Mon, 8 Nov 2021 06:24:12 +0200
Subject: [PATCH] Add the instance to the script property evaluation. This
 needs to be done so that tool scripts are evaluated properly and their
 properties are categorized in the inspector (due to a bug of them being
 missing). I am talking about the properties coming from _get_property_list.
 Also added a fallback so that we don't lose properties in the inspector even
 if they are not properly categorized.

---
 core/object/script_language.h                 |   2 +-
 editor/editor_inspector.cpp                   |  25 +++-
 editor/property_selector.cpp                  |   2 +-
 .../gdnative/nativescript/nativescript.cpp    |   6 +-
 modules/gdnative/nativescript/nativescript.h  |   2 +-
 .../pluginscript/pluginscript_instance.cpp    |   2 +-
 .../pluginscript/pluginscript_script.cpp      |   2 +-
 .../pluginscript/pluginscript_script.h        |   2 +-
 modules/gdscript/gdscript.cpp                 | 127 ++++++++++--------
 modules/gdscript/gdscript.h                   |   3 +-
 modules/gdscript/gdscript_editor.cpp          |   2 +-
 modules/mono/csharp_script.cpp                |   2 +-
 modules/mono/csharp_script.h                  |   2 +-
 modules/visual_script/visual_script.cpp       |   2 +-
 modules/visual_script/visual_script.h         |   2 +-
 .../visual_script_property_selector.cpp       |   2 +-
 16 files changed, 104 insertions(+), 81 deletions(-)

diff --git a/core/object/script_language.h b/core/object/script_language.h
index 8d76cbf479..d012595efe 100644
--- a/core/object/script_language.h
+++ b/core/object/script_language.h
@@ -150,7 +150,7 @@ public:
 
 	virtual void update_exports() {} //editor tool
 	virtual void get_script_method_list(List<MethodInfo> *p_list) const = 0;
-	virtual void get_script_property_list(List<PropertyInfo> *p_list) const = 0;
+	virtual void get_script_property_list(List<PropertyInfo> *p_list, const ScriptInstance *p_instance = nullptr) const = 0;
 
 	virtual int get_member_line(const StringName &p_member) const { return -1; }
 
diff --git a/editor/editor_inspector.cpp b/editor/editor_inspector.cpp
index 4ffa90777c..8f6aa39907 100644
--- a/editor/editor_inspector.cpp
+++ b/editor/editor_inspector.cpp
@@ -3459,7 +3459,7 @@ void EditorInspector::_update_script_class_properties(const Object &p_object, Li
 		}
 
 		List<PropertyInfo> props;
-		s->get_script_property_list(&props);
+		s->get_script_property_list(&props, p_object.get_script_instance());
 
 		// Script Variables -> NodeA -> bottom (insert_here)
 		List<PropertyInfo>::Element *category = r_list.insert_before(insert_here, PropertyInfo(Variant::NIL, name, PROPERTY_HINT_NONE, path, PROPERTY_USAGE_CATEGORY));
@@ -3481,11 +3481,24 @@ void EditorInspector::_update_script_class_properties(const Object &p_object, Li
 
 	// NodeC -> C props... -> NodeB..C..
 	if (script_variables) {
-		r_list.erase(script_variables);
-		List<PropertyInfo>::Element *to_delete = bottom->next();
-		while (to_delete && !(to_delete->get().usage & PROPERTY_USAGE_CATEGORY)) {
-			r_list.erase(to_delete);
-			to_delete = bottom->next();
+		bool properties_left = false;
+		List<PropertyInfo>::Element *current = bottom->next();
+		List<PropertyInfo>::Element *previous = bottom;
+		while (current && !(current->get().usage & PROPERTY_USAGE_CATEGORY)) {
+			// Fallback check so that we don't lose editor fields
+			// for properties coming from _get_property_list
+			if (added.has(current->get().name)) {
+				r_list.erase(current);
+			} else {
+				properties_left = true;
+				previous = current;
+			}
+
+			current = previous->next();
+		}
+
+		if (!properties_left) {
+			r_list.erase(script_variables);
 		}
 		r_list.erase(bottom);
 	}
diff --git a/editor/property_selector.cpp b/editor/property_selector.cpp
index f167ded4e7..48ce56ba06 100644
--- a/editor/property_selector.cpp
+++ b/editor/property_selector.cpp
@@ -105,7 +105,7 @@ void PropertySelector::_update_search() {
 			Object *obj = ObjectDB::get_instance(script);
 			if (Object::cast_to<Script>(obj)) {
 				props.push_back(PropertyInfo(Variant::NIL, "Script Variables", PROPERTY_HINT_NONE, "", PROPERTY_USAGE_CATEGORY));
-				Object::cast_to<Script>(obj)->get_script_property_list(&props);
+				Object::cast_to<Script>(obj)->get_script_property_list(&props, obj->get_script_instance());
 			}
 
 			StringName base = base_type;
diff --git a/modules/gdnative/nativescript/nativescript.cpp b/modules/gdnative/nativescript/nativescript.cpp
index 368eb67fa6..4b25bfc1ac 100644
--- a/modules/gdnative/nativescript/nativescript.cpp
+++ b/modules/gdnative/nativescript/nativescript.cpp
@@ -96,7 +96,7 @@ void NativeScript::_update_placeholder(PlaceHolderScriptInstance *p_placeholder)
 	ERR_FAIL_COND(!script_data);
 
 	List<PropertyInfo> info;
-	get_script_property_list(&info);
+	get_script_property_list(&info, p_placeholder);
 	Map<StringName, Variant> values;
 	for (const PropertyInfo &E : info) {
 		Variant value;
@@ -413,7 +413,7 @@ void NativeScript::get_script_method_list(List<MethodInfo> *p_list) const {
 	}
 }
 
-void NativeScript::get_script_property_list(List<PropertyInfo> *p_list) const {
+void NativeScript::get_script_property_list(List<PropertyInfo> *p_list, const ScriptInstance *p_instance) const {
 	NativeScriptDesc *script_data = get_script_desc();
 
 	Set<StringName> existing_properties;
@@ -651,7 +651,7 @@ bool NativeScriptInstance::get(const StringName &p_name, Variant &r_ret) const {
 }
 
 void NativeScriptInstance::get_property_list(List<PropertyInfo> *p_properties) const {
-	script->get_script_property_list(p_properties);
+	script->get_script_property_list(p_properties, this);
 
 	NativeScriptDesc *script_data = GET_SCRIPT_DESC();
 
diff --git a/modules/gdnative/nativescript/nativescript.h b/modules/gdnative/nativescript/nativescript.h
index 2364c6c0f6..d59335d8a1 100644
--- a/modules/gdnative/nativescript/nativescript.h
+++ b/modules/gdnative/nativescript/nativescript.h
@@ -173,7 +173,7 @@ public:
 
 	virtual void update_exports() override; //editor tool
 	virtual void get_script_method_list(List<MethodInfo> *p_list) const override;
-	virtual void get_script_property_list(List<PropertyInfo> *p_list) const override;
+	virtual void get_script_property_list(List<PropertyInfo> *p_list, const ScriptInstance *p_instance = nullptr) const override;
 
 	virtual const Vector<Multiplayer::RPCConfig> get_rpc_methods() const override;
 
diff --git a/modules/gdnative/pluginscript/pluginscript_instance.cpp b/modules/gdnative/pluginscript/pluginscript_instance.cpp
index feae81397e..efa84353d0 100644
--- a/modules/gdnative/pluginscript/pluginscript_instance.cpp
+++ b/modules/gdnative/pluginscript/pluginscript_instance.cpp
@@ -68,7 +68,7 @@ Variant::Type PluginScriptInstance::get_property_type(const StringName &p_name,
 }
 
 void PluginScriptInstance::get_property_list(List<PropertyInfo> *p_properties) const {
-	_script->get_script_property_list(p_properties);
+	_script->get_script_property_list(p_properties, this);
 }
 
 void PluginScriptInstance::get_method_list(List<MethodInfo> *p_list) const {
diff --git a/modules/gdnative/pluginscript/pluginscript_script.cpp b/modules/gdnative/pluginscript/pluginscript_script.cpp
index 04a293ddbd..cc4ba245e7 100644
--- a/modules/gdnative/pluginscript/pluginscript_script.cpp
+++ b/modules/gdnative/pluginscript/pluginscript_script.cpp
@@ -373,7 +373,7 @@ void PluginScript::get_script_method_list(List<MethodInfo> *r_methods) const {
 	}
 }
 
-void PluginScript::get_script_property_list(List<PropertyInfo> *r_properties) const {
+void PluginScript::get_script_property_list(List<PropertyInfo> *r_properties, const ScriptInstance *p_instance) const {
 	ASSERT_SCRIPT_VALID();
 	for (Map<StringName, PropertyInfo>::Element *e = _properties_info.front(); e != nullptr; e = e->next()) {
 		r_properties->push_back(e->get());
diff --git a/modules/gdnative/pluginscript/pluginscript_script.h b/modules/gdnative/pluginscript/pluginscript_script.h
index 1a12a130d1..a425ec7e8a 100644
--- a/modules/gdnative/pluginscript/pluginscript_script.h
+++ b/modules/gdnative/pluginscript/pluginscript_script.h
@@ -132,7 +132,7 @@ public:
 
 	virtual void update_exports() override;
 	virtual void get_script_method_list(List<MethodInfo> *r_methods) const override;
-	virtual void get_script_property_list(List<PropertyInfo> *r_properties) const override;
+	virtual void get_script_property_list(List<PropertyInfo> *r_properties, const ScriptInstance *p_instance = nullptr) const override;
 
 	virtual int get_member_line(const StringName &p_member) const override;
 
diff --git a/modules/gdscript/gdscript.cpp b/modules/gdscript/gdscript.cpp
index e4d3ed118c..317366aa2a 100644
--- a/modules/gdscript/gdscript.cpp
+++ b/modules/gdscript/gdscript.cpp
@@ -280,6 +280,68 @@ void GDScript::get_script_method_list(List<MethodInfo> *r_list) const {
 	_get_script_method_list(r_list, true);
 }
 
+void GDScript::_call_get_script_property_list(List<PropertyInfo> *r_list, const ScriptInstance *p_instance, bool p_include_base) const {
+	const GDScript *sptr = this;
+	List<PropertyInfo> props;
+
+	while (sptr) {
+		const Map<StringName, GDScriptFunction *>::Element *E = sptr->member_functions.find(GDScriptLanguage::get_singleton()->strings._get_property_list);
+		if (E) {
+			Callable::CallError err;
+			Variant ret = const_cast<GDScriptFunction *>(E->get())->call(const_cast<GDScriptInstance *>(dynamic_cast<const GDScriptInstance *>(p_instance)), nullptr, 0, err);
+			if (err.error == Callable::CallError::CALL_OK) {
+				ERR_FAIL_COND_MSG(ret.get_type() != Variant::ARRAY, "Wrong type for _get_property_list, must be an array of dictionaries.");
+
+				Array arr = ret;
+				for (int i = 0; i < arr.size(); i++) {
+					Dictionary d = arr[i];
+					ERR_CONTINUE(!d.has("name"));
+					ERR_CONTINUE(!d.has("type"));
+					PropertyInfo pinfo;
+					pinfo.type = Variant::Type(d["type"].operator int());
+					ERR_CONTINUE(pinfo.type < 0 || pinfo.type >= Variant::VARIANT_MAX);
+					pinfo.name = d["name"];
+					ERR_CONTINUE(pinfo.name == "");
+					if (d.has("hint")) {
+						pinfo.hint = PropertyHint(d["hint"].operator int());
+					}
+					if (d.has("hint_string")) {
+						pinfo.hint_string = d["hint_string"];
+					}
+					if (d.has("usage")) {
+						pinfo.usage = d["usage"];
+					}
+
+					props.push_back(pinfo);
+				}
+			}
+		}
+
+		//instance a fake script for editing the values
+
+		Vector<_GDScriptMemberSort> msort;
+		for (const KeyValue<StringName, PropertyInfo> &F : sptr->member_info) {
+			_GDScriptMemberSort ms;
+			ERR_CONTINUE(!sptr->member_indices.has(F.key));
+			ms.index = sptr->member_indices[F.key].index;
+			ms.name = F.key;
+			msort.push_back(ms);
+		}
+
+		msort.sort();
+		msort.reverse();
+		for (int i = 0; i < msort.size(); i++) {
+			props.push_front(sptr->member_info[msort[i].name]);
+		}
+
+		sptr = sptr->_base;
+	}
+
+	for (const PropertyInfo &E : props) {
+		r_list->push_back(E);
+	}
+}
+
 void GDScript::_get_script_property_list(List<PropertyInfo> *r_list, bool p_include_base) const {
 	const GDScript *sptr = this;
 	List<PropertyInfo> props;
@@ -311,8 +373,12 @@ void GDScript::_get_script_property_list(List<PropertyInfo> *r_list, bool p_incl
 	}
 }
 
-void GDScript::get_script_property_list(List<PropertyInfo> *r_list) const {
+void GDScript::get_script_property_list(List<PropertyInfo> *r_list, const ScriptInstance *p_instance) const {
 	_get_script_property_list(r_list, true);
+
+	if (p_instance) {
+		_call_get_script_property_list(r_list, p_instance, true);
+	}
 }
 
 bool GDScript::has_method(const StringName &p_method) const {
@@ -1406,64 +1472,7 @@ void GDScriptInstance::get_property_list(List<PropertyInfo> *p_properties) const
 	// exported members, not done yet!
 
 	const GDScript *sptr = script.ptr();
-	List<PropertyInfo> props;
-
-	while (sptr) {
-		const Map<StringName, GDScriptFunction *>::Element *E = sptr->member_functions.find(GDScriptLanguage::get_singleton()->strings._get_property_list);
-		if (E) {
-			Callable::CallError err;
-			Variant ret = const_cast<GDScriptFunction *>(E->get())->call(const_cast<GDScriptInstance *>(this), nullptr, 0, err);
-			if (err.error == Callable::CallError::CALL_OK) {
-				ERR_FAIL_COND_MSG(ret.get_type() != Variant::ARRAY, "Wrong type for _get_property_list, must be an array of dictionaries.");
-
-				Array arr = ret;
-				for (int i = 0; i < arr.size(); i++) {
-					Dictionary d = arr[i];
-					ERR_CONTINUE(!d.has("name"));
-					ERR_CONTINUE(!d.has("type"));
-					PropertyInfo pinfo;
-					pinfo.type = Variant::Type(d["type"].operator int());
-					ERR_CONTINUE(pinfo.type < 0 || pinfo.type >= Variant::VARIANT_MAX);
-					pinfo.name = d["name"];
-					ERR_CONTINUE(pinfo.name == "");
-					if (d.has("hint")) {
-						pinfo.hint = PropertyHint(d["hint"].operator int());
-					}
-					if (d.has("hint_string")) {
-						pinfo.hint_string = d["hint_string"];
-					}
-					if (d.has("usage")) {
-						pinfo.usage = d["usage"];
-					}
-
-					props.push_back(pinfo);
-				}
-			}
-		}
-
-		//instance a fake script for editing the values
-
-		Vector<_GDScriptMemberSort> msort;
-		for (const KeyValue<StringName, PropertyInfo> &F : sptr->member_info) {
-			_GDScriptMemberSort ms;
-			ERR_CONTINUE(!sptr->member_indices.has(F.key));
-			ms.index = sptr->member_indices[F.key].index;
-			ms.name = F.key;
-			msort.push_back(ms);
-		}
-
-		msort.sort();
-		msort.reverse();
-		for (int i = 0; i < msort.size(); i++) {
-			props.push_front(sptr->member_info[msort[i].name]);
-		}
-
-		sptr = sptr->_base;
-	}
-
-	for (const PropertyInfo &E : props) {
-		p_properties->push_back(E);
-	}
+	sptr->_call_get_script_property_list(p_properties, this, true);
 }
 
 void GDScriptInstance::get_method_list(List<MethodInfo> *p_list) const {
diff --git a/modules/gdscript/gdscript.h b/modules/gdscript/gdscript.h
index 791f8a1431..98059bcb2d 100644
--- a/modules/gdscript/gdscript.h
+++ b/modules/gdscript/gdscript.h
@@ -227,10 +227,11 @@ public:
 	bool get_property_default_value(const StringName &p_property, Variant &r_value) const override;
 
 	virtual void get_script_method_list(List<MethodInfo> *p_list) const override;
+	void _call_get_script_property_list(List<PropertyInfo> *r_list, const ScriptInstance *p_instance, bool p_include_base) const;
 	virtual bool has_method(const StringName &p_method) const override;
 	virtual MethodInfo get_method_info(const StringName &p_method) const override;
 
-	virtual void get_script_property_list(List<PropertyInfo> *p_list) const override;
+	virtual void get_script_property_list(List<PropertyInfo> *p_list, const ScriptInstance *p_instance = nullptr) const override;
 
 	virtual ScriptLanguage *get_language() const override;
 
diff --git a/modules/gdscript/gdscript_editor.cpp b/modules/gdscript/gdscript_editor.cpp
index 71d2699c2e..e3874e9121 100644
--- a/modules/gdscript/gdscript_editor.cpp
+++ b/modules/gdscript/gdscript_editor.cpp
@@ -1938,7 +1938,7 @@ static bool _guess_identifier_type_from_base(GDScriptParser::CompletionContext &
 
 					if (!is_static) {
 						List<PropertyInfo> members;
-						scr->get_script_property_list(&members);
+						scr->get_script_property_list(&members, nullptr);
 						for (const PropertyInfo &prop : members) {
 							if (prop.name == p_identifier) {
 								r_type = _type_from_property(prop);
diff --git a/modules/mono/csharp_script.cpp b/modules/mono/csharp_script.cpp
index d9b291489b..32fa76e840 100644
--- a/modules/mono/csharp_script.cpp
+++ b/modules/mono/csharp_script.cpp
@@ -3503,7 +3503,7 @@ Ref<Script> CSharpScript::get_base_script() const {
 	return Ref<Script>();
 }
 
-void CSharpScript::get_script_property_list(List<PropertyInfo> *r_list) const {
+void CSharpScript::get_script_property_list(List<PropertyInfo> *r_list, const ScriptInstance *p_instance) const {
 	List<PropertyInfo> props;
 
 	for (OrderedHashMap<StringName, PropertyInfo>::ConstElement E = member_info.front(); E; E = E.next()) {
diff --git a/modules/mono/csharp_script.h b/modules/mono/csharp_script.h
index c998d9c1e4..ba6804f7ce 100644
--- a/modules/mono/csharp_script.h
+++ b/modules/mono/csharp_script.h
@@ -215,7 +215,7 @@ public:
 	void get_script_signal_list(List<MethodInfo> *r_signals) const override;
 
 	bool get_property_default_value(const StringName &p_property, Variant &r_value) const override;
-	void get_script_property_list(List<PropertyInfo> *r_list) const override;
+	void get_script_property_list(List<PropertyInfo> *r_list, const ScriptInstance *p_instance = nullptr) const override;
 	void update_exports() override;
 
 	void get_members(Set<StringName> *p_members) override;
diff --git a/modules/visual_script/visual_script.cpp b/modules/visual_script/visual_script.cpp
index 700cc85672..d056bf5de3 100644
--- a/modules/visual_script/visual_script.cpp
+++ b/modules/visual_script/visual_script.cpp
@@ -925,7 +925,7 @@ MethodInfo VisualScript::get_method_info(const StringName &p_method) const {
 	return mi;
 }
 
-void VisualScript::get_script_property_list(List<PropertyInfo> *p_list) const {
+void VisualScript::get_script_property_list(List<PropertyInfo> *p_list, const ScriptInstance *instance) const {
 	List<StringName> vars;
 	get_variable_list(&vars);
 
diff --git a/modules/visual_script/visual_script.h b/modules/visual_script/visual_script.h
index 39cef8f68b..2e75bd0eff 100644
--- a/modules/visual_script/visual_script.h
+++ b/modules/visual_script/visual_script.h
@@ -358,7 +358,7 @@ public:
 	virtual bool has_method(const StringName &p_method) const override;
 	virtual MethodInfo get_method_info(const StringName &p_method) const override;
 
-	virtual void get_script_property_list(List<PropertyInfo> *p_list) const override;
+	virtual void get_script_property_list(List<PropertyInfo> *p_list, const ScriptInstance *instance = nullptr) const override;
 
 	virtual int get_member_line(const StringName &p_member) const override;
 
diff --git a/modules/visual_script/visual_script_property_selector.cpp b/modules/visual_script/visual_script_property_selector.cpp
index d8b88d6cd3..c5a88d6200 100644
--- a/modules/visual_script/visual_script_property_selector.cpp
+++ b/modules/visual_script/visual_script_property_selector.cpp
@@ -154,7 +154,7 @@ void VisualScriptPropertySelector::_update_search() {
 			} else {
 				Object *obj = ObjectDB::get_instance(script);
 				if (Object::cast_to<Script>(obj)) {
-					Object::cast_to<Script>(obj)->get_script_property_list(&props);
+					Object::cast_to<Script>(obj)->get_script_property_list(&props, obj->get_script_instance());
 				} else {
 					ClassDB::get_property_list(E, &props, true);
 				}