Browse Source

Addressed issues found in/by the Spawnables benchmarks and unit tests.

Signed-off-by: AMZN-koppersr <[email protected]>
AMZN-koppersr 3 years ago
parent
commit
1efbb7216f

+ 3 - 0
Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesInterface.h

@@ -10,6 +10,7 @@
 
 #include <AzCore/Asset/AssetCommon.h>
 #include <AzCore/Interface/Interface.h>
+#include <AzCore/Memory/SystemAllocator.h>
 #include <AzCore/RTTI/TypeSafeIntegral.h>
 #include <AzCore/std/functional.h>
 #include <AzFramework/Spawnable/Spawnable.h>
@@ -164,6 +165,8 @@ namespace AzFramework
     public:
         friend class SpawnableEntitiesDefinition;
 
+        AZ_CLASS_ALLOCATOR(AzFramework::EntitySpawnTicket, AZ::SystemAllocator, 0);
+
         using Id = uint32_t;
 
         EntitySpawnTicket() = default;

+ 6 - 19
Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesManager.cpp

@@ -499,12 +499,8 @@ namespace AzFramework
                 for (auto it = newEntitiesBegin; it != newEntitiesEnd; ++it)
                 {
                     AZ::Entity* clone = (*it);
-                    // The entity component framework doesn't handle entities without TransformComponent safely.
-                    if (!clone->GetComponents().empty())
-                    {
-                        clone->SetSpawnTicketId(request.m_ticketId);
-                        GameEntityContextRequestBus::Broadcast(&GameEntityContextRequestBus::Events::AddGameEntity, *it);
-                    }
+                    clone->SetSpawnTicketId(request.m_ticketId);
+                    GameEntityContextRequestBus::Broadcast(&GameEntityContextRequestBus::Events::AddGameEntity, clone);
                 }
 
                 // Let other systems know about newly spawned entities for any post-processing after adding to the scene/game context.
@@ -636,12 +632,8 @@ namespace AzFramework
                 for (auto it = ticket.m_spawnedEntities.begin() + spawnedEntitiesInitialCount; it != ticket.m_spawnedEntities.end(); ++it)
                 {
                     AZ::Entity* clone = (*it);
-                    // The entity component framework doesn't handle entities without TransformComponent safely.
-                    if (!clone->GetComponents().empty())
-                    {
-                        clone->SetSpawnTicketId(request.m_ticketId);
-                        GameEntityContextRequestBus::Broadcast(&GameEntityContextRequestBus::Events::AddGameEntity, *it);
-                    }
+                    clone->SetSpawnTicketId(request.m_ticketId);
+                    GameEntityContextRequestBus::Broadcast(&GameEntityContextRequestBus::Events::AddGameEntity, *it);
                 }
 
                 if (request.m_completionCallback)
@@ -668,7 +660,7 @@ namespace AzFramework
             {
                 if (entity != nullptr)
                 {
-                    // Setting it to 0 is needed to avoid the infite loop between GameEntityContext and SpawnableEntitiesManager.
+                    // Setting it to 0 is needed to avoid the infinite loop between GameEntityContext and SpawnableEntitiesManager.
                     entity->SetSpawnTicketId(0);
                     GameEntityContextRequestBus::Broadcast(
                         &GameEntityContextRequestBus::Events::DestroyGameEntity, entity->GetId());
@@ -702,7 +694,7 @@ namespace AzFramework
             {
                 if (*entityIterator != nullptr && (*entityIterator)->GetId() == request.m_entityId)
                 {
-                    // Setting it to 0 is needed to avoid the infite loop between GameEntityContext and SpawnableEntitiesManager.
+                    // Setting it to 0 is needed to avoid the infinite loop between GameEntityContext and SpawnableEntitiesManager.
                     (*entityIterator)->SetSpawnTicketId(0);
                     GameEntityContextRequestBus::Broadcast(
                         &GameEntityContextRequestBus::Events::DestroyGameEntity, (*entityIterator)->GetId());
@@ -949,11 +941,6 @@ namespace AzFramework
                     GameEntityContextRequestBus::Broadcast(
                         &GameEntityContextRequestBus::Events::DestroyGameEntity, entity->GetId());
                 }
-                else
-                {
-                    // Entities without components wouldn't have been send to the GameEntityContext.
-                    delete entity;
-                }
             }
             delete request.m_ticket;
 

+ 2 - 2
Code/Framework/AzFramework/Tests/Spawnable/SpawnableEntitiesManagerTests.cpp

@@ -111,7 +111,7 @@ namespace UnitTest
             m_spawnable = aznew AzFramework::Spawnable(
                 AZ::Data::AssetId::CreateString("{EB2E8A2B-F253-4A90-BBF4-55F2EED786B8}:0"), AZ::Data::AssetData::AssetStatus::Ready);
             m_spawnableAsset = new AZ::Data::Asset<AzFramework::Spawnable>(m_spawnable, AZ::Data::AssetLoadBehavior::Default);
-            m_ticket = new AzFramework::EntitySpawnTicket(*m_spawnableAsset);
+            m_ticket = aznew AzFramework::EntitySpawnTicket(*m_spawnableAsset);
 
             auto managerInterface = AzFramework::SpawnableEntitiesInterface::Get();
             m_manager = azrtti_cast<AzFramework::SpawnableEntitiesManager*>(managerInterface);
@@ -516,7 +516,7 @@ namespace UnitTest
             // Make sure we start with a fresh ticket each time, or else each iteration through this loop would continue to build up
             // more and more entities.
             delete m_ticket;
-            m_ticket = new AzFramework::EntitySpawnTicket(*m_spawnableAsset);
+            m_ticket = aznew AzFramework::EntitySpawnTicket(*m_spawnableAsset);
 
             constexpr size_t NumEntities = 4;
             FillSpawnable(NumEntities);

+ 5 - 4
Code/Framework/AzToolsFramework/Tests/Prefab/Benchmark/Spawnable/SpawnAllEntitiesBenchmarks.cpp

@@ -25,7 +25,7 @@ namespace Benchmark
         for (auto _ : state)
         {
             state.PauseTiming();
-            m_spawnTicket = new AzFramework::EntitySpawnTicket(m_spawnableAsset);
+            m_spawnTicket = aznew AzFramework::EntitySpawnTicket(m_spawnableAsset);
             state.ResumeTiming();
 
             for (uint64_t spwanableCounter = 0; spwanableCounter < spawnAllEntitiesCallCount; spwanableCounter++)
@@ -62,7 +62,7 @@ namespace Benchmark
         for (auto _ : state)
         {
             state.PauseTiming();
-            m_spawnTicket = new AzFramework::EntitySpawnTicket(m_spawnableAsset);
+            m_spawnTicket = aznew AzFramework::EntitySpawnTicket(m_spawnableAsset);
             state.ResumeTiming();
 
             AzFramework::SpawnableEntitiesInterface::Get()->SpawnAllEntities(*m_spawnTicket);
@@ -93,15 +93,16 @@ namespace Benchmark
 
         SetUpSpawnableAsset(entityCountInSpawnable);
 
+        auto spawner = AzFramework::SpawnableEntitiesInterface::Get();
         for (auto _ : state)
         {
             state.PauseTiming();
-            m_spawnTicket = new AzFramework::EntitySpawnTicket(m_spawnableAsset);
+            m_spawnTicket = aznew AzFramework::EntitySpawnTicket(m_spawnableAsset);
             state.ResumeTiming();
 
             for (uint64_t spawnCallCounter = 0; spawnCallCounter < spawnCallCount; spawnCallCounter++)
             {
-                AzFramework::SpawnableEntitiesInterface::Get()->SpawnAllEntities(*m_spawnTicket);
+                spawner->SpawnAllEntities(*m_spawnTicket);
             }
 
             m_rootSpawnableInterface->ProcessSpawnableQueue();

+ 1 - 1
Gems/Vegetation/Code/Source/PrefabInstanceSpawner.cpp

@@ -342,7 +342,7 @@ namespace Vegetation
         // Create the EntitySpawnTicket here.  This pointer is going to get handed off to the vegetation system as opaque instance data,
         // where it will be tracked and held onto for the lifetime of the vegetation instance.  The vegetation system will pass it back
         // in to DestroyInstance at the end of the lifetime, so that's the one place where we will delete the ticket pointers.
-        AzFramework::EntitySpawnTicket* ticket = new AzFramework::EntitySpawnTicket(m_spawnableAsset);
+        AzFramework::EntitySpawnTicket* ticket = aznew AzFramework::EntitySpawnTicket(m_spawnableAsset);
         if (ticket->IsValid())
         {
             // Track the ticket that we've created.