#2004 Fix issue where observers were not always called for deferred DontFragment operations

Author: SanderMertens

distr/flecs.c

@@ -6526,8 +6526,11 @@ void flecs_cmd_batch_for_entity(
     /* If destination table has new sparse components, make sure they're created
      * for the entity. */
     if ((table_diff.added_flags & (EcsTableHasSparse|EcsTableHasDontFragment)) && added.count) {
-        flecs_sparse_on_add(
-            world, table, ECS_RECORD_TO_ROW(r->row), 1, &added, true);
+        if (flecs_sparse_on_add(
+            world, table, ECS_RECORD_TO_ROW(r->row), 1, &added, true))
+        {
+            table_diff.added_flags |= EcsTableHasOnAdd;
+        }
     }
 
     /* If the batch contains set commands, copy the component value from the 
@@ -6626,7 +6629,7 @@ void flecs_cmd_batch_for_entity(
     if (added.count) {
         ecs_table_diff_t add_diff = ECS_TABLE_DIFF_INIT;
         add_diff.added = added;
-        add_diff.added_flags = diff->added_flags;
+        add_diff.added_flags = table_diff.added_flags;
 
         if (r->row & EcsEntityIsTraversable) {
             /* Update monitors since we didn't do this in flecs_commit. Do this

src/commands.c

@@ -943,8 +943,11 @@ void flecs_cmd_batch_for_entity(
     /* If destination table has new sparse components, make sure they're created
      * for the entity. */
     if ((table_diff.added_flags & (EcsTableHasSparse|EcsTableHasDontFragment)) && added.count) {
-        flecs_sparse_on_add(
-            world, table, ECS_RECORD_TO_ROW(r->row), 1, &added, true);
+        if (flecs_sparse_on_add(
+            world, table, ECS_RECORD_TO_ROW(r->row), 1, &added, true))
+        {
+            table_diff.added_flags |= EcsTableHasOnAdd;
+        }
     }
 
     /* If the batch contains set commands, copy the component value from the 
@@ -1043,7 +1046,7 @@ void flecs_cmd_batch_for_entity(
     if (added.count) {
         ecs_table_diff_t add_diff = ECS_TABLE_DIFF_INIT;
         add_diff.added = added;
-        add_diff.added_flags = diff->added_flags;
+        add_diff.added_flags = table_diff.added_flags;
 
         if (r->row & EcsEntityIsTraversable) {
             /* Update monitors since we didn't do this in flecs_commit. Do this

test/core/project.json

@@ -738,7 +738,9 @@
                 "check_regular_exclusive_target_in_sparse_observer",
                 "child_of_component_w_sparse",
                 "child_of_component_w_sparse_exclusive",
-                "create_entity_in_on_remove"
+                "create_entity_in_on_remove",
+                "defer_add_two_sparse_w_observer",
+                "defer_remove_two_sparse_w_observer"
             ]
         }, {
             "id": "NonFragmentingChildOf",

test/core/src/Sparse.c

@@ -7227,3 +7227,100 @@ void Sparse_create_entity_in_on_remove(void) {
 
     ecs_fini(world);
 }
+
+static int OnAddTagA_invoked = 0;
+static int OnAddTagB_invoked = 0;
+static int OnRemoveTagA_invoked = 0;
+static int OnRemoveTagB_invoked = 0;
+
+static void OnAddTagA(ecs_iter_t *it) {
+    OnAddTagA_invoked += it->count;
+}
+
+static void OnAddTagB(ecs_iter_t *it) {
+    OnAddTagB_invoked += it->count;
+}
+
+static void OnRemoveTagA(ecs_iter_t *it) {
+    OnRemoveTagA_invoked += it->count;
+}
+
+static void OnRemoveTagB(ecs_iter_t *it) {
+    OnRemoveTagB_invoked += it->count;
+}
+
+void Sparse_defer_add_two_sparse_w_observer(void) {
+    ecs_world_t *world = ecs_mini();
+
+    ecs_entity_t DontFragA = ecs_entity(world, { .name = "DontFragA" });
+    if (!fragment) ecs_add_id(world, DontFragA, EcsDontFragment);
+
+    ecs_entity_t DontFragB = ecs_entity(world, { .name = "DontFragB" });
+    if (!fragment) ecs_add_id(world, DontFragB, EcsDontFragment);
+
+    ecs_observer(world, {
+        .query.terms = {{ .id = DontFragA }},
+        .events = { EcsOnAdd },
+        .callback = OnAddTagA
+    });
+
+    ecs_observer(world, {
+        .query.terms = {{ .id = DontFragB }},
+        .events = { EcsOnAdd },
+        .callback = OnAddTagB
+    });
+
+    OnAddTagA_invoked = 0;
+    OnAddTagB_invoked = 0;
+
+    ecs_entity_t e = ecs_new(world);
+
+    ecs_defer_begin(world);
+    ecs_add_id(world, e, DontFragA);
+    ecs_add_id(world, e, DontFragB);
+    ecs_defer_end(world);
+
+    test_int(OnAddTagA_invoked, 1);
+    test_int(OnAddTagB_invoked, 1);
+
+    ecs_fini(world);
+}
+
+void Sparse_defer_remove_two_sparse_w_observer(void) {
+    ecs_world_t *world = ecs_mini();
+
+    ecs_entity_t DontFragA = ecs_entity(world, { .name = "DontFragA" });
+    if (!fragment) ecs_add_id(world, DontFragA, EcsDontFragment);
+
+    ecs_entity_t DontFragB = ecs_entity(world, { .name = "DontFragB" });
+    if (!fragment) ecs_add_id(world, DontFragB, EcsDontFragment);
+
+    ecs_observer(world, {
+        .query.terms = {{ .id = DontFragA }},
+        .events = { EcsOnRemove },
+        .callback = OnRemoveTagA
+    });
+
+    ecs_observer(world, {
+        .query.terms = {{ .id = DontFragB }},
+        .events = { EcsOnRemove },
+        .callback = OnRemoveTagB
+    });
+
+    OnRemoveTagA_invoked = 0;
+    OnRemoveTagB_invoked = 0;
+
+    ecs_entity_t e = ecs_new(world);
+    ecs_add_id(world, e, DontFragA);
+    ecs_add_id(world, e, DontFragB);
+
+    ecs_defer_begin(world);
+    ecs_remove_id(world, e, DontFragA);
+    ecs_remove_id(world, e, DontFragB);
+    ecs_defer_end(world);
+
+    test_int(OnRemoveTagA_invoked, 1);
+    test_int(OnRemoveTagB_invoked, 1);
+
+    ecs_fini(world);
+}

test/core/src/main.c

@@ -709,6 +709,8 @@ void Sparse_check_regular_exclusive_target_in_sparse_observer(void);
 void Sparse_child_of_component_w_sparse(void);
 void Sparse_child_of_component_w_sparse_exclusive(void);
 void Sparse_create_entity_in_on_remove(void);
+void Sparse_defer_add_two_sparse_w_observer(void);
+void Sparse_defer_remove_two_sparse_w_observer(void);
 
 // Testsuite 'NonFragmentingChildOf'
 void NonFragmentingChildOf_set_parent_no_ordered_children(void);
@@ -5961,6 +5963,14 @@ bake_test_case Sparse_testcases[] = {
     {
         "create_entity_in_on_remove",
         Sparse_create_entity_in_on_remove
+    },
+    {
+        "defer_add_two_sparse_w_observer",
+        Sparse_defer_add_two_sparse_w_observer
+    },
+    {
+        "defer_remove_two_sparse_w_observer",
+        Sparse_defer_remove_two_sparse_w_observer
     }
 };
 
@@ -15871,7 +15881,7 @@ static bake_test_suite suites[] = {
         "Sparse",
         Sparse_setup,
         NULL,
-        219,
+        221,
         Sparse_testcases,
         1,
         Sparse_params