Collapse groups in animation track editor#113479
Conversation
baaca3a to
5fb5bec
Compare
|
Thanks for the review! A lot of this was coded up in a bit of an excited hyperfocused frenzy, so there may be some sloppiness in the code that has to be addressed, haha 😅 . I'll address each of the comments in their respective threads, but for the big point: I discussed handling how to save the collapse states with some folks in the RocketChat last night, and they suggested I use the same approach as Godot currently does for collapsing and uncollapsing nodes in scene views. Today I'm planning to work on figuring that out and implementing it here. This definitely won't be ready in time for 4.6, but with all the time that'll be available, hopefully it'll make it in for 4.7 😄 |
f39b37d to
4b8e71b
Compare
|
I think mentioned that inspection editor collapse strategy is preferred. I suspect the node tree collapse in the scenetree is saved but we don’t want runtime impact. We discussed matching and concerns about runtime bloat and version control spam. |
|
It looks to me like the scene tree fold state saving is implemented in |
896a793 to
248e214
Compare
248e214 to
20c97e1
Compare
| @@ -31,6 +31,7 @@ | |||
| #pragma once | |||
|
|
|||
| #include "scene/main/node.h" | |||
| #include "scene/resources/animation.h" | |||
There was a problem hiding this comment.
| #include "scene/resources/animation.h" | |
| class Animation; |
I'd say
|
Fold status is now saved in the editor config files, and is also updated when a node is renamed so that the underlying node keeps its fold status! From my bit of testing, the node-renaming functionality worked, but I am a bit nervous that there could be edge cases with it that I'm unaware of, so feedback from people who've used the Animation system more extensively (or are more familiar with its internals) would be appreciated! |
mihe
left a comment
There was a problem hiding this comment.
It's a bit of shame to have to add new methods to EditorFolding, seeing as how we already have Object::editor_section_folding, but obviously we can't really use that without having to save out the default value (true) for every single track of every single animation player.
Apart from the minor requested changes, this looks good to me and @Arnklit.
|
Wonderful, thanks! I'll try to get to these suggestions soon 🙂 Edit: Suggestions have been applied, thank you again! |
20c97e1 to
5527176
Compare
| Node *n = root->get_node_or_null(node); | ||
| if (n) { | ||
| editor_selection->add_node(n); | ||
| Rect2 fold_area_rect = Rect2(0, 0, get_size().height, get_size().height); |
There was a problem hiding this comment.
I'd say this needs an explanation, using height for both looks odd without it
There was a problem hiding this comment.
I agree, it does seem rather arbitrary. For now I've reworked it a bit to use the width of the left margin plus the width of the icon. I'm a little worried that this means the clickable area might change when the row is folded versus unfolded, but I tried toggling it a bit and it felt natural to me.
a2d3ee7 to
735838e
Compare
|
Looks like folding is only saved for animations stored in their own file. Is this intended? Usually the animations are built-in in the scene. |
|
Hm, my recollection from testing was that for an animation as part of the scene, the file with the saved folding data would use a format like |
That's not a valid filename. |
|
I definitely remember having that same thought (about the file name not being valid) and taking a bit of a detour to try and resolve it. What platform are you testing on? I'm on macOS; I wonder if maybe it lets that kind of filename be used, but other platforms don't? Alternatively it's possible that something subtly broke in the rebase. Will test it again once I'm at my computer. |
|
I'm on Windows. |
|
I don't know if I just never looked at the actual file on disk, or if something changed since the review, but yeah, it ends up trying to write to: ... which ends up actually "working" on Linux somehow, but gets truncated to: On Windows |
|
It should now be updated so that, rather than creating a dedicated file for animation group folding, it does the following: For animations that are built-in to a scene, the folded groups are stored similarly to For animations that are in their own |
|
Got some error: after saving a built-in animation to a file. |
|
This appears to be an existing bug in Godot itself; just opening the "Manage Animations..." menu in the "Animation" tab, when running off the master branch (bf95b62) gives me the error as well (obviously the hash is different): This is on v4.7.dev.custom_build [bf95b62]. I'll look into filing an issue for it, but at the very least it doesn't seem to be caused by this feature PR. Edit: Issue has been logged at #116293, and fix PR has been submitted at #116295 |
2b14ee3 to
5dfeef8
Compare
fire
left a comment
There was a problem hiding this comment.
Animation Meeting Discussion:
@TokageItLab Trusts @KoBeWi 's review.
… on left Save group collapsed state during editing session Save collapsed groups in Animation resource so they persist across sessions Update editor/animation/animation_track_editor.h Remove data duplication and unnecessary method Prevent error about negative-sized Rect2 Move animation group folding to editor cfg files Clean up length of some lines of code Keep fold state of groups when renamed Update scene/resources/animation.h Make fold_area_rect calculation more accurate Improve animation includes Store animation fold state in scene folding file Fix animation fold saving for independent resource animations Apply suggestions from code review Update scene/resources/animation.h Co-authored-by: A Thousand Ships <96648715+AThousandShips@users.noreply.github.com> Co-authored-by: Mikael Hermansson <mikael@hermansson.io> Co-authored-by: Tomasz Chabora <kobewi4e@gmail.com> Co-authored-by: Thaddeus Crews <repiteo@outlook.com>
6fdab10 to
5c4500a
Compare
|
Thanks! |
|
After collapseing Group can we add little hint or light gray animation length to astimate How this group has animation long! may be like this! or this already implimented? sorry i don't have Computer so i sharing my though without checking this pr |
|
As of right now, the groups do not show the "sum" of keyframes within them. I know this is behavior that other tools have (Blender, and I'm pretty sure Unity as well). It could be added in a follow-up PR, although at the moment I'm not sure if I'd have the time for it. If no one else is interested in implementing it though, I can add it to my to-do list 🙂 |
|
Another follow-up to this that would make a lot of sense is the ability to collapse groups in the bezier animation editor as well. |
|
Sorry, I see the resume of changes of the new version: https://godotengine.org/article/dev-snapshot-godot-4-7-dev-2/ And I think that the collapsed group in track editor should show the max time of the items in the group. Such as my quick and dirty mockup:
|
|
@mdtrooper See #113479 (comment) - I agree I think that'd be a good feature to add, but I probably won't have time to work on it myself for at least a little while yet. If someone else is able to start working on it then that will be great, otherwise I may eventually be able to get to it 😅 |
Also had a similar thought, but think it should be greyed out when collapsed:
And if possible, bunched into one |
Yes 👍 it is better your mockup the colour grey and show the points. |
|
We put up a pull request to draw the keys on top of the collapsed groups: #117321 |




Closes godotengine/godot-proposals#10276 .
This PR adds a disclosure button on the left side of each track group in the Animation tab's track editor. Clicking it hides all of the child tracks:
CleanShot.2025-12-02.at.15.52.30.mp4
Collapsed group data is saved in Animation resources, so it persists across editing sessions. However, toggling the visibility of a group does not currently count as an action that dirties the scene/resource (i.e., it doesn't make it display as "unsaved" the way that moving a keyframe does).
TODOs
EditorFoldingclass ineditor/settings/editor_folding.cpp.