From 4a1f4f0cf96856945453520bf9eaba4763b68d15 Mon Sep 17 00:00:00 2001 From: Sander Mertens Date: Sun, 8 Dec 2024 19:23:25 -0800 Subject: [PATCH] Allow for infinitely nested observer events --- distr/flecs.c | 86 ++++++++++++++---------------------- src/addons/script/template.c | 5 +-- src/entity.c | 49 ++++++++++---------- src/private_types.h | 6 +-- src/stage.c | 20 +-------- src/stage.h | 6 --- test/core/project.json | 3 +- test/core/src/Commands.c | 50 +++++++++++++++++++++ test/core/src/Event.c | 2 + test/core/src/main.c | 7 ++- 10 files changed, 126 insertions(+), 108 deletions(-) diff --git a/distr/flecs.c b/distr/flecs.c index 30c8cc37eb..7d4039fa07 100644 --- a/distr/flecs.c +++ b/distr/flecs.c @@ -852,10 +852,10 @@ struct ecs_stage_t { /* Zero if not deferred, positive if deferred, negative if suspended */ int32_t defer; - /* Command queue stack, for nested execution */ + /* Command queue */ ecs_commands_t *cmd; - ecs_commands_t cmd_stack[ECS_MAX_DEFER_STACK]; - int32_t cmd_sp; + ecs_commands_t cmd_stack[2]; /* Two so we can flush one & populate the other */ + bool cmd_flushing; /* Ensures only one defer_end call flushes */ /* Thread context */ ecs_world_t *thread_ctx; /* Points to stage when a thread stage */ @@ -2791,12 +2791,6 @@ void flecs_enqueue( ecs_stage_t *stage, ecs_event_desc_t *desc); -void flecs_commands_push( - ecs_stage_t *stage); - -void flecs_commands_pop( - ecs_stage_t *stage); - ecs_entity_t flecs_stage_set_system( ecs_stage_t *stage, ecs_entity_t system); @@ -9141,26 +9135,18 @@ void ecs_enable_id( ecs_check(flecs_can_toggle(world, id), ECS_INVALID_OPERATION, "add CanToggle trait to component"); + ecs_entity_t bs_id = id | ECS_TOGGLE; + ecs_add_id(world, entity, bs_id); + if (flecs_defer_enable(stage, entity, id, enable)) { return; } - ecs_record_t *r = flecs_entities_get(world, entity); - ecs_entity_t bs_id = id | ECS_TOGGLE; - + ecs_record_t *r = flecs_entities_get(world, entity); ecs_table_t *table = r->table; - int32_t index = -1; - if (table) { - index = ecs_table_get_type_index(world, table, bs_id); - } + int32_t index = ecs_table_get_type_index(world, table, bs_id); + ecs_assert(index != -1, ECS_INTERNAL_ERROR, NULL); - if (index == -1) { - ecs_add_id(world, entity, bs_id); - flecs_defer_end(world, stage); - ecs_enable_id(world, entity, id, enable); - return; - } - ecs_assert(table->_ != NULL, ECS_INTERNAL_ERROR, NULL); index -= table->_->bs_offset; ecs_assert(index >= 0, ECS_INTERNAL_ERROR, NULL); @@ -10611,7 +10597,7 @@ bool flecs_defer_end( ecs_assert(stage->defer > 0, ECS_INTERNAL_ERROR, NULL); - if (!--stage->defer) { + if (!--stage->defer && !stage->cmd_flushing) { ecs_os_perf_trace_push("flecs.commands.merge"); /* Test whether we're flushing to another queue or whether we're @@ -10621,11 +10607,17 @@ bool flecs_defer_end( merge_to_world = world->stages[0]->defer == 0; } - ecs_stage_t *dst_stage = flecs_stage_from_world(&world); - ecs_commands_t *commands = stage->cmd; - ecs_vec_t *queue = &commands->queue; + do { + ecs_stage_t *dst_stage = flecs_stage_from_world(&world); + ecs_commands_t *commands = stage->cmd; + ecs_vec_t *queue = &commands->queue; + + if (!ecs_vec_count(queue)) { + break; + } + + stage->cmd_flushing = true; - if (ecs_vec_count(queue)) { /* Internal callback for capturing commands */ if (world->on_commands_active) { world->on_commands_active(stage, queue, @@ -10637,7 +10629,12 @@ bool flecs_defer_end( ecs_table_diff_builder_t diff; flecs_table_diff_builder_init(world, &diff); - flecs_commands_push(stage); + + 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]; @@ -10795,10 +10792,10 @@ bool flecs_defer_end( } } + stage->cmd_flushing = false; + flecs_stack_reset(&commands->stack); ecs_vec_clear(queue); - flecs_commands_pop(stage); - flecs_table_diff_builder_fini(world, &diff); /* Internal callback for capturing commands, signal queue is done */ @@ -10806,7 +10803,7 @@ bool flecs_defer_end( world->on_commands_active(stage, NULL, world->on_commands_ctx_active); } - } + } while (true); ecs_os_perf_trace_pop("flecs.commands.merge"); @@ -17685,22 +17682,6 @@ void flecs_commands_fini( flecs_sparse_fini(&cmd->entries); } -void flecs_commands_push( - ecs_stage_t *stage) -{ - int32_t sp = ++ stage->cmd_sp; - ecs_assert(sp < ECS_MAX_DEFER_STACK, ECS_INTERNAL_ERROR, NULL); - stage->cmd = &stage->cmd_stack[sp]; -} - -void flecs_commands_pop( - ecs_stage_t *stage) -{ - int32_t sp = -- stage->cmd_sp; - ecs_assert(sp >= 0, ECS_INTERNAL_ERROR, NULL); - stage->cmd = &stage->cmd_stack[sp]; -} - ecs_entity_t flecs_stage_set_system( ecs_stage_t *stage, ecs_entity_t system) @@ -17733,7 +17714,7 @@ ecs_stage_t* flecs_stage_new( ecs_vec_init_t(a, &stage->post_frame_actions, ecs_action_elem_t, 0); int32_t i; - for (i = 0; i < ECS_MAX_DEFER_STACK; i ++) { + for (i = 0; i < 2; i ++) { flecs_commands_init(stage, &stage->cmd_stack[i]); } @@ -17759,7 +17740,7 @@ void flecs_stage_free( ecs_vec_fini(NULL, &stage->operations, 0); int32_t i; - for (i = 0; i < ECS_MAX_DEFER_STACK; i ++) { + for (i = 0; i < 2; i ++) { flecs_commands_fini(stage, &stage->cmd_stack[i]); } @@ -58624,16 +58605,15 @@ void flecs_on_template_set_event( ecs_assert(ecs_is_deferred(it->world), ECS_INTERNAL_ERROR, NULL); EcsTemplateSetEvent *evt = it->param; - ecs_suspend_readonly_state_t srs; ecs_world_t *world = it->real_world; ecs_assert(flecs_poly_is(world, ecs_world_t), ECS_INTERNAL_ERROR, NULL); - flecs_suspend_readonly(world, &srs); + ecs_defer_suspend(world); flecs_script_template_instantiate( world, evt->template_entity, evt->entities, evt->data, evt->count); - flecs_resume_readonly(world, &srs); + ecs_defer_resume(world); } /* Template on_set handler to update contents for new property values */ diff --git a/src/addons/script/template.c b/src/addons/script/template.c index 3d3a132716..94ed51015c 100644 --- a/src/addons/script/template.c +++ b/src/addons/script/template.c @@ -196,16 +196,15 @@ void flecs_on_template_set_event( ecs_assert(ecs_is_deferred(it->world), ECS_INTERNAL_ERROR, NULL); EcsTemplateSetEvent *evt = it->param; - ecs_suspend_readonly_state_t srs; ecs_world_t *world = it->real_world; ecs_assert(flecs_poly_is(world, ecs_world_t), ECS_INTERNAL_ERROR, NULL); - flecs_suspend_readonly(world, &srs); + ecs_defer_suspend(world); flecs_script_template_instantiate( world, evt->template_entity, evt->entities, evt->data, evt->count); - flecs_resume_readonly(world, &srs); + ecs_defer_resume(world); } /* Template on_set handler to update contents for new property values */ diff --git a/src/entity.c b/src/entity.c index b7e0635f7b..b3945fcac0 100644 --- a/src/entity.c +++ b/src/entity.c @@ -3636,26 +3636,18 @@ void ecs_enable_id( ecs_check(flecs_can_toggle(world, id), ECS_INVALID_OPERATION, "add CanToggle trait to component"); + ecs_entity_t bs_id = id | ECS_TOGGLE; + ecs_add_id(world, entity, bs_id); + if (flecs_defer_enable(stage, entity, id, enable)) { return; } - ecs_record_t *r = flecs_entities_get(world, entity); - ecs_entity_t bs_id = id | ECS_TOGGLE; - + ecs_record_t *r = flecs_entities_get(world, entity); ecs_table_t *table = r->table; - int32_t index = -1; - if (table) { - index = ecs_table_get_type_index(world, table, bs_id); - } + int32_t index = ecs_table_get_type_index(world, table, bs_id); + ecs_assert(index != -1, ECS_INTERNAL_ERROR, NULL); - if (index == -1) { - ecs_add_id(world, entity, bs_id); - flecs_defer_end(world, stage); - ecs_enable_id(world, entity, id, enable); - return; - } - ecs_assert(table->_ != NULL, ECS_INTERNAL_ERROR, NULL); index -= table->_->bs_offset; ecs_assert(index >= 0, ECS_INTERNAL_ERROR, NULL); @@ -5106,7 +5098,7 @@ bool flecs_defer_end( ecs_assert(stage->defer > 0, ECS_INTERNAL_ERROR, NULL); - if (!--stage->defer) { + if (!--stage->defer && !stage->cmd_flushing) { ecs_os_perf_trace_push("flecs.commands.merge"); /* Test whether we're flushing to another queue or whether we're @@ -5116,11 +5108,17 @@ bool flecs_defer_end( merge_to_world = world->stages[0]->defer == 0; } - ecs_stage_t *dst_stage = flecs_stage_from_world(&world); - ecs_commands_t *commands = stage->cmd; - ecs_vec_t *queue = &commands->queue; + do { + ecs_stage_t *dst_stage = flecs_stage_from_world(&world); + ecs_commands_t *commands = stage->cmd; + ecs_vec_t *queue = &commands->queue; + + if (!ecs_vec_count(queue)) { + break; + } + + stage->cmd_flushing = true; - if (ecs_vec_count(queue)) { /* Internal callback for capturing commands */ if (world->on_commands_active) { world->on_commands_active(stage, queue, @@ -5132,7 +5130,12 @@ bool flecs_defer_end( ecs_table_diff_builder_t diff; flecs_table_diff_builder_init(world, &diff); - flecs_commands_push(stage); + + 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]; @@ -5290,10 +5293,10 @@ bool flecs_defer_end( } } + stage->cmd_flushing = false; + flecs_stack_reset(&commands->stack); ecs_vec_clear(queue); - flecs_commands_pop(stage); - flecs_table_diff_builder_fini(world, &diff); /* Internal callback for capturing commands, signal queue is done */ @@ -5301,7 +5304,7 @@ bool flecs_defer_end( world->on_commands_active(stage, NULL, world->on_commands_ctx_active); } - } + } while (true); ecs_os_perf_trace_pop("flecs.commands.merge"); diff --git a/src/private_types.h b/src/private_types.h index bd5b208b3a..cd30e6f05a 100644 --- a/src/private_types.h +++ b/src/private_types.h @@ -201,10 +201,10 @@ struct ecs_stage_t { /* Zero if not deferred, positive if deferred, negative if suspended */ int32_t defer; - /* Command queue stack, for nested execution */ + /* Command queue */ ecs_commands_t *cmd; - ecs_commands_t cmd_stack[ECS_MAX_DEFER_STACK]; - int32_t cmd_sp; + ecs_commands_t cmd_stack[2]; /* Two so we can flush one & populate the other */ + bool cmd_flushing; /* Ensures only one defer_end call flushes */ /* Thread context */ ecs_world_t *thread_ctx; /* Points to stage when a thread stage */ diff --git a/src/stage.c b/src/stage.c index c876f2de13..2a862d5ff2 100644 --- a/src/stage.c +++ b/src/stage.c @@ -591,22 +591,6 @@ void flecs_commands_fini( flecs_sparse_fini(&cmd->entries); } -void flecs_commands_push( - ecs_stage_t *stage) -{ - int32_t sp = ++ stage->cmd_sp; - ecs_assert(sp < ECS_MAX_DEFER_STACK, ECS_INTERNAL_ERROR, NULL); - stage->cmd = &stage->cmd_stack[sp]; -} - -void flecs_commands_pop( - ecs_stage_t *stage) -{ - int32_t sp = -- stage->cmd_sp; - ecs_assert(sp >= 0, ECS_INTERNAL_ERROR, NULL); - stage->cmd = &stage->cmd_stack[sp]; -} - ecs_entity_t flecs_stage_set_system( ecs_stage_t *stage, ecs_entity_t system) @@ -639,7 +623,7 @@ ecs_stage_t* flecs_stage_new( ecs_vec_init_t(a, &stage->post_frame_actions, ecs_action_elem_t, 0); int32_t i; - for (i = 0; i < ECS_MAX_DEFER_STACK; i ++) { + for (i = 0; i < 2; i ++) { flecs_commands_init(stage, &stage->cmd_stack[i]); } @@ -665,7 +649,7 @@ void flecs_stage_free( ecs_vec_fini(NULL, &stage->operations, 0); int32_t i; - for (i = 0; i < ECS_MAX_DEFER_STACK; i ++) { + for (i = 0; i < 2; i ++) { flecs_commands_fini(stage, &stage->cmd_stack[i]); } diff --git a/src/stage.h b/src/stage.h index ced22e7ea2..3f6e0f29d5 100644 --- a/src/stage.h +++ b/src/stage.h @@ -94,12 +94,6 @@ void flecs_enqueue( ecs_stage_t *stage, ecs_event_desc_t *desc); -void flecs_commands_push( - ecs_stage_t *stage); - -void flecs_commands_pop( - ecs_stage_t *stage); - ecs_entity_t flecs_stage_set_system( ecs_stage_t *stage, ecs_entity_t system); diff --git a/test/core/project.json b/test/core/project.json index 3b2436d1e0..caff9b928d 100644 --- a/test/core/project.json +++ b/test/core/project.json @@ -2244,7 +2244,8 @@ "defer_emplace_after_remove", "batched_w_table_change_in_observer", "redefine_named_in_threaded_app", - "batched_cmd_w_component_init" + "batched_cmd_w_component_init", + "deep_command_nesting" ] }, { "id": "SingleThreadStaging", diff --git a/test/core/src/Commands.c b/test/core/src/Commands.c index f89dbbbdd5..89285b993d 100644 --- a/test/core/src/Commands.c +++ b/test/core/src/Commands.c @@ -4243,3 +4243,53 @@ void Commands_batched_cmd_w_component_init(void) { ecs_fini(world); } + +typedef struct TestNestEvent { + int32_t depth; +} TestNestEvent; + +static ECS_COMPONENT_DECLARE(TestNestEvent); + +static int test_nest_invoked = 0; + +static +void test_nest_observer(ecs_iter_t *it) { + TestNestEvent *param = it->param; + + TestNestEvent evt = { .depth = param->depth - 1 }; + + test_nest_invoked ++; + + if (param->depth) { + ecs_enqueue(it->world, &(ecs_event_desc_t) { + .event = ecs_id(TestNestEvent), + .param = &evt, + .entity = EcsAny + }); + } +} + +void Commands_deep_command_nesting(void) { + ecs_world_t *world = ecs_mini(); + + ECS_COMPONENT_DEFINE(world, TestNestEvent); + + ecs_observer(world, { + .events = {{ ecs_id(TestNestEvent) }}, + .query = { .terms = {{ .id = EcsAny }}}, + .callback = test_nest_observer + }); + + ecs_log_set_level(0); + ecs_emit(world, &(ecs_event_desc_t) { + .event = ecs_id(TestNestEvent), + .param = &(TestNestEvent){ .depth = 32 }, + .entity = EcsAny + }); + + test_int(test_nest_invoked, 33); + + ecs_log_set_level(-1); + + ecs_fini(world); +} diff --git a/test/core/src/Event.c b/test/core/src/Event.c index 69e1ac2bf6..5f3137481e 100644 --- a/test/core/src/Event.c +++ b/test/core/src/Event.c @@ -1215,9 +1215,11 @@ void Event_enqueue_event_not_alive_w_data_copy(void) { static void system_delete_callback(ecs_iter_t *it) { + ecs_defer_suspend(it->world); for (int i = 0; i < it->count; i ++) { ecs_delete(it->world, it->entities[i]); } + ecs_defer_resume(it->world); } void Event_enqueue_event_not_alive_after_delete_during_merge(void) { diff --git a/test/core/src/main.c b/test/core/src/main.c index 956cb0439a..46d9a5b7d6 100644 --- a/test/core/src/main.c +++ b/test/core/src/main.c @@ -2164,6 +2164,7 @@ void Commands_defer_emplace_after_remove(void); void Commands_batched_w_table_change_in_observer(void); void Commands_redefine_named_in_threaded_app(void); void Commands_batched_cmd_w_component_init(void); +void Commands_deep_command_nesting(void); // Testsuite 'SingleThreadStaging' void SingleThreadStaging_setup(void); @@ -10716,6 +10717,10 @@ bake_test_case Commands_testcases[] = { { "batched_cmd_w_component_init", Commands_batched_cmd_w_component_init + }, + { + "deep_command_nesting", + Commands_deep_command_nesting } }; @@ -11600,7 +11605,7 @@ static bake_test_suite suites[] = { "Commands", NULL, NULL, - 142, + 143, Commands_testcases }, {