#1400 Fix issue with getting field in OnSet observer after removing o…

…verride
SanderMertens · Oct 15, 2024 · 2bf5eb3 · 2bf5eb3
1 parent 6b8112e
commit 2bf5eb3
Show file tree

Hide file tree

Showing 8 changed files with 268 additions and 9 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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<Position>().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<Position>()
+        .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<Position>();
+
+    test_int(count, 1);
+}
+
+void Observer_on_set_after_remove_override_create_observer_before(void) {
+    flecs::world ecs;
+
+    ecs.component<Position>().add(flecs::OnInstantiate, flecs::Inherit);
+
+    int count = 0;
+
+    flecs::entity base = ecs.prefab();
+    flecs::entity e1 = ecs.entity();
+
+    ecs.observer<Position>()
+        .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<Position>().is_a(base);
+
+    test_int(count, 0);
+
+    e1.remove<Position>();
+
+    test_int(count, 1);
+}
+
+void Observer_on_set_w_override_after_delete(void) {
+    flecs::world ecs;
+
+    ecs.component<Position>().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<Position>()
+        .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<Position>().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<Position>()
+        .event(flecs::OnSet)
+        .each([&](flecs::iter& it, size_t row, Position& p) {
+            count ++;
+        });
+
+    e1.clear();
+
+    test_int(count, 0);
+}