Use approximate test when comparing properties, thoroughly

Author: RandomShaper

core/array.cpp

@@ -89,7 +89,7 @@ void Array::clear() {
 	_p->array.clear();
 }
 
-bool Array::deep_equal(const Array &p_array, int p_recursion_count) const {
+bool Array::deep_equal(const Array &p_array, int p_recursion_count, bool p_approximate) const {
 	// Cheap checks
 	ERR_FAIL_COND_V_MSG(p_recursion_count > MAX_RECURSION, true, "Max recursion reached");
 	if (_p == p_array._p) {
@@ -105,7 +105,7 @@ bool Array::deep_equal(const Array &p_array, int p_recursion_count) const {
 	// Heavy O(n) check
 	p_recursion_count++;
 	for (int i = 0; i < size; i++) {
-		if (!a1[i].deep_equal(a2[i], p_recursion_count)) {
+		if (!a1[i].deep_equal(a2[i], p_recursion_count, p_approximate)) {
 			return false;
 		}
 	}

core/array.h

@@ -56,7 +56,7 @@ class Array {
 	bool empty() const;
 	void clear();
 
-	bool deep_equal(const Array &p_array, int p_recursion_count = 0) const;
+	bool deep_equal(const Array &p_array, int p_recursion_count = 0, bool p_approximate = false) const;
 	bool operator==(const Array &p_array) const;
 
 	uint32_t hash() const;

core/dictionary.cpp

@@ -162,7 +162,7 @@ bool Dictionary::erase(const Variant &p_key) {
 	return _p->variant_map.erase(p_key);
 }
 
-bool Dictionary::deep_equal(const Dictionary &p_dictionary, int p_recursion_count) const {
+bool Dictionary::deep_equal(const Dictionary &p_dictionary, int p_recursion_count, bool p_approximate) const {
 	// Cheap checks
 	ERR_FAIL_COND_V_MSG(p_recursion_count > MAX_RECURSION, 0, "Max recursion reached");
 	if (_p == p_dictionary._p) {
@@ -178,8 +178,8 @@ bool Dictionary::deep_equal(const Dictionary &p_dictionary, int p_recursion_coun
 	p_recursion_count++;
 	while (this_E && other_E) {
 		if (
-				!this_E.key().deep_equal(other_E.key(), p_recursion_count) ||
-				!this_E.value().deep_equal(other_E.value(), p_recursion_count)) {
+				!this_E.key().deep_equal(other_E.key(), p_recursion_count, p_approximate) ||
+				!this_E.value().deep_equal(other_E.value(), p_recursion_count, p_approximate)) {
 			return false;
 		}
 

core/dictionary.h

@@ -72,7 +72,7 @@ class Dictionary {
 
 	bool erase(const Variant &p_key);
 
-	bool deep_equal(const Dictionary &p_dictionary, int p_recursion_count = 0) const;
+	bool deep_equal(const Dictionary &p_dictionary, int p_recursion_count = 0, bool p_approximate = false) const;
 	bool operator==(const Dictionary &p_dictionary) const;
 	bool operator!=(const Dictionary &p_dictionary) const;
 

core/variant.cpp

@@ -601,35 +601,69 @@ bool Variant::can_convert_strict(Variant::Type p_type_from, Variant::Type p_type
 	return false;
 }
 
-bool Variant::deep_equal(const Variant &p_variant, int p_recursion_count) const {
-	ERR_FAIL_COND_V_MSG(p_recursion_count > MAX_RECURSION, true, "Max recursion reached");
+template <typename T>
+static bool _equal_approx_recursive(const Variant &p_a, const Variant &p_b, int p_recursion_count, bool p_approximate) {
+	if (p_a.get_type() != p_b.get_type()) {
+		return false;
+	}
 
-	// Containers must be handled with recursivity checks
-	switch (type) {
-		case Variant::Type::DICTIONARY: {
-			if (p_variant.type != Variant::Type::DICTIONARY) {
-				return false;
-			}
+	const T &a_as_t = p_a.operator T();
+	const T &b_as_t = p_b.operator T();
 
-			const Dictionary v1_as_d = Dictionary(*this);
-			const Dictionary v2_as_d = Dictionary(p_variant);
+	return a_as_t.deep_equal(b_as_t, p_recursion_count + 1, p_approximate);
+}
 
-			return v1_as_d.deep_equal(v2_as_d, p_recursion_count + 1);
-		} break;
-		case Variant::Type::ARRAY: {
-			if (p_variant.type != Variant::Type::ARRAY) {
-				return false;
-			}
+template <typename T>
+static bool _equal_approx_primitive(const Variant &p_a, const Variant &p_b) {
+	if (p_a.get_type() != p_b.get_type()) {
+		return false;
+	}
 
-			const Array v1_as_a = Array(*this);
-			const Array v2_as_a = Array(p_variant);
+	const T &a_as_t = p_a.operator T();
+	const T &b_as_t = p_b.operator T();
 
-			return v1_as_a.deep_equal(v2_as_a, p_recursion_count + 1);
-		} break;
-		default: {
-			return *this == p_variant;
-		} break;
+	return a_as_t.is_equal_approx(b_as_t);
+}
+
+bool Variant::deep_equal(const Variant &p_variant, int p_recursion_count, bool p_approximate) const {
+	ERR_FAIL_COND_V_MSG(p_recursion_count > MAX_RECURSION, true, "Max recursion reached");
+
+	// Containers must be handled with recursivity checks
+	if (type == ARRAY) {
+		return _equal_approx_recursive<Array>(*this, p_variant, p_recursion_count, p_approximate);
+	} else if (type == DICTIONARY) {
+		return _equal_approx_recursive<Dictionary>(*this, p_variant, p_recursion_count, p_approximate);
+	} else if (p_approximate) {
+		switch (type) {
+			case REAL:
+				return Math::is_equal_approx((double)*this, (double)p_variant);
+			case VECTOR2:
+				return _equal_approx_primitive<Vector2>(*this, p_variant);
+			case RECT2:
+				return _equal_approx_primitive<Rect2>(*this, p_variant);
+			case VECTOR3:
+				return _equal_approx_primitive<Vector3>(*this, p_variant);
+			case TRANSFORM2D:
+				return _equal_approx_primitive<Transform2D>(*this, p_variant);
+			case PLANE:
+				return _equal_approx_primitive<Plane>(*this, p_variant);
+			case QUAT:
+				return _equal_approx_primitive<Quat>(*this, p_variant);
+			case AABB:
+				return _equal_approx_primitive<::AABB>(*this, p_variant);
+			case BASIS:
+				return _equal_approx_primitive<Basis>(*this, p_variant);
+			case TRANSFORM:
+				return _equal_approx_primitive<Transform>(*this, p_variant);
+			case DICTIONARY:
+				return _equal_approx_recursive<Dictionary>(*this, p_variant, p_recursion_count, p_approximate);
+			case ARRAY:
+				return _equal_approx_recursive<Array>(*this, p_variant, p_recursion_count, p_approximate);
+			default: {
+			}
+		}
 	}
+	return *this == p_variant;
 }
 
 bool Variant::operator==(const Variant &p_variant) const {

core/variant.h

@@ -408,7 +408,7 @@ class Variant {
 
 	//argsVariant call()
 
-	bool deep_equal(const Variant &p_variant, int p_recursion_count = 0) const;
+	bool deep_equal(const Variant &p_variant, int p_recursion_count = 0, bool p_approximate = false) const;
 	bool operator==(const Variant &p_variant) const;
 	bool operator!=(const Variant &p_variant) const;
 	bool operator<(const Variant &p_variant) const;

scene/property_utils.cpp

@@ -37,15 +37,11 @@
 #include "scene/resources/packed_scene.h"
 
 bool PropertyUtils::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 {
-		// For our purposes, treating null object as NIL is the right thing to do
-		const Variant &a = p_a.get_type() == Variant::OBJECT && (Object *)p_a == nullptr ? Variant() : p_a;
-		const Variant &b = p_b.get_type() == Variant::OBJECT && (Object *)p_b == nullptr ? Variant() : p_b;
-		return !a.deep_equal(b);
-	}
+	// For our purposes, treating null object as NIL is the right thing to do
+	const Variant &a = p_a.get_type() == Variant::OBJECT && (Object *)p_a == nullptr ? Variant() : p_a;
+	const Variant &b = p_b.get_type() == Variant::OBJECT && (Object *)p_b == nullptr ? Variant() : p_b;
+	// Approximmation must be used because, as some scenes save as text, there might be a tiny difference in floats due to numerical error
+	return !a.deep_equal(b, 0, true);
 }
 
 Variant PropertyUtils::get_property_default_value(const Object *p_object, const StringName &p_property, bool *r_is_valid, const Vector<SceneState::PackState> *p_states_stack_cache, bool p_update_exports, const Node *p_owner, bool *r_is_class_default) {