From 34dc148c29bdc2557de85b36e1616713d97a7dd2 Mon Sep 17 00:00:00 2001 From: Sander Mertens Date: Wed, 4 Sep 2024 13:28:03 -0700 Subject: [PATCH] Fix issue where up traversal could return duplicate result --- distr/flecs.c | 23 +---- src/query/engine/engine.h | 18 ---- src/query/engine/trav_down_cache.c | 5 + test/query/project.json | 4 + test/query/src/Traversal.c | 158 +++++++++++++++++++++++++++++ test/query/src/main.c | 22 +++- 6 files changed, 193 insertions(+), 37 deletions(-) diff --git a/distr/flecs.c b/distr/flecs.c index c54e84d54..305c6b2ac 100644 --- a/distr/flecs.c +++ b/distr/flecs.c @@ -2338,24 +2338,6 @@ bool flecs_query_self_up_with( const ecs_query_run_ctx_t *ctx, bool id_only); -bool flecs_query_up_select( - const ecs_query_op_t *op, - bool redo, - const ecs_query_run_ctx_t *ctx, - ecs_query_up_select_trav_kind_t trav_kind, - ecs_query_up_select_kind_t kind); - -bool flecs_query_up_with( - const ecs_query_op_t *op, - bool redo, - const ecs_query_run_ctx_t *ctx); - -bool flecs_query_self_up_with( - const ecs_query_op_t *op, - bool redo, - const ecs_query_run_ctx_t *ctx, - bool id_only); - /* Transitive relationship traversal */ @@ -72323,6 +72305,11 @@ void flecs_trav_entity_down_isa( continue; } + if (ecs_table_has_id(world, table, idr_with->id)) { + /* Table owns component */ + continue; + } + const ecs_entity_t *entities = ecs_table_entities(table); int32_t i, count = ecs_table_count(table); for (i = 0; i < count; i ++) { diff --git a/src/query/engine/engine.h b/src/query/engine/engine.h index 54eaf9145..6a89c408f 100644 --- a/src/query/engine/engine.h +++ b/src/query/engine/engine.h @@ -314,24 +314,6 @@ bool flecs_query_self_up_with( const ecs_query_run_ctx_t *ctx, bool id_only); -bool flecs_query_up_select( - const ecs_query_op_t *op, - bool redo, - const ecs_query_run_ctx_t *ctx, - ecs_query_up_select_trav_kind_t trav_kind, - ecs_query_up_select_kind_t kind); - -bool flecs_query_up_with( - const ecs_query_op_t *op, - bool redo, - const ecs_query_run_ctx_t *ctx); - -bool flecs_query_self_up_with( - const ecs_query_op_t *op, - bool redo, - const ecs_query_run_ctx_t *ctx, - bool id_only); - /* Transitive relationship traversal */ diff --git a/src/query/engine/trav_down_cache.c b/src/query/engine/trav_down_cache.c index c6d9ce009..0ffb84d61 100644 --- a/src/query/engine/trav_down_cache.c +++ b/src/query/engine/trav_down_cache.c @@ -122,6 +122,11 @@ void flecs_trav_entity_down_isa( continue; } + if (ecs_table_has_id(world, table, idr_with->id)) { + /* Table owns component */ + continue; + } + const ecs_entity_t *entities = ecs_table_entities(table); int32_t i, count = ecs_table_count(table); for (i = 0; i < count; i ++) { diff --git a/test/query/project.json b/test/query/project.json index d44e6547d..196371a22 100644 --- a/test/query/project.json +++ b/test/query/project.json @@ -1411,8 +1411,12 @@ "up_all_owned", "this_self_up_childof_inherited", "this_up_childof_inherited", + "this_self_up_childof_inherited_override", + "this_up_childof_inherited_override", "this_written_self_up_childof_inherited", "this_written_up_childof_inherited", + "this_written_self_up_childof_inherited_override", + "this_written_up_childof_inherited_override", "var_self_up_childof_inherited", "var_up_childof_inherited", "var_written_self_up_childof_inherited", diff --git a/test/query/src/Traversal.c b/test/query/src/Traversal.c index 1d6f82555..a5cd26f33 100644 --- a/test/query/src/Traversal.c +++ b/test/query/src/Traversal.c @@ -2811,6 +2811,78 @@ void Traversal_this_up_childof_inherited(void) { ecs_fini(world); } +void Traversal_this_self_up_childof_inherited_override(void) { + ecs_world_t *world = ecs_mini(); + + ECS_ENTITY(world, Foo, (OnInstantiate, Inherit)); + + ecs_entity_t base = ecs_entity(world, { .add = ecs_ids(Foo) }); + ecs_entity_t parent = ecs_entity(world, { .add = ecs_ids(ecs_isa(base), Foo) }); + ecs_entity_t child = ecs_entity(world, { .parent = parent }); + + ecs_query_t *r = ecs_query(world, { + .expr = "Foo(self|up)", + .cache_kind = cache_kind + }); + + test_assert(r != NULL); + + ecs_iter_t it = ecs_query_iter(world, r); + test_bool(true, ecs_query_next(&it)); + test_int(1, it.count); + test_uint(base, it.entities[0]); + test_uint(0, ecs_field_src(&it, 0)); + test_uint(Foo, ecs_field_id(&it, 0)); + + test_bool(true, ecs_query_next(&it)); + test_int(1, it.count); + test_uint(parent, it.entities[0]); + test_uint(0, ecs_field_src(&it, 0)); + test_uint(Foo, ecs_field_id(&it, 0)); + + test_bool(true, ecs_query_next(&it)); + test_int(1, it.count); + test_uint(child, it.entities[0]); + test_uint(parent, ecs_field_src(&it, 0)); + test_uint(Foo, ecs_field_id(&it, 0)); + + test_bool(false, ecs_query_next(&it)); + + ecs_query_fini(r); + + ecs_fini(world); +} + +void Traversal_this_up_childof_inherited_override(void) { + ecs_world_t *world = ecs_mini(); + + ECS_ENTITY(world, Foo, (OnInstantiate, Inherit)); + + ecs_entity_t base = ecs_entity(world, { .add = ecs_ids(Foo) }); + ecs_entity_t parent = ecs_entity(world, { .add = ecs_ids(ecs_isa(base), Foo) }); + ecs_entity_t child = ecs_entity(world, { .parent = parent }); + + ecs_query_t *r = ecs_query(world, { + .expr = "Foo(up)", + .cache_kind = cache_kind + }); + + test_assert(r != NULL); + + ecs_iter_t it = ecs_query_iter(world, r); + test_bool(true, ecs_query_next(&it)); + test_int(1, it.count); + test_uint(child, it.entities[0]); + test_uint(parent, ecs_field_src(&it, 0)); + test_uint(Foo, ecs_field_id(&it, 0)); + + test_bool(false, ecs_query_next(&it)); + + ecs_query_fini(r); + + ecs_fini(world); +} + void Traversal_this_written_self_up_childof_inherited(void) { ecs_world_t *world = ecs_mini(); @@ -2889,6 +2961,92 @@ void Traversal_this_written_up_childof_inherited(void) { ecs_fini(world); } +void Traversal_this_written_self_up_childof_inherited_override(void) { + ecs_world_t *world = ecs_mini(); + + ECS_ENTITY(world, Foo, (OnInstantiate, Inherit)); + ECS_ENTITY(world, Tag, (OnInstantiate, Inherit)); + + ecs_query_t *r = ecs_query(world, { + .expr = "Tag(self), Foo(self|up)", + .cache_kind = cache_kind + }); + + test_assert(r != NULL); + + ecs_set_with(world, Tag); + ecs_entity_t base = ecs_entity(world, { .add = ecs_ids(Foo) }); + ecs_entity_t parent = ecs_entity(world, { .add = ecs_ids(ecs_isa(base), Foo) }); + ecs_entity_t child = ecs_entity(world, { .parent = parent }); + ecs_set_with(world, 0); + + ecs_iter_t it = ecs_query_iter(world, r); + test_bool(true, ecs_query_next(&it)); + test_int(1, it.count); + test_uint(base, it.entities[0]); + test_uint(0, ecs_field_src(&it, 0)); + test_uint(0, ecs_field_src(&it, 1)); + test_uint(Tag, ecs_field_id(&it, 0)); + test_uint(Foo, ecs_field_id(&it, 1)); + + test_bool(true, ecs_query_next(&it)); + test_int(1, it.count); + test_uint(parent, it.entities[0]); + test_uint(0, ecs_field_src(&it, 0)); + test_uint(0, ecs_field_src(&it, 1)); + test_uint(Tag, ecs_field_id(&it, 0)); + test_uint(Foo, ecs_field_id(&it, 1)); + + test_bool(true, ecs_query_next(&it)); + test_int(1, it.count); + test_uint(child, it.entities[0]); + test_uint(0, ecs_field_src(&it, 0)); + test_uint(parent, ecs_field_src(&it, 1)); + test_uint(Tag, ecs_field_id(&it, 0)); + test_uint(Foo, ecs_field_id(&it, 1)); + + test_bool(false, ecs_query_next(&it)); + + ecs_query_fini(r); + + ecs_fini(world); +} + +void Traversal_this_written_up_childof_inherited_override(void) { + ecs_world_t *world = ecs_mini(); + + ECS_ENTITY(world, Foo, (OnInstantiate, Inherit)); + ECS_ENTITY(world, Tag, (OnInstantiate, Inherit)); + + ecs_query_t *r = ecs_query(world, { + .expr = "Tag(self), Foo(up)", + .cache_kind = cache_kind + }); + + test_assert(r != NULL); + + ecs_set_with(world, Tag); + ecs_entity_t base = ecs_entity(world, { .add = ecs_ids(Foo) }); + ecs_entity_t parent = ecs_entity(world, { .add = ecs_ids(ecs_isa(base), Foo) }); + ecs_entity_t child = ecs_entity(world, { .parent = parent }); + ecs_set_with(world, 0); + + ecs_iter_t it = ecs_query_iter(world, r); + test_bool(true, ecs_query_next(&it)); + test_int(1, it.count); + test_uint(child, it.entities[0]); + test_uint(0, ecs_field_src(&it, 0)); + test_uint(parent, ecs_field_src(&it, 1)); + test_uint(Tag, ecs_field_id(&it, 0)); + test_uint(Foo, ecs_field_id(&it, 1)); + + test_bool(false, ecs_query_next(&it)); + + ecs_query_fini(r); + + ecs_fini(world); +} + void Traversal_var_self_up_childof_inherited(void) { ecs_world_t *world = ecs_mini(); diff --git a/test/query/src/main.c b/test/query/src/main.c index aeec6141d..c4b5b83ae 100644 --- a/test/query/src/main.c +++ b/test/query/src/main.c @@ -1356,8 +1356,12 @@ void Traversal_self_up_all_owned(void); void Traversal_up_all_owned(void); void Traversal_this_self_up_childof_inherited(void); void Traversal_this_up_childof_inherited(void); +void Traversal_this_self_up_childof_inherited_override(void); +void Traversal_this_up_childof_inherited_override(void); void Traversal_this_written_self_up_childof_inherited(void); void Traversal_this_written_up_childof_inherited(void); +void Traversal_this_written_self_up_childof_inherited_override(void); +void Traversal_this_written_up_childof_inherited_override(void); void Traversal_var_self_up_childof_inherited(void); void Traversal_var_up_childof_inherited(void); void Traversal_var_written_self_up_childof_inherited(void); @@ -7353,6 +7357,14 @@ bake_test_case Traversal_testcases[] = { "this_up_childof_inherited", Traversal_this_up_childof_inherited }, + { + "this_self_up_childof_inherited_override", + Traversal_this_self_up_childof_inherited_override + }, + { + "this_up_childof_inherited_override", + Traversal_this_up_childof_inherited_override + }, { "this_written_self_up_childof_inherited", Traversal_this_written_self_up_childof_inherited @@ -7361,6 +7373,14 @@ bake_test_case Traversal_testcases[] = { "this_written_up_childof_inherited", Traversal_this_written_up_childof_inherited }, + { + "this_written_self_up_childof_inherited_override", + Traversal_this_written_self_up_childof_inherited_override + }, + { + "this_written_up_childof_inherited_override", + Traversal_this_written_up_childof_inherited_override + }, { "var_self_up_childof_inherited", Traversal_var_self_up_childof_inherited @@ -10236,7 +10256,7 @@ static bake_test_suite suites[] = { "Traversal", Traversal_setup, NULL, - 123, + 127, Traversal_testcases, 1, Traversal_params