From a0b1dec929e336b43aadaabb99d0e95ccabf7bfc Mon Sep 17 00:00:00 2001 From: santorac <55155825+santorac@users.noreply.github.com> Date: Wed, 20 Oct 2021 15:31:01 -0700 Subject: [PATCH] Added support for material version updates in MaterialSourceData. This is necessary for tools like Material Editor and Asset Processor to work with the latest property names. - Added MaterialSourceData::ApplyVersionUpdates() for updating the properties. This should be called by tools after loading the MaterialSourceData. (But can be omitted if a tool wants to read the data exactly as it appears in the .material file). - Updated MaterialTypeSourceData::FindProperty to support applying version update renames, including a ApplyPropertyRenames utility function, which are necessary for MaterialSourceData to be able to find the necessary property definitons while loading. - Added a new context struct to JsonMaterialPropertyValueSerializer for passing down the material type version number, to help with applying property renames. - Renamed the .material file format "propertyLayoutVersion" to "materialTypeVersion" which is more accurate. This shouldn't hurt existing data as this field wasn't actually used for anything before. - Updated Material Editor to again store the material type version number in .material files. MaterialSourceDataTests updates... - Updated to include both a .materialtype file and a MaterialTypeAsset for the test material type. Both are used by the MaterialTypeSourceData class. - The default test material type now includes some version update steps; these are only used for version update tests and won't impact the other test functions. - Updated the path for storing temp files to disk, to just be in a "temp" folder in the exe path. (Originally they were saved to the gem folder near MaterialSourceDataTests.cpp, but at some point someone changed it to be under the exe folder, so there's no reason to use the full gem path anymore). MaterialTypeSourceDataTests updates... - Moved some code that was accidentally added to LoadAllFieldsUsingOldFormat but should have been in LoadAndStoreJson_AllFields. - Added test cases for unsupported version update operations Signed-off-by: santorac <55155825+santorac@users.noreply.github.com> --- Code/Framework/AzCore/AzCore/Utils/Utils.cpp | 2 +- .../MaterialPropertyValueSerializer.h | 7 + .../RPI.Edit/Material/MaterialSourceData.h | 6 +- .../Material/MaterialTypeSourceData.h | 13 +- .../Atom/RPI.Reflect/Material/MaterialAsset.h | 3 +- .../RPI.Builders/Material/MaterialBuilder.cpp | 5 + .../MaterialPropertyValueSerializer.cpp | 4 +- .../RPI.Edit/Material/MaterialSourceData.cpp | 39 +++ .../Material/MaterialSourceDataSerializer.cpp | 9 +- .../Material/MaterialTypeSourceData.cpp | 73 +++++- .../RPI/Code/Tests/Common/AssetSystemStub.cpp | 10 +- .../Material/MaterialSourceDataTests.cpp | 244 +++++++++++++++--- .../Material/MaterialTypeSourceDataTests.cpp | 177 ++++++++++++- .../Code/Source/Document/MaterialDocument.cpp | 9 + 14 files changed, 535 insertions(+), 66 deletions(-) diff --git a/Code/Framework/AzCore/AzCore/Utils/Utils.cpp b/Code/Framework/AzCore/AzCore/Utils/Utils.cpp index 2031d14d08..e6bfd78806 100644 --- a/Code/Framework/AzCore/AzCore/Utils/Utils.cpp +++ b/Code/Framework/AzCore/AzCore/Utils/Utils.cpp @@ -120,7 +120,7 @@ namespace AZ::Utils AZ::Outcome WriteFile(AZStd::string_view content, AZStd::string_view filePath) { AZ::IO::FixedMaxPath filePathFixed = filePath; // Because FileIOStream requires a null-terminated string - AZ::IO::FileIOStream stream(filePathFixed.c_str(), AZ::IO::OpenMode::ModeWrite); + AZ::IO::FileIOStream stream(filePathFixed.c_str(), AZ::IO::OpenMode::ModeWrite | AZ::IO::OpenMode::ModeCreatePath); bool success = false; diff --git a/Gems/Atom/RPI/Code/Include/Atom/RPI.Edit/Material/MaterialPropertyValueSerializer.h b/Gems/Atom/RPI/Code/Include/Atom/RPI.Edit/Material/MaterialPropertyValueSerializer.h index befbb7c990..29371618cf 100644 --- a/Gems/Atom/RPI/Code/Include/Atom/RPI.Edit/Material/MaterialPropertyValueSerializer.h +++ b/Gems/Atom/RPI/Code/Include/Atom/RPI.Edit/Material/MaterialPropertyValueSerializer.h @@ -24,6 +24,13 @@ namespace AZ AZ_RTTI(AZ::RPI::JsonMaterialPropertyValueSerializer, "{A52B1ED8-C849-4269-9AA7-9D0814D2EC59}", BaseJsonSerializer); AZ_CLASS_ALLOCATOR_DECL; + //! A LoadContext object must be passed down to the serializer via JsonDeserializerContext::GetMetadata().Add(...) + struct LoadContext + { + AZ_TYPE_INFO(JsonMaterialPropertyValueSerializer::LoadContext, "{5E0A891A-27F6-4AD7-88A5-B9EA50F88B45}"); + uint32_t m_materialTypeVersion; //!< The version number from the .materialtype file + }; + JsonSerializationResult::Result Load(void* outputValue, const Uuid& outputValueTypeId, const rapidjson::Value& inputValue, JsonDeserializerContext& context) override; diff --git a/Gems/Atom/RPI/Code/Include/Atom/RPI.Edit/Material/MaterialSourceData.h b/Gems/Atom/RPI/Code/Include/Atom/RPI.Edit/Material/MaterialSourceData.h index 02607a7954..deeace0d41 100644 --- a/Gems/Atom/RPI/Code/Include/Atom/RPI.Edit/Material/MaterialSourceData.h +++ b/Gems/Atom/RPI/Code/Include/Atom/RPI.Edit/Material/MaterialSourceData.h @@ -50,7 +50,7 @@ namespace AZ AZStd::string m_parentMaterial; //!< The immediate parent of this material - uint32_t m_propertyLayoutVersion = 0; //!< The version of the property layout, defined in the material type, which was used to configure this material + uint32_t m_materialTypeVersion = 0; //!< The version of the material type that was used to configure this material struct Property { @@ -64,6 +64,10 @@ namespace AZ PropertyGroupMap m_properties; + //! Checks the material type version and potentially applies a series of property changes (most common are simple property renames) + //! based on the MaterialTypeAsset's version update procedure. + bool ApplyVersionUpdates(); + //! Creates a MaterialAsset from the MaterialSourceData content. //! @param assetId ID for the MaterialAsset //! @param materialSourceFilePath Indicates the path of the .material file that the MaterialSourceData represents. Used for resolving file-relative paths. diff --git a/Gems/Atom/RPI/Code/Include/Atom/RPI.Edit/Material/MaterialTypeSourceData.h b/Gems/Atom/RPI/Code/Include/Atom/RPI.Edit/Material/MaterialTypeSourceData.h index 44ac926ecf..99eb15db24 100644 --- a/Gems/Atom/RPI/Code/Include/Atom/RPI.Edit/Material/MaterialTypeSourceData.h +++ b/Gems/Atom/RPI/Code/Include/Atom/RPI.Edit/Material/MaterialTypeSourceData.h @@ -13,6 +13,7 @@ #include #include #include +#include namespace AZ { @@ -179,7 +180,12 @@ namespace AZ const GroupDefinition* FindGroup(AZStd::string_view groupName) const; - const PropertyDefinition* FindProperty(AZStd::string_view groupName, AZStd::string_view propertyName) const; + //! Searches for a specific property. + //! Note this function can find properties using old versions of the property name; in that case, + //! the name in the returned PropertyDefinition* will not match the @propertyName that was searched for. + //! @param materialTypeVersion indicates the version number of the property name being passed in. Only renames above this version number will be applied. + //! @return the requested property, or null if it could not be found + const PropertyDefinition* FindProperty(AZStd::string_view groupName, AZStd::string_view propertyName, uint32_t materialTypeVersion = 0) const; //! Construct a complete list of group definitions, including implicit groups, arranged in the same order as the source data //! Groups with the same name will be consolidated into a single entry @@ -205,6 +211,11 @@ namespace AZ bool ConvertPropertyValueToSourceDataFormat(const PropertyDefinition& propertyDefinition, MaterialPropertyValue& propertyValue) const; Outcome> CreateMaterialTypeAsset(Data::AssetId assetId, AZStd::string_view materialTypeSourceFilePath = "", bool elevateWarnings = true) const; + + //! Possibly renames @propertyId based on the material version update steps. + //! @param materialTypeVersion indicates the version number of the property name being passed in. Only renames above this version number will be applied. + //! @return true if the property was renamed + bool ApplyPropertyRenames(MaterialPropertyId& propertyId, uint32_t materialTypeVersion = 0) const; }; //! The wrapper class for derived material functors. diff --git a/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Material/MaterialAsset.h b/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Material/MaterialAsset.h index e14ff30f87..2a1de6debd 100644 --- a/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Material/MaterialAsset.h +++ b/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Material/MaterialAsset.h @@ -122,7 +122,8 @@ namespace AZ //! from m_materialTypeAsset. void RealignPropertyValuesAndNames(); - //! Renames properties in m_propertyNames based on the MaterialTypeAsset's version update. Note that only version upgrades are supported. + //! Checks the material type version and potentially applies a series of property changes (most common are simple property renames) + //! based on the MaterialTypeAsset's version update procedure. void ApplyVersionUpdates(); //! Called by asset creators to assign the asset to a ready state. diff --git a/Gems/Atom/RPI/Code/Source/RPI.Builders/Material/MaterialBuilder.cpp b/Gems/Atom/RPI/Code/Source/RPI.Builders/Material/MaterialBuilder.cpp index 86ea8ab688..0680c5c428 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Builders/Material/MaterialBuilder.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Builders/Material/MaterialBuilder.cpp @@ -287,6 +287,11 @@ namespace AZ return {}; } + if (!material.GetValue().ApplyVersionUpdates()) + { + return {}; + } + auto materialAssetOutcome = material.GetValue().CreateMaterialAsset(Uuid::CreateRandom(), materialSourceFilePath, true); if (!materialAssetOutcome.IsSuccess()) { diff --git a/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialPropertyValueSerializer.cpp b/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialPropertyValueSerializer.cpp index 3b2d36451a..5e04365ffb 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialPropertyValueSerializer.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialPropertyValueSerializer.cpp @@ -62,6 +62,8 @@ namespace AZ return context.Report(JsonSerializationResult::Tasks::ReadField, JsonSerializationResult::Outcomes::Catastrophic, "Material type reference not found."); } + const JsonMaterialPropertyValueSerializer::LoadContext* loadContext = context.GetMetadata().Find(); + // Construct the full property name (groupName.propertyName) by parsing it from the JSON path string. size_t startPropertyName = context.GetPath().Get().rfind('/'); size_t startGroupName = context.GetPath().Get().rfind('/', startPropertyName-1); @@ -70,7 +72,7 @@ namespace AZ JSR::ResultCode result(JSR::Tasks::ReadField); - auto propertyDefinition = materialType->FindProperty(groupName, propertyName); + auto propertyDefinition = materialType->FindProperty(groupName, propertyName, loadContext->m_materialTypeVersion); if (!propertyDefinition) { AZStd::string message = AZStd::string::format("Property '%.*s.%.*s' not found in material type.", AZ_STRING_ARG(groupName), AZ_STRING_ARG(propertyName)); diff --git a/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialSourceData.cpp b/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialSourceData.cpp index f697f33a3f..e43095f639 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialSourceData.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialSourceData.cpp @@ -72,6 +72,45 @@ namespace AZ materialAssetCreator.SetPropertyValue(propertyId, entry.second); } } + + bool MaterialSourceData::ApplyVersionUpdates() + { + auto materialTypeSourceDataOutcome = MaterialUtils::LoadMaterialTypeSourceData(m_materialType); + if (!materialTypeSourceDataOutcome.IsSuccess()) + { + return false; + } + + MaterialTypeSourceData materialTypeSourceData = materialTypeSourceDataOutcome.TakeValue(); + + // Note that the only kind of property update currently supported is rename... + + for (auto& groupPair : m_properties) + { + PropertyMap& propertyMap = groupPair.second; + + PropertyMap newPropertyMap; + + for (auto& propertyPair : propertyMap) + { + MaterialPropertyId propertyId{groupPair.first, propertyPair.first}; + if (materialTypeSourceData.ApplyPropertyRenames(propertyId, m_materialTypeVersion)) + { + newPropertyMap[propertyId.GetPropertyName().GetStringView()] = propertyPair.second; + } + else + { + newPropertyMap[propertyPair.first] = propertyPair.second; + } + } + + propertyMap = newPropertyMap; + } + + m_materialTypeVersion = materialTypeSourceData.m_version; + + return true; + } Outcome > MaterialSourceData::CreateMaterialAsset(Data::AssetId assetId, AZStd::string_view materialSourceFilePath, bool elevateWarnings, bool includeMaterialPropertyNames) const { diff --git a/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialSourceDataSerializer.cpp b/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialSourceDataSerializer.cpp index 5e4af07aae..cdd434e894 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialSourceDataSerializer.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialSourceDataSerializer.cpp @@ -8,6 +8,7 @@ #include #include +#include #include #include #include @@ -47,7 +48,7 @@ namespace AZ result.Combine(ContinueLoadingFromJsonObjectField(&materialSourceData->m_description, azrtti_typeid(), inputValue, "description", context)); result.Combine(ContinueLoadingFromJsonObjectField(&materialSourceData->m_materialType, azrtti_typeid(), inputValue, "materialType", context)); result.Combine(ContinueLoadingFromJsonObjectField(&materialSourceData->m_parentMaterial, azrtti_typeid(), inputValue, "parentMaterial", context)); - result.Combine(ContinueLoadingFromJsonObjectField(&materialSourceData->m_propertyLayoutVersion, azrtti_typeid(), inputValue, "propertyLayoutVersion", context)); + result.Combine(ContinueLoadingFromJsonObjectField(&materialSourceData->m_materialTypeVersion, azrtti_typeid(), inputValue, "materialTypeVersion", context)); if (materialSourceData->m_materialType.empty()) { @@ -118,6 +119,10 @@ namespace AZ context.GetMetadata().Add(AZStd::move(materialTypeData)); + JsonMaterialPropertyValueSerializer::LoadContext materialPropertyValueLoadContext; + materialPropertyValueLoadContext.m_materialTypeVersion = materialSourceData->m_materialTypeVersion; + context.GetMetadata().Add(materialPropertyValueLoadContext); + result.Combine(ContinueLoadingFromJsonObjectField(&materialSourceData->m_properties, azrtti_typeid(), inputValue, "properties", context)); if (result.GetProcessing() == JsonSerializationResult::Processing::Completed) @@ -148,7 +153,7 @@ namespace AZ resultCode.Combine(ContinueStoringToJsonObjectField(outputValue, "description", &materialSourceData->m_description, nullptr, azrtti_typeid(), context)); resultCode.Combine(ContinueStoringToJsonObjectField(outputValue, "materialType", &materialSourceData->m_materialType, nullptr, azrtti_typeid(), context)); resultCode.Combine(ContinueStoringToJsonObjectField(outputValue, "parentMaterial", &materialSourceData->m_parentMaterial, nullptr, azrtti_typeid(), context)); - resultCode.Combine(ContinueStoringToJsonObjectField(outputValue, "propertyLayoutVersion", &materialSourceData->m_propertyLayoutVersion, nullptr, azrtti_typeid(), context)); + resultCode.Combine(ContinueStoringToJsonObjectField(outputValue, "materialTypeVersion", &materialSourceData->m_materialTypeVersion, nullptr, azrtti_typeid(), context)); resultCode.Combine(ContinueStoringToJsonObjectField(outputValue, "properties", &materialSourceData->m_properties, nullptr, azrtti_typeid(), context)); return context.Report(resultCode, "Processed material."); diff --git a/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialTypeSourceData.cpp b/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialTypeSourceData.cpp index 5e2ceacafc..ea2e499972 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialTypeSourceData.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialTypeSourceData.cpp @@ -129,7 +129,38 @@ namespace AZ return nullptr; } - const MaterialTypeSourceData::PropertyDefinition* MaterialTypeSourceData::FindProperty(AZStd::string_view groupName, AZStd::string_view propertyName) const + bool MaterialTypeSourceData::ApplyPropertyRenames(MaterialPropertyId& propertyId, uint32_t materialTypeVersion) const + { + bool renamed = false; + + for (const VersionUpdateDefinition& versionUpdate : m_versionUpdates) + { + if (materialTypeVersion >= versionUpdate.m_toVersion) + { + continue; + } + + for (const VersionUpdatesRenameOperationDefinition& action : versionUpdate.m_actions) + { + if (action.m_operation == "rename") + { + if (action.m_renameFrom == propertyId.GetFullName().GetStringView()) + { + propertyId = MaterialPropertyId::Parse(action.m_renameTo); + renamed = true; + } + } + else + { + AZ_Warning("Material source data", false, "Unsupported material version update operation '%s'", action.m_operation.c_str()); + } + } + } + + return renamed; + } + + const MaterialTypeSourceData::PropertyDefinition* MaterialTypeSourceData::FindProperty(AZStd::string_view groupName, AZStd::string_view propertyName, uint32_t materialTypeVersion) const { auto groupIter = m_propertyLayout.m_properties.find(groupName); if (groupIter == m_propertyLayout.m_properties.end()) @@ -145,6 +176,27 @@ namespace AZ } } + // Property has not been found, try looking for renames in the version history + + MaterialPropertyId propertyId = MaterialPropertyId{groupName, propertyName}; + ApplyPropertyRenames(propertyId, materialTypeVersion); + + // Do the search again with the new names + + groupIter = m_propertyLayout.m_properties.find(propertyId.GetGroupName().GetStringView()); + if (groupIter == m_propertyLayout.m_properties.end()) + { + return nullptr; + } + + for (const PropertyDefinition& property : groupIter->second) + { + if (property.m_name == propertyId.GetPropertyName().GetStringView()) + { + return &property; + } + } + return nullptr; } @@ -302,17 +354,24 @@ namespace AZ // Set materialtype version and add each version update object into MaterialTypeAsset. materialTypeAssetCreator.SetVersion(m_version); { - const AZ::Name rename = AZ::Name{ "rename" }; - const AZ::Name from = AZ::Name{ "from" }; - const AZ::Name to = AZ::Name{ "to" }; + const AZ::Name rename = AZ::Name{ "rename" }; + const AZ::Name from = AZ::Name{ "from" }; + const AZ::Name to = AZ::Name{ "to" }; for (const auto& versionUpdate : m_versionUpdates) { MaterialVersionUpdate materialVersionUpdate; for (const auto& action : versionUpdate.m_actions) { - materialVersionUpdate.AddAction(MaterialVersionUpdate::Action(rename, { - { from, AZ::Name{ action.m_renameFrom } }, - { to, AZ::Name{ action.m_renameTo } } })); + if (action.m_operation == rename.GetStringView()) + { + materialVersionUpdate.AddAction(MaterialVersionUpdate::Action(rename, { + { from, AZ::Name{ action.m_renameFrom } }, + { to, AZ::Name{ action.m_renameTo } } })); + } + else + { + materialTypeAssetCreator.ReportWarning("Unsupported material version update operation '%s'", action.m_operation.c_str()); + } } materialTypeAssetCreator.AddVersionUpdate(versionUpdate.m_toVersion, materialVersionUpdate); } diff --git a/Gems/Atom/RPI/Code/Tests/Common/AssetSystemStub.cpp b/Gems/Atom/RPI/Code/Tests/Common/AssetSystemStub.cpp index a5511edea1..31a46e9a6f 100644 --- a/Gems/Atom/RPI/Code/Tests/Common/AssetSystemStub.cpp +++ b/Gems/Atom/RPI/Code/Tests/Common/AssetSystemStub.cpp @@ -7,6 +7,7 @@ */ #include +#include namespace UnitTest { @@ -36,12 +37,17 @@ namespace UnitTest // Because GetSourceInfoBySourcePath should always return 0 for the sub-id, since it's about the source file not product file. sourceInfo.m_assetInfo.m_assetId.m_subId = 0; - m_sourceInfoMap.emplace(sourcePath, sourceInfo); + AZStd::string normalizedSourcePath = sourcePath; + AzFramework::StringFunc::Path::Normalize(normalizedSourcePath); + m_sourceInfoMap.emplace(normalizedSourcePath, sourceInfo); } bool AssetSystemStub::GetSourceInfoBySourcePath(const char* sourcePath, AZ::Data::AssetInfo& assetInfo, AZStd::string& watchFolder) { - auto iter = m_sourceInfoMap.find(sourcePath); + AZStd::string normalizedSourcePath = sourcePath; + AzFramework::StringFunc::Path::Normalize(normalizedSourcePath); + + auto iter = m_sourceInfoMap.find(normalizedSourcePath); if (iter != m_sourceInfoMap.end()) { diff --git a/Gems/Atom/RPI/Code/Tests/Material/MaterialSourceDataTests.cpp b/Gems/Atom/RPI/Code/Tests/Material/MaterialSourceDataTests.cpp index dd3b3b2711..a6ce6504fb 100644 --- a/Gems/Atom/RPI/Code/Tests/Material/MaterialSourceDataTests.cpp +++ b/Gems/Atom/RPI/Code/Tests/Material/MaterialSourceDataTests.cpp @@ -17,6 +17,7 @@ #include #include #include +#include #include #include @@ -60,22 +61,72 @@ namespace UnitTest localFileIO->SetAlias("@exefolder@", rootPath); m_testMaterialSrgLayout = CreateCommonTestMaterialSrgLayout(); - m_testShaderAsset = CreateTestShaderAsset(Uuid::CreateRandom(), m_testMaterialSrgLayout); + m_assetSystemStub.RegisterSourceInfo("@exefolder@/Temp/test.shader", m_testShaderAsset.GetId()); + + // The MaterialSourceData relies on both MaterialTypeSourceData and MaterialTypeAsset. We have to make sure the + // .materialtype file is present on disk, and that the MaterialTypeAsset is available through the asset database stub... + + const char* materialTypeJson = R"( + { + "version": 10, + "propertyLayout": { + "properties": { + "general": [ + {"name": "MyBool", "type": "bool"}, + {"name": "MyInt", "type": "Int"}, + {"name": "MyUInt", "type": "UInt"}, + {"name": "MyFloat", "type": "Float"}, + {"name": "MyFloat2", "type": "Vector2"}, + {"name": "MyFloat3", "type": "Vector3"}, + {"name": "MyFloat4", "type": "Vector4"}, + {"name": "MyColor", "type": "Color"}, + {"name": "MyImage", "type": "Image"}, + {"name": "MyEnum", "type": "Enum", "enumValues": ["Enum0", "Enum1", "Enum2"], "defaultValue": "Enum0"} + ] + } + }, + "shaders": [ + { + "file": "@exefolder@/Temp/test.shader" + } + ], + "versionUpdates": [ + { + "toVersion": 2, + "actions": [ + {"op": "rename", "from": "general.testColorNameA", "to": "general.testColorNameB"} + ] + }, + { + "toVersion": 4, + "actions": [ + {"op": "rename", "from": "general.testColorNameB", "to": "general.testColorNameC"} + ] + }, + { + "toVersion": 10, + "actions": [ + {"op": "rename", "from": "general.testColorNameC", "to": "general.MyColor"} + ] + } + ] + } + )"; + + AZ::Utils::WriteFile(materialTypeJson, "@exefolder@/Temp/test.materialtype"); - MaterialTypeAssetCreator materialTypeCreator; - materialTypeCreator.Begin(Uuid::CreateRandom()); - materialTypeCreator.AddShader(m_testShaderAsset); - AddCommonTestMaterialProperties(materialTypeCreator, "general."); - materialTypeCreator.End(m_testMaterialTypeAsset); + MaterialTypeSourceData materialTypeSourceData; + LoadTestDataFromJson(materialTypeSourceData, materialTypeJson); + m_testMaterialTypeAsset = materialTypeSourceData.CreateMaterialTypeAsset(Uuid::CreateRandom()).TakeValue(); // Since this test doesn't actually instantiate a Material, it won't need to instantiate this ImageAsset, so all we // need is an asset reference with a valid ID. m_testImageAsset = Data::Asset{ Data::AssetId{Uuid::CreateRandom(), StreamingImageAsset::GetImageAssetSubId()}, azrtti_typeid() }; // Register the test assets with the AssetSystemStub so CreateMaterialAsset() can use AssetUtils. - m_assetSystemStub.RegisterSourceInfo("test.materialtype", m_testMaterialTypeAsset.GetId()); - m_assetSystemStub.RegisterSourceInfo("test.streamingimage", m_testImageAsset.GetId()); + m_assetSystemStub.RegisterSourceInfo("@exefolder@/Temp/test.materialtype", m_testMaterialTypeAsset.GetId()); + m_assetSystemStub.RegisterSourceInfo("@exefolder@/Temp/test.streamingimage", m_testImageAsset.GetId()); } void TearDown() override @@ -88,12 +139,12 @@ namespace UnitTest RPITestFixture::TearDown(); } }; - + void AddPropertyGroup(MaterialSourceData& material, AZStd::string_view groupName) { material.m_properties.insert(groupName); } - + void AddProperty(MaterialSourceData& material, AZStd::string_view groupName, AZStd::string_view propertyName, const MaterialPropertyValue& anyValue) { material.m_properties[groupName][propertyName].m_value = anyValue; @@ -103,7 +154,7 @@ namespace UnitTest { MaterialSourceData sourceData; - sourceData.m_materialType = "test.materialtype"; + sourceData.m_materialType = "@exefolder@/Temp/test.materialtype"; AddPropertyGroup(sourceData, "general"); AddProperty(sourceData, "general", "MyBool", true); AddProperty(sourceData, "general", "MyInt", -10); @@ -113,7 +164,7 @@ namespace UnitTest AddProperty(sourceData, "general", "MyFloat2", AZ::Vector2(2.1f, 2.2f)); AddProperty(sourceData, "general", "MyFloat3", AZ::Vector3(3.1f, 3.2f, 3.3f)); AddProperty(sourceData, "general", "MyFloat4", AZ::Vector4(4.1f, 4.2f, 4.3f, 4.4f)); - AddProperty(sourceData, "general", "MyImage", AZStd::string("test.streamingimage")); + AddProperty(sourceData, "general", "MyImage", AZStd::string("@exefolder@/Temp/test.streamingimage")); AddProperty(sourceData, "general", "MyEnum", AZStd::string("Enum1")); auto materialAssetOutcome = sourceData.CreateMaterialAsset(Uuid::CreateRandom(), "", true); @@ -139,7 +190,7 @@ namespace UnitTest EXPECT_STREQ(a.m_materialType.data(), b.m_materialType.data()); EXPECT_STREQ(a.m_description.data(), b.m_description.data()); EXPECT_STREQ(a.m_parentMaterial.data(), b.m_parentMaterial.data()); - EXPECT_EQ(a.m_propertyLayoutVersion, b.m_propertyLayoutVersion); + EXPECT_EQ(a.m_materialTypeVersion, b.m_materialTypeVersion); EXPECT_EQ(a.m_properties.size(), b.m_properties.size()); for (auto& groupA : a.m_properties) @@ -170,7 +221,7 @@ namespace UnitTest auto& propertyA = propertyIterA.second; auto& propertyB = propertyIterB->second; - + bool typesMatch = propertyA.m_value.GetTypeId() == propertyB.m_value.GetTypeId(); EXPECT_TRUE(typesMatch); if (typesMatch) @@ -229,8 +280,8 @@ namespace UnitTest " } \n" "} \n"; - const char* materialTypeFilePath = "@exefolder@/Gems/Atom/RPI/Code/Tests/Material/Temp/roundTripTest.materialtype"; - + const char* materialTypeFilePath = "@exefolder@/Temp/roundTripTest.materialtype"; + AZ::IO::FileIOStream file; EXPECT_TRUE(file.Open(materialTypeFilePath, AZ::IO::OpenMode::ModeWrite | AZ::IO::OpenMode::ModeCreatePath)); file.Write(strlen(materialTypeJson), materialTypeJson); @@ -240,7 +291,7 @@ namespace UnitTest sourceDataOriginal.m_materialType = materialTypeFilePath; sourceDataOriginal.m_parentMaterial = materialTypeFilePath; sourceDataOriginal.m_description = "This is a description"; - sourceDataOriginal.m_propertyLayoutVersion = 7; + sourceDataOriginal.m_materialTypeVersion = 7; AddPropertyGroup(sourceDataOriginal, "groupA"); AddProperty(sourceDataOriginal, "groupA", "MyBool", true); AddProperty(sourceDataOriginal, "groupA", "MyInt", -10); @@ -252,14 +303,14 @@ namespace UnitTest AddPropertyGroup(sourceDataOriginal, "groupC"); AddProperty(sourceDataOriginal, "groupC", "MyFloat4", AZ::Vector4(4.1f, 4.2f, 4.3f, 4.4f)); AddProperty(sourceDataOriginal, "groupC", "MyColor", AZ::Color{0.1f, 0.2f, 0.3f, 0.4f}); - AddProperty(sourceDataOriginal, "groupC", "MyImage", AZStd::string("test.streamingimage")); + AddProperty(sourceDataOriginal, "groupC", "MyImage", AZStd::string("@exefolder@/Temp/test.streamingimage")); AZStd::string sourceDataSerialized; JsonTestResult storeResult = StoreTestDataToJson(sourceDataOriginal, sourceDataSerialized); MaterialSourceData sourceDataCopy; JsonTestResult loadResult = LoadTestDataFromJson(sourceDataCopy, sourceDataSerialized); - + CheckEqual(sourceDataOriginal, sourceDataCopy); } @@ -277,10 +328,10 @@ namespace UnitTest ] } } - } + } )"; - const char* materialTypeFilePath = "@exefolder@/Gems/Atom/RPI/Code/Tests/Material/Temp/simpleMaterialType.materialtype"; + const char* materialTypeFilePath = "@exefolder@/Temp/simpleMaterialType.materialtype"; AZ::IO::FileIOStream file; EXPECT_TRUE(file.Open(materialTypeFilePath, AZ::IO::OpenMode::ModeWrite | AZ::IO::OpenMode::ModeCreatePath)); @@ -296,7 +347,7 @@ namespace UnitTest "testColor": [0.1,0.2,0.3] } }, - "materialType": "@exefolder@/Gems/Atom/RPI/Code/Tests/Material/Temp/simpleMaterialType.materialtype" + "materialType": "@exefolder@/Temp/simpleMaterialType.materialtype" } )"; @@ -330,7 +381,7 @@ namespace UnitTest { const AZStd::string inputJson = R"( { - "propertyLayoutVersion": 1, + "materialTypeVersion": 1, "properties": { "baseColor": { "color": [1.0,1.0,1.0] @@ -354,7 +405,7 @@ namespace UnitTest const AZStd::string inputJson = R"( { "materialType": "DoesNotExist.materialtype", - "propertyLayoutVersion": 1, + "materialTypeVersion": 1, "properties": { "baseColor": { "color": [1.0,1.0,1.0] @@ -387,10 +438,10 @@ namespace UnitTest ] } } - } + } )"; - const char* materialTypeFilePath = "@exefolder@/Gems/Atom/RPI/Code/Tests/Material/Temp/simpleMaterialType.materialtype"; + const char* materialTypeFilePath = "@exefolder@/Temp/simpleMaterialType.materialtype"; AZ::IO::FileIOStream file; EXPECT_TRUE(file.Open(materialTypeFilePath, AZ::IO::OpenMode::ModeWrite | AZ::IO::OpenMode::ModeCreatePath)); @@ -399,8 +450,8 @@ namespace UnitTest const AZStd::string inputJson = R"( { - "materialType": "@exefolder@/Gems/Atom/RPI/Code/Tests/Material/Temp/simpleMaterialType.materialtype", - "propertyLayoutVersion": 1, + "materialType": "@exefolder@/Temp/simpleMaterialType.materialtype", + "materialTypeVersion": 1, "properties": { "general": { "testColor": [1.0,1.0,1.0] @@ -433,10 +484,10 @@ namespace UnitTest ] } } - } + } )"; - const char* materialTypeFilePath = "@exefolder@/Gems/Atom/RPI/Code/Tests/Material/Temp/simpleMaterialType.materialtype"; + const char* materialTypeFilePath = "@exefolder@/Temp/simpleMaterialType.materialtype"; AZ::IO::FileIOStream file; EXPECT_TRUE(file.Open(materialTypeFilePath, AZ::IO::OpenMode::ModeWrite | AZ::IO::OpenMode::ModeCreatePath)); @@ -445,8 +496,8 @@ namespace UnitTest const AZStd::string inputJson = R"( { - "materialType": "@exefolder@/Gems/Atom/RPI/Code/Tests/Material/Temp/simpleMaterialType.materialtype", - "propertyLayoutVersion": 1, + "materialType": "@exefolder@/Temp/simpleMaterialType.materialtype", + "materialTypeVersion": 1, "properties": { "general": { "doesNotExist": [1.0,1.0,1.0] @@ -467,20 +518,20 @@ namespace UnitTest TEST_F(MaterialSourceDataTests, CreateMaterialAsset_MultiLevelDataInheritance) { MaterialSourceData sourceDataLevel1; - sourceDataLevel1.m_materialType = "test.materialtype"; + sourceDataLevel1.m_materialType = "@exefolder@/Temp/test.materialtype"; AddPropertyGroup(sourceDataLevel1, "general"); AddProperty(sourceDataLevel1, "general", "MyFloat", 1.5f); AddProperty(sourceDataLevel1, "general", "MyColor", AZ::Color{0.1f, 0.2f, 0.3f, 0.4f}); MaterialSourceData sourceDataLevel2; - sourceDataLevel2.m_materialType = "test.materialtype"; + sourceDataLevel2.m_materialType = "@exefolder@/Temp/test.materialtype"; sourceDataLevel2.m_parentMaterial = "level1.material"; AddPropertyGroup(sourceDataLevel2, "general"); AddProperty(sourceDataLevel2, "general", "MyColor", AZ::Color{0.15f, 0.25f, 0.35f, 0.45f}); AddProperty(sourceDataLevel2, "general", "MyFloat2", AZ::Vector2{4.1f, 4.2f}); MaterialSourceData sourceDataLevel3; - sourceDataLevel3.m_materialType = "test.materialtype"; + sourceDataLevel3.m_materialType = "@exefolder@/Temp/test.materialtype"; sourceDataLevel3.m_parentMaterial = "level2.material"; AddPropertyGroup(sourceDataLevel3, "general"); AddProperty(sourceDataLevel3, "general", "MyFloat", 3.5f); @@ -497,7 +548,7 @@ namespace UnitTest auto materialAssetLevel3 = sourceDataLevel3.CreateMaterialAsset(Uuid::CreateRandom(), "", true); EXPECT_TRUE(materialAssetLevel3.IsSuccess()); - + auto layout = m_testMaterialTypeAsset->GetMaterialPropertiesLayout(); MaterialPropertyIndex myFloat = layout->FindPropertyIndex(Name("general.MyFloat")); MaterialPropertyIndex myFloat2 = layout->FindPropertyIndex(Name("general.MyFloat2")); @@ -535,14 +586,14 @@ namespace UnitTest m_assetSystemStub.RegisterSourceInfo("otherBase.materialtype", otherMaterialType.GetId()); MaterialSourceData sourceDataLevel1; - sourceDataLevel1.m_materialType = "test.materialtype"; + sourceDataLevel1.m_materialType = "@exefolder@/Temp/test.materialtype"; MaterialSourceData sourceDataLevel2; - sourceDataLevel2.m_materialType = "test.materialtype"; + sourceDataLevel2.m_materialType = "@exefolder@/Temp/test.materialtype"; sourceDataLevel2.m_parentMaterial = "level1.material"; MaterialSourceData sourceDataLevel3; - sourceDataLevel3.m_materialType = "otherBase.materialtype"; + sourceDataLevel3.m_materialType = "@exefolder@/Temp/otherBase.materialtype"; sourceDataLevel3.m_parentMaterial = "level2.material"; auto materialAssetLevel1 = sourceDataLevel1.CreateMaterialAsset(Uuid::CreateRandom(), "", true); @@ -570,7 +621,7 @@ namespace UnitTest { MaterialSourceData sourceData; - sourceData.m_materialType = "test.materialtype"; + sourceData.m_materialType = "@exefolder@/Temp/test.materialtype"; AddPropertyGroup(sourceData, "general"); @@ -587,7 +638,7 @@ namespace UnitTest { MaterialSourceData sourceData; - sourceData.m_materialType = "test.materialtype"; + sourceData.m_materialType = "@exefolder@/Temp/test.materialtype"; AddPropertyGroup(sourceData, "general"); @@ -629,7 +680,7 @@ namespace UnitTest expectWarning([](MaterialSourceData& materialSourceData) { - AddProperty(materialSourceData, "general", "DoesNotExist", AZStd::string("test.streamingimage")); + AddProperty(materialSourceData, "general", "DoesNotExist", AZStd::string("@exefolder@/Temp/test.streamingimage")); }); // Missing image reference @@ -638,6 +689,115 @@ namespace UnitTest AddProperty(materialSourceData, "general", "MyImage", AZStd::string("doesNotExist.streamingimage")); }, 3); // Expect a 3rd error because AssetUtils reports its own assertion failure } + + + TEST_F(MaterialSourceDataTests, Load_MaterialTypeVersionUpdate) + { + const AZStd::string inputJson = R"( + { + "materialType": "@exefolder@/Temp/test.materialtype", + "materialTypeVersion": 1, + "properties": { + "general": { + "testColorNameA": [0.1, 0.2, 0.3] + } + } + } + )"; + + MaterialSourceData material; + JsonTestResult loadResult = LoadTestDataFromJson(material, inputJson); + + EXPECT_EQ(AZ::JsonSerializationResult::Tasks::ReadField, loadResult.m_jsonResultCode.GetTask()); + EXPECT_EQ(AZ::JsonSerializationResult::Processing::Completed, loadResult.m_jsonResultCode.GetProcessing()); + + // Initially, the loaded material data will match the .material file exactly. This gives us the accurate representation of + // what's actually saved on disk. + + EXPECT_NE(material.m_properties["general"].find("testColorNameA"), material.m_properties["general"].end()); + EXPECT_EQ(material.m_properties["general"].find("testColorNameB"), material.m_properties["general"].end()); + EXPECT_EQ(material.m_properties["general"].find("testColorNameC"), material.m_properties["general"].end()); + EXPECT_EQ(material.m_properties["general"].find("MyColor"), material.m_properties["general"].end()); + + AZ::Color testColor = material.m_properties["general"]["testColorNameA"].m_value.GetValue(); + EXPECT_TRUE(AZ::Color(0.1f, 0.2f, 0.3f, 1.0f).IsClose(testColor, 0.01)); + + EXPECT_EQ(1, material.m_materialTypeVersion); + + // Then we force the material data to update to the latest material type version specification + material.ApplyVersionUpdates(); + + // Now the material data should match the latest material type. + // Look for the property under the latest name in the material type, not the name used in the .material file. + + EXPECT_EQ(material.m_properties["general"].find("testColorNameA"), material.m_properties["general"].end()); + EXPECT_EQ(material.m_properties["general"].find("testColorNameB"), material.m_properties["general"].end()); + EXPECT_EQ(material.m_properties["general"].find("testColorNameC"), material.m_properties["general"].end()); + EXPECT_NE(material.m_properties["general"].find("MyColor"), material.m_properties["general"].end()); + + testColor = material.m_properties["general"]["MyColor"].m_value.GetValue(); + EXPECT_TRUE(AZ::Color(0.1f, 0.2f, 0.3f, 1.0f).IsClose(testColor, 0.01)); + + EXPECT_EQ(10, material.m_materialTypeVersion); + } + + TEST_F(MaterialSourceDataTests, Load_MaterialTypeVersionPartialUpdate) + { + // This case is similar to Load_MaterialTypeVersionUpdate but we start at a later + // version so only some of the version updates are applied. + + const AZStd::string inputJson = R"( + { + "materialType": "@exefolder@/Temp/test.materialtype", + "materialTypeVersion": 3, + "properties": { + "general": { + "testColorNameB": [0.1, 0.2, 0.3] + } + } + } + )"; + + MaterialSourceData material; + JsonTestResult loadResult = LoadTestDataFromJson(material, inputJson); + + EXPECT_EQ(AZ::JsonSerializationResult::Tasks::ReadField, loadResult.m_jsonResultCode.GetTask()); + EXPECT_EQ(AZ::JsonSerializationResult::Processing::Completed, loadResult.m_jsonResultCode.GetProcessing()); + + material.ApplyVersionUpdates(); + + AZ::Color testColor = material.m_properties["general"]["MyColor"].m_value.GetValue(); + EXPECT_TRUE(AZ::Color(0.1f, 0.2f, 0.3f, 1.0f).IsClose(testColor, 0.01)); + + EXPECT_EQ(10, material.m_materialTypeVersion); + } + + TEST_F(MaterialSourceDataTests, Load_Error_MaterialTypeVersionUpdateWithMismatchedVersion) + { + const AZStd::string inputJson = R"( + { + "materialType": "@exefolder@/Temp/test.materialtype", + "materialTypeVersion": 3, // At this version, the property should be testColorNameB not testColorNameA + "properties": { + "general": { + "testColorNameA": [0.1, 0.2, 0.3] + } + } + } + )"; + + MaterialSourceData material; + JsonTestResult loadResult = LoadTestDataFromJson(material, inputJson); + + loadResult.ContainsMessage("/properties/general/testColorNameA", "Property 'general.testColorNameA' not found in material type."); + + EXPECT_FALSE(material.m_properties["general"]["testColorNameA"].m_value.IsValid()); + + material.ApplyVersionUpdates(); + + EXPECT_FALSE(material.m_properties["general"]["MyColor"].m_value.IsValid()); + } + } diff --git a/Gems/Atom/RPI/Code/Tests/Material/MaterialTypeSourceDataTests.cpp b/Gems/Atom/RPI/Code/Tests/Material/MaterialTypeSourceDataTests.cpp index 778fca5402..709d6b84ec 100644 --- a/Gems/Atom/RPI/Code/Tests/Material/MaterialTypeSourceDataTests.cpp +++ b/Gems/Atom/RPI/Code/Tests/Material/MaterialTypeSourceDataTests.cpp @@ -10,6 +10,7 @@ #include #include #include +#include #include #include @@ -1070,7 +1071,12 @@ namespace UnitTest EXPECT_EQ(material.m_description, "This is a general description about the material"); - EXPECT_EQ(material.m_propertyLayout.m_version, 2); + EXPECT_EQ(material.m_version, 2); + EXPECT_EQ(material.m_versionUpdates.size(), 1); + EXPECT_EQ(material.m_versionUpdates[0].m_toVersion, 2); + EXPECT_EQ(material.m_versionUpdates[0].m_actions[0].m_operation, "rename"); + EXPECT_EQ(material.m_versionUpdates[0].m_actions[0].m_renameFrom, "groupA.fooPrev"); + EXPECT_EQ(material.m_versionUpdates[0].m_actions[0].m_renameTo, "groupA.foo"); EXPECT_EQ(material.m_propertyLayout.m_groups.size(), 2); EXPECT_TRUE(material.FindGroup("groupA") != nullptr); @@ -1215,12 +1221,7 @@ namespace UnitTest JsonTestResult loadResult = LoadTestDataFromJson(material, inputJson); EXPECT_EQ(material.m_description, "This is a general description about the material"); - EXPECT_EQ(material.m_version, 2); - EXPECT_EQ(material.m_versionUpdates.size(), 1); - EXPECT_EQ(material.m_versionUpdates[0].m_toVersion, 2); - EXPECT_EQ(material.m_versionUpdates[0].m_actions[0].m_operation, "rename"); - EXPECT_EQ(material.m_versionUpdates[0].m_actions[0].m_renameFrom, "groupA.fooPrev"); - EXPECT_EQ(material.m_versionUpdates[0].m_actions[0].m_renameTo, "groupA.foo"); + EXPECT_EQ(material.m_propertyLayout.m_groups.size(), 2); EXPECT_TRUE(material.FindGroup("groupA") != nullptr); EXPECT_TRUE(material.FindGroup("groupB") != nullptr); @@ -1277,7 +1278,6 @@ namespace UnitTest { "description": "", "propertyLayout": { - "version": 2, "groups": [ { "name": "general", @@ -1316,4 +1316,165 @@ namespace UnitTest CheckPropertyValue>(materialTypeAsset, Name{ "general.absolute" }, m_testImageAsset2); CheckPropertyValue>(materialTypeAsset, Name{ "general.relative" }, m_testImageAsset2); } + + + TEST_F(MaterialTypeSourceDataTests, FindPropertyUsingOldName) + { + const AZStd::string inputJson = R"( + { + "version": 10, + "versionUpdates": [ + { + "toVersion": 2, + "actions": [ + { "op": "rename", "from": "general.fooA", "to": "general.fooB" } + ] + }, + { + "toVersion": 4, + "actions": [ + { "op": "rename", "from": "general.barA", "to": "general.barB" } + ] + }, + { + "toVersion": 6, + "actions": [ + { "op": "rename", "from": "general.fooB", "to": "general.fooC" }, + { "op": "rename", "from": "general.barB", "to": "general.barC" } + ] + }, + { + "toVersion": 7, + "actions": [ + { "op": "rename", "from": "general.bazA", "to": "otherGroup.bazB" } + ] + } + ], + "propertyLayout": { + "properties": { + "general": [ + { + "name": "fooC", + "type": "Bool" + }, + { + "name": "barC", + "type": "Float" + } + ], + "otherGroup": [ + { + "name": "dontMindMe", + "type": "Bool" + }, + { + "name": "bazB", + "type": "Float" + } + ] + } + } + } + )"; + + MaterialTypeSourceData materialType; + JsonTestResult loadResult = LoadTestDataFromJson(materialType, inputJson); + + EXPECT_EQ(materialType.m_version, 10); + + // First find the properties using their correct current names + const MaterialTypeSourceData::PropertyDefinition* foo = materialType.FindProperty("general", "fooC"); + const MaterialTypeSourceData::PropertyDefinition* bar = materialType.FindProperty("general", "barC"); + const MaterialTypeSourceData::PropertyDefinition* baz = materialType.FindProperty("otherGroup", "bazB"); + + EXPECT_TRUE(foo); + EXPECT_TRUE(bar); + EXPECT_TRUE(baz); + EXPECT_EQ(foo->m_name, "fooC"); + EXPECT_EQ(bar->m_name, "barC"); + EXPECT_EQ(baz->m_name, "bazB"); + + // Now try doing the property lookup using old versions of the name and make sure the same property can be found + + EXPECT_EQ(foo, materialType.FindProperty("general", "fooA")); + EXPECT_EQ(foo, materialType.FindProperty("general", "fooB")); + EXPECT_EQ(bar, materialType.FindProperty("general", "barA")); + EXPECT_EQ(bar, materialType.FindProperty("general", "barB")); + EXPECT_EQ(baz, materialType.FindProperty("general", "bazA")); + + EXPECT_EQ(nullptr, materialType.FindProperty("general", "fooX")); + EXPECT_EQ(nullptr, materialType.FindProperty("general", "barX")); + EXPECT_EQ(nullptr, materialType.FindProperty("general", "bazX")); + EXPECT_EQ(nullptr, materialType.FindProperty("general", "bazB")); + EXPECT_EQ(nullptr, materialType.FindProperty("otherGroup", "bazA")); + } + + TEST_F(MaterialTypeSourceDataTests, FindPropertyUsingOldName_Error_UnsupportedVersionUpdate) + { + const AZStd::string inputJson = R"( + { + "version": 10, + "versionUpdates": [ + { + "toVersion": 2, + "actions": [ + { "op": "notRename", "from": "general.fooA", "to": "general.fooB" } + ] + } + ], + "propertyLayout": { + "properties": { + "general": [ + { + "name": "fooB", + "type": "Bool" + } + ] + } + } + } + )"; + + MaterialTypeSourceData materialType; + JsonTestResult loadResult = LoadTestDataFromJson(materialType, inputJson); + + ErrorMessageFinder errorMessageFinder; + errorMessageFinder.AddExpectedErrorMessage("Unsupported material version update operation 'notRename'"); + + + const MaterialTypeSourceData::PropertyDefinition* foo = materialType.FindProperty("general", "fooA"); + + EXPECT_EQ(nullptr, foo); + + errorMessageFinder.CheckExpectedErrorsFound(); + } + + TEST_F(MaterialTypeSourceDataTests, CreateMaterialTypeAsset_Error_UnsupportedVersionUpdate) + { + MaterialTypeSourceData sourceData; + + MaterialTypeSourceData::PropertyDefinition propertySource; + propertySource.m_name = "a"; + propertySource.m_dataType = MaterialPropertyDataType::Int; + propertySource.m_value = 0; + sourceData.m_propertyLayout.m_properties["general"].push_back(propertySource); + + sourceData.m_version = 2; + + MaterialTypeSourceData::VersionUpdateDefinition versionUpdate; + versionUpdate.m_toVersion = 2; + MaterialTypeSourceData::VersionUpdatesRenameOperationDefinition updateAction; + updateAction.m_operation = "operationNotKnown"; + versionUpdate.m_actions.push_back(updateAction); + sourceData.m_versionUpdates.push_back(versionUpdate); + + ErrorMessageFinder errorMessageFinder; + errorMessageFinder.AddExpectedErrorMessage("Unsupported material version update operation 'operationNotKnown'"); + errorMessageFinder.AddIgnoredErrorMessage("Failed to build MaterialTypeAsset", true); + + auto materialTypeOutcome = sourceData.CreateMaterialTypeAsset(Uuid::CreateRandom()); + EXPECT_FALSE(materialTypeOutcome.IsSuccess()); + + errorMessageFinder.CheckExpectedErrorsFound(); + } } diff --git a/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.cpp b/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.cpp index 2aca8ce1f2..51164a70f4 100644 --- a/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.cpp +++ b/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.cpp @@ -232,6 +232,9 @@ namespace MaterialEditor MaterialSourceData sourceData; sourceData.m_materialType = m_materialSourceData.m_materialType; sourceData.m_parentMaterial = m_materialSourceData.m_parentMaterial; + + AZ_Assert(m_materialAsset && m_materialAsset->GetMaterialTypeAsset(), "When IsOpen() is true, these assets should not be null."); + sourceData.m_materialTypeVersion = m_materialAsset->GetMaterialTypeAsset()->GetVersion(); // Force save data to store forward slashes AzFramework::StringFunc::Replace(sourceData.m_materialType, "\\", "/"); @@ -303,6 +306,9 @@ namespace MaterialEditor MaterialSourceData sourceData; sourceData.m_materialType = m_materialSourceData.m_materialType; sourceData.m_parentMaterial = m_materialSourceData.m_parentMaterial; + + AZ_Assert(m_materialAsset && m_materialAsset->GetMaterialTypeAsset(), "When IsOpen() is true, these assets should not be null."); + sourceData.m_materialTypeVersion = m_materialAsset->GetMaterialTypeAsset()->GetVersion(); // Force save data to store forward slashes AzFramework::StringFunc::Replace(sourceData.m_materialType, "\\", "/"); @@ -372,6 +378,9 @@ namespace MaterialEditor // create source data from properties MaterialSourceData sourceData; sourceData.m_materialType = m_materialSourceData.m_materialType; + + AZ_Assert(m_materialAsset && m_materialAsset->GetMaterialTypeAsset(), "When IsOpen() is true, these assets should not be null."); + sourceData.m_materialTypeVersion = m_materialAsset->GetMaterialTypeAsset()->GetVersion(); // Only assign a parent path if the source was a .material if (AzFramework::StringFunc::Path::IsExtension(m_relativePath.c_str(), MaterialSourceData::Extension))