95 lines
13 KiB
Markdown
95 lines
13 KiB
Markdown
# 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 `IOException`s; 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.
|