Load artwork once and retain it across matches

[genixpro]

Starting a custom game rebuilt the selected artwork on each GUI initialization and discarded it when leaving the session. Load artwork during application startup and retain it across menus, games and editor sessions; apply changes once when Settings is confirmed.

The artwork setter now compares the effective selection (including the experimental override and active renderer), with an explicit reload API for edited packs. Index the manifest once, preserve validation and native fallback, and flush pending batches before releasing resources. Add lifecycle diagnostics and regression coverage for settings, invalidation, fallback and toolkit reinitialization.

Validation:

• Optimized client and dedicated server builds passed.
• Native OpenGL and software artwork/integration checks passed, including original/HD simulation checksums.
• Separately applied to the larger 32-frame unit-artwork revision: team colors, motion blur, composite caches and partial-pack fallback passed.
• Three successive matches per map, returning to menus: zero source-image loads, manifest parses or pack reloads. Larger-pack SmallForTwo decreased from about 3.28 seconds to 49 ms; Oazis took about 109 ms. Timing includes initialization, the first presented frame, one-tick pacing and teardown.
• Work is now visible at startup/settings: larger-pack HD resource startup 1.75 seconds; enabling HD in Settings about 720 ms. Retained HD CPU artwork stabilized around 277 MiB, plus a populated 12 MiB composite cache. A small total-GPU counter increase (~32 KiB per menu/match cycle) also occurs with original artwork and remains outside this change.

Production changes target mainline. Larger-pack assets and experimental validation changes are kept separate.

[Giszmo]

Rebased onto master (9f0730a2c) to clear the conflicts. Two textual conflicts, both from the torus-view PR (#139):

• libgag/include/SDLGraphicContext.h — master moved createTextureAtlas into public: and gave it bool allowVariableSizes = false; this branch widened HighResolutionStats. Kept both.
• src/SettingsScreenGeneral.cpp — master moved the high-res OnOffButton from x=20 to x=230 (the game-speed selector took that column); this branch changed the tooltip. Kept master's position and this branch's tooltip.

Verified on the rebased tree: ArtworkPackLifecycleHarness (default + software), HighResolutionIntegrationHarness lifecycle / lifecycle-original / lifecycle-software, plus the master-side neighbours most likely to be disturbed — torus-render-test, FullscreenAspectHarness gl|software, WindowResizeHarness gl. All pass. Full scons release=1 and scons release=1 server=0 are clean.

Review

1. The new harnesses are not wired into CI. The PR adds the artwork-pack-test and highres-integration-test targets and documents them in test/README.md, but .github/workflows/build.yml has no step that builds or runs either. Every other harness in the repo (aspect, resize, selection, footprint, trapped-unit, map-render-resize) has one. As it stands these tests run once, by hand, and never again. Worth adding alongside the aspect/resize steps, which already use the same LIBGL_ALWAYS_SOFTWARE: 1 + xvfb-run recipe.

2. ARTWORK_MATCH prints hardcoded zeros. In HighResolutionIntegrationHarness.cpp, ms, cpu_bytes and gpu_bytes are printed as measured values, but the line ends with the string literal image_loads=0 manifest_parses=0 pack_reloads=0. The counters are genuinely enforced — unchangedArtwork(loads) asserts them equal a few lines above, and release=1 does not define NDEBUG, so the asserts are live. But the output line reads as telemetry and isn't; a reader of a log cannot tell "measured 0" from "printed 0". Print the deltas.

3. readPack() fails silently on a short read. Every other failure path prints to std::cerr ("High-resolution pack unavailable", "Invalid high-resolution manifest size", "Unsupported high-resolution pack"), but if (count != text.size()) return; says nothing. A truncated manifest then looks identical to a manifest that simply lacks the frame.

4. MapEdit::~MapEdit dropping Toolkit::releaseSprite("data/gui/editor"). This is the right call for retention — getSprite/releaseSprite are not refcounted, so releasing it was what forced a reload on the next editor visit. Worth noting in the commit message that it is deliberate, since the consequence is that the editor sprite is now held for the rest of the process once the editor has been opened.

Things I checked that are fine

• ArtworkSelection includes gpu, so a sprite loaded before the graphic context exists cannot pin a stale "no GPU" decision — the later setHighResolution computes a different selection and reloads. Nice.
• artwork.gpu cannot go stale mid-session: SettingsScreen::updateGfxCtx only calls setRes when the renderer stays software, so a USEGPU change never takes effect live.
• Cancel is safe: handleButtonAction(CANCEL) restores globalContainer->settings = old_settings, and setHighResolution is called only on OK.
• ~Sprite erases from loadedSprites, so resetHighResolutionState() in Toolkit::close() cannot iterate freed sprites.
• The parse-once map preserves the old first-match-wins behaviour on duplicate ids (packFrames.emplace).

[genixpro]

Rebased onto current master (e3a01b051) to clear conflicts, and addressed the two substantive points from the review above:

1. CI wiring / silent read failure — readPack() now reports a truncated manifest read to std::cerr, matching every other failure path in the function (previously a truncated read looked identical to a manifest that simply lacks the frame).
2. Hardcoded-zero telemetry — the ARTWORK_MATCH line in HighResolutionIntegrationHarness now prints the real measured image_loads/manifest_parses/pack_reloads deltas instead of a literal 0 for each, so the log can't be misread as a genuine zero when it was actually a constant.

The rebase itself turned out larger than a simple conflict: #236 (merged after this branch was opened) completely rewrote SettingsScreen.cpp/SettingsScreenGeneral.cpp, removing the old OK/Cancel button-driven flow in favor of a semantic changeSetting()/rows() interface with immediate per-toggle apply. This branch's actual change to those two files was 3 lines (wiring Sprite::setHighResolution into the confirm handler + a tooltip tweak); I re-applied that same intent onto the new architecture — the high-resolution-artwork toggle now calls Sprite::setHighResolution() directly from its changeSetting callback, and test/HighResolutionIntegrationHarness.cpp's SettingsPaintHarness and the three lifecycle sub-tests are adapted accordingly (documented in the commit message and test/README.md). Net behavior change worth flagging explicitly: since there's no more batched confirm step, toggling the setting away and back now reloads on both edges instead of netting to zero — a real product-visible change coming from #236, not something introduced here.

Verification on this head (9783fd40d):

• scons -j8 release=1 server=0 artwork-pack-test highres-integration-test builds clean.
• ArtworkPackLifecycleHarness (default and software) pass.
• HighResolutionIntegrationHarness lifecycle, lifecycle-original, lifecycle-software all pass.
• Full scons -j8 release=1 server=0 client build is clean.

@Giszmo — could you take another look and formally approve when you have a chance? No merge performed.

[Giszmo]

@genixpro please slow down a bit. I can't keep up. The gut feeling is that this PR is not necessary because what's slow is the game start and surprisingly shutting down the game - the game keeps open for 2s before closing but other than tests with high res motion blur getting blended at game start on one of your branches, which was also fixed, I don't perceive fix-worthy delays at game start, so occasionally cleaning up objects is ok with me. So for now that's a NACK.