godot
godot copied to clipboard
godotengine
Allow changing imported AnimationLibrary names in AnimationPlayer in the editor
Is this the right way? Changes to externally imported resources are not saved and their editing should be prohibited. @SaracenOne @reduz
There are already several PRs to make them unique and editable instead, and I think those should be prioritized (but I do not remember if those worked with AnimationLibrary or not).
This is not a property on AnimationLibrary, it's a property of the AnimationPlayer, as explained in the linked issue. It belongs to the scene and does get saved.
I remember discussing with @reduz earlier that a scene might temporarily store key values, but the problem is that those changes are lost when the resource is re-imported.
I think that's referring to keys used by libraries to reference animations, not keys used by players to reference libraries. I was modifying these keys before this change happened, and I've been editing .tscn files in a text editor as a workaround since, and reimporting hasn't reset anything.
Ah, I may have been a bit misleading, by "key" do you mean the animation library name in AnimationPlayer? It is indeed a property of AnimationPlayer. I had it confused with Key in Animation Track.
AnimationPlayer should be temporarily read-only when imported from outside, but after the scene is Make Local, library renaming in AnimationPlayer should be allowed. In any case, I think that this PR needs to be reviewed by @reduz.
I think this should depend on whether the scene is editable rather than the animation library. Should not be always editable. @SaracenOne may give some good feedback.
In what situation is a scene not editable, and how do you check? Currently, imported scenes can be edited, the editor just displays some warnings that changes won't be saved.
So, I apologize for not looking over this sooner. Admittedly, the 'rules' around editable vs non-editable is a quite confusing, and it does require me to repeatedly test things to make sure they do indeed work as intended. Admittedly, a lot of my testing was based around animation libraries embeded into imported scenes rather than this workflow which I really learned about recently, and I think it's permissible to allow editing the names of the keys since the implicit result is, due to the fact that renaming the keys simply modifies the libraries property which is just key/value dictionary, it's not affecting the underlying libraries which have much stricter restrictions on what you can do with them.
I did run into an issue though where upon modifying the [global] key in one of my test scenes which reference a library embedded in another, things still continued to work as expected, the library was referenced as embedded in another scene, but upon investigating, still seemed everything was referenced properly. However, upon reverting the scene, the external library reference got broken. I think it's related to another bug though which I've raised a potential fix for here. https://github.com/godotengine/godot/pull/82750
As such, since that bug seems to be out of the scope of this PR, I'm going to tentitively approve this for merging. Apologies again for being so overdue.
Happy PR anniversary :tada: :sweat_smile:
Could you rebase the PR on latest master? There's no conflict so it's not strictly speaking needed, but since the commit is so old it would be good to refresh it so it shows up close to its merge commit.
