Переглянути джерело

Fixes a crash and improves detach Prefab (#17664)

* Fixes a crash and improves detach Prefab

This change allows Prefab containers to be set as "Editor Only" which will
cause their children to be exported into the runtime and CTRL+G mode but
not the container itself.

This bug was found with experimenting with prefab containers and detach.

I also noticed that detach was not symmetrical with creating a prefab,
that is, creating a prefeb creates the container and moves the children
into the container, but the default behavior of detach does not
do the opposite.

I reworked detach to do the opposite, but kept the underlying APIs
and functions the same as before for backward compatibility, so all
existing uses of prefab detach will still be have the same.

the only difference is the user facing menu "detach" now has
"detach" (new behavior) and "Detach and keep container entity" (prior
behavior)

Adds a new setting and reworks the way the settings are added
note that the new SettingsRegistryUtils.h contents were moved Framework
the viewportSettings.h file instead, since it is more general.

Signed-off-by: Nicholas Lawson <[email protected]>
Nicholas Lawson 1 рік тому
батько
коміт
b5e81ee621
19 змінених файлів з 966 додано та 143 видалено
  1. 18 4
      Code/Editor/EditorPreferencesPageGeneral.cpp
  2. 3 1
      Code/Editor/EditorPreferencesPageGeneral.h
  3. 1 0
      Code/Editor/EditorViewportSettings.cpp
  4. 55 0
      Code/Framework/AzToolsFramework/AzToolsFramework/API/SettingsRegistryUtils.h
  5. 27 4
      Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Instance/Instance.cpp
  6. 2 1
      Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Instance/Instance.h
  7. 213 97
      Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.cpp
  8. 3 0
      Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.h
  9. 13 0
      Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicInterface.h
  10. 11 0
      Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicRequestBus.h
  11. 6 0
      Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicRequestHandler.cpp
  12. 1 0
      Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicRequestHandler.h
  13. 22 0
      Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabSettings.h
  14. 11 2
      Code/Framework/AzToolsFramework/AzToolsFramework/UI/Prefab/PrefabIntegrationManager.cpp
  15. 1 0
      Code/Framework/AzToolsFramework/AzToolsFramework/Viewport/ViewportSettings.cpp
  16. 0 34
      Code/Framework/AzToolsFramework/AzToolsFramework/Viewport/ViewportSettings.h
  17. 2 0
      Code/Framework/AzToolsFramework/AzToolsFramework/aztoolsframework_files.cmake
  18. 576 0
      Code/Framework/AzToolsFramework/Tests/Prefab/PrefabDetachPrefabAndRemoveContainerTests.cpp
  19. 1 0
      Code/Framework/AzToolsFramework/Tests/aztoolsframeworktests_files.cmake

+ 18 - 4
Code/Editor/EditorPreferencesPageGeneral.cpp

@@ -13,7 +13,9 @@
 #include <QMessageBox>
 
 // AzToolsFramework
+#include <AzToolsFramework/API/SettingsRegistryUtils.h>
 #include <AzToolsFramework/UI/UICore/WidgetHelpers.h>
+#include <AzToolsFramework/Prefab/PrefabSettings.h>
 #include <AzQtComponents/Components/StyleManager.h>
 
 // Editor
@@ -25,6 +27,7 @@
 #define UNDOSLICESAVE_VALON "UndoSliceSaveValueOn"
 #define UNDOSLICESAVE_VALOFF "UndoSliceSaveValueOff"
 
+
 void CEditorPreferencesPage_General::Reflect(AZ::SerializeContext& serialize)
 {
     serialize.Class<GeneralSettings>()
@@ -41,9 +44,13 @@ void CEditorPreferencesPage_General::Reflect(AZ::SerializeContext& serialize)
         ->Field("EnableSceneInspector", &GeneralSettings::m_enableSceneInspector)
         ->Field("RestoreViewportCamera", &GeneralSettings::m_restoreViewportCamera);
 
+    // note, despite this class name being LevelSaveSettings, it is used for general prefab settings
+    // and the name is retained to avoid breaking things
     serialize.Class<LevelSaveSettings>()
         ->Version(1)
-        ->Field("SaveAllPrefabsPreference", &LevelSaveSettings::m_saveAllPrefabsPreference);
+        ->Field("SaveAllPrefabsPreference", &LevelSaveSettings::m_saveAllPrefabsPreference)
+        ->Field("DetachPrefabRemovesContainer", &LevelSaveSettings::m_bDetachPrefabRemovesContainer);
+
 
     serialize.Class<Messaging>()
         ->Version(2)
@@ -86,7 +93,9 @@ void CEditorPreferencesPage_General::Reflect(AZ::SerializeContext& serialize)
                     "This option controls whether nested prefabs should be saved when a prefab is saved.")
                 ->EnumAttribute(AzToolsFramework::Prefab::SaveAllPrefabsPreference::AskEveryTime, "Ask every time")
                 ->EnumAttribute(AzToolsFramework::Prefab::SaveAllPrefabsPreference::SaveAll, "Save all")
-                ->EnumAttribute(AzToolsFramework::Prefab::SaveAllPrefabsPreference::SaveNone, "Save none");
+                ->EnumAttribute(AzToolsFramework::Prefab::SaveAllPrefabsPreference::SaveNone, "Save none")
+            ->DataElement(AZ::Edit::UIHandlers::CheckBox, &LevelSaveSettings::m_bDetachPrefabRemovesContainer, "Detach removes container entity", 
+                    "When you choose the 'detach' option on a prefab container, should the container entity be removed also?");
 
         editContext->Class<Messaging>("Messaging", "")
             ->DataElement(AZ::Edit::UIHandlers::CheckBox, &Messaging::m_showDashboard, "Show Welcome to Open 3D Engine at startup", "Show Welcome to Open 3D Engine at startup");
@@ -100,7 +109,7 @@ void CEditorPreferencesPage_General::Reflect(AZ::SerializeContext& serialize)
             ->ClassElement(AZ::Edit::ClassElements::EditorData, "")
             ->Attribute(AZ::Edit::Attributes::Visibility, AZ_CRC("PropertyVisibility_ShowChildrenOnly", 0xef428f20))
             ->DataElement(AZ::Edit::UIHandlers::Default, &CEditorPreferencesPage_General::m_generalSettings, "General Settings", "General Editor Preferences")
-            ->DataElement(AZ::Edit::UIHandlers::Default, &CEditorPreferencesPage_General::m_levelSaveSettings, "Prefab Save Settings", "File>Save")
+            ->DataElement(AZ::Edit::UIHandlers::Default, &CEditorPreferencesPage_General::m_levelSaveSettings, "Prefab Settings", "General Prefab Settings")
             ->DataElement(AZ::Edit::UIHandlers::Default, &CEditorPreferencesPage_General::m_messaging, "Messaging", "Messaging")
             ->DataElement(AZ::Edit::UIHandlers::Default, &CEditorPreferencesPage_General::m_undo, "Undo", "Undo Preferences");
     }
