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

Make Subresources Unique should affect nested sub resources #2737

Closed
jasonwinterpixel opened this issue May 17, 2021 · 8 comments
Closed

Make Subresources Unique should affect nested sub resources #2737

jasonwinterpixel opened this issue May 17, 2021 · 8 comments
Milestone

Comments

@jasonwinterpixel
Copy link

jasonwinterpixel commented May 17, 2021

Describe the project you are working on

Tank Kings, a fast paced action game with fully server authoritative multiplayer and buttery smooth client side prediction.

Describe the problem or limitation you are having in your project

Duplicating Nodes with Nested Resources often leads to unintended sharing of sub resources because only one layer of Resources are made unique when using Make Sub Resources Unique.

eg) Duplicating a Particle System requires the user to right click -> Make Unique on all the Gradient and Gradient Textures. We often have situations where we accidentally destroy a particle systems' gradients because we didnt realize the gradient or gradient texture was shared between the two nodes.

Describe the feature / enhancement and how it helps to overcome the problem or limitation

Make Sub Resources Unique should make all sub resources unique, including nested ones

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

We should make the Make Subresources Unique feature recursively make all nested subresources unique, as well.

If this enhancement will not be used often, can it be worked around with a few lines of script?

No

Is there a reason why this should be core and not an add-on in the asset library?

There is no use case to retain the current implementation of Make Sub resources Unique which is unintuitive and less useful than this proposal.

@jasonwinterpixel
Copy link
Author

jasonwinterpixel commented May 17, 2021

This (extremely simple) PR is the change that I am advocating for.
We are using this single line of code change on Tank Kings so that our designer can easily duplicate a Particle System without having to tediously make all the Gradient and Gradient Textures unique.

https://github.com/godotengine/godot/pull/33824/files

@Calinou
Copy link
Member

Calinou commented May 17, 2021

See also #317 and #4672.

There are some use cases where it's better to keep resources partially shared, so I think there needs to be a way to opt-out of this proposed behavior somehow. We also need to be careful about cyclic references. I would make the proposed behavior the default/recommended one, still.

@jasonwinterpixel
Copy link
Author

jasonwinterpixel commented May 17, 2021

There are some use cases where it's better to keep resources partially shared, so I think there needs to be a way to opt-out of this proposed behavior somehow.

I think the use cases where a user would not want this behaviour is fully encapsulated by right clicking an individual resource and selecting Make Unique.

As for #317, if we implement this proposal, then duplicating a Node, then going to Make SubResources Unique would make the new node fully unique.

Currently, there is not solid workflow for duplicating a Node and then making it unique (eg particle systems, you must go through all fields to find and make any nested sub resources unique).

If we implement this proposal, perhaps it will help #317

@lukostello
Copy link

I think the use cases where a user would not want this behaviour is fully encapsulated by right clicking an individual resource and selecting Make Unique.

how does that opt them out of the behavior? If the subresources are also now made unique through the recursive make_local then wouldn't it already be unique? Seems redundant. There would need to be good documentation on how to do that if what you're suggesting actually works.

@lukostello
Copy link

lukostello commented Oct 22, 2021

if this is implemented suppose resource C is made local to scene and is a sub resource of resource B which is not made local to scene, and resource B is a property of Scene A then scene A is instantiated twice (A1 and A2) will A1 and A2 share the same B? C?
I think right now they would share both B and C.

@Calinou
Copy link
Member

Calinou commented Dec 12, 2022

@pc9098 Please don't bump issues without contributing significant new information. Use the 👍 reaction button on the first post instead.

@MewPurPur
Copy link

This has been addressed by the implementation of Make Unique Recursive

@Calinou
Copy link
Member

Calinou commented Aug 15, 2023

This was implemented by Make Unique (Recursive) and further refined by godotengine/godot#77855, closing.

@Calinou Calinou closed this as completed Aug 15, 2023
@Calinou Calinou added this to the 4.0 milestone Aug 15, 2023
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

4 participants