llamamod/notes/dev/bug-review-launcher.md

Launcher and mod pipeline – bug review (2026-11-07, agent "lxbugs")

Scope: Main (CLI), ui, platform (Installer, IsoImage, GameSource, Integrity, GameLauncher, ExePatcher, Config, FileOps, RegFile), save, mods (ModManager, ModRegistry, options, cache, world-9 fixup, ApplyLog), exe, all mods.*. Method: code review, decompile checks (re/groove_decomp.c), programmatic combination tests against the real groove.exe, ISO images built with genisoimage, in-game smoke tests with tools/apptest.py (display :8).

New tests

Test	What it does
mods/ExeModCombinationsTest	all 31 combinations of the exe mods (intro-skip, cheats, practice, freecam, boot-to-level) × {defaults, every exe option on}: apply against the real groove.exe, re-parse the result (ExeImage.parse), sections sorted and non-overlapping (VA and raw), SizeOfImage covers the last section, objdump -h accepts the all-mods exe and lists .llamac/.llamad, restore gives the HKCU base exe, re-apply is 100 % cache hits and byte-identical (journal replay)
platform/IsoVariantsTest	ISOs built with genisoimage: Joliet (like the CD), Rock Ridge only, plain ISO 9660 → install + restore → lower-case game/groove.exe with the HKCU patch, checkGameExe OK, save game visible to the save editor, long names kept (Joliet/RR); truncated ISO, text file, all-zero file, ISO without game data → clear IOExceptions; setup again from a changed source / resume after cancel
mods/ModManagerConcurrencyTest	second instance applying/clearing the cache while the first applies → clean error, first finishes; clear cache refused while the game runs; exe written by another instance is not reported as "changed by antivirus"
save/SaveStoreTest	three saves within one second keep three backups; (null)kingN.ksv migration (empty slot, used slot, wrong size), corrupt slot, import of a wrong-size file, read-only slot file
MainCliTest	--install without/with two arguments, --apply/--launch/--unlock before setup, --unlock with missing/non-numeric/out-of-range slot, world, chapter (no file written), valid world-9 chapter 49
platform/GameLauncherPathsTest	ANSI code page / length check of game paths; Wine data drive L: used for CDPath, exe and registry file
platform/ConfigTest	truncated config.json with an existing installation → still installed, copy kept as config.json.broken; wrong value types
mods/levels/LevelProjectModTest.versionFollowsTheFileWhenItIsSavedAgain	cache key changes when the project file is saved again

Fixed bugs

