docs: clarify rule #2's diagnostic scope + define rule-2 sign-off process

Widens the written scope of AGENTS.md non-negotiable #2's ABI exception to
match established practice (runtime struct/union misreads, not just
ROM-serialized structs — the D209/D210/D255 pattern), states explicitly
that diagnosis into src/game is never restricted (only the fix is), and
defines a real three-bucket taxonomy + sign-off procedure (dev-process.md
§7) for the rare genuine-behavior-difference case, replacing the
undefined ad hoc "ask the user" gesture that's been referenced without
being defined across ~10 places in the docs.

Mirrors the same framing into CONTRIBUTING.md and the PR template for
external contributors. Extends porting-notes.md §A1 with D255 as a third
worked example and a tell-tale-signs checklist, explicitly distinguishing
it from D284 (same epistemic shape, different mechanism, no rule-2
question at all). Appends a ready-to-dispatch pass-10 brief to the D236
entry applying the new taxonomy to its actual next step.

Docs-only; no code/build changes. Per docs/dev/notes/RULE-2-CLARIFICATION-PLAN.md.
This commit is contained in:
James Dansereau committed 2026-09-16 17:45:58 -04:00
1 parent deac53f892
commit 1c60da6f46
7 files changed
+247 -19

No files matched your search

+8 -3
View File
@@ -13,9 +13,14 @@ rules there are non-negotiable.
## Scope check
- [ ] No changes under `src/` or `include/` — **or** the only changes are the
narrow `#ifdef PORT` ABI exception (CONTRIBUTING.md rule 2), and each is
documented in `docs/porting-notes.md` / `docs/dev/findings.md`.
- [ ] No changes under `src/` or `include/` — **or** the only changes are
the narrow `#ifdef PORT` ABI exception (CONTRIBUTING.md rule 2),
checked against `docs/porting-notes.md` §A1, and documented in
`docs/dev/findings.md` (§F/D3x).
- [ ] If this PR contains a genuine `src/game` behavior change (not an
ABI/layout fix): an explicit rule-2 sign-off was granted
(`docs/dev-process.md` §7) and is recorded in a
`RULE-2-SIGNOFF`-tagged `docs/dev/findings.md` entry linked here.
- [ ] `Makefile`, `tools/`, `rsp/`, `ld/` untouched (N64 build).
- [ ] If `CMakeLists.txt` `REGION_DEFS` changed, it still matches the N64
`Makefile` per-region macro set exactly.
+1 -1
View File
@@ -11,7 +11,7 @@
## Non-negotiables
1. **N64 build untouched.** `Makefile`, `tools/`, `rsp/`, `ld/` belong to the N64 build. Never modify them for the PC port.
2. **Game logic is unmodified.** The decomp's control flow and behavior are ground truth for 1:1 fidelity — never change them. All N64 *hardware* dependencies are satisfied by the `port/` layer; if a game file seems to need a behavioral change, stop — the fix belongs in `port/`. **Narrow exception (ABI/layout only):** the 32→64-bit transition forces a small class of mechanical, semantics-preserving edits that cannot be isolated in `port/` — chiefly pointer-width reconciliation in ROM-serialized structs (a struct with a 32-bit-pointer field misaligns when read as 64-bit). These follow the PD ground-truth pattern (store the embedded address as `u32`, cast to a real pointer at the use site), change no logic or behavior, and are each documented in `docs/dev/findings.md` §F/D3x. No other game-code edits are permitted.
2. **Game logic is unmodified.** The decomp's control flow and behavior are ground truth for 1:1 fidelity — never change them. All N64 *hardware* dependencies are satisfied by the `port/` layer; if a game file seems to need a behavioral change, stop and check the exception below before assuming the fix belongs in `port/`. **Diagnosis is never restricted — only the fix.** Trace a bug as deep into `src/game` state as the evidence leads, and name the exact struct/field/logic responsible, before deciding which of three buckets it falls in (full framing: `docs/dev-process.md`). **Narrow exception (ABI/layout only):** the 32→64-bit pointer-width transition forces a small class of mechanical, semantics-preserving edits that cannot be isolated in `port/` — **any struct-layout or pointer-width-driven misread caused by 32→64-bit widening**, whether in a ROM-serialized record (a struct with a 32-bit-pointer field misaligns when read as 64-bit) or a **live runtime struct/union** (a raw-byte offset alias into a union arm whose true field shifted because an earlier member in the same union widened — the D209/D210/D255 pattern; see `docs/porting-notes.md` §A1 for the full catalogue and its diagnostic tells). These follow the PD ground-truth pattern (store the embedded address as `u32` and cast to a real pointer at the use site; or read the correctly-named/typed field instead of a raw-offset/mistyped alias), change no logic or behavior, and are each documented in `docs/dev/findings.md` §F/D3x, cross-tagged to §A1 where that pattern applies. **A genuine behavioral difference** — the decomp's byte-identical code, given verified-correct inputs, still diverging from real N64 behavior — is not covered by this exception and requires the rule-2 sign-off procedure (`docs/dev-process.md`) before any `src/game` edit. No other game-code edits are permitted.
3. **Region macros mirror the Makefile.** `CMakeLists.txt` `REGION_DEFS` must match the N64 Makefile's per-region macro set exactly (finding A1). Divergence = silent branch divergence + link failures.
4. **`src/libultrare/Makefile.libultrare` is ground truth** for original-vs-Rare libultra files (finding B3). The PC build compiles: `libultra/audio`, `libultrare/audio` (drvrNew/env/reverb), `libultra/gu`, and `libultrare/io/vitbl.c` only. All other `io/` + `os/` files are excluded and shimmed in `port/src/libultra.c`.
5. **`rsp/graphics/gmain.s` is the RSP ground truth** — the authoritative reference for which GBI commands GE emits (modified fast3d, 1545 lines). We do not run it on PC; `port/fast3d/` replaces it. Use it to validate the software RSP's command decoding and the custom CC/RM modes.
+13 -6
View File
@@ -11,12 +11,19 @@ keep the port faithful to the original game.
belong to the N64 build and are never touched for PC work. If PC code seems
to need a behavioural change in the game, the fix belongs in `port/`.
2. **Narrow ABI exception.** The 32→64-bit transition forces a small class of
mechanical, semantics-preserving edits to ROM-serialized structs (storing an
embedded 32-bit pointer as `u32` and casting at the use site). These are
allowed, must be guarded with `#ifdef PORT`, must change no behaviour, and
must be documented. See `docs/porting-notes.md` for the catalogue of
patterns.
2. **Narrow ABI exception; diagnosis is always allowed.** Trace a bug as
deep into the decomp's state as the evidence leads — that's never
restricted. Only the *fix* is constrained: the 32→64-bit transition
forces a small class of mechanical, semantics-preserving edits — any
struct-layout or pointer-width-driven misread caused by 32→64-bit
widening, in a ROM-serialized record or a live runtime struct/union alike
(see `docs/porting-notes.md` §A1 for the pattern and its tells). These
are allowed, must be guarded with `#ifdef PORT`, must change no
behaviour, and must be documented. A genuine behavioral difference from
real N64 behavior is a separate, much rarer case that needs explicit
maintainer sign-off first — see `docs/dev-process.md`'s rule-2 sign-off
procedure; it is not something a contributor can self-approve or infer
from this exception.
3. **Region macros mirror the Makefile.** `CMakeLists.txt` `REGION_DEFS` must
match the N64 `Makefile`'s per-region macro set exactly.
+111 -1
View File
@@ -22,6 +22,45 @@ faster than its predecessor. That project bit-matches compiler output;
this one matches runtime behavior, so the mechanics differ; but the
project-structure lessons carry over directly.
### Diagnosis is unrestricted; only the fix is constrained
AGENTS.md non-negotiable #2 restricts *changing* `src/game` behavior. It
has never restricted *investigating* it. An agent that stops short of
naming the exact `src/game` struct, field, or logic responsible for a bug
— because touching that file "feels" off-limits — is doing worse
diagnosis than the rules require, not better compliance with them.
Several of this port's hardest, highest-impact bugs (D209, D210, D255)
were exactly this shape: a `src/game` state variable reading wrong,
root-caused only by tracing fully into the game file that owned it.
Every investigation should end able to say which of three buckets
applies:
- **(a) ABI/layout-class.** The true cause is a struct-layout or
pointer-width misread introduced by the 32→64-bit transition — in a
ROM-serialized record or a live runtime struct/union alike. Fixable
under the narrow exception, entirely inside the game file under
`#ifdef PORT`, no sign-off needed. Check `docs/porting-notes.md` §A1
first (the pattern's tells + worked examples) — this is the most
common shape a "`src/game` behavior looks wrong" report turns out to
have.
- **(b) Port-layer bug that merely manifests through game state.** The
game code is reading state that's correct for what N64 would have
produced; something in `port/` (a shim, fast3d, frame timing, an
uninitialized or mis-ordered port-owned value) is what actually
differs. Fixable entirely in `port/`, no `src/game` touch, no sign-off
needed.
- **(c) Genuine game-logic behavior difference.** The decomp's
byte-identical control flow, given verified-correct inputs, provably
diverges from real N64 behavior. Rare, and requires the rule-2
sign-off procedure (§7 below) before any `src/game` edit.
Bucket (c) is rare by design — most bugs that look like (c) turn out to
be (a) or (b) in disguise — but ruling it in or out requires actually
tracing the state to its source, not stopping at the file boundary.
"This state lives in `src/game`" is a reason to keep tracing, not a
reason to stop.
## 1. Every investigation task gets a visible budget
An open-ended tool; for them the permuter, for us "rebuild and run the game
@@ -147,6 +186,68 @@ justifies. Rules:
is still true as a fresh, small PR against current `main`. Merging stale
content back over newer information is a regression, not a save.
## 7. The rule-2 sign-off procedure (genuine `src/game` behavior changes)
Use this only for bucket (c) from the diagnosis framing above — never for
the ABI/layout exception (§A1-class fixes need no sign-off, only
documentation) and never for anything expressible in `port/`.
**To request sign-off, the write-up must state:**
1. The exact `src/game` file:line and the specific behavior being
proposed for change.
2. **Proof of divergence.** Given verified-correct inputs — i.e. every
upstream ABI/layout suspect (§A1) and every port-layer suspect has
been checked and ruled out, with the ruling-out cited — the
byte-identical decomp code still produces behavior that differs from
real N64 reference behavior. Name the reference (a capture, documented
prior N64 behavior, a cited external source). "It looks wrong" is not
proof; "it disagrees with `<reference>`, verified by `<method>`" is.
3. Why the fix cannot be expressed in `port/`, and why it does not
qualify under the ABI/layout exception — the two questions that must
be answered "no" before this procedure applies at all.
4. The proposed fix, scoped as tightly as possible to the one named
behavior, plus a same-engine precedent if one exists — the Perfect
Dark port is the standing reference; cite the analogous PD change the
way `docs/dev/WIDESCREEN-FOV-PLAN.md`'s "Rule #2 exception framing"
section did.
**Granting.** The user reviews the above and replies with an explicit
go-ahead in-thread. There is no implicit or inferred sign-off: silence,
"seems reasonable," a prior similar grant, or approval of a *different*
behavior in the same file do not carry over to a new request.
**Recording.** Every sign-off gets its own `docs/dev/findings.md` entry
(next `Dxx` label), tagged **`RULE-2-SIGNOFF`** in its status line
alongside the normal finding classification, and recording: the request
as stated above, the grant (quoted or summarized, with date), and the
resulting diff. Add the label to `findings-index.csv` like any other row,
so `tools_pc/gen_findings_index.py` keeps it discoverable — chat history
is not the record of approval; the finding entry is.
**Constraints that still apply after approval:**
- Still `#ifdef PORT` — the N64 build is untouched, and the `#else` arm
(or the file's un-PORT-guarded default) still matches the original
decomp exactly.
- Still scoped to the one named behavior — a grant is not a standing
license; a different behavior difference in the same file needs its own
request, even if related.
- Still fully documented in the `RULE-2-SIGNOFF` finding entry — that
entry is the permanent record of *why* this specific exception to
non-negotiable #2 exists.
- If the fix would visibly change behavior at *default*
settings/difficulty/region beyond the one bug being fixed, treat that
as a signal the scope drifted mid-implementation — stop and go back for
a fresh sign-off on the wider scope rather than shipping it under the
narrower original grant.
This generalizes the ad hoc precedent in
`docs/dev/WIDESCREEN-FOV-PLAN.md`'s "Rule #2 exception framing" section (a
real, already-granted case, kept as historical record) into a reusable
process; new requests cite this section rather than re-deriving the case
for a process each time.
## Investigation-brief template
```
@@ -156,7 +257,16 @@ FILES YOU MAY TOUCH: <disjoint from any other in-flight work>.
KNOWN-GOOD / RULED OUT: <list; do not re-investigate>.
PRE-FLIGHT ATTACHED: <probe output, baseline capture, PD-port pointer>.
BUDGET: <N build->run cycles / ~M min>. On expiry: revert probes, write up with confidence.
CONSTRAINTS: no game-logic changes; ABI/layout/format only; #ifdef PORT; documented in the finding log.
CONSTRAINTS: diagnosis may trace as deep into src/game state as the evidence
needs — name the exact struct/field/logic responsible before concluding
which bucket applies (see "Diagnosis is unrestricted" above). The FIX is
still constrained: no src/game behavior changes unless (a) it's an
ABI/layout-class struct-layout or pointer-width misread — check
docs/porting-notes.md §A1 first; #ifdef PORT; documented in the finding
log under §F/D3x — or (b) it's fixable entirely in port/ (no src/game
touch). A genuine game-logic behavior difference (bucket c) is NOT
authorized by this brief — stop and write up a rule-2 sign-off request
per docs/dev-process.md §7 instead of applying it.
VERIFY: <the exact tools_pc/verify.sh (or probe) invocation that proves this done; or, if runtime verification is impossible in this environment, say so and name what a human must run>.
REPORT: (a) root cause + file:line evidence (b) fix diff, or why not
(c) probes left in tree (d) confidence.
+4
View File
@@ -203,6 +203,10 @@ byte-identical to today.
### Rule #2 exception framing (needs explicit sign-off to start Phase 2)
(This project's now-generalized rule-2 sign-off procedure lives in
`docs/dev-process.md` §7; this section is the original case that
motivated it, kept for its PD-precedent argument.)
This is a behavior-affecting game-code edit class, outside the ABI-only D3x
exception. The case for granting it:
+54
View File
@@ -10664,6 +10664,60 @@ crash class C2 (Facility `-level_34`, Runway `-level_35`): `import_texture_i8` A
**Status (from §F table):** OPEN — ninth pass closed with no fix; see D276/D280
**Next pass framing (post rule-2-clarification):**
```
TASK: D236 pass 10 — determine why the 0x0c184b50 discrete tree-card class
instantiates inconsistently (~4 tris on deterministic -level_36 boot vs
~720 in a free run) despite being geometrically in front of the confirmed
N64-faithful noise-quad "wall" (D280).
READ FIRST: docs/porting-notes.md §A1 (pattern + tells, now including D255);
docs/dev-process.md's "Diagnosis is unrestricted" section and §7 sign-off
procedure; docs/dev/findings.md D236 full entry (D265, D271, D272,
D273/D274, D276, D280) plus D209/D210/D255 as worked A1 examples.
FILES YOU MAY TOUCH: none pre-committed — this pass is diagnosis-first.
Touching src/game requires first clearing buckets (a) and (b) below, with
evidence, per CONSTRAINTS.
KNOWN-GOOD / RULED OUT (do not re-investigate): IA4/IA8 decode, blend
pipeline, fog combine (verified N64-faithful, ±2 truncation over 2505
verts), portal-culling over-draw (separately fixed, not the cause),
converter/UV-conversion, CI palette bit-layout, mip presence, "identical UV
window" for the CI8 classes.
PRE-FLIGHT ATTACHED: D280's z/w depth-race data (0x0c184b50 confirmed in
front of the noise quad); the instantiation tri-count delta (4 vs 720)
between deterministic and free boot.
BUDGET: <=6 build->run cycles / ~90 min. On expiry: revert probes, write up
with confidence, no forced fix.
CONSTRAINTS: Trace the instantiation-count/gating decision into src/game
(prop-spawn, room-streaming, or distance-cull logic — likely bg.c) as far
as the evidence leads; name the exact struct/field/counter responsible
before concluding anything. Specifically, before writing this up as bucket
(c) (a genuine timing/streaming behavior difference worth a rule-2 sign-off
request):
- Check bucket (a) first: is the count/visibility-gating field read at a
struct offset that shifted under 32->64-bit pointer widening? (§A1
tells: deterministic-but-wrong output; a raw/literal offset or narrow
field read into a struct/union with any pointer member anywhere in its
layout.)
- Check bucket (b) next: is a port-layer timing/order-of-operations
difference (frame pacing, load order, an uninitialized or
differently-timed port-owned counter) feeding src/game a different
input than N64 would have produced upstream of the gating logic?
- Only once both are actively ruled out with cited evidence (not
assumption) does this qualify for a rule-2 sign-off write-up under
docs/dev-process.md §7. Do not request sign-off preemptively or as a
fallback for "didn't find an ABI/port explanation in the time budget."
VERIFY: the deterministic -level_36 boot repro + tri-count probe already
used in D280; attach before/after instantiation counts for any change.
REPORT: (a) the exact struct/field/counter gating instantiation, file:line,
and which bucket (a/b/c) it falls in, with the evidence for that
classification (b) fix diff if (a) or (b), or a full rule-2
sign-off request per §7 if (c), never a partial/ambiguous one
(c) probes left in tree (d) confidence.
Append any generalisable quirk to docs/porting-notes.md — extend
§A1 if this turns out to be another instance of the pattern.
```
## D237 — QoL ask (M-106, user QA): F10 options overlay — make it scrollable with the mouse wheel, expose more settings into it (e.g. screen-flash / no-hit-flash…
**QoL ask (M-106, user QA): F10 options overlay — make it scrollable with the mouse wheel, expose more settings into it (e.g. screen-flash / no-hit-flash `Game.NoHitFlash`, and other easily-exposed config keys), and organise rows by category (mouse settings together, video together, etc.).** The overlay (`port/src/optionsoverlay.c`) already renders over the registered `config.c` option list; what's missing is wheel-scroll nav, a broader row set, and grouping headers. Cross-ref `OPTIONS-MENU-PLAN.md` §3 (v1 minimum set) + QOL-INVENTORY "PD has, GE lacks".
+56 -8
View File
@@ -55,8 +55,9 @@ state.
`.act_*.attack_item`; aliased `WeaponObjRecord.weaponnum` at 0x80 on
N64 via act-union@0x2C+84; act union moves to ~0x38 on PC).
**A1. Raw-byte aliases into a union (`act_ubytes.padding[N]`); the silent
variant.** The nastiest members of this class do not crash: a literal byte
**A1. Raw-byte or mistyped aliases into a union/struct whose true field
shifted because a sibling member widened; the silent variant.** The nastiest
members of this class do not crash: a literal byte
index into a union that overlays pointer-bearing structs still *reads*, it
just reads the wrong byte, so the game runs and merely behaves wrong. Grep
for hardcoded indices into any `padding[]`/`u8[]` union arm.
@@ -86,13 +87,60 @@ for hardcoded indices into any `padding[]`/`u8[]` union arm.
named writes of the *same* arm. The rest of the `padding[N]` sweep
(`chrlvTickAnim`/`chrlvTickDead` → `act_anim`/`act_dead`, both pointer-free)
is layout-stable.
- **D255** (a mistyped-field variant, not a raw-offset variant — same
mechanism, different syntax): `vtxstore_fix_refs` read a record's
`Model*` through `((Model*)((ChrRecord*)prop->chr)->chrflags)` —
`ChrRecord.chrflags` is typed `CHRFLAG`, a **4-byte enum**. On 32-bit
N64 the cast is bit-exact; on 64-bit PC it reads only the pointer's
low 32 bits and zero-extends, discarding the real high bits. The
truncated result is `(nil)` whenever the real pointer's low 32 bits
happen to be zero — which read exactly like a legitimate NULL-model
case, and cost two prior investigation sessions chasing a
"destroy-path race" theory before the field itself was the suspect.
**Tell that generalizes past D209/D210's `padding[N]` shape:** it
doesn't have to be a raw numeric offset — a field that is *correctly
named* but *undersized/mistyped relative to a pointer it's secretly
meant to hold* (an enum, a `u32`, a `s32` doing double duty as a
pointer slot) is the same bug through a different syntax. Fix: read
through the correctly-typed sibling field (`ObjectRecord.model`)
instead, `#ifdef PORT`, N64 line/cast kept verbatim under `#else`.
§F **D255** (M-140).
**Lesson.** When a value reads as a clean constant (0, 1) rather than
garbage, a pointer byte is a prime suspect; the high bytes of a low-4GB
heap pointer are all zero, so a misaligned read looks like a legitimate
"feature off" value. Prefer the named field under `#ifdef PORT`; the alias
is only correct at 32-bit pointer width. Also: verify that every caller
reaches the site with the union arm you are naming actually active.
**Lesson.** When a value reads as a clean constant (0, 1, NULL) rather
than garbage, a pointer byte is a prime suspect; the high bytes of a
low-4GB heap pointer are all zero, so a misaligned or truncated read
looks like a legitimate "feature off" / "not set" value. **Tell-tale
signs of this pattern, any one of which should trigger an A1-first
check before assuming a bug is genuine game-logic behavior:**
- The observed value is **byte-exact-looking but wrong from the very
first call/frame** — not intermittent corruption, not "usually right
but occasionally wrong."
- A **raw/literal offset or byte-width read** into a union or struct
that has **any pointer member**, anywhere in that union/struct's
layout (not just the field being read) — the read doesn't need to
touch the pointer itself; any member *after* one that widened is at
risk.
- A field that is **correctly named but too narrow to hold what it's
actually storing** (an enum/`u32`/`s32` reused as a truncated pointer
slot — the D255 shape, distinct from D209/D210's literal-offset shape
but the same underlying cause).
- The resulting behavior is **deterministic and internally
self-consistent, but demonstrably not what the N64 original would
produce** — the code isn't crashing or producing garbage, it's
confidently doing the wrong (but stable) thing.
- Prefer the named field (or a correctly-typed sibling field) under
`#ifdef PORT`; the alias/pun is only correct at 32-bit pointer width.
Verify every caller reaches the site with the union/struct arm you're
naming actually active.
**Distinguish from D284** (`port/src/random.c`'s MIPS64
shift-emulation bug): same *epistemic* shape — deterministic,
self-consistent, silently not-N64 — but a different *mechanism*
(instruction semantics, not struct layout/pointer width) and,
critically, **entirely inside `port/`, no `src/game` touch at all**.
D284 was never a rule-2 question in the first place; don't fold it
into this catalogue's "looks like game logic, is really ABI"
framing — it never looked like game logic to begin with.
- **Open landmine:** raw hardcoded-offset accessors into `struct player`
/ `struct hand`; see `docs/dev/AUDIT-M6-player-offsets.md`.
- **D126 corollary; a trailing runtime list pointer in a ROM-serialized