Fix issue with invoking observers/hooks while deferring is suspended

SanderMertens · Sep 30, 2023 · 1771406 · 1771406
1 parent 50e0390
commit 1771406
Show file tree

Hide file tree

Showing 6 changed files with 182 additions and 2 deletions.
diff --git a/flecs.c b/flecs.c
@@ -4405,6 +4405,11 @@ void flecs_invoke_hook(
     ecs_entity_t event,
     ecs_iter_action_t hook)
 {
+    int32_t defer = world->stages[0].defer;
+    if (defer < 0) {
+        world->stages[0].defer *= -1;
+    }
+
     ecs_iter_t it = { .field_count = 1};
     it.entities = entities;
 
@@ -4424,6 +4429,8 @@ void flecs_invoke_hook(
     flecs_iter_validate(&it);
     hook(&it);
     ecs_iter_fini(&it);
+
+    world->stages[0].defer = defer;
 }
 
 void flecs_notify_on_set(
@@ -14904,6 +14911,12 @@ void flecs_emit(
     int32_t i, r, count = desc->count;
     ecs_flags32_t table_flags = table->flags;
 
+    /* Deferring cannot be suspended for observers */
+    int32_t defer = world->stages[0].defer;
+    if (defer < 0) {
+        world->stages[0].defer *= -1;
+    }
+
     /* Table events are emitted for internal table operations only, and do not
      * provide component data and/or entity ids. */
     bool table_event = desc->flags & EcsEventTableOnly;
@@ -15241,6 +15254,8 @@ void flecs_emit(
     }
 
 error:
+    world->stages[0].defer = defer;
+
     if (measure_time) {
         world->info.emit_time_total += (ecs_ftime_t)ecs_time_measure(&t);
     }

diff --git a/src/entity.c b/src/entity.c
@@ -1033,6 +1033,11 @@ void flecs_invoke_hook(
     ecs_entity_t event,
     ecs_iter_action_t hook)
 {
+    int32_t defer = world->stages[0].defer;
+    if (defer < 0) {
+        world->stages[0].defer *= -1;
+    }
+
     ecs_iter_t it = { .field_count = 1};
     it.entities = entities;
 
@@ -1052,6 +1057,8 @@ void flecs_invoke_hook(
     flecs_iter_validate(&it);
     hook(&it);
     ecs_iter_fini(&it);
+
+    world->stages[0].defer = defer;
 }
 
 void flecs_notify_on_set(

diff --git a/src/observable.c b/src/observable.c
@@ -1000,6 +1000,12 @@ void flecs_emit(
     int32_t i, r, count = desc->count;
     ecs_flags32_t table_flags = table->flags;
 
+    /* Deferring cannot be suspended for observers */
+    int32_t defer = world->stages[0].defer;
+    if (defer < 0) {
+        world->stages[0].defer *= -1;
+    }
+
     /* Table events are emitted for internal table operations only, and do not
      * provide component data and/or entity ids. */
     bool table_event = desc->flags & EcsEventTableOnly;
@@ -1337,6 +1343,8 @@ void flecs_emit(
     }
 
 error:
+    world->stages[0].defer = defer;
+
     if (measure_time) {
         world->info.emit_time_total += (ecs_ftime_t)ecs_time_measure(&t);
     }

diff --git a/test/api/project.json b/test/api/project.json
@@ -2549,7 +2549,11 @@
                 "defer_2_sets_w_observer_same_component",
                 "defer_2_sets_w_observer_other_component",
                 "on_remove_after_deferred_clear_and_add",
-                "defer_delete_recycle_same_id"
+                "defer_delete_recycle_same_id",
+                "observer_while_defer_suspended",
+                "on_add_hook_while_defer_suspended",
+                "on_set_hook_while_defer_suspended",
+                "on_remove_hook_while_defer_suspended"
             ]
         }, {
             "id": "SingleThreadStaging",

diff --git a/test/api/src/DeferredActions.c b/test/api/src/DeferredActions.c
@@ -3446,3 +3446,129 @@ void DeferredActions_defer_delete_recycle_same_id(void) {
 
     ecs_fini(world);
 }
+
+static ECS_COMPONENT_DECLARE(Velocity);
+
+static
+void AddWhileSuspended(ecs_iter_t *it) {
+    for (int i = 0; i < it->count; i ++) {
+        ecs_add(it->world, it->entities[i], Velocity);
+    }
+}
+
+void DeferredActions_observer_while_defer_suspended(void) {
+    ecs_world_t *world = ecs_mini();
+
+    ECS_COMPONENT(world, Position);
+    ECS_COMPONENT_DEFINE(world, Velocity);
+
+    ecs_observer(world, {
+        .filter.terms = {
+            { .id = ecs_id(Position) }
+        },
+        .events = { EcsOnAdd },
+        .callback = AddWhileSuspended,
+    });
+
+    ecs_defer_begin(world);
+    ecs_defer_suspend(world);
+
+    ecs_entity_t e = ecs_new(world, Position);
+    test_assert(e != 0);
+    test_assert(ecs_has(world, e, Position));
+    test_assert(!ecs_has(world, e, Velocity));
+
+    ecs_defer_resume(world);
+    ecs_defer_end(world);
+
+    test_assert(ecs_has(world, e, Position));
+    test_assert(ecs_has(world, e, Velocity));
+
+    ecs_fini(world);
+}
+
+void DeferredActions_on_add_hook_while_defer_suspended(void) {
+    ecs_world_t *world = ecs_mini();
+
+    ECS_COMPONENT(world, Position);
+    ECS_COMPONENT_DEFINE(world, Velocity);
+
+    ecs_set_hooks(world, Position, {
+        .on_add = AddWhileSuspended
+    });
+
+    ecs_defer_begin(world);
+    ecs_defer_suspend(world);
+
+    ecs_entity_t e = ecs_new(world, Position);
+    test_assert(e != 0);
+    test_assert(ecs_has(world, e, Position));
+    test_assert(!ecs_has(world, e, Velocity));
+
+    ecs_defer_resume(world);
+    ecs_defer_end(world);
+
+    test_assert(ecs_has(world, e, Position));
+    test_assert(ecs_has(world, e, Velocity));
+
+    ecs_fini(world);
+}
+
+void DeferredActions_on_set_hook_while_defer_suspended(void) {
+    ecs_world_t *world = ecs_mini();
+
+    ECS_COMPONENT(world, Position);
+    ECS_COMPONENT_DEFINE(world, Velocity);
+
+    ecs_set_hooks(world, Position, {
+        .on_set = AddWhileSuspended
+    });
+
+    ecs_defer_begin(world);
+    ecs_defer_suspend(world);
+
+    ecs_entity_t e = ecs_set(world, 0, Position, {10, 20});
+    test_assert(e != 0);
+    test_assert(ecs_has(world, e, Position));
+    test_assert(!ecs_has(world, e, Velocity));
+
+    ecs_defer_resume(world);
+    ecs_defer_end(world);
+
+    test_assert(ecs_has(world, e, Position));
+    test_assert(ecs_has(world, e, Velocity));
+
+    ecs_fini(world);
+}
+
+void DeferredActions_on_remove_hook_while_defer_suspended(void) {
+    ecs_world_t *world = ecs_mini();
+
+    ECS_COMPONENT(world, Position);
+    ECS_COMPONENT_DEFINE(world, Velocity);
+
+    ecs_set_hooks(world, Position, {
+        .on_remove = AddWhileSuspended
+    });
+
+    ecs_entity_t e = ecs_new(world, Position);
+    test_assert(e != 0);
+    test_assert(ecs_has(world, e, Position));
+    test_assert(!ecs_has(world, e, Velocity));
+
+    ecs_defer_begin(world);
+    ecs_defer_suspend(world);
+
+    test_assert(ecs_has(world, e, Position));
+    ecs_remove(world, e, Position);
+    test_assert(!ecs_has(world, e, Position));
+    test_assert(!ecs_has(world, e, Velocity));
+
+    ecs_defer_resume(world);
+    ecs_defer_end(world);
+
+    test_assert(!ecs_has(world, e, Position));
+    test_assert(ecs_has(world, e, Velocity));
+
+    ecs_fini(world);
+}
diff --git a/test/api/src/main.c b/test/api/src/main.c
@@ -2459,6 +2459,10 @@ void DeferredActions_defer_2_sets_w_observer_same_component(void);
 void DeferredActions_defer_2_sets_w_observer_other_component(void);
 void DeferredActions_on_remove_after_deferred_clear_and_add(void);
 void DeferredActions_defer_delete_recycle_same_id(void);
+void DeferredActions_observer_while_defer_suspended(void);
+void DeferredActions_on_add_hook_while_defer_suspended(void);
+void DeferredActions_on_set_hook_while_defer_suspended(void);
+void DeferredActions_on_remove_hook_while_defer_suspended(void);
 
 // Testsuite 'SingleThreadStaging'
 void SingleThreadStaging_setup(void);
@@ -12164,6 +12168,22 @@ bake_test_case DeferredActions_testcases[] = {
     {
         "defer_delete_recycle_same_id",
         DeferredActions_defer_delete_recycle_same_id
+    },
+    {
+        "observer_while_defer_suspended",
+        DeferredActions_observer_while_defer_suspended
+    },
+    {
+        "on_add_hook_while_defer_suspended",
+        DeferredActions_on_add_hook_while_defer_suspended
+    },
+    {
+        "on_set_hook_while_defer_suspended",
+        DeferredActions_on_set_hook_while_defer_suspended
+    },
+    {
+        "on_remove_hook_while_defer_suspended",
+        DeferredActions_on_remove_hook_while_defer_suspended
     }
 };
 
@@ -13074,7 +13094,7 @@ static bake_test_suite suites[] = {
         "DeferredActions",
         NULL,
         NULL,
-        116,
+        120,
         DeferredActions_testcases
     },
     {