Support more kinds of system params in buildable systems.

[chescock]

Objective

Support more kinds of system params in buildable systems, such as a ParamSet or Vec containing buildable params or tuples of buildable params.

Solution

Replace the BuildableSystemParam trait with SystemParamBuilder to make it easier to compose builders. Provide implementations for existing buildable params, plus tuples, ParamSet, and Vec.

Examples

// ParamSet of tuple: 
let system = (ParamSetBuilder((
    QueryParamBuilder::new(|builder| { builder.with::<B>(); }),
    QueryParamBuilder::new(|builder| { builder.with::<C>(); }),
)),)
    .build_state(&mut world)
    .build_system(|mut params: ParamSet<(Query<&mut A>, Query<&mut A>)>| {
        params.p0().iter().count() + params.p1().iter().count()
    });
	
// ParamSet of Vec:
let system = (ParamSetBuilder(vec![
    QueryParamBuilder::new_box(|builder| { builder.with::<B>(); }),
    QueryParamBuilder::new_box(|builder| { builder.with::<C>(); }),
]),)
    .build_state(&mut world)
    .build_system(|mut params: ParamSet<Vec<Query<&mut A>>>| {
        let mut count = 0;
        params.for_each(|mut query| count += query.iter_mut().count());
        count
    });

Migration Guide

The API for SystemBuilder has changed. Instead of constructing a builder with a world and then adding params, you first create a tuple of param builders and then supply the world.

// Before
let system = SystemBuilder::<()>::new(&mut world)
    .local::<u64>()
    .builder::<Local<u64>>(|x| *x = 10)
    .builder::<Query<&A>>(|builder| { builder.with::<B>(); })
    .build(system);

// After
let system = (
    ParamBuilder,
    LocalBuilder(10),
    QueryParamBuilder::new(|builder| { builder.with::<B>(); }),
)
    .build_state(&mut world)
    .build_system(system);

Possible Future Work

Here are a few possible follow-up changes. I coded them up to prove that this API can support them, but they aren't necessary for this PR.

• Support dynamic system parameters. chescock/bevy#1
• Enable type inference of closure parameter when building dynamic systems. chescock/bevy#2
• Allow deriving a SystemParamBuilder struct when deriving SystemParam. chescock/bevy#3

[ScottKane]

It would be nice if this API could support some of the other SystemParam's, right now I'm working on bindings to bevy using the existing API. As it stands I can create components and systems from C# and they work, however there is currently no way to query for Res/ResMut even though they can be inserted on the world by component id. Would also be nice if this supported Commands, I think this is what this feature needs to really help with other languages working with bevy.

[ScottKane]

In case it's useful to see some of the things I'm using this API to do, currently this will only support one Query per system because doing this for a series of queries is a bit tricky.

#[no_mangle]
pub unsafe extern "cdecl" fn system_new(app: *mut App, request: SystemData) {
    let data = std::slice::from_raw_parts(request.queries, request.count)
        .iter()
        .map(|&ptr| *ptr)
        .collect::<Vec<_>>();

    let system = SystemBuilder::<()>::new((*app).world_mut())
        .builder::<Query<FilteredEntityMut>>(|query| {
            for component in &data {
                let id = ComponentId::new(component.id);
                let kind = &component.kind;

                match kind {
                    ComponentKind::Reference => {
                        query.ref_id(id);
                    }
                    ComponentKind::Mutable => {
                        query.mut_id(id);
                    }
                    ComponentKind::With => {
                        query.with_id(id);
                    }
                    ComponentKind::Without => {
                        query.without_id(id);
                    }
                }
            }
        })
        .build(move |query: Query<FilteredEntityMut>| {
            let mut count = 0usize;

            let components: Vec<*mut u8> = query
                .iter()
                .flat_map(|entity| {
                    count += 1;

                    data.iter()
                        .filter_map(|component| match component.kind {
                            ComponentKind::Reference | ComponentKind::Mutable => Some(
                                entity
                                    .get_by_id(ComponentId::new(component.id))
                                    .unwrap()
                                    .as_ptr(),
                            ),
                            _ => None,
                        })
                        .collect::<Vec<_>>()
                })
                .collect();

            let components_ptr = components.as_ptr();
            std::mem::forget(components);
            (request.callback)(QueryResult {
                components: components_ptr,
                count,
            });
        });

    (*app).add_systems(Update, system);
}

[chescock]

Would also be nice if this supported Commands

Both the existing API and the one in this PR handle arbitrary SystemParams. You can get a Commands today using .param::<Commands>(), and with this PR using ParamBuilder. Is there something more you need there?

---

there is currently no way to query for Res/ResMut even though they can be inserted on the world by component id.

Maybe you want a buildable SystemParam that lets you access a filtered set of resources from the world? It should be possible to make something where you provide ComponentIds of resources to read and write in the builder, and then have methods on the param that check the access then get the resource from the UnsafeWorldCell. It would be a bit like like a FilteredEntityMut for resources.

I don't think this PR would make that easier or harder to implement.

---

currently this will only support one Query per system because doing this for a series of queries is a bit tricky.

This PR implements SystemParam for Vec, which might help if you want a Vec<Query>.

[alice-i-cecile]

Question: what's the point of being able to extract a vec of queries? This seems...generally unhelpful.

[chescock]

I plan to use it for scripting. The idea is to load a script at runtime that does any number of queries, and then build a script runner system with a Vec<Query<FilteredEntityMut>> based on those queries (or ParamSet<Vec<Query<FilteredEntityMut>>> if they might conflict). The script runner system would bind the queries in the script by indexing the Vec.

(Buildable Vec and ParamSet<Vec> are the features I actually wanted here, and it would have been straightforward to implement BuildableSystemParam for them. It just felt wrong to implement buildable ParamSet<Vec> without buildable ParmSet<(tuple,)>, and I didn't see a clean way to build tuples with a builder type.)

I'm happy to split it out to a follow-up PR if that makes this easier to review!

[alice-i-cecile]

Yes please, I'd much prefer reviewing it as a separate PR :)

[alice-i-cecile]

This safety comment isn't clear enough IMO. Make sure to tie back to the safety requirements of SystemParam explicitly.

[alice-i-cecile]

Neutral on the API changes. As is, mildly useful, as it allows for the construction of more complex system param like ParamSet. Generally well-made: good docs and tests.

I genuinely don't understand the value of the Vec SystemParam, and would like to cut it unless there's strong motivation. If it's borderline, please split it out into its own PR.

I quite like all three follow-ups, but I'd like them to each be their own PRs.

P.S. SystemState::build_system is particularly interesting 🤔 Definitely feels like the sort of tool that will get used for shenanigans eventually.

[ScottKane]

there is currently no way to query for Res/ResMut even though they can be inserted on the world by component id.

Maybe you want a buildable SystemParam that lets you access a filtered set of resources from the world? It should be possible to make something where you provide ComponentIds of resources to read and write in the builder, and then have methods on the param that check the access then get the resource from the UnsafeWorldCell. It would be a bit like like a FilteredEntityMut for resources.

I don't think this PR would make that easier or harder to implement.

Yeah I think it wouldn't make sense for this PR, I think it's something that would be needed. From the rust docs for 0.14 I was going off Query and Local both being the only things that implement BuildableSystemParam. I think it would be nice to have Res/ResMut also implement this trait.

[james-j-obrien]

Implementation looks good, although I don't love how it looks aesthetically the utility for composition of paramsets is there.

I think using ParamBuilder in this way will be a little confusing for new users, there's nothing in this snippet that tells me what that identifier is there for so without knowing how the feature works it reads strangely:

let system = (
    ParamBuilder,
    LocalBuilder(10),
    QueryParamBuilder::new(|builder| { builder.with::<B>(); }),
)
    .build_state(&mut world)
    .build_system(system);

Maybe including "default" in there somewhere could help.

Although I see in follow up PRs you can also express it more explicitly like this:
ParamBuilder::local::<u64>()
So I suppose it's not a huge deal.

[chescock]

Maybe including "default" in there somewhere could help.

Yeah, I don't think I quite got the names right, but I couldn't think of better ones. I'm happy to rename things if you have a suggestion!