Add `EntityMut::components` and `EntityRef` and `EntityWorldMut` equivalent about bevy

[cart]

But a single unified API (that exactly matches the Query API) does feel like a solid unification.

[cart]

I was worried our Query infrastructure was missing the necessary pieces to do this efficiently (namely determining query matches without allocating and setting FilteredAccess). But thanks to matches_component_set we have all we need. I have an impl incoming.

[iiYese]

How about:

entity.data::<(&mut A, &B)>();
enttiy.get_data::<(&mut A, &B)>();

entity.data_mut::<(&mut A, &B)>();
enttiy.get_data_mut::<(&mut A, &B)>();

like QueryData.

[cart]

I think try_get and get is a reasonable / std compatible interpretation. I think this particular ambiguity has never been resolved in std and both interpretations are valid.

[iiYese]

These extensions should also be added to FilteredEntityRef & FilteredEntityMut.

[cart]

We could add a get_components_mut method, and have get_components as a way to fetch the read-only form. Given that this is a convenience API designed for handcrafted code, I don't think this is a useful transformation to expose here at the cost of ergonomics.

I think we probably want a read-only variant. Disallowing things like this seems overly borrow-checker-restrictive, especially given that this exists to lift borrow checker constraints:

let (a, b) = entity.components::<(&A, &B)>();
if SOME_CONDITION {
  let (c, d) = entity.components::<(&C, &D)>();
}

I think the only question is naming:

1. components (read only), components_mut (unrestricted)
2. components_read_only (read only), components (unrestricted)
3. read_components (read only), components (unrestricted)

We could also consider replacing the current get/get_mut with this api (especially if the perf is the same ... get/get_mut currently use a smaller / simpler code path, so they might perform differently).

let a = entity.get::<&A>();
let mut a = entity.get_mut::<&A>();
let (a, b) = entity.get::<(&A, &B)>();
let (mut a, b) = entity.get_mut::<(&mut A, &B)>();

Of course, this would be a breaking change. And it does notably make single component accesses less ergonomic:

let a = entity.get::<&A>();

vs

let a = entity.get::<A>();

[cart]

Even more so for get_mut:

let a = entity.get_mut::<&mut A>();

vs

let a = entity.get_mut::<A>();

[cart]

// EntityMut
let (mut a, b) = entity.get_mut::<(&mut A, &B)>;

// Direct World Queries
let mut query = world.query::<(&mut A, &B)>;
let (mut a, b) = query.get_mut(world, SOME_ENTITY);

// Systems
fn system(query: Query<(&mut A, &B)>) {
    let (mut a, b) = query.get_mut(SOME_ENTITY);
} 

Hard to deny how nice the overlap is.

[cart]

(I've wrapped up an implementation so the only remaining question is how we expose it)

[iiYese]

I would like replacing get_(mut) anyway because it gives it parity with QueryData & makes it naturally consistent with the other bevy APIs like add_systems, insert, add_plugins etc. The breaking change is very minor.

[cart]

Just realized that I forgot to do the unwraps on all of those get/get_mut calls :)

I am personally very interested in implicit-unwrap variants (both for Queries and Entities) as I think a high percentage of use cases benefit from that ergonomically. I end up throwing a lot of get().unwrap() into my game code and I would prefer a single friendly method to call for those cases (ideally using the same verb as the fallible variant).

get_X is our convention for returning a result/option for X and x() is our convention for a panicking version of that. For our current methods get() / get_mut(), that leaves us with no X. Not quite sure how to solve that naming problem in a satisfying way.

If we could implement Index for Query, this would be pretty satisfying, although even if query[entity] were possible, the mut vs read-only question comes into play:

let (mut a, b) = entity.query::<(&mut A, &B)>();
let (mut a, b) = entity.get_query::<(&mut A, &B>().unwrap();
let (mut a, b) = query[entity];
let (mut a, b) = query.get_mut(entity).unwrap();

I think our best bet might be get() and try_get()

let (mut a, b) = entity.get::<(&mut A, &B)>();
let (mut a, b) = entity.try_get::<(&mut A, &B>().unwrap();
let (mut a, b) = query.get(entity);
let (mut a, b) = query.try_get(entity).unwrap();

(Notably, GetComponent and TryGetComponent is used for Unity component access)

[alice-i-cecile]

I'm on board with a mutable / immutable split, and unification with get. Panicking variants are fine too: I can just not use them ;)

[cart]

like QueryData.

I think Data is irrelevant / implicit from a user perspective. Everything is "data". And its not an ECS term that users think about.

[iiYese]

I'm more a fan of that redundancy than try_get which is 2 rust terms for fallible.

[cart]

I'm on board with a mutable / immutable split, and unification with get. Panicking variants are fine too: I can just not use them ;)

I think the biggest ergonomic causality of the unified try (non panicking) get (retrieve components) and mut (allow mutations) approach is:

let mut a = entity.get_mut::<A>().unwrap();

Which becomes:

let mut a = entity.try_get_mut::<&mut A>().unwrap();

However it also makes this possible (and often preferable), so I'll call it a win:

let mut a = entity.get_mut::<&mut A>();

We could also invert things / make mutable access the default. That does sort of make sense in the context of queries, where "read only"-ness is an additional constraint added:

let mut a = entity.get::<&mut A>();
// still a mutable `entity` access
let b = entity.get::<&B>();
let c = entity.get_ref::<&C>();

That approach would also remove a bunch of mut stutters in cases like this:

fn system(query: Query<&mut A>) {
  let mut a = query.get(ENTITY);
}

At the cost of making ref necessary in cases where multiple reads take place:

fn system(query: Query<&A>) {
  // this is fine, but it borrows query mutably
  let a = query.get(ENTITY);
}

fn system(query: Query<&A>) {
  let a1 = query.get(ENTITY_1);
   // this would fail
  let a2 = query.get(ENTITY_2);
}

fn system(query: Query<&A>) {
  let a1 = query.get_ref(ENTITY_1);
  // this succeeds
  let a2 = query.get_ref(ENTITY_2);
}

[cart]

I'm more a fan of that redundancy than try_get which is 2 rust terms for fallible.

I do see your point, but get does also fall into the "non fallible" category in std:

• https://doc.rust-lang.org/std/cell/struct.Cell.html#method.get
• https://doc.rust-lang.org/std/collections/btree_map/struct.OccupiedEntry.html#method.get
• https://doc.rust-lang.org/std/num/struct.NonZero.html#method.get-10

I see it as more of a "generic retrieve some thing based on context" verb.

[cart]

I will concede that those are all cases where there is no fallible option. In std, in cases where there is a fallible variant, get is that variant.

That being said, std tends to use the Index operator for the panicking variants in these cases, which isn't an option for us.

[iiYese]

We could also invert things / make mutable access the default.

I don't think that's a good idea because:

• It would be the only inconsistent exception among every other access API like Query::get_(mut) & World::(get)_resource_(mut).
• It's also inconsistent with all of rust because rust is immutable by default making it not only an exception among bevy ecs APIs but also an exception among many rust library APIs including libraries in bevy.
• Read access is the thing you can get multiple times while mut access you can only get once. Meaning the thing you can do multiple times is the more verbose thing.

Edit: Missed the query rename but point stands.

[cart]

Agreed those are good arguments :)