Skip to content

Split the launcher into combo/core modules - #191

Draft
Varuuna wants to merge 14 commits into
developfrom
refactor/launcher-decomposition
Draft

Split the launcher into combo/core modules#191
Varuuna wants to merge 14 commits into
developfrom
refactor/launcher-decomposition

Conversation

@Varuuna

@Varuuna Varuuna commented Aug 28, 2026

Copy link
Copy Markdown
Owner

combo/ComboShip.cpp had grown to 3,364 lines holding every cross-cutting concern in the launcher. It is now 567 linesmain() and the boot sequence — with the rest split into 14 modules under combo/core/.

Pure refactor: no behavior change, and no vendored files touched (soh/, mm/, libultraship/, extractors are all untouched), so it costs nothing at upstream-merge time.

What it improves

  • The save container can no longer call a DLL under its lock. ComboContainer.cpp doesn't include ComboDllApi.h, so it can't name an export. The "release the lock before calling into a game DLL" rule previously lived in comments at six sites; 19 raw reach-ins into g_containerMutex / LoadOrCreateContainer / FlushContainer became 7 typed accessors.
  • One fill prologue instead of two kept in step by hand. ComboShip.exe and comborando ran the same setup sequence (goal push, dumps, forced placements, entrance shuffle) as separate copies — and comborando is what validate-seed.yml uses to certify player seeds. They now share core/ComboFillDriver.h, so CI validates seeds through the path that generates them.
  • Transition state has one home. "Which game is in front" had six declaration sites; now core/ComboTransition.
  • Fixed a latent macro clash. Windows.h's min/max only avoided colliding with std::min in the rando headers because of include order inside the single TU. Now handled with NOMINMAX instead of luck.
  • Dropped dead code — 4 exports resolved but never called, plus 2 unused helpers.

Export pointers keep global scope and their exact export names, so ~600 call sites are untouched and each module's diff reads as code motion. scripts/check-export-bindings.ps1 verifies every pointer is declared, resolved, and named after the symbol it resolves from (139/139).

Testing

  • Playtested per step: boot, OOT↔MM round-trips, save/load/copy/erase, resume-into-MM, Ctrl+R and owl-save returns, generation, reload, cross-game items, timers.
  • Seed determinism: comborando seeds 1 / combo-test / 9999 produce byte-identical consolidated spoilers before and after; in-game regeneration of the same input produced identical placements and hash icons.

Build Artifacts

Varuuna added 14 commits August 28, 2026 03:32
DLL load/sym/free helpers, the terminate handler and the late-crash filter move out
of ComboShip.cpp unchanged. First step of splitting the launcher into modules.
…hip.cpp

core/ComboDllApi: all 143 export pointers plus the two resolve entry points. Pointers keep
global scope and their exact export names, so call sites are untouched and the resolve text
moves byte-identical. scripts/check-export-bindings.ps1 verifies decl/resolve/name pairing.

core/ComboSeedMath: ComboHash + ResolveStartingGameMM, previously duplicated in
ComboRandoHeadless.cpp and kept in sync by hand. Both char*/std::string overloads are
provided so no call site changes.

The soh export gate now runs after the full resolve instead of after three symbols.
…typed accessors

The .combosav load/flush/cache and all per-slot IO move out of ComboShip.cpp. The launcher
no longer touches g_containerMutex, LoadOrCreateContainer or FlushContainer (19 call sites);
it goes through 7 typed accessors that lock, touch JSON and return by value.

ComboContainer.cpp does not include ComboDllApi.h, so it cannot name a DLL export and cannot
call one under the lock - the rule the old code kept by convention. Erase/copy keep their
notification tails, now in launcher-side wrappers of the same name.
…d-math tests

core/ComboSeedFile: consolidated-seed discovery and validation.
core/ComboBootstrap: archive/config existence plus the JSON helpers the settings import uses.
Neither touches a DLL export, so both are linkable outside the launcher.

combo_seed_math_tests pins ComboHash and ResolveStartingGameMM to literal values, including
the Random starting-game derivation string that ComboShip.exe and comborando must agree on.
The socket, receive thread and roster move out of ComboShip.cpp. sActiveGame and
sResyncPending stay private to the module; the three callers that reached into them
now go through ActiveGame() and TakeResyncPending().

SDL_net moves with it - the launcher no longer includes SDL at all.
…s into core modules

core/ComboCrossItems, core/ComboGoal, core/ComboHintReveal. The forward declaration
DeliverCrossItem needed for Combo_OnTriforceProgress is gone - the header breaks the cycle.

ComboPlatform.h now defines NOMINMAX: Windows.h used to be included after the rando
headers in the single TU, so its min/max macros never reached std::min there.
…core modules

core/ComboSeedState carries the worker->main handoff globals with the barrier contract written
down: g_ComboPendingFinalize's seq_cst store/exchange IS the synchronisation, and
g_ComboProgress's address is live in soh.dll for the process lifetime.

core/ComboGeneration takes RunComboFill whole and untouched, plus the gen-test, playthrough and
finalize paths. core/ComboReload takes Combo_OnReloadRequest.

ComboShip.cpp is now main() plus the save/transition callbacks.
…dules

core/ComboTransition consolidates foreground/handoff state - g_PendingMMFileNum,
g_pendingOOTReturn, g_mmReturnKind, g_MmSaveInMemorySlot - which had six declaration sites for
one concept, plus SetForegroundGame, the erase/copy notify wrappers and the return callbacks.

core/ComboSlotBind takes the new-file and load-file callbacks; its forward declaration of
Combo_ResumeMMIfLastSavedThere is now the transition header.

ComboShip.cpp is main() and the DLL handle.
16 tests over ComboContainer: per-game section preservation, sentinel slots, corrupt-file
set-aside, notes round-trip and debounce, goal/completion, hintsRead set semantics including
matchField, and the release gate (patch difference keeps saves, minor difference evicts once).

The container links into lus_tests at all only because it names no DLL export.
ComboShip.exe generates seeds and comborando certifies them (validate-seed.yml), so the two ran
the same setup sequence kept in step by hand. core/ComboFillDriver.h is that sequence once;
both build the same hook table from their own resolved exports.

Scoped to those two on purpose. RunComboGenTest and RunComboPlaythrough skip the goal push and
the Triforce pool check, so folding them in would change what they exercise.

comborando now also reports an empty static-data dump instead of continuing with one.

Verified: seeds 1 / combo-test / 9999 produce byte-identical consolidated spoilers before and
after.
SOH_Extract, MM_Extract, MM_ArchiveCount and SOH_SetOnComboGenerateCallback were resolved and
never called; MMArchivesExist and ComboUI::IsGameActive were defined and never called. The only
remaining mentions of the extract exports were a stale boot-flow comment, corrected here.

Export bindings: 139 declared, 139 resolved.
They found no bugs, and lus_tests is in no CI workflow - two of its existing tests have been
failing on develop unnoticed, which is the argument against carrying more. Coverage here comes
from playtesting, code review, validate-seed.yml and the spoiler determinism diff.

libultraship/tests is back to its develop state.
Export surface now lives in combo/core/ComboDllApi; the boot list described extraction as running
SOH_Extract/MM_Extract, which the launcher no longer resolves - it goes through comboui's
extraction screen.
@Varuuna
Varuuna marked this pull request as draft August 29, 2026 22:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant