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

run_if for SystemConfigs via anonymous system sets #7676

Merged
merged 5 commits into from
Mar 30, 2023
Merged
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
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
41 changes: 36 additions & 5 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 Down Expand Up @@ -167,8 +168,11 @@ impl SystemConfigs {
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 an 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
65 changes: 59 additions & 6 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,11 @@ impl ScheduleGraph {
}
}
}
SystemConfigs::Configs { configs, chained } => {
SystemConfigs::Configs {
configs,
collective_conditions,
chained,
} => {
let mut config_iter = configs.into_iter();
let mut nodes_in_scope = Vec::new();
let mut densely_chained = true;
Expand All @@ -536,9 +544,26 @@ impl ScheduleGraph {
densely_chained: true
}
};
let more_than_one_entry = config_iter.len() > 0;
let anonymous_set = if more_than_one_entry && !collective_conditions.is_empty()
cart marked this conversation as resolved.
Show resolved Hide resolved
cart marked this conversation as resolved.
Show resolved Hide resolved
{
Some(AnonymousSet::new())
} else {
None
};
let prev = if let Some(anonymous_set) = anonymous_set {
cart marked this conversation as resolved.
Show resolved Hide resolved
prev.in_set(anonymous_set)
} else {
prev
};
let mut previous_result = self.add_systems_inner(prev, true);
densely_chained = previous_result.densely_chained;
for current in config_iter {
let current = if let Some(anonymous_set) = anonymous_set {
current.in_set(anonymous_set)
} else {
current
};
let current_result = self.add_systems_inner(current, true);
densely_chained = densely_chained && current_result.densely_chained;
match (
Expand Down Expand Up @@ -608,7 +633,18 @@ impl ScheduleGraph {
}
} else {
let more_than_one_entry = config_iter.len() > 1;
let anonymous_set = if more_than_one_entry && !collective_conditions.is_empty()
{
Some(AnonymousSet::new())
} else {
None
};
for config in config_iter {
let config = if let Some(anonymous_set) = anonymous_set {
config.in_set(anonymous_set)
} else {
config
};
let result = self.add_systems_inner(config, ancestor_chained);
densely_chained = densely_chained && result.densely_chained;
if ancestor_chained {
Expand Down Expand Up @@ -705,8 +741,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 +777,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 +1291,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