Fix issue with pair components where relationship id is recycled

SanderMertens · Nov 14, 2023 · ff72706 · ff72706
1 parent 3b28f75
commit ff72706
Show file tree

Hide file tree

Showing 9 changed files with 170 additions and 5 deletions.
diff --git a/flecs.c b/flecs.c
@@ -7476,6 +7476,7 @@ ecs_entity_t ecs_get_typeid(
     ecs_check(world != NULL, ECS_INVALID_PARAMETER, NULL);
     const ecs_type_info_t *ti = ecs_get_type_info(world, id);
     if (ti) {
+        ecs_assert(ti->component != 0, ECS_INTERNAL_ERROR, NULL);
         return ti->component;
     }
 error:
@@ -42173,6 +42174,7 @@ ecs_id_record_t* flecs_id_record_new(
     if (is_pair) {
         // rel = ecs_pair_first(world, id);
         rel = ECS_PAIR_FIRST(id);
+        rel = flecs_entities_get_generation(world, rel);
         ecs_assert(rel != 0, ECS_INTERNAL_ERROR, NULL);
 
         /* Relationship object can be 0, as tables without a ChildOf 
@@ -42182,7 +42184,7 @@ ecs_id_record_t* flecs_id_record_new(
 #ifdef FLECS_DEBUG
         /* Check constraints */
         if (tgt) {
-            tgt = ecs_get_alive(world, tgt);
+            tgt = flecs_entities_get_generation(world, tgt);
             ecs_assert(tgt != 0, ECS_INTERNAL_ERROR, NULL);
         }
         if (tgt && !ecs_id_is_wildcard(tgt)) {

diff --git a/src/entity.c b/src/entity.c
@@ -4091,6 +4091,7 @@ ecs_entity_t ecs_get_typeid(
     ecs_check(world != NULL, ECS_INVALID_PARAMETER, NULL);
     const ecs_type_info_t *ti = ecs_get_type_info(world, id);
     if (ti) {
+        ecs_assert(ti->component != 0, ECS_INTERNAL_ERROR, NULL);
         return ti->component;
     }
 error:

diff --git a/src/storage/id_index.c b/src/storage/id_index.c
@@ -156,6 +156,7 @@ ecs_id_record_t* flecs_id_record_new(
     if (is_pair) {
         // rel = ecs_pair_first(world, id);
         rel = ECS_PAIR_FIRST(id);
+        rel = flecs_entities_get_generation(world, rel);
         ecs_assert(rel != 0, ECS_INTERNAL_ERROR, NULL);
 
         /* Relationship object can be 0, as tables without a ChildOf 
@@ -165,7 +166,7 @@ ecs_id_record_t* flecs_id_record_new(
 #ifdef FLECS_DEBUG
         /* Check constraints */
         if (tgt) {
-            tgt = ecs_get_alive(world, tgt);
+            tgt = flecs_entities_get_generation(world, tgt);
             ecs_assert(tgt != 0, ECS_INTERNAL_ERROR, NULL);
         }
         if (tgt && !ecs_id_is_wildcard(tgt)) {

diff --git a/test/addons/project.json b/test/addons/project.json
@@ -375,6 +375,8 @@
                 "inherit_from_multiple",
                 "assign_pair_component",
                 "assign_pair_component_in_scope",
+                "assign_pair_component_in_script",
+                "assign_pair_component_in_script_update",
                 "set_entity_names",
                 "oneof",
                 "invalid_oneof",

diff --git a/test/addons/src/Plecs.c b/test/addons/src/Plecs.c
@@ -3406,6 +3406,13 @@ void Plecs_inherit_from_multiple(void) {
     ecs_fini(world);
 }
 
+// using flecs.meta
+// Struct(Position) {
+//   x :- {f32}
+//   y :- {f32}
+// }
+// Foo :- (Position, Bar){x: 10, y: 20}
+
 void Plecs_assign_pair_component(void) {
     ecs_world_t *world = ecs_init();
 
@@ -3484,6 +3491,82 @@ void Plecs_assign_pair_component_in_scope(void) {
     ecs_fini(world);
 }
 
+void Plecs_assign_pair_component_in_script(void) {
+    ecs_world_t *world = ecs_init();
+
+    const char *expr =
+    HEAD "using flecs.meta"
+    LINE
+    LINE "Struct(Position) {"
+    LINE "  x :- {f32}"
+    LINE "  y :- {f32}"
+    LINE "}"
+    LINE
+    LINE "Foo :- (Position, Bar){x: 10, y: 20}";
+
+    ecs_entity_t s = ecs_script(world, {
+        .entity = ecs_entity(world, { .name = "main" }),
+        .str = expr
+    });
+    test_assert(s != 0);
+
+    ecs_entity_t foo = ecs_lookup_fullpath(world, "Foo");
+    ecs_entity_t bar = ecs_lookup_fullpath(world, "Bar");
+    ecs_entity_t ecs_id(Position) = ecs_lookup_fullpath(world, "Position");
+
+    test_assert(foo != 0);
+    test_assert(bar != 0);
+    test_assert(ecs_id(Position) != 0);
+
+    test_assert( ecs_has_pair(world, foo, ecs_id(Position), bar));
+
+    const Position *ptr = ecs_get_pair(world, foo, Position, bar);
+    test_assert(ptr != NULL);
+    test_int(ptr->x, 10);
+    test_int(ptr->y, 20);
+
+    ecs_fini(world);
+}
+
+void Plecs_assign_pair_component_in_script_update(void) {
+    ecs_world_t *world = ecs_init();
+
+    const char *expr =
+    HEAD "using flecs.meta"
+    LINE
+    LINE "Struct(Position) {"
+    LINE "  x :- {f32}"
+    LINE "  y :- {f32}"
+    LINE "}"
+    LINE
+    LINE "Foo :- (Position, Bar){x: 10, y: 20}";
+
+    ecs_entity_t s = ecs_script(world, {
+        .entity = ecs_entity(world, { .name = "main" }),
+        .str = expr
+    });
+    test_assert(s != 0);
+
+    test_assert(ecs_script_update(world, s, 0, expr, NULL) == 0);
+
+    ecs_entity_t foo = ecs_lookup_fullpath(world, "Foo");
+    ecs_entity_t bar = ecs_lookup_fullpath(world, "Bar");
+    ecs_entity_t ecs_id(Position) = ecs_lookup_fullpath(world, "Position");
+
+    test_assert(foo != 0);
+    test_assert(bar != 0);
+    test_assert(ecs_id(Position) != 0);
+
+    test_assert( ecs_has_pair(world, foo, ecs_id(Position), bar));
+
+    const Position *ptr = ecs_get_pair(world, foo, Position, bar);
+    test_assert(ptr != NULL);
+    test_int(ptr->x, 10);
+    test_int(ptr->y, 20);
+
+    ecs_fini(world);
+}
+
 void Plecs_set_entity_names(void) {
     ecs_world_t *world = ecs_init();
 

diff --git a/test/addons/src/main.c b/test/addons/src/main.c
@@ -368,6 +368,8 @@ void Plecs_scope_w_1_subj_and_2_pairs(void);
 void Plecs_inherit_from_multiple(void);
 void Plecs_assign_pair_component(void);
 void Plecs_assign_pair_component_in_scope(void);
+void Plecs_assign_pair_component_in_script(void);
+void Plecs_assign_pair_component_in_script_update(void);
 void Plecs_set_entity_names(void);
 void Plecs_oneof(void);
 void Plecs_invalid_oneof(void);
@@ -3031,6 +3033,14 @@ bake_test_case Plecs_testcases[] = {
         "assign_pair_component_in_scope",
         Plecs_assign_pair_component_in_scope
     },
+    {
+        "assign_pair_component_in_script",
+        Plecs_assign_pair_component_in_script
+    },
+    {
+        "assign_pair_component_in_script_update",
+        Plecs_assign_pair_component_in_script_update
+    },
     {
         "set_entity_names",
         Plecs_set_entity_names
@@ -7768,7 +7778,7 @@ static bake_test_suite suites[] = {
         "Plecs",
         NULL,
         NULL,
-        235,
+        237,
         Plecs_testcases
     },
     {

diff --git a/test/api/project.json b/test/api/project.json
@@ -1863,7 +1863,9 @@
                 "oneof_other",
                 "oneof_self_constraint_violated",
                 "oneof_other_constraint_violated",
-                "oneof_other_rel_parent_constraint_violated"
+                "oneof_other_rel_parent_constraint_violated",
+                "set_w_recycled_rel",
+                "set_w_recycled_tgt"
             ]
         }, {
            "id": "Trigger",

diff --git a/test/api/src/Pairs.c b/test/api/src/Pairs.c
@@ -2985,3 +2985,57 @@ void Pairs_oneof_other_rel_parent_constraint_violated(void) {
     test_expect_abort();
     ecs_add_pair(world, e, Rel, ObjC);
 }
+
+void Pairs_set_w_recycled_rel(void) {
+    ecs_world_t *world = ecs_mini();
+
+    ecs_entity_t dummy = ecs_new_id(world);
+    ecs_delete(world, dummy);
+
+    ecs_entity_t ecs_id(Position) = ecs_component(world, {
+        .entity = ecs_new_id(world),
+        .type.size = ECS_SIZEOF(Position),
+        .type.alignment = ECS_ALIGNOF(Position)
+    });
+
+    test_assert(ecs_id(Position) != (uint32_t)ecs_id(Position));
+
+    ecs_entity_t e = ecs_new_id(world);
+    ecs_entity_t tgt = ecs_new_id(world);
+    ecs_set_pair(world, e, Position, tgt, {10, 20});
+    test_assert(ecs_has_pair(world, e, ecs_id(Position), tgt));
+
+    const Position *p = ecs_get_pair(world, e, Position, tgt);
+    test_assert(p != NULL);
+    test_int(p->x, 10);
+    test_int(p->y, 20);
+
+    ecs_fini(world);
+}
+
+void Pairs_set_w_recycled_tgt(void) {
+    ecs_world_t *world = ecs_mini();
+
+    ecs_entity_t dummy = ecs_new_id(world);
+    ecs_delete(world, dummy);
+
+    ecs_entity_t ecs_id(Position) = ecs_component(world, {
+        .entity = ecs_new_id(world),
+        .type.size = ECS_SIZEOF(Position),
+        .type.alignment = ECS_ALIGNOF(Position)
+    });
+
+    test_assert(ecs_id(Position) != (uint32_t)ecs_id(Position));
+
+    ecs_entity_t e = ecs_new_id(world);
+    ecs_entity_t rel = ecs_new_id(world);
+    ecs_set_pair_second(world, e, rel, Position, {10, 20});
+    test_assert(ecs_has_pair(world, e, rel, ecs_id(Position)));
+
+    const Position *p = ecs_get_pair_second(world, e, rel, Position);
+    test_assert(p != NULL);
+    test_int(p->x, 10);
+    test_int(p->y, 20);
+
+    ecs_fini(world);
+}
diff --git a/test/api/src/main.c b/test/api/src/main.c
@@ -1797,6 +1797,8 @@ void Pairs_oneof_other(void);
 void Pairs_oneof_self_constraint_violated(void);
 void Pairs_oneof_other_constraint_violated(void);
 void Pairs_oneof_other_rel_parent_constraint_violated(void);
+void Pairs_set_w_recycled_rel(void);
+void Pairs_set_w_recycled_tgt(void);
 
 // Testsuite 'Trigger'
 void Trigger_on_add_trigger_before_table(void);
@@ -9629,6 +9631,14 @@ bake_test_case Pairs_testcases[] = {
     {
         "oneof_other_rel_parent_constraint_violated",
         Pairs_oneof_other_rel_parent_constraint_violated
+    },
+    {
+        "set_w_recycled_rel",
+        Pairs_set_w_recycled_rel
+    },
+    {
+        "set_w_recycled_tgt",
+        Pairs_set_w_recycled_tgt
     }
 };
 
@@ -13175,7 +13185,7 @@ static bake_test_suite suites[] = {
         "Pairs",
         NULL,
         NULL,
-        115,
+        117,
         Pairs_testcases
     },
     {