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

return a configuration when scheduling a state changes to break out of loop #2155

Closed
wants to merge 8 commits into from

Conversation

mockersf
Copy link
Member

@mockersf mockersf commented May 13, 2021

fixes #1700

State changes have an extra parameter rerun that will keep the current behaviour when true, but exit the current in-frame loop when false, allowing state changes to happen only once per trigger in some case.

I'm not very happy about the rerun name or the doc for it, please help!

All methods scheduling a state change (set, replace, ...) now returns a StateTransitionConfig that can change what happens after the state change:

state.set(AppState::Game).unwrap().end_frame();

@mockersf mockersf changed the title add parameter on state changes to break out of loop return a builder when scheduling a state changes to break out of loop May 13, 2021
@mockersf mockersf changed the title return a builder when scheduling a state changes to break out of loop return a configuration when scheduling a state changes to break out of loop May 13, 2021
@mockersf mockersf added C-Bug An unexpected or incorrect behavior A-ECS Entities, components, systems, and events labels May 13, 2021
@msvbg
Copy link
Contributor

msvbg commented May 13, 2021

I see you just made some changes, but allow me to bikeshed on the API some more!

  1. I think the builder-style pattern here encourages unwrapping, as shown in the example code: state.set(AppState::Game).unwrap().next(Next::NextFrame);.
  2. Since this is just a slightly different version of set, can it simply be another function? e.g. set_next_frame. It feels a little bit unnecessary to have a whole config type when there's just one other way of doing it.

@mockersf
Copy link
Member Author

mockersf commented May 14, 2021

I think the builder-style pattern here encourages unwrapping

I don't really have an issue with unwrap in application code. Also given why you would need this, your game is already crashing 😄
I removed the unwrap in the doc example

Since this is just a slightly different version of set

There are already 8 different methods, adding a variation for each would bump that to 16...

@msvbg
Copy link
Contributor

msvbg commented May 14, 2021

There are already 8 different methods, adding a variation for each would bump that to 16...

Ah, I didn't read the PR properly. Yeah then it makes a lot more sense.

@NathanSWard
Copy link
Contributor

I'm still not 100% sure I like the name Next for the enum, however I can't seem to find a better name.
So if other people are ok with the name, I'm happy to approve this 🙂

@cart cart added the S-Pre-Relicense This PR was made before Bevy added the Apache license. Cannot be merged or used for other work label Jul 23, 2021
@mockersf mockersf removed the S-Pre-Relicense This PR was made before Bevy added the Apache license. Cannot be merged or used for other work label Jul 24, 2021
@mockersf
Copy link
Member Author

probably won't be relevant anymore soon

@mockersf mockersf closed this Apr 25, 2022
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
None yet
Development

Successfully merging this pull request may close these issues.

triggering state transition from player input locks the game
5 participants