From 1b1a90cf13a23f74cb183f2c927843132b1aaee2 Mon Sep 17 00:00:00 2001 From: grom Date: Thu, 29 Oct 2026 14:15:26 -0600 Subject: [PATCH] A swing speed of 0 latched for the session, and [noslamclip] never fired MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit grom's own probe is the record of it: st=0 max=0 pct=nil win=0 idx=1 Four readings of one cause. `UnitAttackSpeed("player")` answered `0` between the VM's birth and the first `feed_char` push, and that window always contains the addon walk — `load_ingame_ui_on_world_entry` mints the VM, seeds the player's roster record and runs every addon's file scope and `ADDON_LOADED` before a Bevy feed system gets a turn. SP_SwingTimer latches `st_timerMax` there behind `if not st_timerMax`, and `0` is truthy in Lua, so the guard never re-fires: the first write to that global in a session is always the pre-push answer. With `0` latched, `CleveRoids.GetSwingPercentElapsed()` answers nil (its own `attackSpeed > 0` gate), `GetSlamWindowPercent()` answers 0, and `ValidateNoSlamClip()` takes its "requires SP_SwingTimer" branch and returns false. So `[noslamclip]` is false and `[slamclip]` is true for the rest of the session, and the macro picks Heroic Strike every time with no indication — which is exactly what he reported. `idx=1` in the same probe says the `stance:1/3` half was never the problem. I had called this latch faithful, on the grounds that `reference/1.12-shapes.tsv` has `UnitAttackSpeed` returning exactly 2 values and never none. That is about the arity and says nothing about a `0`, which is a swing speed no 1.12 client can answer: the player's UNIT_FIELD_BASEATTACKTIME is in the create block the real client has before `UI_Init`, and a unit with no weapon still carries the engine's 2000. `ui_char::base_swing` already wrote the rule down — "0 or absent read as not yet streamed: the vanilla 2000, never zero, as the reference divides damage by it" — and `UnitCombatStats::default()`, which *is* the pre-push answer, broke it. Now both read `VANILLA_SWING_MS` from one place. It was never only this macro: BetterCharacterStats' DPS rows divide by the same number and read `inf` in that window. The bench test that documented the latch as harmless now pins it as the defect, and asserts what the stand-in buys: a drained swing timer measures 100% elapsed rather than nil, `[noslamclip]` is false out of combat by the window math rather than by the addon's own bail, and nothing tells him in chat to install the addon he already has. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn --- .../src/addon_harness/tick_bench_tests.rs | 59 +++++++++++++++---- crates/benilla-app/src/ui_char.rs | 8 ++- crates/benilla-ui/src/script/char_stats.rs | 38 ++++++++++-- crates/benilla-ui/src/script/mod.rs | 1 + 4 files changed, 88 insertions(+), 18 deletions(-) diff --git a/crates/benilla-app/src/addon_harness/tick_bench_tests.rs b/crates/benilla-app/src/addon_harness/tick_bench_tests.rs index 648f4bee..c6e170d7 100644 --- a/crates/benilla-app/src/addon_harness/tick_bench_tests.rs +++ b/crates/benilla-app/src/addon_harness/tick_bench_tests.rs @@ -1545,16 +1545,32 @@ fn a_cursor_seated_sign_plate_prices_its_own_re_anchor() { /// instead of 44% and would read in play as "Slam never fires". /// * The `` delta reaching `SP_ST_OnUpdate(arg1)` as seconds. /// -/// **All five answer correctly**, which is the finding: measured here the conditional is `true` -/// at the top of a swing, with a 44.4% window off a 1.5 s scanned cast time. So whatever he is -/// seeing is not one of these, and this test exists so that stays true. +/// **All five answer correctly** once the sheet is pushed, with a 44.4% window off a 1.5 s +/// scanned cast time — and the asymmetry this test originally only *documented* turned out to be +/// the whole defect. It is the first bullet's edge: before the stats feed has pushed, +/// `UnitAttackSpeed("player")` answered `0`, and SP_SwingTimer's `ADDON_LOADED` arm latches that +/// into `st_timerMax` behind `if not st_timerMax` — where `0` is truthy, so the guard never +/// re-fires. The first write to that global in a session is therefore always the pre-push answer. /// -/// The one real asymmetry it documents is the first bullet's edge: before any swing has landed, -/// `st_timerMax` is latched to `0` by SP_SwingTimer's `ADDON_LOADED` arm, because -/// `UnitAttackSpeed("player")` answers `0` while the stats feed has not pushed yet — and `0` is -/// truthy in Lua, so its own `if not st_timerMax` guard never re-fires. That is *faithful* -/// (`reference/1.12-shapes.tsv` has `UnitAttackSpeed` returning exactly 2 values, never none), and -/// it heals on the first swing, as the second half of this test shows. +/// With `0` latched, `CleveRoids.GetSwingPercentElapsed()` answers `nil` (its own `attackSpeed > 0` +/// gate), `GetSlamWindowPercent()` answers 0, and `ValidateNoSlamClip()` takes its "requires +/// SP_SwingTimer" branch and returns false — so `[noslamclip]` is false and `[slamclip]` is true +/// for the session, which is exactly the "it still uses Heroic Strike, with no indication" grom +/// reported. His own `/script` probe is the record: +/// +/// ```text +/// st=0 max=0 pct=nil win=0 idx=1 +/// ``` +/// +/// Four readings of one cause: `max` is the latch, `pct` is the gate it fails, `win` is the +/// divide it poisons, and `idx` (Battle Stance) proves the `stance:1/3` half of the macro was +/// never the problem. I had called the latch faithful on the grounds that +/// `reference/1.12-shapes.tsv` has `UnitAttackSpeed` returning exactly 2 values and never none — +/// that is about the *arity*, and says nothing about a `0`, which is a swing speed no 1.12 client +/// can answer (the player's `UNIT_FIELD_BASEATTACKTIME` is in the create block the real client +/// has before `UI_Init`, and a unit with no weapon still carries the engine's 2000). +/// [`benilla_ui::script::VANILLA_SWING_MS`] is now that stand-in in +/// `UnitCombatStats::default()`, which is what the pre-push window reads. #[test] fn the_slam_clip_conditional_answers_on_the_players_own_interface() { let _data = benilla_formats::wow_data_or_skip!(); @@ -1640,11 +1656,30 @@ fn the_slam_clip_conditional_answers_on_the_players_own_interface() { fallback leaves a 7% window against a 2.70 s swing and reads as `[noslamclip]` never \ being true" ); - // The latch, before any swing: documented, faithful, and healed below. + // The latch, before any swing. This loader runs the addon walk before the `set_*` calls + // above, exactly as `load_ingame_ui_on_world_entry` runs it before any feed system — so this + // assertion is the live pre-push window, and `"0"` here is the bug he played. assert_eq!( ask(&mut s, "st_timerMax"), - "0", - "SP_SwingTimer's ADDON_LOADED arm latched the pre-feed `UnitAttackSpeed` answer" + "2", + "SP_SwingTimer's ADDON_LOADED arm latches whatever `UnitAttackSpeed` answers before the feed pushes, once and for the session — so that answer has to be a swing speed" + ); + // ...and the conditional still answers, which is the point of the stand-in: a swing that is + // not running reads as fully elapsed, so `[noslamclip]` is false out of combat, as it is in + // the reference. What it must not do is answer `nil` and print "requires SP_SwingTimer". + assert_eq!( + ask(&mut s, "CleveRoids.GetSwingPercentElapsed()"), + "100", + "a drained swing timer is 100% elapsed, not an unanswerable `nil`" + ); + assert_eq!( + ask(&mut s, "CleveRoids.ValidateNoSlamClip()"), + "false", + "with no swing in flight the window is shut — false, by the window math rather than by the addon's own \"this conditional needs a swing timer\" bail" + ); + assert!( + !ask(&mut s, "CleveRoids._slamClipErrorShown").starts_with('1'), + "`ValidateNoSlamClip` took its no-swing-timer branch and told him in chat to go install the addon he already has" ); // One auto-attack lands, exactly as `ui_chat::frames::route` fires it. diff --git a/crates/benilla-app/src/ui_char.rs b/crates/benilla-app/src/ui_char.rs index 5d3c076f..bfbd1d96 100644 --- a/crates/benilla-app/src/ui_char.rs +++ b/crates/benilla-app/src/ui_char.rs @@ -435,9 +435,13 @@ fn skill_pair(store: &ObjectStore, skill_id: u32) -> (i32, i32) { } /// A swing time in ms, with 0 or absent read as not yet streamed: the vanilla 2000, never zero, -/// as the reference divides damage by it. +/// as the reference divides damage by it. The same stand-in +/// [`UnitCombatStats::default`](benilla_ui::script::UnitCombatStats::default) uses before this +/// feed has pushed at all, so the two answers agree across the push. fn base_swing(streamed: Option) -> u32 { - streamed.filter(|&ms| ms != 0).unwrap_or(2000) + streamed + .filter(|&ms| ms != 0) + .unwrap_or(benilla_ui::script::VANILLA_SWING_MS) } pub(crate) fn unit_combat_stats(store: &ObjectStore) -> UnitCombatStats { diff --git a/crates/benilla-ui/src/script/char_stats.rs b/crates/benilla-ui/src/script/char_stats.rs index b9d238ba..1cc2b174 100644 --- a/crates/benilla-ui/src/script/char_stats.rs +++ b/crates/benilla-ui/src/script/char_stats.rs @@ -124,8 +124,38 @@ pub struct UnitCombatStats { pub block_percent: f32, } +/// The engine's own base attack time, in ms: what a unit with no weapon swings at, and the +/// stand-in for a field that has not streamed. `UnitAttackSpeed` never answers 0 in the +/// reference, and nothing that divides by a swing time survives one. +pub const VANILLA_SWING_MS: u32 = 2000; + impl Default for UnitCombatStats { - /// All zeros but `damage_percent` 1.0, which stock divides by (`PaperDollFrame.lua:298`). + /// All zeros but `damage_percent` 1.0, which stock divides by (`PaperDollFrame.lua:298`), and + /// the three attack times, which are the vanilla 2000 and never zero. + /// + /// This default **is** the answer every stats verb gives between a VM's birth and the first + /// `feed_char` push (`with_unit_stats` falls back to it), and that window always contains the + /// addon walk: `load_ingame_ui_on_world_entry` mints the VM, seeds the player's roster record + /// and runs every addon's file scope and `ADDON_LOADED` before any Bevy feed system gets a + /// turn — and on a fresh login the player's descriptor has not even streamed yet, which is + /// why the record is seeded from the roster at all. A swing time of 0 in that window is not + /// what the reference can answer: the player's `UNIT_FIELD_BASEATTACKTIME` is in the create + /// block the real client has before `UI_Init`, and a unit with no weapon still carries the + /// engine's 2000. + /// + /// A 0 is read by addons as a real number, not as "not yet": + /// + /// * SP_SwingTimer latches `st_timerMax = UnitAttackSpeed("player")` in its `ADDON_LOADED` + /// arm behind `if not st_timerMax`, and `0` is truthy in Lua, so the guard never re-fires. + /// Everything downstream divides by it: `CleveRoids.GetSwingPercentElapsed` answers `nil` + /// (its own `attackSpeed > 0` gate), `GetSlamWindowPercent` answers 0, and every + /// `[noslamclip]`/`[swingtimer]` macro silently takes its other branch for the whole + /// session unless an auto-attack lands and re-assigns it. + /// * BetterCharacterStats' `damage / speed` DPS rows (`BetterCharacterStats.lua:599-609`) + /// read `inf`. + /// + /// 2000 is the same stand-in `ui_char::base_swing` already puts in front of an unstreamed + /// field for exactly this reason, so the pre-push answer and the post-push answer agree. fn default() -> Self { UnitCombatStats { stats: [0; 5], @@ -141,8 +171,8 @@ impl Default for UnitCombatStats { damage_bonus_pos: [0; 7], damage_bonus_neg: [0; 7], damage_percent: 1.0, - main_attack_time_ms: 0, - offhand_attack_time_ms: 0, + main_attack_time_ms: VANILLA_SWING_MS, + offhand_attack_time_ms: VANILLA_SWING_MS, has_offhand: false, attack_power: 0, attack_power_pos: 0, @@ -150,7 +180,7 @@ impl Default for UnitCombatStats { ranged_attack_power: 0, ranged_attack_power_pos: 0, ranged_attack_power_neg: 0, - ranged_attack_time_ms: 0, + ranged_attack_time_ms: VANILLA_SWING_MS, ranged_min_damage: 0.0, ranged_max_damage: 0.0, main_weapon_skill: (0, 0), diff --git a/crates/benilla-ui/src/script/mod.rs b/crates/benilla-ui/src/script/mod.rs index e18684d1..0fdc0dc5 100644 --- a/crates/benilla-ui/src/script/mod.rs +++ b/crates/benilla-ui/src/script/mod.rs @@ -186,6 +186,7 @@ pub use channel::{ChannelCommand, ZoneChannelRow}; pub use char_stats::{ weapon_subclass_skill, BankBagSlots, InvSlotView, InventorySlots, ItemRawFields, UnitCombatStats, BANK_BAG_SLOT_COUNT, INVENTORY_SLOT_COUNT, SKILL_DEFENSE, SKILL_UNARMED, + VANILLA_SWING_MS, }; pub use chat_bubble::BubbleState; pub use chat_misc::EmoteRequest;