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

Resource.duplicate(true) does not duplicate subresources in arrays #82348

Closed
RolandMarchand opened this issue Sep 26, 2023 · 5 comments
Closed

Comments

@RolandMarchand
Copy link

RolandMarchand commented Sep 26, 2023

Godot version

v4.2.dev.custom_build [2048fe5]

System information

Godot v4.2.dev (2048fe5) - Gentoo 2.14 - Wayland - Vulkan (Forward+) - dedicated AMD Radeon RX 6600 (RADV NAVI23) () - AMD Ryzen 5 5600 6-Core Processor (12 Threads)

Issue description

When duplicating Resources with duplicate(), a new Resource with its properties copied over is returned. Though, if there are other Resources as properties, only the references to them are copied, not their contents. Unless the argument true is given to the function, in which case the Resources contained in the new are unique, not referencing the same ones as the original.

This statement is not true when those Resources are contained within an array. Here are screenshots comparing two resources duplicated with duplicate(true) at runtime.

Note: Notice how the subresources are different, duplicated as they should be, but not the elements of array_of_subresources.

Original resource:

Screenshot from 2023-09-25 22-33-47

Duplicated resource:

Screenshot from 2023-09-25 22-33-50

Steps to reproduce

  1. Create a custom resource:
extends Resource
class_name CustomResource

# All the resources this array is containing won't be duplicated when using `duplicate(true)`
@export var array_of_subresources: Array[Resource]

# This resource will be duplicated with `duplicate(true)`
@export var subresource: Resource
  1. Populate its array with any resource, duplicate it with duplicate(true), and compare the two resources from the arrays:
extends Node2D

func _ready():
	var cr := CustomResource.new()
	cr.array_of_subresources = [Resource.new()]
	cr.subresource = Resource.new()
	
	var cr_dup := cr.duplicate(true)
	
	# Success
	assert(cr.subresource != cr_dup.subresource)
	# Failure					
	assert(cr.array_of_subresources[0] != cr_dup.array_of_subresources[0])

Minimal reproduction project

godot.zip

@lyuma
Copy link
Contributor

lyuma commented Sep 26, 2023

Interesting.

I am quite concerned that AnimationLibrary is a resource which holds Animation sub-resources in a Dictionary. Could it have the same issue, or does it only affect arrays.

It would be interesting to test duplicating an AnimationLibrary with Animations inside and see if it possibly has the same issue.

@aviboddu
Copy link

aviboddu commented Sep 26, 2023

Taking a look at the array and dictionary code, it looks like the core issue is that Array::duplicate calls Array::recursive_duplicate, which in turns calls recursive_duplicate for the object.

Seems like that's implemented in the variant_setget.cpp file (no clue why it's there). There, we can see that the object duplication does not occur since it 'breaks stuff'.

(The same issue seems to exist for dictionaries as well)

@bitsawer
Copy link
Member

@lyuma Looks like your concern about AnimationLibrary duplication might have been answered in #82421 if I understood that right. I guess the recursive duplication is currently commented out as mentioned by aviboddu in previous message.

@AnidemDex
Copy link

Is this related to #74918 ?

@Calinou
Copy link
Member

Calinou commented Feb 12, 2024

Thanks for the report! Consolidating in #74918.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants