Page MenuHome

VSE: add a Set frame range to Strips operator
ClosedPublic

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

Details

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

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?

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.

Alessio Monti di Sopra (a.monti) edited the summary of this revision. (Show Details)
  • 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.

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;
  }
Richard Antalik (ISS) added inline comments.
source/blender/editors/space_sequencer/sequencer_edit.c
4205

Ok, In any case this is more a user interface issue.

This revision is now accepted and ready to land.Oct 21 2019, 9:29 PM