Skip to content

Add browser builds with persistence, WebGL2, and YOG cross-play - #203

Open
genixpro wants to merge 14 commits into
masterfrom
codex/browser-experiment
Open

Add browser builds with persistence, WebGL2, and YOG cross-play#203
genixpro wants to merge 14 commits into
masterfrom
codex/browser-experiment

Conversation

@genixpro

@genixpro genixpro commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Globulation 2 can now run as a full-page WebAssembly browser client while sharing the existing game, UI, save, map, replay, and YOG logic with native builds. The browser supports single-player play, durable local persistence and import/export, an opt-in WebGL2 renderer, and browser/native YOG cross-play through a WebSocket gateway.

This branch is reconstructed from the working browser implementation as five reviewable commits on current origin/master. The rewrite removes development diaries, handoff notes, screenshots, machine-specific benchmark output, stale migration commentary, and historical checklists. It preserves the existing implementation and formats while fixing current-master integration issues and adding focused safety coverage.

Commit series

  1. Add isolated WebAssembly browser build and host gateway — build identities, shared source manifests, Emscripten isolation, packaging, and the gateway skeleton.
  2. Run browser gameplay through scheduled screen lifecycles — the application loop, screen scheduling, cooperative loading/generation, cancellation, and interpreter lifetime handling.
  3. Add browser YOG cross-play and deployment support — shared framing, TCP/WSS transports, multiplayer screens, gateway routing, and deployment files.
  4. Add durable storage, imports, resizing, and WebGL2 — persistence and file services, validated imports, responsive viewports, and the optional GPU renderer.
  5. Stabilize and qualify the browser release — input/audio fixes, shutdown behavior, current-master integration, map allocation limits, regression coverage, ADRs, and concise operating documentation.

Each commit completes a native release build independently.

Behavior and compatibility

  • Native gameplay remains on the existing code paths and defaults.
  • Browser profiles persist settings, campaign progress, maps, saves, and replays in the existing browser storage layout. Imports are validated before installation, and supported files retain their existing formats.
  • The software renderer remains the default. ?renderer=webgl2 selects the WebGL2 path, including context-loss restoration.
  • The HTML shell presents a branded background, animated progress indicator, and current Emscripten loading phase until the game is ready.
  • Browser viewports use their actual positive CSS dimensions from startup onward. There is no 800×600 gate; live games keep rendering and simulating through small resizes, while existing fixed-size dialogs reveal their full layout again when the window grows.
  • Browser multiplayer uses the existing YOG protocol through WSS; native clients retain TCP and verified WSS support. Native WSS is enabled by default; wss=0 produces a TCP-only client without OpenSSL or Boost.Beast and fails closed for WSS addresses. LAN is compiled out of WebAssembly because browsers cannot provide its direct TCP listener and discovery model.
  • Map dimensions are validated before size-dependent allocation. Supported exponents are 4 through 9, corresponding to 16×16 through 512×512 maps; larger or malformed content is rejected as invalid.
  • Existing wire protocol and save/map layouts are unchanged. Newly generated layouts change because generator randomness is now isolated and reproducible; existing map/save files are unaffected. YOG distributes the host-selected map file, so browser and native players use the same map bytes in a match.
  • Native synchronous screen adapters remain for their existing callers. The finite compatibility inventory and the rule for new interactive flows are recorded in ADR 003; coroutine borrowing, cancellation, and reentrancy rules are recorded in ADR 004.

Build and run

python3 browser/setup.py
scons target=web release=1 -j8
python3 -m http.server 8765 --bind 127.0.0.1 --directory build/emscripten/client/release

Open http://127.0.0.1:8765/ for software rendering or add ?renderer=webgl2. See browser/README.md for the pinned toolchain, tests, gateway setup, and troubleshooting.

A hosted single-player build is available at Launch Globulation 2. Multiplayer gateway hosting and a project-website Play button are not included in that demo. A later browser multiplayer experience can build shareable match links, lightweight guest identity, and instant matchmaking on top of this PR’s YOG cross-play foundation.

Validation

Local validation on the rewritten branch includes:

  • native and WebAssembly release builds;
  • native build-system, screen/session, savegame, deterministic replay, TCP/WSS transport, gateway, and buffered-stream tests;
  • clean session/savegame harness rebuilds after correcting stale-fixture assumptions and removing duplicated rebase residue;
  • deterministic replay fixture regenerated at format 93 after current master raised the compatibility floor; replay import and truncated-stream rejection pass against it;
  • AddressSanitizer and UndefinedBehaviorSanitizer runs for malicious map/save input and TCP framing/transport;
  • strict translation validation with zero structural errors;
  • 15 browser JavaScript unit tests and the complete 225-case Playwright suite across Chromium, Firefox, and WebKit, including browser/native YOG cross-play;
  • 42 focused software/WebGL2 checks for loading presentation and viewport behavior across Chromium, Firefox, and WebKit;
  • hosted Chromium checks for the loading shell, live and initial small viewports, responsive menus, editor dialogs, and high-density rendering.

