From 6e64dd10e87d91ec4739387d95a5688c869d543b Mon Sep 17 00:00:00 2001 From: Alessio Quaglino Date: Fri, 12 Jul 2024 05:07:02 -0700 Subject: [PATCH] Fix bug in RemoveFromList. The function was interpreting that the elements that were expected to be detached contained useful user errors, and therefore it was not detaching them. Also, the function tried to compare the namespace between two models, but the namespace is already fused in the references at detach time. PiperOrigin-RevId: 651738364 Change-Id: Ifdaa9bd5b658bfd18b544b7138a5c2c56b66e936 --- src/user/user_model.cc | 26 +++++++++++--------------- src/user/user_model.h | 4 ++-- test/user/testdata/lengthrange.xml | 18 +----------------- 3 files changed, 14 insertions(+), 34 deletions(-) diff --git a/src/user/user_model.cc b/src/user/user_model.cc index c3ad2ff6..c71900e1 100644 --- a/src/user/user_model.cc +++ b/src/user/user_model.cc @@ -168,6 +168,7 @@ void mjCModel::CopyList(std::vector& dest, candidate->ResolveReferences(this); } catch (mjCError err) { // if not present, skip the element + // TODO: do not skip elements that contain user errors delete candidate; continue; } @@ -256,25 +257,20 @@ mjCModel& mjCModel::operator+=(const mjCModel& other) { template -void mjCModel::RemoveFromList(std::vector& list, const mjCModel& other) { +void mjCModel::RemoveFromList(std::vector& list) { int nlist = (int)list.size(); int removed = 0; for (int i = 0; i < nlist; i++) { T* element = list[i]; element->id -= removed; - try { - // check if the element contains an error - element->NameSpace(&other); - element->CopyFromSpec(); - element->ResolveReferences(&other); - } catch (mjCError err) { - continue; - } try { // check if the element references something that was removed + // TODO: do not remove elements that contain user errors element->NameSpace(this); + element->CopyFromSpec(); element->ResolveReferences(this); } catch (mjCError err) { + ids[element->elemtype].erase(element->name); delete element; list.erase(list.begin() + i); nlist--; @@ -315,12 +311,12 @@ mjCModel& mjCModel::operator-=(const mjCBody& subtree) { ProcessLists(/*checkrepeat=*/false); // check if we have to remove anything else - RemoveFromList(pairs_, oldmodel); - RemoveFromList(excludes_, oldmodel); - RemoveFromList(tendons_, oldmodel); - RemoveFromList(equalities_, oldmodel); - RemoveFromList(actuators_, oldmodel); - RemoveFromList(sensors_, oldmodel); + RemoveFromList(pairs_); + RemoveFromList(excludes_); + RemoveFromList(tendons_); + RemoveFromList(equalities_); + RemoveFromList(actuators_); + RemoveFromList(sensors_); // restore to the original state if (!compiled) { diff --git a/src/user/user_model.h b/src/user/user_model.h index b35222b1..7b86aaef 100644 --- a/src/user/user_model.h +++ b/src/user/user_model.h @@ -342,8 +342,8 @@ class mjCModel : public mjCModel_, private mjSpec { template void CopyList(std::vector& dest, const std::vector& sources); - // delete from list the elements that are compatible with other but not this model - template void RemoveFromList(std::vector& list, const mjCModel& other); + // delete from list the elements that cause an error + template void RemoveFromList(std::vector& list); // create mjCBase lists from children lists void CreateObjectLists(); diff --git a/test/user/testdata/lengthrange.xml b/test/user/testdata/lengthrange.xml index 8e12b6d2..b1b0219e 100644 --- a/test/user/testdata/lengthrange.xml +++ b/test/user/testdata/lengthrange.xml @@ -23,22 +23,6 @@ - - - - - - - - - - - - - - - - - +