Browse Source

ATOM-14889: Fix for scriptProcessorRule doesn't save with field empty

* removed the script rule from the Editor, now will only be supported via a script or JSON manual edits
* Mesh Serialization - scriptProcessorRule doesn't save with field empty, but produces no error
* added a test to make sure Script Processor Rule operates with an empty filename

Jira: https://jira.agscollab.com/browse/ATOM-14889
Tests: Launched the Editor to removed the script rule from the Editor
jackalbe 4 years ago
parent
commit
77d06ecef7

+ 59 - 0
Code/Framework/AzCore/AzCore/UnitTest/Mocks/MockSettingsRegistry.h

@@ -0,0 +1,59 @@
+/*
+* All or portions of this file Copyright (c) Amazon.com, Inc. or its affiliates or
+* its licensors.
+*
+* For complete copyright and license terms please see the LICENSE at the root of this
+* distribution (the "License"). All use of this software is governed by the License,
+* or, if provided, by the license below or the license accompanying this file. Do not
+* remove or modify any license notices. This file is distributed on an "AS IS" BASIS,
+* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+*
+*/
+#pragma once
+
+#include <AzCore/UnitTest/UnitTest.h>
+#include <AzCore/Settings/SettingsRegistry.h>
+#include <gmock/gmock.h>
+
+namespace AZ
+{
+    class MockSettingsRegistry;
+    using NiceSettingsRegistrySimpleMock = ::testing::NiceMock<MockSettingsRegistry>;
+
+    class MockSettingsRegistry
+        : public AZ::SettingsRegistryInterface
+    {
+    public:
+        MOCK_CONST_METHOD1(GetType, Type(AZStd::string_view));
+        MOCK_CONST_METHOD2(Visit, bool(Visitor&, AZStd::string_view));
+        MOCK_CONST_METHOD2(Visit, bool(const VisitorCallback&, AZStd::string_view));
+        MOCK_METHOD1(RegisterNotifier, NotifyEventHandler(const NotifyCallback&));
+        MOCK_METHOD1(RegisterNotifier, NotifyEventHandler(NotifyCallback&&));
+
+        MOCK_CONST_METHOD2(Get, bool(bool&, AZStd::string_view));
+        MOCK_CONST_METHOD2(Get, bool(s64&, AZStd::string_view));
+        MOCK_CONST_METHOD2(Get, bool(u64&, AZStd::string_view));
+        MOCK_CONST_METHOD2(Get, bool(double&, AZStd::string_view));
+        MOCK_CONST_METHOD2(Get, bool(AZStd::string&, AZStd::string_view));
+        MOCK_CONST_METHOD2(Get, bool(FixedValueString&, AZStd::string_view));
+        MOCK_CONST_METHOD3(GetObject, bool(void*, Uuid, AZStd::string_view));
+
+        MOCK_METHOD2(Set, bool(AZStd::string_view, bool));
+        MOCK_METHOD2(Set, bool(AZStd::string_view, s64));
+        MOCK_METHOD2(Set, bool(AZStd::string_view, u64));
+        MOCK_METHOD2(Set, bool(AZStd::string_view, double));
+        MOCK_METHOD2(Set, bool(AZStd::string_view, AZStd::string_view));
+        MOCK_METHOD2(Set, bool(AZStd::string_view, const char*));
+        MOCK_METHOD3(SetObject, bool(AZStd::string_view, const void*, Uuid));
+
+        MOCK_METHOD1(Remove, bool(AZStd::string_view));
+
+        MOCK_METHOD3(MergeCommandLineArgument, bool(AZStd::string_view, AZStd::string_view, const CommandLineArgumentSettings&));
+        MOCK_METHOD2(MergeSettings, bool(AZStd::string_view, Format));
+        MOCK_METHOD4(MergeSettingsFile, bool(AZStd::string_view, Format, AZStd::string_view, AZStd::vector<char>*));
+        MOCK_METHOD5(
+            MergeSettingsFolder,
+            bool(AZStd::string_view, const Specializations&, AZStd::string_view, AZStd::string_view, AZStd::vector<char>*));
+    };
+} // namespace AZ
+

+ 1 - 0
Code/Framework/AzCore/AzCore/azcoretestcommon_files.cmake

@@ -15,4 +15,5 @@ set(FILES
     UnitTest/UnitTest.h
     UnitTest/TestTypes.h
     UnitTest/Mocks/MockFileIOBase.h
+    UnitTest/Mocks/MockSettingsRegistry.h
 )

+ 2 - 0
Code/Tools/SceneAPI/SceneCore/DllMain.cpp

