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

Replace startup systems with a built-in state enum that handles resource cleanup #5437

Open
alice-i-cecile opened this issue Jul 23, 2022 · 6 comments
Labels
A-App Bevy apps and plugins A-Assets Load files from disk to use for things like images, models, and sounds A-ECS Entities, components, systems, and events C-Feature A new feature, making something new possible C-Usability A targeted quality-of-life change that makes Bevy easier to use X-Controversial There is active debate or serious implications around merging this PR

Comments

@alice-i-cecile
Copy link
Member

alice-i-cecile commented Jul 23, 2022

Problem

  1. Startup systems are largely special-cased, they rely on custom stage implementations and it's quite hard to teach the model.
  2. It is challenging to ensure appropriate cleanup of resources on AppExit: we must listen for the event and carefully time the resources.

Proposed solution

/// A built-in game state that controls app control flow
///
/// This is enabled by default, and user systems are added to the `Main` variant by default
enum AppState {
    Startup,
    Main,
    Exit,
}

This allows us to use standard on-enter, on-exit and state transition methods to control things like asset loading, cleanup, pausing the game, autosaving on exit and so on.

Extensions

We could add an explicit asset loading state as well. This may work well, but could be controversial.

Context

Discussed with @IceSentry on Discord after trying to help a user ensure data is cleaned up correctly.

This builds on but does not require the patterns espoused by the [Stageless RFC}(https://github.com/bevyengine/rfcs/pull/45).

Would address #1353. If we were to implement this, we would likely want to have a more robust event strategy by default, to ensure that gameplay events do not get lost during a pause.

@alice-i-cecile alice-i-cecile added C-Feature A new feature, making something new possible A-ECS Entities, components, systems, and events A-Assets Load files from disk to use for things like images, models, and sounds C-Usability A targeted quality-of-life change that makes Bevy easier to use A-App Bevy apps and plugins X-Controversial There is active debate or serious implications around merging this PR labels Jul 23, 2022
@mockersf
Copy link
Member

Anything that's part of an Exit state can't be reliable.

The game will panic, crash, the user will force quit it.

The most reliable thing we have from Rust is to implement Drop on everything that needs a custom cleanup.

@alice-i-cecile
Copy link
Member Author

Mhmm 🤔 Could we use a drop impl to attempt auto-saving? I suppose if you could somehow have a custom drop impl on World that might work, but a) orphan rules and b) you won't have a good way to get disk access...

@Nilirad
Copy link
Contributor

Nilirad commented Jul 24, 2022

Bikeshedding: there is no concept of “Game” in Bevy, so it should be named something like AppState.

I think adding a Paused state by default is not something we would want: pausing is such a high level feature that games should manage by their own. Also it is not pertinent about the other more “lifecycle” variants.

@alice-i-cecile
Copy link
Member Author

Bikeshedding: there is no concept of “Game” in Bevy, so it should be named something like AppState.

Agreed.

I think adding a Paused state by default is not something we would want: pausing is such a high level feature that games should manage by their own.

Part of my concern here is that I want app.add_system to do the right thing by default. However, I think you're probably right; we can cut this to reduce complexity and enable more flexibility.

@Weibye
Copy link
Contributor

Weibye commented Jul 29, 2022

Could we also get a Build state? Ref #5016

(Expecting to this to not be trivial)

@Nilirad
Copy link
Contributor

Nilirad commented Jul 29, 2022

@Weibye Plugins are built during App creation, so I don't think it's something that can be defined with states.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-App Bevy apps and plugins A-Assets Load files from disk to use for things like images, models, and sounds A-ECS Entities, components, systems, and events C-Feature A new feature, making something new possible C-Usability A targeted quality-of-life change that makes Bevy easier to use X-Controversial There is active debate or serious implications around merging this PR
Projects
Status: Needs Implementation
Development

No branches or pull requests

4 participants