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

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

Closed
wants to merge 12 commits into from
Prev Previous commit
Next Next commit
Add a fn to convert a regular function to an adapter
JoJoJet committed Aug 25, 2022
commit abe715f7938a6a98d3ca763da19f37b6a82c7c8c
23 changes: 23 additions & 0 deletions crates/bevy_ecs/src/system/system_chaining.rs
Original file line number Diff line number Diff line change
@@ -148,6 +148,26 @@ pub mod adapter {
use crate::system::In;
use std::fmt::Debug;

/// Converts a regular function into a system adapter.
///
/// # Examples
/// ```
/// use bevy_ecs::prelude::*;
///
/// return1
/// .chain(system_adapter::new(u32::try_from))
/// .chain(system_adapter::unwrap)
/// .chain(print);
///
/// fn return1() -> u64 { 1 }
/// fn print(In(x): In<impl std::fmt::Debug>) {
/// println!("{x:?}");
/// }
/// ```
pub fn new<T, U>(mut f: impl FnMut(T) -> U) -> impl FnMut(In<T>) -> U {
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Member Author

@JoJoJet JoJoJet Aug 26, 2022

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

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

move |In(x)| f(x)
}

/// System adapter that converts [`Result<T, _>`] into [`Option<T>`].
pub fn ok<T, E>(In(res): In<Result<T, E>>) -> Option<T> {
res.ok()
@@ -261,6 +281,8 @@ pub mod adapter {
#[cfg(test)]
#[test]
fn assert_systems() {
use std::str::FromStr;

use crate::{prelude::*, system::assert_is_system};

/// Mocks a system that returns a value of type `T`.
@@ -274,5 +296,6 @@ pub mod adapter {

assert_is_system(returning::<Result<u32, std::io::Error>>.chain(unwrap));
assert_is_system(returning::<Option<()>>.chain(ignore));
assert_is_system(returning::<&str>.chain(new(u64::from_str)).chain(unwrap));
}
}