Implement consistent functionality for select, copy, paste, and duplicate in AnimationPlayer

[CookieBadger]

This PR seeks to implement the copying and pasting of keyframes (#3419), but to do so consistently, also modifies the behavior of selection and duplication of keyframes.
My idea is, to separate this PR into three individual commits. As there are really many inconsistencies in the AnimationPlayerEditor, I would like to tackle all that are relevant for copy-pasting in the same PR, as I am touching a lot of the AnimationTrackEditor code anyway. The commits would do the following:

1. Modify selection behavior

for copy, paste, and duplicate to work efficiently, the user needs to be able to select the track they want to paste on individually from the keyframes. Previously, clicking on a track would always deselect all keyframes. With this change, holding CTRL or Shift while clicking will not deselect the keyframes anymore. This is consistent with selection in other areas, for example, the scene tree.

Previously, you had to awkwardly select a keyframe on the track you wanted to select and deselect it again, and if there was no keyframe on that track you first had to insert one — bad UX.

2. Implement Copy and Paste of Keyframes in AnimationTrackEditor and AnimationBezierEditor

The bulk of this PR. The implementation adds a clipboard to the AnimationTrackEditor, where a list of keys can be stored. Then, it adds a right-click menu action ("Copy Key(s)") and a shortcut (CTRL+C), that takes the current selection of keys and adds them to the clipboard. With another menu action ("Paste Key(s)") that is only visible when the clipboard is not empty, or shortcut (CTRL+V), the keys then get added to the currently selected tracks. This behavior works in tandem between the regular AnimationTrackEditor and the AnimationBezierEditor, i.e. keys can be copied in one and pasted in the other.

2a. Copy-Pasting from different tracks

1. The implementation follows the behavior set by "duplicate transposed" and takes the lowest track index top (the uppermost track) of the copied keys as a reference. Upon pasting, the program will attempt to insert the keys from the uppermost track on the currently selected track (index s), any other key at track index top+x will be attempted to be pasted on track s+x. While this behavior is intuitive in the default case, we should discuss if there is a need for pasting all keys from different tracks on the same track, and if this additional "Paste on same track" should then have the shortcut CTRL+SHIFT+V. I argue against this being the default case, and having the former action be "Paste transposed", as "Duplicate transposed" already proved to be an obscure and unintuitive description.
2. Now this leaves us with a problem, that currently also exists for duplicate functionality: Track display order can be different between the list and the group (default) view. When some track in the list view is moved, and this is not reflected in the group view, the behavior for copy-paste and duplicate becomes incomprehensible from a group-view user's perspective, when he pastes keys from different tracks on a track that was not the top track. My proposed solutions would be:
   a: show a warning when keys from multiple tracks are copied/duplicated if this is the case OR
   b: keep a separate track index list for the group view, and use the respective indexing in the current view (more complex)
   No matter the solution, the user will still be confused when he copies frames, moves/deletes a track in between and then pastes. I think this is a problem that cannot be solved, and the only possible countermeasure could be invalidating the clipboard on these actions, but I'm happy to hear your suggestions and will then implement accordingly.

2b. Cross-Track Compatibility
To make matters as intuitive as possible, you can copy-paste property keys across tracks that modify the same type. You can also copy-paste Vector3 keys across track types that accept a Vector3, and you can copy-paste keys across float property and bezier tracks. I.e. this is handled the following way:
Pasting keyframes can be performed under the following conditions (target track is the track where the keys are pasted on):

1. if the target track is a property track, the type of the copied key's value is the same as the property (most common use-case)
2. if the target tack is a property track modifying a VECTOR3, and the copied key was on a POSITION or SCALE track
3. if the target track is a property track modifying a QUATERNION and the copied key was a ROTATION track
4. if the target track is a property track modifying a FLOAT and the copied key is a bezier key, in that case the handles are discarded and only the value is copied
5. if the target track is a POSITION or SCALE track and the copied key's value is a VECTOR3
6. if the target track is a ROTATION track and the copied key's value is a QUATERNION
7. otherwise only if the target track type is the same than the copied key's track type (for Animation Tracks, Method Call tracks, Blend Shape Tracks (not tested! I don't know how to use them), etc.)

