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

Added a generic systems example to the examples folder in bevy. #2191

Closed
wants to merge 8 commits into from
Closed

Added a generic systems example to the examples folder in bevy. #2191

wants to merge 8 commits into from

Conversation

AmrDhaliwal
Copy link

Hi @alice-i-cecile,

Sorry this took me so long but, here's the code could you take a look and let
me know if I am heading in the right direction.

Thanks!

@alice-i-cecile alice-i-cecile added C-Feature A new feature, making something new possible C-Examples An addition or correction to our examples labels May 17, 2021
@NathanSWard
Copy link
Contributor

NathanSWard commented May 17, 2021

So far this seems like a good starting point!
However, it'll definitely need some fleshing out.

A few logistical things,

  1. This will need to be added as an [[example]] in the primary Cargo.toml file
  2. This will need to be added in the examples/README.md

Now with a few more suggestions

  1. All example are runnable, meaning they have a fn main() associated with them so that a user is able to run the example.
    • So having a fn main() along with an App::build().add_system(MyGenericSystem::<T>.system()) will be needed.
  2. Since this is covering generic system, we need to highlight the primary reason we'd use them. And that is re-usability.
    • So ideally, you would have 1 generic system, and then you would add it it at least 2 times to the primary application with a different generic type for each addition.

for example

struct A;
struct B;

fn system<T>(_: Query<&T>) {}

fn main() {
    App::build()
        .add_system(system::<A>.system())
        .add_system(system::<B>.system())
        .run();
}
  1. While the remove_entity system you provided was a great start, there won't actually be an entities alive to remove making the system do nothing.
    • It would be nice to have the system do something the the user can view. Perhaps this is a spawn_entity<T>, or a log_component<T> for example.

Again great start and please ask if you have any questions along the way! 😄

@alice-i-cecile
Copy link
Member

Awesome, thanks for contributing! You may also find this advice on writing examples helpful to you :)

Nathan's advice above is great. You'll need to use .add_system(my_system::<A>.system()) to get it to actually register though :p

I think it makes sense to think about when you want to use this pattern, and communicate that to users through your choice of example. To me, generic systems are useful when you need to perform the same operation on several distinct but similar component or resource types. In Bevy itself, we use it for events :) Coming up with those details is a lot of the fun of this work, but let us know if you'd like suggestions.

P.S. This fits nicely into the ecs folder of the examples, since it's just showcasing an advanced way to use systems.

@AmrDhaliwal
Copy link
Author

Wow, thanks for all the advice and now I think I understand what the generic system should actually be doing.
I will go ahead and start working on them.

Thanks again!

@AmrDhaliwal
Copy link
Author

Hi @alice-i-cecile,

I added the generic_systems to the examples for the README and Cargo.toml.
I was hoping you could take a look at the code and see if it is more in line than before.

Thanks!

examples/ecs/generic_systems.rs Outdated Show resolved Hide resolved
examples/ecs/generic_systems.rs Outdated Show resolved Hide resolved
@AmrDhaliwal
Copy link
Author

Hi @alice-i-cecile and @NathanSWard,

I added mouse inputs in hopes that when either mouse left or right is clicked it would spawn an entity.
Would this be a good example?

@alice-i-cecile
Copy link
Member

FYI @AmrDhaliwal, as a PR author on GitHub you can press "Resolve Conversation" on comments in this thread. This makes it easier to skim and see which concerns still need to be addressed :)

let id = commands.spawn().insert(T).id();
info!("Spawned entity {:?} with component {}", id, std::any::type_name::<T>());
}
mouse_button_inputs.just_pressed(MouseButton::Left);
Copy link
Member

Choose a reason for hiding this comment

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

This line and the one below don't seem to do anything?

examples/ecs/generic_systems.rs Outdated Show resolved Hide resolved
examples/ecs/generic_systems.rs Outdated Show resolved Hide resolved
examples/ecs/generic_systems.rs Outdated Show resolved Hide resolved
@AmrDhaliwal
Copy link
Author

@NathanSWard Hopefully I got this right.

fn main() {
App::build()
.add_plugins(DefaultPlugins)
.add_system(spawn_entities_on_click::<A>.system())
Copy link
Member

Choose a reason for hiding this comment

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

I think this needs the second type parameter to work :)

Copy link
Author

Choose a reason for hiding this comment

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

@alice-i-cecile do you mean the second parameter type for the function?

@Ratysz
Copy link
Contributor

Ratysz commented Jun 5, 2021

Hopefully I got this right.

That's... not really how this works. Have you tried running the example you wrote? It doesn't compile, because the syntax is incorrect.

I would also recommend using cargo fmt, whenever you compile or even just save the file.

@alice-i-cecile
Copy link
Member

bors try

bors bot added a commit that referenced this pull request Jun 7, 2021
@bors
Copy link
Contributor

bors bot commented Jun 7, 2021

try

Build failed:

@Weibye
Copy link
Contributor

Weibye commented Jun 18, 2021

You will need to add the example to the examples/README.md in order for the CI to succeed :)

@mockersf mockersf added the S-Pre-Relicense This PR was made before Bevy added the Apache license. Cannot be merged or used for other work label Jul 16, 2021
@alice-i-cecile
Copy link
Member

I tried my hand at my own generic system example for the book. Working with UI is way too verbose for that though :(

Here's my almost-complete code; feel free to use or learn from it:

use bevy::{app::AppExit, prelude::*};

fn main() {
    App::new()
        .add_plugins(DefaultPlugins)
        .add_startup_system(spawn_buttons_system)
        // Events are automatically cleaned up using generic systems added with add_event!
        .add_event::<Hello>()
        .add_event::<Goodbye>()
        // This system operates on buttons with a Hello component
        .add_system(event_button_system::<Hello>)
        // But this one only operates on those with a Goodbye component
        .add_system(event_button_system::<Goodbye>)
        // We're not using generic systems for event handling]
        // because we need different behavior
        .add_system(hello_system)
        .add_system(goodbye_system)
        .run()
}

// We're using these as both a components
// and event types
#[derive(Clone)]
struct Hello {
    name: String,
}
#[derive(Clone)]
struct Goodbye;

fn spawn_buttons_system(mut commands: Commands) {
    // We need a UI camera so we can see our buttons
    commands.spawn_bundle(UiCameraBundle::default());

    // We're quickly building 3 buttons in a UI hierarchy
    // FIXME: add buttons
}

fn event_button_system<E: Send + Sync + Clone + 'static>(
    // Interaction is the built-in type used to
    // detect mouse interactions with buttons
    query: Query<(&E, &Interaction)>,
    mut writer: EventWriter<E>,
) {
    // By using & here we can avoid dereferencing our unpacked components later
    for (event_component, &interaction) in query.iter() {
        if interaction == Interaction::Clicked {
            // This will send any data stored in the component in the event as well
            writer.send(event_component.clone());
        }
    }
}

fn hello_system(mut reader: EventReader<Hello>) {
    for hello_event in reader.iter() {
        println!("Hello {}!", hello_event.name);
    }
}

// AppExit is a built-in event type that quits the app when it is received
fn goodbye_system(mut goodbye_reader: EventReader<Goodbye>, mut exit_writer: EventWriter<AppExit>) {
    if goodbye_reader.iter().count() > 0 {
        println!("Goodbye!");
        exit_writer.send(AppExit);
    }
}

@alice-i-cecile
Copy link
Member

I'm going to close this out in favor of #2636 <3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-Examples An addition or correction to our examples C-Feature A new feature, making something new possible S-Pre-Relicense This PR was made before Bevy added the Apache license. Cannot be merged or used for other work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants