From c821af22d6f8cea361b761e0616f98c1b23dec17 Mon Sep 17 00:00:00 2001 From: Ashton Larkin Date: Mon, 12 Jul 2021 14:48:42 -0600 Subject: [PATCH] fix crash on GUI entity removal with levels Signed-off-by: Ashton Larkin --- .../ignition/gazebo/EntityComponentManager.hh | 5 +++-- src/EntityComponentManager.cc | 6 +++++- src/EntityComponentManager_TEST.cc | 21 +++++++++++++++++++ 3 files changed, 29 insertions(+), 3 deletions(-) diff --git a/include/ignition/gazebo/EntityComponentManager.hh b/include/ignition/gazebo/EntityComponentManager.hh index e0ddcca297..5ae95dc60d 100644 --- a/include/ignition/gazebo/EntityComponentManager.hh +++ b/include/ignition/gazebo/EntityComponentManager.hh @@ -181,7 +181,8 @@ namespace ignition /// the component. /// \param[in] _data Data used to construct the component. /// \return A pointer to the component that was created. nullptr is - /// returned if the component was not able to be created. + /// returned if the component was not able to be created. If _entity + /// does not exist, nullptr will be returned. public: template ComponentTypeT *CreateComponent( const Entity _entity, @@ -210,7 +211,7 @@ namespace ignition /// \param[in] _default The value that should be used to construct /// the component in case the component doesn't exist. /// \return The component of the specified type assigned to the specified - /// entity. + /// entity. If _entity does not exist, nullptr is returned. public: template ComponentTypeT *ComponentDefault(Entity _entity, const typename ComponentTypeT::Type &_default = diff --git a/src/EntityComponentManager.cc b/src/EntityComponentManager.cc index 65fc8c3204..9a54bac74a 100644 --- a/src/EntityComponentManager.cc +++ b/src/EntityComponentManager.cc @@ -569,6 +569,10 @@ bool EntityComponentManager::CreateComponentImplementation( const Entity _entity, const ComponentTypeId _componentTypeId, const components::BaseComponent *_data) { + // make sure that the entity exists + if (!this->HasEntity(_entity)) + return false; + // if this is the first time this component type is being created, make sure // the component type to be created is valid if (!this->HasComponentType(_componentTypeId) && @@ -597,7 +601,7 @@ bool EntityComponentManager::CreateComponentImplementation( switch (compAddResult) { case detail::ComponentAdditionResult::FAILED_ADDITION: - ignwarn << "Attempt to create a component of type [" << _componentTypeId + ignerr << "Attempt to create a component of type [" << _componentTypeId << "] attached to entity [" << _entity << "] failed.\n"; return false; case detail::ComponentAdditionResult::NEW_ADDITION: diff --git a/src/EntityComponentManager_TEST.cc b/src/EntityComponentManager_TEST.cc index 4d878ee92c..95cb38a785 100644 --- a/src/EntityComponentManager_TEST.cc +++ b/src/EntityComponentManager_TEST.cc @@ -250,6 +250,27 @@ TEST_P(EntityComponentManagerFixture, EntitiesAndComponents) EXPECT_TRUE(manager.EntityHasComponentType(entity, IntComponent::typeId)); EXPECT_EQ(123, intComp->Data()); + // Try to create/query a component from an entity that does not exist. nullptr + // should be returned since a component cannot be attached to a non-existent + // entity + EXPECT_FALSE(manager.HasEntity(kNullEntity)); + /* + * TODO(adlarkin) test with the following methods once the deprecation + * process has been sorted out: + * EntityHasComponent + * EntityHasComponentType + */ + EXPECT_EQ(nullptr, manager.CreateComponent(kNullEntity, + IntComponent(123))); + EXPECT_EQ(nullptr, manager.ComponentDefault(kNullEntity, 123)); + EXPECT_EQ(nullptr, manager.Component(kNullEntity)); + EXPECT_FALSE(manager.ComponentData(kNullEntity).has_value()); + EXPECT_EQ(ComponentState::NoChange, manager.ComponentState(kNullEntity, + IntComponent::typeId)); + // (make sure the entity wasn't implicitly created during the invalid + // component calls) + EXPECT_FALSE(manager.HasEntity(kNullEntity)); + // Remove all entities manager.RequestRemoveEntities(); EXPECT_EQ(3u, manager.EntityCount());