diff --git a/crates/benilla-app/assets/ui/OptionsFrame.xml b/crates/benilla-app/assets/ui/OptionsFrame.xml
index 644fa37c..b1a4a83b 100644
--- a/crates/benilla-app/assets/ui/OptionsFrame.xml
+++ b/crates/benilla-app/assets/ui/OptionsFrame.xml
@@ -116,13 +116,21 @@
rows, reflowed — values, writes and dependencies all work mid-search. The two
strings ("Search Results", "No results found.") are ours: the era's
SETTINGS_SEARCH_* globals live in the client binary, not the dump.
- - **No scroll bars on the CVar pages**: the static category tree (9 rows, 2 headers) fits
- its 199px column outright, and the deepest CVar page (Audio, 7 rows) fits its area.
- MinimalScrollBar arrives when one outgrows the space. The KEYBINDINGS page (decision
- 1008 — the 1.15 Settings panel's own Keybindings category, module + provenance in
- KeyBindingsPage.xml) is the one page that can't fit and rides the shared faux-scroll
- kit; its search results reflow live binding rows under a Keybindings group head whose
- click jumps to the page (the era redirect law), capped at the static pool's size.
+ - **The PAGE AREA scrolls; the category tree does not.** The static tree (9 rows, 2
+ headers) fits its 199px column outright, and every CVar page fits its area, so nothing
+ on a settled page has a bar. The page area is a real ScrollFrame all the same (B217 —
+ the era's SettingsList is a ScrollBox with a MinimalScrollBar, and results and pages
+ ride the same one): a SEARCH chains group heads and matched rows from every category
+ into one column, which for a broad query outran the page and drew out through the
+ window onto the world. The body is the scroll child, sized to the visible rect until
+ the content outgrows it (OptionsScroll_Fit). The KEYBINDINGS page (decision 1008 — the
+ 1.15 Settings panel's own Keybindings category, module + provenance in
+ KeyBindingsPage.xml) keeps its OWN faux-scroll list inside that body — it fills the
+ body exactly, so the outer scroll's range on it is 0 and the two bars are never live
+ together; its search results reflow live binding rows under a Keybindings group head
+ whose click jumps to the page (the era redirect law), capped at the static pool's size.
+ Both bars wear the shared kit's trough (the recessed channel — 1.12 authors that art
+ per window, so it is opt-in there and declared beside each bar here).
- **Setting-page cuts (0957 Audio, 0959 Graphics)**: setting-row tooltips (the era
OPTION_TOOLTIP_* hovers) are not wired; Defaults resets the VISIBLE page only (the
era confirm dialog is not modeled); era rows with no 1.12 CVar +
@@ -375,6 +383,67 @@
return order, groups
end
+ -- ── The page scroll (B217) ──────────────────────────────────────────────────────────
+ -- The body is the scroll child (the ScrollFrame above it), so what these two do is
+ -- decide how TALL it is: the content's own extent, or the visible page rect, whichever
+ -- is larger. Below the rect nothing changes at all — range 0, no bar, every page seated
+ -- exactly where its XML puts it — and above it the overflow becomes a scroll instead of
+ -- a spill.
+ --
+ -- The measured set is every element that takes a seat in the ONE layout law's chain:
+ -- each page's rows, the search group heads, and the keybinding result pool. All of them
+ -- hang off the body's TOP, so their extent is independent of the height being computed —
+ -- which is what makes this converge. Nothing anchored to the body's BOTTOM may join them
+ -- (the Keybindings page's output line is the live example): growing the body would move
+ -- it down, which would grow the body. IsVisible, not IsShown: a hidden page's rows keep
+ -- their own shown flag, and only what the player can actually see may set the height.
+ function OptionsScroll_ContentBottom()
+ local bottom = nil
+ local function take(name)
+ local f = getglobal(name)
+ if f and f:IsVisible() then
+ local b = f:GetBottom()
+ if b and (not bottom or b < bottom) then bottom = b end
+ end
+ end
+ for _, key in ipairs(OPTIONS_CATEGORY_KEYS) do
+ if OPTIONS_PAGE_ROWS[key] then
+ take("OptionsFrameContainerBodySearchHead" .. key)
+ for _, rkey in ipairs(OPTIONS_PAGE_ROWS[key]) do
+ take("OptionsFrameContainerBody" .. key .. rkey)
+ end
+ end
+ end
+ take("OptionsFrameContainerBodySearchHeadKeybindings")
+ for i = 1, KEYBINDINGS_SEARCH_ROWS do
+ take("OptionsFrameContainerBodyKeybindSearch" .. i)
+ end
+ return bottom
+ end
+
+ -- Size the body to that, from the scroll frame's OnUpdate. Frame-late by nature (rects
+ -- resolve after Lua runs, so a reflow answers a frame later — the OptionsTab_FitWidth
+ -- idiom), and write-gated: SetWidth/SetHeight invalidate layout, so an every-frame write
+ -- would fight the resolver. UpdateScrollChildRect re-drives the bar through
+ -- OnScrollRangeChanged, which is what shows and hides it (and its trough).
+ function OptionsScroll_Fit()
+ local sf = OptionsFrameContainerScroll
+ local body = OptionsFrameContainerBody
+ local fw, fh = sf:GetWidth(), sf:GetHeight()
+ if not fw or fw <= 0 or not fh or fh <= 0 then return end
+ if math.abs(body:GetWidth() - fw) >= 0.5 then body:SetWidth(fw) end
+ local h = fh
+ local top, bottom = body:GetTop(), OptionsScroll_ContentBottom()
+ if top and bottom then
+ local content = (top - bottom) + BENILLA_SCROLL_PAD
+ if content > h then h = content end
+ end
+ if math.abs(body:GetHeight() - h) >= 0.5 then
+ body:SetHeight(h)
+ sf:UpdateScrollChildRect()
+ end
+ end
+
-- THE one layout law (era SettingsList: verticalPad 10, padLeft 25/padRight 0 →
-- first element at (10,-12) off the body, spacing 9 → chain at (0,-9)). The rows'
-- authored XML anchors state exactly this chain as the initial layout; search
@@ -412,6 +481,8 @@
-- search holds the page (l.684-688), and an empty result shows the no-results line
-- (l.680-682).
function OptionsSearch_Display(text)
+ -- A new result set starts at the top (the era's own ScrollBox reset on a search).
+ OptionsFrameContainerScroll:SetVerticalScroll(0)
for k in pairs(OPTIONS_PAGE_ROWS) do
getglobal("OptionsFrameContainerBody" .. k):Show()
for _, rkey in ipairs(OPTIONS_PAGE_ROWS[k]) do
@@ -490,6 +561,7 @@
-- selection re-applied — SelectCategory re-shows the one body, retitles, and brings
-- Defaults back (the era's no-text arm, l.655-658).
function OptionsSearch_Restore()
+ OptionsFrameContainerScroll:SetVerticalScroll(0)
for k in pairs(OPTIONS_PAGE_ROWS) do
OptionsPage_Layout(k)
getglobal("OptionsFrameContainerBodySearchHead" .. k):Hide()
@@ -1446,515 +1518,600 @@
-
-
-
-
-
-
-
-
-
+
+
+ self:SetScrollChild(OptionsFrameContainerBody)
+ OptionsScroll_Fit()
+ BenillaScroll_OnMouseWheel(this, arg1)
+ BenillaScroll_SyncBar(this)
+ BenillaScroll_SyncBar(this)
+
-
-
-
-
-
-
-
- OptionsRow_OnLoad(self, "deselectOnClick", "Sticky Targeting")
- self.invert = 1
-
-
- OptionsRow_OnLoad(self, "autoLootDefault", "Auto Loot")
-
- OptionsRow_OnLoad(self, "mouseInvertPitch", "Invert Mouse")
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+ OptionsRow_OnLoad(self, "deselectOnClick", "Sticky Targeting")
+ self.invert = 1
+
+
+ OptionsRow_OnLoad(self, "autoLootDefault", "Auto Loot")
+
+ OptionsRow_OnLoad(self, "mouseInvertPitch", "Invert Mouse")
+
+
+
+
+
+
+
+
+ OptionsRow_OnLoad(self, "MasterSoundEffects", "Enable All Sound")
+ self.soundQuirk = 1
+
+
+ OptionsRow_OnLoad(self, "MasterVolume", "Master Volume")
+
+
+ OptionsRow_OnLoad(self, "SoundVolume", "Sound Volume", 1)
+ -- era parentInitializer (search pulls Master in
+ -- above a matched child, SettingsPanel.lua l.665)
+ self.parentRow = "RowMaster"
+
+
+
+ OptionsRow_OnLoad(self, "MusicVolume", "Music Volume", 1)
+ self.parentRow = "RowMaster"
+
+
+
+ OptionsRow_OnLoad(self, "AmbienceVolume", "Ambience Volume", 1)
+ self.parentRow = "RowMaster"
+
+
+
+ OptionsRow_OnLoad(self, "EnableMusic", "Enable Music")
+ self.soundQuirk = 1
+
+
+
+ OptionsRow_OnLoad(self, "EnableAmbience", "Enable Ambience")
+ self.soundQuirk = 1
+
+
+
+
+
+
+
+
+
+ OptionsRow_OnLoad(self, "uiScale", "UI Scale")
+ OptionsSliderRow_Setup(self, 0.64, 1.0, 0.01)
+ -- era registers its uiscale row CommitFlag.Apply
+ -- (Graphics.lua l.684): the one row whose live
+ -- write would rescale the very slider mid-drag —
+ -- it stages and commits through the Apply button.
+ self.deferred = 1
+
+
+
+ OptionsRow_OnLoad(self, "WorldDetail", "Environment Detail")
+ -- The stop names are the era preset ladder's (its
+ -- quality dropdowns say Low/Medium/High); 1.12's
+ -- slider had no stop labels to quote.
+ OptionsDropdownRow_Setup(self, {
+ { text = "Low", value = "0" },
+ { text = "Medium", value = "1" },
+ { text = "High", value = "2" },
+ })
+
+
+
+
+
+
+
+
+ OptionsRow_OnLoad(self, "UnitNamePlayer", "Player Names")
+
+ OptionsRow_OnLoad(self, "UnitNameNPC", "NPC Names")
+
+ OptionsRow_OnLoad(self, "UnitNameOwn", "Show Own Name")
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+ KeyBindings_CharacterToggle(this)
+
+ GameTooltip:SetOwner(this, "ANCHOR_RIGHT")
+ GameTooltip:SetText(KeyBindings_String("CHARACTER_SPECIFIC_KEYBINDING_TOOLTIP",
+ "Click this to toggle between general key bindings and key bindings specific to this character."), nil, nil, nil, nil, 1)
+
+ GameTooltip:Hide()
+
+
+
+
+
+ getglobal(this:GetName() .. "Label"):SetText(
+ KeyBindings_String("CHARACTER_SPECIFIC_KEYBINDINGS", "Character Specific Key Bindings"))
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+ BenillaFauxScrollFrame_OnLoad(this)
+ this.updateFunc = KeyBindingsPage_Update
+
+
+
+
+
+
+
+ if not KeyBindingsPage.selected then
+ BenillaFauxScrollFrame_OnMouseWheel(getglobal(this:GetName() .. "ScrollFrame"), arg1)
+ end
+
+
+
+
+
+
+ self.categoryKey = "Controls"
+ getglobal(self:GetName() .. "Label"):SetText("Controls")
+
+
+
+ self.categoryKey = "Audio"
+ getglobal(self:GetName() .. "Label"):SetText("Audio")
+
+
+
+ self.categoryKey = "Graphics"
+ getglobal(self:GetName() .. "Label"):SetText("Graphics")
+
+
+
+ self.categoryKey = "Nameplates"
+ getglobal(self:GetName() .. "Label"):SetText("Nameplates")
+
+
+
+ self.categoryKey = "Keybindings"
+ getglobal(self:GetName() .. "Label"):SetText("Keybindings")
+
+
+
-
-
-
-
-
-
-
- OptionsRow_OnLoad(self, "MasterSoundEffects", "Enable All Sound")
- self.soundQuirk = 1
-
-
- OptionsRow_OnLoad(self, "MasterVolume", "Master Volume")
-
-
- OptionsRow_OnLoad(self, "SoundVolume", "Sound Volume", 1)
- -- era parentInitializer (search pulls Master in
- -- above a matched child, SettingsPanel.lua l.665)
- self.parentRow = "RowMaster"
-
-
-
- OptionsRow_OnLoad(self, "MusicVolume", "Music Volume", 1)
- self.parentRow = "RowMaster"
-
-
-
- OptionsRow_OnLoad(self, "AmbienceVolume", "Ambience Volume", 1)
- self.parentRow = "RowMaster"
-
-
-
- OptionsRow_OnLoad(self, "EnableMusic", "Enable Music")
- self.soundQuirk = 1
-
-
-
- OptionsRow_OnLoad(self, "EnableAmbience", "Enable Ambience")
- self.soundQuirk = 1
-
-
-
-
-
-
-
-
-
- OptionsRow_OnLoad(self, "uiScale", "UI Scale")
- OptionsSliderRow_Setup(self, 0.64, 1.0, 0.01)
- -- era registers its uiscale row CommitFlag.Apply
- -- (Graphics.lua l.684): the one row whose live
- -- write would rescale the very slider mid-drag —
- -- it stages and commits through the Apply button.
- self.deferred = 1
-
-
-
- OptionsRow_OnLoad(self, "WorldDetail", "Environment Detail")
- -- The stop names are the era preset ladder's (its
- -- quality dropdowns say Low/Medium/High); 1.12's
- -- slider had no stop labels to quote.
- OptionsDropdownRow_Setup(self, {
- { text = "Low", value = "0" },
- { text = "Medium", value = "1" },
- { text = "High", value = "2" },
- })
-
-
-
-
-
-
-
-
- OptionsRow_OnLoad(self, "UnitNamePlayer", "Player Names")
-
- OptionsRow_OnLoad(self, "UnitNameNPC", "NPC Names")
-
- OptionsRow_OnLoad(self, "UnitNameOwn", "Show Own Name")
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
- KeyBindings_CharacterToggle(this)
-
- GameTooltip:SetOwner(this, "ANCHOR_RIGHT")
- GameTooltip:SetText(KeyBindings_String("CHARACTER_SPECIFIC_KEYBINDING_TOOLTIP",
- "Click this to toggle between general key bindings and key bindings specific to this character."), nil, nil, nil, nil, 1)
-
- GameTooltip:Hide()
-
-
-
-
-
- getglobal(this:GetName() .. "Label"):SetText(
- KeyBindings_String("CHARACTER_SPECIFIC_KEYBINDINGS", "Character Specific Key Bindings"))
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
- BenillaFauxScrollFrame_OnLoad(this)
- this.updateFunc = KeyBindingsPage_Update
-
-
-
-
-
-
-
- if not KeyBindingsPage.selected then
- BenillaFauxScrollFrame_OnMouseWheel(getglobal(this:GetName() .. "ScrollFrame"), arg1)
- end
-
-
-
-
-
-
- self.categoryKey = "Controls"
- getglobal(self:GetName() .. "Label"):SetText("Controls")
-
-
-
- self.categoryKey = "Audio"
- getglobal(self:GetName() .. "Label"):SetText("Audio")
-
-
-
- self.categoryKey = "Graphics"
- getglobal(self:GetName() .. "Label"):SetText("Graphics")
-
-
-
- self.categoryKey = "Nameplates"
- getglobal(self:GetName() .. "Label"):SetText("Nameplates")
-
-
-
- self.categoryKey = "Keybindings"
- getglobal(self:GetName() .. "Label"):SetText("Keybindings")
-
-
- BenillaScrollBar_OnValueChanged(self, arg1)
-
+
diff --git a/crates/benilla-app/assets/ui/PetActionBar.xml b/crates/benilla-app/assets/ui/PetActionBar.xml
index b4be12ba..4800bc58 100644
--- a/crates/benilla-app/assets/ui/PetActionBar.xml
+++ b/crates/benilla-app/assets/ui/PetActionBar.xml
@@ -44,6 +44,15 @@
them off the same cursor transition that drives ACTIONBAR_SHOWGRID/HIDEGRID but on a
disjoint predicate, so a held spell lights the action bar's empty slots and a held pet
action lights this bar's — never both, because neither payload can land on the other bar.
+ - NO HotKey corner, and none is missing (1052). The ref's button inherits
+ ActionButtonTemplate's $parentHotKey and repaints it on UPDATE_BINDINGS — but
+ PetActionButton_SetHotkeys (l.311-319) writes the label only when
+ `bindingSuffix == this:GetID()`, a STRING against a NUMBER, which Lua never equates. So
+ the real 1.12 pet bar renders an empty hotkey on every slot, always, and the reporter's
+ own reference shot for B218 shows exactly that: CTRL-1..CTRL-5 bound in the window, no
+ glyph on any pet button. An empty corner is the faithful render; a filled one would be
+ the divergence. (Our own action bar's corners are real — ActionBar.xml — because the
+ ref's ActionButton_UpdateHotkeys has no such bug.)
- The autocast sparkle is TEXTURES, not the ref's . benilla has no Model widget
(the Cooldown and the bag ItemAnim took the same posture): model the mechanism, let our
own renderer draw it. The numbers below are the shipped model's own — see BENILLA_PET_
@@ -228,6 +237,31 @@
end
end
+ -- The KEYBIND pair (decision 1052) — ref PetActionButtonDown/Up l.218-231, which is where
+ -- the BONUSACTIONBUTTONn bindings land: their bodies call BonusActionButtonDown/Up, and
+ -- those are one-liners onto these (BonusActionBarFrame.lua:106-112). The ref's own
+ -- runOnUp shape: DOWN only shows the pushed art, UP fires.
+ --
+ -- Note what the key path does NOT do — it is a bare CastPetAction, with none of the click
+ -- path's forks: no shift pick-up, no right-button autocast, and no IsPetAttackActive
+ -- call-off. That is the reference's split, not a simplification of ours: the fork lives in
+ -- PetActionButton_OnClick, which a key press never reaches. So CTRL-1 on a pet already
+ -- attacking re-issues Attack where a left click would call it off.
+ function BenillaPetActionButtonDown(id)
+ local button = getglobal("BenillaPetActionButton" .. id)
+ if ( button:GetButtonState() == "NORMAL" ) then
+ button:SetButtonState("PUSHED")
+ end
+ end
+
+ function BenillaPetActionButtonUp(id)
+ local button = getglobal("BenillaPetActionButton" .. id)
+ if ( button:GetButtonState() == "PUSHED" ) then
+ button:SetButtonState("NORMAL")
+ CastPetAction(id)
+ end
+ end
+
-- Both drag ends are the SAME verb (ref l.269-283, byte-verified at `0x4be180`): with an
-- empty cursor it picks the slot up, with a pet payload already held it drops onto this
-- slot. There is no separate "place" binding on this bar, which is also why a spell or an
diff --git a/crates/benilla-app/assets/ui/ScrollTemplates.xml b/crates/benilla-app/assets/ui/ScrollTemplates.xml
index 14458f89..1a023928 100644
--- a/crates/benilla-app/assets/ui/ScrollTemplates.xml
+++ b/crates/benilla-app/assets/ui/ScrollTemplates.xml
@@ -1,7 +1,10 @@
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
BenillaSpellButton_OnLoad(self)
BenillaSpellButton_OnEvent(self)
- BenillaSpellButton_OnClick(self)
+ BenillaSpellButton_OnClick(self, nil, arg1)
BenillaSpellButton_UpdateButton(self)
@@ -542,6 +691,41 @@
+
+ ToggleSpellBook(self.bookType)
+
+ GameTooltip:SetOwner(self, "ANCHOR_RIGHT")
+ -- ref l.53: the label plus its bound key. MicroButtonTooltipText is MicroMenu.xml's
+ -- (loaded before this file); nil-guarded like every other cross-file call here.
+ if MicroButtonTooltipText then
+ GameTooltip:SetText(MicroButtonTooltipText(self:GetText(), self.binding), 1.0, 1.0, 1.0)
+ else
+ GameTooltip:SetText(self:GetText(), 1.0, 1.0, 1.0)
+ end
+ GameTooltip:Show()
+
+ GameTooltip:Hide()
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
@@ -719,6 +928,7 @@
BenillaSpellBookFrame_OnEvent(self)
BenillaSpellBookFrame_OnShow()
BenillaSpellBookFrame_OnHide()
+ BenillaSpellBookFrame_OnUpdate(self, arg1)
diff --git a/crates/benilla-app/src/assets/mod.rs b/crates/benilla-app/src/assets/mod.rs
index 0dbc0e0e..974150cb 100644
--- a/crates/benilla-app/src/assets/mod.rs
+++ b/crates/benilla-app/src/assets/mod.rs
@@ -91,6 +91,20 @@ pub(crate) fn normalize_path(path: &str) -> String {
path.replace('/', "\\").to_ascii_lowercase()
}
+/// The archive key a UI **sprite** reference resolves to: [`normalize_path`] plus the FrameXML
+/// extensionless→`.blp` rule (`Interface\Buttons\UI-Quickslot2` → `interface\buttons\ui-quickslot2.blp`;
+/// a path that already carries an extension passes through). One function because every sprite
+/// entry point needs it — the five decoders below and, deliberately, the shipped-XML sweep that
+/// asserts each `file=` in our own UI actually resolves (`ui_script::shipped_xml_tests`): a sweep
+/// re-implementing this rule could agree with itself while disagreeing with the renderer.
+pub(crate) fn sprite_key(path: &str) -> String {
+ let mut key = normalize_path(path);
+ if !key.rsplit('\\').next().is_some_and(|f| f.contains('.')) {
+ key.push_str(".blp");
+ }
+ key
+}
+
/// Mutate an asset only when `differs` says its current state doesn't already match.
///
/// `Assets::get_mut` alone marks the asset Modified — a uniform re-upload and (on the Metal
@@ -181,10 +195,7 @@ impl WorldAssets {
// FrameXML/DBC texture references are canonically EXTENSIONLESS
// (`Interface\Buttons\UI-Quickslot2`) — the real client appends `.blp` at resolve. Paths
// that already carry an extension (`textures\moon.blp`) pass through unchanged.
- let mut key = normalize_path(path);
- if !key.rsplit('\\').next().is_some_and(|f| f.contains('.')) {
- key.push_str(".blp");
- }
+ let key = sprite_key(path);
// Cached (hits AND misses): the UI extractor asks per quad per frame — uncached, every
// frame re-decoded every BLP into a fresh `Image` (unbounded asset growth) and the fresh
// handles defeated the quad-list dirty check.
@@ -203,10 +214,7 @@ impl WorldAssets {
/// which resizes the 128×32 frame to the plate's exact size). Same extensionless→`.blp` resolve
/// as [`Self::sprite_texture`]; not cached (a one-shot decode at load, not a per-frame ask).
pub(crate) fn decode_rgba(&mut self, path: &str) -> Option<(u32, u32, Vec)> {
- let mut key = normalize_path(path);
- if !key.rsplit('\\').next().is_some_and(|f| f.contains('.')) {
- key.push_str(".blp");
- }
+ let key = sprite_key(path);
read_texture_rgba(&mut self.chain.lock_recover(), &key).ok()
}
@@ -220,10 +228,7 @@ impl WorldAssets {
path: &str,
images: &mut Assets,
) -> Option> {
- let mut key = normalize_path(path);
- if !key.rsplit('\\').next().is_some_and(|f| f.contains('.')) {
- key.push_str(".blp");
- }
+ let key = sprite_key(path);
if let Some(cached) = self.tiled_sprites.get(&key) {
return cached.clone();
}
@@ -243,10 +248,7 @@ impl WorldAssets {
path: &str,
images: &mut Assets,
) -> Option> {
- let mut key = normalize_path(path);
- if !key.rsplit('\\').next().is_some_and(|f| f.contains('.')) {
- key.push_str(".blp");
- }
+ let key = sprite_key(path);
if let Some(cached) = self.portraits.get(&key) {
return cached.clone();
}
@@ -265,10 +267,7 @@ impl WorldAssets {
path: &str,
images: &mut Assets,
) -> Option> {
- let mut key = normalize_path(path);
- if !key.rsplit('\\').next().is_some_and(|f| f.contains('.')) {
- key.push_str(".blp");
- }
+ let key = sprite_key(path);
if let Some(cached) = self.masks.get(&key) {
return cached.clone();
}
diff --git a/crates/benilla-app/src/bindings.rs b/crates/benilla-app/src/bindings.rs
index ee5cf8ea..26333c17 100644
--- a/crates/benilla-app/src/bindings.rs
+++ b/crates/benilla-app/src/bindings.rs
@@ -671,6 +671,40 @@ mod tests {
assert!(!state(&app).fired(cmd::TOGGLE_SHEATH));
}
+ /// **The pet lane routes on the CTRL digits, and the number row is untouched** (B218,
+ /// decision 1052). The two share their base keys, so the only thing keeping them apart is
+ /// the exact-modifier law — worth pinning on the pair that actually collides rather than
+ /// trusting the law in the abstract. CTRL-0 is slot **10**, the 1.12 cache's own wrap.
+ #[test]
+ fn the_pet_lane_dispatches_on_the_ctrl_digits() {
+ let by_name =
+ |n: &str| Cmd(SPECS.iter().position(|s| s.name == n).expect("registered") as u16);
+ let mut app = harness();
+ press_key(&mut app, KeyCode::ControlLeft);
+ press_key(&mut app, KeyCode::Digit1);
+ app.update();
+ assert!(state(&app).pressed(by_name("BONUSACTIONBUTTON1")));
+ assert!(
+ !state(&app).pressed(by_name("ACTIONBUTTON1")),
+ "the modifier decides: CTRL-1 is not the number row's"
+ );
+ // The runOnUp half: the latch drops on the BASE key's release (which is what runs
+ // BenillaPetActionButtonUp → CastPetAction in the app's VM), Ctrl still held.
+ release_key(&mut app, KeyCode::Digit1);
+ app.update();
+ assert!(!state(&app).pressed(by_name("BONUSACTIONBUTTON1")));
+ // CTRL-0 → slot 10.
+ press_key(&mut app, KeyCode::Digit0);
+ app.update();
+ assert!(state(&app).pressed(by_name("BONUSACTIONBUTTON10")));
+ // Bare 1 is still the action bar's, with no pet command in sight.
+ let mut app = harness();
+ press_key(&mut app, KeyCode::Digit1);
+ app.update();
+ assert!(state(&app).pressed(by_name("ACTIONBUTTON1")));
+ assert!(!state(&app).pressed(by_name("BONUSACTIONBUTTON1")));
+ }
+
#[test]
fn the_typing_gate_blocks_new_input_and_clears_held_latches_once() {
let mut app = harness();
diff --git a/crates/benilla-app/src/bindings/commands.rs b/crates/benilla-app/src/bindings/commands.rs
index 9d984da5..6112ba88 100644
--- a/crates/benilla-app/src/bindings/commands.rs
+++ b/crates/benilla-app/src/bindings/commands.rs
@@ -85,15 +85,15 @@ pub(crate) mod cmd {
pub(crate) const OPEN_CHAT: Cmd = Cmd(12);
pub(crate) const OPEN_CHAT_SLASH: Cmd = Cmd(13);
pub(crate) const REPLY: Cmd = Cmd(17);
- pub(crate) const TARGET_NEAREST_ENEMY: Cmd = Cmd(40);
- pub(crate) const TARGET_PREVIOUS_ENEMY: Cmd = Cmd(41);
- pub(crate) const NAMEPLATES: Cmd = Cmd(52);
- pub(crate) const FRIEND_NAMEPLATES: Cmd = Cmd(53);
- pub(crate) const ALL_NAMEPLATES: Cmd = Cmd(54);
- pub(crate) const ATTACK_TARGET: Cmd = Cmd(55);
- pub(crate) const TOGGLE_UI: Cmd = Cmd(76);
- pub(crate) const CAMERA_ZOOM_IN: Cmd = Cmd(77);
- pub(crate) const CAMERA_ZOOM_OUT: Cmd = Cmd(78);
+ pub(crate) const TARGET_NEAREST_ENEMY: Cmd = Cmd(50);
+ pub(crate) const TARGET_PREVIOUS_ENEMY: Cmd = Cmd(51);
+ pub(crate) const NAMEPLATES: Cmd = Cmd(62);
+ pub(crate) const FRIEND_NAMEPLATES: Cmd = Cmd(63);
+ pub(crate) const ALL_NAMEPLATES: Cmd = Cmd(64);
+ pub(crate) const ATTACK_TARGET: Cmd = Cmd(65);
+ pub(crate) const TOGGLE_UI: Cmd = Cmd(86);
+ pub(crate) const CAMERA_ZOOM_IN: Cmd = Cmd(87);
+ pub(crate) const CAMERA_ZOOM_OUT: Cmd = Cmd(88);
}
/// The registry, 1.12 `Bindings.xml` order. Sub-tables (action buttons, shapeshift, raid
@@ -334,6 +334,114 @@ pub(crate) static SPECS: &[Spec] = &[
Some("CTRL-F10"),
None
),
+ // The PET bar's row — 1.12 calls it BONUSACTIONBUTTON and files it under this same ACTIONBAR
+ // header (Bindings.xml:321-390 carries no `header` attribute, so it inherits l.121's), which
+ // is why "Secondary Action Button 1-10" reads under Action Bar in the reference's own window.
+ // The ref's body is BonusActionButtonDown/Up(id), and those two are one-liners onto
+ // PetActionButtonDown/Up (BonusActionBarFrame.lua:106-112) — the PET bar's buttons, not the
+ // bonus bar's. benilla has no bonus bar at all, so the pet bar is the whole lane, exactly as
+ // the reference wires it. Defaults are byte-real and unanimous: CTRL-1..CTRL-0 in all three
+ // of the install's `bindings-cache.wtf` files (unlike TOGGLEUI's rebind, 0870).
+ spec!(
+ "BONUSACTIONBUTTON1",
+ ACTIONBAR,
+ Kind::EdgeUpDown(
+ "BenillaPetActionButtonDown(1)",
+ "BenillaPetActionButtonUp(1)"
+ ),
+ Some("CTRL-1"),
+ None
+ ),
+ spec!(
+ "BONUSACTIONBUTTON2",
+ ACTIONBAR,
+ Kind::EdgeUpDown(
+ "BenillaPetActionButtonDown(2)",
+ "BenillaPetActionButtonUp(2)"
+ ),
+ Some("CTRL-2"),
+ None
+ ),
+ spec!(
+ "BONUSACTIONBUTTON3",
+ ACTIONBAR,
+ Kind::EdgeUpDown(
+ "BenillaPetActionButtonDown(3)",
+ "BenillaPetActionButtonUp(3)"
+ ),
+ Some("CTRL-3"),
+ None
+ ),
+ spec!(
+ "BONUSACTIONBUTTON4",
+ ACTIONBAR,
+ Kind::EdgeUpDown(
+ "BenillaPetActionButtonDown(4)",
+ "BenillaPetActionButtonUp(4)"
+ ),
+ Some("CTRL-4"),
+ None
+ ),
+ spec!(
+ "BONUSACTIONBUTTON5",
+ ACTIONBAR,
+ Kind::EdgeUpDown(
+ "BenillaPetActionButtonDown(5)",
+ "BenillaPetActionButtonUp(5)"
+ ),
+ Some("CTRL-5"),
+ None
+ ),
+ spec!(
+ "BONUSACTIONBUTTON6",
+ ACTIONBAR,
+ Kind::EdgeUpDown(
+ "BenillaPetActionButtonDown(6)",
+ "BenillaPetActionButtonUp(6)"
+ ),
+ Some("CTRL-6"),
+ None
+ ),
+ spec!(
+ "BONUSACTIONBUTTON7",
+ ACTIONBAR,
+ Kind::EdgeUpDown(
+ "BenillaPetActionButtonDown(7)",
+ "BenillaPetActionButtonUp(7)"
+ ),
+ Some("CTRL-7"),
+ None
+ ),
+ spec!(
+ "BONUSACTIONBUTTON8",
+ ACTIONBAR,
+ Kind::EdgeUpDown(
+ "BenillaPetActionButtonDown(8)",
+ "BenillaPetActionButtonUp(8)"
+ ),
+ Some("CTRL-8"),
+ None
+ ),
+ spec!(
+ "BONUSACTIONBUTTON9",
+ ACTIONBAR,
+ Kind::EdgeUpDown(
+ "BenillaPetActionButtonDown(9)",
+ "BenillaPetActionButtonUp(9)"
+ ),
+ Some("CTRL-9"),
+ None
+ ),
+ spec!(
+ "BONUSACTIONBUTTON10",
+ ACTIONBAR,
+ Kind::EdgeUpDown(
+ "BenillaPetActionButtonDown(10)",
+ "BenillaPetActionButtonUp(10)"
+ ),
+ Some("CTRL-0"),
+ None
+ ),
// ── Targeting (BINDING_HEADER_TARGETING) ────────────────────────────────────────────
spec!(
"TARGETNEARESTENEMY",
@@ -988,6 +1096,23 @@ mod tests {
}
}
+ /// No two commands ship the SAME default chord. One key, one command is the binding table's
+ /// own law (the steal pass in [`super::store::resolve`] enforces it at load), so a duplicate
+ /// here would ship a table that eats itself: registration order would decide the winner and
+ /// the loser would come up silently unbound. Cheap tripwire for the one mistake a new block
+ /// of rows can make — 1052's CTRL-1..CTRL-0 went in with nothing to catch a collision.
+ #[test]
+ fn no_two_commands_ship_the_same_default_chord() {
+ let mut seen: std::collections::HashMap<&str, &str> = std::collections::HashMap::new();
+ for s in SPECS {
+ for d in [s.d1, s.d2].into_iter().flatten() {
+ if let Some(prev) = seen.insert(d, s.name) {
+ panic!("default '{d}' is on both {prev} and {}", s.name);
+ }
+ }
+ }
+ }
+
/// Names are unique (the table is keyed by them everywhere: files, Lua, the window).
#[test]
fn names_are_unique() {
diff --git a/crates/benilla-app/src/capture/probes.rs b/crates/benilla-app/src/capture/probes.rs
index fd867a34..c0bd9f58 100644
--- a/crates/benilla-app/src/capture/probes.rs
+++ b/crates/benilla-app/src/capture/probes.rs
@@ -175,8 +175,13 @@ fn probe_key_by_name(name: &str) -> Option {
// Added because the harness could not reach the leg that broke decision 0793: the director's
// first real run was a boosted free-fly, whose camera crosses the art radius in ~5 s, and
// reproducing it needed a held `Ctrl` + `W` behind an `F`.
+ //
+ // The toggle is the dev chord + `F` since decision 1043 (it was a bare `F`), so a probe that
+ // wants free-fly holds `Ctrl` and `Shift` across the `F` tap — hence `Shift` here. `Ctrl`
+ // stays held afterwards for the ×5 boost, which is unchanged.
"F" => KeyCode::KeyF,
"Ctrl" => KeyCode::ControlLeft,
+ "Shift" => KeyCode::ShiftLeft,
_ => return None,
})
}
diff --git a/crates/benilla-app/src/creature_anim.rs b/crates/benilla-app/src/creature_anim.rs
index c97fb152..85d56a1b 100644
--- a/crates/benilla-app/src/creature_anim.rs
+++ b/crates/benilla-app/src/creature_anim.rs
@@ -274,6 +274,136 @@ pub(crate) fn stop_attack_local(
}
}
+/// The one local **StartAttack** — benilla's `0x5ecb70`, [`stop_attack_local`]'s twin (wow-re §5
+/// `melee-autorepeat-exclusion.md` §5c–§5f). Two halves, and keeping them apart is the whole point:
+///
+/// - **the send** (`0x5eccfd`) is gated by the already-attacking test at `0x5eccda` — skipped when
+/// the attack lock is held, *unless* a stop is in flight (`[+0xc54]`), which is why
+/// stop → select → re-swing re-points at a new target instead of going quiet;
+/// - **the tail** (`0x5ecd78`–`0x5ecd95`) — melee sheath SNAP then the local auto-repeat cancel
+/// `0x6ea080` — runs either way, because the skip's `jmp` lands *inside* it. That is what makes
+/// "start swinging" and "keep auto-shooting" mutually exclusive.
+///
+/// `stop_in_flight` is a **parameter, not stored state**: the ref's `[+0xc54]` is set by
+/// [`stop_attack_local`] and consumed here, and the only caller that spans both is the target
+/// switch, which does them back to back in one call chain. Decision 1036 removed the persistent
+/// latch for exactly this reason — outside that chain it has no reader.
+///
+/// The caller owes the reference's own two entry conditions, which differ per site and are not
+/// re-derived here: a cast press must not call this at all while engaged (`TryCast`'s `6e51cb`/
+/// `6e51d2` gate), and the Attack button forks to [`stop_attack_local`] instead (`0x6131a0` @
+/// `0x6131d9`). `0x5ecb70`'s own validator legs — the target's alive-or-feign + `CanAttack
+/// 0x606980` walk, and the `[0xb4b3e4]` world gate — stay with the callers that already compute
+/// them (`target::scan`'s `new_attackable`, the drain's `attack_actor_refusal`).
+#[allow(clippy::too_many_arguments)] // the tail writes four sinks; the alternative is a bundle
+pub(crate) fn start_attack_local(
+ entity: Entity,
+ target: u64,
+ engaged: bool,
+ stop_in_flight: bool,
+ auto_repeat: &mut crate::ui_action::AutoRepeatActive,
+ sheath: &mut MessageWriter,
+ commands: &mut Commands,
+ net: &crate::net::NetCommands,
+) {
+ if !engaged || stop_in_flight {
+ let _ = net
+ .0
+ .send(crate::net::ClientCommand::AttackSwing { guid: target });
+ }
+ // The tail. The sheath setter is idempotent — the client's own "no-op if already melee" — so
+ // re-running it on a re-point costs nothing and matches `0x5ecd80`'s unconditional call.
+ sheath.write(SheathRequest {
+ entity,
+ state: 1,
+ ceremony: false,
+ });
+ cancel_auto_repeat_local(Some(entity), auto_repeat, commands, net);
+}
+
+/// The **Attack toggle** — benilla's `0x6131a0`, the third of the trio and the one the base Attack
+/// pseudo-spell runs (`TryCast`'s effect-`0x4e` short-circuit at `0x6e4c7a` dispatches straight to
+/// it, ahead of every cast gate). It forks on the attack lock and nothing else: already attacking →
+/// `0x6131d9 call 0x5ecac0` [`stop_attack_local`], otherwise `0x6131ee call 0x5ecb70`
+/// [`start_attack_local`] (wow-re `melee-autorepeat-exclusion.md` §5f).
+///
+/// The fork is the whole content, and it is worth a name because both halves used to be wrong here:
+/// the button never toggled melee OFF, and it cancelled a running auto-repeat on *every* press —
+/// but only the start arm reaches `0x5ecd8c`, so in the reference toggling melee off leaves Auto
+/// Shot running.
+///
+/// `[0xb4b3e4]` — the reference's second condition on the stop arm, a world/session global also
+/// tested at `0x5ecbc0` — is unmodelled; nothing in benilla can be false there while a press is
+/// being drained.
+#[allow(clippy::too_many_arguments)] // the seams' write set, minus the bundle a caller can't hold
+pub(crate) fn toggle_attack_local(
+ entity: Entity,
+ target: u64,
+ engaged: bool,
+ queued_melee: &mut crate::ui_cast::QueuedMeleeSpell,
+ auto_repeat: &mut crate::ui_action::AutoRepeatActive,
+ sheath: &mut MessageWriter,
+ commands: &mut Commands,
+ net: &crate::net::NetCommands,
+) {
+ if engaged {
+ stop_attack_local(engaged, queued_melee, net);
+ } else {
+ start_attack_local(
+ entity,
+ target,
+ engaged,
+ false,
+ auto_repeat,
+ sheath,
+ commands,
+ net,
+ );
+ }
+}
+
+/// **The local attack seams' whole write set, as ONE [`SystemParam`]** — so a caller threading
+/// [`stop_attack_local`]/[`start_attack_local`] through a call chain takes one param instead of
+/// six, and so a new sink in either seam lands in every caller at once.
+///
+/// The targeting side needs exactly this: `target::scan`'s `commit` runs the reference's
+/// stop → select → re-swing law (`SetSelection 0x493540`'s own `0x493a08 call 0x5ecac0` and
+/// `0x4938c8 call 0x5ecb70`), so every selection writer that can fire it has to carry both seams'
+/// inputs. [`crate::ui_action::cast_send::CastLadder`] already carries them field by field for the
+/// cast path and calls the free functions directly.
+#[derive(bevy::ecs::system::SystemParam)]
+pub(crate) struct AttackSeam<'w, 's> {
+ pub(crate) net: Res<'w, crate::net::NetCommands>,
+ pub(crate) queued_melee: ResMut<'w, crate::ui_cast::QueuedMeleeSpell>,
+ pub(crate) auto_repeat: ResMut<'w, crate::ui_action::AutoRepeatActive>,
+ pub(crate) sheath: MessageWriter<'w, SheathRequest>,
+ pub(crate) ecs: Commands<'w, 's>,
+ /// Our own entity — the sheath snap's and the auto-repeat cancel's subject.
+ pub(crate) me: Query<'w, 's, Entity, With>,
+}
+
+impl AttackSeam<'_, '_> {
+ /// `0x5ecac0` — see [`stop_attack_local`].
+ pub(crate) fn stop(&mut self, engaged: bool) {
+ stop_attack_local(engaged, &mut self.queued_melee, &self.net);
+ }
+
+ /// `0x5ecb70` — see [`start_attack_local`]. A no-op before our own entity streams in.
+ pub(crate) fn start(&mut self, target: u64, engaged: bool, stop_in_flight: bool) {
+ let Ok(e) = self.me.single() else { return };
+ start_attack_local(
+ e,
+ target,
+ engaged,
+ stop_in_flight,
+ &mut self.auto_repeat,
+ &mut self.sheath,
+ &mut self.ecs,
+ &self.net,
+ );
+ }
+}
+
/// A unit is mid-cast (`SMSG_SPELL_START` .. `GO`/`FAILED_OTHER`, decision 0099 phase 1) — the
/// wire-truth state seam. Inserted on `SpellStart` only when it carries a nonzero cast time (an
/// instant cast's `SpellGo` follows with nothing to interrupt, so it never gets one); removed on
diff --git a/crates/benilla-app/src/creature_anim/driver/tests.rs b/crates/benilla-app/src/creature_anim/driver/tests.rs
index 80944767..b5b5a3aa 100644
--- a/crates/benilla-app/src/creature_anim/driver/tests.rs
+++ b/crates/benilla-app/src/creature_anim/driver/tests.rs
@@ -100,16 +100,58 @@ fn catalog() -> AnimData {
]))
}
+/// What a bare `app.update()` advances the harness clock by — small and nonzero, which is the
+/// regime every assertion in this file was written against (a real frame used to land here by
+/// accident). Zero is NOT equivalent: a frame with no delta never ticks a transition at all, so
+/// clips the tests expect to find mid-fade are never started.
+const FRAME_STEP: std::time::Duration = std::time::Duration::from_millis(1);
+
+/// The delta [`step_clock`] will use for the next frame, when a test asked for more than
+/// [`FRAME_STEP`]. It has to travel as a resource rather than a plain pre-`update()` call on
+/// `Time`, because `Time::advance_by` *sets* the frame's delta rather than accumulating it — so a
+/// value written before `update()` is simply overwritten by the in-frame step.
+#[derive(Resource, Default)]
+struct NextStep(Option);
+
+/// The harness's whole clock, in place of `TimePlugin`'s real one (see [`app`]): one frame of
+/// [`FRAME_STEP`], or of whatever [`advance`] asked for.
+fn step_clock(mut time: ResMut, mut next: ResMut) {
+ time.advance_by(next.0.take().unwrap_or(FRAME_STEP));
+}
+
+/// Run one frame whose delta is exactly `ms` — the only way time moves further than
+/// [`FRAME_STEP`] (see [`app`]). Replaces `thread::sleep` + `update()`: the same intent, but the
+/// delta is the number written here rather than however long the machine happened to take.
+fn advance(app: &mut App, ms: u64) {
+ app.world_mut().resource_mut::().0 = Some(std::time::Duration::from_millis(ms));
+ app.update();
+}
+
fn app() -> App {
let mut app = App::new();
// Asset + animation plugins so tests with REAL clip assets (the watchdog test) get Bevy's
// `advance_animations` ticking completions; units without a graph handle are skipped by it,
// so the asset-less tenants are unaffected.
+ //
+ // **`TimePlugin` is deliberately disabled, and the clock is ours** ([`step_clock`]). With it,
+ // `Time`'s delta is the REAL gap between two `app.update()` calls, and this file is full of
+ // assertions that a fade or clip is still mid-flight. A stalled frame — parallel `cargo test`
+ // on a loaded machine, a debugger, a cold page — then runs the whole fade out in one delta and
+ // the clip is gone from the player, which is a coin flip rather than a test: it red-lit main's
+ // gates on 2026-08-06 in `the_swim_relatch_holds_the_kick_but_a_ground_cut_freezes_it`, and
+ // injecting a 400 ms stall before that test's ground cut reproduces the failure exactly. The
+ // tests that DO need time to pass used to buy it with `thread::sleep`, which is the same coin
+ // flip pointed the other way; they now say how far the clock moves, through [`advance`].
+ //
+ // The step is small-but-nonzero rather than zero on purpose — see [`FRAME_STEP`].
app.add_plugins((
- MinimalPlugins,
+ MinimalPlugins.build().disable::(),
AssetPlugin::default(),
bevy::animation::AnimationPlugin,
));
+ app.init_resource::();
+ app.init_resource::();
+ app.add_systems(bevy::app::First, step_clock);
app.add_message::()
.add_message::()
.add_message::()
@@ -1232,8 +1274,7 @@ fn a_looping_arm_advances_through_its_variations_at_window_end() {
let mut seen = std::collections::HashSet::new();
for _ in 0..60 {
- std::thread::sleep(std::time::Duration::from_millis(25));
- app.update();
+ advance(&mut app, 25);
let tr = app
.world()
.entity(unit)
@@ -1684,8 +1725,7 @@ fn a_finished_masked_cast_fades_out_instead_of_snapping() {
// Step in small frames until the clip completes (a big frame would run the whole 150 ms fade
// out in one go — right behaviour, useless assertion).
for _ in 0..60 {
- std::thread::sleep(std::time::Duration::from_millis(20));
- app.update();
+ advance(&mut app, 20);
if app
.world()
.entity(unit)
@@ -1719,8 +1759,7 @@ fn a_finished_masked_cast_fades_out_instead_of_snapping() {
assert!(active.weight() > 0.0, "still blended in as λ decays");
}
// Past the 150 ms window: the slot self-releases, exactly like the kernel's `+0xd0 = -1`.
- std::thread::sleep(std::time::Duration::from_millis(200));
- app.update();
+ advance(&mut app, 200);
let e = app.world().entity(unit);
assert!(
e.get::().unwrap().overlay_fade.is_none(),
@@ -2079,8 +2118,7 @@ fn a_melee_to_ranged_toggle_stows_both_hands_before_it_reaches_for_the_bow() {
let mut seen = vec![[1u8, 1]];
let mut settled = false;
for _ in 0..60 {
- std::thread::sleep(std::time::Duration::from_millis(20));
- app.update();
+ advance(&mut app, 20);
match visual(&app) {
Some(v) if seen.last() != Some(&v) => seen.push(v),
None => {
diff --git a/crates/benilla-app/src/debug_panel/mod.rs b/crates/benilla-app/src/debug_panel/mod.rs
index 7bb939c7..44358c31 100644
--- a/crates/benilla-app/src/debug_panel/mod.rs
+++ b/crates/benilla-app/src/debug_panel/mod.rs
@@ -2,7 +2,7 @@
//!
//! The panel is a rounded, translucent **window floated off the top-right corner** (a gap from the
//! edges, like the perf pill) — it overlays the 3D view, which renders full-screen underneath and is
-//! never letterboxed. Fixed width (no resize handle). Press the backtick key (`` ` ``) to show/hide it.
+//! never letterboxed. Fixed width (no resize handle). [`DEV_CHORD`]+`D` shows/hides it (1043, 1048).
//! Its look (translucent dark backing + crisp text) is shared with the perf pill and the inspector card
//! via [`OVERLAY_FILL`] / [`overlay_text`], so the dev overlays read as one family.
//!
@@ -234,9 +234,10 @@ impl Plugin for DebugPanelPlugin {
.add_systems(
Update,
(
- // After the UI keyboard feed so a backtick typed into a focused EditBox never
- // toggles the panel.
- toggle_panel.after(crate::ui_script::UiInput),
+ // No ordering against the UI keyboard feed any more: the toggle is a dev chord
+ // (1043), which no focused EditBox can consume and which needs no capture gate —
+ // the same reason `perf`'s and `sound`'s toggles never needed one.
+ toggle_panel,
apply_model_visibility
.after(crate::wmo_portal::WmoPvsSet)
.in_set(ModelVisSet),
@@ -267,14 +268,14 @@ fn spawn_egui_camera(mut commands: Commands) {
));
}
-fn toggle_panel(
- keys: Res>,
- ui_capture: Res,
- mut debug: ResMut,
-) {
- // Don't toggle while the chat input is open or a focused EditBox owns the keyboard — a backtick
- // keypress there belongs in the text.
- if !ui_capture.0 && keys.just_pressed(KeyCode::Backquote) {
+fn toggle_panel(keys: Res>, mut debug: ResMut) {
+ // The dev chord + `D` (decision 1048). It was a *bare* backtick until 1043 — backtick reads as
+ // "not a game key", but it is one ([`crate::bindings::chord`] gives it the token `` ` ``, so the
+ // reference's binding UI can bind it like any other), which made a bare toggle here exactly the
+ // squat 0585 moved the perf HUD off `P` for. 1043 put it on the chord; `` ` `` is a bad key to
+ // hold a chord on, so it became a letter like the rest of the fleet. The chat-bar/EditBox gate
+ // the bare key needed is gone either way: a chord can't be mistaken for typed text.
+ if dev_chord(&keys, KeyCode::KeyD) {
debug.open = !debug.open;
}
}
@@ -284,7 +285,8 @@ fn toggle_panel(
/// what [`dev_chord`] actually listens for.
pub(crate) const DEV_CHORD: &str = "Ctrl+Shift";
-/// Did the **dev-overlay chord** — [`DEV_CHORD`]+*key* — just fire? (decisions 0585, 0867, 0870.)
+/// Did the **dev-overlay chord** — [`DEV_CHORD`]+*key* — just fire? (decisions 0585, 0867, 0870,
+/// 1043 — which moved the last two dev keys onto it, so the whole fleet is here now.)
///
/// The dev instruments used to sit on bare letters, which is a namespace we don't own: every letter is
/// a *game* binding in the reference client, so `P` both opened the spellbook and toggled the perf HUD
@@ -674,7 +676,7 @@ fn debug_panel_ui(
ui.separator();
ui.label(
egui::RichText::new(format!(
- "` panel · {DEV_CHORD}: P perf · I inspect · M mute"
+ "{DEV_CHORD}: D panel · P perf · I inspect · M mute · F free-fly"
))
.small()
.color(OVERLAY_TEXT_DIM),
diff --git a/crates/benilla-app/src/debug_panel/state.rs b/crates/benilla-app/src/debug_panel/state.rs
index eed4cfea..d4c983e4 100644
--- a/crates/benilla-app/src/debug_panel/state.rs
+++ b/crates/benilla-app/src/debug_panel/state.rs
@@ -11,7 +11,7 @@ use bevy::prelude::*;
/// (`open: false`), each section its own `Default` (so this derives cleanly).
#[derive(Resource, Default)]
pub struct DebugState {
- /// Panel visible? Hidden by default; toggled with the backtick key.
+ /// Panel visible? Hidden by default; toggled with the dev chord + `D` (decisions 1043, 1048).
pub open: bool,
pub models: ModelDebug,
pub lighting: LightingDebug,
diff --git a/crates/benilla-app/src/interact.rs b/crates/benilla-app/src/interact.rs
index 7fc29482..a3e91bbe 100644
--- a/crates/benilla-app/src/interact.rs
+++ b/crates/benilla-app/src/interact.rs
@@ -15,7 +15,7 @@
//! misses).
//! - the **inspector surface** ([`inspect_ui`]) — a standalone, key-toggleable overlay (**the dev
//! chord + `I`**): a weak "armed" pill plus a small identity card that follows the cursor over any
-//! picked object. It's its own surface, *not* a section of the backtick debug panel, so
+//! picked object. It's its own surface, *not* a section of the dev-chord debug panel, so
//! identifying a thing costs one chord and no panel.
//! - the **cast journal** ([`journal`]) — the *temporal* half of the same instrument: a spell is an
//! event, gone before a cursor could reach it, so every cast edge is recorded as it flows past and
diff --git a/crates/benilla-app/src/lib.rs b/crates/benilla-app/src/lib.rs
index aec60612..2d0976f0 100644
--- a/crates/benilla-app/src/lib.rs
+++ b/crates/benilla-app/src/lib.rs
@@ -16,9 +16,11 @@
//! sits at the login screen, which is what the real one does. The scene harness (`$WOW_CAPTURE`)
//! boots straight in-world and is unaffected.
//!
-//! Controls: WASD walks the avatar (Ctrl sprints); right-drag turns it, left-drag orbits the camera
-//! (both hide/freeze the cursor while held), scroll wheel zooms; `F` toggles free-fly (then WASD flies
-//! the camera with Space up / C down, Ctrl boost).
+//! Controls: WASD walks the avatar; right-drag turns it, left-drag orbits the camera (both
+//! hide/freeze the cursor while held), scroll wheel zooms. Those are all **bindings** now (0997) —
+//! rebindable, and nothing in the client squats on a bare key beside them (1043). The dev chord
+//! (`Ctrl`+`Shift`) + `F` toggles free-fly, then WASD flies the camera with Space up / C down and
+//! `Ctrl` boosts — a boost that exists only inside free-fly, itself behind the chord.
mod area;
mod area_trigger;
@@ -130,6 +132,7 @@ mod ui_net;
mod ui_party;
mod ui_pass;
mod ui_pet;
+mod ui_pet_book;
mod ui_pet_stats;
mod ui_quest;
mod ui_quest_log;
@@ -216,6 +219,7 @@ use ui_net::UiNetPlugin;
use ui_party::UiPartyPlugin;
use ui_pass::PlayerUiPlugin;
use ui_pet::UiPetPlugin;
+use ui_pet_book::UiPetBookPlugin;
use ui_pet_stats::UiPetStatsPlugin;
use ui_quest::UiQuestPlugin;
use ui_quest_log::UiQuestLogPlugin;
@@ -715,6 +719,7 @@ pub fn run(build: BuildId) -> AppExit {
// so this renders the ten packed words the last `SMSG_PET_SPELLS` delivered and sends
// intents back. After UiActionPlugin (shares `Spells` and the cooldown triple's clock).
.add_plugins(UiPetPlugin)
+ .add_plugins(UiPetBookPlugin)
// The pet's paper-doll stat block (happiness/loyalty/XP/training points). Its own plugin
// because it runs off descriptor fields and two DBC tables rather than off `SMSG_PET_SPELLS`.
.add_plugins(UiPetStatsPlugin)
diff --git a/crates/benilla-app/src/net.rs b/crates/benilla-app/src/net.rs
index f4a75b47..97d93bf2 100644
--- a/crates/benilla-app/src/net.rs
+++ b/crates/benilla-app/src/net.rs
@@ -704,6 +704,19 @@ pub(crate) enum ClientCommand {
/// player twin: no answer packet — the removal arrives as a `UNIT_FIELD_AURA` delta on the
/// pet, which is also what puts the slot's icon back.
PetCancelAura { pet_guid: u64, spell_id: u32 },
+ /// Flip one pet **spellbook** entry's autocast (`CMSG_PET_SPELL_AUTOCAST` 0x2F3, decision
+ /// 1032) — `ToggleSpellAutocast`'s send, and **not** the pet bar's autocast verb.
+ ///
+ /// The bar's right click is [`Self::PetSetAction`]: it rewrites a *slot's word*, so its body
+ /// names a position. This one names a **spell id**, because the book has no positions —
+ /// `0x4b4240` looks the id up in `0xb6f098` and hands `0x4bccb0` the id, never an index. Both
+ /// end up flipping the same bit on the same word; only one of them can say which word by slot.
+ /// No reply packet either way.
+ PetSpellAutocast {
+ pet_guid: u64,
+ spell_id: u32,
+ enabled: bool,
+ },
/// Start melee auto-attack on `guid` (`CMSG_ATTACKSWING`); echoed as `SMSG_ATTACKSTART`.
AttackSwing { guid: u64 },
/// Stop melee auto-attack (`CMSG_ATTACKSTOP`); echoed as `SMSG_ATTACKSTOP`, whose receive path
diff --git a/crates/benilla-app/src/net/apply.rs b/crates/benilla-app/src/net/apply.rs
index c4fb10c3..d87f2589 100644
--- a/crates/benilla-app/src/net/apply.rs
+++ b/crates/benilla-app/src/net/apply.rs
@@ -1038,6 +1038,7 @@ pub(super) fn apply_net_updates(
ui_actions.11.as_deref(),
&mut items,
&net_commands,
+ &mut pet_bar,
),
play_seq.next(),
),
diff --git a/crates/benilla-app/src/net/apply/pet.rs b/crates/benilla-app/src/net/apply/pet.rs
index 563d5625..d1153551 100644
--- a/crates/benilla-app/src/net/apply/pet.rs
+++ b/crates/benilla-app/src/net/apply/pet.rs
@@ -83,6 +83,27 @@ pub(super) fn pet_spells(spells: PetSpells, catalog: Option<&Spells>, bar: &mut
.collect::>()
.join(" ")
);
+ // The spell LIST, spelled out the same way and for the same reason (decision 1032). It is a
+ // different question from the bar's — "why is the pet book shorter than the packet said" — and
+ // the answer is always one of three things this line names: the id resolves to no `Spell.dbc`
+ // record, it carries `DO_NOT_DISPLAY` (the book's whole add-gate, `0x4b2f90`), or it is in.
+ // vmangos sends the pet's runtime passives here alongside its real spells, so the gap between
+ // the two counts is usually large and entirely correct.
+ debug!(
+ "net: pet spellbook — {}",
+ spells
+ .spells
+ .iter()
+ .map(|e| {
+ match catalog.and_then(|c| c.catalog.get(e.action())) {
+ Some(d) if d.in_pet_book() => format!("{} ({})", d.name, e.action()),
+ Some(d) => format!("{} ({}) DO_NOT_DISPLAY", d.name, e.action()),
+ None => format!("{} NOT IN CATALOG", e.action()),
+ }
+ })
+ .collect::>()
+ .join(" · ")
+ );
let now = Instant::now();
bar.cooldowns = crate::cooldowns::Cooldowns::default();
for cd in &spells.cooldowns {
diff --git a/crates/benilla-app/src/net/apply/spells.rs b/crates/benilla-app/src/net/apply/spells.rs
index 5d4c5834..fbfebe30 100644
--- a/crates/benilla-app/src/net/apply/spells.rs
+++ b/crates/benilla-app/src/net/apply/spells.rs
@@ -309,12 +309,15 @@ pub(super) fn spell_go(
queued_melee: &mut QueuedMeleeSpell,
text: &mut MessageWriter,
go_lid: &mut MessageWriter,
- // The cooldown store + what its start laws read (grouped: one arm-body concern).
+ // The cooldown store + what its start laws read (grouped: one arm-body concern). The last
+ // member is the PET's store — the reference inserts into two banks from this one handler
+ // ([`pet_go_cooldown`]).
cooldown_ctx: (
&mut Cooldowns,
Option<&Spells>,
&mut crate::items::Items,
&crate::net::NetCommands,
+ &mut crate::ui_pet::PetBar,
),
seq: u64,
) {
@@ -331,6 +334,9 @@ pub(super) fn spell_go(
if let Some(go_guid) = go_target {
go_lid.write(crate::go_anim::GoLidOpen { go_guid, spell_id });
}
+ let (cooldowns, spells, items, net_commands, pet_bar) = cooldown_ctx;
+ let now = Instant::now();
+ let display = spells.and_then(|s| s.catalog.get(spell_id));
// Our own launch completes the cast bar (a shown bar fills green and fades; auto-repeat
// shots — GO with no bar showing — no-op in the reference Lua) and opens the in-flight guard
// (spell-id-keyed, so a triggered proc's GO mid-cast doesn't unblock the running cast early).
@@ -366,9 +372,6 @@ pub(super) fn spell_go(
// normal source — this insert is how a Charge sweep appears on vmangos, which sends no
// cooldown packet for a plain cast. A pre-launch failure never reaches here, so nothing
// needs reverting.
- let (cooldowns, spells, items, net_commands) = cooldown_ctx;
- let now = Instant::now();
- let display = spells.and_then(|s| s.catalog.get(spell_id));
// The ranged-shot pad (the category scaler `0x6e2b60`, byte-verified — wow-re
// `ranged-cooldown-sweep.md`, decision 0378): a ranged-slot cast folds our live
// `UNIT_FIELD_RANGEDATTACKTIME` (haste-scaled, server-written) into the category
@@ -415,6 +418,28 @@ pub(super) fn spell_go(
}
}
}
+ // **The PET leg of the same insert** (decision 1031) — a second, independent `if` in the very
+ // same handler, not an else of the one above.
+ if let Some(d) = display.filter(|_| pet_go_cooldown(caster, self_guid, index, stores)) {
+ // No ranged pad here: `0x6e2b60` is called on the self leg only (`0x6e845d`), and there is
+ // nothing on a pet to read a `UNIT_FIELD_RANGEDATTACKTIME` from that the client uses.
+ pet_bar.cooldowns.start_spell(spell_id, d, 0, now);
+ // `0x6e85fc` + `0x6e8601`: SPELL_UPDATE_COOLDOWN and PET_BAR_UPDATE_COOLDOWN. benilla
+ // collapses both into the pet bar's one repaint event, which the feed fires off its own
+ // diff — and the diff moves, because the slot's cooldown triple just changed. The signal
+ // bump is belt-and-braces for the case where the same spell re-arms to an identical
+ // triple (a zero-length re-cast), which the diff would otherwise swallow.
+ pet_bar.bar_signals = pet_bar.bar_signals.wrapping_add(1);
+ if crate::dbg_trace::enabled() {
+ crate::dbg_trace::line(
+ "cd",
+ &format!(
+ "arm spell={spell_id} rec={}ms cat={}:{}ms (GO pet-insert)",
+ d.recovery_ms, d.category, d.category_recovery_ms
+ ),
+ );
+ }
+ }
// The miss list's floating words (0137 phase 2, the `0x6e7a70` handler): one outcome word
// over each missed target — except REFLECT, which re-anchors to the caster (`0x6e7e51`).
// Gate A applies to whichever unit the word lands over. Source-classified first (the color
@@ -602,6 +627,53 @@ pub(super) fn cancel_auto_repeat(
crate::creature_anim::cancel_auto_repeat_local(self_e, auto_repeat, commands, net);
}
+/// **Does this `SMSG_SPELL_GO` arm the PET's cooldown bank?** (decision 1031; `0x6e857a`-`0x6e85ad`.)
+///
+/// The reference's GO handler makes two independent cooldown inserts, into two banks
+/// (`0x6e2ea0`'s `bankHead = 0xcecaec + 24*bank`): the self leg above writes bank 0 at
+/// `0x6e8493 mov ecx, 0xcecaec`, and this one writes **bank 1** at `0x6e85f2 mov ecx, 0xcecb04` —
+/// the same bank `SMSG_PET_SPELLS`' cooldown tail seeds (`0x4bdaa8 push 1`) and the same bank
+/// `GetPetActionCooldown` and `GetSpellCooldown(id, "pet")` read. One packet can arm both.
+///
+/// The gate is **ownership of the caster**, read off the caster's own descriptor and nothing else
+/// — not the pet-bar guid, not `UNIT_FIELD_PETNUMBER`:
+///
+/// ```text
+/// 0x6e858b edi = fields[0x14] ; edx = fields[0x10] ; CHARMEDBY as a 64-bit pair
+/// 0x6e8594 or edx, edi ; jne -> edi = &fields[0x10]
+/// 0x6e859a else edi = &fields[0x18] ; SUMMONEDBY
+/// 0x6e859d call 0x468550 ; the active player's guid
+/// 0x6e85a2 if (*edi, *(edi+4)) != that guid -> skip
+/// ```
+///
+/// i.e. `charmedBy` when set, else **`summonedBy`** — `OwnerFallback::SummonedBy`, the pair the
+/// `PET_ATTACK_*` callback uses, and **not** `0x5ee5a0`'s `createdBy` fallback. Picking the wrong
+/// one would arm the bank for totems and miss real pets: exactly the silent failure that enum
+/// exists to prevent.
+///
+/// **Why this leg matters at all on vmangos**: the server sends *no* cooldown packet for a pet's
+/// own cast. `Creature::AddCooldown` (`Objects/Creature.cpp:3259-3282`) stores the cooldown and
+/// returns; its one `SendSpellCooldown` call sits in the `else` branch, reached only by a
+/// **charmed non-pet** casting an instant under mind control. So without this insert a hunter's
+/// Growl or a warlock imp's Firebolt shows no sweep at all until the next `SMSG_PET_SPELLS`
+/// reseeds the bank — and that is a summon, a swap or a learned spell, never a cast.
+fn pet_go_cooldown(
+ caster: u64,
+ self_guid: &SelfGuid,
+ index: &GuidIndex,
+ stores: &Query<&mut ObjectStore>,
+) -> bool {
+ let Some(me) = self_guid.0 else {
+ return false;
+ };
+ index
+ .0
+ .get(&caster)
+ .and_then(|&e| stores.get(e).ok())
+ .and_then(|s| s.0.unit_owner(benilla_protocol::OwnerFallback::SummonedBy))
+ == Some(me)
+}
+
/// `SMSG_SPELL_COOLDOWN` (`0x6e9460`) — server-pushed cooldowns (school lockouts, and the pet's
/// own list). **Which unit's store** it lands in is the caller's decision
/// ([`super::addressed_store`]): the four cooldown packets all carry a caster guid, and resolving
@@ -821,6 +893,7 @@ mod tests {
.init_resource::()
.init_resource::()
.init_resource::()
+ .init_resource::()
.init_resource::();
// The self player mid-cast on Fireball (133): the bar is up, `Casting{133}` marks it.
@@ -861,6 +934,7 @@ mod tests {
mut text: MessageWriter,
mut go_lid: MessageWriter,
mut cooldowns: ResMut,
+ mut pet_bar: ResMut,
mut items: ResMut| {
let net_commands = crate::net::NetCommands(tx.clone());
spell_go(
@@ -886,7 +960,13 @@ mod tests {
&mut queued_melee,
&mut text,
&mut go_lid,
- (&mut cooldowns, None, &mut items, &net_commands),
+ (
+ &mut cooldowns,
+ None,
+ &mut items,
+ &net_commands,
+ &mut pet_bar,
+ ),
1,
);
},
@@ -912,6 +992,166 @@ mod tests {
assert!(matches!(feed[0], CastBarEdge::Stop), "…with a STOP");
}
+ /// **The GO handler's PET leg** (decision 1031): a spell going off on a unit WE own arms the
+ /// pet's own cooldown bank, and nothing else does — vmangos sends no cooldown packet for a
+ /// pet's cast, so without this the pet bar's sweep never runs in play.
+ ///
+ /// Three things are pinned, and each is a way the leg could be subtly wrong:
+ /// - the insert lands in the **pet's** store, not the player's (two banks, `0xcecaec` /
+ /// `0xcecb04`);
+ /// - the owner read falls back to **SUMMONEDBY**, not CREATEDBY (`0x6e859a`) — a totem, which
+ /// carries only CREATEDBY, must not arm it;
+ /// - a stranger's cast arms neither.
+ #[test]
+ fn a_pets_own_go_arms_the_pet_bank_and_only_the_pet_bank() {
+ use crate::combat_text::CombatTextSpawn;
+ use crate::creature_anim::Casting;
+ use crate::go_anim::GoLidOpen;
+ use crate::net::{Guid, SelfPlayer};
+ use bevy::ecs::system::RunSystemOnce;
+
+ /// `UNIT_FIELD_CHARMEDBY` / `SUMMONEDBY` / `CREATEDBY` — 64-bit, so each takes two dwords.
+ const CHARMEDBY: u16 = 10;
+ const SUMMONEDBY: u16 = 12;
+ const CREATEDBY: u16 = 14;
+
+ const GROWL: u32 = 2649;
+ let growl = || benilla_formats::SpellDisplay {
+ name: "Growl".into(),
+ recovery_ms: 5000,
+ ..Default::default()
+ };
+ let make_spells = || crate::ui_action::Spells {
+ catalog: benilla_formats::SpellCatalog::from_displays(
+ [(GROWL, growl())].into_iter().collect(),
+ ),
+ forms: Default::default(),
+ ranges: Default::default(),
+ cast_times: Default::default(),
+ durations: Default::default(),
+ radii: Default::default(),
+ };
+
+ // A world with us (guid 10), our pet (20, SUMMONEDBY us), a totem (30, CREATEDBY us only)
+ // and a stranger's pet (40, SUMMONEDBY somebody else).
+ let owned = |field: u16, owner: u64| {
+ ObjectStore(benilla_protocol::ObjectFields::from_pairs(&[
+ (field, owner as u32),
+ (field + 1, (owner >> 32) as u32),
+ ]))
+ };
+ let fire = |caster: u64, store: ObjectStore| {
+ let mut app = App::new();
+ app.add_message::()
+ .add_message::()
+ .add_message::()
+ .add_message::()
+ .init_resource::()
+ .init_resource::()
+ .init_resource::()
+ .init_resource::()
+ .init_resource::()
+ .init_resource::()
+ .init_resource::()
+ .init_resource::();
+ let self_e = app
+ .world_mut()
+ .spawn((Guid(10), SelfPlayer, ObjectStore::default()))
+ .id();
+ let caster_e = app.world_mut().spawn((Guid(caster), store)).id();
+ {
+ let mut index = app.world_mut().resource_mut::();
+ index.0.insert(10, self_e);
+ index.0.insert(caster, caster_e);
+ }
+ app.world_mut().resource_mut::().0 = Some(10);
+
+ let (tx, _rx) = crossbeam_channel::unbounded();
+ let spells = make_spells();
+ app.world_mut()
+ .run_system_once(
+ move |mut commands: Commands,
+ index: Res,
+ casting: Query<&Casting>,
+ mut cast_events: MessageWriter,
+ mut go_targets: MessageWriter,
+ self_guid: Res,
+ stores: Query<&mut ObjectStore>,
+ mut cast_bar: ResMut,
+ mut pending: ResMut,
+ mut queued_melee: ResMut,
+ mut text: MessageWriter,
+ mut go_lid: MessageWriter,
+ mut cooldowns: ResMut,
+ mut pet_bar: ResMut,
+ mut items: ResMut| {
+ let net_commands = crate::net::NetCommands(tx.clone());
+ spell_go(
+ caster,
+ GROWL,
+ 0,
+ vec![],
+ vec![],
+ None,
+ None,
+ None,
+ None,
+ Some(caster),
+ &mut commands,
+ &index,
+ &casting,
+ &mut cast_events,
+ &mut go_targets,
+ &self_guid,
+ &stores,
+ &mut cast_bar,
+ &mut pending,
+ &mut queued_melee,
+ &mut text,
+ &mut go_lid,
+ (
+ &mut cooldowns,
+ Some(&spells),
+ &mut items,
+ &net_commands,
+ &mut pet_bar,
+ ),
+ 1,
+ );
+ },
+ )
+ .unwrap();
+ let now = Instant::now();
+ let armed = |c: &Cooldowns| c.info(GROWL, 0, Some(&growl()), now).remaining_ms > 0;
+ let world = app.world();
+ (
+ armed(world.resource::()),
+ armed(&world.resource::().cooldowns),
+ world.resource::().bar_signals,
+ )
+ };
+
+ // Our pet: the PET bank only, and a forced repaint with it.
+ let (player, pet, signals) = fire(20, owned(SUMMONEDBY, 10));
+ assert!(pet, "our pet's GO arms the pet bank");
+ assert!(!player, "…and never the player's");
+ assert_eq!(signals, 1, "PET_BAR_UPDATE_COOLDOWN's repaint");
+
+ // A charm reads CHARMEDBY first — the same leg, the other field.
+ let (_, charmed, _) = fire(20, owned(CHARMEDBY, 10));
+ assert!(charmed, "a charmed unit's GO arms it too");
+
+ // A TOTEM carries CREATEDBY and no SUMMONEDBY. `0x5ee5a0` would accept it; this leg's own
+ // fallback (`0x6e859a`) does not, and reading the wrong one is invisible until a shaman
+ // drops a totem and the pet bar sweeps.
+ let (_, totem, _) = fire(30, owned(CREATEDBY, 10));
+ assert!(!totem, "CREATEDBY alone is not this leg's owner test");
+
+ // Somebody else's pet: neither bank.
+ let (p2, pet2, _) = fire(40, owned(SUMMONEDBY, 99));
+ assert!(!p2 && !pet2, "a stranger's pet arms nothing");
+ }
+
/// **Both** producers of the caster's chain-hop array (decision 0955): `SMSG_SPELL_GO`'s own
/// hit list — the leg that makes a non-channelled chain spell draw at all — and the 816 packet.
/// Each drops the caster's own guid and each clears before it fills; unstreamed targets fall
@@ -936,6 +1176,7 @@ mod tests {
.init_resource::()
.init_resource::()
.init_resource::()
+ .init_resource::()
.init_resource::();
// A caster and two streamed victims; guid 40 is never streamed to us.
@@ -977,6 +1218,7 @@ mod tests {
mut text: MessageWriter,
mut go_lid: MessageWriter,
mut cooldowns: ResMut,
+ mut pet_bar: ResMut,
mut items: ResMut| {
let net_commands = crate::net::NetCommands(tx.clone());
spell_go(
@@ -1002,7 +1244,13 @@ mod tests {
&mut queued_melee,
&mut text,
&mut go_lid,
- (&mut cooldowns, None, &mut items, &net_commands),
+ (
+ &mut cooldowns,
+ None,
+ &mut items,
+ &net_commands,
+ &mut pet_bar,
+ ),
1,
);
},
diff --git a/crates/benilla-app/src/net/io.rs b/crates/benilla-app/src/net/io.rs
index a8a3acd2..225322d4 100644
--- a/crates/benilla-app/src/net/io.rs
+++ b/crates/benilla-app/src/net/io.rs
@@ -709,6 +709,11 @@ fn writer_loop(
ClientCommand::PetCancelAura { pet_guid, spell_id } => {
w.pet_cancel_aura(pet_guid, spell_id)
}
+ ClientCommand::PetSpellAutocast {
+ pet_guid,
+ spell_id,
+ enabled,
+ } => w.pet_spell_autocast(pet_guid, spell_id, enabled),
ClientCommand::AttackSwing { guid } => w.attack_swing(guid),
ClientCommand::AttackStop => w.attack_stop(),
ClientCommand::SetSheathed { state } => w.set_sheathed(state),
diff --git a/crates/benilla-app/src/player.rs b/crates/benilla-app/src/player.rs
index d97cb43b..939b263d 100644
--- a/crates/benilla-app/src/player.rs
+++ b/crates/benilla-app/src/player.rs
@@ -12,7 +12,8 @@
//! distance (clamped to the vanilla `cameraDistanceMax` range; the camera *glides* to the new
//! distance). A left-drag orbit offset *persists* — the vanilla `cameraSmoothStyle` auto-follow that
//! swung the camera back behind the character while moving is deliberately removed (director's call).
-//! `F` toggles free-fly; the **dev chord + `G`** lands the avatar where the camera is ([`land`]).
+//! The **dev chord + `F`** toggles free-fly (1043); the **dev chord + `G`** lands the avatar where
+//! the camera is ([`land`]).
//!
//! Movement is a thin kinematic capsule controller over avian's `MoveAndSlide` (decision 0009).
@@ -260,7 +261,8 @@ fn mirror_self_collision_height(
/// Camera + avatar controller. Free-flies until the server reports our position; then takes
/// third-person control (WASD walks the avatar; right-drag turns it, left-drag orbits the camera,
-/// wheel zooms) and streams our movement to the server as the confirmed mover. `F` toggles free-fly.
+/// wheel zooms) and streams our movement to the server as the confirmed mover. The dev chord + `F`
+/// toggles free-fly (decision 1043).
#[allow(clippy::too_many_arguments, clippy::type_complexity)]
fn control(
time: Res,
@@ -307,8 +309,8 @@ fn control(
Res,
Res,
// The binding dispatch (decision 0997): every rebindable input below reads command
- // state from here — raw `keys` remain only for the dev instruments (F free-fly, the
- // Ctrl boost) and the look-session mouse.
+ // state from here — raw `keys` remain only for the dev chord's free-fly toggle
+ // (decision 1043) and the look-session mouse.
Res,
),
mut commands: Commands,
@@ -396,27 +398,14 @@ fn control(
let binds = &speed_capsule.7;
let dt = time.delta_secs();
// While a focused UI EditBox (the chat input, a mail field) owns the keyboard, keyboard reads see
- // "no keys held" — so WASD/F/Ctrl/Z don't also drive the avatar while typing (a `.tele` command).
- // Mouse still works. The gate is `UiKeyboardCapture`, which the focused chat EditBox drives.
+ // "no keys held" — so the avatar isn't also driven while typing (a `.tele` command). Mouse still
+ // works. The gate is `UiKeyboardCapture`, which the focused chat EditBox drives; the free-fly
+ // chord below is deliberately outside it, like every dev chord ([`debug_panel::dev_chord`]).
let typing = ui_capture.0;
- let keys_pressed = |k: KeyCode| !typing && keys.pressed(k);
- let keys_just_pressed = |k: KeyCode| !typing && keys.just_pressed(k);
// The rebindable inputs all read `binds` (decision 0997): the dispatch already enforced the
// typing gate and 0585's exact-modifier law when it latched, so this module carries neither
- // anymore. What stays on raw keys: the F free-fly toggle and the Ctrl boost (dev
- // instruments, not 1.12 bindings — F still wants the bare-binding rule locally so CTRL-F
- // and friends stay distinct) and the mouse look session.
- let modified = keys.any_pressed([
- KeyCode::ShiftLeft,
- KeyCode::ShiftRight,
- KeyCode::ControlLeft,
- KeyCode::ControlRight,
- KeyCode::AltLeft,
- KeyCode::AltRight,
- KeyCode::SuperLeft,
- KeyCode::SuperRight,
- ]);
- let bare_binding = |k: KeyCode| keys_just_pressed(k) && !modified;
+ // anymore. Nothing here reads a bare key any more (decision 1043) — the free-fly toggle is on
+ // the dev chord, and the Ctrl run boost is gone.
// Both mouse buttons held together = vanilla's "both-button run": the avatar runs forward while
// the character steers with the mouse (turns like a right-drag), regardless of which button went
@@ -513,7 +502,11 @@ fn control(
- binds.amount(crate::bindings::cmd::CAMERA_ZOOM_OUT);
apply_zoom_scroll(zoom, dt, &mut rig);
- if bare_binding(KeyCode::KeyF) {
+ // Free-fly is a dev instrument, so it sits on the dev chord, not a bare `F` (decision 1043).
+ // A bare `F` is a key the reference lets a player bind — our own store test binds it to JUMP —
+ // and a dev doesn't get to squat on the game's namespace (0585, the same rule that moved the
+ // perf HUD off bare `P`). Ungated on `typing` like every chord: it can't be mistaken for text.
+ if crate::debug_panel::dev_chord(&keys, KeyCode::KeyF) {
player.detached = !player.detached;
}
@@ -889,11 +882,6 @@ fn control(
}
}
}
- let boost = if keys_pressed(KeyCode::ControlLeft) {
- 2.5
- } else {
- 1.0
- };
// Backpedaling is slower: the backward move-flag selects the backward speed, dominating
// strafe (binary-VERIFIED — see RUN_BACK_RATIO). Net-backward = the S key held without a
// forward override (W or both-button run). The backward arm is a **min**, not a plain
@@ -912,12 +900,11 @@ fn control(
Some(s) if !move_speed.env_override => (s.run, s.run_back),
_ => (move_speed.value, move_speed.value * RUN_BACK_RATIO),
};
- let speed = boost
- * if net_backward {
- run_back_speed.min(run_speed)
- } else {
- run_speed
- };
+ let speed = if net_backward {
+ run_back_speed.min(run_speed)
+ } else {
+ run_speed
+ };
let mut want_jump = binds.fired(crate::bindings::cmd::JUMP) && !player.modes.rooted;
// Swim vs walk: the water over our feet decides. Hysteresis-latched (`update_swimming`,
diff --git a/crates/benilla-app/src/player/land.rs b/crates/benilla-app/src/player/land.rs
index 013b6b64..40062750 100644
--- a/crates/benilla-app/src/player/land.rs
+++ b/crates/benilla-app/src/player/land.rs
@@ -85,7 +85,10 @@ pub(crate) fn land_here(
return;
}
if !player.detached {
- info!("land: not free-flying — press F, fly somewhere, then land");
+ info!(
+ "land: not free-flying — press {chord}+F, fly somewhere, then land",
+ chord = crate::debug_panel::DEV_CHORD
+ );
return;
}
let Ok(cam) = camera.single() else {
diff --git a/crates/benilla-app/src/player/state.rs b/crates/benilla-app/src/player/state.rs
index c06ba6a7..e5726f29 100644
--- a/crates/benilla-app/src/player/state.rs
+++ b/crates/benilla-app/src/player/state.rs
@@ -359,7 +359,8 @@ pub(crate) fn incapacitated_flags(flags: u32, rooted: bool, stunned: bool) -> u3
}
/// Our controllable avatar. Until `active`, the camera free-flies; once the server reports our
-/// position we take control (third-person) and drive movement. Toggle free-fly with `F`.
+/// position we take control (third-person) and drive movement. Toggle free-fly with the dev
+/// chord + `F` (decision 1043).
/// `active`/`pos`/`detached` are `pub(crate)` so terrain streaming can center the loaded block on the
/// avatar in third-person and on the free-flying camera while detached.
#[derive(Resource, Default)]
diff --git a/crates/benilla-app/src/player/wire_in.rs b/crates/benilla-app/src/player/wire_in.rs
index 2a128f7c..62e9e569 100644
--- a/crates/benilla-app/src/player/wire_in.rs
+++ b/crates/benilla-app/src/player/wire_in.rs
@@ -246,9 +246,10 @@ pub(super) fn apply_server_moves(
// cross-map worldport despawns and re-streams this entity while `player.active` stays
// true — per-entity state attached only on this one-shot edge would be lost on transfer.
info!(
- "took control of player @ {:?} facing {:.3} ('F' toggles free-fly)",
+ "took control of player @ {:?} facing {:.3} ({} + F toggles free-fly)",
player.pos,
- yaw.rem_euclid(std::f32::consts::TAU)
+ yaw.rem_euclid(std::f32::consts::TAU),
+ crate::debug_panel::DEV_CHORD
);
}
}
diff --git a/crates/benilla-app/src/schedule.rs b/crates/benilla-app/src/schedule.rs
index 935fa9bf..c9a9db52 100644
--- a/crates/benilla-app/src/schedule.rs
+++ b/crates/benilla-app/src/schedule.rs
@@ -53,8 +53,9 @@ pub(crate) enum WorldStage {
Present,
}
-/// Installs the [`WorldStage`] ordering. Add before the subsystem plugins (order doesn't matter — set
-/// configuration is resolved at schedule build).
+/// Installs the [`WorldStage`] ordering + the boot-order correction below. Add before the subsystem
+/// plugins (order doesn't matter — set configuration is resolved at schedule build), but *after*
+/// `DefaultPlugins`, which is where `StatesPlugin` seeds the order this fixes.
pub(crate) struct SchedulePlugin;
impl Plugin for SchedulePlugin {
@@ -69,5 +70,91 @@ impl Plugin for SchedulePlugin {
)
.chain(),
);
+ move_initial_state_transition_after_startup(app);
+ }
+}
+
+/// **The initial state's `OnEnter` runs AFTER the app is built, not before it.**
+///
+/// `bevy_state` seeds the startup order with (`bevy_state-0.18.1/src/app.rs:308-309`):
+///
+/// ```ignore
+/// schedule.insert_after(PreUpdate, StateTransition);
+/// schedule.insert_startup_before(PreStartup, StateTransition); // ← this one
+/// ```
+///
+/// so the one-shot transition that enters the *initial* state runs before `PreStartup` — ahead of
+/// every `Startup` system, i.e. ahead of the patch chain ([`crate::assets::AssetSet::Open`]), the
+/// Lua VM, the mixer and every catalog. An `OnEnter` for the state the app boots into therefore
+/// sees an empty world, which is not what "on entering this state" means anywhere else.
+///
+/// Three subsystems each hit this independently and each wrote the same local workaround — spawn
+/// from a per-frame `Update` poll instead of the state edge:
+/// [`crate::login::screen::materialize_screen`], [`crate::char_select::screen::enter_select`] and
+/// `sound::glue::start_glue_music`. Their comments name it "the boot-order trap". It is one cause
+/// with three patches, which is the shape §3 says to go after rather than add a fourth to.
+///
+/// The fix is to move that single startup entry to the end: the initial transition now runs after
+/// `PostStartup`, so `OnEnter()` observes a fully built app exactly like every later
+/// transition does. Safe here because **no `Startup` system in the workspace writes `NextState`**
+/// (every writer is an `Update` system), so nothing can be waiting on the transition to be applied
+/// mid-startup; and `insert_state`/`init_state` insert the `State` resource directly at plugin
+/// build, so a `Startup` system reading `Res>` is unaffected either way.
+///
+/// This does not retire the three workarounds — two of them also re-spawn when *async* client art
+/// lands, which is a different clock. It removes the boot-order reason to write a fourth.
+fn move_initial_state_transition_after_startup(app: &mut App) {
+ use bevy::app::MainScheduleOrder;
+ use bevy::state::state::StateTransition;
+
+ let mut order = app.world_mut().resource_mut::();
+ let before = order.startup_labels.len();
+ order.startup_labels.retain(|l| !(**l).eq(&StateTransition));
+ // Only re-seat what was actually there — if bevy_state ever stops seeding it, do not invent it.
+ if order.startup_labels.len() < before {
+ order.insert_startup_after(PostStartup, StateTransition);
+ }
+}
+
+#[cfg(test)]
+mod tests {
+ use bevy::prelude::*;
+ use bevy::state::app::StatesPlugin;
+
+ use super::SchedulePlugin;
+ use crate::char_select::ClientState;
+
+ #[derive(Resource)]
+ struct BuiltAtStartup;
+
+ /// What [`super::move_initial_state_transition_after_startup`] buys: an `OnEnter` for the state
+ /// the app BOOTS INTO sees the resources `Startup` built. Without the re-seat this is false —
+ /// the initial transition runs ahead of `PreStartup` — and that is the boot-order trap three
+ /// subsystems each worked around by hand.
+ #[test]
+ fn the_initial_states_on_enter_sees_what_startup_built() {
+ #[derive(Resource, Default)]
+ struct Saw(Option);
+
+ let mut app = App::new();
+ app.add_plugins((MinimalPlugins, StatesPlugin))
+ .add_plugins(SchedulePlugin)
+ .insert_state(ClientState::InWorld)
+ .init_resource::()
+ .add_systems(Startup, |mut c: Commands| c.insert_resource(BuiltAtStartup))
+ .add_systems(
+ OnEnter(ClientState::InWorld),
+ |built: Option>, mut saw: ResMut| {
+ saw.0 = Some(built.is_some());
+ },
+ );
+
+ app.update();
+
+ assert_eq!(
+ app.world().resource::().0,
+ Some(true),
+ "the initial state's OnEnter ran before Startup — the boot-order trap is back"
+ );
}
}
diff --git a/crates/benilla-app/src/target/by_name.rs b/crates/benilla-app/src/target/by_name.rs
index 0305ec97..86052424 100644
--- a/crates/benilla-app/src/target/by_name.rs
+++ b/crates/benilla-app/src/target/by_name.rs
@@ -71,7 +71,7 @@ use bevy::prelude::*;
use benilla_protocol::guid;
use crate::names::NameCache;
-use crate::net::{Guid, GuidIndex, NetCommands, ObjectStore, SelfPlayer};
+use crate::net::{Guid, GuidIndex, ObjectStore, SelfPlayer};
use super::{scan, Selection};
@@ -567,7 +567,7 @@ pub(super) fn follow_requests(
#[allow(clippy::type_complexity)] // one bundled system param — the app's convention for big query sets
pub(crate) struct SelectCommit<'w, 's> {
selection: ResMut<'w, Selection>,
- net: Res<'w, NetCommands>,
+ seam: crate::creature_anim::AttackSeam<'w, 's>,
me: Query<
'w,
's,
@@ -588,7 +588,7 @@ impl SelectCommit<'_, '_> {
let me = self.me.single().ok();
// The new-target classification `scan::commit` expects from its callers (it does not
// re-derive one) — the same `can_attack` the cursor and the TAB scan pass.
- let attackable = scan::can_attack(
+ let attackable = super::relations::can_attack(
self.stores.get(entity).ok(),
self.factions.as_deref(),
&self.reputations,
@@ -596,7 +596,7 @@ impl SelectCommit<'_, '_> {
);
scan::commit(
&mut self.selection,
- &self.net,
+ &mut self.seam,
entity,
guid,
me.is_some_and(|(_, _, engaged)| engaged),
diff --git a/crates/benilla-app/src/target/click.rs b/crates/benilla-app/src/target/click.rs
index 27c1f2fa..d29c296e 100644
--- a/crates/benilla-app/src/target/click.rs
+++ b/crates/benilla-app/src/target/click.rs
@@ -56,7 +56,7 @@ pub(super) fn select_on_click(
hovered: Res,
cursor: Res,
mut selection: ResMut,
- net: Res,
+ mut seam: crate::creature_anim::AttackSeam,
self_q: Query<(&Guid, Has), With>,
payload_held: Res,
occlusion: Res,
@@ -95,7 +95,7 @@ pub(super) fn select_on_click(
// neutral, hover-refreshed every frame) IS Attack `0x5ecb70`'s new-target validation.
scan::commit(
&mut selection,
- &net,
+ &mut seam,
entity,
guid,
engaged,
@@ -113,7 +113,7 @@ pub(super) fn select_on_click(
// at 0 = an empty-world click keeps the target (1.12's own inverted checkbox).
_ => {
if click_cfg.deselect_on_click && (!payload_held.0 || occlusion.distance.is_finite()) {
- clear(&mut selection, &net, engaged);
+ clear(&mut selection, &mut seam, engaged);
}
}
}
@@ -149,9 +149,8 @@ pub(super) fn act_on_right_click(
hovered_object: Res,
cursor: Res,
mut selection: ResMut,
- net: Res,
+ mut seam: crate::creature_anim::AttackSeam,
self_player: Query<(Entity, &Guid, Has), With>,
- mut sheath: MessageWriter,
mut emote: MessageWriter,
mut play_seq: ResMut,
// The GameObject lock-routing inputs (decisions 0239 / 0545 / 0752) as one [`GoLockInputs`]
@@ -227,13 +226,13 @@ pub(super) fn act_on_right_click(
&player_actions.spells,
go,
me_store,
- &net,
+ &seam.net,
) {
GoAction::Use if cursor.unable => {}
GoAction::OpenLock(_) | GoAction::OpenByKey { .. } if cursor.unable => {}
GoAction::Use => {
debug!("right-click gameobject use: {guid:#x}");
- let _ = net.0.send(ClientCommand::GameObjUse { guid });
+ let _ = seam.net.0.send(ClientCommand::GameObjUse { guid });
}
GoAction::OpenLock(spell_id) => {
// The opener cast funnels through the ref's TryCast like any other cast
@@ -254,7 +253,7 @@ pub(super) fn act_on_right_click(
return;
}
debug!("right-click gameobject open-lock: cast {spell_id} at {guid:#x}");
- let _ = net.0.send(ClientCommand::CastSpellGameObject {
+ let _ = seam.net.0.send(ClientCommand::CastSpellGameObject {
spell_id,
go_guid: guid,
});
@@ -277,7 +276,7 @@ pub(super) fn act_on_right_click(
// *spell* as well as its block ordinal, and the binder has no GameObject
// arm (`CastCommit::Item::on_object` is the seam that awaits it). Stated
// gap, decision 0914.
- let _ = net.0.send(ClientCommand::UseItem {
+ let _ = seam.net.0.send(ClientCommand::UseItem {
bag_index,
slot,
spell_index,
@@ -315,7 +314,7 @@ pub(super) fn act_on_right_click(
// it never swings at them.
let outcome = scan::commit(
&mut selection,
- &net,
+ &mut seam,
entity,
guid,
me.is_some_and(|(_, _, e)| e),
@@ -335,23 +334,14 @@ pub(super) fn act_on_right_click(
// refused — selection stands, no swing
} else {
debug!("right-click attack: {guid:#x}");
- // Auto-draw through the anim layer's ONE setter (decision 0080) — a SNAP, no
- // ceremony, no sound: the attack path passes `(newState=1, bInstant=1,
- // bFireEvent=1)` at `0x5ecd80` (wow-re `sheath-policy.md`). The setter's
- // idempotency is the client's own "no-op if already melee".
- if let Some((e, _, _)) = me {
- sheath.write(crate::creature_anim::SheathRequest {
- entity: e,
- state: 1,
- ceremony: false,
- });
- }
- // The commit's engaged-switch law may already have re-pointed the swing at this
- // guid (the ref's `0x5ecb70` skips a second send while the attack lock is set) —
- // only the fresh attack-start still owes its ATTACKSWING.
- if !outcome.swung {
- let _ = net.0.send(ClientCommand::AttackSwing { guid });
- }
+ // The right-click's own StartAttack, through the one seam (auto-draw + the swing +
+ // the auto-repeat cancel — `0x5ecb70`'s whole body). The commit above may already
+ // have re-pointed the swing at this guid, in which case the ref's `0x5eccda` is the
+ // thing that suppresses a second send, so pass it the real lock state and let the
+ // seam decide: `swung` means the re-swing already went out and the lock is ours, so
+ // no stop is in flight any more.
+ let engaged = me.is_some_and(|(_, _, e)| e);
+ seam.start(guid, engaged || outcome.swung, false);
}
} else if loot {
// A dead unit carrying UNIT_DYNFLAG_LOOTABLE (the Pickup loot cursor, decision 0084): open
@@ -360,7 +350,7 @@ pub(super) fn act_on_right_click(
// No EmoteTalk: looting is not an NPC interaction, the corpse plays no talk.
if !cursor.unable {
debug!("right-click loot: {guid:#x}");
- let _ = net.0.send(ClientCommand::Loot { guid });
+ let _ = seam.net.0.send(ClientCommand::Loot { guid });
// The kneel is client-predicted AT THE SEND: the real client's `CMSG_LOOT` sender
// (`0x5df253`) sets the loot-target latch `[player+0x1d28]` and plays Loot 50 before
// any server response (decision 0515). Arm the latch the anim driver's loot leg
@@ -377,7 +367,7 @@ pub(super) fn act_on_right_click(
if !cursor.unable {
if let Some(spell_id) = learned.skinning {
debug!("right-click skin: {guid:#x} (spell {spell_id})");
- let _ = net.0.send(ClientCommand::CastSpell {
+ let _ = seam.net.0.send(ClientCommand::CastSpell {
spell_id,
target: Some(guid),
});
@@ -395,7 +385,7 @@ pub(super) fn act_on_right_click(
.unwrap_or(0);
if let Some(cmd) = interact_command(cursor.kind, guid, npc_flags) {
debug!("right-click interact: {guid:#x} ({:?})", cursor.kind);
- let _ = net.0.send(cmd);
+ let _ = seam.net.0.send(cmd);
// Play EmoteTalk on our avatar — the reconcile stows a drawn weapon, persistently
// (decisions 0080/0081; no sheath wiring here).
if let Some((e, _, _)) = me {
@@ -674,14 +664,14 @@ fn interact_command(
pub(super) fn clear_target_requests(
script: Option>,
mut selection: ResMut,
- net: Res,
+ mut seam: crate::creature_anim::AttackSeam,
engaged: Query<(), (With, With)>,
) {
let Some(mut script) = script else {
return;
};
if script.take_target_clear() {
- clear(&mut selection, &net, !engaged.is_empty());
+ clear(&mut selection, &mut seam, !engaged.is_empty());
}
}
@@ -698,7 +688,7 @@ pub(super) fn clear_target_requests(
pub(super) fn target_unit_requests(
script: Option>,
mut selection: ResMut,
- net: Res,
+ mut seam: crate::creature_anim::AttackSeam,
self_q: Query<(Entity, &Guid, Has), With>,
group: Res,
index: Res,
@@ -739,7 +729,7 @@ pub(super) fn target_unit_requests(
// switch fired from here can only ever STOP the swing — never re-point it.
scan::commit(
&mut selection,
- &net,
+ &mut seam,
entity,
guid,
engaged,
@@ -755,13 +745,18 @@ pub(super) fn target_unit_requests(
/// one is running (`engaged`, our server-echoed [`Engaged`]): `CMSG_ATTACKSTOP` — the ref stops
/// swinging and drops the attack stance on Esc/click-off/target-death alike (the stance itself falls
/// when the `SMSG_ATTACKSTOP` echo removes [`Engaged`]). Weapons *stay drawn* — combat never stows.
-pub(super) fn clear(selection: &mut Selection, net: &NetCommands, engaged: bool) {
+pub(super) fn clear(
+ selection: &mut Selection,
+ seam: &mut crate::creature_anim::AttackSeam,
+ engaged: bool,
+) {
if selection.target.take().is_some() {
selection.guid = None;
- let _ = net.0.send(ClientCommand::SetSelection { guid: 0 });
- if engaged {
- let _ = net.0.send(ClientCommand::AttackStop);
- }
+ let _ = seam.net.0.send(ClientCommand::SetSelection { guid: 0 });
+ // `SetSelection 0x493540`'s own `0x493a08 call 0x5ecac0` — the real StopAttack, so
+ // losing the target also un-queues a pending on-next-swing strike. It was a bare
+ // `CMSG_ATTACKSTOP` before the seam existed.
+ seam.stop(engaged);
}
}
diff --git a/crates/benilla-app/src/target/flash.rs b/crates/benilla-app/src/target/flash.rs
index 2e9842bc..56d0c8e4 100644
--- a/crates/benilla-app/src/target/flash.rs
+++ b/crates/benilla-app/src/target/flash.rs
@@ -25,7 +25,7 @@ use bevy::prelude::*;
use crate::creature_anim::Engaged;
use crate::net::{ObjectStore, Reputations, SelfPlayer};
-use super::scan::can_attack;
+use super::relations::can_attack;
use super::{Factions, Selection};
/// This frame's flash verdict + the global pulse clock. Recomputed every frame by
diff --git a/crates/benilla-app/src/target/mod.rs b/crates/benilla-app/src/target/mod.rs
index 9b23a732..bc3545e2 100644
--- a/crates/benilla-app/src/target/mod.rs
+++ b/crates/benilla-app/src/target/mod.rs
@@ -51,6 +51,7 @@ mod flash;
mod highlight;
mod hover;
pub(crate) mod lock;
+mod relations;
mod reticle;
mod ring;
mod scan;
@@ -65,7 +66,8 @@ pub(crate) use flash::CombatFlash;
// (`scan`) answers it with the same core TAB uses. `attack_order_target` + `EnemyScan` are that
// same core called *synchronously*, by the pet bar's ATTACK arm: the pet's order has to leave in
// the frame it was pressed carrying the acquired guid, so it cannot go round through a request.
-pub(crate) use scan::{attack_order_target, can_attack, AttackNearestRequest, EnemyScan};
+pub(crate) use relations::{can_assist, can_attack};
+pub(crate) use scan::{attack_order_target, AttackNearestRequest, EnemyScan};
// The chat layer's by-name selection asks (`/target`, `/assist` — decision 0886), answered by the
// shared resolver the reference parameterises per caller.
pub(crate) use by_name::{AssistRequest, TargetByNameRequest};
diff --git a/crates/benilla-app/src/target/relations.rs b/crates/benilla-app/src/target/relations.rs
new file mode 100644
index 00000000..98c3e3b9
--- /dev/null
+++ b/crates/benilla-app/src/target/relations.rs
@@ -0,0 +1,101 @@
+//! **Who may I attack, who may I help** — the two unit-relationship predicates the reference
+//! shares across systems, kept together because they are one concern and neither belongs to the
+//! system that happens to call them first.
+//!
+//! Both are byte-derived and both key off [`ring_reaction`], on opposite sides of the same bar:
+//! `CanAttack 0x606980` wants reaction ≤ 3 (not friendly), `CanAssist 0x6066f0` wants ≥ 4
+//! (friendly) plus two flag clauses. They lived in `scan.rs` while `can_attack` had exactly one
+//! caller; `can_assist` (the `UnitBuff` gate, decision 1035) made that a second concern in a file
+//! about TAB targeting.
+
+use benilla_protocol::messages::OwnerFallback;
+
+use crate::net::{ObjectStore, Reputations};
+
+use super::{ring_reaction, Factions};
+
+/// The five `UNIT_FIELD_FLAGS` disqualifiers `CanAttack 0x606980` tests (bit positions
+/// byte-verified; names vmangos-corroborated): NON_ATTACKABLE(1), NOT_ATTACKABLE_1(7),
+/// NON_ATTACKABLE_2(16), TAXI_FLIGHT(20), NOT_SELECTABLE(25).
+pub(super) const FLAG_DISQUALIFIERS: u32 = (1 << 1) | (1 << 7) | (1 << 16) | (1 << 20) | (1 << 25);
+
+/// `CanAttack 0x606980` — the shared attackability predicate (the combat flash's gate and the
+/// scan's filter 3): the flag disqualifiers clear AND an attackable reaction. A unit without a
+/// store passes the flag leg (nothing known to disqualify). The duel/PVP legs are deferred with
+/// duels/PvP.
+///
+/// The reaction leg is **≤ neutral (rank ≤ 3)** — attackable means *not friendly*. First
+/// director-observed (0170), then **byte-confirmed** by the `0x606980` re-pin: the function has
+/// three reaction legs selected by `UNIT_FLAG_PVP_ATTACKABLE` (bit 3) on both parties, and the
+/// player-vs-creature case is Leg B — `UnitReaction(player→target) < 4` (`cmp eax,4; setl`),
+/// single direction, friendly-only blocked. The old "≤ 1 hostile" gloss was Leg A (both parties
+/// UN-flagged: NPC-vs-NPC, bidirectional), never the player's check. Leg C (both flagged: the
+/// PvP/duel/group machinery) is deferred with PvP.
+pub(crate) fn can_attack(
+ store: Option<&ObjectStore>,
+ factions: Option<&Factions>,
+ reputations: &Reputations,
+ self_store: Option<&ObjectStore>,
+) -> bool {
+ if store.is_some_and(|s| s.0.unit_flags() & FLAG_DISQUALIFIERS != 0) {
+ return false;
+ }
+ ring_reaction(factions, reputations, store, self_store) <= 3
+}
+
+/// `UNIT_FLAG_NOT_SELECTABLE` (bit 25) — `CanAssist`'s own first disqualifier, and one of
+/// [`FLAG_DISQUALIFIERS`]' five (vmangos `UnitDefines.h`).
+const UNIT_FLAG_NOT_SELECTABLE: u32 = 1 << 25;
+/// `UNIT_FLAG_PVP` (bit 12) — what `IsPvP 0x605ff0` tests, after resolving the unit's owner
+/// (vmangos `UnitDefines.h:UNIT_FLAG_PVP = 0x1000`).
+const UNIT_FLAG_PVP: u32 = 0x1000;
+/// `UNIT_FLAG_PVP_ATTACKABLE` (bit 3) — behaviourally "player-controlled", the same bit
+/// [`ring_reaction`]'s duel leg selects on.
+const UNIT_FLAG_PLAYER_CONTROLLED: u32 = 0x8;
+
+/// `CanAssist 0x6066f0` — "may I help this unit?", the predicate `UnitBuff`'s unit-level gate runs
+/// (see [`crate::ui_aura::buffs_visible_on`]). Byte-derived in wow-re's
+/// `ui/scratch/aura-display-pipeline.md` §9b; named by its own Lua registrar pair
+/// (`.data 0x8504c8 = {"UnitCanAssist", 0x516bb0}`), not by resemblance.
+///
+/// Three clauses: `UNIT_FLAG_NOT_SELECTABLE` clear, `UnitReaction ≥ 4`, and — for a unit that is
+/// **not** player-controlled — `IsPvP 0x605ff0` on the unit (owner-chased). The reaction bar is the
+/// **internal** scale, not the Lua one: `UnitReaction 0x5167e0` does `inc eax` at `0x51683e` before
+/// pushing, so Lua's FRIENDLY(5) is internal 4. [`ring_reaction`] already returns the raw internal
+/// rank, so the comparison is against 4 directly — **neutral fails**.
+///
+/// The player-controlled arm (`0x60673e`–`0x60679f`) keys on an `[obj+0xe68]` record whose fields
+/// wow-re did **not** name, so it is deliberately NOT modelled: a player-controlled unit takes the
+/// permissive answer here. That is conservative in the only direction that matters (it shows what
+/// we already showed) and it keeps the un-derived arm out of the one place it could silently blank
+/// a player's or a pet's buffs.
+pub(crate) fn can_assist(
+ store: Option<&ObjectStore>,
+ factions: Option<&Factions>,
+ reputations: &Reputations,
+ self_store: Option<&ObjectStore>,
+ owner_store: impl FnOnce(u64) -> Option,
+) -> bool {
+ let Some(store) = store else {
+ return false;
+ };
+ let flags = store.0.unit_flags();
+ if flags & UNIT_FLAG_NOT_SELECTABLE != 0 {
+ return false;
+ }
+ if ring_reaction(factions, reputations, Some(store), self_store) < 4 {
+ return false;
+ }
+ if flags & UNIT_FLAG_PLAYER_CONTROLLED != 0 {
+ // The un-derived arm — see the doc comment.
+ return true;
+ }
+ // `IsPvP 0x605ff0`: the flag on the unit's OWNER when it has one (charmedBy, else createdBy —
+ // the `0x5ee5a0` fallback pair), otherwise on the unit itself.
+ let owned = store
+ .0
+ .unit_owner(OwnerFallback::CreatedBy)
+ .and_then(owner_store);
+ let pvp_flags = owned.as_ref().map_or(flags, |o| o.0.unit_flags());
+ pvp_flags & UNIT_FLAG_PVP != 0
+}
diff --git a/crates/benilla-app/src/target/ring.rs b/crates/benilla-app/src/target/ring.rs
index 5e1a445a..a3c2ad8f 100644
--- a/crates/benilla-app/src/target/ring.rs
+++ b/crates/benilla-app/src/target/ring.rs
@@ -46,7 +46,7 @@ use bevy::prelude::*;
use crate::assets::{LockRecover, WorldAssets};
use crate::collision::GroundDecalSurface;
use crate::decal::{project_decal, DecalFrame};
-use crate::net::{NetCommands, NetEntity, ObjectStore, Reputations, SelfPlayer};
+use crate::net::{NetEntity, ObjectStore, Reputations, SelfPlayer};
use crate::particles::buffer::{EffectBlend, EffectDrawSpec, EffectFog, EffectQuads, EffectVertex};
use super::click::clear;
@@ -276,7 +276,7 @@ pub(super) fn update_ring(
// `.respawn`ed creature reuses its spawn guid, so change-armed state goes stale and misses the
// second kill (the bug this replaces).
mut last_vitals: Local>,
- net_commands: Res,
+ mut seam: crate::creature_anim::AttackSeam,
// Net entities are roots, so their `Transform` is already world-space + current this frame
// (net motion ran in `WorldStage::Net`), avoiding the 1-frame lag a `GlobalTransform` read
// would add. Tupled into one param (the 16-param ceiling): `.0` the target's own components;
@@ -375,7 +375,7 @@ pub(super) fn update_ring(
.is_some_and(|g| *last_vitals == Some((g, false)));
*last_vitals = selection.guid.map(|g| (g, is_dead));
if died {
- clear(&mut selection, &net_commands, !engaged.is_empty());
+ clear(&mut selection, &mut seam, !engaged.is_empty());
*last_vitals = None;
state.shown = false;
state.verts.clear();
@@ -416,7 +416,7 @@ pub(super) fn update_ring(
// selection-death-clear RE; this is also what drops a selected corpse at respawn, when
// the server destroys it ahead of the fresh create).
Err(_) => {
- clear(&mut selection, &net_commands, !engaged.is_empty());
+ clear(&mut selection, &mut seam, !engaged.is_empty());
*last_vitals = None;
*logged_guid = None;
true
diff --git a/crates/benilla-app/src/target/scan.rs b/crates/benilla-app/src/target/scan.rs
index 4edc73f0..0a6bfa26 100644
--- a/crates/benilla-app/src/target/scan.rs
+++ b/crates/benilla-app/src/target/scan.rs
@@ -53,41 +53,11 @@ use bevy::prelude::*;
use crate::assets::{LockRecover, WorldAssets};
use crate::creature_anim::{Engaged, SwingMessage};
use crate::names::NameCache;
-use crate::net::{
- ClientCommand, Guid, NetCommands, NetEntity, ObjectStore, Reputations, SelfPlayer,
-};
+use crate::net::{ClientCommand, Guid, NetEntity, ObjectStore, Reputations, SelfPlayer};
+use super::relations::can_attack;
use super::{ring_reaction, Factions, Selection};
-/// The five `UNIT_FIELD_FLAGS` disqualifiers `CanAttack 0x606980` tests (bit positions
-/// byte-verified; names vmangos-corroborated): NON_ATTACKABLE(1), NOT_ATTACKABLE_1(7),
-/// NON_ATTACKABLE_2(16), TAXI_FLIGHT(20), NOT_SELECTABLE(25).
-pub(super) const FLAG_DISQUALIFIERS: u32 = (1 << 1) | (1 << 7) | (1 << 16) | (1 << 20) | (1 << 25);
-
-/// `CanAttack 0x606980` — the shared attackability predicate (the combat flash's gate and the
-/// scan's filter 3): the flag disqualifiers clear AND an attackable reaction. A unit without a
-/// store passes the flag leg (nothing known to disqualify). The duel/PVP legs are deferred with
-/// duels/PvP.
-///
-/// The reaction leg is **≤ neutral (rank ≤ 3)** — attackable means *not friendly*. First
-/// director-observed (0170), then **byte-confirmed** by the `0x606980` re-pin: the function has
-/// three reaction legs selected by `UNIT_FLAG_PVP_ATTACKABLE` (bit 3) on both parties, and the
-/// player-vs-creature case is Leg B — `UnitReaction(player→target) < 4` (`cmp eax,4; setl`),
-/// single direction, friendly-only blocked. The old "≤ 1 hostile" gloss was Leg A (both parties
-/// UN-flagged: NPC-vs-NPC, bidirectional), never the player's check. Leg C (both flagged: the
-/// PvP/duel/group machinery) is deferred with PvP.
-pub(crate) fn can_attack(
- store: Option<&ObjectStore>,
- factions: Option<&Factions>,
- reputations: &Reputations,
- self_store: Option<&ObjectStore>,
-) -> bool {
- if store.is_some_and(|s| s.0.unit_flags() & FLAG_DISQUALIFIERS != 0) {
- return false;
- }
- ring_reaction(factions, reputations, store, self_store) <= 3
-}
-
// == The Classic-priority dials ==
// Each names the Classic Era cvar it stands in for. The VALUES are tuned, not pinned — the
// binary's defaults are packer-encrypted; when the director's in-game `GetCVarInfo` dump lands,
@@ -498,7 +468,7 @@ pub(super) struct CommitOutcome {
/// the swing and does NOT re-point (the tail's self exception).
pub(super) fn commit(
selection: &mut Selection,
- net: &NetCommands,
+ seam: &mut crate::creature_anim::AttackSeam,
entity: Entity,
guid: u64,
engaged: bool,
@@ -513,12 +483,19 @@ pub(super) fn commit(
selection.guid = Some(guid);
let stop_and_repoint = engaged && had_old;
if stop_and_repoint {
- let _ = net.0.send(ClientCommand::AttackStop);
+ // `0x493a08 call 0x5ecac0` — the real StopAttack, so the switch also cancels a queued
+ // on-next-swing strike (the seam's `0x6e6f30` tail). Before this routed through the seam
+ // it was a bare `CMSG_ATTACKSTOP` and a queued Heroic Strike survived the switch.
+ seam.stop(engaged);
}
- let _ = net.0.send(ClientCommand::SetSelection { guid });
+ let _ = seam.net.0.send(ClientCommand::SetSelection { guid });
let swung = stop_and_repoint && new_attackable && self_guid != Some(guid);
if swung {
- let _ = net.0.send(ClientCommand::AttackSwing { guid });
+ // `0x4938c8 call 0x5ecb70` with a stop in flight (`[+0xc54]`, set by the call above), so
+ // the swing goes out despite the still-set lock. Its tail cancels a running auto-repeat —
+ // wow-re `melee-autorepeat-exclusion.md` §6 REFUTES `nocked-ammo-cancel.md`'s
+ // direct-callers-only census, which had this chain never reaching `0x6ea080`.
+ seam.start(guid, engaged, true);
}
CommitOutcome {
changed: true,
@@ -537,7 +514,7 @@ pub(super) fn tab_target(
scan: EnemyScan,
mut history: ResMut,
mut selection: ResMut,
- net: Res,
+ mut seam: crate::creature_anim::AttackSeam,
engaged: Query<(), (With, With)>,
) {
let reverse = binds.fired(crate::bindings::cmd::TARGET_PREVIOUS_ENEMY);
@@ -605,7 +582,7 @@ pub(super) fn tab_target(
}
let out = commit(
&mut selection,
- &net,
+ &mut seam,
entity,
guid,
!engaged.is_empty(),
@@ -650,7 +627,7 @@ pub(super) fn tab_target(
fn acquire_nearest_enemy(
scan: &EnemyScan,
selection: &mut Selection,
- net: &NetCommands,
+ seam: &mut crate::creature_anim::AttackSeam,
errors: &mut crate::ui_action::UiErrorKeys,
) -> Option<(Entity, u64)> {
let cands = scan.build();
@@ -666,7 +643,7 @@ fn acquire_nearest_enemy(
// `engaged = false`: the switch law only fires when we were already swinging at the OLD
// selection, and every path that reaches here had no selection or a non-hostile one — neither
// is a thing you can be engaged with.
- commit(selection, net, c.entity, c.guid, false, None, false);
+ commit(selection, seam, c.entity, c.guid, false, None, false);
Some((c.entity, c.guid))
}
@@ -703,14 +680,14 @@ fn acquire_nearest_enemy(
pub(crate) fn attack_order_target(
scan: &EnemyScan,
selection: &mut Selection,
- net: &NetCommands,
+ seam: &mut crate::creature_anim::AttackSeam,
errors: &mut crate::ui_action::UiErrorKeys,
) -> Option {
// `0x61306b` + `0x6130a3`: the selection is the candidate, and it survives only if the actor is
// hostile to it. Otherwise `0x6130b5` acquires.
let guid = match keeps_held_target(selection.guid, |g| scan.reaction_hostile(g)) {
Some(kept) => kept,
- None => acquire_nearest_enemy(scan, selection, net, errors)?.1,
+ None => acquire_nearest_enemy(scan, selection, seam, errors)?.1,
};
// `0x613167`'s final gate on whatever we ended up with. `0x61312e`'s legs are the ACTOR's and
// are Phase A's, already run by the caller; these two are the target's.
@@ -777,11 +754,9 @@ pub(super) fn acquire_and_attack(
mut requests: MessageReader,
scan: EnemyScan,
mut selection: ResMut,
- net: Res,
- self_player: Query>,
+ mut seam: crate::creature_anim::AttackSeam,
self_store: Query<&crate::net::ObjectStore, With>,
mut ui_error_keys: ResMut,
- mut sheath: MessageWriter,
) {
if requests.read().last().is_none() {
return;
@@ -809,20 +784,16 @@ pub(super) fn acquire_and_attack(
// `0x6130b5` and `0x6130d9`, shared with the pet bar's ATTACK arm — see
// [`acquire_nearest_enemy`]. The selection is empty (checked above), which is exactly the
// `candidate == 0` leg of [`attack_order_target`]; the follow-through below is what differs.
- let Some((_, guid)) = acquire_nearest_enemy(&scan, &mut selection, &net, &mut ui_error_keys)
+ let Some((_, guid)) =
+ acquire_nearest_enemy(&scan, &mut selection, &mut seam, &mut ui_error_keys)
else {
return;
};
debug!("attack acquire: best candidate {guid:#x} → select + swing");
- // The attack start (`0x6131a0` continuation): auto-draw (snap, decision 0080) + the swing.
- if let Ok(e) = self_player.single() {
- sheath.write(crate::creature_anim::SheathRequest {
- entity: e,
- state: 1,
- ceremony: false,
- });
- }
- let _ = net.0.send(ClientCommand::AttackSwing { guid });
+ // `0x6131a0`'s continuation — StartAttack through the one seam, so the acquire path gets the
+ // auto-draw AND the auto-repeat cancel its hand-rolled copy used to miss. The selection was
+ // empty on entry, so we cannot have been engaged: no stop is in flight.
+ seam.start(guid, false, false);
}
/// Auto-acquire the attacker (behavior 2): the ATTACKERSTATEUPDATE victim handler's
@@ -833,7 +804,7 @@ pub(super) fn auto_acquire_attacker(
self_player: Query>,
guids: Query<&Guid>,
mut selection: ResMut,
- net: Res,
+ mut seam: crate::creature_anim::AttackSeam,
) {
for s in swings.read() {
if selection.target.is_some() {
@@ -853,126 +824,138 @@ pub(super) fn auto_acquire_attacker(
guid.0
);
// Selection is empty here (checked above): no engaged-switch law, selection only.
- commit(&mut selection, &net, s.attacker, guid.0, false, None, false);
+ commit(
+ &mut selection,
+ &mut seam,
+ s.attacker,
+ guid.0,
+ false,
+ None,
+ false,
+ );
}
}
#[cfg(test)]
mod tests {
use super::*;
+ use crate::net::NetCommands;
/// The `SetSelection` wire law, byte-read from `0x493540` (the director's "TAB while
/// auto-attacking kills the attack" bug): an engaged SWITCH is **stop → select → re-swing**
/// — the attack follows the new target — while a plain select sends only the selection, a
/// self-target stops without re-pointing, and an unattackable new target (Attack `0x5ecb70`'s
/// validation) switches and stops but never swings.
+ ///
+ /// Driven through a World because `commit` now runs the two real seams
+ /// ([`crate::creature_anim::AttackSeam`]) rather than emitting bare packets — which is what
+ /// the second half of this test pins: the switch's stop un-queues an on-next-swing strike
+ /// (`0x493a08 call 0x5ecac0` → `0x6e6f30`) and its re-swing cancels a running auto-repeat
+ /// (`0x4938c8 call 0x5ecb70` → `0x5ecd8c`). Both were missing while this path hand-rolled
+ /// its own packets.
#[test]
fn commit_follows_the_stop_select_reswing_law() {
+ use bevy::ecs::system::RunSystemOnce;
+
let (tx, rx) = crossbeam_channel::unbounded();
- let net = NetCommands(tx);
- let mut selection = Selection::default();
+ let mut world = World::new();
+ world.insert_resource(NetCommands(tx));
+ world.init_resource::();
+ world.init_resource::();
+ world.init_resource::>();
+ world.init_resource::();
+ world.spawn(SelfPlayer);
+
let drain = |rx: &crossbeam_channel::Receiver| {
rx.try_iter()
.map(|c| match c {
ClientCommand::SetSelection { .. } => "select",
ClientCommand::AttackStop => "stop",
ClientCommand::AttackSwing { .. } => "swing",
+ ClientCommand::CancelCast { .. } => "cancel-cast",
+ ClientCommand::CancelAutoRepeat => "cancel-repeat",
_ => "other",
})
.collect::>()
};
+ // One `commit` through a one-shot system, returning its outcome.
+ fn go(
+ world: &mut World,
+ guid: u64,
+ engaged: bool,
+ self_guid: Option,
+ attackable: bool,
+ ) -> CommitOutcome {
+ world
+ .run_system_once(
+ move |mut selection: ResMut,
+ mut seam: crate::creature_anim::AttackSeam| {
+ commit(
+ &mut selection,
+ &mut seam,
+ Entity::PLACEHOLDER,
+ guid,
+ engaged,
+ self_guid,
+ attackable,
+ )
+ },
+ )
+ .expect("commit runs as a one-shot system")
+ }
// Not engaged: first select and a switch are selection-only; a same-guid re-commit dedups.
- assert!(
- commit(
- &mut selection,
- &net,
- Entity::PLACEHOLDER,
- 0xA,
- false,
- Some(1),
- true
- )
- .changed
- );
- assert!(
- commit(
- &mut selection,
- &net,
- Entity::PLACEHOLDER,
- 0xB,
- false,
- Some(1),
- true
- )
- .changed
- );
- assert!(
- !commit(
- &mut selection,
- &net,
- Entity::PLACEHOLDER,
- 0xB,
- false,
- Some(1),
- true
- )
- .changed
- );
+ assert!(go(&mut world, 0xA, false, Some(1), true).changed);
+ assert!(go(&mut world, 0xB, false, Some(1), true).changed);
+ assert!(!go(&mut world, 0xB, false, Some(1), true).changed);
assert_eq!(drain(&rx), ["select", "select"]);
// Engaged switch onto an attackable unit: the byte order stop → select → swing.
- let out = commit(
- &mut selection,
- &net,
- Entity::PLACEHOLDER,
- 0xC,
- true,
- Some(1),
- true,
- );
+ let out = go(&mut world, 0xC, true, Some(1), true);
assert!(out.changed && out.swung);
assert_eq!(drain(&rx), ["stop", "select", "swing"]);
// Engaged switch onto MYSELF (TargetUnit("player") mid-combat): stop, no re-point.
- let out = commit(
- &mut selection,
- &net,
- Entity::PLACEHOLDER,
- 0x1,
- true,
- Some(1),
- true,
- );
+ let out = go(&mut world, 0x1, true, Some(1), true);
assert!(out.changed && !out.swung);
assert_eq!(drain(&rx), ["stop", "select"]);
// Engaged switch onto an unattackable unit (vendor/corpse): stop, no swing at it.
- let out = commit(
- &mut selection,
- &net,
- Entity::PLACEHOLDER,
- 0xD,
- true,
- Some(1),
- false,
- );
+ let out = go(&mut world, 0xD, true, Some(1), false);
assert!(out.changed && !out.swung);
assert_eq!(drain(&rx), ["stop", "select"]);
// Engaged FIRST select (no old target): the [ebp-1] latch is off — selection only.
- let mut fresh = Selection::default();
- let out = commit(
- &mut fresh,
- &net,
- Entity::PLACEHOLDER,
- 0xE,
- true,
- Some(1),
- true,
- );
+ *world.resource_mut::() = Selection::default();
+ let out = go(&mut world, 0xE, true, Some(1), true);
assert!(out.changed && !out.swung);
assert_eq!(drain(&rx), ["select"]);
+
+ // **The two edges the seam brought.** A queued strike and a running auto-repeat, then an
+ // engaged switch onto an attackable unit: the stop un-queues the strike, the re-swing
+ // kills the repeat.
+ world
+ .resource_mut::()
+ .arm(78);
+ world.resource_mut::().0 = Some(75);
+ let out = go(&mut world, 0xF, true, Some(1), true);
+ assert!(out.changed && out.swung);
+ assert_eq!(
+ drain(&rx),
+ ["stop", "cancel-cast", "select", "swing", "cancel-repeat"]
+ );
+ assert_eq!(
+ world
+ .resource::()
+ .current(),
+ None,
+ "the switch un-queued Heroic Strike"
+ );
+ assert_eq!(
+ world.resource::().0,
+ None,
+ "and the re-swing killed Auto Shot"
+ );
}
fn cand(guid: u64, score: f32, on_screen: bool) -> Candidate {
diff --git a/crates/benilla-app/src/ui_action/cast_send.rs b/crates/benilla-app/src/ui_action/cast_send.rs
index 540ab3e5..f7ef7581 100644
--- a/crates/benilla-app/src/ui_action/cast_send.rs
+++ b/crates/benilla-app/src/ui_action/cast_send.rs
@@ -612,21 +612,18 @@ fn send_spell_cast(
// above means an auto-repeat can't be running while we swing in the first place.
if !engaged {
debug!("ui_action: cast {spell_id} initiates auto-attack at {guid:#x}");
- sheath.write(crate::creature_anim::SheathRequest {
- entity: e,
- state: 1,
- ceremony: false,
- });
- // `0x5ecb70`'s tail `0x5ecd78`–`0x5ecd95`: the melee sheath snap
- // (`0x611cf0(1,1,1)`) then the local auto-repeat cancel (`0x6ea080` at
- // `0x5ecd8c`) — you can't start swinging and keep auto-shooting.
- crate::creature_anim::cancel_auto_repeat_local(
- Some(e),
+ // No stop is ever in flight on this route: the gate above IS the ref's
+ // `6e51d2`, so we only get here with the lock clear.
+ crate::creature_anim::start_attack_local(
+ e,
+ guid,
+ engaged,
+ false,
auto_repeat,
+ sheath,
ecs,
commands,
);
- let _ = commands.0.send(ClientCommand::AttackSwing { guid });
}
}
}
@@ -881,13 +878,15 @@ mod tests {
assert!(
matches!(rx.try_recv(), Ok(ClientCommand::CastSpell { spell_id, .. }) if spell_id == RAPTOR_STRIKE)
);
- assert!(
- matches!(rx.try_recv(), Ok(ClientCommand::CancelAutoRepeat)),
- "the attack-start tail's `0x6ea080`"
- );
+ // The reference's order, which the hand-rolled copy this seam replaced had backwards:
+ // the swing send is `0x5eccfd`, the auto-repeat cancel is `0x5ecd8c` in the tail below it.
assert!(
matches!(rx.try_recv(), Ok(ClientCommand::AttackSwing { guid }) if guid == MOB),
- "and then the swing itself"
+ "the swing goes out first"
+ );
+ assert!(
+ matches!(rx.try_recv(), Ok(ClientCommand::CancelAutoRepeat)),
+ "then the attack-start tail's `0x6ea080`"
);
assert_eq!(
world.resource::().0,
diff --git a/crates/benilla-app/src/ui_action/drain.rs b/crates/benilla-app/src/ui_action/drain.rs
index 7a9ef441..e784207f 100644
--- a/crates/benilla-app/src/ui_action/drain.rs
+++ b/crates/benilla-app/src/ui_action/drain.rs
@@ -108,22 +108,25 @@ pub(super) fn attack_target_binding(
}
match targeting.selection.guid {
Some(guid) => {
- debug!("bindings: ATTACKTARGET swing at {guid:#x}");
- if let Ok((e, _)) = ladder.self_player.single() {
- ladder.sheath.write(crate::creature_anim::SheathRequest {
- entity: e,
- state: 1,
- ceremony: false,
- });
- }
- let self_e = ladder.self_player.single().ok().map(|(e, _)| e);
- crate::creature_anim::cancel_auto_repeat_local(
- self_e,
+ let Ok((e, engaged)) = ladder.self_player.single() else {
+ return;
+ };
+ debug!(
+ "bindings: ATTACKTARGET {} at {guid:#x}",
+ if engaged { "toggled off" } else { "swing" }
+ );
+ // The same `0x6131a0` this binding's doc says it shares with the action button — so
+ // it takes the same seam, toggle and all, instead of a second copy that drifts.
+ crate::creature_anim::toggle_attack_local(
+ e,
+ guid,
+ engaged,
+ &mut ladder.queued_melee,
&mut ladder.auto_repeat,
+ &mut ladder.sheath,
&mut ladder.ecs,
&ladder.commands,
);
- let _ = ladder.commands.0.send(ClientCommand::AttackSwing { guid });
}
None => {
debug!("bindings: ATTACKTARGET with no target — acquiring nearest");
@@ -168,29 +171,34 @@ pub(super) fn drain_action_uses(
}
match selection.guid {
Some(guid) => {
- debug!("ui_action: attack swing at {guid:#x}");
- // Auto-draw: initiating melee requests melee sheath state through the
- // anim layer's ONE setter (decision 0080) — a SNAP, no ceremony, no
- // sound: the attack path passes `(newState=1, bInstant=1, bFireEvent=1)`
- // at `0x5ecd80` (wow-re `sheath-policy.md`). The setter's idempotency
- // is the client's own "no-op if already melee".
- if let Ok((e, _)) = ladder.self_player.single() {
- ladder.sheath.write(crate::creature_anim::SheathRequest {
- entity: e,
- state: 1,
- ceremony: false,
- });
- }
- // Melee attack-start cancels a running auto-repeat UNCONDITIONALLY —
- // the client's `0x5ecd8c` (wow-re `nocked-ammo-cancel.md` §Q-B-5).
- let self_e = ladder.self_player.single().ok().map(|(e, _)| e);
- crate::creature_anim::cancel_auto_repeat_local(
- self_e,
+ // **The Attack button is a TOGGLE** — `0x6131a0`, which the base Attack
+ // pseudo-spell reaches through `TryCast`'s effect-0x4e short-circuit
+ // (`0x6e4c7a`), forks on `0x60ecb0`: already attacking →
+ // `0x6131d9 call 0x5ecac0` **StopAttack**, else `0x6131ee call 0x5ecb70`
+ // **StartAttack** (wow-re `melee-autorepeat-exclusion.md` §5f).
+ //
+ // Both halves were wrong here. There was no toggle-off at all, and the
+ // press cancelled a running auto-repeat *unconditionally* — but only the
+ // START arm reaches `0x5ecd8c`, so in the reference toggling melee OFF
+ // leaves Auto Shot running. The seams carry the sheath snap and the cancel
+ // now, so neither is spelled out twice.
+ let Ok((e, engaged)) = ladder.self_player.single() else {
+ continue;
+ };
+ debug!(
+ "ui_action: attack {} at {guid:#x}",
+ if engaged { "toggled off" } else { "swing" }
+ );
+ crate::creature_anim::toggle_attack_local(
+ e,
+ guid,
+ engaged,
+ &mut ladder.queued_melee,
&mut ladder.auto_repeat,
+ &mut ladder.sheath,
&mut ladder.ecs,
&ladder.commands,
);
- let _ = ladder.commands.0.send(ClientCommand::AttackSwing { guid });
}
// No target: the client's attack resolver runs the nearest-enemy core and
// swings at the winner (`0x612df0` @ `6130b5`) — `target::scan` answers.
diff --git a/crates/benilla-app/src/ui_aura.rs b/crates/benilla-app/src/ui_aura.rs
index 12bf0042..108239ef 100644
--- a/crates/benilla-app/src/ui_aura.rs
+++ b/crates/benilla-app/src/ui_aura.rs
@@ -49,8 +49,10 @@ use benilla_protocol::messages::{UnitAuraSlot, AURA_FLAG_CANCELABLE, UNIT_AURA_P
use benilla_ui::script::{AuraState, ScriptValue, TrackingState, UiScript};
use crate::char_select::ClientState;
-use crate::net::{ClientCommand, Guid, GuidIndex, NetCommands, ObjectStore, SelfPlayer};
-use crate::target::Selection;
+use crate::net::{
+ ClientCommand, Guid, GuidIndex, NetCommands, ObjectStore, Reputations, SelfPlayer,
+};
+use crate::target::{can_assist, Factions, Selection};
use crate::ui_action::Spells;
use crate::ui_script::UiInput;
use crate::ui_unit::UnitFeed;
@@ -298,10 +300,21 @@ fn projection_of(list: &[AuraState]) -> Vec {
/// only thing any of them can show is what `UNIT_FIELD_AURA` holds. Filtering here rather than in
/// the Lua binding keeps `UnitBuff(token, i)` returning the i-th *shown* aura, matching the
/// reference's own indices.
-fn other_unit_auras(store: &ObjectStore, catalog: Option<&SpellCatalog>) -> Vec {
+///
+/// `buffs_visible` is the **unit-level** gate (decision 1035): `UnitBuff 0x519500` decides, once
+/// per unit and *before* it walks a single slot, whether this unit's positive half is enumerable at
+/// all — see [`buffs_visible_on`]. False drops the helpful half entirely, which is exactly what the
+/// reference does (a single nil for every index), so `UnitBuff(token, i)` goes nil while
+/// `UnitDebuff(token, i)` is untouched: `UnitDebuff 0x5198f0` has no such gate.
+fn other_unit_auras(
+ store: &ObjectStore,
+ catalog: Option<&SpellCatalog>,
+ buffs_visible: bool,
+) -> Vec {
store
.0
.unit_auras()
+ .filter(|a| buffs_visible || a.slot >= UNIT_AURA_POSITIVE_SLOTS)
.filter(|a| shown_in_aura_ui(catalog, a.spell_id))
.map(|a| {
let display = catalog.and_then(|cat| cat.get(a.spell_id));
@@ -325,6 +338,50 @@ fn other_unit_auras(store: &ObjectStore, catalog: Option<&SpellCatalog>) -> Vec<
.collect()
}
+/// **`UnitBuff 0x519500`'s unit-level gate** — may this unit's *buffs* be enumerated at all?
+/// (Decision 1035; byte-derived in wow-re `ui/scratch/aura-display-pipeline.md` §9b.)
+///
+/// This sits **before** the per-slot walk and outranks it: when it fails, `UnitBuff` pushes a
+/// single nil for **every** index without examining one slot, so the unit shows *no buffs*
+/// whatever its aura array holds. `UnitDebuff 0x5198f0` has no counterpart — debuffs are always
+/// enumerable, which is why a hostile mob shows its debuffs and nothing else.
+///
+/// ```text
+/// (unit.UNIT_FIELD_FLAGS & UNIT_FLAG_AURAS_VISIBLE) // 0x08000000
+/// || (CanAssist(activePlayer, unit) && activePlayer.UNIT_FIELD_CHARMEDBY == 0)
+/// ```
+///
+/// The first clause is **Detect Magic** (spell 2855 → `SPELL_AURA_AURAS_VISIBLE`, vmangos
+/// `UnitDefines.h:518` *"Detect Magic. Added by SPELL_AURA_AURAS_VISIBLE"*). That spell exists
+/// precisely *because* the second clause denies a hostile unit's buffs — which is the independent
+/// gameplay corroboration that this gate is real and not an artifact of one screenshot.
+///
+/// **It is also why GM mode changes the answer**, and why the same mob read two ways for a while:
+/// vmangos ORs the bit into the outgoing `UNIT_FIELD_FLAGS` *for GameMasters only* (`Object.cpp`
+/// 765-766, *"Gamemasters should be always able to select units and view auras"*). So the server
+/// already tailors this per viewer and we need know nothing about GM state — reading the flag off
+/// the descriptor as received is both simpler and exactly right in both regimes.
+fn buffs_visible_on(
+ store: &ObjectStore,
+ self_store: Option<&ObjectStore>,
+ factions: Option<&Factions>,
+ reputations: &Reputations,
+ owner_store: impl FnOnce(u64) -> Option,
+) -> bool {
+ if store.0.unit_flags() & UNIT_FLAG_AURAS_VISIBLE != 0 {
+ return true;
+ }
+ // `activePlayer.UNIT_FIELD_CHARMEDBY == 0` — while something else is driving you, you assist
+ // nobody (`0x5195f1`-`0x5195f7`).
+ if self_store.is_some_and(|s| s.0.unit_charmed_by().is_some()) {
+ return false;
+ }
+ can_assist(Some(store), factions, reputations, self_store, owner_store)
+}
+
+/// `UNIT_FLAG_AURAS_VISIBLE` — see [`buffs_visible_on`].
+const UNIT_FLAG_AURAS_VISIBLE: u32 = 0x0800_0000;
+
/// The reference's aura display filter (decisions 0268 + 0417): an aura is shown iff its spell is
/// *not* flagged never-display (`SPELL_ATTR_DO_NOT_DISPLAY` / `SPELL_ATTR_EX_NO_AURA_ICON`, via
/// `SpellDisplay::hidden_from_aura_bar`) **and** is not a tracking spell
@@ -335,7 +392,16 @@ fn other_unit_auras(store: &ObjectStore, catalog: Option<&SpellCatalog>) -> Vec<
/// display — the player's own bar and any other unit's rows alike (the director watched the
/// reference hide a target's Battle Stance; 0417 corrects 0268's player-only scope note and
/// wow-re §9). A spell the catalog can't resolve stays visible — fail-open, like every other
-/// catalog miss in the feed (and like the reference's own no-SpellRec path, which inserts).
+/// catalog miss in the feed.
+///
+/// **That fail-open is ours, not the reference's, on this path** — the parenthetical that used to
+/// justify it here ("like the reference's own no-SpellRec path, which inserts") was true of the
+/// *player cache* (§3) and is FALSE of the non-player walk: `UnitBuff`/`UnitDebuff` **skip** a slot
+/// whose id has no `Spell.dbc` row (`id > [0xc0d78c]` or a null row; wow-re §9c, decision 1035).
+/// Kept as fail-open deliberately: every id on the wire is a real spell, so the two behaviours are
+/// indistinguishable in practice, and failing *closed* would let a catalog load hiccup silently
+/// blank every aura on every frame. A knowing divergence, recorded — not a comment asserting
+/// something untrue.
fn shown_in_aura_ui(catalog: Option<&SpellCatalog>, spell_id: u32) -> bool {
catalog
.and_then(|c| c.get(spell_id))
@@ -374,6 +440,11 @@ fn feed_auras(
// The pet half (decision 0990): the bar owns the pet's identity, the index resolves it.
pet: Res,
index: Res,
+ // The `UnitBuff` unit-level gate's inputs ([`buffs_visible_on`]) — the same reaction resolve
+ // the selection ring and the name plates run, so one law answers "is this unit friendly to me"
+ // everywhere.
+ factions: Option>,
+ reputations: Res,
// Read-only over the stamps: the feed joins them, the net apply path writes them, and only the
// session end drops them (decision 0900). Nothing the feed sees may invalidate one.
durations: Res,
@@ -530,7 +601,25 @@ fn feed_auras(
if guid == self_guid.0 {
return Some(list.clone());
}
- Some(other_unit_auras(stores.get(e).ok()?, catalog))
+ // NOT named `store` — that is the SELF player's, bound above and needed here as the
+ // gate's second argument.
+ let target_store = stores.get(e).ok()?;
+ // The gate is resolved per unit, once, exactly where the reference resolves it —
+ // before any slot is read (decision 1035).
+ let buffs = buffs_visible_on(
+ target_store,
+ Some(store),
+ factions.as_deref(),
+ &reputations,
+ |owner| {
+ index
+ .0
+ .get(&owner)
+ .and_then(|&e| stores.get(e).ok())
+ .cloned()
+ },
+ );
+ Some(other_unit_auras(target_store, catalog, buffs))
});
// The pet's list — the pet frame's four debuff buttons (decision 0990). Same other-unit law as
@@ -543,7 +632,13 @@ fn feed_auras(
.then(|| index.0.get(&pet_guid))
.flatten()
.and_then(|&e| stores.get(e).ok())
- .map(|store| other_unit_auras(store, catalog));
+ // **The pet keeps the ungated read, deliberately** (decision 1035). A pet is
+ // PLAYER_CONTROLLED, which sends `CanAssist` down an arm (`0x60673e`-`0x60679f`, keyed on
+ // an `[obj+0xe68]` record) wow-re did NOT name — so the gate is not known here, and
+ // guessing it is the one mistake that could blank a working pet frame. `true` is the
+ // status quo; the pet frame draws only debuffs today anyway (0990), so nothing observable
+ // rides on it until that arm is derived.
+ .map(|store| other_unit_auras(store, catalog, true));
let target_cur = selection
.guid
@@ -685,6 +780,10 @@ mod tests {
// the `"pet"` token stays cleared and the feed's pet leg is inert.
.init_resource::()
.init_resource::()
+ // The `UnitBuff` gate's reaction inputs (decision 1035). `Factions` is an
+ // `Option>` (it needs the DBC catalog), but `Reputations` is not — the feed
+ // reads it unconditionally, so a bare app must carry it or the system fails validation.
+ .init_resource::()
.insert_resource(NetCommands(tx))
.add_plugins(UiAuraPlugin);
app
@@ -971,4 +1070,108 @@ mod tests {
"expiry rebased onto the script clock, got {expiry}"
);
}
+ // == Decision 1035 — `UnitBuff 0x519500`'s unit-level gate ==
+ //
+ // The B216 case: a stealthed Ridge Stalker carries TWO auras that both resolve to
+ // `Ability_Stealth` — 5916 "Shadowstalker Stealth" in a POSITIVE slot (creature_addon) and
+ // 22766 "Sneak" in a NEGATIVE slot (EventAI; vmangos files it harmful because its
+ // MOD_DECREASE_SPEED effect is negative). We drew both. The reference draws only the debuff,
+ // because `UnitBuff` never enumerates a hostile creature's positive half at all.
+
+ /// `UNIT_FIELD_FLAGS` index — the fixture writes it directly (`from_pairs` is the public
+ /// fixture constructor; live code only ever decodes the wire).
+ const F_FLAGS: u16 = 46;
+ /// `UNIT_FIELD_CHARMEDBY`'s low dword — a guid pair, so the high half is the next index.
+ const F_CHARMEDBY: u16 = 10;
+
+ /// The B216 mob's two slots: 5916 helpful, 22766 harmful.
+ fn ridge_stalker_slots(flags: u32) -> ObjectStore {
+ ObjectStore(benilla_protocol::ObjectFields::from_pairs(&[
+ (F_FLAGS, flags),
+ (47, 5916), // UNIT_FIELD_AURA slot 0 — the positive half
+ (47 + 32, 22766), // slot 32 — the negative half
+ (95, 0x0000_0008), // AURAFLAGS slot 0 nibble 0x8 (eff0)
+ (95 + 4, 0x0000_0008), // slot 32 nibble 0x8
+ ]))
+ }
+
+ /// **The gate's consequence, both ways.** With buffs denied, `other_unit_auras` returns the
+ /// harmful half ONLY — which is what makes `UnitBuff("target", i)` nil for every i while
+ /// `UnitDebuff` is untouched, exactly as the reference behaves (a single nil for every buff
+ /// index, no slot examined). With buffs allowed, both come back.
+ #[test]
+ fn the_unit_gate_drops_the_whole_helpful_half_and_never_the_harmful_one() {
+ let store = ridge_stalker_slots(0);
+
+ let denied = other_unit_auras(&store, None, false);
+ assert_eq!(
+ denied.iter().map(|a| a.spell_id).collect::>(),
+ [22766],
+ "a hostile creature shows its debuffs and NOT its buffs — the B216 divergence"
+ );
+ assert!(denied.iter().all(|a| !a.helpful));
+
+ let allowed = other_unit_auras(&store, None, true);
+ assert_eq!(
+ allowed.iter().map(|a| a.spell_id).collect::>(),
+ [5916, 22766],
+ "with the gate open both halves enumerate, ascending slot"
+ );
+ }
+
+ /// **Detect Magic / GM mode short-circuits the gate.** `UNIT_FLAG_AURAS_VISIBLE` is clause
+ /// one, tested before any reaction work — and it is why the same mob reads two ways: vmangos
+ /// ORs the bit in for GameMasters only (`Object.cpp:765-766`), so a GM sees the buffs and an
+ /// ordinary player does not. Nothing here consults GM state; the flag arrives on the wire.
+ #[test]
+ fn detect_magic_or_gm_mode_opens_the_gate_on_the_flag_alone() {
+ let reputations = Reputations::default();
+ // No factions catalog → `ring_reaction` falls through to NEUTRAL (3), which FAILS the
+ // `>= 4` bar. So the flag is the only thing that can open the gate in this fixture.
+ let hostile = ridge_stalker_slots(0);
+ assert!(
+ !buffs_visible_on(&hostile, None, None, &reputations, |_| None),
+ "neutral reaction, not player-controlled, no PvP flag → no buffs"
+ );
+
+ let detected = ridge_stalker_slots(UNIT_FLAG_AURAS_VISIBLE);
+ assert!(
+ buffs_visible_on(&detected, None, None, &reputations, |_| None),
+ "UNIT_FLAG_AURAS_VISIBLE alone opens it, whatever the reaction says"
+ );
+ }
+
+ /// **`NOT_SELECTABLE` and the charm clause.** Both are outside the reaction ladder: the first
+ /// is `CanAssist`'s own first test, the second is `UnitBuff`'s `activePlayer.CHARMEDBY == 0`
+ /// (`0x5195f1`-`0x5195f7`) — while something else drives you, you assist nobody.
+ #[test]
+ fn not_selectable_and_being_charmed_each_close_the_gate_on_their_own() {
+ let reputations = Reputations::default();
+
+ // AURAS_VISIBLE would open it — but it is clause one and NOT_SELECTABLE lives inside
+ // CanAssist, so the flag still wins. Pin that ordering, it is the reference's.
+ let both = ridge_stalker_slots(UNIT_FLAG_AURAS_VISIBLE | (1 << 25));
+ assert!(
+ buffs_visible_on(&both, None, None, &reputations, |_| None),
+ "AURAS_VISIBLE is tested FIRST and short-circuits CanAssist entirely"
+ );
+
+ // Player-controlled + friendly would otherwise pass the permissive arm; being charmed
+ // ourselves closes it before CanAssist runs at all.
+ let friendly_pc = ridge_stalker_slots(0x8);
+ let charmed_self = ObjectStore(benilla_protocol::ObjectFields::from_pairs(&[
+ (F_CHARMEDBY, 0x1234),
+ (F_CHARMEDBY + 1, 0),
+ ]));
+ assert!(
+ !buffs_visible_on(
+ &friendly_pc,
+ Some(&charmed_self),
+ None,
+ &reputations,
+ |_| None
+ ),
+ "activePlayer.CHARMEDBY != 0 → no buffs on anyone"
+ );
+ }
}
diff --git a/crates/benilla-app/src/ui_cast.rs b/crates/benilla-app/src/ui_cast.rs
index 2e79c310..597585d5 100644
--- a/crates/benilla-app/src/ui_cast.rs
+++ b/crates/benilla-app/src/ui_cast.rs
@@ -188,15 +188,13 @@ impl PendingCast {
/// **an auto-repeat press** (`0x6e5976`), target change/death, mount, interact — never movement.
/// That chain is [`crate::creature_anim::stop_attack_local`], and the auto-repeat arm inside
/// [`crate::ui_action::cast_send`]'s commit runs it whole: starting Auto Shot un-queues the
-/// strike, which is what darkens its checked ring. **The two target-side StopAttack senders do
-/// not route through it yet** — `target::scan`'s `commit` (the engaged-switch law) and
-/// `target::click`'s `clear` still emit a bare `CMSG_ATTACKSTOP`, so a target switch or a
-/// click-off leaves the queue standing where the reference cancels it. The same two sites owe a
-/// second edge the 2026-08-06 §5 pinned: a mid-melee target switch reaches `Attack 0x5ecb70`
-/// (`SetSelection 0x493540` @ `0x4938c8`, gated on `0x60ecb0` already true) and therefore runs
-/// its `0x5ecd8c` **auto-repeat cancel** — refuting `nocked-ammo-cancel.md`'s direct-callers-only
-/// census. Threading the seam through those eight callers is the named follow-up; the wire still
-/// resolves the queue either way.
+/// strike, which is what darkens its checked ring. Every attack edge reaches that seam since
+/// 1044 — the Attack toggle, a target switch, a click-off, the ring's death teardown.
+///
+/// **Escape is the other route, and it is not that chain at all** (1049). In the reference a
+/// queued strike simply *is* the inflight spell, so `Script::SpellStopCasting 0x6e6e80`'s plain
+/// `IsCasting 0x6e3d30` branch cancels it like any cast — no melee-specific code anywhere in it.
+/// [`Inflight`] is where our two slots are re-joined for that reader.
#[derive(Resource, Default)]
pub(crate) struct QueuedMeleeSpell(Option);
@@ -221,6 +219,57 @@ impl QueuedMeleeSpell {
}
}
+/// What the reference's single inflight spell id (`0xceca88`) holds — as our **two** slots see it.
+///
+/// The client has one slot, with one save beneath it for the nesting. An ordinary cast and a
+/// queued on-next-swing strike both live in `0xceca88`, which is exactly why `IsCasting 0x6e3d30`
+/// — a bare `ceca88 != 0` — answers true for either, and why `AbortCast 0x6e4940` cancels
+/// whichever is there without ever asking which kind it was. [`PendingCast`] and
+/// [`QueuedMeleeSpell`] split that one slot in two (its doc says why), which leaves every reader
+/// of *"what is in flight"* to re-join them — and the one reader that forgot was the Esc ladder,
+/// which is the bug decision 1049 fixes. So the join gets a name, once, here.
+///
+/// **The order is the reference's nesting, not a preference.** An ordinary cast started over a
+/// queued strike is allowed through `TryCast`'s already-casting refusal precisely because the
+/// inflight rec has `Attributes & 0x404` (`0x6e4d97`/`0x6e4dbe`), and it *pushes*
+/// (`PushPopNestedCast 0x6e4ad0` @ `0x6e5026`): the cast becomes `0xceca88`, the strike the save
+/// in `0xcecaa8`. `AbortCast` cancels the cast and pops the strike back up — so the cast dies on
+/// the first Esc and the strike on the second. Reading `PendingCast` first reproduces that
+/// without our needing the push/pop pair.
+#[derive(Clone, Copy, Debug, PartialEq, Eq)]
+pub(crate) enum Inflight {
+ /// An ordinary cast — ours to send-guard, to paint a bar for, and to reap.
+ Cast(u32),
+ /// A queued on-next-swing strike waiting for the swing. No bar ever opened for it and it
+ /// holds no [`Casting`], so cancelling it is the wire message and the slot, nothing else.
+ Strike(u32),
+}
+
+impl Inflight {
+ /// The spell id in the slot — what a cancel names on the wire, either way.
+ pub(crate) fn spell_id(self) -> u32 {
+ match self {
+ Self::Cast(id) | Self::Strike(id) => id,
+ }
+ }
+}
+
+/// Resolve the reference's inflight slot from our two — see [`Inflight`] for the order and the
+/// why. `started` is the self [`Casting`] spell id, which covers a cast that never armed the send
+/// guard (an item use) exactly as the ref's one id covers it.
+pub(crate) fn inflight(
+ pending: &PendingCast,
+ queued_melee: &QueuedMeleeSpell,
+ started: Option,
+ now: Instant,
+) -> Option {
+ pending
+ .current(now)
+ .or(started)
+ .map(Inflight::Cast)
+ .or_else(|| queued_melee.current().map(Inflight::Strike))
+}
+
/// Slack past the server-named channel end, so a lost/late `MSG_CHANNEL_UPDATE(0)` can't pin the
/// checked ring on forever — the natural end clears through the deadline instead.
const CHANNEL_SLACK: Duration = Duration::from_secs(2);
@@ -335,6 +384,7 @@ fn local_self_cancel(
script: Option>,
mut moved: ResMut,
mut pending: ResMut,
+ mut queued_melee: ResMut,
channel: Res,
mut auto_repeat: ResMut,
mut feed: ResMut,
@@ -378,12 +428,16 @@ fn local_self_cancel(
.and_then(|s| s.catalog.get(spell_id))
.is_none_or(pick)
};
- // The cast half. The inflight id is the ref's `0xceca88` union: our own send guard
- // (pre-START, armed by the spell-cast send path only) OR the started self cast (`Casting`,
- // set at `SMSG_SPELL_START` for ANY cast — an item use's cast, which never arms the guard,
- // is still move/Esc-cancelable this way, exactly as the ref's one inflight id covers it).
+ // The cast half — the ref's ONE inflight slot ([`Inflight`]), which is an ordinary cast or a
+ // queued on-next-swing strike. `AbortCast` doesn't distinguish, so neither does the send or
+ // the movement gate; only the local teardown differs, because only a cast ever had a bar.
let started = self_e.and_then(|e| casting.get(e).ok()).map(|c| c.spell_id);
- if let Some(spell_id) = pending.current(now).or(started) {
+ if let Some(slot) = inflight(&pending, &queued_melee, started, now) {
+ let spell_id = slot.spell_id();
+ // Esc bypasses the flags gate; movement consults the slot's own `InterruptFlags`, which
+ // is what spares a queued strike (Heroic Strike 78, Cleave 845, Raptor Strike 2973 all
+ // ship `InterruptFlags = 0x0` — verified against Spell.dbc, vs Fireball's `0xf`). So
+ // charging in with a strike queued keeps it queued, and only Esc takes it.
if esc
|| flags_open(
|d| d.interrupt_flags & SPELL_INTERRUPT_MOVEMENT != 0,
@@ -392,29 +446,46 @@ fn local_self_cancel(
{
if *crate::net::CAST_TRACE {
info!(
- "cast-trace: LOCAL self-cancel — spell {spell_id} ({}), CMSG_CANCEL_CAST",
+ "cast-trace: LOCAL self-cancel — {slot:?} ({}), CMSG_CANCEL_CAST",
if esc { "esc" } else { "moved" }
);
}
let _ = net.0.send(ClientCommand::CancelCast { spell_id });
- pending.clear_if(spell_id);
- // The ref's two-step at RTT→0: STOP arms the bar's flash overlay, the echo's
- // INTERRUPTED repaints it red and starts the hold — so after the hold the flash
- // BURSTS white over the red bar, then the fade runs. One edge alone misses the
- // burst (which the enemy-interrupt path — no prior STOP — correctly lacks).
- feed.0.push(CastBarEdge::Stop);
- feed.0.push(CastBarEdge::Interrupted);
- // The reap — `spell_failed_other`'s self arm, run early so the echo finds it done.
- if let Some(e) = self_e {
- if casting.get(e).is_ok_and(|c| c.spell_id == spell_id) {
- ecs.entity(e).remove::();
+ match slot {
+ Inflight::Cast(_) => {
+ pending.clear_if(spell_id);
+ // The ref's two-step at RTT→0: STOP arms the bar's flash overlay, the echo's
+ // INTERRUPTED repaints it red and starts the hold — so after the hold the
+ // flash BURSTS white over the red bar, then the fade runs. One edge alone
+ // misses the burst (which the enemy-interrupt path — no prior STOP —
+ // correctly lacks).
+ feed.0.push(CastBarEdge::Stop);
+ feed.0.push(CastBarEdge::Interrupted);
+ // The reap — `spell_failed_other`'s self arm, run early so the echo finds it
+ // done.
+ if let Some(e) = self_e {
+ if casting.get(e).is_ok_and(|c| c.spell_id == spell_id) {
+ ecs.entity(e).remove::();
+ }
+ cast_events.write(CastEvent {
+ entity: e,
+ spell_id,
+ kind: CastEventKind::Fail,
+ seq: play_seq.next(),
+ });
+ }
+ }
+ Inflight::Strike(_) => {
+ // Clearing the slot IS the un-toggle: the checked ring reads it
+ // (`ui_action::state`), so the button darkens this frame instead of waiting
+ // for vmangos's FAILED_OTHER + CAST_RESULT echo (which then finds the slot
+ // already open and is idempotent). `AbortCast` fires its `0x152`
+ // SPELLCAST_STOP here too — inert on a bar that never opened
+ // (`CastingBar.xml` guards every arm on `IsShown`), so it is fired for
+ // fidelity, not effect. No red INTERRUPTED: there is nothing to repaint.
+ queued_melee.clear_if(spell_id);
+ feed.0.push(CastBarEdge::Stop);
}
- cast_events.write(CastEvent {
- entity: e,
- spell_id,
- kind: CastEventKind::Fail,
- seq: play_seq.next(),
- });
}
}
}
@@ -443,16 +514,22 @@ fn local_self_cancel(
/// argless STOP / FAILED / INTERRUPTED / CHANNEL_STOP. A channel update of `0` fires
/// CHANNEL_STOP — the server ends both the natural finish and the interrupt that way.
///
-/// Also pushes the frame's stoppable mirror (running auto-repeat OR cast in flight — NOT a
-/// channel, which `SpellStopCasting()` answers nil for: wow-re `esc-stopcasting.md`) into the
+/// Also pushes the frame's stoppable mirror (running auto-repeat OR the [`Inflight`] slot — NOT
+/// a channel, which `SpellStopCasting()` answers nil for: wow-re `esc-stopcasting.md`) into the
/// VM **after** [`local_self_cancel`] resolved, so the ESC chain's `SpellStopCasting()` reads
/// post-cancel truth when `UiInput` runs later this frame.
+///
+/// The mirror is what decides whether the press is EATEN, and that is half of decision 1049: a
+/// queued strike makes `IsCasting` true in the reference, so `SpellStopCasting()` returns `1` and
+/// `ToggleGameMenu`'s ladder never reaches `ClearTarget()` (`UIParent.lua` l.1489 vs l.1492).
+/// Reading only the ordinary-cast half here is what dropped the target on the first Esc.
#[allow(clippy::too_many_arguments)] // a Bevy system's full input set
fn feed_cast_bar(
script: Option>,
mut feed: ResMut,
spells: Option>,
pending: Res,
+ queued_melee: Res,
auto_repeat: Res,
self_guid: Res,
index: Res,
@@ -462,14 +539,18 @@ fn feed_cast_bar(
return;
};
let now = Instant::now();
- // The same pending ∪ started union as the cancel's (an item use's cast must answer
- // `SpellStopCasting()` truthily too), plus the auto-repeat key (`0xceac30`) it reads first.
+ // The same slot the cancel resolves, so the mirror and the drain can never disagree about
+ // whether a press had something to spend itself on — plus the auto-repeat key (`0xceac30`)
+ // the ref reads first.
let started = self_guid
.0
.as_ref()
.and_then(|g| index.0.get(g))
- .is_some_and(|&e| casting.get(e).is_ok());
- script.set_casting(auto_repeat.0.is_some() || pending.current(now).is_some() || started);
+ .and_then(|&e| casting.get(e).ok())
+ .map(|c| c.spell_id);
+ script.set_casting(
+ auto_repeat.0.is_some() || inflight(&pending, &queued_melee, started, now).is_some(),
+ );
let name = |id: u32| -> String {
spells
.as_ref()
@@ -660,6 +741,7 @@ mod tests {
use std::collections::HashMap;
const ARCANE_MISSILES: u32 = 5143;
+ const HEROIC_STRIKE: u32 = 78;
fn harness(
displays: HashMap,
@@ -669,6 +751,7 @@ mod tests {
app.add_message::()
.init_resource::()
.init_resource::()
+ .init_resource::()
.init_resource::()
.init_resource::()
.init_resource::()
@@ -967,6 +1050,148 @@ mod tests {
);
}
+ /// **The director's report, decision 1049: Esc with Heroic Strike queued un-toggles the
+ /// STRIKE and keeps the target.** In the reference a queued on-next-swing spell simply
+ /// *is* the inflight spell (`0xceca88`), so `SpellStopCasting 0x6e6e80` takes its plain
+ /// `IsCasting 0x6e3d30` branch — `AbortCast` → `CMSG_CANCEL_CAST` — and returns `1`,
+ /// which EATS the press so `ToggleGameMenu`'s ladder never reaches `ClearTarget()`. We
+ /// split that one slot in two and the Esc reader saw only the cast half, so the binding
+ /// answered nil: the strike stayed lit *and* the target dropped, both from one cause.
+ #[test]
+ fn esc_unqueues_a_strike_and_eats_the_press() {
+ let (mut app, rx) = harness(HashMap::from([(
+ HEROIC_STRIKE,
+ SpellDisplay {
+ attributes: 0x5_0014, // the shipped row: on-next-swing (`& 0x404`)
+ interrupt_flags: 0, // …and no movement bit, also shipped
+ ..Default::default()
+ },
+ )]));
+ app.insert_non_send_resource(UiScript::new().unwrap());
+ app.world_mut()
+ .resource_mut::()
+ .arm(HEROIC_STRIKE);
+
+ app.world_mut().run_system_once(feed_cast_bar).unwrap();
+ assert_eq!(
+ app.world()
+ .non_send_resource::()
+ .eval::>("return SpellStopCasting()")
+ .unwrap(),
+ Some(1),
+ "the queued strike makes IsCasting true, so the binding EATS the press — which \
+ is what spares the target three rungs further down the ladder"
+ );
+ app.world_mut().run_system_once(local_self_cancel).unwrap();
+ assert!(
+ matches!(
+ rx.try_recv(),
+ Ok(crate::net::ClientCommand::CancelCast {
+ spell_id: HEROIC_STRIKE
+ })
+ ),
+ "AbortCast names the queued strike on the wire, exactly as it would a cast"
+ );
+ assert_eq!(
+ app.world().resource::().current(),
+ None,
+ "the slot opens locally — the checked ring darkens this frame, not one RTT later"
+ );
+ assert!(
+ matches!(
+ app.world().resource::().0[..],
+ [CastBarEdge::Stop]
+ ),
+ "the ref's `0x152` STOP and nothing else: no red INTERRUPTED for a spell that \
+ never opened a bar"
+ );
+ }
+
+ /// …but MOVEMENT does not take it. The movement path consults the inflight spell's own
+ /// `InterruptFlags`, and Heroic Strike 78 / Cleave 845 / Raptor Strike 2973 all ship
+ /// `InterruptFlags = 0x0` (read off the shipped `Spell.dbc`; Fireball is `0xf`). So
+ /// charging in with a strike queued keeps it queued — Esc is the only local taker.
+ #[test]
+ fn a_move_edge_leaves_a_queued_strike_queued() {
+ let (mut app, rx) = harness(HashMap::from([(
+ HEROIC_STRIKE,
+ SpellDisplay {
+ interrupt_flags: 0,
+ ..Default::default()
+ },
+ )]));
+ app.world_mut()
+ .resource_mut::()
+ .arm(HEROIC_STRIKE);
+
+ run(&mut app, true);
+
+ assert!(rx.try_recv().is_err(), "running at the mob cancels nothing");
+ assert_eq!(
+ app.world().resource::().current(),
+ Some(HEROIC_STRIKE)
+ );
+ }
+
+ /// The nesting order (`Inflight`'s doc): an ordinary cast started over a queued strike
+ /// pushes it down (`PushPopNestedCast 0x6e4ad0`), so `0xceca88` holds the CAST. One Esc
+ /// takes the cast; `AbortCast`'s pop restores the strike, and the next Esc takes that.
+ /// One press, one thing — the ladder's law all the way down.
+ #[test]
+ fn esc_takes_the_cast_first_and_the_strike_on_the_next_press() {
+ let (mut app, rx) = harness(HashMap::new());
+ app.insert_non_send_resource(UiScript::new().unwrap());
+ app.world_mut()
+ .resource_mut::()
+ .arm(HEROIC_STRIKE);
+ app.world_mut()
+ .resource_mut::()
+ .arm(FIREBALL, Instant::now());
+ mark_casting(&mut app, FIREBALL);
+
+ app.world_mut().run_system_once(feed_cast_bar).unwrap();
+ assert_eq!(
+ app.world()
+ .non_send_resource::()
+ .eval::>("return SpellStopCasting()")
+ .unwrap(),
+ Some(1)
+ );
+ app.world_mut().run_system_once(local_self_cancel).unwrap();
+ assert!(
+ matches!(
+ rx.try_recv(),
+ Ok(crate::net::ClientCommand::CancelCast { spell_id: FIREBALL })
+ ),
+ "the nested cast is what sits in the inflight slot — it dies first"
+ );
+ assert!(rx.try_recv().is_err());
+ assert_eq!(
+ app.world().resource::().current(),
+ Some(HEROIC_STRIKE),
+ "the strike is the SAVE underneath, untouched by the first press"
+ );
+
+ // Press 2 — the pop put the strike back in the slot.
+ app.world_mut().run_system_once(feed_cast_bar).unwrap();
+ assert_eq!(
+ app.world()
+ .non_send_resource::()
+ .eval::>("return SpellStopCasting()")
+ .unwrap(),
+ Some(1),
+ "still stoppable — so this press is eaten too, and the target still survives"
+ );
+ app.world_mut().run_system_once(local_self_cancel).unwrap();
+ assert!(matches!(
+ rx.try_recv(),
+ Ok(crate::net::ClientCommand::CancelCast {
+ spell_id: HEROIC_STRIKE
+ })
+ ));
+ assert_eq!(app.world().resource::().current(), None);
+ }
+
/// The channel half is the SEND and nothing else — the verified asymmetry (0445): the
/// ref's `0x6e9b70` ships CMSG_CANCEL_CHANNELLING but fires no FrameScript event and
/// clears no state; the channel bar and the mirror both close on the server's
diff --git a/crates/benilla-app/src/ui_pet.rs b/crates/benilla-app/src/ui_pet.rs
index 15ab3921..412a8286 100644
--- a/crates/benilla-app/src/ui_pet.rs
+++ b/crates/benilla-app/src/ui_pet.rs
@@ -681,6 +681,7 @@ fn drain_pet_actions(
spells: Option>,
mut ui_errors: ResMut,
scan: crate::target::EnemyScan,
+ mut seam: crate::creature_anim::AttackSeam,
) {
let Some(mut script) = script else {
return;
@@ -747,7 +748,7 @@ fn drain_pet_actions(
|| match crate::target::attack_order_target(
&scan,
&mut selection,
- &commands,
+ &mut seam,
&mut ui_errors,
) {
Some(guid) => {
diff --git a/crates/benilla-app/src/ui_pet_book.rs b/crates/benilla-app/src/ui_pet_book.rs
new file mode 100644
index 00000000..fbabd9fd
--- /dev/null
+++ b/crates/benilla-app/src/ui_pet_book.rs
@@ -0,0 +1,434 @@
+//! The **pet spellbook** — the spellbook window's second tab (decision 1032).
+//!
+//! Its source has been on the wire and parsed since 0988 and nothing read it:
+//! [`benilla_protocol::messages::PetSpells::spells`], the packed-word list `SMSG_PET_SPELLS`
+//! carries after the ten bar slots. This module is the join that turns it into a book.
+//!
+//! **It is not the player book with a flag on it**, and the three differences are the reason this
+//! is its own module rather than an arm of [`crate::ui_spellbook`]:
+//!
+//! - **A different add-gate.** `0x4b2f90` keeps a spell out only on `Attributes & 0x80`
+//! (`DO_NOT_DISPLAY`) — `SpellDisplay::in_pet_book`. The player book's `IS_TRADESKILL` and
+//! `castUI` legs are simply absent.
+//! - **No tabs.** `GetNumSpellTabs`/`GetSpellTabInfo` never see this book;
+//! `SpellBook_GetSpellID`'s pet arm is a bare `return id`, so a button's own 1..12 id *is* the
+//! book id.
+//! - **Two extra per-slot answers the player book has no analogue for** — the autocast pair, read
+//! off the pet's **raw** words (bits 31/30) rather than the filtered book, and the packed word
+//! itself, which is what `PickupSpell(id, "pet")` puts on the cursor.
+//!
+//! The **order** is shared, though: `0x4b2fd0(ecx = 0, edx = 1)` sorts the pet array with
+//! `0x4b30c0`, the player book's own comparator (name, then the parsed rank number), so
+//! [`crate::ui_spellbook::spell_sort_key`] serves both.
+//!
+//! ## Where each answer comes from
+//!
+//! | Lua | source |
+//! |---|---|
+//! | `GetSpellName` / `GetSpellTexture` / `IsSpellPassive` | `Spell.dbc` via [`Spells`] |
+//! | `GetSpellCooldown(id, "pet")` | **bank 1** — [`PetBar::cooldowns`], the pet's own store. The reference reaches it with the same `0x6e2ea0(edx = isPet)` `GetPetActionCooldown` uses (`0x4b40dd`), so a spell on the bar and the same spell in the book read one timer, and they do here too. |
+//! | `GetSpellAutocast` | the raw word's bits 31/30 |
+//! | `IsCurrentCast` | the pet's own auras — `0x4b36f0`'s pet arm scans `[pet.fields + 0xa4]`, which is exactly the predicate the bar's [`crate::ui_pet`] already applies per slot |
+//! | `HasPetSpells`'s token | `ChrClasses.dbc` field 4 (`benilla_formats::PetNameTokens`) |
+//!
+//! ## The two verbs
+//!
+//! Both are pet verbs, and neither is the player book's:
+//!
+//! - **`CastSpell(id, "pet")` → `CMSG_PET_ACTION`** with a **synthesized** type-1 word,
+//! `0x0100_0000 | (spellId & 0xFFFF)` (`0x4b350a`-`0x4b3516`), aimed at the passed target or,
+//! absent one, the current selection (`0x4b34af`). So the book can cast a pet spell that is not
+//! on the bar at all — the word is built for the send, never looked up.
+//! - **`ToggleSpellAutocast(id, "pet")` → `CMSG_PET_SPELL_AUTOCAST 0x2F3`**, naming the **spell**.
+//! Its sender `0x4bccb0` does three things before the packet leaves, and all three are
+//! reproduced in [`drain_pet_book`]: it flips bit 30 in the raw word in place, it **mirrors that
+//! bit onto every bar slot carrying the same action** (`0x4bcd20`-`0x4bcd5a`, compared under
+//! `& 0x3FFFFFFF`), and it fires `PET_BAR_UPDATE`. That mirror is why toggling autocast in the
+//! book lights the sparkle on the bar button in the same frame.
+
+use std::time::Instant;
+
+use bevy::prelude::*;
+
+use benilla_formats::{PetNameTokens, SpellDisplay};
+use benilla_protocol::messages::PetActionEntry;
+use benilla_ui::script::{PetBookState, SpellSlotView, UiScript};
+
+use crate::assets::{AssetSet, LockRecover, WorldAssets};
+use crate::net::{ClientCommand, GuidIndex, NetCommands, ObjectStore, SelfPlayer};
+use crate::target::Selection;
+use crate::ui_action::Spells;
+use crate::ui_pet::PetBar;
+use crate::ui_script::UiInput;
+use crate::ui_unit::UnitFeed;
+
+/// `ChrClasses.dbc`'s pet-name-token column — absent when the client data failed to load, in which
+/// case [`benilla_ui`]'s own `"PET"` fallback stands (the reference's `0x4b44a6` arm).
+#[derive(Resource)]
+pub(crate) struct PetNameTokenTable(pub(crate) PetNameTokens);
+
+pub(crate) struct UiPetBookPlugin;
+
+impl Plugin for UiPetBookPlugin {
+ fn build(&self, app: &mut App) {
+ app.add_systems(Startup, load_pet_name_tokens.after(AssetSet::Open))
+ .add_systems(
+ Update,
+ (
+ // Rides the unit feed beside the player book's, and before the input pass so a
+ // tab flipped this frame already reads a populated book.
+ feed_pet_book
+ .in_set(UnitFeed)
+ .before(crate::ui_action::CooldownEvents)
+ .before(UiInput),
+ // After the input pass, and — like `ui_pet`'s own drain — writing back into
+ // `PetBar`, whose next feed carries the mirrored autocast bit.
+ drain_pet_book.after(UiInput),
+ ),
+ );
+ }
+}
+
+fn load_pet_name_tokens(mut commands: Commands, assets: Option>) {
+ let Some(assets) = assets else { return };
+ let loaded = {
+ let mut chain = assets.chain.lock_recover();
+ benilla_formats::load_pet_name_tokens(&mut chain)
+ };
+ match loaded {
+ Ok(tokens) => commands.insert_resource(PetNameTokenTable(tokens)),
+ Err(e) => warn!(
+ "ui_pet_book: ChrClasses.dbc failed to load — every pet book tab reads the client's \
+ own \"PET\" fallback, so a warlock's says Pet rather than Demon: {e:#}"
+ ),
+ }
+}
+
+/// What the feed last pushed, so `SPELLS_CHANGED` fires on a real change rather than every frame
+/// (the [`crate::ui_spellbook`] memory pattern).
+#[derive(Default)]
+struct FeedMemory {
+ pushed: PetBookState,
+}
+
+#[allow(clippy::too_many_arguments)] // a Bevy system's full input set
+fn feed_pet_book(
+ script: Option>,
+ bar: Res,
+ spells: Option>,
+ tokens: Option>,
+ self_q: Query<&ObjectStore, With>,
+ index: Res,
+ stores: Query<&ObjectStore>,
+ clock: Res,
+ mut memory: Local,
+) {
+ let Some(mut script) = script else {
+ return;
+ };
+ // Nothing to resolve a name/icon/passive from yet — try again once Spell.dbc lands. Unlike the
+ // bar (which can still draw its command tokens), a book with no catalog has no slots at all.
+ let Some(spells) = spells.as_deref() else {
+ return;
+ };
+ let now = Instant::now();
+ let (anchor, ui_now) = (clock.anchor, clock.ui_now);
+ let pet_store = index
+ .0
+ .get(&bar.spells.pet_guid)
+ .and_then(|&e| stores.get(e).ok());
+
+ let mut fresh = PetBookState {
+ // The token is the PLAYER's class, not the pet's — `0x4b4463` reads `player.fields + 0x79`.
+ // A warlock reading "Demon" while a hunter reads "Pet" is the whole of it.
+ token: self_q.single().ok().and_then(|s| {
+ let class = u32::from(s.0.unit_class()?);
+ tokens.as_deref().map(|t| t.0.token(class).to_string())
+ }),
+ slots: Vec::new(),
+ };
+ if bar.has_bar() {
+ for &entry in &bar.spells.spells {
+ let spell_id = entry.action();
+ let Some(d) = spells.catalog.get(spell_id) else {
+ // `0x4b2f90`'s first gate: no `Spell.dbc` record, no book slot.
+ continue;
+ };
+ if !d.in_pet_book() {
+ continue;
+ }
+ fresh
+ .slots
+ .push(slot_view(entry, d, &bar, pet_store, now, anchor, ui_now));
+ }
+ // `0x4b2fd0`'s re-sort, with the player book's own comparator.
+ fresh
+ .slots
+ .sort_by_key(|s| crate::ui_spellbook::spell_sort_key_of(&s.name, s.rank.as_deref()));
+ }
+
+ if fresh != memory.pushed {
+ debug!("ui_pet_book: fed {} pet spell(s)", fresh.slots.len());
+ let changed = book_changed(&fresh, &memory.pushed);
+ script.set_pet_book(fresh.clone());
+ memory.pushed = fresh;
+ // The reference fires ONE `SPELLS_CHANGED` for both books off the shared re-sort
+ // (`0x4b2fd0` tail-jumps `SignalEvent(0x104)`), which is also what re-reads the spell
+ // buttons. The cooldown/autocast half rides `PET_BAR_UPDATE` instead — `SpellButton_OnLoad`
+ // registers it precisely so the pet page can repaint without a book change
+ // (`SpellBookFrame.lua:214`, `l.227-231`) — and `ui_pet`'s own feed already fires that.
+ if changed {
+ script.fire_event("SPELLS_CHANGED", vec![]);
+ }
+ }
+}
+
+/// Did the book itself move, as opposed to a per-slot cooldown/autocast/ring bit? Only the former
+/// is `SPELLS_CHANGED`; the latter reaches the buttons through `PET_BAR_UPDATE`.
+fn book_changed(fresh: &PetBookState, old: &PetBookState) -> bool {
+ fresh.token != old.token
+ || fresh.slots.len() != old.slots.len()
+ || fresh.slots.iter().zip(&old.slots).any(|(a, b)| {
+ (a.spell_id, &a.name, &a.rank, &a.texture, a.passive)
+ != (b.spell_id, &b.name, &b.rank, &b.texture, b.passive)
+ })
+}
+
+/// One book slot, fully resolved.
+#[allow(clippy::too_many_arguments)]
+fn slot_view(
+ entry: PetActionEntry,
+ d: &SpellDisplay,
+ bar: &PetBar,
+ pet_store: Option<&ObjectStore>,
+ now: Instant,
+ anchor: Instant,
+ ui_now: f64,
+) -> SpellSlotView {
+ let spell_id = entry.action();
+ SpellSlotView {
+ spell_id,
+ name: d.name.clone(),
+ rank: d.rank.clone(),
+ texture: d.icon.clone(),
+ passive: d.passive,
+ // `IsCurrentCast`'s pet arm (`0x4b36f0`, and the delegate `0x4b3600` it shares a shape
+ // with): the ring lights while the spell's aura is on the PET. Same predicate the bar's
+ // ActiveIcon swap uses, so a lit book slot and a lit bar slot always agree.
+ current: pet_store
+ .is_some_and(|s| crate::ui_action::toggle::active_action_toggle(spell_id, d, s)),
+ // **Bank 1** — the pet's own store, the same one `GetPetActionCooldown` reads.
+ cooldown: bar
+ .cooldowns
+ .info(spell_id, 0, Some(d), now)
+ .ui_triple(anchor, ui_now),
+ // The autocast pair off the RAW word. `autocast_allowed` is additionally gated on the
+ // spell resolving in `Spell.dbc` (`0x4bdd65`'s own second test) — which it has, or we
+ // would not be here.
+ autocast: Some((entry.autocast_allowed(), entry.autocast_on())),
+ packed: entry.packed,
+ }
+}
+
+/// Drain the pet book's two intents.
+fn drain_pet_book(
+ script: Option>,
+ mut bar: ResMut,
+ selection: Res,
+ commands: Res,
+ spells: Option>,
+ pet: crate::ui_pet::PetUnit,
+) {
+ let Some(mut script) = script else {
+ return;
+ };
+ let casts = script.take_pet_spell_casts();
+ let autocasts = script.take_pet_spell_autocasts();
+ if casts.is_empty() && autocasts.is_empty() {
+ return;
+ }
+ let pet_guid = bar.spells.pet_guid;
+ if pet_guid == 0 {
+ debug!("ui_pet_book: dropping queued pet book intents — the bar is gone");
+ return;
+ }
+ let target_guid = selection.guid.unwrap_or(0);
+ let pet_store = pet.store(pet_guid);
+
+ for spell_id in casts {
+ // **The press-again-to-cancel arm comes first** (`0x4b33af`-`0x4b3461`), exactly as it does
+ // on the bar: a click on a pet spell whose aura is already running on the pet sends
+ // `CMSG_PET_CANCEL_AURA` (0x26B) **and returns** — the ordinary order never leaves. Same
+ // predicate, same order of tests, one implementation
+ // (`crate::ui_action::toggle::active_action_toggle`).
+ let display = spells.as_ref().and_then(|s| s.catalog.get(spell_id));
+ if let (Some(d), Some(store)) = (display, pet_store) {
+ if crate::ui_action::toggle::active_action_toggle(spell_id, d, store) {
+ debug!("ui_pet_book: cast {spell_id} cancels the pet's own aura");
+ let _ = commands
+ .0
+ .send(ClientCommand::PetCancelAura { pet_guid, spell_id });
+ continue;
+ }
+ }
+ // The synthesized word, `0x4b350a`-`0x4b3516`: `(spellId & 0xFFFF) | 0x01000000`. Type 1
+ // is the client's spell branch and the autocast bits are left clear — this word describes
+ // the ORDER, not the slot, so nothing is read back out of it.
+ let packed = 0x0100_0000 | (spell_id & 0xFFFF);
+ debug!("ui_pet_book: cast {spell_id} (target {target_guid:#x})");
+ let _ = commands.0.send(ClientCommand::PetAction {
+ pet_guid,
+ packed,
+ target_guid,
+ });
+ }
+
+ for spell_id in autocasts {
+ let Some(on) = flip_autocast(&mut bar.spells, spell_id) else {
+ continue;
+ };
+ // `0x4bcdc6`'s `SignalEvent(0x161)` — the bar repainted before the packet left, which is
+ // the same "write locally, never wait for a reply" law the rest of this bar runs on
+ // (`ui_pet`'s module doc). Bumping the signal count is how that repaint reaches the feed.
+ bar.bar_signals = bar.bar_signals.wrapping_add(1);
+ debug!("ui_pet_book: autocast {spell_id} -> {on}");
+ let _ = commands.0.send(ClientCommand::PetSpellAutocast {
+ pet_guid,
+ spell_id,
+ enabled: on,
+ });
+ }
+}
+
+/// `0x4bccb0`'s local half, in its own order: find the spell's **raw** word, flip bit 30 in place,
+/// and mirror the new bit onto every **bar** slot carrying the same action. Returns the new state,
+/// or `None` when the reference would have aborted before sending anything.
+///
+/// The mirror is the load-bearing half and the easy one to miss (`0x4bcd20`-`0x4bcd5a`): the loop
+/// walks all ten bar words, compares `(bar[i] ^ word) & 0x3FFFFFFF` — type and action, autocast
+/// bits excluded — and copies bit 30 across on a match. Without it, flipping autocast in the book
+/// leaves the bar button's sparkle stale until the next `SMSG_PET_SPELLS`.
+///
+/// The gate is `0x4bccf5`: **the word must be autocast-ALLOWED** (bit 31). A passive's is not, and
+/// neither is a command token's — so neither can be toggled from anywhere.
+fn flip_autocast(
+ spells: &mut benilla_protocol::messages::PetSpells,
+ spell_id: u32,
+) -> Option {
+ let word = spells
+ .spells
+ .iter_mut()
+ .find(|w| w.action() == spell_id && w.autocast_allowed())?;
+ let on = !word.autocast_on();
+ *word = word.with_autocast(on);
+ let action = word.packed & 0x3FFF_FFFF;
+ for slot in &mut spells.bar {
+ if slot.packed & 0x3FFF_FFFF == action {
+ *slot = slot.with_autocast(on);
+ }
+ }
+ Some(on)
+}
+
+#[cfg(test)]
+mod tests {
+ use super::*;
+ use benilla_protocol::messages::{
+ PetSpells, PET_ACT_DISABLED, PET_ACT_ENABLED, PET_ACT_PASSIVE,
+ };
+
+ fn word(kind: u8, spell_id: u32) -> PetActionEntry {
+ PetActionEntry::from((u32::from(kind) << 24) | spell_id)
+ }
+
+ /// **The pet book's add-gate is not the player book's.** `0x4b2f90` tests exactly one bit —
+ /// `Attributes & 0x80` (DO_NOT_DISPLAY) — where the player book also refuses `IS_TRADESKILL`
+ /// (`0x20`) and `castUI != 0`. A spell with either of those two SHOWS in a pet book.
+ #[test]
+ fn the_pet_books_gate_is_do_not_display_alone() {
+ let shown = |attributes: u32, cast_ui: u32| SpellDisplay {
+ attributes,
+ cast_ui,
+ ..Default::default()
+ };
+ assert!(shown(0, 0).in_pet_book());
+ assert!(!shown(0x80, 0).in_pet_book(), "DO_NOT_DISPLAY hides it");
+ // The two the PLAYER book additionally refuses — kept here, and this is the whole point.
+ assert!(
+ shown(0x20, 0).in_pet_book(),
+ "IS_TRADESKILL is not a pet-book gate"
+ );
+ assert!(!shown(0x20, 0).in_spellbook());
+ assert!(shown(0, 1).in_pet_book(), "castUI is not a pet-book gate");
+ assert!(!shown(0, 1).in_spellbook());
+ // A passive is shown in both — being passive was never a book gate.
+ assert!(shown(0x40, 0).in_pet_book() && shown(0x40, 0).in_spellbook());
+ }
+
+ /// The autocast toggle's local half (`0x4bccb0`): the flip lands on the raw **book** word AND
+ /// on every **bar** slot carrying the same action, compared under `& 0x3FFFFFFF`. Missing the
+ /// mirror leaves the bar button's sparkle stale until the next `SMSG_PET_SPELLS`.
+ #[test]
+ fn the_book_toggle_mirrors_onto_the_bar() {
+ const CLAW: u32 = 16827;
+ let mut spells = PetSpells {
+ pet_guid: 0x2A,
+ spells: vec![
+ word(PET_ACT_DISABLED, CLAW), // autocastable, currently OFF
+ word(PET_ACT_PASSIVE, 3025), // a passive: not autocastable
+ ],
+ ..Default::default()
+ };
+ // Claw sits in bar slot 4; slot 5 holds a DIFFERENT spell with the same low bits pattern
+ // is impossible, so use a plain second spell to prove the compare is by action.
+ spells.bar[4] = word(PET_ACT_DISABLED, CLAW);
+ spells.bar[5] = word(PET_ACT_DISABLED, 2649);
+
+ assert_eq!(flip_autocast(&mut spells, CLAW), Some(true));
+ assert!(spells.spells[0].autocast_on(), "the book word flipped");
+ assert!(spells.bar[4].autocast_on(), "…and so did the bar slot");
+ assert!(
+ !spells.bar[5].autocast_on(),
+ "a different action is untouched"
+ );
+
+ // Flipping back does the same in reverse.
+ assert_eq!(flip_autocast(&mut spells, CLAW), Some(false));
+ assert!(!spells.spells[0].autocast_on());
+ assert!(!spells.bar[4].autocast_on());
+
+ // `0x4bccf5`: a word that is not autocast-ALLOWED aborts before anything is written.
+ assert_eq!(
+ flip_autocast(&mut spells, 3025),
+ None,
+ "a passive cannot be toggled"
+ );
+ assert_eq!(
+ flip_autocast(&mut spells, 99),
+ None,
+ "nor can a spell the pet lacks"
+ );
+ }
+
+ /// A word already ON flips OFF and an `ACT_ENABLED` word starts ON — the two server states our
+ /// decode collapses onto one type (`0988`), which is exactly why the flip reads bit 30 and not
+ /// the type byte.
+ #[test]
+ fn the_flip_reads_bit_thirty_not_the_type_byte() {
+ const GROWL: u32 = 2649;
+ let mut spells = PetSpells {
+ spells: vec![word(PET_ACT_ENABLED, GROWL)],
+ ..Default::default()
+ };
+ assert!(
+ spells.spells[0].autocast_on(),
+ "ACT_ENABLED means bit 30 is set"
+ );
+ assert_eq!(flip_autocast(&mut spells, GROWL), Some(false));
+ assert!(!spells.spells[0].autocast_on());
+ assert!(
+ spells.spells[0].autocast_allowed(),
+ "…and it stays ALLOWED — only bit 30 moves"
+ );
+ }
+}
diff --git a/crates/benilla-app/src/ui_script/chat_tests.rs b/crates/benilla-app/src/ui_script/chat_tests.rs
index 97b34591..db99f804 100644
--- a/crates/benilla-app/src/ui_script/chat_tests.rs
+++ b/crates/benilla-app/src/ui_script/chat_tests.rs
@@ -266,6 +266,7 @@ fn chat_click_dismisses_a_stuck_spell_but_not_an_item() {
passive: false,
current: false,
cooldown: None,
+ ..Default::default()
}],
});
s.run("ToggleSpellBook(BOOKTYPE_SPELL)").unwrap();
diff --git a/crates/benilla-app/src/ui_script/keybindings_tests.rs b/crates/benilla-app/src/ui_script/keybindings_tests.rs
index 0ac7b56f..0864c0d2 100644
--- a/crates/benilla-app/src/ui_script/keybindings_tests.rs
+++ b/crates/benilla-app/src/ui_script/keybindings_tests.rs
@@ -486,6 +486,23 @@ fn the_wheel_bubbles_from_the_rows_and_the_bar_rides_the_gutter() {
.eval::(&format!("return {SF}ScrollBar:IsVisible()"))
.unwrap());
s.resolve();
+ // The page area's OWN scroll (B217) stays out of it: this page fills the body exactly, so the
+ // outer range is 0 and its bar never joins this one in the gutter the two share.
+ for _ in 0..4 {
+ s.tick(0.016);
+ s.resolve();
+ }
+ assert_eq!(
+ s.eval::("return OptionsFrameContainerScroll:GetVerticalScrollRange()")
+ .unwrap(),
+ 0.0,
+ "the page scroll has nothing to scroll here — the list's own bar owns the gutter"
+ );
+ assert!(
+ !s.eval::("return OptionsFrameContainerScrollBar:IsVisible()")
+ .unwrap(),
+ "…so only one bar is on screen"
+ );
let body_right = s
.eval::("return OptionsFrameContainerBodyKeybindings:GetRight()")
.unwrap();
@@ -504,6 +521,49 @@ fn the_wheel_bubbles_from_the_rows_and_the_bar_rides_the_gutter() {
bar_right <= body_right - 8.0,
"bar inset from the body edge ({bar_right} vs body {body_right})"
);
+ // …wearing its TROUGH: the recessed channel (the shared kit's, shown and hidden with the bar),
+ // which seats ITSELF on the bar — 31 wide against the bar's 16, hung 8 units left, and
+ // overhanging it 21/20 so each arrow drops into the 16-tall socket the art carries (B224; the
+ // law and its ref citation are in BenillaScrollTrough_Seat). Asserted off the ARROWS here,
+ // which is the half the eye judges: 5 units of cap above the up arrow, 4 below the down arrow.
+ const TROUGH: &str = "OptionsFrameContainerBodyKeybindingsScrollFrameScrollBarTrough";
+ let bar = format!("{SF}ScrollBar");
+ let g = |f: &str, m: &str| s.eval::(&format!("return {f}:{m}()")).unwrap();
+ assert!(
+ s.eval::(&format!("return {TROUGH}:IsVisible()"))
+ .unwrap(),
+ "the trough shows with the bar"
+ );
+ assert!((g(TROUGH, "GetWidth") - 31.0).abs() < 0.01);
+ assert!((bar_left - g(TROUGH, "GetLeft") - 8.0).abs() < 0.01);
+ assert!((g(TROUGH, "GetTop") - g(&bar, "GetTop") - 21.0).abs() < 0.01);
+ assert!((g(&bar, "GetBottom") - g(TROUGH, "GetBottom") - 20.0).abs() < 0.01);
+ assert!(
+ (g(TROUGH, "GetTop") - g(&format!("{bar}ScrollUpButton"), "GetTop") - 5.0).abs() < 0.01
+ );
+ assert!(
+ (g(&format!("{bar}ScrollDownButton"), "GetBottom") - g(TROUGH, "GetBottom") - 4.0).abs()
+ < 0.01
+ );
+ // A list that FITS shows neither.
+ let set_all = |s: &UiScript, open: bool| {
+ s.run(&format!(
+ "for i = 1, table.getn(KeyBindingsPage.sections) do KeyBindings_ExpandSection(i, {}) end
+ KeyBindingsPage_Update()",
+ if open { "true" } else { "nil" }
+ ))
+ .unwrap();
+ };
+ set_all(&s, false);
+ for f in [format!("{SF}ScrollBar"), TROUGH.to_string()] {
+ assert!(
+ !s.eval::(&format!("return {f}:IsVisible()")).unwrap(),
+ "{f} goes away when the list fits"
+ );
+ }
+ set_all(&s, true);
+ s.resolve();
+
// A spin over a row NAME — the dead zone when the handler lived on the sibling.
let quads = s.extract();
let (wx, wy) = quads
@@ -563,3 +623,68 @@ fn the_wheel_bubbles_from_the_rows_and_the_bar_rides_the_gutter() {
s.run("KeyBindings_SetSelected(nil)").unwrap();
assert!(s.errors().is_empty(), "{:?}", s.errors());
}
+
+/// **The pet lane reaches the page** (B218 / decision 1052). 1008 registered the two multibars
+/// and stopped; 1.12's `BONUSACTIONBUTTON1-10` — "Secondary Action Button" in its own window —
+/// never joined, so the pet bar was the one visible bar with no way to bind it.
+///
+/// Pinned on both sides of the seam, because the report is about the *page*: the registry carries
+/// the ten rows under the ACTION BAR header with their byte-real `CTRL-1..CTRL-0` defaults (1.12
+/// files them under l.121's header, having none of their own), and the page's own search — the
+/// path a player actually finds a binding by — paints the row live with that key on its capsule.
+#[test]
+fn the_pet_lane_is_registered_under_the_action_bar_header() {
+ let mut s = harness();
+ on_page(&mut s);
+
+ // Registry side: the whole lane, in Bindings.xml order right after the shapeshift block,
+ // sharing the action bar's category token.
+ let category = |s: &mut UiScript, name: &str| {
+ s.eval::(&format!(
+ r#"for i = 1, GetNumBindings() do
+ local n, c = GetBinding(i)
+ if n == "{name}" then return c end
+ end"#
+ ))
+ .unwrap()
+ };
+ for i in 1..=10 {
+ let name = format!("BONUSACTIONBUTTON{i}");
+ assert_eq!(
+ category(&mut s, &name),
+ "BINDING_HEADER_ACTIONBAR",
+ "{name} files under the action bar, as 1.12 does"
+ );
+ let key = s
+ .eval::(&format!(r#"return GetBindingKey("{name}")"#))
+ .unwrap();
+ assert_eq!(key, format!("CTRL-{}", i % 10), "{name}'s 1.12 default");
+ }
+ // …and it is a runOnUp pair like the action bar's own rows, which is what makes the wheel
+ // refusal apply to it (SetBinding answers nil rather than binding MOUSEWHEELUP).
+ assert!(
+ s.eval::>(r#"return SetBinding("MOUSEWHEELUP", "BONUSACTIONBUTTON1")"#)
+ .unwrap()
+ .is_none(),
+ "press+release commands refuse the wheel"
+ );
+
+ // Page side: search finds it and paints a live row with the capsule filled.
+ s.run(r#"OptionsFrameSearchBox:SetText("bonusactionbutton1")"#)
+ .unwrap();
+ assert!(s
+ .eval::("return OptionsFrameContainerBodySearchHeadKeybindings:IsVisible()")
+ .unwrap());
+ assert_eq!(
+ s.eval::("return OptionsFrameContainerBodyKeybindSearch1Description:GetText()")
+ .unwrap(),
+ "BONUSACTIONBUTTON1",
+ "the raw-token fallback — the app's GlobalStrings read 'Secondary Action Button 1'"
+ );
+ assert_eq!(
+ s.eval::("return OptionsFrameContainerBodyKeybindSearch1Key1ButtonText:GetText()")
+ .unwrap(),
+ "CTRL-1"
+ );
+ assert!(s.errors().is_empty(), "script errors: {:?}", s.errors());
+}
diff --git a/crates/benilla-app/src/ui_script/manifest.rs b/crates/benilla-app/src/ui_script/manifest.rs
new file mode 100644
index 00000000..9a2e15a2
--- /dev/null
+++ b/crates/benilla-app/src/ui_script/manifest.rs
@@ -0,0 +1,369 @@
+//! The shipped FrameXML manifest and the three ways it is loaded.
+//!
+//! Split out of `ui_script/mod.rs` as its own concern: a 250-entry ordered data table plus the
+//! loader that walks it. The interesting thing here is the **seam at index 0** — see
+//! [`load_default_ui`].
+
+use bevy::prelude::*;
+
+use benilla_ui::script::UiScript;
+
+/// The shipped FrameXML manifest, in load order. Index 0 (`Fonts.xml`) is the **font-object
+/// registry** and is deliberately separable from the rest: it materializes zero frames, and the
+/// glyph atlas bake ([`crate::ui_text`]) reads its objects to build the bake plan. Splitting the
+/// manifest here is what lets the in-game UI load at world entry while the glue screens — which
+/// share our one atlas — still get every (face, size, outline) combination they need (1051).
+const UI_MANIFEST: &[&str] = &[
+ // The named virtual Font objects (decision 0084) — must load first so any FontString's
+ // `inherits="GameFontNormal"` resolves against a registered object.
+ "Fonts.xml",
+ // The UIPanel slot manager (decision 0084 §2): script-only, no frames of its own — must load
+ // before any panel frame (Gossip/Merchant today) so ShowUIPanel/HideUIPanel already exist
+ // when those frames' OnLoad/OnEvent reference them.
+ "UiPanels.xml",
+ // The managed bottom-stack positions (decision 0272): script-only —
+ // UIPARENT_MANAGED_FRAME_POSITIONS + UIParent_ManageFramePositions, the ref
+ // UIParent.lua mechanism that re-anchors the cast bar / chat over the bottom bars.
+ // Loaded early so the stance bar's OnShow/OnHide can call it; applied once by the
+ // post-load bootstrap below.
+ "UIParent.xml",
+ // The tooltip: a hover window the bag/merchant/loot/unit frames drive. Loaded before them so
+ // `GameTooltip` + its verbs exist when their OnEnter handlers fire (runtime, so order
+ // is not strictly required — but it keeps the dependency legible).
+ "GameTooltip.xml",
+ // The dropdown kit (0203 phase 2's closing sub-slice, widened for 0434 phase 5):
+ // UIDropDownMenuTemplate + DropDownList1/2 + the UIDropDownMenu_* API — before its
+ // first customers (the unit popups below, the world map's pickers, the trainer filter).
+ "UIDropDownMenu.xml",
+ // The unit right-click popups (0434 phase 5): UnitPopupMenus/Buttons + UnitPopup_ShowMenu
+ // — after the dropdown kit it drives, before the unit/party frames whose DropDown
+ // children's OnLoad initialize into it.
+ "UnitPopup.xml",
+ // The chat-link click router (SetItemRef): item links → ItemRefTooltip:SetHyperlink; a
+ // player name → left-click whisper / right-click FRIEND dropdown. After GameTooltip.xml
+ // (GameTooltip_OnLoad) AND the dropdown kit + UnitPopup — its FriendsDropDown inherits
+ // UIDropDownMenuTemplate and opens a UnitPopup FRIEND menu.
+ "ItemRef.xml",
+ "UnitFrames.xml",
+ // The combo-point dots (decision 0869) — anchored to BenillaTargetFrame, so after
+ // UnitFrames.xml; its fade chain needs UiPanels.xml's UIFrameFade above.
+ "ComboFrame.xml",
+ // The center-screen scrolling combat text (decision 0578) — the Blizzard_CombatText
+ // transcription; consumes COMBAT_TEXT_UPDATE + the regen/health/power events, needs
+ // only Fonts.xml (CombatTextFont) + UIParent before it.
+ "CombatText.xml",
+ // The party member frames + the PARTY_INVITE popup + its event driver (decision 0434
+ // phase 2): a StaticPopup registry entry, so after UiPanels.xml.
+ "PartyFrame.xml",
+ // The death arc's dialogs + event driver (decision 0308): registry entries on the shared
+ // StaticPopup engine, so after UiPanels.xml.
+ "DeathFrame.xml",
+ // The two duel dialogs + their event driver (decision 0633): registry entries on the same
+ // StaticPopup engine, and DUEL_OUTOFBOUNDS leans on its per-tick countdown branch.
+ "DuelFrame.xml",
+ // The two enchant-apply confirms + their event driver (decision 0928): the same StaticPopup
+ // registry again. Not CraftFrame's — the gate that raises them is the generic item-target
+ // bind, reached by a poison or a sharpening stone with no profession window open.
+ "EnchantConfirm.xml",
+ // The shared CooldownFrame_SetTimer (the ref's Cooldown.lua file split) — before its
+ // consumers (ActionBar's buttons, the multibars, the stance bar, BagFrame's slots).
+ "Cooldown.xml",
+ "ActionBar.xml",
+ // The two always-on bottom multibars (actions 61-72 / 49-60): pure XML/Lua over
+ // ActionBar.xml's shared button template + handler set — loads right after it
+ // (inherits BenillaActionButtonTemplate, anchors to BenillaActionButton1, and its
+ // buttons join the BENILLA_ACTION_BUTTONS roster ActionBar.xml declares).
+ "MultiBars.xml",
+ // The stance/shapeshift bar: hidden until `crate::ui_shapeshift`'s feed pushes a
+ // non-empty form list (a formless class never shows it). After Cooldown.xml
+ // (CooldownFrame_SetTimer) + ActionBar.xml (BENILLA_FALLBACK_ICON, the BenillaActionBar
+ // anchor target).
+ "StanceBar.xml",
+ // The pet action bar (decision 0982): hidden until `crate::ui_pet`'s feed reports a bar,
+ // i.e. until the server sends `SMSG_PET_SPELLS` for a live pet or charm — so a class that
+ // never has one never sees it. Right after the stance bar, whose row it shares (either
+ // one showing raises UIParent.xml's managed "pet" delta), and after Cooldown.xml
+ // (CooldownFrame_SetTimer), ActionBar.xml (the `BenillaActionBar` anchor target) and
+ // GameTooltip.xml (its hover).
+ "PetActionBar.xml",
+ // The micro-button row in the bar's right-hand recess (the ref's own file split, its TOC
+ // loads MainMenuBarMicroButtons right after MainMenuBar). After ActionBar.xml: it anchors
+ // into BenillaActionBarArtFrame by name and seats itself with that file's
+ // BenillaActionBarArt_SeatAbove. It must come BEFORE the panels it toggles, because each of
+ // their OnShow/OnHide calls the `UpdateMicroButtons` this file defines (the panel frames
+ // themselves are read back through getglobal, so the reverse dependency is call-time).
+ "MicroMenu.xml",
+ // The HUD minimap cluster (decision 0203 phase 1): the chrome around the engine-rendered
+ // widget hole (`crate::minimap` fills it). After GameTooltip (its zone-text
+ // hover uses the shared tooltip).
+ "MinimapCluster.xml",
+ // The zone-entry splash (decision 0287): ZoneTextFrame/SubZoneTextFrame + the inlined
+ // FadingFrame kit, driven by the ZONE_CHANGED family `crate::area` fires. Needs only
+ // Fonts (ZoneTextFont/SubZoneTextFont) + UIParent; kept by the minimap cluster — the
+ // other consumer of the zone-text host globals.
+ "ZoneText.xml",
+ // The faux-scroll kit (decisions 0247/0250/0251): BenillaScrollBarTemplate (the draggable
+ // Slider scroll bar) + BenillaFauxScrollFrameTemplate + the BenillaFauxScrollFrame_* API —
+ // before its first customer (the trainer list) below.
+ "ScrollTemplates.xml",
+ // The fullscreen world map (decision 0203 phase 2), over the worldmap host API
+ // (`crate::ui_world_map` feeds it); the 'M' binding below calls its ToggleWorldMap().
+ "WorldMapFrame.xml",
+ // The cast bar (decision 0137 phase 1) — the extracted 1.12 CastingBarFrame, driven by
+ // the SPELLCAST_* events `crate::ui_cast` fires. Only needs Fonts before it.
+ "CastingBar.xml",
+ // The mirror timers (decision 0874): the breath/fatigue bars at top-center, driven by the
+ // MIRROR_TIMER_* events `crate::ui_mirror` fires. Needs only Fonts (GameFontHighlight)
+ // before it; kept beside the cast bar, whose border art it shares.
+ "MirrorTimer.xml",
+ // The player buff/debuff bar (decisions 0255/0257 — the player aura arc): the HUD row of
+ // aura icons under the minimap, driven by the UnitAura feed (`crate::ui_aura`). Needs Fonts
+ // (its FontStrings) and ActionBar's `BENILLA_FALLBACK_ICON` global (both already loaded).
+ "BuffFrame.xml",
+ // The durability alert (the "armor guy"): under the minimap beside BuffFrame, driven by
+ // UPDATE_INVENTORY_ALERTS off the same inventory push the character window reads. Needs
+ // MinimapCluster (its anchor) — loaded above.
+ "DurabilityFrame.xml",
+ "ErrorsFrame.xml",
+ "BagFrame.xml",
+ // The stack-split spinner (decision 0216 §6/slice 2): loaded beside BagFrame, whose slot
+ // click handler (`BenillaBagSlot_OnClick`) calls `OpenStackSplitFrame`. Order between the
+ // two doesn't matter for correctness (Lua globals resolve at call time, and neither file's
+ // XML `inherits=` reaches into the other's templates) — kept adjacent for legibility.
+ "StackSplit.xml",
+ // The character window (decision 0208 phase 1a): the C-key paper doll. Loaded after BagFrame
+ // (needs nothing from it, but keeps player-state windows grouped) and before the NPC session
+ // windows (Gossip/Merchant/Quest*) — it depends only on Fonts/UiPanels/GameTooltip, all
+ // already loaded above.
+ "CharacterFrame.xml",
+ // The Skills tab (decision 0437 phase 4): a CharacterFrame child page, but a separate
+ // top-level file (SkillFrame.xml's own header comment — this engine has no cross-file
+ // `parent=` attachment, so it's positioned by name via `` instead). Loaded immediately after CharacterFrame.xml so it
+ // paints ON TOP of it (later file = higher z) and so `BenillaCharacterFrame` already exists
+ // for that anchor to resolve against.
+ "SkillFrame.xml",
+ // The inspect window (decision 0631): another player's paper doll. Beside the character
+ // window it mirrors — it needs the same three (Fonts/UiPanels/GameTooltip) plus
+ // `CharacterFrameTabButtonTemplate` from UiPanels.xml, and nothing from CharacterFrame.xml
+ // itself (its slot template, handlers, and booth slot are all its own — the two windows
+ // share only the reference's art).
+ "InspectFrame.xml",
+ // The spellbook window (decision 0216 §8, slice 5): the P-key window, the spell SOURCE
+ // for the cursor-payload arc. Loaded right after CharacterFrame (the same "player-state
+ // windows grouped" posture, same Fonts/UiPanels/GameTooltip-only dependency set); the
+ // 'P' binding below calls its bare ToggleSpellBook(BOOKTYPE_SPELL).
+ "SpellBookFrame.xml",
+ // The talent window (decision 0304 §4): the N-key window, the class talent grid over the
+ // `benilla-ui::script::talent` engine seam. Loaded right after SpellBookFrame (the same
+ // "player-state windows grouped" posture; needs Fonts/UiPanels/GameTooltip plus
+ // ScrollTemplates.xml — its real ScrollFrame's scrollbar reuses BenillaScrollBarTemplate —
+ // all already loaded above). The 'N' binding (ui_script/input.rs) calls its bare
+ // ToggleTalentFrame().
+ "TalentFrame.xml",
+ // The social window (decision 0668): the O-key friends/ignore/who window over the social
+ // API (`benilla-ui/src/script/social.rs`) + `crate::ui_social`'s feed/drain. Needs
+ // UiPanels (the panel manager, `CharacterFrameTabButtonTemplate`, and the StaticPopup
+ // engine its two name-entry dialogs register into), ScrollTemplates (three faux lists),
+ // GameTooltip (the newbie tips) and UIDropDownMenu (the who list's variable column) —
+ // all loaded above.
+ "FriendsFrame.xml",
+ "GossipFrame.xml",
+ "MerchantFrame.xml",
+ // The mail window (decision 0544 P1/P2): the mailbox inbox, open-letter, and send tab over
+ // the Era mail API (`benilla-ui/src/script/mail.rs`) + `crate::ui_mail`'s feed/drain. Loaded
+ // right after MerchantFrame because it REUSES that file's global BenillaMoney_Set/_Clear
+ // coin helpers (postage/enclosed/COD displays) — they must be defined first.
+ "MailFrame.xml",
+ // The player-to-player trade window (decision 0592 P1): the two-sided offer over the trade
+ // API (`benilla-ui/src/script/trade.rs`) + `crate::ui_trade`'s feed/drain. Loaded after
+ // MerchantFrame because it REUSES that file's global BenillaMoney_Set coin helpers (the two
+ // gold displays) — they must be defined first.
+ "TradeFrame.xml",
+ // The item-text reader: letters (mail-made permanent copies) and, later, books/plaques —
+ // the ItemTextGet* seam (`benilla-ui/src/script/item_text.rs`) over `crate::ui_item_text`'s
+ // feed/drain. Loaded right after MailFrame (the same arc; needs only Fonts/UiPanels, both
+ // loaded above).
+ "ItemTextFrame.xml",
+ // The trainer window (decision 0237 phase 3): the read-only class/profession trainer list.
+ // Loaded right after MerchantFrame per the director's brief (kept adjacent even though this
+ // v1 uses plain-text costs, not MerchantFrame's coin helpers) and before the other NPC
+ // session windows below.
+ "TrainerFrame.xml",
+ // The bank window (decision 0604 phase 4): the vault, purchase ladder, and the 6 bank-bag
+ // popouts, over the C_Container feed already carrying container -1/5..10 (ui_items.rs) +
+ // the bank Lua surface (benilla-ui/src/script/bank.rs) + crate::ui_bank's feed/drain.
+ // Loaded AFTER MerchantFrame.xml (reuses its BenillaMoney_* coin helpers for the purchase
+ // row + purse) and AFTER BagFrame.xml, already above (its BenillaBagSlotTemplate/
+ // BenillaBagWindowTemplate + the BenillaBagSlot_*/BenillaBagFrame_* function family are
+ // reused verbatim — the 24 generic slots and the 6 bank-bag popouts are, respectively, a
+ // container-slot grid and ordinary container windows, exactly like the bag arc's own).
+ // Kept in the "NPC session windows grouped" cluster beside Trainer/Taxi.
+ "BankFrame.xml",
+ // The taxi-map window (decision 0484 phase 2): the flight-master session, driven by the
+ // taxi engine seam (`benilla-ui/src/script/taxi.rs`) over `crate::ui_taxi`'s feed/drain.
+ // Loaded right after TrainerFrame per the same "NPC session windows grouped" posture — it
+ // needs only Fonts/UiPanels/GameTooltip (SetTooltipMoney), both already loaded above.
+ "TaxiFrame.xml",
+ // The crafting window (decision 0437 phase 2): NOT an NPC session (opens off your own cast of
+ // an effect-47 opener spell, TradeSkillFrame.xml's own header comment) — loaded right after
+ // TrainerFrame per the same "player-state/profession window" grouping and because it depends
+ // on nothing TrainerFrame doesn't already need (Fonts/UiPanels/GameTooltip/ScrollTemplates,
+ // all loaded above).
+ "TradeSkillFrame.xml",
+ // The Enchanting window (decision 0437 phase 3): the exact twin of TradeSkillFrame above —
+ // same "opens off your own cast, not an NPC session" shape (CraftFrame.xml's own header
+ // comment) — loaded right after it so `BuildColoredListString` (a guarded GLOBAL
+ // TradeSkillFrame.xml defines) already exists when CraftFrame.xml reuses it without
+ // redefining (CraftFrame.xml's own header comment, deviation 2).
+ "CraftFrame.xml",
+ "LootFrame.xml",
+ // The group-loot roll popups (decision 0591): the Need/Greed/Pass dialogs, a sibling of
+ // LootFrame.xml (loaded right after it) over the `benilla-ui::script::loot_roll` seam
+ // (`crate::ui_loot_roll`'s feed/drain). NOT a UIPanel — four `frameStrata="DIALOG"` toplevel
+ // popups with their own hardcoded anchors — so it needs only Fonts (GameFontNormalSmall) +
+ // GameTooltip (the item/PASS/NEED/GREED hovers), both already loaded above.
+ "GroupLootFrame.xml",
+ // The questgiver window (decision 0088): four sub-panels over the Era quest API. Loaded after
+ // MerchantFrame (the BenillaMoney coin helpers live there) and GossipFrame (shared parchment
+ // art conventions); its title/panel-button labels use the auto-centred ButtonText slot.
+ "QuestFrame.xml",
+ // The quest log window (decision 0088 arc, the quest-log slice): durable player state, not an
+ // NPC session — loaded after MerchantFrame (the BenillaMoney coin helpers it reuses) and
+ // alongside QuestFrame (same arc); the 'L' binding below calls its ToggleQuestLog().
+ "QuestLogFrame.xml",
+ "ChatFrame.xml",
+ // The macro window (decision 0983): the editor + its name/icon popup, driven over the
+ // engine's OWN macro table (`benilla_ui::script::macros` — 1.12 macros have no server
+ // side, so this is the one window whose model is not an app feed). Needs UiPanels (the
+ // panel manager + `TabButtonTemplate`), ScrollTemplates (BOTH kits — the real one scrolls
+ // the body box, the faux one the icon grid), MicroMenu (`UpdateMicroButtons` on
+ // show/hide), and it must come AFTER SpellBookFrame, whose shift-click reaches
+ // `BenillaMacroFrame_AddMacroLine`. Before GameMenuFrame, whose Macros button opens it.
+ "MacroFrame.xml",
+ // The Keybindings page MODULE (decision 1008, superseding 0997's standalone window):
+ // the templates + script of the Options window's Keybindings category, over the
+ // engine's binding table (benilla_ui::script::keybind + crate::bindings — the 1.12
+ // GetBinding/SetBinding Lua API). Needs UiPanels (popup engine) and GameTooltip (the
+ // character-specific checkbox's hover); MUST load before OptionsFrame.xml, whose
+ // Keybindings body inherits these templates and calls this script at OnLoad.
+ "KeyBindingsPage.xml",
+ // The options window (the 0985 provenance split: 0950's era structure, 0978's
+ // 1.12-native art, 0981/0984's 1.14 chrome + working era search, 0989's directed
+ // cuts — bare fully-live sliders, no corner X; 0992's dropdown rows + the Nameplates
+ // page): the era-shaped OptionsFrame the game menu's Options button opens. Needs
+ // Fonts' objects, UiPanels' panel manager, and UIDropDownMenu's capsule template
+ // (all far above); sits right before GameMenuFrame so the menu — which reaches it by
+ // name at click time — stays the last, outermost layer.
+ "OptionsFrame.xml",
+ // The game menu (decision 0674): the frame ESC opens, plus the CAMP/QUIT dialogs and their
+ // event driver. LAST on purpose — it is the outermost layer of the shell, it depends on
+ // everything it touches being loaded (UiPanels' popup engine + panel manager, MicroMenu's
+ // UpdateMicroButtons, BagFrame's Disable_BagButtons), and nothing depends on it: the ESC
+ // chain and the micro button both reach it by name at call time.
+ "GameMenuFrame.xml",
+];
+
+/// Load a slice of [`UI_MANIFEST`] into the VM, in the order given. Returns per-file errors.
+///
+/// `bootstrap_positions` runs decision 0272's load-time `UIParent_ManageFramePositions()` pass —
+/// only meaningful once the frames that table names exist, so the font-registry-only load
+/// ([`load_font_registry`]) skips it. It is defined in `UIParent.xml`, which is in the deferred
+/// half; calling it after `Fonts.xml` alone is a nil-global error, not a no-op.
+fn load_ui_files(script: &UiScript, files: &[&str], bootstrap_positions: bool) -> Vec {
+ let mut failures = Vec::new();
+ let dir = std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("assets/ui");
+ // Provider for FrameXML/Lua references: try the path as given and by basename (Blizzard-style
+ // backslash paths, dir-relative), resolved against our own assets/ui dir.
+ let provider = |req: &str| -> Option {
+ let norm = req.replace('\\', "/");
+ let base = norm.rsplit('/').next().unwrap_or(&norm);
+ std::fs::read_to_string(dir.join(&norm))
+ .or_else(|_| std::fs::read_to_string(dir.join(base)))
+ .ok()
+ };
+ for file in files {
+ let entry = dir.join(file);
+ let text = match std::fs::read_to_string(&entry) {
+ Ok(t) => t,
+ Err(e) => {
+ error!("ui_script: reading {}: {e}", entry.display());
+ continue;
+ }
+ };
+ let doc = match benilla_ui::framexml::parse(&text) {
+ Ok(d) => d,
+ Err(e) => {
+ error!("ui_script: parsing {file}: {e}");
+ continue;
+ }
+ };
+ let report = benilla_ui::loader::load(script, &doc, &provider);
+ for w in &report.warnings {
+ warn!("ui_script({file}): {w}");
+ }
+ for e in &report.errors {
+ error!("ui_script({file}): {e}");
+ failures.push(format!("{file}: {e}"));
+ }
+ info!(
+ "ui_script: {file} loaded ({} frames materialized)",
+ report.frames
+ );
+ }
+
+ // The managed positions' startup pass (decision 0272): the ref applies
+ // UIPARENT_MANAGED_FRAME_POSITIONS once at load, then re-fires from the bottom bars'
+ // OnShow/OnHide. Every frame the table names now exists, so this is that load-time
+ // application; the stance bar's show/hide handles the rest at runtime.
+ if bootstrap_positions {
+ if let Err(e) = script.run("UIParent_ManageFramePositions()") {
+ error!("ui_script: managed-positions bootstrap: {e}");
+ failures.push(format!("managed-positions bootstrap: {e}"));
+ }
+ }
+ failures
+}
+
+/// Load benilla's own default UI (`assets/ui/*.xml` — the unit frames and the action bar) through
+/// the engine-free loader, resolving any ``/`