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] 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