@@ -42,6 +42,7 @@
 #include <SceneAPI/SceneCore/DataTypes/Rules/IMeshAdvancedRule.h>
 #include <SceneAPI/SceneCore/DataTypes/Rules/ILodRule.h>
 #include <SceneAPI/SceneCore/DataTypes/Rules/ISkeletonProxyRule.h>
+#include <SceneAPI/SceneCore/DataTypes/Rules/IScriptProcessorRule.h>
 #include <SceneAPI/SceneCore/DataTypes/GraphData/IAnimationData.h>
 #include <SceneAPI/SceneCore/DataTypes/GraphData/IBlendShapeData.h>
 #include <SceneAPI/SceneCore/DataTypes/GraphData/IBoneData.h>
@@ -168,6 +169,7 @@ namespace AZ
                     context->Class<AZ::SceneAPI::DataTypes::IMeshAdvancedRule, AZ::SceneAPI::DataTypes::IRule>()->Version(1);
                     context->Class<AZ::SceneAPI::DataTypes::ILodRule, AZ::SceneAPI::DataTypes::IRule>()->Version(1);
                     context->Class<AZ::SceneAPI::DataTypes::ISkeletonProxyRule, AZ::SceneAPI::DataTypes::IRule>()->Version(1);
+                    context->Class<AZ::SceneAPI::DataTypes::IScriptProcessorRule, AZ::SceneAPI::DataTypes::IRule>()->Version(1);
                     // Register graph data interfaces
                     context->Class<AZ::SceneAPI::DataTypes::IAnimationData, AZ::SceneAPI::DataTypes::IGraphObject>()->Version(1);
                     context->Class<AZ::SceneAPI::DataTypes::IBlendShapeData, AZ::SceneAPI::DataTypes::IGraphObject>()->Version(1);

+ 5 - 4
Code/Tools/SceneAPI/SceneData/Behaviors/ScriptProcessorRuleBehavior.h

@@ -12,6 +12,7 @@
 
 #pragma once
 
+#include <SceneAPI/SceneData/SceneDataConfiguration.h>
 #include <AzCore/std/string/string.h>
 #include <SceneAPI/SceneCore/Components/BehaviorComponent.h>
 #include <SceneAPI/SceneCore/Events/AssetImportRequest.h>
