Keep HUD window state across GUI revisions; add save layout contract and restore matrix

[akacross]

Status after merging main

main's 6e13a0f and 49a2710 fixed the three restore failures this PR started with, using a different design:

• HUD revisions under guis/savecompat/;
• an interpolator mirror for payloads before build 1582;
• release builds mapped to their tagged commit counts.

I merged main, took its versions of every shared file, and dropped this branch's competing fixes. Its interpolator layout restore in particular would misread every save main has written since build 1582. What remains is what main doesn't have.

1. Keep HUD window state when reading an earlier GUI revision

ReadUserInterfaceRevision copied only the state dictionary. Window-local values that scripts set were lost: showBossBar sets BossBar::visible, and restoring the boss doesn't replay StartBossBattle, so an older save taken in a boss fight loaded without its boss bar. This is the gap raised in review.

idUserInterface::ReadFromSaveGameWithLayout is appended after the existing vtable slots, so older modules keep their slot layout. It parses the saved definition twice in private:

• one copy consumes the record with every normal check;
• the other shows what that definition starts with.

The state dictionary, flags and cursor move to the live GUI, along with every window value the game changed, wherever a window and variable of the same name and type exist. Running transitions restart. SP and MP both call it.

Same v3 save, taken by build 1109 during game/waste's boss fight (trigger target_bossbattle_bar), restored on Linux x64:

Build	Result
main	Restores; no boss bar
this PR	Restores with the boss bar; log: restored saved Bossbar::visible '1' (layout 'guis/savecompat/hud_strogg_r1.gui' starts with '0'), plus p_boss_health_bar::rect and 37 more

2. savegame_gui_layout_contract.py

Rewritten for main's revision table:

• every openQ4 GUI is classified as serialized or never serialized (arena_pause.gui is new and never serialized);
• each serialized GUI's current definition is pinned, with 2126a232's matscaley removal recorded as layout-neutral;
• OPENQ4_SAVEGAME_GUI_REVISIONS must match, and each guis/savecompat/ copy must equal what 38bdc687^ shipped once its header is stripped;
• each marker window must exist only in the current definition;
• no orphan revision files;
• both readers must use the engine migration.

On main it fails only on that last check. A simulated HUD edit fails it until the edit is classified.

3. savegame_restore_matrix.py

An opt-in runtime restore check. It never deletes anything, and before writing it refuses linked inputs and inputs inside its output root (the earlier review comment). --screenshot captures the restored scene, and --self-test runs in CI.

On this branch it restores my v0.13.2 build-878 save, the 0.14.000 build-1574 save and the boss-fight save, and leaves the inputs unchanged. main restores the same three.

Also

#include <cstdint> in src/ui/retained/Motion.h for GCC 16.

Tests

• savegame_corruption_contract, savegame_pointer_width_safety, savegame_v3_contract, savegame_gui_layout_contract, gui_transition_operand_contract, ui_manager_lifecycle and validation_hardening pass, as does savegame_restore_matrix.py --self-test (7/7).
• CI's Validation Script Smoke currently fails on main itself at ui_number_clipboard.py. I'm opening a separate PR for that.

Not verified

Windows, macOS and ARM64 restores; gameplay state beyond the HUD.

🤖 Generated with Claude Code

[themuffinator]

The version-specific GUI decoder and format-4 build-number change address real compatibility problems. I ran savegame_v3_contract.py, savegame_pointer_width_safety.py, savegame_corruption_contract.py, savegame_gui_layout_contract.py and validation_hardening.py on 842acc2; all five passed on Windows.

Two issues need correction before merging: the runtime harness can delete an input save when an output directory contains it, and the GUI migration discards persistent HUD window state, including boss-bar visibility. The input-deletion case was reproduced with an isolated disposable fixture and no game launch. The HUD issue is based on tracing the restore, GUI definition and gameplay update paths; I have not run an in-game restore.

