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

Unintuitive behavior of setting entity names as numbers #1203

Closed
GsLogiMaker opened this issue May 1, 2024 · 4 comments
Closed

Unintuitive behavior of setting entity names as numbers #1203

GsLogiMaker opened this issue May 1, 2024 · 4 comments
Labels
enhancement New feature or request

Comments

@GsLogiMaker
Copy link

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 GsLogiMaker added the bug Something isn't working label May 1, 2024
@GsLogiMaker
Copy link
Author

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
Copy link
Contributor

copygirl commented May 1, 2024

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 SanderMertens added enhancement New feature or request and removed bug Something isn't working labels May 2, 2024
@SanderMertens
Copy link
Owner

SanderMertens commented May 2, 2024

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

@SanderMertens
Copy link
Owner

I just checked in a fix on the v4 branch. To lookup an entity by id you'll have to use a #:

world.lookup("100"); // returns entity with name "100"
world.lookup("#100"); // returns entity with id 100

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants