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

Unexpected SystemInMultipleBaseSets panic due to on_update #7632

Closed
NiklasEi opened this issue Feb 11, 2023 · 1 comment
Closed

Unexpected SystemInMultipleBaseSets panic due to on_update #7632

NiklasEi opened this issue Feb 11, 2023 · 1 comment
Labels
A-ECS Entities, components, systems, and events C-Bug An unexpected or incorrect behavior C-Usability A targeted quality-of-life change that makes Bevy easier to use
Milestone
0.10

Comments

@NiklasEi
Copy link
Member

Bevy version

main branch a1e4114

What you did

I am trying to order an exclusive system in a pretty standard way for Plugins using states:

  1. The system should run early in every frame (before any "normal" user systems)
  2. The system should only run in the given state
            .add_system(
                exclusive_system
                    .in_base_set(CoreSet::PreUpdate)
                    .in_set(MySet::Label)
                    .on_update(MyStates::State)

What went wrong

This triggers the unexpected panic SystemInMultipleBaseSets { system: "Some(System exclusive_system: {is_exclusive})", first_set: "StateTransitions", second_set: "PreUpdate" }'

Additional information

  • The fact that this is an exclusive system seems irrelevant; the same panic happens with a normal system
  • replacing on_update with run_if(state_equals(MyStates::State)) removes the panic, but is the exact behaviour I would have expected from on_update
@NiklasEi NiklasEi added C-Bug An unexpected or incorrect behavior S-Needs-Triage This issue needs to be labelled labels Feb 11, 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 and removed S-Needs-Triage This issue needs to be labelled labels Feb 11, 2023
@alice-i-cecile alice-i-cecile added this to the 0.10 milestone Feb 11, 2023
@NiklasEi NiklasEi changed the title 0.10 unexpected SystemInMultipleBaseSets panic due to on_update Unexpected SystemInMultipleBaseSets panic due to on_update Feb 11, 2023
@alice-i-cecile alice-i-cecile mentioned this issue Feb 11, 2023
Closed
@lewiszlw
Copy link
Member

Agree. Current behaviour is weird, we can't choose to put in which base set when using .on_update(MyStates::State).

@NiklasEi NiklasEi mentioned this issue Feb 13, 2023
Closed
@alice-i-cecile alice-i-cecile mentioned this issue Feb 13, 2023
Closed
@bors bors bot closed this as completed in 0a9c469 Feb 14, 2023
myreprise1 pushed a commit to myreprise1/bevy that referenced this issue Feb 15, 2023
@alice-i-cecile @myreprise1
Remove .on_update method to improve API consistency and clarity (bevy…
2411c72 
…engine#7667)

# Objective

Fixes bevyengine#7632.

As discussed in bevyengine#7634, it can be quite challenging for users to intuit the mental model of how states now work.

## Solution

Rather than change the behavior of the `OnUpdate` system set, instead work on making sure it's easy to understand what's going on.

Two things have been done:

1. Remove the `.on_update` method from our bevy of system building traits. This was special-cased and made states feel much more magical than they need to.
2. Improve the docs for the `OnUpdate` system set.
myreprise1 pushed a commit to myreprise1/bevy that referenced this issue Feb 15, 2023
@alice-i-cecile @myreprise1
Remove .on_update method to improve API consistency and clarity (bevy…
20e3c3e 
…engine#7667)

# Objective

Fixes bevyengine#7632.

As discussed in bevyengine#7634, it can be quite challenging for users to intuit the mental model of how states now work.

## Solution

Rather than change the behavior of the `OnUpdate` system set, instead work on making sure it's easy to understand what's going on.

Two things have been done:

1. Remove the `.on_update` method from our bevy of system building traits. This was special-cased and made states feel much more magical than they need to.
2. Improve the docs for the `OnUpdate` system set.
myreprise1 pushed a commit to myreprise1/bevy that referenced this issue Feb 15, 2023
@alice-i-cecile @myreprise1
Remove .on_update method to improve API consistency and clarity (bevy…
a2adf4f 
…engine#7667)

# Objective

Fixes bevyengine#7632.

As discussed in bevyengine#7634, it can be quite challenging for users to intuit the mental model of how states now work.

## Solution

Rather than change the behavior of the `OnUpdate` system set, instead work on making sure it's easy to understand what's going on.

Two things have been done:

1. Remove the `.on_update` method from our bevy of system building traits. This was special-cased and made states feel much more magical than they need to.
2. Improve the docs for the `OnUpdate` system set.
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-Bug An unexpected or incorrect behavior C-Usability A targeted quality-of-life change that makes Bevy easier to use
Projects
None yet
Milestone
0.10
3 participants
@alice-i-cecile @NiklasEi @lewiszlw