@@ -125,6 +134,8 @@ QIcon& CEditorPreferencesPage_General::GetIcon()
 
 void CEditorPreferencesPage_General::OnApply()
 {
+    using namespace AzToolsFramework::Prefab::Settings;
+
     //general settings
     gSettings.bPreviewGeometryWindow = m_generalSettings.m_previewPanel;
     gSettings.enableSourceControl = m_generalSettings.m_enableSourceControl;
@@ -136,6 +147,7 @@ void CEditorPreferencesPage_General::OnApply()
     gSettings.stylusMode = m_generalSettings.m_stylusMode;
     gSettings.restoreViewportCamera = m_generalSettings.m_restoreViewportCamera;
     gSettings.enableSceneInspector = m_generalSettings.m_enableSceneInspector;
+    AzToolsFramework::SetRegistry(AzToolsFramework::Prefab::Settings::DetachPrefabRemovesContainerName, m_levelSaveSettings.m_bDetachPrefabRemovesContainer);
 
     if (static_cast<int>(m_generalSettings.m_toolbarIconSize) != gSettings.gui.nToolbarIconSize)
     {
@@ -152,6 +164,8 @@ void CEditorPreferencesPage_General::OnApply()
 
 void CEditorPreferencesPage_General::InitializeSettings()
 {
+    using namespace AzToolsFramework::Prefab::Settings;
+
     //general settings
     m_generalSettings.m_previewPanel = gSettings.bPreviewGeometryWindow;
     m_generalSettings.m_enableSourceControl = gSettings.enableSourceControl;
@@ -162,11 +176,11 @@ void CEditorPreferencesPage_General::InitializeSettings()
     m_generalSettings.m_stylusMode = gSettings.stylusMode;
     m_generalSettings.m_restoreViewportCamera = gSettings.restoreViewportCamera;
     m_generalSettings.m_enableSceneInspector = gSettings.enableSceneInspector;
-
     m_generalSettings.m_toolbarIconSize = static_cast<AzQtComponents::ToolBar::ToolBarIconSize>(gSettings.gui.nToolbarIconSize);
 
     //prefabs
     m_levelSaveSettings.m_saveAllPrefabsPreference = gSettings.levelSaveSettings.saveAllPrefabsPreference;
+    m_levelSaveSettings.m_bDetachPrefabRemovesContainer = AzToolsFramework::GetRegistry(DetachPrefabRemovesContainerName, DetachPrefabRemovesContainerDefault);
 
     //Messaging
     m_messaging.m_showDashboard = gSettings.bShowDashboardAtStartup;

+ 3 - 1
Code/Editor/EditorPreferencesPageGeneral.h

@@ -58,13 +58,15 @@ private:
         bool m_stylusMode;
         bool m_restoreViewportCamera;
         bool m_bShowNews;
+        
         bool m_enableSceneInspector;
     };
 
-    struct LevelSaveSettings
+    struct LevelSaveSettings // do not change the name or the UUID of this struct for backward settings compat.
     {
         AZ_TYPE_INFO(LevelSaveSettings, "{E297DAE3-3985-4BC2-8B43-45F3B1522F6B}");
         AzToolsFramework::Prefab::SaveAllPrefabsPreference m_saveAllPrefabsPreference;
+        bool m_bDetachPrefabRemovesContainer;
     };
 
     struct Messaging

+ 1 - 0
Code/Editor/EditorViewportSettings.cpp

@@ -16,6 +16,7 @@
 #include <AzCore/Settings/SettingsRegistryMergeUtils.h>
 #include <AzCore/std/string/string_view.h>
 #include <AzToolsFramework/Viewport/ViewportSettings.h>
+#include <AzToolsFramework/API/SettingsRegistryUtils.h>
 
 namespace SandboxEditor
 {

+ 55 - 0
Code/Framework/AzToolsFramework/AzToolsFramework/API/SettingsRegistryUtils.h

@@ -0,0 +1,55 @@
+/*
+ * Copyright (c) Contributors to the Open 3D Engine Project.
+ * For complete copyright and license terms please see the LICENSE at the root of this distribution.
+ *
+ * SPDX-License-Identifier: Apache-2.0 OR MIT
+ *
+ */
+
+#pragma once
+
+#include <AzCore/Settings/SettingsRegistry.h>
+
+//! @file - contains a few utility functions for dealing with the settings registry.
+
+namespace AzToolsFramework
+{
+    //! Set a value in the Settings Registry.
+    //! setting needs to be a fully formed path, like "O3DE/Editor/General/Something"
+    template<typename T>
+    void SetRegistry(const AZStd::string_view setting, T&& value)
+    {
+        if (auto* registry = AZ::SettingsRegistry::Get())
+        {
+            registry->Set(setting, AZStd::forward<T>(value));
+        }
+    }
+
+    //! Get a value from the Settings Registry.
+    //! setting needs to be a fully formed path, like "O3DE/Editor/General/Something"
+    //! defaultValue is returned if the setting is not found.
+    template<typename T>
+    AZStd::remove_cvref_t<T> GetRegistry(const AZStd::string_view setting, T&& defaultValue)
+    {
+        AZStd::remove_cvref_t<T> value = AZStd::forward<T>(defaultValue);
+        if (const auto* registry = AZ::SettingsRegistry::Get())
+        {
+            AZStd::remove_cvref_t<T> potentialValue;
+            if (registry->Get(potentialValue, setting))
+            {
+                value = AZStd::move(potentialValue);
+            }
+        }
+        return value;
+    }
+
+    //! Clear a value from the Settings Registry.
+    //! setting needs to be a fully formed path, like "O3DE/Editor/General/Something"
+    inline void ClearRegistry(const AZStd::string_view setting)
+    {
+        if (auto* registry = AZ::SettingsRegistry::Get())
+        {
+            registry->Remove(setting);
+        }
+    }
+}

+ 27 - 4
Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Instance/Instance.cpp

@@ -104,7 +104,9 @@ namespace AzToolsFramework
 
         Instance::~Instance()
         {
-            Reset();
+            // when destroying an instance in its destructor, do not re-create any 
+            // new entities or instances (Avoids a leak)
+            Reset(false);
         }
 
         void Instance::Reflect(AZ::ReflectContext* context)
@@ -313,7 +315,7 @@ namespace AzToolsFramework
             }
         }
 
-        void Instance::Reset()
+        void Instance::Reset(bool forReuse)
         {
             m_templateInstanceMapper->UnregisterInstance(*this);
 
@@ -321,14 +323,35 @@ namespace AzToolsFramework
 
             m_nestedInstances.clear();
             m_cachedInstanceDom = PrefabDom();
-            m_containerEntity.reset(aznew AZ::Entity());
-            RegisterEntity(m_containerEntity->GetId(), GenerateEntityAlias());
 
+            // forReuse will be true unless we're in destructor
+            // If we're in a destructor, there is no point in creating a new entity for the container
+            // or registering a new one in the map, otherwise we would leak elements in the map.
+            if (forReuse)
+            {
+                m_containerEntity.reset(aznew AZ::Entity());
+                RegisterEntity(m_containerEntity->GetId(), GenerateEntityAlias());
+            }
+            else
+            {
+                m_containerEntity.reset();
+            }
         }
 
         void Instance::RemoveEntities(
             const AZStd::function<bool(const AZStd::unique_ptr<AZ::Entity>&)>& filter)
         {
+            // It is possible for the container entity to be the one being asked to be removed
+            // This can happen if the prefab is being exported to a spawnable, and the container
+            // entity is set to "editor only" or even "not exported".  This is what allows
+            // prefab container entities to be set to Editor Only without causing a crash in the Editor.
+            if ((m_containerEntity)&&(m_entityIdInstanceRelationship == EntityIdInstanceRelationship::OneToOne))
+            {
+                if (filter(m_containerEntity)) // asked to filter out the container
+                {
+                    DestroyContainerEntity();
+                }
+            }
             AZStd::erase_if(m_entities,
                 [this, &filter](const auto& item)
                 {

+ 2 - 1
Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Instance/Instance.h

@@ -181,7 +181,8 @@ namespace AzToolsFramework
 
             //! Resets the instance to an initial state.
             //! It unregisters the instance, entities and nested instances.
-            void Reset();
+            //! @param forReuse If true, the instance can be reused after reset, otherwise it is partially destroyed
+            void Reset(bool forReuse=true);
 
             //! Gets the aliases for the entities in the Instance DOM.
             //! @return The list of EntityAliases.

+ 213 - 97
Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.cpp

@@ -1556,6 +1556,16 @@ namespace AzToolsFramework
         }
 
         PrefabOperationResult PrefabPublicHandler::DetachPrefab(const AZ::EntityId& containerEntityId)
+        {
+            return DetachPrefabImpl(containerEntityId, true /* Keep Container Entity */);
+        }
+
+        PrefabOperationResult PrefabPublicHandler::DetachPrefabAndRemoveContainerEntity(const AZ::EntityId& containerEntityId)
+        {
+            return DetachPrefabImpl(containerEntityId, false /* Discard Container Entity */);
+        }
+        
+        PrefabOperationResult PrefabPublicHandler::DetachPrefabImpl(const AZ::EntityId& containerEntityId, bool keepContainerEntity)
         {
             if (!containerEntityId.IsValid())
             {
@@ -1576,115 +1586,221 @@ namespace AzToolsFramework
                 return AZ::Failure(AZStd::string("Input entity should be its owning Instance's container entity."));
             }
 
-            AZ_PROFILE_FUNCTION(AzToolsFramework);
+            // Detaching a prefab follows a few steps here.
+            // 1.  Detach the selected container entity from whichever instance owns it (so its parent instance)
+            // 2.  remove the prefab component from the container entity to turn it into a normal entity.
+            // 3.  (If keeping the container) reattach the container entity to the parent as a normal entity
+            // 4.  For each loose entity in the container prefab, reattach them to the parent instead as normal entities.
+            // 5.  For each prefab instance in the container entity, relink them to the parent instead of the container.
+            // 6.  Reorder entities to maintain the proper order
+            // 7.  (If not keeping the container) destroy the container entity.
 
+            AZ_PROFILE_FUNCTION(AzToolsFramework);
             {
-                AZ_PROFILE_SCOPE(AzToolsFramework, "Internal::DetachPrefab::UndoCapture");
+                ScopedUndoBatch undoBatch("Detach Prefab");  // outer undo is for the entire thing - detach AND (optional) delete
 
-                ScopedUndoBatch undoBatch("Detach Prefab");
-
-                InstanceOptionalReference getParentInstanceResult = owningInstance->get().GetParentInstance();
-                AZ_Assert(getParentInstanceResult.has_value(), "Can't get parent Instance from Instance of given container entity.");
+                {
+                    ScopedUndoBatch undoBatch("Detach Prefab - Actual Detach"); // inner undo is just for the detach part.
+                    AZ_PROFILE_SCOPE(AzToolsFramework, "Internal::DetachPrefab::UndoCapture");
 
-                Instance& parentInstance = getParentInstanceResult->get();
-                const TemplateId parentTemplateId = parentInstance.GetTemplateId();
+                    InstanceOptionalReference getParentInstanceResult = owningInstance->get().GetParentInstance();
+                    AZ_Assert(getParentInstanceResult.has_value(), "Can't get parent Instance from Instance of given container entity.");
 
-                // Block detaching prefab as override since it is not supported.
-                if (!m_prefabFocusHandler.IsOwningPrefabBeingFocused(parentInstance.GetContainerEntityId()))
-                {
-                    return AZ::Failure(AZStd::string("Detaching prefab as an override edit is currently not supported.\n"
-                        "To perform a prefab edit, please first enter Prefab Edit Mode on the direct owning prefab."));
-                }
+                    Instance& parentInstance = getParentInstanceResult->get();
+                    const TemplateId parentTemplateId = parentInstance.GetTemplateId();
 
-                // Get parent entity and generate a DOM before we modify its children
-                AZ::Entity* parentEntity = nullptr;
-                {
-                    AZ::EntityId parentEntityId;
-                    AZ::TransformBus::EventResult(parentEntityId, containerEntityId, &AZ::TransformBus::Events::GetParentId);
-                    parentEntity = GetEntityById(parentEntityId);
-                }
-                AZ_Assert(parentEntity, "Can't get the parent entity of the detached prefab instance.");
-
-                PrefabDom parentEntityDomBefore;
-                m_instanceToTemplateInterface->GenerateEntityDomBySerializing(parentEntityDomBefore, *parentEntity);
-
-                // Detach the prefab instance and remove the link
-                AZStd::unique_ptr<Instance> detachedInstance =
-                    parentInstance.DetachNestedInstance(owningInstance->get().GetInstanceAlias());
-                AZ_Assert(detachedInstance, "Can't detach selected Instance from its parent Instance.");
-
-                RemoveLink(detachedInstance, parentTemplateId, undoBatch.GetUndoBatch());
-
-                // Detach container entity from the detached instance
-                AZStd::unique_ptr<AZ::Entity> containerEntityPtr = detachedInstance->DetachContainerEntity();
-                AZ::Entity& containerEntity = *containerEntityPtr.release();
-                EditorPrefabComponent* editorPrefabComponent = containerEntity.FindComponent<EditorPrefabComponent>();
-                containerEntity.Deactivate();
-                [[maybe_unused]] const bool editorPrefabComponentRemoved = containerEntity.RemoveComponent(editorPrefabComponent);
-                AZ_Assert(editorPrefabComponentRemoved, "Remove EditorPrefabComponent failed.");
-                delete editorPrefabComponent;
-                containerEntity.Activate();
-
-                // Add the container entity to parent instance and add it to list so that we can update it in template as well.
-                [[maybe_unused]] const bool containerEntityAdded = parentInstance.AddEntity(containerEntity);
-                AZ_Assert(containerEntityAdded, "Add target Instance's container entity to its parent Instance failed.");
-
-                AZStd::vector<const AZ::Entity*> detachedEntitiesToUpdate;
-                detachedEntitiesToUpdate.push_back(&containerEntity);
-
-                // Detach entities and add them to the parent instance.
-                detachedInstance->DetachEntities(
-                    [&detachedEntitiesToUpdate, &parentInstance](AZStd::unique_ptr<AZ::Entity> entityPtr)
+                    // Block detaching prefab as override since it is not supported.
+                    if (!m_prefabFocusHandler.IsOwningPrefabBeingFocused(parentInstance.GetContainerEntityId()))
                     {
-                        AZ::Entity* detachedEntity = entityPtr.release();
-                        [[maybe_unused]] const bool entityAdded = parentInstance.AddEntity(*detachedEntity);
-                        AZ_Assert(entityAdded, "Add target Instance's entity to its parent Instance failed.");
-
-                        detachedEntitiesToUpdate.push_back(detachedEntity);
-                    });
-
-                // Detach nested instances and add them to the parent instance.
-                // This step needs to happen after detaching entities to make sure the parent entity reference inside nested instance
-                // is up to date since its parent entity is also moved to a new owning instance.
-                detachedInstance->DetachNestedInstances(
-                    [&](AZStd::unique_ptr<Instance> detachedNestedInstance)
-                    {
-                        PrefabDom& nestedInstanceTemplateDom =
-                            m_prefabSystemComponentInterface->FindTemplateDom(detachedNestedInstance->GetTemplateId());
+                        return AZ::Failure(AZStd::string("Detaching prefab as an override edit is currently not supported.\n"
+                            "To perform a prefab edit, please first enter Prefab Edit Mode on the direct owning prefab."));
+                    }
 
-                        Instance& nestedInstanceUnderNewParent = parentInstance.AddInstance(AZStd::move(detachedNestedInstance));
+                    // Get parent entity and generate a DOM before we modify its children
+                    AZ::Entity* parentEntity = nullptr;
+                    AZ::EntityId containerParentId(AZ::EntityId::InvalidEntityId);
+                    AZ::TransformBus::EventResult(containerParentId, containerEntityId, &AZ::TransformBus::Events::GetParentId);
+                    parentEntity = GetEntityById(containerParentId);
+                    AZ_Assert(parentEntity, "Can't get the parent entity of the detached prefab instance.");
 
-                        PrefabDom nestedInstanceDomUnderNewParent;
-                        m_instanceToTemplateInterface->GenerateInstanceDomBySerializing(
-                            nestedInstanceDomUnderNewParent, nestedInstanceUnderNewParent);
+                    // Capture the order that entities were in the container before we do any moving around
+                    AZStd::vector<AZ::EntityId> priorOrder = AzToolsFramework::GetEntityChildOrder(containerEntityId);
 
-                        nestedInstanceDomUnderNewParent.RemoveMember(PrefabDomUtils::LinkIdName);
+                    PrefabDom parentEntityDomBefore;
+                    m_instanceToTemplateInterface->GenerateEntityDomBySerializing(parentEntityDomBefore, *parentEntity);
 
-                        PrefabDom reparentPatch;
-                        m_instanceToTemplateInterface->GeneratePatch(
-                            reparentPatch, nestedInstanceTemplateDom, nestedInstanceDomUnderNewParent);
+                    // before we detach the container entity, capture its alias path in case we need to remove it.
+                    const AZStd::string containerEntityAliasPath = m_instanceToTemplateInterface->GenerateEntityAliasPath(containerEntityId);
+                    // Detach the prefab instance and remove the link
+                    AZStd::unique_ptr<Instance> detachedInstance =
+                        parentInstance.DetachNestedInstance(owningInstance->get().GetInstanceAlias());
+                    AZ_Assert(detachedInstance, "Can't detach selected Instance from its parent Instance.");
+
+                    RemoveLink(detachedInstance, parentTemplateId, undoBatch.GetUndoBatch());
+
+                    // Detach container entity from the detached instance
+                    AZStd::unique_ptr<AZ::Entity> containerEntity = detachedInstance->DetachContainerEntity();
+                    EditorPrefabComponent* editorPrefabComponent = containerEntity->FindComponent<EditorPrefabComponent>();
+                    containerEntity->Deactivate();
+                    [[maybe_unused]] const bool editorPrefabComponentRemoved = containerEntity->RemoveComponent(editorPrefabComponent);
+                    AZ_Assert(editorPrefabComponentRemoved, "Remove EditorPrefabComponent failed.");
+                    delete editorPrefabComponent;
+                    containerEntity->Activate();
+
+                    AZStd::vector<const AZ::Entity*> detachedEntitiesToUpdate;
+                    AZStd::vector<AZ::EntityId> entitiesToReorder; // entities that need to be moved in the parent order.
+                    // Add the container entity to parent instance and add it to list so that we can update it in template as well.
+                    // it will be deleted later.
+                    detachedEntitiesToUpdate.push_back(containerEntity.get());
+                    [[maybe_unused]] const bool containerEntityAdded = parentInstance.AddEntity(AZStd::move(containerEntity));
+                    AZ_Assert(containerEntityAdded, "Add target Instance's container entity to its parent Instance failed.");
+
+                    // Detach entities and add them to the parent instance.
+                    detachedInstance->DetachEntities(
+                        [&](AZStd::unique_ptr<AZ::Entity> entityPtr)
+                        {
+                            AZ::Entity* detachedEntity = entityPtr.release();
+                            [[maybe_unused]] const bool entityAdded = parentInstance.AddEntity(*detachedEntity);
+                            AZ_Assert(entityAdded, "Add target Instance's entity to its parent Instance failed.");
 
-                        // Create link and update template with the new instance DOM.
-                        CreateLink(
-                            nestedInstanceUnderNewParent, parentTemplateId, undoBatch.GetUndoBatch(), AZStd::move(reparentPatch), true);
-                    });
+                            detachedEntitiesToUpdate.push_back(detachedEntity);
+                            if (!keepContainerEntity)
+                            {
+                                // before we capture the new dom and make a patch, update its parent if necessary:
+                                AZ::EntityId currentEntityId(detachedEntity->GetId());
+                                AZ::EntityId currentParentId(AZ::EntityId::InvalidEntityId);
+                                AZ::TransformBus::EventResult(currentParentId, currentEntityId, &AZ::TransformBus::Events::GetParentId);
+                                if (currentParentId == containerEntityId)
+                                {
+                                    // if it was parented to the container, update its parent to the parent of the container
+                                    AZ::TransformBus::Event(currentEntityId, &AZ::TransformBus::Events::SetParent, containerParentId);
+                                }
+                            }
+                        });
 
-                // Update template with the new entity DOMs.
-                PrefabUndoHelpers::AddEntityDoms(detachedEntitiesToUpdate, parentInstance.GetTemplateId(), undoBatch.GetUndoBatch());
+                    // Detach nested instances and add them to the parent instance.
+                    // This step needs to happen after detaching entities to make sure the parent entity reference inside nested instance
+                    // is up to date since its parent entity is also moved to a new owning instance.
+                    detachedInstance->DetachNestedInstances(
+                        [&](AZStd::unique_ptr<Instance> detachedNestedInstance)
+                        {
+                            PrefabDom& nestedInstanceTemplateDom =
+                                m_prefabSystemComponentInterface->FindTemplateDom(detachedNestedInstance->GetTemplateId());
 
-                // Update parent entity of the container entity in template with the new sort order information
-                // Note: Currently, we do not update the parent entity DOM value in cached instance DOM of parent instance.
-                // So, in prefab template progation it would trigger reloading on the parent entity to consume the detached container
-                // entity (id) that was also reloaded. If not, the detached container would be added to the end of list.
-                PrefabDom parentEntityDomAfter;
-                m_instanceToTemplateInterface->GenerateEntityDomBySerializing(parentEntityDomAfter, *parentEntity);
-                PrefabUndoHelpers::UpdateEntity(
-                    parentEntityDomBefore, parentEntityDomAfter, parentEntity->GetId(), undoBatch.GetUndoBatch(), false);
-            }
+                            Instance& nestedInstanceUnderNewParent = parentInstance.AddInstance(AZStd::move(detachedNestedInstance));
 
-            AzToolsFramework::ToolsApplicationRequestBus::Broadcast(
-                &AzToolsFramework::ToolsApplicationRequestBus::Events::ClearDirtyEntities);
+                            if (!keepContainerEntity)
+                            {
+                                // before we capture the new dom and make a patch, update its parent if necessary:
+                                AZ::EntityId currentEntityId(nestedInstanceUnderNewParent.GetContainerEntityId());
+                                AZ::EntityId currentParentId(AZ::EntityId::InvalidEntityId);
+                                AZ::TransformBus::EventResult(currentParentId, currentEntityId, &AZ::TransformBus::Events::GetParentId);
+                                if (currentParentId == containerEntityId)
+                                {
+                                    // if it was parented to the container, update its parent to the parent of the container
+                                    AZ::TransformBus::Event(currentEntityId, &AZ::TransformBus::Events::SetParent, containerParentId);
+                                }
+                            }
+                            
+                            PrefabDom nestedInstanceDomUnderNewParent;
+                            m_instanceToTemplateInterface->GenerateInstanceDomBySerializing(
+                                nestedInstanceDomUnderNewParent, nestedInstanceUnderNewParent);
+
+                            nestedInstanceDomUnderNewParent.RemoveMember(PrefabDomUtils::LinkIdName);
+
+                            PrefabDom reparentPatch;
+                            m_instanceToTemplateInterface->GeneratePatch(
+                                reparentPatch, nestedInstanceTemplateDom, nestedInstanceDomUnderNewParent);
+
+                            // Create link and update template with the new instance DOM.
+                            CreateLink(
+                                nestedInstanceUnderNewParent, parentTemplateId, undoBatch.GetUndoBatch(), AZStd::move(reparentPatch), true);
+                        });
+                    PrefabUndoHelpers::AddEntityDoms(detachedEntitiesToUpdate, parentInstance.GetTemplateId(), undoBatch.GetUndoBatch());
+
+                    if (!keepContainerEntity)
+                    {
+                        // Reorder entities in the parent instance to maintain the proper order.
+                        // note that the predicted final entityIds must be used since the instance will re-instantiate
+                        // the entities with new Ids, and the new Ids will be based on their new path in the hierarchy.
+
+                        AZStd::vector<AZ::EntityId> predictedChildEntityIds;
+                        predictedChildEntityIds.reserve(priorOrder.size());
+                        for (const auto& entityId : priorOrder)
+                        {
+                            // predict the final EntityId of the child entities
+                            auto resultInstance = parentInstance.FindInstanceAndAlias(entityId);
+                            if (resultInstance.first)
+                            {
+                                // get the absolute alias path of the parent instance (the parent of the container entity
+                                AliasPath aliasPath = resultInstance.first->GetAbsoluteInstanceAliasPath();
+                        
+                                aliasPath.Append(resultInstance.second);
+                                AZ::EntityId newId = InstanceEntityIdMapper::GenerateEntityIdForAliasPath(aliasPath);
+                                if (newId.IsValid())
+                                {
+                                    predictedChildEntityIds.push_back(newId);
+                                }
+                            }
+                        }
+                        AZStd::vector<AZ::EntityId> currentChildOrder = AzToolsFramework::GetEntityChildOrder(containerParentId);
+                        // rearrange the order - move any reparented children from the currentChildOrder list to
+                        // be after the entity that was before the container.
 
+                        // remove any entities from the currentChildOrder that could potentially be reordered so that they
+                        // can then be re-inserted in the correct place:
+                        auto removeElementsInVectorMatchingAnotherVector = [](AZStd::vector<AZ::EntityId>& vector, const AZStd::vector<AZ::EntityId>& toRemove)
+                        {
+                            // use erase-remove-if idiom:
+                            vector.erase(
+                                
+                                AZStd::remove_if(vector.begin(), vector.end(), [&toRemove](const AZ::EntityId& entityId)
+                                {
+                                    return AZStd::find(toRemove.begin(), toRemove.end(), entityId) != toRemove.end();
+                                }),
+                                vector.end());
+                        };
+                        removeElementsInVectorMatchingAnotherVector(currentChildOrder, priorOrder);
+                        removeElementsInVectorMatchingAnotherVector(currentChildOrder, predictedChildEntityIds);
+
+                        auto insertPoint = AZStd::find(currentChildOrder.begin(), currentChildOrder.end(), containerEntityId);
+                        currentChildOrder.insert(insertPoint, predictedChildEntityIds.begin(), predictedChildEntityIds.end());
+                        AzToolsFramework::SetEntityChildOrder(containerParentId, currentChildOrder);
+
+                        // update the undo cache with the new parenting information for the parents that changed,
+                        // since we will be clearing all the dirty flags and the cache won't thus update itself.
+                        for (const auto& entityId : priorOrder)
+                        {
+                            m_prefabUndoCache.UpdateCache(entityId);
+                        }
+                    }
+
+                    // Update parent entity of the container entity in template with the new sort order information
+                    // Note: Currently, we do not update the parent entity DOM value in cached instance DOM of parent instance.
+                    // So, in prefab template propagation it would trigger reloading on the parent entity to consume the detached container
+                    // entity (id) that was also reloaded. If not, the detached container would be added to the end of list.
+                    PrefabDom parentEntityDomAfter;
+                    m_instanceToTemplateInterface->GenerateEntityDomBySerializing(parentEntityDomAfter, *parentEntity);
+                    PrefabUndoHelpers::UpdateEntity(parentEntityDomBefore, parentEntityDomAfter, parentEntity->GetId(), undoBatch.GetUndoBatch(), false);
+
+                    // before the current undo batch expires, clear any dirty entities.
+                    ToolsApplicationRequestBus::Broadcast(&ToolsApplicationRequestBus::Events::ClearDirtyEntities);
+                } // end of first "inner" undo batch
+                
+                // now that a complete undo batch is done for the operation which leaves the data intact, we can delete
+                // the leftover container entity in a normal "delete this thing" step.  We can behave as if the entity
+                // is always attached.
+
+                // note that this is still within the scope of the "outer" undo batch, so it still counts as one operation.
+                if (!keepContainerEntity)
+                {   
+                    DeleteFromInstance({containerEntityId});
+                }
+
+                // before the current undo batch expires, clear any dirty entities.
+                ToolsApplicationRequestBus::Broadcast(&ToolsApplicationRequestBus::Events::ClearDirtyEntities);
+            } // end of the "outer" undo batch.
+            
             return AZ::Success();
         }
 
@@ -1986,7 +2102,7 @@ namespace AzToolsFramework
             const EntityAlias& parentEntityAlias,
             const EntityAlias& entityToAddAlias)
         {
-            // Find the parent entity to get its sort order component
+                        // Find the parent entity to get its sort order component
             auto findParentEntity = [&]() -> rapidjson::Value*
             {
                 if (auto containerEntityIter = domToAddEntityUnder.FindMember(PrefabDomUtils::ContainerEntityName);
@@ -2037,7 +2153,7 @@ namespace AzToolsFramework
             AzToolsFramework::EntityIdList selectedEntities;
             AzToolsFramework::ToolsApplicationRequestBus::BroadcastResult(
                 selectedEntities, &AzToolsFramework::ToolsApplicationRequests::GetSelectedEntities);
-
+            
             // Find the EditorEntitySortComponent DOM
             auto componentsIter = parentEntityValue->FindMember(PrefabDomUtils::ComponentsName);
             if (componentsIter == parentEntityValue->MemberEnd())
@@ -2114,7 +2230,7 @@ namespace AzToolsFramework
 
                 // Replace the order with our newly constructed one
                 orderMembersIter->value.Swap(newOrder);
-                break;
+                                break;
             }
         }
 
@@ -2240,7 +2356,7 @@ namespace AzToolsFramework
                 if (!parentEntityAlias.empty())
                 {
                     AddNewEntityToSortOrder(commonOwningInstance, domToAddDuplicatedEntitiesUnder, parentEntityAlias, newEntityAlias);
-                }
+                                    }
 
                 // Add the new Entity DOM to the Entities member of the instance
                 rapidjson::Value aliasName(newEntityAlias.c_str(), static_cast<rapidjson::SizeType>(newEntityAlias.length()), domToAddDuplicatedEntitiesUnder.GetAllocator());

