From fb6e6e339fe8777bc0304b33d60098ac87f11534 Mon Sep 17 00:00:00 2001 From: Esteban Papp <81431996+amznestebanpapp@users.noreply.github.com> Date: Tue, 30 Nov 2021 15:07:57 -0800 Subject: [PATCH 1/9] Add CRC validator (#5857) * Adds crc validation checks Signed-off-by: Esteban Papp <81431996+amznestebanpapp@users.noreply.github.com> * Fixes invalid CRCs Signed-off-by: Esteban Papp <81431996+amznestebanpapp@users.noreply.github.com> * Changes test to smoke suite Signed-off-by: Esteban Papp <81431996+amznestebanpapp@users.noreply.github.com> * excludes some test data from the validator Signed-off-by: Esteban Papp <81431996+amznestebanpapp@users.noreply.github.com> * uses pathlib instead of os.path Signed-off-by: Esteban Papp <81431996+amznestebanpapp@users.noreply.github.com> * fixes wrong path to test scripts Signed-off-by: Esteban Papp <81431996+amznestebanpapp@users.noreply.github.com> * Escape not needed Signed-off-by: Esteban Papp <81431996+amznestebanpapp@users.noreply.github.com> --- Code/Framework/AzCore/AzCore/Math/Crc.h | 4 +- .../AzToolsFramework/Viewport/ActionBus.h | 10 ++-- .../Code/Source/AtomBridgeSystemComponent.cpp | 4 +- .../AssetCollectionAsyncLoaderTestComponent.h | 4 +- ...tomViewportDisplayIconsSystemComponent.cpp | 2 +- ...clusionCullingPlaneComponentController.cpp | 4 +- .../Source/Editor/EditorSystemComponent.h | 4 +- .../NodePalette/InputOutputNodePaletteItem.h | 2 +- .../NodePalette/ModuleNodePaletteItem.h | 2 +- .../NodePalette/StandardNodePaletteItem.h | 2 +- .../Shape/EditorTubeShapeComponentMode.cpp | 2 +- .../PropertyHandlerUiParticleColorKeyframe.h | 2 +- .../PropertyHandlerUiParticleFloatKeyframe.h | 2 +- .../LyShine/Code/Source/LyShineLoadScreen.cpp | 4 +- .../Code/Editor/ColliderComponentMode.cpp | 8 ++-- .../Code/Source/EditorSystemComponent.h | 2 +- .../EditorWhiteBoxDefaultMode.cpp | 4 +- scripts/commit_validation/CMakeLists.txt | 6 +-- .../commit_validation/commit_validation.py | 2 + .../tests/validators/test_crc_validator.py | 47 ++++++++++++++++++ .../validators/crc_validator.py | 48 +++++++++++++++++++ 21 files changed, 130 insertions(+), 35 deletions(-) create mode 100644 scripts/commit_validation/commit_validation/tests/validators/test_crc_validator.py create mode 100644 scripts/commit_validation/commit_validation/validators/crc_validator.py diff --git a/Code/Framework/AzCore/AzCore/Math/Crc.h b/Code/Framework/AzCore/AzCore/Math/Crc.h index 9ba2a83139..c71339e92e 100644 --- a/Code/Framework/AzCore/AzCore/Math/Crc.h +++ b/Code/Framework/AzCore/AzCore/Math/Crc.h @@ -16,7 +16,7 @@ // // When AZ_CRC("My string") is used by default it will map to AZ::Crc32("My string"). // We do have a pro-processor program which will precompute the crc for you and -// transform that macro to AZ_CRC("My string",0xabcdef00) this will expand to just 0xabcdef00. +// transform that macro to AZ_CRC("My string", 0x18fbd270) this will expand to just 0x18fbd270. // This will remove completely the "My string" from your executable, it will add it to a database and so on. // WHen you want to update the string, just change the string. // If you don't run the precompile step the code should still run fine, except it will be slower, @@ -24,7 +24,7 @@ // a constant expression. // For example // switch(id) { -// case AZ_CRC("My string",0xabcdef00): {} break; // this will compile fine +// case AZ_CRC("My string",0x18fbd270): {} break; // this will compile fine // case AZ_CRC("My string"): {} break; // this will cause "error C2051: case expression not constant" // } // So it's you choice what you do, depending on your needs. diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Viewport/ActionBus.h b/Code/Framework/AzToolsFramework/AzToolsFramework/Viewport/ActionBus.h index 4dde676511..c0562025b6 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Viewport/ActionBus.h +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Viewport/ActionBus.h @@ -23,11 +23,11 @@ namespace AzToolsFramework /// @name Reverse URLs. /// Used to identify common actions and override them when necessary. //@{ - static const AZ::Crc32 s_backAction = AZ_CRC("com.o3de.action.common.back", 0xd772a2af); - static const AZ::Crc32 s_deleteAction = AZ_CRC("com.o3de.action.common.delete", 0x5731f6cb); - static const AZ::Crc32 s_duplicateAction = AZ_CRC("com.o3de.action.common.duplicate", 0x08ccf461); - static const AZ::Crc32 s_nextComponentMode = AZ_CRC("com.o3de.action.common.nextComponentMode", 0xcc26094f); - static const AZ::Crc32 s_previousComponentMode = AZ_CRC("com.o3de.action.common.previousComponentMode", 0x0d18ff39); + static const AZ::Crc32 s_backAction = AZ_CRC("com.o3de.action.common.back", 0x80c3030f); + static const AZ::Crc32 s_deleteAction = AZ_CRC("com.o3de.action.common.delete", 0x58e78eed); + static const AZ::Crc32 s_duplicateAction = AZ_CRC("com.o3de.action.common.duplicate", 0xbc5a4a23); + static const AZ::Crc32 s_nextComponentMode = AZ_CRC("com.o3de.action.common.nextComponentMode", 0xf9aca3a8); + static const AZ::Crc32 s_previousComponentMode = AZ_CRC("com.o3de.action.common.previousComponentMode", 0x0580eaec); //@} /// Specific Action properties to be sent to a type implementing diff --git a/Gems/AtomLyIntegration/AtomBridge/Code/Source/AtomBridgeSystemComponent.cpp b/Gems/AtomLyIntegration/AtomBridge/Code/Source/AtomBridgeSystemComponent.cpp index d822b16486..7a980f9824 100644 --- a/Gems/AtomLyIntegration/AtomBridge/Code/Source/AtomBridgeSystemComponent.cpp +++ b/Gems/AtomLyIntegration/AtomBridge/Code/Source/AtomBridgeSystemComponent.cpp @@ -67,12 +67,12 @@ namespace AZ void AtomBridgeSystemComponent::GetProvidedServices(ComponentDescriptor::DependencyArrayType& provided) { - provided.push_back(AZ_CRC("AtomBridgeService", 0xdb816a99)); + provided.push_back(AZ_CRC("AtomBridgeService", 0x92d990b5)); } void AtomBridgeSystemComponent::GetIncompatibleServices(ComponentDescriptor::DependencyArrayType& incompatible) { - incompatible.push_back(AZ_CRC("AtomBridgeService", 0xdb816a99)); + incompatible.push_back(AZ_CRC("AtomBridgeService", 0x92d990b5)); } void AtomBridgeSystemComponent::GetRequiredServices(ComponentDescriptor::DependencyArrayType& required) diff --git a/Gems/AtomLyIntegration/AtomBridge/Code/Source/Editor/AssetCollectionAsyncLoaderTestComponent.h b/Gems/AtomLyIntegration/AtomBridge/Code/Source/Editor/AssetCollectionAsyncLoaderTestComponent.h index 9ab25741af..3af1ad8907 100644 --- a/Gems/AtomLyIntegration/AtomBridge/Code/Source/Editor/AssetCollectionAsyncLoaderTestComponent.h +++ b/Gems/AtomLyIntegration/AtomBridge/Code/Source/Editor/AssetCollectionAsyncLoaderTestComponent.h @@ -109,12 +109,12 @@ namespace AZ static void GetProvidedServices(AZ::ComponentDescriptor::DependencyArrayType& services) { - services.push_back(AZ_CRC("AssetCollectionAsyncLoaderTest", 0xdd5ab934)); + services.push_back(AZ_CRC("AssetCollectionAsyncLoaderTest", 0x66d04369)); } static void GetIncompatibleServices(AZ::ComponentDescriptor::DependencyArrayType& services) { - services.push_back(AZ_CRC("AssetCollectionAsyncLoaderTest", 0xdd5ab934)); + services.push_back(AZ_CRC("AssetCollectionAsyncLoaderTest", 0x66d04369)); } static void Reflect(AZ::ReflectContext* context); diff --git a/Gems/AtomLyIntegration/AtomViewportDisplayIcons/Code/Source/AtomViewportDisplayIconsSystemComponent.cpp b/Gems/AtomLyIntegration/AtomViewportDisplayIcons/Code/Source/AtomViewportDisplayIconsSystemComponent.cpp index 38679432ea..fa7a0cf2bf 100644 --- a/Gems/AtomLyIntegration/AtomViewportDisplayIcons/Code/Source/AtomViewportDisplayIconsSystemComponent.cpp +++ b/Gems/AtomLyIntegration/AtomViewportDisplayIcons/Code/Source/AtomViewportDisplayIconsSystemComponent.cpp @@ -73,7 +73,7 @@ namespace AZ::Render void AtomViewportDisplayIconsSystemComponent::GetRequiredServices(AZ::ComponentDescriptor::DependencyArrayType& required) { required.push_back(AZ_CRC("RPISystem", 0xf2add773)); - required.push_back(AZ_CRC("AtomBridgeService", 0xdb816a99)); + required.push_back(AZ_CRC("AtomBridgeService", 0x92d990b5)); } void AtomViewportDisplayIconsSystemComponent::GetDependentServices([[maybe_unused]] AZ::ComponentDescriptor::DependencyArrayType& dependent) diff --git a/Gems/AtomLyIntegration/CommonFeatures/Code/Source/OcclusionCullingPlane/OcclusionCullingPlaneComponentController.cpp b/Gems/AtomLyIntegration/CommonFeatures/Code/Source/OcclusionCullingPlane/OcclusionCullingPlaneComponentController.cpp index 4c78a0afd7..4addbde3ef 100644 --- a/Gems/AtomLyIntegration/CommonFeatures/Code/Source/OcclusionCullingPlane/OcclusionCullingPlaneComponentController.cpp +++ b/Gems/AtomLyIntegration/CommonFeatures/Code/Source/OcclusionCullingPlane/OcclusionCullingPlaneComponentController.cpp @@ -59,12 +59,12 @@ namespace AZ void OcclusionCullingPlaneComponentController::GetProvidedServices(AZ::ComponentDescriptor::DependencyArrayType& provided) { - provided.push_back(AZ_CRC("OcclusionCullingPlaneService", 0x9123f33d)); + provided.push_back(AZ_CRC("OcclusionCullingPlaneService", 0x7d036c2e)); } void OcclusionCullingPlaneComponentController::GetIncompatibleServices(AZ::ComponentDescriptor::DependencyArrayType& incompatible) { - incompatible.push_back(AZ_CRC("OcclusionCullingPlaneService", 0x9123f33d)); + incompatible.push_back(AZ_CRC("OcclusionCullingPlaneService", 0x7d036c2e)); } void OcclusionCullingPlaneComponentController::GetRequiredServices(AZ::ComponentDescriptor::DependencyArrayType& required) diff --git a/Gems/Blast/Code/Source/Editor/EditorSystemComponent.h b/Gems/Blast/Code/Source/Editor/EditorSystemComponent.h index 31daae1a18..fe17bfaac0 100644 --- a/Gems/Blast/Code/Source/Editor/EditorSystemComponent.h +++ b/Gems/Blast/Code/Source/Editor/EditorSystemComponent.h @@ -31,12 +31,12 @@ namespace Blast private: static void GetProvidedServices(AZ::ComponentDescriptor::DependencyArrayType& provided) { - provided.push_back(AZ_CRC("BlastEditorService", 0x0a61cda5)); + provided.push_back(AZ_CRC("BlastEditorService", 0xeddfed0d)); } static void GetRequiredServices(AZ::ComponentDescriptor::DependencyArrayType& required) { - required.push_back(AZ_CRC("BlastService", 0x75beae2d)); + required.push_back(AZ_CRC("BlastService", 0x46927a9f)); } AZStd::unique_ptr m_editorBlastChunksAssetHandler; diff --git a/Gems/GraphModel/Code/Include/GraphModel/Integration/NodePalette/InputOutputNodePaletteItem.h b/Gems/GraphModel/Code/Include/GraphModel/Integration/NodePalette/InputOutputNodePaletteItem.h index 15f308c4d2..99a32d965b 100644 --- a/Gems/GraphModel/Code/Include/GraphModel/Integration/NodePalette/InputOutputNodePaletteItem.h +++ b/Gems/GraphModel/Code/Include/GraphModel/Integration/NodePalette/InputOutputNodePaletteItem.h @@ -33,7 +33,7 @@ namespace GraphModelIntegration //! Constructor //! \param nodeName Name of the node that will show up in the Palette - //! \param editorId Unique name of the client system editor (ex: AZ_CRC("ShaderCanvas", 0xa6d1a85a)) + //! \param editorId Unique name of the client system editor (ex: AZ_CRC("ShaderCanvas", 0x0a1dff96)) //! \param dataType The type of data that the InputGraphNode or OutputGraphNode will represent InputOutputNodePaletteItem(AZStd::string_view nodeName, GraphCanvas::EditorId editorId, GraphModel::DataTypePtr dataType) : DraggableNodePaletteTreeItem(nodeName, editorId) diff --git a/Gems/GraphModel/Code/Include/GraphModel/Integration/NodePalette/ModuleNodePaletteItem.h b/Gems/GraphModel/Code/Include/GraphModel/Integration/NodePalette/ModuleNodePaletteItem.h index a0121ac035..51e1fca55b 100644 --- a/Gems/GraphModel/Code/Include/GraphModel/Integration/NodePalette/ModuleNodePaletteItem.h +++ b/Gems/GraphModel/Code/Include/GraphModel/Integration/NodePalette/ModuleNodePaletteItem.h @@ -95,7 +95,7 @@ namespace GraphModelIntegration AZ_CLASS_ALLOCATOR(ModuleNodePaletteItem, AZ::SystemAllocator, 0); //! Constructor - //! \param editorId Unique name of the client system editor (ex: AZ_CRC("ShaderCanvas", 0xa6d1a85a)) + //! \param editorId Unique name of the client system editor (ex: AZ_CRC("ShaderCanvas", 0x0a1dff96)) //! \param sourceFileId The unique id for the module node graph source file. //! \param sourceFilePath The path to the module node graph source file. This will be used for node naming and debug output. ModuleNodePaletteItem(GraphCanvas::EditorId editorId, AZ::Uuid sourceFileId, AZStd::string_view sourceFilePath) diff --git a/Gems/GraphModel/Code/Include/GraphModel/Integration/NodePalette/StandardNodePaletteItem.h b/Gems/GraphModel/Code/Include/GraphModel/Integration/NodePalette/StandardNodePaletteItem.h index 5d2914d7ad..e38e3b7478 100644 --- a/Gems/GraphModel/Code/Include/GraphModel/Integration/NodePalette/StandardNodePaletteItem.h +++ b/Gems/GraphModel/Code/Include/GraphModel/Integration/NodePalette/StandardNodePaletteItem.h @@ -34,7 +34,7 @@ namespace GraphModelIntegration //! Constructor //! \param nodeName Name of the node that will show up in the Palette - //! \param editorId Unique name of the client system editor (ex: AZ_CRC("ShaderCanvas", 0xa6d1a85a)) + //! \param editorId Unique name of the client system editor (ex: AZ_CRC("ShaderCanvas", 0x0a1dff96)) StandardNodePaletteItem(AZStd::string_view nodeName, GraphCanvas::EditorId editorId) : DraggableNodePaletteTreeItem(nodeName, editorId) { diff --git a/Gems/LmbrCentral/Code/Source/Shape/EditorTubeShapeComponentMode.cpp b/Gems/LmbrCentral/Code/Source/Shape/EditorTubeShapeComponentMode.cpp index 340752251d..fd91c84c12 100644 --- a/Gems/LmbrCentral/Code/Source/Shape/EditorTubeShapeComponentMode.cpp +++ b/Gems/LmbrCentral/Code/Source/Shape/EditorTubeShapeComponentMode.cpp @@ -22,7 +22,7 @@ namespace LmbrCentral { AZ_CLASS_ALLOCATOR_IMPL(EditorTubeShapeComponentMode, AZ::SystemAllocator, 0) - static const AZ::Crc32 s_resetVariableRadii = AZ_CRC("com.o3de.action.tubeshape.reset_radii", 0x0f2ef8e2); + static const AZ::Crc32 s_resetVariableRadii = AZ_CRC("com.o3de.action.tubeshape.reset_radii", 0xa987659c); static const char* const s_resetRadiiTitle = "Reset Radii"; static const char* const s_resetRadiiDesc = "Reset all variable radius values to the default"; diff --git a/Gems/LyShine/Code/Editor/PropertyHandlerUiParticleColorKeyframe.h b/Gems/LyShine/Code/Editor/PropertyHandlerUiParticleColorKeyframe.h index 9fdc8e61c4..639d042b34 100644 --- a/Gems/LyShine/Code/Editor/PropertyHandlerUiParticleColorKeyframe.h +++ b/Gems/LyShine/Code/Editor/PropertyHandlerUiParticleColorKeyframe.h @@ -50,7 +50,7 @@ class PropertyHandlerUiParticleColorKeyframe public: AZ_CLASS_ALLOCATOR(PropertyHandlerUiParticleColorKeyframe, AZ::SystemAllocator, 0); - AZ::u32 GetHandlerName(void) const override { return AZ_CRC("UiParticleColorKeyframeCtrl", 0x8cb3a9f1); } + AZ::u32 GetHandlerName(void) const override { return AZ_CRC("UiParticleColorKeyframeCtrl", 0xe3ef28b6); } bool IsDefaultHandler() const override { return true; } QWidget* CreateGUI(QWidget* pParent) override; void ConsumeAttribute(PropertyUiParticleColorKeyframeCtrl* GUI, AZ::u32 attrib, AzToolsFramework::PropertyAttributeReader* attrValue, const char* debugName) override; diff --git a/Gems/LyShine/Code/Editor/PropertyHandlerUiParticleFloatKeyframe.h b/Gems/LyShine/Code/Editor/PropertyHandlerUiParticleFloatKeyframe.h index 39e5dd27c2..df3983dc3e 100644 --- a/Gems/LyShine/Code/Editor/PropertyHandlerUiParticleFloatKeyframe.h +++ b/Gems/LyShine/Code/Editor/PropertyHandlerUiParticleFloatKeyframe.h @@ -50,7 +50,7 @@ class PropertyHandlerUiParticleFloatKeyframe public: AZ_CLASS_ALLOCATOR(PropertyHandlerUiParticleFloatKeyframe, AZ::SystemAllocator, 0); - AZ::u32 GetHandlerName(void) const override { return AZ_CRC("UiParticleFloatKeyframeCtrl", 0xba9359a2); } + AZ::u32 GetHandlerName(void) const override { return AZ_CRC("UiParticleFloatKeyframeCtrl", 0x448a90ec); } bool IsDefaultHandler() const override { return true; } QWidget* CreateGUI(QWidget* pParent) override; void ConsumeAttribute(PropertyUiParticleFloatKeyframeCtrl* GUI, AZ::u32 attrib, AzToolsFramework::PropertyAttributeReader* attrValue, const char* debugName) override; diff --git a/Gems/LyShine/Code/Source/LyShineLoadScreen.cpp b/Gems/LyShine/Code/Source/LyShineLoadScreen.cpp index 279c0527bd..831bdb048a 100644 --- a/Gems/LyShine/Code/Source/LyShineLoadScreen.cpp +++ b/Gems/LyShine/Code/Source/LyShineLoadScreen.cpp @@ -31,12 +31,12 @@ namespace LyShine void LyShineLoadScreenComponent::GetProvidedServices(AZ::ComponentDescriptor::DependencyArrayType& provided) { - provided.emplace_back(AZ_CRC("LyShineLoadScreenService", 0xBB5EAB17)); + provided.emplace_back(AZ_CRC("LyShineLoadScreenService", 0xbb5eab17)); } void LyShineLoadScreenComponent::GetIncompatibleServices(AZ::ComponentDescriptor::DependencyArrayType& incompatible) { - incompatible.emplace_back(AZ_CRC("LyShineLoadScreenService", 0xBB5EAB17)); + incompatible.emplace_back(AZ_CRC("LyShineLoadScreenService", 0xbb5eab17)); } void LyShineLoadScreenComponent::Init() diff --git a/Gems/PhysX/Code/Editor/ColliderComponentMode.cpp b/Gems/PhysX/Code/Editor/ColliderComponentMode.cpp index e5e75d88a3..b91b3e5bb9 100644 --- a/Gems/PhysX/Code/Editor/ColliderComponentMode.cpp +++ b/Gems/PhysX/Code/Editor/ColliderComponentMode.cpp @@ -27,10 +27,10 @@ namespace PhysX namespace { //! Uri's for shortcut actions. - const AZ::Crc32 SetDimensionsSubModeActionUri = AZ_CRC("com.o3de.action.physx.setdimensionssubmode", 0x77b70dd6); - const AZ::Crc32 SetOffsetSubModeActionUri = AZ_CRC("com.o3de.action.physx.setoffsetsubmode", 0xc06132e5); - const AZ::Crc32 SetRotationSubModeActionUri = AZ_CRC("com.o3de.action.physx.setrotationsubmode", 0xc4225918); - const AZ::Crc32 ResetSubModeActionUri = AZ_CRC("com.o3de.action.physx.resetsubmode", 0xb70b120e); + const AZ::Crc32 SetDimensionsSubModeActionUri = AZ_CRC("com.o3de.action.physx.setdimensionssubmode", 0x508b1781); + const AZ::Crc32 SetOffsetSubModeActionUri = AZ_CRC("com.o3de.action.physx.setoffsetsubmode", 0x777ac743); + const AZ::Crc32 SetRotationSubModeActionUri = AZ_CRC("com.o3de.action.physx.setrotationsubmode", 0xf1a8f3ff); + const AZ::Crc32 ResetSubModeActionUri = AZ_CRC("com.o3de.action.physx.resetsubmode", 0x599d1594); } // namespace AZ_CLASS_ALLOCATOR_IMPL(ColliderComponentMode, AZ::SystemAllocator, 0); diff --git a/Gems/PhysXDebug/Code/Source/EditorSystemComponent.h b/Gems/PhysXDebug/Code/Source/EditorSystemComponent.h index 14f37ef01e..fc0d60640c 100644 --- a/Gems/PhysXDebug/Code/Source/EditorSystemComponent.h +++ b/Gems/PhysXDebug/Code/Source/EditorSystemComponent.h @@ -32,7 +32,7 @@ namespace PhysXDebug static void GetProvidedServices(AZ::ComponentDescriptor::DependencyArrayType& provided) { - provided.push_back(AZ_CRC("PhysXDebugEditorService", 0xe3dde7d8)); + provided.push_back(AZ_CRC("PhysXDebugEditorService", 0xf8611967)); } static void GetRequiredServices(AZ::ComponentDescriptor::DependencyArrayType& required) diff --git a/Gems/WhiteBox/Code/Source/SubComponentModes/EditorWhiteBoxDefaultMode.cpp b/Gems/WhiteBox/Code/Source/SubComponentModes/EditorWhiteBoxDefaultMode.cpp index 3d6d9a4c0a..608605be62 100644 --- a/Gems/WhiteBox/Code/Source/SubComponentModes/EditorWhiteBoxDefaultMode.cpp +++ b/Gems/WhiteBox/Code/Source/SubComponentModes/EditorWhiteBoxDefaultMode.cpp @@ -33,8 +33,8 @@ namespace WhiteBox AZ::Color, cl_whiteBoxVertexIndicatorColor, AZ::Color::CreateFromRgba(0, 0, 0, 102), nullptr, AZ::ConsoleFunctorFlags::Null, "The color of the vertex indicator"); - static const AZ::Crc32 HideEdge = AZ_CRC("com.o3de.action.whitebox.hide_edge", 0x6a60ae23); - static const AZ::Crc32 HideVertex = AZ_CRC("com.o3de.action.whitebox.hide_vertex", 0x4a4bd092); + static const AZ::Crc32 HideEdge = AZ_CRC("com.o3de.action.whitebox.hide_edge", 0x84f6a9b9); + static const AZ::Crc32 HideVertex = AZ_CRC("com.o3de.action.whitebox.hide_vertex", 0x5f81c937); static const char* const HideEdgeTitle = "Hide Edge"; static const char* const HideEdgeDesc = "Hide the selected edge to merge the two connected polygons"; diff --git a/scripts/commit_validation/CMakeLists.txt b/scripts/commit_validation/CMakeLists.txt index 6079515eb8..00e8506ef1 100644 --- a/scripts/commit_validation/CMakeLists.txt +++ b/scripts/commit_validation/CMakeLists.txt @@ -6,10 +6,8 @@ # # -# this ctest makes sure that the commit validation function -# also runs its tests during commit validation! ly_add_pytest( NAME test_commit_validation - PATH ${CMAKE_CURRENT_LIST_DIR} + PATH ${CMAKE_CURRENT_LIST_DIR}/commit_validation/tests + TEST_SUITE smoke ) - diff --git a/scripts/commit_validation/commit_validation/commit_validation.py b/scripts/commit_validation/commit_validation/commit_validation.py index 513244a735..be8fc4ce1e 100755 --- a/scripts/commit_validation/commit_validation/commit_validation.py +++ b/scripts/commit_validation/commit_validation/commit_validation.py @@ -181,4 +181,6 @@ EXCLUDED_VALIDATION_PATTERNS = [ 'restricted/*/Tools/*RemoteControl', '*/user/Cache/*', '*/user/log/*', + '*/user/log_test_1/*', + '*/user/log_test_2/*', ] diff --git a/scripts/commit_validation/commit_validation/tests/validators/test_crc_validator.py b/scripts/commit_validation/commit_validation/tests/validators/test_crc_validator.py new file mode 100644 index 0000000000..6fbe86ab29 --- /dev/null +++ b/scripts/commit_validation/commit_validation/tests/validators/test_crc_validator.py @@ -0,0 +1,47 @@ +# +# 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 +# +# + +import unittest +from unittest.mock import patch, mock_open + +from commit_validation.tests.mocks.mock_commit import MockCommit +from commit_validation.validators.crc_validator import CrcValidator + + +class CrcValidatorTests(unittest.TestCase): + + @patch('builtins.open', mock_open(read_data='This file does not contain an AZ_CRC macro')) + def test_fileWithNoCrc_passes(self): + commit = MockCommit(files=['/someCppFile.cpp']) + error_list = [] + self.assertTrue(CrcValidator().run(commit, error_list)) + self.assertEqual(len(error_list), 0, f"Unexpected errors: {error_list}") + + @patch('builtins.open', mock_open(read_data='This file contains an invalid CRC macro AZ_CRC("My string", 0xabcdef00)')) + def test_fileWithInvalidCrc_fails(self): + commit = MockCommit(files=['/someCppFile.cpp']) + error_list = [] + self.assertFalse(CrcValidator().run(commit, error_list)) + self.assertNotEqual(len(error_list), 0, f"Errors were expected but none were returned.") + + @patch('builtins.open', mock_open(read_data='This file contains a valid CRC macro AZ_CRC("My string", 0x18fbd270)')) + def test_fileWithValidCrc_fails(self): + commit = MockCommit(files=['/someCppFile.cpp']) + error_list = [] + self.assertTrue(CrcValidator().run(commit, error_list)) + self.assertEqual(len(error_list), 0, f"Unexpected errors: {error_list}") + + @patch('builtins.open', mock_open(read_data='This file contains an invalid CRC macro AZ_CRC("My string", 0xabcdef00)')) + def test_fileExtensionIgnored_passes(self): + commit = MockCommit(files=['/someCppFile.somerandomextension']) + error_list = [] + self.assertTrue(CrcValidator().run(commit, error_list)) + self.assertEqual(len(error_list), 0, f"Unexpected errors: {error_list}") + +if __name__ == '__main__': + unittest.main() diff --git a/scripts/commit_validation/commit_validation/validators/crc_validator.py b/scripts/commit_validation/commit_validation/validators/crc_validator.py new file mode 100644 index 0000000000..db99d7b50b --- /dev/null +++ b/scripts/commit_validation/commit_validation/validators/crc_validator.py @@ -0,0 +1,48 @@ +# +# 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 +# +# + +import binascii +import fnmatch +import pathlib +import re +from typing import Type, List + +from commit_validation.commit_validation import Commit, CommitValidator, SOURCE_FILE_EXTENSIONS, EXCLUDED_VALIDATION_PATTERNS, VERBOSE + +class CrcValidator(CommitValidator): + """A file-level validator that makes sure a file does not contain an invalid CRC""" + + def run(self, commit: Commit, errors: List[str]) -> bool: + for file_name in commit.get_files(): + for pattern in EXCLUDED_VALIDATION_PATTERNS: + if fnmatch.fnmatch(file_name, pattern): + if VERBOSE: print(f'{file_name}::{self.__class__.__name__} SKIPPED - Validation pattern excluded on path.') + break + else: + if pathlib.Path(file_name).suffix.lower() not in SOURCE_FILE_EXTENSIONS: + if VERBOSE: print(f'{file_name}::{self.__class__.__name__} SKIPPED - File excluded based on extension.') + continue + + with open(file_name, mode='r', encoding='utf8') as fh: + fileContents = fh.read() + matchesFound = re.findall(r'AZ_CRC\("([^"]+)",([^)]*)\)', fileContents) + for element in matchesFound: + stringInCode = element[0] + valueInCode = element[1].strip() + expectedValue = "{0:#0{1}x}".format(binascii.crc32(stringInCode.lower().encode('utf8')), 10) + if expectedValue != valueInCode: + error_message = str(f'{file_name}::{self.__class__.__name__} FAILED - Source file contains a CRC mismatch!\n' + f' AZ_CRC("{stringInCode}", {valueInCode}), expected value {expectedValue}') + if VERBOSE: print(error_message) + errors.append(error_message) + return (not errors) + + +def get_validator() -> Type[CrcValidator]: + """Returns the validator class for this module""" + return CrcValidator From 93ec5de5525ad573b203ae33ea671fe23b82c047 Mon Sep 17 00:00:00 2001 From: Esteban Papp <81431996+amznestebanpapp@users.noreply.github.com> Date: Tue, 30 Nov 2021 15:09:20 -0800 Subject: [PATCH 2/9] Enables monolithic for ServerLauncher (#5883) Signed-off-by: Esteban Papp <81431996+amznestebanpapp@users.noreply.github.com> --- cmake/Monolithic.cmake | 1 - 1 file changed, 1 deletion(-) diff --git a/cmake/Monolithic.cmake b/cmake/Monolithic.cmake index 1e121c2b20..2310d8d0d7 100644 --- a/cmake/Monolithic.cmake +++ b/cmake/Monolithic.cmake @@ -16,7 +16,6 @@ if(LY_MONOLITHIC_GAME) ly_set(PAL_TRAIT_BUILD_HOST_TOOLS FALSE) ly_set(PAL_TRAIT_BUILD_HOST_GUI_TOOLS FALSE) ly_set(PAL_TRAIT_BUILD_TESTS_SUPPORTED FALSE) - ly_set(PAL_TRAIT_BUILD_SERVER_SUPPORTED FALSE) else() ly_set(PAL_TRAIT_MONOLITHIC_DRIVEN_LIBRARY_TYPE SHARED) ly_set(PAL_TRAIT_MONOLITHIC_DRIVEN_MODULE_TYPE GEM_MODULE) From 28096617941313ecce272038378a7ae3735cc885 Mon Sep 17 00:00:00 2001 From: jckand-amzn <82226555+jckand-amzn@users.noreply.github.com> Date: Tue, 30 Nov 2021 17:43:53 -0600 Subject: [PATCH 3/9] Xfailing 2 DynVeg tests that fail to create levels in AR (#6051) * Xfailing 2 DynVeg tests that fail to create levels in AR Signed-off-by: jckand-amzn <82226555+jckand-amzn@users.noreply.github.com> * Adding xfail to one more test, and updating reason Signed-off-by: jckand-amzn <82226555+jckand-amzn@users.noreply.github.com> --- .../largeworlds/dyn_veg/TestSuite_Main_Optimized.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/AutomatedTesting/Gem/PythonTests/largeworlds/dyn_veg/TestSuite_Main_Optimized.py b/AutomatedTesting/Gem/PythonTests/largeworlds/dyn_veg/TestSuite_Main_Optimized.py index d83f88cad2..2211c28060 100644 --- a/AutomatedTesting/Gem/PythonTests/largeworlds/dyn_veg/TestSuite_Main_Optimized.py +++ b/AutomatedTesting/Gem/PythonTests/largeworlds/dyn_veg/TestSuite_Main_Optimized.py @@ -148,6 +148,7 @@ class TestAutomation(EditorTestSuite): class test_SlopeFilter_ComponentAndOverrides_InstancesPlantOnValidSlopes(EditorParallelTest): from .EditorScripts import SlopeFilter_ComponentAndOverrides_InstancesPlantOnValidSlope as test_module + @pytest.mark.xfail(reason="Intermittently fails to create level") class test_DynamicSliceInstanceSpawner_Embedded_E2E_Editor(EditorSingleTest): from .EditorScripts import DynamicSliceInstanceSpawner_Embedded_E2E as test_module @@ -156,6 +157,7 @@ class TestAutomation(EditorTestSuite): file_system.delete([os.path.join(workspace.paths.engine_root(), "AutomatedTesting", "Levels", "tmp_level")], True, True) + @pytest.mark.xfail(reason="Intermittently fails to create level") class test_DynamicSliceInstanceSpawner_External_E2E_Editor(EditorSingleTest): from .EditorScripts import DynamicSliceInstanceSpawner_External_E2E as test_module @@ -163,7 +165,8 @@ class TestAutomation(EditorTestSuite): def teardown(self, request, workspace, editor, editor_test_results, launcher_platform): file_system.delete([os.path.join(workspace.paths.engine_root(), "AutomatedTesting", "Levels", "tmp_level")], True, True) - + + @pytest.mark.xfail(reason="Intermittently fails to create level") class test_LayerBlender_E2E_Editor(EditorSingleTest): from .EditorScripts import LayerBlender_E2E_Editor as test_module From 58be7c27edec0319cc76b22f5389e1f77cda398b Mon Sep 17 00:00:00 2001 From: Shirang Jia Date: Tue, 30 Nov 2021 16:46:03 -0800 Subject: [PATCH 4/9] Make scrubber/validator not depend on legacy packaging scripts (#6053) * Make validator not depend on legacy packaging scripts Signed-off-by: Shirang Jia * Remove unused glob_to_regex.py Signed-off-by: Shirang Jia * Remove unsued import path Signed-off-by: Shirang Jia --- scripts/build/package/glob_to_regex.py | 130 ------------------------- scripts/scrubbing/scrubbing_job.py | 29 ------ scripts/scrubbing/validator.py | 37 +++---- 3 files changed, 12 insertions(+), 184 deletions(-) delete mode 100755 scripts/build/package/glob_to_regex.py delete mode 100755 scripts/scrubbing/scrubbing_job.py diff --git a/scripts/build/package/glob_to_regex.py b/scripts/build/package/glob_to_regex.py deleted file mode 100755 index 53ad3d850d..0000000000 --- a/scripts/build/package/glob_to_regex.py +++ /dev/null @@ -1,130 +0,0 @@ -# -# 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 -# -# -from __future__ import absolute_import -import os -import re -import json -import sys -try: - import six -except ImportError: - import pip - pip.main(['install', 'six', '--ignore-installed', '-q']) - import six -from pathlib import Path - -this_file_path = os.path.dirname(os.path.realpath(__file__)) - -# resolve symlinks and eliminate ".." components -engine_root_path = Path(__file__).resolve().parents[3] - -def convert_glob_pattern_to_regex_pattern(glob_pattern): - # switch to forward slashes because way easier to pattern match against - pattern = re.sub(r'\\', r'/', glob_pattern) - - # Replace the dots and question marks - pattern = re.sub(r'\.', r'\\.', pattern) - pattern = re.sub(r'\?', r'.', pattern) - - # Handle the * vs ** expansions - pattern = re.sub(r'([^*])\*($|[^*])', r'\1[^/\\\\]*\2', pattern) - pattern = re.sub(r'\*\*/', r'(.*/)?', pattern) - pattern = re.sub(r'\*\*', r'.*', pattern) - - # replace the forward slashes with [/\\] so it works on PC/unix - pattern = re.sub(r'([^^])/', r'\1[/\\\\]', pattern) - return pattern - -# Convert the package json into a pair of regexes we can use to look for includes and excludes -def convert_glob_list_to_regex_list(filelist, prefix): - includes = [] - excludes = [] - for key, value in six.iteritems(filelist): - glob_pattern = os.path.join(prefix, key) - if isinstance(value, dict): - (sub_includes, sub_excludes) = convert_glob_list_to_regex_list(value, glob_pattern) - includes.extend(sub_includes) - excludes.extend(sub_excludes) - else: - # Simulate what glob would do with file walking to scope the * within a directory - # and ** across directories - regex_pattern = convert_glob_pattern_to_regex_pattern(os.path.normpath(glob_pattern)) - - # Deal with the commands. include/exclude are straight forward. Moves/renames are to be considered - # includes, and we will stick with validating the original contents for now - if value == "#include": - includes.append(regex_pattern) - elif value == "#exclude": - excludes.append(regex_pattern) - elif value.startswith('#move:'): - includes.append(regex_pattern) - elif value.startswith('#rename:'): - includes.append(regex_pattern) - else: - pass - return (includes, excludes) - -def generate_excludes_for_platform(root, platform): - if platform == 'all': - platform_exclusions_filename = os.path.join(this_file_path, 'platform_exclusions.json') - with open(platform_exclusions_filename, 'r') as platform_exclusions_file: - platform_exclusions = json.load(platform_exclusions_file) - else: - # Use real path in case root is a symlink path - if os.name == 'posix' and os.path.islink(root): - root = os.readlink(root) - # "root" is the root of the folder structure we're validating - # "engine_root_path" is the engine root where the restricted platform folder is linked - relative_folder = os.path.relpath(this_file_path, engine_root_path) - platform_exclusions_filename = os.path.join(engine_root_path, 'restricted', platform, relative_folder, platform.lower() + '_exclusions.json') - with open(platform_exclusions_filename, 'r') as platform_exclusions_file: - platform_exclusions = json.load(platform_exclusions_file) - - if platform not in platform_exclusions: - raise KeyError('No {} found in {}'.format(platform, platform_exclusions_filename)) - if '@lyengine' not in platform_exclusions[platform]: - raise KeyError('No {}/@lyengine found in {}'.format(platform, package_file_list)) - (_, excludes) = convert_glob_list_to_regex_list(platform_exclusions[platform]['@lyengine'], root) - del _ - return excludes - -def generate_include_exclude_regexes(package_platform, package_type, root, prohibited_platforms): - # The general contents will be indicated by the package file - if package_type == 'all': - package_file_list = os.path.join(this_file_path, 'package_filelists', 'all.json') - else: - # Search non-restricted platform first - package_file_list = os.path.join(this_file_path, 'Platform', package_platform, 'package_filelists', f'{package_type}.json') - if not os.path.exists(filelist): - # Use real path in case root is a symlink path - if os.name == 'posix' and os.path.islink(root): - root = os.readlink(root) - # "root" is the root of the folder structure we're validating - # "engine_root_path" is the engine root where the restricted platform folder is linked - rel_path = os.path.relpath(this_file_path, engine_root_path) - package_file_list = os.path.join(engine_root_path, 'restricted', package_platform, rel_path, 'package_filelists', - f'{package_type}.json') - with open(package_file_list, 'r') as package_file: - package = json.load(package_file) - - if '@lyengine' not in package: - raise KeyError('No @lyengine found in {}'.format(package_file_list)) - - (includes_list, excludes_list) = convert_glob_list_to_regex_list(package['@lyengine'], root) - prohibited_platforms.append('all') - - # Add the exclusions of each prohibited platform - for p in prohibited_platforms: - excludes_list.extend(generate_excludes_for_platform(root, p)) - - includes = re.compile('|'.join(includes_list), re.IGNORECASE) - excludes = re.compile('|'.join(excludes_list), re.IGNORECASE) - return (includes, excludes) - -def generate_exclude_regexes_for_platform(root, platform): - return re.compile('|'.join(generate_excludes_for_platform(root, platform)), re.IGNORECASE) diff --git a/scripts/scrubbing/scrubbing_job.py b/scripts/scrubbing/scrubbing_job.py deleted file mode 100755 index 11dbf5ea5a..0000000000 --- a/scripts/scrubbing/scrubbing_job.py +++ /dev/null @@ -1,29 +0,0 @@ -# -# 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 -# -# - -import os -import sys -cur_dir = cur_dir = os.path.dirname(os.path.abspath(__file__)) -sys.path.insert(0, os.path.abspath(f'{cur_dir}/../build/package')) -import util - -# Run validator -success = True -validator_path = os.path.join(os.path.dirname(os.path.realpath(__file__)), 'validator.py') -engine_root = os.path.dirname(os.path.dirname(os.path.dirname(os.path.realpath(__file__)))) -if sys.platform == 'win32': - python = os.path.join(engine_root, 'python', 'python.cmd') -else: - python = os.path.join(engine_root, 'python', 'python.sh') -args = [python, validator_path, '--package_platform', 'Windows', '--package_type', 'all', engine_root] -return_code = util.safe_execute_system_call(args) -if return_code != 0: - success = False -if not success: - util.error('Restricted file validator failed.') -print('Restricted file validator completed successfully.') diff --git a/scripts/scrubbing/validator.py b/scripts/scrubbing/validator.py index 39f1a40a18..3b53417755 100755 --- a/scripts/scrubbing/validator.py +++ b/scripts/scrubbing/validator.py @@ -29,8 +29,6 @@ else: from io import StringIO import validator_data_LEGAL_REVIEW_REQUIRED # pull in the data we need to configure this tool -sys.path.append(os.path.join(os.path.dirname(os.path.realpath(__file__)), '..', 'build', 'package')) -from glob_to_regex import generate_include_exclude_regexes class Validator(object): """Class to contain the validator program""" @@ -212,9 +210,6 @@ class Validator(object): # TODO: Perhaps the directories to skip should become a parameter so we can use the validator # on non-Lumberyard trees. def validate_directory_tree(self, root, platform): - prohibited_platforms = validator_data_LEGAL_REVIEW_REQUIRED.get_prohibited_platforms_for_package(self.options.package_platform) - (includes, excludes) = generate_include_exclude_regexes(self.options.package_platform, self.options.package_type, root, prohibited_platforms) - """Walk from root to find all files to validate and call the validator on each file. Return 0 if no problems where found, and 1 if any validation failures occured.""" counter = 0 @@ -227,28 +222,22 @@ class Validator(object): # First deal with the files in the current directory for filename in filenames: filepath = os.path.join(dirname, filename) - include_match = includes.match(filepath) - exclude_match = excludes.match(filepath) - allowed = include_match and not exclude_match - - if self.options.all or allowed: - scanned += 1 - file_failed = self.validate_file(os.path.normpath(filepath)) - if file_failed: - platform_failed = file_failed - else: - validations += 1 - counter += 1 + scanned += 1 + file_failed = self.validate_file(os.path.normpath(filepath)) + if file_failed: + platform_failed = file_failed + else: + validations += 1 # Trim out allowlisted subdirectories in the current directory if allowed for name in bypassed_directories: if name in dirnames: dirnames.remove(name) - if counter == 0 or scanned == 0: + if scanned == 0: logging.error('No files scanned at target search directory: %s', root) platform_failed = 1 else: - print('validated {} of {} package files ({} non-package files skipped)'.format(validations, scanned, counter - scanned)) + print('validated {} of {} files'.format(validations, scanned)) return platform_failed @@ -387,8 +376,6 @@ def parse_options(): choices=platform_choices, dest='package_platform', help='Package platform to validate. Must be one of {}.'.format(platform_choices)) - parser.add_option('--package_type', action='store', type='string', default='all', dest='package_type', - help='Package type to validate.') parser.add_option('-s', '--store-exceptions', action='store', type='string', default='', dest='exception_file', help='Store list of lines that the validator gave exceptions to by matching accepted use patterns. These can be diffed with prior runs to see what is changing.') @@ -430,7 +417,6 @@ def main(): package_failed = 0 package_platform = validator.options.package_platform - package_type = validator.options.package_type prohibited_platforms = validator_data_LEGAL_REVIEW_REQUIRED.get_prohibited_platforms_for_package(package_platform) if validator.options.exception_file != '': @@ -441,19 +427,20 @@ def main(): sys.exit(1) for platform in prohibited_platforms: - print('validating {} against {} for package platform {} package type {}'.format(args[0], platform, package_platform, package_type)) + print('validating {} against {} for package platform {}'.format(args[0], platform, package_platform)) platform_failed = validator.validate(platform) if platform_failed: - print('{} FAILED validation against {} for package platform {} package type {}'.format(args[0], platform, package_platform, package_type)) + print('{} FAILED validation against {} for package platform {}'.format(args[0], platform, package_platform)) package_failed = platform_failed else: - print('{} is VALIDATED against {} for package platform {} package type {}'.format(args[0], platform, package_platform, package_type)) + print('{} is VALIDATED against {} for package platform {}'.format(args[0], platform, package_platform)) if validator.options.exception_file != '': validator.exceptions_output.close() return package_failed + if __name__ == '__main__': # pylint: disable-msg=C0103 main_results = main() From b2c13b24ffea6f5c57ddded4ac8cef05ae2422f3 Mon Sep 17 00:00:00 2001 From: bosnichd Date: Wed, 1 Dec 2021 07:49:22 -0700 Subject: [PATCH 5/9] Fix ImGui Gamepad Input (#6055) This fixes gamepad input for both of our ImGui integrations (which should probably be combined at some point). Signed-off-by: bosnichd --- .../Common/Code/Source/ImGui/ImGuiPass.cpp | 23 +++++++++++-- .../Common/Code/Source/ImGui/ImGuiPass.h | 1 + Gems/ImGui/Code/Source/ImGuiManager.cpp | 34 +++++-------------- Gems/ImGui/Code/Source/ImGuiManager.h | 5 --- 4 files changed, 31 insertions(+), 32 deletions(-) diff --git a/Gems/Atom/Feature/Common/Code/Source/ImGui/ImGuiPass.cpp b/Gems/Atom/Feature/Common/Code/Source/ImGui/ImGuiPass.cpp index 0ff8d2c165..7197b95917 100644 --- a/Gems/Atom/Feature/Common/Code/Source/ImGui/ImGuiPass.cpp +++ b/Gems/Atom/Feature/Common/Code/Source/ImGui/ImGuiPass.cpp @@ -140,9 +140,18 @@ namespace AZ m_drawData.push_back(drawData); } + int ImGuiPass::GetTickOrder() + { + // We have to call ImGui::NewFrame (which happens in ImGuiPass::OnTick) after setting + // ImGui::GetIO().NavInputs (which happens in ImGuiPass::OnInputChannelEventFiltered), + // but before ImGui::Render (which happens in ImGuiPass::SetupFrameGraphDependencies). + return AZ::ComponentTickBus::TICK_PRE_RENDER; + } + void ImGuiPass::OnTick(float deltaTime, [[maybe_unused]] AZ::ScriptTimePoint timePoint) { auto imguiContextScope = ImguiContextScope(m_imguiContext); + ImGui::NewFrame(); auto& io = ImGui::GetIO(); io.DeltaTime = deltaTime; @@ -413,6 +422,7 @@ namespace AZ void ImGuiPass::Init() { + auto imguiContextScope = ImguiContextScope(m_imguiContext); auto& io = ImGui::GetIO(); // ImGui IO Setup @@ -421,7 +431,6 @@ namespace AZ { io.KeyMap[static_cast(i)] = static_cast(i); } - io.NavActive = true; // Touch input const AzFramework::InputDevice* inputDevice = nullptr; @@ -434,6 +443,17 @@ namespace AZ io.ConfigFlags |= ImGuiConfigFlags_IsTouchScreen; } + // Gamepad input + inputDevice = nullptr; + AzFramework::InputDeviceRequestBus::EventResult(inputDevice, + AzFramework::InputDeviceGamepad::IdForIndex0, + &AzFramework::InputDeviceRequests::GetInputDevice); + if (inputDevice && inputDevice->IsSupported()) + { + io.BackendFlags |= ImGuiBackendFlags_HasGamepad; + io.ConfigFlags |= ImGuiConfigFlags_NavEnableGamepad; + } + // Set initial display size to something reasonable (this will be updated in FramePrepare) io.DisplaySize.x = 1920; io.DisplaySize.y = 1080; @@ -571,7 +591,6 @@ namespace AZ auto imguiContextScope = ImguiContextScope(m_imguiContext); ImGui::GetIO().MouseWheel = m_lastFrameMouseWheel; m_lastFrameMouseWheel = 0.0; - ImGui::NewFrame(); } void ImGuiPass::BuildCommandListInternal(const RHI::FrameGraphExecuteContext& context) diff --git a/Gems/Atom/Feature/Common/Code/Source/ImGui/ImGuiPass.h b/Gems/Atom/Feature/Common/Code/Source/ImGui/ImGuiPass.h index 018d2d46b7..6c9dd11914 100644 --- a/Gems/Atom/Feature/Common/Code/Source/ImGui/ImGuiPass.h +++ b/Gems/Atom/Feature/Common/Code/Source/ImGui/ImGuiPass.h @@ -77,6 +77,7 @@ namespace AZ void RenderImguiDrawData(const ImDrawData& drawData); // TickBus::Handler overrides... + int GetTickOrder() override; void OnTick(float deltaTime, AZ::ScriptTimePoint timePoint) override; // AzFramework::InputTextEventListener overrides... diff --git a/Gems/ImGui/Code/Source/ImGuiManager.cpp b/Gems/ImGui/Code/Source/ImGuiManager.cpp index b30eda8825..f90aa60a88 100644 --- a/Gems/ImGui/Code/Source/ImGuiManager.cpp +++ b/Gems/ImGui/Code/Source/ImGuiManager.cpp @@ -172,7 +172,6 @@ void ImGuiManager::Initialize() // Broadcast ImGui Ready to Listeners ImGuiUpdateListenerBus::Broadcast(&IImGuiUpdateListener::OnImGuiInitialize); - m_currentControllerIndex = -1; m_button1Pressed = m_button2Pressed = false; m_menuBarStatusChanged = false; @@ -227,6 +226,7 @@ void ImGui::ImGuiManager::RestoreRenderWindowSizeToDefault() void ImGui::ImGuiManager::SetDpiScalingFactor(float dpiScalingFactor) { + ImGui::ImGuiContextScope contextScope(m_imguiContext); ImGuiIO& io = ImGui::GetIO(); // Set the global font scale to size our UI to the scaling factor // Note: Currently we use the default, 13px fixed-size IMGUI font, so this can get somewhat blurry @@ -235,6 +235,7 @@ void ImGui::ImGuiManager::SetDpiScalingFactor(float dpiScalingFactor) float ImGui::ImGuiManager::GetDpiScalingFactor() const { + ImGui::ImGuiContextScope contextScope(m_imguiContext); ImGuiIO& io = ImGui::GetIO(); return io.FontGlobalScale; } @@ -406,7 +407,7 @@ bool ImGuiManager::OnInputChannelEventFiltered(const InputChannel& inputChannel) // Cycle through ImGui Menu Bar States on Home button press if (inputChannelId == InputDeviceKeyboard::Key::NavigationHome) { - ToggleThroughImGuiVisibleState(-1); + ToggleThroughImGuiVisibleState(); } // Cycle through Standalone Editor Window States @@ -453,19 +454,10 @@ bool ImGuiManager::OnInputChannelEventFiltered(const InputChannel& inputChannel) } // Handle Controller Inputs - int inputControllerIndex = -1; - bool controllerInput = false; if (InputDeviceGamepad::IsGamepadDevice(inputDeviceId)) { - inputControllerIndex = inputDeviceId.GetIndex(); - controllerInput = true; - } - - - if (controllerInput) - { - // Only pipe in Controller Nav Inputs if we are the current Controller Index and at least 1 of the two controller modes are enabled. - if (m_currentControllerIndex == inputControllerIndex && m_controllerModeFlags) + // Only pipe in Controller Nav Inputs when at least 1 of the two controller modes are enabled. + if (m_controllerModeFlags) { const auto lyButtonToImGuiNav = s_lyInputToImGuiNavIndexMap.find(inputChannelId); if (lyButtonToImGuiNav != s_lyInputToImGuiNavIndexMap.end()) @@ -476,7 +468,7 @@ bool ImGuiManager::OnInputChannelEventFiltered(const InputChannel& inputChannel) } //Switch menu bar display only if two buttons are pressed at the same time - if (inputChannelId == InputDeviceGamepad::Button::L3) + if (inputChannelId == InputDeviceGamepad::Button::L1) { if (inputChannel.IsStateBegan()) { @@ -488,7 +480,7 @@ bool ImGuiManager::OnInputChannelEventFiltered(const InputChannel& inputChannel) m_menuBarStatusChanged = false; } } - if (inputChannelId == InputDeviceGamepad::Button::R3) + if (inputChannelId == InputDeviceGamepad::Button::R1) { if (inputChannel.IsStateBegan()) { @@ -502,7 +494,7 @@ bool ImGuiManager::OnInputChannelEventFiltered(const InputChannel& inputChannel) } if (!m_menuBarStatusChanged && m_button1Pressed && m_button2Pressed) { - ToggleThroughImGuiVisibleState(inputControllerIndex); + ToggleThroughImGuiVisibleState(); } // If we have the Discrete Input Mode Enabled.. and we are in the Visible State, then consume input here @@ -627,14 +619,13 @@ bool ImGuiManager::OnInputTextEventFiltered(const AZStd::string& textUTF8) return io.WantTextInput && m_clientMenuBarState == DisplayState::Visible;; } -void ImGuiManager::ToggleThroughImGuiVisibleState(int controllerIndex) +void ImGuiManager::ToggleThroughImGuiVisibleState() { ImGui::ImGuiContextScope contextScope(m_imguiContext); switch (m_clientMenuBarState) { case DisplayState::Hidden: - m_currentControllerIndex = controllerIndex; m_clientMenuBarState = DisplayState::Visible; // Draw the ImGui Mouse cursor if either the hardware mouse is connected, or the controller mouse is enabled. @@ -669,7 +660,6 @@ void ImGuiManager::ToggleThroughImGuiVisibleState(int controllerIndex) default: m_clientMenuBarState = DisplayState::Hidden; - m_currentControllerIndex = -1; // Enable system cursor if it's in editor and it's not editor game mode if (gEnv->IsEditor() && !gEnv->IsEditorGameMode()) @@ -686,12 +676,6 @@ void ImGuiManager::ToggleThroughImGuiVisibleState(int controllerIndex) m_setEnabledEvent.Signal(m_clientMenuBarState == DisplayState::Hidden); } -void ImGuiManager::ToggleThroughImGuiVisibleState() -{ - ToggleThroughImGuiVisibleState(-1); -} - - void ImGuiManager::RenderImGuiBuffers(const ImVec2& scaleRects) { ImGui::ImGuiContextScope contextScope(m_imguiContext); diff --git a/Gems/ImGui/Code/Source/ImGuiManager.h b/Gems/ImGui/Code/Source/ImGuiManager.h index c4fa5169f7..c0071c94c5 100644 --- a/Gems/ImGui/Code/Source/ImGuiManager.h +++ b/Gems/ImGui/Code/Source/ImGuiManager.h @@ -76,9 +76,6 @@ namespace ImGui // Sets up initial window size and listens for changes void InitWindowSize(); - // A function to toggle through the available ImGui Visibility States - void ToggleThroughImGuiVisibleState(int controllerIndex); - private: ImGuiContext* m_imguiContext = nullptr; DisplayState m_clientMenuBarState = DisplayState::Hidden; @@ -96,8 +93,6 @@ namespace ImGui std::vector m_idxBuffer; //Controller navigation - static const int MaxControllerNumber = 4; - int m_currentControllerIndex; bool m_button1Pressed, m_button2Pressed, m_menuBarStatusChanged; bool m_hardwardeMouseConnected = false; From 2065225099e73a535c8e3c1bb756a7acabf4405e Mon Sep 17 00:00:00 2001 From: amzn-mike <80125227+amzn-mike@users.noreply.github.com> Date: Wed, 1 Dec 2021 09:04:49 -0600 Subject: [PATCH 6/9] Fixed LodRuleBehavior using wrong loop index (#5915) * Fixed LodRuleBehavior using wrong loop index Signed-off-by: amzn-mike <80125227+amzn-mike@users.noreply.github.com> * Add unit test for LOD auto-add crash Signed-off-by: amzn-mike <80125227+amzn-mike@users.noreply.github.com> * Fix macro usage Signed-off-by: amzn-mike <80125227+amzn-mike@users.noreply.github.com> * Fix include Signed-off-by: amzn-mike <80125227+amzn-mike@users.noreply.github.com> --- .../SceneData/Behaviors/LodRuleBehavior.cpp | 2 +- .../SceneData/Behaviors/LodRuleBehavior.h | 18 +++-- .../SceneAPI/SceneData/Rules/LodRule.cpp | 1 - Code/Tools/SceneAPI/SceneData/Rules/LodRule.h | 20 ++--- .../SceneData/SceneData_testing_files.cmake | 1 + .../SceneData/Tests/GraphData/RulesTests.cpp | 78 +++++++++++++++++++ 6 files changed, 100 insertions(+), 20 deletions(-) create mode 100644 Code/Tools/SceneAPI/SceneData/Tests/GraphData/RulesTests.cpp diff --git a/Code/Tools/SceneAPI/SceneData/Behaviors/LodRuleBehavior.cpp b/Code/Tools/SceneAPI/SceneData/Behaviors/LodRuleBehavior.cpp index f51c670fc8..c47330620d 100644 --- a/Code/Tools/SceneAPI/SceneData/Behaviors/LodRuleBehavior.cpp +++ b/Code/Tools/SceneAPI/SceneData/Behaviors/LodRuleBehavior.cpp @@ -185,7 +185,7 @@ namespace AZ if (lodCount > 0) { rule->AddLod(); - selection.CopyTo(rule->GetNodeSelectionList(index)); + selection.CopyTo(rule->GetNodeSelectionList(lodLevel)); } else { diff --git a/Code/Tools/SceneAPI/SceneData/Behaviors/LodRuleBehavior.h b/Code/Tools/SceneAPI/SceneData/Behaviors/LodRuleBehavior.h index d152386940..c9416c0aa8 100644 --- a/Code/Tools/SceneAPI/SceneData/Behaviors/LodRuleBehavior.h +++ b/Code/Tools/SceneAPI/SceneData/Behaviors/LodRuleBehavior.h @@ -13,6 +13,7 @@ #include #include #include +#include namespace AZ { @@ -27,7 +28,7 @@ namespace AZ { class LodRule; - class LodRuleBehavior + class SCENE_DATA_CLASS LodRuleBehavior : public SceneCore::BehaviorComponent , public Events::ManifestMetaInfoBus::Handler , public Events::AssetImportRequestBus::Handler @@ -36,18 +37,19 @@ namespace AZ public: AZ_COMPONENT(LodRuleBehavior, "{D2E19864-9A4B-41FD-8ACC-DA6756728CB3}", SceneCore::BehaviorComponent); - ~LodRuleBehavior() override = default; + SCENE_DATA_API ~LodRuleBehavior() 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); - void InitializeObject(const Containers::Scene& scene, DataTypes::IManifestObject& target) override; - Events::ProcessingResult UpdateManifest(Containers::Scene& scene, ManifestAction action, + SCENE_DATA_API void InitializeObject(const Containers::Scene& scene, DataTypes::IManifestObject& target) override; + SCENE_DATA_API Events::ProcessingResult UpdateManifest( + Containers::Scene& scene, ManifestAction action, RequestingApplication requester) override; - void GetVirtualTypeName(AZStd::string& name, Crc32 type) override; - void GetAllVirtualTypes(AZStd::set& types) override; + SCENE_DATA_API void GetVirtualTypeName(AZStd::string& name, Crc32 type) override; + SCENE_DATA_API void GetAllVirtualTypes(AZStd::set& types) override; private: size_t SelectLodMeshes(const Containers::Scene& scene, DataTypes::ISceneNodeSelectionList& selection, size_t lodLevel) const; diff --git a/Code/Tools/SceneAPI/SceneData/Rules/LodRule.cpp b/Code/Tools/SceneAPI/SceneData/Rules/LodRule.cpp index f893751caf..a6c624397a 100644 --- a/Code/Tools/SceneAPI/SceneData/Rules/LodRule.cpp +++ b/Code/Tools/SceneAPI/SceneData/Rules/LodRule.cpp @@ -21,7 +21,6 @@ namespace AZ { const size_t LodRule::m_maxLods; - AZ_CLASS_ALLOCATOR_IMPL(LodRule, SystemAllocator, 0) SceneNodeSelectionList& LodRule::GetNodeSelectionList(size_t index) { diff --git a/Code/Tools/SceneAPI/SceneData/Rules/LodRule.h b/Code/Tools/SceneAPI/SceneData/Rules/LodRule.h index 0d9bf0a9a6..fd7d6bacd8 100644 --- a/Code/Tools/SceneAPI/SceneData/Rules/LodRule.h +++ b/Code/Tools/SceneAPI/SceneData/Rules/LodRule.h @@ -25,26 +25,26 @@ namespace AZ } namespace SceneData { - class LodRule + class SCENE_DATA_CLASS LodRule : public DataTypes::ILodRule { public: AZ_RTTI(LodRule, "{6E796AC8-1484-4909-860A-6D3F22A7346F}", DataTypes::ILodRule); - AZ_CLASS_ALLOCATOR_DECL + AZ_CLASS_ALLOCATOR(LodRule, AZ::SystemAllocator, 0) - ~LodRule() override = default; + SCENE_DATA_API ~LodRule() override = default; - SceneNodeSelectionList& GetNodeSelectionList(size_t index); + SCENE_DATA_API SceneNodeSelectionList& GetNodeSelectionList(size_t index); - DataTypes::ISceneNodeSelectionList& GetSceneNodeSelectionList(size_t index) override; - const DataTypes::ISceneNodeSelectionList& GetSceneNodeSelectionList(size_t index) const override; - size_t GetLodCount() const override; + SCENE_DATA_API DataTypes::ISceneNodeSelectionList& GetSceneNodeSelectionList(size_t index) override; + SCENE_DATA_API const DataTypes::ISceneNodeSelectionList& GetSceneNodeSelectionList(size_t index) const override; + SCENE_DATA_API size_t GetLodCount() const override; - void AddLod(); + SCENE_DATA_API void AddLod(); static void Reflect(ReflectContext* context); - //The engine supports 6 total lods. 1 for the base model then 5 more lods. - //The rule only captures lods past level 0 so this is set to 5. + //The engine supports 6 total lods. 1 for the base model then 5 more lods. + //The rule only captures lods past level 0 so this is set to 5. static const size_t m_maxLods = 5; protected: diff --git a/Code/Tools/SceneAPI/SceneData/SceneData_testing_files.cmake b/Code/Tools/SceneAPI/SceneData/SceneData_testing_files.cmake index 51f3dfc9e7..3a51180ca1 100644 --- a/Code/Tools/SceneAPI/SceneData/SceneData_testing_files.cmake +++ b/Code/Tools/SceneAPI/SceneData/SceneData_testing_files.cmake @@ -11,5 +11,6 @@ set(FILES Tests/GraphData/MeshDataTests.cpp Tests/GraphData/MeshDataPrimitiveUtilsTests.cpp Tests/GraphData/GraphDataBehaviorTests.cpp + Tests/GraphData/RulesTests.cpp Tests/SceneManifest/SceneManifestRuleTests.cpp ) diff --git a/Code/Tools/SceneAPI/SceneData/Tests/GraphData/RulesTests.cpp b/Code/Tools/SceneAPI/SceneData/Tests/GraphData/RulesTests.cpp new file mode 100644 index 0000000000..a6ccdfa59d --- /dev/null +++ b/Code/Tools/SceneAPI/SceneData/Tests/GraphData/RulesTests.cpp @@ -0,0 +1,78 @@ +/* + * 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 +#include +#include +#include +#include +#include +#include +#include +#include + +namespace AZ +{ + namespace SceneData + { + struct SoftNameMock + : SceneAPI::Events::GraphMetaInfoBus::Handler + { + SoftNameMock() + { + BusConnect(); + } + + ~SoftNameMock() override + { + BusDisconnect(); + } + + void GetVirtualTypes(AZStd::set& types, const SceneAPI::Containers::Scene&, SceneAPI::Containers::SceneGraph::NodeIndex) override + { + // Indicate this node is a LOD1 type + types.emplace(AZ_CRC_CE("LODMesh1")); + } + }; + + TEST(LOD, LODRuleTest) + { + // Test that UpdateManifest doesn't crash when trying to auto-add new LOD levels + SoftNameMock softNameMock; + + SceneAPI::SceneData::LodRuleBehavior lod; + SceneAPI::Containers::Scene scene("test"); + + auto lodRule = AZStd::shared_ptr(aznew SceneAPI::SceneData::LodRule()); + scene.GetManifest().AddEntry(lodRule); + + auto group = AZStd::shared_ptr(aznew SceneAPI::SceneData::MeshGroup()); + + // Add a bunch of other rules first + // This is necessary to replicate the bug condition where the index of the rule is used instead of the index of the LOD + for (int i = 0; i < 5; ++i) + { + auto tangentsRule = AZStd::shared_ptr(aznew SceneAPI::SceneData::TangentsRule()); + group->GetRuleContainer().AddRule(tangentsRule); + } + + group->GetRuleContainer().AddRule(lodRule); + scene.GetManifest().AddEntry(group); + + auto meshData = AZStd::shared_ptr(new GraphData::MeshData()); + scene.GetGraph().AddChild(scene.GetGraph().GetRoot(), "test", meshData); + + EXPECT_EQ(lodRule->GetLodCount(), 0); + + // This should auto-add 1 LOD because of the "test" node we added above along with the SoftNameMock which will report it as an LOD1 + lod.UpdateManifest(scene, SceneAPI::Events::AssetImportRequest::Update, SceneAPI::Events::AssetImportRequest::Generic); + + EXPECT_EQ(lodRule->GetLodCount(), 1); + } + } +} From 828431f185a39846bdad5630dcdf7c7d117ed931 Mon Sep 17 00:00:00 2001 From: amzn-mike <80125227+amzn-mike@users.noreply.github.com> Date: Wed, 1 Dec 2021 09:05:35 -0600 Subject: [PATCH 7/9] Update AssetManager unit tests to not interact with the disk (#5815) * Changed AssetManager tests to use memory streams for asset reading/writing Signed-off-by: amzn-mike <80125227+amzn-mike@users.noreply.github.com> * Fix compilation on non-unity builds Signed-off-by: amzn-mike <80125227+amzn-mike@users.noreply.github.com> * Fixed handling of path lookups when test folder path is non-empty Signed-off-by: amzn-mike <80125227+amzn-mike@users.noreply.github.com> * Add more detailed error message for "asset is not loaded" Signed-off-by: amzn-mike <80125227+amzn-mike@users.noreply.github.com> * Make numThreads a constexpr Signed-off-by: amzn-mike <80125227+amzn-mike@users.noreply.github.com> * Add FindFile function Signed-off-by: amzn-mike <80125227+amzn-mike@users.noreply.github.com> * Remove unused lambda capture Signed-off-by: amzn-mike <80125227+amzn-mike@users.noreply.github.com> * Remove trailing whitespace Signed-off-by: amzn-mike <80125227+amzn-mike@users.noreply.github.com> * Add size to assert Signed-off-by: amzn-mike <80125227+amzn-mike@users.noreply.github.com> --- .../AzCore/AzCore/Asset/AssetCommon.h | 4 +- .../Tests/Asset/AssetManagerLoadingTests.cpp | 176 +++++------- .../Tests/Asset/BaseAssetManagerTest.cpp | 250 ++++++++++++++++++ .../AzCore/Tests/Asset/BaseAssetManagerTest.h | 66 ++++- Code/Framework/AzCore/Tests/TestCatalog.cpp | 8 +- 5 files changed, 393 insertions(+), 111 deletions(-) diff --git a/Code/Framework/AzCore/AzCore/Asset/AssetCommon.h b/Code/Framework/AzCore/AzCore/Asset/AssetCommon.h index c45bb21c6d..0b181f6782 100644 --- a/Code/Framework/AzCore/AzCore/Asset/AssetCommon.h +++ b/Code/Framework/AzCore/AzCore/Asset/AssetCommon.h @@ -325,13 +325,13 @@ namespace AZ T& operator*() const { - AZ_Assert(m_assetData, "Asset is not loaded"); + AZ_Assert(m_assetData, "Asset %s (%s) is not loaded", m_assetId.ToString().c_str(), m_assetHint.c_str()); return *Get(); } T* operator->() const { - AZ_Assert(m_assetData, "Asset is not loaded"); + AZ_Assert(m_assetData, "Asset %s (%s) is not loaded", m_assetId.ToString().c_str(), m_assetHint.c_str()); return Get(); } diff --git a/Code/Framework/AzCore/Tests/Asset/AssetManagerLoadingTests.cpp b/Code/Framework/AzCore/Tests/Asset/AssetManagerLoadingTests.cpp index fe38451101..043eb1aee6 100644 --- a/Code/Framework/AzCore/Tests/Asset/AssetManagerLoadingTests.cpp +++ b/Code/Framework/AzCore/Tests/Asset/AssetManagerLoadingTests.cpp @@ -26,6 +26,7 @@ #include #include #include +#include #include #include #include @@ -131,8 +132,8 @@ namespace UnitTest * This will test the aspect of the system where ObjectStreams and asset jobs loading dependent * assets will do the work in their own thread. */ - class AssetJobsFloodTest - : public BaseAssetManagerTest + + class AssetJobsFloodTest : public DisklessAssetManagerBase { public: TestAssetManager* m_testAssetManager{ nullptr }; @@ -183,15 +184,14 @@ namespace UnitTest void SetUp() override { - BaseAssetManagerTest::SetUp(); + DisklessAssetManagerBase::SetUp(); SetupTest(); } void TearDown() override { - TearDownTest(); AssetManager::Destroy(); - BaseAssetManagerTest::TearDown(); + DisklessAssetManagerBase::TearDown(); } void SetupAssets() @@ -257,9 +257,9 @@ namespace UnitTest AssetWithSerializedData ap2; AssetWithSerializedData ap3; - EXPECT_TRUE(AZ::Utils::SaveObjectToFile(GetTestFolderPath() + "TestAsset4.txt", AZ::DataStream::ST_XML, &ap1, m_serializeContext)); - EXPECT_TRUE(AZ::Utils::SaveObjectToFile(GetTestFolderPath() + "TestAsset5.txt", AZ::DataStream::ST_XML, &ap2, m_serializeContext)); - EXPECT_TRUE(AZ::Utils::SaveObjectToFile(GetTestFolderPath() + "TestAsset6.txt", AZ::DataStream::ST_XML, &ap3, m_serializeContext)); + EXPECT_TRUE(m_streamerWrapper->WriteMemoryFile("TestAsset4.txt", &ap1, m_serializeContext)); + EXPECT_TRUE(m_streamerWrapper->WriteMemoryFile("TestAsset5.txt", &ap2, m_serializeContext)); + EXPECT_TRUE(m_streamerWrapper->WriteMemoryFile("TestAsset6.txt", &ap3, m_serializeContext)); AssetWithAssetReference assetWithPreload1; AssetWithAssetReference assetWithPreload2; @@ -273,11 +273,11 @@ namespace UnitTest noLoadAsset.m_asset = m_testAssetManager->CreateAsset(MyAsset2Id, AssetLoadBehavior::NoLoad); EXPECT_EQ(m_assetHandlerAndCatalog->m_numCreations, 4); - EXPECT_TRUE(AZ::Utils::SaveObjectToFile(GetTestFolderPath() + "TestAsset1.txt", AZ::DataStream::ST_XML, &assetWithPreload1, m_serializeContext)); - EXPECT_TRUE(AZ::Utils::SaveObjectToFile(GetTestFolderPath() + "TestAsset2.txt", AZ::DataStream::ST_XML, &assetWithPreload2, m_serializeContext)); - EXPECT_TRUE(AZ::Utils::SaveObjectToFile(GetTestFolderPath() + "TestAsset3.txt", AZ::DataStream::ST_XML, &assetWithPreload3, m_serializeContext)); - EXPECT_TRUE(AZ::Utils::SaveObjectToFile(GetTestFolderPath() + "DelayLoadAsset.txt", AZ::DataStream::ST_XML, &delayedAsset, m_serializeContext)); - EXPECT_TRUE(AZ::Utils::SaveObjectToFile(GetTestFolderPath() + "NoLoadAsset.txt", AZ::DataStream::ST_XML, &noLoadAsset, m_serializeContext)); + EXPECT_TRUE(m_streamerWrapper->WriteMemoryFile("TestAsset1.txt", &assetWithPreload1, m_serializeContext)); + EXPECT_TRUE(m_streamerWrapper->WriteMemoryFile("TestAsset2.txt", &assetWithPreload2, m_serializeContext)); + EXPECT_TRUE(m_streamerWrapper->WriteMemoryFile("TestAsset3.txt", &assetWithPreload3, m_serializeContext)); + EXPECT_TRUE(m_streamerWrapper->WriteMemoryFile("DelayLoadAsset.txt", &delayedAsset, m_serializeContext)); + EXPECT_TRUE(m_streamerWrapper->WriteMemoryFile("NoLoadAsset.txt", &noLoadAsset, m_serializeContext)); AssetWithQueueAndPreLoadReferences preLoadRoot; AssetWithQueueAndPreLoadReferences preLoadA; @@ -297,16 +297,16 @@ namespace UnitTest preLoadBrokenA.m_preLoad = m_testAssetManager->CreateAsset(PreloadBrokenDepBId, AssetLoadBehavior::PreLoad); preLoadBrokenB.m_preLoad = m_testAssetManager->CreateAsset(PreloadAssetNoDataId, AssetLoadBehavior::PreLoad); - EXPECT_TRUE(AZ::Utils::SaveObjectToFile(GetTestFolderPath() + "PreLoadRoot.txt", AZ::DataStream::ST_XML, &preLoadRoot, m_serializeContext)); - EXPECT_TRUE(AZ::Utils::SaveObjectToFile(GetTestFolderPath() + "PreLoadA.txt", AZ::DataStream::ST_XML, &preLoadA, m_serializeContext)); - EXPECT_TRUE(AZ::Utils::SaveObjectToFile(GetTestFolderPath() + "PreLoadB.txt", AZ::DataStream::ST_XML, &noRefs, m_serializeContext)); - EXPECT_TRUE(AZ::Utils::SaveObjectToFile(GetTestFolderPath() + "PreLoadC.txt", AZ::DataStream::ST_XML, &noRefs, m_serializeContext)); - EXPECT_TRUE(AZ::Utils::SaveObjectToFile(GetTestFolderPath() + "QueueLoadA.txt", AZ::DataStream::ST_XML, &queueLoadA, m_serializeContext)); - EXPECT_TRUE(AZ::Utils::SaveObjectToFile(GetTestFolderPath() + "QueueLoadB.txt", AZ::DataStream::ST_XML, &noRefs, m_serializeContext)); - EXPECT_TRUE(AZ::Utils::SaveObjectToFile(GetTestFolderPath() + "QueueLoadC.txt", AZ::DataStream::ST_XML, &noRefs, m_serializeContext)); - EXPECT_TRUE(AZ::Utils::SaveObjectToFile(GetTestFolderPath() + "PreLoadBrokenA.txt", AZ::DataStream::ST_XML, &preLoadBrokenA, m_serializeContext)); - EXPECT_TRUE(AZ::Utils::SaveObjectToFile(GetTestFolderPath() + "PreLoadBrokenB.txt", AZ::DataStream::ST_XML, &preLoadBrokenB, m_serializeContext)); - EXPECT_TRUE(AZ::Utils::SaveObjectToFile(GetTestFolderPath() + "PreLoadNoData.txt", AZ::DataStream::ST_XML, &noRefs, m_serializeContext)); + EXPECT_TRUE(m_streamerWrapper->WriteMemoryFile("PreLoadRoot.txt", &preLoadRoot, m_serializeContext)); + EXPECT_TRUE(m_streamerWrapper->WriteMemoryFile("PreLoadA.txt", &preLoadA, m_serializeContext)); + EXPECT_TRUE(m_streamerWrapper->WriteMemoryFile("PreLoadB.txt", &noRefs, m_serializeContext)); + EXPECT_TRUE(m_streamerWrapper->WriteMemoryFile("PreLoadC.txt", &noRefs, m_serializeContext)); + EXPECT_TRUE(m_streamerWrapper->WriteMemoryFile("QueueLoadA.txt", &queueLoadA, m_serializeContext)); + EXPECT_TRUE(m_streamerWrapper->WriteMemoryFile("QueueLoadB.txt", &noRefs, m_serializeContext)); + EXPECT_TRUE(m_streamerWrapper->WriteMemoryFile("QueueLoadC.txt", &noRefs, m_serializeContext)); + EXPECT_TRUE(m_streamerWrapper->WriteMemoryFile("PreLoadBrokenA.txt", &preLoadBrokenA, m_serializeContext)); + EXPECT_TRUE(m_streamerWrapper->WriteMemoryFile("PreLoadBrokenB.txt", &preLoadBrokenB, m_serializeContext)); + EXPECT_TRUE(m_streamerWrapper->WriteMemoryFile("PreLoadNoData.txt", &noRefs, m_serializeContext)); AssetWithQueueAndPreLoadReferences circularA; AssetWithQueueAndPreLoadReferences circularB; @@ -318,43 +318,15 @@ namespace UnitTest circularC.m_preLoad = m_testAssetManager->CreateAsset(CircularBId, AssetLoadBehavior::PreLoad); circularD.m_preLoad = circularC.m_preLoad; - EXPECT_TRUE(AZ::Utils::SaveObjectToFile(GetTestFolderPath() + "CircularA.txt", AZ::DataStream::ST_XML, &circularA, m_serializeContext)); - EXPECT_TRUE(AZ::Utils::SaveObjectToFile(GetTestFolderPath() + "CircularB.txt", AZ::DataStream::ST_XML, &circularB, m_serializeContext)); - EXPECT_TRUE(AZ::Utils::SaveObjectToFile(GetTestFolderPath() + "CircularC.txt", AZ::DataStream::ST_XML, &circularC, m_serializeContext)); - EXPECT_TRUE(AZ::Utils::SaveObjectToFile(GetTestFolderPath() + "CircularD.txt", AZ::DataStream::ST_XML, &circularD, m_serializeContext)); + EXPECT_TRUE(m_streamerWrapper->WriteMemoryFile("CircularA.txt", &circularA, m_serializeContext)); + EXPECT_TRUE(m_streamerWrapper->WriteMemoryFile("CircularB.txt", &circularB, m_serializeContext)); + EXPECT_TRUE(m_streamerWrapper->WriteMemoryFile("CircularC.txt", &circularC, m_serializeContext)); + EXPECT_TRUE(m_streamerWrapper->WriteMemoryFile("CircularD.txt", &circularD, m_serializeContext)); + m_assetHandlerAndCatalog->m_numCreations = 0; } } - void TearDownTest() - { - DeleteAssetFromDisk(GetTestFolderPath() + "TestAsset4.txt"); - DeleteAssetFromDisk(GetTestFolderPath() + "TestAsset5.txt"); - DeleteAssetFromDisk(GetTestFolderPath() + "TestAsset6.txt"); - - DeleteAssetFromDisk(GetTestFolderPath() + "TestAsset1.txt"); - DeleteAssetFromDisk(GetTestFolderPath() + "TestAsset2.txt"); - DeleteAssetFromDisk(GetTestFolderPath() + "TestAsset3.txt"); - DeleteAssetFromDisk(GetTestFolderPath() + "DelayLoadAsset.txt"); - DeleteAssetFromDisk(GetTestFolderPath() + "NoLoadAsset.txt"); - - DeleteAssetFromDisk(GetTestFolderPath() + "PreLoadRoot.txt"); - DeleteAssetFromDisk(GetTestFolderPath() + "PreLoadA.txt"); - DeleteAssetFromDisk(GetTestFolderPath() + "PreLoadB.txt"); - DeleteAssetFromDisk(GetTestFolderPath() + "PreLoadC.txt"); - DeleteAssetFromDisk(GetTestFolderPath() + "QueueLoadA.txt"); - DeleteAssetFromDisk(GetTestFolderPath() + "QueueLoadB.txt"); - DeleteAssetFromDisk(GetTestFolderPath() + "QueueLoadC.txt"); - DeleteAssetFromDisk(GetTestFolderPath() + "PreLoadBrokenA.txt"); - DeleteAssetFromDisk(GetTestFolderPath() + "PreLoadBrokenB.txt"); - DeleteAssetFromDisk(GetTestFolderPath() + "PreLoadNoData.txt"); - - DeleteAssetFromDisk(GetTestFolderPath() + "CircularA.txt"); - DeleteAssetFromDisk(GetTestFolderPath() + "CircularB.txt"); - DeleteAssetFromDisk(GetTestFolderPath() + "CircularC.txt"); - DeleteAssetFromDisk(GetTestFolderPath() + "CircularD.txt"); - } - void CheckFinishedCreationsAndDestructions() { // Make sure asset jobs have finished before validating the number of destroyed assets, because it's possible that the asset job @@ -367,7 +339,7 @@ namespace UnitTest }; static constexpr AZStd::chrono::seconds MaxDispatchTimeoutSeconds = BaseAssetManagerTest::DefaultTimeoutSeconds * 12; - + template bool DispatchEventsUntilCondition(AZ::Data::AssetManager& assetManager, Pred&& conditionPredicate, AZStd::chrono::seconds logIntervalSeconds = BaseAssetManagerTest::DefaultTimeoutSeconds, @@ -608,7 +580,7 @@ namespace UnitTest AZ::Data::AssetData::AssetStatus expected_base_status = AZ::Data::AssetData::AssetStatus::Ready; EXPECT_EQ(baseStatus, expected_base_status); } - + TEST_F(AssetJobsFloodTest, RapidAcquireAndRelease) { auto assetUuids = { @@ -641,7 +613,7 @@ namespace UnitTest { Asset asset1 = m_testAssetManager->GetAsset(assetUuid, azrtti_typeid(), AZ::Data::AssetLoadBehavior::PreLoad); - + if (checkLoaded) { asset1.BlockUntilLoadComplete(); @@ -714,8 +686,8 @@ namespace UnitTest AssetWithSerializedData ap; - EXPECT_TRUE(AZ::Utils::SaveObjectToFile(GetTestFolderPath() + "a.txt", AZ::DataStream::ST_XML, &ap, m_serializeContext)); - EXPECT_TRUE(AZ::Utils::SaveObjectToFile(GetTestFolderPath() + "b.txt", AZ::DataStream::ST_XML, &ap, m_serializeContext)); + EXPECT_TRUE(m_streamerWrapper->WriteMemoryFile("a.txt", &ap, m_serializeContext)); + EXPECT_TRUE(m_streamerWrapper->WriteMemoryFile("b.txt", &ap, m_serializeContext)); } auto& assetManager = AssetManager::Instance(); @@ -778,7 +750,7 @@ namespace UnitTest * Verify that loads without using the Asset Container still work correctly */ class AssetContainerDisableTest - : public BaseAssetManagerTest + : public DisklessAssetManagerBase { public: static inline const AZ::Uuid MyAsset1Id{ "{5B29FE2B-6B41-48C9-826A-C723951B0560}" }; @@ -797,7 +769,7 @@ namespace UnitTest void SetUp() override { - BaseAssetManagerTest::SetUp(); + DisklessAssetManagerBase::SetUp(); SetupTest(); } @@ -807,7 +779,7 @@ namespace UnitTest AssetManager::Instance().UnregisterHandler(m_assetHandlerAndCatalog); delete m_assetHandlerAndCatalog; AssetManager::Destroy(); - BaseAssetManagerTest::TearDown(); + DisklessAssetManagerBase::TearDown(); } void SetupAssets() @@ -849,9 +821,9 @@ namespace UnitTest AssetWithSerializedData ap2; AssetWithSerializedData ap3; - EXPECT_TRUE(AZ::Utils::SaveObjectToFile(GetTestFolderPath() + "TestAsset4.txt", AZ::DataStream::ST_XML, &ap1, m_serializeContext)); - EXPECT_TRUE(AZ::Utils::SaveObjectToFile(GetTestFolderPath() + "TestAsset5.txt", AZ::DataStream::ST_XML, &ap2, m_serializeContext)); - EXPECT_TRUE(AZ::Utils::SaveObjectToFile(GetTestFolderPath() + "TestAsset6.txt", AZ::DataStream::ST_XML, &ap3, m_serializeContext)); + EXPECT_TRUE(m_streamerWrapper->WriteMemoryFile("TestAsset4.txt", &ap1, m_serializeContext)); + EXPECT_TRUE(m_streamerWrapper->WriteMemoryFile("TestAsset5.txt", &ap2, m_serializeContext)); + EXPECT_TRUE(m_streamerWrapper->WriteMemoryFile("TestAsset6.txt", &ap3, m_serializeContext)); AssetWithAssetReference assetWithPreload1; AssetWithAssetReference assetWithPreload2; @@ -862,9 +834,9 @@ namespace UnitTest assetWithPreload3.m_asset = m_testAssetManager->CreateAsset(MyAsset6Id, AssetLoadBehavior::PreLoad); EXPECT_EQ(m_assetHandlerAndCatalog->m_numCreations, 3); - EXPECT_TRUE(AZ::Utils::SaveObjectToFile(GetTestFolderPath() + "TestAsset1.txt", AZ::DataStream::ST_XML, &assetWithPreload1, m_serializeContext)); - EXPECT_TRUE(AZ::Utils::SaveObjectToFile(GetTestFolderPath() + "TestAsset2.txt", AZ::DataStream::ST_XML, &assetWithPreload2, m_serializeContext)); - EXPECT_TRUE(AZ::Utils::SaveObjectToFile(GetTestFolderPath() + "TestAsset3.txt", AZ::DataStream::ST_XML, &assetWithPreload3, m_serializeContext)); + EXPECT_TRUE(m_streamerWrapper->WriteMemoryFile("TestAsset1.txt", &assetWithPreload1, m_serializeContext)); + EXPECT_TRUE(m_streamerWrapper->WriteMemoryFile("TestAsset2.txt", &assetWithPreload2, m_serializeContext)); + EXPECT_TRUE(m_streamerWrapper->WriteMemoryFile("TestAsset3.txt", &assetWithPreload3, m_serializeContext)); m_assetHandlerAndCatalog->m_numCreations = 0; } @@ -2014,11 +1986,12 @@ namespace UnitTest CheckFinishedCreationsAndDestructions(); m_assetHandlerAndCatalog->AssetCatalogRequestBus::Handler::BusDisconnect(); } + /** * Run multiple threads that get and release assets simultaneously to test AssetManager's thread safety */ class AssetJobsMultithreadedTest - : public BaseAssetManagerTest + : public DisklessAssetManagerBase { public: static inline const AZ::Uuid MyAsset1Id{ "{5B29FE2B-6B41-48C9-826A-C723951B0560}" }; @@ -2028,6 +2001,7 @@ namespace UnitTest static inline const AZ::Uuid MyAsset5Id{ "{D9CDAB04-D206-431E-BDC0-1DD615D56197}" }; static inline const AZ::Uuid MyAsset6Id{ "{B2F139C3-5032-4B52-ADCA-D52A8F88E043}" }; + // Initialize the Job Manager with 2 threads for the Asset Manager to use. size_t GetNumJobManagerThreads() const override { return 2; } @@ -2078,9 +2052,9 @@ namespace UnitTest AssetWithSerializedData ap2; AssetWithSerializedData ap3; - EXPECT_TRUE(AZ::Utils::SaveObjectToFile(GetTestFolderPath() + "TestAsset4.txt", AZ::DataStream::ST_XML, &ap1, &context)); - EXPECT_TRUE(AZ::Utils::SaveObjectToFile(GetTestFolderPath() + "TestAsset5.txt", AZ::DataStream::ST_XML, &ap2, &context)); - EXPECT_TRUE(AZ::Utils::SaveObjectToFile(GetTestFolderPath() + "TestAsset6.txt", AZ::DataStream::ST_XML, &ap3, &context)); + EXPECT_TRUE(m_streamerWrapper->WriteMemoryFile("TestAsset4.txt", &ap1, &context)); + EXPECT_TRUE(m_streamerWrapper->WriteMemoryFile("TestAsset5.txt", &ap2, &context)); + EXPECT_TRUE(m_streamerWrapper->WriteMemoryFile("TestAsset6.txt", &ap3, &context)); AssetWithAssetReference assetWithPreload1; AssetWithAssetReference assetWithPreload2; @@ -2089,9 +2063,9 @@ namespace UnitTest assetWithPreload2.m_asset = AssetManager::Instance().CreateAsset(MyAsset5Id, AssetLoadBehavior::PreLoad); assetWithPreload3.m_asset = AssetManager::Instance().CreateAsset(MyAsset6Id, AssetLoadBehavior::PreLoad); - EXPECT_TRUE(AZ::Utils::SaveObjectToFile(GetTestFolderPath() + "TestAsset1.txt", AZ::DataStream::ST_XML, &assetWithPreload1, &context)); - EXPECT_TRUE(AZ::Utils::SaveObjectToFile(GetTestFolderPath() + "TestAsset2.txt", AZ::DataStream::ST_XML, &assetWithPreload2, &context)); - EXPECT_TRUE(AZ::Utils::SaveObjectToFile(GetTestFolderPath() + "TestAsset3.txt", AZ::DataStream::ST_XML, &assetWithPreload3, &context)); + EXPECT_TRUE(m_streamerWrapper->WriteMemoryFile("TestAsset1.txt", &assetWithPreload1, &context)); + EXPECT_TRUE(m_streamerWrapper->WriteMemoryFile("TestAsset2.txt", &assetWithPreload2, &context)); + EXPECT_TRUE(m_streamerWrapper->WriteMemoryFile("TestAsset3.txt", &assetWithPreload3, &context)); EXPECT_TRUE(assetHandlerAndCatalog->m_numCreations == 3); assetHandlerAndCatalog->m_numCreations = 0; @@ -2191,22 +2165,22 @@ namespace UnitTest // A will be saved to disk with MyAsset1Id AssetWithAssetReference a; a.m_asset = AssetManager::Instance().CreateAsset(MyAsset2Id); - EXPECT_TRUE(AZ::Utils::SaveObjectToFile(GetTestFolderPath() + "TestAsset1.txt", AZ::DataStream::ST_XML, &a, &context)); + EXPECT_TRUE(m_streamerWrapper->WriteMemoryFile("TestAsset1.txt", &a, &context)); AssetWithAssetReference b; b.m_asset = AssetManager::Instance().CreateAsset(MyAsset3Id); - EXPECT_TRUE(AZ::Utils::SaveObjectToFile(GetTestFolderPath() + "TestAsset2.txt", AZ::DataStream::ST_XML, &b, &context)); + EXPECT_TRUE(m_streamerWrapper->WriteMemoryFile("TestAsset2.txt", &b, &context)); AssetWithAssetReference c; c.m_asset = AssetManager::Instance().CreateAsset(MyAsset4Id); - EXPECT_TRUE(AZ::Utils::SaveObjectToFile(GetTestFolderPath() + "TestAsset3.txt", AZ::DataStream::ST_XML, &c, &context)); + EXPECT_TRUE(m_streamerWrapper->WriteMemoryFile("TestAsset3.txt", &c, &context)); AssetWithAssetReference d; d.m_asset = AssetManager::Instance().CreateAsset(MyAsset5Id); - EXPECT_TRUE(AZ::Utils::SaveObjectToFile(GetTestFolderPath() + "TestAsset4.txt", AZ::DataStream::ST_XML, &d, &context)); + EXPECT_TRUE(m_streamerWrapper->WriteMemoryFile("TestAsset4.txt", &d, &context)); AssetWithAssetReference e; e.m_asset = AssetManager::Instance().CreateAsset(MyAsset6Id); - EXPECT_TRUE(AZ::Utils::SaveObjectToFile(GetTestFolderPath() + "TestAsset5.txt", AZ::DataStream::ST_XML, &e, &context)); + EXPECT_TRUE(m_streamerWrapper->WriteMemoryFile("TestAsset5.txt", &e, &context)); AssetWithAssetReference f; f.m_asset = AssetManager::Instance().CreateAsset(MyAsset1Id); // refer back to asset1 - EXPECT_TRUE(AZ::Utils::SaveObjectToFile(GetTestFolderPath() + "TestAsset6.txt", AZ::DataStream::ST_XML, &f, &context)); + EXPECT_TRUE(m_streamerWrapper->WriteMemoryFile("TestAsset6.txt", &f, &context)); EXPECT_TRUE(assetHandlerAndCatalog->m_numCreations == 6); assetHandlerAndCatalog->m_numCreations = 0; @@ -2347,26 +2321,26 @@ namespace UnitTest // AssetD is MYASSETD AssetWithSerializedData d; d.m_data = 42; - EXPECT_TRUE(AZ::Utils::SaveObjectToFile(GetTestFolderPath() + "TestAsset4.txt", AZ::DataStream::ST_XML, &d, &context)); + EXPECT_TRUE(m_streamerWrapper->WriteMemoryFile("TestAsset4.txt", &d, &context)); // AssetC is MYASSETC AssetWithAssetReference c; c.m_asset = db.CreateAsset(AssetId(MyAssetDId)); // point at D - EXPECT_TRUE(AZ::Utils::SaveObjectToFile(GetTestFolderPath() + "TestAsset3.txt", AZ::DataStream::ST_XML, &c, &context)); + EXPECT_TRUE(m_streamerWrapper->WriteMemoryFile("TestAsset3.txt", &c, &context)); // AssetB is MYASSETB AssetWithAssetReference b; b.m_asset = db.CreateAsset(AssetId(MyAssetCId)); // point at C - EXPECT_TRUE(AZ::Utils::SaveObjectToFile(GetTestFolderPath() + "TestAsset2.txt", AZ::DataStream::ST_XML, &b, &context)); + EXPECT_TRUE(m_streamerWrapper->WriteMemoryFile("TestAsset2.txt", &b, &context)); // AssetA will be written to disk as MYASSETA AssetWithAssetReference a; a.m_asset = db.CreateAsset(AssetId(MyAssetBId)); // point at B - EXPECT_TRUE(AZ::Utils::SaveObjectToFile(GetTestFolderPath() + "TestAsset1.txt", AZ::DataStream::ST_XML, &a, &context)); + EXPECT_TRUE(m_streamerWrapper->WriteMemoryFile("TestAsset1.txt", &a, &context)); } - const size_t numThreads = 4; - AZStd::atomic_int threadCount(numThreads); + constexpr size_t NumThreads = 4; + AZStd::atomic_int threadCount(NumThreads); AZStd::condition_variable cv; AZStd::vector threads; AZStd::atomic_bool keepDispatching(true); @@ -2381,7 +2355,7 @@ namespace UnitTest AZStd::thread dispatchThread(dispatch); - for (size_t threadIdx = 0; threadIdx < numThreads; ++threadIdx) + for (size_t threadIdx = 0; threadIdx < NumThreads; ++threadIdx) { threads.emplace_back([&threadCount, &db, &cv]() { @@ -2569,7 +2543,6 @@ namespace UnitTest #if AZ_TRAIT_DISABLE_FAILED_ASSET_MANAGER_TESTS TEST_F(AssetJobsMultithreadedTest, DISABLED_ParallelDeepAssetReferences) #else - // temporarily disabled until sporadic failures can be root caused TEST_F(AssetJobsMultithreadedTest, ParallelDeepAssetReferences) #endif // AZ_TRAIT_DISABLE_FAILED_ASSET_MANAGER_TESTS { @@ -2577,7 +2550,7 @@ namespace UnitTest } class AssetManagerTests - : public BaseAssetManagerTest + : public DisklessAssetManagerBase { protected: static inline const AZ::Uuid MyAsset1Id{ "{5B29FE2B-6B41-48C9-826A-C723951B0560}" }; @@ -2592,7 +2565,7 @@ namespace UnitTest void SetUp() override { - BaseAssetManagerTest::SetUp(); + DisklessAssetManagerBase::SetUp(); m_console = AZStd::make_unique(); AZ::Interface::Register(m_console.get()); @@ -2631,7 +2604,7 @@ namespace UnitTest AssetManager::Destroy(); AZ::Interface::Unregister(m_console.get()); m_console = nullptr; - BaseAssetManagerTest::TearDown(); + DisklessAssetManagerBase::TearDown(); } }; @@ -2982,7 +2955,7 @@ namespace UnitTest * the middle of loading. The tests help ensure that assets can't get stuck in perpetual loading states. **/ class AssetManagerClearAssetReferenceTests - : public BaseAssetManagerTest + : public DisklessAssetManagerBase { protected: static inline const AZ::Uuid RootAssetId{ "{AB13F568-C676-41FE-A7E9-341F71A78104}" }; @@ -3001,7 +2974,7 @@ namespace UnitTest void SetUp() override { - BaseAssetManagerTest::SetUp(); + DisklessAssetManagerBase::SetUp(); // create the database AssetManager::Descriptor desc; @@ -3039,21 +3012,18 @@ namespace UnitTest // Create and save the dependent asset first, so that we can get a reference to it. AssetWithSerializedData dependentBlockingAsset; - EXPECT_TRUE(AZ::Utils::SaveObjectToFile(GetTestFolderPath() + "DependentPreloadBlockingAsset.txt", - AZ::DataStream::ST_XML, &dependentBlockingAsset, m_serializeContext)); + EXPECT_TRUE(m_streamerWrapper->WriteMemoryFile("DependentPreloadBlockingAsset.txt", &dependentBlockingAsset, m_serializeContext)); AssetWithAssetReference dependentAsset; dependentAsset.m_asset = AssetManager::Instance().CreateAsset( NestedDependentPreloadBlockingAssetId, AssetLoadBehavior::PreLoad); - EXPECT_TRUE(AZ::Utils::SaveObjectToFile(GetTestFolderPath() + "DependentPreloadAsset.txt", - AZ::DataStream::ST_XML, &dependentAsset, m_serializeContext)); + EXPECT_TRUE(m_streamerWrapper->WriteMemoryFile("DependentPreloadAsset.txt", &dependentAsset, m_serializeContext)); // Create and save the top-level asset. AssetWithAssetReference rootAsset; rootAsset.m_asset = AssetManager::Instance().CreateAsset( DependentPreloadAssetId, AssetLoadBehavior::PreLoad); - EXPECT_TRUE(AZ::Utils::SaveObjectToFile(GetTestFolderPath() + "RootAsset.txt", - AZ::DataStream::ST_XML, &rootAsset, m_serializeContext)); + EXPECT_TRUE(m_streamerWrapper->WriteMemoryFile("RootAsset.txt", &rootAsset, m_serializeContext)); } void TearDown() override @@ -3065,7 +3035,7 @@ namespace UnitTest delete m_assetHandlerAndCatalog; AssetManager::Destroy(); - BaseAssetManagerTest::TearDown(); + DisklessAssetManagerBase::TearDown(); } }; diff --git a/Code/Framework/AzCore/Tests/Asset/BaseAssetManagerTest.cpp b/Code/Framework/AzCore/Tests/Asset/BaseAssetManagerTest.cpp index c6fa296cbc..532cb0a1d8 100644 --- a/Code/Framework/AzCore/Tests/Asset/BaseAssetManagerTest.cpp +++ b/Code/Framework/AzCore/Tests/Asset/BaseAssetManagerTest.cpp @@ -165,4 +165,254 @@ namespace UnitTest EXPECT_FALSE(AssetManager::Instance().HasActiveJobsOrStreamerRequests()); } + + MemoryStreamerWrapper::MemoryStreamerWrapper() + { + using ::testing::_; + using ::testing::NiceMock; + using ::testing::Return; + + ON_CALL(m_mockStreamer, SuspendProcessing()).WillByDefault([this]() + { + m_suspended = true; + }); + + ON_CALL(m_mockStreamer, ResumeProcessing()).WillByDefault([this]() + { + AZStd::unique_lock lock(m_mutex); + + m_suspended = false; + + while (!m_processingQueue.empty()) + { + FileRequestHandle requestHandle = m_processingQueue.front(); + m_processingQueue.pop(); + + const auto& onCompleteCallback = GetReadRequest(requestHandle)->m_callback; + + if (onCompleteCallback) + { + onCompleteCallback(requestHandle); + } + } + }); + + ON_CALL(m_mockStreamer, Read(_, ::testing::An(), _, _, _, _)) + .WillByDefault( + [this]( + [[maybe_unused]] AZStd::string_view relativePath, IStreamerTypes::RequestMemoryAllocator& allocator, size_t size, + AZStd::chrono::microseconds deadline, IStreamerTypes::Priority priority, [[maybe_unused]] size_t offset) + { + AZStd::unique_lock lock(m_mutex); + + ReadRequest request; + + // Save off the requested deadline and priority + request.m_deadline = deadline; + request.m_priority = priority; + request.m_data = allocator.Allocate(size, size, 8); + + const auto* virtualFile = FindFile(relativePath); + + AZ_Assert( + virtualFile->size() == size, "Streamer read request size did not match size of saved file: %d vs %d (%.*s)", + virtualFile->size(), size, + relativePath.size(), relativePath.data()); + AZ_Assert(size > 0, "Size is zero %.*s", relativePath.size(), relativePath.data()); + + memcpy(request.m_data.m_address, virtualFile->data(), size); + + // Create a real file request result and return it + request.m_request = m_context.GetNewExternalRequest(); + + m_readRequests.push_back(request); + + return request.m_request; + }); + + ON_CALL(m_mockStreamer, SetRequestCompleteCallback(_, _)) + .WillByDefault([this](FileRequestPtr& request, AZ::IO::IStreamer::OnCompleteCallback callback) -> FileRequestPtr& + { + // Save off the callback just so that we can call it when the request is "done" + AZStd::unique_lock lock(m_mutex); + ReadRequest* readRequest = GetReadRequest(request); + readRequest->m_callback = callback; + + return request; + }); + + ON_CALL(m_mockStreamer, QueueRequest(_)) + .WillByDefault([this](const auto& fileRequest) + { + if (!m_suspended) + { + decltype(ReadRequest::m_callback) onCompleteCallback; + + AZStd::unique_lock lock(m_mutex); + ReadRequest* readRequest = GetReadRequest(fileRequest); + onCompleteCallback = readRequest->m_callback; + + if (onCompleteCallback) + { + onCompleteCallback(fileRequest); + + m_readRequests.erase(readRequest); + } + } + else + { + AZStd::unique_lock lock(m_mutex); + + m_processingQueue.push(fileRequest); + } + }); + + ON_CALL(m_mockStreamer, GetRequestStatus(_)) + .WillByDefault([]([[maybe_unused]] FileRequestHandle request) + { + // Return whatever request status has been set in this class + return IO::IStreamerTypes::RequestStatus::Completed; + }); + + ON_CALL(m_mockStreamer, GetReadRequestResult(_, _, _, _)) + .WillByDefault([this]( + [[maybe_unused]] FileRequestHandle request, void*& buffer, AZ::u64& numBytesRead, + IStreamerTypes::ClaimMemory claimMemory) + { + // Make sure the requestor plans to free the data buffer we allocated. + EXPECT_EQ(claimMemory, IStreamerTypes::ClaimMemory::Yes); + + AZStd::unique_lock lock(m_mutex); + + ReadRequest* readRequest = GetReadRequest(request); + + // Provide valid data buffer results. + numBytesRead = readRequest->m_data.m_size; + buffer = readRequest->m_data.m_address; + + return true; + }); + + ON_CALL(m_mockStreamer, RescheduleRequest(_, _, _)) + .WillByDefault([this](IO::FileRequestPtr target, AZStd::chrono::microseconds newDeadline, IO::IStreamerTypes::Priority newPriority) + { + AZStd::unique_lock lock(m_mutex); + ReadRequest* readRequest = GetReadRequest(target); + + readRequest->m_deadline = newDeadline; + readRequest->m_priority = newPriority; + + return target; + }); + } + + ReadRequest* MemoryStreamerWrapper::GetReadRequest(FileRequestHandle request) + { + auto itr = AZStd::find_if( + m_readRequests.begin(), m_readRequests.end(), + [request](const ReadRequest& searchItem) -> bool + { + return (searchItem.m_request == request); + }); + + return itr; + } + + AZStd::vector* MemoryStreamerWrapper::FindFile(AZStd::string_view path) + { + auto itr = m_virtualFiles.find(path); + + if (itr == m_virtualFiles.end()) + { + // Path didn't work as-is, does it have the test folder prefixed? If so try removing it + if (AZ::StringFunc::StartsWith(path, GetTestFolderPath())) + { + AZStd::string_view pathWithoutFolder = path; + + pathWithoutFolder = AZ::StringFunc::LStrip(pathWithoutFolder, GetTestFolderPath().c_str()); + itr = m_virtualFiles.find(pathWithoutFolder); + } + else // Path isn't prefixed, so try adding it + { + itr = m_virtualFiles.find(GetTestFolderPath().append(path)); + } + } + + if (itr != m_virtualFiles.end()) + { + return &itr->second; + } + + // Currently no test expects a file not to exist so we assert to make it easy to quickly find where something went wrong + // If we ever need to test for a non-existent file this assert should just be conditionally disabled for that specific test + AZ_Assert(false, "Failed to find virtual file %*.s", path.size(), path.data()) + + return nullptr; + } + + void DisklessAssetManagerBase::SetUp() + { + using ::testing::_; + using ::testing::NiceMock; + using ::testing::Return; + + BaseAssetManagerTest::SetUp(); + + ON_CALL(m_fileIO, Size(::testing::Matcher(::testing::_), _)) + .WillByDefault( + [this](const char* path, u64& size) + { + AZStd::scoped_lock lock(m_streamerWrapper->m_mutex); + + const auto* file = m_streamerWrapper->FindFile(path); + + if (file) + { + size = file->size(); + return ResultCode::Success; + } + + AZ_Error("DisklessAssetManagerBase", false, "Failed to find virtual file %.*s", path); + + return ResultCode::Error; + }); + + m_prevFileIO = IO::FileIOBase::GetInstance(); + IO::FileIOBase::SetInstance(nullptr); + IO::FileIOBase::SetInstance(&m_fileIO); + } + + void DisklessAssetManagerBase::TearDown() + { + IO::FileIOBase::SetInstance(nullptr); + IO::FileIOBase::SetInstance(m_prevFileIO); + + BaseAssetManagerTest::TearDown(); + } + + IO::IStreamer* DisklessAssetManagerBase::CreateStreamer() + { + m_streamerWrapper = AZStd::make_unique(); + + return &(m_streamerWrapper->m_mockStreamer); + } + + void DisklessAssetManagerBase::DestroyStreamer(IO::IStreamer*) + { + m_streamerWrapper = nullptr; + } + + void DisklessAssetManagerBase::WriteAssetToDisk(const AZStd::string& assetName, const AZStd::string&) + { + AZStd::string assetFileName = GetTestFolderPath() + assetName; + + AssetWithCustomData asset; + + EXPECT_TRUE(m_streamerWrapper->WriteMemoryFile(assetFileName, &asset, m_serializeContext)); + } + + void DisklessAssetManagerBase::DeleteAssetFromDisk(const AZStd::string&) + { + + } } diff --git a/Code/Framework/AzCore/Tests/Asset/BaseAssetManagerTest.h b/Code/Framework/AzCore/Tests/Asset/BaseAssetManagerTest.h index 29c2c124cd..af48c74a60 100644 --- a/Code/Framework/AzCore/Tests/Asset/BaseAssetManagerTest.h +++ b/Code/Framework/AzCore/Tests/Asset/BaseAssetManagerTest.h @@ -20,7 +20,8 @@ #include #include #include - +#include +#include namespace UnitTest { @@ -58,7 +59,11 @@ namespace UnitTest // Subclasses can optionally override the streamer creation and destruction virtual IO::IStreamer* CreateStreamer() { return aznew IO::Streamer(AZStd::thread_desc{}, StreamerComponent::CreateStreamerStack()); } - virtual void DestroyStreamer(IO::IStreamer* streamer) { delete streamer; } + virtual void DestroyStreamer(IO::IStreamer* streamer) + { + delete streamer; + streamer = nullptr; + } void SetUp() override; void TearDown() override; @@ -66,8 +71,8 @@ namespace UnitTest static void SuppressTraceOutput(bool suppress); // Helper methods to create and destroy actual assets on the disk for true end-to-end asset loading. - void WriteAssetToDisk(const AZStd::string& assetName, const AZStd::string& assetIdGuid); - void DeleteAssetFromDisk(const AZStd::string& assetName); + virtual void WriteAssetToDisk(const AZStd::string& assetName, const AZStd::string& assetIdGuid); + virtual void DeleteAssetFromDisk(const AZStd::string& assetName); void BlockUntilAssetJobsAreComplete(); @@ -82,4 +87,57 @@ namespace UnitTest AZStd::vector m_assetsWritten; }; + + struct ReadRequest + { + AZStd::chrono::milliseconds m_deadline{}; + AZ::IO::IStreamerTypes::Priority m_priority{}; + IO::IStreamerTypes::RequestMemoryAllocatorResult m_data{ nullptr, 0, IO::IStreamerTypes::MemoryType::ReadWrite }; + AZ::IO::IStreamer::OnCompleteCallback m_callback; + IO::FileRequestPtr m_request; + }; + + struct MemoryStreamerWrapper + { + MemoryStreamerWrapper(); + ~MemoryStreamerWrapper() = default; + + ReadRequest* GetReadRequest(IO::FileRequestHandle request); + + template + bool WriteMemoryFile(const AZStd::string& filePath, TObject* object, AZ::SerializeContext* context) + { + auto& buffer = m_virtualFiles[filePath]; + ByteContainerStream stream(&buffer); + + return AZ::Utils::SaveObjectToStream(stream, DataStream::StreamType::ST_XML, object, context); + } + + AZStd::vector* FindFile(AZStd::string_view path); + + ::testing::NiceMock m_mockStreamer; + IO::StreamerContext m_context; + AZStd::atomic_bool m_suspended{ false }; + + AZStd::recursive_mutex m_mutex; + AZStd::queue m_processingQueue; // Keeps tracks of requests that have been queued while processing is suspended + AZStd::vector m_readRequests; + AZStd::unordered_map> m_virtualFiles; + }; + + struct DisklessAssetManagerBase : BaseAssetManagerTest + { + void SetUp() override; + void TearDown() override; + IO::IStreamer* CreateStreamer() override; + void DestroyStreamer(IO::IStreamer*) override; + + void WriteAssetToDisk(const AZStd::string& assetName, const AZStd::string& assetIdGuid) override; + void DeleteAssetFromDisk(const AZStd::string& assetName) override; + + AZStd::unique_ptr m_streamerWrapper; + ::testing::NiceMock m_fileIO; + IO::FileIOBase* m_prevFileIO{}; + }; + } diff --git a/Code/Framework/AzCore/Tests/TestCatalog.cpp b/Code/Framework/AzCore/Tests/TestCatalog.cpp index c633c6391b..cb8fa11c72 100644 --- a/Code/Framework/AzCore/Tests/TestCatalog.cpp +++ b/Code/Framework/AzCore/Tests/TestCatalog.cpp @@ -167,7 +167,8 @@ namespace UnitTest if (!info.m_streamName.empty()) { AZStd::string fullName = GetTestFolderPath() + info.m_streamName; - info.m_dataLen = static_cast(IO::SystemFile::Length(fullName.c_str())); + IO::FileIOBase* io = IO::FileIOBase::GetInstance(); + io->Size(fullName.c_str(), info.m_dataLen); } else { @@ -187,8 +188,11 @@ namespace UnitTest if (!info.m_streamName.empty()) { + IO::FileIOBase* io = AZ::IO::FileIOBase::GetInstance(); + AZStd::string fullName = GetTestFolderPath() + info.m_streamName; - info.m_dataLen = static_cast(IO::SystemFile::Length(fullName.c_str())); + + io->Size(fullName.c_str(), info.m_dataLen); } else { From 57d688fbc1dcd51d9dce146c70457bb5c5c8acb3 Mon Sep 17 00:00:00 2001 From: AMZN-nggieber <52797929+AMZN-nggieber@users.noreply.github.com> Date: Wed, 1 Dec 2021 08:03:36 -0800 Subject: [PATCH 8/9] Added Tests for Gem Catalog Filtering (#5999) * Added Tests for Gem Catalog Filtering Signed-off-by: nggieber <52797929+AMZN-nggieber@users.noreply.github.com> * Addressed PR feedback, Renamed all tests to Osherove naming pattern Signed-off-by: nggieber <52797929+AMZN-nggieber@users.noreply.github.com> --- Code/Tools/ProjectManager/CMakeLists.txt | 1 + .../Source/GemCatalog/GemInfo.h | 4 +- .../ProjectManager/tests/GemCatalogTests.cpp | 543 +++++++++++++++++- 3 files changed, 529 insertions(+), 19 deletions(-) diff --git a/Code/Tools/ProjectManager/CMakeLists.txt b/Code/Tools/ProjectManager/CMakeLists.txt index a47ccb62c9..9974125ffb 100644 --- a/Code/Tools/ProjectManager/CMakeLists.txt +++ b/Code/Tools/ProjectManager/CMakeLists.txt @@ -92,6 +92,7 @@ if(PAL_TRAIT_BUILD_TESTS_SUPPORTED) AZ::AzTest AZ::AzFramework AZ::AzFrameworkTestShared + AZ::AzQtComponents AZ::ProjectManager.Static ) diff --git a/Code/Tools/ProjectManager/Source/GemCatalog/GemInfo.h b/Code/Tools/ProjectManager/Source/GemCatalog/GemInfo.h index 5c1bc90c6e..23e95cf487 100644 --- a/Code/Tools/ProjectManager/Source/GemCatalog/GemInfo.h +++ b/Code/Tools/ProjectManager/Source/GemCatalog/GemInfo.h @@ -73,10 +73,10 @@ namespace O3DE::ProjectManager QString m_path; QString m_name = "Unknown Gem Name"; - QString m_displayName = "Unknown Gem Name"; + QString m_displayName; QString m_creator = "Unknown Creator"; GemOrigin m_gemOrigin = Local; - bool m_isAdded = false; //! Is the gem currently added and enabled in the project? + bool m_isAdded = false; //! Is the gem explicitly added (not a dependency) and enabled in the project? QString m_summary = "No summary provided."; Platforms m_platforms; Types m_types; //! Asset and/or Code and/or Tool diff --git a/Code/Tools/ProjectManager/tests/GemCatalogTests.cpp b/Code/Tools/ProjectManager/tests/GemCatalogTests.cpp index f5c6d5196a..701a1ddcef 100644 --- a/Code/Tools/ProjectManager/tests/GemCatalogTests.cpp +++ b/Code/Tools/ProjectManager/tests/GemCatalogTests.cpp @@ -8,8 +8,8 @@ #include #include -#include +#include namespace O3DE::ProjectManager { @@ -17,14 +17,22 @@ namespace O3DE::ProjectManager : public ::UnitTest::ScopedAllocatorSetupFixture { public: + void SetUp() override + { + m_gemModel.reset(new GemModel()); + } - GemCatalogTests() = default; + void TearDown() override + { + m_gemModel.release(); + } + + protected: + AZStd::unique_ptr m_gemModel; }; - TEST_F(GemCatalogTests, GemCatalog_Displays_But_Does_Not_Add_Dependencies) + TEST_F(GemCatalogTests, GemCatalog_GemWithDependencies_DisplaysButDoesNotAddDependencies) { - GemModel* gemModel = new GemModel(); - // given 3 gems a,b,c where a depends on b which depends on c GemInfo gemA, gemB, gemC; QModelIndex indexA, indexB, indexC; @@ -35,30 +43,531 @@ namespace O3DE::ProjectManager gemA.m_dependencies = QStringList({ "b" }); gemB.m_dependencies = QStringList({ "c" }); - gemModel->AddGem(gemA); - indexA = gemModel->FindIndexByNameString(gemA.m_name); + indexA = m_gemModel->AddGem(gemA); + indexB = m_gemModel->AddGem(gemB); + indexC = m_gemModel->AddGem(gemC); - gemModel->AddGem(gemB); - indexB = gemModel->FindIndexByNameString(gemB.m_name); - - gemModel->AddGem(gemC); - indexC = gemModel->FindIndexByNameString(gemC.m_name); - - gemModel->UpdateGemDependencies(); + m_gemModel->UpdateGemDependencies(); EXPECT_FALSE(GemModel::IsAdded(indexA)); EXPECT_FALSE(GemModel::IsAddedDependency(indexB) || GemModel::IsAddedDependency(indexC)); // when a is added - GemModel::SetIsAdded(*gemModel, indexA, true); + GemModel::SetIsAdded(*m_gemModel, indexA, true); // expect b and c are now dependencies of an added gem but not themselves added // cmake will handle dependencies EXPECT_TRUE(GemModel::IsAddedDependency(indexB) && GemModel::IsAddedDependency(indexC)); - EXPECT_TRUE(!GemModel::IsAdded(indexB) && !GemModel::IsAdded(indexC)); + EXPECT_FALSE(GemModel::IsAdded(indexB) || GemModel::IsAdded(indexC)); - QVector gemsToAdd = gemModel->GatherGemsToBeAdded(); + const QVector& gemsToAdd = m_gemModel->GatherGemsToBeAdded(); EXPECT_TRUE(gemsToAdd.size() == 1); EXPECT_EQ(GemModel::GetName(gemsToAdd.at(0)), gemA.m_name); } + + class GemCatalogFilterTests + : public GemCatalogTests + { + public: + void SetUp() override + { + GemCatalogTests::SetUp(); + m_proxyModel.reset(new GemSortFilterProxyModel(m_gemModel.get())); + } + + void TearDown() override + { + m_proxyModel.release(); + GemCatalogTests::TearDown(); + } + + protected: + AZStd::unique_ptr m_proxyModel; + }; + + class GemCatalogSearchFilterTests + : public GemCatalogFilterTests + { + public: + void SetUp() override + { + GemCatalogFilterTests::SetUp(); + + GemInfo gemfilterName, gemfilterDisplayName, gemfilterCreator, gemfilterSummary, gemfilterFeature; + + gemfilterName.m_name = "Name"; + gemfilterDisplayName.m_name = "D"; + gemfilterCreator.m_name = "C"; + gemfilterSummary.m_name = "S"; + gemfilterFeature.m_name = "F"; + + gemfilterDisplayName.m_displayName = "Display Name"; + gemfilterCreator.m_creator = "Johnathon Doe"; + gemfilterSummary.m_summary = "Unique Summary"; + gemfilterFeature.m_features.append("Creative Feature"); + + m_gemRows.append(m_gemModel->AddGem(gemfilterName).row()); + m_gemRows.append(m_gemModel->AddGem(gemfilterDisplayName).row()); + m_gemRows.append(m_gemModel->AddGem(gemfilterCreator).row()); + m_gemRows.append(m_gemModel->AddGem(gemfilterSummary).row()); + m_gemRows.append(m_gemModel->AddGem(gemfilterFeature).row()); + } + + protected: + enum RowOrder + { + Name, + DisplayName, + Creator, + Summary, + Features + }; + + QVector m_gemRows; + }; + + TEST_F(GemCatalogSearchFilterTests, GemCatalogFilters_SearchStringName_ShowsNameGems) + { + m_proxyModel->SetSearchString("Name"); + + EXPECT_TRUE(m_proxyModel->filterAcceptsRow(m_gemRows[Name], QModelIndex())); + EXPECT_TRUE(m_proxyModel->filterAcceptsRow(m_gemRows[DisplayName], QModelIndex())); + EXPECT_FALSE(m_proxyModel->filterAcceptsRow(m_gemRows[Creator], QModelIndex())); + EXPECT_FALSE(m_proxyModel->filterAcceptsRow(m_gemRows[Summary], QModelIndex())); + EXPECT_FALSE(m_proxyModel->filterAcceptsRow(m_gemRows[Features], QModelIndex())); + } + + TEST_F(GemCatalogSearchFilterTests, GemCatalogFilters_SearchStringDisplayName_ShowsDisplayNameGem) + { + m_proxyModel->SetSearchString("Display Name"); + + EXPECT_FALSE(m_proxyModel->filterAcceptsRow(m_gemRows[Name], QModelIndex())); + EXPECT_TRUE(m_proxyModel->filterAcceptsRow(m_gemRows[DisplayName], QModelIndex())); + EXPECT_FALSE(m_proxyModel->filterAcceptsRow(m_gemRows[Creator], QModelIndex())); + EXPECT_FALSE(m_proxyModel->filterAcceptsRow(m_gemRows[Summary], QModelIndex())); + EXPECT_FALSE(m_proxyModel->filterAcceptsRow(m_gemRows[Features], QModelIndex())); + } + + TEST_F(GemCatalogSearchFilterTests, GemCatalogFilters_SearchStringCreator_ShowsCreatorGem) + { + m_proxyModel->SetSearchString("Johnathon Doe"); + + EXPECT_FALSE(m_proxyModel->filterAcceptsRow(m_gemRows[Name], QModelIndex())); + EXPECT_FALSE(m_proxyModel->filterAcceptsRow(m_gemRows[DisplayName], QModelIndex())); + EXPECT_TRUE(m_proxyModel->filterAcceptsRow(m_gemRows[Creator], QModelIndex())); + EXPECT_FALSE(m_proxyModel->filterAcceptsRow(m_gemRows[Summary], QModelIndex())); + EXPECT_FALSE(m_proxyModel->filterAcceptsRow(m_gemRows[Features], QModelIndex())); + } + + TEST_F(GemCatalogSearchFilterTests, GemCatalogFilters_SearchStringSummary_ShowsSummaryGem) + { + m_proxyModel->SetSearchString("Unique Summary"); + + EXPECT_FALSE(m_proxyModel->filterAcceptsRow(m_gemRows[Name], QModelIndex())); + EXPECT_FALSE(m_proxyModel->filterAcceptsRow(m_gemRows[DisplayName], QModelIndex())); + EXPECT_FALSE(m_proxyModel->filterAcceptsRow(m_gemRows[Creator], QModelIndex())); + EXPECT_TRUE(m_proxyModel->filterAcceptsRow(m_gemRows[Summary], QModelIndex())); + EXPECT_FALSE(m_proxyModel->filterAcceptsRow(m_gemRows[Features], QModelIndex())); + } + + TEST_F(GemCatalogSearchFilterTests, GemCatalogFilters_SearchStringFeatures_ShowsFeatureGem) + { + m_proxyModel->SetSearchString("Creative"); + + EXPECT_FALSE(m_proxyModel->filterAcceptsRow(m_gemRows[Name], QModelIndex())); + EXPECT_FALSE(m_proxyModel->filterAcceptsRow(m_gemRows[DisplayName], QModelIndex())); + EXPECT_FALSE(m_proxyModel->filterAcceptsRow(m_gemRows[Creator], QModelIndex())); + EXPECT_FALSE(m_proxyModel->filterAcceptsRow(m_gemRows[Summary], QModelIndex())); + EXPECT_TRUE(m_proxyModel->filterAcceptsRow(m_gemRows[Features], QModelIndex())); + } + + TEST_F(GemCatalogSearchFilterTests, GemCatalogFilters_SearchStringEmpty_ShowsAll) + { + m_proxyModel->SetSearchString(""); + + EXPECT_TRUE(m_proxyModel->filterAcceptsRow(m_gemRows[Name], QModelIndex())); + EXPECT_TRUE(m_proxyModel->filterAcceptsRow(m_gemRows[DisplayName], QModelIndex())); + EXPECT_TRUE(m_proxyModel->filterAcceptsRow(m_gemRows[Creator], QModelIndex())); + EXPECT_TRUE(m_proxyModel->filterAcceptsRow(m_gemRows[Summary], QModelIndex())); + EXPECT_TRUE(m_proxyModel->filterAcceptsRow(m_gemRows[Features], QModelIndex())); + } + + TEST_F(GemCatalogSearchFilterTests, GemCatalogFilters_SearchStringCommonCharacter_ShowsAll) + { + // All gems contain "a" in a searchable field so all should be shown + m_proxyModel->SetSearchString("a"); + + EXPECT_TRUE(m_proxyModel->filterAcceptsRow(m_gemRows[Name], QModelIndex())); + EXPECT_TRUE(m_proxyModel->filterAcceptsRow(m_gemRows[DisplayName], QModelIndex())); + EXPECT_TRUE(m_proxyModel->filterAcceptsRow(m_gemRows[Creator], QModelIndex())); + EXPECT_TRUE(m_proxyModel->filterAcceptsRow(m_gemRows[Summary], QModelIndex())); + EXPECT_TRUE(m_proxyModel->filterAcceptsRow(m_gemRows[Features], QModelIndex())); + } + + TEST_F(GemCatalogSearchFilterTests, GemCatalogFilters_SearchStringDifferentCaseCommonCharacter_ShowsAll) + { + // No gems contain the character "A" but search should be case insensitive + m_proxyModel->SetSearchString("A"); + + EXPECT_TRUE(m_proxyModel->filterAcceptsRow(m_gemRows[Name], QModelIndex())); + EXPECT_TRUE(m_proxyModel->filterAcceptsRow(m_gemRows[DisplayName], QModelIndex())); + EXPECT_TRUE(m_proxyModel->filterAcceptsRow(m_gemRows[Creator], QModelIndex())); + EXPECT_TRUE(m_proxyModel->filterAcceptsRow(m_gemRows[Summary], QModelIndex())); + EXPECT_TRUE(m_proxyModel->filterAcceptsRow(m_gemRows[Features], QModelIndex())); + } + + TEST_F(GemCatalogSearchFilterTests, GemCatalogFilters_SearchStringNoneContainCharacter_ShowsNone) + { + // No gems contain the character "z" or "Z" so none should be shown + m_proxyModel->SetSearchString("z"); + + EXPECT_FALSE(m_proxyModel->filterAcceptsRow(m_gemRows[Name], QModelIndex())); + EXPECT_FALSE(m_proxyModel->filterAcceptsRow(m_gemRows[DisplayName], QModelIndex())); + EXPECT_FALSE(m_proxyModel->filterAcceptsRow(m_gemRows[Creator], QModelIndex())); + EXPECT_FALSE(m_proxyModel->filterAcceptsRow(m_gemRows[Summary], QModelIndex())); + EXPECT_FALSE(m_proxyModel->filterAcceptsRow(m_gemRows[Features], QModelIndex())); + } + + TEST_F(GemCatalogSearchFilterTests, GemCatalogFilters_SearchStringPartialMatchString_ShowsNone) + { + // Token matching is currently not supported + // The whole string must match a substring + m_proxyModel->SetSearchString("Name Token"); + + EXPECT_FALSE(m_proxyModel->filterAcceptsRow(m_gemRows[Name], QModelIndex())); + EXPECT_FALSE(m_proxyModel->filterAcceptsRow(m_gemRows[DisplayName], QModelIndex())); + EXPECT_FALSE(m_proxyModel->filterAcceptsRow(m_gemRows[Creator], QModelIndex())); + EXPECT_FALSE(m_proxyModel->filterAcceptsRow(m_gemRows[Summary], QModelIndex())); + EXPECT_FALSE(m_proxyModel->filterAcceptsRow(m_gemRows[Features], QModelIndex())); + } + + class GemCatalogSelectedActiveFilterTests + : public GemCatalogFilterTests + { + public: + void SetUp() override + { + GemCatalogFilterTests::SetUp(); + + GemInfo gemSelected, gemSelectedDep, gemUnselected, gemUnselectedDep, gemActive, gemInactive; + + gemSelected.m_name = "selected"; + gemSelectedDep.m_name = "selectedDep"; + gemUnselected.m_name = "unselected"; + gemUnselectedDep.m_name = "unselectedDep"; + gemActive.m_name = "active"; + gemInactive.m_name = "inactive"; + + gemSelected.m_dependencies = QStringList({ "selectedDep" }); + gemUnselected.m_dependencies = QStringList({ "unselectedDep" }); + + m_gemIndices.append(m_gemModel->AddGem(gemSelected)); + m_gemIndices.append(m_gemModel->AddGem(gemSelectedDep)); + m_gemIndices.append(m_gemModel->AddGem(gemUnselected)); + m_gemIndices.append(m_gemModel->AddGem(gemUnselectedDep)); + m_gemIndices.append(m_gemModel->AddGem(gemActive)); + m_gemIndices.append(m_gemModel->AddGem(gemInactive)); + + m_gemModel->UpdateGemDependencies(); + + // Set intial state of catalog with the to be unselected gem currently added along with active gem + GemModel::SetIsAdded(*m_gemModel, m_gemIndices[Unselected], true); + GemModel::SetWasPreviouslyAdded(*m_gemModel, m_gemIndices[Unselected], true); + GemModel::SetIsAdded(*m_gemModel, m_gemIndices[Active], true); + GemModel::SetWasPreviouslyAdded(*m_gemModel, m_gemIndices[Active], true); + + // Add selected gem and remove unselected gem + GemModel::SetIsAdded(*m_gemModel, m_gemIndices[Selected], true); + GemModel::SetIsAdded(*m_gemModel, m_gemIndices[Unselected], false); + } + + protected: + enum IndexOrder + { + Selected, + SelectedDep, + Unselected, + UnselectedDep, + Active, + Inactive + }; + + QVector m_gemIndices; + }; + + TEST_F(GemCatalogSelectedActiveFilterTests, GemCatalogFilters_SelectedActiveIntialState_AddedGemsAndDependenciesAreAdded) + { + // Check if gems are all in expected state + // if this test fails all other Selected/Active tests are invalid + EXPECT_TRUE(GemModel::IsAdded(m_gemIndices[Selected])); + EXPECT_TRUE(GemModel::IsAddedDependency(m_gemIndices[SelectedDep])); + EXPECT_FALSE(GemModel::IsAdded(m_gemIndices[Unselected])); + EXPECT_FALSE(GemModel::IsAddedDependency(m_gemIndices[UnselectedDep])); + EXPECT_TRUE(GemModel::IsAdded(m_gemIndices[Active])); + EXPECT_FALSE(GemModel::IsAdded(m_gemIndices[Inactive])); + } + + TEST_F(GemCatalogSelectedActiveFilterTests, GemCatalogFilters_SelectedActiveNoFilter_ShowsAll) + { + // Filter is clear + EXPECT_TRUE(m_proxyModel->filterAcceptsRow(m_gemIndices[Selected].row(), QModelIndex())); + EXPECT_TRUE(m_proxyModel->filterAcceptsRow(m_gemIndices[SelectedDep].row(), QModelIndex())); + EXPECT_TRUE(m_proxyModel->filterAcceptsRow(m_gemIndices[Unselected].row(), QModelIndex())); + EXPECT_TRUE(m_proxyModel->filterAcceptsRow(m_gemIndices[UnselectedDep].row(), QModelIndex())); + EXPECT_TRUE(m_proxyModel->filterAcceptsRow(m_gemIndices[Active].row(), QModelIndex())); + EXPECT_TRUE(m_proxyModel->filterAcceptsRow(m_gemIndices[Inactive].row(), QModelIndex())); + } + + TEST_F(GemCatalogSelectedActiveFilterTests, GemCatalogFilters_FilterSelected_ShowsSelectedAndDependencies) + { + // Check selected filter + // Selected dependencies should also be shown + m_proxyModel->SetGemSelected(GemSortFilterProxyModel::GemSelected::Selected); + + EXPECT_TRUE(m_proxyModel->filterAcceptsRow(m_gemIndices[Selected].row(), QModelIndex())); + EXPECT_TRUE(m_proxyModel->filterAcceptsRow(m_gemIndices[SelectedDep].row(), QModelIndex())); + EXPECT_FALSE(m_proxyModel->filterAcceptsRow(m_gemIndices[Unselected].row(), QModelIndex())); + EXPECT_FALSE(m_proxyModel->filterAcceptsRow(m_gemIndices[UnselectedDep].row(), QModelIndex())); + EXPECT_FALSE(m_proxyModel->filterAcceptsRow(m_gemIndices[Active].row(), QModelIndex())); + EXPECT_FALSE(m_proxyModel->filterAcceptsRow(m_gemIndices[Inactive].row(), QModelIndex())); + } + + TEST_F(GemCatalogSelectedActiveFilterTests, GemCatalogFilters_FilterUnselected_ShowsUnselectedAndDependencies) + { + // Check unselected filter + // Unselected dependencies should also be shown + m_proxyModel->SetGemSelected(GemSortFilterProxyModel::GemSelected::Unselected); + + EXPECT_FALSE(m_proxyModel->filterAcceptsRow(m_gemIndices[Selected].row(), QModelIndex())); + EXPECT_FALSE(m_proxyModel->filterAcceptsRow(m_gemIndices[SelectedDep].row(), QModelIndex())); + EXPECT_TRUE(m_proxyModel->filterAcceptsRow(m_gemIndices[Unselected].row(), QModelIndex())); + EXPECT_TRUE(m_proxyModel->filterAcceptsRow(m_gemIndices[UnselectedDep].row(), QModelIndex())); + EXPECT_FALSE(m_proxyModel->filterAcceptsRow(m_gemIndices[Active].row(), QModelIndex())); + EXPECT_FALSE(m_proxyModel->filterAcceptsRow(m_gemIndices[Inactive].row(), QModelIndex())); + } + + TEST_F(GemCatalogSelectedActiveFilterTests, GemCatalogFilters_FilterSelectedAndUnselected_ShowsAllChangesAndDependencies) + { + // Check both un/selected filter + m_proxyModel->SetGemSelected(GemSortFilterProxyModel::GemSelected::Both); + + EXPECT_TRUE(m_proxyModel->filterAcceptsRow(m_gemIndices[Selected].row(), QModelIndex())); + EXPECT_TRUE(m_proxyModel->filterAcceptsRow(m_gemIndices[SelectedDep].row(), QModelIndex())); + EXPECT_TRUE(m_proxyModel->filterAcceptsRow(m_gemIndices[Unselected].row(), QModelIndex())); + EXPECT_TRUE(m_proxyModel->filterAcceptsRow(m_gemIndices[UnselectedDep].row(), QModelIndex())); + EXPECT_FALSE(m_proxyModel->filterAcceptsRow(m_gemIndices[Active].row(), QModelIndex())); + EXPECT_FALSE(m_proxyModel->filterAcceptsRow(m_gemIndices[Inactive].row(), QModelIndex())); + } + + TEST_F(GemCatalogSelectedActiveFilterTests, GemCatalogFilters_FilterActive_ShowsActive) + { + // Check active filter + // Active dependencies should also be shown + m_proxyModel->SetGemActive(GemSortFilterProxyModel::GemActive::Active); + + EXPECT_TRUE(m_proxyModel->filterAcceptsRow(m_gemIndices[Selected].row(), QModelIndex())); + EXPECT_TRUE(m_proxyModel->filterAcceptsRow(m_gemIndices[SelectedDep].row(), QModelIndex())); + EXPECT_FALSE(m_proxyModel->filterAcceptsRow(m_gemIndices[Unselected].row(), QModelIndex())); + EXPECT_FALSE(m_proxyModel->filterAcceptsRow(m_gemIndices[UnselectedDep].row(), QModelIndex())); + EXPECT_TRUE(m_proxyModel->filterAcceptsRow(m_gemIndices[Active].row(), QModelIndex())); + EXPECT_FALSE(m_proxyModel->filterAcceptsRow(m_gemIndices[Inactive].row(), QModelIndex())); + } + + TEST_F(GemCatalogSelectedActiveFilterTests, GemCatalogFilters_FilterActive_ShowsInactive) + { + // Check inactive filter + m_proxyModel->SetGemActive(GemSortFilterProxyModel::GemActive::Inactive); + + EXPECT_FALSE(m_proxyModel->filterAcceptsRow(m_gemIndices[Selected].row(), QModelIndex())); + EXPECT_FALSE(m_proxyModel->filterAcceptsRow(m_gemIndices[SelectedDep].row(), QModelIndex())); + EXPECT_TRUE(m_proxyModel->filterAcceptsRow(m_gemIndices[Unselected].row(), QModelIndex())); + EXPECT_TRUE(m_proxyModel->filterAcceptsRow(m_gemIndices[UnselectedDep].row(), QModelIndex())); + EXPECT_FALSE(m_proxyModel->filterAcceptsRow(m_gemIndices[Active].row(), QModelIndex())); + EXPECT_TRUE(m_proxyModel->filterAcceptsRow(m_gemIndices[Inactive].row(), QModelIndex())); + } + + class GemCatalogMiscFilterTests + : public GemCatalogFilterTests + { + public: + void SetUp() override + { + GemCatalogFilterTests::SetUp(); + + GemInfo gemA, gemB, gemC; + + gemA.m_name = "Default Audio"; + gemB.m_name = "Mobile UX"; + gemC.m_name = "City Props"; + + gemA.m_gemOrigin = GemInfo::GemOrigin::Open3DEngine; + gemB.m_gemOrigin = GemInfo::GemOrigin::Local; + gemC.m_gemOrigin = GemInfo::GemOrigin::Remote; + + gemA.m_types = GemInfo::Type::Code; + gemB.m_types = GemInfo::Type::Code | GemInfo::Type::Tool; + gemC.m_types = GemInfo::Type::Asset; + + using Plat = GemInfo::Platform; + gemA.m_platforms = Plat::Windows; + gemB.m_platforms = Plat::Android | Plat::iOS; + gemC.m_platforms = Plat::Android | Plat::iOS | Plat::Linux | Plat::macOS | Plat::Windows; + + gemA.m_features = QStringList({ "Audio", "Framework", "SDK" }); + gemB.m_features = QStringList({ "Framework", "Tools", "UI" }); + gemC.m_features = QStringList({ "Assets", "Content", "Environment" }); + + m_gemRows.append(m_gemModel->AddGem(gemA).row()); + m_gemRows.append(m_gemModel->AddGem(gemB).row()); + m_gemRows.append(m_gemModel->AddGem(gemC).row()); + } + + protected: + enum RowOrder + { + DefaultAudio, + MobileUX, + CityProps + }; + + QVector m_gemRows; + }; + + TEST_F(GemCatalogMiscFilterTests, GemCatalogFilters_MiscNoFilter_ShowsAll) + { + // No filter + EXPECT_TRUE(m_proxyModel->filterAcceptsRow(m_gemRows[DefaultAudio], QModelIndex())); + EXPECT_TRUE(m_proxyModel->filterAcceptsRow(m_gemRows[MobileUX], QModelIndex())); + EXPECT_TRUE(m_proxyModel->filterAcceptsRow(m_gemRows[CityProps], QModelIndex())); + } + + TEST_F(GemCatalogMiscFilterTests, GemCatalogFilters_FilterSingleOrigin_ShowsOriginMatch) + { + m_proxyModel->SetGemOrigins(GemInfo::GemOrigin::Open3DEngine); + + EXPECT_TRUE(m_proxyModel->filterAcceptsRow(m_gemRows[DefaultAudio], QModelIndex())); + EXPECT_FALSE(m_proxyModel->filterAcceptsRow(m_gemRows[MobileUX], QModelIndex())); + EXPECT_FALSE(m_proxyModel->filterAcceptsRow(m_gemRows[CityProps], QModelIndex())); + + m_proxyModel->SetGemOrigins(GemInfo::GemOrigin::Local); + + EXPECT_FALSE(m_proxyModel->filterAcceptsRow(m_gemRows[DefaultAudio], QModelIndex())); + EXPECT_TRUE(m_proxyModel->filterAcceptsRow(m_gemRows[MobileUX], QModelIndex())); + EXPECT_FALSE(m_proxyModel->filterAcceptsRow(m_gemRows[CityProps], QModelIndex())); + + m_proxyModel->SetGemOrigins(GemInfo::GemOrigin::Remote); + + EXPECT_FALSE(m_proxyModel->filterAcceptsRow(m_gemRows[DefaultAudio], QModelIndex())); + EXPECT_FALSE(m_proxyModel->filterAcceptsRow(m_gemRows[MobileUX], QModelIndex())); + EXPECT_TRUE(m_proxyModel->filterAcceptsRow(m_gemRows[CityProps], QModelIndex())); + } + + TEST_F(GemCatalogMiscFilterTests, GemCatalogFilters_FilterMultipleOrigins_ShowsMultipleOriginMatches) + { + m_proxyModel->SetGemOrigins(GemInfo::GemOrigin::Open3DEngine | GemInfo::GemOrigin::Local); + + EXPECT_TRUE(m_proxyModel->filterAcceptsRow(m_gemRows[DefaultAudio], QModelIndex())); + EXPECT_TRUE(m_proxyModel->filterAcceptsRow(m_gemRows[MobileUX], QModelIndex())); + EXPECT_FALSE(m_proxyModel->filterAcceptsRow(m_gemRows[CityProps], QModelIndex())); + } + + TEST_F(GemCatalogMiscFilterTests, GemCatalogFilters_FilterSingleType_ShowsTypeMatch) + { + m_proxyModel->SetTypes(GemInfo::Type::Code); + + EXPECT_TRUE(m_proxyModel->filterAcceptsRow(m_gemRows[DefaultAudio], QModelIndex())); + EXPECT_TRUE(m_proxyModel->filterAcceptsRow(m_gemRows[MobileUX], QModelIndex())); + EXPECT_FALSE(m_proxyModel->filterAcceptsRow(m_gemRows[CityProps], QModelIndex())); + + m_proxyModel->SetTypes(GemInfo::Type::Tool); + + EXPECT_FALSE(m_proxyModel->filterAcceptsRow(m_gemRows[DefaultAudio], QModelIndex())); + EXPECT_TRUE(m_proxyModel->filterAcceptsRow(m_gemRows[MobileUX], QModelIndex())); + EXPECT_FALSE(m_proxyModel->filterAcceptsRow(m_gemRows[CityProps], QModelIndex())); + + m_proxyModel->SetTypes(GemInfo::Type::Asset); + + EXPECT_FALSE(m_proxyModel->filterAcceptsRow(m_gemRows[DefaultAudio], QModelIndex())); + EXPECT_FALSE(m_proxyModel->filterAcceptsRow(m_gemRows[MobileUX], QModelIndex())); + EXPECT_TRUE(m_proxyModel->filterAcceptsRow(m_gemRows[CityProps], QModelIndex())); + } + + TEST_F(GemCatalogMiscFilterTests, GemCatalogFilters_FilterMultipleTypes_ShowsMultipleTypeMatches) + { + m_proxyModel->SetTypes(GemInfo::Type::Tool | GemInfo::Type::Asset); + + EXPECT_FALSE(m_proxyModel->filterAcceptsRow(m_gemRows[DefaultAudio], QModelIndex())); + EXPECT_TRUE(m_proxyModel->filterAcceptsRow(m_gemRows[MobileUX], QModelIndex())); + EXPECT_TRUE(m_proxyModel->filterAcceptsRow(m_gemRows[CityProps], QModelIndex())); + } + + TEST_F(GemCatalogMiscFilterTests, GemCatalogFilters_FilterSinglePlatform_ShowsPlatformMatch) + { + m_proxyModel->SetPlatforms(GemInfo::Platform::Windows); + + EXPECT_TRUE(m_proxyModel->filterAcceptsRow(m_gemRows[DefaultAudio], QModelIndex())); + EXPECT_FALSE(m_proxyModel->filterAcceptsRow(m_gemRows[MobileUX], QModelIndex())); + EXPECT_TRUE(m_proxyModel->filterAcceptsRow(m_gemRows[CityProps], QModelIndex())); + + m_proxyModel->SetPlatforms(GemInfo::Platform::Android); + + EXPECT_FALSE(m_proxyModel->filterAcceptsRow(m_gemRows[DefaultAudio], QModelIndex())); + EXPECT_TRUE(m_proxyModel->filterAcceptsRow(m_gemRows[MobileUX], QModelIndex())); + EXPECT_TRUE(m_proxyModel->filterAcceptsRow(m_gemRows[CityProps], QModelIndex())); + + m_proxyModel->SetPlatforms(GemInfo::Platform::macOS); + + EXPECT_FALSE(m_proxyModel->filterAcceptsRow(m_gemRows[DefaultAudio], QModelIndex())); + EXPECT_FALSE(m_proxyModel->filterAcceptsRow(m_gemRows[MobileUX], QModelIndex())); + EXPECT_TRUE(m_proxyModel->filterAcceptsRow(m_gemRows[CityProps], QModelIndex())); + } + + TEST_F(GemCatalogMiscFilterTests, GemCatalogFilters_FilterMultiplePlatforms_ShowsMultiplePlatformMatches) + { + m_proxyModel->SetPlatforms(GemInfo::Platform::Android | GemInfo::Platform::iOS); + + EXPECT_FALSE(m_proxyModel->filterAcceptsRow(m_gemRows[DefaultAudio], QModelIndex())); + EXPECT_TRUE(m_proxyModel->filterAcceptsRow(m_gemRows[MobileUX], QModelIndex())); + EXPECT_TRUE(m_proxyModel->filterAcceptsRow(m_gemRows[CityProps], QModelIndex())); + } + + TEST_F(GemCatalogMiscFilterTests, GemCatalogFilters_FilterSingleFeature_ShowsFeatureMatch) + { + m_proxyModel->SetFeatures({ "Audio" }); + + EXPECT_TRUE(m_proxyModel->filterAcceptsRow(m_gemRows[DefaultAudio], QModelIndex())); + EXPECT_FALSE(m_proxyModel->filterAcceptsRow(m_gemRows[MobileUX], QModelIndex())); + EXPECT_FALSE(m_proxyModel->filterAcceptsRow(m_gemRows[CityProps], QModelIndex())); + + m_proxyModel->SetFeatures({ "Tools", }); + + EXPECT_FALSE(m_proxyModel->filterAcceptsRow(m_gemRows[DefaultAudio], QModelIndex())); + EXPECT_TRUE(m_proxyModel->filterAcceptsRow(m_gemRows[MobileUX], QModelIndex())); + EXPECT_FALSE(m_proxyModel->filterAcceptsRow(m_gemRows[CityProps], QModelIndex())); + + m_proxyModel->SetFeatures({ "Environment" }); + + EXPECT_FALSE(m_proxyModel->filterAcceptsRow(m_gemRows[DefaultAudio], QModelIndex())); + EXPECT_FALSE(m_proxyModel->filterAcceptsRow(m_gemRows[MobileUX], QModelIndex())); + EXPECT_TRUE(m_proxyModel->filterAcceptsRow(m_gemRows[CityProps], QModelIndex())); + } + + TEST_F(GemCatalogMiscFilterTests, GemCatalogFilters_FilterMultipleFeatures_ShowsMultipleFeatureMatches) + { + m_proxyModel->SetFeatures({ "Assets", "Framework" }); + + EXPECT_TRUE(m_proxyModel->filterAcceptsRow(m_gemRows[DefaultAudio], QModelIndex())); + EXPECT_TRUE(m_proxyModel->filterAcceptsRow(m_gemRows[MobileUX], QModelIndex())); + EXPECT_TRUE(m_proxyModel->filterAcceptsRow(m_gemRows[CityProps], QModelIndex())); + } + + TEST_F(GemCatalogMiscFilterTests, GemCatalogFilters_FilterPartialMatchFeature_ShowsNone) + { + // Features must be an exact match to filter by them directly + m_proxyModel->SetFeatures({ "Frame" }); + + EXPECT_FALSE(m_proxyModel->filterAcceptsRow(m_gemRows[DefaultAudio], QModelIndex())); + EXPECT_FALSE(m_proxyModel->filterAcceptsRow(m_gemRows[MobileUX], QModelIndex())); + EXPECT_FALSE(m_proxyModel->filterAcceptsRow(m_gemRows[CityProps], QModelIndex())); + } } From c492d644da2bc1fc0881a1408b32dade88b4b7f3 Mon Sep 17 00:00:00 2001 From: Nicholas Lawson <70027408+lawsonamzn@users.noreply.github.com> Date: Wed, 1 Dec 2021 08:18:39 -0800 Subject: [PATCH 9/9] Fixes #5909 hash file stats missing from AP stats log (#5913) The "begin and end" markers were removed due to a merge conflict. This restores them. It also stops printing out sections that are empty - for example, if the AP runs without processing anything, there will no longer be a "top 10 processed files" section. Signed-off-by: lawsonamzn <70027408+lawsonamzn@users.noreply.github.com> --- Code/Tools/AssetProcessor/native/utilities/StatsCapture.cpp | 6 ++++++ Code/Tools/AssetProcessor/native/utilities/assetUtils.cpp | 4 ++++ 2 files changed, 10 insertions(+) diff --git a/Code/Tools/AssetProcessor/native/utilities/StatsCapture.cpp b/Code/Tools/AssetProcessor/native/utilities/StatsCapture.cpp index 1174e2500c..f6b7c07a91 100644 --- a/Code/Tools/AssetProcessor/native/utilities/StatsCapture.cpp +++ b/Code/Tools/AssetProcessor/native/utilities/StatsCapture.cpp @@ -132,6 +132,12 @@ namespace AssetProcessor // calls PrintStat on each element in the vector. void PrintStatsArray(AZStd::vector& keys, int maxToPrint, const char* header) { + // don't print anything out at all, not even a header, if the keys are empty. + if (keys.empty()) + { + return; + } + if ((m_dumpHumanReadableStats)&&(header)) { AZ_TracePrintf(AssetProcessor::ConsoleChannel,"Top %i %s\n", maxToPrint, header); diff --git a/Code/Tools/AssetProcessor/native/utilities/assetUtils.cpp b/Code/Tools/AssetProcessor/native/utilities/assetUtils.cpp index ae70ee7fe5..6bcd0dec01 100644 --- a/Code/Tools/AssetProcessor/native/utilities/assetUtils.cpp +++ b/Code/Tools/AssetProcessor/native/utilities/assetUtils.cpp @@ -1182,7 +1182,11 @@ namespace AssetUtilities } } + // keep track of how much time we spend actually hashing files. + AZStd::string statName = AZStd::string::format("HashFile,%s", filePath); + AssetProcessor::StatsCapture::BeginCaptureStat(statName.c_str()); hash = AssetBuilderSDK::GetFileHash(filePath, bytesReadOut, hashMsDelay); + AssetProcessor::StatsCapture::EndCaptureStat(statName.c_str()); return hash; }