From 4a5784f4d9adb546a8d47079290b9d7f6987bd94 Mon Sep 17 00:00:00 2001 From: Sander Mertens Date: Tue, 23 Apr 2024 10:57:08 -0700 Subject: [PATCH] #1169 Fix issue with overcounting the number of nodata terms in a query with OR terms --- flecs.c | 23 +++++++++++----- src/filter.c | 23 +++++++++++----- test/api/project.json | 4 ++- test/api/src/Filter.c | 64 ++++++++++++++++++++++++++++++++++++++++++- test/api/src/main.c | 18 +++++++++--- test/meta/src/main.c | 1 + 6 files changed, 113 insertions(+), 20 deletions(-) diff --git a/flecs.c b/flecs.c index 54c4f4fa65..12608dcf68 100644 --- a/flecs.c +++ b/flecs.c @@ -11198,17 +11198,20 @@ ecs_term_t* flecs_filter_or_other_type( if (f->terms[t].oper != EcsOr) { break; } + first = &f->terms[t]; } if (first) { ecs_world_t *world = f->world; const ecs_type_info_t *first_type; + if (first->idr) { first_type = first->idr->type_info; } else { first_type = ecs_get_type_info(world, first->id); } + const ecs_type_info_t *term_type; if (term->idr) { term_type = term->idr->type_info; @@ -11219,6 +11222,7 @@ ecs_term_t* flecs_filter_or_other_type( if (first_type == term_type) { return NULL; } + return first; } else { return NULL; @@ -11328,7 +11332,9 @@ int ecs_filter_finalize( ecs_term_t *first = flecs_filter_or_other_type(f, i); if (first) { if (first == &term[-1]) { - filter_terms ++; + if (!(term[-1].flags & EcsTermNoData)) { + filter_terms ++; + } } filter_term = true; } @@ -12206,6 +12212,7 @@ bool flecs_term_match_table( bool first, ecs_flags32_t iter_flags) { + ecs_assert(term != NULL, ECS_INTERNAL_ERROR, NULL); const ecs_term_id_t *src = &term->src; ecs_oper_kind_t oper = term->oper; const ecs_table_t *match_table = table; @@ -13297,9 +13304,11 @@ bool ecs_filter_next_instanced( /* Match the remainder of the terms */ int32_t skip_term = pivot_term; - if (ecs_id_is_wildcard(filter->terms[pivot_term].id)) { - skip_term = -1; - iter->matches_left = 1; + if (pivot_term != -1) { + if (ecs_id_is_wildcard(filter->terms[pivot_term].id)) { + skip_term = -1; + iter->matches_left = 1; + } } match = flecs_filter_match_table(world, filter, table, @@ -13351,10 +13360,10 @@ bool ecs_filter_next_instanced( } int32_t t, term_count = filter->term_count; - ecs_term_t *term = NULL; + ecs_term_t *cur_term = NULL; for (t = 0; t < term_count; t ++) { if (filter->terms[t].field_index == i) { - term = &filter->terms[t]; + cur_term = &filter->terms[t]; break; } } @@ -13362,7 +13371,7 @@ bool ecs_filter_next_instanced( ecs_assert(term != NULL, ECS_INTERNAL_ERROR, NULL); it->columns[i] = column + 1; - flecs_term_match_table(world, term, table, + flecs_term_match_table(world, cur_term, table, &it->ids[i], &it->columns[i], &it->sources[i], &it->match_indices[i], false, it->flags); diff --git a/src/filter.c b/src/filter.c index 57c3e92260..0be6c4da7a 100644 --- a/src/filter.c +++ b/src/filter.c @@ -1148,17 +1148,20 @@ ecs_term_t* flecs_filter_or_other_type( if (f->terms[t].oper != EcsOr) { break; } + first = &f->terms[t]; } if (first) { ecs_world_t *world = f->world; const ecs_type_info_t *first_type; + if (first->idr) { first_type = first->idr->type_info; } else { first_type = ecs_get_type_info(world, first->id); } + const ecs_type_info_t *term_type; if (term->idr) { term_type = term->idr->type_info; @@ -1169,6 +1172,7 @@ ecs_term_t* flecs_filter_or_other_type( if (first_type == term_type) { return NULL; } + return first; } else { return NULL; @@ -1278,7 +1282,9 @@ int ecs_filter_finalize( ecs_term_t *first = flecs_filter_or_other_type(f, i); if (first) { if (first == &term[-1]) { - filter_terms ++; + if (!(term[-1].flags & EcsTermNoData)) { + filter_terms ++; + } } filter_term = true; } @@ -2156,6 +2162,7 @@ bool flecs_term_match_table( bool first, ecs_flags32_t iter_flags) { + ecs_assert(term != NULL, ECS_INTERNAL_ERROR, NULL); const ecs_term_id_t *src = &term->src; ecs_oper_kind_t oper = term->oper; const ecs_table_t *match_table = table; @@ -3247,9 +3254,11 @@ bool ecs_filter_next_instanced( /* Match the remainder of the terms */ int32_t skip_term = pivot_term; - if (ecs_id_is_wildcard(filter->terms[pivot_term].id)) { - skip_term = -1; - iter->matches_left = 1; + if (pivot_term != -1) { + if (ecs_id_is_wildcard(filter->terms[pivot_term].id)) { + skip_term = -1; + iter->matches_left = 1; + } } match = flecs_filter_match_table(world, filter, table, @@ -3301,10 +3310,10 @@ bool ecs_filter_next_instanced( } int32_t t, term_count = filter->term_count; - ecs_term_t *term = NULL; + ecs_term_t *cur_term = NULL; for (t = 0; t < term_count; t ++) { if (filter->terms[t].field_index == i) { - term = &filter->terms[t]; + cur_term = &filter->terms[t]; break; } } @@ -3312,7 +3321,7 @@ bool ecs_filter_next_instanced( ecs_assert(term != NULL, ECS_INTERNAL_ERROR, NULL); it->columns[i] = column + 1; - flecs_term_match_table(world, term, table, + flecs_term_match_table(world, cur_term, table, &it->ids[i], &it->columns[i], &it->sources[i], &it->match_indices[i], false, it->flags); diff --git a/test/api/project.json b/test/api/project.json index 1f85fc5029..b650903cb5 100644 --- a/test/api/project.json +++ b/test/api/project.json @@ -1310,7 +1310,9 @@ "filter_iter_3_or", "filter_iter_2_or_other_type", "filter_iter_2_or_same_type", - "filter_or_w_wildcard", + "filter_iter_or_w_wildcard", + "filer_iter_or_w_component_and_tag", + "filer_iter_or_w_tag_and_component", "filter_iter_1_component", "filter_iter_2_components", "filter_iter_pair_id", diff --git a/test/api/src/Filter.c b/test/api/src/Filter.c index 0a90932ff8..3c68029cb9 100644 --- a/test/api/src/Filter.c +++ b/test/api/src/Filter.c @@ -4643,7 +4643,7 @@ void Filter_filter_iter_2_or_same_type(void) { ecs_fini(world); } -void Filter_filter_or_w_wildcard(void) { +void Filter_filter_iter_or_w_wildcard(void) { ecs_world_t *world = ecs_mini(); ECS_TAG(world, Rel); @@ -4687,6 +4687,68 @@ void Filter_filter_or_w_wildcard(void) { ecs_fini(world); } +void Filter_filer_iter_or_w_component_and_tag(void) { + ecs_world_t *world = ecs_mini(); + + ECS_COMPONENT(world, Position); + ECS_TAG(world, TagA); + + ecs_filter_t *q = ecs_filter(world, { + .terms = { + { ecs_id(Position), .oper = EcsOr }, + { TagA } + } + }); + + test_assert(q != NULL); + + ecs_entity_t e = ecs_new_id(world); + ecs_set(world, e, Position, {10, 20}); + ecs_add(world, e, TagA); + + ecs_iter_t it = ecs_filter_iter(world, q); + test_bool(true, ecs_filter_next(&it)); + test_int(1, it.count); + test_uint(e, it.entities[0]); + test_uint(ecs_id(Position), ecs_field_id(&it, 1)); + test_bool(false, ecs_filter_next(&it)); + + ecs_filter_fini(q); + + ecs_fini(world); +} + +void Filter_filer_iter_or_w_tag_and_component(void) { + ecs_world_t *world = ecs_mini(); + + ECS_COMPONENT(world, Position); + ECS_TAG(world, TagA); + + ecs_filter_t *q = ecs_filter(world, { + .terms = { + { TagA, .oper = EcsOr }, + { ecs_id(Position) } + } + }); + + test_assert(q != NULL); + + ecs_entity_t e = ecs_new_id(world); + ecs_set(world, e, Position, {10, 20}); + ecs_add(world, e, TagA); + + ecs_iter_t it = ecs_filter_iter(world, q); + test_bool(true, ecs_filter_next(&it)); + test_int(1, it.count); + test_uint(e, it.entities[0]); + test_uint(TagA, ecs_field_id(&it, 1)); + test_bool(false, ecs_filter_next(&it)); + + ecs_filter_fini(q); + + ecs_fini(world); +} + void Filter_filter_iter_2_or(void) { ecs_world_t *world = ecs_mini(); diff --git a/test/api/src/main.c b/test/api/src/main.c index 0ede358ad9..c71707488b 100644 --- a/test/api/src/main.c +++ b/test/api/src/main.c @@ -1251,7 +1251,9 @@ void Filter_filter_iter_2_or(void); void Filter_filter_iter_3_or(void); void Filter_filter_iter_2_or_other_type(void); void Filter_filter_iter_2_or_same_type(void); -void Filter_filter_or_w_wildcard(void); +void Filter_filter_iter_or_w_wildcard(void); +void Filter_filer_iter_or_w_component_and_tag(void); +void Filter_filer_iter_or_w_tag_and_component(void); void Filter_filter_iter_1_component(void); void Filter_filter_iter_2_components(void); void Filter_filter_iter_pair_id(void); @@ -7568,8 +7570,16 @@ bake_test_case Filter_testcases[] = { Filter_filter_iter_2_or_same_type }, { - "filter_or_w_wildcard", - Filter_filter_or_w_wildcard + "filter_iter_or_w_wildcard", + Filter_filter_iter_or_w_wildcard + }, + { + "filer_iter_or_w_component_and_tag", + Filter_filer_iter_or_w_component_and_tag + }, + { + "filer_iter_or_w_tag_and_component", + Filter_filer_iter_or_w_tag_and_component }, { "filter_iter_1_component", @@ -13653,7 +13663,7 @@ static bake_test_suite suites[] = { "Filter", NULL, NULL, - 305, + 307, Filter_testcases }, { diff --git a/test/meta/src/main.c b/test/meta/src/main.c index 192b54f15b..2a7000980d 100644 --- a/test/meta/src/main.c +++ b/test/meta/src/main.c @@ -5738,6 +5738,7 @@ bake_test_case Misc_testcases[] = { } }; + static bake_test_suite suites[] = { { "PrimitiveTypes",