CI is also incomplete: four checks passed, twelve were cancelled and one was skipped. Windows x64 exceeded its 60-minute limit while running retained-UI tests; Linux x64 GCC/libdecor exceeded 90 minutes. These timeouts need investigation and completed validation before merge.

[themuffinator]

[P2] Preserve persistent HUD window state during the v3 migration

Copying State() does not copy window-local variables restored into saved->desktop. For example, Bossbar::visible defaults to 0 in both live HUD definitions and is set to 1 by showBossBar when idPlayer::StartBossBattle runs. Restoring bossEnemy does not call StartBossBattle again; UpdateHudStats only updates boss_health / emits updateBossBar, which does not restore that visibility. Consequently a v3 save taken during a boss fight loses the boss bar after migration (and the retained HUD also derives its join state from the legacy window). This is persistent state loss beyond the documented animation restart. Transfer compatible persistent presentation variables or explicitly reconstruct that HUD state after restore, in both SP and MP, and cover a save taken during a boss fight.

[akacross]

Agreed, and fixed in 7185ae2. Copying only State() lost window-local values such as Bossbar::visible.

How it works now. idUserInterface::ReadFromSaveGameWithLayout, appended after the existing vtable slots, parses the saved v3 definition twice in private:

• one copy consumes the v3 window tree with every normal check;
• the other shows what that definition starts with.

Besides the dictionary, flags and cursor, every window value the game had changed moves to the live GUI, wherever a window and variable of the same name and type exist. Variables bound to the GUI state already arrive with the dictionary. Transitions and timeline progress still restart. SP and MP both use it, and savegame_gui_layout_contract.py now requires that.

Boss-fight check. I made a v3 save with the v3-only build 1109 during game/waste's boss fight (trigger target_bossbattle_bar) and restored it on this branch. With developer 1 the log shows:

gui 'guis/hud_strogg.gui': restored saved Bossbar::visible '1' (layout 'guis/savegame/v3/hud_strogg.gui' starts with '0')
gui 'guis/hud_strogg.gui': restored saved p_boss_health_bar::rect '266 40 148.9 50' (layout ... starts with '266 40 0 50')

The post-restore screenshot shows the boss bar, and it matches a boss fight started fresh on this build.

A separate bug this exposed on main. After merging main, every v3 and v4 save, including 0.14.000's own, failed at the player's animator:

idAnimator::Restore: saved joint count 0 does not match model joint count 65 for model 'model_player_marine' on entity 'player1'

The cause is f15a41f, which added a bool cacheValid to idExtrapolate/idInterpolate. jointMod_t (which embeds idInterpolateAccelDecelLinear<idAngles>) and running GUI transitions (<idVec4>) are saved as native images, so each grew by 4 bytes. Compiled sizes for idExtrapolate, the angle interpolator and the vec4 interpolator: 0.14.000 is 64/104/128, main is 68/108/132.

dcddd68 marks the cache stale with a NaN currentTime instead. That restores the original layout and keeps the negative-time fix: InterpolationTest.cpp passes unchanged with GCC and Clang at -O0 and -O2. static_asserts beside both raw writes now require these types to hold only their declared members, and they fail against f15a41f's headers.

[themuffinator]

Re-reviewed the current revision against the consolidated roadmap (Q4-05/Q4-07/UI-09). The destructive output-directory cleanup has been removed; the five available path/retention self-tests pass on Windows here (the two symlink cases were skipped because symlink creation was unavailable). I am resolving that original input-deletion thread.

The new migration carries window values, but still loses their saved evaluation state; the inline finding below needs correction and a behavioral regression that advances GUI evaluation after restore. Keep the reported Linux boss-fight evidence, but a copied value or a single initial capture does not establish persistence through subsequent updates. No in-game restore was run in this review.

CI run 39138909016 stops at the unrelated missing Mock::Authored accessor in ui_number_clipboard.py; #190 addresses that failure. Complete validation against current main after the test-harness fixes. Also keep the self-test's temporary files under the repository's .tmp directory rather than the system temporary directory.

[themuffinator]

[P2] Preserve the saved evaluation flag as well as the variable value

Script_Set calls SetEval(false), and that flag is serialized by the idWinVar readers. This transfer only calls Set(), which leaves the live variable's newly parsed evaluation flag intact. idRegister::GetFromRegs then remains allowed to overwrite a scripted value with its expression on a subsequent GUI update. The value-only equality filter above also skips an explicit override whose saved value equals the initial value even though its evaluation ownership changed. Transfer compatible evaluation state (including that equal-value case) without freezing values that should still follow expressions, and exercise restore followed by changing GUI state and several evaluation/redraw ticks.

[akacross]

Fixed in 035b340. The migration now carries a value whenever its value or its eval flag differs from the saved definition's, and sets the live flag to the saved one (liveVar->SetEval( savedVar->GetEval() )). So:

• a scripted override stays frozen, including the equal-value case;
• a variable that was still following its expression keeps doing so.

Behavioural regression. game/mcc_1's main() sends hideHud at once, which sets p_health::visible/p_armor::visible to 0 against their expressions ("gui::mp" == 0) and ("gui::player_armor" > 0 && …). I made a v3 save there with build 1109 after give armor, then restored it and:

1. read the live HUD with a new cheat command, hudGet <window::variable> (SP and MP);
2. changed GUI state with give armor;
3. ran 300 frames;
4. read it again.

	right after restore	after armor change + 300 frames
value-only transfer	p_health::visible = 1, p_armor::visible = 1	1, 1
this commit	0, 0	0, 0

The developer 1 log shows the equal-value case being carried: p_armor::visible '0' eval 0 (layout … starts with '0' eval 1). savegame_restore_matrix.py now takes --after-load commands and --expect-log patterns, and that's how this runs as an assertion:

--after-load 'give armor' --after-load 'wait 300' --after-load 'echo AFTER_UPDATES' \
--after-load 'hudGet p_health::visible' --after-load 'hudGet p_armor::visible' \
--expect-log 'AFTER_UPDATES\s*\nhudGet p_health::visible = 0\s*\nhudGet p_armor::visible = 0'

The same build keeps Bossbar::visible = 1 after 300 frames on the game/waste boss-fight save, and still restores the 0.14.000 (build 1574) and v0.13.2 (build 878) saves. savegame_gui_layout_contract.py now requires the flag transfer and fails if it's removed. The harness self-test keeps its scratch files under the repository's .tmp/.

One gotcha I hit while building fixtures: a save made after using the console script command can't be restored, because the program records a pseudo-file named console. So the fixture uses give armor instead.

[akacross]

CI status for 035b3409 (run 39169298509). Every failure is on a step that also fails on main's latest push run for cbe967f2 (run 39156525461), where every platform job fails too:

PR job	Failing step	Same step on main
Linux ARM64	Renderer startup validation matrix (missing self-test markers); build and Meson tests passed, 0 failures	Linux, Linux ARM64, macOS Metal and macOS Intel Metal fail the renderer startup matrix
Windows x64	Dedicated server smoke	Windows fails the dedicated server smoke
macOS ARM64 OpenGL / Metal	macOS dedicated server smoke	macOS OpenGL / Metal fail push verification
macOS Intel x64 OpenGL / Metal, Windows ARM64, Linux x64 libdecor, sync-window-ops	Cancelled at the 90-minute PR limit in the main validation step	macOS Intel OpenGL cancelled; Windows ARM64 fails native tests

Passing on this PR: Validation Script Smoke (which includes the save-compatibility contracts and the restore harness self-test), Linux x64 GCC and Clang no-libdecor, Linux ARM64 GCC libdecor and Clang no-libdecor, Linux x64 ASan+UBSan, and the ARM64 cross build.

The timeouts look like runner time rather than this change. sync-window-ops passed on #189 with the same base, and #191 roughly halves the Python check time, which should keep most of these jobs under the limit.

The review fix for the expression-override (eval) flag is in 035b3409, as described in my reply above. It's ready for another look whenever you have time.