#1359 Fix issues with hooks for sparse components

SanderMertens · Sep 18, 2024 · 8871d57 · 8871d57
1 parent 486f5d1
commit 8871d57
Show file tree

Hide file tree

Showing 10 changed files with 398 additions and 11 deletions.
diff --git a/distr/flecs.c b/distr/flecs.c
@@ -6251,6 +6251,7 @@ void flecs_invoke_hook(
     it.table = table;
     it.trs[0] = tr;
     it.row_fields = !!(((ecs_id_record_t*)tr->hdr.cache)->flags & EcsIdIsSparse);
+    it.ref_fields = it.row_fields;
     it.sizes = ECS_CONST_CAST(ecs_size_t*, &ti->size);
     it.ids[0] = id;
     it.event = event;
@@ -6260,6 +6261,7 @@ void flecs_invoke_hook(
     it.count = count;
     it.offset = row;
     it.flags = EcsIterIsValid;
+
     hook(&it);
     ecs_iter_fini(&it);
 
@@ -11283,7 +11285,7 @@ void* ecs_field_at_w_size(
 
     ecs_entity_t src = it->sources[index];
     if (!src) {
-        src = ecs_table_entities(it->table)[row];
+        src = ecs_table_entities(it->table)[row + it->offset];
     }
 
     return flecs_sparse_get_any(idr->sparse, flecs_uto(int32_t, size), src);
@@ -34748,7 +34750,7 @@ bool flecs_query_finalize_simple(
             if (idr->flags & EcsIdIsSparse) {
                 term->flags_ |= EcsTermIsSparse;
                 cacheable = false; trivial = false;
-                q->row_fields |= (1llu << i);
+                q->row_fields |= flecs_uto(uint32_t, 1llu << i);
             }
         }
 

diff --git a/src/entity.c b/src/entity.c
@@ -1263,6 +1263,7 @@ void flecs_invoke_hook(
     it.table = table;
     it.trs[0] = tr;
     it.row_fields = !!(((ecs_id_record_t*)tr->hdr.cache)->flags & EcsIdIsSparse);
+    it.ref_fields = it.row_fields;
     it.sizes = ECS_CONST_CAST(ecs_size_t*, &ti->size);
     it.ids[0] = id;
     it.event = event;
@@ -1272,6 +1273,7 @@ void flecs_invoke_hook(
     it.count = count;
     it.offset = row;
     it.flags = EcsIterIsValid;
+
     hook(&it);
     ecs_iter_fini(&it);
 

diff --git a/src/iter.c b/src/iter.c
@@ -184,7 +184,7 @@ void* ecs_field_at_w_size(
 
     ecs_entity_t src = it->sources[index];
     if (!src) {
-        src = ecs_table_entities(it->table)[row];
+        src = ecs_table_entities(it->table)[row + it->offset];
     }
 
     return flecs_sparse_get_any(idr->sparse, flecs_uto(int32_t, size), src);

diff --git a/src/query/validator.c b/src/query/validator.c
@@ -1633,7 +1633,7 @@ bool flecs_query_finalize_simple(
             if (idr->flags & EcsIdIsSparse) {
                 term->flags_ |= EcsTermIsSparse;
                 cacheable = false; trivial = false;
-                q->row_fields |= (1llu << i);
+                q->row_fields |= flecs_uto(uint32_t, 1llu << i);
             }
         }
 

diff --git a/test/core/project.json b/test/core/project.json
@@ -489,6 +489,7 @@
                 "on_add_remove_after_fini",
                 "on_set_after_set",
                 "on_set_after_modified",
+                "on_set_at_offset",
                 "on_add_observer",
                 "on_set_observer_set",
                 "on_set_observer_modified",

diff --git a/test/core/src/Sparse.c b/test/core/src/Sparse.c
@@ -807,6 +807,7 @@ static int position_dtor_invoked = 0;
 static int position_on_add_invoked = 0;
 static int position_on_remove_invoked = 0;
 static int position_on_set_invoked = 0;
+static Position position_on_set_value = {0, 0};
 
 static ECS_CTOR(Position, ptr, {
     position_ctor_invoked ++;
@@ -847,8 +848,7 @@ static void Position_on_set(ecs_iter_t *it) {
     test_int(0, position_dtor_invoked);
     Position *p = ecs_field_at(it, Position, 0, 0);
     test_assert(p != NULL);
-    test_int(p->x, 30);
-    test_int(p->y, 40);
+    position_on_set_value = *p;
     test_uint(it->event, EcsOnSet);
 
     position_on_set_invoked ++;
@@ -1182,6 +1182,8 @@ void Sparse_on_set_after_set(void) {
     test_int(1, position_ctor_invoked);
     test_int(1, position_on_add_invoked);
     test_int(1, position_on_set_invoked);
+    test_int(30, position_on_set_value.x);
+    test_int(40, position_on_set_value.y);
 
     {
         const Position *ptr = ecs_get(world, e, Position);
@@ -1217,6 +1219,8 @@ void Sparse_on_set_after_modified(void) {
     test_int(1, position_ctor_invoked);
     test_int(1, position_on_add_invoked);
     test_int(1, position_on_set_invoked);
+    test_int(30, position_on_set_value.x);
+    test_int(40, position_on_set_value.y);
 
     {
         const Position *ptr = ecs_get(world, e, Position);
@@ -1227,6 +1231,45 @@ void Sparse_on_set_after_modified(void) {
     ecs_fini(world);
 }
 
+void Sparse_on_set_at_offset(void) {
+    ecs_world_t *world = ecs_mini();
+
+    ECS_COMPONENT(world, Position);
+    ECS_TAG(world, Tag);
+
+    ecs_add_id(world, ecs_id(Position), EcsSparse);
+
+    ecs_set_hooks(world, Position, {
+        .ctor = ecs_ctor(Position),
+        .on_add = Position_on_add,
+        .on_set = Position_on_set
+    });
+
+    ecs_entity_t e = ecs_new(world);
+    ecs_set(world, e, Position, {10, 20});
+
+    test_int(1, position_ctor_invoked);
+    test_int(1, position_on_add_invoked);
+    test_int(1, position_on_set_invoked);
+    test_int(10, position_on_set_value.x);
+    test_int(20, position_on_set_value.y);
+
+    position_ctor_invoked  = 0;
+    position_on_add_invoked = 0;
+    position_on_set_invoked = 0;
+
+    ecs_entity_t e2 = ecs_new(world);
+    ecs_set(world, e2, Position, {30, 40});
+
+    test_int(1, position_ctor_invoked);
+    test_int(1, position_on_add_invoked);
+    test_int(1, position_on_set_invoked);
+    test_int(30, position_on_set_value.x);
+    test_int(40, position_on_set_value.y);
+
+    ecs_fini(world);
+}
+
 static
 void Position_add_observer(ecs_iter_t *it) {
     probe_iter(it);

diff --git a/test/core/src/main.c b/test/core/src/main.c
@@ -462,6 +462,7 @@ void Sparse_on_add_remove_after_delete(void);
 void Sparse_on_add_remove_after_fini(void);
 void Sparse_on_set_after_set(void);
 void Sparse_on_set_after_modified(void);
+void Sparse_on_set_at_offset(void);
 void Sparse_on_add_observer(void);
 void Sparse_on_set_observer_set(void);
 void Sparse_on_set_observer_modified(void);
@@ -3984,6 +3985,10 @@ bake_test_case Sparse_testcases[] = {
         "on_set_after_modified",
         Sparse_on_set_after_modified
     },
+    {
+        "on_set_at_offset",
+        Sparse_on_set_at_offset
+    },
     {
         "on_add_observer",
         Sparse_on_add_observer
@@ -10911,7 +10916,7 @@ static bake_test_suite suites[] = {
         "Sparse",
         NULL,
         NULL,
-        86,
+        87,
         Sparse_testcases
     },
     {

diff --git a/test/cpp/project.json b/test/cpp/project.json
@@ -987,6 +987,15 @@
                 "on_add_hook_w_entity",
                 "on_remove_hook_w_entity",
                 "on_set_hook_w_entity",
+                "on_add_hook_sparse",
+                "on_remove_hook_sparse",
+                "on_set_hook_sparse",
+                "on_add_hook_sparse_w_entity",
+                "on_remove_hook_sparse_w_entity",
+                "on_set_hook_sparse_w_entity",
+                "on_add_hook_sparse_w_iter",
+                "on_remove_hook_sparse_w_iter",
+                "on_set_hook_sparse_w_iter",
                 "chained_hooks",
                 "ctor_w_2_worlds",
                 "ctor_w_2_worlds_explicit_registration",