-
-
Notifications
You must be signed in to change notification settings - Fork 22.8k
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
base: master
Are you sure you want to change the base?
Conversation
use_shared_copy
for Resource
I'm concerned that this will create too many similar but not-quite-the-same options for resource sharing. We already have |
@JoNax97
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).
You are right about this, although they are in fact functionally different now. Another thought is we can replace
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 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. |
Maybe property should be called Copy (or Duplicate) with 2 options Shared and Full. With Shared as default? |
This comment was marked as duplicate.
This comment was marked as duplicate.
Imo even these two are quite confusing, |
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:
Related proposals:
godotengine/godot-proposals#4672
godotengine/godot-proposals#9603
godotengine/godot-proposals#11312
Keynotes (from proposals)
Conclusion (mine)
Solution
Implement property
use_shared_copy
inResource
So I come up with this PR, acts as a new proposal and proof of concept:
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 affectResource::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):
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:
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 aAnimationPlayer
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. NamelyConcavePolygonShape3D
,ConvexPolygonShape3D
,HeightMapShape3D
will default to use shared copy, the others uses full copy.