Skip to content

Version 2026.7.1 - #404

Open
Mustard2 wants to merge 15 commits into
mainfrom
2026.7.1
Open

Version 2026.7.1#404
Mustard2 wants to merge 15 commits into
mainfrom
2026.7.1

Conversation

@Mustard2

@Mustard2 Mustard2 commented Aug 22, 2026

Copy link
Copy Markdown
Owner
  • Enhancement: Some Creator Tools now offer the possibility to remove unused data (null shape keys, unused vertex groups, attributes, UV Maps).
  • Enhancement How to setup the new Jiggle physics? #399: The Jiggle Cage (Quick) operator now infers the direction automatically by default.
  • Fix Custom Properties bug #402: Improved the section selection for custom properties in Configuration mode, in order to be less confusing and restore the custom property list correct icons indentation.
  • Fix Create Morphs Preset error #403: Morphs with shape keys might fail to be written.
  • Fix: Context in active model selection.

@Mustard2 Mustard2 self-assigned this Aug 22, 2026
@Mustard2 Mustard2 added the bug Something isn't working label Aug 22, 2026
This was linked to issues Aug 22, 2026
@Mustard2

Copy link
Copy Markdown
Owner Author

@claude Review

@claude

claude Bot commented Aug 22, 2026

Copy link
Copy Markdown

Claude finished @Mustard2's task in 3m 48s —— View job


Code Review — PR #404 (Version 2026.7.1)

Reviewed the diff against origin/main (13 files, +446/-80). Overall the changes are clean and match the repo conventions in CLAUDE.md (register/unregister ordering, mustardui_active_object usage, ops_*/settings_* naming). Two issues found, both minor:

1. Dead fallback / wrong comparison in the custom-property section icon (custom_properties/ui_list.py:44-47)

section = rig_settings.body_custom_properties_sections.get(item.section)
icon = "RECORD_OFF"
if section is not None:
    icon = section.icon if section not in {"", "NONE"} else "DOT"

section is a MustardUI_SectionItem PropertyGroup instance, not a string, so section not in {"", "NONE"} is always True — the "DOT" fallback branch is unreachable, and a section with an unset icon (""/"NONE") won't fall back to "DOT" as intended. The correct comparison (and the one already used a few lines away in the new custom_properties/ops_set_section.py:23) is against section.icon, not section itself:

icon = section.icon if section.icon not in {"", "NONE"} else "DOT"

