Raise error when binding to a removed element.

PiperOrigin-RevId: 774462737
Change-Id: I820c0e1c010ae9fbf6d9667c3364d4b65bf21002

doc/APIreference/functions.rst

@@ -3783,15 +3783,6 @@ Attachment
 
 Attach child to a parent, return the attached element if success or NULL otherwise.
 
-.. _mjs_detach:
-
-`mjs_detach <#mjs_detach>`__
-~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-
-.. mujoco-include:: mjs_detach
-
-Detach but not delete object corresponding to the given element, return 0 on success.
-
 .. _AddTreeElements:
 
 Tree elements

doc/includes/references.h

@@ -3411,7 +3411,6 @@ 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_detach(mjSpec* spec, mjsElement* element);
 mjsBody* mjs_addBody(mjsBody* body, const mjsDefault* def);
 mjsSite* mjs_addSite(mjsBody* body, const mjsDefault* def);
 mjsJoint* mjs_addJoint(mjsBody* body, const mjsDefault* def);

include/mujoco/mujoco.h

@@ -1411,8 +1411,6 @@ MJAPI void mju_taskJoin(mjTask* task);
 MJAPI mjsElement* mjs_attach(mjsElement* parent, const mjsElement* child,
                              const char* prefix, const char* suffix);
 
-// Detach but not delete object corresponding to the given element, return 0 on success.
-MJAPI int mjs_detach(mjSpec* spec, mjsElement* element);
 
 //---------------------------------- Tree elements -------------------------------------------------
 
@@ -1440,7 +1438,7 @@ MJAPI mjsLight* mjs_addLight(mjsBody* body, const mjsDefault* def);
 // Add frame to body.
 MJAPI mjsFrame* mjs_addFrame(mjsBody* body, mjsFrame* parentframe);
 
-// Delete object corresponding to the given element, return 0 on success.
+// Detach but not delete object corresponding to the given element, return 0 on success.
 MJAPI int mjs_delete(mjSpec* spec, mjsElement* element);
 
 

mjx/mujoco/mjx/_src/support_test.py

@@ -379,6 +379,19 @@ class SupportTest(parameterized.TestCase):
         ' 15297169659434471387 != 2785811613804955188',
     )
 
+    # what happens when we bind to an actuator that was removed?
+    bygone_actuators = []
+    for act in s.actuators:
+      bygone_actuators.append(act)
+      s.delete(act)
+    m = s.compile()
+    d = mujoco.MjData(m)
+    mx = mjx.put_model(m)
+    dx = mjx.put_data(m, d)
+    dx = mjx.step(mx, dx)
+    with self.assertRaisesRegex(KeyError, 'invalid id: -1'):
+      dx.bind(mx, bygone_actuators)
+
   _CONTACTS = """
     <mujoco>
       <worldbody>

python/mujoco/introspect/functions.py

@@ -8987,26 +8987,6 @@ 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_detach',
-     FunctionDecl(
-         name='mjs_detach',
-         return_type=ValueType(name='int'),
-         parameters=(
-             FunctionParameterDecl(
-                 name='spec',
-                 type=PointerType(
-                     inner_type=ValueType(name='mjSpec'),
-                 ),
-             ),
-             FunctionParameterDecl(
-                 name='element',
-                 type=PointerType(
-                     inner_type=ValueType(name='mjsElement'),
-                 ),
-             ),
-         ),
-         doc='Detach but not delete object corresponding to the given element, return 0 on success.',  # pylint: disable=line-too-long
-     )),
     ('mjs_addBody',
      FunctionDecl(
          name='mjs_addBody',
@@ -9195,7 +9175,7 @@ FUNCTIONS: Mapping[str, FunctionDecl] = dict([
                  ),
              ),
          ),
-         doc='Delete object corresponding to the given element, 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_addActuator',
      FunctionDecl(

src/user/user_api.cc

@@ -341,28 +341,6 @@ int mj_copyBack(mjSpec* s, const mjModel* m) {
 
 
 // detach body from mjSpec, return 0 on success
-int mjs_detach(mjSpec* s, mjsElement* element) {
-  mjCModel* model = static_cast<mjCModel*>(s->element);
-  if (!element) {
-    model->SetError(mjCError(0, "Element is null."));
-    return -1;
-  }
-  try {
-    if (element->elemtype == mjOBJ_DEFAULT) {
-      throw mjCError(0, "Detach is not implemented for defaults.");
-    } else {
-      *model -= element;
-    }
-    return 0;
-  } catch (mjCError& e) {
-    model->SetError(e);
-    return -1;
-  }
-}
-
-
-
-// delete object, return 0 on success
 int mjs_delete(mjSpec* s, mjsElement* element) {
   mjCModel* model = static_cast<mjCModel*>(s->element);
   if (!element) {
@@ -374,8 +352,7 @@ int mjs_delete(mjSpec* s, mjsElement* element) {
       mjCDef* def = static_cast<mjCDef*>(element);
       *model -= *def;
     } else {
-      mjs_detach(s, element);
+      *model -= element;
-      model->DeleteElement(element);
     }
     return 0;
   } catch (mjCError& e) {
@@ -1031,7 +1008,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->DeleteElement((*body)->element);
+  *bodyC->model -= (*body)->element;
   *body = nullptr;
   return &frameC->spec;
 }

src/user/user_api.h

@@ -76,8 +76,6 @@ 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 but not delete object corresponding to the given element, return 0 on success.
-MJAPI int mjs_detach(mjSpec* s, mjsElement* element);
 
 //---------------------------------- Add tree elements ---------------------------------------------
 
@@ -105,7 +103,7 @@ MJAPI mjsLight* mjs_addLight(mjsBody* body, const mjsDefault* def);
 // Add frame to body.
 MJAPI mjsFrame* mjs_addFrame(mjsBody* body, mjsFrame* parentframe);
 
-// Delete object corresponding to the given element, return 0 on success.
+// Detach but not delete object corresponding to the given element, return 0 on success.
 MJAPI int mjs_delete(mjSpec* s, mjsElement* element);
 
 

src/user/user_model.cc

@@ -746,7 +746,6 @@ void mjCModel::DeleteSubtreePlugin(mjCBody* subtree) {
   mjsPlugin* plugin = &(subtree->spec.plugin);
   if (plugin->active && plugin->name->empty()) {
     *this -= plugin->element;
-    detached_.push_back(static_cast<mjCBase*>(plugin->element));
   }
   for (auto* body : subtree->Bodies()) {
     DeleteSubtreePlugin(body);
@@ -755,13 +754,6 @@ void mjCModel::DeleteSubtreePlugin(mjCBody* subtree) {
 
 
 
-// delete an object from the model
-void mjCModel::DeleteElement(mjsElement* el) {
-  static_cast<mjCBase*>(el)->Release();
-}
-
-
-
 // remove the element from the model
 void mjCModel::operator-=(mjsElement* el) {
   if (el->elemtype == mjOBJ_BODY) {
@@ -769,6 +761,7 @@ void mjCModel::operator-=(mjsElement* el) {
     *this -= *body;
   }
 
+  detached_.push_back(static_cast<mjCBase*>(el));
   ResetTreeLists();
 
   if (el->elemtype != mjOBJ_DEFAULT) {
@@ -800,7 +793,6 @@ void mjCModel::operator-=(mjsElement* el) {
       mjCGeom* geom = static_cast<mjCGeom*>(el);
       if (geom->plugin.active && geom->plugin.name->empty()) {
         *this -= geom->plugin.element;
-        detached_.push_back(static_cast<mjCBase*>(geom->plugin.element));
       }
       deletefromlist(&(geom->body->geoms), el);
       break;
@@ -827,7 +819,6 @@ void mjCModel::operator-=(mjsElement* el) {
       mjCMesh* mesh = static_cast<mjCMesh*>(el);
       if (mesh->plugin.active && mesh->plugin.name->empty()) {
         *this -= mesh->plugin.element;
-        detached_.push_back(static_cast<mjCBase*>(mesh->plugin.element));
       }
       deletefromlist(object_lists_[mjOBJ_MESH], el);
       break;
@@ -838,7 +829,6 @@ void mjCModel::operator-=(mjsElement* el) {
       mjCActuator* actuator = static_cast<mjCActuator*>(el);
       if (actuator->plugin.active && actuator->plugin.name->empty()) {
         *this -= actuator->plugin.element;
-        detached_.push_back(static_cast<mjCBase*>(actuator->plugin.element));
       }
       deletefromlist(object_lists_[mjOBJ_ACTUATOR], el);
       break;
@@ -849,7 +839,6 @@ void mjCModel::operator-=(mjsElement* el) {
       mjCSensor* sensor = static_cast<mjCSensor*>(el);
       if (sensor->plugin.active && sensor->plugin.name->empty()) {
         *this -= sensor->plugin.element;
-        detached_.push_back(static_cast<mjCBase*>(sensor->plugin.element));
       }
       deletefromlist(object_lists_[mjOBJ_SENSOR], el);
       break;

src/user/user_model.h

@@ -229,9 +229,6 @@ class mjCModel : public mjCModel_, private mjSpec {
   // delete object from the corresponding list
   void operator-=(mjsElement* el);
 
-  // delete object
-  void DeleteElement(mjsElement* el);
-
   // delete default and all descendants
   void RemoveDefault(mjCDef* def);