Browse Source

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

Michele Caini 6 years ago
parent
commit
5e6cda7c5e
3 changed files with 31 additions and 4 deletions
  1. 1 0
      TODO
  2. 8 4
      src/entt/entity/registry.hpp
  3. 22 0
      test/entt/entity/group.cpp

+ 1 - 0
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

+ 8 - 4
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,7 +166,8 @@ 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), ...);
@@ -172,7 +175,8 @@ class basic_registry {
             } 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), ...);

+ 22 - 0
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);
+}