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

[Merged by Bors] - Add new SystemState and rename old SystemState to SystemMeta #2283

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
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
37 changes: 21 additions & 16 deletions crates/bevy_ecs/macros/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -221,40 +221,45 @@ pub fn impl_query_set(_input: TokenStream) -> TokenStream {
type Fetch = QuerySetState<(#(QueryState<#query, #filter>,)*)>;
}

// SAFE: Relevant query ComponentId and ArchetypeComponentId access is applied to SystemState. If any QueryState conflicts
// SAFE: All Queries are constrained to ReadOnlyFetch, so World is only read
unsafe impl<#(#query: WorldQuery + 'static,)* #(#filter: WorldQuery + 'static,)*> ReadOnlySystemParamFetch for QuerySetState<(#(QueryState<#query, #filter>,)*)>
where #(#query::Fetch: ReadOnlyFetch,)* #(#filter::Fetch: FilterFetch,)*
{ }

// SAFE: Relevant query ComponentId and ArchetypeComponentId access is applied to SystemMeta. If any QueryState conflicts
// with any prior access, a panic will occur.
unsafe impl<#(#query: WorldQuery + 'static,)* #(#filter: WorldQuery + 'static,)*> SystemParamState for QuerySetState<(#(QueryState<#query, #filter>,)*)>
where #(#filter::Fetch: FilterFetch,)*
{
type Config = ();
fn init(world: &mut World, system_state: &mut SystemState, config: Self::Config) -> Self {
fn init(world: &mut World, system_meta: &mut SystemMeta, config: Self::Config) -> Self {
#(
let mut #query = QueryState::<#query, #filter>::new(world);
assert_component_access_compatibility(
&system_state.name,
&system_meta.name,
std::any::type_name::<#query>(),
std::any::type_name::<#filter>(),
&system_state.component_access_set,
&system_meta.component_access_set,
&#query.component_access,
world,
);
)*
#(
system_state
system_meta
.component_access_set
.add(#query.component_access.clone());
system_state
system_meta
.archetype_component_access
.extend(&#query.archetype_component_access);
)*
QuerySetState((#(#query,)*))
}

fn new_archetype(&mut self, archetype: &Archetype, system_state: &mut SystemState) {
fn new_archetype(&mut self, archetype: &Archetype, system_meta: &mut SystemMeta) {
let (#(#query,)*) = &mut self.0;
#(
#query.new_archetype(archetype);
system_state
system_meta
.archetype_component_access
.extend(&#query.archetype_component_access);
)*
Expand All @@ -271,12 +276,12 @@ pub fn impl_query_set(_input: TokenStream) -> TokenStream {
#[inline]
unsafe fn get_param(
state: &'a mut Self,
system_state: &'a SystemState,
system_meta: &'a SystemMeta,
world: &'a World,
change_tick: u32,
) -> Self::Item {
let (#(#query,)*) = &state.0;
QuerySet((#(Query::new(world, #query, system_state.last_change_tick, change_tick),)*))
QuerySet((#(Query::new(world, #query, system_meta.last_change_tick, change_tick),)*))
}
}

Expand Down Expand Up @@ -388,15 +393,15 @@ pub fn derive_system_param(input: TokenStream) -> TokenStream {

unsafe impl<TSystemParamState: #path::system::SystemParamState, #punctuated_generics> #path::system::SystemParamState for #fetch_struct_name<TSystemParamState, #punctuated_generic_idents> {
type Config = TSystemParamState::Config;
fn init(world: &mut #path::world::World, system_state: &mut #path::system::SystemState, config: Self::Config) -> Self {
fn init(world: &mut #path::world::World, system_meta: &mut #path::system::SystemMeta, config: Self::Config) -> Self {
Self {
state: TSystemParamState::init(world, system_state, config),
state: TSystemParamState::init(world, system_meta, config),
marker: std::marker::PhantomData,
}
}

fn new_archetype(&mut self, archetype: &#path::archetype::Archetype, system_state: &mut #path::system::SystemState) {
self.state.new_archetype(archetype, system_state)
fn new_archetype(&mut self, archetype: &#path::archetype::Archetype, system_meta: &mut #path::system::SystemMeta) {
self.state.new_archetype(archetype, system_meta)
}

fn default_config() -> TSystemParamState::Config {
Expand All @@ -412,12 +417,12 @@ pub fn derive_system_param(input: TokenStream) -> TokenStream {
type Item = #struct_name#ty_generics;
unsafe fn get_param(
state: &'a mut Self,
system_state: &'a #path::system::SystemState,
system_meta: &'a #path::system::SystemMeta,
world: &'a #path::world::World,
change_tick: u32,
) -> Self::Item {
#struct_name {
#(#fields: <<#field_types as SystemParam>::Fetch as #path::system::SystemParamFetch>::get_param(&mut state.state.#field_indices, system_state, world, change_tick),)*
#(#fields: <<#field_types as SystemParam>::Fetch as #path::system::SystemParamFetch>::get_param(&mut state.state.#field_indices, system_meta, world, change_tick),)*
#(#ignored_fields: <#ignored_field_types>::default(),)*
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,26 +3,27 @@ use crate::{
component::ComponentId,
query::{Access, FilteredAccessSet},
system::{
check_system_change_tick, System, SystemId, SystemParam, SystemParamFetch, SystemParamState,
check_system_change_tick, ReadOnlySystemParamFetch, System, SystemId, SystemParam,
SystemParamFetch, SystemParamState,
},
world::World,
};
use bevy_ecs_macros::all_tuples;
use std::{borrow::Cow, marker::PhantomData};

/// The state of a [`System`].
pub struct SystemState {
/// The metadata of a [`System`].
pub struct SystemMeta {
pub(crate) id: SystemId,
pub(crate) name: Cow<'static, str>,
pub(crate) component_access_set: FilteredAccessSet<ComponentId>,
pub(crate) archetype_component_access: Access<ArchetypeComponentId>,
// NOTE: this must be kept private. making a SystemState non-send is irreversible to prevent
// NOTE: this must be kept private. making a SystemMeta non-send is irreversible to prevent
// SystemParams from overriding each other
is_send: bool,
pub(crate) last_change_tick: u32,
}

impl SystemState {
impl SystemMeta {
fn new<T>() -> Self {
Self {
name: std::any::type_name::<T>().into(),
Expand All @@ -49,6 +50,86 @@ impl SystemState {
}
}

// TODO: Actually use this in FunctionSystem. We should probably only do this once Systems are constructed using a World reference
// (to avoid the need for unwrapping to retrieve SystemMeta)
/// Holds on to persistent state required to drive [`SystemParam`] for a [`System`].
pub struct SystemState<Param: SystemParam> {
meta: SystemMeta,
param_state: <Param as SystemParam>::Fetch,
change_tick: u32,
cart marked this conversation as resolved.
Show resolved Hide resolved
}

impl<Param: SystemParam> SystemState<Param> {
pub fn new(world: &mut World) -> Self {
let config = <Param::Fetch as SystemParamState>::default_config();
Self::with_config(world, config)
}

pub fn with_config(
world: &mut World,
config: <Param::Fetch as SystemParamState>::Config,
) -> Self {
let mut meta = SystemMeta::new::<Param>();
let param_state = <Param::Fetch as SystemParamState>::init(world, &mut meta, config);
Self {
meta,
param_state,
change_tick: 0,
}
}

#[inline]
pub fn meta(&self) -> &SystemMeta {
&self.meta
}

/// Retrieve the [`SystemParam`] values. This can only be called when all parameters are read-only.
pub fn get<'a>(&'a mut self, world: &'a World) -> <Param::Fetch as SystemParamFetch<'a>>::Item
cart marked this conversation as resolved.
Show resolved Hide resolved
where
Param::Fetch: ReadOnlySystemParamFetch,
{
// SAFE: Param is read-only and doesn't allow mutable access to World
unsafe { self.get_unchecked(world) }
}

/// Retrieve the mutable [`SystemParam`] values.
pub fn get_mut<'a>(
&'a mut self,
world: &'a mut World,
) -> <Param::Fetch as SystemParamFetch<'a>>::Item {
// SAFE: World is uniquely borrowed
unsafe { self.get_unchecked(world) }
}

/// Applies all state queued up for [`SystemParam`] values. For example, this will apply commands queued up
/// by a [`Commands`](`super::Commands`) parameter to the given [`World`].
/// This function should be called manually after the values returned by [`SystemState::get`] and [`SystemState::get_mut`]
/// are finished being used.
pub fn apply(&mut self, world: &mut World) {
self.param_state.apply(world);
}

/// Retrieve the [`SystemParam`] values.
///
/// # Safety
/// This call might access any of the input parameters in a way that violates Rust's mutability rules. Make sure the data
/// access is safe in the context of global [`World`] access.
#[inline]
pub unsafe fn get_unchecked<'a>(
&'a mut self,
world: &'a World,
) -> <Param::Fetch as SystemParamFetch<'a>>::Item {
let change_tick = world.increment_change_tick();
self.change_tick = change_tick;
<Param::Fetch as SystemParamFetch>::get_param(
&mut self.param_state,
&self.meta,
world,
change_tick,
)
}
}

/// Conversion trait to turn something into a [`System`].
///
/// Use this to get a system from a function. Also note that every system implements this trait as
Expand Down Expand Up @@ -116,7 +197,7 @@ where
{
func: F,
param_state: Option<Param::Fetch>,
system_state: SystemState,
system_meta: SystemMeta,
config: Option<<Param::Fetch as SystemParamState>::Config>,
// NOTE: PhantomData<fn()-> T> gives this safe Send/Sync impls
#[allow(clippy::type_complexity)]
Expand Down Expand Up @@ -161,7 +242,7 @@ where
func: self,
param_state: None,
config: Some(<Param::Fetch as SystemParamState>::default_config()),
system_state: SystemState::new::<F>(),
system_meta: SystemMeta::new::<F>(),
marker: PhantomData,
}
}
Expand All @@ -180,33 +261,33 @@ where

#[inline]
fn name(&self) -> Cow<'static, str> {
self.system_state.name.clone()
self.system_meta.name.clone()
}

#[inline]
fn id(&self) -> SystemId {
self.system_state.id
self.system_meta.id
}

#[inline]
fn new_archetype(&mut self, archetype: &Archetype) {
let param_state = self.param_state.as_mut().unwrap();
param_state.new_archetype(archetype, &mut self.system_state);
param_state.new_archetype(archetype, &mut self.system_meta);
}

#[inline]
fn component_access(&self) -> &Access<ComponentId> {
&self.system_state.component_access_set.combined_access()
&self.system_meta.component_access_set.combined_access()
}

#[inline]
fn archetype_component_access(&self) -> &Access<ArchetypeComponentId> {
&self.system_state.archetype_component_access
&self.system_meta.archetype_component_access
}

#[inline]
fn is_send(&self) -> bool {
self.system_state.is_send
self.system_meta.is_send
}

#[inline]
Expand All @@ -215,11 +296,11 @@ where
let out = self.func.run(
input,
self.param_state.as_mut().unwrap(),
&self.system_state,
&self.system_meta,
world,
change_tick,
);
self.system_state.last_change_tick = change_tick;
self.system_meta.last_change_tick = change_tick;
out
}

Expand All @@ -233,17 +314,17 @@ where
fn initialize(&mut self, world: &mut World) {
self.param_state = Some(<Param::Fetch as SystemParamState>::init(
world,
&mut self.system_state,
&mut self.system_meta,
self.config.take().unwrap(),
));
}

#[inline]
fn check_change_tick(&mut self, change_tick: u32) {
check_system_change_tick(
&mut self.system_state.last_change_tick,
&mut self.system_meta.last_change_tick,
change_tick,
self.system_state.name.as_ref(),
self.system_meta.name.as_ref(),
);
}
}
Expand All @@ -254,7 +335,7 @@ pub trait SystemParamFunction<In, Out, Param: SystemParam, Marker>: Send + Sync
&mut self,
input: In,
state: &mut Param::Fetch,
system_state: &SystemState,
system_meta: &SystemMeta,
world: &World,
change_tick: u32,
) -> Out;
Expand All @@ -270,9 +351,9 @@ macro_rules! impl_system_function {
FnMut($(<<$param as SystemParam>::Fetch as SystemParamFetch>::Item),*) -> Out + Send + Sync + 'static, Out: 'static
{
#[inline]
fn run(&mut self, _input: (), state: &mut <($($param,)*) as SystemParam>::Fetch, system_state: &SystemState, world: &World, change_tick: u32) -> Out {
fn run(&mut self, _input: (), state: &mut <($($param,)*) as SystemParam>::Fetch, system_meta: &SystemMeta, world: &World, change_tick: u32) -> Out {
unsafe {
let ($($param,)*) = <<($($param,)*) as SystemParam>::Fetch as SystemParamFetch>::get_param(state, system_state, world, change_tick);
let ($($param,)*) = <<($($param,)*) as SystemParam>::Fetch as SystemParamFetch>::get_param(state, system_meta, world, change_tick);
self($($param),*)
}
}
Expand All @@ -286,9 +367,9 @@ macro_rules! impl_system_function {
FnMut(In<Input>, $(<<$param as SystemParam>::Fetch as SystemParamFetch>::Item),*) -> Out + Send + Sync + 'static, Out: 'static
{
#[inline]
fn run(&mut self, input: Input, state: &mut <($($param,)*) as SystemParam>::Fetch, system_state: &SystemState, world: &World, change_tick: u32) -> Out {
fn run(&mut self, input: Input, state: &mut <($($param,)*) as SystemParam>::Fetch, system_meta: &SystemMeta, world: &World, change_tick: u32) -> Out {
unsafe {
cart marked this conversation as resolved.
Show resolved Hide resolved
let ($($param,)*) = <<($($param,)*) as SystemParam>::Fetch as SystemParamFetch>::get_param(state, system_state, world, change_tick);
let ($($param,)*) = <<($($param,)*) as SystemParam>::Fetch as SystemParamFetch>::get_param(state, system_meta, world, change_tick);
self(In(input), $($param),*)
}
}
Expand Down
Loading