World cell queries

[Frizi]

trafficstars

Rendered Implementation PR

Add query and command API on WorldCell instances with shared access.

[Davier]

I see two goals in this RFC:

1. fine-grained runtime checked queries in WorldCell
2. make "overlayed" changes to a WorldCell

Goal 1 seems reasonable, and if I remember correctly it wasn't implemented in ECSv2 mainly due to a lack of use cases. Quoting that PR: "The api is currently limited to resource access, but it can and should be extended to queries / entity component access."

Goal 2 looks like a complex solution to a problem that I'm not yet convinced exist. All of the examples given can be tweaked to work with the current World API, usually by collecting the queries when they are iterated.

#[derive(Component, Clone, Copy)]
struct A;
#[derive(Component, Clone, Copy)]
struct B;

fn test() {
    let mut world = World::default();

    let mut query = world.query_filtered::<Entity, With<A>>();
    let entities = query.iter(&world).collect::<Vec<_>>();
    for entity in entities {
        world.entity_mut(entity).insert(B);
    }
}

fn test2() {
    // create world with two entities
    let mut world = World::default();
    let e1 = world.spawn().insert(A).insert(B).id();
    let e2 = world.spawn().insert(A).id();

    let mut query = world.query_filtered::<Entity, (With<A>, Without<B>)>();

    // verify query output before any modifications
    assert_eq!(query.iter(&world).collect::<Vec<_>>(), vec![e2]);

    // remove C from one entity, insert on another
    world.entity_mut(e1).remove::<B>();
    world.entity_mut(e2).insert(B);

    // new query results reflect the changes immediately
    assert_eq!(query.iter(&world).collect::<Vec<_>>(), vec![e1]);
}

fn test3() {
    let mut world = World::default();
    let mut query = world.query::<(Entity, &A)>();

    let result1 = query
        .iter(&world)
        .map(|(entity, a)| (entity, *a))
        .collect::<Vec<_>>();
    let inserted = world.spawn().insert(A).id();
    let result2 = query.iter(&world).collect::<Vec<_>>();

    // iterator created before insert doesn't include new entity
    assert!(!result1.iter().any(|(id, _)| *id == inserted));
    // iterator created after insert does include new entity
    assert!(result2.iter().any(|(id, _)| *id == inserted));
}

Could you provide examples that require the world overlays? Meanwhile I would recommend splitting Goal 1 into its own PR (not sure an RFC is needed since the API should be the same as World::query)

[alice-i-cecile]

I understand and emphasize with the pain here: we don't have great solutions for rapid prototyping and complex one-off logic. We absolutely need better tools for "fine-grained runtime checked" access to the World: a number of very common situations are needlessly painful.

However, I don't think this is the way. "Working" prototype code has a nasty habit of turning into production code, and this feature will be a large, hard-to-manage source of technical debt and bugs.

The API uses too big of a gun in the name of usability-when-first-writing: users are encouraged to just toss messy, arbitrary code in these exclusive systems, which will panic in unpredictable ways at runtime based on the world's state. The performance issues are fine: I agree with you that performance vs. ease of development should be a choice left to the end user. I just genuinely don't think this will make writing complex gameplay code easier when the full lifecycle is accounted.

[kabergstrom]

The goal of this RFC is to be able to write code with looser restrictions than the existing APIs, while sacrificing performance in a few ways. Parallelism will not be possible, and there will be more runtime overhead. Borrowing semantics will be upheld at runtime. Queries must follow visibility rules when adding/removing components/entities during iteration, which is not free.

The changes should enable writing familiar code that closely resembles what can be written in popular game engines where gameplay code is single-threaded-only. The key benefit here compared to other engines is that code can access the same data as the existing parallel APIs, meaning users can migrate code between the APIs depending on the evolving needs of their project.

On Discord, there has been a lot of discussion around whether this API enables "bad code", or whether the usecase that has lead to this work could be implemented in some way with existing APIs. I am not interested in further discussing either topics, since it derails from the goals of this API proposal: to be able to write gameplay code with fewer restrictions. Just let me write shit code.

I understand there to be two separate conversations:

1. Should bevy_ecs support an API that is minimally restrictive by trading off performance and runtime panics?
2. What is the best way to implement the goals I've described in bevy_ecs?

[kabergstrom]

Could you provide examples that require the world overlays?

Arbitrary callbacks which add/remove entities or components, even while borrows to disjoint components are held elsewhere.

[hymm]

I'm against the idea of instant commands. Not having to confront the complexities of deferred commands at all will lead to code that is impossible to pull apart and the prototype code will end up in production. Commands should be as similar as possible to what you can do in normal systems and then applied by an explicit call like command_queue.apply(world).

[alice-i-cecile]

Coming back to this, I have a hard time following the logical flow of this proposal and agree with Davier above. Why do these changes belong together?

I can get on board with "maximally granular" WorldCell at the expense of performance and runtime panics, provided we have very strong and clear docs for it. Escape hatches are important, and while we can gradually reduce the necessity of this tool, we're not there yet and will probably never be able to predict all possibly useful access patterns. Hopefully we can even provide a nice set of errors for it and make some of the errors recoverable!

We need to solve https://github.com/bevyengine/bevy/issues/3096 for exclusive world access to feel usable for arbitrary game logic. But "instant commands" in the fashion proposed is too error-prone and the behavior is too different from the behavior of the commands used in ordinary systems.

[Davier]

Could you provide examples that require the world overlays?

Arbitrary callbacks which add/remove entities or components, even while borrows to disjoint components are held elsewhere.

My point is that you don't need to hold borrows elsewhere, that is what entity ids are for. I think this represents what you describe:

#[derive(Component)]
struct A;
#[derive(Component)]
struct B;

fn test() {
    // Create world
    let mut world = World::default();
    let e1 = world.spawn().insert(A).insert(B).id();
    let e2 = world.spawn().insert(A).id();
    
    // Borrow stuff
    let mut cell = world.cell();
    let e1_a = cell.entity(e1).get_mut::<A>().unwrap();
    let e2_a = cell.entity(e2).get_mut::<A>();
    let query = cell.query::<A>();
    
    // Access borrowed things
    for a in query.iter() {}
    
    // Callbacks
    f1(&mut cell, query, e1_a, e2_a);
    
    // Access borrowed things
    for a in query.iter() {}
}

fn f1(cell: &mut WorldCell, query: _, e1_a: _, e2_a: _) {
    // Access borrowed things
    *e1_a = *e2_a;
    for a in query.iter() {}
    // Use commands
    cell.spawn().insert(A);
}

And this is what I suggest:

fn test() {
    // Create world
    let mut world = World::default();
    let e1 = world.spawn().insert(A).insert(B).id();
    let e2 = world.spawn().insert(A).id();
    let query = world.query::<A>();
    
    // Borrow only what is needed, when needed
    for a in query.iter(&world) {}
    
    // Callbacks using `&mut World` and entity ids
    f1(&mut world, query, e1, e2);
    
    // Borrow again
    for a in query.iter(&world) {}
}

fn f1(world: &mut World, query: QueryState<A>, e1: Entity, e2: Entity) {
    // Borrow right before use, possibly using WorldCell
    let cell = world.cell();
    let e1_a = cell.entity(e1).get_mut::<A>().unwrap();
    let e2_a = cell.entity(e2).get::<A>().unwrap();
    // Access borrowed things
    *e1_a = *e2_a;
    for a in query.iter(&world) {}
    // Use commands
    world.spawn().insert(A);
}

Does that solve your use case? If you're worried about the verbosity of calling things like cell.entity(e1).get_mut::<A>().unwrap(); in multiple places, I think there is a simpler solution. Notice how the query is created once, passed around, and iterated using world. Maybe we could have a similar wrapper type for component:

// Nothing is borrowed here
let e1_a = world.get_wrapper::<A>(e1);
let e2_a = world.get_wrapper::<A>(e2);

// In a callback
*e1_a.get_mut(world) = *e2_a.get(world);