Check operators for potentially missing "UNDO" in bl_options #82899

Open
opened 2020-11-21 12:38:46 +01:00 by Robert Guetzkow · 4 comments

Description:
Python operators that modify ID data without setting "UNDO" in their bl_options can result in a crash when the user undoes changes. Since #82763 showed that not all Blender add-ons have been tested and update to comply with the changes made to the undo system and there are several operators using just bl_options = {"REGISTER"} or bl_options = {'REGISTER'}, it would be a good idea to check that the remaining operators do not make the same mistake.

Work plan
The following is a list of all operators that use bl_options with REGISTER but without UNDO. Not all of them modify ID data, hence some will not require UNDO to be included. However, all of the need to be checked whether or not that is the case.

Blender Add-ons

  • Works with mesh data and modifies the UVs. I think this should fall into the category of ID data that would require an "UNDO".
  • Instead of calling bpy.ops.ed.undo() the operator should likely be modified to use "UNDO". Additionally, the reference keeping on object, edges and vertices should be double checked. At first glance this looks like it may keep invalid references when the modal operator is cancelled as this wouldn't call _exit that clears to variables, but this would have to verified. Storing references to this data is definitely against best practice though, as documented on the Gotcha page for the Python API docs.
  • Modifies FloatProperty in PropertyGroup, should probably use "UNDO".

Blender Add-ons Contrib

Blender Startup Scripts

