Added backward-compatible support for the old "id" key in material type files, which is being renamed to "name".

This required the use of custom serializers, because the JSON serialization system does not have any means of supporting field name aliases through SerializeContext.

Testing: RPI unit test pass and AtomSampleViewer material screenshot test script passes.

Signed-off-by: santorac <55155825+santorac@users.noreply.github.com>

Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialPropertyConnectionSerializer.cpp

@@ -0,0 +1,126 @@
+/*
+ * 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 <Atom/RPI.Edit/Material/MaterialPropertyConnectionSerializer.h>
+#include <Atom/RPI.Edit/Material/MaterialUtils.h>
+
+#include <AzCore/Serialization/Json/BaseJsonSerializer.h>
+#include <AzCore/Serialization/Json/JsonSerializationResult.h>
+#include <AzCore/Serialization/Json/JsonSerialization.h>
+#include <AzCore/Serialization/Json/StackedString.h>
+
+namespace AZ
+{
+    namespace RPI
+    {
+        namespace JsonMaterialPropertyConnectionSerializerInternal
+        {
+            namespace Field
+            {
+                static constexpr const char type[] = "type";
+                static constexpr const char name[] = "name";
+                static constexpr const char id[] = "id"; // For backward compatibility
+                static constexpr const char shaderIndex[] = "shaderIndex";
+            }
+
+            static const AZStd::string_view AcceptedFields[] =
+            {
+                Field::type,
+                Field::name,
+                Field::id,
+                Field::shaderIndex
+            };
+        }
+
+        AZ_CLASS_ALLOCATOR_IMPL(JsonMaterialPropertyConnectionSerializer, SystemAllocator, 0);
+
+        JsonSerializationResult::Result JsonMaterialPropertyConnectionSerializer::Load(void* outputValue, const Uuid& outputValueTypeId,
+            const rapidjson::Value& inputValue, JsonDeserializerContext& context)
+        {
+            namespace JSR = JsonSerializationResult;
+            using namespace JsonMaterialPropertyConnectionSerializerInternal;
+
+            AZ_Assert(azrtti_typeid<MaterialTypeSourceData::PropertyConnection>() == outputValueTypeId,
+                "Unable to deserialize material property connection to json because the provided type is %s",
+                outputValueTypeId.ToString<AZStd::string>().c_str());
+            AZ_UNUSED(outputValueTypeId);
+
+            MaterialTypeSourceData::PropertyConnection* propertyConnection = reinterpret_cast<MaterialTypeSourceData::PropertyConnection*>(outputValue);
+            AZ_Assert(propertyConnection, "Output value for JsonMaterialPropertyConnectionSerializer can't be null.");
+
+            JSR::ResultCode result(JSR::Tasks::ReadField);
+
+            if (!inputValue.IsObject())
+            {
+                return context.Report(JsonSerializationResult::Tasks::ReadField, JsonSerializationResult::Outcomes::Unsupported, "Property connection must be a JSON object.");
+            }
+            
+            MaterialUtils::CheckForUnrecognizedJsonFields(AcceptedFields, AZ_ARRAY_SIZE(AcceptedFields), inputValue, context, result);
+
+            result.Combine(ContinueLoadingFromJsonObjectField(&propertyConnection->m_type, azrtti_typeid<MaterialPropertyOutputType>(), inputValue, Field::type, context));
+            
+            JsonSerializationResult::ResultCode nameResult = ContinueLoadingFromJsonObjectField(&propertyConnection->m_fieldName, azrtti_typeid<AZStd::string>(), inputValue, Field::name, context);
+            if (nameResult.GetOutcome() == JsonSerializationResult::Outcomes::DefaultsUsed)
+            {
+                // This "id" key is for backward compatibility.
+                result.Combine(ContinueLoadingFromJsonObjectField(&propertyConnection->m_fieldName, azrtti_typeid<AZStd::string>(), inputValue, Field::id, context));
+            }
+            else
+            {
+                result.Combine(nameResult);
+            }
+
+            result.Combine(ContinueLoadingFromJsonObjectField(&propertyConnection->m_shaderIndex, azrtti_typeid<int32_t>(), inputValue, Field::shaderIndex, context));
+
+            if (result.GetProcessing() == JsonSerializationResult::Processing::Completed)
+            {
+                return context.Report(result, "Successfully loaded property connection.");
+            }
+            else
+            {
+                return context.Report(result, "Partially loaded property connection.");
+            }
+        }
+        
+
+        JsonSerializationResult::Result JsonMaterialPropertyConnectionSerializer::Store(rapidjson::Value& outputValue, const void* inputValue,
+            [[maybe_unused]] const void* defaultValue, const Uuid& valueTypeId, JsonSerializerContext& context)
+        {
+            namespace JSR = JsonSerializationResult;
+            using namespace JsonMaterialPropertyConnectionSerializerInternal;
+
+            AZ_Assert(azrtti_typeid<MaterialTypeSourceData::PropertyConnection>() == valueTypeId,
+                "Unable to serialize material property connection to json because the provided type is %s",
+                valueTypeId.ToString<AZStd::string>().c_str());
+            AZ_UNUSED(valueTypeId);
+
+            const MaterialTypeSourceData::PropertyConnection* propertyConnection = reinterpret_cast<const MaterialTypeSourceData::PropertyConnection*>(inputValue);
+            AZ_Assert(propertyConnection, "Input value for JsonMaterialPropertyConnectionSerializer can't be null.");
+            
+            JSR::ResultCode result(JSR::Tasks::WriteValue);
+
+            outputValue.SetObject();
+
+            MaterialTypeSourceData::PropertyConnection defaultConnection;
+
+            result.Combine(ContinueStoringToJsonObjectField(outputValue, Field::type, &propertyConnection->m_type, &defaultConnection.m_type, azrtti_typeid<MaterialPropertyOutputType>(), context));
+            result.Combine(ContinueStoringToJsonObjectField(outputValue, Field::name, &propertyConnection->m_fieldName, &defaultConnection.m_fieldName, azrtti_typeid<AZStd::string>(), context));
+            result.Combine(ContinueStoringToJsonObjectField(outputValue, Field::shaderIndex, &propertyConnection->m_shaderIndex, &defaultConnection.m_shaderIndex, azrtti_typeid<int32_t>(), context));
+
+            if (result.GetProcessing() == JsonSerializationResult::Processing::Completed)
+            {
+                return context.Report(result, "Successfully stored property connection.");
+            }
+            else
+            {
+                return context.Report(result, "Partially stored property connection.");
+            }
+        }
+
+    } // namespace RPI
+} // namespace AZ

Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialPropertyGroupSerializer.cpp

@@ -0,0 +1,125 @@
+/*
+ * 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 <Atom/RPI.Edit/Material/MaterialPropertyGroupSerializer.h>
+#include <Atom/RPI.Edit/Material/MaterialUtils.h>
+
+#include <AzCore/Serialization/Json/BaseJsonSerializer.h>
+#include <AzCore/Serialization/Json/JsonSerializationResult.h>
+#include <AzCore/Serialization/Json/JsonSerialization.h>
+#include <AzCore/Serialization/Json/StackedString.h>
+
+namespace AZ
+{
+    namespace RPI
+    {
+        namespace JsonMaterialPropertyGroupSerializerInternal
+        {
+            namespace Field
+            {
+                static constexpr const char name[] = "name";
+                static constexpr const char id[] = "id"; // For backward compatibility
+                static constexpr const char displayName[] = "displayName";
+                static constexpr const char description[] = "description";
+            }
+
+            static const AZStd::string_view AcceptedFields[] =
+            {
+                Field::name,
+                Field::id,
+                Field::displayName,
+                Field::description
+            };
+        }
+
+        AZ_CLASS_ALLOCATOR_IMPL(JsonMaterialPropertyGroupSerializer, SystemAllocator, 0);
+
+        JsonSerializationResult::Result JsonMaterialPropertyGroupSerializer::Load(void* outputValue, const Uuid& outputValueTypeId,
+            const rapidjson::Value& inputValue, JsonDeserializerContext& context)
+        {
+            namespace JSR = JsonSerializationResult;
+            using namespace JsonMaterialPropertyGroupSerializerInternal;
+
+            AZ_Assert(azrtti_typeid<MaterialTypeSourceData::GroupDefinition>() == outputValueTypeId,
+                "Unable to deserialize material property group to json because the provided type is %s",
+                outputValueTypeId.ToString<AZStd::string>().c_str());
+            AZ_UNUSED(outputValueTypeId);
+
+            MaterialTypeSourceData::GroupDefinition* propertyGroup = reinterpret_cast<MaterialTypeSourceData::GroupDefinition*>(outputValue);
+            AZ_Assert(propertyGroup, "Output value for JsonMaterialPropertyGroupSerializer can't be null.");
+
+            JSR::ResultCode result(JSR::Tasks::ReadField);
+
+            if (!inputValue.IsObject())
+            {
+                return context.Report(JsonSerializationResult::Tasks::ReadField, JsonSerializationResult::Outcomes::Unsupported, "Property group must be a JSON object.");
+            }
+            
+            MaterialUtils::CheckForUnrecognizedJsonFields(AcceptedFields, AZ_ARRAY_SIZE(AcceptedFields), inputValue, context, result);
+                        
+            JsonSerializationResult::ResultCode nameResult = ContinueLoadingFromJsonObjectField(&propertyGroup->m_name, azrtti_typeid<AZStd::string>(), inputValue, Field::name, context);
+            if (nameResult.GetOutcome() == JsonSerializationResult::Outcomes::DefaultsUsed)
+            {
+                // This "id" key is for backward compatibility.
+                result.Combine(ContinueLoadingFromJsonObjectField(&propertyGroup->m_name, azrtti_typeid<AZStd::string>(), inputValue, Field::id, context));
+            }
+            else
+            {
+                result.Combine(nameResult);
+            }
+            
+            result.Combine(ContinueLoadingFromJsonObjectField(&propertyGroup->m_displayName, azrtti_typeid<AZStd::string>(), inputValue, Field::displayName, context));
+            result.Combine(ContinueLoadingFromJsonObjectField(&propertyGroup->m_description, azrtti_typeid<AZStd::string>(), inputValue, Field::description, context));
+
+            if (result.GetProcessing() == JsonSerializationResult::Processing::Completed)
+            {
+                return context.Report(result, "Successfully loaded property group.");
+            }
+            else
+            {
+                return context.Report(result, "Partially loaded property group.");
+            }
+        }
+        
+
+        JsonSerializationResult::Result JsonMaterialPropertyGroupSerializer::Store(rapidjson::Value& outputValue, const void* inputValue,
+            [[maybe_unused]] const void* defaultValue, const Uuid& valueTypeId, JsonSerializerContext& context)
+        {
+            namespace JSR = JsonSerializationResult;
+            using namespace JsonMaterialPropertyGroupSerializerInternal;
+
+            AZ_Assert(azrtti_typeid<MaterialTypeSourceData::GroupDefinition>() == valueTypeId,
+                "Unable to serialize material property group to json because the provided type is %s",
+                valueTypeId.ToString<AZStd::string>().c_str());
+            AZ_UNUSED(valueTypeId);
+
+            const MaterialTypeSourceData::GroupDefinition* propertyGroup = reinterpret_cast<const MaterialTypeSourceData::GroupDefinition*>(inputValue);
+            AZ_Assert(propertyGroup, "Input value for JsonMaterialPropertyGroupSerializer can't be null.");
+            
+            JSR::ResultCode result(JSR::Tasks::WriteValue);
+
+            outputValue.SetObject();
+
+            AZStd::string defaultEmpty;
+
+            result.Combine(ContinueStoringToJsonObjectField(outputValue, Field::name, &propertyGroup->m_name, &defaultEmpty, azrtti_typeid<AZStd::string>(), context));
+            result.Combine(ContinueStoringToJsonObjectField(outputValue, Field::displayName, &propertyGroup->m_displayName, &defaultEmpty, azrtti_typeid<AZStd::string>(), context));
+            result.Combine(ContinueStoringToJsonObjectField(outputValue, Field::description, &propertyGroup->m_description, &defaultEmpty, azrtti_typeid<AZStd::string>(), context));
+
+            if (result.GetProcessing() == JsonSerializationResult::Processing::Completed)
+            {
+                return context.Report(result, "Successfully stored property group.");
+            }
+            else
+            {
+                return context.Report(result, "Partially stored property group.");
+            }
+        }
+
+    } // namespace RPI
+} // namespace AZ

Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialPropertySerializer.cpp

@@ -8,6 +8,7 @@
 
 #include <Atom/RPI.Edit/Material/MaterialPropertySerializer.h>
 #include <Atom/RPI.Edit/Material/MaterialPropertyId.h>
+#include <Atom/RPI.Edit/Material/MaterialUtils.h>
 
 #include <AzCore/Serialization/Json/BaseJsonSerializer.h>
 #include <AzCore/Serialization/Json/JsonSerializationResult.h>
@@ -23,11 +24,12 @@ namespace AZ
 {
     namespace RPI
     {
-        namespace // Avoid conflicts in uber builds
+        namespace JsonMaterialPropertySerializerInternal
         {
             namespace Field
             {
                 static constexpr const char name[] = "name";
+                static constexpr const char id[] = "id"; // For backward compatibility
                 static constexpr const char displayName[] = "displayName";
                 static constexpr const char description[] = "description";
                 static constexpr const char type[] = "type";
@@ -47,6 +49,7 @@ namespace AZ
             static const AZStd::string_view AcceptedFields[] =
             {
                 Field::name,
+                Field::id,
                 Field::displayName,
                 Field::description,
                 Field::type,
@@ -103,6 +106,7 @@ namespace AZ
             JsonDeserializerContext& context)
         {
             namespace JSR = JsonSerializationResult;
+            using namespace JsonMaterialPropertySerializerInternal;
 
             JSR::ResultCode result(JSR::Tasks::ReadField);
 
@@ -160,6 +164,7 @@ namespace AZ
             JsonDeserializerContext& context)
         {
             namespace JSR = JsonSerializationResult;
+            using namespace JsonMaterialPropertySerializerInternal;
 
             JSR::ResultCode result(JSR::Tasks::ReadField);
 
@@ -181,6 +186,7 @@ namespace AZ
             const rapidjson::Value& inputValue, JsonDeserializerContext& context)
         {
             namespace JSR = JsonSerializationResult;
+            using namespace JsonMaterialPropertySerializerInternal;
 
             AZ_Assert(azrtti_typeid<MaterialTypeSourceData::PropertyDefinition>() == outputValueTypeId,
                 "Unable to deserialize material property to json because the provided type is %s",
@@ -197,28 +203,19 @@ namespace AZ
                 return context.Report(JsonSerializationResult::Tasks::ReadField, JsonSerializationResult::Outcomes::Unsupported, "Property definition must be a JSON object.");
             }
 
-            // First check for unexpected fields
-            for (auto iter = inputValue.MemberBegin(); iter != inputValue.MemberEnd(); ++iter)
+            MaterialUtils::CheckForUnrecognizedJsonFields(AcceptedFields, AZ_ARRAY_SIZE(AcceptedFields), inputValue, context, result);
+
+            JsonSerializationResult::ResultCode nameResult = ContinueLoadingFromJsonObjectField(&property->m_name, azrtti_typeid<AZStd::string>(), inputValue, Field::name, context);
+            if (nameResult.GetOutcome() == JsonSerializationResult::Outcomes::DefaultsUsed)
             {
-                bool matched = false;
-
-                for (int i = 0; i < AZ_ARRAY_SIZE(AcceptedFields); ++i)
-                {
-                    if (iter->name.GetString() == AcceptedFields[i])
-                    {
-                        matched = true;
-                        break;
-                    }
-                }
-
-                if (!matched)
-                {
-                    ScopedContextPath subPath{context, iter->name.GetString()};
-                    result.Combine(context.Report(JSR::Tasks::ReadField, JSR::Outcomes::Skipped, "Skipping unrecognized field"));
-                }
+                // This "id" key is for backward compatibility.
+                result.Combine(ContinueLoadingFromJsonObjectField(&property->m_name, azrtti_typeid<AZStd::string>(), inputValue, Field::id, context));
+            }
+            else
+            {
+                result.Combine(nameResult);
             }
 
-            result.Combine(ContinueLoadingFromJsonObjectField(&property->m_name, azrtti_typeid<AZStd::string>(), inputValue, Field::name, context));
             result.Combine(ContinueLoadingFromJsonObjectField(&property->m_displayName, azrtti_typeid<AZStd::string>(), inputValue, Field::displayName, context));
             result.Combine(ContinueLoadingFromJsonObjectField(&property->m_description, azrtti_typeid<AZStd::string>(), inputValue, Field::description, context));
             result.Combine(ContinueLoadingFromJsonObjectField(&property->m_dataType, azrtti_typeid<MaterialPropertyDataType>(), inputValue, Field::type, context));
@@ -302,6 +299,8 @@ namespace AZ
             JsonSerializerContext& context)
         {
             namespace JSR = JsonSerializationResult;
+            using namespace JsonMaterialPropertySerializerInternal;
+
             JSR::ResultCode result(JSR::Tasks::WriteValue);
 
             if (property->m_value.Is<T>())
@@ -345,6 +344,8 @@ namespace AZ
             JsonSerializerContext& context)
         {
             namespace JSR = JsonSerializationResult;
+            using namespace JsonMaterialPropertySerializerInternal;
+
             JsonSerializationResult::ResultCode result(JSR::Tasks::WriteValue);
 
             if (property->m_value.Is<T>())
@@ -360,6 +361,7 @@ namespace AZ
             [[maybe_unused]] const void* defaultValue, const Uuid& valueTypeId, JsonSerializerContext& context)
         {
             namespace JSR = JsonSerializationResult;
+            using namespace JsonMaterialPropertySerializerInternal;
 
             AZ_Assert(azrtti_typeid<MaterialTypeSourceData::PropertyDefinition>() == valueTypeId,
                 "Unable to serialize material property to json because the provided type is %s",
@@ -452,6 +454,8 @@ namespace AZ
             const rapidjson::Value& inputValue, JsonDeserializerContext& context)
         {
             namespace JSR = JsonSerializationResult;
+            using namespace JsonMaterialPropertySerializerInternal;
+
             JSR::ResultCode result(JSR::Tasks::ReadField);
 
             if (inputValue.HasMember(Field::vectorLabels))
@@ -467,6 +471,8 @@ namespace AZ
         {
             AZStd::string emptyString;
             namespace JSR = JsonSerializationResult;
+            using namespace JsonMaterialPropertySerializerInternal;
+
             JsonSerializationResult::ResultCode result(JSR::Tasks::WriteValue);
 
             if (!property->m_vectorLabels.empty())

Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialTypeSourceData.cpp

@@ -9,6 +9,8 @@
 #include <Atom/RPI.Edit/Material/MaterialTypeSourceData.h>
 #include <Atom/RPI.Edit/Material/MaterialPropertySerializer.h>
 #include <Atom/RPI.Edit/Material/MaterialFunctorSourceDataSerializer.h>
+#include <Atom/RPI.Edit/Material/MaterialPropertyConnectionSerializer.h>
+#include <Atom/RPI.Edit/Material/MaterialPropertyGroupSerializer.h>
 #include <Atom/RPI.Edit/Material/MaterialUtils.h>
 
 #include <Atom/RPI.Edit/Common/AssetUtils.h>
@@ -46,29 +48,17 @@ namespace AZ
             if (JsonRegistrationContext* jsonContext = azrtti_cast<JsonRegistrationContext*>(context))
             {
                 jsonContext->Serializer<JsonMaterialPropertySerializer>()->HandlesType<MaterialTypeSourceData::PropertyDefinition>();
+                jsonContext->Serializer<JsonMaterialPropertyConnectionSerializer>()->HandlesType<MaterialTypeSourceData::PropertyConnection>();
+                jsonContext->Serializer<JsonMaterialPropertyGroupSerializer>()->HandlesType<MaterialTypeSourceData::GroupDefinition>();
             }
             else if (auto* serializeContext = azrtti_cast<SerializeContext*>(context))
             {
-                serializeContext->Class<PropertyConnection>()
-                    ->Version(2)
-                    ->Field("type", &PropertyConnection::m_type)
-                    ->Field("name", &PropertyConnection::m_fieldName)
-                    ->Field("shaderIndex", &PropertyConnection::m_shaderIndex)
-                    ;
+                serializeContext->Class<PropertyConnection>()->Version(3);
+                serializeContext->Class<GroupDefinition>()->Version(4);
+                serializeContext->Class<PropertyDefinition>()->Version(1);
 
                 serializeContext->RegisterGenericType<PropertyConnectionList>();
 
-                serializeContext->Class<GroupDefinition>()
-                    ->Version(2)
-                    ->Field("name", &GroupDefinition::m_name)
-                    ->Field("displayName", &GroupDefinition::m_displayName)
-                    ->Field("description", &GroupDefinition::m_description)
-                    ;
-
-                serializeContext->Class<PropertyDefinition>()
-                    ->Version(1)
-                    ;
-
                 serializeContext->Class<ShaderVariantReferenceData>()
                     ->Version(2)
                     ->Field("file", &ShaderVariantReferenceData::m_shaderFilePath)

Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialUtils.cpp

@@ -17,6 +17,8 @@
 #include <Atom/RPI.Edit/Common/JsonFileLoadContext.h>
 #include <Atom/RPI.Edit/Common/JsonUtils.h>
 #include <AzCore/Serialization/Json/JsonUtils.h>
+#include <AzCore/Serialization/Json/BaseJsonSerializer.h>
+#include <AzCore/Serialization/Json/JsonSerializationResult.h>
 
 #include <AzCore/std/string/string.h>
 
@@ -99,6 +101,29 @@ namespace AZ
                     return AZ::Success(AZStd::move(materialType));
                 }
             }
+
+            void CheckForUnrecognizedJsonFields(const AZStd::string_view* acceptedFieldNames, uint32_t acceptedFieldNameCount, const rapidjson::Value& object, JsonDeserializerContext& context, JsonSerializationResult::ResultCode &result)
+            {
+                for (auto iter = object.MemberBegin(); iter != object.MemberEnd(); ++iter)
+                {
+                    bool matched = false;
+
+                    for (uint32_t i = 0; i < acceptedFieldNameCount; ++i)
+                    {
+                        if (iter->name.GetString() == acceptedFieldNames[i])
+                        {
+                            matched = true;
+                            break;
+                        }
+                    }
+
+                    if (!matched)
+                    {
+                        ScopedContextPath subPath{context, iter->name.GetString()};
+                        result.Combine(context.Report(JsonSerializationResult::Tasks::ReadField, JsonSerializationResult::Outcomes::Skipped, "Skipping unrecognized field"));
+                    }
+                }
+            }
         }
     }
 }