diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabDomUtils.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabDomUtils.cpp index 0bffd26be0..145a293ab8 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabDomUtils.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabDomUtils.cpp @@ -199,6 +199,43 @@ namespace AzToolsFramework return true; } + void GetTemplateSourcePaths(const PrefabDomValue& prefabDom, AZStd::unordered_set& templateSourcePaths) + { + PrefabDomValueConstReference findSourceResult = PrefabDomUtils::FindPrefabDomValue(prefabDom, PrefabDomUtils::SourceName); + if (!findSourceResult.has_value() || !(findSourceResult->get().IsString()) || + findSourceResult->get().GetStringLength() == 0) + { + AZ_Assert( + false, + "PrefabDomUtils::GetDependentTemplatePath - Source value of prefab in the provided DOM is not a valid string."); + return; + } + + templateSourcePaths.emplace(findSourceResult->get().GetString()); + PrefabDomValueConstReference instancesReference = GetInstancesValue(prefabDom); + if (instancesReference.has_value()) + { + const PrefabDomValue& instances = instancesReference->get(); + + for (PrefabDomValue::ConstMemberIterator instanceIterator = instances.MemberBegin(); + instanceIterator != instances.MemberEnd(); ++instanceIterator) + { + GetTemplateSourcePaths(instanceIterator->value, templateSourcePaths); + } + } + } + + PrefabDomValueConstReference GetInstancesValue(const PrefabDomValue& prefabDom) + { + PrefabDomValueConstReference findInstancesResult = FindPrefabDomValue(prefabDom, PrefabDomUtils::InstancesName); + if (!findInstancesResult.has_value() || !(findInstancesResult->get().IsObject())) + { + return AZStd::nullopt; + } + + return findInstancesResult->get(); + } + void PrintPrefabDomValue( [[maybe_unused]] const AZStd::string_view printMessage, [[maybe_unused]] const PrefabDomValue& prefabDomValue) diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabDomUtils.h b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabDomUtils.h index c7c2827770..5ee91c85ae 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabDomUtils.h +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabDomUtils.h @@ -100,6 +100,20 @@ namespace AzToolsFramework .Append(instanceName); }; + /** + * Gets a set of all the template source paths in the given dom. + * @param prefabDom The DOM to get the template source paths from. + * @param[out] templateSourcePaths The set of template source paths to populate. + */ + void GetTemplateSourcePaths(const PrefabDomValue& prefabDom, AZStd::unordered_set& templateSourcePaths); + + /** + * Gets the instances DOM value from the given prefab DOM. + * + * @return the instances DOM value or AZStd::nullopt if it instances can't be found. + */ + PrefabDomValueConstReference GetInstancesValue(const PrefabDomValue& prefabDom); + /** * Prints the contents of the given prefab DOM value to the debug output console in a readable format. * @param printMessage The message that will be printed before printing the PrefabDomValue diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.cpp index 5ecff637a1..200a5c96fb 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.cpp @@ -210,25 +210,30 @@ namespace AzToolsFramework auto relativePath = m_prefabLoaderInterface->GetRelativePathToProject(filePath); Prefab::TemplateId templateId = m_prefabSystemComponentInterface->GetTemplateIdFromFilePath(relativePath); - // If the template isn't currently loaded, there's no way for it to be in the hierarchy so we just skip the check. - if (templateId != Prefab::InvalidTemplateId && IsPrefabInInstanceAncestorHierarchy(templateId, instanceToParentUnder->get())) + if (templateId == InvalidTemplateId) { - return AZ::Failure( - AZStd::string::format( - "Instantiate Prefab operation aborted - Cyclical dependency detected\n(%s depends on %s).", - relativePath.Native().c_str(), - instanceToParentUnder->get().GetTemplateSourcePath().Native().c_str() - ) - ); + // Load the template from the file + templateId = m_prefabLoaderInterface->LoadTemplateFromFile(filePath); + AZ_Assert(templateId != InvalidTemplateId, "Template with source path %s couldn't be loaded correctly.", filePath); } - + + const PrefabDom& templateDom = m_prefabSystemComponentInterface->FindTemplateDom(templateId); + AZStd::unordered_set templatePaths; + PrefabDomUtils::GetTemplateSourcePaths(templateDom, templatePaths); + + if (IsCyclicalDependencyFound(instanceToParentUnder->get(), templatePaths)) + { + return AZ::Failure(AZStd::string::format( + "Instantiate Prefab operation aborted - Cyclical dependency detected\n(%s depends on %s).", + relativePath.Native().c_str(), instanceToParentUnder->get().GetTemplateSourcePath().Native().c_str())); + } + { // Initialize Undo Batch object ScopedUndoBatch undoBatch("Instantiate Prefab"); PrefabDom instanceToParentUnderDomBeforeCreate; - m_instanceToTemplateInterface->GenerateDomForInstance( - instanceToParentUnderDomBeforeCreate, instanceToParentUnder->get()); + m_instanceToTemplateInterface->GenerateDomForInstance(instanceToParentUnderDomBeforeCreate, instanceToParentUnder->get()); // Instantiate the Prefab auto instanceToCreate = prefabEditorEntityOwnershipInterface->InstantiatePrefab(relativePath, instanceToParentUnder); @@ -242,8 +247,7 @@ namespace AzToolsFramework PrefabUndoHelpers::UpdatePrefabInstance( instanceToParentUnder->get(), "Update prefab instance", instanceToParentUnderDomBeforeCreate, undoBatch.GetUndoBatch()); - CreateLink({}, instanceToCreate->get(), instanceToParentUnder->get().GetTemplateId(), - undoBatch.GetUndoBatch(), parent); + CreateLink({}, instanceToCreate->get(), instanceToParentUnder->get().GetTemplateId(), undoBatch.GetUndoBatch(), parent); AZ::EntityId containerEntityId = instanceToCreate->get().GetContainerEntityId(); // Apply position @@ -296,17 +300,17 @@ namespace AzToolsFramework return AZ::Success(); } - bool PrefabPublicHandler::IsPrefabInInstanceAncestorHierarchy(TemplateId prefabTemplateId, InstanceOptionalConstReference instance) + bool PrefabPublicHandler::IsCyclicalDependencyFound( + InstanceOptionalConstReference instance, const AZStd::unordered_set& templateSourcePaths) { InstanceOptionalConstReference currentInstance = instance; while (currentInstance.has_value()) { - if (currentInstance->get().GetTemplateId() == prefabTemplateId) + if (templateSourcePaths.contains(currentInstance->get().GetTemplateSourcePath())) { return true; } - currentInstance = currentInstance->get().GetParentInstance(); } @@ -1003,5 +1007,5 @@ namespace AzToolsFramework return true; } - } -} + } // namespace Prefab +} // namespace AzToolsFramework diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.h b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.h index 138bc84aa0..d88086dda9 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.h +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.h @@ -12,8 +12,8 @@ #pragma once -#include #include +#include #include #include @@ -107,13 +107,15 @@ namespace AzToolsFramework const AZStd::vector& entityIds, EntityList& inputEntityList, EntityList& topLevelEntities, AZ::EntityId& commonRootEntityId, InstanceOptionalReference& commonRootEntityOwningInstance); - /* Detects whether an instance of prefabTemplateId is present in the hierarchy of ancestors of instance. + /* Checks whether the template source path of any of the ancestors in the instance hierarchy matches with one of the + * paths provided in a set. * - * \param prefabTemplateId The template id to test for - * \param instance The instance whose ancestor hierarchy prefabTemplateId will be tested against. - * \return true if an instance of the template of id prefabTemplateId could be found in the ancestor hierarchy of instance, false otherwise. + * \param instance The instance whose ancestor hierarchy the provided set of template source paths will be tested against. + * \param templateSourcePaths The template source paths provided to be checked against the instance ancestor hierarchy. + * \return true if any of the template source paths could be found in the ancestor hierarchy of instance, false otherwise. */ - bool IsPrefabInInstanceAncestorHierarchy(TemplateId prefabTemplateId, InstanceOptionalConstReference instance); + bool IsCyclicalDependencyFound( + InstanceOptionalConstReference instance, const AZStd::unordered_set& templateSourcePaths); static Instance* GetParentInstance(Instance* instance); static Instance* GetAncestorOfInstanceThatIsChildOfRoot(const Instance* ancestor, Instance* descendant); @@ -129,5 +131,5 @@ namespace AzToolsFramework uint64_t m_newEntityCounter = 1; }; - } -} + } // namespace Prefab +} // namespace AzToolsFramework