From 6ec808e2ce3af289ab3ddea6f6628eb11243e245 Mon Sep 17 00:00:00 2001 From: Taylor Howell Date: Mon, 23 Feb 2026 07:38:02 -0800 Subject: [PATCH] Fix attach for spatial tendon. Fixes #3119. PiperOrigin-RevId: 874067521 Change-Id: If8d415278dde2bccf269cc90e8c176e3d7edefa6 --- doc/changelog.rst | 5 ++ src/user/user_objects.cc | 8 ++- test/user/user_api_test.cc | 127 +++++++++++++++++++++++++++++++++++++ 3 files changed, 138 insertions(+), 2 deletions(-) diff --git a/doc/changelog.rst b/doc/changelog.rst index eeb6fab0..21552163 100644 --- a/doc/changelog.rst +++ b/doc/changelog.rst @@ -10,6 +10,11 @@ MJX - Add batch rendering support for MJX-Warp. See the :ref:`MJX-Warp batch rendering` section for details. +Bug fixes +^^^^^^^^^ + +- Fixed a bug where :ref:`mjs_attach` silently dropped spatial tendons with wrapping geometries that had no + ``sidesite`` attribute (:issue:`3119`, reported by :github:user:`tomstewart89`). Version 3.5.0 (February 12, 2026) --------------------------------- diff --git a/src/user/user_objects.cc b/src/user/user_objects.cc index 9dfa3346..4b5ee2a4 100644 --- a/src/user/user_objects.cc +++ b/src/user/user_objects.cc @@ -6267,12 +6267,16 @@ void mjCTendon::ResolveReferences(const mjCModel* m) { try { // look for wrapped element with namespace path[i]->name = prefix + pname + suffix; - path[i]->sidesite = prefix + psidesite + suffix; + if (!psidesite.empty()) { + path[i]->sidesite = prefix + psidesite + suffix; + } path[i]->ResolveReferences(m); } catch(mjCError) { // remove namespace from wrap names path[i]->name = pname; - path[i]->sidesite = psidesite; + if (!psidesite.empty()) { + path[i]->sidesite = psidesite; + } path[i]->ResolveReferences(m); nfailure++; } diff --git a/test/user/user_api_test.cc b/test/user/user_api_test.cc index 73e36a48..e9f519e5 100644 --- a/test/user/user_api_test.cc +++ b/test/user/user_api_test.cc @@ -1079,6 +1079,133 @@ TEST_F(MujocoTest, AttachSame) { mj_deleteModel(m_expected); } +TEST_F(MujocoTest, AttachSpatialTendonWithoutSidesite) { + static constexpr char xml_parent[] = R"( + + + + + + + )"; + + static constexpr char xml_child[] = R"( + + + + + + + + + + + + + + + + + + + + + + )"; + + std::array er; + mjSpec* parent = mj_parseXMLString(xml_parent, 0, er.data(), er.size()); + ASSERT_THAT(parent, NotNull()) << er.data(); + mjSpec* child = mj_parseXMLString(xml_child, 0, er.data(), er.size()); + ASSERT_THAT(child, NotNull()) << er.data(); + + mjsBody* parent_body = mjs_findBody(parent, "parent_body"); + ASSERT_THAT(parent_body, NotNull()); + mjsSite* attach_site = mjs_addSite(parent_body, 0); + mjs_setName(attach_site->element, "attach_site"); + + mjs_attach(attach_site->element, + mjs_findBody(child, "child_body")->element, "", "_child"); + + EXPECT_THAT(mjs_findElement(parent, mjOBJ_TENDON, + "tendon_with_sidesite_child"), NotNull()); + EXPECT_THAT(mjs_findElement(parent, mjOBJ_TENDON, + "tendon_without_sidesite_child"), NotNull()); + + mjModel* model = mj_compile(parent, nullptr); + ASSERT_THAT(model, NotNull()) << mjs_getError(parent); + EXPECT_EQ(model->ntendon, 2); + + mj_deleteModel(model); + mj_deleteSpec(parent); + mj_deleteSpec(child); +} + +TEST_F(MujocoTest, AttachSpatialTendonGitHubIssue3119) { + static constexpr char parent_xml[] = R"( + + + + + + + )"; + + static constexpr char child_xml[] = R"( + + + + + + + + + + + + + + + + + + + + + + )"; + + std::array er; + mjSpec* parent_spec = + mj_parseXMLString(parent_xml, 0, er.data(), er.size()); + ASSERT_THAT(parent_spec, NotNull()) << er.data(); + mjSpec* child_spec = + mj_parseXMLString(child_xml, 0, er.data(), er.size()); + ASSERT_THAT(child_spec, NotNull()) << er.data(); + + mjsBody* parent_body = mjs_findBody(parent_spec, "parent_body"); + ASSERT_THAT(parent_body, NotNull()); + mjsSite* attach_site = mjs_addSite(parent_body, 0); + mjs_setName(attach_site->element, "attach_site"); + + mjs_attach(attach_site->element, + mjs_findBody(child_spec, "child_body")->element, + "", "_child"); + + EXPECT_THAT(mjs_findElement(parent_spec, mjOBJ_TENDON, + "tendon_with_sidesite_child"), NotNull()); + EXPECT_THAT(mjs_findElement(parent_spec, mjOBJ_TENDON, + "tendon_without_sidesite_child"), NotNull()); + + mjModel* model = mj_compile(parent_spec, nullptr); + ASSERT_THAT(model, NotNull()) << mjs_getError(parent_spec); + EXPECT_EQ(model->ntendon, 2); + + mj_deleteModel(model); + mj_deleteSpec(parent_spec); + mj_deleteSpec(child_spec); +} + TEST_F(MujocoTest, AttachDifferent) { std::array er; mjtNum tol = 0;