Skip to content

Fixes cloner to skip cloning the source environment.#4731

Open
ooctipus wants to merge 2 commits intoisaac-sim:developfrom
ooctipus:fix/develop/self_clone_bug
Open

Fixes cloner to skip cloning the source environment.#4731
ooctipus wants to merge 2 commits intoisaac-sim:developfrom
ooctipus:fix/develop/self_clone_bug

Conversation

@ooctipus
Copy link
Collaborator

@ooctipus ooctipus commented Feb 25, 2026

Description

Cloning the source environment can leads to some unexpected bug, this PR skips the cloning source environment during usd_replicate and physx_replicate

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Screenshots

Please attach before and after screenshots of the change if applicable.

Checklist

  • I have read and understood the contribution guidelines
  • I have run the pre-commit checks with ./isaaclab.sh --format
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • I have updated the changelog and the corresponding version in the extension's config/extension.toml file
  • I have added my name to the CONTRIBUTORS.md or my name already exists there

@github-actions github-actions bot added bug Something isn't working isaac-lab Related to Isaac Lab team labels Feb 25, 2026
@ooctipus ooctipus force-pushed the fix/develop/self_clone_bug branch from 84cbd3f to 01f0449 Compare February 25, 2026 22:14
@ooctipus ooctipus marked this pull request as ready for review February 25, 2026 22:15
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Feb 25, 2026

Greptile Summary

This PR fixes a critical bug where cloning environments could cause self-replication when the source environment path matches a destination path. The fix prevents Sdf.CopySpec and PhysX replication from copying a prim onto itself, which would create duplicate PhysX bodies and cause erratic joint behavior.

Key Changes:

  • usd_replicate: Added explicit guard to skip Sdf.CopySpec when src == dp
  • physx_replicate: Extracts environment ID from source path using template structure and filters it from target worlds list
  • Added comprehensive tests with parametrized cases covering self-world exclusion scenarios
  • Properly documented in changelog with version bump to 4.2.1

The implementation is clean, well-tested, and follows semantic versioning conventions.

Confidence Score: 5/5

  • This PR is safe to merge with minimal risk
  • The fix directly addresses a specific bug with clear logic. Tests provide comprehensive coverage of the edge cases, including self-world scenarios and external templates. The changes are minimal, focused, and properly documented.
  • No files require special attention

Important Files Changed

Filename Overview
source/isaaclab/isaaclab/cloner/cloner_utils.py Added guard to skip Sdf.CopySpec when source equals destination, preventing redundant self-copy operations
source/isaaclab_physx/isaaclab_physx/cloner/physx_replicate.py Extracts environment ID from source path and filters it from target worlds to prevent PhysX body duplication
source/isaaclab/test/sim/test_cloner.py Added comprehensive parametrized tests verifying self-copy prevention in both USD and PhysX replication paths

Last reviewed commit: 01f0449

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

5 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

@ooctipus ooctipus force-pushed the fix/develop/self_clone_bug branch 2 times, most recently from 3626bfe to d4a814f Compare February 26, 2026 02:17
@ooctipus ooctipus force-pushed the fix/develop/self_clone_bug branch from d4a814f to 439e197 Compare February 26, 2026 02:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working isaac-lab Related to Isaac Lab team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant