Replace mjs_detachBody and mjs_detachDefault with mjs_detach.

In a follow-up, mjs_detach will be changed to only remove without deleting an element.

PiperOrigin-RevId: 773637351
Change-Id: I9ee4d3584ba88ccc225591337cf036bab806e25c

doc/APIreference/functions.rst

@@ -3783,23 +3783,14 @@ Attachment
 
 Attach child to a parent, return the attached element if success or NULL otherwise.
 
-.. _mjs_detachBody:
+.. _mjs_detach:
 
-`mjs_detachBody <#mjs_detachBody>`__
+`mjs_detach <#mjs_detach>`__
-~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
-.. mujoco-include:: mjs_detachBody
+.. mujoco-include:: mjs_detach
 
-Delete body and descendants from mjSpec, remove all references, return 0 on success.
+Detach but not delete object corresponding to the given element, return 0 on success.
-
-.. _mjs_detachDefault:
-
-`mjs_detachDefault <#mjs_detachDefault>`__
-~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-
-.. mujoco-include:: mjs_detachDefault
-
-Delete default class and descendants from mjSpec, remove all references, return 0 on success.
 
 .. _AddTreeElements:
 

doc/changelog.rst

@@ -5,6 +5,13 @@ Changelog
 Upcoming version (not yet released)
 -----------------------------------
 
+.. admonition:: Breaking API changes
+   :class: attention
+
+   - The functions ``mjs_detachBody`` and ``mjs_detachDefault`` have been replaced by :ref:`mjs_delete`. Additionally,
+     `:ref:mjs_detach` was added in order to remove an element from the spec without deleting it.
+   - The Python functions ``element.delete`` have been replaced by ``spec.delete(element)``.
+
 Bug fixes
 ^^^^^^^^^
 - Inverse dynamics were not being computed correctly when :ref:`tendon armature<tendon-spatial-armature>` was present,

doc/includes/references.h

@@ -3410,8 +3410,7 @@ void mju_defaultTask(mjTask* task);
 void mju_taskJoin(mjTask* task);
 mjsElement* mjs_attach(mjsElement* parent, const mjsElement* child,
                        const char* prefix, const char* suffix);
-int mjs_detachBody(mjSpec* s, mjsBody* b);
+int mjs_detach(mjSpec* spec, mjsElement* element);
-int mjs_detachDefault(mjSpec* s, mjsDefault* d);
 mjsBody* mjs_addBody(mjsBody* body, const mjsDefault* def);
 mjsSite* mjs_addSite(mjsBody* body, const mjsDefault* def);
 mjsJoint* mjs_addJoint(mjsBody* body, const mjsDefault* def);
@@ -3420,7 +3419,7 @@ mjsGeom* mjs_addGeom(mjsBody* body, const mjsDefault* def);
 mjsCamera* mjs_addCamera(mjsBody* body, const mjsDefault* def);
 mjsLight* mjs_addLight(mjsBody* body, const mjsDefault* def);
 mjsFrame* mjs_addFrame(mjsBody* body, mjsFrame* parentframe);
-int mjs_delete(mjsElement* element);
+int mjs_delete(mjSpec* spec, mjsElement* element);
 mjsActuator* mjs_addActuator(mjSpec* s, const mjsDefault* def);
 mjsSensor* mjs_addSensor(mjSpec* s);
 mjsFlex* mjs_addFlex(mjSpec* s);

doc/python.rst

@@ -607,7 +607,7 @@ For elements that can have children (bodies and defaults), the methods ``spec.de
 ``spec.detach_default(def)`` remove, respectively, ``body`` and ``def`` from the spec, together with all of their
 children. When detaching body subtrees, all elements which reference elements in the subtree, will also be removed. For
 all other elements, the method ``delete()`` removes the corresponding element from the spec, e.g.
-``spec.geom('my_geom').delete()`` will remove the geom named "my_geom" and all of the elements that reference it.
+``spec.delete(spec.geom('my_geom'))`` will remove the geom named "my_geom" and all of the elements that reference it.
 
 Tree traversal
 ^^^^^^^^^^^^^^

include/mujoco/mujoco.h

@@ -1411,11 +1411,8 @@ MJAPI void mju_taskJoin(mjTask* task);
 MJAPI mjsElement* mjs_attach(mjsElement* parent, const mjsElement* child,
                              const char* prefix, const char* suffix);
 
