Skip to content

Commit 080668d

Browse files
yuvaltassacopybara-github
authored andcommitted
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
1 parent 5637f74 commit 080668d

2 files changed

Lines changed: 56 additions & 5 deletions

File tree

src/user/user_resolver.cc

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ namespace {
3232
// format a numeric value for conflict messages
3333
template <typename T>
3434
std::string fmtVal(T val) {
35-
if constexpr (std::is_same_v<T, mjtNum>) {
35+
if constexpr (std::is_floating_point_v<T>) {
3636
char buf[32];
3737
snprintf(buf, sizeof(buf), "%g", val);
3838
return buf;
@@ -87,8 +87,10 @@ struct Resolver {
8787

8888
// "FIELD: parent has X, child has Y"
8989
auto prefix = [&]() {
90-
return std::string(name) + ": parent has " + fmtVal(pval) +
91-
", child has " + fmtVal(cval);
90+
std::string p_str = fmtVal(pval) + (parent_authored ? "" : " (default)");
91+
std::string c_str = fmtVal(cval) + (child_authored ? "" : " (default)");
92+
return std::string(name) + ": parent has " + p_str +
93+
", child has " + c_str;
9294
};
9395

9496
// only child authored: adopt or keep
@@ -168,8 +170,10 @@ struct Resolver {
168170

169171
// "FIELD: parent has X Y Z, child has X Y Z"
170172
auto prefix = [&]() {
171-
return std::string(name) + ": parent has " + fmtArr(pval, N) +
172-
", child has " + fmtArr(cval, N);
173+
std::string p_str = fmtArr(pval, N) + (parent_authored ? "" : " (default)");
174+
std::string c_str = fmtArr(cval, N) + (child_authored ? "" : " (default)");
175+
return std::string(name) + ": parent has " + p_str +
176+
", child has " + c_str;
173177
};
174178

175179
// only child authored: adopt or keep

test/user/user_resolver_test.cc

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -914,5 +914,52 @@ TEST_F(MujocoTest, AttachWarningBoundaryAndPreservation) {
914914
mj_deleteSpec(child2);
915915
}
916916

917+
TEST_F(MujocoTest, AttachConflictWarningZFarDefaultMessage) {
918+
mock_warning_handler.ExpectWarnings();
919+
920+
static constexpr char parent_xml[] = R"(
921+
<mujoco>
922+
<compiler conflict="warning"/>
923+
<worldbody/>
924+
</mujoco>
925+
)";
926+
927+
static constexpr char child_xml[] = R"(
928+
<mujoco>
929+
<visual>
930+
<map zfar="30"/>
931+
</visual>
932+
<worldbody/>
933+
</mujoco>
934+
)";
935+
936+
std::array<char, 1024> error;
937+
mjSpec* parent =
938+
mj_parseXMLString(parent_xml, nullptr, error.data(), error.size());
939+
ASSERT_THAT(parent, NotNull()) << error.data();
940+
941+
mjSpec* child =
942+
mj_parseXMLString(child_xml, nullptr, error.data(), error.size());
943+
ASSERT_THAT(child, NotNull()) << error.data();
944+
945+
mjsBody* world = mjs_findBody(parent, "world");
946+
mjsElement* attached =
947+
mjs_attach(world->element, child->element, "child_", "");
948+
ASSERT_THAT(attached, NotNull()) << "Error details: " << mjs_getError(parent);
949+
950+
// Since only the child authored 'zfar' and the parent relied on defaults,
951+
// we warning-log and state that the parent has the default value.
952+
EXPECT_TRUE(mjs_isWarning(parent));
953+
EXPECT_EQ(mjs_numWarnings(parent), 1);
954+
EXPECT_THAT(
955+
mjs_getWarning(parent, 0),
956+
HasSubstr("zfar: parent has 50 (default), child has "
957+
"30, keeping parent value"));
958+
959+
mj_deleteSpec(parent);
960+
mj_deleteSpec(child);
961+
}
962+
917963
} // namespace
918964
} // namespace mujoco
965+

0 commit comments

Comments
 (0)