From 2bf5eb3986d1fc34e3ddbffb0d65734f2deb1def Mon Sep 17 00:00:00 2001 From: Sander Mertens Date: Tue, 15 Oct 2024 10:47:42 -0700 Subject: [PATCH] #1400 Fix issue with getting field in OnSet observer after removing override --- distr/flecs.c | 8 ++- src/observable.c | 8 ++- test/core/project.json | 4 ++ test/core/src/ObserverOnSet.c | 103 ++++++++++++++++++++++++++++++++- test/core/src/main.c | 22 ++++++- test/cpp/project.json | 6 +- test/cpp/src/Observer.cpp | 104 ++++++++++++++++++++++++++++++++++ test/cpp/src/main.cpp | 22 ++++++- 8 files changed, 268 insertions(+), 9 deletions(-) diff --git a/distr/flecs.c b/distr/flecs.c index 944e93eb50..f7880ce70f 100644 --- a/distr/flecs.c +++ b/distr/flecs.c @@ -13740,6 +13740,7 @@ void flecs_emit( int32_t ider_i, ider_count = 0; bool is_pair = ECS_IS_PAIR(id); void *override_ptr = NULL; + ecs_table_record_t *base_tr = NULL; ecs_entity_t base = 0; bool id_can_override = can_override; ecs_flags64_t id_bit = 1llu << i; @@ -13793,7 +13794,6 @@ void flecs_emit( /* Initialize overridden components with value from base */ ti = idr->type_info; if (ti) { - ecs_table_record_t *base_tr = NULL; int32_t base_column = ecs_search_relation(world, table, 0, id, EcsIsA, EcsUp, &base, NULL, &base_tr); if (base_column != -1) { @@ -13875,7 +13875,7 @@ void flecs_emit( * with the value of the overridden component. */ flecs_override_copy(world, table, tr, ti, ptr, override_ptr, offset, count); - } else if (er_onset) { + } else if (er_onset && it.other_table) { /* If an override was removed, this re-exposes the * overridden component. Because this causes the actual * (now inherited) value of the component to change, an @@ -13888,6 +13888,9 @@ void flecs_emit( /* Set the source temporarily to the base and base * component pointer. */ it.sources[0] = base; + it.trs[0] = base_tr; + it.up_fields = 1; + for (ider_set_i = 0; ider_set_i < ider_set_count; ider_set_i ++) { ecs_event_id_record_t *ider = iders_set[ider_set_i]; flecs_observers_invoke( @@ -13901,6 +13904,7 @@ void flecs_emit( } it.sources[0] = 0; + it.trs[0] = tr; } } } diff --git a/src/observable.c b/src/observable.c index 69974fdfef..a9db3a439f 100644 --- a/src/observable.c +++ b/src/observable.c @@ -1236,6 +1236,7 @@ void flecs_emit( int32_t ider_i, ider_count = 0; bool is_pair = ECS_IS_PAIR(id); void *override_ptr = NULL; + ecs_table_record_t *base_tr = NULL; ecs_entity_t base = 0; bool id_can_override = can_override; ecs_flags64_t id_bit = 1llu << i; @@ -1289,7 +1290,6 @@ void flecs_emit( /* Initialize overridden components with value from base */ ti = idr->type_info; if (ti) { - ecs_table_record_t *base_tr = NULL; int32_t base_column = ecs_search_relation(world, table, 0, id, EcsIsA, EcsUp, &base, NULL, &base_tr); if (base_column != -1) { @@ -1371,7 +1371,7 @@ void flecs_emit( * with the value of the overridden component. */ flecs_override_copy(world, table, tr, ti, ptr, override_ptr, offset, count); - } else if (er_onset) { + } else if (er_onset && it.other_table) { /* If an override was removed, this re-exposes the * overridden component. Because this causes the actual * (now inherited) value of the component to change, an @@ -1384,6 +1384,9 @@ void flecs_emit( /* Set the source temporarily to the base and base * component pointer. */ it.sources[0] = base; + it.trs[0] = base_tr; + it.up_fields = 1; + for (ider_set_i = 0; ider_set_i < ider_set_count; ider_set_i ++) { ecs_event_id_record_t *ider = iders_set[ider_set_i]; flecs_observers_invoke( @@ -1397,6 +1400,7 @@ void flecs_emit( } it.sources[0] = 0; + it.trs[0] = tr; } } } diff --git a/test/core/project.json b/test/core/project.json index da419eac6a..0408970b98 100644 --- a/test/core/project.json +++ b/test/core/project.json @@ -1673,6 +1673,10 @@ "add_base_to_2_overridden", "add_base_to_1_of_2_overridden", "on_set_after_remove_override", + "on_set_after_remove_override_isa_before_add", + "on_set_w_override_after_delete", + "on_set_w_override_after_clear", + "on_set_w_override_after_delete_w_ecs_init", "no_set_after_remove_base", "un_set_after_remove", "un_set_after_remove_base", diff --git a/test/core/src/ObserverOnSet.c b/test/core/src/ObserverOnSet.c index 9bb7e8afe3..627318697c 100644 --- a/test/core/src/ObserverOnSet.c +++ b/test/core/src/ObserverOnSet.c @@ -2,6 +2,7 @@ static void OnPosition(ecs_iter_t *it) { + test_assert(ecs_field(it, Position, 0) != NULL); probe_iter(it); } @@ -454,10 +455,8 @@ void ObserverOnSet_on_set_after_remove_override(void) { ecs_world_t *world = ecs_mini(); ECS_COMPONENT(world, Position); - ECS_COMPONENT(world, Velocity); ecs_add_pair(world, ecs_id(Position), EcsOnInstantiate, EcsInherit); - ecs_add_pair(world, ecs_id(Velocity), EcsOnInstantiate, EcsInherit); ECS_ENTITY(world, Base, Position); ECS_OBSERVER(world, OnPosition, EcsOnSet, Position); @@ -485,6 +484,106 @@ void ObserverOnSet_on_set_after_remove_override(void) { ecs_fini(world); } +void ObserverOnSet_on_set_after_remove_override_isa_before_add(void) { + ecs_world_t *world = ecs_mini(); + + ECS_COMPONENT(world, Position); + + ecs_add_pair(world, ecs_id(Position), EcsOnInstantiate, EcsInherit); + + ECS_ENTITY(world, Base, Position); + + Probe ctx = { 0 }; + ecs_set_ctx(world, &ctx, NULL); + + ecs_entity_t e = ecs_new_w_pair(world, EcsIsA, Base); + ecs_add(world, e, Position); + + ECS_OBSERVER(world, OnPosition, EcsOnSet, Position); + + ecs_remove(world, e, Position); + test_int(ctx.invoked, 1); + test_int(ctx.count, 1); + test_int(ctx.system, OnPosition); + test_int(ctx.term_count, 1); + test_null(ctx.param); + + test_int(ctx.e[0], e); + test_int(ctx.c[0][0], ecs_id(Position)); + test_int(ctx.s[0][0], Base); + + ecs_fini(world); +} + +void ObserverOnSet_on_set_w_override_after_delete(void) { + ecs_world_t *world = ecs_mini(); + + ECS_COMPONENT(world, Position); + + ecs_add_pair(world, ecs_id(Position), EcsOnInstantiate, EcsInherit); + + ECS_ENTITY(world, Base, Position); + + Probe ctx = { 0 }; + ecs_set_ctx(world, &ctx, NULL); + + ecs_entity_t e = ecs_new_w_pair(world, EcsIsA, Base); + ecs_add(world, e, Position); + + ECS_OBSERVER(world, OnPosition, EcsOnSet, Position); + + ecs_delete(world, e); + test_int(ctx.invoked, 0); + + ecs_fini(world); +} + +void ObserverOnSet_on_set_w_override_after_clear(void) { + ecs_world_t *world = ecs_mini(); + + ECS_COMPONENT(world, Position); + + ecs_add_pair(world, ecs_id(Position), EcsOnInstantiate, EcsInherit); + + ECS_ENTITY(world, Base, Position); + + Probe ctx = { 0 }; + ecs_set_ctx(world, &ctx, NULL); + + ecs_entity_t e = ecs_new_w_pair(world, EcsIsA, Base); + ecs_add(world, e, Position); + + ECS_OBSERVER(world, OnPosition, EcsOnSet, Position); + + ecs_clear(world, e); + test_int(ctx.invoked, 0); + + ecs_fini(world); +} + +void ObserverOnSet_on_set_w_override_after_delete_w_ecs_init(void) { + ecs_world_t *world = ecs_init(); + + ECS_COMPONENT(world, Position); + + ecs_add_pair(world, ecs_id(Position), EcsOnInstantiate, EcsInherit); + + ECS_ENTITY(world, Base, Position); + + Probe ctx = { 0 }; + ecs_set_ctx(world, &ctx, NULL); + + ecs_entity_t e = ecs_new_w_pair(world, EcsIsA, Base); + ecs_add(world, e, Position); + + ECS_OBSERVER(world, OnPosition, EcsOnSet, Position); + + ecs_delete(world, e); + test_int(ctx.invoked, 0); + + ecs_fini(world); +} + void ObserverOnSet_no_set_after_remove_base(void) { ecs_world_t *world = ecs_mini(); diff --git a/test/core/src/main.c b/test/core/src/main.c index 9cc8bafa27..36e7ac222a 100644 --- a/test/core/src/main.c +++ b/test/core/src/main.c @@ -1610,6 +1610,10 @@ void ObserverOnSet_add_base_to_1_overridden(void); void ObserverOnSet_add_base_to_2_overridden(void); void ObserverOnSet_add_base_to_1_of_2_overridden(void); void ObserverOnSet_on_set_after_remove_override(void); +void ObserverOnSet_on_set_after_remove_override_isa_before_add(void); +void ObserverOnSet_on_set_w_override_after_delete(void); +void ObserverOnSet_on_set_w_override_after_clear(void); +void ObserverOnSet_on_set_w_override_after_delete_w_ecs_init(void); void ObserverOnSet_no_set_after_remove_base(void); void ObserverOnSet_un_set_after_remove(void); void ObserverOnSet_un_set_after_remove_base(void); @@ -8502,6 +8506,22 @@ bake_test_case ObserverOnSet_testcases[] = { "on_set_after_remove_override", ObserverOnSet_on_set_after_remove_override }, + { + "on_set_after_remove_override_isa_before_add", + ObserverOnSet_on_set_after_remove_override_isa_before_add + }, + { + "on_set_w_override_after_delete", + ObserverOnSet_on_set_w_override_after_delete + }, + { + "on_set_w_override_after_clear", + ObserverOnSet_on_set_w_override_after_clear + }, + { + "on_set_w_override_after_delete_w_ecs_init", + ObserverOnSet_on_set_w_override_after_delete_w_ecs_init + }, { "no_set_after_remove_base", ObserverOnSet_no_set_after_remove_base @@ -11237,7 +11257,7 @@ static bake_test_suite suites[] = { "ObserverOnSet", NULL, NULL, - 21, + 25, ObserverOnSet_testcases }, { diff --git a/test/cpp/project.json b/test/cpp/project.json index bba6a48876..3c75642c23 100644 --- a/test/cpp/project.json +++ b/test/cpp/project.json @@ -946,7 +946,11 @@ "other_table_w_pair_wildcard", "on_add_inherited", "on_set_inherited", - "on_remove_inherited" + "on_remove_inherited", + "on_set_after_remove_override", + "on_set_after_remove_override_create_observer_before", + "on_set_w_override_after_delete", + "on_set_w_override_after_clear" ] }, { "id": "ComponentLifecycle", diff --git a/test/cpp/src/Observer.cpp b/test/cpp/src/Observer.cpp index f50271f65e..02958d9c82 100644 --- a/test/cpp/src/Observer.cpp +++ b/test/cpp/src/Observer.cpp @@ -1370,3 +1370,107 @@ void Observer_on_remove_inherited(void) { test_int(count, 1); test_assert(i == matched); } + +void Observer_on_set_after_remove_override(void) { + flecs::world ecs; + + ecs.component().add(flecs::OnInstantiate, flecs::Inherit); + + auto base = ecs.prefab() + .set(Position{1, 2}); + + auto e1 = ecs.entity().is_a(base) + .set(Position{10, 20}); + + int count = 0; + + ecs.observer() + .event(flecs::OnSet) + .each([&](flecs::iter& it, size_t row, Position& p) { + test_assert(it.entity(row) == e1); + test_assert(it.src(0) == base); + test_int(p.x, 1); + test_int(p.y, 2); + count ++; + }); + + e1.remove(); + + test_int(count, 1); +} + +void Observer_on_set_after_remove_override_create_observer_before(void) { + flecs::world ecs; + + ecs.component().add(flecs::OnInstantiate, flecs::Inherit); + + int count = 0; + + flecs::entity base = ecs.prefab(); + flecs::entity e1 = ecs.entity(); + + ecs.observer() + .event(flecs::OnSet) + .each([&](flecs::iter& it, size_t row, Position& p) { + test_assert(it.entity(row) == e1); + test_assert(it.src(0) == base); + count ++; + }); + + base.set(Position{1, 2}); + e1.add().is_a(base); + + test_int(count, 0); + + e1.remove(); + + test_int(count, 1); +} + +void Observer_on_set_w_override_after_delete(void) { + flecs::world ecs; + + ecs.component().add(flecs::OnInstantiate, flecs::Inherit); + + auto base = ecs.prefab() + .set(Position{1, 2}); + + auto e1 = ecs.entity().is_a(base) + .set(Position{10, 20}); + + int count = 0; + + ecs.observer() + .event(flecs::OnSet) + .each([&](flecs::iter& it, size_t row, Position& p) { + count ++; + }); + + e1.destruct(); + + test_int(count, 0); +} + +void Observer_on_set_w_override_after_clear(void) { + flecs::world ecs; + + ecs.component().add(flecs::OnInstantiate, flecs::Inherit); + + auto base = ecs.prefab() + .set(Position{1, 2}); + + auto e1 = ecs.entity().is_a(base) + .set(Position{10, 20}); + + int count = 0; + + ecs.observer() + .event(flecs::OnSet) + .each([&](flecs::iter& it, size_t row, Position& p) { + count ++; + }); + + e1.clear(); + + test_int(count, 0); +} diff --git a/test/cpp/src/main.cpp b/test/cpp/src/main.cpp index b7981d8666..3e4da779cc 100644 --- a/test/cpp/src/main.cpp +++ b/test/cpp/src/main.cpp @@ -914,6 +914,10 @@ 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); +void Observer_on_set_after_remove_override(void); +void Observer_on_set_after_remove_override_create_observer_before(void); +void Observer_on_set_w_override_after_delete(void); +void Observer_on_set_w_override_after_clear(void); // Testsuite 'ComponentLifecycle' void ComponentLifecycle_ctor_on_add(void); @@ -4938,6 +4942,22 @@ bake_test_case Observer_testcases[] = { { "on_remove_inherited", Observer_on_remove_inherited + }, + { + "on_set_after_remove_override", + Observer_on_set_after_remove_override + }, + { + "on_set_after_remove_override_create_observer_before", + Observer_on_set_after_remove_override_create_observer_before + }, + { + "on_set_w_override_after_delete", + Observer_on_set_w_override_after_delete + }, + { + "on_set_w_override_after_clear", + Observer_on_set_w_override_after_clear } }; @@ -6846,7 +6866,7 @@ static bake_test_suite suites[] = { "Observer", NULL, NULL, - 55, + 59, Observer_testcases }, {