Support both semantic (by default) and numeric Variant hash comparison

Hash comparison for Variant continues to perform semantic/logical comparison with NaN's considered equal by default (to prevent #16114, #7354, #6947, #8081), but now optionally allows for numeric comparison that does not consider NaN's equal to support proper value comparison (for #72222)
godotengine · Aug 31, 2023 · ee27254 · ee27254
1 parent 549fcce
commit ee27254
Show file tree

Hide file tree

Showing 4 changed files with 10 additions and 6 deletions.
diff --git a/core/variant/array.cpp b/core/variant/array.cpp
@@ -137,7 +137,7 @@ bool Array::recursive_equal(const Array &p_array, int recursion_count) const {
 	}
 	recursion_count++;
 	for (int i = 0; i < size; i++) {
-		if (!a1[i].hash_compare(a2[i], recursion_count)) {
+		if (!a1[i].hash_compare(a2[i], recursion_count, false)) {
 			return false;
 		}
 	}

diff --git a/core/variant/dictionary.cpp b/core/variant/dictionary.cpp
@@ -210,7 +210,7 @@ bool Dictionary::recursive_equal(const Dictionary &p_dictionary, int recursion_c
 	recursion_count++;
 	for (const KeyValue<Variant, Variant> &this_E : _p->variant_map) {
 		HashMap<Variant, Variant, VariantHasher, StringLikeVariantComparator>::ConstIterator other_E(p_dictionary._p->variant_map.find(this_E.key));
-		if (!other_E || !this_E.value.hash_compare(other_E->value, recursion_count)) {
+		if (!other_E || !this_E.value.hash_compare(other_E->value, recursion_count, false)) {
 			return false;
 		}
 	}

diff --git a/core/variant/variant.cpp b/core/variant/variant.cpp
@@ -3235,8 +3235,11 @@ uint32_t Variant::recursive_hash(int recursion_count) const {
 	return 0;
 }
 
+#define hash_compare_scalar_base(p_lhs, p_rhs, semantic_comparison) \
+	(((p_lhs) == (p_rhs)) || (semantic_comparison && Math::is_nan(p_lhs) && Math::is_nan(p_rhs)))
+
 #define hash_compare_scalar(p_lhs, p_rhs) \
-	(((p_lhs) == (p_rhs)) || (Math::is_nan(p_lhs) && Math::is_nan(p_rhs)))
+	(hash_compare_scalar_base(p_lhs, p_rhs, true))
 
 #define hash_compare_vector2(p_lhs, p_rhs)        \
 	(hash_compare_scalar((p_lhs).x, (p_rhs).x) && \
@@ -3282,7 +3285,7 @@ uint32_t Variant::recursive_hash(int recursion_count) const {
                                                                         \
 	return true
 
-bool Variant::hash_compare(const Variant &p_variant, int recursion_count) const {
+bool Variant::hash_compare(const Variant &p_variant, int recursion_count, bool semantic_comparison) const {
 	if (type != p_variant.type) {
 		return false;
 	}
@@ -3293,7 +3296,7 @@ bool Variant::hash_compare(const Variant &p_variant, int recursion_count) const
 		} break;
 
 		case FLOAT: {
-			return hash_compare_scalar(_data._float, p_variant._data._float);
+			return hash_compare_scalar_base(_data._float, p_variant._data._float, semantic_comparison);
 		} break;
 
 		case STRING: {

diff --git a/core/variant/variant.h b/core/variant/variant.h
@@ -751,7 +751,8 @@ class Variant {
 	uint32_t hash() const;
 	uint32_t recursive_hash(int recursion_count) const;
 
-	bool hash_compare(const Variant &p_variant, int recursion_count = 0) const;
+	// By default, performs a semantic comparison. Otherwise, numeric/binary comparison (if appropriate).
+	bool hash_compare(const Variant &p_variant, int recursion_count = 0, bool semantic_comparison = true) const;
 	bool identity_compare(const Variant &p_variant) const;
 	bool booleanize() const;
 	String stringify(int recursion_count = 0) const;