Gitea #38 ROOT CAUSE + FIX: mech paint lost on geometry re-load (view toggle / respawn)

Found from the user's live 2-node demo: a Crimson MadCat went GREY in its own
view after a V (inside/outside) toggle, with NO new [paint] or
MakeMechRenderables line in the log -- so no rebuild, just a re-parse.

ROOT CAUSE: the per-pilot colour/badge/patch is applied by rewriting MATERIAL
NAMES while a BGF parses, and only while the substitution callback is installed
(SetupMaterialSubstitutionList .. TearDown, bgfload.cpp:15-18).
BTL4VideoRenderer::ApplyViewSkeleton re-parses every shown segment BGF on each
view toggle AND on respawn (that is what its fresh graphic-state read is for),
but did so OUTSIDE that bracket -> raw %color% placeholders -> unpainted
materials -> grey.  Nothing caches the geometry (d3d_OBJECT caches only
textures), so every call re-parses.  This is #38's mechanism: 'colors not
preserved on respawn' was never a replication or teardown bug.

FIX: re-install the substitution list around the reload, reusing the serial the
mech was BUILT with -- SetupMaterialSubstitutionList advances the global %serno%
per call, so a naive re-bracket would stamp a different serial and still resolve
the wrong names.  MechRenderTree gains paintSerno (captured before Setup at
build); ApplyViewSkeleton installs/restores around the loop and logs it.

Rig-verified: the crimson MadCat stays crimson (yellow patch intact) across
repeated inside/outside toggles; each toggle now logs '[paint] color=red
serno=0' + '(paint serno 0)'.

ALSO -- corrections to yesterday's #44 name plate from the adversarial pass:
 - kPlateARGB 0xFF808080 -> 0x80FFFFFF.  BMAP.BMF tag 0x0027 is
   dpfB_MATERIAL_OPACITY_TAG [T0 libDPL/dsys/PFBIZTAG.H], NOT a diffuse colour:
   the materials carry NO colour tag at all, so the plate is the unmodulated
   callsign raster at ~50% opacity, not a grey label.  (Two independent
   workflows converged on this.)
 - the plate's K = 1/2.8 is NOT the hotbox's constant (2.8145) -- de-unified.
 - the Lock attribute is HUD attr id 10, an int/Logical*, not a Scalar*.
 - the PNAME pair split is the V half, not U (our per-player texture is one
   128x32 cell, so the port quad samples v 0..1).
 - retracted bgf-format's 'tag 0x0027 likely SPECULAR [T4]'.

KB: new reconstruction-gotchas section on the whole bug class (load-time-only
state must be re-installed on every RE-load, incl. the serno trap).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0166KTsC7ADm7VXEi1HF1jNg
This commit is contained in:
arcattack
2026-07-24 19:06:04 -05:00
co-authored by Claude Opus 5
parent 35c750dc7c
commit 6c3fca2f67
5 changed files with 124 additions and 19 deletions
+7 -4
View File
@@ -111,10 +111,13 @@ the PLACE 2×2 grid. Corollaries:
- The placeholder cells are stored **scanline-reversed** vs the compositor's top-down order
(`LoadBitSliceTexture` writes y=0..31 top-down), so a naive PNAME draw gives the RIGHT
number upside-down. Don't chase it — implement the compositor instead. [T1 / T3 fix choice]
- These 6 materials carry **no DIFFUSE/AMBIENT** — only `NAME`, `MATERIAL_TEXTURE` and an
**unhandled tag 0x0027** (12 B = {0.5,0.5,0.5}; only 41 uses in 1618 BMFs, likely SPECULAR
[T4]). `bgfload` therefore falls back to the gray placeholder `0xFFB0B0B8`; force WHITE for
these unlit text plates. And `decodeBSL` emits **alpha=255** for mono slices, so the plate
- These 6 materials carry **no DIFFUSE/AMBIENT/EMISSIVE/RAMP** — only `NAME`,
`MATERIAL_TEXTURE` and tag **`0x0027`** (12 B = {0.5,0.5,0.5}), which is
**`dpfB_MATERIAL_OPACITY_TAG`** — `[T0` `engine/MUNGA_L4/libDPL/dsys/PFBIZTAG.H]`, NOT specular
(that earlier [T4] guess is retracted). So the material contributes **no colour at all**: the
authentic plate is the **unmodulated callsign raster at ~50% opacity**. `bgfload` would fall
back to the grey placeholder `0xFFB0B0B8`; the reticle plate's 2D path therefore draws it as
`0x80FFFFFF` (white, half alpha) — see [[gauges-hud]] §Lock ring. And `decodeBSL` emits **alpha=255** for mono slices, so the plate
draws as an OPAQUE black rectangle — a billboard needs black→alpha-0 keying (which is exactly
what the 2D path's A4R4G4B4 `0xFFFF/0x0000` upload does).
+36
View File
@@ -545,3 +545,39 @@ exposed the stale-cursor blit as an offset ghost. LESSON: when transcribing a d
sequence, verify EVERY vtbl call in the decomp is present -- and test the REDRAW/second-use
path, not just first render. (Live-pinned by the operator's exact repro; bisect showed it
was never a regression -- it shipped with the feature.)
## Load-time-only state must be re-installed on every RE-load (issue #38, 2026-07-24)
**The bug class:** a piece of state is installed around a resource load, mutates the load's
*output*, and is then torn down — so any LATER re-load of that resource silently produces a
different (wrong) result. Our port re-loads things the 1995 engine loaded once, which is
exactly where this bites.
**The archetype — mech paint.** The per-pilot colour/badge/patch is applied by REWRITING
MATERIAL NAMES while a BGF parses: `SetupMaterialSubstitutionList(entity)` installs the
callback (`dpl_ApplyMaterialNameCallback`, `engine/MUNGA_L4/bgfload.cpp:15-18`),
`MakeMechRenderables` parses, `TearDownMaterialSubstitutionList()` removes it
(`game/reconstructed/btl4vid.cpp` MechClassID case). But
`BTL4VideoRenderer::ApplyViewSkeleton` **re-parses every shown segment BGF** on each
inside/outside view toggle AND on respawn — and did so OUTSIDE that bracket, so the reload
resolved the raw `%color%` placeholders and the mech rendered **grey**. There is no geometry
cache to hide it (`d3d_OBJECT` caches only TEXTURES, `mTextureCache`), so every call re-parses.
**Why it hid for months:** nothing logs, nothing crashes, and the FIRST build is correct — the
mech is painted right until something re-loads it. Reported as "mech COLORS not preserved on
respawn" (#38) and blamed on replication/teardown for weeks; it is neither.
**The tell:** the symptom appears after a *view change or respawn* with **no new
`[paint]` / `MakeMechRenderables` line in the log** — i.e. the geometry changed appearance
without a rebuild. If a visual property is installed at load time, grep every call site of the
loader, not just the "build" path.
**The extra trap when you fix it:** `SetupMaterialSubstitutionList` ADVANCES the global
`%serno%` counter (`gSerno`) per call, so naively re-bracketing stamps a DIFFERENT serial and
still resolves the wrong material names. The fix must reuse the serial the mech was BUILT with
(`MechRenderTree::paintSerno`, captured before Setup, restored around the re-install so the
global sequence is untouched).
**Rule:** when reconstructing, ask of every load-time-scoped hook — *what happens if this
resource loads twice?* If the answer differs from the first load, either re-install the scope
at every load site or cache the loaded object. Both are legitimate; silently re-parsing is not.