+ 3 - 0
Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.h

@@ -69,8 +69,11 @@ namespace AzToolsFramework
             DuplicatePrefabResult DuplicateEntitiesInInstance(const EntityIdList& entityIds) override;
 
             PrefabOperationResult DetachPrefab(const AZ::EntityId& containerEntityId) override;
+            PrefabOperationResult DetachPrefabAndRemoveContainerEntity(const AZ::EntityId& containerEntityId) override;
 
         private:
+            PrefabOperationResult DetachPrefabImpl(const AZ::EntityId& containerEntityId, bool keepContainerEntity);
+
             PrefabOperationResult DeleteFromInstance(const EntityIdList& entityIds);
             PrefabOperationResult RetrieveAndSortPrefabEntitiesAndInstances(
                 const EntityList& inputEntities,

+ 13 - 0
Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicInterface.h

@@ -190,10 +190,23 @@ namespace AzToolsFramework
               * instance and the parent, removing links between this instance and its nested instances, and adding entities directly
               * owned by this instance under the parent instance.
               * Bails if the entity is not a container entity or belongs to the level prefab instance.
+              * Note that this function retains the container entities, unlike the below function
+              * @ref DetachPrefabAndRemoveContainerEntity.
               * @param containerEntityId The container entity id of the instance to detach.
               * @return An outcome object; on failure, it comes with an error message detailing the cause of the error.
               */
             virtual PrefabOperationResult DetachPrefab(const AZ::EntityId& containerEntityId) = 0;
+
+            /**
+              * Does the same thing as @ref DetachPrefab, but also removes the container entity representing the prefab instance.
+              * This re-parents anything that used to be a child of the container entity to the container entity's parent.
+              * This operation is essentially the reverse of what happens when you create a prefab (which creates a new 
+              * container entity and re-parents the entities under it.
+              * Note that the previous API only had "DetachPrefab", which kept the container entities, 
+              * and by way of introducing as little risk a possible in an API change, the old function
+              * retains its original behavior and name.
+              */
+            virtual PrefabOperationResult DetachPrefabAndRemoveContainerEntity(const AZ::EntityId& containerEntityId) = 0;
         };
 
     } // namespace Prefab