@@ -27,7 +28,7 @@ namespace AZ
     {
         namespace Behaviors
         {
-            class ScriptProcessorRuleBehavior
+            class SCENE_DATA_CLASS ScriptProcessorRuleBehavior
                 : public SceneCore::BehaviorComponent
                 , public Events::AssetImportRequestBus::Handler
             {
@@ -36,12 +37,12 @@ namespace AZ
 
                 ~ScriptProcessorRuleBehavior() override = default;
 
-                void Activate() override;
-                void Deactivate() override;
+                SCENE_DATA_API void Activate() override;
+                SCENE_DATA_API void Deactivate() override;
                 static void Reflect(ReflectContext* context);
 
                 // AssetImportRequestBus::Handler
-                Events::ProcessingResult UpdateManifest(
+                SCENE_DATA_API Events::ProcessingResult UpdateManifest(
                     Containers::Scene& scene,
                     ManifestAction action,
                     RequestingApplication requester) override;

+ 0 - 2
Code/Tools/SceneAPI/SceneData/ManifestMetaInfoHandler.cpp

@@ -26,7 +26,6 @@
 #include <SceneAPI/SceneData/Rules/LodRule.h>
 #include <SceneAPI/SceneData/Rules/MaterialRule.h>
 #include <SceneAPI/SceneData/Rules/StaticMeshAdvancedRule.h>
-#include <SceneAPI/SceneData/Rules/ScriptProcessorRule.h>
 #include <SceneAPI/SceneData/Rules/SkeletonProxyRule.h>
 #include <SceneAPI/SceneData/Rules/SkinMeshAdvancedRule.h>
 #include <SceneAPI/SceneData/Rules/SkinRule.h>
@@ -55,7 +54,6 @@ namespace AZ
             {
                 AZ_TraceContext("Object Type", target.RTTI_GetTypeName());
                 modifiers.push_back(SceneData::CommentRule::TYPEINFO_Uuid());
-                modifiers.push_back(SceneData::ScriptProcessorRule::TYPEINFO_Uuid());
 
                 if (target.RTTI_IsTypeOf(DataTypes::IMeshGroup::TYPEINFO_Uuid()))
                 {

+ 54 - 2
Code/Tools/SceneAPI/SceneData/Tests/SceneManifest/SceneManifestRuleTests.cpp

@@ -13,19 +13,23 @@
 #include <AzTest/AzTest.h>
 
 #include <SceneAPI/SceneCore/Containers/SceneManifest.h>
+#include <SceneAPI/SceneCore/Containers/Scene.h>
+#include <SceneAPI/SceneCore/DataTypes/Rules/IScriptProcessorRule.h>
 #include <SceneAPI/SceneData/ReflectionRegistrar.h>
 #include <SceneAPI/SceneData/Rules/CoordinateSystemRule.h>
+#include <SceneAPI/SceneData/Behaviors/ScriptProcessorRuleBehavior.h>
 
+#include <AzCore/Math/Quaternion.h>
 #include <AzCore/Name/NameDictionary.h>
 #include <AzCore/RTTI/BehaviorContext.h>
 #include <AzCore/RTTI/ReflectionManager.h>
-#include <AzCore/Serialization/Json/RegistrationContext.h>
 #include <AzCore/Serialization/Json/JsonSystemComponent.h>
+#include <AzCore/Serialization/Json/RegistrationContext.h>
 #include <AzCore/std/smart_ptr/make_shared.h>
 #include <AzCore/std/smart_ptr/shared_ptr.h>
+#include <AzCore/UnitTest/Mocks/MockSettingsRegistry.h>
 #include <AzCore/UnitTest/TestTypes.h>
 #include <AzFramework/FileFunc/FileFunc.h>
-#include <AzCore/Math/Quaternion.h>
 
 namespace AZ
 {
@@ -94,6 +98,19 @@ namespace AZ
 
                 m_jsonSystemComponent = AZStd::make_unique<JsonSystemComponent>();
                 m_jsonSystemComponent->Reflect(m_jsonRegistrationContext.get());
+
+                m_data.reset(new DataMembers);
+
+                using FixedValueString = AZ::SettingsRegistryInterface::FixedValueString;
+
+                ON_CALL(m_data->m_settings, Get(::testing::Matcher<FixedValueString&>(::testing::_), testing::_))
+                    .WillByDefault([](FixedValueString& value, AZStd::string_view) -> bool
+                        {
+                            value = "mock_path";
+                            return true;
+                        });
+
+                AZ::SettingsRegistry::Register(&m_data->m_settings);
             }
 
             void TearDown() override
@@ -106,9 +123,19 @@ namespace AZ
                 m_jsonRegistrationContext.reset();
                 m_jsonSystemComponent.reset();
 
+                AZ::SettingsRegistry::Unregister(&m_data->m_settings);
+                m_data.reset();
+
                 AZ::NameDictionary::Destroy();
                 UnitTest::AllocatorsFixture::TearDown();
             }
+
+            struct DataMembers
+            {
+                AZ::NiceSettingsRegistrySimpleMock  m_settings;
+            };
+
+            AZStd::unique_ptr<DataMembers> m_data;
         };
 
         TEST_F(SceneManifest_JSON, LoadFromString_BlankManifest_HasDefaultParts)
@@ -223,5 +250,30 @@ namespace AZ
             EXPECT_THAT(jsonText.c_str(), ::testing::HasSubstr(R"(3.0)"));
             EXPECT_THAT(jsonText.c_str(), ::testing::HasSubstr(R"("scale": 10.0)"));
         }
+
+        TEST_F(SceneManifest_JSON, ScriptProcessorRule_LoadWithEmptyScriptFilename_ReturnsEarly)
+        {
+            using namespace SceneAPI::Containers;
+            using namespace SceneAPI::Events;
+
+            constexpr const char* jsonManifest = { R"JSON(
+            {
+                "values": [
+                    {
+                        "$type": "ScriptProcessorRule",
+                        "scriptFilename": ""
+                    }
+                ]
+            })JSON" };
+
+            auto scene = AZ::SceneAPI::Containers::Scene("mock");
+            auto result = scene.GetManifest().LoadFromString(jsonManifest, m_serializeContext.get(), m_jsonRegistrationContext.get());
+            EXPECT_TRUE(result.IsSuccess());
+            EXPECT_FALSE(scene.GetManifest().IsEmpty());
+
+            auto scriptProcessorRuleBehavior =  AZ::SceneAPI::Behaviors::ScriptProcessorRuleBehavior();
+            auto update = scriptProcessorRuleBehavior.UpdateManifest(scene, AssetImportRequest::Update, AssetImportRequest::Generic);
+            EXPECT_EQ(update, ProcessingResult::Ignored);
+        }
     }
 }