From a7a54c6d02f99ed17836f04a9098967117a5acf4 Mon Sep 17 00:00:00 2001 From: grom Date: Thu, 5 Nov 2026 10:38:02 -0700 Subject: [PATCH] Dismount and CancelShapeshiftForm, because our own pfUI port calls both MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `pfUI/modules/autoshift.lua:45` answers `ERR_CANT_INTERACT_SHAPESHIFTED` with if IsMounted() then Dismount() elseif GetShapeshiftFormID() ~= 0 then CancelShapeshiftForm() end and grom's 2026-11-05 log carries `attempt to call global 'Dismount' (a nil value)` from exactly there. That is a port written against verbs this client does not have, with a comment claiming both "do their own engine-side aura scan and send the cancel packet directly" — which was the design and not the state. Both were on `compat/classic_api/state.rs`'s not-built list for a reason that turned out to be answerable. Each needs the SPELL ID behind a state the engine reports as a flag (`UNIT_FIELD_MOUNTDISPLAYID`) or an index (`SelfMovement::shapeshift_form`), because `CMSG_CANCEL_AURA` carries an id and nothing else; and the player's aura list carries spell ids and nothing about what each aura IS. The missing half was one derivation — `EffectApplyAuraName` 78 (`SPELL_AURA_MOUNTED`) and 36 (`SPELL_AURA_MOD_SHAPESHIFT`), now `SpellDisplay::mount_aura`/`shapeshift_aura` and two fields on `AuraState` beside `channeled`, which the feed derives the same way from the same row it already reads. Each verb is then the aura it names cancelled by id, which is what the reference's own right-click on the buff does. Neither is gated on `aura::cancel_authorized`, unlike `CancelPlayerBuff`: that gate is the reference's check inside THAT verb, this is not that verb, the server makes its own decision about a cancel it is sent, and a mount whose aura is not flagged `AFLAG_CANCELABLE` would otherwise be undismountable with no way to tell why. Allowlisted in `reference_surface` with the reason, as the four taint globals were: 1.12 has no verb for this because 1.12's player has the buff icon under the cursor, and an addon reacting to an error message has neither an icon nor a cursor. Verified: `dismount_and_cancel_form_each_cancel_their_own_aura_by_id` seats a plain buff, a form and a mount in one list and asserts each verb finds its own id past the others, and that neither fires with nothing to cancel. benilla-ui 2128/0, benilla-app at its recorded baseline. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn --- crates/benilla-app/src/ui_aura.rs | 4 + .../benilla-app/src/ui_script/buff_tests.rs | 2 + .../src/ui_script/pet_frame_tests.rs | 2 + .../benilla-app/src/ui_script/quiver_tests.rs | 2 + .../src/ui_script/target_aura_tests.rs | 4 + .../src/ui_script/tooltip_anchor_tests.rs | 2 + .../src/ui_script/tot_frame_tests.rs | 2 + crates/benilla-formats/src/spells/display.rs | 14 +++ crates/benilla-ui/src/script/aura/mod.rs | 11 ++ .../benilla-ui/src/script/aura/player_buff.rs | 2 + .../src/script/compat/classic_api/spell.rs | 2 + .../src/script/compat/classic_api/state.rs | 113 +++++++++++++++++- .../script/compat/classic_api/unit_auras.rs | 2 + .../src/script/tests/reference_surface.rs | 10 ++ 14 files changed, 168 insertions(+), 4 deletions(-) diff --git a/crates/benilla-app/src/ui_aura.rs b/crates/benilla-app/src/ui_aura.rs index 05e79bab..57c8763c 100644 --- a/crates/benilla-app/src/ui_aura.rs +++ b/crates/benilla-app/src/ui_aura.rs @@ -581,6 +581,8 @@ fn other_unit_auras( cancelable: a.flags & AURA_FLAG_CANCELABLE != 0, // `untilCancelled` is the player cache record's alone (`0xbc6040` `+0xc`). until_cancelled: false, + mount: display.is_some_and(benilla_formats::SpellDisplay::mount_aura), + shapeshift: display.is_some_and(benilla_formats::SpellDisplay::shapeshift_aura), channeled: display .is_some_and(|d| d.attributes_ex & SPELL_ATTR_EX_IS_CHANNELED != 0), // The raw wire slot, for nampower's `GetUnitField`; every other reader indexes the @@ -760,6 +762,8 @@ fn feed_auras( c.flags & AURA_FLAG_CANCELABLE != 0, expiration_time, ), + mount: display.is_some_and(benilla_formats::SpellDisplay::mount_aura), + shapeshift: display.is_some_and(benilla_formats::SpellDisplay::shapeshift_aura), channeled: display .is_some_and(|d| d.attributes_ex & SPELL_ATTR_EX_IS_CHANNELED != 0), // The player cache keeps the wire slot it was built from, so `GetUnitField` diff --git a/crates/benilla-app/src/ui_script/buff_tests.rs b/crates/benilla-app/src/ui_script/buff_tests.rs index 1b5f3a6b..440f7f1d 100644 --- a/crates/benilla-app/src/ui_script/buff_tests.rs +++ b/crates/benilla-app/src/ui_script/buff_tests.rs @@ -65,6 +65,8 @@ fn aura( // The feed takes this from the spell (`ui_aura::until_cancelled`); here, from the expiry. until_cancelled: expiration_time == 0.0, channeled: false, + mount: false, + shapeshift: false, } } diff --git a/crates/benilla-app/src/ui_script/pet_frame_tests.rs b/crates/benilla-app/src/ui_script/pet_frame_tests.rs index 675c424b..95943ff7 100644 --- a/crates/benilla-app/src/ui_script/pet_frame_tests.rs +++ b/crates/benilla-app/src/ui_script/pet_frame_tests.rs @@ -112,6 +112,8 @@ fn pet_buff(spell_id: u32, name: &str, count: u8) -> AuraState { cancelable: false, until_cancelled: false, channeled: false, + mount: false, + shapeshift: false, } } diff --git a/crates/benilla-app/src/ui_script/quiver_tests.rs b/crates/benilla-app/src/ui_script/quiver_tests.rs index ed26cb61..eebbfbfa 100644 --- a/crates/benilla-app/src/ui_script/quiver_tests.rs +++ b/crates/benilla-app/src/ui_script/quiver_tests.rs @@ -338,6 +338,8 @@ fn seat_a_buff(s: &mut UiScript, spell_id: u32, name: &str, icon: &str) { until_cancelled: true, slot: 0, channeled: false, + mount: false, + shapeshift: false, }]), ); s.fire_event("PLAYER_AURAS_CHANGED", vec![]); diff --git a/crates/benilla-app/src/ui_script/target_aura_tests.rs b/crates/benilla-app/src/ui_script/target_aura_tests.rs index 9e9b7f0e..a8880ecf 100644 --- a/crates/benilla-app/src/ui_script/target_aura_tests.rs +++ b/crates/benilla-app/src/ui_script/target_aura_tests.rs @@ -76,6 +76,8 @@ fn debuff(spell_id: u32, name: &str, count: u8, debuff_type: Option<&str>) -> Au // Only the player's auras carry `untilCancelled`, read by `GetPlayerBuff` alone. until_cancelled: false, channeled: false, + mount: false, + shapeshift: false, } } @@ -93,6 +95,8 @@ fn buff(spell_id: u32, name: &str) -> AuraState { cancelable: false, until_cancelled: false, channeled: false, + mount: false, + shapeshift: false, } } diff --git a/crates/benilla-app/src/ui_script/tooltip_anchor_tests.rs b/crates/benilla-app/src/ui_script/tooltip_anchor_tests.rs index 2286e3e0..3245e733 100644 --- a/crates/benilla-app/src/ui_script/tooltip_anchor_tests.rs +++ b/crates/benilla-app/src/ui_script/tooltip_anchor_tests.rs @@ -455,6 +455,8 @@ fn buff_hover_hangs_below_left_of_the_button() { until_cancelled: false, slot: 0, channeled: false, + mount: false, + shapeshift: false, }]), ); // The event the stock buff buttons register (`BuffFrame.lua:113`). diff --git a/crates/benilla-app/src/ui_script/tot_frame_tests.rs b/crates/benilla-app/src/ui_script/tot_frame_tests.rs index a31a2943..6ac4691a 100644 --- a/crates/benilla-app/src/ui_script/tot_frame_tests.rs +++ b/crates/benilla-app/src/ui_script/tot_frame_tests.rs @@ -107,6 +107,8 @@ fn debuff(spell_id: u32, name: &str, debuff_type: Option<&str>) -> AuraState { cancelable: false, until_cancelled: false, channeled: false, + mount: false, + shapeshift: false, } } diff --git a/crates/benilla-formats/src/spells/display.rs b/crates/benilla-formats/src/spells/display.rs index 4b597c7d..28b4c057 100644 --- a/crates/benilla-formats/src/spells/display.rs +++ b/crates/benilla-formats/src/spells/display.rs @@ -512,6 +512,20 @@ impl SpellDisplay { .any(|a| TRACKING_AURA_TYPES.contains(a)) } + /// Does this spell apply `SPELL_AURA_MOUNTED` (`EffectApplyAuraName` 78)? The player's aura + /// list carries spell ids and nothing else about what each aura *is*, so this is how + /// `Dismount()` finds the one aura to cancel: `CMSG_CANCEL_AURA` is by spell id, and + /// `UNIT_FIELD_MOUNTDISPLAYID` says only that a mount exists. + pub fn mount_aura(&self) -> bool { + self.effect_apply_aura.contains(&78) + } + + /// Does this spell apply `SPELL_AURA_MOD_SHAPESHIFT` (`EffectApplyAuraName` 36)? + /// `CancelShapeshiftForm()`'s half of the same question. + pub fn shapeshift_aura(&self) -> bool { + self.effect_apply_aura.contains(&36) + } + /// `AttributesEx2` bit 19: `Stances` lists forms the spell may also be cast in, not forms it /// needs. The form gate `0x612480` and the tooltip's form line (`0x52f115`) both test it; no /// spell that truly needs a form (stances, druid forms, stealth openers) carries it. diff --git a/crates/benilla-ui/src/script/aura/mod.rs b/crates/benilla-ui/src/script/aura/mod.rs index ef7d6b9f..8ef5a931 100644 --- a/crates/benilla-ui/src/script/aura/mod.rs +++ b/crates/benilla-ui/src/script/aura/mod.rs @@ -47,6 +47,15 @@ pub struct AuraState { /// from the spell (`0x4e452e`-`0x4e45c5`), not from `expiration_time`, so it holds before any /// `SMSG_UPDATE_AURA_DURATION` and a permanent aura never shows a `0 s` timer. pub until_cancelled: bool, + /// Does this aura's spell apply `SPELL_AURA_MOUNTED` (78)? The one thing that identifies + /// *which* of the player's auras is the mount, which `Dismount()` has to cancel by spell id — + /// `CMSG_CANCEL_AURA` carries the id, and `UNIT_FIELD_MOUNTDISPLAYID` says only that there is + /// one. Derived from `effect_apply_aura`, as `channeled` is derived from `AttributesEx`. + pub mount: bool, + /// Does this aura's spell apply `SPELL_AURA_MOD_SHAPESHIFT` (36)? `CancelShapeshiftForm()`'s + /// half of the same question: the engine knows the form *index* + /// ([`crate::script::SelfMovement::shapeshift_form`]) and not the spell behind it. + pub shapeshift: bool, /// `AttributesEx & 0x4` (`SPELL_ATTR_EX_IS_CHANNELED`), [`cancel_authorized`]'s second arm. pub channeled: bool, /// The **raw wire slot**, `UNIT_FIELD_AURA`'s own index, 0-based: 0..31 helpful, 32..47 @@ -361,6 +370,8 @@ mod tests { cancelable, until_cancelled: false, channeled: false, + mount: false, + shapeshift: false, } } diff --git a/crates/benilla-ui/src/script/aura/player_buff.rs b/crates/benilla-ui/src/script/aura/player_buff.rs index 4f180eb2..66225020 100644 --- a/crates/benilla-ui/src/script/aura/player_buff.rs +++ b/crates/benilla-ui/src/script/aura/player_buff.rs @@ -220,6 +220,8 @@ mod tests { cancelable, until_cancelled: false, channeled: false, + mount: false, + shapeshift: false, } } diff --git a/crates/benilla-ui/src/script/compat/classic_api/spell.rs b/crates/benilla-ui/src/script/compat/classic_api/spell.rs index 2638190d..9e183231 100644 --- a/crates/benilla-ui/src/script/compat/classic_api/spell.rs +++ b/crates/benilla-ui/src/script/compat/classic_api/spell.rs @@ -775,6 +775,8 @@ mod tests { cancelable, until_cancelled: false, channeled: false, + mount: false, + shapeshift: false, }; s.set_auras( "player", diff --git a/crates/benilla-ui/src/script/compat/classic_api/state.rs b/crates/benilla-ui/src/script/compat/classic_api/state.rs index fb330ace..f7616c42 100644 --- a/crates/benilla-ui/src/script/compat/classic_api/state.rs +++ b/crates/benilla-ui/src/script/compat/classic_api/state.rs @@ -12,10 +12,21 @@ //! - `GetMirrorTimerInfo`, `GetMirrorTimerProgress`: benilla's mirror timers (breath, fatigue) are //! event-driven only — `benilla-app`'s `ui_mirror.rs` fires `MIRROR_TIMER_START`/`_PAUSE`/`_STOP` //! and keeps no queryable snapshot, which these two getters would need. -//! - `Dismount`, `CancelShapeshiftForm`: actions, not getters. Vanilla cancels a form by re-casting -//! its own spell, so an honest `CancelShapeshiftForm` needs the active form's spell id (the -//! engine knows the form *index*, [`SelfMovement::shapeshift_form`], not the spell behind it); -//! `Dismount` has no stock verb here to reuse at all. +//! +//! `Dismount` and `CancelShapeshiftForm` are the two **actions** here, and they were on the list +//! above until 2026-11-05 for a reason that turned out to be answerable: both needed the spell id +//! behind a state the engine reports as a flag or an index, and the player's aura list carries +//! spell ids and nothing about what each aura *is*. The missing half was one derivation — +//! `EffectApplyAuraName` 78 (`SPELL_AURA_MOUNTED`) and 36 (`SPELL_AURA_MOD_SHAPESHIFT`), now on +//! [`crate::script::AuraState`] beside `channeled`, which is derived the same way. With that, each +//! verb is the aura it names cancelled by id, which is exactly what the reference's own right-click +//! on the buff does. +//! +//! They were not optional: our own pfUI port calls both. `pfUI/modules/autoshift.lua:45` answers +//! `ERR_CANT_INTERACT_SHAPESHIFTED` with `if IsMounted() then Dismount() else +//! CancelShapeshiftForm() end`, and grom's 2026-11-05 log carries +//! `attempt to call global 'Dismount' (a nil value)` from exactly there — a port written against +//! verbs this client did not have. use mlua::Lua; @@ -85,9 +96,53 @@ pub(super) fn install(lua: &Lua) -> mlua::Result<()> { })?, )?; + // Dismount() — the mount aura cancelled by spell id. `CMSG_CANCEL_AURA` carries an id, which + // is why the aura has to be identified rather than merely known to exist. + // + // Not gated on [`crate::script::aura::cancel_authorized`], unlike `CancelPlayerBuff`: that + // gate is the reference's own check inside *that* verb, and this is not that verb. The server + // makes its own decision about a cancel it is sent, and a mount whose aura is not flagged + // `AFLAG_CANCELABLE` would otherwise be undismountable with no way to tell why. + g.set( + "Dismount", + lua.create_function(|lua, ()| { + queue_cancel(lua, |a| a.mount); + Ok(()) + })?, + )?; + + // CancelShapeshiftForm() — the same, for the form aura. Vanilla's own way out of a form is to + // re-cast its spell, which needs the *bar slot*; cancelling the aura needs only the id, and + // reaches a form the bar does not list. + g.set( + "CancelShapeshiftForm", + lua.create_function(|lua, ()| { + queue_cancel(lua, |a| a.shapeshift); + Ok(()) + })?, + )?; + Ok(()) } +/// Queue `CMSG_CANCEL_AURA` for the first of the player's auras that `want` accepts; a no-op when +/// none does, as both verbs are when the player is neither mounted nor shifted. +fn queue_cancel(lua: &Lua, want: impl Fn(&crate::script::AuraState) -> bool) { + let hit = { + let model = lua.app_data_ref::().expect("model app_data"); + model + .auras + .get("player") + .and_then(|list| list.iter().find(|a| want(a)).map(|a| a.spell_id)) + }; + if let Some(id) = hit { + lua.app_data_mut::() + .expect("model app_data") + .cancel_aura_requests + .push(id); + } +} + #[cfg(test)] mod tests { use crate::script::{SelfMovement, UiScript}; @@ -147,6 +202,56 @@ mod tests { ); } + /// `Dismount` and `CancelShapeshiftForm` each cancel the one aura that *is* the state, by + /// spell id, and neither can be confused for the other or for an ordinary buff. Our pfUI port + /// calls both from one `if`, which is why they have to be two different ids and not "the first + /// cancellable buff". + #[test] + fn dismount_and_cancel_form_each_cancel_their_own_aura_by_id() { + use crate::script::AuraState; + + let aura = |spell_id: u32, mount: bool, shapeshift: bool| AuraState { + spell_id, + helpful: true, + cancelable: true, + count: 1, + mount, + shapeshift, + ..Default::default() + }; + let mut s = UiScript::new().unwrap(); + + // Neither mounted nor shifted: both are silent no-ops, not errors. + s.set_auras("player", Some(vec![aura(1126, false, false)])); + s.run("Dismount() CancelShapeshiftForm()").unwrap(); + assert!( + s.take_cancel_aura_requests().is_empty(), + "an ordinary buff is neither a mount nor a form" + ); + + // Mounted, in a form, and with a plain buff ahead of both in the list. + s.set_auras( + "player", + Some(vec![ + aura(1126, false, false), + aura(768, false, true), + aura(458, true, false), + ]), + ); + s.run("Dismount()").unwrap(); + assert_eq!( + s.take_cancel_aura_requests(), + vec![458], + "the mount's spell id, past the buff and the form" + ); + s.run("CancelShapeshiftForm()").unwrap(); + assert_eq!( + s.take_cancel_aura_requests(), + vec![768], + "the form's own spell id" + ); + } + #[test] fn group_predicates_read_the_roster_not_the_posture_snapshot() { use crate::script::{PartyMemberInfo, PartyState}; diff --git a/crates/benilla-ui/src/script/compat/classic_api/unit_auras.rs b/crates/benilla-ui/src/script/compat/classic_api/unit_auras.rs index 3933f184..87137a6f 100644 --- a/crates/benilla-ui/src/script/compat/classic_api/unit_auras.rs +++ b/crates/benilla-ui/src/script/compat/classic_api/unit_auras.rs @@ -482,6 +482,8 @@ mod tests { cancelable: true, until_cancelled: false, channeled: false, + mount: false, + shapeshift: false, } } diff --git a/crates/benilla-ui/src/script/tests/reference_surface.rs b/crates/benilla-ui/src/script/tests/reference_surface.rs index ae036fb8..687d5e1b 100644 --- a/crates/benilla-ui/src/script/tests/reference_surface.rs +++ b/crates/benilla-ui/src/script/tests/reference_surface.rs @@ -69,6 +69,16 @@ fn allowed_beyond_1_12() -> HashSet<&'static str> { "securecall", "issecure", "forceinsecure", + // ── the two actions 1.12 leaves to the buff frame's right-click ── + // Both cancel one aura by spell id, which is the only thing `CMSG_CANCEL_AURA` carries, + // and in 1.12 a player reaches that by right-clicking the buff. There is no verb for it + // because there was no need: the stock UI has the icon under the cursor. An addon that + // reacts to `ERR_CANT_INTERACT_SHAPESHIFTED` has no icon and no cursor, which is why every + // later client added these two and why our own pfUI port calls them — + // `pfUI/modules/autoshift.lua:45`, whose `attempt to call global 'Dismount'` is in grom's + // 2026-11-05 log. A 1.12 addon that feature-detects them finds a working pair. + "Dismount", + "CancelShapeshiftForm", // ── WoW API past 1.12 ── // Only `SubmitChatInput` is called from our UI files (`ScriptLogFrame.xml`'s slash // commands); each of the rest awaits its 1.12 equivalent, removal or a reason.