Skip to content

Modify Dictionary::operator== to do real key/value comparison

Created by: touilleMan

Here is my attempt to fix the current infamous dictionary comparison behavior (see #27615)

Basically the issue is currently two Dictionary object only considered equal if they point on the same underlying hashmap. This is very error prone given this behavior leaks into GDScript:

var a = {}
var b = {}
var c = a
print(a == b)  # false
print(a == c)  # true

There is already PR #29222 that tries to fix this by modifying Variant comparison. However I believe this is not the right way to go given it only fix GDScript, but not GDNative !

Currently GDNative's godot_dictionary_operator_equal is pretty useless (I would even say it is extremely error prone, I've personally been bitten multiple time by this function) and so GDNative users has to implement their own comparison (see https://github.com/touilleMan/godot-python/blob/72df9d1b38120aeec3c702004e14dcb82e95bde5/tools/builtins_templates/dictionary.tmpl.pxi#L180-L185) which is both cumbersome and inefficient.

Given I couldn't really figure out the usecase for this underlying-hashmap-only comparison, I've recompiled a modified version of Godot with Dictionary::operator== and Dictionary::operator!= removed (see https://github.com/touilleMan/godot/commit/2409aa62bed81be948e3efbc99ae3c578fda6502). The only place those operator are used are in variant_op.cpp in the OP_EQUAL and OP_NOT_EQUAL operators. So I guess I can officially say the current implementation of Dictionary::operator== is useless \o/

Of course, I think we should do the same for Array::operator==, I'll update this PR with those change if we agree on the idea ;-)

Merge request reports

Loading