Fix crash on GUI entity removal with levels (#913)

Signed-off-by: Ashton Larkin <[email protected]>

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<typename ComponentTypeT>
               ComponentTypeT *CreateComponent(
                   const Entity _entity,
@@ -226,7 +227,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 =

src/EntityComponentManager.cc

@@ -574,7 +574,7 @@ bool EntityComponentManager::CreateComponentImplementation(
       << "exist. This create component request will be ignored." << std::endl;
     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) &&
@@ -603,7 +603,7 @@ bool EntityComponentManager::CreateComponentImplementation(
   switch (compAddResult)
   {
     case 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 ComponentAdditionResult::NEW_ADDITION:

src/EntityComponentManager_TEST.cc

@@ -230,7 +230,7 @@ TEST_P(EntityComponentManagerFixture, EntitiesAndComponents)
   EXPECT_FALSE(manager.HasEntity(kNullEntity));
   EXPECT_FALSE(manager.EntityHasComponentType(kNullEntity,
         IntComponent::typeId));
-  EXPECT_EQ(ComponentKey(), manager.CreateComponent<IntComponent>(kNullEntity,
+  EXPECT_EQ(nullptr, manager.CreateComponent<IntComponent>(kNullEntity,
         IntComponent(123)));
   EXPECT_FALSE(manager.HasEntity(kNullEntity));
   EXPECT_FALSE(manager.EntityHasComponentType(kNullEntity,
@@ -261,6 +261,21 @@ 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));
+  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());