SanderMertens#1975 Fix issue with observer up traversal and parent hierarchies

Author: SanderMertens

distr/flecs.c

@@ -20836,26 +20836,28 @@ void flecs_spawner_instantiate(
             .added_flags = flags
         };
 
-        int32_t row = ecs_table_count(table);
-        r->table = table;
-        r->row = (uint32_t)row;
-        flecs_table_append(world, table, entity, true, true);
-        flecs_actions_new(world, table, row, 1, &table_diff, 0, false, true);
-
         ecs_entity_t parent = parents[spawn_child->parent_index];
+        ecs_assert(parent != 0, ECS_INTERNAL_ERROR, NULL);
         if (parent != old_parent) {
             cr = flecs_components_ensure(world, ecs_childof(parent));
             old_parent = parent;
         }
 
         ecs_assert(cr != NULL, ECS_INTERNAL_ERROR, NULL);
 
+        int32_t row = ecs_table_count(table);
+        r->table = table;
+        r->row = (uint32_t)row;
+        flecs_table_append(world, table, entity, true, true);
+
         int32_t parent_column = table->component_map[ecs_id(EcsParent)];
         ecs_assert(parent_column != 0, ECS_INTERNAL_ERROR, NULL);
         EcsParent *parent_ptr = table->data.columns[parent_column - 1].data;
         parent_ptr = &parent_ptr[row];
         parent_ptr->value = parent;
 
+        flecs_actions_new(world, table, row, 1, &table_diff, 0, false, true);
+
         if (is_prefab && spawn_child->child_name) {
             ecs_set_name(world, entity, spawn_child->child_name);
         }

src/tree_spawner.c

@@ -279,26 +279,28 @@ void flecs_spawner_instantiate(
             .added_flags = flags
         };
 
-        int32_t row = ecs_table_count(table);
-        r->table = table;
-        r->row = (uint32_t)row;
-        flecs_table_append(world, table, entity, true, true);
-        flecs_actions_new(world, table, row, 1, &table_diff, 0, false, true);
-
         ecs_entity_t parent = parents[spawn_child->parent_index];
+        ecs_assert(parent != 0, ECS_INTERNAL_ERROR, NULL);
         if (parent != old_parent) {
             cr = flecs_components_ensure(world, ecs_childof(parent));
             old_parent = parent;
         }
 
         ecs_assert(cr != NULL, ECS_INTERNAL_ERROR, NULL);
 
+        int32_t row = ecs_table_count(table);
+        r->table = table;
+        r->row = (uint32_t)row;
+        flecs_table_append(world, table, entity, true, true);
+
         int32_t parent_column = table->component_map[ecs_id(EcsParent)];
         ecs_assert(parent_column != 0, ECS_INTERNAL_ERROR, NULL);
         EcsParent *parent_ptr = table->data.columns[parent_column - 1].data;
         parent_ptr = &parent_ptr[row];
         parent_ptr->value = parent;
 
+        flecs_actions_new(world, table, row, 1, &table_diff, 0, false, true);
+
         if (is_prefab && spawn_child->child_name) {
             ecs_set_name(world, entity, spawn_child->child_name);
         }

test/core/project.json

@@ -863,6 +863,7 @@
                 "instantiate_tree_1_child",
                 "instantiate_tree_2_children",
                 "instantiate_tree_3_children",
+                "instantiate_tree_w_on_set_up_childof_observer_crash",
                 "instantiate_w_dont_inherit",
                 "instantiate_w_inherit",
                 "instantiate_w_auto_override",

test/core/src/NonFragmentingChildOf.c

@@ -2,6 +2,32 @@
 
 static ECS_COMPONENT_DECLARE(Position);
 
