Added ability to convert multiply-nested slices (#1239)

* Addressed feedback from previous PR
* Change missing entity aliases to be deterministic.
When converting slices, this helps produce the same results on multiple reconversions of the same data.
* Exposed the asset filter callback.
This allows the slice converter to specifically load nested slices as opposed to not loading *any* referenced assets.
* Added support for multiply-nested slice instance conversion.

Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Instance/InstanceEntityIdMapper.cpp

@@ -154,7 +154,7 @@ namespace AzToolsFramework
             InstanceOptionalReference owningInstanceReference = m_storingInstance->m_instanceEntityMapper->FindOwningInstance(entityId);
 
             // Start with an empty alias to build out our reference path
-            // If we can't resolve this id we'll return a random new alias instead of a reference path
+            // If we can't resolve this id we'll return a new alias based on the entity ID instead of a reference path
             AliasPath relativeEntityAliasPath;
             if (!owningInstanceReference)
             {
@@ -162,7 +162,7 @@ namespace AzToolsFramework
                     "Prefab - EntityIdMapper: Entity with Id %s has no registered owning instance",
                     entityId.ToString().c_str());
 
-                return Instance::GenerateEntityAlias();
+                return AZStd::string::format("Entity_%s", entityId.ToString().c_str());
             }
 
             Instance* owningInstance = &(owningInstanceReference->get());

Code/Tools/SerializeContextTools/Application.cpp

@@ -35,7 +35,7 @@ namespace AZ
         Application::Application(int argc, char** argv)
             : AzToolsFramework::ToolsApplication(&argc, &argv)
         {
-            // We need a specialized variant of EditorEntityContextCompnent for the SliceConverter, so we register the descriptor here.
+            // We need a specialized variant of EditorEntityContextComponent for the SliceConverter, so we register the descriptor here.
             RegisterComponentDescriptor(AzToolsFramework::SliceConverterEditorEntityContextComponent::CreateDescriptor());
 
             AZ::IO::FixedMaxPath projectPath = AZ::Utils::GetProjectPath();

Code/Tools/SerializeContextTools/SliceConverter.cpp

@@ -128,8 +128,7 @@ namespace AZ
             return result;
         }
 
-        bool SliceConverter::ConvertSliceFile(
+        bool SliceConverter::ConvertSliceFile(AZ::SerializeContext* serializeContext, const AZStd::string& slicePath, bool isDryRun)
-            AZ::SerializeContext* serializeContext, const AZStd::string& slicePath, bool isDryRun)
         {
             /* To convert a slice file, we read the input file in via ObjectStream, then use the "class ready" callback to convert
             * the data in memory to a Prefab.
@@ -177,11 +176,22 @@ namespace AZ
                 }
 
                 AZ::Entity* rootEntity = reinterpret_cast<AZ::Entity*>(classPtr);
-                return ConvertSliceToPrefab(context, outputPath, isDryRun, rootEntity);
+                bool convertResult = ConvertSliceToPrefab(context, outputPath, isDryRun, rootEntity);
+                // Clear out the references to any nested slices so that the nested assets get unloaded correctly at the end of
+                // the conversion.  
+                ClearSliceAssetReferences(rootEntity);
+                return convertResult;
             };
 
             // Read in the slice file and call the callback on completion to convert the read-in slice to a prefab.
-            if (!Utilities::InspectSerializedFile(inputPath.c_str(), serializeContext, callback))
+            // This will also load dependent slice assets, but no other dependent asset types.
+            // Since we're not actually initializing any of the entities, we don't need any of the non-slice assets to be loaded.
+            if (!Utilities::InspectSerializedFile(
+                    inputPath.c_str(), serializeContext, callback,
+                    [](const AZ::Data::AssetFilterInfo& filterInfo)
+                    {
+                        return (filterInfo.m_assetType == azrtti_typeid<AZ::SliceAsset>());
+                    }))
             {
                 AZ_Warning("Convert-Slice", false, "Failed to load '%s'. File may not contain an object stream.", inputPath.c_str());
                 result = false;
@@ -256,7 +266,7 @@ namespace AZ
             for (auto& alias : entityAliases)
             {
                 auto id = sourceInstance->GetEntityId(alias);
-                auto result = m_aliasIdMapper.emplace(TemplateEntityIdPair(templateId, id), alias);
+                auto result = m_aliasIdMapper.emplace(id, SliceEntityMappingInfo(templateId, alias));
                 if (!result.second)
                 {
                     AZ_Printf("Convert-Slice", "  Duplicate entity alias -> entity id entries found, conversion may not be successful.\n");
@@ -342,10 +352,9 @@ namespace AZ
             // For each nested slice, convert it.
             for (auto& slice : sliceList)
             {
-                // Get the nested slice asset
+                // Get the nested slice asset.  These should already be preloaded due to loading the root asset.
                 auto sliceAsset = slice.GetSliceAsset();
-                sliceAsset.QueueLoad();
+                AZ_Assert(sliceAsset.IsReady(), "slice asset hasn't been loaded yet!");
-                sliceAsset.BlockUntilLoadComplete();
 
                 // The slice list gives us asset IDs, and we need to get to the source path.  So first we get the asset path from the ID,
                 // then we get the source path from the asset path.
@@ -429,6 +438,28 @@ namespace AZ
             auto instanceToTemplateInterface = AZ::Interface<AzToolsFramework::Prefab::InstanceToTemplateInterface>::Get();
             auto prefabSystemComponentInterface = AZ::Interface<AzToolsFramework::Prefab::PrefabSystemComponentInterface>::Get();
 
+            // When creating the new instance, we would like to have deterministic instance aliases.  Prefabs that depend on this one
+            // will have patches that reference the alias, so if we reconvert this slice a second time, we would like it to produce
+            // the same results.  To get a deterministic and unique alias, we rely on the slice instance.  The slice instance contains
+            // a map of slice entity IDs to unique instance entity IDs.  We'll just consistently use the first entry in the map as the
+            // unique instance ID.
+            AZStd::string instanceAlias;
+            auto entityIdMap = instance.GetEntityIdMap();
+            if (!entityIdMap.empty())
+            {
+                instanceAlias = AZStd::string::format("Instance_%s", entityIdMap.begin()->second.ToString().c_str());
+            }
+            else
+            {
+                instanceAlias = AZStd::string::format("Instance_%s", AZ::Entity::MakeId().ToString().c_str());
+            }
+
+            // Before processing any further, save off all the known entity IDs from this instance and how they map back to the base
+            // nested prefab that they've come from (i.e. this one).  As we proceed up the chain of nesting, this will build out a
+            // hierarchical list of owning instances for each entity that we can trace upwards to know where to add the entity into
+            // our nested prefab instance.
+            UpdateSliceEntityInstanceMappings(instance.GetEntityIdToBaseMap(), instanceAlias);
+
             // Create a new unmodified prefab Instance for the nested slice instance.
             auto nestedInstance = AZStd::make_unique<AzToolsFramework::Prefab::Instance>();
             AzToolsFramework::Prefab::Instance::EntityList newEntities;
@@ -465,15 +496,62 @@ namespace AZ
             auto instantiated =
                 dataPatch.Apply(&sourceObjects, dependentSlice->GetSerializeContext(), filterDesc, sourceDataFlags, targetDataFlags);
 
-            // Run through all the instantiated entities and fix up their parent hierarchy:
+            // Replace all the entities in the instance with the new patched ones.  To do this, we'll remove all existing entities
-            // - Invalid parents need to get set to the container.
+            // throughout the entire nested hierarchy, then add the new patched entities back in at the appropriate place in the hierarchy.
-            // - Valid parents into the top-level instance mean that the nested slice instance is also child-nested under an entity.
+            // (This is easier than trying to figure out what the patched data changes are - we can let the JSON patch handle it for us)
-            //   Prefabs handle this type of nesting differently - we need to set the parent to the container, and the container's
+
-            //   parent to that other instance.
+            nestedInstance->RemoveNestedEntities(
-            auto containerEntity = nestedInstance->GetContainerEntity();
+                [](const AZStd::unique_ptr<AZ::Entity>&)
-            auto containerEntityId = containerEntity->get().GetId();
+                {
-            for (auto entity : instantiated->m_entities)
+                    return true;
+                });
+
+            AZStd::vector<AZStd::pair<AZ::Entity*, AzToolsFramework::Prefab::Instance*>> addedEntityList;
+
+            for (auto& entity : instantiated->m_entities)
             {
+                auto entityEntry = m_aliasIdMapper.find(entity->GetId());
+                if (entityEntry != m_aliasIdMapper.end())
+                {
+                    auto& mappingStruct = entityEntry->second;
+
+                    // Starting with the current nested instance, walk downwards through the nesting hierarchy until we're at the
+                    // correct level for this instanced entity ID, then add it.  Because we're adding it with the non-instanced alias,
+                    // it doesn't matter what the slice's instanced entity ID is, and the JSON patch will correctly pick up the changes
+                    // we've made for this instance.
+                    AzToolsFramework::Prefab::Instance* addingInstance = nestedInstance.get();
+                    for (auto it = mappingStruct.m_nestedInstanceAliases.rbegin(); it != mappingStruct.m_nestedInstanceAliases.rend(); it++)
+                    {
+                        auto foundInstance = addingInstance->FindNestedInstance(*it);
+                        if (foundInstance.has_value())
+                        {
+                            addingInstance = &(foundInstance->get());
+                        }
+                        else
+                        {
+                            AZ_Assert(false, "Couldn't find nested instance %s", it->c_str());
+                        }
+                    }
+                    addingInstance->AddEntity(*entity, mappingStruct.m_entityAlias);
+                    addedEntityList.emplace_back(entity, addingInstance);
+                }
+                else
+                {
+                    AZ_Assert(false, "Failed to find entity alias.");
+                    nestedInstance->AddEntity(*entity);
+                    addedEntityList.emplace_back(entity, nestedInstance.get());
+                }
+            }
+
+            for (auto& [entity, addingInstance] : addedEntityList)
+            {
+                // Fix up the parent hierarchy:
+                // - Invalid parents need to get set to the container.
+                // - Valid parents into the top-level instance mean that the nested slice instance is also child-nested under an entity.
+                //   Prefabs handle this type of nesting differently - we need to set the parent to the container, and the container's
+                //   parent to that other instance.
+                auto containerEntity = addingInstance->GetContainerEntity();
+                auto containerEntityId = containerEntity->get().GetId();
                 AzToolsFramework::Components::TransformComponent* transformComponent =
                     entity->FindComponent<AzToolsFramework::Components::TransformComponent>();
                 if (transformComponent)
@@ -482,45 +560,61 @@ namespace AZ
                     auto parentId = transformComponent->GetParentId();
                     if (parentId.IsValid())
                     {
-                        auto parentAlias = m_aliasIdMapper.find(TemplateEntityIdPair(topLevelInstance->GetTemplateId(), parentId));
+                        // Look to see if the parent ID exists in the same instance (i.e. an entity in the nested slice is a
-                        if (parentAlias != m_aliasIdMapper.end())
+                        // child of an entity in the containing slice).  If this case exists, we need to adjust the parents so that
+                        // the child entity connects to the prefab container, and the *container* is the child of the entity in the
+                        // containing slice.  (i.e. go from A->B to A->container->B)
+                        auto parentEntry = m_aliasIdMapper.find(parentId);
+                        if (parentEntry != m_aliasIdMapper.end())
                         {
-                            // Set the container's parent to this entity's parent, and set this entity's parent to the container
+                            auto& parentMappingInfo = parentEntry->second;
-                            // (i.e. go from A->B to A->container->B)
+                            if (parentMappingInfo.m_templateId != addingInstance->GetTemplateId())
-                            auto newParentId = topLevelInstance->GetEntityId(parentAlias->second);
+                            {
-                            SetParentEntity(containerEntity->get(), newParentId, false);
+                                if (topLevelInstance->GetTemplateId() == parentMappingInfo.m_templateId)
-                            onlySetIfInvalid = false;
+                                {
+                                    parentId = topLevelInstance->GetEntityId(parentMappingInfo.m_entityAlias);
+                                }
+                                else
+                                {
+                                    AzToolsFramework::Prefab::Instance* parentInstance = addingInstance;
+
+                                    while ((parentInstance->GetParentInstance().has_value()) &&
+                                           (parentInstance->GetTemplateId() != parentMappingInfo.m_templateId))
+                                    {
+                                        parentInstance = &(parentInstance->GetParentInstance()->get());
+                                    }
+
+                                    if (parentInstance->GetTemplateId() == parentMappingInfo.m_templateId)
+                                    {
+                                        parentId = parentInstance->GetEntityId(parentMappingInfo.m_entityAlias);
+                                    }
+                                    else
+                                    {
+                                        AZ_Assert(false, "Could not find parent instance");
+                                    }
+                                }
+
+                                // Set the container's parent to this entity's parent, and set this entity's parent to the container
+                                // auto newParentId = topLevelInstance->GetEntityId(parentMappingInfo.m_entityAlias);
+                                SetParentEntity(containerEntity->get(), parentId, false);
+                                onlySetIfInvalid = false;
+                            }
                         }
+
+                        // If the parent ID is valid, but NOT in the top-level instance, then it's just a nested hierarchy inside
+                        // the slice and we don't need to adjust anything.  "onlySetIfInvalid" will still be true, which means we
+                        // won't change the parent ID below.
                     }
 
-                    SetParentEntity(*entity, containerEntityId, onlySetIfInvalid);                    
+                    SetParentEntity(*entity, containerEntityId, onlySetIfInvalid);
                 }
             }
 
-            // Replace all the entities in the instance with the new patched ones.
-            // (This is easier than trying to figure out what the patched data changes are - we can let the JSON patch handle it for us)
-            nestedInstance->RemoveNestedEntities(
-                [](const AZStd::unique_ptr<AZ::Entity>&)
-                {
-                    return true;
-                });
-            for (auto& entity : instantiated->m_entities)
-            {
-                auto entityAlias = m_aliasIdMapper.find(TemplateEntityIdPair(nestedInstance->GetTemplateId(), entity->GetId()));
-                if (entityAlias != m_aliasIdMapper.end())
-                {
-                    nestedInstance->AddEntity(*entity, entityAlias->second);
-                }
-                else
-                {
-                    AZ_Assert(false, "Failed to find entity alias.");
-                    nestedInstance->AddEntity(*entity);
-                }
-            }
 
             // Set the container entity of the nested prefab to have the top-level prefab as the parent if it hasn't already gotten
             // another entity as its parent.
             {
+                auto containerEntity = nestedInstance->GetContainerEntity();
                 constexpr bool onlySetIfInvalid = true;
                 SetParentEntity(containerEntity->get(), topLevelInstance->GetContainerEntityId(), onlySetIfInvalid);
             }
@@ -531,21 +625,7 @@ namespace AZ
             AzToolsFramework::Prefab::PrefabDom topLevelInstanceDomBefore;
             instanceToTemplateInterface->GenerateDomForInstance(topLevelInstanceDomBefore, *topLevelInstance);
 
-            // When creating the new instance, we would like to have deterministic instance aliases.  Prefabs that depend on this one
+            // Use the deterministic instance alias for this new instance
-            // will have patches that reference the alias, so if we reconvert this slice a second time, we would like it to produce
-            // the same results.  To get a deterministic and unique alias, we rely on the slice instance.  The slice instance contains
-            // a map of slice entity IDs to unique instance entity IDs.  We'll just consistently use the first entry in the map as the
-            // unique instance ID.
-            AZStd::string instanceAlias;
-            auto entityIdMap = instance.GetEntityIdMap();
-            if (!entityIdMap.empty())
-            {
-                instanceAlias = AZStd::string::format("Instance_%s", entityIdMap.begin()->second.ToString().c_str());
-            }
-            else
-            {
-                instanceAlias = AZStd::string::format("Instance_%s", AZ::Entity::MakeId().ToString().c_str());
-            }
             AzToolsFramework::Prefab::Instance& addedInstance = topLevelInstance->AddInstance(AZStd::move(nestedInstance), instanceAlias);
 
             AzToolsFramework::Prefab::PrefabDom topLevelInstanceDomAfter;
@@ -670,5 +750,48 @@ namespace AZ
             AZ_Error("Convert-Slice", disconnected, "Asset Processor failed to disconnect successfully.");
         }
 
+        void SliceConverter::ClearSliceAssetReferences(AZ::Entity* rootEntity)
+        {
+            SliceComponent* sliceComponent = AZ::EntityUtils::FindFirstDerivedComponent<SliceComponent>(rootEntity);
+            // Make a copy of the slice list and remove all of them from the loaded component.
+            AZ::SliceComponent::SliceList slices = sliceComponent->GetSlices();
+            for (auto& slice : slices)
+            {
+                sliceComponent->RemoveSlice(&slice);
+            }
+        }
+
+        void SliceConverter::UpdateSliceEntityInstanceMappings(
+            const AZ::SliceComponent::EntityIdToEntityIdMap& sliceEntityIdMap, const AZStd::string& currentInstanceAlias)
+        {
+            // For each instanced entity, map its ID all the way back to the original prefab template and entity ID that it came from.
+            // This counts on being run recursively from the leaf nodes upwards, so we first get B->A,
+            // then C->B which becomes a C->A entry, then D->C which becomes D->A, etc.
+            for (auto& [newId, oldId] : sliceEntityIdMap)
+            {
+                // Try to find the conversion chain from the old ID.  if it's there, copy it and use it for the new ID, plus add this
+                // instance's name to the end of the chain.  If it's not there, skip it, since it's probably the slice metadata entity,
+                // which we didn't convert.
+                auto parentEntry = m_aliasIdMapper.find(oldId);
+                if (parentEntry != m_aliasIdMapper.end())
+                {
+                    // Only add this instance's name if we don't already have an entry for the new ID.
+                    if (m_aliasIdMapper.find(newId) == m_aliasIdMapper.end())
+                    {
+                        auto newMappingEntry = m_aliasIdMapper.emplace(newId, parentEntry->second).first;
+                        newMappingEntry->second.m_nestedInstanceAliases.emplace_back(currentInstanceAlias);
+                    }
+                    else
+                    {
+                        // If we already had an entry for the new ID, it might be because the old and new ID are the same.  This happens
+                        // when nesting multiple prefabs directly underneath each other without a nesting entity in-between.
+                        // If the IDs are different, it's an unexpected error condition.
+                        AZ_Assert(oldId == newId, "The same entity instance ID has unexpectedly appeared twice in the same nested prefab.");
+                    }
+                }
+            }
+        }
+
+
     } // namespace SerializeContextTools
 } // namespace AZ

Code/Tools/SerializeContextTools/SliceConverter.h

@@ -42,8 +42,6 @@ namespace AZ
             bool ConvertSliceFiles(Application& application);
 
         private:
-            using TemplateEntityIdPair = AZStd::pair<AzToolsFramework::Prefab::TemplateId, AZ::EntityId>;
-
             bool ConnectToAssetProcessor();
             void DisconnectFromAssetProcessor();
 
@@ -60,10 +58,32 @@ namespace AZ
             void SetParentEntity(const AZ::Entity& entity, const AZ::EntityId& parentId, bool onlySetIfInvalid);
             void PrintPrefab(AzToolsFramework::Prefab::TemplateId templateId);
             bool SavePrefab(AZ::IO::PathView outputPath, AzToolsFramework::Prefab::TemplateId templateId);
+            void ClearSliceAssetReferences(AZ::Entity* rootEntity);
+            void UpdateSliceEntityInstanceMappings(
+                const AZ::SliceComponent::EntityIdToEntityIdMap& sliceEntityIdMap,
+                const AZStd::string& currentInstanceAlias);
 
-            // Track all of the entity IDs created and the prefab entity aliases that map to them.  This mapping is used
+            // When converting slice entities, especially for nested slices, we need to keep track of the original
-            // with nested slice conversion to remap parent entity IDs to the correct prefab entity IDs.
+            // entity ID, the entity alias it uses in the prefab, and which template and nested instance path it maps to.
-            AZStd::unordered_map<TemplateEntityIdPair, AzToolsFramework::Prefab::EntityAlias> m_aliasIdMapper;
+            // As we encounter each instanced entity ID, we can look it up in this structure and use this to determine how to properly
+            // add it to the correct place in the hierarchy.
+            struct SliceEntityMappingInfo
+            {
+                SliceEntityMappingInfo(AzToolsFramework::Prefab::TemplateId templateId, AzToolsFramework::Prefab::EntityAlias entityAlias)
+                    : m_templateId(templateId)
+                    , m_entityAlias(entityAlias)
+                {
+                }
+
+                AzToolsFramework::Prefab::TemplateId m_templateId;
+                AzToolsFramework::Prefab::EntityAlias m_entityAlias;
+                AZStd::vector<AzToolsFramework::Prefab::InstanceAlias> m_nestedInstanceAliases;
+            };
+
+            // Track all of the entity IDs created and associate them with enough conversion information to know how to place the
+            // entities in the correct place in the prefab hierarchy and fix up parent entity ID mappings to work with the nested
+            // prefab schema.
+            AZStd::unordered_map<AZ::EntityId, SliceEntityMappingInfo> m_aliasIdMapper;
 
             // Track all of the created prefab template IDs on a slice conversion so that they can get removed at the end of the
             // conversion for that file.

Code/Tools/SerializeContextTools/SliceConverterEditorEntityContextComponent.h

@@ -36,7 +36,7 @@ namespace AzToolsFramework
         SliceConverterEditorEntityContextComponent() : EditorEntityContextComponent() {}
 
         // Simple API to selectively disable this logic *only* when performing slice to prefab conversion.
-        static void DisableOnContextEntityLogic()
+        static inline void DisableOnContextEntityLogic()
         {
             m_enableOnContextEntityLogic = false;
         }

Code/Tools/SerializeContextTools/Utilities.cpp

@@ -209,7 +209,11 @@ namespace AZ::SerializeContextTools
         return result;
     }
 
-    bool Utilities::InspectSerializedFile(const char* filePath, SerializeContext* sc, const ObjectStream::ClassReadyCB& classCallback)
+    bool Utilities::InspectSerializedFile(
+        const char* filePath,
+        SerializeContext* sc,
+        const ObjectStream::ClassReadyCB& classCallback,
+        Data::AssetFilterCB assetFilterCallback)
     {
         if (!AZ::IO::FileIOBase::GetInstance()->Exists(filePath))
         {
@@ -248,9 +252,9 @@ namespace AZ::SerializeContextTools
         AZ::IO::MemoryStream stream(data.data(), fileLength);
 
         ObjectStream::FilterDescriptor filter;
-        // Never load dependencies. That's another file that would need to be processed
+        // By default, never load dependencies. That's another file that would need to be processed
         // separately from this one.
-        filter.m_assetCB = AZ::Data::AssetFilterNoAssetLoading;
+        filter.m_assetCB = assetFilterCallback;
         if (!ObjectStream::LoadBlocking(&stream, *sc, classCallback, filter))
         {
             AZ_Printf("Verify", "Failed to deserialize '%s'\n", filePath);

Code/Tools/SerializeContextTools/Utilities.h

@@ -39,7 +39,11 @@ namespace AZ
 
             static AZStd::vector<AZ::Uuid> GetSystemComponents(const Application& application);
 
-            static bool InspectSerializedFile(const char* filePath, SerializeContext* sc, const ObjectStream::ClassReadyCB& classCallback);
+            static bool InspectSerializedFile(
+                const char* filePath,
+                SerializeContext* sc,
+                const ObjectStream::ClassReadyCB& classCallback,
+                Data::AssetFilterCB assetFilterCallback = AZ::Data::AssetFilterNoAssetLoading);
 
         private:
             Utilities() = delete;