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

Node / Resource duplication usability improvements #9603

Open
reduz opened this issue Apr 26, 2024 · 22 comments
Open

Node / Resource duplication usability improvements #9603

reduz opened this issue Apr 26, 2024 · 22 comments

Comments

@reduz
Copy link
Member

reduz commented Apr 26, 2024

Describe the project you are working on

Godot

Describe the problem or limitation you are having in your project

Duplicating nodes in Godot can be really confusing:

  • It is not clear when internal subresources are duplicated or remain shared.
  • Even after duplication, when loading a scene, telling the amount of users of a subresource is hard.

Just choosing whether to share or copy internal resources (like in Blender) is not enough for Godot. Some resources (like Meshes or materials) you never want copied. Others (like particle parameters) it may depend. Animations, when making a duplicate, most of the time you want them copied, but not always.

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

This proposal describes a set of usability improvements to make duplication of internal resources more user friendly.

  • A pop up (toaster like) that shows what is being duplicated.
  • A way to see who else is using a sub-resource in the current scene.

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

Duplication sharing control

Every time the user duplicates something (ctrl-d), a small toaster like popup will appear at the bottom left corner of the viewport:

image

The popup shows:

  • The types of internal resources that are part of the duplication process that are being duplicated as par as the duplication action.
  • By default, some resources that you should never copy (like meshes) will be grayed out to show the users these will always be shared (if you really want to duplicate something like this, do it from the resource inspector).
  • Other resources work depending on their default behavior, showing shared or copy.
  • Remember external resources are not even tracked here (textures, etc) this is only for internal ones.

Changing these values will call undo and then redo the duplication automatically with the new options. These options are remembered while the editor is open.

Shared resource usage tracking

Community asked several times why Godot can't show an amount of "users" in a resource. The answer is complicated: The resource is just a reference counter and it may be referenced from other loaded scenes, or even exist in the undo-redo stack, clipboard, etc. Due to this, estimating the number of users is hard (the reference count is misleading).

To do this properly, we will do as follows:

  • For internal resources that are part of the scene will be shown by the inspector.
  • This will only be implemented for resources that belong to a node, as others are impossible to track. In practice, this solves far most users issues (As this is what users want duplicated).
  • When the scene changes (nodes are added / removed) or when the undo/redo modifies a property in a node, the editor will scan the current scene to track a map of resources and which nodes uses them (likely something as a HashMap<ObjectID,List>). The code in this PR already makes it easy to access properties only that are resource containers, so we may want to merge it before.

With this information, resource properties of nodes would display like this:

image

(ignore the fact this is a texture, its just a mock-up).

Clicking the number will show the other nodes in the scene tree currently using this resource.

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?

This is core

@reduz reduz changed the title Node duplication usability improvements Node / Resource duplication usability improvements Apr 26, 2024
@WolfgangSenff
Copy link

I don't know if it makes sense here, but I noticed there's a GlobalScope value of name PROPERTY_USAGE_ALWAYS_DUPLICATE. It doesn't appear, as far as I can tell, that this is usable (in any useful sense) anywhere actually from GDScript. Would it be possible to add it as part of an export annotation? Such a thing would actually improve the usability here some as well, I believe - creators of a resource could designate other exported resources as ALWAYS_DUPLICATE and not have to worry about it when hitting ctrl + D, same goes for Resources exported on Nodes.

@reduz
Copy link
Member Author

reduz commented Apr 26, 2024

@WolfgangSenff I think we never really exposed usage flags to GDScript, may be a good idea to do so.

@WolfgangSenff
Copy link

@WolfgangSenff I think we never really exposed usage flags to GDScript, may be a good idea to do so.

If you did, then it should also simplify the dialog proposed, because then ones which are always dupe can be grayed out or even ignored as well, giving users fewer ways to mess up when doing the copying.

The reason this is even relevant is we ran into it this past week in the Godot Wild Jam, when we designed everything around resources, but then when we had a game over, everything had been duplicated incorrectly and not very easy to reset that. I did find a way, but it was ugly.

@PLyczkowski
Copy link

My thought on the UX:

