57518578903a44c1632b60e8633894f236a7e8ed
654
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
  gromandClaude Opus 5
|
5751857890 |
The realm's client is not stock 1.12, and its Lua grammar is not 1.12's either
SoftSpoken (z1xel/SoftSpoken, a 1.12 narration port) failed to load: nine files across six addons would not compile, and the three that did then raised at runtime indexing namespaces the failed chunks never built. Every one of the nine is valid Lua 5.1 and invalid Lua 5.0 — `#`, `%`, `0x3040`, `local NAME = ...`. `third_party/lua-src` is a fork that deliberately 5.0-ifies the grammar: three arms deleted from `lparser.c` (`simpleexp`'s `TK_DOTS`, `getunopr`'s `'#'`, `getbinopr`'s `'%'`) and `read_numeral` replaced with 5.0's, which has no hex. Every deletion is byte-read out of `WoW.exe`'s own parser and every one is right about stock 1.12. No OctoWoW client runs stock 1.12. ClassicAPI.dll is in the realm's bundle and rewrites every chunk at the `luaL_loadbuffer` chokepoint — file scripts, `loadstring` and XML `OnLoad` alike — adding exactly five constructs: `#`, `%`, `...` as an expression, `0x` hex and `[=[ ]=]`. Confirmed three ways: the repo's own highlights line, PR #28, and the DLL's string pool, which twice says "Our 5.1 syntax backport makes the probe pass". `docs/ADDON_COMPAT.md`'s row for this said "free — benilla's VM is mlua/Lua 5.1 already". That was the error. So the target grammar is the union, and the fork now carries it: * the three parser arms restored, as stock 5.1 writes them; * `read_numeral` grown an explicit `0x` arm. It could not take 5.1's version back wholesale: 5.1 reached hex through a trailing `isalnum` slurp, and that slurp has to stay gone or `1then` breaks again (`AdvancedTradeSkillWindow2.lua:3878`, which the client compiles). The prefix arm gives both; * addon file chunks now called with `(addonName, addonPrivateTable)` (`UiScript::run_addon_chunk`, four call sites). Taking `...` without passing the arguments only moves the failure one line later, which is what ClassicAPI's own notes record; * `string.reverse` added to the stdlib backport — SoftSpoken calls it. 1.12's own dialect is untouched. `LUA_COMPAT_VARARG` withholds the `arg` table only from a function that *uses* `...` as a value, so the shipped FrameXML's `function f(...) … arg.n … end` still gets `arg`: the two dialects coexist one function at a time, which is what the client-plus-ClassicAPI does. The cost is the era probe. 170 corpus `loadstring` sites ask `loadstring("return function(...) return ... end")` and 92 library files in 24 folders branch on the answer; they now take the 5.1 branch, as they do on the real client. Two branch wrongly there and ClassicAPI ships a compensation for each — Compost-2.0 rev 17406+'s no-argument `Erase` stub (bundled by BigWigs and QuickHeal; it silently stops clearing in place) and Cartographer_Notes' `WorldMapDeathRelease`. Both are now carried in `lua51_probe_fallout.rs`, probed by behaviour rather than by revision and re-checked on every `ADDON_LOADED`, because AceLibrary can upgrade a library mid-session. Measured on grom's own AddOns folder: SoftSpoken, SoftSpoken_Stories, SoftSpoken_Places, SoftSpoken_Writings, SpokenZonesAudio, SpokenBooksAudio, SpokenQuestsAudio{Horde,Alliance,Gossip,Shared}, SSS_VOFallback and AI_VoiceOverData_Turtle all load with zero load errors and zero session errors, 98 `Spoken*` globals, `/softspoken` registered, and PLAYER_ENTERING_WORLD, QUEST_DETAIL, GOSSIP_SHOW and ITEM_TEXT_BEGIN reaching 108/108, 1/1, 6/6 and 2/2 handlers clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
gromandClaude Opus 5
|
7a3ab5e7bf |
A tooltip raise cost 6.6 ms because every frame it minted re-derived the layout graph
`WOW_UI_VERBS=1` over the hover bench answered it in one line: the rebuilt frame's cost was **`frame:GetLeft`, 3 calls at 2188 us each — 6.56 of the 6.6 ms** — inside `UIParent:OnUpdate`. `GetLeft` forces a resolve, and each of those resolves was a whole-graph derivation of ~19,000 frames and ~44,000 regions, because `Model::frame_id` gave up naming the graph on every mint. OctoPawn mints two frames on every `GameTooltip` OnShow purely to own a one-shot `OnUpdate` — the 1.12 way to write `C_Timer.After` — so a raise dirtied the graph twice and the three `GetLeft`s each paid for it. A just-minted frame is now **appended to the cached graph as a leaf** (`LayoutScope::append_frame_leaf`). Sound because the id was minted this instant, so no anchor anywhere can name it: nothing cached reads the new node and no cached edge moves. What it reads is its own anchors' targets, which the append is handed — a mint is not always a creation, since `frame_id` mints on the first *fetch* too, and a frame built in Rust (a nameplate, a chat bubble) is first fetched when Lua asks for its wrapper with its anchors long written. `last` stays `NO_NODE` so the incremental seed sees the hash move; `commit_incremental` counts the slot, which keeps the next full pass's `matched != last_count` node-death test honest. Refused when the frame already carries regions: a region is a graph node without needing a minted id — the derivation mints region ids inside its own walk — so only a derivation can add those rows. **Measured, same bench, same interface: the rebuild costs -0.08 ms/frame under forty churning plates and +0.14 ms with none up, from 6.63 and 6.78.** The rebuilt windows now report no derivation sites at all. Two things about how this was found, both in the bench's doc now: - Its own earlier conclusion was that the cost was "the plate's line walk and the hooks hanging off it, flat, and not the graph it dirties", because the crowded and the empty shape cost the same. They cost the same because **both** were paying the full derivation. A fair experiment that was blind to its own answer, and one env var would have named it. - `WOW_LAYOUT_VERIFY` over the real interface is red on this change — and it is the documented pre-existing 48-region shortfall, reproduced identically with the whole change stashed: `frames 13544 -> 13544, regions 29481 -> 29529` either way. All 2069 benilla-ui tests pass, and they run that verifier forced on after every resolve. 189 workspace failures, unchanged, no regressions. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
gromandClaude Opus 5
|
3bbd99ebaf |
The slam macro, end to end through the path the game takes
He re-tested and it still never casts Slam, so I drove the whole thing rather
than the conditional: `UseAction` on a macro slot → `run_macro` fires
`EXECUTE_CHAT_LINE` per line (`0x4f14e0`) → the stock ChatFrame sends it through
its edit box (`ChatFrame.lua:1343`) → `ChatEdit_ParseText` finds
`SlashCmdList["CAST"]` → CleveRoids' hook → `GetParsedMsg` → `DoWithConditionals`
→ `Keywords.noslamclip` → `ValidateNoSlamClip` → `CastSpellByName`. On his own
interface, with his own 2.70 s weapon and Battle Stance, the line casts **Slam**
at the top of a swing and **Heroic Strike** 1.6 s in — which is the assertion the
old test never made, and the reason "the conditional answers true" meant nothing.
Three things came out of the drive.
**The load-time latch is worse than I wrote this morning, and it is what he is
seeing.** `GetSlamWindowPercent` reads SP_SwingTimer's `st_timerMax` cache, not
`UnitAttackSpeed`, so the `ADDON_LOADED` latch decides the window for the whole
first fight: the stand-in gives (2.0 - 1.5)/2.0 = **25%** against his weapon's
44%, while `st_timer` is still 0 so the swing reads **100% elapsed** — and
`[noslamclip]` is false with nothing said about it. Before the stand-in it was
false *and* printed "requires SP_SwingTimer" once; the fix traded a visible
failure for a silent one. Both states are pinned in the test now.
So the sheet is **seeded before the addon walk**, beside the roster record and
unit snapshot that are already seeded there for the same reason
(`ui_char::seed_combat_stats`). A `/reload` — and anyone mid-fight — now latches
the real weapon. A fresh login still has no self store (the create block has not
arrived, which is why the record comes off the roster), so the stand-in stands
there and heals on the first swing.
**And the stand-in now says so in the log.** One line per VM per token, through
the channel `/errors` lists: a combat-stats verb answering for "player" before
anything pushed that unit's sheet. One such read is expected at load; what the
line is for is the case where it never stops, which no test of the verb can see
because every one of them seats the sheet first. `UnitAttackSpeed` reaching Lua
with the store's own `UNIT_FIELD_BASEATTACKTIME` is now asserted through the real
`feed_char`, which is the test that was missing underneath all of this.
The dual-wield road is pinned too, because it is the one shape that can strand
the main-hand timer: `CheckDamageSource` returns "MAIN" on line one for a
two-hander and never runs its arithmetic, but dual-wielding it weighs the hit
against `UnitDamage`'s ranges scaled by the target's armour — and
`UnitArmor("target")` is 0 on any 1.12 client, ours included, because another
unit's resistances are not public. Every hit then reads UNKNOWN and falls to
`ClosestSwing == true or HeroicStrikeQueued()`, whose first half compares against
a *function* and whose second is true whenever this very macro has queued a
Heroic Strike — and `ResetTimer(true)` refills the **off-hand** timer while
`st_timer` runs to 0, where the elapsed read is 100 for good. That loop is the
addon's own and reproduces on any client; what we must not do is enter it when
the player is two-handed, which is `has_offhand` alone, and his combat log says
two-handed: 2.70, 2.71, 2.72, 2.68 between consecutive hits, with no 0.2 s
off-hand pairing anywhere.
189 failures, unchanged, no regressions.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn
|
||
|
gromandClaude Opus 5
|
76e022ba78 |
Two real macro-window bugs, and three expectations the realm had moved on from
The macro window's four reds were four separate things, two of them bugs a player meets. **Every macro made through the icon chooser had a blank icon.** This realm rewrote `Blizzard_MacroUI` to keep bare upper-cased icon names in its own `MACRO_ICON_FILENAMES` — `UpdateMacroIconFilenames` strips `INTERFACE\ICONS\` off `GetMacroIconInfo` — and `MacroPopupOkayButton_OnClick` hands that name straight to `CreateMacro`. `icon_path` took any string as a finished path, so `GetMacroInfo` answered `ABILITY_AMBUSH` and `MacroFrame_Update`'s `macroIcon:SetTexture(texture)` resolved nothing. The engine composes the prefix itself: `Interface\Icons\` is at `0x44ca64`, inside `UIMacros.cpp`'s own string block between the `DeleteMacro`/`GetMacroInfo`/`GetNumMacros`/`CreateMacro` registration names and `MACRONODE`, which is what says the macro code owns it. A number is still a chooser index, and a string that already carries a separator is still a path. **Selecting a macro with no icon raised and took the whole details pane down.** `Blizzard_MacroUI.lua:84-85` feeds `GetMacroInfo`'s texture to `SetTexture` and to `strupper` with no guard, so a nil answered there is `bad argument #1 to 'strupper' (string expected, got nil)` inside `MacroFrame_Update` — no editor, no body box, no character counter. The reference cannot be answering nil, or its own UI would raise the same way: a macro with no icon of its own now reads as the client's own `INV_Misc_QuestionMark` (`0x447fe4`, same UI block), which is also the name the realm's chooser puts in row 1. The other three were stale readings of a file the realm has rewritten, each checked against the chain copy rather than guessed: - the character counter is `MacroFrameCharLimitText:SetText(this:GetNumLetters() .. "/" .. this:GetMaxLetters())` in the realm's own XML — "21/510", with no `MACROFRAME_CHAR_LIMIT` and no "Characters Used" tail; - the chooser shows `GetNumMacroIcons() + 1` rows, because `UpdateMacroIconFilenames` inserts `INV_MISC_QUESTIONMARK` at index 1 ahead of anything the engine offers, so every list index is one past the engine's; - both tabs call `PanelTemplates_TabResize(0)` with no cap and read `text="GENERAL"`/`text="CHARACTER"`, where stock passed -15, capped tab 2 at 150 and formatted the player's name into it. That test was *about* the name, so it now checks the row is independent of it instead. 195 failures from 203, no regressions (`comm -13` over the sorted names). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
gromandClaude Opus 5
|
f3fec804eb |
Two addons another player asked for, and the Linux-only gap that killed one
AlternativeClassicActionBars and UnitPlates, both now in `other-addons/`
(untracked — third-party source, not ours to vendor). Measured with the survey
and the read-back probe on a root holding only those two, so neither borrows the
other's globals. Both already loaded clean; three things were missing, and the
first was costing an entire addon's keybinding surface.
**A binding table spelled `bindings.xml`.** `ui_script::addons` asks for
`Bindings.xml` by the reference's own fixed name — `0x51f400` reads the name,
not a `.toc` entry — and the open was case-sensitive, so on Linux ACAB's 69
`<Binding>` rows never registered. Silently: an absent binding table is the
normal case, so nothing raised and nothing warned. Hover-bind, edit-mode escape
and every bar page key were dead. The texture and font side of the same file
namespace has always resolved case-insensitively (`loose_addon_file`, which
walks the path a component at a time and enumerates a directory when the spelling
does not match); the Lua and XML side now shares one resolver with it, and both
crates' private `read_under` copies delegate to it. Measured after: 69 bindings
listed by `GetBinding`, `SetBinding("CTRL-9", "ACABBIND1")` round-trips, and the
addon's drawn-frame count went 42 -> 175, because its login sequence had been
stopping short of its own bars.
**`PixelUtil`.** Called unguarded on ACAB's bar-geometry path at four sites,
three of them the same question — one screen pixel in UI units, which is what
every pixel-perfect border in that addon is drawn from. Not in ClassicAPI's
published list, but ACAB disables itself without ClassicAPI, so on this server's
kit it is present by implication. A Lua chunk beside `math_ext`, whose `Round` it
uses. One deviation, written down where it lives: upstream reads
`region:GetEffectiveScale()`, which a 1.12 texture does not answer, so the scale
read falls back to the parent and then `UIParent` — the same number in every case
that matters.
**`UnitXP("nop", "nop")` answering the boolean.** The presence probe.
SuperCleveRoidMacros keeps only `pcall`'s `ok`, but ACAB's `HasUnitXPSP3` wants
`ok and result == true`, and the fall-through handed it the stock XP getter's
`0` — truthy, so it passed the loose reading and failed the strict one. ACAB
reported UnitXP_SP3 absent on a client that answers the whole family.
Left alone with the reason in `docs/ADDON_COMPAT.md`: `IsStackableAction`
(post-1.12, guarded, and stock 1.12's own ActionButton.lua shows a count off
`IsConsumableAction` alone), `CursorHasMacro` (post-1.12 and redundant — ACAB
tests `GetCursorInfo() == "macro"` first), and five third-party tables that are
missing because those addons are not installed.
The harness grew a `--plates [n]` step, because a nameplate addon is the one kind
it could not see working: the subject is a `WorldFrame` child `vplates.rs` mints
per frame, so with none seated the addon's scan finds nothing, does nothing, and
reports a clean load over an empty screen. It seats n live plates and counts how
many an addon hung a frame on, which doubles as the contract check on the plate's
own shape — a plate that stopped being a `Button`, or stopped putting its border
texture first, would read as an addon adopting nothing. UnitPlates adopts every
plate and reads its name, health and level off ours.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn
|
||
|
gromandClaude Opus 5
|
1b1a90cf13 |
A swing speed of 0 latched for the session, and [noslamclip] never fired
grom's own probe is the record of it:
st=0 max=0 pct=nil win=0 idx=1
Four readings of one cause. `UnitAttackSpeed("player")` answered `0` between the
VM's birth and the first `feed_char` push, and that window always contains the
addon walk — `load_ingame_ui_on_world_entry` mints the VM, seeds the player's
roster record and runs every addon's file scope and `ADDON_LOADED` before a
Bevy feed system gets a turn. SP_SwingTimer latches `st_timerMax` there behind
`if not st_timerMax`, and `0` is truthy in Lua, so the guard never re-fires:
the first write to that global in a session is always the pre-push answer.
With `0` latched, `CleveRoids.GetSwingPercentElapsed()` answers nil (its own
`attackSpeed > 0` gate), `GetSlamWindowPercent()` answers 0, and
`ValidateNoSlamClip()` takes its "requires SP_SwingTimer" branch and returns
false. So `[noslamclip]` is false and `[slamclip]` is true for the rest of the
session, and the macro picks Heroic Strike every time with no indication —
which is exactly what he reported. `idx=1` in the same probe says the
`stance:1/3` half was never the problem.
I had called this latch faithful, on the grounds that `reference/1.12-shapes.tsv`
has `UnitAttackSpeed` returning exactly 2 values and never none. That is about
the arity and says nothing about a `0`, which is a swing speed no 1.12 client
can answer: the player's UNIT_FIELD_BASEATTACKTIME is in the create block the
real client has before `UI_Init`, and a unit with no weapon still carries the
engine's 2000. `ui_char::base_swing` already wrote the rule down — "0 or absent
read as not yet streamed: the vanilla 2000, never zero, as the reference divides
damage by it" — and `UnitCombatStats::default()`, which *is* the pre-push
answer, broke it. Now both read `VANILLA_SWING_MS` from one place.
It was never only this macro: BetterCharacterStats' DPS rows divide by the same
number and read `inf` in that window.
The bench test that documented the latch as harmless now pins it as the defect,
and asserts what the stand-in buys: a drained swing timer measures 100% elapsed
rather than nil, `[noslamclip]` is false out of combat by the window math rather
than by the addon's own bail, and nothing tells him in chat to install the addon
he already has.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn
|
||
|
gromandClaude Opus 5
|
998ab3f2f0 |
Two one-cause clusters: the pet bar never loaded, the auction window never built
**The pet bar, nine tests.** No script errors, and the shelf simply never drew. PetActionBarFrame had no scripts attached at all: the kit seats a bare stand-in under that name from the managed-positions pass — which it must, since the pass does PetActionBarFrame:IsShown() (UIParent.lua:2065) — and the real frame could never take the name back, a publish to _G being non-overwriting (0x701bd0). So PetActionBar_OnLoad never ran, showgrid stayed nil instead of 0, and ShowPetActionBar's `showgrid == 0` gate refused forever. The kit was loading BonusActionBarFrame.xml, whose ShapeshiftBar_OnLoad calls that pass, *before* PetActionBarFrame.xml. The realm's FrameXML.toc has them at rows 83 and 84, the other way round. Loading them in toc order fixes eight; the ninth asserted stock 1.12's unguarded `Attack ()`, and the realm now wraps that append in `if ( key ~= "" )`, so the client was right. Same shape and the same seat list that cost 137 tests through ChatFrame1. **The auction window, fourteen tests.** The whole Blizzard_AuctionUI load raised: the realm's rewritten CalculateAuctionDeposit is `floor(price * (duration/120) * (1 + (maxStack - stackSize) * 0.05) * 0.025)`, and GetAuctionSellItemInfo answered the reference's six values — 1.12 returns exactly six — so maxStack was nil and the arithmetic raised out of AuctionFrameAuctions_OnLoad. The window never built. It answers eight now: maxStack from the template's stack size, and the link. Then the realm's own CalculateAuctionDeposit replaces the engine binding, as it is meant to, which is why that test's numbers move — its fixture also needed Linen Cloth's real stack of 20, since a missing one reads 1 and makes the factor shrink below 1 for a two-item stack. Also recorded, because it changes what every earlier count in this session meant: `cargo test --workspace` stops at the first failing binary, so the "96 failures" I had been tracking was benilla-app's lib target alone. With --no-fail-fast the real total was 257; these two clusters take it to 243. benilla-ui is fully green (2061), and 80 of the rest are the known benilla-formats data goldens. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
gromandClaude Opus 5
|
9d0b4d068a |
The stock chat bubble drew under pfUI's, because its art had no handle
His screenshot shows two plates: a rounded grey one with pfUI's square box laid across the middle of it. pfUI's bubble module blanks every Texture it finds in the bubble's regions and lays its own backdrop over the result (modules/bubbles.lua:30-48). Our bubble held its two art paths only in the Backdrop struct, so the loop found the tail and nothing else. The note in this file reasoned that pfUI's own box covers the plate anyway — it does not: pfUI anchors that box 10 px wider and 10 px *shorter* than the bubble and fills it at 75% alpha, so the stock plate goes on showing above and below, through a translucent square. That is exactly the photograph. The background and the border are now also Texture regions, listed first as the reference lists a frame's engine regions first. They are anchorless and draw nothing themselves; the backdrop still paints. drive() reads them back each frame, and a region that has been blanked or hidden clears that half of the backdrop — never restored, since a skin blanks a pool slot once and keeps managing that frame for the session. This is the reference's own shape rather than a deviation from it: a 1.12 backdrop's pieces are CSimpleTexture objects the frame owns, which GetRegions (0x773f60) walks. Keeping the geometry in one struct is our shortcut; these put the handles back. The transcribed-pfUI test now expects three textures instead of one, and goes on to assert the thing that actually matters — after the blank, the extract carries none of the bubble's own art. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
gromandClaude Opus 5
|
fff48cd75c |
The sell-junk diagnostic sat behind the gate that fails
He hit sell junk again and the log said nothing — not because the sweep had nothing to report, but because the sweep never ran. pfUI's Sell Grey Items button is `if GetNumJunkItems() <= 0 then return end` and only then SellAllJunkItems (modules/autovendor.lua). That is the one path he clicks, so a zero count ends the click there. Last session's instrument lived inside SellAllJunkItems, downstream of the thing that was answering zero, and it could not be reached. Both now share one junk_survey of the carried bags, so the count and the sweep can never disagree about what they are looking at, and each says its half: GetNumJunkItems, when it answers zero while occupied slots have no resolved quality, says so and names them — a truthful zero stays silent; SellAllJunkItems says what it queued as well as what it could not judge; and ui_merchant's drain now warns, rather than debugs, when a queued slot resolves to no item guid. That was the last silent place the click could die, and debug does not reach his log. So whichever of the three branches is at fault, his next click answers it. Also checked and cleared on the way: the drain's slot.saturating_sub(1) is right — the feed keys container slots 1-based. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
gromandClaude Opus 5
|
7ebae975d9 |
A recipe never names the spell it teaches, so "Already known" never fired
He is still told he can learn recipes he already knows. The check was there; it was written against a shape no recipe in the game has. Read off his own item cache, the realm's Plans: Copper Chain Vest (3609) carries exactly one spell: 3349, at trigger 0. The rule looked for trigger 6 carrying the taught spell. Neither half matches — and 3349 is not the craft spell either, it is a learn *wrapper* whose single SPELL_EFFECT_LEARN_SPELL teaches 3321. Nothing in the item record names 3321 at all, so no check that reads only the item can ever answer this question. The app now makes the hop while it has the catalog in hand, through SpellCatalog::learned_spell, and hands the result over as ItemTemplateView::teaches. The tooltip reds ITEM_SPELL_KNOWN when any of those is in the known-spell set. The trigger-6 arm stays for an item that really does carry its taught spell. The old test passed throughout, which is the part worth keeping in mind: its fixture was invented — trigger 6, spell 3330 — and 3330 is Silvered Bronze Shoulders, nothing to do with this item. It now uses the realm's own record, and a second test pins the 3349 -> 3321 hop against the shipped Spell.dbc so the fixture can never drift from the data again. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
gromandClaude Opus 5
|
0da710810e |
An EditBox's engine quads answered nil, and ate the trade window's money row
His trade window has no money amounts: one empty box where the gold/silver/ copper row belongs. The cause is four regions that answer the wrong thing. pfUI skins that row by walking the gold EditBox's regions for the first Texture whose texture names "MoneyIcons", showing it, and re-anchoring it to the right of the box — then the silver box hangs off that icon and the copper box off the silver one (api/ui-widgets.lua:1225, api/api.lua:1693). Its scan reads a nil GetTexture() as "no name to match against, take this one". An EditBox's first four regions are the engine's own: the three selection highlights and the caret, which GetRegions lists ahead of the authored <Layers> ones. They paint host-side here, so they carried no texture and no fill and answered nil — and pfUI took the caret for the gold coin. Silver and copper then anchored off a zero-size region and the whole row collapsed. The reference builds those four as colour-set textures, not empty ones, so GetTexture answers them "Solid Texture" — the string is in WoW.exe, and pfUI's scan does filter that out. They answer it here now. Nothing renders differently: the host still draws the highlight and caret from sel_start / sel_end / caret_shown, and the fill stays unset. The new test runs pfUI's scan itself rather than asserting on our side of it, because what matters is which region that loop picks. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
gromandClaude Opus 5
|
0c698b8797 |
Hovering a sign re-derived the whole layout graph, every frame
grom reports a frame drop on hovering signs, and asked for that one first. A signpost is the only GameObject whose plate follows the cursor — GO_TYPE_GENERIC with floating_tooltip, the `cursor_seated` leg of drive_mouseover_tooltip — so every frame the hand on the mouse moves, GameTooltip is re-anchored. That re-anchor called the whole-graph Model::touch_layout for a write that moves two offsets and nothing else. Measured on his own interface, not guessed at: a new bench window prices a held plate against a drifting one against no plate at all. A drifting pointer cost +8.6 ms a frame over the baseline; a held one cost +0.0, so the plate is not the cost, the re-anchor is. It is now +0.2 ms. tooltip_unit::seat_on_cursor is the shared seat for both arms (the raise in world_tooltip_gameobject and the follow in world_tooltip_move). An offset-only move names the frame — touch_layout_frame, exactly as SetPoint's own write_anchor does for a dragged window — and only a roster or target change gives up naming. The bench asserts the ratio rather than the absolute, so a loaded machine scales every window together, and reads GetLeft at the end: a "fix" that stopped the plate moving would price identically, and this test would otherwise applaud it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
gromandClaude Opus 5
|
b5259bd608 |
The missing item values were eight absent id getters, not the tooltips
grom reported no item values on quest-reward tooltips. The value line is pfUI's, and `modules/sellvalue.lua` says plainly how it works: it hooks every `Set*Item` tooltip verb and takes the row's **id** — never its link — then asks `C_Item.GetItemSellPriceByID`. An absent getter answers nil and `AddVendorPrices` returns on its first line. So it was never about quests. This client answered exactly two members of that family, `C_Container.GetContainerItemID` and `GetInventoryItemID`, which is why his bag and equipped items had a price line and his quest rewards, loot rows, loot rolls, merchant rows, mail attachments and trade-skill rows did not. The other eight are here: `GetQuestItemID`, `GetQuestLogItemID`, `GetLootSlotItemID`, `GetLootRollItemID`, `GetMerchantItemID`, `GetInboxItemID`, `GetTradeSkillItemID`, `GetTradeSkillReagentItemID`. Each sits beside that row's own `*Link` getter, in that row's own module, reading state already held — no new feed, no new state. The id is also the **earlier** fact, which is the part worth keeping in mind. An item id rides the packet; a link is built only once the item's template has landed to give it a name and quality, and `ui_quest::resolve_item` says so in as many words. A quest reward is close to the worst case for that — it is usually an item the player has never seen, so its template is not in the 14,000-row cache and the first hover beats the answer home. These getters answer there, where the link beside them is still nil; the test asserts exactly that contrast. Also, from the same thread of play reports: the sell-junk sweep now says what it could not judge. `SellAllJunkItems` and `GetNumJunkItems` both gate on a *known* poor quality, and a carried slot's quality is `None` until its template arrives, so a grey sitting in such a slot is neither counted nor sold — which is what "not all grey items sell" looks like from outside. Whether that is in fact the cause is not something this end can tell, so the sweep records the slots it skipped for that reason, with their item ids, once per click. If the next log names some, the fix is to wait for those templates; if it names none, the cause is downstream in the drain or the server's refusal, and the line says that too. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
gromandClaude Opus 5
|
c8c4e7f855 |
Four defects out of his log, and the 65 warnings that are not ours
His 19-minute session put 8 errors on screen from two causes, and 73 WARN lines of which 8 are ours. The whole triage is written down in the play-test memory, because most of the log is the addons' own and nothing should re-chase it: five files their `.toc`s name and do not ship, a QuickHeal template that is never defined, an ATSW global string that only exists in its deDE branch, inspect frames from a load-on-demand addon with no `## Dependencies` line, `<Binding>` elements the frame loader is *supposed* to log, and a world-map lag line that reported its own recovery 5.2 s later. Four of the eight are fixed here. `GameTooltip:SetSpellByID` was nil, and it is not a keypress: the traceback is pfUI's `ButtonEnter` into `SuperCleveRoidMacros/Core.lua:4217`, so it raised on every action-bar hover of a macro that resolves to a spell — 7 times, each leaving the button with no tooltip at all. `compat/classic_api/tooltip.rs` had declined the whole `Set*ByID` family pending "an argument-shape check against the stock verb each aliases". Done, and the check is why this is four lines: there is no aliasing to do, because `tooltip_spell::set_spell_by_id` is the body all four of 1.12's spell verbs already funnel through, and it takes a bare id — the book slot or list row is what *they* have to convert first. `show_rank` is the action's rather than the spellbook's, because the book draws "Rank N" itself as the button's `subSpellName` and an id-named caller has no such label. The caller's own comment says it reaches this only for a spell in neither spellbook, so the cold-view path is the common one here; that needs no special case, since the app's feed builds a view for any id in the catalog and an action button re-asks its tooltip every `TOOLTIP_UPDATE_TIME` while hovered. `resolve_spell_fx` despawned an entity the world no longer had. `FxInstance`'s root is a *stored handle*, and `attach_spell_fx` is the system that notices one lost with the unit's whole visual and re-arms or drops the instance — so between that loss and that pass, the same-slot replace, the spell-id reap and the self-termination clock all hold handles to entities that are gone. `try_despawn` at those three, as `lifecycle.rs` already does; `tend_world_plants` keeps its plain `despawn` because its entity comes from a query, not from a record. The first paste of every Wayland session could never have worked. `smithay-clipboard` sets `latest_seat` only on a `wl_keyboard` Key/Modifiers event or a pointer press (`state.rs:522`, `:386`) — `Enter` sets `has_focus` and nothing else — and refuses to load a selection without it. `HostClipboard` opened its backend on first *use*, so the backend that read the clipboard was created by the very Ctrl+V asking for it and its keyboard object was younger than that chord's own key events. It is now opened as soon as the window can give the `wl_display`. Costs +2 on the `Update` ambiguity ceiling, both pairs against the schedule's two exclusive `&mut World` systems, which pair with everything they have no declared order against. And the one `changed_windows` ERROR a login prints had no remedy behind it: no client can warp a free pointer on Wayland, so the loading screen's cursor hand-back now asks only where the answer can be yes — off Wayland, or with the pointer locked, which is winit's own rule. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
gromandClaude Opus 5
|
b57f49faa8 |
fmt: blank lines left by the dropped trainer tests
Three tests the #1947 conflict hunk carried in from other upstream commits were dropped; their removal left a triple blank line. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
Sam SungandClaude Opus 5
|
f0adc9a3ba |
GetTrainerServiceInfo answers "" for a rankless service's subText (#1947)
`GetTrainerServiceInfo` answered nil for a rankless service's subText. In 1.12.1 the subText is the Spell record's NameSubtext (`0x4d8b50`, read at `0x4d8b84`), the empty string for a spell with no rank; only a service with no Spell record answers nil (`0x4d8b58`). The resolver now keeps an empty rank as `""` and the binding pushes None as nil. Tests cover both rows and the install-backed Apprentice Blacksmith trainer. Supersedes #1935. Co-authored-by: kalistoking <148718830+kalistoking@users.noreply.github.com> 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit fdbb4bb070382a6cf6b30bdaf33214c972cd02f4) # Conflicts: # crates/benilla-ui/src/script/trainer/tests.rs # docs/DEVIATIONS.md Absorbed into the octo fork. Mostly already true here: this fork's GetTrainerServiceInfo already answered "" for a rankless service, with its own note on the stock window's unguarded format. What is kept is upstream's test for it and the field doc. Two tests the conflict hunk dragged in from other upstream commits are dropped: a_service_row_answers_one_for_its_fourth_value and service_description_is_nil_without_text need struct shapes this fork lacks (an Option description). A third, a_service_without_a_spell_record_answers_nil_for_its_subtext, is dropped because it contradicts our deliberate choice — we answer "" for a missing Spell record too, and that is now recorded as a Deviation on the field rather than left as an undocumented difference. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
andi79h-claudeandClaude Opus 5
|
5fa31a073b |
IsRaidOfficer answers 1 for an assistant or the leader, as 1.12.1 does (#1894)
It was a nil stub, so an assistant got no raid target icons in the unit popup (UnitPopup.lua offers RAID_TARGET_ICON to the leader or to IsRaidOfficer) and the raid frame's add-member button stayed off for them. 0x4bb910 walks the raid roster (0xb712a8, count [0xb713e0]) for the row whose guid is the local player's and pushes 1 when that row's rank ([row+0xc], GetRaidRosterInfo's return 2) is above 0, so for an assistant and for the leader; outside a raid, without a player or without our row it answers nil. Verified: cargo test -p benilla-ui --lib (is_raid_officer_reads_our_own_roster_rank), clippy -D warnings. Built on macOS only. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit ed630ffb784a4038a0aee343fb909d35147aa388) # Conflicts: # crates/benilla-ui/src/script/party.rs Absorbed into the octo fork. Adjacency only: our SuperWoW SetRaidTarget flag test sits immediately before the test upstream retitled, so both are kept. The binding change applies unchanged; party:: tests green (34). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
badmagick329
|
4c98a8d981 |
GetTradeSkillNumMade answers 1, 1 on a header or past the rows (#1459)
`GetTradeSkillNumMade` answered 0, 0 for a header row or an index past the rows. 1.12.1's `GetTradeSkillNumMade` (`0x4fdc50`) sends a bad index, a header (spell -1, `0x4fdcaa`) and a missing spell record to the fallback at `0x4fdd13`, which pushes 1 twice. A recipe with no spell record never becomes a row in benilla, so the past-the-rows case covers it. The miss now answers 1, 1. Tests: the header, no-window and past-the-rows cases pin 1, 1. Fixes #1262 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit 129b3bb852b2f328de529f3c698595c14baf5fa5) |
||
|
gromandClaude Opus 5
|
44aac335fd |
classicapi: the last two with data behind them — item sets and area triggers
Both over DBCs 1.12 reads C-side and publishes to Lua nowhere, so both
are facts no addon on this realm could reach.
`C_Item.GetItemSetInfo`: the tooltip prints a set block and `GetItemInfo`
reports no set at all, so nothing can tell which pieces complete one. The
table's shape is its caller's own documentation (`SuperCleveRoidMacros/
ClassicAPI.lua:171-174`) — including the bonus pair being
`{ spellID, threshold }`, which is the REVERSE of the DBC's column order,
so the swap happens once in the host and not in every reader.
`C_Map.GetAreaTriggerInfo`: pfQuest's own comment says it reads trigger
geometry from this "instead of a shipped coordinate table", and the three
fields it takes — areaID, mapX, mapY — go straight into `pfMap:AddNode`,
whose coordinates are 0-100. So the percentages are the consumer's
measured shape rather than a guess. The zone resolution is the host's,
through the same `zone_uv` law the map window's own hover uses
(`WorldMapUiData::zone_at`), so a trigger lands in the zone the map would
name for that point — quirk included: a point exactly on a rect's east or
south edge reads as outside, which is that law's own `(0, 0)`. The three
map fields are absent TOGETHER when the centre resolves to no zone (an
instance interior), which is exactly the case pfQuest guards for.
Both ride host probes rather than pushed tables: the resolution needs the
`WorldMapArea` rects, which do not cross the seam, and neither catalog is
something to copy per VM for a verb nothing calls at boot. Two catalogs
gained an `iter()` for it, each said to be for this.
That leaves four of the original 56. Two have no caller in the corpus;
`C_FriendList.SetFriendNotes` has no note field on the 1.12 wire at all,
so a note typed and lost at logout would be worse than a button that does
nothing; and `C_BattleNet.SendGameData` stays deliberately absent, because
every caller guards it and registering it would claim a BattleNet this
client does not have.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn
|
||
|
gromandClaude Opus 5
|
546e60765d |
classicapi: fourteen members over four namespaces — the measured tail down to six
Of the 153 `C_<Namespace>.<Member>` names grom's addons reference, the fresh VM was missing 56; this takes it to 6. Fourteen members here, each over data or a stock verb that already existed: `C_Spell` gains the four melee-timing predicates and `IsSpellInRange`. 1.12 answers all of those only about an ACTION BAR SLOT — `IsAttackAction` and `IsAutoRepeatAction` — so a spell the player knows but has not placed on a bar cannot be asked at all, which is the gap a condition engine keeps running into. The three attribute answers ride named booleans on the spell lookup rather than a raw attribute word, so the bits and their reference citations stay in `benilla-formats` where they can be checked; `do_not_reset_combat_timers` is named there for the first time, beside the `ranged_speed_cooldown` that already tested the same bit. The melee auto-attack is an ID (6603) rather than an attribute, because no bit marks it — the ranged half has the auto-repeat flag and the melee half is that spell. `C_SpellBook.GetSkillLineName` is the most-read missing member of the whole family (nine call sites) and is one delegation to `GetSpellTabInfo`. `IsAutoAttackSpellBookItem` resolves by id against the same 6603, so the two cannot disagree about what auto-attack is. `C_Item.GetCurrentItemLevel` answers a number 1.12 prints in a tooltip and returns from nothing, so no addon can compare two pieces of gear. `RequestLoadItemDataByID` is the modern async load over the ask-once queue `ItemMixin:ContinueOnItemLoad` already drives. `GetWeaponEnchantInfo` is a pass-through. `C_QuestLog`'s abandon pair delegates to the stock two-step, so the mark, the still-in-the-log search and the send keep one owner — a namespaced abandon cannot drop a quest the stock path would have refused. The six left are each written down with what they need. Two are a host probe over a DBC `benilla-formats` already parses (`AreaTrigger` for pfQuest's trigger geometry, `ItemSet`), two have no caller in the corpus, `SetFriendNotes` has no note field on the 1.12 wire at all — a note typed and lost at logout is worse than a button that does nothing — and `C_BattleNet.SendGameData` stays deliberately absent, because every caller guards it and registering it would claim a BattleNet we do not have. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
gromandClaude Opus 5
|
7bfdefb206 |
classicapi: C_EncodingUtil for real, and two namespaces 1.12 has no subsystem for
`C_EncodingUtil` is the one namespace in this family that is pure algorithm, which is why it is built properly rather than stubbed: all eight members, in the REAL formats. pfUI's profile sharing is the reader and its own header names the pipeline — SerializeCBOR → CompressString (zlib) → EncodeBase64, prefixed `!pf1!` — with the export path NOT inside a pcall, so clicking Export raised, and the import path inside one, so it failed silently. A profile string is pasted between players, so a blob this client writes has to be readable by the real one and the other way round. So: zlib through flate2 (already the workspace's inflate for MPQ sectors and its compressor for CMSG_AUTH_SESSION's addon block, so this adds a crate to one more manifest and not to the build), RFC 4648 base64, RFC 8949 CBOR, RFC 8259 JSON. Nothing invented here — the trap a hand-rolled codec falls into is round-tripping with ITSELF while being wrong. The decoders are written for hostile input, because a profile string arrives pasted from a forum: every length is checked against what is actually left, nesting is capped, the inflate output is capped at 16 MiB, and a malformed blob answers nil rather than raising or allocating what a crafted header asked for. Tests run the RFCs' own vectors in both directions — including the half-float and tag forms we never write but a real encoder may — a truncated/overlong/deep-nested set, and pfUI's whole export/import pipeline end to end. `C_EquipmentSet` and `C_LootHistory` are subsystems 1.12 does not have at all, and both answer an honest nothing rather than being absent, because pfUI builds both windows and reaches one member of each OUTSIDE its own gate (the equipment manager's `CanUseEquipmentSets` test is one line in one addon; the loot window's Clear button has no test at all). What each would take is written down rather than guessed at. Equipment sets are persistent state 1.12's wire has no concept of — a feature to decide on, not a gap to close, and pfUI already supplies the UI. The loot log is closer than it looks: both packets already carry what the getters answer and `ui_loot_roll.rs` handles them, so it is a bounded list plus three events — but the getters' tuple order is the one thing here that IS a guess, read out of pfUI's own destructures, and building the log would make that guess load-bearing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
gromandClaude Opus 5
|
65609f6db4 |
classicapi: C_ChatInfo and C_LossOfControl — two namespaces that were raising
Measured the gap rather than trusting the stale sweep in the doc: of 153 `C_<Namespace>.<Member>` names grom's addons reference, 56 are missing — 24 where the namespace exists and the member does not, and **32 where the whole namespace is absent**, which is the dangerous half. An absent namespace is not a missing feature: `C_Foo.Bar(...)` raises. Two of those are indexed with no guard at all: - AceComm-3.0's ChatThrottleLib takes `_G.C_ChatInfo.SendAddonMessage` out of the table before calling it (`:638`, and `:653` for the logged twin), so every addon-message send through the throttler raised. Two of its other reads DO guard, which is what made it easy to miss: the library tests for the namespace in one place and assumes it in another. AI_VoiceOver's comms ride this. - `SuperCleveRoidMacros/ClassicAPI.lua:237` reads the loss-of-control count with `or 0` — which guards the result, not the namespace. Its own comment four lines up says "Returns 0 for a client without C_LossOfControl", which is what the author intended and not what the code does. `API.GetSchoolLockout()` is reached from the conditionals. `C_ChatInfo` delegates to the stock globals, so the distribution whitelist and the usage errors keep one owner. It adds exactly one behaviour: **a lane 1.12 does not have is dropped, not raised.** AceComm's whisper lane calls it with "WHISPER", the stock binding answers the reference's own `Unknown addon chat type` raise, and a raise inside the throttler takes down the queue carrying the message. Of raise / misdeliver to the party / drop, dropping is the only one that neither stops the caller nor sends addon data to an audience that did not ask for it. Which lanes exist is asked of the binding's own table, not copied. `C_LossOfControl` answers an empty list, which is the answer its caller was written to handle. What a real one needs is written down: the only member with content on this wire is the school interrupt, and a lockout arrives as ordinary cooldowns on every spell of the locked school — so recognising one means deciding a batch is a lockout rather than a coincidence, and a wrong guess makes those conditionals refuse to cast. Also the seven missing `C_AddOns` members, all delegations. The enable state takes its arguments in the namespaced order, measured rather than assumed: AI_VoiceOver aliases the two forms and writes the swap out at its own call site. It answers 2 or 0 and never 1, because 1.12 keeps one enable bit per addon with no per-character dimension to be partial over. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
gromandClaude Opus 5
|
38a6b6f149 |
unitxp: inSight and behind — four macro conditionals that silently never worked
Not a gap: a defect. Defining `UnitXP` at all is what makes
SuperCleveRoidMacros believe the whole family is present —
`hasUnitXP = pcall(UnitXP, "nop", "nop")` (`Init.lua:274`), which
succeeds here because the dispatcher's fall-through does not raise — so
four macro conditionals have been running against commands this client
did not implement: `[insight]`, `[noinsight]`, `[behind]`, `[nobehind]`,
each with a count mode too.
The fall-through answers the stock `UnitXP(unit)` getter's 0, and every
one of those conditionals compares against `true` LITERALLY (`== true`,
`~= true`). So `[insight]` and `[behind]` could never fire while
`[noinsight]` and `[nobehind]` always did, with nothing to notice it by.
That is why both commands answer a boolean rather than 1.12's `1`/nil
flag shape, and why a pair they cannot measure answers nil rather than a
fabricated `false`: the two directions are read separately, so "cannot
say" has to be distinguishable from "no".
`inSight`'s blocker is gone, and it was gone before this: the row said
"needs a real raycast with no current binding-layer access", and the
nameplate-occlusion work built exactly that raycast. This is the same
body-layer ray from the player's chest instead of the camera's eye, fed
into the per-token `UnitReach` record `distanceBetween` already reads —
only the app can trace the world, which is the whole reason the command
sat blocked in the binding.
`behind` this file called *unconfirmed* ("treat as unconfirmed until seen
elsewhere"). It is now seen elsewhere: ten call sites, with the argument
order documented by its own readers, so `behind(A, B)` asks whether A
stands in B's rear arc. The arc is the rear HEMISPHERE, which is what
this realm's server means by behind — vmangos tests `!HasInArc(M_PI,
attacker)`, the same rule that accepts or refuses Backstab — taken flat,
so a ledge above a mob's back counts and directly overhead does not.
Two things my own tests caught, both worth keeping:
- A two-argument `inSight` answered `true`. These are recognised
commands, so they now match on the NAME alone and say nil when the
arguments make no pair; the arity fall-through is right for an
unrecognised command and wrong here, because neither name is a unit
token so the stock getter can only answer a meaningless 0 — which is
truthy in Lua, i.e. a confident "yes, in sight".
- A `#[cfg(test)]` module placed mid-file hid every `fire_event` after it
from `event_shape_gate`, which truncates each file at the first one:
six unit-field bridge events read as unproduced. The test moved to the
file's own tests module, where the one-test-module-per-file convention
that gate assumes puts it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn
|
||
|
gromandClaude Opus 5
|
1e9ef80c41 |
superwow: pin the bubble's seat, the one placement that could be silently wrong
The widget tests proved the frame's size, its alpha and its text; none of them would have noticed an anchor on the wrong point, which would still size and draw — just in the wrong place. The bubble hangs from its bottom centre on the projected point and grows UPWARD (`0x4b0c30`), so the test now reads `GetCenter`, `GetBottom` and `GetTop` back and pins all three. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
gromandClaude Opus 5
|
434d514879 |
superwow: the chat bubbles are widgets — CREATE_CHATBUBBLE and pfUI's skin
The last real SuperWoW feature, and one piece of work closed three things: `CREATE_CHATBUBBLE` (which could not honestly be fired, because its payload can only be the bubble), `C_ChatBubbles.GetAllChatBubbles` (which answered an empty list, truthfully, for the same reason) and pfUI's whole bubble skin, which walks that list. `benilla-ui`'s `script/chat_bubble.rs` is a pool of real frames — anonymous WorldFrame children with a backdrop (`0x4b0a35` IS `SetBackdrop`), a tail texture and a FontString, which is what the reference's own `CGChatBubbleFrame` is and how addons of the era found bubbles before SuperWoW published the event. The app keeps everything a bubble's existence depends on — the spawn gate, the lifetime, the fade, the range re-test, the stacking sort and the text measurement that sizes the frame — and hands it over as `BubbleState`: the same split `vplates.rs` already makes for the plates, so the engine decides what is on screen, the widget layer decides what Lua sees, and neither can disagree about a bubble because there is one feed. Three things improved by the move rather than being preserved by it: - The bubbles sit at frame levels 2, 3, 4… over the V-plates at level 1, which is the reference's own order (`0x4b12d5`-`0x4b1312`). The quad band had them UNDERNEATH — a deviation `overlay_z` carried in a comment. That band is gone and the lane is down to one tenant. - The art is loaded, tiled and bleed-inset by the UI pass like any other frame's, so this file drops its own texture resource and its own half-texel inset helper. - The system moved BEFORE the paint. It sat in `UiQuadAppend`, the window *after* the extract: right for an appender, a frame late for a feed. The test that matters transcribes pfUI's own `ScanBubbles` loop — it finds the FontString with the line and its chat-type colour, blanks the one Texture region and parents its own frame to the bubble — and then checks the EXTRACT for drawn quads: both backdrop halves, the tail, and the text at the em the app measured the box with. A test that only checked the widgets would have passed with nothing on screen. Two architectural gates caught this and both were answered rather than silenced. The one-shot gate: `drive_bubbles` now holds the VM and drains a queue, so it is EXEMPT with the argument — the queue's product is app-side state, the widget sync is idempotent and binds on the first frame there is a WorldFrame, and gating the system instead would pile lines up and spawn them all at once with fresh lifetimes. The VM wall: its ceiling rises by one with the reason, which is that this file is `vplates.rs`'s exact twin. `CREATE_CHATBUBBLE` fires per APPEARANCE, not per pool slot ever built — the opposite of `NAME_PLATE_CREATED`'s rule, because a recycled frame has been re-anchored, re-sized and re-texted since it was last seen. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
gromandClaude Opus 5
|
bead2cb22b |
superwow: Clickthrough is live — the last function on the DLL's manifest
It had been stored and read by nothing, which is "a knob with two positions and one picture". Now the world pick looks through a dead unit that holds no loot for us, which is what SuperAPI's own description asks for: "click through corpses to loot corpses underneath them" — the pile after an AoE pull, where the one body you want is behind four empty ones. The test is `CanLootUnit`'s own, negated (`clickthrough_skips`), so "lootless" means one thing to that verb, to this pick and to the loot sparkle; and because `UNIT_DYNFLAG_LOOTABLE` is per-viewer on the server, a corpse somebody else may loot is lootless TO US, which is exactly the pile the feature is for. A living unit is never skipped whatever its flags — that would make mobs unclickable — and a unit whose descriptor has not streamed is not known to be empty. The flag is mirrored out of the VM once a frame rather than read in the pick, which is a `SystemParam` with no reach into the script; a one-frame lag on a preference set once is immaterial. The resource is `Option` in the pick, like `factions` beside it: a harness that drives this pick builds the world it needs and nothing more, and absent means off, which is also the default. The stale aggregate row this closes is rewritten rather than deleted, because what it got wrong is the lesson: it called six functions "blocked — each needs new app-side state", and three of those blockers were answered by data the tree already had. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
gromandClaude Opus 5
|
0e02401325 |
superwow: CastSpellByName at a unit, FoV, NameplateMotion, the raw log
The DLL publishes its own **usage strings**, in one block beside the names it registers, and reading them turned up four things this file had wrong — three of them bugs rather than gaps. `CastSpellByName(name [, "unit"])`. A unit ID, which is what every real call site passes and what `Model::unit` already resolves. It used to fall through to the ordinary queue, which casts at the player's OWN selection — a silent wrong target at six sites in grom's corpus: pfUI's /castfocus (a GUID) and /castmouseover (a token), Decursive's cure-at-guid, BigWigs' spell requests and two encounter scripts. Each asked for a unit and got whatever was targeted. The stock `onSelf` meaning keeps its behaviour: only a string that resolves to a live unit takes the new path. `CombatLogAdd`'s second argument names a FILE, not the event. There is a third log literal in the same pool — `Logs\WoWRawCombatLog.txt` — and it is the GUID-form log, the same lines RAW_COMBATLOG carries as arg2. `ui_chat/logging.rs` keeps it beside the ordinary one now, opened by the same `LoggingCombat`, which also makes the arg2 composition worth paying for when nothing is registered but the log is on (`wants_raw_form`). My first reading, "also fire the event", was an inference from the name alone, and the second literal settles it. `FoV`'s hold-back is resolved, and the answer was already in the tree. The row was held because 1.57 looked irreconcilable with CAM_FOVY's 0.785. It is the same number: 1.57 is π/2, the reference's own `[camera+0x40]` default of 90° in radians, and that field is not a vertical FOV at all — `scoped_view.rs` already divides by it as a DENOMINATOR, because what it expresses is a ratio. So upstream's own default is this client's existing lens by construction, which is what the test asserts; the DLL even registers the literal `1.57079`. Live, where upstream's panel says "Requires UI Reload". `NameplateMotion` asked for an investigation that was also already answered: benilla spreads plates with the reference's own SmartScreenRect solver. 0 (Overlap) bypasses it, 1 (Default spread) is it; 2 and 3 are the mod's own algorithms and fall back to 1, a documented silent-equal rather than a refused row. Also: `GetTradeSkillItemLink(index[, spellLink])`, the only way to link a recipe that makes no item — an enchant, whose product_item is 0 and which answers nothing at all on the stock surface. And four stored-only rows (SelectionCircleStyle, UncapSounds, HealingText, InterfaceVolume), each with its own reason, so SuperAPI's options panel round-trips. Three ghosts retired: outline/shadow text, force-show-auras and XP text are in no part of the DLL's string pool, nor is NameplateVerticalSpacing. They came from the early wiki reading, not from the binary. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
gromandClaude Opus 5
|
323f3ae03c |
superwow: the map pair, GetQuestLinkForID, GetWeaponEnchantID
Four more off the DLL's manifest, and two measured corrections. GetMapPositionWorldLoc is the INVERSE of GetPlayerMapPosition, which is what the test asserts rather than three numbers: `map_proj::zone_world` is the reference's `0x4a7100`, whose own note is that used one axis per call it inverts the `zone_uv` that verb answers from. The law is transcribed into benilla-ui (which cannot depend on benilla-world) and the round trip is welded, which is the thing that actually keeps the two in step. GetMapBoundaries shares one new `worldmap::loc_rect_at` with the hover grid's own windowing, so no two readers of a (continent, zone) selection can disagree about which rect it means. GetWeaponEnchantID is the one thing GetWeaponEnchantInfo cannot say: the reference reads that very dword as its gate and then reports only that there WAS an enchant, so nothing on the stock surface can tell Windfury from Rockbiter. The id was already in hand and discarded as a `?` gate, so this was one field on the snapshot, plus a test that the two functions cannot disagree about which hands are enchanted. GetQuestLinkForID answers for a quest in the log and nil otherwise — which is also what upstream answers on the first call for one it fetches. The gap is one feed, not new protocol: the ask-once CMSG_QUEST_QUERY cache already exists app-side. That row also corrected the LINK FORMAT for both quest getters. The DLL's own formatter is `|cffffff00|Hquest:%d:%d|h[%s]|h|r`; ours was built without the colour wrapper on the reasoning that it matched the stock quest-log shift-click — but 1.12 has no quest links at all, so there was no stock shape to match and the measured string is the only authority. The four sound-driver getters are deliberately NOT built, with the reason recorded: every one of their return shapes is unknowable from here, none is called in the corpus, and this client has one backend and no enumeration to report — the same bargain KEY_DOWN/KEY_UP lost. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
gromandClaude Opus 5
|
400457ff82 |
superwow: TrackUnit/UntrackUnit, and the four chat-bubble CVars
TrackUnit is the one with a live consumer and a real feature behind it: 1.12 draws party members on the minimap and raid members NOWHERE, and pfUI puts the whole raid on it with these two — for every party and raid token on PARTY_MEMBERS_CHANGED/RAID_ROSTER_UPDATE/PLAYER_ENTERING_WORLD, each inside a pcall, so on a client without the function the feature is simply absent, which is what it has been here. The set is guids, not tokens, so a tracked raid member keeps its dot while the roster shuffles its token out from under it. The dot itself is drawn by a fourth always-show clause beside the reference's own three (`shows_tracking_dot`) — `tracked_creature` stays exactly `0x5ed210`, so the predicate that file documents is still the client's, and a tracked unit gets the ordinary cell, hover name and distance cull with nothing new drawn. The set is read off the VM handle the blip system already holds rather than behind a resource of its own: `BlipInputs` is at the 16-param ceiling and its last element is already a nested pair for that very reason. The bubble four come straight off the DLL's string pool. Three are nampower's `NP_ChatBubbles*` under SuperWoW's unprefixed names, writing the same fields — two mods, one knob, last writer wins, as the nameplate reach already is — each keeping 0 because the reference bubbles none of those kinds. `ChatBubblesCreatures` is SuperWoW's alone and the only behaviour of the four: it NARROWS `ChatBubbles` rather than standing beside it, because the mod hooks the reference's own gate and a switch added at that site can only subtract; the other reading would bubble creatures with the master switch off. It ships 1, so an untouched client bubbles monster say and yell exactly as 1.12 does. All four are welded to their struct literals in the defaults-mirror test. CREATE_CHATBUBBLE is deliberately NOT fired, and the row says why: its payload can only be the bubble, and this client's bubbles are not Lua frames — the same wall `C_ChatBubbles.GetAllChatBubbles` already answers an empty list for. Argless would be worse than silence, since an addon written against upstream reaches for arg1's regions and raises once per bubble. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
gromandClaude Opus 5
|
ac857ada36 |
superwow: CanLootUnit, GetSpeed and UnitNameplate
Three of the DLL's own manifest, and two of them needed no new feed at all — which is the point worth recording, because the row that called this family "todo, blocked: each needs new app-side state" was wrong about two of the three. `CanLootUnit` is the loot sparkle's test read through the token map: `UnitState` already carries `dead` and raw `dynamic_flags`, so a corpse that sparkles answers 1 and there is one source of truth for "has loot for me". The bit is per-viewer on the server, so "can" means can. `UnitNameplate` goes through the SAME `plate_frame` wrapper as ClassicAPI's `C_NamePlate.GetNamePlateForUnit`, welded by a test that asserts both names hand back one object — an addon may feature-detect either, and two different frames for one plate is the kind of bug that only shows up as an anchor in the wrong place. `GetSpeed` is the one that needed a field: the posture snapshot gained `speed`, filled from `net::current_speed`, the reference's own `CMovement::GetCurrentSpeed 0x7c4c90`. What could not be settled is written down rather than guessed over — upstream publishes no prose and nothing in the corpus calls it, so "current" is a reading, taken because it is the more informative of the two and because it is the only function in the client named after speed that a hook would return. Standing still it is 0, as that getter is. `SelfMovement` loses its `Eq` (an f32 field); nothing compared two snapshots — the feed assigns a fresh one each tick. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
gromandClaude Opus 5
|
3b6e91be35 |
superwow: RAW_COMBATLOG and CombatLogAdd, the only two names his addons call
Of SuperWoW's whole surface these were the two names grom's corpus calls
that this client lacked (`scripts/dll-surface.py --wanted`), and the first
is load-bearing out of proportion to its size: `SuperCleveRoidMacros`
funnels FIVE subsystems through one `RAW_COMBATLOG` frame — immunity and
resist learning, hidden-CC detection, debuff-duration learning, and the
reactive-proc window that lights Overpower and Revenge. Its own comment
says why the last of those can come from nowhere else: "SPELL_GO only
provides hit/miss COUNTS, not miss TYPES".
`route` is the fire site: the one funnel every combat line already passes
on its way to a window and to WoWCombatLog.txt. `arg1` is the printed
sentence; `arg2` is the same sentence with each unit name spelled as its
GUID, composed a second time through the same `compose_line` so the two
can only differ in the name slots. Fired before the `CHAT_MSG_*`, which is
what "raw" means upstream. Both the fire and the `arg2` composition are
gated on `has_listener` — a raid's combat log is tens of lines a second.
`CombatLogAdd` is also BigWigs' SuperWoW detection (`if CombatLogAdd
then`), so defining it turns that module's path on.
Three corrections came out of reading the DLL's own usage strings, which
it publishes beside the names — the arity and argument meaning of every
remaining SuperWoW function is now measured rather than inferred, and
`docs/ADDON_COMPAT.md` carries the manifest:
- `CombatLogAdd("text" [, addToRawLog])` has a SECOND argument. Nothing
in the corpus passes it; it would have been missed.
- `SUPERWOW_COMBAT_LOG_EVENT` does not exist. The DLL registers exactly
three events, and that name is in no part of its string pool.
- `IsIndoors` is capital-I beside `IsSwimming`/`IsMounted`, not the
lowercase `isIndoors` two files guessed at — all three are already live
off one feed, so that clause of the "blocked" row was already answered.
- The detection pair is `SUPERWOW_STRING="SuperWoW 2.2 by Balake"` and
`SUPERWOW_VERSION="2.2"`, both quoted. Ours had the string without its
author clause and the version as a number.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn
|
||
|
gromandClaude Opus 5
|
a9bb9624e9 |
app+ui: nampower's press-interception queue — the half that changes how keys feel
The half of upstream's queue that `queue.rs` has said for three rounds it deliberately left out. A press the client would refuse for a **timing** reason is now held in the same slot an explicit `QueueSpellByName` uses, and the drain sends it the moment the guard clears: no packet, no global cooldown, no error line while it waits. One queue, because upstream has one — a second set of slots beside the first would be a different feature wearing the same name. **The guard reopens early, and that is the headline.** `PendingCast` kept one timestamp and now keeps two. The guard reopens at the cast's own already-known length plus `NP_MinBufferTimeMs`; the record still lives for the cast plus two seconds, so a resolution arriving a round trip later is still recognised as the answer to that cast. That split is not tidiness — the `modalNextSpell` chain that starts a hunter's Auto Shot reads the record, and a record that died with the guard would break shooting. The buffer is not a latency figure, which is why upstream's default is 55 ms and not a ping: our packet reaches the server half a trip after the send, the server finishes its cast half a trip after our own cast time elapses, so a next cast sent at exactly `cast_time` arrives just as the server frees up and the latency cancels out of the arithmetic. The buffer is jitter margin on top. `NP_NonGcdBufferTimeMs` is wider because a non-GCD spell has nothing *but* the guard between two presses. **`NP_RetryServerRejectedSpells` is the mitigation for that**, and that is why upstream defaults it on. If the margin is ever wrong the server refuses the next cast and the press is simply gone — the dropped cast the early guard buys its speed with. It goes back once, held a buffer's worth, on exactly two codes: 0x3c not-ready and 0x61 another-action. Every other refusal is the server telling the player something true, and re-sending one would be a packet a second that never succeeds. **What a window measures, and how that was decided.** Upstream publishes four and no sentence saying. Two readings fit: how long an entry survives, or how close to the guard's end a press must land. This takes the first, because `queue.rs` already reads `NP_SpellQueueWindowMs` that way and one number meaning two things in one client is a trap nobody finds twice; because `Replacing queued nonGcd spell X with Y` means the last press wins, so holding every press costs nothing in staleness; and because the other reading would make the 250 ms cooldown window refuse almost every press made during a global cooldown, which is the complaint nampower exists to answer. Two rows do read a window as remaining time, and both say so themselves. `NP_InterruptChannelsOutsideQueueWindow`'s name states one. And `NP_CooldownQueueWindowMs` has to be one: a cast ends in a second, a Hearthstone cooldown is half an hour, and holding that press means either firing it half an hour later or swallowing it with no error line. A quarter of a second is an allowance for pressing a touch early, not a queue. **Where the rung sits.** Five rungs stay above it — the profession opener, the auto-repeat re-press, the dead caster, the targeting cursor's clear and the on-swing re-press — because each is what the press *means* whatever is in flight. Everything below is a real refusal a player should read: no reagent, out of range, no power, stunned, moving, wrong form. Upstream hooks the cast entry and holds those too, so its error arrives a second later about something the player stopped doing. Refusing them now is the deviation and the better half of the trade. Fourteen CVar rows went live. What stays stored is named with its cost: the dynamic buffer optimizer and its ceiling (off in upstream's own defaults), `NP_OnSwingBufferCooldownMs` (nothing to rate-limit — a re-press of the queued strike already takes the reference's silent bail), the channel latency trim, and the three quickcast rows, all off by default and all needing the world point under the mouse in the cast path. `CastSpellNoQueue`/`CastSpellByNameNoQueue` became meaningful: they take `CastLadder::send_now`, which skips the rung. The queue's own pop takes it too — a pop that could be re-held is a loop that never casts anything. The test worlds now seed the **registered** defaults rather than `Default`'s all-zeroes, which is a configuration nobody runs: three ladder tests were passing only because the queue was off in them. Each now asserts the queue behaviour and keeps the reference's 0x61 in a world where a player unticked the queue. And one test drives the whole cycle through the real drain — press held, nothing on the wire, guard cleared, one packet out — because the failure this guards against is a keypress that produced no packet, no error and no cast, which is the one a player cannot describe. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
gromandClaude Opus 5
|
68f397af72 |
app+protocol: nampower's combat-log family — 35 events off one decode
Every packet this family reports was already decoded: `combat_log/` builds its chat lines from exactly these structs. `ui_np_events.rs` is a second consumer of that decode, not a second decoder, which is also why its arguments cannot drift from the chat log's. Built: `SPELL_DAMAGE_EVENT_*`, `AUTO_ATTACK_*`, the three `SPELL_HEAL_*`, the three `SPELL_ENERGIZE_*`, `SPELL_MISS_*`, `DAMAGE_SHIELD_*`, `ENVIRONMENTAL_DMG_*`, `SPELL_DISPEL_BY_*`, `UNIT_DIED`, the `*_UPDATE_DURATION_SELF` pair, the thirteen `UNIT_*_GUID`/ `PLAYER_GUILD_UPDATE_GUID` rows and `UNIT_COMBAT_GUID`. **A registration is the switch.** Upstream's README calls every `NP_Enable*Events` CVar deprecated — "registering the event is preferred" — and three of the eleven families have no CVar at all, so a CVar-only reading would leave `SPELL_DAMAGE_EVENT_*` unreachable for every addon and `SPELL_ENERGIZE_*` unreachable for all of them, since nothing installed writes its CVar. `refresh_listeners` reads the VM's own registration table (the same one `GetFramesRegisteredForEvent` reads), so a family is wanted when its CVar is set OR something is listening. That is also what keeps the cost honest: with nobody registered a damage packet queues nothing, rather than building eight arguments for a fire that reaches no frame. Nine `NP_*` CVars move from stored-only to LIVE, and `NP_EnableSpellEnergizeEvents` is registered for the first time — the first pass took the names off `NampowerSettings`' writes rather than off the DLL's own CVar table. The two reasons written at the auto-attack and heal rows were both about the *cast* path, and both were answered by the combat-log decode that was already there. The descriptor indices the GUID-keyed family watches are written out and then cross-checked against the two tables the tree already keeps — `unit_rec::FIELDS` and the `FIELD_UNIT_*` constants. A hand-written descriptor index is exactly the number that is plausibly wrong and silently so: the event simply never fires, which no amount of play distinguishes from "the server never sent one". `AttackerState` gains `sub_damage_count`, which is `AUTO_ATTACK_*`'s arg6; vmangos writes one block, but a server splitting a swing across schools sends more. Two test notes worth keeping: the edge tests run through the schedule, because `run_system_once` builds a fresh system each call and the `MessageReader` cursor restarts — a test written that way passes whatever the system does with the second edge. And the data-gated suite held at exactly 269, after the event gate correctly named all 35 new names. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
gromandClaude Opus 5
|
702eb475a4 |
app+ui: the queue, the disenchant sequencer — 62/62
QueueSpellByName, QueueScript, ChannelStopCastingNextTick, DisenchantAll. Every one of nampower's 62 registered Lua functions is now defined; the `comm -23` in the doc prints nothing. **Upstream's queue has two halves and one is deliberately not built.** The Lua surface — remember this spell, send it when the client will accept it — is `spell/queue.rs`. The other half intercepts *every ordinary press*, so a keypress during a cast is held rather than refused, and it is governed by NP_QueueCastTimeSpells, NP_QueueInstantSpells, the buffer rows and the quickcast pair. That half is not a binding: it changes how every key in the game feels, its bugs are double-casts and dropped casts, and it is not something to switch on as a side effect of porting three function names. Those twenty CVars stay stored-only, each row saying so; the machinery they would sit on is now here, and turning it on is its own piece of work with its own live test. Two of the twenty-two went live, because they are the explicit queue's own knobs: NP_SpellQueueWindowMs and NP_QueueSpellsOnCooldown. Three slots, because upstream has three and its own event says so: SPELL_QUEUE_EVENT's code is 0/1 on-swing, 2/3 normal, 4/5 non-GCD, so the queue is keyed by what kind of cast a spell is — and the kind comes off its own Spell.dbc row through the same three tests the send path makes. Each pops on its own condition: non-GCD and on-swing as soon as no cast guards (neither uses the global cooldown), normal when the GCD has run out too. Three decisions at the queue, each pinned: - A replacement raises no popped event. A second queue of one kind replaces the first — upstream's own log line is `Replacing queued nonGcd spell X with Y` — and the replaced entry was never sent, so a consumer counting pops against queues would see one too many. - An expiry raises nothing either. `*_POPPED` means "this was sent". - The expiry is measured from the end of the cast in flight, not from the moment of queueing, so queueing early during a long cast is not punished. A lapsed deadline cannot shorten the window. **`DisenchantAll` is specified completely by the DLL's own strings**, which is why it is built this way rather than designed. `0x93087` onward carries the argument note, the precondition, the three quality keywords, every stop reason and the three chat lines — one of them as a Lua chunk that builds the item link itself. "move during cast to cancel" is the abort, and it needed nothing new: moving already cancels a cast here, the cast then fails, and a failed cast stops the run. The one piece of machinery a sequencer usually needs is the client's own, and the player's way out is the one upstream documents. How a step's outcome is read: the item is gone, or it is not. There is no "disenchant succeeded" packet — the item leaves the bag. Each step remembers the guid it cast at and waits for it to vanish with a ten-second deadline. An observable test with no new wire plumbing, right for the same reason the cast bar's own edges are. The queue's drain and the sequencer both live inside `drain_spell_casts`, which that file's own note explains: only one system may hold `CastLadder`, because a second would share its resources with every other cast-send system in an order the ambiguity gate cannot know is safe. `spell/queue.rs` names no VM — the first draft mentioned `UiScript` in one comment and `ui_script_wall.rs` counted it as gameplay code naming the VM, which is the gate working as intended: that module really is gameplay state and not a feed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
gromandClaude Opus 5
|
b652c90a4b |
app+ui+protocol: the four measured-out names, and the unit descriptor by name
Each of these had a reason for being out. Each reason turned out to be
answerable, and two of them were simply wrong.
`GetSpellPower(school)` is **delegated to `GetSpellBonusDamage`**, which
this client already answered — one number under two mods' names, as
SuperCleveRoidMacros says itself ("Reads the same client field
nampower's GetSpellPower does"). Delegating is the only way to guarantee
they cannot drift, and the test asserts equality across all seven
schools and the off-range values rather than a literal. The earlier
"needs a descriptor band fed" was wrong: it was already fed.
`GetRaidTargets()` is the 8-slot **board**, now pushed beside the party
roster. The earlier reason was right about the scan it rejected and
wrong about the alternative: `GroupState::raid_targets` is the
reference's own table and copying it costs eight words a push. A mark
with nothing on it is absent, because a zero GUID is a string that
passes `if t[4]`.
`LearnTalentRank` is one `LearnTalent` press per missing rank, which is
what a player clicking the button does. The earlier reason — the burst
cannot be checked before it is sent — is true and is not a reason to
withhold it: the check that matters is the server's and the client has
never had another. What can be checked locally is, so a rank already
held queues nothing and one past the maximum is clamped.
`GetUnitData` is the whole descriptor, from a new raw block on
`UnitState`. The earlier reason — "a decision about which fields" — is
now a table of seventy columns under the DLL's own names, read off its
string pool in UnitFields order; `GetUnitField`'s scalar half reads the
same block, so the two cannot answer differently for one name, and a
test walks the table asserting exactly that.
**The indices are derived and the derivation is checked.** Only thirteen
`FIELD_UNIT_*` constants existed here; the rest come from 1.12's block
sizes. Those thirteen span index 22 to 147 and every one lands where the
layout puts it — thirteen independent confirmations, run as a test
rather than asserted in a comment, with two more that no columns overlap
and that nothing falls inside the aura blocks' 47..=124.
Three decisions at that mapping:
- An unstreamed column is absent, not zero. `ObjectFields::at` is
`get_raw`'s strictness rather than `get_u32`'s absent-is-zero
convenience: `health = 0` for a unit whose health has not arrived is a
number a caller divides by.
- `aura`/`auraApplications` come from the filtered per-unit aura list,
not the raw slots — the same place `GetUnitField` has always answered
them from. An unfiltered second answer would be a different unit by
the same name.
- `status` and `zoneId` are left out of nampower's seven computed
columns: nothing says what `status` is a status *of*, and a zone is
only knowable for the player. The other five are filled by the caller,
which knows them.
`SpellRecValue` is now `RecValue`: it serves both record readers, and a
second copy of the same four shapes would drift.
58 of 62 registered names. The four left are `DisenchantAll` and the
queue trio.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn
|
||
|
gromandClaude Opus 5
|
e716709bca |
app+ui: the nine file and credential verbs, fenced and keyed
Built on grom's explicit say-so, after a round where they were withheld and the reason recorded. Both halves of that decision stay in the doc, because the reasoning is what makes the second half a decision rather than a drift. ReadCustomFile/ImportFile (one operation, two upstream names), WriteCustomFile/ExportFile, CustomFileExists, ExecuteCustomLuaFile, StartItemExport, EncryptPassword, EncryptedServerLogin. **The fence is app-side and it is not a `..` scan.** `local_state::custom_file_path` admits one relative name whose every component is an ordinary name, inside `benilla-config/CustomFiles/`, and refuses a leaf that is already a symlink. Each escape a string check misses has its own rule and its own test row: an absolute path (`join` *replaces* the base rather than appending), a Windows prefix, a `..` nested inside a longer path, and a planted symlink — the one no amount of sanitising the string will catch. A subdirectory rather than the config root, because a verb that could create `config.toml` or `autologin.toml` by name would let an addon rewrite the client's own settings; that is a different power from "keep a file of your own". The VM side holds three closures and no paths, so no spelling of a `filename` reaches outside that directory. **The credential pair's deviation is forced, not chosen.** Both are registered by nampower under "Registering *Glue* Lua functions" and this client has no glue Lua VM at all — login, character-select and character-create are Rust. `EncryptPassword` is pure and works the same anywhere. `EncryptedServerLogin` cannot mean "submit the login form" in the only VM there is, so it stores the credential for the next login: `enabled` on, the plaintext field cleared, the blob kept, and `AutoLogin::effective_password` reads it back at the refill. What that protects is an `autologin.toml` or a SavedVariables file read by someone else or pasted into a forum: the blob is authenticated and keyed on 32 random bytes in a different file, mode 0600. What it does not protect is anything running as this user, which can read the key as easily as the blob — there is no user secret in the chain and there cannot be, because the point is logging in without typing one. Both halves are at the module. Encrypt-then-MAC with HMAC-SHA1 as the PRF for both halves, from the two primitives the tree already had (sha1 moves from a dev-dependency to a real one). The HMAC is tested against RFC 2202's own vectors, because a hand-written one still round-trips with itself when it is wrong — and a first draft of the key reader did exactly that kind of silent wrong: it returned raw bytes on creation and hex text on read, and the caller guessed between them with "32 bytes and not all hex digits". It guessed wrong the moment a random key came out all-hex. Returning one thing is the fix, and the note is at the function. **The existing plaintext path is untouched.** Storing the password in plaintext was grom's own explicit choice when the alternatives were a keyring or nothing, so this is additive and nothing migrates by itself. The machinery to make the Remember Password tick-box write a blob instead is now all here — one line — and that is his call. Base64 was missing from the whole tree; it is in `benilla-ui`'s `script/encoding.rs` with RFC 4648's own vectors, and `C_EncodingUtil.EncodeBase64` will want the same one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
gromandClaude Opus 5
|
f87ef148b8 |
app+ui: the trinket cooldown — and a stock verb that always said "cold"
Built: GetTrinketCooldown, UseTrinket, GetSpellRangeData, SetLocalRaidTargetIndex, GetQuestLogQuestIds, GetQuestDialogQuestId, CombatLogFlush. The find on the way through is not a nampower verb. **`GetInventoryItemCooldown` is a stock 1.12 verb and it answered `(0, 0, 1)` unconditionally** — right shape, always "cold", so nothing errored and no worn trinket ever drew its cooldown swirl on the character pane. Stock PaperDollFrame.lua:691 calls it on every slot update. Feeding an equipped slot's cooldown for GetTrinketCooldown fixes that one off the same column, and the test runs `CooldownFrame_SetTimer`'s own arithmetic against it. The feed keys its query on the (use spell, item entry) pair exactly as the bag feed does, so one trinket reads the same cooldown from the doll and from a bag. **The risk was the gate, and my first test for it was vacuous.** `feed_char` sits behind a change gate and no descriptor moves when a cooldown is armed, so `Cooldowns::feed_epoch` had to become a gate input. The first test passed with that input deleted: `run_system_once` builds a fresh system each call, so the gate's `Local` memo resets and `vm_reset` opens the gate every time. A test for a gate input has to run the system the way the schedule does. Rewritten with `add_systems` + `update`, it fails with the input removed — verified by probe, and the reason is written at the test. `feed_char` also hit Bevy's sixteen-parameter ceiling, which was a fair signal its parameters wanted grouping by what they are for: `ItemCooldownInputs` and `RepairInputs` replace six. Two decisions recorded at the code: - `GetSpellRangeData(rangeIndex)` answers three values, not a table. Every table-returning verb in this DLL has its key names in the string pool and this row's are not there. `flags` bit 0 is the melee family, where the engine takes the combat-reach sum instead of the authored pair — a caller ignoring the bit reads 0..5 for every melee ability. - `CombatLogFlush()` is a deliberate no-op. Upstream buffers its combat-log writes and this empties the buffer, so what an addon asks for is "my lines are on disk now" — and `ui_chat/logging.rs` writes each line to an unbuffered File as the window shows it, so that holds at all times. Defining it is the honest answer: the capability is present, and a missing global makes `HasFeature` report it absent. Also fed, both one wire column each: `QuestState::quest_id` (the wire names the quest on all three giver panels and 1.12's Lua names it nowhere — a title is not a key) and `SpellDisplay::stack_amount`'s neighbour work from the previous commit. Five names stay out, each now with the field list that makes it a decision rather than a shrug — `GetUnitData` (80 raw descriptor columns, read off the pool, zero call sites, and a partial answer is worse than none), `GetRaidTargets` (the reference keeps a board; this client feeds the mark per unit, and a scan over named tokens would silently omit a marked unit), `DisenchantAll` (a server-paced sequencer, which is the queue), `GetSpellPower`, `StartItemExport` (writes a file — it belongs with the withheld seven). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
gromandClaude Opus 5
|
f06caa9968 |
ui+app+formats: the Spell.dbc row, and seven columns that answered nil
`GetSpellRecField` knew three names. Counting what the corpus actually asks it for: `name` (4 sites), `school` (3), `stackAmount` (2), `spellIconID` (2), then one each of `rank`, `rangeIndex`, `mechanic`, `manaCost`, `effectMechanic`, `effectApplyAuraName`. Seven of those ten answered nil, and each was a feature quietly off rather than an error — DoiteAuras caps its stack display on `stackAmount` (DoitePlayerAuras.lua:539) and SuperCleveRoidMacros classifies a crowd-control effect off the two arrays (Utility.lua:7776-7794). The eighty field names are the DLL's own, read off its pool in Spell.dbc column order. The mapping to `SpellDisplay` lives in `ui_script/spell_rec.rs`, beside the struct it reads — a copy on the VM side would be free to drift — and `GetSpellRec(spellId)` is that same field list walked through that same per-field reader, so the two verbs cannot answer differently for one name. A test asserts exactly that, key by key, over one row. The array columns are a 1-based table of three, which is measured, not guessed: SuperCleveRoidMacros walks `effectMechanic` as `for i = 1, 3 do local em = effectMechanics[i]`. `totem`, `reagent` and `reagentCount` take the same shape, two and eight long. One new DBC column — `StackAmount`, 39, bracketed by two constants this tree already had (38 ModalNextSpell, 40 Totem[0]). It is the only one added: fifteen further names upstream answers are left nil rather than putting fifteen unused u32s on a struct the spell catalog holds one of per row. They are listed at `FIELDS` with that reason, so the gap is a decision rather than an omission. `GetSpellRecField`'s three string columns keep the spellbook fallback the verb had before the probe, so a VM with a book and no catalog — a capture, or the boot VM before the catalog loads — still answers a learnt spell's name and rank. Every other column is a DBC row and has nowhere else to come from, which is why the fallback is exactly three names wide and says so. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
gromandClaude Opus 5
|
cf0e157f34 |
ui+app: the bag walk, the modifier fold, and two shapes the DLL's pool corrected
Built: `GetBagItems`, `GetSpellModifiers`, `GetSpellIdCooldown` — plus
fixes to two verbs that already existed and had the wrong *shape*,
found the same way `GetItemIconTexture`'s wrong argument was: by
comparing what we answer against nampower's own string pool.
`GetBagItems()` hands back every occupied bag slot as
`{ [bagIndex] = { [slot] = row } }`. DoiteAuras asks seven times over
and walks both levels with `pairs`, reading `itemId` and `stackCount` —
it is how that addon answers "do I have this item" without 100
`GetContainerItemInfo` calls per condition evaluation. Empty slots and
empty bags are absent, because a nil value in a Lua table *is* absence
and `pairs` is how both levels are read.
`GetSpellModifiers(spellId, modifierType)` answers
`flat, percent, hasModifier`, where percent is the final multiplier (90
means 90% of the original) — the consumer says so in its own words and
multiplies by `percent / 100`. The 26 modifier types come from the
DLL's usage string. The fold is `spell/mods.rs`' existing reader,
reached through a new `spell_modifier_probe` the lifecycle reinstalls
whenever the wire moves the tables: a VM-side reimplementation of a
four-gate 64-bit fold would be free to drift from the mana cost the
action bar greys a button on. The third return is the point — the
consumer tests it before touching either number, and the duration path
multiplies, so `0, 0` for an unmodified spell would zero every debuff
timer that asks.
`GetSpellIdCooldown(spellId)` answers the twenty-key table, from a new
`Cooldowns::legs`: `info`'s three legs kept apart, same walk and same
arms, so the folded number a swirl draws and the legs an addon reads
cannot describe different timers.
**`GetBagItem`'s row had the wrong key names** — `bag`, `slot`,
`count`, `quality`, `link`, `locked`, this client's own
`GetContainerItemInfo` spellings. nampower's row is contiguous at
`0xa8a68` and is the block `GetEquippedItem` already used, so a caller
reading `.stackCount` got nil and no error. One `bag_slot_table` now
serves both, with three columns absent rather than zeroed: this client
resolves a bag slot's enchants to names at the app boundary and keeps
no random-properties id for a container slot, and a zeroed enchant id
would be a claim.
**`GetItemIdCooldown` returned the wrong kind of value** — the stock
`start, duration, enable` triple. At the version we report it is a
table, and the addon layer settles it: `API.GetItemCooldownInfo` and
`API.GetSpellCooldownInfo` return these calls' results *directly* and
their callers index them. `cd.gcdCategoryRemainingMs` is the primary
path of `API.GetGCDRemainingMs`, which the `[gcd]`/`[nogcd]` macro
conditionals divide by 1000 — against a triple that is
`(0.0).gcdCategoryRemainingMs`, which raises. Latent rather than live
(nothing calls those wrappers today), still wrong.
Two things the schedule gates caught and improved:
- The probe refresher started as an exclusive system, which is
ambiguous against everything in Update it is not ordered against:
+274 pairs over the ceiling. `NonSendMut` is all it needs.
- Its memo started as a bare `Local`, which `session.rs`' own gate
refuses — a memo about what this VM has been told must die with the
VM. As a `VmMemo` it now reinstalls on the first frame after a
`/reload`, which is better than the reasoning I had written for the
`Local`: a fresh VM has no closure at all until something installs
one.
One count the earlier sweep got wrong, corrected in the doc:
`GetSpellPower`'s "7 call sites" are BetterCharacterStats' own method
`BCS:GetSpellPower(school)`, not nampower's global. Nothing in the
corpus calls the global at all. A bare name match is not a call site.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn
|
||
|
gromandClaude Opus 5
|
ecee994ab6 |
app+ui: the player's own cast state, and SPELL_CAST_EVENT off the commit
`GetCurrentCastingInfo` and `GetCastInfo` are the two most-called
nampower functions in grom's corpus — 25 and 20 call sites outside
`NampowerAPI.lua` — and one of them calls with no existence guard at
all: `local _,_,_,_,_,_,autoattack = GetCurrentCastingInfo()`
(SuperCleveRoidMacros/Core.lua:94), in the frame that keeps clearing the
target while an autoattack is queued. On this client that line raised.
The earlier doc row said the client had "a spell id, a deadline and a
guard bit — no start instant, no channel state, no GCD end". That was
true of `PendingCast` and wrong about the tree: `ui_cast_events.rs`
already folds a cast window per caster off the same packets, with a
start, an end, a channel bit and the accumulated pushback, behind
`C_Spell.UnitCastingInfo`. What was actually missing was the caster's
own machinery — the reference's cast globals, which have no descriptor
field anywhere — and the GCD, which `spell/cooldowns.rs` has run
correctly all along and never published.
So `GetCastInfo` folds two halves: its timing columns come from the
player's own window, its identity and GCD columns from a new per-tick
`SelfCast` snapshot. The feed declares itself after the window drain, so
the two halves can never describe different casts. Added:
- `SelfCast` (benilla-ui) — the inflight spell `0xceca88`, the bar's
spell `0xceac48`, the auto-repeat key `0xceac30`, casting/channeling/
onswing/autoattack, the cast counter, the cast type, the target, and
the GCD's start and end on the GetTime clock.
- `ui_self_cast.rs` (benilla-app) — the push, plus `SPELL_CAST_EVENT`.
- `Cooldowns::gcd` — the running global cooldown as one timer. `info`
still answers the per-category question every cooldown swirl asks;
this answers "when does the GCD end", which has no spell to key on
because its consumers divide it (`API.GetGCDRemainingMs`) or compare
it against seconds (`[gcd:<1]`).
- `PendingCast`'s cast counter, in the one place every class of commit
already passes through.
Three decisions where the tempting answer was the wrong one:
- `GetCastInfo` is **nil** when nothing is casting, and an instant's GCD
does not count. Answering a table whenever the GCD is up would make
the consumer's fast path always hit — and `API.IsCasting()` *is*
`GetCastInfo() ~= nil`, so `[casting]` would read true for a second
and a half after every instant. The wrapper's own comment says what to
do instead ("GetSpellIdCooldown … works for instant casts").
- `castId` is two different numbers under one upstream name: a counter
in `GetCastInfo`'s table, beside a separate `spellId`; a **spell id**
in `GetCurrentCastingInfo`'s returns, which is how both of its readers
treat it (Init.lua:89 stores it as castingSpellId, Core.lua:5427 hands
it to C_Spell.GetSpellName). Each is implemented as what its own
consumer reads, and both are spelled out at the binding.
- `SPELL_CAST_EVENT` fires for commits, not refusals. Upstream's
`success` can be 0; this client's local refusals funnel through
`CastErrors`, which carries a spell id and a reason byte and neither a
cast type nor a target — a row from there would invent two of five
fields. Nothing in the corpus reads the failing branch
(Core.lua:5177 is one `if success == 1 then` with no else). Written
down at the type rather than left to be found.
Tests run the consumers' own bodies transcribed, as `cast_info.rs` next
door does: `API.GetCastInfo` with its `GetCurrentCastingInfo` fallback
arm, `API.IsCasting`, `API.GetGCDRemainingMs`, `API.GetCastRemainingMs`
and `CleveRoids.UpdateCastingState`. A positional seven-return shape and
a ten-key table are only correct relative to the code that reads them.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn
|
||
|
gromandClaude Opus 5
|
aab2130c62 |
ui+docs: nampower's own registration table, and the icon id it takes
`scripts/dll-surface.py` answers "what might this DLL publish". For nampower.dll there is a better question with an exact answer: the names it registers sit contiguously in the string pool, in registration order, bracketed by the DLL's own log lines — `Registering Custom Lua functions` immediately before the first, `Registering Glue Lua functions` before the last two. That is 60 functions plus 2 glue ones, with no inference at all, where the pointer filter gives a candidate list a reader still has to judge. The event table (59 names) and the CVar names sit the same way, just before it. A compiler emits one string literal per `lua_register(L, "<name>", fn)` and pools them in source order, so one registration function's names land in one run, and the log line that opens it is the literal printed immediately before the first call. Look for the log line, then read forward until the strings stop looking like identifiers. Two things fell out of the same pool that corrected code already written: - The 44 `Usage:` strings are the argument contract, printed verbatim when a call's arguments are wrong. `GetSpellRangeData(rangeIndex)` takes a SpellRange.dbc row index, not a spell id, which this doc had wrong; `GetSpellModifiers(spellId, modifierType)` enumerates all 26 modifier constants. - `GetItemIconTexture(displayInfoId)` takes an ItemDisplayInfo row id, the exact sibling of `GetSpellIconTexture(spellIconId)`. Ours took an *item* id and resolved the display id itself, which is a different function: upstream's addon layer keeps that hop in a wrapper of its own (`NampowerAPI.lua:2648` is the pass-through, `:2665` is `GetItemIconTextureByItemId`). So a caller handing us a real display id got "Item not found in DBCache", and one handing an item id got an icon the real DLL would not have answered. Fixed, and the test now pins that an item id answers nothing — it populates the item cache on purpose so the wrong reading would be visible if it came back. With the table read, the surface is no longer a count of what a heuristic flagged: 62 names, each in exactly one bucket (32 built, 8 withheld pending a decision that is not mine, 4 not ours to add, 18 to build), with the `comm -23` that reproduces it. The two at the head of the last bucket are the most-called nampower functions in the whole corpus, and they are coupled: SuperCleveRoidMacros registers SPELL_CAST_EVENT gated on `GetCurrentCastingInfo` existing, so defining the function without firing the event would move it from never registered to registered and silent. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
gromandClaude Opus 5
|
7861ab2710 |
ui: ClassicAPI's GetCraftReagentItemID
The one column 1.12's `GetCraftReagentInfo` does not answer: it gives the reagent's name, icon and the needed/held counts, and no item id at all, so nothing that keys a table on an item — pfUI's reagent counter — can use the stock verb. Same `(craftIndex, reagentIndex)` pair and the same rows that verb walks, so the two cannot disagree about which reagent is meant. Out of range on either index is nil, matching the stock verb's own nil-first return. All nine gates green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
gromandClaude Opus 5
|
de3a3411e7 |
ui: ClassicAPI's GUID player info, no-toggle cast and stable-id skill rank
Third DLL tranche, from ClassicAPI's side of the work list. Its 70 corpus-called names are mostly **not** functions — `Clear`, `Fire`, `Size`, `Armor`, `Nature`, `Normal`, `None` and the rest are widget methods and enum keys its DLL carries as strings, and several more (`GetSpellSchool`, `RunMacro`, `GetSpellLink`) the corpus defines for itself. Reading the call sites left nine genuine gaps; three are landed here and the rest need state this client does not feed yet. - `GetPlayerInfoByGUID(guid)` -> `localizedClass, englishClass, localizedRace, englishRace, sex, name, realm`. 1.12 has no verb that takes a GUID at all. Answered through the same `Model::unit` as the whole unit surface, so a raw GUID, a `0x`-prefixed one and every other token spelling resolve alike; an unknown GUID is seven nils, not a raise. `realm` is **nil** on purpose: the field means the realm the *named* character is from, which 1.12 has no concept of, and our own realm name there would be a guess that reads as data. - `CastSpellNoToggle(spellId)`: the plain cast by id. The toggle lives in the client's cast path and benilla's queue toggles nothing — a press is a send — so this is what the caller asked for, on the same queue a bar press uses. Same reasoning as nampower's `CastSpellNoQueue`. - `C_SpellBook.GetSkillLineRank(skillLineID)` -> `rank, maxRank`. 1.12 reaches a skill only by its index into the skills *window*, which moves as lines are learnt and categories collapse — a `SkillLine.dbc` id is stable, which is why pfQuest asks by one. It reuses `skills::displayed_ranks`, so the stable-id getter and `GetSkillLineInfo` fold bonuses and the mono override identically and cannot disagree. A line the character has no row for is `nil, nil`, not `0, 0`: untrained and unknown are different answers. All nine gates green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
gromandClaude Opus 5
|
801bc09c05 |
ui+docs: SuperWoW's quest-log getters, and the DLL work list worked name by name
`GetQuestIDForLogIndex(index)` and `GetQuestLinkForLogIndex(index)`: 1.12's `GetQuestLogTitle` answers the title and never the id, so neither the id nor a chat link for a log row is derivable from the stock API. The id reads the same `quest_log.entries` ClassicAPI's `C_QuestLog.GetQuestIDForLogIndex` does, so an addon that feature-detects one and falls back to the other gets one answer; a header row is id `0` and **no** link, rather than a link to quest 0. `docs/ADDON_COMPAT.md` now carries the whole work list with a status per name, and the most useful finding in it is not a missing function: `SuperCleveRoidMacros/NampowerAPI.lua` is a capability registry whose `HasFeature` checks the version **and** `_G[name] ~= nil`, so nothing on the list was broken and raising `GetNampowerVersion` would not have helped. Defining the global is the only honest way to make one live. Counts, so the list cannot be read as more alarming than it is: 16 done across this tranche and the two before it; 12 that were already present and only looked absent because the script compares against `dump_globals` on a bare VM (and because an *event* is never a global); 6 that need a feed, each with the specific thing missing; 3 that are the spell queue this client deliberately does not have; and 7 withheld. The seven withheld are `ExportFile`, `ImportFile`, `WriteCustomFile`, `ReadCustomFile`, `CustomFileExists`, `ExecuteCustomLuaFile` and `EncryptPassword`. Six of those give addon Lua read, write and **execute** on files under the client folder and the seventh takes a password. They are real in nampower and used for config export, but they are a different kind of surface from a getter and the right default for a client that did not have them yesterday is not to grow them quietly. `GetCastInfo` and `GetCurrentCastingInfo` are left out for a reason worth keeping: their consumers divide by the cast and GCD timings, and this client has a spell id, a deadline and a guard bit — no start instant, no channel flag, no GCD end. A table of three real fields and seven zeros is worse than the nil the addon's own fallback already handles correctly. 269 data-gated failures, identical to the baseline. All nine gates green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
gromandClaude Opus 5
|
5555bf9c89 |
ui+app: nampower's bag group, aura cancels, no-queue casts, unit-event CVars
Second tranche. Eight more verbs and six CVar rows, all from state this client already owns. The bag group — `FindPlayerItemSlot(idOrName)`, `GetBagItem(bag, slot)`, `GetItemIdCooldown(itemId)`, `UseItemIdOrName(idOrName)` — shares one lookup, deliberately: the three that search must agree about *which* of two identical stacks they mean, and they answer the first, reading bags and slots ascending, as the reference's own auto-store search walks (`0x4f8030`) and as a player reads their bags. A name matches through the item cache, case-insensitively, since a bag slot carries no name of its own. `GetItemIdCooldown` answers the stock `0, 0, 1` for a cold or uncarried item rather than nil, because the consumer does arithmetic on the triple. `UseItemIdOrName` calls `UseContainerItem` itself rather than reaching past it, so the cursor-clear and repair-mode rules the stock verb applies still happen. `CancelPlayerAuraSpellId(spellId, [ignoreMissing])` and `CancelPlayerAuraSlot(auraSlot)`: the two keys into the `CMSG_CANCEL_AURA` queue `CancelPlayerBuff` already fills, so the three cannot disagree. The slot is the **raw 0-based** descriptor slot upstream documents and the buff events report, not the Lua index. `ignoreMissing` is accepted and changes nothing — it asks upstream not to complain, and nothing here would have complained. `CastSpellNoQueue(spellId, spellBook [, unit])` and `CastSpellByNameNoQueue(name [, unit])`: the plain cast, which is the honest mapping and not a stub — benilla has no spell queue to bypass, so "cast this now, do not queue it" is exactly what happens. The point of them is the first argument: a **spell id** where stock `CastSpell` takes a book slot. `"mouseover"` routes to the queue SuperWoW's `CastSpellByName(name, "CLICK")` already uses. The six `NP_EnableUnitEventsX` switches are registered stored-only, one step further back than the four `NP_Enable*` rows beside them: benilla raises no GUID-keyed unit event at all, so there is no stream for a token filter to narrow. Registering them stops the unknown-CVar warning and lets `SuperCleveRoidMacros`'s writes persist; it claims nothing about the events arriving, and `cvars.rs`'s own reader gate carries that reason. All nine gates green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
gromandClaude Opus 5
|
f120e8003d |
ui: six of nampower's reads, and the item-icon probe behind two of them
First tranche off the DLL work list. The method that produced it is worth recording with the code: `SuperCleveRoidMacros`'s `NampowerAPI.lua` is a capability registry — a `VERSION_REQUIREMENTS` table of every nampower name with its minimum version, reached through `HasFeature`, which tests the version **and** `_G[name] ~= nil`. So none of this was broken, and raising `GetNampowerVersion` would not have helped: the only honest way to make one of those features live is to define the global. - `PlayerIsMoving()`, `PlayerIsRooted()`, `PlayerIsSwimming()` — `1` or nil, as upstream's own doc lines state, never a boolean. All three read the per-tick posture snapshot `SelfMovement` already carries, which gains `rooted` (`MOVEFLAG_ROOT`) and `moving` (any of the six movement bits, the keyboard turns included — they are input to the same word the client tests, and a turn in place does interrupt a cast). - `IsAuraHidden(spellId)` — from a new `SpellLookup::hidden_aura`, filled with exactly `ui_aura::shown_in_aura_ui`'s predicate inverted, so it cannot contradict the aura list an addon reads beside it. An unknown spell is nil: "not hidden" is the honest answer about an aura this client knows nothing about, and the lists show a catalog miss too. - `GetItemIconTexture(itemId)` — two hops, because an item template carries a display id and not an icon: the cache row's `displayInfoID`, then `ItemDisplayInfo`'s `Icon` through a new `ItemIconProbe`. A known item with no display row is nil (a data gap); an item the cache has never seen raises and records the ask, as every `item_record` verb here does. ClassicAPI's `GetItemIcon` wants the same probe, which is why it is a probe and not a closure. - `GetTrinkets()` — slots 13 and 14 in `GetEquippedItem`'s own row shape, an empty slot absent rather than nil-valued, since the consumer walks it with `if t[i] then`. Each is in `reference_surface.rs`'s `allowed_beyond_1_12` with why 1.12 has no spelling to prefer — the client reads its own movement word and tells Lua nothing, applies the aura attribute test inside `UnitBuff`/`UnitDebuff` without exposing either, and can only reach an item icon through a bag slot it already holds. All nine gates green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
gromandClaude Opus 5
|
ada08720bb |
ui+docs: the GUID token's real spelling, 40-yard interact, and the whole patch map
Working down the 29 `.tdata` hooks from the realm's patched `WoW.exe`.
Most turned out to be already ported; two were wrong here, and one of
those was wrong in a way that mattered.
**The GUID token was the wrong string.** `unit::guid_token` wrote 16
lowercase hex digits and no prefix. The realm writes `0x` + 16 upper-case
digits: its formatter at `0xd06500` — which `UnitExists`'s second return
goes through — lays down `0x30 0x78` then sixteen nibbles each `+0x37`
above 9, and its parser at `0xd06650` demands the `0x` and takes upper
case only. grom's own addons agree from the other end: BigWigs decides a
string is a GUID with `string.sub(guidOrName, 1, 2) == "0x"`
(`MageTools.lua:963`), and AI_VoiceOver reads fields from position 3 and
mints its own with `format("0x%04X%06X%06X", …)`.
Same number, wrong string, and wrong where it mattered most:
`UNIT_CASTEVENT` and the aura events carry these tokens, so every such
test read our GUIDs as unit *names* and every `sub(3, …)` field read came
out two characters off. Now written the realm's way; both spellings and
either case are accepted on the way in, since refusing one buys nothing.
Pinned literally rather than by round trip — a round trip held fine
through the whole bug.
**`CheckInteractDistance` type 4 is 40 yards, not 30.** `0x48bafd` jumps
to `0xd0686c`, which tests the type against 4 and compares against the
float the section carries at `0xd06868`: `1600.0`, squared thresholds.
Stock 1.12's `900.0` is still what the other three types use.
`docs/ADDON_COMPAT.md` now carries all 39 hook sites of both sections
with a status each: 10 in `.octoql` (one feature, the quest log, ported),
and 29 in `.tdata` — the aura ids, the master looter, `UnitExists`'s
GUID, the char-create mask and `CHAT_MSG_HARDCORE` already done; the two
challenge-mode functions needing nothing, because the realm's own
`ChallengeList.lua` defines them in Lua for exactly our case; four read
but unported with the reason (unnamed struct fields, no consumer in the
corpus); and three that cannot apply to a client that loads no DLLs.
Also `scripts/dll-surface.py`, which asks the injected DLLs the same
question. Their export tables are one `DllMain` each, but a DLL that
registers a Lua function must point at the name: collecting NUL-delimited
identifier strings the image holds a pointer to cuts `ClassicAPI.dll`
from 2872 CamelCase candidates to 706, and subtracting the 1.12 census,
Lua's own names and benilla's real surface leaves a candidate list the
addon corpus can rank. It is a list to read, not a specification —
statically linked libraries contribute their own strings, and
`UnitXP_SP3.dll` yields nothing. It found 38 names in `nampower.dll` that
grom's addons call and this file did not know about; they are written
down rather than guessed at.
269 data-gated failures, identical to the baseline, all nine gates green.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn
|
||
|
gromandClaude Opus 5
|
cf0e56a4cf |
ui: give the worst-call ordering test margins a stall cannot invert
`the_worst_call_is_one_calls_cost_and_not_the_mean` reads both its orderings off a wall clock, and the margins were too thin to survive the full suite: it failed about one run in three, on an idle box as well as a loaded one. One of the steady handler's ten calls would pick up a ~0.9 ms scheduler stall and overtake the spike it is meant to rank below — `self_us` 5109 / `worst_us` 1405 against the spike's 1398 / 1361. Twenty steady calls instead of ten raise the window cost without raising any single call's, and an 8x spike instead of 3x puts the worst-call margin about four times beyond the stall that broke it. Six consecutive full-suite runs clean, then a full gate chain. The doc comment carries the measured numbers and says to keep both ratios, since the absolute durations are not what the test rests on. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
gromandClaude Opus 5
|
49b4f7b55e |
ui+app: say what a fired event reached, and whether the UI answered
grom's stale-minimap report survived the same-interior-skip fix, and his log says why that fix was the wrong suspect: at the gate the zone-text interior claim reads `outdoors`, so the skip was never consulted there. The module doc no longer claims otherwise. What his run leaves is sharper and stranger. The election cannot have declined — leaf 1537 → 809 "Gates of Ironforge" takes the zone from 1537 to 1 and the subzone text with it, so both `ZONE_CHANGED_NEW_AREA` and `MINIMAP_ZONE_CHANGED` fire — and two separate addons call `SetMapToCurrentZone()` on those while the map is closed (`pfQuest/map.lua:1466`, `pfUI/modules/minimap.lua:183`). Zero calls arrived, with nothing raised anywhere in the session. Offline, against the same addons and saved variables, both events reach every registered handler and are answered every time, so the difference is live-only and the engine cannot currently see which of three things it is. So it can now. `UiScript::fire_event_reach` answers an `EventReach` — frames registered when the dispatch began, frames whose handler ran, and the name of the frame a cut-short walk stopped on (our documented deviation: a handler that unregisters the walk's saved next ends the dispatch, and every later registrant silently never hears the event). `UiScript::frame_shown` reads the shown bit addons branch on, which no amount of engine-side logging could otherwise explain. `fire_event` is unchanged for its nineteen callers. `area::feed_zone_events` logs the whole handshake at each fire: what was elected, reached/registered per event, the `SetMapToCurrentZone` delta, and `WorldMapFrame`'s shown bit. `0/0` is nothing registered, `reached` below `registered` is a cut-short walk, and every handler run with `0×` asked is the guard both answerers share. `addon_harness --fire` reports the reach too. Gates green (459 s). His corpus offline: 10/10, 8/8 and 4/4 handlers reached. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
gromandClaude Opus 5
|
9e13856cfd |
app: say in the log when the world map stops following the player
The stale-minimap report needs one fact: did the selection follow the player into the new zone? Everything map-derived rests on it, because `GetPlayerMapPosition` projects onto the **displayed** map — a selection left a zone behind seats pfQuest's minimap pins against the wrong rect, and plausibly rather than obviously wrong wherever two rects overlap, as Ironforge's does inside Dun Morogh's. The `zone:` line could not answer it even after it started printing both pairs, because the pairs disagree constantly and mostly correctly: for a frame or two after every crossing, since this feed publishes the new zone and the `ZONE_CHANGED*` an addon answers by reselecting fires after it; and for as long as the map is open on another zone, which is what browsing is. A marker on that line would have cried wolf on both. So the rule is its own thing, `SelectionLag::step`: an episode starts only on a frame where the *player* moved to a zone the map is not on, and only an episode outliving a 1 s grace is reported — once, at WARN, with the catch-up line when something finally reselects. A selection that wanders under a standing player is never an episode. Five tests, including the 600-frame browse that must stay silent. The line also forks the cause, which is the part that saves a round trip: `UiScript::world_map_resolver_calls` counts Lua's `SetMapToCurrentZone` calls, so the report can say whether the UI never asked to follow — the event did not reach the addon — or asked and still landed wrong, which is the verb reading a zone the feed had not published. Same symptom, different fixes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |