Engine Initialisation and Lifecycle improvement

[immortius]

This issue will deal with a restructure of the central engine initialization and lifecycle. The goal of this restructure is:

• Assist in the removal of CoreRegistry
• Fix a variety of interdependency and ordering issues, such as the dependencies between serializers and systems that may not exist until later.
• Centralize core lifecycle concerns such as the ModuleEnvironment, AssetManager and EntitySystem lifecycles.
• Move concerns to Subsystems where appropriate.
• Expand on the subsystem lifecycle events, and ensure each event has a well defined meaning and usage.

Some additional possibilities:

• Handle the main menu and ingame states more generically. May move main menu into a module, and have the game start with a default module set. Starting a game is just a transition to a different module set. Loading a game is a transition to a different module set, but also providing a save state. So essentially all major state changes involve a module set change and optionally a save state.
• Removal of the State classes.

[immortius]

Firstly the GameEngine needs some cleanup. It has a number of odd methods that it really shouldn't in it. Additionally, there is an obvious need for monitoring the status of engine from outside (currently the Terasology engine has a hardcoded linkage to the splashscreen which is questionable.

Something like this would satisfy external needs.

public interface GameEngine extends AutoCloseable {
    /**
     * Registers a Subsystem
     * @throws IllegalStateException if the engine is running or disposed
     */
    void register(EngineSubsystem subsystem);

    /**
     * The current status of the engine (e.g. loading config, updating asset system, etc)
     */
    EngineStatus getStatus();

    /**
     * Subscribe for notification of engine status changes.
     */
    void register(EngineStatusSubscriber subscriber);

    /**
     * Runs the engine, which will block the thread.
     * Invalid for a disposed engine
     */
    void run(Set<Name> initialModules);

    /**
     * Request the engine to stop running
     */
    void shutdown();

    /**
     * Cleans up the engine. Can only be called after shutdown.
     * This method should not throw exceptions.
     */
    void close();

    /**
     * @return Whether the engine is running
     */
    boolean isRunning();

    /**
     * @return Whether the engine is shutting down
     */
    boolean isShuttingDown();

    /**
     * @return Whether the engine has been disposed
     */
    boolean isDisposed();
}

Internally I think the only methods needed are to trigger environment switch, and to access the current context:

    Context getCurrentContext();

    void switchEnvironment(Set<Name> modules);

Loading games, hosting/joining games, starting solo games and loading the menu would all sit above this.

[emanuele3d]

Some number issues, for easy replying:

1. So, current GameStates would become "environment switches" and it would then be up to the modules to organize their own startup/shutdown sequence and deal with their own dependencies, right?
2. "Statuses" are instead more fine-grained, engine-specific operations, i.e. what is currently done during the initialization phase and perhaps other things happening on disposal? In this context I'm wondering why bother exposing status-related functionality at all? Is it because some modules would now be loaded much earlier, i.e. modules providing serializers?
3. Also, I kind of hate that the AutoCloseable interface imposes a close() method when what we'd really want to have is a dispose(). Should we perhaps add that too then? It would follow the pattern used elsewhere in Terasology's codebase, it would have the javadoc proposed for close() but would perhaps be allowed to throw exceptions. We could then change the close() method javadoc to "Required by the AutoCloseable interface, will probably just call dispose() but must not throw exceptions."
4. I like the idea of registering subsystems instead of statically providing them on construction by the way. This theoretically allows for registration anytime at runtime though. Is that intended?
5. Finally: why are there subsystems at all? Could they be made into "simple" modules?

[immortius]

1. No. It would be up to engine subsystems to deal with what environment switches mean to them. Modules would be unaffected - they should all be working through component systems / asset types / other objects with their own defined lifecycles. My thought is that some things which are currently hardcoded into the engine (main menu mainly) could be handled through these mechanisms instead.
2. I envision Status as basically being an enum-like thing (with some space for extension). They exist to provide external code a sense of what the engine is doing. They don't actually do anything though.
3. close() is allowed to throw exceptions. I think that quibbling over the exact name of the method is not worth it. close() in Java is used for disposal, so it shouldn't be confusing or anything.
4. As the javadoc says, registering a subsystem when the engine is running will provoke an IllegalStateException. I'm not sure whether I'll actually have this method though - setting them in the constructor or through a builder is probably cleaner.
5. No. Subsystems are still a core part of the engine itself - they slot into its main lifecycle and provide a wide range of functionality with high security requirements. They exist for the lifetime of the engine, and have state across any given module environment. Modules are more transient - it is not appropriate for modules to provide this sort of functionality. Not to mention module support itself either comes from the engine or a subsystem in the first place.

