Skip to content

Commit

Permalink
Apply suggestion, and more detailed documentation.
Browse files Browse the repository at this point in the history
  • Loading branch information
hxYuki committed Nov 10, 2023
1 parent af94d07 commit 63f3dec
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 37 deletions.
9 changes: 7 additions & 2 deletions crates/bevy_ecs/src/system/commands/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -527,11 +527,16 @@ impl<'w, 's> Commands<'w, 's> {
self.queue.push(RunSystem::new(id));
}

/// Runs the system by itself.
/// Runs the system by referring it. This method will register the system newly called with,
/// whether you have registered or not. Different calls with same system will share same state.
///
/// Specially note when running closures with capture, the captured value won't update after first call.
/// Examples see [`World::run_system_singleton`]. Register them each, use [`World::run_system`] instead.
///
/// Systems are ran in an exclusive and single threaded way.
/// Running slow systems can become a bottleneck.
///
/// Calls [`World::run_system_singleton`](crate::system::World::run_system_singleton).
/// Calls [`World::run_system_singleton`].
pub fn run_system_singleton<M>(&mut self, system: impl IntoSystem<(), (), M>) {
self.queue.push(RunSystemSingleton::new(
IntoSystem::<(), (), M>::into_system(system),
Expand Down
85 changes: 50 additions & 35 deletions crates/bevy_ecs/src/system/system_registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use crate::system::{BoxedSystem, Command, IntoSystem};
use crate::world::World;
use crate::{self as bevy_ecs};
use bevy_ecs_macros::{Component, Resource};
use bevy_utils::tracing::info;
use thiserror::Error;

use super::System;
Expand Down Expand Up @@ -189,16 +190,35 @@ impl World {
/// # Limitations
///
/// See [`World::run_system`].
///
/// # Attention
///
/// Please note that closure is identified by its **occurrence** no matter what it captures,
/// thus this code will call the first cached one rather than actual passed one.
///
/// That means, codes like below will not work as expected.
///
/// ```rust
/// use bevy_ecs::prelude::World;
/// let mut world = World::new();
/// // This will creates closures regarded as one same closure
/// let make_system = |n| move || println!("{n}");
/// let _ = world.run_system_singleton(make_system(0)); // prints 0
/// let _ = world.run_system_singleton(make_system(1)); // prints 0 same
/// ```
pub fn run_system_singleton<M, T: IntoSystem<(), (), M>>(
&mut self,
system: T,
) -> Result<(), UnregisteredSystemError> {
// create the hashmap if it doesn't exist
let mut system_map = self.get_resource_or_insert_with(|| SystemMap {
map: bevy_utils::HashMap::default(),
});
let mut system_map = self.get_resource_or_insert_with(|| SystemMap::default());

let system = IntoSystem::<(), (), M>::into_system(system);

// check captures of closure
if std::mem::size_of::<T>() != 0 {
info!(target: "run_system_singleton", "Closure with capture(s) runs, be sure that you're not relying on differently captured versions of one closure.")
}
// use the system type id as the key
let system_type_id = system.type_id();
// take system out
Expand Down Expand Up @@ -234,8 +254,8 @@ impl World {
system.apply_deferred(self);

// put system back if resource still exists
if let Some(mut sys_map) = self.get_resource_mut::<SystemMap>() {
sys_map.map.insert(
if let Some(mut system_map) = self.get_resource_mut::<SystemMap>() {
system_map.map.insert(
system_type_id,
UnregisteredSystem {
initialized,
Expand Down Expand Up @@ -313,12 +333,19 @@ impl<T: System<In = (), Out = ()>> Command for RunSystemSingleton<T> {
struct SystemMap {
map: bevy_utils::HashMap<std::any::TypeId, UnregisteredSystem>,
}
impl Default for SystemMap {
fn default() -> Self {
Self {
map: bevy_utils::HashMap::default(),
}
}
}

/// An operation with [`World::run_system_singleton`] systems failed.
#[derive(Debug, Error)]
pub enum UnregisteredSystemError {
/// A system tried to run itself recursively.
#[error("RunSystemAlong: System tried to run itself recursively")]
#[error("RunSystemSingleton: System tried to run itself recursively")]
Recursive,
}

Expand Down Expand Up @@ -371,27 +398,21 @@ mod tests {
assert_eq!(*world.resource::<Counter>(), Counter(4));

// Test for `run_system_singleton` with closure
// This is needed for closure's `TypeId` is related to its occurrence
fn run_count_up_changed_indirect(
world: &mut World,
) -> Result<(), crate::system::UnregisteredSystemError> {
world.run_system_singleton(
|mut counter: ResMut<Counter>, change_detector: ResMut<ChangeDetector>| {
if change_detector.is_changed() {
counter.0 += 1;
}
},
)
}

let _ = run_count_up_changed_indirect(&mut world);
let run_count_up_changed_closure =
|mut counter: ResMut<Counter>, change_detector: ResMut<ChangeDetector>| {
if change_detector.is_changed() {
counter.0 += 1;
}
};

let _ = world.run_system_singleton(run_count_up_changed_closure);
assert_eq!(*world.resource::<Counter>(), Counter(5));
// Nothing changed
let _ = run_count_up_changed_indirect(&mut world);
let _ = world.run_system_singleton(run_count_up_changed_closure);
assert_eq!(*world.resource::<Counter>(), Counter(5));
// Making a change
world.resource_mut::<ChangeDetector>().set_changed();
let _ = run_count_up_changed_indirect(&mut world);
let _ = world.run_system_singleton(run_count_up_changed_closure);
assert_eq!(*world.resource::<Counter>(), Counter(6));
}

Expand Down Expand Up @@ -426,22 +447,16 @@ mod tests {

// Test for `run_system_singleton` with closure
// This is needed for closure's `TypeId` is related to its occurrence
fn run_doubling_indirect(
world: &mut World,
) -> Result<(), crate::system::UnregisteredSystemError> {
world.run_system_singleton(
|last_counter: Local<Counter>, mut counter: ResMut<Counter>| {
counter.0 += last_counter.0 .0;
last_counter.0 .0 = counter.0;
},
)
}
let doubling_closure = |last_counter: Local<Counter>, mut counter: ResMut<Counter>| {
counter.0 += last_counter.0 .0;
last_counter.0 .0 = counter.0;
};

let _ = run_doubling_indirect(&mut world);
let _ = world.run_system_singleton(doubling_closure);
assert_eq!(*world.resource::<Counter>(), Counter(32));
let _ = run_doubling_indirect(&mut world);
let _ = world.run_system_singleton(doubling_closure);
assert_eq!(*world.resource::<Counter>(), Counter(64));
let _ = run_doubling_indirect(&mut world);
let _ = world.run_system_singleton(doubling_closure);
assert_eq!(*world.resource::<Counter>(), Counter(128));
}

Expand Down

0 comments on commit 63f3dec

Please sign in to comment.