bevy icon indicating copy to clipboard operation
bevy copied to clipboard

Support more kinds of system params in buildable systems.

Open chescock opened this issue 1 year ago • 3 comments

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.

  • chescock/bevy#1
  • chescock/bevy#2
  • chescock/bevy#3

chescock avatar Jun 27 '24 14:06 chescock

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 avatar Jul 12 '24 20:07 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);
}

ScottKane avatar Jul 12 '24 20:07 ScottKane

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>.

chescock avatar Jul 13 '24 17:07 chescock

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.

ScottKane avatar Jul 15 '24 09:07 ScottKane

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!

chescock avatar Aug 07 '24 01:08 chescock