From ae2525564deb488d62b41675303c1268c7ff5dba Mon Sep 17 00:00:00 2001 From: pereslav Date: Wed, 22 Sep 2021 13:29:55 +0100 Subject: [PATCH 01/18] Updated NetworkSpawnableHolderComponent to use TransformBus instead of FindComponent. Added dependency on TransformService Signed-off-by: pereslav --- .../Pipeline/NetworkSpawnableHolderComponent.cpp | 15 ++++++++------- .../Pipeline/NetworkSpawnableHolderComponent.h | 5 +++-- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/Gems/Multiplayer/Code/Source/Pipeline/NetworkSpawnableHolderComponent.cpp b/Gems/Multiplayer/Code/Source/Pipeline/NetworkSpawnableHolderComponent.cpp index d33c3274c2..052b81de64 100644 --- a/Gems/Multiplayer/Code/Source/Pipeline/NetworkSpawnableHolderComponent.cpp +++ b/Gems/Multiplayer/Code/Source/Pipeline/NetworkSpawnableHolderComponent.cpp @@ -24,6 +24,13 @@ namespace Multiplayer } } + void NetworkSpawnableHolderComponent::GetDependentServices(AZ::ComponentDescriptor::DependencyArrayType& dependent) + { + // TransformService isn't strictly required in this component (Identity transform will be used by default) + // However we need to make sure if there's a component providing TransformService it is activated first. + dependent.push_back(AZ_CRC_CE("TransformService")); + } + NetworkSpawnableHolderComponent::NetworkSpawnableHolderComponent() { } @@ -37,13 +44,7 @@ namespace Multiplayer if(shouldSpawnNetEntities) { AZ::Transform rootEntityTransform = AZ::Transform::CreateIdentity(); - - AzFramework::TransformComponent* rootEntityTransformComponent = - GetEntity()->FindComponent(); - if (rootEntityTransformComponent) - { - rootEntityTransform = rootEntityTransformComponent->GetWorldTM(); - } + AZ::TransformBus::EventResult(rootEntityTransform, GetEntityId(), &AZ::TransformInterface::GetWorldTM); INetworkEntityManager* networkEntityManager = GetNetworkEntityManager(); AZ_Assert(networkEntityManager != nullptr, diff --git a/Gems/Multiplayer/Code/Source/Pipeline/NetworkSpawnableHolderComponent.h b/Gems/Multiplayer/Code/Source/Pipeline/NetworkSpawnableHolderComponent.h index 3836212c48..c95a1a5442 100644 --- a/Gems/Multiplayer/Code/Source/Pipeline/NetworkSpawnableHolderComponent.h +++ b/Gems/Multiplayer/Code/Source/Pipeline/NetworkSpawnableHolderComponent.h @@ -22,11 +22,12 @@ namespace Multiplayer public: AZ_COMPONENT(NetworkSpawnableHolderComponent, "{B0E3ADEE-FCB4-4A32-8D4F-6920F1CB08E4}"); - static void Reflect(AZ::ReflectContext* context); - NetworkSpawnableHolderComponent();; ~NetworkSpawnableHolderComponent() override = default; + static void Reflect(AZ::ReflectContext* context); + static void GetDependentServices(AZ::ComponentDescriptor::DependencyArrayType& dependent); + //! AZ::Component overrides. //! @{ void Activate() override; From 1e5e6094f5e53cdd94fa47ff58905cebbf8c700b Mon Sep 17 00:00:00 2001 From: amzn-tommy Date: Wed, 22 Sep 2021 15:46:29 -0700 Subject: [PATCH 02/18] Expose Transform::CreateLookAt to behavior context to use with hydra tests Signed-off-by: amzn-tommy --- Code/Framework/AzCore/AzCore/Math/Transform.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/Code/Framework/AzCore/AzCore/Math/Transform.cpp b/Code/Framework/AzCore/AzCore/Math/Transform.cpp index 5cb09fe9a8..1701820bae 100644 --- a/Code/Framework/AzCore/AzCore/Math/Transform.cpp +++ b/Code/Framework/AzCore/AzCore/Math/Transform.cpp @@ -353,6 +353,7 @@ namespace AZ Method("CreateFromMatrix3x3AndTranslation", &Transform::CreateFromMatrix3x3AndTranslation)-> Method("CreateUniformScale", &Transform::CreateUniformScale)-> Method("CreateTranslation", &Transform::CreateTranslation)-> + Method("CreateLookAt", &Transform::CreateLookAt)-> Method("ConstructFromValuesNumeric", &Internal::ConstructTransformFromValues); } } From c2c18f094bf22d9b7f5d14e680736377176d4eff Mon Sep 17 00:00:00 2001 From: Gene Walters Date: Thu, 23 Sep 2021 09:46:55 -0700 Subject: [PATCH 03/18] Exposing NetworkCharacterComponent::TryMoveWithVelocity to script. Updating Multiplayer AutoComponent baseclass behavior context to Reflect itself instead of its derived (human made) component. This is so the derived class can also create behaviorcontext classes of its own if needed. Misc copyright header edit. Signed-off-by: Gene Walters --- .../Components/NetworkCharacterComponent.h | 28 ++++++++++++++++++- .../Components/NetworkRigidBodyComponent.h | 3 +- .../Source/AutoGen/AutoComponent_Source.jinja | 2 +- .../Components/NetworkCharacterComponent.cpp | 19 +++++++++++-- .../Components/NetworkRigidBodyComponent.cpp | 3 +- 5 files changed, 48 insertions(+), 7 deletions(-) diff --git a/Gems/Multiplayer/Code/Include/Multiplayer/Components/NetworkCharacterComponent.h b/Gems/Multiplayer/Code/Include/Multiplayer/Components/NetworkCharacterComponent.h index 478c925299..511a29c683 100644 --- a/Gems/Multiplayer/Code/Include/Multiplayer/Components/NetworkCharacterComponent.h +++ b/Gems/Multiplayer/Code/Include/Multiplayer/Components/NetworkCharacterComponent.h @@ -19,6 +19,22 @@ namespace Physics namespace Multiplayer { + //! NetworkCharacterRequests + //! ComponentBus handled by NetworkCharacterComponentController. + //! Bus was created for exposing controller methods to script; C++ users should access the controller directly. + class NetworkCharacterRequests : public AZ::ComponentBus + { + public: + //! TryMoveWithVelocity + //! Will move this character entity kinematically through physical world while also ensuring the network stays in-sync. + //! Velocity will be applied over delta-time to determine the movement amount. + //! Returns this entity's world-space position after the move. + virtual AZ::Vector3 TryMoveWithVelocity(const AZ::Vector3& velocity, float deltaTime) = 0; + }; + + typedef AZ::EBus NetworkCharacterRequestBus; + + //! NetworkCharacterComponent //! Provides multiplayer support for game-play player characters. class NetworkCharacterComponent @@ -39,6 +55,12 @@ namespace Multiplayer incompatible.push_back(AZ_CRC_CE("NetworkRigidBodyService")); } + static void GetRequiredServices(AZ::ComponentDescriptor::DependencyArrayType& required) + { + NetworkCharacterComponentBase::GetRequiredServices(required); + required.push_back(AZ_CRC("PhysXCharacterControllerService", 0x428de4fa)); + } + // AZ::Component void OnInit() override {} void OnActivate(Multiplayer::EntityIsMigrating entityIsMigrating) override; @@ -65,18 +87,22 @@ namespace Multiplayer //! Class provides the ability to move characters in physical space while keeping the network in-sync. class NetworkCharacterComponentController : public NetworkCharacterComponentControllerBase + , private NetworkCharacterRequestBus::Handler { public: + AZ_RTTI(NetworkCharacterComponentController, "{C91851A2-8B95-4484-9F97-BFF9D1F528A0}") + static void Reflect(AZ::ReflectContext* context); NetworkCharacterComponentController(NetworkCharacterComponent& parent); // NetworkCharacterComponentControllerBase void OnActivate(Multiplayer::EntityIsMigrating entityIsMigrating) override; void OnDeactivate(Multiplayer::EntityIsMigrating entityIsMigrating) override; + // NetworkCharacterRequestBus::Handler //! TryMoveWithVelocity //! Will move this character entity kinematically through physical world while also ensuring the network stays in-sync. //! Velocity will be applied over delta-time to determine the movement amount. //! Returns this entity's world-space position after the move. - AZ::Vector3 TryMoveWithVelocity(const AZ::Vector3& velocity, float deltaTime); + AZ::Vector3 TryMoveWithVelocity(const AZ::Vector3& velocity, float deltaTime) override; }; } diff --git a/Gems/Multiplayer/Code/Include/Multiplayer/Components/NetworkRigidBodyComponent.h b/Gems/Multiplayer/Code/Include/Multiplayer/Components/NetworkRigidBodyComponent.h index 19379fc959..cec73b1b71 100644 --- a/Gems/Multiplayer/Code/Include/Multiplayer/Components/NetworkRigidBodyComponent.h +++ b/Gems/Multiplayer/Code/Include/Multiplayer/Components/NetworkRigidBodyComponent.h @@ -1,5 +1,6 @@ /* - * 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. + * 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 * diff --git a/Gems/Multiplayer/Code/Source/AutoGen/AutoComponent_Source.jinja b/Gems/Multiplayer/Code/Source/AutoGen/AutoComponent_Source.jinja index eb4b81ea96..9e43ccd3d6 100644 --- a/Gems/Multiplayer/Code/Source/AutoGen/AutoComponent_Source.jinja +++ b/Gems/Multiplayer/Code/Source/AutoGen/AutoComponent_Source.jinja @@ -1518,7 +1518,7 @@ namespace {{ Component.attrib['Namespace'] }} {{ ReflectRpcEventDescs(Component, ComponentName, 'Authority', 'Autonomous')|indent(4) -}} {{ ReflectRpcEventDescs(Component, ComponentName, 'Authority', 'Client')|indent(4) }} - behaviorContext->Class<{{ ComponentName }}>("{{ ComponentName }}") + behaviorContext->Class<{{ ComponentBaseName }}>("{{ ComponentBaseName }}") ->Attribute(AZ::Script::Attributes::Module, "{{ LowerFirst(Component.attrib['Namespace']) }}") ->Attribute(AZ::Script::Attributes::Category, "{{ UpperFirst(Component.attrib['Namespace']) }}") diff --git a/Gems/Multiplayer/Code/Source/Components/NetworkCharacterComponent.cpp b/Gems/Multiplayer/Code/Source/Components/NetworkCharacterComponent.cpp index b14fc8761f..c9b6e91adc 100644 --- a/Gems/Multiplayer/Code/Source/Components/NetworkCharacterComponent.cpp +++ b/Gems/Multiplayer/Code/Source/Components/NetworkCharacterComponent.cpp @@ -83,7 +83,7 @@ namespace Multiplayer return physx::PxQueryHitType::eNONE; } - void NetworkCharacterComponent::NetworkCharacterComponent::Reflect(AZ::ReflectContext* context) + void NetworkCharacterComponent::Reflect(AZ::ReflectContext* context) { AZ::SerializeContext* serializeContext = azrtti_cast(context); if (serializeContext) @@ -92,6 +92,7 @@ namespace Multiplayer ->Version(1); } NetworkCharacterComponentBase::Reflect(context); + NetworkCharacterComponentController::Reflect(context); } NetworkCharacterComponent::NetworkCharacterComponent() @@ -161,6 +162,18 @@ namespace Multiplayer return state.touchedActor != nullptr || (state.collisionFlags & physx::PxControllerCollisionFlag::eCOLLISION_DOWN) != 0; } + void NetworkCharacterComponentController::Reflect(AZ::ReflectContext* context) + { + if (AZ::BehaviorContext* behaviorContext = azrtti_cast(context)) + { + behaviorContext->EBus("NetworkCharacterRequestBus") + ->Event("TryMoveWithVelocity", &NetworkCharacterRequestBus::Events::TryMoveWithVelocity); + + behaviorContext->Class("NetworkCharacterComponentController") + ->RequestBus("NetworkCharacterRequestBus"); + } + } + NetworkCharacterComponentController::NetworkCharacterComponentController(NetworkCharacterComponent& parent) : NetworkCharacterComponentControllerBase(parent) { @@ -169,12 +182,12 @@ namespace Multiplayer void NetworkCharacterComponentController::OnActivate([[maybe_unused]] Multiplayer::EntityIsMigrating entityIsMigrating) { - ; + NetworkCharacterRequestBus::Handler::BusConnect(GetEntity()->GetId()); } void NetworkCharacterComponentController::OnDeactivate([[maybe_unused]] Multiplayer::EntityIsMigrating entityIsMigrating) { - ; + NetworkCharacterRequestBus::Handler::BusDisconnect(GetEntity()->GetId()); } AZ::Vector3 NetworkCharacterComponentController::TryMoveWithVelocity(const AZ::Vector3& velocity, [[maybe_unused]] float deltaTime) diff --git a/Gems/Multiplayer/Code/Source/Components/NetworkRigidBodyComponent.cpp b/Gems/Multiplayer/Code/Source/Components/NetworkRigidBodyComponent.cpp index bcf855d834..725ebc024c 100644 --- a/Gems/Multiplayer/Code/Source/Components/NetworkRigidBodyComponent.cpp +++ b/Gems/Multiplayer/Code/Source/Components/NetworkRigidBodyComponent.cpp @@ -1,5 +1,6 @@ /* - * 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. + * 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 cd85df35b65cd274c14ff43ba947f151ff5bc672 Mon Sep 17 00:00:00 2001 From: Gene Walters Date: Thu, 23 Sep 2021 12:45:44 -0700 Subject: [PATCH 04/18] Make sure Multiplayer AutoComponents dont generate property OnChange script events if GenerateEventBindings is disabled Signed-off-by: Gene Walters --- Gems/Multiplayer/Code/Source/AutoGen/AutoComponent_Source.jinja | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Gems/Multiplayer/Code/Source/AutoGen/AutoComponent_Source.jinja b/Gems/Multiplayer/Code/Source/AutoGen/AutoComponent_Source.jinja index 9e43ccd3d6..cf62f6f901 100644 --- a/Gems/Multiplayer/Code/Source/AutoGen/AutoComponent_Source.jinja +++ b/Gems/Multiplayer/Code/Source/AutoGen/AutoComponent_Source.jinja @@ -909,6 +909,7 @@ enum class NetworkProperties controller->Set{{ UpperFirst(Property.attrib['Name']) }}({{ LowerFirst(Property.attrib['Name']) }}); {% endif %} }) +{% if Property.attrib['GenerateEventBindings']|booleanTrue %} {% if Property.attrib['Container'] == 'Vector' or Property.attrib['Container'] == 'Array' -%} ->Method("GetOn{{ UpperFirst(Property.attrib['Name']) }}ChangedEvent", [](AZ::EntityId id) -> AZ::Event* {% else %} @@ -936,6 +937,7 @@ enum class NetworkProperties {% else %} ->Attribute(AZ::Script::Attributes::AzEventDescription, AZ::BehaviorAzEventDescription{ "On {{ UpperFirst(Property.attrib['Name']) }} Changed Event", {"New {{ Property.attrib['Type'] }}"} }) {% endif %} +{% endif %} {% endif %} {% endcall %} From f142e30da6f2bcf32c37935cc754545d6d9a02a6 Mon Sep 17 00:00:00 2001 From: puvvadar Date: Thu, 23 Sep 2021 12:56:46 -0700 Subject: [PATCH 05/18] Fix AutoGen of RPCs with no params Signed-off-by: puvvadar --- .../Code/Source/AutoGen/AutoComponent_Common.jinja | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/Gems/Multiplayer/Code/Source/AutoGen/AutoComponent_Common.jinja b/Gems/Multiplayer/Code/Source/AutoGen/AutoComponent_Common.jinja index 5bf0a4823f..02ab556cea 100644 --- a/Gems/Multiplayer/Code/Source/AutoGen/AutoComponent_Common.jinja +++ b/Gems/Multiplayer/Code/Source/AutoGen/AutoComponent_Common.jinja @@ -170,12 +170,20 @@ AZ::Event<{{ Property.attrib['Type'] }}> {% set PropertyName = UpperFirst(Property.attrib['Name']) %} {{ ParseRpcParams(Property, paramNames, paramTypes, paramDefines) }} {% if IsOverride %} +{% if paramDefines|count > 0 %} void Handle{{ PropertyName }}(AzNetworking::IConnection* invokingConnection, {{ ', '.join(paramDefines) }}) override {} +{% else %} +void Handle{{ PropertyName }}(AzNetworking::IConnection* invokingConnection) override {} +{% endif %} {% else %} //! {{ PropertyName }} Handler //! {{ Property.attrib['Description'] }} //! HandleOn {{ HandleOn }} +{% if paramDefines|count > 0 %} virtual void Handle{{ PropertyName }}([[maybe_unused]] AzNetworking::IConnection* invokingConnection, [[maybe_unused]] {{ ', [[maybe_unused]] '.join(paramDefines) }}) {} +{% else %} +virtual void Handle{{ PropertyName }}([[maybe_unused]] AzNetworking::IConnection* invokingConnection) {} +{% endif %} {% endif %} {% endmacro %} {# From a44af20f9a7091f1cbc1806e3cd9a24ce1361971 Mon Sep 17 00:00:00 2001 From: Gene Walters Date: Thu, 23 Sep 2021 13:01:34 -0700 Subject: [PATCH 06/18] Adding parameter names to NetworkCharacterComponent script events so people know what the parameters are used for in scriptcanvas Signed-off-by: Gene Walters --- .../Code/Source/Components/NetworkCharacterComponent.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gems/Multiplayer/Code/Source/Components/NetworkCharacterComponent.cpp b/Gems/Multiplayer/Code/Source/Components/NetworkCharacterComponent.cpp index c9b6e91adc..33eb26653a 100644 --- a/Gems/Multiplayer/Code/Source/Components/NetworkCharacterComponent.cpp +++ b/Gems/Multiplayer/Code/Source/Components/NetworkCharacterComponent.cpp @@ -167,7 +167,7 @@ namespace Multiplayer if (AZ::BehaviorContext* behaviorContext = azrtti_cast(context)) { behaviorContext->EBus("NetworkCharacterRequestBus") - ->Event("TryMoveWithVelocity", &NetworkCharacterRequestBus::Events::TryMoveWithVelocity); + ->Event("TryMoveWithVelocity", &NetworkCharacterRequestBus::Events::TryMoveWithVelocity, {{ { "Velocity" }, { "DeltaTime" } }}); behaviorContext->Class("NetworkCharacterComponentController") ->RequestBus("NetworkCharacterRequestBus"); From c91b0f9d31863e6a55504c71a866a85847ba0cec Mon Sep 17 00:00:00 2001 From: Gene Walters Date: Thu, 23 Sep 2021 13:29:04 -0700 Subject: [PATCH 07/18] Small fix for color node tooltip to ask for values 0-1 instead of 0-255 Signed-off-by: Gene Walters --- Assets/Editor/Translation/scriptcanvas_en_us.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Assets/Editor/Translation/scriptcanvas_en_us.ts b/Assets/Editor/Translation/scriptcanvas_en_us.ts index 937f6a4d96..7b8361c879 100644 --- a/Assets/Editor/Translation/scriptcanvas_en_us.ts +++ b/Assets/Editor/Translation/scriptcanvas_en_us.ts @@ -8098,7 +8098,7 @@ COLOR_FROMVALUES_PARAM0_TOOLTIP - The Red value of hte Color [0, 255] + The Red value of the Color [0.0-1.0] COLOR_FROMVALUES_PARAM1_NAME @@ -8107,7 +8107,7 @@ COLOR_FROMVALUES_PARAM1_TOOLTIP - The Green value of the Color [0, 255] + The Green value of the Color [0.0-1.0] COLOR_FROMVALUES_PARAM2_NAME @@ -8116,7 +8116,7 @@ COLOR_FROMVALUES_PARAM2_TOOLTIP - The Blue value of the Color [0, 255] + The Blue value of the Color [0.0-1.0] COLOR_FROMVALUES_PARAM3_NAME @@ -8125,7 +8125,7 @@ COLOR_FROMVALUES_PARAM3_TOOLTIP - The Alpha value of the Color [0, 255] + The Alpha value of the Color [0.0-1.0] From 2dc7b517d14856b852fe2996c9314fe177579ce0 Mon Sep 17 00:00:00 2001 From: dmcdiar Date: Thu, 23 Sep 2021 13:49:10 -0700 Subject: [PATCH 08/18] Cached BLAS objects at the sub-mesh level Signed-off-by: dmcdiar --- .../RayTracing/RayTracingFeatureProcessor.cpp | 44 ++++++++++++------- .../RayTracing/RayTracingFeatureProcessor.h | 13 ++++-- 2 files changed, 38 insertions(+), 19 deletions(-) diff --git a/Gems/Atom/Feature/Common/Code/Source/RayTracing/RayTracingFeatureProcessor.cpp b/Gems/Atom/Feature/Common/Code/Source/RayTracing/RayTracingFeatureProcessor.cpp index a17ecaf1aa..520ae5c0e3 100644 --- a/Gems/Atom/Feature/Common/Code/Source/RayTracing/RayTracingFeatureProcessor.cpp +++ b/Gems/Atom/Feature/Common/Code/Source/RayTracing/RayTracingFeatureProcessor.cpp @@ -105,14 +105,31 @@ namespace AZ m_meshes[objectIndex].m_subMeshes = subMeshes; } + Mesh& mesh = m_meshes[objectIndex]; + + // search for an existing BLAS instance entry for this mesh using the assetId + BlasInstanceMap::iterator itMeshBlasInstance = m_blasInstanceMap.find(assetId); + if (itMeshBlasInstance == m_blasInstanceMap.end()) + { + // make a new BLAS map entry for this mesh + MeshBlasInstance meshBlasInstance; + meshBlasInstance.m_count = 1; + meshBlasInstance.m_subMeshes.reserve(mesh.m_subMeshes.size()); + itMeshBlasInstance = m_blasInstanceMap.insert({ assetId, meshBlasInstance }).first; + } + else + { + itMeshBlasInstance->second.m_count++; + } + // create the BLAS buffers for each sub-mesh, or re-use existing BLAS objects if they were already created. // Note: all sub-meshes must either create new BLAS objects or re-use existing ones, otherwise it's an error (it's the same model in both cases) // Note: the buffer is just reserved here, the BLAS is built in the RayTracingAccelerationStructurePass - Mesh& mesh = m_meshes[objectIndex]; bool blasInstanceFound = false; - - for (auto& subMesh : mesh.m_subMeshes) + for (uint32_t subMeshIndex = 0; subMeshIndex < mesh.m_subMeshes.size(); ++subMeshIndex) { + SubMesh& subMesh = mesh.m_subMeshes[subMeshIndex]; + RHI::RayTracingBlasDescriptor blasDescriptor; blasDescriptor.Build() ->Geometry() @@ -121,13 +138,11 @@ namespace AZ ->IndexBuffer(subMesh.m_indexBufferView) ; - // search for an existing BLAS object for this model - RayTracingBlasMap::iterator itBlas = m_blasMap.find(assetId); - if (itBlas != m_blasMap.end()) + // determine if we have an existing BLAS object for this subMesh + if (itMeshBlasInstance->second.m_subMeshes.size() >= subMeshIndex + 1) { // re-use existing BLAS - subMesh.m_blas = itBlas->second.m_blas; - itBlas->second.m_count++; + subMesh.m_blas = itMeshBlasInstance->second.m_subMeshes[subMeshIndex].m_blas; // keep track of the fact that we re-used a BLAS blasInstanceFound = true; @@ -143,8 +158,7 @@ namespace AZ subMesh.m_blas->CreateBuffers(*device, &blasDescriptor, *m_bufferPools); // store the BLAS in the side list - RayTracingBlasInstance blasInstance = { subMesh.m_blas, 1 }; - m_blasMap.insert({ assetId, blasInstance }); + itMeshBlasInstance->second.m_subMeshes.push_back({ subMesh.m_blas }); } } @@ -182,16 +196,16 @@ namespace AZ m_meshes.erase(itMesh); m_revision++; - // decrement the count from the BLAS instance, and check to see if we can remove it - RayTracingBlasMap::iterator itBlas = m_blasMap.find(itMesh->second.m_assetId); - if (itBlas != m_blasMap.end()) + // decrement the count from the BLAS instances, and check to see if we can remove them + BlasInstanceMap::iterator itBlas = m_blasInstanceMap.find(itMesh->second.m_assetId); + if (itBlas != m_blasInstanceMap.end()) { itBlas->second.m_count--; if (itBlas->second.m_count == 0) { - m_blasMap.erase(itBlas); + m_blasInstanceMap.erase(itBlas); } - } + } } m_meshInfoBufferNeedsUpdate = true; diff --git a/Gems/Atom/Feature/Common/Code/Source/RayTracing/RayTracingFeatureProcessor.h b/Gems/Atom/Feature/Common/Code/Source/RayTracing/RayTracingFeatureProcessor.h index be75f0fac9..52bb67f547 100644 --- a/Gems/Atom/Feature/Common/Code/Source/RayTracing/RayTracingFeatureProcessor.h +++ b/Gems/Atom/Feature/Common/Code/Source/RayTracing/RayTracingFeatureProcessor.h @@ -268,14 +268,19 @@ namespace AZ bool m_materialInfoBufferNeedsUpdate = false; // side list for looking up existing BLAS objects so they can be re-used when the same mesh is added multiple times - struct RayTracingBlasInstance + struct SubMeshBlasInstance { RHI::Ptr m_blas; - uint32_t m_count = 0; }; - using RayTracingBlasMap = AZStd::unordered_map; - RayTracingBlasMap m_blasMap; + struct MeshBlasInstance + { + uint32_t m_count = 0; + AZStd::vector m_subMeshes; + }; + + using BlasInstanceMap = AZStd::unordered_map; + BlasInstanceMap m_blasInstanceMap; }; } } From 94dfc508ea624ae275cb1b6a6296912ee51f7960 Mon Sep 17 00:00:00 2001 From: Benjamin Jillich <43751992+amzn-jillich@users.noreply.github.com> Date: Fri, 24 Sep 2021 09:14:47 +0200 Subject: [PATCH 09/18] EMotion FX: Asset Processor generates an Actor for every FBX (#4284) Fixing the actor behavior to ignore actor asset generation for files that only contain nodes/bones. We're only checking for blend shapes and skins now. In case neither is present in an FBX, we don't export the actor asset. This makes a plain skeleton a special case where users manually need to add the actor group for the fbx. The most common use case for these are animation files anyway, where we don't want to export actor assets. Also increased the version number of the actor group so that they are all getting reprocessed and the unneeded actor assets get removed. Signed-off-by: Benjamin Jillich --- .../Behaviors/ActorGroupBehavior.cpp | 17 ++++------------- .../Pipeline/SceneAPIExt/Groups/ActorGroup.cpp | 2 +- 2 files changed, 5 insertions(+), 14 deletions(-) diff --git a/Gems/EMotionFX/Code/EMotionFX/Pipeline/SceneAPIExt/Behaviors/ActorGroupBehavior.cpp b/Gems/EMotionFX/Code/EMotionFX/Pipeline/SceneAPIExt/Behaviors/ActorGroupBehavior.cpp index f1f1361585..d89e41f1a6 100644 --- a/Gems/EMotionFX/Code/EMotionFX/Pipeline/SceneAPIExt/Behaviors/ActorGroupBehavior.cpp +++ b/Gems/EMotionFX/Code/EMotionFX/Pipeline/SceneAPIExt/Behaviors/ActorGroupBehavior.cpp @@ -176,19 +176,10 @@ namespace EMotionFX return AZ::SceneAPI::Events::ProcessingResult::Ignored; } - const bool hasBoneData = AZ::SceneAPI::Utilities::DoesSceneGraphContainDataLike(scene, true); + // Skip adding the actor group if it doesn't contain any skin and blendshape data. const bool hasSkinData = AZ::SceneAPI::Utilities::DoesSceneGraphContainDataLike(scene, true); - const bool hasBlendShapeData = - AZ::SceneAPI::Utilities::DoesSceneGraphContainDataLike(scene, true); - // Skip adding the actor group if it doesn't contain any bone, skin and blendshape data. - if (!hasBoneData && !hasSkinData && !hasBlendShapeData) - { - return AZ::SceneAPI::Events::ProcessingResult::Ignored; - } - - const bool hasAnimationData = AZ::SceneAPI::Utilities::DoesSceneGraphContainDataLike(scene, true); - // Skip adding the actor group if it contains animation data but doesn't contain any skin or blendshape data. - if (hasAnimationData && !hasSkinData && !hasBlendShapeData) + const bool hasBlendShapeData = AZ::SceneAPI::Utilities::DoesSceneGraphContainDataLike(scene, true); + if (!hasSkinData && !hasBlendShapeData) { return AZ::SceneAPI::Events::ProcessingResult::Ignored; } @@ -197,7 +188,7 @@ namespace EMotionFX AZStd::shared_ptr group = AZStd::make_shared(); // This is a group that's generated automatically so may not be saved to disk but would need to be recreated - // in the same way again. To guarantee the same uuid, generate a stable one instead. + // in the same way again. To guarantee the same uuid, generate a stable one instead. group->OverrideId(AZ::SceneAPI::DataTypes::Utilities::CreateStableUuid(scene, Group::ActorGroup::TYPEINFO_Uuid())); EBUS_EVENT(AZ::SceneAPI::Events::ManifestMetaInfoBus, InitializeObject, scene, *group); diff --git a/Gems/EMotionFX/Code/EMotionFX/Pipeline/SceneAPIExt/Groups/ActorGroup.cpp b/Gems/EMotionFX/Code/EMotionFX/Pipeline/SceneAPIExt/Groups/ActorGroup.cpp index 9e080fca3b..90c25c575f 100644 --- a/Gems/EMotionFX/Code/EMotionFX/Pipeline/SceneAPIExt/Groups/ActorGroup.cpp +++ b/Gems/EMotionFX/Code/EMotionFX/Pipeline/SceneAPIExt/Groups/ActorGroup.cpp @@ -108,7 +108,7 @@ namespace EMotionFX serializeContext->Class()->Version(3, IActorGroupVersionConverter); - serializeContext->Class()->Version(6, ActorVersionConverter) + serializeContext->Class()->Version(7, ActorVersionConverter) ->Field("name", &ActorGroup::m_name) ->Field("selectedRootBone", &ActorGroup::m_selectedRootBone) ->Field("id", &ActorGroup::m_id) From 4c0796f11efc4f40c718987e95f1d3e3ac63e48a Mon Sep 17 00:00:00 2001 From: Benjamin Jillich <43751992+amzn-jillich@users.noreply.github.com> Date: Fri, 24 Sep 2021 09:14:55 +0200 Subject: [PATCH 10/18] EMotion FX: Crash in the actor builder when fbx meta data contains node groups (#4283) Fixing a crash related to the small array conversion that made multiple assets crash the actor builder. Signed-off-by: Benjamin Jillich --- Gems/EMotionFX/Code/EMotionFX/Source/NodeGroup.cpp | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/Gems/EMotionFX/Code/EMotionFX/Source/NodeGroup.cpp b/Gems/EMotionFX/Code/EMotionFX/Source/NodeGroup.cpp index 74e59150e0..8eae2fb127 100644 --- a/Gems/EMotionFX/Code/EMotionFX/Source/NodeGroup.cpp +++ b/Gems/EMotionFX/Code/EMotionFX/Source/NodeGroup.cpp @@ -6,12 +6,10 @@ * */ -// include the required headers #include "NodeGroup.h" #include "ActorInstance.h" #include - namespace EMotionFX { AZ_CLASS_ALLOCATOR_IMPL(NodeGroup, NodeAllocator, 0) @@ -104,10 +102,7 @@ namespace EMotionFX // remove a given node by its node number void NodeGroup::RemoveNodeByNodeIndex(uint16 nodeIndex) { - if (const auto found = AZStd::find(begin(m_nodes), end(m_nodes), nodeIndex); found) - { - m_nodes.erase(found); - } + m_nodes.erase(AZStd::remove(m_nodes.begin(), m_nodes.end(), nodeIndex), m_nodes.end()); } From 4d11025d64bc9746b76e908b4c1c6d82ea4af175 Mon Sep 17 00:00:00 2001 From: pereslav Date: Fri, 24 Sep 2021 13:00:40 +0100 Subject: [PATCH 11/18] Fixed Entity::GetTransform to work for the components of an activating entity. Made the cached transform lazy evaluated. Signed-off-by: pereslav --- .../AzCore/AzCore/Component/Entity.cpp | 19 +++++++++++++------ .../AzCore/AzCore/Component/Entity.h | 9 ++++----- 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/Code/Framework/AzCore/AzCore/Component/Entity.cpp b/Code/Framework/AzCore/AzCore/Component/Entity.cpp index 00c1895261..2e92873238 100644 --- a/Code/Framework/AzCore/AzCore/Component/Entity.cpp +++ b/Code/Framework/AzCore/AzCore/Component/Entity.cpp @@ -207,12 +207,6 @@ namespace AZ ActivateComponent(**it); } - // Cache the transform interface to the transform interface - // Generally this pattern is not recommended unless for component event buses - // As we have a guarantee (by design) that components can't change during active state) - // Even though technically they can connect disconnect from the bus. - m_transform = TransformBus::FindFirstHandler(m_id); - SetState(State::Active); EBUS_EVENT_ID(m_id, EntityBus, OnEntityActivated, m_id); @@ -1320,6 +1314,19 @@ namespace AZ return *processSignature; } + AZ::TransformInterface* Entity::GetTransform() const + { + // Lazy evaluation of the cached entity transform. + if(!m_transform) + { + // Generally this pattern is not recommended unless for component event buses + // As we have a guarantee (by design) that components can't change during active state) + // Even though technically they can connect disconnect from the bus. + m_transform = TransformBus::FindFirstHandler(m_id); + } + return m_transform; + } + //========================================================================= // MakeId // Ids must be unique across a project at authoring time. Runtime doesn't matter diff --git a/Code/Framework/AzCore/AzCore/Component/Entity.h b/Code/Framework/AzCore/AzCore/Component/Entity.h index 7ec63a56ac..356533f268 100644 --- a/Code/Framework/AzCore/AzCore/Component/Entity.h +++ b/Code/Framework/AzCore/AzCore/Component/Entity.h @@ -354,10 +354,9 @@ namespace AZ //! @return The Process Signature of the local machine. static AZ::u32 GetProcessSignature(); - /// @cond EXCLUDE_DOCS - //! @deprecated Use the TransformBus to communicate with the TransformInterface. - inline TransformInterface* GetTransform() const { return m_transform; } - /// @endcond + //! Gets the TransformInterface for the entity. + //! @return The TransformInterface for the entity. + TransformInterface* GetTransform() const; //! Sorts an entity's components based on the dependencies between components. //! If all dependencies are met, the required services can be activated @@ -406,7 +405,7 @@ namespace AZ //! A cached pointer to the transform interface. //! We recommend using AZ::TransformBus and caching locally instead of accessing //! the transform interface directly through this pointer. - TransformInterface* m_transform; + mutable TransformInterface* m_transform; //! A user-friendly name for the entity. This makes error messages easier to read. AZStd::string m_name; From 44c8ae28727d0713a00d6ff1dd2e389124859adb Mon Sep 17 00:00:00 2001 From: pereslav Date: Fri, 24 Sep 2021 13:03:02 +0100 Subject: [PATCH 12/18] Changed NetworkSpawnableHolderComponent to use Entity::GetTransform instead of an ebus call Signed-off-by: pereslav --- .../Source/Pipeline/NetworkSpawnableHolderComponent.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/Gems/Multiplayer/Code/Source/Pipeline/NetworkSpawnableHolderComponent.cpp b/Gems/Multiplayer/Code/Source/Pipeline/NetworkSpawnableHolderComponent.cpp index 052b81de64..71019a5270 100644 --- a/Gems/Multiplayer/Code/Source/Pipeline/NetworkSpawnableHolderComponent.cpp +++ b/Gems/Multiplayer/Code/Source/Pipeline/NetworkSpawnableHolderComponent.cpp @@ -44,7 +44,12 @@ namespace Multiplayer if(shouldSpawnNetEntities) { AZ::Transform rootEntityTransform = AZ::Transform::CreateIdentity(); - AZ::TransformBus::EventResult(rootEntityTransform, GetEntityId(), &AZ::TransformInterface::GetWorldTM); + + auto* transformInterface = GetEntity()->GetTransform(); + if(transformInterface) + { + rootEntityTransform = transformInterface->GetWorldTM(); + } INetworkEntityManager* networkEntityManager = GetNetworkEntityManager(); AZ_Assert(networkEntityManager != nullptr, From 43244d30e10c4ac56c8ae49a8f78f3d200e77fc5 Mon Sep 17 00:00:00 2001 From: Alex Peterson <26804013+AMZN-alexpete@users.noreply.github.com> Date: Fri, 24 Sep 2021 09:01:17 -0700 Subject: [PATCH 13/18] Project Manager Gem Dependencies (#4132) * Fix engine API change and add gem dependencies Signed-off-by: AMZN-alexpete <26804013+AMZN-alexpete@users.noreply.github.com> * Add GemCatalog dependency test Signed-off-by: AMZN-alexpete <26804013+AMZN-alexpete@users.noreply.github.com> * Clarify display name and fix missing const Signed-off-by: AMZN-alexpete <26804013+AMZN-alexpete@users.noreply.github.com> * Moving a couple helper functions into private scope Signed-off-by: AMZN-alexpete <26804013+AMZN-alexpete@users.noreply.github.com> * Update gem count when unselecting a gem #4074 This addresses the following issue https://github.com/o3de/o3de/issues/4074 Signed-off-by: AMZN-alexpete <26804013+AMZN-alexpete@users.noreply.github.com> * Active/Inactive filter and dependency tooltips Signed-off-by: AMZN-alexpete <26804013+AMZN-alexpete@users.noreply.github.com> * Accessors for previously added and dependencies Signed-off-by: AMZN-alexpete <26804013+AMZN-alexpete@users.noreply.github.com> * Cart displays gem dependency changes Signed-off-by: AMZN-alexpete <26804013+AMZN-alexpete@users.noreply.github.com> * Shorten titles to fit in summary popup Signed-off-by: AMZN-alexpete <26804013+AMZN-alexpete@users.noreply.github.com> * Remove QString::number Signed-off-by: AMZN-alexpete <26804013+AMZN-alexpete@users.noreply.github.com> * Remove extra space Signed-off-by: AMZN-alexpete <26804013+AMZN-alexpete@users.noreply.github.com> * Consolidate source model accesor helpers Signed-off-by: AMZN-alexpete <26804013+AMZN-alexpete@users.noreply.github.com> * Addressing minor feedback Signed-off-by: AMZN-alexpete <26804013+AMZN-alexpete@users.noreply.github.com> * Remove unused local variable Signed-off-by: AMZN-alexpete <26804013+AMZN-alexpete@users.noreply.github.com> --- .../GemCatalog/GemCatalogHeaderWidget.cpp | 162 +++++++----- .../GemCatalog/GemCatalogHeaderWidget.h | 14 +- .../Source/GemCatalog/GemCatalogScreen.cpp | 2 + .../Source/GemCatalog/GemFilterWidget.cpp | 92 ++++--- .../Source/GemCatalog/GemInfo.h | 3 +- .../Source/GemCatalog/GemInspector.cpp | 9 +- .../Source/GemCatalog/GemInspector.h | 1 - .../Source/GemCatalog/GemItemDelegate.cpp | 81 +++++- .../Source/GemCatalog/GemItemDelegate.h | 5 +- .../Source/GemCatalog/GemListHeaderWidget.cpp | 9 +- .../Source/GemCatalog/GemListView.cpp | 20 ++ .../Source/GemCatalog/GemModel.cpp | 250 +++++++++++++++--- .../Source/GemCatalog/GemModel.h | 35 ++- .../GemCatalog/GemSortFilterProxyModel.cpp | 38 ++- .../GemCatalog/GemSortFilterProxyModel.h | 21 +- .../ProjectManager/Source/PythonBindings.cpp | 8 + .../project_manager_tests_files.cmake | 1 + .../ProjectManager/tests/GemCatalogTests.cpp | 64 +++++ 18 files changed, 627 insertions(+), 188 deletions(-) create mode 100644 Code/Tools/ProjectManager/tests/GemCatalogTests.cpp diff --git a/Code/Tools/ProjectManager/Source/GemCatalog/GemCatalogHeaderWidget.cpp b/Code/Tools/ProjectManager/Source/GemCatalog/GemCatalogHeaderWidget.cpp index 909cd93cda..9fca6040d4 100644 --- a/Code/Tools/ProjectManager/Source/GemCatalog/GemCatalogHeaderWidget.cpp +++ b/Code/Tools/ProjectManager/Source/GemCatalog/GemCatalogHeaderWidget.cpp @@ -7,6 +7,7 @@ */ #include +#include #include #include #include @@ -23,7 +24,7 @@ namespace O3DE::ProjectManager m_layout = new QVBoxLayout(); m_layout->setSpacing(0); - m_layout->setMargin(0); + m_layout->setMargin(5); m_layout->setAlignment(Qt::AlignTop); setLayout(m_layout); @@ -41,74 +42,111 @@ namespace O3DE::ProjectManager hLayout->addWidget(closeButton); m_layout->addLayout(hLayout); - // enabled - { - m_enabledWidget = new QWidget(); - m_enabledWidget->setFixedWidth(s_width); - m_layout->addWidget(m_enabledWidget); + // added + CreateGemSection( tr("Gem to be activated"), tr("Gems to be activated"), [=] + { + QVector gems; + const QVector toBeAdded = m_gemModel->GatherGemsToBeAdded(/*includeDependencies=*/false); - QVBoxLayout* layout = new QVBoxLayout(); - layout->setAlignment(Qt::AlignTop); - m_enabledWidget->setLayout(layout); + // don't include gems that were already active because they were dependencies + for (const QModelIndex& modelIndex : toBeAdded) + { + if (!GemModel::WasPreviouslyAddedDependency(modelIndex)) + { + gems.push_back(modelIndex); + } + } + return gems; + }); - m_enabledLabel = new QLabel(); - m_enabledLabel->setObjectName("GemCatalogCartOverlaySectionLabel"); - layout->addWidget(m_enabledLabel); - m_enabledTagContainer = new TagContainerWidget(); - layout->addWidget(m_enabledTagContainer); - } + // removed + CreateGemSection( tr("Gem to be deactivated"), tr("Gems to be deactivated"), [=] + { + QVector gems; + const QVector toBeAdded = m_gemModel->GatherGemsToBeRemoved(/*includeDependencies=*/false); - // disabled - { - m_disabledWidget = new QWidget(); - m_disabledWidget->setFixedWidth(s_width); - m_layout->addWidget(m_disabledWidget); + // don't include gems that are still active because they are dependencies + for (const QModelIndex& modelIndex : toBeAdded) + { + if (!GemModel::IsAddedDependency(modelIndex)) + { + gems.push_back(modelIndex); + } + } + return gems; + }); - QVBoxLayout* layout = new QVBoxLayout(); - layout->setAlignment(Qt::AlignTop); - m_disabledWidget->setLayout(layout); + // added dependencies + CreateGemSection( tr("Dependency to be activated"), tr("Dependencies to be activated"), [=] + { + QVector dependencies; + const QVector toBeAdded = m_gemModel->GatherGemsToBeAdded(/*includeDependencies=*/true); - m_disabledLabel = new QLabel(); - m_disabledLabel->setObjectName("GemCatalogCartOverlaySectionLabel"); - layout->addWidget(m_disabledLabel); - m_disabledTagContainer = new TagContainerWidget(); - layout->addWidget(m_disabledTagContainer); - } + // only include gems that are dependencies and not explicitly added + for (const QModelIndex& modelIndex : toBeAdded) + { + if (GemModel::IsAddedDependency(modelIndex) && !GemModel::IsAdded(modelIndex)) + { + dependencies.push_back(modelIndex); + } + } + return dependencies; + }); + + // removed dependencies + CreateGemSection( tr("Dependency to be deactivated"), tr("Dependencies to be deactivated"), [=] + { + QVector dependencies; + const QVector toBeRemoved = m_gemModel->GatherGemsToBeRemoved(/*includeDependencies=*/true); + + // don't include gems that were explicitly removed - those are listed in a different section + for (const QModelIndex& modelIndex : toBeRemoved) + { + if (!GemModel::WasPreviouslyAdded(modelIndex)) + { + dependencies.push_back(modelIndex); + } + } + return dependencies; + }); setWindowFlags(Qt::FramelessWindowHint | Qt::Dialog); - - Update(); - connect(gemModel, &GemModel::dataChanged, this, [=] - { - Update(); - }); } - void CartOverlayWidget::Update() + void CartOverlayWidget::CreateGemSection(const QString& singularTitle, const QString& pluralTitle, GetTagIndicesCallback getTagIndices) { - const QVector toBeAdded = m_gemModel->GatherGemsToBeAdded(); - if (toBeAdded.isEmpty()) - { - m_enabledWidget->hide(); - } - else - { - m_enabledTagContainer->Update(ConvertFromModelIndices(toBeAdded)); - m_enabledLabel->setText(QString("%1 %2").arg(QString::number(toBeAdded.size()), tr("Gems to be enabled"))); - m_enabledWidget->show(); - } + QWidget* widget = new QWidget(); + widget->setFixedWidth(s_width); + m_layout->addWidget(widget); - const QVector toBeRemoved = m_gemModel->GatherGemsToBeRemoved(); - if (toBeRemoved.isEmpty()) + QVBoxLayout* layout = new QVBoxLayout(); + layout->setAlignment(Qt::AlignTop); + widget->setLayout(layout); + + QLabel* label = new QLabel(); + label->setObjectName("GemCatalogCartOverlaySectionLabel"); + layout->addWidget(label); + + TagContainerWidget* tagContainer = new TagContainerWidget(); + layout->addWidget(tagContainer); + + auto update = [=]() { - m_disabledWidget->hide(); - } - else - { - m_disabledTagContainer->Update(ConvertFromModelIndices(toBeRemoved)); - m_disabledLabel->setText(QString("%1 %2").arg(QString::number(toBeRemoved.size()), tr("Gems to be disabled"))); - m_disabledWidget->show(); - } + const QVector tagIndices = getTagIndices(); + if (tagIndices.isEmpty()) + { + widget->hide(); + } + else + { + tagContainer->Update(ConvertFromModelIndices(tagIndices)); + label->setText(QString("%1 %2").arg(tagIndices.size()).arg(tagIndices.size() == 1 ? singularTitle : pluralTitle)); + widget->show(); + } + }; + + connect(m_gemModel, &GemModel::dataChanged, this, update); + update(); } QStringList CartOverlayWidget::ConvertFromModelIndices(const QVector& gems) const @@ -154,15 +192,15 @@ namespace O3DE::ProjectManager // Adjust the label text whenever the model gets updated. connect(gemModel, &GemModel::dataChanged, [=] { - const QVector toBeAdded = m_gemModel->GatherGemsToBeAdded(); - const QVector toBeRemoved = m_gemModel->GatherGemsToBeRemoved(); + const QVector toBeAdded = m_gemModel->GatherGemsToBeAdded(/*includeDependencies=*/true); + const QVector toBeRemoved = m_gemModel->GatherGemsToBeRemoved(/*includeDependencies=*/true); const int count = toBeAdded.size() + toBeRemoved.size(); m_countLabel->setText(QString::number(count)); m_dropDownButton->setVisible(!toBeAdded.isEmpty() || !toBeRemoved.isEmpty()); - // Automatically close the overlay window in case there are no gems to be enabled or disabled anymore. + // Automatically close the overlay window in case there are no gems to be activated or deactivated anymore. if (m_cartOverlay && toBeAdded.isEmpty() && toBeRemoved.isEmpty()) { m_cartOverlay->deleteLater(); @@ -186,8 +224,8 @@ namespace O3DE::ProjectManager void CartButton::ShowOverlay() { - const QVector toBeAdded = m_gemModel->GatherGemsToBeAdded(); - const QVector toBeRemoved = m_gemModel->GatherGemsToBeRemoved(); + const QVector toBeAdded = m_gemModel->GatherGemsToBeAdded(/*includeDependencies=*/true); + const QVector toBeRemoved = m_gemModel->GatherGemsToBeRemoved(/*includeDependencies=*/true); if (toBeAdded.isEmpty() && toBeRemoved.isEmpty()) { return; diff --git a/Code/Tools/ProjectManager/Source/GemCatalog/GemCatalogHeaderWidget.h b/Code/Tools/ProjectManager/Source/GemCatalog/GemCatalogHeaderWidget.h index 4c21fbbbe3..2cfda4c790 100644 --- a/Code/Tools/ProjectManager/Source/GemCatalog/GemCatalogHeaderWidget.h +++ b/Code/Tools/ProjectManager/Source/GemCatalog/GemCatalogHeaderWidget.h @@ -8,6 +8,8 @@ #pragma once +#include + #if !defined(Q_MOC_RUN) #include #include @@ -30,22 +32,16 @@ namespace O3DE::ProjectManager public: CartOverlayWidget(GemModel* gemModel, QWidget* parent = nullptr); - void Update(); private: QStringList ConvertFromModelIndices(const QVector& gems) const; + using GetTagIndicesCallback = AZStd::function()>; + void CreateGemSection(const QString& singularTitle, const QString& pluralTitle, GetTagIndicesCallback getTagIndices); + QVBoxLayout* m_layout = nullptr; GemModel* m_gemModel = nullptr; - QWidget* m_enabledWidget = nullptr; - QLabel* m_enabledLabel = nullptr; - TagContainerWidget* m_enabledTagContainer = nullptr; - - QWidget* m_disabledWidget = nullptr; - QLabel* m_disabledLabel = nullptr; - TagContainerWidget* m_disabledTagContainer = nullptr; - inline constexpr static int s_width = 240; }; diff --git a/Code/Tools/ProjectManager/Source/GemCatalog/GemCatalogScreen.cpp b/Code/Tools/ProjectManager/Source/GemCatalog/GemCatalogScreen.cpp index 04d4d6999b..a41a81b448 100644 --- a/Code/Tools/ProjectManager/Source/GemCatalog/GemCatalogScreen.cpp +++ b/Code/Tools/ProjectManager/Source/GemCatalog/GemCatalogScreen.cpp @@ -100,6 +100,8 @@ namespace O3DE::ProjectManager m_gemModel->AddGem(gemInfo); } + m_gemModel->UpdateGemDependencies(); + // Gather enabled gems for the given project. auto enabledGemNamesResult = PythonBindingsInterface::Get()->GetEnabledGemNames(projectPath); if (enabledGemNamesResult.IsSuccess()) diff --git a/Code/Tools/ProjectManager/Source/GemCatalog/GemFilterWidget.cpp b/Code/Tools/ProjectManager/Source/GemCatalog/GemFilterWidget.cpp index c9ea138b55..b425c15dee 100644 --- a/Code/Tools/ProjectManager/Source/GemCatalog/GemFilterWidget.cpp +++ b/Code/Tools/ProjectManager/Source/GemCatalog/GemFilterWidget.cpp @@ -226,13 +226,20 @@ namespace O3DE::ProjectManager QVector elementCounts; const int totalGems = m_gemModel->rowCount(); const int selectedGemTotal = m_gemModel->TotalAddedGems(); + const int enabledGemTotal = m_gemModel->TotalAddedGems(/*includeDependencies=*/true); - elementNames.push_back(GemSortFilterProxyModel::GetGemStatusString(GemSortFilterProxyModel::GemStatus::Unselected)); + elementNames.push_back(GemSortFilterProxyModel::GetGemSelectedString(GemSortFilterProxyModel::GemSelected::Unselected)); elementCounts.push_back(totalGems - selectedGemTotal); - elementNames.push_back(GemSortFilterProxyModel::GetGemStatusString(GemSortFilterProxyModel::GemStatus::Selected)); + elementNames.push_back(GemSortFilterProxyModel::GetGemSelectedString(GemSortFilterProxyModel::GemSelected::Selected)); elementCounts.push_back(selectedGemTotal); + elementNames.push_back(GemSortFilterProxyModel::GetGemActiveString(GemSortFilterProxyModel::GemActive::Inactive)); + elementCounts.push_back(totalGems - enabledGemTotal); + + elementNames.push_back(GemSortFilterProxyModel::GetGemActiveString(GemSortFilterProxyModel::GemActive::Active)); + elementCounts.push_back(enabledGemTotal); + bool wasCollapsed = false; if (m_statusFilter) { @@ -253,48 +260,53 @@ namespace O3DE::ProjectManager m_statusFilter->deleteLater(); m_statusFilter = filterWidget; - const GemSortFilterProxyModel::GemStatus currentFilterState = m_filterProxyModel->GetGemStatus(); const QList buttons = m_statusFilter->GetButtonGroup()->buttons(); - for (int statusFilterIndex = 0; statusFilterIndex < buttons.size(); ++statusFilterIndex) + + QAbstractButton* unselectedButton = buttons[0]; + QAbstractButton* selectedButton = buttons[1]; + unselectedButton->setChecked(m_filterProxyModel->GetGemSelected() == GemSortFilterProxyModel::GemSelected::Unselected); + selectedButton->setChecked(m_filterProxyModel->GetGemSelected() == GemSortFilterProxyModel::GemSelected::Selected); + + auto updateGemSelection = [=]([[maybe_unused]] bool checked) { - const GemSortFilterProxyModel::GemStatus gemStatus = static_cast(statusFilterIndex); - QAbstractButton* button = buttons[statusFilterIndex]; - - if (static_cast(statusFilterIndex) == currentFilterState) + if (unselectedButton->isChecked() && !selectedButton->isChecked()) { - button->setChecked(true); + m_filterProxyModel->SetGemSelected(GemSortFilterProxyModel::GemSelected::Unselected); } + else if (!unselectedButton->isChecked() && selectedButton->isChecked()) + { + m_filterProxyModel->SetGemSelected(GemSortFilterProxyModel::GemSelected::Selected); + } + else + { + m_filterProxyModel->SetGemSelected(GemSortFilterProxyModel::GemSelected::NoFilter); + } + }; + connect(unselectedButton, &QAbstractButton::toggled, this, updateGemSelection); + connect(selectedButton, &QAbstractButton::toggled, this, updateGemSelection); - connect( - button, &QAbstractButton::toggled, this, - [=](bool checked) - { - GemSortFilterProxyModel::GemStatus filterStatus = m_filterProxyModel->GetGemStatus(); - if (checked) - { - if (filterStatus == GemSortFilterProxyModel::GemStatus::NoFilter) - { - filterStatus = gemStatus; - } - else - { - filterStatus = GemSortFilterProxyModel::GemStatus::NoFilter; - } - } - else - { - if (filterStatus != gemStatus) - { - filterStatus = static_cast(!gemStatus); - } - else - { - filterStatus = GemSortFilterProxyModel::GemStatus::NoFilter; - } - } - m_filterProxyModel->SetGemStatus(filterStatus); - }); - } + QAbstractButton* inactiveButton = buttons[2]; + QAbstractButton* activeButton = buttons[3]; + inactiveButton->setChecked(m_filterProxyModel->GetGemActive() == GemSortFilterProxyModel::GemActive::Inactive); + activeButton->setChecked(m_filterProxyModel->GetGemActive() == GemSortFilterProxyModel::GemActive::Active); + + auto updateGemActive = [=]([[maybe_unused]] bool checked) + { + if (inactiveButton->isChecked() && !activeButton->isChecked()) + { + m_filterProxyModel->SetGemActive(GemSortFilterProxyModel::GemActive::Inactive); + } + else if (!inactiveButton->isChecked() && activeButton->isChecked()) + { + m_filterProxyModel->SetGemActive(GemSortFilterProxyModel::GemActive::Active); + } + else + { + m_filterProxyModel->SetGemActive(GemSortFilterProxyModel::GemActive::NoFilter); + } + }; + connect(inactiveButton, &QAbstractButton::toggled, this, updateGemActive); + connect(activeButton, &QAbstractButton::toggled, this, updateGemActive); } void GemFilterWidget::AddGemOriginFilter() @@ -487,7 +499,7 @@ namespace O3DE::ProjectManager const QString& feature = elementNames[i]; QAbstractButton* button = buttons[i]; - // Adjust the proxy model and enable or disable the clicked feature used for filtering. + // Adjust the proxy model and enable the clicked feature used for filtering. connect(button, &QAbstractButton::toggled, this, [=](bool checked) { QSet features = m_filterProxyModel->GetFeatures(); diff --git a/Code/Tools/ProjectManager/Source/GemCatalog/GemInfo.h b/Code/Tools/ProjectManager/Source/GemCatalog/GemInfo.h index 4312b08998..311eeb93f6 100644 --- a/Code/Tools/ProjectManager/Source/GemCatalog/GemInfo.h +++ b/Code/Tools/ProjectManager/Source/GemCatalog/GemInfo.h @@ -75,8 +75,7 @@ namespace O3DE::ProjectManager QString m_version = "Unknown Version"; QString m_lastUpdatedDate = "Unknown Date"; int m_binarySizeInKB = 0; - QStringList m_dependingGemUuids; - QStringList m_conflictingGemUuids; + QStringList m_dependencies; }; } // namespace O3DE::ProjectManager diff --git a/Code/Tools/ProjectManager/Source/GemCatalog/GemInspector.cpp b/Code/Tools/ProjectManager/Source/GemCatalog/GemInspector.cpp index 6dd6c52612..282aa2193e 100644 --- a/Code/Tools/ProjectManager/Source/GemCatalog/GemInspector.cpp +++ b/Code/Tools/ProjectManager/Source/GemCatalog/GemInspector.cpp @@ -83,9 +83,8 @@ namespace O3DE::ProjectManager m_reqirementsTextLabel->hide(); } - // Depending and conflicting gems + // Depending gems m_dependingGems->Update("Depending Gems", "The following Gems will be automatically enabled with this Gem.", m_model->GetDependingGemNames(modelIndex)); - m_conflictingGems->Update("Conflicting Gems", "The following Gems will be automatically disabled with this Gem.", m_model->GetConflictingGemNames(modelIndex)); // Additional information m_versionLabel->setText(QString("Gem Version: %1").arg(m_model->GetVersion(modelIndex))); @@ -173,15 +172,11 @@ namespace O3DE::ProjectManager m_mainLayout->addSpacing(20); - // Depending and conflicting gems + // Depending gems m_dependingGems = new GemsSubWidget(); m_mainLayout->addWidget(m_dependingGems); m_mainLayout->addSpacing(20); - m_conflictingGems = new GemsSubWidget(); - m_mainLayout->addWidget(m_conflictingGems); - m_mainLayout->addSpacing(20); - // Additional information QLabel* additionalInfoLabel = CreateStyledLabel(m_mainLayout, 14, s_headerColor); additionalInfoLabel->setText("Additional Information"); diff --git a/Code/Tools/ProjectManager/Source/GemCatalog/GemInspector.h b/Code/Tools/ProjectManager/Source/GemCatalog/GemInspector.h index 97c23f7df2..0c41d1033c 100644 --- a/Code/Tools/ProjectManager/Source/GemCatalog/GemInspector.h +++ b/Code/Tools/ProjectManager/Source/GemCatalog/GemInspector.h @@ -78,7 +78,6 @@ namespace O3DE::ProjectManager // Depending and conflicting gems GemsSubWidget* m_dependingGems = nullptr; - GemsSubWidget* m_conflictingGems = nullptr; // Additional information QLabel* m_versionLabel = nullptr; diff --git a/Code/Tools/ProjectManager/Source/GemCatalog/GemItemDelegate.cpp b/Code/Tools/ProjectManager/Source/GemCatalog/GemItemDelegate.cpp index 99a2cd8db7..dc24c13009 100644 --- a/Code/Tools/ProjectManager/Source/GemCatalog/GemItemDelegate.cpp +++ b/Code/Tools/ProjectManager/Source/GemCatalog/GemItemDelegate.cpp @@ -8,9 +8,14 @@ #include #include +#include #include +#include #include #include +#include +#include +#include namespace O3DE::ProjectManager { @@ -149,8 +154,7 @@ namespace O3DE::ProjectManager return true; } } - - if (event->type() == QEvent::MouseButtonPress) + else if (event->type() == QEvent::MouseButtonPress ) { QMouseEvent* mouseEvent = static_cast(event); @@ -169,6 +173,69 @@ namespace O3DE::ProjectManager return QStyledItemDelegate::editorEvent(event, model, option, modelIndex); } + QString GetGemNameList(const QVector modelIndices) + { + QString gemNameList; + for (int i = 0; i < modelIndices.size(); ++i) + { + if (!gemNameList.isEmpty()) + { + if (i == modelIndices.size() - 1) + { + gemNameList.append(" and "); + } + else + { + gemNameList.append(", "); + } + } + + gemNameList.append(GemModel::GetDisplayName(modelIndices[i])); + } + + return gemNameList; + } + + bool GemItemDelegate::helpEvent(QHelpEvent* event, QAbstractItemView* view, const QStyleOptionViewItem& option, const QModelIndex& index) + { + if (event->type() == QEvent::ToolTip) + { + QRect fullRect, itemRect, contentRect; + CalcRects(option, fullRect, itemRect, contentRect); + const QRect buttonRect = CalcButtonRect(contentRect); + if (buttonRect.contains(event->pos())) + { + if (!QToolTip::isVisible()) + { + if(GemModel::IsAddedDependency(index) && !GemModel::IsAdded(index)) + { + const GemModel* gemModel = GemModel::GetSourceModel(index.model()); + AZ_Assert(gemModel, "Failed to obtain GemModel"); + + // we only want to display the gems that must be de-selected to automatically + // disable this dependency, so don't include any that haven't been selected (added) + constexpr bool addedOnly = true; + QVector dependents = gemModel->GatherDependentGems(index, addedOnly); + QString nameList = GetGemNameList(dependents); + if (!nameList.isEmpty()) + { + QToolTip::showText(event->globalPos(), tr("This gem is a dependency of %1.\nTo disable this gem, first disable %1.").arg(nameList)); + } + } + } + return true; + } + else if (QToolTip::isVisible()) + { + QToolTip::hideText(); + event->ignore(); + return true; + } + } + + return QStyledItemDelegate::helpEvent(event, view, option, index); + } + void GemItemDelegate::CalcRects(const QStyleOptionViewItem& option, QRect& outFullRect, QRect& outItemRect, QRect& outContentRect) const { outFullRect = QRect(option.rect); @@ -260,14 +327,20 @@ namespace O3DE::ProjectManager const QRect buttonRect = CalcButtonRect(contentRect); QPoint circleCenter; - const bool isAdded = GemModel::IsAdded(modelIndex); - if (isAdded) + if (GemModel::IsAdded(modelIndex)) { painter->setBrush(m_buttonEnabledColor); painter->setPen(m_buttonEnabledColor); circleCenter = buttonRect.center() + QPoint(buttonRect.width() / 2 - s_buttonBorderRadius + 1, 1); } + else if (GemModel::IsAddedDependency(modelIndex)) + { + painter->setBrush(m_buttonImplicitlyEnabledColor); + painter->setPen(m_buttonImplicitlyEnabledColor); + + circleCenter = buttonRect.center() + QPoint(buttonRect.width() / 2 - s_buttonBorderRadius + 1, 1); + } else { circleCenter = buttonRect.center() + QPoint(-buttonRect.width() / 2 + s_buttonBorderRadius, 1); diff --git a/Code/Tools/ProjectManager/Source/GemCatalog/GemItemDelegate.h b/Code/Tools/ProjectManager/Source/GemCatalog/GemItemDelegate.h index a0a3dbb36a..d842f63ae7 100644 --- a/Code/Tools/ProjectManager/Source/GemCatalog/GemItemDelegate.h +++ b/Code/Tools/ProjectManager/Source/GemCatalog/GemItemDelegate.h @@ -29,7 +29,6 @@ namespace O3DE::ProjectManager ~GemItemDelegate() = default; void paint(QPainter* painter, const QStyleOptionViewItem& option, const QModelIndex& modelIndex) const override; - bool editorEvent(QEvent* event, QAbstractItemModel* model, const QStyleOptionViewItem& option, const QModelIndex& modelIndex) override; QSize sizeHint(const QStyleOptionViewItem& option, const QModelIndex& modelIndex) const override; // Colors @@ -39,6 +38,7 @@ namespace O3DE::ProjectManager const QColor m_itemBackgroundColor = QColor("#404040"); // Background color of the gem item const QColor m_borderColor = QColor("#1E70EB"); const QColor m_buttonEnabledColor = QColor("#00B931"); + const QColor m_buttonImplicitlyEnabledColor = QColor("#BCBCBE"); // Item inline constexpr static int s_height = 105; // Gem item total height @@ -65,6 +65,9 @@ namespace O3DE::ProjectManager inline constexpr static int s_featureTagSpacing = 7; protected: + bool editorEvent(QEvent* event, QAbstractItemModel* model, const QStyleOptionViewItem& option, const QModelIndex& modelIndex) override; + bool helpEvent(QHelpEvent* event, QAbstractItemView* view, const QStyleOptionViewItem& option, const QModelIndex& index) override; + void CalcRects(const QStyleOptionViewItem& option, QRect& outFullRect, QRect& outItemRect, QRect& outContentRect) const; QRect GetTextRect(QFont& font, const QString& text, qreal fontSize) const; QRect CalcButtonRect(const QRect& contentRect) const; diff --git a/Code/Tools/ProjectManager/Source/GemCatalog/GemListHeaderWidget.cpp b/Code/Tools/ProjectManager/Source/GemCatalog/GemListHeaderWidget.cpp index 16234f4900..ab51c7511c 100644 --- a/Code/Tools/ProjectManager/Source/GemCatalog/GemListHeaderWidget.cpp +++ b/Code/Tools/ProjectManager/Source/GemCatalog/GemListHeaderWidget.cpp @@ -60,11 +60,14 @@ namespace O3DE::ProjectManager QLabel* showCountLabel = new QLabel(); showCountLabel->setObjectName("GemCatalogHeaderShowCountLabel"); topLayout->addWidget(showCountLabel); - connect(proxyModel, &GemSortFilterProxyModel::OnInvalidated, this, [=] - { + + auto refreshGemCountUI = [=]() { const int numGemsShown = proxyModel->rowCount(); showCountLabel->setText(QString(tr("showing %1 Gems")).arg(numGemsShown)); - }); + }; + + connect(proxyModel, &GemSortFilterProxyModel::OnInvalidated, this, refreshGemCountUI); + connect(proxyModel->GetSourceModel(), &GemModel::dataChanged, this, refreshGemCountUI); topLayout->addSpacing(GemItemDelegate::s_contentMargins.right() + GemItemDelegate::s_borderWidth); diff --git a/Code/Tools/ProjectManager/Source/GemCatalog/GemListView.cpp b/Code/Tools/ProjectManager/Source/GemCatalog/GemListView.cpp index d393a30ed9..afdb9697c9 100644 --- a/Code/Tools/ProjectManager/Source/GemCatalog/GemListView.cpp +++ b/Code/Tools/ProjectManager/Source/GemCatalog/GemListView.cpp @@ -9,9 +9,26 @@ #include #include #include +#include namespace O3DE::ProjectManager { + class GemListViewProxyStyle : public QProxyStyle + { + public: + using QProxyStyle::QProxyStyle; + int styleHint(StyleHint hint, const QStyleOption* option = nullptr, const QWidget* widget = nullptr, QStyleHintReturn* returnData = nullptr) const override + { + if (hint == QStyle::SH_ToolTip_WakeUpDelay || hint == QStyle::SH_ToolTip_FallAsleepDelay) + { + // no delay + return 0; + } + + return QProxyStyle::styleHint(hint, option, widget, returnData); + } + }; + GemListView::GemListView(QAbstractItemModel* model, QItemSelectionModel* selectionModel, QWidget* parent) : QListView(parent) { @@ -21,5 +38,8 @@ namespace O3DE::ProjectManager setModel(model); setSelectionModel(selectionModel); setItemDelegate(new GemItemDelegate(model, this)); + + // use a custom proxy style so we get immediate tooltips for gem radio buttons + setStyle(new GemListViewProxyStyle(this->style())); } } // namespace O3DE::ProjectManager diff --git a/Code/Tools/ProjectManager/Source/GemCatalog/GemModel.cpp b/Code/Tools/ProjectManager/Source/GemCatalog/GemModel.cpp index 7daea174e7..0941541793 100644 --- a/Code/Tools/ProjectManager/Source/GemCatalog/GemModel.cpp +++ b/Code/Tools/ProjectManager/Source/GemCatalog/GemModel.cpp @@ -8,6 +8,7 @@ #include #include +#include #include namespace O3DE::ProjectManager @@ -40,8 +41,7 @@ namespace O3DE::ProjectManager item->setData(gemInfo.m_isAdded, RoleIsAdded); item->setData(gemInfo.m_directoryLink, RoleDirectoryLink); item->setData(gemInfo.m_documentationLink, RoleDocLink); - item->setData(gemInfo.m_dependingGemUuids, RoleDependingGems); - item->setData(gemInfo.m_conflictingGemUuids, RoleConflictingGems); + item->setData(gemInfo.m_dependencies, RoleDependingGems); item->setData(gemInfo.m_version, RoleVersion); item->setData(gemInfo.m_lastUpdatedDate, RoleLastUpdated); item->setData(gemInfo.m_binarySizeInKB, RoleBinarySize); @@ -60,6 +60,39 @@ namespace O3DE::ProjectManager clear(); } + void GemModel::UpdateGemDependencies() + { + m_gemDependencyMap.clear(); + m_gemReverseDependencyMap.clear(); + + for (auto iter = m_nameToIndexMap.begin(); iter != m_nameToIndexMap.end(); ++iter) + { + const QString& key = iter.key(); + const QModelIndex modelIndex = iter.value(); + QSet dependencies; + GetAllDependingGems(modelIndex, dependencies); + if (!dependencies.isEmpty()) + { + m_gemDependencyMap.insert(key, dependencies); + } + } + + for (auto iter = m_gemDependencyMap.begin(); iter != m_gemDependencyMap.end(); ++iter) + { + const QString& dependant = iter.key(); + for (const QModelIndex& dependency : iter.value()) + { + const QString& dependencyName = dependency.data(RoleName).toString(); + if (!m_gemReverseDependencyMap.contains(dependencyName)) + { + m_gemReverseDependencyMap.insert(dependencyName, QSet()); + } + + m_gemReverseDependencyMap[dependencyName].insert(m_nameToIndexMap[dependant]); + } + } + } + QString GemModel::GetName(const QModelIndex& modelIndex) { return modelIndex.data(RoleName).toString(); @@ -125,49 +158,46 @@ namespace O3DE::ProjectManager return {}; } - void GemModel::FindGemNamesByNameStrings(QStringList& inOutGemNames) + void GemModel::FindGemDisplayNamesByNameStrings(QStringList& inOutGemNames) { - for (QString& dependingGemString : inOutGemNames) + for (QString& name : inOutGemNames) { - QModelIndex modelIndex = FindIndexByNameString(dependingGemString); + QModelIndex modelIndex = FindIndexByNameString(name); if (modelIndex.isValid()) { - dependingGemString = GetDisplayName(modelIndex); + name = GetDisplayName(modelIndex); } } } - QStringList GemModel::GetDependingGemUuids(const QModelIndex& modelIndex) + QStringList GemModel::GetDependingGems(const QModelIndex& modelIndex) { return modelIndex.data(RoleDependingGems).toStringList(); } + void GemModel::GetAllDependingGems(const QModelIndex& modelIndex, QSet& inOutGems) + { + QStringList dependencies = GetDependingGems(modelIndex); + for (const QString& dependency : dependencies) + { + QModelIndex dependencyIndex = FindIndexByNameString(dependency); + if (!inOutGems.contains(dependencyIndex)) + { + inOutGems.insert(dependencyIndex); + GetAllDependingGems(dependencyIndex, inOutGems); + } + } + } + QStringList GemModel::GetDependingGemNames(const QModelIndex& modelIndex) { - QStringList result = GetDependingGemUuids(modelIndex); + QStringList result = GetDependingGems(modelIndex); if (result.isEmpty()) { return {}; } - FindGemNamesByNameStrings(result); - return result; - } - - QStringList GemModel::GetConflictingGemUuids(const QModelIndex& modelIndex) - { - return modelIndex.data(RoleConflictingGems).toStringList(); - } - - QStringList GemModel::GetConflictingGemNames(const QModelIndex& modelIndex) - { - QStringList result = GetConflictingGemUuids(modelIndex); - if (result.isEmpty()) - { - return {}; - } - - FindGemNamesByNameStrings(result); + FindGemDisplayNamesByNameStrings(result); return result; } @@ -201,29 +231,146 @@ namespace O3DE::ProjectManager return modelIndex.data(RoleRequirement).toString(); } + GemModel* GemModel::GetSourceModel(QAbstractItemModel* model) + { + GemSortFilterProxyModel* proxyModel = qobject_cast(model); + if (proxyModel) + { + return proxyModel->GetSourceModel(); + } + else + { + return qobject_cast(model); + } + } + + const GemModel* GemModel::GetSourceModel(const QAbstractItemModel* model) + { + const GemSortFilterProxyModel* proxyModel = qobject_cast(model); + if (proxyModel) + { + return proxyModel->GetSourceModel(); + } + else + { + return qobject_cast(model); + } + } + bool GemModel::IsAdded(const QModelIndex& modelIndex) { return modelIndex.data(RoleIsAdded).toBool(); } + bool GemModel::IsAddedDependency(const QModelIndex& modelIndex) + { + return modelIndex.data(RoleIsAddedDependency).toBool(); + } + void GemModel::SetIsAdded(QAbstractItemModel& model, const QModelIndex& modelIndex, bool isAdded) { model.setData(modelIndex, isAdded, RoleIsAdded); + + UpdateDependencies(model, modelIndex); + } + + bool GemModel::HasDependentGems(const QModelIndex& modelIndex) const + { + QVector dependentGems = GatherDependentGems(modelIndex); + for (const QModelIndex& dependency : dependentGems) + { + if (IsAdded(dependency)) + { + return true; + } + } + return false; + } + + void GemModel::UpdateDependencies(QAbstractItemModel& model, const QModelIndex& modelIndex) + { + GemModel* gemModel = GetSourceModel(&model); + AZ_Assert(gemModel, "Failed to obtain GemModel"); + + QVector dependencies = gemModel->GatherGemDependencies(modelIndex); + if (IsAdded(modelIndex)) + { + for (const QModelIndex& dependency : dependencies) + { + SetIsAddedDependency(*gemModel, dependency, true); + } + } + else + { + // still a dependency if some added gem depends on this one + SetIsAddedDependency(model, modelIndex, gemModel->HasDependentGems(modelIndex)); + + for (const QModelIndex& dependency : dependencies) + { + SetIsAddedDependency(*gemModel, dependency, gemModel->HasDependentGems(dependency)); + } + } + } + + void GemModel::SetIsAddedDependency(QAbstractItemModel& model, const QModelIndex& modelIndex, bool isAdded) + { + model.setData(modelIndex, isAdded, RoleIsAddedDependency); } void GemModel::SetWasPreviouslyAdded(QAbstractItemModel& model, const QModelIndex& modelIndex, bool wasAdded) { model.setData(modelIndex, wasAdded, RoleWasPreviouslyAdded); + + if (wasAdded) + { + // update all dependencies + GemModel* gemModel = GetSourceModel(&model); + AZ_Assert(gemModel, "Failed to obtain GemModel"); + QVector dependencies = gemModel->GatherGemDependencies(modelIndex); + for (const QModelIndex& dependency : dependencies) + { + SetWasPreviouslyAddedDependency(*gemModel, dependency, true); + } + } } - bool GemModel::NeedsToBeAdded(const QModelIndex& modelIndex) + void GemModel::SetWasPreviouslyAddedDependency(QAbstractItemModel& model, const QModelIndex& modelIndex, bool wasAdded) { - return (!modelIndex.data(RoleWasPreviouslyAdded).toBool() && modelIndex.data(RoleIsAdded).toBool()); + model.setData(modelIndex, wasAdded, RoleWasPreviouslyAddedDependency); } - bool GemModel::NeedsToBeRemoved(const QModelIndex& modelIndex) + bool GemModel::WasPreviouslyAdded(const QModelIndex& modelIndex) { - return (modelIndex.data(RoleWasPreviouslyAdded).toBool() && !modelIndex.data(RoleIsAdded).toBool()); + return modelIndex.data(RoleWasPreviouslyAdded).toBool(); + } + + bool GemModel::WasPreviouslyAddedDependency(const QModelIndex& modelIndex) + { + return modelIndex.data(RoleWasPreviouslyAddedDependency).toBool(); + } + + bool GemModel::NeedsToBeAdded(const QModelIndex& modelIndex, bool includeDependencies) + { + bool previouslyAdded = modelIndex.data(RoleWasPreviouslyAdded).toBool(); + bool added = modelIndex.data(RoleIsAdded).toBool(); + if (includeDependencies) + { + previouslyAdded |= modelIndex.data(RoleWasPreviouslyAddedDependency).toBool(); + added |= modelIndex.data(RoleIsAddedDependency).toBool(); + } + return !previouslyAdded && added; + } + + bool GemModel::NeedsToBeRemoved(const QModelIndex& modelIndex, bool includeDependencies) + { + bool previouslyAdded = modelIndex.data(RoleWasPreviouslyAdded).toBool(); + bool added = modelIndex.data(RoleIsAdded).toBool(); + if (includeDependencies) + { + previouslyAdded |= modelIndex.data(RoleWasPreviouslyAddedDependency).toBool(); + added |= modelIndex.data(RoleIsAddedDependency).toBool(); + } + return previouslyAdded && !added; } bool GemModel::HasRequirement(const QModelIndex& modelIndex) @@ -244,13 +391,44 @@ namespace O3DE::ProjectManager return false; } - QVector GemModel::GatherGemsToBeAdded() const + QVector GemModel::GatherGemDependencies(const QModelIndex& modelIndex) const + { + QVector result; + const QString& gemName = modelIndex.data(RoleName).toString(); + if (m_gemDependencyMap.contains(gemName)) + { + for (const QModelIndex& dependency : m_gemDependencyMap[gemName]) + { + result.push_back(dependency); + } + } + return result; + } + + QVector GemModel::GatherDependentGems(const QModelIndex& modelIndex, bool addedOnly) const + { + QVector result; + const QString& gemName = modelIndex.data(RoleName).toString(); + if (m_gemReverseDependencyMap.contains(gemName)) + { + for (const QModelIndex& dependency : m_gemReverseDependencyMap[gemName]) + { + if (!addedOnly || GemModel::IsAdded(dependency)) + { + result.push_back(dependency); + } + } + } + return result; + } + + QVector GemModel::GatherGemsToBeAdded(bool includeDependencies) const { QVector result; for (int row = 0; row < rowCount(); ++row) { const QModelIndex modelIndex = index(row, 0); - if (NeedsToBeAdded(modelIndex)) + if (NeedsToBeAdded(modelIndex, includeDependencies)) { result.push_back(modelIndex); } @@ -258,13 +436,13 @@ namespace O3DE::ProjectManager return result; } - QVector GemModel::GatherGemsToBeRemoved() const + QVector GemModel::GatherGemsToBeRemoved(bool includeDependencies) const { QVector result; for (int row = 0; row < rowCount(); ++row) { const QModelIndex modelIndex = index(row, 0); - if (NeedsToBeRemoved(modelIndex)) + if (NeedsToBeRemoved(modelIndex, includeDependencies)) { result.push_back(modelIndex); } @@ -272,13 +450,13 @@ namespace O3DE::ProjectManager return result; } - int GemModel::TotalAddedGems() const + int GemModel::TotalAddedGems(bool includeDependencies) const { int result = 0; for (int row = 0; row < rowCount(); ++row) { const QModelIndex modelIndex = index(row, 0); - if (IsAdded(modelIndex)) + if (IsAdded(modelIndex) || (includeDependencies && IsAddedDependency(modelIndex))) { ++result; } diff --git a/Code/Tools/ProjectManager/Source/GemCatalog/GemModel.h b/Code/Tools/ProjectManager/Source/GemCatalog/GemModel.h index ce004ee875..0591094c11 100644 --- a/Code/Tools/ProjectManager/Source/GemCatalog/GemModel.h +++ b/Code/Tools/ProjectManager/Source/GemCatalog/GemModel.h @@ -28,13 +28,11 @@ namespace O3DE::ProjectManager void AddGem(const GemInfo& gemInfo); void Clear(); + void UpdateGemDependencies(); QModelIndex FindIndexByNameString(const QString& nameString) const; - void FindGemNamesByNameStrings(QStringList& inOutGemNames); - QStringList GetDependingGemUuids(const QModelIndex& modelIndex); QStringList GetDependingGemNames(const QModelIndex& modelIndex); - QStringList GetConflictingGemUuids(const QModelIndex& modelIndex); - QStringList GetConflictingGemNames(const QModelIndex& modelIndex); + bool HasDependentGems(const QModelIndex& modelIndex) const; static QString GetName(const QModelIndex& modelIndex); static QString GetDisplayName(const QModelIndex& modelIndex); @@ -51,22 +49,36 @@ namespace O3DE::ProjectManager static QStringList GetFeatures(const QModelIndex& modelIndex); static QString GetPath(const QModelIndex& modelIndex); static QString GetRequirement(const QModelIndex& modelIndex); + static GemModel* GetSourceModel(QAbstractItemModel* model); + static const GemModel* GetSourceModel(const QAbstractItemModel* model); static bool IsAdded(const QModelIndex& modelIndex); + static bool IsAddedDependency(const QModelIndex& modelIndex); static void SetIsAdded(QAbstractItemModel& model, const QModelIndex& modelIndex, bool isAdded); + static void SetIsAddedDependency(QAbstractItemModel& model, const QModelIndex& modelIndex, bool isAdded); static void SetWasPreviouslyAdded(QAbstractItemModel& model, const QModelIndex& modelIndex, bool wasAdded); - static bool NeedsToBeAdded(const QModelIndex& modelIndex); - static bool NeedsToBeRemoved(const QModelIndex& modelIndex); + static bool WasPreviouslyAdded(const QModelIndex& modelIndex); + static void SetWasPreviouslyAddedDependency(QAbstractItemModel& model, const QModelIndex& modelIndex, bool wasAdded); + static bool WasPreviouslyAddedDependency(const QModelIndex& modelIndex); + static bool NeedsToBeAdded(const QModelIndex& modelIndex, bool includeDependencies = false); + static bool NeedsToBeRemoved(const QModelIndex& modelIndex, bool includeDependencies = false); static bool HasRequirement(const QModelIndex& modelIndex); + static void UpdateDependencies(QAbstractItemModel& model, const QModelIndex& modelIndex); bool DoGemsToBeAddedHaveRequirements() const; - QVector GatherGemsToBeAdded() const; - QVector GatherGemsToBeRemoved() const; + QVector GatherGemDependencies(const QModelIndex& modelIndex) const; + QVector GatherDependentGems(const QModelIndex& modelIndex, bool addedOnly = false) const; + QVector GatherGemsToBeAdded(bool includeDependencies = false) const; + QVector GatherGemsToBeRemoved(bool includeDependencies = false) const; - int TotalAddedGems() const; + int TotalAddedGems(bool includeDependencies = false) const; private: + void FindGemDisplayNamesByNameStrings(QStringList& inOutGemNames); + void GetAllDependingGems(const QModelIndex& modelIndex, QSet& inOutGems); + QStringList GetDependingGems(const QModelIndex& modelIndex); + enum UserRole { RoleName = Qt::UserRole, @@ -76,11 +88,12 @@ namespace O3DE::ProjectManager RolePlatforms, RoleSummary, RoleWasPreviouslyAdded, + RoleWasPreviouslyAddedDependency, RoleIsAdded, + RoleIsAddedDependency, RoleDirectoryLink, RoleDocLink, RoleDependingGems, - RoleConflictingGems, RoleVersion, RoleLastUpdated, RoleBinarySize, @@ -92,5 +105,7 @@ namespace O3DE::ProjectManager QHash m_nameToIndexMap; QItemSelectionModel* m_selectionModel = nullptr; + QHash> m_gemDependencyMap; + QHash> m_gemReverseDependencyMap; }; } // namespace O3DE::ProjectManager diff --git a/Code/Tools/ProjectManager/Source/GemCatalog/GemSortFilterProxyModel.cpp b/Code/Tools/ProjectManager/Source/GemCatalog/GemSortFilterProxyModel.cpp index 6edfced6e5..199692f200 100644 --- a/Code/Tools/ProjectManager/Source/GemCatalog/GemSortFilterProxyModel.cpp +++ b/Code/Tools/ProjectManager/Source/GemCatalog/GemSortFilterProxyModel.cpp @@ -50,11 +50,21 @@ namespace O3DE::ProjectManager } } - // Gem status - if (m_gemStatusFilter != GemStatus::NoFilter) + // Gem selected + if (m_gemSelectedFilter != GemSelected::NoFilter) { - const GemStatus sourceGemStatus = static_cast(GemModel::IsAdded(sourceIndex)); - if (m_gemStatusFilter != sourceGemStatus) + const GemSelected sourceGemStatus = static_cast(GemModel::IsAdded(sourceIndex)); + if (m_gemSelectedFilter != sourceGemStatus) + { + return false; + } + } + + // Gem enabled + if (m_gemActiveFilter != GemActive::NoFilter) + { + const GemActive sourceGemStatus = static_cast(GemModel::IsAdded(sourceIndex) || GemModel::IsAddedDependency(sourceIndex)); + if (m_gemActiveFilter != sourceGemStatus) { return false; } @@ -148,19 +158,31 @@ namespace O3DE::ProjectManager return true; } - QString GemSortFilterProxyModel::GetGemStatusString(GemStatus status) + QString GemSortFilterProxyModel::GetGemSelectedString(GemSelected status) { switch (status) { - case Unselected: + case GemSelected::Unselected: return "Unselected"; - case Selected: + case GemSelected::Selected: return "Selected"; default: - return ""; + return ""; } } + QString GemSortFilterProxyModel::GetGemActiveString(GemActive status) + { + switch (status) + { + case GemActive::Inactive: + return "Inactive"; + case GemActive::Active: + return "Active"; + default: + return ""; + } + } void GemSortFilterProxyModel::InvalidateFilter() { invalidate(); diff --git a/Code/Tools/ProjectManager/Source/GemCatalog/GemSortFilterProxyModel.h b/Code/Tools/ProjectManager/Source/GemCatalog/GemSortFilterProxyModel.h index 4ec170aef2..74b1e915eb 100644 --- a/Code/Tools/ProjectManager/Source/GemCatalog/GemSortFilterProxyModel.h +++ b/Code/Tools/ProjectManager/Source/GemCatalog/GemSortFilterProxyModel.h @@ -25,16 +25,23 @@ namespace O3DE::ProjectManager Q_OBJECT // AUTOMOC public: - enum GemStatus + enum class GemSelected { NoFilter = -1, Unselected, Selected }; + enum class GemActive + { + NoFilter = -1, + Inactive, + Active + }; GemSortFilterProxyModel(GemModel* sourceModel, QObject* parent = nullptr); - static QString GetGemStatusString(GemStatus status); + static QString GetGemSelectedString(GemSelected status); + static QString GetGemActiveString(GemActive status); bool filterAcceptsRow(int sourceRow, const QModelIndex& sourceParent) const override; @@ -43,8 +50,11 @@ namespace O3DE::ProjectManager void SetSearchString(const QString& searchString) { m_searchString = searchString; InvalidateFilter(); } - GemStatus GetGemStatus() const { return m_gemStatusFilter; } - void SetGemStatus(GemStatus gemStatus) { m_gemStatusFilter = gemStatus; InvalidateFilter(); } + GemSelected GetGemSelected() const { return m_gemSelectedFilter; } + void SetGemSelected(GemSelected selected) { m_gemSelectedFilter = selected; InvalidateFilter(); } + + GemActive GetGemActive() const { return m_gemActiveFilter; } + void SetGemActive(GemActive enabled) { m_gemActiveFilter = enabled; InvalidateFilter(); } GemInfo::GemOrigins GetGemOrigins() const { return m_gemOriginFilter; } void SetGemOrigins(const GemInfo::GemOrigins& gemOrigins) { m_gemOriginFilter = gemOrigins; InvalidateFilter(); } @@ -69,7 +79,8 @@ namespace O3DE::ProjectManager AzQtComponents::SelectionProxyModel* m_selectionProxyModel = nullptr; QString m_searchString; - GemStatus m_gemStatusFilter = GemStatus::NoFilter; + GemSelected m_gemSelectedFilter = GemSelected::NoFilter; + GemActive m_gemActiveFilter = GemActive::NoFilter; GemInfo::GemOrigins m_gemOriginFilter = {}; GemInfo::Platforms m_platformFilter = {}; GemInfo::Types m_typeFilter = {}; diff --git a/Code/Tools/ProjectManager/Source/PythonBindings.cpp b/Code/Tools/ProjectManager/Source/PythonBindings.cpp index c92ad53cd4..9f0c6a51ba 100644 --- a/Code/Tools/ProjectManager/Source/PythonBindings.cpp +++ b/Code/Tools/ProjectManager/Source/PythonBindings.cpp @@ -675,6 +675,14 @@ namespace O3DE::ProjectManager } } + if (data.contains("dependencies")) + { + for (auto dependency : data["dependencies"]) + { + gemInfo.m_dependencies.push_back(Py_To_String(dependency)); + } + } + QString gemType = Py_To_String_Optional(data, "type", ""); if (gemType == "Asset") { diff --git a/Code/Tools/ProjectManager/project_manager_tests_files.cmake b/Code/Tools/ProjectManager/project_manager_tests_files.cmake index 2b22ced910..2bfe343038 100644 --- a/Code/Tools/ProjectManager/project_manager_tests_files.cmake +++ b/Code/Tools/ProjectManager/project_manager_tests_files.cmake @@ -11,6 +11,7 @@ set(FILES Resources/ProjectManager.qss tests/ApplicationTests.cpp tests/PythonBindingsTests.cpp + tests/GemCatalogTests.cpp tests/main.cpp tests/UtilsTests.cpp ) diff --git a/Code/Tools/ProjectManager/tests/GemCatalogTests.cpp b/Code/Tools/ProjectManager/tests/GemCatalogTests.cpp new file mode 100644 index 0000000000..f5c6d5196a --- /dev/null +++ b/Code/Tools/ProjectManager/tests/GemCatalogTests.cpp @@ -0,0 +1,64 @@ +/* + * 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 + + +namespace O3DE::ProjectManager +{ + class GemCatalogTests + : public ::UnitTest::ScopedAllocatorSetupFixture + { + public: + + GemCatalogTests() = default; + }; + + TEST_F(GemCatalogTests, GemCatalog_Displays_But_Does_Not_Add_Dependencies) + { + 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; + gemA.m_name = "a"; + gemB.m_name = "b"; + gemC.m_name = "c"; + + gemA.m_dependencies = QStringList({ "b" }); + gemB.m_dependencies = QStringList({ "c" }); + + gemModel->AddGem(gemA); + indexA = gemModel->FindIndexByNameString(gemA.m_name); + + gemModel->AddGem(gemB); + indexB = gemModel->FindIndexByNameString(gemB.m_name); + + gemModel->AddGem(gemC); + indexC = gemModel->FindIndexByNameString(gemC.m_name); + + gemModel->UpdateGemDependencies(); + + EXPECT_FALSE(GemModel::IsAdded(indexA)); + EXPECT_FALSE(GemModel::IsAddedDependency(indexB) || GemModel::IsAddedDependency(indexC)); + + // when a is added + GemModel::SetIsAdded(*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)); + + QVector gemsToAdd = gemModel->GatherGemsToBeAdded(); + EXPECT_TRUE(gemsToAdd.size() == 1); + EXPECT_EQ(GemModel::GetName(gemsToAdd.at(0)), gemA.m_name); + } +} From bf1ebea202c56f47fb4313f5b269e061d6bed7aa Mon Sep 17 00:00:00 2001 From: pereslav Date: Fri, 24 Sep 2021 17:12:58 +0100 Subject: [PATCH 14/18] Moved local variable to a smaller scope Signed-off-by: pereslav --- .../Code/Source/Pipeline/NetworkSpawnableHolderComponent.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Gems/Multiplayer/Code/Source/Pipeline/NetworkSpawnableHolderComponent.cpp b/Gems/Multiplayer/Code/Source/Pipeline/NetworkSpawnableHolderComponent.cpp index 71019a5270..5d677c6101 100644 --- a/Gems/Multiplayer/Code/Source/Pipeline/NetworkSpawnableHolderComponent.cpp +++ b/Gems/Multiplayer/Code/Source/Pipeline/NetworkSpawnableHolderComponent.cpp @@ -45,8 +45,7 @@ namespace Multiplayer { AZ::Transform rootEntityTransform = AZ::Transform::CreateIdentity(); - auto* transformInterface = GetEntity()->GetTransform(); - if(transformInterface) + if(auto* transformInterface = GetEntity()->GetTransform()) { rootEntityTransform = transformInterface->GetWorldTM(); } From 9b14d49f87c27610c86df83acb6b94e5a014b44c Mon Sep 17 00:00:00 2001 From: AMZN-nggieber <52797929+AMZN-nggieber@users.noreply.github.com> Date: Fri, 24 Sep 2021 10:50:45 -0700 Subject: [PATCH 15/18] Adds Inspector to Gem Repo Screen (#4242) * Adds the gem repo screen with the UI built but with mocked data and not connected to the o3de scripts Signed-off-by: nggieber * Changed name of added to enabled, disabled define, removed unused functions Signed-off-by: nggieber * Added Repo Screen Inspector UI Signed-off-by: nggieber * Addressed minor PR feedback Signed-off-by: nggieber * Add some more minor PR changes Signed-off-by: nggieber --- .../Resources/ProjectManager.qss | 29 ++++ .../Source/GemCatalog/GemInspector.cpp | 26 +--- .../Source/GemCatalog/GemInspector.h | 20 +-- .../Source/GemRepo/GemRepoInfo.cpp | 6 +- .../Source/GemRepo/GemRepoInfo.h | 14 +- .../Source/GemRepo/GemRepoInspector.cpp | 144 ++++++++++++++++++ .../Source/GemRepo/GemRepoInspector.h | 59 +++++++ .../Source/GemRepo/GemRepoListView.cpp | 3 +- .../Source/GemRepo/GemRepoListView.h | 3 +- .../Source/GemRepo/GemRepoModel.cpp | 61 ++++++++ .../Source/GemRepo/GemRepoModel.h | 13 +- .../Source/GemRepo/GemRepoScreen.cpp | 12 +- .../Source/GemRepo/GemRepoScreen.h | 3 +- .../ProjectManager/Source/GemsSubWidget.cpp | 45 ++++++ .../ProjectManager/Source/GemsSubWidget.h | 35 +++++ .../ProjectManager/Source/LinkWidget.cpp | 7 +- Code/Tools/ProjectManager/Source/LinkWidget.h | 3 +- .../ProjectManager/Source/PythonBindings.cpp | 12 +- .../Source/PythonBindingsInterface.h | 5 +- .../project_manager_files.cmake | 4 + 20 files changed, 439 insertions(+), 65 deletions(-) create mode 100644 Code/Tools/ProjectManager/Source/GemRepo/GemRepoInspector.cpp create mode 100644 Code/Tools/ProjectManager/Source/GemRepo/GemRepoInspector.h create mode 100644 Code/Tools/ProjectManager/Source/GemsSubWidget.cpp create mode 100644 Code/Tools/ProjectManager/Source/GemsSubWidget.h diff --git a/Code/Tools/ProjectManager/Resources/ProjectManager.qss b/Code/Tools/ProjectManager/Resources/ProjectManager.qss index 30117d6636..957b2b4fa6 100644 --- a/Code/Tools/ProjectManager/Resources/ProjectManager.qss +++ b/Code/Tools/ProjectManager/Resources/ProjectManager.qss @@ -498,6 +498,18 @@ QProgressBar::chunk { font-size: 10px; } +/************** Gems SubWidget **************/ + +#gemSubWidgetTitleLabel { + color: #FFFFFF; + font-size: 16px; +} + +#gemSubWidgetTextLabel { + color: #DDDDDD; + font-size: 10px; +} + /************** Gem Catalog (Inspector) **************/ #GemCatalogInspector { @@ -597,3 +609,20 @@ QProgressBar::chunk { #gemRepoInspector { background: #444444; } + +/************** Gem Repo Inspector **************/ + +#gemRepoInspectorNameLabel { + font-size: 18px; + color: #FFFFFF; +} + +#gemRepoInspectorBodyLabel { + font-size: 12px; + color: #DDDDDD; +} + +#gemRepoInspectorAddInfoTitleLabel { + font-size: 16px; + color: #FFFFFF; +} \ No newline at end of file diff --git a/Code/Tools/ProjectManager/Source/GemCatalog/GemInspector.cpp b/Code/Tools/ProjectManager/Source/GemCatalog/GemInspector.cpp index 282aa2193e..7630e92e88 100644 --- a/Code/Tools/ProjectManager/Source/GemCatalog/GemInspector.cpp +++ b/Code/Tools/ProjectManager/Source/GemCatalog/GemInspector.cpp @@ -8,6 +8,7 @@ #include #include + #include #include #include @@ -89,7 +90,7 @@ namespace O3DE::ProjectManager // Additional information m_versionLabel->setText(QString("Gem Version: %1").arg(m_model->GetVersion(modelIndex))); m_lastUpdatedLabel->setText(QString("Last Updated: %1").arg(m_model->GetLastUpdated(modelIndex))); - m_binarySizeLabel->setText(QString("Binary Size: %1 KB").arg(QString::number(m_model->GetBinarySizeInKB(modelIndex)))); + m_binarySizeLabel->setText(QString("Binary Size: %1 KB").arg(m_model->GetBinarySizeInKB(modelIndex))); m_mainWidget->adjustSize(); m_mainWidget->show(); @@ -185,27 +186,4 @@ namespace O3DE::ProjectManager m_lastUpdatedLabel = CreateStyledLabel(m_mainLayout, 12, s_textColor); m_binarySizeLabel = CreateStyledLabel(m_mainLayout, 12, s_textColor); } - - GemInspector::GemsSubWidget::GemsSubWidget(QWidget* parent) - : QWidget(parent) - { - m_layout = new QVBoxLayout(); - m_layout->setAlignment(Qt::AlignTop); - m_layout->setMargin(0); - setLayout(m_layout); - - m_titleLabel = GemInspector::CreateStyledLabel(m_layout, 16, s_headerColor); - m_textLabel = GemInspector::CreateStyledLabel(m_layout, 10, s_textColor); - m_textLabel->setWordWrap(true); - - m_tagWidget = new TagContainerWidget(); - m_layout->addWidget(m_tagWidget); - } - - void GemInspector::GemsSubWidget::Update(const QString& title, const QString& text, const QStringList& gemNames) - { - m_titleLabel->setText(title); - m_textLabel->setText(text); - m_tagWidget->Update(gemNames); - } } // namespace O3DE::ProjectManager diff --git a/Code/Tools/ProjectManager/Source/GemCatalog/GemInspector.h b/Code/Tools/ProjectManager/Source/GemCatalog/GemInspector.h index 0c41d1033c..ca36cef240 100644 --- a/Code/Tools/ProjectManager/Source/GemCatalog/GemInspector.h +++ b/Code/Tools/ProjectManager/Source/GemCatalog/GemInspector.h @@ -9,10 +9,11 @@ #pragma once #if !defined(Q_MOC_RUN) -#include -#include #include #include +#include +#include + #include #include #include @@ -43,21 +44,6 @@ namespace O3DE::ProjectManager void OnSelectionChanged(const QItemSelection& selected, const QItemSelection& deselected); private: - // Title, description and tag widget container used for the depending and conflicting gems - class GemsSubWidget - : public QWidget - { - public: - GemsSubWidget(QWidget* parent = nullptr); - void Update(const QString& title, const QString& text, const QStringList& gemNames); - - private: - QLabel* m_titleLabel = nullptr; - QLabel* m_textLabel = nullptr; - QVBoxLayout* m_layout = nullptr; - TagContainerWidget* m_tagWidget = nullptr; - }; - void InitMainWidget(); GemModel* m_model = nullptr; diff --git a/Code/Tools/ProjectManager/Source/GemRepo/GemRepoInfo.cpp b/Code/Tools/ProjectManager/Source/GemRepo/GemRepoInfo.cpp index 3e524d8ec8..88216398db 100644 --- a/Code/Tools/ProjectManager/Source/GemRepo/GemRepoInfo.cpp +++ b/Code/Tools/ProjectManager/Source/GemRepo/GemRepoInfo.cpp @@ -11,10 +11,12 @@ namespace O3DE::ProjectManager { GemRepoInfo::GemRepoInfo( - const QString& name, const QString& creator, const QString& summary, const QDateTime& lastUpdated, bool isEnabled = true) + const QString& name, + const QString& creator, + const QDateTime& lastUpdated, + bool isEnabled = true) : m_name(name) , m_creator(creator) - , m_summary(summary) , m_lastUpdated(lastUpdated) , m_isEnabled(isEnabled) { diff --git a/Code/Tools/ProjectManager/Source/GemRepo/GemRepoInfo.h b/Code/Tools/ProjectManager/Source/GemRepo/GemRepoInfo.h index 6f4f828951..14c76bd0c2 100644 --- a/Code/Tools/ProjectManager/Source/GemRepo/GemRepoInfo.h +++ b/Code/Tools/ProjectManager/Source/GemRepo/GemRepoInfo.h @@ -19,19 +19,25 @@ namespace O3DE::ProjectManager { public: GemRepoInfo() = default; - GemRepoInfo(const QString& name, const QString& creator, const QString& summary, const QDateTime& lastUpdated, bool isEnabled); + GemRepoInfo( + const QString& name, + const QString& creator, + const QDateTime& lastUpdated, + bool isEnabled); bool IsValid() const; bool operator<(const GemRepoInfo& gemRepoInfo) const; - QString m_path; + QString m_path = ""; QString m_name = "Unknown Gem Repo Name"; QString m_creator = "Unknown Creator"; bool m_isEnabled = false; //! Is the repo currently enabled for this engine? QString m_summary = "No summary provided."; - QString m_directoryLink; - QString m_repoLink; + QString m_additionalInfo = ""; + QString m_directoryLink = ""; + QString m_repoLink = ""; + QStringList m_includedGemPaths = {}; QDateTime m_lastUpdated; }; } // namespace O3DE::ProjectManager diff --git a/Code/Tools/ProjectManager/Source/GemRepo/GemRepoInspector.cpp b/Code/Tools/ProjectManager/Source/GemRepo/GemRepoInspector.cpp new file mode 100644 index 0000000000..93f5890b94 --- /dev/null +++ b/Code/Tools/ProjectManager/Source/GemRepo/GemRepoInspector.cpp @@ -0,0 +1,144 @@ +/* + * 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 + +namespace O3DE::ProjectManager +{ + GemRepoInspector::GemRepoInspector(GemRepoModel* model, QWidget* parent) + : QScrollArea(parent) + , m_model(model) + { + setObjectName("gemRepoInspector"); + setWidgetResizable(true); + setHorizontalScrollBarPolicy(Qt::ScrollBarAlwaysOff); + setVerticalScrollBarPolicy(Qt::ScrollBarAsNeeded); + + m_mainWidget = new QWidget(); + setWidget(m_mainWidget); + + m_mainLayout = new QVBoxLayout(); + m_mainLayout->setMargin(15); + m_mainLayout->setAlignment(Qt::AlignTop); + m_mainWidget->setLayout(m_mainLayout); + + InitMainWidget(); + + connect(m_model->GetSelectionModel(), &QItemSelectionModel::selectionChanged, this, &GemRepoInspector::OnSelectionChanged); + Update({}); + } + + void GemRepoInspector::OnSelectionChanged(const QItemSelection& selected, [[maybe_unused]] const QItemSelection& deselected) + { + const QModelIndexList selectedIndices = selected.indexes(); + if (selectedIndices.empty()) + { + Update({}); + return; + } + + Update(selectedIndices[0]); + } + + void GemRepoInspector::Update(const QModelIndex& modelIndex) + { + if (!modelIndex.isValid()) + { + m_mainWidget->hide(); + } + + // Repo name and url link + m_nameLabel->setText(m_model->GetName(modelIndex)); + m_repoLinkLabel->setText(m_model->GetRepoLink(modelIndex)); + m_repoLinkLabel->SetUrl(m_model->GetRepoLink(modelIndex)); + + // Repo summary + m_summaryLabel->setText(m_model->GetSummary(modelIndex)); + m_summaryLabel->adjustSize(); + + // Additional information + if (m_model->HasAdditionalInfo(modelIndex)) + { + m_addInfoTitleLabel->show(); + m_addInfoTextLabel->show(); + + m_addInfoSpacer->changeSize(0, 20, QSizePolicy::Fixed, QSizePolicy::Fixed); + + m_addInfoTextLabel->setText(m_model->GetAdditionalInfo(modelIndex)); + } + else + { + m_addInfoTitleLabel->hide(); + m_addInfoTextLabel->hide(); + + m_addInfoSpacer->changeSize(0, 0, QSizePolicy::Fixed, QSizePolicy::Fixed); + } + + // Included Gems + m_includedGems->Update(tr("Included Gems"), "", m_model->GetIncludedGemNames(modelIndex)); + + m_mainWidget->adjustSize(); + m_mainWidget->show(); + } + + void GemRepoInspector::InitMainWidget() + { + // Repo name and url link + m_nameLabel = new QLabel(); + m_nameLabel->setObjectName("gemRepoInspectorNameLabel"); + m_mainLayout->addWidget(m_nameLabel); + + m_repoLinkLabel = new LinkLabel(tr("Repo Url"), QUrl(""), 12, this); + m_mainLayout->addWidget(m_repoLinkLabel); + m_mainLayout->addSpacing(5); + + // Repo summary + m_summaryLabel = new QLabel(); + m_summaryLabel->setObjectName("gemRepoInspectorBodyLabel"); + m_summaryLabel->setWordWrap(true); + m_summaryLabel->setTextInteractionFlags(Qt::TextBrowserInteraction); + m_summaryLabel->setOpenExternalLinks(true); + m_mainLayout->addWidget(m_summaryLabel); + m_mainLayout->addSpacing(20); + + // Separating line + QFrame* hLine = new QFrame(); + hLine->setFrameShape(QFrame::HLine); + hLine->setObjectName("horizontalSeparatingLine"); + m_mainLayout->addWidget(hLine); + m_mainLayout->addSpacing(10); + + // Additional information + m_addInfoTitleLabel = new QLabel(); + m_addInfoTitleLabel->setObjectName("gemRepoInspectorAddInfoTitleLabel"); + m_addInfoTitleLabel->setText(tr("Additional Information")); + m_mainLayout->addWidget(m_addInfoTitleLabel); + + m_addInfoTextLabel = new QLabel(); + m_addInfoTextLabel->setObjectName("gemRepoInspectorBodyLabel"); + m_addInfoTextLabel->setWordWrap(true); + m_addInfoTextLabel->setTextInteractionFlags(Qt::TextBrowserInteraction); + m_addInfoTextLabel->setOpenExternalLinks(true); + m_mainLayout->addWidget(m_addInfoTextLabel); + + // Conditional spacing for additional info section + m_addInfoSpacer = new QSpacerItem(0, 0, QSizePolicy::Expanding); + m_mainLayout->addSpacerItem(m_addInfoSpacer); + + // Included Gems + m_includedGems = new GemsSubWidget(); + m_mainLayout->addWidget(m_includedGems); + m_mainLayout->addSpacing(20); + } +} // namespace O3DE::ProjectManager diff --git a/Code/Tools/ProjectManager/Source/GemRepo/GemRepoInspector.h b/Code/Tools/ProjectManager/Source/GemRepo/GemRepoInspector.h new file mode 100644 index 0000000000..a14472e6a6 --- /dev/null +++ b/Code/Tools/ProjectManager/Source/GemRepo/GemRepoInspector.h @@ -0,0 +1,59 @@ +/* + * 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 + * + */ + +#pragma once + +#if !defined(Q_MOC_RUN) +#include +#include +#include + +#include +#include +#include +#include +#endif + +QT_FORWARD_DECLARE_CLASS(QVBoxLayout) +QT_FORWARD_DECLARE_CLASS(QLabel) + +namespace O3DE::ProjectManager +{ + class GemRepoInspector : public QScrollArea + { + Q_OBJECT // AUTOMOC + + public : explicit GemRepoInspector(GemRepoModel* model, QWidget* parent = nullptr); + ~GemRepoInspector() = default; + + void Update(const QModelIndex& modelIndex); + + private slots: + void OnSelectionChanged(const QItemSelection& selected, const QItemSelection& deselected); + + private: + void InitMainWidget(); + + GemRepoModel* m_model = nullptr; + QWidget* m_mainWidget = nullptr; + QVBoxLayout* m_mainLayout = nullptr; + + // General info section + QLabel* m_nameLabel = nullptr; + LinkLabel* m_repoLinkLabel = nullptr; + QLabel* m_summaryLabel = nullptr; + + // Additional information + QLabel* m_addInfoTitleLabel = nullptr; + QLabel* m_addInfoTextLabel = nullptr; + QSpacerItem* m_addInfoSpacer = nullptr; + + // Included Gems + GemsSubWidget* m_includedGems = nullptr; + }; +} // namespace O3DE::ProjectManager diff --git a/Code/Tools/ProjectManager/Source/GemRepo/GemRepoListView.cpp b/Code/Tools/ProjectManager/Source/GemRepo/GemRepoListView.cpp index 519d52cb35..54d5b337e5 100644 --- a/Code/Tools/ProjectManager/Source/GemRepo/GemRepoListView.cpp +++ b/Code/Tools/ProjectManager/Source/GemRepo/GemRepoListView.cpp @@ -11,13 +11,14 @@ namespace O3DE::ProjectManager { - GemRepoListView::GemRepoListView(QAbstractItemModel* model, QWidget* parent) + GemRepoListView::GemRepoListView(QAbstractItemModel* model, QItemSelectionModel* selectionModel, QWidget* parent) : QListView(parent) { setObjectName("gemRepoListView"); setVerticalScrollMode(QAbstractItemView::ScrollPerPixel); setModel(model); + setSelectionModel(selectionModel); setItemDelegate(new GemRepoItemDelegate(model, this)); } } // namespace O3DE::ProjectManager diff --git a/Code/Tools/ProjectManager/Source/GemRepo/GemRepoListView.h b/Code/Tools/ProjectManager/Source/GemRepo/GemRepoListView.h index 0fd5d5c180..b71b49f390 100644 --- a/Code/Tools/ProjectManager/Source/GemRepo/GemRepoListView.h +++ b/Code/Tools/ProjectManager/Source/GemRepo/GemRepoListView.h @@ -10,6 +10,7 @@ #if !defined(Q_MOC_RUN) #include +#include #endif QT_FORWARD_DECLARE_CLASS(QAbstractItemModel) @@ -22,7 +23,7 @@ namespace O3DE::ProjectManager Q_OBJECT // AUTOMOC public: - explicit GemRepoListView(QAbstractItemModel* model, QWidget* parent = nullptr); + explicit GemRepoListView(QAbstractItemModel* model, QItemSelectionModel* selectionModel, QWidget* parent = nullptr); ~GemRepoListView() = default; }; } // namespace O3DE::ProjectManager diff --git a/Code/Tools/ProjectManager/Source/GemRepo/GemRepoModel.cpp b/Code/Tools/ProjectManager/Source/GemRepo/GemRepoModel.cpp index 7a42c135e9..61ac6dc8a3 100644 --- a/Code/Tools/ProjectManager/Source/GemRepo/GemRepoModel.cpp +++ b/Code/Tools/ProjectManager/Source/GemRepo/GemRepoModel.cpp @@ -7,8 +7,10 @@ */ #include +#include #include +#include namespace O3DE::ProjectManager { @@ -16,6 +18,7 @@ namespace O3DE::ProjectManager : QStandardItemModel(parent) { m_selectionModel = new QItemSelectionModel(this, parent); + m_gemModel = new GemModel(this); } QItemSelectionModel* GemRepoModel::GetSelectionModel() const @@ -37,8 +40,17 @@ namespace O3DE::ProjectManager item->setData(gemRepoInfo.m_repoLink, RoleRepoLink); item->setData(gemRepoInfo.m_lastUpdated, RoleLastUpdated); item->setData(gemRepoInfo.m_path, RolePath); + item->setData(gemRepoInfo.m_additionalInfo, RoleAdditionalInfo); + item->setData(gemRepoInfo.m_includedGemPaths, RoleIncludedGems); appendRow(item); + + QVector includedGemInfos = GetIncludedGemInfos(item->index()); + + for (const GemInfo& gemInfo : includedGemInfos) + { + m_gemModel->AddGem(gemInfo); + } } void GemRepoModel::Clear() @@ -61,6 +73,11 @@ namespace O3DE::ProjectManager return modelIndex.data(RoleSummary).toString(); } + QString GemRepoModel::GetAdditionalInfo(const QModelIndex& modelIndex) + { + return modelIndex.data(RoleAdditionalInfo).toString(); + } + QString GemRepoModel::GetDirectoryLink(const QModelIndex& modelIndex) { return modelIndex.data(RoleDirectoryLink).toString(); @@ -81,6 +98,45 @@ namespace O3DE::ProjectManager return modelIndex.data(RolePath).toString(); } + QStringList GemRepoModel::GetIncludedGemPaths(const QModelIndex& modelIndex) + { + return modelIndex.data(RoleIncludedGems).toStringList(); + } + + QStringList GemRepoModel::GetIncludedGemNames(const QModelIndex& modelIndex) + { + QStringList gemNames; + QVector gemInfos = GetIncludedGemInfos(modelIndex); + + for (const GemInfo& gemInfo : gemInfos) + { + gemNames.append(gemInfo.m_displayName); + } + + return gemNames; + } + + QVector GemRepoModel::GetIncludedGemInfos(const QModelIndex& modelIndex) + { + QVector allGemInfos; + QStringList repoGemPaths = GetIncludedGemPaths(modelIndex); + + for (const QString& gemPath : repoGemPaths) + { + AZ::Outcome gemInfoResult = PythonBindingsInterface::Get()->GetGemInfo(gemPath); + if (gemInfoResult.IsSuccess()) + { + allGemInfos.append(gemInfoResult.GetValue()); + } + else + { + QMessageBox::critical(nullptr, tr("Gem Not Found"), tr("Cannot find info for gem %1.").arg(gemPath)); + } + } + + return allGemInfos; + } + bool GemRepoModel::IsEnabled(const QModelIndex& modelIndex) { return modelIndex.data(RoleIsEnabled).toBool(); @@ -91,4 +147,9 @@ namespace O3DE::ProjectManager model.setData(modelIndex, isEnabled, RoleIsEnabled); } + bool GemRepoModel::HasAdditionalInfo(const QModelIndex& modelIndex) + { + return !modelIndex.data(RoleAdditionalInfo).toString().isEmpty(); + } + } // namespace O3DE::ProjectManager diff --git a/Code/Tools/ProjectManager/Source/GemRepo/GemRepoModel.h b/Code/Tools/ProjectManager/Source/GemRepo/GemRepoModel.h index 2f1537d339..ad139bc12b 100644 --- a/Code/Tools/ProjectManager/Source/GemRepo/GemRepoModel.h +++ b/Code/Tools/ProjectManager/Source/GemRepo/GemRepoModel.h @@ -11,6 +11,7 @@ #if !defined(Q_MOC_RUN) #include #include +#include #endif QT_FORWARD_DECLARE_CLASS(QItemSelectionModel) @@ -32,13 +33,19 @@ namespace O3DE::ProjectManager static QString GetName(const QModelIndex& modelIndex); static QString GetCreator(const QModelIndex& modelIndex); static QString GetSummary(const QModelIndex& modelIndex); + static QString GetAdditionalInfo(const QModelIndex& modelIndex); static QString GetDirectoryLink(const QModelIndex& modelIndex); static QString GetRepoLink(const QModelIndex& modelIndex); static QDateTime GetLastUpdated(const QModelIndex& modelIndex); static QString GetPath(const QModelIndex& modelIndex); + static QStringList GetIncludedGemPaths(const QModelIndex& modelIndex); + static QStringList GetIncludedGemNames(const QModelIndex& modelIndex); + static QVector GetIncludedGemInfos(const QModelIndex& modelIndex); + static bool IsEnabled(const QModelIndex& modelIndex); static void SetEnabled(QAbstractItemModel& model, const QModelIndex& modelIndex, bool isEnabled); + static bool HasAdditionalInfo(const QModelIndex& modelIndex); private: enum UserRole @@ -50,9 +57,13 @@ namespace O3DE::ProjectManager RoleDirectoryLink, RoleRepoLink, RoleLastUpdated, - RolePath + RolePath, + RoleAdditionalInfo, + RoleIncludedGems, }; QItemSelectionModel* m_selectionModel = nullptr; + + GemModel* m_gemModel = nullptr; }; } // namespace O3DE::ProjectManager diff --git a/Code/Tools/ProjectManager/Source/GemRepo/GemRepoScreen.cpp b/Code/Tools/ProjectManager/Source/GemRepo/GemRepoScreen.cpp index 82de53a0d0..c0b17904f8 100644 --- a/Code/Tools/ProjectManager/Source/GemRepo/GemRepoScreen.cpp +++ b/Code/Tools/ProjectManager/Source/GemRepo/GemRepoScreen.cpp @@ -10,6 +10,7 @@ #include #include #include +#include #include #include @@ -40,10 +41,6 @@ namespace O3DE::ProjectManager hLayout->addSpacing(60); - m_gemRepoInspector = new QFrame(this); - m_gemRepoInspector->setObjectName(tr("gemRepoInspector")); - m_gemRepoInspector->setFixedWidth(240); - QVBoxLayout* middleVLayout = new QVBoxLayout(); middleVLayout->setMargin(0); middleVLayout->setSpacing(0); @@ -99,10 +96,13 @@ namespace O3DE::ProjectManager m_gemRepoHeaderTable->horizontalHeader()->setStyleSheet("QHeaderView::section { background-color:transparent; color:white; font-size:12px; text-align:left; border-style:none; }"); middleVLayout->addWidget(m_gemRepoHeaderTable); - m_gemRepoListView = new GemRepoListView(m_gemRepoModel, this); + m_gemRepoListView = new GemRepoListView(m_gemRepoModel, m_gemRepoModel->GetSelectionModel(), this); middleVLayout->addWidget(m_gemRepoListView); hLayout->addLayout(middleVLayout); + + m_gemRepoInspector = new GemRepoInspector(m_gemRepoModel, this); + m_gemRepoInspector->setFixedWidth(240); hLayout->addWidget(m_gemRepoInspector); Reinit(); @@ -134,7 +134,7 @@ namespace O3DE::ProjectManager } else { - QMessageBox::critical(this, tr("Operation failed"), QString("Cannot retrieve gem repos for engine.\n\nError:\n%2").arg(allGemRepoInfosResult.GetError().c_str())); + QMessageBox::critical(this, tr("Operation failed"), tr("Cannot retrieve gem repos for engine.\n\nError:\n%2").arg(allGemRepoInfosResult.GetError().c_str())); } } diff --git a/Code/Tools/ProjectManager/Source/GemRepo/GemRepoScreen.h b/Code/Tools/ProjectManager/Source/GemRepo/GemRepoScreen.h index b5316db84f..f7d943fc2a 100644 --- a/Code/Tools/ProjectManager/Source/GemRepo/GemRepoScreen.h +++ b/Code/Tools/ProjectManager/Source/GemRepo/GemRepoScreen.h @@ -19,6 +19,7 @@ QT_FORWARD_DECLARE_CLASS(QTableWidget) namespace O3DE::ProjectManager { + QT_FORWARD_DECLARE_CLASS(GemRepoInspector) QT_FORWARD_DECLARE_CLASS(GemRepoListView) QT_FORWARD_DECLARE_CLASS(GemRepoModel) @@ -40,7 +41,7 @@ namespace O3DE::ProjectManager QTableWidget* m_gemRepoHeaderTable = nullptr; QHeaderView* m_gemRepoListHeader = nullptr; GemRepoListView* m_gemRepoListView = nullptr; - QFrame* m_gemRepoInspector = nullptr; + GemRepoInspector* m_gemRepoInspector = nullptr; GemRepoModel* m_gemRepoModel = nullptr; QLabel* m_lastAllUpdateLabel; diff --git a/Code/Tools/ProjectManager/Source/GemsSubWidget.cpp b/Code/Tools/ProjectManager/Source/GemsSubWidget.cpp new file mode 100644 index 0000000000..eb24008eb1 --- /dev/null +++ b/Code/Tools/ProjectManager/Source/GemsSubWidget.cpp @@ -0,0 +1,45 @@ +/* + * 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 + +namespace O3DE::ProjectManager +{ + GemsSubWidget::GemsSubWidget(QWidget* parent) + : QWidget(parent) + { + m_layout = new QVBoxLayout(); + m_layout->setAlignment(Qt::AlignTop); + m_layout->setMargin(0); + setLayout(m_layout); + + m_titleLabel = new QLabel(); + m_titleLabel->setObjectName("gemSubWidgetTitleLabel"); + m_layout->addWidget(m_titleLabel); + + m_textLabel = new QLabel(); + m_textLabel->setObjectName("gemSubWidgetTextLabel"); + m_textLabel->setWordWrap(true); + m_layout->addWidget(m_textLabel); + + m_tagWidget = new TagContainerWidget(); + m_layout->addWidget(m_tagWidget); + } + + void GemsSubWidget::Update(const QString& title, const QString& text, const QStringList& gemNames) + { + m_titleLabel->setText(title); + m_textLabel->setText(text); + m_tagWidget->Update(gemNames); + } +} // namespace O3DE::ProjectManager diff --git a/Code/Tools/ProjectManager/Source/GemsSubWidget.h b/Code/Tools/ProjectManager/Source/GemsSubWidget.h new file mode 100644 index 0000000000..1b10ec8861 --- /dev/null +++ b/Code/Tools/ProjectManager/Source/GemsSubWidget.h @@ -0,0 +1,35 @@ +/* + * 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 + * + */ + +#pragma once + +#if !defined(Q_MOC_RUN) +#include +#include +#endif + +QT_FORWARD_DECLARE_CLASS(QVBoxLayout) +QT_FORWARD_DECLARE_CLASS(QLabel) + +namespace O3DE::ProjectManager +{ + // Title, description and tag widget container used for the depending and conflicting gems + class GemsSubWidget + : public QWidget + { + public: + GemsSubWidget(QWidget* parent = nullptr); + void Update(const QString& title, const QString& text, const QStringList& gemNames); + + private: + QLabel* m_titleLabel = nullptr; + QLabel* m_textLabel = nullptr; + QVBoxLayout* m_layout = nullptr; + TagContainerWidget* m_tagWidget = nullptr; + }; +} // namespace O3DE::ProjectManager diff --git a/Code/Tools/ProjectManager/Source/LinkWidget.cpp b/Code/Tools/ProjectManager/Source/LinkWidget.cpp index ccee7e8ec6..9c8c78ed37 100644 --- a/Code/Tools/ProjectManager/Source/LinkWidget.cpp +++ b/Code/Tools/ProjectManager/Source/LinkWidget.cpp @@ -14,9 +14,10 @@ namespace O3DE::ProjectManager { - LinkLabel::LinkLabel(const QString& text, const QUrl& url, QWidget* parent) + LinkLabel::LinkLabel(const QString& text, const QUrl& url, int fontSize, QWidget* parent) : QLabel(text, parent) , m_url(url) + , m_fontSize(fontSize) { SetDefaultStyle(); } @@ -33,7 +34,7 @@ namespace O3DE::ProjectManager void LinkLabel::enterEvent([[maybe_unused]] QEvent* event) { - setStyleSheet("font-size: 10px; color: #94D2FF; text-decoration: underline;"); + setStyleSheet(QString("font-size: %1px; color: #94D2FF; text-decoration: underline;").arg(m_fontSize)); } void LinkLabel::leaveEvent([[maybe_unused]] QEvent* event) @@ -48,6 +49,6 @@ namespace O3DE::ProjectManager void LinkLabel::SetDefaultStyle() { - setStyleSheet("font-size: 10px; color: #94D2FF;"); + setStyleSheet(QString("font-size: %1px; color: #94D2FF;").arg(m_fontSize)); } } // namespace O3DE::ProjectManager diff --git a/Code/Tools/ProjectManager/Source/LinkWidget.h b/Code/Tools/ProjectManager/Source/LinkWidget.h index a50007cf1f..eb0b9bb528 100644 --- a/Code/Tools/ProjectManager/Source/LinkWidget.h +++ b/Code/Tools/ProjectManager/Source/LinkWidget.h @@ -25,7 +25,7 @@ namespace O3DE::ProjectManager Q_OBJECT // AUTOMOC public: - LinkLabel(const QString& text = {}, const QUrl& url = {}, QWidget* parent = nullptr); + LinkLabel(const QString& text = {}, const QUrl& url = {}, int fontSize = 10, QWidget* parent = nullptr); void SetUrl(const QUrl& url); @@ -40,5 +40,6 @@ namespace O3DE::ProjectManager private: QUrl m_url; + int m_fontSize; }; } // namespace O3DE::ProjectManager diff --git a/Code/Tools/ProjectManager/Source/PythonBindings.cpp b/Code/Tools/ProjectManager/Source/PythonBindings.cpp index 9f0c6a51ba..284ed9dcec 100644 --- a/Code/Tools/ProjectManager/Source/PythonBindings.cpp +++ b/Code/Tools/ProjectManager/Source/PythonBindings.cpp @@ -954,8 +954,16 @@ namespace O3DE::ProjectManager return AZ::Failure(result.GetError().c_str()); } #else - gemRepos.push_back(GemRepoInfo("JohnCreates", "John Smith", "", QDateTime(QDate(2021, 8, 31), QTime(11, 57)), true)); - gemRepos.push_back(GemRepoInfo("JanesGems", "Jane Doe", "", QDateTime(QDate(2021, 9, 10), QTime(18, 23)), false)); + GemRepoInfo mockJohnRepo("JohnCreates", "John Smith", QDateTime(QDate(2021, 8, 31), QTime(11, 57)), true); + mockJohnRepo.m_summary = "John's Summary. Lorem ipsum dolor sit amet, consectetur adipiscing elit. Fusce sollicitudin dapibus urna"; + mockJohnRepo.m_repoLink = "https://github.com/o3de/o3de"; + mockJohnRepo.m_additionalInfo = "John's additional info. Lorem ipsum dolor sit amet, consectetur adipiscing elit. Fusce sollicitu."; + gemRepos.push_back(mockJohnRepo); + + GemRepoInfo mockJaneRepo("JanesGems", "Jane Doe", QDateTime(QDate(2021, 9, 10), QTime(18, 23)), false); + mockJaneRepo.m_summary = "Jane's Summary."; + mockJaneRepo.m_repoLink = "https://github.com/o3de/o3de.org"; + gemRepos.push_back(mockJaneRepo); #endif // MOCK_GEM_REPO_INFO std::sort(gemRepos.begin(), gemRepos.end()); diff --git a/Code/Tools/ProjectManager/Source/PythonBindingsInterface.h b/Code/Tools/ProjectManager/Source/PythonBindingsInterface.h index 9fd3002f93..ccf217d25b 100644 --- a/Code/Tools/ProjectManager/Source/PythonBindingsInterface.h +++ b/Code/Tools/ProjectManager/Source/PythonBindingsInterface.h @@ -56,8 +56,9 @@ namespace O3DE::ProjectManager // Gems /** - * Get info about a Gem - * @param projectPath the absolute path to the Gem + * Get info about a Gem. + * @param path The absolute path to the Gem + * @param projectPath (Optional) The absolute path to the Gem project * @return an outcome with GemInfo on success */ virtual AZ::Outcome GetGemInfo(const QString& path, const QString& projectPath = {}) = 0; diff --git a/Code/Tools/ProjectManager/project_manager_files.cmake b/Code/Tools/ProjectManager/project_manager_files.cmake index 7a336972e0..f71ae290e7 100644 --- a/Code/Tools/ProjectManager/project_manager_files.cmake +++ b/Code/Tools/ProjectManager/project_manager_files.cmake @@ -27,6 +27,8 @@ set(FILES Source/FormFolderBrowseEditWidget.cpp Source/FormImageBrowseEditWidget.h Source/FormImageBrowseEditWidget.cpp + Source/GemsSubWidget.h + Source/GemsSubWidget.cpp Source/PathValidator.h Source/PathValidator.cpp Source/ProjectManagerWindow.h @@ -104,6 +106,8 @@ set(FILES Source/GemRepo/GemRepoScreen.cpp Source/GemRepo/GemRepoInfo.h Source/GemRepo/GemRepoInfo.cpp + Source/GemRepo/GemRepoInspector.h + Source/GemRepo/GemRepoInspector.cpp Source/GemRepo/GemRepoItemDelegate.h Source/GemRepo/GemRepoItemDelegate.cpp Source/GemRepo/GemRepoListView.h From caf161ec638fdab6814366f611486bdff4f292d2 Mon Sep 17 00:00:00 2001 From: LesaelR <89800757+LesaelR@users.noreply.github.com> Date: Fri, 24 Sep 2021 13:14:31 -0700 Subject: [PATCH 16/18] Signed-off-by: LesaelR (#4278) * Signed-off-by: LesaelR * Removing the un-needed sandbox marks. Signed-off-by: LesaelR --- .../PythonTests/assetpipeline/fbx_tests/CMakeLists.txt | 9 +++++++++ .../Gem/PythonTests/assetpipeline/fbx_tests/fbx_tests.py | 5 +++++ 2 files changed, 14 insertions(+) diff --git a/AutomatedTesting/Gem/PythonTests/assetpipeline/fbx_tests/CMakeLists.txt b/AutomatedTesting/Gem/PythonTests/assetpipeline/fbx_tests/CMakeLists.txt index 4a26500ee2..52682e70bb 100644 --- a/AutomatedTesting/Gem/PythonTests/assetpipeline/fbx_tests/CMakeLists.txt +++ b/AutomatedTesting/Gem/PythonTests/assetpipeline/fbx_tests/CMakeLists.txt @@ -17,5 +17,14 @@ if(PAL_TRAIT_BUILD_TESTS_SUPPORTED AND PAL_TRAIT_BUILD_HOST_TOOLS) AZ::AssetProcessorBatch AZ::AssetProcessor ) + + ly_add_pytest( + NAME AssetPipelineTests.Fbx_Tests + PATH ${CMAKE_CURRENT_LIST_DIR}/fbx_test/fbx_test.py + TEST_SUITE sandbox + RUNTIME_DEPENDENCIES + AZ::AssetProcessorBatch + AZ::AssetProcessor + ) endif() diff --git a/AutomatedTesting/Gem/PythonTests/assetpipeline/fbx_tests/fbx_tests.py b/AutomatedTesting/Gem/PythonTests/assetpipeline/fbx_tests/fbx_tests.py index 2b90f53fc3..5cb61da68e 100755 --- a/AutomatedTesting/Gem/PythonTests/assetpipeline/fbx_tests/fbx_tests.py +++ b/AutomatedTesting/Gem/PythonTests/assetpipeline/fbx_tests/fbx_tests.py @@ -34,11 +34,13 @@ logger = logging.getLogger(__name__) targetProjects = ["AutomatedTesting"] @pytest.fixture +@pytest.mark.SUITE_sandbox def local_resources(request, workspace, ap_setup_fixture): ap_setup_fixture["tests_dir"] = os.path.dirname(os.path.realpath(__file__)) @dataclass +@pytest.mark.SUITE_sandbox class BlackboxAssetTest: test_name: str asset_folder: str @@ -338,9 +340,11 @@ blackbox_fbx_special_tests = [ @pytest.mark.usefixtures("local_resources") @pytest.mark.parametrize("project", targetProjects) @pytest.mark.assetpipeline +@pytest.mark.SUITE_sandbox class TestsFBX_AllPlatforms(object): @pytest.mark.BAT + @pytest.mark.SUITE_sandbox @pytest.mark.parametrize("blackbox_param", blackbox_fbx_tests) def test_FBXBlackboxTest_SourceFiles_Processed_ResultInExpectedProducts(self, workspace, ap_setup_fixture, asset_processor, project, @@ -359,6 +363,7 @@ class TestsFBX_AllPlatforms(object): asset_processor, project, blackbox_param) @pytest.mark.BAT + @pytest.mark.SUITE_sandbox @pytest.mark.parametrize("blackbox_param", blackbox_fbx_special_tests) def test_FBXBlackboxTest_AssetInfoModified_AssetReprocessed_ResultInExpectedProducts(self, workspace, ap_setup_fixture, From 8c4ff8802cdf72d120a5930a94fa3c5f91d7a98b Mon Sep 17 00:00:00 2001 From: Gene Walters Date: Fri, 24 Sep 2021 13:25:33 -0700 Subject: [PATCH 17/18] Small fix to use AZ_CRC_CE Signed-off-by: Gene Walters --- .../Include/Multiplayer/Components/NetworkCharacterComponent.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gems/Multiplayer/Code/Include/Multiplayer/Components/NetworkCharacterComponent.h b/Gems/Multiplayer/Code/Include/Multiplayer/Components/NetworkCharacterComponent.h index 511a29c683..9af22b1fdf 100644 --- a/Gems/Multiplayer/Code/Include/Multiplayer/Components/NetworkCharacterComponent.h +++ b/Gems/Multiplayer/Code/Include/Multiplayer/Components/NetworkCharacterComponent.h @@ -58,7 +58,7 @@ namespace Multiplayer static void GetRequiredServices(AZ::ComponentDescriptor::DependencyArrayType& required) { NetworkCharacterComponentBase::GetRequiredServices(required); - required.push_back(AZ_CRC("PhysXCharacterControllerService", 0x428de4fa)); + required.push_back(AZ_CRC_CE("PhysXCharacterControllerService")); } // AZ::Component From 2ce7bbd945815d62ea29fa660466d47840e36e85 Mon Sep 17 00:00:00 2001 From: lumberyard-employee-dm <56135373+lumberyard-employee-dm@users.noreply.github.com> Date: Fri, 24 Sep 2021 17:36:26 -0500 Subject: [PATCH 18/18] Moved the SettingsRegistryFileReader class to more general IO Reader (#4189) The new FileReader class allows using either a FileIOBase or SystemFile classes for reading files. If a non-nullptr FileIOBase is supplied, the FileIOBase is used, otherwise SystemFile is used. Updated the SettingsRegistryMergeUtils `MergeSettingsToRegistry_ConfigFile` function to use the FileReader. This allows .cfg such as the autoexec.cfg to be read using FileIO, therefore allowing it to be read from the engine.pak file. Because the AZ::Console::ExecuteConfigFile uses the `MergeSettingsToRegistry_ConfigFile` function for merging *.cfg files to the SettingsRegistry, this allows the AZ::Console to run console commands from files within pak files thanks to the ArchiveFileIO being the default FileIOBase instance Signed-off-by: lumberyard-employee-dm <56135373+lumberyard-employee-dm@users.noreply.github.com> --- .../Framework/AzCore/AzCore/IO/FileReader.cpp | 200 ++++++++++++++++++ Code/Framework/AzCore/AzCore/IO/FileReader.h | 92 ++++++++ .../Framework/AzCore/AzCore/IO/SystemFile.cpp | 4 +- Code/Framework/AzCore/AzCore/IO/SystemFile.h | 4 +- .../AzCore/Settings/SettingsRegistryImpl.cpp | 115 +--------- .../Settings/SettingsRegistryMergeUtils.cpp | 36 +++- .../Settings/SettingsRegistryMergeUtils.h | 9 + .../AzCore/AzCore/azcore_files.cmake | 2 + .../AzCore/Tests/IO/FileReaderTests.cpp | 72 +++++++ .../AzCore/Tests/azcoretests_files.cmake | 1 + 10 files changed, 415 insertions(+), 120 deletions(-) create mode 100644 Code/Framework/AzCore/AzCore/IO/FileReader.cpp create mode 100644 Code/Framework/AzCore/AzCore/IO/FileReader.h create mode 100644 Code/Framework/AzCore/Tests/IO/FileReaderTests.cpp diff --git a/Code/Framework/AzCore/AzCore/IO/FileReader.cpp b/Code/Framework/AzCore/AzCore/IO/FileReader.cpp new file mode 100644 index 0000000000..94118cdfe4 --- /dev/null +++ b/Code/Framework/AzCore/AzCore/IO/FileReader.cpp @@ -0,0 +1,200 @@ +/* + * 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 + +namespace AZ::IO +{ + FileReader::FileReader() = default; + + FileReader::FileReader(AZ::IO::FileIOBase* fileIoBase, const char* filePath) + { + Open(fileIoBase, filePath); + } + + FileReader::~FileReader() + { + Close(); + } + + FileReader::FileReader(FileReader&& other) + { + AZStd::swap(m_file, other.m_file); + AZStd::swap(m_fileIoBase, other.m_fileIoBase); + } + + FileReader& FileReader::operator=(FileReader&& other) + { + // Close the current file and take over other file + Close(); + m_file = AZStd::move(other.m_file); + m_fileIoBase = AZStd::move(other.m_fileIoBase); + other.m_file = AZStd::monostate{}; + other.m_fileIoBase = {}; + + return *this; + } + + bool FileReader::Open(AZ::IO::FileIOBase* fileIoBase, const char* filePath) + { + // Close file if the FileReader has an instance open + Close(); + + if (fileIoBase != nullptr) + { + AZ::IO::HandleType fileHandle; + if (fileIoBase->Open(filePath, IO::OpenMode::ModeRead, fileHandle)) + { + m_file = fileHandle; + m_fileIoBase = fileIoBase; + return true; + } + } + else + { + AZ::IO::SystemFile file; + if (file.Open(filePath, IO::SystemFile::OpenMode::SF_OPEN_READ_ONLY)) + { + m_file = AZStd::move(file); + return true; + } + } + + return false; + } + + bool FileReader::IsOpen() const + { + if (auto fileHandle = AZStd::get_if(&m_file); fileHandle != nullptr) + { + return *fileHandle != AZ::IO::InvalidHandle; + } + else if (auto systemFile = AZStd::get_if(&m_file); systemFile != nullptr) + { + return systemFile->IsOpen(); + } + + return false; + } + + void FileReader::Close() + { + if (auto fileHandle = AZStd::get_if(&m_file); fileHandle != nullptr) + { + if (AZ::IO::FileIOBase* fileIo = m_fileIoBase; fileIo != nullptr) + { + fileIo->Close(*fileHandle); + } + } + + m_file = AZStd::monostate{}; + m_fileIoBase = {}; + } + + auto FileReader::Length() const -> SizeType + { + if (auto fileHandle = AZStd::get_if(&m_file); fileHandle != nullptr) + { + if (SizeType fileSize{}; m_fileIoBase->Size(*fileHandle, fileSize)) + { + return fileSize; + } + } + else if (auto systemFile = AZStd::get_if(&m_file); systemFile != nullptr) + { + return systemFile->Length(); + } + + return 0; + } + + auto FileReader::Read(SizeType byteSize, void* buffer) -> SizeType + { + if (auto fileHandle = AZStd::get_if(&m_file); fileHandle != nullptr) + { + if (SizeType bytesRead{}; m_fileIoBase->Read(*fileHandle, buffer, byteSize, false, &bytesRead)) + { + return bytesRead; + } + } + else if (auto systemFile = AZStd::get_if(&m_file); systemFile != nullptr) + { + return systemFile->Read(byteSize, buffer); + } + + return 0; + } + + auto FileReader::Tell() const -> SizeType + { + if (auto fileHandle = AZStd::get_if(&m_file); fileHandle != nullptr) + { + if (SizeType fileOffset{}; m_fileIoBase->Tell(*fileHandle, fileOffset)) + { + return fileOffset; + } + } + else if (auto systemFile = AZStd::get_if(&m_file); systemFile != nullptr) + { + return systemFile->Tell(); + } + + return 0; + } + + bool FileReader::Seek(AZ::s64 offset, SeekType type) + { + if (auto fileHandle = AZStd::get_if(&m_file); fileHandle != nullptr) + { + return m_fileIoBase->Seek(*fileHandle, offset, type); + } + else if (auto systemFile = AZStd::get_if(&m_file); systemFile != nullptr) + { + systemFile->Seek(offset, static_cast(type)); + return true; + } + + return false; + } + + bool FileReader::Eof() const + { + if (auto fileHandle = AZStd::get_if(&m_file); fileHandle != nullptr) + { + return m_fileIoBase->Eof(*fileHandle); + } + else if (auto systemFile = AZStd::get_if(&m_file); systemFile != nullptr) + { + return systemFile->Eof(); + } + + return false; + } + + bool FileReader::GetFilePath(AZ::IO::FixedMaxPath& filePath) const + { + if (auto fileHandle = AZStd::get_if(&m_file); fileHandle != nullptr) + { + AZ::IO::FixedMaxPathString& pathStringRef = filePath.Native(); + if (m_fileIoBase->GetFilename(*fileHandle, pathStringRef.data(), pathStringRef.capacity())) + { + pathStringRef.resize_no_construct(AZStd::char_traits::length(pathStringRef.data())); + return true; + } + } + else if (auto systemFile = AZStd::get_if(&m_file); systemFile != nullptr) + { + filePath = systemFile->Name(); + return true; + } + + return false; + } +} diff --git a/Code/Framework/AzCore/AzCore/IO/FileReader.h b/Code/Framework/AzCore/AzCore/IO/FileReader.h new file mode 100644 index 0000000000..4fdb18b2b2 --- /dev/null +++ b/Code/Framework/AzCore/AzCore/IO/FileReader.h @@ -0,0 +1,92 @@ +/* + * 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 + * + */ +#pragma once + +#include +#include +#include + +namespace AZ::IO +{ + class FileIOBase; + enum class SeekType : AZ::u32; + + //! Structure which encapsulates delegates File Read operations + //! to either the FileIOBase or SystemFile classes based if a FileIOBase* instance has been supplied + //! to the FileSystemReader class + //! the SettingsRegistry option to use FileIO + class FileReader + { + using HandleType = AZ::u32; + using FileHandleType = AZStd::variant; + public: + using SizeType = AZ::u64; + + //! Creates FileReader instance in the default state with no file opend + FileReader(); + ~FileReader(); + + //! Creates a new FileReader instance and attempts to open the file at the supplied path + //! Uses the FileIOBase instance if supplied + //! @param fileIOBase pointer to fileIOBase instance + //! @param null-terminated filePath to open + FileReader(AZ::IO::FileIOBase* fileIoBase, const char* filePath); + + //! Takes ownership of the supplied FileReader handle + FileReader(FileReader&& other); + + //! Moves ownership of FileReader handle to this instance + FileReader& operator=(FileReader&& other); + + //! Opens a File using the FileIOBase instance if non-nullptr + //! Otherwise fall back to use SystemFile + //! @param fileIOBase pointer to fileIOBase instance + //! @param null-terminated filePath to open + //! @return true if the File is opened successfully + bool Open(AZ::IO::FileIOBase* fileIoBase, const char* filePath); + + //! Returns true if a file is currently open + //! @return true if the file is open + bool IsOpen() const; + + //! Closes the File + void Close(); + + //! Retrieve the length of the OpenFile + SizeType Length() const; + + //! Attempts to read up to byte size bytes into the supplied buffer + //! @param byteSize - Maximum number of bytes to read + //! @param buffer - Buffer to read bytes into + //! @returns the number of bytes read if the file is open, otherwise 0 + SizeType Read(SizeType byteSize, void* buffer); + + //! Returns the current file offset + //! @returns file offset if the file is open, otherwise 0 + SizeType Tell() const; + + //! Seeks within the open file to the offset supplied + //! @param offset File offset to seek to + //! @param type parameter to indicate the reference point to start the seek from + //! @returns true if the file is open and the seek succeeded + bool Seek(AZ::s64 offset, SeekType type); + + //! Returns true if the file is open and in the EOF state + bool Eof() const; + + //! Store the file path of the open file into the output file path parameter + //! The filePath reference is left unmodified, if the path was not stored + //! @return true if the filePath was stored + bool GetFilePath(AZ::IO::FixedMaxPath& filePath) const; + + private: + + FileHandleType m_file; + AZ::IO::FileIOBase* m_fileIoBase{}; + }; +} diff --git a/Code/Framework/AzCore/AzCore/IO/SystemFile.cpp b/Code/Framework/AzCore/AzCore/IO/SystemFile.cpp index 5bff79b422..651abb89fe 100644 --- a/Code/Framework/AzCore/AzCore/IO/SystemFile.cpp +++ b/Code/Framework/AzCore/AzCore/IO/SystemFile.cpp @@ -160,12 +160,12 @@ void SystemFile::Seek(SeekSizeType offset, SeekMode mode) Platform::Seek(m_handle, this, offset, mode); } -SystemFile::SizeType SystemFile::Tell() +SystemFile::SizeType SystemFile::Tell() const { return Platform::Tell(m_handle, this); } -bool SystemFile::Eof() +bool SystemFile::Eof() const { return Platform::Eof(m_handle, this); } diff --git a/Code/Framework/AzCore/AzCore/IO/SystemFile.h b/Code/Framework/AzCore/AzCore/IO/SystemFile.h index 8a5b2b2521..551ce89ce7 100644 --- a/Code/Framework/AzCore/AzCore/IO/SystemFile.h +++ b/Code/Framework/AzCore/AzCore/IO/SystemFile.h @@ -72,9 +72,9 @@ namespace AZ /// Seek in current file. void Seek(SeekSizeType offset, SeekMode mode); /// Get the cursor position in the current file. - SizeType Tell(); + SizeType Tell() const; /// Is the cursor at the end of the file? - bool Eof(); + bool Eof() const; /// Get the time the file was last modified. AZ::u64 ModificationTime(); /// Read data from a file synchronous. Return number of bytes actually read in the buffer. diff --git a/Code/Framework/AzCore/AzCore/Settings/SettingsRegistryImpl.cpp b/Code/Framework/AzCore/AzCore/Settings/SettingsRegistryImpl.cpp index 7ef1fa661d..92f546815e 100644 --- a/Code/Framework/AzCore/AzCore/Settings/SettingsRegistryImpl.cpp +++ b/Code/Framework/AzCore/AzCore/Settings/SettingsRegistryImpl.cpp @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include @@ -1116,118 +1117,6 @@ namespace AZ } } - //! Structure which encapsulates Commands to either the FileIOBase or SystemFile classes based on - //! the SettingsRegistry option to use FileIO - struct SettingsRegistryFileReader - { - using FileHandleType = AZStd::variant; - - SettingsRegistryFileReader() = default; - SettingsRegistryFileReader(bool useFileIo, const char* filePath) - { - Open(useFileIo, filePath); - } - - ~SettingsRegistryFileReader() - { - if (auto fileHandle = AZStd::get_if(&m_file); fileHandle != nullptr) - { - if (AZ::IO::FileIOBase* fileIo = AZ::IO::FileIOBase::GetInstance(); fileIo != nullptr) - { - fileIo->Close(*fileHandle); - } - } - } - - bool Open(bool useFileIo, const char* filePath) - { - Close(); - if (AZ::IO::FileIOBase* fileIo = useFileIo ? AZ::IO::FileIOBase::GetInstance() : nullptr; fileIo != nullptr) - { - AZ::IO::HandleType fileHandle; - if (fileIo->Open(filePath, IO::OpenMode::ModeRead, fileHandle)) - { - m_file = fileHandle; - return true; - } - } - else - { - AZ::IO::SystemFile file; - if (file.Open(filePath, IO::SystemFile::OpenMode::SF_OPEN_READ_ONLY)) - { - m_file = AZStd::move(file); - return true; - } - } - - return false; - } - - bool IsOpen() const - { - if (auto fileHandle = AZStd::get_if(&m_file); fileHandle != nullptr) - { - return *fileHandle != AZ::IO::InvalidHandle; - } - else if (auto systemFile = AZStd::get_if(&m_file); systemFile != nullptr) - { - return systemFile->IsOpen(); - } - - return false; - } - - void Close() - { - if (auto fileHandle = AZStd::get_if(&m_file); fileHandle != nullptr) - { - if (AZ::IO::FileIOBase* fileIo = AZ::IO::FileIOBase::GetInstance(); fileIo != nullptr) - { - fileIo->Close(*fileHandle); - } - } - - m_file = AZStd::monostate{}; - } - - u64 Length() const - { - if (auto fileHandle = AZStd::get_if(&m_file); fileHandle != nullptr) - { - if (u64 fileSize{}; AZ::IO::FileIOBase::GetInstance()->Size(*fileHandle, fileSize)) - { - return fileSize; - } - } - else if (auto systemFile = AZStd::get_if(&m_file); systemFile != nullptr) - { - return systemFile->Length(); - } - - return 0; - } - - AZ::IO::SizeType Read(AZ::IO::SizeType byteSize, void* buffer) - { - if (auto fileHandle = AZStd::get_if(&m_file); fileHandle != nullptr) - { - if (AZ::u64 bytesRead{}; AZ::IO::FileIOBase::GetInstance()->Read(*fileHandle, buffer, byteSize, false, &bytesRead)) - { - return bytesRead; - } - } - else if (auto systemFile = AZStd::get_if(&m_file); systemFile != nullptr) - { - return systemFile->Read(byteSize, buffer); - } - - return 0; - } - - FileHandleType m_file; - }; - bool SettingsRegistryImpl::MergeSettingsFileInternal(const char* path, Format format, AZStd::string_view rootKey, AZStd::vector& scratchBuffer) { @@ -1236,7 +1125,7 @@ namespace AZ Pointer pointer(AZ_SETTINGS_REGISTRY_HISTORY_KEY "/-"); - SettingsRegistryFileReader fileReader(m_useFileIo, path); + FileReader fileReader(m_useFileIo ? AZ::IO::FileIOBase::GetInstance(): nullptr, path); if (!fileReader.IsOpen()) { AZ_Error("Settings Registry", false, R"(Unable to open registry file "%s".)", path); diff --git a/Code/Framework/AzCore/AzCore/Settings/SettingsRegistryMergeUtils.cpp b/Code/Framework/AzCore/AzCore/Settings/SettingsRegistryMergeUtils.cpp index 5290c9b02a..da7110e36e 100644 --- a/Code/Framework/AzCore/AzCore/Settings/SettingsRegistryMergeUtils.cpp +++ b/Code/Framework/AzCore/AzCore/Settings/SettingsRegistryMergeUtils.cpp @@ -6,6 +6,8 @@ * */ +#include +#include #include #include #include @@ -388,8 +390,36 @@ namespace AZ::SettingsRegistryMergeUtils const ConfigParserSettings& configParserSettings) { auto configPath = FindEngineRoot(registry) / filePath; - IO::SystemFile configFile; - if (!configFile.Open(configPath.c_str(), IO::SystemFile::OpenMode::SF_OPEN_READ_ONLY)) + IO::FileReader configFile; + bool configFileOpened{}; + switch (configParserSettings.m_fileReaderClass) + { + case ConfigParserSettings::FileReaderClass::UseFileIOIfAvailableFallbackToSystemFile: + { + auto fileIo = AZ::IO::FileIOBase::GetInstance(); + configFileOpened = configFile.Open(fileIo, configPath.c_str()); + break; + } + case ConfigParserSettings::FileReaderClass::UseSystemFileOnly: + { + configFileOpened = configFile.Open(nullptr, configPath.c_str()); + break; + } + case ConfigParserSettings::FileReaderClass::UseFileIOOnly: + { + auto fileIo = AZ::IO::FileIOBase::GetInstance(); + if (fileIo == nullptr) + { + return false; + } + configFileOpened = configFile.Open(fileIo, configPath.c_str()); + break; + } + default: + AZ_Error("SettingsRegistryMergeUtils", false, "An Invalid FileReaderClass enum value has been supplied"); + return false; + } + if (!configFileOpened) { AZ_Warning("SettingsRegistryMergeUtils", false, R"(Unable to open file "%s")", configPath.c_str()); return false; @@ -480,7 +510,7 @@ namespace AZ::SettingsRegistryMergeUtils AZ_Error("SettingsRegistryMergeUtils", false, R"(The config file "%s" contains a line which is longer than the max line length of %zu.)" "\n" R"(Parsing will halt. The line content so far is:)" "\n" - R"("%.*s")" "\n", configFile.Name(), configBuffer.max_size(), + R"("%.*s")" "\n", configPath.c_str(), configBuffer.max_size(), aznumeric_cast(configBuffer.size()), configBuffer.data()); configFileParsed = false; break; diff --git a/Code/Framework/AzCore/AzCore/Settings/SettingsRegistryMergeUtils.h b/Code/Framework/AzCore/AzCore/Settings/SettingsRegistryMergeUtils.h index 02346c2ba1..daa64c0343 100644 --- a/Code/Framework/AzCore/AzCore/Settings/SettingsRegistryMergeUtils.h +++ b/Code/Framework/AzCore/AzCore/Settings/SettingsRegistryMergeUtils.h @@ -155,6 +155,15 @@ namespace AZ::SettingsRegistryMergeUtils //! structure which is forwarded to the SettingsRegistryInterface MergeCommandLineArgument function //! The structure contains a functor which returns true if a character is a valid delimiter SettingsRegistryInterface::CommandLineArgumentSettings m_commandLineSettings; + + //! enumeration to indicate if AZ::IO::FileIOBase should be used to open the config file over AZ::IO::SystemFile + enum class FileReaderClass + { + UseFileIOIfAvailableFallbackToSystemFile, + UseSystemFileOnly, + UseFileIOOnly + }; + FileReaderClass m_fileReaderClass = FileReaderClass::UseFileIOIfAvailableFallbackToSystemFile; }; //! Loads basic configuration files which have structures similar to Windows INI files //! It is inspired by the Python configparser module: https://docs.python.org/3.10/library/configparser.html diff --git a/Code/Framework/AzCore/AzCore/azcore_files.cmake b/Code/Framework/AzCore/AzCore/azcore_files.cmake index 6c498c3335..aa07959997 100644 --- a/Code/Framework/AzCore/AzCore/azcore_files.cmake +++ b/Code/Framework/AzCore/AzCore/azcore_files.cmake @@ -166,6 +166,8 @@ set(FILES IO/FileIO.cpp IO/FileIO.h IO/FileIOEventBus.h + IO/FileReader.cpp + IO/FileReader.h IO/IOUtils.h IO/IOUtils.cpp IO/IStreamer.h diff --git a/Code/Framework/AzCore/Tests/IO/FileReaderTests.cpp b/Code/Framework/AzCore/Tests/IO/FileReaderTests.cpp new file mode 100644 index 0000000000..691b3f2821 --- /dev/null +++ b/Code/Framework/AzCore/Tests/IO/FileReaderTests.cpp @@ -0,0 +1,72 @@ +/* + * 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 + +namespace UnitTest +{ + template + class FileReaderTestFixture + : public ScopedAllocatorSetupFixture + { + public: + void SetUp() override + { + if constexpr (AZStd::is_same_v) + { + m_fileIo = AZStd::make_unique(); + } + } + + void TearDown() override + { + m_fileIo.reset(); + } + + protected: + AZStd::unique_ptr m_fileIo{}; + }; + + using FileIOTypes = ::testing::Types; + + TYPED_TEST_CASE(FileReaderTestFixture, FileIOTypes); + + TYPED_TEST(FileReaderTestFixture, ConstructorWithFilePath_OpensFileSuccessfully) + { + AZ::IO::FileReader fileReader(this->m_fileIo.get(), AZ::IO::SystemFile::GetNullFilename()); + EXPECT_TRUE(fileReader.IsOpen()); + } + + TYPED_TEST(FileReaderTestFixture, Open_OpensFileSucessfully) + { + AZ::IO::FileReader fileReader; + fileReader.Open(this->m_fileIo.get(), AZ::IO::SystemFile::GetNullFilename()); + EXPECT_TRUE(fileReader.IsOpen()); + } + + TYPED_TEST(FileReaderTestFixture, Eof_OnNULDeviceFile_Succeeds) + { + AZ::IO::FileReader fileReader(this->m_fileIo.get(), AZ::IO::SystemFile::GetNullFilename()); + EXPECT_TRUE(fileReader.Eof()); + } + + TYPED_TEST(FileReaderTestFixture, GetFilePath_ReturnsNULDeviceFilename_Succeeds) + { + AZ::IO::FileReader fileReader(this->m_fileIo.get(), AZ::IO::SystemFile::GetNullFilename()); + AZ::IO::FixedMaxPath filePath; + EXPECT_TRUE(fileReader.GetFilePath(filePath)); + AZ::IO::FixedMaxPath nulFilename{ AZ::IO::SystemFile::GetNullFilename() }; + if (this->m_fileIo) + { + EXPECT_TRUE(this->m_fileIo->ResolvePath(nulFilename, nulFilename)); + } + EXPECT_EQ(nulFilename, filePath); + } + +} // namespace UnitTest diff --git a/Code/Framework/AzCore/Tests/azcoretests_files.cmake b/Code/Framework/AzCore/Tests/azcoretests_files.cmake index d4d107f094..c36d37d874 100644 --- a/Code/Framework/AzCore/Tests/azcoretests_files.cmake +++ b/Code/Framework/AzCore/Tests/azcoretests_files.cmake @@ -37,6 +37,7 @@ set(FILES FileIOBaseTestTypes.h Geometry2DUtils.cpp Interface.cpp + IO/FileReaderTests.cpp IO/Path/PathTests.cpp IPC.cpp Jobs.cpp