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

Allow Arrays and Dictionaries to be duplicated or made anew via the inspector #15368

Closed

Conversation

bojidar-bg
Copy link
Contributor

Closes #13971

I couldn't make the "Changes may be lost" button appearing when the Array comes from another scene though.

image

@bojidar-bg bojidar-bg added this to the 3.0 milestone Jan 5, 2018
@Geequlim
Copy link
Contributor

Geequlim commented Jan 6, 2018

@bojidar-bg Any idea with #13346

@kubecz3k
Copy link
Contributor

kubecz3k commented Jan 6, 2018

I tested the PR and I'm sorry to say, but I'm still able to observe the issue

@kubecz3k
Copy link
Contributor

kubecz3k commented Jan 6, 2018

Oh I probably misunderstood how the fix works. I assume I should choose 'new array' after creating the scene, in such case it works fine!

@kubecz3k
Copy link
Contributor

kubecz3k commented Jan 6, 2018

But I don't quite understand use-case for those options, are there any cases when I will not want for my array to be separate from the one that is possessed by parent? (pre 3.0 they were automatically cloned I assume)

@bojidar-bg
Copy link
Contributor Author

@Geequlim No, that other issue appears unrelated to the one solved. I don't know how it should be solved.
@kubecz3k Well, you don't want it to be a separate array in case you are not making changes to it.
Pre 3.0 they might have been saved even if not modified, which would lead to some sort of auto-cloning.

@kubecz3k
Copy link
Contributor

kubecz3k commented Jan 8, 2018

Is there any way to detect if the array is separated or not? I assume the best work-flow would be to duplicate the array automatically at the time of the first modification since no other inspector element need that kind of additional action. Also as far as I understand it's impossible for user to tell if the array is already 'independent' from parent scene or not (so it's hard to tell when it should be duplicated).

@bojidar-bg
Copy link
Contributor Author

Well, I tried to think of some simple way, but couldn't. Maybe for 3.1.

@bojidar-bg
Copy link
Contributor Author

After discussion on IRC, it was decided that we can make a more proper workaround.

<reduz> bojidar_bg: we could force duplicating the property in PackedScene when instancing for editing
<reduz> bojidar_bg: and also force duplicating the property on editor duplicate
<reduz> bojidar_bg: i think that may fix the problem
<bojidar_bg> yes, it would fix it as well
[..]
<reduz> bojidar_bg: I suggested adding a deep copy argument to duplicate() in Array and Dictionary, we can call that in Node::duplicate and PackedScene::instance (when used from editor)

I will open another PR when I implement it, but it is likely to land post-3.0.

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

Successfully merging this pull request may close these issues.

3 participants