diff --git a/src/user/user_mesh.cc b/src/user/user_mesh.cc index 34e29119..2a398c6f 100644 --- a/src/user/user_mesh.cc +++ b/src/user/user_mesh.cc @@ -269,6 +269,9 @@ void mjCMesh::CopyFromSpec() { mjCMesh::~mjCMesh() { if (center_) mju_free(center_); if (graph_) mju_free(graph_); + if (spec.plugin.active && spec.plugin.instance_name->empty()) { + model->DeleteElement(spec.plugin.instance); + } } diff --git a/src/user/user_model.cc b/src/user/user_model.cc index 13679ef9..ed3e4528 100644 --- a/src/user/user_model.cc +++ b/src/user/user_model.cc @@ -516,6 +516,7 @@ void mjCModel::DeleteElement(mjsElement* el) { } if (compiled) { + ResetTreeLists(); // in case of a nested delete MakeLists(world); ProcessLists(/*checkrepeat=*/false); } @@ -586,6 +587,9 @@ void mjCModel::CopyFromSpec() { // destructor mjCModel::~mjCModel() { + // do not rebuild lists if we are in the process of deleting the model + compiled = false; + // delete kinematic tree and all objects allocated in it delete bodies_[0]; diff --git a/src/user/user_objects.cc b/src/user/user_objects.cc index b3c17780..d7ebe82f 100644 --- a/src/user/user_objects.cc +++ b/src/user/user_objects.cc @@ -984,6 +984,10 @@ mjCBody::~mjCBody() { sites.clear(); cameras.clear(); lights.clear(); + + if (spec.plugin.active && spec.plugin.instance_name->empty()) { + model->DeleteElement(spec.plugin.instance); + } } @@ -2107,6 +2111,14 @@ mjCGeom::mjCGeom(const mjCGeom& other) { +mjCGeom::~mjCGeom() { + if (spec.plugin.active && spec.plugin.instance_name->empty()) { + model->DeleteElement(spec.plugin.instance); + } +} + + + mjCGeom& mjCGeom::operator=(const mjCGeom& other) { if (this != &other) { this->spec = other.spec; @@ -5521,6 +5533,14 @@ mjCActuator::mjCActuator(const mjCActuator& other) { +mjCActuator::~mjCActuator() { + if (spec.plugin.active && spec.plugin.instance_name->empty()) { + model->DeleteElement(spec.plugin.instance); + } +} + + + mjCActuator& mjCActuator::operator=(const mjCActuator& other) { if (this != &other) { this->spec = other.spec; @@ -5875,6 +5895,14 @@ mjCSensor::mjCSensor(const mjCSensor& other) { +mjCSensor::~mjCSensor() { + if (spec.plugin.active && spec.plugin.instance_name->empty()) { + model->DeleteElement(spec.plugin.instance); + } +} + + + mjCSensor& mjCSensor::operator=(const mjCSensor& other) { if (this != &other) { this->spec = other.spec; diff --git a/src/user/user_objects.h b/src/user/user_objects.h index 1de789c2..5a763e43 100644 --- a/src/user/user_objects.h +++ b/src/user/user_objects.h @@ -511,6 +511,7 @@ class mjCGeom : public mjCGeom_, private mjsGeom { mjCGeom(mjCModel* = nullptr, mjCDef* = nullptr); mjCGeom(const mjCGeom& other); mjCGeom& operator=(const mjCGeom& other); + ~mjCGeom(); using mjCBase::name; mjsGeom spec; // variables set by user @@ -1456,6 +1457,7 @@ class mjCActuator : public mjCActuator_, private mjsActuator { mjCActuator(mjCModel* = nullptr, mjCDef* = nullptr); mjCActuator(const mjCActuator& other); mjCActuator& operator=(const mjCActuator& other); + ~mjCActuator(); mjsActuator spec; using mjCBase::name; @@ -1516,6 +1518,7 @@ class mjCSensor : public mjCSensor_, private mjsSensor { mjCSensor(mjCModel*); mjCSensor(const mjCSensor& other); mjCSensor& operator=(const mjCSensor& other); + ~mjCSensor(); mjsSensor spec; using mjCBase::name; diff --git a/test/user/user_api_test.cc b/test/user/user_api_test.cc index cdad61b3..b27f26e2 100644 --- a/test/user/user_api_test.cc +++ b/test/user/user_api_test.cc @@ -165,6 +165,57 @@ TEST_F(PluginTest, ActivatePlugin) { mj_deleteModel(model); } +TEST_F(PluginTest, DeletePlugin) { + std::string plugin_name = "mujoco.pid"; + mjSpec* spec = mj_makeSpec(); + + // get slot of requested plugin + int plugin_slot = -1; + const mjpPlugin* plugin = mjp_getPlugin(plugin_name.c_str(), &plugin_slot); + ASSERT_THAT(plugin, NotNull()); + + // activated plugin in the slot + std::vector> active_plugins; + active_plugins.emplace_back(std::make_pair(plugin, plugin_slot)); + mjs_setActivePlugins(spec, &active_plugins); + + // create body + mjsBody* body = mjs_addBody(mjs_findBody(spec, "world"), 0); + mjsJoint* joint = mjs_addJoint(body, 0); + mjsGeom* geom = mjs_addGeom(body, 0); + mjs_setString(joint->name, "j1"); + joint->type = mjJNT_SLIDE; + geom->size[0] = 1; + + // add actuator + mjsActuator* actuator = mjs_addActuator(spec, 0); + mjs_setString(actuator->target, "j1"); + mjs_setString(actuator->plugin.name, plugin_name.c_str()); + actuator->plugin.instance = mjs_addPlugin(spec)->instance; + actuator->plugin.active = true; + actuator->trntype = mjTRN_JOINT; + + // compile and check that the plugin is present + mjModel* model = mj_compile(spec, NULL); + EXPECT_THAT(model, NotNull()); + EXPECT_THAT(model->nu, 1); + EXPECT_THAT(model->nplugin, 1); + EXPECT_THAT(model->actuator_plugin[0], 0); + + // delete actuator + mjs_delete(actuator->element); + + // recompile and check that the plugin is not present + mjModel* newmodel = mj_compile(spec, NULL); + EXPECT_THAT(newmodel, NotNull()); + EXPECT_THAT(newmodel->nu, 0); + EXPECT_THAT(newmodel->nplugin, 0); + + mj_deleteSpec(spec); + mj_deleteModel(model); + mj_deleteModel(newmodel); +} + TEST_F(MujocoTest, RecompileFails) { mjSpec* spec = mj_makeSpec(); mjsBody* body = mjs_addBody(mjs_findBody(spec, "world"), 0);