diff --git a/crates/benilla-formats/tests/wmo_short_mogp.rs b/crates/benilla-formats/tests/wmo_short_mogp.rs new file mode 100644 index 00000000..eb2f52b0 --- /dev/null +++ b/crates/benilla-formats/tests/wmo_short_mogp.rs @@ -0,0 +1,99 @@ +//! A group WMO's MOGP runs to EOF, whatever size it declares. +//! +//! MOGP is a group file's last top-level chunk. Seven files in this realm's chain declare less +//! than they hold, which left their geometry sub-chunks as top-level siblings *after* the declared +//! payload: the parse answered a group with no vertices, no indices and no batches — a building +//! that draws nothing, with no error anywhere to say why. +//! +//! `wmo_chunk_overrun.rs` covers the other direction (Undercity_144 declares one byte *past* EOF) +//! and only asks for the MOGP header, which survives either way. This asks for the geometry. + +use benilla_formats::Chain; +use benilla_wmo::{parse_wmo, ParsedWmo}; + +/// The seven: six Shadowmoon Slagpit groups declaring 0x40 against a 0x44 header, and a Westfall +/// house group declaring 0. All seven are the winning copy in this chain (`patch-3.mpq`). +#[test] +fn the_short_mogp_groups_in_the_chain_yield_their_geometry() { + let data = benilla_formats::wow_data_or_skip!(); + let chain = Chain::open(&data).expect("open vanilla patch chain"); + + let mut checked = 0u32; + for name in [ + "World\\wmo\\Outland\\Shadowmoon\\Slagpit\\Shadowmoon_Slagpit01_000.wmo", + "World\\wmo\\Outland\\Shadowmoon\\Slagpit\\Shadowmoon_Slagpit01_001.wmo", + "World\\wmo\\Outland\\Shadowmoon\\Slagpit\\Shadowmoon_Slagpit01_002.wmo", + "World\\wmo\\Outland\\Shadowmoon\\Slagpit\\Shadowmoon_Slagpit01_003.wmo", + "World\\wmo\\Outland\\Shadowmoon\\Slagpit\\Shadowmoon_Slagpit01_004.wmo", + "World\\wmo\\Outland\\Shadowmoon\\Slagpit\\Shadowmoon_Slagpit01_005.wmo", + "HumanHouse\\WestfallHouse_MediumStraw_000.wmo", + ] { + let Ok(bytes) = chain.read(name) else { + continue; // a realm that stops shipping one of these is not a failure here + }; + // The premise: the declared MOGP size really is short of the payload. + let declared = u32::from_le_bytes([bytes[0x10], bytes[0x11], bytes[0x12], bytes[0x13]]); + assert!( + &bytes[0x0c..0x10] == b"PGOM" && 0x14 + (declared as usize) < bytes.len(), + "{name}: expected a MOGP declaring less than it holds" + ); + let Ok(ParsedWmo::Group(g)) = parse_wmo(&mut std::io::Cursor::new(bytes.as_slice())) else { + panic!("{name}: expected a group"); + }; + assert!( + !g.vertex_positions.is_empty() && !g.vertex_indices.is_empty(), + "{name}: parsed to an empty group ({} verts, {} indices)", + g.vertex_positions.len(), + g.vertex_indices.len() + ); + checked += 1; + } + assert!( + checked >= 6, + "only {checked} of the seven were in the chain" + ); +} + +/// The invariant the seven broke, over the whole corpus: a group file with real bytes in it parses +/// to real geometry. 0-byte and stub files ship in the corpus and are not groups with lost +/// geometry, so the floor is 1 KiB — every one of the seven is 32 KiB or more. +#[test] +fn no_substantial_group_wmo_parses_to_an_empty_group() { + let data = benilla_formats::wow_data_or_skip!(); + let mut chain = Chain::open(&data).expect("open vanilla patch chain"); + let mut paths: Vec = chain + .list() + .expect("list the patch chain") + .into_iter() + .map(|e| e.name) + .filter(|n| n.to_ascii_lowercase().ends_with(".wmo")) + .collect(); + paths.sort(); + paths.dedup(); + assert!( + paths.len() > 5000, + "expected the full WMO corpus, found {} files", + paths.len() + ); + + let mut checked = 0u32; + let mut empty: Vec = Vec::new(); + for p in &paths { + let Ok(b) = chain.read_file(p) else { continue }; + if b.len() < 1024 || b.len() < 16 || &b[12..16] != b"PGOM" { + continue; + } + checked += 1; + match parse_wmo(&mut std::io::Cursor::new(b.as_slice())) { + Ok(ParsedWmo::Group(g)) if g.vertex_positions.is_empty() => empty.push(p.clone()), + _ => {} + } + } + assert!(checked > 3000, "only checked {checked} group files"); + assert!( + empty.is_empty(), + "{} of {checked} group files parsed to an empty group: {:?}", + empty.len(), + &empty[..empty.len().min(10)] + ); +} diff --git a/crates/benilla-wmo/src/lib.rs b/crates/benilla-wmo/src/lib.rs index 53217776..2681ac72 100644 --- a/crates/benilla-wmo/src/lib.rs +++ b/crates/benilla-wmo/src/lib.rs @@ -164,17 +164,37 @@ type Result = std::result::Result; pub fn parse_wmo(cursor: &mut Cursor<&[u8]>) -> Result { let b: &[u8] = cursor.get_ref(); // A group file carries a MOGP super-chunk; a root carries MOHD. - for (magic, payload) in chunks(b) { - if &magic == b"PGOM" { - return Ok(ParsedWmo::Group(parse_group(payload)?)); - } - if &magic == b"DHOM" { - return Ok(ParsedWmo::Root(parse_root(b)?)); - } + if let Some(payload) = mogp_payload(b) { + return Ok(ParsedWmo::Group(parse_group(payload)?)); + } + if chunks(b).any(|(magic, _)| &magic == b"DHOM") { + return Ok(ParsedWmo::Root(parse_root(b)?)); } Err(Error::NotWmo) } +/// The MOGP (`PGOM` as stored) payload of a group file: from its data start to **EOF**, whatever +/// size it declares. MOGP is always a group file's last top-level chunk, so EOF is its real end, +/// and a declared size shorter than the payload leaves the geometry sub-chunks as top-level +/// siblings after it — where `chunks` would walk straight past them and [`parse_group`] would +/// answer an empty group, a building that draws nothing. +/// +/// This realm's chain has seven: the six `World\wmo\Outland\Shadowmoon\Slagpit\ +/// Shadowmoon_Slagpit01_00{0..5}.wmo` declare 0x40, short of the 0x44 header, and +/// `HumanHouse\WestfallHouse_MediumStraw_000.wmo` declares 0. +pub fn mogp_payload(b: &[u8]) -> Option<&[u8]> { + let mut pos = 0usize; + while pos.checked_add(8)? <= b.len() { + let size = b.u32_at(pos + 4)? as usize; + let start = pos + 8; + if &b[pos..pos + 4] == b"PGOM" { + return Some(&b[start..]); + } + pos = start.saturating_add(size).min(b.len()); + } + None +} + fn parse_root(b: &[u8]) -> Result { let mut root = WmoRoot { textures: Vec::new(), @@ -485,6 +505,31 @@ mod tests { assert!(g.liquid.is_none()); } + #[test] + fn mogp_declaring_less_than_its_header_still_owns_the_sibling_chunks() { + // Shadowmoon_Slagpit01_00x: MOGP declares 0x40 (< the 0x44 header) and MOVT/MOVI/MOPY + // follow as top-level siblings up to EOF. + let mut hdr = vec![0u8; 68]; + hdr[8..12].copy_from_slice(&0x48u32.to_le_bytes()); // flags + let mut b = chunk(b"REVM", &17u32.to_le_bytes()); + b.extend_from_slice(b"PGOM"); + b.extend_from_slice(&64u32.to_le_bytes()); + b.extend_from_slice(&hdr); + let mut vert = Vec::new(); + for f in [0f32, 0., 0., 1., 0., 0., 0., 1., 0.] { + vert.extend_from_slice(&f.to_le_bytes()); + } + b.extend(chunk(b"TVOM", &vert)); + b.extend(chunk(b"IVOM", &[0u8, 0, 1, 0, 2, 0])); + b.extend(chunk(b"YPOM", &[0u8, 0])); + let Ok(ParsedWmo::Group(g)) = parse(&b) else { + panic!("expected a group"); + }; + assert_eq!(g.flags, 0x48); + assert_eq!(g.vertex_positions.len(), 3); + assert_eq!(g.vertex_indices.len(), 3); + } + #[test] fn truncated_mohd_is_an_error_not_a_panic() { // A 3-byte MOHD payload cannot hold nGroups at 4.