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

Conditional execution of systems using Resources #434

Closed
wants to merge 6 commits into from

Conversation

BimDav
Copy link
Contributor

@BimDav BimDav commented Sep 4, 2020

On the subject of:
#128

This enables the user to define systems that will only be called on certain conditions, stored in Resources. This is implemented with the same logic as the ChangedRes resource query defined in #388.

I added an example of what can be done with FlagRes and ChangedRes by modifying the "greet" example from the Bevy book. The greet_people system can be rewritten as a for_each system, run on a change of the Frame resource and its call can be blocked by the Paused Resource:

use bevy::prelude::*;

struct Person;
struct Name(String);
struct GreetTimer(Timer);
struct Frame(u32);
struct Paused(bool);

impl FlagResource for Paused {
    fn flag(&self) -> bool {
        self.0
    }
}

fn fixed_timestep(time: Res<Time>, mut timer: ResMut<GreetTimer>, mut frame: ResMut<Frame>) {
    timer.0.tick(time.delta_seconds);
    if timer.0.finished {
        frame.0 += 1;
    }
}

fn greet_people(_paused: FlagRes<Paused>, frame: ChangedRes<Frame>, _person: &Person, name: &Name) {
    println!("hello {} on frame {}!", name.0, frame.0);
}

fn pause(
    mut paused: ResMut<Paused>,
    _frame: ChangedRes<Frame>,
    keyboard_input: Res<Input<KeyCode>>,
) {
    if keyboard_input.pressed(KeyCode::Left) {
        paused.0 = !paused.0;
        println!("Pause: {}", paused.0)
    }
}

fn add_people(mut commands: Commands) {
    commands
        .spawn((Person, Name("Elaina Proctor".to_string())))
        .spawn((Person, Name("Renzo Hume".to_string())))
        .spawn((Person, Name("Zayna Nieves".to_string())));
}

fn main() {
    App::build()
        .add_resource(GreetTimer(Timer::from_seconds(2.0, true)))
        .add_resource(Frame(0))
        .add_resource(Paused(false))
        .add_startup_system(add_people.system())
        .add_system(fixed_timestep.system())
        .add_system(pause.system())
        .add_system(greet_people.system())
        .add_default_plugins()
        .run();
}

@Moxinilian Moxinilian added A-ECS Entities, components, systems, and events C-Feature A new feature, making something new possible labels Sep 4, 2020
@tristanpemble
Copy link
Contributor

just my opinion, but this API feels awkward to me.

@cart
Copy link
Member

cart commented Sep 4, 2020

There is a certain amount of value in abstracting this out, but in my opinion the abstraction isn't worth the additional cognitive load. I think porting your example above to just use an if statement illustrates this:

struct Person;
struct Name(String);
struct GreetTimer(Timer);
struct Frame(u32);
struct Paused(bool);

fn fixed_timestep(time: Res<Time>, mut timer: ResMut<GreetTimer>, mut frame: ResMut<Frame>) {
    timer.0.tick(time.delta_seconds);
    if timer.0.finished {
        frame.0 += 1;
    }
}

fn greet_people(paused: Res<Paused>, frame: ChangedRes<Frame>, _person: &Person, name: &Name) {
    if paused.0 { return }
    println!("hello {} on frame {}!", name.0, frame.0);
}

This reads much more clearly. I do think it makes sense to eventually allow enabling/disabling systems, but i think it should happen at the schedule and/or scene level.

@cart
Copy link
Member

cart commented Sep 4, 2020

I think I'll close this for now. I really appreciate that you explored the idea as we now have a concrete example of one of our options in this space. But I do want to explore our other options too. We can re-open this later if it turns out this is our best option.

@cart cart closed this Sep 4, 2020
@BimDav
Copy link
Contributor Author

BimDav commented Sep 7, 2020

No problem, I learned a lot implementing this. Nonetheless, I think that this is not so unnatural once we are used to using systems with Changed and ChangedRes. The name may be poorly chosen, "OnlyIfRes" or "IfRes" may be better.

The comparison that you make with if paused.0 { return } is not really fair in my opinion because you may be borrowing archetypes and iterating on a lot of entities just to return?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ECS Entities, components, systems, and events C-Feature A new feature, making something new possible
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants