diff --git a/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesManager.cpp b/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesManager.cpp index 22918b7173..a018a55210 100644 --- a/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesManager.cpp +++ b/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesManager.cpp @@ -303,18 +303,11 @@ namespace AzFramework AZ::Entity* SpawnableEntitiesManager::CloneSingleEntity(const AZ::Entity& entityTemplate, EntityIdMap& templateToCloneMap, AZ::SerializeContext& serializeContext) { - if (!entityTemplate.GetComponents().empty()) - { - // If the same ID gets remapped more than once, preserve the original remapping instead of overwriting it. - constexpr bool allowDuplicateIds = false; + // If the same ID gets remapped more than once, preserve the original remapping instead of overwriting it. + constexpr bool allowDuplicateIds = false; - return AZ::IdUtils::Remapper::CloneObjectAndGenerateNewIdsAndFixRefs( - &entityTemplate, templateToCloneMap, &serializeContext); - } - else - { - return nullptr; - } + return AZ::IdUtils::Remapper::CloneObjectAndGenerateNewIdsAndFixRefs( + &entityTemplate, templateToCloneMap, &serializeContext); } AZ::Entity* SpawnableEntitiesManager::CloneSingleAliasedEntity( @@ -468,9 +461,8 @@ namespace AzFramework if (aliasIt == aliasEnd || aliasIt->m_sourceIndex != i) { - AZ::Entity* clone = - CloneSingleEntity(*entitiesToSpawn[i], ticket.m_entityIdReferenceMap, *request.m_serializeContext); - spawnedEntities.emplace_back(clone); + spawnedEntities.emplace_back( + CloneSingleEntity(*entitiesToSpawn[i], ticket.m_entityIdReferenceMap, *request.m_serializeContext)); spawnedEntityIndices.push_back(i); } else @@ -483,21 +475,9 @@ namespace AzFramework AZ::Entity* clone = CloneSingleAliasedEntity( *entitiesToSpawn[i], *aliasIt, ticket.m_entityIdReferenceMap, previousEntity, *request.m_serializeContext); - // Not all alias operations create a new instance. It's also possible for an empty entity to be left behind, - // in which case it's also filtered out as the entity component framework doesn't handle these gracefully. - if (clone) - { - if (!clone->GetComponents().empty()) - { - previousEntity = clone; - spawnedEntities.emplace_back(clone); - spawnedEntityIndices.push_back(i); - } - else - { - delete clone; - } - } + previousEntity = clone; + spawnedEntities.emplace_back(clone); + spawnedEntityIndices.push_back(i); ++aliasIt; } while (aliasIt != aliasEnd && aliasIt->m_sourceIndex == i); } @@ -519,8 +499,13 @@ namespace AzFramework // Add to the game context, now the entities are active for (auto it = newEntitiesBegin; it != newEntitiesEnd; ++it) { - (*it)->SetSpawnTicketId(request.m_ticketId); - GameEntityContextRequestBus::Broadcast(&GameEntityContextRequestBus::Events::AddGameEntity, *it); + AZ::Entity* clone = (*it); + // The entity component framework doesn't handle entities without TransformComponent safely. + if (!clone->GetComponents().empty()) + { + clone->SetSpawnTicketId(request.m_ticketId); + GameEntityContextRequestBus::Broadcast(&GameEntityContextRequestBus::Events::AddGameEntity, *it); + } } // Let other systems know about newly spawned entities for any post-processing after adding to the scene/game context. @@ -585,10 +570,8 @@ namespace AzFramework RefreshEntityIdMapping( entitiesToSpawn[index].get()->GetId(), ticket.m_entityIdReferenceMap, ticket.m_previouslySpawned); - AZ::Entity* clone = - CloneSingleEntity(*entitiesToSpawn[index], ticket.m_entityIdReferenceMap, *request.m_serializeContext); - AZ_Assert(clone != nullptr, "Failed to clone spawnable entity."); - spawnedEntities.push_back(clone); + spawnedEntities.push_back( + CloneSingleEntity(*entitiesToSpawn[index], ticket.m_entityIdReferenceMap, *request.m_serializeContext)); spawnedEntityIndices.push_back(index); } } @@ -612,9 +595,8 @@ namespace AzFramework if (aliasIt == aliasEnd) { - AZ::Entity* clone = - CloneSingleEntity(*entitiesToSpawn[index], ticket.m_entityIdReferenceMap, *request.m_serializeContext); - spawnedEntities.emplace_back(clone); + spawnedEntities.emplace_back( + CloneSingleEntity(*entitiesToSpawn[index], ticket.m_entityIdReferenceMap, *request.m_serializeContext)); spawnedEntityIndices.push_back(index); } else @@ -627,22 +609,10 @@ namespace AzFramework AZ::Entity* clone = CloneSingleAliasedEntity( *entitiesToSpawn[index], *aliasIt, ticket.m_entityIdReferenceMap, previousEntity, *request.m_serializeContext); - // Not all alias operations create a new instance. It's also possible for an empty entity to be left - // behind, in which case it's also filtered out as the entity component framework doesn't handle these - // gracefully. - if (clone) - { - if (!clone->GetComponents().empty()) - { - previousEntity = clone; - spawnedEntities.emplace_back(clone); - spawnedEntityIndices.push_back(index); - } - else - { - delete clone; - } - } + previousEntity = clone; + spawnedEntities.emplace_back(clone); + spawnedEntityIndices.push_back(index); + ++aliasIt; } while (aliasIt != aliasEnd && aliasIt->m_sourceIndex == index); } @@ -663,8 +633,13 @@ namespace AzFramework // Add to the game context, now the entities are active for (auto it = ticket.m_spawnedEntities.begin() + spawnedEntitiesInitialCount; it != ticket.m_spawnedEntities.end(); ++it) { - (*it)->SetSpawnTicketId(request.m_ticketId); - GameEntityContextRequestBus::Broadcast(&GameEntityContextRequestBus::Events::AddGameEntity, *it); + AZ::Entity* clone = (*it); + // The entity component framework doesn't handle entities without TransformComponent safely. + if (!clone->GetComponents().empty()) + { + clone->SetSpawnTicketId(request.m_ticketId); + GameEntityContextRequestBus::Broadcast(&GameEntityContextRequestBus::Events::AddGameEntity, *it); + } } if (request.m_completionCallback) @@ -965,13 +940,18 @@ namespace AzFramework { for (AZ::Entity* entity : request.m_ticket->m_spawnedEntities) { - if (entity != nullptr) + if (entity != nullptr && !entity->GetComponents().empty()) { - // Setting it to 0 is needed to avoid the infite loop between GameEntityContext and SpawnableEntitiesManager. + // Setting it to 0 is needed to avoid the infinite loop between GameEntityContext and SpawnableEntitiesManager. entity->SetSpawnTicketId(0); GameEntityContextRequestBus::Broadcast( &GameEntityContextRequestBus::Events::DestroyGameEntity, entity->GetId()); } + else + { + // Entities without components wouldn't have been send to the GameEntityContext. + delete entity; + } } delete request.m_ticket; diff --git a/Code/Framework/AzFramework/Tests/Spawnable/SpawnableEntitiesManagerTests.cpp b/Code/Framework/AzFramework/Tests/Spawnable/SpawnableEntitiesManagerTests.cpp index 2dc32d14d5..ff48f73769 100644 --- a/Code/Framework/AzFramework/Tests/Spawnable/SpawnableEntitiesManagerTests.cpp +++ b/Code/Framework/AzFramework/Tests/Spawnable/SpawnableEntitiesManagerTests.cpp @@ -403,7 +403,7 @@ namespace UnitTest static constexpr size_t NumEntities = 4; FillSpawnable(NumEntities); - AZStd::vector indices = { 0, 2, 3, 1 }; + AZStd::vector indices = { 0, 2, 3, 1 }; size_t spawnedEntitiesCount = 0; auto callback = [&spawnedEntitiesCount](AzFramework::EntitySpawnTicket::Id, AzFramework::SpawnableConstEntityContainerView entities) @@ -423,7 +423,7 @@ namespace UnitTest static constexpr size_t NumEntities = 1; FillSpawnable(NumEntities); - AZStd::vector indices = { 0, 0 }; + AZStd::vector indices = { 0, 0 }; size_t spawnedEntitiesCount = 0; auto callback = @@ -444,7 +444,7 @@ namespace UnitTest static constexpr size_t NumEntities = 4; FillSpawnable(NumEntities); - AZStd::vector indices = { 0, 2, 3, 1 }; + AZStd::vector indices = { 0, 2, 3, 1 }; size_t spawnedEntitiesCount = 0; auto callback = @@ -467,7 +467,7 @@ namespace UnitTest FillSpawnable(NumEntities); CreateSingleParent(); - AZStd::vector indices = { 0, 1, 2, 3 }; + AZStd::vector indices = { 0, 1, 2, 3 }; AZStd::vector parents; auto callback = [&parents](AzFramework::EntitySpawnTicket::Id, AzFramework::SpawnableConstEntityContainerView entities) @@ -499,7 +499,7 @@ namespace UnitTest FillSpawnable(NumEntities); CreateSingleParent(); - AZStd::vector indices = { 0, 1, 2, 3 }; + AZStd::vector indices = { 0, 1, 2, 3 }; AZStd::vector parents; auto callback =