4.3 Compatibility & Editable Hotkeys #1

Merged
Demeter Dzadik merged 4 commits from demeters_branch into main 2024-08-15 12:47:59 +02:00
Collaborator

Preface: I got commit rights from Dalai by asking. As far as we know, this add-on has no active maintainer. I asked Nick Berkley for review, just so I don't YOLO push my first contribution. But in the future, I'd like to take over maintenance of this add-on, which I've been using for many years, although I always felt like it could be a lot better.

There are only two commits in this PR, and it could be split into two separate PRs, but I I don't really expect anyone to have time for actual code review, and I wanted to keep things speedy.


4.3 Compatibility

Since the brush system was changed in 4.3, the brush pie broke. I restored it to its original functionality by taking the old brush icon .dat files and packaging them with the add-on. There is also backwards compatibility (one if statement), so users who want to stick to 4.2 LTS can get future updates.


Hotkey UI

It was previously not possible to edit the hotkeys other than disabling them.
I lifted the hotkey system from my add-on CloudRig, which allows editing the shortcuts. Some shortcuts that were previously only 1 entry are now 2 entries (one for each context that the hotkey is available in.)

Before After

I also fixed the Alt+Space manipulator toggle not working due to a typo, and changed some UI strings to be more consistent.

Preface: I got commit rights from Dalai by asking. As far as we know, this add-on has no active maintainer. I asked Nick Berkley for review, just so I don't YOLO push my first contribution. But in the future, I'd like to take over maintenance of this add-on, which I've been using for many years, although I always felt like it could be a lot better. There are only two commits in this PR, and it could be split into two separate PRs, but I I don't really expect anyone to have time for actual code review, and I wanted to keep things speedy. ---- ### **4.3 Compatibility** Since the brush system was changed in 4.3, the brush pie broke. I restored it to its original functionality by taking the old brush icon .dat files and packaging them with the add-on. There is also backwards compatibility (one if statement), so users who want to stick to 4.2 LTS can get future updates. <img src="/attachments/2d656a6f-e6c9-4319-b329-f547c02c0416" width="400"> --- ### **Hotkey UI** It was previously not possible to edit the hotkeys other than disabling them. I lifted the hotkey system from my add-on CloudRig, which allows editing the shortcuts. Some shortcuts that were previously only 1 entry are now 2 entries (one for each context that the hotkey is available in.) | Before | After| | -------- | ------- | | <img src="/attachments/dbc90f20-34b8-4cd9-8b63-3de12ede83d3" width="400"> | <img src="/attachments/33a414f8-0528-42cb-a8da-df19d96775cc" width=400> | --- I also fixed the Alt+Space manipulator toggle not working due to a typo, and changed some UI strings to be more consistent.
Demeter Dzadik added 2 commits 2024-08-14 20:38:15 +02:00
Since brush icons no longer ship with Blender, I had to take them from the 4.2 files and package them with the add-on.

No functional changes. Backwards compatible with 4.2 up to presumably 2.80.
About the hotkey system:
Previously, it was not possible or very inconvenient at best, to customize the keybindings of this add-on. For example, to edit the Proportional Editing Pie's shortcut from Shift O to just O, the user would have to find the 3 registered hotkeys in 3 different places in their keymap editor, which is of course, hot garbage.
Now, keymap entries are displayed in the add-on's preferences in a way that they are editable.

About the registering system:
I'm not sure why, but previously, this add-on would not reload its code when Reload Scripts is executed inside Blender, so for every code change, Blender had to be restarted. This is no longer the case, and the add-on cleanly unregisters itself without any residual registered classes or hotkeys. And Reload Scripts works fine.

Since this commit affects basically the entire codebase, I took the opportunity to format everything with Black.

Also fixed the Manipulator Gizmo Toggle not working, a typo discovered while testing.

I have nothing to do with this add-on but just a few UI thoughts:

  1. The icon usually used for Restore/Reset is LOOP_BACK, not BACK.
  2. The list of menus might be easier to scan if the editor was before the menu name, and without parenthesis.
    For example:
  * 3D View - Pie Shading
  * 3D View - Pie Views Menu
  * 3D View - Pie Manipulator
  1. If the list plans to grow perhaps a UIList would be more fitting, and provide search for free.

Nice to see people keeping old add-ons alive.

I have nothing to do with this add-on but just a few UI thoughts: 1. The icon usually used for Restore/Reset is `LOOP_BACK`, not `BACK`. 2. The list of menus might be easier to scan if the editor was before the menu name, and without parenthesis. For example: ```plaintext * 3D View - Pie Shading * 3D View - Pie Views Menu * 3D View - Pie Manipulator ``` 3. If the list plans to grow perhaps a UIList would be more fitting, and provide search for free. Nice to see people keeping old add-ons alive.
Nika Kutsniashvili requested changes 2024-08-15 11:40:15 +02:00
Dismissed
Nika Kutsniashvili left a comment
Owner

