EstimateActionPointCost adds the post-fence start-run flat at the
fence tile; real spend adds it on the next tile, with the diagonal
x1.4 - cursor read 1 AP low. Sum ActionPointCost direct instead, and
mark the fence landing non-running so the next tile re-charges
start-run itself: right tile, right multiplier.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The footstep-colour budget kept its own inline per-mode copy of the
movement-cost modifiers (walk/crawl/swat/run), the last duplicate of the
cost math left in PlotPath. Replace it with one EstimateActionPointCost
call per stance, so the reachability colours use the same per-tile cost
as the real spend, and drop the now-orphaned per-tile TerrainActionPoints
recompute and its dead locals (sTileCost, sMovementAPsCost,
sExtraCostStand).
Cosmetic-only: footprint colours now include the diagonal x1.4 the old
budget omitted, so they track the real reachable distance more closely.
The water->walk terrain override stays; it keeps the start-run charge
correct when a run path crosses water.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
PlotPath carried its own inline copy of the per-tile movement-cost math,
which had drifted from ActionPointCost (the function the real per-step
movement and the AI path estimate both already use). That divergence is
what made the movement cursor mis-predict the real AP spend - e.g. the
start-run penalty getting the diagonal x1.4 in the real cost but not the
cursor.
Replace the inline switch (and its fence/start-run special-cases) with a
per-tile EstimateActionPointCost call, threading the simulated previous
tile mode so the one-time start-run charge lands exactly once. The cursor
estimate now equals what movement deducts, by construction.
Behavioural: player movement-cursor AP numbers (and path reachability
colouring that keys off the same total) now match the real spend. Real
per-step deduction and AI estimates are unchanged - they already used
ActionPointCost. The footstep-colour budget still has its own cost copy;
left for a follow-up.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Forwards to the new ActionPointCost prev-mode overload so a path estimator
can pass its simulated prior tile mode. Existing 6-arg callers use the
soldier's live anim state and are unchanged.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add an ActionPointCost overload taking the previous tile's movement mode
explicitly, used to charge the one-time start-run penalty. The existing
4-arg overload forwards the soldier's live anim state, so real per-step
movement and every other caller are unchanged. This lets a path-cost
estimator - which does not move the soldier - supply its simulated prior
mode instead of reading a frozen live anim state, so the estimate and the
real deduction can share one cost function.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The /libpath flags for the MSVC, UCRT, and Windows SDK x86 libraries
were only injected into CMAKE_EXE_LINKER_FLAGS_INIT, so linking a shared
library or module produced unresolved externals. Hoist the paths into a
local variable and feed it to the EXE, SHARED, and MODULE init flags.
Never bit because nothing in the tree builds a shared lib yet.
Select the clang-cl-asan CMake preset (RelWithDebInfo, clang-cl,
ADDRESS_SANITIZER=ON) to instrument first-party code with AddressSanitizer.
The wiring lives in cmake/AddressSanitizer.cmake; SANITIZERS.md tells how to
add the clang-cl tools, build, and read the report.
Details:
- Add the clang-cl-asan preset so the asan build is one selection in Visual
Studio, and a base for a CMakeUserPresets.json to inherit.
- Instrument first-party code only; the vendored libraries keep default flags.
- Use the release CRT and disable MSVC-STL container annotations, so
instrumented and un-instrumented TUs stay compatible.
- Pass /bigobj to the TUs asan inflates past the COFF section cap.
- Link the asan runtime for clang-cl (lld-link does not infer it).
- Stub Bink into the exe: retail binkw32.dll cannot load in an asan process
(its image base is the 32-bit shadow), so compile no-op exports instead.
- Route the asan report to gamedir/asan.report.<pid>, since every app is a
WIN32 GUI app with no console to receive the default stderr report.
- Opt functions with 32-bit inline __asm out of instrumentation with
cmake/asan-ignorelist.txt, one function at a time (asan reserves a register
the asm needs). The rest of each translation unit stays instrumented.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
When no cardinal direction resolves, ubDirection stays 255 and the
tail feeds it to GetSectorMvtTimeForGroup, which reads
SectorInfo[...].ubTraversability[255] on a UINT8[5]. This runs in
release too, since that read is a plain call, not an assert. Bail out
instead.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
gfEnteringMapScreenToEnterPreBattleInterface, gfAutomaticallyStartAutoResolve
and gfDelayAutoResolveStart are now file-static in PreBattle Interface.cpp
(the latter's definition moved here from Strategic Movement.cpp). External
access goes through accessors declared in the header; PBI.cpp touches the
statics directly.
Accessors exist only where an external caller needs one:
AutomaticallyStartAutoResolve() getter (read in Town Militia,
gamescreen, Player Command); SetAutomaticallyStartAutoResolve (set in
Creature Spreading, strategicmap); SetDelayAutoResolveStart (set in
Strategic Movement);
SetEnteringMapScreenToEnterPreBattleInterface (set in strategicmap).
gfEnteringMapScreen stays a raw cross-read into mapscreen state, not
PBI-owned. No behavior change: getter returns BOOLEAN.
Verify: grep the three flag names across the tree hits only PreBattle
Interface.cpp (three static defs, four accessor bodies, and in-file
Handle/reader access). No header externs, no external raw refs. Build: JA2.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The flag was initialized FALSE (mapscreen.cpp), read once in
HandlePreBattleInterfaceStates, and only ever set FALSE — never TRUE anywhere
in the tree. Its else-if branch was unreachable and its body did nothing but
re-clear the always-false flag.
Verify: grep 'gfTransitionMapscreenToAutoResolve' across the tree now hits
nothing. Build: JA2.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
gfPreBattleInterfaceActive and gfUsePersistantPBI are now file-static in
PreBattle Interface.cpp, reached only through Is/Set accessors declared in
the header. No behavior change: getters return BOOLEAN so ==TRUE/==FALSE
sites are untouched semantically.
Verify: grep 'gfPreBattleInterfaceActive\|gfUsePersistantPBI' across the tree
hits only the two static definitions, the four accessor bodies (all in
PreBattle Interface.cpp), and two commented-out Asserts in Strategic
Movement.cpp. Every other former site now calls an accessor. Build: JA2.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* prepare gamedir for running the game
* Stop telling installers to overwrite the vanilla game
The release no longer contains anything that lands on a vanilla file:
its base data lives in Base, so unpacking it over the game directory
and copying the game's Data into the release now amount to the same
installation.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* Give the mod's base data its own VFS profile
Its files ship in Base now, so that a vanilla Data directory can be
copied in whole without a single collision. Mount Base right above the
vanilla dirs in every profile stack — the slot where the installer's
overwrites used to end up.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* Mount Base in the language overlays' VFS configs too
Assembling a release copies a <Language>_Version directory over
gamedir, its own copies of the configs included, so without the same
Base profile every non-English release would mount nothing from Base.
Their libraries move along with the rest of the base data: each
language slf, and the Russian data.slf that shadows the vanilla
Data.slf by name.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* Ignore a vanilla Data directory dropped into gamedir
Running the game here needs one copied in, and it is an untouched copy
of somebody's retail install — nothing this repository should track.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* Ignore only the player profile directory, not every Profiles
The unanchored pattern also swallowed Base/TableData/Profiles, the
mod's soldier profile XMLs, which are tracked and belong in a release.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* move gamedir/Data to gamedir/Base
* move gamedir-languages' Data to Base
---------
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
sgp.cpp held two of these. One was a file-scope vfs::FileLogger* that was
never assigned, whose only other mention was a delete in shutdown that
could therefore never fire; it goes. The other is the adapter VFS actually
logs through, which was a bare new that nothing freed.
Make that one a function-local static rather than a scoped object. VFS
keeps the bare pointer and still logs from the shutdown that atexit runs
after WinMain has returned, so the adapter has to outlive the frame it is
declared in. Constructing it before InitializeStandardGamingPlatform
registers that handler is what puts its destructor after the handler
rather than before it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Nothing Chromium-derived is left in either file - the stack tracer that
came from base/debug_util was the last of it - so the copyright header
credited Google for code it did not write and pointed at a LICENSE file
this repository does not have. Rename to what the files actually are.
While in there: say what the exception code's customer bit does and does
not guarantee, and note next to it how to stop a debugger breaking on
every assertion, which is what someone will be looking for when they grep
that number.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ENABLE_STACK_TRACE has been 0 for as long as the file has been here, so
StackTrace's constructor captured nothing and every line it ever wrote to
stack_trace.log was a bare message with an empty frame list behind it. The
DbgHelp singleton underneath it resolved symbols for that empty list, and
the game linked dbghelp.lib to do it.
The crash reports cover what this was meant to cover, and the VFS errors
that were its only real content are already in vfs.log and game_log.log.
Drop the tracer, the log, and the dbghelp dependency.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
_ExceptionMessage builds the full call stack of the exception it was given
and then calls _FailMessage with "",0,"", so every caught sgp:: or
vfs::Exception produces an identical, locationless report - the bucket
telemetry will see most of and can act on least. Fail with the innermost
frame instead, which is the frame that knows where it came from.
That feeds a runtime-built string to a _FailMessage that passed it to
sprintf as the format string, so copy it bounded instead. AssertMsg call
sites already build messages out of game state, and a %s in one of those
would read arguments that were never passed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Assertions ship in every configuration - Ja2/builddefines.h defines
FORCE_ASSERTS_ON unconditionally - so a player on a release build hits the
runtime error screen with a line and a file, and that is all anyone ever
gets. The crash handler that would have written a report never runs,
because an assertion faults nothing.
Raise a software exception from _FailMessage so it does. The code has the
customer bit set, the handler recognizes it, and the report carries the
assertion's line, file and message alongside the usual registers, module
table and frame chain, which symbolizes back to the assertion site. The
exception is swallowed again immediately: first-chance is all the handler
needs, and letting it travel further would kill a game that means to show
its error screen.
The dumper needed two adjustments to cope with an exception nobody faulted:
its one-report-per-address rule keys on the assertion's own file and line,
since every raise shares RaiseException's address, and the re-entry latch
is lifted in the __except, which is the only code that runs if writing a
report faults.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
EvalLua had no callers anywhere in the tree — it converted a wide string
to UTF-8, ran it as a chunk and printf'd the error, which was the hook for
a developer console that is not wired up. It was the only user of stdio,
MemMan and windows.h in this file.
InitializeLua and ShutdownLua are live, called from InitOverhead and
ShutdownOverhead. The calls in InitializeGame and ShutdownGame were
commented out when they moved there; remove them, and the commented
ACCESSOR_TABLE block in InitializeLua that refers to a macro no longer
defined anywhere.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The tree carried two prebuilt Lua static libraries and a copy of the Lua
public headers. lua51.lib is 5.1.2 and matched the headers; lua51.vc9.lib
is 5.1.4 and was dead weight — it came second in Ja2_Libraries, so the
linker resolved every Lua symbol out of lua51.lib and never pulled an
object from it. Just as well, since it asks for /DEFAULTLIB:MSVCRT while
we build /MT.
ext/lua-5.1.5 is the upstream tarball unmodified, built as lua51 the way
the other vendored libraries are, and its src directory replaces lua/ as
the home of lua.h, luaconf.h, lauxlib.h and lualib.h. Those four headers
were stock 5.1.2 retabbed, so 5.1.5 is bugfix-only against what the game
compiled against; the bytecode format is unchanged across 5.1.x and every
script under gamedir is plain source anyway. lua/lua.hpp had no includers
and returns to etc/ where upstream keeps it.
/SAFESEH:NO goes with it. Its two stated reasons were lua51.lib and the
smackw32 import library, and both are now gone: every remaining prebuilt
static library is SAFESEH-clean (libexpatMT.lib 5 of 5 members with
@feat.00 = 0x1, RakNetLibStatic.lib 79 of 79), and lld-link emits a
3155-entry SEHandlerTable without it.
The /MT comment blamed the wrong library. lua51.lib carried no linker
directives at all; RakNetLibStatic.lib is what pins us to the static
runtime, with /DEFAULTLIB:LIBCMT and /DEFAULTLIB:libcpmt.
Verified by building all four applications in Debug and Release, and by
linking the tarball's own lua.c against our lua51.lib with the build's
clang-cl flags and running it under Wine: 5.1 stdlib, GC, coroutines and
the x86 __asm fld/fistp lua_number2int fast path all behave. The game
itself could not be launched here — this checkout's gamedir has loose
Data directories but no SLF archives, so VFS aborts on Data\Ambient.slf
long before any script runs.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The assemble job only ran on Windows for the case-insensitive filesystem, which
the overlays no longer depend on. Checking out the ~93000 game data files is
most of what the job does, and ubuntu runners are quicker and cheaper at it.
Naming the executable has to be exact now: the artifact is named after the
matrix entry, which is lowercase, while the executable inside it is named after
the CMake target, which is the uppercased application. Only NTFS was making
those agree.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Nothing stops the inconsistency the previous commit cleaned up from coming
straight back. The game reads its data through FileMan, which hands every path
to VFS, and VFS compares paths case-insensitively (vfs::Path::Less ->
vfs::String::less). A file added under Data/Mercedt/ therefore works perfectly
well beside Data/MercEdt/ right up until a release is assembled on a
case-sensitive filesystem and both of them survive into the archive, at which
point the game serves whichever the directory yields first.
Directories count as much as files: a translated-only file below Data/Mercedt/
collides with Data/MercEdt/ even when no file does, so each overlay is merged
onto gamedir the way a release does and every path prefix is compared.
Runs on pull requests only, and in its own workflow rather than in build.yml,
which release.yml also calls: a check has no business running again while a
release is assembled. It reads the git index rather than the files, so a
blobless sparse checkout is enough and no game data is fetched.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The overlays were authored on Windows, where a filesystem that folds case hid
the fact that thousands of translated files spell their path differently from
the gamedir file they are meant to replace: Data-1.13/Mercedt/170.EDT over
Data-1.13/MercEdt/170.EDT, 149_ATTN.wav over 149_ATTN.WAV. Copying an overlay
onto gamedir on a case-sensitive filesystem leaves both files instead of
replacing one, and VFS, which folds case itself, then serves whichever the
directory happened to yield first.
Every overlay path is now spelled component for component the way gamedir
spells it. gamedir was already self-consistent, so nothing there moves and no
new convention is invented. The transformation is mechanical and no file gains
or loses content:
git diff --shortstat HEAD~1 HEAD
5374 files changed, 0 insertions(+), 0 deletions(-)
git diff --name-status -M HEAD~1 HEAD | cut -f1 | sort -u
R100
git ls-tree -r --name-only HEAD gamedir-languages | wc -l
28884, the same as before
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The Smacker header declares a max audio chunk size, and libsmacker sized its
output buffer from it — then wrote each frame's audio trusting the frame's
own unpacked size, unchecked. The fan-localized intro videos (Chinese among
others) declare max_buffer=2304 but carry ~97KB audio frames: every decode
was a heap overflow, crashing the intro. The original SMACKW32.DLL played
these files, so treat the per-frame size as truth and grow the buffer,
bounded by a 16MB sanity cap; a chunk beyond that fails the frame as corrupt.
Covers both the raw-PCM and DPCM paths.
Verified with an ASan/UBSan harness over all 19 vanilla and Chinese intro
SMKs: previously all 8 Chinese files faulted, now all decode both passes
clean.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A "build local" report has no released PDB behind it — the telemetry sink
cannot symbolize it. Skip these when draining reports at startup: not sent,
not reaped by the 30-day cleanup, left on disk for the developer.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
These triggered source's release workflow on gamedir pushes. In the
monorepo a push to master triggers it directly, and GitHub ignores
workflow files outside the repository root anyway.
gamedir and gamedir-languages now live in this repository, so the build
no longer clones them separately or pins their commits in versions.env:
one commit SHA identifies everything. The compile job sparse-checks-out
the source only, and the assemble job the game data only, both with
blob:none so neither downloads the other half. Dist names drop the
G....L.... gamedir suffixes since GAME_VERSION now covers the data too.
Full history of majcosta/gamedir-languages imported via git filter-repo
--to-subdirectory-filter. Blobs unreachable from its HEAD whose content
is binary (11847, NUL-byte sniff as git does it) were stripped; text
file history is kept in full. Working tree content is identical to
gamedir-languages HEAD; verify with:
git diff --stat gl/master HEAD -- gamedir-languages/
Full history of majcosta/gamedir imported via git filter-repo
--to-subdirectory-filter. Blobs unreachable from its HEAD whose content
is binary (37314, NUL-byte sniff as git does it) were stripped; text
file history is kept in full. Working tree content is identical to
gamedir HEAD; verify with:
git diff --stat gd/master HEAD -- gamedir/
Fix case of WinFont.h include in WinFont.cpp
Fix case of "Strategic Status.h" include in Queen Command.cpp
Fix case of "mapscreen.h" include in Strategic Merc Handler.cpp
Fix case of "Handle Items.h" include across Tactical/TileEngine/Utils
Mechanical: normalized every #include of handle items.h (any case) to
match the actual filename "Handle Items.h". Verify with:
grep -rn "handle items.h" -i --include=*.cpp --include=*.h . | grep -v "\"Handle Items.h\""
Fix case of "World Items.h" include across Tactical/Editor
Mechanical: normalized every #include of world items.h (any case) to
match the actual filename "World Items.h". Verify with:
grep -rn "world items.h" -i --include=*.cpp --include=*.h . | grep -v "\"World Items.h\""
Fix case of "Arms Dealer Init.h" include in Overhead.cpp
Fix case of "Meanwhile.h" include in TeamTurns.cpp
Fix case of "timer.h" include in Utils All.h and Event Pump.cpp
Fix case of "Store Inventory.h" include in XML_Items.cpp
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Frame pointers on: dbghelp's stack walker gets usable frames in crash
reports. /Oy- rather than -fno-omit-frame-pointer -- clang-cl rejects
the GNU spelling:("unknown argument ignored"), and cl.exe takes /Oy-
too. x86-only option; a 64-bit target would ignore it and rely on
unwind data instead.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
OBJECTTYPE::exists() and SOLDIERTYPE::exists()/DeleteSoldier() are
called on null pointers by design and guard with `this == NULL`. That
is undefined behaviour, so clang infers `this` is non-null: it deletes
the guard inside the callee *and* deletes null checks that follow a
call in the caller, which is an access violation at /O2 in code MSVC
has always compiled the naive way. The caller-side inference happens
in every translation unit that calls one of these, so this has to be
global rather than per-file; there is no per-function attribute or
pragma for it. Drop it once nothing relies on a null `this`.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The client would send up to 256 KB and the sink answered a settling 400 above
64 KB, so the two disagreed about what a valid report is, and the client
deletes what it is told is invalid. Unreachable in practice — the module table
and backtrace are both bounded, which puts the ceiling near 10 KB — but the
two constants have to agree for the disagreement not to matter.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The endpoint is public and unauthenticated, so the whole uploaded file is
attacker-chosen, not only the part the client copied from Ja2.ini. The build
field sat inside backticks a backtick closes, and the access-violation text
went in raw, so either could carry markdown or a link into the channel.
One clean(): printable ASCII minus what Discord reads as markup or a URL,
length-capped. It replaces the handle's own stripping and absorbs the .trim()
the field getters did, which also drops the CR that "(.+)$" captures off a
CRLF report.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The module table is what makes a report symbolizable, but a full path also
carries the player's Windows account name and wherever they keep the game,
and the report leaves their machine. Symbolizing only ever matched on module
name and base — symbolize_crash already ran every path through baseName()
before printing it — so the directory was read by nothing.
Say so in the consent prompt too. "No personal information" was not true of
a table full of paths, and is only worth claiming if it holds.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Adds tools/symbolize_crash.cpp, built alongside the game as a console exe:
it reads a crash_report_*.txt written by writeExceptionBacktrace and prints
the backtrace with function names, source lines and inlined frames, in call
order, coloured when standard output is a console.
Symbols come from DbgHelp against the build's PDB rather than an external
symbolizer. SymLoadModuleEx takes the runtime base out of the report's
module table, so the relocation arithmetic /DYNAMICBASE forces on us is
DbgHelp's problem now; a report predating the module table loads at the
image's preferred base, which is where it ran. Verified under Wine: PDB
line info and inline traces both resolve.
The tool wants C++23 (std::print, std::format) where the game is C++17, so
the standard is set on the target alone. Both MSVC 19.51 and clang-cl build
it clean in all three configurations.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The other half of the feature, and the half that decides whether a player's report
survives: the endpoint CRASH_TELEMETRY_URL points at. It takes the POST from
processCrashTelemetry and forwards the report to a Discord webhook as a file
attachment. It stores nothing -- a report is only worth reading next to the PDB it
was built against, and that never leaves a developer's machine, so there is nothing
for a bucket to do here that the channel we already read bug reports in does not do
better. That also keeps the whole thing inside the Workers free tier, where the
10 ms budget is CPU, not wall clock, and waiting on Discord is not metered.
In the repo rather than in someone's home directory because the status codes are a
contract with the client and the two have to be changed together. reportIsSettled()
in sgp/crash_telemetry.cpp deletes the player's copy on 2xx and on 400/413/415 and
keeps it on everything else, so a settling 4xx returned for a failure on our side
silently destroys the report. Every failure path here is therefore a 503, each
naming its own cause in wrangler's console, and the one 4xx that is safe -- 429,
which the client does not settle -- is the one the rate limiter returns.
That rate limiter is a binding with a .limit() call, not a dashboard rule: WAF rate
limiting rules need a zone and a workers.dev subdomain is not one. Per-IP, 50 a
minute, which has to clear kMaxUploadsPerRun (20) in the client or a player draining
a backlog throttles themselves. It is checked before the body is read.
A report is attacker-controlled text arriving at a public, unauthenticated endpoint
whose URL ships in every player's Ja2.ini, so the summary line strips markdown from
the player handle and the payload sets allowed_mentions to nothing. The size cap and
the "*** CRASH" check keep drive-by scanners out; anything determined gets through,
and the blast radius is a message we delete.
test.mjs covers the whole contract against a stubbed fetch, no network and no
webhook needed. DISCORD_WEBHOOK is a secret and lives nowhere in this tree;
.dev.vars, which holds a live one for local development, is gitignored.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A crash report is worth nothing sitting on the player's disk. On startup, drain
the crash_report_*.txt files the handler left behind to the endpoint named by
CRASH_TELEMETRY_URL in Ja2 Settings; an empty or absent key turns the feature off
entirely. The first launch asks the player once and remembers the answer in
telemetry.consent -- declined means the reports simply keep accumulating locally.
This lands in its own translation unit rather than in more of debug_win_util.cpp.
Everything in that file runs inside a faulting thread and may not allocate;
everything here runs at startup with a healthy heap and is ordinary code. Two
files keep the no-heap rule easy to see and easy to hold.
The draining runs on a detached thread. The uploads are synchronous WinHttp calls
with seconds-long timeouts, and this sits on the startup path, so on the main
thread an unreachable endpoint is a stall the player watches before the splash
screen. Nothing waits on the result: if the player quits first the process exits
from under the thread, which costs nothing, since an interrupted upload leaves the
file on disk and it goes out next launch. The consent prompt stays on the main
thread on purpose -- it is a question, and a question has to be asked before
anything is sent.
Which reports get deleted is chosen so that a mistake cannot destroy them. A file
goes away on 2xx, and on 400/413/415, i.e. content the server will never accept.
Everything else keeps it: no connection, 5xx, and notably the 403/404 of a
mistyped CRASH_TELEMETRY_URL, which would otherwise silently eat every player's
crash history. Bounds all round: every WinHttp phase has a timeout, a report over
256 KB is not one of ours and never goes on the wire, at most 20 uploads per
launch so a crash-looping build cannot turn startup into an upload session, and
reports older than 30 days are dropped unsent.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Report crashes for every player, not only those running under Wine's WINEDEBUG.
A last-chance UnhandledExceptionFilter is no good: faults on the message-pump /
WindowProcedure path have no game __except on their stack, and under Wine the
WndProc dispatch swallows them before they ever reach "unhandled". Install a
vectored handler instead -- it runs first-chance, ahead of every frame handler,
records the fault and returns EXCEPTION_CONTINUE_SEARCH, so normal SEH is
unaffected.
The dump is deliberately heap-free. The crash most worth reporting is often heap
corruption, so anything that allocates (the game Logger, std::vector, DbgHelp's
Sym* family) would fault again and take the report down with it. Using only stack
buffers and raw Win32, writeExceptionBacktrace writes the registers, the faulting
address, a UTC timestamp, the build id, the loaded-module table and a
bounds-checked manual EBP walk into a fresh numbered crash_report_NNN.txt.
The module table is what makes the addresses mean anything. Our executables are
linked /DYNAMICBASE and keep their .reloc section, so the loader is free to move
the image: Wine leaves it at the preferred base, Windows ASLR does not. A report
listing only runtime addresses is symbolizable by luck, and silently wrong once
the luck runs out. Recording where each image actually landed turns the offset
into arithmetic:
llvm-symbolizer --obj=JA2.exe --adjust-vma=$((<JA2.exe base> - 0x400000))
It is walked off the PEB loader list because both alternatives -- EnumProcessModules
and DbgHelp's module APIs -- allocate, and this path must not. Having the table
also frees the backtrace from restricting itself to our own module's return
addresses: a fault inside ddraw/fmod/bink is exactly the case worth seeing, and
an address belonging to no module is recognizable as the frame-pointer debris it
is.
Two things go into the report beside the machine state. czVersionString stamps
the build, so a report matches the exact PDB it has to be symbolized against, and
the optional HANDLE from Ja2 Settings names the player, so a report can be tied
to whoever raises it with us. The handle is sanitized where it is set rather than
where it is used: it is player input that lands in a line-oriented text report,
so anything that could forge a line (CR/LF, control and non-ASCII characters) is
dropped and the length is capped.
Finally the player is told. The message is composed in the handler, while the
fault details and the report's filename are still in hand, but shown from
SGPExit: first-chance means the exception may yet be handled downstream, and a
message box pumps messages, which on the heap that just faulted is a second
crash. It replaces the generic "Unhandled exception. Unable to recover." box
rather than adding to it -- it says more, and it names the file we need attached
to the bug report.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
