Avoid changing semantics of mjsPlugin.name across XML sections.

The `extension` section used to discard the plugin name and write the instance name in `plugin.name`, while all of the other sections wrote the instance name in `plugin.instance_name` and optionally the plugin name in `plugin.name` (via the function `OnePlugin`). Now all plugin names are in `plugin.plugin_name` and the instance is written in `plugin.name`, so that it is possible to lookup a plugin via its unique instance name.

PiperOrigin-RevId: 683183804
Change-Id: I179ef5d221143b720db24d745b09e327bf1449ef

doc/includes/references.h

@@ -1740,7 +1740,7 @@ typedef struct mjsOrientation_ {   // alternative orientation specifiers
 typedef struct mjsPlugin_ {        // plugin specification
   mjsElement* element;             // element type
   mjString* name;                  // name
-  mjString* instance_name;         // instance name
+  mjString* plugin_name;           // plugin name
   int plugin_slot;                 // global registered slot number of the plugin
   mjtByte active;                  // is the plugin active
   mjString* info;                  // message appended to compiler errors

include/mujoco/mjspec.h

@@ -183,7 +183,7 @@ typedef struct mjsOrientation_ {   // alternative orientation specifiers
 typedef struct mjsPlugin_ {        // plugin specification
   mjsElement* element;             // element type
   mjString* name;                  // name
-  mjString* instance_name;         // instance name
+  mjString* plugin_name;           // plugin name
   int plugin_slot;                 // global registered slot number of the plugin
   mjtByte active;                  // is the plugin active
   mjString* info;                  // message appended to compiler errors

introspect/structs.py

@@ -8579,11 +8579,11 @@ STRUCTS: Mapping[str, StructDecl] = dict([
                  doc='name',
              ),
              StructFieldDecl(
-                 name='instance_name',
+                 name='plugin_name',
                  type=PointerType(
                      inner_type=ValueType(name='mjString'),
                  ),
-                 doc='instance name',
+                 doc='plugin name',
              ),
              StructFieldDecl(
                  name='plugin_slot',

python/mujoco/codegen/generate_spec_bindings.py

@@ -457,7 +457,7 @@ def generate_add() -> None:
                   throw pybind11::value_error("plugin.name should be a string.");
                 }
                 try {
-                  plugin.instance_name = input->instance_name;
+                  plugin.plugin_name = input->plugin_name;
                 } catch (const py::cast_error &e) {
                   throw pybind11::value_error("plugin.instance_name should be a string.");
                 }

python/mujoco/specs_test.py

@@ -205,14 +205,14 @@ class SpecsTest(absltest.TestCase):
 
     # Add plugin.
     plugin = spec.add_plugin(
-        name='name',
-        instance_name='instance',
+        name='instance_name',
+        plugin_name='mujoco.plugin',
         plugin_slot=7,
         active=True,
         info='info',
     )
-    self.assertEqual(plugin.name, 'name')
-    self.assertEqual(plugin.instance_name, 'instance')
+    self.assertEqual(plugin.name, 'instance_name')
+    self.assertEqual(plugin.plugin_name, 'mujoco.plugin')
     self.assertEqual(plugin.plugin_slot, 7)
     self.assertEqual(plugin.active, True)
     self.assertEqual(plugin.info, 'info')
@@ -227,8 +227,8 @@ class SpecsTest(absltest.TestCase):
 
     # Add a body with a plugin.
     body_with_plugin = spec.worldbody.add_body(plugin=plugin)
-    self.assertEqual(body_with_plugin.plugin.name, 'name')
-    self.assertEqual(body_with_plugin.plugin.instance_name, 'instance')
+    self.assertEqual(body_with_plugin.plugin.name, 'instance_name')
+    self.assertEqual(body_with_plugin.plugin.plugin_name, 'mujoco.plugin')
     self.assertEqual(body_with_plugin.plugin.plugin_slot, 7)
     self.assertEqual(body_with_plugin.plugin.active, True)
     self.assertEqual(body_with_plugin.plugin.info, 'info')
@@ -731,7 +731,7 @@ class SpecsTest(absltest.TestCase):
     self.assertIsNotNone(spec.worldbody)
 
     body = spec.worldbody.add_body()
-    body.plugin.name = 'mujoco.elasticity.cable'
+    body.plugin.plugin_name = 'mujoco.elasticity.cable'
     body.plugin.id = spec.add_plugin()
     body.plugin.active = True
     self.assertEqual(body.plugin.id, 0)

src/user/user_composite.cc

@@ -67,8 +67,8 @@ mjCComposite::mjCComposite(void) {
   mjs_defaultPlugin(&plugin);
   plugin_name = "";
   plugin_instance_name = "";
-  plugin.name = (mjString*)&plugin_name;
-  plugin.instance_name = (mjString*)&plugin_instance_name;
+  plugin.plugin_name = (mjString*)&plugin_name;
+  plugin.name = (mjString*)&plugin_instance_name;
 
   // cable
   curve[0] = curve[1] = curve[2] = mjCOMPSHAPE_ZERO;
@@ -715,8 +715,8 @@ mjsBody* mjCComposite::AddCableBody(mjCModel* model, mjsBody* body, int ix,
     mjsPlugin* pplugin = &body->plugin;
     pplugin->active = true;
     pplugin->element = plugin.element;
-    mjs_setString(pplugin->name, mjs_getString(plugin.name));
-    mjs_setString(pplugin->instance_name, plugin_instance_name.c_str());
+    mjs_setString(pplugin->plugin_name, mjs_getString(plugin.plugin_name));
+    mjs_setString(pplugin->name, plugin_instance_name.c_str());
   }
 
   // update orientation

src/user/user_flexcomp.cc

@@ -91,8 +91,8 @@ mjCFlexcomp::mjCFlexcomp(void) {
   mjs_defaultOrientation(&alt);
   plugin_name = "";
   plugin_instance_name = "";
-  plugin.name = (mjString*)&plugin_name;
-  plugin.instance_name = (mjString*)&plugin_instance_name;
+  plugin.plugin_name = (mjString*)&plugin_name;
+  plugin.name = (mjString*)&plugin_instance_name;
 }
 
 
@@ -441,8 +441,8 @@ bool mjCFlexcomp::Make(mjSpec* spec, mjsBody* body, char* error, int error_sz) {
         mjsPlugin* pplugin = &body->plugin;
         pplugin->active = true;
         pplugin->element = static_cast<mjsElement*>(plugin.element);
-        mjs_setString(pplugin->name, mjs_getString(plugin.name));
-        mjs_setString(pplugin->instance_name, plugin_instance_name.c_str());
+        mjs_setString(pplugin->plugin_name, mjs_getString(plugin.plugin_name));
+        mjs_setString(pplugin->name, plugin_instance_name.c_str());
       }
     }
 
@@ -505,8 +505,8 @@ bool mjCFlexcomp::Make(mjSpec* spec, mjsBody* body, char* error, int error_sz) {
         mjsPlugin* pplugin = &pb->plugin;
         pplugin->active = true;
         pplugin->element = static_cast<mjsElement*>(plugin.element);
-        mjs_setString(pplugin->name, mjs_getString(plugin.name));
-        mjs_setString(pplugin->instance_name, plugin_instance_name.c_str());
+        mjs_setString(pplugin->plugin_name, mjs_getString(plugin.plugin_name));
+        mjs_setString(pplugin->name, plugin_instance_name.c_str());
       }
     }
   }

src/user/user_mesh.cc

@@ -204,8 +204,8 @@ void mjCMesh::PointToLocal() {
   spec.userface = &spec_face_;
   spec.usertexcoord = &spec_texcoord_;
   spec.userfacetexcoord = &spec_facetexcoord_;
-  spec.plugin.name = &plugin_name;
-  spec.plugin.instance_name = &plugin_instance_name;
+  spec.plugin.plugin_name = &plugin_name;
+  spec.plugin.name = &plugin_instance_name;
   spec.info = &info;
   file = nullptr;
   content_type = nullptr;
@@ -249,8 +249,8 @@ void mjCMesh::CopyFromSpec() {
   maxhullvert_ = spec.maxhullvert;
   plugin.active = spec.plugin.active;
   plugin.element = spec.plugin.element;
+  plugin.plugin_name = spec.plugin.plugin_name;
   plugin.name = spec.plugin.name;
-  plugin.instance_name = spec.plugin.instance_name;
 
   // clear precompiled asset. TODO: use asset cache
   if (center_) mju_free(center_);
@@ -271,7 +271,7 @@ void mjCMesh::CopyFromSpec() {
 mjCMesh::~mjCMesh() {
   if (center_) mju_free(center_);
   if (graph_) mju_free(graph_);
-  if (spec.plugin.active && spec.plugin.instance_name->empty()) {
+  if (spec.plugin.active && spec.plugin.name->empty()) {
     model->DeleteElement(spec.plugin.element);
   }
 }

src/user/user_model.cc

@@ -311,7 +311,7 @@ void mjCModel::CopyPlugin(std::vector<mjCPlugin*>& dest,
 
   // only copy plugins that are referenced
   for (const auto& plugin : source) {
-    if (plugin->instance_name.empty() && plugin->model == this) {
+    if (plugin->name.empty() && plugin->model == this) {
       continue;
     }
     mjCPlugin* candidate = new mjCPlugin(*plugin);

src/user/user_objects.cc

@@ -957,8 +957,8 @@ void mjCBody::PointToLocal() {
   spec.name = &name;
   spec.childclass = &classname;
   spec.userdata = &spec_userdata_;
-  spec.plugin.name = &plugin_name;
-  spec.plugin.instance_name = (&plugin_instance_name);
+  spec.plugin.plugin_name = &plugin_name;
+  spec.plugin.name = (&plugin_instance_name);
   spec.info = &info;
   userdata = nullptr;
 }
@@ -969,8 +969,8 @@ void mjCBody::CopyFromSpec() {
   userdata_ = spec_userdata_;
   plugin.active = spec.plugin.active;
   plugin.element = spec.plugin.element;
+  plugin.plugin_name = spec.plugin.plugin_name;
   plugin.name = spec.plugin.name;
-  plugin.instance_name = spec.plugin.instance_name;
 }
 
 
@@ -994,7 +994,7 @@ mjCBody::~mjCBody() {
   cameras.clear();
   lights.clear();
 
-  if (spec.plugin.active && spec.plugin.instance_name->empty()) {
+  if (spec.plugin.active && spec.plugin.name->empty()) {
     model->DeleteElement(spec.plugin.element);
   }
 }
@@ -2161,7 +2161,7 @@ mjCGeom::mjCGeom(const mjCGeom& other) {
 
 
 mjCGeom::~mjCGeom() {
-  if (spec.plugin.active && spec.plugin.instance_name->empty()) {
+  if (spec.plugin.active && spec.plugin.name->empty()) {
     model->DeleteElement(spec.plugin.element);
   }
 }
@@ -2189,8 +2189,8 @@ void mjCGeom::PointToLocal(void) {
   spec.material = &spec_material_;
   spec.meshname = &spec_meshname_;
   spec.hfieldname = &spec_hfieldname_;
-  spec.plugin.name = &plugin_name;
-  spec.plugin.instance_name = &plugin_instance_name;
+  spec.plugin.plugin_name = &plugin_name;
+  spec.plugin.name = &plugin_instance_name;
   userdata = nullptr;
   hfieldname = nullptr;
   meshname = nullptr;
@@ -2207,8 +2207,8 @@ void mjCGeom::CopyFromSpec() {
   material_ = spec_material_;
   plugin.active = spec.plugin.active;
   plugin.element = spec.plugin.element;
+  plugin.plugin_name = spec.plugin.plugin_name;
   plugin.name = spec.plugin.name;
-  plugin.instance_name = spec.plugin.instance_name;
 }
 
 
@@ -5586,7 +5586,7 @@ mjCActuator::mjCActuator(const mjCActuator& other) {
 
 
 mjCActuator::~mjCActuator() {
-  if (spec.plugin.active && spec.plugin.instance_name->empty()) {
+  if (spec.plugin.active && spec.plugin.name->empty()) {
     model->DeleteElement(spec.plugin.element);
   }
 }
@@ -5644,8 +5644,8 @@ void mjCActuator::PointToLocal() {
   spec.target = &spec_target_;
   spec.refsite = &spec_refsite_;
   spec.slidersite = &spec_slidersite_;
-  spec.plugin.name = &plugin_name;
-  spec.plugin.instance_name = &plugin_instance_name;
+  spec.plugin.plugin_name = &plugin_name;
+  spec.plugin.name = &plugin_instance_name;
   spec.info = &info;
   userdata = nullptr;
   target = nullptr;
@@ -5677,8 +5677,8 @@ void mjCActuator::CopyFromSpec() {
   slidersite_ = spec_slidersite_;
   plugin.active = spec.plugin.active;
   plugin.element = spec.plugin.element;
+  plugin.plugin_name = spec.plugin.plugin_name;
   plugin.name = spec.plugin.name;
-  plugin.instance_name = spec.plugin.instance_name;
 }
 
 
@@ -5951,7 +5951,7 @@ mjCSensor::mjCSensor(const mjCSensor& other) {
 
 
 mjCSensor::~mjCSensor() {
-  if (spec.plugin.active && spec.plugin.instance_name->empty()) {
+  if (spec.plugin.active && spec.plugin.name->empty()) {
     model->DeleteElement(spec.plugin.element);
   }
 }
@@ -5978,8 +5978,8 @@ void mjCSensor::PointToLocal() {
   spec.userdata = &spec_userdata_;
   spec.objname = &spec_objname_;
   spec.refname = &spec_refname_;
-  spec.plugin.name = &plugin_name;
-  spec.plugin.instance_name = &plugin_instance_name;
+  spec.plugin.plugin_name = &plugin_name;
+  spec.plugin.name = &plugin_instance_name;
   spec.info = &info;
   userdata = nullptr;
   objname = nullptr;
@@ -6008,8 +6008,8 @@ void mjCSensor::CopyFromSpec() {
   refname_ = spec_refname_;
   plugin.active = spec.plugin.active;
   plugin.element = spec.plugin.element;
+  plugin.plugin_name = spec.plugin.plugin_name;
   plugin.name = spec.plugin.name;
-  plugin.instance_name = spec.plugin.instance_name;
 }
 
 
@@ -6921,13 +6921,13 @@ mjCPlugin::mjCPlugin(mjCModel* _model) {
   parent = this;
   model = _model;
   name.clear();
-  instance_name.clear();
+  plugin_name.clear();
 
   // public interface
   mjs_defaultPlugin(&spec);
   elemtype = mjOBJ_PLUGIN;
+  spec.plugin_name = &plugin_name;
   spec.name = &name;
-  spec.instance_name = &instance_name;
   spec.info = &info;
 }
 
@@ -6951,15 +6951,6 @@ mjCPlugin& mjCPlugin::operator=(const mjCPlugin& other) {
 
 
 
-void mjCPlugin::NameSpace(const mjCModel* m) {
-  mjCBase::NameSpace(m);
-  if (!instance_name.empty()) {
-    instance_name = m->prefix + instance_name + m->suffix;
-  }
-}
-
-
-
 // compiler
 void mjCPlugin::Compile(void) {
   const mjpPlugin* plugin = mjp_getPluginAtSlot(spec.plugin_slot);

src/user/user_objects.h

@@ -1412,7 +1412,7 @@ class mjCPlugin_ : public mjCBase {
   std::vector<char> flattened_attributes;  // config attributes flattened in plugin-declared order;
 
  protected:
-  std::string instance_name;
+  std::string plugin_name;
 };
 
 class mjCPlugin : public mjCPlugin_ {
@@ -1428,7 +1428,6 @@ class mjCPlugin : public mjCPlugin_ {
 
  private:
   void Compile(void);              // compiler
-  void NameSpace(const mjCModel* m);
 };
 
 

src/xml/xml_native_reader.cc

@@ -2769,8 +2769,8 @@ void mjXReader::OnePlugin(XMLElement* elem, mjsPlugin* plugin) {
   string instance_name = "";
   ReadAttrTxt(elem, "plugin", name);
   ReadAttrTxt(elem, "instance", instance_name);
-  mjs_setString(plugin->name, name.c_str());
-  mjs_setString(plugin->instance_name, instance_name.c_str());
+  mjs_setString(plugin->plugin_name, name.c_str());
+  mjs_setString(plugin->name, instance_name.c_str());
   if (instance_name.empty()) {
     plugin->element = mjs_addPlugin(spec)->element;
     ReadPluginConfigs(elem, plugin);
@@ -2901,6 +2901,7 @@ void mjXReader::Extension(XMLElement* section) {
           }
           string name;
           mjsPlugin* p = mjs_addPlugin(spec);
+          mjs_setString(p->plugin_name, plugin_name.c_str());
           mjs_setString(p->info, ("line " + std::to_string(elem->GetLineNum())).c_str());
           ReadAttrTxt(child, "name", name, /* required = */ true);
           mjs_setString(p->name, name.c_str());

src/xml/xml_native_writer.cc

@@ -820,8 +820,8 @@ void mjXWriter::OneActuator(XMLElement* elem, const mjCActuator* actuator, mjCDe
 
 // write plugin
 void mjXWriter::OnePlugin(XMLElement* elem, const mjsPlugin* plugin) {
-  const string instance_name = string(mjs_getString(plugin->instance_name));
-  const string plugin_name = string(mjs_getString(plugin->name));
+  const string instance_name = string(mjs_getString(plugin->name));
+  const string plugin_name = string(mjs_getString(plugin->plugin_name));
   if (!instance_name.empty()) {
     WriteAttrTxt(elem, "instance", instance_name);
   } else {

test/user/user_api_test.cc

@@ -170,7 +170,7 @@ TEST_F(PluginTest, ActivatePlugin) {
 
   // associate plugin to body
   mjsBody* body = mjs_addBody(mjs_findBody(spec, "world"), 0);
-  mjs_setString(body->plugin.name, "mujoco.elasticity.cable");
+  mjs_setString(body->plugin.plugin_name, "mujoco.elasticity.cable");
   body->plugin.element = mjs_addPlugin(spec)->element;
   body->plugin.active = true;
   mjsGeom* geom = mjs_addGeom(body, 0);
@@ -204,7 +204,7 @@ TEST_F(PluginTest, DeletePlugin) {
   // add actuator
   mjsActuator* actuator = mjs_addActuator(spec, 0);
   mjs_setString(actuator->target, "j1");
-  mjs_setString(actuator->plugin.name, "mujoco.pid");
+  mjs_setString(actuator->plugin.plugin_name, "mujoco.pid");
   actuator->plugin.element = mjs_addPlugin(spec)->element;
   actuator->plugin.active = true;
   actuator->trntype = mjTRN_JOINT;