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.

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 reregistering a component's symbol after deletion #1477

Merged
merged 1 commit into from
Dec 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 1 addition & 2 deletions distr/flecs.h
Original file line number Diff line number Diff line change
Expand Up @@ -27282,7 +27282,7 @@ struct type_impl {
// component has not yet been registered, or the component is used
// across more than one binary), or if the id does not exists in the
// world (indicating a multi-world application), register it.
if (!s_id || (world && !ecs_exists(world, s_id))) {
if (!s_id || (world && !ecs_is_alive(world, s_id))) {
init(s_id ? s_id : id, allow_tag);

ecs_assert(!id || s_id == id, ECS_INTERNAL_ERROR, NULL);
Expand Down Expand Up @@ -31188,7 +31188,6 @@ flecs::entity import(world& world) {
ecs_entity_t m = ecs_lookup_symbol(world, symbol, true, false);

if (!_::type<T>::registered(world)) {

/* Module is registered with world, initialize static data */
if (m) {
_::type<T>::init(m, false);
Expand Down
2 changes: 1 addition & 1 deletion include/flecs/addons/cpp/component.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ struct type_impl {
// component has not yet been registered, or the component is used
// across more than one binary), or if the id does not exists in the
// world (indicating a multi-world application), register it.
if (!s_id || (world && !ecs_exists(world, s_id))) {
if (!s_id || (world && !ecs_is_alive(world, s_id))) {
init(s_id ? s_id : id, allow_tag);

ecs_assert(!id || s_id == id, ECS_INTERNAL_ERROR, NULL);
Expand Down
1 change: 0 additions & 1 deletion include/flecs/addons/cpp/mixins/module/impl.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ flecs::entity import(world& world) {
ecs_entity_t m = ecs_lookup_symbol(world, symbol, true, false);

if (!_::type<T>::registered(world)) {

/* Module is registered with world, initialize static data */
if (m) {
_::type<T>::init(m, false);
Expand Down
4 changes: 3 additions & 1 deletion test/cpp/project.json
Original file line number Diff line number Diff line change
Expand Up @@ -1104,7 +1104,8 @@
"rename_namespace_longer",
"rename_namespace_nested",
"rename_reparent_root_module",
"no_recycle_after_rename_reparent"
"no_recycle_after_rename_reparent",
"reimport_after_delete"
]
}, {
"id": "ImplicitComponents",
Expand Down Expand Up @@ -1167,6 +1168,7 @@
"reregister_after_reset",
"reregister_after_reset_w_namespace",
"reregister_namespace",
"reregister_after_delete",
"implicit_reregister_after_reset",
"reregister_after_reset_different_name",
"register_short_template",
Expand Down
1 change: 1 addition & 0 deletions test/cpp/src/Entity.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4896,6 +4896,7 @@ void Entity_iter_empty_type(void) {
int32_t count = 0;

for (auto id : e.type()) {
test_assert(id != 0);
count ++;
}

Expand Down
18 changes: 18 additions & 0 deletions test/cpp/src/Module.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -528,3 +528,21 @@ void Module_no_recycle_after_rename_reparent(void) {
test_assert(p == 0);
test_str(m.name(), "MyModule");
}

void Module_reimport_after_delete(void) {
flecs::world ecs;

{
auto m = ecs.import<Module>();
test_assert(m.lookup("Position") == ecs.component<Position>());
test_assert(m == ecs.entity<Module>());
}

ecs.entity<Module>().destruct();

{
auto m = ecs.import<Module>();
test_assert(m.lookup("Position") == ecs.component<Position>());
test_assert(m == ecs.entity<Module>());
}
}
21 changes: 21 additions & 0 deletions test/cpp/src/World.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,27 @@ void World_reregister_after_reset_different_name(void) {
ecs.component<Position>("Velocity");
}

void World_reregister_after_delete(void) {
flecs::world ecs;

auto c = ecs.component<Position>();
test_str(c.name(), "Position");
test_str(c.path(), "::Position");
test_str(c.symbol(), "Position");

c.destruct();

test_assert(!c.is_alive());

auto d = ecs.component<Position>();
test_assert(c == d);
test_assert(c.is_alive());

test_str(c.name(), "Position");
test_str(c.path(), "::Position");
test_str(c.symbol(), "Position");
}

void World_register_component_w_reset_in_multithreaded(void) {
flecs::world ecs;

Expand Down
14 changes: 12 additions & 2 deletions test/cpp/src/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1066,6 +1066,7 @@ void Module_rename_namespace_longer(void);
void Module_rename_namespace_nested(void);
void Module_rename_reparent_root_module(void);
void Module_no_recycle_after_rename_reparent(void);
void Module_reimport_after_delete(void);

// Testsuite 'ImplicitComponents'
void ImplicitComponents_add(void);
Expand Down Expand Up @@ -1122,6 +1123,7 @@ void World_different_comp_same_name(void);
void World_reregister_after_reset(void);
void World_reregister_after_reset_w_namespace(void);
void World_reregister_namespace(void);
void World_reregister_after_delete(void);
void World_implicit_reregister_after_reset(void);
void World_reregister_after_reset_different_name(void);
void World_register_short_template(void);
Expand Down Expand Up @@ -5558,6 +5560,10 @@ bake_test_case Module_testcases[] = {
{
"no_recycle_after_rename_reparent",
Module_no_recycle_after_rename_reparent
},
{
"reimport_after_delete",
Module_reimport_after_delete
}
};

Expand Down Expand Up @@ -5768,6 +5774,10 @@ bake_test_case World_testcases[] = {
"reregister_namespace",
World_reregister_namespace
},
{
"reregister_after_delete",
World_reregister_after_delete
},
{
"implicit_reregister_after_reset",
World_implicit_reregister_after_reset
Expand Down Expand Up @@ -7002,7 +7012,7 @@ static bake_test_suite suites[] = {
"Module",
NULL,
NULL,
22,
23,
Module_testcases
},
{
Expand All @@ -7023,7 +7033,7 @@ static bake_test_suite suites[] = {
"World",
NULL,
NULL,
116,
117,
World_testcases
},
{
Expand Down
Loading