From 5c9d919a0c87907a2aa98b4d8b3d9a2f4a60abc3 Mon Sep 17 00:00:00 2001 From: Sam Date: Tue, 15 Sep 2026 17:02:24 +0700 Subject: [PATCH] signpost tooltips survive the map, and the quest log stops blanking MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - a signpost or mailbox tooltip shows with Questie loaded, and after opening the map: the cursor-seated plate carries an owner, so Questie's OnShow hook no longer hides it (2255) - picking up a quest whose template is not cached no longer blanks the quest log or jumps the selection: the missing quest is skipped, not the pass (2256) - the spellbook's skill-line tab flashes when a spell lands in it (2252) - the GPU meter's resolve rides the frame's own encoder, with no submit or poll of its own: −1.8 ms on Intel's DX12 driver when armed (2254) - the audio starvation watch no longer charges a stream's own spin-up as silence (2253) Co-Authored-By: Claude Fable 5.1 --- crates/benilla-app/src/net/apply.rs | 6 + crates/benilla-app/src/net/apply/spells.rs | 57 ++- crates/benilla-app/src/perf/gpu.rs | 147 ++++-- crates/benilla-app/src/sound/mixer.rs | 458 +++++++++++++++--- crates/benilla-app/src/target/hover.rs | 38 +- crates/benilla-app/src/target/hover_probe.rs | 144 +++++- crates/benilla-app/src/target/mod.rs | 8 +- crates/benilla-app/src/ui_quest_log.rs | 56 ++- crates/benilla-app/src/ui_script/input.rs | 6 + crates/benilla-app/src/ui_script/mod.rs | 6 + .../benilla-app/src/ui_script/reference_ui.rs | 6 - .../src/ui_script/tooltip_anchor_tests.rs | 50 ++ .../src/ui_script/verb_event_gate.rs | 418 ++++++++++++++++ crates/benilla-app/src/ui_spellbook.rs | 184 ++++++- crates/benilla-app/src/ui_tooltip/mod.rs | 46 +- crates/benilla-ui/src/script/tooltip_unit.rs | 48 +- 16 files changed, 1492 insertions(+), 186 deletions(-) create mode 100644 crates/benilla-app/src/ui_script/verb_event_gate.rs diff --git a/crates/benilla-app/src/net/apply.rs b/crates/benilla-app/src/net/apply.rs index eb321b52..1ec77cb3 100644 --- a/crates/benilla-app/src/net/apply.rs +++ b/crates/benilla-app/src/net/apply.rs @@ -356,6 +356,10 @@ pub(crate) fn apply_net_updates( // and this is where the room is. One bundle rather than three `Res`, so the use sites // read `cvars.ranges` instead of `ui_actions.1 .7`. crate::ui_chat::combat::CombatFeedbackCvars<'_>, + // Spells learned mid-session, awaiting `LEARNED_SPELL_IN_TAB` (2252). A queue for the + // same reason the reference sorts before it fires: the tab index is only correct + // against the rebuilt tab list, which is the spellbook feed's, not this apply's. + ResMut, ), ResMut, ResMut, @@ -1146,6 +1150,7 @@ pub(crate) fn apply_net_updates( &mut ui_actions.0, ui_actions.10.as_deref(), &mut ui_error_keys, + &mut ui_actions.1 .6, ), SessionEvent::SpellRemoved { spell_id } => removed_spell( spell_id, @@ -1162,6 +1167,7 @@ pub(crate) fn apply_net_updates( &mut ui_actions.0, ui_actions.10.as_deref(), &mut ui_error_keys, + &mut ui_actions.1 .6, ), SessionEvent::CastResult { spell_id, diff --git a/crates/benilla-app/src/net/apply/spells.rs b/crates/benilla-app/src/net/apply/spells.rs index 8b40482f..edec3857 100644 --- a/crates/benilla-app/src/net/apply/spells.rs +++ b/crates/benilla-app/src/net/apply/spells.rs @@ -14,6 +14,7 @@ use crate::creature_anim::{CastEvent, CastEventKind, Casting, SpellGoTargets}; use crate::ui_action::{AutoRepeatActive, CastErrors, PlayerActions, Spells, UiError, UiErrorKeys}; use crate::ui_aura::AuraDurations; use crate::ui_cast::{ActiveChannel, CastBarEdge, CastBarFeed, PendingCast, QueuedMeleeSpell}; +use crate::ui_spellbook::LearnedInTab; use super::super::{GuidIndex, ObjectStore, SelfGuid}; @@ -60,12 +61,14 @@ pub(super) fn learned_spell( actions: &mut PlayerActions, spells: Option<&Spells>, errors: &mut UiErrorKeys, + tab_flash: &mut LearnedInTab, ) { debug!("net: learned spell {spell_id}"); if actions.spells.insert(spell_id) { actions.dirty = true; } announce_learn(spell_id, spells, errors); + tab_flash.0.push(spell_id); } /// **The learn announcement** — the `ERR_LEARN_*` chat line the registrar `0x4b25b0` prints at @@ -162,6 +165,7 @@ pub(super) fn superceded_spell( actions: &mut PlayerActions, spells: Option<&Spells>, errors: &mut UiErrorKeys, + tab_flash: &mut LearnedInTab, ) { debug!("net: superceded spell {old_spell_id} -> {new_spell_id}"); actions.spells.remove(&old_spell_id); @@ -171,6 +175,9 @@ pub(super) fn superceded_spell( // calls the unlearn `0x4b2c50` and then the registrar `0x4b25b0` with `mov edx,0x1` // (`0x4b2f61`), and the unlearn half holds no `DisplayError` call at all (decision 2243). announce_learn(new_spell_id, spells, errors); + // The rank-up reaches the registrar with the same flag set (`0x4b2f61 mov edx,0x1`), so it + // flashes the tab too — for the NEW rank, whose tab is the one it lands in (2252). + tab_flash.0.push(new_spell_id); } /// The server's verdict on our cast (`SMSG_CAST_RESULT`). @@ -1121,11 +1128,14 @@ mod tests { ); let mut actions = PlayerActions::default(); let mut errors = UiErrorKeys::default(); - learned_spell(78, &mut actions, Some(&spells), &mut errors); + let mut flash = LearnedInTab::default(); + learned_spell(78, &mut actions, Some(&spells), &mut errors, &mut flash); assert_eq!(errors.0.len(), 1, "one line, once"); assert_eq!(errors.0[0].key, "ERR_LEARN_ABILITY_S"); assert_eq!(errors.0[0].arg_s(), Some("Heroic Strike (Rank 1)")); + // The same live-mutation flag gates the tab flash, so the queue takes it too (2252). + assert_eq!(flash.0, vec![78], "queued for LEARNED_SPELL_IN_TAB"); } /// The other two arms of the same block, from the same entry point: a plain row is a *spell*, @@ -1148,6 +1158,7 @@ mod tests { &mut PlayerActions::default(), Some(&spells), &mut errors, + &mut LearnedInTab::default(), ); assert_eq!(errors.0[0].key, "ERR_LEARN_SPELL_S"); assert_eq!(errors.0[0].arg_s(), Some("Fireball (Rank 1)")); @@ -1167,6 +1178,7 @@ mod tests { &mut PlayerActions::default(), Some(&spells), &mut errors, + &mut LearnedInTab::default(), ); assert_eq!(errors.0[0].key, "ERR_LEARN_RECIPE_S"); assert_eq!( @@ -1209,9 +1221,22 @@ mod tests { let mut actions = PlayerActions::default(); actions.spells.insert(78); let mut errors = UiErrorKeys::default(); - superceded_spell(78, 284, &mut actions, Some(&spells), &mut errors); + let mut flash = LearnedInTab::default(); + superceded_spell( + 78, + 284, + &mut actions, + Some(&spells), + &mut errors, + &mut flash, + ); assert_eq!(errors.0.len(), 1, "one line for a rank-up, not two"); + assert_eq!( + flash.0, + vec![284], + "the NEW rank's tab flashes, not the old one's" + ); assert_eq!(errors.0[0].key, "ERR_LEARN_ABILITY_S"); assert_eq!(errors.0[0].arg_s(), Some("Heroic Strike (Rank 2)")); } @@ -1235,12 +1260,14 @@ mod tests { &mut PlayerActions::default(), Some(&spells), &mut errors, + &mut LearnedInTab::default(), ); learned_spell( 99999, &mut PlayerActions::default(), Some(&spells), &mut errors, + &mut LearnedInTab::default(), ); assert!( errors.0.is_empty(), @@ -1252,12 +1279,24 @@ mod tests { fn learned_spell_adds_to_the_book_once() { let mut actions = PlayerActions::default(); let mut errors = UiErrorKeys::default(); - learned_spell(6603, &mut actions, None, &mut errors); + learned_spell( + 6603, + &mut actions, + None, + &mut errors, + &mut LearnedInTab::default(), + ); assert!(actions.spells.contains(&6603)); assert!(actions.dirty, "a new spell dirties the feed"); actions.dirty = false; - learned_spell(6603, &mut actions, None, &mut errors); + learned_spell( + 6603, + &mut actions, + None, + &mut errors, + &mut LearnedInTab::default(), + ); assert!( !actions.dirty, "re-learning a known spell is a no-op (insert returns false)" @@ -1279,7 +1318,14 @@ mod tests { }, ); - superceded_spell(78, 284, &mut actions, None, &mut UiErrorKeys::default()); + superceded_spell( + 78, + 284, + &mut actions, + None, + &mut UiErrorKeys::default(), + &mut LearnedInTab::default(), + ); assert!( !actions.spells.contains(&78), @@ -2129,6 +2175,7 @@ mod tests { &mut PlayerActions::default(), Some(&spells), &mut errors, + &mut LearnedInTab::default(), ); assert_eq!(errors.0.len(), 1, "the learn block never reads castUI"); assert_eq!(errors.0[0].key, "ERR_LEARN_SPELL_S"); diff --git a/crates/benilla-app/src/perf/gpu.rs b/crates/benilla-app/src/perf/gpu.rs index 50d1b8fa..25c8a13f 100644 --- a/crates/benilla-app/src/perf/gpu.rs +++ b/crates/benilla-app/src/perf/gpu.rs @@ -8,14 +8,24 @@ //! bevy's own `RenderDiagnosticsPlugin` needs INSIDE_PASSES and reads zero GPU spans here — the //! `perf` module header's ⚠ note, measured in 1389). //! -//! **The resolve rides its own submission, one frame later.** 1389's recorded trap: resolving -//! the query set in the SAME command buffer as the timed work returns zeros on Metal (5 of 6 -//! reads; the 6th a plausible-looking near-miss). So the cleanup system resolves last frame's -//! set into a small ring of MAP_READ buffers via a fresh encoder, and reads the mapping two -//! frames behind. The published number is the freshest mapped delta, nanoseconds in one -//! `AtomicU64` shared with the main world — `FPS_PROBE` samples it per frame under the same env -//! and prints its own percentiles, so a leg gets `gpu_p50/gpu_p99` beside `cpu_ms` with zero -//! cost when the env is off (nothing registers). +//! **The resolve rides the NEXT frame's command buffer, ahead of that frame's own stamps.** +//! 1389's recorded trap: resolving the query set in the SAME command buffer as the timed work +//! returns zeros on Metal (5 of 6 reads; the 6th a plausible-looking near-miss) — resolve on a +//! later submission. The first build of this meter did that with a submission of its own: a +//! fresh encoder, a second `queue.submit` and an explicit `device.poll` in `Cleanup`, every +//! frame — a per-frame poll that traced at 1.8 ms on Intel's DX12 driver (2236) and priced +//! untraced in 2254. So the query set holds TWO stamp pairs and the frames alternate between +//! them ([`pairs`]): the `begin` node first resolves the pair the previous frame wrote — a later +//! submission, 1389's rule kept — and copies it into a free slot of a small ring of MAP_READ +//! buffers, on the graph's own shared encoder; then it writes this frame's pair. `Cleanup` only +//! kicks the map on the slot the graph just filled, and that map lands inside the NEXT frame's +//! own `queue.submit`, which runs wgpu's device maintenance as part of every submit +//! (`wgpu_core::device::queue::Queue::submit` ends in `Device::maintain(Poll)`) — the shape of +//! bevy's own `DiagnosticsRecorder`, and no submission or poll of the meter's own. The +//! published number is the freshest mapped delta, nanoseconds in one `AtomicU64` shared with +//! the main world — `FPS_PROBE` samples it per frame under the same env and prints its own +//! percentiles, so a leg gets `gpu_p50/gpu_p99` beside `cpu_ms` with zero cost when the env is +//! off (nothing registers). use std::sync::atomic::{AtomicU64, AtomicU8, Ordering}; use std::sync::Arc; @@ -29,13 +39,11 @@ use bevy::render::render_graph::{ Node, NodeRunError, RenderGraph, RenderGraphContext, RenderLabel, }; use bevy::render::render_phase::{ViewBinnedRenderPhases, ViewSortedRenderPhases}; -use bevy::render::render_resource::{ - Buffer, BufferDescriptor, BufferUsages, CommandEncoderDescriptor, MapMode, -}; +use bevy::render::render_resource::{Buffer, BufferDescriptor, BufferUsages, MapMode}; use bevy::render::renderer::{RenderContext, RenderDevice, RenderQueue}; use bevy::render::{Render, RenderApp, RenderSystems}; use bevy::ui_render::TransparentUi; -use wgpu::{PollType, QuerySet, QuerySetDescriptor, QueryType}; +use wgpu::{QuerySet, QuerySetDescriptor, QueryType}; /// Is the meter armed? One read; everything registers behind it. pub(crate) fn enabled() -> bool { @@ -167,7 +175,19 @@ const FREE: u8 = 0; const PENDING: u8 = 1; const MAPPED: u8 = 2; -/// Render-world state: the query set, its resolve buffer, and the mapping ring. +/// Which stamp pair a graph run writes and which it resolves, for the [`GpuStamp::frame`] it +/// runs under: the two pairs alternate, and a run resolves the pair the run before it wrote — +/// a later submission than the timed work (1389's rule), and one whose queries the previous +/// command buffer reset before writing (a resolve of a never-reset query is 2211's hang). Pair +/// `p` is queries `2p` (begin) and `2p + 1` (end). +fn pairs(frame: usize) -> (u32, u32) { + #[allow(clippy::cast_possible_truncation)] + let write = (frame % 2) as u32; + (write, 1 - write) +} + +/// Render-world state: the query set (two stamp pairs, alternating by frame), the resolve +/// buffer, and the mapping ring. #[derive(Resource)] struct GpuStamp { query_set: QuerySet, @@ -176,16 +196,23 @@ struct GpuStamp { sentinel_view: wgpu::TextureView, resolve: Buffer, ring: Vec<(Buffer, Arc)>, - /// Frames this resource has seen a `readback` for. **Zero means the stamps have never been - /// written**: `init_stamp` inserts the resource in `Cleanup`, the scheduler's sync point - /// applies it before `readback` runs in the same set, and the sentinel nodes that write — - /// and, in wgpu, reset — the two queries only run in the NEXT frame's graph. A resolve on - /// that first pass reads a query pool no command has ever reset, which Vulkan forbids - /// (`VUID-vkCmdCopyQueryPoolResults-None-09402`) and answers, on NVIDIA's driver, by - /// waiting on it forever — a GPU hang, the TDR, and the lost device 2205 met at the - /// swapchain acquire. Metal hands back zeros for the same read, which is why the meter's - /// first live runs (1389) saw "zeros" and never a hang. Decision 2211. + /// `Cleanup`s this resource has seen, counting the one that created it; the graph run that + /// follows each writes pair `frame % 2` and resolves the other ([`pairs`]). **The creation + /// `Cleanup` precedes the first stamped frame**: `init_stamp` inserts the resource in + /// `Cleanup`, the scheduler's sync point applies it before `readback` runs in the same set, + /// and the sentinel nodes that write — and, in wgpu, reset — the queries only run in the + /// NEXT frame's graph. A resolve on that first run would read a query pool no command has + /// ever reset, which Vulkan forbids (`VUID-vkCmdCopyQueryPoolResults-None-09402`) and + /// answers, on NVIDIA's driver, by waiting on it forever — a GPU hang, the TDR, and the + /// lost device 2205 met at the swapchain acquire. Metal hands back zeros for the same read, + /// which is why the meter's first live runs (1389) saw "zeros" and never a hang. Decision + /// 2211. So the first run is armed with no [`Self::slot`] and resolves nothing. frame: usize, + /// The ring slot the NEXT graph run copies the previous run's stamps into: chosen in + /// `Cleanup` from the FREE slots — `None` while no run has written a pair yet, or while the + /// ring is full (that sample is skipped; the meter is a meter). The `begin` node reads it, + /// the `Cleanup` after that run maps it. + slot: Option, period: f32, shared: Arc, } @@ -207,14 +234,30 @@ impl Node for GpuStampNode { world: &World, ) -> Result<(), NodeRunError> { if let Some(stamp) = world.get_resource::() { + let (write, resolve) = pairs(stamp.frame); + if self.index == 0 { + if let Some(slot) = stamp.slot { + // The previous run's pair, resolved and copied out on the graph's own + // encoder: a later submission than the stamps it reads (1389), and none of + // the meter's own. + let encoder = render_context.command_encoder(); + encoder.resolve_query_set( + &stamp.query_set, + resolve * 2..resolve * 2 + 2, + &stamp.resolve, + 0, + ); + encoder.copy_buffer_to_buffer(&stamp.resolve, 0, &stamp.ring[slot].0, 0, 16); + } + } // A SENTINEL PASS, not a bare `write_timestamp`: Apple Silicon samples counters only // at stage boundaries, and an encoder-level stamp with no stages around it resolves // to zero (measured on this meter's first live run — 1389's "two sentinel nodes" // meant exactly this). An empty compute pass is the cheapest stage boundary there is. let (begin, end) = if self.index == 0 { - (Some(0), None) + (Some(write * 2), None) } else { - (None, Some(1)) + (None, Some(write * 2 + 1)) }; render_context .command_encoder() @@ -261,7 +304,7 @@ fn init_stamp( let query_set = device.wgpu_device().create_query_set(&QuerySetDescriptor { label: Some("gpu-ms stamps"), ty: QueryType::Timestamp, - count: 2, + count: 4, }); let resolve = device.create_buffer(&BufferDescriptor { label: Some("gpu-ms resolve"), @@ -304,40 +347,32 @@ fn init_stamp( resolve, ring, frame: 0, + slot: None, period: queue.get_timestamp_period(), shared: shared.0.clone(), }); } -/// After the graph's own submission: resolve LAST frame's stamps on a fresh encoder (its own -/// command buffer — the 1389 trap), kick the map, and publish the oldest mapped delta. -fn readback(stamp: Option>, device: Res, queue: Res) { +/// `Cleanup`, after the graph's own submission: kick the map on the slot the graph just filled, +/// publish every landed mapping, and arm a slot for the next run. No encoder, no submit and no +/// poll of the meter's own — the map lands in the next frame's `queue.submit`, which maintains +/// the device (the module header). +fn readback(stamp: Option>) { let Some(mut stamp) = stamp else { return }; - // The first cleanup after creation precedes the first stamped frame: nothing to resolve - // yet, and resolving anyway is the Vulkan device loss above (see `frame`). - if stamp.frame == 0 { - stamp.frame = 1; - return; - } - // Copy into a FREE slot only (none free → skip this frame's sample; the meter is a meter). - if let Some(i) = (0..RING).find(|&i| stamp.ring[i].1.load(Ordering::Relaxed) == FREE) { - let mut encoder = device.create_command_encoder(&CommandEncoderDescriptor { - label: Some("gpu-ms resolve"), - }); - encoder.resolve_query_set(&stamp.query_set, 0..2, &stamp.resolve, 0); - encoder.copy_buffer_to_buffer(&stamp.resolve, 0, &stamp.ring[i].0, 0, 16); - queue.submit([encoder.finish()]); + // The slot the graph run just copied into: FREE when armed, so that submission is the only + // one in flight on it, and it is out of the arming below until the mapping is read and + // released — submitting into a mapped or map-pending buffer is a wgpu validation error, + // which the first live run demonstrated. + if let Some(i) = stamp.slot.take() { let flag = stamp.ring[i].1.clone(); flag.store(PENDING, Ordering::Relaxed); - let cb_flag = flag.clone(); stamp.ring[i] .0 .slice(..) .map_async(MapMode::Read, move |r| { - cb_flag.store(if r.is_ok() { MAPPED } else { FREE }, Ordering::Relaxed); + flag.store(if r.is_ok() { MAPPED } else { FREE }, Ordering::Relaxed); }); } - let _ = device.poll(PollType::Poll); // Publish every landed mapping; slots return to FREE. for i in 0..RING { if stamp.ring[i].1.load(Ordering::Relaxed) == MAPPED { @@ -354,6 +389,11 @@ fn readback(stamp: Option>, device: Res, queue: R } } } + // Arm the next run — which resolves the pair the run that just finished wrote — unless no + // run has written one yet (the creation `Cleanup`; see `frame`). None FREE → no sample. + if stamp.frame >= 1 { + stamp.slot = (0..RING).find(|&i| stamp.ring[i].1.load(Ordering::Relaxed) == FREE); + } stamp.frame += 1; } @@ -382,3 +422,20 @@ pub(crate) fn plugin(app: &mut App) { graph.add_node_edge(GpuStampBeginLabel, CameraDriverLabel); graph.add_node_edge(CameraDriverLabel, GpuStampEndLabel); } + +#[cfg(test)] +mod tests { + use super::pairs; + + /// The ordering law the Vulkan hang (2211) and the Metal zeros (1389) both hang on: a run + /// never resolves the pair it writes, and always resolves the one the run before it wrote. + #[test] + fn a_run_resolves_the_pair_the_run_before_it_wrote() { + for frame in 1..12 { + let (write, resolve) = pairs(frame); + assert_ne!(write, resolve); + assert!(write < 2 && resolve < 2); + assert_eq!(resolve, pairs(frame - 1).0); + } + } +} diff --git a/crates/benilla-app/src/sound/mixer.rs b/crates/benilla-app/src/sound/mixer.rs index cf3370c3..13dd1fff 100644 --- a/crates/benilla-app/src/sound/mixer.rs +++ b/crates/benilla-app/src/sound/mixer.rs @@ -732,6 +732,13 @@ impl symphonia::core::io::MediaSource for PromotingSource { const STREAM_STARVED_MIN_SECS: f64 = 0.1; /// The accounting window [`StreamWatch`] compares over. const STREAM_WATCH_WINDOW_SECS: f64 = 1.0; +/// How long a stream may sit audible with its position still pinned where it was constructed +/// before the watch says it never played at all (decision 2253). +/// +/// Well past any spin-up this machine produces — the login theme's is the worst of them, because +/// the output device opens in the same frame — and short enough that a cue which never arrives is +/// named while it should still be playing. +const STREAM_START_MAX_SECS: f64 = 3.0; /// Position-freeze watch over a live stream — the starvation meter [`MixHealth`] lacks (decision /// 1109; `poll_mix_health`'s docs name this exact blind spot). @@ -743,6 +750,18 @@ const STREAM_WATCH_WINDOW_SECS: f64 = 1.0; /// and WARNs when more than [`STREAM_STARVED_MIN_SECS`] went missing. Playback rate is always /// 1.0 here (WoW pitches no music), so the two clocks agree to drift well under the threshold. /// +/// **Every stream opens with a frozen position, and that is not a dropout** (decision 2253). +/// kira's `Shared::new` publishes `state = Playing` and `position = 0.0` when `play()` returns, so +/// the handle reads audible before the renderer has ever seen the sound; and +/// `StreamingSound::process` opens with `if self.frame_consumer.slots() < 2 { out.fill(ZERO); +/// return; }` — it zero-fills *without advancing the position* until the decode thread has two +/// frames ready. The interval between `play()` and the first sample heard is therefore a frozen +/// position by construction, and it is spin-up latency (the cue begins a fraction of a second +/// late), never silence punched into audio that was already playing. So the watch does not start +/// counting until the position has actually moved ([`Phase::Starting`]), and says at `debug!` what +/// it skipped. Before that it charged the spin-up as loss: the 2026-09-15 login reported the glue +/// theme's own first ~235 ms as "injected silence", which is what a crackle does *not* sound like. +/// /// **Known blind spot (1112, deliberate):** the watch samples on the main thread, so a stream /// that starves *during a main-thread stall* and reaches `Stopped` before the next frame is /// never accounted — the window is discarded by the reset. Reconstructing it from the stop @@ -751,32 +770,67 @@ const STREAM_WATCH_WINDOW_SECS: f64 = 1.0; /// thread was doing. pub(crate) struct StreamWatch { label: &'static str, - last_pos: Option, + phase: Phase, + /// Wall time counted into the open window, summed from the fed frame deltas. expected: f64, + /// Stream position gained over those same frames. advanced: f64, - /// When the open window began, in WALL time — carried so the report can state the span it - /// actually covered beside the time it counted. +} + +/// Where a watched stream is in its life: nothing to watch, spinning up, or playing. +#[derive(Clone, Copy)] +enum Phase { + /// No stream on the slot, or one that is not audible. Position legitimately holds. + Idle, + /// Audible, and the position has not moved off `pos` yet — the spin-up (see [`StreamWatch`]). + /// Nothing is counted here; `warned` holds [`STREAM_START_MAX_SECS`] to one line per stream. + Starting { + since: std::time::Instant, + pos: f64, + warned: bool, + }, + /// Advancing. `window_start` is the WALL instant this window's counting began — stamped at + /// that same instant and only there, so the counted time and the reported span are two + /// measurements of one interval and *cannot* disagree. /// - /// **Because the two disagreed and nothing could say why.** A 2026-09-06 login closed a 1.0 s - /// window 0.756 s of wall after the stream started, which is arithmetically impossible if the - /// fed `dt` is the real frame delta — and the fix that put this watch on `Time` did not - /// change it. Rather than reason further from log timestamps, the line now carries both - /// numbers: if they disagree the caller's clock is still wrong, and if they agree the - /// starvation is real and the timestamps were the misreading. - window_start: Option, - /// The closing window's counted totals, kept for the report (`observe` zeroes them). - closed: (f64, f64), + /// **They did disagree, across three runs, and that is why this is a field rather than an + /// `Instant::now()` taken in the report.** It used to be stamped lazily, on the first frame + /// that found `expected == 0.0` — which is the frame *after* the one the first delta reaches + /// back to, so the span dropped exactly one frame of the interval it claimed to cover. Two + /// 2026-09-06 logins called the result arithmetically impossible (a 1.0 s window closing + /// 0.81 s and 0.756 s of wall after the stream started) and suspected the fed clock; the + /// 2026-09-15 login that first carried the span printed `counted 1.00 s` over a `0.70 s` + /// window, and the missing 0.30 s is the 298 ms frame hitch stamped in that same instant. + /// The clock was right all along; the span was one frame short of what was counted. + Running { + last_pos: f64, + window_start: std::time::Instant, + }, +} + +/// What [`StreamWatch::observe`] found this frame. Returned rather than logged so the accounting +/// stays pure and the tests can drive it without a device. +enum Verdict { + /// A window closed having lost more than [`STREAM_STARVED_MIN_SECS`] of audible time. + Starved { + lost: f64, + counted: f64, + advanced: f64, + span: f64, + }, + /// The stream went audible and its position never moved — nothing from it was ever heard. + NeverStarted { waited: f64 }, + /// The stream began advancing this long after it went audible: its spin-up. + Began { after: f64 }, } impl StreamWatch { pub(crate) fn new(label: &'static str) -> Self { Self { label, - last_pos: None, + phase: Phase::Idle, expected: 0.0, advanced: 0.0, - window_start: None, - closed: (0.0, 0.0), } } @@ -784,62 +838,149 @@ impl StreamWatch { pub(crate) fn feed(&mut self, handle: &StreamingSoundHandle, dt: f64) { use kira::sound::PlaybackState as S; let audible = matches!(handle.state(), S::Playing | S::Stopping); - if audible && self.expected == 0.0 { - self.window_start = Some(std::time::Instant::now()); - } - if let Some(lost) = self.observe(audible, handle.position(), dt) { - let span = self - .window_start - .take() - .map_or(f64::NAN, |t| t.elapsed().as_secs_f64()); - let (counted, advanced) = self.closed; - // **No cause is named.** This used to say "(decode thread outrun)", which is one of - // three mechanisms that freeze a stream's position identically — a starved decoder, a - // render thread that did not run, or a closed/rebuilding device — and the meter cannot - // tell them apart. Asserting one of the three in the line is how a log hands a reader - // a conclusion the instrument never reached. - warn!( - "audio: {} stream starved — ~{:.0} ms of injected silence over a {span:.2} s \ - window (counted {counted:.2} s, position advanced {advanced:.2} s) — this is \ - what a crackle sounds like", + match self.observe(audible, handle.position(), dt, std::time::Instant::now()) { + Some(Verdict::Starved { + lost, + counted, + advanced, + span, + }) => { + // **No cause is named.** This used to say "(decode thread outrun)", which is one + // of three mechanisms that freeze a stream's position identically — a starved + // decoder, a render thread that did not run, or a closed/rebuilding device — and + // the meter cannot tell them apart. Asserting one of the three in the line is how + // a log hands a reader a conclusion the instrument never reached. + warn!( + "audio: {} stream starved — ~{:.0} ms of injected silence over a {span:.2} s \ + window (counted {counted:.2} s, position advanced {advanced:.2} s) — this is \ + what a crackle sounds like", + self.label, + lost * 1000.0, + ); + } + Some(Verdict::NeverStarted { waited }) => warn!( + "audio: {} has been audible {waited:.1} s with its position still at the start — \ + not one sample of it has been heard", self.label, - lost * 1000.0, - ); + ), + // A bound, not a stopwatch, and it says which bound: the clock starts when the WATCH + // first saw the handle (within a frame of `play()`, but that frame can be a long one + // during a login burst), and it can only stop on a frame, so a spin-up that ends + // inside a long frame is reported as that whole frame. + Some(Verdict::Began { after }) => debug!( + "audio: {} took {:.0} ms to start advancing after the watch first saw it — \ + spin-up, not counted", + self.label, + after * 1000.0, + ), + None => {} } } /// Drop the baseline — call when the watched handle is dropped/replaced, so the next /// stream's position (starting at 0, i.e. *behind* the old one's) can't read as a freeze. pub(crate) fn reset(&mut self) { - self.last_pos = None; + self.phase = Phase::Idle; self.expected = 0.0; self.advanced = 0.0; - self.window_start = None; } - /// The accounting core, pure so the tests below can drive it without a device. Returns - /// `Some(lost_secs)` when a window closes starved. - fn observe(&mut self, audible: bool, pos: f64, dt: f64) -> Option { + /// Open a counting window at `now` — which is both the position baseline and the wall origin. + /// The pair is set together on purpose; see [`Phase::Running`] for what happened when it + /// wasn't. + fn begin(&mut self, pos: f64, now: std::time::Instant) { + self.phase = Phase::Running { + last_pos: pos, + window_start: now, + }; + self.expected = 0.0; + self.advanced = 0.0; + } + + /// The accounting core, pure so the tests below can drive it without a device. + fn observe( + &mut self, + audible: bool, + pos: f64, + dt: f64, + now: std::time::Instant, + ) -> Option { // Non-audible states (stopped, paused) drop the baseline: position legitimately holds. if !audible { self.reset(); return None; } - let Some(last) = self.last_pos.replace(pos) else { - return None; // first audible frame — baseline only - }; - self.expected += dt; - // `max(0.0)`: a track replaced on the slot mid-window jumps backwards once (the new - // stream starts at 0); count it as no advance for that one frame, not as negative. - self.advanced += (pos - last).max(0.0); - if self.expected < STREAM_WATCH_WINDOW_SECS { - return None; + match self.phase { + Phase::Idle => { + self.phase = Phase::Starting { + since: now, + pos, + warned: false, + }; + None + } + Phase::Starting { + since, + pos: start, + warned, + } => { + let waited = now.duration_since(since).as_secs_f64(); + if pos > start { + self.begin(pos, now); + return Some(Verdict::Began { after: waited }); + } + if !warned && waited > STREAM_START_MAX_SECS { + self.phase = Phase::Starting { + since, + pos: start, + warned: true, + }; + return Some(Verdict::NeverStarted { waited }); + } + None + } + Phase::Running { + last_pos, + window_start, + } => { + // A track swapped onto the slot mid-window starts at 0 — *behind* the one it + // replaced. That is a new stream, not a freeze, so it goes back to `Starting` and + // gets its own spin-up excluded exactly like a fresh one. No watched stream ever + // loops in place: `loop_region` belongs to the static path, never a streamed one + // (see `stream_from_bytes`), so a backwards jump has no other reading. + if pos < last_pos { + self.phase = Phase::Starting { + since: now, + pos, + warned: false, + }; + self.expected = 0.0; + self.advanced = 0.0; + return None; + } + self.expected += dt; + self.advanced += pos - last_pos; + self.phase = Phase::Running { + last_pos: pos, + window_start, + }; + if self.expected < STREAM_WATCH_WINDOW_SECS { + return None; + } + let (counted, advanced) = (self.expected, self.advanced); + let span = now.duration_since(window_start).as_secs_f64(); + // The next window's origin is THIS instant — the one its first delta reaches back + // to. + self.begin(pos, now); + let lost = counted - advanced; + (lost > STREAM_STARVED_MIN_SECS).then_some(Verdict::Starved { + lost, + counted, + advanced, + span, + }) + } } - let lost = self.expected - self.advanced; - self.closed = (self.expected, self.advanced); - self.expected = 0.0; - self.advanced = 0.0; - (lost > STREAM_STARVED_MIN_SECS).then_some(lost) } } @@ -1367,20 +1508,32 @@ mod tests { ); } - /// The starvation accounting (decision 1109): a stream that advances in lockstep with wall - /// time stays quiet; one whose position freezes mid-window (kira's zero-fill) reports the - /// missing time; a slot swap's one-frame backward jump never reads as a freeze; and going + /// Frame `i`'s wall instant, so a test reads as a timeline rather than as bookkeeping. + fn wall(t0: std::time::Instant, secs: f64) -> std::time::Instant { + t0 + std::time::Duration::from_secs_f64(secs) + } + + /// The starvation accounting (decisions 1109, 2253): a stream that advances in lockstep with + /// wall time stays quiet; one whose position freezes mid-window (kira's zero-fill) reports the + /// missing time; a slot swap's backward jump is a new stream, not a freeze; and going /// non-audible drops the baseline so a later stream starts clean. #[test] fn stream_watch_accounts_freezes_not_swaps() { let dt = 1.0 / 60.0; + let t0 = std::time::Instant::now(); - // Healthy: position tracks wall time exactly — a full window closes clean. + // Healthy: position tracks wall time exactly — every window closes clean. let mut w = StreamWatch::new("test"); let mut pos = 0.0; - for _ in 0..90 { - assert_eq!(w.observe(true, pos, dt), None, "healthy stream reported"); + for i in 0..90 { pos += dt; + assert!( + !matches!( + w.observe(true, pos, dt, wall(t0, f64::from(i) * dt)), + Some(Verdict::Starved { .. }) + ), + "healthy stream reported" + ); } // Starved: 18 frames (~0.3 s) frozen inside the window → that window reports ~0.3 s. @@ -1391,26 +1544,35 @@ mod tests { if !(30..48).contains(&i) { pos += dt; } - if let Some(lost) = w.observe(true, pos, dt) { - reports.push(lost); + if let Some(Verdict::Starved { + lost, + counted, + span, + .. + }) = w.observe(true, pos, dt, wall(t0, f64::from(i) * dt)) + { + reports.push((lost, counted, span)); } } assert_eq!(reports.len(), 1, "one starved window: {reports:?}"); assert!( - (reports[0] - 0.3).abs() < 0.05, + (reports[0].0 - 0.3).abs() < 0.05, "lost ≈ 0.3 s, got {reports:?}" ); - // Slot swap: position jumps backwards once (new track starts at 0) — no report. + // Slot swap: the position jumps backwards once (the new track starts at 0). That is a new + // stream, not a freeze — it re-enters the spin-up, and nobody is charged for the jump. let mut w = StreamWatch::new("test"); let mut pos = 40.0; - for i in 0..90 { + for i in 0..120 { if i == 30 { pos = 0.0; } - assert_eq!( - w.observe(true, pos, dt), - None, + assert!( + !matches!( + w.observe(true, pos, dt, wall(t0, f64::from(i) * dt)), + Some(Verdict::Starved { .. }) + ), "slot swap reported as freeze" ); pos += dt; @@ -1420,18 +1582,168 @@ mod tests { // the old one's) opens a fresh window instead of inheriting a phantom freeze. let mut w = StreamWatch::new("test"); let mut pos = 70.0; - for _ in 0..30 { - assert_eq!(w.observe(true, pos, dt), None); + for i in 0..30 { + assert!(w + .observe(true, pos, dt, wall(t0, f64::from(i) * dt)) + .is_none_or(|v| matches!(v, Verdict::Began { .. }))); pos += dt; } - assert_eq!(w.observe(false, pos, dt), None); + assert!(w.observe(false, pos, dt, wall(t0, 0.5)).is_none()); let mut pos = 0.0; - for _ in 0..90 { - assert_eq!(w.observe(true, pos, dt), None, "restart read as freeze"); + for i in 30..120 { + assert!( + !matches!( + w.observe(true, pos, dt, wall(t0, f64::from(i) * dt)), + Some(Verdict::Starved { .. }) + ), + "restart read as freeze" + ); pos += dt; } } + /// **The span is the interval that was counted** — the pin on decision 2253's off-by-one. + /// + /// The window used to be wall-stamped lazily, on the first frame that found nothing counted + /// yet — one frame *after* the frame its first delta reaches back to. So a hitch at the head + /// of a window was counted but not spanned, and the line contradicted itself: the 2026-09-15 + /// login printed `counted 1.00 s` over a `0.70 s` window with a 298 ms hitch in the gap, and + /// two earlier logins had called the same arithmetic impossible and suspected the clock. + /// Here the head of the window *is* that hitch, and the stream froze through it. + #[test] + fn stream_watch_spans_the_window_it_counted() { + let dt = 1.0 / 60.0; + let t0 = std::time::Instant::now(); + let hitch = 0.298; + let mut w = StreamWatch::new("test"); + + // `play()` has returned; nothing has been heard yet, so nothing is counted yet. + assert!(w.observe(true, 0.0, dt, wall(t0, 0.0)).is_none()); + // The first sample lands: the window opens here, at this instant. + assert!(matches!( + w.observe(true, 0.001, dt, wall(t0, 0.010)), + Some(Verdict::Began { .. }) + )); + // The hitch — 298 ms of wall in one frame, the stream frozen through it. Its delta + // reaches back to the instant the window opened, so the span must contain it. + assert!(w + .observe(true, 0.001, hitch, wall(t0, 0.010 + hitch)) + .is_none()); + + let mut pos = 0.001; + let mut report = None; + for i in 0..90 { + pos += dt; + let t = 0.010 + hitch + f64::from(i + 1) * dt; + if let Some(v) = w.observe(true, pos, dt, wall(t0, t)) { + report = Some(v); + break; + } + } + let Some(Verdict::Starved { + lost, + counted, + span, + .. + }) = report + else { + panic!("the frozen hitch went unreported"); + }; + assert!( + (counted - span).abs() < 1e-6, + "the window must span the interval it counted: counted {counted}, span {span}" + ); + assert!( + (lost - hitch).abs() < 0.01, + "the hitch is the whole loss: {lost}" + ); + } + + /// **A stream's spin-up is not a dropout** (decision 2253), driven here on the timeline of + /// the 2026-09-15 login that reported the glue theme's own first 235 ms as injected silence. + /// + /// kira publishes `Playing` with `position = 0.0` the moment `play()` returns, and + /// `StreamingSound::process` zero-fills *without advancing the position* until the decode + /// thread has two frames ready. So every stream opens frozen, and that interval is latency + /// before the first sample — never silence cut into audio that was already playing. + /// + /// That login's shape, relative to the frame the theme started on: the output device opened + /// and the stream was played inside a frame that then blocked for 298 ms, and the first + /// sample landed 233 ms in (~100 ms of mix-ahead, ~11 ms of IO buffer, the rest decode). + /// The old accounting charged all of it, closed its first window at 1.00 s counted against + /// 0.77 s advanced, and called the difference a crackle. + #[test] + fn stream_watch_does_not_charge_a_stream_for_starting() { + let dt = 1.0 / 60.0; + let t0 = std::time::Instant::now(); + let hitch = 0.298; + let first_sample = 0.233; + // Position as the handle reports it: pinned where it was constructed until the renderer + // has audio to consume, then advancing with wall time. + let pos_at = |t: f64| (t - first_sample).max(0.0); + + let mut w = StreamWatch::new("test"); + let mut began = Vec::new(); + let mut starved = 0; + // The frame the theme started on — audible, position still 0 — and then the hitch. + let mut ticks = vec![0.0, hitch]; + // Then three seconds of ordinary frames. + for i in 1..=180 { + ticks.push(hitch + f64::from(i) * dt); + } + for (i, t) in ticks.iter().enumerate() { + let delta = if i == 0 { dt } else { t - ticks[i - 1] }; + match w.observe(true, pos_at(*t), delta, wall(t0, *t)) { + Some(Verdict::Began { after }) => began.push(after), + Some(Verdict::Starved { + lost, + counted, + span, + .. + }) => { + starved += 1; + eprintln!("starved: lost {lost} counted {counted} span {span}"); + } + Some(Verdict::NeverStarted { .. }) => { + panic!("a playing stream read as never started") + } + None => {} + } + } + assert_eq!( + starved, 0, + "the login's own spin-up was charged as injected silence again" + ); + assert_eq!(began.len(), 1, "one spin-up line per stream: {began:?}"); + // Frame-quantized: the first advance is only visible on the frame after the hitch. + assert!( + (began[0] - hitch).abs() < 1e-6, + "the skipped spin-up, stated as the bound it is: {began:?}" + ); + } + + /// Excluding the spin-up must not mean excluding a spin-up that never ends: a stream that + /// goes audible and never plays a sample is still named, once (decision 2253). + #[test] + fn stream_watch_names_a_stream_that_never_starts() { + let dt = 1.0 / 60.0; + let t0 = std::time::Instant::now(); + let mut w = StreamWatch::new("test"); + let mut waited = Vec::new(); + for i in 0..600 { + if let Some(Verdict::NeverStarted { waited: s }) = + w.observe(true, 0.0, dt, wall(t0, f64::from(i) * dt)) + { + waited.push(s); + } + } + assert_eq!(waited.len(), 1, "one line per stream, not one per frame"); + assert!( + waited[0] > STREAM_START_MAX_SECS && waited[0] < STREAM_START_MAX_SECS + 0.1, + "named as soon as the wait is past the bound: {waited:?}" + ); + } + /// The fade contract every transition rests on: `stop(tween)` fades the channel to silence /// over the tween's duration on the audio thread, and **dropping the handle immediately after /// does not cut it short** — the command is already published to the renderer's triple-buffer diff --git a/crates/benilla-app/src/target/hover.rs b/crates/benilla-app/src/target/hover.rs index ac78cfe2..839d988a 100644 --- a/crates/benilla-app/src/target/hover.rs +++ b/crates/benilla-app/src/target/hover.rs @@ -564,6 +564,9 @@ pub(super) struct GoPickSet<'w, 's> { /// nothing ever reads. frame: Local<'s, u64>, report: Local<'s, super::hover_probe::ProbeReport>, + /// `WOW_HOVER_PROBE=lock`'s held target (2255) — the object the probe rests its aim on for the + /// remainder of the run. + lock: Local<'s, super::hover_probe::LockedAim>, } /// The number of `ChildOf` hops [`net_entity_of`] will climb before giving up — a malformed-data @@ -677,20 +680,32 @@ pub(super) fn update_hovered_object( hovered.target = None; hovered.guid = None; hovered.distance = f32::MAX; - if rig.is_looking() || pointer_over_ui.0 { + // The sticky-hover cache drops the moment the pointer is not ours, exactly as before — the + // probe's aim below must not change *when* that happens. + let yielded = rig.is_looking() || pointer_over_ui.0; + if yielded { *last_pick = None; - return; } let (Ok((camera, cam_tf)), Ok(window)) = (camera.single(), window.single()) else { return; }; // The aim. A person's pointer always wins; the probe (2250) answers only for a window that has // no cursor at all, which is every automated run — see [`super::hover_probe`]. + // + // **Resolved and PUBLISHED ahead of the yield below** (2255), because the UI mouse feed reads + // the published aim as its pointer: a frame that returned early without publishing would take + // the pointer off the UI, drop `PointerOverUi`, and unlatch the very gate that skipped the + // pick — the probe would flicker the interface on and off instead of resting on it. let probing = super::hover_probe::armed(); - let Some(cursor) = window - .cursor_position() - .or_else(|| super::hover_probe::point(window, *cache.frame)) - else { + let probe_aim = cache + .lock + .point(camera, cam_tf, |e| parts.get(e).ok().map(|(_, gt, ..)| *gt)) + .or_else(|| super::hover_probe::point(window, *cache.frame)); + super::hover_probe::publish(probe_aim); + if yielded { + return; + } + let Some(cursor) = window.cursor_position().or(probe_aim) else { return; }; if pickable.is_empty() { @@ -861,6 +876,17 @@ pub(super) fn update_hovered_object( tmpl.map(|t| t.highlight_column), occlusion.distance, )); + // `lock` takes its target here and nowhere else: past the eligibility gate, so the + // probe holds something that actually publishes a mouseover rather than the first + // lamp-post the grid grazed. The hit point is re-expressed in the PART's frame so the + // aim survives the camera settling and the object moving (see [`LockedAim`]). + if super::hover_probe::locks() { + if let Some((t, part)) = best { + if let Ok((_, gt, ..)) = parts.get(part) { + cache.lock.hold(part, gt, ray.origin + *ray.direction * t); + } + } + } } } hovered.target = Some(net_entity); diff --git a/crates/benilla-app/src/target/hover_probe.rs b/crates/benilla-app/src/target/hover_probe.rs index 3a3f8cbe..6aeeb51b 100644 --- a/crates/benilla-app/src/target/hover_probe.rs +++ b/crates/benilla-app/src/target/hover_probe.rs @@ -20,12 +20,23 @@ //! | `centre` / `center` | the window's middle | //! | `,` | that point | //! | `sweep` | a 7×5 grid across the middle half of the window, one point per frame, cycling | +//! | `lock` | sweep until something is picked, then HOLD that object for the rest of the run | //! //! `sweep` is the one that makes a rig run useful without a camera: standing a body in front of a //! known object and asking *"what is anywhere near the middle of the screen"* answers the question //! a fixed point can only answer if the aim was already right. It is a probe, not a camera search — //! the grid is fixed, bounded and the same every run. //! +//! **`lock` is what a resting pointer looks like, and `sweep` cannot stand in for it** (2255). A +//! sweep re-enters the object every cycle, and a *re-enter* is precisely what rebuilds the +//! mouseover plate — so a whole class of defect, the one where a plate that is already up never +//! comes back after something else takes the tooltip, is invisible to a sweep by construction. It +//! is also what a fixed `x,y` cannot do: the camera settles over the first seconds of a rig run and +//! the object slides across the screen, so a hand-aimed point that hit at 30 s misses at 60 s. +//! `lock` holds the hit POINT IN THE OBJECT'S OWN FRAME and re-projects it every frame, so the aim +//! follows the object through camera drift and through the object's own motion. It never releases: +//! that is the point — it stands for a pointer a person put down and left there. +//! //! **Armed only when the window reports no cursor**, so an attended run is never overridden: a //! person's pointer always wins, and leaving the variable set costs a player nothing. @@ -38,6 +49,7 @@ enum Aim { Centre, At(i32, i32), Sweep, + Lock, } fn aim() -> Option { @@ -48,16 +60,23 @@ fn aim() -> Option { match v { "centre" | "center" => Some(Aim::Centre), "sweep" => Some(Aim::Sweep), + "lock" => Some(Aim::Lock), _ => match v.split_once(',') { Some((x, y)) => match (x.trim().parse(), y.trim().parse()) { (Ok(x), Ok(y)) => Some(Aim::At(x, y)), _ => { - warn!("hover probe: WOW_HOVER_PROBE={v:?} is not `centre`, `sweep` or `x,y`"); + warn!( + "hover probe: WOW_HOVER_PROBE={v:?} is not `centre`, `sweep`, \ + `lock` or `x,y`" + ); None } }, None => { - warn!("hover probe: WOW_HOVER_PROBE={v:?} is not `centre`, `sweep` or `x,y`"); + warn!( + "hover probe: WOW_HOVER_PROBE={v:?} is not `centre`, `sweep`, `lock` or \ + `x,y`" + ); None } }, @@ -79,8 +98,9 @@ pub(super) fn point(window: &Window, frame: u64) -> Option { Aim::At(x, y) => Some(Vec2::new(x as f32, y as f32)), // The grid: 7 columns × 5 rows over the middle half, so the edges of the frame (chrome, // sky, the player's own back) are never the answer. One cell per frame, cycling — at frame - // rate the whole grid is covered ~2× a second. - Aim::Sweep => { + // rate the whole grid is covered ~2× a second. `lock` searches with the same grid; once it + // holds an object [`LockedAim::point`] answers ahead of this and the grid stops mattering. + Aim::Sweep | Aim::Lock => { const COLS: u64 = 7; const ROWS: u64 = 5; let cell = frame % (COLS * ROWS); @@ -92,6 +112,79 @@ pub(super) fn point(window: &Window, frame: u64) -> Option { } } +/// Does the aim HOLD its first pick? (`WOW_HOVER_PROBE=lock`.) +pub(super) fn locks() -> bool { + matches!(aim(), Some(Aim::Lock)) +} + +/// **This frame's aim**, published by the pick and read by everything else that needs to know +/// where the probe is pointing: the UI mouse feed (so `PointerOverUi` rises and falls over a panel +/// exactly as it does for a person) and the tooltip's cursor-seated arm. +/// +/// It is a process-global rather than a resource for two reasons. The probe already is one — the +/// env var behind [`aim`] — and its three readers sit in three different systems that are each at +/// or near Bevy's parameter ceiling, so a resource would cost a bundle refactor apiece for a debug +/// instrument with exactly one writer. +/// +/// **One aim, published once per frame, is the correctness claim and not a convenience.** Before +/// 2255 each reader called [`point`] again for itself, and the tooltip's call passed frame `0` — +/// so on a `sweep` the plate was seated at grid cell 0 while the pick was aiming at cell N, and the +/// two disagreed about where the pointer was on 34 frames out of every 35. +static AIM_NOW: std::sync::Mutex> = std::sync::Mutex::new(None); + +/// Publish this frame's aim. Called by the pick BEFORE its own gates: a frame that skipped the +/// pick must still report a pointer, or the UI feed would take the pointer off the UI and unlatch +/// the very gate that skipped it. +pub(super) fn publish(point: Option) { + *AIM_NOW.lock().unwrap_or_else(|e| e.into_inner()) = point; +} + +/// This frame's aim, as published. `None` when the probe is not armed. +pub(super) fn now() -> Option { + *AIM_NOW.lock().unwrap_or_else(|e| e.into_inner()) +} + +/// `lock` mode's held target: the part the probe first picked, and the point it hit **in that +/// part's own frame**. Re-projected every frame, so the aim follows the object rather than going +/// stale the moment the camera settles or the object moves. +#[derive(Default)] +pub(super) struct LockedAim { + held: Option<(Entity, Vec3)>, +} + +impl LockedAim { + /// Where the held point is on screen now, or `None` while nothing is held (the grid still + /// searches) or while it has gone off-camera (the grid searches again). + pub(super) fn point( + &self, + camera: &Camera, + cam_tf: &GlobalTransform, + transform_of: impl Fn(Entity) -> Option, + ) -> Option { + let (part, local) = self.held?; + let gt = transform_of(part)?; + camera + .world_to_viewport(cam_tf, gt.transform_point(local)) + .ok() + } + + /// Hold this hit — **once**. A lock that never releases is the whole point; re-locking onto + /// whatever happened to be picked later would make the aim wander exactly like the sweep it + /// exists to replace. + /// + /// Deliberately does **not** ask [`locks`] itself: the caller gates on the mode (a `sweep` that + /// held its first hit would stop sweeping), and keeping this half pure is what lets a test + /// drive it without an env var behind a `OnceLock`. + pub(super) fn hold(&mut self, part: Entity, gt: &GlobalTransform, hit_world: Vec3) { + if self.held.is_some() { + return; + } + let local = gt.affine().inverse().transform_point3(hit_world); + info!("hover probe: LOCKED on {part} at its own {local:?} — the aim holds here now"); + self.held = Some((part, local)); + } +} + /// One line per *change*, so a stationary probe does not flood the log: what the pick found at the /// aim point, and every term of the GameObject tooltip ladder the card shows a person (2248) — /// eligibility, the published mouseover, and the ask-once template the plate needs. @@ -130,6 +223,49 @@ mod tests { } } + /// **The lock holds its FIRST target and keeps it**, and holds it in the object's own frame — + /// the two properties that make a locked aim stand for a resting pointer rather than a slowly + /// wandering one. + #[test] + fn the_lock_holds_its_first_target_in_the_objects_own_frame() { + let mut l = LockedAim::default(); + let first = Entity::from_raw_u32(1).expect("a valid test entity id"); + let later = Entity::from_raw_u32(2).expect("a valid test entity id"); + // A part standing 10 yd east; the ray landed 2 yd up its face. + let gt = GlobalTransform::from_translation(Vec3::new(10.0, 0.0, 0.0)); + l.hold(first, >, Vec3::new(10.0, 2.0, 0.0)); + assert_eq!( + l.held, + Some((first, Vec3::new(0.0, 2.0, 0.0))), + "the hit is stored relative to the part, not in world space" + ); + l.hold(later, >, Vec3::ZERO); + assert_eq!( + l.held.map(|(e, _)| e), + Some(first), + "a later pick never steals the lock" + ); + } + + /// Nothing held is nothing aimed — the search half keeps running until the first hit. + #[test] + fn an_unheld_lock_aims_at_nothing() { + let l = LockedAim::default(); + let cam = Camera::default(); + assert!(l + .point(&cam, &GlobalTransform::IDENTITY, |_| None) + .is_none()); + } + + /// The published aim round-trips, and clearing it is how a frame says "no pointer here". + #[test] + fn the_published_aim_round_trips() { + publish(Some(Vec2::new(3.0, 4.0))); + assert_eq!(now(), Some(Vec2::new(3.0, 4.0))); + publish(None); + assert_eq!(now(), None); + } + /// A repeated line is said once — the property that makes the probe readable in a log rather /// than 60 identical lines a second. #[test] diff --git a/crates/benilla-app/src/target/mod.rs b/crates/benilla-app/src/target/mod.rs index 26a65ac1..274e9b3b 100644 --- a/crates/benilla-app/src/target/mod.rs +++ b/crates/benilla-app/src/target/mod.rs @@ -73,9 +73,11 @@ pub(crate) mod hover; /// The headless hover probe (2250) — see its header. mod hover_probe; -/// The probe's aim for a window with no OS cursor, for the tooltip's cursor-seated arm (2250). -pub(crate) fn hover_probe_point(window: &bevy::window::Window) -> Option { - hover_probe::point(window, 0) +/// The probe's aim for a window with no OS cursor — for the tooltip's cursor-seated arm and the +/// UI mouse feed (2250; 2255 made it THIS frame's aim as the pick published it, rather than a +/// second, independently recomputed one that disagreed with the pick on 34 sweep frames in 35). +pub(crate) fn hover_probe_point() -> Option { + hover_probe::now() } /// Is the headless hover probe armed? (Gates its own log lines outside this module.) diff --git a/crates/benilla-app/src/ui_quest_log.rs b/crates/benilla-app/src/ui_quest_log.rs index 36cb602a..2f268dbd 100644 --- a/crates/benilla-app/src/ui_quest_log.rs +++ b/crates/benilla-app/src/ui_quest_log.rs @@ -612,34 +612,38 @@ fn feed_quest_log( .filter(|t| !t.is_empty()) }; - // ── The IN-FLIGHT GATE — all of the log, or none of it ────────────────────────────────────── - // The reference's rebuild (`0x4de510`) counts cache misses into `[0xbb7498]`, skips the row - // for each miss entirely (`0x4de66b`/`0x4de67e` — no row AND no header for its group), and - // then `0x4de545` makes any FURTHER rebuild a total no-op — not even `QUEST_LOG_UPDATE` — - // while the counter is non-zero. Only the arriving response decrements it, rebuilding once - // when it reaches 0. So a cold cache shows an EMPTY log (`GetNumQuestLogEntries()` → `0, 0`, - // and the stock UI paints `EmptyQuestLogFrame`), and then the whole thing appears in ONE step, - // complete. **It is never partially filled** (wow-re `ui/scratch/questlog-list-rebuild.md`, - // §5 trio + arbitration; our B3 claim — a placeholder row per in-flight quest — was REFUTED). + // ── The IN-FLIGHT SKIP — the cached rows, and only those ──────────────────────────────────── + // The reference's rebuild (`0x4de510`) counts cache misses into `[0xbb7498]` and skips the + // MISSING QUEST — not the pass. The miss at `0x4de67e` is a `jmp 0x4de729`, which is the slot + // loop's own increment block, one instruction above the back-edge: every later slot is still + // visited and every later cache hit still appends its row at `0x4de68e`. The hit path's own + // "this group already has a header" branch (`0x4de6f1`) jumps to the same address right after + // appending a row, which a discard could not do. The tail past the loop is unconditional — + // both sorts, the collapse restore, the watch prune, and `QUEST_LOG_UPDATE` (`0x4de811`). + // `0x4de545` then makes any FURTHER rebuild a no-op until the last outstanding response + // decrements the counter to 0 and `0x4de8b0` rebuilds once more. // - // The empty window is the COLD case only: the reference loads `WDB/questcache.wdb` into this - // same cache before world-enter (`0x554f9b`), so a returning character's rows are there - // immediately. We have no such disk cache yet, so ours is the cold path every login — which is - // exactly the reference's own behaviour on a fresh `WDB`, not a deviation. + // So `GetNumQuestLogEntries()` answers `(visible rows, CACHED quest count)`, and `0, 0` is the + // all-cold extreme rather than the general law. Skipping the whole log on any miss — what this + // did, on a note that read the cold case as the rule — blanked the entire window for a round + // trip every time a quest whose template we had never seen entered the log. That took the + // engine selection with it (`remap_selection` reads an empty list as "your quest is gone"), so + // the detail pane jumped to row 1 on every single pickup, and addons reading the log across + // the blank saw a log that briefly held nothing. Settled at the bytes by wow-re, whose + // `scratch/questlog-list-rebuild.md` §1.1 now carries the partial case (decision 2256). // - // `template()` is still called for every row first, because the miss is what SENDS the query - // (the reference's rebuild sends `CMSG_QUEST_QUERY` too, once per id ever — our `pending` set - // is that same dedupe). - // Deliberately a loop and NOT `.all(..)`: `template()` is what SENDS the query, and the - // reference asks for every missing id, not just the first. Short-circuiting here would leave - // the rest of a cold log unrequested — and then nothing would ever arrive to un-gate it. - let mut all_cached = true; - for r in &rows { - if quest_log.template(r.quest_id, &commands).is_none() { - all_cached = false; - } - } - let rows: Vec = if all_cached { rows } else { Vec::new() }; + // `template()` is called for EVERY row before any filtering, because the miss is what SENDS + // the query (our `pending` set is the reference's once-per-id dedupe) — short-circuiting would + // leave the rest of a cold log unrequested, and then nothing would arrive to fill it in. + let cached: Vec = rows + .iter() + .map(|r| quest_log.template(r.quest_id, &commands).is_some()) + .collect(); + let rows: Vec = rows + .into_iter() + .zip(cached) + .filter_map(|(r, hit)| hit.then_some(r)) + .collect(); // ── Section headers (the ref's zone/sort groups) ──────────────────────────────────────────── // One header per distinct `ZoneOrSort` (positive → AreaTable zone, negative → QuestSort sort — diff --git a/crates/benilla-app/src/ui_script/input.rs b/crates/benilla-app/src/ui_script/input.rs index d1c99bd0..0aa463ba 100644 --- a/crates/benilla-app/src/ui_script/input.rs +++ b/crates/benilla-app/src/ui_script/input.rs @@ -139,8 +139,14 @@ pub(super) fn feed_ui_input( // nobody can see must not eat the click or arm a tooltip, and the else-arm below is exactly the // "no pointer here" bookkeeping (leave the hovered frame once, disarm any press/drag every // frame) that keeps a stale gesture from firing when the UI comes back. + // The headless hover probe's aim stands in for a cursor the window does not have (2250/2255), + // so `PointerOverUi` rises over a panel and falls off it in an automated run exactly as it does + // for a person — which is what lets a rig run reproduce "open the map, close it, and the world + // under it goes quiet". A person's pointer always wins; an unarmed probe answers `None` and + // nothing here changes. if let Some(cursor) = window .cursor_position() + .or_else(crate::target::hover_probe_point) .filter(|_| !ui_hidden && !synthetic) { // Window cursor is logical px, y-down from top-left; the UI is y-up 768-virtual units diff --git a/crates/benilla-app/src/ui_script/mod.rs b/crates/benilla-app/src/ui_script/mod.rs index 233db30a..837d8774 100644 --- a/crates/benilla-app/src/ui_script/mod.rs +++ b/crates/benilla-app/src/ui_script/mod.rs @@ -1019,6 +1019,12 @@ mod shape_gate; #[cfg(test)] mod event_shape_gate; +/// The VERB-FIRED event gate (decision 2251) — the third question on the same seam: 1883/1889 +/// compare names and 2140 compares arguments; this asks WHO fires it, because a stock file that +/// calls a verb for its side effect of an event repaints nothing when the verb fires nothing. +#[cfg(test)] +mod verb_event_gate; + /// The reference's BasicControls.xml — TEXT/message/_ERRORMESSAGE and the ScriptErrors dialog, /// none of which benilla itself calls: every test enters from Lua the way an addon does. #[cfg(test)] diff --git a/crates/benilla-app/src/ui_script/reference_ui.rs b/crates/benilla-app/src/ui_script/reference_ui.rs index 67868267..2963606d 100644 --- a/crates/benilla-app/src/ui_script/reference_ui.rs +++ b/crates/benilla-app/src/ui_script/reference_ui.rs @@ -2357,12 +2357,6 @@ mod tests { flow works without it", ), ("ITEM_TEXT_TRANSLATION", "ItemTextFrame.lua"), - ( - "LEARNED_SPELL_IN_TAB", - "SpellBookFrame.lua — the tab flash when a new spell lands in it; the reference \ - fires it with the tab index on spell add, which benilla's spellbook feed does not \ - derive yet (1952)", - ), ("PET_UI_CLOSE", "PetPaperDollFrame.lua"), ("PET_UI_UPDATE", "PetPaperDollFrame.lua"), ("PLAYER_DAMAGE_DONE_MODS", "PaperDollFrame.lua"), diff --git a/crates/benilla-app/src/ui_script/tooltip_anchor_tests.rs b/crates/benilla-app/src/ui_script/tooltip_anchor_tests.rs index 1941f0c3..4fa81f39 100644 --- a/crates/benilla-app/src/ui_script/tooltip_anchor_tests.rs +++ b/crates/benilla-app/src/ui_script/tooltip_anchor_tests.rs @@ -515,3 +515,53 @@ fn buff_hover_hangs_below_left_of_the_button() { "buff tooltip hangs its TOPRIGHT on the button's BOTTOMLEFT" ); } + +/// **The cursor-seated GameObject plate carries an OWNER** — the store the reference's publisher +/// makes through the SetOwner core (`0x492a01 → 0x52ffe0(owner, 6, 0, 0)`, whose `0x53000c` +/// writes `+0x314`), and the one arm of ours that used to skip it (decision 2255). +/// +/// Every other world plate reached an owner by accident, through Lua: the corner arm and the unit +/// flow both fire `OnTooltipSetDefaultAnchor`, and the stock handler calls +/// `GameTooltip:SetOwner(UIParent, …)`. The cursor arm fires nothing, so `IsOwned` answered false +/// for exactly the GENERIC(5) objects — a signpost, a mailbox — and the next test is what that +/// cost. +#[test] +fn a_cursor_seated_gameobject_plate_is_owned() { + let mut s = harness(&[]); + assert!(s.world_tooltip_gameobject("Brill", &[], Some((512.0, 384.0)))); + let owned: bool = s.eval("return GameTooltip:IsOwned(UIParent)").unwrap(); + assert!( + owned, + "the signpost plate is owned; errors: {:?}", + s.errors() + ); +} + +/// **An addon's `OnShow` hook must not hide the plate the world hover just built** — the +/// director's signpost with no tooltip (decision 2255). +/// +/// `!Questie` installs an `OnShow` on GameTooltip at PLAYER_LOGIN (`Questie:hookTooltip` — it +/// installs one precisely *because* the stock plate has none) whose handler ends in +/// `GameTooltip:Show()`. Lua's `:Show()` is the reference's EXISTENCE GATE `0x530a80`: owner and +/// line count both non-zero, or it takes the effective-hide `0x530a60` instead. So an unowned +/// plate hides itself the instant it is shown — through our own faithful implementation of that +/// gate, ~26 ms after the engine built it, on every signpost, for the whole session. +/// +/// The reference cannot reach that state, because its publisher writes the owner *before* the +/// plate is ever shown. With the owner written, so do we. +#[test] +fn a_cursor_seated_gameobject_plate_survives_an_addons_on_show_hook() { + let mut s = harness(&[]); + // Questie's hook, in one line: the plate's own show event calls Show() again. + s.run(r#"GameTooltip:SetScript("OnShow", function() GameTooltip:Show() end)"#) + .unwrap(); + assert!(s.world_tooltip_gameobject("Brill", &[], Some((512.0, 384.0)))); + let shown: bool = s + .eval("return GameTooltip:IsShown() and true or false") + .unwrap(); + assert!( + shown, + "the signpost plate is still up; errors: {:?}", + s.errors() + ); +} diff --git a/crates/benilla-app/src/ui_script/verb_event_gate.rs b/crates/benilla-app/src/ui_script/verb_event_gate.rs new file mode 100644 index 00000000..710edceb --- /dev/null +++ b/crates/benilla-app/src/ui_script/verb_event_gate.rs @@ -0,0 +1,418 @@ +//! **The VERB-FIRED event gate** — `reference/1.12-verb-events.tsv` against the module that +//! registers each verb (decision 2251). +//! +//! ## The third question on the event seam +//! +//! Two gates compare event NAMES (`reference_ui`: a stock listener nothing feeds, 1889; a fire the +//! reference has no name for, 1883) and one compares ARGUMENTS (`event_shape_gate`, 2140). None of +//! them asks **who** fires it, and that is the hole B389/2244 fell through. The reference's +//! `SetTrainerServiceTypeFilter` commits through `0x4d8c90`, whose whole body is *write the mask, +//! re-run the finalizer, fire `TRAINER_UPDATE`*; so the stock `Blizzard_TrainerUI.lua` never +//! repaints the list itself — it calls the verb for its side effect. benilla's verb set the mask +//! and fired nothing. The name gate stayed green because the packet arm fires the same event, and +//! the tests stayed green because they asserted the mask. Our own retired `TrainerFrame.xml` had +//! repainted explicitly, so the behaviour left with the file that was compensating for it (1957), +//! and the director found it twelve days later. Every window that went stock (1751) carries the +//! same exposure: a verb the stock Lua calls *for the event*. +//! +//! ## The rule +//! +//! For every pair the reference fires on the verb's own call path (the table: the verb's body, or +//! a helper that only registered verbs call), and every verb benilla registers: **a file that +//! registers the verb also fires the event** — or the pair is declared below, and the declaration +//! is *checked*, never trusted: +//! +//! * [`ELSEWHERE`] — benilla fires it from a named feed instead, on the state the verb changed +//! (the verb bumps a generation, sets a touched flag, queues an intent the app drains). That +//! file must fire it, and the verb's own file must not, or the row is stale. +//! * [`GAP`] — nothing fires it on this verb's path, for a stated reason. The verb's own file +//! must still not fire it, or the row is stale. +//! +//! A verb no file registers is skipped: unbuilt verbs are `scripts/api-coverage.sh`'s queue +//! (1178), not this gate's. +//! +//! ## What "registers" and "fires" mean here, and the limit +//! +//! Text, not execution: the quoted literal `"Verb"` or `"EVENT"` in a non-test source file, with +//! comment lines dropped and the trailing test module cut. Lenient in exactly one direction — any +//! file that names the verb and fires the event passes — because the honest alternative, proving +//! the path *executes*, is a per-verb fixture, and 2244's own test is the measure of a fixture that +//! asserts the wrong thing. What this cannot see: a fire that is in the file but off the verb's +//! path — an `ELSEWHERE` row is a claim the session that wrote it read at the feed. What it sees +//! every time is the migration class itself: a verb whose module fires nothing. + +use std::collections::BTreeSet; +use std::path::PathBuf; + +/// One row of `reference/1.12-verb-events.tsv`. +struct Pair { + verb: String, + event: String, + shape: String, + via: String, +} + +/// Pairs benilla fires from a feed, on the state the verb changes. `(verb, event, the file that +/// fires it)`; the file is matched as a path suffix. Each was read at the feed when the row was +/// written — the trigger is named so the next reader can check it in one grep. +const ELSEWHERE: &[(&str, &str, &str)] = &[ + // The sell slot's diff (`auction_sell_item` moved, or `sell_slot_dirty`). + ( + "CalculateAuctionDeposit", + "NEW_AUCTION_UPDATE", + "benilla-app/src/ui_auction/mod.rs", + ), + ( + "ClickAuctionSellItemButton", + "NEW_AUCTION_UPDATE", + "benilla-app/src/ui_auction/mod.rs", + ), + // The query goes to the wire; the inbox feed announces the list when it lands. + ( + "CheckInbox", + "MAIL_INBOX_UPDATE", + "benilla-app/src/ui_mail/mod.rs", + ), + // `craft_close` / `trade_skill_close` are drained by the app, whose store transition + // `(Some, None)` is the fire. + ("CloseCraft", "CRAFT_CLOSE", "benilla-app/src/ui_craft.rs"), + ( + "CloseTradeSkill", + "TRADE_SKILL_CLOSE", + "benilla-app/src/ui_tradeskill.rs", + ), + // `take_quest_log_collapses`: the app owns the collapse set, re-feeds the list, fires. + ( + "CollapseQuestHeader", + "QUEST_LOG_UPDATE", + "benilla-app/src/ui_quest_log.rs", + ), + ( + "ExpandQuestHeader", + "QUEST_LOG_UPDATE", + "benilla-app/src/ui_quest_log.rs", + ), + // `trade_skill_touched`, set by all four verbs, drained by `take_trade_skill_touched`. + ( + "CollapseTradeSkillSubClass", + "TRADE_SKILL_UPDATE", + "benilla-app/src/ui_tradeskill.rs", + ), + ( + "ExpandTradeSkillSubClass", + "TRADE_SKILL_UPDATE", + "benilla-app/src/ui_tradeskill.rs", + ), + ( + "SetTradeSkillInvSlotFilter", + "TRADE_SKILL_UPDATE", + "benilla-app/src/ui_tradeskill.rs", + ), + ( + "SetTradeSkillSubClassFilter", + "TRADE_SKILL_UPDATE", + "benilla-app/src/ui_tradeskill.rs", + ), + // `repeat_count` moved (`repeat_changed`). + ( + "DoTradeSkill", + "UPDATE_TRADESKILL_RECAST", + "benilla-app/src/ui_tradeskill.rs", + ), + // `take_loot_roll_confirms`: a Need/Greed on a bind-on-pickup roll is diverted to the popup. + ( + "ConfirmLootRoll", + "CANCEL_LOOT_ROLL", + "benilla-app/src/ui_loot_roll.rs", + ), + ( + "ConfirmLootRoll", + "CONFIRM_LOOT_ROLL", + "benilla-app/src/ui_loot_roll.rs", + ), + ( + "RollOnLoot", + "CANCEL_LOOT_ROLL", + "benilla-app/src/ui_loot_roll.rs", + ), + ( + "RollOnLoot", + "CONFIRM_LOOT_ROLL", + "benilla-app/src/ui_loot_roll.rs", + ), + // `keybinds.generation` moves; `sync_dispatch` re-derives the table and fires. + ( + "SetBinding", + "UPDATE_BINDINGS", + "benilla-app/src/bindings.rs", + ), + // A `SessionRequest::StopCinematic`; the player stops, and the flip to not-playing fires. + ( + "StopCinematic", + "CINEMATIC_STOP", + "benilla-app/src/cinematic.rs", + ), + // The toggle goes to the wire and the bar's pushed key changes on the reply — one round trip + // later than the reference, which fires locally from the toggle. + ( + "TogglePetAutocast", + "PET_BAR_UPDATE", + "benilla-app/src/ui_pet/bar.rs", + ), + ( + "ToggleSpellAutocast", + "PET_BAR_UPDATE", + "benilla-app/src/ui_pet/bar.rs", + ), +]; + +/// Pairs nothing fires on the verb's path, each with its reason. A row is a gap with a name, +/// never a reason to stop; the reference's own site is in the table for the grep into wow-re. +const GAP: &[(&str, &str, &str)] = &[ + ( + "CancelSkillUps", + "SKILL_LINES_CHANGED", + "resets temp skill points over a table this model keeps empty (`skills.rs`), so nothing \ + moves and `ui_char.rs`'s feed has nothing to announce; the reference fires from \ + 0x4d3e3a regardless", + ), + ( + "ClickTargetTradeButton", + "TRADE_REPLACE_ENCHANT", + "the trade-window enchant leg (0592 P3) is not built; the verb is the money arm only", + ), + ( + "CloseTrade", + "PLAYER_TRADE_MONEY", + "`ui_trade.rs` announces money only while the trade is open; the reference's close zeroes \ + the offer and fires from 0x4bfdce, under a frame that TRADE_CLOSED hides first", + ), + ( + "CollapseCraftSkillLine", + "CRAFT_UPDATE", + "a no-op here: the craft list is flat, the header law was never ported from TradeSkill \ + (0446, 0530's follow-up), so there is nothing to repaint", + ), + ( + "ExpandCraftSkillLine", + "CRAFT_UPDATE", + "the same no-op as CollapseCraftSkillLine", + ), + ( + "PetDismiss", + "PET_DISMISS_START", + "no shipped file listens; the dismiss reaches the wire through the app's drain and the \ + pet frame follows the unit's departure", + ), + ( + "SelectGossipOption", + "GOSSIP_ENTER_CODE", + "the code-entry gossip option kind is not modelled (`reference_ui`'s UNPRODUCED row)", + ), + ( + "SetPetStablePaperdoll", + "PET_STABLE_UPDATE_PAPERDOLL", + "inert by design: the stable's model pane is an app-side booth that follows the selection \ + every frame (1676), so the VM has nothing to point", + ), + ( + "UpdateMapHighlight", + "CLOSE_WORLD_MAP", + "0x4a7620 fires it (site 0x4a7835, id 373) on a path the highlight verb reaches under a \ + condition nobody has read; unbuilt until carved", + ), +]; + +fn table() -> Vec { + let tsv = concat!( + env!("CARGO_MANIFEST_DIR"), + "/../../reference/1.12-verb-events.tsv" + ); + let text = std::fs::read_to_string(tsv).expect("reference/1.12-verb-events.tsv"); + text.lines() + .filter(|l| !l.starts_with('#') && !l.starts_with("verb\t")) + .filter_map(|l| { + let f: Vec<&str> = l.split('\t').collect(); + (f.len() >= 4).then(|| Pair { + verb: f[0].to_string(), + event: f[1].to_string(), + shape: f[2].to_string(), + via: f[3].to_string(), + }) + }) + .collect() +} + +/// Every non-test `.rs` under `crates/`: comment lines dropped, the trailing test module cut. +fn sources() -> Vec<(PathBuf, String)> { + let root = std::path::Path::new(env!("CARGO_MANIFEST_DIR")) + .parent() + .and_then(|p| p.parent()) + .expect("workspace root") + .join("crates"); + let mut out = Vec::new(); + let mut stack = vec![root]; + while let Some(dir) = stack.pop() { + for e in std::fs::read_dir(&dir).into_iter().flatten().flatten() { + let p = e.path(); + if p.is_dir() { + stack.push(p); + continue; + } + // The same file-name rule the sibling gates use: a test's own literals are not a + // surface we ship — and neither are the gates' own declared lists, this file's + // included (a `*_gate.rs` is a `#[cfg(test)]` module whose name does not say so). + let name = p.to_string_lossy(); + if p.extension().is_none_or(|x| x != "rs") + || name.contains("test") + || name.ends_with("_gate.rs") + { + continue; + } + let whole = std::fs::read_to_string(&p).unwrap_or_default(); + let head = whole + .rfind("#[cfg(test)]\nmod ") + .map_or(whole.as_str(), |i| &whole[..i]); + let text: String = head + .lines() + .filter(|l| !l.trim_start().starts_with("//")) + .collect::>() + .join("\n"); + out.push((p, text)); + } + } + out.sort(); + out +} + +fn quoted(text: &str, name: &str) -> bool { + text.contains(&format!("\"{name}\"")) +} + +fn short(p: &std::path::Path) -> String { + let s = p.to_string_lossy(); + s.split_once("/crates/") + .map_or(s.to_string(), |(_, t)| t.to_string()) +} + +/// The files that register `verb`, and whether any of them fires `event`. +fn verb_module_fires(src: &[(PathBuf, String)], verb: &str, event: &str) -> (Vec, bool) { + let registrars: Vec<&(PathBuf, String)> = src.iter().filter(|(_, t)| quoted(t, verb)).collect(); + let fires = registrars.iter().any(|(_, t)| quoted(t, event)); + (registrars.iter().map(|(p, _)| short(p)).collect(), fires) +} + +/// **Every event the reference fires from inside a verb is fired by the module that registers +/// the verb** — or declared, with the declaration checked. +#[test] +fn every_event_the_reference_fires_from_a_verb_is_fired_by_the_verb_s_module() { + let src = sources(); + assert!(src.len() > 100, "source walk found {} files", src.len()); + let declared: BTreeSet<(&str, &str)> = ELSEWHERE + .iter() + .chain(GAP.iter()) + .map(|(v, e, _)| (*v, *e)) + .collect(); + + let mut surprises = Vec::new(); + let mut seen = 0; + for pair in table() { + let (registrars, fires) = verb_module_fires(&src, &pair.verb, &pair.event); + if registrars.is_empty() { + continue; // an unbuilt verb: api-coverage.sh's queue, not this gate's + } + seen += 1; + if fires || declared.contains(&(pair.verb.as_str(), pair.event.as_str())) { + continue; + } + let fired_by: Vec = src + .iter() + .filter(|(_, t)| quoted(t, &pair.event)) + .map(|(p, _)| short(p)) + .collect(); + surprises.push(format!( + "{} -> {} [{}{}] registered in {}; {}", + pair.verb, + pair.event, + pair.shape, + if pair.via == "-" { + String::new() + } else { + format!(" via {}", pair.via) + }, + registrars.join(", "), + if fired_by.is_empty() { + "fired NOWHERE in benilla".to_string() + } else { + format!("fired only by {}", fired_by.join(", ")) + }, + )); + } + assert!( + seen > 20, + "only {seen} pairs had a registered verb — the walk or the table is off" + ); + assert!( + surprises.is_empty(), + "the reference fires these events from inside the verb, and the module that registers \ + the verb here fires nothing — the stock Lua that calls the verb for its side effect \ + repaints nothing (2244's class). Fire it from the verb, or declare the pair: ELSEWHERE \ + when a feed fires it on THIS verb's change and you read that at the feed, GAP with the \ + reason otherwise:\n {}", + surprises.join("\n ") + ); +} + +/// **A declaration is checked, not trusted** — an `ELSEWHERE` file that stopped firing, a verb +/// module that now fires what its row says it does not, a row for a pair the table no longer +/// carries: each fails until the row is corrected. +#[test] +fn the_declared_rows_are_still_true() { + let src = sources(); + let table = table(); + let in_table = + |verb: &str, event: &str| table.iter().any(|p| p.verb == verb && p.event == event); + let mut wrong = Vec::new(); + for (verb, event, file) in ELSEWHERE { + if !in_table(verb, event) { + wrong.push(format!( + "ELSEWHERE {verb} -> {event}: not a pair in the table any more" + )); + } + let named: Vec<&(PathBuf, String)> = src + .iter() + .filter(|(p, _)| p.to_string_lossy().ends_with(file)) + .collect(); + if named.is_empty() { + wrong.push(format!( + "ELSEWHERE {verb} -> {event}: no source file ends with {file}" + )); + } else if !named.iter().any(|(_, t)| quoted(t, event)) { + wrong.push(format!( + "ELSEWHERE {verb} -> {event}: {file} no longer fires it" + )); + } + if verb_module_fires(&src, verb, event).1 { + wrong.push(format!( + "ELSEWHERE {verb} -> {event}: the verb's own module fires it now — take the row out" + )); + } + } + for (verb, event, _) in GAP { + if !in_table(verb, event) { + wrong.push(format!( + "GAP {verb} -> {event}: not a pair in the table any more" + )); + } + if verb_module_fires(&src, verb, event).1 { + wrong.push(format!( + "GAP {verb} -> {event}: the verb's own module fires it now — take the row out" + )); + } + } + assert!( + wrong.is_empty(), + "stale declarations:\n {}", + wrong.join("\n ") + ); +} diff --git a/crates/benilla-app/src/ui_spellbook.rs b/crates/benilla-app/src/ui_spellbook.rs index 0c7771f8..aaac0715 100644 --- a/crates/benilla-app/src/ui_spellbook.rs +++ b/crates/benilla-app/src/ui_spellbook.rs @@ -36,7 +36,7 @@ use std::collections::{BTreeMap, BTreeSet}; use bevy::prelude::*; use benilla_formats::{SkillLineCatalog, SpellCatalog}; -use benilla_ui::script::{SpellBookState, SpellSlotView, SpellTabView, UiScript}; +use benilla_ui::script::{ScriptValue, SpellBookState, SpellSlotView, SpellTabView, UiScript}; use crate::entities::ItemDisplays; use crate::items::Items; @@ -68,11 +68,30 @@ const NO_LINE: u32 = 0; /// a `SkillLine.dbc` lookup (key 0 has no DBC row). Extensionless, as the DBC/BLP loader expects. const GENERAL_TAB_ICON: &str = "Interface\\Icons\\Ability_Kick"; +/// **Spells learned mid-session, awaiting their tab flash** — the queue behind +/// `LEARNED_SPELL_IN_TAB` (event 510; decision 2252). +/// +/// Filled by the two net arms the reference reaches its announce block from +/// (`crate::net::apply::spells`' learn and rank-up), never by the `SMSG_INITIAL_SPELLS` bulk load: +/// the reference gates all of this on `0x4b25b0`'s live-mutation flag, which the login drain +/// passes clear. Drained by [`feed_spellbook`]. +/// +/// **It is a queue rather than a direct fire because of ordering, and the reference has the same +/// ordering for the same reason.** `0x4b2b5a` re-sorts the tab registry and fires `SPELLS_CHANGED` +/// *first*, and only then does `0x4b2b92` fire the tab event — an index read before that sort can +/// be stale. The reference threads this by caching the pre-sort index and re-searching only when +/// the tab was newly created (`0x4b2b5f cmp edi,0xffffffff`, the one case where the sort can move +/// things); benilla has no pre-sort index to cache, because the feed computes it from the tab list +/// it has just rebuilt — so the two paths collapse into one and the result is the same index. +#[derive(Resource, Default)] +pub(crate) struct LearnedInTab(pub(crate) Vec); + pub(crate) struct UiSpellbookPlugin; impl Plugin for UiSpellbookPlugin { fn build(&self, app: &mut App) { - app.add_systems(Startup, load_skill_lines.after(AssetSet::Open)) + app.init_resource::() + .add_systems(Startup, load_skill_lines.after(AssetSet::Open)) .add_systems( Update, ( @@ -140,6 +159,7 @@ fn feed_spellbook( cooldowns: Res, clock: Res, mut memory: Local>, + mut learned: ResMut, ) { let Some(mut script) = script else { return; @@ -178,6 +198,10 @@ fn feed_spellbook( ("spells", spells_changed), ("skill_lines", lines_changed), ("icons", icons_added), + // A queued learn must not be able to sit behind a skipped frame: it is only ever + // pushed on a frame that also dirties `PlayerActions`, but depending on that is a + // silent coupling, and this is the file's own way of saying so. + ("learned", !learned.0.is_empty()), ], ); let gate = gate::Gate::new( @@ -191,11 +215,15 @@ fn feed_spellbook( || actions_changed || spells_changed || lines_changed - || icons_added, + || icons_added + || !learned.0.is_empty(), ); if gate.skip() { return; } + // Drained here, before any later `return` can strand it: an id we cannot resolve is an id + // that fires nothing, exactly as the reference's own null-record bail at the registrar's head. + let learned = std::mem::take(&mut learned.0); // Nothing to resolve a name/icon/passive from yet — try again once Spell.dbc lands. let Some(spells) = spells.as_deref() else { return; @@ -217,7 +245,7 @@ fn feed_spellbook( // serves the page. let ranged_icon = store.and_then(|s| ranged_weapon_icon(s, &mut items, icons.as_deref(), &commands)); - let mut fresh = build_book( + let (mut fresh, tab_lines) = build_book( &actions.spells, &spells.catalog, skill_lines.as_deref().map(|s| &s.catalog), @@ -286,10 +314,86 @@ fn feed_spellbook( script.fire_event("CURRENT_SPELL_CAST_CHANGED", vec![]); } } + // …and then the tab flash, in the reference's own order — `0x4b2b5a`'s re-sort + + // `SPELLS_CHANGED` precedes `0x4b2b92`'s `LEARNED_SPELL_IN_TAB` (decision 2252). Outside the + // diff block on purpose: a re-learn of a spell already in the book changes no snapshot, and + // the reference fires on the packet, not on a diff. + fire_tab_flashes( + &mut script, + &learned, + &tab_lines, + spells, + skill_lines.as_deref().map(|s| &s.catalog), + race, + class, + ); +} + +/// Fire `LEARNED_SPELL_IN_TAB` for each spell that just landed in the book — the tail of the +/// reference's registrar (`0x4b2b86 inc edi; push edi; push "%d"; push 0x1fe`), whose argument is +/// the **1-based** index of the spell's tab in the sorted tab list (decision 2252). +/// +/// The stock handler is `SpellBookFrame.lua:81` — `getglobal("SpellBookSkillLineTab"..arg1.."Flash") +/// :Show()` — so the index is an index into exactly the tab strip `GetNumSpellTabs`/ +/// `GetSpellTabInfo` publish, which is why it is read off `tab_lines` (the list this frame's +/// `build_book` just produced) rather than derived a second way. +/// +/// **The gate is `in_spellbook`, and that is not an approximation.** The reference reaches this +/// tail only past three returns — `Attributes & 0x80` (`0x4b2911`), `Attributes & 0x20` +/// (`0x4b2944`, the recipe arm returns outright) and `castUI > 0` (`0x4b29c4 jg`) — which is +/// bit-for-bit the book add-gate `SpellDisplay::in_spellbook` already models (decision 0227). A +/// spell that is not booked flashes nothing, and cannot: it has no tab. +fn fire_tab_flashes( + script: &mut UiScript, + learned: &[u32], + tab_lines: &[u32], + spells: &Spells, + skill_lines: Option<&SkillLineCatalog>, + race: u8, + class: u8, +) { + for &spell_id in learned { + let Some(index) = tab_flash_index(spell_id, tab_lines, spells, skill_lines, race, class) + else { + continue; + }; + debug!("ui_spellbook: spell {spell_id} landed in tab {index} — flash"); + script.fire_event("LEARNED_SPELL_IN_TAB", vec![ScriptValue::Int(index)]); + } +} + +/// The event's whole argument law, as a function of the book that was just built: the **1-based** +/// index of `spell_id`'s tab in `tab_lines`, or `None` for a spell that flashes nothing. +/// +/// `None` has exactly two causes and both are the reference's: a spell the book add-gate excludes +/// (see [`fire_tab_flashes`] — the three returns before `0x4b29c4`), and a spell whose tab is not +/// in the strip at all, which cannot happen for a booked spell because the strip was built from +/// the same set on the same frame. +fn tab_flash_index( + spell_id: u32, + tab_lines: &[u32], + spells: &Spells, + skill_lines: Option<&SkillLineCatalog>, + race: u8, + class: u8, +) -> Option { + if !spells + .catalog + .get(spell_id) + .is_some_and(|d| d.in_spellbook()) + { + return None; + } + let tab = skill_lines.map_or(NO_LINE, |c| c.spell_tab(spell_id, race, class)); + let index = tab_lines.iter().position(|&l| l == tab)?; + Some(index as i64 + 1) } /// Build the whole book (module doc): the app's own resolve — the engine holds no spell /// knowledge, only what's pushed here. +/// Returns the book **and the skill-line id behind each published tab, in the same order** — +/// the second half exists only for [`LearnedInTab`]'s index, and it is returned rather than +/// recomputed so there is exactly one tab ordering in the client (decision 2252). fn build_book( known: &BTreeSet, catalog: &SpellCatalog, @@ -298,7 +402,7 @@ fn build_book( class: u8, attack_icon: Option, ranged_icon: Option, -) -> SpellBookState { +) -> (SpellBookState, Vec) { let mut by_line: BTreeMap> = BTreeMap::new(); for &spell_id in known { // The byte-verified add-gate (module doc; `SpellDisplay::in_spellbook`): languages, @@ -338,8 +442,10 @@ fn build_book( lines.sort_by(|a, b| (a.0 != NO_LINE).cmp(&(b.0 != NO_LINE)).then(a.1.cmp(&b.1))); let mut tabs = Vec::with_capacity(lines.len()); + let mut tab_lines = Vec::with_capacity(lines.len()); let mut slots = Vec::new(); - for (_, name, texture, mut spell_ids) in lines { + for (line_id, name, texture, mut spell_ids) in lines { + tab_lines.push(line_id); spell_ids.sort_by_key(|&a| spell_sort_key(catalog, a)); let offset = slots.len() as u32; let num_spells = spell_ids.len() as u32; @@ -384,7 +490,7 @@ fn build_book( num_spells, }); } - SpellBookState { tabs, slots } + (SpellBookState { tabs, slots }, tab_lines) } /// The book comparator `0x4b30c0`'s name → rank tail (module doc; the category-ordinal head is the @@ -506,6 +612,60 @@ mod tests { } } + /// The flash argument: the 1-based index of the spell's tab in the strip `build_book` just + /// published — the reference's `0x4b2b86 inc edi` (decision 2252). With no skill-line catalog + /// every booked spell is in the pinned General tab, which is index 1. + #[test] + fn the_flash_index_is_one_based_into_the_published_tab_strip() { + let mut map = HashMap::new(); + map.insert(133, spell("Fireball", Some("Rank 2"), 0x10000)); + map.insert(668, spell("Language: Common", None, 0x80)); // DO_NOT_DISPLAY + map.insert(818, spell("Cooking", None, 0x20)); // IS_TRADESKILL + let mut castbar = spell("Some Castbar Spell", None, 0); + castbar.cast_ui = 2; + map.insert(1234, castbar); + let catalog = SpellCatalog::from_displays(map); + let known: BTreeSet = [133, 668, 818, 1234].into_iter().collect(); + let (book, tab_lines) = build_book(&known, &catalog, None, 1, 1, None, None); + + assert_eq!( + tab_lines.len(), + book.tabs.len(), + "one line id per published tab" + ); + assert_eq!(tab_lines, vec![NO_LINE], "General is the only tab here"); + + let spells = Spells { + catalog, + ..Spells::empty_for_tests() + }; + assert_eq!( + tab_flash_index(133, &tab_lines, &spells, None, 1, 1), + Some(1), + "a booked spell flashes its tab, 1-based" + ); + + // The three the reference returns before ever reaching `0x4b2b92` — bit-for-bit the book + // add-gate, so a spell that is not in the book has no tab to flash. + for (id, why) in [ + (668u32, "DO_NOT_DISPLAY returns at 0x4b2911"), + (818, "IS_TRADESKILL returns at 0x4b2944"), + (1234, "castUI > 0 returns at 0x4b29c4"), + ] { + assert_eq!( + tab_flash_index(id, &tab_lines, &spells, None, 1, 1), + None, + "{why}" + ); + } + + assert_eq!( + tab_flash_index(99999, &tab_lines, &spells, None, 1, 1), + None, + "an id with no Spell.dbc record flashes nothing" + ); + } + /// The add-gate + General tab through `build_book`, no skill lines (every shown spell lands /// in the pinned General tab): hidden spells (DO_NOT_DISPLAY / IS_TRADESKILL) never appear, /// shown spells sort name→rank, and the General tab takes the client's hardcoded name + icon. @@ -521,7 +681,7 @@ mod tests { let known: BTreeSet = [133, 145, 2136, 668, 818].into_iter().collect(); // No skill-line catalog → every shown spell lands in General (race/class irrelevant). - let book = build_book(&known, &catalog, None, 1, 1, None, None); + let book = build_book(&known, &catalog, None, 1, 1, None, None).0; // One General tab, hardcoded face; only the three shown spells. assert_eq!(book.tabs.len(), 1); @@ -562,14 +722,14 @@ mod tests { "Interface\\Icons\\INV_Sword_04", "Interface\\Buttons\\Spell-Reset", ] { - let book = build_book(&known, &catalog, None, 1, 1, Some(resolved.into()), None); + let book = build_book(&known, &catalog, None, 1, 1, Some(resolved.into()), None).0; assert_eq!(book.slots.len(), 1); assert_eq!(book.slots[0].spell_id, ATTACK); assert_eq!(book.slots[0].texture.as_deref(), Some(resolved)); } // No character to read (attack_icon None): falls back to the spell's own icon. - let bare = build_book(&known, &catalog, None, 1, 1, None, None); + let bare = build_book(&known, &catalog, None, 1, 1, None, None).0; assert_eq!( bare.slots[0].texture.as_deref(), Some("Interface\\Icons\\Temp") @@ -594,7 +754,7 @@ mod tests { let known: BTreeSet = [AUTO_SHOT, THROW].into_iter().collect(); let bow = "Interface\\Icons\\INV_Weapon_Bow_02"; - let book = build_book(&known, &catalog, None, 1, 1, None, Some(bow.into())); + let book = build_book(&known, &catalog, None, 1, 1, None, Some(bow.into())).0; let icon_of = |b: &SpellBookState, id: u32| { b.slots .iter() @@ -609,7 +769,7 @@ mod tests { ); // No ranged weapon: Auto Shot keeps its own icon (no Spell-Reset on the ranged path). - let bare = build_book(&known, &catalog, None, 1, 1, None, None); + let bare = build_book(&known, &catalog, None, 1, 1, None, None).0; assert_eq!( icon_of(&bare, AUTO_SHOT).as_deref(), Some("Interface\\Icons\\Ability_AutoShot") diff --git a/crates/benilla-app/src/ui_tooltip/mod.rs b/crates/benilla-app/src/ui_tooltip/mod.rs index eca054ec..37952b12 100644 --- a/crates/benilla-app/src/ui_tooltip/mod.rs +++ b/crates/benilla-app/src/ui_tooltip/mod.rs @@ -642,6 +642,27 @@ enum LastHover { /// The snapshot fields the unit tooltip's LINES read (everything except the bar's /// health/power) — the rebuild key: a change here means the rendered lines are stale. +/// The world-hover driver's own memory, bundled — which plate it put up, the line-affecting fields +/// that plate was built from, and the headless probe's say-once-on-change trace line (2255). +/// +/// One [`SystemParam`](bevy::ecs::system::SystemParam) rather than three parameters for the reason +/// [`crate::target::hover::GoPickSet`] is one: [`drive_mouseover_tooltip`] sits at Bevy's 16-param +/// function-system ceiling. A bare tuple did the same job and tripped `clippy::type_complexity`, +/// which is the lint asking for exactly this. +#[derive(bevy::ecs::system::SystemParam)] +pub(crate) struct HoverMemo<'s> { + /// Which world plate is currently up — the driver's "do not rebuild this every frame" memo. + /// **Faithful**: on an unchanged mouseover the reference makes no call at all (`0x482090` + /// returns at `0x4820b5`), so a plate Lua takes mid-hover staying gone is reference behaviour + /// and not a bug to fix here. See 2255. + last: Local<'s, crate::ui_script::VmMemo>, + /// The line-affecting fields the current unit plate was built from, so a late-arriving name or + /// creature-info rebuilds it under the same hover. + last_lines: Local<'s, crate::ui_script::VmMemo>>, + /// The headless probe's last trace line, so a stationary probe says it once (2255). + trace: Local<'s, String>, +} + fn lines_view(s: &UnitState) -> UnitState { UnitState { health: 0, @@ -701,8 +722,7 @@ fn drive_mouseover_tooltip( player_actions: Res, // The cursor seat crosses the VM seam (0582/0584): the anchor below is UI units, not px. ui_scale: Res, - mut last: Local>, - mut last_lines: Local>>, + mut memo: HoverMemo, // `ChrClasses.dbc` field 16 — `UnitHasRelicSlot`'s only input. Absent when the client data // failed to load, in which case no class reads as having a relic slot. classes: Option>, @@ -710,6 +730,7 @@ fn drive_mouseover_tooltip( let Some(mut script) = script else { return; }; + let (last, last_lines, trace) = (&mut memo.last, &mut memo.last_lines, &mut *memo.trace); let last = last.get(&script); let last_lines = last_lines.get(&script); let self_store = self_q.iter().next(); @@ -829,6 +850,15 @@ fn drive_mouseover_tooltip( // pick out the same set here. They diverge only for the three always-eligible types // (SPELL_FOCUS 8 / DUEL_ARBITER 16 / FISHINGHOLE 25), which is exactly where a pin would // settle it. Flagged INTERIM in 0766 rather than presented as verified. + // + // **THE PIN HAS ARRIVED, AND IT IS NARROWER THAN THIS LINE** (2255): `[vtbl+0x5c]` is + // `0x5f8630` = `template.data[0x621b00(type, 0x13)] != 0`, and semantic key `0x13` exists + // on exactly ONE of the 31 types — GENERIC(5), at `data[0]`. So the cursor arm is "GENERIC + // **with `data[0]` set**", and a GENERIC with it clear is corner-seated; the three + // always-eligible types above are corner-seated too, so 0766's "GENERIC" reading wins over + // its "not interactable" one. Applying it moves 190 of the 1387 hoverable type-5 templates + // from the cursor to the corner — a visible change, so it is its own slice and not a + // passenger on a bug fix (2255 carries the counts and the reasoning). let cursor_seated = stores.get(entity).map(|s| s.0.gameobject_type_id()) == Ok(GO_TYPE_GENERIC); // Window px → the VM's y-up 768-virtual units (÷s, the input seam's own conversion) — @@ -842,12 +872,22 @@ fn drive_mouseover_tooltip( // (2250), so an automated run can carry a GENERIC plate — which is // cursor-seated — all the way to the screen. A person's pointer always wins. w.cursor_position() - .or_else(|| crate::target::hover_probe_point(w)) + .or_else(crate::target::hover_probe_point) .map(|c| (c.x / s, (w.height() - c.y) / s)) }) }) .flatten(); if *last == LastHover::Go(guid) { + if crate::target::hover_probe_armed() { + let line = format!( + "held Go({guid:#x}) — plate up {}", + script.world_tooltip_up() + ); + if *trace != line { + info!("hover probe/tooltip: {line}"); + *trace = line; + } + } // The cursor arm follows the pointer; the corner arm has nothing to re-seat. if let Some((x, y)) = cursor_ui { script.world_tooltip_move(x, y); diff --git a/crates/benilla-ui/src/script/tooltip_unit.rs b/crates/benilla-ui/src/script/tooltip_unit.rs index 54945107..4b39b485 100644 --- a/crates/benilla-ui/src/script/tooltip_unit.rs +++ b/crates/benilla-ui/src/script/tooltip_unit.rs @@ -31,7 +31,7 @@ use mlua::{Lua, Table}; use super::object::frame_handle_of; -use super::tooltip::{append_line, clear_content, fire_cleared, show_or_hide_empty}; +use super::tooltip::{append_line, clear_content, fire_cleared, show_or_hide_empty, tip_mut}; // The grey band + trivial/GREY check (`0x5f0700`, the CIVILIAN line's last gate: a green-or- // better con never warns of a dishonorable kill) and the "??" gate live in one shared home // (`unit.rs`), alongside `UnitLevel`'s −1 return and the `GetQuestGreenRange` binding. @@ -397,7 +397,7 @@ impl super::UiScript { lines: &[(String, TooltipTint)], cursor: Option<(f32, f32)>, ) -> bool { - let (h, id, root_id) = { + let (h, id, root, root_id) = { let mut model = self.model_mut(); let Some(h) = model.arena.lookup("GameTooltip") else { return false; @@ -406,13 +406,37 @@ impl super::UiScript { return false; }; let (id, root_id) = (model.frame_id(h), model.frame_id(root)); - (h, id, root_id) + (h, id, root, root_id) }; match cursor { // The cursor arm: seated centred above the pointer, clamped by the tooltip frame's own // flag. Compare-then-touch so a still pointer never re-layouts. Some((ui_x, ui_y)) => { let mut model = self.model_mut(); + // **The OWNER, which this arm used to skip** (decision 2255). The + // reference's cursor arm is `0x492a01 → 0x52ffe0(owner, 6, 0, 0)` — the SetOwner + // CORE, not a bare re-anchor — and the core's `0x53000c` stores that owner into + // `+0x314`. Every OTHER world plate we build reaches an owner by accident: the + // corner arm below and the unit flow both fire `OnTooltipSetDefaultAnchor`, whose + // FrameXML handler calls `GameTooltip:SetOwner(UIParent, …)` in Lua. This arm + // fires nothing, so `+0x314` stayed NULL on exactly the cursor-seated objects — + // a signpost, a mailbox, every GENERIC(5). + // + // A null owner is not cosmetic, because **`:Show()` is an existence gate**: + // `0x530a80` shows only when `+0x314` AND `+0x31c` are both non-zero, and + // otherwise takes the effective-hide `0x530a60` — which we implement faithfully + // (see the `Show` verb). So any Lua that reached `GameTooltip:Show()` while one of + // these plates was up HID IT, and hid it through our own correct code. + // + // That is not hypothetical: `!Questie` hooks GameTooltip's `OnShow` at + // PLAYER_LOGIN (`Questie:hookTooltip`, installed because stock GameTooltip has no + // OnShow of its own) and its handler ends in `GameTooltip:Show()`. So the plate's + // own show event hid the plate, ~26 ms after the engine built it, on every + // signpost, for the whole session — the director's report. The reference cannot + // reach that state: `0x492a01` writes the owner before the plate is ever shown. + if let Ok(t) = tip_mut(&mut model, h) { + t.owner = Some(root); + } let input = model.layout_inputs.entry(h).or_default(); let new = Anchor::new(Point::Bottom, root_id, Point::BottomLeft, ui_x, ui_y); let same = input.anchors.len() == 1 @@ -585,6 +609,24 @@ impl super::UiScript { } } + /// **Is `GameTooltip` still the plate a world hover put up?** — world-owned (no Lua + /// `SetOwner`, no content `Set*` and no `Hide` has taken it since) and still shown. + /// + /// The world-hover driver keeps a host-side memo of which plate it put up so it does not + /// rebuild one per frame; that memo is about the HOVER, and this is the other half of the + /// question — whether the plate that memo describes is still on the screen. + pub fn world_tooltip_up(&mut self) -> bool { + let model = self.model_mut(); + let Some(h) = model.arena.lookup("GameTooltip") else { + return false; + }; + model + .arena + .frame(h) + .map(|f| matches!(&f.kind_state, KindState::Tooltip(t) if t.world_owned) && f.shown) + .unwrap_or(false) + } + /// Arm the mouseover tooltip's fade-out (hover loss — the byte law arms a timestamped fade, /// never an instant hide). No-op when the tooltip isn't showing a world unit. pub fn world_tooltip_fade(&mut self) {