From 293b8e57fbddb2fa7f2efca89c38b51c8a079e2e Mon Sep 17 00:00:00 2001 From: Sam Date: Wed, 23 Sep 2026 12:19:22 +0700 Subject: [PATCH] creatures keep their first corner, and characters never share files - a creature walking a path no longer cuts its first corner: the path's first waypoint was dropped at decode (2342) - two characters whose names differ only in accented or non-Latin letters no longer share macros, bindings and SavedVariables; a file that failed to load is not overwritten at logout (2342) - custom chat channels survive a logout, a collapsed quest header keeps its quests watched, and a trackpad swipe scrolls as far as it adds up to (2342) - a hearth off a zeppelin no longer lands back on its deck, and an opened lockbox with loot left in it unlocks (2342) - every snapshot runs on Vulkan, in a container, before it ships (2344) Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/benilla-app/src/addon_harness/mod.rs | 1 + crates/benilla-app/src/bindings.rs | 58 ++++- crates/benilla-app/src/combat_log/chat.rs | 2 +- crates/benilla-app/src/combat_log/mod.rs | 4 +- crates/benilla-app/src/cvars.rs | 37 ++- crates/benilla-app/src/game_plugins.rs | 24 +- crates/benilla-app/src/local_state.rs | 45 +++- crates/benilla-app/src/pending_item_ops.rs | 39 ++- crates/benilla-app/src/player/controller.rs | 20 +- crates/benilla-app/src/player/movement_net.rs | 125 ++++++++++ crates/benilla-app/src/player/wire_in.rs | 81 ++++-- crates/benilla-app/src/realm_select/input.rs | 68 ++++- crates/benilla-app/src/spell/cast_send.rs | 52 +++- crates/benilla-app/src/ui_auction/net.rs | 4 +- crates/benilla-app/src/ui_chat/edit.rs | 37 +++ crates/benilla-app/src/ui_chat/feed.rs | 3 + crates/benilla-app/src/ui_chat/input/mod.rs | 3 +- crates/benilla-app/src/ui_chat/recruitment.rs | 72 +++++- crates/benilla-app/src/ui_chat/settings.rs | 233 +++++++++++++----- crates/benilla-app/src/ui_items/drain.rs | 7 +- crates/benilla-app/src/ui_loot/net.rs | 174 ++++++++++++- crates/benilla-app/src/ui_mail/net.rs | 4 +- crates/benilla-app/src/ui_quest_log.rs | 11 +- crates/benilla-app/src/ui_saved.rs | 66 ++++- crates/benilla-app/src/ui_script/addons.rs | 14 +- .../benilla-app/src/ui_script/extract/mod.rs | 12 +- crates/benilla-app/src/ui_script/input.rs | 121 ++++++++- crates/benilla-app/src/ui_script/lifecycle.rs | 80 +++--- crates/benilla-app/src/ui_script/manifest.rs | 112 +++++++-- crates/benilla-app/src/ui_script/mod.rs | 1 + .../src/ui_script/options_tests.rs | 6 +- .../src/ui_script/quest_timer_tests.rs | 1 + .../src/ui_script/questlog_tests.rs | 2 + .../src/ui_script/trainer_tests.rs | 2 +- .../src/ui_script/world_entry_tests.rs | 60 +++++ .../src/ui_script/world_map_tests.rs | 49 ++++ crates/benilla-app/src/ui_trade/net.rs | 4 +- crates/benilla-app/src/ui_unit.rs | 6 +- .../src/messages/monster_move.rs | 116 +++++---- crates/benilla-protocol/src/messages/parse.rs | 24 +- crates/benilla-ui/src/script/addon.rs | 38 ++- crates/benilla-ui/src/script/button.rs | 3 +- crates/benilla-ui/src/script/chat_window.rs | 5 - crates/benilla-ui/src/script/colorselect.rs | 3 +- crates/benilla-ui/src/script/cursor.rs | 10 +- crates/benilla-ui/src/script/cvars.rs | 11 +- crates/benilla-ui/src/script/editbox.rs | 6 +- crates/benilla-ui/src/script/event.rs | 32 +-- crates/benilla-ui/src/script/keyboard.rs | 6 +- crates/benilla-ui/src/script/merchant.rs | 100 +++++++- crates/benilla-ui/src/script/model.rs | 9 +- crates/benilla-ui/src/script/quest_log.rs | 72 +++++- crates/benilla-ui/src/script/saved.rs | 154 ++++++++---- crates/benilla-ui/src/script/scrollframe.rs | 6 +- crates/benilla-ui/src/script/slider.rs | 3 +- crates/benilla-ui/src/script/statusbar.rs | 6 +- crates/benilla-ui/src/script/tests/mod.rs | 48 ++++ crates/benilla-ui/src/script/tooltip/mod.rs | 3 +- .../benilla-ui/src/script/tooltip_item/mod.rs | 3 +- crates/benilla-ui/src/script/tooltip_unit.rs | 3 +- 60 files changed, 1876 insertions(+), 425 deletions(-) diff --git a/crates/benilla-app/src/addon_harness/mod.rs b/crates/benilla-app/src/addon_harness/mod.rs index 1e2d0ba7..bef8944f 100644 --- a/crates/benilla-app/src/addon_harness/mod.rs +++ b/crates/benilla-app/src/addon_harness/mod.rs @@ -1812,6 +1812,7 @@ fn seat_a_session(script: &mut UiScript) { script.set_quest_log(QuestLogState { entries: vec![header, in_progress, done], num_quests: 2, + hidden_quest_ids: Vec::new(), }); } diff --git a/crates/benilla-app/src/bindings.rs b/crates/benilla-app/src/bindings.rs index 6379fd1e..1d6a221c 100644 --- a/crates/benilla-app/src/bindings.rs +++ b/crates/benilla-app/src/bindings.rs @@ -42,7 +42,7 @@ //! on the confirmed switch back to general — the reference's own semantics. use bevy::input::keyboard::KeyboardInput; -use bevy::input::mouse::AccumulatedMouseScroll; +use bevy::input::mouse::{AccumulatedMouseScroll, MouseScrollUnit}; use bevy::input::ButtonState; use bevy::prelude::*; @@ -434,6 +434,51 @@ fn sync_dispatch(script: Option>, mut dispatch: ResMut f32 { + match unit { + MouseScrollUnit::Line => dy, + MouseScrollUnit::Pixel => dy / MouseScrollUnit::SCROLL_UNIT_CONVERSION_FACTOR, + } +} + +/// **Whole notches out of a stream of line deltas**, carrying the fraction between frames. +/// +/// For the consumers that act per NOTCH rather than per unit of travel — the UI's +/// `OnMouseWheel` (stock `ScrollFrameTemplate_OnMouseWheel` moves half a pane per call and +/// `ChatFrame_OnMouseWheel` a line, both off the sign alone) and the realm list's row step. Fed a +/// trackpad's per-frame pixel trickle raw, each of those fired once per frame, so a gentle swipe +/// slammed a pane to its end; through this, ten frames of a tenth of a line are one notch. +/// +/// A reversal drops the carried fraction: travel the other way is a new gesture, and it should +/// not first have to undo the remainder of the last one. +/// +/// A `Resource` for the UI's feed (a fact about the input device's gesture, which outlives any one +/// VM), a `Local` for the realm list's. +#[derive(Default, Resource)] +pub(crate) struct WheelNotches { + carry: f32, +} + +impl WheelNotches { + /// Add `lines` of travel; returns the signed whole notches it completes (usually 0 or ±1). + pub(crate) fn feed(&mut self, lines: f32) -> i32 { + if lines == 0.0 || !lines.is_finite() { + return 0; + } + if self.carry != 0.0 && self.carry.signum() != lines.signum() { + self.carry = 0.0; + } + self.carry += lines; + let whole = self.carry.trunc(); + self.carry -= whole; + whole as i32 + } +} + /// The dispatch pass — see the module doc. Runs right after the UI key feed (same frame's /// capture gate), before `WorldStage::Input` (a bound key must act this frame, once). fn latch_and_dispatch( @@ -686,14 +731,9 @@ fn latch_and_dispatch( // `RunCommand 0x4b7b50` no-op: `UP + !runOnUp` returns without running anything). Before this // the notch was a press with no release, which quietly made every press+release command a // dead wheel binding. Over UI the wheel belongs to the hovered frame. - // Trackpads report pixel deltas — normalized to line-equivalents so the zoom consumer's - // feel is unchanged from when it read the scroll itself. - let wheel = match scroll.unit { - bevy::input::mouse::MouseScrollUnit::Line => scroll.delta.y, - bevy::input::mouse::MouseScrollUnit::Pixel => { - scroll.delta.y / bevy::input::mouse::MouseScrollUnit::SCROLL_UNIT_CONVERSION_FACTOR - } - }; + // Trackpads report pixel deltas — normalized to line-equivalents ([`wheel_lines`]) so the + // zoom consumer's feel is unchanged from when it read the scroll itself. + let wheel = wheel_lines(scroll.unit, scroll.delta.y); // **Over CHROME**, not over any UI at all: the wheel still zooms with the cursor on a // nameplate, which is a mouse-enabled widget and not a panel (`PointerOverUiPanel`). Plates sit // over exactly the things you look at, so the raw flag silently killed scroll-zoom wherever one diff --git a/crates/benilla-app/src/combat_log/chat.rs b/crates/benilla-app/src/combat_log/chat.rs index c90d0429..c8b1d3a1 100644 --- a/crates/benilla-app/src/combat_log/chat.rs +++ b/crates/benilla-app/src/combat_log/chat.rs @@ -3,7 +3,7 @@ //! //! The split is the reference's own. `0x629b60` is the display dispatcher: one classification of //! both endpoints, then a branch per outcome into a formatter that emits *text*. The floating -//! number comes off a different path entirely (`0x625010`, the world-anchored spawn), gated by +//! number comes off a different path entirely (`0x629d30`, which fires `COMBAT_TEXT_UPDATE`), gated by //! different CVars, with a different source law — which is why our two live in different files //! rather than in one arm with two tails. Everything here composes; the law it composes with is //! [`crate::ui_chat::combat`]. diff --git a/crates/benilla-app/src/combat_log/mod.rs b/crates/benilla-app/src/combat_log/mod.rs index 4b123b73..71576b1d 100644 --- a/crates/benilla-app/src/combat_log/mod.rs +++ b/crates/benilla-app/src/combat_log/mod.rs @@ -2,8 +2,8 @@ //! handler table since 2323, moved out of the drain's `combat_chat` and `combat_log` arm files). //! Every combat packet has two consumers, and the reference emits both from one handler: the //! **chat line** ([`chat`], the `0x629b60` display dispatcher's text) and the **floating -//! number** with the portrait's `UNIT_COMBAT` flash ([`text`], the `0x625010` world-anchored -//! spawn). They are gated by different CVars and classify by different laws, which is why the +//! number** with the portrait's `UNIT_COMBAT` flash ([`text`], `0x629d30` → +//! `COMBAT_TEXT_UPDATE`; `0x625010` is the melee blood spurt, not this). They are gated by different CVars and classify by different laws, which is why the //! legs are two files; they answer one packet, which is why the handler is one — the line first, //! the number second, as the drain's match ran them. //! diff --git a/crates/benilla-app/src/cvars.rs b/crates/benilla-app/src/cvars.rs index 96a3ba82..f67c0445 100644 --- a/crates/benilla-app/src/cvars.rs +++ b/crates/benilla-app/src/cvars.rs @@ -29,9 +29,9 @@ //! written workaround). The registry never applies anything itself: it has no knob, and no arm. //! //! - **The latch** ([`Row::pending`]): a write to a latched row is staged, `GetCVar` keeps -//! answering the applied value, and nothing fires until [`Cvars::commit_latched`] — which is -//! the reference's `CVar::Update 0x63e060`, called for the `gx*` rows from inside `RestartGx` -//! (the video window's Okay). A staged value that is never committed is dropped at exit, as +//! answering the applied value, and nothing fires until [`Cvars::commit_latched`] — the +//! reference's `0x639ec0`, `CVar::Commit 0x63e060` on the fourteen gx records, called from +//! inside `RestartGx` (the video window's Okay). A staged value that is never committed is dropped at exit, as //! the reference's is (`SaveConfig` writes `rec+0x20`, the applied value). //! //! - **Boot**: read `benilla-config/config.toml` ([`crate::local_state`]) into the registry and @@ -1433,14 +1433,26 @@ impl Cvars { true } - /// **The latch boundary** — the reference's `CVar::Update 0x63e060`: every staged value - /// becomes the applied one, fires, persists, and reaches the mirror. Returns how many moved. - /// Called for the `gx*` rows from `RestartGx` ([`crate::video`]); a row nothing ever commits - /// (`SoundBufferSize`, `gxApi`) holds its stage until exit, and loses it there, as the - /// reference does. + /// **The latch boundary `RestartGx` crosses** — the reference's `0x639ec0`, fourteen + /// `CVar::Commit 0x63e060` calls on fourteen gx records (`[0xc4ea90]` … `[0xc4eab4]`, + /// `0x639ec0..0x639f5a`): each of THOSE staged values becomes the applied one, fires, + /// persists, and reaches the mirror. Returns how many moved. + /// + /// **Only the `gx*` rows.** A latched row outside the video set is not on that list and + /// nothing else commits it: `SoundBufferSize`'s register site (`0x4571ca`) discards the + /// record pointer `0x63db90` returns, so no global holds it for any commit to name. It + /// holds its stage until exit and loses it there, as the reference's does. Until 2343 this + /// walked every row, and a console `SoundBufferSize` rode the next video Okay into the file. + /// + /// The reference runs a gx CVar's change callback at `SetCVar` time (`0x63df50`), and the + /// commit fires nothing; our observers fire here instead. Nothing reads the difference + /// today — the appliers are the device rebuild's, and that runs after this boundary. pub(crate) fn commit_latched(&mut self) -> usize { let mut moved = 0; for row in &mut self.rows { + if !row.name.starts_with("gx") { + continue; + } let Some(staged) = row.pending.take() else { continue; }; @@ -3283,6 +3295,15 @@ mod tests { assert_eq!(cvars.set("gxVSync", "0"), SetOutcome::Unchanged); assert_eq!(cvars.row("gxVSync").unwrap().pending, None); assert_eq!(cvars.commit_latched(), 0); + // A latched row outside the gx set is not the video restart's to commit (2343). + assert_eq!(cvars.set("SoundBufferSize", "200"), SetOutcome::Staged); + assert_eq!(cvars.commit_latched(), 0); + assert_eq!(cvars.get("SoundBufferSize"), Some("100")); + assert_eq!( + cvars.row("SoundBufferSize").unwrap().pending.as_deref(), + Some("200") + ); + assert!(!cvars.compose().contains_key("SoundBufferSize")); // An unlatched row applies at once, fires, and dirties. assert_eq!(cvars.set("MusicVolume", "0.7"), SetOutcome::Changed); assert_eq!(cvars.get("MusicVolume"), Some("0.7")); diff --git a/crates/benilla-app/src/game_plugins.rs b/crates/benilla-app/src/game_plugins.rs index e03a584e..25665a78 100644 --- a/crates/benilla-app/src/game_plugins.rs +++ b/crates/benilla-app/src/game_plugins.rs @@ -754,13 +754,13 @@ pub(crate) mod schedule_tests { /// writers commute, and a drain's order against a writer is one frame of latency, never /// a loss. (2283's ten pairs were all this shape: a new verb drain against its siblings /// over the chat and error sinks.) - /// - **An exclusive edge** — a pair bevy reports with NO component list: one side is a - /// build-inserted `ApplyDeferred` sync point or an exclusive system, which conflicts on - /// `World` itself. Its order against a system it shares nothing with is immaterial by - /// construction (the build already placed the barrier after the commands it flushes and - /// before their dependents), and there is nothing to declare it against. Until 2304 an - /// empty list read as "VM only" vacuously, so that class rose and fell with the barrier - /// count; now it is named. + /// - **Not an exclusive edge.** A pair bevy reports with NO component list has an exclusive + /// system on one side (`bevy_ecs` `node.rs:625`: it reports every unordered pair with an + /// exclusive system and never reads its access). 2304 explained that class as the + /// build's barriers; the ratchet reads the declared graph since 2337, which has none, so + /// what it held was only the hand-written exclusive systems — the net drain, whose 143 + /// unordered partners read this frame's packets or the last frame's by coin, and + /// `finish_colliders`. Those are undeclared orders like any other: actionable (2343). /// - **A random stream** — `SoundKits`, which every sound system holds to play a kit: a /// decode cache, a per-kit last-variation memory and one xorshift stream. Any /// interleaving of draws is a valid draw, and that is the reference's own contract for @@ -855,8 +855,10 @@ pub(crate) mod schedule_tests { /// Which class explains this pair — `None` if it is actionable. pub fn class_of(&self, what: &[ComponentId]) -> Option<&'static str> { + // An exclusive system on one side: bevy cannot see its access, so nothing here can + // argue the pair away (see the doc above: not an exclusive edge). if what.is_empty() { - return Some("exclusive"); + return None; } if !what.iter().all(|id| self.explains(*id)) { return None; @@ -1013,7 +1015,11 @@ pub(crate) mod schedule_tests { /// Raising this ceiling is a claim that a new undeclared order is acceptable; make it with /// the reason, or declare the order instead (`.after`, a set, a `chain`). If the pair is /// about a resource that commutes by construction, the claim belongs in [`Classes`]. - const UPDATE_ACTIONABLE_CEILING: usize = 5_063; + /// **5,608 (decision 2343)** — not a raise: a re-measurement. The 556 pairs with an + /// exclusive system on one side were read as "explained" by a class argued for the build's + /// barriers, which the declared graph (2337) no longer has; 413 are `finish_colliders`', 143 + /// the net drain's. Same tree, the class dropped, nothing else moved: 5,052 + 556. + const UPDATE_ACTIONABLE_CEILING: usize = 5_608; const UPDATE_ACTIONABLE_SLACK: usize = 40; fn ratchet(what: &str, n: usize, ceiling: usize, slack: usize) { diff --git a/crates/benilla-app/src/local_state.rs b/crates/benilla-app/src/local_state.rs index e1b82c01..e28e9087 100644 --- a/crates/benilla-app/src/local_state.rs +++ b/crates/benilla-app/src/local_state.rs @@ -327,13 +327,22 @@ pub(crate) fn fps_journal_path() -> Option { diagnostics_dir().map(|d| d.join("fps-journal.csv")) } -/// Make an arbitrary realm/character name safe as one path component: anything outside -/// `[A-Za-z0-9_]` becomes `_`, so a realm called `Hydraxian Waterlords` or one with a slash cannot +/// Make an arbitrary realm/character name safe as one path component: anything that is not a +/// letter or digit becomes `_`, so a realm called `Hydraxian Waterlords` or one with a slash cannot /// escape the folder or collide with the path separator. +/// +/// **Letters are any script's, kept as they are** — the reference names its per-character folder +/// with the raw name (`WTF/Account////`). This was ASCII-only, which folded every +/// other letter to `_`: vmangos accepts extended-Latin, Cyrillic and East-Asian names by default +/// (`StrictPlayerNames = 0`), so `Zoë` and `Zoé` — or any two Cyrillic names of one length — shared +/// one set of macros, bindings, addon state and SavedVariables, and each login saved over the +/// other's. A character name is letters only (vmangos `isValidString`), so keeping every letter +/// makes the token one-to-one for them; an ASCII name maps exactly as before, so no existing file +/// moves. fn file_token(s: &str) -> String { let t: String = s .chars() - .map(|c| if c.is_ascii_alphanumeric() { c } else { '_' }) + .map(|c| if c.is_alphanumeric() { c } else { '_' }) .collect(); if t.is_empty() { "unknown".into() @@ -346,13 +355,19 @@ fn file_token(s: &str) -> String { /// a crash mid-write leaves the old file intact, never a truncated one. The one write path for /// every resident of the folder. pub(crate) fn write_atomic(path: &Path, contents: &str) -> std::io::Result<()> { + write_atomic_bytes(path, contents.as_bytes()) +} + +/// [`write_atomic`] for a resident that is bytes, not text — the saved-variables files, which +/// carry a Lua byte string's bytes as they are. +pub(crate) fn write_atomic_bytes(path: &Path, contents: &[u8]) -> std::io::Result<()> { if let Some(dir) = path.parent() { std::fs::create_dir_all(dir)?; } let tmp = path.with_extension("tmp"); { let mut f = std::fs::File::create(&tmp)?; - f.write_all(contents.as_bytes())?; + f.write_all(contents)?; f.sync_all()?; } std::fs::rename(&tmp, path) @@ -394,6 +409,28 @@ mod tests { use super::test_env::{EnvGuard, ENV_LOCK}; use super::*; + /// **Two characters never share a file.** Every per-character resident keys on this token, so + /// a collision is one character's macros, bindings and SavedVariables loaded and then saved + /// over by another. ASCII names — every file already on disk — keep their exact paths. + #[test] + fn distinct_names_never_share_a_token() { + assert_ne!(file_token("Вася"), file_token("Петя")); + assert_ne!(file_token("Zoë"), file_token("Zoé")); + assert_eq!(file_token("Zoë"), "Zoë"); + assert_eq!(file_token("Onehunter"), "Onehunter"); + assert_eq!(file_token("Hydraxian Waterlords"), "Hydraxian_Waterlords"); + assert_eq!( + file_token("a/b\\c:d"), + "a_b_c_d", + "separators never survive" + ); + assert_eq!( + file_token("realm-name"), + "realm_name", + "the key's own `-` stays unforgeable" + ); + } + /// The residents' layout, exercised through the override — which is the *only* step of /// [`home`] whose answer a test can state, since the other two are the machine's project /// folder and the test runner's own exe directory. Those two get their own tests below; what diff --git a/crates/benilla-app/src/pending_item_ops.rs b/crates/benilla-app/src/pending_item_ops.rs index 5687cc06..04b8db64 100644 --- a/crates/benilla-app/src/pending_item_ops.rs +++ b/crates/benilla-app/src/pending_item_ops.rs @@ -118,7 +118,21 @@ impl PendingItemOps { if !matched { return self.clear_all(); } + self.clear_by_guid(item_guid) + } + + /// **`UnlockItem 0x495420`** — clear every entry naming `item_guid`, and nothing else: a guid + /// no entry recorded (a corpse, a chest, 0) unlocks nothing, exactly as the reference's + /// resolve-as-ITEM (typemask 2) finds nothing to clear for a non-item. The loot close is the + /// caller that needs it: an opened lockbox/clam is locked at the `CMSG_OPEN_ITEM` send (0916) + /// and, closed with loot left, never changes its slot — so neither [`Self::resolve`] nor a + /// failure ever clears it (wow-re `ui/ledger.tsv` `0x48f200` @ `48f299`). Returns the + /// deduplicated `(bag, slot)` pairs that unlocked. + pub(crate) fn clear_by_guid(&mut self, item_guid: u64) -> Vec<(i64, u32)> { let mut unlocked = Vec::new(); + if item_guid == 0 { + return unlocked; + } self.entries.retain(|entry| { let hit = entry.iter().any(|&(_, (guid, _))| guid == item_guid); if hit { @@ -151,9 +165,10 @@ impl PendingItemOps { } } -/// `(bag, slot)` pairs whose app-lock cleared this frame, from EITHER clear: the server failure -/// (`net/apply/loot.rs::inventory_failure`, which owns the wire event but has no `UiScript` to -/// fire `ITEM_LOCK_CHANGED` through) and the resolving field-update watch +/// `(bag, slot)` pairs whose app-lock cleared this frame, from ANY clear: the server failure +/// (`ui_items::net::inventory_failure`, which owns the wire event but has no `UiScript` to +/// fire `ITEM_LOCK_CHANGED` through), the loot close's item unlock (`ui_loot::net`, the same +/// shape) and the resolving field-update watch /// (`ui_items::feed::resolve_item_locks`, which runs ahead of every feed so that no feed can push /// a `locked` the clear has already invalidated). `feed_containers` drains this and fires — the /// exact shape `ui_items::EquipErrors` already uses for the same reason. @@ -253,6 +268,24 @@ mod tests { assert!(p.contains(1, 3), "op B untouched — a different guid"); } + /// Unlike the failure clear, the guid clear has no clear-all fallback: an unmatched or zero + /// guid (a corpse's release) must leave every other lock standing. + #[test] + fn clear_by_guid_clears_only_the_named_item() { + let mut p = PendingItemOps::default(); + p.add([(0, 3, 100, 1)]); // the opened lockbox + p.add([(1, 2, 200, 1)]); // an unrelated move + + assert!(p.clear_by_guid(999).is_empty(), "an unrecorded guid"); + assert!(p.clear_by_guid(0).is_empty(), "the zero guid fires nothing"); + assert!(p.contains(0, 3) && p.contains(1, 2)); + + let epoch = p.epoch(); + assert_eq!(p.clear_by_guid(100), vec![(0, 3)]); + assert!(!p.contains(0, 3) && p.contains(1, 2)); + assert!(p.epoch() > epoch, "the feed's gate sees the lock set move"); + } + #[test] fn clear_by_failure_clears_everything_on_zero_or_unmatched_guid() { let mut p = PendingItemOps::default(); diff --git a/crates/benilla-app/src/player/controller.rs b/crates/benilla-app/src/player/controller.rs index c739be57..a9276750 100644 --- a/crates/benilla-app/src/player/controller.rs +++ b/crates/benilla-app/src/player/controller.rs @@ -560,6 +560,8 @@ pub(super) fn control( if !player.server_riding { movement_net::park_mover(&net.0 .0, &mut player); } + // After the park, so a fear's ack carries the stopped word the park just reported. + movement_net::ack_speeds_undriven(&net.0 .0, &player, &speed_acks); return; } // This frame's netted movement axes, the mouselook/turn modes they imply, and the autorun @@ -1182,22 +1184,8 @@ pub(super) fn control( // jump/fall lifecycle, and a ~500 ms heartbeat, each carrying the live `MovementInfo` (decisions // 0052 + 0053). vmangos relays it to nearby players, who extrapolate from the flags. See the // [`movement_net`] module (the outbound mirror of `net::motion`'s remote integration). - // The rider's local pose for the wire's ON_TRANSPORT tail: `bevy_to_wow` is a pure basis - // rotation, so the boat-local Bevy vector converts directly, and the local orientation is - // `face_yaw − boat_yaw` (the GetAbsoluteFacing law in reverse), normalized like any wire - // orientation. - let wire_transport = player.ride.as_ref().map(|r| { - let local = benilla_assets::coords::bevy_to_wow(r.local_pos); - benilla_protocol::TransportPose { - guid: r.guid, - pos: benilla_protocol::wire::Vector3d { - x: local[0], - y: local[1], - z: local[2], - }, - orientation: (player.face_yaw - r.boat_yaw).rem_euclid(std::f32::consts::TAU), - } - }); + // The rider's local pose for the wire's ON_TRANSPORT tail. + let wire_transport = movement_net::wire_transport(&player); // The skipped-time clock (decision 1935): a held frame is a frame of movement simulation // we advanced through without integrating, and the mover we name is the one we are // actually driving — a possessed unit while we hold its reins. Read out before the call diff --git a/crates/benilla-app/src/player/movement_net.rs b/crates/benilla-app/src/player/movement_net.rs index 93756e30..5f630852 100644 --- a/crates/benilla-app/src/player/movement_net.rs +++ b/crates/benilla-app/src/player/movement_net.rs @@ -580,6 +580,64 @@ pub(super) fn park_mover(sender: &Sender, player: &mut Player) { player.last_facing = facing; } +/// The rider's boat-local pose for the wire's `ON_TRANSPORT` tail, or `None` off a deck. +/// `bevy_to_wow` is a pure basis rotation, so the boat-local Bevy vector converts directly, and the +/// local orientation is `face_yaw − boat_yaw` (the GetAbsoluteFacing law in reverse), normalized +/// like any wire orientation. +pub(super) fn wire_transport(player: &Player) -> Option { + player.ride.as_ref().map(|r| { + let local = bevy_to_wow(r.local_pos); + TransportPose { + guid: r.guid, + pos: benilla_protocol::wire::Vector3d { + x: local[0], + y: local[1], + z: local[2], + }, + orientation: (player.face_yaw - r.boat_yaw).rem_euclid(std::f32::consts::TAU), + } + }) +} + +/// **The forced-speed acks owed on a frame the controller does not drive** — a server spline +/// (`server_riding`), a fear (`control_lost`) or a mover hand-off (`reseat`). Those frames return +/// before [`stream_self_movement`], which is where a controlled frame's acks go out, so until this +/// the change was applied locally and never answered: vmangos holds an unacked change for +/// `PendingAckResponseTime` (4 s) before enforcing it, counts it in `OnFailedToAckChange` (a kick +/// under the default anticheat penalty) and blocks the graveyard repop while one is pending. The +/// reference acks from its per-mover drain whoever is driving (`0x616142`/`0x61812d`). +/// +/// The payload is the honest state we last reported: our streamed flags (a ride's deliberate +/// FORWARD included) with the transport tail when we are on a deck, minus the airborne pair — no +/// arc is being integrated on these frames, so there is no jump tail to send with them. +pub(super) fn ack_speeds_undriven( + sender: &Sender, + player: &Player, + acks: &[crate::net::SpeedChangeMessage], +) { + let transport = wire_transport(player); + let mut flags = + player.move_flags & OUTBOUND_FLAG_MASK & !(move_flags::FALLING | move_flags::FALLING_FAR); + if transport.is_none() { + flags &= !move_flags::ON_TRANSPORT; // flag and tail travel together + } + for ack in acks { + let _ = sender.send(ClientCommand::ForceSpeedAck { + kind: ack.kind, + guid: ack.guid, + counter: ack.counter, + speed: ack.speed, + flags, + pos: bevy_to_wow(player.pos), + orientation: player.face_yaw.rem_euclid(std::f32::consts::TAU), + pitch: 0.0, + fall_time: 0, + jump: None, + transport: transport.filter(|_| flags & move_flags::ON_TRANSPORT != 0), + }); + } +} + #[cfg(test)] mod tests { /// A frame that integrated everything it was given — the skipped-time report's no-op input, @@ -1440,6 +1498,73 @@ mod tests { assert_eq!(player.move_flags, 0, "bookkeeping is zeroed after parking"); } + /// **A forced speed change is acked on a frame nobody is driving** — a fear, a server spline, + /// a mover hand-off. Those frames return before the stream, which is where a controlled + /// frame's acks go out, so the change was applied and never answered (vmangos enforces it 4 s + /// late, counts the miss toward its anticheat kick, and blocks the graveyard repop meanwhile). + /// The payload is the honest reported word: the ride's FORWARD kept, the airborne pair dropped + /// (no arc tail exists here), and `ON_TRANSPORT` only with the tail that must travel with it. + #[test] + fn a_speed_change_is_acked_while_not_driving() { + let (tx, rx) = crossbeam_channel::unbounded(); + let ack = crate::net::SpeedChangeMessage { + guid: 7, + kind: benilla_protocol::SpeedKind::Run, + counter: 3, + speed: 3.5, + }; + + // A spline ride reporting FORWARD, off any deck, with a stale ON_TRANSPORT bit. + let player = Player { + move_flags: move_flags::FORWARD | move_flags::FALLING | move_flags::ON_TRANSPORT, + face_yaw: -1.0, + ..Default::default() + }; + ack_speeds_undriven(&tx, &player, &[ack]); + let Ok(ClientCommand::ForceSpeedAck { + counter, + speed, + flags, + orientation, + transport, + jump, + .. + }) = rx.try_recv() + else { + panic!("the change is acked"); + }; + assert_eq!((counter, speed), (3, 3.5), "the ack echoes the change"); + assert_eq!( + flags, + move_flags::FORWARD, + "FORWARD kept; FALLING and a tailless ON_TRANSPORT dropped" + ); + assert!(transport.is_none() && jump.is_none()); + assert!((0.0..TAU).contains(&orientation)); + assert!(rx.try_recv().is_err(), "one ack per change"); + + // On a deck: the transport bit rides with its tail. + let player = Player { + move_flags: move_flags::ON_TRANSPORT, + ride: Some(super::super::state::PlayerRide { + entity: bevy::ecs::entity::Entity::PLACEHOLDER, + guid: 0x1F, + local_pos: bevy::math::Vec3::ZERO, + boat_yaw: 0.0, + }), + ..Default::default() + }; + ack_speeds_undriven(&tx, &player, &[ack]); + let Ok(ClientCommand::ForceSpeedAck { + flags, transport, .. + }) = rx.try_recv() + else { + panic!("the change is acked"); + }; + assert_eq!(flags, move_flags::ON_TRANSPORT); + assert_eq!(transport.map(|t| t.guid), Some(0x1F)); + } + /// **A knockback launch acks, and sends no `MSG_MOVE_JUMP`** (decision 1702). /// /// Two laws in one frame, and they are the same law read from both ends. `MSG_MOVE_JUMP` has a diff --git a/crates/benilla-app/src/player/wire_in.rs b/crates/benilla-app/src/player/wire_in.rs index efe10d09..e6b2afaf 100644 --- a/crates/benilla-app/src/player/wire_in.rs +++ b/crates/benilla-app/src/player/wire_in.rs @@ -299,23 +299,7 @@ pub(super) fn apply_server_moves( // Same-map teleport (the bridge only emits ours). Snap + echo the ack — without it the server // freezes our movement until relog. for t in teleports.read() { - player.pos = wow_to_bevy(t.position); - player.face_yaw = t.orientation; - cam.yaw = t.orientation; - // Stop any in-progress walk — server now sees us at the new spot. - player.move_flags = 0; - player.airborne_since = None; // a snap ends any in-progress jump arc (no phantom FALL_LAND) - player.settling = true; // hold (gravity off) until the destination's ground/buildings load - player.settle_since = time.elapsed_secs(); - player.settle_deadline = time.elapsed_secs() + SETTLE_TIMEOUT; - // A far same-map teleport relocates us over ground that has not streamed either — the - // tiles we were standing on are still the resident ones. See [`Player::world_stale`]. - player.world_stale = true; - // The relocation voids any in-progress self server-ride (the taxi flight-end teleport - // beats our own spline end by ~latency): `drive_self_ride` takes this flag next frame - // and drops the ride instead of mirroring the stale flight pose back over this snap - // (decision 0501 — the 4-yd hover + full-6s settle at every taxi landing). - player.ride_abort = true; + snap_near_teleport(player, &mut cam.yaw, t, time.elapsed_secs()); // The echo goes now, and it is the WHOLE near-teleport handshake (decision 1340). The // real client echoes on the very next movement tick (wow-re: the 0xC7 drain applies the // snap, then `0x60e0a0` sends guid+counter+time) and sends nothing else — no @@ -523,6 +507,34 @@ pub(super) fn apply_server_moves( speed_acks } +/// **Apply one same-map teleport's snap** (`MSG_MOVE_TELEPORT_ACK` inbound) — everything but the +/// echo, which the caller sends. `cam_yaw` is the fly-cam's yaw, carried to the new facing. +fn snap_near_teleport(player: &mut Player, cam_yaw: &mut f32, t: &TeleportMessage, now: f32) { + // A near teleport always arrives detached: vmangos takes this path only with no `m_transport` + // (`Player::TeleportTo`, after `RemovePassenger`), and the reference's apply (`0x6186b0`) + // re-parents to the packet's transport guid — zero here. A ride left standing let this same + // frame's `ride::carry` compose the old deck pose back over the snap, and the airborne settle + // kept it attached, so a hearth from a zeppelin or a lift landed us back on the deck. + player.ride = None; + player.pos = wow_to_bevy(t.position); + player.face_yaw = t.orientation; + *cam_yaw = t.orientation; + // Stop any in-progress walk — server now sees us at the new spot. + player.move_flags = 0; + player.airborne_since = None; // a snap ends any in-progress jump arc (no phantom FALL_LAND) + player.settling = true; // hold (gravity off) until the destination's ground/buildings load + player.settle_since = now; + player.settle_deadline = now + SETTLE_TIMEOUT; + // A far same-map teleport relocates us over ground that has not streamed either — the tiles we + // were standing on are still the resident ones. See [`Player::world_stale`]. + player.world_stale = true; + // The relocation voids any in-progress self server-ride (the taxi flight-end teleport beats our + // own spline end by ~latency): `drive_self_ride` takes this flag next frame and drops the ride + // instead of mirroring the stale flight pose back over this snap (decision 0501 — the 4-yd + // hover + full-6s settle at every taxi landing). + player.ride_abort = true; +} + /// Merge a server-authored packet's `MOVEMENTFLAGS` into our own — the reference's masked merge /// (`0x618c30 @0x618deb`: `new = old ^ ((old ^ wire) & 0x75a07dff)`), not an assignment. Pure, so /// the omission that actually bites is pinned by test: `ON_TRANSPORT` sits **outside** the mask, so @@ -983,3 +995,38 @@ mod control_tests { assert_eq!(control_verdict(ME, true, None), ControlVerdict::Granted(ME)); } } + +#[cfg(test)] +mod teleport_tests { + use super::*; + + /// **A near teleport lands us where the server says, not back on the deck.** Hearthing off a + /// zeppelin or a lift is a same-map teleport, and vmangos removes the passenger before it + /// takes that path; the snap has to drop the ride too, or the same frame's platform carry + /// recomposes the old deck pose over it. + #[test] + fn a_near_teleport_leaves_the_transport() { + let mut player = Player { + active: true, + ride: Some(super::super::state::PlayerRide { + entity: Entity::PLACEHOLDER, + guid: 0x1F, + local_pos: Vec3::new(1.0, 0.0, 2.0), + boat_yaw: 0.5, + }), + ..Default::default() + }; + let mut cam_yaw = 0.0; + let t = TeleportMessage { + guid: 0x45, + counter: 1, + position: [-8_913.0, -136.0, 81.0], + orientation: 1.25, + }; + snap_near_teleport(&mut player, &mut cam_yaw, &t, 10.0); + assert!(player.ride.is_none(), "the snap deboards"); + assert_eq!(player.pos, wow_to_bevy(t.position)); + assert_eq!((player.face_yaw, cam_yaw), (1.25, 1.25)); + assert!(player.settling && player.ride_abort && player.world_stale); + } +} diff --git a/crates/benilla-app/src/realm_select/input.rs b/crates/benilla-app/src/realm_select/input.rs index b7fafcb9..a7a51c36 100644 --- a/crates/benilla-app/src/realm_select/input.rs +++ b/crates/benilla-app/src/realm_select/input.rs @@ -9,6 +9,7 @@ use bevy::input::mouse::MouseWheel; use bevy::prelude::*; +use crate::bindings::WheelNotches; use crate::net::{RealmChoice, RealmRequest}; use crate::sound::GlueSound; @@ -72,7 +73,7 @@ pub(super) fn clicks( /// `RealmList_OnKeyDown` (ESCAPE / ENTER), plus arrow-key row cycling and the wheel. pub(super) fn keys( keys: Res>, - mut wheel: MessageReader, + (mut wheel, mut wheel_carry): (MessageReader, Local), mut realms: ResMut, choice: Res, mut sounds: MessageWriter, @@ -111,10 +112,7 @@ pub(super) fn keys( // The wheel scrolls the window over the list, in the reference's own 16 px steps translated // back to rows (`RealmListScrollFrame_OnVerticalScroll` divides the bar value by // `REALM_BUTTON_HEIGHT`, so one notch is one row). - let mut notches = 0i32; - for ev in wheel.read() { - notches -= ev.y.signum() as i32; - } + let notches = wheel_rows(&mut wheel_carry, wheel.read()); if notches != 0 { let max = rows.len().saturating_sub(MAX_ROWS); let next_off = (realms.offset as i32 + notches).clamp(0, max as i32) as usize; @@ -122,6 +120,20 @@ pub(super) fn keys( } } +/// The rows this frame's wheel messages scroll the list by — positive is DOWN the list, one row +/// per whole notch ([`WheelNotches`]). Each message is normalised to lines in its own unit first: +/// a trackpad sends a gesture as a trickle of `Pixel` messages, and stepping a row per message +/// ran the list to its end on a gentle swipe. +fn wheel_rows<'a>( + carry: &mut WheelNotches, + wheel: impl IntoIterator, +) -> i32 { + wheel + .into_iter() + .map(|ev| -carry.feed(crate::bindings::wheel_lines(ev.unit, ev.y))) + .sum() +} + /// The realm on a given **screen** row, honouring the scroll offset. fn realm_at(realms: &Realms, row: usize) -> Option { let rows = realms.rows(); @@ -193,3 +205,49 @@ fn try_enter(realms: &mut Realms, choice: &RealmChoice, sounds: &mut MessageWrit realms.hide(); realms.enter(choice, name); } + +#[cfg(test)] +mod tests { + use super::*; + use bevy::input::mouse::MouseScrollUnit; + + fn ev(unit: MouseScrollUnit, y: f32) -> MouseWheel { + MouseWheel { + unit, + x: 0.0, + y, + window: Entity::PLACEHOLDER, + } + } + + /// **A trackpad trickle scrolls the rows it adds up to, not a row per message** — ten + /// `Pixel` messages of a tenth of a line apiece are one notch, so at most one row. + #[test] + fn a_trackpad_trickle_scrolls_only_the_rows_it_adds_up_to() { + let mut carry = WheelNotches::default(); + let step = MouseScrollUnit::SCROLL_UNIT_CONVERSION_FACTOR / 10.0; + let trickle: Vec = (0..10).map(|_| ev(MouseScrollUnit::Pixel, -step)).collect(); + let rows: i32 = trickle + .iter() + .map(|e| wheel_rows(&mut carry, std::iter::once(e))) + .sum(); + assert!( + (0..=1).contains(&rows), + "one line of travel moved {rows} rows" + ); + } + + /// A mouse wheel's notch is one `Line` message and one row, as it always was. + #[test] + fn a_line_notch_scrolls_one_row() { + let mut carry = WheelNotches::default(); + assert_eq!( + wheel_rows(&mut carry, &[ev(MouseScrollUnit::Line, -1.0)]), + 1 + ); + assert_eq!( + wheel_rows(&mut carry, &[ev(MouseScrollUnit::Line, 1.0)]), + -1 + ); + } +} diff --git a/crates/benilla-app/src/spell/cast_send.rs b/crates/benilla-app/src/spell/cast_send.rs index 3e29aa5b..547d97fe 100644 --- a/crates/benilla-app/src/spell/cast_send.rs +++ b/crates/benilla-app/src/spell/cast_send.rs @@ -327,8 +327,9 @@ fn send_spell_cast( ground.clear(); } // The cast classes at this seam. A ranged/auto-repeat shot (Auto Shot, wand Shoot, Throw) is - // not a cast-bar cast — it runs the ranged-stance / `AutoRepeatArmed` path, outside the - // in-flight guard. An on-next-swing spell (`Attributes & 0x404` — Heroic Strike, Cleave) + // not a cast-bar cast — it runs the ranged-stance / `AutoRepeatArmed` path, and its own + // record does not guard (1601), though it is refused like any press while a guarding cast + // holds. An on-next-swing spell (`Attributes & 0x404` — Heroic Strike, Cleave) // queues on the server's melee slot: it arms [`crate::spell::QueuedMeleeSpell`], never the // in-flight guard, so a queued strike cannot block the next cast (the ref's `6e4d97` // exemption on the inflight rec's 0x404 bits — wow-re `wave-cast.md`). @@ -340,11 +341,15 @@ fn send_spell_cast( debug!("ui_action: cast {spell_id} suppressed — already queued on next swing"); return; } - if (normal_cast || on_next_swing) && pending.in_flight(now) { + if pending.in_flight(now) { // The ref's already-casting refusal: the same spell bails silently (`6e4d43`); a // different one errors reason 0x61 "Another action is in progress" (`6e4d97` → - // `HandleCastFailed`) — the inflight rec here is always an ordinary cast, so even an - // on-next-swing press is refused while it holds. + // `HandleCastFailed`). The gate tests the **inflight** rec's `Attributes & 0x404`, never + // the pressed spell's (wow-re `combat-feel-law.md` §B1, VERIFIED byte-exact), and a + // guarding record here is always an ordinary cast or an item use — so EVERY press class + // is refused while it holds: an on-next-swing strike, and a ranged/auto-repeat shot too. + // Exempting the shot let it commit and its non-guarding arm overwrite the running cast's + // record, leaving that cast unguarded once the server refused the shot. if pending.current(now) != Some(spell_id) { cast_errors.push_local(spell_id, 0x61); } @@ -1072,6 +1077,43 @@ mod tests { ); } + /// **A shot pressed mid-cast is refused, and the cast keeps its guard.** The reference's + /// already-casting gate (`6e4d97`, wow-re `combat-feel-law.md` §B1, VERIFIED byte-exact) + /// tests the **inflight** rec's `Attributes & 0x404`, never the pressed spell's — so a wand + /// Shoot / Auto Shot pressed during a Frostbolt meets the same `0x61` as any other press. + /// Ours exempted the ranged class at the gate, so the shot went out, its non-guarding arm + /// overwrote the Frostbolt's record, vmangos refused the shot `SPELL_IN_PROGRESS`, and the + /// still-running Frostbolt was left unguarded for the next mashed key. + #[test] + fn a_shot_pressed_mid_cast_is_refused_and_the_cast_keeps_its_guard() { + const FROSTBOLT: u32 = 116; + let (mut world, rx) = combat_world(false); + world + .resource_mut::() + .arm(FROSTBOLT, Instant::now(), true); + + send_at(&mut world, AUTO_SHOT, MOB); + + assert!(rx.try_recv().is_err(), "nothing goes out on the wire"); + assert_eq!( + world.resource::().0, + vec![CastFail::local(AUTO_SHOT, 0x61)], + "a different spell mid-cast is \"Another action is in progress\", whatever its class" + ); + assert_eq!( + world + .resource::() + .current(Instant::now()), + Some(FROSTBOLT), + "the Frostbolt still holds the guard" + ); + assert_eq!( + world.resource::().0, + None, + "and the refused shot armed no repeat" + ); + } + /// [`world`] plus the catalog and the self player the commit tail needs — `engaged` is our /// server-echoed mirror of the ref's attack lock `[+0xc48]`. fn combat_world(engaged: bool) -> (World, Receiver) { diff --git a/crates/benilla-app/src/ui_auction/net.rs b/crates/benilla-app/src/ui_auction/net.rs index d286d40e..fc484d2d 100644 --- a/crates/benilla-app/src/ui_auction/net.rs +++ b/crates/benilla-app/src/ui_auction/net.rs @@ -101,8 +101,8 @@ fn on_removed_notification(In(ev): In, mut auction: ResMut, mut auction: ResMut) { auction.clear_session(); } diff --git a/crates/benilla-app/src/ui_chat/edit.rs b/crates/benilla-app/src/ui_chat/edit.rs index 8ee1d5af..da4b0f42 100644 --- a/crates/benilla-app/src/ui_chat/edit.rs +++ b/crates/benilla-app/src/ui_chat/edit.rs @@ -176,6 +176,24 @@ pub(crate) struct ChannelState { /// drift from it; the saver refuses to compose a file from `None` for the same reason /// (writing `ZONECHANNELS 0` is the damage 2120 repaired). pub zone_mask: Option, + /// **The custom channels the character is in** — the chat cache header's `CHANNELS` list, the + /// names the next login re-joins. Durable state, for exactly [`Self::zone_mask`]'s reason: the + /// reference writes that list from its live slot array (`0x499b90`-`0x499bb1`: every slot + /// with a number, state 0 and no DBC id) at the chat teardown, when its slots are still + /// there — ours are cleared by the session end *before* the flush (the + /// `Disconnected{LoggedOut}` twin runs in `Update`, a frame ahead of `OnExit(InWorld)`), so a + /// list read off [`Self::joined`] at write time was empty on every logout and no custom + /// channel was ever re-joined (2184 §6). + /// + /// So it moves where the slot array's custom half moves, minus the session end: seated from + /// the file by `restore_chat_looks` (an overwrite, like the mask), grown on a + /// server-confirmed join of a channel with no DBC id ([`Self::note_custom_channel_joined`]), + /// shrunk on an explicit leave ([`Self::note_custom_channel_left`]), and nothing else. + /// + /// **Named divergence:** a channel whose re-join the server refuses (a ban, a changed + /// password) or that the player is kicked from never takes a confirmed slot in the reference + /// and so drops out of its file; here it stays listed until the player `/leave`s it. + pub custom: Vec, } /// One joined-channel record — the reference's `[0xb4fe04] + n*0xa0` slot, in the fields this @@ -275,6 +293,25 @@ impl ChannelState { } } + /// A server-confirmed join of a **custom** channel (no `ChatChannels.dbc` id) enters the + /// durable re-join list ([`Self::custom`]) — the slot the reference's writer would list at + /// teardown. A zone channel is a no-op here; it travels as its mask bit instead. + pub(crate) fn note_custom_channel_joined(&mut self, name: &str) { + if self.channels.zone_channel_id(name) != 0 + || self.custom.iter().any(|c| c.eq_ignore_ascii_case(name)) + { + return; + } + self.custom.push(name.to_string()); + } + + /// An **explicit** leave drops a custom channel from the re-join list — the one thing + /// besides the file that shrinks it ([`Self::custom`]). Case-insensitive, as the server's + /// channel names are. + pub(crate) fn note_custom_channel_left(&mut self, name: &str) { + self.custom.retain(|c| !c.eq_ignore_ascii_case(name)); + } + /// Does the mask carry `id`'s bit — is this `ChatChannels.dbc` row one the walk joins? The /// reference's live predicate `0x49a494` (decision 2144). `None` (not seated) answers false. pub(crate) fn zone_row_wanted(&self, id: u32) -> bool { diff --git a/crates/benilla-app/src/ui_chat/feed.rs b/crates/benilla-app/src/ui_chat/feed.rs index 6543308f..91e428b7 100644 --- a/crates/benilla-app/src/ui_chat/feed.rs +++ b/crates/benilla-app/src/ui_chat/feed.rs @@ -704,6 +704,9 @@ pub(super) fn deliver( // (decision 2120). Outside the slot claim below because it is not about slots — a // re-confirmation of a channel we already number still owns the bit. channels.note_zone_channel_joined(&event.channel); + // …and a custom channel's confirmed join is what enters the durable re-join list the + // chat cache writes (`ChannelState::custom`), the same "server agreed" edge. + channels.note_custom_channel_joined(&event.channel); // …and the slot the walk RENAMED is already numbered, so the claim below skips it — but // its state still has to come back to `Joined`, or the next notice on that row reads as // another rename. A no-op for a channel we hold no slot for, which is the claim's case. diff --git a/crates/benilla-app/src/ui_chat/input/mod.rs b/crates/benilla-app/src/ui_chat/input/mod.rs index 233efded..174f84bc 100644 --- a/crates/benilla-app/src/ui_chat/input/mod.rs +++ b/crates/benilla-app/src/ui_chat/input/mod.rs @@ -989,12 +989,13 @@ pub(super) fn drain_chat_input( // `LeaveChannelByName` (`0x4a0000` → `0x49ee70`): the VM composed a // shortcut or passed a custom name; a number names a confirmed slot here // or the call is a no-op. The mask clear is this path's and no other's - // (decisions 2120, 2144). + // (decisions 2120, 2144), and so is the custom re-join list's. let Some(name) = channels.leave_target(&name) else { continue; }; manual_join_or_leave(&channels, &mut script, &name); channels.note_zone_channel_left(&name); + channels.note_custom_channel_left(&name); ClientCommand::LeaveChannel { name } } C::List { name } => ClientCommand::ChannelList { name }, diff --git a/crates/benilla-app/src/ui_chat/recruitment.rs b/crates/benilla-app/src/ui_chat/recruitment.rs index d934c6d8..05bb0f68 100644 --- a/crates/benilla-app/src/ui_chat/recruitment.rs +++ b/crates/benilla-app/src/ui_chat/recruitment.rs @@ -10,8 +10,9 @@ //! the local player's own `PLAYER_GUILDID` and acts on the wire: //! //! - **guilded** ⇒ leave-by-name `"GuildRecruitment - City"` (`0x49eb17` → `0x49ee70`): -//! `CMSG_LEAVE_CHANNEL`, the channel stripped from every chat window's list, its `ZONECHANNELS` -//! bit cleared, then `UPDATE_CHAT_WINDOWS`. **No already-a-member check.** +//! `CMSG_LEAVE_CHANNEL` and its `ZONECHANNELS` bit cleared, then `UPDATE_CHAT_WINDOWS`. **No +//! already-a-member check.** The window entry the join registered **stays**: the leave's strip +//! key is the full name, which no window carries (wow-re `leavechannelbyname-contract.md` §6/§9). //! - **unguilded, in a capital** (`AreaTable Flags & 0x100`) ⇒ join `"GuildRecruitment"` //! (`0x49eb55` → `0x49eb70`): a slot, the composed name into chat window 1's list, //! `CMSG_JOIN_CHANNEL`, then `UPDATE_CHAT_WINDOWS`. **No already-joined check.** @@ -193,13 +194,7 @@ pub(super) fn guild_recruitment_cascade( match action { Cascade::Leave => { info!("chat: guild recruitment cascade — guilded, leaving {name:?}"); - let _ = commands - .0 - .send(ClientCommand::LeaveChannel { name: name.clone() }); - // `0x49ee70`'s other two halves: the strip from all ten windows, keyed on the - // Shortcut (§6), and the mask bit (`0x49f10a`/`0x49f11a`). - script.strip_chat_window_channel(&row.shortcut); - channels.note_zone_channel_left(&name); + cascade_leave(&mut channels, &commands, name); } Cascade::Join => { info!("chat: guild recruitment cascade — unguilded in a capital, joining {name:?}"); @@ -229,6 +224,22 @@ pub(super) fn guild_recruitment_cascade( state.pending = false; } +/// The cascade's LEAVE arm — leave-by-name `0x49ee70` over the composed name +/// `"GuildRecruitment - City"`: the packet and the mask bit (`0x49f10a`/`0x49f11a`). +/// +/// **No window strip.** `0x49ee70` does strip every window's list, but keyed on the DBC Shortcut +/// only when its *argument* matched a shortcut — else on the argument verbatim (`0x49eff6` / +/// `0x49f017`, wow-re `leavechannelbyname-contract.md` §6, VERIFIED). The full name matches no +/// shortcut, and the join arm registered window 1's entry under the Shortcut +/// `"GuildRecruitment"`, so the scan misses in every window and the entry survives (§9). Stripping +/// by the Shortcut here removed an entry the reference keeps. +fn cascade_leave(channels: &mut ChannelState, commands: &NetCommands, name: String) { + let _ = commands + .0 + .send(ClientCommand::LeaveChannel { name: name.clone() }); + channels.note_zone_channel_left(&name); +} + #[cfg(test)] mod tests { use super::*; @@ -252,6 +263,49 @@ mod tests { assert_eq!(cascade(true, 0, false), Some(Cascade::Deferred)); } + /// **The cascade's own leave cannot strip its window entry** (wow-re + /// `leavechannelbyname-contract.md` §6/§9, VERIFIED). The join arm registered window 1's entry + /// under the Shortcut `"GuildRecruitment"`; the leave's strip key is its argument, the full + /// `"GuildRecruitment - City"`, which matches no DBC shortcut and so is used verbatim — and + /// equals no window entry. The packet goes out and the mask bit clears; the entry stays. + #[test] + fn the_cascades_leave_keeps_the_windows_entry() { + const GUILD_RECRUITMENT: u32 = 25; + let mut script = UiScript::new().expect("a VM"); + assert!(script.register_chat_window_channel(0, "GuildRecruitment", GUILD_RECRUITMENT)); + let mut channels = ChannelState { + channels: benilla_formats::ChatChannelsCatalog::from_rows(vec![ + benilla_formats::ChatChannelRow { + id: GUILD_RECRUITMENT, + flags: 0x32, + pattern: "GuildRecruitment - %s".into(), + shortcut: "GuildRecruitment".into(), + }, + ]), + zone_mask: Some(1 << (GUILD_RECRUITMENT - 1)), + ..Default::default() + }; + channels.claim_slot("GuildRecruitment - City"); + let (tx, rx) = crossbeam_channel::unbounded(); + + cascade_leave( + &mut channels, + &NetCommands(tx), + "GuildRecruitment - City".into(), + ); + + assert!( + matches!(rx.try_recv(), Ok(ClientCommand::LeaveChannel { name }) if name == "GuildRecruitment - City"), + "the CMSG_LEAVE_CHANNEL goes out" + ); + assert_eq!(channels.zone_mask, Some(0), "the mask bit clears"); + assert_eq!( + script.chat_window_looks()[0].channels, + vec![("GuildRecruitment".to_string(), GUILD_RECRUITMENT)], + "window 1 still carries the entry the join registered" + ); + } + /// The watcher: the first sight of the guild id is a change (the player-create trigger), a /// repeat is not, a move is, and the session end forgets — so the next login's first sight is /// a change again. diff --git a/crates/benilla-app/src/ui_chat/settings.rs b/crates/benilla-app/src/ui_chat/settings.rs index b495232f..bc95efb0 100644 --- a/crates/benilla-app/src/ui_chat/settings.rs +++ b/crates/benilla-app/src/ui_chat/settings.rs @@ -191,20 +191,21 @@ impl Default for Parsed { /// Render the file exactly as the writer does (§1), window order. /// -/// `joined` is the client's current channel roster and supplies the header's `CHANNELS` names — -/// **the custom ones only** (`id == 0`): a zone channel is never written as a name, it travels as -/// its bit (wow-re `chat-cache-grammar.md` §1.1, which is why all 34 stock files have an empty -/// per-window `CHANNELS` block beside a non-zero `ZONECHANNELS`). +/// `custom` is [`super::edit::ChannelState::custom`] and supplies the header's `CHANNELS` names — +/// **the custom channels only**: a zone channel is never written as a name, it travels as its bit +/// (wow-re `chat-cache-grammar.md` §1.1, which is why all 34 stock files have an empty per-window +/// `CHANNELS` block beside a non-zero `ZONECHANNELS`). Durable, not the live roster, for the +/// mask's own reason: the session end clears the roster before the flush reads it. /// /// `zone_mask` is [`super::edit::ChannelState::zone_mask`], written raw into the header /// (`0x499c19`) and ANDed with each window's own bits for its block (`0x49a133`/`0x49a138`). It is -/// **passed in rather than derived from `joined`** — decision 2120, and the whole bug: derived, it +/// **passed in rather than derived from the live roster** — decision 2120, and the whole bug: derived, it /// was 0 on any save taken while the roster was empty, and `window AND 0` erased the window's /// channel list for good. fn render( looks: &[ChatWindowLook], colors: &[ChatTypeColor], - joined: &[(String, u32)], + custom: &[String], zone_mask: u32, guild_recruitment_auto: bool, ) -> String { @@ -222,11 +223,9 @@ fn render( out.push_str(&format!( "\nVERSION 2\n\nADDEDVERSION 2\n\nOPTION_GUILD_RECRUITMENT_CHANNEL {recruitment}\n\nCHANNELS\n" )); - for (name, id) in joined { - if *id == 0 { - out.push_str(name); - out.push('\n'); - } + for name in custom { + out.push_str(name); + out.push('\n'); } out.push_str(&format!("END\n\nZONECHANNELS {zone_mask}\n\nCOLORS\n")); for c in colors { @@ -494,15 +493,6 @@ fn shortcut_rows(channels: &super::edit::ChannelState) -> Vec<(u32, String)> { .collect() } -/// The client's channel roster as `(name, zone id)` — what the writer's header and masks are made -/// of. -fn roster(channels: &super::edit::ChannelState) -> Vec<(String, u32)> { - channels - .iter_names() - .map(|name| (name.to_string(), channels.channels.zone_channel_id(name))) - .collect() -} - /// Restore the file into a fresh VM — once per character per VM — and fire the loader's two /// events, file or no file. /// @@ -632,6 +622,9 @@ pub(crate) fn restore_chat_looks(world: &mut World, script: &mut UiScript) { // `Some` is the reference's "chat system ready" flag (`0x499a18`): the walk and the // guild-recruitment cascade both hold until this line has run (decision 2144). channels.zone_mask = Some(mask); + // The custom re-join list is durable on the same terms: seated from the file here, then + // moved only by a confirmed join and an explicit leave (`ChannelState::custom`). + channels.custom = parsed.joined.clone(); } script.set_guild_recruitment_mode(u8::from(parsed.guild_recruitment_auto)); script.set_chat_colors(parsed.colors); @@ -691,7 +684,7 @@ fn write(script: &UiScript, channels: &super::edit::ChannelState, path: &std::pa let body = render( &script.chat_window_looks(), &script.chat_colors(), - &roster(channels), + &channels.custom, zone_mask, script.guild_recruitment_mode() != 0, ); @@ -886,14 +879,9 @@ mod tests { named, ]; let colors = colors(&[("SAY", [1, 2, 3]), ("CHANNEL7", [4, 5, 6])]); - let joined = vec![ - ("General - Elwynn Forest".to_string(), 1), - ("Trade - City".to_string(), 2), - ("MyChan".to_string(), 0), - ]; - // The mask is the character's durable one now (2120), passed in rather than derived — - // here, the two zone channels the roster holds. - let text = render(&looks, &colors, &joined, 0b11, true); + // The mask and the custom list are the character's durable state (2120), passed in rather + // than derived from the live roster. + let text = render(&looks, &colors, &["MyChan".to_string()], 0b11, true); assert!( text.contains("\nCHANNELS\nMyChan\nEND\n\nZONECHANNELS 3\n"), "{text}" @@ -966,13 +954,7 @@ mod tests { w.channels = vec![("General".into(), 1), ("Trade".into(), 2)]; // The mask holds General alone — Trade was explicitly left, so the window's own Trade // bit is ANDed away on the way out. - let text = render( - &[w], - &[], - &[("General - Elwynn Forest".to_string(), 1)], - 1, - true, - ); + let text = render(&[w], &[], &[], 1, true); let windows: Vec<&str> = text.split("WINDOW 1").collect(); assert!(windows[1].contains("ZONECHANNELS 1\n"), "{text}"); } @@ -1074,6 +1056,30 @@ mod tests { ); } + /// The custom re-join list moves on the same two edges as the mask — a confirmed join of a + /// channel with no DBC id, and an explicit leave — and a zone channel never enters it. + #[test] + fn the_custom_list_moves_on_join_and_explicit_leave_only() { + let mut state = super::super::edit::ChannelState { + channels: benilla_formats::ChatChannelsCatalog::from_rows(vec![ + benilla_formats::ChatChannelRow { + id: 1, + flags: 0x11, + pattern: "General - %s".into(), + shortcut: "General".into(), + }, + ]), + ..Default::default() + }; + state.note_custom_channel_joined("General - Elwynn Forest"); + assert!(state.custom.is_empty(), "a zone channel travels as its bit"); + state.note_custom_channel_joined("MyChan"); + state.note_custom_channel_joined("mychan"); + assert_eq!(state.custom, vec!["MyChan".to_string()], "once, by name"); + state.note_custom_channel_left("MYCHAN"); + assert!(state.custom.is_empty(), "an explicit leave drops it"); + } + /// **The teardown write is unconditional; only the debounce reads the dirty flag** (decision /// 2144). A `/leave General` moves host state alone — the mask — and the reference persists /// it because its teardown saver has no dirty flag; ours gated the same write on a Lua-side @@ -1272,34 +1278,7 @@ mod tests { world.init_resource::(); world.init_resource::(); crate::ui_script::setup_script(&mut world); - world.insert_resource(crate::char_select::Roster::with_pending_pick( - vec![benilla_protocol::Character { - guid: 1, - name: "Reloadprobe".into(), - race: 1, // Human → Alliance - class: 1, // Warrior - gender: 0, - level: 60, - skin: 0, - face: 0, - hair_style: 0, - hair_color: 0, - facial_hair: 0, - zone: 0, - map: 0, - position: benilla_protocol::wire::Vector3d { - x: 0.0, - y: 0.0, - z: 0.0, - }, - flags: 0, - equipment: [benilla_protocol::CharEnumItem::default(); 19], - pet_display_id: 0, - pet_level: 0, - pet_family: 0, - }], - 1, - )); + world.insert_resource(probe_roster("Reloadprobe")); crate::ui_script::load_ingame_ui_on_world_entry(&mut world); let path = world @@ -1346,4 +1325,130 @@ mod tests { drop(world); let _ = std::fs::remove_dir_all(&tmp); } + + /// A roster with one character picked — what `load_ingame_ui_on_world_entry` keys the + /// character's chat cache path on. + fn probe_roster(name: &str) -> crate::char_select::Roster { + crate::char_select::Roster::with_pending_pick( + vec![benilla_protocol::Character { + guid: 1, + name: name.into(), + race: 1, // Human → Alliance + class: 1, // Warrior + gender: 0, + level: 60, + skin: 0, + face: 0, + hair_style: 0, + hair_color: 0, + facial_hair: 0, + zone: 0, + map: 0, + position: benilla_protocol::wire::Vector3d { + x: 0.0, + y: 0.0, + z: 0.0, + }, + flags: 0, + equipment: [benilla_protocol::CharEnumItem::default(); 19], + pet_display_id: 0, + pet_level: 0, + pet_family: 0, + }], + 1, + ) + } + + /// **A custom channel survives `/logout`** (2184 §6, the question it left open). The header's + /// `CHANNELS` list is the character's custom channels to re-join at the next login. It was + /// composed from the live roster, and the logout clears that roster first: the + /// `Disconnected{LoggedOut}` that arrives in the same net drain as the logout runs + /// `end_session_channels_on_disconnect` in `Update`, a frame before `OnExit(InWorld)` reaches + /// `end_ui_session`'s flush. So the file was written with an empty list and the channel was + /// never re-joined. The same order is driven here, by the same functions. + #[test] + fn a_custom_channel_survives_a_logout() { + use bevy::ecs::system::RunSystemOnce; + + let _l = crate::local_state::test_env::ENV_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let tmp = std::env::temp_dir().join(format!("benilla-chat-custom-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&tmp); + std::fs::create_dir_all(tmp.join("benilla-config")).expect("hermetic home"); + let _capture = crate::local_state::test_env::EnvGuard::unset("WOW_CAPTURE"); + let _home = crate::local_state::test_env::EnvGuard::set( + "BENILLA_HOME", + tmp.join("benilla-config") + .to_str() + .expect("utf-8 temp path"), + ); + + let roster = probe_roster("Channelprobe"); + let id = crate::ui_macro::identity(&roster).expect("a picked character"); + let path = crate::local_state::chat_character_path(&id.0, &id.1).expect("a cache path"); + std::fs::create_dir_all(path.parent().expect("a parent")).expect("the character dir"); + // A file this writer made (the marker), listing one custom channel in the header. + let file = format!( + "{WRITER_GENERATION}\nVERSION 2\n\nCHANNELS\nMyChan\nEND\n\nZONECHANNELS 3\n\n\ + COLORS\nEND\n\nWINDOW 1\nSIZE 0\n\nMESSAGES\nSAY\nEND\n\nCHANNELS\nEND\n\n\ + ZONECHANNELS 3\n\nEND\n" + ); + std::fs::write(&path, file).expect("seed the file"); + + let mut world = World::new(); + world.init_resource::(); + world.init_resource::(); + world.init_resource::(); + world.init_resource::(); + world.init_resource::(); + world.init_resource::(); + world.init_resource::>(); + world.init_resource::(); + crate::ui_script::setup_script(&mut world); + world.insert_resource(roster); + crate::ui_script::load_ingame_ui_on_world_entry(&mut world); + + // The login's re-join is confirmed: the server's YOU_JOINED, through the real feed arm. + world.resource_scope( + |world, mut channels: Mut| { + let mut script = world.non_send_resource_mut::(); + let mut windows = super::super::frames::ChatWindows::default(); + let mut e = super::super::event::ChatEvent::text_only( + super::super::event::ChatEventKind::ChannelNotice, + String::new(), + ); + e.channel = "MyChan".into(); + e.notice = "2".into(); // YOU_JOINED + super::super::feed::deliver(&mut script, &mut windows, &mut channels, &mut e); + }, + ); + assert_eq!( + world + .resource::() + .number_of("MyChan"), + Some(1), + "precondition: the channel is joined and numbered" + ); + + // `/logout`: the disconnect lands in `Update` first… + world.write_message(crate::net::DisconnectedMessage::new( + "logged out".into(), + benilla_protocol::SessionEnd::LoggedOut, + )); + world + .run_system_once(super::super::channels::end_session_channels_on_disconnect) + .expect("the disconnect twin runs"); + // …and `OnExit(InWorld)` flushes the dying VM's cache a frame later. + crate::ui_script::end_ui_session(&mut world); + + let on_disk = std::fs::read_to_string(&path).expect("the flush wrote the file"); + assert!( + on_disk.contains("\nCHANNELS\nMyChan\nEND\n\nZONECHANNELS"), + "the custom channel is still in the header's re-join list; file was:\n{on_disk}" + ); + + drop(world); + let _ = std::fs::remove_dir_all(&tmp); + } } diff --git a/crates/benilla-app/src/ui_items/drain.rs b/crates/benilla-app/src/ui_items/drain.rs index 4399f2ae..3745ad14 100644 --- a/crates/benilla-app/src/ui_items/drain.rs +++ b/crates/benilla-app/src/ui_items/drain.rs @@ -563,9 +563,10 @@ pub(super) fn drain_container_uses( // the lock setter `0x4953e0` at `0x5edcd9` and only then ships `CMSG_OPEN_ITEM` // (wow-re `inventory-change-failure-display.md` §8, decision 0916). So a clam, // lockbox or loot bag greys the instant you right-click it and stays grey until the - // server answers — the loot landing (a resolving field update) or a refusal - // (`EQUIP_ERR_ITEM_LOCKED` on a still-locked junkbox), both of which - // `PendingItemOps` already clears on. + // open resolves — the emptied item vanishing (a resolving field update), a refusal + // (`EQUIP_ERR_ITEM_LOCKED` on a still-locked junkbox), or the window closed with loot + // left, whose `SMSG_LOOT_RELEASE_RESPONSE` unlocks it by guid (`ui_loot::net`'s + // `loot_release_response`, the reference's `UnlockItem` at `48f299`). // // Deliberately NOT armed on the gift-unwrap arm above, which sends the same opcode: // its emitter `0x5edd60` contains neither call — no lock setter and no latch write. diff --git a/crates/benilla-app/src/ui_loot/net.rs b/crates/benilla-app/src/ui_loot/net.rs index 09a99da7..12ea9104 100644 --- a/crates/benilla-app/src/ui_loot/net.rs +++ b/crates/benilla-app/src/ui_loot/net.rs @@ -10,6 +10,7 @@ use bevy::prelude::*; use super::{LootLatch, LootState}; use crate::net::{ClientCommand, NetCommands, NetHandlerApp, SelfGuid}; +use crate::pending_item_ops::{LockTransitions, PendingItemOps}; use crate::ui_action::{UiError, UiErrorKeys}; /// Register the loot handlers — called from [`super::UiLootPlugin`]. One per kind, plus the @@ -52,9 +53,15 @@ fn on_error( In(ev): In, mut errors: ResMut, mut latch: ResMut, + mut pending: ResMut, + mut lock_cleared: ResMut, ) { if let SessionEvent::LootError { guid, error } = ev { - loot_error(guid, error, &mut errors, &mut latch); + let unlock = ItemUnlock { + pending: &mut pending, + lock_cleared: &mut lock_cleared, + }; + loot_error(guid, error, &mut errors, &mut latch, unlock); } } @@ -80,9 +87,30 @@ fn on_release_response( In(ev): In, mut loot: ResMut, mut latch: ResMut, + mut pending: ResMut, + mut lock_cleared: ResMut, ) { if let SessionEvent::LootReleaseResponse { guid } = ev { - loot_release_response(guid, &mut loot, &mut latch); + let unlock = ItemUnlock { + pending: &mut pending, + lock_cleared: &mut lock_cleared, + }; + loot_release_response(guid, &mut loot, &mut latch, unlock); + } +} + +/// **`UnlockItem 0x495420`** as the loot closes call it — on the loot guid, which unlocks +/// something only when that guid is an item we locked (a lockbox, clam or loot bag, locked at its +/// `CMSG_OPEN_ITEM` send — decision 0916). The unlocked slots queue in [`LockTransitions`] for the +/// container feed to fire `ITEM_LOCK_CHANGED`, like the inventory failure's clear. +struct ItemUnlock<'a> { + pending: &'a mut PendingItemOps, + lock_cleared: &'a mut LockTransitions, +} + +impl ItemUnlock<'_> { + fn unlock(self, guid: u64) { + self.lock_cleared.0.extend(self.pending.clear_by_guid(guid)); } } @@ -306,7 +334,12 @@ fn loot_refusal(reason: u8) -> LootRefusal { /// localization. Before this, benilla composed its own eight sentences here and six of them said /// something the client never says. /// -/// **Two gaps this deliberately does not close** (surfaced by the wow-re §5, named here rather +/// The arms that release run the reference's release tail `0x5ebac2`, which also calls +/// `UnlockItem 0x495420` on the packet's guid (wow-re `loot-anim-leg.md` §7.1): a refused lockbox +/// open drops the item's pending lock. For a corpse or chest guid there is no item, and nothing +/// unlocks. +/// +/// **A gap this deliberately does not close** (surfaced by the wow-re §5, named here rather /// than left to a later bug report): /// /// 1. **A guid-MISMATCHED error takes a different arm in the reference.** The error leg is @@ -315,11 +348,13 @@ fn loot_refusal(reason: u8) -> LootRefusal { /// unconditionally, shows **nothing**, and (because `lootType == 0`) sends nothing. benilla has /// no admission gate on the error shape — it displays the line and keeps the latch. Reachable /// only under the corpse-switch race, and it belongs with the gate (1477), not with the text. -/// 2. **`UnlockItem` fires `ITEM_LOCK_CHANGED` (188)** on the way through the tail, clearing -/// `[item+0x314]` bit 0. That matters only when the loot source is an ITEM guid — a lockbox — -/// where a refused open should drop the item's pending lock; for a corpse or chest guid there -/// is no item to unlock. benilla does not touch [`LockTransitions`] here. -fn loot_error(guid: u64, error: u8, errors: &mut UiErrorKeys, latch: &mut LootLatch) { +fn loot_error( + guid: u64, + error: u8, + errors: &mut UiErrorKeys, + latch: &mut LootLatch, + unlock: ItemUnlock, +) { let LootRefusal { key, releases } = loot_refusal(error); debug!("net: loot error {error} on {guid:#x} → {key} (releases: {releases})"); errors.0.push(UiError::key(key)); @@ -327,6 +362,7 @@ fn loot_error(guid: u64, error: u8, errors: &mut UiErrorKeys, latch: &mut LootLa // The latch armed at the `CMSG_LOOT` send drops (guid-matched — see [`LootLatch`]), or the // character would kneel forever at a corpse whose window never opened (decision 0515). latch.clear_for(guid); + unlock.unlock(guid); } } @@ -355,10 +391,22 @@ fn loot_clear_money(loot: &mut LootState) { /// Idempotent — a client-side close already cleared. The latch clear is **guid-matched**: under /// the corpse-switch race (loot B requested while A was open) the old window's release response /// must not drop the latch the new request just armed (decision 0515). -fn loot_release_response(guid: u64, loot: &mut LootState, latch: &mut LootLatch) { +/// +/// **An item loot unlocks here.** The handler (`0x5ec090`) ends in `0x48f200(cl=0, dl=0)`, whose +/// `48f299` leg calls `UnlockItem 0x495420` when the loot object is an ITEM (wow-re +/// `ui/ledger.tsv` `0x48f200`). It is the only clear an opened lockbox closed with loot left +/// ever gets: vmangos' `DoLootRelease` destroys the item only once it is fully looted, so its +/// slot never changes and [`PendingItemOps::resolve`] has nothing to see. +fn loot_release_response( + guid: u64, + loot: &mut LootState, + latch: &mut LootLatch, + unlock: ItemUnlock, +) { debug!("net: loot released {guid:#x}"); loot.clear(); latch.clear_for(guid); + unlock.unlock(guid); } /// The master-loot candidate list (`SMSG_LOOT_MASTER_LIST`, decision 1675) — who the master looter @@ -416,6 +464,19 @@ fn item_push_result( #[cfg(test)] mod tests { use super::*; + use bevy::ecs::system::RunSystemOnce; + + use crate::pending_item_ops::{LockTransitions, PendingItemOps}; + + /// An [`ItemUnlock`] over an empty lock set — for the tests whose loot source is no item. + macro_rules! no_item { + () => { + ItemUnlock { + pending: &mut PendingItemOps::default(), + lock_cleared: &mut LockTransitions::default(), + } + }; + } /// Both fish-verdict keys queue as **yellow** (type-1 / `UI_INFO_MESSAGE`) entries — the /// byte-verified arm, wow-re `fish-msg-handlers.md` — and both resolve to the exact 1.12 @@ -564,14 +625,14 @@ mod tests { for code in [4u8, 5, 6, 7, 8, 9, 11, 0, 1, 2, 3, 15, 16, 200, 255] { let mut errors = UiErrorKeys::default(); let mut latch = LootLatch(Some(CORPSE)); - loot_error(CORPSE, code, &mut errors, &mut latch); + loot_error(CORPSE, code, &mut errors, &mut latch, no_item!()); assert_eq!(latch.0, None, "code {code} should release"); assert_eq!(errors.0.len(), 1, "code {code} shows exactly one line"); } for code in [10u8, 12, 13, 14] { let mut errors = UiErrorKeys::default(); let mut latch = LootLatch(Some(CORPSE)); - loot_error(CORPSE, code, &mut errors, &mut latch); + loot_error(CORPSE, code, &mut errors, &mut latch, no_item!()); assert_eq!( latch.0, Some(CORPSE), @@ -598,7 +659,7 @@ mod tests { fn the_error_legs_clear_is_guid_matched() { let mut errors = UiErrorKeys::default(); let mut latch = LootLatch(Some(CHEST)); - loot_error(CORPSE, 8, &mut errors, &mut latch); + loot_error(CORPSE, 8, &mut errors, &mut latch, no_item!()); assert_eq!(latch.0, Some(CHEST)); } @@ -637,7 +698,7 @@ mod tests { // The ordinary close path still ends it — the latch is guid-matched, and a chest guid is // no different from a corpse one there. - loot_release_response(CHEST, &mut loot, &mut latch); + loot_release_response(CHEST, &mut loot, &mut latch, no_item!()); assert_eq!(latch.0, None, "the release ends the session"); } @@ -692,6 +753,93 @@ mod tests { ); } + /// A lockbox's item guid (HIGHGUID_ITEM is 0x4000 in the high word). + const LOCKBOX: u64 = 0x4000_0000_0000_0007; + + /// A World with what the release / error handlers write, and the lockbox's open-item lock + /// already armed at bag 0 slot 3 — `ui_items::drain`'s `CMSG_OPEN_ITEM` arm (0916). + fn opened_lockbox_world() -> World { + let mut world = World::new(); + world.init_resource::(); + world.insert_resource(LootLatch(Some(LOCKBOX))); + world.init_resource::(); + world.init_resource::(); + let mut pending = PendingItemOps::default(); + pending.add([(0, 3, LOCKBOX, 1)]); + world.insert_resource(pending); + world + } + + /// **The release unlocks an opened lockbox.** `SMSG_LOOT_RELEASE_RESPONSE` → `0x5ec090` → + /// `0x48f200(cl=0, dl=0)`, whose `48f299` leg calls `UnlockItem 0x495420` when the loot + /// object is an ITEM (wow-re `ui/ledger.tsv` `0x48f200`). Closing a lockbox's window with + /// loot left destroys nothing server-side (vmangos `DoLootRelease` destroys only a fully + /// looted item), so no field update ever resolves the lock — the release is the clear. + #[test] + fn a_release_for_the_opened_item_unlocks_its_slot() { + let mut world = opened_lockbox_world(); + world + .run_system_once_with( + on_release_response, + SessionEvent::LootReleaseResponse { guid: LOCKBOX }, + ) + .expect("the handler runs as a one-shot system"); + assert!( + !world.resource::().contains(0, 3), + "the lockbox is no longer grey and locked" + ); + assert_eq!( + world.resource::().0, + vec![(0, 3)], + "…and the container feed fires ITEM_LOCK_CHANGED for it" + ); + } + + /// The control: a release naming some other object (a corpse closing) leaves the lockbox's + /// lock exactly where it was. + #[test] + fn a_release_for_another_guid_leaves_the_lock() { + let mut world = opened_lockbox_world(); + world + .run_system_once_with( + on_release_response, + SessionEvent::LootReleaseResponse { guid: CORPSE }, + ) + .expect("the handler runs as a one-shot system"); + assert!(world.resource::().contains(0, 3)); + assert!(world.resource::().0.is_empty()); + } + + /// The error leg's release tail `0x5ebac2` calls the same `UnlockItem 0x495420` on the + /// packet's guid (wow-re `loot-anim-leg.md` §7.1) — on the arms that release, and only those. + #[test] + fn a_releasing_loot_error_on_the_opened_item_unlocks_it() { + let mut world = opened_lockbox_world(); + // PLAYER_NOT_FOUND (10) does not reach the tail: nothing unlocks. + world + .run_system_once_with( + on_error, + SessionEvent::LootError { + guid: LOCKBOX, + error: 10, + }, + ) + .expect("the handler runs as a one-shot system"); + assert!(world.resource::().contains(0, 3)); + // TOO_FAR (4) does. + world + .run_system_once_with( + on_error, + SessionEvent::LootError { + guid: LOCKBOX, + error: 4, + }, + ) + .expect("the handler runs as a one-shot system"); + assert!(!world.resource::().contains(0, 3)); + assert_eq!(world.resource::().0, vec![(0, 3)]); + } + const ME: u64 = 0x0000_0000_0000_002A; const THEM: u64 = 0x0000_0000_0000_00FF; diff --git a/crates/benilla-app/src/ui_mail/net.rs b/crates/benilla-app/src/ui_mail/net.rs index ce43e108..6e910358 100644 --- a/crates/benilla-app/src/ui_mail/net.rs +++ b/crates/benilla-app/src/ui_mail/net.rs @@ -82,8 +82,8 @@ fn on_next_mail_time(In(ev): In, mut pending: ResMut) /// An open mailbox dies with the socket, and the arrival countdown is login-scoped (decision /// 0544 P3): a fresh login re-queries `MSG_QUERY_NEXT_MAIL_TIME` at world-enter, so nothing -/// carries over across a reconnect. A listener on the session end, which the drain's dispatch -/// match still owns (a second handler on the kind, after the bridge's own teardown). +/// carries over across a reconnect. A listener on the session end (a second handler on +/// `Disconnected`, after the bridge's own teardown, `net::session::on_disconnected`). fn on_session_end( In(_): In, mut mail: ResMut, diff --git a/crates/benilla-app/src/ui_quest_log.rs b/crates/benilla-app/src/ui_quest_log.rs index 153b1a9d..e0c1d9be 100644 --- a/crates/benilla-app/src/ui_quest_log.rs +++ b/crates/benilla-app/src/ui_quest_log.rs @@ -736,6 +736,11 @@ fn feed_quest_log( let mut entries: Vec = Vec::new(); let mut entry_slots: Vec> = Vec::new(); let mut header_keys: Vec> = Vec::new(); + // The quests folded under a collapsed header: out of the visible list, still in the log. The + // engine's watch prune counts them, as the reference's does (`0x4de7a7`–`0x4de80f` scans the + // whole row array, hidden rows included — wow-re `questlog-list-rebuild.md` §7/§8); leaving + // them out made every collapse drop its quests' watches for good. + let mut hidden_quest_ids: Vec = Vec::new(); for (_, name, row_idxs) in &groups { let collapsed = quest_log.collapsed.contains(name); entries.push(QuestLogEntryView { @@ -756,7 +761,9 @@ fn feed_quest_log( for &ri in row_idxs { let r = &rows[ri]; if collapsed { - continue; // folded: the quest stays in the log, just not in the visible list + // Folded: the quest stays in the log, just not in the visible list. + hidden_quest_ids.push(r.quest_id); + continue; } let (title, level, tag, pushable, objectives, detail) = match quest_log.template(r.quest_id, &commands) { @@ -846,6 +853,7 @@ fn feed_quest_log( let fresh = QuestLogState { entries, num_quests: rows.len() as u32, + hidden_quest_ids, }; if fresh == *last { return; @@ -1725,6 +1733,7 @@ mod tests { QuestLogState { num_quests: entries.iter().filter(|e| !e.is_header).count() as u32, entries, + hidden_quest_ids: Vec::new(), } } diff --git a/crates/benilla-app/src/ui_saved.rs b/crates/benilla-app/src/ui_saved.rs index 76e67133..e0dc6a2d 100644 --- a/crates/benilla-app/src/ui_saved.rs +++ b/crates/benilla-app/src/ui_saved.rs @@ -46,8 +46,9 @@ impl Plugin for UiSavedPlugin { /// /// Called from the in-game UI load ([`crate::ui_script`]) at the reference's own seam — after the /// XML, before anything consumes it. A missing file is the normal first-run case (defaults stand); -/// a malformed one warns and is left on disk untouched, so a hand edit that fails to parse costs -/// this session's settings and not the file. +/// a malformed or unreadable one warns and is **held** ([`UiScript::hold_saved_file`]), so a hand +/// edit that fails to parse costs this session's settings and not the file. Read as **bytes** +/// (1193): the writer keeps a Lua byte string's bytes, so the file need not be UTF-8. /// /// **`host_settings` is the third store's turn** (decision 2132). A handful of the reference's /// `RegisterForSave` globals are settings benilla persists in `config.toml` instead — the @@ -62,19 +63,24 @@ pub(crate) fn load_saved_variables( ) { // `None` = hermetic capture, or no install — session-only state, and the event below still fires. if let Some(path) = crate::local_state::saved_variables_path() { - match std::fs::read_to_string(&path) { - Ok(text) => { - if let Err(e) = script.run(&text) { + match std::fs::read(&path) { + Ok(bytes) => { + if let Err(e) = script.run_chunk(&bytes) { warn!( - "saved variables: {} did not load ({e}) — running on defaults", + "saved variables: {} did not load ({e}) — running on defaults, and \ + leaving the file as it is", path.display() ); + script.hold_saved_file(&path); } else { info!("saved variables: loaded {}", path.display()); } } Err(e) if e.kind() == std::io::ErrorKind::NotFound => {} - Err(e) => warn!("saved variables: cannot read {}: {e}", path.display()), + Err(e) => { + warn!("saved variables: cannot read {}: {e}", path.display()); + script.hold_saved_file(&path); + } } } host_settings(script); @@ -98,11 +104,19 @@ pub(crate) fn save(script: &mut UiScript) { let Some(path) = crate::local_state::saved_variables_path() else { return; }; - let body = script.saved_variables_text(); + if script.saved_file_held(&path) { + warn!( + "saved variables: {} did not load this session — left as it is", + path.display() + ); + return; + } + let mut body = HEADER.as_bytes().to_vec(); + body.extend(script.saved_variables_bytes()); for w in script.take_warnings() { warn!("saved variables: {w}"); } - match crate::local_state::write_atomic(&path, &format!("{HEADER}{body}")) { + match crate::local_state::write_atomic_bytes(&path, &body) { Ok(()) => info!( "saved variables: wrote {} ({} names)", path.display(), @@ -173,6 +187,40 @@ mod tests { load_saved_variables(&mut broken, |_| {}); assert_eq!(broken.eval::("return KEPT").unwrap(), 1); assert_eq!(std::fs::read_to_string(&path).unwrap(), "KEPT = = 3\n"); + // …and the shutdown write leaves it alone too. This is the half that used to fail: the + // load kept its promise, and the save then wrote this session's default over the file. + save(&mut broken); + assert_eq!( + std::fs::read_to_string(&path).unwrap(), + "KEPT = = 3\n", + "a file that did not load is not replaced by the defaults the session ran on" + ); + std::fs::remove_dir_all(&tmp).ok(); + } + + /// **The bytes survive the folder.** A value that is not UTF-8 is written raw and read back + /// raw — the load reads bytes, so the file it now writes is one it can always read (a + /// `read_to_string` here would call it unreadable and the next save would replace it). + #[test] + fn a_byte_string_survives_the_file() { + let _l = ENV_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let tmp = std::env::temp_dir().join(format!("benilla-svbytes-{}", std::process::id())); + std::fs::remove_dir_all(&tmp).ok(); + let _c = EnvGuard::unset("WOW_CAPTURE"); + let _h = EnvGuard::set("BENILLA_HOME", tmp.to_str().unwrap()); + + let mut s = script("string.char(65, 233, 255)"); + save(&mut s); + let mut fresh = script("'x'"); + load_saved_variables(&mut fresh, |_| {}); + assert!(fresh + .eval::( + "return string.len(KEPT) == 3 and string.byte(KEPT, 2) == 233 \ + and string.byte(KEPT, 3) == 255" + ) + .unwrap()); std::fs::remove_dir_all(&tmp).ok(); } diff --git a/crates/benilla-app/src/ui_script/addons.rs b/crates/benilla-app/src/ui_script/addons.rs index ce8ed73f..7c00e3a8 100644 --- a/crates/benilla-app/src/ui_script/addons.rs +++ b/crates/benilla-app/src/ui_script/addons.rs @@ -1051,9 +1051,19 @@ pub(super) fn save_addon_variables(script: &mut UiScript, identity: Option<&(Str continue; } let Some(dir) = dir else { continue }; - let body = script.saved_variables_text_for(names); let path = dir.join(format!("{name}.lua")); - match crate::local_state::write_atomic(&path, &format!("{SAVED_HEADER}{body}")) { + if script.saved_file_held(&path) { + // It failed to load this session: its globals are the addon's defaults, and + // writing them would replace the player's file with them. + warn!( + "ui_script: {} did not load this session — left as it is", + path.display() + ); + continue; + } + let mut body = SAVED_HEADER.as_bytes().to_vec(); + body.extend(script.saved_variables_bytes_for(names)); + match crate::local_state::write_atomic_bytes(&path, &body) { Ok(()) => info!("ui_script: wrote {}", path.display()), Err(e) => warn!("ui_script: cannot write {}: {e}", path.display()), } diff --git a/crates/benilla-app/src/ui_script/extract/mod.rs b/crates/benilla-app/src/ui_script/extract/mod.rs index 72569ad2..441d9b8d 100644 --- a/crates/benilla-app/src/ui_script/extract/mod.rs +++ b/crates/benilla-app/src/ui_script/extract/mod.rs @@ -650,16 +650,10 @@ pub(super) fn tick_script( { let _span = bevy::log::info_span!("ui_script: tick").entered(); let resized = script.set_screen_size(w / s, if h > 0.0 { h / s } else { 768.0 }); - // A resize re-runs the bottom-stack manage pass (decision 1499). Anchors follow the new - // screen rect by themselves; what does not is a seat somebody COMPUTED from the old - // height — the open-bag stack starts a fresh column when the current one would run off - // the top, and that decision is made from `GetScreenHeight()` at layout time. Without - // this, dragging the window smaller leaves the bag columns wrapped for the old height - // until the next bag opens. Existence-guarded: the pass is defined by `UIParent.xml`, - // which is in-game UI, and this system also runs on the glue screens. + // A resize re-runs everything the interface COMPUTED from the old screen size — see + // [`super::manifest::on_screen_resized`]. if resized { - let _ = script - .run("if UIParent_ManageFramePositions then UIParent_ManageFramePositions() end"); + super::manifest::on_screen_resized(&script); } script.tick(time.delta_secs()); } diff --git a/crates/benilla-app/src/ui_script/input.rs b/crates/benilla-app/src/ui_script/input.rs index 0aa463ba..df19e619 100644 --- a/crates/benilla-app/src/ui_script/input.rs +++ b/crates/benilla-app/src/ui_script/input.rs @@ -13,6 +13,7 @@ use bevy::window::PrimaryWindow; use benilla_ui::script::UiScript; use super::{CursorPayloadHeld, PlayerUiClickConsumed, PlayerUiHover, UiKeyboardCapture}; +use crate::bindings::WheelNotches; use crate::textinput::{self, keymap, HostClipboard}; /// The pointer-side state [`feed_ui_input`] reads and writes, as one @@ -58,6 +59,28 @@ impl PointerFeed<'_> { } } +/// The frame's wheel travel into the pane under the cursor's `OnMouseWheel` — **one call per whole +/// notch**, `arg1 = ±1`. +/// +/// The travel is normalised to lines first ([`crate::bindings::wheel_lines`]) and the fraction +/// carried in `notches`: a trackpad reports a gesture as a `Pixel` trickle across many frames, and +/// passing each frame's raw delta fired the handler once per frame — the stock handlers act on +/// the sign alone (`ScrollFrameTemplate_OnMouseWheel` scrolls half a pane per call), so a gentle +/// swipe slammed the pane to its end. A mouse wheel's `Line` notch still fires once, as before. +fn feed_wheel( + script: &mut UiScript, + notches: &mut WheelNotches, + x: f32, + y: f32, + scroll: &AccumulatedMouseScroll, +) { + let whole = notches.feed(crate::bindings::wheel_lines(scroll.unit, scroll.delta.y)); + let step = whole.signum() as f32; + for _ in 0..whole.unsigned_abs() { + script.mouse_wheel(x, y, step); + } +} + /// Feed the window's cursor + buttons + wheel + keyboard into the UI engine (after /// [`super::extract::tick_script`] has resolved this frame's rects), firing /// OnEnter/OnLeave/OnClick/OnMouseWheel and the EditBox @@ -73,7 +96,9 @@ pub(super) fn feed_ui_input( // winit has actually created the surface. window: Query<(&Window, Option<&bevy::window::RawHandleWrapper>), With>, buttons: Res>, - scroll: Res, + // The frame's wheel travel, and the fraction of a notch carried between frames + // ([`feed_wheel`]) — one param for clippy's argument ceiling. + (scroll, mut notches): (Res, ResMut), // One [`PointerFeed`] (clippy's argument ceiling): the hover + click-consumed outputs this // pass writes, the world pick that routes the world-click payload legs (decision 0571), and // the payload-held mirror written for the Send-side world-click consumers. @@ -219,9 +244,7 @@ pub(super) fn feed_ui_input( script.mouse_button(x, y, name, false); } } - if scroll.delta.y != 0.0 { - script.mouse_wheel(x, y, scroll.delta.y); - } + feed_wheel(&mut script, &mut notches, x, y, &scroll); } else if !synthetic { // The OS pointer left the window: leave whatever frame was hovered (once, on the // Some→None transition) and — every frame it stays outside — clear any armed press/drag @@ -417,3 +440,93 @@ pub(super) fn feed_ui_input( // bare-key window toggles — moved into the command registry (`crate::bindings::commands`, // decision 0997): one chord→command table, rebindable, with 0585's modifier law enforced once // in the dispatch instead of per branch here. + +#[cfg(test)] +mod tests { + use super::*; + use bevy::input::mouse::MouseScrollUnit; + + /// A bare VM with one wheel-enabled pane at the screen's centre whose `OnMouseWheel` counts + /// its calls and sums its `arg1`s. + fn wheel_pane() -> UiScript { + let mut s = UiScript::new().unwrap(); + s.set_screen_size(1024.0, 768.0); + s.run( + r#" + local f = CreateFrame("Frame", "WheelProbe") + f:SetWidth(200) f:SetHeight(200) f:SetPoint("CENTER", nil, "CENTER", 0, 0) + f:EnableMouseWheel(true) + f:SetScript("OnMouseWheel", function() + WheelCalls = (WheelCalls or 0) + 1 + WheelSum = (WheelSum or 0) + arg1 + end) + f:Show() + "#, + ) + .unwrap(); + s.resolve(); + s + } + + fn calls(s: &UiScript) -> (i64, f64) { + s.eval::<(Option, Option)>("return WheelCalls, WheelSum") + .map(|(c, v)| (c.unwrap_or(0), v.unwrap_or(0.0))) + .unwrap() + } + + fn frame_of(unit: MouseScrollUnit, dy: f32) -> AccumulatedMouseScroll { + AccumulatedMouseScroll { + unit, + delta: Vec2::new(0.0, dy), + } + } + + /// **A gentle trackpad swipe is not a notch a frame.** Ten frames of a small `Pixel` delta — + /// a tenth of a line apiece at Bevy's factor, one line in total — must fire the pane's + /// `OnMouseWheel` at most the ONE notch they add up to. Fed raw, it fired ten times, and a + /// stock `ScrollFrameTemplate_OnMouseWheel` moves half a pane per call off the sign alone. + #[test] + fn a_trackpad_trickle_fires_only_the_notches_it_adds_up_to() { + let mut s = wheel_pane(); + let mut notches = WheelNotches::default(); + let step = MouseScrollUnit::SCROLL_UNIT_CONVERSION_FACTOR / 10.0; + for _ in 0..10 { + feed_wheel( + &mut s, + &mut notches, + 512.0, + 384.0, + &frame_of(MouseScrollUnit::Pixel, step), + ); + } + let (n, sum) = calls(&s); + assert!( + n <= 1, + "ten frames adding up to one line fired {n} wheel calls (sum {sum})" + ); + assert!(sum <= 1.0, "…and they may move at most one notch: {sum}"); + } + + /// A mouse wheel's notch arrives as one `Line` delta and fires exactly once, `arg1 = 1`. + #[test] + fn a_line_notch_fires_once() { + let mut s = wheel_pane(); + let mut notches = WheelNotches::default(); + feed_wheel( + &mut s, + &mut notches, + 512.0, + 384.0, + &frame_of(MouseScrollUnit::Line, 1.0), + ); + assert_eq!(calls(&s), (1, 1.0)); + feed_wheel( + &mut s, + &mut notches, + 512.0, + 384.0, + &frame_of(MouseScrollUnit::Line, -1.0), + ); + assert_eq!(calls(&s), (2, 0.0), "and the other way is a notch down"); + } +} diff --git a/crates/benilla-app/src/ui_script/lifecycle.rs b/crates/benilla-app/src/ui_script/lifecycle.rs index abbea161..9ae603d1 100644 --- a/crates/benilla-app/src/ui_script/lifecycle.rs +++ b/crates/benilla-app/src/ui_script/lifecycle.rs @@ -13,6 +13,7 @@ use bevy::prelude::*; use benilla_assets::LockRecover; use benilla_ui::script::UiScript; +use super::manifest::silenced_ui_load; use super::{ load_font_registry, load_ingame_ui, CursorPayloadHeld, PlayerUiHover, UiClock, UiKeyboardCapture, @@ -584,43 +585,54 @@ pub(crate) fn load_ingame_ui_on_world_entry(world: &mut World) { // through its whole load edge and only converged a frame later off whatever poll the caller // had written to survive it. seat_raster_seam_for_load(world, &mut script); - let _ = load_ingame_ui(&mut script, identity.as_ref(), &roster, version_check); - // The Minimap widget was born a moment ago with `MinimapState::default()`; seed its two live - // zoom indices from the persisted CVars now, before anything reads them — the reference's own - // minimap reset path copying each CVar object's int into its live index (decision 1131). Once - // only: from here the widget's index is the live truth and `Minimap:SetZoom` writes the CVar - // back. Startup always precedes this state edge (1038), so the knob is already loaded. - let zoom = world.resource::(); - script.set_minimap_zoom(zoom.outdoor, zoom.inside); - // The saved-variables chunk runs HERE — after the XML assigned its file-scope defaults, before - // any consumer reads them — then `VARIABLES_LOADED`. That is the reference's own load order - // (`AddOn_Load 0x51f240` steps 2 → 4 → 6, decision 1128); reversing it means the defaults - // always win and nothing can ever be remembered. - // - // **And the chat cache restores inside it, between `VARIABLES_LOADED` and `PLAYER_LOGIN`** — - // the reference's own slot for the reader's `UPDATE_CHAT_WINDOWS` + `UPDATE_CHAT_COLOR` burst - // (`0x4900d6`, after `0x4900b2` and before `0x490959`; decisions 2119 and 2125). It is the - // sole firer of `UPDATE_CHAT_WINDOWS`, which is the only thing that registers a chat frame for - // any `CHAT_MSG_*` (ref `ChatFrame.lua` l.1261-1273) — as an `Update` system it landed after - // the session's first chat had already been routed, and the login MOTD went to a window - // registered for nothing. - // The plate pair is read out of the world FIRST: the second closure borrows `world` for the - // chat-cache restore, and a `Copy` of two bools costs nothing next to fighting that borrow. - // Absent in a bare test world, where "both off" is also the resource's own default. + // The minimap zoom pair and the plate pair are read out of the world FIRST: the bracket's body + // borrows `world` for the chat-cache restore, and two `Copy` pairs cost nothing next to + // fighting that borrow. The plates are absent in a bare test world, where "both off" is also + // the resource's own default. + let zoom = { + let z = world.resource::(); + (z.outdoor, z.inside) + }; let plates = world .get_resource::() .copied() .unwrap_or_default(); - finish_ui_load_with( - &mut script, - // `NAMEPLATES_ON` / `FRIENDNAMEPLATES_ON` (2132). This seat, and not the `Update` feed - // beside it, is what fixes the bug: `UIParent_OnEvent`'s first `UpdateNameplates()` runs - // inside the `VARIABLES_LOADED` fired at the end of this very call. - |script| crate::vplates::push_plate_globals(script, plates), - |script| { - crate::ui_chat::restore_chat_looks(world, script); - }, - ); + // **The whole load edge is silent** — the TOC walk, every addon, the saved variables and + // `PLAYER_LOGIN`, the span `0x48fbf0` brackets itself across (`0x48fbfa` → `0x49016d`). Both + // login and `ReloadUI` come through here; see [`silenced_ui_load`]. + silenced_ui_load(&mut script, |script| { + let _ = load_ingame_ui(script, identity.as_ref(), &roster, version_check); + // The Minimap widget was born a moment ago with `MinimapState::default()`; seed its two + // live zoom indices from the persisted CVars now, before anything reads them — the + // reference's own minimap reset path copying each CVar object's int into its live index + // (decision 1131). Once only: from here the widget's index is the live truth and + // `Minimap:SetZoom` writes the CVar back. Startup always precedes this state edge (1038), + // so the knob is already loaded. + script.set_minimap_zoom(zoom.0, zoom.1); + // The saved-variables chunk runs HERE — after the XML assigned its file-scope defaults, + // before any consumer reads them — then `VARIABLES_LOADED`. That is the reference's own + // load order (`AddOn_Load 0x51f240` steps 2 → 4 → 6, decision 1128); reversing it means the + // defaults always win and nothing can ever be remembered. + // + // **And the chat cache restores inside it, between `VARIABLES_LOADED` and `PLAYER_LOGIN`** + // — the reference's own slot for the reader's `UPDATE_CHAT_WINDOWS` + `UPDATE_CHAT_COLOR` + // burst (`0x4900d6`, after `0x4900b2` and before `0x490959`; decisions 2119 and 2125). It + // is the sole firer of `UPDATE_CHAT_WINDOWS`, which is the only thing that registers a chat + // frame for any `CHAT_MSG_*` (ref `ChatFrame.lua` l.1261-1273) — as an `Update` system it + // landed after the session's first chat had already been routed, and the login MOTD went to + // a window registered for nothing. + finish_ui_load_with( + script, + // `NAMEPLATES_ON` / `FRIENDNAMEPLATES_ON` (2132). This seat, and not the `Update` + // feed beside it, is what fixes the bug: `UIParent_OnEvent`'s first + // `UpdateNameplates()` runs inside the `VARIABLES_LOADED` fired at the end of this + // very call. + |script| crate::vplates::push_plate_globals(script, plates), + |script| { + crate::ui_chat::restore_chat_looks(world, script); + }, + ); + }); // **Say it out loud when an addon didn't load** (decision 1495). Every failure the walk found // is retained now, but a log nobody knows to open does not fix silence — and silence is the // actual defect B293 reports: *"there are a lot of addons that still doesn't work"*, with @@ -961,7 +973,7 @@ pub(crate) fn end_ui_session(world: &mut World) { // that lands in an engine-side store that survives), before the VM is replaced. The next // VM's registration seeds from what this writes ([`crate::cvars`]'s saved base). crate::cvars::fold_dying_vm_cvars(world); - // The chat cache, on the same terms and for the same reason (decision NNNN): it composes the + // The chat cache, on the same terms and for the same reason (decision 2184): it composes the // player's file out of the DYING VM, and `/reload` never crosses the `OnExit(InWorld)` edge // its flush used to hang on — so a window moved in the last second before a reload was // written nowhere and re-read stale from disk. diff --git a/crates/benilla-app/src/ui_script/manifest.rs b/crates/benilla-app/src/ui_script/manifest.rs index 326eb3fe..428d2bd6 100644 --- a/crates/benilla-app/src/ui_script/manifest.rs +++ b/crates/benilla-app/src/ui_script/manifest.rs @@ -150,6 +150,69 @@ pub(super) fn apply_buff_durations(script: &UiScript) -> Result<(), String> { .map_err(|e| e.to_string()) } +/// **What a screen-size change re-runs** — called from `extract::tick_script`'s resize arm, the +/// frame [`UiScript::set_screen_size`] reports a change on. Anchors follow the new screen rect by +/// themselves; what does not is a size or seat somebody COMPUTED from the old one. +/// +/// The bottom-stack manage pass (decision 1499): the open-bag stack starts a fresh column when +/// the current one would run off the top, and that decision is made from `GetScreenHeight()` at +/// layout time. Without this, dragging the window smaller leaves the bag columns wrapped for the +/// old height until the next bag opens. Existence-guarded: the pass is defined by `UIParent.xml`, +/// which is in-game UI, and the resize arm also runs on the glue screens. +/// +/// **And the three full-screen quads the stock files size once** — a stated repair of a reference +/// gap, in the same posture as [`install_durability_reseat`]: nothing of Blizzard's is edited. +/// `WorldMapFrame_OnLoad` sizes `BlackoutWorld` and `CinematicFrame_OnLoad` sizes +/// `UpperBlackBar`/`LowerBlackBar` from `GetScreenWidth()`/`GetScreenHeight()` at load, and no +/// stock handler touches them again (no `DISPLAY_SIZE_CHANGED` listener) — the reference could +/// afford that because its screen changed only across a restart. 2242 fixed the load edge; this +/// is the other edge: after a window resize or a `uiScale` change the blackout stopped short of +/// the new screen and the world showed through beside the map. [`FULLSCREEN_QUADS_RESEAT`] is the +/// two OnLoads' own arithmetic re-run, so a quad after a resize is exactly what a fresh load at +/// the new size would have made. A stateless chunk rather than an installed hook, so a `ReloadUI` +/// has nothing to re-install and nothing can stack. +pub(super) fn on_screen_resized(script: &UiScript) { + let _ = script.run("if UIParent_ManageFramePositions then UIParent_ManageFramePositions() end"); + if let Err(e) = script.run(FULLSCREEN_QUADS_RESEAT) { + error!("ui_script: full-screen quad re-seat: {e}"); + } +} + +/// `WorldMapFrame.lua` `WorldMapFrame_OnLoad` l.19-28 and `CinematicFrame.lua` +/// `CinematicFrame_OnLoad` l.5-19, verbatim in their arithmetic, each existence-guarded (the +/// glue screens have neither). One addition to the letterbox: the stock OnLoad does nothing below +/// 4:3, which leaves the bars at `CinematicFrame.xml`'s declared `1024 x 128` — so a resize to +/// below 4:3 puts that declared size back, rather than keeping a wide screen's recompute. +const FULLSCREEN_QUADS_RESEAT: &str = r#" +if BlackoutWorld then + local width = GetScreenWidth() + local height = GetScreenHeight() + if ( width / height < 4 / 3 ) then + width = width * 1.25 + height = height * 1.25 + end + BlackoutWorld:SetWidth( width ) + BlackoutWorld:SetHeight( height ) +end +if UpperBlackBar and LowerBlackBar then + local width = GetScreenWidth() + local height = GetScreenHeight() + local barWidth, blackBarHeight = 1024, 128 + if ( width / height > 4 / 3 ) then + local desiredHeight = width / 2 + if ( desiredHeight > height ) then + desiredHeight = height + end + barWidth = width + blackBarHeight = ( height - desiredHeight ) / 2 + end + UpperBlackBar:SetHeight( blackBarHeight ) + UpperBlackBar:SetWidth( barWidth ) + LowerBlackBar:SetHeight( blackBarHeight ) + LowerBlackBar:SetWidth( barWidth ) +end +"#; + /// **A stated divergence from the reference, installed rather than edited in** (1751 window 4). /// /// The reference seats `DurabilityFrame` 20 further in when one of its three side glyphs is up @@ -186,6 +249,35 @@ if DurabilityFrame and not DurabilityFrame.benillaReseat then end "#; +/// **The UI load's sound-suppression bracket — the one site both load paths go through.** +/// +/// `CGGameUI::Initialize 0x48fbf0` brackets ITSELF in the counted suppression scope — +/// `0x48fbfa call 0x458f50` on entry, `0x49016d call 0x458f60` on exit — across the TOC walk, +/// Bindings.xml, the AddOns, the saved variables and the world-enter cascade it calls at +/// `0x490168` (`PLAYER_LOGIN`), so both of its callers (login `0x48f681` and `/reloadui` +/// `0x495669`) load without a sound. The only reader of that depth is `PlaySoundByName +/// 0x458030`, which drops the call outright (wow-re +/// `system/ui/scratch/framexml-load-sound-suppression.md`). +/// +/// This is the mechanism, not a workaround for one noisy handler: stock `TargetFrame_OnHide` +/// really does fire at load (the frame ships shown and its OnLoad hides it) and really does call +/// `PlaySound("INTERFACESOUND_LOSTTARGETUNIT")`. The engine throws it away. Decision 1033 reached +/// the right rule from the director's ear; this is the binary agreeing. +/// +/// One function rather than a push/pop pair at each caller because the production edge +/// ([`super::load_ingame_ui_on_world_entry`]) spent its life unbracketed while the tests' whole- +/// manifest load was — every `/reload` played the lost-target click. Generic over the borrow so +/// the `&UiScript` test loader and the owning production edge share it. +pub(super) fn silenced_ui_load, R>( + script: &mut S, + body: impl FnOnce(&mut S) -> R, +) -> R { + script.borrow().push_sound_suppression(); + let out = body(script); + script.borrow().pop_sound_suppression(); + out +} + /// Load benilla's own default UI — every file [`MANIFEST`] names — through the engine-free loader. /// This is our content (MIT/Apache), committed and **compiled into the binary** /// ([`super::content`], decision 1175); a dev build still prefers the copy on disk, so editing a @@ -219,20 +311,12 @@ pub(crate) fn load_default_ui(script: &UiScript) -> Vec { // have been registered here with real consumers since 1493/1502; the probes simply never had // them, and 218 tests found that out the hour this row went on the manifest. script.register_cvars(crate::cvars::registered_pairs()); - // **Silent, the way the client's own load is.** `0x48fbf0` brackets ITSELF in the counted - // sound-suppression scope — `0x48fbfa call 0x458f50` on entry, `0x49016d call 0x458f60` on - // exit — across the TOC walk, Bindings.xml and the AddOns, so both of its callers (login - // `0x48f681` and `/reloadui` `0x495669`) load without a sound. The only reader of that depth - // is `PlaySoundByName 0x458030`, which drops the call outright. - // - // This is the mechanism, not a workaround for one noisy handler: stock `TargetFrame_OnHide` - // really does fire at load (the frame ships shown and its OnLoad hides it) and really does - // call `PlaySound("INTERFACESOUND_LOSTTARGETUNIT")`. The engine throws it away. Decision 1033 - // reached the right rule from the director's ear; this is the binary agreeing. - script.push_sound_suppression(); - let mut failures = load_manifest(script, &Addon::builtin().toc.files); - failures.extend(bootstrap_positions(script)); - script.pop_sound_suppression(); + // Silent, the way the client's own load is — see [`silenced_ui_load`]. + let mut failures = silenced_ui_load(&mut &*script, |script| { + let mut failures = load_manifest(script, &Addon::builtin().toc.files); + failures.extend(bootstrap_positions(script)); + failures + }); // **A handler that raised DURING the walk is a load failure.** The loader's own report holds // the raises it dispatched itself (a `