From 8871d5773fb37481480a7aaafff2c947aa4bf6c6 Mon Sep 17 00:00:00 2001 From: Sander Mertens Date: Tue, 17 Sep 2024 22:21:09 -0700 Subject: [PATCH] #1359 Fix issues with hooks for sparse components --- distr/flecs.c | 6 +- src/entity.c | 2 + src/iter.c | 2 +- src/query/validator.c | 2 +- test/core/project.json | 1 + test/core/src/Sparse.c | 47 ++++- test/core/src/main.c | 7 +- test/cpp/project.json | 9 + test/cpp/src/ComponentLifecycle.cpp | 286 +++++++++++++++++++++++++++- test/cpp/src/main.cpp | 47 ++++- 10 files changed, 398 insertions(+), 11 deletions(-) diff --git a/distr/flecs.c b/distr/flecs.c index 7518fe78b..e5d488ada 100644 --- a/distr/flecs.c +++ b/distr/flecs.c @@ -6251,6 +6251,7 @@ void flecs_invoke_hook( it.table = table; it.trs[0] = tr; it.row_fields = !!(((ecs_id_record_t*)tr->hdr.cache)->flags & EcsIdIsSparse); + it.ref_fields = it.row_fields; it.sizes = ECS_CONST_CAST(ecs_size_t*, &ti->size); it.ids[0] = id; it.event = event; @@ -6260,6 +6261,7 @@ void flecs_invoke_hook( it.count = count; it.offset = row; it.flags = EcsIterIsValid; + hook(&it); ecs_iter_fini(&it); @@ -11283,7 +11285,7 @@ void* ecs_field_at_w_size( ecs_entity_t src = it->sources[index]; if (!src) { - src = ecs_table_entities(it->table)[row]; + src = ecs_table_entities(it->table)[row + it->offset]; } return flecs_sparse_get_any(idr->sparse, flecs_uto(int32_t, size), src); @@ -34748,7 +34750,7 @@ bool flecs_query_finalize_simple( if (idr->flags & EcsIdIsSparse) { term->flags_ |= EcsTermIsSparse; cacheable = false; trivial = false; - q->row_fields |= (1llu << i); + q->row_fields |= flecs_uto(uint32_t, 1llu << i); } } diff --git a/src/entity.c b/src/entity.c index 6e3214e5e..b28908305 100644 --- a/src/entity.c +++ b/src/entity.c @@ -1263,6 +1263,7 @@ void flecs_invoke_hook( it.table = table; it.trs[0] = tr; it.row_fields = !!(((ecs_id_record_t*)tr->hdr.cache)->flags & EcsIdIsSparse); + it.ref_fields = it.row_fields; it.sizes = ECS_CONST_CAST(ecs_size_t*, &ti->size); it.ids[0] = id; it.event = event; @@ -1272,6 +1273,7 @@ void flecs_invoke_hook( it.count = count; it.offset = row; it.flags = EcsIterIsValid; + hook(&it); ecs_iter_fini(&it); diff --git a/src/iter.c b/src/iter.c index aa785b636..4d9d8d348 100644 --- a/src/iter.c +++ b/src/iter.c @@ -184,7 +184,7 @@ void* ecs_field_at_w_size( ecs_entity_t src = it->sources[index]; if (!src) { - src = ecs_table_entities(it->table)[row]; + src = ecs_table_entities(it->table)[row + it->offset]; } return flecs_sparse_get_any(idr->sparse, flecs_uto(int32_t, size), src); diff --git a/src/query/validator.c b/src/query/validator.c index b000d9f2f..594f9e0f6 100644 --- a/src/query/validator.c +++ b/src/query/validator.c @@ -1633,7 +1633,7 @@ bool flecs_query_finalize_simple( if (idr->flags & EcsIdIsSparse) { term->flags_ |= EcsTermIsSparse; cacheable = false; trivial = false; - q->row_fields |= (1llu << i); + q->row_fields |= flecs_uto(uint32_t, 1llu << i); } } diff --git a/test/core/project.json b/test/core/project.json index 097800aec..69bd2a0aa 100644 --- a/test/core/project.json +++ b/test/core/project.json @@ -489,6 +489,7 @@ "on_add_remove_after_fini", "on_set_after_set", "on_set_after_modified", + "on_set_at_offset", "on_add_observer", "on_set_observer_set", "on_set_observer_modified", diff --git a/test/core/src/Sparse.c b/test/core/src/Sparse.c index 0abf966c2..3ab5c5914 100644 --- a/test/core/src/Sparse.c +++ b/test/core/src/Sparse.c @@ -807,6 +807,7 @@ static int position_dtor_invoked = 0; static int position_on_add_invoked = 0; static int position_on_remove_invoked = 0; static int position_on_set_invoked = 0; +static Position position_on_set_value = {0, 0}; static ECS_CTOR(Position, ptr, { position_ctor_invoked ++; @@ -847,8 +848,7 @@ static void Position_on_set(ecs_iter_t *it) { test_int(0, position_dtor_invoked); Position *p = ecs_field_at(it, Position, 0, 0); test_assert(p != NULL); - test_int(p->x, 30); - test_int(p->y, 40); + position_on_set_value = *p; test_uint(it->event, EcsOnSet); position_on_set_invoked ++; @@ -1182,6 +1182,8 @@ void Sparse_on_set_after_set(void) { test_int(1, position_ctor_invoked); test_int(1, position_on_add_invoked); test_int(1, position_on_set_invoked); + test_int(30, position_on_set_value.x); + test_int(40, position_on_set_value.y); { const Position *ptr = ecs_get(world, e, Position); @@ -1217,6 +1219,8 @@ void Sparse_on_set_after_modified(void) { test_int(1, position_ctor_invoked); test_int(1, position_on_add_invoked); test_int(1, position_on_set_invoked); + test_int(30, position_on_set_value.x); + test_int(40, position_on_set_value.y); { const Position *ptr = ecs_get(world, e, Position); @@ -1227,6 +1231,45 @@ void Sparse_on_set_after_modified(void) { ecs_fini(world); } +void Sparse_on_set_at_offset(void) { + ecs_world_t *world = ecs_mini(); + + ECS_COMPONENT(world, Position); + ECS_TAG(world, Tag); + + ecs_add_id(world, ecs_id(Position), EcsSparse); + + ecs_set_hooks(world, Position, { + .ctor = ecs_ctor(Position), + .on_add = Position_on_add, + .on_set = Position_on_set + }); + + ecs_entity_t e = ecs_new(world); + ecs_set(world, e, Position, {10, 20}); + + test_int(1, position_ctor_invoked); + test_int(1, position_on_add_invoked); + test_int(1, position_on_set_invoked); + test_int(10, position_on_set_value.x); + test_int(20, position_on_set_value.y); + + position_ctor_invoked = 0; + position_on_add_invoked = 0; + position_on_set_invoked = 0; + + ecs_entity_t e2 = ecs_new(world); + ecs_set(world, e2, Position, {30, 40}); + + test_int(1, position_ctor_invoked); + test_int(1, position_on_add_invoked); + test_int(1, position_on_set_invoked); + test_int(30, position_on_set_value.x); + test_int(40, position_on_set_value.y); + + ecs_fini(world); +} + static void Position_add_observer(ecs_iter_t *it) { probe_iter(it); diff --git a/test/core/src/main.c b/test/core/src/main.c index 87f83401b..856bbfde4 100644 --- a/test/core/src/main.c +++ b/test/core/src/main.c @@ -462,6 +462,7 @@ void Sparse_on_add_remove_after_delete(void); void Sparse_on_add_remove_after_fini(void); void Sparse_on_set_after_set(void); void Sparse_on_set_after_modified(void); +void Sparse_on_set_at_offset(void); void Sparse_on_add_observer(void); void Sparse_on_set_observer_set(void); void Sparse_on_set_observer_modified(void); @@ -3984,6 +3985,10 @@ bake_test_case Sparse_testcases[] = { "on_set_after_modified", Sparse_on_set_after_modified }, + { + "on_set_at_offset", + Sparse_on_set_at_offset + }, { "on_add_observer", Sparse_on_add_observer @@ -10911,7 +10916,7 @@ static bake_test_suite suites[] = { "Sparse", NULL, NULL, - 86, + 87, Sparse_testcases }, { diff --git a/test/cpp/project.json b/test/cpp/project.json index 3ca677883..19e0bc868 100644 --- a/test/cpp/project.json +++ b/test/cpp/project.json @@ -987,6 +987,15 @@ "on_add_hook_w_entity", "on_remove_hook_w_entity", "on_set_hook_w_entity", + "on_add_hook_sparse", + "on_remove_hook_sparse", + "on_set_hook_sparse", + "on_add_hook_sparse_w_entity", + "on_remove_hook_sparse_w_entity", + "on_set_hook_sparse_w_entity", + "on_add_hook_sparse_w_iter", + "on_remove_hook_sparse_w_iter", + "on_set_hook_sparse_w_iter", "chained_hooks", "ctor_w_2_worlds", "ctor_w_2_worlds_explicit_registration", diff --git a/test/cpp/src/ComponentLifecycle.cpp b/test/cpp/src/ComponentLifecycle.cpp index 8251db297..129313704 100644 --- a/test/cpp/src/ComponentLifecycle.cpp +++ b/test/cpp/src/ComponentLifecycle.cpp @@ -1448,9 +1448,100 @@ void ComponentLifecycle_on_add_hook_w_entity(void) { test_int(0, count); test_assert(e_arg == 0); + auto e1 = ecs.entity().add(); + test_int(1, count); + test_assert(e_arg == e1); + + e1.add(); + test_int(1, count); + + auto e2 = ecs.entity().add(); + test_int(2, count); + test_assert(e_arg == e2); + } + + test_int(2, count); +} + +void ComponentLifecycle_on_remove_hook_w_entity(void) { + int count = 0; + flecs::entity e_arg; + flecs::entity e2; + + { + flecs::world ecs; + + ecs.component().on_remove([&](flecs::entity arg, Position& p){ + e_arg = arg; + count ++; + }); + + test_int(0, count); + test_assert(e_arg == 0); + + auto e1 = ecs.entity().add(); + e2 = ecs.entity().add(); + test_int(0, count); + + e1.remove(); + test_int(1, count); + test_assert(e_arg == e1); + } + + test_int(2, count); + test_assert(e_arg == e2); +} + +void ComponentLifecycle_on_set_hook_w_entity(void) { + int count = 0; + Position v = {0}; + flecs::entity e_arg; + + { + flecs::world ecs; + + ecs.component().on_set([&](flecs::entity arg, Position& p) { + count ++; + v = p; + e_arg = arg; + }); + + test_int(0, count); + + auto e1 = ecs.entity().add(); + test_int(0, count); + + e1.set({10, 20}); + test_int(1, count); + test_assert(e_arg == e1); + test_int(10, v.x); + test_int(20, v.y); + + auto e2 = ecs.entity().set({30, 40}); + test_int(2, count); + test_assert(e_arg == e2); + test_int(30, v.x); + test_int(40, v.y); + } + + test_int(2, count); +} + +void ComponentLifecycle_on_add_hook_sparse(void) { + int count = 0; + + { + flecs::world ecs; + + ecs.component().add(flecs::Sparse); + ecs.component().on_add([&](Position& p) { + count ++; + }); + + test_int(0, count); + auto e = ecs.entity().add(); test_int(1, count); - test_assert(e_arg == e); e.add(); test_int(1, count); @@ -1459,7 +1550,94 @@ void ComponentLifecycle_on_add_hook_w_entity(void) { test_int(1, count); } -void ComponentLifecycle_on_remove_hook_w_entity(void) { +void ComponentLifecycle_on_remove_hook_sparse(void) { + int count = 0; + + { + flecs::world ecs; + + ecs.component().add(flecs::Sparse); + ecs.component().on_remove([&](Position& p) { + count ++; + }); + + test_int(0, count); + + auto e1 = ecs.entity().add(); + ecs.entity().add(); + test_int(0, count); + + e1.remove(); + test_int(1, count); + } + + test_int(2, count); +} + +void ComponentLifecycle_on_set_hook_sparse(void) { + int count = 0; + Position v = {0}; + + { + flecs::world ecs; + + ecs.component().add(flecs::Sparse); + ecs.component().on_set([&](Position& p) { + count ++; + v = p; + }); + + test_int(0, count); + + auto e1 = ecs.entity().add(); + test_int(0, count); + + e1.set({10, 20}); + test_int(1, count); + test_int(10, v.x); + test_int(20, v.y); + + ecs.entity().set({30, 40}); + test_int(2, count); + test_int(30, v.x); + test_int(40, v.y); + } + + test_int(2, count); +} + +void ComponentLifecycle_on_add_hook_sparse_w_entity(void) { + int count = 0; + flecs::entity e_arg; + + { + flecs::world ecs; + + ecs.component().add(flecs::Sparse); + ecs.component().on_add([&](flecs::entity arg, Position& p) { + e_arg = arg; + count ++; + }); + + test_int(0, count); + test_assert(e_arg == 0); + + auto e1 = ecs.entity().add(); + test_int(1, count); + test_assert(e_arg == e1); + + e1.add(); + test_int(1, count); + + auto e2 = ecs.entity().add(); + test_int(2, count); + test_assert(e_arg == e2); + } + + test_int(2, count); +} + +void ComponentLifecycle_on_remove_hook_sparse_w_entity(void) { int count = 0; flecs::entity e_arg; flecs::entity e2; @@ -1467,6 +1645,7 @@ void ComponentLifecycle_on_remove_hook_w_entity(void) { { flecs::world ecs; + ecs.component().add(flecs::Sparse); ecs.component().on_remove([&](flecs::entity arg, Position& p){ e_arg = arg; count ++; @@ -1488,7 +1667,7 @@ void ComponentLifecycle_on_remove_hook_w_entity(void) { test_assert(e_arg == e2); } -void ComponentLifecycle_on_set_hook_w_entity(void) { +void ComponentLifecycle_on_set_hook_sparse_w_entity(void) { int count = 0; Position v = {0}; flecs::entity e_arg; @@ -1496,6 +1675,7 @@ void ComponentLifecycle_on_set_hook_w_entity(void) { { flecs::world ecs; + ecs.component().add(flecs::Sparse); ecs.component().on_set([&](flecs::entity arg, Position& p) { count ++; v = p; @@ -1523,6 +1703,106 @@ void ComponentLifecycle_on_set_hook_w_entity(void) { test_int(2, count); } +void ComponentLifecycle_on_add_hook_sparse_w_iter(void) { + int count = 0; + flecs::entity e_arg; + + { + flecs::world ecs; + + ecs.component().add(flecs::Sparse); + ecs.component() + .on_add([&](flecs::iter& it, size_t row, Position& p) { + e_arg = it.entity(row); + count ++; + }); + + test_int(0, count); + test_assert(e_arg == 0); + + auto e1 = ecs.entity().add(); + test_int(1, count); + test_assert(e_arg == e1); + + e1.add(); + test_int(1, count); + + auto e2 = ecs.entity().add(); + test_int(2, count); + test_assert(e_arg == e2); + } + + test_int(2, count); +} + +void ComponentLifecycle_on_remove_hook_sparse_w_iter(void) { + int count = 0; + flecs::entity e_arg; + flecs::entity e2; + + { + flecs::world ecs; + + ecs.component().add(flecs::Sparse); + ecs.component() + .on_remove([&](flecs::iter& it, size_t row, Position& p){ + e_arg = it.entity(row); + count ++; + }); + + test_int(0, count); + test_assert(e_arg == 0); + + auto e1 = ecs.entity().add(); + e2 = ecs.entity().add(); + test_int(0, count); + + e1.remove(); + test_int(1, count); + test_assert(e_arg == e1); + } + + test_int(2, count); + test_assert(e_arg == e2); +} + +void ComponentLifecycle_on_set_hook_sparse_w_iter(void) { + int count = 0; + Position v = {0}; + flecs::entity e_arg; + + { + flecs::world ecs; + + ecs.component().add(flecs::Sparse); + ecs.component() + .on_set([&](flecs::iter& it, size_t row, Position& p) { + count ++; + v = p; + e_arg = it.entity(row); + }); + + test_int(0, count); + + auto e1 = ecs.entity().add(); + test_int(0, count); + + e1.set({10, 20}); + test_int(1, count); + test_assert(e_arg == e1); + test_int(10, v.x); + test_int(20, v.y); + + auto e2 = ecs.entity().set({30, 40}); + test_int(2, count); + test_assert(e_arg == e2); + test_int(30, v.x); + test_int(40, v.y); + } + + test_int(2, count); +} + void ComponentLifecycle_chained_hooks(void) { flecs::world ecs; diff --git a/test/cpp/src/main.cpp b/test/cpp/src/main.cpp index 4c5422311..827047453 100644 --- a/test/cpp/src/main.cpp +++ b/test/cpp/src/main.cpp @@ -952,6 +952,15 @@ void ComponentLifecycle_on_set_hook(void); void ComponentLifecycle_on_add_hook_w_entity(void); void ComponentLifecycle_on_remove_hook_w_entity(void); void ComponentLifecycle_on_set_hook_w_entity(void); +void ComponentLifecycle_on_add_hook_sparse(void); +void ComponentLifecycle_on_remove_hook_sparse(void); +void ComponentLifecycle_on_set_hook_sparse(void); +void ComponentLifecycle_on_add_hook_sparse_w_entity(void); +void ComponentLifecycle_on_remove_hook_sparse_w_entity(void); +void ComponentLifecycle_on_set_hook_sparse_w_entity(void); +void ComponentLifecycle_on_add_hook_sparse_w_iter(void); +void ComponentLifecycle_on_remove_hook_sparse_w_iter(void); +void ComponentLifecycle_on_set_hook_sparse_w_iter(void); void ComponentLifecycle_chained_hooks(void); void ComponentLifecycle_ctor_w_2_worlds(void); void ComponentLifecycle_ctor_w_2_worlds_explicit_registration(void); @@ -5054,6 +5063,42 @@ bake_test_case ComponentLifecycle_testcases[] = { "on_set_hook_w_entity", ComponentLifecycle_on_set_hook_w_entity }, + { + "on_add_hook_sparse", + ComponentLifecycle_on_add_hook_sparse + }, + { + "on_remove_hook_sparse", + ComponentLifecycle_on_remove_hook_sparse + }, + { + "on_set_hook_sparse", + ComponentLifecycle_on_set_hook_sparse + }, + { + "on_add_hook_sparse_w_entity", + ComponentLifecycle_on_add_hook_sparse_w_entity + }, + { + "on_remove_hook_sparse_w_entity", + ComponentLifecycle_on_remove_hook_sparse_w_entity + }, + { + "on_set_hook_sparse_w_entity", + ComponentLifecycle_on_set_hook_sparse_w_entity + }, + { + "on_add_hook_sparse_w_iter", + ComponentLifecycle_on_add_hook_sparse_w_iter + }, + { + "on_remove_hook_sparse_w_iter", + ComponentLifecycle_on_remove_hook_sparse_w_iter + }, + { + "on_set_hook_sparse_w_iter", + ComponentLifecycle_on_set_hook_sparse_w_iter + }, { "chained_hooks", ComponentLifecycle_chained_hooks @@ -6693,7 +6738,7 @@ static bake_test_suite suites[] = { "ComponentLifecycle", NULL, NULL, - 79, + 88, ComponentLifecycle_testcases }, {