Skip to content

Implement use_shared_copy for Resource #107570

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

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

daniel080400
Copy link
Contributor

@daniel080400 daniel080400 commented Jun 15, 2025

This PR only affects internal resources, external resources (saved to .res, tres, or in .tscn) will stay shared (if local_to_scene is false).

TL;DR After this PR you can do this in Godot by default:

pr_vid_1

Related proposals:

godotengine/godot-proposals#4672
godotengine/godot-proposals#9603
godotengine/godot-proposals#11312

Keynotes (from proposals)

  • We cannot determine if a resource should use a full copy or shared copy solely by its type. (eg: Material)
  • If forcing internal resources to use full copy every time, it might unintentionally cause bloat and performance issues.
  • Adding pop-ups every time a node is duplicated for users adjust the resource copy is not good for UX, and some people is against it.

Conclusion (mine)

  • The solution needs to provide users the option to determine if a full copy is needed per resource, not per class.
  • The solution also has to provide reasonable defaults that fits most of the use cases, so user don't need to adjust resource copy behavior frequently.
  • External resources should always stay shared after node duplication
  • As in those proposals we didn't come to an common agreement, I'd say we can at least have a simple solution, which works well for now, also being easy to refactor, and keep discussing our "perfect solution" later.

Solution

Implement property use_shared_copy in Resource

So I come up with this PR, acts as a new proposal and proof of concept:

pr_img_1

Defaults to true (use shared copy) for most resources, and defaults to false (use full copy) where reasonable. If user wants a specific resource to behave differently on copy, they can adjust it freely.

This boolean is only taken into account at Node::duplicate(), it doesn't affect Resource::duplicate(). It's main purpose is to improve UX when using editor duplicate tool.

It also doesn't conflict with Resource::local_to_scene. It only accounts for node duplication in the current local scene.

Resources that defaults to full copy

Whether a resource should default to full copy, we need to consider the following (top priority to lowest):

  1. If user DOES NOT notice it's shared, how likely will he lose work because of it?
  2. If user DOES notice it's shared, will it take a lot of time by managing it?
  3. If user DOES NOT touch the default behavior at all, will it possibly cause too much bloat?

If a developer cares much about optimization (point 3), they will likely discover this option actively by themselves. But most devs do not care and will not know the resource is shared or not unless they run into problems, worst of all, those problems are mostly related to lose of work and time (point 1), this could drive users away from Godot, it's serious.

Here's the list of resource classes that defaults to full copy, IMO:

// Internal resources that uses full copy by default

Animation
AnimationLibrary

BoxShape3D
CapsuleShape3D
CylinderShape3D
SphereShape3D
SeparationRayShape3D
WorldBoundaryShape3D

Shape2D (All derived classes)

CameraAttributes (All derived classes)

Curve
Curve2D
Curve3D

Environment

Noise (All derived classes)

Gradient

LabelSettings

MultiMesh

OccluderPolygon2D

Some resources have very even pros and cons when deciding to make it default to full-copy on node duplication, I'll write my decision down here and open for discussion:

  • Material (shared copy): Very likely to run into draw call performance hit if using full copy every time, and possibly time-consuming to manually set all nodes to use the same Material again.

  • Animation, AnimationLibrary (full copy): The most possible scenario where this will be copied is duplicating a AnimationPlayer node, and the reason for copy is most likely -- It's an animation created using Godot, the user wants a full copy of the animation and modify it. Another common scenario is The user wants to share animation to multiple similar skeletons, which is a more advanced usage, I consider the user doing this is more likely to think of sharing resources, and even if they don't, it'll still work. We should make this default to full copy for novice users that only wants to animate something in Godot.

  • Shape3D, Shape2D (depends on shape): Whether to use full copy by default depends on how likely the shape is going to be tweaked by hand, and performance costs. Namely ConcavePolygonShape3D, ConvexPolygonShape3D, HeightMapShape3D will default to use shared copy, the others uses full copy.

@daniel080400 daniel080400 changed the title Implement use_shared_copy for Resource Implement use_shared_copy for Resource Jun 15, 2025
@JoNax97
Copy link
Contributor

JoNax97 commented Jun 16, 2025

I'm concerned that this will create too many similar but not-quite-the-same options for resource sharing.

We already have local to scene and make unique. These are different enough that most people can grasp them without much trouble. If we add this new shared copy option, I feel new users will have a hard time understanding the nuances and when to use which one.

@daniel080400
Copy link
Contributor Author

@JoNax97
Looking at the proposals I mentioned, if we want a solution that even new users can understand at first glance, it will likely be this: godotengine/godot-proposals#9603, but after reading the conversation you can see, it'll run into other UX issues:

  1. The node could be duplicated while in Script view mode
  2. Duplicate is used so often that the window will virtually be pinned there

Then a full-blown pop-up window for every node duplication is suggested, but people disagree with that for good reasons.

So I stick with godotengine/godot-proposals#4672, which suggested we should have good defaults when copying resources (shared or full copy).


I'm concerned that this will create too many similar but not-quite-the-same options for resource sharing.

You are right about this, although they are in fact functionally different now. Another thought is we can replace local_to_scene by shared_copy, and we leave only this option. Then shared_copy can be used both in node duplication and scene instantiation.

If we add this new shared copy option, I feel new users will have a hard time understanding the nuances and when to use which one.

I disagree with this.

Most people that discuss this UX issue should have moderate experience with Godot engine, and we might forget the time when we are new users to Godot. I personally, looked into the option local_to_scene only after I run into issues.

If we have good defaults, I don't think it'll confuse new users, as they will just leave it be if there's no problem about it.

@seppoday
Copy link

seppoday commented Jun 16, 2025

Maybe property should be called Copy (or Duplicate) with 2 options Shared and Full. With Shared as default?

@daniel080400

This comment was marked as duplicate.

@SatLess
Copy link
Contributor

SatLess commented Jun 28, 2025

We already have local to scene and make unique. These are different enough that most people can grasp them without much trouble. If we add this new shared copy option, I feel new users will have a hard time understanding the nuances and when to use which one.

Imo even these two are quite confusing, local_to_scene is more used for runtime instances, but also works for editor instances. At the end of the day, they end up doing the same thing. I think a preferable option would be to unify these options, but it's far out of scope for this PR

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.

5 participants