{lyn8865} Adding DataTypes::ScriptProcessorFallbackLogic (#6396)

* {lyn8865} Adding DataTypes::ScriptProcessorFallbackLogic

- Give the user an option how to handle fallback logic for script rules

Signed-off-by: Allen Jackson <23512001+jackalbe@users.noreply.github.com>

* the new code found an error in a Python script... it seems to work!

Signed-off-by: Allen Jackson <23512001+jackalbe@users.noreply.github.com>

* fixing up the regression test

Signed-off-by: Jackson <23512001+jackalbe@users.noreply.github.com>

* dump version number

Signed-off-by: Allen Jackson <23512001+jackalbe@users.noreply.github.com>

AutomatedTesting/Gem/PythonTests/assetpipeline/fbx_tests/assets/TwoSceneFiles_OneWithPythonOneWithout_PythonOnlyRunsOnFirstScene/python_builder.py

@@ -23,7 +23,7 @@ def output_test_data(scene):
         # Just write something to the file, but the filename is the main information
         # used for the test.
         f.write(f"scene.sourceFilename: {scene.sourceFilename}\n")
-    return True
+    return ''
 
 mySceneJobHandler = None
 

AutomatedTesting/TestAssets/test_chunks_builder.py

@@ -28,7 +28,7 @@ def update_manifest(scene):
         meshGroup = sceneManifest.add_mesh_group(chunkName)
         meshGroup['id'] = '{' + str(uuid.uuid5(uuid.NAMESPACE_DNS, scene.sourceFilename + chunkName)) + '}'
         sceneManifest.mesh_group_add_comment(meshGroup, 'auto generated by test_chunks_builder')
-        sceneManifest.mesh_group_set_origin(meshGroup, None, 0, 0, 0, 1.0)
+        sceneManifest.mesh_group_add_advanced_coordinate_system(meshGroup)
         for meshIndex in range(len(chunkNameList)):
             if (activeMeshIndex == meshIndex):
                 sceneManifest.mesh_group_select_node(meshGroup, chunkNameList[meshIndex])

Code/Tools/SceneAPI/SceneCore/DataTypes/Rules/IScriptProcessorRule.h

@@ -17,6 +17,12 @@ namespace AZ
     {
         namespace DataTypes
         {
+            enum class ScriptProcessorFallbackLogic
+            {
+                FailBuild, // this will log error & fail the build
+                ContinueBuild // this will log the errors but continue the build logic
+            };
+
             class IScriptProcessorRule
                 : public IRule
             {
@@ -26,6 +32,8 @@ namespace AZ
                 virtual ~IScriptProcessorRule() override = default;
 
                 virtual const AZStd::string& GetScriptFilename() const = 0;
+
+                virtual ScriptProcessorFallbackLogic GetScriptProcessorFallbackLogic() const = 0;
             };
         }  // DataTypes
     }  // SceneAPI

Code/Tools/SceneAPI/SceneData/Behaviors/ScriptProcessorRuleBehavior.cpp

@@ -173,10 +173,13 @@ namespace AZ::SceneAPI::Behaviors
         UnloadPython();
     }
 
-    bool ScriptProcessorRuleBehavior::LoadPython(const AZ::SceneAPI::Containers::Scene& scene, AZStd::string& scriptPath)
+    bool ScriptProcessorRuleBehavior::LoadPython(const AZ::SceneAPI::Containers::Scene& scene, AZStd::string& scriptPath, Events::ProcessingResult& fallbackResult)
     {
+        using namespace AZ::SceneAPI;
+
+        fallbackResult = Events::ProcessingResult::Failure;
         int scriptDiscoveryAttempts = 0;
-        const AZ::SceneAPI::Containers::SceneManifest& manifest = scene.GetManifest();
+        const Containers::SceneManifest& manifest = scene.GetManifest();
         auto view = Containers::MakeDerivedFilterView<DataTypes::IScriptProcessorRule>(manifest.GetValueStorage());
         for (const auto& scriptItem : view)
         {
@@ -188,6 +191,8 @@ namespace AZ::SceneAPI::Behaviors
             }
 
             ++scriptDiscoveryAttempts;
+            fallbackResult = (scriptItem.GetScriptProcessorFallbackLogic() == DataTypes::ScriptProcessorFallbackLogic::ContinueBuild) ?
+                Events::ProcessingResult::Ignored : Events::ProcessingResult::Failure;
 
             // check for file exist via absolute path
             if (!IO::FileIOBase::GetInstance()->Exists(scriptFilename.c_str()))
@@ -301,7 +306,8 @@ namespace AZ::SceneAPI::Behaviors
             }
         };
 
-        if (LoadPython(context.GetScene(), scriptPath))
+        [[maybe_unused]] Events::ProcessingResult fallbackResult;
+        if (LoadPython(context.GetScene(), scriptPath, fallbackResult))
         {
             EditorPythonConsoleNotificationHandler logger;
             m_editorPythonEventsInterface->ExecuteWithLock(executeCallback);
@@ -333,8 +339,9 @@ namespace AZ::SceneAPI::Behaviors
             return Events::ProcessingResult::Ignored;
         }
 
+        Events::ProcessingResult fallbackResult;
         AZStd::string scriptPath;
-        if (LoadPython(scene, scriptPath))
+        if (LoadPython(scene, scriptPath, fallbackResult))
         {
             AZStd::string manifestUpdate;
             auto executeCallback = [&scene, &manifestUpdate, &scriptPath]()
@@ -349,6 +356,12 @@ namespace AZ::SceneAPI::Behaviors
             EditorPythonConsoleNotificationHandler logger;
             m_editorPythonEventsInterface->ExecuteWithLock(executeCallback);
 
+            // if the returned scene manifest is empty then ignore the script update
+            if (manifestUpdate.empty())
+            {
+                return Events::ProcessingResult::Ignored;
+            }
+
             EntityUtilityBus::Broadcast(&EntityUtilityBus::Events::ResetEntityContext);
             AZ::Interface<Prefab::PrefabSystemComponentInterface>::Get()->RemoveAllTemplates();
 
@@ -364,6 +377,11 @@ namespace AZ::SceneAPI::Behaviors
                 }
                 return Events::ProcessingResult::Success;
             }
+            else
+            {
+                // if the manifest was not updated by the script, then return back the fallback result
+                return fallbackResult;
+            }
         }
         return Events::ProcessingResult::Ignored;
     }

Code/Tools/SceneAPI/SceneData/Behaviors/ScriptProcessorRuleBehavior.h

@@ -54,7 +54,7 @@ namespace AZ::SceneAPI::Behaviors
 
         SCENE_DATA_API void GetManifestDependencyPaths(AZStd::vector<AZStd::string>& paths) override;
     protected:
-        bool LoadPython(const AZ::SceneAPI::Containers::Scene& scene, AZStd::string& scriptPath);
+        bool LoadPython(const AZ::SceneAPI::Containers::Scene& scene, AZStd::string& scriptPath, Events::ProcessingResult& fallbackResult);
         void UnloadPython();
         bool DoPrepareForExport(Events::PreExportEventContext& context);
 

Code/Tools/SceneAPI/SceneData/Rules/ScriptProcessorRule.cpp

@@ -14,6 +14,9 @@
 
 namespace AZ
 {
+    // Enum types must have a TypeId tied to it in order for the reflection to succeed.
+    AZ_TYPE_INFO_SPECIALIZE(SceneAPI::DataTypes::ScriptProcessorFallbackLogic, "{3DCABF3D-E8EF-43E7-B3C7-373E05825F60}");
+
     namespace SceneAPI
     {
         namespace SceneData
@@ -23,13 +26,23 @@ namespace AZ
                 return m_scriptFilename;
             }
 
+            DataTypes::ScriptProcessorFallbackLogic ScriptProcessorRule::GetScriptProcessorFallbackLogic() const
+            {
+                return m_fallbackLogic;
+            }
+
             void ScriptProcessorRule::Reflect(AZ::ReflectContext* context)
             {
                 AZ::SerializeContext* serializeContext = azrtti_cast<AZ::SerializeContext*>(context);
                 if (serializeContext)
                 {
-                    serializeContext->Class<ScriptProcessorRule, DataTypes::IScriptProcessorRule>()->Version(1)
+                    serializeContext->Class<ScriptProcessorRule, DataTypes::IScriptProcessorRule>()->Version(2)
-                        ->Field("scriptFilename", &ScriptProcessorRule::m_scriptFilename);
+                        ->Field("scriptFilename", &ScriptProcessorRule::m_scriptFilename)
+                        ->Field("fallbackLogic", &ScriptProcessorRule::m_fallbackLogic);
+
+                    serializeContext->Enum<DataTypes::ScriptProcessorFallbackLogic>()
+                        ->Value("FailBuild", DataTypes::ScriptProcessorFallbackLogic::FailBuild)
+                        ->Value("ContinueBuild", DataTypes::ScriptProcessorFallbackLogic::ContinueBuild);
 
                     AZ::EditContext* editContext = serializeContext->GetEditContext();
                     if (editContext)

Code/Tools/SceneAPI/SceneData/Rules/ScriptProcessorRule.h

@@ -35,10 +35,13 @@ namespace AZ
                     m_scriptFilename = AZStd::move(scriptFilename);
                 }
 
+                DataTypes::ScriptProcessorFallbackLogic GetScriptProcessorFallbackLogic() const override;
+
                 static void Reflect(ReflectContext* context);
 
             protected:
                 AZStd::string m_scriptFilename;
+                DataTypes::ScriptProcessorFallbackLogic m_fallbackLogic = DataTypes::ScriptProcessorFallbackLogic::FailBuild;
             };
         } // SceneData
     } // SceneAPI

