Skip to content

Commit d14d753

Browse files
SanderMertensSanderMertens
authored
[core] Fix issue where hooks provided access to uninitialized components (#2032)
1 parent 63c85d1 commit d14d753

File tree

6 files changed

+58
-64
lines changed

6 files changed

+58
-64
lines changed

‎distr/flecs.c‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8014,10 +8014,11 @@ ecs_record_t* flecs_new_entity(
80148014
ecs_flags32_t evt_flags)
80158015
{
80168016
ecs_assert(r != NULL, ECS_INTERNAL_ERROR, NULL);
8017-
int32_t row = ecs_table_count(table);
8017+
r->table = &world->store.root;
8018+
flecs_table_append(world, table, entity, ctor, true);
8019+
int32_t row = ecs_table_count(table) - 1;
80188020
r->table = table;
80198021
r->row = ECS_ROW_TO_RECORD(row, r->row & ECS_ROW_FLAGS_MASK);
8020-
flecs_table_append(world, table, entity, ctor, true);
80218022

80228023
ecs_assert(ecs_table_count(table) > row, ECS_INTERNAL_ERROR, NULL);
80238024
flecs_actions_new(world, table, row, 1, diff, evt_flags, ctor, true);
@@ -8056,14 +8057,12 @@ void flecs_move_entity(
80568057
/* Invoke remove actions for removed components */
80578058
flecs_actions_move_remove(world, src_table, dst_table, src_row, 1, diff);
80588059

8059-
record->table = dst_table;
8060-
record->row = ECS_ROW_TO_RECORD(dst_row, record->row & ECS_ROW_FLAGS_MASK);
8061-
80628060
/* Copy entity & components from src_table to dst_table */
80638061
flecs_table_move(world, entity, entity, dst_table, dst_row,
80648062
src_table, src_row, ctor);
8065-
ecs_assert(record->table == dst_table, ECS_INTERNAL_ERROR, NULL);
8066-
8063+
record->table = dst_table;
8064+
record->row = ECS_ROW_TO_RECORD(dst_row, record->row & ECS_ROW_FLAGS_MASK);
8065+
80678066
flecs_table_delete(world, src_table, src_row, false);
80688067

80698068
flecs_actions_move_add(world, dst_table, src_table, dst_row, 1, diff,

‎src/entity.c‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -171,10 +171,11 @@ ecs_record_t* flecs_new_entity(
171171
ecs_flags32_t evt_flags)
172172
{
173173
ecs_assert(r != NULL, ECS_INTERNAL_ERROR, NULL);
174-
int32_t row = ecs_table_count(table);
174+
r->table = &world->store.root;
175+
flecs_table_append(world, table, entity, ctor, true);
176+
int32_t row = ecs_table_count(table) - 1;
175177
r->table = table;
176178
r->row = ECS_ROW_TO_RECORD(row, r->row & ECS_ROW_FLAGS_MASK);
177-
flecs_table_append(world, table, entity, ctor, true);
178179

179180
ecs_assert(ecs_table_count(table) > row, ECS_INTERNAL_ERROR, NULL);
180181
flecs_actions_new(world, table, row, 1, diff, evt_flags, ctor, true);
@@ -213,14 +214,12 @@ void flecs_move_entity(
213214
/* Invoke remove actions for removed components */
214215
flecs_actions_move_remove(world, src_table, dst_table, src_row, 1, diff);
215216

216-
record->table = dst_table;
217-
record->row = ECS_ROW_TO_RECORD(dst_row, record->row & ECS_ROW_FLAGS_MASK);
218-
219217
/* Copy entity & components from src_table to dst_table */
220218
flecs_table_move(world, entity, entity, dst_table, dst_row,
221219
src_table, src_row, ctor);
222-
ecs_assert(record->table == dst_table, ECS_INTERNAL_ERROR, NULL);
223-
220+
record->table = dst_table;
221+
record->row = ECS_ROW_TO_RECORD(dst_row, record->row & ECS_ROW_FLAGS_MASK);
222+
224223
flecs_table_delete(world, src_table, src_row, false);
225224

226225
flecs_actions_move_add(world, dst_table, src_table, dst_row, 1, diff,

‎test/core/project.json‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -982,7 +982,6 @@
982982
"delete_mixed_tree_3",
983983
"delete_mixed_tree_4",
984984
"delete_mixed_tree_5",
985-
"instantiate_parent_w_has_in_hook",
986985
"add_prefab_tag_after_hierarchy_creation",
987986
"defer_add_prefab_tag_after_hierarchy_creation",
988987
"add_prefab_tag_after_hierarchy_creation_2",
@@ -1713,7 +1712,8 @@
17131712
"has_in_on_add_hook_new",
17141713
"has_in_on_add_hook_move",
17151714
"get_in_on_add_hook_new",
1716-
"get_in_on_add_hook_move"
1715+
"get_in_on_add_hook_move",
1716+
"get_name_in_on_add_hook_move"
17171717
]
17181718
}, {
17191719
"id": "Pairs",

‎test/core/src/ComponentLifecycle.c‎

Lines changed: 37 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4418,7 +4418,7 @@ static void HasHook(ecs_iter_t *it) {
44184418

44194419
for (int i = 0; i < it->count; i ++) {
44204420
ecs_entity_t e = it->entities[i];
4421-
test_assert(ecs_has(world, e, Position));
4421+
test_assert(!ecs_has(world, e, Position));
44224422
has_hook_invoked ++;
44234423
}
44244424
}
@@ -4431,13 +4431,12 @@ static void GetHook(ecs_iter_t *it) {
44314431

44324432
for (int i = 0; i < it->count; i ++) {
44334433
ecs_entity_t e = it->entities[i];
4434-
test_assert(ecs_has(world, e, Position));
4435-
test_assert(ecs_get(world, e, Position) == &p[i]);
4434+
test_assert(!ecs_has(world, e, Position));
4435+
test_assert(ecs_get(world, e, Position) == NULL);
44364436
get_hook_invoked ++;
44374437
}
44384438
}
44394439

4440-
44414440
void ComponentLifecycle_has_in_on_add_hook_new(void) {
44424441
ecs_world_t *world = ecs_mini();
44434442

@@ -4513,3 +4512,37 @@ void ComponentLifecycle_get_in_on_add_hook_move(void) {
45134512

45144513
ecs_fini(world);
45154514
}
4515+
4516+
static int get_name_hook_invoked = 0;
4517+
4518+
static void GetNameHook(ecs_iter_t *it) {
4519+
ecs_world_t *world = it->world;
4520+
4521+
for (int i = 0; i < it->count; i ++) {
4522+
ecs_entity_t e = it->entities[i];
4523+
const char *name = ecs_get_name(world, e);
4524+
test_assert(name != NULL);
4525+
test_str(name, "TestEntity");
4526+
get_name_hook_invoked ++;
4527+
}
4528+
}
4529+
4530+
void ComponentLifecycle_get_name_in_on_add_hook_move(void) {
4531+
ecs_world_t *world = ecs_mini();
4532+
4533+
ECS_COMPONENT_DEFINE(world, Position);
4534+
4535+
ecs_set_hooks(world, Position, {
4536+
.on_add = GetNameHook
4537+
});
4538+
4539+
ecs_entity_t e = ecs_entity(world, { .name = "TestEntity" });
4540+
test_int(get_name_hook_invoked, 0);
4541+
4542+
ecs_add(world, e, Position);
4543+
test_int(get_name_hook_invoked, 1);
4544+
4545+
test_str(ecs_get_name(world, e), "TestEntity");
4546+
4547+
ecs_fini(world);
4548+
}

‎test/core/src/NonFragmentingChildOf.c‎

Lines changed: 0 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -6055,43 +6055,6 @@ void NonFragmentingChildOf_delete_mixed_tree_5(void) {
60556055
ecs_fini(world);
60566056
}
60576057

6058-
static int dummy_hook_invoked = 0;
6059-
6060-
static void DummyHook(ecs_iter_t *it) {
6061-
ecs_world_t *world = it->world;
6062-
Position *p = ecs_field(it, Position, 0);
6063-
6064-
for (int i = 0; i < it->count; i ++) {
6065-
ecs_entity_t e = it->entities[i];
6066-
test_assert(ecs_has(world, e, Position));
6067-
test_assert(ecs_get(world, e, Position) == &p[i]);
6068-
dummy_hook_invoked ++;
6069-
}
6070-
}
6071-
6072-
void NonFragmentingChildOf_instantiate_parent_w_has_in_hook(void) {
6073-
ecs_world_t* world = ecs_mini();
6074-
6075-
ECS_COMPONENT_DEFINE(world, Position);
6076-
6077-
ecs_set_hooks(world, Position, {
6078-
.on_add = DummyHook
6079-
});
6080-
6081-
ecs_entity_t prefab = ecs_new_w_id(world, EcsPrefab);
6082-
ecs_entity_t prefab_child = ecs_new_w_parent(world, prefab, NULL);
6083-
ecs_set(world, prefab_child, Position, {10, 20});
6084-
test_int(dummy_hook_invoked, 1);
6085-
6086-
ecs_new_w_pair(world, EcsIsA, prefab);
6087-
test_int(dummy_hook_invoked, 2);
6088-
6089-
ecs_new_w_pair(world, EcsIsA, prefab);
6090-
test_int(dummy_hook_invoked, 3);
6091-
6092-
ecs_fini(world);
6093-
}
6094-
60956058
void NonFragmentingChildOf_add_prefab_tag_after_hierarchy_creation(void) {
60966059
ecs_world_t* world = ecs_mini();
60976060

‎test/core/src/main.c‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -950,7 +950,6 @@ void NonFragmentingChildOf_delete_mixed_tree_2(void);
950950
void NonFragmentingChildOf_delete_mixed_tree_3(void);
951951
void NonFragmentingChildOf_delete_mixed_tree_4(void);
952952
void NonFragmentingChildOf_delete_mixed_tree_5(void);
953-
void NonFragmentingChildOf_instantiate_parent_w_has_in_hook(void);
954953
void NonFragmentingChildOf_add_prefab_tag_after_hierarchy_creation(void);
955954
void NonFragmentingChildOf_defer_add_prefab_tag_after_hierarchy_creation(void);
956955
void NonFragmentingChildOf_add_prefab_tag_after_hierarchy_creation_2(void);
@@ -1653,6 +1652,7 @@ void ComponentLifecycle_has_in_on_add_hook_new(void);
16531652
void ComponentLifecycle_has_in_on_add_hook_move(void);
16541653
void ComponentLifecycle_get_in_on_add_hook_new(void);
16551654
void ComponentLifecycle_get_in_on_add_hook_move(void);
1655+
void ComponentLifecycle_get_name_in_on_add_hook_move(void);
16561656

16571657
// Testsuite 'Pairs'
16581658
void Pairs_type_w_one_pair(void);
@@ -6938,10 +6938,6 @@ bake_test_case NonFragmentingChildOf_testcases[] = {
69386938
"delete_mixed_tree_5",
69396939
NonFragmentingChildOf_delete_mixed_tree_5
69406940
},
6941-
{
6942-
"instantiate_parent_w_has_in_hook",
6943-
NonFragmentingChildOf_instantiate_parent_w_has_in_hook
6944-
},
69456941
{
69466942
"add_prefab_tag_after_hierarchy_creation",
69476943
NonFragmentingChildOf_add_prefab_tag_after_hierarchy_creation
@@ -9655,6 +9651,10 @@ bake_test_case ComponentLifecycle_testcases[] = {
96559651
{
96569652
"get_in_on_add_hook_move",
96579653
ComponentLifecycle_get_in_on_add_hook_move
9654+
},
9655+
{
9656+
"get_name_in_on_add_hook_move",
9657+
ComponentLifecycle_get_name_in_on_add_hook_move
96589658
}
96599659
};
96609660

@@ -15965,7 +15965,7 @@ static bake_test_suite suites[] = {
1596515965
"NonFragmentingChildOf",
1596615966
NULL,
1596715967
NULL,
15968-
239,
15968+
238,
1596915969
NonFragmentingChildOf_testcases
1597015970
},
1597115971
{
@@ -16063,7 +16063,7 @@ static bake_test_suite suites[] = {
1606316063
"ComponentLifecycle",
1606416064
ComponentLifecycle_setup,
1606516065
NULL,
16066-
138,
16066+
139,
1606716067
ComponentLifecycle_testcases
1606816068
},
1606916069
{

0 commit comments

Comments
 (0)