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) doesn't duplicate subresources in Array or Dictionary #32

Closed
limbonaut opened this issue Mar 14, 2023 · 3 comments
Labels
bug Something isn't working

Comments

@limbonaut
Copy link

Related to #4.

Because of this behavior, set_point_array(make_unique=true) is not working as expected. SS2D_Point resources are not duplicated, when SS2D_Point_Array is duplicated. Only dictionary that holds them is duplicated, preserving SS2D_Point references that are stored as dictionary values.

This is a part of deeper design issue involving copy-pasting of shapes, meta shapes and sharing/not sharing shape properties. At this moment, copy-paste of a shape produces a shape copy in a state that is not easily fixable via UI.

@limbonaut limbonaut added the bug Something isn't working label Mar 14, 2023
@mphe
Copy link
Owner

mphe commented Mar 14, 2023

So, the engine internal implementation of Resource does not pass the deep copy parameter of duplicate to the duplicate call of its array/dictionary properties?
That sounds like an engine bug.

@limbonaut
Copy link
Author

limbonaut commented Mar 14, 2023

And I found the root of the problem:
https://github.com/godotengine/godot/blob/fc7adaab7b3856a7831d402ea2bbb27efe7b7d8a/core/variant/variant_setget.cpp#L1889-L1900

So, the engine internal implementation of Resource does not pass the deep copy parameter of duplicate to the duplicate call of its array/dictionary properties?

It passes, they don't act on it as expected it seems. Only non-Object Variant values are deep-copied.

@mphe
Copy link
Owner

mphe commented Mar 14, 2023

What the hell. I found a related issue here: godotengine/godot#37162. The documentation could really be more clear on this.

Alright, so we need to add duplicate back, at least in some classes, under a different name to not override the internal duplicate, manually deep copy affected properties, and then adapt other code to use the custom duplicate function.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants