Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SystemParamBuilder - Support buildable Vec parameters #14821

Merged
merged 2 commits into from
Aug 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
101 changes: 101 additions & 0 deletions crates/bevy_ecs/src/system/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,15 @@ macro_rules! impl_system_param_builder_tuple {

all_tuples!(impl_system_param_builder_tuple, 0, 16, P, B);

// SAFETY: implementors of each `SystemParamBuilder` in the vec have validated their impls
unsafe impl<P: SystemParam, B: SystemParamBuilder<P>> SystemParamBuilder<Vec<P>> for Vec<B> {
fn build(self, world: &mut World, meta: &mut SystemMeta) -> <Vec<P> as SystemParam>::State {
self.into_iter()
.map(|builder| builder.build(world, meta))
.collect()
}
}

/// A [`SystemParamBuilder`] for a [`ParamSet`].
/// To build a [`ParamSet`] with a tuple of system parameters, pass a tuple of matching [`SystemParamBuilder`]s.
/// To build a [`ParamSet`] with a `Vec` of system parameters, pass a `Vec` of matching [`SystemParamBuilder`]s.
Expand Down Expand Up @@ -251,6 +260,38 @@ macro_rules! impl_param_set_builder_tuple {

all_tuples!(impl_param_set_builder_tuple, 1, 8, P, B, meta);

// SAFETY: Relevant parameter ComponentId and ArchetypeComponentId access is applied to SystemMeta. If any ParamState conflicts
// with any prior access, a panic will occur.
unsafe impl<'w, 's, P: SystemParam, B: SystemParamBuilder<P>>
SystemParamBuilder<ParamSet<'w, 's, Vec<P>>> for ParamSetBuilder<Vec<B>>
{
fn build(
self,
world: &mut World,
system_meta: &mut SystemMeta,
) -> <Vec<P> as SystemParam>::State {
let mut states = Vec::with_capacity(self.0.len());
let mut metas = Vec::with_capacity(self.0.len());
for builder in self.0 {
let mut meta = system_meta.clone();
states.push(builder.build(world, &mut meta));
metas.push(meta);
}
if metas.iter().any(|m| !m.is_send()) {
system_meta.set_non_send();
}
for meta in metas {
system_meta
.component_access_set
.extend(meta.component_access_set);
system_meta
.archetype_component_access
.extend(&meta.archetype_component_access);
}
states
}
}

/// A [`SystemParamBuilder`] for a [`DynSystemParam`].
pub struct DynParamBuilder<'a>(
Box<dyn FnOnce(&mut World, &mut SystemMeta) -> DynSystemParamState + 'a>,
Expand Down Expand Up @@ -385,6 +426,37 @@ mod tests {
assert_eq!(result, 1);
}

#[test]
fn vec_builder() {
let mut world = World::new();

world.spawn((A, B, C));
world.spawn((A, B));
world.spawn((A, C));
world.spawn((A, C));
world.spawn_empty();

let system = (vec![
QueryParamBuilder::new_box(|builder| {
builder.with::<B>().without::<C>();
}),
QueryParamBuilder::new_box(|builder| {
builder.with::<C>().without::<B>();
}),
],)
.build_state(&mut world)
.build_system(|params: Vec<Query<&mut A>>| {
let mut count: usize = 0;
params
.into_iter()
.for_each(|mut query| count += query.iter_mut().count());
count
});

let result = world.run_system_once(system);
assert_eq!(result, 3);
}

#[test]
fn param_set_builder() {
let mut world = World::new();
Expand Down Expand Up @@ -412,6 +484,35 @@ mod tests {
assert_eq!(result, 5);
}

#[test]
fn param_set_vec_builder() {
let mut world = World::new();

world.spawn((A, B, C));
world.spawn((A, B));
world.spawn((A, C));
world.spawn((A, C));
world.spawn_empty();

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
});

let result = world.run_system_once(system);
assert_eq!(result, 5);
}

#[test]
fn dyn_builder() {
let mut world = World::new();
Expand Down
133 changes: 133 additions & 0 deletions crates/bevy_ecs/src/system/system_param.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1450,6 +1450,139 @@ unsafe impl SystemParam for SystemChangeTick {
}
}

// SAFETY: When initialized with `init_state`, `get_param` returns an empty `Vec` and does no access.
// Therefore, `init_state` trivially registers all access, and no accesses can conflict.
// Note that the safety requirements for non-empty `Vec`s are handled by the `SystemParamBuilder` impl that builds them.
unsafe impl<T: SystemParam> SystemParam for Vec<T> {
type State = Vec<T::State>;

type Item<'world, 'state> = Vec<T::Item<'world, 'state>>;

fn init_state(_world: &mut World, _system_meta: &mut SystemMeta) -> Self::State {
Vec::new()
}

unsafe fn get_param<'world, 'state>(
state: &'state mut Self::State,
system_meta: &SystemMeta,
world: UnsafeWorldCell<'world>,
change_tick: Tick,
) -> Self::Item<'world, 'state> {
state
.iter_mut()
// SAFETY:
// - We initialized the state for each parameter in the builder, so the caller ensures we have access to any world data needed by each param.
// - The caller ensures this was the world used to initialize our state, and we used that world to initialize parameter states
.map(|state| unsafe { T::get_param(state, system_meta, world, change_tick) })
.collect()
}

unsafe fn new_archetype(
state: &mut Self::State,
archetype: &Archetype,
system_meta: &mut SystemMeta,
) {
for state in state {
// SAFETY: The caller ensures that `archetype` is from the World the state was initialized from in `init_state`.
unsafe { T::new_archetype(state, archetype, system_meta) };
}
}

fn apply(state: &mut Self::State, system_meta: &SystemMeta, world: &mut World) {
for state in state {
T::apply(state, system_meta, world);
}
}

fn queue(state: &mut Self::State, system_meta: &SystemMeta, mut world: DeferredWorld) {
for state in state {
T::queue(state, system_meta, world.reborrow());
}
}
}

// SAFETY: When initialized with `init_state`, `get_param` returns an empty `Vec` and does no access.
// Therefore, `init_state` trivially registers all access, and no accesses can conflict.
// Note that the safety requirements for non-empty `Vec`s are handled by the `SystemParamBuilder` impl that builds them.
unsafe impl<T: SystemParam> SystemParam for ParamSet<'_, '_, Vec<T>> {
type State = Vec<T::State>;

type Item<'world, 'state> = ParamSet<'world, 'state, Vec<T>>;

fn init_state(_world: &mut World, _system_meta: &mut SystemMeta) -> Self::State {
Vec::new()
}

unsafe fn get_param<'world, 'state>(
state: &'state mut Self::State,
system_meta: &SystemMeta,
world: UnsafeWorldCell<'world>,
change_tick: Tick,
) -> Self::Item<'world, 'state> {
ParamSet {
param_states: state,
system_meta: system_meta.clone(),
world,
change_tick,
}
}

unsafe fn new_archetype(
state: &mut Self::State,
archetype: &Archetype,
system_meta: &mut SystemMeta,
) {
for state in state {
// SAFETY: The caller ensures that `archetype` is from the World the state was initialized from in `init_state`.
unsafe { T::new_archetype(state, archetype, system_meta) }
}
}

fn apply(state: &mut Self::State, system_meta: &SystemMeta, world: &mut World) {
for state in state {
T::apply(state, system_meta, world);
}
}

fn queue(state: &mut Self::State, system_meta: &SystemMeta, mut world: DeferredWorld) {
for state in state {
T::queue(state, system_meta, world.reborrow());
}
}
}

impl<T: SystemParam> ParamSet<'_, '_, Vec<T>> {
/// Accesses the parameter at the given index.
/// No other parameters may be accessed while this one is active.
pub fn get_mut(&mut self, index: usize) -> T::Item<'_, '_> {
// SAFETY:
// - We initialized the state for each parameter in the builder, so the caller ensures we have access to any world data needed by any param.
// We have mutable access to the ParamSet, so no other params in the set are active.
// - The caller of `get_param` ensured that this was the world used to initialize our state, and we used that world to initialize parameter states
unsafe {
T::get_param(
&mut self.param_states[index],
&self.system_meta,
self.world,
self.change_tick,
)
}
}

/// Calls a closure for each parameter in the set.
pub fn for_each(&mut self, mut f: impl FnMut(T::Item<'_, '_>)) {
self.param_states.iter_mut().for_each(|state| {
f(
// SAFETY:
// - We initialized the state for each parameter in the builder, so the caller ensures we have access to any world data needed by any param.
// We have mutable access to the ParamSet, so no other params in the set are active.
// - The caller of `get_param` ensured that this was the world used to initialize our state, and we used that world to initialize parameter states
unsafe { T::get_param(state, &self.system_meta, self.world, self.change_tick) },
);
});
}
}

macro_rules! impl_system_param_tuple {
($(#[$meta:meta])* $($param: ident),*) => {
$(#[$meta])*
Expand Down