[3.x] Rationalize property reversion

core/script_language.h

@@ -119,6 +119,8 @@ class Script : public Resource {
 
 	virtual Ref<Script> get_base_script() const = 0; //for script inheritance
 
+	virtual bool inherits_script(const Ref<Script> &p_script) const = 0;
+
 	virtual StringName get_instance_base_type() const = 0; // this may not work in all scripts, will return empty if so
 	virtual ScriptInstance *instance_create(Object *p_this) = 0;
 	virtual PlaceHolderScriptInstance *placeholder_instance_create(Object *p_this) { return nullptr; }

editor/editor_inspector.cpp

@@ -343,7 +343,7 @@ bool EditorPropertyRevert::may_node_be_in_instance(Node *p_node) {
 	return might_be; // or might not be
 }
 
-bool EditorPropertyRevert::get_instanced_node_original_property(Node *p_node, const StringName &p_prop, Variant &value) {
+bool EditorPropertyRevert::get_instanced_node_original_property(Node *p_node, const StringName &p_prop, Variant &value, bool p_check_class_default) {
 	Node *node = p_node;
 	Node *orig = node;
 
@@ -381,7 +381,7 @@ bool EditorPropertyRevert::get_instanced_node_original_property(Node *p_node, co
 		node = node->get_owner();
 	}
 
-	if (!found && p_node) {
+	if (p_check_class_default && !found && p_node) {
 		//if not found, try default class value
 		Variant attempt = ClassDB::class_get_default_property_value(p_node->get_class_name(), p_prop);
 		if (attempt.get_type() != Variant::NIL) {
@@ -419,6 +419,7 @@ bool EditorPropertyRevert::is_node_property_different(Node *p_node, const Varian
 
 			if (ss.is_valid()) {
 				found_state = true;
+				break;
 			}
 			if (node == edited_scene) {
 				//just in case
@@ -432,59 +433,71 @@ bool EditorPropertyRevert::is_node_property_different(Node *p_node, const Varian
 		}
 	}
 
-	if (p_current.get_type() == Variant::REAL && p_orig.get_type() == Variant::REAL) {
-		float a = p_current;
-		float b = p_orig;
+	return is_property_value_different(p_current, p_orig);
+}
 
-		return !Math::is_equal_approx(a, b); //this must be done because, as some scenes save as text, there might be a tiny difference in floats due to numerical error
+bool EditorPropertyRevert::is_property_value_different(const Variant &p_a, const Variant &p_b) {
+	if (p_a.get_type() == Variant::REAL && p_b.get_type() == Variant::REAL) {
+		//this must be done because, as some scenes save as text, there might be a tiny difference in floats due to numerical error
+		return !Math::is_equal_approx((float)p_a, (float)p_b);
+	} else {
+		return p_a != p_b;
 	}
-
-	return bool(Variant::evaluate(Variant::OP_NOT_EQUAL, p_current, p_orig));
 }
 
-bool EditorPropertyRevert::can_property_revert(Object *p_object, const StringName &p_property) {
-	bool has_revert = false;
+Variant EditorPropertyRevert::get_property_revert_value(Object *p_object, const StringName &p_property) {
+	// If the object implements property_can_revert, rely on that completely
+	// (i.e. don't then try to revert to default value - the property_get_revert implementation
+	// can do that if so desired)
+	if (p_object->has_method("property_can_revert") && p_object->call("property_can_revert", p_property)) {
+		return p_object->call("property_get_revert", p_property);
+	}
 
+	Ref<Script> scr = p_object->get_script();
 	Node *node = Object::cast_to<Node>(p_object);
-
 	if (node && EditorPropertyRevert::may_node_be_in_instance(node)) {
-		//check for difference including instantiation
-		Variant vorig;
-		if (EditorPropertyRevert::get_instanced_node_original_property(node, p_property, vorig)) {
-			Variant v = p_object->get(p_property);
-
-			if (EditorPropertyRevert::is_node_property_different(node, v, vorig)) {
-				has_revert = true;
+		//if this node is an instance or inherits, but it has a script attached which is unrelated
+		//to the one set for the parent and also has a default value for the property, consider that
+		//has precedence over the value from the parent, because that is an explicit source of defaults
+		//closer in the tree to the current node
+		bool ignore_parent = false;
+		if (scr.is_valid()) {
+			Variant sorig;
+			if (EditorPropertyRevert::get_instanced_node_original_property(node, "script", sorig) && !scr->inherits_script(sorig)) {
+				Variant dummy;
+				if (scr->get_property_default_value(p_property, dummy)) {
+					ignore_parent = true;
+				}
 			}
 		}
-	} else {
-		//check for difference against default class value instead
-		Variant default_value = ClassDB::class_get_default_property_value(p_object->get_class_name(), p_property);
-		if (default_value != Variant() && default_value != p_object->get(p_property)) {
-			has_revert = true;
+
+		if (!ignore_parent) {
+			//check for difference including instantiation
+			Variant vorig;
+			if (EditorPropertyRevert::get_instanced_node_original_property(node, p_property, vorig, false)) {
+				return vorig;
+			}
 		}
 	}
 
-	// If the object implements property_can_revert, rely on that completely
-	// (i.e. don't then try to revert to default value - the property_get_revert implementation
-	// can do that if so desired)
-	if (p_object->has_method("property_can_revert")) {
-		has_revert = p_object->call("property_can_revert", p_property).operator bool();
-	} else {
-		if (!has_revert && !p_object->get_script().is_null()) {
-			Ref<Script> scr = p_object->get_script();
-			if (scr.is_valid()) {
-				Variant orig_value;
-				if (scr->get_property_default_value(p_property, orig_value)) {
-					if (orig_value != p_object->get(p_property)) {
-						has_revert = true;
-					}
-				}
-			}
+	if (scr.is_valid()) {
+		Variant orig_value;
+		if (scr->get_property_default_value(p_property, orig_value)) {
+			return orig_value;
 		}
 	}
 
-	return has_revert;
+	//report default class value instead
+	return ClassDB::class_get_default_property_value(p_object->get_class_name(), p_property);
+}
+
+bool EditorPropertyRevert::can_property_revert(Object *p_object, const StringName &p_property) {
+	Variant revert_value = EditorPropertyRevert::get_property_revert_value(p_object, p_property);
+	if (revert_value.get_type() == Variant::NIL) {
+		return false;
+	}
+	Variant current_value = p_object->get(p_property);
+	return EditorPropertyRevert::is_property_value_different(current_value, revert_value);
 }
 
 void EditorProperty::update_reload_status() {
@@ -656,41 +669,11 @@ void EditorProperty::_gui_input(const Ref<InputEvent> &p_event) {
 		}
 
 		if (revert_rect.has_point(mb->get_position())) {
-			Variant vorig;
-
-			Node *node = Object::cast_to<Node>(object);
-			if (node && EditorPropertyRevert::may_node_be_in_instance(node) && EditorPropertyRevert::get_instanced_node_original_property(node, property, vorig)) {
-				emit_changed(property, vorig.duplicate(true));
-				update_property();
-				return;
-			}
-
-			if (object->call("property_can_revert", property).operator bool()) {
-				Variant rev = object->call("property_get_revert", property);
-				emit_changed(property, rev);
-				update_property();
-				return;
-			}
-
-			if (!object->get_script().is_null()) {
-				Ref<Script> scr = object->get_script();
-				if (scr.is_valid()) {
-					Variant orig_value;
-					if (scr->get_property_default_value(property, orig_value)) {
-						emit_changed(property, orig_value);
-						update_property();
-						return;
-					}
-				}
-			}
-
-			Variant default_value = ClassDB::class_get_default_property_value(object->get_class_name(), property);
-			if (default_value != Variant()) {
-				emit_changed(property, default_value);
-				update_property();
-				return;
-			}
+			Variant revert_value = EditorPropertyRevert::get_property_revert_value(object, property);
+			emit_changed(property, revert_value);
+			update_property();
 		}
+
 		if (check_rect.has_point(mb->get_position())) {
 			checked = !checked;
 			update();

editor/editor_inspector.h

@@ -40,8 +40,10 @@ class UndoRedo;
 class EditorPropertyRevert {
 public:
 	static bool may_node_be_in_instance(Node *p_node);
-	static bool get_instanced_node_original_property(Node *p_node, const StringName &p_prop, Variant &value);
+	static bool get_instanced_node_original_property(Node *p_node, const StringName &p_prop, Variant &value, bool p_check_class_default = true);
 	static bool is_node_property_different(Node *p_node, const Variant &p_current, const Variant &p_orig);
+	static bool is_property_value_different(const Variant &p_a, const Variant &p_b);
+	static Variant get_property_revert_value(Object *p_object, const StringName &p_property);
 
 	static bool can_property_revert(Object *p_object, const StringName &p_property);
 };

modules/gdnative/nativescript/nativescript.cpp

@@ -111,6 +111,29 @@ void NativeScript::_placeholder_erased(PlaceHolderScriptInstance *p_placeholder)
 
 #endif
 
+bool NativeScript::inherits_script(const Ref<Script> &p_script) const {
+	Ref<NativeScript> ns = p_script;
+	if (ns.is_null()) {
+		return false;
+	}
+
+	const NativeScriptDesc *other_s = ns->get_script_desc();
+	if (!other_s) {
+		return false;
+	}
+
+	const NativeScriptDesc *s = get_script_desc();
+
+	while (s) {
+		if (s == other_s) {
+			return true;
+		}
+		s = s->base_data;
+	}
+
+	return false;
+}
+
 void NativeScript::set_class_name(String p_class_name) {
 	class_name = p_class_name;
 }

modules/gdnative/nativescript/nativescript.h

@@ -130,6 +130,8 @@ class NativeScript : public Script {
 public:
 	inline NativeScriptDesc *get_script_desc() const;
 
+	bool inherits_script(const Ref<Script> &p_script) const;
+
 	void set_class_name(String p_class_name);
 	String get_class_name() const;
 

modules/gdnative/pluginscript/pluginscript_script.cpp

@@ -136,6 +136,24 @@ bool PluginScript::can_instance() const {
 	return can;
 }
 
+bool PluginScript::inherits_script(const Ref<Script> &p_script) const {
+	Ref<PluginScript> ps = p_script;
+	if (ps.is_null()) {
+		return false;
+	}
+
+	const PluginScript *s = this;
+
+	while (s) {
+		if (s == p_script.ptr()) {
+			return true;
+		}
+		s = Object::cast_to<PluginScript>(s->_ref_base_parent.ptr());
+	}
+
+	return false;
+}
+
 Ref<Script> PluginScript::get_base_script() const {
 	if (_ref_base_parent.is_valid()) {
 		return Ref<PluginScript>(_ref_base_parent);

modules/gdnative/pluginscript/pluginscript_script.h

@@ -71,6 +71,8 @@ class PluginScript : public Script {
 protected:
 	static void _bind_methods();
 
+	bool inherits_script(const Ref<Script> &p_script) const;
+
 	PluginScriptInstance *_create_instance(const Variant **p_args, int p_argcount, Object *p_owner, Variant::CallError &r_error);
 	Variant _new(const Variant **p_args, int p_argcount, Variant::CallError &r_error);
 

modules/gdscript/gdscript.cpp

@@ -815,6 +815,24 @@ Ref<GDScript> GDScript::get_base() const {
 	return base;
 }
 
+bool GDScript::inherits_script(const Ref<Script> &p_script) const {
+	Ref<GDScript> gd = p_script;
+	if (gd.is_null()) {
+		return false;
+	}
+
+	const GDScript *s = this;
+
+	while (s) {
+		if (s == p_script.ptr()) {
+			return true;
+		}
+		s = s->_base;
+	}
+
+	return false;
+}
+
 bool GDScript::has_script_signal(const StringName &p_signal) const {
 	if (_signals.has(p_signal)) {
 		return true;

modules/gdscript/gdscript.h

@@ -146,6 +146,8 @@ class GDScript : public Script {
 public:
 	virtual bool is_valid() const { return valid; }
 
+	bool inherits_script(const Ref<Script> &p_script) const;
+
 	const Map<StringName, Ref<GDScript>> &get_subclasses() const { return subclasses; }
 	const Map<StringName, Variant> &get_constants() const { return constants; }
 	const Set<StringName> &get_members() const { return members; }

modules/mono/csharp_script.cpp

@@ -3205,6 +3205,23 @@ void CSharpScript::get_script_signal_list(List<MethodInfo> *r_signals) const {
 	}
 }
 
+bool CSharpScript::inherits_script(const Ref<Script> &p_script) const {
+	Ref<CSharpScript> cs = p_script;
+	if (cs.is_null()) {
+		return false;
+	}
+
+	if (script_class == nullptr || cs->script_class == nullptr) {
+		return false;
+	}
+
+	if (script_class == cs->script_class) {
+		return true;
+	}
+
+	return cs->script_class->is_assignable_from(script_class);
+}
+
 Ref<Script> CSharpScript::get_base_script() const {
 	// TODO search in metadata file once we have it, not important any way?
 	return Ref<Script>();

modules/mono/csharp_script.h

@@ -184,6 +184,8 @@ class CSharpScript : public Script {
 	virtual bool is_tool() const { return tool; }
 	virtual bool is_valid() const { return valid; }
 
+	bool inherits_script(const Ref<Script> &p_script) const;
+
 	virtual Ref<Script> get_base_script() const;
 	virtual ScriptLanguage *get_language() const;
 

modules/visual_script/visual_script.cpp

@@ -1265,6 +1265,10 @@ VisualScript::VisualScript() {
 	is_tool_script = false;
 }
 
+bool VisualScript::inherits_script(const Ref<Script> &p_script) const {
+	return this == p_script.ptr(); //there is no inheritance in visual scripts, so this is enough
+}
+
 StringName VisualScript::get_default_func() const {
 	return StringName("f_312843592");
 }

modules/visual_script/visual_script.h

@@ -260,6 +260,8 @@ class VisualScript : public Script {
 	static void _bind_methods();
 
 public:
+	bool inherits_script(const Ref<Script> &p_script) const;
+
 	// TODO: Remove it in future when breaking changes are acceptable
 	StringName get_default_func() const;
 	void add_function(const StringName &p_name);