registry: fixed signal race on groups (close #320)

skypjack · Sep 19, 2019 · 5e6cda7 · 5e6cda7
1 parent 9c71b53
commit 5e6cda7
Show file tree

Hide file tree

Showing 3 changed files with 31 additions and 4 deletions.
diff --git a/TODO b/TODO
@@ -34,3 +34,4 @@ TODO
 * range based registry::remove and some others?
 * nested groups: AB/ABC/ABCD/... (hints: sort, check functions)
   - sink::before and ordered calls
+* ::size_type - c'mon
diff --git a/src/entt/entity/registry.hpp b/src/entt/entity/registry.hpp
@@ -134,13 +134,15 @@ class basic_registry {
         void maybe_valid_if(const Entity entt) {
             if constexpr(std::disjunction_v<std::is_same<Get, Component>...>) {
                 if(((std::is_same_v<Component, Get> || std::get<pool_type<Get> *>(cpools)->has(entt)) && ...)
-                        && (!std::get<pool_type<Exclude> *>(cpools)->has(entt) && ...))
+                        && (!std::get<pool_type<Exclude> *>(cpools)->has(entt) && ...)
+                        && !set.has(entt))
                 {
                     set.construct(entt);
                 }
             } else if constexpr(std::disjunction_v<std::is_same<Exclude, Component>...>) {
                 if((std::get<pool_type<Get> *>(cpools)->has(entt) && ...)
-                        && ((std::is_same_v<Exclude, Component> || !std::get<pool_type<Exclude> *>(cpools)->has(entt)) && ...))
+                        && ((std::is_same_v<Exclude, Component> || !std::get<pool_type<Exclude> *>(cpools)->has(entt)) && ...)
+                        && !set.has(entt))
                 {
                     set.construct(entt);
                 }
@@ -164,15 +166,17 @@ class basic_registry {
             if constexpr(std::disjunction_v<std::is_same<Owned, Component>..., std::is_same<Get, Component>...>) {
                 if(((std::is_same_v<Component, Owned> || std::get<pool_type<Owned> *>(cpools)->has(entt)) && ...)
                         && ((std::is_same_v<Component, Get> || std::get<pool_type<Get> *>(cpools)->has(entt)) && ...)
-                        && (!std::get<pool_type<Exclude> *>(cpools)->has(entt) && ...))
+                        && (!std::get<pool_type<Exclude> *>(cpools)->has(entt) && ...)
+                        && !(std::get<0>(cpools)->index(entt) < owned))
                 {
                     const auto pos = owned++;
                     (std::get<pool_type<Owned> *>(cpools)->swap(std::get<pool_type<Owned> *>(cpools)->data()[pos], entt), ...);
                 }
             } else if constexpr(std::disjunction_v<std::is_same<Exclude, Component>...>) {
                 if((std::get<pool_type<Owned> *>(cpools)->has(entt) && ...)
                         && (std::get<pool_type<Get> *>(cpools)->has(entt) && ...)
-                        && ((std::is_same_v<Exclude, Component> || !std::get<pool_type<Exclude> *>(cpools)->has(entt)) && ...))
+                        && ((std::is_same_v<Exclude, Component> || !std::get<pool_type<Exclude> *>(cpools)->has(entt)) && ...)
+                        && !(std::get<0>(cpools)->index(entt) < owned))
                 {
                     const auto pos = owned++;
                     (std::get<pool_type<Owned> *>(cpools)->swap(std::get<pool_type<Owned> *>(cpools)->data()[pos], entt), ...);

diff --git a/test/entt/entity/group.cpp b/test/entt/entity/group.cpp
@@ -463,6 +463,17 @@ TEST(NonOwningGroup, Less) {
     });
 }
 
+TEST(NonOwningGroup, SignalRace) {
+    entt::registry registry;
+    registry.on_construct<double>().connect<&entt::registry::assign_or_replace<int>>(registry);
+    registry.group(entt::get<int, double>);
+
+    auto entity = registry.create();
+    registry.assign<double>(entity);
+
+    ASSERT_EQ(registry.group(entt::get<int, double>).size(), 1u);
+}
+
 TEST(OwningGroup, Functionalities) {
     entt::registry registry;
     auto group = registry.group<int>(entt::get<char>);
@@ -983,3 +994,14 @@ TEST(OwningGroup, Less) {
         ASSERT_EQ(entity, entt);
     });
 }
+
+TEST(OwningGroup, SignalRace) {
+    entt::registry registry;
+    registry.on_construct<double>().connect<&entt::registry::assign_or_replace<int>>(registry);
+    registry.group<int>(entt::get<double>);
+
+    auto entity = registry.create();
+    registry.assign<double>(entity);
+
+    ASSERT_EQ(registry.group<int>(entt::get<double>).size(), 1u);
+}