rfcs icon indicating copy to clipboard operation
rfcs copied to clipboard

Shutdown and cleanup missing behavior

Open AnneKitsune opened this issue 7 years ago • 11 comments

Currently, when you cleanly quit the game, there is no clean-up actions that can be ran (disconnecting from a server, saving something to a file, etc...)

There should be a way that the engine call the code from the user when the game is shutting down.

There is a method in Application::shutdown which is unused and could be re-used for that purpose.

There are two places (or more?) where exit code could be placed: State and System.

State is annoying, because you will probably end up copy pasting the shutdown method between your states.

System on the other hand would be a bit better since you could have a shutdown() method added to them. However, usually you will want your systems to be stateless, so having a cleanup method isn't really idea here either.

An rfc would need to be opened for discussion by the person taking this issue in charge. Thanks!

AnneKitsune avatar Nov 10 '18 05:11 AnneKitsune

What about providing ways to do the shutdown in different places for different things?

I don't really get the shutdown method for a system, since its stateless, however, for example when a state is destroyed, it should clean up its resources. There might be more states alive, though. Then, what happens if all states are destroyed? It's on a different level than systems, too, and it probably should be provided in a central place, like the application builder which also initializes the game.

minecrawler avatar Nov 12 '18 00:11 minecrawler

What I meant is that having shutdown() on a system doesn't make sense, so you can skip that one when designing how to make a shutdown procedure.

AnneKitsune avatar Nov 12 '18 01:11 AnneKitsune

I don't really get the shutdown method for a system, since its stateless, however, for example when a state is destroyed, it should clean up its resources.

I think it is handled by pair on_start/on_stop?

I think it would be good for application level to have some sorta callback, but for that it would be better to Application or to be precise CoreApplication would become trait itself. Or as alternative you could have trait parameter that could describe set of callbacks: for example application start and application shutdown as set of static methods that can accept World

pub trait ApplicationHook {
    ///Runs when application starts
    fn on_start(_world: &mut World) {
    }

    ///Runs before application is going to be shutdown
    fn on_shutdown(_world: &mut World) {
    }
}

pub struct DefaultAppHook;
impl trait ApplicationHook for DefaultAppHook {}

It would require new trait parameter and changing Application alias to Application<H: ApplicationHook while providing type DefaultApplication = Application<DefaultAppHook>;

DoumanAsh avatar Nov 12 '18 06:11 DoumanAsh

on_stop is called on all pushed states when the state machine stops, so that's one way. In addition to that adding what @DoumanAsh suggest above should catch all other scenarios tbh. Then again, you can do exactly the same thing simply by pushing a baseline state at the bottom of the stack on startup.

Rhuagh avatar Nov 12 '18 07:11 Rhuagh

@Rhuagh Does amethyst::Trans::Quit triggers on_stop for all State in stack?

DoumanAsh avatar Nov 12 '18 07:11 DoumanAsh

Yes.

            while let Some(mut state) = self.state_stack.pop() {
                state.on_stop(StateData { world, data });
            }

Rhuagh avatar Nov 12 '18 07:11 Rhuagh

Hmm, this is certainly on option instead of making global shutdown. Though I feel like it is not the best option for something global, but as currently there is always only single initial state, I think it is nice alternative to adding anything new.

DoumanAsh avatar Nov 12 '18 07:11 DoumanAsh

Ah so I guess this issue is already resolved :D I thought on_stop wasn't called for all states for some reason.

AnneKitsune avatar Nov 12 '18 14:11 AnneKitsune

is 'all_pushed' always the same as 'all_states'?

SkylineP1g3on avatar Jan 07 '19 22:01 SkylineP1g3on

This issue could be repurposed into a documentation issue

khionu avatar Jan 07 '19 22:01 khionu

Moving this to the RFC repo.

fhaynes avatar Jan 08 '19 02:01 fhaynes