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] - Optional .system(), part 2 #2403

Closed
wants to merge 2 commits into from

Conversation

Ratysz
Copy link
Contributor

@Ratysz Ratysz commented Jun 27, 2021

Objective

Solution

  • Slight change to ParallelSystemDescriptorCoercion signature and implementors.

I haven't touched exclusive systems, because it looks like the only two other solutions are going back to doubling our system insertion methods, or starting to lean into stageless. The latter will invalidate the former, so I think exclusive systems should remian pariahs until stageless.

I can grep & nuke .system() thorughout the codebase now, which might take a while, or we can do that in subsequent PR(s).

@github-actions github-actions bot added the S-Needs-Triage This issue needs to be labelled label Jun 27, 2021
@NathanSWard NathanSWard added C-Code-Quality A section of code that is hard to understand or change A-ECS Entities, components, systems, and events and removed S-Needs-Triage This issue needs to be labelled labels Jun 29, 2021
@cart
Copy link
Member

cart commented Jun 29, 2021

bors r+

bors bot pushed a commit that referenced this pull request Jun 29, 2021
# Objective

- Extend work done in #2398.
- Make `.system()` syntax optional when using system descriptor API.

## Solution

- Slight change to `ParallelSystemDescriptorCoercion` signature and implementors.

---

I haven't touched exclusive systems, because it looks like the only two other solutions are going back to doubling our system insertion methods, or starting to lean into stageless. The latter will invalidate the former, so I think exclusive systems should remian pariahs until stageless.

I can grep & nuke `.system()` thorughout the codebase now, which might take a while, or we can do that in subsequent PR(s).
@bors bors bot changed the title Optional .system(), part 2 [Merged by Bors] - Optional .system(), part 2 Jun 29, 2021
@bors bors bot closed this Jun 29, 2021
@Ratysz Ratysz deleted the optional_system_pt2 branch July 1, 2021 08:46
bors bot pushed a commit that referenced this pull request Jul 1, 2021
# Objective

- Continue work of #2398 and #2403.
- Make `.system()` syntax optional when using `.config()` API.

## Solution

- Introduce new prelude trait, `ConfigurableSystem`, that shorthands `my_system.system().config(...)` as `my_system.config(...)`.
- Expand `configure_system_local` test to also cover the new syntax.
@Ratysz Ratysz added A-Core Common functionality for all bevy apps C-Usability A targeted quality-of-life change that makes Bevy easier to use and removed C-Code-Quality A section of code that is hard to understand or change A-ECS Entities, components, systems, and events labels Jul 4, 2021
ostwilkens pushed a commit to ostwilkens/bevy that referenced this pull request Jul 27, 2021
# Objective

- Extend work done in bevyengine#2398.
- Make `.system()` syntax optional when using system descriptor API.

## Solution

- Slight change to `ParallelSystemDescriptorCoercion` signature and implementors.

---

I haven't touched exclusive systems, because it looks like the only two other solutions are going back to doubling our system insertion methods, or starting to lean into stageless. The latter will invalidate the former, so I think exclusive systems should remian pariahs until stageless.

I can grep & nuke `.system()` thorughout the codebase now, which might take a while, or we can do that in subsequent PR(s).
ostwilkens pushed a commit to ostwilkens/bevy that referenced this pull request Jul 27, 2021
# Objective

- Continue work of bevyengine#2398 and bevyengine#2403.
- Make `.system()` syntax optional when using `.config()` API.

## Solution

- Introduce new prelude trait, `ConfigurableSystem`, that shorthands `my_system.system().config(...)` as `my_system.config(...)`.
- Expand `configure_system_local` test to also cover the new syntax.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Core Common functionality for all bevy apps C-Usability A targeted quality-of-life change that makes Bevy easier to use
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants