add context menus for scenes in the outliner (just a single entry atm.: "Delete")
Details
Details
Diff Detail
Diff Detail
Event Timeline
Comment Actions
this was actually asked for in T45661: Delete scene from Outliner (and I had this done locally already)
| source/blender/editors/space_outliner/outliner_tools.c | ||
|---|---|---|
| 292 | Instead of passing context in as the optional void* arg, it'd be better to have this as the first arg instead. So, the function signature for this method would look more like: void scene_cb(bContext* C, eOutliner_PropSceneOps event, TreeElement *te, TreeStoreElem *tselem, void *arg) or maybe the event arg could even be moved to be just before arg, so void scene_cb(bContext *C, TreeElement *te, TreeStoreElem *tselem, eOutliner_PropSceneOps event, void *arg) | |
| 1666 | Have you checked whether the initial checks for objectlevel/datalevel/etc. are still relevant? | |
Comment Actions
adressing review by @Joshua Leung (aligorith):
- adjusted function signature (actually got rid of void arg as well -- not needed and can still be added back if we need it for other scene operations)
- added back checks for mixed selection (this slipped through, sorry)
note: objectlevel is not checking mixed selction either? (but this is for another commit)
note2: still have to track down BKE_report: it is not showing in the banner (is this normal? -- also marked with XXX in previous code...)
Comment Actions
Everything working fine for me, logic seems correct, just have some minor suggestions :)
| source/blender/editors/space_outliner/outliner_tools.c | ||
|---|---|---|
| 305 | Think it's better to make this const. | |
| 311 | Doesn't seem like we need to do a manual undo push here? Just use operator flags OPTYPE_REGISTER | OPTYPE_UNDO below. | |
| 312 | Would add: else {
BLI_assert(0);
return OPERATOR_CANCELLED;
}here. | |
| 322 | Picky, esp since it currently isn't visible in UI: We usually don't use title case in descriptions, also would use a bit more 'fluent' wording, like "Context menu for scene operations" (see OUTLINER_OT_operation). | |
Comment Actions
adressing review by @Julian Eisel (Severin):
-left in the manual undo push because it is easier to distinguish between the different scene operations (once there are more than just one...) - would that make senses?
Comment Actions
Don't see why a manual undo push would make that easier? Also I can't think of a scene operation that wouldn't need an undo push. So still think this should be done using ot->flag.
Comment Actions
because each individual operation will get it's own named undo push (e.g. inspect the undo history) vs. one generic "Outliner Scene Operation"...
[dont think this is terribly important, but still more clear to be able to distinguish, no?]
vs
see how the other "_operation" OPs do it as well in outliner_tools?
Agree that every operation will have an undo push...
Comment Actions
Yep, LGTM. Also agree that manual undo push is better here, totally forgot about the undo history.