This is similar to Blender's 'last operation settings' popup, which in turn is a solution on how to deliver these kind of one-off operation settings without a full-blown interaction-blocking popup that requires confirmation.

For this to be user-friendly I think the popup shouldn't be time-limited (since it's not user-friendly to have time-limited gui elements that disappear on the user), but also shouldn't stay on and take up viewport space until another operation is made. I guess the popup folding on viewport navigation, and going away on first operation (the kind that goes into History) would be ok.

My main problem with this is that Viewport isn't always visible when you duplicate (you can also duplicate in script view), and there isn't really a good alternative screen area that is ok to block.

I still think a full-blown blocking popup with a confirmation button, enabled by default and with an option to disable it in the settings is the cleanest solution. It does introduce an additional step to duplicating, but in turn it would teach new users how duplicating works, and allow power user to duplicate without it and handle making resources unique after that manually.

@lander-vr
Copy link

lander-vr commented Apr 26, 2024

Are there other potential operations where this same blender-style operator popup window could be useful? From a consistency standpoint in UI and UX, it seems a bit weird to me to add a unique UI element disconnected from the rest of the UI in the engine just for one operation.

I'm a fan of this proposal, but I think the popup would make more sense if it serves a purpose beyond just this unique use-case, and is a recurring element seen in other operations too.

To add some further context to Blenders version of this window:
Every operation in Blender is practically built around that popup and accompanied by that popup. It provides various context driven settings to adjust, even with very rudimentary operations such as move, rotate, or scale. It offers an additional opportunity for users to fine-tune or adjust whatever they are doing, and allows the developer to add a variety of additional operator settings without cluttering other parts of the UI permanently, or requiring the user to rely on extensive shortcuts.
It's extremely consistent and predictable for users, which I think is important to take into account.

@Lateasusual
Copy link

I think the tracking for which reference counts "count" that's proposed is good, but I wonder if that could also include things like preload() or other serialized references to the resource (nested references in other resources)? I suppose that's something that could be added later without changing the design much.

For the UX side of the proposal, I think taking a vaguely similar approach to how Blender does it might work well, where some defaults can be set for which referenced resources should be duplicated and which should be "linked" between copied objects:
image
And then to have the regular Duplicate follow those rules (without a popup), and a separate Duplicate (linked?) operator that gives the modal popup to select which resources to copy

@passivestar
Copy link

Are there other potential operations where this same blender-style operator popup window could be useful? From a consistency standpoint in UI and UX, it seems a bit weird to me to add a unique UI element disconnected from the rest of the UI in the engine just for one operation.

This

I still think a full-blown blocking popup with a confirmation button, enabled by default and with an option to disable it in the settings is the cleanest solution.

Please no. Godot is already way too heavy on blocking popups, if anything godot needs less of them, not more. It's just inefficient

Also speaking of the original proposal, whats gonna happen when there are a lot of internal resources on the node? A scrollbar in the popup?

@amarraff
Copy link

amarraff commented Apr 26, 2024

This sounds like a great improvement. I'm not quite sure if it would solve my main problem with Godot's duplication, so I'll clarify - My main desire from a change like this is that I won't have to click "Make Unique" on every duplicate scene to prevent them from sharing the same position data, inspector settings, etc. Duplicating a scene should function the same as adding a scene to the current one twice. Two separate instances of the same scene, instead of two that are linked together. This functionality is significantly better in situations where you want to quickly add multiple instances of the same scene via CTRL + D, but don't want them to all inherit from the first one.

I describe an issue I ran into with the default "shared" behavior in #317 here.

If this change would fix these issues, I'm all for it. Sounds great, except for the popup window. I think the methods suggested throughout #317 could be better. For example, have regular "copy" duplication as the default CTRL + D, with another option in the context menu for creating a "shared" duplication and its own keyboard shortcut. A section in the Godot Docs explaining the difference would be sufficient, instead of a popup window.

@KoBeWi
Copy link
Member

KoBeWi commented Apr 26, 2024

btw internal resource usages are already tracked by scene tree dock:
image

Would it be possible to add it as part of an export annotation?

