Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix issue with nested observers and implicit namespaced component reg… #1476

Merged
merged 1 commit into from
Dec 17, 2024
Merged

Fix issue with nested observers and implicit namespaced component reg… #1476

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 30 additions & 15 deletions distr/flecs.c
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Loading