diff --git a/src/entt/entity/sparse_set.hpp b/src/entt/entity/sparse_set.hpp index 6650bd50a..22904f78e 100644 --- a/src/entt/entity/sparse_set.hpp +++ b/src/entt/entity/sparse_set.hpp @@ -173,11 +173,25 @@ class basic_sparse_set { } protected: - /*! @brief Swaps two entities in the internal packed array. */ - virtual void swap_at(const std::size_t, const std::size_t) {} + /** + * @brief Swaps two entities in the internal packed array. + * @param lhs A valid position of an entity within storage. + * @param rhs A valid position of an entity within storage. + */ + virtual void swap_at(const std::size_t lhs, const std::size_t rhs) {} - /*! @brief Attempts to remove an entity from the internal packed array. */ - virtual void swap_and_pop(const std::size_t, void *) {} + /** + * @brief Attempts to remove an entity from the internal packed array. + * @param pos A valid position of an entity within storage. + */ + virtual void swap_and_pop(const std::size_t pos) {} + + /** + * @brief Last chance to use an entity that is about to be removed. + * @param entity A valid entity identifier. + * @param ud Optional user data that are forwarded as-is to derived classes. + */ + virtual void about_to_remove(const Entity entity, void *ud) {} public: /*! @brief Underlying entity identifier. */ @@ -445,19 +459,23 @@ public: */ void remove(const entity_type entt, void *ud = nullptr) { ENTT_ASSERT(contains(entt), "Set does not contain entity"); - auto &ref = sparse[page(entt)][offset(entt)]; // last chance to use the entity for derived classes and mixins, if any - swap_and_pop(size_type{to_integral(ref)}, ud); + about_to_remove(entt, ud); + + auto &ref = sparse[page(entt)][offset(entt)]; + const auto pos = size_type{to_integral(ref)}; const auto other = packed.back(); sparse[page(other)][offset(other)] = ref; - // if it looks weird, imagine what the subtle bugs it prevents are - ENTT_ASSERT((packed.back() = entt, true), ""); - packed[size_type{to_integral(ref)}] = other; ref = null; + // if it looks weird, imagine what the subtle bugs it prevents are + ENTT_ASSERT((packed.back() = entt, true), ""); + packed[pos] = other; packed.pop_back(); + + swap_and_pop(pos); } /** diff --git a/src/entt/entity/storage.hpp b/src/entt/entity/storage.hpp index 85259a785..be7308c51 100644 --- a/src/entt/entity/storage.hpp +++ b/src/entt/entity/storage.hpp @@ -159,22 +159,16 @@ class basic_storage: public basic_sparse_set { }; protected: - /** - * @copybrief basic_sparse_set::swap_at - * @param lhs A valid position of an entity within storage. - * @param rhs A valid position of an entity within storage. - */ - void swap_at(const std::size_t lhs, const std::size_t rhs) { + /*! @copydoc basic_sparse_set::swap_at */ + void swap_at(const std::size_t lhs, const std::size_t rhs) final { std::swap(instances[lhs], instances[rhs]); } - /** - * @copybrief basic_sparse_set::swap_and_pop - * @param pos A valid position of an entity within storage. - */ - void swap_and_pop(const std::size_t pos, void *) { - auto other = std::move(instances.back()); - instances[pos] = std::move(other); + /*! @copydoc basic_sparse_set::swap_and_pop */ + void swap_and_pop(const std::size_t pos) final { + // required because pop_back isn't guaranteed to shrink before removing the element + [[maybe_unused]] auto other = std::move(instances[pos]); + instances[pos] = std::move(instances.back()); instances.pop_back(); } @@ -632,17 +626,13 @@ struct storage_adapter_mixin: Type { */ template class sigh_storage_mixin final: public Type { - /** - * @copybrief basic_sparse_set::swap_and_pop - * @param pos A valid position of an entity within storage. - * @param ud Optional user data that are forwarded as-is to derived classes. - */ - void swap_and_pop(const std::size_t pos, void *ud) final { + using Entity = typename Type::entity_type; + + /*! @copydoc basic_sparse_set::about_to_remove */ + void about_to_remove(const Entity entity, void *ud) final { ENTT_ASSERT(ud != nullptr, "Invalid pointer to registry"); - const auto entity = basic_sparse_set::operator[](pos); destruction.publish(*static_cast *>(ud), entity); - // the position may have changed due to the actions of a listener - Type::swap_and_pop(this->index(entity), ud); + Type::about_to_remove(entity, ud); } public: diff --git a/test/entt/entity/storage.cpp b/test/entt/entity/storage.cpp index 4b1f0ffb1..17461c4f7 100644 --- a/test/entt/entity/storage.cpp +++ b/test/entt/entity/storage.cpp @@ -24,6 +24,17 @@ struct throwing_component { int data; }; +struct update_from_destructor { + ~update_from_destructor() { + if(target != entt::null) { + storage->remove(target); + } + } + + entt::storage *storage{}; + entt::entity target{entt::null}; +}; + TEST(Storage, Functionalities) { entt::storage pool; @@ -649,3 +660,28 @@ TEST(Storage, ConstructorExceptionDoesNotAddToStorage) { ASSERT_TRUE(pool.empty()); } + +TEST(Storage, UpdateFromDestructor) { + static constexpr auto size = 10u; + + auto test = [](const auto target) { + entt::storage pool; + + for(std::size_t next{}; next < size; ++next) { + pool.emplace(entt::entity(next), &pool); + } + + for(std::size_t next{}; next < size; ++next) { + ASSERT_EQ(pool.get(entt::entity(next)).target, entt::entity{entt::null}); + } + + pool.get(entt::entity(size/2)).target = target; + pool.remove(entt::entity(size/2)); + + ASSERT_EQ(pool.size(), size - 1u - (target != entt::null)); + }; + + test(entt::null); + test(entt::entity(size - 1u)); + test(entt::entity{0u}); +}