[observers] Fix issues with variables in triggered term of observer (#2023)

Author: SanderMertens

distr/flecs.c

@@ -16819,6 +16819,38 @@ void flecs_default_uni_observer_run_callback(ecs_iter_t *it) {
     o->callback(it);
 }
 
+static
+bool flecs_observer_query_has_range(
+    const ecs_query_t *query,
+    ecs_table_range_t *range,
+    const ecs_term_t *term,
+    ecs_id_t event_id,
+    ecs_iter_t *it)
+{
+    bool first_var = (term->first.id & EcsIsVariable) && term->first.name;
+    bool second_var = (term->second.id & EcsIsVariable) && term->second.name;
+    if (!first_var && !second_var) {
+        return ecs_query_has_range(query, range, it);
+    }
+
+    *it = ecs_query_iter(query->world, query);
+    ecs_iter_set_var_as_range(it, 0, range);
+    if (first_var) {
+        ecs_iter_set_var(it, ecs_query_find_var(query, term->first.name),
+            ECS_IS_PAIR(event_id) ? ECS_PAIR_FIRST(event_id) : event_id);
+    }
+    if (second_var) {
+        if (!ECS_IS_PAIR(event_id)) {
+            ecs_iter_fini(it);
+            return false;
+        }
+        ecs_iter_set_var(it, ecs_query_find_var(query, term->second.name),
+            ECS_PAIR_SECOND(event_id));
+    }
+
+    return ecs_query_next(it);
+}
+
 static
 void flecs_observer_invoke(
     ecs_observer_t *o,
@@ -17018,14 +17050,18 @@ void flecs_multi_observer_invoke(
 
     bool match;
     if (is_not) {
-        match = ecs_query_has_table(o->query, table, &user_it);
+        ecs_table_range_t range = { .table = table };
+        match = flecs_observer_query_has_range(
+            o->query, &range, term, it->event_id, &user_it);
         if (match) {
             /* The target table matches but the entity hasn't moved to it yet. 
              * Now match the not_query, which will populate the iterator with
              * data from the table the entity is still stored in. */
             user_it.flags |= EcsIterSkip; /* Prevent change detection on fini */
             ecs_iter_fini(&user_it);
-            match = ecs_query_has_table(impl->not_query, prev_table, &user_it);
+            ecs_table_range_t prev_range = { .table = prev_table };
+            match = flecs_observer_query_has_range(
+                impl->not_query, &prev_range, term, it->event_id, &user_it);
 
             /* A not query replaces Not terms with Optional terms, so if the 
              * regular query matches, the not_query should also match. */
@@ -17038,15 +17074,19 @@ void flecs_multi_observer_invoke(
             .count = it->count
         };
 
-        match = ecs_query_has_range(o->query, &range, &user_it);
+        match = flecs_observer_query_has_range(
+            o->query, &range, term, it->event_id, &user_it);
     }
 
     if (match) {
         /* Monitor observers only invoke when the query matches for the first
          * time with an entity */
         if (impl->flags & EcsObserverIsMonitor) {
             ecs_iter_t table_it;
-            if (ecs_query_has_table(o->query, prev_table, &table_it)) {
+            ecs_table_range_t prev_range = { .table = prev_table };
+            if (flecs_observer_query_has_range(
+                o->query, &prev_range, term, it->event_id, &table_it)) 
+            {
                 /* Prevent change detection on fini */
                 user_it.flags |= EcsIterSkip;
                 table_it.flags |= EcsIterSkip;

src/observer.c

@@ -360,6 +360,38 @@ void flecs_default_uni_observer_run_callback(ecs_iter_t *it) {
     o->callback(it);
 }
 
+static
+bool flecs_observer_query_has_range(
+    const ecs_query_t *query,
+    ecs_table_range_t *range,
+    const ecs_term_t *term,
+    ecs_id_t event_id,
+    ecs_iter_t *it)
+{
+    bool first_var = (term->first.id & EcsIsVariable) && term->first.name;
+    bool second_var = (term->second.id & EcsIsVariable) && term->second.name;
+    if (!first_var && !second_var) {
+        return ecs_query_has_range(query, range, it);
+    }
+
+    *it = ecs_query_iter(query->world, query);
+    ecs_iter_set_var_as_range(it, 0, range);
+    if (first_var) {
+        ecs_iter_set_var(it, ecs_query_find_var(query, term->first.name),
+            ECS_IS_PAIR(event_id) ? ECS_PAIR_FIRST(event_id) : event_id);
+    }
+    if (second_var) {
+        if (!ECS_IS_PAIR(event_id)) {
+            ecs_iter_fini(it);
+            return false;
+        }
+        ecs_iter_set_var(it, ecs_query_find_var(query, term->second.name),
+            ECS_PAIR_SECOND(event_id));
+    }
+
+    return ecs_query_next(it);
+}
+
 static
 void flecs_observer_invoke(
     ecs_observer_t *o,
@@ -559,14 +591,18 @@ void flecs_multi_observer_invoke(
 
     bool match;
     if (is_not) {
-        match = ecs_query_has_table(o->query, table, &user_it);
+        ecs_table_range_t range = { .table = table };
+        match = flecs_observer_query_has_range(
+            o->query, &range, term, it->event_id, &user_it);
         if (match) {
             /* The target table matches but the entity hasn't moved to it yet. 
              * Now match the not_query, which will populate the iterator with
              * data from the table the entity is still stored in. */
             user_it.flags |= EcsIterSkip; /* Prevent change detection on fini */
             ecs_iter_fini(&user_it);
-            match = ecs_query_has_table(impl->not_query, prev_table, &user_it);
+            ecs_table_range_t prev_range = { .table = prev_table };
+            match = flecs_observer_query_has_range(
+                impl->not_query, &prev_range, term, it->event_id, &user_it);
 
             /* A not query replaces Not terms with Optional terms, so if the 
              * regular query matches, the not_query should also match. */
@@ -579,15 +615,19 @@ void flecs_multi_observer_invoke(
             .count = it->count
         };
 
-        match = ecs_query_has_range(o->query, &range, &user_it);
+        match = flecs_observer_query_has_range(
+            o->query, &range, term, it->event_id, &user_it);
     }
 
     if (match) {
         /* Monitor observers only invoke when the query matches for the first
          * time with an entity */
         if (impl->flags & EcsObserverIsMonitor) {
             ecs_iter_t table_it;
-            if (ecs_query_has_table(o->query, prev_table, &table_it)) {
+            ecs_table_range_t prev_range = { .table = prev_table };
+            if (flecs_observer_query_has_range(
+                o->query, &prev_range, term, it->event_id, &table_it)) 
+            {
                 /* Prevent change detection on fini */
                 user_it.flags |= EcsIterSkip;
                 table_it.flags |= EcsIterSkip;

test/core/project.json

@@ -2123,6 +2123,10 @@
                 "propagate_add_childof_of_base",
                 "propagate_remove_childof_of_base",
                 "emit_for_parent_w_prefab_child_and_instance",
+                "query_eval_w_component_that_triggered_observer",
+                "query_eval_w_pair_first_var_that_triggered_observer",
+                "query_eval_w_pair_second_var_that_triggered_observer",
+                "query_eval_w_pair_both_vars_that_triggered_observer",
                 "observer_w_2_fixed_src",
                 "emit_for_recreated_id_after_remove_all",
                 "emit_for_recreated_id_after_remove_all_wildcard",

test/core/src/Observer.c

@@ -127,6 +127,28 @@ void Dummy(ecs_iter_t *it) {
     dummy_called = true;
 }
 
+typedef struct ObserverEventIdCtx {
+    ecs_entity_t entry_event;
+    ecs_id_t expected_id;
+    ecs_id_t next_id;
+    int32_t invoked;
+} ObserverEventIdCtx;
+
+static
+void ObserverOnEventId(ecs_iter_t *it) {
+    ObserverEventIdCtx *ctx = it->ctx;
+    test_int(it->count, 1);
+    test_int(it->ids[0], ctx->expected_id);
+    if (!ctx->invoked ++) {
+        ecs_add_id(it->world, it->entities[0], ctx->next_id);
+        ecs_enqueue(it->world, &(ecs_event_desc_t){
+            .event = ctx->entry_event,
+            .ids = &(ecs_type_t){ .count = 1, .array = (ecs_id_t[]){ ctx->next_id } },
+            .entity = it->entities[0]
+        });
+    }
+}
+
 void Observer_on_add_before_edge(void) {
     ecs_world_t *world = ecs_mini();
 
@@ -6059,6 +6081,150 @@ void Observer_emit_for_parent_w_prefab_child_and_instance(void) {
     ecs_fini(world);
 }
 
+void Observer_query_eval_w_component_that_triggered_observer(void) {
+    ecs_world_t *world = ecs_init();
+
+    ecs_entity_t entry_event = ecs_new(world);
+    ecs_entity_t sequence_shared = ecs_entity(world, { .name = "SequenceShared" });
+    ecs_add_id(world, sequence_shared, EcsTrait);
+    ecs_entity_t sequence = ecs_new_w_id(world, sequence_shared);
+    ecs_entity_t child = ecs_new(world);
+
+    ObserverEventIdCtx ctx = {
+        .entry_event = entry_event,
+        .expected_id = sequence,
+        .next_id = child
+    };
+
+    ecs_observer(world, {
+        .query.expr = "$Sequence, SequenceShared($Sequence)",
+        .events = { entry_event },
+        .callback = ObserverOnEventId,
+        .ctx = &ctx
+    });
+
+    ecs_enqueue(world, &(ecs_event_desc_t){
+        .event = entry_event,
+        .ids = &(ecs_type_t){ .count = 1, .array = (ecs_id_t[]){ sequence } },
+        .entity = ecs_new_w_id(world, sequence)
+    });
+
+    test_int(ctx.invoked, 1);
+
+    ecs_fini(world);
+}
+
+void Observer_query_eval_w_pair_first_var_that_triggered_observer(void) {
+    ecs_world_t *world = ecs_init();
+
+    ecs_entity_t entry_event = ecs_new(world);
+    ecs_entity_t rel_tag = ecs_entity(world, { .name = "RelTag" });
+    ecs_entity_t rel = ecs_entity(world, { .name = "MatchRel" });
+    ecs_entity_t tgt = ecs_entity(world, { .name = "MatchTgt" });
+    ecs_entity_t other_rel = ecs_entity(world, { .name = "OtherRel" });
+    ecs_add_id(world, rel, rel_tag);
+
+    ObserverEventIdCtx ctx = {
+        .entry_event = entry_event,
+        .expected_id = ecs_pair(rel, tgt),
+        .next_id = ecs_pair(other_rel, tgt)
+    };
+
+    ecs_observer(world, {
+        .query.expr = "($Rel, MatchTgt), RelTag($Rel)",
+        .events = { entry_event },
+        .callback = ObserverOnEventId,
+        .ctx = &ctx
+    });
+
+    ecs_entity_t e = ecs_new(world);
+    ecs_add_pair(world, e, rel, tgt);
+    ecs_enqueue(world, &(ecs_event_desc_t){
+        .event = entry_event,
+        .ids = &(ecs_type_t){ .count = 1, .array = (ecs_id_t[]){ ctx.expected_id } },
+        .entity = e
+    });
+
+    test_int(ctx.invoked, 1);
+
+    ecs_fini(world);
+}
+
+void Observer_query_eval_w_pair_second_var_that_triggered_observer(void) {
+    ecs_world_t *world = ecs_init();
+
+    ecs_entity_t entry_event = ecs_new(world);
+    ecs_entity_t tgt_tag = ecs_entity(world, { .name = "TgtTag" });
+    ecs_entity_t rel = ecs_entity(world, { .name = "MatchRel" });
+    ecs_entity_t tgt = ecs_entity(world, { .name = "MatchTgt" });
+    ecs_entity_t other_tgt = ecs_entity(world, { .name = "OtherTgt" });
+    ecs_add_id(world, tgt, tgt_tag);
+
+    ObserverEventIdCtx ctx = {
+        .entry_event = entry_event,
+        .expected_id = ecs_pair(rel, tgt),
+        .next_id = ecs_pair(rel, other_tgt)
+    };
+
+    ecs_observer(world, {
+        .query.expr = "(MatchRel, $Tgt), TgtTag($Tgt)",
+        .events = { entry_event },
+        .callback = ObserverOnEventId,
+        .ctx = &ctx
+    });
+
+    ecs_entity_t e = ecs_new(world);
+    ecs_add_pair(world, e, rel, tgt);
+    ecs_enqueue(world, &(ecs_event_desc_t){
+        .event = entry_event,
+        .ids = &(ecs_type_t){ .count = 1, .array = (ecs_id_t[]){ ctx.expected_id } },
+        .entity = e
+    });
+
+    test_int(ctx.invoked, 1);
+
+    ecs_fini(world);
+}
+
+void Observer_query_eval_w_pair_both_vars_that_triggered_observer(void) {
+    ecs_world_t *world = ecs_init();
+
+    ecs_entity_t entry_event = ecs_new(world);
+    ecs_entity_t rel_tag = ecs_entity(world, { .name = "RelTag" });
+    ecs_entity_t tgt_tag = ecs_entity(world, { .name = "TgtTag" });
+    ecs_entity_t rel = ecs_entity(world, { .name = "MatchRel" });
+    ecs_entity_t tgt = ecs_entity(world, { .name = "MatchTgt" });
+    ecs_entity_t other_rel = ecs_entity(world, { .name = "OtherRel" });
+    ecs_entity_t other_tgt = ecs_entity(world, { .name = "OtherTgt" });
+    ecs_add_id(world, rel, rel_tag);
+    ecs_add_id(world, tgt, tgt_tag);
+
+    ObserverEventIdCtx ctx = {
+        .entry_event = entry_event,
+        .expected_id = ecs_pair(rel, tgt),
+        .next_id = ecs_pair(other_rel, other_tgt)
+    };
+
+    ecs_observer(world, {
+        .query.expr = "($Rel, $Tgt), RelTag($Rel), TgtTag($Tgt)",
+        .events = { entry_event },
+        .callback = ObserverOnEventId,
+        .ctx = &ctx
+    });
+
+    ecs_entity_t e = ecs_new(world);
+    ecs_add_pair(world, e, rel, tgt);
+    ecs_enqueue(world, &(ecs_event_desc_t){
+        .event = entry_event,
+        .ids = &(ecs_type_t){ .count = 1, .array = (ecs_id_t[]){ ctx.expected_id } },
+        .entity = e
+    });
+
+    test_int(ctx.invoked, 1);
+
+    ecs_fini(world);
+}
+
 void Observer_observer_w_2_fixed_src(void) {
     ecs_world_t *world = ecs_init();
 

test/core/src/main.c

@@ -2056,6 +2056,10 @@ void Observer_propagate_remove_isa_of_parent(void);
 void Observer_propagate_add_childof_of_base(void);
 void Observer_propagate_remove_childof_of_base(void);
 void Observer_emit_for_parent_w_prefab_child_and_instance(void);
+void Observer_query_eval_w_component_that_triggered_observer(void);
+void Observer_query_eval_w_pair_first_var_that_triggered_observer(void);
+void Observer_query_eval_w_pair_second_var_that_triggered_observer(void);
+void Observer_query_eval_w_pair_both_vars_that_triggered_observer(void);
 void Observer_observer_w_2_fixed_src(void);
 void Observer_emit_for_recreated_id_after_remove_all(void);
 void Observer_emit_for_recreated_id_after_remove_all_wildcard(void);
@@ -11247,6 +11251,22 @@ bake_test_case Observer_testcases[] = {
         "emit_for_parent_w_prefab_child_and_instance",
         Observer_emit_for_parent_w_prefab_child_and_instance
     },
+    {
+        "query_eval_w_component_that_triggered_observer",
+        Observer_query_eval_w_component_that_triggered_observer
+    },
+    {
+        "query_eval_w_pair_first_var_that_triggered_observer",
+        Observer_query_eval_w_pair_first_var_that_triggered_observer
+    },
+    {
+        "query_eval_w_pair_second_var_that_triggered_observer",
+        Observer_query_eval_w_pair_second_var_that_triggered_observer
+    },
+    {
+        "query_eval_w_pair_both_vars_that_triggered_observer",
+        Observer_query_eval_w_pair_both_vars_that_triggered_observer
+    },
     {
         "observer_w_2_fixed_src",
         Observer_observer_w_2_fixed_src
@@ -16054,7 +16074,7 @@ static bake_test_suite suites[] = {
         "Observer",
         NULL,
         NULL,
-        328,
+        332,
         Observer_testcases
     },
     {