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

Locals in exclusive systems #3946

Closed
Show file tree
Hide file tree
Changes from 3 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
4 changes: 2 additions & 2 deletions crates/bevy_ecs/src/schedule/system_descriptor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,9 +78,9 @@ impl IntoSystemDescriptor<()> for ExclusiveSystemDescriptor {
}
}

impl<F> IntoSystemDescriptor<()> for ExclusiveSystemFn<F>
impl<F, P> IntoSystemDescriptor<()> for ExclusiveSystemFn<F, P>
where
F: FnMut(&mut crate::prelude::World) + Send + Sync + 'static,
ExclusiveSystemFn<F, P>: ExclusiveSystem,
{
fn into_descriptor(self) -> SystemDescriptor {
new_exclusive_descriptor(Box::new(self)).into_descriptor()
Expand Down
95 changes: 58 additions & 37 deletions crates/bevy_ecs/src/system/exclusive_system.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
use bevy_ecs_macros::all_tuples;

use crate::{
archetype::ArchetypeGeneration,
system::{check_system_change_tick, BoxedSystem, IntoSystem},
prelude::{FromWorld, Local},
system::{check_system_change_tick, BoxedSystem, IntoSystem, Resource},
world::World,
};
use std::borrow::Cow;
Expand All @@ -15,59 +18,77 @@ pub trait ExclusiveSystem: Send + Sync + 'static {
fn check_change_tick(&mut self, change_tick: u32);
}

pub struct ExclusiveSystemFn<F> {
pub struct ExclusiveSystemFn<F, L> {
func: F,
name: Cow<'static, str>,
last_change_tick: u32,
locals: Option<L>,
}

impl<F> ExclusiveSystem for ExclusiveSystemFn<F>
where
F: FnMut(&mut World) + Send + Sync + 'static,
{
fn name(&self) -> Cow<'static, str> {
self.name.clone()
}
macro_rules! impl_exclusive_system {
($($t:ident),*) => {
#[allow(non_snake_case)]
impl<F, $($t: FromWorld + Resource,)*> ExclusiveSystem for ExclusiveSystemFn<F, ($($t,)*)>
where
F: FnMut(&mut World, $(Local<$t>,)*) + Send + Sync + 'static,
{
fn name(&self) -> Cow<'static, str> {
self.name.clone()
}

fn run(&mut self, world: &mut World) {
// The previous value is saved in case this exclusive system is run by another exclusive
// system
let saved_last_tick = world.last_change_tick;
world.last_change_tick = self.last_change_tick;
fn run(&mut self, world: &mut World) {
// The previous value is saved in case this exclusive system is run by another exclusive
// system
let saved_last_tick = world.last_change_tick;
world.last_change_tick = self.last_change_tick;

(self.func)(world);
let ($($t,)*) = self.locals.as_mut().unwrap();
(self.func)(world, $(Local::wrap($t),)*);

let change_tick = world.change_tick.get_mut();
self.last_change_tick = *change_tick;
*change_tick += 1;
let change_tick = world.change_tick.get_mut();
self.last_change_tick = *change_tick;
*change_tick += 1;

world.last_change_tick = saved_last_tick;
}
world.last_change_tick = saved_last_tick;
}

fn initialize(&mut self, _: &mut World) {}
fn initialize(&mut self, _world: &mut World) {
self.locals = Some(($($t::from_world(_world),)*));
}

fn check_change_tick(&mut self, change_tick: u32) {
check_system_change_tick(&mut self.last_change_tick, change_tick, self.name.as_ref());
}
fn check_change_tick(&mut self, change_tick: u32) {
check_system_change_tick(
&mut self.last_change_tick,
change_tick,
self.name.as_ref(),
);
}
}


impl<F, $($t: FromWorld + Resource,)*> IntoExclusiveSystem<&mut World, ExclusiveSystemFn<F, ($($t,)*)>> for F
where
F: FnMut(&mut World, $(Local<$t>,)*) + Send + Sync + 'static,
{
fn exclusive_system(self) -> ExclusiveSystemFn<F, ($($t,)*)> {
ExclusiveSystemFn {
func: self,
name: core::any::type_name::<F>().into(),
last_change_tick: 0,
locals: None,
}
}
}

};
}

all_tuples!(impl_exclusive_system, 0, 12, L);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
all_tuples!(impl_exclusive_system, 0, 12, L);
all_tuples!(impl_exclusive_system, 0, 16, L);

I would rather have all all_tuples work on the same maximum number of items in a tuple, so 16. It would be less surprising for users


pub trait IntoExclusiveSystem<Params, SystemType> {
fn exclusive_system(self) -> SystemType;
}

impl<F> IntoExclusiveSystem<&mut World, ExclusiveSystemFn<F>> for F
where
F: FnMut(&mut World) + Send + Sync + 'static,
{
fn exclusive_system(self) -> ExclusiveSystemFn<F> {
ExclusiveSystemFn {
func: self,
name: core::any::type_name::<F>().into(),
last_change_tick: 0,
}
}
}

pub struct ExclusiveSystemCoerced {
system: BoxedSystem<(), ()>,
archetype_generation: ArchetypeGeneration,
Expand Down
7 changes: 7 additions & 0 deletions crates/bevy_ecs/src/system/function_system.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use crate::{
archetype::{Archetype, ArchetypeComponentId, ArchetypeGeneration, ArchetypeId},
component::ComponentId,
prelude::FromWorld,
query::{Access, FilteredAccessSet},
system::{
check_system_change_tick, ReadOnlySystemParamFetch, System, SystemParam, SystemParamFetch,
Expand Down Expand Up @@ -168,6 +169,12 @@ impl<Param: SystemParam> SystemState<Param> {
}
}

impl<Param: SystemParam> FromWorld for SystemState<Param> {
fn from_world(world: &mut World) -> Self {
Self::new(world)
}
}

/// A trait for defining systems with a [`SystemParam`] associated type.
///
/// This facilitates the creation of systems that are generic over some trait
Expand Down
6 changes: 6 additions & 0 deletions crates/bevy_ecs/src/system/system_param.rs
Original file line number Diff line number Diff line change
Expand Up @@ -627,6 +627,12 @@ impl<'w, 's> SystemParamFetch<'w, 's> for WorldState {
/// ```
pub struct Local<'a, T: Resource>(&'a mut T);

impl<'a, T: Resource> Local<'a, T> {
pub(crate) fn wrap(x: &'a mut T) -> Self {
Self(x)
}
}

// SAFE: Local only accesses internal state
unsafe impl<T: Resource> ReadOnlySystemParamFetch for LocalState<T> {}

Expand Down
20 changes: 20 additions & 0 deletions crates/bevy_ecs/src/world/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ mod spawn_batch;
mod world_cell;

pub use crate::change_detection::Mut;
use bevy_ecs_macros::all_tuples;
pub use entity_ref::*;
pub use spawn_batch::*;
pub use world_cell::*;
Expand Down Expand Up @@ -1224,6 +1225,25 @@ impl<T: Default> FromWorld for T {
}
}

/// Wrapper type to enable getting multiple [`FromWorld`] types in one type.
///
/// This wrapper is necessary because a [`FromWorld`] implementation for a tuple of
/// [`FromWorld`] implementing types would conflict with the [`FromWorld`] impl over all
/// [`Default`] type.
pub struct FromWorldWrap<T>(pub T);

macro_rules! impl_from_world {
($($t:ident),*) => {
impl<$($t: FromWorld,)*> FromWorld for FromWorldWrap<($($t,)*)> {
fn from_world(_world: &mut World) -> Self {
FromWorldWrap(($($t::from_world(_world),)*))
}
}
};
}

all_tuples!(impl_from_world, 0, 12, T);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same


struct MainThreadValidator {
main_thread: std::thread::ThreadId,
}
Expand Down