From 1ef8bb723c83bad36079ab0d51c9a1769665e2e0 Mon Sep 17 00:00:00 2001 From: Wojtek Figat Date: Wed, 27 Jul 2022 09:33:01 +0200 Subject: [PATCH] Fix reparenting inside the nested prefabs when using multiple instanced of the nested prefab in PrefabManager #690 --- Source/Engine/Level/Prefabs/PrefabManager.cpp | 1 + Source/Engine/Level/SceneObjectsFactory.cpp | 27 +++- Source/Engine/Level/SceneObjectsFactory.h | 2 + Source/Engine/Tests/TestPrefabs.cpp | 120 ++++++++++++++++++ 4 files changed, 148 insertions(+), 2 deletions(-) diff --git a/Source/Engine/Level/Prefabs/PrefabManager.cpp b/Source/Engine/Level/Prefabs/PrefabManager.cpp index 7bee237cd..79cb7bd91 100644 --- a/Source/Engine/Level/Prefabs/PrefabManager.cpp +++ b/Source/Engine/Level/Prefabs/PrefabManager.cpp @@ -152,6 +152,7 @@ Actor* PrefabManager::SpawnPrefab(Prefab* prefab, Actor* parent, DictionaryIdsMapping); } diff --git a/Source/Engine/Level/SceneObjectsFactory.cpp b/Source/Engine/Level/SceneObjectsFactory.cpp index a8ee1bee5..210c2765d 100644 --- a/Source/Engine/Level/SceneObjectsFactory.cpp +++ b/Source/Engine/Level/SceneObjectsFactory.cpp @@ -18,6 +18,19 @@ SceneObjectsFactory::Context::Context(ISerializeModifier* modifier) { } +void SceneObjectsFactory::Context::SetupIdsMapping(const SceneObject* obj) +{ + int32 instanceIndex; + if (ObjectToInstance.TryGet(obj->GetID(), instanceIndex) && instanceIndex != CurrentInstance) + { + // Apply the current prefab instance objects ids table to resolve references inside a prefab properly + CurrentInstance = instanceIndex; + auto& instance = Instances[instanceIndex]; + for (auto& e : instance.IdsMapping) + Modifier->IdsMapping[e.Key] = e.Value; + } +} + SceneObject* SceneObjectsFactory::Spawn(Context& context, ISerializable::DeserializeStream& stream) { // Get object id @@ -302,7 +315,7 @@ void SceneObjectsFactory::SetupPrefabInstances(Context& context, PrefabSyncData& { PROFILE_CPU_NAMED("SetupPrefabInstances"); const int32 count = data.Data.Size(); - ASSERT(count <= data.SceneObjects.Count()) + ASSERT(count <= data.SceneObjects.Count()); for (int32 i = 0; i < count; i++) { SceneObject* obj = data.SceneObjects[i]; @@ -314,7 +327,16 @@ void SceneObjectsFactory::SetupPrefabInstances(Context& context, PrefabSyncData& continue; if (!JsonTools::GetGuidIfValid(prefabId, stream, "PrefabID")) continue; - const Guid parentId = JsonTools::GetGuid(stream, "ParentID"); + Guid parentId = JsonTools::GetGuid(stream, "ParentID"); + for (int32 j = 0; j < i; j++) + { + // Find instance ID of the parent to this object (use data in json for relationship) + if (parentId == JsonTools::GetGuid(data.Data[j], "ID") && data.SceneObjects[j]) + { + parentId = data.SceneObjects[j]->GetID(); + break; + } + } const Guid id = obj->GetID(); auto prefab = Content::LoadAsync(prefabId); @@ -483,6 +505,7 @@ void SceneObjectsFactory::SynchronizePrefabInstances(Context& context, PrefabSyn LOG(Info, "Object {0} has invalid parent object {4} -> {5} (PrefabObjectID: {1}, PrefabID: {2}, Path: {3})", obj->GetSceneObjectId(), prefabObjectId, prefab->GetID(), prefab->GetPath(), parentPrefabObjectId, actualParentPrefabId); // Map actual prefab object id to the current scene objects collection + context.SetupIdsMapping(obj); data.Modifier->IdsMapping.TryGet(actualParentPrefabId, actualParentPrefabId); // Find parent diff --git a/Source/Engine/Level/SceneObjectsFactory.h b/Source/Engine/Level/SceneObjectsFactory.h index 9ba75fc75..8215f99d0 100644 --- a/Source/Engine/Level/SceneObjectsFactory.h +++ b/Source/Engine/Level/SceneObjectsFactory.h @@ -26,6 +26,8 @@ public: Dictionary ObjectToInstance; Context(ISerializeModifier* modifier); + + void SetupIdsMapping(const SceneObject* obj); }; /// diff --git a/Source/Engine/Tests/TestPrefabs.cpp b/Source/Engine/Tests/TestPrefabs.cpp index 4073f1fbe..72e063132 100644 --- a/Source/Engine/Tests/TestPrefabs.cpp +++ b/Source/Engine/Tests/TestPrefabs.cpp @@ -119,6 +119,126 @@ TEST_CASE("Prefabs") REQUIRE(instanceA->Children[0]->Children[0]->Children[0]->GetName() == TEXT("Prefab B.Child")); REQUIRE(instanceA->Children[0]->Children[0]->Children[0]->GetChildrenCount() == 0); + // Cleanup + instanceA->DeleteObject(); + instanceB->DeleteObject(); + Content::DeleteAsset(prefabA); + Content::DeleteAsset(prefabB); + } + SECTION("Test Adding Object in Nested Prefab") + { + // https://github.com/FlaxEngine/FlaxEngine/issues/690 + + // Create Prefab B with just root object + AssetReference prefabB = Content::CreateVirtualAsset(); + REQUIRE(prefabB); + Guid id; + Guid::Parse("25dbe4b0416be0777a6ce59e8788b10f", id); + prefabB->ChangeID(id); + auto prefabBInit = prefabB->Init(Prefab::TypeName, + "[" + "{" + "\"ID\": \"aac6b9644492fbca1a6ab0a7904a557e\"," + "\"TypeName\": \"FlaxEngine.EmptyActor\"," + "\"Name\": \"Prefab B.Root\"" + "}" + "]"); + REQUIRE(!prefabBInit); + + // Create Prefab A with two nested Prefab B attached to the root + AssetReference prefabA = Content::CreateVirtualAsset(); + REQUIRE(prefabA); + Guid::Parse("4cb744714f746e31855f41815612d14b", id); + prefabA->ChangeID(id); + auto prefabAInit = prefabA->Init(Prefab::TypeName, + "[" + "{" + "\"ID\": \"244274a04cc60d56a2f024bfeef5772d\"," + "\"TypeName\": \"FlaxEngine.EmptyActor\"," + "\"Name\": \"Prefab A.Root\"" + "}," + "{" + "\"ID\": \"1e51f1094f430733333f8280e78dfcc3\"," + "\"PrefabID\": \"25dbe4b0416be0777a6ce59e8788b10f\"," + "\"PrefabObjectID\": \"aac6b9644492fbca1a6ab0a7904a557e\"," + "\"ParentID\": \"244274a04cc60d56a2f024bfeef5772d\"" + "}," + "{" + "\"ID\": \"2e1f2bae4aaedeab8725908ce1aec325\"," + "\"PrefabID\": \"25dbe4b0416be0777a6ce59e8788b10f\"," + "\"PrefabObjectID\": \"aac6b9644492fbca1a6ab0a7904a557e\"," + "\"ParentID\": \"244274a04cc60d56a2f024bfeef5772d\"" + "}" + "]"); + REQUIRE(!prefabAInit); + + // Spawn test instances of both prefabs + ScriptingObjectReference instanceB = PrefabManager::SpawnPrefab(prefabB); + ScriptingObjectReference instanceA = PrefabManager::SpawnPrefab(prefabA); + + // Verify initial scenario + REQUIRE(instanceA); + REQUIRE(instanceB); + REQUIRE(instanceA->GetName() == TEXT("Prefab A.Root")); + REQUIRE(instanceA->GetChildrenCount() == 2); + REQUIRE(instanceA->Children[0]->GetName() == TEXT("Prefab B.Root")); + REQUIRE(instanceA->Children[0]->GetChildrenCount() == 0); + REQUIRE(instanceA->Children[1]->GetName() == TEXT("Prefab B.Root")); + REQUIRE(instanceA->Children[1]->GetChildrenCount() == 0); + + // Modify Prefab B instance to add a new actor as a child so at appears in both nested instances in Prefab A instance + Guid newChildId; + Guid::Parse(TEXT("123456a04cc60d56a2f024bfeef57723"), newChildId); + auto newChild = EmptyActor::Spawn(ScriptingObject::SpawnParams(newChildId, EmptyActor::TypeInitializer)); + newChild->SetName(TEXT("Prefab B.Child")); + newChild->SetParent(instanceB); + + // Apply nested prefab changes + auto applyResult = PrefabManager::ApplyAll(instanceB); + REQUIRE(!applyResult); + + // Verify if instance of Prefab B nested instances in instance of Prefab A was properly updated + REQUIRE(instanceA); + REQUIRE(instanceB); + REQUIRE(instanceA->GetName() == TEXT("Prefab A.Root")); + REQUIRE(instanceA->GetChildrenCount() == 2); + REQUIRE(instanceA->Children[0]->GetName() == TEXT("Prefab B.Root")); + REQUIRE(instanceA->Children[0]->GetChildrenCount() == 1); + REQUIRE(instanceA->Children[0]->Children[0]->GetName() == TEXT("Prefab B.Child")); + REQUIRE(instanceA->Children[0]->Children[0]->GetChildrenCount() == 0); + REQUIRE(instanceA->Children[1]->GetName() == TEXT("Prefab B.Root")); + REQUIRE(instanceA->Children[1]->GetChildrenCount() == 1); + REQUIRE(instanceA->Children[1]->Children[0]->GetName() == TEXT("Prefab B.Child")); + REQUIRE(instanceA->Children[1]->Children[0]->GetChildrenCount() == 0); + + // Add another child + Guid::Parse(TEXT("678906a04cc60d56a2f024bfeef57723"), newChildId); + newChild = EmptyActor::Spawn(ScriptingObject::SpawnParams(newChildId, EmptyActor::TypeInitializer)); + newChild->SetName(TEXT("Prefab B.Child 2")); + newChild->SetParent(instanceB); + + // Apply nested prefab changes + applyResult = PrefabManager::ApplyAll(instanceB); + REQUIRE(!applyResult); + + // Reparent another child into the first child + newChild->SetParent(instanceB->Children[0]); + + // Apply nested prefab changes + applyResult = PrefabManager::ApplyAll(instanceB); + REQUIRE(!applyResult); + + // Verify if instance of Prefab B nested instances in instance of Prefab A was properly updated + REQUIRE(instanceA); + REQUIRE(instanceB); + REQUIRE(instanceA->GetChildrenCount() == 2); + REQUIRE(instanceA->Children[0]->GetChildrenCount() == 1); + REQUIRE(instanceA->Children[0]->Children[0]->GetChildrenCount() == 1); + REQUIRE(instanceA->Children[0]->Children[0]->Children[0]->GetChildrenCount() == 0); + REQUIRE(instanceA->Children[1]->GetChildrenCount() == 1); + REQUIRE(instanceA->Children[1]->Children[0]->GetChildrenCount() == 1); + REQUIRE(instanceA->Children[1]->Children[0]->Children[0]->GetChildrenCount() == 0); + // Cleanup instanceA->DeleteObject(); instanceB->DeleteObject();