From 4ad0560d06bd849f8ccdaaa473fbafffc97fc641 Mon Sep 17 00:00:00 2001 From: AMZN-koppersr <82230785+AMZN-koppersr@users.noreply.github.com> Date: Wed, 9 Jun 2021 12:11:04 -0700 Subject: [PATCH] Removed AddOn(De)SpawnedHandler from Spawnable Entities Interface The calls AddOnSpawnedHandler and AddOnDespawnedHandler were removed from the SpawnableEntitiesInterface. These functions will eventually be called from multiple threads and AZ::Event currently doesn't have a thread-safe version to support this. There's also a performance concern as these callbacks are called for each individual (de)spawn requests which can lead to multiple handlers being called without information that's relevant to the callback. It would be better to batch up all (de)spawn requests per ProcessQueue call and only have a single event do a single signal. Since both events are currently not being used they have been removed for now, but can be introduced -with the previously mentioned concerns in mind- when needed. --- .../Spawnable/SpawnableEntitiesInterface.h | 6 ------ .../Spawnable/SpawnableEntitiesManager.cpp | 20 ------------------- .../Spawnable/SpawnableEntitiesManager.h | 6 ------ 3 files changed, 32 deletions(-) diff --git a/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesInterface.h b/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesInterface.h index d2a5872fc8..8236683163 100644 --- a/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesInterface.h +++ b/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesInterface.h @@ -322,12 +322,6 @@ namespace AzFramework //! @param optionalArgs Optional additional arguments, see BarrierOptionalArgs. virtual void Barrier(EntitySpawnTicket& ticket, BarrierCallback completionCallback, BarrierOptionalArgs optionalArgs = {}) = 0; - //! Register a handler for OnSpawned events. - virtual void AddOnSpawnedHandler(AZ::Event>::Handler& handler) = 0; - - //! Register a handler for OnDespawned events. - virtual void AddOnDespawnedHandler(AZ::Event>::Handler& handler) = 0; - protected: [[nodiscard]] virtual AZStd::pair CreateTicket(AZ::Data::Asset&& spawnable) = 0; virtual void DestroyTicket(void* ticket) = 0; diff --git a/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesManager.cpp b/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesManager.cpp index a482c1d4f9..d6d005a944 100644 --- a/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesManager.cpp +++ b/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesManager.cpp @@ -150,16 +150,6 @@ namespace AzFramework QueueRequest(ticket, optionalArgs.m_priority, AZStd::move(queueEntry)); } - void SpawnableEntitiesManager::AddOnSpawnedHandler(AZ::Event>::Handler& handler) - { - handler.Connect(m_onSpawnedEvent); - } - - void SpawnableEntitiesManager::AddOnDespawnedHandler(AZ::Event>::Handler& handler) - { - handler.Connect(m_onDespawnedEvent); - } - auto SpawnableEntitiesManager::ProcessQueue(CommandQueuePriority priority) -> CommandQueueStatus { CommandQueueStatus result = CommandQueueStatus::NoCommandsLeft; @@ -320,8 +310,6 @@ namespace AzFramework ticket.m_spawnedEntities.begin() + spawnedEntitiesInitialCount, ticket.m_spawnedEntities.end())); } - m_onSpawnedEvent.Signal(ticket.m_spawnable); - ticket.m_currentRequestId++; return true; } @@ -401,8 +389,6 @@ namespace AzFramework ticket.m_spawnedEntities.begin() + spawnedEntitiesInitialCount, ticket.m_spawnedEntities.end())); } - m_onSpawnedEvent.Signal(ticket.m_spawnable); - ticket.m_currentRequestId++; return true; } @@ -434,8 +420,6 @@ namespace AzFramework request.m_completionCallback(request.m_ticketId); } - m_onDespawnedEvent.Signal(ticket.m_spawnable); - ticket.m_currentRequestId++; return true; } @@ -463,8 +447,6 @@ namespace AzFramework } } - m_onDespawnedEvent.Signal(ticket.m_spawnable); - // Rebuild the list of entities. ticket.m_spawnedEntities.clear(); const Spawnable::EntityList& entities = request.m_spawnable->GetEntities(); @@ -517,8 +499,6 @@ namespace AzFramework ticket.m_currentRequestId++; - m_onSpawnedEvent.Signal(ticket.m_spawnable); - return true; } else diff --git a/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesManager.h b/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesManager.h index 638559f3f1..5f659182d3 100644 --- a/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesManager.h +++ b/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesManager.h @@ -73,9 +73,6 @@ namespace AzFramework void Barrier(EntitySpawnTicket& spawnInfo, BarrierCallback completionCallback, BarrierOptionalArgs optionalArgs = {}) override; - void AddOnSpawnedHandler(AZ::Event>::Handler& handler) override; - void AddOnDespawnedHandler(AZ::Event>::Handler& handler) override; - // // The following function is thread safe but intended to be run from the main thread. // @@ -200,9 +197,6 @@ namespace AzFramework Queue m_highPriorityQueue; Queue m_regularPriorityQueue; - AZ::Event> m_onSpawnedEvent; - AZ::Event> m_onDespawnedEvent; - AZ::SerializeContext* m_defaultSerializeContext { nullptr }; //! The threshold used to determine if a request goes in the regular (if bigger than the value) or high priority queue (if smaller //! or equal to this value). The starting value of 64 is chosen as it's between default values SpawnablePriority_High and