From a7aad78fd00f88ca7f1627719b1995a41b07a9ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pedro=20J=2E=20Est=C3=A9banez?= Date: Wed, 7 Oct 2020 19:23:06 +0200 Subject: [PATCH] Add recursive comparison to Array and Dictionary ...and expose it to GDScript. Co-authored-by: Emmanuel Leblond --- core/array.cpp | 24 +++++++++++++++++ core/array.h | 1 + core/dictionary.cpp | 28 +++++++++++++++++++ core/dictionary.h | 1 + core/ordered_hash_map.h | 6 ++++- core/typedefs.h | 3 +++ core/variant.cpp | 31 ++++++++++++++++++++++ core/variant.h | 1 + modules/gdscript/doc_classes/@GDScript.xml | 13 +++++++++ modules/gdscript/gdscript_functions.cpp | 10 +++++++ modules/gdscript/gdscript_functions.h | 1 + 11 files changed, 118 insertions(+), 1 deletion(-) diff --git a/core/array.cpp b/core/array.cpp index f28f1b5340..87212d0405 100644 --- a/core/array.cpp +++ b/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 &a1 = _p->array; + const Vector &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; } diff --git a/core/array.h b/core/array.h index caaa9f9db9..d850c9db3b 100644 --- a/core/array.h +++ b/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; diff --git a/core/dictionary.cpp b/core/dictionary.cpp index b0df027fe5..5f99cc01a1 100644 --- a/core/dictionary.cpp +++ b/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::Element this_E = _p->variant_map.front(); + OrderedHashMap::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; } diff --git a/core/dictionary.h b/core/dictionary.h index d76fc7855b..8ab9d97051 100644 --- a/core/dictionary.h +++ b/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; diff --git a/core/ordered_hash_map.h b/core/ordered_hash_map.h index 70cc16532c..19c2e9c47b 100644 --- a/core/ordered_hash_map.h +++ b/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(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(&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); diff --git a/core/typedefs.h b/core/typedefs.h index e7dc583a62..e43ccbe313 100644 --- a/core/typedefs.h +++ b/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 diff --git a/core/variant.cpp b/core/variant.cpp index ddb22b2b48..e74e70fa4b 100644 --- a/core/variant.cpp +++ b/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; diff --git a/core/variant.h b/core/variant.h index e0dbd90227..a9ca4778b2 100644 --- a/core/variant.h +++ b/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; diff --git a/modules/gdscript/doc_classes/@GDScript.xml b/modules/gdscript/doc_classes/@GDScript.xml index a9dc716e18..1e86182e15 100644 --- a/modules/gdscript/doc_classes/@GDScript.xml +++ b/modules/gdscript/doc_classes/@GDScript.xml @@ -231,6 +231,19 @@ [/codeblock] + + + + + + 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]. + + diff --git a/modules/gdscript/gdscript_functions.cpp b/modules/gdscript/gdscript_functions.cpp index 12e18e28bb..166dcc47fa 100644 --- a/modules/gdscript/gdscript_functions.cpp +++ b/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; diff --git a/modules/gdscript/gdscript_functions.h b/modules/gdscript/gdscript_functions.h index c3b189fca0..868931680d 100644 --- a/modules/gdscript/gdscript_functions.h +++ b/modules/gdscript/gdscript_functions.h @@ -126,6 +126,7 @@ public: INSTANCE_FROM_ID, LEN, IS_INSTANCE_VALID, + DEEP_EQUAL, FUNC_MAX };