Skip to content

Commit

Permalink
Fix issue with yield_existing observers and not terms
Browse files Browse the repository at this point in the history
Summary: This diff fixes an issue where a yield_existing observers with a first term that has a not operator would fail to yield existing entities.

Differential Revision: D64272384
  • Loading branch information
SanderMertens authored and facebook-github-bot committed Oct 11, 2024
1 parent 88e4ccf commit 61280ca
Show file tree
Hide file tree
Showing 5 changed files with 119 additions and 8 deletions.
38 changes: 35 additions & 3 deletions distr/flecs.c
Original file line number Diff line number Diff line change
Expand Up @@ -14567,8 +14567,6 @@ void flecs_multi_observer_invoke(

ecs_table_unlock(it->world, table);
flecs_stage_set_system(world->stages[0], old_system);

return;
} else {
/* While the observer query was strictly speaking evaluated, it's more
* useful to measure how often the observer was actually invoked. */
Expand All @@ -14579,6 +14577,40 @@ void flecs_multi_observer_invoke(
return;
}

static
void flecs_multi_observer_invoke_no_query(
ecs_iter_t *it)
{
ecs_observer_t *o = it->ctx;
flecs_poly_assert(o, ecs_observer_t);

ecs_world_t *world = it->real_world;
ecs_table_t *table = it->table;
ecs_iter_t user_it = *it;

user_it.ctx = o->ctx;
user_it.callback_ctx = o->callback_ctx;
user_it.run_ctx = o->run_ctx;
user_it.param = it->param;
user_it.callback = o->callback;
user_it.system = o->entity;
user_it.event = it->event;

ecs_entity_t old_system = flecs_stage_set_system(
world->stages[0], o->entity);
ecs_table_lock(it->world, table);

if (o->run) {
user_it.next = flecs_default_next_callback;
o->run(&user_it);
} else {
user_it.callback(&user_it);
}

ecs_table_unlock(it->world, table);
flecs_stage_set_system(world->stages[0], old_system);
}

/* For convenience, so applications can (in theory) use a single run callback
* that uses ecs_iter_next to iterate results */
bool flecs_default_next_callback(ecs_iter_t *it) {
Expand Down Expand Up @@ -14620,7 +14652,7 @@ void flecs_observer_yield_existing(
{
ecs_run_action_t run = o->run;
if (!run) {
run = flecs_multi_observer_invoke;
run = flecs_multi_observer_invoke_no_query;
}

ecs_run_aperiodic(world, EcsAperiodicEmptyTables);
Expand Down
38 changes: 35 additions & 3 deletions src/observer.c
Original file line number Diff line number Diff line change
Expand Up @@ -551,8 +551,6 @@ void flecs_multi_observer_invoke(

ecs_table_unlock(it->world, table);
flecs_stage_set_system(world->stages[0], old_system);

return;
} else {
/* While the observer query was strictly speaking evaluated, it's more
* useful to measure how often the observer was actually invoked. */
Expand All @@ -563,6 +561,40 @@ void flecs_multi_observer_invoke(
return;
}

static
void flecs_multi_observer_invoke_no_query(
ecs_iter_t *it)
{
ecs_observer_t *o = it->ctx;
flecs_poly_assert(o, ecs_observer_t);

ecs_world_t *world = it->real_world;
ecs_table_t *table = it->table;
ecs_iter_t user_it = *it;

user_it.ctx = o->ctx;
user_it.callback_ctx = o->callback_ctx;
user_it.run_ctx = o->run_ctx;
user_it.param = it->param;
user_it.callback = o->callback;
user_it.system = o->entity;
user_it.event = it->event;

ecs_entity_t old_system = flecs_stage_set_system(
world->stages[0], o->entity);
ecs_table_lock(it->world, table);

if (o->run) {
user_it.next = flecs_default_next_callback;
o->run(&user_it);
} else {
user_it.callback(&user_it);
}

ecs_table_unlock(it->world, table);
flecs_stage_set_system(world->stages[0], old_system);
}

/* For convenience, so applications can (in theory) use a single run callback
* that uses ecs_iter_next to iterate results */
bool flecs_default_next_callback(ecs_iter_t *it) {
Expand Down Expand Up @@ -604,7 +636,7 @@ void flecs_observer_yield_existing(
{
ecs_run_action_t run = o->run;
if (!run) {
run = flecs_multi_observer_invoke;
run = flecs_multi_observer_invoke_no_query;
}

ecs_run_aperiodic(world, EcsAperiodicEmptyTables);
Expand Down
3 changes: 2 additions & 1 deletion test/core/project.json
Original file line number Diff line number Diff line change
Expand Up @@ -487,7 +487,7 @@
"emplace_inherited",
"override_component",
"delete_w_override_component",
"delete_w_override_on_remove_isa",
"delete_w_override_on_remove_isa",
"ctor_after_emplace",
"ctor_dtor_after_remove",
"ctor_dtor_after_clear",
Expand Down Expand Up @@ -1537,6 +1537,7 @@
"yield_existing_flags_w_multi_observer",
"yield_on_create_without_on_add",
"yield_on_delete_without_on_remove",
"yield_existing_w_not_first_term",
"observer_superset_wildcard",
"observer_superset_wildcard_add_isa",
"observer_superset_wildcard_add_isa_at_offset",
Expand Down
41 changes: 41 additions & 0 deletions test/core/src/Observer.c
Original file line number Diff line number Diff line change
Expand Up @@ -4479,6 +4479,47 @@ void Observer_yield_on_delete_without_on_remove(void) {
ecs_fini(world);
}

void Observer_yield_existing_w_not_first_term(void) {
ecs_world_t *world = ecs_mini();

ECS_TAG(world, Foo);
ECS_TAG(world, Bar);

/* Create entities before trigger */
ecs_entity_t e1 = ecs_new_w(world, Foo);
ecs_entity_t e2 = ecs_new_w(world, Foo);
ecs_entity_t e3 = ecs_new_w(world, Foo);

test_assert(e1 != 0);
test_assert(e2 != 0);
test_assert(e3 != 0);

ecs_add(world, e3, Bar);

Probe ctx = {0};
ecs_entity_t t = ecs_observer_init(world, &(ecs_observer_desc_t){
.query.terms = {{ Bar, .oper = EcsNot }, { Foo }},
.events = {EcsOnAdd},
.callback = Observer,
.ctx = &ctx,
.yield_existing = true
});

test_int(ctx.invoked, 1);
test_int(ctx.count, 2);
test_int(ctx.system, t);
test_int(ctx.event, EcsOnAdd);
test_int(ctx.term_count, 2);
test_null(ctx.param);

test_int(ctx.e[0], e1);
test_int(ctx.e[1], e2);
test_int(ctx.c[0][0], Bar);
test_int(ctx.c[0][1], Foo);

ecs_fini(world);
}

void Observer_observer_superset_wildcard(void) {
ecs_world_t *world = ecs_mini();

Expand Down
7 changes: 6 additions & 1 deletion test/core/src/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -1476,6 +1476,7 @@ void Observer_on_add_remove_no_on_remove_yield_existing(void);
void Observer_yield_existing_flags_w_multi_observer(void);
void Observer_yield_on_create_without_on_add(void);
void Observer_yield_on_delete_without_on_remove(void);
void Observer_yield_existing_w_not_first_term(void);
void Observer_observer_superset_wildcard(void);
void Observer_observer_superset_wildcard_add_isa(void);
void Observer_observer_superset_wildcard_add_isa_at_offset(void);
Expand Down Expand Up @@ -7970,6 +7971,10 @@ bake_test_case Observer_testcases[] = {
"yield_on_delete_without_on_remove",
Observer_yield_on_delete_without_on_remove
},
{
"yield_existing_w_not_first_term",
Observer_yield_existing_w_not_first_term
},
{
"observer_superset_wildcard",
Observer_observer_superset_wildcard
Expand Down Expand Up @@ -11225,7 +11230,7 @@ static bake_test_suite suites[] = {
"Observer",
NULL,
NULL,
222,
223,
Observer_testcases
},
{
Expand Down

0 comments on commit 61280ca

Please sign in to comment.