diff --git a/crates/benilla-assets/src/blp.rs b/crates/benilla-assets/src/blp.rs index cddaf251..eebc8c46 100644 --- a/crates/benilla-assets/src/blp.rs +++ b/crates/benilla-assets/src/blp.rs @@ -56,13 +56,18 @@ impl AssetLoader for BlpImageLoader { &self, reader: &mut dyn Reader, settings: &Self::Settings, - _ctx: &mut LoadContext<'_>, + ctx: &mut LoadContext<'_>, ) -> Result { let mut bytes = Vec::new(); reader.read_to_end(&mut bytes).await?; let to_io = |e: anyhow::Error| std::io::Error::other(format!("{e:#}")); Ok(match settings.variant { - BlpVariant::WorldArt => world_art_image(blp_bytes_to_mip_chain(&bytes).map_err(to_io)?), + BlpVariant::WorldArt => world_art_image( + blp_bytes_to_mip_chain(&bytes).map_err(to_io)?, + // The address mode rides the asset path (`crate::texture_url`, decision 0763): the + // sampler is a property of this upload, and one `.blp` legitimately has two. + crate::sampler_mode_of(&ctx.path().to_string()), + ), BlpVariant::Sprite => { let (w, h, rgba) = blp_to_rgba(&bytes).map_err(to_io)?; sprite_image(w, h, rgba) @@ -91,7 +96,7 @@ fn color_format() -> TextureFormat { /// World/model albedo: the BLP's authored mip pyramid laid in verbatim, repeat + trilinear + /// anisotropic. (Port of the old `repeat_texture_authored`.) -fn world_art_image(chain: BlpMipChain) -> Image { +fn world_art_image(chain: BlpMipChain, wrap: (bool, bool)) -> Image { let levels = chain.mips.len() as u32; let mip0 = chain.mips[0].clone(); // satisfies Image::new's length assert; full chain set below let mut data = Vec::with_capacity(chain.mips.iter().map(Vec::len).sum()); @@ -111,9 +116,16 @@ fn world_art_image(chain: BlpMipChain) -> Image { ); image.data = Some(data); image.texture_descriptor.mip_level_count = levels; + let mode = |repeat: bool| { + if repeat { + ImageAddressMode::Repeat + } else { + ImageAddressMode::ClampToEdge + } + }; image.sampler = ImageSampler::Descriptor(ImageSamplerDescriptor { - address_mode_u: ImageAddressMode::Repeat, - address_mode_v: ImageAddressMode::Repeat, + address_mode_u: mode(wrap.0), + address_mode_v: mode(wrap.1), mag_filter: ImageFilterMode::Linear, min_filter: ImageFilterMode::Linear, mipmap_filter: ImageFilterMode::Linear, diff --git a/crates/benilla-assets/src/lib.rs b/crates/benilla-assets/src/lib.rs index 9ae98595..e67fdd23 100644 --- a/crates/benilla-assets/src/lib.rs +++ b/crates/benilla-assets/src/lib.rs @@ -68,9 +68,13 @@ impl MpqAssetReader { impl AssetReader for MpqAssetReader { async fn read<'a>(&'a self, path: &'a Path) -> Result { - let internal = path + let raw = path .to_str() .ok_or_else(|| AssetReaderError::NotFound(path.to_path_buf()))?; + // Strip the sampler-mode marker ([`texture_url`]) before the archive lookup — it is part of + // the ASSET identity, not of the file name. + let stripped = strip_sampler_marker(raw); + let internal = stripped.as_deref().unwrap_or(raw); if !self.chain.contains(internal) { return Err(AssetReaderError::NotFound(path.to_path_buf())); } @@ -99,6 +103,58 @@ impl AssetReader for MpqAssetReader { } } +/// The sampler-mode marker separator in an `mpq://` texture URL (see [`texture_url`]). +const SAMPLER_MARKER: char = '@'; + +/// Build the `mpq://` URL for a model texture at a given sampler address mode. +/// +/// The address mode lives on the GPU sampler, which in Bevy rides the `Image`, which is keyed by +/// asset path — so two modes of one `.blp` need two asset paths. **243 of the corpus's 4677 texture +/// paths are asked for more than one mode** (`benilla-extract texmodescan`), so this is not a +/// hypothetical: without it, whichever model loaded a shared sheet first would decide the mode for +/// every later one. +/// +/// Repeat/repeat — the overwhelming majority — keeps the bare path, so the common case is one upload +/// and every pre-existing URL is unchanged. Any other mode gets a marker **before the extension** +/// (`…\leaves01@cc.blp`), because Bevy selects the loader by extension and a trailing marker would +/// stop `.blp` resolving. [`MpqAssetReader::read`] strips it back off for the archive lookup. +/// Decision 0763. +pub fn texture_url(internal: &str, wrap: (bool, bool)) -> String { + let path = internal.replace('\\', "/").to_ascii_lowercase(); + if wrap == (true, true) { + return format!("mpq://{path}"); + } + let tag = match wrap { + (true, false) => "rc", + (false, true) => "cr", + _ => "cc", + }; + match path.rsplit_once('.') { + Some((stem, ext)) => format!("mpq://{stem}{SAMPLER_MARKER}{tag}.{ext}"), + None => format!("mpq://{path}{SAMPLER_MARKER}{tag}"), + } +} + +/// The address mode encoded in an asset path by [`texture_url`]; repeat/repeat when unmarked. +pub fn sampler_mode_of(path: &str) -> (bool, bool) { + let Some((stem, _)) = path.rsplit_once('.') else { + return (true, true); + }; + match stem.rsplit_once(SAMPLER_MARKER) { + Some((_, "rc")) => (true, false), + Some((_, "cr")) => (false, true), + Some((_, "cc")) => (false, false), + _ => (true, true), + } +} + +/// Remove a [`texture_url`] marker, yielding the real archive path. `None` when there is none. +fn strip_sampler_marker(path: &str) -> Option { + let (stem, ext) = path.rsplit_once('.')?; + let (base, tag) = stem.rsplit_once(SAMPLER_MARKER)?; + matches!(tag, "rc" | "cr" | "cc").then(|| format!("{base}.{ext}")) +} + /// Register the `mpq://` asset source on `app`, backed by the patch chain at `data_dir`. /// /// Must be called **before** Bevy's `AssetPlugin` builds (i.e. before `DefaultPlugins`): asset @@ -137,6 +193,36 @@ mod tests { std::path::PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("../../WoW/Data") } + /// The sampler-mode URL round-trips, and repeat/repeat stays byte-identical to the old bare + /// path — so the common case keeps ONE upload and no pre-existing URL changed (decision 0763). + #[test] + fn sampler_mode_rides_the_asset_path_and_round_trips() { + let tex = "World\\KhazModan\\Ironforge\\PassiveDoodads\\Trees\\IronForgeleaves01.blp"; + // The default is the bare path — unchanged from before this scheme existed. + let repeat = texture_url(tex, (true, true)); + assert_eq!( + repeat, + "mpq://world/khazmodan/ironforge/passivedoodads/trees/ironforgeleaves01.blp" + ); + assert_eq!(sampler_mode_of(&repeat), (true, true)); + // Every other mode marks the STEM, so the `.blp` extension still selects the loader. + for wrap in [(false, false), (true, false), (false, true)] { + let url = texture_url(tex, wrap); + assert!(url.ends_with(".blp"), "extension must survive: {url}"); + assert_ne!(url, repeat, "a marked mode is a distinct asset path"); + assert_eq!(sampler_mode_of(&url), wrap, "round-trip {wrap:?}"); + // ...and the marker comes back off for the archive lookup. + assert_eq!( + strip_sampler_marker(url.strip_prefix("mpq://").unwrap()).as_deref(), + Some("world/khazmodan/ironforge/passivedoodads/trees/ironforgeleaves01.blp"), + ); + } + // A bare path has nothing to strip, and an unrelated `@` is not a marker. + assert_eq!(strip_sampler_marker("world/foo.blp"), None); + assert_eq!(strip_sampler_marker("world/foo@bar.blp"), None); + assert_eq!(sampler_mode_of("world/foo@bar.blp"), (true, true)); + } + #[test] fn mpq_reader_loads_real_client_bytes_through_the_assetreader() { let data = vanilla_data_dir(); diff --git a/crates/benilla-assets/src/m2.rs b/crates/benilla-assets/src/m2.rs index 390e868f..40a936a6 100644 --- a/crates/benilla-assets/src/m2.rs +++ b/crates/benilla-assets/src/m2.rs @@ -277,12 +277,12 @@ impl AssetLoader for M2ModelLoader { // (the `BlpImageLoader` registers `blp`), so an uppercase extension silently falls back to // type-based resolution — which is ambiguous with Bevy's built-in image loader and spams a // "Multiple AssetLoaders found" warning per texture. Lowercasing also dedupes case variants. - let texture = sub.texture.as_deref().map(|t| { - ctx.load::(format!( - "mpq://{}", - t.replace('\\', "/").to_ascii_lowercase() - )) - }); + // The URL carries the batch's authored sampler address mode, because two modes of one + // `.blp` are two different `Image` uploads (decision 0763 — `crate::texture_url`). + let texture = sub + .texture + .as_deref() + .map(|t| ctx.load::(crate::texture_url(t, (sub.wrap_x, sub.wrap_y)))); submeshes.push(ModelSubmesh { mesh, skinned_mesh, diff --git a/crates/benilla-formats/src/bin/benilla-extract/m2dump.rs b/crates/benilla-formats/src/bin/benilla-extract/m2dump.rs index 9ddca211..11e780ef 100644 --- a/crates/benilla-formats/src/bin/benilla-extract/m2dump.rs +++ b/crates/benilla-formats/src/bin/benilla-extract/m2dump.rs @@ -785,6 +785,41 @@ pub fn m2batch(chain: &mut Chain, internal_path: &str) -> Result<()> { flags.join(" "), tex, ); + // The batch's UV extent, and the texel-per-yard density it implies. Two questions this + // answers that nothing else did: does the batch tile (range outside 0..1 — the sampler + // repeats, and a wrap discontinuity across a triangle blows up the derivative and drags + // the sampled mip to the coarsest level), and is the authored density so far above the + // screen's that even mip 0 is a minification? A cutout batch is where either shows up + // first, because a coarser mip does not merely soften it — it dissolves the silhouette + // the alpha key cuts (the Dun Morogh snow-fir report, B52). + if !s.uvs.is_empty() && !s.positions.is_empty() { + let uext = |axis: usize| { + s.uvs.iter().fold((f32::MAX, f32::MIN), |(lo, hi), t| { + (lo.min(t[axis]), hi.max(t[axis])) + }) + }; + let (u, v) = (uext(0), uext(1)); + let (du, dv) = (u.1 - u.0, v.1 - v.0); + // Model-space diagonal of the batch, as the yard scale the UV span stretches over. + let diag = { + let (x, y, z) = (ext(0), ext(1), ext(2)); + ((x.1 - x.0).powi(2) + (y.1 - y.0).powi(2) + (z.1 - z.0).powi(2)).sqrt() + }; + let tiles = if du > 1.001 || dv > 1.001 { + " TILES" + } else { + "" + }; + println!( + " uv u[{:+.3}..{:+.3}] v[{:+.3}..{:+.3}] span {du:.3}x{dv:.3}{tiles} \ + uv-per-yard {:.4}", + u.0, + u.1, + v.0, + v.1, + if diag > 1e-6 { du.max(dv) / diag } else { 0.0 }, + ); + } // Which way the geometry FACES, in model space — the question a single-sided batch that // renders when it shouldn't (or doesn't when it should) turns on. `facet` is the winding // normal of the first triangle (`(p1−p0)×(p2−p0)`, WoW model axes); `vnorm` is the mean diff --git a/crates/benilla-formats/src/bin/benilla-extract/main.rs b/crates/benilla-formats/src/bin/benilla-extract/main.rs index f7c3561a..3726d357 100644 --- a/crates/benilla-formats/src/bin/benilla-extract/main.rs +++ b/crates/benilla-formats/src/bin/benilla-extract/main.rs @@ -228,6 +228,34 @@ enum Command { /// Internal-path prefix filter (e.g. `spells`), case-insensitive; all models if omitted. prefix: Option, }, + /// Sweep every `.m2` (optionally under a path prefix) and list the particle emitters whose + /// **file slot 0 is dead while a later slot is alive** (`peak0 <= 0 < peakN`). The reference + /// samples the PLAYING sequence's rate window every frame, so an emitter whose burst is keyed + /// in a later variation is ordinary content — but a consumer that PINS slot 0 renders it as + /// nothing at all, for ever, on every placement, while the emitter still builds, pools and + /// ticks. The population instrument for that silent class (decision 0760; found on + /// `BlastedLandsLightningbolt01.m2`, the Blasted Lands strike that never fires). + Partslotscan { + /// Internal-path prefix filter (e.g. `world`), case-insensitive; all models if omitted. + prefix: Option, + }, + /// Sweep every `.m2` (optionally under a path prefix) and list batches whose texture is authored + /// **CLAMP** (`M2Texture.flags` bit 0/1 clear) while the batch's UVs run **outside `0..1`** — the + /// population a repeat-sampling renderer draws wrong. That margin is deliberate: clamped it + /// samples the sheet's transparent border and the card fades to nothing; wrapped it folds into + /// the opposite edge and draws as solid geometry with a seam at the crossing (decision 0763, + /// bugs B52/B96 — the Dun Morogh snow-firs and the Plaguelands bush). + Uvwrapscan { + /// Internal-path prefix filter (e.g. `world`), case-insensitive; all models if omitted. + prefix: Option, + }, + /// Sweep every `.m2` and report which sampler ADDRESS MODES the corpus asks of each texture + /// path, and how many paths are asked for more than one — the design check behind decision + /// 0763 (the mode lives on the sampler, which rides the `Image`, which is keyed by path). + Texmodescan { + /// Internal-path prefix filter, case-insensitive; all models if omitted. + prefix: Option, + }, /// Sweep every `.m2` (optionally under a path prefix) and count the two halves of the /// **owner-last draw-order** law per model: the EFFECTS it authors (particle emitters + /// ribbon trails) and the TRANSPARENT-pass batches of its own body those effects must draw @@ -518,6 +546,9 @@ fn main() -> Result<()> { Command::Goanimscan => scan::goanimscan(&mut chain)?, Command::Bonescan { prefix } => scan::bonescan(&mut chain, prefix.as_deref())?, Command::Partcensus { prefix } => scan::partcensus(&mut chain, prefix.as_deref())?, + Command::Partslotscan { prefix } => scan::partslotscan(&mut chain, prefix.as_deref())?, + Command::Uvwrapscan { prefix } => scan::uvwrapscan(&mut chain, prefix.as_deref())?, + Command::Texmodescan { prefix } => scan::texmodescan(&mut chain, prefix.as_deref())?, Command::Fxordercensus { prefix } => scan::fxordercensus(&mut chain, prefix.as_deref())?, Command::Partscan { mask, prefix } => { let mask = parse_u32_maybe_hex(&mask) diff --git a/crates/benilla-formats/src/bin/benilla-extract/scan.rs b/crates/benilla-formats/src/bin/benilla-extract/scan.rs index 19eaf5ad..f27f9c80 100644 --- a/crates/benilla-formats/src/bin/benilla-extract/scan.rs +++ b/crates/benilla-formats/src/bin/benilla-extract/scan.rs @@ -2514,3 +2514,255 @@ pub fn shadeat(chain: &mut Chain, map: &str, x: f32, y: f32) -> Result<()> { } Ok(()) } + +/// Sweep every `.m2` (optionally under a path prefix) and list the emitters whose **file slot 0 is +/// dead while another slot is alive** — the shape that makes a pinned-slot-0 consumer silently +/// render nothing (decision 0760, found on `BlastedLandsLightningbolt01.m2`, B63). +/// +/// The reference samples the **playing** sequence's rate window every frame (wow-re +/// `part-emission-rate-animated.md` §2); a consumer that pins slot 0 instead is only correct while +/// slot 0 carries the emitter's whole story. When an author keys the burst in a *later* variation — +/// a lightning strike that fires on 5 % of arms, an ambient prop with a rare flourish — slot 0 is a +/// flat zero and the pinned consumer emits nothing, for ever, on every placement. That is invisible +/// from the outside: the emitter is built, pooled, and ticking; it just never births a particle. +/// +/// `peak0` is slot 0's own peak rate, `peakN` the best any other slot reaches. A listed emitter has +/// `peak0 <= 0 < peakN`. `slots` counts FILE sequence slots (the axis `EmitTiming` bakes on). +pub fn partslotscan(chain: &mut Chain, prefix: Option<&str>) -> Result<()> { + let pfx = prefix.map(|p| p.to_ascii_lowercase().replace('/', "\\")); + let names: Vec = chain + .list() + .context("listing chain contents")? + .into_iter() + .map(|e| e.name) + .filter(|n| { + let l = n.to_ascii_lowercase(); + l.ends_with(".m2") && pfx.as_deref().is_none_or(|p| l.starts_with(p)) + }) + .collect(); + let (mut scanned, mut with_emitters, mut emitters, mut dead0) = (0u32, 0u32, 0u32, 0u32); + let mut models = 0u32; + for name in names { + let Ok(bytes) = chain.read_file(&name) else { + continue; + }; + scanned += 1; + let Ok(defs) = benilla_formats::parse_m2_particle_emitters(&bytes) else { + continue; + }; + if defs.is_empty() { + continue; + } + with_emitters += 1; + let mut lines = Vec::new(); + for (i, d) in defs.iter().enumerate() { + emitters += 1; + let views = d.timing.slot_views(); + if views.len() < 2 { + continue; // one slot ⇒ nothing for a slot pick to get wrong + } + // Peak of a slot's baked rate keys; an unkeyed slot emits nothing (`None` ⇒ rate 0). + let peak = |s: usize| -> f32 { + views + .get(s) + .and_then(|v| v.1) + .map(|keys| keys.iter().map(|&(_, v)| v).fold(0.0, f32::max)) + .unwrap_or(0.0) + }; + let peak0 = peak(0); + let (mut best, mut best_slot) = (0.0f32, 0usize); + for s in 1..views.len() { + if peak(s) > best { + best = peak(s); + best_slot = s; + } + } + if peak0 <= 0.0 && best > 0.0 { + dead0 += 1; + lines.push(format!( + " emitter {i:>2}: slots {:>2} peak0 {peak0:>7.1} peakN {best:>7.1} \ + @ slot {best_slot} tex {}", + views.len(), + d.texture.as_deref().unwrap_or("NONE"), + )); + } + } + if !lines.is_empty() { + models += 1; + println!("{name}"); + for l in lines { + println!("{l}"); + } + } + } + eprintln!( + "{scanned} models scanned, {with_emitters} with emitters, {emitters} emitter(s): \ + {dead0} DEAD-IN-SLOT-0 across {models} model(s) — these emit nothing at all under a \ + pinned-slot-0 consumer" + ); + Ok(()) +} + +/// Sweep every `.m2` (optionally under a path prefix) and list the batches whose texture is +/// authored **CLAMP** (`M2Texture.flags` bit 0/1 clear) while the batch's own UVs run **outside +/// `0..1`** — the exact population a repeat-sampling renderer draws wrong (decision 0763, B52/B96). +/// +/// The margin outside `0..1` is deliberate authoring: clamped, it samples the texture's transparent +/// border and the card fades out to nothing. Sampled with repeat it wraps into the opposite edge — +/// on a cutout sheet, the opaque middle — so the margin draws as solid geometry with a hard seam +/// where u or v crosses the wrap. That is why a snow-fir grows pale plates with a crease down each +/// bough, and why the artefact never looked like an extra primitive: it is the *same* card, +/// sampling the wrong texels. +/// +/// `over` is how far past the edge the batch reaches, in UV units — the width of the wrongly-drawn +/// margin as a fraction of the sheet. +pub fn uvwrapscan(chain: &mut Chain, prefix: Option<&str>) -> Result<()> { + let pfx = prefix.map(|p| p.to_ascii_lowercase().replace('/', "\\")); + let names: Vec = chain + .list() + .context("listing chain contents")? + .into_iter() + .map(|e| e.name) + .filter(|n| { + let l = n.to_ascii_lowercase(); + l.ends_with(".m2") && pfx.as_deref().is_none_or(|p| l.starts_with(p)) + }) + .collect(); + let (mut scanned, mut batches, mut hits, mut models) = (0u32, 0u32, 0u32, 0u32); + let mut cutout_hits = 0u32; + for name in names { + let Ok(bytes) = chain.read_file(&name) else { + continue; + }; + scanned += 1; + let dir = name.rsplit_once('\\').map(|(d, _)| d).unwrap_or(""); + let Ok(subs) = benilla_formats::parse_m2_render_submeshes(&bytes, dir, &[]) else { + continue; + }; + let mut lines = Vec::new(); + for (i, s) in subs.iter().enumerate() { + if s.uvs.is_empty() { + continue; + } + batches += 1; + let ext = |axis: usize| { + s.uvs.iter().fold((f32::MAX, f32::MIN), |(lo, hi), t| { + (lo.min(t[axis]), hi.max(t[axis])) + }) + }; + let (u, v) = (ext(0), ext(1)); + // Only an axis authored CLAMP can be drawn wrong by repeat; a wrapping axis is meant + // to tile. A hair of float slop past the edge is not a margin — require 1/512 of a + // sheet, well under the thinnest authored border and well over rounding. + const SLOP: f32 = 1.0 / 512.0; + let bad_u = !s.wrap_x && (u.0 < -SLOP || u.1 > 1.0 + SLOP); + let bad_v = !s.wrap_y && (v.0 < -SLOP || v.1 > 1.0 + SLOP); + if !bad_u && !bad_v { + continue; + } + hits += 1; + let cutout = matches!( + s.blend, + benilla_formats::ModelBlend::AlphaTest | benilla_formats::ModelBlend::Blend + ); + if cutout { + cutout_hits += 1; + } + let over = [ + (-u.0).max(0.0), + (u.1 - 1.0).max(0.0), + (-v.0).max(0.0), + (v.1 - 1.0).max(0.0), + ] + .into_iter() + .fold(0.0f32, f32::max); + lines.push(format!( + " batch {i:>3}: {:?} {} verts u[{:+.3}..{:+.3}] v[{:+.3}..{:+.3}] \ + over {over:.3} {}{} tex {}", + s.blend, + s.positions.len(), + u.0, + u.1, + v.0, + v.1, + if bad_u { "U" } else { "-" }, + if bad_v { "V" } else { "-" }, + s.texture.as_deref().unwrap_or("NONE"), + )); + } + if !lines.is_empty() { + models += 1; + println!("{name}"); + for l in lines { + println!("{l}"); + } + } + } + eprintln!( + "{scanned} models scanned, {batches} textured batch(es): {hits} CLAMP-AUTHORED BATCHES \ + SAMPLING OUTSIDE 0..1 across {models} model(s) — {cutout_hits} of them cutout/blend, \ + where wrapping changes the silhouette rather than just the colour" + ); + Ok(()) +} + +/// Sweep every `.m2` and report, per texture path, which sampler ADDRESS MODES the corpus asks of +/// it — and how many paths are asked for **more than one** (decision 0763). +/// +/// The design question behind it: the address mode lives on the GPU sampler, which in our asset +/// layer is a property of the loaded `Image`, which is keyed by path. If a `.blp` is only ever +/// asked for one mode, path-keying stays correct and the mode can simply ride the load. Every path +/// asked for two needs two uploads, or one of its users renders wrong. +pub fn texmodescan(chain: &mut Chain, prefix: Option<&str>) -> Result<()> { + let pfx = prefix.map(|p| p.to_ascii_lowercase().replace('/', "\\")); + let names: Vec = chain + .list() + .context("listing chain contents")? + .into_iter() + .map(|e| e.name) + .filter(|n| { + let l = n.to_ascii_lowercase(); + l.ends_with(".m2") && pfx.as_deref().is_none_or(|p| l.starts_with(p)) + }) + .collect(); + // texture path -> set of (wrap_x, wrap_y) asked for, as a 4-bit mask + let mut modes: std::collections::BTreeMap = std::collections::BTreeMap::new(); + let mut scanned = 0u32; + for name in names { + let Ok(bytes) = chain.read_file(&name) else { + continue; + }; + scanned += 1; + let dir = name.rsplit_once('\\').map(|(d, _)| d).unwrap_or(""); + let Ok(subs) = benilla_formats::parse_m2_render_submeshes(&bytes, dir, &[]) else { + continue; + }; + for s in &subs { + let Some(tex) = s.texture.as_deref() else { + continue; + }; + let bit = 1u8 << ((s.wrap_x as u8) | ((s.wrap_y as u8) << 1)); + *modes.entry(tex.to_ascii_lowercase()).or_default() |= bit; + } + } + let mut conflicted = 0u32; + for (path, mask) in &modes { + if mask.count_ones() > 1 { + conflicted += 1; + let want = |b: u8, s: &'static str| if mask & (1 << b) != 0 { s } else { "" }; + println!( + "CONFLICT {path} asked as: {}{}{}{}", + want(0, "[clamp,clamp] "), + want(1, "[repeat,clamp] "), + want(2, "[clamp,repeat] "), + want(3, "[repeat,repeat] "), + ); + } + } + eprintln!( + "{scanned} models scanned, {} distinct texture path(s): {conflicted} asked for MORE THAN \ + ONE address mode (each needs its own upload, or one of its users renders wrong)", + modes.len() + ); + Ok(()) +} diff --git a/crates/benilla-formats/src/emit_timing.rs b/crates/benilla-formats/src/emit_timing.rs index 41b51571..a8730c67 100644 --- a/crates/benilla-formats/src/emit_timing.rs +++ b/crates/benilla-formats/src/emit_timing.rs @@ -254,4 +254,61 @@ mod tests { assert!(!t.emitting(None, 0.8), "first pass: past it"); assert!(t.emitting(None, 1.1), "second pass: the window re-fires"); } + + /// **The dead-slot-0 shape** — `BlastedLandsLightningbolt01.m2`'s emitter 2, synthesized + /// (decision 0760, bug B63). Two sequences, both anim id 0: a variation chain. Slot 0 keys a + /// single 0 — a flat silence for its whole band — while the strike itself, `0 → 30 → 0`, is + /// keyed only in slot 1, which the arm's frequency-weighted roll reaches ~5 % of the time. + /// + /// A consumer that PINS slot 0 therefore emits **nothing at all, for ever**, on every + /// placement: the emitter builds, pools and ticks, and never births a particle. That is what + /// the placed-doodad lane did until it started passing the slot its own arm actually rolled. + /// `peak_rate()` folding across all slots is why such an emitter survives the spawn cull — it + /// looks alive to the build and is dead to the clock. `partslotscan` counts 947 of these. + #[test] + fn a_burst_keyed_only_in_a_later_variation_is_silent_in_slot_0() { + // Absolute timeline: slot 0's band 0..1333, slot 1's band 1367..2667 (the real model's). + let rate = track( + 1, + &[ + (0, 0.0), + (1633, 0.0), + (1667, 30.0), + (1800, 30.0), + (1833, 0.0), + ], + &[], + ); + let t = EmitTiming::bake( + &rate, + &M2ScalarTrack::default(), + &slots(&[(0, 1333), (1367, 2667)]), + vec![true, true], + &[], + ); + // Slot 0 — what the old pinned consumer sampled. Silent across its whole band. + for s in [0.0, 0.3, 0.6, 0.9, 1.2] { + assert_eq!(t.rate(Some(0), s), 0.0, "slot 0 is a flat zero at {s}s"); + } + assert_eq!( + t.rate(None, 0.5), + 0.0, + "`None` degrades to slot 0 — also silent" + ); + // Slot 1 — what the arm actually rolled. The strike is here, and only here. + assert_eq!(t.rate(Some(1), 0.0), 0.0, "slot 1 opens closed"); + assert_eq!(t.rate(Some(1), 0.316), 30.0, "the burst fires mid-band"); + assert_eq!(t.rate(Some(1), 0.5), 0.0, "and self-closes"); + // The trap that hid it: the build-time cull sees a live emitter either way. + assert_eq!( + t.peak_rate(), + 30.0, + "peak folds ACROSS slots — never culled" + ); + assert_eq!( + t.constant_rate(), + None, + "and it is not a constant-rate emitter" + ); + } } diff --git a/crates/benilla-formats/src/factions.rs b/crates/benilla-formats/src/factions.rs index 99bc1d6f..d89c583e 100644 --- a/crates/benilla-formats/src/factions.rs +++ b/crates/benilla-formats/src/factions.rs @@ -617,6 +617,37 @@ mod tests { assert_eq!(Reaction::from_rank(7), Reaction::Friendly); } + /// The GameObject faction term (decision 0764), pinned on the real `FactionTemplate.dbc`: the + /// factions that shipped GameObjects actually carry, resolved **GO → player** (the direction + /// `0x606640` uses). A column slip here would silently blank ~7,563 shipped GO spawns, or fail + /// to blank them — so the two decisive rows are asserted by value. Skips without client data. + #[test] + fn real_gameobject_factions_resolve_toward_both_player_templates() { + let data = vanilla_data_dir(); + if !data.is_dir() { + eprintln!("skipping: vanilla client not present at {}", data.display()); + return; + } + let mut chain = crate::open_chain(&data).expect("open chain"); + let cat = load_faction_catalog(&mut chain).expect("load factions"); + let (alliance, horde) = ( + cat.template(1).expect("Alliance player template"), + cat.template(2).expect("Horde player template"), + ); + + // 114 "monster" — what Deadmines' Factory Door and Stratholme's scenery portcullises carry. + // Hostile to BOTH sides, so `> 1` fails and the object is not highlightable for anyone. + let monster = cat.template(114).expect("FactionTemplate 114"); + assert_eq!(monster.reaction_toward(alliance), Reaction::Hostile); + assert_eq!(monster.reaction_toward(horde), Reaction::Hostile); + + // 35 — the levers/torches, and (the reason this matters) Arathi Basin's capture banners. + // FRIENDLY to both, so the faction gate must never touch them. + let usable = cat.template(35).expect("FactionTemplate 35"); + assert_eq!(usable.reaction_toward(alliance), Reaction::Friendly); + assert_eq!(usable.reaction_toward(horde), Reaction::Friendly); + } + /// The real 5875 FactionGroup table: the territory-line names resolve by mask bit — Alliance /// mask 2, Horde mask 4, nothing for an unowned 0 (the `GetZonePVPInfo` lookup, wow-re ui /// `zonetext-pvpinfo.md`). Skips without client data. diff --git a/crates/benilla-formats/src/models/m2_batches.rs b/crates/benilla-formats/src/models/m2_batches.rs index c6f2cc8e..4e537bc2 100644 --- a/crates/benilla-formats/src/models/m2_batches.rs +++ b/crates/benilla-formats/src/models/m2_batches.rs @@ -306,11 +306,14 @@ pub fn parse_m2_render_submeshes( if global_indices.is_empty() { continue; } - let (texture, skin_slot, char_slot) = model + let tex_record = model .raw_data .texture_lookup_table .get(batch.texture_combo_index as usize) - .and_then(|&ti| model.textures.get(ti as usize)) + .and_then(|&ti| model.textures.get(ti as usize)); + // The record's address mode (`flags & 0x1/0x2`). Absent record ⇒ repeat, the old default. + let (wrap_x, wrap_y) = tex_record.map_or((true, true), |t| (t.wrap_x, t.wrap_y)); + let (texture, skin_slot, char_slot) = tex_record .map(|t| resolve_texture(t, dir, skins)) .unwrap_or((None, None, None)); let material = model.materials.get(batch.material_index as usize); @@ -516,6 +519,8 @@ pub fn parse_m2_render_submeshes( sub.fog_policy = fog_policy; // render flag 0x02 / the per-blend fog table sub.skin_slot = skin_slot; // so a skin-less load can fill this batch at spawn sub.geoset_id = section.id; // the skinSectionId — character geoset selection keys on it + sub.wrap_x = wrap_x; // texture record flags 0x1/0x2 — clamp is a silhouette decision + sub.wrap_y = wrap_y; sub.char_slot = char_slot; // character runtime slot (body/hair) — filled per-player at spawn // Skeletal skin binding (decision 0019), per local vertex in `globals` order: the M2 // vertex's 4 bone indices (global bone-array indices → joint indices directly) + their diff --git a/crates/benilla-formats/src/models/mod.rs b/crates/benilla-formats/src/models/mod.rs index 4994ba9d..d6871568 100644 --- a/crates/benilla-formats/src/models/mod.rs +++ b/crates/benilla-formats/src/models/mod.rs @@ -82,6 +82,10 @@ fn remap_submesh( geoset_id: 0, // set by the caller per batch (the M2 path; WMO leaves it 0) char_slot: None, // set by the caller per batch (M2 only) blend, + // Repeat is the pre-0763 behaviour and stays WMO's; the M2 batch loop overrides both + // from the texture record's own flags. + wrap_x: true, + wrap_y: true, two_sided, vertex_colors, joints: Vec::new(), // M2 path fills these from `globals`; WMO leaves them empty diff --git a/crates/benilla-formats/src/models/types.rs b/crates/benilla-formats/src/models/types.rs index 609fa544..3efaa87a 100644 --- a/crates/benilla-formats/src/models/types.rs +++ b/crates/benilla-formats/src/models/types.rs @@ -222,6 +222,15 @@ pub struct RenderSubmesh { /// material onto this batch. `None` for everything that carries its own texture (most M2s, all WMOs). pub char_slot: Option, pub blend: ModelBlend, + /// Sampler address mode from the M2 texture record's `flags` (`0x1` = repeat U, `0x2` = repeat V; + /// clear = **clamp to edge**). Not a tiling detail — a *silhouette* one. Content authors a cutout + /// card's UVs deliberately outside `0..1` so the margin clamps to the texture's transparent border + /// and the card fades to nothing; sampled with repeat instead, that margin wraps into the opaque + /// middle of the sheet and draws as solid geometry with a hard seam at the wrap (decision 0763, + /// bugs B52/B96). `true`/`true` for WMO, which carries its own material flags and keeps today's + /// behaviour. + pub wrap_x: bool, + pub wrap_y: bool, /// Render both faces (no backface culling)? From the M2 material's `0x04` flag (WMO: kept `true` /// for now). When `false` the batch is single-sided like the real client — visible from one side. pub two_sided: bool, diff --git a/crates/benilla-m2/src/lib.rs b/crates/benilla-m2/src/lib.rs index a86a5ad0..4f5cd57b 100644 --- a/crates/benilla-m2/src/lib.rs +++ b/crates/benilla-m2/src/lib.rs @@ -218,6 +218,14 @@ pub fn parse_m2(cursor: &mut Cursor<&[u8]>) -> Result { for i in 0..textures.0 as usize { let t = get(textures.1 as usize + i * 16, 16)?; let ttype = M2TextureType::from_u32(t.u32_at(0).ok_or(Error::Truncated)?); + // `+0x04` — the ADDRESS MODE, and it decides silhouettes, not just tiling. Bit 0 = repeat U, + // bit 1 = repeat V; clear = clamp to edge. Content authors a cutout card's UVs *outside* + // `0..1` deliberately, so the border clamps to the texture's transparent edge and the card + // fades out to nothing — sampled with repeat instead, those margins wrap around into the + // opaque middle of the sheet and draw as solid geometry, with a hard seam where u crosses + // the wrap. This field was documented in this very comment and read by nobody until B52 + // (the Dun Morogh snow-firs) was traced to it; decision 0763. + let tflags = t.u32_at(4).ok_or(Error::Truncated)?; let (fcount, fofs) = ( t.u32_at(8).ok_or(Error::Truncated)? as usize, t.u32_at(12).ok_or(Error::Truncated)? as usize, @@ -227,6 +235,8 @@ pub fn parse_m2(cursor: &mut Cursor<&[u8]>) -> Result { let string = CString::new(&raw[..end]).unwrap_or_default(); texs.push(M2Texture { texture_type: ttype, + wrap_x: tflags & 0x1 != 0, + wrap_y: tflags & 0x2 != 0, filename: M2ArrayString { string }, }); } diff --git a/crates/benilla-m2/src/model.rs b/crates/benilla-m2/src/model.rs index 0de29fa7..1e11a1f5 100644 --- a/crates/benilla-m2/src/model.rs +++ b/crates/benilla-m2/src/model.rs @@ -52,6 +52,10 @@ pub struct M2ArrayString { /// One M2 texture definition. pub struct M2Texture { pub texture_type: M2TextureType, + /// Address mode on U (`flags & 0x1`): set = the sampler REPEATS, clear = it CLAMPS to edge. + pub wrap_x: bool, + /// Address mode on V (`flags & 0x2`). + pub wrap_y: bool, pub filename: M2ArrayString, } diff --git a/crates/benilla-protocol/src/messages/chat.rs b/crates/benilla-protocol/src/messages/chat.rs index 3204b547..0cc34a2e 100644 --- a/crates/benilla-protocol/src/messages/chat.rs +++ b/crates/benilla-protocol/src/messages/chat.rs @@ -59,13 +59,25 @@ pub const CHAT_MSG_BATTLEGROUND_LEADER: u8 = 0x5D; /// The chat types the real client runs its `$`-macro expander over, and the **only** ones — every /// other type, player chat included, reaches the frame verbatim. /// -/// VERIFIED at the bytes in `WoW.exe` (build 5875), `0x49cf36-0x49cf5d`: a `cmp`-chain on exactly -/// these seven values guards the `call 0x506f70` at `0x49cf70`. Note what the set is and isn't — -/// the four monster shapes plus the three **battleground system** lines (`BG_SYSTEM_NEUTRAL`/ -/// `_ALLIANCE`/`_HORDE`, vmangos `SharedDefines.h:1279-1281`), which is where lines like -/// "$n has taken the flag!" come from. `RAID_BOSS_WHISPER`/`_EMOTE` (0x59/0x5A) are **not** in it, -/// despite reading as boss text. -pub const MACRO_EXPANDED_TYPES: [u8; 7] = [ +/// **There are two gates in the reference, and this is the WIRE one** (decision 0759 corrects 0754, +/// which shipped the other). VERIFIED in `WoW.exe` (5875): +/// +/// - the **wire** path — `0x49d5e4-0x49d606` in the `SMSG_MESSAGECHAT` parser `0x49d560` — routes +/// `0x0B 0x0C 0x0D 0x1A` **and `0x5A`** into the expanding branch at `0x49da3d`, plus +/// `0x52 0x53 0x54` into the one at `0x49d961`. **Eight types.** +/// - the **pending-chat** path — `0x49cf36-0x49cf5d`, guarding `0x49cf70` — is the *re*-expansion +/// after a name query answers, and its chain is seven: it omits `0x5A`. +/// +/// benilla decodes the wire, so the wire set is the one that applies. 0754 cited the pending gate +/// and so dropped `RAID_BOSS_EMOTE` (`0x5A`) — a boss emote's `$n` would have stayed literal. +/// +/// Note what the set is and isn't: the four monster shapes and `RAID_BOSS_EMOTE`, plus the three +/// **battleground system** lines (`BG_SYSTEM_NEUTRAL`/`_ALLIANCE`/`_HORDE`, vmangos +/// `SharedDefines.h:1279-1281`) — which is where "$n has taken the flag!" comes from, and why +/// "the monster/boss family" is the wrong name for it. `RAID_BOSS_WHISPER` (`0x59`) is **not** in +/// either gate: it takes its own branch at `0x49d610`, never expands, and is remapped to a plain +/// whisper (`mov byte [ebp+0xf],0x6` @ `0x49d67e`). +pub const MACRO_EXPANDED_TYPES: [u8; 8] = [ CHAT_MSG_MONSTER_SAY, // 0x0B CHAT_MSG_MONSTER_YELL, // 0x0C CHAT_MSG_MONSTER_EMOTE, // 0x0D @@ -73,6 +85,7 @@ pub const MACRO_EXPANDED_TYPES: [u8; 7] = [ CHAT_MSG_BG_SYSTEM_NEUTRAL, // 0x52 CHAT_MSG_BG_SYSTEM_ALLIANCE, // 0x53 CHAT_MSG_BG_SYSTEM_HORDE, // 0x54 + CHAT_MSG_RAID_BOSS_EMOTE, // 0x5A — wire gate only; absent from the pending-chat gate ]; /// `#if SUPPORTED_CLIENT_BUILD > CLIENT_BUILD_1_6_1` — active for 5875. The battleground system diff --git a/crates/benilla-protocol/src/messages/update_object/fields/mod.rs b/crates/benilla-protocol/src/messages/update_object/fields/mod.rs index 371ed550..113ed4dd 100644 --- a/crates/benilla-protocol/src/messages/update_object/fields/mod.rs +++ b/crates/benilla-protocol/src/messages/update_object/fields/mod.rs @@ -23,6 +23,9 @@ const FIELD_GAMEOBJECT_FACING: u16 = 18; // vmangos' `UpdateFields_1_5_1.h` inserts a stray `GAMEOBJECT_TIMESTAMP` that would shift this to 20, // but that header is not the 5875 layout — STATE@14 / TYPE_ID@21 both work against this same server). const FIELD_GAMEOBJECT_DYN_FLAGS: u16 = 19; +// GAMEOBJECT_FACTION = OBJECT_END(6) + field 14 — the slot between DYN_FLAGS(13) and TYPE_ID(15), +// i.e. the binary's `[go+0x110]+0x38` (wow-re cursor-system §4a's faction term, decision 0764). +const FIELD_GAMEOBJECT_FACTION: u16 = 20; const FIELD_GAMEOBJECT_TYPE_ID: u16 = 21; // GAMEOBJECT_LEVEL = OBJECT_END(6) + 0x10 (vmangos UpdateFields_1_12_1.h:317). const FIELD_GAMEOBJECT_LEVEL: u16 = 22; diff --git a/crates/benilla-protocol/src/messages/update_object/fields/player.rs b/crates/benilla-protocol/src/messages/update_object/fields/player.rs index 2fa9af17..1a010862 100644 --- a/crates/benilla-protocol/src/messages/update_object/fields/player.rs +++ b/crates/benilla-protocol/src/messages/update_object/fields/player.rs @@ -442,6 +442,13 @@ impl ObjectFields { pub fn gameobject_flags(&self) -> u32 { self.get_u32(FIELD_GAMEOBJECT_FLAGS).unwrap_or(0) } + /// `GAMEOBJECT_FACTION` — the object's `FactionTemplate.dbc` id, or `0` for "no faction" + /// (which the reference resolves as NEUTRAL). The highlightable predicate's faction term reads + /// exactly this (`[go+0x110]+0x38`, decision 0764): a GameObject whose template is **hostile to + /// the player** is not highlightable, so it shows no cursor, no tooltip and no hover brighten. + pub fn gameobject_faction(&self) -> u32 { + self.get_u32(FIELD_GAMEOBJECT_FACTION).unwrap_or(0) + } /// `GAMEOBJECT_DYN_FLAGS` — the **per-player** dynamic flags; bit `0x1` = `GO_DYNFLAG_LO_ACTIVATE` /// (the sparkle / "usable for me now"), which the server sets from `GameObject::ActivateToQuest`. /// Consulted only when `GAMEOBJECT_FLAGS & INTERACT_COND` is set (decision 0243). diff --git a/crates/benilla-protocol/tests/chat.rs b/crates/benilla-protocol/tests/chat.rs index 484b0606..728d0149 100644 --- a/crates/benilla-protocol/tests/chat.rs +++ b/crates/benilla-protocol/tests/chat.rs @@ -382,17 +382,23 @@ fn random_roll_broadcast_decodes() { /// The `$`-macro gate is a byte-level fact, not a design choice — pin it. /// -/// VERIFIED in `WoW.exe` (build 5875) at `0x49cf36-0x49cf5d`: the `cmp`-chain guarding the -/// `call 0x506f70` at `0x49cf70` tests exactly `0x0b 0x0c 0x0d 0x1a 0x52 0x53 0x54`. Two things -/// this guards against, both of which read as plausible and are wrong: adding the RAID_BOSS pair -/// (0x59/0x5A) because they look like boss text, and dropping the BG_SYSTEM trio (0x52-0x54) -/// because they look like plain system lines. Player chat is never expanded. +/// **The reference has TWO gates, and we want the WIRE one** (decision 0759 corrects 0754). The +/// `SMSG_MESSAGECHAT` parser `0x49d560` routes `0x0B 0x0C 0x0D 0x1A 0x5A` into the expanding branch +/// at `0x49da3d` (`0x49d5e4-0x49d606`) and `0x52 0x53 0x54` into the one at `0x49d961` — **eight**. +/// The seven-value chain at `0x49cf36-0x49cf5d` is the *pending-chat re-expansion* gate and omits +/// `0x5A`; 0754 cited that one by mistake, leaving boss emotes unexpanded. +/// +/// Three plausible-and-wrong edits this guards against: dropping `RAID_BOSS_EMOTE` (0x5A) because +/// the other gate omits it, adding `RAID_BOSS_WHISPER` (0x59) because it looks like its sibling, and +/// dropping the BG_SYSTEM trio (0x52-0x54) because they look like plain system lines. Player chat is +/// never expanded. #[test] -fn the_macro_expanded_chat_types_are_the_reference_seven() { - assert_eq!( - messages::MACRO_EXPANDED_TYPES, - [0x0B, 0x0C, 0x0D, 0x1A, 0x52, 0x53, 0x54] - ); +fn the_macro_expanded_chat_types_are_the_reference_wire_gate() { + let mut got = messages::MACRO_EXPANDED_TYPES; + got.sort_unstable(); + assert_eq!(got, [0x0B, 0x0C, 0x0D, 0x1A, 0x52, 0x53, 0x54, 0x5A]); + // 0x59 RAID_BOSS_WHISPER takes its own branch (`0x49d610`), never expands, and is remapped to a + // plain whisper — it must never creep in alongside 0x5A. for t in [ messages::CHAT_MSG_SAY, messages::CHAT_MSG_YELL, @@ -401,7 +407,6 @@ fn the_macro_expanded_chat_types_are_the_reference_seven() { messages::CHAT_MSG_CHANNEL, messages::CHAT_MSG_SYSTEM, messages::CHAT_MSG_RAID_BOSS_WHISPER, - messages::CHAT_MSG_RAID_BOSS_EMOTE, ] { assert!( !messages::MACRO_EXPANDED_TYPES.contains(&t), diff --git a/crates/benilla-srp/src/lib.rs b/crates/benilla-srp/src/lib.rs index b36c003c..52c6b5db 100644 --- a/crates/benilla-srp/src/lib.rs +++ b/crates/benilla-srp/src/lib.rs @@ -206,32 +206,26 @@ fn calculate_u(client_public_key: &PublicKey, server_public_key: &PublicKey) -> ]) } -/// Fold the shared secret `S` (32 LE bytes) into the 40-byte session key: split the even / odd bytes -/// (after trimming an even count of low-order zero bytes), SHA-1 each half, then interleave the two -/// digests. This is WoW's specific `SHA1_Interleave`. +/// Fold the shared secret `S` (32 LE bytes) into the 40-byte session key: split the even / odd bytes, +/// SHA-1 each half, then interleave the two digests. This is WoW's specific `SHA1_Interleave`. +/// +/// The SRP-6 RFC strips low-order zero bytes of `S` before the split; WoW's does **not** — it hashes +/// all 32 bytes unconditionally (`SRP6::HashSessionKey`, vmangos `src/shared/Crypto/Authentication/ +/// SRP6.cpp:127`). Trimming derives a different `K` — and so a different `M1` — for the ~1-in-256 +/// handshake whose `S` happens to end in a zero low byte, which the server answers with +/// `WOW_FAIL_UNKNOWN_ACCOUNT` (0x04): an intermittent "wrong password" on a correct one. fn calculate_interleaved(s: &[u8; 32]) -> [u8; 40] { - // Trim leading (low-order) zero bytes, keeping the offset even so the even/odd split is stable. - let mut lead = 0; - while s[lead] == 0 { - lead += 1; - } - if lead % 2 != 0 { - lead += 1; - } - let s = &s[lead..]; - let half = s.len() / 2; - let mut e = [0u8; 16]; for (i, b) in s.iter().step_by(2).enumerate() { e[i] = *b; } - let g = sha1(&[&e[..half]]); + let g = sha1(&[&e]); let mut f = [0u8; 16]; for (i, b) in s.iter().skip(1).step_by(2).enumerate() { f[i] = *b; } - let h = sha1(&[&f[..half]]); + let h = sha1(&[&f]); let mut out = [0u8; 40]; for (i, (gi, hi)) in g.iter().zip(h.iter()).enumerate() { @@ -444,6 +438,39 @@ mod tests { } } + /// A low-order zero byte in `S` must *not* shorten the split: WoW hashes all 32 bytes, so `S` + /// and `S` with its low byte zeroed differ only in that byte, never in length. (Trimming — the + /// SRP-6 RFC behaviour — is what made ~1 login in 256 come back as "wrong password".) + #[test] + fn interleave_does_not_trim_low_order_zero_bytes() { + let mut s = [0u8; 32]; + s.copy_from_slice(&hx( + "8F4CEBD60DFC34E5C007E51BD4F3A4FF2BC1D930E2D3EA770D8D3EEDFF2DCCFC", + )); + let mut zeroed = s; + zeroed[0] = 0; + zeroed[1] = 0; + + // Even halves: byte 0 is the only difference; odd halves: byte 1. Both must change, and the + // trimming implementation would instead have re-split the remaining 30 bytes wholesale. + let mut expect_even = [0u8; 16]; + let mut expect_odd = [0u8; 16]; + for i in 0..16 { + expect_even[i] = zeroed[i * 2]; + expect_odd[i] = zeroed[i * 2 + 1]; + } + let g = sha1(&[&expect_even]); + let h = sha1(&[&expect_odd]); + let mut expected = [0u8; 40]; + for (i, (gi, hi)) in g.iter().zip(h.iter()).enumerate() { + expected[i * 2] = *gi; + expected[i * 2 + 1] = *hi; + } + + assert_eq!(calculate_interleaved(&zeroed), expected); + assert_ne!(calculate_interleaved(&zeroed), calculate_interleaved(&s)); + } + #[test] fn calculate_x_known_answer() { // Fixed salt (big-endian in the corpus); user/pass → x (big-endian in the corpus). We store diff --git a/crates/benilla/src/assets/images.rs b/crates/benilla/src/assets/images.rs index a9fd9959..50a7bfad 100644 --- a/crates/benilla/src/assets/images.rs +++ b/crates/benilla/src/assets/images.rs @@ -33,7 +33,7 @@ pub(crate) fn color_texture_format() -> TextureFormat { /// coverage-correct, leaf-coloured downsample, so we lay them in as-is under the alpha test — no /// re-filter, no alpha-to-coverage (1.12 does neither). Mips + anisotropy give the no-shimmer look /// at distance without softening the up-close art. -pub(crate) fn repeat_texture_authored(chain: BlpMipChain) -> Image { +pub(crate) fn repeat_texture_authored(chain: BlpMipChain, wrap: (bool, bool)) -> Image { let levels = chain.mips.len() as u32; let mip0 = chain.mips[0].clone(); // satisfies Image::new's length assert; full chain set below let mut data = Vec::with_capacity(chain.mips.iter().map(Vec::len).sum()); @@ -53,9 +53,23 @@ pub(crate) fn repeat_texture_authored(chain: BlpMipChain) -> Image { ); image.data = Some(data); image.texture_descriptor.mip_level_count = levels; + // The address mode is the M2 texture record's own (`flags & 0x1/0x2`), not a blanket Repeat. + // On a cutout sheet it decides the SILHOUETTE: content authors a card's UVs past `0..1` so the + // margin clamps to the transparent border and the card ends there — repeat instead folds that + // margin into the opposite edge, drawing the sheet's opaque middle as solid geometry with a hard + // seam at the crossing. 1250 batches across 775 models author clamp and sample outside the unit + // square (`benilla-extract uvwrapscan`); 802 are cutout/blend, where it changes shape and not + // merely colour. Decision 0763 (bugs B52/B96 — Dun Morogh's snow-firs, the Plaguelands bush). + let mode = |repeat: bool| { + if repeat { + ImageAddressMode::Repeat + } else { + ImageAddressMode::ClampToEdge + } + }; image.sampler = ImageSampler::Descriptor(ImageSamplerDescriptor { - address_mode_u: ImageAddressMode::Repeat, - address_mode_v: ImageAddressMode::Repeat, + address_mode_u: mode(wrap.0), + address_mode_v: mode(wrap.1), // Trilinear + anisotropic: smooth, sharp-where-it-should-be, no shimmer — the vanilla look. mag_filter: ImageFilterMode::Linear, min_filter: ImageFilterMode::Linear, diff --git a/crates/benilla/src/assets/mod.rs b/crates/benilla/src/assets/mod.rs index 787dd388..d6d0a4e2 100644 --- a/crates/benilla/src/assets/mod.rs +++ b/crates/benilla/src/assets/mod.rs @@ -40,7 +40,12 @@ pub(crate) struct WorldAssets { /// startup the only main-thread reads are the occasional map-change WDL load + lazy clutter M2s. pub(crate) chain: Arc>, /// Decoded GPU textures by normalized path. - pub(crate) textures: HashMap>, + /// Keyed by `(normalized path, wrap_u, wrap_v)`: the SAME BLP is legitimately sampled both + /// ways by different models (a sheet that tiles on a wall and clamps on a cutout card), and the + /// address mode lives on the Bevy `Image`'s sampler — so each mode needs its own upload + /// (decision 0763). Before that key existed, whichever model loaded the texture first decided + /// the mode for every later one. + pub(crate) textures: HashMap<(String, bool, bool), Handle>, /// Decoded UI sprite textures by resolved path (`None` = a miss, cached so a bad path never /// re-walks the chain per frame). Separate from `textures`: sprites are sRGB/clamped, world /// art is Unorm/repeat — the same BLP can legitimately live in both. @@ -72,8 +77,10 @@ pub(crate) struct WorldAssets { /// alpha ref + a distance fade) doesn't collide with a tree sharing the same texture. #[derive(Clone, PartialEq, Eq, Hash)] pub(crate) enum MaterialKey { - /// `(texture, blend, two_sided, alpha_key_u8, fade_far_yd_u16, is_wmo, is_fade_variant)`. - Textured(String, ModelBlend, bool, u8, u16, bool, bool), + /// `(texture, blend, two_sided, alpha_key_u8, fade_far_yd_u16, is_wmo, is_fade_variant, + /// wrap_u, wrap_v)` — the address mode is part of the identity because it selects a different + /// `Image` upload (decision 0763). + Textured(String, ModelBlend, bool, u8, u16, bool, bool, bool, bool), Fallback(bool), } @@ -146,14 +153,15 @@ impl WorldAssets { pub(crate) fn texture( &mut self, path: &str, + wrap: (bool, bool), images: &mut Assets, ) -> Option> { - let key = normalize_path(path); + let key = (normalize_path(path), wrap.0, wrap.1); if let Some(handle) = self.textures.get(&key) { return Some(handle.clone()); } - let chain = read_texture_mip_chain(&mut self.chain.lock_recover(), &key).ok()?; - let handle = images.add(repeat_texture_authored(chain)); + let chain = read_texture_mip_chain(&mut self.chain.lock_recover(), &key.0).ok()?; + let handle = images.add(repeat_texture_authored(chain, wrap)); self.textures.insert(key, handle.clone()); Some(handle) } @@ -299,6 +307,8 @@ impl WorldAssets { fade_far: Option, is_wmo: bool, is_fade_variant: bool, + // The batch's authored sampler address mode (`RenderSubmesh::wrap_x/wrap_y`). + wrap: (bool, bool), images: &mut Assets, materials: &mut Assets, ) -> Handle { @@ -310,8 +320,10 @@ impl WorldAssets { Some(far) => Vec4::new(far * 0.75, far, 0.0, 1.0), None => Vec4::ZERO, }; - let resolved = - texture.and_then(|t| self.texture(t, images).map(|h| (normalize_path(t), h))); + let resolved = texture.and_then(|t| { + self.texture(t, wrap, images) + .map(|h| (normalize_path(t), h)) + }); // Only AlphaTest consumes the cutoff, so don't fragment opaque/blend materials by it. let key_alpha = if blend == ModelBlend::AlphaTest { (cutoff * 255.0).round() as u8 @@ -328,6 +340,8 @@ impl WorldAssets { key_fade, is_wmo, is_fade_variant, + wrap.0, + wrap.1, ), None => MaterialKey::Fallback(is_wmo), }; diff --git a/crates/benilla/src/clutter.rs b/crates/benilla/src/clutter.rs index a1a026d0..d3014b5e 100644 --- a/crates/benilla/src/clutter.rs +++ b/crates/benilla/src/clutter.rs @@ -359,6 +359,7 @@ fn build_chunk_clutter( Some(fade_far), // detail-doodad distance fade (≈70 yd horizon — opacity, dc30e1f) false, // clutter is not WMO (keeps its ground-normal N·L path) false, // not a doodad fade twin (clutter's own clutter_fade path handles it) + (sub.wrap_x, sub.wrap_y), // the M2 texture record's own address mode (0763) images, materials, ); diff --git a/crates/benilla/src/doodad_anim.rs b/crates/benilla/src/doodad_anim.rs index 24a6b15f..2e10d2d7 100644 --- a/crates/benilla/src/doodad_anim.rs +++ b/crates/benilla/src/doodad_anim.rs @@ -77,6 +77,16 @@ pub(crate) struct AnimHostSpawn { pub slot: u16, /// The looping first-sequence node + duration, `None` on the gseq-only tier. pub clip: Option<(AnimationNodeIndex, f32)>, + /// The **file sequence slot** this placement's arm actually rolled ([`AnimClip::seq_index`]) — + /// the axis every per-sequence bake is keyed on, and what the placement's particle emitters + /// must sample their rate/gate tracks against. `None` on the gseq-only tier (no arm). + /// + /// This exists because the variation roll below is a *per-placement* decision that more than + /// one consumer depends on. The emitters used to pin slot 0 regardless, which silently killed + /// every emitter whose rate is keyed only in a later variation — 947 of them across 184 models + /// (`benilla-extract partslotscan`), including the Blasted Lands lightning strike that keys + /// its whole burst in slot 1 and therefore never fired once (decision 0760, bug B63). + pub seq: Option, } /// Spawn the animation host for one placed M2, if [`classify`] says it animates: an anim-root entity @@ -128,6 +138,7 @@ pub(crate) fn spawn_anim_host( commands.entity(root).insert(bb); } let mut clip_info = None; + let mut armed_seq = None; if let DoodadAnimTier::FirstSeq(head) = tier { // The effective load arm rolls `variationIdx = −1` — a frequency-weighted pick over the // first sequence's variation chain (wow-re `doodad-anim-host.md` §4a: the holder setup @@ -154,6 +165,9 @@ pub(crate) fn spawn_anim_host( .insert((bone_target_id(i as u16), AnimatedBy(root))); } clip_info = Some((clip.node, clip.duration)); + // The slot the roll landed on — not necessarily 0, and every per-sequence bake the + // placement's other consumers read is keyed on it (see [`AnimHostSpawn::seq`]). + armed_seq = Some(clip.seq_index); } if let Some(drive) = GlobalSeqDrive::new(&anims.global_bones, &joints) { commands.entity(root).insert(drive); @@ -164,6 +178,7 @@ pub(crate) fn spawn_anim_host( inverse_bindposes: m.inverse_bindposes.clone(), slot, clip: clip_info, + seq: armed_seq, }) } diff --git a/crates/benilla/src/entities/attach/char_skin.rs b/crates/benilla/src/entities/attach/char_skin.rs index 9ea0e001..6603566b 100644 --- a/crates/benilla/src/entities/attach/char_skin.rs +++ b/crates/benilla/src/entities/attach/char_skin.rs @@ -360,7 +360,7 @@ pub(super) fn build_char_skin_materials( worn, ) .ok()??; - let handle = images.add(repeat_texture_authored(composed)); + let handle = images.add(repeat_texture_authored(composed, (true, true))); skin_cache.insert(key, handle.clone()); Some(handle) } diff --git a/crates/benilla/src/entities/wmo_props.rs b/crates/benilla/src/entities/wmo_props.rs index 90d0a5ef..53f24e33 100644 --- a/crates/benilla/src/entities/wmo_props.rs +++ b/crates/benilla/src/entities/wmo_props.rs @@ -250,7 +250,7 @@ pub(super) fn spawn_wmo_gameobject_props( } slot }); - let (ents, joints) = spawn_model_entities( + let (ents, host) = spawn_model_entities( &mut commands, &mut entity_mats.0, &mut materials, @@ -304,8 +304,9 @@ pub(super) fn spawn_wmo_gameobject_props( // ride as a terrain doodad — an unanimated chain reproduces the static path // exactly); a boneless prop follows its root. Either owner's propagated // global composes the moving hull. - let owner = joints - .as_deref() + let owner = host + .as_ref() + .map(|h| h.joints.as_slice()) .and_then(|js| js.get(em.def.bone as usize)) .map_or((root, [0.0; 3]), |&j| (j, em.bone_pivot)); if let Some(e) = particles::spawn_emitter( @@ -318,7 +319,13 @@ pub(super) fn spawn_wmo_gameobject_props( // reference re-anchors every cloud to the emitter's live position), while // an animated bone still never drags it. Some(root), - particles::EmitClock::Pinned, // a placed prop: the doodad law + // A placed prop: the doodad law — one arm, spawn clock, but at the slot + // that arm's variation roll landed on (decision 0760; pinning slot 0 here + // killed every emitter keyed only in a later variation, silently). + match host.as_ref().and_then(|h| h.seq) { + Some(s) => particles::EmitClock::PinnedSeq(s), + None => particles::EmitClock::Pinned, + }, ) { // Parented: the off-map hide reaches the flame, and despawn cascades // (the sim's direct GlobalTransform write runs post-propagation, so the @@ -329,8 +336,9 @@ pub(super) fn spawn_wmo_gameobject_props( } // Ribbon trails — the same host-bone ride; a trail self-despawns with its owner. for rb in &m.ribbons { - let (owner, use_pivot) = joints - .as_deref() + let (owner, use_pivot) = host + .as_ref() + .map(|h| h.joints.as_slice()) .and_then(|js| js.get(rb.def.bone as usize)) .map_or((root, false), |&j| (j, true)); if crate::ribbons::spawn_ribbon( diff --git a/crates/benilla/src/go_anim.rs b/crates/benilla/src/go_anim.rs index c7aea95b..019918f6 100644 --- a/crates/benilla/src/go_anim.rs +++ b/crates/benilla/src/go_anim.rs @@ -425,14 +425,28 @@ fn drive_go_anim( /// the client's only option, and the faithful one. Keyed off the **wire** state (a door's real /// passability is the server's, and it holds for an animation-less door too — no [`GoAnim`] required), and /// scoped to the door/button types ([`collision_follows_state`]); a chest keeps its collider whatever its -/// state. Reconciles on any `ObjectStore` change (idempotent), so a door that streams in open starts -/// passable and one that streams in closed starts solid. +/// state. +/// +/// **Reconciles on an `ObjectStore` change OR on the collider's arrival** (idempotent either way). +/// The `Added` half is load-bearing, not belt-and-braces (decision 0763): a streamed +/// GameObject's descriptor lands with the create block, but its `Collider` is baked from the M2 +/// hull and inserted by `entities::attach` only when the **asset finishes loading**, frames later. +/// Watching `Changed` alone meant the two conditions were never true in the same frame +/// for a freshly streamed object — the descriptor changed while there was no collider, the collider +/// arrived while the descriptor was quiet — so the reconcile never ran at all, and every door kept +/// the enabled collider it was born with. A *closed* door is solid by default and looked correct; +/// an **open** one stayed solid, which is the entire reported symptom. It also explains the +/// reporter's workaround exactly: toggling the object from the GM panel changes the descriptor +/// *after* the collider exists, so the reconcile finally fires. +/// +/// Same shape as [`sync_wire_go_state`]'s `Or<(Changed, Added)>` — the +/// seed-plus-delta pair this file already uses for every other state consumer. #[allow(clippy::type_complexity)] fn drive_go_collision( mut commands: Commands, gos: Query< (Entity, &ObjectStore, Has), - (With, Changed), + (With, Or<(Changed, Added)>), >, ) { for (entity, store, disabled) in &gos { diff --git a/crates/benilla/src/go_templates.rs b/crates/benilla/src/go_templates.rs index adaac859..ac6d0841 100644 --- a/crates/benilla/src/go_templates.rs +++ b/crates/benilla/src/go_templates.rs @@ -38,6 +38,11 @@ pub(crate) struct GoTemplate { pub(crate) lock_id: u32, /// The display name — the hover tooltip's gold first line (decision 0276's GO law). pub(crate) name: String, + /// GENERIC (type 5) only: the template's `data[1]`, the vanilla **highlight** column, nonzero + /// on 1387 of the 1870 shipped type-5 templates. It is GENERIC's whole mouseover-eligibility + /// answer (`0x5f4830`, decision 0762) — a road signpost hovers because this is 1, and its + /// neighbours with 0 are scenery the reference never lets you hover at all. + pub(crate) generic_highlight: bool, /// MO_TRANSPORT (type 15) path parameters — `Some` only for boats/zeppelins (decision 0438): /// the template's `data0..2` = (taxiPathId, moveSpeed, accelRate), the inputs the transport /// timetable is built from. @@ -86,6 +91,7 @@ impl GameObjectTemplates { .unwrap_or(0); // MO_TRANSPORT (15): data0..2 = taxiPathId / moveSpeed / accelRate (vmangos // `GameObjectInfo::moTransport`; decision 0438). + let generic_highlight = type_id == 5 && data[1] != 0; let mo_transport = (type_id == 15).then(|| MoTransport { taxi_path_id: data[0].max(0) as u32, move_speed: data[1].max(0) as f32, @@ -96,6 +102,7 @@ impl GameObjectTemplates { GoTemplate { lock_id, name, + generic_highlight, mo_transport, }, ); diff --git a/crates/benilla/src/interact.rs b/crates/benilla/src/interact.rs index 8aa912f1..8b9fd1bf 100644 --- a/crates/benilla/src/interact.rs +++ b/crates/benilla/src/interact.rs @@ -170,6 +170,15 @@ fn kind_color(kind: ModelKind) -> egui::Color32 { } } +/// The inspector's GameObject collision readout (decision 0763): does a hull exist, is it disabled +/// right now, and what stored state does the passability gate see. Named so the bundled `stores` +/// param stays readable. +type GoCollisionReadout = ( + Has, + Has, + Option<&'static crate::go_anim::GoAnim>, +); + /// The inspector overlay, drawn only while armed: a weak top-centre "armed" pill (so it's obvious the /// mode is on and how to leave it) and, whenever the cursor is over an identified object, a compact /// identity card pinned to the cursor. No chrome, no panel — its own lightweight surface. @@ -184,7 +193,13 @@ fn inspect_ui( parents: Query<&ChildOf>, // Bundled into one param (the same arity ceiling as `click_input` below): the descriptor // store + the coarse kind the line gates go by. - stores: (Query<&ObjectStore>, Query<&crate::net::NetEntity>), + stores: ( + Query<&ObjectStore>, + Query<&crate::net::NetEntity>, + // The GameObject collision readout (decision 0763): whether a hull exists, whether it is + // currently disabled, and the client's stored state the gate reads. + Query, + ), guids: Query<&crate::net::Guid>, castings: Query<&crate::creature_anim::Casting>, drivers: Query<&crate::creature_anim::AnimDriver>, @@ -240,7 +255,7 @@ fn inspect_ui( .entity .and_then(|e| parents.get(e).ok()) .map(|c| c.parent()); - let (stores, kinds) = (&stores.0, &stores.1); + let (stores, kinds, collision) = (&stores.0, &stores.1, &stores.2); let store = net_entity.and_then(|p| stores.get(p).ok()); // The unit's server name through the query cache — asks on first hover, fills on a later frame // (the same ask-once path the unit frames use). @@ -257,6 +272,52 @@ fn inspect_ui( Some(benilla_protocol::EntityKind::Unit | benilla_protocol::EntityKind::Player) ); let is_player = kind == Some(benilla_protocol::EntityKind::Player); + // The GameObject collision + state readout (decision 0763) — the line that closes the loop on + // "this door is drawn open but I can't walk through it". It answers, for the object under the + // cursor, the three facts the passability gate turns on: the client's stored `GAMEOBJECT_STATE` + // (`go_anim::go_state`, i.e. what we believe open/closed is), whether a collision hull exists at + // all, and whether it is currently disabled. A door reading `state 0 open · SOLID` is the bug + // live on screen; `state 0 open · passable` says the gate ran and something else is blocking. + let go_line = store + .filter(|_| kind == Some(benilla_protocol::EntityKind::GameObject)) + .map(|s| { + let (has_hull, disabled, anim) = net_entity + .and_then(|p| collision.get(p).ok()) + .unwrap_or((false, false, None)); + let state = crate::go_anim::go_state(anim, s); + let word = match state { + 0 => "open(ACTIVE)", + 1 => "closed(READY)", + 2 => "alt(DESTROYED)", + _ => "?", + }; + let solidity = match (has_hull, disabled) { + (false, _) => "no hull", + (true, true) => "passable", + (true, false) => "SOLID", + }; + let flags = s.0.gameobject_flags(); + let mut named = Vec::new(); + for (bit, name) in [ + (0x1, "IN_USE"), + (0x2, "LOCKED"), + (0x4, "INTERACT_COND"), + (0x10, "NO_INTERACT"), + ] { + if flags & bit != 0 { + named.push(name); + } + } + let flag_text = if named.is_empty() { + String::new() + } else { + format!(" [{}]", named.join("|")) + }; + format!( + "go type {} · state {state} {word} · {solidity} · flags {flags:#x}{flag_text}", + s.0.gameobject_type_id() + ) + }); let vitals_line = store.filter(|_| is_unit).map(|s| { format!( "hp {}/{} · level {}", @@ -323,6 +384,9 @@ fn inspect_ui( if !obj.detail.is_empty() { lines.push(obj.detail.clone()); } + if let Some(line) = &go_line { + lines.push(line.clone()); + } if let Some(line) = &vitals_line { lines.push(line.clone()); } @@ -367,6 +431,9 @@ fn inspect_ui( if !obj.detail.is_empty() { ui.label(egui::RichText::new(&obj.detail).color(OVERLAY_TEXT_DIM)); } + if let Some(line) = &go_line { + ui.label(egui::RichText::new(line).color(OVERLAY_TEXT_DIM)); + } if let Some(line) = &vitals_line { ui.label(egui::RichText::new(line).color(OVERLAY_TEXT_DIM)); } diff --git a/crates/benilla/src/npc_text.rs b/crates/benilla/src/npc_text.rs index 3915beac..92a141cd 100644 --- a/crates/benilla/src/npc_text.rs +++ b/crates/benilla/src/npc_text.rs @@ -56,7 +56,22 @@ pub(crate) struct MacroContext<'a> { /// `$` and leaves the rest of the text literal — so an un-landed player name shows `$N` for the /// moment rather than a hole, and the feeds re-substitute when it arrives. pub(crate) fn substitute(text: &str, ctx: &MacroContext) -> String { + substitute_checked(text, ctx).0 +} + +/// [`substitute`], plus the reference driver's **return flag**: `true` when no token failed. +/// +/// The reference's `0x506f70` returns exactly this — "no unrecognized token was hit" — and its +/// callers branch on it. The panel seams ignore it (they show the `$`-preserving text either way, +/// which is what [`substitute`] hands back); the **chat** seam must not, because the reference's +/// chat path never displays a `$` — it drops or defers the line instead. See +/// `ui_chat::feed`'s use, decision 0759. +/// +/// Note what the flag does NOT mean: it says nothing about truncation, and a `false` can come +/// either from an unresolvable subject or from a token outside the accepted set. +pub(crate) fn substitute_checked(text: &str, ctx: &MacroContext) -> (String, bool) { let subject = ctx.subject; + let mut clean = true; let chars: Vec = text.chars().collect(); let mut out = String::with_capacity(text.len()); let mut i = 0; @@ -158,9 +173,10 @@ pub(crate) fn substitute(text: &str, ctx: &MacroContext) -> String { if fail { out.push('$'); i = j; + clean = false; } } - out + (out, clean) } /// Parse a `$G`/`$T` branch body `first:second;` starting at `start` (already past the marker and @@ -246,7 +262,7 @@ pub(crate) fn subject_for_guid( #[cfg(test)] mod tests { - use super::{substitute, MacroContext, Subject}; + use super::{substitute, substitute_checked, MacroContext, Subject}; use crate::world_state::WorldStates; /// Thrall the night-elf priest — race 4 / class 5, so both table lookups are real rows. @@ -271,6 +287,45 @@ mod tests { ) } + /// The driver's return flag ([`substitute_checked`]) — `true` only when no token failed. The + /// chat seam branches on it (drop / defer / show raw), so a wrong flag silently loses chat lines + /// rather than merely showing a stray `$`. Decision 0759. + #[test] + fn the_return_flag_reports_whether_any_token_failed() { + let s = subject(0); + let states = WorldStates::default(); + let ctx = |subject| MacroContext { + subject, + states: &states, + }; + + // No `$` at all, and a token that resolves: clean. + assert_eq!( + substitute_checked("plain text", &ctx(None)), + ("plain text".to_string(), true) + ); + assert_eq!( + substitute_checked("hi $N", &ctx(Some(&s))), + ("hi Thrall".to_string(), true) + ); + // No subject: the person-token fails, the `$` comes back, flag false. + assert_eq!( + substitute_checked("hi $N", &ctx(None)), + ("hi $N".to_string(), false) + ); + // Outside the accepted set: fails even WITH a subject — the case the chat path drops + // outright rather than deferring, because no name query can fix it. + assert_eq!( + substitute_checked("hi $X", &ctx(Some(&s))), + ("hi $X".to_string(), false) + ); + // One failure anywhere poisons the flag, though the rest still expands. + assert_eq!( + substitute_checked("$N and $X", &ctx(Some(&s))), + ("Thrall and $X".to_string(), false) + ); + } + #[test] fn name_and_newline() { let s = subject(0); diff --git a/crates/benilla/src/sun/setup.rs b/crates/benilla/src/sun/setup.rs index 4b2028c6..cb155750 100644 --- a/crates/benilla/src/sun/setup.rs +++ b/crates/benilla/src/sun/setup.rs @@ -215,10 +215,10 @@ pub(super) fn setup_sun( // across patches with different weights. Base-colour alpha is driven per-frame. let tex_white = world_assets .as_mut() - .and_then(|a| a.texture("Environments\\Stars\\Stars.blp", &mut images)); + .and_then(|a| a.texture("Environments\\Stars\\Stars.blp", (true, true), &mut images)); let tex_blue = world_assets .as_mut() - .and_then(|a| a.texture("Environments\\Stars\\Stars2.blp", &mut images)); + .and_then(|a| a.texture("Environments\\Stars\\Stars2.blp", (true, true), &mut images)); for sub in &subs { let positions: Vec<[f32; 3]> = sub .positions diff --git a/crates/benilla/src/target/cursor_mode.rs b/crates/benilla/src/target/cursor_mode.rs index be113486..c04e6f06 100644 --- a/crates/benilla/src/target/cursor_mode.rs +++ b/crates/benilla/src/target/cursor_mode.rs @@ -218,13 +218,27 @@ const GO_DYNFLAG_ACTIVATE: u32 = 0x1; /// ordinary door (no INTERACT_COND) is always highlightable regardless of its (zero) activate bit. The /// client's faction-reaction>1 term is INFERRED and not the quest gate, so it is not modeled here; /// `usable` (lock / range / player-state → the grayed twin) rides on top and is a later refinement. -fn highlightable_flags(type_id: i32, flags: u32, dyn_flags: u32) -> bool { +fn highlightable_flags(type_id: i32, flags: u32, dyn_flags: u32, reaction: Option) -> bool { if matches!( type_id, GO_TYPE_GENERIC | GO_TYPE_TRANSPORT | GO_TYPE_MAP_OBJECT | GO_TYPE_MO_TRANSPORT ) { return false; } + // The FACTION term (`0x5f2f80` @ `0x5f3026/29`, decision 0764). `reaction` is the GameObject's + // reaction **toward us** ([`go_reaction`]); the ordinary test is `> 1`, i.e. anything but + // hostile. `None` = unresolvable (no faction catalog / our store not streamed) and passes, so a + // data gap never blanks the world. + let ordinary_faction_ok = |r: Option| r.is_none_or(|r| r > 1); + if type_id == GO_TYPE_TRAP { + // TRAP alone INVERTS it (`0x5f2fc6/c9: cmp eax,1; jg` — reject when greater): a trap is + // highlightable only to whoever it is hostile to. Everything below still applies. + if !reaction.is_none_or(|r| r == 1) { + return false; + } + } else if !ordinary_faction_ok(reaction) { + return false; + } if flags & GO_FLAG_IN_USE_OR_NO_INTERACT != 0 { return false; } @@ -234,6 +248,86 @@ fn highlightable_flags(type_id: i32, flags: u32, dyn_flags: u32) -> bool { true } +/// `GAMEOBJECT_TYPE_TRAP` (6) — the one type whose faction term is inverted. +const GO_TYPE_TRAP: i32 = 6; + +/// A GameObject's reaction **toward us** — the reference's `0x5f7fd0` → `0x606530` → `0x606640` +/// chain, whose direction is GO→player (`this` = the GO's own `FactionTemplate` row, the argument = +/// the player; `0x606640` tests `self.enemyGroupMask & other.ourMask`). On the client's 1/3/4 scale. +/// +/// `None` when it cannot be resolved (no catalog, our store not streamed, either template missing) — +/// callers treat that as "no opinion" and pass. A GameObject with **no** faction resolves NEUTRAL(3), +/// which is the binary's own default (`0x5f8025: mov eax,3`). +/// +/// INTERIM, and narrower than the unit path on purpose: the reference's `0x606530` also has a +/// player-controlled/reputation arm, but every GameObject faction that ships (114 monster, 35 +/// friendly-to-players, 14, 1375) resolves through the plain template comparator, so the +/// reputation branch is not modelled here. `GAMEOBJECT_CREATED_BY` (a player-summoned object taking +/// its creator's reaction) is likewise not modelled. +pub(crate) fn go_reaction( + factions: Option<&Factions>, + go_faction: u32, + self_store: Option<&ObjectStore>, +) -> Option { + if go_faction == 0 { + return Some(benilla_formats::Reaction::Neutral as u8); + } + let catalog = factions?.catalog(); + let go_tpl = catalog.template(go_faction)?; + let self_tpl = catalog.template(self_store?.0.unit_faction_template()?)?; + Some(go_tpl.reaction_toward(self_tpl) as u8) +} + +/// The **mouseover-eligibility** virtual `[obj->vtbl+0x54]` — the gate that decides whether a picked +/// object becomes the mouseover **at all** (wow-re `tooltip-content-law.md` §2-GAMEOBJECT, rewritten +/// 2026-07-29; decision 0762). +/// +/// This sits one level above everything the tooltip law used to be reasoned from. The per-frame +/// classifier `CGWorldFrame::UpdateMouseoverCursor 0x4828d0` calls it on the picked object and, when +/// it answers false, publishes the **null** mouseover GUID (`0x482985 test eax,eax; je 0x4829ed` → +/// `0x482090(0,0)` + `ResetCursor 0x523d30`). The publisher `0x492890`, the tooltip builder +/// `0x52aa20` and the +64 brighten `0x4945e0` are then never reached: **no tooltip, no highlight, no +/// cursor — nothing.** That is the "some GameObjects show nothing at all on hover" report. +/// +/// For a GameObject, slot `+0x54` is `0x5f8620`, which tail-calls the **per-GO-TYPE strategy slot +/// `+0xc`** — and that slot is *not* uniform: +/// +/// | `[strat_vtbl+0xc]` | types | +/// |---|---| +/// | `0x5f9db0` = `jmp [+0x14]` = **[`highlightable_flags`] itself** | 0 DOOR · 1 BUTTON · 2 QUESTGIVER · 3 CHEST · 4 BINDER · 6 TRAP · 7 CHAIR · 9 TEXT · 10 GOOBER · 12 AREADAMAGE · 13 CAMERA · 17 FISHINGNODE · 18 RITUAL · 19 MAILBOX · 20 AUCTIONHOUSE · 22 SPELLCASTER · 23 MEETINGSTONE · 24 FLAGSTAND · 26 FLAGDROP · 27 MINI_GAME · 28 LOTTERY_KIOSK · + the default | +/// | `0x5f4830` = **`template.data[1] != 0`** (the vanilla "highlight" column) | 5 GENERIC | +/// | `mov al,1` — always | 8 SPELL_FOCUS · 16 DUEL_ARBITER · 25 FISHINGHOLE | +/// | `xor al,al` — never | 11 TRANSPORT · 14 MAP_OBJECT · 15 MO_TRANSPORT | +/// +/// **This refutes the old "the tooltip is not gated by highlightable" reading** — that note verified +/// the publisher correctly and then generalised **GENERIC's** behaviour to all 31 types. The +/// signpost half survives (GENERIC really does answer from `data[1]`, and 1387 of 1870 shipped +/// type-5 templates carry `data1 = 1`); the chest / IN_USE / INTERACT_COND half is wrong, and this +/// is why a pre-quest Stone of Binding or a Stratholme portcullis shows nothing in the reference. +/// +/// `generic_highlight` is GENERIC's `data[1]`; `None` = its ask-once template hasn't answered, which +/// reads as eligible so a signpost isn't blank for the first frames of its query. +pub(crate) fn mouseover_eligible( + type_id: i32, + flags: u32, + dyn_flags: u32, + generic_highlight: Option, + reaction: Option, +) -> bool { + match type_id { + GO_TYPE_TRANSPORT | GO_TYPE_MAP_OBJECT | GO_TYPE_MO_TRANSPORT => false, + GO_TYPE_SPELL_FOCUS | GO_TYPE_DUEL_ARBITER | GO_TYPE_FISHINGHOLE => true, + GO_TYPE_GENERIC => generic_highlight.unwrap_or(true), + _ => highlightable_flags(type_id, flags, dyn_flags, reaction), + } +} + +/// The three types whose eligibility slot is a constant `mov al,1` — always the mouseover, whatever +/// their flags say. +const GO_TYPE_SPELL_FOCUS: i32 = 8; +const GO_TYPE_DUEL_ARBITER: i32 = 16; +const GO_TYPE_FISHINGHOLE: i32 = 25; + /// [`highlightable_flags`] read off a hovered GameObject's descriptor store. An absent /// `GAMEOBJECT_TYPE_ID` is the wire default `0` = DOOR (vmangos omits the zero field), so a door /// resolves to a highlightable type rather than being wrongly rejected as "unknown". Gates the @@ -241,11 +335,12 @@ fn highlightable_flags(type_id: i32, flags: u32, dyn_flags: u32) -> bool { /// (§5-VERIFIED, wow-re 2026-07-20): the pick registers every GO regardless of highlightable and /// the publisher dispatches the tooltip by kind, so a GENERIC(5) signpost tooltips (0349's /// reference close-up) while showing no cursor. -fn go_highlightable(store: &ObjectStore) -> bool { +fn go_highlightable(store: &ObjectStore, reaction: Option) -> bool { highlightable_flags( store.0.gameobject_type_id(), store.0.gameobject_flags(), store.0.gameobject_dynamic_flags(), + reaction, ) } @@ -397,7 +492,12 @@ pub(super) fn classify_cursor( let (go_tf, store, _, anim) = units.get(hovered_object.target?).ok()?; let store = store?; let (self_tf, self_store) = self_q.single().ok()?; - if !go_highlightable(store) { + let reaction = go_reaction( + factions.as_deref(), + store.0.gameobject_faction(), + Some(self_store), + ); + if !go_highlightable(store, reaction) { return None; } // The type's own cursor wins; a base type reads its lock's data-named cursor (needs the @@ -530,28 +630,89 @@ mod tests { assert_eq!(point.stem(), "Point"); } + /// The mouseover-eligibility table (decision 0762) — the gate that decides whether an object + /// becomes the mouseover at all, so a false here is "no tooltip, no brighten, no cursor". + #[test] + fn mouseover_eligibility_matches_the_per_type_slot_table() { + // The three constant-TRUE slots (`mov al,1`) ignore their flags entirely. + for t in [ + GO_TYPE_SPELL_FOCUS, + GO_TYPE_DUEL_ARBITER, + GO_TYPE_FISHINGHOLE, + ] { + assert!(mouseover_eligible(t, GO_FLAG_INTERACT_COND, 0, None, None)); + assert!(mouseover_eligible(t, 0x10, 0, None, None)); + } + // The three constant-FALSE slots (`xor al,al`) — the transport family is never a mouseover. + for t in [GO_TYPE_TRANSPORT, GO_TYPE_MAP_OBJECT, GO_TYPE_MO_TRANSPORT] { + assert!(!mouseover_eligible(t, 0, GO_DYNFLAG_ACTIVATE, None, None)); + } + // GENERIC answers from its template's `data[1]` alone — the signpost that hovers vs the + // scenery beside it that does not. Not from `highlightable_flags`, which rejects GENERIC. + assert!(mouseover_eligible(GO_TYPE_GENERIC, 0, 0, Some(true), None)); + assert!(!mouseover_eligible( + GO_TYPE_GENERIC, + 0, + 0, + Some(false), + None + )); + assert!( + mouseover_eligible(GO_TYPE_GENERIC, 0, 0, None, None), + "template not answered yet reads eligible, so a signpost isn't blank while it queries" + ); + // Everything else IS highlightable — which is the whole correction. A pre-quest + // INTERACT_COND object (the Stone of Binding, a Stratholme portcullis at flags 0x24) and an + // IN_USE / NO_INTERACT object show NOTHING, where we used to tooltip them. + assert!(!mouseover_eligible(0, GO_FLAG_INTERACT_COND, 0, None, None)); + assert!(mouseover_eligible( + 0, + GO_FLAG_INTERACT_COND, + GO_DYNFLAG_ACTIVATE, + None, + None + )); + assert!(!mouseover_eligible(3, 0x10, 0, None, None)); // NO_INTERACT chest + assert!(!mouseover_eligible(3, 0x1, 0, None, None)); // IN_USE chest + // The FACTION term (decision 0764): a door whose template is hostile to us is not + // eligible — no cursor, no tooltip, no brighten. This is Deadmines' Factory Door + // (faction 114, flags 0x20), which the director could still hover and open. + assert!(!mouseover_eligible(0, 0x20, 0, None, Some(1))); + assert!(mouseover_eligible(0, 0x20, 0, None, Some(3))); + assert!(mouseover_eligible(0, 0x20, 0, None, Some(4))); + // TRAP inverts it: hostile-to-us is exactly the case a trap DOES highlight for. + assert!(mouseover_eligible(GO_TYPE_TRAP, 0, 0, None, Some(1))); + assert!(!mouseover_eligible(GO_TYPE_TRAP, 0, 0, None, Some(3))); + // An unresolvable reaction never blanks the world. + assert!(mouseover_eligible(0, 0x20, 0, None, None)); + // A plain door is still eligible — the everyday case must not regress. + assert!(mouseover_eligible(0, 0, 0, None, None)); + assert!(mouseover_eligible(19, 0, 0, None, None)); // mailbox + } + #[test] fn highlightable_gates_the_quest_object_but_not_the_plain_door() { // An ordinary unlocked door (no INTERACT_COND) is highlightable regardless of its zero activate // bit — plain doors must never gray out. - assert!(highlightable_flags(0, 0, 0)); // DOOR, no flags - // A GENERIC decoration is never highlightable. - assert!(!highlightable_flags(GO_TYPE_GENERIC, 0, 0)); + assert!(highlightable_flags(0, 0, 0, None)); // DOOR, no flags + // A GENERIC decoration is never highlightable. + assert!(!highlightable_flags(GO_TYPE_GENERIC, 0, 0, None)); // Neither is the transport family (the byte-dumped constant-false +0x14 slots): no gear, // no tooltip, no USE on a boat / elevator / map object — flags can't make them so. for t in [GO_TYPE_TRANSPORT, GO_TYPE_MAP_OBJECT, GO_TYPE_MO_TRANSPORT] { - assert!(!highlightable_flags(t, 0, GO_DYNFLAG_ACTIVATE)); + assert!(!highlightable_flags(t, 0, GO_DYNFLAG_ACTIVATE, None)); } // A busy (IN_USE) or NO_INTERACT object is not highlightable. - assert!(!highlightable_flags(3, 0x1, 0)); // CHEST, IN_USE - assert!(!highlightable_flags(3, 0x10, 0)); // CHEST, NO_INTERACT - // The quest gate: an INTERACT_COND object is highlightable only with the activate bit set — the - // exact bug, a quest chest without the quest. - assert!(!highlightable_flags(3, GO_FLAG_INTERACT_COND, 0)); // quest chest, no quest → clear + assert!(!highlightable_flags(3, 0x1, 0, None)); // CHEST, IN_USE + assert!(!highlightable_flags(3, 0x10, 0, None)); // CHEST, NO_INTERACT + // The quest gate: an INTERACT_COND object is highlightable only with the activate bit set — the + // exact bug, a quest chest without the quest. + assert!(!highlightable_flags(3, GO_FLAG_INTERACT_COND, 0, None)); // quest chest, no quest → clear assert!(highlightable_flags( 3, GO_FLAG_INTERACT_COND, - GO_DYNFLAG_ACTIVATE + GO_DYNFLAG_ACTIVATE, + None )); // quest chest, quest held → usable } diff --git a/crates/benilla/src/target/hover.rs b/crates/benilla/src/target/hover.rs index 534dcd6f..d4f3901a 100644 --- a/crates/benilla/src/target/hover.rs +++ b/crates/benilla/src/target/hover.rs @@ -319,6 +319,11 @@ pub(super) fn update_hovered_object( child_of: Query<&ChildOf>, guids: Query<&Guid>, stores: Query<&ObjectStore>, + // GENERIC's eligibility is its template's `data[1]` (decision 0762) — the ask-once cache. + go_templates: Res, + // The faction term of eligibility (decision 0764): the GO's own template reaction toward us. + factions: Option>, + self_q: Query<&ObjectStore, With>, mut ray_cast: MeshRayCast, ) { hovered.target = None; @@ -375,6 +380,34 @@ pub(super) fn update_hovered_object( let Ok(guid) = guids.get(net_entity) else { return; }; + // **The mouseover-eligibility gate** (`[obj->vtbl+0x54]` at `0x482982`, decision 0762): an + // ineligible object publishes the NULL mouseover, so it gets no tooltip, no +64 brighten and no + // cursor — nothing. Applied here, at the one place the GO mouseover is published, so all three + // consumers fall out together exactly as they do in the reference (which reaches `0x492890`, + // `0x52aa20` and `0x4945e0` only past this gate). + // + // KNOWN RESIDUAL: the reference nulls the **whole** mouseover, so a unit standing behind an + // ineligible portcullis is not hoverable through it either. benilla picks unit and GameObject + // separately and arbitrates by distance, so here the unit behind it stays hoverable. Narrower + // than the old behaviour (which tooltipped the portcullis itself) and documented rather than + // silently accepted; closing it wants the single-pick arbitration, which is its own slice. + if let Ok(store) = stores.get(net_entity) { + let generic_highlight = go_templates.get(guid.0).map(|t| t.generic_highlight); + let reaction = crate::target::cursor_mode::go_reaction( + factions.as_deref(), + store.0.gameobject_faction(), + self_q.single().ok(), + ); + if !crate::target::cursor_mode::mouseover_eligible( + store.0.gameobject_type_id(), + store.0.gameobject_flags(), + store.0.gameobject_dynamic_flags(), + generic_highlight, + reaction, + ) { + return; + } + } hovered.target = Some(net_entity); hovered.guid = Some(guid.0); hovered.distance = distance; diff --git a/crates/benilla/src/terrain_stream/spawn/assemble.rs b/crates/benilla/src/terrain_stream/spawn/assemble.rs index 60082038..2d00ff53 100644 --- a/crates/benilla/src/terrain_stream/spawn/assemble.rs +++ b/crates/benilla/src/terrain_stream/spawn/assemble.rs @@ -18,6 +18,17 @@ use crate::model_fade::DoodadFade; use crate::model_render::{model_material, MaterialCache, ShadeSel}; use crate::terrain::WowModelMaterial; +/// What one placement's animation host armed, for the consumers that spawn alongside its submeshes. +/// Both fields are *per placement*, not per model: the joint set is this instance's, and `seq` is +/// the slot this instance's own frequency-weighted variation roll landed on. +pub(crate) struct PlacementHost { + /// Bone-indexed joint entities — an emitter or ribbon rides its host bone's joint off this. + pub joints: Vec, + /// The armed FILE sequence slot ([`crate::doodad_anim::AnimHostSpawn::seq`]); `None` on the + /// gseq-only tier, where nothing was armed and slot 0 is the honest answer. + pub seq: Option, +} + /// Spawn a model's submeshes at `transform` with the full doodad/WMO component set. Returns the spawned /// entities (for refcounted despawn). /// @@ -65,9 +76,11 @@ pub(crate) fn spawn_model_entities( // parents that list, and a card is a world root the billboard pass writes absolutely). // `None` for world-static placements (terrain), whose pivots never move. card_owner: Option, - // Returns the spawned entities + the anim host's joint set (bone-indexed) when the model - // animates — the emitter spawn rides its host bone's joint off it (0130 phase 4). -) -> (Vec, Option>) { + // Returns the spawned entities + what the anim host armed for this placement, when the model + // animates: the joint set (bone-indexed) the emitter spawn rides its host bone off (0130 phase + // 4), and the FILE sequence slot its variation roll landed on, which the emitters' rate/gate + // tracks must be sampled against (decision 0760). +) -> (Vec, Option) { let kind = if is_wmo { ModelKind::Wmo } else { @@ -320,6 +333,7 @@ pub(crate) fn spawn_model_entities( // Arm the draw gate: animation runs iff any of these submeshes is drawn (`doodad_anim`) — // or, for a meshless (particles-only) host, iff the placement's fade sphere is in the draw // set (the same 0171 law its emitters gate on). + let armed_seq = host.as_ref().and_then(|h| h.seq); if let Some(h) = host { commands.entity(h.root).insert(DoodadAnimHost { meshes: skinned_meshes, @@ -329,5 +343,11 @@ pub(crate) fn spawn_model_entities( active: true, }); } - (out, skin.map(|(joints, _)| joints)) + ( + out, + skin.map(|(joints, _)| PlacementHost { + joints, + seq: armed_seq, + }), + ) } diff --git a/crates/benilla/src/terrain_stream/spawn/fx.rs b/crates/benilla/src/terrain_stream/spawn/fx.rs index f5c7001f..c87d566f 100644 --- a/crates/benilla/src/terrain_stream/spawn/fx.rs +++ b/crates/benilla/src/terrain_stream/spawn/fx.rs @@ -111,12 +111,23 @@ pub(super) fn spawn_wmo_lights_for( /// candle flames swing with it. Riding is unconditional given a joint set: for a bone whose chain /// never animates the joint holds its telescoped rest pivot, so `joint · (position − pivot)` is /// byte-for-byte the static `placement · position` — no per-emitter classification needed. +/// +/// `armed_seq` is the FILE sequence slot this placement's own arm rolled, and the emitters sample +/// their rate/gate tracks against **that** slot. This site used to pass `EmitClock::Pinned` — slot 0, +/// unconditionally — on the reasoning that a placed doodad arms once, so slot 0 is all it can ever +/// play. The arm has rolled a frequency-weighted *variation* since 0130, so that stopped being true +/// and nothing noticed: an emitter whose rate is keyed only in a later variation sampled a flat zero +/// for ever and emitted nothing, on every placement, while still building, pooling and ticking. +/// `benilla-extract partslotscan` counts **947** such emitters across **184** models; the one that +/// surfaced it is the Blasted Lands lightning strike, whose whole burst lives in slot 1 +/// (decision 0760, bug B63). #[allow(clippy::too_many_arguments)] // a spawn plumbing fn: every arg is a distinct resource pub(super) fn spawn_emitters_for( commands: &mut Commands, emitters: &[ModelEmitter], transform: Transform, joints: Option<&[Entity]>, + armed_seq: Option, fade: (f32, Vec3), out: &mut Vec, ) { @@ -136,9 +147,14 @@ pub(super) fn spawn_emitters_for( em, transform, owner, - None, // placed doodads/props are never attached models + None, // placed doodads/props are never attached models None, // anchor at the placement: an animated bone never drags the risen cloud - particles::EmitClock::Pinned, // a placed doodad's one-time arm: slot 0, spawn clock + // A placed doodad's one-time arm, on the spawn clock — but at the slot the arm's + // variation roll actually landed on, not slot 0. No host: the arm never changes. + match armed_seq { + Some(s) => particles::EmitClock::PinnedSeq(s), + None => particles::EmitClock::Pinned, + }, ) { commands.entity(e).insert(particles::EmitterFade { radius: fade_radius, diff --git a/crates/benilla/src/terrain_stream/spawn/mod.rs b/crates/benilla/src/terrain_stream/spawn/mod.rs index 4cd44201..31d79e8b 100644 --- a/crates/benilla/src/terrain_stream/spawn/mod.rs +++ b/crates/benilla/src/terrain_stream/spawn/mod.rs @@ -105,7 +105,7 @@ pub(super) fn spawn_loaded_placements( ShadeResolve::Pending => continue, }; let (radius, center) = m2_fade(&m.bounds, p.transform.scale.x); - let (mut ents, joints) = spawn_model_entities( + let (mut ents, host) = spawn_model_entities( &mut commands, mat_cache, materials, @@ -148,7 +148,8 @@ pub(super) fn spawn_loaded_placements( &mut commands, &m.emitters, p.transform, - joints.as_deref(), + host.as_ref().map(|h| h.joints.as_slice()), + host.as_ref().and_then(|h| h.seq), (radius, center), &mut ents, ); @@ -156,7 +157,7 @@ pub(super) fn spawn_loaded_placements( &mut commands, &m.ribbons, p.transform, - joints.as_deref(), + host.as_ref().map(|h| h.joints.as_slice()), ents.first().copied(), ); spawn_lights_for(&mut commands, &m.lights, p.transform, &mut ents); @@ -401,7 +402,7 @@ pub(super) fn spawn_loaded_placements( slot } }; - let (mut ents, joints) = spawn_model_entities( + let (mut ents, host) = spawn_model_entities( &mut commands, mat_cache, materials, @@ -458,7 +459,8 @@ pub(super) fn spawn_loaded_placements( &mut commands, &m.emitters, d.transform, - joints.as_deref(), + host.as_ref().map(|h| h.joints.as_slice()), + host.as_ref().and_then(|h| h.seq), (radius, center), &mut ents, ); @@ -466,7 +468,7 @@ pub(super) fn spawn_loaded_placements( &mut commands, &m.ribbons, d.transform, - joints.as_deref(), + host.as_ref().map(|h| h.joints.as_slice()), ents.first().copied(), ); spawn_lights_for(&mut commands, &m.lights, d.transform, &mut ents); diff --git a/crates/benilla/src/ui_chat/feed.rs b/crates/benilla/src/ui_chat/feed.rs index 2c9779d1..a2dc91e1 100644 --- a/crates/benilla/src/ui_chat/feed.rs +++ b/crates/benilla/src/ui_chat/feed.rs @@ -321,11 +321,22 @@ pub(super) fn feed_chat( None => c, }) .unwrap_or_default(); - // `$`-macro expansion (decision 0754): the reference runs its one server-text - // expander over monster + BG-system chat and nothing else, against the guid the - // line is ADDRESSED to. Every other type reaches the frame verbatim. - let text = if MACRO_EXPANDED_TYPES.contains(&msg.chat_type) { - // The addressee where the shape carries one, else the only guid it has. + // `$`-macro expansion (decision 0754, corrected by 0759): the reference runs its one + // server-text expander over the monster/boss + BG-system types and nothing else, + // against the guid the line is ADDRESSED to. Every other type reaches the frame + // verbatim. + // + // The FAILURE arms are the reference's, and they are not what a panel does: the chat + // path never puts a `$` on screen. VERIFIED `0x49dac2-0x49db1e` (mirrored at + // `0x49d9c9`): expanded → show; failed with a zero subject → **drop the line**; + // failed with the name already known → **drop** (a known name means retrying cannot + // help); failed with the name still unknown → hold the RAW text, let the name query + // run, and re-expand when it answers — only a second failure there shows the raw + // text. Our bounded `tries` retry IS that hold, and exhausting it is that second + // failure. + let expanded = if MACRO_EXPANDED_TYPES.contains(&msg.chat_type) { + // The addressee where the shape carries one, else the only guid it has (the + // `default:`-shaped BG_SYSTEM lines have no target slot at all). let subject_guid = if msg.target_guid != 0 { msg.target_guid } else { @@ -338,16 +349,36 @@ pub(super) fn feed_chat( &mut names, &commands, ); - crate::npc_text::substitute( + let (text, clean) = crate::npc_text::substitute_checked( &msg.text, &crate::npc_text::MacroContext { subject: subject.as_ref(), states: &states, }, - ) + ); + if clean { + Some(text) + } else if subject_guid == 0 || names.peek(subject_guid).is_some() { + debug!( + "chat: dropping unexpandable [{:#04x}] {:?} (subject {subject_guid:#x})", + msg.chat_type, msg.text + ); + continue; // the reference drops it — no line at all + } else if tries < NAME_MAX_TRIES { + // Name query is already in flight (`subject_for_guid` issued it); hold the + // raw line and re-expand when it lands. + still.push(Pending::Wire { + msg, + tries: tries + 1, + }); + continue; + } else { + None // the post-query second failure: show the raw text verbatim + } } else { - msg.text.clone() + None }; + let text = expanded.unwrap_or_else(|| msg.text.clone()); let event = ChatEvent { kind, text: text.clone(), diff --git a/crates/benilla/src/ui_gossip.rs b/crates/benilla/src/ui_gossip.rs index 300a8745..56971183 100644 --- a/crates/benilla/src/ui_gossip.rs +++ b/crates/benilla/src/ui_gossip.rs @@ -20,8 +20,7 @@ use benilla_ui::script::{GossipMenu, GossipOptionView, GossipQuestRow, ScriptVal use crate::names::NameCache; use crate::net::{ClientCommand, Guid, NetCommands, ObjectStore, SelfPlayer}; -use crate::ui_quest::is_active_quest; -use crate::ui_quest_log::QuestLog; +use crate::ui_quest::{row_is_active, row_is_one_click}; use crate::ui_script::UiInput; use crate::ui_session::{close_npc_session_out_of_range, npc_switched, NpcSession}; @@ -151,18 +150,20 @@ fn gossip_icon_type(icon: u8) -> &'static str { } /// Build the Lua-facing snapshot from [`GossipState`] — `None` when no menu is open. -fn snapshot(state: &GossipState, quest_log: &QuestLog) -> Option { +fn snapshot(state: &GossipState) -> Option { state.npc?; Some(GossipMenu { greeting: state.greeting.clone(), // Quest rows riding the gossip packet (decision 0088): split active-vs-available by the same - // real predicate the quest window's greeting panel uses — log membership, not the wire icon. + // predicate the quest window's greeting panel uses — the WIRE ICON. The reference runs the + // identical `{3,4}` test here, just lazily (`0x4e2430`/`0x4e2580`, behind + // `GetGossipAvailableQuests`/`GetGossipActiveQuests`) rather than at parse time. Decision 0758. quests: state .quests .iter() - .map(|(id, _icon, title)| GossipQuestRow { + .map(|(_id, icon, title)| GossipQuestRow { title: title.clone(), - active: is_active_quest(*id, quest_log), + active: row_is_active(*icon), }) .collect(), options: state @@ -187,7 +188,6 @@ fn feed_gossip( self_q: Query<(&ObjectStore, &Guid), With>, mut names: ResMut, commands: Res, - quest_log: Res, states: Res, mut last: Local>, mut last_name: Local>, @@ -196,7 +196,7 @@ fn feed_gossip( let Some(mut script) = script else { return; }; - let mut fresh = snapshot(&state, &quest_log); + let mut fresh = snapshot(&state); // Expand the greeting's chat-text macros ($N/$B/$G/$w) client-side, as the real client does. if let Some(greeting) = fresh.as_mut().and_then(|m| m.greeting.as_mut()) { let player = crate::npc_text::player_identity(&self_q, &mut names, &commands); @@ -253,7 +253,6 @@ fn drain_gossip( script: Option>, mut state: ResMut, commands: Res, - quest_log: Res, ) { let Some(mut script) = script else { return; @@ -284,15 +283,17 @@ fn drain_gossip( .checked_sub(1) .and_then(|i| state.quests.get(i as usize)) { - Some((quest_id, _icon, _title)) => { - let quest = *quest_id; - let active = is_active_quest(quest, &quest_log); - let cmd = if active { + Some((quest_id, icon, _title)) => { + let (quest, icon) = (*quest_id, *icon); + let active = row_is_active(icon); + // Same opcode law as the greeting panel: an active row is always a turn-in; an + // available row turns into one only when its one-click flag is set. + let cmd = if active || row_is_one_click(icon) { ClientCommand::QuestgiverComplete { npc, quest } } else { ClientCommand::QuestgiverQuery { npc, quest } }; - debug!("ui_gossip: quest row {pos} (quest {quest}, active {active})"); + debug!("ui_gossip: quest row {pos} (quest {quest}, icon {icon}, active {active})"); let _ = commands.0.send(cmd); } None => debug!("ui_gossip: SelectGossipQuest({pos}) out of range — ignored"), @@ -322,8 +323,7 @@ mod tests { #[test] fn snapshot_is_none_until_a_menu_opens() { let mut state = GossipState::default(); - let quest_log = QuestLog::default(); - assert!(snapshot(&state, &quest_log).is_none()); + assert!(snapshot(&state).is_none()); state.npc = Some(0x42); state.greeting = Some("Hello".into()); state.options = vec![GossipOption { @@ -332,7 +332,7 @@ mod tests { coded: false, message: "Browse".into(), }]; - let menu = snapshot(&state, &quest_log).expect("open"); + let menu = snapshot(&state).expect("open"); assert_eq!(menu.greeting.as_deref(), Some("Hello")); assert_eq!(menu.options.len(), 1); assert_eq!(menu.options[0].icon_type, "vendor"); diff --git a/crates/benilla/src/ui_quest.rs b/crates/benilla/src/ui_quest.rs index f67de1e9..4dbaac23 100644 --- a/crates/benilla/src/ui_quest.rs +++ b/crates/benilla/src/ui_quest.rs @@ -30,7 +30,6 @@ use crate::names::NameCache; use crate::net::{ClientCommand, Guid, NetCommands, ObjectStore, SelfPlayer}; use crate::ui_action::{ui_error_text, UiError}; use crate::ui_chat::{ChatEvent, ChatEventKind, ChatLog}; -use crate::ui_quest_log::QuestLog; use crate::ui_script::UiInput; use crate::ui_session::{close_npc_session_out_of_range, npc_switched, NpcSession}; @@ -265,14 +264,49 @@ impl NpcSession for QuestGiver { } /// The greeting panel's active-vs-available split — decision 0088's deferred item, now resolved by -/// the quest-log slice ([`crate::ui_quest_log`]): a row is ACTIVE iff its quest id currently occupies -/// a `PLAYER_QUEST_LOG` descriptor slot, read live off [`QuestLog`] — never derived from the wire -/// `QUEST_LIST` icon (the old icon-derived guess misclassified an auto-complete AVAILABLE quest -/// carrying a REWARD_REP icon). One helper so the snapshot and the drain's re-walk can't diverge. -pub(crate) fn is_active_quest(quest_id: u32, quest_log: &QuestLog) -> bool { - quest_log.contains(quest_id) +/// the **wire icon**, which is the reference's own and only predicate. +/// +/// VERIFIED at the bytes (wow-re `system/ui/scratch/questgiver-quest-pool.md`, dispatched for this; +/// the split is `0x5dbbfe-0x5dbc08`): `icon == 3 || icon == 4` → ACTIVE, and **every other `u32`** → +/// AVAILABLE. A flat two-way `cmp`/`je`, no range test and no third arm — and the client never +/// consults its own quest log on this path. The values are vmangos's `__QuestGiverStatus` +/// (`QuestDef.h:118-130`): 3 = `DIALOG_STATUS_INCOMPLETE` (held, unfinished), 4 = +/// `DIALOG_STATUS_REWARD_REP` (hand it in). The gossip packet's quest rows use the identical test, +/// applied lazily (`0x4e2430`/`0x4e2580`), so both seams share this helper. +/// +/// **This reverses an earlier call of ours, and the reversal is the whole point.** benilla used to +/// read the icon, saw an auto-complete quest arrive carrying REWARD_REP while absent from the quest +/// log, read that as a misclassification, and switched to log membership. It was not a +/// misclassification: an auto-complete quest is *never* in the log — that is what auto-complete +/// means — and `Player::PrepareQuestMenu` (vmangos `Objects/Player.cpp:12501`) marks it REWARD_REP +/// deliberately, so the client sends COMPLETE_QUEST and gets the request-items panel. Deriving the +/// pool from the log instead made every such quest permanently un-turn-in-able and rendered its +/// (empty) detail text as a blank window — ledger B95, decision 0758. +pub(crate) fn row_is_active(icon: u32) -> bool { + matches!(icon, 3 | 4) } +/// An AVAILABLE row's **one-click** flag: `icon == 0` makes the *available* select send +/// COMPLETE_QUEST rather than QUERY_QUEST. +/// +/// VERIFIED: `test ebx,ebx; sete cl` at `0x5dbc13` stores it into the pool record's `+0x48`, which +/// `SelectAvailableQuest` (`0x5012a0`) reads at `0x5012db` to pick opcode `0x18a` over `0x186`. (The +/// ACTIVE pool's `+0x48` is written literal `0` and read nowhere in the binary — `SelectActiveQuest` +/// always sends `0x18a`.) +/// +/// wow-re flags exactly one thing here as open: whether a real 1.12 server ever emitted `icon == 0` +/// on this packet at all — vmangos emits only 2..=5, and its `DIALOG_STATUS_NONE` belongs to the +/// separate `SMSG_QUESTGIVER_STATUS` path. The arm is byte-verified and correctly wired; only its +/// live *reachability* is unknown, and the client predicate is total over `u32`, so implementing it +/// as written is faithful either way. +pub(crate) fn row_is_one_click(icon: u32) -> bool { + icon == 0 +} + +/// One greeting pool as the drain re-walks it: `(quest_id, wire icon)` per row. The icon rides along +/// because the AVAILABLE arm needs it a second time, for [`row_is_one_click`]. +type Pool = Vec<(u32, u32)>; + /// Resolve one wire reward/required triple into a Lua-facing [`QuestItemView`]: icon from the wire /// display id (immediate), name + quality from the ask-once item-template cache (`None`/white while /// in flight — the row shows a placeholder and fills in, exactly like a bag slot / vendor row). @@ -317,7 +351,6 @@ fn snapshot( items: &mut Items, icons: Option<&ItemDisplays>, commands: &NetCommands, - quest_log: &QuestLog, macros: &crate::npc_text::MacroContext, ) -> Option { let sub = |t: &str| crate::npc_text::substitute(t, macros); @@ -326,7 +359,7 @@ fn snapshot( let mut active_titles = Vec::new(); let mut available_titles = Vec::new(); for q in &l.quests { - if is_active_quest(q.quest_id, quest_log) { + if row_is_active(q.icon) { active_titles.push(q.title.clone()); } else { available_titles.push(q.title.clone()); @@ -393,7 +426,6 @@ fn feed_quest( icons: Option>, commands: Res, mut names: ResMut, - quest_log: Res, states: Res, self_q: Query<(&ObjectStore, &Guid), With>, mut chat: ResMut, @@ -440,7 +472,6 @@ fn feed_quest( &mut items, icons.as_deref(), &commands, - &quest_log, &crate::npc_text::MacroContext { subject: player.as_ref(), states: &states, @@ -495,7 +526,6 @@ fn drain_quest( script: Option>, mut giver: ResMut, commands: Res, - quest_log: Res, ) { let Some(mut script) = script else { return; @@ -512,24 +542,25 @@ fn drain_quest( let Some(QuestView::Greeting(list)) = giver.view.as_ref() else { continue; }; - // Re-walk the same active/available split the snapshot used (`is_active_quest`, backed by - // the live quest log — not the wire icon), keeping quest ids this time. - let (mut active, mut available): (Vec, Vec) = (Vec::new(), Vec::new()); + // Re-walk the same split the snapshot used (`row_is_active`, off the wire icon), keeping the + // id AND the icon this time — the available arm needs the icon again for its one-click flag. + let (mut active, mut available): (Pool, Pool) = (Vec::new(), Vec::new()); for q in &list.quests { - if is_active_quest(q.quest_id, &quest_log) { - active.push(q.quest_id); + if row_is_active(q.icon) { + active.push((q.quest_id, q.icon)); } else { - available.push(q.quest_id); + available.push((q.quest_id, q.icon)); } } let pool = if sel.active { &active } else { &available }; - let Some(&quest) = sel.index.checked_sub(1).and_then(|i| pool.get(i as usize)) else { + let Some(&(quest, icon)) = sel.index.checked_sub(1).and_then(|i| pool.get(i as usize)) + else { debug!("ui_quest: greeting select {sel:?} out of range — ignored"); continue; }; - // Active quest = a turn-in (COMPLETE_QUEST → progress); available = a new quest to look at - // (QUERY_QUEST → details). - let cmd = if sel.active { + // `SelectActiveQuest` (`0x501320`) sends COMPLETE_QUEST unconditionally. `SelectAvailableQuest` + // (`0x5012a0`) picks by the row's one-click flag: COMPLETE_QUEST when set, else QUERY_QUEST. + let cmd = if sel.active || row_is_one_click(icon) { ClientCommand::QuestgiverComplete { npc, quest } } else { ClientCommand::QuestgiverQuery { npc, quest } @@ -595,16 +626,48 @@ mod tests { } } + /// The greeting split is the reference's flat two-way test on the WIRE ICON — `{3,4}` ACTIVE, + /// everything else AVAILABLE (`0x5dbbfe-0x5dbc08`, decision 0758). Pinned because this reverses + /// an earlier call that read the same icon and concluded the opposite. #[test] - fn greeting_split_by_quest_log() { - // The real semantics (decision 0088's deferred item, resolved): membership in our quest - // log, not the wire icon. A quest in the log is ACTIVE regardless of which dialog-status - // icon the greeting carried for it (the icon-derived guess this replaces misclassified an - // auto-complete AVAILABLE quest carrying a REWARD_REP icon). - let mut quest_log = QuestLog::default(); - quest_log.set_active_quests([100]); - assert!(is_active_quest(100, &quest_log)); - assert!(!is_active_quest(200, &quest_log)); + fn greeting_split_reads_the_wire_icon() { + // 3 = DIALOG_STATUS_INCOMPLETE, 4 = DIALOG_STATUS_REWARD_REP. + assert!(row_is_active(3)); + assert!(row_is_active(4)); + // 5 = DIALOG_STATUS_AVAILABLE (a genuinely new quest), 2 = DIALOG_STATUS_CHAT. + assert!(!row_is_active(5)); + assert!(!row_is_active(2)); + // No range test and no third arm: every other u32 is AVAILABLE, including 0, the ones + // vmangos never emits, and anything a future server invents. + for icon in [0, 1, 6, 7, 100, u32::MAX] { + assert!(!row_is_active(icon), "icon {icon} must fall to AVAILABLE"); + } + } + + /// **The B95 case.** An auto-complete quest (`QuestMethod == 0`) is never in the player's quest + /// log, and vmangos marks its greeting row `DIALOG_STATUS_REWARD_REP` anyway + /// (`Objects/Player.cpp:12501`) precisely so the client treats it as a turn-in. Binning that row + /// off log membership sent `QUERY_QUEST`, which vmangos answers with DETAILS unconditionally — + /// and quest 7793 "A Donation of Silk" has empty `Details`/`Objectives`, so the window came up + /// blank. The row must be ACTIVE on the icon alone. + #[test] + fn an_autocomplete_row_is_active_though_it_is_not_in_the_log() { + const REWARD_REP: u32 = 4; + assert!( + row_is_active(REWARD_REP), + "an auto-complete turn-in must bin ACTIVE without ever entering the quest log" + ); + assert!(!row_is_one_click(REWARD_REP), "it is active, not one-click"); + } + + /// The AVAILABLE pool's one-click flag: `icon == 0` alone (`test ebx,ebx; sete cl` @ `0x5dbc13`), + /// which makes the *available* select send COMPLETE_QUEST instead of QUERY_QUEST. + #[test] + fn the_one_click_flag_is_icon_zero_alone() { + assert!(row_is_one_click(0)); + for icon in [1, 2, 3, 4, 5, u32::MAX] { + assert!(!row_is_one_click(icon)); + } } /// An item-sourced quest window (decision 0664) reports **no** NPC to the session face, so the @@ -658,7 +721,6 @@ mod tests { let mut items = Items::default(); let (tx, _rx) = crossbeam_channel::unbounded(); let commands = NetCommands(tx); - let quest_log = QuestLog::default(); let player = crate::npc_text::Subject { name: "Tri".into(), race: 1, @@ -670,7 +732,6 @@ mod tests { &mut items, None, &commands, - &quest_log, &crate::npc_text::MacroContext { subject: Some(&player), states: &crate::world_state::WorldStates::default(), @@ -710,13 +771,11 @@ mod tests { let mut items = Items::default(); let (tx, _rx) = crossbeam_channel::unbounded(); let commands = NetCommands(tx); - let quest_log = QuestLog::default(); let snap = snapshot( &giver, &mut items, None, &commands, - &quest_log, &crate::npc_text::MacroContext { subject: None, states: &crate::world_state::WorldStates::default(), diff --git a/crates/benilla/src/ui_quest_log.rs b/crates/benilla/src/ui_quest_log.rs index 91b6f7e4..857d1f83 100644 --- a/crates/benilla/src/ui_quest_log.rs +++ b/crates/benilla/src/ui_quest_log.rs @@ -25,9 +25,14 @@ //! //! [`drain_quest_log_abandons`] maps the confirmed abandon's 1-based Lua entry index back to its //! descriptor slot (this frame's push order, kept on [`QuestLog`]) and sends -//! `CMSG_QUESTLOG_REMOVE_QUEST`. [`QuestLog::contains`] also answers decision 0088's other deferred -//! item — the questgiver greeting's active/available split, now the log's real membership instead -//! of an icon guess (see [`crate::ui_quest`]). +//! `CMSG_QUESTLOG_REMOVE_QUEST`. +//! +//! This module used to also answer the questgiver greeting's active/available split, via a +//! `contains` membership test over the live descriptor slots. **It doesn't any more, and must not +//! again** — the reference splits that list on the wire icon alone and never consults its quest log +//! there ([`crate::ui_quest::row_is_active`], decision 0758). Membership was wrong for exactly the +//! quests that are never in the log: auto-complete turn-ins. The state that backed it is gone +//! rather than left dead, so the retired law can't be reached for by accident. use std::collections::{HashMap, HashSet}; @@ -66,10 +71,6 @@ struct Row { /// /// - `templates`/`pending` — the `SMSG_QUEST_QUERY_RESPONSE` cache, ask-once by quest id (the exact /// twin of [`Items`]'s item-template cache). -/// - `active_quest_ids` — which quest ids occupy a descriptor slot *this frame*, refreshed every -/// [`feed_quest_log`] call. [`Self::contains`] is decision 0088's deferred greeting-split fix -/// ([`crate::ui_quest`]): a row is ACTIVE iff it's really in our log, not however the wire -/// `QUEST_LIST` icon happened to read. /// - `entry_slots` — this frame's pushed entry order → descriptor slot, so /// [`drain_quest_log_abandons`] can turn a confirmed abandon's 1-based Lua index back into the /// `CMSG_QUESTLOG_REMOVE_QUEST` slot it came from (slots aren't contiguous — an abandoned/turned-in @@ -78,7 +79,6 @@ struct Row { pub(crate) struct QuestLog { templates: HashMap, pending: HashSet, - active_quest_ids: HashSet, entry_slots: Vec>, /// Collapsed section headers, keyed by header TITLE (two zones sharing a name share a header /// row, so the fold state naturally shares too). Owned here — the engine only reports the @@ -116,20 +116,6 @@ impl QuestLog { self.templates.insert(template.quest_id, template); } - /// Refresh which quest ids currently occupy a descriptor slot — called once per - /// [`feed_quest_log`] frame; also the hook a unit test seeds directly (no ECS needed) to exercise - /// [`Self::contains`]. - pub(crate) fn set_active_quests(&mut self, ids: impl IntoIterator) { - self.active_quest_ids = ids.into_iter().collect(); - } - - /// Whether `quest_id` occupies one of our quest-log descriptor slots right now — the greeting - /// active/available split's real predicate (decision 0088's deferred item; see - /// [`crate::ui_quest::is_active_quest`]). - pub(crate) fn contains(&self, quest_id: u32) -> bool { - self.active_quest_ids.contains(&quest_id) - } - /// Disconnect: drop everything, including the templates. Unlike item templates (which persist — /// decision: static, stable across sessions), a stale log slice carried across a reconnect would /// pair fresh descriptor slots (possibly renumbered) with old titles; templates are cheap enough @@ -137,7 +123,6 @@ impl QuestLog { pub(crate) fn clear_session(&mut self) { self.templates.clear(); self.pending.clear(); - self.active_quest_ids.clear(); self.entry_slots.clear(); self.collapsed.clear(); self.header_keys.clear(); @@ -437,7 +422,6 @@ fn feed_quest_log( } *prior_quest_ids = Some(current); } - quest_log.set_active_quests(rows.iter().map(|r| r.quest_id)); // ── Section headers (the ref's zone/sort groups) ──────────────────────────────────────────── // Group rows by their template's ZoneOrSort NAME (positive → AreaTable zone, negative → @@ -1092,20 +1076,4 @@ mod tests { // A turn-in/abandon that removes the quest fires nothing either. assert!(quests_with_progressed_objectives(&with_entry, &empty).is_empty()); } - - // ── QuestLog::contains — the greeting split's real predicate ──────────────────────────────────── - - #[test] - fn quest_log_contains_reflects_the_last_set_active_ids() { - let mut log = QuestLog::default(); - assert!(!log.contains(100)); - log.set_active_quests([100, 200]); - assert!(log.contains(100)); - assert!(log.contains(200)); - assert!(!log.contains(300)); - // A later refresh replaces, not accumulates. - log.set_active_quests([300]); - assert!(!log.contains(100)); - assert!(log.contains(300)); - } }