#1357 Trigger OnRemove yield_existing observer when it is deleted

SanderMertens · Sep 16, 2024 · 2616e66 · 2616e66
1 parent ec54821
commit 2616e66
Show file tree

Hide file tree

Showing 10 changed files with 664 additions and 47 deletions.
diff --git a/distr/flecs.c b/distr/flecs.c
@@ -14571,7 +14571,8 @@ void flecs_multi_observer_builtin_run(ecs_iter_t *it) {
 static
 void flecs_observer_yield_existing(
     ecs_world_t *world,
-    ecs_observer_t *o)
+    ecs_observer_t *o,
+    bool yield_on_remove)
 {
     ecs_run_action_t run = o->run;
     if (!run) {
@@ -14586,6 +14587,19 @@ void flecs_observer_yield_existing(
      * the event, if the event is iterable. */
     int i, count = o->event_count;
     for (i = 0; i < count; i ++) {
+        ecs_entity_t event = o->events[i];
+
+        /* We only yield for OnRemove events if the observer is deleted. */
+        if (event == EcsOnRemove) {
+            if (!yield_on_remove) {
+                continue;
+            }
+        } else {
+            if (yield_on_remove) {
+                continue;
+            }
+        }
+
         ecs_iter_t it = ecs_query_iter(world, o->query);
         it.system = o->entity;
         it.ctx = o;
@@ -14919,6 +14933,8 @@ ecs_observer_t* flecs_observer_init(
     impl->term_index = desc->term_index_;
     impl->flags = desc->flags_;
 
+    bool yield_on_create = false;
+
     /* Check if observer is monitor. Monitors are created as multi observers
      * since they require pre/post checking of the filter to test if the
      * entity is entering/leaving the monitor. */
@@ -14937,8 +14953,19 @@ ecs_observer_t* flecs_observer_init(
             o->events[1] = EcsOnRemove;
             o->event_count ++;
             impl->flags |= EcsObserverIsMonitor;
+            if (desc->yield_existing) {
+                impl->flags |= EcsObserverYieldOnDelete;
+                yield_on_create = true;
+            }
         } else {
             o->events[i] = event;
+            if (desc->yield_existing) {
+                if (event == EcsOnRemove) {
+                    impl->flags |= EcsObserverYieldOnDelete;
+                } else {
+                    yield_on_create = true;
+                }
+            }
         }
 
         o->event_count ++;
@@ -14972,8 +14999,8 @@ ecs_observer_t* flecs_observer_init(
         }
     }
 
-    if (desc->yield_existing) {
-        flecs_observer_yield_existing(world, o);
+    if (yield_on_create) {
+        flecs_observer_yield_existing(world, o, false);
     }
 
     return o;
@@ -15098,6 +15125,10 @@ void flecs_observer_fini(
     ecs_world_t *world = o->query->world;
     ecs_observer_impl_t *impl = flecs_observer_impl(o);
 
+    if (impl->flags & EcsObserverYieldOnDelete) {
+        flecs_observer_yield_existing(world, o, true);
+    }
+
     if (impl->flags & EcsObserverIsMulti) {
         ecs_observer_t **children = ecs_vec_first(&impl->children);
         int32_t i, children_count = ecs_vec_count(&impl->children);

diff --git a/distr/flecs.h b/distr/flecs.h
@@ -510,7 +510,8 @@ extern "C" {
 #define EcsObserverIsMonitor           (1u << 2u)  /* Is observer a monitor */
 #define EcsObserverIsDisabled          (1u << 3u)  /* Is observer entity disabled */
 #define EcsObserverIsParentDisabled    (1u << 4u)  /* Is module parent of observer disabled  */
-#define EcsObserverBypassQuery         (1u << 5u)
+#define EcsObserverBypassQuery         (1u << 5u)  /* Don't evaluate query for multi-component observer*/
+#define EcsObserverYieldOnDelete       (1u << 6u)  /* Yield matching entities when deleting observer */
 
 ////////////////////////////////////////////////////////////////////////////////
 //// Table flags (used by ecs_table_t::flags)

diff --git a/include/flecs/private/api_flags.h b/include/flecs/private/api_flags.h
@@ -188,7 +188,8 @@ extern "C" {
 #define EcsObserverIsMonitor           (1u << 2u)  /* Is observer a monitor */
 #define EcsObserverIsDisabled          (1u << 3u)  /* Is observer entity disabled */
 #define EcsObserverIsParentDisabled    (1u << 4u)  /* Is module parent of observer disabled  */
-#define EcsObserverBypassQuery         (1u << 5u)
+#define EcsObserverBypassQuery         (1u << 5u)  /* Don't evaluate query for multi-component observer*/
+#define EcsObserverYieldOnDelete       (1u << 6u)  /* Yield matching entities when deleting observer */
 
 ////////////////////////////////////////////////////////////////////////////////
 //// Table flags (used by ecs_table_t::flags)

diff --git a/src/observer.c b/src/observer.c
@@ -599,7 +599,8 @@ void flecs_multi_observer_builtin_run(ecs_iter_t *it) {
 static
 void flecs_observer_yield_existing(
     ecs_world_t *world,
-    ecs_observer_t *o)
+    ecs_observer_t *o,
+    bool yield_on_remove)
 {
     ecs_run_action_t run = o->run;
     if (!run) {
@@ -614,6 +615,19 @@ void flecs_observer_yield_existing(
      * the event, if the event is iterable. */
     int i, count = o->event_count;
     for (i = 0; i < count; i ++) {
+        ecs_entity_t event = o->events[i];
+
+        /* We only yield for OnRemove events if the observer is deleted. */
+        if (event == EcsOnRemove) {
+            if (!yield_on_remove) {
+                continue;
+            }
+        } else {
+            if (yield_on_remove) {
+                continue;
+            }
+        }
+
         ecs_iter_t it = ecs_query_iter(world, o->query);
         it.system = o->entity;
         it.ctx = o;
@@ -947,6 +961,8 @@ ecs_observer_t* flecs_observer_init(
     impl->term_index = desc->term_index_;
     impl->flags = desc->flags_;
 
+    bool yield_on_create = false;
+
     /* Check if observer is monitor. Monitors are created as multi observers
      * since they require pre/post checking of the filter to test if the
      * entity is entering/leaving the monitor. */
@@ -965,8 +981,19 @@ ecs_observer_t* flecs_observer_init(
             o->events[1] = EcsOnRemove;
             o->event_count ++;
             impl->flags |= EcsObserverIsMonitor;
+            if (desc->yield_existing) {
+                impl->flags |= EcsObserverYieldOnDelete;
+                yield_on_create = true;
+            }
         } else {
             o->events[i] = event;
+            if (desc->yield_existing) {
+                if (event == EcsOnRemove) {
+                    impl->flags |= EcsObserverYieldOnDelete;
+                } else {
+                    yield_on_create = true;
+                }
+            }
         }
 
         o->event_count ++;
@@ -1000,8 +1027,8 @@ ecs_observer_t* flecs_observer_init(
         }
     }
 
-    if (desc->yield_existing) {
-        flecs_observer_yield_existing(world, o);
+    if (yield_on_create) {
+        flecs_observer_yield_existing(world, o, false);
     }
 
     return o;
@@ -1126,6 +1153,10 @@ void flecs_observer_fini(
     ecs_world_t *world = o->query->world;
     ecs_observer_impl_t *impl = flecs_observer_impl(o);
 
+    if (impl->flags & EcsObserverYieldOnDelete) {
+        flecs_observer_yield_existing(world, o, true);
+    }
+
     if (impl->flags & EcsObserverIsMulti) {
         ecs_observer_t **children = ecs_vec_first(&impl->children);
         int32_t i, children_count = ecs_vec_count(&impl->children);

diff --git a/test/core/include/core/bake_config.h b/test/core/include/core/bake_config.h
@@ -1,25 +1,25 @@
-/*
-                                   )
-                                  (.)
-                                  .|.
-                                  | |
-                              _.--| |--._
-                           .-';  ;`-'& ; `&.
-                          \   &  ;    &   &_/
-                           |"""---...---"""|
-                           \ | | | | | | | /
-                            `---.|.|.|.---'
-
- * This file is generated by bake.lang.c for your convenience. Headers of
- * dependencies will automatically show up in this file. Include bake_config.h
- * in your main project file. Do not edit! */
-
-#ifndef CORE_BAKE_CONFIG_H
-#define CORE_BAKE_CONFIG_H
-
-/* Headers of public dependencies */
-#include <flecs.h>
-#include <bake_test.h>
-
-#endif
-
+/*
+                                   )
+                                  (.)
+                                  .|.
+                                  | |
+                              _.--| |--._
+                           .-';  ;`-'& ; `&.
+                          \   &  ;    &   &_/
+                           |"""---...---"""|
+                           \ | | | | | | | /
+                            `---.|.|.|.---'
+
+ * This file is generated by bake.lang.c for your convenience. Headers of
+ * dependencies will automatically show up in this file. Include bake_config.h
+ * in your main project file. Do not edit! */
+
+#ifndef CORE_BAKE_CONFIG_H
+#define CORE_BAKE_CONFIG_H
+
+/* Headers of public dependencies */
+#include <flecs.h>
+#include <bake_test.h>
+
+#endif
+
diff --git a/test/core/project.json b/test/core/project.json
@@ -1499,6 +1499,13 @@
                 "on_add_yield_existing_wildcard",
                 "on_add_yield_existing_wildcard_multi",
                 "on_add_yield_existing_wildcard_multi_w_wildcard_pivot",
+                "on_remove_yield_existing",
+                "on_remove_yield_existing_2_tables",
+                "on_remove_yield_existing_2_terms",
+                "on_remove_yield_existing_wildcard",
+                "on_remove_yield_existing_wildcard_multi",
+                "on_remove_yield_existing_wildcard_multi_w_wildcard_pivot",
+                "on_add_remove_yield_existing",
                 "observer_superset_wildcard",
                 "observer_superset_wildcard_add_isa",
                 "observer_superset_wildcard_add_isa_at_offset",
@@ -1557,8 +1564,10 @@
                 "notify_after_defer_batched_2_entities_in_table_w_tgt",
                 "multi_observer_table_fill_w_singleton",
                 "wildcard_propagate_w_other_table",
-                "add_in_yield_existing",
-                "add_in_yield_existing_multi",
+                "add_in_on_add_yield_existing",
+                "add_in_on_add_yield_existing_multi",
+                "add_in_on_remove_yield_existing",
+                "add_in_on_remove_yield_existing_multi",
                 "disable_observer",
                 "disable_observer_module",
                 "disable_observer_module_nested",
@@ -1725,7 +1734,8 @@
                 "monitor_w_wildcard",
                 "monitor_at_fini",
                 "monitor_other_table",
-                "monitor_component"
+                "monitor_component",
+                "yield_existing"
             ]
         }, {
             "id": "Prefab",

diff --git a/test/core/src/Monitor.c b/test/core/src/Monitor.c
@@ -733,3 +733,59 @@ void Monitor_monitor_component(void) {
     test_int(ctx.invoked, 4);
     test_assert(ctx.result == expect);
 }
+
+void Monitor_yield_existing(void) {
+    ecs_world_t *world = ecs_mini();
+
+    ECS_TAG(world, Tag);
+
+    /* Create entities before trigger */
+    ecs_entity_t e1 = ecs_new_w(world, Tag);
+    ecs_entity_t e2 = ecs_new_w(world, Tag);
+    ecs_entity_t e3 = ecs_new_w(world, Tag);
+
+    test_assert(e1 != 0);
+    test_assert(e2 != 0);
+    test_assert(e3 != 0);
+
+    Probe ctx = {0};
+    ecs_entity_t t = ecs_observer_init(world, &(ecs_observer_desc_t){
+        .query.terms = {{ Tag }},
+        .events = {EcsMonitor},
+        .callback = Monitor,
+        .ctx = &ctx,
+        .yield_existing = true
+    });
+
+    test_int(ctx.invoked, 1);
+    test_int(ctx.count, 3);
+    test_int(ctx.system, t);
+    test_int(ctx.event, EcsOnAdd);
+    test_int(ctx.event_id, Tag);
+    test_int(ctx.term_count, 1);
+    test_null(ctx.param);
+
+    test_int(ctx.e[0], e1);
+    test_int(ctx.e[1], e2);
+    test_int(ctx.e[2], e3);
+    test_int(ctx.c[0][0], Tag);
+
+    ecs_os_zeromem(&ctx);
+
+    ecs_delete(world, t);
+
+    test_int(ctx.invoked, 1);
+    test_int(ctx.count, 3);
+    test_int(ctx.system, t);
+    test_int(ctx.event, EcsOnRemove);
+    test_int(ctx.event_id, Tag);
+    test_int(ctx.term_count, 1);
+    test_null(ctx.param);
+
+    test_int(ctx.e[0], e1);
+    test_int(ctx.e[1], e2);
+    test_int(ctx.e[2], e3);
+    test_int(ctx.c[0][0], Tag);
+
+    ecs_fini(world);
+}