Code/Tools/SceneAPI/SceneData/Tests/SceneManifest/SceneManifestRuleTests.cpp

@@ -11,6 +11,7 @@
 #include <SceneAPI/SceneCore/Containers/SceneManifest.h>
 #include <SceneAPI/SceneCore/Containers/Scene.h>
 #include <SceneAPI/SceneCore/DataTypes/Rules/IScriptProcessorRule.h>
+#include <SceneAPI/SceneCore/Containers/Utilities/Filters.h>
 #include <SceneAPI/SceneData/ReflectionRegistrar.h>
 #include <SceneAPI/SceneData/Rules/CoordinateSystemRule.h>
 #include <SceneAPI/SceneData/Behaviors/ScriptProcessorRuleBehavior.h>
@@ -271,5 +272,79 @@ namespace AZ
             auto update = scriptProcessorRuleBehavior.UpdateManifest(scene, AssetImportRequest::Update, AssetImportRequest::Generic);
             EXPECT_EQ(update, ProcessingResult::Ignored);
         }
+
+        TEST_F(SceneManifest_JSON, ScriptProcessorRule_DefaultFallbackLogic_Works)
+        {
+            using namespace AZ::SceneAPI;
+
+            constexpr const char* defaultJson = { R"JSON(
+            {
+                "values": [
+                    {
+                        "$type": "ScriptProcessorRule",
+                        "scriptFilename": "foo.py"
+                    }
+                ]
+            })JSON" };
+
+            auto scene = Containers::Scene("mock");
+            auto result = scene.GetManifest().LoadFromString(defaultJson, m_serializeContext.get(), m_jsonRegistrationContext.get());
+            EXPECT_TRUE(result.IsSuccess());
+            EXPECT_FALSE(scene.GetManifest().IsEmpty());
+            ASSERT_EQ(scene.GetManifest().GetEntryCount(), 1);
+
+            auto view = Containers::MakeDerivedFilterView<DataTypes::IScriptProcessorRule>(scene.GetManifest().GetValueStorage());
+            EXPECT_EQ(view.begin()->GetScriptProcessorFallbackLogic(), DataTypes::ScriptProcessorFallbackLogic::FailBuild);
+        }
+
+        TEST_F(SceneManifest_JSON, ScriptProcessorRule_ExplicitFallbackLogic_Works)
+        {
+            using namespace AZ::SceneAPI;
+
+            constexpr const char* fallbackLogicJson = { R"JSON(
+            {
+                "values": [
+                    {
+                        "$type": "ScriptProcessorRule",
+                        "scriptFilename": "foo.py",
+                        "fallbackLogic": "FailBuild"
+                    }
+                ]
+            })JSON" };
+
+            auto scene = Containers::Scene("mock");
+            auto result = scene.GetManifest().LoadFromString(fallbackLogicJson, m_serializeContext.get(), m_jsonRegistrationContext.get());
+            EXPECT_TRUE(result.IsSuccess());
+            EXPECT_FALSE(scene.GetManifest().IsEmpty());
+            ASSERT_EQ(scene.GetManifest().GetEntryCount(), 1);
+
+            auto view = Containers::MakeDerivedFilterView<DataTypes::IScriptProcessorRule>(scene.GetManifest().GetValueStorage());
+            EXPECT_EQ(view.begin()->GetScriptProcessorFallbackLogic(), DataTypes::ScriptProcessorFallbackLogic::FailBuild);
+        }
+
+        TEST_F(SceneManifest_JSON, ScriptProcessorRule_ContinueBuildFallbackLogic_Works)
+        {
+            using namespace AZ::SceneAPI;
+
+            constexpr const char* fallbackLogicJson = { R"JSON(
+            {
+                "values": [
+                    {
+                        "$type": "ScriptProcessorRule",
+                        "scriptFilename": "foo.py",
+                        "fallbackLogic": "ContinueBuild"
+                    }
+                ]
+            })JSON" };
+
+            auto scene = Containers::Scene("mock");
+            auto result = scene.GetManifest().LoadFromString(fallbackLogicJson, m_serializeContext.get(), m_jsonRegistrationContext.get());
+            EXPECT_TRUE(result.IsSuccess());
+            EXPECT_FALSE(scene.GetManifest().IsEmpty());
+            ASSERT_EQ(scene.GetManifest().GetEntryCount(), 1);
+
+            auto view = Containers::MakeDerivedFilterView<DataTypes::IScriptProcessorRule>(scene.GetManifest().GetValueStorage());
+            EXPECT_EQ(view.begin()->GetScriptProcessorFallbackLogic(), DataTypes::ScriptProcessorFallbackLogic::ContinueBuild);
+        }
     }
 }