Merge pull request #42625 from RandomShaper/fix_dict_3.2

core/array.cpp

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

core/array.h

@@ -56,6 +56,7 @@ public:
 	bool empty() const;
 	void clear();
 
+	bool deep_equal(const Array &p_array, int p_recursion_count = 0) const;
 	bool operator==(const Array &p_array) const;
 
 	uint32_t hash() const;

core/dictionary.cpp

@@ -140,6 +140,34 @@ 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 {
+	// Cheap checks
+	ERR_FAIL_COND_V_MSG(p_recursion_count > MAX_RECURSION, 0, "Max recursion reached");
+	if (_p == p_dictionary._p) {
+		return true;
+	}
+	if (_p->variant_map.size() != p_dictionary._p->variant_map.size()) {
+		return false;
+	}
+
+	// Heavy O(n) check
+	OrderedHashMap<Variant, Variant, VariantHasher, VariantComparator>::Element this_E = _p->variant_map.front();
+	OrderedHashMap<Variant, Variant, VariantHasher, VariantComparator>::Element other_E = p_dictionary._p->variant_map.front();
+	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)) {
+			return false;
+		}
+
+		this_E = this_E.next();
+		other_E = other_E.next();
+	}
+
+	return !this_E && !other_E;
+}
+
 bool Dictionary::operator==(const Dictionary &p_dictionary) const {
 	return _p == p_dictionary._p;
 }

core/dictionary.h

@@ -68,6 +68,7 @@ public:
 
 	bool erase(const Variant &p_key);
 
+	bool deep_equal(const Dictionary &p_dictionary, int p_recursion_count = 0) const;
 	bool operator==(const Dictionary &p_dictionary) const;
 	bool operator!=(const Dictionary &p_dictionary) const;
 

core/ordered_hash_map.h

@@ -213,8 +213,12 @@ public:
 			(*list_element)->get().second = p_value;
 			return Element(*list_element);
 		}
-		typename InternalList::Element *new_element = list.push_back(Pair<const K *, V>(NULL, p_value));
+		// Incorrectly set the first value of the pair with a value that will
+		// be invalid as soon as we leave this function...
+		typename InternalList::Element *new_element = list.push_back(Pair<const K *, V>(&p_key, p_value));
+		// ...this is needed here in case the hashmap recursively reference itself...
 		typename InternalMap::Element *e = map.set(p_key, new_element);
+		// ...now we can set the right value !
 		new_element->get().first = &e->key();
 
 		return Element(new_element);

core/typedefs.h

@@ -350,4 +350,7 @@ struct _GlobalLock {
 #define FALLTHROUGH
 #endif
 
+// Limit the depth of recursive algorithms when dealing with Array/Dictionary
+#define MAX_RECURSION 100
+
 #endif // TYPEDEFS_H

core/variant.cpp

@@ -601,6 +601,37 @@ 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");
+
+	// Containers must be handled with recursivity checks
+	switch (type) {
+		case Variant::Type::DICTIONARY: {
+			if (p_variant.type != Variant::Type::DICTIONARY) {
+				return false;
+			}
+
+			const Dictionary v1_as_d = Dictionary(*this);
+			const Dictionary v2_as_d = Dictionary(p_variant);
+
+			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;
+			}
+
+			const Array v1_as_a = Array(*this);
+			const Array v2_as_a = Array(p_variant);
+
+			return v1_as_a.deep_equal(v2_as_a, p_recursion_count + 1);
+		} break;
+		default: {
+			return *this == p_variant;
+		} break;
+	}
+}
+
 bool Variant::operator==(const Variant &p_variant) const {
 	if (type != p_variant.type) { //evaluation of operator== needs to be more strict
 		return false;

core/variant.h

@@ -408,6 +408,7 @@ public:
 
 	//argsVariant call()
 
+	bool deep_equal(const Variant &p_variant, int p_recursion_count = 0) const;
 	bool operator==(const Variant &p_variant) const;
 	bool operator!=(const Variant &p_variant) const;
 	bool operator<(const Variant &p_variant) const;

modules/gdscript/doc_classes/@GDScript.xml

@@ -231,6 +231,19 @@
 				[/codeblock]
 			</description>
 		</method>
+		<method name="deep_equal">
+			<return type="bool" />
+			<argument index="0" name="a" type="Variant" />
+			<argument index="1" name="b" type="Variant" />
+			<description>
+				Compares two values by checking their actual contents, recursing into any `Array` or `Dictionary` up to its deepest level.
+				This compares to [code]==[/code] in a number of ways:
+				- For [code]null[/code], [code]int[/code], [code]float[/code], [code]String[/code], [code]Object[/code] and [code]RID[/code] both [code]deep_equal[/code] and [code]==[/code] work the same.
+				- For [code]Dictionary[/code], [code]==[/code] considers equality if, and only if, both variables point to the very same [code]Dictionary[/code], with no recursion or awareness of the contents at all.
+				- For [code]Array[/code], [code]==[/code] considers equality if, and only if, each item in the first [code]Array[/code] is equal to its counterpart in the second [code]Array[/code], as told by [code]==[/code] itself. That implies that [code]==[/code] recurses into [code]Array[/code], but not into [code]Dictionary[/code].
+				In short, whenever a [code]Dictionary[/code] is potentially involved, if you want a true content-aware comparison, you have to use [code]deep_equal[/code].
+			</description>
+		</method>
 		<method name="deg2rad">
 			<return type="float" />
 			<argument index="0" name="deg" type="float" />

modules/gdscript/gdscript_functions.cpp

@@ -134,6 +134,7 @@ const char *GDScriptFunctions::get_func_name(Function p_func) {
 		"instance_from_id",
 		"len",
 		"is_instance_valid",
+		"deep_equal",
 	};
 
 	return _names[p_func];
@@ -1386,6 +1387,10 @@ void GDScriptFunctions::call(Function p_func, const Variant **p_args, int p_arg_
 			}
 
 		} break;
+		case DEEP_EQUAL: {
+			VALIDATE_ARG_COUNT(2);
+			r_ret = p_args[0]->deep_equal(*p_args[1]);
+		} break;
 		case FUNC_MAX: {
 			ERR_FAIL();
 		} break;
@@ -1955,6 +1960,11 @@ MethodInfo GDScriptFunctions::get_info(Function p_func) {
 			mi.return_val.type = Variant::BOOL;
 			return mi;
 		} break;
+		case DEEP_EQUAL: {
+			MethodInfo mi("deep_equal", PropertyInfo(Variant::NIL, "a"), PropertyInfo(Variant::NIL, "b"));
+			mi.return_val.type = Variant::BOOL;
+			return mi;
+		} break;
 		default: {
 			ERR_FAIL_V(MethodInfo());
 		} break;

modules/gdscript/gdscript_functions.h

@@ -126,6 +126,7 @@ public:
 		INSTANCE_FROM_ID,
 		LEN,
 		IS_INSTANCE_VALID,
+		DEEP_EQUAL,
 		FUNC_MAX
 	};
 

scene/property_utils.cpp

@@ -44,7 +44,7 @@ bool PropertyUtils::is_property_value_different(const Variant &p_a, const Varian
 		// 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 != b;
+		return !a.deep_equal(b);
 	}
 }