Rule 3b, requested after a commit went out with a failing test in it.
cargo test -q --workspace | grep "^test result: ok" | awk '{s += $4}'
That sums the green test binaries and ignores the red one. A broken build still
prints a number, and the number looks entirely reasonable -- six healthy counts
added together is not a number anyone questions. I used that filtered sum as the
evidence of health in commit messages for the better part of a session, and it
was reporting green on a build that was not.
What actually happened is the part worth keeping. The count came back lower than
the previous run -- 100 instead of 278. That is a genuine signal and I explained
it away twice as an environment difference, the disc not being set, before
checking. Both explanations were plausible. Neither was tested. Only the
discrepancy was noticed, and noticing it is what surfaced the failure.
So the rule covers both halves: do not commit or push with a failing test, and do
not derive the number you report with a filter that drops the failing binaries.
The second half is the one that is easy to get wrong again, because
`cargo test -q` is quiet, the grep looks like a reasonable way to total up, and
the total is the only part anyone reads.
Also records the mistake as a mechanism rather than a symptom. Same defect as
rules 3 and 3a, one level up: a quantity that should have had one definition had
two, and nothing compared them.
Added a line to "Working rhythm" pointing at it, since that is the list someone
reads before committing.
243 lines
11 KiB
Markdown
243 lines
11 KiB
Markdown
# AGENTS.md — working agreements for this repository
|
|
|
|
Guidance for coding agents and for anyone picking this up cold. Read this
|
|
before changing anything.
|
|
|
|
## What this project is
|
|
|
|
A from-scratch native PC reimplementation of **Ratchet & Clank: Going Commando**
|
|
(PlayStation 2, 2003), written in Rust. The game is **reimplemented**, not
|
|
emulated and not translated: there is no MIPS interpreter and no CPU
|
|
emulator anywhere in this repository.
|
|
|
|
The goal is a build that plays the game the way the original does. "Close
|
|
enough" is not the bar — the point is that it feels like the game you remember.
|
|
|
|
## Hard rules
|
|
|
|
### 1. No game data in this repository. Ever.
|
|
|
|
Not the ISO, not extracted assets, not textures, not models, not audio, not
|
|
BIOS dumps, not SDK binaries. Not in a branch, not in a stash, not "just for a
|
|
test".
|
|
|
|
- Everything is covered by `.gitignore`.
|
|
- `.githooks/pre-commit` blocks disc images, asset directories, container
|
|
signatures and files over 50 MiB. It is activated via
|
|
`git config core.hooksPath .githooks` — **check that this is still set** in a
|
|
fresh clone, or run `git config core.hooksPath .githooks`.
|
|
- To undo the hook for a legitimate source change: `git commit --no-verify`.
|
|
Legitimate source changes should not need it.
|
|
- Game data is an **input**, supplied by the user from their own legally
|
|
obtained disc. Repositories that need the disc state its serial and hashes.
|
|
|
|
### 2. Reproduce the PS2 float model.
|
|
|
|
The Emotion Engine's FPU is not IEEE-754. It has **no denormals**, and rounding
|
|
and exception behaviour differ from x86 in ways that are visible in gameplay.
|
|
|
|
Gameplay and animation maths must model this deliberately rather than using
|
|
Rust's native `f32` and hoping. When porting a routine, find out how the
|
|
original computes it and reproduce that, including the odd cases — ReRAC's
|
|
reference implementation deliberately reproduces a "post-scale list bug" in the
|
|
moby animation evaluator because doing so is what makes the result match.
|
|
|
|
Do not "fix" something that looks like a bug until you have confirmed the
|
|
original does the same thing. Fidelity beats correctness.
|
|
|
|
### 3. Never report a number without its bounds and its method.
|
|
|
|
Any size, count or distribution quoted in a commit message, a doc or a test
|
|
comes with two things attached: **which byte range it describes**, and **how
|
|
it was measured**.
|
|
|
|
This is not ceremony. Four separate numbers in one session looked
|
|
load-bearing and turned out to be artifacts:
|
|
|
|
- a format identified as VIF-packed, from a handful of recurring byte
|
|
patterns, where a control put it on the random baseline;
|
|
- a body size computed over a span whose bounds were wrong, so the last
|
|
class absorbed most of the lump's tail;
|
|
- a "6.2 MB of unidentified data" region that was texture data;
|
|
- "3 unexplained blocks" that were an artifact of sampling too coarsely.
|
|
|
|
All four shared one cause: measuring before bounding, and naming bytes before
|
|
looking at them. None of them errored, which is why they survived long enough
|
|
to be believed.
|
|
|
|
### 3a. Partition the measurement, or you have not measured it.
|
|
|
|
A number computed over a heterogeneous population describes nobody. Report a
|
|
measurement **per partition** -- per level, per chunk type, per record size --
|
|
before reporting it in aggregate.
|
|
|
|
Three errors in one stretch were all of this kind, and all three survived the
|
|
control check that rule 3 asks for, because a matched control bounds
|
|
*significance* while saying nothing about *scope*:
|
|
|
|
- a 368-byte "period" attributed to a whole region, when it was the stride of
|
|
one chunk among ~15; averaging them put the peak at the noise floor, which
|
|
read as "no structure" rather than "wrong scope";
|
|
- a ten-entry table disconfirmed, when it existed at a fixed stride among
|
|
several;
|
|
- a moby class array reported at `u16` slots 129-140, which was simply the
|
|
largest-stride chunk type dominating a pooled scan. Per type, every one of
|
|
the 26 places its array somewhere else.
|
|
|
|
In every case the aggregate number was arithmetically correct and the
|
|
conclusion drawn from it was wrong. The re-run that caught all three was the
|
|
same each time: group by the thing that varies, then look.
|
|
|
|
- State the partition before the number, not after.
|
|
- If a claim is made over a pooled set, also report it for each partition, or
|
|
say explicitly that it is pooled and unverified.
|
|
- A structure found in one partition is not a property of the whole until
|
|
every partition agrees.
|
|
- When a result changes under partitioning, the pooled result was not a weak
|
|
version of the truth. It was a different and wrong claim. Retract it rather
|
|
than soften it.
|
|
|
|
### 3b. Never report a test count that a filter could have hidden a failure in.
|
|
|
|
Do not commit or push with a failing test. If one fails, it is fixed or reverted
|
|
in the same commit — never "fixed in the next one".
|
|
|
|
The same rule governs the *number* you report for the suite. Do not derive it
|
|
with a filter that drops the failing binaries:
|
|
|
|
```sh
|
|
# WRONG. Sums only the binaries that passed. A red build still prints a number,
|
|
# and the number looks fine.
|
|
cargo test -q --workspace | grep "^test result: ok" | awk '{s += $4} END {print s}'
|
|
```
|
|
|
|
A commit was pushed with a broken test because of exactly this. The filtered sum
|
|
reported a healthy count, because `^test result: ok` matches the six green test
|
|
binaries and ignores the seventh, which was red. The giveaway was that the count
|
|
came back lower than the previous run's, and the difference was mistaken for
|
|
"the disc tests were skipped" rather than "one of them failed". It was read as
|
|
an environment difference and moved on.
|
|
|
|
Use the exit status, or count the lines that are not green:
|
|
|
|
```sh
|
|
cargo test --workspace # then read every "test result:" line
|
|
cargo test --workspace && echo ok # the exit status is the real answer
|
|
```
|
|
|
|
Concretely:
|
|
|
|
- Run the full suite with the disc present when the change touches disc-decoding
|
|
code, and say so in the commit body. A skipped test and a passing one both
|
|
report success.
|
|
- If the reported count moves between runs, find out why before writing anything
|
|
down. A drop means either an environment difference or a failure, and the two
|
|
look identical from the summary line.
|
|
- Never quote a number you computed rather than read.
|
|
|
|
This is the same defect as rules 3 and 3a, one level up: a quantity that should
|
|
have had one definition had two, and nothing compared them.
|
|
|
|
### 4. Controls, and heuristic counts that are not identifications.
|
|
|
|
Concretely:
|
|
|
|
- Derive ranges from `gc_formats::regions::core_layout`, not by running from
|
|
one known offset to `core_data.len()`. Where a bound is unknown it is `None`,
|
|
not a guess.
|
|
- Before calling a byte pattern a format, test it against a control. If a
|
|
statistic cannot tell your format apart from uniform random, it cannot tell
|
|
you anything.
|
|
- A heuristic count is not an identification. Count distinct byte values, then
|
|
*look* at the blocks that stand out before naming them.
|
|
|
|
### 5. Verify against the real disc, not against a guess.
|
|
|
|
A parser is correct when it round-trips your own disc's bytes, checked by hash.
|
|
Commit size-and-hash tables (never content) so a test can assert "this matches
|
|
retail" rather than "this matches whatever was on my disk".
|
|
|
|
### 6. Bound every read.
|
|
|
|
Asset data is untrusted input from a 2003 disc format. Parsers return typed
|
|
errors; they never panic and never read past the end of a buffer.
|
|
|
|
## Orientation
|
|
|
|
Read these before working in an unfamiliar area:
|
|
|
|
| Path | What it is |
|
|
|---|---|
|
|
| `docs/disc-identification.md` | Which disc we build against, verified |
|
|
| `docs/roadmap.md` | Phases, gates, and what is done |
|
|
| `docs/plan/` | One file per subsystem, written **as work proceeds** |
|
|
| `docs/formats/` | Format specs, written **as formats are reversed** |
|
|
| `PROVENANCE.md` | Upstream projects we build on, and their licences |
|
|
| `docs/plan/pcsx2-verification.md` | PCSX2 is set up on this machine, and how to run the comparison loop |
|
|
| `vendor/reRAC/` | Pinned reference checkout (not tracked) |
|
|
|
|
## Crates
|
|
|
|
| Crate | Responsibility |
|
|
|---|---|
|
|
| `gc-formats` | Parse asset formats into plain Rust values. No I/O beyond the caller's buffer. |
|
|
| `gc-extract` | Disc pipeline: identify, unpack, verify. |
|
|
| `gc-game` | Game logic: hero, weapons, classes, progression, saves. |
|
|
| `gc-video` | IPU/MPEG-2 decode. |
|
|
| `gc-engine` | Bevy app: GS, VU0/VU1, renderer, frame loop. Also owns the **hardware models** — `ps2.rs` is the Emotion Engine's FPU, and the tfrag lighting pass runs on it. |
|
|
|
|
The dependency direction is `gc-formats` ← `gc-extract`; `gc-formats` ←
|
|
`gc-game` ← `gc-engine`; `gc-video` ← `gc-game` ← `gc-engine`. `gc-formats`
|
|
depends on nothing of ours. Do not invert these — a format parser that can
|
|
reach into the engine cannot be tested in isolation.
|
|
|
|
A hardware model belongs in `gc-engine`, not `gc-formats`. `gc-formats`
|
|
decodes *inputs* — including the data a hardware model consumes, such as the
|
|
overlay's sine table — but arithmetic that reproduces how the PS2 computes
|
|
something is an engine concern. `tfrag_light.rs` (formats) holds the normal
|
|
table and the light bank framing; `tfrag_light.rs` (engine) holds the pass
|
|
that uses them.
|
|
|
|
## Conventions
|
|
|
|
- Rust 2021 edition, workspace inheritance for all shared config. Do not
|
|
hardcode a version in a member crate.
|
|
- `rustfmt` defaults. Clippy warnings are errors in spirit; do not silence them
|
|
without a comment explaining why.
|
|
- Error types: `thiserror` for library errors in `gc-formats` and `gc-game`,
|
|
`anyhow` at the binary/`xtask` boundary.
|
|
- Comments explain **why**, especially for anything that looks like a bug. The
|
|
`.githooks` and crate docs deliberately have few inline comments; match the
|
|
surrounding density rather than adding noise.
|
|
- Tests live beside the code (`#[cfg(test)] mod tests`) for unit-level checks,
|
|
and in `tests/` for golden-file checks against real disc data.
|
|
|
|
## Working rhythm
|
|
|
|
This is a long project. Keep it navigable:
|
|
|
|
1. Work in small, coherent commits with a substantive body explaining what was
|
|
non-obvious. Match the existing style in `git log`.
|
|
2. **Update `docs/roadmap.md`** when a phase advances or a gate is met.
|
|
3. **Write the `docs/plan/*.md` file for a subsystem when you start it**, and
|
|
update it as you learn. The reasoning is more valuable than the code; the
|
|
code is in git.
|
|
4. **Update this file** if you change a convention.
|
|
|
|
## When you get stuck
|
|
|
|
Reverse engineering means long stretches of not knowing. That is normal, not a
|
|
failure. Write down what you tried and what it ruled out in the relevant
|
|
`docs/plan/` file before moving on — the next attempt (which may be you, in
|
|
three months) needs the negative results as much as the positive ones.
|
|
|
|
If something is genuinely blocked, say so plainly and say what would unblock
|
|
it. Do not paper over a gap with an approximation that looks like it works.
|
|
|
|
## Legal
|
|
|
|
Ratchet & Clank is a trademark of Sony Interactive Entertainment, developed by
|
|
Insomniac Games. This is an unofficial fan project, not affiliated with or
|
|
endorsed by either. We redistribute no game content. Upstream projects we learn
|
|
from keep their own licences — see `PROVENANCE.md`, and keep those notices
|
|
intact when porting their code. |