PR CI keeps the expensive coverage bounded: one WebAssembly build, the complete Chromium behavior suite, focused Firefox/WebKit compatibility checks, focused Chromium WebGL2 and visibility coverage, and a 60-minute cap on the browser job. Native builds remain in the repository’s existing compiler lanes instead of being repeated in the browser job. Browser executions were reduced from 359 to 110, the prior build-order matrix and automatic daily run were removed, and large development artifacts upload only for manual runs with seven-day retention.

The final four-job CI run for the rewritten SHA is the merge gate.

@genixpro
genixpro force-pushed the codex/browser-experiment branch 2 times, most recently from 36d3687 to 94cdfbe Compare September 8, 2026 15:14
@genixpro
genixpro requested review from Giszmo and kylelutze and removed request for kylelutze September 9, 2026 01:07
@genixpro genixpro changed the title WIP: Make browser support a first-class Glob2 platform Add the browser platform with WebGL2, persistence and YOG cross-play Sep 9, 2026
@genixpro
genixpro marked this pull request as ready for review September 9, 2026 04:45
@Giszmo

Giszmo commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

This is genuinely exciting. A browser build is the single biggest thing we could do for
reach: "click a link and you are playing" removes the packaging, distro and driver
problems that cost us most new players. Being able to put a Play button on
globulation2.org is something I have wanted for a long time, and I appreciate that you
went at it by compiling the real engine rather than reimplementing the game in JS —
around 130 lines of shipped JS glue and only 8 __EMSCRIPTEN__ sites across src/ and
libgag/ is a much better outcome than I expected. The shared scons/sources.py
manifest is the right call.

Some things I want to be explicit about before this lands, mostly because the cost of
this port is not in the browser directory — it is in the shared code.

Generated map layouts change. ADR 005 states this outright. I accept the reasoning:
mixing the synchronized generator with libc rand, time-based reseeding and shared
static Perlin tables meant a seed never actually reproduced a map, and a noise helper
could perturb unrelated noise including clouds. That was a latent bug and the new
seed-to-map contract is worth having. But it is a user-visible change for existing
desktop players, so it should be called out in the release notes rather than discovered.
Related: the ADR says bit-exact generation across native and Wasm is not yet established
because of floating-point height maps. What happens today if a browser and a native
client generate a map in the same session?

Two screen-execution models now coexist. ADR 003 is honest that execute() remains a
compatibility polling host, overlay modal loops are unconverted, and calling a legacy
child execute() from a callback still blocks. That is reasonable for an incremental
migration, but half-finished migrations tend to stay half-finished once the motivating
work ships. Could you write down what is left as a tracked list, so it does not become
folklore?

C++20 coroutines in the loader. CooperativeTask brings new lifetime rules — jobs
must outlive nothing they borrow, no reentrant advancing — into the load path, which is
exactly where subtle bugs are most expensive. You raise the hand-written state machine
alternative yourself in ADR 004; I would rather keep the coroutines, but I want that
tradeoff visible to whoever debugs it in two years.

Networking was rewritten. NetConnectionThread and its message queues are gone in
favour of the NetTransport interface. I checked that TcpTransport still runs a real
worker thread, so desktop keeps its threading model — good. Still, every desktop
multiplayer path now runs through new code, and multiplayer is the area where we have
the fewest users to catch regressions for us.

Small thing: three surviving files lose their // Copyright (C) ... Bradley Arsenault
line (src/FertilityCalculator.h, src/net/NetConnection.h, src/net/NetConnection.cpp)
and NetConnection.h loses its /// documentation comments in the rewrite. They are
your lines to remove, but in a GPL project I would rather keep headers intact and
document the new interface.

What reassures me most is the shape of your evidence, not its volume: freezing the old
fertility algorithm in LegacyFertilityReference.h as an equivalence oracle is exactly
the right instinct, and the new native harnesses mean desktop is not just being trusted
to still work.

One expectation to set: the description defers production hosting and operations, and the
current demo is a single-player bucket at about 31 MB with no loading indicator. So this
PR is what makes globulation2.org hosting possible, not what delivers it. I am glad to
treat hosting as separate work — I just do not want anyone reading this thread to think
the Play button arrives with the merge.

Thank you for the ADRs and for the candour in status.md about what is still
experimental. That made this much easier to review.

@Giszmo

Giszmo commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

