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

Result::unwrap() on an Err value: StateAlreadyQueued' #5552

Closed
GetAGripGal opened this issue Aug 2, 2022 · 12 comments
Closed

Result::unwrap() on an Err value: StateAlreadyQueued' #5552

GetAGripGal opened this issue Aug 2, 2022 · 12 comments
Labels
A-ECS Entities, components, systems, and events C-Bug An unexpected or incorrect behavior

Comments

@GetAGripGal
Copy link

GetAGripGal commented Aug 2, 2022

Bevy version

0.8

Relevant system information

OS: Windows 11

What you did

I attempted to set change my gamestate like so:

// Change the state
if *state.current() != event.0 {
    state.set(event.0).unwrap();
}

What went wrong

Doing so crashes the game with the following error:
Result::unwrap()` on an `Err` value: StateAlreadyQueued'

Additional information

The current workaround i took was to use State::overwrite_set instead like so:

// Change the state
if *state.current() != event.0 {
    // Using [`State::overwrite_set`] instead of [`State::set`] to cirvumvent the 'Aleady Scheduled' bug. 
    state.overwrite_set(event.0).unwrap();
}

Edit: To clarify, this system is only being called once per frame and the state isn't currently being changed anywhere else in the code.

@GetAGripGal GetAGripGal added C-Bug An unexpected or incorrect behavior S-Needs-Triage This issue needs to be labelled labels Aug 2, 2022
@alice-i-cecile alice-i-cecile added A-ECS Entities, components, systems, and events and removed S-Needs-Triage This issue needs to be labelled labels Aug 2, 2022
@mockersf
Copy link
Member

mockersf commented Aug 2, 2022

This error means that you already changed the state in this frame. The overwrite_* method is made to ignore ongoing transitions.

Without more context, this seems to work exactly as expected 👍

@klemola
Copy link

klemola commented Aug 4, 2022

@mockersf I'm getting this panic after updating to version 0.8, without any code changes related to states or system sets.

When I print the current app state at the app_state.set call site, the current one is not the one I'm scheduling. Logically I can't think of a way my code could possibly schedule the state twice 🤔

app state ResMut(State { transition: Some(Entering(A, B)), stack: [B], scheduled: None, end_next_loop: false })

Let's call the states A, B, and C by their logical order. I'm trying to transition to state C, and as you can see from above, C can't be found from the current state as scheduled or as the current state.

Edit: The state change is scheduled from a system that runs at SystemSet::on_enter. I also believe that A->B and B->C transitions occur on the same frame/update.

@mockersf
Copy link
Member

mockersf commented Aug 4, 2022

The error is saying there is already a transition ongoing, which you can see by transition not being None.

It doesn't have the best name, it's StateAlreadyQueued as in "there's already a new state queued", not "the state you're trying to set is already queued"

@klemola
Copy link

klemola commented Aug 4, 2022

The error is saying there is already a transition ongoing, which you can see by transition not being None.

It doesn't have the best name, it's StateAlreadyQueued as in "there's already a new state queued", not "the state you're trying to set is already queued"

Ah, I see. Thanks for the clarification! Still, how can the SystemSet::on_enter system of state B run before the transition is completed? Has something changed in 0.8 that caused this panic, as it seems to me?

@mockersf
Copy link
Member

mockersf commented Aug 4, 2022

I think it's a bug that it was accepted before. #4890 is probably the source of your panic.

setting a new state in on_exit was causing issues so controls where added and it seems those controls also impact on_enter

@klemola
Copy link

klemola commented Aug 4, 2022

I think it's a bug that it was accepted before. #4890 is probably the source of your panic.

Yeah! That must be it. I'll see if there's a better way to handle the three states, maybe by making state B redundant.

@thephet
Copy link

thephet commented Aug 5, 2022

I am having a similar problem. Code was fine in 0.7 but now in 0.8 I get the same error.

You can see here the system that handles changing the states (turns in my case). It is only 45 lines:

https://github.com/thephet/BevyRoguelike/blob/main/src/systems/end_turn.rs

@GetAGripGal
Copy link
Author

This error means that you already changed the state in this frame. The overwrite_* method is made to ignore ongoing transitions.

Without more context, this seems to work exactly as expected 👍

I am not changing the state anywhere else in the code and the system is only called once per frame. The code worked fine in 0.7 but broke after 0.8.

@neocturne
Copy link
Contributor

neocturne commented Aug 6, 2022

I'm seeing this error when trying to change the state again in a system running on_enter() on 0.8, which didn't happen on 0.7. When moving the system to on_update(), changing the state again does work on 0.8 (but it is less convenient for my usecase). Using the overwrite_* methods might be a good enough solution though.

@klemola
Copy link

klemola commented Aug 7, 2022

@NeoRaider by @mockersf 's description, it seems that changing a state in on_enter() is not even meant to work. I take it that the transition from the previous state is meant to be active when on_enter() is triggered. I'm not sure if it's very intuitive though.

In general terms, should the transition be finished before a FSM state is entered? In FSM's I've authored, the transition belongs to the "previous" state (the one where the transition begins), and the next state should not need to be aware of the transition (it has a separate set of transitions).

Pseudocode:

let stateA = { id: 'A', transitions: [ 'B' ]' }

let stateB = { id: 'B', transitions: [ 'C' ]' }

let stateC = { id: 'C', transitions: [ 'D', 'A' ]' }

fn transitionTo(from: State, to: State) -> Result<Transition> {
  if to in from.transitions {
    Ok(to)
  } else {
    Err("Not transition from {from} to {to}")
  }
}

let transition = transitionTo(stateA, stateB)

@malachid
Copy link

Using https://github.com/bevyengine/bevy/blob/latest/examples/ecs/state.rs as the inspiration, I find this behavior confusing.

This is enough to reproduce the StateAlreadyQueued

impl Plugin for GameLoopPlugin {
    fn build(&self, app: &mut App) {
        app
            .add_state(GameState::StateA)
            .add_system_set(SystemSet::on_enter(GameState::StateA).with_system(do_state_a))
            .add_system_set(SystemSet::on_enter(GameState::StateB).with_system(do_state_b))
        ;
    }
}

#[derive(Debug, Clone, Eq, PartialEq, Hash)]
enum GameState {
    StateA,
    StateB,
}

fn do_state_a(
    mut state: ResMut<State<GameState>>,
) {
    state.set(StateB).unwrap();
}

fn do_state_b() {
    // no-op
}

Using push instead of set doesn't make any difference.
Using overwrite_set instead does work; but doesn't match the official example in the link above.

btw I am on bevy 0.9.1 on Linux

@alice-i-cecile
Copy link
Member

This error and the entire state stack was removed in #7267.

brianlove added a commit to brianlove/MinesweeperBevy-tutorial-Qongzi that referenced this issue Feb 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ECS Entities, components, systems, and events C-Bug An unexpected or incorrect behavior
Projects
Archived in project
Development

No branches or pull requests

7 participants