From ec36c06271bb3794ee48591e9139601a4a52251c Mon Sep 17 00:00:00 2001 From: Sam Date: Mon, 3 Aug 2026 17:52:05 +0700 Subject: [PATCH] buff timers survive a teleport, and ogres walk at ogre pace MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - a buff or debuff keeps its duration timer through a teleport: the table dies with the session now, not with the map change - a scaled-up creature paces its animation by its size — the Dire Maul ogre's walk drops from 2.2× to true speed, mounts likewise - the phantom "you have mail" icon at login is gone: no mail means no icon - underwater there is no sky, and the camera eye never straddles the waterline Co-Authored-By: Claude Fable 5 --- .../src/bin/benilla-extract/m2dump.rs | 8 +- crates/benilla/src/clouds/layer.rs | 7 +- crates/benilla/src/clouds/mod.rs | 7 +- crates/benilla/src/creature_anim.rs | 20 ++ crates/benilla/src/creature_anim/driver.rs | 68 +++++-- .../benilla/src/creature_anim/driver/tests.rs | 118 ++++++++++++ crates/benilla/src/creature_anim/select.rs | 33 +++- .../benilla/src/creature_anim/select/tests.rs | 38 +++- crates/benilla/src/interact.rs | 14 +- crates/benilla/src/lighting/mod.rs | 6 +- crates/benilla/src/liquid/mod.rs | 17 +- crates/benilla/src/liquid/query.rs | 16 ++ crates/benilla/src/player.rs | 12 +- crates/benilla/src/player/camera.rs | 100 ++++++++++ crates/benilla/src/player/setup.rs | 3 +- crates/benilla/src/sky.rs | 16 +- crates/benilla/src/sun/mod.rs | 33 +++- crates/benilla/src/ui_aura.rs | 174 +++++++++++++++--- crates/benilla/src/ui_mail.rs | 109 +++++++++-- 19 files changed, 699 insertions(+), 100 deletions(-) diff --git a/crates/benilla-formats/src/bin/benilla-extract/m2dump.rs b/crates/benilla-formats/src/bin/benilla-extract/m2dump.rs index bc17cf4b..f13a99aa 100644 --- a/crates/benilla-formats/src/bin/benilla-extract/m2dump.rs +++ b/crates/benilla-formats/src/bin/benilla-extract/m2dump.rs @@ -87,7 +87,7 @@ pub fn m2seq(chain: &mut Chain, internal_path: &str) -> Result<()> { // non-global-sequence track selects its keys from it, so it is what says whether a track is // actually keyed HERE or is holding a clamped value from some other sequence's band. // NB `idx` is this list's index, not the file's: zero-duration sequences are dropped. - println!("idx anim mode dur(s) band(ms) freq replay bones keys"); + println!("idx anim mode dur(s) mspd band(ms) freq replay bones keys"); for (i, s) in seqs.iter().enumerate() { // How much data the sequence's own time band actually holds: bones with any // keyed track, and total keys across T/R/S (clamp constants included — a bone @@ -105,10 +105,14 @@ pub fn m2seq(chain: &mut Chain, internal_path: &str) -> Result<()> { .map(|b| b.translation.len() + b.rotation.len() + b.scale.len()) .sum(); println!( - "{i:>3} {:>4} {} {:>7.3} {:>7}..{:<7} {:>5} ({}, {}) {bones:>5} {keys:>5}", + "{i:>3} {:>4} {} {:>7.3} {:>5.2} {:>7}..{:<7} {:>5} ({}, {}) {bones:>5} {keys:>5}", s.anim_id, if s.looping { "loop " } else { "clamp" }, s.duration, + // `mspd` = the sequence's authored design movement speed (yd/s) — the DIVISOR of the + // locomotion playback rate (`speed / (mspd · |modelScale|)`, wow-re `0x5fe2f0` + // @0x5fe4be..0x5fe550). `0.00` ⇒ not a locomotion sequence, so it plays at rate 1×. + s.move_speed, s.start_ms, s.end_ms, s.frequency, diff --git a/crates/benilla/src/clouds/layer.rs b/crates/benilla/src/clouds/layer.rs index 76424825..0f1a5603 100644 --- a/crates/benilla/src/clouds/layer.rs +++ b/crates/benilla/src/clouds/layer.rs @@ -182,13 +182,18 @@ pub(super) fn setup_cloud_layer( /// **all six** sky elements together (`0x6d4a3b test edi,edi; je`), so the painted art is the whole /// sky rather than a backdrop the procedural clouds keep drawing over. The art carries its own cloud /// banks; layering ours on top lifted the painted zenith out of its near-black. +/// +/// A **submerged eye** hides them the same way: the scene driver's `0x6812a4` submerged test skips +/// the whole `CSky::Render` call (byte-VERIFIED, wow-re terrain "the liquid render state") — from +/// under the surface there is no sky, only the murk. pub(super) fn apply_cloud_visibility( debug: Res, wmo_skybox: Res, + underwater: Res, mut dome: Query<&mut Visibility, With>, ) { if let Ok(mut vis) = dome.single_mut() { - *vis = if debug.lighting.disable_sky_dome || wmo_skybox.0.is_some() { + *vis = if debug.lighting.disable_sky_dome || wmo_skybox.0.is_some() || underwater.0.any() { Visibility::Hidden } else { Visibility::Inherited diff --git a/crates/benilla/src/clouds/mod.rs b/crates/benilla/src/clouds/mod.rs index bb44d1a9..0b6a4c22 100644 --- a/crates/benilla/src/clouds/mod.rs +++ b/crates/benilla/src/clouds/mod.rs @@ -77,8 +77,11 @@ impl Plugin for CloudsPlugin { ( tick_clouds, // After the resolve: the dome and the painted skybox must agree WITHIN a frame, - // or one frame draws both (the ordering `crate::sky`'s gate already takes). - layer::apply_cloud_visibility.after(crate::wmo_sky::WmoSkyResolve), + // or one frame draws both (the ordering `crate::sky`'s gate already takes) — + // and after the submersion verdict, for the same one-frame-agreement reason. + layer::apply_cloud_visibility + .after(crate::wmo_sky::WmoSkyResolve) + .after(crate::liquid::SubmersionVerdict), ), ) // Camera-anchored placement post-propagation, like the sky dome (decision 0504). diff --git a/crates/benilla/src/creature_anim.rs b/crates/benilla/src/creature_anim.rs index da548c0b..8e40f154 100644 --- a/crates/benilla/src/creature_anim.rs +++ b/crates/benilla/src/creature_anim.rs @@ -515,7 +515,13 @@ pub(crate) use spell_visual::{ }; /// The per-unit animation state machine. +/// +/// `Transform` is required, not optional: the driver reads the entity's render scale as the +/// locomotion rate divisor (decision 0903), so a driver on a transform-less entity would silently +/// stop being driven at all. Every real unit is spawned with one — the requirement is here so a +/// hand-assembled entity (a test world, a future harness) can't quietly miss it. #[derive(Component)] +#[require(Transform)] pub(crate) struct AnimDriver { mode: Mode, /// The gait id currently targeted while in [`Mode::Gait`] (so we cross-fade only on a change). `None` @@ -593,6 +599,13 @@ pub(crate) struct AnimDriver { /// walks a multi-part dance. `None` = the main arm is a one-shot (its budget is a `Count` /// repeat) or a deliberate freeze (ranged Load / Loot). loop_window: Option<(bevy::animation::graph::AnimationNodeIndex, u32)>, + /// The playback rate last written to the base slot — `speed / (moveSpeed · |modelScale|)` + /// ([`select::playback_rate`], decision 0903); `1.0` for anything that is not a rate-scaled + /// locomotion clip. Purely an **instrument**: the hover inspector's anim line reads it, so + /// "this creature's walk looks too fast" is answerable by hovering the creature instead of by + /// working the divisor out by hand. Recorded rather than re-derived, so the card can never + /// disagree with what is actually playing. + gait_rate: f32, /// The Special wanted LAST frame — the driver's Special **edge** detector (decision 0864). /// An edge is a play in the client (the jump/pose entry, the FALLINGFAR latch's Fall, the /// land pick), so it clears the deferred-combat cache like any normal arm (`0x5fe48e`); @@ -619,6 +632,12 @@ impl AnimDriver { }; (base, self.overlay.map(|o| o.id)) } + + /// The playback rate the base slot is running at ([`Self::gait_rate`]) — the inspector card's + /// `rate` readout. `1.0` for every clip outside the locomotion whitelist. + pub(crate) fn rate(&self) -> f32 { + self.gait_rate + } } /// A masked upper-body play over the base ([`AnimDriver::overlay`]): the graph node it drives @@ -681,6 +700,7 @@ impl Default for AnimDriver { deferred: None, ranged_held: false, loop_window: None, + gait_rate: 1.0, last_special: None, } } diff --git a/crates/benilla/src/creature_anim/driver.rs b/crates/benilla/src/creature_anim/driver.rs index f2d54f10..d3fffa27 100644 --- a/crates/benilla/src/creature_anim/driver.rs +++ b/crates/benilla/src/creature_anim/driver.rs @@ -280,6 +280,11 @@ pub(super) fn drive_animations( Has, Option<&crate::entities::mount::MountBody>, Has, + // The rendered model scale — `OBJECT_FIELD_SCALE_X`, baked onto the entity transform by + // `entities::attach`. The locomotion playback rate divides by it (decision 0903), so an + // ogre at 2.2× cycles its legs 2.2× slower than a same-speed human. Read-only: nothing + // in this system moves a unit. + &Transform, ), )>, // A mount child's movement view is its HOST's (decision 0441): the same @@ -293,6 +298,11 @@ pub(super) fn drive_animations( Option<&UnitSpeeds>, Option<&FacingStep>, Has, + // …and the rider's own `OBJECT_FIELD_SCALE_X`: a mount child's transform carries only its + // `CreatureDisplayInfo` column, and the mount renders at the PRODUCT of the two (the + // byte-verified mount composition, wow-re `0x613ef0`). The rate divisor wants that product + // — the mount model's world scale — not the child's local column (decision 0903). + &Transform, )>, mut swings: MessageReader, mut impacts: MessageReader, @@ -413,26 +423,33 @@ pub(super) fn drive_animations( wielded, cast_hold, facing_step, - (engaged, auto_repeat, ranged_hold, is_self, mount_body, creature_swimming), + (engaged, auto_repeat, ranged_hold, is_self, mount_body, creature_swimming, transform), ) in &mut units { // A mount child drives from its HOST's movement view (decision 0441) — same inputs the // rider's own pass reads, so the mount plays exactly the locomotion the rider suppresses. // A vanished host (teardown race) reads as stationary until the child despawns with it. - let (spline, remote, movement, speeds, facing_step, creature_swimming) = match mount_body { - Some(mb) => match mount_hosts.get(mb.host) { - Ok((s, r, m, sp, f, sw)) => (s, r, m, sp, f, sw), - Err(_) => (None, None, None, None, None, false), - }, - None => ( - spline, - remote, - movement, - speeds, - facing_step, - creature_swimming, - ), - }; + // The host also supplies the missing half of the mount's world scale (see the query docs). + let (spline, remote, movement, speeds, facing_step, creature_swimming, host_scale) = + match mount_body { + Some(mb) => match mount_hosts.get(mb.host) { + Ok((s, r, m, sp, f, sw, t)) => (s, r, m, sp, f, sw, t.scale.x), + Err(_) => (None, None, None, None, None, false, 1.0), + }, + None => ( + spline, + remote, + movement, + speeds, + facing_step, + creature_swimming, + 1.0, + ), + }; + // The rendered world scale of the model playing these clips — the `0x5fe2f0` rate divisor's + // `|modelScale|` (decision 0903). A plain unit's transform already IS its world scale; a + // mount child's is its display column, which composes under the rider's. + let model_scale = transform.scale.x * host_scale; let walk = speeds.map_or(DEFAULT_WALK_SPEED, |s| s.0.walk); // Dead ⇒ health 0 with a real max. Absent health counts as ZERO (`unit_is_dead`): a create // block omits zero fields, so an already-dead corpse streams in with no HEALTH at all — @@ -1193,8 +1210,10 @@ pub(super) fn drive_animations( // mid-air revive must re-select, not hold the corpse pose.) if let Some(c) = drv.gait.and_then(|g| find_resolved(anims, g, catalog)) { for v in anims.clips.iter().filter(|v| v.anim_id == c.anim_id) { + let rate = playback_rate(v, mv.speed, model_scale); if let Some(active) = player.animation_mut(v.node) { - active.set_speed(playback_rate(v, mv.speed)); + active.set_speed(rate); + drv.gait_rate = rate; } } } @@ -1321,8 +1340,10 @@ pub(super) fn drive_animations( // (decision 0123) — sync the rate on whichever variation node is // live (a stale fade-out node getting the same rate is harmless). for v in anims.clips.iter().filter(|v| v.anim_id == c.anim_id) { + let rate = playback_rate(v, mv.speed, model_scale); if let Some(active) = player.animation_mut(v.node) { - active.set_speed(playback_rate(v, mv.speed)); + active.set_speed(rate); + drv.gait_rate = rate; } } // One completed pass of the ranged Load clip arms the HOLD twin @@ -1359,7 +1380,7 @@ pub(super) fn drive_animations( "PLAY gait {} (was {:?}) rate {:.2}", c.anim_id, drv.gait, - playback_rate(c, mv.speed) + playback_rate(c, mv.speed, model_scale) ), ); } @@ -1381,7 +1402,8 @@ pub(super) fn drive_animations( drv.loop_window = Some((c.node, budget)); bevy::animation::RepeatAnimation::Forever }; - play_clip(&mut tr, &mut player, c, repeat, playback_rate(c, mv.speed)); + drv.gait_rate = playback_rate(c, mv.speed, model_scale); + play_clip(&mut tr, &mut player, c, repeat, drv.gait_rate); drv.gait = Some(target); // A fresh pick of the drawn HOLD re-nocks (INTERIM, decision 0409): the // hold resuming after a fire clip (or after the Load promotion) shows @@ -1509,12 +1531,18 @@ pub(super) fn drive_animations( // a boolean, not a countdown, so a fade doesn't spam a line per frame. if is_self && crate::dbg_trace::enabled() { let state = format!( - "mode={:?} gait={:?} special={:?} flags={:08x} speed={:.2} upper={:?}{}", + "mode={:?} gait={:?} special={:?} flags={:08x} speed={:.2} scale={:.2} \ + rate={:.2} upper={:?}{}", drv.mode, drv.gait, special, mv.flags, mv.speed, + // The rate's new input and its result (decision 0903): `speed` alone stopped + // predicting the gait's playback the moment the model scale joined the divisor, + // so a "the gait looks wrong" trace carrying only speed could no longer answer it. + model_scale, + drv.gait_rate, drv.overlay.map(|ov| ov.id), if drv.overlay_fade.is_some() { "+fade" diff --git a/crates/benilla/src/creature_anim/driver/tests.rs b/crates/benilla/src/creature_anim/driver/tests.rs index fa253abf..4d52deff 100644 --- a/crates/benilla/src/creature_anim/driver/tests.rs +++ b/crates/benilla/src/creature_anim/driver/tests.rs @@ -2206,3 +2206,121 @@ fn a_run_starting_under_a_cast_still_transplants_it_up() { "a locomotion re-arm moves the cast to the torso instead of cutting it" ); } + +/// A walker's model: Stand plus a Walk(4) authored at the 2.5 yd/s design speed every 1.12.1 +/// creature and character rig shares (byte-read from `ogremage.m2` and `humanmale.m2` with +/// `benilla-extract m2seq`). +fn walker_model() -> ModelAnimations { + let mut walk = clip(4, 2, true); + walk.move_speed = 2.5; + ModelAnimations { + graph: Handle::default(), + clips: vec![clip(0, 1, true), walk], + hand_close: [None, None], + playable_animation_lookup: Vec::new(), + animation_lookup: Vec::new(), + global_bones: Vec::new(), + first_seq: None, + pose: Default::default(), + } +} + +/// The rate the driver actually wrote onto the walk node — the number the fix is about, read back +/// off the live `AnimationPlayer` rather than off the driver's own bookkeeping. +fn walk_rate(app: &App, unit: Entity) -> f32 { + app.world() + .entity(unit) + .get::() + .unwrap() + .animation(AnimationNodeIndex::new(2)) + .expect("the walk node is playing") + .speed() +} + +/// The director's ogre, end to end through the real system (decision 0903): a Gordok Ogre-Mage +/// (`CreatureDisplayInfo.CreatureModelScale` 2.2, so `OBJECT_FIELD_SCALE_X` 2.2) walking at +/// vmangos' `speed_walk` 1.6 × 2.5 = 4.0 yd/s must cycle its legs at 4.0 / (2.5 × 2.2) = 0.73×. +/// Scale-blind it read 1.60× — the "too fast walk" report. The control is the same unit at scale +/// 1.0, which must still read the un-divided 1.60×: the fix may not touch ordinary-size creatures. +#[test] +fn a_scaled_creatures_walk_cycles_slower_than_an_unscaled_ones() { + let walking = MovementState { + flags: move_flags::FORWARD, + speed: 4.0, + ..Default::default() + }; + let mut app = app(); + let ogre = app + .world_mut() + .spawn(( + walker_model(), + AnimationPlayer::default(), + AnimationTransitions::new(), + AnimDriver::default(), + Transform::from_scale(Vec3::splat(2.2)), + walking, + )) + .id(); + let human = app + .world_mut() + .spawn(( + walker_model(), + AnimationPlayer::default(), + AnimationTransitions::new(), + AnimDriver::default(), + Transform::from_scale(Vec3::splat(1.0)), + walking, + )) + .id(); + app.update(); + assert!( + (walk_rate(&app, ogre) - 0.727_27).abs() < 1e-3, + "the 2.2x ogre walks its cycle 2.2x slower, not at the scale-blind 1.60x: got {}", + walk_rate(&app, ogre) + ); + assert!( + (walk_rate(&app, human) - 1.6).abs() < 1e-3, + "an unscaled unit is untouched by the divisor: got {}", + walk_rate(&app, human) + ); +} + +/// The mounted half of the same law: the client's divisor reads the MOUNT model +/// (`[unit+0xdc] ?: [unit+0xd8]`), whose rendered scale is the rider's `OBJECT_FIELD_SCALE_X` +/// times the mount's own `CreatureDisplayInfo` column (wow-re `0x613ef0`). Our mount child carries +/// only that column on its transform, so the driver must compose the host's in — a 1.5× sabre +/// under a 2.0× rider divides by 3.0, not 1.5. +#[test] +fn a_mounts_gait_rate_composes_the_riders_scale_with_the_mounts() { + let mut app = app(); + let rider = app + .world_mut() + .spawn(( + Transform::from_scale(Vec3::splat(2.0)), + MovementState { + flags: move_flags::FORWARD, + speed: 4.0, + ..Default::default() + }, + )) + .id(); + let mount = app + .world_mut() + .spawn(( + walker_model(), + AnimationPlayer::default(), + AnimationTransitions::new(), + AnimDriver::default(), + Transform::from_scale(Vec3::splat(1.5)), + crate::entities::mount::MountBody { host: rider }, + )) + .id(); + app.update(); + // 4.0 / (2.5 · 1.5 · 2.0) = 0.533… — the mount also inherits the rider's movement view, so the + // speed feeding the divide is the rider's, exactly as the client's per-unit call reads it. + assert!( + (walk_rate(&app, mount) - 0.533_33).abs() < 1e-3, + "the mount divides by rider x mount scale: got {}", + walk_rate(&app, mount) + ); +} diff --git a/crates/benilla/src/creature_anim/select.rs b/crates/benilla/src/creature_anim/select.rs index c9ef6289..4795eb92 100644 --- a/crates/benilla/src/creature_anim/select.rs +++ b/crates/benilla/src/creature_anim/select.rs @@ -992,12 +992,33 @@ pub(super) fn gait_is_locomotion(state: &MovementState, walk_speed: f32) -> bool .is_some_and(|id| is_locomotion(*id)) } -/// The playback rate for a clip given the unit's live speed (wow-5875-re `0x5fe2f0`): `speed / moveSpeed` -/// for a rate-scaled locomotion clip, else 1×. (The client also divides by the model's scale magnitude, -/// 1.0 for an unscaled character.) Guarded on `moveSpeed > 0`. -pub(super) fn playback_rate(clip: &AnimClip, speed: f32) -> f32 { - if clip.move_speed > 0.0 && RATE_SCALED.contains(&clip.anim_id) { - speed / clip.move_speed +/// The playback rate for a clip given the unit's live speed and its rendered model scale — the +/// client's `0x5fe2f0` divide, VERIFIED byte-for-byte (wow-5875-re `swim-mechanism.md` §TU-I, +/// `0x5fe4be..0x5fe550`): +/// +/// ```text +/// 5fe508 call 0x711a20 ; DIVISOR = |modelScale| · sequence.moveSpeed +/// 5fe515 fcomp 0.0 ; jne ; GUARD A: divisor > 0 (else stay 1×) +/// 5fe528 call 0x5fee80 ; GUARD B: the locomotion id whitelist (else stay 1×) +/// 5fe53c call 0x7c4c90 ; NUMERATOR = the flag-scalar current speed +/// 5fe541 fdiv ; rate = speed / (moveSpeed · |modelScale|) +/// ``` +/// +/// **`model_scale` is the half we were missing** (decision 0903). A sequence's `moveSpeed` is the +/// ground speed its authored cycle covers *at the model's authored size*; render that model at `s×` +/// and one cycle covers `s×` the ground, so the legs must cycle `s×` slower to hold the same speed. +/// Without the divisor every off-1.0-scale creature ran its gait exactly `s×` too fast — a Gordok +/// Ogre-Mage (`CreatureDisplayInfo.CreatureModelScale` 2.2) scurried its walk at 2.2× rate, and a +/// 1.5× riding sabre its run at 1.5×. +/// +/// The scale to pass is the **rendered world scale of the model actually playing the clip**: for a +/// unit, `OBJECT_FIELD_SCALE_X` (the server has already folded the DBC scale in — `entities::attach`); +/// for a mount child, its own `CreatureDisplayInfo` column composed under the rider's `SCALE_X` — what +/// the client's `[unit+0xdc] ?: [unit+0xd8]` model pick resolves to. +pub(super) fn playback_rate(clip: &AnimClip, speed: f32, model_scale: f32) -> f32 { + let divisor = clip.move_speed * model_scale.abs(); + if divisor > 0.0 && RATE_SCALED.contains(&clip.anim_id) { + speed / divisor } else { 1.0 } diff --git a/crates/benilla/src/creature_anim/select/tests.rs b/crates/benilla/src/creature_anim/select/tests.rs index c8900ca8..0ab84bc1 100644 --- a/crates/benilla/src/creature_anim/select/tests.rs +++ b/crates/benilla/src/creature_anim/select/tests.rs @@ -379,15 +379,39 @@ fn reconcile_ranged_exemption_and_remote_pull_through() { #[test] fn backpedal_rate_speeds_up_a_slow_design_speed() { // Backpedaling at 4.5 yd/s against WalkBack authored for 2.5 → 1.8× (the "too slow" fix). - assert!((playback_rate(&clip(13, 2.5), 4.5) - 1.8).abs() < 1e-5); - assert!((playback_rate(&clip(5, 7.0), 7.0) - 1.0).abs() < 1e-5); + assert!((playback_rate(&clip(13, 2.5), 4.5, 1.0) - 1.8).abs() < 1e-5); + assert!((playback_rate(&clip(5, 7.0), 7.0, 1.0) - 1.0).abs() < 1e-5); } #[test] fn non_locomotion_clips_play_at_unit_rate() { - assert_eq!(playback_rate(&clip(0, 0.0), 9.0), 1.0); // idle - assert_eq!(playback_rate(&clip(38, 0.0), 9.0), 1.0); // jump hang (moveSpeed 0) - assert_eq!(playback_rate(&clip(60, 2.0), 9.0), 1.0); // an id outside the scaled set + assert_eq!(playback_rate(&clip(0, 0.0), 9.0, 1.0), 1.0); // idle + assert_eq!(playback_rate(&clip(38, 0.0), 9.0, 1.0), 1.0); // jump hang (moveSpeed 0) + assert_eq!(playback_rate(&clip(60, 2.0), 9.0, 1.0), 1.0); // an id outside the scaled set +} + +/// The `0x5fe2f0` divisor is `moveSpeed · |modelScale|`, not `moveSpeed` alone (decision 0903) — +/// the real numbers behind the director's two reports, so a regression names the creature it broke. +#[test] +fn a_big_model_cycles_its_legs_slower_for_the_same_ground_speed() { + // The Gordok Ogre-Mage (creature 11443, display 12472): `CreatureModelScale` 2.2, walking at + // vmangos' `speed_walk` 1.6 × the 2.5 yd/s base = 4.0 yd/s, against ogremage.m2's Walk(4) + // authored `moveSpeed` 2.5. Scale-blind that reads 1.60× — the scurry the director reported. + assert!((playback_rate(&clip(4, 2.5), 4.0, 2.2) - 0.727_27).abs() < 1e-4); + // The riding sabre (model 457, `CreatureModelScale` 1.5) at a 60% mount's 11.2 yd/s against + // Run(5)'s authored 6.94: 1.08×, not the scale-blind 1.61×. + assert!((playback_rate(&clip(5, 6.94), 11.2, 1.5) - 1.075_89).abs() < 1e-4); + // An unscaled model is the identity — every 1.0-scale creature and player is unaffected. + assert!((playback_rate(&clip(5, 6.94), 11.2, 1.0) - 1.613_83).abs() < 1e-4); +} + +/// GUARD A tests the *divisor*, so a degenerate scale falls through to 1× instead of dividing by +/// zero (a 0-scale unit is reachable: `OBJECT_FIELD_SCALE_X` is server-set and briefly 0 on a +/// half-applied morph). `|modelScale|` is a magnitude — a negative scale mirrors, never reverses. +#[test] +fn a_degenerate_model_scale_falls_through_to_unit_rate() { + assert_eq!(playback_rate(&clip(4, 2.5), 4.0, 0.0), 1.0); + assert!((playback_rate(&clip(4, 2.5), 4.0, -2.2) - 0.727_27).abs() < 1e-4); } #[test] @@ -904,6 +928,6 @@ fn the_prowl_idle_is_the_lowest_priority_stand() { /// at 1× however fast the unit is actually travelling. #[test] fn the_prowl_creep_is_not_rate_scaled() { - assert_eq!(playback_rate(&clip(STEALTH_WALK, 2.5), 7.0), 1.0); - assert_ne!(playback_rate(&clip(4, 2.5), 7.0), 1.0); + assert_eq!(playback_rate(&clip(STEALTH_WALK, 2.5), 7.0, 1.0), 1.0); + assert_ne!(playback_rate(&clip(4, 2.5), 7.0, 1.0), 1.0); } diff --git a/crates/benilla/src/interact.rs b/crates/benilla/src/interact.rs index 1da4516a..b9e71f68 100644 --- a/crates/benilla/src/interact.rs +++ b/crates/benilla/src/interact.rs @@ -586,9 +586,19 @@ fn inspect_ui( }; let (base, overlay) = d.playing(); let base = base.map(&fmt).unwrap_or_else(|| "—".into()); + // The base slot's live playback rate (decision 0903) — `speed / (moveSpeed · modelScale)` + // on a locomotion clip, a flat 1× on everything else. Shown so a "its walk is too fast" + // report is a hover away from a number instead of a hand-worked divisor; suppressed at + // exactly 1× so the ordinary case doesn't carry noise. + let rate = d.rate(); + let rate = if (rate - 1.0).abs() > 1e-3 { + format!(" · rate {rate:.2}×") + } else { + String::new() + }; match overlay { - Some(o) => format!("anim {base} + overlay {}", fmt(o)), - None => format!("anim {base}"), + Some(o) => format!("anim {base} + overlay {}{rate}", fmt(o)), + None => format!("anim {base}{rate}"), } }); // The light lane + the attach that found it (decision 0776). Absent until the classifier has diff --git a/crates/benilla/src/lighting/mod.rs b/crates/benilla/src/lighting/mod.rs index 79dbf0a9..b8da6e06 100644 --- a/crates/benilla/src/lighting/mod.rs +++ b/crates/benilla/src/lighting/mod.rs @@ -259,7 +259,11 @@ impl Plugin for LightingPlugin { (update_time_lighting, apply_sky_backdrop) .chain() // The storm blend reads this frame's weather densities (decision 0302). - .after(crate::weather::WeatherTick), + .after(crate::weather::WeatherTick) + // The submerged atmosphere reads THIS frame's submersion verdict — unordered, + // the murk could arrive a frame after the eye went under (and out of step with + // the sky-pass suppression, which reads the same verdict). + .after(crate::liquid::SubmersionVerdict), ); // The shared global-light buffer (build_light_data after the resolve above; the extract + // render-world upload). Materials read this instead of carrying their own light copy. diff --git a/crates/benilla/src/liquid/mod.rs b/crates/benilla/src/liquid/mod.rs index 799d916a..b266ab28 100644 --- a/crates/benilla/src/liquid/mod.rs +++ b/crates/benilla/src/liquid/mod.rs @@ -65,11 +65,20 @@ mod surface; // the against-real-client-files tests — they span both halves // client may name. `LiquidHit` and `LiquidSurface` are deliberately absent — they are returned and // spawned, never named from outside; add them here the day something needs to. pub(crate) use query::{ - camera_claim, describe_at, liquid_at, player_claim, unit_claim, water_surface_at, FoamPatch, - LiquidClaim, LiquidSource, Underwater, WaterChunkInfo, WmoPool, + camera_claim, describe_at, liquid_at, player_claim, surfaces_at, unit_claim, water_surface_at, + FoamPatch, LiquidClaim, LiquidSource, Underwater, WaterChunkInfo, WmoPool, }; pub(crate) use surface::{spawn_liquids, spawn_wmo_liquids, LiquidAssets, LiquidSoundSource}; +/// 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 +/// colour, and the sky-pass suppression all flip on it), so a consumer reading a frame-old verdict +/// shows one mixed frame on every surface crossing — the murk with the sun still up, or clear water +/// with the sky already gone. One label, so the "same frame" contract is written once instead of +/// re-derived per consumer. +#[derive(bevy::ecs::schedule::SystemSet, Debug, Clone, Copy, PartialEq, Eq, Hash)] +pub(crate) struct SubmersionVerdict; + /// The water subsystem: load the per-kind frame arrays + shared materials at startup, then cycle the /// animation frame each update. Spawning the per-chunk surfaces happens in the terrain streamer (via /// [`spawn_liquids`], water lives *with* its tile), reading [`LiquidAssets`]. @@ -89,7 +98,9 @@ impl Plugin for LiquidPlugin { // the eye's position, and a frame-old room would flash the wrong atmosphere for // one frame on every doorway crossing — which, for a whole-screen filter, is // exactly the artefact that reads worst. - query::detect_submersion.after(crate::wmo_portal::WmoPvsSet), + query::detect_submersion + .after(crate::wmo_portal::WmoPvsSet) + .in_set(SubmersionVerdict), ), ) // The surface-render kill-switch (see [`hide_liquid_surfaces`]) — inert without the env diff --git a/crates/benilla/src/liquid/query.rs b/crates/benilla/src/liquid/query.rs index 7b87f769..5ffa1159 100644 --- a/crates/benilla/src/liquid/query.rs +++ b/crates/benilla/src/liquid/query.rs @@ -663,6 +663,22 @@ pub(crate) fn describe_at<'a>( out.into_iter().map(|(_, line)| line).collect() } +/// Every **admitted** liquid surface height over `wow`'s XY under the given claim — the same +/// delegation [`submersion_at`] applies (the 0634/0696 scoping: a pool in another building or +/// storey never answers), with no over/under verdict attached: the consumer that needs surfaces +/// the subject is merely *near* — above or below — is the camera's waterline snap +/// (`player::camera::waterline_snap`), which keeps the eye out of the near-plane band around +/// every admitted plane. Kept here so the delegation rule has one owner; `answers` stays private. +pub(crate) fn surfaces_at<'a>( + liquids: impl Iterator + 'a, + wow: [f32; 3], + claim: LiquidClaim, +) -> impl Iterator + 'a { + liquids + .filter(move |w| w.answers(claim, wow[2])) + .filter_map(move |w| w.surface_z_at(wow[0], wow[1])) +} + /// [`liquid_at`] restricted to **water** kinds — the query for the consumers that are about water /// specifically (the wade splash, footstep depth, the remote-motion spline's depth), which must not /// fire in the Great Forge's lava or Undercity's slime. Swim mode deliberately does NOT use this one. diff --git a/crates/benilla/src/player.rs b/crates/benilla/src/player.rs index 5a5de241..2fd98845 100644 --- a/crates/benilla/src/player.rs +++ b/crates/benilla/src/player.rs @@ -62,7 +62,7 @@ use camera::{ apply_zoom_scroll, run_look_session, seat_camera, CameraProbe, FlyCam, LookButton, CAM_COLLISION_RADIUS, CAM_DIST_DEFAULT, CAM_PIVOT_FALLBACK, }; -pub(crate) use camera::{head_height, CameraControl, CameraPivot, WorldCamera, CAM_NEAR}; +pub(crate) use camera::{head_height, CameraControl, CameraPivot, WorldCamera, CAM_FOVY, CAM_NEAR}; pub(crate) use follow::{FollowRequest, FollowState}; // The shared avatar state + movement constants live in [`state`]; the private re-imports below are // what lets this module and the concern modules beside it keep naming them `super::X` unchanged. @@ -363,10 +363,16 @@ fn control( // (decisions 0634 + 0696, the "swim in air" family). Fourth slot here rather than a 17th // top-level param. Res, + // The CAMERA eye's own claim, for the waterline snap — the eye's room is not the player's + // (0696's lesson, applied to this consumer): a pool in another storey must not snap a dry + // camera. Published by the portal pass AFTER input, so this is last frame's room — a + // one-frame stale claim at a doorway-plus-waterline crossing, accepted. + Res, ), ) { let (water, transports, child_of) = (&world_q.0, &world_q.1, &world_q.2); let claim = crate::liquid::player_claim(&world_q.3); + let cam_claim = crate::liquid::camera_claim(&world_q.4); let (left_click, right_click) = (&mut *click_test.0, &mut *click_test.1); let Ok((mut cam_t, mut cam, camera)) = cameras.single_mut() else { return; @@ -577,6 +583,8 @@ fn control( &mut cam_t, &move_and_slide, cam_probe, + water, + cam_claim, ); return; } @@ -1369,6 +1377,8 @@ fn control( &mut cam_t, &move_and_slide, cam_probe, + water, + cam_claim, ); // The cast bar's local self-cancel trigger (`ui_cast::local_self_cancel`): a fresh diff --git a/crates/benilla/src/player/camera.rs b/crates/benilla/src/player/camera.rs index be8adeb5..60d424d4 100644 --- a/crates/benilla/src/player/camera.rs +++ b/crates/benilla/src/player/camera.rs @@ -91,6 +91,50 @@ pub(crate) fn head_height(pivot: Option<&CameraPivot>, scale: f32) -> f32 { /// as the near plane would begin to slice it (the reference couples the two; ours is larger than the /// client's ~0.1 because our depth range keeps a 1.0 near for precision). pub(crate) const CAM_NEAR: f32 = 1.0; +/// The camera's vertical field of view (radians) — one constant shared by the projection +/// ([`setup`]) and the waterline band ([`waterline_band`], which needs the near rectangle's true +/// height). 45°, the value the projection has always used (Bevy's `PerspectiveProjection` default, +/// ≈ the reference's 44.1° — [`crate::sun`]'s projection note); naming it here just stops the two +/// consumers drifting apart. +pub(crate) const CAM_FOVY: f32 = std::f32::consts::FRAC_PI_4; +/// Margin (yd) added to the geometric near-plane band of the waterline snap ([`waterline_band`]): +/// float slack + a visual guard so the surface never grazes the near rectangle's very edge. +const WATERLINE_MARGIN: f32 = 0.05; + +/// The half-height of the eye band a liquid plane must never enter: the camera near rectangle's +/// **world-vertical reach** at the current pitch, plus [`WATERLINE_MARGIN`]. +/// +/// The near rectangle sits `CAM_NEAR` ahead of the eye, spans `±tan(fovy/2)·CAM_NEAR` vertically, +/// and pitches with the camera (no roll, so its width never contributes world height). Its corners' +/// world-Y offset from the eye is therefore bounded by `half_h·cos(pitch) + near·sin(|pitch|)` — +/// with the eye farther than that from a horizontal plane, the rectangle cannot intersect it, so +/// the waterline can never lie across the screen. ~0.46 yd looking level, ~1.05 yd at steep pitch: +/// our band is ~10× the reference's because our near plane is (`CAM_NEAR`'s doc) — the *mechanism* +/// is faithful, the magnitude is necessarily our own. +pub(super) fn waterline_band(pitch: f32) -> f32 { + let half_h = (CAM_FOVY * 0.5).tan() * CAM_NEAR; + half_h * pitch.cos().abs() + CAM_NEAR * pitch.sin().abs() + WATERLINE_MARGIN +} + +/// The waterline **snap** (director's call, 2026-08-03: "it's either snap under or above" — never +/// the half-submerged view with the waterline across the screen): given the desired eye height and +/// the admitted liquid surface heights under/over its XY, return the corrected eye height, or +/// `None` when the eye already clears every surface by [`waterline_band`]'s `band`. +/// +/// The eye keeps the **side it was already on** (`d = eye − surface` signed; `d = 0` counts as +/// above) and is pushed to `surface ± band` — the nearest surface by |d| wins where several stack. +/// Pure, in WoW-Z (== Bevy Y), so the rule is testable without ECS scaffolding, like +/// [`crate::liquid`]'s `submersion_at`. +pub(super) fn waterline_snap( + eye_z: f32, + band: f32, + surfaces: impl Iterator, +) -> Option { + let (d, z) = surfaces + .map(|z| (eye_z - z, z)) + .min_by(|a, b| a.0.abs().total_cmp(&b.0.abs()))?; + (d.abs() < band).then_some(if d >= 0.0 { z + band } else { z - band }) +} /// A small sphere swept from the camera pivot toward the desired camera seat each frame to keep walls /// from sliding between the camera and the character (camera collision). Built once at startup like @@ -377,6 +421,8 @@ pub(super) fn seat_camera( cam_t: &mut Transform, move_and_slide: &MoveAndSlide, cam_probe: &Collider, + water: &Query<&crate::liquid::WaterChunkInfo>, + eye_claim: crate::liquid::LiquidClaim, ) { // A keyboard turn carries the camera RIGIDLY (char and camera rotate as one — the reference // look, director's call closing 0050's open "camera follow on turn"): an eased chase of a @@ -432,6 +478,22 @@ pub(super) fn seat_camera( }; let frac = (rig.collision_distance / boom_len).clamp(0.0, 1.0); cam_t.translation = head + boom * frac; + // The waterline snap: the eye never sits within the near rectangle's reach of a liquid plane, + // so the screen is always wholly above or wholly below the surface — never the half-and-half + // straddle with the waterline across it (director's call, 2026-08-03; [`waterline_snap`]). + // Candidates take the EYE's own room claim, not the player's — a pool in another storey must + // not snap a dry camera (the 0696 delegation, applied to this consumer too). Runs before the + // dump below so probes read the realized pose, and before the self-fade so a snapped-in eye + // still fades the avatar by where the camera actually is. (Known edge, accepted: the vertical + // nudge happens after the collision sweep, so waterline + overhead geometry within a yard can + // momentarily overlap the probe radius — rare enough to take over re-sweeping per frame.) + { + let eye_wow = benilla_assets::coords::bevy_to_wow(cam_t.translation); + let admitted = crate::liquid::surfaces_at(water.iter(), eye_wow, eye_claim); + if let Some(z) = waterline_snap(eye_wow[2], waterline_band(cam.pitch), admitted) { + cam_t.translation.y = z; // WoW Z is Bevy Y — no round-trip needed for a height fix + } + } // `WOW_CAM_DUMP=frame`: the REALIZED pose, per frame, bit-exact — not the pose that was asked for. // // Every scripted probe sets `yaw`/`pitch`/`distance` and we then reason as though the camera is @@ -760,6 +822,44 @@ mod tests { use crate::billboard::BillboardCard; use crate::mesh_tag::alpha_bits; + /// The waterline band is the near rectangle's true world-vertical reach: at level pitch the + /// half-height alone, at steep pitch the near distance dominates, and it never collapses below + /// the margin — the geometry the snap's whole guarantee rests on. + #[test] + fn waterline_band_tracks_the_near_rectangle() { + let half_h = (CAM_FOVY * 0.5).tan() * CAM_NEAR; + assert!((waterline_band(0.0) - (half_h + 0.05)).abs() < 1e-6); + // Steeper pitch ⇒ the rectangle's far edge dips farther below the eye than half_h alone. + assert!(waterline_band(1.0) > waterline_band(0.0)); + // Symmetric in look-up vs look-down. + assert!((waterline_band(0.7) - waterline_band(-0.7)).abs() < 1e-6); + } + + /// The snap rule: inside the band the eye is pushed to `surface ± band` on the side it was + /// already on (0 counts as above); outside the band, and with no surface at all, it is left + /// alone; where surfaces stack the nearest one judges. + #[test] + fn waterline_snap_keeps_the_side_and_clears_the_band() { + let band = 0.5; + // Above, inside the band → pushed up to surface + band. + assert_eq!(waterline_snap(10.2, band, [10.0].into_iter()), Some(10.5)); + // Below, inside the band → pushed down to surface − band. + assert_eq!(waterline_snap(9.8, band, [10.0].into_iter()), Some(9.5)); + // Exactly on the plane counts as above (a deterministic tie, not a flicker source). + assert_eq!(waterline_snap(10.0, band, [10.0].into_iter()), Some(10.5)); + // Clear of the band → untouched. + assert_eq!(waterline_snap(10.6, band, [10.0].into_iter()), None); + assert_eq!(waterline_snap(9.4, band, [10.0].into_iter()), None); + // No admitted surface → untouched. + assert_eq!(waterline_snap(10.0, band, std::iter::empty()), None); + // Two stacked surfaces: the NEAREST judges — an eye under a high lake but hovering over a + // low pool snaps off the pool, not the lake 5 yd up. + assert_eq!( + waterline_snap(10.1, band, [15.0, 10.0].into_iter()), + Some(10.5) + ); + } + /// The self-avatar's zoom-to-first-person fade reaches its BILLBOARD cards — the night-elf eye /// glow (ledger B71: two additive quads left burning in mid-air after the body was hidden). /// A card is a world ROOT following an anchor inside the model (decision 0153), so the fade's diff --git a/crates/benilla/src/player/setup.rs b/crates/benilla/src/player/setup.rs index 159eac38..bd9b6081 100644 --- a/crates/benilla/src/player/setup.rs +++ b/crates/benilla/src/player/setup.rs @@ -16,7 +16,7 @@ use crate::SPAWN_XY; use super::{ CameraControl, CameraProbe, FlyCam, MoveSpeed, Player, PlayerCapsule, CAM_COLLISION_RADIUS, - CAM_DIST_DEFAULT, CAM_NEAR, CAPSULE_HEIGHT, CAPSULE_RADIUS, + CAM_DIST_DEFAULT, CAM_FOVY, CAM_NEAR, CAPSULE_HEIGHT, CAPSULE_RADIUS, }; /// Default avatar speed in yards/second — the **VERIFIED** vanilla run speed (`MOVE_RUN` 7.0). Ctrl @@ -101,6 +101,7 @@ pub(super) fn setup_player( Projection::from(PerspectiveProjection { far: cam_far, near: CAM_NEAR, + fov: CAM_FOVY, ..default() }), // HDR render target (linear `Rgba16Float`) — the prerequisite for Bevy's `Bloom`. `Hdr` is a diff --git a/crates/benilla/src/sky.rs b/crates/benilla/src/sky.rs index 002e6098..058ca5d6 100644 --- a/crates/benilla/src/sky.rs +++ b/crates/benilla/src/sky.rs @@ -123,7 +123,9 @@ impl Plugin for SkyPlugin { update_sky_colors, // The dome stands down for a WMO skybox, so its gate must read the SETTLED // resolve, not whichever side of it the executor picked (`crate::wmo_sky`). - apply_sky_visibility.after(crate::wmo_sky::WmoSkyResolve), + apply_sky_visibility + .after(crate::wmo_sky::WmoSkyResolve) + .after(crate::liquid::SubmersionVerdict), ), ) // Camera-anchored placement runs post-propagation (the BillboardPlace slot) off the @@ -243,18 +245,22 @@ fn follow_camera( } /// Hide/show the dome from the debug panel's "disable sky dome" toggle — and stand it down entirely -/// while a **WMO skybox** owns the backdrop ([`crate::wmo_sky`]): a building whose group asks for its -/// MOSB model replaces this gradient, it does not layer over it. With the dome hidden the -/// `ClearColor` backdrop (the row-7 fog colour, or black via "black backdrop") shows through. +/// while a **WMO skybox** owns the backdrop ([`crate::wmo_sky`]), or while the eye is **submerged**: +/// a building whose group asks for its MOSB model replaces this gradient, it does not layer over it, +/// and underwater the reference skips the whole sky pass (the scene driver's `0x6812a4` submerged +/// test gates `CSky::Render 0x6d4940` — stars, discs, gradient band and cloud dome together; +/// byte-VERIFIED, wow-re terrain "the liquid render state"). With the dome hidden the `ClearColor` +/// backdrop (the row-7 fog colour — submerged, the murk — or black via "black backdrop") shows through. fn apply_sky_visibility( debug: Res, wmo_skybox: Res, + underwater: Res, mut dome: Query<&mut Visibility, With>, ) { let Ok(mut vis) = dome.single_mut() else { return; }; - let want = if debug.lighting.disable_sky_dome || wmo_skybox.0.is_some() { + let want = if debug.lighting.disable_sky_dome || wmo_skybox.0.is_some() || underwater.0.any() { Visibility::Hidden } else { Visibility::Visible diff --git a/crates/benilla/src/sun/mod.rs b/crates/benilla/src/sun/mod.rs index b23837d2..351602eb 100644 --- a/crates/benilla/src/sun/mod.rs +++ b/crates/benilla/src/sun/mod.rs @@ -125,17 +125,21 @@ impl Plugin for SunPlugin { PostUpdate, (follow_sun, follow_moons, follow_stars).in_set(crate::billboard::BillboardPlace), ) - // The WMO-skybox suppression, ordered after the resolve for the same reason - // `crate::sky`'s dome gate is: both backdrops must agree WITHIN a frame, or a frame - // shows the painted sky with our stars still punched through it. + // The WMO-skybox + submersion suppression, ordered after both resolves for the same + // reason `crate::sky`'s dome gate is: backdrop and atmosphere must agree WITHIN a + // frame, or a frame shows the painted sky with our stars still punched through it + // (or the murk with the sun still up). .add_systems( Update, - apply_celestial_visibility.after(crate::wmo_sky::WmoSkyResolve), + apply_celestial_visibility + .after(crate::wmo_sky::WmoSkyResolve) + .after(crate::liquid::SubmersionVerdict), ); } } -/// Hide the sky pass's own elements while a WMO skybox owns the sky ([`crate::wmo_sky`]). +/// Hide the sky pass's own elements while a WMO skybox owns the sky ([`crate::wmo_sky`]) — or while +/// the eye is **submerged**. /// /// `CSky::Render` gates its six draws on **one shared boolean** — `0x6d49cd` sets it, a filled skybox /// slot clears it (`0x6d49fb`/`0x6d4a2e`, once a slot's weight exceeds `[0x808aac]` = 0.99), and @@ -144,23 +148,32 @@ impl Plugin for SunPlugin { /// five. Live-confirmed: standing in King's Square the reference's whole `[0.975, 0.98]` slice is /// three `count=12` draws — the skybox cube — and nothing else. /// +/// The **submerged eye** suppresses the same set one level up: the scene driver's `0x6812a4` +/// submerged test skips the whole `CSky::Render` call (byte-VERIFIED, wow-re terrain "the liquid +/// render state" — "the surface is drawn identically from below; what changes underwater is +/// scene-wide atmosphere only"). Without it the discs keep drawing under the murk, tinted by the +/// underwater LightParams' celestial band — which is black there, so the sun read as a black ball +/// from under water (director report, 2026-08-03). +/// /// **The GLARES are deliberately exempt.** They are not in this pass: the reference renders them /// last, on their own path (`0x483740 → 0x6d48c0 → 0x7e57e0`), which the boolean never reaches — so -/// a sun flare still blooms over the painted sky. The gradient band is [`crate::sky`]'s dome and the -/// cloud dome is [`crate::clouds`]'s; each keeps its own authority (decision 0025) reading this same -/// resource. +/// a sun flare still blooms over the painted sky. (Whether the glare path carries its own submersion +/// gate is a wow-re question in flight; until it lands the flare keeps its envelope behaviour.) The +/// gradient band is [`crate::sky`]'s dome and the cloud dome is [`crate::clouds`]'s; each keeps its +/// own authority (decision 0025) reading these same resources. #[allow(clippy::type_complexity)] fn apply_celestial_visibility( wmo_skybox: Res, + underwater: Res, mut suns: Query<(&SunSprite, &mut Visibility), Without>, mut moons: Query<(&MoonSprite, &mut Visibility), Without>, mut stars: Query<&mut Visibility, (With, Without, Without)>, ) { - let owned = wmo_skybox.0.is_some(); + let suppressed = wmo_skybox.0.is_some() || underwater.0.any(); // `Inherited`, not `Visible`: these sprites hang off the celestial rig and must keep deferring to // it when the skybox stands down, exactly as the cloud dome's gate does. let want = |in_sky_pass: bool| { - if in_sky_pass && owned { + if in_sky_pass && suppressed { Visibility::Hidden } else { Visibility::Inherited diff --git a/crates/benilla/src/ui_aura.rs b/crates/benilla/src/ui_aura.rs index 96a669e4..d01657ad 100644 --- a/crates/benilla/src/ui_aura.rs +++ b/crates/benilla/src/ui_aura.rs @@ -48,6 +48,7 @@ use benilla_formats::SpellCatalog; use benilla_protocol::messages::{UnitAuraSlot, AURA_FLAG_CANCELABLE, UNIT_AURA_POSITIVE_SLOTS}; use benilla_ui::script::{AuraState, ScriptValue, TrackingState, UiScript}; +use crate::char_select::ClientState; use crate::net::{ClientCommand, Guid, NetCommands, ObjectStore, SelfPlayer}; use crate::target::Selection; use crate::ui_action::Spells; @@ -57,9 +58,13 @@ use crate::ui_unit::UnitFeed; /// The self-only aura durations, keyed by raw `UNIT_FIELD_AURA` slot — the decoded /// `SMSG_UPDATE_AURA_DURATION` payload, stamped with the real time it arrived. Written by the net /// apply path (which owns the event stream), read by [`feed_auras`]. A slot's entry is overwritten -/// by each fresh packet (apply/refresh) and dropped only when the avatar goes — never per-frame on -/// slot occupancy, which would delete every stamp in the frame before its own aura arrives -/// (decision 0846). Bounded by construction: one entry per raw slot, ≤ 48. +/// by each fresh packet (apply/refresh) and dropped only when the *session* ends (leaving +/// `InWorld`) — never per-frame on slot occupancy, which would delete every stamp in the frame +/// before its own aura arrives (decision 0846), and never on the avatar entity's absence, which a +/// cross-map worldport produces mid-session while the auras themselves live on (decision 0900). +/// This is the reference's `0xbc5f68` expiry array: raw-slot-keyed, written only by the duration +/// packet, and a process global that no world change touches. Bounded by construction: one entry +/// per raw slot, ≤ 48. #[derive(Resource, Default)] pub(crate) struct AuraDurations { by_slot: HashMap, @@ -139,10 +144,14 @@ impl Plugin for UiAuraPlugin { fn build(&self, app: &mut App) { app.init_resource::() .init_resource::() + .init_resource::() // Feed before the VM dispatch (like the unit feed), so a frame's OnEvent sees the fresh // list; drain the cancels after, once the VM has queued them. .add_systems(Update, feed_auras.in_set(UnitFeed).before(UiInput)) - .add_systems(Update, drain_aura_cancels.after(UiInput)); + .add_systems(Update, drain_aura_cancels.after(UiInput)) + // The ONLY teardown of the aura state, and it hangs off the session edge — never off + // the avatar entity's existence, which a worldport interrupts mid-session (0900). + .add_systems(OnExit(ClientState::InWorld), end_session_aura_state); } } @@ -185,8 +194,10 @@ fn reconcile(cache: &mut Vec, live: &[UnitAuraSlot], now: f64) { /// a `UNIT_AURA` trigger. type AuraProjection = (u32, u8, bool, Option); -/// The feed's edge-detection memory: per token, the projection of the list last pushed. -#[derive(Default)] +/// The feed's edge-detection memory: per token, the projection of the list last pushed. A resource +/// rather than a `Local` (the [`crate::ui_unit::UnitFeedState`] pattern) because the session-end +/// teardown resets it from its own system — see [`end_session_aura_state`]. +#[derive(Resource, Default)] struct AuraFeedMemory { present: bool, last: Vec, @@ -321,28 +332,36 @@ fn feed_auras( selection: Res, stores: Query<&ObjectStore>, spells: Option>, - mut durations: ResMut, + // Read-only over the stamps: the feed joins them, the net apply path writes them, and only the + // session end drops them (decision 0900). Nothing the feed sees may invalidate one. + durations: Res, mut cache: ResMut, time: Res>, - mut mem: Local, + mut mem: ResMut, ) { let Some(mut script) = script else { return; }; let Ok((store, self_guid)) = self_q.single() else { - // No avatar yet: clear once, so a stale list can't linger across a logout. - if mem.present || mem.target_last.is_some() { - script.set_auras("player", None); - script.set_auras("target", None); - script.set_tracking(None); - cache.auras.clear(); - // The stamps go with the cache: nothing prunes them per-frame (see the note in the - // join below), so losing the avatar is where they are dropped. A stale one could not - // survive a re-entry anyway — the freshness gate rejects a stamp older than the aura — - // but leaving the map to accumulate across logins would be sloppy state. - durations.by_slot.clear(); - *mem = AuraFeedMemory::default(); - } + // No avatar entity — and that is NOT, on its own, the end of the aura state, so nothing is + // torn down here (that is [`end_session_aura_state`]'s job, on the session edge). A + // cross-map worldport despawns every tracked entity, ours included, and the new map + // re-streams it under a fresh entity with the same guid (`net::apply::tag_self_player`'s + // own note); for the frames of that gap there is no avatar while every aura on us is still + // live, and the server sends nothing to rebuild the timers with — vmangos emits + // `SMSG_UPDATE_AURA_DURATION` only on an aura's apply/refresh and at + // `Map::ExistingPlayerLogin` (the already-online relog), never on a worldport. + // + // The reference has nothing to lose across that gap: its buff cache (`0xbc6040`) and expiry + // array (`0xbc5f68`) are process globals, zeroed once at startup (`0x4e40c0`, reached only + // from the one-shot init at `0x48f5a9`, itself called only from `0x401602`) and otherwise + // touched by exactly two sites — the duration packet's setter `0x4e43a6` and + // `GetPlayerBuffTimeLeft`'s reader `0x4e4467`. That is the whole xref set: `0xbc5f68` + // appears three times in `WoW.exe`. Nothing clears them on a world change, so a debuff's + // countdown simply carries across the loading screen. Clearing here was the "after a tele + // my disease has no timer" defect (decision 0900): the stamps were dropped, and even + // without that every survivor's `appeared_at` restarted at the re-stream, so the freshness + // gate below would have rejected whatever stamps remained. return; }; @@ -371,7 +390,7 @@ fn feed_auras( // freshness gate below, which compares a stamp against the aura it would be joined to rather // than against a slot's momentary emptiness — the defence 0257 §3 built for exactly this. // The map cannot grow: it is keyed by raw slot (≤ 48), each entry overwritten by the next - // packet for that slot, and cleared with the rest of the aura state when the avatar goes. + // packet for that slot, and cleared with the rest of the aura state when the session ends. let script_now = script.now(); @@ -566,6 +585,35 @@ fn join_duration( .unwrap_or((0.0, 0.0)) } +/// The end of the session (`OnExit(ClientState::InWorld)` — a confirmed `/logout`, back to the glue +/// layer): drop the whole aura state, lists and duration stamps alike, so nothing reaches the next +/// character's bar. +/// +/// This is the *only* place the state is dropped, and the edge it hangs off is the point. The +/// reference drops nothing, ever — `0xbc5f68` is zeroed once at client startup and written only by +/// the duration packet (decision 0900) — because at this boundary it was protected by a server that +/// re-sent every aura's duration in its login preamble; mangos still carries the bare +/// `// SMSG_UPDATE_AURA_DURATION` placeholder where that packet went in +/// `Player::SendInitialPacketsBeforeAddToMap`, and vmangos never fills it in. So a stamp we kept +/// across a logout would sit in its raw slot with nothing to overwrite it, and the next character's +/// aura in that slot could inherit a timer that was never theirs. One divergence from the reference, +/// at the one boundary where the reference relied on a server we don't have. +fn end_session_aura_state( + script: Option>, + mut durations: ResMut, + mut cache: ResMut, + mut mem: ResMut, +) { + if let Some(mut script) = script { + script.set_auras("player", None); + script.set_auras("target", None); + script.set_tracking(None); + } + cache.auras.clear(); + durations.by_slot.clear(); + *mem = AuraFeedMemory::default(); +} + /// Drain the spell ids `CancelUnitBuff` queued this frame and send one `CMSG_CANCEL_AURA` each. The /// server cancels by spell, not slot (decision 0257 B8); it refuses anything the wire's /// `AFLAG_CANCELABLE` bit didn't allow, which the binding already gated on. @@ -582,6 +630,88 @@ fn drain_aura_cancels(script: Option>, net: Res App { + // The cancel drain's outbound channel. Nothing sends on it here (its only source is the + // script VM, which this app has none of), so the dropped receiver is inert. + let (tx, _) = crossbeam_channel::unbounded(); + let mut app = App::new(); + app.add_plugins((MinimalPlugins, bevy::state::app::StatesPlugin)) + .insert_state(ClientState::InWorld) + .init_resource::() + .insert_resource(NetCommands(tx)) + .add_plugins(UiAuraPlugin); + app + } + + /// Seed the app with one live aura carrying a running timer — the state a teleport must not + /// disturb and a logout must. + fn seed_one_timed_aura(app: &mut App) { + app.world_mut() + .resource_mut::() + .set(32, 300_000, 10.0); + app.world_mut() + .resource_mut::() + .auras + .push(CachedAura { + slot: 32, // the first harmful slot — a debuff, like the report's disease + spell_id: 11976, + appeared_at: 10.0, + flags: 0x8, + level: 60, + stacks: 1, + }); + app.world_mut().resource_mut::().present = true; + } + + /// **Decision 0900, pinned.** A cross-map worldport despawns every tracked entity — our avatar + /// included — and the new map re-streams it, so for a stretch of frames mid-session there is no + /// `SelfPlayer` while every aura on us is still live. Nothing may tear the aura state down on + /// that: vmangos re-sends `SMSG_UPDATE_AURA_DURATION` only on apply/refresh (and at an + /// already-online relog), never on a worldport, so a dropped stamp is gone for the aura's whole + /// remaining life — the director's "after a tele my disease has no timer". + /// + /// The session edge is the one place it *does* go, and this pins both halves — the second one + /// squarely, the first one for what a VM-less app can reach: the Update schedule leaves a + /// seeded, avatar-less aura state alone. The teardown having exactly one site, on that edge, is + /// what makes the first half hold in the real app, and that is structural, not asserted here. + #[test] + fn the_aura_state_survives_an_avatar_less_frame_and_dies_only_with_the_session() { + let mut app = aura_app(); + seed_one_timed_aura(&mut app); + + // Frames with no avatar entity at all — the worldport gap. + app.update(); + app.update(); + assert_eq!( + app.world().resource::().auras.len(), + 1, + "the display cache survives the gap — its `appeared_at` is the freshness gate's anchor" + ); + assert!( + app.world() + .resource::() + .by_slot + .contains_key(&32), + "the duration stamp survives the gap — nothing will re-send it" + ); + + // The session ends: `/logout` back to the glue layer. + app.world_mut() + .resource_mut::>() + .set(ClientState::CharSelect); + app.update(); + assert!(app.world().resource::().auras.is_empty()); + assert!(app.world().resource::().by_slot.is_empty()); + assert!( + !app.world().resource::().present, + "the edge memory resets too, so the next character's first list counts as an edge" + ); + } + fn slot(slot: u8, spell_id: u32) -> UnitAuraSlot { UnitAuraSlot { slot, diff --git a/crates/benilla/src/ui_mail.rs b/crates/benilla/src/ui_mail.rs index c5c535c1..878d7236 100644 --- a/crates/benilla/src/ui_mail.rs +++ b/crates/benilla/src/ui_mail.rs @@ -22,14 +22,16 @@ //! window when the player leaves the mailbox's interaction range (wow-re §5: the client's generic //! interaction-target range gate — past it every mail `CMSG` silently fails its `CheckMailBox`). //! -//! **The arrival layer ([`MailPending`], decision 0544 P3, wow-re §5 `mail-interaction.md` §7).** -//! `HasNewMail()` mirrors the real client's own model: a countdown float (`0x845eac`) fed by -//! `MSG_QUERY_NEXT_MAIL_TIME`'s reply and `SMSG_RECEIVED_MAIL`, decremented per frame, flipping -//! true at `<= 0.0`. vmangos only ever sends `0.0` ("waiting now") or `-86400.0` ("none") — never -//! a positive value — but the countdown is implemented in full since that's the client's real -//! model, not a `0.0`/`-86400.0` special case. [`send_query_next_mail_time_on_enter`] fires the -//! query once per world-enter (0548 §7: "once at UI/login load"); [`feed_mail`] decrements the -//! float every frame, pushes `HasNewMail()` to the VM +//! **The arrival layer ([`MailPending`], decisions 0544 P3 / 0904, wow-re §5 `mail-interaction.md` +//! §4).** `HasNewMail()` mirrors the real client's own model: a countdown float (`0x845eac`) fed by +//! `MSG_QUERY_NEXT_MAIL_TIME`'s reply and `SMSG_RECEIVED_MAIL`, stepped down per frame, and read as +//! **`|countdown| < ε`** — *near zero*, not `<= 0` ([`MailPending::has_new_mail`]; the sign matters, +//! decision 0904: vmangos's "no mail" reply is `-86400.0`, which a `<= 0` predicate reads as "you +//! have mail" at every login). vmangos only ever sends `0.0` ("waiting now") or `-86400.0` ("none") +//! — never a positive value — but the countdown is implemented in full since that's the client's +//! real model, not a `0.0`/`-86400.0` special case. [`send_query_next_mail_time_on_enter`] fires the +//! query once per world-enter (0548 §7: "once at UI/login load"); [`feed_mail`] steps the +//! float every frame ([`MailPending::step`]), pushes `HasNewMail()` to the VM //! ([`benilla_ui::script::UiScript::set_has_new_mail`]) and fires `UPDATE_PENDING_MAIL` on every //! value transition — the only event the reference `MiniMapMailFrame` listens for //! (`Minimap.xml` l.278-289: `OnEvent` just re-checks `HasNewMail()` and shows/hides; no @@ -193,13 +195,36 @@ impl NpcSession for MailOpen { #[derive(Resource, Default)] pub(crate) struct MailPending(pub(crate) Option); +/// The `HasNewMail()` / countdown-step epsilon — the real client's `[0x8029d4]` = `2^-22`, the +/// `fabs(x)`-vs-this threshold both `0x4afea0` (`HasNewMail`) and `0x4ade60` (the per-frame step) +/// compare against (VERIFIED byte-exact in wow-re, `crates/ui/src/glue_geom_4a8.rs::EPS_BITS`). +const MAIL_TIME_EPSILON: f32 = f32::from_bits(0x3480_0000); // 2^-22, wow-re's `EPS_BITS` + impl MailPending { - /// `HasNewMail()` (decision 0544 P3, wow-re §5 §7): the countdown has reached zero (vmangos - /// sends only `0.0` or `-86400.0`, never a positive value in between, but the countdown model - /// is implemented in full — a positive reply would count down and flip true at `0`, exactly - /// as the real client's own `0x845eac` float does). + /// `HasNewMail()` (decisions 0544 P3 / 0904; wow-re §5 §4, `0x4afea0` byte-exact: + /// `fld [0x845eac]; fabs; fcomp [0x8029d4]`) — true iff the countdown is **within ε of zero**. + /// + /// Near-zero, *not* `<= 0`: the sign carries meaning. vmangos answers + /// `MSG_QUERY_NEXT_MAIL_TIME` with `-86400.0` when nothing is unread (`MailHandler.cpp` + /// `HandleQueryNextMailTime`: `HasUnreadMail() ? 0.0f : -float(DAY)`), so a `<= 0` predicate + /// lights the minimap icon on every login for a character with no mail (decision 0904). fn has_new_mail(&self) -> bool { - self.0.is_some_and(|s| s <= 0.0) + self.0.is_some_and(|s| s.abs() < MAIL_TIME_EPSILON) + } + + /// The per-frame countdown step (wow-re §5 §4, `0x4ade60` byte-exact — carved as + /// `glue_geom_4a8::step_value`): a **non-positive countdown is left alone** (the `-86400.0` + /// "no mail" sentinel must never drift toward zero, and a reached-zero countdown must not sail + /// past it into negative — either would flip [`Self::has_new_mail`] the wrong way), and a + /// positive one steps down floor-clamped at `0.0`. The subtraction runs the client's x87 chain + /// (f64 under PC_53, narrowed at the store). + fn step(&mut self, delta_secs: f32) { + if let Some(cur) = self.0.as_mut() { + if *cur > 0.0 { + let diff = f64::from(*cur) - f64::from(delta_secs); // fsub, then fcom vs 0.0 + *cur = if diff > 0.0 { diff as f32 } else { 0.0 }; + } + } } } @@ -467,12 +492,10 @@ fn feed_mail( *last = fresh; *last_mailbox = mail.mailbox; - // The arrival layer (decision 0544 P3): decrement the client-local countdown, then push/fire + // The arrival layer (decisions 0544 P3 / 0904): step the client-local countdown, then push/fire // on every `HasNewMail()` value transition — the reference `MiniMapMailFrame`'s only listened // event (`Minimap.xml` l.278-289, quoted in the module doc). - if let Some(secs) = pending.0.as_mut() { - *secs -= time.delta_secs(); - } + pending.step(time.delta_secs()); let has_new_mail = pending.has_new_mail(); if has_new_mail != *last_has_new_mail { script.set_has_new_mail(has_new_mail); @@ -806,4 +829,56 @@ mod tests { // The item name/texture are in flight (no template answer yet). assert!(row.item_name.is_none()); } + + /// `HasNewMail()` is `|countdown| < ε`, not `countdown <= 0` (decision 0904). The regression + /// this pins: vmangos's "no unread mail" answer is `-86400.0`, and the old `<= 0` predicate + /// read it as "you have mail" — the phantom minimap icon on every login. + #[test] + fn has_new_mail_is_near_zero_not_non_positive() { + // Never queried — the reference's pre-first-query state. + assert!(!MailPending(None).has_new_mail()); + // vmangos's two real answers. + assert!(MailPending(Some(0.0)).has_new_mail()); + assert!(!MailPending(Some(-86400.0)).has_new_mail()); + // The threshold itself: inside ε counts, ε and beyond does not — either sign. + assert!(MailPending(Some(MAIL_TIME_EPSILON / 2.0)).has_new_mail()); + assert!(MailPending(Some(-MAIL_TIME_EPSILON / 2.0)).has_new_mail()); + assert!(!MailPending(Some(MAIL_TIME_EPSILON)).has_new_mail()); + assert!(!MailPending(Some(-MAIL_TIME_EPSILON)).has_new_mail()); + // A still-running countdown is not "waiting now". + assert!(!MailPending(Some(30.0)).has_new_mail()); + } + + /// The per-frame step (`0x4ade60`): non-positive is untouched, positive steps down and floors + /// at exactly `0.0` — so a countdown that expires flips `HasNewMail()` true and *stays* true. + #[test] + fn step_leaves_non_positive_alone_and_floors_at_zero() { + // The "no mail" sentinel never drifts toward zero, however long the session runs. + let mut none_waiting = MailPending(Some(-86400.0)); + for _ in 0..100 { + none_waiting.step(1.0 / 60.0); + } + assert_eq!(none_waiting.0, Some(-86400.0)); + assert!(!none_waiting.has_new_mail()); + + // A reached-zero countdown stays at zero rather than sailing past into negative. + let mut expired = MailPending(Some(0.0)); + expired.step(1.0 / 60.0); + assert_eq!(expired.0, Some(0.0)); + assert!(expired.has_new_mail()); + + // A positive countdown steps down, then floors (never overshoots past 0). + let mut counting = MailPending(Some(0.5)); + counting.step(0.125); // exact in binary — the step's value is pinned, not approximated + assert_eq!(counting.0, Some(0.375)); + assert!(!counting.has_new_mail()); + counting.step(10.0); + assert_eq!(counting.0, Some(0.0)); + assert!(counting.has_new_mail()); + + // Never queried stays never-queried. + let mut unqueried = MailPending(None); + unqueried.step(1.0); + assert_eq!(unqueried.0, None); + } }