From 2b0a2d838823a7bed2a6289ec6096b52ec6de049 Mon Sep 17 00:00:00 2001 From: Sander Mertens Date: Mon, 16 Jan 2023 21:30:27 -0800 Subject: [PATCH] Fix observer crash when matching a wildcard id on base entity --- flecs.c | 24 +++++++--- flecs.h | 9 ++-- src/filter.c | 12 +++-- src/observer.c | 9 +++- src/search.c | 3 +- test/api/project.json | 2 + test/api/src/Observer.c | 92 ++++++++++++++++++++++++++++++++++++++ test/api/src/Stresstests.c | 4 +- test/api/src/main.c | 12 ++++- 9 files changed, 149 insertions(+), 18 deletions(-) diff --git a/flecs.c b/flecs.c index 40353fc846..bf6ed5bb13 100644 --- a/flecs.c +++ b/flecs.c @@ -45195,13 +45195,19 @@ bool flecs_filter_match_table( } for (i = 0; i < count; i ++) { + ecs_term_t *term = &terms[i]; + ecs_oper_kind_t oper = term->oper; if (i == skip_term) { - continue; + if (oper == EcsOr) { + is_or = true; + or_result = true; + } + if (oper != EcsAndFrom && oper != EcsOrFrom && oper != EcsNotFrom) { + continue; + } } - ecs_term_t *term = &terms[i]; ecs_term_id_t *src = &term->src; - ecs_oper_kind_t oper = term->oper; const ecs_table_t *match_table = table; int32_t t_i = term->field_index; @@ -46153,6 +46159,7 @@ bool flecs_type_can_inherit_id( const ecs_id_record_t *idr, ecs_id_t id) { + ecs_assert(idr != NULL, ECS_INTERNAL_ERROR, NULL); if (idr->flags & EcsIdDontInherit) { return false; } @@ -46289,7 +46296,7 @@ int32_t flecs_search_relation_w_idr( flags = flags ? flags : (EcsSelf|EcsUp); - if (!idr && !offset) { + if (!idr) { idr = flecs_query_id_record_get(world, id); if (!idr) { return -1; @@ -46873,9 +46880,10 @@ bool flecs_multi_observer_invoke(ecs_iter_t *it) { user_it.columns[0] = 0; user_it.columns[pivot_term] = column; + user_it.sources[pivot_term] = it->sources[0]; if (flecs_filter_match_table(world, &o->filter, table, user_it.ids, - user_it.columns, user_it.sources, NULL, NULL, false, -1, + user_it.columns, user_it.sources, NULL, NULL, false, pivot_term, user_it.flags)) { /* Monitor observers only invoke when the filter matches for the first @@ -46897,10 +46905,14 @@ bool flecs_multi_observer_invoke(ecs_iter_t *it) { user_it.columns, user_it.sources, NULL, NULL, false, -1, user_it.flags | EcsFilterPopulate); } - + flecs_iter_populate_data(world, &user_it, it->table, it->offset, it->count, user_it.ptrs, user_it.sizes); + if (it->ptrs) { + user_it.ptrs[pivot_term] = it->ptrs[0]; + user_it.sizes[pivot_term] = it->sizes[0]; + } user_it.ids[pivot_term] = it->event_id; user_it.system = o->filter.entity; user_it.term_index = pivot_term; diff --git a/flecs.h b/flecs.h index 319dd8f784..c696f17ee0 100644 --- a/flecs.h +++ b/flecs.h @@ -17439,10 +17439,13 @@ namespace flecs */ template struct ref { - ref(world_t *world, entity_t entity, flecs::id_t id = 0) - : m_world( world ) - , m_ref() + ref(world_t *world, entity_t entity, flecs::id_t id = 0) + : m_ref() { + // the world we were called with may be a stage; convert it to a world + // here if that is the case + m_world = world ? const_cast(ecs_get_world(world)) + : nullptr; if (!id) { id = _::cpp_type::id(world); } diff --git a/src/filter.c b/src/filter.c index dd721f1c05..f7b5bc5759 100644 --- a/src/filter.c +++ b/src/filter.c @@ -1713,13 +1713,19 @@ bool flecs_filter_match_table( } for (i = 0; i < count; i ++) { + ecs_term_t *term = &terms[i]; + ecs_oper_kind_t oper = term->oper; if (i == skip_term) { - continue; + if (oper == EcsOr) { + is_or = true; + or_result = true; + } + if (oper != EcsAndFrom && oper != EcsOrFrom && oper != EcsNotFrom) { + continue; + } } - ecs_term_t *term = &terms[i]; ecs_term_id_t *src = &term->src; - ecs_oper_kind_t oper = term->oper; const ecs_table_t *match_table = table; int32_t t_i = term->field_index; diff --git a/src/observer.c b/src/observer.c index 9315245c39..42ab1b7508 100644 --- a/src/observer.c +++ b/src/observer.c @@ -436,9 +436,10 @@ bool flecs_multi_observer_invoke(ecs_iter_t *it) { user_it.columns[0] = 0; user_it.columns[pivot_term] = column; + user_it.sources[pivot_term] = it->sources[0]; if (flecs_filter_match_table(world, &o->filter, table, user_it.ids, - user_it.columns, user_it.sources, NULL, NULL, false, -1, + user_it.columns, user_it.sources, NULL, NULL, false, pivot_term, user_it.flags)) { /* Monitor observers only invoke when the filter matches for the first @@ -460,10 +461,14 @@ bool flecs_multi_observer_invoke(ecs_iter_t *it) { user_it.columns, user_it.sources, NULL, NULL, false, -1, user_it.flags | EcsFilterPopulate); } - + flecs_iter_populate_data(world, &user_it, it->table, it->offset, it->count, user_it.ptrs, user_it.sizes); + if (it->ptrs) { + user_it.ptrs[pivot_term] = it->ptrs[0]; + user_it.sizes[pivot_term] = it->sizes[0]; + } user_it.ids[pivot_term] = it->event_id; user_it.system = o->filter.entity; user_it.term_index = pivot_term; diff --git a/src/search.c b/src/search.c index 778e2c83c3..c5f5d9e127 100644 --- a/src/search.c +++ b/src/search.c @@ -68,6 +68,7 @@ bool flecs_type_can_inherit_id( const ecs_id_record_t *idr, ecs_id_t id) { + ecs_assert(idr != NULL, ECS_INTERNAL_ERROR, NULL); if (idr->flags & EcsIdDontInherit) { return false; } @@ -204,7 +205,7 @@ int32_t flecs_search_relation_w_idr( flags = flags ? flags : (EcsSelf|EcsUp); - if (!idr && !offset) { + if (!idr) { idr = flecs_query_id_record_get(world, id); if (!idr) { return -1; diff --git a/test/api/project.json b/test/api/project.json index 60790903d9..d7eb7f9b61 100644 --- a/test/api/project.json +++ b/test/api/project.json @@ -1816,6 +1816,8 @@ "on_add_2_pairs_w_multi_observer", "on_set_2_pairs_w_uni_observer", "on_set_2_pairs_w_multi_observer", + "on_remove_target_from_base_at_offset", + "on_remove_target_component_from_base_at_offset", "cache_test_1", "cache_test_2", "cache_test_3", diff --git a/test/api/src/Observer.c b/test/api/src/Observer.c index 0868923403..6323c85896 100644 --- a/test/api/src/Observer.c +++ b/test/api/src/Observer.c @@ -3728,6 +3728,98 @@ void Observer_on_set_2_pairs_w_multi_observer() { ecs_fini(world); } +void Observer_on_remove_target_from_base_at_offset() { + ecs_world_t *world = ecs_mini(); + + ecs_entity_t R = ecs_new_id(world); + ecs_entity_t T1 = ecs_new_id(world); + ecs_entity_t T2 = ecs_new_id(world); + ecs_entity_t C = ecs_new_id(world); + + Probe ctx = { 0 }; + ecs_entity_t o = ecs_observer(world, { + .filter.terms = { + { .id = ecs_pair(R, EcsWildcard), .src.flags = EcsUp }, + { .id = C }, + }, + .events = { EcsOnRemove }, + .callback = Observer, + .ctx = &ctx + }); + + ecs_entity_t base = ecs_new_id(world); + ecs_add_pair(world, base, R, T1); + ecs_add_pair(world, base, R, T2); + + ecs_entity_t e = ecs_new_id(world); + ecs_add_pair(world, e, EcsIsA, base); + ecs_add_id(world, e, C); + + test_int(ctx.invoked, 0); + ecs_delete(world, T2); + test_int(ctx.invoked, 1); + test_int(ctx.count, 1); + test_int(ctx.system, o); + test_int(ctx.term_count, 2); + test_int(ctx.event_id, ecs_pair(R, T2)); + test_int(ctx.event, EcsOnRemove); + + ecs_fini(world); +} + +static void Observer_base_component(ecs_iter_t *it) { + probe_system_w_ctx(it, it->ctx); + + Position *p = ecs_field(it, Position, 1); + test_assert(p != NULL); + test_int(p->x, 30); + test_int(p->y, 40); +} + +void Observer_on_remove_target_component_from_base_at_offset() { + ecs_world_t *world = ecs_mini(); + + ECS_COMPONENT(world, Position); + ecs_entity_t T1 = ecs_new_id(world); + ecs_entity_t T2 = ecs_new_id(world); + ecs_entity_t C = ecs_new_id(world); + + Probe ctx = { 0 }; + ecs_entity_t o = ecs_observer(world, { + .filter.terms = { + { .id = ecs_pair(ecs_id(Position), EcsWildcard), .src.flags = EcsUp }, + { .id = C }, + }, + .events = { EcsOnRemove }, + .callback = Observer_base_component, + .ctx = &ctx + }); + + ecs_entity_t base = ecs_new_id(world); + ecs_set_pair(world, base, Position, T1, {10, 20}); + ecs_set_pair(world, base, Position, T2, {30, 40}); + + ecs_entity_t e = ecs_new_id(world); + ecs_add_pair(world, e, EcsIsA, base); + ecs_add_id(world, e, C); + + test_int(ctx.invoked, 0); + ecs_delete(world, T2); + test_int(ctx.invoked, 1); + test_int(ctx.count, 1); + test_int(ctx.system, o); + test_int(ctx.term_count, 2); + test_int(ctx.e[0], e); + test_int(ctx.s[0][0], base); + test_int(ctx.s[0][1], 0); + test_int(ctx.event_id, ecs_pair(ecs_id(Position), T2)); + test_int(ctx.event, EcsOnRemove); + + ecs_delete(world, o); + + ecs_fini(world); +} + void Observer_cache_test_1() { ecs_world_t *world = ecs_mini(); diff --git a/test/api/src/Stresstests.c b/test/api/src/Stresstests.c index 90919ecc7d..09a6e5302d 100644 --- a/test/api/src/Stresstests.c +++ b/test/api/src/Stresstests.c @@ -92,7 +92,7 @@ static void create_delete_entity_random_components_staged( int32_t threads) { - test_quarantine("---"); + test_quarantine("16 Jan 2023"); return; ecs_world_t *world = ecs_init(); @@ -139,7 +139,7 @@ static void set_entity_random_components( int32_t threads) { - test_quarantine("---"); + test_quarantine("16 Jan 2023"); return; ecs_world_t *world = ecs_init(); diff --git a/test/api/src/main.c b/test/api/src/main.c index f952732ad2..11b7cf2b61 100644 --- a/test/api/src/main.c +++ b/test/api/src/main.c @@ -1747,6 +1747,8 @@ void Observer_on_add_2_pairs_w_uni_observer(void); void Observer_on_add_2_pairs_w_multi_observer(void); void Observer_on_set_2_pairs_w_uni_observer(void); void Observer_on_set_2_pairs_w_multi_observer(void); +void Observer_on_remove_target_from_base_at_offset(void); +void Observer_on_remove_target_component_from_base_at_offset(void); void Observer_cache_test_1(void); void Observer_cache_test_2(void); void Observer_cache_test_3(void); @@ -9108,6 +9110,14 @@ bake_test_case Observer_testcases[] = { "on_set_2_pairs_w_multi_observer", Observer_on_set_2_pairs_w_multi_observer }, + { + "on_remove_target_from_base_at_offset", + Observer_on_remove_target_from_base_at_offset + }, + { + "on_remove_target_component_from_base_at_offset", + Observer_on_remove_target_component_from_base_at_offset + }, { "cache_test_1", Observer_cache_test_1 @@ -11607,7 +11617,7 @@ static bake_test_suite suites[] = { "Observer", NULL, NULL, - 102, + 104, Observer_testcases }, {