From 6c3fca2f671408a29a026e15474128e1145fc990 Mon Sep 17 00:00:00 2001 From: arcattack Date: Fri, 24 Jul 2026 19:06:04 -0500 Subject: [PATCH] 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) Claude-Session: https://claude.ai/code/session_0166KTsC7ADm7VXEi1HF1jNg --- context/bgf-format.md | 11 +++-- context/reconstruction-gotchas.md | 36 ++++++++++++++ engine/MUNGA_L4/L4VIDEO.cpp | 5 +- game/reconstructed/btl4vid.cpp | 81 ++++++++++++++++++++++++++----- game/reconstructed/btl4vid.hpp | 10 ++-- 5 files changed, 124 insertions(+), 19 deletions(-) diff --git a/context/bgf-format.md b/context/bgf-format.md index 03ae9e8..829f81f 100644 --- a/context/bgf-format.md +++ b/context/bgf-format.md @@ -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). diff --git a/context/reconstruction-gotchas.md b/context/reconstruction-gotchas.md index 2e70af2..d7c5215 100644 --- a/context/reconstruction-gotchas.md +++ b/context/reconstruction-gotchas.md @@ -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. diff --git a/engine/MUNGA_L4/L4VIDEO.cpp b/engine/MUNGA_L4/L4VIDEO.cpp index d760224..1982a4b 100644 --- a/engine/MUNGA_L4/L4VIDEO.cpp +++ b/engine/MUNGA_L4/L4VIDEO.cpp @@ -9941,7 +9941,10 @@ void DPLRenderer::LoadNameBitmaps() // BT: the RETICLE TARGET NAME PLATE's texture (the 1995 PNAMEx.bgf plate art). // The plate meshes are quads UV-mapped onto a shared BITMAP material -- e.g. // PNAME1.BGF and PNAME2.BGF both reference `bmap:name12_mtl` and differ only in -// their u-half (verified: the two files differ in 4 UV bytes) -- i.e. the plate +// their V-half (verified: the two files differ in 4 UV bytes -- the V floats; +// U runs 0..1 in both). Our per-player texture is 128x32 = ONE cell, so the +// port's quad samples v 0..1 -- porting the literal PNAMEn.BGF UVs onto it +// would render half a callsign. I.e. the plate // shows the player's CALLSIGN raster, which on our side is exactly this // per-player 128x32 name texture (4:1, the plate quad's 1.0 x 0.25 aspect). // These load for EVERY mission from StartImplementation, so a cockpit session diff --git a/game/reconstructed/btl4vid.cpp b/game/reconstructed/btl4vid.cpp index 0dbcc3e..5bd354e 100644 --- a/game/reconstructed/btl4vid.cpp +++ b/game/reconstructed/btl4vid.cpp @@ -147,10 +147,24 @@ void // substitution list back down. // SetFogStyle(updateFogSetting); // FUN_0045d3cc(this,0x68) + // REMEMBER the serial this mech's paint is stamped with: gSerno is + // what SetupMaterialSubstitutionList stamps into %serno% and it + // ADVANCES per call, so a later geometry RE-parse (ApplyViewSkeleton, + // which the 1995 engine never did -- it had all skeleton variants + // resident) must re-install the list with THIS serial, not the next + // one. Without it the reload resolves unpainted material names and + // the mech turns grey (Gitea #38). + const char built_serno = gSerno; SetupMaterialSubstitutionList(entity); // FUN_004d0cc0 mech_root = MakeMechRenderables( // FUN_004cef28 entity, model_resource, view_type); TearDownMaterialSubstitutionList(); // FUN_004d11e8 + { + std::map::iterator ti = + mMechRenderTrees.find(entity); + if (ti != mMechRenderTrees.end()) + ti->second.paintSerno = built_serno; + } // NB: the RootRenderable built by MakeMechRenderables registers // itself with the renderer (AddRenderable) and hooks to the entity's // localToWorld in its ctor -- no explicit AddDynamicRenderable here @@ -1872,17 +1886,21 @@ void // dpl_TranslateDCS(plate, K*reticlePos.x + -0.0f, // K*reticlePos.y + -0.08f, -1.0f); // dpl_FlushDCS (plate); - // K = the x87 long double @0x4cee64 = 0.35714286 = 1/2.8 -- the SAME - // 2.8 projection constant the hotbox uses, i.e. K converts the reticle's - // -1..+1 screen space into eye-space extents at z = -1. + // K = the x87 long double @0x4cee64 = 0.35714286322496386, which + // reproduces 1.0f/2.8f bit-for-bit -- it converts the reticle's -1..+1 + // screen space into eye-space extents at z = -1. NOTE the HOTBOX uses a + // DIFFERENT authored constant (the double 2.8145 with the +/-1.6 edge + // thresholds) -- 2.8 != 2.8145, do not unify them. [T1] // @004cec98: the plate's MESH is the target player's (see // BTReticleTargetPlate in mech4.cpp), visibility 1; no owning player // -> visibility 0. // @004cdd75 / @004cddc1: reticle Off, or On-but-simple-X (PrimaryHudOn // clear) -> visibility 0 and the cached target cleared. // @004cebf9-@004cec47 -- THE LOCK GATE: the block is entered on a change - // of EITHER the target (Reticle+0x1c) or the LOCK attribute (the - // Scalar* at this+0x184, cached at +0x188), and when that lock value + // of EITHER the target (Reticle+0x1c) or the LOCK attribute (HUD attr + // id 10 "Lock", an int/Logical* at this+0x184 cached at +0x188 -- the + // producer writes 0/1, the consumer does `mov eax,[ecx]; test eax,eax` + // @0x4cec12), and when that lock value // reads 0 it branches to the hide path (@004ced87). So the plate needs // a real fire-control LOCK, not merely something under the crosshair -- // the same `gBTHudLockState == 2` rule the lock RING uses below. @@ -1914,11 +1932,17 @@ void const float kPlateH = 0.25f; // ... and its y extent (4:1) const float kInvK = 2.8f; // 1/K -- eye-space -> reticle const float kPlateDY = 0.08f * kInvK; // = 0.224 below the aim point - // The plate's own material colour: content/VIDEO/MAT/BMAP.BMF gives - // `name12_mtl` (and each sibling) a NEUTRAL (0.5, 0.5, 0.5) -- the pod's - // plate is a grey-white label, NOT phosphor green like the reticle - // glyphs. 0.5 * 255 = 128. - const unsigned long kPlateARGB = 0xFF808080; + // content/VIDEO/MAT/BMAP.BMF: name12/34/56/78_mtl carry ONLY a 0x0021 + // MATERIAL_TEXTURE plus 0x0027 = `dpfB_MATERIAL_OPACITY_TAG` {0.5,0.5,0.5} + // [T0 libDPL/dsys/PFBIZTAG.H]. There is NO diffuse / ambient / emissive / + // ramp tag -- so the material contributes NO COLOUR, only a texture and + // ~50% opacity. The plate is therefore the UNMODULATED callsign raster at + // half alpha (it blends with the scene behind it), NOT a grey label and + // NOT phosphor green like the reticle glyphs. + // [T3 on the exact ARGB: the opacity payload is an RGB triple, so it could + // be per-channel transmissivity; all three channels are 0.5 here, so a + // scalar alpha is the only reading the data distinguishes.] + const unsigned long kPlateARGB = 0x80FFFFFF; const int plate = (gBTHudPrimary && gBTHudLockState == 2) ? BTReticleTargetPlate() : 0; @@ -2440,6 +2464,35 @@ int ? (int)EntitySegment::SkeletonType_A : render_tree.skeletonType; + // + // PAINT ON RELOAD (Gitea #38 root cause, 2026-07-24). The loop below + // re-parses every shown segment's BGF through d3d_OBJECT::LoadObject, and + // there is NO geometry cache (only mTextureCache) -- so each call is a fresh + // parse. The per-pilot colour/badge/patch is applied by REWRITING MATERIAL + // NAMES during that parse, and only while the substitution callback is + // installed (`dpl_ApplyMaterialNameCallback`, engine/MUNGA_L4/bgfload.cpp:15-18; + // installed by SetupMaterialSubstitutionList, removed by TearDown). Reloading + // outside that bracket resolved the raw `%color%` placeholders -> unpainted + // materials -> the mech rendered GREY. Reproduced live 2026-07-24: a crimson + // MadCat went grey in its own view after a V (inside/outside) toggle, with NO + // new [paint]/MakeMechRenderables line in the log -- i.e. no rebuild, just + // this re-parse. The RESPAWN path lands here too (that is what the + // fresh-graphic-state read below is for), which is issue #38's mechanism. + // Re-install the list with the serial the mech was BUILT with, then restore + // the global counter so other mechs' serials are untouched. + // [T2 -- our port re-parses where the 1995 engine kept every skeleton variant + // resident, so this bracket is a PORT-NECESSARY restoration of the binary's + // invariant: mech geometry is only ever parsed with its paint installed.] + // + const int reinstall_paint = (render_tree.paintSerno != 0); + if (reinstall_paint) + { + const char saved_serno = gSerno; + gSerno = render_tree.paintSerno; + SetupMaterialSubstitutionList(viewpoint); + gSerno = saved_serno; // Setup advanced it -- keep the sequence + } + JointedMover *jm = (JointedMover *)viewpoint; EntitySegment::SegmentTableIterator it(jm->segmentTable); EntitySegment *segment; @@ -2491,10 +2544,16 @@ int render_tree.segGState[slot] = (int)gstate; if (obj) ++shown; else ++hidden; } + if (reinstall_paint) + { + TearDownMaterialSubstitutionList(); + } DEBUG_STREAM << "[view] skeleton " << (inside ? "A (inside)" : "N (outside)") << ": " << shown << " segment mesh(es) shown, " << hidden - << " hidden\n" << std::flush; + << " hidden (paint serno " + << (render_tree.paintSerno ? render_tree.paintSerno : '-') + << ")\n" << std::flush; return shown; } diff --git a/game/reconstructed/btl4vid.hpp b/game/reconstructed/btl4vid.hpp index cf8389f..c7eb114 100644 --- a/game/reconstructed/btl4vid.hpp +++ b/game/reconstructed/btl4vid.hpp @@ -485,9 +485,9 @@ class BTReticleRenderable: // the pip-row origin/scale and the two tick-ladder frames), which this // port ctor reproduces directly. The one live input the glyphs consume // is the target RANGE (drives the range-scale caret); it binds to a - // Scalar the targeting step updates. The 3D marker chain + the - // PNAME1-8.bgf pip meshes (the floating 3D target designator) are a - // separate deferred piece -- see phases/phase-02-dpl2d-reticle.md. + // Scalar the targeting step updates. The PNAME1-8.bgf TARGET NAME PLATE + // (the target's callsign under the crosshair) is RECONSTRUCTED -- see + // Draw() and phases/phase-02-dpl2d-reticle.md. // BTReticleRenderable( // @004cc40c (port signature) Entity *entity, @@ -707,6 +707,10 @@ extern void BTDrawReticle(struct IDirect3DDevice9 *device); d3d_OBJECT *wreckFlamesObj; std::map segRenderable; // slot -> joint renderable std::map segGState; // slot -> last applied graphic state + char paintSerno; // the %serno% this mech was BUILT with + // (0 = none) -- ApplyViewSkeleton re-parses + // segment BGFs, so the paint substitutions + // must be re-installed with the SAME serial }; std::map mMechRenderTrees;