From 8ff6e538336135837630e313027826e9bd8d036e Mon Sep 17 00:00:00 2001 From: Hein-Pieter van Braam Date: Fri, 14 Apr 2017 11:28:51 +0200 Subject: Correct Variant::hash_compare() There was a logic error in #7815 which made Variant.hash_compare() == Variant.hash_compare() always true. In an attempt to short-circuit the NaN check I made an (in hindsight) obvious error: 10 == 12 || is_nan(10) == is_nan(12) This will be true for all inputs, except for the NaN, not-NaN case. The macro has been updated to now generate: (10 == 12) || (is_nan(10) && is_nan(10)) so: (10 == 12) || (is_nan(10) && is_nan(12)) = false False or (False and False) is False (10 == 10) || (is_nan(10) && is_nan(10)) = true True or (False and False) is True (Nan == 10) || (is_nan(NaN) && is_nan(10)) = false False or (True and False) is False (Nan == Nan) || (is_nan(NaN) && is_nan(NaN)) = true False or (True and True) is True Which is correct for all cases. This bug was triggered because the hash function for floating point numbers can very easily generate collisions for the tested Vector3(). I've also added an extra hashing step to the float hash function to make this less likely to occur. This fixes #8081 and probably many more random weirdness. --- core/variant.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'core/variant.cpp') diff --git a/core/variant.cpp b/core/variant.cpp index 6e675d07de..67ce8af483 100644 --- a/core/variant.cpp +++ b/core/variant.cpp @@ -2839,7 +2839,7 @@ uint32_t Variant::hash() const { } #define hash_compare_scalar(p_lhs, p_rhs) \ - ((p_lhs) == (p_rhs)) || (Math::is_nan(p_lhs) == Math::is_nan(p_rhs)) + ((p_lhs) == (p_rhs)) || (Math::is_nan(p_lhs) && Math::is_nan(p_rhs)) #define hash_compare_vector2(p_lhs, p_rhs) \ (hash_compare_scalar((p_lhs).x, (p_rhs).x)) && \ -- cgit v1.2.3