diff --git a/AutomatedTesting/Gem/PythonTests/assetpipeline/fbx_tests/assets/TwoSceneFiles_OneWithPythonOneWithout_PythonOnlyRunsOnFirstScene/python_builder.py b/AutomatedTesting/Gem/PythonTests/assetpipeline/fbx_tests/assets/TwoSceneFiles_OneWithPythonOneWithout_PythonOnlyRunsOnFirstScene/python_builder.py index 7ad5894a86..a9759e3d33 100644 --- a/AutomatedTesting/Gem/PythonTests/assetpipeline/fbx_tests/assets/TwoSceneFiles_OneWithPythonOneWithout_PythonOnlyRunsOnFirstScene/python_builder.py +++ b/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 diff --git a/AutomatedTesting/TestAssets/test_chunks_builder.py b/AutomatedTesting/TestAssets/test_chunks_builder.py index 2fd1ad9db9..b18902a34c 100755 --- a/AutomatedTesting/TestAssets/test_chunks_builder.py +++ b/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]) diff --git a/Code/Tools/SceneAPI/SceneCore/DataTypes/Rules/IScriptProcessorRule.h b/Code/Tools/SceneAPI/SceneCore/DataTypes/Rules/IScriptProcessorRule.h index 36be5f61d3..2b2abae561 100644 --- a/Code/Tools/SceneAPI/SceneCore/DataTypes/Rules/IScriptProcessorRule.h +++ b/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 diff --git a/Code/Tools/SceneAPI/SceneData/Behaviors/ScriptProcessorRuleBehavior.cpp b/Code/Tools/SceneAPI/SceneData/Behaviors/ScriptProcessorRuleBehavior.cpp index 8d413b6038..6046bfa620 100644 --- a/Code/Tools/SceneAPI/SceneData/Behaviors/ScriptProcessorRuleBehavior.cpp +++ b/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(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::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; } diff --git a/Code/Tools/SceneAPI/SceneData/Behaviors/ScriptProcessorRuleBehavior.h b/Code/Tools/SceneAPI/SceneData/Behaviors/ScriptProcessorRuleBehavior.h index a9ddf88df9..8903a8f257 100644 --- a/Code/Tools/SceneAPI/SceneData/Behaviors/ScriptProcessorRuleBehavior.h +++ b/Code/Tools/SceneAPI/SceneData/Behaviors/ScriptProcessorRuleBehavior.h @@ -54,7 +54,7 @@ namespace AZ::SceneAPI::Behaviors SCENE_DATA_API void GetManifestDependencyPaths(AZStd::vector& 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); diff --git a/Code/Tools/SceneAPI/SceneData/Rules/ScriptProcessorRule.cpp b/Code/Tools/SceneAPI/SceneData/Rules/ScriptProcessorRule.cpp index 5893764eb1..15c8c1b7f3 100644 --- a/Code/Tools/SceneAPI/SceneData/Rules/ScriptProcessorRule.cpp +++ b/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(context); if (serializeContext) { - serializeContext->Class()->Version(1) - ->Field("scriptFilename", &ScriptProcessorRule::m_scriptFilename); + serializeContext->Class()->Version(2) + ->Field("scriptFilename", &ScriptProcessorRule::m_scriptFilename) + ->Field("fallbackLogic", &ScriptProcessorRule::m_fallbackLogic); + + serializeContext->Enum() + ->Value("FailBuild", DataTypes::ScriptProcessorFallbackLogic::FailBuild) + ->Value("ContinueBuild", DataTypes::ScriptProcessorFallbackLogic::ContinueBuild); AZ::EditContext* editContext = serializeContext->GetEditContext(); if (editContext) diff --git a/Code/Tools/SceneAPI/SceneData/Rules/ScriptProcessorRule.h b/Code/Tools/SceneAPI/SceneData/Rules/ScriptProcessorRule.h index ad5e1de063..80cb670f9f 100644 --- a/Code/Tools/SceneAPI/SceneData/Rules/ScriptProcessorRule.h +++ b/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 diff --git a/Code/Tools/SceneAPI/SceneData/Tests/SceneManifest/SceneManifestRuleTests.cpp b/Code/Tools/SceneAPI/SceneData/Tests/SceneManifest/SceneManifestRuleTests.cpp index e0d5cca484..2bbbc1054a 100644 --- a/Code/Tools/SceneAPI/SceneData/Tests/SceneManifest/SceneManifestRuleTests.cpp +++ b/Code/Tools/SceneAPI/SceneData/Tests/SceneManifest/SceneManifestRuleTests.cpp @@ -11,6 +11,7 @@ #include #include #include +#include #include #include #include @@ -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(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(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(scene.GetManifest().GetValueStorage()); + EXPECT_EQ(view.begin()->GetScriptProcessorFallbackLogic(), DataTypes::ScriptProcessorFallbackLogic::ContinueBuild); + } } }