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); + + // 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 we didn't find the name, check to see if there was a rename that should be used. - if (materialPropertyIndex.IsNull()) + 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;