From 080668df42e2258b5c84f9630816f0a43bdcb562 Mon Sep 17 00:00:00 2001 From: Yuval Tassa Date: Tue, 23 Jun 2026 09:43:38 -0700 Subject: [PATCH] Add "(default)" to conflict warnings in user resolver. When reporting conflicts between parent and child elements during attachment, the warning message now indicates if a value is derived from the default rather than being explicitly authored in the XML. This improves clarity in conflict resolution warnings. PiperOrigin-RevId: 936724265 Change-Id: I68c677b19020c19bf0260f5b3b77fe6ca3661bb9 --- src/user/user_resolver.cc | 14 ++++++---- test/user/user_resolver_test.cc | 47 +++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 5 deletions(-) diff --git a/src/user/user_resolver.cc b/src/user/user_resolver.cc index 621afc26..965887c6 100644 --- a/src/user/user_resolver.cc +++ b/src/user/user_resolver.cc @@ -32,7 +32,7 @@ namespace { // format a numeric value for conflict messages template std::string fmtVal(T val) { - if constexpr (std::is_same_v) { + if constexpr (std::is_floating_point_v) { char buf[32]; snprintf(buf, sizeof(buf), "%g", val); return buf; @@ -87,8 +87,10 @@ struct Resolver { // "FIELD: parent has X, child has Y" auto prefix = [&]() { - return std::string(name) + ": parent has " + fmtVal(pval) + - ", child has " + fmtVal(cval); + std::string p_str = fmtVal(pval) + (parent_authored ? "" : " (default)"); + std::string c_str = fmtVal(cval) + (child_authored ? "" : " (default)"); + return std::string(name) + ": parent has " + p_str + + ", child has " + c_str; }; // only child authored: adopt or keep @@ -168,8 +170,10 @@ struct Resolver { // "FIELD: parent has X Y Z, child has X Y Z" auto prefix = [&]() { - return std::string(name) + ": parent has " + fmtArr(pval, N) + - ", child has " + fmtArr(cval, N); + std::string p_str = fmtArr(pval, N) + (parent_authored ? "" : " (default)"); + std::string c_str = fmtArr(cval, N) + (child_authored ? "" : " (default)"); + return std::string(name) + ": parent has " + p_str + + ", child has " + c_str; }; // only child authored: adopt or keep diff --git a/test/user/user_resolver_test.cc b/test/user/user_resolver_test.cc index 7bbcf346..742d91a6 100644 --- a/test/user/user_resolver_test.cc +++ b/test/user/user_resolver_test.cc @@ -914,5 +914,52 @@ TEST_F(MujocoTest, AttachWarningBoundaryAndPreservation) { mj_deleteSpec(child2); } +TEST_F(MujocoTest, AttachConflictWarningZFarDefaultMessage) { + mock_warning_handler.ExpectWarnings(); + + static constexpr char parent_xml[] = R"( + + + + + )"; + + static constexpr char child_xml[] = R"( + + + + + + + )"; + + std::array error; + mjSpec* parent = + mj_parseXMLString(parent_xml, nullptr, error.data(), error.size()); + ASSERT_THAT(parent, NotNull()) << error.data(); + + mjSpec* child = + mj_parseXMLString(child_xml, nullptr, error.data(), error.size()); + ASSERT_THAT(child, NotNull()) << error.data(); + + mjsBody* world = mjs_findBody(parent, "world"); + mjsElement* attached = + mjs_attach(world->element, child->element, "child_", ""); + ASSERT_THAT(attached, NotNull()) << "Error details: " << mjs_getError(parent); + + // Since only the child authored 'zfar' and the parent relied on defaults, + // we warning-log and state that the parent has the default value. + EXPECT_TRUE(mjs_isWarning(parent)); + EXPECT_EQ(mjs_numWarnings(parent), 1); + EXPECT_THAT( + mjs_getWarning(parent, 0), + HasSubstr("zfar: parent has 50 (default), child has " + "30, keeping parent value")); + + mj_deleteSpec(parent); + mj_deleteSpec(child); +} + } // namespace } // namespace mujoco +