diff --git a/doc/APIreference/functions.rst b/doc/APIreference/functions.rst index c80c9628..590aaf72 100644 --- a/doc/APIreference/functions.rst +++ b/doc/APIreference/functions.rst @@ -3918,7 +3918,7 @@ Add frame to body. .. mujoco-include:: mjs_delete -Delete object corresponding to the given element. +Delete object corresponding to the given element, return 0 on success. .. _AddNonTreeElements: diff --git a/doc/includes/references.h b/doc/includes/references.h index 8cd73e5a..e1432bd2 100644 --- a/doc/includes/references.h +++ b/doc/includes/references.h @@ -3578,7 +3578,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); -void mjs_delete(mjsElement* element); +int mjs_delete(mjsElement* element); mjsActuator* mjs_addActuator(mjSpec* s, const mjsDefault* def); mjsSensor* mjs_addSensor(mjSpec* s); mjsFlex* mjs_addFlex(mjSpec* s); diff --git a/include/mujoco/mujoco.h b/include/mujoco/mujoco.h index 66f5fa98..9e4d25ff 100644 --- a/include/mujoco/mujoco.h +++ b/include/mujoco/mujoco.h @@ -121,7 +121,7 @@ MJAPI void mj_freeLastXML(void); // If length of the output buffer is too small, returns the required size. MJAPI int mj_saveXMLString(const mjSpec* s, char* xml, int xml_sz, char* error, int error_sz); -// Save spec to XML file, return 1 on success, 0 otherwise. +// Save spec to XML file, return 0 on success, -1 otherwise. MJAPI int mj_saveXML(const mjSpec* s, const char* filename, char* error, int error_sz); @@ -1453,8 +1453,8 @@ 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. -MJAPI void mjs_delete(mjsElement* element); +// Delete object corresponding to the given element, return 0 on success. +MJAPI int mjs_delete(mjsElement* element); //---------------------------------- Non-tree elements --------------------------------------------- diff --git a/introspect/functions.py b/introspect/functions.py index b16bee97..fa2141a9 100644 --- a/introspect/functions.py +++ b/introspect/functions.py @@ -379,7 +379,7 @@ FUNCTIONS: Mapping[str, FunctionDecl] = dict([ type=ValueType(name='int'), ), ), - doc='Save spec to XML file, return 1 on success, 0 otherwise.', + doc='Save spec to XML file, return 0 on success, -1 otherwise.', )), ('mj_step', FunctionDecl( @@ -9277,7 +9277,7 @@ FUNCTIONS: Mapping[str, FunctionDecl] = dict([ ('mjs_delete', FunctionDecl( name='mjs_delete', - return_type=ValueType(name='void'), + return_type=ValueType(name='int'), parameters=( FunctionParameterDecl( name='element', @@ -9286,7 +9286,7 @@ FUNCTIONS: Mapping[str, FunctionDecl] = dict([ ), ), ), - doc='Delete object corresponding to the given element.', + doc='Delete object corresponding to the given element, return 0 on success.', # pylint: disable=line-too-long )), ('mjs_addActuator', FunctionDecl( diff --git a/src/user/user_api.cc b/src/user/user_api.cc index af7cd9b5..7e819007 100644 --- a/src/user/user_api.cc +++ b/src/user/user_api.cc @@ -254,10 +254,17 @@ int mjs_activatePlugin(mjSpec* s, const char* name) { -// delete object, it will call the appropriate destructor since ~mjCBase is virtual -void mjs_delete(mjsElement* element) { +// delete object, return 0 if success +int mjs_delete(mjsElement* element) { mjCBase* object = static_cast(element); - object->model->DeleteElement(element); + try { + // it will call the appropriate destructor since ~mjCBase is virtual + object->model->DeleteElement(element); + return 0; + } catch (mjCError& e) { + object->model->SetError(e); + return -1; + } } diff --git a/src/user/user_api.h b/src/user/user_api.h index 70eb6af2..ac545e74 100644 --- a/src/user/user_api.h +++ b/src/user/user_api.h @@ -108,8 +108,8 @@ 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. -MJAPI void mjs_delete(mjsElement* element); +// Delete object corresponding to the given element, return 0 on success. +MJAPI int mjs_delete(mjsElement* element); //---------------------------------- Add non-tree elements ----------------------------------------- diff --git a/src/user/user_model.cc b/src/user/user_model.cc index 0cf6a4ba..7359c1f0 100644 --- a/src/user/user_model.cc +++ b/src/user/user_model.cc @@ -530,7 +530,6 @@ mjCModel& mjCModel::operator-=(const mjCBody& subtree) { ResetTreeLists(); } - PointToLocal(); return *this; } diff --git a/src/xml/xml_native_writer.cc b/src/xml/xml_native_writer.cc index e9a47920..3ae9fc22 100644 --- a/src/xml/xml_native_writer.cc +++ b/src/xml/xml_native_writer.cc @@ -858,7 +858,7 @@ mjXWriter::mjXWriter(void) { // cast model void mjXWriter::SetModel(const mjSpec* _spec, const mjModel* m) { if (_spec) { - model = (mjCModel*)_spec->element; + model = static_cast(_spec->element); } if (m) { model->CopyBack(m); @@ -2207,6 +2207,10 @@ void mjXWriter::Keyframe(XMLElement* root) { // create section XMLElement* section = InsertEnd(root, "keyframe"); + if (!model->key_pending_.empty()) { + throw mjXError(0, "Model has pending keyframes. It must be (re)compiled before writing XML."); + } + // write all keyframes for (int i=0; inkey; i++) { XMLElement* elem = InsertEnd(section, "key"); diff --git a/test/user/user_api_test.cc b/test/user/user_api_test.cc index c6620c1c..e641db58 100644 --- a/test/user/user_api_test.cc +++ b/test/user/user_api_test.cc @@ -1222,9 +1222,21 @@ void TestDetachBody(bool compile) { mjsBody* body = mjs_findBody(child, "body"); EXPECT_THAT(body, NotNull()); + // get an error if trying to delete the body + EXPECT_EQ(mjs_delete(body->element), -1); + 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 e; + std::array s; + EXPECT_EQ(mj_saveXMLString(child, s.data(), 1024, e.data(), 1024), -1); + EXPECT_THAT(e.data(), compile + ? HasSubstr("Model has pending keyframes") + : HasSubstr("Only compiled model can be written")); + // compile new model mjModel* m_detached = mj_compile(child, 0); EXPECT_THAT(m_detached, NotNull());