Make component access in the Query abstraction safe

[cart]

Unlike the old legion api, bevy_ecs systems now no longer have direct world (or subworld) access. Instead the plan is to give system queries safe direct access to the components/archetypes in the query.

fn system(mut query: Query<(&mut A, &B )>) {
  // iteration
  for (a, b) in &mut query.iter() {
    // do stuff here
  }

  // direct component access
  let component = query.get::<A>(some_entity).unwrap();
}

Currently direct component access works by just letting queries access their internal World reference directly:

query.get::<Component>(entity) is equivalent to world.get::<Component>(entity).

This is technically safe because hecs doesnt allow parallel access to components (it would panic if two queries tried to get write access to the same component in parallel), but its harder to reason about and could cause programs that appear to work to intermittently fail when invalid accesses occur. Queries should have a layer of checks when accessing an entity's components directly to ensure the query has permission to the given entity's component. This allows potentially invalid component accesses to fail fast (and consistently). It would mean that if you want to access an entity's component directly from a query, that component needs to match the query's archetype / mutability exactly.

When combined with a proper parallel scheduler, this gets us most of the way there to acceptable / consistent safety levels, because two systems with conflicting sets of queries would not be scheduled to run at the same time. The last "gap" would then be two queries within the same system potentially accessing the same components.

for example:

fn system(mut query_1: Query<(&mut A, &B )>, mut query_2: Query<&mut A>) {
  for (a, b) in &mut query_1.iter() {
    for a_might_collide in &mut query_2.iter() { /* might panic here */ }
  }
}

There are a few solutions to this problem:

1. re-add "subworlds" with a world-splitting api like legion. this would add compile time "safety" at the cost of some clarity and ergonomics. note: legion's approach still has runtime panics for passing in the wrong subworld into a query.
2. add some shared state between queries that ensures we fail immediately when something is potentially unsafe. this would be relatively inexpensive and would actually be logically similar to what happens in legion's subworld splitting apis.
3. just rely on the existing safety checks

I personally prefer (2). I really like not needing to pass in and split subworlds. I think the planned Query api is much more pleasant to use and I'm willing to allow run time failures in this case as long as we fail fast and consistently if you try to do something "wrong".

That being said, if anyone has other ideas or opinions on the right path forward, feel free to chime in.

[cart]

Oooh good news: it turns out I was wrong about the existing safety checks in the query_1 and query_2 example. They will already immediately fail if two mutable archetype accesses collide, the checks arent per-entity. hecs already implement the solution outlined in (2) for us.

So the main question at this point is: "do we want to rely on that behavior or use a legion-style world api".

[Moxinilian]

We should rely on the original behavior in my opinion. Not only is it easier to reason about for users, it’s also less work and does exactly what is needed.

[cart]

closed by a7bab75