From 8beb732cb9a1cd1d7032697e787549dbe4ccf828 Mon Sep 17 00:00:00 2001 From: Wojtek Figat Date: Tue, 26 Jul 2022 23:04:55 +0200 Subject: [PATCH] Fix duplicated actors after reparenting actor in Prefab #718 --- Source/Engine/Level/Prefabs/Prefab.Apply.cpp | 29 +++++++++++++------ Source/Engine/Level/Prefabs/PrefabManager.cpp | 15 +++++----- Source/Engine/Level/SceneObjectsFactory.cpp | 4 +-- 3 files changed, 29 insertions(+), 19 deletions(-) diff --git a/Source/Engine/Level/Prefabs/Prefab.Apply.cpp b/Source/Engine/Level/Prefabs/Prefab.Apply.cpp index 30e451105..5de65e2d9 100644 --- a/Source/Engine/Level/Prefabs/Prefab.Apply.cpp +++ b/Source/Engine/Level/Prefabs/Prefab.Apply.cpp @@ -85,7 +85,6 @@ namespace class PrefabInstanceData { public: - // Don't copy or anything PrefabInstanceData() { @@ -111,11 +110,10 @@ public: } public: - /// /// The prefab instance root actor. /// - Actor* TargetActor; + ScriptingObjectReference TargetActor; /// /// The cached order in parent of the target actor. Used to preserve it after prefab changes synchronization. @@ -133,7 +131,6 @@ public: Dictionary PrefabInstanceIdToDataIndex; public: - /// /// Collects all the valid prefab instances to update on prefab data synchronization. /// @@ -187,6 +184,8 @@ void PrefabInstanceData::CollectPrefabInstances(Array& prefa for (int32 instanceIndex = 0; instanceIndex < instances.Count(); instanceIndex++) { const auto instance = instances[instanceIndex]; + if ((instance->Flags & ObjectFlags::WasMarkedToDelete) != 0) + continue; if (instance != defaultInstance && targetActor != instance && !targetActor->HasActorInHierarchy(instance)) usedCount++; } @@ -196,14 +195,13 @@ void PrefabInstanceData::CollectPrefabInstances(Array& prefa { // Skip default instance because it will be recreated, skip input actor because it needs just to be linked Actor* instance = instances[instanceIndex]; + if ((instance->Flags & ObjectFlags::WasMarkedToDelete) != 0) + continue; if (instance != defaultInstance && targetActor != instance && !targetActor->HasActorInHierarchy(instance)) { auto& data = prefabInstancesData[dataIndex++]; data.TargetActor = instance; data.OrderInParent = instance->GetOrderInParent(); - - // Check if actor has not been deleted (the memory access exception could fire) - ASSERT(data.TargetActor->GetID() == data.TargetActor->GetID()); } } } @@ -270,6 +268,8 @@ bool PrefabInstanceData::SynchronizePrefabInstances(Array& p // If prefab object root was changed during changes apply then update the TargetActor to point a valid object Actor* oldTargetActor = instance.TargetActor; + if (!oldTargetActor || (oldTargetActor->Flags & ObjectFlags::WasMarkedToDelete) != 0) + continue; Actor* newTargetActor = FindActorWithPrefabObjectId(instance.TargetActor, defaultInstance->GetID()); if (!newTargetActor) { @@ -449,6 +449,8 @@ bool PrefabInstanceData::SynchronizePrefabInstances(Array& p } Level::callActorEvent(Level::ActorEventType::OnActorNameChanged, actor, nullptr); Level::callActorEvent(Level::ActorEventType::OnActorOrderInParentChanged, actor, nullptr); + if (!actor->IsDuringPlay() && actor->GetParent()) + Level::callActorEvent(Level::ActorEventType::OnActorParentChanged, actor, actor->GetParent()); } } @@ -1007,9 +1009,7 @@ bool Prefab::ApplyAllInternal(Actor* targetActor, bool linkTargetActorObjectToPr { auto obj = sceneObjects.Value->At(i); if (obj) - { obj->Initialize(); - } } // Update transformations @@ -1218,8 +1218,19 @@ bool Prefab::SyncChangesInternal() return true; } + // Recreate default instance but with synchronization since otherwise it might contain old data (eg. nested prefab hierarchy could be changed) + DeleteDefaultInstance(); + ObjectsRemovalService::Flush(); + { + ScopeLock lock(Locker); + _isCreatingDefaultInstance = true; + _defaultInstance = PrefabManager::SpawnPrefab(this, nullptr, &ObjectsCache, true); + _isCreatingDefaultInstance = false; + } + // Instantiate prefab instance from prefab (default spawning logic) // Note: it will get any added or removed objects from the nested prefabs + // TODO: try to optimize by using recreated default instance to ApplyAllInternal (will need special path there if apply is done with default instance to unlink it instead of destroying) const auto targetActor = PrefabManager::SpawnPrefab(this, nullptr, nullptr, true); if (targetActor == nullptr) { diff --git a/Source/Engine/Level/Prefabs/PrefabManager.cpp b/Source/Engine/Level/Prefabs/PrefabManager.cpp index d0d47926d..5b73f795d 100644 --- a/Source/Engine/Level/Prefabs/PrefabManager.cpp +++ b/Source/Engine/Level/Prefabs/PrefabManager.cpp @@ -29,7 +29,6 @@ CriticalSection PrefabManager::PrefabsReferencesLocker; class PrefabManagerService : public EngineService { public: - PrefabManagerService() : EngineService(TEXT("Prefab Manager"), 110) { @@ -180,6 +179,13 @@ Actor* PrefabManager::SpawnPrefab(Prefab* prefab, Actor* parent, Dictionary_parent = parent; if (parent) @@ -193,13 +199,6 @@ Actor* PrefabManager::SpawnPrefab(Prefab* prefab, Actor* parent, DictionaryInitialize(); } - // Synchronize prefab instances (prefab may have new objects added or some removed so deserialized instances need to synchronize with it) - if (withSynchronization) - { - // TODO: resave and force sync scenes during game cooking so this step could be skipped in game - SceneObjectsFactory::SynchronizePrefabInstances(context, prefabSyncData); - } - // Delete objects without parent or with invalid linkage to the prefab for (int32 i = 1; i < sceneObjects->Count(); i++) { diff --git a/Source/Engine/Level/SceneObjectsFactory.cpp b/Source/Engine/Level/SceneObjectsFactory.cpp index 87c3723e2..a8ee1bee5 100644 --- a/Source/Engine/Level/SceneObjectsFactory.cpp +++ b/Source/Engine/Level/SceneObjectsFactory.cpp @@ -418,8 +418,8 @@ void SceneObjectsFactory::SynchronizeNewPrefabInstances(Context& context, Prefab const Guid jParentId = JsonTools::GetGuid(jData, "ParentID"); //if (jParentId == actorParentId) // break; - if (jParentId != actorId) - continue; + //if (jParentId != actorId) + // continue; const Guid jPrefabObjectId = JsonTools::GetGuid(jData, "PrefabObjectID"); if (jPrefabObjectId != prefabObjectId) continue;