From b2b0798c2a20b7c1b43c7bfb2ff1f01de5ab1df5 Mon Sep 17 00:00:00 2001 From: Nathan Date: Wed, 25 Mar 2026 14:56:38 -0600 Subject: [PATCH] fix rmorig ARP --- ...0-29-12Z-retargeting-focus-object-issue.md | 793 ++++++++++++++++++ .specstory/statistics.json | 8 +- ui/operators.py | 2 +- utils/remove_original.py | 28 +- 4 files changed, 823 insertions(+), 8 deletions(-) diff --git a/.specstory/history/2026-03-13_20-29-12Z-retargeting-focus-object-issue.md b/.specstory/history/2026-03-13_20-29-12Z-retargeting-focus-object-issue.md index 372b7e9..9843cc7 100644 --- a/.specstory/history/2026-03-13_20-29-12Z-retargeting-focus-object-issue.md +++ b/.specstory/history/2026-03-13_20-29-12Z-retargeting-focus-object-issue.md @@ -4942,3 +4942,796 @@ _**Agent (composer-2-fast)**_ Tool use: **edit_file_v2** + +_**Agent (composer-2-fast)**_ + +
Thought Process +**Refactoring migrator** + +I'm extracting the mesh processing logic into a new function _process_mig_bbody_mesh. I'm handling cases where orig_base can be None and fixing manual path logic for shape key processing. I'm updating the manual override logic to use context.scene.dynamic_link_manager for props access. I'm processing mesh bodies with auto flag detection and handling descendants.
+ +_**Agent (composer-2-fast)**_ + + +Extracting `_process_mig_bbody_mesh` and updating `run_mig_bbody_shapekeys`. + + + +_**Agent (composer-2-fast)**_ + +The string to replace was not found in the file. + +
+Tool use: **edit_file_v2** + +
 + +_**Agent (composer-2-fast)**_ + + + + + + +_**Agent (composer-2-fast)**_ + + +
Tool use: **read_file_v2** • Read file: T:/Dynamic-Link-Manager/ops/migrator.py + +
+ + +
+Tool use: **read_file_v2** + +
 + +_**Agent (composer-2-fast)**_ + +
Thought Process +**Replacing migration body** + +I'm replacing the run_mig_bbody_shapekeys function body with the new implementation.
+ +_**Agent (composer-2-fast)**_ + + +Inserting the helper before `run_mig_bbody_shapekeys`, then replacing that function. + + + +_**Agent (composer-2-fast)**_ + + + +
Tool use: **edit_file_v2** • Edit file: T:/Dynamic-Link-Manager/ops/migrator.py + +
+ + + +
+Tool use: **edit_file_v2** + +
 + +_**Agent (composer-2-fast)**_ + + + + + + +_**Agent (composer-2-fast)**_ + + +
Tool use: **read_file_v2** • Read file: T:/Dynamic-Link-Manager/ops/migrator.py + +
+ + +
+Tool use: **read_file_v2** + +
 + +_**Agent (composer-2-fast)**_ + + +Fixing broken indentation, removing the invalid `orig.name` reference, and restoring `run_mig_bbody_shapekeys`. + + + +_**Agent (composer-2-fast)**_ + + + +
Tool use: **edit_file_v2** • Edit file: T:/Dynamic-Link-Manager/ops/migrator.py + +
+ + + +
+Tool use: **edit_file_v2** + +
 + +_**Agent (composer-2-fast)**_ + +
Thought Process +**Updating operator** + +I'm updating the operator to distinguish between manual and auto modes with clearer UI feedback. I'm implementing logic to handle override state transitions between manual and auto modes. + +I'm handling early returns for override state transitions and updating the operator message to clarify manual mode states. I'm updating the operator to report a clearer message when auto-detection finds no base meshes and manual override is enabled. I'm fixing the order of manual override checks to ensure proper fallback behavior when fields are empty. I'm adjusting the manual override logic to skip auto execution when both fields are filled, even with override enabled. + +I'm adding a check to skip auto execution when both manual override fields are filled. I'm refining the warning logic to only show when manual override is set due to auto-detection failure. + +I'm updating the operator to warn when manual override is set and no CC-style mesh is matched. I found a bug where manual override persists across runs and causes redundant manual execution when auto path is skipped.
+ +_**Agent (composer-2-fast)**_ + + + + + + +_**Agent (composer-2-fast)**_ + + + +
Tool use: **edit_file_v2** • Edit file: T:/Dynamic-Link-Manager/ui/operators.py + +
+ + + +
+Tool use: **edit_file_v2** + +
 + +_**Agent (composer-2-fast)**_ + + +
+Tool use: **read_lints** + + +Parameters: + +```json +{ + "paths": [ + "T:/Dynamic-Link-Manager/ops/migrator.py", + "T:/Dynamic-Link-Manager/ui/properties.py", + "T:/Dynamic-Link-Manager/ui/panels.py", + "T:/Dynamic-Link-Manager/ui/operators.py" + ] +} +``` +Result: + +```json +{ + "linterErrorsByFile": [ + { + "relativeWorkspacePath": "T:/Dynamic-Link-Manager/ops/migrator.py" + }, + { + "relativeWorkspacePath": "T:/Dynamic-Link-Manager/ui/properties.py" + }, + { + "relativeWorkspacePath": "T:/Dynamic-Link-Manager/ui/panels.py" + }, + { + "relativeWorkspacePath": "T:/Dynamic-Link-Manager/ui/operators.py" + } + ] +} +``` +Status: **completed** + + +
+ + +
+Tool use: **read_lints** + +
 + +_**Agent (composer-2-fast)**_ + +
Thought Process +**Refining warning logic** + +I'm adjusting the warning conditions to avoid false positives when manual override is set but bodies are still empty.
+ +_**Agent (composer-2-fast)**_ + + +Here’s what was added: + +### Scene properties ([`ui/properties.py`](ui/properties.py)) +- **`migbbody_manual_override`** (bool, default **False**) – “Manual body meshes”; turns **on automatically** when auto-detection doesn’t find any CC-style base mesh (`any_auto` stays false). You can also turn it on yourself to skip auto and use picks only. +- **`migbbody_orig_body`** / **`migbbody_rep_body`** – `PointerProperty` to **mesh** objects (`poll`: `type == "MESH"`). + +### UI ([`ui/panels.py`](ui/panels.py)) +- Above **MigBBodyShapeKeys**: checkbox **Manual body meshes**, then **Original body** and **Replacement body**. +- The two pickers use **`row.enabled = props.migbbody_manual_override`** so they stay **inactive until** the checkbox is on **or** MigBBody auto-enables it after a failed pass. + +### Logic ([`ops/migrator.py`](ops/migrator.py)) +- **`_process_mig_bbody_mesh(orig_base, ob, context)`** – shared implementation (overrides + shape keys + action). +- **`run_mig_bbody_shapekeys`**: + 1. If **`migbbody_manual_override`** and **both** pointers are set → run **`_process_mig_bbody_mesh(orig, rep)`** once and **return** (no CC name checks). + 2. Otherwise run the **existing** auto loop (name + armature modifier). + 3. If **`any_auto` is still false** and **`props` exists** → **`props.migbbody_manual_override = True`** so the next time the pickers are usable. + +### Operator ([`ui/operators.py`](ui/operators.py)) +- If manual mode is on but **either** body pointer is missing → **WARNING**: no CC-style mesh matched; pick meshes and run again. +- Otherwise → **INFO**: migrate done. + +**Flow:** Run MigBBody once without CC rigs → auto finds nothing → **Manual body meshes** turns on → choose original/replacement body meshes → run again for the manual path. + +--- + +_**User**_ + +that works great! + +ARP's rmorig is failing again: Failed to remove original: bpy_prop_collection.__contains__: expected a string or a tuple of strings + + +--- + +_**Agent (composer-2-fast)**_ + + + + + + +_**Agent (composer-2-fast)**_ + + +
Tool use: **read_file_v2** • Read file: T:/Dynamic-Link-Manager/utils/remove_original.py + +
+ + +
+Tool use: **read_file_v2** + +
 + +_**Agent (composer-2-fast)**_ + +
Thought Process +**Fixing collection lookup** + +I'm correcting the collection lookup to use string names instead of object references. I'm updating the collection lookup to use string names instead of object references as per the Blender API documentation. + +**Updating collection lookup** + +I'm fixing the collection lookup to use `coll.name in parent.children` after finding that Blender 5 requires string names for the `in` operator.
+ +_**Agent (composer-2-fast)**_ + + +Fixing collection membership checks: in Blender 5, `coll in parent.children` is invalid — `children` expects string keys, so use `coll.name in parent.children` or iterate and compare objects. + + + +_**Agent (composer-2-fast)**_ + + + +
Tool use: **edit_file_v2** • Edit file: T:/Dynamic-Link-Manager/utils/remove_original.py + +
+ + + +
+Tool use: **edit_file_v2** + +
 + +_**Agent (composer-2-fast)**_ + + + + + + +_**Agent (composer-2-fast)**_ + + +
+Tool use: **read_lints** + + +Parameters: + +```json +{ + "paths": [ + "T:/Dynamic-Link-Manager/utils/remove_original.py" + ] +} +``` +Result: + +```json +{ + "linterErrorsByFile": [ + { + "relativeWorkspacePath": "T:/Dynamic-Link-Manager/utils/remove_original.py" + } + ] +} +``` +Status: **completed** + + +
+ + +
+Tool use: **read_lints** + +
 + +_**Agent (composer-2-fast)**_ + + +Here’s what was going wrong. + +In **Blender 5**, `Collection.children` is a `bpy_prop_collection` whose **`in` / `__contains__`** only accepts **names** (strings), not `Collection` instances. The old code used `coll in master.children` and `coll in p.children`, which triggers: + +`bpy_prop_collection.__contains__: expected a string or a tuple of strings` + +**Change:** use the collection’s name: + +- `coll.name in master.children` +- `coll.name in p.children` + +in [`utils/remove_original.py`](utils/remove_original.py) inside `_parent_collection`. + +ARP Remove Original should get past collection resolution again; try the operator once more. + +--- + +_**User**_ + +@c:\Users\Nathan\.cursor\projects\t-Dynamic-Link-Manager\terminals\3.txt it's removing both orig and rep again. + +--- + +_**Agent (composer-2-fast)**_ + +
Thought Process +**Tracing removal logic** + +The issue is that removing the top-level collection deletes both original and replacement armatures if they share the same collection subtree. I'm implementing a fix to prevent deletion of replacement armatures when removing collections. I need to understand the remove original operator and resolver flow to determine how to properly handle deletion of original objects vs. shared collections during migration. I'll avoid removing collections that contain replacements and instead fallback to object-level removal. I'm identifying the correct collection to remove by walking up from orig's users_collection and checking for collections containing both orig and rep. I'm simplifying the removal logic to check if `rep` is contained in the collection subtree being removed. + +I'm implementing a helper function to check if an object exists in a collection or its children. I'm adding `rep` parameter to `_collection_for_remove_original` and implementing recursive containment checks. I'm implementing a check to return None if the resolved collection contains `rep`, ensuring operator removes only the `orig` object. + +I need to verify Blender's behavior for object deletion when collections are removed. I found that removing a collection deletes all its members if they have no other collection membership, so I'll implement a check to never remove collections containing `rep` objects. + +I'm implementing a recursive check to detect if a collection contains an object and will always return None for collection removal when orig and rep are in the same collection.
+ +_**Agent (composer-2-fast)**_ + + +Checking the Remove Original flow: removing a collection that contains both armatures deletes both. Adding a guard so we don't remove a collection that contains the replacement armature. + + + +_**Agent (composer-2-fast)**_ + + +
Tool use: **read_file_v2** • Read file: T:/Dynamic-Link-Manager/ui/operators.py + +
+ + +
+Tool use: **read_file_v2** + +
 + +_**Agent (composer-2-fast)**_ + +
Thought Process +**Updating remove logic** + +I'm adding recursive collection checking and refining the target selection logic to pick the smallest containing collection. I'm implementing the recursive collection traversal algorithm to find the correct removal target. + +I'm implementing recursive collection traversal to safely remove objects while preserving references. I'm analyzing the chain traversal to identify the outermost safe collection for removal. I identified the minimal removal unit by traversing the chain from inner to outer to find the smallest collection containing orig but not rep. I'm implementing recursive collection traversal to properly identify object containment across nested structures. I'm iterating through child collections in Blender 5 to properly resolve containment. I'm identifying the smallest collection containing only the original object for deletion. + +I'm implementing edge case handling for nested collections and merging with library heuristics. I'm implementing the target selection logic for nested collection removal. I'm refining the chain traversal logic to properly handle nested collections where inner levels might contain the target while outer levels don't. I'm handling the edge case where parent collections may incorrectly report containment due to recursive traversal.
+ +_**Agent (composer-2-fast)**_ + + +Filtering out collections that contain the replacement armature before removing — removing a shared parent collection deleted both characters. + + + +_**Agent (composer-2-fast)**_ + + +
Tool use: **read_file_v2** • Read file: T:/Dynamic-Link-Manager/utils/remove_original.py + +
+ + +
+Tool use: **read_file_v2** + +
 + +_**Agent (composer-2-fast)**_ + + + + + + +_**Agent (composer-2-fast)**_ + + +
+Tool use: **ripgrep_raw_search** + + +Parameters: + +```json +{ + "caseInsensitive": false, + "path": "T:/Dynamic-Link-Manager", + "pattern": "resolve_collection_for_remove_original" +} +``` +Additional data: + +```json +{ + "isPruned": true, + "outputMode": "content", + "path": "T:/Dynamic-Link-Manager", + "pattern": "resolve_collection_for_remove_original", + "topFiles": [ + { + "matchCount": 1, + "uri": "utils\\remove_original.py" + }, + { + "matchCount": 10, + "uri": ".specstory\\history\\2026-03-13_20-29-12Z-retargeting-focus-object-issue.md" + }, + { + "matchCount": 2, + "uri": "ui\\operators.py" + } + ], + "totalFiles": 3, + "totalMatches": 13 +} +``` +Status: **completed** + + +
+ + +
+Tool use: **ripgrep_raw_search** + +
 + +_**Agent (composer-2-fast)**_ + + + + + + +_**Agent (composer-2-fast)**_ + + +
Tool use: **read_file_v2** • Read file: T:/Dynamic-Link-Manager/ui/operators.py + +
+ + +
+Tool use: **read_file_v2** + +
 + +_**Agent (composer-2-fast)**_ + +
Thought Process +**Implementing recursive collection check** + +I'm implementing `_collection_contains_object_recursive()` and updating `resolve_collection_for_remove_original()` to handle recursive object detection. I'm handling edge cases in recursive collection checks where safe collections might not form a contiguous prefix in the filter chain. + +**Handling recursive checks** + +I found that skipping parents in recursive checks can create gaps where safe inner collections are separated from unsafe parents. I realized that if a collection contains a rep, all its ancestors must also contain that rep due to the recursive relationship. + +**Analyzing recursive relationships** + +The unsafe collections form a prefix from outer or suffix from inner based on rep containment. I determined that only inner collections can be safe if they don't contain the rep object recursively. I found that if inner contains only orig and rep is in parent, then inner doesn't contain rep and parent.objects might. + +The recursive search doesn't traverse into parent objects, so `inner` doesn't contain `rep` even though `rep` exists in `parent.objects`. I found that `_collection_contains_object_recursive(parent, rep)` returns True, making parent unsafe, and the safe chain is [inner] only. I found that when orig and rep are in the same collection, both are unsafe and None is returned. I'm exploring how removing parent nodes affects recursive containment when orig and rep are in different hierarchies. I found that `_pick_remove_target` prefers outermost nodes for linked removal, which could accidentally delete unrelated subtrees if rep isn't in the target subtree. I'm identifying edge cases where full chain removal may incorrectly target unrelated collections. + +I identified that parent P is safe while grandparent G is unsafe in the recursive chain.
+ +_**Agent (composer-2-fast)**_ + + +Implementing recursive containment check and filtering the walk-up chain so we never remove a collection whose subtree includes the replacement armature. + + + +_**Agent (composer-2-fast)**_ + + + +
Tool use: **edit_file_v2** • Edit file: T:/Dynamic-Link-Manager/utils/remove_original.py + +
+ + + +
+Tool use: **edit_file_v2** + +
 + +_**Agent (composer-2-fast)**_ + + + +
Tool use: **edit_file_v2** • Edit file: T:/Dynamic-Link-Manager/utils/remove_original.py + +
+ + + +
+Tool use: **edit_file_v2** + +
 + +_**Agent (composer-2-fast)**_ + + + +
Tool use: **edit_file_v2** • Edit file: T:/Dynamic-Link-Manager/ui/operators.py + +
+ + + +
+Tool use: **edit_file_v2** + +
 + +_**Agent (composer-2-fast)**_ + + + + + + +_**Agent (composer-2-fast)**_ + + +
+Tool use: **ripgrep_raw_search** + + +Parameters: + +```json +{ + "caseInsensitive": false, + "glob": "*.py", + "path": "T:/Dynamic-Link-Manager", + "pattern": "resolve_collection_for_remove_original" +} +``` +Additional data: + +```json +{ + "isPruned": true, + "outputMode": "content", + "path": "T:/Dynamic-Link-Manager", + "pattern": "resolve_collection_for_remove_original", + "topFiles": [ + { + "matchCount": 2, + "uri": "ui\\operators.py" + }, + { + "matchCount": 1, + "uri": "utils\\remove_original.py" + } + ], + "totalFiles": 2, + "totalMatches": 3 +} +``` +Status: **completed** + + +
+ + +
+Tool use: **ripgrep_raw_search** + +
 + +_**Agent (composer-2-fast)**_ + + +
+Tool use: **read_lints** + + +Parameters: + +```json +{ + "paths": [ + "T:/Dynamic-Link-Manager/utils/remove_original.py", + "T:/Dynamic-Link-Manager/ui/operators.py" + ] +} +``` +Result: + +```json +{ + "linterErrorsByFile": [ + { + "relativeWorkspacePath": "T:/Dynamic-Link-Manager/utils/remove_original.py" + }, + { + "relativeWorkspacePath": "T:/Dynamic-Link-Manager/ui/operators.py" + } + ] +} +``` +Status: **completed** + + +
+ + +
+Tool use: **read_lints** + +
 + +_**Agent (composer-2-fast)**_ + + + + + + +_**Agent (composer-2-fast)**_ + + +
+Tool use: **ripgrep_raw_search** + + +Parameters: + +```json +{ + "caseInsensitive": false, + "contextAfter": 35, + "path": "T:/Dynamic-Link-Manager/ui/operators.py", + "pattern": "def _get_migrator_pair" +} +``` +Additional data: + +```json +{ + "isPruned": true, + "outputMode": "content", + "path": "T:/Dynamic-Link-Manager/ui/operators.py", + "pattern": "def _get_migrator_pair", + "topFiles": [ + { + "matchCount": 36, + "uri": "ui/operators.py" + } + ], + "totalFiles": 1, + "totalMatches": 1 +} +``` +Status: **completed** + + +
+ + +
+Tool use: **ripgrep_raw_search** + +
 + +_**Agent (composer-2-fast)**_ + + + + + + +_**Agent (composer-2-fast)**_ + + +
