From 57989c35db45b57f2f73a3dcf7d26d2a865659da Mon Sep 17 00:00:00 2001
From: santorac <55155825+santorac@users.noreply.github.com>
Date: Thu, 11 Nov 2021 12:50:17 -0800
Subject: [PATCH] Changed material builder to not fail on warnings. The main
 reason for this is to give consistent results between the AP and Material
 Editor, where a placholder texture can be used if a texture is missing.
 Otherwise, you could get a placeholder texture in Material Editor and stale
 data in the runtime; this inconsistency would be confusing. As a consequence,
 it is possible for example that the user could mess up the name of a property
 in a .material file and not notice the problem because it is now a warning
 instead of an error. If warnings-as-errors is desirable, you can enable the
 new "/O3DE/Atom/RPI/MaterialBuilder/WarningsAsErrors" registry setting.

Signed-off-by: santorac <55155825+santorac@users.noreply.github.com>
---
 .../RPI.Builders/Material/MaterialBuilder.cpp  | 18 ++++++++++++++----
 .../RPI.Builders/Material/MaterialBuilder.h    |  5 +++++
 .../RPI.Edit/Material/MaterialSourceData.cpp   |  2 +-
 3 files changed, 20 insertions(+), 5 deletions(-)

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 e9cebf29c7..f11b2f94ac 100644
--- a/Gems/Atom/RPI/Code/Source/RPI.Builders/Material/MaterialBuilder.cpp
+++ b/Gems/Atom/RPI/Code/Source/RPI.Builders/Material/MaterialBuilder.cpp
@@ -63,11 +63,21 @@ namespace AZ
         {
             BusDisconnect();
         }
+        
+        bool MaterialBuilder::ReportMaterialAssetWarningsAsErrors() const
+        {
+            bool warningsAsErrors = false;
+            if (auto settingsRegistry = AZ::SettingsRegistry::Get(); settingsRegistry != nullptr)
+            {
+                settingsRegistry->Get(warningsAsErrors, "/O3DE/Atom/RPI/MaterialBuilder/WarningsAsErrors");
+            }
+            return warningsAsErrors;
+        }
 
         //! Adds all relevant dependencies for a referenced source file, considering that the path might be relative to the original file location or a full asset path.
         //! This will usually include multiple source dependencies and a single job dependency, but will include only source dependencies if the file is not found.
         //! Note the AssetBuilderSDK::JobDependency::m_platformIdentifier will not be set by this function. The calling code must set this value before passing back
-        //! to the AssetBuilderSDK::CreateJobsResponse. If isOrderedOnceForMaterialTypes is true and the dependency is a materialtype file, the job dependency type
+        //! to the AssetBuilderSDK::CreateJobsResponse. If isOrderedOnceForMaterialTypes is true and the dependency is a .materialtype file, the job dependency type
         //! will be set to JobDependencyType::OrderOnce.
         void AddPossibleDependencies(AZStd::string_view currentFilePath,
             AZStd::string_view referencedParentPath,
@@ -277,8 +287,8 @@ namespace AZ
 
             return materialTypeAssetOutcome.GetValue();
         }
-
-        AZ::Data::Asset<MaterialAsset> CreateMaterialAsset(AZStd::string_view materialSourceFilePath, const rapidjson::Value& json)
+        
+        AZ::Data::Asset<MaterialAsset> MaterialBuilder::CreateMaterialAsset(AZStd::string_view materialSourceFilePath, const rapidjson::Value& json) const
         {
             auto material = LoadSourceData<MaterialSourceData>(json, materialSourceFilePath);
 
@@ -292,7 +302,7 @@ namespace AZ
                 return {};
             }
 
-            auto materialAssetOutcome = material.GetValue().CreateMaterialAsset(Uuid::CreateRandom(), materialSourceFilePath, true);
+            auto materialAssetOutcome = material.GetValue().CreateMaterialAsset(Uuid::CreateRandom(), materialSourceFilePath, ReportMaterialAssetWarningsAsErrors());
             if (!materialAssetOutcome.IsSuccess())
             {
                 return {};
diff --git a/Gems/Atom/RPI/Code/Source/RPI.Builders/Material/MaterialBuilder.h b/Gems/Atom/RPI/Code/Source/RPI.Builders/Material/MaterialBuilder.h
index afb0789dcf..4fa5f3cf10 100644
--- a/Gems/Atom/RPI/Code/Source/RPI.Builders/Material/MaterialBuilder.h
+++ b/Gems/Atom/RPI/Code/Source/RPI.Builders/Material/MaterialBuilder.h
@@ -9,6 +9,8 @@
 #pragma once
 
 #include <AssetBuilderSDK/AssetBuilderBusses.h>
+#include <Atom/RPI.Reflect/Material/MaterialAsset.h>
+#include <AzCore/JSON/document.h>
 
 namespace AZ
 {
@@ -37,6 +39,9 @@ namespace AZ
 
         private:
             
+            AZ::Data::Asset<MaterialAsset> CreateMaterialAsset(AZStd::string_view materialSourceFilePath, const rapidjson::Value& json) const;
+            bool ReportMaterialAssetWarningsAsErrors() const;
+            
             bool m_isShuttingDown = false;
         };
 
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 a8d61465a5..b2c449a05d 100644
--- a/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialSourceData.cpp
+++ b/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialSourceData.cpp
@@ -303,7 +303,7 @@ namespace AZ
                     MaterialPropertyId propertyId{ group.first, property.first };
                     if (!property.second.m_value.IsValid())
                     {
-                        AZ_Warning("Material source data", false, "Source data for material property value is invalid.");
+                        materialAssetCreator.ReportWarning("Source data for material property value is invalid.");
                     }
                     else
                     {