Godot: Erasing an element from a dictionary while iterating fails silently

[Zylann]

Godot 3.0.2
Windows 10 64 bits

I've been debugging a strange behavior in my editor plugin for an hour, until I realize it was due to my tool script:
I was removing an element from a dictionary, while iterating on that same dictionary. It was hard to notice because the place where I was iterating is far from the place where I erase.
As a result, the iteration abruptly skipped all the following keys, but SILENTLY.
I was using the commandline debugger to catch as much errors as possible, but that one went through unnoticed.

Repro:

var d = {"a": 1, "b": 2, "c": 3}
for k in d:
    d.erase(k)
print(d.keys())

Prints:

[b, c]

If Godot doesn't support this without copying all the keys, I expected an error.

[capnm]

Yeah, the pythons are doing it right:
RuntimeError: dictionary changed size during iteration

Someone could at least put a loud DON'T DO THIS note in the docs ;)
http://docs.godotengine.org/en/latest/getting_started/scripting/gdscript/gdscript_basics.html#dictionary
http://docs.godotengine.org/en/latest/getting_started/scripting/gdscript/gdscript_basics.html#for

[vnen]

I wouldn't expect this to work anyway, but it should throw some error. However, I'm not sure if this can be properly detected.

[ghost]

I wouldn't expect this to work anyway

why not? looks valid to me.

the bug should be, why this isn't working.

[capnm]

why not? looks valid to me.

Because if you destroy what you have under your feet, then you can't jump?

[ghost]

Because if you destroy what you have under your feet, then you can't jum

but it's just a key (a part of the dictionary), not "what you have under your feet"

[capnm]

For the sake of efficiency in implementation, a dictionary can not be changed while iterating over it. A serious language designer wouldn't allow that.

Iterating over the key array it works fine. Try it:

var d = {
    "Alle meine": 1,
    "Entchen": 2,
    "schwimmen auf dem": 3,
    "See": 4,
}
print("before ", d)
for k in d.keys():
    d.erase(k)
print("after ", d)

[mhilbrunner]

https://github.com/godotengine/godot/blob/master/core/ordered_hash_map.h (which dictionaries seem to be based on) states:

Deletion during iteration is safe and will preserve the order.

According to reduz, this should probably still be true - so something fishy seems to be going on here.

[vnen]

According to reduz, this should probably still be true - so something fishy seems to be going on here.

This is probably true from the C++ code, where you can iterate as a list of pairs. I don't think GDScript is iterating that way.

[KoBeWi]

Still valid in 07db4d57d

As someone commented, doing for k in d.keys() in the repro code will produce a correct result. So iterating normally should throw an error/warning when the keys are modified and point to use keys().