From 450eea2cb3ad43d17bb312076f7d2d94939309b1 Mon Sep 17 00:00:00 2001 From: Sam Date: Mon, 3 Aug 2026 17:52:40 +0700 Subject: [PATCH] double-clicking your mount no longer kills the cast bar - an item use is a real cast with an in-flight gate: the double-click never hides the bar and never ships a duplicate use - the action bar answers both mouse buttons, and three more click-registration gaps are closed - a cast right after a camera drag no longer dies to the position you already left - landing while mounted keeps the gait's cadence, and particle clouds and ribbon trails sort correctly above and below the water surface Co-Authored-By: Claude Fable 5 --- crates/benilla-assets/src/model/anims.rs | 8 + .../src/bin/benilla-extract/main.rs | 5 +- crates/benilla/assets/ui/ActionBar.xml | 13 +- crates/benilla/assets/ui/BagFrame.xml | 7 +- crates/benilla/assets/ui/BankFrame.xml | 5 + crates/benilla/assets/ui/ChatFrame.xml | 9 +- crates/benilla/src/capture/mod.rs | 3 +- crates/benilla/src/capture/probes.rs | 40 ++-- crates/benilla/src/creature_anim.rs | 10 + crates/benilla/src/creature_anim/driver.rs | 131 +++++++----- .../benilla/src/creature_anim/driver/play.rs | 72 ++++++- .../benilla/src/creature_anim/driver/tests.rs | 124 +++++++++++ crates/benilla/src/creature_anim/select.rs | 57 ++++-- .../benilla/src/creature_anim/select/tests.rs | 15 ++ crates/benilla/src/liquid/surface.rs | 9 + crates/benilla/src/main.rs | 19 ++ crates/benilla/src/particles.rs | 3 + crates/benilla/src/particles/sim.rs | 116 ++++++++++- crates/benilla/src/player/movement_net.rs | 135 +++++++++++- crates/benilla/src/player/state.rs | 10 + crates/benilla/src/ribbons.rs | 12 +- crates/benilla/src/sky_order.rs | 24 +++ crates/benilla/src/ui_action/drain.rs | 24 ++- crates/benilla/src/ui_cast.rs | 28 +++ crates/benilla/src/ui_items/drain.rs | 79 ++++--- crates/benilla/src/ui_items/mod.rs | 193 +++++++++++++++++- .../benilla/src/ui_script/action_bar_tests.rs | 57 ++++++ crates/benilla/src/ui_script/bag_tests.rs | 18 +- crates/benilla/src/ui_script/bank_tests.rs | 34 +++ crates/benilla/src/water_fx/mod.rs | 8 +- 30 files changed, 1139 insertions(+), 129 deletions(-) diff --git a/crates/benilla-assets/src/model/anims.rs b/crates/benilla-assets/src/model/anims.rs index 3d275e58..4ff53276 100644 --- a/crates/benilla-assets/src/model/anims.rs +++ b/crates/benilla-assets/src/model/anims.rs @@ -26,6 +26,14 @@ pub struct AnimClip { /// The sequence's authored design movement speed (`ModelAnimation::move_speed`, yd/s) — the /// denominator the locomotion playback-rate scaler divides the unit's live speed by, so a fast /// mover cycles its legs proportionally faster. `0.0` for a non-locomotion clip (left at rate 1×). + /// + /// **Signed, and the sign is load-bearing — never `abs()` it.** A backwards gait is authored + /// NEGATIVE (`RidingKodo.m2` seq 14, WalkBackwards: `-2.5`), and the client's rate guard is a + /// strict `divisor > 0` (`0x5fe2f0`, §5-verified — decisions 0903/0910/0912). So an authored + /// backwards clip plays at a flat 1×, while a model that has *no* WalkBackwards and falls back + /// to Walk (`+2.5`) gets speed-scaled. That asymmetry is the reference's behaviour; taking the + /// magnitude here — an inviting-looking tidy-up — inverts it on every model authoring a reverse + /// gait. pub move_speed: f32, /// The sequence's blend-in time (`ModelAnimation::blend_time`, seconds) — the cross-fade duration the /// spawn site uses when transitioning *into* this clip, so a gait change eases instead of snapping. diff --git a/crates/benilla-formats/src/bin/benilla-extract/main.rs b/crates/benilla-formats/src/bin/benilla-extract/main.rs index ef8a7110..40f9743e 100644 --- a/crates/benilla-formats/src/bin/benilla-extract/main.rs +++ b/crates/benilla-formats/src/bin/benilla-extract/main.rs @@ -80,8 +80,9 @@ enum Command { internal_path: String, }, /// Dump an M2's animation sequences in file order: `AnimationData.dbc` id, loop/clamp flag, - /// duration, variation frequency, replay range — the variation/replay instrument - /// (decisions 0114/0117; sequences sharing an id are its variation chain). + /// duration, authored design speed (the rate divisor), variation frequency, replay range — + /// the variation/replay instrument (decisions 0114/0117; sequences sharing an id are its + /// variation chain). M2seq { /// Internal path to the `.m2` (forward or back slashes accepted). internal_path: String, diff --git a/crates/benilla/assets/ui/ActionBar.xml b/crates/benilla/assets/ui/ActionBar.xml index 81a4fd7d..5165366b 100644 --- a/crates/benilla/assets/ui/ActionBar.xml +++ b/crates/benilla/assets/ui/ActionBar.xml @@ -217,11 +217,20 @@ button.hotkey:SetText(BENILLA_ACTIONBAR_HOTKEYS[index]) end table.insert(BENILLA_ACTION_BUTTONS, button) + -- BOTH mouse buttons, ref-verbatim (ActionButton_OnLoad, ActionButton.lua:108-109): + -- an action button is a two-button button. The OnClick body reads no `arg1` — in the + -- ref either button runs the same PickupAction/UseAction fork — so right-click USES + -- the action, which is how food and spells are used off the bar. Without this the + -- widget default ({"LeftButtonUp"}, `benilla_ui::script::button`) silently swallowed + -- every right-click on the bar (director's report B200). + button:RegisterForClicks("LeftButtonUp", "RightButtonUp") -- The drag trio (decision 0216 §3/§7): click-carry and drag share one downstream path -- by construction (OnClick/OnDragStart both call PickupAction; OnReceiveDrag calls - -- PlaceAction — ref ActionBarFrame.xml:12-38). LOCK_ACTIONBAR stays the vanilla + -- PlaceAction — ref ActionBarFrame.xml:12-38). Both buttons drag, ref-verbatim too + -- (l.108) — a started drag suppresses the click, so right-press-and-move relocates the + -- action while right-click-in-place still uses it. LOCK_ACTIONBAR stays the vanilla -- default (unlocked), so unlike the ref's cvar guard, drag is always live here. - button:RegisterForDrag("LeftButton") + button:RegisterForDrag("LeftButton", "RightButton") BenillaActionButton_Update(button) end diff --git a/crates/benilla/assets/ui/BagFrame.xml b/crates/benilla/assets/ui/BagFrame.xml index 0b70ecd7..13cbb99c 100644 --- a/crates/benilla/assets/ui/BagFrame.xml +++ b/crates/benilla/assets/ui/BagFrame.xml @@ -653,7 +653,12 @@ button.bagId = bagId button.icon = getglobal(button:GetName() .. "Icon") if BenillaActionBarArt_SeatAbove then BenillaActionBarArt_SeatAbove(button) end - button:RegisterForClicks("LeftButtonUp") + -- Both buttons (ref BagSlotButtonTemplate inherits PaperDollItemSlotButtonTemplate, and + -- its OnLoad calls PaperDollItemSlotButton_OnLoad — PaperDollFrame.lua:86 — so a bag-bar + -- slot registers the pair like every doll slot; MainMenuBarBagButtons.xml:78 does the + -- same for the backpack). BagSlotButton_OnClick reads no button: right-click opens the + -- bag, exactly as left does. + button:RegisterForClicks("LeftButtonUp", "RightButtonUp") -- Drag-to-equip (decision 0208 phase 1b's deferred bag-icon arm, now landed): the bar -- slot is a paper-doll equipment slot for bags. LEFT drag picks the equipped bag up; -- a drag/receive drops a carried bag in to equip it (PickupInventoryItem — the exact diff --git a/crates/benilla/assets/ui/BankFrame.xml b/crates/benilla/assets/ui/BankFrame.xml index 6da86bcd..c69c6f9d 100644 --- a/crates/benilla/assets/ui/BankFrame.xml +++ b/crates/benilla/assets/ui/BankFrame.xml @@ -181,6 +181,11 @@ function BenillaBankBagButton_OnLoad(button, id) button:SetID(id) button.icon = getglobal(button:GetName() .. "Icon") + -- Both buttons, ref-verbatim (BankItemButtonBagTemplate's OnLoad runs + -- BankFrameBagButton_OnLoad -> BankFrameBaseButton_OnLoad, BankFrame.lua:12); the vault + -- slots get the same pair through BenillaBagSlot_OnLoad. The OnClick below reads no + -- button — right-click places/opens exactly as left does. + button:RegisterForClicks("LeftButtonUp", "RightButtonUp") button:RegisterForDrag("LeftButton") end diff --git a/crates/benilla/assets/ui/ChatFrame.xml b/crates/benilla/assets/ui/ChatFrame.xml index de82f50e..fa11186e 100644 --- a/crates/benilla/assets/ui/ChatFrame.xml +++ b/crates/benilla/assets/ui/ChatFrame.xml @@ -500,7 +500,11 @@ - this:RegisterForClicks("LeftButtonDown", "LeftButtonUp") + + this:RegisterForClicks("LeftButtonDown", "LeftButtonUp", "RightButtonUp", "RightButtonDown") BenillaChatScrollButton_OnClick(this, arg2, "down", "igChatScrollDown") @@ -521,7 +525,8 @@ - this:RegisterForClicks("LeftButtonDown", "LeftButtonUp") + + this:RegisterForClicks("LeftButtonDown", "LeftButtonUp", "RightButtonUp", "RightButtonDown") BenillaChatScrollButton_OnClick(this, arg2, "up", "igChatScrollUp") diff --git a/crates/benilla/src/capture/mod.rs b/crates/benilla/src/capture/mod.rs index 9949832b..eaa2b014 100644 --- a/crates/benilla/src/capture/mod.rs +++ b/crates/benilla/src/capture/mod.rs @@ -102,7 +102,8 @@ pub(crate) use probe_rig::{rig_char_name_from_env, ProbeRigPlugin}; pub(crate) use probe_taxi::ProbeTaxiPlugin; pub(crate) use probes::{ EntityCensusPlugin, LiveFpsPlugin, NodeProbePlugin, ParticleCensusPlugin, ProbeChatPlugin, - ProbeClock, ProbeExitPlugin, ProbeKeyPlugin, ProbeLuaPlugin, ProbeResizePlugin, + ProbeClock, ProbeExitPlugin, ProbeFocusPlugin, ProbeKeyPlugin, ProbeLuaPlugin, + ProbeResizePlugin, }; use scenarios::{Scenario, SubjectKind, UiFixture, GROUND_EYE, SCENARIOS}; diff --git a/crates/benilla/src/capture/probes.rs b/crates/benilla/src/capture/probes.rs index b1bda145..4fda18e4 100644 --- a/crates/benilla/src/capture/probes.rs +++ b/crates/benilla/src/capture/probes.rs @@ -93,6 +93,32 @@ struct ProbeChat { /// controller ever saw it. pub(crate) struct ProbeKeyPlugin; +/// Keep a probe run's window **un-occludable** — the one defence against macOS's ~1 fps +/// throttle for a fully covered window (decisions 0713/0777, method.md's `caffeinate` note). +/// +/// It used to live inside the FPS probe alone, which reads as "a frame-rate concern". It is not: +/// **every** scripted probe schedule is wall-clock ([`ProbeClock`]), so a throttled run doesn't +/// just measure slowly, it *executes the wrong script* — this session's mounted-jump run fired +/// `W@16` and `Space@19` in the SAME frame at ~1 fps, i.e. it jumped from a standstill instead of +/// mid-run, and the leg had to be re-read to notice (decision 0906). Any probe env arms it, so a +/// key/chat/Lua probe defends itself exactly like the FPS one. Write-gated: re-marking `Window` +/// every frame would re-apply its whole state through winit. +pub(crate) struct ProbeFocusPlugin; + +impl Plugin for ProbeFocusPlugin { + fn build(&self, app: &mut App) { + app.add_systems(Update, keep_probe_window_on_top); + } +} + +fn keep_probe_window_on_top(mut windows: Query<&mut Window, With>) { + if let Ok(mut w) = windows.single_mut() { + if w.window_level != bevy::window::WindowLevel::AlwaysOnTop { + w.window_level = bevy::window::WindowLevel::AlwaysOnTop; + } + } +} + impl Plugin for ProbeKeyPlugin { fn build(&self, app: &mut App) { let spec = std::env::var("WOW_PROBE_KEY").unwrap_or_default(); @@ -651,18 +677,8 @@ fn drive_live_fps( match probe.phase { LiveFpsPhase::Done => {} LiveFpsPhase::Waiting => { - // A probe window that can't be covered can't be occlusion-throttled: macOS drops - // any FULLY covered window to ~1 fps drawables, and a detached probe launch spawns - // unfocused, possibly entirely behind other windows. Asserted from the FIRST tick, - // not at the uncap: an occluded SETTLE phase streams the world at ~1 fps and - // under-warms the scene before sampling even starts. Probe-only — a normal run - // never enters this system. (Write-gated: re-marking `Window` every frame would - // re-apply its whole state through winit.) - if let Ok(mut w) = windows.single_mut() { - if w.window_level != bevy::window::WindowLevel::AlwaysOnTop { - w.window_level = bevy::window::WindowLevel::AlwaysOnTop; - } - } + // (The un-occludable window that keeps the SETTLE phase from streaming the world at + // ~1 fps is [`ProbeFocusPlugin`]'s now — every probe needs it, not just this one.) if time.elapsed_secs() < probe.at || self_player.is_empty() { return; } diff --git a/crates/benilla/src/creature_anim.rs b/crates/benilla/src/creature_anim.rs index 8e40f154..643e6ec7 100644 --- a/crates/benilla/src/creature_anim.rs +++ b/crates/benilla/src/creature_anim.rs @@ -612,6 +612,15 @@ pub(crate) struct AnimDriver { /// the *level* must not (mid-air the airborne-freeze issues no plays, so a fast-path park /// survives to its clip's end). last_special: Option, + /// The **airborne snapshot** node whose clock [`driver::play::leave_special`] stopped + /// (decision 0503 — the client blends out of a cut airborne clip from a frozen pose, not from + /// a still-running one). The per-frame rate write + /// ([`driver::play::sync_base_rate`](driver::play)) must not restart it, so it is named here + /// rather than inferred from a zero speed — a rate-scaled clip legitimately reads 0 when the + /// body is standing still, and skipping *that* would strand it frozen forever. Self-clearing: + /// the moment anything else is armed the node stops being the main animation and the sync + /// drops the name (decision 0906). + frozen: Option, } impl AnimDriver { @@ -702,6 +711,7 @@ impl Default for AnimDriver { loop_window: None, gait_rate: 1.0, last_special: None, + frozen: None, } } } diff --git a/crates/benilla/src/creature_anim/driver.rs b/crates/benilla/src/creature_anim/driver.rs index d3fffa27..735f1261 100644 --- a/crates/benilla/src/creature_anim/driver.rs +++ b/crates/benilla/src/creature_anim/driver.rs @@ -303,6 +303,8 @@ pub(super) fn drive_animations( // 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, + // …and whether the rider is US, which only the debug trace reads (decision 0906). + Has, )>, mut swings: MessageReader, mut impacts: MessageReader, @@ -335,7 +337,7 @@ pub(super) fn drive_animations( mut rng: Local, // The SELF unit's last-written anim state line of the `WOW_MOVE_TRACE` debug trace (the // diff-only filter; see the trace block after the mode machine). - mut anim_trace_last: Local>, + mut anim_trace_last: Local>, ) { let (emote_sounds, loot_latch, time) = aux; let dt = time.delta_secs(); @@ -430,26 +432,47 @@ pub(super) fn drive_animations( // 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. // 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, - ), - }; + // + // `ridden_by_self` only feeds the debug trace: while mounted, the animation the director is + // *looking at* is the mount child's (the rider is pinned to Mount 91), so a trace that + // followed `SelfPlayer` alone went blind on exactly the body in question — which is how a + // gallop cadence report had to be diagnosed from asset bytes instead of a run + // (decision 0906). It deliberately does NOT feed `is_self`: the sheath reconcile and the + // wire echoes below mean the player's OWN unit, and a mount is not it. + let ( + spline, + remote, + movement, + speeds, + facing_step, + creature_swimming, + host_scale, + ridden_by_self, + ) = match mount_body { + Some(mb) => match mount_hosts.get(mb.host) { + Ok((s, r, m, sp, f, sw, t, host_self)) => { + (s, r, m, sp, f, sw, t.scale.x, host_self) + } + Err(_) => (None, None, None, None, None, false, 1.0, false), + }, + None => ( + spline, + remote, + movement, + speeds, + facing_step, + creature_swimming, + 1.0, + false, + ), + }; // 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 traced = is_self || ridden_by_self; + // Which body a trace line is about — the rider's own, or the mount it is sitting on. + let subject = if mount_body.is_some() { "mount " } else { "" }; 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 — @@ -458,9 +481,9 @@ pub(super) fn drive_animations( // Mounted (decision 0441): `UNIT_FIELD_MOUNTDISPLAYID` nonzero — the wire's one mounted // signal. The rider's base pins to Mount(91) below, Specials/one-shot full-body routes are // suppressed, and the sheath reconcile force-stows; the locomotion the selector would have - // picked plays on the MOUNT child entity instead (its own driver pass, fed the host's - // movement view by `sync_mount_motion`). A mount child itself has no `ObjectStore`, so it - // can never read as mounted here. + // picked plays on the MOUNT child entity instead (its own driver pass, fed the host's own + // movement view through `MountBody.host` — the `mount_hosts` fetch at the top of this + // loop). A mount child itself has no `ObjectStore`, so it can never read as mounted here. let mounted = store.is_some_and(|s| s.0.unit_mount_display_id() != 0); // First time we drive this unit (nothing chosen yet) — used to settle a corpse to its end pose. let first = drv.gait.is_none() && drv.mode == Mode::Gait; @@ -962,6 +985,7 @@ pub(super) fn drive_animations( } else if special != Some(sp) { // What we're entering is no longer wanted before the enter even finished — preempt // to the new Special, to the gait (a pose cut by movement), or to this one's exit. + let drv = &mut *drv; // two disjoint field borrows below (window + frozen) drv.mode = leave_special( sp, special, @@ -974,6 +998,7 @@ pub(super) fn drive_animations( catalog, &mut rng, &mut drv.loop_window, + &mut drv.frozen, ); } else if oneshot_finished(&player, anims, sp.enter(), catalog) { play( @@ -993,6 +1018,7 @@ pub(super) fn drive_animations( } Mode::Looping(sp) => { if special != Some(sp) { + let drv = &mut *drv; // two disjoint field borrows below (window + frozen) drv.mode = leave_special( sp, special, @@ -1005,6 +1031,7 @@ pub(super) fn drive_animations( catalog, &mut rng, &mut drv.loop_window, + &mut drv.frozen, ); } } @@ -1083,6 +1110,7 @@ pub(super) fn drive_animations( if incoming_locomotion { transplant_up(&mut drv, &mut player, &tr, anims, entity, id); } + let drv = &mut *drv; // two disjoint field borrows below (window + frozen) drv.mode = if let Some(sp) = under { leave_special( sp, @@ -1096,6 +1124,7 @@ pub(super) fn drive_animations( catalog, &mut rng, &mut drv.loop_window, + &mut drv.frozen, ) } else if let Some(sp) = special { enter_special( @@ -1206,17 +1235,9 @@ pub(super) fn drive_animations( // pins further down the chain (the stationary cast hold, the loot kneel, the // Ready/ranged idles, the state-emote idle) cannot swap the clip until // touchdown. Rate stays synced — the client's per-frame rate write is outside - // the selector. (DEATH is excluded: the dead-override owns that gait, and a + // the selector, so it is [`play::sync_base_rate`]'s below and this arm does + // nothing at all. (DEATH is excluded: the dead-override owns that gait, and a // 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(rate); - drv.gait_rate = rate; - } - } - } } else { // A bracket-less step-off fall landing needs NO case of its own: the arc never // latched FALLINGFAR, so the `0x602c60` land dispatcher is a verified no-op @@ -1336,16 +1357,11 @@ pub(super) fn drive_animations( .find_map(|&id| find_resolved(anims, id, catalog)); if drv.gait == Some(target) { if let Some(c) = clip { - // The armed clip may be a *rolled variation* of the resolved head - // (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(rate); - drv.gait_rate = rate; - } - } + // The gait is already armed and stays armed — its rate is + // [`play::sync_base_rate`]'s per-frame write below, which finds + // whichever *rolled variation* (decision 0123) is actually the live + // one rather than sweeping every node of the id. + // // One completed pass of the ranged Load clip arms the HOLD twin // (the `ranged_load` map above) — next frame the idle cross-fades // to the drawn hold instead of replaying the nock. @@ -1370,14 +1386,14 @@ pub(super) fn drive_animations( // watchdog window). A re-armed Stand landing on its rare look-around // variations IS the idle fidget. let (c, budget) = roll_loop(anims, c, relaxed, &mut rng); - if is_self && crate::dbg_trace::enabled() { + if traced && crate::dbg_trace::enabled() { // Every fresh gait play, including a same-clip replay (which the // settled-state diff below cannot see) — the exact restart-from-head // event a "frames snap" report is hunting. crate::dbg_trace::line( "anim", &format!( - "PLAY gait {} (was {:?}) rate {:.2}", + "{subject}PLAY gait {} (was {:?}) rate {:.2}", c.anim_id, drv.gait, playback_rate(c, mv.speed, model_scale) @@ -1431,6 +1447,17 @@ pub(super) fn drive_animations( } } + // ── The base slot's **playback rate**, written once per frame over whatever the mode + // machine settled on — the client's rate write sits OUTSIDE the selector (`0x5fe2f0`), so + // it is not the gait's private business (decision 0906). The landing is why it matters: + // JumpLandRun 187 resolves to **Run(5)** on every creature model's baked lookup, so a + // mount's or a travel form's land clip is its own gallop cycle and must be rate-scaled + // like any other locomotion — at the arm's literal 1× it ran ~35% slow for the clip's + // whole 0.8 s. It also records the result in `gait_rate` — the number the hover card and + // the trace's `rate=` read (decision 0903) — so those stay honest in every mode too. See + // [`play::sync_base_rate`]. + play::sync_base_rate(&mut drv, &tr, &mut player, anims, mv.speed, model_scale); + // ── Masked one-shot completion — the **fade-to-rest** (decision 0878, correcting 0087 (c)): // a finished overlay is NOT stopped. The client latches the completion in its per-frame // model advance (`0x719370`) and delivers a deferred event the same frame to @@ -1529,10 +1556,20 @@ pub(super) fn drive_animations( // settled key-bone state: without it a torso report ("the cast got cut") could not be told // from a base one at all (decision 0878). `+fade` marks the 150 ms fade-to-rest running — // a boolean, not a countdown, so a fade doesn't spam a line per frame. - if is_self && crate::dbg_trace::enabled() { + // + // `base=` is the **resolved** clip the full-body slot actually holds — not the id the + // selector *asked* for, which `gait=`/`mode=` already carry. A cadence report ("the run + // goes slow when I land") is unreadable without it: the request was JumpLandRun 187, the + // clip playing was Run 5, and `rate=` was the wrong number for exactly that clip + // (decision 0906). `?` = the slot holds a node this model has no clip record for. + if traced && crate::dbg_trace::enabled() { + let base = tr + .get_main_animation() + .and_then(|n| anims.clips.iter().find(|c| c.node == n)) + .map_or_else(|| "?".to_string(), |c| c.anim_id.to_string()); let state = format!( - "mode={:?} gait={:?} special={:?} flags={:08x} speed={:.2} scale={:.2} \ - rate={:.2} upper={:?}{}", + "{subject}mode={:?} gait={:?} base={base} special={:?} flags={:08x} \ + speed={:.2} scale={:.2} rate={:.2} upper={:?}{}", drv.mode, drv.gait, special, @@ -1550,9 +1587,9 @@ pub(super) fn drive_animations( "" }, ); - if anim_trace_last.as_deref() != Some(&state) { + if anim_trace_last.get(&entity) != Some(&state) { crate::dbg_trace::line("anim", &state); - *anim_trace_last = Some(state); + anim_trace_last.insert(entity, state); } } diff --git a/crates/benilla/src/creature_anim/driver/play.rs b/crates/benilla/src/creature_anim/driver/play.rs index 7a34a937..a36d6f0c 100644 --- a/crates/benilla/src/creature_anim/driver/play.rs +++ b/crates/benilla/src/creature_anim/driver/play.rs @@ -10,7 +10,7 @@ use bevy::animation::transition::AnimationTransitions; use bevy::animation::RepeatAnimation; use bevy::prelude::*; -use super::super::find_resolved; +use super::super::{find_resolved, AnimDriver}; use super::select::{self, jump_land_pick, Mode, Special}; /// Cross-fade the player into an **already-resolved** clip over its blend-in time. `repeat` sets its @@ -127,6 +127,63 @@ pub(super) fn play( } } +/// Write the **playback rate** of whatever the full-body slot currently holds — run once per frame, +/// after the mode machine has settled, in every mode alike. +/// +/// The client's rate write lives **outside the selector** (`0x5fe2f0`, per-frame over the armed +/// clip), so it is not the gait's private business: it applies to a landing, a bracket, a swing — +/// anything the base slot holds. Ours used to be two loops inside [`Mode::Gait`]'s arms, which left +/// every other mode playing at its arm-time literal `1.0` — and that is the jump-landing bug +/// (decision 0906). [`jump_land_pick`] requests JumpLandRun **187**, and **every creature model +/// resolves 187 → Run(5)** through its own baked PlayableAnimationLookup: Horse, Tiger (the druid +/// travel form) and Cat all carry `playable[187] = 5`; only character models author 187 itself. So +/// a mount's landing clip *is* its gallop cycle — a rate-scaled locomotion clip — and playing it at +/// 1× ran it at ~65% of the cadence a mount's 14 yd/s calls for (Horse Run moveSpeed 9.028) for the +/// clip's whole 0.8 s, snapping straight only when the gait re-picked after it. On foot the same +/// defect is invisible: the character's own 187 carries moveSpeed 6.944 against a run speed of 7.0, +/// so the correct rate *is* 1×. +/// +/// It writes **only where the scaler applies** ([`select::scaled_rate`]) — a locomotion clip with +/// an authored design speed. Everything else keeps whatever armed it: the scaler is one rate +/// producer among several (the combat fast-path's 2×, the whiff's 0.5×, 0503's 0× freeze), and a +/// blanket `1.0` here stomps all three. +/// +/// [`AnimDriver::frozen`] names the one node this must leave alone even so — the airborne snapshot +/// decision 0503 stopped on purpose ([`leave_special`]), whose clip *is* rate-scaled (Jump 38 is in +/// the locomotion set; it is only the real assets' `moveSpeed = 0` that would spare it) — and is +/// cleared as soon as anything else is armed. +/// +/// It also records what the slot ended up running at in [`AnimDriver::gait_rate`] — the hover +/// card's `rate` readout and the trace's `rate=` (decision 0903). Read back off the node rather +/// than recomputed, so the instrument reports the swing's 2× or the freeze's 0× as faithfully as +/// it reports a gait. +pub(super) fn sync_base_rate( + drv: &mut AnimDriver, + tr: &AnimationTransitions, + player: &mut AnimationPlayer, + anims: &ModelAnimations, + speed: f32, + model_scale: f32, +) { + let Some(node) = tr.get_main_animation() else { + return; + }; + if drv.frozen != Some(node) { + drv.frozen = None; + if let Some(rate) = anims + .clips + .iter() + .find(|c| c.node == node) + .and_then(|c| select::scaled_rate(c, speed, model_scale)) + { + if let Some(active) = player.animation_mut(node) { + active.set_speed(rate); + } + } + } + drv.gait_rate = player.animation(node).map_or(1.0, |a| a.speed()); +} + /// Whether the one-shot clip `id` has finished playing (resolved through the model's own baked /// fallback first, decision 0082 — matching [`play`], which is what started it) — or the model lacks /// even the substitute, so the machine doesn't wait forever. Checked across the id's **variations** @@ -214,6 +271,7 @@ pub(super) fn leave_special( catalog: Option<&AnimDataCatalog>, rng: &mut u32, window: &mut Option<(bevy::animation::graph::AnimationNodeIndex, u32)>, + frozen: &mut Option, ) -> Mode { // The client blends OUT of a cut airborne clip from a **pose snapshot** — op4 `0x7121a0` // with blendFlag≠0 copies the outgoing pose to `+0xc4` and *decays the frozen pose* under @@ -225,12 +283,14 @@ pub(super) fn leave_special( // the outgoing node reproduces the snapshot-decay. Scoped to the airborne cut, where the // divergence is visible; the client's snapshot law is universal (every blended arm), and // adopting it for all cross-fades is 0503's recorded follow-up. + // …and the frozen node is NAMED (decision 0906), so the per-frame rate write + // ([`sync_base_rate`]) skips it instead of restarting the clock a line above just stopped. if matches!(sp, Special::Jump | Special::Fall) { - if let Some(active) = tr - .get_main_animation() - .and_then(|n| player.animation_mut(n)) - { - active.set_speed(0.0); + if let Some(node) = tr.get_main_animation() { + if let Some(active) = player.animation_mut(node) { + active.set_speed(0.0); + *frozen = Some(node); + } } } if let Some(next) = special { diff --git a/crates/benilla/src/creature_anim/driver/tests.rs b/crates/benilla/src/creature_anim/driver/tests.rs index 4d52deff..94a8638e 100644 --- a/crates/benilla/src/creature_anim/driver/tests.rs +++ b/crates/benilla/src/creature_anim/driver/tests.rs @@ -2225,6 +2225,46 @@ fn walker_model() -> ModelAnimations { } } +/// A **mount's** model, as the real assets are shaped (decision 0906): Horse.m2 authors no +/// JumpLandRun 187 at all, and its baked PlayableAnimationLookup answers a 187 request with +/// **Run(5)** — `playable[187] = 5`, the same row Tiger.m2 (the druid travel form) and Cat.m2 +/// carry. The Run clip's authored design speed is the horse's real 9.028 yd/s. +fn mount_model() -> ModelAnimations { + let mut run = clip(5, 2, true); + run.move_speed = 9.028; // Horse.m2 sequence 16's ModelAnimation::move_speed + let mut table = vec![ + benilla_formats::PlayableAnim { + resolved_id: 0, + dir_flags: 0, + }; + 203 + ]; + for id in [0u16, 5, 37, 38, 39] { + table[id as usize].resolved_id = id; + } + // A mount authors neither Sprint 143 (what the selector picks at mount speed) nor + // JumpLandRun 187 (what the landing picks): its baked table answers BOTH with the gallop + // cycle, so the whole galloping-jump-landing sequence is one clip at three rates. + table[143].resolved_id = 5; + table[187].resolved_id = 5; + ModelAnimations { + graph: Handle::default(), + clips: vec![ + clip(0, 1, true), + run, + clip(37, 3, false), // JumpStart + clip(38, 4, true), // Jump hang + clip(39, 5, false), // JumpEnd + ], + hand_close: [None, None], + playable_animation_lookup: table, + 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 { @@ -2324,3 +2364,87 @@ fn a_mounts_gait_rate_composes_the_riders_scale_with_the_mounts() { walk_rate(&app, mount) ); } + +/// The director's report (decision 0906): "jumping while running forward, mounted or in druid +/// travel form, slows the running animation on landing and it takes ~1-2 s to snap back". +/// +/// The landing request is JumpLandRun 187; on every creature model that resolves to **Run(5)**, +/// a rate-scaled locomotion clip — so the land clip IS the gallop cycle and must run at +/// `speed / moveSpeed` like the gait it continues. It used to be armed at the call site's literal +/// `1.0` and nothing rewrote it until `Mode::Land` ended, which is the clip's whole length of +/// visibly-slow legs. The rate write is per-frame and mode-independent now (`sync_base_rate`), so +/// the landing runs at the same rate the airborne gait did. +#[test] +fn a_mounted_landing_runs_at_the_gaits_rate_not_at_one_times() { + let mut app = app(); + let unit = app + .world_mut() + .spawn(( + mount_model(), + AnimationPlayer::default(), + AnimationTransitions::new(), + AnimDriver::default(), + Transform::from_scale(Vec3::splat(1.0)), + MovementState::default(), + )) + .id(); + // Galloping forward at a 100% mount's 14 yd/s: Run at 14 / 9.028 ≈ 1.551×. + let running = MovementState { + flags: move_flags::FORWARD, + speed: 14.0, + ..Default::default() + }; + app.world_mut().entity_mut(unit).insert(running); + app.update(); + let rate = |app: &App| { + app.world() + .entity(unit) + .get::() + .unwrap() + .animation(AnimationNodeIndex::new(2)) // the Run node + .map(|a| a.speed()) + }; + let expected = 14.0 / 9.028; + assert!( + rate(&app).is_some_and(|r| (r - expected).abs() < 1e-4), + "the gallop scales by speed: {:?} vs {expected}", + rate(&app) + ); + + // Up: the jump bracket takes the body (JumpStart, then the hang). + app.world_mut().entity_mut(unit).insert(MovementState { + flags: move_flags::FORWARD | move_flags::FALLING, + speed: 14.0, + vertical_speed: 7.9, + ..Default::default() + }); + app.update(); + assert!( + matches!( + drv_of(&app, unit).mode, + super::super::select::Mode::Entering(super::super::select::Special::Jump) + ), + "an upward launch enters the jump bracket" + ); + + // Touchdown, still holding forward: the land pick is 187 → the model plays Run for it. + app.world_mut().entity_mut(unit).insert(running); + app.update(); + assert_eq!( + drv_of(&app, unit).mode, + super::super::select::Mode::Land { + id: 187, + flags: move_flags::FORWARD + }, + "a forward landing picks JumpLandRun" + ); + assert!( + rate(&app).is_some_and(|r| (r - expected).abs() < 1e-4), + "the landing clip is the gallop cycle and runs at the gallop's rate: {:?} vs {expected}", + rate(&app) + ); +} + +fn drv_of(app: &App, unit: Entity) -> &AnimDriver { + app.world().entity(unit).get::().unwrap() +} diff --git a/crates/benilla/src/creature_anim/select.rs b/crates/benilla/src/creature_anim/select.rs index 4795eb92..cdff05bd 100644 --- a/crates/benilla/src/creature_anim/select.rs +++ b/crates/benilla/src/creature_anim/select.rs @@ -993,15 +993,18 @@ pub(super) fn gait_is_locomotion(state: &MovementState, walk_speed: f32) -> bool } /// 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`): +/// client's `0x5fe2f0` divide, VERIFIED byte-for-byte and §5 cross-checked (wow-5875-re +/// `anim-rate-divisor.md`, the canonical note; `0x5fe4be..0x5fe550`): /// /// ```text -/// 5fe508 call 0x711a20 ; DIVISOR = |modelScale| · sequence.moveSpeed +/// 5fe508 call 0x711a20 ; DIVISOR = ‖row0(M+0xbc)‖ · M2Sequence[reqId].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|) +/// 5fe541 fdiv ; rate = speed / (moveSpeed · scale) +/// +/// M = CGUnit+0xdc (the mount) if nonzero, else CGUnit+0xd8 (the body) +/// ‖row0‖ = (mounted ? CGUnit+0x9c : 1.0) · CGUnit+0x94 · CGUnit+0x90 /// ``` /// /// **`model_scale` is the half we were missing** (decision 0903). A sequence's `moveSpeed` is the @@ -1011,17 +1014,45 @@ pub(super) fn gait_is_locomotion(state: &MovementState, walk_speed: f32) -> bool /// 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. +/// The scale to pass is the **rendered world scale of the model actually playing the clip** — which +/// is what the two terms above are: `CGUnit+0x90` is `OBJECT_FIELD_SCALE_X` (the server has already +/// folded the DBC scale in — `entities::attach`), and `+0x9c` is a mount's own +/// `CreatureDisplayInfo` column, composed under the rider's. Our unit transform and mount-child +/// transform carry exactly those, so `transform.scale.x × host` reproduces the product (decision +/// 0910). Two exactness notes from the cross-check: +/// +/// - The client takes the **L2 length of row 0** of the model's world matrix, not an `abs()`. For a +/// *uniform* scale — which every unit of ours is (`Vec3::splat`) — the two are identical at any +/// rotation, since row 0 is `s · (a unit row)`. `abs()` is the cheaper spelling of the same +/// number, and is only a distinction if unit scale ever stops being uniform. +/// - `CGUnit+0x94`, a spell-visual scale multiplier clamped to `[0.75, 2.0]` (1.0 when idle), is a +/// **third** factor we do not carry — because we have no producer for it: the aura CharProc +/// dispatch (`crate::aura_visual::node_for`) implements Alpha/Tint/AnimRate and nothing that +/// scales a unit. Whoever builds that layer must multiply it in here, or a scaling spell visual +/// will re-time the gait wrongly for its duration. (`CGUnit+0x98`, the fourth slot, is a literal +/// constant 1.0 in the binary — its constructor is its only writer.) pub(super) fn playback_rate(clip: &AnimClip, speed: f32, model_scale: f32) -> f32 { + scaled_rate(clip, speed, model_scale).unwrap_or(1.0) +} + +/// [`playback_rate`] as the scaler's own question — **`Some` only where the scaler applies at +/// all**: both of `0x5fe2f0`'s guards passed (a positive divisor, a locomotion id). +/// +/// The distinction is what the per-frame rate write +/// ([`sync_base_rate`](super::driver::play::sync_base_rate)) needs, and it is not pedantry: the +/// scaler is one rate producer among several. The combat fast-path doubles an already-playing +/// swing to 2× (decision 0406, op6 `2.0f`), the whiff slows a missed one to 0.5×, decision 0503 +/// freezes an airborne snapshot to 0×. Each owns the clip it wrote, and a per-frame write that +/// returned a blanket `1.0` for "the guards failed" would silently stomp all three (it did — the +/// suite caught it, decision 0906). +/// The `abs()` is on the **scale only** — deliberately, and the asymmetry is the client's (decision +/// 0912). `‖row0‖` is a matrix-row length, so it is never negative; `moveSpeed` is *signed*, and a +/// backwards gait is authored negative (`RidingKodo.m2` WalkBackwards, `-2.5`). Guard A's strict +/// `> 0` therefore leaves an authored backwards clip at a flat 1× while a model that falls back to +/// forward Walk (`+2.5`) gets speed-scaled. Do not "tidy" this into `move_speed.abs()`. +pub(super) fn scaled_rate(clip: &AnimClip, speed: f32, model_scale: f32) -> Option { let divisor = clip.move_speed * model_scale.abs(); - if divisor > 0.0 && RATE_SCALED.contains(&clip.anim_id) { - speed / divisor - } else { - 1.0 - } + (divisor > 0.0 && RATE_SCALED.contains(&clip.anim_id)).then(|| speed / divisor) } /// Build the unified movement view, in precedence order: a populated [`MovementState`] (our avatar, diff --git a/crates/benilla/src/creature_anim/select/tests.rs b/crates/benilla/src/creature_anim/select/tests.rs index 0ab84bc1..08ea3015 100644 --- a/crates/benilla/src/creature_anim/select/tests.rs +++ b/crates/benilla/src/creature_anim/select/tests.rs @@ -414,6 +414,21 @@ fn a_degenerate_model_scale_falls_through_to_unit_rate() { assert!((playback_rate(&clip(4, 2.5), 4.0, -2.2) - 0.727_27).abs() < 1e-4); } +/// The SIGN of `moveSpeed` is load-bearing, and the `abs()` belongs to the scale ALONE (decision +/// 0912, §5-verified). A backwards gait is authored negative — `RidingKodo.m2` seq 14 is +/// WalkBackwards at **−2.5**, byte-read here with `benilla-extract m2seq` — and Guard A is a strict +/// `divisor > 0`, so the reference leaves that clip at a flat 1×. A model authoring NO +/// WalkBackwards falls back to forward Walk (+2.5) and *is* speed-scaled. Pinned because +/// `move_speed.abs()` is an inviting-looking tidy-up that would invert both halves at once. +#[test] +fn an_authored_backwards_gait_is_not_rate_scaled_but_its_fallback_is() { + // The authored clip (RidingKodo WalkBackwards, −2.5) — flat 1×, at any scale. + assert_eq!(playback_rate(&clip(13, -2.5), 4.5, 1.0), 1.0); + assert_eq!(playback_rate(&clip(13, -2.5), 4.5, 2.2), 1.0); + // …while a model lacking it substitutes forward Walk and scales normally. + assert!((playback_rate(&clip(13, 2.5), 4.5, 1.0) - 1.8).abs() < 1e-5); +} + #[test] fn ranged_load_idle_selects_by_weapon_and_ranks_below_ready() { // The 0x5fd530 LUT (0099 phase 5): ranged-slot subclass → the held Load/Hold clip. diff --git a/crates/benilla/src/liquid/surface.rs b/crates/benilla/src/liquid/surface.rs index 137790e0..668c9ba9 100644 --- a/crates/benilla/src/liquid/surface.rs +++ b/crates/benilla/src/liquid/surface.rs @@ -336,6 +336,15 @@ pub(super) fn setup_liquid( alpha_mode, cull_mode: None, double_sided: true, + // The transparent water kinds take the fixed water-pass slot: below every + // unclassified world transparent, above the far-side effects (the reference's + // 0x483460 interleave — `sky_order::WATER_BIAS`, where the byte story lives). + // Opaque kinds (magma/slime) draw in the opaque pass and take no rung. + depth_bias: if kind.is_fullbright() { + 0.0 + } else { + crate::sky_order::WATER_BIAS + }, ..default() }, extension: LiquidExt { diff --git a/crates/benilla/src/main.rs b/crates/benilla/src/main.rs index 0bbf8c10..9c172800 100644 --- a/crates/benilla/src/main.rs +++ b/crates/benilla/src/main.rs @@ -742,6 +742,25 @@ fn main() -> AppExit { if std::env::var("WOW_RIG").is_ok() { app.add_plugins(capture::ProbeRigPlugin); } + // Any scripted probe keeps its window un-occludable: a fully covered macOS window drops to + // ~1 fps drawables, and every probe schedule is wall-clock — a throttled run doesn't measure + // slowly, it runs the wrong script (see `capture::ProbeFocusPlugin`, decision 0906). + // (`WOW_LIVE_FPS` is in the list because an occluded SETTLE phase streams the world at ~1 fps + // and under-warms the scene before sampling even starts — the assertion has to be live from + // the first tick, not at the uncap.) + if [ + "WOW_PROBE", + "WOW_PROBE_CHAT", + "WOW_PROBE_KEY", + "WOW_PROBE_LUA", + "WOW_RIG", + "WOW_LIVE_FPS", + ] + .iter() + .any(|k| std::env::var(k).is_ok()) + { + app.add_plugins(capture::ProbeFocusPlugin); + } // The probe-chat one-shot: `WOW_PROBE_CHAT=".go xyz …"` sends GM/chat lines once in-world — // the "park the probe character anywhere" instrument (see `capture::ProbeChatPlugin`). if std::env::var("WOW_PROBE_CHAT").is_ok() { diff --git a/crates/benilla/src/particles.rs b/crates/benilla/src/particles.rs index 04b81805..ece27702 100644 --- a/crates/benilla/src/particles.rs +++ b/crates/benilla/src/particles.rs @@ -38,6 +38,9 @@ mod sim; use emit::{emit_local, next_u32, rand01, rand_s11}; use sim::simulate_particles; +// The water-plane interleave classification — shared with the ribbon sim (a trail is one of the +// model's emitters and classifies the same way; `sky_order::EFFECT_FAR_SIDE_BIAS`). +pub(crate) use sim::{far_side_of_water, WaterInterleave}; /// Hard cap on a single emitter's live particle count — a backstop against a pathological model. Real /// props sit far under this (a campfire's steady state is `rate·lifespan` ≈ 30 + 24 particles). diff --git a/crates/benilla/src/particles/sim.rs b/crates/benilla/src/particles/sim.rs index a3a946f2..1b4f8213 100644 --- a/crates/benilla/src/particles/sim.rs +++ b/crates/benilla/src/particles/sim.rs @@ -175,6 +175,65 @@ pub(super) struct SceneGates<'w> { camera_claim: Res<'w, crate::wmo_portal::CameraInteriorClaim>, } +/// The **water-plane interleave** inputs ([`crate::sky_order::EFFECT_FAR_SIDE_BIAS`], where the +/// byte story lives): the loaded liquid surfaces, the eye's submersion verdict, and the room +/// components an anchor's liquid claim resolves from. One `SystemParam` for the same 16-cap +/// reason as [`SceneGates`]. +#[derive(bevy::ecs::system::SystemParam)] +pub(crate) struct WaterInterleave<'w, 's> { + water: Query<'w, 's, &'static crate::liquid::WaterChunkInfo>, + underwater: Res<'w, crate::liquid::Underwater>, + rooms: Query<'w, 's, &'static crate::wmo_portal::UnitWmoRoom>, + parents: Query<'w, 's, &'static ChildOf>, +} + +/// Is this emitter's cloud on the EYE's **far side** of its local water plane? +/// +/// The reference classifies every particle emitter onto exactly ONE of the two transparent +/// lists — above-water or below-water — by its own position against the owning model's liquid +/// hit (`0x7084a0`), and the frame interleave draws the far-side list *before* the water pass +/// (dry eye: below-water is far; submerged: above-water is — `0x4836d6`). This is that +/// classification: the anchor against the nearest **admitted** surface over its XY, the claim +/// resolved from the nearest ancestor with a room verdict (a joint owner sits under its unit; +/// `Unknown` when nothing carries one — it admits both sources, still floor-bounded per pool). +/// No admitted surface → near side, the reference's pass-1 default. +pub(crate) fn far_side_of_water( + w: &WaterInterleave, + claim_seed: Option, + anchor_world: Vec3, +) -> bool { + let wow = benilla_assets::coords::bevy_to_wow(anchor_world); + let mut seed = claim_seed; + let mut room = None; + for _ in 0..8 { + let Some(e) = seed else { break }; + if let Ok(r) = w.rooms.get(e) { + room = Some(r); + break; + } + seed = w.parents.get(e).ok().map(ChildOf::parent); + } + let claim = crate::liquid::unit_claim(room); + let Some(d) = crate::liquid::surfaces_at(w.water.iter(), wow, claim) + .map(|z| wow[2] - z) + .min_by(|a, b| a.abs().total_cmp(&b.abs())) + else { + return false; + }; + far_side(d, w.underwater.0.any()) +} + +/// The side law alone, pure for the unit test: `d` = anchor − surface (WoW Z), `submerged` = +/// the eye's state. Dry eye ⇒ the below-water list is far; submerged ⇒ the above-water one. +/// `d = 0` sits with the above-water list, matching [`crate::player::camera`]'s waterline tie. +fn far_side(d: f32, submerged: bool) -> bool { + if submerged { + d >= 0.0 + } else { + d < 0.0 + } +} + /// Per-frame: emit, integrate, and expand each emitter's pool into the shared effect-quad /// stream ([`super::buffer::EffectQuads`]). #[allow(clippy::too_many_arguments, clippy::type_complexity)] // one Bevy system's full input set @@ -184,6 +243,8 @@ pub(super) fn simulate_particles( // The draw-set gate's scene inputs — the far-clip wall (0678) and the exterior window test // (0786); see [`SceneGates`]. gates: SceneGates, + // The water-plane interleave inputs — see [`WaterInterleave`] and [`far_side_of_water`]. + interleave: WaterInterleave, mut commands: Commands, cam: Query<(Entity, &GlobalTransform, &Frustum, &Camera, &Projection), With>, // Owner reads (joints/units/roots — never emitter or child-draw entities): disjoint from @@ -417,6 +478,10 @@ pub(super) fn simulate_particles( model_instances, gated: _, } = &mut *emitter; + // The water-interleave claim seed, captured before the draw-anchor local shadows the + // `anchor` field below: the owner (creature/GameObject/joint) when streamed, else the + // cloud anchor (the MODEL) — whichever exists is where the room walk starts. + let claim_seed = (*owner).or(*anchor); *age += dt; // Anchored mode (see [`Particle`]): positions are emitter-relative, so tracking a moving // owner needs nothing beyond refreshing `placement` — the cloud rides the anchor for free @@ -824,8 +889,37 @@ pub(super) fn simulate_particles( // render via [`super::model::update_model_particles`]. let want_quads = def.geometry_model.is_none() && images.contains(&*texture); // Every effect of one model takes the SAME rung (0719/0721) — computed here per frame, - // where it used to be baked into the material's `depth_bias` at spawn. - let bias = super::owner_last_bias(*owner_reach); + // where it used to be baked into the material's `depth_bias` at spawn. Plus the + // water-plane interleave ([`far_side_of_water`]): a cloud on the eye's far side of its + // local water plane drops under the water pass, so the surface paints over it — the + // swimming-enchant erase was these two sorting by raw view-z and flipping with camera + // angle. Booth emitters belong to their own camera and take no world rung. + // Classified at the EMITTER's own live position (`placement`), not the model anchor — + // the reference's grain (`0x7084a0` reads the emitter): a swimmer's body sits below the + // line while their shoulder emitter sits above it, and the model-anchor shortcut would + // put the visible glow on the wrong list. + let far = !is_booth && far_side_of_water(&interleave, claim_seed, placement.translation); + // The classification's own trace (`WOW_MOVE_TRACE_TAGS=fx`): which clouds dropped under + // the water pass this frame — the numeric read for "the swimmer's enchant survives the + // surface", where a pixel can't say which side a draw sorted to. + if far && !particles.is_empty() && crate::dbg_trace::enabled() { + crate::dbg_trace::line( + "fx", + &format!( + "far-side cloud at=[{:.1},{:.1},{:.1}] n={}", + placement.translation.x, + placement.translation.y, + placement.translation.z, + particles.len() + ), + ); + } + let bias = super::owner_last_bias(*owner_reach) + + if far { + crate::sky_order::EFFECT_FAR_SIDE_BIAS + } else { + 0.0 + }; let start = quads.begin(); if want_quads && !particles.is_empty() { expand_quads(def, particles, &frame, placement, &cam, &mut quads.verts); @@ -906,9 +1000,25 @@ pub(super) fn simulate_particles( #[cfg(test)] mod tests { - use super::{inherit_trigger, integrate_particle, ChildEmitter, Particle, StepEnv, Vec3}; + use super::{ + far_side, inherit_trigger, integrate_particle, ChildEmitter, Particle, StepEnv, Vec3, + }; use bevy::prelude::{Quat, Transform}; + /// The water-plane interleave's side law (`0x4836d6`): dry eye ⇒ the below-water list is the + /// far one; submerged ⇒ the above-water list is. `d = 0` sits with the above-water list. + #[test] + fn far_side_follows_the_eye() { + // Dry eye: below the surface is far (drawn before the water), above is near. + assert!(far_side(-0.5, false)); + assert!(!far_side(0.5, false)); + assert!(!far_side(0.0, false)); + // Submerged: the sides swap. + assert!(far_side(0.5, true)); + assert!(far_side(0.0, true)); + assert!(!far_side(-0.5, true)); + } + fn particle(pos: Vec3, vel: Vec3) -> Particle { Particle { pos, diff --git a/crates/benilla/src/player/movement_net.rs b/crates/benilla/src/player/movement_net.rs index 1c546034..f1998e40 100644 --- a/crates/benilla/src/player/movement_net.rs +++ b/crates/benilla/src/player/movement_net.rs @@ -10,7 +10,7 @@ //! what we send verbatim and observers extrapolate it from the moveFlags, so any divergence strands them //! on stale state: a flag we set but never clear is a *phantom* walk/spin, and an out-of-range value is //! silently dropped before relay (vmangos rejects `|orientation| > 4π` in `VerifyMovementInfo`, -//! regardless of anticheat). Two rules keep us honest, both enforced here at the wire boundary: +//! regardless of anticheat). Three rules keep us honest, all enforced here at the wire boundary: //! - **Every outbound `orientation` is normalized into `[0, 2π)`** — `face_yaw`/`cam.yaw` are unbounded //! accumulators, but the real client always sends a normalized facing and the server's validity gate //! demands it. @@ -18,6 +18,12 @@ //! Stop and clears our flags on entering free-fly; the held frames of a post-teleport settle stream //! zeroed flags (so [`stream_self_movement`]'s own diff emits the Stop) — so observers never //! extrapolate motion that isn't happening locally. +//! - **The server's copy of our *position* may never go stale** — the reconcile at the bottom of +//! [`stream_self_movement`] (decision 0907). A resting body whose resolver settles it a fraction +//! of a millimetre after the packet that reported the rest used to keep that to itself, and +//! vmangos — which compares positions with exact float equality — read the next packet's +//! accumulated delta as movement and cancelled the cast in flight. Drift at rest is news; it goes +//! out the frame it happens. use benilla_assets::coords::bevy_to_wow; use benilla_protocol::{JumpInfo, TransportPose}; @@ -70,6 +76,18 @@ const OUTBOUND_FLAG_MASK: u32 = move_flags::FORWARD | move_flags::HOVER | move_flags::ON_TRANSPORT; +/// The bits that mean **the body is genuinely in motion**, and so that its position is expected to +/// change every frame: the direction bits, the airborne arc, swimming, and riding a transport. The +/// position reconcile at the bottom of [`stream_self_movement`] fires only when NONE of them is set +/// — at rest, a position change is news; in motion it is the whole point of the transition + +/// heartbeat stream (0052/0053), which already carries it. Turning in place is deliberately absent: +/// a keyboard turn moves nothing, so a drift under it is still news. +const IN_MOTION: u32 = move_flags::ANY_MOVE + | move_flags::FALLING + | move_flags::FALLING_FAR + | move_flags::SWIMMING + | move_flags::ON_TRANSPORT; + /// This frame's **arc edges** — the airborne lifecycle as the send law reads it. One struct rather /// than four adjacent bools in the argument list, where a miscount is silent and the symptom is a /// wrong opcode on the wire. @@ -177,6 +195,9 @@ pub(super) fn stream_self_movement( jump: wire_jump, transport, }); + // The ack's `MovementInfo` relocates the mover server-side exactly like a Move packet, so + // it is also a position report — the reconcile below must not re-send what it just told. + player.last_pos = wow_pos; } let prev = player.move_flags; let added = wire_flags & !prev; @@ -210,6 +231,9 @@ pub(super) fn stream_self_movement( transport, }); sent = true; + // What the server now believes, for the reconcile below — every packet in this family + // carries `wow_pos`, and vmangos relocates the mover to it (`HandleMoverRelocation`). + player.last_pos = wow_pos; }}; } const FB: u32 = move_flags::FORWARD | move_flags::BACKWARD; @@ -329,6 +353,31 @@ pub(super) fn stream_self_movement( if !sent && wire_flags != 0 && !falling && now - player.last_heartbeat >= HEARTBEAT_INTERVAL { send_move!(MoveKind::Heartbeat); } + // ── The position reconcile (decision 0907) ── **the server's copy of where we are may never go + // stale.** Our resolver settles a body that is already at rest by a fraction of a millimetre + // *after* the packet that reported the rest: a landing reports the touchdown pose and the next + // frame's step-down snap takes 2e-5 yd off it; a login/teleport lands on a server-authored + // position our own collision resolves a hair differently. Until 0617 nothing carried that + // difference, and while standing still nothing else goes out — so the client and the server + // silently held two different positions, and the next packet of ANY kind delivered the whole + // accumulated delta at once. + // + // vmangos reads that delta as movement, on an **exact float compare**: `Player::SetPosition` + // sets `positionChanged = old_x != x || old_y != y || old_z != z` and hands it to + // `Unit::HandleInterruptsOnMovement`, which runs `InterruptSpellsWithInterruptFlags( + // SPELL_INTERRUPT_FLAG_MOVEMENT)`. No epsilon: 2e-5 yd cancels a cast exactly as a yard would. + // (Verified in the deployed server's source, `Objects/Player.cpp` + `Objects/Unit.cpp`; the + // 0.5-yd tolerance elsewhere in `Spell::update` is a different, later test.) + // + // The observable was the director's: right-dragging to look around killed a cast. Mouse-look is + // the one thing that puts packets on the wire while standing still (SET_FACING at frame + // cadence), so it was the messenger — the first one after a run, a jump, or a login carried a + // stale-position "you moved" into the middle of the cast. Reporting the drift when it *happens* + // — at rest, so once per settle rather than per frame — keeps the two copies identical, and a + // cast then dies to real movement only. + if !sent && wire_flags & IN_MOTION == 0 && wow_pos != player.last_pos { + send_move!(MoveKind::Heartbeat); + } if sent { player.last_heartbeat = now; } @@ -352,10 +401,12 @@ pub(super) fn park_mover(sender: &Sender, player: &mut Player) { return; } let facing = player.face_yaw.rem_euclid(std::f32::consts::TAU); + let pos = bevy_to_wow(player.pos); + player.last_pos = pos; // the park is a position report too (decision 0907's reconcile) let _ = sender.send(ClientCommand::Move { kind: MoveKind::Stop, flags: 0, - pos: bevy_to_wow(player.pos), + pos, orientation: facing, pitch: 0.0, // flags cleared → not swimming → no pitch tail written fall_time: 0, @@ -822,6 +873,86 @@ mod tests { assert_eq!(kinds, vec![MoveKind::StartStrafeRight, MoveKind::SetFacing]); } + /// One idle frame at `pos`, nothing else happening — the shape every reconcile test needs. + fn idle_frame(tx: &Sender, player: &mut Player, pos: bevy::prelude::Vec3) { + player.pos = pos; + stream_self_movement( + tx, + player, + player.move_flags, + 0.0, + ArcEdges { + jumped: false, + air_nudged: false, + landed: false, + started_falling: false, + fall_time: 0, + }, + 0.0, + &[], + None, + ); + } + + #[test] + fn a_resting_body_that_drifts_reports_it_once() { + // Decision 0907. Our resolver settles a body already at rest a fraction of a millimetre after + // the packet that reported the rest (a landing, a login, a teleport onto a server-authored + // pose), and standing still nothing else goes out — so the server's copy of our position went + // stale, and the next packet of any kind (in practice a right-drag's SET_FACING) delivered the + // whole delta at once. vmangos compares positions with EXACT float equality and reads any + // difference as movement, cancelling a movement-interrupt cast. So: at rest, a changed + // position IS the news, and one heartbeat carries it — then silence again. + let (tx, rx) = crossbeam_channel::unbounded(); + let mut player = Player::default(); + idle_frame(&tx, &mut player, bevy::prelude::Vec3::new(1.0, 2.0, 3.0)); + let ClientCommand::Move { + kind, flags, pos, .. + } = rx.try_recv().expect("the drift is reported") + else { + panic!("expected a Move command"); + }; + assert_eq!(kind, MoveKind::Heartbeat); + assert_eq!(flags, 0, "at rest — the reconcile invents no motion"); + assert_eq!(pos, bevy_to_wow(bevy::prelude::Vec3::new(1.0, 2.0, 3.0))); + // The same position again: the server's copy is current, so nothing goes out. + idle_frame(&tx, &mut player, bevy::prelude::Vec3::new(1.0, 2.0, 3.0)); + assert!( + rx.try_recv().is_err(), + "an unchanged resting position is silent — one packet per settle, not per frame" + ); + // A settle that is one float wide still counts: vmangos's compare has no epsilon. + idle_frame( + &tx, + &mut player, + bevy::prelude::Vec3::new(1.0, f32::from_bits(2.0f32.to_bits() - 1), 3.0), + ); + assert!( + rx.try_recv().is_ok(), + "a one-ULP settle is exactly what the server's exact compare would read as movement" + ); + } + + #[test] + fn the_reconcile_never_fires_while_the_body_is_in_motion() { + // In motion the position changes every frame BY DESIGN, and the transition + ~500 ms + // heartbeat stream is what carries it (0052/0053) — a per-frame reconcile there would be a + // packet flood, and the reference sends nothing of the kind. The gate is the motion mask, so + // a runner mid-stride and a body mid-arc are both silent between their own packets. + let (tx, rx) = crossbeam_channel::unbounded(); + for flags in [move_flags::FORWARD, move_flags::FALLING] { + let mut player = Player { + move_flags: flags, // already streaming this state: no transition this frame + ..Default::default() + }; + idle_frame(&tx, &mut player, bevy::prelude::Vec3::new(9.0, 9.0, 9.0)); + assert!( + rx.try_recv().is_err(), + "moving ({flags:#x}): the position rides the movement stream, not a reconcile" + ); + } + } + #[test] fn park_mover_flushes_a_stop_and_clears_stale_flags() { // We were last streaming FORWARD when the controller stopped driving us (free-fly). Parking must diff --git a/crates/benilla/src/player/state.rs b/crates/benilla/src/player/state.rs index f7f73c81..c06ba6a7 100644 --- a/crates/benilla/src/player/state.rs +++ b/crates/benilla/src/player/state.rs @@ -405,6 +405,16 @@ pub(crate) struct Player { /// streams a `MSG_MOVE_SET_FACING` that frame, moving or standing, so observers see us aim /// (decision 0617). Updated every frame whether or not a packet went out. pub(super) last_facing: f32, + /// The **position we last told the server** (WoW coords, exactly the floats that went on the + /// wire). Diffed against this frame's live position to catch a drift we would otherwise never + /// report: our resolver settles a resting body a fraction of a millimetre after the packet that + /// reported the rest (a landing, a login, a teleport onto a server-authored pose), and while + /// standing still nothing else goes out. vmangos compares an incoming position to its stored one + /// with EXACT float equality and interrupts a movement-interrupt cast on any difference, so a + /// stale copy turns the next packet — in practice the first `MSG_MOVE_SET_FACING` of a + /// right-drag — into a mid-cast "you moved". Decision 0907; the reconcile lives in + /// [`super::movement_net::stream_self_movement`]. + pub(super) last_pos: [f32; 3], /// The stand state we last volunteered (`CMSG_STANDSTATECHANGE`) whose echo into our /// `UNIT_FIELD_BYTES_1` hasn't landed yet — the local commit (the client's `SetStandState` /// `0x6127b0` applies immediately *and* sends; decision 0080c). `None` = at the echoed value. diff --git a/crates/benilla/src/ribbons.rs b/crates/benilla/src/ribbons.rs index 47f47987..056f4e29 100644 --- a/crates/benilla/src/ribbons.rs +++ b/crates/benilla/src/ribbons.rs @@ -195,6 +195,9 @@ pub(crate) fn simulate_ribbons( // Trails belong to the world lane (no booth ribbons; a booth-parked owner's strip is eaten // by the shader's farclip wall, exactly as on the material path). world_cam: Query>, + // The water-plane interleave inputs — a trail is one of the model's emitters and classifies + // above/below water like the quad clouds ([`crate::particles::far_side_of_water`]). + interleave: crate::particles::WaterInterleave, mut trails: Query<( Entity, &mut RibbonTrail, @@ -366,7 +369,14 @@ pub(crate) fn simulate_ribbons( // particle "unfogged" file flag — pass 0.) fog: EffectFog::for_blend(0, def.blend), anchor, - bias: *bias, + // The owner rung, dropped under the water pass when the head node sits on the + // eye's far side of its water plane — the same interleave the quad clouds take. + bias: *bias + + if crate::particles::far_side_of_water(&interleave, *owner, anchor) { + crate::sky_order::EFFECT_FAR_SIDE_BIAS + } else { + 0.0 + }, raster_bias: 0, main_entity: entity, light: None, // trails never carry a light override (world lane only) diff --git a/crates/benilla/src/sky_order.rs b/crates/benilla/src/sky_order.rs index 00a9dd3a..70be49ca 100644 --- a/crates/benilla/src/sky_order.rs +++ b/crates/benilla/src/sky_order.rs @@ -86,6 +86,24 @@ pub(crate) const WHITE_MOON_BIAS: f32 = -8.1e5; pub(crate) const MOON02_BIAS: f32 = -8.0e5; /// The cloud dome — last of the sky pass (`0x6d4a71`): clouds blend over a setting sun. pub(crate) const CLOUDS_BIAS: f32 = -6.0e5; +/// **Far-side model effects** — the water-plane interleave's early half (byte-VERIFIED, wow-re +/// `water-frame-straddle.md`): the reference splits M2 transparents into an above-water and a +/// below-water list per model (per **emitter** for particles — each classifies onto exactly ONE +/// list, `0x7084a0`), and `0x483460` draws the list on the eye's FAR side of the water plane +/// *before* the water pass, the near side *after*. An effect classified far-side takes this rung +/// (plus its owner-last rung, capped at 32) so it lands under [`WATER_BIAS`]; near-side effects +/// keep their natural slot above it. The flip on submersion is the interleave inversion +/// (`0x4836d6 cmp eax,0xf`). Sort-only — the effect lane splits sort bias from raster bias. +pub(crate) const EFFECT_FAR_SIDE_BIAS: f32 = -4.0e4; +/// **The water surface** — river/ocean/WMO water draw in a fixed frame slot between the two +/// transparent halves (`0x6701d0 → 0x6816d0`: ocean → river → WMO liquid → foam), never view-z +/// sorted against model transparents. One rung below the world band puts every unclassified +/// world transparent after the water (the reference's near-side default) and the far-side +/// effects before it. Magnitude: must clear the far plane (a few ×10³) so no world view-z can +/// reorder a water chunk across it, while staying small as a rasterizer bias (this one rides a +/// `StandardMaterial` base, so it IS also a depth-test bias — ~0.24% depth pull, half of what +/// the nameplates ship at +4e4; the water-noon/name-water capture diffs are the shoreline check). +pub(crate) const WATER_BIAS: f32 = -2.0e4; /// The sun/moon glare quads — the frame's last render (`0x483740` tail): over the clouds and the /// rain, under the nameplates; the z-buffer (their forced far depth, `celestial.wgsl`) is what /// occludes them. Only ~6× the far plane, not ~10⁵: a rung this far up is also a rasterizer bias @@ -102,6 +120,12 @@ const _: () = { assert!(SUN_DISC_BIAS - STARS_BIAS > 1.0e4); assert!(WHITE_MOON_BIAS > SUN_DISC_BIAS && MOON02_BIAS > WHITE_MOON_BIAS); assert!(CLOUDS_BIAS - MOON02_BIAS > 1.0e4); + // The water-plane interleave: sky < far-side effects < water < world transparents (the + // near-side default). The far-side band is far-effect-bias + owner rung; owner rungs are + // capped well under the 1e4 margin (benilla_formats::owner_last_rung's ceiling). + assert!(EFFECT_FAR_SIDE_BIAS - CLOUDS_BIAS > 1.0e4); + assert!(WATER_BIAS - EFFECT_FAR_SIDE_BIAS > 1.0e4); + assert!(-3.0e3 - WATER_BIAS > 1.0e4); // world view-z floor = −far (the ~3 km projection) stays above assert!(crate::ground_fx::GROUND_FX_DEPTH_BIAS - CLOUDS_BIAS > 1.0e4); assert!(GLARE_BIAS - crate::ground_fx::GROUND_FX_DEPTH_BIAS > 1.0e4); assert!(crate::nameplates::NAMEPLATE_DEPTH_BIAS - GLARE_BIAS > 1.0e4); diff --git a/crates/benilla/src/ui_action/drain.rs b/crates/benilla/src/ui_action/drain.rs index c985a85c..7bd35ba3 100644 --- a/crates/benilla/src/ui_action/drain.rs +++ b/crates/benilla/src/ui_action/drain.rs @@ -277,17 +277,27 @@ pub(super) fn drain_action_uses( // same function at `0x4e607b`), so a quest-starter on the bar offers its quest // instead of a `CMSG_USE_ITEM` the server can only refuse (decision 0664). The // wire's third byte is the spell BLOCK ordinal, not a flag (decision 0666). + // The fork carries the in-flight gate — an item use IS a cast through the same + // `TryCast` (decision 0908; [`crate::ui_items::send_item_use`] is the law), so + // the mount's second click no longer ships a duplicate the server bounces back + // as a cast-bar cancel. let spell_index = template.use_spell_index().unwrap_or(0); debug!( "ui_action: item action {action} use (wire {bag_index}/{slot0}, spell #{spell_index})" ); - let _ = commands.0.send(crate::ui_items::item_use_command( - Some(guid), - template.start_quest, - bag_index, - slot0, - spell_index, - )); + crate::ui_items::send_item_use( + crate::ui_items::ItemUse { + guid: Some(guid), + start_quest: template.start_quest, + bag_index, + slot: slot0, + spell_index, + use_spell: template.use_spell.map(|u| u.spell_id), + }, + &mut pending, + &mut errors.0, + &commands, + ); } } Some(b) => { diff --git a/crates/benilla/src/ui_cast.rs b/crates/benilla/src/ui_cast.rs index d7d3952c..aafacd6e 100644 --- a/crates/benilla/src/ui_cast.rs +++ b/crates/benilla/src/ui_cast.rs @@ -49,6 +49,19 @@ pub(crate) struct CastBarFeed(pub(crate) Vec); /// guard first, never reaches it. Generous on purpose — the resolution packets do the real clearing. const SEND_PROVISIONAL: Duration = Duration::from_secs(5); +/// The same provisional for an **item** use's cast ([`PendingCast::arm_item`]) — deliberately much +/// tighter, because the two arms have different worst cases *on this wire*. Every +/// `CMSG_CAST_SPELL` vmangos accepts is answered (`Spell::SendCastResult` on any rejection, +/// `SMSG_SPELL_START`/`GO` otherwise), so a spell's guard is always cleared by a packet and +/// [`SEND_PROVISIONAL`] never actually governs. `CMSG_USE_ITEM` has legs that answer with +/// `SMSG_INVENTORY_CHANGE_FAILURE` and **no cast result at all** — `HandleUseItemOpcode`'s +/// equip-error gates (`CanUseItem`, the in-combat non-combat-spell refusal, item-not-found) — +/// and there the provisional IS the safety net, so it has to be short enough that a silently +/// refused use cannot lock casting out for long. It only ever covers the send→`SMSG_SPELL_START` +/// window anyway: [`PendingCast::refine`] stretches the guard to the server's real cast time the +/// moment START names it, so a use that *does* become a cast is guarded for its whole length. +const ITEM_SEND_PROVISIONAL: Duration = Duration::from_millis(1_500); + /// Pushback slack added to the server's cast time at `SMSG_SPELL_START`, so a cast the server /// delays (`SMSG_SPELL_DELAYED`) still guards past its stretched end. const CAST_SLACK: Duration = Duration::from_secs(2); @@ -108,6 +121,21 @@ impl PendingCast { }); } + /// Arm the guard on an **item** use's send. The reference has no separate item-cast slot: the + /// item-use dispatcher's cast tail (`CGItem::Use 0x5d8d00` @ `0x5d9258`) calls `0x6e5a90`, + /// whose entire 54-byte body is `call 0x6e4b60` — `TryCast` itself (VERIFIED at the bytes, + /// wow-re `disasm-full.txt`; corroborated by its `action-button-state-api.md` §"dispatcher + /// `0x6e5a90` (→ `CastSpell 0x6e4b60`)" and `cursor-system.md` §536). So an item use writes + /// the SAME inflight id (`0xceca88`) and is refused by the same IsCasting gate — which is why + /// [`crate::ui_items::send_item_use`] runs both. Differs from [`PendingCast::arm`] only in the + /// deadline ([`ITEM_SEND_PROVISIONAL`], whose doc is the why). + pub(crate) fn arm_item(&mut self, spell_id: u32, now: Instant) { + self.0 = Some(PendingCastState { + spell_id, + deadline: now + ITEM_SEND_PROVISIONAL, + }); + } + /// Tighten the deadline to the server's real cast time once `SMSG_SPELL_START` names it. pub(crate) fn refine(&mut self, cast_time_ms: u32, now: Instant) { if let Some(p) = &mut self.0 { diff --git a/crates/benilla/src/ui_items/drain.rs b/crates/benilla/src/ui_items/drain.rs index b22d6356..7013b895 100644 --- a/crates/benilla/src/ui_items/drain.rs +++ b/crates/benilla/src/ui_items/drain.rs @@ -83,6 +83,8 @@ pub(super) fn drain_inventory_uses( mut items: ResMut, self_q: Query<&ObjectStore, With>, commands: Res, + mut pending: ResMut, + mut cast_errors: ResMut, ) { let Some(mut script) = script else { return; @@ -95,7 +97,7 @@ pub(super) fn drain_inventory_uses( let slot = (id - 1) as u8; // The doll's own slot ids ARE the wire slots (`wire_pos`'s EQUIPMENT_BAG law), so the // equipped instance resolves off the player's INV array directly. - let (guid, start_quest, spell_index) = self_q + let (guid, start_quest, spell_index, use_spell) = self_q .iter() .next() .and_then(|store| slot_guid(&store.0, EQUIPMENT_BAG, slot, &items)) @@ -104,17 +106,28 @@ pub(super) fn drain_inventory_uses( let t = items.template(entry, guid, &commands)?; // The wire's spell byte is a template BLOCK ordinal (decision 0666) — the // template is already in hand here for `start_quest`, so name the real one. - Some((Some(guid), t.start_quest, t.use_spell_index().unwrap_or(0))) + Some(( + Some(guid), + t.start_quest, + t.use_spell_index().unwrap_or(0), + t.use_spell.map(|u| u.spell_id), + )) }) - .unwrap_or((None, 0, 0)); + .unwrap_or((None, 0, 0, None)); debug!("ui_items: use equipped item, lua slot {id} (wire 255/{slot})"); - let _ = commands.0.send(super::item_use_command( - guid, - start_quest, - BAG_PLAYER_INVENTORY, - slot, - spell_index, - )); + super::send_item_use( + super::ItemUse { + guid, + start_quest, + bag_index: BAG_PLAYER_INVENTORY, + slot, + spell_index, + use_spell, + }, + &mut pending, + &mut cast_errors, + &commands, + ); } } @@ -128,6 +141,8 @@ pub(super) fn drain_container_uses( bank: Res, mut equip_sound: MessageWriter, mut item_text: ResMut, + mut pending: ResMut, + mut cast_errors: ResMut, ) { let Some(mut script) = script else { return; @@ -233,6 +248,7 @@ pub(super) fn drain_container_uses( // The wire's spell byte is a template BLOCK ordinal (decision 0666) — the // template is right here, so send the real one rather than assuming 0. spell_index: t.use_spell_index().unwrap_or(0), + use_spell: t.use_spell.map(|u| u.spell_id), unwraps_gift: t.unwraps_gift(inst_flags), opens_loot: t.opens_loot(), }) @@ -299,13 +315,19 @@ pub(super) fn drain_container_uses( "ui_items: quest-starter {:#x} offers quest {}", c.guid, c.start_quest ); - let _ = commands.0.send(super::item_use_command( - Some(c.guid), - c.start_quest, - bag_index, - wire_slot, - c.spell_index, - )); + super::send_item_use( + super::ItemUse { + guid: Some(c.guid), + start_quest: c.start_quest, + bag_index, + slot: wire_slot, + spell_index: c.spell_index, + use_spell: c.use_spell, + }, + &mut pending, + &mut cast_errors, + &commands, + ); continue; } // #6 — readable: an item INSTANCE carrying `ITEM_FIELD_ITEM_TEXT_ID` (a mail-made @@ -353,13 +375,19 @@ pub(super) fn drain_container_uses( // The tail: a plain use (food, potions, hearthstone). The quest arm already fired above, // so the shared fork's own quest leg is inert here by construction. debug!("ui_items: use item (lua bag {bag} → wire {bag_index}/{wire_slot})"); - let _ = commands.0.send(super::item_use_command( - clicked.map(|c| c.guid), - 0, - bag_index, - wire_slot, - clicked.map_or(0, |c| c.spell_index), - )); + super::send_item_use( + super::ItemUse { + guid: clicked.map(|c| c.guid), + start_quest: 0, + bag_index, + slot: wire_slot, + spell_index: clicked.map_or(0, |c| c.spell_index), + use_spell: clicked.and_then(|c| c.use_spell), + }, + &mut pending, + &mut cast_errors, + &commands, + ); } } @@ -378,6 +406,9 @@ struct Clicked { display_info_id: u32, start_quest: u32, spell_index: u8, + /// The template's on-use SPELL id — what the cast tail's in-flight guard is keyed on + /// (decision 0908); `None` = this item casts nothing. + use_spell: Option, /// `ItemInfo::unwraps_gift` for this instance — dispatcher arm #2. unwraps_gift: bool, /// `ItemInfo::opens_loot` for this template — dispatcher arm #8. diff --git a/crates/benilla/src/ui_items/mod.rs b/crates/benilla/src/ui_items/mod.rs index bd179f5e..ac5b6c71 100644 --- a/crates/benilla/src/ui_items/mod.rs +++ b/crates/benilla/src/ui_items/mod.rs @@ -493,6 +493,81 @@ pub(crate) fn item_use_command( } } +/// One resolved item-use click, as [`send_item_use`] needs it: the wire position, the two +/// template scalars the fork reads, and the on-use SPELL the cast tail is keyed on. +#[derive(Clone, Copy, Debug)] +pub(crate) struct ItemUse { + /// The live instance's guid (`None` = it didn't resolve; the fork falls through to a plain use). + pub(crate) guid: Option, + /// Template `StartQuest` — non-zero diverts to the quest offer, ahead of the cast tail. + pub(crate) start_quest: u32, + /// The wire position (`255` = the player array). + pub(crate) bag_index: u8, + /// The wire slot, 0-based. + pub(crate) slot: u8, + /// The template spell BLOCK ordinal the server should cast (decision 0666). + pub(crate) spell_index: u8, + /// The template's on-use SPELL id. `None` = this item casts nothing, so it never reaches the + /// reference's cast tail and takes neither the gate nor the arm. + pub(crate) use_spell: Option, +} + +/// **The item-use fork's cast tail** — [`item_use_command`] plus the one gate that makes an item +/// use a *cast* rather than a bare packet, and the only place a `CMSG_USE_ITEM` leaves benilla. +/// +/// The reference has no separate item-send path at all. `CGItem::Use 0x5d8d00`'s tail reaches +/// `0x5d9249`–`0x5d9258` and calls `0x6e5a90(item, spell, target)`, whose whole 54-byte body is +/// `call 0x6e4b60` — **`TryCast`**, the same function the spellbook, `/cast` and the action bar's +/// SPELL arm run (VERIFIED at the bytes in wow-re's `system/ui/scratch/disasm-full.txt`; +/// independently recorded in its `action-button-state-api.md` §"dispatcher `0x6e5a90` +/// (→ `CastSpell 0x6e4b60`)" and `cursor-system.md` §536). So an item use inherits TryCast's +/// **IsCasting refusal** (`0x6e3d30` over the inflight id `0xceca88`, byte-verified in +/// `wave-cast.md`): the same spell bails silently (`6e4d43`), a different one errors 0x61 +/// "Another action is in progress" — and never sends. +/// +/// Ours did not, and that was decision 0908's bug (director's report B200): double-clicking a +/// mount shipped a second `CMSG_USE_ITEM`, vmangos answered it `SPELL_FAILED_SPELL_IN_PROGRESS` +/// (`Spell::prepare`'s `IsNonMeleeSpellCasted` gate), and the failure — spell-id-keyed like every +/// reap, and naming the *same* spell as the running cast — red-faded the **running** cast's bar +/// while the original cast completed anyway. The spell arm had had this rung since the identical +/// spam-cancel bug ([`crate::ui_cast::PendingCast`]'s own doc); the item arm bypassed it because +/// it bypassed the ladder. One guard resource, every cast source arms and checks it — the +/// reference's one inflight id. +/// +/// Returns whether the use went out (`false` = refused by the gate), for the callers that log. +pub(crate) fn send_item_use( + it: ItemUse, + pending: &mut crate::ui_cast::PendingCast, + cast_errors: &mut crate::ui_action::CastErrors, + commands: &crate::net::NetCommands, +) -> bool { + let command = item_use_command( + it.guid, + it.start_quest, + it.bag_index, + it.slot, + it.spell_index, + ); + // The quest offer returns long before the cast tail in the reference too (arm #3 at + // `0x5d8dd2`, well ahead of `0x5d9249`) — an offer is not a cast, so no gate and no arm. Same + // for an item with no on-use spell: it never reaches the tail either. + let (crate::net::ClientCommand::UseItem { .. }, Some(spell)) = (&command, it.use_spell) else { + let _ = commands.0.send(command); + return true; + }; + let now = std::time::Instant::now(); + if pending.in_flight(now) { + if pending.current(now) != Some(spell) { + cast_errors.0.push((spell, 0x61)); + } + debug!("ui_items: item use (spell {spell}) suppressed — a cast is already in flight"); + return false; + } + let _ = commands.0.send(command); + pending.arm_item(spell, now); + true +} + /// The client's quality→color escape for an item link (`GetItemQualityColor`'s table) — shared by /// [`item_link`] and everything downstream of it (one table, no drifting twins). /// @@ -717,7 +792,10 @@ impl Plugin for UiItemsPlugin { #[cfg(test)] mod tests { - use super::{find_equip_slot, item_use_command, keyring_size, wire_pos, KEYRING_CONTAINER}; + use super::{ + find_equip_slot, item_use_command, keyring_size, send_item_use, wire_pos, ItemUse, + KEYRING_CONTAINER, + }; use crate::net::ClientCommand; use benilla_ui::script::EQUIPMENT_BAG; @@ -789,6 +867,119 @@ mod tests { )); } + /// The cast tail's in-flight gate (decision 0908) — the fix for the director's B200: an item + /// use is a cast through the same `TryCast` a spell press runs (`0x5d9258` → `0x6e5a90` → + /// `0x6e4b60`), so a second one while the first is still in flight never reaches the wire. + /// Before this, the duplicate `CMSG_USE_ITEM` drew `SPELL_FAILED_SPELL_IN_PROGRESS` naming the + /// *running* cast's own spell, which red-faded its bar while the cast completed anyway. + #[test] + fn a_second_item_use_mid_cast_never_reaches_the_wire() { + const HEARTHSTONE: u32 = 8690; + const MOUNT: u32 = 470; + let hearth = |use_spell| ItemUse { + guid: Some(0x4000_0000_0000_0001), + start_quest: 0, + bag_index: 255, + slot: 24, + spell_index: 0, + use_spell, + }; + let (tx, rx) = crossbeam_channel::unbounded(); + let net = crate::net::NetCommands(tx); + let mut pending = crate::ui_cast::PendingCast::default(); + let mut errors = crate::ui_action::CastErrors::default(); + + assert!(send_item_use( + hearth(Some(HEARTHSTONE)), + &mut pending, + &mut errors, + &net + )); + assert!( + matches!(rx.try_recv(), Ok(ClientCommand::UseItem { .. })), + "the first click goes out and arms the guard" + ); + + // The double-click: the same spell bails SILENTLY — the ref's `6e4d43` same-spell leg. + assert!(!send_item_use( + hearth(Some(HEARTHSTONE)), + &mut pending, + &mut errors, + &net + )); + assert!(rx.try_recv().is_err(), "no duplicate on the wire"); + assert!( + errors.0.is_empty(), + "the same spell's re-press is silent, not a red line" + ); + + // A *different* item mid-cast is the ref's `6e4d97` leg: reason 0x61, still no packet. + assert!(!send_item_use( + hearth(Some(MOUNT)), + &mut pending, + &mut errors, + &net + )); + assert!(rx.try_recv().is_err()); + assert_eq!( + errors.0, + vec![(MOUNT, 0x61)], + "\"Another action is in progress\"" + ); + } + + /// What the gate must NOT hold: the arms that return before the reference's cast tail. A + /// quest-starter offers its quest (arm #3, `0x5d8dd2`) and an item with no on-use spell never + /// reaches `0x5d9249` at all — neither is a cast, so neither is gated or arms the guard. + #[test] + fn the_gate_spares_the_arms_that_are_not_casts() { + let (tx, rx) = crossbeam_channel::unbounded(); + let net = crate::net::NetCommands(tx); + let mut pending = crate::ui_cast::PendingCast::default(); + let mut errors = crate::ui_action::CastErrors::default(); + let letter = 0x4000_0000_0000_0BAD_u64; + + // Arm the guard as if a hearthstone were casting. + pending.arm_item(8690, std::time::Instant::now()); + + assert!(send_item_use( + ItemUse { + guid: Some(letter), + start_quest: 373, + bag_index: 255, + slot: 23, + spell_index: 0, + use_spell: None + }, + &mut pending, + &mut errors, + &net, + )); + assert!( + matches!(rx.try_recv(), Ok(ClientCommand::QuestgiverQuery { .. })), + "an offer is not a cast — it goes out mid-cast" + ); + + assert!(send_item_use( + ItemUse { + guid: Some(letter), + start_quest: 0, + bag_index: 19, + slot: 4, + spell_index: 0, + use_spell: None + }, + &mut pending, + &mut errors, + &net, + )); + assert!( + matches!(rx.try_recv(), Ok(ClientCommand::UseItem { .. })), + "a spell-less item casts nothing, so the in-flight cast cannot block it" + ); + assert!(errors.0.is_empty()); + } + /// A dozen representative `InventoryType`s (decision 0208 phase 1b's own ask), spanning the /// single-slot rows, the two-slot rows, the weapon rows' MAINHAND/OFFHAND split, and the /// not-equippable rows — cross-checked against vmangos diff --git a/crates/benilla/src/ui_script/action_bar_tests.rs b/crates/benilla/src/ui_script/action_bar_tests.rs index bb3b84a1..b26859f7 100644 --- a/crates/benilla/src/ui_script/action_bar_tests.rs +++ b/crates/benilla/src/ui_script/action_bar_tests.rs @@ -246,6 +246,63 @@ fn state_feedback_drives_cooldown_checked_and_usable_through_the_xml() { assert!(s.errors().is_empty(), "script errors: {:?}", s.errors()); } +/// An action button is a TWO-button button (decision 0908; director's report B200: "I can't right +/// click food on my bar to eat it or right click spells"). The ref's `ActionButton_OnLoad` +/// registers `("LeftButtonUp", "RightButtonUp")` (ActionButton.lua:109) and its OnClick body reads +/// no `arg1` — so either button runs the same fork and right-click USES the action. The widget +/// default is `{"LeftButtonUp"}` (`benilla_ui::script::button::wants_click`), so without the +/// explicit registration the input path silently swallowed every right-click on the bar. Driven +/// through the real shipped XML and the real `mouse_button` path, which is the only place the +/// registration set is consulted. +#[test] +fn a_right_click_on_an_action_button_uses_the_action() { + let mut s = UiScript::new().unwrap(); + s.set_screen_size(1024.0, 768.0); + load_action_bar(&s); + + s.set_action( + 1, + Some(ActionSlot { + texture: Some("Interface\\Icons\\INV_Misc_Food_11".into()), + kind: 0x80, // an ITEM action — the food/mount case the report is about + action: 4540, + count: 5, + }), + ); + s.fire_event("PLAYER_ENTERING_WORLD", vec![]); + s.resolve(); + + // Button 1's center (geometry as in the tests above): (26, 22). + s.mouse_button(26.0, 22.0, "RightButton", true); + s.mouse_button(26.0, 22.0, "RightButton", false); + assert_eq!( + s.take_action_uses(), + vec![1], + "right-click queues the same UseAction a left-click does" + ); + + // The middle button is registered by neither the ref nor us: it stays swallowed, which is what + // proves the assertion above is the REGISTRATION and not the gate having been removed. + s.mouse_button(26.0, 22.0, "MiddleButton", true); + s.mouse_button(26.0, 22.0, "MiddleButton", false); + assert!( + s.take_action_uses().is_empty(), + "an unregistered button still reaches nothing" + ); + + // Shift+right-click picks up, exactly as shift+left does — the OnClick fork is button-blind. + s.set_modifiers(true, false, false); + s.mouse_button(26.0, 22.0, "RightButton", true); + s.mouse_button(26.0, 22.0, "RightButton", false); + s.set_modifiers(false, false, false); + assert!(s.take_action_uses().is_empty()); + assert!( + s.cursor_payload().is_some(), + "shift+right-click carries the action, like shift+left" + ); + assert!(s.errors().is_empty(), "script errors: {:?}", s.errors()); +} + /// Decision 0216 §7 (byte-verified 0218 §4) driven through the REAL shipped XML, not the engine /// unit tests directly — the modifier-key mirror gating `PickupAction` vs `UseAction`, end to end. #[test] diff --git a/crates/benilla/src/ui_script/bag_tests.rs b/crates/benilla/src/ui_script/bag_tests.rs index 3cd624fb..0e0d460b 100644 --- a/crates/benilla/src/ui_script/bag_tests.rs +++ b/crates/benilla/src/ui_script/bag_tests.rs @@ -270,10 +270,22 @@ fn bag_bar_buttons_light_while_their_bag_is_open() { ); // A bar-slot click reopens bag 2 and relights it (the click auto-toggle + the ref's - // re-derive tail agree here). - s.run("BenillaBagBarSlot_OnClick(BenillaBagBarSlot2)") + // re-derive tail agree here). Driven as a RIGHT-click through the real input path: the ref's + // BagSlotButtonTemplate inherits PaperDollItemSlotButtonTemplate, whose OnLoad registers + // ("LeftButtonUp", "RightButtonUp") — PaperDollFrame.lua:86 — and BagSlotButton_OnClick reads + // no button, so either one opens the bag. Ours registered LeftButtonUp only until 0908. + let (cx, cy): (f64, f64) = s + .eval( + "return (BenillaBagBarSlot2:GetLeft() + BenillaBagBarSlot2:GetRight()) / 2, \ + (BenillaBagBarSlot2:GetTop() + BenillaBagBarSlot2:GetBottom()) / 2", + ) .unwrap(); - assert!(checked(&mut s, "BenillaBagBarSlot2"), "bar click relights"); + s.mouse_button(cx as f32, cy as f32, "RightButton", true); + s.mouse_button(cx as f32, cy as f32, "RightButton", false); + assert!( + checked(&mut s, "BenillaBagBarSlot2"), + "a RIGHT-click on the bar slot reopens bag 2, exactly as a left one does" + ); // ESC's sweep closes everything → every ring dark. s.run("CloseAllWindows()").unwrap(); diff --git a/crates/benilla/src/ui_script/bank_tests.rs b/crates/benilla/src/ui_script/bank_tests.rs index c6397842..bfe191c7 100644 --- a/crates/benilla/src/ui_script/bank_tests.rs +++ b/crates/benilla/src/ui_script/bank_tests.rs @@ -360,6 +360,40 @@ fn purchase_flow_shows_popup_queues_the_intent_and_the_row_hides_when_full() { ); } +/// The bank's bag buttons take BOTH mouse buttons (decision 0908): the ref's +/// `BankItemButtonBagTemplate` OnLoad runs `BankFrameBagButton_OnLoad` → +/// `BankFrameBaseButton_OnLoad`, which registers `("LeftButtonUp","RightButtonUp")` +/// (BankFrame.lua:12), and `BankFrameItemButtonBag_OnClick` reads no button. Ours registered +/// nothing, so the widget default (`{"LeftButtonUp"}`) swallowed every right-click. Asserted on +/// the click sound the handler plays before any of its own forks — the one observable that does +/// not need a bag object fed into the slot. +#[test] +fn a_bank_bag_button_answers_the_right_button_too() { + let mut s = setup(); + s.set_bank(Some(BankState::default())); + s.fire_event("BANKFRAME_OPENED", vec![]); + s.resolve(); + let _ = s.take_sounds(); + + let (cx, cy): (f64, f64) = s + .eval( + "return (BenillaBankBagButton1:GetLeft() + BenillaBankBagButton1:GetRight()) / 2, \ + (BenillaBankBagButton1:GetTop() + BenillaBankBagButton1:GetBottom()) / 2", + ) + .unwrap(); + s.mouse_button(cx as f32, cy as f32, "RightButton", true); + let consumed = s.mouse_button(cx as f32, cy as f32, "RightButton", false); + assert!(consumed, "the right-click lands on the button"); + assert_eq!( + s.take_sounds(), + vec![benilla_ui::script::SoundRequest::KitName( + "BAGMENUBUTTONPRESS".into() + )], + "a right-click runs the same OnClick a left-click does" + ); + assert!(s.errors().is_empty(), "click errors: {:?}", s.errors()); +} + /// Test 6 — Clicking item slot 1 with an empty cursor queues a `PickupContainerItem(-1, 1)` — read off /// the cursor's own resulting payload (`bag_tests`' pattern: drive the real click through /// `mouse_button`, not a bare `run()`, so the XML's `OnClick`/`RegisterForClicks` wiring is diff --git a/crates/benilla/src/water_fx/mod.rs b/crates/benilla/src/water_fx/mod.rs index fb283dd2..5ada97d0 100644 --- a/crates/benilla/src/water_fx/mod.rs +++ b/crates/benilla/src/water_fx/mod.rs @@ -197,8 +197,12 @@ struct FoamAssets { wake: Handle, } -/// The foam draw's sort-bias over its water (the old material's `depth_bias: 1.0`). -const FOAM_BIAS: f32 = 1.0; +/// The foam draw's sort-bias: **just over its water**, riding the water-pass rung. The reference +/// draws foam inside the water group, right after the liquid surfaces (`0x6816d0`: ocean → river +/// → WMO liquid → foam) — so when the water pass moved to its fixed frame slot +/// ([`crate::sky_order::WATER_BIAS`]), the foam moved with it: still +1 over the surface for the +/// coplanar tie, now *below* the near-side world transparents like the reference's. +const FOAM_BIAS: f32 = crate::sky_order::WATER_BIAS + 1.0; /// Load the stencils raw (CLAMP, no mips — the reference's measured sampler state). fn setup_water_fx(