From 7c0a99c006749936dc63bc7430b993a3e9f849bd Mon Sep 17 00:00:00 2001 From: grom Date: Fri, 30 Oct 2026 09:42:59 -0600 Subject: [PATCH] =?UTF-8?q?A=20substituted=20swing=20speed=20is=20invisibl?= =?UTF-8?q?e=20to=20everything=20downstream=20=E2=80=94=20say=20it=20once?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit grom reports the `[noslamclip]` macro still never picks Slam, and that the same macro works on the normal client. That rules out my last answer: if the window were the 7.4% the realm's numbers give (2.70 s weapon, 2.5 s Slam), he would land Slam on about one press in fourteen. His log has **0 in 477**. A narrow window is not what that is; a conditional pinned false is. `GetSlamWindowPercent()` returns a hard `0` whenever `st_timerMax <= slamCastTime`, and `st_timerMax` can only ever be `UnitAttackSpeed("player")`. That value is either the streamed `UNIT_FIELD_BASEATTACKTIME` or exactly `VANILLA_SWING_MS`, because `base_swing` substitutes 2000 for absent-or-zero. 2.0 < 2.5, so a substituted swing makes `[noslamclip]` false for every press of the session, silently — and his own swing interval, measured off consecutive `You hit` lines in `WoWCombatLog.txt`, is 2.71 s. So the question is whether that field reaches us, and **nothing downstream can answer it**: `UnitAttackSpeed` returns a number either way, `char_stats`' stand-in warning fires only when no sheet at all was pushed, and 2.00 s is a real weapon speed, so a substituted sheet and a genuine 2.00 s weapon are indistinguishable from Lua, from the log, and from every test — because every test seats a sheet before it asks. That is the same blind spot that let me invent a cast time and call the chain healthy. `warn_once_if_main_swing_substituted` says it at the one place that still knows: the feed, holding the store, before `base_swing` erases the difference. Once per process, naming the field and pointing at the combat log as the cross-check. This is an instrument, not a fix. It makes the next session's log answer the question instead of another round of reasoning from code that reads correctly under both hypotheses. --- crates/benilla-app/src/ui_char.rs | 47 +++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/crates/benilla-app/src/ui_char.rs b/crates/benilla-app/src/ui_char.rs index db90cd34..02b03854 100644 --- a/crates/benilla-app/src/ui_char.rs +++ b/crates/benilla-app/src/ui_char.rs @@ -438,12 +438,58 @@ fn skill_pair(store: &ObjectStore, skill_id: u32) -> (i32, i32) { /// 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. +/// +/// **The substitution is invisible once it is made**, which is what +/// [`warn_once_if_main_swing_substituted`] exists to fix: a sheet carrying 2000 reads exactly like +/// a sheet for a genuine 2.00 s weapon, and the addons that divide by it cannot tell the two +/// apart either. fn base_swing(streamed: Option) -> u32 { streamed .filter(|&ms| ms != 0) .unwrap_or(benilla_ui::script::VANILLA_SWING_MS) } +/// Whether the player's main-hand swing has already been reported as substituted this process. +static MAIN_SWING_SUBSTITUTED: std::sync::atomic::AtomicBool = + std::sync::atomic::AtomicBool::new(false); + +/// Say once, on the host channel, that the player's `UNIT_FIELD_BASEATTACKTIME` was absent or zero +/// and [`base_swing`] substituted the vanilla 2000. +/// +/// This is the one fact no reader downstream can recover. `UnitAttackSpeed("player")` answers a +/// number either way, `char_stats`' own stand-in warning fires only when *no sheet at all* has +/// been pushed, and 2.00 s is a real weapon speed — so a substituted sheet and a genuine 2.00 s +/// weapon are indistinguishable from Lua, from the log, and from every test that seats a sheet +/// before asking. +/// +/// It matters because the value is latched, not polled. SP_SwingTimer caches +/// `UnitAttackSpeed("player")` into `st_timerMax`, and SuperCleveRoidMacros computes +/// `[noslamclip]`'s whole window from that cache as `(st_timerMax - slamCastTime) / st_timerMax`. +/// On this realm Slam is a **2.5 s** cast, so a substituted 2.0 makes `maxSlamStart` negative and +/// `GetSlamWindowPercent()` returns a hard `0` — the conditional is then false for every press of +/// the session, with nothing raised and nothing in the log. A real 2.70 s weapon would have given +/// a 7.4% window instead. Those two outcomes look identical in play until this line exists. +fn warn_once_if_main_swing_substituted(store: &ObjectStore) { + if store + .0 + .unit_base_attack_time(0) + .filter(|&ms| ms != 0) + .is_some() + || MAIN_SWING_SUBSTITUTED.swap(true, std::sync::atomic::Ordering::Relaxed) + { + return; + } + warn!( + "char: the player's UNIT_FIELD_BASEATTACKTIME (field {}) is absent or zero, so the sheet \ + carries the vanilla {} ms stand-in. Every addon that divides by a swing speed is now \ + working from it — SP_SwingTimer latches it into `st_timerMax` and `[noslamclip]` reads \ + a zero-width window off that. Compare it against the real interval between your own \ + `You hit` lines in Logs/WoWCombatLog.txt: if they disagree, this is why.", + benilla_protocol::field::FIELD_UNIT_BASEATTACKTIME, + benilla_ui::script::VANILLA_SWING_MS, + ); +} + pub(crate) fn unit_combat_stats(store: &ObjectStore) -> UnitCombatStats { let f = &store.0; @@ -1178,6 +1224,7 @@ pub(crate) fn feed_char( return; } + warn_once_if_main_swing_substituted(store); let stats = combat_stats(store, objects, &items, &commands); if memo.last_stats.as_ref() != Some(&stats) { gate.audit("feed_char", "the combat-stats snapshot");