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

Add animation filtering to animation editor #103130

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Arnklit
Copy link
Contributor

@Arnklit Arnklit commented Feb 21, 2025

This adds track filtering to the Animation Editor. This will close godotengine/godot-proposals#11741

cW20avecEd.mp4

Note: Currently the filtering is a simple contains check. I had a go at using the fuzzy_search class for better filtering, but I wasn't getting great results. I'll look into that futher and add it in a later PR.

Copy link
Member

@SaracenOne SaracenOne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me! A significant UX improvement to the animation editor and works as expected. You might want to consider adding tooltips to the add menu button and the new filter tracks field, but otherwise, I think this is good. 👍

filter_track->set_placeholder(TTR("Filter Tracks"));
filter_track->connect(SceneStringName(text_changed), callable_mp((AnimationTrackEditor *)this, &AnimationTrackEditor::_on_filter_updated));
filter_track->set_clear_button_enabled(true);
filter_track->hide();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here: filter_track->set_tooltip_text(TTR("Filter tracks by name by entering part of their name."));

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"property or Node name" but i wonder if the entire NodePath from the root affects the filtering (did not see the code, sorry)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. It searches the whole property path which includes the node name and the property. I went for this:

filter_track->set_tooltip_text(TTR("Filter tracks by entering part of their node name or property."));

Copy link
Contributor

@MarianoGnu MarianoGnu Mar 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well, my concern was if you want to filter properties for a Sprite node, but you also have tracks for childs of this node, those tracks may also appear here if the entire NodePath is used in the filtering, because RigidBody2D/Sprite2D/Light2D/SomeCollisionShape:disabled includes the word "Sprite" on it

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm good point. Let me do a bit of testing on this tomorrow and get back to you.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Notes:

  • when you create a track for a node with % access it will use it. so the node path could be "%Sprite2D:position"
  • when creating tracks to the property of a resource, it looks like this: "%CollisionShape2D:shape:size"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@MarianoGnu Looking this over I feel it's ok that it's using the whole path. It feels like there would be cases where users are expecting to be able to filter like that as well, so I'm not sure it makes sense to remove it. It makes more sense to me to leave it like this first and then change it if users report that it's not doing what they expect.

I don't understand your notes about % access. If I set a node to have % access it makes no difference to the track paths that get added to the animation player when I start animating its properties.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, i was just pointing out you may find strings like that 👍
In the case of %, not full path
In the case of subresources, several : simbols

@SaracenOne SaracenOne self-assigned this Mar 11, 2025
@Arnklit Arnklit force-pushed the filter-animations branch 3 times, most recently from f68f2b0 to ff8ff06 Compare March 13, 2025 11:05
@Arnklit Arnklit force-pushed the filter-animations branch from ff8ff06 to 6bb7536 Compare March 14, 2025 12:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

Add a filter field to the animation editor to allow filtering tracks
4 participants