So what's the goal of this? Instant custom games and campaigns (single player) from our website or YOG and LAN, too? The former would already be awesome and could be its own branch if the consequences for merging this to master were somehow too controversial.

@genixpro
genixpro force-pushed the codex/browser-experiment branch from 495a657 to 3dcbde1 Compare September 9, 2026 16:46
@genixpro genixpro changed the title Add the browser platform with WebGL2, persistence and YOG cross-play Add browser builds with persistence, WebGL2, and YOG cross-play Sep 9, 2026
@genixpro
genixpro force-pushed the codex/browser-experiment branch 6 times, most recently from 5dd4fe9 to bd49479 Compare September 9, 2026 20:11
@genixpro

genixpro commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

Thanks — the intended scope is now clearer after cleaning the branch up.

This PR makes custom games, campaigns, saves/imports, and the existing YOG flow work in the browser. YOG goes through the WebSocket gateway and supports browser/native cross-play. LAN is compiled out of the WebAssembly UI: browser sandboxing does not provide the direct TCP/listen/discovery model that Glob2 LAN uses.

For generated maps, YOG clients do not independently regenerate the selected map. The host supplies the MapHeader and file ID, and a client that does not already have that exact map downloads the map file before the match starts. The floating-point caveat in ADR 005 therefore affects portability of offline seed-to-map generation; it should not create browser/native map divergence inside one YOG match. I agree the desktop-visible generator change belongs in release notes.

Longer term, I would like the browser release to grow a web-native multiplayer entry flow on top of this cross-play foundation: shareable match links, guest or lightweight identity, and instant matchmaking without requiring account creation first. That product flow is follow-up work rather than part of this implementation cleanup.

I also restored the three copyright lines you identified and added API documentation to the new NetConnection interface in the rewritten branch. The PR is now five commits based on current master; the migration/status artifacts from the exploratory branch are gone.

@genixpro
genixpro force-pushed the codex/browser-experiment branch 9 times, most recently from cfd2e9d to 7962c30 Compare September 10, 2026 01:23

@Giszmo Giszmo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requesting changes, but I want to lead with what this review is not about.

I was worried about the +16,330 / −4,203 and about what a browser port would cost the desktop
game. I built both sides and measured rather than guessed, and on those two questions the PR
holds up: the simulation is bit-identical and the engine gets slightly simpler. The four
changes below are dependency and packaging issues, not architecture.

Requested changes

1. OpenSSL is now a hard requirement for every desktop build — please put it behind a flag

SConstruct:166 appends to missing and fails the build if OpenSSL is absent, for any
server=0 configuration. There is no flag and no fallback, so every packager on every
platform now needs OpenSSL to build a client that may never open a wss:// connection.

The size cost is concentrated in one file. Attributing the binary growth per object on the
link line:

src/net/WssTransport.o     828,479 bytes of .text
all 362 other shared .o    +162,873 bytes total (+5.5%)

WssTransport.cpp is 168 lines, but it includes boost/asio.hpp, boost/asio/ssl.hpp and
four boost/beast headers. The linked binary goes 5,486,152 → 7,656,528 bytes (+40%, .text
+1.50 MB) and picks up libssl.so.3 and libcrypto.so.3.

I am not asking you to drop it — NativeTransport::open (src/net/NetTransport.cpp:106)
selects WssTransport for a wss:// address, so this is reachable code that desktop
cross-play needs. I am asking for a build option (default on is fine) so a distro or a
minimal build can compile the client without OpenSSL and Boost.Beast, with
NativeTransport falling back to TCP-only.

2. debian/control was not updated

Build-Depends still has no libssl-dev, and nothing under darwin/ changed. The CI jobs
install libssl-dev and mingw-w64-x86_64-openssl explicitly, so CI stays green and the
first thing that breaks is a package build. Please add it (and whatever the macOS bundle
needs) in this PR.

3. Explain the regenerated determinism baselines

games/cross-replay.game is unchanged, but both baselines were replaced:

  • tests/baselines/cross-replay.replay: 256,668 → 610,606 bytes, diverging at byte 24
  • tests/baselines/cross-replay.checksums: same size, identical for the first 4,668 bytes,
    then 9,209 differing bytes

That reads alarming, so I checked it before writing this: the engine is bit-identical on that
exact save (evidence below), so this is a new, longer recording rather than a behaviour
change. But no in-repo test compares against cross-replay.checksums — the only in-repo
consumers of tests/baselines/ are two Playwright specs that use the .replay as an import
fixture. The oracle these files serve lives in the sibling cross-replay tooling, so replacing
them silently rebases something outside this repo.

Please say in the PR description why they were re-recorded and confirm the sibling tooling
was regenerated to match. If they did not need to change, revert them.

4. Hoist the per-tile work out of the FertilityCalculator kernel loop

Job::advance (src/FertilityCalculator.cpp:82) spends its operations budget per kernel
cell
, and kKernelSide is 31 — so 961 iterations per tile. Each of those calls
State::coordinate() (:72), which is cursor / map.getH() and cursor % map.getH().
Map::getH() returns a runtime member (src/map/Map.h:155), so that is real integer division
961× per tile where the old nested loop did it once, plus a coordToIndex and distance[]
lookup that used to be hoisted out of the kernel loop.

I did not benchmark this, so treat it as a code reading. The scope is narrow — only maps older
than FILE_FORMAT_VERSION_PRE_FERTILITY reach it, at load time (src/Game_io.cpp:306) — but
computing x, y and the grass/reachability test once per tile instead of 961 times is a
small change and keeps the incremental version honest against the one it replaces.

Evidence for the parts I am not asking you to change

Merge base 84a9b8b6 vs head 7962c30e, both built scons -j4 release=1 server=0, identical
flags (-O3 -std=gnu++20), native Linux.

Simulation is bit-identical. Headless --nox, SDL_VIDEODRIVER=dummy:

run master this PR final checksum
20,000 ticks, cross-replay.game (SmallForTwo, 2 AI) 1680 / 1650 / 1656 ms 1672 / 1664 / 1668 ms c44b9dfa both
20,000 ticks, gd-large-4ai.game 6643 / 6558 ms 6599 / 6632 ms 3d4ce420 both
1,500 ticks with GLOB2_CHECKSUM_SIDECAR=1 3,660,536 bytes of per-tick, per-unit and per-building checksums, cmp-identical

Save files cross-load in both directions: a save written by this branch loads on master and
yields the same checkSum() = 302ef289 after 500 ticks.

The line count is not engine code. By destination: browser/ +3,251, data/ +2,614, tests
+2,593, docs and ADRs +1,153, build system +840, deploy and CI +385. Shared engine code moves
+5,494 / −4,070 across 225 files, net +1,424. src/ + libgag/ total goes 134,145 → 136,076
lines, +1.4%. Unit.cpp, Map.cpp and Building.cpp are untouched; Game.cpp is −1 line.
There are 11 __EMSCRIPTEN__ sites in shared code across 6 files, and browser/ ships ~267
lines of JavaScript against 2,031 lines of Playwright tests.

