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

add Clone to common conditions #8060

Merged
merged 6 commits into from
Mar 13, 2023
Merged
Show file tree
Hide file tree
Changes from 2 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
26 changes: 13 additions & 13 deletions crates/bevy_ecs/src/schedule/condition.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ pub mod common_conditions {

/// Generates a [`Condition`](super::Condition)-satisfying closure that returns `true`
/// if the first time the condition is run and false every time after
pub fn run_once() -> impl FnMut() -> bool {
pub fn run_once() -> impl Clone + FnMut() -> bool {
Copy link
Member

Choose a reason for hiding this comment

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

readability nit: FnMut is more important here than Clone so IMO it should come first.

Copy link
Member

Choose a reason for hiding this comment

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

FnMut() -> bool + Clone looks weird to me. It makes it look like the function is returning bool + Clone somehow.

let mut has_run = false;
move || {
if !has_run {
Expand All @@ -156,7 +156,7 @@ pub mod common_conditions {

/// Generates a [`Condition`](super::Condition)-satisfying closure that returns `true`
/// if the resource exists.
pub fn resource_exists<T>() -> impl FnMut(Option<Res<T>>) -> bool
pub fn resource_exists<T>() -> impl Clone + FnMut(Option<Res<T>>) -> bool
where
T: Resource,
{
Expand Down Expand Up @@ -192,7 +192,7 @@ pub mod common_conditions {

/// Generates a [`Condition`](super::Condition)-satisfying closure that returns `true`
/// if the resource of the given type has been added since the condition was last checked.
pub fn resource_added<T>() -> impl FnMut(Option<Res<T>>) -> bool
pub fn resource_added<T>() -> impl Clone + FnMut(Option<Res<T>>) -> bool
where
T: Resource,
{
Expand All @@ -213,7 +213,7 @@ pub mod common_conditions {
/// # Panics
///
/// The condition will panic if the resource does not exist.
pub fn resource_changed<T>() -> impl FnMut(Res<T>) -> bool
pub fn resource_changed<T>() -> impl Clone + FnMut(Res<T>) -> bool
where
T: Resource,
{
Expand All @@ -231,7 +231,7 @@ pub mod common_conditions {
/// This run condition does not detect when the resource is removed.
///
/// The condition will return `false` if the resource does not exist.
pub fn resource_exists_and_changed<T>() -> impl FnMut(Option<Res<T>>) -> bool
pub fn resource_exists_and_changed<T>() -> impl Clone + FnMut(Option<Res<T>>) -> bool
where
T: Resource,
{
Expand All @@ -253,7 +253,7 @@ pub mod common_conditions {
/// has been removed since the run condition was last checked.
///
/// The condition will return `false` if the resource does not exist.
pub fn resource_changed_or_removed<T>() -> impl FnMut(Option<Res<T>>) -> bool
pub fn resource_changed_or_removed<T>() -> impl Clone + FnMut(Option<Res<T>>) -> bool
where
T: Resource,
{
Expand All @@ -273,7 +273,7 @@ pub mod common_conditions {

/// Generates a [`Condition`](super::Condition)-satisfying closure that returns `true`
/// if the resource of the given type has been removed since the condition was last checked.
pub fn resource_removed<T>() -> impl FnMut(Option<Res<T>>) -> bool
pub fn resource_removed<T>() -> impl Clone + FnMut(Option<Res<T>>) -> bool
where
T: Resource,
{
Expand All @@ -293,7 +293,7 @@ pub mod common_conditions {

/// Generates a [`Condition`](super::Condition)-satisfying closure that returns `true`
/// if the state machine exists.
pub fn state_exists<S: States>() -> impl FnMut(Option<Res<State<S>>>) -> bool {
pub fn state_exists<S: States>() -> impl Clone + FnMut(Option<Res<State<S>>>) -> bool {
move |current_state: Option<Res<State<S>>>| current_state.is_some()
}

Expand All @@ -303,7 +303,7 @@ pub mod common_conditions {
/// # Panics
///
/// The condition will panic if the resource does not exist.
pub fn in_state<S: States>(state: S) -> impl FnMut(Res<State<S>>) -> bool {
pub fn in_state<S: States>(state: S) -> impl Clone + FnMut(Res<State<S>>) -> bool {
move |current_state: Res<State<S>>| current_state.0 == state
}

Expand All @@ -313,7 +313,7 @@ pub mod common_conditions {
/// The condition will return `false` if the state does not exist.
pub fn state_exists_and_equals<S: States>(
state: S,
) -> impl FnMut(Option<Res<State<S>>>) -> bool {
) -> impl Clone + FnMut(Option<Res<State<S>>>) -> bool {
move |current_state: Option<Res<State<S>>>| match current_state {
Some(current_state) => current_state.0 == state,
None => false,
Expand All @@ -329,13 +329,13 @@ pub mod common_conditions {
/// # Panics
///
/// The condition will panic if the resource does not exist.
pub fn state_changed<S: States>() -> impl FnMut(Res<State<S>>) -> bool {
pub fn state_changed<S: States>() -> impl Clone + FnMut(Res<State<S>>) -> bool {
move |current_state: Res<State<S>>| current_state.is_changed()
}

/// Generates a [`Condition`](super::Condition)-satisfying closure that returns `true`
/// if there are any new events of the given type since it was last called.
pub fn on_event<T: Event>() -> impl FnMut(EventReader<T>) -> bool {
pub fn on_event<T: Event>() -> impl Clone + FnMut(EventReader<T>) -> bool {
// The events need to be consumed, so that there are no false positives on subsequent
// calls of the run condition. Simply checking `is_empty` would not be enough.
// PERF: note that `count` is efficient (not actually looping/iterating),
Expand All @@ -345,7 +345,7 @@ pub mod common_conditions {

/// Generates a [`Condition`](super::Condition)-satisfying closure that returns `true`
/// if there are any entities with the given component type.
pub fn any_with_component<T: Component>() -> impl FnMut(Query<(), With<T>>) -> bool {
pub fn any_with_component<T: Component>() -> impl Clone + FnMut(Query<(), With<T>>) -> bool {
move |query: Query<(), With<T>>| !query.is_empty()
}

Expand Down
30 changes: 29 additions & 1 deletion crates/bevy_ecs/src/schedule/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,16 @@ mod tests {
X,
}

#[derive(States, Default, Debug, Clone, PartialEq, Eq, Hash)]
enum TestState {
#[default]
A,
B,
}

#[derive(Resource, Default)]
struct SystemOrder(Vec<u32>);

struct SystemOrder(Vec<u32>);
#[derive(Resource, Default)]
struct RunConditionBool(pub bool);

Expand Down Expand Up @@ -198,6 +205,8 @@ mod tests {
}

mod conditions {
use condition::common_conditions::{in_state, resource_added, resource_changed};

use crate::change_detection::DetectChanges;

use super::*;
Expand All @@ -209,6 +218,7 @@ mod tests {

world.init_resource::<RunConditionBool>();
world.init_resource::<SystemOrder>();
world.init_resource::<State<TestState>>();

schedule.add_system(
make_function_system(0).run_if(|condition: Res<RunConditionBool>| condition.0),
Expand Down Expand Up @@ -248,6 +258,24 @@ mod tests {
assert_eq!(world.resource::<SystemOrder>().0, vec![0]);
}

#[test]
fn distribute_common_conditions() {
let mut world = World::default();
let mut schedule = Schedule::default();

world.init_resource::<RunConditionBool>();
world.init_resource::<SystemOrder>();
world.init_resource::<State<TestState>>();

// Ensure [`distributive_run_if`] compiles with usage of common conditions
schedule.add_systems(
(make_function_system(0), make_function_system(1))
.distributive_run_if(resource_added::<SystemOrder>())
.distributive_run_if(resource_changed::<SystemOrder>())
.distributive_run_if(in_state(TestState::A)),
);
}

#[test]
fn run_exclusive_system_with_condition() {
let mut world = World::default();
Expand Down
8 changes: 4 additions & 4 deletions crates/bevy_input/src/common_conditions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ use std::hash::Hash;
/// }
///
/// ```
pub fn input_toggle_active<T>(default: bool, input: T) -> impl FnMut(Res<Input<T>>) -> bool
pub fn input_toggle_active<T>(default: bool, input: T) -> impl Clone + FnMut(Res<Input<T>>) -> bool
where
T: Copy + Eq + Hash + Send + Sync + 'static,
{
Expand All @@ -60,7 +60,7 @@ where
}

/// Run condition that is active if [`Input::pressed`] is true for the given input.
pub fn input_pressed<T>(input: T) -> impl FnMut(Res<Input<T>>) -> bool
pub fn input_pressed<T>(input: T) -> impl Clone + FnMut(Res<Input<T>>) -> bool
where
T: Copy + Eq + Hash + Send + Sync + 'static,
{
Expand All @@ -81,15 +81,15 @@ where
///
/// # fn jump() {}
/// ```
pub fn input_just_pressed<T>(input: T) -> impl FnMut(Res<Input<T>>) -> bool
pub fn input_just_pressed<T>(input: T) -> impl Clone + FnMut(Res<Input<T>>) -> bool
where
T: Copy + Eq + Hash + Send + Sync + 'static,
{
move |inputs: Res<Input<T>>| inputs.just_pressed(input)
}

/// Run condition that is active if [`Input::just_released`] is true for the given input.
pub fn input_just_released<T>(input: T) -> impl FnMut(Res<Input<T>>) -> bool
pub fn input_just_released<T>(input: T) -> impl Clone + FnMut(Res<Input<T>>) -> bool
where
T: Copy + Eq + Hash + Send + Sync + 'static,
{
Expand Down
4 changes: 2 additions & 2 deletions crates/bevy_time/src/common_conditions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ use bevy_utils::Duration;
/// For more accurate timers, use the [`Timer`] class directly (see
/// [`Timer::times_finished_this_tick`] to address the problem mentioned above), or
/// use fixed timesteps that allow systems to run multiple times per frame.
pub fn on_timer(duration: Duration) -> impl FnMut(Res<Time>) -> bool {
pub fn on_timer(duration: Duration) -> impl Clone + FnMut(Res<Time>) -> bool {
let mut timer = Timer::new(duration, TimerMode::Repeating);
move |time: Res<Time>| {
timer.tick(time.delta());
Expand Down Expand Up @@ -67,7 +67,7 @@ pub fn on_timer(duration: Duration) -> impl FnMut(Res<Time>) -> bool {
/// Note that this run condition may not behave as expected if `duration` is smaller
/// than the fixed timestep period, since the timer may complete multiple times in
/// one fixed update.
pub fn on_fixed_timer(duration: Duration) -> impl FnMut(Res<FixedTime>) -> bool {
pub fn on_fixed_timer(duration: Duration) -> impl Clone + FnMut(Res<FixedTime>) -> bool {
let mut timer = Timer::new(duration, TimerMode::Repeating);
move |time: Res<FixedTime>| {
timer.tick(time.period);
Expand Down