Fix cloner failing to clone USD prims with tree-structured prim paths#6604
Fix cloner failing to clone USD prims with tree-structured prim paths#6604jnskkmhr wants to merge 3 commits into
Conversation
Greptile SummaryThis PR fixes a one-line bug in
Confidence Score: 4/5Safe to merge — the fix is a minimal, well-scoped change that unblocks a crash path without affecting single-wildcard prim paths or any other logic. The change is a single-character addition that correctly resolves a crash when prim paths contain more than one source/isaaclab/isaaclab/cloner/cloner_utils.py and the corresponding test file source/isaaclab/test/sim/test_cloner.py — a new test case for multi-wildcard prim paths would be valuable here. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["prim_path\n/World/envs/env_.*/Robot/.*/link"] --> B["prim_path.replace('.*', '{}', 1)"]
B --> C["destination\n/World/envs/env_{}/Robot/.*/link"]
C --> D{homogeneous?}
D -- yes --> E["destination.format(0)\n→ /World/envs/env_0/Robot/.*/link"]
D -- no --> F["destination.format(env_id)\n→ /World/envs/env_N/Robot/.*/link"]
E --> G["set_spawn_paths / ClonePlan"]
F --> G
G --> H["Cloner handles remaining .* wildcard\nfor tree-structured prims"]
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A["prim_path\n/World/envs/env_.*/Robot/.*/link"] --> B["prim_path.replace('.*', '{}', 1)"]
B --> C["destination\n/World/envs/env_{}/Robot/.*/link"]
C --> D{homogeneous?}
D -- yes --> E["destination.format(0)\n→ /World/envs/env_0/Robot/.*/link"]
D -- no --> F["destination.format(env_id)\n→ /World/envs/env_N/Robot/.*/link"]
E --> G["set_spawn_paths / ClonePlan"]
F --> G
G --> H["Cloner handles remaining .* wildcard\nfor tree-structured prims"]
Reviews (1): Last reviewed commit: "handle tree USD in cloner." | Re-trigger Greptile |
| if count <= 0: | ||
| raise ValueError(f"Spawner at '{prim_path}' must have at least one variant.") | ||
| destination = prim_path.replace(".*", "{}") | ||
| destination = prim_path.replace(".*", "{}", 1) |
There was a problem hiding this comment.
No regression test for the multi-wildcard case
The existing tests in test_cloner.py only exercise prim paths with a single .* (e.g. /World/envs/env_.*/Robot). There is no test that covers a path like /World/envs/env_.*/Robot/.*/link, so the bug being fixed here has no automated guard against regressions. A unit test for make_clone_plan with a two-wildcard path (asserting that destination.format(env_id) produces the expected resolved path and that ClonePlan is populated correctly) would lock in this fix.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
|
@jnskkmhr can you add the missing changelog fragment? :) |
|
@AntoineRichard |
Description
The cloner fails to clone assets whose
prim_pathcontains more than one regex wildcard (.*), which is common for tree-structured USD prims where a wildcard is used both for the environment index and for an inner subtree.For example, given:
make_clone_planbuilds a destination template by replacing every.*with a{}format placeholder:"/World/envs/env_{}/Robot/{}/link"It then formats this template with a single environment index (e.g.
0), which only fills the first{}. The second placeholder is left unfilled, sostr.formatraises and cloning fails.Fixes
Replace only the first
.*with the{}placeholder and leave any remaining.*in place:The leading wildcard (the per-environment index) is substituted with the env id, while the rest of the path keeps its
.*, so the cloner can handle tree-structured prim paths.Type of change
Screenshots
Please attach before and after screenshots of the change if applicable.
Checklist
pre-commitchecks with./isaaclab.sh --formatconfig/extension.tomlfileCONTRIBUTORS.mdor my name already exists there🤖 Generated with Claude Code