From e27a666bacd9f3b0946981d5069d1a1838f7ee4c Mon Sep 17 00:00:00 2001 From: AMZN-koppersr <82230785+AMZN-koppersr@users.noreply.github.com> Date: Tue, 25 Jan 2022 10:11:38 -0800 Subject: [PATCH 1/5] Fixed missing assets when entering Play-In-Editor. At the root the problem is that some editor-only components store information to construct an asset for the runtime component but not the asset itself. This behavior caused the assets to not be correctly detected in two places. The first place was due to the recent move to PrefDocument to avoid repeated (de)serialization of the PrefabDOM when converting to spawnables. Due to the caching the change from editor-only component to runtime component didn't record the new asset. This has been fixed by allowing assets to be collected on store as well and to check the cache validity when retrieving the list of referenced assets. The second problem was with loading assets from the in-memory spawnable that's created for Play-In-Editor. Because the newly created assets wouldn't be loaded they need to be explicitly loaded. The original code used the collected list of assets from the PrefabDocument and checked if they were loaded, depending on hot-reloading to trigger a reload on the actual asset. This turned out to not be universally applicable, so instead the Serialize Context is now used to find all the assets that aren't loaded yet and queues a load. This is a bit more expensive to do, but to offset this cost checks are done to only do any operations on assets that haven't been loaded yet which reduces the number of calls to the Asset Manager. Signed-off-by: AMZN-koppersr <82230785+AMZN-koppersr@users.noreply.github.com> --- .../AzCore/Asset/AssetJsonSerializer.cpp | 6 + .../Prefab/PrefabDomUtils.cpp | 54 ++++++++ .../AzToolsFramework/Prefab/PrefabDomUtils.h | 24 +++- .../InMemorySpawnableAssetContainer.cpp | 119 +++++++++++------- .../InMemorySpawnableAssetContainer.h | 5 +- .../Prefab/Spawnable/PrefabDocument.cpp | 34 +++-- .../Prefab/Spawnable/PrefabDocument.h | 4 +- 7 files changed, 181 insertions(+), 65 deletions(-) diff --git a/Code/Framework/AzCore/AzCore/Asset/AssetJsonSerializer.cpp b/Code/Framework/AzCore/AzCore/Asset/AssetJsonSerializer.cpp index 0d28036646..838caf467a 100644 --- a/Code/Framework/AzCore/AzCore/Asset/AssetJsonSerializer.cpp +++ b/Code/Framework/AzCore/AzCore/Asset/AssetJsonSerializer.cpp @@ -92,6 +92,12 @@ namespace AZ::Data result.Combine(resultHint); } + if (SerializedAssetTracker* assetTracker = context.GetMetadata().Find(); + assetTracker != nullptr && result.GetProcessing() == JSR::Processing::Completed) + { + assetTracker->AddAsset(*instance); + } + return context.Report(result, result.GetProcessing() == JSR::Processing::Completed ? "Successfully stored Asset." : "Failed to store Asset."); } diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabDomUtils.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabDomUtils.cpp index 50c61e6877..3be3fbce11 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabDomUtils.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabDomUtils.cpp @@ -119,6 +119,60 @@ namespace AzToolsFramework return true; } + bool StoreInstanceInPrefabDom( + const Instance& instance, + PrefabDom& prefabDom, + AZStd::vector>& referencedAssets, + StoreFlags flags) + { + InstanceEntityIdMapper entityIdMapper; + entityIdMapper.SetStoringInstance(instance); + + // Need to store the id mapper as both its type and its base type + // Meta data is found by type id and we need access to both types at different levels (Instance, EntityId) + AZ::JsonSerializerSettings settings; + settings.m_metadata.Add(static_cast(&entityIdMapper)); + settings.m_metadata.Add(&entityIdMapper); + settings.m_metadata.Add(AZ::Data::SerializedAssetTracker{}); + + if ((flags & StoreFlags::StripDefaultValues) != StoreFlags::StripDefaultValues) + { + settings.m_keepDefaults = true; + } + + if ((flags & StoreFlags::StoreLinkIds) != StoreFlags::None) + { + settings.m_metadata.Create(); + } + + AZStd::string scratchBuffer; + auto issueReportingCallback = [&scratchBuffer]( + AZStd::string_view message, AZ::JsonSerializationResult::ResultCode result, + AZStd::string_view path) -> AZ::JsonSerializationResult::ResultCode + { + return Internal::JsonIssueReporter(scratchBuffer, message, result, path); + }; + + settings.m_reporting = AZStd::move(issueReportingCallback); + + AZ::JsonSerializationResult::ResultCode result = + AZ::JsonSerialization::Store(prefabDom, prefabDom.GetAllocator(), instance, settings); + + if (result.GetProcessing() == AZ::JsonSerializationResult::Processing::Halted) + { + AZ_Error( + "Prefab", false, + "Failed to serialize prefab instance with source path %s. " + "Unable to proceed.", + instance.GetTemplateSourcePath().c_str()); + + return false; + } + + referencedAssets = AZStd::move(settings.m_metadata.Find()->GetTrackedAssets()); + return true; + } + bool StoreEntityInPrefabDomFormat(const AZ::Entity& entity, Instance& owningInstance, PrefabDom& prefabDom, StoreFlags flags) { InstanceEntityIdMapper entityIdMapper; diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabDomUtils.h b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabDomUtils.h index 89d1a046e5..e336cf0fca 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabDomUtils.h +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabDomUtils.h @@ -57,14 +57,28 @@ namespace AzToolsFramework AZ_DEFINE_ENUM_BITWISE_OPERATORS(StoreFlags); /** - * Stores a valid Prefab Instance within a Prefab Dom. Useful for generating Templates - * @param instance The instance to store - * @param prefabDom The prefabDom that will be used to store the Instance data - * @param flags Controls behavior such as whether to store default values - * @return bool on whether the operation succeeded + * Stores a valid Prefab Instance within a Prefab Dom. Useful for generating Templates. + * @param instance The instance to store. + * @param prefabDom The prefabDom that will be used to store the Instance data. + * @param flags Controls behavior such as whether to store default values. + * @return bool on whether the operation succeeded. */ bool StoreInstanceInPrefabDom(const Instance& instance, PrefabDom& prefabDom, StoreFlags flags = StoreFlags::None); + /** + * Stores a valid Prefab Instance within a Prefab Dom. Useful for generating Templates. + * @param instance The instance to store. + * @param prefabDom The prefabDom that will be used to store the Instance data. + * @param referencedAssets Collect a list of the assets that are referenced during storing. + * @param flags Controls behavior such as whether to store default values. + * @return bool on whether the operation succeeded. + */ + bool StoreInstanceInPrefabDom( + const Instance& instance, + PrefabDom& prefabDom, + AZStd::vector>& referencedAssets, + StoreFlags flags = StoreFlags::None); + /** * Stores a valid entity in Prefab Dom format. * @param entity The entity to store diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Spawnable/InMemorySpawnableAssetContainer.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Spawnable/InMemorySpawnableAssetContainer.cpp index dc5cd299b8..634cb9d448 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Spawnable/InMemorySpawnableAssetContainer.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Spawnable/InMemorySpawnableAssetContainer.cpp @@ -6,12 +6,14 @@ * */ -#include #include #include +#include +#include #include #include +#include #include namespace AzToolsFramework::Prefab::PrefabConversionUtils @@ -161,13 +163,10 @@ namespace AzToolsFramework::Prefab::PrefabConversionUtils { return AZ::Failure(AZStd::string::format("Failed to produce the target spawnable '%.*s'.", AZ_STRING_ARG(spawnableName))); } - + if (loadReferencedAssets) { - for (auto& product : context.GetProcessedObjects()) - { - LoadReferencedAssets(product.GetReferencedAssets()); - } + LoadReferencedAssets(spawnableAssetData); } auto& spawnableAssetDataAdded = m_spawnableAssets.emplace(spawnableName, spawnableAssetData).first->second; @@ -213,63 +212,91 @@ namespace AzToolsFramework::Prefab::PrefabConversionUtils return m_spawnableAssets; } - void InMemorySpawnableAssetContainer::LoadReferencedAssets(AZStd::vector>& referencedAssets) + void InMemorySpawnableAssetContainer::LoadReferencedAssets(SpawnableAssetData& spawnable) { - // Start our loads on all assets by calling GetAsset from the AssetManager - for (AZ::Data::Asset& asset : referencedAssets) + // Get the referenced assets directly from the product. This is done for two reasons: + // 1. Avoids calls to the Asset Manager for assets that are already loaded. + // 2. Gets the exact asset to load to avoid issues with assets that don't reload. + AZStd::vector*> blockingAssets; + + for (AZ::Data::Asset& asset : spawnable.m_assets) { - if (!asset.GetId().IsValid()) + AZ::SerializeContext* sc = nullptr; + AZ::ComponentApplicationBus::BroadcastResult(sc, &AZ::ComponentApplicationBus::Events::GetSerializeContext); + AZ_Assert( + sc, "Unable to locate Serialize Context while resolving asset references in the in-memory spawnable asset container."); + + auto callback = [&blockingAssets]( + void* object, const AZ::SerializeContext::ClassData* classData, + [[maybe_unused]]const AZ::SerializeContext::ClassElement* elementData) -> bool { - AZ_Error("Prefab", false, "Invalid asset found referenced in scene while entering game mode"); - continue; - } + if (classData->m_typeId == AZ::GetAssetClassId()) + { + auto asset = reinterpret_cast*>(object); - const AZ::Data::AssetLoadBehavior loadBehavior = asset.GetAutoLoadBehavior(); + if (!asset->GetId().IsValid()) + { + AZ_Error("Prefab", false, "Invalid asset found referenced in scene while entering game mode"); + return false; + } - if (loadBehavior == AZ::Data::AssetLoadBehavior::NoLoad) - { - continue; - } + if (asset->GetStatus() != AZ::Data::AssetData::AssetStatus::NotLoaded) + { + // Already loaded so no need to do anything. + return false; + } - AZ::Data::AssetId assetId = asset.GetId(); - AZ::Data::AssetType assetType = asset.GetType(); + const AZ::Data::AssetLoadBehavior loadBehavior = asset->GetAutoLoadBehavior(); + if (loadBehavior == AZ::Data::AssetLoadBehavior::NoLoad) + { + return false; + } - asset = AZ::Data::AssetManager::Instance().GetAsset(assetId, assetType, loadBehavior); + AZ::Data::AssetId assetId = asset->GetId(); + AZ::Data::AssetType assetType = asset->GetType(); - if (!asset.GetId().IsValid()) - { - AZ_Error("Prefab", false, "Invalid asset found referenced in scene while entering game mode"); - continue; - } + // Always queue load as the next step will stop loading for all PreLoad assets + *asset = AZ::Data::AssetManager::Instance().GetAsset(assetId, assetType, AZ::Data::AssetLoadBehavior::QueueLoad); + + if (!asset->GetId().IsValid()) + { + AZ_Error("Prefab", false, "Invalid asset found referenced in scene while entering game mode"); + return false; + } + + if (loadBehavior == AZ::Data::AssetLoadBehavior::PreLoad) + { + // Only assets that are preloaded need to be waited on. + blockingAssets.push_back(asset); + } + return false; + } + return true; + }; + + AZ::SerializeContext::EnumerateInstanceCallContext enumerationContext( + callback, nullptr, sc, AZ::SerializeContext::ENUM_ACCESS_FOR_READ, nullptr); + sc->EnumerateInstance(&enumerationContext, asset.GetData(), asset.GetType(), nullptr, nullptr); } // For all Preload assets we block until they're ready // We do this as a separate pass so that we don't interrupt queuing up all other asset loads - for (AZ::Data::Asset& asset : referencedAssets) + for (AZ::Data::Asset* asset : blockingAssets) { - if (!asset.GetId().IsValid()) + asset->BlockUntilLoadComplete(); + + if (asset->IsError()) { - AZ_Error("Prefab", false, "Invalid asset found referenced in scene while entering game mode"); + AZ_Error( + "Prefab", false, "Asset with id %s failed to preload while entering game mode", + asset->GetId().ToString().c_str()); + continue; } - const AZ::Data::AssetLoadBehavior loadBehavior = asset.GetAutoLoadBehavior(); - - if (loadBehavior != AZ::Data::AssetLoadBehavior::PreLoad) - { - continue; - } - - asset.BlockUntilLoadComplete(); - - if (asset.IsError()) - { - AZ_Error("Prefab", false, "Asset with id %s failed to preload while entering game mode", - asset.GetId().ToString().c_str()); - - continue; - } + // Reset the load behavior back to preload because the async load will have caused the behavior to be set to queued. Some assets + // will complain if they're not set to the correct loading behavior. + asset->SetAutoLoadBehavior(AZ::Data::AssetLoadBehavior::PreLoad); } } - } // namespace AzToolsFramework::Prefab::PrefabConversionUtils diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Spawnable/InMemorySpawnableAssetContainer.h b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Spawnable/InMemorySpawnableAssetContainer.h index f567c99b00..6f11976fc9 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Spawnable/InMemorySpawnableAssetContainer.h +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Spawnable/InMemorySpawnableAssetContainer.h @@ -21,7 +21,6 @@ namespace AzToolsFramework::Prefab namespace AzToolsFramework::Prefab::PrefabConversionUtils { - class InMemorySpawnableAssetContainer { public: @@ -58,8 +57,8 @@ namespace AzToolsFramework::Prefab::PrefabConversionUtils const SpawnableAssets& GetAllInMemorySpawnableAssets() const; private: - void LoadReferencedAssets(AZStd::vector>& referencedAssets); - + void LoadReferencedAssets(SpawnableAssetData& spawnable); + SpawnableAssets m_spawnableAssets; PrefabConversionUtils::PrefabConversionPipeline m_converter; AZStd::string_view m_stockProfile; diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Spawnable/PrefabDocument.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Spawnable/PrefabDocument.cpp index 230c2226cd..f13ddc027f 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Spawnable/PrefabDocument.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Spawnable/PrefabDocument.cpp @@ -53,21 +53,14 @@ namespace AzToolsFramework::Prefab::PrefabConversionUtils const PrefabDom& PrefabDocument::GetDom() const { - if (m_isDirty) - { - m_isDirty = !PrefabDomUtils::StoreInstanceInPrefabDom(*m_instance, m_dom); - } + RefreshPrefabDom(); return m_dom; } PrefabDom&& PrefabDocument::TakeDom() { - if (m_isDirty) - { - [[maybe_unused]] bool storedSuccessfully = PrefabDomUtils::StoreInstanceInPrefabDom(*m_instance, m_dom); - AZ_Assert(storedSuccessfully, "Failed to store Instance '%s' to PrefabDom.", m_name.c_str()); - m_isDirty = false; - } + RefreshPrefabDom(); + // After the PrefabDom is moved an empty PrefabDom is left behind. This should be reflected in the Instance, // so reset it so it's empty as well. m_instance->Reset(); @@ -126,11 +119,13 @@ namespace AzToolsFramework::Prefab::PrefabConversionUtils AZStd::vector>& PrefabDocument::GetReferencedAssets() { + RefreshPrefabDom(); return m_referencedAssets; } const AZStd::vector>& PrefabDocument::GetReferencedAssets() const { + RefreshPrefabDom(); return m_referencedAssets; } @@ -158,4 +153,23 @@ namespace AzToolsFramework::Prefab::PrefabConversionUtils return false; } } + + bool PrefabDocument::RefreshPrefabDom() const + { + if (m_isDirty) + { + m_referencedAssets.clear(); + if (PrefabDomUtils::StoreInstanceInPrefabDom(*m_instance, m_dom, m_referencedAssets)) + { + m_isDirty = false; + return true; + } + else + { + AZ_Assert(false, "Failed to store Instance '%s' to PrefabDom.", m_name.c_str()); + return false; + } + } + return true; + } } // namespace AzToolsFramework::Prefab::PrefabConversionUtils diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Spawnable/PrefabDocument.h b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Spawnable/PrefabDocument.h index 661dba5edf..a651d73e63 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Spawnable/PrefabDocument.h +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Spawnable/PrefabDocument.h @@ -58,11 +58,13 @@ namespace AzToolsFramework::Prefab::PrefabConversionUtils private: bool ConstructInstanceFromPrefabDom(const PrefabDom& prefab); + // Marked const so this function can be called from other const functions. It will only operate on mutable variables. + bool RefreshPrefabDom() const; mutable PrefabDom m_dom; AZStd::unique_ptr m_instance; AZStd::string m_name; - AZStd::vector> m_referencedAssets; + mutable AZStd::vector> m_referencedAssets; mutable bool m_isDirty{ false }; }; } // namespace AzToolsFramework::Prefab::PrefabConversionUtils From 27a75fa6a1b0f3b6a37ca5b3afd48d565ccc1db4 Mon Sep 17 00:00:00 2001 From: AMZN-koppersr <82230785+AMZN-koppersr@users.noreply.github.com> Date: Tue, 25 Jan 2022 18:15:09 -0800 Subject: [PATCH 2/5] Addressing feedback for the Play In Editor missing asset fixes. - Deduplicated code for StoreInstanceInPrefabDom. - Used QueueLoad to load an asset instead of starting a load by getting a new asset instance. - Extended the error information when failing to load an asset. - RefreshPrefabDom no longer returns a boolean as it wasn't used. Signed-off-by: AMZN-koppersr <82230785+AMZN-koppersr@users.noreply.github.com> --- .../Prefab/PrefabDomUtils.cpp | 153 ++++++++---------- .../InMemorySpawnableAssetContainer.cpp | 26 +-- .../Prefab/Spawnable/PrefabDocument.cpp | 5 +- .../Prefab/Spawnable/PrefabDocument.h | 2 +- 4 files changed, 80 insertions(+), 106 deletions(-) diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabDomUtils.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabDomUtils.cpp index 3be3fbce11..d6faed4b0a 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabDomUtils.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabDomUtils.cpp @@ -28,7 +28,7 @@ namespace AzToolsFramework { namespace Internal { - AZ::JsonSerializationResult::ResultCode JsonIssueReporter(AZStd::string& scratchBuffer, + static AZ::JsonSerializationResult::ResultCode JsonIssueReporter(AZStd::string& scratchBuffer, AZStd::string_view message, AZ::JsonSerializationResult::ResultCode result, AZStd::string_view path) { namespace JSR = AZ::JsonSerializationResult; @@ -48,6 +48,66 @@ namespace AzToolsFramework return result; } + + static bool StoreInstanceInPrefabDom( + const Instance& instance, + PrefabDom& prefabDom, + AZStd::vector>* referencedAssets, + StoreFlags flags) + { + InstanceEntityIdMapper entityIdMapper; + entityIdMapper.SetStoringInstance(instance); + + // Need to store the id mapper as both its type and its base type + // Metadata is found by type id and we need access to both types at different levels (Instance, EntityId) + AZ::JsonSerializerSettings settings; + settings.m_metadata.Add(static_cast(&entityIdMapper)); + settings.m_metadata.Add(&entityIdMapper); + if (referencedAssets) + { + settings.m_metadata.Add(AZ::Data::SerializedAssetTracker{}); + } + + if ((flags & StoreFlags::StripDefaultValues) != StoreFlags::StripDefaultValues) + { + settings.m_keepDefaults = true; + } + + if ((flags & StoreFlags::StoreLinkIds) != StoreFlags::None) + { + settings.m_metadata.Create(); + } + + AZStd::string scratchBuffer; + auto issueReportingCallback = [&scratchBuffer]( + AZStd::string_view message, AZ::JsonSerializationResult::ResultCode result, + AZStd::string_view path) -> AZ::JsonSerializationResult::ResultCode + { + return Internal::JsonIssueReporter(scratchBuffer, message, result, path); + }; + + settings.m_reporting = AZStd::move(issueReportingCallback); + + AZ::JsonSerializationResult::ResultCode result = + AZ::JsonSerialization::Store(prefabDom, prefabDom.GetAllocator(), instance, settings); + + if (result.GetProcessing() == AZ::JsonSerializationResult::Processing::Halted) + { + AZ_Error( + "Prefab", false, + "Failed to serialize prefab instance with source path %s. " + "Unable to proceed.", + instance.GetTemplateSourcePath().c_str()); + + return false; + } + + if (referencedAssets) + { + *referencedAssets = AZStd::move(settings.m_metadata.Find()->GetTrackedAssets()); + } + return true; + } } PrefabDomValueReference FindPrefabDomValue(PrefabDomValue& parentValue, const char* valueName) @@ -74,49 +134,7 @@ namespace AzToolsFramework bool StoreInstanceInPrefabDom(const Instance& instance, PrefabDom& prefabDom, StoreFlags flags) { - InstanceEntityIdMapper entityIdMapper; - entityIdMapper.SetStoringInstance(instance); - - // Need to store the id mapper as both its type and its base type - // Meta data is found by type id and we need access to both types at different levels (Instance, EntityId) - AZ::JsonSerializerSettings settings; - settings.m_metadata.Add(static_cast(&entityIdMapper)); - settings.m_metadata.Add(&entityIdMapper); - - if ((flags & StoreFlags::StripDefaultValues) != StoreFlags::StripDefaultValues) - { - settings.m_keepDefaults = true; - } - - if ((flags & StoreFlags::StoreLinkIds) != StoreFlags::None) - { - settings.m_metadata.Create(); - } - - AZStd::string scratchBuffer; - auto issueReportingCallback = [&scratchBuffer] - (AZStd::string_view message, AZ::JsonSerializationResult::ResultCode result, - AZStd::string_view path) -> AZ::JsonSerializationResult::ResultCode - { - return Internal::JsonIssueReporter(scratchBuffer, message, result, path); - }; - - settings.m_reporting = AZStd::move(issueReportingCallback); - - AZ::JsonSerializationResult::ResultCode result = - AZ::JsonSerialization::Store(prefabDom, prefabDom.GetAllocator(), instance, settings); - - if (result.GetProcessing() == AZ::JsonSerializationResult::Processing::Halted) - { - AZ_Error("Prefab", false, - "Failed to serialize prefab instance with source path %s. " - "Unable to proceed.", - instance.GetTemplateSourcePath().c_str()); - - return false; - } - - return true; + return Internal::StoreInstanceInPrefabDom(instance, prefabDom, nullptr, flags); } bool StoreInstanceInPrefabDom( @@ -125,52 +143,7 @@ namespace AzToolsFramework AZStd::vector>& referencedAssets, StoreFlags flags) { - InstanceEntityIdMapper entityIdMapper; - entityIdMapper.SetStoringInstance(instance); - - // Need to store the id mapper as both its type and its base type - // Meta data is found by type id and we need access to both types at different levels (Instance, EntityId) - AZ::JsonSerializerSettings settings; - settings.m_metadata.Add(static_cast(&entityIdMapper)); - settings.m_metadata.Add(&entityIdMapper); - settings.m_metadata.Add(AZ::Data::SerializedAssetTracker{}); - - if ((flags & StoreFlags::StripDefaultValues) != StoreFlags::StripDefaultValues) - { - settings.m_keepDefaults = true; - } - - if ((flags & StoreFlags::StoreLinkIds) != StoreFlags::None) - { - settings.m_metadata.Create(); - } - - AZStd::string scratchBuffer; - auto issueReportingCallback = [&scratchBuffer]( - AZStd::string_view message, AZ::JsonSerializationResult::ResultCode result, - AZStd::string_view path) -> AZ::JsonSerializationResult::ResultCode - { - return Internal::JsonIssueReporter(scratchBuffer, message, result, path); - }; - - settings.m_reporting = AZStd::move(issueReportingCallback); - - AZ::JsonSerializationResult::ResultCode result = - AZ::JsonSerialization::Store(prefabDom, prefabDom.GetAllocator(), instance, settings); - - if (result.GetProcessing() == AZ::JsonSerializationResult::Processing::Halted) - { - AZ_Error( - "Prefab", false, - "Failed to serialize prefab instance with source path %s. " - "Unable to proceed.", - instance.GetTemplateSourcePath().c_str()); - - return false; - } - - referencedAssets = AZStd::move(settings.m_metadata.Find()->GetTrackedAssets()); - return true; + return Internal::StoreInstanceInPrefabDom(instance, prefabDom, &referencedAssets, flags); } bool StoreEntityInPrefabDomFormat(const AZ::Entity& entity, Instance& owningInstance, PrefabDom& prefabDom, StoreFlags flags) diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Spawnable/InMemorySpawnableAssetContainer.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Spawnable/InMemorySpawnableAssetContainer.cpp index 634cb9d448..a927f60789 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Spawnable/InMemorySpawnableAssetContainer.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Spawnable/InMemorySpawnableAssetContainer.cpp @@ -255,20 +255,23 @@ namespace AzToolsFramework::Prefab::PrefabConversionUtils AZ::Data::AssetId assetId = asset->GetId(); AZ::Data::AssetType assetType = asset->GetType(); - // Always queue load as the next step will stop loading for all PreLoad assets - *asset = AZ::Data::AssetManager::Instance().GetAsset(assetId, assetType, AZ::Data::AssetLoadBehavior::QueueLoad); - - if (!asset->GetId().IsValid()) - { - AZ_Error("Prefab", false, "Invalid asset found referenced in scene while entering game mode"); - return false; - } - if (loadBehavior == AZ::Data::AssetLoadBehavior::PreLoad) { // Only assets that are preloaded need to be waited on. blockingAssets.push_back(asset); + // Queue any pending request in parallel. Assets that were set to PreLoad will be waited for resulting + // in the same overall load guarantees. + asset->SetAutoLoadBehavior(AZ::Data::AssetLoadBehavior::QueueLoad); } + if (!asset->QueueLoad()) + { + AZ_Error( + "Prefab", false, "Failed to queue asset '%s' (%s) of type '%s' for loading while entering game mode.", + asset->GetHint().c_str(), asset->GetId().ToString>().c_str(), + asset->GetType().ToString>().c_str()); + return false; + } + return false; } return true; @@ -288,8 +291,9 @@ namespace AzToolsFramework::Prefab::PrefabConversionUtils if (asset->IsError()) { AZ_Error( - "Prefab", false, "Asset with id %s failed to preload while entering game mode", - asset->GetId().ToString().c_str()); + "Prefab", false, "Asset '%s' (%s) of type '%s' failed to preload while entering game mode", asset->GetHint().c_str(), + asset->GetId().ToString>().c_str(), + asset->GetType().ToString>().c_str()); continue; } diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Spawnable/PrefabDocument.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Spawnable/PrefabDocument.cpp index f13ddc027f..097acbb47c 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Spawnable/PrefabDocument.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Spawnable/PrefabDocument.cpp @@ -154,7 +154,7 @@ namespace AzToolsFramework::Prefab::PrefabConversionUtils } } - bool PrefabDocument::RefreshPrefabDom() const + void PrefabDocument::RefreshPrefabDom() const { if (m_isDirty) { @@ -162,14 +162,11 @@ namespace AzToolsFramework::Prefab::PrefabConversionUtils if (PrefabDomUtils::StoreInstanceInPrefabDom(*m_instance, m_dom, m_referencedAssets)) { m_isDirty = false; - return true; } else { AZ_Assert(false, "Failed to store Instance '%s' to PrefabDom.", m_name.c_str()); - return false; } } - return true; } } // namespace AzToolsFramework::Prefab::PrefabConversionUtils diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Spawnable/PrefabDocument.h b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Spawnable/PrefabDocument.h index a651d73e63..cff6e8368f 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Spawnable/PrefabDocument.h +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Spawnable/PrefabDocument.h @@ -59,7 +59,7 @@ namespace AzToolsFramework::Prefab::PrefabConversionUtils private: bool ConstructInstanceFromPrefabDom(const PrefabDom& prefab); // Marked const so this function can be called from other const functions. It will only operate on mutable variables. - bool RefreshPrefabDom() const; + void RefreshPrefabDom() const; mutable PrefabDom m_dom; AZStd::unique_ptr m_instance; From 498e5b738f935569b5b770bfbcefd9c75b9b32b8 Mon Sep 17 00:00:00 2001 From: AMZN-koppersr <82230785+AMZN-koppersr@users.noreply.github.com> Date: Wed, 26 Jan 2022 16:40:53 -0800 Subject: [PATCH 3/5] Removed SetAutoLoadBehavior from InMemorySpawnableAssetContainer. These calls didn't have any practical effect and created confusion, so they were removed. Signed-off-by: AMZN-koppersr <82230785+AMZN-koppersr@users.noreply.github.com> --- .../Prefab/Spawnable/InMemorySpawnableAssetContainer.cpp | 7 ------- 1 file changed, 7 deletions(-) diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Spawnable/InMemorySpawnableAssetContainer.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Spawnable/InMemorySpawnableAssetContainer.cpp index a927f60789..f0fbd1a299 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Spawnable/InMemorySpawnableAssetContainer.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Spawnable/InMemorySpawnableAssetContainer.cpp @@ -259,9 +259,6 @@ namespace AzToolsFramework::Prefab::PrefabConversionUtils { // Only assets that are preloaded need to be waited on. blockingAssets.push_back(asset); - // Queue any pending request in parallel. Assets that were set to PreLoad will be waited for resulting - // in the same overall load guarantees. - asset->SetAutoLoadBehavior(AZ::Data::AssetLoadBehavior::QueueLoad); } if (!asset->QueueLoad()) { @@ -297,10 +294,6 @@ namespace AzToolsFramework::Prefab::PrefabConversionUtils continue; } - - // Reset the load behavior back to preload because the async load will have caused the behavior to be set to queued. Some assets - // will complain if they're not set to the correct loading behavior. - asset->SetAutoLoadBehavior(AZ::Data::AssetLoadBehavior::PreLoad); } } } // namespace AzToolsFramework::Prefab::PrefabConversionUtils From 94919fe270b0332cb9323b27ba8f4a4bb62ebc0e Mon Sep 17 00:00:00 2001 From: AMZN-koppersr <82230785+AMZN-koppersr@users.noreply.github.com> Date: Thu, 27 Jan 2022 09:47:12 -0800 Subject: [PATCH 4/5] Moved Serialize Context retrieval out of a loop and expanded an error message in InMemorySpawnableAssetContainer::LoadReferencedAssets. Signed-off-by: AMZN-koppersr <82230785+AMZN-koppersr@users.noreply.github.com> --- .../Spawnable/InMemorySpawnableAssetContainer.cpp | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Spawnable/InMemorySpawnableAssetContainer.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Spawnable/InMemorySpawnableAssetContainer.cpp index f0fbd1a299..fa7bebfaea 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Spawnable/InMemorySpawnableAssetContainer.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Spawnable/InMemorySpawnableAssetContainer.cpp @@ -219,13 +219,12 @@ namespace AzToolsFramework::Prefab::PrefabConversionUtils // 2. Gets the exact asset to load to avoid issues with assets that don't reload. AZStd::vector*> blockingAssets; + AZ::SerializeContext* sc = nullptr; + AZ::ComponentApplicationBus::BroadcastResult(sc, &AZ::ComponentApplicationBus::Events::GetSerializeContext); + AZ_Assert(sc, "Unable to locate Serialize Context while resolving asset references in the in-memory spawnable asset container."); + for (AZ::Data::Asset& asset : spawnable.m_assets) { - AZ::SerializeContext* sc = nullptr; - AZ::ComponentApplicationBus::BroadcastResult(sc, &AZ::ComponentApplicationBus::Events::GetSerializeContext); - AZ_Assert( - sc, "Unable to locate Serialize Context while resolving asset references in the in-memory spawnable asset container."); - auto callback = [&blockingAssets]( void* object, const AZ::SerializeContext::ClassData* classData, [[maybe_unused]]const AZ::SerializeContext::ClassElement* elementData) -> bool @@ -236,7 +235,10 @@ namespace AzToolsFramework::Prefab::PrefabConversionUtils if (!asset->GetId().IsValid()) { - AZ_Error("Prefab", false, "Invalid asset found referenced in scene while entering game mode"); + AZ_Error( + "Prefab", false, + "Invalid asset found referenced in scene while entering game mode. The asset was stored in an instance of %s.", + classData->m_name); return false; } From 3a08a3b325430359ed41041d4d198069724bfa9b Mon Sep 17 00:00:00 2001 From: AMZN-koppersr <82230785+AMZN-koppersr@users.noreply.github.com> Date: Thu, 27 Jan 2022 12:38:57 -0800 Subject: [PATCH 5/5] Removed some unused local variables. Signed-off-by: AMZN-koppersr <82230785+AMZN-koppersr@users.noreply.github.com> --- .../Prefab/Spawnable/InMemorySpawnableAssetContainer.cpp | 3 --- 1 file changed, 3 deletions(-) diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Spawnable/InMemorySpawnableAssetContainer.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Spawnable/InMemorySpawnableAssetContainer.cpp index fa7bebfaea..567ccc3619 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Spawnable/InMemorySpawnableAssetContainer.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Spawnable/InMemorySpawnableAssetContainer.cpp @@ -254,9 +254,6 @@ namespace AzToolsFramework::Prefab::PrefabConversionUtils return false; } - AZ::Data::AssetId assetId = asset->GetId(); - AZ::Data::AssetType assetType = asset->GetType(); - if (loadBehavior == AZ::Data::AssetLoadBehavior::PreLoad) { // Only assets that are preloaded need to be waited on.