diff --git a/crates/benilla-app/src/capture/mod.rs b/crates/benilla-app/src/capture/mod.rs index 91bc918e..01ff10b0 100644 --- a/crates/benilla-app/src/capture/mod.rs +++ b/crates/benilla-app/src/capture/mod.rs @@ -121,10 +121,10 @@ pub(crate) use probe_rig::ProbeRigPlugin; pub(crate) use probe_taxi::ProbeTaxiPlugin; pub(crate) use probes::{ fx_draw_census_plugin, DressCensusPlugin, EntityCensusPlugin, GroundCensusPlugin, - JitterMeterPlugin, LiveFpsPlugin, NodeProbePlugin, ParticleCensusPlugin, ProbeChatPlugin, - ProbeClock, ProbeDragPlugin, ProbeExitPlugin, ProbeFocusPlugin, ProbeHoverPlugin, - ProbeKeyPlugin, ProbeLuaPlugin, ProbeResizePlugin, RevealAuditPlugin, SchedCensusPlugin, - UnitVisualsPlugin, + JitterMeterPlugin, LiftCensusPlugin, LiveFpsPlugin, NodeProbePlugin, ParticleCensusPlugin, + ProbeChatPlugin, ProbeClock, ProbeDragPlugin, ProbeExitPlugin, ProbeFocusPlugin, + ProbeHoverPlugin, ProbeKeyPlugin, ProbeLuaPlugin, ProbeResizePlugin, RevealAuditPlugin, + SchedCensusPlugin, UnitVisualsPlugin, }; use scenarios::GlueScreen; use scenarios::{Scenario, SubjectKind, UiFixture, GLUE_SCENARIOS, GROUND_EYE, SCENARIOS}; diff --git a/crates/benilla-app/src/capture/probe_crossing.rs b/crates/benilla-app/src/capture/probe_crossing.rs index 1a2bbae6..a1d972e7 100644 --- a/crates/benilla-app/src/capture/probe_crossing.rs +++ b/crates/benilla-app/src/capture/probe_crossing.rs @@ -86,7 +86,7 @@ fn crossing_probe( if !(transport.touches_map(0) && transport.touches_map(1)) { continue; } - let sample = transport.sample_at(anchor); + let sample = transport.sample_at(anchor, map); if sample.map != map || sample.moving { continue; } @@ -144,7 +144,7 @@ fn crossing_probe( } else if let Some((_, transport, anchor)) = transports.iter().find(|(g, ..)| g.0 == boat) { - let sample = transport.sample_at(anchor); + let sample = transport.sample_at(anchor, map); if sample.map == to_map && !sample.moving { info!( "PROBE crossing: SUCCESS — docked on map {to_map} still aboard {boat:#x}" diff --git a/crates/benilla-app/src/capture/probes/lift_census.rs b/crates/benilla-app/src/capture/probes/lift_census.rs new file mode 100644 index 00000000..59fa089b --- /dev/null +++ b/crates/benilla-app/src/capture/probes/lift_census.rs @@ -0,0 +1,230 @@ +//! The **transport census** (`WOW_LIFT_CENSUS=[,]`) — the instrument that turns +//! *"the Thunder Bluff elevators aren't there"* into a line of numbers. +//! +//! An absent lift is a screenshot, and a screenshot cannot separate the four ways a type-11 +//! elevator (or a type-15 boat) can fail to appear — each of which calls for a different fix: +//! +//! - the server never streamed it (**no entity** — nothing on the wire, our count is 0); +//! - it streamed but never **armed** (`state=seed`/`bare`): a transport spawns +//! [`Visibility::Hidden`] and is unhidden by its first ticked pose (decision 0438), so an arm +//! that never completes leaves a car that is present, solid, and permanently invisible — +//! B168's "an invisible wall in its place"; +//! - it armed and ticks, but the model never built (`meshes=0`) — an asset gap; +//! - it armed, ticks and draws, and the car is simply somewhere else in its cycle +//! (`cycle=`/`pos=` say where, against the authored keyframes). +//! +//! ```text +//! LIFT 0xF11000000000504B entry=4170 disp=360 type=11 state=lift period=30033 cycle=17421 +//! moving=0 vis=Inherited inh=1 meshes=4 attached=1 pos=(-1286.24,189.72,130.08) d=9.4 +//! ``` +//! +//! - **`state`** is the arm verdict read off the components themselves: `lift`/`taxi` (armed), +//! `seed` (a type-11 waiting for its keyframe catalog), `bare` (an anchored transport with no +//! drive — the boat arm waiting on a template, or a type-11 whose create carried no entry), +//! `parked` (no anchor: a pathless type-11 the arm deliberately released, or a type the wire +//! never flagged). +//! - **`vis`/`inh`** are the root's own [`Visibility`] and the propagated +//! [`InheritedVisibility`]. `vis=Hidden` on a transport is the finding: either the tick never ran +//! for it, or it ran and judged the car off-map. The headline `hidden=` count is that number, and +//! `map=` beside it is the live [`CurrentMap`] the tick judged against — the pair that named +//! decision 1654 (`hidden=11 … map=1`, every lift on Kalimdor, all of them armed and ticking). +//! - **`meshes`** counts render descendants (the whole subtree, not just direct children — a GO +//! model hangs its submeshes under an anim host). +//! +//! Pair it with the slot-keyed probe identity and a `.go` to the reported spot: +//! +//! ```text +//! WOW_USER=probeN WOW_PASS=pprobeN WOW_CHAR=Probe WOW_NOSOUND=1 \ +//! WOW_PROBE_CHAT=".go xyz -1286.2 189.7 132.0 1" \ +//! WOW_LIFT_CENSUS="30,15" WOW_PROBE_EXIT_AT=75 cargo run -q -p benilla +//! ``` +//! +//! The repeat form is the load-race half: a census at 30 s and again at 45 s says whether a +//! `state=seed` is a transient (the catalog had not opened yet) or the verdict. + +use benilla_assets::coords::bevy_to_wow; +use benilla_protocol::EntityKind; +use bevy::camera::visibility::InheritedVisibility; +use bevy::prelude::*; + +use super::ProbeClock; +use crate::entities::VisualAttached; +use crate::net::{Guid, NetEntity, ObjectStore, SelfPlayer}; +use crate::transport::{ElevatorSeed, Transport, TransportAnchor}; +use benilla_world::world_map::CurrentMap; + +/// How deep the render-descendant walk goes. A GO's submeshes sit at most a couple of levels +/// under the net root (root → anim host → submesh); eight is slack, not a limit anything reaches. +const WALK_DEPTH: u32 = 8; + +pub(crate) struct LiftCensusPlugin; + +impl Plugin for LiftCensusPlugin { + fn build(&self, app: &mut App) { + let raw = std::env::var("WOW_LIFT_CENSUS").unwrap_or_default(); + let mut parts = raw + .split(',') + .map(|s| s.trim().parse::().unwrap_or(0.0)); + let at = parts.next().filter(|v| *v > 0.0).unwrap_or(30.0); + let every = parts.next().unwrap_or(0.0); + app.insert_resource(LiftCensus { next: at, every }) + .add_systems(Update, fire_lift_census); + } +} + +/// [`LiftCensusPlugin`] state: when the next census fires, and how often after that (`0` = once). +#[derive(Resource)] +struct LiftCensus { + next: f32, + every: f32, +} + +/// What the census reads per entity: identity, kind/display, pose, the three transport +/// components (each one a distinct arm stage), the visibility pair, and the visual gate. +type CensusQuery = ( + &'static Guid, + &'static NetEntity, + &'static Transform, + &'static ObjectStore, + Option<&'static Transport>, + Option<&'static TransportAnchor>, + Has, + Option<&'static Visibility>, + Option<&'static InheritedVisibility>, + Option<&'static Children>, + Has, +); + +/// One line per streamed transport GameObject — hidden ones first, then by distance — under a +/// summary line naming the counts that matter. No radius: vmangos sends a map's **whole** transport +/// set at world entry and keeps it resident (`Map::SendInitTransports`, `Map.cpp:1719`; every +/// type-11 joins `m_transports` at `Map::Add`) — decision 0678 measured the same thing from a +/// ship's lanterns at 4853 yd. So the population is a handful and all of it is in reach. +fn fire_lift_census( + mut probe: ResMut, + time: ProbeClock, + current_map: Option>, + body: Query<&Transform, With>, + entities: Query, + children: Query<&Children>, + meshes: Query<(), With>, +) { + let now = time.elapsed_secs(); + if probe.next <= 0.0 || now < probe.next { + return; + } + probe.next = if probe.every > 0.0 { + now + probe.every + } else { + -1.0 + }; + let at = body.single().ok().map(|t| t.translation); + + let mut rows: Vec<(bool, i64, String)> = Vec::new(); + let (mut hidden_n, mut unarmed_n, mut meshless_n) = (0u32, 0u32, 0u32); + for (guid, net, t, store, transport, anchor, seed, vis, inherited, kids, attached) in &entities + { + let go_type = store.0.gameobject_type_id(); + // Every ticking GO type (the reference's own RF-0051 pair) plus anything already wearing + // a transport component — so a type field we misread still shows up rather than vanishing + // from the instrument that exists to find it. + if !(net.kind == EntityKind::GameObject && matches!(go_type, 11 | 15) + || transport.is_some() + || anchor.is_some() + || seed) + { + continue; + } + let state = match (transport, anchor, seed) { + (Some(tr), _, _) => tr.drive_label(), + (None, Some(_), true) => "seed", + (None, Some(_), false) => "bare", + (None, None, _) => "parked", + }; + let hidden = vis == Some(&Visibility::Hidden) || inherited.is_some_and(|i| !i.get()); + let mesh_n = render_descendants(entity_children(kids), &children, &meshes); + if hidden { + hidden_n += 1; + } + if transport.is_none() { + unarmed_n += 1; + } + if attached && mesh_n == 0 { + meshless_n += 1; + } + let cycle = transport.zip(anchor).map(|(tr, a)| tr.cycle_ms(a)); + let sample = transport + .zip(anchor) + .map(|(tr, a)| tr.sample_at(a, current_map.as_ref().map_or(0, |m| m.0))); + let wow = bevy_to_wow(t.translation); + let dist = at.map_or(f32::NAN, |b| t.translation.distance(b)); + rows.push(( + hidden, + (dist * 100.0) as i64, + format!( + "LIFT {:#018x} entry={:<7} disp={:<6} type={go_type:<3} state={state:<7} \ + period={:<6} cycle={:<6} moving={} vis={:<9} inh={} meshes={mesh_n:<3} \ + attached={} pos=({:.2},{:.2},{:.2}) d={dist:.1}", + guid.0, + store.0.object_entry().unwrap_or(0), + net.display_id.unwrap_or(0), + transport.map_or(0, Transport::period_ms), + cycle.map_or_else(|| "-".into(), |c| c.to_string()), + sample.map_or(0, |s| u8::from(s.moving)), + vis.map_or_else(|| "absent".into(), |v| format!("{v:?}")), + inherited.map_or(0, |i| u8::from(i.get())), + u8::from(attached), + wow[0], + wow[1], + wow[2], + ), + )); + } + rows.sort_by(|a, b| b.0.cmp(&a.0).then(a.1.cmp(&b.1))); + println!( + "LIFT_CENSUS t={now:.1} transports={} hidden={hidden_n} unarmed={unarmed_n} \ + meshless={meshless_n} map={} body={}", + rows.len(), + current_map.map_or_else(|| "none".into(), |m| m.0.to_string()), + at.map_or_else( + || "none".into(), + |b| { + let w = bevy_to_wow(b); + format!("({:.2},{:.2},{:.2})", w[0], w[1], w[2]) + } + ), + ); + for (_, _, line) in &rows { + println!("{line}"); + } +} + +/// A borrowed child slice, or an empty one — the `Option<&Children>` unwrap the walk starts from. +fn entity_children(kids: Option<&Children>) -> Vec { + kids.map(|c| c.iter().collect()).unwrap_or_default() +} + +/// Count the render descendants of a subtree — the whole subtree, because a GameObject's +/// submeshes hang under an anim host rather than directly off the net root. +fn render_descendants( + roots: Vec, + children: &Query<&Children>, + meshes: &Query<(), With>, +) -> u32 { + let mut frontier = roots; + let mut found = 0; + for _ in 0..WALK_DEPTH { + if frontier.is_empty() { + break; + } + let mut next = Vec::new(); + for e in frontier.drain(..) { + found += u32::from(meshes.contains(e)); + if let Ok(kids) = children.get(e) { + next.extend(kids.iter()); + } + } + frontier = next; + } + found +} diff --git a/crates/benilla-app/src/capture/probes/live_fps.rs b/crates/benilla-app/src/capture/probes/live_fps.rs index 8be3718c..7cd9d938 100644 --- a/crates/benilla-app/src/capture/probes/live_fps.rs +++ b/crates/benilla-app/src/capture/probes/live_fps.rs @@ -376,8 +376,17 @@ fn drive_live_fps( let culled = match seen.cull.as_ref() { Some(v) => format!( " cull_windows={} cull_frusta={} cull_tested={} cull_hidden={} \ - cull_unbounded={} cull_bodies={} cull_bodies_hidden={}", - v.windows, v.frusta, v.tested, v.hidden, v.unbounded, v.bodies, v.bodies_hidden + cull_unbounded={} cull_bodies={} cull_bodies_hidden={} \ + cull_liquid={} cull_liquid_hidden={}", + v.windows, + v.frusta, + v.tested, + v.hidden, + v.unbounded, + v.bodies, + v.bodies_hidden, + v.liquid, + v.liquid_hidden ), None => String::new(), }; diff --git a/crates/benilla-app/src/capture/probes/mod.rs b/crates/benilla-app/src/capture/probes/mod.rs index 79340f2b..6dc68384 100644 --- a/crates/benilla-app/src/capture/probes/mod.rs +++ b/crates/benilla-app/src/capture/probes/mod.rs @@ -36,6 +36,12 @@ pub(crate) use particle_census::ParticleCensusPlugin; mod ground_census; pub(crate) use ground_census::GroundCensusPlugin; +/// The transport census — per type-11/15 GameObject, the arm stage, the visibility pair and the +/// render-descendant count: the reader that separates a lift the server never sent from one that +/// never armed and so never lifted its spawn-hide (B168). +mod lift_census; +pub(crate) use lift_census::LiftCensusPlugin; + /// The unit-visual census — per-entity "what visual did this display actually get", which is what /// separates a debug cube (a gap of ours) from a model that legitimately draws nothing (B13, /// decision 1403). diff --git a/crates/benilla-app/src/capture/probes/visual_census.rs b/crates/benilla-app/src/capture/probes/visual_census.rs index e7137da6..03b3d4e2 100644 --- a/crates/benilla-app/src/capture/probes/visual_census.rs +++ b/crates/benilla-app/src/capture/probes/visual_census.rs @@ -13,13 +13,20 @@ //! One line per streamed entity says which, without an eye: //! //! ```text -//! UVIS 0xF130003A7200013B Unit display=13069 d=6.4 cube=1 meshes=0 Zandalarian Event Generator +//! UVIS 0xF130003A7200013B Unit display=13069 d=6.4 cube=1 meshes=0 held=[] Zandalarian Event Generator //! ``` //! //! - **`cube`** is the [`FallbackCube`] marker — literally the arm that spawned it, not a guess //! from the picture. The headline `cubes=` count is the number that names the bug. -//! - **`meshes`** counts the entity's spawned render children. `cube=0 meshes=0` is the *correct* -//! reading for a trigger creature: attached, and drawing nothing. +//! - **`meshes`** counts the entity's spawned render children — the BODY's own batches. `cube=0 +//! meshes=0` is the *correct* reading for a trigger creature: attached, and drawing nothing. +//! - **`held`** is the attach-slot list actually hanging off its skeleton +//! ([`HeldAttached::spawned_slots`], the dress census's own reader). It is here because +//! `meshes` counts *direct* children and an attached model is a grandchild, so a unit whose +//! whole visible self is a held weapon — the Naxxramas `Unholy Axe`, an `InvisibleStalker` body +//! with an axe in its hand — reads `meshes=0` whether the axe is there or not. That blind spot +//! is what let decision 1618's defect sit under a green census; `held=[main]` is the line that +//! says the weapon arrived. //! - **`pending`** marks an entity whose visual has not been built yet (a model still streaming) — //! never to be confused with one that built nothing, which is the distinction the census exists //! to keep. @@ -41,7 +48,7 @@ use benilla_protocol::EntityKind; use bevy::prelude::*; use super::ProbeClock; -use crate::entities::{FallbackCube, VisualAttached}; +use crate::entities::{FallbackCube, HeldAttached, VisualAttached, ATTACH_SLOT_NAMES}; use crate::names::NameCache; use crate::net::{Guid, NetEntity, SelfPlayer}; @@ -90,6 +97,9 @@ type VisualQuery = ( &'static Transform, Option<&'static Children>, Has, + // The attach slots standing under this unit — the visual truth for everything that rides a + // BONE rather than the entity, which `Children` above cannot see. + Option<&'static HeldAttached>, ); /// One line per streamed entity within [`UnitVisuals::radius`] of the body — cubes first, then @@ -123,7 +133,7 @@ fn fire_unit_visuals( let mut rows: Vec<(bool, i64, String)> = Vec::new(); let (mut cube_n, mut blank_n, mut pending_n) = (0u32, 0u32, 0u32); let mut cube_displays: Vec = Vec::new(); - for (guid, net, t, children, attached) in &entities { + for (guid, net, t, children, attached, held) in &entities { if matches!(net.kind, EntityKind::DynamicObject | EntityKind::Other) || t.translation.distance_squared(body.translation) > radius2 { @@ -149,15 +159,25 @@ fn fire_unit_visuals( blank_n += 1; } let dist = t.translation.distance(body.translation); + let held: Vec<&str> = held + .map(|h| { + h.spawned_slots() + .iter() + .zip(ATTACH_SLOT_NAMES) + .filter_map(|(slot, name)| slot.map(|_| name)) + .collect() + }) + .unwrap_or_default(); rows.push(( cube, (dist * 100.0) as i64, format!( "UVIS {:#018x} {:<12} display={display:<6} d={dist:6.1} cube={} meshes={mesh_n:<3} \ - {:<8} {}", + held=[{:<12}] {:<8} {}", guid.0, format!("{:?}", net.kind), u8::from(cube), + held.join(","), if attached { "attached" } else { "PENDING" }, names.peek(guid.0).unwrap_or("?"), ), diff --git a/crates/benilla-app/src/creature_anim/driver.rs b/crates/benilla-app/src/creature_anim/driver.rs index cea472ae..d94533e7 100644 --- a/crates/benilla-app/src/creature_anim/driver.rs +++ b/crates/benilla-app/src/creature_anim/driver.rs @@ -1106,28 +1106,45 @@ pub(super) fn drive_animations( // the callback-less map doodads, which loop one variation forever, are `doodad_anim`'s // separate concern). A combat/cast re-arm keeps the deterministic head, like any base // arm. The carried rate keeps the gait's scaling; the per-frame sync re-syncs it anyway. + // + // **In the GAIT slot the completion is a re-SELECTION, not a re-arm** (decision 1655). + // The client's callback is `RecomputeBaseAnim(−1)` — `0x5fc3f0`'s jump-table row + // `0x5fc844` is literally `push -1; call 0x5fd9e0`, which runs the whole `0x5fd8b0` + // selector chain. For Fly and /dance the chain hands back the id it was already playing, + // so a same-id re-arm and a re-selection are indistinguishable and this code read as + // right for two years. For the turn-shuffle they are the difference between stepping once + // and shuffling for ever: the shuffle's window is the ONLY thing that ends it (the + // per-frame tail refuses to — see [`select::SHUFFLE_LEFT`] and `mode`'s hold), so if the + // completion re-armed 11 the vendor would never stop. Clearing the target IS the + // re-selection — `mode::run` picks fresh a few lines below, rolling the variation exactly + // as this arm would have. Outside the gait slot (a Special loop) the id is not the + // chain's to choose, so the same-id re-arm stands. if let Some((node, budget)) = drv.loop_window { if tr.get_main_animation() == Some(node) && player .animation(node) .is_some_and(|a| a.completions() >= budget) { - let head = anims - .clips - .iter() - .find(|c| c.node == node) - .and_then(|armed| find_resolved(anims, armed.anim_id, catalog)); - if let Some(head) = head { - let rate = player.animation(node).map_or(1.0, |a| a.speed()); - let (c, fresh) = roll_loop(anims, head, relaxed, &mut rng); - play_clip( - &mut tr, - &mut player, - c, - bevy::animation::RepeatAnimation::Forever, - rate, - ); - drv.loop_window = Some((c.node, fresh)); + if drv.mode == Mode::Gait { + drv.gait = None; + } else { + let head = anims + .clips + .iter() + .find(|c| c.node == node) + .and_then(|armed| find_resolved(anims, armed.anim_id, catalog)); + if let Some(head) = head { + let rate = player.animation(node).map_or(1.0, |a| a.speed()); + let (c, fresh) = roll_loop(anims, head, relaxed, &mut rng); + play_clip( + &mut tr, + &mut player, + c, + bevy::animation::RepeatAnimation::Forever, + rate, + ); + drv.loop_window = Some((c.node, fresh)); + } } } } diff --git a/crates/benilla-app/src/creature_anim/driver/mode.rs b/crates/benilla-app/src/creature_anim/driver/mode.rs index 912d1df7..26f3efa3 100644 --- a/crates/benilla-app/src/creature_anim/driver/mode.rs +++ b/crates/benilla-app/src/creature_anim/driver/mode.rs @@ -479,6 +479,34 @@ pub(super) fn run( cands }; let target = cands[0]; + // **The turn-shuffle is released by its own clip window, not by the turn ending** + // (decision 1655, wow-re `object-layer/scratch/turn-shuffle-lifecycle.md`). + // + // The client's only per-frame poll of a standing unit's base animation is + // `0x607ed0`'s tail, and it *refuses* this exact transition: with the shuffle + // armed and no turn bit set it computes target 0, sees `0xb != 0`, and calls + // `0x5fce30` — whose first gate (`0x5fce33`/`0x5fce42`) needs `moveflags & 0x30` + // or `d58 & 0x1800`, precisely what just went false. `0x6084da je` then skips the + // recompute. **The tail can only START a shuffle, never stop one.** What stops it + // is the animation-completion callback every in-world unit installs + // (`0x60781b`), one clip window later — which is why the reference plays a + // discrete little step rather than a stub of one: with `replay = (0,0)` on all + // 1130 shipped shuffle records the window is exactly one span, 500 ms on 1100 of + // them, so the step is `ceil(t_turn / span) · span` long however brief the turn. + // + // Only against **Stand**. Every other target — a walk, the mount pin, the cast + // hold, the Ready idle, a state emote, the loot kneel — arrives in the client + // through one of `0x5fd9e0`'s 37 edge-triggered call sites, which fire regardless + // of the turn bits and preempt the shuffle at once. The refusal is the tail's + // alone, and the tail's target is a hardcoded 0. + let target = match drv.gait { + Some(g @ (select::SHUFFLE_LEFT | select::SHUFFLE_RIGHT)) + if target == STAND && !window_complete(drv, player) => + { + g + } + _ => target, + }; // Each RF-0057 candidate, in priority order, resolved through the model's own // baked fallback (decision 0082) before moving to the next candidate — a model // missing the exact id still plays its baked substitute rather than stepping @@ -547,3 +575,18 @@ pub(super) fn run( } } } + +/// Has the base slot's armed looping clip finished its **replay window** — the client's +/// `windowHi = windowLo + span·R` (decision 0516 §7d, wow-re `loop-replay-fidget.md` §7a)? +/// +/// `true` when there is no window to wait on at all: a slot with nothing armed, or one whose +/// window a newer arm superseded, is not something to hold a shuffle against. The driver's own +/// watchdog reads the same pair one phase earlier and, in the gait slot, answers a completion by +/// clearing the target — so a `true` here and a re-selection there are the same event. +fn window_complete(drv: &AnimDriver, player: &AnimationPlayer) -> bool { + drv.loop_window.is_none_or(|(node, budget)| { + player + .animation(node) + .is_none_or(|a| a.completions() >= budget) + }) +} diff --git a/crates/benilla-app/src/creature_anim/driver/tests.rs b/crates/benilla-app/src/creature_anim/driver/tests.rs index 8f8b78f5..aa8bbba8 100644 --- a/crates/benilla-app/src/creature_anim/driver/tests.rs +++ b/crates/benilla-app/src/creature_anim/driver/tests.rs @@ -293,48 +293,92 @@ fn moving_cast_hold_keeps_its_stow_between_plays() { assert_eq!(sheath(&app), Some(1), "the stop's Ready play re-draws"); } -/// A fidgeter's model: Stand as a two-variation chain — a zero-frequency head plus a -/// max-frequency "look around" variation, so the first `_rand` roll (38, from the LCG's zero -/// seed) deterministically lands on the variation — and a ShuffleLeft clip for the turn latch. -fn fidget_model() -> ModelAnimations { - let mut head = clip(0, 1, true); // Stand — the head variation - head.frequency = 0; - let mut look = clip(0, 6, true); // Stand — the rare look-around variation - look.frequency = 32767; - ModelAnimations { - graph: Handle::default(), - clips: vec![head, look, clip(11, 7, true) /* ShuffleLeft */], +/// A fidgeter's model, on **real Bevy clip assets** (see [`spawn_vendor`] for why: this tenant now +/// runs to a window completion, and `completions()` only ticks for a real graph): Stand as a +/// two-variation chain — a zero-frequency head plus a max-frequency "look around" variation, so +/// the first `_rand` roll (38, from the LCG's zero seed) deterministically lands on the variation — +/// and a ShuffleLeft for the turn latch. Stand's span is long enough that its own window never +/// completes inside the test; the shuffle's is short so the test does not have to run a real +/// half-second to reach the release. +/// +/// Returns the entity and the graph nodes in clip order: Stand-head, Stand-look, ShuffleLeft. +fn spawn_fidgeter(app: &mut App) -> (Entity, Vec) { + use bevy::animation::graph::{AnimationGraph, AnimationGraphHandle}; + use bevy::animation::AnimationClip; + + const STAND_SPAN: f32 = 4.0; + const SHUFFLE_SPAN: f32 = 0.1; + let handles: Vec<_> = [STAND_SPAN, STAND_SPAN, SHUFFLE_SPAN] + .iter() + .map(|d| { + let mut c = AnimationClip::default(); + c.set_duration(*d); + app.world_mut() + .resource_mut::>() + .add(c) + }) + .collect(); + let (graph, nodes) = AnimationGraph::from_clips(handles); + let graph_handle = app + .world_mut() + .resource_mut::>() + .add(graph); + let seq = |id: u16, node, duration: f32, frequency: u16| AnimClip { + node, + duration, + blend_time: 0.0, + frequency, + replay: (0, 0), + ..clip(id, 0, true) + }; + let anims = ModelAnimations { + graph: graph_handle.clone(), + clips: vec![ + seq(0, nodes[0], STAND_SPAN, 0), // Stand — the head variation + seq(0, nodes[1], STAND_SPAN, 32767), // Stand — the rare look-around variation + seq(11, nodes[2], SHUFFLE_SPAN, 0), // ShuffleLeft + ], hand_close: [None, None], playable_animation_lookup: Vec::new(), animation_lookup: Vec::new(), global_bones: Vec::new(), first_seq: None, pose: Default::default(), - } + }; + let unit = app + .world_mut() + .spawn(( + anims, + AnimationPlayer::default(), + AnimationTransitions::new(), + AnimationGraphHandle(graph_handle), + AnimDriver::default(), + )) + .id(); + (unit, nodes) } /// The emergent idle fidget (decision 0123 — wow-re `loop-replay-fidget.md` §5b): a RELAXED base /// arm rolls its variation (the client's `variationIdx = −1`), an engaged one is forced to the /// deterministic head, and the idle re-face turn-shuffle ([`crate::net::FacingStep`]) drives the /// Shuffle↔Stand churn whose every return to Stand re-rolls. +/// +/// **What the return is triggered BY changed in 1655**, and that is the middle of this test. 0123 +/// had the shuffle released the frame the yaw ease settled; the reference cannot do that +/// (`0x607ed0`'s tail refuses to recompute a shuffle with no turn bit set — `0x5fce30`'s gate), +/// and releases it a clip window later through the completion callback instead. The churn 0123 +/// describes is intact and the re-roll still rides it; it simply happens at the window boundary, +/// which is also why the shuffle is long enough to be seen. #[test] fn relaxed_base_arms_roll_variations_and_the_shuffle_drives_them() { let mut app = app(); - let unit = app - .world_mut() - .spawn(( - fidget_model(), - AnimationPlayer::default(), - AnimationTransitions::new(), - AnimDriver::default(), - )) - .id(); - let active = |app: &App, node: u32| { + let (unit, nodes) = spawn_fidgeter(&mut app); + let active = |app: &App, node: AnimationNodeIndex| { app.world() .entity(unit) .get::() .unwrap() - .animation(AnimationNodeIndex::new(node as usize)) + .animation(node) .is_some() }; let gait = |app: &App| app.world().entity(unit).get::().unwrap().gait; @@ -342,8 +386,8 @@ fn relaxed_base_arms_roll_variations_and_the_shuffle_drives_them() { // The first (relaxed) Stand arm rolls: the weighted walk lands on the look-around variation. app.update(); assert_eq!(gait(&app), Some(0)); - assert!(active(&app, 6), "the rolled variation is what armed"); - assert!(!active(&app, 1), "not the head"); + assert!(active(&app, nodes[1]), "the rolled variation is what armed"); + assert!(!active(&app, nodes[0]), "not the head"); // The idle re-face steps its yaw: the turn latch routes the gait to the foot-shuffle. app.world_mut() @@ -352,14 +396,25 @@ fn relaxed_base_arms_roll_variations_and_the_shuffle_drives_them() { app.update(); assert_eq!(gait(&app), Some(11), "stepping yaw → ShuffleLeft"); - // The ease settles: Shuffle → Stand is a fresh relaxed re-arm — a fresh roll (the fidget). + // The ease settles — and the feet keep going, because the settle is not what releases them. app.world_mut() .entity_mut(unit) .remove::(); - app.update(); - assert_eq!(gait(&app), Some(0), "settled → back to Stand"); + advance(&mut app, 25); + assert_eq!( + gait(&app), + Some(11), + "the settle does not release the shuffle (1655)" + ); + + // Its window completes: THAT is the release, and the re-selection it runs re-arms Stand with a + // fresh roll — the fidget. + for _ in 0..6 { + advance(&mut app, 25); + } + assert_eq!(gait(&app), Some(0), "the completed window → back to Stand"); assert!( - active(&app, 1) || active(&app, 6), + active(&app, nodes[0]) || active(&app, nodes[1]), "some Stand variation re-armed" ); } @@ -369,25 +424,14 @@ fn relaxed_base_arms_roll_variations_and_the_shuffle_drives_them() { #[test] fn engaged_base_arms_keep_the_head_variation() { let mut app = app(); - let unit = app - .world_mut() - .spawn(( - fidget_model(), - AnimationPlayer::default(), - AnimationTransitions::new(), - AnimDriver::default(), - Engaged, - )) - .id(); + let (unit, nodes) = spawn_fidgeter(&mut app); + app.world_mut().entity_mut(unit).insert(Engaged); app.update(); let player = app.world().entity(unit).get::().unwrap(); // Engaged with no weapon: the Ready pick resolves down to Stand — armed as the HEAD. + assert!(player.animation(nodes[0]).is_some(), "the head variation"); assert!( - player.animation(AnimationNodeIndex::new(1)).is_some(), - "the head variation" - ); - assert!( - player.animation(AnimationNodeIndex::new(6)).is_none(), + player.animation(nodes[1]).is_none(), "no roll while engaged" ); } @@ -3211,3 +3255,195 @@ fn every_shot_of_a_volley_re_arms_the_fire_clip_through_the_emote_lane() { "dropping the arm stands the shooter up" ); } + +/// A vendor built on **real Bevy clip assets**, not the stand-in `clip()` handles the rest of this +/// file uses: this tenant is about a window *completing*, and `completions()` only ticks for an +/// entity whose graph is a real asset (`advance_animations` skips the others — see [`app`]). The +/// timings are the shipped HumanMale's (`benilla-extract m2seq`): Stand 2.667 s / blend 0.500 s, +/// ShuffleLeft & ShuffleRight 0.500 s / blend 0.250 s, and `replay = (0,0)` on both shuffles — +/// which is what every one of the 1130 shipped shuffle records carries, so `R` is a deterministic +/// 1 and the window is exactly one span. +/// +/// Returns the entity plus the three graph nodes, in id order: Stand, ShuffleLeft, ShuffleRight. +fn spawn_vendor(app: &mut App) -> (Entity, Vec) { + use benilla_protocol::EntityKind; + use bevy::animation::graph::{AnimationGraph, AnimationGraphHandle}; + use bevy::animation::AnimationClip; + + let handles: Vec<_> = [2.667f32, 0.5, 0.5] + .iter() + .map(|d| { + let mut c = AnimationClip::default(); + c.set_duration(*d); + app.world_mut() + .resource_mut::>() + .add(c) + }) + .collect(); + let (graph, nodes) = AnimationGraph::from_clips(handles); + let graph_handle = app + .world_mut() + .resource_mut::>() + .add(graph); + let seq = |id: u16, node, duration: f32, blend_time: f32| AnimClip { + node, + duration, + blend_time, + replay: (0, 0), + ..clip(id, 0, true) + }; + let anims = ModelAnimations { + graph: graph_handle.clone(), + clips: vec![ + seq(0, nodes[0], 2.667, 0.5), + seq(11, nodes[1], 0.5, 0.25), + seq(12, nodes[2], 0.5, 0.25), + ], + hand_close: [None, None], + playable_animation_lookup: Vec::new(), + animation_lookup: Vec::new(), + global_bones: Vec::new(), + first_seq: None, + pose: Default::default(), + }; + // The vendor five yards east of the world origin, authored facing +x — its back to a player + // standing north-west of it, which is the reporter's own screenshot in B110. + let npc = app + .world_mut() + .spawn(( + anims, + AnimationPlayer::default(), + AnimationTransitions::new(), + AnimationGraphHandle(graph_handle), + AnimDriver::default(), + crate::net::NetEntity { + kind: EntityKind::Unit, + display_id: None, + scale: 1.0, + }, + crate::net::ObjectStore::default(), + Transform { + translation: benilla_assets::coords::wow_to_bevy([5.0, 0.0, 0.0]), + rotation: Quat::from_rotation_y(0.0), + ..default() + }, + )) + .id(); + (npc, nodes) +} + +/// The **cross-seam** test for the turn-shuffle (decision 1655): `net::motion::facing` produces the +/// [`crate::net::FacingStep`] latch and this driver consumes it, and until this test the two had +/// only ever been exercised apart — the producer's tests read the transform, the consumer's tests +/// inserted the component by hand. Neither could see the seam, which is how a mechanism that was +/// all present and firing produced nothing anyone could see: "when interacting with a vendor the +/// NPC doesn't shuffle its feet, it turns frozen" (director, 2026-08-27). +/// +/// Two laws meet here, and the test is written so that failing either one is legible: +/// +/// - **the latch runs the ease out** — the client tests the yaw its pump APPLIED against ±1e-5, +/// not the gap remaining against an eyeballed 3°, so the shuffle blends in properly instead of +/// being handed back at 0.32 weight; +/// - **the shuffle is released by its own clip window, never by the turn ending** — `0x607ed0`'s +/// tail can only start one (`0x5fce30`'s gate needs a turn bit, which is exactly what going +/// still clears), so what ends it is the completion callback one span later. That is why the +/// reference plays a discrete little step and not a stub of one, and it is why the middle +/// assertion here is that the feet are STILL moving long after the body has stopped. +#[test] +fn the_interaction_face_me_shuffles_its_feet_for_the_whole_turn() { + let mut app = app(); + app.init_resource::() + .init_resource::(); + app.add_systems( + Update, + crate::net::drive_display_facing.before(drive_animations), + ); + // Us off the vendor's axis — a bearing of ~2.60 rad, the representative case. (A goal at + // exactly ±pi sits on the yaw wrap, where the client's own unfolded-delta dead-band never + // snaps and the ease runs to float underflow instead; `net::motion::facing`'s tests say the + // same thing about the same corner.) + app.world_mut().spawn(( + crate::net::SelfPlayer, + crate::net::ActiveMover, + Transform::from_translation(benilla_assets::coords::wow_to_bevy([0.0, 3.0, 0.0])), + )); + let (npc, nodes) = spawn_vendor(&mut app); + + let gait = |app: &App| app.world().entity(npc).get::().unwrap().gait; + let turning = |app: &App| app.world().entity(npc).contains::(); + // How heavily either shuffle is being blended in — the "is it actually on screen" number. + let shuffle_weight = |app: &App| { + let p = app.world().entity(npc).get::().unwrap(); + nodes[1..] + .iter() + .filter_map(|n| p.animation(*n).map(|a| a.weight())) + .fold(0.0f32, f32::max) + }; + // Run `ms` of 16 ms frames, reporting the heaviest the shuffle got and whether it held the + // gait slot throughout. + let run = |app: &mut App, ms: u64, want: u16| { + let (mut peak, mut held) = (0.0f32, true); + for _ in 0..(ms / 16) { + advance(app, 16); + peak = peak.max(shuffle_weight(app)); + held &= gait(app) == Some(want); + } + (peak, held) + }; + + // The control: nothing open, nothing turning — the vendor stands, and keeps standing. + let (peak, _) = run(&mut app, 128, 0); + assert_eq!(gait(&app), Some(0), "no window, no turn"); + assert_eq!(peak, 0.0, "and no shuffle to be seen"); + + // Open its window. It turns to us — and its feet move while it does. 256 ms is past the ease + // (this bearing closes in ten pumps ~= 160 ms) and past the shuffle's own 250 ms blend-in. + app.world_mut() + .resource_mut::() + .0 = Some(npc); + let (peak, held) = run(&mut app, 256, 11); + assert!(held, "the whole turn is ShuffleLeft, got {:?}", gait(&app)); + assert!( + peak > 0.95, + "the shuffle blends the whole way in; the frozen-feet defect peaked at 0.32, got {peak}" + ); + + // **The body has stopped and the feet have not.** The ease settled ~100 ms ago; the shuffle is + // held to its 500 ms window, which is what makes it a step rather than a twitch. A driver that + // released on the settle fails here — and so does one that released on the latch, one frame + // after the yaw went quiet. + assert!(!turning(&app), "the ease has settled"); + let (_, held) = run(&mut app, 192, 11); + assert!( + held, + "the shuffle is held past the settle to its own window, got {:?}", + gait(&app) + ); + + // …and released AT that window, not never. `0x5fc3f0`'s completion row is a + // `RecomputeBaseAnim(-1)`, so the chain now answers Stand — with a freshly rolled variation, + // which is the idle fidget of 0123. + run(&mut app, 160, 0); + assert_eq!( + gait(&app), + Some(0), + "the completed window returns it to Stand" + ); + // Let the idle finish arriving before the next half — Stand's own blend is 500 ms, and a + // window closed mid-fade would put THREE tracks on the slot at once (the retiring shuffle, + // the half-arrived Stand, the new shuffle) and split the weight three ways. That is a real + // divergence we carry knowingly — the reference's `0x7125d4` refuses to re-seed a blend that + // has not passed half weight (1565 §6, 1570) — but it is not what this test is about, and a + // player browsing a vendor's stock is well past it either way. + run(&mut app, 640, 0); + + // Close it. The goal falls back to the untouched wire facing and it swings home — "and again + // when it turns back" (the report's second half), so the feet move for that turn too, on the + // other side. + app.world_mut() + .resource_mut::() + .0 = None; + let (peak, held) = run(&mut app, 256, 12); + assert!(held, "the swing back is ShuffleRight, got {:?}", gait(&app)); + assert!(peak > 0.95, "and blends the whole way in: {peak}"); +} diff --git a/crates/benilla-app/src/creature_anim/select.rs b/crates/benilla-app/src/creature_anim/select.rs index 4f154904..55024999 100644 --- a/crates/benilla-app/src/creature_anim/select.rs +++ b/crates/benilla-app/src/creature_anim/select.rs @@ -22,6 +22,15 @@ pub(super) const STAND: u16 = 0; pub(super) const DEATH: u16 = 1; +/// ShuffleLeft (11) / ShuffleRight (12) — the **turn-in-place foot-shuffle**, picked below when +/// the unit turns without translating. Named because their *lifecycle* is unlike any other gait: +/// once armed they are held to their own clip window rather than released when the turn ends +/// (decision 1655, wow-re `object-layer/scratch/turn-shuffle-lifecycle.md`), and the driver has +/// to be able to say so. +pub(super) const SHUFFLE_LEFT: u16 = 11; +/// See [`SHUFFLE_LEFT`]. +pub(super) const SHUFFLE_RIGHT: u16 = 12; + /// StealthWalk (119) — the **prowl creep**, and with [`STEALTH_STAND`] the entire difference /// stealth makes to a body's *pose*. Byte-verified (wow-re `rf57-movement-anim-select.md`, the /// 2026-07-18 §5): both ids are gated by the same descriptor bit — `[[unit+0x110]+0x213] & 2`, @@ -516,10 +525,10 @@ pub(super) fn gait_candidates( // ShuffleLeft 11 / ShuffleRight 12). Only reached when not moving — moving with a turn curves the // run path and plays the gait above. if f & TURN_LEFT != 0 { - return &[11, 0]; + return &[SHUFFLE_LEFT, STAND]; } if f & TURN_RIGHT != 0 { - return &[12, 0]; + return &[SHUFFLE_RIGHT, STAND]; } // Standing + engaged in melee: the weapon-class Ready idle (decision 0073 — gated on engagement, // not sheath state; outranks the chair poses, since a fighting unit isn't seated). diff --git a/crates/benilla-app/src/debug_panel/world.rs b/crates/benilla-app/src/debug_panel/world.rs index 7478ff2c..94231458 100644 --- a/crates/benilla-app/src/debug_panel/world.rs +++ b/crates/benilla-app/src/debug_panel/world.rs @@ -122,8 +122,9 @@ pub(super) fn world_section(ui: &mut egui::Ui, world: &mut WorldReadout) { // * `windows N` with N large, or a window covering most of the screen — we are indoors and the // doorway rects are too generous. // * `windows 0` — sealed, nothing exterior may draw; terrain visible anyway means something - // is reaching the screen that is not tagged `ExteriorScene` (world WMO placements and - // open-world liquid are knowingly not gated yet). + // is reaching the screen that is not tagged `ExteriorScene`. Every exterior bucket is + // tagged now — WMO placements since 0784, open-world liquid since 1652 — so at `windows 0` + // the honest reading of anything still drawn is a bug, not a known gap. // Two numbers is the whole diagnosis, and neither was readable from the chair before. let room_line = match world.room.0 { Some(claim) => format!("room g{:02}", claim.room.group), diff --git a/crates/benilla-app/src/dev.rs b/crates/benilla-app/src/dev.rs index 4a23896b..a4fc1a8a 100644 --- a/crates/benilla-app/src/dev.rs +++ b/crates/benilla-app/src/dev.rs @@ -201,6 +201,15 @@ impl Plugin for DevProbesPlugin { if std::env::var("WOW_GROUND_CENSUS").is_ok() { app.add_plugins(crate::capture::GroundCensusPlugin); } + // The transport census: `WOW_LIFT_CENSUS=[,]` prints one line per + // type-11/15 transport GameObject on the map — its arm stage, its cycle, its + // visibility pair and its render-descendant count. The instrument B168 was missing: a + // lift that isn't on screen is either unsent, unarmed (and so still wearing its spawn + // hide), model-less, or simply elsewhere in its cycle, and only numbers tell those + // four apart (see `capture::LiftCensusPlugin`). + if std::env::var("WOW_LIFT_CENSUS").is_ok() { + app.add_plugins(crate::capture::LiftCensusPlugin); + } // The unit-visual census: `WOW_UNIT_VISUALS=[,]` prints one line per // streamed entity near the body — whether it got a debug cube, real geometry, or // nothing at all. The instrument B13 was missing: a black slab in a screenshot cannot diff --git a/crates/benilla-app/src/entities/attach/mod.rs b/crates/benilla-app/src/entities/attach/mod.rs index a09b254d..b628f11c 100644 --- a/crates/benilla-app/src/entities/attach/mod.rs +++ b/crates/benilla-app/src/entities/attach/mod.rs @@ -57,12 +57,21 @@ pub(super) use redress::redress_player_looks; /// has no inverse bindposes. Slot `0` = the palette table was full: anchors still resolve /// (emitters/attachments ride them), but parts fall back to the static bind-pose mesh. No /// animations ⇒ the pose just holds bind pose (Milestone A). +/// +/// `skins` is whether anything will actually **skin through this slot** — the same law the item +/// lane's rider states (`equipment::spawn`: "a slot only earns its keep if something will +/// actually SKIN through it"). A body that drew no batch at all (the invisible trigger creature +/// holding a visible weapon — decision 1618) still needs the *pose*, because its attachment +/// points carry what IS drawn, but it has nothing of its own to skin: it takes the slot-less rig +/// [`benilla_world::rig_anim::finalize_rig_worlds`] has supported since 1365, and no `RigStarved` +/// marker either (there is no starvation to heal). fn setup_skinned_instance( commands: &mut Commands, palettes: &mut benilla_world::rig_palette::RigPalettes, entity: Entity, joints_root: Entity, d: &DisplayModel, + skins: bool, ) -> Option { let ibp = d.inverse_bindposes.as_ref()?; let nbones = d.skeleton.joints.len(); @@ -80,11 +89,16 @@ fn setup_skinned_instance( // The owned palette rig (decision 0720): the world pass writes this rig's composed frames × // these bindposes into the slot; every skinned part below tags the slot so the vertex stage // finds its palette. The on-replace hook frees the slot with the visual teardown. - let slot = match benilla_world::rig_palette::RigSkin::allocate_bones( - palettes, - nbones as u32, - ibp.clone(), - ) { + let slot = match skins + .then(|| { + benilla_world::rig_palette::RigSkin::allocate_bones( + palettes, + nbones as u32, + ibp.clone(), + ) + }) + .flatten() + { Some(rig) => { let slot = rig.slot; // The marker's invariant is "currently slot-less": a heal-triggered (or any @@ -95,6 +109,10 @@ fn setup_skinned_instance( .remove::(); slot } + // Nothing to skin (`skins == false`): the slot-less rig, on purpose and for ever — no + // starvation marker, or `heal_rig_starved` would rebuild this body every time the table + // gained headroom, chasing a slot it will never ask for. + None if !skins => 0, None => { // Table full (warned): parts render the static bind-pose mesh — but no longer // for ever. The marker hands the unit to `heal_rig_starved` (decision 0863), @@ -300,30 +318,38 @@ pub(super) fn attach_entity_visuals( // CreatureDisplayInfoExtra columns. Zeroed (a beast / GameObject / no data) = the naked body. let worn = resolve_worn_equip(net, equipment, dm); let equip = worn.bodyslots; - // A model still loading (`parts == None`) waits — leave it un-attached and retry next frame - // rather than flash a cube we'd swap out. A built-but-empty model draws nothing. - let model = match dm { - Some(d) => match &d.parts { - None => continue, - Some(parts) if !parts.is_empty() => Some(parts.as_slice()), - Some(_) => None, - }, - None => None, - }; // **Did the display name a model file at all?** An empty `parts` list means two opposite // things, and the cube below may only answer one of them (decision 1403): a display that // resolved to a model which built zero batches has been *answered* — the model draws // nothing — while a display that resolved to no model at all is a gap of ours. let named_a_model = dm.is_some_and(DisplayModel::names_a_model); + // A model still loading (`parts == None`) waits — leave it un-attached and retry next frame + // rather than flash a cube we'd swap out. + // + // **The gate is "did the display name a model", not "did it build a batch"** (decision + // 1618). A model that named a file and built ZERO batches is a *loaded model instance that + // draws nothing* — the reference builds its `CM2Model` like any other and the per-batch + // loop simply has no trip to make — and everything an entity takes from its model but its + // pixels comes from that instance: the skeleton, the attachment points a held weapon rides, + // the sequence clock, the PlayerName anchor, the emitters. Keying the whole build on a + // drawn batch is what left the Naxxramas weapon mobs — an `InvisibleStalker` body holding + // a visible axe — with no axe and their name plate flat on the floor. + let model = match dm { + Some(d) => match &d.parts { + None => continue, + Some(parts) => named_a_model.then_some(parts.as_slice()), + }, + None => None, + }; // Invisible interaction-zone GameObjects (the forge, fishing-bobber zone, aura generators, …) // carry a *transparent* placeholder M2, and invisible **trigger creatures** an empty or // constant-zero-alpha one: the real client's mesh gate is **type-independent** — it draws // any loaded model and the per-batch zero-alpha cull skips the transparent geometry // (decision 0024, superseding 0023's wrong marker-type gate; verified wow-re go-render-gate). - // Our M2 alpha cull already reduces those models to zero submeshes, so `model` is `None` - // here and they render nothing — neither kind needs a special case, and `named_a_model` - // above is what keeps the unit arm's debug cube off them (1403). + // Our M2 alpha cull already reduces those models to zero submeshes, so `parts` below is + // EMPTY for them: the spawn loop draws nothing, and `named_a_model` is what keeps the unit + // arm's debug cube off them (1403). if let Some(parts) = model { // ── Mounts (decision 0441): a mounted unit is TWO skeletons. The mount is a child // entity carrying a plain creature `NetEntity` — this very system builds it like any @@ -437,6 +463,11 @@ pub(super) fn attach_entity_visuals( && stores .get(entity) .is_ok_and(|s| crate::go_anim::go_animates(s.0.gameobject_type_id())); + // Will anything actually SKIN through this instance's palette slot? Every M2 part + // carries a skinned twin, so for an ordinary body this is simply "it drew something"; + // a body that drew NOTHING (decision 1618's invisible weapon-holder) gets the rig + // without the slot — see [`setup_skinned_instance`]. + let skins = parts.iter().any(|p| p.skinned_mesh.is_some()); let mut skin: Option = match (net.kind, dm) { // A creature (or player body — decision 0041) with a real skeleton. The `!is_empty` // guard keeps a degenerate boneless model on the static mesh (its skinned twin would @@ -472,7 +503,14 @@ pub(super) fn attach_entity_visuals( commands.entity(entity).add_child(node); joints_root = node; } - setup_skinned_instance(&mut commands, &mut palettes, entity, joints_root, d) + setup_skinned_instance( + &mut commands, + &mut palettes, + entity, + joints_root, + d, + skins, + ) } // A GameObject whose model authors a real skeleton + animation draws through the // skinned twin like a creature. Two flavours share the rig: a door/button/chest @@ -501,7 +539,14 @@ pub(super) fn attach_entity_visuals( .is_some_and(|a| a.clips.iter().any(|c| c.poses_bones)); ((state_machine && poses) || ambient) .then(|| { - setup_skinned_instance(&mut commands, &mut palettes, entity, entity, d) + setup_skinned_instance( + &mut commands, + &mut palettes, + entity, + entity, + d, + skins, + ) }) .flatten() } @@ -1019,3 +1064,84 @@ fn display_label(handle: &ModelHandle) -> String { path.map(|p| p.path().to_string_lossy().into_owned()) .unwrap_or_default() } + +#[cfg(test)] +mod tests { + use super::*; + use benilla_assets::{ModelJoint, ModelSkeleton}; + use bevy::ecs::world::CommandQueue; + use bevy::mesh::skinning::SkinnedMeshInverseBindposes; + + /// A one-bone rest skeleton + its bindposes — the smallest rig `setup_skinned_instance` will + /// build, so the test is about the slot policy and nothing else. + fn one_bone(ibps: &mut Assets) -> DisplayModel { + DisplayModel { + skeleton: ModelSkeleton { + joints: vec![ModelJoint { + parent: -1, + local_translation: Vec3::ZERO, + billboard: None, + parent_arm: None, + }], + spine_bone: None, + head_bone: None, + }, + inverse_bindposes: Some( + ibps.add(SkinnedMeshInverseBindposes::from(vec![Mat4::IDENTITY])), + ), + ..super::super::display::empty_shell() + } + } + + /// **A body with nothing to skin gets the rig without the palette slot** (decision 1656). + /// + /// The invisible trigger creature holding a visible weapon is a complete model instance — it + /// needs the pose, because its attachment points carry what IS drawn — and has no geometry of + /// its own for a palette slot to serve. It must also take **no `RigStarved`**: that marker + /// means "currently slot-less and wants one", and `heal_rig_starved` would rebuild this body + /// every time the table gained headroom, for ever, chasing a slot it never asks for. + #[test] + fn a_body_that_skins_nothing_takes_the_rig_without_a_slot() { + use benilla_world::rig_palette::{RigPalettes, RigSkin, RigStarved}; + + let mut app = App::new(); + app.init_resource::() + .init_resource::>(); + let mut ibps = app + .world_mut() + .remove_resource::>() + .unwrap(); + let d = one_bone(&mut ibps); + + for (skins, want_slot) in [(false, false), (true, true)] { + let mut world = std::mem::take(app.world_mut()); + let entity = world.spawn_empty().id(); + let mut palettes = world.remove_resource::().unwrap(); + let build = { + let mut queue = CommandQueue::default(); + let mut commands = Commands::new(&mut queue, &world); + let build = + setup_skinned_instance(&mut commands, &mut palettes, entity, entity, &d, skins); + queue.apply(&mut world); + build + }; + world.insert_resource(palettes); + assert!( + build.is_some(), + "the pose is built either way (skins={skins})" + ); + let e = world.entity(entity); + assert_eq!( + e.contains::(), + want_slot, + "skins={skins} ⇒ palette slot {want_slot}" + ); + assert!( + !e.contains::(), + "skins={skins}: a table with room never starves — and a body that skins nothing \ + must never ask to be healed" + ); + *app.world_mut() = world; + } + } +} diff --git a/crates/benilla-app/src/entities/chain_beam.rs b/crates/benilla-app/src/entities/chain_beam.rs index 3298a791..288855ad 100644 --- a/crates/benilla-app/src/entities/chain_beam.rs +++ b/crates/benilla-app/src/entities/chain_beam.rs @@ -39,10 +39,19 @@ //! the `+2` is a floor, so even a one-yard hop bends. //! - each interior point takes an independent 3-vector in `[−1, 1]³` scaled by `len × noiseScale`, //! **re-rolled every frame**, and the live polyline advects toward it `0.75` old / `0.25` new. -//! - the ribbon's cross-section is the **world-horizontal** perpendicular `(−d.y, d.x, 0)` in WoW -//! axes — [`Vec3::Y.cross`] here, the same vector — normalized only when it is longer than -//! `0.001`, so a hop pointing straight up (or straight at a camera looking down it) collapses to -//! nothing. That is the reference's own regime, not an artefact. +//! - the ribbon's cross-section is `(−d.y, d.x, 0)` taken **in EYE space**, not world space: the +//! strip is built from points already run through the world→eye transform (`0x7bca80` against +//! `0xcf5800 = T(−cameraPos)·VIEW`, assembled at `0x7affb1`–`0x7b00c4`; `VIEW` is the device's +//! slot-10 matrix, whose producer `0x50ab70` is `LookAt(eye = origin, …)` — **pure rotation, zero +//! translation row**, which is why the pre-translate is not a double translate). Eye axes are X +//! right, Y up, **+Z into the screen**, so the formula is the *screen*-plane 90° rotation and the +//! ribbon is **view-plane aligned**. In world axes it is `normalize(d × camForward)` — the same +//! vector, and the same magnitude, so the reference's `0.001` guard transfers unchanged. The +//! transform is an isometry with no projection (`0x7bca80` is row-vector affine, no w-divide) and +//! both the WORLD and VIEW slots are identity at the submit, so the strip is drawn in eye space +//! and the width stays **`2 × field2` world yards at any distance**. Decision 1653 corrects 0964, +//! which read the formula as world-space and drew every beam as a horizontal slab — a hairline +//! from any camera near the beam's own plane. //! - an interior vertex sits at `p ± 0.5·(perp₍ᵢ₋₁₎ + perpᵢ)·halfWidth`; **both ends collapse to a //! point** with `v = 0.5` — the beam is a spindle, tapered at caster and target. //! - `u` runs `0 → 1` caster→target, translated by `−(phase / period)` where @@ -90,8 +99,16 @@ const CHAIN_ATTACH_FALLBACK: u16 = 34; /// The subdivision floor's constant (`fadd [0x801628] = 2.0f` @ `0x7af716`): `n = trunc(len/avg + 2)`. const SUBDIVISION_FLOOR: f32 = 2.0; -/// The normalisation guard on the cross-section vector (`fcom [0x801360]` @ `0x7b01bd`): a -/// perpendicular shorter than this is left **un-normalized**, collapsing that segment's width. +/// The normalisation guard on the cross-section vector (`fcom [0x801360]` @ `0x7b01bd`, strictly +/// `>`): a perpendicular shorter than this is left **un-normalized**, collapsing that segment's +/// width. The length it measures is the segment's *screen*-plane projection, so what it guards is a +/// hop aimed at the camera — not one aimed at the sky, which is what 0964 believed. +/// +/// It is a **numerical guard, not a visible behaviour**: because everything above it is normalized +/// to full width, the pinch cone is `arcsin(0.001 / |d|)` — about `0.023°` for the ~2.5 yd +/// sub-segments a 30 yd link subdivides into, and `noiseScale`'s per-frame re-roll walks the +/// segment out of it. A beam does **not** thin as you turn to look down its length; it holds its +/// width and then, at an angle no player can hold, degenerates instead of producing a NaN. const PERP_EPSILON: f32 = 0.001; /// The per-frame advection weight on the live polyline (`0x7afc10`/`0x7afc38`): @@ -373,20 +390,36 @@ fn subdivide(a: Vec3, b: Vec3, effect: &ChainEffect, rng: &mut u32, out: &mut Ve /// One strand's polyline, written into the shared stream as a triangle strip expressed in quads /// (the ribbon lane's conversion: strip triangles `(t₀,b₀,t₁),(b₀,b₁,t₁)` = quad `[b₀,b₁,t₁,t₀]`). /// -/// Cross-section, ends and texcoords are the reference's (`0x7b0196`–`0x7b0541`): the world- -/// horizontal perpendicular, un-normalized below [`PERP_EPSILON`]; an interior vertex at +/// Cross-section, ends and texcoords are the reference's (`0x7b0196`–`0x7b0541`): the **view-plane +/// aligned** perpendicular, un-normalized below [`PERP_EPSILON`]; an interior vertex at /// `p ± 0.5·(perp₍ᵢ₋₁₎ + perpᵢ)·halfWidth`; both ends collapsed to a point at `v = 0.5`; `u` /// running 0→1 caster→target plus the scroll translate. The colour is the beam's constant /// `0xFFFFFFFF` — emissive white, never tinted. -fn push_strand(verts: &mut Vec, pts: &[Vec3], half_width: f32, u_scroll: f32) { +/// +/// `cam_forward` is the world camera's unit forward axis (into the screen) — **one axis for the +/// whole strand**, not a per-point eye ray: the reference's only use of the camera position is the +/// single pre-translate that builds the world→eye matrix (`0x7affb1`), after which every point +/// shares one linear map. So the ribbon lies in the view plane and keeps its authored world width +/// from every angle. +fn push_strand( + verts: &mut Vec, + pts: &[Vec3], + half_width: f32, + u_scroll: f32, + cam_forward: Vec3, +) { let count = pts.len(); if count < 2 { return; } let seg_perp = |i: usize| -> Vec3 { - // `(−d.y, d.x, 0)` in WoW axes IS `Y × d` in ours — same vector, same magnitude, so the - // reference's 0.001 guard on the raw length transfers unchanged. - let raw = Vec3::Y.cross(pts[i + 1] - pts[i]); + // `(−d.y, d.x, 0)` on the EYE-space delta is `d × camForward` in world axes: same vector, + // same magnitude — `|d|` projected onto the screen plane — so the reference's 0.001 guard + // on the raw length transfers unchanged. The order is `d × F`, not `F × d`: the two differ + // by a sign, which mirrors `v` across the beam's centreline, and this is the one that puts + // the `v = 0` rail on the same side the reference does (eye axes X right / Y up / +Z into + // the screen — a beam running rightwards across the screen carries `v = 0` on top). + let raw = (pts[i + 1] - pts[i]).cross(cam_forward); if raw.length() > PERP_EPSILON { raw.normalize() } else { @@ -436,16 +469,20 @@ pub(crate) fn simulate_chain_beams( mut commands: Commands, mut draw: benilla_world::particles::buffer::WorldEffectDraw, images: Res>, - world_cam: Query>, + world_cam: Query<(Entity, &GlobalTransform), With>, units: super::missile::AttachPosQuery, joints: Query<&GlobalTransform>, heights: Query<&OverheadFallback>, mut beams: Query<(Entity, &mut ChainBeam)>, mut scratch: Local>, ) { - let Ok(cam) = world_cam.single() else { + let Ok((cam, cam_xf)) = world_cam.single() else { return; }; + // The reference's world→eye map is rebuilt from the live camera every `CLightning::Render` + // (`0x7aff82`–`0x7b00c4`), so the cross-section follows the camera frame by frame; one axis + // serves the whole beam. + let cam_forward = *cam_xf.forward(); let dt = time.delta_secs().min(0.1); let now = time.elapsed_secs(); for (entity, mut beam) in &mut beams { @@ -517,7 +554,7 @@ pub(crate) fn simulate_chain_beams( pts.clear(); pts.extend_from_slice(&scratch); } - push_strand(batch.verts_mut(), pts, half_width, u_scroll); + push_strand(batch.verts_mut(), pts, half_width, u_scroll, cam_forward); } } if anchor_n == 0.0 { @@ -535,7 +572,7 @@ pub(crate) fn simulate_chain_beams( } /// The beam's arithmetic, against the numbers wow-re read out of the binary — the subdivision floor, -/// the spindle taper, the world-horizontal cross-section and its degenerate regime, and the signed +/// the spindle taper, the view-plane cross-section and its degenerate regime, and the signed /// scroll. Each of these is a value that would go straight into pixels, and two of the four columns /// they read were mis-named in the community schemas until the §5 (decision 0955). #[cfg(test)] @@ -688,8 +725,9 @@ mod tests { assert_eq!(moved, out.len() - 2, "every interior point takes a draw"); } - /// The ribbon spans `2 × halfWidth` on its **world-horizontal** cross-section, and **both ends - /// collapse to a point** — the beam is a spindle, not a slab (`0x7b04b9`–`0x7b0541`). + /// The ribbon spans `2 × halfWidth` across its cross-section, that cross-section is square to + /// the **camera** (perpendicular to the view axis, not to world up), and **both ends collapse + /// to a point** — the beam is a spindle, not a slab (`0x7b0196`–`0x7b0541`). #[test] fn the_ribbon_is_a_spindle_two_half_widths_across() { let pts = [ @@ -697,8 +735,10 @@ mod tests { Vec3::new(0.0, 0.0, -5.0), Vec3::new(0.0, 0.0, -10.0), ]; + // A camera off to the side, looking along −X at a hop that runs along −Z. + let cam_forward = -Vec3::X; let mut verts = Vec::new(); - push_strand(&mut verts, &pts, 0.5, 0.0); + push_strand(&mut verts, &pts, 0.5, 0.0, cam_forward); // 2 segments × one quad each. assert_eq!(verts.len(), 8); // Quad corner order is [b₀, b₁, t₁, t₀]: the first quad's b₀/t₀ are the collapsed caster @@ -707,36 +747,98 @@ mod tests { assert_eq!(verts[3].pos, pts[0].to_array()); assert_eq!(verts[5].pos, pts[2].to_array()); assert_eq!(verts[6].pos, pts[2].to_array()); - // The middle point carries the full width, across the horizontal — the hop runs along −Z, - // so its cross-section is ±X. + // The middle point carries the full width… let (mid_bottom, mid_top) = (Vec3::from(verts[1].pos), Vec3::from(verts[2].pos)); assert!( (mid_top.distance(mid_bottom) - 1.0).abs() < 1e-5, "2 × 0.5 yd" ); - assert!( - mid_top.y.abs() < 1e-6 && mid_bottom.y.abs() < 1e-6, - "horizontal" - ); + // …spread square to the view axis, which for this camera means straight up/down. + let across = (mid_top - mid_bottom).normalize(); + assert!(across.dot(cam_forward).abs() < 1e-6, "square to the camera"); + assert!(across.x.abs() < 1e-6 && across.z.abs() < 1e-6, "±Y here"); // …and the collapsed ends stamp v = 0.5 on both rails. assert_eq!(verts[0].uv[1], 0.5); assert_eq!(verts[3].uv[1], 0.5); } - /// A hop pointing straight up has **no** horizontal perpendicular: the reference skips the - /// normalisation below `0.001` (`0x7b01bd`) and the segment collapses toward zero width. That - /// regime — a beam vanishing when seen end-on — is the reference's, not a bug of ours. + /// **The defect decision 1653 fixes.** A long, level hop watched from a camera in its own + /// plane — the Razorgore-room beam, and Chain Lightning's mob-to-mob jumps — kept its full + /// authored width in the reference and collapsed to a hairline for us, because 0964 spread the + /// ribbon across *world* horizontal instead of the *screen* plane. Any camera axis must give + /// the same `2 × halfWidth` span, and it must never lie along the view direction. #[test] - fn a_vertical_hop_collapses_instead_of_exploding() { + fn a_level_hop_keeps_its_width_from_a_level_camera() { + let pts = [Vec3::ZERO, Vec3::X * 15.0, Vec3::X * 30.0]; + for cam_forward in [ + Vec3::NEG_Z, // level, square on + Vec3::new(0.0, -0.2, -1.0), // the player's slightly-above-eye camera + Vec3::new(-0.3, -0.05, -1.0), // level and off to one side + Vec3::NEG_Y, // straight down + ] { + let cam_forward = cam_forward.normalize(); + let mut verts = Vec::new(); + push_strand(&mut verts, &pts, 0.5, 0.0, cam_forward); + let (mid_bottom, mid_top) = (Vec3::from(verts[1].pos), Vec3::from(verts[2].pos)); + let across = mid_top - mid_bottom; + assert!( + (across.length() - 1.0).abs() < 1e-5, + "{cam_forward}: spans 2 × 0.5 yd, not {}", + across.length() + ); + assert!( + across.normalize().dot(cam_forward).abs() < 1e-5, + "{cam_forward}: the width never runs into the screen" + ); + } + } + + /// The `v = 0` rail sits where the reference puts it. Eye axes are X right, Y up, `+Z` into + /// the screen, and the perp is `(−d.y, d.x, 0)`, so a beam running **rightwards** across the + /// screen carries `v = 0` along its **top** edge. In world axes that is `d × camForward`; the + /// opposite order is the same line mirrored, which would flip the texture across the beam's + /// centreline. + #[test] + fn the_v_zero_rail_is_the_top_edge_of_a_rightward_beam() { + // Camera at the origin looking along −Z with +Y up: a hop along +X runs left→right. + let pts = [Vec3::ZERO, Vec3::X * 5.0, Vec3::X * 10.0]; + let mut verts = Vec::new(); + push_strand(&mut verts, &pts, 0.5, 0.0, Vec3::NEG_Z); + // Quad corner order is [b₀, b₁, t₁, t₀]; index 2 is the mid point's `t` rail. + let (mid_bottom, mid_top) = (Vec3::from(verts[1].pos), Vec3::from(verts[2].pos)); + assert_eq!(verts[2].uv[1], 0.0, "the t rail is v = 0"); + assert!( + mid_top.y > mid_bottom.y, + "…and it is the upper edge on screen" + ); + } + + /// A hop pointing **at the camera** has no screen-plane extent, and the reference skips the + /// normalisation below `0.001` (`0x7b01bd`), so the segment degenerates instead of producing a + /// NaN. This is a **numerical guard, not a behaviour you can see**: the cone is + /// `arcsin(0.001/|d|)` ≈ `0.023°` on a real sub-segment, and every angle outside it is + /// normalized to full width. A hop pointing straight *up* is not in the regime at all — that + /// was 0964's world-space misreading, and it drew a vertical beam as nothing. + #[test] + fn a_hop_aimed_at_the_camera_collapses_instead_of_exploding() { let pts = [Vec3::ZERO, Vec3::Y * 5.0, Vec3::Y * 10.0]; let mut verts = Vec::new(); - push_strand(&mut verts, &pts, 0.5, 0.0); + push_strand(&mut verts, &pts, 0.5, 0.0, Vec3::NEG_Y); let (mid_bottom, mid_top) = (Vec3::from(verts[1].pos), Vec3::from(verts[2].pos)); assert!( mid_top.distance(mid_bottom) < 1e-6, - "a vertical hop has no width, and certainly no NaN" + "a hop seen end-on has no width, and certainly no NaN" ); assert!(verts.iter().all(|v| v.pos.iter().all(|c| c.is_finite()))); + + // …and the same hop, from a camera beside it, is at full width. + verts.clear(); + push_strand(&mut verts, &pts, 0.5, 0.0, Vec3::NEG_Z); + let (mid_bottom, mid_top) = (Vec3::from(verts[1].pos), Vec3::from(verts[2].pos)); + assert!( + (mid_top.distance(mid_bottom) - 1.0).abs() < 1e-5, + "2 × 0.5 yd" + ); } /// `u` runs 0 → 1 caster→target (`0x7afb20`), translated by the scroll (`0x7b057e`). @@ -744,11 +846,11 @@ mod tests { fn u_runs_caster_to_target_and_the_scroll_translates_it() { let pts = [Vec3::ZERO, Vec3::X * 5.0, Vec3::X * 10.0]; let mut verts = Vec::new(); - push_strand(&mut verts, &pts, 0.5, 0.0); + push_strand(&mut verts, &pts, 0.5, 0.0, Vec3::NEG_Z); assert_eq!(verts[0].uv[0], 0.0, "caster end"); assert_eq!(verts[5].uv[0], 1.0, "target end"); verts.clear(); - push_strand(&mut verts, &pts, 0.5, -0.25); + push_strand(&mut verts, &pts, 0.5, -0.25, Vec3::NEG_Z); assert_eq!( verts[0].uv[0], -0.25, "the whole run slides with the scroll" diff --git a/crates/benilla-app/src/net.rs b/crates/benilla-app/src/net.rs index 4e4a23a8..67c9b35e 100644 --- a/crates/benilla-app/src/net.rs +++ b/crates/benilla-app/src/net.rs @@ -37,9 +37,12 @@ use apply::{apply_net_updates, tag_self_player}; // and are pulled in here for the plugin + the event bridge. pub(crate) use io::LoginRequest; use motion::{ - drain_pending_moves, drive_display_facing, extrapolate_remote_units, ground_clamp_creatures, - mark_swimming_creatures, sample_splines, + drain_pending_moves, extrapolate_remote_units, ground_clamp_creatures, mark_swimming_creatures, + sample_splines, }; +// The client-local facing turn. `pub(crate)` because the shuffle latch it produces is read in +// `creature_anim`, and the cross-seam test that pins the pair runs both systems in one app. +pub(crate) use motion::drive_display_facing; pub(crate) use motion::{ jump_seed, CreatureSwimming, FacingStep, RemoteMotion, Spline, SplineStopped, }; diff --git a/crates/benilla-app/src/net/motion.rs b/crates/benilla-app/src/net/motion.rs index 230973e8..7c13bcc6 100644 --- a/crates/benilla-app/src/net/motion.rs +++ b/crates/benilla-app/src/net/motion.rs @@ -29,9 +29,10 @@ mod spline; #[cfg(test)] mod tests; +pub(crate) use facing::drive_display_facing; +pub(in crate::net) use facing::resolve_facing; pub(in crate::net) use facing::DisplayFacing; pub(crate) use facing::FacingStep; -pub(super) use facing::{drive_display_facing, resolve_facing}; pub(crate) use relay::{PendingMove, RelayMove}; pub(crate) use remote::jump_seed; pub(crate) use remote::RemoteMotion; diff --git a/crates/benilla-app/src/net/motion/facing.rs b/crates/benilla-app/src/net/motion/facing.rs index a6e0f424..043f4f9d 100644 --- a/crates/benilla-app/src/net/motion/facing.rs +++ b/crates/benilla-app/src/net/motion/facing.rs @@ -27,12 +27,20 @@ use bevy::prelude::*; use super::super::{ActiveMover, GuidIndex, NetEntity, ObjectStore, SelfPlayer}; use super::{yaw_of, RemoteMotion, Spline}; -/// The remaining yaw error (radians) under which a facing ease counts as **settled** — the -/// [`FacingStep`] latch drops and the anim layer's turn-shuffle releases back to Stand. The -/// client's own small/large-delta thresholds (`0x80c5c4`/`0x80c5c8`, decision 0123) aren't -/// transcribed; this is an eyeballable stand-in (~3°). Shared with the remote facing interp +/// The turn-shuffle latch's **sign dead-band** — the client's `[0x80c5c8]` = +1e-5 / +/// `[0x80c5c4]` = −1e-5 (`0x60843b`–`0x608473`), tested on the yaw **this pump actually applied**. +/// Above it the frame latches `+0xd58` bit `0x800` (→ ShuffleLeft 11), below its negative bit +/// `0x1000` (→ ShuffleRight 12), and between the two nothing latches: it is a *did the body move +/// at all* test, not a "close enough" one. Shared with the remote facing interp /// ([`super::remote`]) — the same latch drives a standing remote's mouse-turn shuffle. -pub(super) const FACING_SETTLED: f32 = 0.05; +/// +/// **This replaces `FACING_SETTLED = 0.05` (~3°), which was wrong twice over** (decision 1655). +/// 0123 recorded `0x80c5c4`/`0x80c5c8` as "the client's small/large-delta thresholds" and left an +/// "eyeballable stand-in" in their place; re-read at the bytes they are ±1e-5, a symmetric pair +/// about zero, and the two bits they gate are *left/right*, not small/large. Sixty times too wide, +/// and applied to the wrong quantity, the stand-in dropped the latch about half-way through every +/// turn — which is most of why a vendor turned to face you with frozen feet. +pub(super) const TURN_LATCH_BAND: f32 = 1.0e-5; /// The filter's **dead-band** (`[0x8029d0]` = 0.01 rad ≈ 0.57°), tested on the *unfolded* goal−current /// delta and **inclusive** (`fcomp` + `jp` at `600eb5`–`600ec0`). Inside it the goal is taken @@ -70,12 +78,16 @@ pub(crate) struct DisplayFacing { hist: [f32; 4], } -/// A stationary unit's display facing is still **stepping** this frame: the signed remaining delta -/// (WoW yaw, positive = counterclockwise = turning left). The anim layer folds it into the unit's -/// turn view — the client's facing-delta shuffle latch (`0x607ed0` bits `0x800`/`0x1000`, wow-re +/// A stationary unit's display facing **moved this frame**: the signed yaw the pump applied (WoW +/// yaw, positive = counterclockwise = turning left). The anim layer folds it into the unit's turn +/// view — the client's facing-delta shuffle latch (`0x607ed0` bits `0x800`/`0x1000`, wow-re /// `loop-replay-fidget.md` §5b; decision 0123) — so a squaring-up creature foot-shuffles instead of -/// pivoting frozen, and each shuffle's return to Stand re-rolls the idle variation. Removed the -/// frame the ease settles. +/// pivoting frozen, and each shuffle's return to Stand re-rolls the idle variation. +/// +/// **The applied step, not the remaining gap** (decision 1655): the client's latch reads +/// `0x607ed0`'s accumulator — the increment it writes to `+0xc94` at `608239` — against +/// [`TURN_LATCH_BAND`]. The two differ at the tail of a per-frame-halving ease, which is exactly +/// where a shuffle is still meant to be playing. Removed the frame the body stops moving. #[derive(Component)] pub(crate) struct FacingStep(pub(crate) f32); @@ -211,8 +223,12 @@ fn filter_step(cur: f32, goal: f32, hist: &mut [f32; 4]) -> f32 { /// **No range, line-of-sight, faction or is-alive test appears anywhere on this path** — the /// `CanInteract` family is ruled out at the client's call graph. The only leash is the window's own /// range close, which [`crate::ui_session::close_npc_session_out_of_range`] already owns. +/// +/// `pub(crate)` rather than `pub(in crate::net)` for one reason: the shuffle latch's *consumer* +/// lives in `creature_anim`, and the only test that can catch the latch being produced but never +/// read has to run both systems in one app (`creature_anim::driver::tests`). #[allow(clippy::type_complexity, clippy::too_many_arguments)] // one Bevy system's full input set -pub(in crate::net) fn drive_display_facing( +pub(crate) fn drive_display_facing( mut commands: Commands, index: Res, // `Option>` throughout: a headless test mounts this system without the UI plugins. @@ -315,11 +331,13 @@ pub(in crate::net) fn drive_display_facing( if tf.rotation != rot { tf.rotation = rot; } - // The turn-shuffle latch (decision 0123): while the ease still has meaningful yaw to - // cover, the anim layer sees a signed turning step; settled drops it below. - let remaining = crate::creature_anim::wrap_pi(goal - new); - if remaining.abs() > FACING_SETTLED { - commands.entity(e).insert(FacingStep(remaining)); + // The turn-shuffle latch (decision 0123, corrected by 1650): the client tests the yaw + // this pump APPLIED (`0x607ed0`'s `param_2`, the increment written to `+0xc94`) against + // the symmetric ±[`TURN_LATCH_BAND`] sign band — not the gap still to cover, and not a + // "close enough" threshold. Folded because a pump may cross the wrap. + let step = crate::creature_anim::wrap_pi(new - cur); + if step.abs() > TURN_LATCH_BAND { + commands.entity(e).insert(FacingStep(step)); stepping.insert(e); } trace_face(e, source, goal, cur, new); @@ -525,6 +543,93 @@ mod tests { ); } + /// The turn-shuffle latch's own law (decision 1655): [`FacingStep`] carries **the yaw this + /// pump applied** — the client's `0x607ed0` accumulator, the increment it writes to `+0xc94` — + /// tested against the symmetric ±[`TURN_LATCH_BAND`] sign band (`[0x80c5c8]`/`[0x80c5c4]`). + /// + /// The regression this fences is the pair of substitutions 0123 made: the *remaining* gap in + /// place of the applied step, and an eyeballed ~3° in place of ±1e-5. Because the ease halves + /// its error every pump, "3° still to go" arrives while the body is still visibly moving, so + /// the latch released about half-way through every turn and the foot-shuffle never blended in. + /// The middle assertion is that exact frame: the latch is still held while the gap left is + /// under the old threshold. + #[test] + fn the_latch_carries_the_step_applied_not_the_gap_remaining() { + let mut app = App::new(); + app.init_resource::() + .init_resource::() + .add_systems(Update, drive_display_facing); + // Us off the vendor's axis: the bearing is +2.601 rad, reached counterclockwise, so every + // step is positive — the left turn, and ShuffleLeft (11) downstream. + app.world_mut().spawn(( + SelfPlayer, + ActiveMover, + Transform::from_translation(wow_to_bevy([0.0, 3.0, 0.0])), + )); + let npc = app + .world_mut() + .spawn(( + NetEntity { + kind: EntityKind::Unit, + display_id: None, + scale: 1.0, + }, + ObjectStore::default(), + Transform { + translation: wow_to_bevy([5.0, 0.0, 0.0]), + rotation: Quat::from_rotation_y(0.0), + ..default() + }, + )) + .id(); + let yaw = |app: &App| yaw_of(app.world().entity(npc).get::().unwrap().rotation); + let step = |app: &App| app.world().entity(npc).get::().map(|f| f.0); + + app.update(); // the seeding frame + app.world_mut() + .resource_mut::() + .0 = Some(npc); + + let goal = 3.0f32.atan2(-5.0); + let (mut latched_inside_the_old_band, mut pumps) = (false, 0); + for _ in 0..24 { + let before = yaw(&app); + app.update(); + let after = yaw(&app); + let applied = crate::creature_anim::wrap_pi(after - before); + match step(&app) { + Some(s) => { + pumps += 1; + assert!( + (s - applied).abs() < 1.0e-6, + "the latch carries the applied step: {s} vs {applied}" + ); + assert!(s > 0.0, "a counterclockwise turn latches LEFT: {s}"); + if crate::creature_anim::wrap_pi(goal - after).abs() < 0.05 { + latched_inside_the_old_band = true; + } + } + // Nothing moved this frame, so nothing may be latched. + None => assert!( + applied.abs() <= TURN_LATCH_BAND, + "unlatched, but the body moved {applied}" + ), + } + } + assert!( + (9..=11).contains(&pumps), + "this bearing closes in ten pumps, got {pumps}" + ); + assert!( + latched_inside_the_old_band, + "the latch outlives the old ~3° stand-in — that gap IS the missing half of the shuffle" + ); + assert!( + !app.world().entity(npc).contains::(), + "and drops once the body stops moving" + ); + } + /// Row 7 sits above row 9: a vendor that is *fighting* keeps facing its attacker, not us. #[test] fn a_target_outranks_the_interaction_face_me() { diff --git a/crates/benilla-app/src/net/motion/remote.rs b/crates/benilla-app/src/net/motion/remote.rs index ae8070e0..bfd1b936 100644 --- a/crates/benilla-app/src/net/motion/remote.rs +++ b/crates/benilla-app/src/net/motion/remote.rs @@ -607,22 +607,26 @@ pub(in crate::net) fn extrapolate_remote_units( // `0x800/0x1000` → `0x712090`, anims 11/12 — not the movement interp, whose integrator // doesn't run flag-less; our display-facing layer models the chase with this latch, the // same confirmed outcome. A keyboard turner needs none of it — its TURN flags pick the - // shuffle already.) Dropped the frame the yaw settles (the apply snaps the remainder). - let step = rm.pending.front().map_or(0.0, |ev| { - let grounded_still = rm.flags - & (move_flags::ANY_MOVE - | move_flags::TURN_LEFT - | move_flags::TURN_RIGHT - | move_flags::FALLING - | move_flags::SWIMMING) - == 0; - if grounded_still { - wrap_pi(ev.mv.orientation - orientation) - } else { - 0.0 - } - }); - if step.abs() > super::facing::FACING_SETTLED { + // shuffle already.) Dropped the frame the body stops moving. + // + // **The yaw this frame APPLIED, against the ±1e-5 sign band** — the client's own latch + // input and its own band (`0x607ed0` `60843b`–`608473`; decision 1655), not the gap still + // to cover measured against an eyeballed ~3°. `facing_lerp` above is what moved it; with + // nothing queued it does not move at all, so the `pending` test the old form needed is + // carried by the quantity itself. + let grounded_still = rm.flags + & (move_flags::ANY_MOVE + | move_flags::TURN_LEFT + | move_flags::TURN_RIGHT + | move_flags::FALLING + | move_flags::SWIMMING) + == 0; + let step = if grounded_still { + wrap_pi(orientation - rm.orientation) + } else { + 0.0 + }; + if step.abs() > super::facing::TURN_LATCH_BAND { commands.entity(e).insert(super::FacingStep(step)); } else if latched { commands.entity(e).remove::(); diff --git a/crates/benilla-app/src/transport.rs b/crates/benilla-app/src/transport.rs index a96806cb..4b8d0df1 100644 --- a/crates/benilla-app/src/transport.rs +++ b/crates/benilla-app/src/transport.rs @@ -115,6 +115,13 @@ enum Drive { Lift(Lift), } +/// **No map field, deliberately** (decision 1654). A lift's map is not a datum it owns: vmangos +/// hands a player the whole map's transport set on entry (`Map::SendInitTransports`) and takes it +/// back on the way out, so a type-11 is only ever resident for someone standing on its own map — +/// the car is always on the map *you* are on. 0611 stamped `CurrentMap` here at arm time, which in +/// the login frame is the startup seed (`arm_transports` runs a stage before `player::wire_in` +/// writes the real map), and the off-map hide below then hid every lift on Kalimdor for the whole +/// session — the director's Thunder Bluff report, and B168's Gnomeregan lift. struct Lift { frames: Arc>, period_ms: u32, @@ -122,12 +129,19 @@ struct Lift { quat: [f32; 4], /// Constant rendered heading (wire orientation, WoW radians). yaw: f32, - /// The map the car lives on — a lift never leaves it (the worldport-spare predicate). - map: u32, } impl Transport { - fn period_ms(&self) -> u32 { + /// Which drive is underneath — for the instruments only (`WOW_LIFT_CENSUS`); no consumer + /// keys on it (decision 0438 §5: the drive is private by design). + pub(crate) fn drive_label(&self) -> &'static str { + match &self.drive { + Drive::Taxi(_) => "taxi", + Drive::Lift(_) => "lift", + } + } + + pub(crate) fn period_ms(&self) -> u32 { match &self.drive { Drive::Taxi(t) => t.period_ms, Drive::Lift(l) => l.period_ms, @@ -142,22 +156,29 @@ impl Transport { } /// Whether any leg of this transport's cycle lies on `map_id` — the cross-map worldport's - /// spare predicate (decision 0455). A lift lives (and dies) on its one map. + /// spare predicate (decision 0455). **A lift always answers no** (1654): the spare exists so a + /// *ridden boat* survives the seam mid-cycle, and a lift never spans one — the server re-sends + /// the destination map's whole transport set on arrival (`SendInitTransports`), so a lift + /// despawns and streams back like every other object. pub(crate) fn touches_map(&self, map_id: u32) -> bool { match &self.drive { Drive::Taxi(t) => t.touches_map(map_id), - Drive::Lift(l) => l.map == map_id, + Drive::Lift(_) => false, } } - /// The cycle sample — position/heading/map/motion at `cycle_ms`, both drives. - fn sample(&self, cycle_ms: u32) -> TransportSample { + /// The cycle sample — position/heading/map/motion at `cycle_ms`, both drives. `here` is the + /// map the viewer is on: a **taxi** ignores it (its timetable knows which continent each leg + /// lies on, which is the whole point of the off-map hide), and a **lift** reports it, because + /// a type-11 exists only for players standing on its own map. That is the honest answer to + /// "which map is this position expressed in", and it is what [`Lift`] refuses to store. + fn sample(&self, cycle_ms: u32, here: u32) -> TransportSample { match &self.drive { Drive::Taxi(t) => t.sample(cycle_ms), Drive::Lift(l) => { let (pos, moving) = elevator_sample(&l.frames, l.base_pos, l.quat, cycle_ms); TransportSample { - map: l.map, + map: here, pos, heading: l.yaw, moving, @@ -168,8 +189,8 @@ impl Transport { /// The live sample for an anchor — the same pose the tick writes this frame. For /// instruments (the crossing probe reads dock state + the deck drop point off it). - pub(crate) fn sample_at(&self, anchor: &TransportAnchor) -> TransportSample { - self.sample(self.cycle_ms(anchor)) + pub(crate) fn sample_at(&self, anchor: &TransportAnchor, here: u32) -> TransportSample { + self.sample(self.cycle_ms(anchor), here) } } @@ -215,12 +236,15 @@ fn setup_taxi_nodes(mut timetables: ResMut, world_assets: Option, templates: Res, - current_map: Option>, // `Or<…, With>`: a re-created lift is already armed (`Transport` present) but // carries a fresh seed from the new create block — it re-enters here so the seed is consumed // (a fresh, identical arm) instead of lingering. Boats never carry a seed, so for them the @@ -265,9 +289,6 @@ fn arm_transports( .vis_chain_only(); continue; }; - let Some(current_map) = current_map.as_ref() else { - continue; // between worlds — next frame - }; let frames = lifts .entry(seed.entry) .or_insert_with(|| Arc::new(frames.to_vec())) @@ -291,7 +312,6 @@ fn arm_transports( base_pos: seed.base_pos, quat, yaw: seed.yaw, - map: current_map.0, }), was_moving: false, }, @@ -353,6 +373,8 @@ fn arm_transports( /// rotation only — the renderer bakes model scale into the transform (`write_pose`'s law). /// Off-map samples (the boat is sailing the other continent's leg) hide the model; the server /// removes the GO around the same time, so this is belt-and-braces for the transition frames. +/// **A lift can never trip it** (decision 1654): its sample is expressed in the map passed in, so +/// the term is a boat's alone — which is what it always meant, and what 0611's stamped map broke. #[allow(clippy::type_complexity)] // one Bevy system's full input set fn tick_transports( current_map: Option>, @@ -373,7 +395,7 @@ fn tick_transports( &mut transports { let cycle = transport.cycle_ms(anchor); - let sample = transport.sample(cycle); + let sample = transport.sample(cycle, current_map.0); if sample.moving != transport.was_moving { transport.was_moving = sample.moving; debug!( @@ -388,6 +410,7 @@ fn tick_transports( // identical pose each frame, and the unconditional writes this replaces re-dirtied the // deck's whole subtree — transform propagation over every submesh plus a `Visibility` // re-propagation — for every docked elevator on the map, every frame. + // Boats only, by construction — see this system's doc. if sample.map != current_map.0 { visibility.set_if_neq(Visibility::Hidden); continue; @@ -474,6 +497,26 @@ mod tests { use super::*; use benilla_formats::ElevatorKeyframe; + /// A two-frame pause path at `base_pos` — the same lift both tests below drive. + fn parked_lift() -> Lift { + Lift { + frames: Arc::new(vec![ + ElevatorKeyframe { + time_ms: 0, + pos: [0.0; 3], + }, + ElevatorKeyframe { + time_ms: 1000, + pos: [0.0; 3], + }, + ]), + period_ms: 1000, + base_pos: [10.0, 20.0, 30.0], + quat: [0.0, 0.0, 0.0, 1.0], + yaw: 0.5, + } + } + /// A docked lift goes QUIET: `tick_transports` writes pose and visibility on change only /// (1473) — the unconditional writes it replaces re-dirtied every docked elevator's whole /// subtree (transform + `Visibility` propagation over all its submeshes), map-wide, every @@ -501,23 +544,7 @@ mod tests { app.world_mut().spawn(( Guid(0xF110_0000_0000_0001), Transport { - drive: Drive::Lift(Lift { - frames: Arc::new(vec![ - ElevatorKeyframe { - time_ms: 0, - pos: [0.0; 3], - }, - ElevatorKeyframe { - time_ms: 1000, - pos: [0.0; 3], - }, - ]), - period_ms: 1000, - base_pos: [10.0, 20.0, 30.0], - quat: [0.0, 0.0, 0.0, 1.0], - yaw: 0.5, - map: 0, - }), + drive: Drive::Lift(parked_lift()), was_moving: false, }, TransportAnchor { @@ -538,4 +565,48 @@ mod tests { "a docked lift must stop dirtying its transform and visibility" ); } + + /// **A lift is never hidden by the off-map term, whatever `CurrentMap` says** (decision 1654). + /// The off-map hide is the boat's: a ferry mid-cycle on the other continent's leg. 0611 gave + /// the lift a `map` stamped from `CurrentMap` at arm time — and `arm_transports` runs a stage + /// *before* `player::wire_in` writes that resource, so at login every type-11 on the map armed + /// against the startup seed and the term hid all of them for the session (Thunder Bluff; B168). + /// Driving the tick on a map the arm never saw is exactly that shape, and the car must show. + #[test] + fn a_lift_is_visible_on_whatever_map_the_viewer_is_on() { + let mut app = App::new(); + // Alterac Valley — a real 1.12 map that carries no type-11 GO at all, so it is a map no + // lift could ever have been stamped with. The point is that the value cannot matter. + app.insert_resource(CurrentMap(30)) + .add_systems(Update, tick_transports); + let lift = app + .world_mut() + .spawn(( + Guid(0xF110_0000_0000_0002), + Transport { + drive: Drive::Lift(parked_lift()), + was_moving: false, + }, + TransportAnchor { + progress_ms: 0, + at: Instant::now(), + }, + Transform::default(), + // As the net apply spawns an anchored transport: hidden until its first ticked + // pose. If the tick declines to unhide it, the car is solid and invisible. + Visibility::Hidden, + )) + .id(); + app.update(); + assert_eq!( + app.world().get::(lift), + Some(&Visibility::Inherited), + "the tick must unhide a lift on the viewer's map, not judge it against a stamped one" + ); + // And the pose it wrote is the car's own — base + its (zero) local offset, in Bevy axes. + assert_eq!( + app.world().get::(lift).map(|t| t.translation), + Some(benilla_assets::coords::wow_to_bevy([10.0, 20.0, 30.0])), + ); + } } diff --git a/crates/benilla-formats/examples/wmo_ownerless_pools.rs b/crates/benilla-formats/examples/wmo_ownerless_pools.rs new file mode 100644 index 00000000..92daac53 --- /dev/null +++ b/crates/benilla-formats/examples/wmo_ownerless_pools.rs @@ -0,0 +1,311 @@ +//! **Which buildings' embedded pools have no visibility authority**, over the whole shipped corpus: +//! `cargo run -p benilla-formats --example wmo_ownerless_pools`. +//! +//! A placed WMO gets a `WmoPortalInstance` only when it has a portal graph **or** an authored +//! `WMOAreaTable` identity — `has_portals || m.wmo_id != 0`, the spawn site in +//! `benilla-world/src/terrain_stream/spawn/mod.rs`. Everything about a building's *rooms* hangs off +//! that instance: it is the per-placement entity the group-visible set lives on, and it is what a +//! pool's `WmoGroupVis`/`WmoRoom` scope is keyed to. +//! +//! So a root that has **neither** portals **nor** a wmoID is instance-less — and until decision 1652 +//! that meant its embedded MLIQ pools were tagged with *neither* `WmoGroupVis` nor `ExteriorScene`, +//! i.e. no system ever wrote their `Visibility` at all, while the same placement's walls were +//! tagged `ExteriorScene` unconditionally and window-culled normally. A sealed well room drawing a +//! disc of water in mid-air is what that *would* look like from outside. +//! +//! Whether any shipped root actually falls in that cell is the question — an unreachable branch and +//! a widespread defect are the same code, and only the corpus tells them apart. +//! +//! **The answer is zero** (run 2026-08-27, decision 1652): of 815 roots, 448 carry no portal graph +//! and exactly **one** carries `wmoID == 0` — `pvp_alterac_ent01.wmo` — and that one *does* have +//! portals. The two conditions never coincide in shipped data, so the instance-less branch is dead +//! and 1652's unconditional tag on pools closes a hole no building reaches. That is worth a census +//! rather than a guess in either direction: it is the difference between a fix that earns a sighting +//! and one that must never be credited with one. +//! +//! Two facts per root, both read from the bytes the runtime reads: +//! +//! * **instance** — `MOPT` and `MOPR` both non-empty (`benilla_assets::WmoModel::portal_infos` / +//! `portal_refs`), or `MOHD.wmoID @0x20 != 0` ([`benilla_formats::wmo_root_id`], the +//! `WMOAreaTable.WMOID` key). +//! * **liquid** — any group file for which [`benilla_formats::wmo_group_liquid_mesh`] yields a +//! surface. That is the *same* call `WmoModel::group_liquids` makes, so a root counted wet here is +//! exactly a root that spawns pool entities at placement. (`resolve_shared_liquid_cells` can drop +//! one group's mesh afterwards, but only in favour of a sibling group of the same root, so it +//! cannot change a root's wet/dry verdict.) A group whose MOGP `groupLiquid` @`0x34` declares +//! whole-group submersion but carries no `MLIQ` grid (13 in the archive, decision 1000) is **not** +//! counted: it spawns no pool entity, so there is nothing for an instance to own. The liquid-group +//! total cross-checks against `wmo_liquid_arms`, which reaches the same groups from the other side +//! — the listfile's `_NNN` group files rather than `MOHD.nGroups` per root. +//! +//! Roots are also split by whether the world actually **places** them — an unplaced WMO can never be +//! stood next to, so it cannot show the defect. Placements come from every ADT's `MODF` plus the +//! WDT global-WMO of the 20 WMO-only maps (decision 0688 — scanning only ADTs would call the Deeprun +//! Tram and every instance-shaped dungeon unplaced). +//! +//! Output is Blizzard data — pipe it to the scratchpad, never into the repo. + +use std::collections::{BTreeMap, BTreeSet}; +use std::io::Cursor; + +use benilla_formats::{ + parse_wmo_portals, parse_wmo_root, wmo_group_liquid_mesh, wmo_root_id, Chain, +}; + +/// What one root says about the two axes. +struct Root { + /// `MOHD.wmoID @0x20` — the `WMOAreaTable.WMOID` key; `0` ⇒ the building has no authored identity. + wmo_id: u32, + /// `MOPT` ∧ `MOPR` both non-empty — the spawn site's `has_portals`. + has_portals: bool, + /// Group files the root declares (`MOHD.nGroups`). + groups: u32, + /// Groups whose MLIQ resolves to a liquid surface. + liquid_groups: u32, + /// Wet cells across those surfaces — the size of what draws. + wet_tiles: u32, + /// Maps the root is placed on (empty ⇒ never placed). + maps: BTreeSet, +} + +impl Root { + /// The spawn site's test, verbatim: no instance ⇒ nothing owns this building's rooms. + fn has_instance(&self) -> bool { + self.has_portals || self.wmo_id != 0 + } +} + +/// Normalize a chain path the way the MPQ hash compares them, so ADT `MWMO` names and listfile +/// names land on the same key. +fn key(name: &str) -> String { + name.replace('/', "\\").to_ascii_lowercase() +} + +/// Every WMO root the world actually places, mapped to the maps it appears on: ADT `MODF` first, +/// then the WDT global-WMO that IS the world on a WMO-only map (decision 0688). +fn placed_roots(chain: &Chain) -> anyhow::Result>> { + let mut placed: BTreeMap> = BTreeMap::new(); + // `World\Maps\\_XX_YY.adt` — the map name is the third path component. + let map_of = |name: &str| key(name).split('\\').nth(2).unwrap_or("?").to_string(); + + let names: Vec = chain.list()?.into_iter().map(|e| e.name).collect(); + let adts: Vec<&String> = names.iter().filter(|n| key(n).ends_with(".adt")).collect(); + eprintln!("scanning {} ADTs for MODF placements…", adts.len()); + for name in adts { + let Ok(bytes) = chain.read(name) else { + continue; + }; + let Ok(benilla_adt::ParsedAdt::Root(adt)) = + benilla_adt::parse_adt(&mut Cursor::new(&*bytes)) + else { + continue; + }; + let map = map_of(name); + for p in &adt.wmo_placements { + let Some(model) = adt.wmos.get(p.name_id as usize) else { + continue; + }; + placed.entry(key(model)).or_default().insert(map.clone()); + } + } + + let wdts: Vec<&String> = names.iter().filter(|n| key(n).ends_with(".wdt")).collect(); + eprintln!("scanning {} WDTs for WMO-only maps…", wdts.len()); + for name in wdts { + let Ok(bytes) = chain.read(name) else { + continue; + }; + let Ok(wdt) = + benilla_wdt::WdtReader::new(Cursor::new(&*bytes), benilla_wdt::WowVersion::Classic) + .read() + else { + continue; + }; + let Some(g) = wdt.global_wmo() else { continue }; + placed + .entry(key(&g.model)) + .or_default() + .insert(map_of(name)); + } + Ok(placed) +} + +fn main() -> anyhow::Result<()> { + let data = benilla_formats::wow_data() + .ok_or_else(|| anyhow::anyhow!("no 1.12.1 install found (set $WOW_DATA)"))?; + let chain = Chain::open(&data)?; + let placed = placed_roots(&chain)?; + eprintln!("{} distinct WMO roots placed in the world", placed.len()); + + // A group file's stem ends `_NNN`; a root's does not. The chain lists both. + // + // The listfile is NOT a complete index (`Chain::list`'s own doc: a file absent from every + // archive's listfile is readable by name but not enumerated), so the scanned set is the UNION of + // the listed roots and every root an ADT/WDT actually references. A placed-but-unlisted root is + // exactly the one this census must not miss, and the two counts printed below say whether the + // union ever added anything. + let listed: BTreeSet = chain + .list()? + .into_iter() + .map(|e| key(&e.name)) + .filter(|k| { + let Some(stem) = k.strip_suffix(".wmo") else { + return false; + }; + !stem + .rsplit('_') + .next() + .is_some_and(|t| t.len() == 3 && t.bytes().all(|b| b.is_ascii_digit())) + }) + .collect(); + let unlisted: Vec<&String> = placed.keys().filter(|k| !listed.contains(*k)).collect(); + eprintln!( + "reading {} listed WMO roots + {} placed-but-unlisted…", + listed.len(), + unlisted.len() + ); + let roots: Vec = listed + .iter() + .chain(unlisted) + .cloned() + .collect::>() + .into_iter() + .collect(); + + let mut census: BTreeMap = BTreeMap::new(); + for k in &roots { + let Ok(bytes) = chain.read(k) else { + continue; + }; + // `parse_wmo_root` is the group-count authority (`MOHD.nGroups`); a file that fails it is + // not a root and has no rooms to own. + let Ok(root) = parse_wmo_root(&bytes) else { + continue; + }; + let portals = parse_wmo_portals(&bytes); + let stem = k.strip_suffix(".wmo").unwrap_or(k).to_string(); + let mut row = Root { + wmo_id: wmo_root_id(&bytes), + has_portals: !portals.refs.is_empty() && !portals.infos.is_empty(), + groups: root.group_count(), + liquid_groups: 0, + wet_tiles: 0, + maps: placed.get(k).cloned().unwrap_or_default(), + }; + for gi in 0..row.groups { + let Ok(gbytes) = chain.read(&format!("{stem}_{gi:03}.wmo")) else { + continue; + }; + let Some(mesh) = wmo_group_liquid_mesh(&gbytes) else { + continue; + }; + row.liquid_groups += 1; + row.wet_tiles += mesh.wet.iter().filter(|w| **w).count() as u32; + } + census.insert(k.clone(), row); + } + + // ---- the cross-tab --------------------------------------------------------------------- + let mut cell = [[0u32; 2]; 2]; // [instance][liquid] + for r in census.values() { + cell[usize::from(r.has_instance())][usize::from(r.liquid_groups > 0)] += 1; + } + println!( + "WMO roots in the 1.12.1 chain: {} parsed of {} found\n", + census.len(), + roots.len() + ); + println!( + "{:<34} {:>10} {:>11} {:>8}", + "", "no liquid", "has liquid", "total" + ); + for (i, label) in [ + "NO INSTANCE (portal-less + wmoID 0)", + "instance (portals or wmoID)", + ] + .iter() + .enumerate() + { + let row = cell[i]; // index 0 = no instance, matching `usize::from(has_instance())` + println!( + "{label:<34} {:>10} {:>11} {:>8}", + row[0], + row[1], + row[0] + row[1] + ); + } + let (dry, wet) = (cell[0][0] + cell[1][0], cell[0][1] + cell[1][1]); + println!("{:<34} {dry:>10} {wet:>11} {:>8}", "total", dry + wet); + + let wet_placed = census + .values() + .filter(|r| r.liquid_groups > 0 && !r.maps.is_empty()) + .count(); + let liquid_groups: u32 = census.values().map(|r| r.liquid_groups).sum(); + + // The two axes on their own. The cross-tab cannot say WHICH half of `has_portals || wmo_id != 0` + // carries a root, and "no root is instance-less" is a claim that has to be falsifiable from the + // same run rather than taken on the strength of one collapsed cell. + let portal_less = census.values().filter(|r| !r.has_portals).count(); + let unnamed = census.values().filter(|r| r.wmo_id == 0).count(); + println!( + "\naxes: {portal_less} roots have NO portal graph; {unnamed} roots have wmoID 0; \ +{} are BOTH (portal-less AND unnamed)", + cell[0][0] + cell[0][1] + ); + println!( + " {wet} roots carry MLIQ liquid ({wet_placed} of them placed), \ +{liquid_groups} liquid groups in all" + ); + + // Every root on the rarer axis, named: `wmoID == 0` is what makes the instance depend on the + // portal graph at all, so the handful that have it are the whole falsifiable surface of the + // cross-tab's empty cell. + println!("\nroots with wmoID 0:"); + for (path, r) in census.iter().filter(|(_, r)| r.wmo_id == 0) { + println!( + " {path} portals {} liquid groups {} {}", + if r.has_portals { "YES" } else { "no" }, + r.liquid_groups, + if r.maps.is_empty() { + "-- never placed --".to_string() + } else { + r.maps.iter().cloned().collect::>().join(", ") + } + ); + } + + // ---- the affected set ------------------------------------------------------------------ + let affected: Vec<(&String, &Root)> = census + .iter() + .filter(|(_, r)| !r.has_instance() && r.liquid_groups > 0) + .collect(); + let placed_count = affected.iter().filter(|(_, r)| !r.maps.is_empty()).count(); + println!( + "\nownerless-with-liquid roots: {} ({placed_count} placed in the world)\n", + affected.len() + ); + if affected.is_empty() { + println!(" (none — no shipped root falls in that cell)"); + return Ok(()); + } + println!( + "{:<52} {:>6} {:>7} {:>6} placed on", + "root", "groups", "wet grp", "cells" + ); + for (path, r) in &affected { + println!( + "{:<52} {:>6} {:>7} {:>6} {}", + path, + r.groups, + r.liquid_groups, + r.wet_tiles, + if r.maps.is_empty() { + "-- never placed --".to_string() + } else { + r.maps.iter().cloned().collect::>().join(", ") + } + ); + } + Ok(()) +} diff --git a/crates/benilla-formats/tests/chain.rs b/crates/benilla-formats/tests/chain.rs index 98359352..aaf7e8dc 100644 --- a/crates/benilla-formats/tests/chain.rs +++ b/crates/benilla-formats/tests/chain.rs @@ -126,6 +126,50 @@ fn trigger_creature_models_carry_no_render_geometry() { ); } +#[test] +fn the_trigger_creature_still_carries_the_attachments_its_visible_self_rides() { + let data = benilla_formats::wow_data_or_skip!(); + let mut chain = open_chain(&data).expect("open vanilla patch chain"); + + // Decision 1656: a trigger creature draws nothing *and is not therefore empty*. The + // Naxxramas weapon mobs (`Unholy Axe`/`Swords`/`Staff`, creature_template 16194/16215/16216, + // all display 15294) ARE an `InvisibleStalker` body — the model above, `nVertices == 0` — + // whose entire visible self hangs off its attachment points: the weapon on HandRight, the + // name plate on PlayerName. If either of these ever stops resolving, the mobs go back to + // being a bare name on the floor, and nothing in the render lane would say so. + let bytes = chain + .read_file("Creature\\InvisibleStalker\\InvisibleStalker.m2") + .expect("read InvisibleStalker.m2"); + let attach = benilla_formats::parse_m2_attachments(&bytes).expect("parse attachments"); + let at = |id: u16| attach.iter().find(|a| a.id == id).copied(); + + let hand = at(1).expect("HandRight (1) — the drawn mainhand, where the axe hangs"); + assert!( + hand.position[2] > 0.5, + "the hand attachment sits on the body, not at the origin: {:?}", + hand.position + ); + let name = at(18).expect("PlayerName (18) — `0x608640`'s overhead anchor"); + assert!( + name.position[2] > 2.0, + "the name anchor is overhead, not at the feet: {:?}", + name.position + ); + + // And the bounds really are degenerate, which is why the plate CANNOT come from the fallback + // (`feet + scale × bbox_z × 1.25` = feet) and must come from the attachment above. + let bounds = benilla_formats::load_m2_bounds( + &mut chain, + "Creature\\InvisibleStalker\\InvisibleStalker.mdx", + ) + .expect("bounds"); + assert_eq!( + bounds.bbox_max[2] - bounds.bbox_min[2], + 0.0, + "a vertex-less model has no vertex box — the overhead fallback is feet-height here" + ); +} + #[test] fn suppresses_white1_invisible_trap_placeholder() { let data = benilla_formats::wow_data_or_skip!(); diff --git a/crates/benilla-world/src/exterior_cull.rs b/crates/benilla-world/src/exterior_cull.rs index 862b8452..60b1f6a7 100644 --- a/crates/benilla-world/src/exterior_cull.rs +++ b/crates/benilla-world/src/exterior_cull.rs @@ -80,9 +80,24 @@ //! ## What is gated so far, and what is knowingly not //! //! **Gated:** ADT terrain (`0x683bf0`), ADT doodad placements (`0x683700`), the WDL far band -//! (`0x683040`) and now the net **bodies** on `0x683340` ([`WorldUnit::bound`]) — the buckets that produce -//! the reported symptoms, and the ones whose entities have no other `Visibility` writer, so this is -//! their sole authority (decision 0025). +//! (`0x683040`), the net **bodies** on `0x683340` ([`WorldUnit::bound`]), and open-world **liquid** +//! (`0x683ab0`) — the buckets whose entities have no other `Visibility` writer, so this is their +//! sole authority (decision 0025). World **WMO placements** (`0x6856c0`), their props and their +//! MLIQ pools are gated too, but by the *model-visibility* authority, which folds [`ExteriorGate`] +//! into its own AND rather than being written here (decision 0784). +//! +//! **Liquid was the last of them** (decision 1652). 0774 deferred it and 0784 deferred it again, +//! both on the same stated ground — that it "already has a visibility authority… its own lane". It +//! did not. An ADT surface is spawned as a world root with `Mesh3d`, and nothing ever wrote its +//! `Visibility`; the only writer in the whole subsystem was the `$WOW_NO_LIQUID` kill-switch, +//! env-gated and `Added`-filtered. So the fix was not the "real design step" both records +//! predicted — it was the tag, and this system was already the right owner. The lesson is 0784's +//! own, one turn later: *"it already has an authority" needs a check, not a memory.* 0774 made that +//! claim about two buckets at once; it was true of WMO placements and false of liquid, and because +//! the two travelled in one sentence, nobody separated them. 0784 checked the half it was closing, +//! found the claim inverted there, and copied the other half forward untested. What that cost is +//! the director's report — from inside a cavern, the lake overhead drawing straight through the +//! ceiling. //! //! **The unit is the drawn object, and for terrain that is the 33.333 yd MCNK cell, not the 533 yd //! tile** (decision 0780). This is a property of the *spawner*, not of anything here — but the cull is @@ -90,12 +105,8 @@ //! window and is admitted whichever way the doorway faces. The cull was correct and looked broken. //! The far band keeps its whole-tile box, which is the reference's own far-tier granularity. //! -//! **NOT yet gated, deliberately:** world **WMO placements** (`0x6856c0`) and open-world **liquid** -//! (`0x683ab0`). Both already have a visibility authority — WMO group submeshes are written by the -//! portal PVS apply, liquid by its own lane — and bolting a second writer onto the same entities is -//! precisely the two-authorities bug 0025 forbids. Folding them in means teaching *those* authorities -//! to consume [`ExteriorWindows`], which is a real design step and not a line to sneak in here. Until -//! then a distant building or lake still shows through a wall where the reference would hide it. +//! **NOT gated, knowingly:** the horizon-occlusion term (`0x686000`) — its own machinery, its +//! own record. Nothing in the exterior *buckets* is left ungated. //! //! **Deviation to be honest about:** the reference tests one AABB per *object*; a doodad placement has //! no root entity in our graph, so we tag and test each **submesh**. Their union is the object, so the @@ -156,6 +167,14 @@ pub struct ExteriorCullVerdict { /// couple of dozen. Summed together, a body leg that reaches nothing at all would be invisible. pub(crate) bodies: usize, pub(crate) bodies_hidden: usize, + /// Open-world liquid surfaces the cull reached, and how many the windows rejected — a + /// **subset** of `tested`/`hidden` above, reported separately for the reason the body leg is + /// (decision 1652). A tile carries a handful of MCNK liquid layers against ~250 terrain cells, + /// so a liquid leg that reached *nothing* would move `tested` by less than its own noise. This + /// is the number that says "the lake is being decided", and before 1652 it was structurally + /// zero. + pub(crate) liquid: usize, + pub(crate) liquid_hidden: usize, } pub(crate) struct ExteriorCullPlugin; @@ -285,6 +304,9 @@ type UnownedScene = ( &'static GlobalTransform, Option<&'static Aabb>, &'static mut Visibility, + // Instrument only — never a term in the verdict. Liquid takes exactly the same test as a + // terrain cell; this only lets the counters say so (see [`ExteriorCullVerdict::liquid`]). + Has, ); /// …and the objects it is allowed to write: exterior scene that nothing else owns. `ModelPart` and @@ -416,11 +438,14 @@ fn apply_exterior_cull( .is_none_or(|f| f.intersects_obb(bound, >.affine(), true, true)) }; let (mut tested, mut hidden, mut unbounded) = (0usize, 0usize, 0usize); - for (gt, aabb, mut vis) in &mut scene { + let (mut liquid, mut liquid_hidden) = (0usize, 0usize); + for (gt, aabb, mut vis, is_liquid) in &mut scene { tested += 1; unbounded += usize::from(aabb.is_none()); let admitted = gate.admits(gt, aabb); hidden += usize::from(!admitted); + liquid += usize::from(is_liquid); + liquid_hidden += usize::from(is_liquid && !admitted); set( &mut vis, if admitted { @@ -501,6 +526,8 @@ fn apply_exterior_cull( unbounded, bodies: body_n, bodies_hidden: body_hidden, + liquid, + liquid_hidden, }; } @@ -901,4 +928,171 @@ mod tests { ); assert_eq!((verdict.bodies, verdict.bodies_hidden), (2, 1)); } + + /// One `apply_exterior_cull` run over a scene of **liquid surfaces** — the ADT spawn's own + /// shape: a world root at `IDENTITY` (MCLQ positions are absolute) carrying a world-space + /// `Aabb`, the `LiquidSurface` marker and the `ExteriorScene` tag. + fn run_liquid( + windows: ExteriorWindows, + surfaces: &[Aabb], + ) -> (Vec, ExteriorCullVerdict) { + let mut app = App::new(); + app.init_resource::() + .insert_resource(Assets::::default()) + .insert_resource(windows) + .add_systems(Update, apply_exterior_cull); + app.world_mut().spawn(( + WorldCamera, + GlobalTransform::IDENTITY, + Projection::Perspective(PerspectiveProjection { + fov: std::f32::consts::FRAC_PI_2, + aspect_ratio: 1.0, + near: 0.1, + far: 1000.0, + ..default() + }), + )); + let ids: Vec = surfaces + .iter() + .map(|bound| { + app.world_mut() + .spawn(( + GlobalTransform::IDENTITY, + *bound, + ExteriorScene, + crate::liquid::LiquidSurface, + Visibility::Inherited, + )) + .id() + }) + .collect(); + app.update(); + let vis = ids + .iter() + .map(|e| *app.world().entity(*e).get::().unwrap()) + .collect(); + (vis, *app.world().resource::()) + } + + /// One MCNK liquid layer's world box, centred at `at` — a flat 33.333 yd sheet, which is the + /// granularity `spawn_liquids` actually produces (one entity per `LiquidMesh`, and an ADT chunk + /// carries its liquid per chunk). Flat on purpose: a lake is a sheet, and the whole question is + /// whether a sheet *overhead* reaches the doorway's part of the view. + fn lake(at: Vec3) -> Aabb { + const CELL: f32 = 33.333 / 2.0; + Aabb::from_min_max( + at - Vec3::new(CELL, 0.05, CELL), + at + Vec3::new(CELL, 0.05, CELL), + ) + } + + /// **The director's report, as a test** (decision 1652): from inside a cavern you could see the + /// lake above through the ceiling. Sealed room ⇒ no windows ⇒ the reference's per-window + /// exterior populate never runs, and its ADT liquid producer `0x683ab0` is reachable from + /// nowhere else — so the lake is not submitted at all. + /// + /// Before 1652 this asserted nothing, because an ADT surface carried no `ExteriorScene` and the + /// cull never reached it. That is what `liquid`/`liquid_hidden` exist to make visible: the + /// counters are the difference between "the cull admitted the lake" and "the cull never saw + /// it", which from a screenshot are the same picture. + #[test] + fn a_sealed_room_hides_the_lake_overhead() { + let overhead = Vec3::new(0.0, 30.0, -60.0); + let (vis, verdict) = run_liquid(ExteriorWindows::Windows(Vec::new()), &[lake(overhead)]); + assert_eq!( + vis, + vec![Visibility::Hidden], + "a sealed cavern draws no exterior liquid — the ceiling is not a depth test" + ); + assert_eq!( + (verdict.liquid, verdict.liquid_hidden), + (1, 1), + "and the cull must have REACHED it — (0, 0) here is the pre-1652 defect, which \ + leaves the surface `Inherited` and looks identical on screen to being admitted" + ); + } + + /// The control that must not change: **outdoors nothing about liquid moves.** The driver's + /// outside leg is one full-screen window (`0x6811ca`), so the gate is `Open` and every surface + /// goes back to whatever else owns it — which, for liquid, is nothing. If this ever fails, the + /// lake has started disappearing in the open world, which is a far worse bug than the one 1652 + /// fixed. + #[test] + fn outdoors_every_lake_is_admitted() { + let (vis, verdict) = run_liquid( + ExteriorWindows::Unrestricted, + &[ + lake(Vec3::new(0.0, 30.0, -60.0)), + lake(Vec3::new(0.0, -2.0, 400.0)), // behind the eye: still not this system's call + ], + ); + assert_eq!(vis, vec![Visibility::Inherited; 2]); + assert_eq!((verdict.liquid, verdict.liquid_hidden), (2, 0)); + } + + /// A doorway shows the water on its own side of the view and no other — the same law the + /// terrain and body legs take, on the bucket that was missing it. Both sheets sit at the same + /// height and depth; the only difference is which way the doorway faces. + #[test] + fn a_doorway_admits_only_the_water_on_its_own_side() { + let (vis, verdict) = run_liquid( + ExteriorWindows::Windows(vec![[0.1, -1.0, 1.0, 1.0]]), + &[ + lake(Vec3::new(60.0, 10.0, -60.0)), + lake(Vec3::new(-60.0, 10.0, -60.0)), + ], + ); + assert_eq!( + vis, + vec![Visibility::Inherited, Visibility::Hidden], + "the window is on the right" + ); + assert_eq!((verdict.liquid, verdict.liquid_hidden), (2, 1)); + } + + /// **The failure mode this change could have had, pinned:** the cavern's OWN pool must not be + /// blanked by the sealed room it sits in. A WMO pool carries `WmoGroupVis` (0689), so it is + /// owned by the model-visibility authority — which exempts the camera's own placement and folds + /// the window term in itself (0784) — and this system's `Without` filter must keep + /// its hands off it entirely, tag or no tag. + /// + /// Asserting on `liquid` is the point: the counter proves the surface was not merely *admitted* + /// but never walked, which is the difference between the two authorities agreeing and one of + /// them silently overwriting the other. + #[test] + fn a_wmo_pool_is_never_written_by_this_system() { + let mut app = App::new(); + app.init_resource::() + .insert_resource(Assets::::default()) + // The sealed room — the one case that would blank it. + .insert_resource(ExteriorWindows::Windows(Vec::new())) + .add_systems(Update, apply_exterior_cull); + let instance = app.world_mut().spawn(()).id(); + let pool = app + .world_mut() + .spawn(( + GlobalTransform::IDENTITY, + lake(Vec3::new(0.0, 0.0, -10.0)), + ExteriorScene, + crate::liquid::LiquidSurface, + crate::wmo_portal::WmoGroupVis { + instance, + groups: std::sync::Arc::from([0u16].as_slice()), + }, + Visibility::Inherited, + )) + .id(); + app.update(); + assert_eq!( + *app.world().entity(pool).get::().unwrap(), + Visibility::Inherited, + "the pool in the room you are standing in belongs to the model-visibility authority" + ); + let verdict = *app.world().resource::(); + assert_eq!( + (verdict.tested, verdict.liquid), + (0, 0), + "`Without` must exclude it from the walk, not just from the verdict" + ); + } } diff --git a/crates/benilla-world/src/liquid/mod.rs b/crates/benilla-world/src/liquid/mod.rs index 961a688b..83c67825 100644 --- a/crates/benilla-world/src/liquid/mod.rs +++ b/crates/benilla-world/src/liquid/mod.rs @@ -84,16 +84,22 @@ mod spatial; mod surface; // the against-real-client-files tests — they span both halves // The submodules are private, so this list IS the subsystem's face: everything the rest of the -// client may name. `LiquidSurface` is deliberately absent — it is spawned, never named from -// outside; add it here the day something needs it. Most of this list is now reached through -// `crate::world_point::WorldPoint` rather than directly (decision 1164). +// client may name. Most of this list is now reached through `crate::world_point::WorldPoint` +// rather than directly (decision 1164). +// +// `LiquidSurface` used to be deliberately absent — "spawned, never named from outside; add it here +// the day something needs it". Decision 1652 is that day: the exterior-window cull counts liquid +// apart from the rest of the scene, because a few dozen surfaces summed into tens of thousands of +// terrain cells is a leg that could reach nothing at all and never show it. pub use query::{ camera_claim, describe_at, liquid_at, player_claim, surfaces_at, unit_claim, water_surface_at, FoamPatch, LiquidClaim, LiquidHit, LiquidSource, RoomPlacements, Underwater, WaterChunkInfo, WmoPool, }; pub(crate) use spatial::{maintain_water_index, WaterIndex}; -pub(crate) use surface::{spawn_liquids, spawn_wmo_liquids, LiquidAssets, LiquidSoundSource}; +pub(crate) use surface::{ + spawn_liquids, spawn_wmo_liquids, LiquidAssets, LiquidSoundSource, LiquidSurface, +}; /// The frame slot where [`Underwater`] is written — the label every consumer of the submersion /// verdict orders itself `.after(..)`. The submerged view is a whole-screen swap (atmosphere, clear @@ -134,9 +140,19 @@ impl Plugin for LiquidPlugin { ) // The surface-render kill-switch (see [`hide_liquid_surfaces`]) — inert without the env // var, so it costs nothing when it isn't being used. + // + // `PostUpdate`, after the two systems that now own a liquid surface's `Visibility` + // every frame — the exterior-window cull (ADT surfaces, decision 1652) and the + // model-visibility authority (WMO pools, 0689/0784) — and before Bevy consumes the + // result. An *override* that runs last, deliberately, rather than a second writer + // trying to compose with them (decision 0025's law, and 0784's reasoning for why + // ordering is the wrong tool for two real terms but the right one for a kill-switch: + // there is nothing to AND here, the switch simply wins). .add_systems( - Update, + PostUpdate, surface::hide_liquid_surfaces + .after(crate::exterior_cull::ExteriorCullSet) + .before(bevy::camera::visibility::VisibilitySystems::VisibilityPropagate) .run_if(|| std::env::var_os("WOW_NO_LIQUID").is_some()), ); } diff --git a/crates/benilla-world/src/liquid/surface.rs b/crates/benilla-world/src/liquid/surface.rs index 354ae497..dd2b8b92 100644 --- a/crates/benilla-world/src/liquid/surface.rs +++ b/crates/benilla-world/src/liquid/surface.rs @@ -185,11 +185,27 @@ pub(crate) struct LiquidSurface; /// wall, and both look like a hard straight seam across the scene. One A/B now separates them; the /// alternative was a screenshot argument. Same shape as `$WOW_NO_PARTICLES` / `$WOW_NO_FFX`. /// -/// Runs on `Added` so it costs one filtered query per frame and catches streamed-in surfaces too -/// (both spawn paths insert [`LiquidSurface`], MCLQ and WMO alike). -pub(super) fn hide_liquid_surfaces(mut surfaces: Query<&mut Visibility, Added>) { +/// **An ordered override, not a second authority** — the shape `apply_self_model_fade` already +/// uses against the model-`Visibility` authority (`model_render::ModelVisSet`). Every liquid +/// surface now has a real per-frame `Visibility` owner: an ADT surface is `ExteriorScene`, written +/// by `exterior_cull::apply_exterior_cull`; a WMO pool carries `WmoGroupVis`, written by +/// `model_render::visibility::apply_model_visibility`'s `group_only` query. So this runs in +/// `PostUpdate` **after** both (decision 1652) and wins the frame. +/// +/// It used to run in `Update` on `Added`, writing `Hidden` exactly once when a surface streamed in. +/// That was already broken for WMO pools before this change — 0689 gave them `WmoGroupVis` in +/// `Update`, so the authority re-wrote `Inherited` over the kill-switch on the very next frame and +/// `$WOW_NO_LIQUID` silently stopped hiding Stormwind's canals and every dungeon pool. Nobody +/// noticed because the switch's usual subject is a lake. Tagging ADT liquid would have taken the +/// remaining half the same way; making it an override fixes both at once. +/// +/// Change-gated, so the steady state is one compare per surface and no change-detection churn — +/// and the system is `run_if`'d off the env var, so an ordinary run never schedules it at all. +pub(super) fn hide_liquid_surfaces(mut surfaces: Query<&mut Visibility, With>) { for mut vis in &mut surfaces { - *vis = Visibility::Hidden; + if *vis != Visibility::Hidden { + *vis = Visibility::Hidden; + } } } @@ -247,6 +263,22 @@ pub(crate) fn spawn_liquids<'a>( MeshMaterial3d(material), Transform::IDENTITY, LiquidSurface, + // **Open-world liquid is exterior scene** — the reference's ADT liquid + // producer `0x683ab0` is called only from the per-window populate + // `0x682fa0`, exactly like ADT terrain (`0x683bf0`) and doodads + // (`0x683700`), so from inside a cavern the lake overhead draws only where a + // doorway window admits it, and not at all from a sealed room (decision + // 1652). This was 0774's last knowingly-ungated bucket, deferred again by + // 0784 on the belief that liquid "has its own lane" — it does not: nothing + // wrote an ADT surface's `Visibility` at all, so `apply_exterior_cull` is its + // sole authority and the tag is the whole change. + // + // Granularity is already the reference's: one entity per MCNK liquid layer + // (33.333 yd), never the 533 yd tile — the half of the cull that decision + // 0780 is about. The `IDENTITY` transform is not incidental either: MCLQ + // positions are absolute, so the mesh `Aabb` Bevy derives is already the + // world-space box the window test wants. + crate::exterior_cull::ExteriorScene, info, LiquidSoundSource { nibble: lq.sound_nibble, @@ -633,6 +665,100 @@ mod tests { ); } } + + /// One 2×2-vertex MCLQ sheet at `at` — the smallest thing `spawn_liquids` will actually build. + fn one_sheet(at: [f32; 3]) -> LiquidMesh { + let [x, y, z] = at; + LiquidMesh { + grid: [2, 2], + wet: vec![true], + shared: vec![false], + positions: vec![ + [x, y, z], + [x + 8.0, y, z], + [x, y + 8.0, z], + [x + 8.0, y + 8.0, z], + ], + uvs: vec![[0.0, 0.0]; 4], + depths: vec![1.0; 4], + indices: vec![0, 1, 2, 1, 3, 2], + sound_nibble: 0, + material_id: None, + kind: LiquidKind::Still, + } + } + + /// A [`LiquidAssets`] whose only entry is the one the ADT arm asks for — `spawn_liquids` + /// `continue`s past a kind whose frames failed to load, so without this the spawn is a no-op + /// and the test below would pass on an empty world. + fn adt_assets() -> LiquidAssets { + LiquidAssets { + materials: HashMap::from([( + LiquidKey { + kind: LiquidKind::Still, + path: LiquidPath::Adt, + scroll: false, + }, + LiquidEntry { + material: Handle::default(), + }, + )]), + } + } + + /// **The spawner's half of the indoor water cull** (decision 1652) — and the half no test in + /// `exterior_cull` can cover, because that module's harness spawns its own entities and tags + /// them itself. `apply_exterior_cull` decides nothing about a surface it never queries, and + /// what puts a surface in its query is this tag, applied here. 0780 left exactly this note on + /// the terrain cull ("nothing in `apply_exterior_cull` can fix that — it is decided by what the + /// spawner tags") and terrain still has no test for it; liquid gets one. + /// + /// The three companion assertions are the filter `UnownedSceneFilter` actually applies: a + /// surface that picked up `ModelPart`, `WmoGroupVis` or `WorldUnit` would be silently dropped + /// from the walk and go back to drawing through the ceiling with this test still green. + #[test] + fn an_adt_liquid_surface_is_tagged_exterior_scene() { + use bevy::ecs::system::RunSystemOnce; + let mut app = App::new(); + app.add_plugins(bevy::asset::AssetPlugin::default()) + .init_asset::() + .init_asset::(); + let sheets = [one_sheet([100.0, 100.0, 5.0])]; + let assets = adt_assets(); + let mut spawned = Vec::new(); + app.world_mut() + .run_system_once( + move |mut commands: Commands, mut meshes: ResMut>| { + let mut ents = Vec::new(); + spawn_liquids( + &mut commands, + sheets.iter(), + Some(&assets), + &mut meshes, + &mut ents, + ); + ents + }, + ) + .map(|ents| spawned = ents) + .expect("the spawn system ran"); + assert_eq!(spawned.len(), 1, "one sheet in, one surface out"); + let e = app.world().entity(spawned[0]); + assert!( + e.contains::(), + "an open-world liquid surface is exterior scene — untagged, the window cull never \ + queries it and the lake draws through a sealed ceiling (the director's report)" + ); + assert!( + e.contains::(), + "…and is still a liquid surface" + ); + // The three exclusions in `UnownedSceneFilter`. An ADT surface must satisfy all of them or + // the tag buys nothing. + assert!(!e.contains::()); + assert!(!e.contains::()); + assert!(!e.contains::()); + } } /// Which fog block a WMO pool takes, against the **real client data**. Skips when the 1.12.1 client diff --git a/crates/benilla-world/src/model_render/visibility.rs b/crates/benilla-world/src/model_render/visibility.rs index 1fb59df5..ddea0dc0 100644 --- a/crates/benilla-world/src/model_render/visibility.rs +++ b/crates/benilla-world/src/model_render/visibility.rs @@ -302,7 +302,18 @@ pub(super) fn apply_model_visibility( // …and the same exterior-window term as the submesh walk: another building's canal is // exterior content, the canal of the building you are standing in is not. let exterior_ok = !exterior || Some(gv.instance) == own_instance || gate.admits(xf, aabb); - let want = if portal_ok && exterior_ok { + // **A building's water rides the building's own toggle** — the reference's WMO liquid drain + // `0x684cd0` gates on `[0xc7b2a4] & 0x100`, the "Map objects" bit, NOT on the `0x1000000` + // "Water" bit the three ADT drains test (wow-re `terrain/scratch/liquid-window-gate.md` §5, + // decision 1657). Our `ModelKind::Wmo` toggle is that bit, and until now it hid a building + // and left its pool hanging in the air — which is also the shape a mis-placed pool takes, so + // the one instrument for telling those apart was itself producing the symptom. + // + // ADT liquid must NOT take this term and does not: it is `apply_exterior_cull`'s, which + // composes no toggles. The asymmetry is the reference's own, and it is per-*bit*, not an + // oversight to tidy up. + let toggled_on = m.kind_visible[kind_index(ModelKind::Wmo)]; + let want = if portal_ok && exterior_ok && toggled_on { Visibility::Inherited } else { Visibility::Hidden @@ -517,4 +528,73 @@ mod tests { "an unreadable owner fails OPEN" ); } + + /// **A building's pool rides the building's own toggle** (decision 1657) — and ADT liquid must + /// not, which is the half that makes this a fidelity fact rather than a tidy-up. + /// + /// The reference gates its WMO liquid drain `0x684cd0` on `[0xc7b2a4] & 0x100`, the "Map + /// objects" bit, while the three ADT drains test the separate `0x1000000` "Water" bit (wow-re + /// `terrain/scratch/liquid-window-gate.md` §5). `ModelKind::Wmo` is our "Map objects" bit, and + /// without this term switching buildings off left every canal, fountain and dungeon pool + /// hanging in mid-air — which happens to be exactly what a mis-placed pool looks like, so the + /// one instrument for telling a water bug from a wall bug was manufacturing the symptom. + #[test] + fn a_wmo_pool_rides_the_map_objects_toggle() { + let mut app = App::new(); + app.init_resource::() + .init_resource::() + .init_resource::() + .init_resource::() + .init_resource::() + .add_systems(Update, apply_model_visibility); + // A portal-less prop's pool: `WmoGroupVis` with no `ModelPart` (the `group_only` audience), + // and an instance that carries no latch, so `portal_ok` fails open and the toggle is the + // only term in play. + let instance = app.world_mut().spawn(()).id(); + let pool = app + .world_mut() + .spawn(( + crate::wmo_portal::WmoGroupVis { + instance, + groups: std::sync::Arc::from([0u16].as_slice()), + }, + GlobalTransform::IDENTITY, + Visibility::Inherited, + )) + .id(); + app.update(); + assert_eq!( + *app.world().entity(pool).get::().unwrap(), + Visibility::Inherited, + "buildings on: the canal draws" + ); + + app.world_mut() + .resource_mut::() + .models + .kind_visible[kind_index(ModelKind::Wmo)] = false; + app.update(); + assert_eq!( + *app.world().entity(pool).get::().unwrap(), + Visibility::Hidden, + "buildings off: the water goes with the building, not on without it" + ); + + // The control: the toggle that is NOT the Map-objects bit must not reach a pool. Doodads + // off leaves the canal alone — a single `kind_visible` bool ANDed carelessly would take it. + app.world_mut() + .resource_mut::() + .models + .kind_visible[kind_index(ModelKind::Wmo)] = true; + app.world_mut() + .resource_mut::() + .models + .kind_visible[kind_index(ModelKind::Doodad)] = false; + app.update(); + assert_eq!( + *app.world().entity(pool).get::().unwrap(), + Visibility::Inherited, + "the doodad toggle is a different bit and must not touch a building's water" + ); + } } diff --git a/crates/benilla-world/src/terrain_stream/spawn/mod.rs b/crates/benilla-world/src/terrain_stream/spawn/mod.rs index 7559d0cc..fac7fcfe 100644 --- a/crates/benilla-world/src/terrain_stream/spawn/mod.rs +++ b/crates/benilla-world/src/terrain_stream/spawn/mod.rs @@ -509,20 +509,48 @@ pub(super) fn spawn_loaded_placements( &m.material_diff_color, &mut ents, ); + // Another building's canal is exterior scene like its walls (`0x6856c0`, + // the same per-window populate). **Unconditional, like the walls at the + // `ExteriorScene` site above** — decision 1652's second half, and a + // structural fix with **no shipped symptom**, which is the honest framing. + // + // It used to be tagged only *inside* the `portal_instance` arm below, + // because `WmoGroupVis` carries the instance the camera's-own-building + // exemption is keyed on. That left a pool with NEITHER component — and so + // no `Visibility` authority at all — whenever a placement got no instance + // (`has_portals || m.wmo_id != 0`, ~line 334), while the same placement's + // walls, tagged unconditionally, were window-culled normally. + // + // **That branch is dead on shipped 1.12.1 content** (`cargo run -p + // benilla-formats --example wmo_ownerless_pools`): of 815 roots, 448 have + // no portal graph and exactly ONE has `wmoID == 0` — and that one + // (`pvp_alterac_ent01.wmo`) *does* carry portals. The two conditions never + // coincide, so zero placements reach the hole and none of them carries + // liquid. Kept anyway because it costs nothing and the asymmetry with the + // walls two blocks up was a real defect in the code even where the data + // never exercised it — but it must never be credited with a sighting. + // + // An instance-less placement can never BE the camera's claimed room (the + // interior down-ray resolves through an instance), so there is no + // exemption to lose: the tag alone is right for it, and + // `apply_exterior_cull` is its sole authority exactly as for ADT liquid. + for &e in &ents[first..] { + commands + .entity(e) + .insert(crate::exterior_cull::ExteriorScene); + } if let Some(instance) = p.portal_instance { let groups: Arc<[u16]> = Arc::from([gi as u16].as_slice()); for &e in &ents[first..] { - commands.entity(e).insert(( - WmoGroupVis { - instance, - groups: groups.clone(), - }, - // Another building's canal is exterior scene like its walls; the - // one you are standing in is exempted by instance (0784). Only - // tagged alongside `WmoGroupVis`, because that component is what - // carries the instance the exemption is keyed on. - crate::exterior_cull::ExteriorScene, - )); + // …and where there IS an instance, the pool rides its group with + // the room (0689) and takes the exemption (0784) — which moves it + // out of `apply_exterior_cull`'s query (`Without`) + // and into the model-visibility authority, the one that can see + // both terms. + commands.entity(e).insert(WmoGroupVis { + instance, + groups: groups.clone(), + }); } } } diff --git a/crates/benilla-world/src/wmo_portal/audit/pin.rs b/crates/benilla-world/src/wmo_portal/audit/pin.rs index 1e9111d1..0afa3501 100644 --- a/crates/benilla-world/src/wmo_portal/audit/pin.rs +++ b/crates/benilla-world/src/wmo_portal/audit/pin.rs @@ -211,6 +211,46 @@ fn wmo_group_census() { .map(|(lo, hi)| format!(" LIQUID local z[{lo:.1},{hi:.1}]")) .unwrap_or_default(), ); + // The group's portal edges, each placed in WoW world space. The group lines above answer + // "which room is that"; without this the doorway between two of them is a local-space + // vertex span nobody can point at in game, so a `.go xyz` report can't name the portal it + // is standing in front of. + let start = g.ref_start as usize; + let end = (start + g.ref_count as usize).min(subject.model.portal_refs.len()); + for r in &subject.model.portal_refs[start..end] { + let Some(info) = subject.model.portal_infos.get(r.portal as usize) else { + continue; + }; + let vs = (info.start_vertex as usize) + ..(info.start_vertex as usize + info.count as usize) + .min(subject.model.portal_vertices.len()); + let verts = &subject.model.portal_vertices[vs]; + if verts.is_empty() { + continue; + } + let mut c = [0.0f32; 3]; + for v in verts { + for k in 0..3 { + c[k] += v[k] / verts.len() as f32; + } + } + let w = bevy_to_wow(placed.world_from_local.transform_point3(wow_to_bevy(c))); + // Widest chord across the polygon (yd) — a doorway's opening, so "did that hop really + // have a 3-yd gap to see through" is answered without re-projecting by hand. + let span = verts + .iter() + .flat_map(|a| { + verts.iter().map(move |b| { + ((a[0] - b[0]).powi(2) + (a[1] - b[1]).powi(2) + (a[2] - b[2]).powi(2)) + .sqrt() + }) + }) + .fold(0.0f32, f32::max); + println!( + " p{:<3} ->g{:03}(side {:+}) world ({:.1},{:.1},{:.1}) span {:.1} yd", + r.portal, r.group, r.side, w[0], w[1], w[2], span, + ); + } } } diff --git a/crates/benilla-world/src/world_census.rs b/crates/benilla-world/src/world_census.rs index bcdea2e7..812bbac4 100644 --- a/crates/benilla-world/src/world_census.rs +++ b/crates/benilla-world/src/world_census.rs @@ -147,6 +147,10 @@ pub struct CullTerms { /// a two-dozen audience into a tens-of-thousands one erases it. pub bodies: usize, pub bodies_hidden: usize, + /// The liquid subset of `tested`/`hidden` — see the verdict's own note for why a few dozen + /// surfaces summed into tens of thousands of terrain cells is an invisible leg (1652). + pub liquid: usize, + pub liquid_hidden: usize, } impl WorldCensus<'_, '_> { @@ -284,6 +288,8 @@ impl WorldCensus<'_, '_> { unbounded: v.unbounded, bodies: v.bodies, bodies_hidden: v.bodies_hidden, + liquid: v.liquid, + liquid_hidden: v.liquid_hidden, }), mats: self.mats.len(), meshes: self.meshes.len(),