-// Delete body and descendants from mjSpec, remove all references, return 0 on success.
+// Detach but not delete object corresponding to the given element, return 0 on success.
-MJAPI int mjs_detachBody(mjSpec* s, mjsBody* b);
+MJAPI int mjs_detach(mjSpec* spec, mjsElement* element);
-
-// Delete default class and descendants from mjSpec, remove all references, return 0 on success.
-MJAPI int mjs_detachDefault(mjSpec* s, mjsDefault* d);
 
 //---------------------------------- Tree elements -------------------------------------------------
 
@@ -1444,7 +1441,7 @@ MJAPI mjsLight* mjs_addLight(mjsBody* body, const mjsDefault* def);
 MJAPI mjsFrame* mjs_addFrame(mjsBody* body, mjsFrame* parentframe);
 
 // Delete object corresponding to the given element, return 0 on success.
-MJAPI int mjs_delete(mjsElement* element);
+MJAPI int mjs_delete(mjSpec* spec, mjsElement* element);
 
 
 //---------------------------------- Non-tree elements ---------------------------------------------

python/mjspec.ipynb

@@ -2000,7 +2000,7 @@
     "\n",
     "# Delete all key frames to avoid name conflicts\n",
     "while humanoid.keys:\n",
-    "  humanoid.keys[-1].delete()\n",
+    "  humanoid.delete(keys[-1])\n",
     "\n",
     "# Create a grid of humanoids by attaching humanoid to spec multiple times\n",
     "for i in range(4):\n",
@@ -2335,7 +2335,7 @@
     "fly = mj.MjSpec.from_file(fly_file)\n",
     "# Remove lights from fly so they are not duplicated in line-up\n",
     "for light in fly.lights:\n",
-    "  light.delete()\n",
+    "  fly.delete(light)\n",
     "\n",
     "small_fly = scale_spec(fly, 1.25)\n",
     "large_fly = scale_spec(fly, 2)\n",

python/mujoco/introspect/functions.py

@@ -8987,45 +8987,25 @@ FUNCTIONS: Mapping[str, FunctionDecl] = dict([
          ),
          doc='Attach child to a parent, return the attached element if success or NULL otherwise.',  # pylint: disable=line-too-long
      )),
