From 045a826c681c1ce903fa7924c7f1d781d784d238 Mon Sep 17 00:00:00 2001 From: AMZN-koppersr <82230785+AMZN-koppersr@users.noreply.github.com> Date: Mon, 1 Nov 2021 14:18:04 -0700 Subject: [PATCH] Updates to the Spawnable entity aliases based on provided feedback on PR. Signed-off-by: AMZN-koppersr <82230785+AMZN-koppersr@users.noreply.github.com> --- .../Spawnable/RootSpawnableInterface.h | 4 ++-- .../AzFramework/Spawnable/Spawnable.cpp | 16 +++++----------- .../AzFramework/Spawnable/Spawnable.h | 7 +++++-- .../Spawnable/SpawnableAssetHandler.cpp | 1 + .../Spawnable/SpawnableEntitiesContainer.h | 4 ++-- .../Spawnable/SpawnableEntitiesManager.cpp | 2 +- .../Spawnable/SpawnableEntitiesManagerTests.cpp | 12 ++++++------ .../Tests/Spawnable/SpawnableTests.cpp | 8 ++++---- .../Prefab/Spawnable/PrefabProcessorContext.h | 2 +- .../Prefab/Spawnable/SpawnableUtils.cpp | 6 +++--- 10 files changed, 30 insertions(+), 32 deletions(-) diff --git a/Code/Framework/AzFramework/AzFramework/Spawnable/RootSpawnableInterface.h b/Code/Framework/AzFramework/AzFramework/Spawnable/RootSpawnableInterface.h index 873123f38b..d3fe62eae3 100644 --- a/Code/Framework/AzFramework/AzFramework/Spawnable/RootSpawnableInterface.h +++ b/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( diff --git a/Code/Framework/AzFramework/AzFramework/Spawnable/Spawnable.cpp b/Code/Framework/AzFramework/AzFramework/Spawnable/Spawnable.cpp index 7548a8f181..a3cbf861cb 100644 --- a/Code/Framework/AzFramework/AzFramework/Spawnable/Spawnable.cpp +++ b/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; diff --git a/Code/Framework/AzFramework/AzFramework/Spawnable/Spawnable.h b/Code/Framework/AzFramework/AzFramework/Spawnable/Spawnable.h index 0a35b81fb1..9058ac7ba4 100644 --- a/Code/Framework/AzFramework/AzFramework/Spawnable/Spawnable.h +++ b/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. diff --git a/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableAssetHandler.cpp b/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableAssetHandler.cpp index 0cd9891948..6ef423fa91 100644 --- a/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableAssetHandler.cpp +++ b/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& assetPendingLoad) diff --git a/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesContainer.h b/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesContainer.h index 1ec6e6a665..cabef38ff5 100644 --- a/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesContainer.h +++ b/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); diff --git a/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesManager.cpp b/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesManager.cpp index d7d3f59154..33fe1601af 100644 --- a/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesManager.cpp +++ b/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: diff --git a/Code/Framework/AzFramework/Tests/Spawnable/SpawnableEntitiesManagerTests.cpp b/Code/Framework/AzFramework/Tests/Spawnable/SpawnableEntitiesManagerTests.cpp index 38847edc07..83c895bc7a 100644 --- a/Code/Framework/AzFramework/Tests/Spawnable/SpawnableEntitiesManagerTests.cpp +++ b/Code/Framework/AzFramework/Tests/Spawnable/SpawnableEntitiesManagerTests.cpp @@ -485,8 +485,8 @@ namespace UnitTest FillSpawnable(NumEntities); InsertEntityAliases( { 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( { 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 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 indices = { 0, 2, 3, 1, 2, 3, 0, 1, 6, 4, 5, 7, 4, 1, 0, 6 }; diff --git a/Code/Framework/AzFramework/Tests/Spawnable/SpawnableTests.cpp b/Code/Framework/AzFramework/Tests/Spawnable/SpawnableTests.cpp index f7d6d5190f..c689295f17 100644 --- a/Code/Framework/AzFramework/Tests/Spawnable/SpawnableTests.cpp +++ b/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); } diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Spawnable/PrefabProcessorContext.h b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Spawnable/PrefabProcessorContext.h index c937b974e9..8e29deadca 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Spawnable/PrefabProcessorContext.h +++ b/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 diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Spawnable/SpawnableUtils.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Spawnable/SpawnableUtils.cpp index 154337e957..7b14ad1228 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Spawnable/SpawnableUtils.cpp +++ b/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(aliasType)); - return ResultPair(nullptr, AzFramework::Spawnable::EntityAliasType::Disabled); + return ResultPair(nullptr, AzFramework::Spawnable::EntityAliasType::Disable); } } }