From 1e35ea187b4ddad89aa92dc97a60883a5719aae8 Mon Sep 17 00:00:00 2001 From: Sander Mertens Date: Wed, 19 Jul 2023 01:41:07 -0700 Subject: [PATCH] Fix issue where C++ component with name of core entity would be incorrectly registered --- flecs.c | 18 +++++++++-------- flecs.h | 13 +++++++++--- include/flecs.h | 9 ++++++++- .../flecs/addons/cpp/mixins/module/impl.hpp | 4 ++-- src/addons/flecs_cpp.c | 4 ++-- src/addons/meta_c.c | 7 ++++--- src/filter.c | 2 +- src/hierarchy.c | 5 +++-- test/addons/src/Modules.c | 4 ++-- test/api/src/Lookup.c | 14 ++++++------- test/cpp_api/project.json | 4 +++- test/cpp_api/src/Module.cpp | 20 +++++++++++++++++++ test/cpp_api/src/World.cpp | 11 ++++++++++ test/cpp_api/src/main.cpp | 14 +++++++++++-- 14 files changed, 95 insertions(+), 34 deletions(-) diff --git a/flecs.c b/flecs.c index b4d79d4fb..ca6143457 100644 --- a/flecs.c +++ b/flecs.c @@ -19151,7 +19151,7 @@ ecs_entity_t ecs_cpp_component_register( /* If no entity is found, lookup symbol to check if the component was * registered under a different name. */ } else if (!implicit_name) { - ent = ecs_lookup_symbol(world, symbol, false); + ent = ecs_lookup_symbol(world, symbol, false, false); ecs_assert(ent == 0 || (ent == id), ECS_INCONSISTENT_COMPONENT_ID, symbol); } @@ -19184,7 +19184,7 @@ ecs_entity_t ecs_cpp_component_register_explicit( if (!name) { // If no name was provided first check if a type with the provided // symbol was already registered. - id = ecs_lookup_symbol(world, symbol, false); + id = ecs_lookup_symbol(world, symbol, false, false); if (id) { existing_name = ecs_get_path_w_sep(world, 0, id, "::", "::"); name = existing_name; @@ -46466,7 +46466,8 @@ ecs_entity_t meta_lookup_array( goto error; } - ecs_entity_t element_type = ecs_lookup_symbol(world, params.type.type, true); + ecs_entity_t element_type = ecs_lookup_symbol( + world, params.type.type, true, true); if (!element_type) { ecs_meta_error(ctx, params_decl, "unknown element type '%s'", params.type.type); @@ -46634,7 +46635,7 @@ ecs_entity_t meta_lookup( } else if (!ecs_os_strcmp(typename, "char*")) { type = ecs_id(ecs_string_t); } else { - type = ecs_lookup_symbol(world, typename, true); + type = ecs_lookup_symbol(world, typename, true, true); } } else { if (!ecs_os_strcmp(typename, "char")) { @@ -46649,7 +46650,7 @@ ecs_entity_t meta_lookup( typename = "flecs.meta.string"; } - type = ecs_lookup_symbol(world, typename, true); + type = ecs_lookup_symbol(world, typename, true, true); } if (count != 1) { @@ -50616,7 +50617,7 @@ int flecs_term_id_lookup( return 0; } - ecs_entity_t e = ecs_lookup_symbol(world, name, true); + ecs_entity_t e = ecs_lookup_symbol(world, name, true, true); if (scope && !e) { e = ecs_lookup_child(world, scope, name); } @@ -62398,7 +62399,8 @@ ecs_entity_t ecs_lookup( ecs_entity_t ecs_lookup_symbol( const ecs_world_t *world, const char *name, - bool lookup_as_path) + bool lookup_as_path, + bool recursive) { if (!name) { return 0; @@ -62413,7 +62415,7 @@ ecs_entity_t ecs_lookup_symbol( } if (lookup_as_path) { - return ecs_lookup_fullpath(world, name); + return ecs_lookup_path_w_sep(world, 0, name, ".", NULL, recursive); } error: diff --git a/flecs.h b/flecs.h index fc3d65921..67e5e76a0 100644 --- a/flecs.h +++ b/flecs.h @@ -5960,12 +5960,19 @@ ecs_entity_t ecs_lookup_path_w_sep( * * This operation can be useful to resolve, for example, a type by its C * identifier, which does not include the Flecs namespacing. + * + * @param world The world. + * @param symbol The symbol. + * @param lookup_as_path If not found as a symbol, lookup as path. + * @param recursive If looking up as path, recursively traverse up the tree. + * @return The entity if found, else 0. */ FLECS_API ecs_entity_t ecs_lookup_symbol( const ecs_world_t *world, const char *symbol, - bool lookup_as_path); + bool lookup_as_path, + bool recursive); /** Get a path identifier for an entity. * This operation creates a path that contains the names of the entities from @@ -27900,7 +27907,7 @@ ecs_entity_t do_import(world& world, const char *symbol) { ecs_set_scope(world, scope); // It should now be possible to lookup the module - ecs_entity_t m = ecs_lookup_symbol(world, symbol, true); + ecs_entity_t m = ecs_lookup_symbol(world, symbol, true, false); ecs_assert(m != 0, ECS_MODULE_UNDEFINED, symbol); ecs_assert(m == m_c, ECS_INTERNAL_ERROR, NULL); @@ -27913,7 +27920,7 @@ template flecs::entity import(world& world) { const char *symbol = _::symbol_name(); - ecs_entity_t m = ecs_lookup_symbol(world, symbol, true); + ecs_entity_t m = ecs_lookup_symbol(world, symbol, true, false); if (!_::cpp_type::registered(world)) { diff --git a/include/flecs.h b/include/flecs.h index ba89b268f..324c98301 100644 --- a/include/flecs.h +++ b/include/flecs.h @@ -3207,12 +3207,19 @@ ecs_entity_t ecs_lookup_path_w_sep( * * This operation can be useful to resolve, for example, a type by its C * identifier, which does not include the Flecs namespacing. + * + * @param world The world. + * @param symbol The symbol. + * @param lookup_as_path If not found as a symbol, lookup as path. + * @param recursive If looking up as path, recursively traverse up the tree. + * @return The entity if found, else 0. */ FLECS_API ecs_entity_t ecs_lookup_symbol( const ecs_world_t *world, const char *symbol, - bool lookup_as_path); + bool lookup_as_path, + bool recursive); /** Get a path identifier for an entity. * This operation creates a path that contains the names of the entities from diff --git a/include/flecs/addons/cpp/mixins/module/impl.hpp b/include/flecs/addons/cpp/mixins/module/impl.hpp index 47f59395d..d99c2e7d2 100644 --- a/include/flecs/addons/cpp/mixins/module/impl.hpp +++ b/include/flecs/addons/cpp/mixins/module/impl.hpp @@ -26,7 +26,7 @@ ecs_entity_t do_import(world& world, const char *symbol) { ecs_set_scope(world, scope); // It should now be possible to lookup the module - ecs_entity_t m = ecs_lookup_symbol(world, symbol, true); + ecs_entity_t m = ecs_lookup_symbol(world, symbol, true, false); ecs_assert(m != 0, ECS_MODULE_UNDEFINED, symbol); ecs_assert(m == m_c, ECS_INTERNAL_ERROR, NULL); @@ -39,7 +39,7 @@ template flecs::entity import(world& world) { const char *symbol = _::symbol_name(); - ecs_entity_t m = ecs_lookup_symbol(world, symbol, true); + ecs_entity_t m = ecs_lookup_symbol(world, symbol, true, false); if (!_::cpp_type::registered(world)) { diff --git a/src/addons/flecs_cpp.c b/src/addons/flecs_cpp.c index 54dbe8dea..0176d4c64 100644 --- a/src/addons/flecs_cpp.c +++ b/src/addons/flecs_cpp.c @@ -334,7 +334,7 @@ ecs_entity_t ecs_cpp_component_register( /* If no entity is found, lookup symbol to check if the component was * registered under a different name. */ } else if (!implicit_name) { - ent = ecs_lookup_symbol(world, symbol, false); + ent = ecs_lookup_symbol(world, symbol, false, false); ecs_assert(ent == 0 || (ent == id), ECS_INCONSISTENT_COMPONENT_ID, symbol); } @@ -367,7 +367,7 @@ ecs_entity_t ecs_cpp_component_register_explicit( if (!name) { // If no name was provided first check if a type with the provided // symbol was already registered. - id = ecs_lookup_symbol(world, symbol, false); + id = ecs_lookup_symbol(world, symbol, false, false); if (id) { existing_name = ecs_get_path_w_sep(world, 0, id, "::", "::"); name = existing_name; diff --git a/src/addons/meta_c.c b/src/addons/meta_c.c index 2d7d1e4ad..256752a31 100644 --- a/src/addons/meta_c.c +++ b/src/addons/meta_c.c @@ -472,7 +472,8 @@ ecs_entity_t meta_lookup_array( goto error; } - ecs_entity_t element_type = ecs_lookup_symbol(world, params.type.type, true); + ecs_entity_t element_type = ecs_lookup_symbol( + world, params.type.type, true, true); if (!element_type) { ecs_meta_error(ctx, params_decl, "unknown element type '%s'", params.type.type); @@ -640,7 +641,7 @@ ecs_entity_t meta_lookup( } else if (!ecs_os_strcmp(typename, "char*")) { type = ecs_id(ecs_string_t); } else { - type = ecs_lookup_symbol(world, typename, true); + type = ecs_lookup_symbol(world, typename, true, true); } } else { if (!ecs_os_strcmp(typename, "char")) { @@ -655,7 +656,7 @@ ecs_entity_t meta_lookup( typename = "flecs.meta.string"; } - type = ecs_lookup_symbol(world, typename, true); + type = ecs_lookup_symbol(world, typename, true, true); } if (count != 1) { diff --git a/src/filter.c b/src/filter.c index 094edadb8..b33b51255 100644 --- a/src/filter.c +++ b/src/filter.c @@ -146,7 +146,7 @@ int flecs_term_id_lookup( return 0; } - ecs_entity_t e = ecs_lookup_symbol(world, name, true); + ecs_entity_t e = ecs_lookup_symbol(world, name, true, true); if (scope && !e) { e = ecs_lookup_child(world, scope, name); } diff --git a/src/hierarchy.c b/src/hierarchy.c index 9a219ecea..699c7a983 100644 --- a/src/hierarchy.c +++ b/src/hierarchy.c @@ -349,7 +349,8 @@ ecs_entity_t ecs_lookup( ecs_entity_t ecs_lookup_symbol( const ecs_world_t *world, const char *name, - bool lookup_as_path) + bool lookup_as_path, + bool recursive) { if (!name) { return 0; @@ -364,7 +365,7 @@ ecs_entity_t ecs_lookup_symbol( } if (lookup_as_path) { - return ecs_lookup_fullpath(world, name); + return ecs_lookup_path_w_sep(world, 0, name, ".", NULL, recursive); } error: diff --git a/test/addons/src/Modules.c b/test/addons/src/Modules.c index 68ecad97c..dac41a459 100644 --- a/test/addons/src/Modules.c +++ b/test/addons/src/Modules.c @@ -280,11 +280,11 @@ void Modules_lookup_by_symbol() { ECS_IMPORT(world, SimpleModule); - ecs_entity_t e = ecs_lookup_symbol(world, "Position", true); + ecs_entity_t e = ecs_lookup_symbol(world, "Position", true, true); test_assert(e != 0); test_assert(e == ecs_id(Position)); - e = ecs_lookup_symbol(world, "SimpleFooComponent", true); + e = ecs_lookup_symbol(world, "SimpleFooComponent", true, true); test_assert(e != 0); test_assert(e == ecs_id(SimpleFooComponent)); diff --git a/test/api/src/Lookup.c b/test/api/src/Lookup.c index 51141543c..2ca3e9aa5 100644 --- a/test/api/src/Lookup.c +++ b/test/api/src/Lookup.c @@ -178,7 +178,7 @@ void Lookup_lookup_symbol_by_id() { ecs_world_t *world = ecs_mini(); ecs_ensure(world, 1000); - ecs_entity_t e = ecs_lookup_symbol(world, "1000", true); + ecs_entity_t e = ecs_lookup_symbol(world, "1000", true, true); test_int(e, 1000); ecs_fini(world); @@ -188,7 +188,7 @@ void Lookup_lookup_symbol_w_digit() { ecs_world_t *world = ecs_mini(); ecs_entity_t e = ecs_set_symbol(world, 0, "10_id"); - ecs_entity_t e2 = ecs_lookup_symbol(world, "10_id", true); + ecs_entity_t e2 = ecs_lookup_symbol(world, "10_id", true, true); test_assert(e == e2); ecs_fini(world); @@ -297,7 +297,7 @@ void Lookup_lookup_null() { void Lookup_lookup_symbol_null() { ecs_world_t *world = ecs_mini(); - test_assert(ecs_lookup_symbol(world, NULL, true) == 0); + test_assert(ecs_lookup_symbol(world, NULL, true, true) == 0); ecs_fini(world); } @@ -389,11 +389,11 @@ void Lookup_lookup_path_wildcard_from_scope() { void Lookup_resolve_builtin_symbols() { ecs_world_t *world = ecs_mini(); - test_assert(ecs_lookup_symbol(world, "EcsComponent", false) == ecs_id(EcsComponent)); - test_assert(ecs_lookup_symbol(world, "EcsIdentifier", false) == ecs_id(EcsIdentifier)); + test_assert(ecs_lookup_symbol(world, "EcsComponent", false, true) == ecs_id(EcsComponent)); + test_assert(ecs_lookup_symbol(world, "EcsIdentifier", false, true) == ecs_id(EcsIdentifier)); - test_assert(ecs_lookup_symbol(world, "EcsName", false) == EcsName); - test_assert(ecs_lookup_symbol(world, "EcsSymbol", false) == EcsSymbol); + test_assert(ecs_lookup_symbol(world, "EcsName", false, true) == EcsName); + test_assert(ecs_lookup_symbol(world, "EcsSymbol", false, true) == EcsSymbol); ecs_fini(world); } diff --git a/test/cpp_api/project.json b/test/cpp_api/project.json index 1bd0425be..17b738f3e 100644 --- a/test/cpp_api/project.json +++ b/test/cpp_api/project.json @@ -986,7 +986,8 @@ "implicit_module", "module_in_namespace_w_root_name", "module_as_entity", - "module_as_component" + "module_as_component", + "module_with_core_name" ] }, { "id": "ImplicitComponents", @@ -1073,6 +1074,7 @@ "reregister_after_reset_w_hooks_and_in_use", "reregister_after_reset_w_hooks_and_in_use_implicit", "register_component_w_reset_in_multithreaded", + "register_component_w_core_name", "count", "count_id", "count_pair", diff --git a/test/cpp_api/src/Module.cpp b/test/cpp_api/src/Module.cpp index 3c230e297..fe87dc3ed 100644 --- a/test/cpp_api/src/Module.cpp +++ b/test/cpp_api/src/Module.cpp @@ -56,6 +56,13 @@ class NamedModuleInRoot { } +struct Module { + Module(flecs::world& world) { + world.module(); + world.component(); + } +}; + void Module_import() { flecs::world world; auto m = world.import(); @@ -256,3 +263,16 @@ void Module_module_as_component() { auto e = world.component(); test_assert(m == e); } + +void Module_module_with_core_name() { + flecs::world world; + + flecs::entity m = world.import(); + test_assert(m != 0); + test_str(m.path().c_str(), "::Module"); + + flecs::entity pos = m.lookup("Position"); + test_assert(pos != 0); + test_str(pos.path().c_str(), "::Module::Position"); + test_assert(pos == world.id()); +} diff --git a/test/cpp_api/src/World.cpp b/test/cpp_api/src/World.cpp index 532269c42..eb739dc7e 100644 --- a/test/cpp_api/src/World.cpp +++ b/test/cpp_api/src/World.cpp @@ -303,6 +303,16 @@ void World_register_component_w_reset_in_multithreaded() { test_int(p->y, 20); } +struct Module { }; + +void World_register_component_w_core_name() { + flecs::world ecs; + + flecs::entity c = ecs.component(); + test_assert(c != 0); + test_str(c.path().c_str(), "::Module"); +} + template struct Tmp { int32_t v; }; struct Test { }; @@ -1704,3 +1714,4 @@ void World_id_from_pair_type() { test_assert(id.first() == ecs.id()); test_assert(id.second() == ecs.id()); } + diff --git a/test/cpp_api/src/main.cpp b/test/cpp_api/src/main.cpp index 7d1e2ba37..98801839f 100644 --- a/test/cpp_api/src/main.cpp +++ b/test/cpp_api/src/main.cpp @@ -945,6 +945,7 @@ void Module_implicit_module(void); void Module_module_in_namespace_w_root_name(void); void Module_module_as_entity(void); void Module_module_as_component(void); +void Module_module_with_core_name(void); // Testsuite 'ImplicitComponents' void ImplicitComponents_add(void); @@ -1023,6 +1024,7 @@ void World_register_meta_after_reset(void); void World_reregister_after_reset_w_hooks_and_in_use(void); void World_reregister_after_reset_w_hooks_and_in_use_implicit(void); void World_register_component_w_reset_in_multithreaded(void); +void World_register_component_w_core_name(void); void World_count(void); void World_count_id(void); void World_count_pair(void); @@ -4902,6 +4904,10 @@ bake_test_case Module_testcases[] = { { "module_as_component", Module_module_as_component + }, + { + "module_with_core_name", + Module_module_with_core_name } }; @@ -5195,6 +5201,10 @@ bake_test_case World_testcases[] = { "register_component_w_reset_in_multithreaded", World_register_component_w_reset_in_multithreaded }, + { + "register_component_w_core_name", + World_register_component_w_core_name + }, { "count", World_count @@ -6195,7 +6205,7 @@ static bake_test_suite suites[] = { "Module", NULL, NULL, - 11, + 12, Module_testcases }, { @@ -6223,7 +6233,7 @@ static bake_test_suite suites[] = { "World", NULL, NULL, - 96, + 97, World_testcases }, {