From 3a11369c49174a61e14b1da0cc79dd200aec2368 Mon Sep 17 00:00:00 2001 From: Sander Mertens Date: Mon, 22 Aug 2022 10:08:32 +0200 Subject: [PATCH] #788 call on_add hook for emplace --- flecs.c | 43 +++++----- src/bootstrap.c | 2 +- src/entity.c | 5 +- src/table.c | 33 ++++---- src/table.h | 3 +- test/api/project.json | 3 + test/api/src/ComponentLifecycle.c | 100 ++++++++++++++++++++---- test/api/src/main.c | 17 +++- test/cpp_api/project.json | 4 +- test/cpp_api/src/ComponentLifecycle.cpp | 32 ++++++++ test/cpp_api/src/main.cpp | 12 ++- 11 files changed, 197 insertions(+), 57 deletions(-) diff --git a/flecs.c b/flecs.c index 1bfb810dd..5a98d3cb3 100644 --- a/flecs.c +++ b/flecs.c @@ -1269,7 +1269,8 @@ int32_t flecs_table_append( ecs_table_t *table, ecs_entity_t entity, ecs_record_t *record, - bool construct); + bool construct, + bool on_add); /* Delete an entity from the table. */ void flecs_table_delete( @@ -3013,11 +3014,14 @@ void add_component( ecs_entity_t *entities, ecs_id_t id, int32_t row, - int32_t count) + int32_t count, + bool construct) { ecs_assert(ti != NULL, ECS_INTERNAL_ERROR, NULL); - ctor_component(ti, column, row, count); + if (construct) { + ctor_component(ti, column, row, count); + } ecs_iter_action_t on_add = ti->hooks.on_add; if (on_add) { @@ -3745,7 +3749,8 @@ int32_t flecs_table_append( ecs_table_t *table, ecs_entity_t entity, ecs_record_t *record, - bool construct) + bool construct, + bool on_add) { ecs_assert(table != NULL, ECS_INTERNAL_ERROR, NULL); ecs_assert(!table->lock, ECS_LOCKED_STORAGE, NULL); @@ -3802,9 +3807,9 @@ int32_t flecs_table_append( ecs_type_info_t *ti = type_info[i]; grow_column(column, ti, 1, size, construct); - ecs_iter_action_t on_add; - if (construct && (on_add = ti->hooks.on_add)) { - on_component_callback(world, table, on_add, EcsOnAdd, column, + ecs_iter_action_t on_add_hook; + if (on_add && (on_add_hook = ti->hooks.on_add)) { + on_component_callback(world, table, on_add_hook, EcsOnAdd, column, &entities[count], table->storage_ids[i], count, 1, ti); } @@ -4110,11 +4115,9 @@ void flecs_table_move( } } else { if (dst_id < src_id) { - if (construct) { - add_component(world, dst_table, dst_type_info[i_new], - &dst_columns[i_new], &dst_entity, dst_id, - dst_index, 1); - } + add_component(world, dst_table, dst_type_info[i_new], + &dst_columns[i_new], &dst_entity, dst_id, + dst_index, 1, construct); } else { remove_component(world, src_table, src_type_info[i_old], &src_columns[i_old], &src_entity, src_id, @@ -4126,11 +4129,10 @@ void flecs_table_move( i_old += dst_id >= src_id; } - if (construct) { - for (; (i_new < dst_column_count); i_new ++) { - add_component(world, dst_table, dst_type_info[i_new], - &dst_columns[i_new], &dst_entity, dst_ids[i_new], dst_index, 1); - } + for (; (i_new < dst_column_count); i_new ++) { + add_component(world, dst_table, dst_type_info[i_new], + &dst_columns[i_new], &dst_entity, dst_ids[i_new], dst_index, 1, + construct); } for (; (i_old < src_column_count); i_old ++) { @@ -6055,7 +6057,8 @@ ecs_record_t* new_entity( record = flecs_entities_ensure(world, entity); } - new_row = flecs_table_append(world, new_table, entity, record, construct); + new_row = flecs_table_append(world, new_table, entity, record, + construct, true); record->table = new_table; record->row = ECS_ROW_TO_RECORD(new_row, record->row & ECS_ROW_FLAGS_MASK); @@ -6098,7 +6101,7 @@ void move_entity( ecs_assert(record == flecs_entities_get(world, entity), ECS_INTERNAL_ERROR, NULL); int32_t dst_row = flecs_table_append(world, dst_table, entity, - record, false); + record, false, false); /* Copy entity & components from src_table to dst_table */ if (src_table->type.count) { @@ -48598,7 +48601,7 @@ void _bootstrap_component( ecs_record_t *record = flecs_entities_ensure(world, entity); record->table = table; - int32_t index = flecs_table_append(world, table, entity, record, false); + int32_t index = flecs_table_append(world, table, entity, record, false, false); record->row = ECS_ROW_TO_RECORD(index, 0); EcsComponent *component = ecs_storage_first(&columns[0]); diff --git a/src/bootstrap.c b/src/bootstrap.c index 6da45a827..fc561871d 100644 --- a/src/bootstrap.c +++ b/src/bootstrap.c @@ -552,7 +552,7 @@ void _bootstrap_component( ecs_record_t *record = flecs_entities_ensure(world, entity); record->table = table; - int32_t index = flecs_table_append(world, table, entity, record, false); + int32_t index = flecs_table_append(world, table, entity, record, false, false); record->row = ECS_ROW_TO_RECORD(index, 0); EcsComponent *component = ecs_storage_first(&columns[0]); diff --git a/src/entity.c b/src/entity.c index 7c8022b48..ea4604984 100644 --- a/src/entity.c +++ b/src/entity.c @@ -826,7 +826,8 @@ ecs_record_t* new_entity( record = flecs_entities_ensure(world, entity); } - new_row = flecs_table_append(world, new_table, entity, record, construct); + new_row = flecs_table_append(world, new_table, entity, record, + construct, true); record->table = new_table; record->row = ECS_ROW_TO_RECORD(new_row, record->row & ECS_ROW_FLAGS_MASK); @@ -869,7 +870,7 @@ void move_entity( ecs_assert(record == flecs_entities_get(world, entity), ECS_INTERNAL_ERROR, NULL); int32_t dst_row = flecs_table_append(world, dst_table, entity, - record, false); + record, false, false); /* Copy entity & components from src_table to dst_table */ if (src_table->type.count) { diff --git a/src/table.c b/src/table.c index 3bf3a9b6b..eb16ce62d 100644 --- a/src/table.c +++ b/src/table.c @@ -734,11 +734,14 @@ void add_component( ecs_entity_t *entities, ecs_id_t id, int32_t row, - int32_t count) + int32_t count, + bool construct) { ecs_assert(ti != NULL, ECS_INTERNAL_ERROR, NULL); - ctor_component(ti, column, row, count); + if (construct) { + ctor_component(ti, column, row, count); + } ecs_iter_action_t on_add = ti->hooks.on_add; if (on_add) { @@ -1466,7 +1469,8 @@ int32_t flecs_table_append( ecs_table_t *table, ecs_entity_t entity, ecs_record_t *record, - bool construct) + bool construct, + bool on_add) { ecs_assert(table != NULL, ECS_INTERNAL_ERROR, NULL); ecs_assert(!table->lock, ECS_LOCKED_STORAGE, NULL); @@ -1523,9 +1527,9 @@ int32_t flecs_table_append( ecs_type_info_t *ti = type_info[i]; grow_column(column, ti, 1, size, construct); - ecs_iter_action_t on_add; - if (construct && (on_add = ti->hooks.on_add)) { - on_component_callback(world, table, on_add, EcsOnAdd, column, + ecs_iter_action_t on_add_hook; + if (on_add && (on_add_hook = ti->hooks.on_add)) { + on_component_callback(world, table, on_add_hook, EcsOnAdd, column, &entities[count], table->storage_ids[i], count, 1, ti); } @@ -1831,11 +1835,9 @@ void flecs_table_move( } } else { if (dst_id < src_id) { - if (construct) { - add_component(world, dst_table, dst_type_info[i_new], - &dst_columns[i_new], &dst_entity, dst_id, - dst_index, 1); - } + add_component(world, dst_table, dst_type_info[i_new], + &dst_columns[i_new], &dst_entity, dst_id, + dst_index, 1, construct); } else { remove_component(world, src_table, src_type_info[i_old], &src_columns[i_old], &src_entity, src_id, @@ -1847,11 +1849,10 @@ void flecs_table_move( i_old += dst_id >= src_id; } - if (construct) { - for (; (i_new < dst_column_count); i_new ++) { - add_component(world, dst_table, dst_type_info[i_new], - &dst_columns[i_new], &dst_entity, dst_ids[i_new], dst_index, 1); - } + for (; (i_new < dst_column_count); i_new ++) { + add_component(world, dst_table, dst_type_info[i_new], + &dst_columns[i_new], &dst_entity, dst_ids[i_new], dst_index, 1, + construct); } for (; (i_old < src_column_count); i_old ++) { diff --git a/src/table.h b/src/table.h index 96aaf59f8..ab3b4e616 100644 --- a/src/table.h +++ b/src/table.h @@ -60,7 +60,8 @@ int32_t flecs_table_append( ecs_table_t *table, ecs_entity_t entity, ecs_record_t *record, - bool construct); + bool construct, + bool on_add); /* Delete an entity from the table. */ void flecs_table_delete( diff --git a/test/api/project.json b/test/api/project.json index bc7a68c79..bde93785d 100644 --- a/test/api/project.json +++ b/test/api/project.json @@ -816,6 +816,9 @@ "valid_entity_in_dtor_after_delete", "ctor_w_emplace", "ctor_w_emplace_defer", + "on_add_w_emplace", + "on_add_w_emplace_existing", + "on_add_w_emplace_defer", "dtor_on_fini", "valid_type_in_dtor_on_fini", "valid_other_type_of_entity_in_dtor_on_fini", diff --git a/test/api/src/ComponentLifecycle.c b/test/api/src/ComponentLifecycle.c index 94ed454fc..98c992e92 100644 --- a/test/api/src/ComponentLifecycle.c +++ b/test/api/src/ComponentLifecycle.c @@ -536,6 +536,24 @@ ECS_MOVE(Position, dst, src, { move_position ++; }) +static int on_add_position = 0; + +static void ecs_on_add(Position)(ecs_iter_t *it) { + test_assert(it->count >= 1); + test_assert(it->event == EcsOnAdd); + + Position *p = ecs_field(it, Position, 1); + for (int i = 0; i < it->count; i ++) { + on_add_position ++; + test_int(p[i].x, 0); + test_int(p[i].y, 0); + } +} + +static void on_add_position_emplace(ecs_iter_t *it) { + on_add_position += it->count; +} + /* Velocity */ static int ctor_velocity = 0; @@ -1327,6 +1345,74 @@ void ComponentLifecycle_ctor_w_emplace_defer() { ecs_fini(world); } +void ComponentLifecycle_on_add_w_emplace() { + ecs_world_t *world = ecs_mini(); + + ECS_COMPONENT(world, Position); + + ecs_set_hooks(world, Position, { + .on_add = on_add_position_emplace + }); + + ecs_entity_t e = ecs_new_id(world); + test_assert(e != 0); + + test_int(on_add_position, 0); + const Position *ptr = ecs_emplace(world, e, Position); + test_assert(ptr != NULL); + test_int(on_add_position, 1); + + ecs_fini(world); +} + +void ComponentLifecycle_on_add_w_emplace_existing() { + ecs_world_t *world = ecs_mini(); + + ECS_COMPONENT(world, Position); + ECS_COMPONENT(world, Velocity); + + ecs_set_hooks(world, Position, { + .ctor = ecs_ctor(Position), + .on_add = on_add_position_emplace + }); + + ecs_entity_t e = ecs_new(world, Velocity); + test_assert(e != 0); + + test_int(ctor_position, 0); + test_int(on_add_position, 0); + const Position *ptr = ecs_emplace(world, e, Position); + test_assert(ptr != NULL); + test_int(ctor_position, 0); + test_int(on_add_position, 1); + + ecs_fini(world); +} + +void ComponentLifecycle_on_add_w_emplace_defer() { + ecs_world_t *world = ecs_mini(); + + ECS_COMPONENT(world, Position); + + ecs_set_hooks(world, Position, { + .on_add = ecs_on_add(Position), + }); + + ecs_entity_t e = ecs_new_id(world); + test_assert(e != 0); + + ecs_defer_begin(world); + test_int(on_add_position, 0); + const Position *ptr = ecs_emplace(world, e, Position); + test_assert(ptr != NULL); + test_int(on_add_position, 0); + ecs_defer_end(world); + + test_int(on_add_position, 1); + + ecs_fini(world); +} + void ComponentLifecycle_dtor_on_fini() { ecs_world_t *world = ecs_mini(); @@ -1798,20 +1884,6 @@ void ComponentLifecycle_on_set_after_set() { ecs_fini(world); } -static int on_add_position = 0; - -static void ecs_on_add(Position)(ecs_iter_t *it) { - test_assert(it->count >= 1); - test_assert(it->event == EcsOnAdd); - - Position *p = ecs_field(it, Position, 1); - for (int i = 0; i < it->count; i ++) { - on_add_position ++; - test_int(p[i].x, 0); - test_int(p[i].y, 0); - } -} - void ComponentLifecycle_on_add_after_new() { ecs_world_t *world = ecs_mini(); diff --git a/test/api/src/main.c b/test/api/src/main.c index 3a673188c..e80809f56 100644 --- a/test/api/src/main.c +++ b/test/api/src/main.c @@ -765,6 +765,9 @@ void ComponentLifecycle_set_lifecycle_after_trigger(void); void ComponentLifecycle_valid_entity_in_dtor_after_delete(void); void ComponentLifecycle_ctor_w_emplace(void); void ComponentLifecycle_ctor_w_emplace_defer(void); +void ComponentLifecycle_on_add_w_emplace(void); +void ComponentLifecycle_on_add_w_emplace_existing(void); +void ComponentLifecycle_on_add_w_emplace_defer(void); void ComponentLifecycle_dtor_on_fini(void); void ComponentLifecycle_valid_type_in_dtor_on_fini(void); void ComponentLifecycle_valid_other_type_of_entity_in_dtor_on_fini(void); @@ -5045,6 +5048,18 @@ bake_test_case ComponentLifecycle_testcases[] = { "ctor_w_emplace_defer", ComponentLifecycle_ctor_w_emplace_defer }, + { + "on_add_w_emplace", + ComponentLifecycle_on_add_w_emplace + }, + { + "on_add_w_emplace_existing", + ComponentLifecycle_on_add_w_emplace_existing + }, + { + "on_add_w_emplace_defer", + ComponentLifecycle_on_add_w_emplace_defer + }, { "dtor_on_fini", ComponentLifecycle_dtor_on_fini @@ -10649,7 +10664,7 @@ static bake_test_suite suites[] = { "ComponentLifecycle", ComponentLifecycle_setup, NULL, - 69, + 72, ComponentLifecycle_testcases }, { diff --git a/test/cpp_api/project.json b/test/cpp_api/project.json index cfd675d65..64d4f2bac 100644 --- a/test/cpp_api/project.json +++ b/test/cpp_api/project.json @@ -816,7 +816,9 @@ "chained_hooks", "ctor_w_2_worlds", "ctor_w_2_worlds_explicit_registration", - "defer_emplace" + "defer_emplace", + "emplace_w_on_add", + "emplace_w_on_add_existing" ] }, { "id": "Refs", diff --git a/test/cpp_api/src/ComponentLifecycle.cpp b/test/cpp_api/src/ComponentLifecycle.cpp index db736438d..330cfc2fc 100644 --- a/test/cpp_api/src/ComponentLifecycle.cpp +++ b/test/cpp_api/src/ComponentLifecycle.cpp @@ -1299,3 +1299,35 @@ void ComponentLifecycle_defer_emplace() { test_int(p->x, 10); test_int(p->y, 20); } + +void ComponentLifecycle_emplace_w_on_add() { + flecs::world ecs; + + flecs::entity e1 = ecs.entity(); + + int on_add = 0; + ecs.component() + .on_add([&](flecs::entity e, Position&) { + on_add = true; + test_assert(e == e1); + }); + + e1.emplace(); + test_int(on_add, 1); +} + +void ComponentLifecycle_emplace_w_on_add_existing() { + flecs::world ecs; + + flecs::entity e1 = ecs.entity().add(); + + int on_add = 0; + ecs.component() + .on_add([&](flecs::entity e, Position&) { + on_add = true; + test_assert(e == e1); + }); + + e1.emplace(); + test_int(on_add, 1); +} diff --git a/test/cpp_api/src/main.cpp b/test/cpp_api/src/main.cpp index 2c782c6f7..e21b858e2 100644 --- a/test/cpp_api/src/main.cpp +++ b/test/cpp_api/src/main.cpp @@ -781,6 +781,8 @@ void ComponentLifecycle_chained_hooks(void); void ComponentLifecycle_ctor_w_2_worlds(void); void ComponentLifecycle_ctor_w_2_worlds_explicit_registration(void); void ComponentLifecycle_defer_emplace(void); +void ComponentLifecycle_emplace_w_on_add(void); +void ComponentLifecycle_emplace_w_on_add_existing(void); // Testsuite 'Refs' void Refs_get_ref_by_ptr(void); @@ -4009,6 +4011,14 @@ bake_test_case ComponentLifecycle_testcases[] = { { "defer_emplace", ComponentLifecycle_defer_emplace + }, + { + "emplace_w_on_add", + ComponentLifecycle_emplace_w_on_add + }, + { + "emplace_w_on_add_existing", + ComponentLifecycle_emplace_w_on_add_existing } }; @@ -4919,7 +4929,7 @@ static bake_test_suite suites[] = { "ComponentLifecycle", NULL, NULL, - 50, + 52, ComponentLifecycle_testcases }, {