diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Application/ToolsApplication.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/Application/ToolsApplication.cpp index ff0c2c586b..95f13dc649 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Application/ToolsApplication.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Application/ToolsApplication.cpp @@ -45,6 +45,7 @@ #include #include #include +#include #include #include #include @@ -1589,7 +1590,13 @@ namespace AzToolsFramework // Multiple changes to the same entity are just split between different undo nodes. for (AZ::EntityId entityId : m_dirtyEntities) { - prefabPublicInterface->GenerateUndoNodesForEntityChangeAndUpdateCache(entityId, m_currentBatchUndo); + auto outcome = prefabPublicInterface->GenerateUndoNodesForEntityChangeAndUpdateCache(entityId, m_currentBatchUndo); + + if (!outcome.IsSuccess()) + { + QMessageBox::warning( + AzToolsFramework::GetActiveWindow(), QString("Error"), QString(outcome.GetError().c_str()), QMessageBox::Ok, QMessageBox::Ok); + } } } } diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.cpp index cf55a02f65..d479b72e37 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.cpp @@ -587,21 +587,21 @@ namespace AzToolsFramework return AZ::Success(entityId); } - void PrefabPublicHandler::GenerateUndoNodesForEntityChangeAndUpdateCache( + PrefabOperationResult PrefabPublicHandler::GenerateUndoNodesForEntityChangeAndUpdateCache( AZ::EntityId entityId, UndoSystem::URSequencePoint* parentUndoBatch) { // Create Undo node on entities if they belong to an instance InstanceOptionalReference owningInstance = m_instanceEntityMapperInterface->FindOwningInstance(entityId); if (!owningInstance.has_value()) { - return; + return AZ::Success(); } AZ::Entity* entity = GetEntityById(entityId); if (!entity) { m_prefabUndoCache.PurgeCache(entityId); - return; + return AZ::Success(); } PrefabDom beforeState; @@ -633,6 +633,41 @@ namespace AzToolsFramework (&beforeOwningInstance->get() != &afterOwningInstance->get())) { isNewParentOwnedByDifferentInstance = true; + + // Detect loops. Assert if an instance has been reparented in such a way to generate circular dependencies. + AZStd::vector instancesInvolved; + + if (isInstanceContainerEntity) + { + instancesInvolved.push_back(&owningInstance->get()); + } + else + { + // Retrieve all nested instances that are part of the subtree under the current entity. + EntityList entities; + RetrieveAndSortPrefabEntitiesAndInstances({ entity }, beforeOwningInstance->get(), entities, instancesInvolved); + } + + for (Instance* instance : instancesInvolved) + { + const PrefabDom& templateDom = + m_prefabSystemComponentInterface->FindTemplateDom(instance->GetTemplateId()); + AZStd::unordered_set templatePaths; + PrefabDomUtils::GetTemplateSourcePaths(templateDom, templatePaths); + + if (IsCyclicalDependencyFound(afterOwningInstance->get(), templatePaths)) + { + // Cancel the operation by restoring the previous parent + AZ::TransformBus::Event(entityId, &AZ::TransformBus::Events::SetParent, beforeParentId); + m_prefabUndoCache.UpdateCache(entityId); + + // Skip the creation of an undo node + return AZ::Failure(AZStd::string::format( + "Reparent Prefab operation aborted - Cyclical dependency detected\n(%s depends on %s).", + instance->GetTemplateSourcePath().Native().c_str(), + afterOwningInstance->get().GetTemplateSourcePath().Native().c_str())); + } + } } } @@ -673,6 +708,8 @@ namespace AzToolsFramework } m_prefabUndoCache.UpdateCache(entityId); + + return AZ::Success(); } void PrefabPublicHandler::Internal_HandleContainerOverride( diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.h b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.h index 687c11cd60..7dbb7b1e71 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.h +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.h @@ -46,7 +46,7 @@ namespace AzToolsFramework PrefabOperationResult SavePrefab(AZ::IO::Path filePath) override; PrefabEntityResult CreateEntity(AZ::EntityId parentId, const AZ::Vector3& position) override; - void GenerateUndoNodesForEntityChangeAndUpdateCache(AZ::EntityId entityId, UndoSystem::URSequencePoint* parentUndoBatch) override; + PrefabOperationResult GenerateUndoNodesForEntityChangeAndUpdateCache(AZ::EntityId entityId, UndoSystem::URSequencePoint* parentUndoBatch) override; bool IsInstanceContainerEntity(AZ::EntityId entityId) const override; bool IsLevelInstanceContainerEntity(AZ::EntityId entityId) const override; diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicInterface.h b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicInterface.h index e65268dcb2..100e403660 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicInterface.h +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicInterface.h @@ -79,8 +79,9 @@ namespace AzToolsFramework * * @param entityId The entity to patch. * @param parentUndoBatch The undo batch the undo nodes should be parented to. + * @return Returns Success if the node was generated correctly, or an error message otherwise. */ - virtual void GenerateUndoNodesForEntityChangeAndUpdateCache( + virtual PrefabOperationResult GenerateUndoNodesForEntityChangeAndUpdateCache( AZ::EntityId entityId, UndoSystem::URSequencePoint* parentUndoBatch) = 0; /**