VSE: add a Set frame range to Strips operator
ClosedPublic
Actions

Authored by Alessio Monti di Sopra (a.monti) on Oct 16 2019, 11:47 AM.

Details

Reviewers

Commits

rB5c83d63151fc: VSE: add a Set frame range to Strips operator
rBd4bef6a21508: VSE: add a Set frame range to Strips operator

Summary

The patch adds a new operator in sequencer_edit.c that sets the frame range, with an option to choose the regular or the preview one, around the selected strips.

In the UI both options are added in the Range sub-menu of the View menu; the Preview range one is also added to the contextual menu.

Diff Detail

Repository: rB Blender

Event Timeline

Alessio Monti di Sopra (a.monti) created this revision.Oct 16 2019, 11:47 AM

Peter Fog (tintwotin) awarded a token.Oct 16 2019, 1:05 PM

Evan Wilson (EAW) awarded a token.Oct 17 2019, 4:21 PM

Really great that you have added and exposed these much needed functions.

Rename 'Set Preview/Frame Range to Strip' to 'Set Preview Range/Frame to Strips', since it is working on all selected strips.

And maybe keep the Frame and the Preview functions separate:

Set Preview Range
Clear Preview Range
Set Preview Range to Strips
(separator)
Set Start Frame
Set End Frame
Set Frame Range to Strips

And I don't think it should be added to the context menu, it is already quite cluttered and needs a clean up, imo.

What is @William Reynish (billreynish) opinion on this?

In D6078#140219, @Peter Fog (tintwotin) wrote:

Rename 'Set Preview/Frame Range to Strip' to 'Set Preview Range/Frame to Strips', since it is working on all selected strips.
And maybe keep the Frame and the Preview functions separate:

All good points, I will update the patch.
Looking around it also seem like the convention in these cases is to also use plural for the actual C operator, so will do that as well.

And I don't think it should be added to the context menu, it is already quite cluttered and needs a clean up, imo.

About this: I personally think that the preview range is something that one would want to access quickly and, while a dedicated shortcut seemed too much to me, I thought that the context menu would have been the right place for this kind of features.

Properly too complicated with "true" context menus in the VSE at this time, so I'm fine with you keep it in the Context Menu, but I don't have a final say in these things.

renamed operator and descriptions to use plural "Strips"
rearranged the Range sub-menu, to keep preview and regular frame range separated

Other than 1 nitpick looks OK.

source/blender/editors/space_sequencer/sequencer_edit.c
4205	Not sure if there is convention for this, but I would use warning for cases where operation was done, but with some exception (such as sfra can not be negative) If operation can not be done, that should be an error.

Alessio Monti di Sopra (a.monti) added inline comments.Oct 21 2019, 11:03 AM

source/blender/editors/space_sequencer/sequencer_edit.c
4205	I'm not aware of a defined conventions either, but for these cases to me an error seemed too much. Looking in the code I actually found different examples of similar warnings, picked two random ones as an example: if (ob->mode & OB_MODE_EDIT) { BKE_report(op->reports, RPT_WARNING, "Cannot join while in edit mode"); return OPERATOR_CANCELLED; } if (ok == false) { BKE_report(op->reports, RPT_WARNING, "Active object is not a selected armature"); return OPERATOR_CANCELLED; }