Skip to content

Commit

Permalink
Fix issue where up traversal could return duplicate result
Browse files Browse the repository at this point in the history
  • Loading branch information
@SanderMertens
SanderMertens committed Sep 4, 2024
1 parent 2fbe476 commit 34dc148
Show file tree
Hide file tree
Showing 6 changed files with 193 additions and 37 deletions.
23 changes: 5 additions & 18 deletions distr/flecs.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 */

Expand Down Expand Up @@ -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 ++) {
Expand Down
18 changes: 0 additions & 18 deletions src/query/engine/engine.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 */

Expand Down
5 changes: 5 additions & 0 deletions src/query/engine/trav_down_cache.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 ++) {
Expand Down
4 changes: 4 additions & 0 deletions test/query/project.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
158 changes: 158 additions & 0 deletions test/query/src/Traversal.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down Expand Up @@ -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();

Expand Down
22 changes: 21 additions & 1 deletion test/query/src/main.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -10236,7 +10256,7 @@ static bake_test_suite suites[] = {
"Traversal",
Traversal_setup,
NULL,
123,
127,
Traversal_testcases,
1,
Traversal_params
Expand Down

0 comments on commit 34dc148

Please sign in to comment.