Skip to content

Allow double-click within tracks to set a new play position#92141

Merged
Repiteo merged 1 commit into
godotengine:masterfrom
SanNull:Anim-Player-Pos-Double-Click
May 19, 2025
Merged

Allow double-click within tracks to set a new play position#92141
Repiteo merged 1 commit into
godotengine:masterfrom
SanNull:Anim-Player-Pos-Double-Click

Conversation

@SanNull
Copy link
Copy Markdown
Contributor

@SanNull SanNull commented May 20, 2024

This feature allows the user to move the Animation Play Position by double clicking, being more intuitive and improving workflow by keeping the mouse on the tracks, as opposed to going at the top only to change its current position.

Showcase

@akien-mga
Copy link
Copy Markdown
Member

akien-mga commented May 20, 2024

Thanks for the contribution, and welcome! I've assigned the Animation team to assess the proposal.

For the record, your commit seems not to be linked to your GitHub account. See: Why are my commits linked to the wrong user? for more info.

@akien-mga akien-mga requested a review from a team May 20, 2024 07:34
@AdriaandeJongh
Copy link
Copy Markdown
Contributor

Considering snapping the play position to frames is also applied everywhere else where you move the play position, wouldn't it make sense if this feature also snapped?

@SanNull
Copy link
Copy Markdown
Contributor Author

SanNull commented May 21, 2024

wouldn't it make sense if this feature also snapped?

Snapping works just the same with it. I assume the signal timeline_changed handles the snapping (if enabled). Here's an example

Snapping

@SanNull
Copy link
Copy Markdown
Contributor Author

SanNull commented Jan 17, 2025

I know this might be late, however since it's a small quality of life feature, could this be pushed to Godot 4.4? I assume it does not conflict with other systems.

@fire fire modified the milestones: 4.x, 4.5 Jan 17, 2025
@fire
Copy link
Copy Markdown
Member

fire commented Jan 17, 2025

I aspire to get this reviewed and in for 4.5.

Copy link
Copy Markdown
Member

@TokageItLab TokageItLab left a comment

Choose a reason for hiding this comment

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

It makes sense as an operation and is a good idea.

However, since the selection cancellation action is similar to a double-click, so it would be necessary to make sure that there is no selection (means this should be into the if (!selected && !editor->is_selection_active()) block).

@SanNull SanNull force-pushed the Anim-Player-Pos-Double-Click branch from 23a0ca8 to 62155ef Compare May 17, 2025 23:35
@SanNull
Copy link
Copy Markdown
Contributor Author

SanNull commented May 17, 2025

@TokageItLab if we use !editor->is_selection_active() that means we can't double click on a selected keyframe that is stationary. So I opted to insert the block of code into if (!moving_selection). I'd also like to ask how probable it is currently for a double click to be misinterpreted as a selection cancellation by the animation player.

@SanNull SanNull requested a review from TokageItLab May 17, 2025 23:37
@fire fire mentioned this pull request May 18, 2025
6 tasks
@TokageItLab
Copy link
Copy Markdown
Member

TokageItLab commented May 18, 2025

It may be solved by adding an additional determination if the click is on a key or not. The case to be avoided is, for example, when a right-click menu is closed with a left click on a specific key, the next left click must be a key selection, not a move.

@SanNull
Copy link
Copy Markdown
Contributor Author

SanNull commented May 18, 2025

I think that tracking if a keyframe was clicked to allow double clicking kinda limits the functionality (e.g if you want to go on an empty space on the track). Additionally I tested this example and double clicking was not an issue — It did not replace selection with a double click.

hipothetical example

hipothetical examplea

Is there any case where the selection consistently gets overridden by a double-click? Or a case where it double clicks and selects at the same time? I'm sorry if I'm a little slow at understanding the issue, but it's as though I'm attempting to fix something that might not be broken.

Copy link
Copy Markdown
Member

@TokageItLab TokageItLab left a comment

Choose a reason for hiding this comment

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

Oh, I was looking at 4.3 and it seems that 4.3 and 4.4 already have different behavior. I don't know where this occurred, but in 4.4 the cancel action and the selection are done at the same time.

Well, considering that fewer clicks may be preferred, it appears that right-click cancellations, etc. are not a problem at this time.

However, I found one problematic behavior: multiple selections with Shift may be made by double-clicking to deselect (at the same time deselecting and seeking occur and become hard to see), so you should need to check that no mod keys (Shift/Alt/Ctrl) are pressed. Also, I think the same changes need to be made to BezierEditor for consistency.

The change request is above two things.

@SanNull SanNull force-pushed the Anim-Player-Pos-Double-Click branch from 62155ef to e64206d Compare May 18, 2025 18:56
@SanNull
Copy link
Copy Markdown
Contributor Author

SanNull commented May 18, 2025

  • Double clicking now works both on the Track Editor as well as the Bezier Editor:

double_click

  • If one of the mod keys (Shift/Alt/Ctrl) are pressed, double click will not be allowed.

@SanNull SanNull requested a review from TokageItLab May 18, 2025 19:06
@TokageItLab TokageItLab moved this from Ready for review to Approved, Waiting for Production in Animation Team Issue Triage May 19, 2025
@Repiteo Repiteo merged commit c4c2009 into godotengine:master May 19, 2025
20 checks passed
@github-project-automation github-project-automation Bot moved this from Approved, Waiting for Production to Done in Animation Team Issue Triage May 19, 2025
@Repiteo
Copy link
Copy Markdown
Contributor

Repiteo commented May 19, 2025

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

7 participants