Skip to content

Commit

Permalink
Add 'World::run_system_with_input' function
Browse files Browse the repository at this point in the history
  • Loading branch information
Nathan-Fenner committed Nov 12, 2023
1 parent bf4f4e4 commit f8df323
Showing 1 changed file with 147 additions and 27 deletions.
174 changes: 147 additions & 27 deletions crates/bevy_ecs/src/system/system_registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,18 @@ use thiserror::Error;

/// A small wrapper for [`BoxedSystem`] that also keeps track whether or not the system has been initialized.
#[derive(Component)]
struct RegisteredSystem {
struct RegisteredSystem<I> {
initialized: bool,
system: BoxedSystem,
system: BoxedSystem<I>,
}

/// A system that has been removed from the registry.
/// It contains the system and whether or not it has been initialized.
///
/// This struct is returned by [`World::remove_system`].
pub struct RemovedSystem {
pub struct RemovedSystem<I = ()> {
initialized: bool,
system: BoxedSystem,
system: BoxedSystem<I>,
}

impl RemovedSystem {
Expand All @@ -38,8 +38,35 @@ impl RemovedSystem {
///
/// These are opaque identifiers, keyed to a specific [`World`],
/// and are created via [`World::register_system`].
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
pub struct SystemId(Entity);
#[derive(Eq)]
pub struct SystemId<I = ()>(Entity, std::marker::PhantomData<I>);

// A manual impl is used because the trait bounds should ignore the `I` phantom parameter.
impl<I> Copy for SystemId<I> {}
// A manual impl is used because the trait bounds should ignore the `I` phantom parameter.
impl<I> Clone for SystemId<I> {
fn clone(&self) -> Self {
*self
}
}
// A manual impl is used because the trait bounds should ignore the `I` phantom parameter.
impl<I> PartialEq for SystemId<I> {
fn eq(&self, other: &Self) -> bool {
self.0 == other.0 && self.1 == other.1
}
}
// A manual impl is used because the trait bounds should ignore the `I` phantom parameter.
impl<I> std::hash::Hash for SystemId<I> {
fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
self.0.hash(state);
}
}
impl<I> std::fmt::Debug for SystemId<I> {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
// The PhantomData field is omitted for simplicity.
f.debug_tuple("SystemId").field(&self.0).finish()
}
}

impl World {
/// Registers a system and returns a [`SystemId`] so it can later be called by [`World::run_system`].
Expand All @@ -51,24 +78,25 @@ impl World {
/// This allows for running systems in a pushed-based fashion.
/// Using a [`Schedule`](crate::schedule::Schedule) is still preferred for most cases
/// due to its better performance and abillity to run non-conflicting systems simultaneously.
pub fn register_system<M, S: IntoSystem<(), (), M> + 'static>(
pub fn register_system<I: 'static, M, S: IntoSystem<I, (), M> + 'static>(
&mut self,
system: S,
) -> SystemId {
) -> SystemId<I> {
self.register_boxed_system(Box::new(IntoSystem::into_system(system)))
}

/// Similar to [`Self::register_system`], but allows passing in a [`BoxedSystem`].
///
/// This is useful if the [`IntoSystem`] implementor has already been turned into a
/// [`System`](crate::system::System) trait object and put in a [`Box`].
pub fn register_boxed_system(&mut self, system: BoxedSystem) -> SystemId {
pub fn register_boxed_system<I: 'static>(&mut self, system: BoxedSystem<I>) -> SystemId<I> {
SystemId(
self.spawn(RegisteredSystem {
initialized: false,
system,
})
.id(),
std::marker::PhantomData,
)
}

Expand All @@ -78,11 +106,14 @@ impl World {
///
/// If no system corresponds to the given [`SystemId`], this method returns an error.
/// Systems are also not allowed to remove themselves, this returns an error too.
pub fn remove_system(&mut self, id: SystemId) -> Result<RemovedSystem, RegisteredSystemError> {
pub fn remove_system<I: 'static>(
&mut self,
id: SystemId<I>,
) -> Result<RemovedSystem<I>, RegisteredSystemError<I>> {
match self.get_entity_mut(id.0) {
Some(mut entity) => {
let registered_system = entity
.take::<RegisteredSystem>()
.take::<RegisteredSystem<I>>()
.ok_or(RegisteredSystemError::SelfRemove(id))?;
entity.despawn();
Ok(RemovedSystem {
Expand All @@ -100,9 +131,10 @@ impl World {
/// This is different from [`RunSystemOnce::run_system_once`](crate::system::RunSystemOnce::run_system_once),
/// because it keeps local state between calls and change detection works correctly.
///
/// In order to run a chained system with an input, use [`World::run_system_with_input`] instead.
///
/// # Limitations
///
/// - Stored systems cannot be chained: they can neither have an [`In`](crate::system::In) nor return any values.
/// - Stored systems cannot be recursive, they cannot call themselves through [`Commands::run_system`](crate::system::Commands).
/// - Exclusive systems cannot be used.
///
Expand Down Expand Up @@ -150,6 +182,44 @@ impl World {
/// let _ = world.run_system(detector); // -> Something happened!
/// ```
pub fn run_system(&mut self, id: SystemId) -> Result<(), RegisteredSystemError> {
self.run_system_with_input(id, ())
}

/// Run a stored chained system by its [`SystemId`], providing an input value.
/// Before running a system, it must first be registered.
/// The method [`World::register_system`] stores a given system and returns a [`SystemId`].
///
/// # Limitations
///
/// - Stored systems cannot be recursive, they cannot call themselves through [`Commands::run_system`](crate::system::Commands).
/// - Exclusive systems cannot be used.
///
/// # Examples
///
/// ```rust
/// # use bevy_ecs::prelude::*;
/// #[derive(Resource, Default)]
/// struct Counter(u8);
///
/// fn increment(In(increment_by): In<u8>, mut counter: Local<Counter>) {
/// counter.0 += increment_by;
/// println!("{}", counter.0);
/// }
///
/// let mut world = World::default();
/// let counter_one = world.register_system(increment);
/// let counter_two = world.register_system(increment);
/// world.run_system_with_input(counter_one, 1); // -> 1
/// world.run_system_with_input(counter_one, 20); // -> 21
/// world.run_system_with_input(counter_two, 30); // -> 51
/// ```
///
/// See [`World::run_system`] for more examples.
pub fn run_system_with_input<I: 'static>(
&mut self,
id: SystemId<I>,
input: I,
) -> Result<(), RegisteredSystemError<I>> {
// lookup
let mut entity = self
.get_entity_mut(id.0)
Expand All @@ -160,20 +230,20 @@ impl World {
mut initialized,
mut system,
} = entity
.take::<RegisteredSystem>()
.take::<RegisteredSystem<I>>()
.ok_or(RegisteredSystemError::Recursive(id))?;

// run the system
if !initialized {
system.initialize(self);
initialized = true;
}
system.run((), self);
system.run(input, self);
system.apply_deferred(self);

// return ownership of system trait object (if entity still exists)
if let Some(mut entity) = self.get_entity_mut(id.0) {
entity.insert::<RegisteredSystem>(RegisteredSystem {
entity.insert::<RegisteredSystem<I>>(RegisteredSystem {
initialized,
system,
});
Expand Down Expand Up @@ -206,19 +276,31 @@ impl Command for RunSystem {
}

/// An operation with stored systems failed.
#[derive(Debug, Error)]
pub enum RegisteredSystemError {
#[derive(Error)]
pub enum RegisteredSystemError<I = ()> {
/// A system was run by id, but no system with that id was found.
///
/// Did you forget to register it?
#[error("System {0:?} was not registered")]
SystemIdNotRegistered(SystemId),
SystemIdNotRegistered(SystemId<I>),
/// A system tried to run itself recursively.
#[error("System {0:?} tried to run itself recursively")]
Recursive(SystemId),
Recursive(SystemId<I>),
/// A system tried to remove itself.
#[error("System {0:?} tried to remove itself")]
SelfRemove(SystemId),
SelfRemove(SystemId<I>),
}

impl<I> std::fmt::Debug for RegisteredSystemError<I> {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
Self::SystemIdNotRegistered(arg0) => {
f.debug_tuple("SystemIdNotRegistered").field(arg0).finish()
}
Self::Recursive(arg0) => f.debug_tuple("Recursive").field(arg0).finish(),
Self::SelfRemove(arg0) => f.debug_tuple("SelfRemove").field(arg0).finish(),
}
}
}

mod tests {
Expand Down Expand Up @@ -248,14 +330,14 @@ mod tests {
assert_eq!(*world.resource::<Counter>(), Counter(0));
// Resources are changed when they are first added.
let id = world.register_system(count_up_iff_changed);
let _ = world.run_system(id);
world.run_system(id).expect("system runs successfully");
assert_eq!(*world.resource::<Counter>(), Counter(1));
// Nothing changed
let _ = world.run_system(id);
world.run_system(id).expect("system runs successfully");
assert_eq!(*world.resource::<Counter>(), Counter(1));
// Making a change
world.resource_mut::<ChangeDetector>().set_changed();
let _ = world.run_system(id);
world.run_system(id).expect("system runs successfully");
assert_eq!(*world.resource::<Counter>(), Counter(2));
}

Expand All @@ -271,16 +353,54 @@ mod tests {
world.insert_resource(Counter(1));
assert_eq!(*world.resource::<Counter>(), Counter(1));
let id = world.register_system(doubling);
let _ = world.run_system(id);
world.run_system(id).expect("system runs successfully");
assert_eq!(*world.resource::<Counter>(), Counter(1));
let _ = world.run_system(id);
world.run_system(id).expect("system runs successfully");
assert_eq!(*world.resource::<Counter>(), Counter(2));
let _ = world.run_system(id);
world.run_system(id).expect("system runs successfully");
assert_eq!(*world.resource::<Counter>(), Counter(4));
let _ = world.run_system(id);
world.run_system(id).expect("system runs successfully");
assert_eq!(*world.resource::<Counter>(), Counter(8));
}

#[test]
fn input_values() {
// Verify that a non-Copy, non-Clone type can be passed in.
struct NonCopy(u8);

fn increment_sys(In(NonCopy(increment_by)): In<NonCopy>, mut counter: ResMut<Counter>) {
counter.0 += increment_by;
}

let mut world = World::new();

let id = world.register_system(increment_sys);

// Insert the resource after registering the system.
world.insert_resource(Counter(1));
assert_eq!(*world.resource::<Counter>(), Counter(1));

world
.run_system_with_input(id, NonCopy(1))
.expect("system runs successfully");
assert_eq!(*world.resource::<Counter>(), Counter(2));

world
.run_system_with_input(id, NonCopy(1))
.expect("system runs successfully");
assert_eq!(*world.resource::<Counter>(), Counter(3));

world
.run_system_with_input(id, NonCopy(20))
.expect("system runs successfully");
assert_eq!(*world.resource::<Counter>(), Counter(23));

world
.run_system_with_input(id, NonCopy(1))
.expect("system runs successfully");
assert_eq!(*world.resource::<Counter>(), Counter(24));
}

#[test]
fn nested_systems() {
use crate::system::SystemId;
Expand Down

0 comments on commit f8df323

Please sign in to comment.