godot: Node `duplicate()` doesn't copy internal variables values

Also my view of this issue has not changed, we should either document it in documentation of duplicate() method or fix it. If we decide to not fix it we at least should ensure it’s possible to overwrite duplicate() from gdscript and provide your own implementation there (and it also should be documented in such case)

duplicate() for nodes is hackish, and not generally recommended. It’s there because some people insists it can be of use, because otherwise i would have removed it long ago. Using preload().instance() is always the better choice.

The reason is that some nodes create subnodes (ie, a list creates it’s scrollbars), so a simple duplicate() function has to be able to tell to a certain degree what to duplicate and what not to duplicate. This is not always obvious.

Maybe the best it would be to document what the function can actually does, or just add more flexibility to it, not really sure what you guys prefer.

On Mon, Jun 12, 2017 at 11:02 AM, eon-s notifications@github.com wrote:

@bojidar-bg https://github.com/bojidar-bg I prefer selective no-storage than having to manually set everything for storage all the time (which may be more error prone), but not sure of the keyword for that.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/godotengine/godot/issues/3393#issuecomment-307798657, or mute the thread https://github.com/notifications/unsubscribe-auth/AF-Z290qBZIy62N7XjfODdHQDbHRhC9Xks5sDUT1gaJpZM4HIicK .

I second the notion that .duplicate() absolutely has many use cases and is a much quicker and easier solution than instancing preloaded scenes.

But it would need an improvement to also duplicate children. Or, create an official clone() method which does that, leaving duplicate to make only a shallow copy as it does right now.

It is still misleading name in my opinion.

Duplicate mean “duplicate” for me. If I wanted another instance of same class I would just make that instead, even if duplication would be very heavy operation, or loop indefinitely or it would cause some flaw - it’s my problem - it still should duplicate 1:1 untill it crashes or finishes this operation.

I read docs on duplicate() and DuplicateFlags multiple times, in description there is note, but it don’t mention fact that internal states aren’t duplicated and it is counterintuitive for me. I’m on older version, but in online docs there is also no mention about it.

Perhaps in other languages cloning or duplicates are meant to be shallow copies - but I never wrote in high level programing language and for my primitive mind meaning is clear - it duplicates object.

Edit : on the other hand - what you can do is overwrite duplicate() to copy some selected variables.

I was searching for a bug in my game for quite some time, until I saw this thread and noticed that it wasn’t a bug at all…

I find the way it is currently done unintuitive. The way it is right now, get_node(S).duplicate() and preload(S).instance() are the same thing. That shouldn’t be the case. Every other attribute of get_node(S) points to the instance of that node, except for duplicate, which points to the un-instance class? I’ve never seen such a design.

And it’s not about shallow vs. deep copy. A “shallow copy” B of an object A (which is an instance of the un-instanced class C) in most C-like languages means that the member variables of B point to the values of the instance A, not to a copy of the variables of C. If I wanted a new object with the values I hard-coded into C, I simply would make a new instance of C. Which is the equivalent of calling ‘preload(S).instance()’ in gdscript, at least that’s what it seems to me.

I would suggest to change the API to the following:

N.duplicate(use_instancing=false, deep_duplicate=true)

Makes a copy of the instance of N. If deep_duplicate is true, all values get copied and get a new place in memory. If it is false, the values are shared. However, I’ve never seen this anywhere else in godot, so maybe this shouldn’t be an option at all. GDscript exists only for developers to make it as easy as possible, and to think about pointers and instancing would make it hard for newcomers, I think.

Anyway, I would call the method that does what your duplicate method currently does differently. I don’t know exactly how your systems work, but from what I know so far, this would make sense:

preload(S) # Returns an un-instanced class
load(S) # This too
File # This is an uninstanced class as well

# To instance a class, call "instance()"

var t = preload(S).instance()
var k = load(S).instance()

f = File.instance() # Instead of new(), for consistency 
f.load(xyz)

# And to do this what duplicate currently does:
t.new_instance()
# or
t.get_root_class().instance()  # Bad name, but it's about the idea

# This could be used to spawn multiple new nodes of t:

t_root_class = t.get_packed_scene()
for i in range(10):
   add_child(t_root_class.instance())

@reduz Well I don’t know this as well… I’m coming from java world, where default ‘clone’ method is shallow copy (copying all the values and references), if developer want different behaviour then he is implementing his own clone() method. Don’t really know how it’s in python world, but as far as I remember it’s basically the same (and I think gdscript should follow python convention). For me personally as long as it’s possible to provide your own ‘duplicate’ method implementation it’s fine, but I would like to know what is the default intended behaviour (would be good idea to have this in docs).

I had the same issue. I had a card game where I wanted to buff some cards and duplicate them. It was frustrating to know that the only way to duplicate the internal states was to manually create a dictionary and copy each variable over to the new card instance. It would be much better to just do card_instance.duplicate() if it also copies the buffs on them.

BUT LUCKILY, I found a solution for me that worked. JUST add @export before the variables you want to maintain the state. Then when you do card_instance.duplicate() you actually ALSO duplicate the variable values.

Thanks @akien-mga, in that case maybe duplicate() doesn’t really have any valid use cases and should be documented with a warning. There was nothing in the docs to indicate it’s not recommended for use.

Cant attach Godot4 reproduction by editing first post (some unknown error), attaching the Godot4 reproduction here: DuplicateTestGd4.zip (The issue is still the same as it was before)

Also to clarify: the issue is about usage of Object.duplicate() method and the fact that variables of returned object (even simple types) are different then in the original one.

Also I think that shallow copy is not necessary the best solution since internal node references will point to nodes in original scene… I would solve this by checking if reference is pointing to the node that is on the lower scene level or not (lower from the point of duplicate root are duplicated with deep copy). When it comes to value variables like int or String the situation for me is clear (I think they should be copied).