All other paste attempts are denied, and will prompt the user with the error (popping up from the notification bar) "Paste failed: Not all animation keys were compatible with their target tracks". For now, I do not see the need of a float->int, Vector2->Vector3 conversion when copying, as this would also complicate matters more and would require a lot more warnings/errors.

3. Adjust behavior of Duplicate actions

3a. Merge Duplicate Transposed Behavior
in response to #55713.

3b. Apply same Cross-Track Compatibility rules for Duplicate as in 2b
Currently, it looks like the editor just casts what he can, or inserts a default keyframe when using Duplicate Transposed. This is unintuitive but also dangerous in my opinion.

3c. Adjust naming
It should be named "Duplicate selected keys", as you can select both track and keys.

3d. Fix undo not restoring selection in BezierEditor

4. Behavior of Right-Clicking

4a. Selection
Hovering over a key and right-clicking should select the key (to be consistent with e.g. the scene tree). This saves an extra click when copying a key.

4b. Right-click action positions
Right-click->Insert Key inserts the key at the current mouse position. I think duplicating, and pasting should do the same. Only when using shortcuts, the keys should be placed at the play position. This can also speed up workflows, as pasting/duplicating would not require moving the play position all the time.

[CookieBadger]

Does anybody have some input on 2a,2); the track order incongruity? I'd appreciate it

[fire]

What would be the behaviour of pasting mixed conversions tracks?

[CookieBadger]

@fire Can you specify what you mean by "Mixed Conversions Tracks"?

[CookieBadger]

I have now finished implementing all the points, and readied the code for review and testing.
Track order incongruity might remain subject to discussion for future work.

[Calinou]

Tested locally (rebased on top of master 10e1114), I get this error when pressing Ctrl + V when attempting to paste a keyframe (nothing happens):

  ./core/object/message_queue.cpp:223 - Error calling deferred method: 'AnimationTrackEditor::AnimationTrackEditor::_anim_paste_keys': Method expected 2 arguments, but called with 1.

Pasting by right-clicking the track and choosing Paste Key(s) works.

PS: Would it be possible to change the text to say Paste Key when there is one key to paste and Paste Keys when there are multiple keys to paste?

[CookieBadger]

@Calinou sorry for this oversight, that I introduced in the last commit. It is fixed now!

About changing the wording of the context menu entry: it would absolutely be possible, but then the same would apply for Delete Key(s), Add RESET Value(s), and Duplicate Key(s). I find it better to adjust to the existing standard, and if you would find it sensible, a new PR, or another commit on top of this PR could be made that changes the wording of all these entries. I am against having some entries that use the generic wording and some that specify the Numerus (singular, plural).

[Calinou]

About changing the wording of the context menu entry: it would absolutely be possible, but then the same would apply for Delete Key(s), Add RESET Value(s), and Duplicate Key(s). I find it better to adjust to the existing standard, and if you would find it sensible, a new PR, or another commit on top of this PR could be made that changes the wording of all these entries. I am against having some entries that use the generic wording and some that specify the Numerus (singular, plural).

That makes sense. I agree with this being a better fit for a future PR 🙂

[Calinou]

I've tested the latest revision of the PR and the copy-paste shortcuts work as expected now.

I took a cursory glance at the code and it seems good to me, but I suggest having someone from @godotengine/animation to give it a final look.

[AThousandShips]

Otherwise the style looks good!

[AThousandShips]

Please squash your commits into one, see here

[CookieBadger]

@AThousandShips Each commit performs strictly distinctive changes, with the state in-between being stable (I made sure to fix all issues in their respective commit). Should I still squash? Just making sure

[AThousandShips]

Yes it's still the same total feature IMO, they're connected IMO, unless there's some specific reason to keep them apart we always squash, unless for example one but not the others might need to be removed etc.

[TokageItLab]

Nice work! I've tried at least UndoRedo, pasting across scenes, copying references, etc., and I didn't encounter any bugs in that. Seems to be quite robustly built.

[akien-mga]

Thanks!

[CookieBadger]

Nobody mentioned cutting (CTRL+X) to be worth implementing as well, and I totally forgot about that one. Might be a good next addition.

EDIT: will be implemented by #88350