diff --git a/CHANGELOG.md b/CHANGELOG.md index e4cdfb45dc..c14e3d151a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,6 +33,7 @@ Scholz et al. (2015). This class encapsulates `SimTK::CableSpan`, the Simbody im geometry for path-based OpenSim components (e.g., `Muscle`, `PathSpring`, etc.). This class can be used as a replacement for `GeometryPath`, providing improved computational performance and stability in wrapping solutions. - Implemented `generateDecorations()` for `Station` to allow visualization in the Simbody visualizer. (#4169) +- Added `Component::removeComponent` and `Component::extractComponent` methods, which enable removing subcomponents that were previously added via `Component::addComponent` or the `` XML element (#4174). v4.5.2 diff --git a/OpenSim/Common/Component.cpp b/OpenSim/Common/Component.cpp index 578aee7671..b784f0307a 100644 --- a/OpenSim/Common/Component.cpp +++ b/OpenSim/Common/Component.cpp @@ -159,6 +159,33 @@ void Component::addComponent(Component* subcomponent) extendAddComponent(subcomponent); } +std::unique_ptr Component::extractComponent(Component* subcomponent) +{ + auto& componentsProp = updProperty_components(); + + // Try to find `subcomponent` in the `components` property. + int idx = -1; + for (int i = 0; i < componentsProp.size(); ++i) { + if (&componentsProp[i] == subcomponent) { + idx = i; + break; + } + } + if (idx == -1) { + return nullptr; // Not found. + } + + // Perform removal + std::unique_ptr rv = componentsProp.extractValueAtIndex(idx); + finalizeFromProperties(); + return rv; +} + +bool Component::removeComponent(Component* subcomponent) +{ + return extractComponent(subcomponent) != nullptr; // `std::unique_ptr` handles destruction +} + void Component::prependComponentPathToConnecteePath( Component& subcomponent) { const std::string compPath = subcomponent.getAbsolutePathString(); diff --git a/OpenSim/Common/Component.h b/OpenSim/Common/Component.h index 1a760f7ec7..3f01fe908b 100644 --- a/OpenSim/Common/Component.h +++ b/OpenSim/Common/Component.h @@ -55,6 +55,8 @@ #include #include +#include +#include #include #include @@ -670,6 +672,41 @@ OpenSim_DECLARE_ABSTRACT_OBJECT(Component, Object); * @param subcomponent is the Component to be added. */ void addComponent(Component* subcomponent); +#ifndef SWIG + /** + * Returns `subcomponent` if `subcomponent` was successfully extracted from + * this `Component`. + * + * Specifically, this tries to extract a direct sub-`Component` that was + * previously added via `addComponent`, or existed in the `` list + * XML of this `Component`. + */ + std::unique_ptr extractComponent(Component* subcomponent); + + template< + typename T, + std::enable_if_t, bool> = true + > + std::unique_ptr extractComponent(T* subcomponent) + { + // If the type of `subcomponent` is known at compile-time then it is also + // known that the return value will be `subcomponent`, which will have the + // same type. + std::unique_ptr typeErased = extractComponent(static_cast(subcomponent)); + return std::unique_ptr{static_cast(typeErased.release())}; + } +#endif + + /** + * Returns `true` if `subcomponent`, which should be a direct subcomponent + * of this component, was successfully removed from this component. + * + * Specifically, this tries to remove a direct component that was + * previously added via `addComponent`, or existed in the `` + * list XML for this component. + */ + bool removeComponent(Component* subcomponent); + /** * Get an iterator through the underlying subcomponents that this component is * composed of. The hierarchy of Components/subComponents forms a tree. diff --git a/OpenSim/Common/Property.h b/OpenSim/Common/Property.h index f14cbe4e88..106c802f57 100644 --- a/OpenSim/Common/Property.h +++ b/OpenSim/Common/Property.h @@ -35,6 +35,7 @@ #include "SimTKcommon/internal/Array.h" #include "SimTKcommon/internal/ClonePtr.h" +#include #include #include @@ -519,15 +520,22 @@ class Property : public AbstractProperty { return adoptAndAppendValueVirtual(value); } /** Remove specific entry of the list at index **/ - void removeValueAtIndex(int index) { - if (index > getNumValues() || index < 0) - throw OpenSim::Exception("Property::removeValueAtIndex(): Property " + getName() - + "invalid index, out of range, ignored."); - if (getNumValues() - 1 < this->getMinListSize() || getNumValues() - 1 > this->getMaxListSize()) - throw OpenSim::Exception("Property::removeValueAtIndex(): Property " + getName() - + "resulting list has improper size, ignored."); - removeValueAtIndexVirtual(index); + void removeValueAtIndex(int index) { extractValueAtIndex(index); } + +#ifndef SWIG + /** Extract a specific entry of the list at index **/ + std::unique_ptr extractValueAtIndex(int index) + { + if (index > getNumValues() || index < 0) { + throw OpenSim::Exception("Property::extractValueAtIndex(): Property " + getName() + "invalid index, out of range, ignored."); + } + if (getNumValues() - 1 < this->getMinListSize() || getNumValues() - 1 > this->getMaxListSize()) { + throw OpenSim::Exception("Property::extractValueAtIndex(): Property " + getName() + "resulting list has improper size, ignored."); + } + return extractValueAtIndexVirtual(index); } +#endif + /** Search the value list for an element that has the given \a value and return its index if found, otherwise -1. This requires only that the template type T supports operator==(). This is a linear search so will @@ -600,7 +608,7 @@ class Property : public AbstractProperty { virtual void setValueVirtual(int index, const T& value) = 0; virtual int appendValueVirtual(const T& value) = 0; virtual int adoptAndAppendValueVirtual(T* value) = 0; - virtual void removeValueAtIndexVirtual(int index) = 0; + virtual std::unique_ptr extractValueAtIndexVirtual(int index) = 0; /** @endcond **/ #endif }; @@ -1017,8 +1025,17 @@ class SimpleProperty : public Property { { values.push_back(*valuep); // make a copy delete valuep; // throw out the old one return values.size()-1; } - void removeValueAtIndexVirtual(int index) override final - { values.erase(&values[index]); } + +#ifndef SWIG + std::unique_ptr extractValueAtIndexVirtual(int index) override final + { + T* p = &values.at(index); + auto extracted = std::make_unique(*p); + values.erase(p); + return extracted; + } +#endif + // This is the default implementation; specialization is required if // the Simbody default behavior is different than OpenSim's; e.g. for // Transform serialization. @@ -1195,10 +1212,17 @@ class ObjectProperty : public Property { return i; return -1; } - // Remove value at specific index - void removeValueAtIndexVirtual(int index) override { - objects.erase(&objects.at(index)); + +#ifndef SWIG + // Extract (remove and return) a value at a specific index + std::unique_ptr extractValueAtIndexVirtual(int index) override + { + SimTK::ClonePtr* p = &objects.at(index); + std::unique_ptr extracted{p->release()}; + objects.erase(p); + return extracted; } +#endif private: // Base class checks the index. const T& getValueVirtual(int index) const override final diff --git a/OpenSim/Common/Test/testComponentInterface.cpp b/OpenSim/Common/Test/testComponentInterface.cpp index 74db2cc5fe..98c1c55f15 100644 --- a/OpenSim/Common/Test/testComponentInterface.cpp +++ b/OpenSim/Common/Test/testComponentInterface.cpp @@ -78,6 +78,18 @@ namespace constructProperty_func(); } }; + + class BlankComponent final : public OpenSim::Component { + OpenSim_DECLARE_CONCRETE_OBJECT(BlankComponent, OpenSim::Component); + }; + + class ComponentWithObjectProperty final : public OpenSim::Component { + OpenSim_DECLARE_CONCRETE_OBJECT(ComponentWithObjectProperty, OpenSim::Component); + public: + OpenSim_DECLARE_PROPERTY(object_property, BlankComponent, "an object property"); + + ComponentWithObjectProperty() { constructProperty_object_property(BlankComponent{}); } + }; } using namespace OpenSim; @@ -3413,6 +3425,51 @@ TEST_CASE("Component Interface: Incorrectly Reading an Optional Object Property } } +TEST_CASE("Component Interface: removeComponent returns `true` if it removes a component added via addComponent") +{ + BlankComponent c; + BlankComponent* child = new BlankComponent{}; + c.addComponent(child); + CHECK(c.removeComponent(child)); +} + +TEST_CASE("Component Interface: removeComponent returns `false` if given a `Component` that wasn't added via addComponent") +{ + BlankComponent c; + auto unrelated = std::make_unique(); + CHECK(!c.removeComponent(unrelated.get())); +} + +TEST_CASE("Component Interface: removeComponent returns `false` if caller tries to remove a direct object property") +{ + ComponentWithObjectProperty c; + Component* property = &c.upd_object_property(); + CHECK(!c.removeComponent(property)); +} + +TEST_CASE("Component Interface: extractComponent returns the component added via addComponent") +{ + BlankComponent c; + BlankComponent* child = new BlankComponent{}; + c.addComponent(child); + auto extracted = c.extractComponent(child); + CHECK(extracted.get() == child); +} + +TEST_CASE("Component Interface: extractComponent returns nullptr if given a `Component` that wasn't added via addComponent") +{ + BlankComponent c; + auto unrelated = std::make_unique(); + CHECK(c.extractComponent(unrelated.get()) == nullptr); +} + +TEST_CASE("Component Interface: extractComponent returns `nullptr` if caller tries to remove a direct object property") +{ + ComponentWithObjectProperty c; + Component* property = &c.upd_object_property(); + CHECK(c.extractComponent(property).get() == nullptr); +} + const bool g_TestFixtureTypesAreRegistered = []() { // ensure new types are globally registered for testing deserialization