From c9fe3d7a4823d60e692fb66cc700d0ed4c04744f Mon Sep 17 00:00:00 2001 From: Sam Date: Fri, 21 Aug 2026 18:10:06 +0700 Subject: [PATCH] the camera follows again, /errors opens the log, cmangos lets you in MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - the camera auto-follow returns, as a setting: Camera Following Style on the Controls options page — Smart (the default), Always, Never — so after an orbit drag the camera swings back behind you while you run; far sight still forces the chase off - broken addons announce themselves: world entry prints "3 addon load failures — type /errors to see them", and the `/errors` window lists every script error this session with its traceback — 256 deduplicated rows, repeats collapsing onto a count — while the five load-failure classes that only ever hit the terminal (missing .toc, unparseable FrameXML, dependency cycles…) finally get a channel - B opens the backpack alone, SHIFT-B opens all bags, F8–F11 toggle the individual slots — the reference's six bag commands instead of one command wearing every chord; and the Skills page gets its CLOSE button, with the ALL fold in the row font - cmangos-classic logons work: `CMSG_AUTH_SESSION` now carries the real client's twelve secure-addon records (342 bytes, compressing to 130) — an empty addon block is a shape no client can send, and cmangos kicks for it where vmangos shrugs; Warden-enabled anticheat remains a separate wall Co-Authored-By: Claude Fable 5 --- crates/benilla-app/assets/ui/BagFrame.xml | 148 +++++-- crates/benilla-app/assets/ui/OptionsFrame.xml | 33 +- .../benilla-app/assets/ui/ScriptLogFrame.xml | 366 ++++++++++++++++++ crates/benilla-app/assets/ui/SkillFrame.xml | 106 ++++- crates/benilla-app/assets/ui/UiPanels.xml | 10 +- crates/benilla-app/assets/ui/benilla.toc | 8 + crates/benilla-app/src/bindings.rs | 98 +++++ crates/benilla-app/src/bindings/commands.rs | 208 ++++++---- crates/benilla-app/src/cvars.rs | 48 ++- crates/benilla-app/src/player.rs | 45 ++- crates/benilla-app/src/player/camera.rs | 147 ++++++- crates/benilla-app/src/ui_chat/commands.rs | 20 +- crates/benilla-app/src/ui_chat/input/parse.rs | 12 + crates/benilla-app/src/ui_chat/tests.rs | 7 +- crates/benilla-app/src/ui_script/addons.rs | 13 + crates/benilla-app/src/ui_script/bag_tests.rs | 200 ++++++++-- crates/benilla-app/src/ui_script/lifecycle.rs | 29 ++ .../src/ui_script/options_tests.rs | 135 ++++++- .../src/ui_script/skills_frame_tests.rs | 166 ++++++++ .../src/ui_script/world_entry_tests.rs | 178 +++++++++ .../benilla-protocol/src/messages/addons.rs | 210 ++++++++++ .../benilla-protocol/src/messages/client.rs | 17 +- crates/benilla-protocol/src/messages/mod.rs | 2 + crates/benilla-protocol/src/world/session.rs | 4 + crates/benilla-protocol/tests/client.rs | 26 +- crates/benilla-ui/src/script/diagnostics.rs | 339 ++++++++++++++++ crates/benilla-ui/src/script/mod.rs | 22 +- crates/benilla-ui/src/script/model.rs | 12 + 28 files changed, 2382 insertions(+), 227 deletions(-) create mode 100644 crates/benilla-app/assets/ui/ScriptLogFrame.xml create mode 100644 crates/benilla-protocol/src/messages/addons.rs create mode 100644 crates/benilla-ui/src/script/diagnostics.rs diff --git a/crates/benilla-app/assets/ui/BagFrame.xml b/crates/benilla-app/assets/ui/BagFrame.xml index 967a9c38..fdd4c8d1 100644 --- a/crates/benilla-app/assets/ui/BagFrame.xml +++ b/crates/benilla-app/assets/ui/BagFrame.xml @@ -497,13 +497,24 @@ BenillaBagWindow_Repaint(frame) end - -- OpenAllBags / CloseAllBags / ToggleBackpack (ref ContainerFrame.lua l.152-217): the 'B' - -- binding (OPENALLBAGS) and the backpack button BOTH open or close EVERY bag at once, not - -- just the backpack — the real client's own behavior (MainMenuBarBackpackButton's OnClick and - -- the OPENALLBAGS binding both route to ToggleBackpack). "All bags" = the backpack (bag 0, - -- which always exists) plus each equipped-bag slot 1..4 that actually holds a bag - -- (GetContainerNumSlots > 0); an empty slot has no window to show. The five windows, indexed - -- by bag id (BENILLA_BAG_WINDOWS[bagId+1]) — one code path over all of them. + -- ToggleBag / ToggleBackpack / OpenAllBags (ref ContainerFrame.lua l.46-82, l.662-700) — + -- TWO DISTINCT KNOBS, and benilla ran them as one until 1494. The reference's split, which + -- is now ours: + -- + -- B / F12 TOGGLEBACKPACK -> ToggleBackpack() — the BACKPACK's toggle. Closed: opens + -- bag 0 ALONE. Already open: hides every container window there is. The + -- asymmetry is the reference's own — one bag out, all bags in. + -- SHIFT-B OPENALLBAGS -> OpenAllBags() — the ALL-bags toggle: everything open + -- already closes the lot, anything else opens the lot. + -- F8..F11 TOGGLEBAG1..4 -> ToggleBag(4..1) — one equipped bag each (note the + -- reference's REVERSED numbering: TOGGLEBAG1 is bag 4). + -- + -- "All bags" = the backpack (bag 0, which always exists) plus each equipped-bag slot 1..4 + -- that actually holds a bag (GetContainerNumSlots > 0); an empty slot has no window to + -- show. The five windows, indexed by bag id (BENILLA_BAG_WINDOWS[bagId+1]) — one code path + -- over all of them. The keyring is a seventh window with its own ToggleKeyRing: swept by + -- the two close arms below (the ref's sweeps are over ALL ContainerFrames, which is what + -- its keyring lives in), never counted as an open bag and never opened by them. BENILLA_BAG_WINDOWS = { "BenillaBagFrame", "BenillaBagFrame1", "BenillaBagFrame2", "BenillaBagFrame3", "BenillaBagFrame4" } -- THE REFERENCE'S NAMES ARE THE REAL ENTRY POINTS, and our own UI calls THEM. @@ -512,7 +523,7 @@ -- client to call its replacement. `Bagnon_Core/core/Overrides.lua` is the canonical shape — -- local bToggleBackpack = ToggleBackpack -- ToggleBackpack = function() ... end - -- and until now that override was INERT here, because the 'B' binding and the bag button + -- and until now that override was INERT here, because the bag bindings and the bag button -- called `BenillaBagToggle_OnClick` directly and nothing ever looked up `ToggleBackpack`. -- Bagnon loaded, appeared in the AddOns list, replaced the globals, and the player still -- got the stock bags. The director saw that; no instrument here could, because the corpus @@ -522,23 +533,50 @@ -- silently inert wherever our UI calls a benilla-named equivalent instead of the -- reference's name. `CloseAllBags`/`OpenBackpack`/`CloseBackpack` already lived under the -- ref's names in UiPanels.xml for exactly this reason; these are the three that did not. - function OpenAllBags() - for bagId = 0, 4 do - -- The backpack always exists; an equipped-bag slot only when a bag is in it. - if bagId == 0 or GetContainerNumSlots(bagId) > 0 then - local frame = getglobal(BENILLA_BAG_WINDOWS[bagId + 1]) - if frame and not frame:IsShown() then - BenillaBagWindow_Repaint(frame) -- bag 0 routes through its observable alias - frame:Show() - end + -- + -- `OpenAllBags(forceOpen)` is the ref's SHIFT-B body (ContainerFrame.lua l.662-700) and it + -- is a TOGGLE, not a plain open: one pass hides every open window while counting them + -- against how many CAN be open, and if everything already was, that pass IS the close and + -- it returns. `forceOpen` (the ref's own argument, used by callers that mean "open") skips + -- the close arm. The open arm calls the GLOBALS `ToggleBackpack()`/`ToggleBag(1..4)` on a + -- now-closed set — the ref's exact tail, so a bag addon holding either name gets both. + function OpenAllBags(forceOpen) + if UIParent and not UIParent:IsVisible() then return end + local totalBags = 1 -- the backpack always counts + for bagId = 1, 4 do + if GetContainerNumSlots(bagId) > 0 then totalBags = totalBags + 1 end + end + local bagsOpen = 0 + for _, name in ipairs(BENILLA_BAG_WINDOWS) do + local frame = getglobal(name) + if frame and frame:IsShown() then + frame:Hide() + bagsOpen = bagsOpen + 1 end end + -- Swept with the rest, but never counted (ref: `if GetID() ~= KEYRING_CONTAINER`). + local keyring = getglobal("BenillaKeyRingFrame") + if keyring and keyring:IsShown() then keyring:Hide() end + if bagsOpen >= totalBags and not forceOpen then return end + ToggleBackpack() + ToggleBag(1) + ToggleBag(2) + ToggleBag(3) + ToggleBag(4) end - -- `ToggleBag(id)` — the ref's per-bag toggle (ContainerFrame.lua l.152-170), and the other + -- `ToggleBag(id)` — the ref's per-bag toggle (ContainerFrame.lua l.46-65), and the other -- global Bagnon replaces. Ours drives the same window the all-bags path does. + -- + -- The ref's `if size > 0` gate is load-bearing here and was missing until 1494: it is what + -- keeps `OpenAllBags`' blind `ToggleBag(1..4)` tail from showing an empty slot's window. + -- Bag 0 is exempt for the same reason `OpenBag` exempts it (it always exists even when the + -- feed has not told us its size yet) — the ref gets that for free from its own always-16 + -- backpack. function ToggleBag(id) - local frame = getglobal(BENILLA_BAG_WINDOWS[(id or 0) + 1]) + id = id or 0 + if id ~= 0 and GetContainerNumSlots(id) <= 0 then return end + local frame = getglobal(BENILLA_BAG_WINDOWS[id + 1]) if not frame then return end if frame:IsShown() then frame:Hide() @@ -584,16 +622,28 @@ return nil end - -- `ToggleBackpack()` — the ref's all-bags toggle ("if numOpen > 0 then CloseAllBags() else - -- OpenAllBags()"). It calls the two GLOBALS by name rather than local bodies, so hooking - -- either of those works too. + -- `ToggleBackpack()` — ref ContainerFrame.lua l.67-82, and NOT the all-bags toggle it was + -- here until 1494: + -- + -- if ( IsBagOpen(0) ) then else ToggleBag(0) end + -- + -- Open arm: `ToggleBag(0)`, the backpack ALONE (through the global, so a bag addon holding + -- `ToggleBag` gets it). Close arm: the ref's own frame sweep — every window, keyring + -- included — and NOT a `CloseAllBags()` call, because the reference does not make one here + -- and an addon holding that name would fire on a path the real client never runs it on. + -- The condition reads bag 0 SPECIFICALLY: with only bag 2 open, B opens the backpack + -- beside it rather than closing anything. function ToggleBackpack() - local anyOpen = false - for _, name in ipairs(BENILLA_BAG_WINDOWS) do - local frame = getglobal(name) - if frame and frame:IsShown() then anyOpen = true end + if IsBagOpen(0) then + for _, name in ipairs(BENILLA_BAG_WINDOWS) do + local frame = getglobal(name) + if frame and frame:IsShown() then frame:Hide() end + end + local keyring = getglobal("BenillaKeyRingFrame") + if keyring and keyring:IsShown() then keyring:Hide() end + else + ToggleBag(0) end - if anyOpen then CloseAllBags() else OpenAllBags() end end -- The backpack button (MainMenuBarBackpackButton): if any bag window is open, close them @@ -643,7 +693,7 @@ -- (backpack included), INCLUDING the real fn's keyring fork: the keyring has its own -- jingling KeyRingOpen/KeyRingClose pair (both real SoundEntries kits), every other window -- the plain igBackPackOpen/igBackPackClose. Fired on every visibility transition (a Show/Hide - -- from a toggle button, the 'B' binding, a close button, or ESC), never at load (every window + -- from a toggle button, a bag binding, a close button, or ESC), never at load (every window -- is authored hidden="true", so none ever transitions on startup). -- -- The `this` legacy global (RF-0025, script/event.rs) names the window that just @@ -844,14 +894,15 @@ -- -- ref MainMenuBarBackpackButton OnEnter (ref-MainMenuBarBagButtons.xml l.91-99). The ref -- appends the live binding key with GameTooltip:AppendText; ours concatenates the same - -- suffix from a LIVE GetBindingKey read since 0997 (our one bag knob is OPENALLBAGS — - -- the toggle this very button runs), rendered through the Key Bindings window's display - -- law when it is loaded. Unbound: the bare label, the ref's own read. + -- suffix from a LIVE GetBindingKey read since 0997, rendered through the Key Bindings + -- window's display law when it is loaded. Unbound: the bare label, the ref's own read. + -- The command is TOGGLEBACKPACK — the ref's own read here, and since 1494 ours too: it is + -- the key that runs what a plain click on this button runs. function BenillaBagToggle_OnEnter(button) if not GameTooltip then return end -- engine tests load file subsets GameTooltip:SetOwner(button, "ANCHOR_LEFT") local text = BACKPACK_TOOLTIP - local key = GetBindingKey("OPENALLBAGS") + local key = GetBindingKey("TOGGLEBACKPACK") if key then if KeyBindings_KeyText then key = KeyBindings_KeyText(key) end text = text .. " " .. NORMAL_FONT_COLOR_CODE .. "(" .. key .. ")" .. FONT_COLOR_CODE_CLOSE @@ -860,9 +911,14 @@ end -- ref BagSlotButton_OnEnter (ref-MainMenuBarBagButtons.lua l.86-96): the equipped bag's own - -- item tooltip, or "Equip Container" for an empty slot. The ref's "(F1)".."(F4)" suffix - -- needs GetBindingKey AND the TOGGLEBAG1..4 bindings — benilla ships neither, so no slot - -- carries one; the day those bind, this is where the suffix goes. + -- item tooltip, or "Equip Container" for an empty slot. The ref's "(F8)".."(F11)" suffix + -- needed GetBindingKey AND the TOGGLEBAG1..4 bindings; those bind as of 1494, so the ONE + -- thing still missing is `GameTooltip:AppendText` — our tooltip has SetText/AddLine and no + -- append verb, and the ref's suffix lands on a plate SetInventoryItem already wrote. That + -- verb's byte semantics (which line does it extend?) are unverified, so the suffix waits + -- rather than guessing; this is where it goes, over `GetBindingKey("TOGGLEBAG"..(5 - + -- button.bagId))` — the ref's index is REVERSED against the bar, so the slot nearest the + -- backpack (bagId 1) is TOGGLEBAG4/F11 and the farthest (bagId 4) is TOGGLEBAG1/F8. function BenillaBagBarSlot_OnEnter(button) if not GameTooltip then return end GameTooltip:SetOwner(button, "ANCHOR_LEFT") @@ -1676,10 +1732,13 @@ exactly as ContainerFrame.lua does it. Its 20 template buttons cover the keyring's 16-slot ceiling; the level-gated remainder hides through the ordinary physIndex-past-size branch. - It is deliberately NOT in BENILLA_BAG_WINDOWS: the keyring is not one of "your bags", so 'B' - / the backpack toggle / a vendor's auto-open never touch it, which is the reference's own - arrangement (OpenAllBags counts it out, and CloseAllBags is id-driven over 1..4). ESC still - closes it — UiPanels.xml's CloseAllWindows hides every open container, the ref's own loop. + It is deliberately NOT in BENILLA_BAG_WINDOWS: the keyring is not one of "your bags", so no + bag toggle ever OPENS it and a vendor's auto-open never touches it — the reference's own + arrangement (OpenAllBags' open arm is ToggleBackpack + ToggleBag(1..4), and its count + explicitly excludes KEYRING_CONTAINER). It is still SWEPT by the two close arms, which the + ref runs over every ContainerFrame — the keyring lives in one there, so 1494 sweeps ours by + name alongside the list. ESC closes it too — UiPanels.xml's CloseAllWindows hides every open + container, the ref's own loop. --> @@ -1750,10 +1809,17 @@ + addon's) — the stated divergence at BenillaBagToggle_OnClick's comment. + The shift fork is the ref's own (MainMenuBarBagButtons.xml l.81-87): the button + carries the SAME pair of knobs the B / SHIFT-B keys do (1494), so a shift-click + opens all bags and a plain one toggles the backpack. --> self:SetChecked(not self:GetChecked()) - BenillaBagToggle_OnClick() + if IsShiftKeyDown() then + OpenAllBags() + else + BenillaBagToggle_OnClick() + end BenillaBagToggle_OnEnter(self) BenillaBagBarButton_OnLeave() diff --git a/crates/benilla-app/assets/ui/OptionsFrame.xml b/crates/benilla-app/assets/ui/OptionsFrame.xml index 13deb82b..0a10b5a9 100644 --- a/crates/benilla-app/assets/ui/OptionsFrame.xml +++ b/crates/benilla-app/assets/ui/OptionsFrame.xml @@ -612,7 +612,7 @@ OPTIONS_PAGE_ROWS = { Controls = { "RowStickyTarget", "RowAutoLoot", "RowInvertMouse", "RowMouseSpeed", - "RowMaxCameraDistance", + "RowCameraFollowStyle", "RowMaxCameraDistance", }, Audio = { "RowEnableAll", "RowMaster", "RowSound", "RowMusic", "RowAmbience", @@ -867,6 +867,12 @@ if match then dd.selectedValue = match.value UIDropDownMenu_SetText(match.text, dd) + -- A dropdown whose ENTRIES each carry their own description repaints the row's + -- plate as the selection moves — the reference's own shape for exactly this row + -- (UIOptionsFrameCameraDropDown_OnClick: `tooltip = getglobal("OPTION_TOOLTIP_ + -- CAMERA"..GetSelectedID())`). Rows whose entries carry no `tip` keep the single + -- key OptionsRow_OnLoad gave them. + if match.tip then row.tip = match.tip end else dd.selectedValue = v UIDropDownMenu_SetText(v, dd) @@ -1983,11 +1989,34 @@ "22" means the wheel stops at 22 yards, which is the thing being chosen. Our default is the factor fully raised (30 yd), where 1.12 ships 1.0. --> - + + + + OptionsRow_OnLoad(self, "cameraSmoothStyle", + "Camera Following Style", "OPTION_TOOLTIP_CAMERA1") + OptionsDropdownRow_Setup(self, { + { text = "Smart", value = "1", tip = "OPTION_TOOLTIP_CAMERA1" }, + { text = "Always", value = "2", tip = "OPTION_TOOLTIP_CAMERA2" }, + { text = "Never", value = "3", tip = "OPTION_TOOLTIP_CAMERA3" }, + }) + + + + + + + OptionsRow_OnLoad(self, "cameraDistanceMaxFactor", "Max Camera Distance", "OPTION_TOOLTIP_MAX_FOLLOW_DIST") diff --git a/crates/benilla-app/assets/ui/ScriptLogFrame.xml b/crates/benilla-app/assets/ui/ScriptLogFrame.xml new file mode 100644 index 00000000..76daed25 --- /dev/null +++ b/crates/benilla-app/assets/ui/ScriptLogFrame.xml @@ -0,0 +1,366 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + self:SetScrollChild(getglobal(self:GetName() .. "Child")) + BenillaScroll_OnMouseWheel(self, arg1) + BenillaScroll_SyncBar(self) + BenillaScroll_SyncBar(self) + + + + + + + + + + + + + + + + + + + + + + + + BenillaScriptLogFrame_OnLoad(self) + + BenillaScriptLogFrame_OnMouseWheel(arg1) + BenillaScriptLog_Update() + BenillaScriptLogFrame_OnUpdate() + + + diff --git a/crates/benilla-app/assets/ui/SkillFrame.xml b/crates/benilla-app/assets/ui/SkillFrame.xml index aa5547c9..ea8c72fb 100644 --- a/crates/benilla-app/assets/ui/SkillFrame.xml +++ b/crates/benilla-app/assets/ui/SkillFrame.xml @@ -56,11 +56,14 @@ 2. NO SORT CHECKBOX (ref l.215-241's own XML-commented-out "Replace once code is ready..." `SkillSortButton`): the ref itself ships this dead/commented, so there is nothing to transcribe (TradeSkillFrame.xml's own "ref's own dead code is dropped" precedent). - 3. NO ACCEPT/CANCEL BUTTONS (ref l.318-357's `SkillFrameAcceptButton`/`SkillFrameCancelButton`, - the latter ALSO commented dead in the ref): both back the training-point economy (Accept/ - CancelSkillUps) this pane doesn't ship, AND this page has no window of its own to close — - `CharacterFrame`'s single shared close X (CharacterFrame.xml) already covers every - tab, matching `PaperDollFrame`'s own lack of a per-page close button. + 3. NO ACCEPT BUTTON (ref l.318-337's `SkillFrameAcceptButton`) — it is XML-commented dead + in the ref, and it backs the training-point economy this pane doesn't ship (deviation 1), + so there is nothing to transcribe. Its neighbour `SkillFrameCancelButton` (l.339-357) is + NOT dead and IS built: the ref's comment closes at l.337, one line above it, and the real + client draws a CLOSE button in the page's bottom-right seat. This list used to claim both + were commented out, and that misread is exactly why that seat rendered empty — decision + 1496. Only its `CancelSkillUps()` call drops (deviation 1's dead economy again); the two + hides are transcribed. 4. THE RANK TEXT FORMAT IS THE REF'S BYTE-EXACT ONE, not the task brief's own paraphrase: `ref-ui/SkillFrame.lua` l.178-186 builds `rank .. " (" .. color .. modifier .. close .. ")/" .. max` (e.g. "57 (|cff20ff20+3|r)/75") — transcribed verbatim (the @@ -333,6 +336,7 @@ end BenillaSkillFrame_UpdateDetail() + BenillaSkillExpandButtonFrame_Fit() end -- A rank-bar click selects it (ref SkillBar_OnClick, l.450-455). @@ -381,6 +385,38 @@ BenillaSkillFrame_Update() end + -- The tab is as wide as its own label (ref SkillFrameExpandButtonFrame's OnLoad, l.312-316: + -- SetWidth(SkillFrameCollapseAllButton:GetTextWidth()+45)) — the 45 is the two 8px caps plus + -- the label's 25px seat plus the margin the ref leaves before the right cap, and the middle + -- slab stretches into whatever that leaves off its own two anchors. + -- + -- Run from BOTH the ref's own OnLoad and every Update, because a measure here is not the + -- ref's synchronous one: benilla seats the VM's font engine at the frame boundary + -- (`ui_script::extract`, ahead of the tick), and XML load happens BEFORE any frame — so the + -- OnLoad call reads 0 and the tab would keep a 45px stump forever. The zero guard is what + -- makes the pair safe: an unmeasured call leaves the width alone rather than squashing the + -- tab, and the first Update after the atlas exists fits it. (The house's own preference is + -- two opposing anchors and no Lua width — UiPanels.xml's tab-highlight note — but a span + -- can only be resolved from two anchors when both edges are frames; a text width has to be + -- asked for.) + function BenillaSkillExpandButtonFrame_Fit() + local label = getglobal("SkillCollapseAllButton") + local w = label:GetTextWidth() + if w and w > 0 then + getglobal("SkillExpandButtonFrame"):SetWidth(w + 45) + end + end + + -- The page's own CLOSE (ref SkillFrameCancelButton's OnClick, l.350-356). The ref's + -- CancelSkillUps() drops out with the training-point economy this pane doesn't ship (this + -- file's header comment deviation 1); the two hides are the ref's own, in its own order — + -- the page first, then the window (CharacterFrame_OnHide's subframe loop would reach this + -- page anyway, but the ref hides it explicitly and so do we). + function BenillaSkillFrameCancelButton_OnClick() + getglobal("SkillFrame"):Hide() + HideUIPanel(getglobal("CharacterFrame")) + end + -- The window's wheel handler (bubbled up the parent chain from a row) → one row via the faux -- kit (TrainerFrame.xml's own note: the engine bubbles a spin up the parent chain). function BenillaSkillFrame_OnMouseWheel(delta) @@ -552,28 +588,72 @@ + QuestLogSortTab art TrainerFrame.xml's own BenillaTrainerExpandButtonFrame uses). + The tab is SIZED BY ITS LABEL, the ref's own OnLoad law (l.312-316: + SetWidth(GetTextWidth()+45)) — the middle slab stretches between the two 8px caps + off its OWN two anchors (ref l.279-287), so the 45 is exactly the two caps plus the + label's seat. Was a fixed 54 with a fixed 38 middle, copied off + TrainerFrame.xml rather than read off THIS ref; the label's own font fix below + (12px, not 10) would have run under the right cap at that width. --> - - + + + + BenillaSkillExpandButtonFrame_Fit() + + + diff --git a/crates/benilla-app/assets/ui/UiPanels.xml b/crates/benilla-app/assets/ui/UiPanels.xml index 27c67ec6..9489f19f 100644 --- a/crates/benilla-app/assets/ui/UiPanels.xml +++ b/crates/benilla-app/assets/ui/UiPanels.xml @@ -34,6 +34,13 @@ -- UISpecialFrames — the table exists now (1206) but it is the ADDONS' list, and mixing our -- windows into it would make an addon's `tremove` able to break the client's own ESC. UIPanelWindows["MailFrame"] = { area = "left", pushable = 0 } + -- The script error log (decision 1495) — OURS, with no reference row to transcribe. It is + -- registered here for exactly one reason: an unregistered frame takes ShowUIPanel's bare + -- `frame:Show()` branch (no left-slot placement, and `CloseAllWindows` never sees it), so + -- without this row the window would flip IsShown() without landing on screen and ESC would + -- not close it. `pushable = 9` puts it behind everything already registered: a diagnostic + -- must never shove a player's real window out of the slot it wanted. + UIPanelWindows["BenillaScriptLogFrame"] = { area = "left", pushable = 9 } -- The trade window (decision 0592 P1, TradeFrame.xml): the ref's own UIParent.lua l.27 -- registration (`UIPanelWindows["TradeFrame"] = { area = "left", pushable = 1 }`). Without -- this row ShowUIPanel takes its unregistered branch — a bare frame:Show() with no left-slot @@ -502,7 +509,8 @@ -- tracks `backpackWasOpen` so a bag you already had open is LEFT open on the window's close; -- benilla stores that on each frame (`wasOpen`). benilla DIVERGES from the ref here on -- purpose: the real OpenBackpack touches only the backpack, but ours opens EVERY equipped - -- bag with the window (director's call — decision 0561 — matching the 'B' all-bags toggle), + -- bag with the window (director's call — decision 0561; its stated reason was matching the + -- 'B' all-bags toggle, which 1494 split apart — the call itself stands on its own), -- so the names keep the ref's caller contract while the body loops the five windows. Every -- step is guarded, so a surface without the bag file — a headless panel test — no-ops -- rather than erroring. diff --git a/crates/benilla-app/assets/ui/benilla.toc b/crates/benilla-app/assets/ui/benilla.toc index 31895cbb..18ea9150 100644 --- a/crates/benilla-app/assets/ui/benilla.toc +++ b/crates/benilla-app/assets/ui/benilla.toc @@ -358,6 +358,14 @@ ChatFrame.xml # show/hide), and it must come AFTER SpellBookFrame, whose shift-click reaches # `BenillaMacroFrame_AddMacroLine`. Before GameMenuFrame, whose Macros button opens it. MacroFrame.xml +# The script error log (decision 1495): the window `/errors` opens — the list B293's two +# reporters asked for, which the reference's one-message-at-a-time ScriptErrors modal cannot +# be. Ours entirely (1.12 has no counterpart), reading the retained log on the model rather +# than any per-frame feed. Needs ScrollTemplates (BOTH kits — the faux one for the fixed row +# list, the real one for the traceback pane), UIPanelTemplates (the Clear/Close buttons) and +# UiPanels (ShowUIPanel/HideUIPanel + the UIPanelWindows row that seats it and gets it closed +# by ESC). Before GameMenuFrame, which is deliberately last. +ScriptLogFrame.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 diff --git a/crates/benilla-app/src/bindings.rs b/crates/benilla-app/src/bindings.rs index 9421efda..2d8883f6 100644 --- a/crates/benilla-app/src/bindings.rs +++ b/crates/benilla-app/src/bindings.rs @@ -1429,4 +1429,102 @@ mod tests { "a UI wheel is the frame's" ); } + + /// **B opens the backpack, SHIFT-B opens all of them** — the director's report, pinned end to + /// end (1494): a real keyboard event, through the chord table and the registry's shipped + /// defaults, into the binding body, into the Lua that shows the windows. + /// + /// This is the seam the bug lived in and nothing else covers it. `bag_tests` drive the Lua + /// globals directly, so they could not see that BOTH chords were registered on one command; + /// the registry tests read the table, so they could not see what the bodies do. Only pressing + /// the key joins the two. + #[test] + fn b_opens_the_backpack_and_shift_b_opens_every_bag() { + let mut script = UiScript::new().expect("VM"); + script.register_bindings(®istry_commands()); + script.set_screen_size(1024.0, 768.0); + for file in [ + "Fonts.xml", + "UiPanels.xml", + "MerchantFrame.xml", + "Cooldown.xml", + "BagFrame.xml", + ] { + let text = std::fs::read_to_string( + std::path::Path::new(env!("CARGO_MANIFEST_DIR")) + .join("assets/ui") + .join(file), + ) + .expect("shipped asset"); + let doc = benilla_ui::framexml::parse(&text).expect("parse"); + let report = benilla_ui::loader::load(&script, &doc, &|_| None); + assert!(report.errors.is_empty(), "{file}: {:?}", report.errors); + } + script.set_money(0); + // The backpack, plus one equipped bag in slot 2 — so "all bags" is two windows and + // "the backpack alone" is visibly different from it. + script.set_container( + 0, + Some(benilla_ui::script::ContainerState { + name: Some("Backpack".into()), + num_slots: 16, + slots: std::collections::HashMap::new(), + }), + ); + script.set_container( + 2, + Some(benilla_ui::script::ContainerState { + name: Some("Small Pouch".into()), + num_slots: 6, + slots: std::collections::HashMap::new(), + }), + ); + let mut app = vm_harness(script); + let shown = |app: &App, name: &str| { + app.world() + .non_send_resource::() + .eval::(&format!("return {name}:IsShown()")) + .expect("eval") + }; + + // B — TOGGLEBACKPACK's shipped default. The backpack, and nothing else. + press_key(&mut app, KeyCode::KeyB); + app.update(); + release_key(&mut app, KeyCode::KeyB); + app.update(); + assert!(shown(&app, "BenillaBagFrame"), "B opens the backpack"); + assert!( + !shown(&app, "BenillaBagFrame2"), + "B does NOT open the equipped bag — this is the bug 1494 fixes" + ); + + // B again: bag 0 is open, so this is the close arm. + press_key(&mut app, KeyCode::KeyB); + app.update(); + release_key(&mut app, KeyCode::KeyB); + app.update(); + assert!(!shown(&app, "BenillaBagFrame"), "B again shuts it"); + + // SHIFT-B — OPENALLBAGS' shipped default, a DIFFERENT command with a different body. + press_key(&mut app, KeyCode::ShiftLeft); + press_key(&mut app, KeyCode::KeyB); + app.update(); + release_key(&mut app, KeyCode::KeyB); + app.update(); + assert!( + shown(&app, "BenillaBagFrame") && shown(&app, "BenillaBagFrame2"), + "SHIFT-B opens every bag" + ); + + // …and closes them again, the count now reading all-open. + press_key(&mut app, KeyCode::KeyB); + app.update(); + release_key(&mut app, KeyCode::KeyB); + release_key(&mut app, KeyCode::ShiftLeft); + app.update(); + assert!( + !shown(&app, "BenillaBagFrame") && !shown(&app, "BenillaBagFrame2"), + "SHIFT-B again shuts them all" + ); + } } diff --git a/crates/benilla-app/src/bindings/commands.rs b/crates/benilla-app/src/bindings/commands.rs index d6283774..fc144ce8 100644 --- a/crates/benilla-app/src/bindings/commands.rs +++ b/crates/benilla-app/src/bindings/commands.rs @@ -19,8 +19,9 @@ //! //! Recorded default divergences (0997): `TOGGLEUI` ships `ALT-Z` (0870: the cache's `CTRL-Z` is //! a player rebind all three accounts inherited, not the shipped default — the one row that does -//! not trust the cache); `OPENALLBAGS` ships `B` + `SHIFT-B` (1.12 splits B/TOGGLEBACKPACK from -//! SHIFT-B/OPENALLBAGS; benilla's one bag knob is the open-all toggle that already lived on B). +//! not trust the cache). That is now the ONLY one — the bag row's divergence (`OPENALLBAGS` wearing +//! both `B` and `SHIFT-B`, because benilla had a single all-bags knob) is gone as of 1494: the +//! reference's split ships whole, keys and bodies alike. /// A command's index into [`SPECS`] — the engine-side handle (`cmd::JUMP`). #[derive(Clone, Copy, PartialEq, Eq, Hash, Debug)] @@ -72,39 +73,77 @@ macro_rules! spec { /// Engine-side handles for the host-dispatched commands (indexes into [`SPECS`], asserted by /// test). Lua-bodied commands need no handle — nothing in Rust names them. pub(crate) mod cmd { - use super::Cmd; - pub(crate) const MOVE_AND_STEER: Cmd = Cmd(0); - pub(crate) const MOVE_FORWARD: Cmd = Cmd(1); - pub(crate) const MOVE_BACKWARD: Cmd = Cmd(2); - pub(crate) const TURN_LEFT: Cmd = Cmd(3); - pub(crate) const TURN_RIGHT: Cmd = Cmd(4); - pub(crate) const STRAFE_LEFT: Cmd = Cmd(5); - pub(crate) const STRAFE_RIGHT: Cmd = Cmd(6); - pub(crate) const JUMP: Cmd = Cmd(7); - pub(crate) const SIT_OR_STAND: Cmd = Cmd(8); - pub(crate) const TOGGLE_SHEATH: Cmd = Cmd(9); - pub(crate) const TOGGLE_AUTORUN: Cmd = Cmd(10); - 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(51); - pub(crate) const TARGET_PREVIOUS_ENEMY: Cmd = Cmd(52); - pub(crate) const NAMEPLATES: Cmd = Cmd(63); - pub(crate) const FRIEND_NAMEPLATES: Cmd = Cmd(64); - pub(crate) const ALL_NAMEPLATES: Cmd = Cmd(65); - pub(crate) const ATTACK_TARGET: Cmd = Cmd(66); - // These three shift whenever a row lands ahead of them in SPECS (1057's `TOGGLECHARACTER3` - // moved them 86/87/88 → 87/88/89; 1136's `TOGGLEACTIONBARLOCK` → 88/89/90). - // `the_cmd_handles_index_their_rows` is what makes that loud and cheap instead of silent: it - // names the handle and the row it actually points at. - pub(crate) const TOGGLE_UI: Cmd = Cmd(88); - pub(crate) const CAMERA_ZOOM_IN: Cmd = Cmd(90); - pub(crate) const CAMERA_ZOOM_OUT: Cmd = Cmd(91); + use super::{Cmd, TABLE}; + + /// Resolve a command NAME to its [`Cmd`] handle at compile time — a linear scan of [`TABLE`] + /// in a `const fn`, so a name that is not in the registry is a BUILD error, not a runtime + /// surprise. + /// + /// These handles used to be hand-written row numbers, and every row that landed ahead of one + /// silently re-pointed it at its neighbour: 1057's `TOGGLECHARACTER3` moved three of them + /// 86/87/88 → 87/88/89, 1136's `TOGGLEACTIONBARLOCK` moved the same three again, and 1494's + /// bag family would have moved them a third time (`TOGGLE_UI` landing on `MINIMAPZOOMOUT`). A + /// guard test named the drift but the numbers still had to be re-typed by hand each time; the + /// name is the thing we actually mean, so the name is what the table is asked for now. + const fn by_name(name: &str) -> Cmd { + let mut i = 0; + while i < TABLE.len() { + if const_str_eq(TABLE[i].name, name) { + return Cmd(i as u16); + } + i += 1; + } + panic!("no such binding command") + } + + const fn const_str_eq(a: &str, b: &str) -> bool { + let (a, b) = (a.as_bytes(), b.as_bytes()); + if a.len() != b.len() { + return false; + } + let mut i = 0; + while i < a.len() { + if a[i] != b[i] { + return false; + } + i += 1; + } + true + } + + pub(crate) const MOVE_AND_STEER: Cmd = by_name("MOVEANDSTEER"); + pub(crate) const MOVE_FORWARD: Cmd = by_name("MOVEFORWARD"); + pub(crate) const MOVE_BACKWARD: Cmd = by_name("MOVEBACKWARD"); + pub(crate) const TURN_LEFT: Cmd = by_name("TURNLEFT"); + pub(crate) const TURN_RIGHT: Cmd = by_name("TURNRIGHT"); + pub(crate) const STRAFE_LEFT: Cmd = by_name("STRAFELEFT"); + pub(crate) const STRAFE_RIGHT: Cmd = by_name("STRAFERIGHT"); + pub(crate) const JUMP: Cmd = by_name("JUMP"); + pub(crate) const SIT_OR_STAND: Cmd = by_name("SITORSTAND"); + pub(crate) const TOGGLE_SHEATH: Cmd = by_name("TOGGLESHEATH"); + pub(crate) const TOGGLE_AUTORUN: Cmd = by_name("TOGGLEAUTORUN"); + pub(crate) const OPEN_CHAT: Cmd = by_name("OPENCHAT"); + pub(crate) const OPEN_CHAT_SLASH: Cmd = by_name("OPENCHATSLASH"); + pub(crate) const REPLY: Cmd = by_name("REPLY"); + pub(crate) const TARGET_NEAREST_ENEMY: Cmd = by_name("TARGETNEARESTENEMY"); + pub(crate) const TARGET_PREVIOUS_ENEMY: Cmd = by_name("TARGETPREVIOUSENEMY"); + pub(crate) const NAMEPLATES: Cmd = by_name("NAMEPLATES"); + pub(crate) const FRIEND_NAMEPLATES: Cmd = by_name("FRIENDNAMEPLATES"); + pub(crate) const ALL_NAMEPLATES: Cmd = by_name("ALLNAMEPLATES"); + pub(crate) const ATTACK_TARGET: Cmd = by_name("ATTACKTARGET"); + pub(crate) const TOGGLE_UI: Cmd = by_name("TOGGLEUI"); + pub(crate) const CAMERA_ZOOM_IN: Cmd = by_name("CAMERAZOOMIN"); + pub(crate) const CAMERA_ZOOM_OUT: Cmd = by_name("CAMERAZOOMOUT"); } /// The registry, 1.12 `Bindings.xml` order. Sub-tables (action buttons, shapeshift, raid /// targets) are written out because each row carries its own Lua body string. -pub(crate) static SPECS: &[Spec] = &[ +/// +/// `TABLE` is the `const` view [`cmd::by_name`] scans at compile time (a `static` cannot be read +/// during const evaluation); `SPECS` is the one everything else uses. +pub(crate) static SPECS: &[Spec] = TABLE; + +const TABLE: &[Spec] = &[ // ── Movement (BINDING_HEADER_MOVEMENT) ────────────────────────────────────────────── spec!("MOVEANDSTEER", MOVEMENT, Kind::Held, Some("BUTTON3"), None), spec!("MOVEFORWARD", MOVEMENT, Kind::Held, Some("W"), Some("UP")), @@ -556,21 +595,64 @@ pub(crate) static SPECS: &[Spec] = &[ Some("C"), None ), - // 1.12 splits TOGGLEBACKPACK (B, F12) from OPENALLBAGS (SHIFT-B); benilla's one bag knob is - // the open-all toggle that already lived on B — so OPENALLBAGS ships with both defaults - // (recorded divergence, 0997). + // The bag family, in the reference's own `Bindings.xml` order and with its own bodies and + // defaults (1494). 0997 collapsed all of this onto ONE command — `OPENALLBAGS` wearing both + // `B` and `SHIFT-B` over a `ToggleBackpack()` that opened every bag — because benilla had a + // single all-bags knob to hang keys off. The reference has three knobs and the director + // reported the difference: `B` opens the backpack ALONE, `SHIFT-B` opens the lot. // - // The body is the bare GLOBAL, exactly the ref's own Bindings.xml (`ToggleBackpack();`) — - // NOT the backpack button's OnClick handler it used to run: the handler carries the button's - // checked bookkeeping, and routing the key through it dragged that onto a path the reference - // keeps clean (with a bag addon holding `ToggleBackpack`, the key press is the path that - // lights the backpack button on the real client — the addon's OnShow write is the last word). + // Every default here is byte-real from the client's own `bindings-cache.wtf`, identical across + // all three independent accounts (ONE, TWO, WINUSER) — which is what rules out a player rebind + // (the `TOGGLEUI` trap, 0870). + // + // Each body is the bare GLOBAL, exactly the ref's own `Bindings.xml` — NOT a button's OnClick + // handler: a handler carries the button's checked bookkeeping, and routing a key through it + // drags that onto a path the reference keeps clean (with a bag addon holding `ToggleBackpack`, + // the key press is the path that lights the backpack button on the real client — the addon's + // OnShow write is the last word). spec!( - "OPENALLBAGS", + "TOGGLEBACKPACK", INTERFACE, Kind::Edge("ToggleBackpack()"), Some("B"), - Some("SHIFT-B") + Some("F12") + ), + // TOGGLEBAG**N** is bag **5-N**: the reference's numbering runs the bar right-to-left, so F8 + // opens the slot FARTHEST from the backpack. Quoted from ref Bindings.xml l.564-575. + spec!( + "TOGGLEBAG1", + INTERFACE, + Kind::Edge("ToggleBag(4)"), + Some("F8"), + None + ), + spec!( + "TOGGLEBAG2", + INTERFACE, + Kind::Edge("ToggleBag(3)"), + Some("F9"), + None + ), + spec!( + "TOGGLEBAG3", + INTERFACE, + Kind::Edge("ToggleBag(2)"), + Some("F10"), + None + ), + spec!( + "TOGGLEBAG4", + INTERFACE, + Kind::Edge("ToggleBag(1)"), + Some("F11"), + None + ), + spec!( + "OPENALLBAGS", + INTERFACE, + Kind::Edge("OpenAllBags()"), + Some("SHIFT-B"), + None ), spec!( "TOGGLECHARACTER1", @@ -1074,43 +1156,6 @@ pub(crate) static SPECS: &[Spec] = &[ mod tests { use super::*; - /// The named handles in [`cmd`] must index the rows they claim — the table is the truth, - /// the consts are the readable view of it. - #[test] - fn the_cmd_handles_index_their_rows() { - for (handle, name) in [ - (cmd::MOVE_AND_STEER, "MOVEANDSTEER"), - (cmd::MOVE_FORWARD, "MOVEFORWARD"), - (cmd::MOVE_BACKWARD, "MOVEBACKWARD"), - (cmd::TURN_LEFT, "TURNLEFT"), - (cmd::TURN_RIGHT, "TURNRIGHT"), - (cmd::STRAFE_LEFT, "STRAFELEFT"), - (cmd::STRAFE_RIGHT, "STRAFERIGHT"), - (cmd::JUMP, "JUMP"), - (cmd::SIT_OR_STAND, "SITORSTAND"), - (cmd::TOGGLE_SHEATH, "TOGGLESHEATH"), - (cmd::TOGGLE_AUTORUN, "TOGGLEAUTORUN"), - (cmd::OPEN_CHAT, "OPENCHAT"), - (cmd::OPEN_CHAT_SLASH, "OPENCHATSLASH"), - (cmd::REPLY, "REPLY"), - (cmd::TARGET_NEAREST_ENEMY, "TARGETNEARESTENEMY"), - (cmd::TARGET_PREVIOUS_ENEMY, "TARGETPREVIOUSENEMY"), - (cmd::NAMEPLATES, "NAMEPLATES"), - (cmd::FRIEND_NAMEPLATES, "FRIENDNAMEPLATES"), - (cmd::ALL_NAMEPLATES, "ALLNAMEPLATES"), - (cmd::ATTACK_TARGET, "ATTACKTARGET"), - (cmd::TOGGLE_UI, "TOGGLEUI"), - (cmd::CAMERA_ZOOM_IN, "CAMERAZOOMIN"), - (cmd::CAMERA_ZOOM_OUT, "CAMERAZOOMOUT"), - ] { - assert_eq!( - SPECS[handle.0 as usize].name, name, - "cmd handle {name} points at SPECS[{}] = {}", - handle.0, SPECS[handle.0 as usize].name - ); - } - } - /// Every default chord in the table parses — a typo'd token would otherwise silently ship /// an unpressable default. #[test] @@ -1214,13 +1259,6 @@ mod tests { s.name ); let ours: Vec<&str> = [s.d1, s.d2].into_iter().flatten().collect(); - if s.name == "OPENALLBAGS" { - // The one recorded divergence (0997): 1.12 splits `B`/TOGGLEBACKPACK from - // `SHIFT-B`/OPENALLBAGS, and benilla's single bag knob is the open-all toggle - // that already lived on B — so it ships wearing both. - assert_eq!(ours, ["B", "SHIFT-B"]); - continue; - } assert_eq!( ours, defaults.get(s.name).cloned().unwrap_or_default(), diff --git a/crates/benilla-app/src/cvars.rs b/crates/benilla-app/src/cvars.rs index 2a7e964d..1c41b60f 100644 --- a/crates/benilla-app/src/cvars.rs +++ b/crates/benilla-app/src/cvars.rs @@ -31,7 +31,7 @@ use bevy::prelude::*; use crate::chat_bubble::BubbleConfig; use crate::minimap::MinimapZoom; use crate::nameplates::NameConfig; -use crate::player::camera::{LookConfig, ZoomLimit, MOUSE_SPEED_RANGE}; +use crate::player::camera::{FollowStyle, LookConfig, ZoomLimit, MOUSE_SPEED_RANGE}; use crate::portrait::PaneRate; use crate::sound::SoundConfig; use crate::target::ClickConfig; @@ -107,6 +107,13 @@ pub(crate) const REGISTERED: &[(&str, &str)] = &[ // raised — because that IS benilla's shipped 30 yd ceiling, a knowing divergence from the // reference's registrar "1" that camera.rs has carried in prose since it was written. ("cameraDistanceMaxFactor", "2"), + // Camera Following Style (1493): 1.12's `cameraSmoothStyle` — the auto-follow that swings the + // camera back behind the character. Registered "1" = Smart, which is BOTH the reference's + // registrar default and the director's call; benilla behaved as "3" (Never) unconditionally + // until this row, so the knob is what un-freezes a camera that was frozen by design. The three + // values are the reference dropdown's own (UIOptionsFrameCameraDropDown: 1 Smart, 2 Always, + // 3 Never) — see `FollowStyle`. + ("cameraSmoothStyle", "1"), // Status Text (1140): 1.12's `statusBarText`, the "always show value / max on a status bar" // switch. **No host knob** — its consumer is Lua (TextStatusBar.xml, decision 1082, which was // written waiting for this key and reads it on every repaint). Default "0": the reference's @@ -234,6 +241,7 @@ struct Knobs<'a> { minimap: &'a mut MinimapZoom, bubbles: &'a mut BubbleConfig, zoom: &'a mut ZoomLimit, + follow: &'a mut FollowStyle, video: &'a mut VideoConfig, pane_rate: &'a mut PaneRate, } @@ -261,6 +269,9 @@ fn apply_to_knobs(name: &str, value: &str, knobs: &mut Knobs) -> bool { "deselectonclick" => knobs.click.deselect_on_click = v != 0.0, "mouseinvertpitch" => knobs.look.invert_pitch = v != 0.0, "cameradistancemaxfactor" => knobs.zoom.set_factor(v), + // The three stops are 1 Smart / 2 Always / 3 Never; anything else reads as the registrar + // default rather than as a dead camera (`FollowStyle::from_cvar`). + "camerasmoothstyle" => *knobs.follow = FollowStyle::from_cvar(v), // The 1.12 slider's own range; an off-grid hand-edit rides between stops, like the others. "mousespeed" => { knobs.look.sensitivity = v.clamp(*MOUSE_SPEED_RANGE.start(), *MOUSE_SPEED_RANGE.end()); @@ -321,6 +332,7 @@ fn load_config( mut minimap: ResMut, mut bubbles: ResMut, mut zoom: ResMut, + mut follow: ResMut, mut video: ResMut, mut pane_rate: ResMut, ) { @@ -336,6 +348,7 @@ fn load_config( minimap: &mut minimap, bubbles: &mut bubbles, zoom: &mut zoom, + follow: &mut follow, video: &mut video, pane_rate: &mut pane_rate, }; @@ -414,6 +427,7 @@ fn sync_cvars( mut minimap: ResMut, mut bubbles: ResMut, mut zoom: ResMut, + mut follow: ResMut, mut video: ResMut, mut pane_rate: ResMut, ) { @@ -435,7 +449,7 @@ fn sync_cvars( ); script.register_cvars(REGISTERED.iter().copied()); let flag = |b: bool| if b { "1" } else { "0" }.to_string(); - let session: [(&str, String); 25] = [ + let session: [(&str, String); 26] = [ ("MasterVolume", sound.master.to_string()), ("SoundVolume", sound.sfx.to_string()), ("MusicVolume", sound.music.to_string()), @@ -450,6 +464,7 @@ fn sync_cvars( ("mouseInvertPitch", flag(look.invert_pitch)), ("mousespeed", look.sensitivity.to_string()), ("cameraDistanceMaxFactor", zoom.factor().to_string()), + ("cameraSmoothStyle", follow.cvar().to_string()), ("autoLootDefault", flag(loot.auto_loot)), ("UnitNamePlayer", flag(names.player)), ("UnitNameNPC", flag(names.npc)), @@ -488,6 +503,7 @@ fn sync_cvars( minimap: &mut minimap, bubbles: &mut bubbles, zoom: &mut zoom, + follow: &mut follow, video: &mut video, pane_rate: &mut pane_rate, }; @@ -532,6 +548,7 @@ pub(crate) fn fold_dying_vm_cvars(world: &mut World) { Option>, Option>, Option>, + Option>, Option>, Option>, )> = bevy::ecs::system::SystemState::new(world); @@ -549,6 +566,7 @@ pub(crate) fn fold_dying_vm_cvars(world: &mut World) { minimap, bubbles, zoom, + follow, video, pane_rate, ) = state.get_mut(world); @@ -572,11 +590,12 @@ pub(crate) fn fold_dying_vm_cvars(world: &mut World) { Some(mut minimap), Some(mut bubbles), Some(mut zoom), + Some(mut follow), Some(mut video), Some(mut pane_rate), ) = ( - sound, scale, view, look, click, loot, names, clutter, minimap, bubbles, zoom, video, - pane_rate, + sound, scale, view, look, click, loot, names, clutter, minimap, bubbles, zoom, follow, + video, pane_rate, ) { let mut knobs = Knobs { sound: &mut sound, @@ -590,6 +609,7 @@ pub(crate) fn fold_dying_vm_cvars(world: &mut World) { minimap: &mut minimap, bubbles: &mut bubbles, zoom: &mut zoom, + follow: &mut follow, video: &mut video, pane_rate: &mut pane_rate, }; @@ -736,6 +756,14 @@ mod tests { ); assert_eq!(d["mousespeed"], LookConfig::default().sensitivity); assert_eq!(d["cameraDistanceMaxFactor"], ZoomLimit::default().factor()); + // The following style (1493) welds to the enum's own Default — and BOTH sides are the + // reference's registrar "1" = Smart, the one row in this arc that agrees with the binary + // outright. + assert_eq!( + d["cameraSmoothStyle"], + FollowStyle::default().cvar().parse::().unwrap() + ); + assert_eq!(FollowStyle::default(), FollowStyle::Smart); assert_eq!(d["autoLootDefault"] != 0.0, LootConfig::default().auto_loot); // The name trio (0992) welds to NameConfig's defaults the same way. let names = NameConfig::default(); @@ -782,6 +810,7 @@ mod tests { let mut minimap = MinimapZoom::default(); let mut bubbles = BubbleConfig::default(); let mut zoom = ZoomLimit::default(); + let mut follow = FollowStyle::default(); let mut video = VideoConfig::default(); let mut pane_rate = PaneRate::default(); let mut knobs = Knobs { @@ -796,6 +825,7 @@ mod tests { minimap: &mut minimap, bubbles: &mut bubbles, zoom: &mut zoom, + follow: &mut follow, video: &mut video, pane_rate: &mut pane_rate, }; @@ -821,6 +851,14 @@ mod tests { assert_eq!(knobs.look.sensitivity, 1.4); assert!(apply_to_knobs("mousespeed", "9", &mut knobs)); assert_eq!(knobs.look.sensitivity, 1.5); + // The following style lands as the enum, on the reference's own numbering — and an + // off-grid value reads as the registrar default rather than as a dead camera. + assert!(apply_to_knobs("cameraSmoothStyle", "3", &mut knobs)); + assert_eq!(*knobs.follow, FollowStyle::Never); + assert!(apply_to_knobs("camerasmoothstyle", "2", &mut knobs)); + assert_eq!(*knobs.follow, FollowStyle::Always); + assert!(apply_to_knobs("cameraSmoothStyle", "0", &mut knobs)); + assert_eq!(*knobs.follow, FollowStyle::Smart); // The max-orbit factor lands as YARDS on the knob (base 15 x factor), clamped to 1..2. assert!(apply_to_knobs("cameraDistanceMaxFactor", "1", &mut knobs)); assert_eq!(knobs.zoom.max, 15.0); @@ -915,6 +953,7 @@ mod tests { .init_resource::() .init_resource::() .init_resource::() + .init_resource::() .init_resource::() .init_resource::() .add_plugins(CvarPlugin); @@ -985,6 +1024,7 @@ mod tests { .init_resource::() .init_resource::() .init_resource::() + .init_resource::() .init_resource::() .init_resource::() .add_plugins(CvarPlugin); diff --git a/crates/benilla-app/src/player.rs b/crates/benilla-app/src/player.rs index adad4aa3..085e2a31 100644 --- a/crates/benilla-app/src/player.rs +++ b/crates/benilla-app/src/player.rs @@ -10,8 +10,10 @@ //! forward** (vanilla's "both-button move"), steering with the mouse like a right-drag. The **scroll //! wheel** zooms the third-person //! 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). +//! distance). A left-drag orbit offset is reeled back in by the **auto-follow** — vanilla's +//! `cameraSmoothStyle`, a real player setting since decision 1493 ([`camera::FollowStyle`]): +//! Smart (the shipped default) chases while the character moves, Always chases always, Never +//! leaves the camera exactly where the hand put it. //! The **dev chord + `F`** toggles free-fly (1043); the **dev chord + `G`** lands the avatar where //! the camera is ([`land`]). //! @@ -187,6 +189,7 @@ impl Plugin for PlayerPlugin { ); app.init_resource::(); app.init_resource::(); + app.init_resource::(); // Far sight: resolve `PLAYER_FARSIGHT` into a pose before `control` reads it to seat the // camera. A separate system rather than another query on `control` for a hard reason — // `control` already holds the self entity's `Transform` mutably, so it cannot also read an @@ -340,13 +343,15 @@ fn control( buttons: Res>, // Nested into one param to stay within Bevy's 16-element system-param tuple limit. (The // scroll wheel left this tuple with 0997: zoom reads the CAMERAZOOM bindings now.) - // The pointer's motion this frame + the two knobs that scale what it does with it. Bundled - // because a Bevy system takes at most 16 parameters and this one is at the ceiling — the - // grouping is the existing `mouse` tuple, widened rather than a seventeenth argument. + // The pointer's motion this frame + the camera knobs that scale what it does with it — the + // two look/zoom knobs and the auto-follow style (decision 1493). Bundled because a Bevy system + // takes at most 16 parameters and this one is at the ceiling — the grouping is the existing + // `mouse` tuple, widened rather than a seventeenth argument. pointer: ( Res, Res, Res, + Res, ), // The net bridge, bundled into one param (16-param limit): the outbound command channel + the // inbound teleport/worldport messages `apply_net_updates` wrote earlier this frame @@ -488,6 +493,15 @@ fn control( let view_subject = &speed_capsule.8; let self_guid = speed_capsule.9 .0; let scoped = &speed_capsule.10; + // The auto-follow style (decision 1493), with far sight's one exception folded in here so both + // camera seats below agree: while the rig orbits somebody ELSE's body (Mind Vision, Sentry + // Totem), our own facing is not what "behind" means, so the chase is forced off rather than + // reeling that camera toward a heading with nothing to do with the view. + let follow_style = if view_subject.remote.is_some() { + camera::FollowStyle::Never + } else { + *pointer.3 + }; 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 the avatar isn't also driven while typing (a `.tele` command). Mouse still @@ -718,9 +732,17 @@ fn control( // which reads as the view detaching into free flight (director, 2026-08-13). A // `reseat` window is excluded — there the resource still describes the body we are // letting go of, and `apply_server_moves` owns the adoption. + // + // The same sync answers the auto-follow's Smart gate (1493): is the body we are NOT + // driving nevertheless moving? A taxi, a Charge, a knockback or a fear all translate + // the avatar while the controller stands down, and a camera that ignored them would + // sit at the angle you left it while the body ran out from under it. A spline ride is + // translation by definition; a driven body is measured against the pose we held. + let mut driven_moving = player.server_riding; if player.control_lost && !player.reseat { if let Ok((_, t, ..)) = body.single() { let yaw = server_ride::yaw_of(t.rotation); + driven_moving |= t.translation.distance_squared(player.pos) > 1.0e-6; player.pos = t.translation; player.face_yaw = yaw; player.model_yaw = yaw; @@ -750,6 +772,11 @@ fn control( &mut cam_t, &collide, cam_probe, + &camera::FollowInput { + style: follow_style, + face_yaw: player.face_yaw, + moving: driven_moving, + }, ); // Flush a stale run once, so observers stop extrapolating it — but never under a ride, // whose FORWARD report is deliberate and would be cancelled every frame. @@ -1679,6 +1706,13 @@ fn control( // — the deck's yaw delta was already applied to `cam.yaw` at the ride block (frame motion // carries the camera unconditionally; only input turns respect `seat_camera`'s // look-session gate). + // The auto-follow's own three facts (decision 1493): the player's style, where "behind" + // is, and whether the character is translating — Smart's gate. + let follow = camera::FollowInput { + style: follow_style, + face_yaw: player.face_yaw, + moving, + }; seat_camera( dt, turn_delta, @@ -1690,6 +1724,7 @@ fn control( &mut cam_t, &collide, cam_probe, + &follow, ); // The cast bar's local self-cancel trigger (`ui_cast::local_self_cancel`): a fresh diff --git a/crates/benilla-app/src/player/camera.rs b/crates/benilla-app/src/player/camera.rs index 524b7c1d..18a5e7f9 100644 --- a/crates/benilla-app/src/player/camera.rs +++ b/crates/benilla-app/src/player/camera.rs @@ -12,6 +12,7 @@ use bevy::window::{CursorGrabMode, CursorOptions}; use avian3d::prelude::*; +use crate::creature_anim::wrap_pi; use crate::net::Embodied; use benilla_assets::materials::WowModelMaterial; use benilla_world::interact::{WorldClick, WorldRightClick, WorldRightPress}; @@ -194,6 +195,95 @@ impl LookConfig { LOOK_SENSITIVITY * self.sensitivity } } +/// The auto-follow's angular rate — 1.12's `cameraYawSmoothSpeed`, whose registrar default is +/// **180 °/s** (`WoW.exe` `[0xbe1070]`, the yaw transition channel's own CVar float — wow-re +/// `ui/scratch/camera-control.md`). The reference exposes it as the AUTO_FOLLOW_SPEED slider +/// (`UIOptionsFrameSliders`, 90 … 270 °/s by 10) and *disables that slider* when the style is +/// Never; benilla ships the registrar default as a constant, since it has no row for it yet. +const FOLLOW_RATE: f32 = std::f32::consts::PI; + +/// **Camera Following Style** — 1.12's `cameraSmoothStyle` (decision 1493): does the camera swing +/// back behind the character on its own? +/// +/// benilla shipped the reference behaviour *removed* from the day the camera was written (the +/// orbit offset simply persisted, a director's call) — this is the setting that gives it back, and +/// the director's call now is the reference's own default: **Smart**. +/// +/// The three values are the reference's, dropdown order and all (`UIOptionsFrameCameraDropDown`, +/// which writes this CVar): `"1"` Smart, `"2"` Always, `"3"` Never — note that Never is **3**, not +/// 0. The registrar default is `"1"`. +#[derive(Resource, Clone, Copy, PartialEq, Eq, Debug, Default)] +pub(crate) enum FollowStyle { + /// Stay where placed, *except while the character is moving* — the reference's own + /// "(Recommended Mode)" and its registrar default. + #[default] + Smart, + /// Always prefer being behind the character. + Always, + /// Never auto-adjust: the orbit offset stays exactly where the hand left it. This is what + /// benilla did unconditionally before 1493. + Never, +} + +impl FollowStyle { + /// From the CVar's number. Anything off the three stops reads as the registrar default — + /// a hand-edited config or an addon's stray write gets the shipped behaviour, not a dead + /// camera. + pub(crate) fn from_cvar(v: f32) -> Self { + match v as i32 { + 2 => Self::Always, + 3 => Self::Never, + _ => Self::Smart, + } + } + + /// The CVar string this style is — the value the table and `config.toml` carry. + pub(crate) fn cvar(self) -> &'static str { + match self { + Self::Smart => "1", + Self::Always => "2", + Self::Never => "3", + } + } + + /// Should the camera chase the character's facing this frame? `moving` is the character's own + /// translation intent (Smart's gate — the reference's "except when your character is moving"). + fn chases(self, moving: bool) -> bool { + match self { + Self::Always => true, + Self::Smart => moving, + Self::Never => false, + } + } +} + +/// What [`seat_camera`] needs to run the auto-follow this frame — the style, where "behind" is, +/// and whether the character is moving. Bundled because the three are one question, and +/// `seat_camera` is already at its argument ceiling. +pub(super) struct FollowInput { + pub(super) style: FollowStyle, + /// The character's own facing (same yaw convention as [`FlyCam::yaw`] — a right-drag couples + /// the two directly). + pub(super) face_yaw: f32, + /// Is the character translating this frame? (Turn-in-place is not movement here: a keyboard + /// turn already carries the camera rigidly, so there is no offset left for the chase.) + pub(super) moving: bool, +} + +impl FollowInput { + /// The yaw this frame's chase adds — the shortest arc to the character's facing, capped at + /// [`FOLLOW_RATE`]·dt so the camera *slews* rather than snapping, and exactly zero on a style + /// (or a stillness) that isn't chasing. The cap stops **at** the target: the clamp can never + /// overshoot, so a settled camera writes nothing and the no-op gate downstream stays quiet. + fn step(&self, cam_yaw: f32, dt: f32) -> f32 { + if !self.style.chases(self.moving) { + return 0.0; + } + let step = FOLLOW_RATE * dt; + wrap_pi(self.face_yaw - cam_yaw).clamp(-step, step) + } +} + /// Camera pitch clamp (radians) — **VERIFIED ±89.00°** (`WoW.exe` `0x8089d8`/`0x8089dc` = /// 1.5533430576 rad; the pitch integrate `FUN_00510120`, wow-re `follow-camera`). A single uniform /// clamp at every zoom level — the reference has **no** distinct first-person look-down limit. @@ -530,7 +620,8 @@ pub(super) fn apply_zoom_scroll(scroll: f32, dt: f32, rig: &mut CameraControl, m /// deck turning under the rider is frame motion and is applied to `cam.yaw` at the ride block in /// [`super::control`], bypassing this function's look-session gate (routing it here was the /// right-drag drift bug — the gate ate the deck's share while a drag was held). A left-drag orbit -/// offset otherwise **persists** (no auto-follow — director's call, see the body). +/// offset is then reeled back in by the **auto-follow**, on the player's `cameraSmoothStyle` +/// setting ([`FollowStyle`], decision 1493) — or kept forever, on Never. /// `head`/`player_pos` are precomputed by [`super::control`] (which owns the avatar capsule /// constants); `cam_pivot_height` is the world pivot height it derived from [`CameraPivot`] this /// frame. @@ -546,6 +637,7 @@ pub(super) fn seat_camera( cam_t: &mut Mut, collide: &benilla_world::collision::WorldCollision<'_, '_>, cam_probe: &Collider, + follow: &FollowInput, ) { // A keyboard turn carries the camera RIGIDLY (char and camera rotate as one — the reference // look, director's call closing 0050's open "camera follow on turn"): an eased chase of a @@ -554,12 +646,14 @@ pub(super) fn seat_camera( // — no INPUT-turn carry against the user's hand. (A transport deck's turn is not an input and // never arrives here — the ride block applies it to `cam.yaw` directly, drag or no drag.) // - // A left-drag orbit offset now **persists** once released: the vanilla `cameraSmoothStyle` - // auto-follow that eased the camera back behind the character while moving/turning is removed - // (director's call — we don't want it even though it's faithful). The camera stays where you - // put it; only a fresh drag, or a right-drag/movement that resyncs `face_yaw`, moves it. + // **The auto-follow** (decision 1493, 1.12's `cameraSmoothStyle`) rides the same gate for the + // same reason — a held drag owns the camera, hand on it: swing the camera back behind the + // character, along the shortest arc, at [`FOLLOW_RATE`]. Which frames it runs on is the + // player's setting — Always every frame, Smart only while the character is translating, Never + // not at all (the behaviour benilla shipped unconditionally until now). if rig.look.is_none() { cam.yaw += turn_delta; + cam.yaw += follow.step(cam.yaw, dt); } // Orient the camera, then orbit it behind the avatar's torso. The framing **pivot** is // `feet + cam_pivot_height` (model-derived, ~neck height — [`CameraPivot`]); the camera looks at @@ -1016,6 +1110,49 @@ mod tests { assert_eq!(slow.rate(), LOOK_SENSITIVITY * 0.5); } + /// **The auto-follow** (decision 1493) — 1.12's `cameraSmoothStyle`, the setting benilla spent + /// its whole life behaving as "Never". Four properties, and the last two are the ones a + /// re-derivation gets wrong: the style gate (Smart is *movement*-gated, Always is not, Never is + /// never), the rate cap, the fact that the chase takes the SHORT way round the ±π seam, and + /// that it stops exactly ON the facing rather than oscillating across it. + #[test] + fn the_auto_follow_chases_on_the_styles_that_chase_and_never_overshoots() { + const DT: f32 = 1.0 / 60.0; + let step = |style, moving, face_yaw, cam_yaw| { + FollowInput { + style, + face_yaw, + moving, + } + .step(cam_yaw, DT) + }; + + // Never is inert — the camera stays exactly where the hand left it, moving or not. + assert_eq!(step(FollowStyle::Never, true, 1.0, 0.0), 0.0); + // Smart is gated on the character actually translating ("except when your character is + // moving"); Always is not. + assert_eq!(step(FollowStyle::Smart, false, 1.0, 0.0), 0.0); + assert!(step(FollowStyle::Smart, true, 1.0, 0.0) > 0.0); + assert!(step(FollowStyle::Always, false, 1.0, 0.0) > 0.0); + + // A big gap slews at the rate, not in one frame. + let capped = step(FollowStyle::Always, true, 3.0, 0.0); + assert!((capped - FOLLOW_RATE * DT).abs() < 1.0e-6, "{capped}"); + // A gap smaller than one frame's slew lands ON the facing — no overshoot, and the next + // frame writes exactly nothing (which is what keeps the no-op transform gate quiet). + let small = 0.5 * FOLLOW_RATE * DT; + let landed = step(FollowStyle::Always, true, small, 0.0); + assert!((landed - small).abs() < 1.0e-6, "{landed} vs {small}"); + assert_eq!(step(FollowStyle::Always, true, 0.0, 0.0), 0.0); + + // Across the ±π seam the chase takes the short way: a facing at −3.1 from a camera at 3.1 + // is 0.08 rad AHEAD, not 6.2 rad behind. + assert!( + step(FollowStyle::Always, true, -3.1, 3.1) > 0.0, + "the shortest arc, not the long way round" + ); + } + /// The second arm: between 200 and 800 world the travel gate applies, per axis and independently /// (`0x514ae0`'s `both accums < 8.0` arm). The thresholds are the reference's 2.25° of yaw and /// 2.0° of pitch — see [`CLICK_HOLD_CEILING`] for why we hold the angle, not the raw literal. diff --git a/crates/benilla-app/src/ui_chat/commands.rs b/crates/benilla-app/src/ui_chat/commands.rs index d18c33c3..3500eb40 100644 --- a/crates/benilla-app/src/ui_chat/commands.rs +++ b/crates/benilla-app/src/ui_chat/commands.rs @@ -93,6 +93,13 @@ pub(crate) enum SlashIndex { /// `GlobalStrings.lua` has no `SLASH_RELOADUI`), so this alias is a literal in [`Self::build`] /// rather than data read off the chain, exactly like the ESC-menu AddOns window is ours (1197). ReloadUi, + /// `/errors` `/err` — **benilla's own addition** (decision 1495): opens the script error log, + /// which is ours because 1.12 has no such window to alias. The reference's whole answer to a + /// Lua fault is the `ScriptErrors` modal, which shows a burst's first message and remembers + /// nothing; B293 is two reporters asking for the list that modal cannot be. **Player-facing, + /// not a `DevCmd`**: the people who need it are the people running addons, and gating it on a + /// dev build would leave exactly the reporters who asked unable to type it. + ScriptErrors, } impl SlashIndex { @@ -137,13 +144,16 @@ impl SlashIndex { // No shipped alias string exists under this key (the walk finds nothing); the `/reload` // alias is inserted as a literal in `build` — see the variant's doc. Self::ReloadUi => "RELOADUI", + // No shipped `SLASH_BENILLASCRIPTERRORS1` exists — the aliases are literals in + // `build`. The key is still ours to name so the variant round-trips like any other. + Self::ScriptErrors => "BENILLASCRIPTERRORS", } } /// Every registered index — the registry proper. A reference command NOT in this list resolves /// nowhere and answers `HELP_TEXT_SIMPLE`, exactly as an unknown command does in the reference /// (better than a registered handler that silently does nothing). - const ALL: [Self; 36] = [ + const ALL: [Self; 37] = [ Self::Reply, Self::Join, Self::Leave, @@ -180,6 +190,7 @@ impl SlashIndex { Self::MacroHelp, Self::Console, Self::ReloadUi, + Self::ScriptErrors, ]; } @@ -297,6 +308,13 @@ impl SlashCommands { "reload", Command::Slash(SlashIndex::ReloadUi), ); + for alias in ["errors", "err"] { + insert( + &mut by_alias, + alias, + Command::Slash(SlashIndex::ScriptErrors), + ); + } let added_aliases = by_alias.len() - slash_aliases - emote_aliases; // 4 · benilla's instruments, last so they can never shadow a shipped command. diff --git a/crates/benilla-app/src/ui_chat/input/parse.rs b/crates/benilla-app/src/ui_chat/input/parse.rs index ed8296a9..c63154f2 100644 --- a/crates/benilla-app/src/ui_chat/input/parse.rs +++ b/crates/benilla-app/src/ui_chat/input/parse.rs @@ -400,6 +400,18 @@ fn slash_command(index: SlashIndex, args: &str) -> ParsedChat { // The console command match is case-insensitive like the engine's console; `0x4035f0` // reads no arguments, so anything after `reloadUI` is ignored rather than an error. S::ReloadUi => ParsedChat::ReloadUi, + // `/errors` (ours, 1495) → the script error log. Routed as Lua rather than a new + // `ParsedChat` variant for the same reason `/macro` is: the window is FrameXML, the + // toggle is a FrameXML function, and a Rust arm would only forward to it. + // `/errors clear` empties the log — the reporter's workflow is clear, reproduce, + // screenshot, and without it the shot carries a session's worth of unrelated rows. + S::ScriptErrors => ParsedChat::Lua { + body: if args.trim().eq_ignore_ascii_case("clear") { + "BenillaScriptLog_Clear()".into() + } else { + "BenillaScriptLog_Toggle()".into() + }, + }, S::Console => match args.split_whitespace().next() { Some(cmd) if cmd.eq_ignore_ascii_case("reloadui") => ParsedChat::ReloadUi, _ => ParsedChat::ConsoleUnknown { diff --git a/crates/benilla-app/src/ui_chat/tests.rs b/crates/benilla-app/src/ui_chat/tests.rs index d7b51ce6..1f614618 100644 --- a/crates/benilla-app/src/ui_chat/tests.rs +++ b/crates/benilla-app/src/ui_chat/tests.rs @@ -1368,8 +1368,11 @@ fn real_alias_table_resolves_the_shipped_commands() { // 1291 added CONSOLE's `/console` — one distinct alias, SLASH_CONSOLE1 and 2 are both the // same string). // - // The third number is benilla's own player-facing additions (1291): `/reload` — present in + // The third number is benilla's own player-facing additions: `/reload` (1291) and + // `/errors` `/err` (1495, the script error log) — 3 aliases over 2 commands. Present in // every build, deliberately counted apart from the shipped surface so the seam stays visible. + // The error log is player-facing on purpose and NOT an instrument: gating it on + // `dev_affordances()` would leave exactly the reporters who asked for it unable to type it. // // The fourth is the instrument **seam** (decision 1179): benilla's own instrument commands // (`/castvis` `/chattest` `/partytest` `/shot` `/liquid` `/reaction` `/react` — 7 aliases over 6 @@ -1383,7 +1386,7 @@ fn real_alias_table_resolves_the_shipped_commands() { }; assert_eq!( table.counts(), - (68, 225, 1, instruments), + (68, 225, 3, instruments), "(slash, emote, benilla addition, instrument) aliases" ); } diff --git a/crates/benilla-app/src/ui_script/addons.rs b/crates/benilla-app/src/ui_script/addons.rs index 0291c4d9..a11cb5a4 100644 --- a/crates/benilla-app/src/ui_script/addons.rs +++ b/crates/benilla-app/src/ui_script/addons.rs @@ -196,6 +196,10 @@ impl Addon { Source::Builtin => error!("ui_script: {e}"), Source::Dir(_) => warn!("ui_script: {e}"), } + // Retained where the player can read it (1495). This is the single commonest way + // an addon "doesn't work" with nothing on screen — the director's own AtlasLoot + // copy lists `Bossnames\BossNames.xml` and ships no such folder. + script.report_load_failure(&e); failures.push(e); continue; }; @@ -225,6 +229,10 @@ impl Addon { Err(e) => { let e = format!("{}/{file}: {e}", self.name); error!("ui_script: parsing {e}"); + // Still log-only as far as the *dialog* goes — the reference answers an + // unparseable document with a FrameXML.log line and silence, and 1495 does not + // change that. What it changes is that the silence is no longer total. + script.report_load_failure(&e); failures.push(e); continue; } @@ -808,6 +816,7 @@ impl Walk { let chain = self.loading.join(" → "); let e = format!("{key}: dependency cycle ({chain} → {key})"); error!("ui_script: {e}"); + script.report_load_failure(&e); self.failures.push(e); self.failed.insert(addon.name.clone()); return Err(()); @@ -835,6 +844,9 @@ impl Walk { addon.name ); error!("ui_script: {e}"); + // The one failure a player can usually FIX themselves — install the dependency — and + // until 1495 the only place it was said was the terminal. + script.report_load_failure(&e); self.failures.push(e); self.failed.insert(addon.name.clone()); return Err(()); @@ -852,6 +864,7 @@ impl Walk { Err(e) => { let e = format!("{}/Bindings.xml: {e}", addon.name); error!("ui_script: {e}"); + script.report_load_failure(&e); self.failures.push(e); } } diff --git a/crates/benilla-app/src/ui_script/bag_tests.rs b/crates/benilla-app/src/ui_script/bag_tests.rs index 2f17dffc..569d1c83 100644 --- a/crates/benilla-app/src/ui_script/bag_tests.rs +++ b/crates/benilla-app/src/ui_script/bag_tests.rs @@ -115,12 +115,19 @@ fn backpack_toggle_plays_open_and_close_kits() { assert!(s.errors().is_empty(), "script errors: {:?}", s.errors()); } -/// The 'B' binding / backpack button opens or closes EVERY bag at once (ref ToggleBackpack), not -/// just the backpack: an equipped bag (here bag 2) opens alongside it, while empty bag slots (1,3,4 -/// — no container, GetContainerNumSlots == 0) stay shut. A second toggle closes them all. And ESC's -/// CloseAllWindows must sweep all of them, not only the backpack. +/// **B / the backpack button open the BACKPACK ALONE** (ref ToggleBackpack, ContainerFrame.lua +/// l.67-82) — the director's report, and the fix in 1494. Until then both ran an all-bags toggle +/// and an equipped bag came up beside the backpack every time. +/// +/// The reference's shape is deliberately asymmetric, and all three arms are pinned here: +/// * shut → `ToggleBag(0)`: bag 0 and nothing else, however many bags are equipped; +/// * bag 0 OPEN → hide every container window there is (the close arm is the all-bags one); +/// * bag 0 shut but ANOTHER bag open → the condition reads bag 0 specifically, so this is still +/// the open arm: the backpack joins the open bag rather than closing it. +/// +/// ESC's CloseAllWindows must still sweep all of them. #[test] -fn b_toggles_all_bags_open_and_closed() { +fn b_opens_the_backpack_alone_and_closes_every_bag() { let mut s = UiScript::new().unwrap(); s.set_screen_size(1024.0, 768.0); load_xml(&s, "Fonts.xml"); @@ -151,40 +158,141 @@ fn b_toggles_all_bags_open_and_closed() { let shown = |s: &mut UiScript, name: &str| s.eval::(&format!("return {name}:IsShown()")).unwrap(); - // Toggle open: backpack + bag 2 open; the empty slots do not. + // Toggle open: the BACKPACK, and only the backpack — bag 2 is equipped and stays shut. s.run("BenillaBagToggle_OnClick()").unwrap(); let _ = s.take_sounds(); assert!(shown(&mut s, "BenillaBagFrame"), "backpack opens"); assert!( - shown(&mut s, "BenillaBagFrame2"), - "the equipped bag opens too" + !shown(&mut s, "BenillaBagFrame2"), + "the equipped bag stays shut — this is the backpack's own toggle, not open-all" ); + assert!( + !shown(&mut s, "BenillaBagFrame1") + && !shown(&mut s, "BenillaBagFrame3") + && !shown(&mut s, "BenillaBagFrame4"), + "and the empty slots have no window to show either way" + ); + + // The close arm IS the all-bags one: with bag 2 also open by some other path, one toggle + // takes the lot down. + s.run("ToggleBag(2)").unwrap(); + assert!(shown(&mut s, "BenillaBagFrame2")); + s.run("BenillaBagToggle_OnClick()").unwrap(); + let _ = s.take_sounds(); + assert!( + !shown(&mut s, "BenillaBagFrame") && !shown(&mut s, "BenillaBagFrame2"), + "bag 0 open ⇒ the toggle hides every container window" + ); + + // Bag 0 shut, bag 2 open: the ref's condition reads bag 0, so this is the OPEN arm — the + // backpack joins the bag already up instead of closing it. + s.run("ToggleBag(2)").unwrap(); + s.run("BenillaBagToggle_OnClick()").unwrap(); + let _ = s.take_sounds(); + assert!( + shown(&mut s, "BenillaBagFrame") && shown(&mut s, "BenillaBagFrame2"), + "with only another bag open, B opens the backpack beside it" + ); + + // ESC's CloseAllWindows sweeps every open bag, not just the backpack. + s.run("CloseAllWindows()").unwrap(); + assert!( + !shown(&mut s, "BenillaBagFrame") && !shown(&mut s, "BenillaBagFrame2"), + "CloseAllWindows hides every bag window" + ); + assert!(s.errors().is_empty(), "script errors: {:?}", s.errors()); +} + +/// **SHIFT-B / a shift-click on the backpack button open ALL of them** (ref OpenAllBags, +/// ContainerFrame.lua l.662-700) — the other half of the split 1494 restored. It is a TOGGLE, and +/// its arms are decided by a COUNT, not by "is anything open": +/// * anything less than everything open → open the lot (the backpack plus each equipped bag); +/// * everything already open → the counting pass IS the close; +/// * `forceOpen` → always the open arm (what a vendor window would pass). +/// +/// The keyring is swept by the counting pass but never counted and never opened — the reference's +/// own `GetID() ~= KEYRING_CONTAINER` exclusion. +#[test] +fn shift_b_toggles_every_bag_at_once() { + let mut s = UiScript::new().unwrap(); + s.set_screen_size(1024.0, 768.0); + load_xml(&s, "Fonts.xml"); + load_xml(&s, "UiPanels.xml"); + load_xml(&s, "MerchantFrame.xml"); + load_xml(&s, "Cooldown.xml"); + load_xml(&s, "BagFrame.xml"); + s.set_money(0); + s.set_container( + 0, + Some(ContainerState { + name: Some("Backpack".into()), + num_slots: 16, + slots: std::collections::HashMap::new(), + }), + ); + s.set_container( + 2, + Some(ContainerState { + name: Some("Small Pouch".into()), + num_slots: 6, + slots: std::collections::HashMap::new(), + }), + ); + let shown = + |s: &mut UiScript, name: &str| s.eval::(&format!("return {name}:IsShown()")).unwrap(); + + // From nothing open: the backpack AND the equipped bag; the empty slots have no window. + s.run("OpenAllBags()").unwrap(); + let _ = s.take_sounds(); + assert!(shown(&mut s, "BenillaBagFrame") && shown(&mut s, "BenillaBagFrame2")); assert!( !shown(&mut s, "BenillaBagFrame1") && !shown(&mut s, "BenillaBagFrame3") && !shown(&mut s, "BenillaBagFrame4"), "empty bag slots (no container) have no window to show" ); + assert!( + !shown(&mut s, "BenillaKeyRingFrame"), + "the keyring is not one of your bags — open-all never opens it" + ); - // Toggle again: every open bag closes. - s.run("BenillaBagToggle_OnClick()").unwrap(); + // Everything open ⇒ the next one closes the lot. + s.run("OpenAllBags()").unwrap(); + let _ = s.take_sounds(); + assert!(!shown(&mut s, "BenillaBagFrame") && !shown(&mut s, "BenillaBagFrame2")); + + // PARTIAL is the open arm, not the close arm — the count is what decides. Backpack alone up + // (1 of 2) ⇒ open-all still opens rather than closing what is there. + s.run("ToggleBag(0)").unwrap(); + s.run("OpenAllBags()").unwrap(); + let _ = s.take_sounds(); + assert!( + shown(&mut s, "BenillaBagFrame") && shown(&mut s, "BenillaBagFrame2"), + "1 of 2 open is not 'all open': the lot opens" + ); + + // An open keyring rides the sweep down without ever counting toward "all open" — so this + // still reads 2-of-2 and closes. + s.run("ToggleKeyRing()").unwrap(); + assert!(shown(&mut s, "BenillaKeyRingFrame")); + s.run("OpenAllBags()").unwrap(); let _ = s.take_sounds(); assert!( !shown(&mut s, "BenillaBagFrame") && !shown(&mut s, "BenillaBagFrame2"), - "the second toggle closes them all" + "the keyring does not count as an open bag" + ); + assert!( + !shown(&mut s, "BenillaKeyRingFrame"), + "…but it IS swept by the same pass" ); - // ESC's CloseAllWindows sweeps every open bag, not just the backpack. - s.run("BenillaBagToggle_OnClick()").unwrap(); + // forceOpen skips the close arm: from all-open, everything stays open. + s.run("OpenAllBags(1)").unwrap(); + s.run("OpenAllBags(1)").unwrap(); let _ = s.take_sounds(); assert!( - shown(&mut s, "BenillaBagFrame2"), - "reopened for the ESC check" - ); - s.run("CloseAllWindows()").unwrap(); - assert!( - !shown(&mut s, "BenillaBagFrame") && !shown(&mut s, "BenillaBagFrame2"), - "CloseAllWindows hides every bag window" + shown(&mut s, "BenillaBagFrame") && shown(&mut s, "BenillaBagFrame2"), + "forceOpen means open, never toggle" ); assert!(s.errors().is_empty(), "script errors: {:?}", s.errors()); } @@ -223,8 +331,9 @@ fn bag_bar_buttons_light_while_their_bag_is_open() { .unwrap() }; - // Open-all: the backpack button and the equipped bag's slot light; the empty slots don't. - s.run("BenillaBagToggle_OnClick()").unwrap(); + // Open-all (SHIFT-B's knob since 1494 — the backpack button alone would light only its own + // ring now): the backpack button and the equipped bag's slot light; the empty slots don't. + s.run("OpenAllBags()").unwrap(); assert!( checked(&mut s, "MainMenuBarBackpackButton"), "backpack ring lights" @@ -1944,7 +2053,7 @@ fn an_addon_that_hooks_toggle_backpack_receives_the_click() { ) .unwrap(); - // The button's own OnClick path — what the 'B' binding and the backpack button both run. + // The button's own OnClick path — what the B binding and a plain backpack-button click run. s.run("BenillaBagToggle_OnClick()").unwrap(); assert_eq!( @@ -2022,13 +2131,15 @@ fn the_bag_slots_carry_the_references_names_and_icon_names() { /// The backpack button's checked law after the stated divergence (BenillaBagToggle_OnClick's /// comment): checked belongs to the WINDOWS' OnShow/OnHide writes, and the button's XML click -/// wrapper only undoes the CheckButton widget's pre-handler flip. The killer case is the click -/// that never touches the backpack window — backpack shut, another bag's window open → -/// ToggleBackpack sees an open window and CLOSES ALL; the backpack never transitions, nothing -/// writes its button, and only the wrapper's undo keeps the widget flip from leaving it -/// lit-while-shut (the job the ref's native-window scan tail used to do). +/// wrapper only undoes the CheckButton widget's pre-handler flip. +/// +/// Driven through REAL input (the widget flip fires only on real clicks) over the case 1494 +/// changed: backpack shut with another bag's window open. That used to be the close-all arm — +/// nothing transitioned the backpack and only the undo kept the button from ending lit-while-shut. +/// It is now the OPEN arm, so the pin runs the whole cycle: the click opens bag 0 beside bag 1 and +/// its OnShow lights the button, the next click closes everything and the OnHide clears it. #[test] -fn a_close_others_click_leaves_the_backpack_button_dark() { +fn the_backpack_buttons_ring_follows_its_own_window_through_real_clicks() { let mut s = UiScript::new().unwrap(); s.set_screen_size(1024.0, 768.0); for file in [ @@ -2070,22 +2181,31 @@ fn a_close_others_click_leaves_the_backpack_button_dark() { s.mouse_move(r[0], r[1]); s.mouse_button(r[0], r[1], "LeftButton", true); s.mouse_button(r[0], r[1], "LeftButton", false); + assert!( + s.eval::("return BenillaBagFrame:IsShown()").unwrap(), + "the click opens the backpack (ToggleBackpack's open arm reads bag 0, not 'anything open')" + ); + assert!( + s.eval::("return BenillaBagFrame1:IsShown()").unwrap(), + "…and leaves the bag that was already up alone" + ); + assert!( + checked(&mut s), + "open ⇒ lit, written by the window's OnShow over the widget flip" + ); + + // The second real click: bag 0 is open now, so this is the close-all arm — both windows go, + // and the OnHide write clears the ring. + s.mouse_button(r[0], r[1], "LeftButton", true); + s.mouse_button(r[0], r[1], "LeftButton", false); + assert!(!s.eval::("return BenillaBagFrame:IsShown()").unwrap()); assert!( !s.eval::("return BenillaBagFrame1:IsShown()").unwrap(), - "the click closes the other bag's window (ToggleBackpack's close-all arm)" + "the close arm takes every container down" ); assert!( !checked(&mut s), - "no backpack transition happened, so the button must stay dark — the widget flip undone" - ); - - // The second real click opens the backpack (plus the equipped bag): its OnShow writes the light. - s.mouse_button(r[0], r[1], "LeftButton", true); - s.mouse_button(r[0], r[1], "LeftButton", false); - assert!(s.eval::("return BenillaBagFrame:IsShown()").unwrap()); - assert!( - checked(&mut s), - "open ⇒ lit, written by the window's OnShow" + "shut ⇒ dark, written by the window's OnHide" ); assert!(s.errors().is_empty(), "script errors: {:?}", s.errors()); } diff --git a/crates/benilla-app/src/ui_script/lifecycle.rs b/crates/benilla-app/src/ui_script/lifecycle.rs index 52d2b608..e1b3c3f3 100644 --- a/crates/benilla-app/src/ui_script/lifecycle.rs +++ b/crates/benilla-app/src/ui_script/lifecycle.rs @@ -273,6 +273,35 @@ pub(crate) fn load_ingame_ui_on_world_entry(world: &mut World) { // (`AddOn_Load 0x51f240` steps 2 → 4 → 6, decision 1128); reversing it means the defaults // always win and nothing can ever be remembered. finish_ui_load(&mut script); + // **Say it out loud when an addon didn't load** (decision 1495). Every failure the walk found + // is retained now, but a log nobody knows to open does not fix silence — and silence is the + // actual defect B293 reports: *"there are a lot of addons that still doesn't work"*, with + // nothing on screen to say which or why. Counted off the retained log rather than the walk's + // `failures` vec so the number matches what `/errors` will show: the log deduplicates, and one + // broken addon that fails four files should not read as four broken addons. The VM is fresh + // per world entry, so every `Load` row here is this load's. + // Placed AFTER `finish_ui_load` so the count covers the WHOLE load edge — the saved-variables + // chunks and `ADDON_LOADED`/`VARIABLES_LOADED` handlers it runs are part of loading, and an + // addon that dies in one of them is as absent as one whose file was missing. + let failed = script + .diagnostics() + .iter() + .filter(|d| d.kind == benilla_ui::script::diagnostics::DiagnosticKind::Load) + .count(); + if failed > 0 { + if let Some(mut chat) = world.get_resource_mut::() { + // Queued, not drawn: `ChatLog` is a pending buffer the chat feed drains once the UI is + // up, which is what makes it safe to push from inside the world-entry load edge. + chat.push_event(crate::ui_chat::ChatEvent::text_only( + crate::ui_chat::ChatEventKind::System, + format!( + "{failed} addon load {} — type /errors to see {}.", + if failed == 1 { "failure" } else { "failures" }, + if failed == 1 { "it" } else { "them" } + ), + )); + } + } // The load edge is over: disarm the instruction bound `load_ingame_ui` installed (decision // 1306). From here every OnUpdate and event handler runs unhooked — a session must not kill // a player's addon for being slow; only a load that never returns is fair game. diff --git a/crates/benilla-app/src/ui_script/options_tests.rs b/crates/benilla-app/src/ui_script/options_tests.rs index d891c3d8..c93b2596 100644 --- a/crates/benilla-app/src/ui_script/options_tests.rs +++ b/crates/benilla-app/src/ui_script/options_tests.rs @@ -1769,13 +1769,15 @@ fn every_row_tooltip_key_resolves_in_the_real_global_strings() { ); checked += 1; } - // 20 CVar rows (Social's two bubble switches are 1139's; Status Bar Text, Mouse Sensitivity - // and Max Camera Distance 1140's; Graphics' Vertical Sync is 1394's) + the Combat page's 14 - // saved-variable rows (1134) + the Interface page's 4 (3 from 1136, Buff Durations 1139) and - // the Action Bars page's 1 (1136) + the Interface page's 2 API rows (Show Cloak / Show Helm, - // 1472) — which is the point of counting here rather than per page: the third store's rows are - // held to the same "the key is 1.12's own and it resolves" bar as the other two. - assert_eq!(checked, 41, "every row but one carries a live 1.12 key"); + // 21 CVar rows (Social's two bubble switches are 1139's; Status Bar Text, Mouse Sensitivity + // and Max Camera Distance 1140's; Graphics' Vertical Sync is 1394's; Camera Following Style + // 1493's) + the Combat page's 14 saved-variable rows (1134) + the Interface page's 4 (3 from + // 1136, Buff Durations 1139) and the Action Bars page's 1 (1136) + the Interface page's 2 API + // rows (Show Cloak / Show Helm, 1472) — which is the point of counting here rather than per + // page: the third store's rows are held to the same "the key is 1.12's own and it resolves" + // bar as the other two. Camera Following Style is counted on the key it wears at rest (Smart's + // OPTION_TOOLTIP_CAMERA1); the other two ride the same census as the selection moves. + assert_eq!(checked, 42, "every row but one carries a live 1.12 key"); assert_eq!( untipped, vec!["ControlsRowAutoLoot".to_string()], @@ -1864,11 +1866,12 @@ fn every_flavor_of_row_raises_its_plate_from_the_page_it_lives_on() { s.errors() ); } - // 20 of the 21 CVar rows (Social's two are 1139's; Status Bar Text, Mouse Sensitivity and - // Max Camera Distance 1140's; Vertical Sync 1394's), plus the Combat page's 14 saved-variable - // rows (1134), the Interface page's 4 (1136, + Buff Durations 1139), Action Bars' 1 (1136) and - // the Interface page's 2 API rows (Show Cloak / Show Helm, 1472). - assert_eq!(raised, 41, "every row but Auto Loot has a 1.12 description"); + // 21 of the 22 CVar rows (Social's two are 1139's; Status Bar Text, Mouse Sensitivity and + // Max Camera Distance 1140's; Vertical Sync 1394's; Camera Following Style 1493's), plus the + // Combat page's 14 saved-variable rows (1134), the Interface page's 4 (1136, + Buff Durations + // 1139), Action Bars' 1 (1136) and the Interface page's 2 API rows (Show Cloak / Show Helm, + // 1472). + assert_eq!(raised, 42, "every row but Auto Loot has a 1.12 description"); } /// The **Combat page** (decision 1134) — the first rows in this window whose store is a @@ -2799,3 +2802,111 @@ fn the_max_camera_distance_slider_stores_a_factor_and_reads_out_yards() { ); assert!(s.errors().is_empty(), "script errors: {:?}", s.errors()); } + +/// **Camera Following Style** (decision 1493) — 1.12's `cameraSmoothStyle`, worn as a Controls-page +/// dropdown, and the setting that decides whether the camera swings back behind the character at +/// all. Three things pinned: the reference's own values *and* their order (Smart `"1"`, Always +/// `"2"`, Never `"3"` — Never is **3**, not 0, which is exactly what a tidy-up would get wrong); +/// the registered default landing on Smart, where benilla behaved as Never for its whole life +/// before this row; and the plate following the SELECTION rather than the row — the reference's own +/// dropdown repaints its tooltip per entry (`OPTION_TOOLTIP_CAMERA1/2/3`) and this is the first row +/// here whose description is not a property of the row alone. +#[test] +fn the_camera_following_style_dropdown_carries_the_reference_values_and_plate() { + let mut s = harness_on(audio_harness()); + s.run("ShowUIPanel(OptionsFrame)").unwrap(); + s.run("OptionsFrameCategoryListRowControls:Click()") + .unwrap(); + + // The registrar default is the reference's, and it is what the director asked to ship. + assert_eq!( + s.eval::( + "return OptionsFrameContainerBodyControlsRowCameraFollowStyleDropdownText:GetText()" + ) + .unwrap(), + "Smart" + ); + assert_eq!( + s.eval::( + "return OptionsFrameContainerBodyControlsRowCameraFollowStyleLabel:GetText()" + ) + .unwrap(), + "Camera Following Style" + ); + assert_eq!( + s.eval::("return OptionsFrameContainerBodyControlsRowCameraFollowStyle.tip") + .unwrap(), + "OPTION_TOOLTIP_CAMERA1" + ); + assert!( + s.take_cvar_changes().is_empty(), + "reading the table on select must not write it back" + ); + + // The list is the reference dropdown's, entry for entry: Smart, Always, Never — with the + // stored value checked. + s.run("OptionsFrameContainerBodyControlsRowCameraFollowStyleDropdownButton:Click()") + .unwrap(); + assert_eq!( + s.eval::("return DropDownList1.numButtons").unwrap(), + 3.0 + ); + assert_eq!( + s.eval::( + "return DropDownList1Button1:GetText() .. \",\" .. DropDownList1Button2:GetText() \ + .. \",\" .. DropDownList1Button3:GetText()" + ) + .unwrap(), + "Smart,Always,Never" + ); + assert!(s + .eval::("return DropDownList1Button1Check:IsVisible()") + .unwrap()); + + // Never stores "3" — the value the reference writes, not the 0 a fresh enum would invent — + // and the row's plate becomes Never's own description. + s.run("DropDownList1Button3:Click()").unwrap(); + assert_eq!( + s.take_cvar_changes(), + vec![("cameraSmoothStyle".to_string(), "3".to_string())] + ); + assert_eq!( + s.eval::( + "return OptionsFrameContainerBodyControlsRowCameraFollowStyleDropdownText:GetText()" + ) + .unwrap(), + "Never" + ); + assert_eq!( + s.eval::("return OptionsFrameContainerBodyControlsRowCameraFollowStyle.tip") + .unwrap(), + "OPTION_TOOLTIP_CAMERA3", + "the plate follows the selection, like the reference dropdown's own" + ); + + // Always is the middle entry and stores "2" (the pairing that goes wrong if the order is read + // off the alphabet instead of off the reference). + s.run("OptionsFrameContainerBodyControlsRowCameraFollowStyleDropdownButton:Click()") + .unwrap(); + s.run("DropDownList1Button2:Click()").unwrap(); + assert_eq!( + s.take_cvar_changes(), + vec![("cameraSmoothStyle".to_string(), "2".to_string())] + ); + + // Defaults walks it back to Smart. + s.run("OptionsFrameContainerDefaults:Click()").unwrap(); + assert_eq!( + s.eval::("return GetCVar(\"cameraSmoothStyle\")") + .unwrap(), + "1" + ); + assert_eq!( + s.eval::( + "return OptionsFrameContainerBodyControlsRowCameraFollowStyleDropdownText:GetText()" + ) + .unwrap(), + "Smart" + ); + assert!(s.errors().is_empty(), "script errors: {:?}", s.errors()); +} diff --git a/crates/benilla-app/src/ui_script/skills_frame_tests.rs b/crates/benilla-app/src/ui_script/skills_frame_tests.rs index 37218385..67544244 100644 --- a/crates/benilla-app/src/ui_script/skills_frame_tests.rs +++ b/crates/benilla-app/src/ui_script/skills_frame_tests.rs @@ -297,3 +297,169 @@ fn a_real_hunters_block_lists_exactly_what_the_reference_client_lists() { "Beast Mastery's skillMaxRank" ); } + +/// The page's own **CLOSE** button (decision 1496). `SkillFrameCancelButton` is live in the +/// reference — the XML comment that swallows its `SkillFrameAcceptButton` neighbour closes one +/// line above it (ref `SkillFrame.xml` l.337/339) — and the director's screenshot of an empty +/// bottom-right seat is what that misread cost. Pins the button's existence, the ref's own seat +/// (80x22 centred on the page's TOPLEFT + (305,-422)), and that it closes the window. +#[test] +fn the_pages_close_button_sits_where_the_reference_seats_it_and_closes_the_window() { + let mut s = shown_skills_page(); + + assert!( + s.eval::("return SkillFrameCancelButton ~= nil") + .unwrap(), + "the ref's SkillFrameCancelButton is built" + ); + assert!( + s.eval::("return SkillFrameCancelButton:IsVisible()") + .unwrap(), + "and shown — nothing in the ref's SkillFrame.lua ever hides it" + ); + + // The ref's geometry, read page-relative so the assertion is the ref's own numbers. + let (page_top, page_left) = s + .eval::<(f64, f64)>("return SkillFrame:GetTop(), SkillFrame:GetLeft()") + .unwrap(); + let (top, bottom, left, right) = s + .eval::<(f64, f64, f64, f64)>( + "local b = SkillFrameCancelButton \ + return b:GetTop(), b:GetBottom(), b:GetLeft(), b:GetRight()", + ) + .unwrap(); + assert_eq!( + ( + (left + right) / 2.0 - page_left, + (top + bottom) / 2.0 - page_top, + right - left, + top - bottom, + ), + (305.0, -422.0, 80.0, 22.0), + "CENTER of the page's TOPLEFT at (305,-422), 80x22 (ref l.339-348)" + ); + + // Its label is the CLOSE global string's seat, in the panel-button gold. + let label = s + .extract() + .iter() + .find_map(|q| match &q.content { + QuadContent::Text { + text: Some(t), + color, + .. + } if t == "CLOSE" => Some(*color), + _ => None, + }) + .expect("the CLOSE label draws"); + assert_eq!( + label, + Some([1.0, 0.82, 0.0, 1.0]), + "GameFontNormal — the UIPanelButtonTemplate face's own normal font" + ); + + // And it does what the ref's OnClick does: page down, window down. + s.run("SkillFrameCancelButton:Click()").unwrap(); + s.resolve(); + assert!( + !s.eval::("return SkillFrame:IsVisible()").unwrap(), + "the page hides" + ); + assert!( + !s.eval::("return CharacterFrame:IsVisible()").unwrap(), + "and the window with it (ref l.352-355)" + ); + assert!(s.errors().is_empty(), "errors: {:?}", s.errors()); +} + +/// The **ALL** fold's face and seat (decision 1496). Both halves of the director's report: the +/// label is the row font — `GameFontHighlight`, WHITE at 12 — not `GameFontNormalSmall`'s yellow +/// 10, and the button rides 3px BELOW the tab cap's centre (the ref's own `(-3,-3)` off the left +/// cap), not 3px above it, which is where the offset copied from `TrainerFrame.xml` put it. +#[test] +fn the_collapse_all_fold_wears_the_row_font_and_the_references_seat() { + let s = shown_skills_page(); + + let label = s + .extract() + .iter() + .find_map(|q| match &q.content { + QuadContent::Text { + text: Some(t), + color, + font_height, + .. + } if t == "ALL" => Some((*color, *font_height)), + _ => None, + }) + .expect("the ALL label draws"); + assert_eq!( + label, + (Some([1.0, 1.0, 1.0, 1.0]), Some(12.0)), + "GameFontHighlight — the SAME face every SkillTypeLabel row wears" + ); + + let (tab_top, tab_bottom) = s + .eval::<(f64, f64)>("return SkillExpandTabLeft:GetTop(), SkillExpandTabLeft:GetBottom()") + .unwrap(); + let (btn_top, btn_bottom, btn_left) = s + .eval::<(f64, f64, f64)>( + "local b = SkillCollapseAllButton \ + return b:GetTop(), b:GetBottom(), b:GetLeft()", + ) + .unwrap(); + assert_eq!( + (tab_top + tab_bottom) / 2.0 - (btn_top + btn_bottom) / 2.0, + 3.0, + "the fold sits 3px BELOW the cap's centre (ref's -3); +3 above was the bug" + ); + let cap_right = s + .eval::("return SkillExpandTabLeft:GetRight()") + .unwrap(); + assert_eq!( + btn_left - cap_right, + -3.0, + "and 3px back over the cap (ref's -3 on x)" + ); + assert!(s.errors().is_empty(), "errors: {:?}", s.errors()); +} + +/// The tab's **fit law** (ref `SkillFrameExpandButtonFrame`'s OnLoad, l.312-316: +/// `SetWidth(GetTextWidth()+45)`). benilla seats the VM's font engine at the frame boundary, so +/// the XML-load call reads 0 — the guard leaves the declared width standing, and the first Update +/// with a measurer fits the tab. Both states are pinned here because only the second one is the +/// reference's, and only the first is what a cold load sees. +#[test] +fn the_expand_tab_fits_its_label_once_a_measure_answers() { + let mut s = shown_skills_page(); + assert_eq!( + s.eval::("return SkillExpandButtonFrame:GetWidth()") + .unwrap(), + 54.0, + "unmeasured, the declared width stands — a 0 measure must not squash the tab to 45" + ); + + s.set_text_measurer(Box::new(super::FixedWidthFont(7.0))); + s.run("BenillaSkillFrame_Update()").unwrap(); + s.resolve(); + assert_eq!( + s.eval::("return SkillExpandButtonFrame:GetWidth()") + .unwrap(), + 7.0 * 3.0 + 45.0, + "then the ref's own law: the label's width + 45" + ); + // The middle slab is the span between the two caps, so the fit reaches the art for free. + let (mid_l, mid_r, cap_r_l) = s + .eval::<(f64, f64, f64)>( + "return SkillExpandTabMiddle:GetLeft(), SkillExpandTabMiddle:GetRight(), \ + SkillExpandTabRight:GetLeft()", + ) + .unwrap(); + assert_eq!(mid_r, cap_r_l, "the middle stretches to the right cap"); + assert_eq!( + mid_r - mid_l, + 66.0 - 16.0, + "and carries the whole span minus the two caps" + ); + assert!(s.errors().is_empty(), "errors: {:?}", s.errors()); +} diff --git a/crates/benilla-app/src/ui_script/world_entry_tests.rs b/crates/benilla-app/src/ui_script/world_entry_tests.rs index 896508be..bb55fd8c 100644 --- a/crates/benilla-app/src/ui_script/world_entry_tests.rs +++ b/crates/benilla-app/src/ui_script/world_entry_tests.rs @@ -772,3 +772,181 @@ fn the_login_one_shots_wait_for_the_in_game_ui() { drop(app); let _ = std::fs::remove_dir_all(&tmp); } + +/// **B293's headline, at the edge that produces it** (decision 1495). An addon that fails to load +/// *without raising* — the commonest shape by far, a `.toc` naming a file the package does not +/// ship — used to `warn!` to the terminal and vanish. Nothing raised, so 1305's dialog could not +/// fire; the walk's failure list was dropped on the floor at `load_ingame_ui_on_world_entry`; and +/// the per-frame drain kept no history. From the player's chair the addon simply was not there and +/// the client said nothing, which is the literal content of *"there are a lot of addons that still +/// doesn't work"*. +/// +/// Three claims, and the third is the one that makes the first two reachable: the failure is +/// **retained**, it is **readable from Lua** (so the window is a view of it, not a second copy), +/// and the player is **told to look**. +#[test] +fn an_addon_that_fails_to_load_without_raising_is_readable_in_the_error_log() { + let _l = ENV_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let (tmp, _c, _h) = hermetic_probe("addon-missing-file"); + // Alphabetically first, so the probe behind it also proves this class drops only itself. + let dir = tmp.join("benilla-config/AddOns/AaMissing"); + std::fs::create_dir_all(&dir).expect("addon dir"); + std::fs::write( + dir.join("AaMissing.toc"), + "## Interface: 11200\nBossnames\\BossNames.xml\n", + ) + .expect("toc"); + // …and no such file is written. This is the director's own AtlasLoot copy, reduced. + let mut world = booted_world(); + world.init_resource::(); + + log_in_as(&mut world, "Onehunter", 1); + + assert_eq!( + probe_saw(&world).as_deref(), + Some("Onehunter"), + "the addon after the broken one still loads" + ); + + let script = world + .get_non_send_resource::() + .expect("VM"); + + // 1 · retained, and tagged as a LOAD failure — the kind that means "this addon is not running". + let rows = script.diagnostics(); + let row = rows + .iter() + .find(|d| d.message.contains("AaMissing")) + .unwrap_or_else(|| panic!("the missing file is in the log; got {rows:#?}")); + assert_eq!( + row.kind, + benilla_ui::script::diagnostics::DiagnosticKind::Load + ); + assert!( + row.message.contains("not found"), + "the row says what went wrong, verbatim as the terminal line: {:?}", + row.message + ); + + // 2 · readable from Lua by the same three reads the window uses. + let count: i64 = script + .eval("local shown = BenillaGetNumScriptErrors() return shown") + .expect("BenillaGetNumScriptErrors is installed"); + assert!(count >= 1, "the window's own read sees it"); + let seen: String = script + .eval( + "local text = '' \ + for i = 1, BenillaGetNumScriptErrors() do \ + local seq, kind, message = BenillaGetScriptErrorInfo(i) \ + if kind == 'load' then text = message end \ + end \ + return text", + ) + .expect("BenillaGetScriptErrorInfo is installed"); + assert!( + seen.contains("AaMissing"), + "the window walks the log and finds it: {seen:?}" + ); + + // …and the window itself materialized, so `/errors` has something to toggle. + assert!( + script + .eval::("return BenillaScriptLogFrame ~= nil") + .expect("eval"), + "ScriptLogFrame.xml loaded and built the window" + ); + + // **The repaint actually runs, over a log that has a row in it.** Asserting the globals exist + // would prove nothing: this drives the real path — `FauxScrollFrame_Update`, the row rebind, + // `strfind`/`strsub`/`strlen`/`format`, `SetTextColor`, the highlight seat and the detail + // pane — and a `nil` global anywhere in it raises here instead of on the player's first + // `/errors` (which is exactly the shape of failure this whole record exists to stop shipping). + script + .eval::<()>("BenillaScriptLog_Update() return nil") + .expect("the window repaints over a real log without raising"); + let row_label: String = script + .eval::>("return BenillaScriptLogRow1Label:GetText()") + .expect("eval") + .unwrap_or_default(); + assert!( + row_label.contains("AaMissing"), + "row 1 shows the failure, trimmed to the row's width: {row_label:?}" + ); + let summary: String = script + .eval::>("return BenillaScriptLogSummary:GetText()") + .expect("eval") + .unwrap_or_default(); + assert!( + summary.contains("error"), + "the summary line counted them: {summary:?}" + ); + + // 3 · **this class deliberately does NOT seize the screen.** Nothing raised; the reference's + // answer to an unparseable/absent document is a log line and silence, and 1495 keeps that. + // What it changes is that the silence is no longer total — hence the chat notice below. + assert!( + !script + .eval::("return ScriptErrors:IsVisible()") + .expect("ScriptErrors exists"), + "a non-raising load failure must not pop the red dialog — that would put non-errors \ + through `_ERRORMESSAGE` and through every addon handler that replaces it" + ); + + // 4 · the player is told to look. Without this the log is a room nobody knows about, and + // silence — not the missing list — is the actual defect B293 reports. + assert_eq!( + world.resource::().pending_len(), + 1, + "world entry queued the 'N addon load failures — type /errors' line" + ); + + drop(world); + let _ = std::fs::remove_dir_all(&tmp); +} + +/// **The burst 1305 measured, as the log sees it.** An `OnUpdate` that raises every frame produced +/// 470–1113 collected errors in 1305's own runs, of which `_ERRORMESSAGE` shows the **first** and +/// the per-frame drain keeps none. Deduplication is what makes a log survive that: one row with a +/// count, not 1,113 rows and not a truncated window of the last few. +#[test] +fn a_repeating_error_is_one_row_with_a_count_not_a_flood() { + let _l = ENV_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let (tmp, _c, _h) = hermetic_probe("addon-repeat"); + let mut world = booted_world(); + log_in_as(&mut world, "Onehunter", 1); + + let mut script = world + .remove_non_send_resource::() + .expect("VM"); + let before = script.diagnostics().len(); + // The same failure, over and over, through the engine's own catch path — a slash command whose + // body raises is the cheapest real one to drive from a test. + script + .run("SlashCmdList = SlashCmdList or {} SLASH_B293BOOM1 = '/b293boom' SlashCmdList['B293BOOM'] = function() error('every frame') end") + .expect("register"); + for _ in 0..500 { + script.run_slash_command("b293boom", ""); + } + + let rows = script.diagnostics(); + assert_eq!( + rows.len(), + before + 1, + "500 identical raises are ONE new row: {rows:#?}" + ); + let row = rows.last().expect("a row"); + assert_eq!(row.count, 500, "the count is where the 500 went"); + assert_eq!( + row.kind, + benilla_ui::script::diagnostics::DiagnosticKind::Error, + "code ran and raised — the addon is loaded, unlike a Load row" + ); + + drop(script); + drop(world); + let _ = std::fs::remove_dir_all(&tmp); +} diff --git a/crates/benilla-protocol/src/messages/addons.rs b/crates/benilla-protocol/src/messages/addons.rs new file mode 100644 index 00000000..c4aeeacb --- /dev/null +++ b/crates/benilla-protocol/src/messages/addons.rs @@ -0,0 +1,210 @@ +//! The **addon-info block** at the tail of `CMSG_AUTH_SESSION` — what the client tells the world +//! server about its *secure* addons (decision 1497). +//! +//! Every 1.12.1 logon carries one of these. It is not optional decoration: it is the last field of +//! the auth packet, and a server reads it before the session exists. benilla used to send a +//! `decompressed_size = 0` + zlib-of-nothing stub, which **no real client can produce** — and +//! cmangos-classic kicks the session for it (B277). See [`addon_block`] for the shape and +//! [`STOCK_SECURE_ADDONS`] for what a stock install actually sends. +//! +//! ## Where the shape comes from +//! +//! VERIFIED in `WoW.exe` (5875) — wow-5875-re `system/net/scratch/cmsg-auth-session-addon-block.md`. +//! The writer is `0x51d910`, called once, from `HandleAuthChallenge` (`0x5b4143`) straight after the +//! 20-byte SHA-1 proof. It appends: +//! +//! ```text +//! u32 uncompressed_size ; byte length of the buffer below +//! u8[] zlib stream ; RFC1950 framing (deflateInit_ windowBits +15), runs to end of packet +//! ``` +//! +//! and the uncompressed buffer is a **bare concatenation** — no leading count, no trailer — +//! of one record per secure addon: +//! +//! ```text +//! CString name ; NUL-terminated +//! u8 flags ; byte 0 of the addon's own .pub signature file +//! u32 modulus_crc ; CRC-32 of the 256 modulus bytes (.pub bytes 1..=256) +//! u32 url_crc ; CRC-32 of the addon's .url string; 0 when it has none +//! ``` +//! +//! **Which** addons: exactly those whose `.toc` declares `## Secure:` non-zero — enable/disable +//! state is not consulted. On a stock install that is precisely the twelve `Blizzard_*` built-ins +//! and nothing else, because no third-party 1.12 addon declares `## Secure`. A client with *zero* +//! secure addons appends **nothing at all** (not a zero dword) — the writer returns before the +//! local store exists. +//! +//! ## Why the emulators disagree about it, and why it matters +//! +//! Both vmangos and cmangos reject `size == 0` with the same comment ("empty addon packet … can't +//! be received from real client") — that judgement is *correct*, and matched the binary all along. +//! What differs is the consequence: +//! +//! - vmangos `WorldSocket.cpp:447` — `if (BuildAddonPacket(...)) SendPacket(addonPacket);`. A +//! rejection just means no `SMSG_ADDON_INFO` goes back; the session lives. That is the only +//! reason benilla's stub ever worked. +//! - cmangos-classic `WorldSocket.cpp:562` — `if (!ReadAddonInfo(...)) { … "sent bad addon info. +//! Kicking."; return false; }`. Same rejection, session killed. Not config-gated: the check is in +//! the anticheat module *and* in `NullSessionAnticheat`, so it runs with the anticheat off. +//! +//! cmangos's two readers also disagree with each other on field order — its anticheat module reads +//! `name, u8, u32, u32` (right, matching the binary) and its `NullSessionAnticheat` reads +//! `name, u32, u32, u8` (wrong, but the same nine bytes, so it parses without throwing). + +/// CRC-32 of the stock Blizzard public-key modulus — what a stock, unmodified addon's `.pub` +/// hashes to, and the value every emulator calls the "standard addon CRC". A server that sees it +/// knows it need not push the 256-byte modulus back in `SMSG_ADDON_INFO`. +/// +/// VERIFIED: `zlib.crc32` of the 256 modulus bytes in this install's `Blizzard_*.pub` files, which +/// are byte-identical to the `modulus`/`tdata` array carried in vmangos and cmangos alike. +pub const STANDARD_MODULUS_CRC: u32 = 0x4C1C_776D; + +/// One record in the addon block — a secure addon as the client describes it to the server. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct SecureAddon<'a> { + /// The addon's folder name (`Blizzard_AuctionUI`), sent as a `CString`. + pub name: &'a str, + /// Byte 0 of the addon's `.pub` file — `1` for a stock signature, `0` when the client found no + /// usable `.pub`. A server may write a different byte back (`SMSG_ADDON_INFO`), which the real + /// client persists into the `.pub` and echoes next logon; cmangos's anticheat uses exactly that + /// channel to stamp a per-install fingerprint across four of the twelve. + pub flags: u8, + /// CRC-32 of the `.pub`'s 256 modulus bytes — [`STANDARD_MODULUS_CRC`] for a stock signature, + /// `0` when `flags` is `0`. + pub modulus_crc: u32, + /// CRC-32 of the addon's `.url` string. `0` for every stock addon (none ship one). + pub url_crc: u32, +} + +impl SecureAddon<'_> { + /// A stock, signed Blizzard built-in: enabled signature, standard modulus, no URL. The shape + /// all twelve take on an unmodified install. + const fn stock(name: &str) -> SecureAddon<'_> { + SecureAddon { + name, + flags: 1, + modulus_crc: STANDARD_MODULUS_CRC, + url_crc: 0, + } + } +} + +/// The twelve `Blizzard_*` built-ins a stock 1.12.1 install reports, in the order the client sends +/// them (ascending ASCII of the folder name). +/// +/// VERIFIED against a real 1.12.1.5875 client's `CMSG_AUTH_SESSION` captured against a live +/// Blizzard server in 2006 (wow-5875-re `system/net/scratch/cmsg-auth-session-addon-block.md`): +/// 342 uncompressed bytes, twelve records, every one `flags = 1`, `modulus_crc =` +/// [`STANDARD_MODULUS_CRC`], `url_crc = 0`, and nothing left over — the arithmetic proof that the +/// buffer carries no count and no trailer. +/// +/// benilla sends this list rather than one derived from the install's own `Interface/AddOns` +/// folder, and the two agree on every stock install: the client's rule is "`## Secure:` non-zero", +/// which no third-party 1.12 addon sets, and benilla's own addons (loaded from +/// `benilla-config/AddOns`) are third-party by construction. Reading each `.pub` off the player's +/// chain would only differ on an install whose signature files have been altered — see 1497. +pub const STOCK_SECURE_ADDONS: [SecureAddon<'static>; 12] = [ + SecureAddon::stock("Blizzard_AuctionUI"), + SecureAddon::stock("Blizzard_BattlefieldMinimap"), + SecureAddon::stock("Blizzard_BindingUI"), + SecureAddon::stock("Blizzard_CombatText"), + SecureAddon::stock("Blizzard_CraftUI"), + SecureAddon::stock("Blizzard_GMSurveyUI"), + SecureAddon::stock("Blizzard_InspectUI"), + SecureAddon::stock("Blizzard_MacroUI"), + SecureAddon::stock("Blizzard_RaidUI"), + SecureAddon::stock("Blizzard_TalentUI"), + SecureAddon::stock("Blizzard_TradeSkillUI"), + SecureAddon::stock("Blizzard_TrainerUI"), +]; + +/// The **uncompressed** addon buffer: one record per addon, concatenated, nothing else. +pub fn addon_block(addons: &[SecureAddon]) -> Vec { + let mut out = Vec::with_capacity(addons.iter().map(|a| a.name.len() + 10).sum()); + for addon in addons { + out.extend_from_slice(addon.name.as_bytes()); + out.push(0); + out.push(addon.flags); + out.extend_from_slice(&addon.modulus_crc.to_le_bytes()); + out.extend_from_slice(&addon.url_crc.to_le_bytes()); + } + out +} + +/// The block as it rides the wire: `u32` uncompressed size + the zlib stream, or **nothing at all** +/// when there are no secure addons (what the real client does — it never emits a zero size). +pub fn addon_tail(addons: &[SecureAddon]) -> Vec { + if addons.is_empty() { + return Vec::new(); + } + let plain = addon_block(addons); + let mut out = Vec::with_capacity(plain.len() / 2 + 8); + out.extend_from_slice(&(plain.len() as u32).to_le_bytes()); + let mut encoder = + flate2::write::ZlibEncoder::new(out, flate2::Compression::default() /* level 6 */); + std::io::Write::write_all(&mut encoder, &plain).expect("zlib write to a Vec cannot fail"); + encoder.finish().expect("zlib finish on a Vec cannot fail") +} + +#[cfg(test)] +mod tests { + use super::*; + + /// The retail ground truth: 342 uncompressed bytes, consumed exactly by twelve records. + #[test] + fn stock_block_matches_the_retail_capture() { + let plain = addon_block(&STOCK_SECURE_ADDONS); + assert_eq!(plain.len(), 342, "retail sent 342 uncompressed bytes"); + + // Walk it back the way a server does — name, flags, modulus crc, url crc — and require it + // to land exactly on the end. That is what proves there is no count and no trailer. + let mut rest = &plain[..]; + let mut seen = Vec::new(); + while !rest.is_empty() { + let nul = rest + .iter() + .position(|&b| b == 0) + .expect("record has a name"); + let name = std::str::from_utf8(&rest[..nul]).expect("name is utf-8"); + rest = &rest[nul + 1..]; + let (flags, tail) = rest.split_first().expect("record has flags"); + let modulus_crc = u32::from_le_bytes(tail[0..4].try_into().unwrap()); + let url_crc = u32::from_le_bytes(tail[4..8].try_into().unwrap()); + assert_eq!(*flags, 1, "{name} is a stock signed addon"); + assert_eq!(modulus_crc, STANDARD_MODULUS_CRC, "{name} modulus crc"); + assert_eq!(url_crc, 0, "{name} ships no .url"); + seen.push(name.to_string()); + rest = &tail[8..]; + } + assert_eq!(seen.len(), 12); + assert!(seen.iter().all(|n| n.starts_with("Blizzard_"))); + let mut sorted = seen.clone(); + sorted.sort(); + assert_eq!( + seen, sorted, + "the client sends them in ascending ASCII order" + ); + } + + /// The wire tail: size prefix, zlib framing, and the retail compressed length. `flate2` at its + /// default level reproduces the 2006 client's 130 bytes byte-for-byte — the real client used + /// `Z_DEFAULT_COMPRESSION` through the same zlib deflate. + #[test] + fn stock_tail_is_size_plus_zlib() { + let tail = addon_tail(&STOCK_SECURE_ADDONS); + assert_eq!( + u32::from_le_bytes(tail[0..4].try_into().unwrap()), + 342, + "size prefix is the uncompressed length" + ); + assert_eq!(tail[4], 0x78, "RFC1950 zlib framing, not raw deflate"); + assert_eq!(tail.len(), 4 + 130, "retail compressed to 130 bytes"); + } + + /// Zero secure addons appends *nothing* — never a zero size, which no real client can emit and + /// which cmangos-classic kicks for. + #[test] + fn no_secure_addons_appends_nothing() { + assert!(addon_tail(&[]).is_empty()); + } +} diff --git a/crates/benilla-protocol/src/messages/client.rs b/crates/benilla-protocol/src/messages/client.rs index a5faf4a3..5558db04 100644 --- a/crates/benilla-protocol/src/messages/client.rs +++ b/crates/benilla-protocol/src/messages/client.rs @@ -2,28 +2,29 @@ //! the `ServerPacket` vocabulary stay in [`super`]; spell/attack builders live with their family //! in [`super::spells`]). Framing/encryption is `world.rs`'s job; these produce plain body bytes. +use super::addons::{addon_tail, SecureAddon}; use super::{CharCreateReq, MovementInfo}; -/// `CMSG_AUTH_SESSION` body: build, server id, account name, client seed, proof, and an empty -/// zlib-wrapped addon block (`decompressed_size = 0` + zlib of no bytes — what the client sends and -/// vmangos accepts). +/// `CMSG_AUTH_SESSION` body: build, server id, account name, client seed, proof, and the +/// **addon-info block** — `addons`' uncompressed size plus its zlib stream, exactly as the writer +/// at `0x51d910` appends it (see [`super::addons`], decision 1497). Pass +/// [`super::STOCK_SECURE_ADDONS`] for what a stock install sends. pub fn auth_session( build: u32, username: &str, client_seed: u32, client_proof: &[u8; 20], + addons: &[SecureAddon], ) -> Vec { - let mut body = Vec::with_capacity(40 + username.len()); + let tail = addon_tail(addons); + let mut body = Vec::with_capacity(34 + username.len() + tail.len()); body.extend_from_slice(&build.to_le_bytes()); body.extend_from_slice(&0u32.to_le_bytes()); // server_id body.extend_from_slice(username.as_bytes()); body.push(0); body.extend_from_slice(&client_seed.to_le_bytes()); body.extend_from_slice(client_proof); - body.extend_from_slice(&0u32.to_le_bytes()); // addon decompressed_size - let encoder = flate2::write::ZlibEncoder::new(Vec::new(), flate2::Compression::default()); - let empty_addon_zlib = encoder.finish().expect("zlib finish on empty input"); - body.extend_from_slice(&empty_addon_zlib); + body.extend_from_slice(&tail); body } diff --git a/crates/benilla-protocol/src/messages/mod.rs b/crates/benilla-protocol/src/messages/mod.rs index 6738f192..6fa3536c 100644 --- a/crates/benilla-protocol/src/messages/mod.rs +++ b/crates/benilla-protocol/src/messages/mod.rs @@ -16,6 +16,7 @@ //! `tests/common`). mod action_bar; +pub mod addons; mod area_trigger; mod attack; mod bank; @@ -60,6 +61,7 @@ mod world_state; pub use action_bar::{ set_action_button, ActionButton, ACTION_KIND_ITEM, ACTION_KIND_MACRO, ACTION_KIND_SPELL, }; +pub use addons::{SecureAddon, STANDARD_MODULUS_CRC, STOCK_SECURE_ADDONS}; pub use area_trigger::area_trigger; pub use attack::{attack_swing, AttackerState}; pub use bank::{ diff --git a/crates/benilla-protocol/src/world/session.rs b/crates/benilla-protocol/src/world/session.rs index b25640e4..a1023826 100644 --- a/crates/benilla-protocol/src/world/session.rs +++ b/crates/benilla-protocol/src/world/session.rs @@ -98,11 +98,15 @@ impl WorldSession { seed.into_client_header_crypto(&username_n, session_key, server_seed); // 3. CMSG_AUTH_SESSION goes out unencrypted; obfuscation begins immediately after. + // The addon block is the tail of this packet and is NOT optional: cmangos-classic kicks a + // session whose addon size is zero, which is what benilla used to send (B277, decision + // 1497). `STOCK_SECURE_ADDONS` is what a stock 1.12.1 install reports. let body = messages::auth_session( u32::from(crate::CLIENT_BUILD), &username.to_uppercase(), client_seed, &client_proof, + &messages::STOCK_SECURE_ADDONS, ); send_packet(&mut stream, None, opcode::CMSG_AUTH_SESSION, &body) .context("sending CMSG_AUTH_SESSION")?; diff --git a/crates/benilla-protocol/tests/client.rs b/crates/benilla-protocol/tests/client.rs index a9ffd458..c6c54f48 100644 --- a/crates/benilla-protocol/tests/client.rs +++ b/crates/benilla-protocol/tests/client.rs @@ -36,11 +36,33 @@ fn faction_language_per_race() { #[test] fn client_bodies_golden() { let proof: [u8; 20] = std::array::from_fn(|i| (i as u8).wrapping_mul(3).wrapping_add(7)); + // The tail is the addon-info block: `342` uncompressed + the zlib stream of the stock twelve + // (decision 1497). Byte-identical to a real 1.12.1.5875 client's — the same 130 compressed + // bytes as the 2006 retail capture wow-5875-re verified this against. assert_eq!( - messages::auth_session(5875, "TESTUSER", 0x1122_3344, &proof), - hx("f31600000000000054455354555345520044332211070a0d101316191c1f2225282b2e3134373a3d4000000000789c030000000001"), + messages::auth_session( + 5875, + "TESTUSER", + 0x1122_3344, + &proof, + &messages::STOCK_SECURE_ADDONS + ), + hx(concat!( + "f31600000000000054455354555345520044332211070a0d101316191c1f2225282b2e3134373a3d40", + "56010000", + "789c75ccbd0ec2300c04e0f21ebc0c614095c842c38c4ce2220bc7a98ccb4f9f1e16240673eb777781", + "695940cb693367a326c7be5bd5c77adf7d12be16c08c7124e41249a8c2e495480ac9c53dd8b67a064b", + "f8340f15467367bb38cc7ac7978bbddc26ccfe3042d6e6ca01a8b8908051fcb7a45070b812f33f2641", + "fdb5379019668f", + )), "CMSG_AUTH_SESSION body" ); + // No secure addons: the tail is *absent*, never a zero size (which no real client emits). + assert_eq!( + messages::auth_session(5875, "TESTUSER", 0x1122_3344, &proof, &[]), + hx("f31600000000000054455354555345520044332211070a0d101316191c1f2225282b2e3134373a3d40"), + "CMSG_AUTH_SESSION body with no secure addons" + ); // Zero-appearance body (the create-if-empty starter): name + [race,class,gender] + 5 zeros + 0. assert_eq!( messages::char_create(&messages::CharCreateReq { diff --git a/crates/benilla-ui/src/script/diagnostics.rs b/crates/benilla-ui/src/script/diagnostics.rs new file mode 100644 index 00000000..5640becb --- /dev/null +++ b/crates/benilla-ui/src/script/diagnostics.rs @@ -0,0 +1,339 @@ +//! The **script error log** — what went wrong, kept where the player can read it (decision 1495). +//! +//! ## Why this exists +//! +//! 1305 gave script errors a *screen*: the engine dispatches every caught error to +//! `geterrorhandler()`, and FrameXML's `_ERRORMESSAGE` puts the red `ScriptErrors` dialog up. That +//! is the reference's behaviour and it stays exactly as it is. It is also, on its own, not enough, +//! and the reporters said so within a day of it shipping — Goudy, *minutes after screenshotting +//! that very dialog working*: *"some kind of lua error frame probably needs to be implemented, as +//! there are a lot of addons that still doesn't work"*; nazriel_0, two days later: *"probably we +//! should start using something like Bug Grabber to report those stack traces better"* (ledger +//! B293, split from B271). +//! +//! Both asks are the same three gaps, and all three are about *memory*: +//! +//! 1. **`_ERRORMESSAGE` shows a burst's FIRST message only** — its own `ScriptErrors:IsVisible()` +//! guard, faithfully transcribed. A world entry that raises 1,113 times (1305 measured exactly +//! that) shows one string. This is precisely why BugGrabber exists in the real world. +//! 2. **An addon can fail to load without ever raising**, and those failures had no channel at all +//! — a manifest entry the package does not ship, a document that will not parse, a dependency +//! cycle, a missing hard dependency, a broken `Bindings.xml`. They logged to the terminal and +//! stopped; the addon simply was not there and the client said nothing. That is the literal +//! content of *"a lot of addons that still doesn't work"*. +//! 3. **Nothing retained anything.** [`super::UiScript::take_errors`] drains to the host log every +//! frame, so once a frame passed, the terminal held the only copy. A player cannot read a +//! terminal, and a bug report written from one is what B271 was debugged off. +//! +//! ## What this is, and what it deliberately is not +//! +//! A bounded, deduplicating, ordered log of everything that went wrong this session, written at +//! the two choke points every failure already passes ([`super::model::Model::record_script_error`] +//! and [`super::UiScript::report_load_failure`]) and read by whatever wants to show it. +//! +//! **"Session" means one world session, not one process.** The log lives on the `Model`, and the +//! in-game UI is rebuilt onto a fresh VM at every world entry (1290) — so a logout or a `ReloadUI` +//! starts an empty log. That is the right boundary and it self-heals: the load walk runs again on +//! the way back in and re-records whatever is still broken. What it is *not* is a place to look for +//! what happened before the reload you just did. +//! +//! It is **purely additive**: no dispatch changes, no `_ERRORMESSAGE` change, no divergence from +//! the reference's own behaviour. The reference simply never had this instrument, and *"a modern, +//! idiomatic client"* covers building one. +//! +//! It is **not** a transcription of BugGrabber, which is somebody else's addon (and not Blizzard's +//! either). It is our own equivalent, and an addon that installs its own `seterrorhandler` still +//! wins outright for the dispatch half exactly as 1195/1305 leave it. +//! +//! ## Dedupe, order, and the bound +//! +//! **Repeats collapse onto the first occurrence and bump a count.** An `OnUpdate` that raises +//! every frame is one row reading `×1113`, not 1,113 rows — without that the log is useless within +//! seconds, and unbounded besides. +//! +//! **Order is first-occurrence, oldest first.** The load-time failures are the causes and the +//! later runtime errors are usually their consequences, so a chronological read is the one that +//! explains itself. A repeat does *not* move its row to the end: a row that jumps around while you +//! read it is worse than a stale position. +//! +//! **[`DIAGNOSTIC_LOG_CAP`] distinct rows**, evicting oldest-first. `seq` is monotonic and never +//! reused, so an evicted prefix is visible as a gap rather than silently rewriting history — a +//! surface can say "showing #251-#500" honestly. The cap binds *distinct* messages, which after +//! dedupe is a much higher ceiling than it looks: 1305's 1,113-raise run produces **one** row. + +use std::collections::VecDeque; + +use mlua::{IntoLua, Lua, MultiValue, Value}; + +use super::Model; + +/// How many **distinct** messages the log retains before evicting the oldest. +/// +/// Sized against the worst real load we have measured rather than a round number: the 218-addon +/// corpus behind 1306, loaded at once, is the widest set of distinct failures this client has ever +/// produced. Dedupe means repeats cost nothing, so this bounds *kinds* of problem, not events. +pub const DIAGNOSTIC_LOG_CAP: usize = 256; + +/// What a retained row *is* — and the split is by **consequence**, not by where it was caught, +/// because that is the only distinction the player can act on. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum DiagnosticKind { + /// **Code ran and raised.** The addon is loaded and working apart from this; the failure may + /// be harmless, may be one frame's bad luck, may be constant. This is the class that already + /// reaches the red dialog. + Error, + /// **An addon did not load.** Definitely broken, definitely worth telling somebody about, and + /// before 1495 invisible unless you were reading the terminal. Both the raising kind (a file + /// scope that blew up) and the silent kind (a missing file, an unparseable document, a + /// dependency that isn't there) land here — from the player's side they are one thing: *the + /// addon isn't running*. + Load, +} + +impl DiagnosticKind { + /// The one-word tag a surface prints. Stable — a surface may key colour off it. + pub fn tag(self) -> &'static str { + match self { + Self::Error => "error", + Self::Load => "load", + } + } +} + +/// One retained row. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct Diagnostic { + /// Monotonic, 1-based, **never reused** — the "#N" a surface shows. Survives eviction, so a + /// gap in the numbers is an honest report that older rows were dropped. + pub seq: u64, + pub kind: DiagnosticKind, + /// The message exactly as the host logged it — same string, same spelling, so a player's + /// screenshot and our terminal line are greppable against each other. + pub message: String, + /// How many times this exact `(kind, message)` has occurred. Starts at 1. + pub count: u32, +} + +/// The log itself. Lives on the model; see the module header for the shape and the why. +#[derive(Debug, Default)] +pub(crate) struct DiagnosticLog { + rows: VecDeque, + /// The last `seq` handed out. Kept separately from `rows.len()` precisely so eviction cannot + /// renumber anything. + seq: u64, +} + +impl DiagnosticLog { + /// Record one failure, collapsing it onto an existing identical row if there is one. + /// + /// The dedupe scan is linear over at most [`DIAGNOSTIC_LOG_CAP`] rows and runs on the failure + /// path only. The pathological caller is an `OnUpdate` raising at frame rate, where the scan + /// hits its row and returns — cheaper by far than the `String` allocation it saves, and + /// arithmetic beside the `mlua` error formatting that produced the message in the first place. + pub(crate) fn record(&mut self, kind: DiagnosticKind, message: &str) { + if let Some(row) = self + .rows + .iter_mut() + .find(|r| r.kind == kind && r.message == message) + { + // Saturating rather than wrapping: a count that rolls over to 0 reads as "this never + // happened", which is the one answer that is never true here. + row.count = row.count.saturating_add(1); + return; + } + self.seq += 1; + if self.rows.len() == DIAGNOSTIC_LOG_CAP { + self.rows.pop_front(); + } + self.rows.push_back(Diagnostic { + seq: self.seq, + kind, + message: message.to_string(), + count: 1, + }); + } + + /// Every retained row, oldest first. + pub(crate) fn rows(&self) -> impl Iterator { + self.rows.iter() + } + + /// Retained rows. + pub(crate) fn len(&self) -> usize { + self.rows.len() + } + + /// Distinct failures **ever** recorded this session, including rows since evicted. The + /// difference from [`Self::len`] is how many the cap ate. + pub(crate) fn total(&self) -> u64 { + self.seq + } + + /// Forget everything — the player's own "I've read these" act. Deliberately does **not** reset + /// `seq`: after a clear, the next row is still #N+1, so a screenshot taken before the clear and + /// one taken after cannot claim the same number for different failures. + pub(crate) fn clear(&mut self) { + self.rows.clear(); + } +} + +impl super::UiScript { + /// Every retained row, oldest first — a clone, because the caller is the app's UI and the + /// model is behind `app_data`. The log is capped at [`DIAGNOSTIC_LOG_CAP`] rows, so this is a + /// bounded copy of a list nothing reads at frame rate. + pub fn diagnostics(&self) -> Vec { + self.model_ref().diagnostics.rows().cloned().collect() + } + + /// `(retained rows, distinct failures ever recorded)`. The two differ once the cap has evicted. + pub fn diagnostic_counts(&self) -> (usize, u64) { + let m = self.model_ref(); + (m.diagnostics.len(), m.diagnostics.total()) + } + + /// Forget the retained rows (the player's "I've read these"). See [`DiagnosticLog::clear`] for + /// why the numbering is deliberately *not* reset. + pub fn clear_diagnostics(&self) { + self.model_mut().diagnostics.clear(); + } + + /// Record an addon that **did not load** for a reason that never raised — a manifest entry the + /// package does not ship, a document that will not parse, a dependency cycle, a hard + /// dependency that is missing, a broken `Bindings.xml`. + /// + /// **Deliberately does not dispatch.** These are not Lua errors: nothing raised, there is no + /// traceback, and handing them to `geterrorhandler()` would put non-errors through + /// `_ERRORMESSAGE` and through every addon handler that ever replaces it — a divergence from + /// the reference for no gain, since the reference's own answer to an unparseable document is a + /// line in `FrameXML.log` and silence on screen. What 1495 changes is that the silence is no + /// longer *total*: the failure is retained and readable, it just does not seize the screen. + /// + /// The caller still logs its own line; this is the retention half, not a replacement for it. + pub fn report_load_failure(&self, msg: &str) { + self.model_mut() + .diagnostics + .record(DiagnosticKind::Load, msg); + } +} + +/// Register the error-log reads the `BenillaScriptLogFrame` polls. +/// +/// **`Benilla`-prefixed because they are ours**, not 1.12's — the prefix rule as +/// `tests::reference_surface` enforces it (1254: a name we invent must not collide, a name the +/// reference owns must not be hidden). 1.12 has no error-log API to shadow, so there is nothing +/// here an addon could have meant by a bare name. +pub(super) fn install(lua: &Lua) -> mlua::Result<()> { + // BenillaGetNumScriptErrors() -> retained, totalEverRecorded + lua.globals().set( + "BenillaGetNumScriptErrors", + lua.create_function(|lua, ()| { + let model = lua.app_data_ref::().expect("model app_data"); + Ok((model.diagnostics.len(), model.diagnostics.total())) + })?, + )?; + // BenillaGetScriptErrorInfo(index) -> seq, kind, message, count (1-based, oldest first; + // nil for an out-of-range index, so a stale row index during a repaint reads as "gone" + // rather than erroring inside the very frame that lists errors.) + lua.globals().set( + "BenillaGetScriptErrorInfo", + lua.create_function(|lua, index: i64| { + let model = lua.app_data_ref::().expect("model app_data"); + let Some(row) = usize::try_from(index) + .ok() + .and_then(|i| i.checked_sub(1)) + .and_then(|i| model.diagnostics.rows().nth(i)) + else { + return Ok(MultiValue::new()); + }; + Ok(MultiValue::from_vec(vec![ + Value::Integer(row.seq as i64), + lua.create_string(row.kind.tag())?.into_lua(lua)?, + lua.create_string(&row.message)?.into_lua(lua)?, + Value::Integer(i64::from(row.count)), + ])) + })?, + )?; + // BenillaClearScriptErrors() + lua.globals().set( + "BenillaClearScriptErrors", + lua.create_function(|lua, ()| { + lua.app_data_mut::() + .expect("model app_data") + .diagnostics + .clear(); + Ok(()) + })?, + ) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn repeats_collapse_onto_the_first_row_and_count() { + let mut log = DiagnosticLog::default(); + for _ in 0..1113 { + log.record(DiagnosticKind::Error, "OnUpdate: boom"); + } + assert_eq!(log.len(), 1, "1,113 raises are ONE row — the whole point"); + let row = log.rows().next().unwrap(); + assert_eq!(row.count, 1113); + assert_eq!(row.seq, 1); + assert_eq!(log.total(), 1); + } + + #[test] + fn kind_is_part_of_identity() { + let mut log = DiagnosticLog::default(); + log.record(DiagnosticKind::Error, "same text"); + log.record(DiagnosticKind::Load, "same text"); + assert_eq!( + log.len(), + 2, + "the same string means different things per kind" + ); + } + + #[test] + fn order_is_first_occurrence_and_a_repeat_does_not_move_its_row() { + let mut log = DiagnosticLog::default(); + log.record(DiagnosticKind::Load, "first"); + log.record(DiagnosticKind::Error, "second"); + log.record(DiagnosticKind::Load, "first"); + let seen: Vec<_> = log.rows().map(|r| r.message.as_str()).collect(); + assert_eq!(seen, ["first", "second"]); + assert_eq!(log.rows().next().unwrap().count, 2); + } + + #[test] + fn eviction_is_oldest_first_and_seq_never_rewinds() { + let mut log = DiagnosticLog::default(); + for i in 0..DIAGNOSTIC_LOG_CAP + 10 { + log.record(DiagnosticKind::Error, &format!("e{i}")); + } + assert_eq!(log.len(), DIAGNOSTIC_LOG_CAP); + let first = log.rows().next().unwrap(); + assert_eq!(first.message, "e10", "the oldest ten were evicted"); + assert_eq!( + first.seq, 11, + "seq is monotonic: the gap IS the honest report that rows were dropped" + ); + assert_eq!(log.total() as usize, DIAGNOSTIC_LOG_CAP + 10); + } + + #[test] + fn clear_forgets_rows_but_not_the_numbering() { + let mut log = DiagnosticLog::default(); + log.record(DiagnosticKind::Error, "a"); + log.record(DiagnosticKind::Error, "b"); + log.clear(); + assert_eq!(log.len(), 0); + log.record(DiagnosticKind::Error, "c"); + assert_eq!( + log.rows().next().unwrap().seq, + 3, + "a number never names two different failures" + ); + } +} diff --git a/crates/benilla-ui/src/script/mod.rs b/crates/benilla-ui/src/script/mod.rs index 6a3fc702..bc8c0325 100644 --- a/crates/benilla-ui/src/script/mod.rs +++ b/crates/benilla-ui/src/script/mod.rs @@ -55,6 +55,7 @@ mod cooldown; mod craft; mod cursor; mod death; +pub mod diagnostics; mod editbox; pub(crate) use editbox::adopt_text_region; pub(crate) mod addon; @@ -569,6 +570,7 @@ impl UiScript { tooltip::install(&lua)?; worldmap::install(&lua)?; net_stats::install(&lua)?; + diagnostics::install(&lua)?; let s = UiScript { lua, @@ -1238,13 +1240,21 @@ impl UiScript { /// Report a script error the host caught **outside** the VM's own dispatch — an addon file /// that failed to compile or raised at file scope during the load walk. It joins the - /// handler-dispatch queue only: the caller has already logged it (the load walk's per-file - /// `error!` + `failures` contract), so putting it in `errors` too would double-log at the - /// app's per-frame drain. + /// handler-dispatch queue and the retained log, but **not** `errors`: the caller has already + /// logged it (the load walk's per-file `error!` + `failures` contract), so putting it there too + /// would double-log at the app's per-frame drain. + /// + /// Its silent sibling is [`Self::report_load_failure`] (decision 1495) — same retention, no + /// dispatch, for the load failures that never raise at all. pub fn report_script_error(&self, msg: &str) { - self.model_mut() - .pending_error_dispatch - .push(msg.to_string()); + let mut model = self.model_mut(); + // Retained as a **Load** row, not an Error one (decision 1495): every caller of this is + // the load walk, and from the player's side "the addon's file scope raised" and "the + // addon's file was missing" are the same fact — the addon is not running. + model + .diagnostics + .record(diagnostics::DiagnosticKind::Load, msg); + model.pending_error_dispatch.push(msg.to_string()); } /// Hand every queued script error to the Lua-side error handler — the reference's own shape: diff --git a/crates/benilla-ui/src/script/model.rs b/crates/benilla-ui/src/script/model.rs index c455c2d2..39559fde 100644 --- a/crates/benilla-ui/src/script/model.rs +++ b/crates/benilla-ui/src/script/model.rs @@ -328,6 +328,12 @@ pub(crate) struct Model { /// inside the failed call). A message produced *by* the handler itself goes to `errors` only — /// that asymmetry is the recursion guard. pub(crate) pending_error_dispatch: Vec, + /// The **retained** script-error log — what [`errors`](Self::errors) and the load walk both + /// throw away once logged (decision 1495). `errors` is a per-frame drain for the host's + /// terminal; this is the session's memory, deduplicated and bounded, and the only thing a + /// player can actually read. See [`super::diagnostics`] for why the reference's own dialog is + /// not enough on its own. + pub(crate) diagnostics: super::diagnostics::DiagnosticLog, /// Non-fatal warnings surfaced to the host (e.g. `CreateFrame`'s ignored `inherits=` template). pub(crate) warnings: Vec, /// The screen-root rect (`[bottom, left, top, right]`), the anchor base for top-level frames. @@ -1194,6 +1200,11 @@ impl Model { /// a failure raised by the error handler *itself* is pushed to `errors` directly instead, /// which is what keeps the dispatch from recursing. pub(crate) fn record_script_error(&mut self, msg: String) { + // Three channels now, and the third is the one with a memory (decision 1495): the dialog + // shows a burst's first message and the host drain empties every frame, so without this + // the 1,112 raises after the first exist nowhere a player can reach. + self.diagnostics + .record(super::diagnostics::DiagnosticKind::Error, &msg); self.pending_error_dispatch.push(msg.clone()); self.errors.push(msg); } @@ -1259,6 +1270,7 @@ impl Model { pending_size_changed: Vec::new(), errors: Vec::new(), pending_error_dispatch: Vec::new(), + diagnostics: Default::default(), warnings: Vec::new(), // Classic Era's UIParent virtual space is 1024×768-ish; a sensible default the host can // override with `set_screen_size`. y-up: [bottom, left, top, right].