Code is very good. Tested a bit, and everything works as well, except I get one error in mesh edit mode select menu (A), where it has Select Camera, and operator fails because bpy.ops.object.select_all expects object mode. Probably missed poll to remove that operator from edit mode.

I'm usually not in favour of add-on checking for Blender versions and trying to accomodate multiple ones, but if 'asset_activate' in dir(bpy.ops.brush) is cheeky check, I like it. In general, because on extensions platform you can set maximum versions it is always best I think to increase minimum version for new versions when they need it, and change the maximum for previous one, and each Blender version gets its own version.


There are some other authors in removed bl_infos that would be good to include in copyrights in blender_manifest, and yourself too.

Code is very good. Tested a bit, and everything works as well, except I get one error in mesh edit mode select menu (A), where it has Select Camera, and operator fails because `bpy.ops.object.select_all` expects object mode. Probably missed poll to remove that operator from edit mode. I'm usually not in favour of add-on checking for Blender versions and trying to accomodate multiple ones, but `if 'asset_activate' in dir(bpy.ops.brush)` is cheeky check, I like it. In general, because on extensions platform you can set maximum versions it is always best I think to increase minimum version for new versions when they need it, and change the maximum for previous one, and each Blender version gets its own version. --- There are some other authors in removed bl_infos that would be good to include in copyrights in blender_manifest, and yourself too.
@ -31,3 +20,4 @@
dirname, base_name = os.path.split(bpy.data.filepath)
base_name_no_ext, ext = os.path.splitext(base_name)
match = re.match(r"(.*)_([\d]+)$", base_name_no_ext)

Extensions review tool raises flag for this line

Use of back-slash (often used for non-portable MS-Windows paths).

I'm not familiar with re module so can't help you with that much, just letting you know.

Extensions review tool raises flag for this line ``` Use of back-slash (often used for non-portable MS-Windows paths). ``` I'm not familiar with re module so can't help you with that much, just letting you know.
Author
Collaborator

\d is a notation in RegularExpression denoting an integer (0-9), so I think this is a false alarm.

\d is a notation in RegularExpression denoting an integer (0-9), so I think this is a false alarm.
Mets marked this conversation as resolved
@ -117,0 +105,4 @@
)
op.asset_library_type = 'ESSENTIALS'
op.relative_asset_identifier = (
"brushes\\essentials_brushes.blend\\Brush\\" + brush_name

Non-portable backslashes here as well. Causes incorrect filepaths on some systems.

Use os here and let it handle that

os.path.join("brushes", "essentials_brushes.blend", "Brush")

Not sure but I THINK it should recognize blend file as a directory, I remember that being the case.

Non-portable backslashes here as well. Causes incorrect filepaths on some systems. Use `os` here and let it handle that ``` os.path.join("brushes", "essentials_brushes.blend", "Brush") ``` Not sure but I THINK it should recognize blend file as a directory, I remember that being the case.
Author
Collaborator

Great point, fixed!

Great point, fixed!
Mets marked this conversation as resolved
Demeter Dzadik added 1 commit 2024-08-15 11:54:34 +02:00
- Changing the hotkeys UI name display from "Pie Name (Keymap)" to "Keymap - Pie Name".
- Change the hotkey reset icon from "BACK" to "LOOP_BACK"

I also changed the names (bl_label) of the pie menus to be more consistent.
Demeter Dzadik added 1 commit 2024-08-15 12:13:38 +02:00
- Use `os.path.join` instead of typing out backslashes.
- Fix edit mode A key was summoning the Object mode pie menu instead of the Mesh Edit one.
Demeter Dzadik added 1 commit 2024-08-15 12:22:12 +02:00
Demeter Dzadik force-pushed demeters_branch from 5a061ca732 to aa3bc281a8 2024-08-15 12:23:17 +02:00 Compare
Demeter Dzadik force-pushed demeters_branch from aa3bc281a8 to 145b448875 2024-08-15 12:23:44 +02:00 Compare
Author
Collaborator

Addressed all feedback so far, updated task description accordingly. Thanks a lot for the testing and feedback, guys! I wasn't expecting it! :)

Addressed all feedback so far, updated task description accordingly. Thanks a lot for the testing and feedback, guys! I wasn't expecting it! :)
Nika Kutsniashvili approved these changes 2024-08-15 12:27:11 +02:00
Nika Kutsniashvili left a comment
Owner

All fine by me. Thanks for taking care of this

All fine by me. Thanks for taking care of this
Demeter Dzadik force-pushed demeters_branch from 145b448875 to 8cc8b274ca 2024-08-15 12:28:39 +02:00 Compare
Demeter Dzadik merged commit 6a9b54593f into main 2024-08-15 12:47:59 +02:00
Sign in to join this conversation.
No reviewers
No Milestone
No project
No Assignees
3 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: extensions/space_view3d_pie_menus#1
No description provided.