SanderMertens / flecs

A fast entity component system (ECS) for C & C++
https://www.flecs.dev
MIT License
5.6k stars 407 forks source link

Unintuitive behavior of setting entity names as numbers #1203

Open GsLogiMaker opened 2 weeks ago

GsLogiMaker commented 2 weeks ago

Describe the bug Entities named by a number (say "1" for example) act unintuitively. I assume the behavior I'm about to describe is intentional, but I would argue that it should not be.

To Reproduce (Case 1)

  1. Create a new entity ecs_entity_t e = ecs_new_id(world);
  2. Set its name to "1" ecs_set_name(world, e, "1");
  3. Get that entity by its name ecs_entity_t got = ecs_lookup(world, "1")
  4. Check the gotten entity to find that its a completely different entity. In this case that is the "Component" entity with an ID of 1.

To Reproduce (Case 2)

  1. Create a new entity named "1" ecs_entity_t one = ecs_set_name(world, 0, "1");
  2. Check the returned entity to find that it actually matched to an already existing entity. In this case that would be the "Component" entity with an ID of 1.

To Reproduce (Case 3)

  1. (Normally, ecs_lookup won't create new entities. However, it makes an exception for number names.)
  2. Lookup entity named "500" (No entity has this ID or name yet) ecs_entity_t got = ecs_lookup(world, "500");
  3. Check the gotten entity to find that it actually created a brand new entity with an ID of 500.

Expected behavior In all of these cases, I expected the name of the entity to be completely separate from the ID of the entity. This unique behavior actually prevents users from naming their entities exclusively by numbers without unexpected consequences. Furthermore, this behavior appears to be completely unmentioned in the documentation.

I believe this feature should be removed, as having ecs_lookup essentially act as a conversion from string to ID is unexpected, redundant (there are better ways of doing this), restrictive (no sane code can use number names), and error prone.

Thank you!

GsLogiMaker commented 2 weeks ago

This would be a breaking change if applied to 3.x, but since 4.x is expected to have breaking changes, and is still in alpha, I hope that this change would make it in there.

copygirl commented 2 weeks ago

At this time you can't completely separate the entity ID and name, because the DSL can parse not only entity names / symbols, but also numeric entity IDs. (This might come in handy when creating queries at runtime, though might be ugly using string expressions.) An alternative solution would be to assert when one attempts to supply a numeric ID to functions that set the entity name. A potential rule could be "entity names and symbols must not start with a digit".

Otherwise, I'd say I agree. When a function expects a name or symbol, it should not look up an entity by numeric ID. If that functionality is desired, perhaps it could be done through a separate function, like one that exposes the same method of lookup that the DSL parser uses? (If that doesn't already exist.)

SanderMertens commented 2 weeks ago

The main reason for this functionality is that interfaces that can only use strings (like the JSON serializer, REST API, and as result the explorer) don't have to treat entities with/without names differently. Say you click on an entity in the explorer that doesn't have a name. To fetch the data for the inspector it'll send a request like this:

localhost:27750/entity/1234

You could differentiate between a name and an id by having something like:

localhost:27750/entity/1234
localhost:27750/entity_id/1234

But that would mean that the explorer (+ any other applications that use the REST API) has to be updated to explicitly handle entities with/without names separately. Since you can't enforce correctness across different clients, this would likely introduce a lot of new bugs where code doesn't correctly handle the two.

Another reason why this historically has been necessary is because 0 has been used to express DSL queries, like:

(ChildOf, 0) // get all root entities

I'm not opposed to changing this behavior (I've also ran into scenarios where I'd just like to use a number as a name), but I do not want to introduce a secondary entirely separate mechanism for uniquely identifying entities. Perhaps a middle ground is to for v4 do this:

1234 // entity with name 1234
#1234 // entity with id 1234