diff --git a/src/user/user_model.cc b/src/user/user_model.cc index 693287fa..fcc0c354 100644 --- a/src/user/user_model.cc +++ b/src/user/user_model.cc @@ -19,6 +19,7 @@ #include #include #include +#include #include #include @@ -842,57 +843,56 @@ void mjCModel::IndexAssets(void) { // if asset name is missing, set to filename -void mjCModel::SetDefaultNames(void) { +template +void mjCModel::SetDefaultNames(std::vector& assets) { string stripped; + std::map> names; - // meshes - for (int i=0; iname.empty()) { - stripped = mjuu_strippath(meshes[i]->file()); - meshes[i]->name = mjuu_stripext(stripped); - - // name cannot be empty - if (meshes[i]->name.empty()) { - throw mjCError(meshes[i], "empty name in mesh"); - } + // use filename if name is missing + for (int i=0; iname.empty()) { + stripped = mjuu_strippath(assets[i]->get_file()); + assets[i]->name = mjuu_stripext(stripped); + names[assets[i]->name].push_back(i); } } - // skins - for (int i=0; iname.empty()) { - stripped = mjuu_strippath(skins[i]->file); - skins[i]->name = mjuu_stripext(stripped); + // add suffix if duplicates + for (auto const& [name, indices] : names) { + if (indices.size() > 1) { + for (int i=0; iname += "_" + std::to_string(i); + } + } + } +} + + + +// throw error if a name is missing +void mjCModel::CheckEmptyNames(void) { + // meshes + for (int i=0; iname.empty()) { + throw mjCError(meshes[i], "empty name in mesh"); } } // hfields for (int i=0; iname.empty()) { - stripped = mjuu_strippath(hfields[i]->file); - hfields[i]->name = mjuu_stripext(stripped); - - // name cannot be empty - if (hfields[i]->name.empty()) { - throw mjCError(hfields[i], "empty name in height field"); - } + throw mjCError(hfields[i], "empty name in height field"); } } // textures for (int i=0; iname.empty()) { - stripped = mjuu_strippath(textures[i]->file); - textures[i]->name = mjuu_stripext(stripped); - - // name cannot be empty, except for skybox - if (textures[i]->name.empty() && textures[i]->type!=mjTEXTURE_SKYBOX) { - throw mjCError(textures[i], "empty name in texture"); - } + if (textures[i]->name.empty() && textures[i]->type!=mjTEXTURE_SKYBOX) { + throw mjCError(textures[i], "empty name in texture"); } } - // materials: name check only + // materials for (int i=0; iname.empty()) { throw mjCError(materials[i], "empty name in material"); @@ -2785,6 +2785,13 @@ void mjCModel::TryCompile(mjModel*& m, mjData*& d, const mjVFS* vfs) { // make lists of objects created in kinematic tree MakeLists(bodies[0]); + // fill missing names and check that they are all filled + SetDefaultNames(meshes); + SetDefaultNames(skins); + SetDefaultNames(hfields); + SetDefaultNames(textures); + CheckEmptyNames(); + // set object ids and default names, check for repeated names processlist(bodies, "body"); processlist(joints, "joint"); @@ -2810,8 +2817,7 @@ void mjCModel::TryCompile(mjModel*& m, mjData*& d, const mjVFS* vfs) { processlist(keys, "key"); processlist(plugins, "plugin"); - // set default names, convert names into indices - SetDefaultNames(); + // convert names into indices IndexAssets(); // mark meshes that need convex hull diff --git a/src/user/user_model.h b/src/user/user_model.h index 77d9d718..f013d832 100644 --- a/src/user/user_model.h +++ b/src/user/user_model.h @@ -176,10 +176,13 @@ class mjCModel { template // add object of any type, with def parameter T* AddObjectDef(std::vector& list, std::string type, mjCDef* def); + template // if asset name is missing, set to filename + void SetDefaultNames(std::vector& assets); + //------------------------ compile phases void MakeLists(mjCBody* body); // make lists of bodies, geoms, joints, sites void IndexAssets(void); // convert asset names into indices - void SetDefaultNames(void); // if mesh or hfield name is missing, set to filename + void CheckEmptyNames(void); // check empty names void SetSizes(void); // compute sizes void AutoSpringDamper(mjModel*);// automatic stiffness and damping computation void LengthRange(mjModel*, mjData*); // compute actuator lengthrange diff --git a/test/user/testdata/load_twice.xml b/test/user/testdata/load_twice.xml new file mode 100644 index 00000000..4e9304a7 --- /dev/null +++ b/test/user/testdata/load_twice.xml @@ -0,0 +1,10 @@ + + + + + + + + + + diff --git a/test/user/user_model_test.cc b/test/user/user_model_test.cc index 3abae496..db1059ef 100644 --- a/test/user/user_model_test.cc +++ b/test/user/user_model_test.cc @@ -14,6 +14,7 @@ // Tests for user/user_model.cc. +#include #include #include @@ -21,6 +22,7 @@ #include #include #include +#include "src/cc/array_safety.h" #include "test/fixture.h" namespace mujoco { @@ -190,6 +192,26 @@ TEST_F(UserDataTest, AutoNUserSensor) { mj_deleteModel(m); } +// ------------- test duplicate names ------------------------------------------ +TEST_F(UserDataTest, DuplicateNames) { + static const char* const kFilePath = "user/testdata/load_twice.xml"; + const std::string xml_path = GetTestDataFilePath(kFilePath); + + std::array error; + mjModel* m = mj_loadXML(xml_path.c_str(), 0, error.data(), error.size()); + + EXPECT_THAT(m, NotNull()) << error.data(); + EXPECT_THAT(m->nmesh, 2); + + for (int i = 0; i < m->nmesh; i++) { + char mesh_name[mjMAXUINAME] = ""; + util::strcat_arr(mesh_name, m->names + m->name_meshadr[i]); + EXPECT_THAT(std::string(mesh_name), "cube_" + std::to_string(i)); + } + + mj_deleteModel(m); +} + // ------------- test fusestatic ----------------------------------------------- using FuseStaticTest = MujocoTest;