+typedef struct ReproObserverCtx {
+    int32_t invoked;
+    ecs_entity_t entity;
+    ecs_entity_t source;
+    Position child_position;
+    Position parent_position;
+} ReproObserverCtx;
+
+static ReproObserverCtx repro_observer_ctx;
+
+static
+void ReproDummyObserver(ecs_iter_t *it) {
+    test_int(it->count, 1);
+
+    Position *child_position = ecs_field(it, Position, 0);
+    Position *parent_position = ecs_field(it, Position, 1);
+    test_assert(child_position != NULL);
+    test_assert(parent_position != NULL);
+
+    repro_observer_ctx.invoked ++;
+    repro_observer_ctx.entity = it->entities[0];
+    repro_observer_ctx.source = it->sources[1];
+    repro_observer_ctx.child_position = child_position[0];
+    repro_observer_ctx.parent_position = parent_position[0];
+}
+
 void NonFragmentingChildOf_set_parent_no_ordered_children(void) {
     ecs_world_t *world = ecs_mini();
 
@@ -3147,6 +3173,49 @@ void NonFragmentingChildOf_instantiate_tree_3_children(void) {
     ecs_fini(world);
 }
 
+void NonFragmentingChildOf_instantiate_tree_w_on_set_up_childof_observer_crash(void) {
+    ecs_world_t *world = ecs_init();
+
+    ECS_COMPONENT(world, Position);
+
+    ecs_entity_t prefab = ecs_new_w_id(world, EcsPrefab);
+    ecs_set(world, prefab, Position, {10, 20});
+
+    ecs_entity_t prefab_child = ecs_new_w_parent(world, prefab, "Child");
+    ecs_set(world, prefab_child, Position, {11, 21});
+
+    ecs_os_zeromem(&repro_observer_ctx);
+
+    ecs_observer(world, {
+        .query.expr = "Position, Position(up ChildOf)",
+        .events = { EcsOnSet },
+        .callback = ReproDummyObserver
+    });
+
+    ecs_entity_t instance = ecs_new_w_pair(world, EcsIsA, prefab);
+    test_assert(instance != 0);
+
+    ecs_entities_t children = ecs_get_ordered_children(world, instance);
+    test_int(children.count, 1);
+    ecs_entity_t instance_child = children.ids[0];
+    test_assert(instance_child != 0);
+
+    const Position *p = ecs_get(world, instance_child, Position);
+    test_assert(p != NULL);
+    test_int(p->x, 11);
+    test_int(p->y, 21);
+
+    test_int(repro_observer_ctx.invoked, 1);
+    test_uint(repro_observer_ctx.entity, instance_child);
+    test_uint(repro_observer_ctx.source, instance);
+    test_int(repro_observer_ctx.child_position.x, 11);
+    test_int(repro_observer_ctx.child_position.y, 21);
+    test_int(repro_observer_ctx.parent_position.x, 10);
+    test_int(repro_observer_ctx.parent_position.y, 20);
+
+    ecs_fini(world);
+}
+
 void NonFragmentingChildOf_instantiate_w_dont_inherit(void) {
     ecs_world_t *world = ecs_mini();
 

test/core/src/main.c

@@ -831,6 +831,7 @@ void NonFragmentingChildOf_prefab_variant_w_nested_children(void);
 void NonFragmentingChildOf_instantiate_tree_1_child(void);
 void NonFragmentingChildOf_instantiate_tree_2_children(void);
 void NonFragmentingChildOf_instantiate_tree_3_children(void);
+void NonFragmentingChildOf_instantiate_tree_w_on_set_up_childof_observer_crash(void);
 void NonFragmentingChildOf_instantiate_w_dont_inherit(void);
 void NonFragmentingChildOf_instantiate_w_inherit(void);
 void NonFragmentingChildOf_instantiate_w_auto_override(void);
@@ -6428,6 +6429,10 @@ bake_test_case NonFragmentingChildOf_testcases[] = {
         "instantiate_tree_3_children",
         NonFragmentingChildOf_instantiate_tree_3_children
     },
+    {
+        "instantiate_tree_w_on_set_up_childof_observer_crash",
+        NonFragmentingChildOf_instantiate_tree_w_on_set_up_childof_observer_crash
+    },
     {
         "instantiate_w_dont_inherit",
         NonFragmentingChildOf_instantiate_w_dont_inherit
@@ -15795,7 +15800,7 @@ static bake_test_suite suites[] = {
         "NonFragmentingChildOf",
         NULL,
         NULL,
-        235,
+        236,
         NonFragmentingChildOf_testcases
     },
     {