**Description:** Python operators that modify ID data without setting `"UNDO"` in their `bl_options` can result in a crash when the user undoes changes. Since #82763 showed that not all Blender add-ons have been tested and update to comply with the changes made to the undo system and there are several operators using just `bl_options = {"REGISTER"}` or `bl_options = {'REGISTER'}`, it would be a good idea to check that the remaining operators do not make the same mistake. **Work plan** The following is a list of all operators that use `bl_options` with `REGISTER` but without `UNDO`. **Not all of them modify ID data, hence some will not require `UNDO` to be included**. However, all of the need to be checked whether or not that is the case. ### Blender Add-ons * - [ ] `chooseDoodad` ([mesh_discombobulator.py ](https://developer.blender.org/diffusion/BA/browse/master/add_mesh_discombobulator/mesh_discombobulator.py)) * Modifies a string array stored in a `PropertyGroup`. This doesn't seem to fall under the category of ID data that would require `"UNDO"` as far as I can currently tell. * - [ ] `unchooseDoodad` ([mesh_discombobulator.py ](https://developer.blender.org/diffusion/BA/browse/master/add_mesh_discombobulator/mesh_discombobulator.py)) * Modifies a string array stored in a `PropertyGroup`. This doesn't seem to fall under the category of ID data that would require `"UNDO"` as far as I can currently tell. * - [x] `POWER_SEQUENCER_OT_jump_time_offset` ([jump_time_offset.py ](https://developer.blender.org/diffusion/BA/browse/master/power_sequencer/operators/jump_time_offset.py)) * - [x] `POWER_SEQUENCER_OT_playback_speed_set` ([playback_speed_set.py ](https://developer.blender.org/diffusion/BA/browse/master/power_sequencer/operators/playback_speed_set.py)) * - [x] `POWER_SEQUENCER_OT_render_apply_preset` ([render_apply_preset.py ](https://developer.blender.org/diffusion/BA/browse/master/power_sequencer/operators/render_apply_preset.py)) * - [x] `PIE_OT_FileIncrementalSave` ([pie_save_open_menu.py ](https://developer.blender.org/diffusion/BA/browse/master/space_view3d_pie_menus/pie_save_open_menu.py)) * - [x] `KillBgProcess` ([bg_blender.py ](https://developer.blender.org/diffusion/BA/browse/master/blenderkit/bg_blender.py)) * - [ ] !!`MUV_OT_UVSculpt` ([uv_sculpt.py ](https://developer.blender.org/diffusion/BA/browse/master/magic_uv/op/uv_sculpt.py))!! - Works with mesh data and modifies the UVs. I think this should fall into the category of ID data that would require an `"UNDO"`. * - [x] `AutoMirror` ([mesh_auto_mirror.py ](https://developer.blender.org/diffusion/BA/browse/master/mesh_auto_mirror.py)) * - [ ] !!`SnapUtilitiesLine` ([op_line.py ](https://developer.blender.org/diffusion/BA/browse/master/mesh_snap_utilities_line/op_line.py))!! - Instead of calling `bpy.ops.ed.undo()` the operator should likely be modified to use `"UNDO"`. Additionally, the reference keeping on object, edges and vertices should be double checked. At first glance this looks like it may keep invalid references when the modal operator is cancelled as this wouldn't call `_exit` that clears to variables, but this would have to verified. Storing references to this data is definitely against best practice though, as documented on the [Gotcha ](https://docs.blender.org/api/current/info_gotcha.html#help-my-script-crashes-blender) page for the Python API docs. * - [ ] !!`TCCallBackCCEN` ([CCEN.py ](https://developer.blender.org/diffusion/BA/browse/master/mesh_tiny_cad/CCEN.py)) !! - Modifies `FloatProperty` in `PropertyGroup`, should probably use `"UNDO"`. * - [x] `WriteGimpPalette` ([paint_palette.py ](https://developer.blender.org/diffusion/BA/browse/master/paint_palette.py)) * - [x] `ColorPickerPopup` ([brush_menu.py ](https://developer.blender.org/diffusion/BA/browse/master/space_view3d_brush_menus/brush_menu.py)) * - [x] `CurvePopup` ([curve_menu.py ](https://developer.blender.org/diffusion/BA/browse/master/space_view3d_brush_menus/curve_menu.py)) * - [x] `DeleteVar` ([__init__.py ](https://developer.blender.org/diffusion/BA/browse/master/space_view3d_math_vis/__init__.py)) * - [x] `ToggleDisplay` ([__init__.py ](https://developer.blender.org/diffusion/BA/browse/master/space_view3d_math_vis/__init__.py)) * - [x] `ToggleLock` ([__init__.py ](https://developer.blender.org/diffusion/BA/browse/master/space_view3d_math_vis/__init__.py)) * - [ ] !!`ToggleMatrixBBoxDisplay` ([__init__.py ](https://developer.blender.org/diffusion/BA/browse/master/space_view3d_math_vis/__init__.py))!! * No issue with undo, but operator attempts to call function from `utils` that doesn't exist * - [x] `CleanupConsole` ([__init__.py ](https://developer.blender.org/diffusion/BA/browse/master/space_view3d_math_vis/__init__.py)) * - [x] `ToggleApplyModifiersView` ([space_view3d_modifier_tools.py ](https://developer.blender.org/diffusion/BA/browse/master/space_view3d_modifier_tools.py)) * - [x] `ToggleAllShowExpanded` ([space_view3d_modifier_tools.py ](https://developer.blender.org/diffusion/BA/browse/master/space_view3d_modifier_tools.py)) * - [x] `PIE_OT_SetAreaType` ([pie_editor_switch_menu.py ](https://developer.blender.org/diffusion/BA/browse/master/space_view3d_pie_menus/pie_editor_switch_menu.py)) * - [x] `PIE_OT_Timeline` ([pie_editor_switch_menu.py ](https://developer.blender.org/diffusion/BA/browse/master/space_view3d_pie_menus/pie_editor_switch_menu.py)) * - [x] `PIE_OT_SetObjectModePie` ([pie_modes_menu.py ](https://developer.blender.org/diffusion/BA/browse/master/space_view3d_pie_menus/pie_modes_menu.py)) * - [x] `VIEW3D_OT_SetObjectMode` ([object_menus.py ](https://developer.blender.org/diffusion/BA/browse/master/space_view3d_spacebar_menu/object_menus.py)) ### Blender Add-ons Contrib * - [ ] !!`CreaPrim` ([object_creaprim.py ](https://developer.blender.org/diffusion/BAC/browse/master/object_creaprim.py))!! - Currently broken, registering of the created primitive doesn't work (`ValueError: register_class(...): expected a class argument, not 'str'`) - Modifies `bpy.data.texts` * - [x] `OBJECT_OT_load_img` ([__init__.py ](https://developer.blender.org/diffusion/BAC/browse/master/lighting_hdri_shortcut/__init__.py)) * - [x] `RenderBackground` ([space_view3d_render_settings.py ](https://developer.blender.org/diffusion/BAC/browse/master/space_view3d_render_settings.py)) ### Blender Startup Scripts * - [x] `CLIP_OT_delete_proxy` ([clip.py ](https://developer.blender.org/diffusion/B/browse/master/release/scripts/startup/bl_operators/clip.py)) * - [x] `CLIP_OT_set_viewport_background` ([clip.py ](https://developer.blender.org/diffusion/B/browse/master/release/scripts/startup/bl_operators/clip.py)) * - [x] `WM_OT_previews_batch_generate` ([file.py ](https://developer.blender.org/diffusion/B/browse/master/release/scripts/startup/bl_operators/file.py)) * - [x] `WM_OT_previews_batch_clear` ([file.py ](https://developer.blender.org/diffusion/B/browse/master/release/scripts/startup/bl_operators/file.py)) * - [x] `WM_OT_blend_strings_utf8_validate` ([file.py ](https://developer.blender.org/diffusion/B/browse/master/release/scripts/startup/bl_operators/file.py)) * - [x] `EditExternally` ([image.py ](https://developer.blender.org/diffusion/B/browse/master/release/scripts/startup/bl_operators/image.py)) * - [x] `ProjectEdit` ([image.py ](https://developer.blender.org/diffusion/B/browse/master/release/scripts/startup/bl_operators/image.py)) * - [x] `ProjectApply` ([image.py ](https://developer.blender.org/diffusion/B/browse/master/release/scripts/startup/bl_operators/image.py)) * - [x] `PlayRenderedAnim` ([screen_play_rendered_anim.py ](https://developer.blender.org/diffusion/B/browse/master/release/scripts/startup/bl_operators/screen_play_rendered_anim.py))
Author
Member

Added subscriber: @rjg

Added subscriber: @rjg
Author
Member

Changed status from 'Needs Triage' to: 'Confirmed'

Changed status from 'Needs Triage' to: 'Confirmed'
Robert Guetzkow changed title from ToDo: Check operators for potentially missing "UNDO" in bl_options to Check operators for potentially missing "UNDO" in bl_options 2020-11-21 12:48:50 +01:00
Member

Added subscriber: @Imaginer

Added subscriber: @Imaginer
Member

ColorPickerPopup and CurvePopup are both UI popups (context.window_manager.invoke_popup) and shouldn't have "UNDO".

ColorPickerPopup and CurvePopup are both UI popups (`context.window_manager.invoke_popup`) and shouldn't have "UNDO".
Sign in to join this conversation.
No Milestone
No project
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: blender/blender-addons#82899
No description provided.