[emanuele3d]

1. Ok, I don't know enough yet about module lifecycle and the lifecycle of the systems/assets/object provided by module so I'll just leave it at that.
2. From the perspective of the code interacting with the proposed GameEngine interface and disregarding game states as they do not seem to be part of this, the engine has effectively two sets of statuses/states: one that can be obtained via getStatus()/EngineStatusSubscriber, the other via the is*() methods. Never mind the latter are not explicitly labelled as returning a status: if the engine isRunning, from the perspective of a piece of external code that's -also- the state/status of the engine, alongside the result of getStatus(). I find this potentially confusing and I wonder if it can be clarified or handled differently. I.e. can we think to the status that can be ascertained through the isRunning/isShuttingDown/isDisposed methods as a more coarse-grained status of the engine compared to the fine-grained status returned by getStatus()?
3. You mention close() is allowed to throw exceptions while the javadoc says it should not. I can see how the term "should" does not imply it cannot, but it's a bit of a fine point that could be made more explicit. Nevertheless, I'll drop the issue.
4. I had missed the exception thrown if registering a subsystem while the engine is running/disposed. Which seems to leave the door open to registering a subsystem while the engine is shutting down? And also this seems to hint to a fourth "uninitialized" state, as in the current implementation. Disregarding the discussion in point 2, wouldn't this warrant an isUninitialized() method? Or do you want the external code registering subsystems realize that's not the right time simply by handling the exception? Given what you say though, it does look like there is no need for a registerSubsystem() method and working through the constructor as is currently done is probably good enough.
5. Thank you for clarifying this.

[immortius]

1. I.e. can we think to the status that can be ascertained through the isRunning/isShuttingDown/isDisposed methods as a more coarse-grained status of the engine compared to the fine-grained status returned by getStatus()?

Yes. I could rename the method to getRunningStatus() perhaps.

(3) I don't know where you looking, but the definition of close() in the Autoclosable interface explicitly throws Exception.

(4) Shutting down is still running. Running only is false after shutting down is complete.

I think I will go with using a builder for setting subsystems and remove the register method.

[Cervator]

Linking this to #1797 although I assume that is just one step and doesn't complete this yet.

[immortius]

Very not ready, but you can get a feel for the direction I'm heading from 90c7579d71359ff596293b20cffe1ec0cf745601

A lot of the initialization logic currently in states and their load processes would be moved into subsystems, and occur in response to well defined engine lifecycle events. This would result in logic for managing a particular aspect being self-contained, and ensure they are treated the same way in both main menu and in game.

This is already solving some initialisation ordering issues which @flo highlighted (e.g. populating component libraries between environment creation and asset environment switching).

The down side is that it can make the order of initialisation between subsystems unclear, especially where two subsystems need to react to the same lifecycle event and one needs to be after the other. For the moment I am seeing whether this will be an issue, and if it is considering whether additional lifecycle events would make sense and solve things. Otherwise I may add some method annotations for providing execution order, though that is a last resort.

Currently there are lifecycle events for main engine initialisation, environment switching, main loop update, and shutdown. Entity system initialisation/loading and network related lifecycle events are also probable.

For environment switching there is a changeModuleEnvironment() method in engine, but I'm considering having something more along the lines of createGame()/loadGame()/joinGame(), with networking and saved game options built in. The main menu would also be a game, just with a specific module.

The branch is currently somewhat non-functional. I'm pretty sure you cannot actually start a game, although the menu works at least. Much work to go.

[Cervator]

Very cool :-) Thanks for the update.

Ping: @MarcinSc @Josharias @emanuele3d @msteiger @skaldarnar

[Cervator]

Curiously, @immortius, is this paired with #1234 (entity system overhaul) or its own thing? Wondering if this should be a v2.0.0 item.