diff --git a/include/ignition/gazebo/EntityComponentManager.hh b/include/ignition/gazebo/EntityComponentManager.hh index bd692a134b..5d5e076535 100644 --- a/include/ignition/gazebo/EntityComponentManager.hh +++ b/include/ignition/gazebo/EntityComponentManager.hh @@ -203,6 +203,22 @@ namespace ignition public: template ComponentTypeT *Component(const Entity _entity); + /// \brief Get a component based on a key. + /// \param[in] _key A key that uniquely identifies a component. + /// \return The component associated with the key, or nullptr if the + /// component could not be found. + public: template + const ComponentTypeT *IGN_DEPRECATED(6) Component( + const ComponentKey &_key) const; + + /// \brief Get a mutable component based on a key. + /// \param[in] _key A key that uniquely identifies a component. + /// \return The component associated with the key, or nullptr if the + /// component could not be found. + public: template + ComponentTypeT *IGN_DEPRECATED(6) Component( + const ComponentKey &_key); + /// \brief Get a mutable component assigned to an entity based on a /// component type. If the component doesn't exist, create it and /// initialize with the given default value. diff --git a/include/ignition/gazebo/Types.hh b/include/ignition/gazebo/Types.hh index 67ae2cf367..d30ae00d06 100644 --- a/include/ignition/gazebo/Types.hh +++ b/include/ignition/gazebo/Types.hh @@ -22,6 +22,8 @@ #include #include +#include "ignition/gazebo/Entity.hh" + namespace ignition { namespace gazebo @@ -78,8 +80,8 @@ namespace ignition /// \brief A unique identifier for a component instance. The uniqueness /// of a ComponentId is scoped to the component's type. /// \sa ComponentKey. - // TODO(anyone) remove this in ign-gazebo7. This is no longer used in - // ign-gazebo6 + /// \deprecated Deprecated on version 6, removed on version 7. Use + /// ComponentTypeId + Entity instead. using ComponentId = int; /// \brief A unique identifier for a component type. A component type @@ -89,9 +91,10 @@ namespace ignition /// \brief A key that uniquely identifies, at the global scope, a component /// instance - // TODO(anyone) remove this in ign-gazebo7. This is no longer used in - // ign-gazebo6 - using ComponentKey = std::pair; + /// \note On version 6, the 2nd element was changed to the entity ID. + /// \deprecated Deprecated on version 6, removed on version 7. Use + /// ComponentTypeId + Entity instead. + using ComponentKey = std::pair; /// \brief typedef for query callbacks using EntityQueryCallback = std::function #include #include +#include #include #include #include @@ -77,10 +78,58 @@ namespace components } }; + /// \brief A base class for an object responsible for creating storages. + class StorageDescriptorBase + { + /// \brief Constructor + public: IGN_DEPRECATED(6) StorageDescriptorBase() = default; + + /// \brief Destructor + public: virtual ~StorageDescriptorBase() = default; + + /// \brief Create an instance of a storage. + /// \return Pointer to a storage. + public: virtual std::unique_ptr Create() const = 0; + }; + + /// \brief A class for an object responsible for creating storages. + /// \tparam ComponentTypeT type of component that the storage will hold. + template + class StorageDescriptor + : public StorageDescriptorBase + { + /// \brief Constructor + public: IGN_DEPRECATED(6) StorageDescriptor() = default; + + /// \brief Create an instance of a storage that holds ComponentTypeT + /// components. + /// \return Pointer to a component. + public: std::unique_ptr Create() const override + { + return std::make_unique>(); + } + }; + /// \brief A factory that generates a component based on a string type. class Factory : public ignition::common::SingletonT { + /// \brief Register a component so that the factory can create instances + /// of the component and its storage based on an ID. + /// \param[in] _type Type of component to register. + /// \param[in] _compDesc Object to manage the creation of ComponentTypeT + /// objects. + /// \param[in] _storageDesc Ignored. + /// \tparam ComponentTypeT Type of component to register. + /// \deprecated See function that doesn't accept a storage + public: template + void IGN_DEPRECATED(6) Register(const std::string &_type, + ComponentDescriptorBase *_compDesc, + StorageDescriptorBase * /*_storageDesc*/) + { + this->Register(_type, _compDesc); + } + /// \brief Register a component so that the factory can create instances /// of the component based on an ID. /// \param[in] _type Type of component to register. @@ -236,6 +285,16 @@ namespace components return comp; } + /// \brief Create a new instance of a component storage. + /// \param[in] _typeId Type of component which the storage will hold. + /// \return Always returns nullptr. + /// \deprecated Storages aren't necessary anymore. + public: std::unique_ptr IGN_DEPRECATED(6) NewStorage( + const ComponentTypeId & /*_typeId*/) + { + return nullptr; + } + /// \brief Get all the registered component types by ID. /// return Vector of component IDs. public: std::vector TypeIds() const diff --git a/include/ignition/gazebo/detail/ComponentStorageBase.hh b/include/ignition/gazebo/detail/ComponentStorageBase.hh new file mode 100644 index 0000000000..cb4e0edcc2 --- /dev/null +++ b/include/ignition/gazebo/detail/ComponentStorageBase.hh @@ -0,0 +1,55 @@ +/* + * Copyright (C) 2018 Open Source Robotics Foundation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * +*/ +#ifndef IGNITION_GAZEBO_DETAIL_COMPONENTSTORAGEBASE_HH_ +#define IGNITION_GAZEBO_DETAIL_COMPONENTSTORAGEBASE_HH_ + +#include "ignition/gazebo/Export.hh" + +namespace ignition +{ + namespace gazebo + { + // Inline bracket to help doxygen filtering. + inline namespace IGNITION_GAZEBO_VERSION_NAMESPACE { + // + /// \brief All component instances of the same type are stored + /// squentially in memory. This is a base class for storing components + /// of a particular type. + class IGNITION_GAZEBO_HIDDEN ComponentStorageBase + { + /// \brief Constructor + public: IGN_DEPRECATED(6) ComponentStorageBase() = default; + + /// \brief Destructor + public: virtual ~ComponentStorageBase() = default; + }; + + /// \brief Templated implementation of component storage. + template + class IGNITION_GAZEBO_HIDDEN ComponentStorage : public ComponentStorageBase + { + /// \brief Constructor + public: explicit IGN_DEPRECATED(6) ComponentStorage() + : ComponentStorageBase() + { + } + }; + } + } +} + +#endif diff --git a/include/ignition/gazebo/detail/EntityComponentManager.hh b/include/ignition/gazebo/detail/EntityComponentManager.hh index 7289679a92..d07f0bbba4 100644 --- a/include/ignition/gazebo/detail/EntityComponentManager.hh +++ b/include/ignition/gazebo/detail/EntityComponentManager.hh @@ -127,6 +127,23 @@ ComponentTypeT *EntityComponentManager::Component(const Entity _entity) this->ComponentImplementation(_entity, typeId)); } +////////////////////////////////////////////////// +template +const ComponentTypeT *EntityComponentManager::Component( + const ComponentKey &_key) const +{ + return static_cast( + this->ComponentImplementation(_key.second, _key.first)); +} + +////////////////////////////////////////////////// +template +ComponentTypeT *EntityComponentManager::Component(const ComponentKey &_key) +{ + return static_cast( + this->ComponentImplementation(_key.second, _key.first)); +} + ////////////////////////////////////////////////// template ComponentTypeT *EntityComponentManager::ComponentDefault(Entity _entity, diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index f9313c9f2a..590ae8e9e0 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -66,24 +66,24 @@ set (gtest_sources ${gtest_sources} Barrier_TEST.cc BaseView_TEST.cc - Component_TEST.cc ComponentFactory_TEST.cc + Component_TEST.cc Conversions_TEST.cc EntityComponentManager_TEST.cc EntityComponentStorage_TEST.cc EventManager_TEST.cc - ign_TEST.cc Link_TEST.cc Model_TEST.cc SdfEntityCreator_TEST.cc SdfGenerator_TEST.cc - Server_TEST.cc ServerConfig_TEST.cc + Server_TEST.cc SimulationRunner_TEST.cc - System_TEST.cc SystemLoader_TEST.cc + System_TEST.cc Util_TEST.cc World_TEST.cc + ign_TEST.cc network/NetworkConfig_TEST.cc network/PeerTracker_TEST.cc network/NetworkManager_TEST.cc diff --git a/src/EntityComponentManager.cc b/src/EntityComponentManager.cc index 8e5b666137..78956b229d 100644 --- a/src/EntityComponentManager.cc +++ b/src/EntityComponentManager.cc @@ -405,9 +405,6 @@ bool EntityComponentManager::RemoveComponent( bool EntityComponentManager::RemoveComponent( const Entity _entity, const ComponentKey &_key) { - ignwarn << "EntityComponentManager::RemoveComponent is deprecated. Please " - << "use the RemoveComponent method with a ComponentTypeId parameter type " - << "instead." << std::endl; return this->RemoveComponent(_entity, _key.first); } @@ -415,9 +412,6 @@ bool EntityComponentManager::RemoveComponent( bool EntityComponentManager::EntityHasComponent(const Entity _entity, const ComponentKey &_key) const { - ignwarn << "EntityComponentManager::EntityHasComponent is deprecated. Please " - << "use EntityComponentManager::EntityHasComponentType instead." - << std::endl; if (!this->HasEntity(_entity)) return false; auto &compSet = this->dataPtr->entityComponents[_entity]; diff --git a/src/EntityComponentManager_TEST.cc b/src/EntityComponentManager_TEST.cc index 4d878ee92c..54af688f4b 100644 --- a/src/EntityComponentManager_TEST.cc +++ b/src/EntityComponentManager_TEST.cc @@ -21,6 +21,7 @@ #include #include #include +#include #include "ignition/gazebo/components/Factory.hh" #include "ignition/gazebo/components/Pose.hh" @@ -2561,6 +2562,81 @@ TEST_P(EntityComponentManagerFixture, RemovedComponentsSyncBetweenServerAndGUI) } } +///////////////////////////////////////////////// +// Check that some widely used deprecated APIs still work +TEST_P(EntityComponentManagerFixture, Deprecated) +{ + IGN_UTILS_WARN_IGNORE__DEPRECATED_DECLARATION + + // Fail to create component for inexistent entity + EXPECT_EQ(nullptr, manager.CreateComponent(789, + IntComponent(123))); + + // Create some entities + auto eInt = manager.CreateEntity(); + auto eDouble = manager.CreateEntity(); + auto eIntDouble = manager.CreateEntity(); + EXPECT_EQ(3u, manager.EntityCount()); + + // Add components and keep their unique ComponentKeys + EXPECT_NE(nullptr, manager.CreateComponent(eInt, + IntComponent(123))); + ComponentKey cIntEInt = {IntComponent::typeId, eInt}; + + EXPECT_NE(nullptr, manager.CreateComponent(eDouble, + DoubleComponent(0.123))); + ComponentKey cDoubleEDouble = {DoubleComponent::typeId, eDouble}; + + EXPECT_NE(nullptr, manager.CreateComponent(eIntDouble, + IntComponent(456))); + ComponentKey cIntEIntDouble = {IntComponent::typeId, eIntDouble}; + + EXPECT_NE(nullptr, manager.CreateComponent(eIntDouble, + DoubleComponent(0.456))); + ComponentKey cDoubleEIntDouble = {DoubleComponent::typeId, eIntDouble}; + + // Check entities have the components + EXPECT_TRUE(manager.EntityHasComponent(eInt, cIntEInt)); + EXPECT_EQ(1u, manager.ComponentTypes(eInt).size()); + EXPECT_EQ(IntComponent::typeId, *manager.ComponentTypes(eInt).begin()); + + EXPECT_TRUE(manager.EntityHasComponent(eDouble, cDoubleEDouble)); + EXPECT_EQ(1u, manager.ComponentTypes(eDouble).size()); + EXPECT_EQ(DoubleComponent::typeId, *manager.ComponentTypes(eDouble).begin()); + + EXPECT_TRUE(manager.EntityHasComponent(eIntDouble, cIntEIntDouble)); + EXPECT_TRUE(manager.EntityHasComponent(eIntDouble, cDoubleEIntDouble)); + EXPECT_EQ(2u, manager.ComponentTypes(eIntDouble).size()); + auto types = manager.ComponentTypes(eIntDouble); + EXPECT_NE(types.end(), types.find(IntComponent::typeId)); + EXPECT_NE(types.end(), types.find(DoubleComponent::typeId)); + + // Remove component by key + EXPECT_TRUE(manager.RemoveComponent(eInt, cIntEInt)); + EXPECT_FALSE(manager.EntityHasComponent(eInt, cIntEInt)); + EXPECT_TRUE(manager.ComponentTypes(eInt).empty()); + + // Remove component by type id + auto typeDouble = DoubleComponent::typeId; + + EXPECT_TRUE(manager.RemoveComponent(eDouble, typeDouble)); + EXPECT_FALSE(manager.EntityHasComponent(eDouble, cDoubleEDouble)); + EXPECT_TRUE(manager.ComponentTypes(eDouble).empty()); + + // Remove component by type + EXPECT_TRUE(manager.RemoveComponent(eIntDouble)); + EXPECT_FALSE(manager.EntityHasComponent(eIntDouble, cIntEIntDouble)); + EXPECT_TRUE(manager.EntityHasComponent(eIntDouble, cDoubleEIntDouble)); + EXPECT_EQ(1u, manager.ComponentTypes(eIntDouble).size()); + + EXPECT_TRUE(manager.RemoveComponent(eIntDouble)); + EXPECT_FALSE(manager.EntityHasComponent(eIntDouble, cIntEIntDouble)); + EXPECT_FALSE(manager.EntityHasComponent(eIntDouble, cDoubleEIntDouble)); + EXPECT_EQ(0u, manager.ComponentTypes(eIntDouble).size()); + + IGN_UTILS_WARN_RESUME__DEPRECATED_DECLARATION +} + // Run multiple times. We want to make sure that static globals don't cause // problems. INSTANTIATE_TEST_SUITE_P(EntityComponentManagerRepeat, diff --git a/test/integration/view.cc b/test/integration/view.cc index 3bfd3c7432..b7497e10f3 100644 --- a/test/integration/view.cc +++ b/test/integration/view.cc @@ -62,10 +62,10 @@ class ViewTest : public::testing::Test } }; -/// \brief Helper class that wraps EntityComponentStorage to provide a simplified -/// API for working with entities and their components. This class also stores -/// a set of user-defined views and handles updating views as needed whenever -/// an entity's components are modified. +/// \brief Helper class that wraps EntityComponentStorage to provide a +/// simplified API for working with entities and their components. This class +/// also stores a set of user-defined views and handles updating views as +/// needed whenever an entity's components are modified. class StorageViewWrapper { /// \brief Constructor @@ -187,8 +187,8 @@ class StorageViewWrapper EXPECT_EQ(entityComp->Data(), _component.Data()); } - /// \brief Helper function that uses EntityComponentStorage::ValidComponent to get - /// a component of a particular type that belongs to an entity. + /// \brief Helper function that uses EntityComponentStorage::ValidComponent + /// to get a component of a particular type that belongs to an entity. /// \param[in] _entity The entity /// \return A pointer to the component of the templated type. If no such /// component exists, nullptr is returned