From eddd13a061f52bf8c966ae2c47d5b3d911cb4a34 Mon Sep 17 00:00:00 2001 From: Sander Mertens Date: Tue, 17 Dec 2024 09:46:46 -0800 Subject: [PATCH] #1476 Fix issue with nested observers and implicit namespaced component registration --- distr/flecs.c | 45 ++++++++++++++++--------- src/entity.c | 14 ++++---- src/stage.c | 3 +- src/world.c | 24 +++++++++++--- src/world.h | 4 ++- test/cpp/project.json | 4 ++- test/cpp/src/Observer.cpp | 70 +++++++++++++++++++++++++++++++++++++++ test/cpp/src/main.cpp | 12 ++++++- 8 files changed, 144 insertions(+), 32 deletions(-) diff --git a/distr/flecs.c b/distr/flecs.c index 7012edc92..5a7da33d7 100644 --- a/distr/flecs.c +++ b/distr/flecs.c @@ -2911,10 +2911,12 @@ void flecs_process_pending_tables( typedef struct ecs_suspend_readonly_state_t { bool is_readonly; bool is_deferred; + bool cmd_flushing; int32_t defer_count; ecs_entity_t scope; ecs_entity_t with; - ecs_commands_t cmd; + ecs_commands_t cmd_stack[2]; + ecs_commands_t *cmd; ecs_stage_t *stage; } ecs_suspend_readonly_state_t; @@ -7900,7 +7902,7 @@ ecs_entity_t ecs_component_init( } flecs_resume_readonly(world, &readonly_state); - + ecs_assert(result != 0, ECS_INTERNAL_ERROR, NULL); ecs_assert(ecs_has(world, result, EcsComponent), ECS_INTERNAL_ERROR, NULL); @@ -10746,6 +10748,12 @@ bool flecs_defer_end( ecs_commands_t *commands = stage->cmd; ecs_vec_t *queue = &commands->queue; + if (stage->cmd == &stage->cmd_stack[0]) { + stage->cmd = &stage->cmd_stack[1]; + } else { + stage->cmd = &stage->cmd_stack[0]; + } + if (!ecs_vec_count(queue)) { break; } @@ -10764,12 +10772,6 @@ bool flecs_defer_end( ecs_table_diff_builder_t diff; flecs_table_diff_builder_init(world, &diff); - if (stage->cmd == &stage->cmd_stack[0]) { - stage->cmd = &stage->cmd_stack[1]; - } else { - stage->cmd = &stage->cmd_stack[0]; - } - for (i = 0; i < count; i ++) { ecs_cmd_t *cmd = &cmds[i]; ecs_entity_t e = cmd->entity; @@ -17857,8 +17859,7 @@ void flecs_commands_fini( ecs_commands_t *cmd) { /* Make sure stage has no unmerged data */ - ecs_assert(ecs_vec_count(&stage->cmd->queue) == 0, - ECS_INTERNAL_ERROR, NULL); + ecs_assert(ecs_vec_count(&cmd->queue) == 0, ECS_INTERNAL_ERROR, NULL); flecs_stack_fini(&cmd->stack); ecs_vec_fini_t(&stage->allocator, &cmd->queue, ecs_cmd_t); @@ -18803,18 +18804,27 @@ ecs_world_t* flecs_suspend_readonly( /* Cannot suspend when running with multiple threads */ ecs_assert(!(world->flags & EcsWorldReadonly) || - !(world->flags & EcsWorldMultiThreaded), ECS_INVALID_WHILE_READONLY, NULL); + !(world->flags & EcsWorldMultiThreaded), + ECS_INVALID_WHILE_READONLY, NULL); state->is_readonly = is_readonly; state->is_deferred = stage->defer != 0; + state->cmd_flushing = stage->cmd_flushing; /* Silence readonly checks */ world->flags &= ~EcsWorldReadonly; + stage->cmd_flushing = false; /* Hack around safety checks (this ought to look ugly) */ state->defer_count = stage->defer; - state->cmd = *stage->cmd; - flecs_commands_init(stage, stage->cmd); + state->cmd_stack[0] = stage->cmd_stack[0]; + state->cmd_stack[1] = stage->cmd_stack[1]; + state->cmd = stage->cmd; + + flecs_commands_init(stage, &stage->cmd_stack[0]); + flecs_commands_init(stage, &stage->cmd_stack[1]); + stage->cmd = &stage->cmd_stack[0]; + state->scope = stage->scope; state->with = stage->with; stage->defer = 0; @@ -18840,8 +18850,13 @@ void flecs_resume_readonly( /* Restore readonly state / defer count */ ECS_BIT_COND(world->flags, EcsWorldReadonly, state->is_readonly); stage->defer = state->defer_count; - flecs_commands_fini(stage, stage->cmd); - *stage->cmd = state->cmd; + stage->cmd_flushing = state->cmd_flushing; + flecs_commands_fini(stage, &stage->cmd_stack[0]); + flecs_commands_fini(stage, &stage->cmd_stack[1]); + stage->cmd_stack[0] = state->cmd_stack[0]; + stage->cmd_stack[1] = state->cmd_stack[1]; + stage->cmd = state->cmd; + stage->scope = state->scope; stage->with = state->with; } diff --git a/src/entity.c b/src/entity.c index 4e2a2e371..33d65b461 100644 --- a/src/entity.c +++ b/src/entity.c @@ -2276,7 +2276,7 @@ ecs_entity_t ecs_component_init( } flecs_resume_readonly(world, &readonly_state); - + ecs_assert(result != 0, ECS_INTERNAL_ERROR, NULL); ecs_assert(ecs_has(world, result, EcsComponent), ECS_INTERNAL_ERROR, NULL); @@ -5122,6 +5122,12 @@ bool flecs_defer_end( ecs_commands_t *commands = stage->cmd; ecs_vec_t *queue = &commands->queue; + if (stage->cmd == &stage->cmd_stack[0]) { + stage->cmd = &stage->cmd_stack[1]; + } else { + stage->cmd = &stage->cmd_stack[0]; + } + if (!ecs_vec_count(queue)) { break; } @@ -5140,12 +5146,6 @@ bool flecs_defer_end( ecs_table_diff_builder_t diff; flecs_table_diff_builder_init(world, &diff); - if (stage->cmd == &stage->cmd_stack[0]) { - stage->cmd = &stage->cmd_stack[1]; - } else { - stage->cmd = &stage->cmd_stack[0]; - } - for (i = 0; i < count; i ++) { ecs_cmd_t *cmd = &cmds[i]; ecs_entity_t e = cmd->entity; diff --git a/src/stage.c b/src/stage.c index b4f97b460..886384716 100644 --- a/src/stage.c +++ b/src/stage.c @@ -584,8 +584,7 @@ void flecs_commands_fini( ecs_commands_t *cmd) { /* Make sure stage has no unmerged data */ - ecs_assert(ecs_vec_count(&stage->cmd->queue) == 0, - ECS_INTERNAL_ERROR, NULL); + ecs_assert(ecs_vec_count(&cmd->queue) == 0, ECS_INTERNAL_ERROR, NULL); flecs_stack_fini(&cmd->stack); ecs_vec_fini_t(&stage->allocator, &cmd->queue, ecs_cmd_t); diff --git a/src/world.c b/src/world.c index 11ee0ea69..ac9c533d0 100644 --- a/src/world.c +++ b/src/world.c @@ -396,18 +396,27 @@ ecs_world_t* flecs_suspend_readonly( /* Cannot suspend when running with multiple threads */ ecs_assert(!(world->flags & EcsWorldReadonly) || - !(world->flags & EcsWorldMultiThreaded), ECS_INVALID_WHILE_READONLY, NULL); + !(world->flags & EcsWorldMultiThreaded), + ECS_INVALID_WHILE_READONLY, NULL); state->is_readonly = is_readonly; state->is_deferred = stage->defer != 0; + state->cmd_flushing = stage->cmd_flushing; /* Silence readonly checks */ world->flags &= ~EcsWorldReadonly; + stage->cmd_flushing = false; /* Hack around safety checks (this ought to look ugly) */ state->defer_count = stage->defer; - state->cmd = *stage->cmd; - flecs_commands_init(stage, stage->cmd); + state->cmd_stack[0] = stage->cmd_stack[0]; + state->cmd_stack[1] = stage->cmd_stack[1]; + state->cmd = stage->cmd; + + flecs_commands_init(stage, &stage->cmd_stack[0]); + flecs_commands_init(stage, &stage->cmd_stack[1]); + stage->cmd = &stage->cmd_stack[0]; + state->scope = stage->scope; state->with = stage->with; stage->defer = 0; @@ -433,8 +442,13 @@ void flecs_resume_readonly( /* Restore readonly state / defer count */ ECS_BIT_COND(world->flags, EcsWorldReadonly, state->is_readonly); stage->defer = state->defer_count; - flecs_commands_fini(stage, stage->cmd); - *stage->cmd = state->cmd; + stage->cmd_flushing = state->cmd_flushing; + flecs_commands_fini(stage, &stage->cmd_stack[0]); + flecs_commands_fini(stage, &stage->cmd_stack[1]); + stage->cmd_stack[0] = state->cmd_stack[0]; + stage->cmd_stack[1] = state->cmd_stack[1]; + stage->cmd = state->cmd; + stage->scope = state->scope; stage->with = state->with; } diff --git a/src/world.h b/src/world.h index 8484b582c..fe183ba20 100644 --- a/src/world.h +++ b/src/world.h @@ -98,10 +98,12 @@ void flecs_process_pending_tables( typedef struct ecs_suspend_readonly_state_t { bool is_readonly; bool is_deferred; + bool cmd_flushing; int32_t defer_count; ecs_entity_t scope; ecs_entity_t with; - ecs_commands_t cmd; + ecs_commands_t cmd_stack[2]; + ecs_commands_t *cmd; ecs_stage_t *stage; } ecs_suspend_readonly_state_t; diff --git a/test/cpp/project.json b/test/cpp/project.json index 7391beeb1..40371a9d7 100644 --- a/test/cpp/project.json +++ b/test/cpp/project.json @@ -962,7 +962,9 @@ "on_set_after_remove_override", "on_set_after_remove_override_create_observer_before", "on_set_w_override_after_delete", - "on_set_w_override_after_clear" + "on_set_w_override_after_clear", + "trigger_on_set_in_on_add_implicit_registration", + "trigger_on_set_in_on_add_implicit_registration_namespaced" ] }, { "id": "ComponentLifecycle", diff --git a/test/cpp/src/Observer.cpp b/test/cpp/src/Observer.cpp index 02958d9c8..b2574e51a 100644 --- a/test/cpp/src/Observer.cpp +++ b/test/cpp/src/Observer.cpp @@ -1474,3 +1474,73 @@ void Observer_on_set_w_override_after_clear(void) { test_int(count, 0); } + +void Observer_trigger_on_set_in_on_add_implicit_registration(void) { + flecs::world world; + + struct Tag {}; + + world.observer() + .event(flecs::OnAdd) + .each([](flecs::entity e, Tag) { + e.set({ 10, 20 }); + }); + + world.observer() + .event(flecs::OnSet) + .each([](flecs::entity e, Position&) { + e.set({ 1, 2 }); + }); + + auto e = world.entity().add(); + + { + const Position *p = e.get(); + test_int(p->x, 10); + test_int(p->y, 20); + } + + { + const Velocity *v = e.get(); + test_int(v->x, 1); + test_int(v->y, 2); + } +} + +namespace ns { + struct Velocity { + float x, y; + }; +} + +void Observer_trigger_on_set_in_on_add_implicit_registration_namespaced(void) { + flecs::world world; + + struct Tag { }; + + world.observer() + .event(flecs::OnAdd) + .each([](flecs::entity e, Tag) { + e.set({ 10, 20 }); + }); + + world.observer() + .event(flecs::OnSet) + .each([](flecs::entity e, Position&) { + e.set({ 1, 2 }); + }); + + auto e = world.entity().add(); + + { + const Position *p = e.get(); + test_int(p->x, 10); + test_int(p->y, 20); + } + + { + const ns::Velocity *v = e.get(); + test_int(v->x, 1); + test_int(v->y, 2); + } +} diff --git a/test/cpp/src/main.cpp b/test/cpp/src/main.cpp index b75c920aa..7c67a43b7 100644 --- a/test/cpp/src/main.cpp +++ b/test/cpp/src/main.cpp @@ -930,6 +930,8 @@ void Observer_on_set_after_remove_override(void); void Observer_on_set_after_remove_override_create_observer_before(void); void Observer_on_set_w_override_after_delete(void); void Observer_on_set_w_override_after_clear(void); +void Observer_trigger_on_set_in_on_add_implicit_registration(void); +void Observer_trigger_on_set_in_on_add_implicit_registration_namespaced(void); // Testsuite 'ComponentLifecycle' void ComponentLifecycle_ctor_on_add(void); @@ -5027,6 +5029,14 @@ bake_test_case Observer_testcases[] = { { "on_set_w_override_after_clear", Observer_on_set_w_override_after_clear + }, + { + "trigger_on_set_in_on_add_implicit_registration", + Observer_trigger_on_set_in_on_add_implicit_registration + }, + { + "trigger_on_set_in_on_add_implicit_registration_namespaced", + Observer_trigger_on_set_in_on_add_implicit_registration_namespaced } }; @@ -6971,7 +6981,7 @@ static bake_test_suite suites[] = { "Observer", NULL, NULL, - 59, + 61, Observer_testcases }, {