You can use the new @export_custom() for that.
And even before that there was _get_property_list() and _validate_property().

@reduz
Copy link
Member Author

reduz commented Apr 26, 2024

@PLyczkowski

Yes it should remain there until the next operation you do or until you close it. Being a toaster like popup will make it appear (non blocking) no matter where you are.

@reduz
Copy link
Member Author

reduz commented Apr 26, 2024

@passivestar

I am not aware of other operations that could use it, but if this is the best way we can think to implement this, it does not really need other operations to exist to do it.

@passivestar
Copy link

passivestar commented Apr 26, 2024

I am not aware of other operations that could use it, but if this is the best way we can think to implement this, it does not really need other operations to exist to do it.

I can think of a lot of ways to use it in custom editor tools so a public API for it will be greatly appreciated

@reduz
Copy link
Member Author

reduz commented Apr 26, 2024

@passivestar oh for sure, there should be no problem with this

@mournguard
Copy link

Great proposal overall but I really, really dislike the idea of assuming anything should "never be duplicated". There are tons of reasons to duplicate materials, and anyone doing mesh generation might want to duplicate meshes as well. I understand the uses might be less frequent, but to me it feels highly opinionated to arbitrarily have different behavior for different elements.

@jknightdoeswork
Copy link

jknightdoeswork commented Apr 26, 2024

The core problem is that duplication shares subresources by default. A secondary problem is that it is hard for users of Godot to find and fix the shared references to subresources. Identifying a subresource is difficult. Users must seek out the "resource path" deep in the Inspector, and then remember it. The proposed solution mostly addresses the 'secondary problem' and it does not elegantly solve the core problem. The core problem is that Sub resources can be shared, at all. External Resources are easily identified and intuitively shared. The core solution is to make Subresources fully copy on Copy/Paste. A more elegant improvement to the secondary problem of visibility is to use the Sub Resource ID in the 'name' of the subresource, ie: Process Material 16. I dont think having the usages of the subresource be so 'front and center' like that is intuitive or elegant. Instead, if we must retain Reference Semantics on sub resources, sub resources should have a View Owners popup to see all usages, which shows NodePaths and resource paths of who references this resource. It is basically a disaster to have External Resource reference a scenes' SubResource, but I have seen it happen. This is a bizarre, and mistaken issue that occasionally does happen.

Here is my proposed solution:

  • Sub resources should use Value Semantics in the Godot Editor, namely, full duplication on Copy/Paste
  • If users want to share a Resource across Nodes/other Resources, they are taught to Save it as an External Rresource
  • This makes the Resource clearly differentiated and identified
  • Passing around a Reference to a Resource in code continues to use Reference Semantics (ie: its shared, not copied)
  • The only way, by default, to share a Subresource is to assign it's reference in code, default workflows in the editor always do a full copy
  • If the people still want Reference Semantics on Subresources (I dont think we need it at all) then a Copy Reference option can be added to the Right Click menu for SubResources
  • External resources should use Reference Semantics by default in the Godot Edtior namely, sharing the resource on Copy/Paste (ie: this is how it works now)
  • Sub resources have a View Owners context menu that is accessible by right clicking on the Sub Resource, that mirrors the View Owners on external resources
  • Sub Resource View Owners popup shows NodePaths of usages

TL;DR
Subresources should basically never be shared across nodes/other resources
External Resources are shared across nodes/other resources

I agree with other posters that the proposed solution sounds in-elegant and unintuitive. I cant see the proposed solution working very well.

@CarpenterBlue
Copy link

I am way more excited about the toaster thingy than the proposal itself but both are very welcome changes.
The toaster thing could be used for stuff like adding nodes into scene too.
For example when you create new Staticbody, It could automatically add collision shape and the toaster undo that.

@TAGames
Copy link

TAGames commented Apr 28, 2024

