Skip to content

run_if for SystemConfigs via anonymous system sets #7676

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
merged 5 commits into from
Mar 30, 2023
Merged
Show file tree
Hide file tree
Changes from all 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
2 changes: 2 additions & 0 deletions crates/bevy_app/src/app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -380,8 +380,10 @@ impl App {
/// # fn system_a() {}
/// # fn system_b() {}
/// # fn system_c() {}
/// # fn should_run() -> bool { true }
/// #
/// app.add_systems(Update, (system_a, system_b, system_c));
/// app.add_systems(Update, (system_a, system_b).run_if(should_run));
/// ```
pub fn add_systems<M>(
&mut self,
Expand Down
45 changes: 38 additions & 7 deletions crates/bevy_ecs/src/schedule/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ pub enum SystemConfigs {
SystemConfig(SystemConfig),
Configs {
configs: Vec<SystemConfigs>,
collective_conditions: Vec<BoxedCondition>,
/// If `true`, adds `before -> after` ordering constraints between the successive elements.
chained: bool,
},
Expand All @@ -78,7 +79,7 @@ impl SystemConfigs {
})
}

fn in_set_inner(&mut self, set: BoxedSystemSet) {
pub(crate) fn in_set_inner(&mut self, set: BoxedSystemSet) {
match self {
SystemConfigs::SystemConfig(config) => {
config.graph_info.sets.push(set);
Expand Down Expand Up @@ -162,13 +163,16 @@ impl SystemConfigs {
}
}

fn run_if_inner(&mut self, condition: BoxedCondition) {
pub(crate) fn run_if_inner(&mut self, condition: BoxedCondition) {
match self {
SystemConfigs::SystemConfig(config) => {
config.conditions.push(condition);
}
SystemConfigs::Configs { .. } => {
todo!("run_if is not implemented for groups of systems yet")
SystemConfigs::Configs {
collective_conditions,
..
} => {
collective_conditions.push(condition);
}
}
}
Expand Down Expand Up @@ -212,12 +216,12 @@ where
///
/// ```
/// # use bevy_ecs::prelude::*;
/// # let mut app = Schedule::new();
/// # let mut schedule = Schedule::new();
/// # fn a() {}
/// # fn b() {}
/// # fn condition() -> bool { true }
/// app.add_systems((a, b).distributive_run_if(condition));
/// app.add_systems((a.run_if(condition), b.run_if(condition)));
/// schedule.add_systems((a, b).distributive_run_if(condition));
/// schedule.add_systems((a.run_if(condition), b.run_if(condition)));
/// ```
///
/// # Note
Expand All @@ -237,6 +241,32 @@ where
///
/// The `Condition` will be evaluated at most once (per schedule run),
/// the first time a system in this set prepares to run.
///
/// If this set contains more than one system, calling `run_if` is equivalent to adding each
/// system to a common set and configuring the run condition on that set, as shown below:
///
/// # Examples
///
/// ```
/// # use bevy_ecs::prelude::*;
/// # let mut schedule = Schedule::new();
/// # fn a() {}
/// # fn b() {}
/// # fn condition() -> bool { true }
/// # #[derive(SystemSet, Debug, Eq, PartialEq, Hash, Clone, Copy)]
/// # struct C;
/// schedule.add_systems((a, b).run_if(condition));
/// schedule.add_systems((a, b).in_set(C)).configure_set(C.run_if(condition));
/// ```
///
/// # Note
///
/// Because the condition will only be evaluated once, there is no guarantee that the condition
/// is upheld after the first system has run. You need to make sure that no other systems that
/// could invalidate the condition are scheduled inbetween the first and last run system.
///
/// Use [`distributive_run_if`](IntoSystemConfigs::distributive_run_if) if you want the
/// condition to be evaluated for each individual system, right before one is run.
fn run_if<M>(self, condition: impl Condition<M>) -> SystemConfigs {
self.into_configs().run_if(condition)
}
Expand Down Expand Up @@ -364,6 +394,7 @@ macro_rules! impl_system_collection {
let ($($sys,)*) = self;
SystemConfigs::Configs {
configs: vec![$($sys.into_configs(),)*],
collective_conditions: Vec::new(),
chained: false,
}
}
Expand Down
54 changes: 47 additions & 7 deletions crates/bevy_ecs/src/schedule/schedule.rs
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,10 @@ impl SystemSetNode {
pub fn is_system_type(&self) -> bool {
self.inner.system_type().is_some()
}

pub fn is_anonymous(&self) -> bool {
self.inner.is_anonymous()
}
}

/// A [`BoxedSystem`] with metadata, stored in a [`ScheduleGraph`].
Expand Down Expand Up @@ -525,7 +529,27 @@ impl ScheduleGraph {
}
}
}
SystemConfigs::Configs { configs, chained } => {
SystemConfigs::Configs {
mut configs,
collective_conditions,
chained,
} => {
let more_than_one_entry = configs.len() > 1;
if !collective_conditions.is_empty() {
if more_than_one_entry {
let set = AnonymousSet::new();
for config in &mut configs {
config.in_set_inner(set.dyn_clone());
}
let mut set_config = set.into_config();
set_config.conditions.extend(collective_conditions);
self.configure_set(set_config);
} else {
for condition in collective_conditions {
configs[0].run_if_inner(condition);
}
}
}
let mut config_iter = configs.into_iter();
let mut nodes_in_scope = Vec::new();
let mut densely_chained = true;
Expand Down Expand Up @@ -607,7 +631,6 @@ impl ScheduleGraph {
nodes_in_scope.append(&mut previous_result.nodes);
}
} else {
let more_than_one_entry = config_iter.len() > 1;
for config in config_iter {
let result = self.add_systems_inner(config, ancestor_chained);
densely_chained = densely_chained && result.densely_chained;
Expand Down Expand Up @@ -705,8 +728,7 @@ impl ScheduleGraph {
match self.system_set_ids.get(set) {
Some(set_id) => {
if id == set_id {
let string = format!("{:?}", &set);
return Err(ScheduleBuildError::HierarchyLoop(string));
return Err(ScheduleBuildError::HierarchyLoop(self.get_node_name(id)));
}
}
None => {
Expand Down Expand Up @@ -742,8 +764,7 @@ impl ScheduleGraph {
match self.system_set_ids.get(set) {
Some(set_id) => {
if id == set_id {
let string = format!("{:?}", &set);
return Err(ScheduleBuildError::DependencyLoop(string));
return Err(ScheduleBuildError::DependencyLoop(self.get_node_name(id)));
}
}
None => {
Expand Down Expand Up @@ -1257,14 +1278,33 @@ impl ScheduleGraph {
name
}
}
NodeId::Set(_) => self.system_sets[id.index()].name(),
NodeId::Set(_) => {
let set = &self.system_sets[id.index()];
if set.is_anonymous() {
self.anonymous_set_name(id)
} else {
set.name()
}
}
};
if self.settings.use_shortnames {
name = bevy_utils::get_short_name(&name);
}
name
}

fn anonymous_set_name(&self, id: &NodeId) -> String {
format!(
"({})",
self.hierarchy
.graph
.edges_directed(*id, Direction::Outgoing)
.map(|(_, member_id, _)| self.get_node_name(&member_id))
.reduce(|a, b| format!("{a}, {b}"))
.unwrap_or_default()
)
}

fn get_node_kind(&self, id: &NodeId) -> &'static str {
match id {
NodeId::System(_) => "system",
Expand Down
28 changes: 28 additions & 0 deletions crates/bevy_ecs/src/schedule/set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use std::any::TypeId;
use std::fmt::Debug;
use std::hash::{Hash, Hasher};
use std::marker::PhantomData;
use std::sync::atomic::{AtomicUsize, Ordering};

pub use bevy_ecs_macros::{ScheduleLabel, SystemSet};
use bevy_utils::define_boxed_label;
Expand All @@ -23,6 +24,10 @@ pub trait SystemSet: DynHash + Debug + Send + Sync + 'static {
None
}

/// Returns `true` if this system set is an [`AnonymousSet`].
fn is_anonymous(&self) -> bool {
false
}
/// Creates a boxed clone of the label corresponding to this system set.
fn dyn_clone(&self) -> Box<dyn SystemSet>;
}
Expand Down Expand Up @@ -102,6 +107,29 @@ impl<T> SystemSet for SystemTypeSet<T> {
}
}

/// A [`SystemSet`] implicitly created when using
/// [`Schedule::add_systems`](super::Schedule::add_systems).
#[derive(Clone, Copy, Debug, Hash, PartialEq, Eq)]
pub struct AnonymousSet(usize);

static NEXT_ANONYMOUS_SET_ID: AtomicUsize = AtomicUsize::new(0);

impl AnonymousSet {
pub(crate) fn new() -> Self {
Self(NEXT_ANONYMOUS_SET_ID.fetch_add(1, Ordering::Relaxed))
}
}

impl SystemSet for AnonymousSet {
fn is_anonymous(&self) -> bool {
true
}

fn dyn_clone(&self) -> Box<dyn SystemSet> {
Box::new(*self)
}
}

/// Types that can be converted into a [`SystemSet`].
pub trait IntoSystemSet<Marker>: Sized {
type Set: SystemSet;
Expand Down