godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
91.58k stars 21.27k forks source link

Animation resources as references aren't clarified well by the editor at design time #30197

Open nobuyukinyuu opened 5 years ago

nobuyukinyuu commented 5 years ago

Godot version: 3.1.1

Issue description: When instancing a scene which inherits from a scene with an AnimationPlayer, any changes to animations and animation tracks will affect all instances, including deleting animations. This also applies to copied-and-pasted animations and tracks across disparate scenes.

This referencing behavior may be considered unexpected to users, as the default behavior of other node properties is typically local-but-inherited and the Animations' appearance as a reference is obfuscated from the inspector view without explicitly opening it from the animation tab. This behavior also occurs when copying and pasting animations across non-inherited scenes, including the deletion of whole Animations and tracks.

I was meaning to post an issue about this a few months ago when I accidentally nuked a few animations in a game jam due to the byRef copy-paste behavior, but this behavior also occurs on testing with inherited scenes.

Possible workaround: Open each animation in the base scene's AnimationPlayer and set its local_to_scene value to true. This workaround has been untested with copy-pasted animations.

TheDuriel commented 5 years ago

Resources are shared by default to avoid unnecessary duplication. Modifying animations at runtime is not something everyone needs, so the vast majority of animations will never need duplicating. I don't think this is a good idea.

You can instead perhaps save your animations to disk (always recommended), duplicate those files. And load them as unique instances as needed.

Or you know... call .duplicate() on them before modifying from code.

ghost commented 5 years ago

It could benefit from some clarity. The concept of Resources and how the AnimationPlayer manages them is not indicated. I'm still never sure exactly when the Animation has been duplicated or is referenced.

As it is currently, there is a legitimate issue with accidentally destroying work when dealing with Animations.

TheDuriel commented 5 years ago

Resources, an thus Animations are never duplicated unless you do so manually.

@avencherus

Resources are indeed in need to a proper docs page. They are 50% of Godots core architecture (together with Nodes), but few people are aware of this. (Resources can do a lot of cool stuff actually.)

nobuyukinyuu commented 5 years ago

@TheDuriel

I never mentioned this being a runtime issue. This is a design time issue, as indicated with my concerns with how the inspector doesn't make it obvious that an animation is a typical ByRef resource unless you explicitly access it from the animation tab's pulldown menu. Animations themselves perhaps should be visible in the AnimationPlayer's inspector context in the same way as other resources are, with the same context menus.

The problem comes when trying to copy animations over to a seperate player, or alter properties of an AnimationPlayer inherited from a base node at design time. Most users will treat the fields of the animation tab context as overridable properties of the inherited instance of the node/scene (or in the case of copied animations to a new player in a new scene, a duplicate for the host the resource is attached to), not as references to the base animation as they actually are.

This may be two separate problems, but the copy paste issue can be solved with clearer menus (paste reference vs paste duplicate). The inherited scene issue at design time may require changes to the host's inspector context (in this case, AnimationPlayer) to clarify.

nobuyukinyuu commented 5 years ago

The way animations are currently stored in an AnimationPlayer is currently accessed by the end-user through functions and isn't exposed as an inspector property. I'm not sure how the context for this node type works, but some sort of exposed access to the animation list via the inspector seems like the way to go. The less "customized" hacking to the context necessary, the better, since I'm sure there's going to be someone arguing that this is unnecessary duplicate functionality.

Mockup of one possible solution. Shows explicitly in the inspector that Animations are References, important for design-time inherited scenes (edit: Probably better represented with an array of tuples seeing as how Animation resources are not tied to their names in an AnimationPlayer): image

Mockup of another (Addresses copying and pasting of Animations in an explicit way as well as allows for duplicate animations to be copied across AnimationPlayers without the hassle of having to save the Resource to an external file): image

Clarification can be further improved by appending resource ID information to the animation name in the dropdown, similar to how Godot 3.0 showed a resource number in the inspector context. In both examples, the second animation was made with copy and paste, the third, with Duplicate, with "expected" results: image

Perhaps there's a better way to clarify than one or more of these?

ChildLearningClub commented 1 year ago

New users to Godot including myself have been caught by this, probably hard to find anyone who hasn’t unless you know prior to using the engine how it works. I understand now that it is to “save on resources” :) ( Sorry had to put that in there, not that anyone’s laughing right now over all their over writing changes)

I like Godot’s built in efficiency. What’s also nice is that the referencing is abstracted away making that built in efficiency automatic. So why not keep that how it is and when a user goes to edit / modify a referenced object ask the user if they would like to “Keep as Reference” . “You are about to edit a referenced object, which will change the original. Would you like to “Continue” / “Continue and Keep as Reference” (Button) or “Copy to New Animation” (Button). This should at least take care of the confusion upfront, having the “appended resource ID information to the animation name” as mentioned above would also be helpful in tracking down which animation is being used, as that newly created object can now be used as a reference elsewhere.

So to summarize objects are copied as reference, until edited, at which point the user is asked if they want to keep as reference or copy to new animation.

AThousandShips commented 1 year ago

It isn't a referenced object though, all resources are accessed as references, just some are unique, there's no original or reference, all are references of each other

ChildLearningClub commented 1 year ago

@AThousandShips, thank you for explaining. I guess I thought that it would be possible to simply have some kind of message displayed when an object is edited or edited object about to be saved that would indicate that it would effect other objects with that resource attached? Or that it’s not “unique”? I hope I’m getting the terminology correct?

AThousandShips commented 1 year ago

That would absolutely be possible! I don't know if it wouldn't just annoy people who want to have the resource reused, as is often the case

TheDuriel commented 1 year ago

You would encounter that message dozens if not hundreds of times during a normal work day.

All resources are shared by default. We don't need to warn users when they are. Only when they are not.

The documentation I suggested years ago in this thread has already been added.

jeanmarc commented 6 months ago

Godot Noob here, but just wanted to suggest something. Blender shows a reference count in its editors for objects/resources that are being used in multiple places (like materials), and provides a means to make them unique by clicking on the reference count. That could be a minimally intrusive way of showing the multiple-use situation and offering a way to make-unique?

Calinou commented 6 months ago

Godot Noob here, but just wanted to suggest something. Blender shows a reference count in its editors for objects/resources that are being used in multiple places (like materials), and provides a means to make them unique by clicking on the reference count. That could be a minimally intrusive way of showing the multiple-use situation and offering a way to make-unique?

This has been discussed previously, but it's difficult to implement: https://github.com/godotengine/godot-proposals/issues/904#issuecomment-1345290691