From c9f9a83c57af7c464e6e7d2b3fef1183aac1616e Mon Sep 17 00:00:00 2001 From: AMZN-koppersr <82230785+AMZN-koppersr@users.noreply.github.com> Date: Mon, 8 Nov 2021 12:00:15 -0800 Subject: [PATCH] Further PR feedback on the Spawnble Entity Aliases. Signed-off-by: AMZN-koppersr <82230785+AMZN-koppersr@users.noreply.github.com> --- .../Spawnable/SpawnableEntitiesManager.cpp | 2 +- .../SpawnableEntitiesManagerTests.cpp | 12 +- .../Prefab/Spawnable/SpawnableUtils.cpp | 118 +++++++++++------- .../Prefab/Spawnable/SpawnableUtils.h | 6 +- 4 files changed, 80 insertions(+), 58 deletions(-) diff --git a/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesManager.cpp b/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesManager.cpp index 17a18dd5e5..1e863877a4 100644 --- a/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesManager.cpp +++ b/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesManager.cpp @@ -944,7 +944,7 @@ namespace AzFramework { for (AZ::Entity* entity : request.m_ticket->m_spawnedEntities) { - if (entity != nullptr && !entity->GetComponents().empty()) + if (entity != nullptr) { // Setting it to 0 is needed to avoid the infinite loop between GameEntityContext and SpawnableEntitiesManager. entity->SetSpawnTicketId(0); diff --git a/Code/Framework/AzFramework/Tests/Spawnable/SpawnableEntitiesManagerTests.cpp b/Code/Framework/AzFramework/Tests/Spawnable/SpawnableEntitiesManagerTests.cpp index 535ceab30a..0dc00f81dd 100644 --- a/Code/Framework/AzFramework/Tests/Spawnable/SpawnableEntitiesManagerTests.cpp +++ b/Code/Framework/AzFramework/Tests/Spawnable/SpawnableEntitiesManagerTests.cpp @@ -607,7 +607,7 @@ namespace UnitTest &target); size_t spawnedEntitiesCount = 0; - bool allReplaced = true; + bool allReplaced = false; auto callback = [&spawnedEntitiesCount, &allReplaced]( AzFramework::EntitySpawnTicket::Id, AzFramework::SpawnableConstEntityContainerView entities) { @@ -636,7 +636,7 @@ namespace UnitTest &target); size_t spawnedEntitiesCount = 0; - bool allAdded = true; + bool allAdded = false; auto callback = [&spawnedEntitiesCount, &allAdded]( AzFramework::EntitySpawnTicket::Id, AzFramework::SpawnableConstEntityContainerView entities) { @@ -665,7 +665,7 @@ namespace UnitTest &target); size_t spawnedEntitiesCount = 0; - bool allMerged = true; + bool allMerged = false; auto callback = [&spawnedEntitiesCount, &allMerged]( AzFramework::EntitySpawnTicket::Id, AzFramework::SpawnableConstEntityContainerView entities) { @@ -1105,7 +1105,7 @@ namespace UnitTest AZStd::vector indices = { 0, 2, 3, 1 }; size_t spawnedEntitiesCount = 0; - bool allReplaced = true; + bool allReplaced = false; auto callback = [&spawnedEntitiesCount, &allReplaced]( AzFramework::EntitySpawnTicket::Id, AzFramework::SpawnableConstEntityContainerView entities) { @@ -1136,7 +1136,7 @@ namespace UnitTest AZStd::vector indices = { 0, 2, 3, 1 }; size_t spawnedEntitiesCount = 0; - bool allAdded = true; + bool allAdded = false; auto callback = [&spawnedEntitiesCount, &allAdded]( AzFramework::EntitySpawnTicket::Id, AzFramework::SpawnableConstEntityContainerView entities) @@ -1168,7 +1168,7 @@ namespace UnitTest AZStd::vector indices = { 0, 2, 3, 1 }; size_t spawnedEntitiesCount = 0; - bool allMerged = true; + bool allMerged = false; auto callback = [&spawnedEntitiesCount, &allMerged]( AzFramework::EntitySpawnTicket::Id, AzFramework::SpawnableConstEntityContainerView entities) { diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Spawnable/SpawnableUtils.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Spawnable/SpawnableUtils.cpp index dfe82167c2..ab3c52f6d0 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Spawnable/SpawnableUtils.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Spawnable/SpawnableUtils.cpp @@ -170,7 +170,7 @@ namespace AzToolsFramework::Prefab::SpawnableUtils AzToolsFramework::Prefab::Instance& source, AZStd::string targetPrefabName, AzToolsFramework::Prefab::Instance& target, - AZ::EntityId entity, + AZ::EntityId entityId, AzToolsFramework::Prefab::PrefabConversionUtils::EntityAliasType aliasType, AzToolsFramework::Prefab::PrefabConversionUtils::EntityAliasSpawnableLoadBehavior loadBehavior, uint32_t tag, @@ -178,28 +178,35 @@ namespace AzToolsFramework::Prefab::SpawnableUtils { using namespace AzToolsFramework::Prefab::PrefabConversionUtils; - AliasPath alias = source.GetAliasPathRelativeToInstance(entity); + AliasPath alias = source.GetAliasPathRelativeToInstance(entityId); if (!alias.empty()) { - auto&& [replacement, storedAliasType] = Internal::ApplyAlias(source, entity, aliasType); + auto&& [replacement, storedAliasType] = Internal::ApplyAlias(source, entityId, aliasType); + if (replacement) + { + AZ::Entity* result = replacement.get(); + target.AddEntity(AZStd::move(replacement), alias.Filename().Native()); - AZ::Entity* result = replacement.get(); - target.AddEntity(AZStd::move(replacement), alias.Filename().Native()); + EntityAliasStore store; + store.m_aliasType = storedAliasType; + store.m_source.emplace(AZStd::move(sourcePrefabName), AZStd::move(alias)); + store.m_target.emplace( + AZStd::move(targetPrefabName), target.GetAliasPathRelativeToInstance(result->GetId())); + store.m_loadBehavior = loadBehavior; + store.m_tag = tag; + context.RegisterSpawnableEntityAlias(AZStd::move(store)); - EntityAliasStore store; - store.m_aliasType = storedAliasType; - store.m_source.emplace(AZStd::move(sourcePrefabName), AZStd::move(alias)); - store.m_target.emplace( - AZStd::move(targetPrefabName), target.GetAliasPathRelativeToInstance(result->GetId())); - store.m_loadBehavior = loadBehavior; - store.m_tag = tag; - context.RegisterSpawnableEntityAlias(AZStd::move(store)); - - return result; + return result; + } + else + { + AZ_Assert(false, "A replacement for entity with id %zu could not be created.", static_cast(entityId)); + return nullptr; + } } else { - AZ_Assert(false, "Entity with id %zu was not found in the source prefab.", static_cast(entity)); + AZ_Assert(false, "Entity with id %zu was not found in the source prefab.", static_cast(entityId)); return nullptr; } } @@ -208,7 +215,7 @@ namespace AzToolsFramework::Prefab::SpawnableUtils AZStd::string sourcePrefabName, AzToolsFramework::Prefab::Instance& source, AzFramework::Spawnable& target, - AZ::EntityId entity, + AZ::EntityId entityId, AzToolsFramework::Prefab::PrefabConversionUtils::EntityAliasType aliasType, AzToolsFramework::Prefab::PrefabConversionUtils::EntityAliasSpawnableLoadBehavior loadBehavior, uint32_t tag, @@ -216,17 +223,58 @@ namespace AzToolsFramework::Prefab::SpawnableUtils { using namespace AzToolsFramework::Prefab::PrefabConversionUtils; - AliasPath alias = source.GetAliasPathRelativeToInstance(entity); + AliasPath alias = source.GetAliasPathRelativeToInstance(entityId); if (!alias.empty()) { - auto&& [replacement, storedAliasType] = Internal::ApplyAlias(source, entity, aliasType); + auto&& [replacement, storedAliasType] = Internal::ApplyAlias(source, entityId, aliasType); + if (replacement) + { + AZ::Entity* result = replacement.get(); + target.GetEntities().push_back(AZStd::move(replacement)); + EntityAliasStore store; + store.m_aliasType = storedAliasType; + store.m_source.emplace(AZStd::move(sourcePrefabName), AZStd::move(alias)); + store.m_target.emplace(target, result->GetId()); + store.m_tag = tag; + store.m_loadBehavior = loadBehavior; + context.RegisterSpawnableEntityAlias(AZStd::move(store)); + + return result; + } + else + { + AZ_Assert(false, "A replacement for entity with id %zu could not be created.", static_cast(entityId)); + return nullptr; + } + } + else + { + AZ_Assert(false, "Entity with id %llu was not found in the source prefab.", static_cast(entityId)); + return nullptr; + } + } + + AZ::Entity* CreateEntityAlias( + AzFramework::Spawnable& source, + AzFramework::Spawnable& target, + AZ::EntityId entityId, + AzToolsFramework::Prefab::PrefabConversionUtils::EntityAliasType aliasType, + AzToolsFramework::Prefab::PrefabConversionUtils::EntityAliasSpawnableLoadBehavior loadBehavior, + uint32_t tag, + AzToolsFramework::Prefab::PrefabConversionUtils::PrefabProcessorContext& context) + { + using namespace AzToolsFramework::Prefab::PrefabConversionUtils; + + auto&& [replacement, storedAliasType] = Internal::ApplyAlias(source, entityId, aliasType); + if (replacement) + { AZ::Entity* result = replacement.get(); target.GetEntities().push_back(AZStd::move(replacement)); - + EntityAliasStore store; store.m_aliasType = storedAliasType; - store.m_source.emplace(AZStd::move(sourcePrefabName), AZStd::move(alias)); + store.m_source.emplace(source, entityId); store.m_target.emplace(target, result->GetId()); store.m_tag = tag; store.m_loadBehavior = loadBehavior; @@ -236,37 +284,11 @@ namespace AzToolsFramework::Prefab::SpawnableUtils } else { - AZ_Assert(false, "Entity with id %llu was not found in the source prefab.", static_cast(entity)); + AZ_Assert(false, "A replacement for entity with id %zu could not be created.", static_cast(entityId)); return nullptr; } } - AZ::Entity* CreateEntityAlias( - AzFramework::Spawnable& source, - AzFramework::Spawnable& target, - AZ::EntityId entity, - AzToolsFramework::Prefab::PrefabConversionUtils::EntityAliasType aliasType, - AzToolsFramework::Prefab::PrefabConversionUtils::EntityAliasSpawnableLoadBehavior loadBehavior, - uint32_t tag, - AzToolsFramework::Prefab::PrefabConversionUtils::PrefabProcessorContext& context) - { - using namespace AzToolsFramework::Prefab::PrefabConversionUtils; - - auto&& [replacement, storedAliasType] = Internal::ApplyAlias(source, entity, aliasType); - AZ::Entity* result = replacement.get(); - target.GetEntities().push_back(AZStd::move(replacement)); - - EntityAliasStore store; - store.m_aliasType = storedAliasType; - store.m_source.emplace(source, entity); - store.m_target.emplace(target, result->GetId()); - store.m_tag = tag; - store.m_loadBehavior = loadBehavior; - context.RegisterSpawnableEntityAlias(AZStd::move(store)); - - return result; - } - uint32_t FindEntityIndex(AZ::EntityId entity, const AzFramework::Spawnable& spawnable) { auto begin = spawnable.GetEntities().begin(); diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Spawnable/SpawnableUtils.h b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Spawnable/SpawnableUtils.h index 892b83455d..ea8a49857e 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Spawnable/SpawnableUtils.h +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Spawnable/SpawnableUtils.h @@ -36,7 +36,7 @@ namespace AzToolsFramework::Prefab::SpawnableUtils AzToolsFramework::Prefab::Instance& source, AZStd::string targetPrefabName, AzToolsFramework::Prefab::Instance& target, - AZ::EntityId entity, + AZ::EntityId entityId, AzToolsFramework::Prefab::PrefabConversionUtils::EntityAliasType aliasType, AzToolsFramework::Prefab::PrefabConversionUtils::EntityAliasSpawnableLoadBehavior loadBehavior, uint32_t tag, @@ -45,7 +45,7 @@ namespace AzToolsFramework::Prefab::SpawnableUtils AZStd::string sourcePrefabName, AzToolsFramework::Prefab::Instance& source, AzFramework::Spawnable& target, - AZ::EntityId entity, + AZ::EntityId entityId, AzToolsFramework::Prefab::PrefabConversionUtils::EntityAliasType aliasType, AzToolsFramework::Prefab::PrefabConversionUtils::EntityAliasSpawnableLoadBehavior loadBehavior, uint32_t tag, @@ -53,7 +53,7 @@ namespace AzToolsFramework::Prefab::SpawnableUtils AZ::Entity* CreateEntityAlias( AzFramework::Spawnable& source, AzFramework::Spawnable& target, - AZ::EntityId entity, + AZ::EntityId entityId, AzToolsFramework::Prefab::PrefabConversionUtils::EntityAliasType aliasType, AzToolsFramework::Prefab::PrefabConversionUtils::EntityAliasSpawnableLoadBehavior loadBehavior, uint32_t tag,