[Merged by Bors] - Add a module for common system `chain`/`pipe` adapters #5776

JoJoJet · 2022-08-23T20:07:02Z

Objective

Right now, users have to implement basic system adapters such as Option <-> Result conversions by themselves. This is slightly annoying and discourages the use of system chaining.

Solution

Add the module system_adapter to the prelude, which contains a collection of common adapters. This is very ergonomic in practice.

Examples

Convenient early returning.

use bevy::prelude::*;

App::new()
    // If the system fails, just try again next frame.
    .add_system(pet_dog.chain(system_adapter::ignore))
    .run();

#[derive(Component)]
struct Dog;

fn pet_dog(dogs: Query<(&Name, Option<&Parent>), With<Dog>>) -> Option<()> {
    let (dog, dad) = dogs.iter().next()?;
    println!("You pet {dog}. He/she/they are a good boy/girl/pupper.");
    let (dad, _) = dogs.get(dad?.get()).ok()?;
    println!("Their dad's name is {dad}");
    Some(())
}

Converting the output of a system

use bevy::prelude::*;

App::new()
    .add_system(
        find_name
            .chain(system_adapter::new(String::from))
            .chain(spawn_with_name),
    )
    .run();

fn find_name() -> &'static str { /* ... */ }
fn spawn_with_name(In(name): In<String>, mut commands: Commands) {
    commands.spawn().insert(Name::new(name));
}

Changelog

Added the module bevy_ecs::prelude::system_adapter, which contains a collection of common system chaining adapters.
- new - Converts a regular fn to a system adapter.
- unwrap - Similar to Result::unwrap
- ignore - Discards the output of the previous system.

alice-i-cecile · 2022-08-23T20:09:24Z

I'd love to see info / debug / warn / error adaptors for Result return types too :) Doesn't have to be in this PR, but it seems like a nice fit.

JoJoJet · 2022-08-23T20:15:51Z

It would be helpful if chained systems had a way of accessing the name of the outer ChainSystem, for debug purposes. I'm planning on opening an issue about that.

afonsolage · 2022-08-23T21:39:40Z

crates/bevy_ecs/src/lib.rs

-            Commands, In, IntoChainSystem, IntoExclusiveSystem, IntoSystem, Local, NonSend,
-            NonSendMut, ParallelCommands, ParamSet, Query, RemovedComponents, Res, ResMut,
-            Resource, System, SystemParamFunction,
+            adapter as system_adapter, Commands, In, IntoChainSystem, IntoExclusiveSystem,


Suggested change

adapter as system_adapter, Commands, In, IntoChainSystem, IntoExclusiveSystem,

adapter as chain_adapter, Commands, In, IntoChainSystem, IntoExclusiveSystem,

Wouldn't it be better to use chain_adapter as those adapters can only be used on ChainSystems?

The current plan is to rename .chain() to .pipe(), so I'd rather not give it a name that we'd have to change later.

Also, system_adapter feels more consistent to me. The two ways of accessing this module are bevy::ecs::system::adapter and bevy::prelude::system_adapter. Like how bevy_ecs and bevy::ecs mean the same thing.

crates/bevy_ecs/src/system/system_chaining.rs

Co-authored-by: Afonso Lage <lage.afonso@gmail.com>

crates/bevy_ecs/src/system/system_chaining.rs

tguichaoua · 2022-08-26T08:26:52Z

crates/bevy_ecs/src/system/system_chaining.rs

