From 4b6c07cd7d7c80767439b805cc31ff0947ca7677 Mon Sep 17 00:00:00 2001 From: Alessio Quaglino Date: Wed, 17 Apr 2024 04:06:40 -0700 Subject: [PATCH] Prevent bodies from overwriting the frame names in xml_native_writer. PiperOrigin-RevId: 625632369 Change-Id: If3bd048ac40e116fcb97beb3dca3d99826fe4ad0 --- doc/changelog.rst | 1 + src/xml/xml_native_writer.cc | 18 +++++++++++++----- test/xml/xml_native_writer_test.cc | 7 +++++-- 3 files changed, 19 insertions(+), 7 deletions(-) diff --git a/doc/changelog.rst b/doc/changelog.rst index 5b9a296f..d1c7c1ec 100644 --- a/doc/changelog.rst +++ b/doc/changelog.rst @@ -19,6 +19,7 @@ MJX Bug fixes ^^^^^^^^^ 4. Defaults of lights were not being saved, now fixed. +5. Prevent overwriting of frame names by body names when saving an XML. Bug introduced in 3.1.4. Version 3.1.4 (April 10th, 2024) diff --git a/src/xml/xml_native_writer.cc b/src/xml/xml_native_writer.cc index a2fa4ec6..cfbdfee4 100644 --- a/src/xml/xml_native_writer.cc +++ b/src/xml/xml_native_writer.cc @@ -1454,7 +1454,7 @@ void mjXWriter::Asset(XMLElement* root) { -// recursive body writer +// recursive body and frame writer void mjXWriter::Body(XMLElement* elem, mjCBody* body, mjCFrame* frame) { double unitq[4] = {1, 0, 0, 0}; @@ -1462,14 +1462,14 @@ void mjXWriter::Body(XMLElement* elem, mjCBody* body, mjCFrame* frame) { throw mjXError(0, "missing body in XML write"); // SHOULD NOT OCCUR } - // write frame if classname is defined + // write frame if defined if (frame) { WriteAttrTxt(elem, "name", frame->name); WriteAttrTxt(elem, "childclass", frame->classname); } // write body attributes and inertial - if (body!=model->GetWorld()) { + else if (body!=model->GetWorld()) { WriteAttrTxt(elem, "name", body->name); WriteAttrTxt(elem, "childclass", body->classname); @@ -1537,10 +1537,18 @@ void mjXWriter::Body(XMLElement* elem, mjCBody* body, mjCFrame* frame) { // write frames for (int i=0; iframes.size(); i++) { - if (body->frames[i]->frame != frame) continue; + // skip current frame + if (body->frames[i]->frame != frame) { + continue; + } + + // write frame if named or has defaults if (!body->frames[i]->name.empty() || !body->frames[i]->classname.empty()) { Body(InsertEnd(elem, "frame"), body, body->frames[i]); - } else { + } + + // otherwise skip + else { Body(elem, body, body->frames[i]); } } diff --git a/test/xml/xml_native_writer_test.cc b/test/xml/xml_native_writer_test.cc index 0b5f1dec..32805357 100644 --- a/test/xml/xml_native_writer_test.cc +++ b/test/xml/xml_native_writer_test.cc @@ -736,7 +736,7 @@ TEST_F(XMLWriterTest, WritesFrameDefaults) { - + @@ -752,7 +752,10 @@ TEST_F(XMLWriterTest, WritesFrameDefaults) { EXPECT_THAT(model, NotNull()) << error.data(); std::string saved_xml = SaveAndReadXml(model); EXPECT_THAT(saved_xml, HasSubstr("frame name=\"f1\"")); - EXPECT_THAT(saved_xml, HasSubstr("frame name=\"f2\" childclass=\"dframe\"")); + EXPECT_THAT(saved_xml, HasSubstr("frame name=\"f2\" childclass=\"dframe\">")); + EXPECT_THAT(saved_xml, HasSubstr("geom pos=\"0 2 0\"")); + EXPECT_THAT(saved_xml, HasSubstr("body pos=\"1 1 0\"")); + EXPECT_THAT(saved_xml, HasSubstr("geom pos=\"0 0 1\"")); EXPECT_THAT(saved_xml, Not(HasSubstr(""))); mj_deleteModel(model); }