From 7c96bc4f1f34dfde8b6bc597d8683cdf29b1bbcb Mon Sep 17 00:00:00 2001 From: amzn-mike <80125227+amzn-mike@users.noreply.github.com> Date: Tue, 7 Dec 2021 13:00:47 -0600 Subject: [PATCH] =?UTF-8?q?Fix=20loaded=20procprefabs=20not=20going=20thro?= =?UTF-8?q?ugh=20the=20asset=20hint=20fixup=20process=E2=80=A6=20(#6077)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Fix loaded procprefabs not going through the asset hint fixup process for all versions of LoadInstanceFromPrefabDom. Fixes procprefab asset references not working when pressing Ctrl+G in the editor Signed-off-by: amzn-mike <80125227+amzn-mike@users.noreply.github.com> * Add unit test Signed-off-by: amzn-mike <80125227+amzn-mike@users.noreply.github.com> * Add missing include Signed-off-by: amzn-mike <80125227+amzn-mike@users.noreply.github.com> --- .../Prefab/PrefabDomUtils.cpp | 52 ++--- .../Tests/Prefab/PrefabAssetFixupTests.cpp | 185 ++++++++++++++++++ .../Tests/aztoolsframeworktests_files.cmake | 2 +- 3 files changed, 214 insertions(+), 25 deletions(-) create mode 100644 Code/Framework/AzToolsFramework/Tests/Prefab/PrefabAssetFixupTests.cpp diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabDomUtils.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabDomUtils.cpp index a84d6bf706..83e8d2e4be 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabDomUtils.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabDomUtils.cpp @@ -150,6 +150,24 @@ namespace AzToolsFramework return result.GetOutcome() == AZ::JsonSerializationResult::Outcomes::Success; } + // some assets may come in from the JSON serialzier with no AssetID, but have an asset hint + // this attempts to fix up the assets using the assetHint field + void FixUpInvalidAssets(AZ::Data::Asset& asset) + { + if (!asset.GetId().IsValid() && !asset.GetHint().empty()) + { + AZ::Data::AssetId assetId; + AZ::Data::AssetCatalogRequestBus::BroadcastResult( + assetId, &AZ::Data::AssetCatalogRequestBus::Events::GetAssetIdByPath, asset.GetHint().c_str(), + AZ::Data::s_invalidAssetType, false); + + if (assetId.IsValid()) + { + asset.Create(assetId, false); + } + } + } + bool LoadInstanceFromPrefabDom(Instance& instance, const PrefabDom& prefabDom, LoadFlags flags) { // When entities are rebuilt they are first destroyed. As a result any assets they were exclusively holding on to will @@ -164,13 +182,17 @@ namespace AzToolsFramework entityIdMapper.SetEntityIdGenerationApproach(InstanceEntityIdMapper::EntityIdGenerationApproach::Random); } + auto tracker = AZ::Data::SerializedAssetTracker{}; + tracker.SetAssetFixUp(&FixUpInvalidAssets); + AZ::JsonDeserializerSettings settings; // The InstanceEntityIdMapper is registered twice because it's used in several places during deserialization where one is // specific for the InstanceEntityIdMapper and once for the generic JsonEntityIdMapper. Because the Json Serializer's meta // data has strict typing and doesn't look for inheritance both have to be explicitly added so they're found both locations. settings.m_metadata.Add(static_cast(&entityIdMapper)); settings.m_metadata.Add(&entityIdMapper); - + settings.m_metadata.Add(tracker); + AZ::JsonSerializationResult::ResultCode result = AZ::JsonSerialization::Load(instance, prefabDom, settings); @@ -203,13 +225,16 @@ namespace AzToolsFramework entityIdMapper.SetEntityIdGenerationApproach(InstanceEntityIdMapper::EntityIdGenerationApproach::Random); } + auto tracker = AZ::Data::SerializedAssetTracker{}; + tracker.SetAssetFixUp(&FixUpInvalidAssets); + AZ::JsonDeserializerSettings settings; // The InstanceEntityIdMapper is registered twice because it's used in several places during deserialization where one is // specific for the InstanceEntityIdMapper and once for the generic JsonEntityIdMapper. Because the Json Serializer's meta // data has strict typing and doesn't look for inheritance both have to be explicitly added so they're found both locations. settings.m_metadata.Add(static_cast(&entityIdMapper)); settings.m_metadata.Add(&entityIdMapper); - settings.m_metadata.Create(); + settings.m_metadata.Add(tracker); AZ::JsonSerializationResult::ResultCode result = AZ::JsonSerialization::Load(instance, prefabDom, settings); @@ -246,29 +271,8 @@ namespace AzToolsFramework entityIdMapper.SetEntityIdGenerationApproach(InstanceEntityIdMapper::EntityIdGenerationApproach::Random); } - // some assets may come in from the JSON serialzier with no AssetID, but have an asset hint - // this attempts to fix up the assets using the assetHint field - auto fixUpInvalidAssets = [](AZ::Data::Asset& asset) - { - if (!asset.GetId().IsValid() && !asset.GetHint().empty()) - { - AZ::Data::AssetId assetId; - AZ::Data::AssetCatalogRequestBus::BroadcastResult( - assetId, - &AZ::Data::AssetCatalogRequestBus::Events::GetAssetIdByPath, - asset.GetHint().c_str(), - AZ::Data::s_invalidAssetType, - false); - - if (assetId.IsValid()) - { - asset.Create(assetId, false); - } - } - }; - auto tracker = AZ::Data::SerializedAssetTracker{}; - tracker.SetAssetFixUp(fixUpInvalidAssets); + tracker.SetAssetFixUp(&FixUpInvalidAssets); AZ::JsonDeserializerSettings settings; // The InstanceEntityIdMapper is registered twice because it's used in several places during deserialization where one is diff --git a/Code/Framework/AzToolsFramework/Tests/Prefab/PrefabAssetFixupTests.cpp b/Code/Framework/AzToolsFramework/Tests/Prefab/PrefabAssetFixupTests.cpp new file mode 100644 index 0000000000..379392553e --- /dev/null +++ b/Code/Framework/AzToolsFramework/Tests/Prefab/PrefabAssetFixupTests.cpp @@ -0,0 +1,185 @@ +/* + * 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 + * + */ + +#include +#include +#include +#include +#include + +namespace UnitTest +{ + using PrefabInstantiateTest = PrefabTestFixture; + + struct MockAsset : AZ::Data::AssetData + { + AZ_RTTI(MockAsset, "{DAB98A3F-1714-4B95-AACB-8C150B0D0628}", AZ::Data::AssetData); + + AZ_CLASS_ALLOCATOR(MockAsset, AZ::SystemAllocator, 0); + + static void Reflect(AZ::ReflectContext* context) + { + if (auto serializeContext = azrtti_cast(context)) + { + serializeContext->Class()->Field("data", &MockAsset::m_data); + } + } + float m_data = 1.f; + }; + + struct MockAssetComponent : AZ::Component + { + AZ_COMPONENT(MockAssetComponent, "{D81B0D06-B495-479E-832A-A63079FD6D37}"); + + static void Reflect(AZ::ReflectContext* context) + { + MockAsset::Reflect(context); + + if (auto serializeContext = azrtti_cast(context)) + { + serializeContext->Class() + ->Field("asset", &MockAssetComponent::m_asset); + } + } + + void Activate() override{} + void Deactivate() override{} + + AZ::Data::Asset m_asset; + }; + + class MockAssetHandler : public AZ::Data::AssetHandler + { + public: + AZ_CLASS_ALLOCATOR(MockAssetHandler, AZ::SystemAllocator, 0); + + AZ::Data::AssetPtr CreateAsset(const AZ::Data::AssetId& id, const AZ::Data::AssetType& type) override + { + (void)id; + EXPECT_TRUE(type == azrtti_typeid()); + if (type == azrtti_typeid()) + { + return aznew MockAsset(); + } + return nullptr; + } + + LoadResult LoadAssetData(const AZ::Data::Asset&, AZStd::shared_ptr, const AZ::Data::AssetFilterCB&) override + { + return LoadResult::Error; + } + + void DestroyAsset(AZ::Data::AssetPtr ptr) override + { + EXPECT_TRUE(ptr->GetType() == azrtti_typeid()); + delete ptr; + } + + void GetHandledAssetTypes(AZStd::vector& assetTypes) override + { + assetTypes.push_back(azrtti_typeid()); + } + }; + + struct PrefabFixupTest : PrefabInstantiateTest + { + void SetUpEditorFixtureImpl() override + { + PrefabInstantiateTest::SetUpEditorFixtureImpl(); + + AZ::SerializeContext* context = nullptr; + + AZ::ComponentApplicationBus::BroadcastResult(context, &AZ::ComponentApplicationBus::Events::GetSerializeContext); + + ASSERT_NE(context, nullptr); + + MockAssetComponent::Reflect(context); + + AZ::Data::AssetManager::Instance().RegisterHandler(&m_handler, azrtti_typeid()); + + auto entity = aznew AZ::Entity(); + auto mockAssetComponent = entity->CreateComponent(); + + mockAssetComponent->m_asset = + AZ::Data::Asset(AZ::Uuid::CreateNull(), AZ::Data::AssetType::CreateNull(), "test.asset"); + + auto newInstance = AZ::Interface::Get()->CreatePrefab({ entity }, {}, "test.prefab"); + + AZStd::string prefabString; + ASSERT_TRUE(m_prefabLoaderInterface->SaveTemplateToString(newInstance->GetTemplateId(), prefabString)); + m_prefabSystemComponent->RemoveAllTemplates(); + + AZ::Outcome readPrefabFileResult = AZ::JsonSerializationUtils::ReadJsonString(prefabString); + + ASSERT_TRUE(readPrefabFileResult.IsSuccess()); + + AZ::Data::AssetCatalogRequestBus::BroadcastResult( + m_assetId, &AZ::Data::AssetCatalogRequestBus::Events::GetAssetIdByPath, "test.asset", azrtti_typeid(), + true); // True to register the asset and generate an AssetId for lookup + + m_prefabDom = readPrefabFileResult.TakeValue(); + } + + void TearDownEditorFixtureImpl() override + { + PrefabInstantiateTest::TearDownEditorFixtureImpl(); + + AZ::Data::AssetManager::Instance().UnregisterHandler(&m_handler); + } + + void CheckInstance(const Instance& instance) + { + const AZ::Entity* loadedEntity = nullptr; + instance.GetConstEntities( + [&loadedEntity](const AZ::Entity& entity) + { + loadedEntity = &entity; + + return false; + }); + + auto loadedComponent = loadedEntity->FindComponent(); + + ASSERT_NE(loadedComponent, nullptr); + + ASSERT_STREQ(loadedComponent->m_asset.GetHint().c_str(), "test.asset"); + ASSERT_EQ(loadedComponent->m_asset->GetId(), m_assetId); + } + + MockAssetHandler m_handler; + PrefabDom m_prefabDom; + AZ::Data::AssetId m_assetId; + }; + + TEST_F(PrefabFixupTest, Test_LoadInstanceFromPrefabDom_Overload1) + { + Instance instance; + ASSERT_TRUE(PrefabDomUtils::LoadInstanceFromPrefabDom(instance, m_prefabDom)); + + CheckInstance(instance); + } + + TEST_F(PrefabFixupTest, Test_LoadInstanceFromPrefabDom_Overload2) + { + Instance instance; + AZStd::vector> referencedAssets; + ASSERT_TRUE(PrefabDomUtils::LoadInstanceFromPrefabDom(instance, m_prefabDom, referencedAssets)); + + CheckInstance(instance); + } + + TEST_F(PrefabFixupTest, Test_LoadInstanceFromPrefabDom_Overload3) + { + Instance instance; + AZStd::vector> referencedAssets; + Instance::EntityList entityList; + (PrefabDomUtils::LoadInstanceFromPrefabDom(instance, entityList, m_prefabDom)); + + CheckInstance(instance); + } +} diff --git a/Code/Framework/AzToolsFramework/Tests/aztoolsframeworktests_files.cmake b/Code/Framework/AzToolsFramework/Tests/aztoolsframeworktests_files.cmake index 1a8819b188..31c70c81a0 100644 --- a/Code/Framework/AzToolsFramework/Tests/aztoolsframeworktests_files.cmake +++ b/Code/Framework/AzToolsFramework/Tests/aztoolsframeworktests_files.cmake @@ -74,7 +74,7 @@ set(FILES Prefab/PrefabEntityAliasTests.cpp Prefab/PrefabInstanceToTemplatePropagatorTests.cpp Prefab/PrefabInstantiateTests.cpp - Prefab/PrefabInstantiateTests.cpp + Prefab/PrefabAssetFixupTests.cpp Prefab/PrefabLoadTemplateTests.cpp Prefab/PrefabTestComponent.cpp Prefab/PrefabTestComponent.h