Add suffix to duplicated assets.

PiperOrigin-RevId: 588775467
Change-Id: I3a0d2a14601cf018b0c8c56728bd3ae22adb8ec2

src/user/user_model.cc

@@ -19,6 +19,7 @@
 #include <cstddef>
 #include <cstdlib>
 #include <cstring>
+#include <map>
 #include <string>
 #include <vector>
 
@@ -842,57 +843,56 @@ void mjCModel::IndexAssets(void) {
 
 
 // if asset name is missing, set to filename
-void mjCModel::SetDefaultNames(void) {
+template <typename T>
+void mjCModel::SetDefaultNames(std::vector<T*>& assets) {
   string stripped;
+  std::map<string, std::vector<int>> names;
 
-  // meshes
+  // use filename if name is missing
-  for (int i=0; i<meshes.size(); i++) {
+  for (int i=0; i<assets.size(); i++) {
-    if (meshes[i]->name.empty()) {
+    if (assets[i]->name.empty()) {
-      stripped = mjuu_strippath(meshes[i]->file());
+      stripped = mjuu_strippath(assets[i]->get_file());
-      meshes[i]->name = mjuu_stripext(stripped);
+      assets[i]->name = mjuu_stripext(stripped);
-
+      names[assets[i]->name].push_back(i);
-      // name cannot be empty
-      if (meshes[i]->name.empty()) {
-        throw mjCError(meshes[i], "empty name in mesh");
-      }
     }
   }
 
-  // skins
+  // add suffix if duplicates
-  for (int i=0; i<skins.size(); i++) {
+  for (auto const& [name, indices] : names) {
-    if (skins[i]->name.empty()) {
+    if (indices.size() > 1) {
-      stripped = mjuu_strippath(skins[i]->file);
+      for (int i=0; i<indices.size(); i++) {
-      skins[i]->name = mjuu_stripext(stripped);
+        assets[indices[i]]->name += "_" + std::to_string(i);
+      }
+    }
+  }
+}
+
+
+
+// throw error if a name is missing
+void mjCModel::CheckEmptyNames(void) {
+  // meshes
+  for (int i=0; i<meshes.size(); i++) {
+    if (meshes[i]->name.empty()) {
+      throw mjCError(meshes[i], "empty name in mesh");
     }
   }
 
   // hfields
   for (int i=0; i<hfields.size(); i++) {
     if (hfields[i]->name.empty()) {
-      stripped = mjuu_strippath(hfields[i]->file);
+      throw mjCError(hfields[i], "empty name in height field");
-      hfields[i]->name = mjuu_stripext(stripped);
-
-      // name cannot be empty
-      if (hfields[i]->name.empty()) {
-        throw mjCError(hfields[i], "empty name in height field");
-      }
     }
   }
 
   // textures
   for (int i=0; i<textures.size(); i++) {
-    if (textures[i]->name.empty()) {
+    if (textures[i]->name.empty() && textures[i]->type!=mjTEXTURE_SKYBOX) {
-      stripped = mjuu_strippath(textures[i]->file);
+      throw mjCError(textures[i], "empty name in texture");
-      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");
-      }
     }
   }
 
-  // materials: name check only
+  // materials
   for (int i=0; i<materials.size(); i++) {
     if (materials[i]->name.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
+  // convert names into indices
-  SetDefaultNames();
   IndexAssets();
 
   // mark meshes that need convex hull

src/user/user_model.h

@@ -176,10 +176,13 @@ class mjCModel {
   template <class T>              // add object of any type, with def parameter
   T* AddObjectDef(std::vector<T*>& list, std::string type, mjCDef* def);
 
+  template<class T>              // if asset name is missing, set to filename
+  void SetDefaultNames(std::vector<T*>& 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

test/user/testdata/load_twice.xml

@@ -0,0 +1,10 @@
+<mujoco>
+  <asset>
+    <mesh file="cube.obj"/>
+    <mesh file="cube.obj"/>
+  </asset>
+  <worldbody>
+    <geom type="mesh" mesh="cube_0"/>
+    <geom type="mesh" mesh="cube_1"/>
+  </worldbody>
+</mujoco>

test/user/user_model_test.cc

@@ -14,6 +14,7 @@
 
 // Tests for user/user_model.cc.
 
+#include <array>
 #include <string>
 
 #include <gmock/gmock.h>
@@ -21,6 +22,7 @@
 #include <absl/strings/str_format.h>
 #include <mujoco/mjmodel.h>
 #include <mujoco/mujoco.h>
+#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<char, 1024> 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;