From 3095237009b7099726c7aad83b190e07a09f139d Mon Sep 17 00:00:00 2001 From: Sander Mertens Date: Fri, 20 Sep 2024 13:39:48 -0700 Subject: [PATCH] #1366 Fix issue with C++ observers and up traversal --- distr/flecs.c | 5 + distr/flecs.h | 1 - include/flecs/addons/cpp/delegate.hpp | 1 - src/observable.c | 5 + test/core/include/core.h | 3 + test/core/project.json | 8 + test/core/src/Observer.c | 291 ++++++++++++++++++++++++++ test/core/src/main.c | 42 +++- test/core/src/util.c | 3 + test/cpp/project.json | 5 +- test/cpp/src/Observer.cpp | 94 ++++++++- test/cpp/src/main.cpp | 17 +- 12 files changed, 465 insertions(+), 10 deletions(-) diff --git a/distr/flecs.c b/distr/flecs.c index 19621d9daf..d4b320712c 100644 --- a/distr/flecs.c +++ b/distr/flecs.c @@ -12784,6 +12784,7 @@ void flecs_emit_propagate_id( it->other_table = NULL; it->offset = 0; it->count = entity_count; + it->up_fields = 1; if (entity_count) { it->entities = ecs_table_entities(table); } @@ -12822,6 +12823,7 @@ void flecs_emit_propagate_id( } it->event_cur = event_cur; + it->up_fields = 0; } static @@ -13116,6 +13118,7 @@ void flecs_emit_forward_id( it->sources[0] = tgt; it->event_id = id; ECS_CONST_CAST(int32_t*, it->sizes)[0] = 0; /* safe, owned by observer */ + it->up_fields = 1; int32_t storage_i = ecs_table_type_to_column_index(tgt_table, column); if (storage_i != -1) { @@ -13176,6 +13179,8 @@ void flecs_emit_forward_id( it->trs[0] = base_tr; } } + + it->up_fields = 0; } static diff --git a/distr/flecs.h b/distr/flecs.h index e365b8c67d..488a822b4e 100644 --- a/distr/flecs.h +++ b/distr/flecs.h @@ -25737,7 +25737,6 @@ struct field_ptrs { void populate_self(const ecs_iter_t *iter, size_t index, T, Targs... comps) { fields_[index].ptr = ecs_field_w_size(iter, sizeof(A), static_cast(index)); - // fields_[index].is_ref = iter->sources[index] != 0; fields_[index].is_ref = false; ecs_assert(iter->sources[index] == 0, ECS_INTERNAL_ERROR, NULL); populate_self(iter, index + 1, comps ...); diff --git a/include/flecs/addons/cpp/delegate.hpp b/include/flecs/addons/cpp/delegate.hpp index c9bdad7ff4..42b22c9c75 100644 --- a/include/flecs/addons/cpp/delegate.hpp +++ b/include/flecs/addons/cpp/delegate.hpp @@ -99,7 +99,6 @@ struct field_ptrs { void populate_self(const ecs_iter_t *iter, size_t index, T, Targs... comps) { fields_[index].ptr = ecs_field_w_size(iter, sizeof(A), static_cast(index)); - // fields_[index].is_ref = iter->sources[index] != 0; fields_[index].is_ref = false; ecs_assert(iter->sources[index] == 0, ECS_INTERNAL_ERROR, NULL); populate_self(iter, index + 1, comps ...); diff --git a/src/observable.c b/src/observable.c index 1897e943a8..13dbee09f7 100644 --- a/src/observable.c +++ b/src/observable.c @@ -281,6 +281,7 @@ void flecs_emit_propagate_id( it->other_table = NULL; it->offset = 0; it->count = entity_count; + it->up_fields = 1; if (entity_count) { it->entities = ecs_table_entities(table); } @@ -319,6 +320,7 @@ void flecs_emit_propagate_id( } it->event_cur = event_cur; + it->up_fields = 0; } static @@ -613,6 +615,7 @@ void flecs_emit_forward_id( it->sources[0] = tgt; it->event_id = id; ECS_CONST_CAST(int32_t*, it->sizes)[0] = 0; /* safe, owned by observer */ + it->up_fields = 1; int32_t storage_i = ecs_table_type_to_column_index(tgt_table, column); if (storage_i != -1) { @@ -673,6 +676,8 @@ void flecs_emit_forward_id( it->trs[0] = base_tr; } } + + it->up_fields = 0; } static diff --git a/test/core/include/core.h b/test/core/include/core.h index ae2a9453bf..7efa0b9a79 100644 --- a/test/core/include/core.h +++ b/test/core/include/core.h @@ -29,6 +29,9 @@ typedef struct Probe { ecs_entity_t e[MAX_ENTITIES]; ecs_entity_t c[MAX_INVOCATIONS][MAX_SYS_COLUMNS]; ecs_entity_t s[MAX_INVOCATIONS][MAX_SYS_COLUMNS]; + ecs_flags32_t ref_fields; + ecs_flags32_t up_fields; + ecs_flags32_t row_fields; void *param; } Probe; diff --git a/test/core/project.json b/test/core/project.json index 48981e75a5..85d74544d8 100644 --- a/test/core/project.json +++ b/test/core/project.json @@ -1628,6 +1628,14 @@ "register_callback_after_run_ctx", "register_run_after_callback_ctx", "on_add_after_new_w_table", + "ref_flag_term_1", + "ref_flag_term_2", + "forward_up_flag_term_1", + "forward_up_flag_term_2", + "propagate_up_flag_term_1", + "propagate_up_flag_term_2", + "row_flag_term_1", + "row_flag_term_2", "cache_test_1", "cache_test_2", "cache_test_3", diff --git a/test/core/src/Observer.c b/test/core/src/Observer.c index f730233a3f..90caf138c8 100644 --- a/test/core/src/Observer.c +++ b/test/core/src/Observer.c @@ -8766,3 +8766,294 @@ void Observer_multi_observer_eval_count(void) { ecs_fini(world); } + +void Observer_ref_flag_term_1(void) { + ecs_world_t *world = ecs_mini(); + + ECS_COMPONENT(world, Position); + + Probe ctx = {0}; + + ecs_entity_t s = ecs_new(world); + + ecs_entity_t o = ecs_observer(world, { + .query.terms = { + { .id = ecs_id(Position), .src.id = s }, + }, + .callback = Observer, + .events = { EcsOnAdd }, + .ctx = &ctx + }); + + ecs_set(world, s, Position, {10, 20}); + + test_int(ctx.invoked, 1); + // test_int(ctx.count, 0); // TODO + test_int(ctx.system, o); + test_int(ctx.event, EcsOnAdd); + test_uint(ctx.s[0][0], s); + test_uint(ctx.ref_fields, (1llu << 0)); + test_uint(ctx.up_fields, 0); + test_uint(ctx.row_fields, 0); + + ecs_fini(world); +} + +void Observer_ref_flag_term_2(void) { + ecs_world_t *world = ecs_mini(); + + ECS_COMPONENT(world, Position); + ECS_COMPONENT(world, Velocity); + + Probe ctx = {0}; + + ecs_entity_t s = ecs_new(world); + + ecs_entity_t o = ecs_observer(world, { + .query.terms = { + { .id = ecs_id(Position), .src.id = s }, + { .id = ecs_id(Velocity), .src.id = s } + }, + .callback = Observer, + .events = { EcsOnAdd }, + .ctx = &ctx + }); + + test_int(ctx.invoked, 0); + + ecs_set(world, s, Position, {10, 20}); + test_int(ctx.invoked, 0); + + ecs_set(world, s, Velocity, {10, 20}); + + test_int(ctx.invoked, 1); + test_int(ctx.count, 0); + test_int(ctx.system, o); + test_int(ctx.event, EcsOnAdd); + test_uint(ctx.s[0][0], s); + test_uint(ctx.s[0][1], s); + test_uint(ctx.ref_fields, (1llu << 0) | (1llu << 1)); + test_uint(ctx.up_fields, 0); + test_uint(ctx.row_fields, 0); + + ecs_fini(world); +} + +void Observer_forward_up_flag_term_1(void) { + ecs_world_t *world = ecs_mini(); + + ECS_COMPONENT(world, Position); + + Probe ctx = {0}; + + ecs_entity_t o = ecs_observer(world, { + .query.terms = { + { .id = ecs_id(Position), .src.id = EcsUp }, + }, + .callback = Observer, + .events = { EcsOnAdd }, + .ctx = &ctx + }); + + ecs_entity_t s = ecs_new(world); + ecs_set(world, s, Position, {10, 20}); + + test_int(ctx.invoked, 0); + + ecs_new_w_pair(world, EcsChildOf, s); + + test_int(ctx.invoked, 1); + test_int(ctx.count, 1); + test_int(ctx.system, o); + test_int(ctx.event, EcsOnAdd); + test_uint(ctx.s[0][0], s); + test_uint(ctx.ref_fields, 0); + test_uint(ctx.up_fields, (1llu << 0)); + test_uint(ctx.row_fields, 0); + + ecs_fini(world); +} + +void Observer_forward_up_flag_term_2(void) { + ecs_world_t *world = ecs_mini(); + + ECS_COMPONENT(world, Position); + ECS_COMPONENT(world, Velocity); + + Probe ctx = {0}; + + ecs_entity_t o = ecs_observer(world, { + .query.terms = { + { .id = ecs_id(Position), .src.id = EcsUp }, + { .id = ecs_id(Velocity), .src.id = EcsUp }, + }, + .callback = Observer, + .events = { EcsOnAdd }, + .ctx = &ctx + }); + + ecs_entity_t s = ecs_new(world); + ecs_set(world, s, Position, {10, 20}); + ecs_set(world, s, Velocity, {1, 2}); + + test_int(ctx.invoked, 0); + + ecs_new_w_pair(world, EcsChildOf, s); + + test_int(ctx.invoked, 1); + test_int(ctx.count, 1); + test_int(ctx.system, o); + test_int(ctx.event, EcsOnAdd); + test_uint(ctx.s[0][0], s); + test_uint(ctx.ref_fields, 0); + test_uint(ctx.up_fields, (1llu << 0) | (1llu << 1)); + test_uint(ctx.row_fields, 0); + + ecs_fini(world); +} + +void Observer_propagate_up_flag_term_1(void) { + ecs_world_t *world = ecs_mini(); + + ECS_COMPONENT(world, Position); + + Probe ctx = {0}; + + ecs_entity_t o = ecs_observer(world, { + .query.terms = { + { .id = ecs_id(Position), .src.id = EcsUp }, + }, + .callback = Observer, + .events = { EcsOnAdd }, + .ctx = &ctx + }); + + ecs_entity_t s = ecs_new(world); + ecs_new_w_pair(world, EcsChildOf, s); + test_int(ctx.invoked, 0); + + ecs_set(world, s, Position, {10, 20}); + + test_int(ctx.invoked, 1); + test_int(ctx.count, 1); + test_int(ctx.system, o); + test_int(ctx.event, EcsOnAdd); + test_uint(ctx.s[0][0], s); + test_uint(ctx.ref_fields, 0); + test_uint(ctx.up_fields, (1llu << 0)); + test_uint(ctx.row_fields, 0); + + ecs_fini(world); +} + +void Observer_propagate_up_flag_term_2(void) { + ecs_world_t *world = ecs_mini(); + + ECS_COMPONENT(world, Position); + ECS_COMPONENT(world, Velocity); + + Probe ctx = {0}; + + ecs_entity_t o = ecs_observer(world, { + .query.terms = { + { .id = ecs_id(Position), .src.id = EcsUp }, + { .id = ecs_id(Velocity), .src.id = EcsUp }, + }, + .callback = Observer, + .events = { EcsOnAdd }, + .ctx = &ctx + }); + + ecs_entity_t s = ecs_new(world); + ecs_new_w_pair(world, EcsChildOf, s); + test_int(ctx.invoked, 0); + + ecs_set(world, s, Position, {10, 20}); + test_int(ctx.invoked, 0); + + ecs_set(world, s, Velocity, {1, 2}); + + test_int(ctx.invoked, 1); + test_int(ctx.count, 1); + test_int(ctx.system, o); + test_int(ctx.event, EcsOnAdd); + test_uint(ctx.s[0][0], s); + test_uint(ctx.ref_fields, 0); + test_uint(ctx.up_fields, (1llu << 0) | (1llu << 1)); + test_uint(ctx.row_fields, 0); + + ecs_fini(world); +} + +void Observer_row_flag_term_1(void) { + ecs_world_t *world = ecs_mini(); + + ECS_COMPONENT(world, Position); + + ecs_add_id(world, ecs_id(Position), EcsSparse); + + Probe ctx = {0}; + + ecs_entity_t o = ecs_observer(world, { + .query.terms = { + { .id = ecs_id(Position) }, + }, + .callback = Observer, + .events = { EcsOnAdd }, + .ctx = &ctx + }); + + ecs_entity_t s = ecs_new(world); + ecs_set(world, s, Position, {10, 20}); + + test_int(ctx.invoked, 1); + test_int(ctx.count, 1); + test_int(ctx.system, o); + test_int(ctx.event, EcsOnAdd); + test_uint(ctx.s[0][0], 0); + test_uint(ctx.ref_fields, (1llu << 0)); + test_uint(ctx.up_fields, 0); + test_uint(ctx.row_fields, (1llu << 0)); + + ecs_fini(world); +} + +void Observer_row_flag_term_2(void) { + ecs_world_t *world = ecs_mini(); + + ECS_COMPONENT(world, Position); + ECS_COMPONENT(world, Velocity); + + ecs_add_id(world, ecs_id(Position), EcsSparse); + ecs_add_id(world, ecs_id(Velocity), EcsSparse); + + Probe ctx = {0}; + + ecs_entity_t o = ecs_observer(world, { + .query.terms = { + { .id = ecs_id(Position) }, + { .id = ecs_id(Velocity) }, + }, + .callback = Observer, + .events = { EcsOnAdd }, + .ctx = &ctx + }); + + ecs_entity_t s = ecs_new(world); + + ecs_set(world, s, Position, {10, 20}); + test_int(ctx.invoked, 0); + + ecs_set(world, s, Velocity, {1, 2}); + + test_int(ctx.invoked, 1); + test_int(ctx.count, 1); + test_int(ctx.system, o); + test_int(ctx.event, EcsOnAdd); + test_uint(ctx.s[0][0], 0); + test_uint(ctx.ref_fields, (1llu << 0) | (1llu << 1)); + test_uint(ctx.up_fields, 0); + test_uint(ctx.row_fields, (1llu << 0) | (1llu << 1)); + + ecs_fini(world); +} diff --git a/test/core/src/main.c b/test/core/src/main.c index 965b451e0e..7081b43068 100644 --- a/test/core/src/main.c +++ b/test/core/src/main.c @@ -1567,6 +1567,14 @@ void Observer_register_run_after_callback(void); void Observer_register_callback_after_run_ctx(void); void Observer_register_run_after_callback_ctx(void); void Observer_on_add_after_new_w_table(void); +void Observer_ref_flag_term_1(void); +void Observer_ref_flag_term_2(void); +void Observer_forward_up_flag_term_1(void); +void Observer_forward_up_flag_term_2(void); +void Observer_propagate_up_flag_term_1(void); +void Observer_propagate_up_flag_term_2(void); +void Observer_row_flag_term_1(void); +void Observer_row_flag_term_2(void); void Observer_cache_test_1(void); void Observer_cache_test_2(void); void Observer_cache_test_3(void); @@ -8308,6 +8316,38 @@ bake_test_case Observer_testcases[] = { "on_add_after_new_w_table", Observer_on_add_after_new_w_table }, + { + "ref_flag_term_1", + Observer_ref_flag_term_1 + }, + { + "ref_flag_term_2", + Observer_ref_flag_term_2 + }, + { + "forward_up_flag_term_1", + Observer_forward_up_flag_term_1 + }, + { + "forward_up_flag_term_2", + Observer_forward_up_flag_term_2 + }, + { + "propagate_up_flag_term_1", + Observer_propagate_up_flag_term_1 + }, + { + "propagate_up_flag_term_2", + Observer_propagate_up_flag_term_2 + }, + { + "row_flag_term_1", + Observer_row_flag_term_1 + }, + { + "row_flag_term_2", + Observer_row_flag_term_2 + }, { "cache_test_1", Observer_cache_test_1 @@ -11095,7 +11135,7 @@ static bake_test_suite suites[] = { "Observer", NULL, NULL, - 208, + 216, Observer_testcases }, { diff --git a/test/core/src/util.c b/test/core/src/util.c index a96336c7b2..7a176f21fd 100644 --- a/test/core/src/util.c +++ b/test/core/src/util.c @@ -16,6 +16,9 @@ void probe_system_w_ctx( ctx->offset = 0; ctx->term_count = it->field_count; ctx->term_index = it->term_index; + ctx->ref_fields = it->ref_fields; + ctx->up_fields = it->up_fields; + ctx->row_fields = it->row_fields; int i; for (i = 0; i < ctx->term_count; i ++) { diff --git a/test/cpp/project.json b/test/cpp/project.json index e0c107d6a2..34cbd78f47 100644 --- a/test/cpp/project.json +++ b/test/cpp/project.json @@ -933,7 +933,10 @@ "register_twice_w_each_run", "other_table", "other_table_w_pair", - "other_table_w_pair_wildcard" + "other_table_w_pair_wildcard", + "on_add_inherited", + "on_set_inherited", + "on_remove_inherited" ] }, { "id": "ComponentLifecycle", diff --git a/test/cpp/src/Observer.cpp b/test/cpp/src/Observer.cpp index c22c8d9e95..cb3b748eca 100644 --- a/test/cpp/src/Observer.cpp +++ b/test/cpp/src/Observer.cpp @@ -1141,17 +1141,19 @@ void Observer_other_table(void) { flecs::world ecs; int32_t count = 0; + flecs::entity matched; ecs.observer() .event(flecs::OnAdd) - .each([&](flecs::iter& it, size_t, Velocity&) { + .each([&](flecs::iter& it, size_t row, Velocity&) { test_assert(it.table().has()); test_assert(!it.other_table().has()); + matched = it.entity(row); count ++; }); flecs::entity e = ecs.entity().add().add(); - + test_assert(e == matched); test_int(count, 1); } @@ -1162,18 +1164,20 @@ void Observer_other_table_w_pair(void) { struct Apples {}; int32_t count = 0; + flecs::entity matched; ecs.observer() .with() .event(flecs::OnAdd) - .each([&](flecs::iter& it, size_t) { + .each([&](flecs::iter& it, size_t row) { test_assert((it.table().has())); test_assert((!it.other_table().has())); + matched = it.entity(row); count ++; }); flecs::entity e = ecs.entity().add().add(); - + test_assert(e == matched); test_int(count, 1); } @@ -1184,17 +1188,97 @@ void Observer_other_table_w_pair_wildcard(void) { struct Apples {}; int32_t count = 0; + flecs::entity matched; ecs.observer() .with() .event(flecs::OnAdd) - .each([&](flecs::iter& it, size_t) { + .each([&](flecs::iter& it, size_t row) { test_assert((it.table().has(flecs::Wildcard))); test_assert((!it.other_table().has(flecs::Wildcard))); + matched = it.entity(row); count ++; }); flecs::entity e = ecs.entity().add().add(); + test_assert(e == matched); + test_int(count, 1); +} + +void Observer_on_add_inherited(void) { + flecs::world ecs; + + ecs.component().add(flecs::OnInstantiate, flecs::Inherit); + + int32_t count = 0; + flecs::entity matched; + + ecs.observer() + .event(flecs::OnAdd) + .each([&](flecs::entity e, Position& p) { + test_int(p.x, 10); + test_int(p.y, 20); + count ++; + matched = e; + }); + + auto p = ecs.prefab().set(Position{10, 20}); + test_int(count, 0); + + auto i = ecs.entity().is_a(p); + test_int(count, 1); + test_assert(i == matched); +} + +void Observer_on_set_inherited(void) { + flecs::world ecs; + + ecs.component().add(flecs::OnInstantiate, flecs::Inherit); + + int32_t count = 0; + flecs::entity matched; + + ecs.observer() + .event(flecs::OnSet) + .each([&](flecs::entity e, Position& p) { + test_int(p.x, 10); + test_int(p.y, 20); + count ++; + matched = e; + }); + + auto p = ecs.prefab().set(Position{10, 20}); + test_int(count, 0); + + auto i = ecs.entity().is_a(p); + test_int(count, 1); + test_assert(i == matched); +} + +void Observer_on_remove_inherited(void) { + flecs::world ecs; + + ecs.component().add(flecs::OnInstantiate, flecs::Inherit); + + int32_t count = 0; + flecs::entity matched; + + ecs.observer() + .event(flecs::OnRemove) + .each([&](flecs::entity e, Position& p) { + test_int(p.x, 10); + test_int(p.y, 20); + count ++; + matched = e; + }); + + auto p = ecs.prefab().set(Position{10, 20}); + test_int(count, 0); + + auto i = ecs.entity().is_a(p); + test_int(count, 0); + p.remove(); test_int(count, 1); + test_assert(i == matched); } diff --git a/test/cpp/src/main.cpp b/test/cpp/src/main.cpp index 0f49f29a19..7d68357119 100644 --- a/test/cpp/src/main.cpp +++ b/test/cpp/src/main.cpp @@ -901,6 +901,9 @@ void Observer_register_twice_w_each_run(void); void Observer_other_table(void); void Observer_other_table_w_pair(void); void Observer_other_table_w_pair_wildcard(void); +void Observer_on_add_inherited(void); +void Observer_on_set_inherited(void); +void Observer_on_remove_inherited(void); // Testsuite 'ComponentLifecycle' void ComponentLifecycle_ctor_on_add(void); @@ -4873,6 +4876,18 @@ bake_test_case Observer_testcases[] = { { "other_table_w_pair_wildcard", Observer_other_table_w_pair_wildcard + }, + { + "on_add_inherited", + Observer_on_add_inherited + }, + { + "on_set_inherited", + Observer_on_set_inherited + }, + { + "on_remove_inherited", + Observer_on_remove_inherited } }; @@ -6781,7 +6796,7 @@ static bake_test_suite suites[] = { "Observer", NULL, NULL, - 49, + 52, Observer_testcases }, {