From 73826ca3d751a7d74792d4ca463b2d1d1d9c2bd4 Mon Sep 17 00:00:00 2001 From: Sam Date: Tue, 25 Aug 2026 18:50:40 +0700 Subject: [PATCH] casting a hunter shot starts Auto Shot, and the ping wears its real art MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Serpent Sting starts Auto Shot in play: the chain built earlier the same day never fired because the in-flight record was armed only for the cast classes it refuses on, and every hunter shot is a ranged attack — the reference records every commit and discriminates at the gate; two tests now run the real ladder and reproduce the shipped symptom - the minimap ping is MinimapPing.mdx, measured: three additive quads — a 12 px spinner on the model's global clock, a static core, and a ring expanding 3→32 px on an 833 ms loop — the interim art was ~3× too big and missing the ring the eye actually reads - a mouse release goes to the frame that took the press and nothing else — the reference's up handler reads the capture alone and never the hover frame, so a press over open space delivers no release to whatever the cursor wandered onto (corrects the morning's fallback) - `WOW_MOVE_TRACE_TAGS=use` traces a right-click end to end (the cursor decision, the use packet, the spell-go answer), and the first headless hover capture parks the cursor on a chat tab so the reveal can be measured against the bare scene Co-Authored-By: Claude Fable 5 --- crates/benilla-app/assets/ui/ChatFrame.xml | 14 +- crates/benilla-app/assets/ui/HonorFrame.xml | 4 +- .../benilla-app/assets/ui/MinimapCluster.xml | 12 +- crates/benilla-app/src/capture/fixtures.rs | 61 ++++ crates/benilla-app/src/capture/scenarios.rs | 16 + crates/benilla-app/src/char_select/addons.rs | 3 +- crates/benilla-app/src/minimap/mod.rs | 40 ++- crates/benilla-app/src/minimap/ping.rs | 327 +++++++++++++++--- crates/benilla-app/src/net/apply/spells.rs | 24 +- crates/benilla-app/src/target/click.rs | 28 ++ crates/benilla-app/src/ui_action/cast_send.rs | 123 ++++++- crates/benilla-app/src/ui_cast.rs | 69 +++- crates/benilla-ui/src/script/pointer.rs | 46 ++- crates/benilla-ui/src/script/tests/input.rs | 97 ++++++ crates/benilla-ui/src/widget/kinds/mod.rs | 5 +- 15 files changed, 753 insertions(+), 116 deletions(-) diff --git a/crates/benilla-app/assets/ui/ChatFrame.xml b/crates/benilla-app/assets/ui/ChatFrame.xml index 3135ccd0..178365a9 100644 --- a/crates/benilla-app/assets/ui/ChatFrame.xml +++ b/crates/benilla-app/assets/ui/ChatFrame.xml @@ -1640,10 +1640,16 @@ So it rides a frame the dock cannot hide. Mounting it on BOTH windows instead would work only while exactly one of them is ever visible — an invariant the dock happens to hold - today and that a second visible window would turn into a double-speed fade. The reference - offers no shape to copy here: 1.12.1 ships `FCF_OnUpdate` DEFINED and never referenced - (0 hits across all of FrameXML), because vanilla's tabs do not hover-fade at all — the - reveal is benilla's (0288), so its mounting point is ours to get right. + today and that a second visible window would turn into a double-speed fade. + + **A frame the dock cannot hide is the reference's own answer**, and this comment used to + claim the opposite: that 1.12.1 ships `FCF_OnUpdate` defined and never referenced, so + vanilla's tabs never hover-fade and the reveal was benilla's own (0288). That is wrong. + `UIParent.xml` l.19 calls `FCF_OnUpdate(arg1)` from **UIParent's** ``, in the + same list as `UIFrameFadeUpdate`/`UIFrameFlashUpdate` — never from a chat frame, for the + reason above. A 1x1 frame parented to UIParent is that mounting point re-expressed; the + reasoning was right and only its premise was invented. The hover reveal, the 0.2 s + stationary delay and the 1.0/0.5 selected/unselected tab alphas are all the reference's. `ChatFrame_OnUpdate` stays on the windows, where it belongs: it reads `this` and blinks that frame's own bottom button. --> diff --git a/crates/benilla-app/assets/ui/HonorFrame.xml b/crates/benilla-app/assets/ui/HonorFrame.xml index f9df161d..24eb3c56 100644 --- a/crates/benilla-app/assets/ui/HonorFrame.xml +++ b/crates/benilla-app/assets/ui/HonorFrame.xml @@ -46,8 +46,8 @@ 1. THE GLOBALSTRINGS ARE NAMED, NEVER COPIED. Older windows here carry an `X = X or ""` fallback so a chainless harness still paints words; this page ships no such copy for its own labels, because three of them are the - `NEWBIE_TOOLTIP_*` paragraphs and shipping Blizzard's prose is exactly what decision - 1234 forbids. The section labels reach the player's own strings through the loader's + `NEWBIE_TOOLTIP_*` paragraphs and shipping Blizzard's prose was what decision 1234 + forbade (retired by 1602; the lookup stays because it is the better design). The section labels reach the player's own strings through the loader's `text=` resolution (a `text=` attribute is a GlobalStrings LOOKUP, `loader::resolve_text`); the row labels and tooltip bodies reach them as plain globals. A harness with no install behind it therefore paints a key where a label goes and raises if you hover a diff --git a/crates/benilla-app/assets/ui/MinimapCluster.xml b/crates/benilla-app/assets/ui/MinimapCluster.xml index 313d4a75..965adc10 100644 --- a/crates/benilla-app/assets/ui/MinimapCluster.xml +++ b/crates/benilla-app/assets/ui/MinimapCluster.xml @@ -24,10 +24,14 @@ flaky, and no corpus addon touches any of that surface (measured: 0 of 218 for MiniMapPing / Minimap_SetPing / MINIMAP_PING / PingLocation / GetPingPosition). - OUT (no wire / later arc slices, per 0203): meeting stone and battlefield buttons; - the ping MODEL (the reference's MiniMapPing is a on MinimapPing.mdx — ours is a flat - stand-in at an INTERIM size, `minimap::ping::PING_PX`); the rotate-minimap CVar. The day/night indicator now lives in - its own GameTime.xml (the reference's own file split), loaded right after this file. The + OUT (no wire / later arc slices, per 0203): meeting stone and battlefield buttons. The ping + MODEL is not out any more and never needed to be: the reference's MiniMapPing is a on + MinimapPing.mdx, and 1600 re-expresses its three drawn quads as flat animated sprites at the + model's own byte-measured sizes and tracks. (The "rotate-minimap CVar" this list used to name + does not EXIST in 5875 — a 214-site CVar::Register census found no such knob, and the address + it was pinned on, 0xceaa60, is the inside-a-WMO flag. Nothing to omit.) The day/night indicator + now lives in its own GameTime.xml (the reference's own file split), loaded right after this + file. The reference's minimapPlayerModel/.mdx arrow renders app-side from its .blp for now (0203 flags the simplification). The reference's on the cluster is dropped (it only enlarges the mouse-blocking hull; the loader models the tag now, so restore it if it ever diff --git a/crates/benilla-app/src/capture/fixtures.rs b/crates/benilla-app/src/capture/fixtures.rs index 09e8139c..179bcd8f 100644 --- a/crates/benilla-app/src/capture/fixtures.rs +++ b/crates/benilla-app/src/capture/fixtures.rs @@ -1218,6 +1218,67 @@ pub(super) fn seed_ui_fixture( warn!("capture: ui-chatedit seed failed: {e}"); } } + UiFixture::ChatTabHover => { + let Some(script) = script.as_mut() else { + return; + }; + // Lines so the window has content, the Combat Log selected, then the cursor parked + // on the GENERAL tab — the unselected-tab hover the director reported. + // + // A capture CAN hover: the app only pumps the OS cursor into the VM on a non-synthetic + // run (`ui_script::input`'s `.filter(|_| !ui_hidden && !synthetic)`), so a fed + // `mouse_move` is what the whole run then sees. That is the hover instrument decision + // 0254 wrote down as missing ("No capture scenario exercises a hover state, so nothing + // in the harness covers the very texture this decision is about") — and 0254's own + // named residual is exactly what this scenario turned out to show. + for (text, r, g, b) in [ + ("[One] says: hello northshire", 1.0, 1.0, 1.0), + ("[One] says: a second line to stack", 1.0, 1.0, 1.0), + ] { + script.add_chat_message("ChatFrame1", text, r, g, b); + } + // `$WOW_TABHOVER` picks which of the five dock states to shoot. It is an A/B rig, + // not a setting: the four alternates are what let a change to the composite be read + // as a difference rather than judged from one frame. Default "1" is the reported + // case, so the golden is stable without the variable. + // 1 (default) Combat Log selected, hovering GENERAL — the report + // 3 General selected, hovering COMBAT LOG — its mirror + // 2 Combat Log selected, hovering COMBAT LOG — hover on the SELECTED tab + // 0 revealed, hovering neither tab — the no-glow control + // 9 cursor off the dock — bare scene, the baseline + // a tab quad is measured against + let mode = std::env::var("WOW_TABHOVER").unwrap_or_else(|_| "1".into()); + let select = if mode == "3" { 1 } else { 2 }; + if let Err(e) = script.run(&format!("BenillaFCF_TabClick({select})")) { + warn!("capture: ui-chat-tabhover select failed: {e}"); + } + script.resolve(); + let expr = match mode.as_str() { + "2" | "3" => "return ChatFrame2Tab:GetCenter()", + "0" => { + "return (ChatFrame2:GetLeft() + ChatFrame2:GetRight()) / 2, \ + (ChatFrame2:GetBottom() + ChatFrame2:GetTop()) / 2" + } + // 9: park far away — the dock conceals itself, so the tab band is bare scene. + "9" => "return 2000, 2000", + _ => "return ChatFrame1Tab:GetCenter()", + }; + let centre: Result<(f32, f32), _> = script.eval(expr); + match centre { + Ok((x, y)) => { + script.mouse_move(x, y); + // Settle the reveal ramp without waiting on the app's own frames. + for _ in 0..40 { + if let Err(e) = script.run("FCF_OnUpdate(0.05)") { + warn!("capture: ui-chat-tabhover pump failed: {e}"); + break; + } + script.resolve(); + } + } + Err(e) => warn!("capture: ui-chat-tabhover centre failed: {e}"), + } + } UiFixture::NameWater => { use benilla_protocol::messages::ObjectFields; // The synthetic self player at the eye (the reaction lookup reads its store, and the diff --git a/crates/benilla-app/src/capture/scenarios.rs b/crates/benilla-app/src/capture/scenarios.rs index a08df370..23062de2 100644 --- a/crates/benilla-app/src/capture/scenarios.rs +++ b/crates/benilla-app/src/capture/scenarios.rs @@ -106,6 +106,11 @@ pub(super) enum UiFixture { /// capture checks the header (left-flush, Say-white), the typed text past the live insets, /// and the three-piece input border. ChatEdit, + /// The chat dock **hovered**: revealed, the Combat Log selected, and the cursor parked on the + /// unselected GENERAL tab — the first capture scenario that exercises a hover state at all, + /// which decision 0254 recorded as the gap that left its own lane uncovered. `$WOW_TABHOVER` + /// swings it through the four alternate dock states (see `fixtures.rs`). + ChatTabHover, /// The social pane (`FriendsFrame`), opened through the live toggle — the instrument for B264 /// (decision 1298). Its `BenillaFriendsDropDown` host is declared with no anchors, exactly as /// the reference's own `FriendsDropDown` is; while an unpositioned owner stood in a zero rect @@ -986,6 +991,17 @@ pub(super) const ON_DEMAND: &[Scenario] = &[ minute: 720, ui: Some(UiFixture::ChatEdit), }, + // The hovered chat dock — the tab plate + its ADD highlight over the world, which is the + // pair the UI-over-world composite space decides. Run with + // `WOW_CAPTURE_UI=1 WOW_CAPTURE=ui-chat-tabhover`. + Scenario { + name: "ui-chat-tabhover", + map: MAP_AZEROTH, + eye: GROUND_EYE, + look: GROUND_LOOK, + minute: 720, + ui: Some(UiFixture::ChatTabHover), + }, // The social pane, for the stray-dropdown regression (B264, decision 1298). Run with // `WOW_CAPTURE_UI=1 WOW_CAPTURE=ui-social`. Scenario { diff --git a/crates/benilla-app/src/char_select/addons.rs b/crates/benilla-app/src/char_select/addons.rs index 19b6cb68..69deae6a 100644 --- a/crates/benilla-app/src/char_select/addons.rs +++ b/crates/benilla-app/src/char_select/addons.rs @@ -9,7 +9,8 @@ //! `UI-Character-ScrollBar` track, and the four-button bottom row in the reference's own order //! (Disable All · Enable All ··· Okay · Cancel). Geometry constants below are the authored //! numbers, resolved top-down into the 640×512 plate. Behaviour is **re-expressed natively** — -//! transcribing GlueXML is forbidden, not discouraged (the contract's FrameXML rule; 1234, 1260). +//! written under 1234's rule, which 1602 has since retired; the native form stays as a design +//! choice, not a law. //! //! ## What the reference does, and what we do //! diff --git a/crates/benilla-app/src/minimap/mod.rs b/crates/benilla-app/src/minimap/mod.rs index 4e1b8c92..dcff2739 100644 --- a/crates/benilla-app/src/minimap/mod.rs +++ b/crates/benilla-app/src/minimap/mod.rs @@ -274,11 +274,10 @@ struct MinimapAssets { /// (`Rotating-MinimapArrow.mdx`) the reference re-animates per blip source. See /// [`blips::RimArrow`] for the sequence→layer table and why there are four of them. rim_arrows: blips::RimArrowArt, - /// The ping marker's art, in draw order (decision 1596). The reference's `MiniMapPing` is a - /// `` frame — `Interface\MiniMap\Ping\MinimapPing.mdx`, XML `scale="0.4"`, 50×50 — - /// so this is the flat stand-in for it: the model's own `ping2`/`ping5` textures stacked, the - /// same pair the paused first version shipped. Empty = no art, and the ping draws nothing. - ping: Vec>, + /// The ping marker's three drawn layers — the flat re-expression of `MinimapPing.mdx`'s + /// coincident additive quads, at the model's own byte-measured sizes and animation + /// ([`ping::PingArt`], decisions 1596/1599). + ping: ping::PingArt, /// The unit-blip atlas (`Interface\Minimap\ObjectIcons`, five 32-px dot cells) — the /// quest-giver dots. object_icons: Option>, @@ -324,15 +323,28 @@ fn setup_minimap( } let object_icons = assets.sprite_texture("Interface\\Minimap\\ObjectIcons", &mut images); - let ping: Vec<_> = [ - "Interface\\Minimap\\Ping\\ping2", - "Interface\\Minimap\\Ping\\ping5", - ] - .into_iter() - .filter_map(|p| assets.sprite_texture(p, &mut images)) - .collect(); - if ping.is_empty() { - warn!("minimap: no Ping art — a minimap ping will place but draw nothing"); + // `ping6` is deliberately absent: the model carries it on two quads whose weight + // track is a single key of 0, so the client culls those batches before it reads their + // blend mode. Loading it would be loading art that never draws. + let ping = ping::PingArt { + ping5: assets.sprite_texture("Interface\\Minimap\\Ping\\ping5", &mut images), + ping2: assets.sprite_texture("Interface\\Minimap\\Ping\\ping2", &mut images), + ping4: assets.sprite_texture("Interface\\Minimap\\Ping\\ping4", &mut images), + }; + // Per layer, not just "none of them": the three do different jobs, and a silently + // absent `ping4` would cost the ring — the one thing the eye actually reads — while + // the marker still looked plausible (1203's rule, applied to art). + for (name, present) in [ + ("ping5", ping.ping5.is_some()), + ("ping2", ping.ping2.is_some()), + ("ping4", ping.ping4.is_some()), + ] { + if !present { + warn!( + "minimap: Interface\\Minimap\\Ping\\{name} missing — the ping marker \ + will draw without that layer" + ); + } } if mask.is_none() { warn!("minimap: MinimapMask.blp missing — the map will draw square"); diff --git a/crates/benilla-app/src/minimap/ping.rs b/crates/benilla-app/src/minimap/ping.rs index 0bcaccb2..67e4f0f2 100644 --- a/crates/benilla-app/src/minimap/ping.rs +++ b/crates/benilla-app/src/minimap/ping.rs @@ -53,28 +53,117 @@ const PING_HOLD: f32 = 5.0; /// The tail after the hold — the reference's `MINIMAPPING_FADE_TIMER`. const PING_FADE: f32 = 0.5; -/// **The reference's ping does not fade; it pops.** `Minimap_OnUpdate` writes -/// `SetAlpha(255 * (t / MINIMAPPING_FADE_TIMER))` — a 0..255 alpha handed to an API that clamps to -/// 0..1, so the marker stays fully opaque until `t` falls below `0.5/255` ≈ 2 ms and then vanishes -/// in one frame. That is what the real client looks like, so it is what we draw. It is a -/// *behaviour* of theirs, not a quirk of ours to hide: flip this to `false` for a real linear fade -/// and nothing else changes. +/// **The reference's ping does not fade; it pops** — now byte-confirmed, not inferred from the Lua +/// (wow-re `minimap-ping-law.md`). `Minimap_OnUpdate` writes `SetAlpha(255 * (t / 0.5))`, and +/// `Frame:SetAlpha` (`0x774e90`) **clamps to [0,1]** before scaling to a 0–255 byte — so the +/// nominal half-second ramp is at full alpha for 498 of its 500 ms and the whole visible fall is +/// ~12 % of one frame at 60 fps. The real client holds bright for ~5 s and vanishes. +/// +/// Reproduced as the clamp rather than as a constant 1.0, so the mechanism stays visible and the +/// `false` branch is a real alternative (a smooth fade, which wow-re notes will visibly diverge). const PING_ALPHA_IS_A_POP: bool = true; -/// The marker's on-screen size, as a fraction of the widget side on the client's byte-pinned -/// 140.8-px minimap basis — the same basis every other blip constant is frozen against -/// ([`super::blips::BLIP_BASIS_PX`]). +/// **The marker, byte-measured** (wow-re `system/ui/scratch/minimap-ping-law.md` §10, VERIFIED). /// -/// **INTERIM — an eyeball, not a measurement.** What the reference declares is verified (ref -/// `Minimap.xml` l.396): `MiniMapPing` is a `` frame on -/// `Interface\MiniMap\Ping\MinimapPing.mdx`, XML `scale="0.4"`, viewport 50×50. What that -/// *renders* at is not: the on-screen size is the model's own quad extent times the frame's -/// px-per-model-unit, the way [`super::blips::ARROW_QUAD_PX`] is 0.0500 · 768. Nobody has measured -/// this model's quad, so 40 px is the paused first version's eyeball carried forward unchanged — -/// deliberately, so the pin can be judged without the art also moving. Out for RE with the model's -/// animation; the rim arrows' precedent says a model like this is a plain textured quad, in which -/// case the flat stand-in becomes geometry-exact at the model's own size. -const PING_PX: f32 = 40.0; +/// `MiniMapPing` is a `` on `Interface\MiniMap\Ping\MinimapPing.mdx` at XML `scale="0.4"`, +/// which puts **1 model unit at 512 px** on the client's stock basis. The model is *five coincident +/// full-UV quads* sharing one centre, every one of them `blendMode = 4` — **additive** +/// `SRC_ALPHA/ONE` — with `flags = 0x0011` (unlit, no depth write). Three of them draw: +/// +/// | texture | model units | px | behaviour | +/// |---|---|---|---| +/// | `ping5` | `0.069 × 0.350634` | **12.39** | spins CW, one turn per 4833 ms, alpha 1 | +/// | `ping2` | `0.025` | **12.80** | fully static — its bone has no tracks at all | +/// | `ping4` | `0.00625 × (1→10)` | **3.20 → 32.00** | the expanding, fading ring | +/// +/// `ping6` (two of the five quads) is **culled, not invisible**: its weight track is a single key +/// of 0, so the batch is skipped before `blendMode` is read. It is not emitted here. +/// +/// Sizes are px on the frozen 140.8-px minimap basis, like every other blip constant +/// ([`super::blips::BLIP_BASIS_PX`]) — the model's px-per-unit and the widget's size both scale +/// with the same screen basis, so their ratio is the constant. +/// +/// The paused first version drew one flat 40 px stack: **~3× too big, and missing the ring** — +/// which is the motion the eye actually reads. +const PING5_PX: f32 = 0.069 * 0.350_634 * PX_PER_MODEL_UNIT; +const PING2_PX: f32 = 0.025 * PX_PER_MODEL_UNIT; +const PING4_PX: f32 = 0.006_25 * PX_PER_MODEL_UNIT; + +/// `scale="0.4"` on a `` frame ⇒ `(5/3) · 0.4 · 768` px per model unit at the reference's +/// screen basis (aspect- and resolution-independent — the terms cancel). +const PX_PER_MODEL_UNIT: f32 = 512.0; + +/// The **sequence** clock: sequence 1 "Stand", looped. `ping4`'s ring rides this. +const PING_SEQ_MS: f32 = 833.0; +/// The **global** clock (`globalSequence = 0`), which never consults the sequence window at all. +/// `ping5`'s spin rides this — the two periods are coprime, so they re-phase only after ~67 min. +const PING_SPIN_MS: f32 = 4833.0; + +/// `ping4`'s alpha: linear `0 → 1` over the first 400 ms of the loop, then `1 → 0` over the +/// remaining 433 ms. Peak alpha lands at 400 ms, when the ring is `1 + 9·400/833 = 5.32×`. +const PING4_ALPHA_PEAK_MS: f32 = 400.0; + +/// `ping5`'s spin, as the model's own 21 keys — `(ms, degrees clockwise on screen)`, unwrapped and +/// monotonic (wow-re §10.3). The rate is **not** uniform: ~67.5 °/s over the first half-turn, +/// ~83.1 °/s over the second, with a 108 °/s burst across 225°→270° that an average hides. +/// +/// Kept as the table rather than collapsed to a constant rate for exactly that reason. wow-re's +/// three transcription warnings do not bite here — they are about interpolating the model's +/// *quaternions* (a shortest-path slerp collapses the revolution to no motion, and a `w ≥ 0` +/// canonicalisation reverses it at 180°). Interpolating the **angle** linearly, as below, is the +/// sanctioned form: it deviates by at most 0.014°. +const PING5_SPIN: [(f32, f32); 21] = [ + (0.0, 0.0), + (333.0, 22.5), + (667.0, 45.0), + (889.0, 60.0), + (1111.0, 75.0), + (1333.0, 90.0), + (1666.0, 112.5), + (2000.0, 135.0), + (2222.0, 150.0), + (2444.0, 165.0), + (2667.0, 180.0), + (2847.0, 195.0), + (3027.0, 210.0), + (3208.0, 225.0), + (3416.0, 247.5), + (3625.0, 270.0), + (3805.0, 285.0), + (3986.0, 300.0), + (4167.0, 315.0), + (4500.0, 337.5), + (4833.0, 360.0), +]; + +/// The spin angle at `ms` into the global clock, in **radians clockwise on screen** — which is our +/// quad `rotation`'s own sense (the player arrow negates a WoW facing for the same reason). +fn spin_radians(ms: f32) -> f32 { + let t = ms.rem_euclid(PING_SPIN_MS); + let deg = PING5_SPIN + .windows(2) + .find(|w| t <= w[1].0) + .map_or(0.0, |w| { + let (t0, a0) = w[0]; + let (t1, a1) = w[1]; + let u = if t1 > t0 { (t - t0) / (t1 - t0) } else { 0.0 }; + a0 + (a1 - a0) * u + }); + deg.to_radians() +} + +/// The **art**, one handle per drawn layer. Named rather than a list: the three do different +/// things, and a `Vec` that silently changed order would change the animation. +#[derive(Default)] +pub(super) struct PingArt { + /// The spinner (draw order 0 in the model — though with additive and no depth write, order + /// cannot change the image). + pub(super) ping5: Option>, + /// The static centre. + pub(super) ping2: Option>, + /// The expanding ring. + pub(super) ping4: Option>, +} /// The live ping. One at a time — the reference keeps no list either. struct LivePing { @@ -97,6 +186,13 @@ struct LivePing { #[derive(Resource, Default)] pub(crate) struct MinimapPing { live: Option, + /// The model's own clock, in **milliseconds, accumulated only while a ping is shown** — which + /// is the reference's behaviour rather than a simplification of it. `SetSequence(0)` runs once + /// in the ref's `OnLoad` and stores an *anchor*; the sampler free-runs from it, and a Model + /// frame's private clock advances only while the frame is shown. The ref then only `Show()`s + /// per ping. So **ping N resumes where ping N−1 left off**, and no two consecutive pings look + /// alike (wow-re §10, VERIFIED). One accumulator reproduces that; a per-ping reset would not. + shown_ms: f32, /// A ping seated since the last [`drive_minimap_ping`] — it still owes the world an outbound /// `MSG_MINIMAP_PING` (if it is ours) and a `MINIMAP_PING` event (either way). fresh: bool, @@ -171,7 +267,7 @@ pub(super) fn emit_ping( ping: &mut MinimapPing, click: Option<(f32, f32)>, map: u32, - art: &[Handle], + art: &PingArt, quads: &mut UiQuads, ) { if let Some(world) = click.and_then(|c| click_to_world(ctx, c, ctx.seam)) { @@ -191,19 +287,46 @@ pub(super) fn emit_ping( if d >= ctx.radius_yd { return; } - let rect = Rect::from_center_size( - ctx.center + ctx.offset([px, py, 0.0]), - Vec2::splat(ctx.side * (PING_PX / super::blips::BLIP_BASIS_PX)), - ); - for layer in art { + let at = ctx.center + ctx.offset([px, py, 0.0]); + // px on the frozen 140.8 basis → this widget's px, the blip layer's own scalar. + let k = ctx.side / super::blips::BLIP_BASIS_PX; + let a = alpha * ctx.alpha; + let mut layer = |art: &Option>, side_px: f32, alpha: f32, rotation: f32| { + let Some(texture) = art.clone() else { return }; + if alpha <= 0.0 { + return; + } quads.overlays.push(UiQuad { - rect, + rect: Rect::from_center_size(at, Vec2::splat(side_px * k)), z_key: ctx.z, - texture: Some(layer.clone()), - color: [1.0, 1.0, 1.0, alpha * ctx.alpha], + texture: Some(texture), + color: [1.0, 1.0, 1.0, alpha], + // Every one of the model's quads is `blendMode = 4`, SRC_ALPHA/ONE. + additive: true, + rotation, ..default() }); - } + }; + + // The spinner, on the 4833 ms global clock. + layer(&art.ping5, PING5_PX, a, spin_radians(ping.shown_ms)); + // The static centre. + layer(&art.ping2, PING2_PX, a, 0.0); + // The ring, on the 833 ms sequence loop: geometry scale `1 + 9·t/833` about its own centre + // (the model carries NO texture-transform chunk, so this is a real size change, not a UV one), + // under a two-leg alpha that peaks 400 ms in. + let t = ping.shown_ms.rem_euclid(PING_SEQ_MS); + let ring_alpha = if t <= PING4_ALPHA_PEAK_MS { + t / PING4_ALPHA_PEAK_MS + } else { + (PING_SEQ_MS - t) / (PING_SEQ_MS - PING4_ALPHA_PEAK_MS) + }; + layer( + &art.ping4, + PING4_PX * (1.0 + 9.0 * t / PING_SEQ_MS), + a * ring_alpha, + 0.0, + ); } /// Age the ping, announce a fresh one, and expire it — everything that is *not* geometry. @@ -229,8 +352,19 @@ pub(super) fn drive_minimap_ping( // whenever a VM next appears. if let Some(live) = ping.live.as_mut() { live.age += time.delta_secs(); + // The model clock runs only while a ping is up — see `shown_ms`. (The reference stops it + // on `Hide()`, which also covers the off-disc case; we keep it running there, a difference + // of a few tenths of a phase on a marker nobody can see.) + ping.shown_ms += time.delta_secs() * 1000.0; } - // A map change drops it: the point it marks is not on this map. + // A map change drops it — a DELIBERATE divergence, now that the law is known. wow-re + // `minimap-ping-law.md` §3 is categorical: **nothing** engine-side ever clears the reference's + // ping (six instructions touch those cells; the only zeroing is a CRT static initializer before + // `WinMain`), so a real 1.12 client that teleports mid-ping keeps drawing a marker at a world + // point that is now somewhere else entirely. That is a consequence of storing the ping in a + // never-cleared global, not a behaviour anyone chose, and the 5 s timer hides it in practice. + // We drop it instead: same observable in every case a player can produce, minus the stale + // marker after a worldport. if let (Some(live), Some(map)) = (ping.live.as_ref(), map.as_ref()) { if live.map != map.0 { ping.live = None; @@ -276,9 +410,11 @@ pub(super) fn drive_minimap_ping( let Some(live) = ping.live.as_ref() else { return; }; - // Ours goes on the wire — raw world floats; the server relays them to the group and does - // nothing at all when we are solo (which is why the marker was drawn locally, not awaited). - if live.sender == 0 { + // Ours goes on the wire — raw world floats — but only when there is a group to relay them to. + // The reference gates its send the same way (`PingLocation` `0x4eeca0` sends only when + // grouped, VERIFIED) while still pinging locally, which is why a solo ping works at all and + // why the marker is drawn at click time rather than awaited off the wire. + if live.sender == 0 && group.in_group { let _ = commands.0.send(ClientCommand::MinimapPing { x: live.world.0, y: live.world.1, @@ -431,33 +567,54 @@ mod tests { assert_eq!(ping.alpha(), None, "over"); } + fn art() -> PingArt { + PingArt { + ping5: Some(Handle::default()), + ping2: Some(Handle::default()), + ping4: Some(Handle::default()), + } + } + /// **It draws, and where.** The pin's whole claim is that the marker's rect comes out of /// [`BlipCtx::offset`] like every other blip's — so this drives the real emitter and checks - /// the rect, one quad per art layer, rather than trusting the caller. + /// the rect, rather than trusting the caller. It also pins the two layers' byte-measured + /// sizes, and the fact that the ring contributes **nothing** at phase 0 (its alpha track + /// starts at zero, so a "three layers ⇒ three quads" assertion would be wrong). #[test] - fn the_emitter_puts_a_quad_per_layer_at_the_pinned_point() { + fn the_emitter_puts_the_measured_layers_at_the_pinned_point() { let c = ctx(); let mut ping = MinimapPing::default(); let mut quads = UiQuads::default(); - let art = vec![Handle::::default(), Handle::::default()]; + let art = art(); // No ping: nothing drawn. emit_ping(&c, &mut ping, None, 0, &art, &mut quads); assert!(quads.overlays.is_empty()); - // A click 30 UI units up (north) at seam 1 seats a ping 30/px_per_yd yards north... + // A click 30 UI units up (north) at seam 1 seats a ping 30/px_per_yd yards north. emit_ping(&c, &mut ping, Some((0.0, 30.0)), 0, &art, &mut quads); - assert_eq!(quads.overlays.len(), 2, "one quad per layer"); + assert_eq!( + quads.overlays.len(), + 2, + "the ring is transparent at phase 0" + ); let want = c.center + c.offset([30.0 / c.px_per_yd, 0.0, 0.0]); + let k = c.side / super::super::blips::BLIP_BASIS_PX; for q in &quads.overlays { let mid = (q.rect.min + q.rect.max) * 0.5; assert!((mid - want).length() < 1e-3, "{mid:?} vs {want:?}"); - let side = c.side * (PING_PX / super::super::blips::BLIP_BASIS_PX); - assert!((q.rect.width() - side).abs() < 1e-3); + assert!(q.additive, "every quad of the model is SRC_ALPHA/ONE"); } + // ping5 (12.39 px) draws first, ping2 (12.80 px) second — the model's own order. + assert!((quads.overlays[0].rect.width() - PING5_PX * k).abs() < 1e-3); + assert!((quads.overlays[1].rect.width() - PING2_PX * k).abs() < 1e-3); + assert!( + (PING5_PX - 12.39).abs() < 0.01 && (PING2_PX - 12.80).abs() < 0.01, + "the byte-measured sizes: {PING5_PX} / {PING2_PX}" + ); - // ...and once it is out of range it stops drawing WITHOUT dying: walk 200 yd away, then - // back. (The reference's Lua hides the marker off-disc; it does not clear the ping.) + // Out of range it stops drawing WITHOUT dying: walk 200 yd away, then back. (The + // reference's Lua hides the marker off-disc; it does not clear the ping.) quads.overlays.clear(); let mut far = ctx(); far.wx = -200.0; @@ -467,6 +624,92 @@ mod tests { assert_eq!(quads.overlays.len(), 2, "back in range: visible again"); } + /// **The ring is the motion the eye reads** (wow-re §10.4): a geometry scale `1 + 9·t/833` + /// from 3.2 px to 32 px across the sequence loop, under an alpha that rises over 400 ms and + /// falls over the remaining 433 — peaking when the ring is 5.32× its base. The paused version + /// had no ring at all. + #[test] + fn the_ring_expands_and_peaks_at_four_hundred_milliseconds() { + let c = ctx(); + let k = c.side / super::super::blips::BLIP_BASIS_PX; + let art = art(); + let ring_at = |ms: f32| { + let mut ping = MinimapPing::default(); + ping.seat((0.0, 0.0), 0, 0); + ping.shown_ms = ms; + let mut quads = UiQuads::default(); + emit_ping(&c, &mut ping, None, 0, &art, &mut quads); + // The ring is the third layer whenever it is visible at all. + quads + .overlays + .get(2) + .map(|q| (q.rect.width() / k, q.color[3])) + }; + assert_eq!(ring_at(0.0), None, "alpha 0 at the loop start: not emitted"); + let (w, a) = ring_at(PING4_ALPHA_PEAK_MS).expect("visible at the peak"); + assert!((a - 1.0).abs() < 1e-3, "peak alpha: {a}"); + assert!( + (w / PING4_PX - 5.32).abs() < 0.01, + "5.32x its base at the peak: {}", + w / PING4_PX + ); + // It keeps growing past the alpha peak, all the way to 10x, while fading out. + let (w_late, a_late) = ring_at(PING_SEQ_MS - 1.0).expect("still visible near the end"); + assert!(w_late > w, "still expanding: {w_late} vs {w}"); + assert!(a_late < 0.02, "nearly gone: {a_late}"); + assert!( + (w_late / PING4_PX - 10.0).abs() < 0.02, + "10x at the loop end: {}", + w_late / PING4_PX + ); + assert!( + (PING4_PX - 3.2).abs() < 0.01 && (PING4_PX * 10.0 - 32.0).abs() < 0.05, + "3.2 → 32.0 px" + ); + } + + /// **The spin is not uniform**, and reproducing it as a constant rate would be wrong by up to + /// 15° (wow-re §10.3's 108 °/s burst across 225°→270°). The table's own keys, and the + /// direction, are what this pins: one clockwise turn per 4833 ms, monotonic, never reversing. + #[test] + fn the_spin_follows_the_models_own_non_uniform_keys() { + for (ms, deg) in PING5_SPIN { + let got = spin_radians(ms).to_degrees(); + // The last key is a full turn, which wraps to 0 — the same rotation. + let want = if ms >= PING_SPIN_MS { 0.0 } else { deg }; + assert!((got - want).abs() < 0.01, "at {ms} ms: {got} vs {want}"); + } + // Monotonic clockwise across the whole revolution (positive rotation = CW on screen, the + // sense the player arrow's `-facing` already establishes). + let mut prev = -1.0; + for i in 0..480 { + let t = i as f32 * (PING_SPIN_MS / 480.0); + let deg = spin_radians(t).to_degrees(); + assert!(deg >= prev - 1e-3, "never reverses: {deg} after {prev}"); + prev = deg; + } + // The burst the half-turn average hides: 225°→270° takes 417 ms, not the ~667 a uniform + // rate would give. + let quarter = spin_radians(3208.0).to_degrees(); + let burst_end = spin_radians(3625.0).to_degrees(); + assert!((quarter - 225.0).abs() < 0.01 && (burst_end - 270.0).abs() < 0.01); + } + + /// The model clock is **not** reset per ping: the reference's Model frame free-runs and only + /// ticks while shown, so consecutive pings resume mid-animation. Seating a second ping must + /// leave the phase alone. + #[test] + fn a_second_ping_resumes_the_animation_rather_than_restarting_it() { + let mut ping = MinimapPing::default(); + ping.seat((0.0, 0.0), 0, 0); + ping.shown_ms = 1234.0; + ping.seat((10.0, 10.0), 0, 0); + assert!( + (ping.shown_ms - 1234.0).abs() < f32::EPSILON, + "the clock is the model's, not the ping's" + ); + } + /// A degenerate frame (the widget has not drawn yet) drops the click rather than seating a /// ping at a garbage point — and, unlike the first version, that is the *only* case in which /// a click is dropped for want of a scale. diff --git a/crates/benilla-app/src/net/apply/spells.rs b/crates/benilla-app/src/net/apply/spells.rs index 1c1d191a..29c394ac 100644 --- a/crates/benilla-app/src/net/apply/spells.rs +++ b/crates/benilla-app/src/net/apply/spells.rs @@ -119,7 +119,7 @@ pub(super) fn cast_result( // BEFORE either arm touches the guard, because both the failure arm's clear below and the // chain's own clear are the reference's ONE `0x6e741a call 0x6e4940(0x1c)` — the in-flight slot // is finished on either outcome, and only then is column 38 read. - let in_flight = pending.current(Instant::now()) == Some(spell_id); + let in_flight = pending.committed(Instant::now()) == Some(spell_id); if !success { // The cast-fail cooldown edges (the client's `HandleCastFailed 0x6e1a00`, wow-re // `wave-cooldown.md` + the 2026-07-10 §5): a plain interactive-cast failure clears ONLY @@ -406,6 +406,23 @@ pub(super) fn spell_go( hits.len(), misses.len() ); + // **The interact chain's third link** (tag `use`, the same one `target::click` writes): the cast + // the server answered a `CMSG_GAMEOBJ_USE` with. `caster_indexed=false` is the load-bearing + // case — every impact, sound and effect model below hangs off `index.0.get(&caster)`, so a + // caster we never streamed drops the whole visual silently. A GameObject IS the caster for a + // SPELLCASTER-type object (vmangos leaves `spellCaster = this` for type 22), which is why this + // reads the index rather than assuming a unit. + if benilla_assets::trace::enabled_for("use") { + benilla_assets::trace::line( + "use", + &format!( + "SPELL_GO spell={spell_id} caster={caster:#x} caster_indexed={} hits={} misses={}", + index.0.contains_key(&caster), + hits.len(), + misses.len() + ), + ); + } // A cast that names a GameObject (an open-lock cast on a chest / locked door) hands off to the GO // animation driver, which gates on the open-lock effect and opens the lid on the cast going off // (decision 0250). Independent of the caster being streamed to us — an observed open still animates. @@ -1846,7 +1863,10 @@ mod tests { if let Some(id) = in_flight { app.world_mut() .resource_mut::() - .arm(id, Instant::now()); + // `guards: false` — a hunter shot is `Attributes & 0x2` ranged, so it is + // recorded as committed and does NOT occupy the refusal. Arming it the other + // way would hide the very regression this test exists for (1601). + .arm(id, Instant::now(), false); } let (tx, _rx) = crossbeam_channel::unbounded(); let cat = spells(); diff --git a/crates/benilla-app/src/target/click.rs b/crates/benilla-app/src/target/click.rs index 82afe328..ecefa8cb 100644 --- a/crates/benilla-app/src/target/click.rs +++ b/crates/benilla-app/src/target/click.rs @@ -196,6 +196,28 @@ pub(super) fn act_on_right_click( // no packet). The lock arm runs first in `0x5f3130`, and it does here too: a `Refuse` toasts // even when we are also out of range. if go_is_nearest(&hovered, &hovered_object) { + // **The interact chain's first link** (tag `use`). "I clicked it and nothing happened" spans + // three systems — this decision, the `CMSG_GAMEOBJ_USE` it sends, and the `SMSG_SPELL_GO` + // the server answers with — and until this line existed only the *taken* branches said + // anything, at `debug!`, which the hardcoded log filter keeps off. Both silent refusals are + // traced here: a `Point` cursor (the reference's highlightable no-op) and `unable` (the + // range gray, which suppresses the send with no toast). Pairs with the `fx` kit lines, so + // ONE run says which link is dead instead of one round-trip per link. + if benilla_assets::trace::enabled_for("use") { + let ty = hovered_object + .target + .and_then(|e| stores.get(e).ok()) + .map_or(-1, |(s, _)| s.0.gameobject_type_id()); + benilla_assets::trace::line( + "use", + &format!( + "right-click go guid={:?} type={ty} cursor={:?} unable={}", + hovered_object.guid.map(|g| format!("{g:#x}")), + cursor.kind, + cursor.unable + ), + ); + } if cursor.kind != cursor_mode::CursorKind::Point { if let Some(guid) = hovered_object.guid { let go = hovered_object @@ -261,6 +283,12 @@ pub(super) fn act_on_right_click( GoAction::OpenLock(_) | GoAction::OpenByKey { .. } if cursor.unable => {} GoAction::Use => { debug!("right-click gameobject use: {guid:#x}"); + if benilla_assets::trace::enabled_for("use") { + benilla_assets::trace::line( + "use", + &format!("SEND CMSG_GAMEOBJ_USE guid={guid:#x}"), + ); + } let _ = seam.net.0.send(ClientCommand::GameObjUse { guid }); } GoAction::OpenLock(spell_id) => { diff --git a/crates/benilla-app/src/ui_action/cast_send.rs b/crates/benilla-app/src/ui_action/cast_send.rs index 0bb7692b..33ee13d0 100644 --- a/crates/benilla-app/src/ui_action/cast_send.rs +++ b/crates/benilla-app/src/ui_action/cast_send.rs @@ -172,7 +172,15 @@ impl CastLadder<'_, '_> { if commit.is_item() { self.pending.arm_item(spell_id, now); } else { - self.pending.arm(spell_id, now); + // `normal_cast`'s own predicate — a targeted-cursor commit is always an ordinary cast + // in practice (a ranged shot never raises the cursor), but computing it keeps this + // arm from drifting from the ladder's if the classes ever meet. + let guards = self + .spells + .as_ref() + .and_then(|s| s.catalog.get(spell_id)) + .is_none_or(|d| !d.ranged_attack() && !d.on_next_swing()); + self.pending.arm(spell_id, now, guards); } if let Some(d) = self.spells.as_ref().and_then(|s| s.catalog.get(spell_id)) { self.cooldowns.start_gcd(spell_id, d, now); @@ -632,16 +640,21 @@ fn send_spell_cast( }, }, }); - if normal_cast { - // One inflight id, every cast source (`0xceca88`). The item arm's provisional is shorter - // because `CMSG_USE_ITEM` has legs vmangos answers with `SMSG_INVENTORY_CHANGE_FAILURE` - // and no cast result at all — [`crate::ui_cast::PendingCast`]'s own doc, decision 0908. - if commit.is_item() { - pending.arm_item(spell_id, now); - } else { - pending.arm(spell_id, now); - } - } else if on_next_swing { + // **One inflight id, every cast source** (`0xceca88`, written at `0x6e5026` for every commit). + // The item arm's provisional is shorter because `CMSG_USE_ITEM` has legs vmangos answers with + // `SMSG_INVENTORY_CHANGE_FAILURE` and no cast result at all — + // [`crate::ui_cast::PendingCast`]'s own doc, decision 0908. + // + // **Every** class is recorded; only `normal_cast` and the item arm *guard* (1601). This used + // to be one thing: the record was armed only for the classes that refuse on it, so a ranged + // shot — every hunter shot is `Attributes & 0x2` — left no trace of having been sent, and the + // `modalNextSpell` chain's "is this reply mine?" test (`0x6e7408`) could never answer yes. + if commit.is_item() { + pending.arm_item(spell_id, now); + } else { + pending.arm(spell_id, now, normal_cast); + } + if on_next_swing { queued_melee.arm(spell_id); } // TryCast's post-send tail (`6e51b5`) — byte-verified whole by the 2026-07-14 wow-re §5 @@ -773,6 +786,7 @@ mod tests { const AUTO_SHOT: u32 = 75; const RAPTOR_STRIKE: u32 = 2973; + const SERPENT_STING: u32 = 1978; const MOB: u64 = 0x0000_0000_0000_00f1; /// The two combat-initiation classes exactly as the real 5875 rows classify them (the data @@ -794,15 +808,102 @@ mod tests { attributes: 0x404, ..Default::default() }; + // The third class the seam has to keep apart: an instant hunter shot. Ranged slot + // (`Attributes & 0x2`) like Auto Shot, but NOT auto-repeat — and it names Auto Shot in + // `modalNextSpell` (1597). + let serpent_sting = benilla_formats::SpellDisplay { + targets: 0x2, + attributes: 0x0001_0002, + attributes_ex2: 0x0002_0000, + modal_next_spell: AUTO_SHOT, + ..Default::default() + }; Spells { catalog: benilla_formats::SpellCatalog::from_displays(HashMap::from([ (AUTO_SHOT, auto_shot), (RAPTOR_STRIKE, raptor_strike), + (SERPENT_STING, serpent_sting), ])), ..Spells::empty_for_tests() } } + /// **A committed cast is recorded as committed — whatever its class** (`0xceca88` is written + /// at `0x6e5026` for every commit; decision 1601). + /// + /// This is the test whose absence shipped B280 broken **twice**. 1597 built the + /// `modalNextSpell` chain and unit-tested it by arming the in-flight record **by hand** — so + /// it passed while the real send path never armed it for a hunter shot at all + /// (`normal_cast = !ranged_attack && !on_next_swing`, and every hunter shot is + /// `Attributes & 0x2`). The chain's "is this reply mine?" test could not answer yes in play, + /// and the fix looked green and did nothing. So this goes through [`send`], the real ladder. + /// + /// The control is the other half: recorded is not the same as *guarding*. A ranged shot must + /// not start refusing the next press with `0x61`, which is what arming the old field for it + /// would have done. + #[test] + fn a_committed_ranged_shot_is_recorded_but_does_not_guard() { + let (mut world, _rx) = combat_world(false); + send_at(&mut world, SERPENT_STING, MOB); + let now = Instant::now(); + let pending = world.resource::(); + assert_eq!( + pending.committed(now), + Some(SERPENT_STING), + "the shot we just sent is the committed cast — this is what `0x6e7408` reads" + ); + assert!( + !pending.in_flight(now), + "...and it does not occupy the refusal: a shot must not block the next press" + ); + assert_eq!( + pending.current(now), + None, + "...nor light the in-flight ring the ordinary casts drive" + ); + + // An ordinary cast still does both, unchanged. + let (mut world, _rx) = combat_world(false); + send_at(&mut world, RAPTOR_STRIKE, MOB); + let pending = world.resource::(); + assert_eq!( + pending.committed(Instant::now()), + Some(RAPTOR_STRIKE), + "an on-next-swing strike is committed too" + ); + } + + /// **The chained Auto Shot survives the ladder** — the wall after the one 1601 knocked down. + /// Recording the sting as committed is only half: the chain then hands 75 back through + /// [`CastLadder::send`], which re-runs every rung. If any of them refused it — the + /// auto-repeat toggle, the in-flight guard the sting just armed, the GCD the sting just + /// started — the chain would still end in silence, and silence is what the bug looked like. + /// + /// So: send the sting, then send what its `modalNextSpell` names, and require the wire to + /// carry it. The sting's own GCD is running by then (it is armed at the commit above), which + /// is exactly the state the real chain fires in — Auto Shot's `StartRecoveryCategory` is 0 + /// against the GCD node's 133, so the getter's GCD leg cannot match it. + #[test] + fn the_chained_auto_shot_is_not_refused_by_the_sting_it_followed() { + let (mut world, rx) = combat_world(false); + send_at(&mut world, SERPENT_STING, MOB); + assert!( + matches!(rx.try_recv(), Ok(ClientCommand::CastSpell { spell_id, .. }) if spell_id == SERPENT_STING), + "the sting goes out first" + ); + // What `cast_result` would queue, handed to the one send path the drain uses. + send_at(&mut world, AUTO_SHOT, MOB); + assert!( + matches!(rx.try_recv(), Ok(ClientCommand::CastSpell { spell_id, .. }) if spell_id == AUTO_SHOT), + "and the chained Auto Shot goes out behind it — no rung eats it" + ); + assert_eq!( + world.resource::().0, + Some(AUTO_SHOT), + "...and the commit arms the repeat, which is the whole observable" + ); + } + /// [`world`] plus the catalog and the self player the commit tail needs — `engaged` is our /// server-echoed mirror of the ref's attack lock `[+0xc48]`. fn combat_world(engaged: bool) -> (World, Receiver) { diff --git a/crates/benilla-app/src/ui_cast.rs b/crates/benilla-app/src/ui_cast.rs index 1f148af2..c2e94c6e 100644 --- a/crates/benilla-app/src/ui_cast.rs +++ b/crates/benilla-app/src/ui_cast.rs @@ -95,12 +95,45 @@ pub(crate) struct PendingCast(Option); struct PendingCastState { spell_id: u32, deadline: Instant, + /// Does this record **guard** — i.e. does it refuse the next press, light the in-flight ring, + /// and gate the by-name walk? True for the classes benilla lets occupy the guard: ordinary + /// casts and item uses. + /// + /// The reference has no such flag, because it has no such split: `[0xceca88]` is written for + /// **every** committed cast (`0x6e5026 call 0x6e4ad0` inside TryCast) and the *gate* does the + /// discriminating instead, on the inflight rec's `Attributes & 0x404` at `6e4d97`. Ours grew + /// up the other way round — the guard is armed only for the classes it refuses on, so a + /// ranged shot was never recorded as committed **at all**. + /// + /// That gap is what shipped B280 broken twice: the `modalNextSpell` chain + /// (`crate::net::apply::spells::cast_result`) has to answer the reference's `0x6e7408 cmp + /// ecx,[0xceca88]` — "is this reply for the cast I just sent?" — and every hunter shot is + /// `Attributes & 0x2` ranged, so the answer was always no and the chain never fired in play. + /// The flag closes it without touching the three consumers that were right: the record is now + /// armed for every commit ([`PendingCast::committed`] reads it), and `guards` keeps + /// [`PendingCast::in_flight`] / [`PendingCast::current`] answering exactly what they did. + guards: bool, } impl PendingCast { - /// Whether a cast we sent is still outstanding (unresolved and inside its safety deadline). + /// Whether a **guarding** cast we sent is still outstanding (unresolved and inside its safety + /// deadline) — the refusal's own question. A ranged shot is committed but does not guard. pub(crate) fn in_flight(&self, now: Instant) -> bool { - self.0.as_ref().is_some_and(|p| now < p.deadline) + self.0 + .as_ref() + .is_some_and(|p| p.guards && now < p.deadline) + } + + /// **The reference's `[0xceca88]` proper** — the id of the cast we last committed and have not + /// yet had a resolution for, whatever its class. This is what `HandleCastResult 0x6e7330` + /// tests at `0x6e7408` before it reads `modalNextSpell`, and the only read that must see a + /// ranged shot. Everything else about the record is unchanged; see [`PendingCastState::guards`] + /// for why the two questions are separate fields here and one global there. + pub(crate) fn committed(&self, now: Instant) -> Option { + self.0 + .as_ref() + .filter(|p| now < p.deadline) + .map(|p| p.spell_id) } /// The outstanding cast's spell id, if one is in flight — the app-side mirror of the client's @@ -109,15 +142,18 @@ impl PendingCast { pub(crate) fn current(&self, now: Instant) -> Option { self.0 .as_ref() - .filter(|p| now < p.deadline) + .filter(|p| p.guards && now < p.deadline) .map(|p| p.spell_id) } - /// Arm the guard on a fresh send (the optimistic `0xceca88` write at the client's cast-send). - pub(crate) fn arm(&mut self, spell_id: u32, now: Instant) { + /// Arm on a fresh send — the optimistic `0xceca88` write at the client's cast-send. `guards` + /// says whether this class also occupies the refusal ([`PendingCastState::guards`]): ordinary + /// casts do, a ranged shot is recorded and does not. + pub(crate) fn arm(&mut self, spell_id: u32, now: Instant, guards: bool) { self.0 = Some(PendingCastState { spell_id, deadline: now + SEND_PROVISIONAL, + guards, }); } @@ -133,6 +169,7 @@ impl PendingCast { self.0 = Some(PendingCastState { spell_id, deadline: now + ITEM_SEND_PROVISIONAL, + guards: true, }); } @@ -720,7 +757,7 @@ mod tests { fn arming_closes_the_guard_and_a_matching_resolution_opens_it() { let t0 = Instant::now(); let mut g = PendingCast::default(); - g.arm(FIREBALL, t0); + g.arm(FIREBALL, t0, true); assert!( g.in_flight(t0), "a cast is in flight — a second one is refused" @@ -736,7 +773,7 @@ mod tests { fn a_different_spells_resolution_leaves_the_guard_closed() { let t0 = Instant::now(); let mut g = PendingCast::default(); - g.arm(FIREBALL, t0); + g.arm(FIREBALL, t0, true); g.clear_if(FIREBALL + 1); // a triggered proc's GO for some other spell mid-cast assert!( g.in_flight(t0), @@ -750,7 +787,7 @@ mod tests { // model): the guard cannot wedge casting shut forever. let t0 = Instant::now(); let mut g = PendingCast::default(); - g.arm(FIREBALL, t0); + g.arm(FIREBALL, t0, true); assert!(g.in_flight(t0 + SEND_PROVISIONAL - Duration::from_secs(1))); assert!(!g.in_flight(t0 + SEND_PROVISIONAL + Duration::from_secs(1))); } @@ -759,7 +796,7 @@ mod tests { fn a_pushback_extends_the_guard_so_it_holds_past_the_stretched_cast() { let t0 = Instant::now(); let mut g = PendingCast::default(); - g.arm(FIREBALL, t0); + g.arm(FIREBALL, t0, true); g.refine(1_000, t0); // a 1 s cast: deadline t0 + 1s + 2s slack = t0 + 3s g.delay(500, t0); // a hit at t0 pushes it +0.5s ⇒ t0 + 3.5s assert!( @@ -772,7 +809,7 @@ mod tests { fn spell_start_tightens_the_deadline_to_the_real_cast_time() { let t0 = Instant::now(); let mut g = PendingCast::default(); - g.arm(FIREBALL, t0); // provisional 5s window + g.arm(FIREBALL, t0, true); // provisional 5s window g.refine(1_500, t0); // SMSG_SPELL_START: a 1.5s cast (+ 2s pushback slack ⇒ 3.5s) let at_four = t0 + Duration::from_secs(4); assert!( @@ -884,7 +921,7 @@ mod tests { )])); app.world_mut() .resource_mut::() - .arm(FIREBALL, Instant::now()); + .arm(FIREBALL, Instant::now(), true); mark_casting(&mut app, FIREBALL); run(&mut app, true); @@ -939,7 +976,7 @@ mod tests { )])); app.world_mut() .resource_mut::() - .arm(FIREBALL, Instant::now()); + .arm(FIREBALL, Instant::now(), true); run(&mut app, true); @@ -961,7 +998,7 @@ mod tests { let (mut app, rx) = harness(HashMap::new()); app.world_mut() .resource_mut::() - .arm(FIREBALL, Instant::now()); + .arm(FIREBALL, Instant::now(), true); run(&mut app, true); @@ -1016,7 +1053,7 @@ mod tests { )])); app.world_mut() .resource_mut::() - .arm(FIREBALL, Instant::now()); + .arm(FIREBALL, Instant::now(), true); run(&mut app, false); @@ -1074,7 +1111,7 @@ mod tests { .0 = Some(AUTO_SHOT); app.world_mut() .resource_mut::() - .arm(FIREBALL, Instant::now()); + .arm(FIREBALL, Instant::now(), true); mark_casting(&mut app, FIREBALL); // Press 1: truthy mirror (auto-repeat ∪ cast) → trigger queued → the drain stops @@ -1234,7 +1271,7 @@ mod tests { .arm(HEROIC_STRIKE); app.world_mut() .resource_mut::() - .arm(FIREBALL, Instant::now()); + .arm(FIREBALL, Instant::now(), true); mark_casting(&mut app, FIREBALL); app.world_mut().run_system_once(feed_cast_bar).unwrap(); diff --git a/crates/benilla-ui/src/script/pointer.rs b/crates/benilla-ui/src/script/pointer.rs index 1726b987..f57f6b65 100644 --- a/crates/benilla-ui/src/script/pointer.rs +++ b/crates/benilla-ui/src/script/pointer.rs @@ -546,24 +546,22 @@ impl UiScript { // ── `OnMouseUp` goes to the frame that took the PRESS, not to whatever is under the // cursor now — the mouse CAPTURE (`root+0x80`). // - // The client's mouse-DOWN handler `0x7662c0` "resolves target = root+0x80 (existing - // capture) else root+0x7c (hover frame) … then title-region drag or **capture**+OnMouseDown" - // (wow-re ledger `0x7662c0`, VERIFIED), so a press takes the pointer. + // The client's mouse-DOWN handler `0x7662c0` takes the pointer: "resolves target = + // root+0x80 (existing capture) else root+0x7c (hover frame) … then title-region drag or + // **capture**+OnMouseDown" (wow-re ledger `0x7662c0`). // - // ⚠ **The release half is INFERRED, not carved.** That the capture is what the *UP* - // handler dispatches through is an extrapolation from the DOWN handler: the sibling - // `0x766420` (category `0xe`) carries only a bulk wave-classify label in wow-re's ledger, - // not a derivation. The inference is strong — a capture nothing consults on release would - // be pointless, and something has to clear `root+0x80` — but it is an inference, and this - // is a whole-UI change (every press-on-A-release-on-B in the client now delivers to A). A - // §5 is dispatched for `0x766420`'s target resolution (decision 1594); when it lands this - // comment says VERIFIED or this code changes. + // The **release** half is `0x766420`, and it is VERIFIED — wow-re + // `system/ui/scratch/mouseup-dispatch-law.md`, a §5 dispatched from this repo for exactly + // this line after it first landed on an extrapolation (decision 1599). It reads the capture + // and **nothing else**: `[mgr+0x80]` is snapshotted into `ebx` at entry (`0x76642b`), + // before anything, and `[mgr+0x7c]` — the hover frame — is **never read** anywhere in + // `[0x766420, 0x7664f0)`. One dispatch, `0x7664a4 call [eax+0x6c]`. // - // We already track exactly the capture - // — [`Model::mouse_down_on`], the press target per button, which `OnClick`'s same-frame - // rule reads — and firing `OnMouseUp` at the *hit* frame instead was the one place it went - // unused. This function's own doc has always said "on the captured frame"; the code did - // not. + // So a press on A released over B fires `OnMouseUp` on **A**, and B gets nothing on the + // release edge. We already track exactly that capture — [`Model::mouse_down_on`], the press + // target per button, which `OnClick`'s same-frame rule reads — and firing `OnMouseUp` at + // the *hit* frame was the one place it went unused. This function's doc always said "on the + // captured frame"; the code did not. // // It is not a nicety: a chat window's resize grip is a 16×16 button that the drag pulls // out from under the cursor the moment the size clamps at `SetMinResize`/`SetMaxResize` @@ -572,10 +570,20 @@ impl UiScript { // rest of the session — the stuck-drag failure `cursor::abandon_drag` exists to prevent on // the other path. // - // A press that captured NOTHING (over the world, or swallowed by a title region) falls - // back to the hit frame, which is `root+0x7c`'s arm of the same resolve. + // **A press that captured NOTHING fires nothing**, and this is the half the first version + // of this code got wrong: it fell back to the frame under the cursor, reading the DOWN + // handler's `+0x80`-else-`+0x7c` precedence as if the UP handler shared it. It does not — + // `0x766498 test ebx,ebx / je 0x7664cd` returns with no virtual call at all. The fallback + // was a divergence that let a press over open space deliver a release to whatever the + // cursor had wandered onto. + // + // (Two things the same §5 settled that are invisible from here: the capture is cleared at + // `0x7664bb`, **after** the dispatch — so a C++ consumer inside `OnMouseUp` still sees it, + // though no Lua binding reads it, `GetMouseFocus 0x48df40` being `+0x7c`; and the clear is + // gated on `evt+0x18 == 0`, the button mask *after* the event, so a **chorded** release + // fires `OnMouseUp` and keeps the capture for the button still held.) let script = if down { "OnMouseDown" } else { "OnMouseUp" }; - if let Some(id) = if down { hit_id } else { captured_id.or(hit_id) } { + if let Some(id) = if down { hit_id } else { captured_id } { if let Err(e) = event::fire_widget_handler(&self.lua, id, script, vec![btn.clone()]) { self.push_error(e); } diff --git a/crates/benilla-ui/src/script/tests/input.rs b/crates/benilla-ui/src/script/tests/input.rs index df0b8a5f..356c5aed 100644 --- a/crates/benilla-ui/src/script/tests/input.rs +++ b/crates/benilla-ui/src/script/tests/input.rs @@ -718,3 +718,100 @@ fn the_repick_re_hovers_without_a_mouse_move() { ); assert!(s.errors().is_empty(), "{:?}", s.errors()); } + +// ───────────────────────────────────────────────────────────────────────────────────────────── +// The mouse-UP dispatch law (decision 1599) — byte-verified, wow-re `mouseup-dispatch-law.md` +// ───────────────────────────────────────────────────────────────────────────────────────────── + +/// Two side-by-side frames, each recording every `OnMouseDown`/`OnMouseUp` it gets. +fn two_frames() -> crate::script::UiScript { + let mut s = script(); + s.set_screen_size(800.0, 600.0); + s.run( + r#" + log = {} + local function mk(name, x) + local f = CreateFrame("Frame", name) + f:SetPoint("BOTTOMLEFT", x, 200); f:SetSize(200, 200); f:EnableMouse(true) + f:SetScript("OnMouseDown", function(self) table.insert(log, self:GetName()..":down") end) + f:SetScript("OnMouseUp", function(self) table.insert(log, self:GetName()..":up") end) + return f + end + mk("A", 100) + mk("B", 400) + "#, + ) + .unwrap(); + s.resolve(); + s +} + +fn log_of(s: &mut crate::script::UiScript) -> String { + s.eval::("return table.concat(log, ' ')").unwrap() +} + +/// **The release goes to the frame that took the press, wherever the cursor has got to.** The +/// client's `0x766420` snapshots `[mgr+0x80]` — the capture — at entry (`0x76642b`) and dispatches +/// through it alone; `[mgr+0x7c]`, the hover frame, is never read in the whole function. So B, which +/// the cursor is over at release, gets nothing on that edge. +/// +/// The concrete thing this buys: a chat window's 16×16 resize grip is pulled out from under the +/// cursor the instant the size clamps at a resize bound, and its `OnMouseUp → FCF_StopResize` has +/// to run anyway or the sizing drag is held for the rest of the session. +#[test] +fn the_release_goes_to_the_frame_that_took_the_press() { + let mut s = two_frames(); + s.mouse_button(200.0, 300.0, "LeftButton", true); // press on A + s.mouse_move(500.0, 300.0); // ... cursor wanders onto B + s.mouse_button(500.0, 300.0, "LeftButton", false); // ... and releases there + assert_eq!( + log_of(&mut s), + "A:down A:up", + "the capture takes the release; B gets nothing on that edge" + ); +} + +/// **A press that captured nothing fires nothing on release** — and this is the half that is easy +/// to get wrong, because the DOWN handler *does* have a hover fallback (`root+0x80` else +/// `root+0x7c`) and the UP handler does **not** share it: `0x766498 test ebx,ebx / je 0x7664cd` +/// returns with no virtual call at all. +/// +/// This shipped briefly as `captured.or(hit)` — a press over open space then delivered its release +/// to whatever the cursor had wandered onto, which is a frame that never saw the press. +#[test] +fn a_press_that_captured_nothing_delivers_no_release() { + let mut s = two_frames(); + s.mouse_button(50.0, 50.0, "LeftButton", true); // press over open space + s.mouse_move(500.0, 300.0); // ... cursor onto B + s.mouse_button(500.0, 300.0, "LeftButton", false); + assert_eq!( + log_of(&mut s), + "", + "no capture, no dispatch — the UP handler has no hover arm" + ); +} + +/// The ordinary case still behaves: press and release on the same frame is down-then-up on it. +#[test] +fn a_press_and_release_on_one_frame_is_unchanged() { + let mut s = two_frames(); + s.mouse_button(200.0, 300.0, "LeftButton", true); + s.mouse_button(200.0, 300.0, "LeftButton", false); + assert_eq!(log_of(&mut s), "A:down A:up"); +} + +/// The capture is **per button**, so a right-button release cannot be answered by a left-button +/// press's capture — `Model::mouse_down_on` is keyed by button name, which is what makes this fall +/// out rather than needing its own arm. +#[test] +fn the_capture_is_per_button() { + let mut s = two_frames(); + s.mouse_button(200.0, 300.0, "LeftButton", true); // A holds the LEFT button + s.mouse_move(500.0, 300.0); + s.mouse_button(500.0, 300.0, "RightButton", false); // a RIGHT release, never pressed + assert_eq!( + log_of(&mut s), + "A:down", + "the right button captured nothing, so its release dispatches nothing" + ); +} diff --git a/crates/benilla-ui/src/widget/kinds/mod.rs b/crates/benilla-ui/src/widget/kinds/mod.rs index edcad45f..f8d779e0 100644 --- a/crates/benilla-ui/src/widget/kinds/mod.rs +++ b/crates/benilla-ui/src/widget/kinds/mod.rs @@ -296,7 +296,10 @@ impl CooldownState { /// `0x8116e8` = `{150,120,90,60,40,25}` yd). `GetZoom`/`SetZoom` read/write whichever is active, and /// each persists across transitions — so zooming in indoors does not disturb the outdoor zoom, and /// vice versa. (An earlier reading held the second index to be a rotate-minimap CVar and the interior -/// scale to be a constant; both were wrong — superseded in wow-re.) `set_zoom` clamps to 5. +/// scale to be a constant; both were wrong — superseded in wow-re. Stronger since the ping RE +/// (`minimap-ping-law.md`): there is **no rotate-minimap CVar in 5875 at all** — a 214-site +/// `CVar::Register` census plus a whole-image string scan found no such knob, so nothing on this +/// path ever needs a rotation term.) `set_zoom` clamps to 5. #[derive(Clone, Debug, PartialEq)] pub struct MinimapState { /// The **outdoor** zoom index, `0..MINIMAP_ZOOM_LEVELS` (0 = widest, 5 = tightest). `SetZoom`