forked from Ports/goldeneye-pc-port
D230: byte-swap RAW16 wavetable samples at cart-map time (BE ROM -> host LE) (#83)
The one broken bass instrument on Control/Cavern/Runway is a plain byte-order bug in a corner the earlier passes never looked at. The instrument bank's only two AL_RAW16_WAVE wavetables (instruments 2 and 51, sharing base 0x2118 / len 790) decode as a clean ~110 Hz soft-attack bass sample when read big-endian s16 -- which is how the N64 CPU reads them. The PC mixer's resampler consumes host-native (little-endian) s16, so every sample arrives byte-swapped: deterministic per-instrument corruption, exactly the M-106 repro. ADPCM nibble data is byte-order agnostic, which is why every other instrument sounds right. romdataFinishCartMap() now walks both bank trees (same ALBankFile layout as romdataFixupAudioBank) and bswap16s each RAW16 [tbl+base,+len) range in place in the cart map -- where alBnkfNew() points the mixer at it directly. Shared ranges are deduped (swapping twice undoes the swap); all reads bounds-checked. D55 in-place-fixup idiom; port-only, no game logic touched. Startup log: 'romdataInit: D230 byte-swapped 1 RAW16 wavetable range(s) in place'. Owes a by-ear pass on the three repro tracks. findings.md D230 -> FIXED with full root cause; release-notes.md drops the music known-issue bullet.
This commit is contained in:
James Dansereau authored and GitHub committed
1 parent
2c95fcd145
commit
99bc2a3a6a
3 files changed
+143
-3
No files matched your search
@@ -57,8 +57,6 @@
|
||||
- **Cutscenes frequently glitch** — skipped beats, wrong camera, misplaced or
|
||||
hovering actors, wrong timing; the Dam level-end cutscene is racy (D243).
|
||||
The most visible gap in this release.
|
||||
- **Music quality on some tracks** — a wrong-sounding bass instrument on a few
|
||||
elevator/level tracks (D230).
|
||||
- **Particle colours cycle through a rainbow palette** — bullet-impact sparks
|
||||
and lingering smoke/explosion residue drift through the hues over time
|
||||
instead of holding their intended grey/orange palette (D252).
|
||||
|
||||
@@ -409,7 +409,7 @@ covers D24–D69; the log continues in §H (D32 procedure, D70–D121).
|
||||
| D226 | **QoL ask: player-adjustable HUD scale for the ammo counter, bottom-left pickup/item status text, and top-of-screen dialogue/subtitle text.** All three route through the same primitive, `textRenderOutlined()` (`src/game/textrelated.c:688`, doc comment: *"Used for ammo counter, bottom left HUD messages, countdown timers"*), called from two specific in-game (not menu) sites in `bondview2.c`: **ammo + bottom-left pickup/status** at `bondview2.c:10005` (`stringbuffer_lowerleft[...]`, anchored at `view_left`/`view_vert`), and **top-of-screen dialogue** at `bondview2.c:10163`/`10165` (`stringbuffer_top[...]`, anchored at `msg.x`/`msg.y`). `textRenderOutlined` → `textDrawGlyphQuad` (`textrelated.c:615`) emits each glyph as a plain RDP `gSPTextureRectangle` at the glyph's native font-metric pixel size, `dsdx`/`dsdy` = `0x400` (1:1 texel:pixel, no scale factor anywhere in the call chain). Same function is also used by menus/options/multiplayer UI (`front.c`, `options.c`, `mpmenu.c`) — **do not scale generically**, those have their own layout at fixed screen fractions and D211 already established HUD anchors must stay put; scale only the 3 in-game HUD call sites named above, about their existing anchor (so bottom-left stays bottom-left, top stays top — same anchor-preserving principle as D211's HUD-static check). | **OPEN — root cause / call sites identified, not designed or implemented.** Two viable directions: (a) a `#ifdef PORT` wrapper at just these 2-3 `bondview2.c` call sites that scales the emitted `gSPTextureRectangle` geometry (`xl/yl/xh/yh`) by a new `f32 portHudScale` (default 1.0 ⇒ byte-identical) while inversely scaling `dsdx/dsdy` to keep the same source glyph texels mapped onto the larger/smaller rect, re-anchored so the opposite edge (bottom for the lower-left block, top for dialogue) doesn't move; or (b) thread a scale parameter into `textRenderOutlined`/`textDrawGlyphQuad` itself, defaulted to 1.0 at every other call site. (a) is more surgical (matches the "don't touch shared menu text" constraint) but duplicates a little geometry math; (b) is more central but touches a much more widely shared function. No PD precedent found (PD's port has no equivalent HUD-scale option). F10 overlay slider candidate once implemented, alongside `Video.FovScale`. |
|
||||
| D227 | **Sky renders as two independently-moving halves — reads as "two sky processes" (M-93, user QA on PR #43's debug build, `-level_22` Statue and Cradle).** Each half of the screen shows a different sky pattern, and the two halves scroll/move at different rates relative to each other. Surfaced *while* verifying D176(a)'s perspective-correct-interpolation fix — D176(a)'s fix measurably improved the scroll-speed symptom on the visible pattern but did not touch this; this is a distinct defect, not a regression from that PR (unclear yet whether it pre-dates PR #43 or was merely easier to notice once the scroll speed calmed down enough to see the seam). User flagged one confound worth ruling out first: might be a widescreen/aspect-ratio artifact of rendering at modern resolutions rather than a logic bug — original N64 output is 4:3/{something narrower}, and the sky-quad tiling math (`skyPortRenderPoly`, `sub_GAME_7F097388` upstream) may assume the original screen split. | **FIXED (M-100b, PR #43) — root cause was a unit error, and it also fixed the "sky scrolls too fast" complaint (M-96) and the over-tiling.** `unk20`/`unk24` are already in the RDP's native S10.5 1/32-texel units; M-82 read them as texel counts and baked `tc = (S - fold) * 32`, making every sky texcoord 32x too large and overflowing the `s16` `tc`. Human play-test confirmed by the user on the previously-failing levels. Full write-up: §H "D227 — M-100b". Historical investigation trail below. **OPEN — visual evidence captured + a concrete mechanistic hypothesis found (M-94), not yet fixed.** Headless `-level_22` capture confirms a real, visible crease/discontinuity in the cloud texture — not a rendering artifact of viewing it live, and present in a single static frame (rules out the "just camera pan" explanation for at least this instance). Screenshot: `docs/img/bugs/d227-sky-seam-statue.png`. **Leading hypothesis:** `skyPortRenderPoly` (D176(a)'s fix) computes `wScale` per call — i.e. per individual triangle in the sky fan (`skyRenderTri`/`skyRenderFull` issue up to 5 separate `skyPortRenderPoly` calls per frame from a 5-vertex fan, `sky.c:900-920`/`1372-1393`) — from that triangle's own 3 vertices' `unk0c`. The position math is exact for any `wScale` (verified algebraically: `clip.x = (ndcX·w/wScale)·wScale = ndcX·w` regardless of `wScale`), so this is not a logic bug in the fix itself, but the `s16`-quantized `ob[]` storage means each triangle's shared-edge vertices get **independently re-quantized at a different granularity** depending on that triangle's own `wScale` — a vertex sitting on the seam between two triangles can round to a measurably different `ob[]` value in each triangle's independent call, producing a small position/continuity crack exactly at triangle boundaries. This would read as "two adjacent regions with different patterns" without requiring two draw calls or two cloud layers — one `skyPortRenderPoly` call producing a slightly different quantization per neighboring primitive is sufficient. **Not yet confirmed** — did not isolate camera motion from sky-scroll motion cleanly enough to independently verify the "each half moves at a different rate" part of the user's report, only the static crease. **M-95: FIXED (the static-crease part) + headless-verified.** Implemented the fix direction above: `skyPortBeginFan()`/`skyPortEndFan()` (new, `#ifdef PORT`, `src/game/sky.c`) compute one shared `wScale` across every vertex in the whole fan (`sp274[]`/`sp94[]`, up to 5 verts), called once right after each fan's vertex array is populated and before its 1-3 `skyRenderTri`/`skyRenderFull` calls; `skyPortRenderPoly` now uses that shared value when set, falling back to its old per-call-local computation otherwise (defensive — no other caller exists today, checked: `grep` finds `skyRenderTri`/`skyRenderFull` used nowhere outside this file). **Verified:** same-frame before/after screenshots at `-level_22` frame 345 — `docs/img/bugs/d227-sky-seam-statue.png` (before, visible kink ~x=1000/1280) vs `docs/img/bugs/d227-sky-seam-statue-fixed.png` (after, same frame, continuous streaks, kink gone). Full 21-level `verify.sh sweep`: 21/21 PASS both before and after this change (no regressions). `bunker1`'s `worst_cell` framediff number is unaffected by this change — reproduced identically on the pre-fix build, it's the already-known stale-golden-baseline issue (see M-84/M-83 notes; owed a re-baseline, unrelated to sky.c). **Cradle checked too:** the "black rectangle" artifact visible there is confirmed pre-existing (identical on a pre-fix build capture) — that's the separate, already-documented FOV-coupled black-sky-past-frustum-edge symptom (D218/D222 family), not this defect; didn't chase it here. **Not fully closed:** did not re-verify the "moves at a different rate" half of the original report post-fix (the M-94 addendum's static-camera diff measurement wasn't repeated on the fix); the aspect-ratio-artiLine truncated
|
||||
| D229 | **Water on `IsWater` levels renders green and pulses blue<->green on an interval (M-101, user playtest, Frigate `-level_26`; Surface 2 also an IsWater level).** Reported alongside green weapon textures on Frigate/Surface 2 — **that half was NOT a bug**: the branch under test was 2 commits behind main and those 2 commits were exactly the D217 fixes (#47 texpool alignment, #49 CI cache palette-content key); after merging main the user confirms the gun is good. | **FIXED (port-layer) — root cause: water image is ZLIB+CI8; pool holds raw 8-bit indices; N64's custom ucode expanded them to RGBA16 in TMEM at load, fast3d read index byte-pairs as 16-bit texels (g=2·idx mod 32 dominates → green). Fix: track `src_fmt` per TMEM load; RGBA/16b tile over a CI-sourced slot now imports via the CI8 palette path (`gfx_pc.cpp` import_texture). Frigate verified live (user: "looks better"); sweep + Surface 2 by-eye still owed.** Pre-existing (A/B proven), mechanism identified as above. Built plain `main` with none of the D227 work: `-level_26` frame 340 shows the **same green** (only the tiling density differs), so D229 is independent of D227. The quad is `sky.c`'s `IsWater` block1 (Frigate confirmed `IsWater=1` via `GE_D176` — which also settles M-99's ranked item 1: that path is textured and does go through `skyPortRenderPoly`). Draw state from `sub_GAME_7F09343C` (`unk_092E50.c:278`): tile 0 and tile 1 both RGBA/16b at **the same TMEM 0**, `masks=maskt=5`, tile 1 offset by `gDPSetTileSize(1, uls=90, ult=150, lrs=0, lrt=0)`, 2-cycle `LERP(TEXEL1, TEXEL0, PRIM_LOD_FRAC)` with cycle 2 `= COMBINED * SHADE` and the LOD fraction driven by **`sinf()`** — that sine IS the reported pulsing, so **one texunit resolves blue and the other green** and the sine fades between them. **Ruled out: the shade path** — `skyChooseWaterVtxColour` with measured `env RGB=16,48,96` / `WaterRGB=255,255,150` spans dark blue to pale yellow and can never be green; measured pixels ~`(5,70..175,20..37)` (R and B crushed) put the green on the texel side. **Not established:** why one texunit decodes green when both tiles share TMEM and format. One unproven oddity: `uls/lrs` are `uint16_t` and `width = ((lrs-uls)>>2)+1` gives tile 1 `-22` -> **65514**, though the matching negative `tex_width2` makes the sub-tile-window branch skip and fall through to the correct mask-based 32-texel wrap, so it may be harmless. **Next:** widen the `GE_D172` multitex probe (it filters on `tile1.tmem != tile0.tmem`, equal here, so it never fires) to dump both texunits' decoded state; capture a *sequence* across the sine period, since a single frame is useless (frame 340 reads green on all builds); and check `skywaterimages[2]`'s real format vs the RGBA/16b `sub_GAME_7F09343C` re-declares. **D227 is deliberately decoupled:** M-100b's safety valve takes `allowShift` and the water quad passes FALSE, leaving that path byte-identical to pre-D227 — shifting both tiles was observed live to flatten the cross-fade into constant green. Full write-up: §H "D229 — M-101". |
|
||||
| D230 | **In-level music quality: a bass instrument sounds corrupted on PC, correct on N64 reference (M-105, user reports; repro refined M-106).** Filed to give the post-D77 residual its own label — D77 (in-level music *absence*) is RESOLVED (PR #42, missing `return` in `sub_GAME_7F0C0BF0` zeroed the seqp volume on the only in-level trigger path), so "music doesn't play" claims in tracked docs are stale; what actually remains is this quality gap. **M-106 repro, deterministic (not intermittent):** Control's elevator track and its regular in-level track both carry the bad instrument throughout; Cavern's elevator track has it too; Runway's track is otherwise fine but the same instrument enters ~5–7s after the track starts and sounds bad from then on. User confirmed against original N64 reference audio that a bass instrument belongs there on Runway — this is **not** a wrong-instrument/wrong-bank selection (rules out the D77 trigger-path family and the seqp-volume fix's neighborhood); the instrument choice is correct, only its *sound* is wrong on PC. Determinism (same track, same spot, every time) points at a static decode/synthesis bug for this instrument's wavetable/ADPCM data — most likely low-frequency-specific (pitch-shift/downsample artifacting, a wrap/loop-point error, or a clipping/gain issue that only bites bass-range samples) — rather than a live mixing/voice-stealing issue (D147/D152/D202 family), which would be intermittent. The mixer layer and the trigger layer are both proven working by every track that sounds right, so the suspect neighborhood is the seq/instrument-bank decode path (`csplayer.c`/`synthesizer.c`, bank loading), not `port/` audio plumbing. | **OPEN — static-analysis pass done (M-108), not runtime-confirmed.** Repro: Control (elevator + regular track, throughout), Cavern (elevator, throughout), Runway (~5–7s in). Traced the suspect neighborhood to the software ADPCM decoder in `port/src/mixer.c` — the PC replacement for the RSP's `aLoadADPCM`/`aADPCMdec` ucode ops (`csplayer.c`/`synthesizer.c` only emit ucode command lists; they don't decode samples themselves). Found two hardcoded assumptions that don't hold for the general ADPCM-book format: (1) `sAdpcmTable` (`mixer.c:174`) is declared `[8][2][8]`, hardcoding predictor `order=2`, but `order`/`npredictors` are per-instrument authored fields (`include/PR/libaudio.h` `ALADPCMBook.order`; `src/libultra/audio/load.c:428-429` computes `bookSize` from them) that the bank-relayout code (`port/src/romdata.c:718-736`) already handles generically — if this bass instrument's authored book uses `order != 2`, the 2-tap recursion at `mixer.c:223-240` would decode it wrong; (2) even at `order=2`, `aLoadADPCMImpl` (`mixer.c:177-192`) silently truncates any book over 256 bytes (`mixer.c:183`, `npredictors > 8`), while the decode loop's 4-bit predictor-select nibble (`mixer.c:219`) can legitimately address up to 16 rows — an instrument with `npredictors > 8` would read truncated/garbage rows for some frames. **Caveat weakening confidence:** N64 SDK ADPCM in practice is conventionally always `order=2` (the RSP ucode's own decode algorithm is essentially fixed to a 2-tap recursion industry-wide), so candidate (1) may not apply to a real N64 title's data; candidate (2) (`npredictors > 8`) is the more plausible of the two absent contrary evidence. Not confirmed against GE's actual ROM-authored book data (no live capture taken this pass). Next: extend the existing `GE_MIXERTRACE=1` trace's `[LOADADPCM]` line (`mixer.c:187`, already logs `count` — `count/16 = order*npredictors`) to flag any book where `count > 256` (proves truncation) during a Control run, and correlate the anomalous book to its channel/instrument via `seqplayer.c:1120`'s program-change. If no anomaly shows, this whole neighborhood is likely a dead end and the next suspect is the resample (`mixer.c:295-341`) or envelope/gain (`mixer.c:379+`) paths for bass-range-specific behavior. **M-120 (local-Qwen triage dispatch): a third hypothesis proposed and CHECKED, RULED OUT.** The dispatched read-only pass proposed that `afFixupBook`'s `port/src/romdata.c:733-737` conversion of the ADPCM book coefficients (BE ROM → LE host, via `afRd16`/`afWr16`) is backwards and should stay big-endian. Verified against the actual code and disproven: (1) that conversion is the identical BE→LE pattern used for every other field in the same function (`order`/`npredictors` two lines above) and in the sibling `afFixupAdpcmLoop`'s ADPCM state array a few lines down — not something special-cased for this one book; (2) `port/src/mixer.c`'s `aLoadADPCMImpl` does a raw `memcpy` of the already-converted book straight into a host-native `s16[8][2][8]` table with no further byte-swap, consistent with the coefficients being intentionally pre-converted to host order; (3) decisively, `afFixupBook` is one shared code path applied uniformly to every ADPCM book in the game — if this conversion werLine truncated
|
||||
| D230 | **In-level music quality: a bass instrument sounds corrupted on PC, correct on N64 reference (M-105, user reports; repro refined M-106).** Filed to give the post-D77 residual its own label — D77 (in-level music *absence*) is RESOLVED (PR #42, missing `return` in `sub_GAME_7F0C0BF0` zeroed the seqp volume on the only in-level trigger path), so "music doesn't play" claims in tracked docs are stale; what actually remains is this quality gap. **M-106 repro, deterministic (not intermittent):** Control's elevator track and its regular in-level track both carry the bad instrument throughout; Cavern's elevator track has it too; Runway's track is otherwise fine but the same instrument enters ~5–7s after the track starts and sounds bad from then on. User confirmed against original N64 reference audio that a bass instrument belongs there on Runway — this is **not** a wrong-instrument/wrong-bank selection (rules out the D77 trigger-path family and the seqp-volume fix's neighborhood); the instrument choice is correct, only its *sound* is wrong on PC. Determinism (same track, same spot, every time) points at a static decode/synthesis bug for this instrument's wavetable/ADPCM data — most likely low-frequency-specific (pitch-shift/downsample artifacting, a wrap/loop-point error, or a clipping/gain issue that only bites bass-range samples) — rather than a live mixing/voice-stealing issue (D147/D152/D202 family), which would be intermittent. The mixer layer and the trigger layer are both proven working by every track that sounds right, so the suspect neighborhood is the seq/instrument-bank decode path (`csplayer.c`/`synthesizer.c`, bank loading), not `port/` audio plumbing. | **OPEN — static-analysis pass done (M-108), not runtime-confirmed.** Repro: Control (elevator + regular track, throughout), Cavern (elevator, throughout), Runway (~5–7s in). Traced the suspect neighborhood to the software ADPCM decoder in `port/src/mixer.c` — the PC replacement for the RSP's `aLoadADPCM`/`aADPCMdec` ucode ops (`csplayer.c`/`synthesizer.c` only emit ucode command lists; they don't decode samples themselves). Found two hardcoded assumptions that don't hold for the general ADPCM-book format: (1) `sAdpcmTable` (`mixer.c:174`) is declared `[8][2][8]`, hardcoding predictor `order=2`, but `order`/`npredictors` are per-instrument authored fields (`include/PR/libaudio.h` `ALADPCMBook.order`; `src/libultra/audio/load.c:428-429` computes `bookSize` from them) that the bank-relayout code (`port/src/romdata.c:718-736`) already handles generically — if this bass instrument's authored book uses `order != 2`, the 2-tap recursion at `mixer.c:223-240` would decode it wrong; (2) even at `order=2`, `aLoadADPCMImpl` (`mixer.c:177-192`) silently truncates any book over 256 bytes (`mixer.c:183`, `npredictors > 8`), while the decode loop's 4-bit predictor-select nibble (`mixer.c:219`) can legitimately address up to 16 rows — an instrument with `npredictors > 8` would read truncated/garbage rows for some frames. **Caveat weakening confidence:** N64 SDK ADPCM in practice is conventionally always `order=2` (the RSP ucode's own decode algorithm is essentially fixed to a 2-tap recursion industry-wide), so candidate (1) may not apply to a real N64 title's data; candidate (2) (`npredictors > 8`) is the more plausible of the two absent contrary evidence. Not confirmed against GE's actual ROM-authored book data (no live capture taken this pass). Next: extend the existing `GE_MIXERTRACE=1` trace's `[LOADADPCM]` line (`mixer.c:187`, already logs `count` — `count/16 = order*npredictors`) to flag any book where `count > 256` (proves truncation) during a Control run, and correlate the anomalous book to its channel/instrument via `seqplayer.c:1120`'s program-change. If no anomaly shows, this whole neighborhood is likely a dead end and the next suspect is the resample (`mixer.c:295-341`) or envelope/gain (`mixer.c:379+`) paths for bass-range-specific behavior. **M-120 (local-Qwen triage dispatch): a third hypothesis proposed and CHECKED, RULED OUT.** The dispatched read-only pass proposed that `afFixupBook`'s `port/src/romdata.c:733-737` conversion of the ADPCM book coefficients (BE ROM → LE host, via `afRd16`/`afWr16`) is backwards and should stay big-endian. Verified against the actual code and disproven: (1) that conversion is the identical BE→LE pattern used for every other field in the same function (`order`/`npredictors` two lines above) and in the sibling `afFixupAdpcmLoop`'s ADPCM state array a few lines down — not something special-cased for this one book; (2) `port/src/mixer.c`'s `aLoadADPCMImpl` does a raw `memcpy` of the already-converted book straight into a host-native `s16[8][2][8]` table with no further byte-swap, consistent with the coefficients being intentionally pre-converted to host order; (3) decisively, `afFixupBook` is one shared code path applied uniformly to every ADPCM book in the game — if this conversion werLine truncated
|
||||
| D233 | **All levels (intermittent): indoor level geometry invisible near doors — you can see through walls/floors into rooms you can't see into, while world objects (terminals, furniture) in that space still draw (M-106, user QA report; screenshots `docs/img/bugs/20260912-cullingbug-bugged.png` vs `-notbugged.png`).** Root-caused M-107 (session-016TaKy6, port/fast3d only): **fast3d's CPU-side "trivial reject" AND-test in `gfx_sp_tri1` (`port/fast3d/gfx_pc.cpp:1455-1460`) has a latent w-sign bug independent of D222/D106.** The per-vertex outcode bits computed in `gfx_sp_vertex` (`x<-w`/`x>w`/`y<-w`/`y>w`/`z>w`, `gfx_pc.cpp:1372-1386`) are only valid half-space tests when `w>0`; for a vertex that has crossed behind the camera plane (`w<0` — common when the camera is close to large room-structure polygons, e.g. walls/ceiling trusses near a doorway) the comparison sense is flipped, so its outcode can come out wrong. If that spurious bit happens to match the other two (genuinely correct) vertices' bits, `v1->clip_rej & v2->clip_rej & v3->clip_rej` fires and the whole triangle is dropped before it ever reaches OpenGL — even though GL's own homogeneous clipper (which *does* handle `w<0` correctly) would have rendered the visible part of it. Confirmed against the two screenshots: same room/camera/ammo-HUD, one frame is missing the room's ceiling trusses/walls (replaced by flat sky) while the console/terminal props in the same room still draw fully — exactly the "big background polys vanish, small prop models don't" split you'd expect, since large near-camera polygons are far more likely to have a vertex cross behind the eye than small self-contained prop meshes. The backface-cull code 6 lines below (`gfx_pc.cpp:1462-1495`) already special-cases this exact `w<0` hazard for its own cross-product sign (`(v1->w<0)^(v2->w<0)^(v3->w<0)`) — the trivial-reject block above it never got the same treatment. **Independent of D222** (D222 is a game-logic frustum-cull bug in `src/game/fr.c`/`bondview2.c` that only manifests at non-default `Video.FovScale`; this is a GPU-pipeline clip-rejection bug in `port/fast3d/` that fires at any FovScale, including the default 100, whenever the camera gets close to a large polygon — matching the report's "all levels, near doors" framing with no FovScale mentioned). Also independent of D106 (D106 fixed the *game-side* portal-visibility bbox computation for a different z==0 edge case in `bg.c`; this is the *renderer-side* per-triangle clip decision, a different stage of the pipeline entirely) — same neighbourhood/symptom vocabulary ("things vanish near a portal-adjacent boundary"), different bug. | **FIXED (port/fast3d/gfx_pc.cpp, `gfx_sp_tri1`) — high confidence on mechanism, UNVERIFIED visually.** Fix: skip the trivial-reject AND-test entirely whenever any of the triangle's 3 vertices has `w<0`, deferring to GL's clipper instead (mirrors the cull code's existing w-sign handling one block below). Change only ever *adds* triangles that were previously dropped — cannot make anything newly invisible, so it's a safe, narrowly-targeted fix with no game-logic touch (fast3d is PC-only code, no N64 build parity to preserve, no `#ifdef PORT` needed). **Build verified green** (`./build-pc.sh` equivalent — MSYS2 mingw64 cmake+ninja, full `ge007.x86_64.exe` link succeeds, only one pre-existing unrelated `-Wformat` warning in the same file). **VISUALLY VERIFIED (M-108):** user played `-level_26` (Frigate — corrected level id, the old `-level_02` note above predates the `bondconstants.h` numbering check) and confirmed the doorway/ship-corridor geometry no longer flickers or vanishes near room boundaries. Closed. |
|
||||
| D234 | **Dam: guard towers along the dam structure invisible except the first one (M-106, user QA report).** Investigated M-107 (session-016TaKy6, port/fast3d scope): **NOT root-caused — no fast3d-side mechanism found that plausibly explains "only the first of N repeated instances renders."** Ruled out: fast3d has no matrix/model instance cache that could explain a first-instance-only bug (the D217-family "keyed by address, not content" cache-bug class doesn't recur here — the *only* content-addressed cache in fast3d is the texture cache, already excluded from this investigation per the M-106 triage note); the modelview matrix stack (`gfx_pc.cpp:105`, depth 11, push properly gated `< 11` at `gfx_pc.cpp:1220`) is correctly bounded and would affect every level's nested-DL geometry, not just Dam's towers specifically; the D233 w<0 clip-rejection bug (above) doesn't fit either — towers are viewed from a distance along the dam, not close enough for a camera-behind-vertex condition, and D233's mechanism is per-triangle/per-frame flicker, not a stable "always the same instances missing" pattern. **Leading hypothesis, NOT verified:** GE's road/structure-instancing for repeated architecture typically goes through the room-graph's bytecode "vis program" (`VISOP_*` opcodes interpreted in `bg.c` ~L4450-4620, e.g. `VISOP_PUSH_IF_ROOM_IN_RANGE`, `VISOP_ADD_VISIBLE_ROOM`) or through per-instance room/portal definitions in the level's compiled BG data — both are level *data*, not code reachable via static analysis without the ROM (this worktree has none extracted). Could equally be a per-instance object/prop transform or cull-volume bug in `src/game/propobj.c`/`chrprop.c` (out of this task's `port/fast3d/` scope, and out of the `src/game/*` model/anim exclusion in the brief regardless). **No fix attempted — do not have enough evidence to touch code.** | **VISUALLY VERIFIED FIXED (M-108) — mechanism still unexplained.** User confirmed on a live Dam playthrough (post-D233 fix, correct `-level_26`/Frigate session's build) that all guard towers along the dam now render, not just the first. No code change was made specifically for D234, and the M-107 investigation's own reasoning argued D233's mechanism doesn't fit this symptom (towers viewed from a distance, not a close-camera w<0 case) — so either that reasoning was wrong, D233's fix had a broader effect than analyzed, or the original symptom was intermittent/state-dependent rather than deterministic. Not closed with high confidence in *why* it's fixed, just closed on live confirmation that it *is*. If it resurfaces, re-open with a fresh `GE_PCDUMP` capture rather than assuming this write-up's ruled-out list still holds. |
|
||||
| D244 | **Self-hosted CI runner (`corpyvt6agent01`) link failure: `undefined reference to 'objectiveGetStatus_WEAK'` (2026-09-12, first-ever build attempt on the reinstalled box, run `34707798707`).** `src/game/objective_status.c:162` uses `#pragma weak objectiveGetStatus_WEAK = get_status_of_objective` (a decomp ground-truth alias, referenced 2-arg from `chrai.c:2538` — comment calls it "horrible hack to get ai matching"). The box's `g++`/`ld` reported as `x86_64-w64-mingw32/16.1.0`; a minimal local repro of the identical `#pragma weak` cross-TU function alias **links cleanly** on this session's own machine (`mingw-w64-x86_64-gcc 16.2.0-3`, `binutils 2.47-3`). Not a game-logic bug (the alias is unmodified decomp code, ground truth) and not a CMakeLists/link-flag issue (no `--gc-sections`/LTO in play) — the leading, unverified explanation is the self-hosted box's MSYS2 packages predate a PE/COFF weak-external fix or behavior change versus current MSYS2, consistent with rollout step 2's earlier finding that the box's mingw64 package set was stale/incomplete at initial setup. | **OPEN, not yet fixed.** Next: on the box, `pacman -Syu` (or at minimum `pacman -S mingw-w64-x86_64-gcc mingw-w64-x86_64-binutils`) to bring the toolchain current, then re-dispatch `selfhosted.yml`. If updating doesn't clear it, the fallback is a narrow `#ifdef PORT` replacement of the alias with an explicit thin wrapper function in `objective_status.c` (ABI-preserving, no logic change) — but try the toolchain update first since the local repro suggests this is a version/environment gap, not a fundamental PE/COFF limitation. |
|
||||
|
||||
@@ -113,6 +113,116 @@ static int romHeaderValid(const u8 *h, char *err, size_t errsz)
|
||||
|
||||
static u32 romdataBswap32(u32 v); /* defined below; used by the D55 fixup */
|
||||
|
||||
/* D230: RAW16 wavetable samples are stored big-endian s16 in the ROM (the
|
||||
* N64 CPU reads them that way), but the PC mixer's resampler consumes
|
||||
* host-native (little-endian) s16 — so every RAW16 instrument plays as a
|
||||
* byte-swapped buzz. ADPCM nibble data is byte-order agnostic, which is why
|
||||
* only RAW16 ranges are affected. The wavetable sample segments stay in the
|
||||
* cart map (alBnkfNew() points straight at them; only the bank structs are
|
||||
* re-laid out, D37), so bswap16 those ranges in place once the map is live.
|
||||
* The walk mirrors the ALBankFile layout romdataFixupAudioBank() uses. */
|
||||
|
||||
#define RAW16_MAX_RANGES 32
|
||||
|
||||
struct raw16Range { u32 start, end; };
|
||||
|
||||
static void romdataRaw16Swap(u8 *tbl, u32 tblSize, u32 base, s32 len,
|
||||
struct raw16Range *done, int *ndone)
|
||||
{
|
||||
int i;
|
||||
if (len <= 0 || base >= tblSize || (u32)len > tblSize - base) {
|
||||
sysLogPrintf(LOG_ERROR, "romdataInit: D230 RAW16 wavetable out of "
|
||||
"range (base 0x%X len %d, segment 0x%X bytes)",
|
||||
base, len, tblSize);
|
||||
return;
|
||||
}
|
||||
for (i = 0; i < *ndone; i++) {
|
||||
/* Shared sample data (two instruments can point at the same range):
|
||||
* swapping twice would undo the first swap. */
|
||||
if (base >= done[i].start && base + (u32)len <= done[i].end)
|
||||
return;
|
||||
}
|
||||
for (i = 0; i + 1 < len; i += 2) {
|
||||
u8 t = tbl[base + i];
|
||||
tbl[base + i] = tbl[base + i + 1];
|
||||
tbl[base + i + 1] = t;
|
||||
}
|
||||
if (*ndone < RAW16_MAX_RANGES) {
|
||||
done[*ndone].start = base;
|
||||
done[*ndone].end = base + (u32)len;
|
||||
(*ndone)++;
|
||||
}
|
||||
}
|
||||
|
||||
static void romdataRaw16Inst(const u8 *ctl, u32 ctlSize, u8 *tbl, u32 tblSize,
|
||||
struct raw16Range *done, int *ndone, u32 ioff)
|
||||
{
|
||||
s16 scount;
|
||||
u32 k;
|
||||
if (!ioff || ioff + 16 > ctlSize)
|
||||
return;
|
||||
scount = (s16)((ctl[ioff + 14] << 8) | ctl[ioff + 15]); /* soundCount */
|
||||
if (scount < 0)
|
||||
return;
|
||||
for (k = 0; k < (u32)scount; k++) {
|
||||
u32 soff, wt;
|
||||
if (ioff + 16 + 4 * k + 4 > ctlSize)
|
||||
break;
|
||||
soff = ((u32)ctl[ioff + 16 + 4 * k] << 24) |
|
||||
((u32)ctl[ioff + 17 + 4 * k] << 16) |
|
||||
((u32)ctl[ioff + 18 + 4 * k] << 8) |
|
||||
(u32)ctl[ioff + 19 + 4 * k];
|
||||
if (!soff || soff + 15 > ctlSize)
|
||||
continue;
|
||||
wt = ((u32)ctl[soff + 8] << 24) | ((u32)ctl[soff + 9] << 16) |
|
||||
((u32)ctl[soff + 10] << 8) | (u32)ctl[soff + 11];
|
||||
if (!wt || wt + 20 > ctlSize)
|
||||
continue;
|
||||
if (ctl[wt + 8] == 1 /* AL_RAW16_WAVE */) {
|
||||
u32 base = ((u32)ctl[wt] << 24) | ((u32)ctl[wt + 1] << 16) |
|
||||
((u32)ctl[wt + 2] << 8) | (u32)ctl[wt + 3];
|
||||
s32 len = (s32)(((u32)ctl[wt + 4] << 24) |
|
||||
((u32)ctl[wt + 5] << 16) |
|
||||
((u32)ctl[wt + 6] << 8) | (u32)ctl[wt + 7]);
|
||||
romdataRaw16Swap(tbl, tblSize, base, len, done, ndone);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
static void romdataRaw16Walk(const u8 *ctl, u32 ctlSize, u8 *tbl, u32 tblSize,
|
||||
struct raw16Range *done, int *ndone)
|
||||
{
|
||||
s16 i;
|
||||
if (ctlSize < 4 || ((ctl[0] << 8) | ctl[1]) != 0x4231 /* 'B1' */)
|
||||
return;
|
||||
for (i = 0; i < (s16)((ctl[2] << 8) | ctl[3]); i++) {
|
||||
u32 boff, j;
|
||||
s16 icount;
|
||||
if (4 + 4 * (u32)i + 4 > ctlSize)
|
||||
break;
|
||||
boff = ((u32)ctl[4 + 4 * i] << 24) | ((u32)ctl[5 + 4 * i] << 16) |
|
||||
((u32)ctl[6 + 4 * i] << 8) | (u32)ctl[7 + 4 * i];
|
||||
if (!boff || boff + 12 > ctlSize)
|
||||
continue;
|
||||
icount = (s16)((ctl[boff] << 8) | ctl[boff + 1]); /* instCount */
|
||||
if (icount < 0)
|
||||
continue;
|
||||
romdataRaw16Inst(ctl, ctlSize, tbl, tblSize, done, ndone,
|
||||
((u32)ctl[boff + 8] << 24) |
|
||||
((u32)ctl[boff + 9] << 16) |
|
||||
((u32)ctl[boff + 10] << 8) | (u32)ctl[boff + 11]);
|
||||
for (j = 0; j < icount; j++) {
|
||||
if (boff + 12 + 4 * (u32)j + 4 > ctlSize)
|
||||
break;
|
||||
romdataRaw16Inst(ctl, ctlSize, tbl, tblSize, done, ndone,
|
||||
((u32)ctl[boff + 12 + 4 * j] << 24) |
|
||||
((u32)ctl[boff + 13 + 4 * j] << 16) |
|
||||
((u32)ctl[boff + 14 + 4 * j] << 8) |
|
||||
(u32)ctl[boff + 15 + 4 * j]);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/* The caller has placed a writable region of (romSize + sidecars) bytes at
|
||||
* CART_BASE. Populate it, load the sidecars, apply the D55 RLE-header fixup,
|
||||
* and mark the ROM live. Shared by the Windows (VirtualAlloc) and POSIX (mmap)
|
||||
@@ -139,6 +249,38 @@ static int romdataFinishCartMap(const char *tok, u8 *img,
|
||||
if ((u16)*hdr > 512 || (u16)(*hdr >> 16) > 512)
|
||||
*hdr = romdataBswap32(*hdr);
|
||||
}
|
||||
|
||||
/* D230: byte-swap the RAW16 wavetable sample ranges in both bank trees
|
||||
* (BE s16 in the ROM -> host LE for the mixer's resampler). */
|
||||
{
|
||||
extern u32 _sfxctlSegmentRomStart;
|
||||
extern u32 _sfxtblSegmentRomStart;
|
||||
extern u32 _instrumentsctlSegmentRomStart;
|
||||
extern u32 _instrumentstblSegmentRomStart;
|
||||
struct raw16Range done[RAW16_MAX_RANGES];
|
||||
int ndone = 0;
|
||||
|
||||
/* Each tbl segment runs to the end of the ROM; bound the swap range
|
||||
* by that (the real segment is smaller, but base/len are validated
|
||||
* against this anyway). */
|
||||
romdataRaw16Walk((const u8 *)&_sfxctlSegmentRomStart,
|
||||
(u32)&_sfxtblSegmentRomStart -
|
||||
(u32)&_sfxctlSegmentRomStart,
|
||||
(u8 *)&_sfxtblSegmentRomStart,
|
||||
CART_BASE + romSize -
|
||||
(u32)&_sfxtblSegmentRomStart,
|
||||
done, &ndone);
|
||||
romdataRaw16Walk((const u8 *)&_instrumentsctlSegmentRomStart,
|
||||
(u32)&_instrumentstblSegmentRomStart -
|
||||
(u32)&_instrumentsctlSegmentRomStart,
|
||||
(u8 *)&_instrumentstblSegmentRomStart,
|
||||
CART_BASE + romSize -
|
||||
(u32)&_instrumentstblSegmentRomStart,
|
||||
done, &ndone);
|
||||
if (ndone)
|
||||
sysLogPrintf(LOG_INFO, "romdataInit: D230 byte-swapped %d RAW16 "
|
||||
"wavetable range(s) in place", ndone);
|
||||
}
|
||||
return 0;
|
||||
}
|
||||
|
||||
|
||||
Reference in new issue
Block a user