+ 11 - 0
Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicRequestBus.h

@@ -82,6 +82,17 @@ namespace AzToolsFramework
               */
             virtual PrefabOperationResult DetachPrefab(const AZ::EntityId& containerEntityId) = 0;
 
+            /**
+              * The same operation as DetachPrefab, except it also removes the container entity and reparents its children 
+              * to the parent of the container entity.  This operation is the opposite operation of creating a prefab, which
+              * creates the container entity and reparents the children to the container entity.
+              * Note that the above function was the original API call, which originally kept the 
+              * container entities.   In order to ensure the API is stable for callers, the above
+              * function's outcome is left unchanged, and the new functionality to remove the container
+              * entity is instead added to a new API call.
+              */
+            virtual PrefabOperationResult DetachPrefabAndRemoveContainerEntity(const AZ::EntityId& containerEntityId) = 0;
+
             /**
               * Duplicates all entities in the owning instance. Bails if the entities don't all belong to the same instance.
               * Return an outcome object with a list of ids of given entities' duplicates if duplication succeeded;

+ 6 - 0
Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicRequestHandler.cpp

@@ -35,6 +35,7 @@ namespace AzToolsFramework
                     ->Event("InstantiatePrefab", &PrefabPublicRequests::InstantiatePrefab)
                     ->Event("DeleteEntitiesAndAllDescendantsInInstance", &PrefabPublicRequests::DeleteEntitiesAndAllDescendantsInInstance)
                     ->Event("DetachPrefab", &PrefabPublicRequests::DetachPrefab)
+                    ->Event("DetachPrefabAndRemoveContainerEntity", &PrefabPublicRequests::DetachPrefabAndRemoveContainerEntity)
                     ->Event("DuplicateEntitiesInInstance", &PrefabPublicRequests::DuplicateEntitiesInInstance)
                     ->Event("GetOwningInstancePrefabPath", &PrefabPublicRequests::GetOwningInstancePrefabPath)
                     ->Event("CreateInMemorySpawnableAsset", &PrefabPublicRequests::CreateInMemorySpawnableAsset)
@@ -81,6 +82,11 @@ namespace AzToolsFramework
             return m_prefabPublicInterface->DetachPrefab(containerEntityId);
         }
 
+        PrefabOperationResult PrefabPublicRequestHandler::DetachPrefabAndRemoveContainerEntity(const AZ::EntityId& containerEntityId)
+        {
+            return m_prefabPublicInterface->DetachPrefabAndRemoveContainerEntity(containerEntityId);
+        }
+
         DuplicatePrefabResult PrefabPublicRequestHandler::DuplicateEntitiesInInstance(const EntityIdList& entityIds)
         {
             return m_prefabPublicInterface->DuplicateEntitiesInInstance(entityIds);

+ 1 - 0
Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicRequestHandler.h

@@ -36,6 +36,7 @@ namespace AzToolsFramework
             InstantiatePrefabResult InstantiatePrefab(AZStd::string_view filePath, AZ::EntityId parent, const AZ::Vector3& position) override;
             PrefabOperationResult DeleteEntitiesAndAllDescendantsInInstance(const EntityIdList& entityIds) override;
             PrefabOperationResult DetachPrefab(const AZ::EntityId& containerEntityId) override;
+            PrefabOperationResult DetachPrefabAndRemoveContainerEntity(const AZ::EntityId& containerEntityId) override;
             DuplicatePrefabResult DuplicateEntitiesInInstance(const EntityIdList& entityIds) override;
             AZStd::string GetOwningInstancePrefabPath(AZ::EntityId entityId) const override;
             CreateSpawnableResult CreateInMemorySpawnableAsset(AZStd::string_view prefabFilePath, AZStd::string_view spawnableName) override;

+ 22 - 0
Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabSettings.h

@@ -0,0 +1,22 @@
+/*
+ * Copyright (c) Contributors to the Open 3D Engine Project.
+ * For complete copyright and license terms please see the LICENSE at the root of this distribution.
+ *
+ * SPDX-License-Identifier: Apache-2.0 OR MIT
+ *
+ */
+
+#pragma once
+
+//! @file contains settings that are tweakable for the Prefab system.
+//! These settings are stored in the settings registry so as to be available to all parts of the system without
+//! the use of the legacy settings system.
+//! These values are in this file in AzToolsFramework so that it can be included without bringing
+//! in any heavy-weight dependencies.
+
+#include <AzCore/std/string/string_view.h>
+namespace AzToolsFramework::Prefab::Settings
+{
+    constexpr AZStd::string_view DetachPrefabRemovesContainerName = "/Amazon/Preferences/Editor/General/DetachPrefabRemovesContainer";
+    constexpr const bool DetachPrefabRemovesContainerDefault = true;
+}

+ 11 - 2
Code/Framework/AzToolsFramework/AzToolsFramework/UI/Prefab/PrefabIntegrationManager.cpp

@@ -20,6 +20,7 @@
 #include <AzToolsFramework/ActionManager/HotKey/HotKeyManagerInterface.h>
 #include <AzToolsFramework/ActionManager/Menu/MenuManagerInterface.h>
 #include <AzToolsFramework/ActionManager/ToolBar/ToolBarManagerInterface.h>
+#include <AzToolsFramework/API/SettingsRegistryUtils.h>
 #include <AzToolsFramework/ComponentMode/EditorComponentModeBus.h>
 #include <AzToolsFramework/ContainerEntity/ContainerEntityInterface.h>
 #include <AzToolsFramework/Editor/ActionManagerIdentifiers/EditorActionUpdaterIdentifiers.h>
@@ -39,6 +40,7 @@
 #include <AzToolsFramework/Prefab/Overrides/PrefabOverridePublicInterface.h>
 #include <AzToolsFramework/Prefab/PrefabPublicInterface.h>
 #include <AzToolsFramework/Prefab/Procedural/ProceduralPrefabAsset.h>
+#include <AzToolsFramework/Prefab/PrefabSettings.h>
 #include <AzToolsFramework/UI/EditorEntityUi/EditorEntityUiInterface.h>
 #include <AzToolsFramework/UI/Prefab/ActionManagerIdentifiers/PrefabActionUpdaterIdentifiers.h>
 #include <AzToolsFramework/UI/Prefab/PrefabIntegrationInterface.h>
@@ -500,7 +502,8 @@ namespace AzToolsFramework
                 AZStd::string actionIdentifier = "o3de.action.prefabs.detach";
                 AzToolsFramework::ActionProperties actionProperties;
                 actionProperties.m_name = "Detach Prefab";
-                actionProperties.m_description = "Creates a prefab out of the currently selected entities.";
+                actionProperties.m_description = "Selected prefab is detached from its prefab file and becomes normal entities instead. "
+                                                 "You can change whether or not this action keeps the container entity in the editor settings panel.";
                 actionProperties.m_category = "Prefabs";
 
                 m_actionManagerInterface->RegisterAction(
@@ -849,6 +852,7 @@ namespace AzToolsFramework
             m_menuManagerInterface->AddActionToMenu(EditorIdentifiers::EntityOutlinerContextMenuIdentifier, "o3de.action.prefabs.closeInstance", 10900);
             m_menuManagerInterface->AddActionToMenu(EditorIdentifiers::EntityOutlinerContextMenuIdentifier, "o3de.action.prefabs.create", 20100);
             m_menuManagerInterface->AddActionToMenu(EditorIdentifiers::EntityOutlinerContextMenuIdentifier, "o3de.action.prefabs.detach", 20200);
+            m_menuManagerInterface->AddActionToMenu(EditorIdentifiers::EntityOutlinerContextMenuIdentifier, "o3de.action.prefabs.detach_and_keep_container_entities", 20210);
             m_menuManagerInterface->AddActionToMenu(EditorIdentifiers::EntityOutlinerContextMenuIdentifier, "o3de.action.prefabs.instantiate", 20300);
             m_menuManagerInterface->AddActionToMenu(EditorIdentifiers::EntityOutlinerContextMenuIdentifier, "o3de.action.prefabs.procedural.instantiate", 20400);
             m_menuManagerInterface->AddActionToMenu(EditorIdentifiers::EntityOutlinerContextMenuIdentifier, "o3de.action.prefabs.save", 30100);
@@ -862,6 +866,7 @@ namespace AzToolsFramework
             m_menuManagerInterface->AddActionToMenu(EditorIdentifiers::ViewportContextMenuIdentifier, "o3de.action.prefabs.closeInstance", 10900);
             m_menuManagerInterface->AddActionToMenu(EditorIdentifiers::ViewportContextMenuIdentifier, "o3de.action.prefabs.create", 20100);
             m_menuManagerInterface->AddActionToMenu(EditorIdentifiers::ViewportContextMenuIdentifier, "o3de.action.prefabs.detach", 20200);
+            m_menuManagerInterface->AddActionToMenu(EditorIdentifiers::ViewportContextMenuIdentifier, "o3de.action.prefabs.detach_and_keep_container_entities", 20210);
             m_menuManagerInterface->AddActionToMenu(EditorIdentifiers::ViewportContextMenuIdentifier, "o3de.action.prefabs.instantiate", 20300);
             m_menuManagerInterface->AddActionToMenu(EditorIdentifiers::ViewportContextMenuIdentifier, "o3de.action.prefabs.procedural.instantiate", 20400);
             m_menuManagerInterface->AddActionToMenu(EditorIdentifiers::ViewportContextMenuIdentifier, "o3de.action.prefabs.save", 30100);
@@ -1294,7 +1299,11 @@ namespace AzToolsFramework
 
         void PrefabIntegrationManager::ContextMenu_DetachPrefab(AZ::EntityId containerEntity)
         {
-            PrefabOperationResult detachPrefabResult = s_prefabPublicInterface->DetachPrefab(containerEntity);
+            bool shouldRemoveContainer = AzToolsFramework::GetRegistry(Settings::DetachPrefabRemovesContainerName, Settings::DetachPrefabRemovesContainerDefault);
+
+            PrefabOperationResult detachPrefabResult = shouldRemoveContainer ? 
+                       s_prefabPublicInterface->DetachPrefabAndRemoveContainerEntity(containerEntity)
+                    : s_prefabPublicInterface->DetachPrefab(containerEntity);
 
             if (!detachPrefabResult.IsSuccess())
             {

+ 1 - 0
Code/Framework/AzToolsFramework/AzToolsFramework/Viewport/ViewportSettings.cpp

@@ -7,6 +7,7 @@
  */
 
 #include <AzToolsFramework/Viewport/ViewportSettings.h>
+#include <AzToolsFramework/API/SettingsRegistryUtils.h>
 
 namespace AzToolsFramework
 {

+ 0 - 34
Code/Framework/AzToolsFramework/AzToolsFramework/Viewport/ViewportSettings.h

@@ -8,44 +8,10 @@
 
 #pragma once
 
-#include <AzCore/Settings/SettingsRegistry.h>
 #include <AzToolsFramework/Viewport/ViewportTypes.h>
 
 namespace AzToolsFramework
 {
-    template<typename T>
-    void SetRegistry(const AZStd::string_view setting, T&& value)
-    {
-        if (auto* registry = AZ::SettingsRegistry::Get())
-        {
-            registry->Set(setting, AZStd::forward<T>(value));
-        }
-    }
-
-    template<typename T>
-    AZStd::remove_cvref_t<T> GetRegistry(const AZStd::string_view setting, T&& defaultValue)
-    {
-        AZStd::remove_cvref_t<T> value = AZStd::forward<T>(defaultValue);
-        if (const auto* registry = AZ::SettingsRegistry::Get())
-        {
-            T potentialValue;
-            if (registry->Get(potentialValue, setting))
-            {
-                value = AZStd::move(potentialValue);
-            }
-        }
-
-        return value;
-    }
-
-    inline void ClearRegistry(const AZStd::string_view setting)
-    {
-        if (auto* registry = AZ::SettingsRegistry::Get())
-        {
-            registry->Remove(setting);
-        }
-    }
-
     inline constexpr float DefaultManipulatorViewBaseScale = 1.0f;
     inline constexpr float MinManipulatorViewBaseScale = 0.25f;
     inline constexpr float MaxManipulatorViewBaseScale = 2.0f;

+ 2 - 0
Code/Framework/AzToolsFramework/AzToolsFramework/aztoolsframework_files.cmake

@@ -91,6 +91,7 @@ set(FILES
     API/EditorViewportIconDisplayInterface.h
     API/PythonLoader.h
     API/PythonLoader.cpp
+    API/SettingsRegistryUtils.h
     API/ViewPaneOptions.h
     API/ViewportEditorModeTrackerInterface.h
     Application/Ticker.h
@@ -864,6 +865,7 @@ set(FILES
     Prefab/PrefabPublicRequestBus.h
     Prefab/PrefabPublicRequestHandler.h
     Prefab/PrefabPublicRequestHandler.cpp
+    Prefab/PrefabSettings.h
     Prefab/PrefabUndoCache.cpp
     Prefab/PrefabUndoCache.h
     Prefab/PrefabUndoHelpers.cpp

+ 576 - 0
Code/Framework/AzToolsFramework/Tests/Prefab/PrefabDetachPrefabAndRemoveContainerTests.cpp

@@ -0,0 +1,576 @@
+/*
+ * Copyright (c) Contributors to the Open 3D Engine Project.
+ * For complete copyright and license terms please see the LICENSE at the root of this distribution.
+ *
+ * SPDX-License-Identifier: Apache-2.0 OR MIT
+ *
+ */
+
+// this file is essentially a duplicate of PrefabDetachPrefabTests.cpp
+// but it calls the API which deletes the container.
+// This means that SOME, but not necessarily all, of the setup for each test is the same
+// But the actual outcome is very different for each test.
+
+#include <AzToolsFramework/Entity/EditorEntityHelpers.h>
+#include <Prefab/PrefabTestFixture.h>
+
+namespace UnitTest
+{
+    using PrefabDetachPrefabTests = PrefabTestFixture;
+
+    TEST_F(PrefabDetachPrefabTests, DetachPrefabAndRemoveContainerEntityUnderLevelSucceeds)
+    {
+        // Level
+        // | Car       (prefab)  <-- detach prefab
+        //   | Tire
+        //     | Belt
+
+        // detaching removes the wrapper prefab object, so result is just
+        // Level
+        //   | Tire
+        //     | Belt
+
+        const AZStd::string carPrefabName = "CarPrefab";
+        const AZStd::string tireEntityName = "Tire";
+        const AZStd::string beltEntityName = "Belt";
+
+        AZ::IO::Path engineRootPath;
+        m_settingsRegistryInterface->Get(engineRootPath.Native(), AZ::SettingsRegistryMergeUtils::FilePathKey_EngineRootFolder);
+        AZ::IO::Path carPrefabFilepath = engineRootPath / carPrefabName;
+
+        AZ::EntityId tireEntityId = CreateEditorEntityUnderRoot(tireEntityName);
+        CreateEditorEntity(beltEntityName, tireEntityId);
+        AZ::EntityId carContainerId = CreateEditorPrefab(carPrefabFilepath, { tireEntityId });
+
+        InstanceAlias carInstanceAlias = FindNestedInstanceAliasInInstance(GetRootContainerEntityId(), carPrefabName);
+
+        // Detach the car prefab.
+        PrefabOperationResult result = m_prefabPublicInterface->DetachPrefabAndRemoveContainerEntity(carContainerId);
+        ASSERT_TRUE(result.IsSuccess());
+
+        PropagateAllTemplateChanges();
+
+        // Validate there is no nested instance in the level prefab instance.
+        ValidateNestedInstanceNotUnderInstance(GetRootContainerEntityId(), carInstanceAlias);
+
+        InstanceOptionalReference levelInstance = m_instanceEntityMapperInterface->FindOwningInstance(GetRootContainerEntityId());
+        ASSERT_TRUE(levelInstance.has_value());
+
+        // Validate there are two entities in the level prefab instance (Tire, Belt)
+        EXPECT_EQ(levelInstance->get().GetEntityAliasCount(), 2);
+
+        // Validate that the car entity (the prefab wrapper) does not exist.
+        AZStd::string carEntityAliasAfterDetach = FindEntityAliasInInstance(GetRootContainerEntityId(), carPrefabName);
+        EXPECT_STREQ(carEntityAliasAfterDetach.c_str(), "");
+
+        // Validate that the tire's parent entity is the level container entity.
+        AZStd::string tireEntityAliasAfterDetach = FindEntityAliasInInstance(GetRootContainerEntityId(), tireEntityName);
+        AZ::EntityId tireEntityIdAfterDetach = levelInstance->get().GetEntityId(tireEntityAliasAfterDetach);
+        EXPECT_TRUE(tireEntityIdAfterDetach.IsValid());
+
+        AZ::EntityId parentEntityIdForTire;
+        AZ::TransformBus::EventResult(parentEntityIdForTire, tireEntityIdAfterDetach, &AZ::TransformInterface::GetParentId);
+        EXPECT_EQ(levelInstance->get().GetContainerEntityId(), parentEntityIdForTire);
+
+        // Validate that the belt's parent entity is the tire.
+        AZStd::string beltEntityAliasAfterDetach = FindEntityAliasInInstance(GetRootContainerEntityId(), beltEntityName);
+        AZ::EntityId beltEntityIdAfterDetach = levelInstance->get().GetEntityId(beltEntityAliasAfterDetach);
+        EXPECT_TRUE(beltEntityIdAfterDetach.IsValid());
+
+        AZ::EntityId parentEntityIdForBelt;
+        AZ::TransformBus::EventResult(parentEntityIdForBelt, beltEntityIdAfterDetach, &AZ::TransformInterface::GetParentId);
+        EXPECT_EQ(tireEntityIdAfterDetach, parentEntityIdForBelt);
+    }
+
+    TEST_F(PrefabDetachPrefabTests, DetachPrefabAndRemoveContainerEntityUnderParentSucceeds)
+    {
+        // Level
+        // | Garage
+        //   | Car       (prefab)  <-- detach prefab
+        //     | Tire
+
+        // expected result
+        // Level
+        // | Garage
+        //     | Tire (car is gone)
+
+        const AZStd::string carPrefabName = "CarPrefab";
+        const AZStd::string garageEntityName = "Garage";
+        const AZStd::string tireEntityName = "Tire";
+
+        AZ::IO::Path engineRootPath;
+        m_settingsRegistryInterface->Get(engineRootPath.Native(), AZ::SettingsRegistryMergeUtils::FilePathKey_EngineRootFolder);
+        AZ::IO::Path carPrefabFilepath = engineRootPath / carPrefabName;
+
+        AZ::EntityId garageEntityId = CreateEditorEntityUnderRoot(garageEntityName);
+        AZ::EntityId tireEntityId = CreateEditorEntity(tireEntityName, garageEntityId);
+        AZ::EntityId carContainerId = CreateEditorPrefab(carPrefabFilepath, { tireEntityId });
+
+        InstanceAlias carInstanceAlias = FindNestedInstanceAliasInInstance(GetRootContainerEntityId(), carPrefabName);
+
+        // Detach the car prefab.
+        PrefabOperationResult result = m_prefabPublicInterface->DetachPrefabAndRemoveContainerEntity(carContainerId);
+        ASSERT_TRUE(result.IsSuccess());
+
+        PropagateAllTemplateChanges();
+
+        // Validate there is no nested instance in the level prefab instance.
+        ValidateNestedInstanceNotUnderInstance(GetRootContainerEntityId(), carInstanceAlias);
+
+        InstanceOptionalReference levelInstance = m_instanceEntityMapperInterface->FindOwningInstance(GetRootContainerEntityId());
+        ASSERT_TRUE(levelInstance.has_value());
+
+        // Validate there are two entities in the level prefab instance (the car should be gone)
+        EXPECT_EQ(levelInstance->get().GetEntityAliasCount(), 2);
+
+        // Validate that the garage's parent entity is the level container entity.
+        AZStd::string garageEntityAliasAfterDetach = FindEntityAliasInInstance(GetRootContainerEntityId(), garageEntityName);
+        AZ::EntityId garageEntityIdAfterDetach = levelInstance->get().GetEntityId(garageEntityAliasAfterDetach);
+        EXPECT_TRUE(garageEntityIdAfterDetach.IsValid());
+
+        AZ::EntityId parentEntityIdForGarage;
+        AZ::TransformBus::EventResult(parentEntityIdForGarage, garageEntityIdAfterDetach, &AZ::TransformInterface::GetParentId);
+        EXPECT_EQ(levelInstance->get().GetContainerEntityId(), parentEntityIdForGarage);
+
+        // Validate that the car is gone
+        AZStd::string carEntityAliasAfterDetach = FindEntityAliasInInstance(GetRootContainerEntityId(), carPrefabName);
+        AZ::EntityId carEntityIdAfterDetach = levelInstance->get().GetEntityId(carEntityAliasAfterDetach);
+        EXPECT_FALSE(carEntityIdAfterDetach.IsValid());
+
+        // Validate that the tire's parent entity is the garage.
+        AZStd::string tireEntityAliasAfterDetach = FindEntityAliasInInstance(GetRootContainerEntityId(), tireEntityName);
+        AZ::EntityId tireEntityIdAfterDetach = levelInstance->get().GetEntityId(tireEntityAliasAfterDetach);
+        EXPECT_TRUE(tireEntityIdAfterDetach.IsValid());
+
+        AZ::EntityId parentEntityIdForTire;
+        AZ::TransformBus::EventResult(parentEntityIdForTire, tireEntityIdAfterDetach, &AZ::TransformInterface::GetParentId);
+        EXPECT_EQ(garageEntityIdAfterDetach, parentEntityIdForTire);
+    }
+
+    TEST_F(PrefabDetachPrefabTests, DetachPrefabAndRemoveContainerEntityWithNestedPrefabSucceeds)
+    {
+        // Level
+        // | Car       (prefab)  <-- detach prefab
+        //   | Wheel   (prefab)
+        //     | Tire
+
+        // expected result
+        // Level
+        //   | Wheel   (prefab), car is gone
+        //     | Tire
+
+
+        const AZStd::string carPrefabName = "CarPrefab";
+        const AZStd::string wheelPrefabName = "WheelPrefab";
+        const AZStd::string tireEntityName = "Tire";
+
+        AZ::IO::Path engineRootPath;
+        m_settingsRegistryInterface->Get(engineRootPath.Native(), AZ::SettingsRegistryMergeUtils::FilePathKey_EngineRootFolder);
+        AZ::IO::Path carPrefabFilepath = engineRootPath / carPrefabName;
+        AZ::IO::Path wheelPrefabFilepath = engineRootPath / wheelPrefabName;
+
+        AZ::EntityId tireEntityId = CreateEditorEntityUnderRoot(tireEntityName);
+        AZ::EntityId wheelContainerId = CreateEditorPrefab(wheelPrefabFilepath, { tireEntityId });
+        EntityAlias tireEntityAlias = FindEntityAliasInInstance(wheelContainerId, tireEntityName);
+
+        AZ::EntityId carContainerId = CreateEditorPrefab(carPrefabFilepath, { wheelContainerId });
+        InstanceAlias carInstanceAlias = FindNestedInstanceAliasInInstance(GetRootContainerEntityId(), carPrefabName);
+
+        // Detach the car prefab.
+        PrefabOperationResult result = m_prefabPublicInterface->DetachPrefabAndRemoveContainerEntity(carContainerId);
+        ASSERT_TRUE(result.IsSuccess());
+
+        PropagateAllTemplateChanges();
+
+        // Validate there is no car instance in the level prefab instance.
+        ValidateNestedInstanceNotUnderInstance(GetRootContainerEntityId(), carInstanceAlias);
+
+        // Validate there is wheel instance in the level prefab instance.
+        InstanceAlias wheelInstanceAlias = FindNestedInstanceAliasInInstance(GetRootContainerEntityId(), wheelPrefabName);
+        ValidateNestedInstanceUnderInstance(GetRootContainerEntityId(), wheelInstanceAlias);
+
+        InstanceOptionalReference levelInstance = m_instanceEntityMapperInterface->FindOwningInstance(GetRootContainerEntityId());
+        ASSERT_TRUE(levelInstance.has_value());
+        
+        AZStd::vector<InstanceOptionalReference> nestedInstances;
+        levelInstance->get().GetNestedInstances(
+            [&nestedInstances](AZStd::unique_ptr<Instance>& nestedInstance)
+            {
+                nestedInstances.push_back(*(nestedInstance.get()));
+            });
+
+        EXPECT_EQ(nestedInstances.size(), 1) << "There should be only one nested instance in level after detaching.";
+        EXPECT_TRUE(nestedInstances[0].has_value());
+
+        // Validate that the car prefab is gone
+        AZStd::string carEntityAliasAfterDetach = FindEntityAliasInInstance(GetRootContainerEntityId(), carPrefabName);
+        AZ::EntityId carEntityIdAfterDetach = levelInstance->get().GetEntityId(carEntityAliasAfterDetach);
+        EXPECT_FALSE(carEntityIdAfterDetach.IsValid());
+
+        // Validate that the wheel's parent entity is the level.
+        Instance& wheelInstanceAfterDetach = nestedInstances[0]->get();
+        AZ::EntityId wheelContainerIdAfterDetach = wheelInstanceAfterDetach.GetContainerEntityId();
+        EXPECT_TRUE(wheelContainerIdAfterDetach.IsValid());
+
+        AZ::EntityId parentEntityIdForWheel;
+        AZ::TransformBus::EventResult(parentEntityIdForWheel, wheelContainerIdAfterDetach, &AZ::TransformInterface::GetParentId);
+        EXPECT_EQ(levelInstance->get().GetContainerEntityId(), parentEntityIdForWheel);
+
+        // Validate that the tire's parent entity is the wheel.
+        tireEntityId = wheelInstanceAfterDetach.GetEntityId(tireEntityAlias);
+        EXPECT_TRUE(tireEntityId.IsValid());
+        AZ::EntityId parentEntityIdForTire;
+        AZ::TransformBus::EventResult(parentEntityIdForTire, tireEntityId, &AZ::TransformInterface::GetParentId);
+        EXPECT_EQ(wheelContainerIdAfterDetach, parentEntityIdForTire);
+    }
+
+    TEST_F(PrefabDetachPrefabTests, DetachPrefabAndRemoveContainerEntityWithNestedPrefabUnderTopLevelEntitySucceeds)
+    {
+        // Level
+        // | Car          (prefab)   <-- detach prefab
+        //   | Wheels                <-- top level entity
+        //     | Wheel    (prefab)
+        //       | Tire
+
+        // result (car is gone)
+        // Level
+        //   | Wheels                <-- top level entity
+        //     | Wheel    (prefab)
+        //       | Tire
+
+        const AZStd::string carPrefabName = "CarPrefab";
+        const AZStd::string wheelPrefabName = "WheelPrefab";
+
+        const AZStd::string wheelsEntityName = "Wheels";
+        const AZStd::string tireEntityName = "Tire";
+
+        AZ::IO::Path engineRootPath;
+        m_settingsRegistryInterface->Get(engineRootPath.Native(), AZ::SettingsRegistryMergeUtils::FilePathKey_EngineRootFolder);
+        AZ::IO::Path carPrefabFilepath = engineRootPath / carPrefabName;
+        AZ::IO::Path wheelPrefabFilepath = engineRootPath / wheelPrefabName;
+
+        // Create the wheels and tire entities.
+        AZ::EntityId wheelsEntityId = CreateEditorEntityUnderRoot(wheelsEntityName);
+        AZ::EntityId tireEntityId = CreateEditorEntity(tireEntityName, wheelsEntityId);
+
+        // Create the wheel prefab.
+        AZ::EntityId wheelContainerId = CreateEditorPrefab(wheelPrefabFilepath, { tireEntityId });
+
+        EntityAlias tireEntityAlias = FindEntityAliasInInstance(wheelContainerId, tireEntityName);
+
+        // Create the car prefab.
+        AZ::EntityId carContainerId = CreateEditorPrefab(carPrefabFilepath, { wheelsEntityId });
+        InstanceAlias carInstanceAlias = FindNestedInstanceAliasInInstance(GetRootContainerEntityId(), carPrefabName);
+
+        // Detach the car prefab.
+        PrefabOperationResult result = m_prefabPublicInterface->DetachPrefabAndRemoveContainerEntity(carContainerId);
+        ASSERT_TRUE(result.IsSuccess());
+
+        PropagateAllTemplateChanges();
+
+        // Validate there is no car instance in the level prefab instance.
+        ValidateNestedInstanceNotUnderInstance(GetRootContainerEntityId(), carInstanceAlias);
+
+        // Validate there is wheels entity in the level prefab instance.
+        EntityAlias wheelsEntityAlias = FindEntityAliasInInstance(GetRootContainerEntityId(), wheelsEntityName);
+        ValidateEntityUnderInstance(GetRootContainerEntityId(), wheelsEntityAlias, wheelsEntityName);
+
+        // Validate there is wheel instance in the level prefab instance.
+        InstanceAlias wheelInstanceAlias = FindNestedInstanceAliasInInstance(GetRootContainerEntityId(), wheelPrefabName);
+        ValidateNestedInstanceUnderInstance(GetRootContainerEntityId(), wheelInstanceAlias);
+
+        InstanceOptionalReference levelInstance = m_instanceEntityMapperInterface->FindOwningInstance(GetRootContainerEntityId());
+        ASSERT_TRUE(levelInstance.has_value());
+
+        AZStd::vector<InstanceOptionalReference> nestedInstances;
+        levelInstance->get().GetNestedInstances(
+            [&nestedInstances](AZStd::unique_ptr<Instance>& nestedInstance)
+            {
+                nestedInstances.push_back(*(nestedInstance.get()));
+            });
+
+        EXPECT_EQ(nestedInstances.size(), 1) << "There should be only one nested instance in level after detaching.";
+        EXPECT_TRUE(nestedInstances[0].has_value());
+
+        // Validate that the car is gone
+        AZStd::string carEntityAliasAfterDetach = FindEntityAliasInInstance(GetRootContainerEntityId(), carPrefabName);
+        AZ::EntityId carEntityIdAfterDetach = levelInstance->get().GetEntityId(carEntityAliasAfterDetach);
+        EXPECT_FALSE(carEntityIdAfterDetach.IsValid());
+
+        // Validate that the wheels' parent entity is the level instance.
+        AZ::EntityId wheelsEntityIdAfterDetach = levelInstance->get().GetEntityId(wheelsEntityAlias);
+        EXPECT_TRUE(wheelsEntityIdAfterDetach.IsValid());
+
+        AZ::EntityId parentEntityIdForWheels;
+        AZ::TransformBus::EventResult(parentEntityIdForWheels, wheelsEntityIdAfterDetach, &AZ::TransformInterface::GetParentId);
+        EXPECT_EQ(levelInstance->get().GetContainerEntityId(), parentEntityIdForWheels);
+
+        // Validate that the wheel prefab's container entity is the "wheels" entity.
+        Instance& wheelInstanceAfterDetach = nestedInstances[0]->get();
+        AZ::EntityId wheelContainerIdAfterDetach = wheelInstanceAfterDetach.GetContainerEntityId();
+        EXPECT_TRUE(wheelContainerIdAfterDetach.IsValid());
+
+        AZ::EntityId parentEntityIdForWheel;
+        AZ::TransformBus::EventResult(parentEntityIdForWheel, wheelContainerIdAfterDetach, &AZ::TransformInterface::GetParentId);
+        EXPECT_EQ(wheelsEntityIdAfterDetach, parentEntityIdForWheel);
+    }
+
+    TEST_F(PrefabDetachPrefabTests, DetachPrefabAndRemoveContainerEntityValidatesDetachedContainerEntityOrder)
+    {
+        // Validate the detached container entity's sort order in its parent.
+        // The detached container entity should not be moved to the beginning or end of the child entity list.
+        //
+        // Level
+        // | Station
+        // | Car       (prefab)  <-- detach prefab
+        //   | Tire
+        // | House
+
+        // result (car is gone)
+        // The detached container entity should not be moved to the beginning or end of the child entity list.
+        //
+        // Level
+        // | Station
+        // | Tire
+        // | House
+
+        const AZStd::string carPrefabName = "CarPrefab";
+
+        const AZStd::string tireEntityName = "Tire";
+        const AZStd::string stationEntityName = "Station";
+        const AZStd::string houseEntityName = "House";
+
+        AZ::IO::Path engineRootPath;
+        m_settingsRegistryInterface->Get(engineRootPath.Native(), AZ::SettingsRegistryMergeUtils::FilePathKey_EngineRootFolder);
+        AZ::IO::Path carPrefabFilepath = engineRootPath / carPrefabName;
+
+        CreateEditorEntityUnderRoot(stationEntityName);
+        AZ::EntityId tireEntityId = CreateEditorEntityUnderRoot(tireEntityName);
+        AZ::EntityId carContainerId = CreateEditorPrefab(carPrefabFilepath, { tireEntityId });
+        CreateEditorEntityUnderRoot(houseEntityName);
+
+        // Validate child entity order before detaching the car prefab.
+        AzToolsFramework::EntityOrderArray entityOrderArrayBeforeDetach =
+            AzToolsFramework::GetEntityChildOrder(GetRootContainerEntityId());
+        EXPECT_EQ(entityOrderArrayBeforeDetach.size(), 3);
+
+        AZStd::string childEntityName;
+        AZ::ComponentApplicationBus::BroadcastResult(
+            childEntityName, &AZ::ComponentApplicationRequests::GetEntityName, entityOrderArrayBeforeDetach[0]);
+        EXPECT_EQ(childEntityName, stationEntityName);
+        AZ::ComponentApplicationBus::BroadcastResult(
+            childEntityName, &AZ::ComponentApplicationRequests::GetEntityName, entityOrderArrayBeforeDetach[1]);
+        EXPECT_EQ(childEntityName, carPrefabName);
+        AZ::ComponentApplicationBus::BroadcastResult(
+            childEntityName, &AZ::ComponentApplicationRequests::GetEntityName, entityOrderArrayBeforeDetach[2]);
+        EXPECT_EQ(childEntityName, houseEntityName);
+
+        // Detach the car prefab.
+        PrefabOperationResult result = m_prefabPublicInterface->DetachPrefabAndRemoveContainerEntity(carContainerId);
+        ASSERT_TRUE(result.IsSuccess());
+
+        PropagateAllTemplateChanges();
+
+        // Validate child entity order after detaching the car prefab.
+        AzToolsFramework::EntityOrderArray entityOrderArrayAfterDetach =
+            AzToolsFramework::GetEntityChildOrder(GetRootContainerEntityId());
+        EXPECT_EQ(entityOrderArrayAfterDetach.size(), 3);
+
+        childEntityName = "";
+        AZ::ComponentApplicationBus::BroadcastResult(
+            childEntityName, &AZ::ComponentApplicationRequests::GetEntityName, entityOrderArrayAfterDetach[0]);
+        EXPECT_EQ(childEntityName, stationEntityName);
+        AZ::ComponentApplicationBus::BroadcastResult(
+            childEntityName, &AZ::ComponentApplicationRequests::GetEntityName, entityOrderArrayAfterDetach[1]);
+        EXPECT_EQ(childEntityName, tireEntityName);
+        AZ::ComponentApplicationBus::BroadcastResult(
+            childEntityName, &AZ::ComponentApplicationRequests::GetEntityName, entityOrderArrayAfterDetach[2]);
+        EXPECT_EQ(childEntityName, houseEntityName);
+    }
+
+    TEST_F(PrefabDetachPrefabTests, DetachPrefabAndRemoveContainerEntityValidatesDetachedChildEntityOrder)
+    {
+        // Validate the sort order of top-level child entities.
+        
+        // Level
+        //    Car (prefab)            child 0
+        //        Engine
+        //        Wheel (prefab)
+        //           Tire
+        //        Battery
+
+        // expected result (car is gone)
+        // Level
+        //     Engine                 child 0
+        //     Wheel (prefab)         child 1
+        //        Tire
+        //     Battery                child 2
+
+        const AZStd::string carPrefabName = "CarPrefab";
+        const AZStd::string wheelPrefabName = "WheelPrefab";
+
+        const AZStd::string tireEntityName = "Tire";
+        const AZStd::string engineEntityName = "Engine";
+        const AZStd::string batteryEntityName = "Battery";
+
+        AZ::IO::Path engineRootPath;
+        m_settingsRegistryInterface->Get(engineRootPath.Native(), AZ::SettingsRegistryMergeUtils::FilePathKey_EngineRootFolder);
+        AZ::IO::Path carPrefabFilepath = engineRootPath / carPrefabName;
+        AZ::IO::Path wheelPrefabFilepath = engineRootPath / wheelPrefabName;
+
+        AZ::EntityId engineEntityId = CreateEditorEntityUnderRoot(engineEntityName);
+        AZ::EntityId tireEntityId = CreateEditorEntityUnderRoot(tireEntityName);
+        AZ::EntityId wheelContainerId = CreateEditorPrefab(wheelPrefabFilepath, { tireEntityId });
+        AZ::EntityId batteryEntityId = CreateEditorEntityUnderRoot(batteryEntityName);
+        AZ::EntityId carContainerId = CreateEditorPrefab(carPrefabFilepath, { engineEntityId, wheelContainerId, batteryEntityId });
+
+        // Validate child entity order under car before detaching the car prefab.
+        AzToolsFramework::EntityOrderArray entityOrderArrayBeforeDetach = AzToolsFramework::GetEntityChildOrder(carContainerId);
+        EXPECT_EQ(entityOrderArrayBeforeDetach.size(), 3);
+
+        AZStd::string childEntityName;
+        AZ::ComponentApplicationBus::BroadcastResult(
+            childEntityName, &AZ::ComponentApplicationRequests::GetEntityName, entityOrderArrayBeforeDetach[0]);
+        EXPECT_EQ(childEntityName, engineEntityName);
+        AZ::ComponentApplicationBus::BroadcastResult(
+            childEntityName, &AZ::ComponentApplicationRequests::GetEntityName, entityOrderArrayBeforeDetach[1]);
+        EXPECT_EQ(childEntityName, wheelPrefabName);
+        AZ::ComponentApplicationBus::BroadcastResult(
+            childEntityName, &AZ::ComponentApplicationRequests::GetEntityName, entityOrderArrayBeforeDetach[2]);
+        EXPECT_EQ(childEntityName, batteryEntityName);
+
+        InstanceOptionalReference levelInstance = m_instanceEntityMapperInterface->FindOwningInstance(GetRootContainerEntityId());
+        ASSERT_TRUE(levelInstance.has_value());
+
+        // before we detach, the level should contain 1 entity (the container entity) and 1 instance (of the car)
+        int count = 0;
+        levelInstance->get().GetEntityIds([&count](AZ::EntityId entityId) { count++; return true; });
+        EXPECT_EQ(count, 1); // expect 1 real entity, that is, the container for the car.
+
+        count = 0;
+        levelInstance->get().GetNestedInstances([&count](AZStd::unique_ptr<Instance>& instance) { count++; });
+        EXPECT_EQ(count, 1); // expect 1 instance of another prefab (The the car)
+
+        // Detach the car prefab.
+        PrefabOperationResult result = m_prefabPublicInterface->DetachPrefabAndRemoveContainerEntity(carContainerId);
+        ASSERT_TRUE(result.IsSuccess());
+
+        PropagateAllTemplateChanges();
+
+        // after we detach, the level should contain 3 entities (2 of them real entities, one of them an instance container)
+        count = 0;
+        levelInstance->get().GetEntityIds([&count](const AZ::EntityId& entityId) { count++; return true; });
+        EXPECT_EQ(count, 3); // expect 2 REAL entities.
+
+        count = 0;
+        levelInstance->get().GetNestedInstances([&count](AZStd::unique_ptr<Instance>& instance) { count++; });
+        EXPECT_EQ(count, 1); // expect 1 instance of another prefab (the wheel)
+
+        // Validate child entity order under the level after detaching the car prefab.  Should be engine, wheel, battery.
+        AzToolsFramework::EntityOrderArray entityOrderArrayAfterDetach =
+            AzToolsFramework::GetEntityChildOrder(levelInstance->get().GetContainerEntityId());
+        EXPECT_EQ(entityOrderArrayAfterDetach.size(), 3);
+
+        childEntityName = "";
+        AZ::ComponentApplicationBus::BroadcastResult(childEntityName, &AZ::ComponentApplicationRequests::GetEntityName, entityOrderArrayAfterDetach[0]);
+        EXPECT_EQ(childEntityName, engineEntityName);
+        AZ::ComponentApplicationBus::BroadcastResult(childEntityName, &AZ::ComponentApplicationRequests::GetEntityName, entityOrderArrayAfterDetach[1]);
+        EXPECT_EQ(childEntityName, wheelPrefabName);
+        AZ::ComponentApplicationBus::BroadcastResult(childEntityName, &AZ::ComponentApplicationRequests::GetEntityName, entityOrderArrayAfterDetach[2]);
+        EXPECT_EQ(childEntityName, batteryEntityName);
+    }
+
+    TEST_F(PrefabDetachPrefabTests, DetachPrefabAndRemoveContainerEntityValidatesTopLevelChildEntityOrder)
+    {
+        // Validate the sort order of child entities and prefabs that are under the top level entity.
+        // 
+        // Level
+        // | Car          (prefab)   <-- detach prefab
+        //   | Wheels                <-- top level entity
+        //     | Red_Wheel
+        //     | Wheel    (prefab)
+        //       | Tire
+        //     | Black_Wheel
+        // 
+        // expected result (car is gone)
+        // Level
+        // | Wheels                <-- top level entity
+        //   | Red_Wheel
+        //   | Wheel    (prefab)
+        //     | Tire
+        //   | Black_Wheel
+
+        const AZStd::string carPrefabName = "CarPrefab";
+        const AZStd::string wheelPrefabName = "WheelPrefab";
+
+        const AZStd::string wheelsEntityName = "Wheels";
+        const AZStd::string redWheelEntityName = "Red_Wheel";
+        const AZStd::string blackWheelEntityName = "Black_Wheel";
+        const AZStd::string tireEntityName = "Tire";
+
+        AZ::IO::Path engineRootPath;
+        m_settingsRegistryInterface->Get(engineRootPath.Native(), AZ::SettingsRegistryMergeUtils::FilePathKey_EngineRootFolder);
+        AZ::IO::Path carPrefabFilepath = engineRootPath / carPrefabName;
+        AZ::IO::Path wheelPrefabFilepath = engineRootPath / wheelPrefabName;
+
+        // Create the wheels, red wheel and tire entities.
+        AZ::EntityId wheelsEntityId = CreateEditorEntityUnderRoot(wheelsEntityName);
+        CreateEditorEntity(redWheelEntityName, wheelsEntityId);
+        AZ::EntityId tireEntityId = CreateEditorEntity(tireEntityName, wheelsEntityId);
+
+        // Create the wheel prefab.
+        CreateEditorPrefab(wheelPrefabFilepath, { tireEntityId });
+
+        // Create the black wheel entity.
+        CreateEditorEntity(blackWheelEntityName, wheelsEntityId);
+
+        // Create the car prefab.
+        AZ::EntityId carContainerId = CreateEditorPrefab(carPrefabFilepath, { wheelsEntityId });
+
+        InstanceOptionalReference levelInstance = m_instanceEntityMapperInterface->FindOwningInstance(GetRootContainerEntityId());
+        ASSERT_TRUE(levelInstance.has_value());
+
+        // Validate child entity order under wheels before detaching the car prefab.
+        EntityAlias wheelsEntityAlias = FindEntityAliasInInstance(carContainerId, wheelsEntityName);
+        InstanceOptionalReference carInstance = m_instanceEntityMapperInterface->FindOwningInstance(carContainerId);
+        EXPECT_TRUE(carInstance.has_value());
+        wheelsEntityId = carInstance->get().GetEntityId(wheelsEntityAlias);
+
+        AzToolsFramework::EntityOrderArray entityOrderArrayBeforeDetach = AzToolsFramework::GetEntityChildOrder(wheelsEntityId);
+        EXPECT_EQ(entityOrderArrayBeforeDetach.size(), 3);
+
+        AZStd::string childEntityName;
+        AZ::ComponentApplicationBus::BroadcastResult(
+            childEntityName, &AZ::ComponentApplicationRequests::GetEntityName, entityOrderArrayBeforeDetach[0]);
+        EXPECT_EQ(childEntityName, redWheelEntityName);
+        AZ::ComponentApplicationBus::BroadcastResult(
+            childEntityName, &AZ::ComponentApplicationRequests::GetEntityName, entityOrderArrayBeforeDetach[1]);
+        EXPECT_EQ(childEntityName, wheelPrefabName);
+        AZ::ComponentApplicationBus::BroadcastResult(
+            childEntityName, &AZ::ComponentApplicationRequests::GetEntityName, entityOrderArrayBeforeDetach[2]);
+        EXPECT_EQ(childEntityName, blackWheelEntityName);
+
+        // Detach the car prefab.
+        PrefabOperationResult result = m_prefabPublicInterface->DetachPrefabAndRemoveContainerEntity(carContainerId);
+        ASSERT_TRUE(result.IsSuccess());
+
+        PropagateAllTemplateChanges();
+
+        // Validate child entity order under wheels after detaching the car prefab.
+        wheelsEntityAlias = FindEntityAliasInInstance(levelInstance->get().GetContainerEntityId(), wheelsEntityName);
+        wheelsEntityId = levelInstance->get().GetEntityId(wheelsEntityAlias);
+
+        AzToolsFramework::EntityOrderArray entityOrderArrayAfterDetach = AzToolsFramework::GetEntityChildOrder(wheelsEntityId);
+        EXPECT_EQ(entityOrderArrayAfterDetach.size(), 3);
+
+        AZ::ComponentApplicationBus::BroadcastResult(
+            childEntityName, &AZ::ComponentApplicationRequests::GetEntityName, entityOrderArrayAfterDetach[0]);
+        EXPECT_EQ(childEntityName, redWheelEntityName);
+        AZ::ComponentApplicationBus::BroadcastResult(
+            childEntityName, &AZ::ComponentApplicationRequests::GetEntityName, entityOrderArrayAfterDetach[1]);
+        EXPECT_EQ(childEntityName, wheelPrefabName);
+        AZ::ComponentApplicationBus::BroadcastResult(
+            childEntityName, &AZ::ComponentApplicationRequests::GetEntityName, entityOrderArrayAfterDetach[2]);
+        EXPECT_EQ(childEntityName, blackWheelEntityName);
+    }
+} // namespace UnitTest

+ 1 - 0
Code/Framework/AzToolsFramework/Tests/aztoolsframeworktests_files.cmake

@@ -110,6 +110,7 @@ set(FILES
     Prefab/PrefabDeleteTests.cpp
     Prefab/PrefabDeleteAsOverrideTests.cpp
     Prefab/PrefabDetachPrefabTests.cpp
+    Prefab/PrefabDetachPrefabAndRemoveContainerTests.cpp
     Prefab/PrefabDuplicateTests.cpp
     Prefab/PrefabEntityAliasTests.cpp
     Prefab/PrefabEditorEntityNotificationTests.cpp