Tool use: **read_file_v2** • Read file: T:/Dynamic-Link-Manager/ui/operators.py + +
+ + +
+Tool use: **read_file_v2** + +
 + +_**Agent (composer-2-fast)**_ + + +Here’s what was going wrong and what we changed. + +**Cause:** `resolve_collection_for_remove_original` could choose a **parent** collection that still held **both** the original and replacement rigs. `bpy.data.collections.remove(coll)` deletes that whole subtree, so the replacement vanished too. + +**Fix:** +1. **`_collection_contains_object_recursive`** – walks `coll.objects` and **child collections** so we know if an armature lives anywhere under a collection. +2. **`resolve_collection_for_remove_original(..., rep=None)`** – when `rep` is set, the walk-up chain is **filtered** to collections that do **not** contain `rep` in that subtree. If nothing is left (e.g. orig and rep in the same collection), the function returns **`None`** and the operator only runs **`bpy.data.objects.remove(orig, do_unlink=True)`**, so the replacement stays. +3. **`DLM_OT_migrator_remove_original`** – passes **`rep`** into the resolver. + +Relevant code: + +```59:71:T:/Dynamic-Link-Manager/utils/remove_original.py +def _collection_contains_object_recursive(coll, ob): + """True if ob is in coll.objects or in any descendant collection (recursive).""" + if coll is None or ob is None: + return False + for o in coll.objects: + if o == ob: + return True + for child in coll.children: + if _collection_contains_object_recursive(child, ob): + return True + return False +``` + +```125:132:T:/Dynamic-Link-Manager/utils/remove_original.py + if rep is not None and rep.name in bpy.data.objects: + chain = [c for c in chain if not _collection_contains_object_recursive(c, rep)] + if not chain: + return None + + return _pick_remove_target_from_chain(orig, chain, rig_family) +``` + +Try **Remove Original** again in your ARP setup; if orig/rep shared a collection, you should get object-only removal of the original instead of nuking both. diff --git a/.specstory/statistics.json b/.specstory/statistics.json index 736b809..255153c 100644 --- a/.specstory/statistics.json +++ b/.specstory/statistics.json @@ -226,13 +226,13 @@ "last_updated": "2026-03-25T20:22:34Z" }, "f09112e7-3ddb-4e4e-b91a-418e38b881bf": { - "user_message_count": 20, - "agent_message_count": 298, + "user_message_count": 22, + "agent_message_count": 347, "start_timestamp": "2026-03-13T14:29:12-06:00", "end_timestamp": "2026-03-13T14:29:12-06:00", - "markdown_size_bytes": 277098, + "markdown_size_bytes": 301678, "provider": "cursoride", - "last_updated": "2026-03-25T20:47:25Z" + "last_updated": "2026-03-25T20:52:25Z" }, "f1bb25c5-e5d4-4aad-88a3-f562ab877040": { "user_message_count": 1, diff --git a/ui/operators.py b/ui/operators.py index ff983d5..b3019fe 100644 --- a/ui/operators.py +++ b/ui/operators.py @@ -543,7 +543,7 @@ class DLM_OT_migrator_remove_original(Operator): props = context.scene.dynamic_link_manager rig_family = getattr(props, "migrator_rig_family", "RIGIFY") try: - coll = resolve_collection_for_remove_original(orig, rig_family, context.scene) + coll = resolve_collection_for_remove_original(orig, rig_family, context.scene, rep) if coll: coll_name = coll.name # Store name BEFORE removal (RNA invalidates after remove) context.scene.dynamic_link_manager.original_character = None diff --git a/utils/remove_original.py b/utils/remove_original.py index 47d92f0..7840a99 100644 --- a/utils/remove_original.py +++ b/utils/remove_original.py @@ -13,10 +13,11 @@ def _parent_collection(scene, coll): if coll is None: return None master = scene.collection - if coll in master.children: + # Blender 5+: children.__contains__ expects collection name strings, not Collection objects. + if coll.name in master.children: return master for p in bpy.data.collections: - if coll in p.children: + if coll.name in p.children: return p return None @@ -41,6 +42,19 @@ def _walk_up_chain(scene, coll): return chain +def _collection_contains_object_recursive(coll, ob): + """True if ob is in coll.objects or in any descendant collection (recursive).""" + if coll is None or ob is None: + return False + for o in coll.objects: + if o == ob: + return True + for child in coll.children: + if _collection_contains_object_recursive(child, ob): + return True + return False + + def _deepest_users_collection(scene, armature): """Among armature.users_collection, pick the most nested (max depth) as the inner anchor.""" colls = list(getattr(armature, "users_collection", []) or []) @@ -95,13 +109,16 @@ def _pick_remove_target_from_chain(orig, chain, rig_family): return chain[-1] -def resolve_collection_for_remove_original(orig, rig_family, scene): +def resolve_collection_for_remove_original(orig, rig_family, scene, rep=None): """ Return a collection to remove for Remove Original, or None to fall back to object-only removal. Walks up from the deepest users_collection so nested linked rigs remove the outer instance, not an inner linked child collection. + If rep is the replacement armature, never remove a collection whose subtree contains rep + (avoids deleting both characters when they share a parent collection). + rig_family: 'RIGIFY' | 'ARP' (ARP skips Rigify name heuristics in _pick_remove_target_from_chain). """ if not orig or orig.type != "ARMATURE" or orig.name not in bpy.data.objects: @@ -115,4 +132,9 @@ def resolve_collection_for_remove_original(orig, rig_family, scene): if not chain: return None + if rep is not None and rep.name in bpy.data.objects: + chain = [c for c in chain if not _collection_contains_object_recursive(c, rep)] + if not chain: + return None + return _pick_remove_target_from_chain(orig, chain, rig_family)