Skip to content

Fix ResolveReferences for Empty Sidesites#3125

Closed
tomstewart89 wants to merge 1 commit intogoogle-deepmind:mainfrom
tomstewart89:main
Closed

Fix ResolveReferences for Empty Sidesites#3125
tomstewart89 wants to merge 1 commit intogoogle-deepmind:mainfrom
tomstewart89:main

Conversation

@tomstewart89
Copy link

Hello! This PR should fix the issue I raised over here: #3119

The Problem

  • Define a model containing a spatial tendon with a wrapping geom that doesn't specify a sidesite
  • Attach that model to another using mjs_attach and specify a prefix and/or suffix
  • Execution finds its way to mjCTendon::ResolveReferences where the sidesite is named prefix + "" + suffix
  • mjCWrap::ResolveReferences tries to find the site prefix + "" + suffix, fails and throws.
  • That exception is caught in mjCModel::CopyList and the tendon is skipped due to this continue statement

The Fix

Don't apply the prefix and suffix when a spatial tendon's sidesite is empty. That'll let mjCTendon::ResolveReferences know that it need not search for the site.

To add some test coverage for this, I also added a spatial tendon to the xml_child used by MujocoTest.AttachSame, MujocoTest.AttachDifferent and MujocoTest.AttachFrame in user_api_test.cc. These tests should all fail without the change added to user_objects.cc

@google-cla
Copy link

google-cla bot commented Feb 21, 2026

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@tomstewart89
Copy link
Author

This fix is now implemented with 6ec808e so I'll go ahead and close this.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant