Improve prefab instance API and add Doxygen comments (#12761)

* Improve prefab instance API and add doxygen comments

Signed-off-by: Junhao Wang <[email protected]>

* Adds a missing comment

Signed-off-by: Junhao Wang <[email protected]>

* Update grouping comments

Signed-off-by: Junhao Wang <[email protected]>

* Fix typo and address PR feedback

Signed-off-by: Junhao Wang <[email protected]>

Signed-off-by: Junhao Wang <[email protected]>

Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Instance/Instance.cpp

@@ -223,10 +223,10 @@ namespace AzToolsFramework
                 }
             }
 
-            return DetachEntity(entityAliasToRemove);
+            return DetachEntityHelper(entityAliasToRemove);
         }
 
-        AZStd::unique_ptr<AZ::Entity> Instance::DetachEntity(const EntityAlias& entityAlias)
+        AZStd::unique_ptr<AZ::Entity> Instance::DetachEntityHelper(const EntityAlias& entityAlias)
         {
             AZStd::unique_ptr<AZ::Entity> removedEntity;
             auto&& entityIterator = m_entities.find(entityAlias);
@@ -283,14 +283,14 @@ namespace AzToolsFramework
             return result;
         }
 
-        void Instance::RemoveNestedEntities(
+        void Instance::RemoveEntitiesInHierarchy(
             const AZStd::function<bool(const AZStd::unique_ptr<AZ::Entity>&)>& filter)
         {
             RemoveEntities(filter);
 
             for (const auto& [instanceAlias, instance] : m_nestedInstances)
             {
-                instance->RemoveNestedEntities(filter);
+                instance->RemoveEntitiesInHierarchy(filter);
             }
         }
 
@@ -467,16 +467,6 @@ namespace AzToolsFramework
             return m_entities.size();
         }
 
-        void Instance::GetNestedEntityIds(const AZStd::function<bool(AZ::EntityId)>& callback) const
-        {
-            GetEntityIds(callback);
-
-            for (auto&&[instanceAlias, instance] : m_nestedInstances)
-            {
-                instance->GetNestedEntityIds(callback);
-            }
-        }
-
         void Instance::GetEntityIds(const AZStd::function<bool(AZ::EntityId)>& callback) const
         {
             for (auto&&[entityAlias, entityId] : m_templateToInstanceEntityIdMap)
@@ -597,6 +587,16 @@ namespace AzToolsFramework
             GetConstEntities_Impl(callback);
         }
 
+        void Instance::GetAllEntityIdsInHierarchy(const AZStd::function<bool(AZ::EntityId)>& callback) const
+        {
+            GetEntityIds(callback);
+
+            for (auto&& [instanceAlias, instance] : m_nestedInstances)
+            {
+                instance->GetAllEntityIdsInHierarchy(callback);
+            }
+        }
+
         void Instance::GetAllEntitiesInHierarchy(const AZStd::function<bool(AZStd::unique_ptr<AZ::Entity>&)>& callback)
         {
             GetAllEntitiesInHierarchy_Impl(callback);
@@ -730,6 +730,11 @@ namespace AzToolsFramework
             return nestedInstanceAliases;
         }
 
+        const InstanceAlias& Instance::GetInstanceAlias() const
+        {
+            return m_alias;
+        }
+
         AliasPath Instance::GetAbsoluteInstanceAliasPath() const
         {
             // Reset the path using our preferred separator
@@ -849,11 +854,6 @@ namespace AzToolsFramework
             return m_parent != nullptr;
         }
 