+    ///     println!("{x:?}");
+    /// }
+    /// ```
+    pub fn new<T, U>(mut f: impl FnMut(T) -> U) -> impl FnMut(In<T>) -> U {


IMHO new may be confusing. In rust new is generally used to create an object.
Since this is a mapping operation, I think map would fit better.

I respectfully disagree. The meaning behind the name system_adapter::new is "create a new system adapter from some fn", which should be clear enough IMO. And the resulting system adapter is an object, of course.

I don't think it makes sense to call it map, since the mapping is semantically being done by the .chain() call.

I didn't take into consideration the mod name. It makes sense.

cart

I'm happy with where this is at. Definitely worth following up with tracing adapters, but those can wait.

cart · 2022-08-30T00:06:11Z

bors r+

# Objective Right now, users have to implement basic system adapters such as `Option` <-> `Result` conversions by themselves. This is slightly annoying and discourages the use of system chaining. ## Solution Add the module `system_adapter` to the prelude, which contains a collection of common adapters. This is very ergonomic in practice. ## Examples Convenient early returning. ```rust use bevy::prelude::*; App::new() // If the system fails, just try again next frame. .add_system(pet_dog.chain(system_adapter::ignore)) .run(); #[derive(Component)] struct Dog; fn pet_dog(dogs: Query<(&Name, Option<&Parent>), With<Dog>>) -> Option<()> { let (dog, dad) = dogs.iter().next()?; println!("You pet {dog}. He/she/they are a good boy/girl/pupper."); let (dad, _) = dogs.get(dad?.get()).ok()?; println!("Their dad's name is {dad}"); Some(()) } ``` Converting the output of a system ```rust use bevy::prelude::*; App::new() .add_system( find_name .chain(system_adapter::new(String::from)) .chain(spawn_with_name), ) .run(); fn find_name() -> &'static str { /* ... */ } fn spawn_with_name(In(name): In<String>, mut commands: Commands) { commands.spawn().insert(Name::new(name)); } ``` --- ## Changelog * Added the module `bevy_ecs::prelude::system_adapter`, which contains a collection of common system chaining adapters. * `new` - Converts a regular fn to a system adapter. * `unwrap` - Similar to `Result::unwrap` * `ignore` - Discards the output of the previous system.

bors · 2022-08-30T00:32:59Z

Pull request successfully merged into main.

Build succeeded:

# Objective Right now, users have to implement basic system adapters such as `Option` <-> `Result` conversions by themselves. This is slightly annoying and discourages the use of system chaining. ## Solution Add the module `system_adapter` to the prelude, which contains a collection of common adapters. This is very ergonomic in practice. ## Examples Convenient early returning. ```rust use bevy::prelude::*; App::new() // If the system fails, just try again next frame. .add_system(pet_dog.chain(system_adapter::ignore)) .run(); #[derive(Component)] struct Dog; fn pet_dog(dogs: Query<(&Name, Option<&Parent>), With<Dog>>) -> Option<()> { let (dog, dad) = dogs.iter().next()?; println!("You pet {dog}. He/she/they are a good boy/girl/pupper."); let (dad, _) = dogs.get(dad?.get()).ok()?; println!("Their dad's name is {dad}"); Some(()) } ``` Converting the output of a system ```rust use bevy::prelude::*; App::new() .add_system( find_name .chain(system_adapter::new(String::from)) .chain(spawn_with_name), ) .run(); fn find_name() -> &'static str { /* ... */ } fn spawn_with_name(In(name): In<String>, mut commands: Commands) { commands.spawn().insert(Name::new(name)); } ``` --- ## Changelog * Added the module `bevy_ecs::prelude::system_adapter`, which contains a collection of common system chaining adapters. * `new` - Converts a regular fn to a system adapter. * `unwrap` - Similar to `Result::unwrap` * `ignore` - Discards the output of the previous system.

# Objective Fixes #6224, add ``dbg``, ``info``, ``warn`` and ``error`` system piping adapter variants to expand #5776, which call the corresponding re-exported [bevy_log macros](https://docs.rs/bevy/latest/bevy/log/macro.info.html) when the result is an error. ## Solution * Added ``dbg``, ``info``, ``warn`` and ``error`` system piping adapter variants to ``system_piping.rs``. * Modified and added tests for these under examples in ``system_piping.rs``.

…gine#6751) # Objective Fixes bevyengine#6224, add ``dbg``, ``info``, ``warn`` and ``error`` system piping adapter variants to expand bevyengine#5776, which call the corresponding re-exported [bevy_log macros](https://docs.rs/bevy/latest/bevy/log/macro.info.html) when the result is an error. ## Solution * Added ``dbg``, ``info``, ``warn`` and ``error`` system piping adapter variants to ``system_piping.rs``. * Modified and added tests for these under examples in ``system_piping.rs``.

# Objective Right now, users have to implement basic system adapters such as `Option` <-> `Result` conversions by themselves. This is slightly annoying and discourages the use of system chaining. ## Solution Add the module `system_adapter` to the prelude, which contains a collection of common adapters. This is very ergonomic in practice. ## Examples Convenient early returning. ```rust use bevy::prelude::*; App::new() // If the system fails, just try again next frame. .add_system(pet_dog.chain(system_adapter::ignore)) .run(); #[derive(Component)] struct Dog; fn pet_dog(dogs: Query<(&Name, Option<&Parent>), With<Dog>>) -> Option<()> { let (dog, dad) = dogs.iter().next()?; println!("You pet {dog}. He/she/they are a good boy/girl/pupper."); let (dad, _) = dogs.get(dad?.get()).ok()?; println!("Their dad's name is {dad}"); Some(()) } ``` Converting the output of a system ```rust use bevy::prelude::*; App::new() .add_system( find_name .chain(system_adapter::new(String::from)) .chain(spawn_with_name), ) .run(); fn find_name() -> &'static str { /* ... */ } fn spawn_with_name(In(name): In<String>, mut commands: Commands) { commands.spawn().insert(Name::new(name)); } ``` --- ## Changelog * Added the module `bevy_ecs::prelude::system_adapter`, which contains a collection of common system chaining adapters. * `new` - Converts a regular fn to a system adapter. * `unwrap` - Similar to `Result::unwrap` * `ignore` - Discards the output of the previous system.

…gine#6751) # Objective Fixes bevyengine#6224, add ``dbg``, ``info``, ``warn`` and ``error`` system piping adapter variants to expand bevyengine#5776, which call the corresponding re-exported [bevy_log macros](https://docs.rs/bevy/latest/bevy/log/macro.info.html) when the result is an error. ## Solution * Added ``dbg``, ``info``, ``warn`` and ``error`` system piping adapter variants to ``system_piping.rs``. * Modified and added tests for these under examples in ``system_piping.rs``.

JoJoJet added 4 commits August 23, 2022 15:49

Add a module for common system adapters

4ae6f19

Add Option/Result conversion adapters

f91ac9f

Add the system_adapter module to the prelude

4b9a5f7

Add module-level documentation

83d5456

alice-i-cecile added A-ECS Entities, components, systems, and events C-Usability A simple quality-of-life change that makes Bevy easier to use labels Aug 23, 2022

JoJoJet added 3 commits August 23, 2022 16:45

Add conversion adapters

baea3d1

Add the parse adapter

da1935b

Add a test to assert that adapters are systems

3c3e050

afonsolage approved these changes Aug 23, 2022

View reviewed changes

JoJoJet and others added 2 commits August 23, 2022 17:57

Remove a note

bddf844

Apply suggestions from code review

cadd7a7

Co-authored-by: Afonso Lage <lage.afonso@gmail.com>

JoJoJet marked this pull request as ready for review August 23, 2022 22:26

JoJoJet mentioned this pull request Aug 23, 2022

Allow systems in a chain to access the name of the outer ChainSystem #5777

Open

cart reviewed Aug 24, 2022

View reviewed changes

crates/bevy_ecs/src/system/system_chaining.rs Show resolved Hide resolved

JoJoJet added 3 commits August 25, 2022 12:45

Remove trivial adapters

62ca3a9

Add a fn to convert a regular function to an adapter

abe715f

Remove ok* adapters

9324272

tguichaoua reviewed Aug 26, 2022

View reviewed changes

cart approved these changes Aug 30, 2022

View reviewed changes

bors bot changed the title ~~Add a module for common system chain/pipe adapters~~ [Merged by Bors] - Add a module for common system chain/pipe adapters Aug 30, 2022

bors bot closed this Aug 30, 2022

JoJoJet deleted the adapters branch August 30, 2022 02:06

alice-i-cecile mentioned this pull request Oct 10, 2022

Add logging variants of system chaining / piping adaptors #6224

Closed

Edwox mentioned this pull request Nov 25, 2022

[Merged by Bors] - [Fixes #6224] Add logging variants of system piping #6751

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Merged by Bors] - Add a module for common system `chain`/`pipe` adapters #5776

[Merged by Bors] - Add a module for common system `chain`/`pipe` adapters #5776

JoJoJet commented Aug 23, 2022 •

edited

alice-i-cecile commented Aug 23, 2022

JoJoJet commented Aug 23, 2022 •

edited

afonsolage Aug 23, 2022

JoJoJet Aug 23, 2022 •

edited

afonsolage Aug 23, 2022

tguichaoua Aug 26, 2022

JoJoJet Aug 26, 2022 •

edited

tguichaoua Aug 26, 2022

cart left a comment

cart commented Aug 30, 2022

bors bot commented Aug 30, 2022

	adapter as system_adapter, Commands, In, IntoChainSystem, IntoExclusiveSystem,
	adapter as chain_adapter, Commands, In, IntoChainSystem, IntoExclusiveSystem,

[Merged by Bors] - Add a module for common system chain/pipe adapters #5776

[Merged by Bors] - Add a module for common system chain/pipe adapters #5776

Conversation

JoJoJet commented Aug 23, 2022 • edited

Objective

Solution

Examples

Changelog

alice-i-cecile commented Aug 23, 2022

JoJoJet commented Aug 23, 2022 • edited

afonsolage Aug 23, 2022

Choose a reason for hiding this comment

JoJoJet Aug 23, 2022 • edited

Choose a reason for hiding this comment

afonsolage Aug 23, 2022

Choose a reason for hiding this comment

tguichaoua Aug 26, 2022

Choose a reason for hiding this comment

JoJoJet Aug 26, 2022 • edited

Choose a reason for hiding this comment

tguichaoua Aug 26, 2022

Choose a reason for hiding this comment

cart left a comment

Choose a reason for hiding this comment

cart commented Aug 30, 2022

bors bot commented Aug 30, 2022

[Merged by Bors] - Add a module for common system `chain`/`pipe` adapters #5776

[Merged by Bors] - Add a module for common system `chain`/`pipe` adapters #5776

JoJoJet commented Aug 23, 2022 •

edited

JoJoJet commented Aug 23, 2022 •

edited

JoJoJet Aug 23, 2022 •

edited

JoJoJet Aug 26, 2022 •

edited