Updates to the Spawnable entity aliases based on provided feedback on PR.

Signed-off-by: AMZN-koppersr <82230785+AMZN-koppersr@users.noreply.github.com>

Code/Framework/AzFramework/AzFramework/Spawnable/RootSpawnableInterface.h

@@ -30,7 +30,7 @@ namespace AzFramework
         
         //! Called when the root spawnable has been assigned a new value. This may be called several times without a call to release
         //!     in between.
-        //! NOTE: The callback is not queued but immediately called from a random thread. This is done because this callback is typically
+        //! @note: The callback is not queued but immediately called from a random thread. This is done because this callback is typically
         //!     used before entities are spawned and if it's queued then the entities spawn before this callback is called.
         //! @param rootSpawnable The new root spawnable that was assigned.
         //! @param generation The generation of the root spawnable. This will increment every time a new spawnable is assigned.
@@ -38,7 +38,7 @@ namespace AzFramework
             [[maybe_unused]] uint32_t generation) {}
         //! Called when the root spawnable has completed spawning of entities. This may be called several times without a call to release
         //!     in between.
-        //! NOTE: This callback is queued and will be called with a delay and from the main thread.
+        //! @note: This callback is queued and will be called with a delay and from the main thread.
         //! @param rootSpawnable The new root spawnable that was used to spawn entities from.
         //! @param generation The generation of the root spawnable. This will increment every time a new spawnable is assigned.
         virtual void OnRootSpawnableReady(

Code/Framework/AzFramework/AzFramework/Spawnable/Spawnable.cpp

@@ -20,7 +20,6 @@ namespace AzFramework
     // EntityAlias
     //
 
-
     bool Spawnable::EntityAlias::HasLowerIndex(const EntityAlias& other) const
     {
         return m_sourceIndex == other.m_sourceIndex ?
@@ -51,7 +50,7 @@ namespace AzFramework
         {
             if (!alias.m_queueLoad ||
                 alias.m_aliasType == Spawnable::EntityAliasType::Original ||
-                alias.m_aliasType == Spawnable::EntityAliasType::Disabled)
+                alias.m_aliasType == Spawnable::EntityAliasType::Disable)
             {
                 continue;
             }
@@ -132,7 +131,6 @@ namespace AzFramework
     // EntityAliasVisitor
     //
 
-
     Spawnable::EntityAliasVisitor::EntityAliasVisitor(Spawnable& owner, EntityAliasList* entityAliasList)
         : m_owner(owner)
         , m_entityAliasList(entityAliasList)
@@ -167,11 +165,7 @@ namespace AzFramework
         if (this != &rhs)
         {
             this->~EntityAliasVisitor();
-            *this = EntityAliasVisitor(rhs.m_owner, rhs.m_entityAliasList);
-            m_dirty = rhs.m_dirty;
-
-            rhs.m_entityAliasList = nullptr;
-            rhs.m_dirty = false;
+            new(this) EntityAliasVisitor(AZStd::move(rhs));
         }
         return *this;
     }
@@ -249,7 +243,7 @@ namespace AzFramework
         {
             if (alias.m_queueLoad &&
                 alias.m_aliasType != Spawnable::EntityAliasType::Original &&
-                alias.m_aliasType != Spawnable::EntityAliasType::Disabled &&
+                alias.m_aliasType != Spawnable::EntityAliasType::Disable &&
                 !alias.m_spawnable.IsLoading() &&
                 !alias.m_spawnable.IsReady() &&
                 !alias.m_spawnable.IsError())
@@ -336,14 +330,14 @@ namespace AzFramework
                 {
                 case Spawnable::EntityAliasType::Original:
                     [[fallthrough]];
-                case Spawnable::EntityAliasType::Disabled:
+                case Spawnable::EntityAliasType::Disable:
                     [[fallthrough]];
                 case Spawnable::EntityAliasType::Replace:
                     // If the previous entry was a disabled, original or replace alias then remove it as it will be overwritten by the
                     // current entry.
                     if (previousIndex == it->m_sourceIndex &&
                         (previousType == Spawnable::EntityAliasType::Original ||
-                         previousType == Spawnable::EntityAliasType::Disabled ||
+                         previousType == Spawnable::EntityAliasType::Disable ||
                          previousType == Spawnable::EntityAliasType::Replace))
                     {
                         previousIndex = it->m_sourceIndex;

Code/Framework/AzFramework/AzFramework/Spawnable/Spawnable.h

@@ -34,7 +34,7 @@ namespace AzFramework
         enum class EntityAliasType : uint8_t
         {
             Original,   //!< The original entity is spawned.
-            Disabled,   //!< No entity will be spawned.
+            Disable,    //!< No entity will be spawned.
             Replace,    //!< The entity alias is spawned instead of the original.
             Additional, //!< The original entity is spawned as well as the alias. The alias will get a new entity id.
             Merge       //!< The original entity is spawned and the components of the alias are added. The caller is responsible for
@@ -105,6 +105,9 @@ namespace AzFramework
             bool HasAliases() const;
             bool AreAllSpawnablesReady() const;
 
+            // Modification of aliases is limited to specific changes that can only be done through the available modification functions.
+            // For this reason access through iterators is limited to unmodifiable constant iterators.
+
             EntityAliasList::const_iterator begin() const;
             EntityAliasList::const_iterator end() const;
             EntityAliasList::const_iterator cbegin() const;
@@ -146,7 +149,7 @@ namespace AzFramework
         class EntityAliasConstVisitor final : public EntityAliasVisitorBase
         {
         public:
-            EntityAliasConstVisitor(const Spawnable& owner, const EntityAliasList* m_entityAliasList);
+            EntityAliasConstVisitor(const Spawnable& owner, const EntityAliasList* entityAliasList);
             ~EntityAliasConstVisitor();
 
             //! Checks if the visitor was able to retrieve data. This needs to be checked before calling any other functions.

Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableAssetHandler.cpp

@@ -118,6 +118,7 @@ namespace AzFramework
             SpawnableAssetEventsBus::Broadcast(
                 &SpawnableAssetEvents::OnResolveAliases, aliases, spawnable->GetMetaData(), spawnable->GetEntities());
 
+            // The aliases will only be optimized if OnResolveAliases has made any changes.
             aliases.Optimize();
             aliases.ListSpawnablesRequiringLoad(
                 [&assetLoadFilterCB, streamingDeadline, streamingPriority](AZ::Data::Asset<Spawnable>& assetPendingLoad)

Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesContainer.h

@@ -79,9 +79,9 @@ namespace AzFramework
         //! other than the calling thread including the main thread. Note that because the alert is queued it can still be called
         //! after the container has been deleted or can be called for a previously assigned spawnable. In the latter case check
         //! if the current generation matches the generation provided with the callback.
-        //! @callback The function called when the alert triggers. This can be called from a different thread than the one that
+        //! @param callback The function called when the alert triggers. This can be called from a different thread than the one that
         //!     the one that made the call to Alert.
-        //! @checkSpawnableIsLoaded If true the alert will also block until the spawnable has been loaded. If false then it will
+        //! @param checkSpawnableIsLoaded If true the alert will also block until the spawnable has been loaded. If false then it will
         //!     be called after all previous calls have completed, but the spawnable may not be loaded at that point.
         void Alert(AlertCallback callback, CheckIfSpawnableIsLoaded spawnableCheck = CheckIfSpawnableIsLoaded::No);
 

Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesManager.cpp

@@ -327,7 +327,7 @@ namespace AzFramework
             clone = CloneSingleEntity(entityTemplate, templateToCloneMap, serializeContext);
             AZ_Assert(clone != nullptr, "Failed to clone spawnable entity.");
             return clone;
-        case Spawnable::EntityAliasType::Disabled:
+        case Spawnable::EntityAliasType::Disable:
             // Do nothing.
             return nullptr;
         case Spawnable::EntityAliasType::Replace:

Code/Framework/AzFramework/Tests/Spawnable/SpawnableEntitiesManagerTests.cpp

@@ -485,8 +485,8 @@ namespace UnitTest
         FillSpawnable(NumEntities);
         InsertEntityAliases<NumEntities>(
             { 0, 1, 2, 3 }, { 0, 1, 2, 3 },
-            { Spawnable::EntityAliasType::Disabled, Spawnable::EntityAliasType::Disabled, Spawnable::EntityAliasType::Disabled,
-              Spawnable::EntityAliasType::Disabled });
+            { Spawnable::EntityAliasType::Disable, Spawnable::EntityAliasType::Disable, Spawnable::EntityAliasType::Disable,
+              Spawnable::EntityAliasType::Disable });
 
         size_t spawnedEntitiesCount = 0;
         auto callback = [&spawnedEntitiesCount](AzFramework::EntitySpawnTicket::Id, AzFramework::SpawnableConstEntityContainerView entities)
@@ -506,7 +506,7 @@ namespace UnitTest
         using namespace AzFramework;
         static constexpr size_t NumEntities = 8;
         FillSpawnable(NumEntities);
-        InsertEntityAliases<2>({ 1, 3 }, { 1, 3 }, { Spawnable::EntityAliasType::Disabled, Spawnable::EntityAliasType::Disabled });
+        InsertEntityAliases<2>({ 1, 3 }, { 1, 3 }, { Spawnable::EntityAliasType::Disable, Spawnable::EntityAliasType::Disable });
 
         size_t spawnedEntitiesCount = 0;
         auto callback = [&spawnedEntitiesCount](AzFramework::EntitySpawnTicket::Id, AzFramework::SpawnableConstEntityContainerView entities)
@@ -1019,8 +1019,8 @@ namespace UnitTest
 
         InsertEntityAliases<NumEntities>(
             { 0, 1, 2, 3 }, { 0, 1, 2, 3 },
-            { Spawnable::EntityAliasType::Disabled, Spawnable::EntityAliasType::Disabled, Spawnable::EntityAliasType::Disabled,
-              Spawnable::EntityAliasType::Disabled });
+            { Spawnable::EntityAliasType::Disable, Spawnable::EntityAliasType::Disable, Spawnable::EntityAliasType::Disable,
+              Spawnable::EntityAliasType::Disable });
 
         AZStd::vector<uint32_t> indices = { 0, 2, 3, 1 };
 
@@ -1043,7 +1043,7 @@ namespace UnitTest
         FillSpawnable(8);
         InsertEntityAliases<3>(
             { 1, 3, 6 }, { 1, 3, 6 },
-            { Spawnable::EntityAliasType::Disabled, Spawnable::EntityAliasType::Disabled, Spawnable::EntityAliasType::Disabled });
+            { Spawnable::EntityAliasType::Disable, Spawnable::EntityAliasType::Disable, Spawnable::EntityAliasType::Disable });
 
         AZStd::vector<uint32_t> indices = { 0, 2, 3, 1, 2, 3, 0, 1, 6, 4, 5, 7, 4, 1, 0, 6 };
 

Code/Framework/AzFramework/Tests/Spawnable/SpawnableTests.cpp

@@ -191,8 +191,8 @@ namespace UnitTest
         InsertEightEntities();
         InsertEightEntityAliases(
             { 0, 0, 0, 0, 0, 0, 0, 0 }, { 0, 1, 2, 3, 4, 5, 6, 7 },
-            { Spawnable::EntityAliasType::Replace, Spawnable::EntityAliasType::Original, Spawnable::EntityAliasType::Disabled,
-              Spawnable::EntityAliasType::Replace, Spawnable::EntityAliasType::Original, Spawnable::EntityAliasType::Disabled,
+            { Spawnable::EntityAliasType::Replace, Spawnable::EntityAliasType::Original, Spawnable::EntityAliasType::Disable,
+              Spawnable::EntityAliasType::Replace, Spawnable::EntityAliasType::Original, Spawnable::EntityAliasType::Disable,
               Spawnable::EntityAliasType::Replace, Spawnable::EntityAliasType::Original });
 
         AzFramework::Spawnable::EntityAliasVisitor visitor = m_spawnable->TryGetAliases();
@@ -245,14 +245,14 @@ namespace UnitTest
         InsertEightEntityAliases(
             { 0, 0, 0, 1, 1, 2, 2, 2 }, { 0, 1, 2, 3, 4, 5, 6, 7 },
             { Spawnable::EntityAliasType::Original, Spawnable::EntityAliasType::Original, Spawnable::EntityAliasType::Original,
-              Spawnable::EntityAliasType::Original, Spawnable::EntityAliasType::Disabled, Spawnable::EntityAliasType::Original,
+              Spawnable::EntityAliasType::Original, Spawnable::EntityAliasType::Disable, Spawnable::EntityAliasType::Original,
               Spawnable::EntityAliasType::Replace, Spawnable::EntityAliasType::Original });
 
         AzFramework::Spawnable::EntityAliasVisitor visitor = m_spawnable->TryGetAliases();
         ASSERT_TRUE(visitor.IsSet());
 
         EXPECT_EQ(2, AZStd::distance(visitor.begin(), visitor.end()));
-        EXPECT_EQ(Spawnable::EntityAliasType::Disabled, visitor.begin()->m_aliasType);
+        EXPECT_EQ(Spawnable::EntityAliasType::Disable, visitor.begin()->m_aliasType);
         EXPECT_EQ(Spawnable::EntityAliasType::Replace, visitor.begin()[1].m_aliasType);
     }
 

Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Spawnable/PrefabProcessorContext.h

@@ -27,7 +27,7 @@ namespace AzToolsFramework::Prefab::PrefabConversionUtils
 {
     enum class EntityAliasType : uint8_t
     {
-        Disabled,        //!< No alias is added.
+        Disable,         //!< No alias is added.
         OptionalReplace, //!< At runtime the entity might be replaced. If the alias is disabled the original entity will be spawned.
                          //!<   The original entity will be left in the spawnable and a copy is returned.
         Replace,         //!< At runtime the entity will be replaced. If the alias is disabled nothing will be spawned not. The original

Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Spawnable/SpawnableUtils.cpp

@@ -114,9 +114,9 @@ namespace AzToolsFramework::Prefab::SpawnableUtils
 
             switch (aliasType)
             {
-            case PCU::EntityAliasType::Disabled:
+            case PCU::EntityAliasType::Disable:
                 // No need to do anything as the alias is disabled.
-                return ResultPair(nullptr, AzFramework::Spawnable::EntityAliasType::Disabled);
+                return ResultPair(nullptr, AzFramework::Spawnable::EntityAliasType::Disable);
             case PCU::EntityAliasType::OptionalReplace:
                 return ResultPair(CloneEntity(entity, source), AzFramework::Spawnable::EntityAliasType::Replace);
             case PCU::EntityAliasType::Replace:
@@ -129,7 +129,7 @@ namespace AzToolsFramework::Prefab::SpawnableUtils
             default:
                 AZ_Assert(
                     false, "Invalid PrefabProcessorContext::EntityAliasType type (%i) provided.", aznumeric_cast<uint64_t>(aliasType));
-                return ResultPair(nullptr, AzFramework::Spawnable::EntityAliasType::Disabled);
+                return ResultPair(nullptr, AzFramework::Spawnable::EntityAliasType::Disable);
             }
         }
     }