entt_traits: review/cleanup

src/entt/entity/entity.hpp

@@ -117,32 +117,17 @@ public:
 
     /**
      * @brief Constructs an identifier from its parts.
+     *
+     * If the version part is not provided, a tombstone is returned.<br/>
+     * If the entity part is not provided, a null identifier is returned.
+     *
      * @param entity The entity part of the identifier.
      * @param version The version part of the identifier.
      * @return A properly constructed identifier.
      */
-    [[nodiscard]] static constexpr value_type to_value(const entity_type entity, const version_type version) ENTT_NOEXCEPT {
+    [[nodiscard]] static constexpr value_type construct(const entity_type entity = traits_type::entity_mask, const version_type version = traits_type::version_mask) ENTT_NOEXCEPT {
         return value_type{(entity & traits_type::entity_mask) | (version << traits_type::entity_shift)};
     }
-
-    /**
-     * @brief Constructs an identifier from its parts.
-     * @param entity The entity part of the identifier.
-     * @param version The version part of the identifier.
-     * @return A properly constructed identifier.
-     */
-    [[nodiscard]] static constexpr value_type to_value(const value_type entity, const value_type version) ENTT_NOEXCEPT {
-        constexpr auto mask = (traits_type::version_mask << traits_type::entity_shift);
-        return value_type{(to_integral(entity) & traits_type::entity_mask) | (to_integral(version) & mask)};
-    }
-
-    /**
-     * @brief Returns the reserved identifer.
-     * @return The reserved identifier.
-     */
-    [[nodiscard]] static constexpr value_type reserved() ENTT_NOEXCEPT {
-        return value_type{traits_type::entity_mask | (traits_type::version_mask << traits_type::entity_shift)};
-    }
 };
 
 
@@ -167,7 +152,7 @@ struct null_t {
      */
     template<typename Entity>
     [[nodiscard]] constexpr operator Entity() const ENTT_NOEXCEPT {
-        return entt_traits<Entity>::reserved();
+        return entt_traits<Entity>::construct();
     }
 
     /**
@@ -218,7 +203,7 @@ struct null_t {
      */
     template<typename Entity>
     [[nodiscard]] constexpr Entity operator|(const Entity entity) const ENTT_NOEXCEPT {
-        return entt_traits<Entity>::to_value(static_cast<Entity>(*this), entity);
+        return entt_traits<Entity>::construct(entt_traits<Entity>::to_entity(*this), entt_traits<Entity>::to_version(entity));
     }
 };
 
@@ -258,7 +243,7 @@ struct tombstone_t {
      */
     template<typename Entity>
     [[nodiscard]] constexpr operator Entity() const ENTT_NOEXCEPT {
-        return entt_traits<Entity>::reserved();
+        return entt_traits<Entity>::construct();
     }
 
     /**
@@ -309,7 +294,7 @@ struct tombstone_t {
      */
     template<typename Entity>
     [[nodiscard]] constexpr Entity operator|(const Entity entity) const ENTT_NOEXCEPT {
-        return entt_traits<Entity>::to_value(entity, static_cast<Entity>(*this));
+        return entt_traits<Entity>::construct(entt_traits<Entity>::to_entity(entity));
     }
 };
 

src/entt/entity/registry.hpp

@@ -129,20 +129,19 @@ class basic_registry {
 
     auto generate_identifier(const std::size_t pos) ENTT_NOEXCEPT {
         ENTT_ASSERT(pos < traits_type::to_integral(null), "No entities available");
-        return traits_type::to_value(static_cast<typename traits_type::entity_type>(pos), {});
+        return traits_type::construct(static_cast<typename traits_type::entity_type>(pos), {});
     }
 
     auto recycle_identifier() ENTT_NOEXCEPT {
         ENTT_ASSERT(free_list != null, "No entities available");
         const auto curr = traits_type::to_entity(free_list);
-        const auto entt = traits_type::to_value(free_list, entities[curr]);
         free_list = (tombstone | entities[curr]);
-        return (entities[curr] = entt);
+        return (entities[curr] = traits_type::construct(curr, traits_type::to_version(entities[curr])));
     }
 
     auto release_entity(const Entity entity, const typename traits_type::version_type version) {
         const auto entt = traits_type::to_entity(entity);
-        entities[entt] = traits_type::to_value(traits_type::to_entity(free_list), version + (traits_type::to_value(null, version) == tombstone));
+        entities[entt] = traits_type::construct(traits_type::to_entity(free_list), version + (version == traits_type::to_version(tombstone)));
         free_list = (tombstone | entity);
         return traits_type::to_version(entities[entt]);
     }
@@ -163,7 +162,7 @@ public:
      * @return The entity identifier without the version.
      */
     [[nodiscard]] static entity_type entity(const entity_type entity) ENTT_NOEXCEPT {
-        return traits_type::to_value(traits_type::to_entity(entity), {});
+        return traits_type::construct(traits_type::to_entity(entity), {});
     }
 
     /**
@@ -418,12 +417,12 @@ public:
             }
 
             return (entities[req] = hint);
-        } else if(req == traits_type::to_entity(entities[req])) {
+        } else if(const auto curr = traits_type::to_entity(entities[req]); req == curr) {
             return create();
         } else {
             auto *it = &free_list;
             for(; traits_type::to_entity(*it) != req; it = &entities[traits_type::to_entity(*it)]);
-            *it = traits_type::to_value(entities[req], *it);
+            *it = traits_type::construct(curr, traits_type::to_version(*it));
             return (entities[req] = hint);
         }
     }

src/entt/entity/sparse_set.hpp

@@ -229,10 +229,10 @@ class basic_sparse_set {
         about_to_pop(entt, ud);
 
         auto &ref = sparse[page(entt)][offset(entt)];
-        const auto pos = size_type{traits_type::to_integral(ref)};
+        const auto pos = size_type{traits_type::to_entity(ref)};
         ENTT_ASSERT(packed[pos] == entt, "Invalid entity identifier");
 
-        packed[pos] = std::exchange(free_list, (tombstone | entity_type{static_cast<typename traits_type::entity_type>(pos)}));
+        packed[pos] = std::exchange(free_list, traits_type::construct(static_cast<typename traits_type::entity_type>(pos)));
         ref = null;
 
         // strong exception guarantee
@@ -244,7 +244,7 @@ class basic_sparse_set {
         about_to_pop(entt, ud);
 
         auto &ref = sparse[page(entt)][offset(entt)];
-        const auto pos = size_type{traits_type::to_integral(ref)};
+        const auto pos = size_type{traits_type::to_entity(ref)};
         ENTT_ASSERT(packed[pos] == entt, "Invalid entity identifier");
 
         auto &last = packed[count - 1u];
@@ -263,7 +263,7 @@ class basic_sparse_set {
             swap_and_pop(pos);
         } ENTT_CATCH {
             last = std::exchange(packed[pos], entt);
-            ref = std::exchange(sparse[page(last)][offset(last)], entity_type{static_cast<typename traits_type::entity_type>(count++)});
+            ref = std::exchange(sparse[page(last)][offset(last)], traits_type::construct(static_cast<typename traits_type::entity_type>(count++)));
             ENTT_THROW;
         }
     }
@@ -553,7 +553,7 @@ public:
      */
     [[nodiscard]] size_type index(const entity_type entt) const ENTT_NOEXCEPT {
         ENTT_ASSERT(contains(entt), "Set does not contain entity");
-        return size_type{traits_type::to_integral(sparse[page(entt)][offset(entt)])};
+        return size_type{traits_type::to_entity(sparse[page(entt)][offset(entt)])};
     }
 
     /**
@@ -593,12 +593,12 @@ public:
                 resize_packed(sz + !(sz > reserved));
             }
 
-            assure_page(page(entt))[offset(entt)] = entity_type{static_cast<typename traits_type::entity_type>(count)};
+            assure_page(page(entt))[offset(entt)] = traits_type::construct(static_cast<typename traits_type::entity_type>(count));
             packed[count++] = entt;
         } else {
             const auto pos = size_type{traits_type::to_entity(free_list)};
             move_and_pop(count, pos);
-            sparse[page(entt)][offset(entt)] = entity_type{static_cast<typename traits_type::entity_type>(pos)};
+            sparse[page(entt)][offset(entt)] = traits_type::construct(static_cast<typename traits_type::entity_type>(pos));
             free_list = std::exchange(packed[pos], entt);
         }
     }
@@ -621,7 +621,7 @@ public:
         for(; first != last; ++first) {
             const auto entt = *first;
             ENTT_ASSERT(!contains(entt), "Set already contains entity");
-            assure_page(page(entt))[offset(entt)] = entity_type{static_cast<typename traits_type::entity_type>(count)};
+            assure_page(page(entt))[offset(entt)] = traits_type::construct(static_cast<typename traits_type::entity_type>(count));
             packed[count++] = entt;
         }
     }
@@ -702,8 +702,8 @@ public:
                 --next;
                 move_and_pop(next, pos);
                 std::swap(packed[next], packed[pos]);
-                sparse[page(packed[pos])][offset(packed[pos])] = entity_type{static_cast<const typename traits_type::entity_type>(pos)};
-                *it = (tombstone | entity_type{static_cast<typename traits_type::entity_type>(next)});
+                sparse[page(packed[pos])][offset(packed[pos])] = traits_type::construct(static_cast<const typename traits_type::entity_type>(pos));
+                *it = traits_type::construct(static_cast<typename traits_type::entity_type>(next));
                 for(; next && packed[next - 1u] == tombstone; --next);
             }
         }
@@ -782,7 +782,7 @@ public:
                 const auto entt = packed[curr];
 
                 swap_at(next, idx);
-                sparse[page(entt)][offset(entt)] = entity_type{static_cast<typename traits_type::entity_type>(curr)};
+                sparse[page(entt)][offset(entt)] = traits_type::construct(static_cast<typename traits_type::entity_type>(curr));
                 curr = std::exchange(next, idx);
             }
         }

test/entt/entity/entity.cpp

@@ -21,16 +21,16 @@ TEST(Entity, Traits) {
     ASSERT_EQ(traits_type::to_entity(other), 1u);
     ASSERT_EQ(traits_type::to_version(other), 0u);
 
-    ASSERT_EQ(traits_type::to_value(traits_type::to_entity(entity), traits_type::to_version(entity)), entity);
-    ASSERT_EQ(traits_type::to_value(traits_type::to_entity(other), traits_type::to_version(other)), other);
-    ASSERT_NE(traits_type::to_value(traits_type::to_integral(entity), {}), entity);
+    ASSERT_EQ(traits_type::construct(traits_type::to_entity(entity), traits_type::to_version(entity)), entity);
+    ASSERT_EQ(traits_type::construct(traits_type::to_entity(other), traits_type::to_version(other)), other);
+    ASSERT_NE(traits_type::construct(traits_type::to_entity(entity), {}), entity);
 
-    ASSERT_EQ(traits_type::to_value(entity, entity), entity);
-    ASSERT_EQ(traits_type::to_value(other, other), other);
-    ASSERT_NE(traits_type::to_value(entity, {}), entity);
+    ASSERT_EQ(traits_type::construct(), entt::tombstone | static_cast<entt::entity>(entt::null));
+    ASSERT_EQ(traits_type::construct(), entt::null | static_cast<entt::entity>(entt::tombstone));
 
-    ASSERT_EQ(traits_type::reserved(), entt::tombstone | static_cast<entt::entity>(entt::null));
-    ASSERT_EQ(traits_type::reserved(), entt::null | static_cast<entt::entity>(entt::tombstone));
+    ASSERT_EQ(traits_type::construct(), static_cast<entt::entity>(entt::null));
+    ASSERT_EQ(traits_type::construct(), static_cast<entt::entity>(entt::tombstone));
+    ASSERT_EQ(traits_type::construct(), entt::entity{0xFFFFFFFF});
 }
 
 TEST(Entity, Null) {
@@ -39,7 +39,7 @@ TEST(Entity, Null) {
     constexpr entt::entity null = entt::null;
 
     ASSERT_FALSE(entt::entity{} == entt::null);
-    ASSERT_TRUE(entt::entity{traits_type::reserved()} == entt::null);
+    ASSERT_TRUE(entt::entity{traits_type::construct()} == entt::null);
 
     ASSERT_TRUE(entt::null == entt::null);
     ASSERT_FALSE(entt::null != entt::null);
@@ -47,7 +47,7 @@ TEST(Entity, Null) {
     entt::registry registry{};
     const auto entity = registry.create();
 
-    ASSERT_EQ((entt::null | entity), (traits_type::to_value(entt::null, entity)));
+    ASSERT_EQ((entt::null | entity), (traits_type::construct(traits_type::to_entity(null), traits_type::to_version(entity))));
     ASSERT_EQ((entt::null | null), null);
     ASSERT_EQ((entt::null | tombstone), null);
 
@@ -71,7 +71,7 @@ TEST(Entity, Tombstone) {
     constexpr entt::entity null = entt::null;
 
     ASSERT_FALSE(entt::entity{} == entt::tombstone);
-    ASSERT_TRUE(entt::entity{traits_type::reserved()} == entt::tombstone);
+    ASSERT_TRUE(entt::entity{traits_type::construct()} == entt::tombstone);
 
     ASSERT_TRUE(entt::tombstone == entt::tombstone);
     ASSERT_FALSE(entt::tombstone != entt::tombstone);
@@ -79,7 +79,7 @@ TEST(Entity, Tombstone) {
     entt::registry registry{};
     const auto entity = registry.create();
 
-    ASSERT_EQ((entt::tombstone | entity), (traits_type::to_value(entity, entt::tombstone)));
+    ASSERT_EQ((entt::tombstone | entity), (traits_type::construct(traits_type::to_entity(entity), traits_type::to_version(tombstone))));
     ASSERT_EQ((entt::tombstone | tombstone), tombstone);
     ASSERT_EQ((entt::tombstone | null), tombstone);
 
@@ -92,7 +92,7 @@ TEST(Entity, Tombstone) {
     ASSERT_TRUE(entt::tombstone != entity);
 
     const auto vers = traits_type::to_version(entt::tombstone);
-    const auto other = traits_type::to_value(traits_type::to_entity(entity), vers);
+    const auto other = traits_type::construct(traits_type::to_entity(entity), vers);
 
     ASSERT_FALSE(registry.valid(entt::tombstone));
     ASSERT_NE(registry.destroy(entity, vers), vers);

test/entt/entity/registry.cpp

@@ -629,7 +629,7 @@ TEST(Registry, VersionOverflow) {
     ASSERT_NE(registry.current(entity), registry.version(entity));
     ASSERT_NE(registry.current(entity), typename traits_type::version_type{});
 
-    registry.destroy(registry.create(), traits_type::to_version(traits_type::reserved()) - 1u);
+    registry.destroy(registry.create(), traits_type::to_version(traits_type::construct()) - 1u);
     registry.destroy(registry.create());
 
     ASSERT_EQ(registry.current(entity), registry.version(entity));
@@ -654,7 +654,7 @@ TEST(Registry, TombstoneVersion) {
 
     const auto other = registry.create();
     const auto vers = traits_type::to_version(entity);
-    const auto required = traits_type::to_value(traits_type::to_entity(other), vers);
+    const auto required = traits_type::construct(traits_type::to_entity(other), vers);
 
     ASSERT_NE(registry.destroy(other, vers), vers);
     ASSERT_NE(registry.create(required), required);

test/entt/entity/sparse_set.cpp

@@ -110,11 +110,8 @@ TEST(SparseSet, Contains) {
     ASSERT_FALSE(set.contains(entt::tombstone));
 
     // tombstones and null entities can trigger false positives
-    ASSERT_TRUE(set.contains(traits_type::to_value(entt::entity{1}, entt::null)));
-    ASSERT_TRUE(set.contains(traits_type::to_value(entt::entity{1}, entt::tombstone)));
-
-    ASSERT_FALSE(set.contains(traits_type::to_value(entt::null, entt::entity{1})));
-    ASSERT_FALSE(set.contains(traits_type::to_value(entt::tombstone, entt::entity{1})));
+    ASSERT_TRUE(set.contains(entt::tombstone | entt::entity{1u}));
+    ASSERT_FALSE(set.contains(entt::null | entt::entity{1u}));
 }
 
 TEST(SparseSet, Move) {