Since this PR's own goal (#402) is to "restore the custom property list correct icon indentation," this is worth fixing for consistency even though it isn't a crash (icon values are constrained by the EnumProperty, so nothing breaks — it's a cosmetic/logic inconsistency).

Fix this →

2. Missing model_body null guard in the new shape-key preset paths (morphs/settings_presets.py:20,65, morphs/ops_defvalue.py:40)

The #403 fix correctly switches from the (non-existent) rig_settings.data to rig_settings.model_body.data, but none of the three call sites check rig_settings.model_body is not None before dereferencing .data:

shape_keys = rig_settings.model_body.data.shape_keys  # settings_presets.py:20, ops_defvalue.py
...
elif m.get("shape_key") and rig_settings.model_body.data.shape_keys:  # settings_presets.py:65

model_body is a PointerProperty that can legitimately be None — the codebase guards against this explicitly elsewhere (configuration/ops_quick_setup.py, configuration/ops_configuration.py, misc/get_ui_objects.py, etc.). In practice these preset/morph paths are only reachable on an enabled/configured model where model_body should already be set, so this is likely low-risk, but an unguarded AttributeError would be a regression vs. the previous defensive rig_settings.data and rig_settings.data.shape_keys check pattern that was dropped.

Fix this →

Other notes (no action needed)

  • model_selection/active_object.py: switching bpy.context.scenecontext.scene is a correct fix — using the passed-in context instead of the global is the right call when this helper can be invoked from contexts where they diverge (matches the "Fix: Context in active model selection" changelog entry).
  • tools_creators/ops_jiggle.py: the automatic Pin-direction inference (border-vertex vs. island-center vector, then nearest-island matching for the generated cages) looks correct, and the refactored gradient projection math is behavior-preserving for the existing manual +X/-X/… options (verified the sign/inversion algebra matches the old per-axis branches).
  • misc/mesh_cleanup.py, ops_optimize_sk.py, ops_collision_cage.py, ops_jiggle_accurate.py: the new cleanup helpers (void shape-key detection/removal, unused vertex groups/attributes) are well-isolated, skip morph-managed shape keys correctly, and are wired behind opt-in operator toggles.

Not run: ruff check . / ruff format --check . — Bash execution requires interactive approval in this environment and none was available; please run these locally/in CI to confirm lint/format still pass (the CI lint job will also catch this).

@Mustard2 Mustard2 linked an issue Aug 25, 2026 that may be closed by this pull request
@Mustard2 Mustard2 linked an issue Aug 25, 2026 that may be closed by this pull request
@Mustard2

Copy link
Copy Markdown
Owner Author

@claude Review

@claude

claude Bot commented Aug 26, 2026

Copy link
Copy Markdown

Claude finished @Mustard2's task in 3m 12s —— View job


Code Review — PR #404 (Version 2026.7.1)

  • Gather context (PR diff, previous review comments)
  • Review changed files against origin/main
  • Check whether previously-flagged issues were addressed
  • Post final review

I re-reviewed the full diff (git diff origin/main...HEAD, 23 files, +677/-108), including the 5 commits pushed after the previous automated review (10f003a08054bf).

Previously-flagged issues — status

  1. Icon fallback in custom_properties/ui_list.py — ✅ fixed. Line 47 now correctly compares section.icon, not section itself.
  2. Missing model_body null guard in morphs/settings_presets.py / morphs/ops_defvalue.py — still unaddressed, but confirmed low-risk: configuration/ops_configuration.py:37 requires rig_settings.model_body to be set before a model can be enabled, and all these code paths gate on config=0 (enabled model only), so model_body should never be None when reached in practice.

New finding

tools_creators/ops_optimize_sk.py:18add_shape_key_mute_driver default flip breaks the "revert" (remove drivers) button on first use

This PR (commit b8bb0ab) changed the property default from True to False:

add_shape_key_mute_driver: bpy.props.BoolProperty(
    default=False,   # was True
    ...
)

This same property is reused for both directions of the operator: the forward "Optimize" call and the "revert" call (menu/menu_tools_creators.py:271-273, .revert = True). In revert mode, the dialog draws only this one checkbox (ops_optimize_sk.py:164-169), and execution is gated by:

if not self.add_shape_key_mute_driver and not remove_void:  # remove_void is always False when reverting
    self.report({"WARNING"}, "MustardUI - No Option Selected.")
    return {"CANCELLED"}

Since Blender operators reuse the last-set property values across invocations of the same bl_idname, the very first time a user clicks the revert (LOOP_BACK) button in a session, add_shape_key_mute_driver is still at its unset default (False), so confirming the dialog immediately cancels with "No Option Selected" instead of removing the mute drivers — the user has to notice this, re-open the dialog, and manually check the box. Previously (default True) this worked out of the box. Low severity (recoverable, not a crash), but likely an unintended side effect of reusing this property for the new opt-in "Remove Void Shape Keys" logic.

Suggested fix: give the revert-mode checkbox its own property (or force add_shape_key_mute_driver = True when self.revert is set in invoke()), so the "remove drivers" default isn't affected by the new forward-flow default.

Fix this →

Other notes (no action needed)

  • custom_properties/ops_set_section.py, ops_menu_settings.py: the new per-property section picker is wired correctly — sections_enum matches the item-caching pattern needed to avoid EnumProperty string GC, index passed to MustardUI_Property_SetSection matches the true arm.MustardUI_CustomProperties collection index (verified against menu/menu_configure_body.py's template_list binding), and custom_prop.section is a plain StringProperty so the ""/SECTION_NONE round-trip is safe.
  • model_selection/active_object.py: bpy.context.scenecontext.scene is correct.
  • physics/settings_presets.py: removal of the unused force parameter from apply_physics_preset is clean — no remaining call sites pass it.
  • presets/ops_create.py, ops_delete.py, ops_transfer.py: adding options={"HIDDEN"} to preset_type is a reasonable cleanup, no behavior change.
  • misc/mesh_cleanup.py, tools_creators/ops_collision_cage.py, tools_creators/ops_jiggle_accurate.py: the new opt-in cleanup helpers (void shape-key/vertex-group/attribute removal) are well-isolated, correctly preserve modifier-referenced vertex groups (e.g. Pin groups) via the keep= parameter, and are gated behind their own operator toggles.
  • tools_creators/ops_transfer_vertex_groups.py: new "All Groups" / "From Bones" bulk-add helpers are straightforward and correctly dedupe against the existing list.

Not run: ruff check . / ruff format --check . — Bash execution requires interactive approval in this environment; please confirm CI's lint job passes.

@Mustard2

Copy link
Copy Markdown
Owner Author

@claude Review now again

@claude

claude Bot commented Aug 26, 2026

Copy link
Copy Markdown

Claude finished @Mustard2's task in 1m 49s —— View job


Code Review — PR #404 (Version 2026.7.1)

  • Gather context (latest PR diff, previous review comments)
  • Check whether previously-flagged ops_optimize_sk.py revert-default issue was addressed
  • Review the 3 new commits since the last review (1828901, e8bdb62, b01d33c)
  • Post final review

Re-reviewed git diff origin/main...HEAD (23 files, +749/-108). Since the last automated review, 3 new commits landed:

Previously-flagged issue — status

tools_creators/ops_optimize_sk.py revert-default flip — ✅ fixed in 1828901. invoke() now forces self.add_shape_key_mute_driver = True when self.revert is set, so the revert (remove drivers) button works correctly on first use again, independent of the new forward-flow default (False).

New commits reviewed

  • e8bdb62 (comments cleanup) — pure docstring/comment trimming in misc/mesh_cleanup.py, no behavior change.
  • b01d33c (broadened scope of vertex group mesh cleanup) — adds collect_used_vertex_groups(), replacing the old "only check modifier.vertex_group" logic with a generic recursive walk (_vertex_group_properties, depth-limited to 3, skipping ID datablocks and large data collections via MAX_WALKED_COLLECTION) plus dedicated handling for Geometry Nodes modifier inputs (modifier.values()), particle systems, soft body settings, shape-key vertex groups, and constraint subtargets (including bone constraints on any object in the file). This is a correctness improvement for clear_unused_vertex_groups — it now avoids deleting vertex groups that are legitimately referenced from more places than before. The implementation is conservative by design (a same-named string custom property on a modifier could theoretically cause a vertex group to be over-preserved via the modifier.values() scan), but since this only makes cleanup more cautious rather than less, it's a safe direction for an opt-in destructive tool. No bugs found in the recursion bounds, None-guards (obj.soft_body, other.pose), or the single call site (tools_creators/ops_jiggle_accurate.py:1113-1116, which still correctly passes keep=[pin_name, structural_group_name] on top of the new automatic detection).

No new issues found. Nothing else outstanding from prior review rounds beyond the already-noted, confirmed-low-risk model_body null-guard observation (not a blocker — all reachable call paths are gated on an enabled/configured model).

Not run: ruff check . / ruff format --check . — Bash execution requires interactive approval in this environment; please confirm CI's lint job passes before merging.
· branch 2026.7.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Image renaming bug Morphs preset problem Create Morphs Preset error Custom Properties bug

1 participant