From 462bfea733f3a2300bb108a4a5e9123aa87d87ea Mon Sep 17 00:00:00 2001 From: Michele Caini Date: Tue, 22 Jun 2021 13:03:32 +0200 Subject: [PATCH] entt_traits: review to reduce the risk of ambiguous calls --- src/entt/entity/entity.hpp | 24 +++++++++++++----------- src/entt/entity/registry.hpp | 17 +++++++++-------- test/entt/entity/entity.cpp | 18 +++++++++--------- test/entt/entity/registry.cpp | 2 +- test/entt/entity/sparse_set.cpp | 8 ++++---- 5 files changed, 36 insertions(+), 33 deletions(-) diff --git a/src/entt/entity/entity.hpp b/src/entt/entity/entity.hpp index 875beffd2..baa968cc3 100644 --- a/src/entt/entity/entity.hpp +++ b/src/entt/entity/entity.hpp @@ -78,6 +78,8 @@ class entt_traits: private internal::entt_traits { using traits_type = internal::entt_traits; 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(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 [[nodiscard]] constexpr Entity operator|(const Entity entity) const ENTT_NOEXCEPT { - return entt_traits::to_type(*this, entity); + return entt_traits::to_value(static_cast(*this), entity); } }; @@ -307,7 +309,7 @@ struct tombstone_t { */ template [[nodiscard]] constexpr Entity operator|(const Entity entity) const ENTT_NOEXCEPT { - return entt_traits::to_type(entity, *this); + return entt_traits::to_value(entity, static_cast(*this)); } }; diff --git a/src/entt/entity/registry.hpp b/src/entt/entity/registry.hpp index 0c716d874..10ba9f7a7 100644 --- a/src/entt/entity/registry.hpp +++ b/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(pos), {}); + return traits_type::to_value(static_cast(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); } } diff --git a/test/entt/entity/entity.cpp b/test/entt/entity/entity.cpp index 51a7c0a77..156ae1c6b 100644 --- a/test/entt/entity/entity.cpp +++ b/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::null)); ASSERT_EQ(traits_type::reserved(), entt::null | static_cast(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); diff --git a/test/entt/entity/registry.cpp b/test/entt/entity/registry.cpp index c795f3ed9..0753aea78 100644 --- a/test/entt/entity/registry.cpp +++ b/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); diff --git a/test/entt/entity/sparse_set.cpp b/test/entt/entity/sparse_set.cpp index ef89ee084..f6dc1cf78 100644 --- a/test/entt/entity/sparse_set.cpp +++ b/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) {