Add system.map(...) for transforming the output of a system
#8526
Conversation
james7132
added
C-Feature
| /// println!("{x:?}"); | ||
| /// } | ||
| /// ``` | ||
| #[deprecated = "use `.map(...)` instead"] |
There was a problem hiding this comment.
Clever: I like being able to deprecate these.
cart
left a comment
cart
left a comment
There was a problem hiding this comment.
Seems like a good complement to pipe. Simpler interface (avoids In<T>). .map is a common rust convention. Saves on some expenses.
I think in a world where we find a way to remove In<T> (in favor of just T) I might advocate for removing map, as I'm not sure avoiding the startup allocations for name() is worth "duplicate" abstractions. We could probably make name lazily computed if that is a concern as most games won't even need system names at runtime.
|
I've fixed the Also, I'm going to temporarily mark this as a draft. I want to see if I can make this abstraction usable for #8705 -- this will require giving the system adapter access to the system's name, but I don't have time to experiment with that at the moment. |
joseph-gio
changed the title
system.map(...) for transforming the output of a system
|
Alright, I've simplified the API for system names which will make it easier to use this in the aforementioned PR (and makes it consistent with I believe I have addressed all of the concerns raised :). |
|
What's the status on this PR? Interesting in using |
alice-i-cecile
added
the
S-Ready-For-Final-Review
|
Is it intentional that the changelog is still "TODO"? |
|
I don't think we still do changelogs. Not sure why it's still in the PR template. |
|
We do still do changelogs, we just lean on auto-generation more. Including changelogs in PRs is still welcome and encouraged as it will make the auto-generated changelog better. |
…gine#8526) # Objective Any time we wish to transform the output of a system, we currently use system piping to do so: ```rust my_system.pipe(|In(x)| do_something(x)) ``` Unfortunately, system piping is not a zero cost abstraction. Each call to `.pipe` requires allocating two extra access sets: one for the second system and one for the combined accesses of both systems. This also adds extra work to each call to `update_archetype_component_access`, which stacks as one adds multiple layers of system piping. ## Solution Add the `AdapterSystem` abstraction: similar to `CombinatorSystem`, this allows you to implement a trait to generically control how a system is run and how its inputs and outputs are processed. Unlike `CombinatorSystem`, this does not have any overhead when computing world accesses which makes it ideal for simple operations such as inverting or ignoring the output of a system. Add the extension method `.map(...)`: this is similar to `.pipe(...)`, only it accepts a closure as an argument instead of an `In<T>` system. ```rust my_system.map(do_something) ``` This has the added benefit of making system names less messy: a system that ignores its output will just be called `my_system`, instead of `Pipe(my_system, ignore)` --- ## Changelog TODO ## Migration Guide The `system_adapter` functions have been deprecated: use `.map` instead, which is a lightweight alternative to `.pipe`. ```rust // Before: my_system.pipe(system_adapter::ignore) my_system.pipe(system_adapter::unwrap) my_system.pipe(system_adapter::new(T::from)) // After: my_system.map(std::mem::drop) my_system.map(Result::unwrap) my_system.map(T::from) // Before: my_system.pipe(system_adapter::info) my_system.pipe(system_adapter::dbg) my_system.pipe(system_adapter::warn) my_system.pipe(system_adapter::error) // After: my_system.map(bevy_utils::info) my_system.map(bevy_utils::dbg) my_system.map(bevy_utils::warn) my_system.map(bevy_utils::error) ``` --------- Co-authored-by: Alice Cecile <[email protected]>
…gine#8526) # Objective Any time we wish to transform the output of a system, we currently use system piping to do so: ```rust my_system.pipe(|In(x)| do_something(x)) ``` Unfortunately, system piping is not a zero cost abstraction. Each call to `.pipe` requires allocating two extra access sets: one for the second system and one for the combined accesses of both systems. This also adds extra work to each call to `update_archetype_component_access`, which stacks as one adds multiple layers of system piping. ## Solution Add the `AdapterSystem` abstraction: similar to `CombinatorSystem`, this allows you to implement a trait to generically control how a system is run and how its inputs and outputs are processed. Unlike `CombinatorSystem`, this does not have any overhead when computing world accesses which makes it ideal for simple operations such as inverting or ignoring the output of a system. Add the extension method `.map(...)`: this is similar to `.pipe(...)`, only it accepts a closure as an argument instead of an `In<T>` system. ```rust my_system.map(do_something) ``` This has the added benefit of making system names less messy: a system that ignores its output will just be called `my_system`, instead of `Pipe(my_system, ignore)` --- ## Changelog TODO ## Migration Guide The `system_adapter` functions have been deprecated: use `.map` instead, which is a lightweight alternative to `.pipe`. ```rust // Before: my_system.pipe(system_adapter::ignore) my_system.pipe(system_adapter::unwrap) my_system.pipe(system_adapter::new(T::from)) // After: my_system.map(std::mem::drop) my_system.map(Result::unwrap) my_system.map(T::from) // Before: my_system.pipe(system_adapter::info) my_system.pipe(system_adapter::dbg) my_system.pipe(system_adapter::warn) my_system.pipe(system_adapter::error) // After: my_system.map(bevy_utils::info) my_system.map(bevy_utils::dbg) my_system.map(bevy_utils::warn) my_system.map(bevy_utils::error) ``` --------- Co-authored-by: Alice Cecile <[email protected]>
6 participants
Objective
Any time we wish to transform the output of a system, we currently use system piping to do so:
Unfortunately, system piping is not a zero cost abstraction. Each call to
.piperequires allocating two extra access sets: one for the second system and one for the combined accesses of both systems. This also adds extra work to each call toupdate_archetype_component_access, which stacks as one adds multiple layers of system piping.Solution
Add the
AdapterSystemabstraction: similar toCombinatorSystem, this allows you to implement a trait to generically control how a system is run and how its inputs and outputs are processed. UnlikeCombinatorSystem, this does not have any overhead when computing world accesses which makes it ideal for simple operations such as inverting or ignoring the output of a system.Add the extension method
.map(...): this is similar to.pipe(...), only it accepts a closure as an argument instead of anIn<T>system.This has the added benefit of making system names less messy: a system that ignores its output will just be called
my_system, instead ofPipe(my_system, ignore)Changelog
TODO
Migration Guide
The
system_adapterfunctions have been deprecated: use.mapinstead, which is a lightweight alternative to.pipe.