From 2dee4d3ac92fef4268ae313ad06b283a68e548d2 Mon Sep 17 00:00:00 2001 From: sconel Date: Wed, 9 Jun 2021 17:37:41 -0700 Subject: [PATCH 1/4] Add support to export nested container entities. Resolves missing parents in spawnables --- .../AzToolsFramework/Prefab/Instance/Instance.cpp | 5 +++-- .../AzToolsFramework/Prefab/Instance/Instance.h | 9 ++++++++- .../AzToolsFramework/Prefab/Spawnable/SpawnableUtils.cpp | 6 +----- 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Instance/Instance.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Instance/Instance.cpp index 766a293b4a..83a76aeb01 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Instance/Instance.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Instance/Instance.cpp @@ -187,13 +187,14 @@ namespace AzToolsFramework return removedEntity; } - void Instance::DetachNestedEntities(const AZStd::function)>& callback) + void Instance::DetachAllEntitiesInHierarchy(const AZStd::function)>& callback) { + callback(AZStd::move(DetachContainerEntity())); DetachEntities(callback); for (const auto& [instanceAlias, instance] : m_nestedInstances) { - instance->DetachNestedEntities(callback); + instance->DetachAllEntitiesInHierarchy(callback); } } diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Instance/Instance.h b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Instance/Instance.h index d14203e2e5..bba20cbe8a 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Instance/Instance.h +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Instance/Instance.h @@ -87,7 +87,14 @@ namespace AzToolsFramework bool AddEntity(AZ::Entity& entity, EntityAlias entityAlias); AZStd::unique_ptr DetachEntity(const AZ::EntityId& entityId); void DetachEntities(const AZStd::function)>& callback); - void DetachNestedEntities(const AZStd::function)>& callback); + + /** + * Detaches all entities in the instance hierarchy, including; all direct and nested entities, all container entities. + * Note that without container entities the hierarchy that remains cannot be used further without restoring new ones + * @param callback A user provided callback that can be used to capture ownership and manipulate the detached entities. + */ + void DetachAllEntitiesInHierarchy(const AZStd::function)>& callback); + void RemoveNestedEntities(const AZStd::function&)>& filter); void Reset(); diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Spawnable/SpawnableUtils.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Spawnable/SpawnableUtils.cpp index 3c91f99b05..241d2a3bd5 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Spawnable/SpawnableUtils.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Spawnable/SpawnableUtils.cpp @@ -38,11 +38,7 @@ namespace AzToolsFramework::Prefab::SpawnableUtils // going to be used to create clones of the entities. { AzFramework::Spawnable::EntityList& entities = spawnable.GetEntities(); - if (instance.HasContainerEntity()) - { - entities.emplace_back(AZStd::move(instance.DetachContainerEntity())); - } - instance.DetachNestedEntities( + instance.DetachAllEntitiesInHierarchy( [&entities](AZStd::unique_ptr entity) { entities.emplace_back(AZStd::move(entity)); From 856daf1485e816dff21eb6a268a67fbcf41ff5d6 Mon Sep 17 00:00:00 2001 From: sconel Date: Thu, 10 Jun 2021 10:46:36 -0700 Subject: [PATCH 2/4] Addressed PR feedback --- .../AzToolsFramework/Prefab/Instance/Instance.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Instance/Instance.h b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Instance/Instance.h index bba20cbe8a..5f36ac10dc 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Instance/Instance.h +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Instance/Instance.h @@ -89,8 +89,9 @@ namespace AzToolsFramework void DetachEntities(const AZStd::function)>& callback); /** - * Detaches all entities in the instance hierarchy, including; all direct and nested entities, all container entities. - * Note that without container entities the hierarchy that remains cannot be used further without restoring new ones + * Detaches all entities in the instance hierarchy. + * Includes all direct entities, all nested entities, and all container entities. + * Note that without container entities the hierarchy that remains cannot be used further without restoring new ones. * @param callback A user provided callback that can be used to capture ownership and manipulate the detached entities. */ void DetachAllEntitiesInHierarchy(const AZStd::function)>& callback); From 9efcc3a6b59a2adb439f248515f5a3dbb66a9b63 Mon Sep 17 00:00:00 2001 From: sconel Date: Thu, 10 Jun 2021 13:15:53 -0700 Subject: [PATCH 3/4] Fix compilation error and failing unit test --- .../Tests/Prefab/SpawnableCreateTests.cpp | 14 ++++++++++---- .../SpawnableRemoveEditorInfoTestFixture.cpp | 2 +- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/Code/Framework/AzToolsFramework/Tests/Prefab/SpawnableCreateTests.cpp b/Code/Framework/AzToolsFramework/Tests/Prefab/SpawnableCreateTests.cpp index af178ec7a4..5cd4cffa35 100644 --- a/Code/Framework/AzToolsFramework/Tests/Prefab/SpawnableCreateTests.cpp +++ b/Code/Framework/AzToolsFramework/Tests/Prefab/SpawnableCreateTests.cpp @@ -15,6 +15,7 @@ #include #include +#pragma optimize("", off) namespace UnitTest { using SpawnableCreateTest = PrefabTestFixture; @@ -71,24 +72,29 @@ namespace UnitTest m_prefabSystemComponent->CreatePrefab({ entitiesCreated[0] }, {}, "test/path1")); ASSERT_TRUE(firstInstance); + ASSERT_TRUE(firstInstance->HasContainerEntity()); + expectedEntityNameSet.insert(firstInstance->GetContainerEntity()->get().GetName()); + AZStd::unique_ptr secondInstance( m_prefabSystemComponent->CreatePrefab({ entitiesCreated[1] }, MakeInstanceList(AZStd::move(firstInstance)), "test/path2")); ASSERT_TRUE(secondInstance); + ASSERT_TRUE(secondInstance->HasContainerEntity()); + expectedEntityNameSet.insert(secondInstance->GetContainerEntity()->get().GetName()); + AZStd::unique_ptr thirdInstance( m_prefabSystemComponent->CreatePrefab({ entitiesCreated[2] }, MakeInstanceList(AZStd::move(secondInstance)), "test/path3")); ASSERT_TRUE(thirdInstance); ASSERT_TRUE(thirdInstance->HasContainerEntity()); - auto& containerEntity = thirdInstance->GetContainerEntity()->get(); - expectedEntityNameSet.insert(containerEntity.GetName()); + expectedEntityNameSet.insert(thirdInstance->GetContainerEntity()->get().GetName()); //Create Spawnable auto& prefabDom = m_prefabSystemComponent->FindTemplateDom(thirdInstance->GetTemplateId()); AzFramework::Spawnable spawnable; AzToolsFramework::Prefab::SpawnableUtils::CreateSpawnable(spawnable, prefabDom); - EXPECT_EQ(spawnable.GetEntities().size() - 1, normalEntityCount); // 1 for container entity + EXPECT_EQ(spawnable.GetEntities().size(), normalEntityCount + 3); // +1 for each container entity const auto& spawnableEntities = spawnable.GetEntities(); AZStd::unordered_set actualEntityNameSet; @@ -97,6 +103,6 @@ namespace UnitTest actualEntityNameSet.insert(spawnableEntity->GetName()); } - EXPECT_EQ(expectedEntityNameSet, actualEntityNameSet); + EXPECT_EQ(actualEntityNameSet, expectedEntityNameSet); } } diff --git a/Code/Framework/AzToolsFramework/Tests/Prefab/SpawnableRemoveEditorInfoTestFixture.cpp b/Code/Framework/AzToolsFramework/Tests/Prefab/SpawnableRemoveEditorInfoTestFixture.cpp index 152d09e9b7..03ce52cfd0 100644 --- a/Code/Framework/AzToolsFramework/Tests/Prefab/SpawnableRemoveEditorInfoTestFixture.cpp +++ b/Code/Framework/AzToolsFramework/Tests/Prefab/SpawnableRemoveEditorInfoTestFixture.cpp @@ -213,7 +213,7 @@ namespace UnitTest AZStd::unique_ptr convertedInstance(aznew Instance()); ASSERT_TRUE(AzToolsFramework::Prefab::PrefabDomUtils::LoadInstanceFromPrefabDom(*convertedInstance, m_prefabDom)); - convertedInstance->DetachNestedEntities( + convertedInstance->DetachAllEntitiesInHierarchy( [this](AZStd::unique_ptr entity) { m_runtimeEntities.emplace_back(entity.release()); From 40fa0a3a46560a0c27212c0671cbd30da2c7dded Mon Sep 17 00:00:00 2001 From: sconel Date: Thu, 10 Jun 2021 13:53:25 -0700 Subject: [PATCH 4/4] Remove optimization pragma --- .../AzToolsFramework/Tests/Prefab/SpawnableCreateTests.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/Code/Framework/AzToolsFramework/Tests/Prefab/SpawnableCreateTests.cpp b/Code/Framework/AzToolsFramework/Tests/Prefab/SpawnableCreateTests.cpp index 5cd4cffa35..14bd977318 100644 --- a/Code/Framework/AzToolsFramework/Tests/Prefab/SpawnableCreateTests.cpp +++ b/Code/Framework/AzToolsFramework/Tests/Prefab/SpawnableCreateTests.cpp @@ -15,7 +15,6 @@ #include #include -#pragma optimize("", off) namespace UnitTest { using SpawnableCreateTest = PrefabTestFixture;