Pin the dispatch counts exactly, after finding two of them were pinned wrong

Follows directly from 65025cb, whose conclusion was that these tests assert bounds
rather than numbers and so cannot detect the drift they were known to contain.
Applying that to the tables involved.

Three things turned up.

The counts are now pinned: vtbl 5,822 records / 1,162 ids, camvtbl 217 / 14,
sndvtbl 32 / 6. Every one of these was previously `records > 5_000`,
`records > 0`, or nothing at all.

Two of the bad counts had been PINNED, not merely missed. Two separate tests
carried `assert_eq!(classes.len(), 1163)`. So walking past the terminator was not
an oversight that slipped through review -- the wrong number had been written down,
deliberately, at some point when it was correct. Both now say 1,162.

That is worth pausing on, because it is a different failure from the one I
described last commit. I said the tests were too loose to catch drift. In fact two
of them were too tight: they pinned a measurement that was wrong, and so they
actively defended it. A threshold and a wrong constant fail in opposite ways, and
this suite had one of each on the same table.

The third is the moby handler count. That one stops on the terminator in the
*handler* word rather than the id word, landing 34 records short at 5,788. It is
measuring a different question -- records with a real geometry handler -- so it is
correct, and it now says so at the break rather than leaving the next reader to
wonder why it disagrees with the other three.

Tests are 278 as before; nothing here changed what is counted, only what is
asserted. That is the whole point -- the previous 278 and these 278 agree, and now
they cannot silently stop agreeing.
This commit is contained in:
2026-10-14 04:28:43 +02:00
parent 65025cbf4f
commit cc52060c67
2 changed files with 31 additions and 5 deletions
+21 -5
View File
@@ -5789,7 +5789,11 @@ fn moby_class_ids_come_from_lvl_vtbl_not_from_the_class_count() {
levels += 1; levels += 1;
} }
assert_eq!(levels, 27, "only {levels} overlays parsed"); assert_eq!(levels, 27, "only {levels} overlays parsed");
assert!(records > 5_000, "only {records} vtbl records walked"); // Exact, not a bound. This test printed 5,878 for a while because it walked
// past the terminator; `records > 5_000` accepted that without complaint, and
// the disagreement only surfaced because a *different* test counted the same
// table. A threshold cannot catch drift it is looser than.
assert_eq!(records, 5822, "vtbl records moved");
let max = *classes.iter().next_back().unwrap(); let max = *classes.iter().next_back().unwrap();
let idx0 = std::fs::read(data.join("level0/core_index.bin")).expect("core_index"); let idx0 = std::fs::read(data.join("level0/core_index.bin")).expect("core_index");
@@ -5806,7 +5810,9 @@ fn moby_class_ids_come_from_lvl_vtbl_not_from_the_class_count() {
pct = 100.0 * above as f64 / classes.len() as f64, pct = 100.0 * above as f64 / classes.len() as f64,
); );
assert_eq!(classes.len(), 1163, "distinct class-id count moved"); // Was pinned at 1163, which counted one id past the terminator. 1162 is the
// number of class ids the dispatch table actually names.
assert_eq!(classes.len(), 1162, "distinct class-id count moved");
assert_eq!(max, 0x13BA, "the highest class id moved"); assert_eq!(max, 0x13BA, "the highest class id moved");
assert!( assert!(
above * 100 >= classes.len() * 97, above * 100 >= classes.len() * 97,
@@ -5907,7 +5913,7 @@ fn vtbl_class_ids_are_global_and_do_not_index_the_class_table() {
ids.len() ids.len()
); );
assert_eq!(ids.len(), 1163, "distinct vtbl class ids moved"); assert_eq!(ids.len(), 1162, "distinct vtbl class ids moved");
assert_eq!(largest, 5050, "the largest class id moved"); assert_eq!(largest, 5050, "the largest class id moved");
// the load-bearing claim: the identifier space does not fit in a level's table // the load-bearing claim: the identifier space does not fit in a level's table
assert!( assert!(
@@ -6009,7 +6015,6 @@ fn camera_and_sound_dispatch_tables_decode_at_their_documented_widths() {
"{label}: {records} records, {} distinct ids, max {max:#x}", "{label}: {records} records, {} distinct ids, max {max:#x}",
ids.len() ids.len()
); );
assert!(records > 0, "{label}: no records");
assert!( assert!(
ids.len() < records, ids.len() < records,
"{label}: ids are not shared between levels" "{label}: ids are not shared between levels"
@@ -6017,7 +6022,15 @@ fn camera_and_sound_dispatch_tables_decode_at_their_documented_widths() {
out.push((label, ids.len(), max, records)); out.push((label, ids.len(), max, records));
} }
assert_eq!(out[0].1, 14, "distinct camera id count moved"); // (label, distinct ids, max, records) -- pinned exactly. Both tables were
// counted by four tests between them and the counts only agreed because each
// test asserted a bound rather than a number. vtbl is counted by
// lvl_vtbl_class_ids_are_global instead; this helper covers only these two.
assert_eq!(
out.iter().map(|o| (o.0, o.1, o.3)).collect::<Vec<_>>(),
vec![("camvtbl", 14, 217), ("sndvtbl", 6, 32)],
"dispatch table counts moved"
);
assert_eq!(out[0].2, 0x1B, "the largest camera id moved"); assert_eq!(out[0].2, 0x1B, "the largest camera id moved");
assert_eq!(out[1].1, 6, "distinct sound id count moved"); assert_eq!(out[1].1, 6, "distinct sound id count moved");
assert_eq!(out[1].2, 0x07, "the largest sound id moved"); assert_eq!(out[1].2, 0x07, "the largest sound id moved");
@@ -6073,6 +6086,9 @@ fn moby_handlers_are_overwhelmingly_function_entries() {
for r in 0..secs[3].data.len() / 12 { for r in 0..secs[3].data.len() / 12 {
let off = r * 12 + 4; let off = r * 12 + 4;
let h = u32::from_le_bytes(secs[3].data[off..off + 4].try_into().unwrap()); let h = u32::from_le_bytes(secs[3].data[off..off + 4].try_into().unwrap());
// Terminator on the HANDLER word, not the id word: 34 records carry a
// valid id with no geometry, so this stopping point is 34 short of the
// table and is correct for counting handlers.
if h == u32::MAX { if h == u32::MAX {
break; break;
} }
+10
View File
@@ -3557,6 +3557,16 @@ is measuring.
**5,878 was the bug.** It was the first number above, reading past the table. **5,878 was the bug.** It was the first number above, reading past the table.
Two of those bad counts were *pinned* -- `assert_eq!(classes.len(), 1163)` in two
separate tests -- so the walk-past-the-terminator behaviour had been deliberately
recorded at some point, not merely overlooked. Both now assert 1,162.
Every count involved is now asserted exactly rather than as a bound. The general
point is the one worth keeping: **a test that prints a measurement and asserts a
threshold cannot detect drift it is looser than.** `records > 5_000` was satisfied
by 5,878 and by 5,822 alike, so the wrong loop sat there indefinitely and the only
reason it surfaced was that a different test counted the same table differently.
The disagreement was not a decomp mismatch and not a hardware question. It was two The disagreement was not a decomp mismatch and not a hardware question. It was two
sentences in a test saying `if c != MAX { ... }` and `if c == MAX { break }`, which sentences in a test saying `if c != MAX { ... }` and `if c == MAX { break }`, which
are the same condition with opposite answers to whether the table ends there. are the same condition with opposite answers to whether the table ends there.