Add object-oriented `EntityId` methods

[marceline-cramer]

As game codebases scale, I begin to find this extremely noisy and hard to read:

// e is an EntityId
entity::get_component(e, component());
entity::despawn_recursive(e);
entity::add_child(e, child);

I think that a much more readable (and idiomatic if you're looking at Ambient packages as objects) alternative is this:

// e is an EntityId
e.get(component());
e.despawn_recursive();
e.add_child(child);

This would apply for all other functions on EntityId in the entity module such as has_component().

[philpax]

This is something we've discussed a fair bit, but my opinion remains that we should not do this because it implies the object referred to the ID always exists, which isn't the case - that is, during that sequence of calls, e can be despawned at any point, and the successive calls will not really make any sense.

I think the form in which we're most likely to see this is #406, in which we can guarantee that an entity exists while we operate on it, which allows us to offer a richer API.

Another idea is reference-counted entities where having an id to them keeps them alive, but I'm not sure if that will happen any time soon, either.

[marceline-cramer]

Right now, guest-side code is single-threaded, so it's impossible for entities to be despawned while a guest is working with them unless the guest despawns them itself. We can help mitigate the ambiguity of whether entities are alive or not by making .despawn() and .despawn_recursive() take ownership of the entity IDs.

No, this will not solve issues of questionable ID validity, but as it stands, the entity module's functions already have that ambiguity AND the code clutter of calling those functions. I can think of a few times while making Flowerpot that entity ownership shenanigans caused an error that would have been less likely to happen if I had been paying attention to that instead of writing the same long function names repeatedly.

Adding a guard to make sure that entities are alive is as simple as this:

if !entity::exists(e) {
    return;
}

But I think that it's a lot more conducive to habitual error checking if it's as simple as this, instead:

if !e.exists() {
    return;
}

[philpax]

Right now, guest-side code is single-threaded, so it's impossible for entities to be despawned while a guest is working with them unless the guest despawns them itself. We can help mitigate the ambiguity of whether entities are alive or not by making .despawn() and .despawn_recursive() take ownership of the entity IDs.

You can have multiple EntityIds for the same entity, so consuming one of them on despawn won't be able to invalidate the other ones. This is also a problem with async:

let id = Entity::new().spawn();
run_async(async {
  sleep(5.0).await;
  id.set(blah(), 5); // `id` could have been despawned in the meantime
});

There's also the possibility that we expose multiple ECS worlds to the guest (#230 maybe?), so that you need to have a reference to the world in order to do anything. This would fairly easily generalise from our existing API - the functions would end up being methods on a world handle - but it ends up being more unclear if we move the methods to the ID.

For reference, here's the discussion we had about this on the Discord that led to #406: https://discord.com/channels/894505972289134632/1078283561540530216/1108329570916118609

I would like to offer a sleeker API - for the same reasons you mention - but I'm also afraid of implying that the IDs represent the entity, instead of merely pointing to one.

I've seen people struggle with the distinction between entity handles and entities before, as well as entity lifetimes, so I want to make it as obvious as possible, even if it comes at a bit of an ergonomics cost.

That being said, one change that I'm not opposed to and that we should probably consider is to rename the methods to make them less verbose (i.e. drop the _component suffixes where possible), as mentioned in that Discord conversation.

I'm also not that opposed to adding EntityId::exists, because that doesn't let you actually interact with the components of the entity in any meaningful way (which would perpetuate the handle=entity semantic misunderstanding).