bevy/crates/bevy_ecs/src/system/mod.rs
Carter Anderson 9d453530fa System Param Lifetime Split (#2605)
# Objective

Enable using exact World lifetimes during read-only access . This is motivated by the new renderer's need to allow read-only world-only queries to outlive the query itself (but still be constrained by the world lifetime).

For example:
115b170d1f/pipelined/bevy_pbr2/src/render/mod.rs (L774)

## Solution

Split out SystemParam state and world lifetimes and pipe those lifetimes up to read-only Query ops (and add into_inner for Res). According to every safety test I've run so far (except one), this is safe (see the temporary safety test commit). Note that changing the mutable variants to the new lifetimes would allow aliased mutable pointers (try doing that to see how it affects the temporary safety tests).

The new state lifetime on SystemParam does make `#[derive(SystemParam)]` more cumbersome (the current impl requires PhantomData if you don't use both lifetimes). We can make this better by detecting whether or not a lifetime is used in the derive and adjusting accordingly, but that should probably be done in its own pr.  

## Why is this a draft?

The new lifetimes break QuerySet safety in one very specific case (see the query_set system in system_safety_test). We need to solve this before we can use the lifetimes given.

This is due to the fact that QuerySet is just a wrapper over Query, which now relies on world lifetimes instead of `&self` lifetimes to prevent aliasing (but in systems, each Query has its own implied lifetime, not a centralized world lifetime).  I believe the fix is to rewrite QuerySet to have its own World lifetime (and own the internal reference). This will complicate the impl a bit, but I think it is doable. I'm curious if anyone else has better ideas.

Personally, I think these new lifetimes need to happen. We've gotta have a way to directly tie read-only World queries to the World lifetime. The new renderer is the first place this has come up, but I doubt it will be the last. Worst case scenario we can come up with a second `WorldLifetimeQuery<Q, F = ()>` parameter to enable these read-only scenarios, but I'd rather not add another type to the type zoo.
2021-08-15 20:51:53 +00:00

830 lines
24 KiB
Rust

mod commands;
mod exclusive_system;
mod function_system;
mod query;
#[allow(clippy::module_inception)]
mod system;
mod system_chaining;
mod system_param;
pub use commands::*;
pub use exclusive_system::*;
pub use function_system::*;
pub use query::*;
pub use system::*;
pub use system_chaining::*;
pub use system_param::*;
#[cfg(test)]
mod tests {
use std::any::TypeId;
use crate::{
archetype::Archetypes,
bundle::Bundles,
component::Components,
entity::{Entities, Entity},
query::{Added, Changed, Or, QueryState, With, Without},
schedule::{Schedule, Stage, SystemStage},
system::{
ConfigurableSystem, IntoExclusiveSystem, IntoSystem, Local, Query, QuerySet,
RemovedComponents, Res, ResMut, System, SystemState,
},
world::{FromWorld, World},
};
#[derive(Debug, Eq, PartialEq, Default)]
struct A;
struct B;
struct C;
struct D;
struct E;
struct F;
#[test]
fn simple_system() {
fn sys(query: Query<&A>) {
for a in query.iter() {
println!("{:?}", a);
}
}
let mut system = sys.system();
let mut world = World::new();
world.spawn().insert(A);
system.initialize(&mut world);
for archetype in world.archetypes.iter() {
system.new_archetype(archetype);
}
system.run((), &mut world);
}
fn run_system<Param, S: IntoSystem<(), (), Param>>(world: &mut World, system: S) {
let mut schedule = Schedule::default();
let mut update = SystemStage::parallel();
update.add_system(system);
schedule.add_stage("update", update);
schedule.run(world);
}
#[test]
fn query_system_gets() {
fn query_system(
mut ran: ResMut<bool>,
entity_query: Query<Entity, With<A>>,
b_query: Query<&B>,
a_c_query: Query<(&A, &C)>,
d_query: Query<&D>,
) {
let entities = entity_query.iter().collect::<Vec<Entity>>();
assert!(
b_query.get_component::<B>(entities[0]).is_err(),
"entity 0 should not have B"
);
assert!(
b_query.get_component::<B>(entities[1]).is_ok(),
"entity 1 should have B"
);
assert!(
b_query.get_component::<A>(entities[1]).is_err(),
"entity 1 should have A, but b_query shouldn't have access to it"
);
assert!(
b_query.get_component::<D>(entities[3]).is_err(),
"entity 3 should have D, but it shouldn't be accessible from b_query"
);
assert!(
b_query.get_component::<C>(entities[2]).is_err(),
"entity 2 has C, but it shouldn't be accessible from b_query"
);
assert!(
a_c_query.get_component::<C>(entities[2]).is_ok(),
"entity 2 has C, and it should be accessible from a_c_query"
);
assert!(
a_c_query.get_component::<D>(entities[3]).is_err(),
"entity 3 should have D, but it shouldn't be accessible from b_query"
);
assert!(
d_query.get_component::<D>(entities[3]).is_ok(),
"entity 3 should have D"
);
*ran = true;
}
let mut world = World::default();
world.insert_resource(false);
world.spawn().insert_bundle((A,));
world.spawn().insert_bundle((A, B));
world.spawn().insert_bundle((A, C));
world.spawn().insert_bundle((A, D));
run_system(&mut world, query_system);
assert!(*world.get_resource::<bool>().unwrap(), "system ran");
}
#[test]
fn or_query_set_system() {
// Regression test for issue #762
fn query_system(
mut ran: ResMut<bool>,
mut set: QuerySet<(
QueryState<(), Or<(Changed<A>, Changed<B>)>>,
QueryState<(), Or<(Added<A>, Added<B>)>>,
)>,
) {
let changed = set.q0().iter().count();
let added = set.q1().iter().count();
assert_eq!(changed, 1);
assert_eq!(added, 1);
*ran = true;
}
let mut world = World::default();
world.insert_resource(false);
world.spawn().insert_bundle((A, B));
run_system(&mut world, query_system);
assert!(*world.get_resource::<bool>().unwrap(), "system ran");
}
#[test]
fn changed_resource_system() {
struct Added(usize);
struct Changed(usize);
fn incr_e_on_flip(
value: Res<bool>,
mut changed: ResMut<Changed>,
mut added: ResMut<Added>,
) {
if value.is_added() {
added.0 += 1;
}
if value.is_changed() {
changed.0 += 1;
}
}
let mut world = World::default();
world.insert_resource(false);
world.insert_resource(Added(0));
world.insert_resource(Changed(0));
let mut schedule = Schedule::default();
let mut update = SystemStage::parallel();
update.add_system(incr_e_on_flip);
schedule.add_stage("update", update);
schedule.add_stage(
"clear_trackers",
SystemStage::single(World::clear_trackers.exclusive_system()),
);
schedule.run(&mut world);
assert_eq!(world.get_resource::<Added>().unwrap().0, 1);
assert_eq!(world.get_resource::<Changed>().unwrap().0, 1);
schedule.run(&mut world);
assert_eq!(world.get_resource::<Added>().unwrap().0, 1);
assert_eq!(world.get_resource::<Changed>().unwrap().0, 1);
*world.get_resource_mut::<bool>().unwrap() = true;
schedule.run(&mut world);
assert_eq!(world.get_resource::<Added>().unwrap().0, 1);
assert_eq!(world.get_resource::<Changed>().unwrap().0, 2);
}
#[test]
#[should_panic]
fn conflicting_query_mut_system() {
fn sys(_q1: Query<&mut A>, _q2: Query<&mut A>) {}
let mut world = World::default();
run_system(&mut world, sys);
}
#[test]
fn disjoint_query_mut_system() {
fn sys(_q1: Query<&mut A, With<B>>, _q2: Query<&mut A, Without<B>>) {}
let mut world = World::default();
run_system(&mut world, sys);
}
#[test]
fn disjoint_query_mut_read_component_system() {
fn sys(_q1: Query<(&mut A, &B)>, _q2: Query<&mut A, Without<B>>) {}
let mut world = World::default();
run_system(&mut world, sys);
}
#[test]
#[should_panic]
fn conflicting_query_immut_system() {
fn sys(_q1: Query<&A>, _q2: Query<&mut A>) {}
let mut world = World::default();
run_system(&mut world, sys);
}
#[test]
fn query_set_system() {
fn sys(mut _set: QuerySet<(QueryState<&mut A>, QueryState<&A>)>) {}
let mut world = World::default();
run_system(&mut world, sys);
}
#[test]
#[should_panic]
fn conflicting_query_with_query_set_system() {
fn sys(_query: Query<&mut A>, _set: QuerySet<(QueryState<&mut A>, QueryState<&B>)>) {}
let mut world = World::default();
run_system(&mut world, sys);
}
#[test]
#[should_panic]
fn conflicting_query_sets_system() {
fn sys(
_set_1: QuerySet<(QueryState<&mut A>,)>,
_set_2: QuerySet<(QueryState<&mut A>, QueryState<&B>)>,
) {
}
let mut world = World::default();
run_system(&mut world, sys);
}
#[derive(Default)]
struct BufferRes {
_buffer: Vec<u8>,
}
fn test_for_conflicting_resources<Param, S: IntoSystem<(), (), Param>>(sys: S) {
let mut world = World::default();
world.insert_resource(BufferRes::default());
world.insert_resource(A);
world.insert_resource(B);
run_system(&mut world, sys);
}
#[test]
#[should_panic]
fn conflicting_system_resources() {
fn sys(_: ResMut<BufferRes>, _: Res<BufferRes>) {}
test_for_conflicting_resources(sys)
}
#[test]
#[should_panic]
fn conflicting_system_resources_reverse_order() {
fn sys(_: Res<BufferRes>, _: ResMut<BufferRes>) {}
test_for_conflicting_resources(sys)
}
#[test]
#[should_panic]
fn conflicting_system_resources_multiple_mutable() {
fn sys(_: ResMut<BufferRes>, _: ResMut<BufferRes>) {}
test_for_conflicting_resources(sys)
}
#[test]
fn nonconflicting_system_resources() {
fn sys(_: Local<BufferRes>, _: ResMut<BufferRes>, _: Local<A>, _: ResMut<A>) {}
test_for_conflicting_resources(sys)
}
#[test]
fn local_system() {
let mut world = World::default();
world.insert_resource(1u32);
world.insert_resource(false);
struct Foo {
value: u32,
}
impl FromWorld for Foo {
fn from_world(world: &mut World) -> Self {
Foo {
value: *world.get_resource::<u32>().unwrap() + 1,
}
}
}
fn sys(local: Local<Foo>, mut modified: ResMut<bool>) {
assert_eq!(local.value, 2);
*modified = true;
}
run_system(&mut world, sys);
// ensure the system actually ran
assert!(*world.get_resource::<bool>().unwrap());
}
#[test]
fn remove_tracking() {
let mut world = World::new();
struct Despawned(Entity);
let a = world.spawn().insert_bundle(("abc", 123)).id();
world.spawn().insert_bundle(("abc", 123));
world.insert_resource(false);
world.insert_resource(Despawned(a));
world.entity_mut(a).despawn();
fn validate_removed(
removed_i32: RemovedComponents<i32>,
despawned: Res<Despawned>,
mut ran: ResMut<bool>,
) {
assert_eq!(
removed_i32.iter().collect::<Vec<_>>(),
&[despawned.0],
"despawning results in 'removed component' state"
);
*ran = true;
}
run_system(&mut world, validate_removed);
assert!(*world.get_resource::<bool>().unwrap(), "system ran");
}
#[test]
fn configure_system_local() {
let mut world = World::default();
world.insert_resource(false);
fn sys(local: Local<usize>, mut modified: ResMut<bool>) {
assert_eq!(*local, 42);
*modified = true;
}
run_system(&mut world, sys.config(|config| config.0 = Some(42)));
// ensure the system actually ran
assert!(*world.get_resource::<bool>().unwrap());
}
#[test]
fn world_collections_system() {
let mut world = World::default();
world.insert_resource(false);
world.spawn().insert_bundle((42, true));
fn sys(
archetypes: &Archetypes,
components: &Components,
entities: &Entities,
bundles: &Bundles,
query: Query<Entity, With<i32>>,
mut modified: ResMut<bool>,
) {
assert_eq!(query.iter().count(), 1, "entity exists");
for entity in query.iter() {
let location = entities.get(entity).unwrap();
let archetype = archetypes.get(location.archetype_id).unwrap();
let archetype_components = archetype.components().collect::<Vec<_>>();
let bundle_id = bundles
.get_id(std::any::TypeId::of::<(i32, bool)>())
.expect("Bundle used to spawn entity should exist");
let bundle_info = bundles.get(bundle_id).unwrap();
let mut bundle_components = bundle_info.components().to_vec();
bundle_components.sort();
for component_id in bundle_components.iter() {
assert!(
components.get_info(*component_id).is_some(),
"every bundle component exists in Components"
);
}
assert_eq!(
bundle_components, archetype_components,
"entity's bundle components exactly match entity's archetype components"
);
}
*modified = true;
}
run_system(&mut world, sys);
// ensure the system actually ran
assert!(*world.get_resource::<bool>().unwrap());
}
#[test]
fn get_system_conflicts() {
fn sys_x(_: Res<A>, _: Res<B>, _: Query<(&C, &D)>) {}
fn sys_y(_: Res<A>, _: ResMut<B>, _: Query<(&C, &mut D)>) {}
let mut world = World::default();
let mut x = sys_x.system();
let mut y = sys_y.system();
x.initialize(&mut world);
y.initialize(&mut world);
let conflicts = x.component_access().get_conflicts(y.component_access());
let b_id = world
.components()
.get_resource_id(TypeId::of::<B>())
.unwrap();
let d_id = world.components().get_id(TypeId::of::<D>()).unwrap();
assert_eq!(conflicts, vec![b_id, d_id]);
}
#[test]
fn query_is_empty() {
fn without_filter(not_empty: Query<&A>, empty: Query<&B>) {
assert!(!not_empty.is_empty());
assert!(empty.is_empty());
}
fn with_filter(not_empty: Query<&A, With<C>>, empty: Query<&A, With<D>>) {
assert!(!not_empty.is_empty());
assert!(empty.is_empty());
}
let mut world = World::default();
world.spawn().insert(A).insert(C);
let mut without_filter = without_filter.system();
without_filter.initialize(&mut world);
without_filter.run((), &mut world);
let mut with_filter = with_filter.system();
with_filter.initialize(&mut world);
with_filter.run((), &mut world);
}
#[test]
#[allow(clippy::too_many_arguments)]
fn can_have_16_parameters() {
fn sys_x(
_: Res<A>,
_: Res<B>,
_: Res<C>,
_: Res<D>,
_: Res<E>,
_: Res<F>,
_: Query<&A>,
_: Query<&B>,
_: Query<&C>,
_: Query<&D>,
_: Query<&E>,
_: Query<&F>,
_: Query<(&A, &B)>,
_: Query<(&C, &D)>,
_: Query<(&E, &F)>,
) {
}
fn sys_y(
_: (
Res<A>,
Res<B>,
Res<C>,
Res<D>,
Res<E>,
Res<F>,
Query<&A>,
Query<&B>,
Query<&C>,
Query<&D>,
Query<&E>,
Query<&F>,
Query<(&A, &B)>,
Query<(&C, &D)>,
Query<(&E, &F)>,
),
) {
}
let mut world = World::default();
let mut x = sys_x.system();
let mut y = sys_y.system();
x.initialize(&mut world);
y.initialize(&mut world);
}
#[test]
fn read_system_state() {
#[derive(Eq, PartialEq, Debug)]
struct A(usize);
#[derive(Eq, PartialEq, Debug)]
struct B(usize);
let mut world = World::default();
world.insert_resource(A(42));
world.spawn().insert(B(7));
let mut system_state: SystemState<(
Res<A>,
Query<&B>,
QuerySet<(QueryState<&C>, QueryState<&D>)>,
)> = SystemState::new(&mut world);
let (a, query, _) = system_state.get(&world);
assert_eq!(*a, A(42), "returned resource matches initial value");
assert_eq!(
*query.single().unwrap(),
B(7),
"returned component matches initial value"
);
}
#[test]
fn write_system_state() {
#[derive(Eq, PartialEq, Debug)]
struct A(usize);
#[derive(Eq, PartialEq, Debug)]
struct B(usize);
let mut world = World::default();
world.insert_resource(A(42));
world.spawn().insert(B(7));
let mut system_state: SystemState<(ResMut<A>, Query<&mut B>)> =
SystemState::new(&mut world);
// The following line shouldn't compile because the parameters used are not ReadOnlySystemParam
// let (a, query) = system_state.get(&world);
let (a, mut query) = system_state.get_mut(&mut world);
assert_eq!(*a, A(42), "returned resource matches initial value");
assert_eq!(
*query.single_mut().unwrap(),
B(7),
"returned component matches initial value"
);
}
#[test]
fn system_state_change_detection() {
#[derive(Eq, PartialEq, Debug)]
struct A(usize);
let mut world = World::default();
let entity = world.spawn().insert(A(1)).id();
let mut system_state: SystemState<Query<&A, Changed<A>>> = SystemState::new(&mut world);
{
let query = system_state.get(&world);
assert_eq!(*query.single().unwrap(), A(1));
}
{
let query = system_state.get(&world);
assert!(query.single().is_err());
}
world.entity_mut(entity).get_mut::<A>().unwrap().0 = 2;
{
let query = system_state.get(&world);
assert_eq!(*query.single().unwrap(), A(2));
}
}
#[test]
#[should_panic]
fn system_state_invalid_world() {
let mut world = World::default();
let mut system_state = SystemState::<Query<&A>>::new(&mut world);
let mismatched_world = World::default();
system_state.get(&mismatched_world);
}
#[test]
fn system_state_archetype_update() {
#[derive(Eq, PartialEq, Debug)]
struct A(usize);
#[derive(Eq, PartialEq, Debug)]
struct B(usize);
let mut world = World::default();
world.spawn().insert(A(1));
let mut system_state = SystemState::<Query<&A>>::new(&mut world);
{
let query = system_state.get(&world);
assert_eq!(
query.iter().collect::<Vec<_>>(),
vec![&A(1)],
"exactly one component returned"
);
}
world.spawn().insert_bundle((A(2), B(2)));
{
let query = system_state.get(&world);
assert_eq!(
query.iter().collect::<Vec<_>>(),
vec![&A(1), &A(2)],
"components from both archetypes returned"
);
}
}
/// this test exists to show that read-only world-only queries can return data that lives as long as 'world
#[test]
#[allow(unused)]
fn long_life_test() {
struct Holder<'w> {
value: &'w A,
}
struct State {
state: SystemState<Res<'static, A>>,
state_q: SystemState<Query<'static, 'static, &'static A>>,
}
impl State {
fn hold_res<'w>(&mut self, world: &'w World) -> Holder<'w> {
let a = self.state.get(world);
Holder {
value: a.into_inner(),
}
}
fn hold_component<'w>(&mut self, world: &'w World, entity: Entity) -> Holder<'w> {
let q = self.state_q.get(world);
let a = q.get(entity).unwrap();
Holder { value: a }
}
fn hold_components<'w>(&mut self, world: &'w World) -> Vec<Holder<'w>> {
let mut components = Vec::new();
let q = self.state_q.get(world);
for a in q.iter() {
components.push(Holder { value: a });
}
components
}
}
}
}
/// ```compile_fail
/// use bevy_ecs::prelude::*;
/// struct A(usize);
/// fn system(mut query: Query<&mut A>, e: Res<Entity>) {
/// let mut iter = query.iter_mut();
/// let a = &mut *iter.next().unwrap();
///
/// let mut iter2 = query.iter_mut();
/// let b = &mut *iter2.next().unwrap();
///
/// // this should fail to compile
/// println!("{}", a.0);
/// }
/// ```
#[allow(unused)]
#[cfg(doc)]
fn system_query_iter_lifetime_safety_test() {}
/// ```compile_fail
/// use bevy_ecs::prelude::*;
/// struct A(usize);
/// fn system(mut query: Query<&mut A>, e: Res<Entity>) {
/// let mut a1 = query.get_mut(*e).unwrap();
/// let mut a2 = query.get_mut(*e).unwrap();
/// // this should fail to compile
/// println!("{} {}", a1.0, a2.0);
/// }
/// ```
#[allow(unused)]
#[cfg(doc)]
fn system_query_get_lifetime_safety_test() {}
/// ```compile_fail
/// use bevy_ecs::prelude::*;
/// struct A(usize);
/// fn query_set(mut queries: QuerySet<(QueryState<&mut A>, QueryState<&A>)>, e: Res<Entity>) {
/// let mut q2 = queries.q0();
/// let mut iter2 = q2.iter_mut();
/// let mut b = iter2.next().unwrap();
///
/// let q1 = queries.q1();
/// let mut iter = q1.iter();
/// let a = &*iter.next().unwrap();
///
/// // this should fail to compile
/// b.0 = a.0
/// }
/// ```
#[allow(unused)]
#[cfg(doc)]
fn system_query_set_iter_lifetime_safety_test() {}
/// ```compile_fail
/// use bevy_ecs::prelude::*;
/// struct A(usize);
/// fn query_set(mut queries: QuerySet<(QueryState<&mut A>, QueryState<&A>)>, e: Res<Entity>) {
/// let q1 = queries.q1();
/// let mut iter = q1.iter();
/// let a = &*iter.next().unwrap();
///
/// let mut q2 = queries.q0();
/// let mut iter2 = q2.iter_mut();
/// let mut b = iter2.next().unwrap();
///
/// // this should fail to compile
/// b.0 = a.0;
/// }
/// ```
#[allow(unused)]
#[cfg(doc)]
fn system_query_set_iter_flip_lifetime_safety_test() {}
/// ```compile_fail
/// use bevy_ecs::prelude::*;
/// struct A(usize);
/// fn query_set(mut queries: QuerySet<(QueryState<&mut A>, QueryState<&A>)>, e: Res<Entity>) {
/// let mut q2 = queries.q0();
/// let mut b = q2.get_mut(*e).unwrap();
///
/// let q1 = queries.q1();
/// let a = q1.get(*e).unwrap();
///
/// // this should fail to compile
/// b.0 = a.0
/// }
/// ```
#[allow(unused)]
#[cfg(doc)]
fn system_query_set_get_lifetime_safety_test() {}
/// ```compile_fail
/// use bevy_ecs::prelude::*;
/// struct A(usize);
/// fn query_set(mut queries: QuerySet<(QueryState<&mut A>, QueryState<&A>)>, e: Res<Entity>) {
/// let q1 = queries.q1();
/// let a = q1.get(*e).unwrap();
///
/// let mut q2 = queries.q0();
/// let mut b = q2.get_mut(*e).unwrap();
/// // this should fail to compile
/// b.0 = a.0
/// }
/// ```
#[allow(unused)]
#[cfg(doc)]
fn system_query_set_get_flip_lifetime_safety_test() {}
/// ```compile_fail
/// use bevy_ecs::prelude::*;
/// use bevy_ecs::system::SystemState;
/// struct A(usize);
/// struct B(usize);
/// struct State {
/// state_r: SystemState<Query<'static, 'static, &'static A>>,
/// state_w: SystemState<Query<'static, 'static, &'static mut A>>,
/// }
///
/// impl State {
/// fn get_component<'w>(&mut self, world: &'w mut World, entity: Entity) {
/// let q1 = self.state_r.get(&world);
/// let a1 = q1.get(entity).unwrap();
///
/// let mut q2 = self.state_w.get_mut(world);
/// let a2 = q2.get_mut(entity).unwrap();
///
/// // this should fail to compile
/// println!("{}", a1.0);
/// }
/// }
/// ```
#[allow(unused)]
#[cfg(doc)]
fn system_state_get_lifetime_safety_test() {}
/// ```compile_fail
/// use bevy_ecs::prelude::*;
/// use bevy_ecs::system::SystemState;
/// struct A(usize);
/// struct B(usize);
/// struct State {
/// state_r: SystemState<Query<'static, 'static, &'static A>>,
/// state_w: SystemState<Query<'static, 'static, &'static mut A>>,
/// }
///
/// impl State {
/// fn get_components<'w>(&mut self, world: &'w mut World) {
/// let q1 = self.state_r.get(&world);
/// let a1 = q1.iter().next().unwrap();
/// let mut q2 = self.state_w.get_mut(world);
/// let a2 = q2.iter_mut().next().unwrap();
/// // this should fail to compile
/// println!("{}", a1.0);
/// }
/// }
/// ```
#[allow(unused)]
#[cfg(doc)]
fn system_state_iter_lifetime_safety_test() {}