diff --git a/crates/gc-extract/tests/disc.rs b/crates/gc-extract/tests/disc.rs index 0ee17e3..f3bdab5 100644 --- a/crates/gc-extract/tests/disc.rs +++ b/crates/gc-extract/tests/disc.rs @@ -5789,7 +5789,11 @@ fn moby_class_ids_come_from_lvl_vtbl_not_from_the_class_count() { levels += 1; } 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 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, ); - 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!( above * 100 >= classes.len() * 97, @@ -5907,7 +5913,7 @@ fn vtbl_class_ids_are_global_and_do_not_index_the_class_table() { 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"); // the load-bearing claim: the identifier space does not fit in a level's table assert!( @@ -6009,7 +6015,6 @@ fn camera_and_sound_dispatch_tables_decode_at_their_documented_widths() { "{label}: {records} records, {} distinct ids, max {max:#x}", ids.len() ); - assert!(records > 0, "{label}: no records"); assert!( ids.len() < records, "{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)); } - 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![("camvtbl", 14, 217), ("sndvtbl", 6, 32)], + "dispatch table counts 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].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 { let off = r * 12 + 4; 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 { break; } diff --git a/docs/formats/core-data-regions.md b/docs/formats/core-data-regions.md index c61b427..cb47216 100644 --- a/docs/formats/core-data-regions.md +++ b/docs/formats/core-data-regions.md @@ -3557,6 +3557,16 @@ is measuring. **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 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.