#	Severity	Bug / repro	Fix	Test
L1	high	Intro skip + Boot to level could not be combined (both default to "skip logos"): apply failed with "intro-skip (skip logo movies and images) (0x0041D56E, 3 bytes) overlaps "skip logo movies" of boot-to-level". Hit every editor play-test (it forces boot-to-level logos=true) for users with Intro skip enabled – 16 of 62 exe-mod combinations failed	IntroSkipMod: skip its logo patch when the identical patch is already there (boot-to-level runs first, order 90 < 100)	ExeModCombinationsTest
L2	medium	ISO without Joliet (Rock Ridge only, e.g. genisoimage -R, or plain ISO 9660): names were the upper-case 8.3 names (GROOVE.EXE, ARGOLOGO.DAT instead of argologompeg.dat); on Linux baseExe did not find original/hd/groove.exe → no exe patch, exe mods failed ("NoSuchFile"), game/groove.exe missing → Play impossible; save games as KING0.KSV next to the editor's king0.ksv	IsoImage: Rock Ridge NM names when there is no Joliet; Installer.setupGameDir: all-upper-case HD names are copied lower-case; ModManager.baseExe finds the original exe case-insensitively	IsoVariantsTest
L3	medium	No lock between instances (second window, CLI --apply, standalone editor play-test): both rebuild cd-active.new, cache/<mod>/<key>.tmp and game/groove.exe → deleted half-built folders, mixed results. The first window's config kept the old exe hash → after the other instance applied, Play showed "groove.exe was removed or changed – probably by antivirus software"	ModManager.apply/clearCache hold an exclusive FileLock on <data>/.apply.lock ("Another LlamaMod window or command is applying mods right now…"); checkGameExe accepts (and adopts) the exe hash stored on disk by another instance	ModManagerConcurrencyTest
L4	medium (data loss)	Save backups are folders named by the second; a second save of the same slot within that second overwrote the backup of the original state (REPLACE_EXISTING)	SaveStore.backupDir: <ts>-2, -3, … when the file already exists	SaveStoreTest
L5	medium	Level projects: LevelProjectMod.version() was the hash of the file when the mod list was built. Saving the project again (editor) while the launcher was open → Apply used the old cache key → the old build came back from the cache. The launcher also never rebuilt its mod list (AppState.registry was final): projects created in the editor appeared only after a restart	LevelProjectMod.version() re-hashes when size/mtime changed; AppState.reloadMods() on every MainFrame.refreshAll() (editor "mods changed", wizard, theme)	versionFollowsTheFileWhenItIsSavedAgain
L6	medium	Setup again (Settings → Run setup again, --install) skipped every file whose size matched the old copy ("resume"), so a repaired or different ISO did not replace damaged same-size files; files not in the new source stayed in original/ (and in cd-active); the mod cache (keyed by names + sizes) served outputs of the old data	Installer: resume only for an interrupted install of the same source (marker original/.install-source), otherwise copy everything, delete stale files, clear the cache	IsoVariantsTest.setupAgainCopiesChangedDataAndDropsTheCache
L7	medium	Game paths: the game builds file names in a 260-byte buffer (0x583748, FUN_00415690, no length check; the pointer at 0x58384C after it is passed to free) and uses ANSI file functions. A data folder path longer than ~230 characters crashes the game; characters outside the ANSI code page (Windows user "Łukasz" on a German system, Cyrillic names, …) become '?' → CD files not found. Linux: same through Wine (home folder with such characters)	Linux: the data folder is mapped as drive L: in our prefix (wine/dosdevices/l:), CDPath = L:\cd-active\, exe L:\game\groove.exe (short, ASCII); Windows (and the Linux fallback to Z:): GameLauncher.checkPaths refuses to start with a clear message (path > 200 chars or not encodable in native.encoding)	GameLauncherPathsTest; in game: all runs below use L:
L8	low	CLI argument errors: --install without path → NoSuchElementException stack trace; --unlock without/with bad numbers → stack traces; --unlock 4 1 1 wrote king3.ksv, --unlock 0 … wrote king-1.ksv, world 10 / chapter 7 of world 1 were written into the save (game crashes on the map); --apply/--launch before setup → "NoSuchFileException: …/original/cd"; --install while the game runs replaced game/groove.exe and then failed	Main: UsageException (exit 2, "Error: …"), range checks (slot 1–3, world 1–9, chapter 1–n), "not set up yet", running-game checks for --install/--unlock; --editor-build/-import/-new refused in a build without editor (Edition)	MainCliTest
L9	low	"Clear cache" (Settings) while the game runs: on Linux with the symlink fallback the running game lost its CD files, on Windows the delete fails half-way on locked hard links, and the following re-apply is refused	ModManager.clearCache refuses while the game runs (and takes the apply lock)	clearCacheIsRefusedWhileTheGameRuns
L10	low	Damaged config.json (cut off by a crash or power loss): defaults → "not installed" → setup wizard again (600 MB copy), and the next save overwrote the damaged file	Config.load: copy to config.json.broken; if original/ and game/groove.exe exist, the installation stays usable (mod selection/options are lost)	ConfigTest
L11	low	Apply wrote game/groove.exe before swapping cd-active; when the swap failed (Windows: cd-active open in Explorer or being scanned) the new exe (e.g. boot-to-level into world 9) ran against the old CD folder; gameExeSha256 was set before the write, so a failed write produced a false "changed by antivirus" alarm	ModManager.apply: cd-active first, then the exe; the hash is recorded after a successful write	existing ModManager tests
L12	low	Windows: replacing a read-only file (save game copied from a CD/old install) failed with AccessDenied	FileOps.writeAtomic clears the read-only flag of the target first	SaveStoreTest (no-op on Linux)