Two things I expected to be debt and are not. src/net excluding the test suite goes
2,507 → 2,088 lines, and that 2,088 already includes the new WssTransport.cpp and
gateway/Gateway.cpp; native-only networking is about a third smaller, and
add_net_thread_message.py — a 103-line code generator — is gone. And the screen migration is
not the half-finished state I assumed from ADR 003: Screen::execute is now implemented on top
of the nonblocking primitives rather than duplicating them, and direct blocking execute( /
runModal() call sites drop from 44 to one (src/Engine.cpp:95, the end screen).

scons release=1 server=0 screen-test session-test builds clean here, and
ScreenExecutionHarness plus test/run-engine-session-test.py give 12 PASS and exit 0.

Non-blocking notes

  • libgag/src/GraphicContextCompound.cpp:85 adds
    if (!surface->textureInfo) glState.setTexture(surface->texture);, but the else branch
    eight lines down binds the same texture, and GLState::setTexture
    (libgag/src/GraphicContext.cpp:80) early-returns when unchanged. Harmless, just redundant
    on the native path.
  • CooperativeTask::promise_type allocates a shared_ptr<State> in get_return_object() that
    Awaiter::await_suspend immediately overwrites with the parent's for every child task. One
    wasted allocation per subtask.
  • The cmd+Q / alt+F4 handling moved from SDL_GetModState() to event.key.keysym.mod. That
    reads intentional — synthetic and replayed events need the event's own modifier state — but
    it is a macOS/Windows behaviour change that no CI job covers.
  • The new web job (Emscripten, npm ci, Playwright across chromium + firefox + webkit,
    pulseaudio, 60-minute timeout) is a real recurring CI cost and a new flake surface. Worth
    deciding on deliberately rather than inheriting.

Thank you for the ADRs and for status.md. Being able to read your reasoning for screen
execution, cooperative loading and generation randomness before reading the diff is what made a
349-file PR reviewable at all.

Opus 5 helped authoring this review.

@genixpro
genixpro force-pushed the codex/browser-experiment branch from 7962c30 to 81f91fb Compare September 10, 2026 12:25
@genixpro

Copy link
Copy Markdown
Contributor Author

Addressed the requested changes in the rebased five-commit series:

  • added an optional wss=0 native build that omits OpenSSL and Boost.Beast and fails closed for WSS addresses; verified the TCP-only link contains neither ssl nor crypto;
  • added the Debian OpenSSL build dependency and macOS packaging guidance;
  • hoisted the fertility kernel’s per-tile coordinate and eligibility work while preserving equivalence in the engine-session harness;
  • rebased onto current master and regenerated the deterministic replay fixture at format 93 because master now rejects older replay formats. The complete-import and truncated-stream regression passes against the new fixture.

CI is running on 81f91fb5e.

@genixpro

Copy link
Copy Markdown
Contributor Author

On item 3 (the regenerated determinism baselines):

tests/baselines/cross-replay.replay/.checksums changed because ReplayReader::open hard-rejects anything below REPLAY_MINIMUM_VERSION_MINOR, and that floor has been climbing on master independent of this branch — every merge that changes simulation behavior bumps it:

The fixture predates all of that, so it fails the floor check outright once rebased past any of those merges. That's not cosmetic: browser/tests/import.spec.js and session-reload.spec.js both import these exact bytes and assert a successful import, so an unloadable fixture is a real, in-repo test failure, not just an external mismatch. There's precedent for this exact regeneration too — e2f4da9e1 did the same thing in July when the format moved 84→87.

Separately: this branch is still pinned at VERSION_MINOR 93, one bump behind master's actual tip (94, from #223, which landed after my last rebase). So this fixture needs one more re-recording regardless, once I rebase past that.

On the sibling cross-replay tooling: that's @kylelutze's Rust reader, not something in this repo, so I can't regenerate or verify its oracle myself. Kyle — could you either grant visibility into it so this can be checked directly in future PRs, or regenerate/confirm its side against the baseline once this branch lands on the new bytes? Happy to ping you again once the next rebase (to version 94) is up.

I don't think this last piece should gate the merge decision here — the in-repo tests that actually load this fixture are covered, and the Rust-side sync is tracked as a follow-up rather than something this PR can resolve on its own — but I wanted it on record rather than left implicit.

genixpro and others added 6 commits September 10, 2026 15:59
Introduce the browser shell and pinned Emscripten toolchain, isolate native and web build outputs, and add the bounded WebSocket gateway. Keep the native build usable alongside browser artifacts.
Drive native and browser entry points through the shared application host and owned screen stack. Convert loading, editing, map generation, and related UI flows to cooperative work that can advance across browser frames.
Connect the existing framed YOG protocol through browser WebSockets, retain native TCP and WSS peers, schedule multiplayer navigation safely, and add the tested gateway deployment configuration and cross-play coverage.
Integrate browser-specific services with the shared source layout, make persistence failures recoverable, support validated file import and export, resize active screens at frame boundaries, and add opt-in WebGL2 context recovery.
Finish settings, audio, navigation, replay-save, and multiplayer edge cases; remove Asyncify; expand native and cross-browser regression coverage; refresh reviewer documentation; and omit machine-specific results, screenshots, and handoff logs from the PR.
The linux and windows jobs still pointed run-savegame-safety-tests.py
at build/src/TrappedUnitLifecycleTest[.exe], the pre-build_layout.py
path. Every sibling harness step in the same jobs already moved to
the per-toolchain/role/mode layout; these two were missed, so both
jobs failed immediately with FileNotFoundError.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XyKsqjkUFwsmf7dxp9SYdV
@genixpro
genixpro force-pushed the codex/browser-experiment branch from 92439b6 to ecc35f7 Compare September 10, 2026 20:34
master's tip (#223, fetch-job apportionment) bumped VERSION_MINOR and
REPLAY_MINIMUM_VERSION_MINOR to 94 after this branch's last recording
at 93. ReplayReader::open() rejects anything below the floor, and
browser/tests/import.spec.js and session-reload.spec.js both import
this exact file and expect it to load.

Re-recorded from the unchanged games/cross-replay.game (seed 42,
SmallForTwo, Econo vs Nicowar) run to its natural end via
`--nox games/cross-replay.game 0 1`, with GLOB2_CHECKSUM_SIDECAR_MAX_TICKS=100
to match the existing checksum sidecar's cap. cross-replay.checksums
came out byte-identical to the prior recording — expected, since the
sidecar only covers the first 100 ticks and Giszmo's review already
established the simulation is bit-identical between master and this
branch on this exact save.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XyKsqjkUFwsmf7dxp9SYdV
@genixpro

Copy link
Copy Markdown
Contributor Author

Rebased onto master's current tip (e3a01b051, picking up #223's VERSION_MINOR 93→94 bump among others). The branch was further behind than I'd realized — 5 master commits, not 1, including the Settings (#236) and custom-game lobby (#237) redesigns, which had independently rewritten CustomGameScreen and SettingsScreen around the same time this branch converted them to the non-blocking ScreenStack architecture. Kept master's redesigned implementations and re-applied this branch's architecture on top of them (constructor now takes ScreenStack&; SettingsScreen::persist()/onTimer now also track the browser's ApplicationHost::persistStorage() flush, so a failed durable write still surfaces through the existing saveFailed()/retry path instead of being silently dropped).

