Fix issue where regular entities would be cleaned up after component …

…entities

flecs.c

@@ -21976,22 +21976,17 @@ void flecs_clean_tables(
 }
 
 static
-void flecs_fini_roots(ecs_world_t *world) {
-    ecs_id_record_t *idr = flecs_id_record_get(world, ecs_pair(EcsChildOf, 0));
-
-    ecs_run_aperiodic(world, EcsAperiodicEmptyTables);
-
+void flecs_fini_root_tables(
+    ecs_world_t *world,
+    ecs_id_record_t *idr,
+    bool fini_targets)
+{
     ecs_table_cache_iter_t it;
+
     bool has_roots = flecs_table_cache_iter(&idr->cache, &it);
     ecs_assert(has_roots == true, ECS_INTERNAL_ERROR, NULL);
     (void)has_roots;
 
-    /* Delete root entities that are not modules. This prioritizes deleting 
-     * regular entities first, which reduces the chance of components getting
-     * destructed in random order because it got deleted before entities,
-     * thereby bypassing the OnDeleteTarget policy. */
-    flecs_defer_begin(world, &world->stages[0]);
-
     const ecs_table_record_t *tr;
     while ((tr = flecs_table_cache_next(&it, ecs_table_record_t))) {
         ecs_table_t *table = tr->hdr.table;
@@ -22002,21 +21997,49 @@ void flecs_fini_roots(ecs_world_t *world) {
         int32_t i, count = table->data.entities.count;
         ecs_entity_t *entities = table->data.entities.array;
 
-        /* Count backwards so that we're always deleting the last entity in the
-         * table which reduces moving components around */
-        for (i = count - 1; i >= 0; i --) {
-            ecs_record_t *r = flecs_entities_get(world, entities[i]);
-            ecs_assert(r != NULL, ECS_INTERNAL_ERROR, NULL);
-
-            ecs_flags32_t flags = ECS_RECORD_TO_ROW_FLAGS(r->row);
-            if (!(flags & EcsEntityIsTarget)) {
-                continue; /* Filter out entities that aren't objects */
+        if (fini_targets) {
+            /* Only delete entities that are used as pair target. Iterate 
+             * backwards to minimize moving entities around in table. */
+            for (i = count - 1; i >= 0; i --) {
+                ecs_record_t *r = flecs_entities_get(world, entities[i]);
+                ecs_assert(r != NULL, ECS_INTERNAL_ERROR, NULL);
+                if (ECS_RECORD_TO_ROW_FLAGS(r->row) & EcsEntityIsTarget) {
+                    ecs_delete(world, entities[i]);
+                }
+            }
+        } else {
+            /* Delete remaining entities that are not in use (added to another
+             * entity). This limits table moves during cleanup and delays 
+             * cleanup of tags. */
+            for (i = count - 1; i >= 0; i --) {
+                ecs_record_t *r = flecs_entities_get(world, entities[i]);
+                ecs_assert(r != NULL, ECS_INTERNAL_ERROR, NULL);
+                if (!ECS_RECORD_TO_ROW_FLAGS(r->row)) {
+                    ecs_delete(world, entities[i]);
+                }
             }
-
-            ecs_delete(world, entities[i]);
         }
     }
+}
+
+static
+void flecs_fini_roots(
+    ecs_world_t *world) 
+{
+    ecs_id_record_t *idr = flecs_id_record_get(world, ecs_pair(EcsChildOf, 0));
 
+    ecs_run_aperiodic(world, EcsAperiodicEmptyTables);
+
+    /* Delete root entities that are not modules. This prioritizes deleting 
+     * regular entities first, which reduces the chance of components getting
+     * destructed in random order because it got deleted before entities,
+     * thereby bypassing the OnDeleteTarget policy. */
+    flecs_defer_begin(world, &world->stages[0]);
+    flecs_fini_root_tables(world, idr, true);
+    flecs_defer_end(world, &world->stages[0]);
+
+    flecs_defer_begin(world, &world->stages[0]);
+    flecs_fini_root_tables(world, idr, false);
     flecs_defer_end(world, &world->stages[0]);
 }
 

flecs.h

@@ -11213,7 +11213,7 @@ ecs_entity_t ecs_system_init(
  * Any system may interrupt execution by setting the interrupted_by member in
  * the ecs_iter_t value. This is particularly useful for manual systems, where
  * the value of interrupted_by is returned by this operation. This, in
- * combination with the param argument lets applications use manual systems
+ * cominbation with the param argument lets applications use manual systems
  * to lookup entities: once the entity has been found its handle is passed to
  * interrupted_by, which is then subsequently returned.
  *

src/world.c

@@ -592,22 +592,17 @@ void flecs_clean_tables(
 }
 
 static
-void flecs_fini_roots(ecs_world_t *world) {
-    ecs_id_record_t *idr = flecs_id_record_get(world, ecs_pair(EcsChildOf, 0));
-
-    ecs_run_aperiodic(world, EcsAperiodicEmptyTables);
-
+void flecs_fini_root_tables(
+    ecs_world_t *world,
+    ecs_id_record_t *idr,
+    bool fini_targets)
+{
     ecs_table_cache_iter_t it;
+
     bool has_roots = flecs_table_cache_iter(&idr->cache, &it);
     ecs_assert(has_roots == true, ECS_INTERNAL_ERROR, NULL);
     (void)has_roots;
 
-    /* Delete root entities that are not modules. This prioritizes deleting 
-     * regular entities first, which reduces the chance of components getting
-     * destructed in random order because it got deleted before entities,
-     * thereby bypassing the OnDeleteTarget policy. */
-    flecs_defer_begin(world, &world->stages[0]);
-
     const ecs_table_record_t *tr;
     while ((tr = flecs_table_cache_next(&it, ecs_table_record_t))) {
         ecs_table_t *table = tr->hdr.table;
@@ -618,21 +613,49 @@ void flecs_fini_roots(ecs_world_t *world) {
         int32_t i, count = table->data.entities.count;
         ecs_entity_t *entities = table->data.entities.array;
 
-        /* Count backwards so that we're always deleting the last entity in the
-         * table which reduces moving components around */
-        for (i = count - 1; i >= 0; i --) {
-            ecs_record_t *r = flecs_entities_get(world, entities[i]);
-            ecs_assert(r != NULL, ECS_INTERNAL_ERROR, NULL);
-
-            ecs_flags32_t flags = ECS_RECORD_TO_ROW_FLAGS(r->row);
-            if (!(flags & EcsEntityIsTarget)) {
-                continue; /* Filter out entities that aren't objects */
+        if (fini_targets) {
+            /* Only delete entities that are used as pair target. Iterate 
+             * backwards to minimize moving entities around in table. */
+            for (i = count - 1; i >= 0; i --) {
+                ecs_record_t *r = flecs_entities_get(world, entities[i]);
+                ecs_assert(r != NULL, ECS_INTERNAL_ERROR, NULL);
+                if (ECS_RECORD_TO_ROW_FLAGS(r->row) & EcsEntityIsTarget) {
+                    ecs_delete(world, entities[i]);
+                }
+            }
+        } else {
+            /* Delete remaining entities that are not in use (added to another
+             * entity). This limits table moves during cleanup and delays 
+             * cleanup of tags. */
+            for (i = count - 1; i >= 0; i --) {
+                ecs_record_t *r = flecs_entities_get(world, entities[i]);
+                ecs_assert(r != NULL, ECS_INTERNAL_ERROR, NULL);
+                if (!ECS_RECORD_TO_ROW_FLAGS(r->row)) {
+                    ecs_delete(world, entities[i]);
+                }
             }
-
-            ecs_delete(world, entities[i]);
         }
     }
+}
+
+static
+void flecs_fini_roots(
+    ecs_world_t *world) 
+{
+    ecs_id_record_t *idr = flecs_id_record_get(world, ecs_pair(EcsChildOf, 0));
+
+    ecs_run_aperiodic(world, EcsAperiodicEmptyTables);
 
+    /* Delete root entities that are not modules. This prioritizes deleting 
+     * regular entities first, which reduces the chance of components getting
+     * destructed in random order because it got deleted before entities,
+     * thereby bypassing the OnDeleteTarget policy. */
+    flecs_defer_begin(world, &world->stages[0]);
+    flecs_fini_root_tables(world, idr, true);
+    flecs_defer_end(world, &world->stages[0]);
+
+    flecs_defer_begin(world, &world->stages[0]);
+    flecs_fini_root_tables(world, idr, false);
     flecs_defer_end(world, &world->stages[0]);
 }
 

test/api/project.json

@@ -789,6 +789,9 @@
                 "add_deleted_in_on_remove",
                 "delete_tree_w_query",
                 "fini_cleanup_order",
+                "fini_cleanup_order_root_id_w_trait",
+                "fini_cleanup_order_entity_after_singleton",
+                "fini_cleanup_order_entity_after_component",
                 "on_delete_parent_w_in_use_id_w_remove",
                 "on_delete_parent_w_in_use_id_w_delete",
                 "create_after_delete_with",

test/api/src/Observer.c

@@ -1838,15 +1838,14 @@ void Observer_unset_on_fini_1(void) {
 
     ecs_fini(world);
 
-    test_int(ctx.invoked, 1);
     test_int(ctx.count, 3);
     test_int(ctx.system, UnSet);
     test_int(ctx.term_count, 1);
     test_null(ctx.param);
 
-    test_int(ctx.e[0], e1);
+    test_int(ctx.e[0], e3);
     test_int(ctx.e[1], e2);
-    test_int(ctx.e[2], e3);
+    test_int(ctx.e[2], e1);
 
     test_int(ctx.c[0][0], ecs_id(Position));
     test_int(ctx.s[0][0], 0);      
@@ -1887,9 +1886,9 @@ void Observer_unset_on_fini_2(void) {
     test_int(ctx.term_count, 2);
     test_null(ctx.param);
 
-    test_int(ctx.e[0], e1);
+    test_int(ctx.e[0], e3);
     test_int(ctx.e[1], e2);
-    test_int(ctx.e[2], e3);
+    test_int(ctx.e[2], e1);
 
     test_int(ctx.c[0][0], ecs_id(Position));
     test_int(ctx.s[0][0], 0); 
@@ -1934,15 +1933,14 @@ void Observer_unset_on_fini_3(void) {
 
     ecs_fini(world);
 
-    test_int(ctx.invoked, 1);
     test_int(ctx.count, 3);
     test_int(ctx.system, UnSet);
     test_int(ctx.term_count, 3);
     test_null(ctx.param);
 
-    test_int(ctx.e[0], e1);
+    test_int(ctx.e[0], e3);
     test_int(ctx.e[1], e2);
-    test_int(ctx.e[2], e3);
+    test_int(ctx.e[2], e1);
 
     test_int(ctx.c[0][0], ecs_id(Position));
     test_int(ctx.s[0][0], 0); 

test/api/src/OnDelete.c

@@ -2727,6 +2727,67 @@ void OnDelete_fini_cleanup_order(void) {
     test_int(test_alive_parent_count, 3);
 }
 
+void OnDelete_fini_cleanup_order_root_id_w_trait(void) {
+    ecs_world_t *world = ecs_mini();
+
+    ecs_entity_t r = ecs_new_id(world);
+    ecs_add_id(world, r, EcsExclusive);
+    ecs_entity_t t = ecs_new_id(world);
+
+    ecs_entity_t e = ecs_new_id(world);
+    ecs_add_pair(world, e, r, t);
+
+    ecs_fini(world);
+
+    test_assert(true); // fini should not assert
+}
+
+static ECS_COMPONENT_DECLARE(Position);
+
+static int on_remove_velocity = 0;
+static void ecs_on_remove(Velocity)(ecs_iter_t *it) {
+    on_remove_velocity ++;
+    test_assert(ecs_is_alive(it->world, ecs_id(Position)));
+}
+
+void OnDelete_fini_cleanup_order_entity_after_singleton(void) {
+    ecs_world_t *world = ecs_mini();
+
+    ECS_COMPONENT_DEFINE(world, Position);
+    ECS_COMPONENT(world, Velocity);
+
+    ecs_set_hooks(world, Velocity, {
+        .on_remove = ecs_on_remove(Velocity)
+    });
+
+    ecs_singleton_set(world, Position, {10, 20});
+
+    ecs_entity_t e = ecs_new_id(world);
+    ecs_add(world, e, Velocity);
+
+    ecs_fini(world);
+
+    test_int(on_remove_velocity, 1);
+}
+
+void OnDelete_fini_cleanup_order_entity_after_component(void) {
+    ecs_world_t *world = ecs_mini();
+
+    ECS_COMPONENT_DEFINE(world, Position);
+    ECS_COMPONENT(world, Velocity);
+
+    ecs_set_hooks(world, Velocity, {
+        .on_remove = ecs_on_remove(Velocity)
+    });
+
+    ecs_entity_t e = ecs_new_id(world);
+    ecs_add(world, e, Velocity);
+
+    ecs_fini(world);
+
+    test_int(on_remove_velocity, 1);
+}
+
 void OnDelete_on_delete_parent_w_in_use_id_w_remove(void) {
     ecs_world_t *world = ecs_mini();
 
@@ -3060,3 +3121,4 @@ void OnDelete_delete_w_low_rel_mixed_cleanup_interleaved_ids(void) {
 
     ecs_fini(world);
 }
+

test/api/src/main.c

@@ -748,6 +748,9 @@ void OnDelete_delete_nested_in_on_remove(void);
 void OnDelete_add_deleted_in_on_remove(void);
 void OnDelete_delete_tree_w_query(void);
 void OnDelete_fini_cleanup_order(void);
+void OnDelete_fini_cleanup_order_root_id_w_trait(void);
+void OnDelete_fini_cleanup_order_entity_after_singleton(void);
+void OnDelete_fini_cleanup_order_entity_after_component(void);
 void OnDelete_on_delete_parent_w_in_use_id_w_remove(void);
 void OnDelete_on_delete_parent_w_in_use_id_w_delete(void);
 void OnDelete_create_after_delete_with(void);
@@ -5467,6 +5470,18 @@ bake_test_case OnDelete_testcases[] = {
         "fini_cleanup_order",
         OnDelete_fini_cleanup_order
     },
+    {
+        "fini_cleanup_order_root_id_w_trait",
+        OnDelete_fini_cleanup_order_root_id_w_trait
+    },
+    {
+        "fini_cleanup_order_entity_after_singleton",
+        OnDelete_fini_cleanup_order_entity_after_singleton
+    },
+    {
+        "fini_cleanup_order_entity_after_component",
+        OnDelete_fini_cleanup_order_entity_after_component
+    },
     {
         "on_delete_parent_w_in_use_id_w_remove",
         OnDelete_on_delete_parent_w_in_use_id_w_remove
@@ -12884,7 +12899,7 @@ static bake_test_suite suites[] = {
         "OnDelete",
         NULL,
         NULL,
-        110,
+        113,
         OnDelete_testcases
     },
     {