Skip to content

Commit

Permalink
fix crash on GUI entity removal with levels
Browse files Browse the repository at this point in the history
Signed-off-by: Ashton Larkin <[email protected]>
  • Loading branch information
adlarkin committed Jul 12, 2021
1 parent ee3d3d0 commit c821af2
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 3 deletions.
5 changes: 3 additions & 2 deletions include/ignition/gazebo/EntityComponentManager.hh
Original file line number Diff line number Diff line change
Expand Up @@ -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<typename ComponentTypeT>
ComponentTypeT *CreateComponent(
const Entity _entity,
Expand Down Expand Up @@ -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<typename ComponentTypeT>
ComponentTypeT *ComponentDefault(Entity _entity,
const typename ComponentTypeT::Type &_default =
Expand Down
6 changes: 5 additions & 1 deletion src/EntityComponentManager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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) &&
Expand Down Expand Up @@ -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:
Expand Down
21 changes: 21 additions & 0 deletions src/EntityComponentManager_TEST.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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<IntComponent>(kNullEntity,
IntComponent(123)));
EXPECT_EQ(nullptr, manager.ComponentDefault<IntComponent>(kNullEntity, 123));
EXPECT_EQ(nullptr, manager.Component<IntComponent>(kNullEntity));
EXPECT_FALSE(manager.ComponentData<IntComponent>(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());
Expand Down

0 comments on commit c821af2

Please sign in to comment.