Keep arranger clip instance length same after change #2886
Conversation
| // log action | ||
| Action* action = actionLogger.getNewAction(ActionType::CLIP_INSTANCE_EDIT, ActionAddition::ALLOWED); | ||
| clipInstance->change(action, output, clipInstance->pos, newLength, newClip); | ||
| clipInstance->change(action, output, clipInstance->pos, desiredLength, newClip); |
There was a problem hiding this comment.
Should we preserve extending the length if the new clip is longer than the old one?
There was a problem hiding this comment.
When editing an existing clip instance it makes sense to me to always keep it the same length. In the context of an arrangement it makes sense for the intention to be: in this specific range of the arrangement, I want to change what happens, no matter the length of the source clip.
When creating a new clip instance, it does make more sense to have the result be the length of the chosen clip. This also gives the user a very quick choice between keeping an existing clip instance the same length (hold + turn select) and changing the instance length to clip length (tap head to delete, hold + turn select to create new clip instance at clip length)
I'll implement this
There was a problem hiding this comment.
Yeah but I think this code also covers the initial clip selection, like if you put in a 4 bar blue clip (e.g. last clip you used) then try to change it to your 8 bar pink clip
|
New clip instances now adhere to the length of the clip that is chosen, while making the initial clip selection Nice bonus behavior: when creating a new clip instance, if the user makes a conscious choice to set the length of the clip instance with a second press, clip instance will from that point stay the chosen length regardless of chosen clip |
| } | ||
| // press on same row, to the left of the currently holding pad, do nothing | ||
| return; |
There was a problem hiding this comment.
Return behavior implementation is a bit inconsistent - this is not absolutely needed here but doesn't hurt either
|
I removed the cherry pick tag as I'm not sure whether this fix should go to beta or not |
Fair! I'm choosing to just implement stuff to my best ability from the perspective of getting things done, but by all means shoot at it if it doesn't seem ideal. |
|
Yeah I think this is an improvement but better to avoid adding things to beta at this point, there might be something we haven't thought about |
|
@RJ-Eckie are you still working on this? I'm not sure if it was clear but we do want to merge it, we just don't want to put it into the current beta (e.g. firmware 1.2). It is definitely a good add to 1.3 |
|
+1 encountered this issue today. Recorded a song to arranger. I missed launching a two bar drum phrase that was to repeat four times. There is a hit that is set to only play once out of four repeats. I tried to add the clip manualIy but can't extend the length. I remember last time (pre community firmware) that I could press the start pad (and hold) and then press the end pad to extend or truncate the length of the clip to cause it to repeat. Adding the clip four distinct times causes that initial hit to fire four times. I see RJ's bonus behavior above which sounds like a fix. Question: Can the length of an existing clip in arranger view be changed or is this behavior only when clip is first added? |
|
I answered my own question: Yes, this change does allow edits of an existing clip's length in arranger view. I went and looked at pull request staged for release 1.3. First time for me in the code. Editing clip length in Arranger view is essential functionality. I hope this change could make its way into the 1.2 release. |
This sounds like an unrelated bug - you should definitely still be able to edit the length of clips in arranger with and without this change |
|
Retested my issue with new 12/04 build of community beta 1.2. Existing clips in arranger mode are now properly extending their length when start/end pads are selected. |
|
Closing since it seems to be dropped |
Fix #2877
This deletes a bunch of safety-checks for the length of the new clip-instance
As we're taking the length from a valid clip-instance, we already know the length is valid