-    ('mjs_detachBody',
+    ('mjs_detach',
      FunctionDecl(
-         name='mjs_detachBody',
+         name='mjs_detach',
          return_type=ValueType(name='int'),
          parameters=(
              FunctionParameterDecl(
-                 name='s',
+                 name='spec',
                  type=PointerType(
                      inner_type=ValueType(name='mjSpec'),
                  ),
              ),
              FunctionParameterDecl(
-                 name='b',
+                 name='element',
                  type=PointerType(
-                     inner_type=ValueType(name='mjsBody'),
+                     inner_type=ValueType(name='mjsElement'),
                  ),
              ),
          ),
-         doc='Delete body and descendants from mjSpec, remove all references, return 0 on success.',  # pylint: disable=line-too-long
+         doc='Detach but not delete object corresponding to the given element, return 0 on success.',  # pylint: disable=line-too-long
-     )),
-    ('mjs_detachDefault',
-     FunctionDecl(
-         name='mjs_detachDefault',
-         return_type=ValueType(name='int'),
-         parameters=(
-             FunctionParameterDecl(
-                 name='s',
-                 type=PointerType(
-                     inner_type=ValueType(name='mjSpec'),
-                 ),
-             ),
-             FunctionParameterDecl(
-                 name='d',
-                 type=PointerType(
-                     inner_type=ValueType(name='mjsDefault'),
-                 ),
-             ),
-         ),
-         doc='Delete default class and descendants from mjSpec, remove all references, return 0 on success.',  # pylint: disable=line-too-long
      )),
     ('mjs_addBody',
      FunctionDecl(
@@ -9202,6 +9182,12 @@ FUNCTIONS: Mapping[str, FunctionDecl] = dict([
          name='mjs_delete',
          return_type=ValueType(name='int'),
          parameters=(
+             FunctionParameterDecl(
+                 name='spec',
+                 type=PointerType(
+                     inner_type=ValueType(name='mjSpec'),
+                 ),
+             ),
              FunctionParameterDecl(
                  name='element',
                  type=PointerType(

python/mujoco/specs.cc

@@ -397,7 +397,7 @@ PYBIND11_MODULE(_specs, m) {
       },
       py::return_value_policy::reference_internal);
   mjSpec.def("detach_default", [](MjSpec& self, raw::MjsDefault& def) {
-    if (mjs_detachDefault(self.ptr, &def) != 0) {
+    if (mjs_delete(self.ptr, def.element) != 0) {
       throw pybind11::value_error(mjs_getError(self.ptr));
     }
   });
@@ -408,7 +408,7 @@ PYBIND11_MODULE(_specs, m) {
       },
       py::return_value_policy::reference_internal);
   mjSpec.def("detach_body", [](MjSpec& self, raw::MjsBody& body) {
-    mjs_detachBody(self.ptr, &body);
+    mjs_delete(self.ptr, body.element);
   });
   mjSpec.def(
       "attach",
@@ -759,7 +759,9 @@ PYBIND11_MODULE(_specs, m) {
 
 
   // ============================= MJSFRAME ====================================
-  mjsFrame.def("delete", [](raw::MjsFrame& self) { mjs_delete(self.element); });
+  mjSpec.def("delete", [](MjSpec& self, raw::MjsFrame& obj) {
+    mjs_delete(self.ptr, obj.element);
+  });
   mjsFrame.def("set_frame", [](raw::MjsFrame& self, raw::MjsFrame& frame) {
     if (mjs_setFrame(self.element, &frame) != 0) {
       throw pybind11::value_error(mjs_getError(mjs_getSpec(self.element)));
@@ -796,7 +798,9 @@ PYBIND11_MODULE(_specs, m) {
       py::return_value_policy::reference_internal);
 
   // ============================= MJSGEOM =====================================
-  mjsGeom.def("delete", [](raw::MjsGeom& self) { mjs_delete(self.element); });
+  mjSpec.def("delete", [](MjSpec& self, raw::MjsGeom& obj) {
+    mjs_delete(self.ptr, obj.element);
+  });
   mjsGeom.def("set_frame", [](raw::MjsGeom& self, raw::MjsFrame& frame) {
     if (mjs_setFrame(self.element, &frame) != 0) {
       throw pybind11::value_error(mjs_getError(mjs_getSpec(self.element)));
@@ -824,7 +828,9 @@ PYBIND11_MODULE(_specs, m) {
       py::return_value_policy::reference_internal);
 
   // ============================= MJSJOINT ====================================
-  mjsJoint.def("delete", [](raw::MjsJoint& self) { mjs_delete(self.element); });
+  mjSpec.def("delete", [](MjSpec& self, raw::MjsJoint& obj) {
+    mjs_delete(self.ptr, obj.element);
+  });
   mjsJoint.def("set_frame", [](raw::MjsJoint& self, raw::MjsFrame& frame) {
     if (mjs_setFrame(self.element, &frame) != 0) {
       throw pybind11::value_error(mjs_getError(mjs_getSpec(self.element)));
@@ -852,7 +858,9 @@ PYBIND11_MODULE(_specs, m) {
       py::return_value_policy::reference_internal);
 
   // ============================= MJSSITE =====================================
-  mjsSite.def("delete", [](raw::MjsSite& self) { mjs_delete(self.element); });
+  mjSpec.def("delete", [](MjSpec& self, raw::MjsSite& obj) {
+    mjs_delete(self.ptr, obj.element);
+  });
   mjsSite.def("set_frame", [](raw::MjsSite& self, raw::MjsFrame& frame) {
     if (mjs_setFrame(self.element, &frame) != 0) {
       throw pybind11::value_error(mjs_getError(mjs_getSpec(self.element)));
@@ -897,8 +905,9 @@ PYBIND11_MODULE(_specs, m) {
       py::return_value_policy::reference_internal);
 
   // ============================= MJSCAMERA ===================================
-  mjsCamera.def("delete",
+  mjSpec.def("delete", [](MjSpec& self, raw::MjsCamera& obj) {
-                [](raw::MjsCamera& self) { mjs_delete(self.element); });
+    mjs_delete(self.ptr, obj.element);
+  });
   mjsCamera.def("set_frame", [](raw::MjsCamera& self, raw::MjsFrame& frame) {
     if (mjs_setFrame(self.element, &frame) != 0) {
       throw pybind11::value_error(mjs_getError(mjs_getSpec(self.element)));
@@ -926,7 +935,9 @@ PYBIND11_MODULE(_specs, m) {
       py::return_value_policy::reference_internal);
 
   // ============================= MJSLIGHT ====================================
-  mjsLight.def("delete", [](raw::MjsLight& self) { mjs_delete(self.element); });
+  mjSpec.def("delete", [](MjSpec& self, raw::MjsLight& obj) {
+    mjs_delete(self.ptr, obj.element);
+  });
   mjsLight.def("set_frame", [](raw::MjsLight& self, raw::MjsFrame& frame) {
     if (mjs_setFrame(self.element, &frame) != 0) {
       throw pybind11::value_error(mjs_getError(mjs_getSpec(self.element)));
@@ -954,8 +965,9 @@ PYBIND11_MODULE(_specs, m) {
       py::return_value_policy::reference_internal);
 
   // ============================= MJSMATERIAL =================================
-  mjsMaterial.def("delete",
+  mjSpec.def("delete", [](MjSpec& self, raw::MjsMaterial& obj) {
-                  [](raw::MjsMaterial& self) { mjs_delete(self.element); });
+    mjs_delete(self.ptr, obj.element);
+  });
   mjsMaterial.def_property(
       "classname",
       [](raw::MjsMaterial& self) -> raw::MjsDefault* {
@@ -966,7 +978,9 @@ PYBIND11_MODULE(_specs, m) {
       });
 
   // ============================= MJSMESH =====================================
-  mjsMesh.def("delete", [](raw::MjsMesh& self) { mjs_delete(self.element); });
+  mjSpec.def("delete", [](MjSpec& self, raw::MjsMesh& obj) {
+    mjs_delete(self.ptr, obj.element);
+  });
   mjsMesh.def_property(
       "classname",
       [](raw::MjsMesh& self) -> raw::MjsDefault* {
@@ -977,7 +991,9 @@ PYBIND11_MODULE(_specs, m) {
       });
 
   // ============================= MJSPAIR =====================================
-  mjsPair.def("delete", [](raw::MjsPair& self) { mjs_delete(self.element); });
+  mjSpec.def("delete", [](MjSpec& self, raw::MjsPair& obj) {
+    mjs_delete(self.ptr, obj.element);
+  });
   mjsPair.def_property(
       "classname",
       [](raw::MjsPair& self) -> raw::MjsDefault* {
@@ -988,8 +1004,9 @@ PYBIND11_MODULE(_specs, m) {
       });
 
   // ============================= MJSEQUAL ====================================
-  mjsEquality.def("delete",
+  mjSpec.def("delete", [](MjSpec& self, raw::MjsEquality& obj) {
-                  [](raw::MjsEquality& self) { mjs_delete(self.element); });
+    mjs_delete(self.ptr, obj.element);
+  });
   mjsEquality.def_property(
       "classname",
       [](raw::MjsEquality& self) -> raw::MjsDefault* {
@@ -1000,8 +1017,9 @@ PYBIND11_MODULE(_specs, m) {
       });
 
   // ============================= MJSACTUATOR =================================
-  mjsActuator.def("delete",
+  mjSpec.def("delete", [](MjSpec& self, raw::MjsActuator& obj) {
-                  [](raw::MjsActuator& self) { mjs_delete(self.element); });
+    mjs_delete(self.ptr, obj.element);
+  });
   mjsActuator.def_property(
       "classname",
       [](raw::MjsActuator& self) -> raw::MjsDefault* {
@@ -1101,8 +1119,9 @@ PYBIND11_MODULE(_specs, m) {
       py::arg("gain"));
 
   // ============================= MJSTENDON ===================================
-  mjsTendon.def("delete",
+  mjSpec.def("delete", [](MjSpec& self, raw::MjsTendon& obj) {
-                [](raw::MjsTendon& self) { mjs_delete(self.element); });
+    mjs_delete(self.ptr, obj.element);
+  });
   mjsTendon.def(
       "default",
       [](raw::MjsTendon& self) -> raw::MjsDefault* {
@@ -1135,39 +1154,54 @@ PYBIND11_MODULE(_specs, m) {
       py::return_value_policy::reference_internal);
 
   // ============================= MJSSENSOR ===================================
-  mjsSensor.def("delete",
+  mjSpec.def("delete", [](MjSpec& self, raw::MjsSensor& obj) {
-                [](raw::MjsSensor& self) { mjs_delete(self.element); });
+    mjs_delete(self.ptr, obj.element);
+  });
 
   // ============================= MJSFLEX =====================================
-  mjsFlex.def("delete", [](raw::MjsFlex& self) { mjs_delete(self.element); });
+  mjSpec.def("delete", [](MjSpec& self, raw::MjsFlex& obj) {
+    mjs_delete(self.ptr, obj.element);
+  });
 
   // ============================= MJSHFIELD ===================================
-  mjsHField.def("delete",
+  mjSpec.def("delete", [](MjSpec& self, raw::MjsHField& obj) {
-                [](raw::MjsHField& self) { mjs_delete(self.element); });
+    mjs_delete(self.ptr, obj.element);
+  });
 
   // ============================= MJSSKIN =====================================
-  mjsSkin.def("delete", [](raw::MjsSkin& self) { mjs_delete(self.element); });
+  mjSpec.def("delete", [](MjSpec& self, raw::MjsSkin& obj) {
+    mjs_delete(self.ptr, obj.element);
+  });
 
   // ============================= MJSTEXTURE ==================================
-  mjsTexture.def("delete",
+  mjSpec.def("delete", [](MjSpec& self, raw::MjsTexture& obj) {
-                 [](raw::MjsTexture& self) { mjs_delete(self.element); });
+    mjs_delete(self.ptr, obj.element);
+  });
 
   // ============================= MJSKEY ======================================
-  mjsKey.def("delete", [](raw::MjsKey& self) { mjs_delete(self.element); });
+  mjSpec.def("delete", [](MjSpec& self, raw::MjsKey& obj) {
+    mjs_delete(self.ptr, obj.element);
+  });
 
   // ============================= MJSTEXT =====================================
-  mjsText.def("delete", [](raw::MjsText& self) { mjs_delete(self.element); });
+  mjSpec.def("delete", [](MjSpec& self, raw::MjsText& obj) {
+    mjs_delete(self.ptr, obj.element);
+  });
 
   // ============================= MJSNUMERIC ==================================
-  mjsNumeric.def("delete",
+  mjSpec.def("delete", [](MjSpec& self, raw::MjsNumeric& obj) {
-                 [](raw::MjsNumeric& self) { mjs_delete(self.element); });
+    mjs_delete(self.ptr, obj.element);
+  });
 
   // ============================= MJSEXCLUDE ==================================
-  mjsExclude.def("delete",
+  mjSpec.def("delete", [](MjSpec& self, raw::MjsExclude& obj) {
-                 [](raw::MjsExclude& self) { mjs_delete(self.element); });
+    mjs_delete(self.ptr, obj.element);
+  });
 
   // ============================= MJSTUPLE ====================================
-  mjsTuple.def("delete", [](raw::MjsTuple& self) { mjs_delete(self.element); });
+  mjSpec.def("delete", [](MjSpec& self, raw::MjsTuple& obj) {
+    mjs_delete(self.ptr, obj.element);
+  });
 
   // ============================= MJSPLUGIN ===================================
   mjsPlugin.def_property(
@@ -1176,8 +1210,9 @@ PYBIND11_MODULE(_specs, m) {
       [](raw::MjsPlugin& self, raw::MjsPlugin* other) {
         self.element = other->element;
       });
-  mjsPlugin.def("delete",
+  mjSpec.def("delete", [](MjSpec& self, raw::MjsPlugin& obj) {
-                [](raw::MjsPlugin& self) { mjs_delete(self.element); });
+    mjs_delete(self.ptr, obj.element);
+  });
   mjsPlugin.def_property(
       "config",
       [](raw::MjsPlugin& self) -> py::dict {

python/mujoco/specs_test.py

@@ -838,9 +838,9 @@ class SpecsTest(absltest.TestCase):
     self.assertIsNotNone(site)
     self.assertEqual(site, spec.site('head'))
 
-    site.delete()
+    spec.delete(site)
-    spec.sensors[-1].delete()
+    spec.delete(spec.sensors[-1])
-    spec.sensors[-1].delete()
+    spec.delete(spec.sensors[-1])
 
     model = spec.compile()
     self.assertIsNotNone(model)
@@ -930,7 +930,7 @@ class SpecsTest(absltest.TestCase):
     """)
     plugin = spec.plugins[0]
     self.assertIsNotNone(plugin)
-    plugin.delete()
+    spec.delete(plugin)
 
     model = spec.compile()
     self.assertIsNotNone(model)

src/user/user_api.cc

@@ -284,35 +284,18 @@ const char* mjs_getError(mjSpec* s) {
 
 
 // detach body from mjSpec, return 0 on success
-int mjs_detachBody(mjSpec* s, mjsBody* b) {
+int mjs_detach(mjSpec* s, mjsElement* element) {
   mjCModel* model = static_cast<mjCModel*>(s->element);
-  mjCBody* body = static_cast<mjCBody*>(b->element);
+  if (!element) {
-  try {
+    model->SetError(mjCError(0, "Element is null."));
-    *model -= *body;
-  } catch (mjCError& e) {
-    model->SetError(e);
     return -1;
   }
-  model->Detach(body);
+  mjCError e(0, "Detach is not implemented yet.");
-  return 0;
+  model->SetError(e);
+  return -1;
 }
 
-// detach default from mjSpec, return 0 on success
+
-int mjs_detachDefault(mjSpec* s, mjsDefault* def) {
-  mjCModel* modelC = static_cast<mjCModel*>(s->element);
-  if (!def) {
-    modelC->SetError(mjCError(0, "Cannot detach, default is null"));
-    return -1;
-  }
-  mjCDef* defC = static_cast<mjCDef*>(def->element);
-  try {
-    *modelC -= *defC;
-  } catch (mjCError& e) {
-    modelC->SetError(e);
-    return -1;
-  }
-  return 0;
-}
 
 // check if model has warnings
 int mjs_isWarning(mjSpec* s) {
@@ -372,15 +355,24 @@ int mj_copyBack(mjSpec* s, const mjModel* m) {
 
 
 // delete object, return 0 on success
-int mjs_delete(mjsElement* element) {
+int mjs_delete(mjSpec* s, mjsElement* element) {
-  mjCModel* model;
+  mjCModel* model = static_cast<mjCModel*>(s->element);
-  if (element->elemtype == mjOBJ_DEFAULT)
+  if (!element) {
-    model = static_cast<mjCDef*>(element)->model;
+    model->SetError(mjCError(0, "Element is null."));
-  else
+    return -1;
-    model = static_cast<mjCBase*>(element)->model;
+  }
   try {
-    // it will call the appropriate destructor since ~mjCBase is virtual
+    if (element->elemtype == mjOBJ_BODY) {
-    model->DeleteElement(element);
+      mjCBody* body = static_cast<mjCBody*>(element);
+      *model -= *body;
+      model->DeleteElement(body);
+    } else if (element->elemtype == mjOBJ_DEFAULT) {
+      mjCDef* def = static_cast<mjCDef*>(element);
+      *model -= *def;
+    } else {
+      // it will call the appropriate destructor since ~mjCBase is virtual
+      model->DeleteElement(element);
+    }
     return 0;
   } catch (mjCError& e) {
     model->SetError(e);
@@ -1035,7 +1027,7 @@ const char* mjs_resolveOrientation(double quat[4], mjtByte degree, const char* s
 mjsFrame* mjs_bodyToFrame(mjsBody** body) {
   mjCBody* bodyC = static_cast<mjCBody*>((*body)->element);
   mjCFrame* frameC = bodyC->ToFrame();
-  bodyC->model->Detach(bodyC);
+  bodyC->model->DeleteElement((*body)->element);
   *body = nullptr;
   return &frameC->spec;
 }

src/user/user_api.h

@@ -76,11 +76,8 @@ MJAPI int mj_copyBack(mjSpec* s, const mjModel* m);
 MJAPI mjsElement* mjs_attach(mjsElement* parent, const mjsElement* child,
                              const char* prefix, const char* suffix);
 
-// Detach body from mjSpec, remove all references and delete the body, return 0 on success.
+// Detach but not delete object corresponding to the given element, return 0 on success.
-MJAPI int mjs_detachBody(mjSpec* s, mjsBody* b);
+MJAPI int mjs_detach(mjSpec* s, mjsElement* element);
-
-// Detach default from mjSpec, remove all references and delete the default, return 0 on success.
-MJAPI int mjs_detachDefault(mjSpec* s, mjsDefault* d);
 
 //---------------------------------- Add tree elements ---------------------------------------------
 
@@ -109,7 +106,7 @@ MJAPI mjsLight* mjs_addLight(mjsBody* body, const mjsDefault* def);
 MJAPI mjsFrame* mjs_addFrame(mjsBody* body, mjsFrame* parentframe);
 
 // Delete object corresponding to the given element, return 0 on success.
-MJAPI int mjs_delete(mjsElement* element);
+MJAPI int mjs_delete(mjSpec* s, mjsElement* element);
 
 
 //---------------------------------- Add non-tree elements -----------------------------------------

src/user/user_model.cc

@@ -742,15 +742,44 @@ void deletefromlist(std::vector<T*>* list, mjsElement* element) {
 
 
 
+// recursively delete all plugins in the subtree
+static void deletesubtreeplugin(mjCBody* subtree, mjCModel* model) {
+  mjsPlugin* plugin = &(subtree->spec.plugin);
+  if (plugin->active && plugin->name->empty()) {
+    model->DeleteElement(plugin->element);
+  }
+  for (auto* body : subtree->Bodies()) {
+    deletesubtreeplugin(body, model);
+  }
+}
+
+
+
 // discard all invalid elements from all lists
 void mjCModel::DeleteElement(mjsElement* el) {
   ResetTreeLists();
 
+  if (el->elemtype != mjOBJ_DEFAULT) {
+    if (static_cast<mjCBase*>(el)->model != this) {
+      throw mjCError(nullptr, "element is not in this model");
+    }
+  } else {
+    if (static_cast<mjCDef*>(el)->model != this) {
+      throw mjCError(nullptr, "default is not in this model");
+    }
+  }
+
   switch (el->elemtype) {
     case mjOBJ_BODY:
+    {
       MakeTreeLists();  // rebuild lists that were reset at the beginning of the function
-      throw mjCError(nullptr, "bodies cannot be deleted, use detach instead");
+      mjCBody* subtree = static_cast<mjCBody*>(el);
+      if (subtree->GetRef() == 1)  {
+        deletesubtreeplugin(subtree, this);
+      }
+      subtree->Release();
       break;
+    }
 
     case mjOBJ_DEFAULT:
       MakeTreeLists();  // rebuild lists that were reset at the beginning of the function
@@ -828,29 +857,6 @@ void mjCModel::DeleteElement(mjsElement* el) {
 
 
 
-// recursively delete all plugins in the subtree
-void deletesubtreeplugin(mjCBody* subtree, mjCModel* model) {
-  mjsPlugin* plugin = &(subtree->spec.plugin);
-  if (plugin->active && plugin->name->empty()) {
-    model->DeleteElement(plugin->element);
-  }
-  for (auto* body : subtree->Bodies()) {
-    deletesubtreeplugin(body, model);
-  }
-}
-
-
-
-// deletes all plugins in the subtree and then the subtree itself
-void mjCModel::Detach(mjCBody* subtree) {
-  if (subtree->GetRef() == 1)  {
-    deletesubtreeplugin(subtree, this);
-  }
-  subtree->Release();
-}
-
-
-
 // TODO: we should not use C-type casting with multiple C++ inheritance
 void mjCModel::CreateObjectLists() {
   for (int i = 0; i < mjNOBJECT; ++i) {

src/user/user_model.h

@@ -232,9 +232,6 @@ class mjCModel : public mjCModel_, private mjSpec {
   // delete default and all descendants
   void RemoveDefault(mjCDef* def);
 
-  // detach subtree from model
-  void Detach(mjCBody* subtree);
-
   // API for access to model elements (outside tree)
   int NumObjects(mjtObj type);              // number of objects in specified list
   mjCBase* GetObject(mjtObj type, int id);  // pointer to specified object

src/xml/xml_native_reader.cc

@@ -3554,7 +3554,7 @@ void mjXReader::Body(XMLElement* section, mjsBody* body, mjsFrame* frame,
       }
 
       // delete subtree
-      if (mjs_detachBody(spec, subtree)) {
+      if (mjs_delete(spec, subtree->element)) {
         throw mjXError(elem, mjs_getError(spec));
       }
     }

test/user/user_api_test.cc

@@ -219,7 +219,7 @@ TEST_F(PluginTest, DeletePlugin) {
   EXPECT_THAT(model->actuator_plugin[0], 0);
 
   // delete actuator
-  mjs_delete(actuator->element);
+  mjs_delete(spec, actuator->element);
 
   // recompile and check that the plugin is not present
   mjModel* newmodel = mj_compile(spec, NULL);
@@ -326,7 +326,7 @@ TEST_F(PluginTest, DetachPlugin) {
   // detach the body and compile
   mjsBody* body_to_detach = mjs_findBody(parent, "child-body");
   EXPECT_THAT(body_to_detach, NotNull());
-  EXPECT_THAT(mjs_detachBody(parent, body_to_detach), 0);
+  EXPECT_THAT(mjs_delete(parent, body_to_detach->element), 0);
   mjModel* model = mj_compile(parent, nullptr);
   EXPECT_THAT(model, NotNull());
   EXPECT_THAT(model->nbody, 2);
@@ -1368,12 +1368,8 @@ void TestDetachBody(bool compile) {
   mjsBody* body = mjs_findBody(child, "body");
   EXPECT_THAT(body, NotNull());
 
-  // get an error if trying to delete the body
+  // delete subtree
-  EXPECT_EQ(mjs_delete(body->element), -1);
+  EXPECT_THAT(mjs_delete(child, body->element), 0);
-  EXPECT_THAT(mjs_getError(child), HasSubstr("use detach instead"));
-
-  // detach subtree
-  EXPECT_THAT(mjs_detachBody(child, body), 0);
 
   // try saving to XML before compiling again
   std::array<char, 1024> e;
@@ -1898,12 +1894,12 @@ TEST_F(MujocoTest, PreserveState) {
   // detach subtree
   mjsBody* body = mjs_findBody(spec, "detachable");
   EXPECT_THAT(body, NotNull());
-  EXPECT_THAT(mjs_detachBody(spec, body), 0);
+  EXPECT_THAT(mjs_delete(spec, body->element), 0);
 
   // detach mocap
   mjsBody* mocap_body = mjs_findBody(spec, "mocap_detach");
   EXPECT_THAT(mocap_body, NotNull());
-  EXPECT_THAT(mjs_detachBody(spec, mocap_body), 0);
+  EXPECT_THAT(mjs_delete(spec, mocap_body->element), 0);
 
   // add body
   mjsBody* newbody = mjs_addBody(mjs_findBody(spec, "world"), 0);
@@ -2787,25 +2783,20 @@ TEST_F(MujocoTest, DetachDefault) {
   mjsDefault* child = mjs_findDefault(spec, "child1");
   EXPECT_THAT(child, NotNull());
 
-  // try using mjs_delete to remove default, should fail
+  // delete default
-  EXPECT_EQ(mjs_delete(child->element), -1);
+  EXPECT_EQ(mjs_delete(spec, child->element), 0);
-
-  // detach default
-  EXPECT_EQ(mjs_detachDefault(spec, child), 0);
   child = mjs_findDefault(spec, "child1");
   EXPECT_THAT(child, IsNull());
 
   // try and detach previously detached default, should fail
-  EXPECT_EQ(mjs_detachDefault(spec, child), -1);
+  EXPECT_EQ(mjs_delete(spec, nullptr), -1);
   child = mjs_findDefault(spec, "child1");
   EXPECT_THAT(child, IsNull());
-  EXPECT_THAT(mjs_getError(spec),
-              HasSubstr("Cannot detach, default is null"));
 
   // detach parent
   mjsDefault* parent = mjs_findDefault(spec, "parent");
   EXPECT_THAT(parent, NotNull());
-  mjs_detachDefault(spec, parent);
+  mjs_delete(spec, parent->element);
 
   // both parent and remaining child should be removed
   parent = mjs_findDefault(spec, "parent");
@@ -2816,7 +2807,7 @@ TEST_F(MujocoTest, DetachDefault) {
   // error when trying to detach the 'main' default
   mjsDefault* main = mjs_findDefault(spec, "main");
   EXPECT_THAT(main, NotNull());
-  EXPECT_EQ(mjs_detachDefault(spec, main), -1);
+  EXPECT_EQ(mjs_delete(spec, main->element), -1);
   EXPECT_THAT(mjs_getError(spec),
               HasSubstr("cannot remove the global default ('main')"));