Conversation
|
@claude Review |
|
Claude finished @Mustard2's task in 3m 13s —— View job Review
Overall this is a well-organized PR — the new One real inconsistency worth fixing, plus a couple of minor notes: 1. if item.type == "CAGE":
found = any(
mod.type in ["CLOTH", "SOFT_BODY"]
or (
mod.type == "NODES"
and mod.node_group
and mod.node_group.name.startswith(CLOTH_DYNAMICS_NODE_GROUP)
)
for mod in item.object.modifiers
)This only counts a 2. Minor — broadened "has physics" detection to any Geometry Nodes modifier 3. Nit — Nothing else stood out — the removed operators ( |
|
@claude Review the latest changes |
|
Claude finished @Mustard2's task in 3m 6s —— View job Review of latest changes
One minor edge case, not introduced by these commits but touched by them: in No registration/wiring issues, no leftover dead code, and nothing else stood out. CI is green for the commits up to |
…moned on top of the hair switched changing hair_list
…moned on top of the hair switched changing hair_list
|
@claude Review, especially if the new masks are correctly implemented and all the possible cases (also with locked outfits) are considered. |
|
Claude finished @Mustard2's task in 6m 9s —— View job Review
I traced the whole mask refactor: Locked outfits are handled correctly in the two places that matter:
Hair wiring ( One real bug found:
elif m.type == "PARTICLE_SYSTEM":
rig_settings.hair_enable_global_particles = True
elif m.type == "MASK":
rig_settings.hair_enable_global_particles = TrueThis MASK branch is a copy-paste of the PARTICLE_SYSTEM one above it — it should set the new Nothing else stood out — no registration/wiring issues, and the rest of the mask logic (including the locked-outfit and shared-modifier edge cases specifically asked about) checks out. |
|
@claude Review also the new changes |
|
Claude finished @Mustard2's task in 4m 19s —— View job Review of latest changes
Good news first: the Since then, One real regression introduced by this consolidation — the master mask switch is now applied per host instead of per driving piece:
The problem: This is a real regression vs. the previous version ( Concrete failure scenario: creator enables the separate Hair mask switcher ( Minor, related note: Nothing else stood out in this round — registration/wiring is unaffected (no new classes in this batch of commits), and the rest of the previously-reviewed mask logic (locked outfits, shared |
|
@claude Is the latest fix solving the issue? |
|
Claude finished @Mustard2's task in 1m 37s —— View job Review
Yes —
This exactly resolves the concrete scenario from my last review: a
I re-traced the rest of the call chain ( One pre-existing minor item from my last review is still unaddressed (not the regression itself, so likely not what "the issue" refers to, but noting for completeness): |
|
@claude Final review please |
|
Claude finished @Mustard2's task in 2m 4s —— View job Final Review
Reviewing the two commits added since my last pass (
try:
bpy.ops.ui.copy_data_path_button(full_path=True)
except Exception:
self.report({"ERROR"}, "MustardUI - Invalid selection.")
return {"FINISHED"}with result = bpy.ops.ui.copy_data_path_button(full_path=True)
if "CANCELLED" in result:
self.report({"ERROR"}, "MustardUI - This property does not support being added...")
return {"FINISHED"}This correctly adds a friendlier message for the case where the operator runs but can't resolve a path (returns Overall PR state: CI is green (lint + Blender 4.2/4.5/5.1 build matrix, run 31218744903). All previously-flagged issues across earlier review rounds (CAGE physics-warning inconsistency, Smart Check MASK-branch typo, the mask-switch per-host-vs-per-driving-piece regression) are confirmed fixed as of |
This version adds a new tool to generate high-res physics cages.
Moreover it adds support for Blender 5.2: in particular cloth dynamics in the Physics panel and hair dynamics in the Hair panel.
Changelog