Verified locally: native client and server both build clean (scons release=1 server=0 / server=1), a headless --nox run completes with a deterministic checksum, and data/check_translations.py --strict is clean across all 34 language files.

On item 3: tests/baselines/cross-replay.replay is re-recorded at the new floor (version 94), from the unchanged games/cross-replay.game, run to its natural end. cross-replay.checksums came out byte-identical to the prior recording, consistent with the simulation being unchanged in the first 100 ticks the sidecar covers.

CI is running on 750ea6d37.

39 keys added for import/export and durable-save-failure UI (Loading
headers, saving to storage, storage restore failed, etc.) had only
their English source text copied into every non-English catalog.
master's newer test_catalogs_do_not_reintroduce_english_fallbacks
(pulled in by the rebase) flags any non-English catalog whose value
for a key equals the English source and isn't reviewed shared
vocabulary — this predates the rebase but was never checked before,
since that test didn't exist on this branch until now.

Translated all 39 keys into all 32 non-English catalogs. Two catalog
codes don't follow ISO 639-1: texts.si.txt holds Slovenian, not
Sinhala (the shipped fonts have no Sinhala glyphs at all, confirmed by
test_font_coverage.py, and none of its existing content is Sinhala
either); texts.sr.txt is Serbian in Cyrillic, matching its existing
content, not Latin.

Verified: check_translations.py --strict (0 structural errors),
test_translations.py, test_font_coverage.py, and
test_text_area_layout.py all pass.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XyKsqjkUFwsmf7dxp9SYdV
@genixpro
genixpro requested a review from Giszmo September 10, 2026 21:10
@genixpro

Copy link
Copy Markdown
Contributor Author

@Giszmo — this should be ready for another look now. Summary of everything since your review:

Your 4 requested changes:

  1. OpenSSL behind a flag — wss=1 (default on), wss=0 builds TCP-only without OpenSSL/Boost.Beast
  2. debian/control — added libssl-dev
  3. Determinism baselines explained — see the earlier reply on this thread; cross-replay.replay/.checksums track REPLAY_MINIMUM_VERSION_MINOR, which master keeps bumping independently of this branch. Re-recorded again since then at version 94 (was 93), because master's tip (Apportion fetch jobs, and price a loaded candidate instead of refusing it #223) bumped the floor again after my last rebase.
  4. FertilityCalculator kernel loop — hoisted the per-tile coordinate/eligibility work out of the 961-iteration inner loop

