SlotMap update

Bigfoot/Sources/Utils/Include/Utils/Containers/SlotMap.hpp

@@ -11,160 +11,147 @@
 #include <EASTL/vector.h>
 
 #include <cstdint>
+#include <limits>
 
 namespace Bigfoot
 {
-template<class TYPE>
+template<class TYPE, class VERSION_TYPE = std::uint32_t>
 class SlotMap
 {
-  private:
-    using IndexType = std::uint32_t;
-    using VersionType = std::uint32_t;
-
   public:
-    struct SlotKey
+    class SlotKey
     {
-        VersionType m_version = 0;
-        IndexType m_index = 0;
+      public:
+        using IndexType = std::uint32_t;
+        using VersionType = VERSION_TYPE;
 
-        bool Invalid() const
+        static constexpr VersionType MAX_VERSION = std::numeric_limits<VersionType>::max();
+        static constexpr VersionType INVALID_VERSION = 0;
+
+        static constexpr IndexType MAX_INDEX = std::numeric_limits<IndexType>::max();
+
+        SlotKey(const VersionType p_version, const IndexType p_index)
         {
-            return m_version == 0;
+            m_key = (static_cast<std::uint64_t>(p_version) << 32) | p_index;
         }
 
-        bool operator==(const SlotKey p_key) const
+        SlotKey():
+            SlotKey(INVALID_VERSION, 0)
         {
-            return m_version == p_key.m_version && m_index == p_key.m_index;
         }
+
+        SlotKey(const SlotKey& p_slotKey) = default;
+        SlotKey(SlotKey&& p_slotKey) = default;
+
+        ~SlotKey() = default;
+
+        bool IsValid() const
+        {
+            return GetVersion() != INVALID_VERSION;
+        }
+
+        VersionType GetVersion() const
+        {
+            return static_cast<VersionType>(m_key >> 32);
+        }
+
+        IndexType GetIndex() const
+        {
+            constexpr std::uint64_t indexMask = 0x00000000FFFFFFFF;
+            return static_cast<IndexType>(m_key & indexMask);
+        }
+
+        SlotKey& operator=(const SlotKey& p_slotKey) = default;
+        SlotKey& operator=(SlotKey&& p_slotKey) = default;
+
+      private:
+        std::uint64_t m_key;
     };
 
     SlotMap():
-        m_freeSlotHead(std::numeric_limits<IndexType>::max())
+        m_freeSlotHead(std::numeric_limits<typename SlotKey::IndexType>::max())
     {
     }
 
-    SlotMap(const SlotMap& p_slotMap) = default;
-    SlotMap(SlotMap&& p_slotMap) = default;
+    SlotMap(const SlotMap& p_map) = default;
+    SlotMap(SlotMap&& p_map) = default;
+
+    ~SlotMap() = default;
 
     template<class... ARGS>
+    [[nodiscard]]
     SlotKey Insert(ARGS&&... p_args)
     {
-        ASSERT(UtilsAssertHandler,
-               m_data.size() < std::numeric_limits<IndexType>::max(),
-               "Too many elements for SlotMap!");
+        ASSERT(UtilsAssertHandler, m_data.size() < SlotKey::MAX_INDEX, "Too many elements for SlotMap!");
 
+        const typename SlotKey::IndexType dataIndex = static_cast<typename SlotKey::IndexType>(m_data.size());
         m_data.emplace_back(std::forward<ARGS>(p_args)...);
 
-        if (m_freeSlotHead != std::numeric_limits<IndexType>::max())
+        if (HasFreeSlots())
         {
-            const IndexType freeSlotIndex = m_freeSlotHead;
-            m_freeSlotHead = m_slots[freeSlotIndex].m_index;
+            const typename SlotKey::IndexType slotIndex = m_freeSlotHead;
 
-            const VersionType version = m_slots[freeSlotIndex].m_version;
-            m_slots[freeSlotIndex] = {.m_version = version, .m_index = static_cast<IndexType>(m_data.size()) - 1};
-            m_dataToSlots.push_back(freeSlotIndex);
+            m_freeSlotHead = m_slots[slotIndex].GetIndex();
 
-            return {.m_version = version, .m_index = freeSlotIndex};
+            m_slots[slotIndex] = SlotKey {m_slots[slotIndex].GetVersion(), dataIndex};
+            m_dataToSlots.push_back(slotIndex);
+
+            return SlotKey {m_slots[slotIndex].GetVersion(), slotIndex};
         }
 
-        const IndexType newSlotIndex = m_slots.size();
-        m_slots.push_back({.m_version = 1, .m_index = static_cast<IndexType>(m_data.size()) - 1});
-        m_dataToSlots.push_back(newSlotIndex);
+        const typename SlotKey::IndexType slotIndex = static_cast<typename SlotKey::IndexType>(m_slots.size());
+        m_slots.emplace_back(1, dataIndex);
+        m_dataToSlots.push_back(slotIndex);
 
-        return {.m_version = 1, .m_index = newSlotIndex};
+        return SlotKey {1, slotIndex};
     }
 
-    void Remove(const SlotKey p_key)
+    void Remove(const SlotKey p_slotKey)
     {
-        const IndexType slotIndex = p_key.m_index;
-        if (slotIndex >= m_slots.size())
-        {
-            return;
-        }
-        if (p_key.m_version != m_slots[slotIndex].m_version)
+        if (!Has(p_slotKey))
         {
             return;
         }
 
-        const IndexType dataIndex = m_slots[slotIndex].m_index;
+        const typename SlotKey::IndexType dataIndex = m_slots[p_slotKey.GetIndex()].GetIndex();
+
         m_data.erase_unsorted(m_data.begin() + dataIndex);
         m_dataToSlots.erase_unsorted(m_dataToSlots.begin() + dataIndex);
 
         if (dataIndex < m_data.size())
         {
-            const IndexType movedSlotIndex = m_dataToSlots[dataIndex];
-            m_slots[movedSlotIndex].m_index = dataIndex;
+            m_slots[m_dataToSlots[dataIndex]] = SlotKey {m_slots[m_dataToSlots[dataIndex]].GetVersion(), dataIndex};
         }
 
-        m_slots[slotIndex] = {.m_version = p_key.m_version + 1, .m_index = m_freeSlotHead};
-        m_freeSlotHead = slotIndex;
-    }
+        const typename SlotKey::VersionType nextVersion = p_slotKey.GetVersion() + 1;
 
-    TYPE* Get(const SlotKey p_key)
-    {
-        const IndexType slotIndex = p_key.m_index;
-        if (slotIndex >= m_slots.size())
+        if (nextVersion == SlotKey::INVALID_VERSION)
         {
-            return nullptr;
-        }
-        if (p_key.m_version != m_slots[slotIndex].m_version)
-        {
-            return nullptr;
-        }
-        return &m_data[m_slots[slotIndex].m_index];
-    }
-
-    const TYPE* Get(const SlotKey p_key) const
-    {
-        const IndexType slotIndex = p_key.m_index;
-        if (slotIndex >= m_slots.size())
-        {
-            return nullptr;
-        }
-        if (p_key.m_version != m_slots[slotIndex].m_version)
-        {
-            return nullptr;
-        }
-        return &m_data[m_slots[slotIndex].m_index];
-    }
-
-    void Reserve(const std::uint32_t p_size)
-    {
-        m_data.reserve(p_size);
-        m_slots.reserve(p_size);
-        m_dataToSlots.reserve(p_size);
-    }
-
-    typename eastl::vector<TYPE>::size_type Size() const
-    {
-        return m_data.size();
-    }
-
-    typename eastl::vector<TYPE>::size_type Capacity() const
-    {
-        return m_data.capacity();
-    }
-
-    bool Empty() const
-    {
-        return m_data.empty();
-    }
-
-    void Clear()
-    {
-        m_data.clear();
-        m_dataToSlots.clear();
-
-        for (IndexType i = 0; i < m_slots.size(); ++i)
-        {
-            const VersionType newVersion = m_slots[i].m_version + 1;
-            const IndexType nextFree = i + 1 < static_cast<IndexType>(m_slots.size())
-                                           ? i + 1
-                                           : std::numeric_limits<IndexType>::max();
-            m_slots[i] = {.m_version = newVersion, .m_index = nextFree};
+            m_slots[p_slotKey.GetIndex()] = SlotKey {SlotKey::INVALID_VERSION, 0};
+            return;
         }
 
-        m_freeSlotHead = m_slots.empty() ? std::numeric_limits<IndexType>::max() : 0;
+        m_slots[p_slotKey.GetIndex()] = SlotKey {nextVersion, m_freeSlotHead};
+        m_freeSlotHead = p_slotKey.GetIndex();
+    }
+
+    [[nodiscard]]
+    bool Has(const SlotKey p_slotKey) const
+    {
+        return p_slotKey.GetIndex() < m_data.size() &&
+               p_slotKey.GetVersion() == m_slots[p_slotKey.GetIndex()].GetVersion();
+    }
+
+    [[nodiscard]]
+    TYPE* Get(const SlotKey p_slotKey)
+    {
+        return Has(p_slotKey) ? &m_data[m_slots[p_slotKey.GetIndex()].GetIndex()] : nullptr;
+    }
+
+    [[nodiscard]]
+    const TYPE* Get(const SlotKey p_slotKey) const
+    {
+        return Has(p_slotKey) ? &m_data[m_slots[p_slotKey.GetIndex()].GetIndex()] : nullptr;
     }
 
     void Reset()
@@ -172,50 +159,25 @@ class SlotMap
         m_data.clear();
         m_slots.clear();
         m_dataToSlots.clear();
-        m_freeSlotHead = std::numeric_limits<IndexType>::max();
+        m_freeSlotHead = std::numeric_limits<typename SlotKey::IndexType>::max();
     }
 
-    typename eastl::vector<TYPE>::iterator begin()
-    {
-        return m_data.begin();
-    }
-
-    typename eastl::vector<TYPE>::iterator end()
-    {
-        return m_data.end();
-    }
-
-    typename eastl::vector<TYPE>::const_iterator begin() const
-    {
-        return m_data.begin();
-    }
-
-    typename eastl::vector<TYPE>::const_iterator end() const
-    {
-        return m_data.end();
-    }
-
-    typename eastl::vector<TYPE>::const_iterator cbegin() const
-    {
-        return m_data.cbegin();
-    }
-
-    typename eastl::vector<TYPE>::const_iterator cend() const
-    {
-        return m_data.cend();
-    }
-
-    ~SlotMap() = default;
-
     SlotMap& operator=(const SlotMap& p_slotMap) = default;
     SlotMap& operator=(SlotMap&& p_slotMap) = default;
 
   private:
+    [[nodiscard]]
+    bool HasFreeSlots() const
+    {
+        return m_freeSlotHead != std::numeric_limits<typename SlotKey::IndexType>::max();
+    }
+
     eastl::vector<TYPE> m_data;
-    eastl::vector<IndexType> m_dataToSlots;
     eastl::vector<SlotKey> m_slots;
 
-    IndexType m_freeSlotHead;
+    eastl::vector<typename eastl::vector<TYPE>::size_type> m_dataToSlots;
+
+    typename SlotKey::IndexType m_freeSlotHead;
 };
 }   // namespace Bigfoot
 

Bigfoot/Tests/Utils/Containers/SlotMap.cpp

@@ -28,11 +28,47 @@ TEST_F(SlotMapFixture, Insert)
     const SlotMap<std::uint32_t>::SlotKey fourthKey = m_slotMap.Insert(3);
     const SlotMap<std::uint32_t>::SlotKey fifthKey = m_slotMap.Insert(65);
 
+    EXPECT_FALSE(m_slotMap.Has(firstKey));
+    EXPECT_FALSE(m_slotMap.Has(secondKey));
+    EXPECT_TRUE(m_slotMap.Has(thirdKey));
+    EXPECT_TRUE(m_slotMap.Has(fourthKey));
+    EXPECT_TRUE(m_slotMap.Has(fifthKey));
+    EXPECT_FALSE(m_slotMap.Has(SlotMap<std::uint32_t>::SlotKey {}));
+    EXPECT_FALSE(m_slotMap.Has(SlotMap<std::uint32_t>::SlotKey {1, 4}));
+
     EXPECT_EQ(m_slotMap.Get(firstKey), nullptr);
     EXPECT_EQ(m_slotMap.Get(secondKey), nullptr);
     EXPECT_EQ(*m_slotMap.Get(thirdKey), 42);
     EXPECT_EQ(*m_slotMap.Get(fourthKey), 3);
     EXPECT_EQ(*m_slotMap.Get(fifthKey), 65);
     EXPECT_EQ(m_slotMap.Get(SlotMap<std::uint32_t>::SlotKey {}), nullptr);
+    EXPECT_EQ(m_slotMap.Get(SlotMap<std::uint32_t>::SlotKey {1, 4}), nullptr);
+}
+
+TEST(SlotMap, VersionOverflowDeactivatesSlot)
+{
+    SlotMap<std::uint32_t, std::uint8_t> slotMap;
+
+    SlotMap<std::uint32_t, std::uint8_t>::SlotKey key = slotMap.Insert(1);
+
+    for (SlotMap<std::uint32_t, std::uint8_t>::SlotKey::VersionType i = 1;
+         i < SlotMap<std::uint32_t, std::uint8_t>::SlotKey::MAX_VERSION;
+         ++i)
+    {
+        slotMap.Remove(key);
+        const SlotMap<std::uint32_t, std::uint8_t>::SlotKey newKey = slotMap.Insert(1);
+        EXPECT_EQ(newKey.GetIndex(), key.GetIndex());
+        key = newKey;
+    }
+
+    // Slot is at MAX_VERSION — one more remove should overflow and permanently deactivate it
+    EXPECT_EQ(key.GetVersion(), (SlotMap<std::uint32_t, std::uint8_t>::SlotKey::MAX_VERSION));
+    slotMap.Remove(key);
+
+    EXPECT_FALSE(slotMap.Has(key));
+
+    // Dead slot must not be recycled; a new insert must allocate a fresh slot index
+    const SlotMap<std::uint32_t, std::uint8_t>::SlotKey newKey = slotMap.Insert(2);
+    EXPECT_NE(newKey.GetIndex(), key.GetIndex());
 }
 }   // namespace Bigfoot