-        const InstanceAlias& Instance::GetInstanceAlias() const
-        {
-            return m_alias;
-        }
-
         bool Instance::IsParentInstance(const Instance& instance) const
         {
             return &instance == m_parent;

Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Instance/Instance.h

@@ -53,13 +53,14 @@ namespace AzToolsFramework
         using EntityOptionalReference = AZStd::optional<AZStd::reference_wrapper<AZ::Entity>>;
         using EntityOptionalConstReference = AZStd::optional<AZStd::reference_wrapper<const AZ::Entity>>;
 
+        //! Enum class that represents entity id and instance relationship.
         enum class EntityIdInstanceRelationship : uint8_t
         {
             OneToOne,
             OneToMany
         };
 
-        // A prefab instance is the container for when a Prefab Template is Instantiated.
+        //! Class that represents a prefab instance instantiated from a prefab template.
         class Instance
         {
         public:
@@ -71,6 +72,8 @@ namespace AzToolsFramework
             using AliasToInstanceMap = AZStd::unordered_map<InstanceAlias, AZStd::unique_ptr<Instance>>;
             using AliasToEntityMap = AZStd::unordered_map<EntityAlias, AZStd::unique_ptr<AZ::Entity>>;
 
+            //! Construtors.
+            //! @{
             Instance();
             explicit Instance(AZStd::unique_ptr<AZ::Entity> containerEntity);
             explicit Instance(InstanceOptionalReference parent);
@@ -80,182 +83,276 @@ namespace AzToolsFramework
             explicit Instance(InstanceAlias alias);
             explicit Instance(EntityIdInstanceRelationship entityIdInstanceRelationship);
             Instance(InstanceAlias alias, EntityIdInstanceRelationship entityIdInstanceRelationship);
+            //! @}
+
+            //! Destructor.
             virtual ~Instance();
 
+            //! Disables copy constructor and copy assignment.
+            //! @{
             Instance(const Instance& rhs) = delete;
             Instance& operator=(const Instance& rhs) = delete;
+            //! @}
 
+            //! AzCore Reflection.
+            //! @param context reflection context.
             static void Reflect(AZ::ReflectContext* context);
 
+            //! Gets template id.
+            //! @return Template id of the instance.
             TemplateId GetTemplateId() const;
+
+            //! Sets template id.
+            //! @param templateId Template id to be set.
             void SetTemplateId(TemplateId templateId);
 
+            //! Gets template source path.
+            //! @return Template source path of the instance.
             const AZ::IO::Path& GetTemplateSourcePath() const;
+
+            //! Sets template source path.
+            //! @param sourcePath Template source path to be set.
             void SetTemplateSourcePath(AZ::IO::Path sourcePath);
+
+            //! Sets container entity name.
+            //! @param containerName New container entity name to be set.
             void SetContainerEntityName(AZStd::string containerName);
 
+            //! Adds entity to the instance.
+            //! It registers the entity id and moves the entity ownership to the instance.
+            //! @{
             bool AddEntity(AZ::Entity& entity);
             bool AddEntity(AZStd::unique_ptr<AZ::Entity>&& entity);
             bool AddEntity(AZ::Entity& entity, EntityAlias entityAlias);
             bool AddEntity(AZStd::unique_ptr<AZ::Entity>&& entity, EntityAlias entityAlias);
+            //! @}
+
+            //! Detaches an entity from the instance.
+            //! It unregisters the entity id and moves the entity ownership to the caller.
+            //! @param entityId The entity id to be detached.
+            //! @return Unique pointer to the entity.
             AZStd::unique_ptr<AZ::Entity> DetachEntity(const AZ::EntityId& entityId);
+
+            //! Detaches entities from the instance.
+            //! It unregisters the entity ids and moves the entity ownerships to the callback provided by the caller.
+            //! @param callback A user provided callback that can be used to capture ownership and manipulate the detached entities.
             void DetachEntities(const AZStd::function<void(AZStd::unique_ptr<AZ::Entity>)>& callback);
-            /**
-             * Replaces the entity stored under the provided alias with a new one.
-             *
-             * @return The original entity or a nullptr if not found.
-             */
-            AZStd::unique_ptr<AZ::Entity> ReplaceEntity(AZStd::unique_ptr<AZ::Entity>&& entity, EntityAliasView alias);
 
-            /**
-            * Detaches all entities in the instance hierarchy.
-            * Includes all direct entities, all nested entities, and all container entities.
-            * Note that without container entities the hierarchy that remains cannot be used further without restoring new ones.
-            * @param callback A user provided callback that can be used to capture ownership and manipulate the detached entities.
-            */
+            //! Detaches all entities in the instance hierarchy.
+            //! Includes all direct entities, all nested entities, and all container entities.
+            //! Note that without container entities the hierarchy that remains cannot be used further without restoring new ones.
+            //! @param callback A user provided callback that can be used to capture ownership and manipulate the detached entities.
             void DetachAllEntitiesInHierarchy(const AZStd::function<void(AZStd::unique_ptr<AZ::Entity>)>& callback);
 
-            void RemoveNestedEntities(const AZStd::function<bool(const AZStd::unique_ptr<AZ::Entity>&)>& filter);
+            //! Replaces the entity stored under the provided alias with a new one.
+            //! @param entity The new entity.
+            //! @param alias The alias of the entity that will be replaced.
+            //! @return The original entity or a nullptr if not found.
+            AZStd::unique_ptr<AZ::Entity> ReplaceEntity(AZStd::unique_ptr<AZ::Entity>&& entity, EntityAliasView alias);
 
-            void Reset();
+            //! Detaches entities in the instance hierarchy based on the provided filter.
+            //! @param filter A user provided filter used to decide whether an entity should be detached. Returns true to detach.
+            void RemoveEntitiesInHierarchy(const AZStd::function<bool(const AZStd::unique_ptr<AZ::Entity>&)>& filter);
 
+            //! Adds a new nested instance.
+            //! @param instance The new nested instance.
+            //! @return Reference to the new nested instance.
             Instance& AddInstance(AZStd::unique_ptr<Instance> instance);
+
+            //! Detaches a nested instance from the instance.
+            //! It removes the nested instance and moves the instance ownership to the caller.
+            //! @param entityId The entity id to be detached.
+            //! @return Unique pointer to the entity.
             AZStd::unique_ptr<Instance> DetachNestedInstance(const InstanceAlias& instanceAlias);
+
+            //! Detaches entities from the instance.
+            //! It removes nested instances and moves the instance ownerships to the callback provided by the caller.
+            //! @param callback A user provided callback that can be used to capture ownership and manipulate the detached instances.
             void DetachNestedInstances(const AZStd::function<void(AZStd::unique_ptr<Instance>)>& callback);
 
-            /**
-            * Gets the aliases for the entities in the Instance DOM.
-            * 
-            * @return The list of EntityAliases
-            */
+            //! Resets the instance to an initial state.
+            //! It unregisters the instance, entities and nested instances.
+            void Reset();
+
+            //! Gets the aliases for the entities in the Instance DOM.
+            //! @return The list of EntityAliases.
             AZStd::vector<EntityAlias> GetEntityAliases();
-            size_t GetEntityAliasCount() const;
 
-            /**
-            * Gets the ids for the entities in the Instance DOM.  Can recursively trace all nested instances.
-            */
-            void GetNestedEntityIds(const AZStd::function<bool(AZ::EntityId)>& callback) const;
+            //! Gets the entity alias count in the Instance DOM.
+            //! @return The entity alias count.
+            size_t GetEntityAliasCount() const;
 
+            //! Gets the entities or entity ids in the Instance DOM.
+            //! A callback can be provided to manipulates the entities.
+            //! Note: These are non-recursive operations.
+            //! @{
             void GetEntityIds(const AZStd::function<bool(AZ::EntityId)>& callback) const;
             void GetEntityIdToAlias(const AZStd::function<bool(AZ::EntityId, EntityAliasView)>& callback) const;
-
-            /**
-            * Gets the entities in the Instance DOM.  Can recursively trace all nested instances.
-            */
             void GetEntities(const AZStd::function<bool(AZStd::unique_ptr<AZ::Entity>&)>& callback);
             void GetConstEntities(const AZStd::function<bool(const AZ::Entity&)>& callback) const;
+            //! @}
+
+            //! Gets the entity ids for all entities in hierarchy in the Instance DOM.
+            //! A callback can be provided to manipulates the entities.
+            //! Note: These are recursive operations that trace all nested instances.
+            //! @{
+            void GetAllEntityIdsInHierarchy(const AZStd::function<bool(AZ::EntityId)>& callback) const;
             void GetAllEntitiesInHierarchy(const AZStd::function<bool(AZStd::unique_ptr<AZ::Entity>&)>& callback);
             void GetAllEntitiesInHierarchyConst(const AZStd::function<bool(const AZ::Entity&)>& callback) const;
+            //! @}
+
+            //! Gets the nested instances in the instance DOM.
+            //! Note: This is a non-recursive operation.
+            //! @param callback A user provided callback that manipulates the nested instances.
             void GetNestedInstances(const AZStd::function<void(AZStd::unique_ptr<Instance>&)>& callback);
 
-            /**
-            * Gets the alias for a given EnitityId in the Instance DOM.
-            *
-            * @return entityAlias via optional
-            */
+            //! Gets the entity alias for a given entity id in the Instance DOM.
+            //! @{
             EntityAliasOptionalReference GetEntityAlias(AZ::EntityId id);
             EntityAliasView GetEntityAlias(AZ::EntityId id) const;
-            /**
-             * Searches for the entity in this instance and its nested instances.
-             *
-             * @return The instance that owns the entity and the alias under which the entity is known.
-             *      If the entity isn't found then the instance will be null and the alias empty.
-             */
+            //! @}
+
+            //! Searches for the entity in this instance and its nested instances.
+            //! If the entity isn't found then the returned instance will be null and the alias will be empty.
+            //! @{
             AZStd::pair<Instance*, EntityAliasView> FindInstanceAndAlias(AZ::EntityId entity);
             AZStd::pair<const Instance*, EntityAliasView> FindInstanceAndAlias(AZ::EntityId entity) const;
+            //! @}
 
+            //! Gets entity by a user provided entity alias.
+            //! @{
             EntityOptionalReference GetEntity(const EntityAlias& alias);
             EntityOptionalConstReference GetEntity(const EntityAlias& alias) const;
+            //! @}
 
-            /**
-            * Gets the id for a given EnitityAlias in the Instance DOM.
-            *
-            * @return entityId, invalid ID if not found
-            */
+            //! Gets the id for a given EnitityAlias in the Instance DOM.
+            //! @param alias Entity alias used to retrieve the entity id.
+            //! @return entityId, invalid ID if not found.
             AZ::EntityId GetEntityId(const EntityAlias& alias) const;
 
-            /**
-            * Retrieves the entity id from an alias path that's relative to this instance.
-            * 
-            * @return entityId, invalid ID if not found
-            */
+            //! Retrieves the entity id from an alias path that's relative to this instance.
+            //! @param relativeAliasPath Entity alias path relative to this instance.
+            //! @return entityId, invalid ID if not found.
             AZ::EntityId GetEntityIdFromAliasPath(AliasPathView relativeAliasPath) const;
-            /**
-             * Retrieves the instance pointer and entity id from an alias path that's relative to this instance.
-             *
-             * @return A pair with the Instance and entity id. The Instance is set to null and entityId is set to invalid if not found.
-             */
+
+            //! Retrieves the instance pointer and entity id from an alias path that's relative to this instance.
+            //! It returns a pair with the Instance and entity id. The Instance is set to null and entityId is set to invalid if not found.
+            //! @{
             AZStd::pair<Instance*, AZ::EntityId> GetInstanceAndEntityIdFromAliasPath(AliasPathView relativeAliasPath);
             AZStd::pair<const Instance*, AZ::EntityId> GetInstanceAndEntityIdFromAliasPath(AliasPathView relativeAliasPath) const;
+            //! @}
 
-            /**
-            * Gets the aliases of all the nested instances, which are sourced by the template with the given id.
-            * 
-            * @param templateId The source template id of the nested instances.
-            * @return The list of aliases of the nested instances.
-            */
+            //! Gets the aliases of all the nested instances, which are sourced by the template with the given id.
+            //! @param templateId The source template id of the nested instances.
+            //! @return The list of aliases of the nested instances.
             AZStd::vector<InstanceAlias> GetNestedInstanceAliases(TemplateId templateId) const;
 
+            //! Activates the container entity.
             void ActivateContainerEntity();
 
+            //! Finds a nested instance non-recursively by instance alias.
+            //! @{
             InstanceOptionalReference FindNestedInstance(const InstanceAlias& nestedInstanceAlias);
-
             InstanceOptionalConstReference FindNestedInstance(const InstanceAlias& nestedInstanceAlias) const;
+            //! @}
 
+            //! Sets link id.
+            //! @param linkId Link id to be set.
             void SetLinkId(LinkId linkId);
-
+            
+            //! Gets link id.
+            //! @return Link id of the instance.
             LinkId GetLinkId() const;
 
+            //! Query functions for parent instance.
+            //! @{
             InstanceOptionalReference GetParentInstance();
-
             InstanceOptionalConstReference GetParentInstance() const;
-
             bool HasParentInstance() const;
-
-            const InstanceAlias& GetInstanceAlias() const;
-
             bool IsParentInstance(const Instance& instance) const;
+            //! @}
 
+            //! Query, getter and setter functions for container entity.
+            //! @{
             AZ::EntityId GetContainerEntityId() const;
-
             bool HasContainerEntity() const;
-
             EntityOptionalReference GetContainerEntity();
             EntityOptionalConstReference GetContainerEntity() const;
-
             void SetContainerEntity(AZ::Entity& entity);
+            //! @}
 
+            //! Detaches the container entity from the instance.
+            //! It unregisters the entity id and moves the entity ownership to the caller.
+            //! @return Unique pointer to the container entity.
             AZStd::unique_ptr<AZ::Entity> DetachContainerEntity();
 
-            static EntityAlias GenerateEntityAlias();
+            //! Getter for instance alias.
+            //! @return Instance alias string.
+            const InstanceAlias& GetInstanceAlias() const;
+
+            //! Gets absolute instance alias path seen from root instance.
+            //! @return Alias path seen from root instance.
             AliasPath GetAbsoluteInstanceAliasPath() const;
+
+            //! Gets relative alias path to entity.
+            //! @param entity The entity that the relative path points to.
+            //! @return Alias path to the provided entity.
             AliasPath GetAliasPathRelativeToInstance(const AZ::EntityId& entity) const;
 
+            //! Entity and instance alias generator functions.
+            //! @{
+            static EntityAlias GenerateEntityAlias();
             static InstanceAlias GenerateInstanceAlias();
+            //! @}
 
+            //! Getter and setter for cached instance DOM.
+            //! @{
             PrefabDomConstReference GetCachedInstanceDom() const;
             PrefabDomReference GetCachedInstanceDom();
             void SetCachedInstanceDom(PrefabDomValueConstReference instanceDom);
+            //! @}
 
         private:
             static constexpr const char s_aliasPathSeparator = '/';
             static constexpr EntityIdInstanceRelationship DefaultEntityIdInstanceRelationship = EntityIdInstanceRelationship::OneToOne;
 
+            //! Private constructor called by public constructor.
             Instance(AZStd::unique_ptr<AZ::Entity> containerEntity, InstanceOptionalReference parent, InstanceAlias alias,
                 EntityIdInstanceRelationship entityIdInstanceRelationship = DefaultEntityIdInstanceRelationship);
 
+            //! Unregisters and erases all entities owned by this instance.
             void ClearEntities();
 
+            //! Detaches entities based on a user provided filter. Returns true to detach.
             void RemoveEntities(const AZStd::function<bool(const AZStd::unique_ptr<AZ::Entity>&)>& filter);
 
+            //! Functions for get-entity implementation.
+            //! @{
             bool GetEntities_Impl(const AZStd::function<bool(AZStd::unique_ptr<AZ::Entity>&)>& callback);
             bool GetConstEntities_Impl(const AZStd::function<bool(const AZ::Entity&)>& callback) const;
             bool GetAllEntitiesInHierarchy_Impl(const AZStd::function<bool(AZStd::unique_ptr<AZ::Entity>&)>& callback);
             bool GetAllEntitiesInHierarchyConst_Impl(const AZStd::function<bool(const AZ::Entity&)>& callback) const;
+            //! @}
 
+            //! Registers an entity for the instance.
+            //! It generates mapping from entity id to entity alias, and vice versa.
+            //! @param entityId The entity id to be registered.
+            //! @param entityAlias The entity alias to be registered.
+            //! @return Returns true if the registration succeeds.
             bool RegisterEntity(const AZ::EntityId& entityId, const EntityAlias& entityAlias);
+
+            //! Unregisters an entity for the instance.
+            //! It erases mapping from entity id to entity alias, and vice versa.
+            //! @param entityId The entity id to be unregistered.
+            //! @param entityAlias The entity alias to be unregistered.
+            //! @return Returns true if the unregistration succeeds.
             bool UnregisterEntity(AZ::EntityId entityId);
-            AZStd::unique_ptr<AZ::Entity> DetachEntity(const EntityAlias& entityAlias);
+
+            //! Helper function for detaching entity.
+            //! It removes an entity from map and returns entity pointer.
+            //! @param entityAlias The entity alias to be erased.
+            //! @return Unique pointer to the entity.
+            AZStd::unique_ptr<AZ::Entity> DetachEntityHelper(const EntityAlias& entityAlias);
 
             // Provide access to private data members in the serializer
             friend class JsonInstanceSerializer;
@@ -284,8 +381,8 @@ namespace AzToolsFramework
             // The source path of the template this instance represents
             AZ::IO::Path m_templateSourcePath;
 
-            //! This can be used to set the DOM that was last used for building the instance object through deserialization.
-            //! This is optional and will only be set when asked explicitly through SetCachedInstanceDom().
+            // This can be used to set the DOM that was last used for building the instance object through deserialization.
+            // This is optional and will only be set when asked explicitly through SetCachedInstanceDom().
             PrefabDom m_cachedInstanceDom;
 
             // The unique ID of the template this Instance belongs to.
@@ -300,6 +397,7 @@ namespace AzToolsFramework
             // Interface for registering the Instance itself for external queries.
             TemplateInstanceMapperInterface* m_templateInstanceMapper = nullptr;
 
+            // Defines entity id and instance relationship. The default relationship is one-to-one.
             EntityIdInstanceRelationship m_entityIdInstanceRelationship = DefaultEntityIdInstanceRelationship;
         };
     } // namespace Prefab

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

@@ -578,7 +578,7 @@ exportComponent, prefabProcessorContext);
                 exportEntitiesMap.emplace(entity->GetId(), entity);
             }
         );
-        sourceInstance.RemoveNestedEntities(
+        sourceInstance.RemoveEntitiesInHierarchy(
             [&exportEntitiesMap](const AZStd::unique_ptr<AZ::Entity>& entity)
             {
                 return exportEntitiesMap.find(entity->GetId()) == exportEntitiesMap.end();

Code/Framework/AzToolsFramework/Tests/Prefab/PrefabTestFixture.cpp

@@ -223,7 +223,7 @@ namespace UnitTest
     void PrefabTestFixture::ValidateInstanceEntitiesActive(Instance& instance)
     {
         AZStd::vector<AZ::EntityId> entityIdVector;
-        instance.GetNestedEntityIds([&entityIdVector](AZ::EntityId entityId) {
+        instance.GetAllEntityIdsInHierarchy([&entityIdVector](AZ::EntityId entityId) {
             entityIdVector.push_back(entityId);
             return true;
         });

Code/Framework/AzToolsFramework/Tests/Prefab/PrefabUpdateWithPatchesTests.cpp

@@ -78,7 +78,7 @@ namespace UnitTest
 
         //get the entity id
         AZStd::vector<AZ::EntityId> entityIdVector;
-        axleInstance->GetNestedEntityIds([&entityIdVector](AZ::EntityId entityId)
+        axleInstance->GetAllEntityIdsInHierarchy([&entityIdVector](AZ::EntityId entityId)
         {
             entityIdVector.push_back(entityId);
             return true;

Code/Tools/SerializeContextTools/SliceConverter.cpp

@@ -595,7 +595,7 @@ namespace AZ
             // throughout the entire nested hierarchy, then add the new patched entities back in at the appropriate place in the hierarchy.
             // (This is easier than trying to figure out what the patched data changes are - we can let the JSON patch handle it for us)
 
-            nestedInstance->RemoveNestedEntities(
+            nestedInstance->RemoveEntitiesInHierarchy(
                 [](const AZStd::unique_ptr<AZ::Entity>&)
                 {
                     return true;