Stage 6: split the Game god class (EngineRuntime / SceneLoader / FrameLoop)#107
Open
mblackman wants to merge 3 commits into
Open
Stage 6: split the Game god class (EngineRuntime / SceneLoader / FrameLoop)#107mblackman wants to merge 3 commits into
mblackman wants to merge 3 commits into
Conversation
First of three Game-split extractions. EngineRuntime owns the platform lifecycle Game::Initialize/Destroy used to inline: SDL + TTF subsystem init, the window/renderer pair, the ImGui backend, and ordered teardown (ImGui shutdown -> destroy renderer/window -> SDL_Quit). Game holds an EngineRuntime member and reaches the SDL handles through runtime_.Window() / runtime_.SdlRenderer(). What deliberately stays in Game::Initialize: GameConfig loading, project path resolution, editor-persistence wiring, window-title/size derivation, the editor font/style/layout pass, and EngineContext population. Those are bootstrap concerns, not runtime plumbing; EngineRuntime takes the title/size Game computed and the imgui ini path, nothing project-aware. Bake is unaffected: it never constructs a window, so the default- constructed EngineRuntime stays idle and GetRenderer() returns nullptr exactly as the old null sdl_renderer_ did. Verified on Linux: editor-release + player-release (no-IMGUI path) build clean; ctest 8/8; headless bake exits 0.
Second of three Game-split extractions. SceneLoader owns LoadScene / ReloadScene / StopScene / TrackSceneAssets / clearSceneEntities plus the scene state Game used to hold (scene_running_, current_scene_assets_). It reads the live SDL renderer + mixer off the registry's EngineContext, so it depends only on Registry + the shared sol::state. Game holds a SceneLoader member (constructed once registry_ exists) and its LuaBindingContext scene-op overrides now delegate straight through, so the `scene.*` Lua module and the editor toolbar drive it via Game unchanged. The ~155-line LoadScene body (table / function / side-effect forms, acquire-before-release) moves verbatim; sdl_renderer_ becomes EngineContext.sdlRenderer. ReadFileViaSDL — shared by Game::LoadGame and SceneLoader — moves to a header-only Engine/SdlFileReader.h instead of living in Game.cpp's anonymous namespace. Game.cpp: 1151 -> 888 lines. Verified on Linux: editor-release + player-release build clean; ctest 8/8; headless bake exits 0.
Final Game-split extraction. FrameLoop owns the four phases Game::Run
drove inline — ProcessInput / WaitTime / Update / Render — plus the
KeyInputEvent handler (Esc -> quit, backtick -> toggle debug GUI) and the
debug-collider query (built once in its ctor, not per frame). Game::Run
keeps the while(running) loop and seeds the first frame via
frame_loop_->Begin().
FrameLoop holds non-owning pointers to the pieces it touches (registry,
event bus, renderer, EngineRuntime) plus the Game it renders the editor
overlay for (RenderDebugGUISystem::Render needs the Game facade). The
running flag stays a Game static; ProcessInput/OnKeyInputEvent call
Game::Quit(). milliseconds_previous_frame_ moves into FrameLoop. Profile
scope names are kept verbatim ("Game::Update", ...) so bench-log parsing
is unaffected. The KeyInputEvent subscription rebinds from Game to the
FrameLoop instance.
FrameLoop.h forward-declares KeyInputEvent rather than including its
header, which is not self-contained (derives from Event without including
EventBus.h); the .cpp includes it after EventBus.h.
Game.cpp: 888 -> 668 lines (1151 across the full Stage 6 split). The
remaining mass is Initialize (config/project bootstrap), Setup (the
bootstrap-spine composition), and the headless bake — Game's own
lifecycle/composition role.
NOTE: the frame loop is not exercised by headless CI (ctest + bake run
no window/loop), so this is a faithful relocation verified by compile +
the existing suites; a manual windowed run is the real check.
Verified on Linux: editor-release + player-release build clean; ctest
8/8; headless bake exits 0.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Stage 6 of the architecture-improvements plan.
Game.cppwas a 1151-line god class mixing SDL/ImGui lifecycle, scene loading, the per-frame loop, bootstrap composition, and the headless bake. This splits the first three concerns into focused classes;Gamestays the coordinator that owns them.Three commits, one extraction each (each builds + tests green on its own):
EngineRuntimeSceneLoaderLoadScene/ReloadScene/StopScene/TrackSceneAssets+ scene state; reads the SDL renderer/mixer offEngineContextFrameLoopProcessInput/WaitTime/Update/Render+ the key handler + debug-collider queryGameholds the three as members and delegates: itsLuaBindingContextscene overrides forward toSceneLoader(so thescene.*Lua module + editor toolbar are unchanged), andGame::Runkeeps thewhile(running)loop drivingFrameLoop.Game.cpp: 1151 → 668 lines. The remainder isInitialize(config/project bootstrap),Setup(the bootstrap-spine composition), and the headless bake —Game's legitimate lifecycle/composition role. (The plan's <250 aspiration isn't reached by these three extractions alone; the dominant remaining mass is composition, not the extracted concerns.)What deliberately stayed in
GameConfig loading, project-path resolution, editor-persistence wiring, the
EngineBootstrapspine inSetup, andRunBakeValidation. These are composition/lifecycle, not the runtime/scene/frame plumbing the new classes capture.Test plan
editor-release+player-releasebuild cleanctest8/8 (LuaApi + AssetPipeline + Process + ProjectIni + ECS/EventBus/system suites)--startup-mode bakeexits 0ProcessInput/Update/Render) is not exercised by headless CI (no window/loop). 6c is a faithful relocation verified by compile + suites, but a real windowed run (editor + player) is the actual check and I can't do it in this headless container.