diff --git a/crates/benilla-app/assets/ui/ActionBar.xml b/crates/benilla-app/assets/ui/ActionBar.xml
index 64589005..ffb565b0 100644
--- a/crates/benilla-app/assets/ui/ActionBar.xml
+++ b/crates/benilla-app/assets/ui/ActionBar.xml
@@ -363,15 +363,27 @@
-- tooltip with its rank column, an ITEM slot the shared item renderer; ref
-- ActionButton_SetTooltip). The re-enter keeps it live across page flips.
-- Anchor: ref ActionButton_SetTooltip (ActionButton.lua l.366-372) branches on the
- -- UberTooltips CVar — "1" → the default corner, else RIGHT/LEFT by bar. UberTooltips
- -- registers with default "1" (byte-read: WoW.exe 0x48fdd9, default string 0x82e748;
- -- sibling rows BlockTrades→"0", UnitNameRenderMode→"2" confirm the layout), and benilla
- -- ships no CVar state, so the branch collapses to the stock-client one: action-bar
- -- tooltips live at the screen's bottom-right, above the bags.
+ -- UberTooltips CVar — "1" → the default corner, else beside the button, LEFT for the bars
+ -- that sit at the screen edge and RIGHT for the rest. This branch was collapsed to the "1"
+ -- leg until B230, on the reason that benilla shipped no CVar state for anything to move
+ -- it; 0954 ended that and cvars.rs now registers the key (default "1", byte-read — the
+ -- evidence lives on that row), so the fork is live and the stock look is unchanged.
+ --
+ -- The ref's LEFT set is {MultiBarBottomRight, MultiBarRight, MultiBarLeft}. Only the first
+ -- exists here — the two VERTICAL bars are deliberately not shipped (see the header of
+ -- MultiBars.xml) — so it is the whole test, and the day they land they join it.
function BenillaActionButton_OnEnter(button)
BenillaActionButton_Adopt(button)
if not GameTooltip then return end -- engine tests load file subsets
- GameTooltip_SetDefaultAnchor(GameTooltip, button)
+ if GetCVar("UberTooltips") == "0" then
+ if button:GetParent() == MultiBarBottomRight then
+ GameTooltip:SetOwner(button, "ANCHOR_LEFT")
+ else
+ GameTooltip:SetOwner(button, "ANCHOR_RIGHT")
+ end
+ else
+ GameTooltip_SetDefaultAnchor(GameTooltip, button)
+ end
GameTooltip:SetAction(BenillaActionButton_Action(button))
end
diff --git a/crates/benilla-app/assets/ui/GameMenuFrame.xml b/crates/benilla-app/assets/ui/GameMenuFrame.xml
index d7811207..f4e50673 100644
--- a/crates/benilla-app/assets/ui/GameMenuFrame.xml
+++ b/crates/benilla-app/assets/ui/GameMenuFrame.xml
@@ -207,7 +207,10 @@
MainMenuFrameButtonTemplate (Classic/Frame/MainMenuFrameTemplates.xml l.11-16) is that same
button — same art, same trio, same size — so the header's ART BYTES note is unchanged. -->
-
+
+
StackSplitFrame_OnHide()
+ StackSplitFrame_OnChar()
+ StackSplitFrame_OnKeyDown()
diff --git a/crates/benilla-app/assets/ui/StanceBar.xml b/crates/benilla-app/assets/ui/StanceBar.xml
index 3529dbf2..43c6a3ba 100644
--- a/crates/benilla-app/assets/ui/StanceBar.xml
+++ b/crates/benilla-app/assets/ui/StanceBar.xml
@@ -62,11 +62,16 @@
-- Hover: the form's spell tooltip through the engine channel (ref ShapeshiftBar's
-- GameTooltip:SetShapeshift; decision 0274 P2). Anchor: the ref's ShapeshiftButton
-- OnEnter (BonusActionBarFrame.xml l.40-45) takes the default corner when UberTooltips
- -- is "1" — its byte-verified stock default (ActionBar.xml has the read) — so the branch
- -- collapses to the default corner here too.
+ -- is "1" — its byte-verified stock default — and sits beside the button otherwise. Like
+ -- the action bar's, that fork was collapsed to the "1" leg until B230 registered the CVar
+ -- (cvars.rs carries the byte-read); the stock look is unchanged, the off leg is reachable.
function BenillaShapeshiftButton_OnEnter(button)
if not GameTooltip then return end -- engine tests load file subsets
- GameTooltip_SetDefaultAnchor(GameTooltip, button)
+ if GetCVar("UberTooltips") == "0" then
+ GameTooltip:SetOwner(button, "ANCHOR_RIGHT")
+ else
+ GameTooltip_SetDefaultAnchor(GameTooltip, button)
+ end
GameTooltip:SetShapeshift(button.index)
end
diff --git a/crates/benilla-app/assets/ui/UIDropDownMenu.xml b/crates/benilla-app/assets/ui/UIDropDownMenu.xml
index e9f22004..17382700 100644
--- a/crates/benilla-app/assets/ui/UIDropDownMenu.xml
+++ b/crates/benilla-app/assets/ui/UIDropDownMenu.xml
@@ -1119,7 +1119,8 @@
it is ONE library file replicated, and only that one of its two sites is unguarded (the
other is `info.textHeight or UIDROPDOWNMENU_DEFAULT_TEXT_HEIGHT or 10`). Writing a
literal here would have been a silent fidelity divergence, not a fix. -->
-
+
+
local fontName, fontHeight, fontFlags = getglobal("DropDownList1Button1NormalText"):GetFont();
@@ -1131,7 +1132,7 @@
-
+
CloseDropDownMenus(3);
diff --git a/crates/benilla-app/assets/ui/UiPanels.xml b/crates/benilla-app/assets/ui/UiPanels.xml
index 1748c712..8785de46 100644
--- a/crates/benilla-app/assets/ui/UiPanels.xml
+++ b/crates/benilla-app/assets/ui/UiPanels.xml
@@ -1712,7 +1712,9 @@
the click's hide so a second instance must be free; the ref's own StaticPopup2 case).
Extend toward 4 the day a third coexists. StaticPopup2 seats below 1 (the ref's
StaticPopup_SetUpPosition stack, simplified to a static anchor). -->
-
, errors: &mut CastErrors) {
debug!("net: pet cast failed — spell {spell_id} reason {reason:?}");
if let Some(reason) = reason {
- errors.0.push((spell_id, reason));
+ errors.push_local(spell_id, reason);
}
}
diff --git a/crates/benilla-app/src/net/apply/spells.rs b/crates/benilla-app/src/net/apply/spells.rs
index 445de67e..671cba29 100644
--- a/crates/benilla-app/src/net/apply/spells.rs
+++ b/crates/benilla-app/src/net/apply/spells.rs
@@ -77,6 +77,7 @@ pub(super) fn cast_result(
spell_id: u32,
success: bool,
reason: Option,
+ arg: Option,
commands: &mut Commands,
self_guid: &SelfGuid,
index: &GuidIndex,
@@ -153,7 +154,11 @@ pub(super) fn cast_result(
// still shows "out of range"/"line of sight" even though no bar is (or should be) up.
if let Some(reason) = reason {
if !dont_report {
- cast_errors.0.push((spell_id, reason));
+ cast_errors.0.push(crate::ui_action::CastFail {
+ spell_id,
+ reason,
+ arg,
+ });
}
}
// The cast bar's red "Failed" — only the showing cast's own failure turns it red.
@@ -1336,6 +1341,7 @@ mod tests {
75,
false,
Some(reason),
+ None,
&mut commands,
&self_guid,
&index,
diff --git a/crates/benilla-app/src/target/click.rs b/crates/benilla-app/src/target/click.rs
index 03bfaf07..8827bed1 100644
--- a/crates/benilla-app/src/target/click.rs
+++ b/crates/benilla-app/src/target/click.rs
@@ -487,7 +487,7 @@ enum KeyFact {
fn resolve_go_action(
guid: u64,
inputs: &mut GoLockInputs,
- known: &std::collections::HashSet,
+ known: &std::collections::BTreeSet,
go: Option<(&ObjectStore, u32)>,
me_store: Option<&ObjectStore>,
net: &NetCommands,
@@ -516,7 +516,14 @@ fn resolve_go_action(
);
let key_entry = match outcome {
super::lock::LockOutcome::Unlocked => return GoAction::Use,
- super::lock::LockOutcome::OpenBySpell(spell_id) => return GoAction::OpenLock(spell_id),
+ super::lock::LockOutcome::OpenBySpell(spell_id) => {
+ // The retest instrument for B247 (decision 1312): which of the player's openers this
+ // lock resolved to, by id. The spell's Spell.dbc name is what the cast bar prints, so
+ // reading the id off a probe run is how "the bar says Opening - No Text" becomes a
+ // machine-checkable fact instead of a screenshot.
+ debug!("target: lock {} → open by spell {spell_id}", tmpl.lock_id);
+ return GoAction::OpenLock(spell_id);
+ }
super::lock::LockOutcome::OpenByKey(entry) => entry,
super::lock::LockOutcome::Unmet => {
// Unopenable — §8.8's routing, which keys off Lock.dbc **slot 0** regardless of which
diff --git a/crates/benilla-app/src/target/lock.rs b/crates/benilla-app/src/target/lock.rs
index ad8e6651..4f141a39 100644
--- a/crates/benilla-app/src/target/lock.rs
+++ b/crates/benilla-app/src/target/lock.rs
@@ -27,7 +27,7 @@
//! the skill block. That is not an approximation on the client's part: the DBC encodes the same
//! number (Pick Lock at 60 → 300, a rogue's cap; Mining at 60 → 300).
-use std::collections::HashSet;
+use std::collections::BTreeSet;
use benilla_formats::{LockSlot, LOCK_KEY_ITEM, LOCK_KEY_SKILL};
use bevy::prelude::*;
@@ -129,13 +129,20 @@ pub(crate) const GO_FLAG_LOCKED: u32 = 0x2;
/// `matched_spell` is the out-param the toast routing needs; it is written for a LockType match
/// even when the value test then fails.
///
-/// One deliberate difference: the reference walks its known-spell **array** in index order, while
-/// `known` is a set. That only decides *which* of several equally-matching spells lands in
-/// `matched_spell` (a miner knows four Mining ranks, all `LockType 3`), never the outcome — the
-/// scan keeps going past an insufficient one, so the best-valued opener still wins.
+/// **The scan order is part of the answer, not an implementation detail** (decision 1312). The
+/// reference walks its known-spell **array** in index order and *returns on the first sufficient
+/// match*, so when two known spells both match the LockType and both clear the requirement, the
+/// order decides which one is cast — and that spell's name is what the cast bar reads. This was
+/// documented here for a long time as a harmless difference ("it only decides which spell lands in
+/// `matched_spell`, never the outcome"), which is false: every character knows both 6478 "Opening"
+/// and 22810 **"Opening - No Text"** — both `SPELL_EFFECT_OPEN_LOCK` on `LockType 13` (Open
+/// Kneeling), both trivially sufficient against the `Skill == 0` slots the ground containers carry
+/// — so iterating a `HashSet` put Blizzard's placeholder name on the cast bar at the hash's whim
+/// (B247). `known` is a [`BTreeSet`] for exactly that reason: ascending spell id is the reference
+/// array's own order after login, the server building `SMSG_INITIAL_SPELLS` out of a `std::map`.
pub(crate) fn resolve_lock(
slots: &[LockSlot],
- known: &HashSet,
+ known: &BTreeSet,
spells: Option<&crate::ui_action::Spells>,
me: Option<&ObjectStore>,
items: &crate::items::Items,
@@ -375,7 +382,7 @@ mod tests {
let mut matched = None;
let out = resolve_lock(
&slots,
- &HashSet::new(),
+ &BTreeSet::new(),
None,
None,
&items,
@@ -461,7 +468,7 @@ mod tests {
assert_eq!(
resolve_lock(
&scholomance,
- &HashSet::from([6247]),
+ &BTreeSet::from([6247]),
Some(&spells),
None,
&items,
@@ -480,7 +487,7 @@ mod tests {
assert_eq!(
resolve_lock(
&scholomance,
- &HashSet::from([6247]),
+ &BTreeSet::from([6247]),
Some(&spells),
None,
&items,
@@ -500,7 +507,7 @@ mod tests {
assert_eq!(
resolve_lock(
&scholomance,
- &HashSet::from([1804]),
+ &BTreeSet::from([1804]),
Some(&spells),
None,
&items,
@@ -528,7 +535,7 @@ mod tests {
assert_eq!(
resolve_lock(
&searing_gorge,
- &HashSet::from([6247]),
+ &BTreeSet::from([6247]),
Some(&spells),
None,
&items,
@@ -559,7 +566,7 @@ mod tests {
assert_eq!(
resolve_lock(
&vein,
- &HashSet::from([2575]),
+ &BTreeSet::from([2575]),
Some(&with_mining),
None,
&items,
@@ -574,6 +581,83 @@ mod tests {
);
}
+ /// **B247** (decision 1312): two known openers on one LockType, both sufficient — the LOWEST
+ /// spell id wins, deterministically, because that is the reference array's order and the scan
+ /// returns on its first sufficient match.
+ ///
+ /// The real values: lock 43 (Hyacinth Mushroom and every other ground container) carries one
+ /// SKILL slot, `LockType 13` "Open Kneeling", `Skill 0`, `Action 0`. Three shipped spells carry
+ /// `SPELL_EFFECT_OPEN_LOCK` on type 13 — 6478 "Opening", 17667 "Light On Fire", and 22810
+ /// **"Opening - No Text"** — and `playercreateinfo_spell` grants 6478 *and* 22810 to every
+ /// race/class. Whichever this scan returns is the spell the click casts, so its Spell.dbc name
+ /// is what the cast bar prints: with a `HashSet` here the bar read Blizzard's placeholder about
+ /// half the time.
+ #[test]
+ fn two_sufficient_openers_pick_the_lower_spell_id() {
+ use benilla_formats::{OpenLock, SpellCatalog, SpellDisplay};
+
+ let mut mushroom = [LockSlot::default(); 8];
+ mushroom[0] = skill_slot(13, 0, 0);
+
+ // Both "Opening" spells: OPEN_LOCK on type 13, flat value 100 (base 99 + dice 1), which
+ // clears the slot's `Skill 0`. Identical in every input the resolver reads — the pick is
+ // decided by visit order and nothing else, which is the whole point.
+ let kneeling_opener = || SpellDisplay {
+ open_lock: Some(OpenLock {
+ lock_type: 13,
+ effect: 0,
+ }),
+ effect_base_points: [99, 0, 0],
+ effect_base_dice: [1, 0, 0],
+ ..Default::default()
+ };
+ let spells = crate::ui_action::Spells {
+ catalog: SpellCatalog::from_displays(
+ [(6478, kneeling_opener()), (22810, kneeling_opener())]
+ .into_iter()
+ .collect(),
+ ),
+ ..crate::ui_action::Spells::empty_for_tests()
+ };
+ let items = crate::items::Items::default();
+ let unlocked = GoFacts {
+ state: GO_STATE_READY,
+ flag_locked: false,
+ level: 0,
+ };
+
+ let mut matched = None;
+ assert_eq!(
+ resolve_lock(
+ &mushroom,
+ &BTreeSet::from([6478, 22810]),
+ Some(&spells),
+ None,
+ &items,
+ unlocked,
+ &mut matched,
+ ),
+ LockOutcome::OpenBySpell(6478),
+ "6478 \"Opening\", never 22810 \"Opening - No Text\""
+ );
+ assert_eq!(matched, Some(6478), "the toast's out-param agrees");
+
+ // Insertion order cannot change it — the set is ordered, so the scan is too.
+ let mut matched = None;
+ assert_eq!(
+ resolve_lock(
+ &mushroom,
+ &BTreeSet::from([22810, 6478]),
+ Some(&spells),
+ None,
+ &items,
+ unlocked,
+ &mut matched,
+ ),
+ LockOutcome::OpenBySpell(6478),
+ );
+ }
+
/// An all-empty row is not a lock at all — `CMSG_GAMEOBJ_USE`.
#[test]
fn an_empty_row_is_unlocked() {
@@ -583,7 +667,7 @@ mod tests {
assert_eq!(
resolve_lock(
&slots,
- &HashSet::new(),
+ &BTreeSet::new(),
None,
None,
&items,
diff --git a/crates/benilla-app/src/ui_action/cast_fail.rs b/crates/benilla-app/src/ui_action/cast_fail.rs
index ac83bb76..1ecf7359 100644
--- a/crates/benilla-app/src/ui_action/cast_fail.rs
+++ b/crates/benilla-app/src/ui_action/cast_fail.rs
@@ -10,18 +10,33 @@
//! screen shows "Spell is not ready yet." while `SPELL_FAILED_NOT_READY` reads
//! "Not yet recovered", and "Not enough rage" for a rage spell's NO_POWER.
//!
+//! 3. A per-reason **argument arm** (the second dispatch `0x6e1d8e`, 13 targets) fills that
+//! message's own `%s`/`%d` from a DBC or item name before it is displayed. The two arms whose
+//! tables benilla already loads live in [`FailArgs::fill`]; the drain fills three more before
+//! calling here (below), and the rest strip.
+//!
//! Strings are never hardcoded here: every message resolves from the VM's loaded
//! `GlobalStrings.lua` by key, so localization rides for free. Suppression is faithful on
//! both mechanisms: reason `0x17` (DONT_REPORT) never reaches display (control-flow), and a
//! key absent from GlobalStrings displays as nothing (data — 0x08/0x21/0x75, happiness
-//! NO_POWER). Known approximations, each marked below: the DBC-name `%s` fills of the
-//! argument-formatted reasons are stripped instead of filled — except `0x78` TOTEMS and
-//! `0x5c` REAGENTS, whose item-name fills (and query-then-redisplay cache-miss behavior) the
-//! drain models before calling here ("Requires Mining Pick", decisions 0545 + 0552) — and
-//! `0x0a`'s item-spell leg (`ERR_INVALID_ITEM_TARGET`) is unmodeled — the drain does not
-//! know item-ness.
+//! NO_POWER).
+//!
+//! **The argument arms, and what is still approximate.** Filled: `0x5e` REQUIRES_SPELL_FOCUS and
+//! `0x5d` REQUIRES_AREA here (decision 1313 — `0x6e1f62`/`0x6e1fad`), and `0x78` TOTEMS / `0x5c`
+//! REAGENTS / `0x19`–`0x1b` EQUIPPED_ITEM_CLASS\* in the drain, which owns them because their
+//! fills need the item caches and the query-then-redisplay cache-miss behavior ("Requires Mining
+//! Pick", decisions 0545 + 0552). Still stripped rather than filled — each needs a DBC we do not
+//! load yet: `0x56` ONLY_SHAPESHIFT (form-name list), `0x8d` PREVENTED_BY_MECHANIC
+//! (`SpellMechanic.dbc`), `0x90` MIN_SKILL (`SkillLine.dbc`), `0x31` NEED_EXOTIC_AMMO and `0x84`
+//! PROSPECT_NEED_MORE. Stripping is a **deliberate divergence**, now byte-confirmed as one: on a
+//! bad id or an absent word the reference jumps to the default arm with the pointer still on the
+//! *unfilled* template, so it displays a literal `Requires %s` (wow-re §WIRE-ARGS C3 — the fill
+//! path's own buffer swap sits after the printf and is skipped). We show the bare stem instead:
+//! "Requires" reads as terse, "Requires %s" reads as broken software (§7 — judge by the result).
+//! `0x0a`'s item-spell leg (`ERR_INVALID_ITEM_TARGET`) is unmodeled — the drain does not know
+//! item-ness.
-use benilla_formats::SpellDisplay;
+use benilla_formats::{AreaTableCatalog, SpellDisplay, SpellFocusCatalog};
/// Wire reason → its `SPELL_FAILED_*` GlobalStrings key (the `0x6e23e0` table, byte-exact).
pub(super) const CAST_FAIL_KEYS: [&str; 146] = [
@@ -194,12 +209,64 @@ fn is_food(spell: Option<&SpellDisplay>) -> bool {
spell.is_some_and(|d| matches!(d.category, 0xA | 0xB))
}
+/// The argument arms' inputs: the wire's reason-specific word ([`super::CastFail::arg`]) and the
+/// DBC name tables the arms read. Both catalogs are `Option` because a client without game data
+/// has neither — the arm then declines and the template strips, exactly as an unmodeled arm does.
+#[derive(Default, Clone, Copy)]
+pub(super) struct FailArgs<'a> {
+ pub(super) arg: Option,
+ /// `SpellFocusObject.dbc` (`0xc0d800`) — the `0x5e` arm's names ("Anvil", "Forge", and the
+ /// Teldrassil moonwells the Crown of the Earth phials name).
+ pub(super) focus: Option<&'a SpellFocusCatalog>,
+ /// `AreaTable.dbc` (`0xc0e048`) — the `0x5d` arm's `AreaName`.
+ pub(super) areas: Option<&'a AreaTableCatalog>,
+}
+
+impl FailArgs<'_> {
+ /// The `%s` fill for the argument-formatted reasons this module owns, or `None` to leave the
+ /// template to the strip fallback.
+ ///
+ /// Both arms are byte-verified and §5 cross-checked (wow-re `cast-fail-strings.md`
+ /// §WIRE-ARGS): each reads the **wire's** first argument word — `[ebx+8]`, the handler's own
+ /// stack slot, never a re-read of `Spell.dbc` — indexes its DBC store, and `SStrPrintf`s the
+ /// reason's `SPELL_FAILED_*` template. The errorId stays the default `0x2c` (`"%s"`) for both,
+ /// so what the player reads IS the filled template. A single-`%s` template is what makes a
+ /// plain `replace` faithful here; the reference runs a real two-pass printf, which would
+ /// matter for a multi-specifier format.
+ ///
+ /// **`0x5e` REQUIRES_SPELL_FOCUS** (`0x6e1f62`) → `SpellFocusObject.dbc` (`0xc0d800`)
+ /// `Name_Lang` at `row + 0x4 + locale*4`, so `"Requires %s"` reads "Requires Starbreeze
+ /// Village Moonwell". The failing spell's own `RequiresSpellFocus` column holds the same
+ /// number vmangos copied onto the wire, so we fall back to it when the word is absent — a
+ /// benilla-side robustness margin, not a transcription: the client itself never reads
+ /// `SpellRec+0x3c` on this path.
+ ///
+ /// **`0x5d` REQUIRES_AREA** (`0x6e1fad`) → `AreaTable.dbc` (`0xc0e048`) `AreaName_Lang` at
+ /// `row + 0x2c + locale*4`, so `"You need to be in %s"` names the zone. This one has **no**
+ /// client-side stand-in: the server derives the id from its own `spell_area` rows and nothing
+ /// in `Spell.dbc` holds it, so an absent word means an unfilled message.
+ fn fill(&self, reason: u8, spell: Option<&SpellDisplay>) -> Option {
+ match reason {
+ 0x5D => self.areas?.name(self.arg?).map(str::to_string),
+ 0x5E => {
+ let id = self
+ .arg
+ .filter(|&id| id != 0)
+ .or_else(|| Some(spell?.requires_spell_focus).filter(|&id| id != 0))?;
+ self.focus?.name(id).map(str::to_string)
+ }
+ _ => None,
+ }
+ }
+}
+
/// The displayed text for a failed cast — `None` = the reference shows nothing. `get` is the
/// VM's GlobalStrings lookup (an absent or empty key resolves to `None`, the data-suppression
/// face). Reasons beyond the table print their code — our debug affordance, not a ref string.
pub(super) fn cast_fail_text(
reason: u8,
spell: Option<&SpellDisplay>,
+ args: FailArgs<'_>,
get: &dyn Fn(&str) -> Option,
) -> Option {
let get_display = |key: &str| get(key).filter(|s| !s.is_empty());
@@ -248,9 +315,12 @@ pub(super) fn cast_fail_text(
return Some(format!("Spell failed ({reason:#04x})"));
};
let text = get_display(key)?;
- // The argument-formatted reasons fill %s/%d from DBC names (0x6e1d8e's arm table) —
- // unmodeled: strip the tokens so the stem reads clean ("Missing reagent: %s" →
- // "Missing reagent"), never a raw % on screen.
+ // The argument arms (`0x6e1d8e`): fill the template's `%s` from the reason's own DBC.
+ if let Some(name) = args.fill(reason, spell) {
+ return Some(text.replace("%s", &name));
+ }
+ // An arm we don't model (or one whose lookup missed) — strip the tokens so the stem reads
+ // clean ("Missing reagent: %s" → "Missing reagent"), never a raw % on screen.
if text.contains('%') {
let stripped = text
.replace("%s", "")
@@ -321,27 +391,36 @@ mod tests {
fn overrides_replace_and_passthrough_reads() {
let m = gs();
let g = getter(&m);
- assert_eq!(cast_fail_text(0x43, None, &g).unwrap(), "Out of ammo");
- assert_eq!(cast_fail_text(0x59, None, &g).unwrap(), "Out of range.");
- assert_eq!(cast_fail_text(0x76, None, &g).unwrap(), "Target too close");
assert_eq!(
- cast_fail_text(0x09, None, &g).unwrap(),
+ cast_fail_text(0x43, None, FailArgs::default(), &g).unwrap(),
+ "Out of ammo"
+ );
+ assert_eq!(
+ cast_fail_text(0x59, None, FailArgs::default(), &g).unwrap(),
+ "Out of range."
+ );
+ assert_eq!(
+ cast_fail_text(0x76, None, FailArgs::default(), &g).unwrap(),
+ "Target too close"
+ );
+ assert_eq!(
+ cast_fail_text(0x09, None, FailArgs::default(), &g).unwrap(),
"You have no target."
);
// 0x3c: plain spell → spell cooldown; Attr&0x10 → ability; potion category → potion.
let plain = spell(0, 0, 0);
assert_eq!(
- cast_fail_text(0x3C, Some(&plain), &g).unwrap(),
+ cast_fail_text(0x3C, Some(&plain), FailArgs::default(), &g).unwrap(),
"Spell is not ready yet."
);
let ability = spell(1, 0, 0x10);
assert_eq!(
- cast_fail_text(0x3C, Some(&ability), &g).unwrap(),
+ cast_fail_text(0x3C, Some(&ability), FailArgs::default(), &g).unwrap(),
"Ability is not ready yet."
);
let potion = spell(0, 4, 0);
assert_eq!(
- cast_fail_text(0x3C, Some(&potion), &g).unwrap(),
+ cast_fail_text(0x3C, Some(&potion), FailArgs::default(), &g).unwrap(),
"Item is not ready yet."
);
}
@@ -354,12 +433,12 @@ mod tests {
let g = getter(&m);
let rage = spell(1, 0, 0);
assert_eq!(
- cast_fail_text(0x4D, Some(&rage), &g).unwrap(),
+ cast_fail_text(0x4D, Some(&rage), FailArgs::default(), &g).unwrap(),
"Not enough rage"
);
let mana = spell(0, 0, 0);
assert_eq!(
- cast_fail_text(0x4D, Some(&mana), &g).unwrap(),
+ cast_fail_text(0x4D, Some(&mana), FailArgs::default(), &g).unwrap(),
"Not enough mana"
);
}
@@ -371,13 +450,16 @@ mod tests {
fn suppression_hex_fallback_and_template_strip() {
let m = gs();
let g = getter(&m);
- assert_eq!(cast_fail_text(0x17, None, &g), None);
- assert_eq!(cast_fail_text(0x08, None, &g), None);
+ assert_eq!(cast_fail_text(0x17, None, FailArgs::default(), &g), None);
+ assert_eq!(cast_fail_text(0x08, None, FailArgs::default(), &g), None);
assert_eq!(
- cast_fail_text(0x92, None, &g).unwrap(),
+ cast_fail_text(0x92, None, FailArgs::default(), &g).unwrap(),
"Spell failed (0x92)"
);
- assert_eq!(cast_fail_text(0x5C, None, &g).unwrap(), "Missing reagent");
+ assert_eq!(
+ cast_fail_text(0x5C, None, FailArgs::default(), &g).unwrap(),
+ "Missing reagent"
+ );
}
/// The RUNTIME leg, end to end on the real data: the shipped `GlobalStrings.lua` executed
@@ -396,30 +478,93 @@ mod tests {
s.run(&String::from_utf8_lossy(&src)).expect("runs clean");
let g = |key: &str| s.lua().globals().get::(key).ok();
- assert_eq!(cast_fail_text(0x43, None, &g).unwrap(), "Out of ammo");
- assert_eq!(cast_fail_text(0x59, None, &g).unwrap(), "Out of range.");
assert_eq!(
- cast_fail_text(0x3C, None, &g).unwrap(),
+ cast_fail_text(0x43, None, FailArgs::default(), &g).unwrap(),
+ "Out of ammo"
+ );
+ assert_eq!(
+ cast_fail_text(0x59, None, FailArgs::default(), &g).unwrap(),
+ "Out of range."
+ );
+ assert_eq!(
+ cast_fail_text(0x3C, None, FailArgs::default(), &g).unwrap(),
"Spell is not ready yet."
);
let rage = spell(1, 0, 0);
assert_eq!(
- cast_fail_text(0x4D, Some(&rage), &g).unwrap(),
+ cast_fail_text(0x4D, Some(&rage), FailArgs::default(), &g).unwrap(),
"Not enough rage"
);
// The environment gate's pair (decision 1056) — both are plain passthroughs, so what the
// player reads IS the GlobalStrings value. A typo'd key here would degrade a real refusal
// to a dead-looking button, which is what this test exists to catch.
assert_eq!(
- cast_fail_text(0x50, None, &g).unwrap(),
+ cast_fail_text(0x50, None, FailArgs::default(), &g).unwrap(),
"Cannot use while swimming"
);
assert_eq!(
- cast_fail_text(0x58, None, &g).unwrap(),
+ cast_fail_text(0x58, None, FailArgs::default(), &g).unwrap(),
"Can only use while swimming"
);
// The data-suppression face on the real file: the absent keys show nothing.
- assert_eq!(cast_fail_text(0x08, None, &g), None);
- assert_eq!(cast_fail_text(0x21, None, &g), None);
+ assert_eq!(cast_fail_text(0x08, None, FailArgs::default(), &g), None);
+ assert_eq!(cast_fail_text(0x21, None, FailArgs::default(), &g), None);
+
+ // B255, end to end on the real data: the argument arms against the real DBCs and the real
+ // GlobalStrings templates. Without the fill these read as the bare stems "Requires" and
+ // "You need to be in" — which is exactly what shipped.
+ let focus =
+ benilla_formats::load_spell_focus_catalog(&mut chain).expect("SpellFocusObject");
+ let areas = benilla_formats::load_area_table_catalog(&mut chain).expect("AreaTable");
+ let args = |arg: u32| FailArgs {
+ arg: Some(arg),
+ focus: Some(&focus),
+ areas: Some(&areas),
+ };
+ // 0x5e REQUIRES_SPELL_FOCUS: the Crown of the Earth phials' own refusal. Focus 12 is the
+ // Starbreeze Village moonwell — using the Jade Phial at any *other* pool is the report.
+ assert_eq!(
+ cast_fail_text(0x5E, None, args(12), &g).unwrap(),
+ "Requires Starbreeze Village Moonwell"
+ );
+ assert_eq!(
+ cast_fail_text(0x5E, None, args(1), &g).unwrap(),
+ "Requires Anvil"
+ );
+ // 0x5d REQUIRES_AREA: area 1657 is Darnassus.
+ assert_eq!(
+ cast_fail_text(0x5D, None, args(1657), &g).unwrap(),
+ "You need to be in Darnassus"
+ );
+ // The fallbacks. An id the DBC doesn't name, and a wire word the server never sent, both
+ // decline the arm and fall through to the strip — never a raw `%s` on screen.
+ assert_eq!(
+ cast_fail_text(0x5E, None, args(999_999), &g).unwrap(),
+ "Requires"
+ );
+ assert_eq!(
+ cast_fail_text(0x5D, None, FailArgs::default(), &g).unwrap(),
+ "You need to be in"
+ );
+ // 0x5e alone has a client-side stand-in: the failing spell's own `RequiresSpellFocus`
+ // column is the very number the server copied onto the wire, so an absent word still
+ // fills. (Spell 4976 "Filling" — the Crystal Phial's — carries focus 11.)
+ let filling = SpellDisplay {
+ requires_spell_focus: 11,
+ ..Default::default()
+ };
+ assert_eq!(
+ cast_fail_text(
+ 0x5E,
+ Some(&filling),
+ FailArgs {
+ focus: Some(&focus),
+ ..FailArgs::default()
+ },
+ &g
+ )
+ .unwrap(),
+ "Requires Shadowglen Moonwell"
+ );
}
}
diff --git a/crates/benilla-app/src/ui_action/cast_send.rs b/crates/benilla-app/src/ui_action/cast_send.rs
index 534d2756..c44b644f 100644
--- a/crates/benilla-app/src/ui_action/cast_send.rs
+++ b/crates/benilla-app/src/ui_action/cast_send.rs
@@ -290,7 +290,7 @@ fn send_spell_cast(
// `HandleCastFailed`) — the inflight rec here is always an ordinary cast, so even an
// on-next-swing press is refused while it holds.
if pending.current(now) != Some(spell_id) {
- cast_errors.0.push((spell_id, 0x61));
+ cast_errors.push_local(spell_id, 0x61);
}
debug!("ui_action: cast {spell_id} suppressed — a cast is already in flight");
return;
@@ -343,7 +343,7 @@ fn send_spell_cast(
debug!(
"ui_action: cast {spell_id} refused locally — unbindable target ({reason:#x})"
);
- cast_errors.0.push((spell_id, reason));
+ cast_errors.push_local(spell_id, reason);
return;
}
},
@@ -371,7 +371,7 @@ fn send_spell_cast(
dist_sq,
) {
debug!("ui_action: cast {spell_id} refused locally — range ({reason:#x})");
- cast_errors.0.push((spell_id, reason));
+ cast_errors.push_local(spell_id, reason);
return;
}
}
@@ -417,7 +417,7 @@ fn send_spell_cast(
}
}
}
- cast_errors.0.push((spell_id, commit.not_ready_reason()));
+ cast_errors.push_local(spell_id, commit.not_ready_reason());
return;
}
// Rung 2 — the power gate (`0x60962c`, SPELL presses only: the item fork's clear-query jump
@@ -430,7 +430,7 @@ fn send_spell_cast(
if let (Some(d), Some(store)) = (def, ctx.rel.self_store) {
if !super::usable::can_afford(d, store) {
debug!("ui_action: cast {spell_id} refused locally — not enough power (0x4d)");
- cast_errors.0.push((spell_id, 0x4d));
+ cast_errors.push_local(spell_id, 0x4d);
return;
}
}
@@ -451,7 +451,7 @@ fn send_spell_cast(
def,
) {
debug!("ui_action: cast {spell_id} refused locally — mounted (0x39)");
- cast_errors.0.push((spell_id, 0x39));
+ cast_errors.push_local(spell_id, 0x39);
return;
}
// The ENVIRONMENT leg of the SAME requirement validator (`0x609d39–0x609d7b`, between the
@@ -463,7 +463,7 @@ fn send_spell_cast(
// (ledger B176) and lets you eat mid-swim (B155's eating half). The condition is [`state`]'s.
if let Some(reason) = state::cast_water_refusal(ctx.self_move_flags, def) {
debug!("ui_action: cast {spell_id} refused locally — water side ({reason:#x})");
- cast_errors.0.push((spell_id, reason));
+ cast_errors.push_local(spell_id, reason);
return;
}
// The moving leg of the SAME requirement validator (`0x609de3`, after the mounted/posture/
@@ -482,7 +482,7 @@ fn send_spell_cast(
let cast_time_ms = spells.map_or(0, |s| s.cast_time_ms(d, caster_level));
if state::cast_moving_refusal(ctx.self_move_flags, cast_time_ms, def) {
debug!("ui_action: cast {spell_id} refused locally — moving (0x2e)");
- cast_errors.0.push((spell_id, 0x2e));
+ cast_errors.push_local(spell_id, 0x2e);
return;
}
}
@@ -507,7 +507,7 @@ fn send_spell_cast(
if let Some(refusal) = d.form_refusal(form, form_is_stance) {
let reason = refusal.reason();
debug!("ui_action: cast {spell_id} refused locally — the form gate ({reason:#x})");
- cast_errors.0.push((spell_id, reason));
+ cast_errors.push_local(spell_id, reason);
return;
}
}
@@ -651,6 +651,7 @@ fn send_spell_cast(
mod tests {
use super::*;
use crate::net::Reputations;
+ use crate::ui_action::CastFail;
use bevy::ecs::system::RunSystemOnce;
use crossbeam_channel::Receiver;
@@ -934,7 +935,7 @@ mod tests {
assert!(rx.try_recv().is_err());
assert_eq!(
world.resource::().0,
- vec![(MOUNT, 0x61)],
+ vec![CastFail::local(MOUNT, 0x61)],
"a different spell mid-cast is 6e4d97's \"Another action is in progress\""
);
}
@@ -959,7 +960,10 @@ mod tests {
send(&mut world, HEARTHSTONE, HEARTH_COMMIT);
assert!(rx.try_recv().is_err(), "an item on cooldown never sends");
- assert_eq!(world.resource::().0, vec![(HEARTHSTONE, 0x28)]);
+ assert_eq!(
+ world.resource::().0,
+ vec![CastFail::local(HEARTHSTONE, 0x28)]
+ );
// The byte law's other half (0948, correcting this test's pre-§5 shape): the record is
// keyed (use-spell, item ENTRY), and a bare SPELL press queries (spell, 0) — the item
@@ -1075,7 +1079,10 @@ mod tests {
})
.expect("one-shot");
assert!(rx.try_recv().is_err(), "an unaffordable press never sends");
- assert_eq!(world.resource::().0, vec![(HEARTHSTONE, 0x4d)]);
+ assert_eq!(
+ world.resource::().0,
+ vec![CastFail::local(HEARTHSTONE, 0x4d)]
+ );
world.resource_mut::().0.clear();
let base = ctx();
@@ -1122,7 +1129,7 @@ mod tests {
assert!(rx.try_recv().is_err());
assert_eq!(
world.resource::().0,
- vec![(MOUNT, 0x61)],
+ vec![CastFail::local(MOUNT, 0x61)],
"mid-cast outranks the cooldown rung (the ref's IsCasting precedes the validator)"
);
}
diff --git a/crates/benilla-app/src/ui_action/errors.rs b/crates/benilla-app/src/ui_action/errors.rs
index 56ed109a..9825d1cc 100644
--- a/crates/benilla-app/src/ui_action/errors.rs
+++ b/crates/benilla-app/src/ui_action/errors.rs
@@ -1,8 +1,8 @@
//! The red error line's queues and resolvers — every route into `UIErrorsFrame`'s top line
//! except the cast-fail two-layer display (its own module, [`super::cast_fail`]):
//!
-//! - [`CastErrors`] — the wire `(spell_id, reason)` pairs (`SMSG_CAST_RESULT` + local cast
-//! refusals), resolved by [`super::cast_fail`] at the drain.
+//! - [`CastErrors`] — the wire [`CastFail`]s (`SMSG_CAST_RESULT` + local cast refusals), resolved
+//! by [`super::cast_fail`] at the drain.
//! - [`MountErrors`] — the `SMSG_MOUNTRESULT`/`SMSG_DISMOUNTRESULT` code pairs, resolved by
//! [`mount_result_key`] (decision 0441 P2).
//! - [`UiErrorKeys`] — client-LOCAL refusals straight by GlobalStrings key, the
@@ -21,12 +21,45 @@ use bevy::prelude::*;
use crate::net::ObjectStore;
-/// Cast failures queued for the UI error line, as `(spell_id, reason)` — the wire pair from
-/// `SMSG_CAST_RESULT` and the local refusals alike. The spell id rides along because the
-/// display layer keys several messages on the failing spell's record ([`super::cast_fail`]:
-/// NO_POWER's power family, the 0x28/0x3c cooldown families).
+/// Cast failures queued for the UI error line — the wire triple from `SMSG_CAST_RESULT` and the
+/// local refusals alike. The spell id rides along because the display layer keys several messages
+/// on the failing spell's record ([`super::cast_fail`]: NO_POWER's power family, the 0x28/0x3c
+/// cooldown families).
#[derive(Resource, Default)]
-pub(crate) struct CastErrors(pub Vec<(u32, u8)>);
+pub(crate) struct CastErrors(pub Vec);
+
+/// One queued cast failure: the failing spell, the wire reason, and the reason-specific argument
+/// word that fills the message's `%s` (the arm table at `0x6e1d8e` — a `SpellFocusObject.dbc` id
+/// for 0x5e, an `AreaTable.dbc` id for 0x5d).
+///
+/// `arg` is `None` for every **client-local** refusal, and that is faithful rather than a gap: the
+/// local refusals the reference raises itself go through `DisplayError` with no argText, and the
+/// two client-generated argument messages it *does* build (the lock toasts, MIN_SKILL) are
+/// [`UiError`]'s tenants, not this queue's.
+#[derive(Clone, Copy, Debug, PartialEq, Eq)]
+pub(crate) struct CastFail {
+ pub spell_id: u32,
+ pub reason: u8,
+ pub arg: Option,
+}
+
+impl CastFail {
+ /// A **client-local** refusal — no wire argument (see [`Self::arg`]).
+ pub(crate) const fn local(spell_id: u32, reason: u8) -> Self {
+ Self {
+ spell_id,
+ reason,
+ arg: None,
+ }
+ }
+}
+
+impl CastErrors {
+ /// Queue a [`CastFail::local`] refusal.
+ pub(crate) fn push_local(&mut self, spell_id: u32, reason: u8) {
+ self.0.push(CastFail::local(spell_id, reason));
+ }
+}
/// (Dis)mount refusals queued for the UI error line, as the wire pair `(mount, code)` from
/// `SMSG_MOUNTRESULT`/`SMSG_DISMOUNTRESULT` (decision 0441 P2) — resolved to text through the
@@ -200,7 +233,7 @@ pub(crate) fn reagent_totem_refusal(
return false;
};
debug!("cast {spell_id} refused locally — missing totem/reagent ({reason:#04x})");
- errors.0.push((spell_id, reason));
+ errors.push_local(spell_id, reason);
true
}
@@ -567,7 +600,7 @@ mod totem_reagent_tests {
&items,
&mut errors
));
- assert_eq!(errors.0.as_slice(), &[(2575, 0x78)]);
+ assert_eq!(errors.0.as_slice(), &[CastFail::local(2575, 0x78)]);
let mut reagents = [(0, 0); 8];
reagents[0] = (17056, 1); // Slow Fall's Light Feather
@@ -580,7 +613,7 @@ mod totem_reagent_tests {
&items,
&mut errors
));
- assert_eq!(errors.0.as_slice(), &[(130, 0x5c)]);
+ assert_eq!(errors.0.as_slice(), &[CastFail::local(130, 0x5c)]);
let both = spell([2901, 0], reagents);
let mut errors = CastErrors::default();
@@ -591,7 +624,7 @@ mod totem_reagent_tests {
&items,
&mut errors
));
- assert_eq!(errors.0.as_slice(), &[(1, 0x78)]);
+ assert_eq!(errors.0.as_slice(), &[CastFail::local(1, 0x78)]);
let plain = spell([0, 0], [(0, 0); 8]);
let mut errors = CastErrors::default();
diff --git a/crates/benilla-app/src/ui_action/feed.rs b/crates/benilla-app/src/ui_action/feed.rs
index eaa094fd..e5095cc2 100644
--- a/crates/benilla-app/src/ui_action/feed.rs
+++ b/crates/benilla-app/src/ui_action/feed.rs
@@ -84,6 +84,10 @@ pub(super) fn feed_actions(
mut items: ResMut,
icons: Option>,
sub_classes: Option>,
+ // The two DBC name tables the cast-fail argument arms read (`FailArgs`): the crafting book's
+ // SpellFocusObject catalog and the map arc's AreaTable one, both already loaded.
+ spell_focus: Option>,
+ areas: Option>,
commands: Res,
mut memory: Local>,
) {
@@ -102,12 +106,22 @@ pub(super) fn feed_actions(
// item-cache miss the ref queries and shows nothing that frame, then its DBCACHECALLBACK
// `0x6e29b0` REDISPLAYS when the answer lands — modeled by keeping the entry queued: the
// ask-once query is away, and the frame the template answers, the fill succeeds and fires.
+ // The DBC-only arms (0x5d REQUIRES_AREA, 0x5e REQUIRES_SPELL_FOCUS) need no round trip and
+ // are filled inside [`cast_fail`] itself, off the wire's argument word.
let self_store = self_q.iter().next();
- let mut await_template: Vec<(u32, u8)> = Vec::new();
+ let mut await_template: Vec = Vec::new();
+ let fail_args = cast_fail::FailArgs {
+ arg: None,
+ focus: spell_focus.as_deref().map(|f| &f.catalog),
+ areas: areas.as_deref().map(|a| &a.0),
+ };
let texts: Vec = cast_errors
.0
.drain(..)
- .filter_map(|(spell_id, reason)| {
+ .filter_map(|fail| {
+ let crate::ui_action::CastFail {
+ spell_id, reason, ..
+ } = fail;
let d = spells.as_ref().and_then(|s| s.catalog.get(spell_id));
let get = |key: &str| script.lua().globals().get::(key).ok();
// 0x19/0x1a/0x1b EQUIPPED_ITEM_CLASS* — the other argument-formatted family whose
@@ -154,7 +168,7 @@ pub(super) fn feed_actions(
// still pending → keep the entry queued for the redisplay.
None if items.template_answered_unknown(failing) => "UNKNOWN".to_string(),
None => {
- await_template.push((spell_id, reason));
+ await_template.push(fail);
return None;
}
};
@@ -167,7 +181,25 @@ pub(super) fn feed_actions(
.filter(|s| !s.is_empty())
.map(|t| t.replace("%s", &name));
}
- cast_fail::cast_fail_text(reason, d, &get)
+ let text = cast_fail::cast_fail_text(
+ reason,
+ d,
+ cast_fail::FailArgs {
+ arg: fail.arg,
+ ..fail_args
+ },
+ &get,
+ );
+ // The retest instrument for this whole bug class (decision 1313). A red-line defect is
+ // reported as *seen* — B255 arrived as a screenshot of the word "Requires" — and until
+ // this line the only way to read what the client resolved was to look at the screen.
+ // Logging the reason, its wire argument and the resolved text makes an argument arm
+ // that silently declined (a missing word, an unnamed id) legible from a probe run.
+ debug!(
+ "ui_action: cast fail — spell {spell_id} reason {reason:#04x} arg {:?} → {:?}",
+ fail.arg, text
+ );
+ text
})
.collect();
cast_errors.0.extend(await_template);
diff --git a/crates/benilla-app/src/ui_action/mod.rs b/crates/benilla-app/src/ui_action/mod.rs
index f9860cb7..c29d2b5a 100644
--- a/crates/benilla-app/src/ui_action/mod.rs
+++ b/crates/benilla-app/src/ui_action/mod.rs
@@ -22,7 +22,7 @@
//! [`errors`] (the red error line's two layers), [`usable`] (the castability walk the stance bar
//! shares), [`weapon_icon`] (the auto-attack's borrowed weapon icon).
-use std::collections::{HashMap, HashSet};
+use std::collections::{BTreeSet, HashMap};
use bevy::prelude::*;
@@ -65,8 +65,8 @@ pub(crate) struct CooldownEvents;
// `cast_send`, so a second send path cannot be written by accident (decision 0914).
pub(crate) use cast_send::{CastCommit, CastLadder};
pub(crate) use errors::{
- attack_actor_refusal, reagent_totem_refusal, ui_error_text, CastErrors, MountErrors, UiError,
- UiErrorKeys,
+ attack_actor_refusal, reagent_totem_refusal, ui_error_text, CastErrors, CastFail, MountErrors,
+ UiError, UiErrorKeys,
};
// `pub(crate)`: the target chain registers the cursor pre-empt + the click commit, and the
// spellbook/stance/craft drains thread the mode through the one cast-send path (decision 0792).
@@ -96,8 +96,17 @@ pub(crate) const SPELL_ATTACK: u32 = 6603;
pub(crate) struct PlayerActions {
/// Wire slot (0-based) → the slot's packed action. Lua action id = slot + 1.
pub buttons: HashMap,
- /// The spell book (`SMSG_INITIAL_SPELLS`), for gating/consumers to come.
- pub spells: HashSet,
+ /// The spell book (`SMSG_INITIAL_SPELLS`).
+ ///
+ /// **Ordered, and that is load-bearing** (decision 1312). The reference keeps its known spells
+ /// in an ARRAY and the scans that hunt it — the GameObject lock resolver `0x5f83d0` above all —
+ /// stop at the first hit, so the visit order picks *which* of several equally-qualified spells
+ /// wins. A `HashSet` made that pick nondeterministic: every character knows both 6478
+ /// "Opening" and 22810 "Opening - No Text" (both `LockType 13`, both trivially sufficient), and
+ /// whichever the hash happened to reach first went on the cast bar (B247). Ascending spell id
+ /// is the array's own order after login — the server sends the initial batch out of a
+ /// `std::map`, so the wire arrives sorted.
+ pub spells: BTreeSet,
/// Set on every book/bar arrival AND every local `action_sets` drain; cleared by the feed
/// after re-resolving each slot's identity (icon/kind/action) and pushing. It is only ONE of
/// the identity resolve's two triggers — a landed item template is the other (decision 0660,
@@ -226,8 +235,13 @@ fn track_learned_abilities(
if !actions.is_changed() && !spells.is_changed() {
return;
}
- let first_with = |effect: u32| {
- actions.spells.iter().copied().find(|&id| {
+ // The **last** matching spell in ascending-id order, not the first: the reference latches at
+ // learn time and each new rank overwrites the global, so what it holds is the newest rank
+ // learned — and a rank chain is ascending by id (Skinning 8613 → 8617 → 8618 → 10768). Which
+ // end we take is only visible when a chain's ranks are known together, but "arbitrary" was
+ // never an answer: the set was a `HashSet` until 1312 and this picked at the hash's whim.
+ let last_with = |effect: u32| {
+ actions.spells.iter().copied().rfind(|&id| {
spells
.catalog
.get(id)
@@ -235,9 +249,9 @@ fn track_learned_abilities(
})
};
let (skinning, skin_player_corpse, feed_pet) = (
- first_with(benilla_formats::SPELL_EFFECT_SKINNING),
- first_with(SPELL_EFFECT_SKIN_PLAYER_CORPSE),
- first_with(SPELL_EFFECT_FEED_PET),
+ last_with(benilla_formats::SPELL_EFFECT_SKINNING),
+ last_with(SPELL_EFFECT_SKIN_PLAYER_CORPSE),
+ last_with(SPELL_EFFECT_FEED_PET),
);
if (skinning, skin_player_corpse, feed_pet)
!= (
diff --git a/crates/benilla-app/src/ui_action/targeting/item.rs b/crates/benilla-app/src/ui_action/targeting/item.rs
index c6bd6fc2..2cb53342 100644
--- a/crates/benilla-app/src/ui_action/targeting/item.rs
+++ b/crates/benilla-app/src/ui_action/targeting/item.rs
@@ -260,7 +260,7 @@ pub(crate) fn commit_item_cast_on_pick(
"ui_action: cast {spell_id} refused at the item bind ({reason:#x}) — \
the cursor stays up"
);
- ladder.cast_errors.0.push((spell_id, reason));
+ ladder.cast_errors.push_local(spell_id, reason);
}
ItemBind::ConfirmBind => {
debug!("ui_action: item bind confirm for {spell_id} on item {item_guid:#x}");
diff --git a/crates/benilla-app/src/ui_cast.rs b/crates/benilla-app/src/ui_cast.rs
index 62796346..d573123e 100644
--- a/crates/benilla-app/src/ui_cast.rs
+++ b/crates/benilla-app/src/ui_cast.rs
@@ -518,6 +518,30 @@ fn local_self_cancel(
}
}
+/// The casting bar's **label** for a spell — the `name` argument of `SPELLCAST_START` /
+/// `SPELLCAST_CHANNEL_START`, which the reference `CastingBarFrame` puts straight into
+/// `CastingBarText:SetText(...)`.
+///
+/// Almost always the spell's `Spell.dbc` name, with two ways to come back empty:
+///
+/// - **`AttributesEx3 & 0x4`** (`SPELL_ATTR_EX3_NO_CASTING_BAR_TEXT`,
+/// [`SpellDisplay::no_casting_bar_text`]) — the attribute that exists precisely so an internal
+/// spell with no player-facing name never puts one on the bar. Three shipped rows carry it, and
+/// one of them is *named after the bit*: **22810 "Opening - No Text"**, the opener the client
+/// casts at `LockType 13` ground containers. Printing the name unconditionally is what put that
+/// placeholder over a gathered Hyacinth Mushroom (B247, decision 1312).
+/// - **no catalog row** (an unknown id, or no client data) — the empty string this always
+/// returned in that case.
+///
+/// The bar still shows, fills and sweeps either way; only the text is suppressed.
+fn cast_bar_label(spells: Option<&crate::ui_action::Spells>, id: u32) -> String {
+ spells
+ .and_then(|s| s.catalog.get(id))
+ .filter(|d| !d.no_casting_bar_text())
+ .map(|d| d.name.clone())
+ .unwrap_or_default()
+}
+
/// Drain the queue into FrameScript events — the reference `CastingBarFrame` contract
/// (extracted 1.12 `CastingBarFrame.lua`): `SPELLCAST_START(name, ms)`,
/// `SPELLCAST_CHANNEL_START(ms, name)`, `SPELLCAST_CHANNEL_UPDATE(remaining_ms)`, and the
@@ -561,13 +585,7 @@ fn feed_cast_bar(
script.set_casting(
auto_repeat.0.is_some() || inflight(&pending, &queued_melee, started, now).is_some(),
);
- let name = |id: u32| -> String {
- spells
- .as_ref()
- .and_then(|s| s.catalog.get(id))
- .map(|d| d.name.clone())
- .unwrap_or_default()
- };
+ let name = |id: u32| cast_bar_label(spells.as_deref(), id);
for edge in feed.0.drain(..) {
let (event, args): (&str, Vec) = match edge {
CastBarEdge::Start {
@@ -636,6 +654,39 @@ mod tests {
const FIREBALL: u32 = 133;
+ /// **B247** (decision 1312): the bar's label honours `SPELL_ATTR_EX3_NO_CASTING_BAR_TEXT`.
+ /// Spell 22810's Spell.dbc name — "Opening - No Text" — is Blizzard's own annotation of the
+ /// attribute, and it reached a player's screen because we printed the name regardless.
+ #[test]
+ fn the_no_casting_bar_text_attribute_blanks_the_label() {
+ use benilla_formats::{SpellCatalog, SpellDisplay};
+ let named = |name: &str, attributes_ex3: u32| SpellDisplay {
+ name: name.to_string(),
+ attributes_ex3,
+ ..Default::default()
+ };
+ let spells = crate::ui_action::Spells {
+ catalog: SpellCatalog::from_displays(
+ [
+ (6478, named("Opening", 0)),
+ (22810, named("Opening - No Text", 0x4)),
+ ]
+ .into_iter()
+ .collect(),
+ ),
+ ..crate::ui_action::Spells::empty_for_tests()
+ };
+ assert_eq!(cast_bar_label(Some(&spells), 6478), "Opening");
+ assert_eq!(
+ cast_bar_label(Some(&spells), 22810),
+ "",
+ "the bar fills and sweeps, but shows no name"
+ );
+ // The pre-existing empty cases are untouched: an unknown id, and no client data at all.
+ assert_eq!(cast_bar_label(Some(&spells), 133), "");
+ assert_eq!(cast_bar_label(None, 6478), "");
+ }
+
#[test]
fn a_fresh_guard_is_open() {
let g = PendingCast::default();
diff --git a/crates/benilla-app/src/ui_script/bag_tests.rs b/crates/benilla-app/src/ui_script/bag_tests.rs
index 26eea7f7..d21b6e40 100644
--- a/crates/benilla-app/src/ui_script/bag_tests.rs
+++ b/crates/benilla-app/src/ui_script/bag_tests.rs
@@ -1006,10 +1006,9 @@ fn shift_click_on_a_stack_opens_the_split_frame() {
/// B180 — the split dialog's parchment plate fills the whole 172×96 frame. The reference authors
/// the plate 256×32 with NO anchors, a vestigial size the real client never renders (the TexCoords
/// crop exactly 172×96 out of the 256×128 art — the frame's own size, a complete panel drawn 1:1
-/// over it). Our sized-anchor-less fallback draws CENTERED at the authored size instead, which
-/// squashed the dialog to a 32-tall bar: the count and arrows floated above it and Okay/Cancel
-/// straddled its bottom edge. The plate's authored size is dropped (StackSplit.xml says why); this
-/// pins the render so the vestigial size cannot come back.
+/// over it). 1308 first dodged this by dropping the size; 1310 then landed the byte-verified law
+/// (an anchor-less region gets an implicit SetAllPoints at creation, size unread under the two
+/// corners) and restored the ref's own text — this pins the render through the real mechanism.
#[test]
fn the_split_frame_plate_fills_the_dialog() {
let mut s = UiScript::new().unwrap();
@@ -1057,6 +1056,132 @@ fn the_split_frame_plate_fills_the_dialog() {
assert!(s.errors().is_empty(), "script errors: {:?}", s.errors());
}
+/// B180's Bagnon follow-on (director, 08-14): the dialog opened UNDER an addon bag window —
+/// Bagnon's windows are `frameStrata="HIGH"`, the dialog's own stratum, and our re-expression had
+/// dropped the reference's `toplevel="true"`, so the dialog's Show raised nothing and lost the
+/// level tie to the later-shown window (its child buttons at level+1 poked through; the plate did
+/// not). With the ref's attrs restored, Show runs the verified raise (toplevel.rs: compact, then
+/// top-occupied-plus-one) and the whole dialog lands above. The synthetic window stands in for
+/// Bagnon: same stratum, shown after load, overlapping the dialog.
+#[test]
+fn the_split_frame_raises_over_a_same_stratum_window() {
+ let mut s = UiScript::new().unwrap();
+ s.set_screen_size(1024.0, 768.0);
+ let (x, y) = open_backpack_with_a_five_stack(&mut s);
+
+ s.run(
+ r#"
+ local w = CreateFrame("Frame", "FakeBagnon", UIParent)
+ w:SetFrameStrata("HIGH")
+ w:SetPoint("BOTTOMLEFT", 0, 0)
+ w:SetSize(1024, 768)
+ local bg = w:CreateTexture(nil, "BACKGROUND")
+ bg:SetTexture("Interface\\FakeBagnonBG")
+ bg:SetAllPoints()
+ w:Show()
+ "#,
+ )
+ .unwrap();
+ s.resolve();
+
+ s.set_modifiers(true, false, false);
+ s.mouse_button(x, y, "LeftButton", true);
+ s.mouse_button(x, y, "LeftButton", false);
+ s.set_modifiers(false, false, false);
+ s.resolve();
+
+ assert!(
+ s.eval::("return StackSplitFrame:IsShown()").unwrap(),
+ "the spinner opened"
+ );
+ let (dialog, bagnon) = s
+ .eval::<(i64, i64)>("return StackSplitFrame:GetFrameLevel(), FakeBagnon:GetFrameLevel()")
+ .unwrap();
+ assert!(
+ dialog > bagnon,
+ "Show must raise the toplevel dialog over the same-stratum window \
+ (dialog level {dialog}, window level {bagnon})"
+ );
+ // The symptom itself: the plate paints AFTER the window's background in draw order.
+ let order: Vec = s
+ .extract()
+ .iter()
+ .filter_map(|q| match &q.content {
+ QuadContent::Texture { path: Some(p), .. }
+ if p.contains("FakeBagnonBG") || p.contains("UI-MoneyFrame") =>
+ {
+ Some(p.clone())
+ }
+ _ => None,
+ })
+ .collect();
+ assert_eq!(order.len(), 2, "both textures on screen: {order:?}");
+ assert!(
+ order[0].contains("FakeBagnonBG"),
+ "the plate must draw over the window, not under it: {order:?}"
+ );
+ assert!(s.errors().is_empty(), "script errors: {:?}", s.errors());
+}
+
+/// Typed-digit entry in the split spinner (decision 1319) — the director's ask, and the deferral
+/// this file's header carried since 0216. Pins the whole chain in one go: the dialog is in the
+/// keyboard walk (`enableKeyboard`), a digit reaches its `OnChar`, the first digit REPLACES the
+/// seeded 1 while later digits append, an over-max entry clamps instead of being rejected,
+/// BACKSPACE drops a digit, and ENTER commits exactly as Okay does.
+#[test]
+fn typing_a_number_into_the_split_spinner_sets_the_count() {
+ let mut s = UiScript::new().unwrap();
+ s.set_screen_size(1024.0, 768.0);
+ let (x, y) = open_backpack_with_a_five_stack(&mut s);
+
+ s.set_modifiers(true, false, false);
+ s.mouse_button(x, y, "LeftButton", true);
+ s.mouse_button(x, y, "LeftButton", false);
+ s.set_modifiers(false, false, false);
+ s.resolve();
+ assert_eq!(
+ s.eval::("return StackSplitFrame.split").unwrap(),
+ 1,
+ "opens seeded at 1"
+ );
+
+ // A digit is consumed by the dialog — which is also what stops it firing action button 3.
+ assert!(s.char_input("3"), "the spinner consumed the digit");
+ assert_eq!(
+ s.eval::("return StackSplitFrame.split").unwrap(),
+ 3,
+ "the first digit REPLACES the seed rather than appending to it"
+ );
+ assert_eq!(
+ s.eval::("return StackSplitText:GetText()").unwrap(),
+ "3",
+ "and the label follows"
+ );
+
+ // A second digit appends, then clamps to the 5-stack rather than being thrown away.
+ assert!(s.char_input("7"));
+ assert_eq!(
+ s.eval::("return StackSplitFrame.split").unwrap(),
+ 5,
+ "37 against a 5-stack clamps to 5"
+ );
+ // BACKSPACE drops a digit off the clamped value.
+ assert!(s.frame_key_input("BACKSPACE"), "the dialog took BACKSPACE");
+ assert_eq!(s.eval::("return StackSplitFrame.split").unwrap(), 1);
+
+ // Type a real value and commit with ENTER — the Okay path, so the carry is picked up.
+ assert!(s.char_input("4"));
+ assert_eq!(s.eval::("return StackSplitFrame.split").unwrap(), 4);
+ assert!(s.key_input("ENTER"), "ENTER is consumed by the dialog");
+ assert!(
+ !s.eval::("return StackSplitFrame:IsShown()").unwrap(),
+ "ENTER commits and closes, like Okay"
+ );
+ let held = s.cursor_item().expect("ENTER committed the split");
+ assert_eq!(held.count, Some(4), "the typed count is what got picked up");
+ assert!(s.errors().is_empty(), "script errors: {:?}", s.errors());
+}
+
/// Okay in the split spinner only picks the split carry up (ref/cursor.rs `SplitContainerItem` —
/// a pickup, not a self-contained move); a SUBSEQUENT placement is what actually queues the
/// `ContainerMove` with `count: Some(n)`, drained the same way any other container move is.
diff --git a/crates/benilla-app/src/ui_script/input.rs b/crates/benilla-app/src/ui_script/input.rs
index fee8054d..32d86774 100644
--- a/crates/benilla-app/src/ui_script/input.rs
+++ b/crates/benilla-app/src/ui_script/input.rs
@@ -205,6 +205,30 @@ pub(super) fn feed_ui_input(
// route law decides; unfocused they return unconsumed, and the camera/turn keys that
// read arrows after UiInput see them exactly as before).
let chord = keymap::chord(ev.key_code, mods, mac);
+ // A KEYBOARD FRAME gets these by NAME before their chord runs (decision 1319). They are
+ // the keys a focused EditBox receives as a semantic `EditAction` instead of a name, so
+ // they never reached `key_input` at all — and a dialog you type into needs its BACKSPACE.
+ // `frame_key_input` walks in the engine's own order and declines at a focused box, so the
+ // box's editing keys can never be stolen by a frame below it; a `true` here means a frame
+ // consumed the key, which suppresses BOTH the chord below and the key's binding (the
+ // reference's existence gate, wow-re `frame-key-script-delivery.md` §3).
+ let frame_named = match ev.key_code {
+ KeyCode::Backspace => Some("BACKSPACE"),
+ KeyCode::Delete => Some("DELETE"),
+ KeyCode::ArrowLeft => Some("LEFT"),
+ KeyCode::ArrowRight => Some("RIGHT"),
+ KeyCode::ArrowUp => Some("UP"),
+ KeyCode::ArrowDown => Some("DOWN"),
+ KeyCode::Home => Some("HOME"),
+ KeyCode::End => Some("END"),
+ _ => None,
+ };
+ if let Some(name) = frame_named {
+ if script.frame_key_input(name) {
+ capture.0 = true; // consumed by a frame: its binding must not also fire
+ continue;
+ }
+ }
if let Some(name) = named {
// The three box-event keys. An unconsumed press is not acted on here: every GAME
// action of a key — ESCAPE's close/cancel ladder (TOGGLEGAMEMENU), TAB's targeting,
@@ -212,7 +236,12 @@ pub(super) fn feed_ui_input(
// dispatch (decision 0997, `crate::bindings`), which runs right after this pass and
// reads the capture gate written above, so a key a focused box consumed this frame
// never also fires its binding — the real client's ESC precedence, table-wide.
- script.key_input(name);
+ // Consumption suppresses the binding — a keyboard FRAME that took this key counts
+ // exactly as a focused box does (decision 1319; `capture.0` above already covers the
+ // box case, and this adds the frame one).
+ if script.key_input(name) {
+ capture.0 = true;
+ }
} else if let Some(chord) = chord {
match chord {
keymap::Chord::Edit(action) => {
@@ -244,7 +273,12 @@ pub(super) fn feed_ui_input(
// plane European layouts type real text with (and macOS Option+letter comes through
// with only `alt`, composing its special characters).
if let Some(text) = &ev.text {
- script.char_input(text);
+ // Same suppression as the named keys, and this is the case that bites: the number
+ // row IS a binding (the action buttons), so a digit typed into a keyboard frame —
+ // the stack-split spinner — must not also fire action button 3 (decision 1319).
+ if script.char_input(text) {
+ capture.0 = true;
+ }
}
}
}
diff --git a/crates/benilla-app/src/ui_script/pet_bar_tests.rs b/crates/benilla-app/src/ui_script/pet_bar_tests.rs
index 50b06b28..dbed812c 100644
--- a/crates/benilla-app/src/ui_script/pet_bar_tests.rs
+++ b/crates/benilla-app/src/ui_script/pet_bar_tests.rs
@@ -184,10 +184,23 @@ fn the_shipped_pet_bar_drives_end_to_end() {
1,
"only Claw can autocast"
);
+ // 4 emitters x the DERIVED trail length, on the one autocasting slot — and on that slot only,
+ // which is the lazy pool's own pin: the other nine buttons have no sparks at all, where they
+ // used to carry a full hidden set each.
+ let trail: usize = s.eval("return BENILLA_PET_AUTOCAST_TRAIL").unwrap();
+ assert_eq!(
+ trail, 23,
+ "the size ramp's own spacing (see PetActionBar.xml's header) comes out at 23 samples"
+ );
assert_eq!(
textures(&quads, "Interface\\Buttons\\GlowStar"),
- 32,
- "4 emitters x 8 trail stars, on the one autocasting slot"
+ 4 * trail,
+ "4 emitters x the trail, on the one autocasting slot"
+ );
+ assert!(
+ s.eval::("return PetActionButton1.sparks == nil")
+ .unwrap(),
+ "a button that never autocasts never pools a spark"
);
// Geometry, quoted from the ref: the bar's TOPLEFT is MainMenuBar's BOTTOMLEFT +(36,97),
@@ -631,3 +644,282 @@ fn the_keybind_pair_pushes_and_casts_without_the_clicks_forks() {
);
assert!(s.errors().is_empty(), "script errors: {:?}", s.errors());
}
+
+/// **B228 — the autocast trail's motion, MEASURED off the shipped handler.**
+///
+/// nazriel reads the shine as "a bit too fast". The health pass answered that from the constants —
+/// the .m2 authors a 2.000 s loop and `BENILLA_PET_AUTOCAST_PERIOD` is 2.0 — which is a read, not a
+/// measurement: it says nothing about whether the OnUpdate wiring turns wall-clock seconds into
+/// that period, and an `elapsed` in the wrong unit would be invisible to it. So this drives the
+/// real `` with real seconds and reads the head spark's own SetPoint back.
+///
+/// What is pinned, all of it off the file (`m2bones` on `Interface\Buttons\UI-AutoCastButton.m2`):
+/// a 28.8 px square (0.02 units × 1200 px/unit × the ref Model's 1.2), one lap per 2.000 s, a
+/// corner every 0.500 s, LINEAR between them (`interp == 1` at the bone record's translation
+/// M2Track — so the half-edge sample sits at the half-time, no easing), and the four emitters one
+/// quarter-lap apart. If the reported overspeed is in the period, it fails here; if this holds and
+/// the report stands, the period is exonerated and the gap is in the trail's SHAPE.
+#[test]
+fn the_autocast_trail_laps_the_rim_once_every_two_seconds() {
+ let mut s = UiScript::new().unwrap();
+ s.set_screen_size(1024.0, 768.0);
+ load_pet_bar(&s);
+ declare_token_strings(&s);
+ s.set_pet_actions(true, true, true, hunter_slots());
+ s.fire_event("PET_BAR_UPDATE", vec![]);
+ s.resolve();
+
+ // Slot 4 is Claw, the one slot in the fixture with autocast RUNNING. `sparks[n]` is the pool
+ // `BenillaPetAutocast_Create` built: emitter-major, so emitter e's HEAD (age 0, the bone's own
+ // position) is index e·TRAIL + 1.
+ let head = |s: &UiScript, emitter: usize| -> (f32, f32) {
+ let expr = |axis: usize| {
+ format!(
+ "local n = {emitter} * BENILLA_PET_AUTOCAST_TRAIL + 1 \
+ local a, b, c, x, y = PetActionButton4.sparks[n]:GetPoint() \
+ return ({axis} == 0) and x or y"
+ )
+ };
+ (
+ s.eval::(&expr(0)).unwrap(),
+ s.eval::(&expr(1)).unwrap(),
+ )
+ };
+ let near = |got: (f32, f32), want: (f32, f32), what: &str| {
+ assert!(
+ (got.0 - want.0).abs() < 0.01 && (got.1 - want.1).abs() < 0.01,
+ "{what}: at {got:?}, expected {want:?}"
+ );
+ };
+
+ // 0.02 units × 1200 px/unit × 1.2 = 28.8 px across, so the corners sit ±14.4 from centre.
+ const H: f32 = 14.4;
+ let (bl, tl, tr, br) = ((-H, -H), (-H, H), (H, H), (H, -H));
+
+ // ── The clock WIRING: four half-second ticks of the shipped OnUpdate, one corner each. This
+ // is the half a constants read cannot reach — `arg1` really is seconds, and the bar's clock
+ // advances 1:1 with it.
+ let corner = |s: &mut UiScript, dt: f32, want: (f32, f32), what: &str| {
+ s.tick(dt);
+ near(head(s, 0), want, what);
+ };
+ corner(
+ &mut s,
+ 0.5,
+ tl,
+ "0.5 s in: one edge, at the top-left corner",
+ );
+ corner(&mut s, 0.5, tr, "1.0 s: half a lap");
+ corner(&mut s, 0.5, br, "1.5 s: three quarters");
+ corner(
+ &mut s,
+ 0.5,
+ bl,
+ "2.0 s: exactly one lap, back where it started",
+ );
+
+ // ── The position LAW, driven at exact clocks so no tick accumulation can blur it. The
+ // mid-edge sample is the `interp == 1` pin: linear means the half-time is the half-edge, and
+ // a spline-eased track would sit short of it.
+ let at = |s: &UiScript, clock: f32| -> (f32, f32) {
+ s.run(&format!(
+ "BenillaPetAutocast_Update(PetActionButton4, {clock})"
+ ))
+ .unwrap();
+ head(s, 0)
+ };
+ near(at(&s, 0.0), bl, "clock 0");
+ near(
+ at(&s, 0.25),
+ (-H, 0.0),
+ "quarter edge in: the LEFT edge's midpoint",
+ );
+ near(at(&s, 0.5), tl, "clock 0.5");
+ near(at(&s, 1.0), tr, "clock 1.0");
+ near(at(&s, 1.5), br, "clock 1.5");
+ near(at(&s, 2.0), bl, "clock 2.0 wraps onto clock 0");
+
+ // ── The trail is CONTINUOUS, which is what deriving the sample count buys (B228). No two
+ // neighbours in a trail sit further apart than the smaller of their two stars is wide, so the
+ // streak has no seam at any age. The 8-even-samples version failed this from mid-life back —
+ // 7.2 px apart with 4.3 px stars — and a row of separated dots is what reads as marching.
+ let worst_gap: f32 = s
+ .eval(
+ "local worst = 0 \
+ for k = 1, BENILLA_PET_AUTOCAST_TRAIL - 1 do \
+ local a, b = PetActionButton4.sparks[k], PetActionButton4.sparks[k + 1] \
+ local _, _, _, ax, ay = a:GetPoint() \
+ local _, _, _, bx, by = b:GetPoint() \
+ local d = math.sqrt((ax - bx) * (ax - bx) + (ay - by) * (ay - by)) \
+ local w = math.min(a:GetWidth(), b:GetWidth()) \
+ if d / w > worst then worst = d / w end \
+ end \
+ return worst",
+ )
+ .unwrap();
+ assert!(
+ worst_gap <= 1.0,
+ "the widest neighbour gap is {worst_gap:.2} star-widths — the streak has a seam"
+ );
+
+ // ── The four emitters are ONE LAP, a quarter apart — the file's four bones at the four
+ // corners, chasing each other (m2bones: pivots at (-0.001,0), (0.019,0), (0.019,0.02),
+ // (-0.001,0.02), same track, one corner of phase between neighbours).
+ s.run("BenillaPetAutocast_Update(PetActionButton4, 0.0)")
+ .unwrap();
+ for (e, want, name) in [
+ (0, bl, "emitter 0"),
+ (1, tl, "emitter 1"),
+ (2, tr, "emitter 2"),
+ (3, br, "emitter 3"),
+ ] {
+ near(head(&s, e), want, name);
+ }
+ assert!(s.errors().is_empty(), "script errors: {:?}", s.errors());
+}
+
+/// The pet bar plus the three files its TOOLTIP needs, in the TOC's own order: Fonts.xml (the
+/// colour codes that wrap the binding), GameTooltip.xml (the plate), and UIParent.xml — already a
+/// prerequisite for the managed stack — for `GetBindingText`, which renders the key.
+fn load_pet_bar_with_tooltip(s: &UiScript) {
+ for file in [
+ "Fonts.xml",
+ "UiPanels.xml",
+ "UIParent.xml",
+ "GameTooltip.xml",
+ "Cooldown.xml",
+ "ActionBar.xml",
+ "PetActionBar.xml",
+ ] {
+ load_xml(s, file);
+ }
+}
+
+/// A hunter bar hovered through the REAL gesture, with the real binding registry and the real CVar
+/// table behind it — `mouse_move` runs the shipped `` with `this` bound, which is the only
+/// way this fork gets exercised the way nazriel exercised it.
+///
+/// Buttons are 30 px chained +8 from the bar's own origin: button 1 spans x[72,102] y[56,86] ⇒
+/// centre (87,71), button 4's left = 72 + 3·38 ⇒ centre (201,71) — the geometry
+/// `clicks_route_through_the_attack_toggle_fork` already leans on.
+const ATTACK_BUTTON: (f32, f32) = (87.0, 71.0);
+const CLAW_BUTTON: (f32, f32) = (201.0, 71.0);
+
+fn hovered_pet_bar() -> UiScript {
+ let mut s = UiScript::new().unwrap();
+ s.set_screen_size(1024.0, 768.0);
+ // The real command set and the real registered defaults, the way the app's own seed installs
+ // them — so `GetBindingKey("BONUSACTIONBUTTON1")` answers CTRL-1 (its byte-real default) and
+ // `GetCVar("UberTooltips")` answers "1" (its byte-read registrar value) rather than nil.
+ s.register_bindings(&crate::bindings::registry_commands());
+ s.register_cvars(crate::cvars::REGISTERED.iter().copied());
+ load_pet_bar_with_tooltip(&s);
+ declare_token_strings(&s);
+ s.set_pet_actions(true, true, true, hunter_slots());
+ s.fire_event("PET_BAR_UPDATE", vec![]);
+ s.resolve();
+ s
+}
+
+fn tooltip_line1(s: &UiScript) -> String {
+ s.eval::("return GameTooltipTextLeft1:GetText() or \"\"")
+ .unwrap()
+}
+
+/// **B230 — Attack/Follow/Stay name their keybinding; the pet's own spells do not.**
+///
+/// nazriel: *"On 1.12 when you hover over Attack, Follow, Stay pet action buttons they show the
+/// keybinding - which is not the case with Benilla"*. The reference agrees with him and is
+/// narrower than "the pet bar": `PetActionButton_OnEnter` (PetActionBarFrame.lua l.285-305) forks
+/// on `isToken or UberTooltips == "0"`, and only THAT branch concatenates
+/// `NORMAL_FONT_COLOR_CODE.." ("..GetBindingText(GetBindingKey("BONUSACTIONBUTTON"..id), "KEY_")..")"`.
+/// The other branch is a bare `SetPetAction` and appends nothing — so a fix that suffixed every
+/// slot would be wrong, and the control below is what says ours doesn't.
+///
+/// The exact string is pinned, colour codes and all, because the ref's concatenation puts the
+/// colour BEFORE the space (`Attack|cffffd200 (CTRL-1)|r`, not `Attack |cffffd200(CTRL-1)|r` —
+/// which is what MicroMenu.xml's different-shaped version of the same idea produces).
+#[test]
+fn token_tooltips_name_their_binding_and_pet_spells_do_not() {
+ let mut s = hovered_pet_bar();
+
+ s.mouse_move(ATTACK_BUTTON.0, ATTACK_BUTTON.1);
+ s.resolve();
+ assert_eq!(
+ tooltip_line1(&s),
+ "Attack|cffffd200 (CTRL-1)|r",
+ "a command token carries BONUSACTIONBUTTON1's key in the normal-font colour"
+ );
+ // Uber on + a token ⇒ still the default corner; only the CVar-off leg owner-anchors.
+ assert!(
+ s.eval::("return GameTooltip.default ~= nil").unwrap(),
+ "a token's plate takes the default corner while UberTooltips is on"
+ );
+
+ // The CONTROL: slot 4 is Claw, a real pet spell, and it goes through the engine channel —
+ // its name, and nothing in parentheses.
+ s.mouse_move(CLAW_BUTTON.0, CLAW_BUTTON.1);
+ s.resolve();
+ let claw = tooltip_line1(&s);
+ assert_eq!(claw, "Claw", "a pet SPELL renders through SetPetAction");
+ assert!(
+ !claw.contains('('),
+ "…and appends no binding: {claw:?} (the ref's other branch)"
+ );
+ assert!(s.errors().is_empty(), "script errors: {:?}", s.errors());
+}
+
+/// The append is UNCONDITIONAL in that branch, so an unbound row really does render empty
+/// parentheses — `GetBindingText` answers `""` for a nil key (UIParent.xml, transcribed from the
+/// ref's own l.1819-1821), and nothing guards the `"("`. That is the reference's behaviour, not a
+/// slip of ours: MicroButtonTooltipText guards its own suffix and this one does not.
+///
+/// Stock it is unreachable — BONUSACTIONBUTTON1-10 default to CTRL-1..CTRL-0 — so it takes a
+/// deliberate unbind to see, which is exactly what this does.
+#[test]
+fn an_unbound_token_row_renders_the_references_empty_parentheses() {
+ let mut s = hovered_pet_bar();
+ s.run(r#"SetBinding("CTRL-1")"#).unwrap();
+ assert!(
+ s.eval::>(r#"return GetBindingKey("BONUSACTIONBUTTON1")"#)
+ .unwrap()
+ .is_none(),
+ "the row really is unbound now"
+ );
+
+ s.mouse_move(ATTACK_BUTTON.0, ATTACK_BUTTON.1);
+ s.resolve();
+ assert_eq!(
+ tooltip_line1(&s),
+ "Attack|cffffd200 ()|r",
+ "unbound: the parentheses stay, empty — the ref's own unguarded concatenation"
+ );
+ assert!(s.errors().is_empty(), "script errors: {:?}", s.errors());
+}
+
+/// The CVar is the fork's OTHER half, and it is not decoration: with `UberTooltips` off, a real pet
+/// SPELL stops going through the engine channel and takes the token branch too — same built text,
+/// same binding suffix — and the plate moves from the screen corner to beside the button
+/// (`SetOwner(this, "ANCHOR_RIGHT")`). Both are the reference's, in the same five lines.
+///
+/// This is also the row's reason to exist in `cvars::REGISTERED`: benilla registers `UberTooltips`
+/// at all because these Lua sites read it (the honest-tree rule's live-consumer half).
+#[test]
+fn ubertooltips_off_takes_the_spells_through_the_token_branch_and_moves_the_plate() {
+ let mut s = hovered_pet_bar();
+ s.set_cvar_engine("UberTooltips", "0");
+
+ s.mouse_move(CLAW_BUTTON.0, CLAW_BUTTON.1);
+ s.resolve();
+ assert_eq!(
+ tooltip_line1(&s),
+ "Claw|cffffd200 (CTRL-4)|r",
+ "with the CVar off a pet spell is built here too, binding and all"
+ );
+ assert!(
+ s.eval::("return GameTooltip:IsOwned(PetActionButton4)")
+ .unwrap(),
+ "…and the plate sits beside the button, not at the screen corner"
+ );
+ assert!(s.errors().is_empty(), "script errors: {:?}", s.errors());
+}
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 c641a9f0..26a1eaad 100644
--- a/crates/benilla-app/src/ui_script/tooltip_anchor_tests.rs
+++ b/crates/benilla-app/src/ui_script/tooltip_anchor_tests.rs
@@ -192,11 +192,16 @@ fn your_own_portrait_explains_the_menu_instead_of_showing_your_health() {
/// Action-bar hovers take the default corner too — ref ActionButton_SetTooltip l.366-372
/// branches on the UberTooltips CVar, whose stock default is "1" (byte-read from WoW.exe
-/// 0x48fdd9 / default string 0x82e748; see ActionBar.xml). An empty slot renders nothing,
+/// 0x48fdd9 / default string 0x82e748; see `cvars::REGISTERED`). An empty slot renders nothing,
/// but the anchor must already be seated — asserted through GetPoint, resolved rect or not.
+///
+/// Registered, not merely absent: before B230 the CVar was not in the table at all, so `GetCVar`
+/// answered nil and the "1" leg was reached by accident rather than by value. This seeds the real
+/// table so the pass means what it says.
#[test]
fn action_button_hover_takes_the_default_corner() {
- let s = harness(&["Cooldown.xml", "ActionBar.xml"]);
+ let mut s = harness(&["Cooldown.xml", "ActionBar.xml"]);
+ s.register_cvars(crate::cvars::REGISTERED.iter().copied());
s.run("BenillaActionButton_OnEnter(ActionButton3)").unwrap();
assert!(s.errors().is_empty(), "hover errors: {:?}", s.errors());
let ok: bool = s
@@ -212,6 +217,105 @@ fn action_button_hover_takes_the_default_corner() {
);
}
+/// The other leg of that same branch, live since B230 registered the CVar: with `UberTooltips`
+/// off, an action button's plate leaves the screen corner and seats BESIDE the button — LEFT for
+/// the bars the reference lists (`MultiBarBottomRight` is the only one benilla ships; the two
+/// vertical bars are not), RIGHT for everything else, including the main bar.
+///
+/// The two anchors are read off the resolved SetPoint pair, which is what `SetOwner` actually
+/// writes: ANCHOR_RIGHT = the plate's BOTTOMLEFT on the button's TOPRIGHT, ANCHOR_LEFT its mirror
+/// (`script/tooltip/verbs.rs`).
+#[test]
+fn ubertooltips_off_seats_action_bar_plates_beside_the_button() {
+ let mut s = harness(&["Cooldown.xml", "ActionBar.xml", "MultiBars.xml"]);
+ s.register_cvars(crate::cvars::REGISTERED.iter().copied());
+ s.set_cvar_engine("UberTooltips", "0");
+
+ let seat = |s: &UiScript| {
+ s.eval::(
+ "local p, rel, rp = GameTooltip:GetPoint() \
+ return p .. \"/\" .. (rel and rel:GetName() or \"?\") .. \"/\" .. rp",
+ )
+ .unwrap()
+ };
+
+ s.run("BenillaActionButton_OnEnter(ActionButton3)").unwrap();
+ assert!(
+ s.eval::("return GameTooltip.default == nil").unwrap(),
+ "off: the main bar's plate is owner-anchored, not the default corner"
+ );
+ assert!(
+ s.eval::("return GameTooltip:IsOwned(ActionButton3)")
+ .unwrap(),
+ "…owned by the button it opened from"
+ );
+ assert_eq!(
+ seat(&s),
+ "BOTTOMLEFT/ActionButton3/TOPRIGHT",
+ "ANCHOR_RIGHT — the main bar is not in the ref's LEFT set"
+ );
+
+ // MultiBarBottomRight IS in that set — the one shipped member of it.
+ s.run("BenillaActionButton_OnEnter(MultiBarBottomRightButton1)")
+ .unwrap();
+ assert_eq!(
+ seat(&s),
+ "BOTTOMRIGHT/MultiBarBottomRightButton1/TOPLEFT",
+ "ANCHOR_LEFT — MultiBarBottomRight opens toward screen centre"
+ );
+
+ // And the CVar back on restores the corner — the fork is a fork, not a one-way door.
+ s.set_cvar_engine("UberTooltips", "1");
+ s.run("BenillaActionButton_OnEnter(MultiBarBottomRightButton1)")
+ .unwrap();
+ assert!(
+ s.eval::("return GameTooltip.default ~= nil").unwrap(),
+ "on: back to the screen corner"
+ );
+ assert!(s.errors().is_empty(), "hover errors: {:?}", s.errors());
+}
+
+/// The stance bar's own leg of the same branch (ref BonusActionBarFrame.xml l.40-45 — ANCHOR_RIGHT
+/// with no bar fork of its own), pinned separately because it is a different file's handler and a
+/// different tooltip verb; it was collapsed to the "1" leg alongside the action bar's and
+/// un-collapsed with it.
+#[test]
+fn ubertooltips_off_seats_stance_plates_beside_the_button() {
+ let mut s = harness(&["Cooldown.xml", "ActionBar.xml", "StanceBar.xml"]);
+ s.register_cvars(crate::cvars::REGISTERED.iter().copied());
+ s.set_shapeshift_forms(vec![benilla_ui::script::ShapeshiftFormView {
+ spell_id: 5487,
+ name: "Bear Form".into(),
+ texture: Some("Interface\\Icons\\Ability_Racial_BearForm".into()),
+ active: false,
+ castable: true,
+ cooldown: None,
+ }]);
+ s.fire_event("UPDATE_SHAPESHIFT_FORMS", vec![]);
+ s.resolve();
+
+ s.run("BenillaShapeshiftButton_OnEnter(ShapeshiftButton1)")
+ .unwrap();
+ assert!(
+ s.eval::("return GameTooltip.default ~= nil").unwrap(),
+ "on (the stock default): the screen corner"
+ );
+
+ s.set_cvar_engine("UberTooltips", "0");
+ s.run("BenillaShapeshiftButton_OnEnter(ShapeshiftButton1)")
+ .unwrap();
+ assert_eq!(
+ s.eval::(
+ "local p, rel, rp = GameTooltip:GetPoint() \
+ return p .. \"/\" .. (rel and rel:GetName() or \"?\") .. \"/\" .. rp",
+ )
+ .unwrap(),
+ "BOTTOMLEFT/ShapeshiftButton1/TOPRIGHT",
+ "off: ANCHOR_RIGHT, beside the button"
+ );
+ assert!(s.errors().is_empty(), "hover errors: {:?}", s.errors());
+}
+
/// Buff hovers hang BELOW the button — ref BuffFrame.xml l.37 is ANCHOR_BOTTOMLEFT (the buff
/// row lives at the screen's top-right): the tooltip's TOPRIGHT seats on the button's
/// BOTTOMLEFT.
diff --git a/crates/benilla-app/src/ui_spellbook.rs b/crates/benilla-app/src/ui_spellbook.rs
index a3aadfca..08096de3 100644
--- a/crates/benilla-app/src/ui_spellbook.rs
+++ b/crates/benilla-app/src/ui_spellbook.rs
@@ -31,7 +31,7 @@
//! and the rank tie-break falling to the rank *string* only when the parsed number is 0 (the
//! client's `SpellLevel` fallback for a digit-less rank is unmodeled — we have no `SpellLevel`).
-use std::collections::{BTreeMap, HashSet};
+use std::collections::{BTreeMap, BTreeSet};
use bevy::prelude::*;
@@ -231,7 +231,7 @@ fn feed_spellbook(
/// Build the whole book (module doc): the app's own resolve — the engine holds no spell
/// knowledge, only what's pushed here.
fn build_book(
- known: &HashSet,
+ known: &BTreeSet,
catalog: &SpellCatalog,
skill_lines: Option<&SkillLineCatalog>,
race: u8,
@@ -458,7 +458,7 @@ mod tests {
map.insert(668, spell("Language: Common", None, 0x80)); // DO_NOT_DISPLAY → hidden
map.insert(818, spell("Cooking", None, 0x20)); // IS_TRADESKILL → hidden
let catalog = SpellCatalog::from_displays(map);
- let known: HashSet = [133, 145, 2136, 668, 818].into_iter().collect();
+ let known: BTreeSet = [133, 145, 2136, 668, 818].into_iter().collect();
// No skill-line catalog → every shown spell lands in General (race/class irrelevant).
let book = build_book(&known, &catalog, None, 1, 1, None, None);
@@ -495,7 +495,7 @@ mod tests {
attack.icon = Some("Interface\\Icons\\Temp".into()); // the real DBC placeholder
attack.effects[0] = 78; // SPELL_EFFECT_ATTACK — makes it the melee auto-attack
let catalog = SpellCatalog::from_displays(HashMap::from([(ATTACK, attack)]));
- let known: HashSet = [ATTACK].into_iter().collect();
+ let known: BTreeSet = [ATTACK].into_iter().collect();
// The feed's resolved icon (armed → the weapon, unarmed → Spell-Reset) wins over Temp.
for resolved in [
@@ -531,7 +531,7 @@ mod tests {
throw.icon = Some("Interface\\Icons\\Ability_Throw".into());
let catalog =
SpellCatalog::from_displays(HashMap::from([(AUTO_SHOT, auto_shot), (THROW, throw)]));
- let known: HashSet = [AUTO_SHOT, THROW].into_iter().collect();
+ let known: BTreeSet = [AUTO_SHOT, THROW].into_iter().collect();
let bow = "Interface\\Icons\\INV_Weapon_Bow_02";
let book = build_book(&known, &catalog, None, 1, 1, None, Some(bow.into()));
diff --git a/crates/benilla-app/src/ui_talent.rs b/crates/benilla-app/src/ui_talent.rs
index 505f6ddc..ea155588 100644
--- a/crates/benilla-app/src/ui_talent.rs
+++ b/crates/benilla-app/src/ui_talent.rs
@@ -28,7 +28,7 @@
//! composed here on a cp2 rise: this feed owns the points diff, and the chat arc's own kinds
//! carry it as a SYSTEM line.
-use std::collections::HashSet;
+use std::collections::BTreeSet;
use bevy::prelude::*;
@@ -151,7 +151,7 @@ fn feed_talents(
}
/// One talent's current rank: the highest rank whose spell is known (module doc).
-fn rank_of(t: &Talent, known: &HashSet) -> u32 {
+fn rank_of(t: &Talent, known: &BTreeSet) -> u32 {
t.ranks
.iter()
.enumerate()
@@ -164,7 +164,7 @@ fn rank_of(t: &Talent, known: &HashSet) -> u32 {
/// Build the pushed snapshot — the app's whole resolve (module doc).
fn build_pages(
catalog: &TalentCatalog,
- known: &HashSet,
+ known: &BTreeSet,
spells: &Spells,
race: u8,
class: u8,
@@ -341,11 +341,11 @@ mod tests {
#[test]
fn rank_is_the_highest_known_rank_spell() {
let t = talent(1, 81, 0, 0, &[100, 101, 102]);
- let known: HashSet = [100, 101].into_iter().collect();
+ let known: BTreeSet = [100, 101].into_iter().collect();
assert_eq!(rank_of(&t, &known), 2);
- assert_eq!(rank_of(&t, &HashSet::new()), 0);
+ assert_eq!(rank_of(&t, &BTreeSet::new()), 0);
// A gap never happens on the live wire (learn-up-to), but the max() read stays honest.
- let holey: HashSet = [102].into_iter().collect();
+ let holey: BTreeSet = [102].into_iter().collect();
assert_eq!(rank_of(&t, &holey), 3);
}
}
diff --git a/crates/benilla-app/src/ui_trainer/mod.rs b/crates/benilla-app/src/ui_trainer/mod.rs
index f50411dd..bc46182b 100644
--- a/crates/benilla-app/src/ui_trainer/mod.rs
+++ b/crates/benilla-app/src/ui_trainer/mod.rs
@@ -22,7 +22,7 @@
//! when the player walks out of the trainer's service range (or the trainer despawns) — the same
//! `CloseTrainer` clear.
-use std::collections::HashSet;
+use std::collections::BTreeSet;
use benilla_formats::{SkillLineCatalog, SpellCatalog};
use benilla_protocol::messages::TrainerSpell;
@@ -157,7 +157,7 @@ fn resolve_service(
trainer_type: u32,
spells: &SpellCatalog,
skill_lines: Option<&SkillLineCatalog>,
- known: &HashSet,
+ known: &BTreeSet,
icons: Option<&ItemDisplays>,
items: &mut Items,
commands: &NetCommands,
@@ -269,7 +269,7 @@ fn snapshot(
open: &TrainerOpen,
spells: &SpellCatalog,
skill_lines: Option<&SkillLineCatalog>,
- known: &HashSet,
+ known: &BTreeSet,
icons: Option<&ItemDisplays>,
items: &mut Items,
commands: &NetCommands,
diff --git a/crates/benilla-app/src/ui_trainer/tests.rs b/crates/benilla-app/src/ui_trainer/tests.rs
index 58854d7e..0c9cd04a 100644
--- a/crates/benilla-app/src/ui_trainer/tests.rs
+++ b/crates/benilla-app/src/ui_trainer/tests.rs
@@ -26,7 +26,7 @@ fn resolve_service(
trainer_type: u32,
spells: &SpellCatalog,
skill_lines: Option<&SkillLineCatalog>,
- known: &HashSet,
+ known: &BTreeSet,
) -> TrainerService {
let mut deps = Deps::new();
super::resolve_service(
@@ -62,7 +62,7 @@ fn snap(open: &TrainerOpen, spells: &SpellCatalog) -> Option {
open,
spells,
None,
- &HashSet::new(),
+ &BTreeSet::new(),
None,
&mut deps.items,
&deps.commands,
@@ -306,7 +306,7 @@ fn resolve_hops_the_learn_wrapper_to_the_taught_ability() {
0,
&spells,
Some(&skills),
- &HashSet::new(),
+ &BTreeSet::new(),
);
assert_eq!(svc.spell_id, 1605, "the buy id stays the wire wrapper");
assert_eq!(
@@ -340,7 +340,7 @@ fn display_name_is_the_wire_spell_not_the_taught_one() {
TRAINER_TYPE_TRADESKILL,
&spells,
Some(&skills),
- &HashSet::new(),
+ &BTreeSet::new(),
);
assert_eq!(svc.name.as_deref(), Some("Apprentice Blacksmith"));
assert_eq!(svc.subtext.as_deref(), None);
@@ -359,7 +359,7 @@ fn display_name_is_the_wire_spell_not_the_taught_one() {
TRAINER_TYPE_TRADESKILL,
&spells,
Some(&skills),
- &HashSet::new(),
+ &BTreeSet::new(),
);
assert_eq!(recipe.group_key, 2);
assert_eq!(recipe.group_name, "Recipes");
@@ -392,7 +392,7 @@ fn resolve_reads_cost_state_and_gates_with_no_catalog() {
let mut w = wire(2018, trainer_spell_state::RED, 1000, 20, 164);
w.req_spells = [78, 0, 0];
// Empty known set: the player knows nothing → the ability gate reads unmet on its own terms.
- let svc = resolve_service(&w, TRAINER_TYPE_TRADESKILL, &spells, None, &HashSet::new());
+ let svc = resolve_service(&w, TRAINER_TYPE_TRADESKILL, &spells, None, &BTreeSet::new());
assert_eq!(svc.spell_id, 2018);
assert!(svc.name.is_none(), "no catalog → name in flight");
assert_eq!(svc.cost, 1000);
@@ -429,7 +429,7 @@ fn resolve_reads_cost_state_and_gates_with_no_catalog() {
0,
&spells,
None,
- &HashSet::new(),
+ &BTreeSet::new(),
);
assert_eq!(plain.skill_req, None);
assert!(plain.ability_reqs.is_empty());
@@ -447,12 +447,12 @@ fn ability_req_met_tracks_known_spells_not_the_service_category() {
w.req_spells = [78, 0, 0]; // requires Heroic Strike (78)
// Player doesn't know 78 → the prerequisite is unmet (red).
- let unknown = resolve_service(&w, 0, &spells, None, &HashSet::new());
+ let unknown = resolve_service(&w, 0, &spells, None, &BTreeSet::new());
assert_eq!(unknown.category, TrainerServiceCategory::Unavailable);
assert!(!unknown.ability_reqs[0].met, "prereq unknown → unmet");
// Player knows 78 → the prerequisite is met (white) EVEN THOUGH the service stays unavailable.
- let known: HashSet = [78].into_iter().collect();
+ let known: BTreeSet = [78].into_iter().collect();
let learned = resolve_service(&w, 0, &spells, None, &known);
assert_eq!(learned.category, TrainerServiceCategory::Unavailable);
assert!(
@@ -473,7 +473,7 @@ fn ability_req_shows_the_required_rank_on_real_data() {
let mut w = wire(846, trainer_spell_state::RED, 100, 20, 0);
w.req_spells = [78, 0, 0]; // Heroic Strike Rank 1
- let known: HashSet = [78].into_iter().collect();
+ let known: BTreeSet = [78].into_iter().collect();
let svc = resolve_service(&w, 0, &spells, None, &known);
assert_eq!(
svc.ability_reqs,
@@ -484,7 +484,7 @@ fn ability_req_shows_the_required_rank_on_real_data() {
"the prereq shows its rank and reads met because the player knows it"
);
// Not known → same name, but unmet (red).
- let svc = resolve_service(&w, 0, &spells, None, &HashSet::new());
+ let svc = resolve_service(&w, 0, &spells, None, &BTreeSet::new());
assert!(!svc.ability_reqs[0].met);
assert_eq!(svc.ability_reqs[0].name, "Heroic Strike (Rank 1)");
}
diff --git a/crates/benilla-formats/src/bin/benilla-extract/m2dump.rs b/crates/benilla-formats/src/bin/benilla-extract/m2dump.rs
index cef8c0bd..da87b534 100644
--- a/crates/benilla-formats/src/bin/benilla-extract/m2dump.rs
+++ b/crates/benilla-formats/src/bin/benilla-extract/m2dump.rs
@@ -1347,13 +1347,23 @@ pub fn m2part(chain: &mut Chain, internal_path: &str) -> Result<()> {
println!(" recursion model (child emitters): {r}");
}
if let Some(s) = &e.spline {
- let pt = |p: &[f32; 3]| format!("({:.2},{:.2},{:.2})", p[0], p[1], p[2]);
+ // EVERY control point, and at full precision. This used to print `first .. last`,
+ // which is actively misleading on a cubic Bézier: the path is `3K+1` points and the
+ // two INTERIOR handles of each segment are what bend it, so an emitter whose ends
+ // both sit at the origin can still fling its particles a long way — or not move them
+ // at all. Reading "(0,0,0) .. (0,0,0)" as "the particles never travel" is a guess
+ // the truncated line invites and cannot support (B228 nearly rested on it).
+ // Wrapped at 4 per line so a long path stays readable.
+ let pt = |p: &[f32; 3]| format!("({:.4},{:.4},{:.4})", p[0], p[1], p[2]);
println!(
- " spline: {} control points, {} .. {}",
+ " spline: {} control points ({} cubic segment(s)), model-local:",
s.points.len(),
- s.points.first().map(pt).unwrap_or_default(),
- s.points.last().map(pt).unwrap_or_default(),
+ s.points.len().saturating_sub(1) / 3,
);
+ for (i, chunk) in s.points.chunks(4).enumerate() {
+ let row: Vec = chunk.iter().map(pt).collect();
+ println!(" [{:>2}] {}", i * 4, row.join(" "));
+ }
}
// Emission timing: the rate/gate pair, per file sequence slot.
@@ -1454,8 +1464,12 @@ pub fn m2part(chain: &mut Chain, internal_path: &str) -> Result<()> {
c[0], c[1], c[2], c[3]
);
}
+ // 4 decimals, not 3: UI models author their sizes in thousandths, where `{:.3}` turns the
+ // autocast shine's 0.0015 mid-key into a printed "0.002" — a 33% error that a transcriber
+ // reads as authored truth (B228). The colour ramp above stays at 3 because 0..1 channels
+ // do not have that problem.
println!(
- " size {:.3} -> {:.3} -> {:.3} yd (half-extent)",
+ " size {:.4} -> {:.4} -> {:.4} yd (half-extent)",
ol.scale[0], ol.scale[1], ol.scale[2]
);
println!(
diff --git a/crates/benilla-formats/src/skill_lines/mod.rs b/crates/benilla-formats/src/skill_lines/mod.rs
index 8e9ba591..113b915a 100644
--- a/crates/benilla-formats/src/skill_lines/mod.rs
+++ b/crates/benilla-formats/src/skill_lines/mod.rs
@@ -337,7 +337,7 @@ impl SkillLineCatalog {
pub fn highest_known_rank(
&self,
spell_id: u32,
- known: &std::collections::HashSet,
+ known: &std::collections::BTreeSet,
) -> Option {
let mut cur = self.chain_head(spell_id);
let mut best = known.contains(&cur).then_some(cur);
diff --git a/crates/benilla-formats/src/skill_lines/tests.rs b/crates/benilla-formats/src/skill_lines/tests.rs
index 97bb415f..7741d492 100644
--- a/crates/benilla-formats/src/skill_lines/tests.rs
+++ b/crates/benilla-formats/src/skill_lines/tests.rs
@@ -361,12 +361,12 @@ fn real_rank_chains_resolve_the_highest_known_rank() {
// From ANY rank, knowing only rank 8, the answer is rank 8 — the walk starts at the
// chain head, so it works downward as well as upward.
- let top: std::collections::HashSet = [11294].into_iter().collect();
+ let top: std::collections::BTreeSet = [11294].into_iter().collect();
for id in SS {
assert_eq!(cat.highest_known_rank(id, &top), Some(11294), "from {id}");
}
// Knowing an intermediate rank resolves to it, not to the top of the chain.
- let mid: std::collections::HashSet = [1759].into_iter().collect();
+ let mid: std::collections::BTreeSet = [1759].into_iter().collect();
assert_eq!(cat.highest_known_rank(1752, &mid), Some(1759));
assert_eq!(cat.highest_known_rank(11294, &mid), Some(1759));
// No rank of the chain known → no answer (the caller leaves the slot alone).
@@ -377,11 +377,11 @@ fn real_rank_chains_resolve_the_highest_known_rank() {
// Frostbolt r1, Healing Touch r1, Renew r1, Immolate r1, Lightning Bolt r1.
for id in [133u32, 143, 116, 5185, 139, 348, 403] {
assert_eq!(cat.rank_successor(id), None, "spell {id} is not chained");
- let known: std::collections::HashSet = [id].into_iter().collect();
+ let known: std::collections::BTreeSet = [id].into_iter().collect();
assert_eq!(cat.highest_known_rank(id, &known), Some(id));
}
// A spell with no `SkillLineAbility` row at all (the auto-attack) is its own chain.
- let attack: std::collections::HashSet = [6603].into_iter().collect();
+ let attack: std::collections::BTreeSet = [6603].into_iter().collect();
assert_eq!(cat.rank_successor(6603), None);
assert_eq!(cat.highest_known_rank(6603, &attack), Some(6603));
}
diff --git a/crates/benilla-formats/src/spells/catalog_tests.rs b/crates/benilla-formats/src/spells/catalog_tests.rs
index c8cb3ae4..1e518ce5 100644
--- a/crates/benilla-formats/src/spells/catalog_tests.rs
+++ b/crates/benilla-formats/src/spells/catalog_tests.rs
@@ -247,6 +247,43 @@ fn real_spell_catalog_reads_open_lock_types() {
"Fireball opens no lock"
);
+ // **B247's data fact** (decision 1312): `LockType 13` "Open Kneeling" — what lock 43 carries,
+ // and with it every ground container from the Hyacinth Mushroom up — has TWO shipped openers
+ // that `playercreateinfo_spell` grants to every race/class, and one of them wears Blizzard's
+ // own placeholder name. The lock resolver returns the FIRST sufficient match in the
+ // known-spell array's order, so which of these it reaches is what the cast bar prints; this
+ // pins the ambiguity to the data instead of leaving it a surprise.
+ assert_eq!(cat.get(6478).unwrap().open_lock_type(), Some(13));
+ assert_eq!(cat.get(6478).unwrap().name, "Opening");
+ assert_eq!(cat.get(22810).unwrap().open_lock_type(), Some(13));
+ assert_eq!(
+ cat.get(22810).unwrap().name,
+ "Opening - No Text",
+ "the placeholder is Blizzard's own Spell.dbc string, not a formatting bug of ours"
+ );
+
+ // ...and the attribute that says never to show it. `AttributesEx3 & 0x4`
+ // (`SPELL_ATTR_EX3_NO_CASTING_BAR_TEXT`) is what the cast bar must honour — 22810 carries it,
+ // 6478 does not, and that ONE bit is the only column separating two otherwise byte-identical
+ // rows. Exactly three rows in the shipped file carry it, so the whole set is pinned here: a
+ // fourth appearing means the data is not what this law was derived from.
+ assert!(cat.get(22810).unwrap().no_casting_bar_text());
+ assert!(!cat.get(6478).unwrap().no_casting_bar_text());
+ let silent: Vec = {
+ let mut v: Vec = cat
+ .iter()
+ .filter(|(_, d)| d.no_casting_bar_text())
+ .map(|(id, _)| id)
+ .collect();
+ v.sort_unstable();
+ v
+ };
+ assert_eq!(
+ silent,
+ vec![6477, 22810, 26380],
+ "6477 Opening / 22810 Opening - No Text / 26380 zzOLDSummon Mouth Tentacle Visual"
+ );
+
// The totem (tool) and reagent columns the pre-send possession check reads (decision 0552;
// the ref's `0x6e4000` at SpellRec+0xA0/+0xA8 = cols 40-41 / 42-49+50-57). A column slip
// here silently breaks "Requires Mining Pick" / "Missing reagent: …".
diff --git a/crates/benilla-formats/src/spells/display.rs b/crates/benilla-formats/src/spells/display.rs
index 0c146e5f..8b220715 100644
--- a/crates/benilla-formats/src/spells/display.rs
+++ b/crates/benilla-formats/src/spells/display.rs
@@ -407,6 +407,19 @@ impl SpellDisplay {
self.attributes_ex3 & ATTR_EX3_NORMAL_RANGED_ATTACK != 0
}
+ /// `AttributesEx3 & 0x4` — **the casting bar shows no name for this spell**
+ /// ([`ATTR_EX3_NO_CASTING_BAR_TEXT`], `SPELL_ATTR_EX3_NO_CASTING_BAR_TEXT`). The cast-bar feed
+ /// hands `SPELLCAST_START`/`SPELLCAST_CHANNEL_START` an empty name string when this is set, so
+ /// the bar fills and sweeps with a blank label.
+ ///
+ /// Only the *name* is suppressed, and only on the bar: the spell still casts, still animates,
+ /// and its failures still name it. Three shipped rows carry the bit; the one that reached a
+ /// player is 22810 **"Opening - No Text"**, the internal opener for `LockType 13` ground
+ /// containers, whose own name spells out what the attribute is for (decision 1312, B247).
+ pub fn no_casting_bar_text(&self) -> bool {
+ self.attributes_ex3 & ATTR_EX3_NO_CASTING_BAR_TEXT != 0
+ }
+
/// The ranged-shot cooldown pad's gate (`0x6e2b60` at `0x6e2c2c`–`0x6e2c47`, byte-verified —
/// wow-re `ranged-cooldown-sweep.md`): on the caster's own SPELL_GO self-insert, a
/// `Attributes & 0x2` spell WITHOUT `AttributesEx2 & 0x20000`
diff --git a/crates/benilla-formats/src/spells/mod.rs b/crates/benilla-formats/src/spells/mod.rs
index 0182f710..e24a27b4 100644
--- a/crates/benilla-formats/src/spells/mod.rs
+++ b/crates/benilla-formats/src/spells/mod.rs
@@ -350,6 +350,15 @@ const COL_EFFECT_ITEM_TYPE_1: usize = 103;
/// `AttributesEx3` bit `0x8000` — damage renders melee-white (`SPELL_ATTR3_NORMAL_RANGED_ATTACK`;
/// the combat-text emitter's `B`-bit flip, wow-re `combattext-color-law.md`).
const ATTR_EX3_NORMAL_RANGED_ATTACK: u32 = 0x8000;
+/// `AttributesEx3` bit `0x4` — **the cast bar shows no name for this spell**
+/// (`SPELL_ATTR_EX3_NO_CASTING_BAR_TEXT`, vmangos `Spells/SpellDefines.h:907`). Decision 1312.
+///
+/// Exactly three rows in the shipped 5875 file carry it, and one of them is *named after the bit*:
+/// 6477 "Opening", **22810 "Opening - No Text"**, 26380 "zzOLDSummon Mouth Tentacle Visual". That
+/// placeholder name is not a string benilla was ever meant to render — it is Blizzard's own note
+/// to themselves about what this attribute does to the spell, and it reached a player's screen
+/// because we printed the name unconditionally.
+const ATTR_EX3_NO_CASTING_BAR_TEXT: u32 = 0x4;
/// `Attributes` bit `0x2` — the "uses the ranged slot" attribute (mangos `SPELL_ATTR_RANGED`).
/// One half of the client's ranged-stance gate (module docs).
const ATTR_RANGED: u32 = 0x2;
diff --git a/crates/benilla-protocol/examples/cooldown_probe.rs b/crates/benilla-protocol/examples/cooldown_probe.rs
index 1f9696e5..53333283 100644
--- a/crates/benilla-protocol/examples/cooldown_probe.rs
+++ b/crates/benilla-protocol/examples/cooldown_probe.rs
@@ -120,6 +120,7 @@ fn main() -> Result<()> {
spell_id,
success,
reason,
+ ..
} => println!(
" pre: cast_result spell={spell_id} success={success} reason={reason:?} ({})",
reason.map(fail_name).unwrap_or("-")
@@ -149,6 +150,7 @@ fn main() -> Result<()> {
spell_id,
success: false,
reason,
+ ..
} if spell_id == CHARGE => {
println!(
" refused: {:?} ({})",
@@ -183,6 +185,7 @@ fn main() -> Result<()> {
spell_id,
success,
reason,
+ ..
} if spell_id == CHARGE => {
let name = reason.map(fail_name).unwrap_or("-");
println!(
diff --git a/crates/benilla-protocol/src/bin/benilla-world/world.rs b/crates/benilla-protocol/src/bin/benilla-world/world.rs
index 8c6ac340..e74c8f00 100644
--- a/crates/benilla-protocol/src/bin/benilla-world/world.rs
+++ b/crates/benilla-protocol/src/bin/benilla-world/world.rs
@@ -436,8 +436,11 @@ impl World {
spell_id,
success,
reason,
+ arg,
} => {
- println!("SMSG_CAST_RESULT: spell {spell_id} success={success} reason={reason:?}");
+ println!(
+ "SMSG_CAST_RESULT: spell {spell_id} success={success} reason={reason:?} arg={arg:?}"
+ );
if self.dest_spell == Some(*spell_id) {
self.dest_verdict = Some((*spell_id, *success, *reason));
} else if self.cast_verdict.is_none() {
diff --git a/crates/benilla-protocol/src/events.rs b/crates/benilla-protocol/src/events.rs
index 7c6a6aa0..2dc08cbc 100644
--- a/crates/benilla-protocol/src/events.rs
+++ b/crates/benilla-protocol/src/events.rs
@@ -467,11 +467,15 @@ pub enum SessionEvent {
new_spell_id: u32,
},
/// The server's verdict on our cast (`SMSG_CAST_RESULT`): `reason` is the failure code when
- /// `success` is false (the client's error-text table keys on it; surfaced raw for now).
+ /// `success` is false (the client's error-text table keys on it), and `arg` the reason-specific
+ /// argument word that fills that message's `%s` — a `SpellFocusObject.dbc` id for
+ /// `REQUIRES_SPELL_FOCUS`, an `AreaTable.dbc` id for `REQUIRES_AREA`
+ /// ([`crate::messages::CastOutcome::Failed`]).
CastResult {
spell_id: u32,
success: bool,
reason: Option,
+ arg: Option,
},
/// The pet action bar's whole state (`SMSG_PET_SPELLS`, decision 0982) — or its **teardown**,
/// which is the same event carrying a zero `pet_guid`. Server-authoritative: this is not a
diff --git a/crates/benilla-protocol/src/events/decode.rs b/crates/benilla-protocol/src/events/decode.rs
index 61a0e3b7..9bf1ab19 100644
--- a/crates/benilla-protocol/src/events/decode.rs
+++ b/crates/benilla-protocol/src/events/decode.rs
@@ -80,7 +80,11 @@ pub fn decode(packet: ServerPacket) -> Vec {
success: outcome == CastOutcome::Ok,
reason: match outcome {
CastOutcome::Ok => None,
- CastOutcome::Failed { reason } => Some(reason),
+ CastOutcome::Failed { reason, .. } => Some(reason),
+ },
+ arg: match outcome {
+ CastOutcome::Ok => None,
+ CastOutcome::Failed { arg, .. } => arg,
},
}],
ServerPacket::PetSpells(spells) => vec![SessionEvent::PetSpells(Box::new(spells))],
@@ -92,7 +96,7 @@ pub fn decode(packet: ServerPacket) -> Vec {
spell_id,
reason: match outcome {
CastOutcome::Ok => None,
- CastOutcome::Failed { reason } => Some(reason),
+ CastOutcome::Failed { reason, .. } => Some(reason),
},
}],
ServerPacket::ItemQueryResponse { entry, info } => {
diff --git a/crates/benilla-protocol/src/messages/pet.rs b/crates/benilla-protocol/src/messages/pet.rs
index 03dec35c..c12e3223 100644
--- a/crates/benilla-protocol/src/messages/pet.rs
+++ b/crates/benilla-protocol/src/messages/pet.rs
@@ -380,12 +380,16 @@ pub(super) fn read_pet_action_feedback(r: &mut impl Read) -> io::Result {
/// `SMSG_CAST_RESULT` — the same `SpellCastResult` vocabulary, decoded the same way (status `2` =
/// fail) so the one message table renders both. vmangos only ever sends the fail status here, but
/// reading it the shared way costs nothing and keeps the two decodes from drifting.
+///
+/// The body stops at the reason: unlike `CastResult`, `PetCastFailed::AppendBodyTo` writes **no
+/// argument words** at all, so a pet failure never carries a `%s` fill.
pub(super) fn read_pet_cast_failed(r: &mut impl Read) -> io::Result<(u32, super::CastOutcome)> {
let spell_id = read_u32_le(r)?;
let status = read_u8(r)?;
let outcome = if status == 2 {
super::CastOutcome::Failed {
reason: read_u8(r)?,
+ arg: None,
}
} else {
super::CastOutcome::Ok
diff --git a/crates/benilla-protocol/src/messages/spells.rs b/crates/benilla-protocol/src/messages/spells.rs
index f8651414..b83114e9 100644
--- a/crates/benilla-protocol/src/messages/spells.rs
+++ b/crates/benilla-protocol/src/messages/spells.rs
@@ -28,24 +28,52 @@ use crate::wire::{
};
/// `SMSG_CAST_RESULT`'s verdict: `u32 spellId, u8 status` — status `0` (`SPELL_RESULT_STATUS_OKAY`)
-/// ends the packet, status `2` (`SPELL_RESULT_STATUS_FAIL`) appends a `u8` failure reason
-/// (vmangos `CastResult::AppendBodyTo`; some reasons append extra arg words we skip).
+/// ends the packet, status `2` (`SPELL_RESULT_STATUS_FAIL`) appends a `u8` failure reason and then
+/// up to two **reason-specific argument words** (vmangos `CastResult::AppendBodyTo`).
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum CastOutcome {
Ok,
- Failed { reason: u8 },
+ Failed {
+ reason: u8,
+ /// The failure's first argument word, when the server sent one — the `%s` source of the
+ /// client's argument-formatted messages (the arm table at `0x6e1d8e`): a
+ /// `SpellFocusObject.dbc` id for `REQUIRES_SPELL_FOCUS` (0x5e), an `AreaTable.dbc` id for
+ /// `REQUIRES_AREA` (0x5d), an `EquippedItemClass` for the 0x19–0x1b family, a
+ /// permanent-cooldown flag for `NOT_READY` (0x3c). `None` = the packet ended at the reason.
+ arg: Option,
+ },
}
-/// Read `SMSG_CAST_RESULT` → `(spell_id, outcome)`. Reason-specific trailing args (a required
-/// spell-focus id, an equip class) are left unread — the slice ends with the packet, nothing
-/// follows in the stream.
-pub(super) fn read_cast_result(r: &mut impl Read) -> io::Result<(u32, CastOutcome)> {
+/// Read `SMSG_CAST_RESULT` → `(spell_id, outcome)`.
+///
+/// The trailing argument words are **positional, gated on the remaining body length** — and that
+/// is the reference's own decode, not merely a convenient one. `CastResultHandler 0x6e7330`
+/// (registered at `0x6e7155`) reads `u32 spellId`, `u8 status`, then on `status == 2` a
+/// `u8 reason`, then each word only `if cursor < size` (`0x6e737a` and `0x6e738c`, comparing
+/// CDataStore `+0x14` against `+0x10`) — never keyed off the reason value (wow-re
+/// `system/spell/scratch/cast-fail-strings.md` §WIRE-ARGS, §5 cross-checked). It has to be: vmangos
+/// writes `if (arg1 || arg2) << u32 arg1; if (arg2) << u32 arg2;`
+/// (`Server/Packets/Spell.cpp`, `CastResult::AppendBodyTo`), so a reason whose argument is
+/// legitimately 0 with no second argument behind it sends no word at all, and a reason-keyed decode
+/// would desync on exactly that case.
+///
+/// **The reference's absent value is `-1`, not zero** (`0x6e736e or eax,0xffffffff` initializes both
+/// slots before the gated reads). `None` here is that sentinel: every consumer treats it and an
+/// out-of-range id the same way — decline the fill — so the two encodings are behaviourally
+/// identical, and `Option` says which case it is at the type level.
+///
+/// The second word is read to keep the shape documented, then dropped: only the
+/// `EQUIPPED_ITEM_CLASS*` family (0x19–0x1b) sends one, and that arm's `%s` fill reads the failing
+/// spell's own `EquippedItemClass`/`EquippedItemSubClassMask` columns — the same two values the
+/// server copied out of `SpellEntry` to build the packet (`Spell::SendCastResult`).
+pub(super) fn read_cast_result(r: &mut &[u8]) -> io::Result<(u32, CastOutcome)> {
let spell_id = read_u32_le(r)?;
let status = read_u8(r)?;
let outcome = if status == 2 {
- CastOutcome::Failed {
- reason: read_u8(r)?,
- }
+ let reason = read_u8(r)?;
+ let arg = (r.len() >= 4).then(|| read_u32_le(r)).transpose()?;
+ let _arg2 = (r.len() >= 4).then(|| read_u32_le(r)).transpose()?;
+ CastOutcome::Failed { reason, arg }
} else {
CastOutcome::Ok
};
diff --git a/crates/benilla-protocol/tests/spells.rs b/crates/benilla-protocol/tests/spells.rs
index 507735b7..fbb696c6 100644
--- a/crates/benilla-protocol/tests/spells.rs
+++ b/crates/benilla-protocol/tests/spells.rs
@@ -142,7 +142,43 @@ fn spell_and_action_wire() {
spell_id,
success,
reason,
- } => assert_eq!((spell_id, success, reason), (6673, false, Some(0x55))),
+ arg,
+ } => assert_eq!(
+ (spell_id, success, reason, arg),
+ (6673, false, Some(0x55), None),
+ "a reason carrying no argument ends the body at the reason byte"
+ ),
+ _ => panic!("cast fail event"),
+ }
+ // The reason-specific argument words (`CastResult::AppendBodyTo`) are POSITIONAL — read by
+ // what is left in the body, never keyed off the reason. One word for 0x5e
+ // REQUIRES_SPELL_FOCUS (the `SpellFocusObject.dbc` id: 12 = "Starbreeze Village Moonwell",
+ // the `%s` of "Requires %s"); two for the 0x19 EQUIPPED_ITEM_CLASS family (class + subclass
+ // mask), whose second is read and dropped.
+ let focus = messages::parse_server(
+ messages::opcode::SMSG_CAST_RESULT,
+ &hx("111a0000025e0c000000"),
+ )
+ .unwrap();
+ match decode(focus).pop().unwrap() {
+ SessionEvent::CastResult { reason, arg, .. } => {
+ assert_eq!((reason, arg), (Some(0x5e), Some(12)));
+ }
+ _ => panic!("cast fail event"),
+ }
+ let equip = messages::parse_server(
+ messages::opcode::SMSG_CAST_RESULT,
+ &hx("111a0000021902000000f3a50200"),
+ )
+ .unwrap();
+ match decode(equip).pop().unwrap() {
+ SessionEvent::CastResult { reason, arg, .. } => {
+ assert_eq!(
+ (reason, arg),
+ (Some(0x19), Some(2)),
+ "arg2 read and dropped"
+ );
+ }
_ => panic!("cast fail event"),
}
diff --git a/crates/benilla-ui/src/loader/scripts.rs b/crates/benilla-ui/src/loader/scripts.rs
index 3f8f7c8e..4a3e0af0 100644
--- a/crates/benilla-ui/src/loader/scripts.rs
+++ b/crates/benilla-ui/src/loader/scripts.rs
@@ -39,10 +39,9 @@ impl Loader<'_> {
// is in the set, `OnDragStop`/`OnReceiveDrag` are NOT. This is XML-load-time
// ONLY: the Lua SetScript binding (`0x7748d0`) never auto-enables, so the law
// lives here and not in SetScript itself (a runtime-created frame still needs an
- // explicit `EnableMouse(true)`, like the real client). The keyboard kinds
- // (`OnChar` = kind 0, `OnKeyDown`/`OnKeyUp` = kind 1) and the wheel kind
- // (`OnMouseWheel` = kind 3) have separate indexes in the real client that this
- // engine doesn't model yet — un-modeled beside the SetScript keyboard gap above.
+ // explicit `EnableMouse(true)`, like the real client). The wheel kind
+ // (`OnMouseWheel` = kind 3) has a separate index this engine doesn't model yet;
+ // the KEYBOARD kinds are modelled and armed just below.
const MOUSE_KIND: [&str; 5] = [
"OnEnter",
"OnLeave",
@@ -53,6 +52,16 @@ impl Loader<'_> {
if MOUSE_KIND.iter().any(|k| name.eq_ignore_ascii_case(k)) {
self.call(wrapper, "EnableMouse", true, dbg);
}
+ // The KEYBOARD kinds, the same walker rule one index over (`OnChar` = kind 0,
+ // `OnKeyDown`/`OnKeyUp` = kind 1). Bucket membership is what the delivery walk
+ // iterates ([`crate::script::keyboard`]) and it is the FLAG, never the presence of
+ // a script — so an XML frame carrying a key handler but no `enableKeyboard`
+ // attribute would never be reached by the dispatcher at all, and its handler could
+ // not fire. XML-load-time only, like the mouse half.
+ const KEY_KIND: [&str; 3] = ["OnChar", "OnKeyDown", "OnKeyUp"];
+ if KEY_KIND.iter().any(|k| name.eq_ignore_ascii_case(k)) {
+ self.call(wrapper, "EnableKeyboard", true, dbg);
+ }
if name.eq_ignore_ascii_case("OnLoad") {
onload = Some(func);
}
diff --git a/crates/benilla-ui/src/loader/tests.rs b/crates/benilla-ui/src/loader/tests.rs
index 25fca7e2..6deda6dc 100644
--- a/crates/benilla-ui/src/loader/tests.rs
+++ b/crates/benilla-ui/src/loader/tests.rs
@@ -461,18 +461,24 @@ mod loader_tests {
assert!(s.eval::("return FineRan == true").unwrap());
}
- /// Unsupported handler names (e.g. the keyboard-focus handlers `OnKeyDown`/`OnChar`, not yet
- /// modeled) are warn-once gaps, not hard errors, and don't stop the frame from building.
+ /// Unsupported handler names are warn-once gaps, not hard errors, and don't stop the frame
+ /// from building. The example is `OnCursorChanged` — caret geometry is host-side here, so its
+ /// four float args would all be zero, which is the silent-drop this warn exists to avoid.
+ /// (It used to be `OnKeyDown`: that one is *fired* since decision 1319 and now belongs to
+ /// `script::tests::keyboard`.)
#[test]
fn unsupported_script_name_is_a_warning() {
let s = UiScript::new().unwrap();
let doc = parse(
- r#"x = 1 x = 1 ("return E:GetText()").unwrap(),
+ "k",
+ "…by inserting it through its own path"
+ );
+ assert_eq!(
+ s.eval::("return fired").unwrap(),
+ 0,
+ "the box's own vtable never chains to the generic OnChar"
+ );
+ assert!(s.errors().is_empty(), "{:?}", s.errors());
}
// ── history recall (`historyLines` / AddHistoryLine — decision 0288 P2) ──────────────────────
diff --git a/crates/benilla-ui/src/script/event.rs b/crates/benilla-ui/src/script/event.rs
index e563e850..65154c40 100644
--- a/crates/benilla-ui/src/script/event.rs
+++ b/crates/benilla-ui/src/script/event.rs
@@ -162,6 +162,23 @@ pub(super) fn fire_visibility_changes(lua: &Lua, changed: Vec) {
}
}
+/// Is a handler bound under `script` for this frame id? — the **existence** question, asked
+/// without firing anything.
+///
+/// It exists for the keyboard walk ([`super::keyboard`]), where the reference's consumption gate is
+/// the presence of the slot rather than anything the handler does: `0x76b7d0` reads `[+0x188]`/
+/// `[+0x190]`, consumes on either, and fires only the first. A lookup failure reads as "absent" —
+/// the gate must never be able to raise out of the middle of a walk.
+pub(super) fn has_widget_handler(lua: &Lua, id: u32, script: &str) -> bool {
+ let Ok(scripts) = lua.named_registry_value::(REG_SCRIPTS) else {
+ return false;
+ };
+ match scripts.get::(id) {
+ Ok(Value::Table(per)) => matches!(per.get::(script), Ok(Value::Function(_))),
+ _ => false,
+ }
+}
+
/// The one firing path. `event_name` is the `event` global + first modern positional (OnEvent only);
/// `extra` are the `arg1..argN` globals + the trailing modern positionals. Globals are saved before
/// and restored after (even on error) — nesting-safe. Holds no model borrow across the call.
diff --git a/crates/benilla-ui/src/script/keyboard.rs b/crates/benilla-ui/src/script/keyboard.rs
new file mode 100644
index 00000000..1d7fef21
--- /dev/null
+++ b/crates/benilla-ui/src/script/keyboard.rs
@@ -0,0 +1,230 @@
+//! **Frame keyboard delivery** — the walk, the existence gate, and the key-name argument.
+//!
+//! Until this module, keys went straight to the focused EditBox and a plain frame could not receive
+//! a key at all: `OnChar`/`OnKeyDown`/`OnKeyUp` were the three script names
+//! [`super::object::events_regions`] deliberately *raised* on, because accepting a name this engine
+//! never fires is the silent-drop class 1203/1205/1211 each recorded. This is the machinery that
+//! note was waiting on, so those three names are now accepted there.
+//!
+//! ## The law (wow-re `system/ui/scratch/frame-key-script-delivery.md`, §5 trio — VERIFIED)
+//!
+//! An OS key event reaches **one dispatcher per channel**, and the dispatcher — not the frame — is
+//! what walks: `0x765f10` (key-down, kind-1 buckets, vtable `+0x60`) and `0x765df0` (char, kind-0
+//! buckets, `+0x5c`). Each walks **9 strata records, TOOLTIP (8) down to WORLD (0)**, and within a
+//! record the frame array **index 0 upward**, which the inserter `0x764aa0` keeps ordered by
+//! **level descending, ties oldest-registration-first**. Every frame in that order is *called*
+//! until one returns nonzero; that one **consumes** and both loops stop. So several frames may be
+//! called, at most one consumes.
+//!
+//! **The consumption gate is EXISTENCE, not handling** (`0x76b7d0`/`0x76bba0`/`0x76b760`, §3):
+//!
+//! | slot state | key-down returns | fires |
+//! |---|---|---|
+//! | `OnKeyDown` set | **consume** | `OnKeyDown` |
+//! | only `OnKeyUp` set | **consume** | *nothing* |
+//! | neither | decline | nothing |
+//!
+//! That asymmetry is real and is transcribed below: a frame carrying only an `OnKeyUp` swallows
+//! every key-down and runs no script. `OnChar` gates on its own slot alone; `OnKeyUp` on `+0x190`
+//! alone (no `OnKeyDown` fallback). **A 1.12 handler cannot signal "handled"** — the fire's return
+//! is discarded at all three call sites (§3.1) — so a Lua handler never influences consumption.
+//!
+//! **Bucket membership is the keyboard-enabled flag**, never the presence of a script (§3.2):
+//! `EnableKeyboard(true)` on a script-less frame puts it *in* the walk, where it is called and
+//! declines. XML `enableKeyboard` enables both kinds; an XML `` block auto-enables per
+//! handler; **Lua `SetScript` auto-enables nothing**, so a runtime-created frame needs an explicit
+//! `EnableKeyboard(true)` exactly as in the real client.
+//!
+//! ## Where the EditBox sits — it is a participant, not a separate stage
+//!
+//! The reference has no "editbox first" rule: `CSimpleEditBox` is a frame in the same buckets whose
+//! own vtable (`0x77b160` key-down / `0x77a900` char) consumes while focused and never chains to
+//! the base. Modelling it as a stage before or after the walk would get the order wrong whenever a
+//! keyboard frame and the focused box are in different strata — a TOOLTIP-strata frame really does
+//! pre-empt a focused HIGH-strata box, and a WORLD-strata one really does not. So [`walk`] visits
+//! the focused box **at its own strata/level**, handing it to the RF-0082 routing this crate
+//! already had ([`super::editbox`]) and taking a `true` as consumption.
+//!
+//! What is deliberately NOT re-derived here: the box's own focus acquisition (`autoFocus`
+//! self-acquire, click-to-focus, `SetFocus`) stays entirely in `editbox`, and an event no frame
+//! consumed still falls through to it — so every existing routing test holds unchanged.
+//!
+//! ## Not modelled, named
+//!
+//! The **sticky per-key-code key-up slot** (§2.1: `0x765fd0` routes key-up to `[root+code*4+0x84]`,
+//! the last frame that consumed *some* down for that code, and does **not** clear it on delivery)
+//! is not built: this engine's host feeds no key-up at all today. `OnKeyUp` is therefore accepted,
+//! stored, gated in the table above — and never fired. That is stated rather than hidden, because
+//! its one observable consequence is live: a frame with only an `OnKeyUp` still consumes key-downs.
+
+use mlua::Lua;
+
+use super::{event, Model};
+use crate::widget::FrameHandle;
+
+/// Which channel a dispatch is on — the two walks the reference registers separately.
+#[derive(Clone, Copy, PartialEq, Eq)]
+enum Channel {
+ /// `0x765df0` — kind-0 buckets, vtable `+0x5c`, fires `OnChar` with the literal character.
+ Char,
+ /// `0x765f10` — kind-1 buckets, vtable `+0x60`, fires `OnKeyDown` with the key NAME.
+ KeyDown,
+}
+
+/// The walk order (§2): every keyboard-enabled, effectively-visible frame, **strata descending,
+/// then level descending, then oldest registration first**.
+///
+/// Built as a sorted `Vec` per dispatch rather than kept as nine live buckets. The reference
+/// maintains the buckets incrementally because it is walking them on every key at 1.12 frame
+/// budgets; here a key press is a human-rate event and the candidate set is the *keyboard-enabled*
+/// frames only — single digits in any real interface — so an index that must be kept coherent with
+/// every Show/Hide/SetFrameLevel/SetFrameStrata would be pure drift surface for no measurable win.
+/// The ORDER is the law; the storage is ours.
+fn walk_order(model: &Model) -> Vec {
+ let mut candidates: Vec<(FrameHandle, u8, u16, u32)> = model
+ .arena
+ .iter_frames()
+ .filter(|(_, f)| f.effective_visible && f.keyboard_enabled)
+ .map(|(h, f)| (h, f.strata as u8, f.level, f.insertion_seq))
+ .collect();
+ // strata DESC, level DESC, insertion ASC — the inserter's own ordering (`0x764aa0`), where a
+ // frame goes before the first entry of strictly lower level and equal levels keep registration
+ // order.
+ candidates.sort_by(|a, b| b.1.cmp(&a.1).then(b.2.cmp(&a.2)).then(a.3.cmp(&b.3)));
+ candidates.into_iter().map(|(h, ..)| h).collect()
+}
+
+/// Does `frame` have a script bound under `name`?
+///
+/// The existence test the C++ gate performs on `+0x180`/`+0x188`/`+0x190` — asked of the script
+/// registry, which is where this engine keeps those slots.
+fn has_script(lua: &Lua, id: u32, name: &str) -> bool {
+ event::has_widget_handler(lua, id, name)
+}
+
+/// The dispatcher for one channel — [`Channel`]'s walk, gate and fire, in the reference's order.
+///
+/// Returns whether the event was **consumed**, which is what the caller must use to suppress the
+/// keybinding system: consumption is decided by the C++ existence gate alone, never by what the
+/// Lua handler did or returned (§3.1).
+fn walk(lua: &Lua, channel: Channel, arg: &str) -> bool {
+ let order = {
+ let model = lua.app_data_ref::().expect("model app_data");
+ walk_order(&model)
+ };
+ for h in order {
+ // The focused EditBox consumes through its OWN vtable, never the base one — so it is asked
+ // in its walk position and its answer is final for this event.
+ let is_focused_box = {
+ let model = lua.app_data_ref::().expect("model app_data");
+ model.focused_editbox == Some(h)
+ };
+ if is_focused_box {
+ let consumed = match channel {
+ Channel::Char => super::editbox::char_input(lua, arg),
+ Channel::KeyDown => super::editbox::key_input(lua, arg),
+ };
+ if consumed {
+ return true;
+ }
+ continue;
+ }
+ let id = {
+ let mut model = lua.app_data_mut::().expect("model app_data");
+ model.frame_id(h)
+ };
+ match channel {
+ // `0x76b760`: gates on the OnChar slot alone.
+ Channel::Char => {
+ if has_script(lua, id, "OnChar") {
+ fire(lua, id, "OnChar", arg);
+ return true;
+ }
+ }
+ // `0x76b7d0`: the OR gate — either key slot consumes, only `OnKeyDown` fires.
+ Channel::KeyDown => {
+ let down = has_script(lua, id, "OnKeyDown");
+ if down || has_script(lua, id, "OnKeyUp") {
+ if down {
+ fire(lua, id, "OnKeyDown", arg);
+ }
+ return true;
+ }
+ }
+ }
+ }
+ false
+}
+
+/// Fire one key handler with its single string argument (`0x7026f0(frame, &slot, "%s", str)` — the
+/// argument is a string on all three channels). Errors land in [`Model::errors`] like every other
+/// widget handler; a raising handler must not eat the key press.
+fn fire(lua: &Lua, id: u32, script: &str, arg: &str) {
+ let val = mlua::Value::String(match lua.create_string(arg) {
+ Ok(s) => s,
+ Err(e) => {
+ lua.app_data_mut::()
+ .expect("model app_data")
+ .errors
+ .push(e.to_string());
+ return;
+ }
+ });
+ if let Err(e) = event::fire_widget_handler(lua, id, script, vec![val]) {
+ lua.app_data_mut::()
+ .expect("model app_data")
+ .errors
+ .push(e.to_string());
+ }
+}
+
+/// The char channel (`0x765df0`) — `arg1` is the literal character, UTF-8 (§4.1: the code is
+/// UTF-8-encoded with no range guard, so a multi-byte codepoint arrives whole).
+pub(super) fn char_input(lua: &Lua, text: &str) -> bool {
+ walk(lua, Channel::Char, text)
+}
+
+/// The key-down channel (`0x765f10`) — `arg1` is the key NAME, with no modifier prefix, from the
+/// same table the keybinding chord uses (§4.2). The host speaks those names already
+/// (`crate::script::UiScript::key_input`'s contract), so no decode happens here.
+pub(super) fn key_input(lua: &Lua, key: &str) -> bool {
+ walk(lua, Channel::KeyDown, key)
+}
+
+/// The same key-down walk, for the keys this engine delivers to a **focused EditBox as a semantic
+/// [`crate::script::EditAction`] chord** rather than by name — BACKSPACE, DELETE, the arrows, HOME,
+/// END (decision 0301: the host's per-OS keymap owns which chord means what, so the box never sees
+/// these as names).
+///
+/// Those keys still have to reach a keyboard frame — a dialog you type into needs its BACKSPACE —
+/// and the ordering question that creates is real: whoever comes FIRST in the walk owns the key.
+/// So this runs the identical order and **declines at the focused box** rather than skipping past
+/// it: a frame above the box in walk order consumes and fires; a box above the frame ends the walk
+/// with `false`, and the caller then dispatches its chord exactly as before. The one thing this
+/// must never do is let a frame *below* the focused box steal the box's editing keys.
+pub(super) fn frame_key_input(lua: &Lua, key: &str) -> bool {
+ let order = {
+ let model = lua.app_data_ref::().expect("model app_data");
+ walk_order(&model)
+ };
+ for h in order {
+ {
+ let model = lua.app_data_ref::().expect("model app_data");
+ if model.focused_editbox == Some(h) {
+ return false; // the box owns this key; its chord path handles it
+ }
+ }
+ let id = {
+ let mut model = lua.app_data_mut::().expect("model app_data");
+ model.frame_id(h)
+ };
+ let down = has_script(lua, id, "OnKeyDown");
+ if down || has_script(lua, id, "OnKeyUp") {
+ if down {
+ fire(lua, id, "OnKeyDown", key);
+ }
+ return true;
+ }
+ }
+ false
+}
diff --git a/crates/benilla-ui/src/script/mod.rs b/crates/benilla-ui/src/script/mod.rs
index f27e6e5e..18a9672f 100644
--- a/crates/benilla-ui/src/script/mod.rs
+++ b/crates/benilla-ui/src/script/mod.rs
@@ -72,6 +72,7 @@ mod inspect;
mod item_stats;
mod item_text;
pub mod keybind;
+mod keyboard;
mod layout;
mod loot;
mod loot_roll;
@@ -353,7 +354,7 @@ pub const SCREEN: crate::layout::Handle = 0;
/// corpus call sites across 91 addons), but it removes working behaviour from those 30 sites, so it
/// is a change to make deliberately with FrameXML fixed first — not a side effect of widening this
/// list.
-const SCRIPT_KINDS: [&str; 32] = [
+const SCRIPT_KINDS: [&str; 35] = [
"OnLoad",
"OnEvent",
"OnUpdate",
@@ -404,6 +405,16 @@ const SCRIPT_KINDS: [&str; 32] = [
// [`crate::layout::size_changed`]'s byte-verified epsilon test — see
// [`UiScript::resolve_layout`]. `OnSizeChanged(self, width, height)`.
"OnSizeChanged",
+ // The three KEY channels, unblocked by [`keyboard`]'s walk (wow-re
+ // `scratch/frame-key-script-delivery.md`, VERIFIED). They were the standing exception in
+ // [`object::events_regions`]'s note — accepted only once something fired them, which is that
+ // module's whole rule. `OnKeyUp` rides in with the other two deliberately: it is *gated* today
+ // (a frame carrying only an OnKeyUp consumes every key-down and runs nothing — the reference's
+ // own asymmetry) even though this engine's host feeds no key-up to fire it with, and accepting
+ // the name is what makes that consumption reachable.
+ "OnChar",
+ "OnKeyDown",
+ "OnKeyUp",
];
// ─────────────────────────────────────────────────────────────────────────────────────────────
@@ -968,7 +979,11 @@ impl UiScript {
/// focused box, inserts it (numeric/cap/password rules apply) or — for the Ctrl+A control code —
/// selects all. Returns `true` if consumed (a focused box consumes every char).
pub fn char_input(&mut self, text: &str) -> bool {
- editbox::char_input(&self.lua, text)
+ // The frame walk first ([`keyboard`]): the focused box is a PARTICIPANT in it, at its own
+ // strata/level, so this is not "frames before boxes" — it is the reference's one dispatcher
+ // in the reference's order. An event no frame consumed still falls through to the box
+ // routing, which owns focus acquisition (`autoFocus` self-acquire and kin).
+ keyboard::char_input(&self.lua, text) || editbox::char_input(&self.lua, text)
}
/// Paste text from the host OS clipboard into the focused EditBox. The engine-free runtime can't
@@ -985,7 +1000,20 @@ impl UiScript {
/// owns which chord means what). Routes per §1/§2; a focused box consumes the key even when
/// it does nothing with it. Returns `true` if consumed.
pub fn key_input(&mut self, key: &str) -> bool {
- editbox::key_input(&self.lua, key)
+ // Same two-stage shape as `char_input` — see its note.
+ keyboard::key_input(&self.lua, key) || editbox::key_input(&self.lua, key)
+ }
+
+ /// A key the host delivers to a focused EditBox as an [`EditAction`] chord rather than by name
+ /// (BACKSPACE, DELETE, the arrows, HOME, END), offered to the **keyboard frames** first.
+ ///
+ /// Returns `true` if a frame consumed it, in which case the caller must NOT also dispatch the
+ /// chord — and the key's binding must not fire either (consumption suppresses it, wow-re §3).
+ /// A `false` means either nothing wanted it or the focused box owns it; the caller proceeds
+ /// exactly as it did before this entry point existed. See [`keyboard::frame_key_input`] for
+ /// why declining at the box is the faithful answer rather than skipping it.
+ pub fn frame_key_input(&mut self, key: &str) -> bool {
+ keyboard::frame_key_input(&self.lua, key)
}
/// One semantic text-editing operation on the focused EditBox — the output of the host's
diff --git a/crates/benilla-ui/src/script/tests/generic_for.rs b/crates/benilla-ui/src/script/tests/generic_for.rs
index 99477d1b..ee7589ec 100644
--- a/crates/benilla-ui/src/script/tests/generic_for.rs
+++ b/crates/benilla-ui/src/script/tests/generic_for.rs
@@ -221,3 +221,65 @@ fn a_nested_long_string_parses_as_lua_50_does() {
.expect("nested long string must parse");
assert_eq!(v, "a [[b]] c");
}
+
+/// **5.0's constructor compat-semicolon, restored** — the third dialect divergence (decision 1315).
+///
+/// Lua 5.0's `constructor()` opens its field loop with `testnext(ls, ';') /* compatibility
+/// only */` — a Lua 4.0 leftover (4.0 separated a constructor's list part from its record part
+/// with `;`) that silently eats ONE extra `;` per separator position inside a table constructor.
+/// 5.1 deleted the line. AtlasLoot's `ButtonRegistry.lua` writes `Back_Title = AL["Factions"];;`
+/// twenty times; the real client loads it (director-verified on the live 1.12.1 client), stock
+/// 5.1 answered "unexpected symbol near ';'" and the addon's registry global never existed (B266).
+#[test]
+fn a_double_semicolon_inside_a_constructor_parses_as_lua_50_does() {
+ let s = script();
+
+ // The exact corpus shape: a record field, its `;` separator, and the compat extra.
+ let out: String = s
+ .eval(
+ r#"
+ local AL = { Factions = "Factions" }
+ local reg = {
+ ["REP1"] = { Title = "The Aldor"; Back_Page = "REPMENU"; Back_Title = AL["Factions"];; };
+ }
+ return reg["REP1"].Back_Title
+ "#,
+ )
+ .expect("the ButtonRegistry shape must parse");
+ assert_eq!(out, "Factions");
+
+ // The compat skip runs at loop TOP, so a lone `;` before `}` — legal 5.0, an empty record
+ // part in 4.0 terms — is eaten the same way.
+ assert_eq!(s.eval::("local t = {;} return 1").unwrap(), 1);
+
+ // List context too: the separator may be `;` and the extra one is still absorbed.
+ let n: i64 = s.eval("local t = { 1;; 2 } return t[1] + t[2]").unwrap();
+ assert_eq!(n, 3);
+}
+
+/// **The restoration is exactly one semicolon wide, and constructor-only.** Both halves of the
+/// boundary are the client's: 5.0 eats ONE extra `;` per position (a third dies in `listfield`),
+/// and statement-level `;;` is rejected by 5.0 and 5.1 alike — the half wow-5875-re byte-verified
+/// in the client's `luaL_loadbuffer` first (`system/ui/scratch/lua-dialect.md`).
+#[test]
+fn the_compat_semicolon_is_one_wide_and_statement_level_stays_rejected() {
+ let s = script();
+
+ let err = s
+ .eval::("local t = { 1;;; 2 } return t[1]")
+ .unwrap_err()
+ .to_string();
+ assert!(
+ err.contains("unexpected symbol"),
+ "a THIRD semicolon must still be a syntax error, as on 5.0: {err}"
+ );
+
+ let err = s
+ .eval::("local x = 1;; return x")
+ .unwrap_err()
+ .to_string();
+ assert!(
+ err.contains("unexpected symbol"),
+ "statement-level `;;` must stay rejected — that half is byte-verified: {err}"
+ );
+}
diff --git a/crates/benilla-ui/src/script/tests/input.rs b/crates/benilla-ui/src/script/tests/input.rs
index bd4a10fc..0f087a4b 100644
--- a/crates/benilla-ui/src/script/tests/input.rs
+++ b/crates/benilla-ui/src/script/tests/input.rs
@@ -598,13 +598,10 @@ fn the_unfired_script_kinds_still_raise_rather_than_silently_accepting() {
s.run(r#"Raiser = CreateFrame("EditBox", "RaiserBox")"#)
.unwrap();
for name in [
- // The FLAG exists (`EnableKeyboard`/`IsKeyboardEnabled`, 1252); what is still absent is the
- // keyboard INDEX and the strata walk that would deliver a key to a frame carrying it, so
- // these three stay unfired (wow-re `scripts-auto-enable.md` kinds 0/1). Being enabled is
- // not being a handler — the carve is explicit that the two are separable.
- "OnKeyDown",
- "OnKeyUp",
- "OnChar",
+ // `OnKeyDown`/`OnKeyUp`/`OnChar` are NOT in this list any more: decision 1319 built the
+ // delivery walk, so they are fired and therefore accepted — the rule this test enforces,
+ // applied in the other direction. Their law lives in `script::tests::keyboard`.
+ //
// Caret geometry is host-side; its four float args would all be zero.
"OnCursorChanged",
// 2.0's secure-frame system — no such slot exists in any 1.12 resolver.
diff --git a/crates/benilla-ui/src/script/tests/keyboard.rs b/crates/benilla-ui/src/script/tests/keyboard.rs
new file mode 100644
index 00000000..cc67d816
--- /dev/null
+++ b/crates/benilla-ui/src/script/tests/keyboard.rs
@@ -0,0 +1,186 @@
+//! The frame keyboard delivery law (decision 1319; wow-re
+//! `system/ui/scratch/frame-key-script-delivery.md`, §5 trio — VERIFIED).
+//!
+//! Each test pins one clause that a plausible-but-wrong implementation gets backwards.
+
+use super::common::script;
+
+/// The bucket gate (§3.2): membership is the **keyboard-enabled flag**, not the presence of a
+/// script. A Lua-created frame auto-enables nothing (the reference's `SetScript` doesn't either),
+/// so its handler is unreachable until `EnableKeyboard(true)` puts it in the walk.
+#[test]
+fn a_key_script_alone_does_not_put_a_frame_in_the_walk() {
+ let mut s = script();
+ s.run(
+ r#"
+ got = nil
+ f = CreateFrame("Frame", "KbUnenabled")
+ f:SetScript("OnChar", function() got = arg1 end)
+ "#,
+ )
+ .unwrap();
+ assert!(
+ !s.char_input("7"),
+ "an unenabled frame is not in the bucket"
+ );
+ assert_eq!(s.eval::>("return got").unwrap(), None);
+
+ s.run("f:EnableKeyboard(true)").unwrap();
+ assert!(
+ s.char_input("7"),
+ "enabled: now in the walk, and it consumes"
+ );
+ assert_eq!(
+ s.eval::>("return got").unwrap().as_deref(),
+ Some("7")
+ );
+ assert!(s.errors().is_empty(), "{:?}", s.errors());
+}
+
+/// The existence gate's asymmetry (§3): a frame carrying **only** an `OnKeyUp` consumes every
+/// key-down and runs nothing. This is the clause an implementation "tidies away" — and doing so
+/// silently un-suppresses that frame's keybindings.
+#[test]
+fn only_an_onkeyup_still_swallows_the_key_down() {
+ let mut s = script();
+ s.run(
+ r#"
+ downs = 0
+ f = CreateFrame("Frame", "KbUpOnly")
+ f:EnableKeyboard(true)
+ f:SetScript("OnKeyUp", function() downs = downs + 1 end)
+ "#,
+ )
+ .unwrap();
+ assert!(s.key_input("ESCAPE"), "the OnKeyUp slot alone consumes");
+ assert_eq!(
+ s.eval::("return downs").unwrap(),
+ 0,
+ "…and fires nothing"
+ );
+
+ // Neither slot: declines outright.
+ s.run("f:SetScript(\"OnKeyUp\", nil)").unwrap();
+ assert!(!s.key_input("ESCAPE"), "no key slot at all: declines");
+ assert!(s.errors().is_empty(), "{:?}", s.errors());
+}
+
+/// The walk order (§2): **strata descending first**, and only then level. A LOW frame can never
+/// take a key from a HIGH one, however high its level — the same "a raise never changes stratum"
+/// shape the toplevel law has.
+#[test]
+fn the_walk_is_strata_then_level_then_registration() {
+ let mut s = script();
+ s.run(
+ r#"
+ winner = nil
+ function mk(name, strata, level)
+ local f = CreateFrame("Frame", name)
+ f:SetFrameStrata(strata)
+ f:SetFrameLevel(level)
+ f:EnableKeyboard(true)
+ f:SetScript("OnKeyDown", function() winner = name end)
+ return f
+ end
+ -- registered first, but the LOWEST stratum: must lose despite its huge level
+ mk("KbLow", "LOW", 99)
+ mk("KbHigh", "HIGH", 1)
+ "#,
+ )
+ .unwrap();
+ assert!(s.key_input("ESCAPE"));
+ assert_eq!(
+ s.eval::("return winner").unwrap(),
+ "KbHigh",
+ "strata beats level"
+ );
+
+ // Within one stratum: higher level wins, and equal levels keep registration order.
+ s.run(
+ r#"
+ winner = nil
+ mk("KbHighTop", "HIGH", 5)
+ mk("KbHighTie", "HIGH", 5)
+ "#,
+ )
+ .unwrap();
+ assert!(s.key_input("ESCAPE"));
+ assert_eq!(
+ s.eval::("return winner").unwrap(),
+ "KbHighTop",
+ "level 5 beats level 1; the earlier of the two 5s wins the tie"
+ );
+ assert!(s.errors().is_empty(), "{:?}", s.errors());
+}
+
+/// **At most one frame consumes** (§2, "Delivery"): the walk stops at the first consumer, so a
+/// second keyboard frame below it never sees the key.
+#[test]
+fn the_first_consumer_ends_the_walk() {
+ let mut s = script();
+ s.run(
+ r#"
+ seen = {}
+ function mk(name, strata)
+ local f = CreateFrame("Frame", name)
+ f:SetFrameStrata(strata)
+ f:EnableKeyboard(true)
+ f:SetScript("OnChar", function() table.insert(seen, name) end)
+ end
+ mk("KbTip", "TOOLTIP")
+ mk("KbDlg", "DIALOG")
+ "#,
+ )
+ .unwrap();
+ assert!(s.char_input("x"));
+ assert_eq!(s.eval::("return table.getn(seen)").unwrap(), 1);
+ assert_eq!(s.eval::("return seen[1]").unwrap(), "KbTip");
+ assert!(s.errors().is_empty(), "{:?}", s.errors());
+}
+
+/// A hidden frame is in no bucket (the link gate the walk shares with the draw order), so it
+/// neither fires nor consumes — and the frame below it gets the key instead.
+#[test]
+fn a_hidden_frame_is_not_in_the_walk() {
+ let mut s = script();
+ s.run(
+ r#"
+ winner = nil
+ function mk(name, strata)
+ local f = CreateFrame("Frame", name)
+ f:SetFrameStrata(strata)
+ f:EnableKeyboard(true)
+ f:SetScript("OnChar", function() winner = name end)
+ return f
+ end
+ top = mk("KbHidden", "TOOLTIP")
+ mk("KbBelow", "DIALOG")
+ top:Hide()
+ "#,
+ )
+ .unwrap();
+ assert!(s.char_input("q"));
+ assert_eq!(s.eval::("return winner").unwrap(), "KbBelow");
+ assert!(s.errors().is_empty(), "{:?}", s.errors());
+}
+
+/// A raising handler must not eat the key or abort the walk's caller: the error is recorded, the
+/// frame still consumed (consumption is the C++ gate's, never the handler's — §3.1).
+#[test]
+fn a_raising_handler_still_consumes_and_is_recorded() {
+ let mut s = script();
+ s.run(
+ r#"
+ f = CreateFrame("Frame", "KbBoom")
+ f:EnableKeyboard(true)
+ f:SetScript("OnChar", function() error("boom") end)
+ "#,
+ )
+ .unwrap();
+ assert!(s.char_input("z"), "the gate consumed regardless");
+ assert!(
+ s.errors().iter().any(|e| e.contains("boom")),
+ "the raise is recorded: {:?}",
+ s.errors()
+ );
+}
diff --git a/crates/benilla-ui/src/script/tests/mod.rs b/crates/benilla-ui/src/script/tests/mod.rs
index 94d6a66f..1e6623de 100644
--- a/crates/benilla-ui/src/script/tests/mod.rs
+++ b/crates/benilla-ui/src/script/tests/mod.rs
@@ -17,6 +17,7 @@ mod font_object;
mod frame_api;
mod generic_for;
mod input;
+mod keyboard;
mod layout_gate;
mod measure;
mod minimap;
diff --git a/third_party/lua-src/BENILLA.md b/third_party/lua-src/BENILLA.md
index 0909aea0..347d032c 100644
--- a/third_party/lua-src/BENILLA.md
+++ b/third_party/lua-src/BENILLA.md
@@ -35,7 +35,7 @@ downgrade, and it is the entire reason this is a one-hunk fork rather than a ven
## The delta, and the command that proves it
-**Current state: two hunks, in two files.** `src/lib.rs` additionally differs by having Lua
+**Current state: three hunks, in three files.** `src/lib.rs` additionally differs by having Lua
5.2/5.3/5.4/5.5 stripped from the `Version` enum (with their source trees deleted) — benilla builds
5.1 and only 5.1, and a fork that still *offered* the other four would answer a request for one with
a missing directory at build time instead of a compile error.
@@ -44,6 +44,7 @@ a missing directory at build time instead of a compile error.
|---|---|---|---|
| `lvm.c` | 5.0's `OP_TFORPREP` table→`next` substitution, folded into the top of `OP_TFORLOOP` | `for k, v in someTable do` raised "attempt to call a table value"; it was the first session-start error for **60** corpus addons | 1215 |
| `luaconf.h` | `LUA_COMPAT_LSTR` `1` → `2` | 5.1 kept 5.0's nesting machinery and put an advisory error in front of it; two corpus addons died on "nesting of `[[...]]` is deprecated" | — |
+| `lparser.c` | 5.0's compat-semicolon skip restored at the top of `constructor()`'s field loop — 5.0's own line, verbatim, comment included | one extra `;` after a field separator inside a table constructor (`Back_Title = AL["Factions"];;`, ×20 in AtlasLoot's `ButtonRegistry.lua`) parses on the client and died here with "unexpected symbol near `;`"; statement-level `;;` stays rejected by both dialects | 1315 |
The ordering was deliberate and it held: the fork landed **inert**, byte-identical to upstream, so
the build pipeline was proven transparent *before* any behavioural payload went into it — and when
@@ -59,7 +60,7 @@ diff -r third_party/lua-src/lua-5.1.5 \
~/.cargo/registry/src/*/lua-src-550.0.0/lua-5.1.5
```
-That must print **exactly** the two hunks in the table above and nothing else. If it ever prints
+That must print **exactly** the three hunks in the table above and nothing else. If it ever prints
more, something crept in.
## Status of the payload
diff --git a/third_party/lua-src/lua-5.1.5/lparser.c b/third_party/lua-src/lua-5.1.5/lparser.c
index dda7488d..a6dde674 100644
--- a/third_party/lua-src/lua-5.1.5/lparser.c
+++ b/third_party/lua-src/lua-5.1.5/lparser.c
@@ -509,6 +509,23 @@ static void constructor (LexState *ls, expdesc *t) {
checknext(ls, '{');
do {
lua_assert(cc.v.k == VVOID || cc.tostore > 0);
+ /* BENILLA: Lua 5.0's constructor compat-semicolon, restored (decision 1315;
+ ** see third_party/lua-src/BENILLA.md). 5.0 opened this loop with exactly the
+ ** line below -- its own comment read "compatibility only", a Lua 4.0 leftover
+ ** (4.0 separated a constructor's list part from its record part with `;`).
+ ** 5.1 deleted the line, so one extra `;` after a field separator -- the
+ ** `Back_Title = AL["Factions"];;` shape AtlasLoot's ButtonRegistry.lua writes
+ ** twenty times -- became "unexpected symbol near ';'" here while the 1.12.1
+ ** client (Lua 5.0) accepts it. Statement-level `;;` stays rejected by both,
+ ** which is the half wow-5875-re byte-verified first (system/ui/scratch/
+ ** lua-dialect.md 9.1), and byte-verified in the client itself rather than
+ ** inferred from stock 5.0: constructor is 0x6fd430, and its loop head
+ ** 0x6fd4a0 calls testnext(ls,';') WITHOUT testing EAX -- a skip, not a
+ ** separator check -- while the loop's continuation at 0x6fd4f5/0x6fd505 does
+ ** test it. Of the 16 call sites of testnext 0x6fccf0 image-wide, the only
+ ** other consumer of `;' is chunk 0x6fcccc, at STATEMENT level, with no such
+ ** head skip. This line restores the constructor context only. */
+ testnext(ls, ';'); /* compatibility only */
if (ls->t.token == '}') break;
closelistfield(fs, &cc);
switch(ls->t.token) {