Skip to content

Commit

Permalink
Fix issue with nested observers and implicit namespaced component reg…
Browse files Browse the repository at this point in the history 
…istration
  • Loading branch information
@SanderMertens
SanderMertens committed Dec 17, 2024
1 parent 1050c12 commit 2b985f6
Show file tree
Hide file tree
Showing 8 changed files with 144 additions and 32 deletions.
45 changes: 30 additions & 15 deletions distr/flecs.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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;
}
Expand All @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand All @@ -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;
}
Expand Down
14 changes: 7 additions & 7 deletions src/entity.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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;
}
Expand All @@ -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;
Expand Down
3 changes: 1 addition & 2 deletions src/stage.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
24 changes: 19 additions & 5 deletions src/world.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
}
Expand Down
4 changes: 3 additions & 1 deletion src/world.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
4 changes: 3 additions & 1 deletion test/cpp/project.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
70 changes: 70 additions & 0 deletions test/cpp/src/Observer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<Tag>()
.event(flecs::OnAdd)
.each([](flecs::entity e, Tag) {
e.set<Position>({ 10, 20 });
});

world.observer<Position>()
.event(flecs::OnSet)
.each([](flecs::entity e, Position&) {
e.set<Velocity>({ 1, 2 });
});

auto e = world.entity().add<Tag>();

{
const Position *p = e.get<Position>();
test_int(p->x, 10);
test_int(p->y, 20);
}

{
const Velocity *v = e.get<Velocity>();
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<Tag>()
.event(flecs::OnAdd)
.each([](flecs::entity e, Tag) {
e.set<Position>({ 10, 20 });
});

world.observer<Position>()
.event(flecs::OnSet)
.each([](flecs::entity e, Position&) {
e.set<ns::Velocity>({ 1, 2 });
});

auto e = world.entity().add<Tag>();

{
const Position *p = e.get<Position>();
test_int(p->x, 10);
test_int(p->y, 20);
}

{
const ns::Velocity *v = e.get<ns::Velocity>();
test_int(v->x, 1);
test_int(v->y, 2);
}
}
12 changes: 11 additions & 1 deletion test/cpp/src/main.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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
}
};

Expand Down Expand Up @@ -6971,7 +6981,7 @@ static bake_test_suite suites[] = {
"Observer",
NULL,
NULL,
59,
61,
Observer_testcases
},
{
Expand Down

0 comments on commit 2b985f6

Please sign in to comment.