Dictionary duplicate(deep=true) creates shallow copy

[paulmiller]

Godot version: 3.2

OS/device including version: Arch Linux (i7-6700, GTX 1070, nvidia driver 440.59-7)

Issue description:
Is duplicate(true) intended to make a deep copy? The parameter is undocumented, but if the dictionary contains Objects, it actually makes a shallow copy, like duplicate(false). If this is intended, then the documentation should explain that. (And maybe the parameter should be renamed.) If it's intended to be a deep copy, then the Objects in the dictionary should be copied.

I wonder if d280b14 is related; it commented out the part of Variant::duplicate that checks the deep flag.

Steps to reproduce:
Try this:

class Thing:
  var x
  func _init(_x):
    x = _x

func _ready():
  var t = Thing.new(1)
  var d1 = { 1:t }
  var d2 = d1.duplicate(true)
  print(d1[1] == d2[1])
  t.x = 3
  print(d1[1].x)
  print(d2[1].x)

It prints:

True
3
3

I'd expect it to print:

False
3
1

Minimal reproduction project: Just paste the above code into any project. Nothing else is needed.

[bojidar-bg]

The deep flag was added with #17382. At that time, some code related to duplicating objects was added in commented, just as a note. At the time I decided that the potential for breaking things if the flag duplicated object was too great.

2dc738c uncommented the code and made it duplicate Resources when deep is on, making sure to pass it on and duplicate other resources recursively.

And finally, the commit you mentioned, d280b14 was added on the very next day. It disabled Resource duplication from Variant::duplicate, with the comment that it breaks stuff :(. A suggestion made in that commit was to add one more argument to duplicate, which would control whether objects are duplicated as well.

[paulmiller]

Is the intention to only deep-copy Resources, and shallow-copy other Objects? We should document that it's not currently implemented, or remove the parameter from the GDScript API until it is. The current behavior is very surprising. Or should I say, I was very surprised :)

[bojidar-bg]

I feel that the best solution is to document that the deep parameter affects only the copying of dictionaries and arrays. There are too many object types one would prefer to not have duplicated by mistake, especially some types of Resources.