Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[3.x] Support deep comparison of Array and Dictionary #42625

Merged
merged 2 commits into from
Nov 9, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions core/array.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
1 change: 1 addition & 0 deletions core/array.h
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ class Array {
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;
Expand Down
28 changes: 28 additions & 0 deletions core/dictionary.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
1 change: 1 addition & 0 deletions core/dictionary.h
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ class Dictionary {

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;

Expand Down
6 changes: 5 additions & 1 deletion core/ordered_hash_map.h
Original file line number Diff line number Diff line change
Expand Up @@ -213,8 +213,12 @@ class OrderedHashMap {
(*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);
Expand Down
3 changes: 3 additions & 0 deletions core/typedefs.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
31 changes: 31 additions & 0 deletions core/variant.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
1 change: 1 addition & 0 deletions core/variant.h
Original file line number Diff line number Diff line change
Expand Up @@ -408,6 +408,7 @@ class Variant {

//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;
Expand Down
13 changes: 13 additions & 0 deletions modules/gdscript/doc_classes/@GDScript.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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" />
Expand Down
10 changes: 10 additions & 0 deletions modules/gdscript/gdscript_functions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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];
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
1 change: 1 addition & 0 deletions modules/gdscript/gdscript_functions.h
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ class GDScriptFunctions {
INSTANCE_FROM_ID,
LEN,
IS_INSTANCE_VALID,
DEEP_EQUAL,
FUNC_MAX
};

Expand Down
2 changes: 1 addition & 1 deletion scene/property_utils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

Expand Down