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.

Sign up for GitHub

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

Jump to bottom

Support run_if for SystemConfigs #7659

Closed
lewiszlw opened this issue Feb 13, 2023 · 3 comments · Fixed by #7676
Closed

Support run_if for SystemConfigs #7659

lewiszlw opened this issue Feb 13, 2023 · 3 comments · Fixed by #7676
Labels
A-ECS Entities, components, systems, and events C-Usability A targeted quality-of-life change that makes Bevy easier to use D-Complex Quite challenging from either a design or technical perspective. Ask for help!
Milestone
0.11

Comments

@lewiszlw
Copy link
Member

What problem does this solve or what need does it fill?

Support code like below

.add_systems(
    (
        a_system,
        b_system,
    )
        .in_base_set(CoreSet::PostUpdate)
        .run_if(xxx)
)
@lewiszlw lewiszlw added C-Feature A new feature, making something new possible S-Needs-Triage This issue needs to be labelled labels Feb 13, 2023
@alice-i-cecile alice-i-cecile added A-ECS Entities, components, systems, and events C-Usability A targeted quality-of-life change that makes Bevy easier to use D-Complex Quite challenging from either a design or technical perspective. Ask for help! and removed C-Feature A new feature, making something new possible S-Needs-Triage This issue needs to be labelled labels Feb 13, 2023
@alice-i-cecile
Copy link
Member

2 doesn't work well, since I can't easily implement .run_if for SystemConfigs, as it seems to require cloning Conditions.

From #7634. I want this and looked into implementing it twice, but it's not a completely trivial change.

@alice-i-cecile alice-i-cecile added this to the 0.11 milestone Feb 13, 2023
This was referenced Feb 14, 2023
Merged
Closed
@bors bors bot closed this as completed in 67a89e9 Feb 18, 2023
myreprise1 pushed a commit to myreprise1/bevy that referenced this issue Feb 20, 2023
@geieredgar @myreprise1
Add distributive_run_if to IntoSystemConfigs (bevyengine#7724)
57bd070 
# Objective

- Fixes bevyengine#7659.

## Solution

- This PR extracted the `distributive_run_if` part of bevyengine#7676, because it does not require the controversial introduction of anonymous system sets.
- The distinctive name should make the user aware about the differences between `IntoSystemConfig::run_if` and `IntoSystemConfigs::distributive_run_if`.
- The documentation explains in detail the consequences of using the API and possible pit falls when using it.
- A test demonstrates the possibility of changing the condition result, resulting in some of the systems not being run.

---

## Changelog

### Added
- Add `distributive_run_if` to `IntoSystemConfigs` to enable adding a run condition to each system when using `add_systems`.
@james7132 james7132 modified the milestones: 0.11, 0.10 Mar 4, 2023
@lewiszlw
Copy link
Member Author

lewiszlw commented Mar 7, 2023

The issue seems not fixed yet. Below code won't compile

        .add_systems(
            (check_collision, remove_piece_component)
                .in_base_set(CoreSet::PostUpdate)
                .distributive_run_if(state_exists_and_equals(GameState::GamePlaying)),
        )

From rust doc, closures implement both Copy and Clone if all of the captured variables do. Many common conditions can't be used by distributive_run_if. Can you reopen this issue for tracking?

@alice-i-cecile alice-i-cecile reopened this Mar 7, 2023
@lewiszlw
Copy link
Member Author

lewiszlw commented Mar 8, 2023

This should be moved into milestone 0.11 .

@james7132 james7132 modified the milestones: 0.10, 0.11 Mar 8, 2023
@cart cart closed this as completed in 300b275 Mar 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-ECS Entities, components, systems, and events C-Usability A targeted quality-of-life change that makes Bevy easier to use D-Complex Quite challenging from either a design or technical perspective. Ask for help!
Projects
None yet
Milestone
0.11
3 participants
@james7132 @alice-i-cecile @lewiszlw