Godot: Dictionary identity is broken for NAN

Created on 24 Jan 2018  路  4Comments  路  Source: godotengine/godot

Godot version:
3.0rc2

OS/device including version:
any

Issue description:
E.g. in GDScript (but actually affects underlying Dictionary in Godot implementation, so not related to GDScript):

    var d = {}
    d[Vector2(NAN, NAN)] = 0
    d[Vector2(NAN, NAN)] = 0
    print(d.size())

Outputs 2.

The same happens for any floating point type that can contain nans (i.e. float, Vector2, Vector3, etc.).

What happens here is that the hash for Vector2(NAN, NAN) is always the same, but the comparator function that runs in dict to actually check item equality always returns false, as NAN != NAN.

Steps to reproduce:
See above.

Minimal reproduction project:
See above.

bug core

Most helpful comment

I disagree with not allowing NaNs as dictionary keys as this could lead to hard to debug crashes for exported games. In any case where this could happen, which could be almost any fp operation, it would then be up to the game author to add NaN checks before any dictionary insertion. Nobody is going to do that.

Since dictionary/list indexing is not a mathematical operation, but rather a symbolic one, I don't see why disallowing NaN makes much sense. This is just an implementation detail of this particular possible symbol.

All 4 comments

CC @hpvb, as you wrote #7815 / #8393.

https://stackoverflow.com/questions/6441857/nans-as-key-in-dictionaries

" [...] it seems a saner idea to avoid NaN as a dictionary key."

It seems that Dictionaries don't use the correct comparator function. This should be an easy fix.

I disagree with not allowing NaNs as dictionary keys as this could lead to hard to debug crashes for exported games. In any case where this could happen, which could be almost any fp operation, it would then be up to the game author to add NaN checks before any dictionary insertion. Nobody is going to do that.

Since dictionary/list indexing is not a mathematical operation, but rather a symbolic one, I don't see why disallowing NaN makes much sense. This is just an implementation detail of this particular possible symbol.

Was this page helpful?
0 / 5 - 0 ratings