Skip to content

Commit 1860547

Browse files
SanderMertensSanderMertens
committed
[core] Fix component inheritance bugs in traversal, query validator and script vars
1 parent 2958db0 commit 1860547

15 files changed

Lines changed: 285 additions & 28 deletions

File tree

‎distr/flecs.c‎

Lines changed: 39 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20257,8 +20257,14 @@ int32_t flecs_relation_depth_walk(
2025720257
return 0;
2025820258
}
2025920259

20260-
int32_t i = tr->index, end = i + tr->count;
20261-
for (; i != end; i ++) {
20260+
int32_t i = tr->index, remaining = tr->count;
20261+
for (; remaining; i ++) {
20262+
i = flecs_table_offset_search_w_inherited(world, table, i, cr->id, NULL);
20263+
if (i == -1) {
20264+
break;
20265+
}
20266+
remaining --;
20267+
2026220268
ecs_entity_t o = ecs_pair_second(world, table->type.array[i]);
2026320269
if (!o) {
2026420270
/* Rare, but can happen during cleanup when an intermediate table is
@@ -20273,7 +20279,7 @@ int32_t flecs_relation_depth_walk(
2027320279
if (!ot) {
2027420280
continue;
2027520281
}
20276-
20282+
2027720283
ecs_assert(ot != first, ECS_CYCLE_DETECTED, NULL);
2027820284
int32_t cur = flecs_relation_depth_walk(world, cr, first, ot);
2027920285
if (cur > result) {
@@ -39305,7 +39311,21 @@ bool flecs_query_finalize_simple(
3930539311
}
3930639312
}
3930739313

39308-
if (flecs_components_get(world, ecs_pair(EcsIsA, first)) != NULL) {
39314+
bool first_inherited =
39315+
flecs_components_get(world, ecs_pair(EcsIsA, first)) != NULL;
39316+
if (!first_inherited) {
39317+
first_inherited = (cr_flags & EcsIdInheritable) != 0;
39318+
}
39319+
if (!first_inherited) {
39320+
ecs_record_t *first_record = flecs_entities_get(world, first);
39321+
ecs_table_t *first_table = first_record ? first_record->table : NULL;
39322+
if (first_table && (first_table->flags & EcsTableHasIsA)) {
39323+
first_inherited = !ecs_table_has_id(
39324+
world, first_table, EcsFinal);
39325+
}
39326+
}
39327+
39328+
if (first_inherited) {
3930939329
term->flags_ |= EcsTermIdInherited;
3931039330
q->flags |= EcsQueryHasComponentInheritance;
3931139331
trivial = false;
@@ -70461,12 +70481,16 @@ void ecs_script_vars_from_iter(
7046170481
continue;
7046270482
}
7046370483

70464-
void *ptr = ecs_field_w_size(it, flecs_itosize(size), i);
70484+
void *ptr = ecs_base_field_w_size(it, flecs_itosize(size), i);
7046570485
if (!ptr) {
7046670486
continue;
7046770487
}
7046870488

70469-
ptr = ECS_OFFSET(ptr, offset * size);
70489+
if (!it->sources[i]) {
70490+
ecs_size_t stride = flecs_uto(ecs_size_t,
70491+
ecs_field_stride(it, i));
70492+
ptr = ECS_OFFSET(ptr, offset * stride);
70493+
}
7047070494

7047170495
const char *name = flecs_script_iter_field_names[i];
7047270496
ecs_script_var_t *var = ecs_script_vars_lookup(vars, name);
@@ -90082,8 +90106,15 @@ ecs_trav_up_t* flecs_query_get_up_cache(
9008290106
return NULL; /* Table doesn't have the relationship */
9008390107
}
9008490108

90085-
int32_t i = tr->index, end = i + tr->count;
90086-
for (; i < end; i ++) {
90109+
int32_t i = tr->index, remaining = tr->count;
90110+
for (; remaining; i ++) {
90111+
i = flecs_table_offset_search_w_inherited(
90112+
world, table, i, cr_trav->id, NULL);
90113+
if (i == -1) {
90114+
break;
90115+
}
90116+
remaining --;
90117+
9008790118
ecs_id_t id = table->type.array[i];
9008890119
ecs_entity_t tgt = ECS_PAIR_SECOND(id);
9008990120
ecs_trav_up_t *result = &cache->up;

‎distr/flecs_no_addons.c‎

Lines changed: 33 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20105,8 +20105,14 @@ int32_t flecs_relation_depth_walk(
2010520105
return 0;
2010620106
}
2010720107

20108-
int32_t i = tr->index, end = i + tr->count;
20109-
for (; i != end; i ++) {
20108+
int32_t i = tr->index, remaining = tr->count;
20109+
for (; remaining; i ++) {
20110+
i = flecs_table_offset_search_w_inherited(world, table, i, cr->id, NULL);
20111+
if (i == -1) {
20112+
break;
20113+
}
20114+
remaining --;
20115+
2011020116
ecs_entity_t o = ecs_pair_second(world, table->type.array[i]);
2011120117
if (!o) {
2011220118
/* Rare, but can happen during cleanup when an intermediate table is
@@ -20121,7 +20127,7 @@ int32_t flecs_relation_depth_walk(
2012120127
if (!ot) {
2012220128
continue;
2012320129
}
20124-
20130+
2012520131
ecs_assert(ot != first, ECS_CYCLE_DETECTED, NULL);
2012620132
int32_t cur = flecs_relation_depth_walk(world, cr, first, ot);
2012720133
if (cur > result) {
@@ -31150,7 +31156,21 @@ bool flecs_query_finalize_simple(
3115031156
}
3115131157
}
3115231158

31153-
if (flecs_components_get(world, ecs_pair(EcsIsA, first)) != NULL) {
31159+
bool first_inherited =
31160+
flecs_components_get(world, ecs_pair(EcsIsA, first)) != NULL;
31161+
if (!first_inherited) {
31162+
first_inherited = (cr_flags & EcsIdInheritable) != 0;
31163+
}
31164+
if (!first_inherited) {
31165+
ecs_record_t *first_record = flecs_entities_get(world, first);
31166+
ecs_table_t *first_table = first_record ? first_record->table : NULL;
31167+
if (first_table && (first_table->flags & EcsTableHasIsA)) {
31168+
first_inherited = !ecs_table_has_id(
31169+
world, first_table, EcsFinal);
31170+
}
31171+
}
31172+
31173+
if (first_inherited) {
3115431174
term->flags_ |= EcsTermIdInherited;
3115531175
q->flags |= EcsQueryHasComponentInheritance;
3115631176
trivial = false;
@@ -52556,8 +52576,15 @@ ecs_trav_up_t* flecs_query_get_up_cache(
5255652576
return NULL; /* Table doesn't have the relationship */
5255752577
}
5255852578

52559-
int32_t i = tr->index, end = i + tr->count;
52560-
for (; i < end; i ++) {
52579+
int32_t i = tr->index, remaining = tr->count;
52580+
for (; remaining; i ++) {
52581+
i = flecs_table_offset_search_w_inherited(
52582+
world, table, i, cr_trav->id, NULL);
52583+
if (i == -1) {
52584+
break;
52585+
}
52586+
remaining --;
52587+
5256152588
ecs_id_t id = table->type.array[i];
5256252589
ecs_entity_t tgt = ECS_PAIR_SECOND(id);
5256352590
ecs_trav_up_t *result = &cache->up;

‎src/addons/script/vars.c‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -287,12 +287,16 @@ void ecs_script_vars_from_iter(
287287
continue;
288288
}
289289

290-
void *ptr = ecs_field_w_size(it, flecs_itosize(size), i);
290+
void *ptr = ecs_base_field_w_size(it, flecs_itosize(size), i);
291291
if (!ptr) {
292292
continue;
293293
}
294294

295-
ptr = ECS_OFFSET(ptr, offset * size);
295+
if (!it->sources[i]) {
296+
ecs_size_t stride = flecs_uto(ecs_size_t,
297+
ecs_field_stride(it, i));
298+
ptr = ECS_OFFSET(ptr, offset * stride);
299+
}
296300

297301
const char *name = flecs_script_iter_field_names[i];
298302
ecs_script_var_t *var = ecs_script_vars_lookup(vars, name);

‎src/query/engine/trav_up_cache.c‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -279,8 +279,15 @@ ecs_trav_up_t* flecs_query_get_up_cache(
279279
return NULL; /* Table doesn't have the relationship */
280280
}
281281

282-
int32_t i = tr->index, end = i + tr->count;
283-
for (; i < end; i ++) {
282+
int32_t i = tr->index, remaining = tr->count;
283+
for (; remaining; i ++) {
284+
i = flecs_table_offset_search_w_inherited(
285+
world, table, i, cr_trav->id, NULL);
286+
if (i == -1) {
287+
break;
288+
}
289+
remaining --;
290+
284291
ecs_id_t id = table->type.array[i];
285292
ecs_entity_t tgt = ECS_PAIR_SECOND(id);
286293
ecs_trav_up_t *result = &cache->up;

‎src/query/validator.c‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1831,7 +1831,21 @@ bool flecs_query_finalize_simple(
18311831
}
18321832
}
18331833

1834-
if (flecs_components_get(world, ecs_pair(EcsIsA, first)) != NULL) {
1834+
bool first_inherited =
1835+
flecs_components_get(world, ecs_pair(EcsIsA, first)) != NULL;
1836+
if (!first_inherited) {
1837+
first_inherited = (cr_flags & EcsIdInheritable) != 0;
1838+
}
1839+
if (!first_inherited) {
1840+
ecs_record_t *first_record = flecs_entities_get(world, first);
1841+
ecs_table_t *first_table = first_record ? first_record->table : NULL;
1842+
if (first_table && (first_table->flags & EcsTableHasIsA)) {
1843+
first_inherited = !ecs_table_has_id(
1844+
world, first_table, EcsFinal);
1845+
}
1846+
}
1847+
1848+
if (first_inherited) {
18351849
term->flags_ |= EcsTermIdInherited;
18361850
q->flags |= EcsQueryHasComponentInheritance;
18371851
trivial = false;

‎src/search.c‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -480,8 +480,14 @@ int32_t flecs_relation_depth_walk(
480480
return 0;
481481
}
482482

483-
int32_t i = tr->index, end = i + tr->count;
484-
for (; i != end; i ++) {
483+
int32_t i = tr->index, remaining = tr->count;
484+
for (; remaining; i ++) {
485+
i = flecs_table_offset_search_w_inherited(world, table, i, cr->id, NULL);
486+
if (i == -1) {
487+
break;
488+
}
489+
remaining --;
490+
485491
ecs_entity_t o = ecs_pair_second(world, table->type.array[i]);
486492
if (!o) {
487493
/* Rare, but can happen during cleanup when an intermediate table is
@@ -496,7 +502,7 @@ int32_t flecs_relation_depth_walk(
496502
if (!ot) {
497503
continue;
498504
}
499-
505+
500506
ecs_assert(ot != first, ECS_CYCLE_DETECTED, NULL);
501507
int32_t cur = flecs_relation_depth_walk(world, cr, first, ot);
502508
if (cur > result) {

‎test/core/project.json‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -409,7 +409,8 @@
409409
"pair_rel_remove_isa",
410410
"pair_rel_multi",
411411
"pair_rel_wildcard",
412-
"pair_rel_wildcard_unrelated"
412+
"pair_rel_wildcard_unrelated",
413+
"depth_w_derived_pairs_non_contiguous"
413414
]
414415
}, {
415416
"id": "Search",

‎test/core/src/ComponentInheritance.c‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1753,3 +1753,32 @@ void ComponentInheritance_pair_rel_wildcard_unrelated(void) {
17531753

17541754
ecs_fini(world);
17551755
}
1756+
1757+
void ComponentInheritance_depth_w_derived_pairs_non_contiguous(void) {
1758+
ecs_world_t *world = ecs_mini();
1759+
1760+
ECS_TAG(world, Rel);
1761+
ECS_TAG(world, DerivedA);
1762+
ECS_TAG(world, Other);
1763+
ECS_TAG(world, DerivedB);
1764+
1765+
ecs_add_id(world, Rel, EcsAcyclic);
1766+
ecs_add_id(world, Rel, EcsTraversable);
1767+
ecs_add_pair(world, DerivedA, EcsIsA, Rel);
1768+
ecs_add_pair(world, DerivedB, EcsIsA, Rel);
1769+
1770+
ecs_entity_t pa = ecs_new(world);
1771+
ecs_entity_t po = ecs_new(world);
1772+
ecs_entity_t pb = ecs_new(world);
1773+
ecs_entity_t pb_parent = ecs_new(world);
1774+
ecs_add_pair(world, pb, Rel, pb_parent);
1775+
1776+
ecs_entity_t e = ecs_new(world);
1777+
ecs_add_pair(world, e, DerivedA, pa);
1778+
ecs_add_pair(world, e, Other, po);
1779+
ecs_add_pair(world, e, DerivedB, pb);
1780+
1781+
test_int(2, ecs_get_depth(world, e, Rel));
1782+
1783+
ecs_fini(world);
1784+
}

‎test/core/src/main.c‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -397,6 +397,7 @@ void ComponentInheritance_pair_rel_remove_isa(void);
397397
void ComponentInheritance_pair_rel_multi(void);
398398
void ComponentInheritance_pair_rel_wildcard(void);
399399
void ComponentInheritance_pair_rel_wildcard_unrelated(void);
400+
void ComponentInheritance_depth_w_derived_pairs_non_contiguous(void);
400401

401402
// Testsuite 'Search'
402403
void Search_search(void);
@@ -4973,6 +4974,10 @@ bake_test_case ComponentInheritance_testcases[] = {
49734974
{
49744975
"pair_rel_wildcard_unrelated",
49754976
ComponentInheritance_pair_rel_wildcard_unrelated
4977+
},
4978+
{
4979+
"depth_w_derived_pairs_non_contiguous",
4980+
ComponentInheritance_depth_w_derived_pairs_non_contiguous
49764981
}
49774982
};
49784983

@@ -16902,7 +16907,7 @@ static bake_test_suite suites[] = {
1690216907
"ComponentInheritance",
1690316908
NULL,
1690416909
NULL,
16905-
80,
16910+
81,
1690616911
ComponentInheritance_testcases
1690716912
},
1690816913
{

‎test/query/project.json‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1449,7 +1449,9 @@
14491449
"pair_wildcard_multi_rel",
14501450
"pair_wildcard_w_owned_base_pair",
14511451
"cached_match_derived_table_added_after",
1452-
"cached_unmatch_derived_table_on_delete"
1452+
"cached_unmatch_derived_table_on_delete",
1453+
"inheritable_trait_terms_query_before_derived",
1454+
"query_base_and_derived_same_table"
14531455
]
14541456
}, {
14551457
"id": "BuiltinPredicates",

0 commit comments

Comments
 (0)