diff --git a/src/entt/entity/sparse_set.hpp b/src/entt/entity/sparse_set.hpp index 6733042ce..b2f016809 100644 --- a/src/entt/entity/sparse_set.hpp +++ b/src/entt/entity/sparse_set.hpp @@ -200,9 +200,7 @@ class basic_sparse_set { std::uninitialized_fill(sparse[page], sparse[page] + traits_type::page_size, null); } - auto &elem = sparse[page][fast_mod(pos, traits_type::page_size)]; - ENTT_ASSERT(elem == null, "Slot not available"); - return elem; + return sparse[page][fast_mod(pos, traits_type::page_size)]; } void release_sparse_pages() { @@ -247,7 +245,7 @@ protected: * @param len The length to use. */ void swap_only_length_temporary_function(const std::size_t len) { - ENTT_ASSERT(mode == deletion_policy::swap_only, "Deletion policy mismatched"); + ENTT_ASSERT(mode == deletion_policy::swap_only, "Deletion policy mismatch"); head = static_cast(len); } @@ -256,18 +254,10 @@ protected: * @param it An iterator to the element to pop. */ void swap_only(const basic_iterator it) { - ENTT_ASSERT(mode == deletion_policy::swap_only, "Deletion policy mismatched"); - - if(const auto pos = static_cast(index(*it)); pos < head) { - bump(traits_type::next(*it)); - - if(const auto next = head - 1u; pos != next) { - swap_elements(packed[pos], packed[next]); - } - - // partition check support in derived classes - --head; - } + ENTT_ASSERT(mode == deletion_policy::swap_only, "Deletion policy mismatch"); + const auto pos = static_cast(index(*it)); + bump(traits_type::next(*it)); + swap_at(pos, static_cast(head -= (pos < head))); } /** @@ -275,7 +265,7 @@ protected: * @param it An iterator to the element to pop. */ void swap_and_pop(const basic_iterator it) { - ENTT_ASSERT(mode == deletion_policy::swap_and_pop, "Deletion policy mismatched"); + ENTT_ASSERT(mode == deletion_policy::swap_and_pop, "Deletion policy mismatch"); auto &self = sparse_ref(*it); const auto entt = traits_type::to_entity(self); sparse_ref(packed.back()) = traits_type::combine(entt, traits_type::to_integral(packed.back())); @@ -292,7 +282,7 @@ protected: * @param it An iterator to the element to pop. */ void in_place_pop(const basic_iterator it) { - ENTT_ASSERT(mode == deletion_policy::in_place, "Deletion policy mismatched"); + ENTT_ASSERT(mode == deletion_policy::in_place, "Deletion policy mismatch"); const auto entt = traits_type::to_entity(std::exchange(sparse_ref(*it), null)); packed[static_cast(entt)] = traits_type::combine(std::exchange(head, entt), tombstone); } @@ -355,23 +345,38 @@ protected: * @return Iterator pointing to the emplaced element. */ virtual basic_iterator try_emplace(const Entity entt, const bool force_back, const void * = nullptr) { - ENTT_ASSERT(!contains(entt), "Set already contains entity"); + auto &elem = assure_at_least(entt); + auto pos = size(); - switch(auto &elem = assure_at_least(entt); mode) { + switch(mode) { case deletion_policy::in_place: if(head != null && !force_back) { - const auto pos = static_cast(head); + pos = static_cast(head); + ENTT_ASSERT(elem == null, "Slot not available"); elem = traits_type::combine(head, traits_type::to_integral(entt)); head = traits_type::to_entity(std::exchange(packed[pos], entt)); - return --(end() - pos); + break; } [[fallthrough]]; - case deletion_policy::swap_only: case deletion_policy::swap_and_pop: packed.push_back(entt); + ENTT_ASSERT(elem == null, "Slot not available"); elem = traits_type::combine(static_cast(packed.size() - 1u), traits_type::to_integral(entt)); - return begin(); + break; + case deletion_policy::swap_only: + if(elem == null) { + packed.push_back(entt); + elem = traits_type::combine(static_cast(packed.size() - 1u), traits_type::to_integral(entt)); + } else { + ENTT_ASSERT(!(traits_type::to_entity(elem) < head), "Slot not available"); + bump(entt); + } + + swap_at(static_cast(traits_type::to_entity(elem)), (pos = static_cast(head++))); + break; } + + return --(end() - pos); } public: diff --git a/src/entt/entity/storage.hpp b/src/entt/entity/storage.hpp index 762214b19..c0722e880 100644 --- a/src/entt/entity/storage.hpp +++ b/src/entt/entity/storage.hpp @@ -1073,8 +1073,8 @@ public: */ entity_type emplace() { const auto len = base_type::free_list(); - base_type::swap_only_length_temporary_function(len + 1u); - return (len == base_type::size()) ? *base_type::try_emplace(entity_at(len), true) : base_type::operator[](len); + const auto entt = (len == base_type::size()) ? entity_at(len) : base_type::at(len); + return *base_type::try_emplace(entt, true); } /** @@ -1093,21 +1093,19 @@ public: const auto pos = static_cast(local_traits_type::to_entity(hint)); while(!(pos < base_type::size())) { - base_type::try_emplace(entity_at(base_type::size()), true); + const auto it = base_type::try_emplace(entity_at(base_type::size()), true); + const auto entt = *it; + + base_type::swap_only(it); + base_type::bump(entt); } - base_type::swap_elements(base_type::data()[pos], base_type::data()[base_type::free_list()]); - base_type::swap_only_length_temporary_function(base_type::free_list() + 1u); + return *base_type::try_emplace(hint, true); } else if(const auto idx = base_type::index(curr); idx < base_type::free_list()) { return emplace(); } else { - base_type::swap_elements(base_type::data()[idx], base_type::data()[base_type::free_list()]); - base_type::swap_only_length_temporary_function(base_type::free_list() + 1u); + return *base_type::try_emplace(hint, true); } - - base_type::bump(hint); - - return hint; } /** @@ -1130,13 +1128,12 @@ public: */ template void insert(It first, It last) { - for(const auto sz = base_type::size(); first != last && base_type::free_list() != sz; ++first, base_type::swap_only_length_temporary_function(base_type::free_list() + 1u)) { - *first = base_type::operator[](base_type::free_list()); + for(const auto sz = base_type::size(); first != last && base_type::free_list() != sz; ++first) { + *first = *base_type::try_emplace(base_type::at(base_type::free_list()), true); } for(; first != last; ++first) { *first = *base_type::try_emplace(entity_at(base_type::free_list()), true); - base_type::swap_only_length_temporary_function(base_type::free_list() + 1u); } } diff --git a/test/entt/entity/registry.cpp b/test/entt/entity/registry.cpp index 70cda4d44..075070280 100644 --- a/test/entt/entity/registry.cpp +++ b/test/entt/entity/registry.cpp @@ -555,8 +555,8 @@ TEST(Registry, CreateWithHint) { auto e3 = registry.create(entt::entity{3}); auto e2 = registry.create(entt::entity{3}); - ASSERT_EQ(e2, entt::entity{1}); - ASSERT_FALSE(registry.valid(entt::entity{0})); + ASSERT_EQ(e2, entt::entity{0}); + ASSERT_FALSE(registry.valid(entt::entity{1})); ASSERT_FALSE(registry.valid(entt::entity{2})); ASSERT_EQ(e3, entt::entity{3}); @@ -568,7 +568,7 @@ TEST(Registry, CreateWithHint) { e2 = registry.create(); auto e1 = registry.create(entt::entity{2}); - ASSERT_EQ(traits_type::to_entity(e2), 1u); + ASSERT_EQ(traits_type::to_entity(e2), 0u); ASSERT_EQ(traits_type::to_version(e2), 1u); ASSERT_EQ(traits_type::to_entity(e1), 2u); diff --git a/test/entt/entity/sparse_set.cpp b/test/entt/entity/sparse_set.cpp index 97ada8a15..d9b4c2fc7 100644 --- a/test/entt/entity/sparse_set.cpp +++ b/test/entt/entity/sparse_set.cpp @@ -559,6 +559,7 @@ TEST(SparseSet, StableErase) { set.erase(entity[1u]); ASSERT_EQ(set.size(), 3u); + ASSERT_EQ(set.free_list(), traits_type::entity_mask); ASSERT_EQ(set.current(entity[0u]), traits_type::to_version(entt::tombstone)); ASSERT_EQ(set.current(entity[1u]), traits_type::to_version(entt::tombstone)); ASSERT_EQ(set.current(entity[2u]), traits_type::to_version(entt::tombstone)); @@ -573,6 +574,7 @@ TEST(SparseSet, StableErase) { set.push(entt::entity{0}); ASSERT_EQ(set.size(), 4u); + ASSERT_EQ(set.free_list(), traits_type::entity_mask); ASSERT_EQ(*set.begin(), entt::entity{0}); ASSERT_EQ(set.at(0u), entity[1u]); ASSERT_EQ(set.at(1u), entity[0u]); @@ -621,18 +623,51 @@ TEST(SparseSet, SwapOnlyErase) { set.erase(set.begin(), set.end()); ASSERT_FALSE(set.empty()); - ASSERT_EQ(set.size(), 3u); ASSERT_EQ(set.free_list(), 0u); + entity[0u] = traits_type::next(entity[0u]); + entity[1u] = traits_type::next(entity[1u]); + entity[2u] = traits_type::next(entity[2u]); + + ASSERT_EQ(set.current(entity[0u]), traits_type::to_version(entity[0u])); + ASSERT_EQ(set.current(entity[1u]), traits_type::to_version(entity[1u])); + ASSERT_EQ(set.current(entity[2u]), traits_type::to_version(entity[2u])); + + set.push(std::begin(entity), std::end(entity)); + set.erase(entity, entity + 2u); + + ASSERT_FALSE(set.empty()); + ASSERT_EQ(set.free_list(), 1u); + + entity[0u] = traits_type::next(entity[0u]); + entity[1u] = traits_type::next(entity[1u]); + + ASSERT_EQ(set.current(entity[0u]), traits_type::to_version(entity[0u])); + ASSERT_EQ(set.current(entity[1u]), traits_type::to_version(entity[1u])); + ASSERT_EQ(set.current(entity[2u]), traits_type::to_version(entity[2u])); + ASSERT_EQ(*set.begin(), entity[0u]); + set.erase(entity[2u]); ASSERT_FALSE(set.empty()); - ASSERT_EQ(set.size(), 3u); ASSERT_EQ(set.free_list(), 0u); - ASSERT_EQ(set.at(0u), entity[0u]); - ASSERT_EQ(set.at(1u), entity[1u]); - ASSERT_EQ(set.at(2u), entity[2u]); + entity[2u] = traits_type::next(entity[2u]); + + ASSERT_EQ(set.current(entity[2u]), traits_type::to_version(entity[2u])); + + set.push(std::begin(entity), std::end(entity)); + std::swap(entity[1u], entity[2u]); + set.erase(entity, entity + 2u); + + ASSERT_FALSE(set.empty()); + ASSERT_EQ(set.free_list(), 1); + + entity[0u] = traits_type::next(entity[0u]); + entity[1u] = traits_type::next(entity[1u]); + + ASSERT_EQ(set.current(entity[2u]), traits_type::to_version(entity[2u])); + ASSERT_EQ(*set.begin(), entity[0u]); } ENTT_DEBUG_TEST(SparseSetDeathTest, SwapOnlyErase) { @@ -646,6 +681,8 @@ ENTT_DEBUG_TEST(SparseSetDeathTest, SwapOnlyErase) { } TEST(SparseSet, CrossSwapOnlyErase) { + using traits_type = entt::entt_traits; + entt::sparse_set set{entt::deletion_policy::swap_only}; entt::sparse_set other{entt::deletion_policy::swap_only}; entt::entity entity[2u]{entt::entity{3}, entt::entity{42}}; @@ -653,6 +690,7 @@ TEST(SparseSet, CrossSwapOnlyErase) { set.push(std::begin(entity), std::end(entity)); other.push(entity[1u]); set.erase(other.begin(), other.end()); + entity[1u] = traits_type::next(entity[1u]); ASSERT_TRUE(set.contains(entity[0u])); ASSERT_TRUE(set.contains(entity[1u])); @@ -891,22 +929,73 @@ TEST(SparseSet, SwapOnlyRemove) { set.push(std::begin(entity), std::end(entity)); - ASSERT_EQ(set.remove(set.begin(), set.end()), 3u); ASSERT_EQ(set.remove(set.begin(), set.end()), 3u); ASSERT_FALSE(set.empty()); - ASSERT_EQ(set.size(), 3u); + + entity[0u] = traits_type::next(entity[0u]); + entity[1u] = traits_type::next(entity[1u]); + entity[2u] = traits_type::next(entity[2u]); + + ASSERT_EQ(set.current(entity[0u]), traits_type::to_version(entity[0u])); + ASSERT_EQ(set.current(entity[1u]), traits_type::to_version(entity[1u])); + ASSERT_EQ(set.current(entity[2u]), traits_type::to_version(entity[2u])); + + set.push(std::begin(entity), std::end(entity)); + + ASSERT_EQ(set.remove(entity, entity + 2u), 2u); + ASSERT_FALSE(set.empty()); + + entity[0u] = traits_type::next(entity[0u]); + entity[1u] = traits_type::next(entity[1u]); + + ASSERT_EQ(set.current(entity[0u]), traits_type::to_version(entity[0u])); + ASSERT_EQ(set.current(entity[1u]), traits_type::to_version(entity[1u])); + ASSERT_EQ(set.current(entity[2u]), traits_type::to_version(entity[2u])); + ASSERT_EQ(*set.begin(), entity[0u]); ASSERT_TRUE(set.remove(entity[2u])); - ASSERT_TRUE(set.remove(entity[2u])); - ASSERT_FALSE(set.empty()); - ASSERT_EQ(set.size(), 3u); + ASSERT_FALSE(set.remove(entity[2u])); - ASSERT_EQ(set.at(0u), entity[0u]); - ASSERT_EQ(set.at(1u), entity[1u]); - ASSERT_EQ(set.at(2u), entity[2u]); + entity[2u] = traits_type::next(entity[2u]); + + ASSERT_TRUE(set.remove(entity[2u])); + ASSERT_FALSE(set.remove(entity[2u])); + ASSERT_FALSE(set.empty()); + ASSERT_EQ(set.current(entity[2u]), traits_type::to_version(traits_type::next(entity[2u]))); + + set.push(entity, entity + 2u); + + ASSERT_EQ(set.remove(std::begin(entity), std::end(entity)), 2u); + + entity[0u] = traits_type::next(entity[0u]); + entity[1u] = traits_type::next(entity[1u]); + entity[2u] = traits_type::next(entity[2u]); + + ASSERT_EQ(set.current(entity[0u]), traits_type::to_version(entity[0u])); + ASSERT_EQ(set.current(entity[1u]), traits_type::to_version(entity[1u])); + ASSERT_EQ(set.current(entity[2u]), traits_type::to_version(entity[2u])); + ASSERT_FALSE(set.empty()); + + set.push(std::begin(entity), std::end(entity)); + std::swap(entity[1u], entity[2u]); + + ASSERT_EQ(set.remove(entity, entity + 2u), 2u); + + entity[0u] = traits_type::next(entity[0u]); + entity[1u] = traits_type::next(entity[1u]); + + ASSERT_EQ(set.current(entity[2u]), traits_type::to_version(entity[2u])); + ASSERT_FALSE(set.empty()); + ASSERT_EQ(*set.begin(), entity[0u]); + + ASSERT_FALSE(set.remove(traits_type::construct(9, 0))); + ASSERT_FALSE(set.remove(entt::tombstone)); + ASSERT_FALSE(set.remove(entt::null)); } TEST(SparseSet, CrossSwapOnlyRemove) { + using traits_type = entt::entt_traits; + entt::sparse_set set{entt::deletion_policy::swap_only}; entt::sparse_set other{entt::deletion_policy::swap_only}; entt::entity entity[2u]{entt::entity{3}, entt::entity{42}}; @@ -914,6 +1003,7 @@ TEST(SparseSet, CrossSwapOnlyRemove) { set.push(std::begin(entity), std::end(entity)); other.push(entity[1u]); set.remove(other.begin(), other.end()); + entity[1u] = traits_type::next(entity[1u]); ASSERT_TRUE(set.contains(entity[0u])); ASSERT_TRUE(set.contains(entity[1u]));