[Merged by Bors] - Allow higher order systems #4833
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
DJMcNab
added
A-ECS
NiklasEi
reviewed
May 24, 2022
DJMcNab
commented
May 30, 2022
| /// This trait can be useful for making your own adapters to systems. | ||
| /// | ||
| /// This should be used in combination with [`ParamSet`] where appropriate to | ||
| /// create higher order systems. |
There was a problem hiding this comment.
Briefly define higher order systems here.
| fn run(&mut self, _input: (), param_value: SystemParamItem< ($($param,)*)>) -> Out { | ||
| // Yes, this is strange, but `rustc` fails to compile this impl | ||
| // without using this function. It fails to recognise that `func` | ||
| // is a function, potentially because of the multiple impls of `FnMut` |
There was a problem hiding this comment.
Good to see this comment expanded.
alice-i-cecile
approved these changes
May 30, 2022
There was a problem hiding this comment.
Please add a description of higher order systems to your PR description. I think they're systems that can generate other systems, but other readers won't even be able to guess.
The code changes make sense, and after seeing the doc example I'm super impressed by the expressivity of this design. Very minimal changes required, and I'm excited to see what can be made with this.
Are you planning to rewrite ChainSystem with this?
|
Rewriting ChainSystem is the linked PR, which people weren't happy with, mainly due to the dynamic system limitations. We need to think up a nice API for dynamic systems. |
TheRawMeatball
approved these changes
May 30, 2022
Also add some more doc comments
DJMcNab
added
the
S-Ready-For-Final-Review
|
bors r+ |
bors bot
pushed a commit
that referenced
this pull request
May 30, 2022
# Objective
- Higher order system could not be created by users.
- However, a simple change to `SystemParamFunction` allows this.
- Higher order systems in this case mean functions which return systems created using other systems, such as `chain` (which is basically equivalent to map)
## Solution
- Change `SystemParamFunction` to be a safe abstraction over `FnMut([In<In>,] ...params)->Out`.
- Note that I believe `SystemParamFunction` should not have been counted as part of our public api before this PR.
- This is because its only use was an unsafe function without an actionable safety comment.
- The safety comment was basically 'call this within bevy code'.
- I also believe that there are no external users in its current form.
- A quick search on Google and in the discord confirmed this.
## See also
- #4666, which uses this and subsumes the example here
---
## Changelog
### Added
- `SystemParamFunction`, which can be used to create higher order systems.
bors
bot
changed the title
james7132
pushed a commit
to james7132/bevy
that referenced
this pull request
Jun 7, 2022
# Objective
- Higher order system could not be created by users.
- However, a simple change to `SystemParamFunction` allows this.
- Higher order systems in this case mean functions which return systems created using other systems, such as `chain` (which is basically equivalent to map)
## Solution
- Change `SystemParamFunction` to be a safe abstraction over `FnMut([In<In>,] ...params)->Out`.
- Note that I believe `SystemParamFunction` should not have been counted as part of our public api before this PR.
- This is because its only use was an unsafe function without an actionable safety comment.
- The safety comment was basically 'call this within bevy code'.
- I also believe that there are no external users in its current form.
- A quick search on Google and in the discord confirmed this.
## See also
- bevyengine#4666, which uses this and subsumes the example here
---
## Changelog
### Added
- `SystemParamFunction`, which can be used to create higher order systems.
ItsDoot
pushed a commit
to ItsDoot/bevy
that referenced
this pull request
Feb 1, 2023
# Objective
- Higher order system could not be created by users.
- However, a simple change to `SystemParamFunction` allows this.
- Higher order systems in this case mean functions which return systems created using other systems, such as `chain` (which is basically equivalent to map)
## Solution
- Change `SystemParamFunction` to be a safe abstraction over `FnMut([In<In>,] ...params)->Out`.
- Note that I believe `SystemParamFunction` should not have been counted as part of our public api before this PR.
- This is because its only use was an unsafe function without an actionable safety comment.
- The safety comment was basically 'call this within bevy code'.
- I also believe that there are no external users in its current form.
- A quick search on Google and in the discord confirmed this.
## See also
- bevyengine#4666, which uses this and subsumes the example here
---
## Changelog
### Added
- `SystemParamFunction`, which can be used to create higher order systems.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Objective
SystemParamFunctionallows this.chain(which is basically equivalent to map)Solution
SystemParamFunctionto be a safe abstraction overFnMut([In<In>,] ...params)->Out.SystemParamFunctionshould not have been counted as part of our public api before this PR.See also
Changelog
Added
SystemParamFunction, which can be used to create higher order systems.