Also since your review, the branch turned out to be 5 master commits behind (not the 1 I first assumed), including the Settings (#236) and custom-game lobby (#237) redesigns landing around the same time — full rebase onto master's current tip is up now, with CustomGameScreen/SettingsScreen reconciled to keep your redesigns and re-apply this branch's ScreenStack architecture on top. Also fixed a stale CI path, a decade-old literal merge marker sitting in texts.br.txt on master itself, and translated 39 new UI strings across all 32 non-English catalogs that a newer master-only test now checks.

Native client and server both build clean locally, headless run is deterministic, and all four translation checks pass. CI is running now on 7662aec13 — should be visible on the PR by the time you look.

@Giszmo Giszmo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My judgement is colored by wanting this to be a thing on globulation2.org so I don't give approval but I hope @stephanemagnenat does ;)

genixpro and others added 6 commits September 10, 2026 17:25
CI caught what local scons/server builds didn't: none of the explicit
harness targets (custom-setup-test, session-test, etc.) build under a
bare `scons`, so the earlier rebase verification never compiled them.

- test/CustomGameSetupHarness.cpp: fix six ScreenStack constructions
  left argument-less by the earlier mechanical patch (ScreenStack has
  no default constructor); rewrite the driver-based UI test to push
  CustomGameScreen onto a real ScreenStack and drive it via
  ScreenStack::execute(), matching SinglePlayerFlow::custom(), since
  Engine::initCustom(void) no longer exists.
- Match speed selection was lost in the CustomGameScreen merge:
  master's redesigned lobby has CustomGameScreen::selectedSpeed(), but
  neither SinglePlayerFlow::custom() nor the old Engine::initCustom(void)
  it replaced actually applied it. Added an optional speed parameter to
  Engine::initCustomTask() (sets/restores previousCustomSpeed, mirroring
  the removed method) and wired it through SinglePlayerFlow and the test.
- test/EngineSessionHarness.cpp used SettingsScreen::OK, an enum from
  the pre-#236 widget-based screen this branch never knew about; master's
  redesign has no such enum. Replaced with a direct done() call.
- SettingsScreen::done() closed synchronously once local writes
  succeeded, never actually waiting on the browser persistence flush
  added during the rebase reconciliation — contradicted this exact
  session-test assertion ("Settings must poll persistence before
  closing"). done() now always confirms durability via persist() and
  only calls endExecute() once any pending flush resolves; onTimer()
  drives that completion (or reopens editing on a flush failure).

Verified: all 24 harness targets CI builds compile clean locally, and
engine-session-test (including "settings close only after persistence
completion") and custom-setup-test both pass.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XyKsqjkUFwsmf7dxp9SYdV
CustomGameSetupHarness (4 invocations) and BuildingExpelHarness were
still on the pre-build_layout.py path, missed by the earlier
trapped-unit-test fix and the merge that introduced these two steps.
Audited every remaining harness path in the workflow against the
build_layout.py <toolchain>/<role>/<mode> scheme; no others were stale.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XyKsqjkUFwsmf7dxp9SYdV
…irty

CI's browser job failed 28 Playwright specs, almost all with the same
symptom: the very first "open Settings, close it untouched" in a fresh
profile never returns to MainMenuScreen (30s timeout). The one directly
relevant spec that passed ("settings wait for durable storage before
closing") deliberately stalls the storage write before ever attempting
a real one — every spec that lets a real, unstalled first close happen
hangs, including specs about unrelated screens/flows that just happen
to touch Settings once during setup.

Master's persist() only calls settings.save()/saveKeyboardLayout() when
the corresponding dirty flag is set. The original (pre-redesign)
savePreferences() this replaced always called them unconditionally.
Every other caller of persist() (commit(), finishInteraction(), the
onTimer retry path) already has at least one dirty flag set by the time
it calls persist(), so gating never mattered there — done() calling
persist() on a completely untouched screen was the one path this
skipped real writes on, and it's exactly the failing scenario. Also
restored the try/catch the original had around this whole sequence,
which persist() had dropped.

I can't run the actual browser/Playwright suite from here to confirm
this is the full fix rather than a partial one; pushing it to let CI
verify.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XyKsqjkUFwsmf7dxp9SYdV
The 28 failing browser Playwright tests all shared one symptom (Settings
never returned to MainMenuScreen), which I first misdiagnosed as a
persist() bug. Running the actual Emscripten+Playwright toolchain locally
(rather than guessing from CI logs) showed persistStorage() was never even
being invoked: the tests click stale pixel coordinates left over from
before master's Settings redesign (#236). A screenshot of the real 1200x900
layout put "Done" at (1015,777), nowhere near the tests' (600,650).

Root-caused via direct instrumentation (Module.persistenceResults,
Module.storage.state) and confirmed by clicking the real coordinate.

While fixing the coordinates I found a genuine behavioral gap the redesign
introduced: the old screen had a Cancel/Continue button that always closed
in one click, independent of the save outcome; the new footer only had
"Done" (gated on success) and no equivalent. Added it back as
SettingsScreen::abandon(), wired to an always-visible footer button that
relabels itself "Continue" once a save has actually failed, reusing the
existing "[settings continue]"/"[settings Cancel]" translated strings.
Verified empirically: Done still blocks while failed and retries on every
click; the new button never retries and always closes in one click,
matching storage.spec.js's single-click "restore failure" test and
settings-storage.spec.js's "continue after failure" test.

Added a formula-based clickSettingsDone/clickSettingsCancel helper to
main-menu.js mirroring SettingsScreen::layout()'s panel/footer math, so the
click position tracks the panel across any viewport instead of a hardcoded
pixel. Verified against real screenshots at 1200x900, 900x650 and 1100x700.

Also fixed a test-design bug this surfaced: the redesigned screen auto-
saves on every change, so settings-storage.spec.js's fault-injection tests
were injecting the storage fault *after* the dirty click, letting the
write land before the fault ever applied. Moved fault injection before the
change in both affected tests.

Added pixels.js:hasDarkText for the redesigned screen's dark-text-on-paper
footer status line (the existing hasLightText assumed the old dark-panel/
light-text styling).

Verified locally with the full Emscripten/Playwright toolchain:
- settings-storage.spec.js: 5/5 pass
- shutdown-storage.spec.js, storage.spec.js's restore-failure test,
  viewport.spec.js's Settings-touching tests: all pass
- Native release build (scons release=1) and web build (scons target=web
  release=1) both clean.

CustomGameScreen (master's other redesign, #237) has the same class of
stale-coordinate problem in single-player.spec.js, storage.spec.js and
rendering.spec.js; that's unstarted and tracked separately.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XyKsqjkUFwsmf7dxp9SYdV
Continuing the same investigation as the Settings fix: the CustomGameScreen
tests failed for the same reason (stale coordinates from before master's
lobby redesign, #237), but digging into the one test I couldn't fix with a
coordinate swap turned up a real crash bug and a real launch bug, both now
fixed and covered by tests.

Bug 1 - AI profile picker crashes the browser tab:
CustomGameScreen::showAIProfile() (wired to each colony's "Info" button)
opened its picker via the old choose()/Screen::execute() blocking-loop
pattern. The browser host has no Asyncify, so ApplicationHost::wait()
inside that loop is a hard error there (docs/browser/adr-003-screen-
execution.md) - clicking Info threw an uncaught exception that unwound
past the scheduled-stack driver, freezing the page on that screen with no
further input reaching it. #237 added this button without a browser build
to test it against; #203 is the first PR that makes CustomGameScreen
reachable in one. Fixed by pushing CustomGameChoiceScreen through the
ScreenStack CustomGameScreen already holds, same as every other dialog
this screen opens, instead of blocking-executing it. choose() is now
unused and removed. Verified with page-error listeners: no crash, correct
CustomGameScreen <-> CustomGameChoiceScreen transitions, and the picked AI
now visibly sticks (confirmed via screenshot: colony row updates from
"Numbi - Easy" to "Warrush - Medium" after Use).

Bug 2 - launching a randomly generated map fails to load:
Engine::initCustomTask(MapHeader, GameHeader, ...) never forwarded a
source file path to initGameTask, so GameGUI::loadFromHeaders fell back to
deriving one from the map's display name. For a premade map that
coincidentally matches a real file (maps/FourSquares1.map), so it worked
by accident; for a generated map ("Random map" -> maps/Random_map.map,
which never exists) it always failed. Fixed by threading
CustomGameScreen::sourceFile() (already tracked, just never passed
through) through initCustom/initCustomTask into initGameTask's
sourceFileName, in both SinglePlayerFlow::custom() and the harness driver.
This also makes premade-map loading correctly use the actual selected
file instead of a name-based library guess, rather than working by luck.

Verified locally:
- CustomGameSetupHarness's ui-mode driver (SDL-event-driven, exercises the
  Info button and a random-map launch end to end) now passes for all three
  controller modes; previously failed identically with or without the
  showAIProfile fix, confirming it's Bug 2, not a regression from Bug 1's
  fix. All of the harness's other CI-invoked modes still pass.
- Native release build, server build, and the web build all clean.
- Full local Playwright run across every previously-failing CustomGameScreen
  test (single-player, storage, rendering, viewport, import, input,
  replay-save, session-reload): 35/35 pass.

Added CustomGameScreen::start()'s footer button as clickCustomGameStart()
in main-menu.js (mirrors renderLobby()'s button rect, verified against a
screenshot) and used it everywhere a test just needs to launch the
preselected default map, replacing the old two-click stale-coordinate
sequences. Rewrote single-player.spec.js's AI/rules test around what the
redesigned lobby actually exposes: "other options" are now inline Game
Rules rows (no separate screen to navigate to and back from), and the AI
picker is exercised for real now that it doesn't crash - this doubles as
the regression test for Bug 1.

import.spec.js's "imports a custom map ... through the normal setup
screen" test still needs attention separately: the redesigned
CustomGameScreen has no import affordance at all anymore (that lives only
in ChooseMapScreen's "Load" flow), so the test's premise - importing
directly into the lobby - has no current equivalent to test. Left
unstarted pending a decision on whether that's a feature gap to close or
a flow the test should follow into ChooseMapScreen instead.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XyKsqjkUFwsmf7dxp9SYdV
…bby button

CustomGameScreen has no import affordance anymore after #237's redesign
(map browsing there is now "Premade maps"/"Your maps" library tabs, not a
file picker) - only ChooseMapScreen still has one, and that screen only
offers map-type import when the editor's "Load Map" opens it. Rather than
add a new lobby affordance or drop coverage, route the test through the
path that actually exists: import via the editor's map chooser, back out
without loading it into the editor, then pick it up from the custom-game
lobby's own "Your maps" library and start a match with it. Verified the
full round trip locally, including that the newly-imported file shows up
correctly in the lobby's map preview before starting.

Full local Playwright run across import/input/replay-save/session-reload
(11 tests, the remaining files touching CustomGameScreen): 11/11 pass.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XyKsqjkUFwsmf7dxp9SYdV
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.

2 participants