Currently I don't know what resources or nodes get referenced and which get copied. It took my a while until I found out that a Marker3D is referenced and not a copy:
https://forum.godotengine.org/t/duplicating-inherited-scenes-weird-behaviour/57566/3
If I remember correctly in Unity most of the gameobjects were copies, except materials (and maybe a few other things I can't remember). This felt comprehensible.

@AdriaandeJongh
Copy link

Would it be possible to put the usage number of resources (currently shown in the mockup at the end of the variable-object-row in the inspector) somewhere else? Especially as the inspector is already fairly cramped together, and regularly resized to make room for the scene view, having another item in that row that is not necessary to be visible at all times only leads to more clutter.

@jasonwinterpixel
Copy link

Would appreciate if someone from Godot could comment on my suggestion that sub resources should default to full copy on copy/paste/duplicate.

I think the positive reception to Juan's suggestion is from people being happy that Godot team is simply acknowledging this core problem, but the proposal as suggested won't improve alleviate the "duplicate then go and make a bunch of stuff unique" workflow. It won't improve the lack of intuition around copy/paste. My suggestion, imo, will solve the problem at a deeper level by eliminating the need for the user to be constantly aware of the shared nature of subresources.

@Calinou
Copy link
Member

Calinou commented May 6, 2024

Would it be possible to put the usage number of resources (currently shown in the mockup at the end of the variable-object-row in the inspector) somewhere else?

This has been discussed previously, but it's difficult to implement: #904 (comment)

Would appreciate if someone from Godot could comment on my suggestion that sub resources should default to full copy on copy/paste/duplicate.

For memory usage and draw call reasons, we can't do this on all resource types, particularly textures, meshes, audio samples and materials1. Even convex/concave collision shapes can be heavy (unlike primitive collision shapes).

This could only be performed on certain resource types that don't require much memory.

Footnotes

  1. If a material is marked as unique, it'll always cause a new draw call even if it's strictly identical to another material.

@jasonwinterpixel
Copy link

jasonwinterpixel commented May 7, 2024

For memory usage and draw call reasons, we can't do this on all resource types, particularly textures, meshes, audio samples and materials1. Even convex/concave collision shapes can be heavy (unlike primitive collision shapes).

I appreciate the response.

Textures and Audio Samples are basically always External Resources, not Sub Resources. Meshes are occasionally sub resources when the user creates them using the built in the Primitive Meshes. These basic meshes do not require much memory, of course.

For Materials, they do appear in the form of a Sub Resource, but then they are typically not shared. it is a best practise to use External Resources to manage sharing Materials. Godot should guide users towards this pattern. This pattern is probably closer to what users would intuit.

Imagine a large project that used Materials saved as sub resources to maintain low draw calls. When a user wants to add a new object, they are told "You must find a random usage of the material you wanna use in a scene somewhere and copy it, then paste it onto your new object"? That is an unscalable workflow.

I am suggesting that Materials that are being shared as subresources are simply shared by chance, the user is not optimizing for draw calls, the project is likely amateur and draw calls will not manifest as a bottleneck. Btw, in my testing, draw calls are not that important anymore. Any project that cares about draw calls is going to need to organize their materials. Organising sub resources is impossible. The materials in question will be saved as External Resources, and if they are saved as Sub Resources, it's because they werent important.

I think this is a rule in general, if you want to share a resource, use External Resources, ie: save it with a filename. It is far too cumbersome to manage shared references to SubResources, and this Proposal merely scratches the surface of the difficulty in managing shared references to Sub Resources.

On our team sharing sub resources at all is a liability. Sub Resources basically only ever have 1 usage. I am convinced that this is the way, and I'm here to advocate and represent that position.

@lander-vr
Copy link

I think this is a rule in general, if you want to share a resource, use External Resources, ie: save it with a filename. It is far too cumbersome to manage shared references to SubResources, and this Proposal merely scratches the surface of the difficulty in managing shared references to Sub Resources.

On our team sharing sub resources at all is a liability. Sub Resources basically only ever have 1 usage. I am convinced that this is the way, and I'm here to advocate and represent that position.

I'd like to voice my support for this too. This is a much much more intuitive way of going about resource sharing, removing the ambiguity that's often surrounding this behavior right now. Relying on external resources is also precedented by how other engines handle resources, making this behavior tried and tested and making this immediately more intuitive for developers and artists coming from those engines.

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