Browse Source

entt_traits: review to reduce the risk of ambiguous calls

Michele Caini 4 years ago
parent
commit
462bfea733

+ 13 - 11
src/entt/entity/entity.hpp

@@ -78,6 +78,8 @@ class entt_traits: private internal::entt_traits<Type> {
     using traits_type = internal::entt_traits<Type>;
 
 public:
+    /*! @brief Value type. */
+    using value_type = Type;
     /*! @brief Underlying entity type. */
     using entity_type = typename traits_type::entity_type;
     /*! @brief Underlying version type. */
@@ -90,7 +92,7 @@ public:
      * @param value The value to convert.
      * @return The integral representation of the given value.
      */
-    [[nodiscard]] static constexpr auto to_integral(const Type value) ENTT_NOEXCEPT {
+    [[nodiscard]] static constexpr entity_type to_integral(const value_type value) ENTT_NOEXCEPT {
         return static_cast<entity_type>(value);
     }
 
@@ -99,7 +101,7 @@ public:
      * @param value The value to convert.
      * @return The integral representation of the entity part.
      */
-    [[nodiscard]] static constexpr auto to_entity(const Type value) ENTT_NOEXCEPT {
+    [[nodiscard]] static constexpr entity_type to_entity(const value_type value) ENTT_NOEXCEPT {
         return (to_integral(value) & traits_type::entity_mask);
     }
 
@@ -108,7 +110,7 @@ public:
      * @param value The value to convert.
      * @return The integral representation of the version part.
      */
-    [[nodiscard]] static constexpr auto to_version(const Type value) ENTT_NOEXCEPT {
+    [[nodiscard]] static constexpr version_type to_version(const value_type value) ENTT_NOEXCEPT {
         constexpr auto mask = (traits_type::version_mask << traits_type::entity_shift);
         return ((to_integral(value) & mask) >> traits_type::entity_shift);
     }
@@ -119,8 +121,8 @@ public:
      * @param version The version part of the identifier.
      * @return A properly constructed identifier.
      */
-    [[nodiscard]] static constexpr auto to_type(const entity_type entity, const version_type version) ENTT_NOEXCEPT {
-        return Type{(entity & traits_type::entity_mask) | (version << traits_type::entity_shift)};
+    [[nodiscard]] static constexpr value_type to_value(const entity_type entity, const version_type version) ENTT_NOEXCEPT {
+        return value_type{(entity & traits_type::entity_mask) | (version << traits_type::entity_shift)};
     }
 
     /**
@@ -129,17 +131,17 @@ public:
      * @param version The version part of the identifier.
      * @return A properly constructed identifier.
      */
-    [[nodiscard]] static constexpr auto to_type(const Type entity, const Type version) ENTT_NOEXCEPT {
+    [[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 Type{(to_integral(entity) & traits_type::entity_mask) | (to_integral(version) & mask)};
+        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 auto reserved() ENTT_NOEXCEPT {
-        return to_type(traits_type::entity_mask, traits_type::version_mask);
+    [[nodiscard]] static constexpr value_type reserved() ENTT_NOEXCEPT {
+        return value_type{traits_type::entity_mask | (traits_type::version_mask << traits_type::entity_shift)};
     }
 };
 
@@ -216,7 +218,7 @@ struct null_t {
      */
     template<typename Entity>
     [[nodiscard]] constexpr Entity operator|(const Entity entity) const ENTT_NOEXCEPT {
-        return entt_traits<Entity>::to_type(*this, entity);
+        return entt_traits<Entity>::to_value(static_cast<Entity>(*this), entity);
     }
 };
 
@@ -307,7 +309,7 @@ struct tombstone_t {
      */
     template<typename Entity>
     [[nodiscard]] constexpr Entity operator|(const Entity entity) const ENTT_NOEXCEPT {
-        return entt_traits<Entity>::to_type(entity, *this);
+        return entt_traits<Entity>::to_value(entity, static_cast<Entity>(*this));
     }
 };
 

+ 9 - 8
src/entt/entity/registry.hpp

@@ -129,20 +129,21 @@ 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_type(static_cast<typename traits_type::entity_type>(pos), {});
+        return traits_type::to_value(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);
-        free_list = traits_type::to_type(entities[curr], tombstone);
-        return (entities[curr] = traits_type::to_type(curr, traits_type::to_version(entities[curr])));
+        const auto entt = traits_type::to_value(free_list, entities[curr]);
+        free_list = (tombstone | entities[curr]);
+        return (entities[curr] = entt);
     }
 
     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_type(traits_type::to_integral(free_list), version + (traits_type::to_type(null, version) == tombstone));
-        free_list = traits_type::to_type(entt, traits_type::to_version(tombstone));
+        entities[entt] = traits_type::to_value(traits_type::to_entity(free_list), version + (traits_type::to_value(null, version) == tombstone));
+        free_list = (tombstone | entity);
         return traits_type::to_version(entities[entt]);
     }
 
@@ -162,7 +163,7 @@ public:
      * @return The entity identifier without the version.
      */
     [[nodiscard]] static entity_type entity(const entity_type entity) ENTT_NOEXCEPT {
-        return traits_type::to_type(traits_type::to_entity(entity), {});
+        return traits_type::to_value(traits_type::to_entity(entity), {});
     }
 
     /**
@@ -417,12 +418,12 @@ public:
             }
 
             return (entities[req] = hint);
-        } else if(const auto curr = traits_type::to_entity(entities[req]); req == curr) {
+        } else if(req == traits_type::to_entity(entities[req])) {
             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_type(curr, traits_type::to_version(*it));
+            *it = traits_type::to_value(entities[req], *it);
             return (entities[req] = hint);
         }
     }

+ 9 - 9
test/entt/entity/entity.cpp

@@ -21,13 +21,13 @@ TEST(Entity, Traits) {
     ASSERT_EQ(traits_type::to_entity(other), 1u);
     ASSERT_EQ(traits_type::to_version(other), 0u);
 
-    ASSERT_EQ(traits_type::to_type(traits_type::to_entity(entity), traits_type::to_version(entity)), entity);
-    ASSERT_EQ(traits_type::to_type(traits_type::to_entity(other), traits_type::to_version(other)), other);
-    ASSERT_NE(traits_type::to_type(traits_type::to_integral(entity), {}), entity);
+    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::to_type(entity, entity), entity);
-    ASSERT_EQ(traits_type::to_type(other, other), other);
-    ASSERT_NE(traits_type::to_type(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::reserved(), entt::tombstone | static_cast<entt::entity>(entt::null));
     ASSERT_EQ(traits_type::reserved(), entt::null | static_cast<entt::entity>(entt::tombstone));
@@ -47,7 +47,7 @@ TEST(Entity, Null) {
     entt::registry registry{};
     const auto entity = registry.create();
 
-    ASSERT_EQ((entt::null | entity), (traits_type::to_type(entt::null, entity)));
+    ASSERT_EQ((entt::null | entity), (traits_type::to_value(entt::null, entity)));
     ASSERT_EQ((entt::null | null), null);
     ASSERT_EQ((entt::null | tombstone), null);
 
@@ -79,7 +79,7 @@ TEST(Entity, Tombstone) {
     entt::registry registry{};
     const auto entity = registry.create();
 
-    ASSERT_EQ((entt::tombstone | entity), (traits_type::to_type(entity, entt::tombstone)));
+    ASSERT_EQ((entt::tombstone | entity), (traits_type::to_value(entity, entt::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_type(traits_type::to_entity(entity), vers);
+    const auto other = traits_type::to_value(traits_type::to_entity(entity), vers);
 
     ASSERT_FALSE(registry.valid(entt::tombstone));
     ASSERT_NE(registry.destroy(entity, vers), vers);

+ 1 - 1
test/entt/entity/registry.cpp

@@ -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_type(traits_type::to_entity(other), vers);
+    const auto required = traits_type::to_value(traits_type::to_entity(other), vers);
 
     ASSERT_NE(registry.destroy(other, vers), vers);
     ASSERT_NE(registry.create(required), required);

+ 4 - 4
test/entt/entity/sparse_set.cpp

@@ -110,11 +110,11 @@ TEST(SparseSet, Contains) {
     ASSERT_FALSE(set.contains(entt::tombstone));
 
     // tombstones and null entities can trigger false positives
-    ASSERT_TRUE(set.contains(traits_type::to_type(entt::entity{1}, entt::null)));
-    ASSERT_TRUE(set.contains(traits_type::to_type(entt::entity{1}, entt::tombstone)));
+    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_type(entt::null, entt::entity{1})));
-    ASSERT_FALSE(set.contains(traits_type::to_type(entt::tombstone, entt::entity{1})));
+    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})));
 }
 
 TEST(SparseSet, Move) {