review: iterators + fixed bug on raw views

skypjack · Mar 30, 2018 · d417984 · d417984
1 parent d38b3e6
commit d417984
Show file tree

Hide file tree

Showing 4 changed files with 65 additions and 21 deletions.
diff --git a/src/entt/entity/sparse_set.hpp b/src/entt/entity/sparse_set.hpp
@@ -3,6 +3,7 @@
 
 
 #include <algorithm>
+#include <iterator>
 #include <numeric>
 #include <utility>
 #include <vector>
@@ -59,6 +60,9 @@ class SparseSet<Entity> {
     struct Iterator final {
         using difference_type = std::size_t;
         using value_type = Entity;
+        using pointer = value_type *;
+        using reference = value_type;
+        using iterator_category = std::input_iterator_tag;
 
         Iterator(const std::vector<value_type> &direct, std::size_t pos)
             : direct{direct}, pos{pos}
@@ -90,7 +94,7 @@ class SparseSet<Entity> {
             return !(*this == other);
         }
 
-        value_type operator*() const noexcept {
+        reference operator*() const noexcept {
             return direct[pos-1];
         }
 
@@ -435,8 +439,11 @@ class SparseSet<Entity, Type>: public SparseSet<Entity> {
     struct Iterator final {
         using difference_type = std::size_t;
         using value_type = Type;
+        using pointer = value_type *;
+        using reference = value_type &;
+        using iterator_category = std::input_iterator_tag;
 
-        Iterator(const std::vector<value_type> &instances, std::size_t pos)
+        Iterator(std::vector<value_type> &instances, std::size_t pos)
             : instances{instances}, pos{pos}
         {}
 
@@ -466,12 +473,16 @@ class SparseSet<Entity, Type>: public SparseSet<Entity> {
             return !(*this == other);
         }
 
-        value_type operator*() const noexcept {
+        reference operator*() noexcept {
             return instances[pos-1];
         }
 
+        pointer operator->() noexcept {
+            return &instances.data()[pos-1];
+        }
+
     private:
-        const std::vector<value_type> &instances;
+        std::vector<value_type> &instances;
         std::size_t pos;
     };
 
@@ -563,7 +574,7 @@ class SparseSet<Entity, Type>: public SparseSet<Entity> {
      *
      * @return An iterator to the first instance of the given type.
      */
-    iterator_type begin() const noexcept {
+    iterator_type begin() noexcept {
         return Iterator{instances, instances.size()};
     }
 
@@ -581,7 +592,7 @@ class SparseSet<Entity, Type>: public SparseSet<Entity> {
      * @return An iterator to the element following the last instance of the
      * given type.
      */
-    iterator_type end() const noexcept {
+    iterator_type end() noexcept {
         return Iterator{instances, 0};
     }
 

diff --git a/src/entt/entity/view.hpp b/src/entt/entity/view.hpp
@@ -397,7 +397,10 @@ class View final {
 
     public:
         using difference_type = typename underlying_iterator_type::difference_type;
-        using value_type = typename view_type::entity_type;
+        using value_type = typename underlying_iterator_type::value_type;
+        using pointer = typename underlying_iterator_type::pointer;
+        using reference = typename underlying_iterator_type::reference;
+        using iterator_category = typename underlying_iterator_type::iterator_category;
 
         Iterator(unchecked_type unchecked, size_type extent, underlying_iterator_type begin, underlying_iterator_type end) noexcept
             : unchecked{unchecked},
@@ -420,8 +423,7 @@ class View final {
         }
 
         Iterator & operator+=(difference_type value) noexcept {
-            begin += value;
-            return *this;
+            return ((begin += value) != end && !valid()) ? ++(*this) : *this;
         }
 
         Iterator operator+(difference_type value) noexcept {

diff --git a/test/entt/entity/sparse_set.cpp b/test/entt/entity/sparse_set.cpp
@@ -453,8 +453,8 @@ TEST(SparseSetWithType, RespectDisjoint) {
     ASSERT_EQ(*(clhs.raw() + 1u), 6);
     ASSERT_EQ(*(clhs.raw() + 2u), 9);
 
-    auto begin = clhs.begin();
-    auto end = clhs.end();
+    auto begin = lhs.begin();
+    auto end = lhs.end();
 
     ASSERT_EQ(*(begin++), 9);
     ASSERT_EQ(*(begin++), 6);
@@ -483,8 +483,8 @@ TEST(SparseSetWithType, RespectOverlap) {
     ASSERT_EQ(*(clhs.raw() + 1u), 9);
     ASSERT_EQ(*(clhs.raw() + 2u), 6);
 
-    auto begin = clhs.begin();
-    auto end = clhs.end();
+    auto begin = lhs.begin();
+    auto end = lhs.end();
 
     ASSERT_EQ(*(begin++), 6);
     ASSERT_EQ(*(begin++), 9);
@@ -629,3 +629,33 @@ TEST(SparseSetWithType, RespectUnordered) {
     ASSERT_EQ(*(rhs.data() + 4u), 4u);
     ASSERT_EQ(*(rhs.data() + 5u), 5u);
 }
+
+TEST(SparseSetWithType, ReferencesGuaranteed) {
+    struct Type { int value; };
+
+    entt::SparseSet<unsigned int, Type> set;
+
+    set.construct(0, 0);
+    set.construct(1, 1);
+
+    ASSERT_EQ(set.get(0).value, 0);
+    ASSERT_EQ(set.get(1).value, 1);
+
+    for(auto &&type: set) {
+        if(type.value) {
+            type.value = 42;
+        }
+    }
+
+    ASSERT_EQ(set.get(0).value, 0);
+    ASSERT_EQ(set.get(1).value, 42);
+
+    auto begin = set.begin();
+
+    while(begin != set.end()) {
+        (begin++)->value = 3;
+    }
+
+    ASSERT_EQ(set.get(0).value, 3);
+    ASSERT_EQ(set.get(1).value, 3);
+}
diff --git a/test/entt/entity/view.cpp b/test/entt/entity/view.cpp
@@ -344,14 +344,6 @@ TEST(PersistentView, Sort) {
     }
 }
 
-
-
-
-
-
-
-////////////////////////////////////////////////////////////////////////////////
-
 TEST(RawView, Functionalities) {
     entt::DefaultRegistry registry;
 
@@ -383,6 +375,15 @@ TEST(RawView, Functionalities) {
     ASSERT_EQ(*(view.raw() + 0), '2');
     ASSERT_EQ(*(static_cast<const decltype(view) &>(view).raw() + 1), '1');
 
+    for(auto &&component: view) {
+        // verifies that iterators return references to components
+        component = '0';
+    }
+
+    for(auto &&component: view) {
+        ASSERT_TRUE(component == '0');
+    }
+
     registry.remove<char>(e0);
     registry.remove<char>(e1);