In-game smoke tests (apptest, display :8, session lxbugs-mx, all with CDPath L:\cd-active\)

Each run: apply via the CLI → (slot 3 unlocked at the level, menu → slot → map → level via tolevel) → key sequence with screenshots → crash check (process, game.log, winedbg, window) → audio client present. Exe mods were additionally checked in every combination by ExeModCombinationsTest (apply, re-parse, objdump, replay, restore).

Run	Mods (options)	Level	Result
orig	none	Village 1	OK, walking
all-v	fog, randomizer, mirror, intro-skip, cheats, practice, freecam, debug-wireframe, testworld (defaults)	Village 1	OK; wireframe + mirror visible, F7 free camera flies, F5/F8
all-t	same	TestLevel 1 (Test World)	OK
introskip	intro-skip (story on)	Village 1	OK
cheats	cheats (diagnostics on)	Village 1	first run: harness timeout on the map (Enter not taken); re-runs cheats-def (defaults) and cheats-diag2 (diagnostics) OK → harness flake
practice	practice (defaults, checkpoints)	Village 1	OK, F5 / F8
freecam	freecam	Village 1	OK, F7 camera and F9 noclip
mrf-v / mrf-r	mirror, randomizer, fog	Village 1 / River 1	OK / OK (raft level mirrored and randomized)
twdbg-t / twdbg-v	testworld, debug-wireframe	TestLevel 1 / Village 1	OK / OK
boot	boot-to-level (world 1, chapter 2)	boots into Village 2	OK
boot-exe	boot-to-level (world 3 ch 1) + intro-skip (story) + cheats + practice + freecam	boots into River 1	OK (this combination failed to apply before L1); F7 works

After rebasing onto the merged main tree (simp-all + launcher UX changes): orig, all-v, boot-exe, mrf-r re-run with the final jar: all OK (CDPath L:\cd-active\).

Screens: all mods in Village (wireframe, mirrored) and the free camera; River with mirror+randomizer+fog; Test World with all mods; boot-to-level + all exe mods (River loading card); practice restore.

Not fixed / recommendations

• Cache key and app updates – fixed in 0.2.0: the chain root contains the app version, the build id (hash of the compiled classes) and ExePatcher.REVISION; entries of other builds (and of 0.1.0) are deleted after an apply.
• Cross-mod hotkeys: practice (save/restore) and freecam (camera/noclip) offer the same key list and do not know each other – the same key in both triggers both. Needs a check across options (e.g. ModsPanel or a Mod.hotkeys() hook).
• Launcher edition (edition.properties editor=false): only hides the editor button and the --editor* commands; built by ./gradlew packageAll (fatJarLauncher, packageLinuxLauncher, packageWindowsLauncher) since 0.2.0. The editor packages must stay in the jar: mods.boottolevel.StartOverride (editor.io/model, tools.playtest.FloorFinder) and mods.levels.* (editor.io) need them at apply time; ModRegistry would only skip level projects (LinkageError is caught), but boot-to-level with a player start override would fail with NoClassDefFoundError.
• OneDrive / redirected Documents (decided: no warning) as data folder: hard links inside a synced folder and on-demand files are fragile; the wizard could warn when the chosen folder is under %OneDrive%.
• Interrupted apply – fixed 2026-11-07: cd-active.new.complete marker + ModManager.recoverInterruptedApply() (GUI start, Play, --launch), see llamamod-app.md. Follow-up 2026-11-07: the GUI tells the user once what was repaired and retries when another window held the apply lock (was silently skipped).
• A config.json saved by a stale window overwrites activeMods/gameExeSha256 – fixed 2026-11-07: field-level merge-on-save under .config.lock (Config.save). Follow-up: the window rebuilds its Mods/Settings tabs after a merge or when activated with a changed config.json (Config.refresh), so its check boxes no longer show stale values.
• Old Wine versions are not detected – 2026-11-07: wine --version in Settings, warning below Wine 5.0 (Settings in red, once before Play); missing Wine gives a clear message.