From c6b209ace0d24ff6fa1bc1d2f4a5c4ce04fbd90b Mon Sep 17 00:00:00 2001 From: santorac <55155825+santorac@users.noreply.github.com> Date: Thu, 21 Oct 2021 23:18:08 -0700 Subject: [PATCH 1/3] Made material property auto-rename procedure apply to Material Component at runtime. This ensures that an material property overrides and any gameplay scripts that work with property overrides can get the benefit of the material type version update procedure. I added an ApplyPropertyRenames function to MaterialTypeAsset very similar to the one in MaterialTypeSourceData. Updated the MaterialAssignment class to apply any property renames when it discovers the old name doesn't work. This will be written to disk when the level or prefab is saved. Signed-off-by: santorac <55155825+santorac@users.noreply.github.com> --- .../Materials/MinimalBlue.material | 17 ++++ .../Source/Material/MaterialAssignment.cpp | 33 +++++++- .../RPI.Reflect/Material/MaterialTypeAsset.h | 4 + .../Material/MaterialVersionUpdate.h | 4 + .../Material/MaterialTypeAsset.cpp | 16 ++++ .../Material/MaterialVersionUpdate.cpp | 16 ++++ .../Tests/Material/MaterialTypeAssetTests.cpp | 83 +++++++++++++++++++ 7 files changed, 172 insertions(+), 1 deletion(-) create mode 100644 AutomatedTesting/Materials/MinimalBlue.material diff --git a/AutomatedTesting/Materials/MinimalBlue.material b/AutomatedTesting/Materials/MinimalBlue.material new file mode 100644 index 0000000000..7310a90250 --- /dev/null +++ b/AutomatedTesting/Materials/MinimalBlue.material @@ -0,0 +1,17 @@ +{ + "description": "", + "parentMaterial": "", + "materialType": "TestData/Materials/Types/MinimalPBR.materialtype", + "materialTypeVersion": 3, + "properties": { + "settings": { + "color": [ + 0.08522164076566696, + 0.11898985505104065, + 1.0, + 1.0 + ], + "roughness": 0.33000001311302185 + } + } +} \ No newline at end of file diff --git a/Gems/Atom/Feature/Common/Code/Source/Material/MaterialAssignment.cpp b/Gems/Atom/Feature/Common/Code/Source/Material/MaterialAssignment.cpp index 8984ec160b..f8a8d17c55 100644 --- a/Gems/Atom/Feature/Common/Code/Source/Material/MaterialAssignment.cpp +++ b/Gems/Atom/Feature/Common/Code/Source/Material/MaterialAssignment.cpp @@ -137,11 +137,33 @@ namespace AZ if (m_materialInstance->CanCompile()) { + AZStd::vector> renamedProperties; + for (const auto& propertyPair : m_propertyOverrides) { if (!propertyPair.second.empty()) { - const auto& materialPropertyIndex = m_materialInstance->FindPropertyIndex(propertyPair.first); + RPI::MaterialPropertyIndex materialPropertyIndex = m_materialInstance->FindPropertyIndex(propertyPair.first); + + // If we didn't find the name, check to see if there was a rename that should be used. + if (materialPropertyIndex.IsNull()) + { + Name propertyId = propertyPair.first; + + if (m_materialInstance->GetAsset()->GetMaterialTypeAsset()->ApplyPropertyRenames(propertyId)) + { + renamedProperties.emplace_back(propertyPair.first, propertyId); + + // We should be able to find the property now, using the new name + materialPropertyIndex = m_materialInstance->FindPropertyIndex(propertyId); + + AZ_Warning("MaterialAssignment", false, + "Material property '%s' has been renamed to '%s', and a material override references the old name. Save the level or prefab to permanently apply this change.", + propertyPair.first.GetCStr(), + propertyId.GetCStr()); + } + } + if (!materialPropertyIndex.IsNull()) { m_materialInstance->SetPropertyValue( @@ -150,6 +172,15 @@ namespace AZ } } + // Now that we're done looping over all the overrides, it's safe to apply any renames that were scheduled + for (auto& pair : renamedProperties) + { + const Name& oldName = pair.first; + const Name& newName = pair.second; + m_propertyOverrides[newName] = m_propertyOverrides[oldName]; + m_propertyOverrides.erase(oldName); + } + return m_materialInstance->Compile(); } diff --git a/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Material/MaterialTypeAsset.h b/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Material/MaterialTypeAsset.h index 9bc0e018d3..f194d84263 100644 --- a/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Material/MaterialTypeAsset.h +++ b/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Material/MaterialTypeAsset.h @@ -129,6 +129,10 @@ namespace AZ const AZStd::vector& GetMaterialVersionUpdateList() const { return m_materialVersionUpdates; } + //! Possibly renames @propertyId based on the material version update steps. + //! @return true if the property was renamed + bool ApplyPropertyRenames(AZ::Name& propertyId) const; + private: bool PostLoadInit() override; diff --git a/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Material/MaterialVersionUpdate.h b/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Material/MaterialVersionUpdate.h index eb71ad9cb7..3ecfe7b5e0 100644 --- a/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Material/MaterialVersionUpdate.h +++ b/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Material/MaterialVersionUpdate.h @@ -44,6 +44,10 @@ namespace AZ uint32_t GetVersion() const; void SetVersion(uint32_t toVersion); + + //! Possibly renames @propertyId based on the material version update steps. + //! @return true if the property was renamed + bool ApplyPropertyRenames(AZ::Name& propertyId) const; //! Apply version updates to the given material asset. //! @return true if any changes were made diff --git a/Gems/Atom/RPI/Code/Source/RPI.Reflect/Material/MaterialTypeAsset.cpp b/Gems/Atom/RPI/Code/Source/RPI.Reflect/Material/MaterialTypeAsset.cpp index 522ba74119..76634201eb 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Reflect/Material/MaterialTypeAsset.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Reflect/Material/MaterialTypeAsset.cpp @@ -169,6 +169,22 @@ namespace AZ return m_version; } + + bool MaterialTypeAsset::ApplyPropertyRenames(AZ::Name& propertyId) const + { + bool renamed = false; + + for (const auto& versionUpdates : m_materialVersionUpdates) + { + if (versionUpdates.ApplyPropertyRenames(propertyId)) + { + renamed = true; + } + } + + return renamed; + } + void MaterialTypeAsset::SetReady() { m_status = AssetStatus::Ready; diff --git a/Gems/Atom/RPI/Code/Source/RPI.Reflect/Material/MaterialVersionUpdate.cpp b/Gems/Atom/RPI/Code/Source/RPI.Reflect/Material/MaterialVersionUpdate.cpp index b387e502e2..f5dfe9e80c 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Reflect/Material/MaterialVersionUpdate.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Reflect/Material/MaterialVersionUpdate.cpp @@ -56,6 +56,22 @@ namespace AZ { m_toVersion = toVersion; } + + bool MaterialVersionUpdate::ApplyPropertyRenames(AZ::Name& propertyId) const + { + bool renamed = false; + + for (const auto& action : m_actions) + { + if (action.m_fromPropertyId == propertyId) + { + propertyId = action.m_toPropertyId; + renamed = true; + } + } + + return renamed; + } bool MaterialVersionUpdate::ApplyVersionUpdates(MaterialAsset& materialAsset) const { diff --git a/Gems/Atom/RPI/Code/Tests/Material/MaterialTypeAssetTests.cpp b/Gems/Atom/RPI/Code/Tests/Material/MaterialTypeAssetTests.cpp index 81b723ec04..382ab4e149 100644 --- a/Gems/Atom/RPI/Code/Tests/Material/MaterialTypeAssetTests.cpp +++ b/Gems/Atom/RPI/Code/Tests/Material/MaterialTypeAssetTests.cpp @@ -1041,5 +1041,88 @@ namespace UnitTest EXPECT_FALSE(materialTypeAsset->GetShaderCollection()[1].MaterialOwnsShaderOption(Name{"o_globalOption_inShaderB"})); } + TEST_F(MaterialTypeAssetTests, ApplyPropertyRenames) + { + Data::Asset materialTypeAsset; + + auto addRenameAction = [](MaterialVersionUpdate& versionUpdate, const char* from, const char* to) + { + versionUpdate.AddAction(MaterialVersionUpdate::RenamePropertyAction( + { + Name{ from }, + Name{ to } + })); + }; + + MaterialTypeAssetCreator materialTypeCreator; + materialTypeCreator.Begin(Uuid::CreateRandom()); + + // Version updates + materialTypeCreator.SetVersion(10); + + MaterialVersionUpdate versionUpdate2(2); + addRenameAction(versionUpdate2, "general.fooA", "general.fooB"); + materialTypeCreator.AddVersionUpdate(versionUpdate2); + + MaterialVersionUpdate versionUpdate4(4); + addRenameAction(versionUpdate4, "general.barA", "general.barB"); + materialTypeCreator.AddVersionUpdate(versionUpdate4); + + MaterialVersionUpdate versionUpdate6(6); + addRenameAction(versionUpdate6, "general.fooB", "general.fooC"); + addRenameAction(versionUpdate6, "general.barB", "general.barC"); + materialTypeCreator.AddVersionUpdate(versionUpdate6); + + MaterialVersionUpdate versionUpdate7(7); + addRenameAction(versionUpdate7, "general.bazA", "otherGroup.bazB"); + materialTypeCreator.AddVersionUpdate(versionUpdate7); + + materialTypeCreator.BeginMaterialProperty(Name{ "general.fooC" }, MaterialPropertyDataType::Bool); + materialTypeCreator.EndMaterialProperty(); + materialTypeCreator.BeginMaterialProperty(Name{ "general.barC" }, MaterialPropertyDataType::Bool); + materialTypeCreator.EndMaterialProperty(); + materialTypeCreator.BeginMaterialProperty(Name{ "otherGroup.bazB" }, MaterialPropertyDataType::Bool); + materialTypeCreator.EndMaterialProperty(); + + EXPECT_TRUE(materialTypeCreator.End(materialTypeAsset)); + + AZ::Name propertyId; + + propertyId = AZ::Name{"doesNotExist"}; + EXPECT_FALSE(materialTypeAsset->ApplyPropertyRenames(propertyId)); + EXPECT_STREQ(propertyId.GetCStr(), "doesNotExist"); + + propertyId = AZ::Name{"general.fooA"}; + EXPECT_TRUE(materialTypeAsset->ApplyPropertyRenames(propertyId)); + EXPECT_STREQ(propertyId.GetCStr(), "general.fooC"); + + propertyId = AZ::Name{"general.fooB"}; + EXPECT_TRUE(materialTypeAsset->ApplyPropertyRenames(propertyId)); + EXPECT_STREQ(propertyId.GetCStr(), "general.fooC"); + + propertyId = AZ::Name{"general.fooC"}; + EXPECT_FALSE(materialTypeAsset->ApplyPropertyRenames(propertyId)); + EXPECT_STREQ(propertyId.GetCStr(), "general.fooC"); + + propertyId = AZ::Name{"general.barA"}; + EXPECT_TRUE(materialTypeAsset->ApplyPropertyRenames(propertyId)); + EXPECT_STREQ(propertyId.GetCStr(), "general.barC"); + + propertyId = AZ::Name{"general.barB"}; + EXPECT_TRUE(materialTypeAsset->ApplyPropertyRenames(propertyId)); + EXPECT_STREQ(propertyId.GetCStr(), "general.barC"); + + propertyId = AZ::Name{"general.barC"}; + EXPECT_FALSE(materialTypeAsset->ApplyPropertyRenames(propertyId)); + EXPECT_STREQ(propertyId.GetCStr(), "general.barC"); + + propertyId = AZ::Name{"general.bazA"}; + EXPECT_TRUE(materialTypeAsset->ApplyPropertyRenames(propertyId)); + EXPECT_STREQ(propertyId.GetCStr(), "otherGroup.bazB"); + + propertyId = AZ::Name{"otherGroup.bazB"}; + EXPECT_FALSE(materialTypeAsset->ApplyPropertyRenames(propertyId)); + EXPECT_STREQ(propertyId.GetCStr(), "otherGroup.bazB"); + } } From 3c331e00ffedc7a69d15bb969eb31abbfbbfa3d5 Mon Sep 17 00:00:00 2001 From: santorac <55155825+santorac@users.noreply.github.com> Date: Fri, 22 Oct 2021 16:41:44 -0700 Subject: [PATCH 2/3] Moved the Material Component property override renaming to EditorMaterialComponent via ApplyAutomaticPropertyUpdates. MaterialAssignment::ApplyProperties() still reports warnings but does not update the m_propertyOverrides. MaterialAssignment::ApplyProperties() will now skip the old name'd overrides if overrides are present for the new names. I'm not sure if this will ever happen, but it did happen while I had some intermediate changes, so I imagine it could happen again. I had to update the Material::FindPropertyIndex function to expose information about renames when they occur. This should make it easier for other systems to get (somewhat) automatic benefit from the version update feature. I also found that there was an issue with material inspector where it wouldn't be initialized the the right override values when renames were present. Now it applies the renames to whatever override data it gets from the Material Component. Signed-off-by: santorac <55155825+santorac@users.noreply.github.com> --- .../Source/Material/MaterialAssignment.cpp | 44 +++++++------------ .../Atom/RPI.Public/Material/Material.h | 4 +- .../Source/RPI.Public/Material/Material.cpp | 34 +++++++++++++- .../RPI/Code/Tests/Material/MaterialTests.cpp | 31 +++++++++++++ .../Material/MaterialComponentBus.h | 3 ++ .../Material/EditorMaterialComponent.cpp | 13 ++++++ .../EditorMaterialComponentInspector.cpp | 21 +++++++++ .../Material/MaterialComponentController.cpp | 34 ++++++++++++++ .../Material/MaterialComponentController.h | 1 + 9 files changed, 154 insertions(+), 31 deletions(-) diff --git a/Gems/Atom/Feature/Common/Code/Source/Material/MaterialAssignment.cpp b/Gems/Atom/Feature/Common/Code/Source/Material/MaterialAssignment.cpp index f8a8d17c55..c79ceb39ba 100644 --- a/Gems/Atom/Feature/Common/Code/Source/Material/MaterialAssignment.cpp +++ b/Gems/Atom/Feature/Common/Code/Source/Material/MaterialAssignment.cpp @@ -137,31 +137,28 @@ namespace AZ if (m_materialInstance->CanCompile()) { - AZStd::vector> renamedProperties; - for (const auto& propertyPair : m_propertyOverrides) { if (!propertyPair.second.empty()) { - RPI::MaterialPropertyIndex materialPropertyIndex = m_materialInstance->FindPropertyIndex(propertyPair.first); + bool wasRenamed = false; + Name newName; + RPI::MaterialPropertyIndex materialPropertyIndex = m_materialInstance->FindPropertyIndex(propertyPair.first, &wasRenamed, &newName); - // If we didn't find the name, check to see if there was a rename that should be used. - if (materialPropertyIndex.IsNull()) + // FindPropertyIndex will have already reported a message about what the old and new names are. Here we just add some extra info to help the user resolve it. + AZ_Warning("MaterialAssignment", !wasRenamed, + "Consider running \"Apply Automatic Property Updates\" to use the latest property names.", + propertyPair.first.GetCStr(), + newName.GetCStr()); + + if (wasRenamed && m_propertyOverrides.find(newName) != m_propertyOverrides.end()) { - Name propertyId = propertyPair.first; - - if (m_materialInstance->GetAsset()->GetMaterialTypeAsset()->ApplyPropertyRenames(propertyId)) - { - renamedProperties.emplace_back(propertyPair.first, propertyId); - - // We should be able to find the property now, using the new name - materialPropertyIndex = m_materialInstance->FindPropertyIndex(propertyId); - - AZ_Warning("MaterialAssignment", false, - "Material property '%s' has been renamed to '%s', and a material override references the old name. Save the level or prefab to permanently apply this change.", - propertyPair.first.GetCStr(), - propertyId.GetCStr()); - } + materialPropertyIndex.Reset(); + + AZ_Warning("MaterialAssignment", false, + "Material property '%s' has been renamed to '%s', and a property override exists for both. The one with the old name will be ignored.", + propertyPair.first.GetCStr(), + newName.GetCStr()); } if (!materialPropertyIndex.IsNull()) @@ -172,15 +169,6 @@ namespace AZ } } - // Now that we're done looping over all the overrides, it's safe to apply any renames that were scheduled - for (auto& pair : renamedProperties) - { - const Name& oldName = pair.first; - const Name& newName = pair.second; - m_propertyOverrides[newName] = m_propertyOverrides[oldName]; - m_propertyOverrides.erase(oldName); - } - return m_materialInstance->Compile(); } diff --git a/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/Material/Material.h b/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/Material/Material.h index d8a0cf0e7d..72c8616a73 100644 --- a/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/Material/Material.h +++ b/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/Material/Material.h @@ -71,7 +71,9 @@ namespace AZ virtual ~Material(); //! Finds the material property index from the material property ID - MaterialPropertyIndex FindPropertyIndex(const Name& propertyId) const; + //! @param wasRenamed optional parameter that is set to true if @propertyId is an old name and an automatic rename was applied to find the index. + //! @param newName optional parameter that is set to the new property name, if the property was renamed. + MaterialPropertyIndex FindPropertyIndex(const Name& propertyId, bool* wasRenamed = nullptr, Name* newName = nullptr) const; //! Sets the value of a material property. The template data type must match the property's data type. //! @return true if property value was changed diff --git a/Gems/Atom/RPI/Code/Source/RPI.Public/Material/Material.cpp b/Gems/Atom/RPI/Code/Source/RPI.Public/Material/Material.cpp index c87646e17d..050ae47749 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Public/Material/Material.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Public/Material/Material.cpp @@ -386,9 +386,39 @@ namespace AZ return m_currentChangeId; } - MaterialPropertyIndex Material::FindPropertyIndex(const Name& propertyId) const + MaterialPropertyIndex Material::FindPropertyIndex(const Name& propertyId, bool* wasRenamed, Name* newName) const { - return m_layout->FindPropertyIndex(propertyId); + if (wasRenamed) + { + *wasRenamed = false; + } + + MaterialPropertyIndex index = m_layout->FindPropertyIndex(propertyId); + if (!index.IsValid()) + { + Name renamedId = propertyId; + + if (m_materialAsset->GetMaterialTypeAsset()->ApplyPropertyRenames(renamedId)) + { + index = m_layout->FindPropertyIndex(renamedId); + + if (wasRenamed) + { + *wasRenamed = true; + } + + if (newName) + { + *newName = renamedId; + } + + AZ_Warning("Material", false, + "Material property '%s' has been renamed to '%s'. Consider updating the corresponding source data.", + propertyId.GetCStr(), + renamedId.GetCStr()); + } + } + return index; } template diff --git a/Gems/Atom/RPI/Code/Tests/Material/MaterialTests.cpp b/Gems/Atom/RPI/Code/Tests/Material/MaterialTests.cpp index f202747bf6..9c3e08ee08 100644 --- a/Gems/Atom/RPI/Code/Tests/Material/MaterialTests.cpp +++ b/Gems/Atom/RPI/Code/Tests/Material/MaterialTests.cpp @@ -857,4 +857,35 @@ namespace UnitTest EXPECT_EQ(material->GetPropertyValue(material->FindPropertyIndex(Name{ "MyInt" })), -7); EXPECT_EQ(srgData->GetConstant(srgData->FindShaderInputConstantIndex(Name{ "m_int" })), -7); } + + TEST_F(MaterialTests, TestFindPropertyIndexUsingOldName) + { + MaterialTypeAssetCreator materialTypeCreator; + materialTypeCreator.Begin(Uuid::CreateRandom()); + materialTypeCreator.AddShader(m_testMaterialShaderAsset); + AddCommonTestMaterialProperties(materialTypeCreator); + materialTypeCreator.SetVersion(2); + MaterialVersionUpdate versionUpdate(2); + versionUpdate.AddAction(MaterialVersionUpdate::RenamePropertyAction({Name{ "OldName" },Name{ "MyInt" }})); + materialTypeCreator.AddVersionUpdate(versionUpdate); + materialTypeCreator.End(m_testMaterialTypeAsset); + + MaterialAssetCreator materialCreator; + materialCreator.Begin(Uuid::CreateRandom(), *m_testMaterialTypeAsset); + materialCreator.End(m_testMaterialAsset); + + Data::Instance material = Material::FindOrCreate(m_testMaterialAsset); + + bool wasRenamed = false; + Name newName; + MaterialPropertyIndex indexFromOldName = material->FindPropertyIndex(Name{"OldName"}, &wasRenamed, &newName); + EXPECT_TRUE(wasRenamed); + EXPECT_EQ(newName, Name{"MyInt"}); + + MaterialPropertyIndex indexFromNewName = material->FindPropertyIndex(Name{"MyInt"}, &wasRenamed, &newName); + EXPECT_FALSE(wasRenamed); + + EXPECT_EQ(indexFromOldName, indexFromNewName); + } + } diff --git a/Gems/AtomLyIntegration/CommonFeatures/Code/Include/AtomLyIntegration/CommonFeatures/Material/MaterialComponentBus.h b/Gems/AtomLyIntegration/CommonFeatures/Code/Include/AtomLyIntegration/CommonFeatures/Material/MaterialComponentBus.h index ed7f1564ac..293080367d 100644 --- a/Gems/AtomLyIntegration/CommonFeatures/Code/Include/AtomLyIntegration/CommonFeatures/Material/MaterialComponentBus.h +++ b/Gems/AtomLyIntegration/CommonFeatures/Code/Include/AtomLyIntegration/CommonFeatures/Material/MaterialComponentBus.h @@ -43,6 +43,9 @@ namespace AZ virtual void ClearInvalidMaterialOverrides() = 0; //! Repair materials that reference missing assets by assigning the default asset virtual void RepairInvalidMaterialOverrides() = 0; + //! Repair material property overrides that reference missing properties by auto-renaming them where possible + //! @return the number of properties that were updated + virtual uint32_t ApplyAutomaticPropertyUpdates() = 0; //! Set default material override virtual void SetDefaultMaterialOverride(const AZ::Data::AssetId& materialAssetId) = 0; //! Get default material override diff --git a/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/EditorMaterialComponent.cpp b/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/EditorMaterialComponent.cpp index a3ae61b14b..94beeb2430 100644 --- a/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/EditorMaterialComponent.cpp +++ b/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/EditorMaterialComponent.cpp @@ -240,6 +240,19 @@ namespace AZ UpdateMaterialSlots(); }); action->setToolTip("Repair materials that reference missing assets by assigning the default asset."); + + action = menu->addAction("Apply Automatic Property Updates", [this]() { + AzToolsFramework::ScopedUndoBatch undoBatch("Applying automatic property updates."); + SetDirty(); + + uint32_t propertiesUpdated = 0; + MaterialComponentRequestBus::EventResult(propertiesUpdated, GetEntityId(), &MaterialComponentRequestBus::Events::ApplyAutomaticPropertyUpdates); + + AZ_Printf("EditorMaterialComponent", "Updated %u property(s).", propertiesUpdated); + + UpdateMaterialSlots(); + }); + action->setToolTip("Repair material property overrides that reference missing properties by auto-renaming them where possible."); } void EditorMaterialComponent::SetPrimaryAsset(const AZ::Data::AssetId& assetId) diff --git a/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/EditorMaterialComponentInspector.cpp b/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/EditorMaterialComponentInspector.cpp index b6875e2e9c..4ef96a13dd 100644 --- a/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/EditorMaterialComponentInspector.cpp +++ b/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/EditorMaterialComponentInspector.cpp @@ -352,6 +352,27 @@ namespace AZ m_editData.m_materialPropertyOverrideMap, m_entityId, &MaterialComponentRequestBus::Events::GetPropertyOverrides, m_materialAssignmentId); + // Apply any automatic property renames so that the material inspector will be property initialized with the right values + // for properties that have new names. + { + AZStd::vector> renamedProperties; + for (auto& propertyOverridePair : m_editData.m_materialPropertyOverrideMap) + { + Name name = propertyOverridePair.first; + if (m_materialInstance->GetAsset()->GetMaterialTypeAsset()->ApplyPropertyRenames(name)) + { + renamedProperties.emplace_back(propertyOverridePair.first, name); + } + } + for (auto& renamePair : renamedProperties) + { + const Name& oldName = renamePair.first; + const Name& newName = renamePair.second; + m_editData.m_materialPropertyOverrideMap[newName] = m_editData.m_materialPropertyOverrideMap[oldName]; + m_editData.m_materialPropertyOverrideMap.erase(oldName); + } + } + for (auto& group : m_groups) { for (auto& property : group.second.m_properties) diff --git a/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/MaterialComponentController.cpp b/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/MaterialComponentController.cpp index 4c26042a26..335434758b 100644 --- a/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/MaterialComponentController.cpp +++ b/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/MaterialComponentController.cpp @@ -49,6 +49,7 @@ namespace AZ ->Event("ClearIncompatibleMaterialOverrides", &MaterialComponentRequestBus::Events::ClearIncompatibleMaterialOverrides) ->Event("ClearInvalidMaterialOverrides", &MaterialComponentRequestBus::Events::ClearInvalidMaterialOverrides) ->Event("RepairInvalidMaterialOverrides", &MaterialComponentRequestBus::Events::RepairInvalidMaterialOverrides) + ->Event("ApplyAutomaticPropertyUpdates", &MaterialComponentRequestBus::Events::ApplyAutomaticPropertyUpdates) ->Event("SetMaterialOverride", &MaterialComponentRequestBus::Events::SetMaterialOverride) ->Event("GetMaterialOverride", &MaterialComponentRequestBus::Events::GetMaterialOverride) ->Event("ClearMaterialOverride", &MaterialComponentRequestBus::Events::ClearMaterialOverride) @@ -406,6 +407,39 @@ namespace AZ } LoadMaterials(); } + + uint32_t MaterialComponentController::ApplyAutomaticPropertyUpdates() + { + uint32_t propertiesUpdated = 0; + + for (auto& materialAssignmentPair : m_configuration.m_materials) + { + MaterialAssignment& materialAssignment = materialAssignmentPair.second; + + AZStd::vector> renamedProperties; + + for (const auto& propertyPair : materialAssignment.m_propertyOverrides) + { + Name propertyId = propertyPair.first; + + if (materialAssignment.m_materialInstance->GetAsset()->GetMaterialTypeAsset()->ApplyPropertyRenames(propertyId)) + { + renamedProperties.emplace_back(propertyPair.first, propertyId); + ++propertiesUpdated; + } + } + + for (auto& pair : renamedProperties) + { + const Name& oldName = pair.first; + const Name& newName = pair.second; + materialAssignment.m_propertyOverrides[newName] = materialAssignment.m_propertyOverrides[oldName]; + materialAssignment.m_propertyOverrides.erase(oldName); + } + } + + return propertiesUpdated; + } void MaterialComponentController::SetDefaultMaterialOverride(const AZ::Data::AssetId& materialAssetId) { diff --git a/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/MaterialComponentController.h b/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/MaterialComponentController.h index 1594456a31..ec542c377c 100644 --- a/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/MaterialComponentController.h +++ b/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/MaterialComponentController.h @@ -58,6 +58,7 @@ namespace AZ void ClearIncompatibleMaterialOverrides() override; void ClearInvalidMaterialOverrides() override; void RepairInvalidMaterialOverrides() override; + uint32_t ApplyAutomaticPropertyUpdates() override; void SetDefaultMaterialOverride(const AZ::Data::AssetId& materialAssetId) override; const AZ::Data::AssetId GetDefaultMaterialOverride() const override; void ClearDefaultMaterialOverride() override; From a892181432e1c29b21c3ca3896002ae7552bd7ea Mon Sep 17 00:00:00 2001 From: santorac <55155825+santorac@users.noreply.github.com> Date: Sat, 23 Oct 2021 00:02:31 -0700 Subject: [PATCH 3/3] Minor updates in response to code review feedback. Signed-off-by: santorac <55155825+santorac@users.noreply.github.com> --- .../Source/Material/EditorMaterialComponentInspector.cpp | 6 ++---- .../Code/Source/Material/MaterialComponentController.cpp | 6 ++---- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/EditorMaterialComponentInspector.cpp b/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/EditorMaterialComponentInspector.cpp index 4ef96a13dd..9af3b41b12 100644 --- a/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/EditorMaterialComponentInspector.cpp +++ b/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/EditorMaterialComponentInspector.cpp @@ -352,7 +352,7 @@ namespace AZ m_editData.m_materialPropertyOverrideMap, m_entityId, &MaterialComponentRequestBus::Events::GetPropertyOverrides, m_materialAssignmentId); - // Apply any automatic property renames so that the material inspector will be property initialized with the right values + // Apply any automatic property renames so that the material inspector will be properly initialized with the right values // for properties that have new names. { AZStd::vector> renamedProperties; @@ -364,10 +364,8 @@ namespace AZ renamedProperties.emplace_back(propertyOverridePair.first, name); } } - for (auto& renamePair : renamedProperties) + for (const auto& [oldName, newName] : renamedProperties) { - const Name& oldName = renamePair.first; - const Name& newName = renamePair.second; m_editData.m_materialPropertyOverrideMap[newName] = m_editData.m_materialPropertyOverrideMap[oldName]; m_editData.m_materialPropertyOverrideMap.erase(oldName); } diff --git a/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/MaterialComponentController.cpp b/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/MaterialComponentController.cpp index 335434758b..07082a87d5 100644 --- a/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/MaterialComponentController.cpp +++ b/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/MaterialComponentController.cpp @@ -428,11 +428,9 @@ namespace AZ ++propertiesUpdated; } } - - for (auto& pair : renamedProperties) + + for (const auto& [oldName, newName] : renamedProperties) { - const Name& oldName = pair.first; - const Name& newName = pair.second; materialAssignment.m_propertyOverrides[newName] = materialAssignment.m_propertyOverrides[oldName]; materialAssignment.m_propertyOverrides.erase(oldName); }