ada08720bb3fc6f7ce440d473888397f14bae719
608
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
  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 |
||
|
gromandClaude Opus 5
|
ce8992bc3e |
Draw flight masters: C_TaxiMap.GetTaxiNodesForMap over TaxiNodes.dbc
grom: "can't see flight master location marks, even tho I have them on."
Nothing was broken — the layer was silently empty. pfQuest-octo has deleted its
shipped pfDB.meta.flight table in favour of this call (db/meta.lua:181 is a stub
whose comment says the data "always comes live from the client (ClassicAPI)"),
and SearchFlightNodes opens with
if not (C_TaxiMap and C_TaxiMap.GetTaxiNodesForMap) then return maps end
so with the namespace absent the option stays ticked, no error is raised, and no
icon is ever drawn. Same silent-by-construction shape as C_Map.GetAreas, same
consumer.
The contract is half read off the bundled ClassicAPI.dll's string pool
(C_TaxiMap, GetTaxiNodesForMap, nodeID, faction, reachable, state and the
FlightPathFaction/FlightPathState enum names) and half off the caller: the
areaID/mapX/mapY triple is not separately visible there, since the pool holds
one copy of each string and C_Map.GetAreaTriggerInfo already uses all three, so
for those SearchFlightNodes itself is the authority. It reads six fields, gates
on `node.areaID and node.mapX and node.mapY`, and feeds the pair into
pfMap:AddNode as meta.x/meta.y, which UpdateNodes divides by 100 — so they are
percentages of the zone rect. Both files say which half is which.
Faction comes from the two MountCreatureID columns, the reference's own gate
(0x4a2650 asks for the mount of the player's half): one column filled is that
faction, both is neutral, and neither means the row is not a flight master at
all — on this install those are exactly the eight Transport/Ferry/zeppelin
endpoints, and passing them on would put "Flight Master" pins on the Menethil
docks.
Attributing a node to a zone needed the file's own names. Geometry over
WorldMapArea boxes proposes — these are axis-aligned boxes around irregular
zones, so a node near a border sits inside two or three of them — but no purely
geometric rule works: "smallest box" misfiled a dozen of ~70 rows (Crossroads
under Mulgore, Aerie Peak under Alterac Mountains), and "most central box",
much better and what zones_at now sorts by, still loses Talonbranch Glade to
Darkshore by about one percent of inset. TaxiNodes.dbc names its rows
"<place>, <zone>" and a city row leads with the city, which is itself a zone, so
trying each comma-separated part against the candidates gets Ironforge over Dun
Morogh *and* Felwood over Darkshore from one rule. zones_at also reaches the
instance-map list, so Alterac Valley's two nodes place on the battleground map
instead of nowhere.
Seeded before the interface loads, after the map catalog it reads the rects
from. That deadline is tighter than file scope: pfQuest replays its saved
tracking selections once on PLAYER_ENTERING_WORLD and then unregisters every
event (database.lua:131-151), so a push one frame later is a push after the only
read.
Verified against the real addon offline, not just by unit test:
SearchFlightNodes now adds 69 nodes through the probe harness. Three tests pin
the rule against the DBC itself — the capitals and the border nodes by hand, and
every named row held to the zone its own name claims, which is the test that
rejected both earlier geometric rules. It also pins the four rows that name no
zone, three of which are this realm's own additions (Northshire Abbey and two
Darkshore canoe routes), so the next custom node shows up as a failing
assertion rather than a pin in the wrong zone.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn
|
||
|
gromandClaude Opus 5
|
b4a5d3b38f |
ui: "do I know this spell?" asks the known set, not the spellbook
Every "Already known" check read `spellbook.slots`, and `SpellDisplay::in_spellbook` filters out `SPELL_ATTR_IS_TRADESKILL` — correctly, since recipes live in the trade-skill window, not the spellbook. So the question "do I know this spell?" answered **no for every crafting recipe in the game**: a recipe item never showed its `ITEM_SPELL_KNOWN` red line, and nothing stood between the player and consuming it a second time. grom learned Plans: Copper Chain Vest (item 3609) twice. `PlayerReqState` gains `known_spells`, filled from `PlayerActions::spells` — `SMSG_INITIAL_SPELLS` plus every learn, which is the set the reference's own check reads (`0xb700b0`). It belongs there: that struct is already "the player state the tooltip's red lines and the usable gate compare against", already change-gated on the action/spell inputs, and already pushed by the same feed. Both readers move over: the taught-spell line and `item_usable_by_id`'s spell requirement, which had the same hole. The existing `verified_families_signable_locked_resists_known` only passed because it made the recipe known by putting it in the spellbook, which no real session does — it now sets the known set instead, and that is the change's own regression story. The new test leaves the spellbook deliberately empty, which is the real shape and what fails if the check ever reads it again; it asserts the not-yet-learned case too, so it cannot pass by always reading red. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
gromandClaude Opus 5
|
a2da3e4da7 |
ui: a skill lock you can open stops reading red
The GameObject tooltip's skill-lock line took the opener-*unknown* arm unconditionally — `LOCKED_WITH_SPELL` in red — whatever the player's skill. The known arm was simply not built, and the code said so where it was missing. The reference forks on whether any known spell opens the slot's LockType (`0x52abcb`): known takes `LOCKED_WITH_SPELL_KNOWN`, which carries the requirement as a `%d` and the colour of the skill-difficulty ramp `0x529fa0`. Built now, both halves. grom reported it on a copper vein at Mining 70, which under the ramp is +69 over the vein's requirement of 1 — the green rung, not the grey one he expected; grey starts at +100, so a copper vein greys at Mining 101. Either way not red, and red is what it showed. The resolve is hoisted out of the `flag_locked` branch, because the "Locked" line's colour and this line's known arm are two readings of one answer and must not disagree with the click. `lock::skill_margin` recomputes the two numbers from the opener the resolver already matched, with the same two functions its own value test uses (`0x5f850f`), so hover and click cannot diverge on which opener is in play. Deviation, recorded at the constants: the three new rung RGBs are the standard trade-skill palette, not values read out of the binary's table. Only the ramp's shape is traced — five rungs, bands at +0/25/50/100 — and the rung a line lands on is what the player reads. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
gromandClaude Opus 5
|
e6ae2e0f67 |
ui: a scroll names the child it moved instead of giving up on the graph
`SetVerticalScroll` and `SetHorizontalScroll` called `Model::touch_layout` on the line before `reanchor_child`. That gave up naming for the whole graph and then *erased* the precise touch that followed: `reanchor_child` re-anchors the scroll child through `write_anchor`, which reads an offsets-only change (same point, same target) and names the child node — but `touch_layout_node` will not un-say an imprecise touch, so the give-up won on every call. Naming the child is the whole truth here. A scroll offset reaches the layout only as that one anchor: anchor targets are ids fixed at `SetPoint`, so no edge moves and the roster is unchanged, and the clip a scroll child draws within is the ScrollFrame's resolved rect rather than the offset — `clip::scroll_clip_sources` rebuilds it per call and caches nothing. Measured on grom's 2026-10-21 17:08 session, which was a map-and-panel run. These two lines were **472 of the run's 806 whole-graph derivations**, the largest single source by a factor of two, and the cost lands on the Lua that reads a frame edge afterwards rather than on the pass's own `resolve` phase: | | n | fps | UI pass | `frame:GetLeft` | |---|---|---|---|---| | windows with >= 10 scroll derivations | 12 | 68.5 (min 53) | 5.93 ms/f | 2.18 ms/f | | windows with no derivations at all | 11 | 92.4 (min 77) | 1.62 ms/f | 0.05 ms/f | `r(scroll derivations, GetLeft us/f) = +0.907` over 57 in-play windows, against `+0.014` for every other derivation site. `[ui-verbs]` has `frame:GetLeft` at 94-376 us a call in those windows and 0.6-1.5 us in the quiet ones, on ~14 calls a frame, because stock FrameXML reads frame edges constantly and each read past the clean epoch re-derived. That is why a scrolling panel — the world map, the quest log, a profession list — was the expensive thing on screen. The test scrolls both axes on the shipped interface and asserts zero derivations, with a positive control so it cannot pass on a dead counter. Put one `touch_layout` back and it reads exactly 10 for 10 scrolls. `WOW_LAYOUT_VERIFY=1` over the real 92-addon interface gives `frames 13379 -> 13379, regions 29597 -> 29645` — identical to the recorded pre-existing baseline, so this narrowing introduced nothing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
gromandClaude Opus 5
|
754d577eaf |
ui: a trade-skill list mutator signals inside its own call
`ExpandTradeSkillSubClass`, `CollapseTradeSkillSubClass`, `SetTradeSkillSubClassFilter` and `SetTradeSkillInvSlotFilter` fire `TRADE_SKILL_UPDATE` from inside the call now, as the one helper all four tail-call in the reference does (`0x4fd710`, `0x4fd730`, `0x4fd750`; `reference/1.12-verb-events.tsv`). It used to be a `trade_skill_touched` flag drained by `ui_tradeskill::drain_trade_skill` and answered the following frame. Inline is load-bearing. These four verbs are called *from* a `TRADE_SKILL_UPDATE` handler: a profession library reads the recipe list by unfolding every header, walking it and folding back what was folded. The signal is therefore re-entrant by construction, and every addon that does it carries its own guard — LibCraftingProfessions has `trade_skill_scan_in_progress`, ATSW an `atsw_scans < 2` counter. Those guards are written against an inline signal and only an inline signal lands inside the window they cover. Deferred to the next frame it landed after the guard had cleared: the library rescanned, the rescan called the verb, the verb re-armed the flag, and the two kept each other alive at one fire a frame for as long as the window was open. grom's 2026-10-21 session shows it directly — `[ui-events]` reporting `TRADE_SKILL_UPDATE · 1.01 fires/f · ~900 call(s)` across 37 consecutive windows, three unbroken minutes, at 0.58-0.84 ms a frame in handlers over four addons rebuilding their lists for nothing. It is the 118-second and 61-second stretches at 55-70 fps in a session whose in-play median was 86. Two tests: each mutator's fire is read with its own `run` still on the stack, so a deferred signal fails; and a library-shaped in-progress guard turns one real update into exactly one scan, with nothing owed to a later tick. The bench's `[events]` block also fires the five non-quest events the live session named. They cost ~0.2-0.5 ms there against 12-72 ms live, because this harness has no NPC in front of it, no trainer list and no open map — recorded as a gap, not an answer. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
gromandClaude Opus 5
|
e269e4dbb4 |
ui: name the event, not the library frame that dispatched it
`WOW_UI_EVENTS=<secs>` (`script/event_prof.rs`): per event, what one fire
costs across every handler it reaches, and which frame took the most of the
worst fire.
The gap it fills is a dispatcher. `WOW_UI_HANDLERS` keys on `(frame, script)`,
which is the right key for an `OnUpdate` and the wrong one for an `OnEvent`,
because every addon library registers *one* frame for *every* event it wants and
fans out in Lua. So the largest identifiable cost in grom's 2026-10-21 session,
eleven separate times, read as
[ui-handlers] 312.5 312.5 0.00 72522.3 AceEvent30Frame:OnEvent
— a true row that names a library's shared frame and no addon, no event and no
cause. `[sys-timing]` saw the same cost as `ui_quest::feed_quest` at 85.7 ms for
one run in 76, because every handler runs synchronously inside whichever system
fired, so the system holding the stack gets the bill.
Two conventions the report sticks to:
- `self` is nest-adjusted, in both columns. A handler's own time has any event it
fired taken back out (`event::fire` differences `event_prof::nested_ns` across
the call), or every event in a cascade reports the cascade and the row naming
the real cost is the one that looks innocent. `total` keeps it, which is what a
`[sys-timing]` span sees.
- A fire is one `fire_event`, however wide it fans out, because that is the unit
a dropped frame is measured in. `calls` is beside it for the width.
Measured on the player's own 92-addon interface (`tick_bench`'s new `[events]`
row, three fires per event from a settled start):
us/fire worst event
5068 14759 QUEST_DETAIL <- HCQuest.lua:139 took 14,097 us
764 1324 QUEST_LOG_UPDATE
662 1564 QUEST_FINISHED
Read `us/fire` against `worst` and that 14.8 ms is **one** fire of three: the
other two cost ~0.1 ms, so it is HCQuest building something once and keeping it,
not a cost paid per quest. With that read correctly, no quest event costs more
than ~1.5 ms recurring offline — so the bench does not reproduce the live 57-73
ms and says so, rather than offering a number that looks like an answer. Hence
the instrument, armed in `./measure.sh`.
Also adds `./measure.sh --frames`, which arms two meters that already existed and
have never been armed: `WOW_FRAME_PHASES=50` and `WOW_RENDER_MS=1`. A frame's
wall time is `max(main app, render app)` since bevy pipelines the render app onto
its own thread, so when the render app is the longer, `wall` climbs while `cpu`,
`main`, `gpu` and every per-system `worst` all read healthy — exactly what that
session's **161 ms frame in a second whose costliest system worst was 2.9 ms over
105 runs** looks like, and what 43 of its 64 spike-seconds look like. Its own
mode because both perturb what they measure.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn
|
||
|
gromandClaude Opus 5
|
0c862838da |
ui: a SetScale names its subtree instead of giving up on the graph
`[ui-phases]`'s new write-site tally answered the combat-fps question on its
first armed session: 463 whole-graph layout derivations across 34 windows, and
**232 of them** at `object/frame_state.rs:427` — the `touch_layout()` inside
`SetScale`. Splitting the per-window count by site and regressing against
resolve time over 43 in-world windows, resolve correlates with `SetScale`
derivations at r = +0.996 and with every other derivation at r = -0.041. The
four costliest windows of the run are 36-64 `SetScale` derivations each and
almost nothing else.
That also corrects the flat "~15.5 ms per derivation" of the round before: a
mint-triggered derivation costs ~1.1 ms, a `SetScale`-triggered one 12-13 ms.
The difference is that `SetScale` gave up *two* ledgers, not one.
The caller is pfUI's action-button press animation
(`pfUI/modules/actionbar.lua:182`, `:207`, `:233`, `:259`): four styles that all
run `this:SetScale(this:GetScale() + fade)` on `OnUpdate` for the ~60 frames a
button's fade lasts. Pressing an ability starts it, which is why a settled
interface never reproduced this and a fight always did.
Both give-ups had precise versions already, written for `SetParent`:
- `touch_layout()` -> `touch_layout_subtree_scale(h)`. A scale write moves the
subtree's `effective_scale` and nothing else: anchor targets are ids fixed at
`SetPoint`, so no edge moves and the roster is unchanged.
- `touch_measure_all()` -> the same walk now names the subtree's regions on the
measure ledger. This was the bigger half. `measure_dirty = None` makes the
next `fontstrings_needing_measure` walk all ~43,000 regions of a real
interface to issue, typically, zero requests; `stale_measure_key` reads only a
region's own data plus *its owner frame's* `effective_scale`, so the subtree's
regions are exactly the ones that can go stale.
Measured on the player's own 92-addon interface, 43,195 regions, one `SetScale`
per frame for 240 frames (`tick_bench`'s new `[scale]` row):
before changing resolve 0.601 ms/f · 0.996 derivations/f (239/240)
after changing resolve 0.005 ms/f · 0.000 derivations/f ( 0/240)
control same resolve 0.005 ms/f · 0.000 derivations/f ( 0/240)
`changing` is now indistinguishable from a control that writes the value it
already holds. The harness understates the live cost (its measure answers are a
6px-per-char stub), so the live saving is to be confirmed in a play-test, not
predicted from that row.
Correctness is left to `WOW_LAYOUT_VERIFY`, which `cfg!(test)` forces on for all
1975 benilla-ui tests: it re-derives the whole graph after every `resolve()` and
panics on any rect the incremental pass got wrong, and runs the full measure
roster beside the ledger drain and panics on any stale region the ledger missed.
So the two new tests assert only that the cheap path was taken, on a graph three
frames deep with a FontString on the leaf. The real-interface verify run fails
identically to its documented pre-existing red (`frames 13379 -> 13379, regions
29597 -> 29645`), so this change adds nothing to it.
Also repairs an assert message in `layout_gate.rs` whose line continuations had
been eaten by a heredoc, leaving runs of spaces mid-sentence.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn
|
||
|
gromandClaude Opus 5
|
c1a56f98ab |
layout: name the write that asks for a whole-graph derivation
`[ui-phases]` could say a window paid for 61 whole-graph derivations. It could not say which of the forty-odd `touch_layout` sites asked for them, and that is the half a fix has to be aimed at. Measured on the player's interface, one derivation costs ~15.5 ms — from two directions: the derivation count explains total resolve time at r = +0.977 across twenty windows (solve count: r = -0.33, so solves are free and derivations are everything), and 26 of 78 worst-single-call figures over 1 ms land in the 15-16 ms bin across eleven unrelated handlers by six authors, two of which fired exactly once for their whole window. Eleven handlers cannot each independently be 15.5 ms slow; it is one shared charge landing on whoever reads the graph first, which is also why the same session shows `resolve` p99 at 14.4 ms and worst frames "mostly tick". One defect, two accounts. So `#[track_caller]` down the naming chain and a tally of `file:line`, printed in the report. Not a backtrace: `WOW_LAYOUT_DERIVE_TRACE` already captures those and has to be rationed to `n` prints because of what they cost. This is a `(&'static str, u32)` key off `Location::caller()`, cheap enough to leave armed, and take-and-rearm so a window reports its own rows and a VM replaced by `/rl` re-arms without anybody noticing it was replaced. Also a `[churn]` window in tick_bench, which records a negative: a plate roster turning over 4-in-40 every ten frames costs **zero** derivations on the player's own interface. `sync_nameplates` pools the plate frames, so a new key rewrites an existing plate and mints nothing. With `resolve_bench`'s hover-sweep, tooltip-content and moving-region tests all asserting zero, four candidate triggers are now ruled out offline and the live tally is what names the real one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
gromandClaude Opus 5
|
112647b919 |
perf(ui): the handler profiler names the worst single call, not only the mean
`WOW_UI_HANDLERS` ranked handlers by self time over the window, which is the right ranking for what costs framerate and the wrong one for what costs a frame. A handler that stalls once every four seconds has a mean near zero over five hundred frames and never reaches the table at all. The player's session had a 15-28 ms tick in nearly every five-second window — one dropped frame every few seconds, which is the kind of thing a player reports as "it hitches" and no average will ever show. The table said `UIParent:OnUpdate 60.8 us/f` and nothing about it. So each slot keeps its costliest single call, the table gets a `worst` column, and the handlers whose worst call cost a frame are listed again underneath — but only the ones the table left out, so the block says something new or nothing. `tick_bench` prints the same ranking. It names the stall on the first run, offline, in this interface: ``` 15768.7 us once (self 17080.8 us over 240 calls) pfQuestMapTracker:OnUpdate 12339.4 us once (self 13042.0 us over 240 calls) AceEvent20Frame:OnUpdate 5380.9 us once (self 16166.0 us over 240 calls) pfQuest\quest.lua:348:OnUpdate ``` 92% of the tracker's whole cost over 240 frames is one call. Both of the top two are steady per-frame allocators, which made the collector the first suspect; the bench's own heap line refutes it — 222.8 MB live against 29.1 kB/frame of churn is ~131 s between 200%-pause cycles, not the ~4 s the stall recurs on. What the per-binding bill says is in there instead: one `pcall` of 7.5 ms, and `frame:GetLeft` at 92 us a call, which is a layout resolve against a dirty graph. `measure.sh` comes with it. The set of instruments worth arming for a play-test is long enough that a run with one of them missing is normal, and such a run looks exactly like a run that found nothing — the session this column came from was missing `WOW_SYSTEM_TIMING`, so it could say what the UI cost and not what the frame cost. The script arms the set and prints what it armed. The test asserts the property the column exists for and not a duration: a handler called ten times for 50k iterations each outranks one called once for 150k by self time, and the second still has the larger worst call. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
gromandClaude Opus 5
|
a8b6fc8ad6 |
perf(layout): the resolve's seed pass walked the graph, not the change
Each solve chooses which nodes to solve and which clean rects to seed it
with. It did that by asking **every id in the graph** — 18 203 frames and
42 284 regions on the player's interface — "is one of your dependents
dirty?", for a dirty closure whose steady-state median is 2 frames and 4
regions.
And nothing was measuring it. `[layout-pre]`'s last lap ends at the change
gate; `[layout-prof]`'s `frame_us`/`region_us` start at the round loop. This
pass sits between them, so neither line charged it — which is how a
`frame:GetLeft` that measures 17.5–18.4 us a call in a live session (six a
frame, through stock `MouseIsOver`'s four edge reads) could force a resolve
the profiler called 3 us. A `seedsel_us` column now closes that gap, and it
stays: a phase between two timers is a phase nobody owns.
Measured on the player's own 80-addon interface, last-quarter solves:
median p75 p90 mean max
before 458 498 607 488 882
after 0 3 9 38 3788
`solved` and `swept` come out identical to the digit (median 2/4, mean
156.3/408.3, max 18183/41415): the same work is selected. Before, 202 of 549
solves cost over 400 us and the dearest of them had dirty sets of 0–13 nodes,
which is the pathology itself; after, 29 of 549 exceed it and every one is
solving 14 000–16 600 frames at 0.48 us a node, against a `frame_us` of
10 170 us on the same pass. The cost moved from the size of the graph to the
size of the change, which was the point; the higher max is that, not a
regression.
**What it is not: an fps fix.** The bench's per-frame totals do not move —
1.294 -> 1.293 ms/frame grouped, `vm-tick` 0.667 -> 0.676 (noise, single
runs). The one column that shifts is `resolve`, 0.011 -> 0.006 ms/frame. The
458 us is per *solve*, and a steady frame does a fraction of one; the live
bound is the whole of `frame:GetLeft`, 105-110 us/f, against a 7.6-9.5 ms
frame. So this removes real waste and buys about 1%, which is worth having
and is not what grom asked for. Chasing it at all was a misread of mine:
`GetLeft` is 54% of the *binding* bill, and the binding bill is ~200 us of a
~9000 us frame.
The same set, by the same relation read the other way round: a clean node is
seeded iff some dirty node reads it, and a node's dependencies are exactly
the three `scope.edge` arguments in the roster walk — its anchors' targets,
plus a region's owner frame, whose scale it reads. `LayoutScope::dirty_ids`
carries the dirty set as a list, written wherever `dirty` turns true so the
two cannot disagree, and `seed_dep` enumerates the dependencies. A
`full_scope` pass skips the walk: everything is dirty, so there are no seeds
to find.
Order is not a property here, which is what makes the rewrite safe. Both
rosters are built from `HashMap` iteration (`frame_to_id`, `region_data`), so
no pass ever had a canonical order, and the region sweep is a fixpoint whose
round cap covers any chain — order moves `rounds`, not rects. Only the seed
lists are sorted, to drop duplicates a shared dependency would otherwise
re-`set_external` every round.
`verify_selection` is the falsifier: under `WOW_LAYOUT_VERIFY` it recomputes
all four lists the old way and requires the same sets. That is a tighter
question than `resolve_layout`'s rect comparison — it isolates this pass, so
a selection bug cannot hide behind an unrelated red, and an unrelated red
cannot be pinned on the selection. Verified non-vacuous by dropping the
region-owner dependency and watching it fail (`0 vs 1 entries`).
Note for whoever runs it: `WOW_LAYOUT_VERIFY=1` over `tick_bench` is **red at
HEAD**, independently of this change — an incremental pass ends with 48 fewer
region rects than a full derivation of the same frame (`regions 29597 ->
29645`), byte-identical with and without this commit. That is a missing
`touch_layout()` somewhere, not a selection bug, and it had never been run
before because `tick_bench` is `--ignored` and the flag is in no gate.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn
|
||
|
gromandClaude Opus 5
|
7322a95a9d |
fix(ui): WOW_UI_VERBS must not wrap setfenv — it unloaded a third of the interface
A wrapper puts its own frame between the caller and the callee, which silently changes the meaning of every stack-level-relative call. `verb_prof::install` wraps every function in `_G`, and `_G` at that point holds Lua's own stdlib as well as our bindings — so it wrapped `setfenv`. pfUI's `api/api.lua:4` is `setfenv(1, pfUI:GetEnvironment())`, and level 1 from inside a wrapper is the wrapper, a C function, so Lua raises ``setfenv' cannot change environment of given object`. One armed session logged **91** of those and lost pfUI, WhatsTraining_Turtle, AI_VoiceOver, MasterTradeSkills, pfUI-addonskinner, pfUI-locplus and PizzaSlices: an instrument that unloaded a third of the interface it was there to measure, and whose numbers were therefore of a client without them. `STACK_RELATIVE` names the four `wrap_table` must leave alone. `setfenv` and `getfenv` take a level; `error(msg, level)` positions its message by one; and `debugstack([start])` is the reference's traceback by level, which `FuBarPlugin-2.0.lua:752` asks for as `debugstack(6, 1, 0)` and `AceDB-2.0.lua:742` reads a specific line of. `pcall`/`xpcall` stay wrapped: no level argument, and an armed session shows them working and worth seeing. The test wraps `_G` and then runs pfUI's line, so it is the bug rather than a restatement of the list; deleting the skip reproduces grom's exact error. The module doc also claimed `_G` "holds our bindings and nothing else" at install time, which is what made this easy to miss. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
gromandClaude Opus 5
|
5764837e35 |
perf(ui): give WOW_UI_VERBS a live window, so the instrument exists in a session
`verb_prof` wrapped every global and every widget method and charged each call two `Instant::now`s — and the only caller of `log_report` was the `addon_harness` **example**'s `--together` load. Armed in the client it paid for every binding call and printed nothing at all. The one question it answers — which binding a handler's self time went into — is exactly the open one: `WOW_UI_HANDLERS` names `UIParent:OnUpdate` at 503 us/f in combat against 48 idle, and stops there. So `WOW_UI_VERBS=<secs>` now prints a window from `report_window`, called at the end of `UiScript::tick` beside the handler and raise reports, one level down from them. The bare `=1` the harnesses already pass reads as one second, which is why `period()` parses the same variable rather than taking a new one. A window is a **delta** against a snapshot (`PREV`), not a `reset`: `ROWS` is the one table and `log_report`'s callers read it whole at a moment of their own choosing — `--together` ticks ten times before printing its load bill, so a window that cleared the table would have handed it whatever happened after the last window instead of the load. The arithmetic is `diff`, split out and tested on its own because `window_delta` reads two process-wide statics and `diff` is the whole of the logic. It earns its keep immediately. `tick_bench` over the player's real 94-addon folder, 121-frame windows: `frame:GetLeft` costs **0.42 us/call** on a settled layout and **29.95 us/call** the moment anything moved — the same two calls a frame, 55% of the frame's whole verb bill — because the four edge getters call `settle()` and a resolve past tier 1 walks every live frame. During the load burst the same call is **370 us**. `GetWidth`/`GetHeight` already carry an authored-span shortcut for precisely this; the edge getters never got one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
gromandClaude Opus 5
|
497570a68c |
addons: an on-demand load says what it loaded
The startup walk names every addon it loads; `LoadAddOn` named none. So when the client froze on 10-19 while learning recipes, the session log could not say whether the trainer window was even open — and `Blizzard_TrainerUI` is LoadOnDemand, so that was the whole question. `benilla-ui` has no logger, so the line travels the way its errors and warnings already do: a `notices` channel on the model, drained to `info!` beside them in the same pass. Silent for a redundant load, because the stock `*_LoadUI` glue calls `LoadAddOn` on every open and a line per call would bury the one that did the work. A refusal carries the reference's own reason token, and a name that is not installed says so. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
gromandClaude Opus 5
|
9782b07fd4 |
lua: a frame that never leaves the VM fails its call, not the client
The load has had an instruction bound since `3b6975fd` — "so a hung load reports instead of freezing the loading screen" — and the end of the load disarmed it, "so no handler is killed for being slow". Right about slow, and it leaves a handler that never returns at all able to freeze the client until the player kills it, which is what happened to him this morning. So the bound does not go at the end of the load, it changes shape. Wall time rather than instructions, because the number has to be obviously beyond anything legitimate and a count is not: a frame still inside Lua after fifteen seconds is not slow, it is stuck. `tick` restamps every frame, so no frame is charged the one before it and a handler reached later in the frame — a click, a fired event — measures from that mark, which can only make the bound tighter. The raise names the chunk and the line, so his log says which addon rather than that something somewhere looped. It shares the one count hook, which costs 5-13% of pure Lua execution time (measured: 30M instructions, 235-260 ms bare against 267-274 ms hooked, which also puts his machine at ~125M VM instructions a second — what sizes the load's 200M at about 1.6 s). Lua is a small part of a play frame, so that is a fraction of a percent of frame time, and the load was already paying it. `WOW_LUA_RUNAWAY=<secs>` moves the bound, `0` disarms it. Two tests: a loop with no exit fails its own call, names its chunk and leaves the VM usable; and a frame is not charged the wall time that passed before it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
gromandClaude Opus 5
|
b7ddc18782 |
load: a reload says what it cost and what the chunk cache did for it
Both lines sat on the world-entry path, so the load the player actually waits through — the `/reload` — reported nothing at all. His 2026-10-19 session has two of them, 4,981 ms and 6,276 ms against the 7.5-9 s he started from, and the log cannot say where either went or whether the bytecode cache was hit, although the cache directory proves it was: 1,585 entries written at first load, 14 added across both reloads. `report_load` now runs on both roots and names which one. The reload's clock starts before the saved-variable writes, because those are part of the one blocked frame he sees, which is also what the `frame hitch` warn measures. `chunk_cache_line` resets its counters on read, so each line is that load's own work rather than a running total in which a warm reload hides behind the cold login before it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
gromandClaude Opus 5
|
d5ddaa1286 |
lua: a compiled chunk is kept, so a load parses only what changed
The compile is a UI load's biggest removable cost and the only half of it that can be kept — running the chunks is what an addon *is*. Measured through the addon harness's `--together` load of the player's own 91 addons, three passes: ``` no cache 4,788 chunks compiled 3,324 ms cold 1,592 compiled, 3,196 from cache 1,482 ms warm 0 compiled, 4,788 from cache 471 ms (+24 ms of keys) ``` **About 940 ms off every login and every `/reload`**, and the first load after an addon changes pays only for the files that changed. `benilla-config/cache/lua/`, one file per chunk, keyed by a hash of the chunk's own bytes and its name. Both chunk paths go through it — `run_chunk_named` and the XML loader's own runner, which is how the biggest files in the corpus arrive (pfQuest's database is 33 MiB of `<Include>`d Lua). It cannot go wrong quietly. A key is a property of the bytes, so an edited file is a different entry and a stale one is never asked for; an entry that is truncated, corrupt or from another Lua build fails `lua_undump`'s header check and the source is parsed instead, so a bad entry costs the parse it meant to save. The dump keeps its debug info — `debugstack` is an addon-facing verb and a stripped chunk would lie about its own lines — which the test asserts by raising from a cached chunk and demanding its file and line back. The folder is capped at 512 MiB (one load's worth is 159 MiB) and emptied wholesale when it is over: a cache that needed an eviction policy would need bookkeeping that costs more than it saves. The first build of this looked like it worked and saved nothing, because `ahash::AHasher::default()` is seeded per process: every session hashed the same file to a different name, found nothing and rewrote the whole cache (4,776 entries, 477 MiB, a steady 1,592 misses a run). Fixed seeds, and the instrument that caught it — `benilla_ui::script::chunk_cache_line()`, hits and misses with the time each spent — stays, because "the cache is installed" and "the cache is working" turned out to be different claims. Verified: the harness's verdicts over all 91 addons are **identical** with the cache and with `WOW_LUA_CACHE=0`, the full gate chain is green, and the `WOW_DATA` failure sets are unchanged (benilla-app 270, benilla-formats 80). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
gromandClaude Opus 5
|
0fe0a99d8a |
layout: a size the author set is not a question for the solver
`GetWidth`/`GetHeight` answer against current layout, so they resolve first — and a resolve that follows any `CreateFrame` re-derives the whole layout graph, because minting a frame id drops the cached one (`Model::frame_id` → `touch_layout`). A UI load is nothing but creation interleaved with reads, so the player's interface paid **839 whole-roster derivations in one load, 2.84 ms each: 2.4 s**, against 650 `GetWidth` calls. `WOW_LAYOUT_DERIVE_TRACE` named the giver-up: every sampled one was `create_frame`. Most of those reads cannot learn anything from a resolve. An edge takes its anchor scan first and only then `combineEdge(center, opposite, size · scale)` (`resolve_left`, `resolve_right`), so an authored size IS the span unless the anchors pin **both** of that axis's edges. `LayoutInput::authored_span` is that rule, written beside the solver whose precedence it mirrors, and it refuses the two cases where only the solver knows: both edges anchored, and a clamped frame, whose rect the screen can squeeze. Measured on the player's own 91 addons through the harness's `--together` load, three passes each: ``` derives per load 839 → 235 preamble per load 2,414 → 560 ms frame:GetWidth 686 µs → 60 µs per call (1,241 ms → 118 ms over three loads) ``` **1.85 seconds off every login and every `/reload`**, and the steady state gains the same way: an addon reading a size it set in an `OnUpdate` no longer walks the roster for it. Proven as well as measured: six UI captures (character, bag, merchant, quest, quest log, tooltip) are **pixel-identical** before and after, MAE 0.000, max 0; the layout suite passes under `WOW_LAYOUT_VERIFY`, which re-derives every frame from scratch and demands the same rects; and `resolve_bench` now asserts both halves — an authored, singly-anchored span answers with no gate walk, while one whose anchors pin both edges still forces the resolve and reports the anchors' distance, not the authored number. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
gromandClaude Opus 5
|
c5911ccd6f |
verbs: an instrument that charges a load's time to the binding it went into
`WOW_UI_HANDLERS` names the handler and `WOW_UI_TICK` names the tick phase, and
between them they left the question this answers. A reload of the player's
interface spends **4.47 s inside handler bodies**, and single calls in there cost
hundreds of milliseconds — `pfBag:OnHide` 394 ms in ONE call — which is not Lua
arithmetic. A handler's self time includes the engine work it provokes, so that
time is ours: it is inside the verbs the handler called, and nothing charged it to
them.
`WOW_UI_VERBS=1` wraps every global and every widget method at the end of
`UiScript::new` — before FrameXML or any addon runs, so everything it finds is
ours — and times each call. Off, nothing is wrapped and nothing is paid.
It works offline, which matters more than the live case: the addon harness's
`--together` mode loads the player's whole folder into one VM in load order, which
is the closest thing to a login burst that needs no server. The first run over his
91 addons, three passes:
```
[ui-verbs] 519 verbs, 17,966,414 calls, 9,148 ms in all
4384 6393 685.8 us pcall (addon code, nested)
1498 16071556 0.1 us type (5.4M calls a load, Ace's)
1241 1950 636.2 us frame:GetWidth ← 650 calls a load at 0.6 ms each
548 33093 16.6 us CreateFrame
219 37524 5.8 us fontstring:SetPoint
95 99 960.3 us frame:GetHeight
67 123 545.9 us frame:GetLeft
17 3 5628.6 us frame:GetCenter
```
A geometry read costing 636 µs is the whole finding, and the fix is the next
commit.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn
|
||
|
gromandClaude Opus 5
|
6affd61c35 |
doc: the model pane's skins link points at the struct that has them
`doc-links` was the last red gate, on one link dating from the fork's import (`81d96af3`): `script::types.rs`'s `QuadContent::ModelPane::skins` cited `widget::kinds::ModelState::skins`, a field that never existed on that struct. The three creature skin variations live on `script::CreatureModelView`, which is what a display id resolves to for a pane, so the link now goes there — by the public re-export rather than through the private `model` module. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
gromandClaude Opus 5
|
2a51c9817a |
frames: EnumerateFrames, the walk over every frame the client has
A real 1.12 global we did not have (`reference/1.12-globals.tsv`), and the natural way for an addon to ask the live UI which frames carry some script or property — the player's backlog item, raised because nothing answered. Read off `0x705f60`. The frame manager owns a Storm list (`[0xcf0bd8]+0xcc0`, link offset `0x304` written by its ctor at `0x7641b8`) and the `CSimpleFrame` ctor links every new frame through `0x7646a0`, which puts the newcomer in the sentinel's own slot `[0xcc4]` and the frame it displaced in the newcomer's `+0x308`. So the list is walked **newest to oldest**: with no frame the binding answers `[0xcc4]`, with one `[frame+0x308]`, and the first frame ever created holds the tagged sentinel `&head|1` there, which is the `test $0x1,%al` that ends the walk with nil. Three details that are not guesses: - The only type test is `lua_type(L, 1) == LUA_TTABLE` (`cmp $0x5`), so a number, a string or a boolean is not an error — it falls through to the head and starts the walk over. - A table is resolved through `T[0]` exactly as our `decode_id` does, and both rejections raise through `luaL_error` (`0x6f4940`, which never returns) with the client's own wording: `0x872f08` for a table with no object behind it, `0x872ecc` for one that is some other kind of object — a region, which is not in this list at all. - Regions and the walk's order aside, hidden and anonymous frames are in it: the link is the ctor's, not the draw list's. The arena grows the list it already implied. `Frame::insertion_seq` could not serve: that is the draw-order stamp, and a `Show` or a strata change restamps it (`resequence_to_tail`), where creation order never moves. So `created` is the handle list in creation order with `created_seq` the frame's index into it, and a step is one lookup. A destroyed frame's slot is kept, not compacted — its handle stops resolving and the walk skips it — because an index shift would break every other frame's `created_seq`, and nothing in play destroys a frame anyway, as nothing in 1.12 can. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
gromandClaude Opus 5
|
e527931885 |
worldmap: WORLD_MAP_UPDATE is signalled inline, so the map opens on the player's zone
The world-map flash, and it is one frame of the wrong paint rather than a wrong selection. `WorldMapFrame.xml`'s OnShow runs `SetMapToCurrentZone()`, and `WorldMapFrame_OnEvent` answers `WORLD_MAP_UPDATE` with `WorldMapFrame_Update()` **only** `if ( this:IsVisible() )`. Our `store_selection` pushed that event on the deferred lane, so it was heard a tick later — by which time the panel was already on screen, still carrying whatever tiles the last `WorldMapFrame_Update` left. Those are the World sheet's: `WorldMapFrame_OnLoad` runs its own `WorldMapFrame_Update` before the player's zone is known, and a nil `GetMapInfo()` takes FrameXML's own `-- Temporary Hack` branch, `mapFileName = "World"` (`WorldMapFrame.lua:47-55`), painting `Interface\WorldMap\World\World1..12` into the twelve detail tiles. Signalled inline instead, as the setter `0x4a67a0`'s own `SignalEvent` is, so the repaint happens inside the OnShow and the first frame drawn is the player's zone. Signalling moves out of `store_selection` — the writer cannot do it, only a caller holding the Lua state can — and each of the four call sites does it: `SetMapZoom`, `SetMapToCurrentZone`, `ProcessMapClick` (only when the click actually moved the selection, since the setter is the only signal site) and the engine-side world-enter sync. `worldmap_update_event_fires_on_next_tick` asserted the deferred behaviour deliberately, on the reason "a binding cannot re-enter dispatch". That was not so: `UpdateSpells` had already been doing exactly this with the same `fire_event_into`, with its own test pinning the synchronous dispatch. Replaced by one that asserts the inline fire and one that covers the two other legs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
gromandClaude Opus 5
|
8e0d1bae8d |
lua: a numeral ends where its digits do, so 1then is two tokens
`AdvancedTradeSkillWindow2.lua:3878` is `if Item.ReplaceWords == 1then`, with no space, and it compiles on the reference. Its `read_numeral` (`0x6ffee0`) is 5.0's: digits, at most one `.` and its digits, then — given an `e`/`E` — the marker, an optional sign (the `0x2b`/`0x2d` test at `0x6fff04`, reached with the marker already saved) and its own digit loop. The only character-class test in the whole routine is the ctype digit mask at `0x6fff47`; past it the buffer is terminated and handed to `luaO_str2d` (`0x6f5900`). 5.1 instead ends with `while (isalnum(ls->current) || ls->current == '_') save_and_next(ls);`, which swallowed `1then` into the numeral and failed it — so we raised `malformed number` and lost a 4000-line file and every global it defines, and its own XML then raised four more times calling them. The addon was dead. The ninth hunk of the `lua-src` fork, which is where a grammar difference belongs: `BENILLA.md` is explicit that rewriting the chunk source means parsing Lua with a regex. A replacement, not a deletion — 5.1 had no exponent-digit loop of its own, so deleting the slurp alone would have cost `1e2`, which is what the new test caught. The same boundary is the deliberate half: 1.12 has **no hex literals**. The digit loop stops at the `x`, so the reference reads `0xFF` as `0` and the name `xFF` and cannot parse the statement around it. A comment- and string-stripping scan of the installed corpus finds hex in code in exactly two files, `pfQuest/toolbox/pngLua/deflate.lua` and `numberlua.lua`, which no `.toc` row reaches — zero loaded sites, the same standard the fork's three other deletions were held to. Our own shipped Lua and his saved variables have none either. Two of our own test chunks did, and are now decimal: no chunk of benilla's Lua may use a construct the client's parser rejects. Verified: the whole chain still loads with no failure, the `WOW_DATA` failure set is unchanged, and `AdvancedTradeSkillWindow2` now publishes `ATSW_OnLoad` and `ATSWTypeColor`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
gromandClaude Opus 5
|
f185f3b549 |
camera: CameraZoomIn and CameraZoomOut, and a bare one is a yard
The first two rows of the reference's camera table `0x84f7a0` (`0x50b400`, `0x50b450`), and the one verb the chain's own barbershop, arena and guild-bank windows call that we did not publish — `Turtle_Barbershop.lua:211` zooms the world camera out two yards as its preview opens. Read off the binary rather than guessed, because the argument handling is not the one every sibling in this file uses: both thunks test is-number (`0x6f34d0`), take `tonumber` (`0x6f3620`) when it holds, and otherwise store `1.0f` outright (`0x50b42d`, `0x50b47d`). So `CameraZoomOut()` steps a yard out, where `SetView()` and `FlipCameraYaw()` return silently. A test pins that, since it is exactly what reading the callers would have got wrong. Deviation, named in the module: the reference turns the yards into a timed glide — divide by the rate at `[0xbe0fbc]+0x24`, scale, truncate (`0x40a2b0`) into a step count — and a call during a running glide adds into its pending-zoom field (`0x50fca9`). Our rig holds one target distance and glides to it every frame, so a second call re-aims the same target instead of queueing behind the first. The clamp is the rig's own, the one the wheel and the zoom keys already share. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
gromandClaude Opus 5
|
02265cc224 |
compat: nampower's equipment and item-cache getters, for the character panel
"then we gotta fix bettercharacterpanel, that one is driving me nuts."
BetterCharacterPanel is built on five nampower getters and had none of them.
`GetEquippedItem` was 8 raises in one session of his log; the other four failed
inside the addon's own `pcall`s, so its item-level column, quality colours,
missing-enchant warning and two-hand test were all dead at once. DoiteAuras
reads the same getter in six places.
The spec is the installed `nampower.dll` itself rather than inference, which is
what unblocked this — the earlier note here said to find the upstream spec
first, and the DLL *is* the spec:
- its usage strings give the signatures,
- "Slot must be a number between 1 and 19" gives the band, so the slots are
1.12's own `GetInventoryItemLink` numbering and `NampowerAPI.lua`'s
`slot - 1` in `IsItemInSlot` is that shim's bug, not the contract,
- its string pool gives the key names in order, and
- "Item not found in DBCache" is a raise, which is exactly why the addon
`pcall`s these three and calls `GetEquippedItem` bare.
`itemLevel` is the one field 1.12 cannot reach at all: `GetItemInfo`'s fourth
value is the *required* level, and later clients are the ones that insert
`itemLevel` there — so the panel's item-level column had no possible source
before this. The cache row now carries it, with `buyPrice` and `displayInfoID`
beside it, and an equipped item carries the raw descriptor numbers the tooltip's
resolved enchant lines could not answer (an id whose `SpellItemEnchantment` row
hides its name is still an id).
Fields left absent rather than guessed are listed in code and in
`docs/ADDON_COMPAT.md`; nothing installed reads any of them.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn
|
||
|
gromandClaude Opus 5
|
13989fb09b |
messageframe: a scroll that moves the view fires OnMessageScrollChanged
WIM sets this script on every window it builds — the history frame at load, then one per correspondent (six whisper windows in one session of his log) — and we refused the name by raise, which killed the rest of the builder function each time, not just the handler. It was on the deliberate "real 1.12 kinds nothing fires raise rather than silently accept" list. That rule is right and this was the wrong side of it: the slot now fires, from the ScrollingMessageFrame's own scroll verbs when a scroll actually moves the view, which is what a chat addon hangs its scroll-down arrow on. A scroll refused at either end moves nothing and says nothing; whether the reference fires one there is untraced. The raise list keeps its rule and its test — `OnInputLanguageChanged` and the movie frame's are still raises, and a name leaves that list only when something fires it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
gromandClaude Opus 5
|
15f069667e |
saved: a widget writes as its name, so a saved frame comes back a frame
Decursive raised `attempt to call method 'AddMessage' (a nil value)` on every
login (`Decursive.lua:1393`). It keeps its output window in its own saved table
and guards the reload with `if Dcr_Saved.Dcr_OutputWindow == nil or not ...`
(`:1245`) — which a non-nil *data* table passes.
Our writer walked the frame like any other table: his `Decursive.lua` came back
holding ChatFrame1's own fields (`buttonSide`, `channelList`, `editBox`, …),
twice the size of the real client's file and with no methods on it at all.
The reference tests the value's `T[0]` identity before its fields —
`lua_rawgeti(v, 0)` then `lua_touserdata`, `0x704613`-`0x704628`, reading the
lightuserdata `0x701bd0` wrote — and never walks a widget. A named one writes as
its **bare global name**, a Lua expression, so the reloaded chunk's own lookup
hands the live object back; an anonymous one as `nil --[[ unnamed object ]]`
(`0x872e0c`). His own real-client file is the proof:
["Dcr_OutputWindow"] = ChatFrame1,
Deviation: a name that is no Lua identifier writes as an anonymous object, where
the reference writes it raw and would leave a file that will not parse.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn
|
||
|
gromandClaude Opus 5
|
ed92574cea |
tooltip: SetOwner hides the plate, so a rebuild gives the addon its OnShow back
Reported from play with a video: hovering a bag item draws the enhanced tooltip — MasterTradeSkills' recipe block, AdvancedTradeSkillWindow2's Bags/Bank/Total — for about nine frames of sixty, then the plate falls back to the bare one and stays bare while the cursor sits still. The loop is stock. `ContainerFrameItemButton_OnUpdate` re-enters `..._OnEnter` every `TOOLTIP_UPDATE_TIME` (`ContainerFrame.lua:693` in his own patch-4.mpq; pfUI's bag slots are `ContainerFrameItemButtonTemplate`, so they run it too), and each turn is `SetOwner` + `SetBagItem` — a clear that wipes every line an addon appended. What puts them back in the reference is a hide/show pair, and we had neither half: the `SetOwner` binding hides the plate before it even reads the owner argument (`0x531179` zeroes the content gate at `+0xd0`, `0x531183` calls `[vtable+0x84]` = `0x530a60`, which is the `SetOwner(NULL)` core plus the base hide `0x76ad50`), and the content `Set*` re-shows it through the tooltip's own Show override (`0x530a80`). So every rebuild raises `OnHide` then `OnShow`. Ours cleared in place, and the base show early-outs on an already-visible frame (`0x76ae3b`), so the plate never hid and the `OnShow` never came. That is why exactly two addons lost their lines while others kept theirs: `LibItemTooltip-1.0`, the tooltip contract of MasterTradeSkills and Gromissingcrafts, hangs entirely on a scratch child frame of `GameTooltip` listening for `OnShow`, where pfUI's ItemID and sell-price lines hook the `Set*` methods themselves and so re-run on every refresh. The reference also clears twice per `SetOwner` (once inside the hide, once in the core) and so fires `OnTooltipCleared` twice; that falls out of calling the same hide, and the existing lifecycle test already allowed for it. Addresses read off the player's own `WoW.exe` with objdump, not from memory. The new test is the hover loop in miniature — three turns of `SetOwner` + a content `Set*` under a `LibItemTooltip`-shaped child probe — and fails on the old behaviour with one `OnShow` instead of three. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
gromandClaude Opus 5
|
0807144f38 |
modelframe: a unit pane holds the unit it was given
`SetUnit` resolved nothing: it stored the token and the extract re-read it every paint. unitscan's alert is the live consumer — it `SetUnit'target'`s the rare it just found by name, and the addon restores the player's own target on the very next line — so the card showed the rare for one frame and then went empty. It only looked right while our by-name targeting was leaving the target on the rare, which is the accident the previous commit removed. The reference resolves the token in `SetUnit`'s own rebuild worker (`0x505b50`, the one `RefreshUnit` also runs) and nothing re-resolves it per paint. Three things say so independently: FrameXML re-issues `SetUnit` from `PaperDollFrame_OnShow` and on `UNIT_MODEL_CHANGED`, which would be pointless against a live token; `RefreshUnit` exists at all; and pfUI detects a portrait change with *no event* by pinning a scratch pane and comparing `GetModel` against the shown one (`api/unitframes.lua:2069`), a test that says nothing unless a pane holds what it was given. `ModelState::unit_guid` is that pin, read through one helper (`Model::unit_pane_subject`) so the paint gate and the extract cannot disagree about whether a pane has a subject. A pane seated *before* its unit streamed in keeps re-reading its token — ours arrives asynchronously where the reference's is already in the object manager — and `RefreshUnit` re-pins, as its worker does. `modelframe.rs`'s own extract test passed only because its `SetUnit` ran with no target; its comment claimed the general rule, so it now states the real one and pins both halves. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
gromandClaude Opus 5
|
3cc71f4c7d |
scrollframe: the child's re-anchor is a SetPoint at the setter, not a per-frame override
`SetVerticalScroll`'s own note here already said it: `0x786db0` stores the offset,
**re-anchors the child (0x787100)** and fires its event, "all only on a change".
0x787100 passes the offsets to `SetPoint` — so the engine writes the child's
TOPLEFT once, when the scroll state changes. This applied it instead as a layout
override, recomputed every frame, which is a different thing: it made the
engine's anchor permanent and unoverridable.
That is why AI_VoiceOver's options could be opened and read but not scrolled.
AceGUI-3.0 scrolls by moving the child **itself** — `SetScroll` is
`content:ClearAllPoints()` then `content:SetPoint("TOPLEFT", 0, offset)`
(AceGUIContainer-ScrollFrame.lua:62-77) and it never calls `SetVerticalScroll` at
all — so the override put the content back at the top on every frame. Measured
through the harness: `SetScroll(1000)` computes offset 170 (content 549 − view
379) and the content's top now moves 563 → 733 where before it did not move.
`SetScrollChild` and both offset setters now write the merged TOPLEFT through
`layout_methods::write_anchor`, which is the commit `SetPoint` itself uses, so
the merge rule is shared rather than reimplemented and a child authored
TOPLEFT/TOPRIGHT keeps the TOPRIGHT that gives it its width. The whole override
path goes away with it: no third element on the layout plan, no override in the
node hash, no special case in the edge walk or in `retarget`, and
`LayoutSolver::set_frame_anchored` is gone.
One behaviour genuinely changes and the test says so now: `SetScrollChild(nil)`
no longer restores the child's authored anchor, because the authored TOPLEFT was
replaced the first time the frame took the child. Nothing was ever saved to
restore — that expectation was an artefact of the override model, not a trace.
The child keeps where the last scroll left it, and its own `SetPoint` wins from
there.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn
|
||
|
gromandClaude Opus 5
|
bbe2e3aaa9 |
target: TargetByName selects inside the call, as the reference's does
unitscan probes for a watched rare by targeting its name, reading the name back,
and calling TargetLastTarget to put the old target back — but only
`if prevTarget ~= foundTarget` (unitscan.lua:112-130). Two things had to be true
inside that one Lua frame and neither was, so the restore never fired and every
scan kept your target. TargetLastTarget itself was fine and was never even
reached.
**The read after the call.** `TargetByName 0x489d60` runs the resolver and
selects before it returns, so the next line sees the pick. Ours queued a request
the app drained later in the frame, and the token snapshots had been pushed
*before* Lua ran — so the probing frame saw no change at all, and by the frame
that did see the mob, `prevTarget` had already been re-read as the mob.
The app now keeps each query a script has asked for resolved a frame ahead
(`by_name::feed_named_targets`, one candidate walk for the whole asked set, and
nothing walked at all when no script uses the verb). With the unit already in
hand the binding answers `"target"` for the rest of the Lua frame and queues the
selection *by GUID*, so the optimistic answer and the committed selection cannot
name different units. The override is dropped at the top of the next tick. A
query asked for the very first time keeps the old one-frame-late path, and that
ask is what starts it being resolved.
**The order of two selection verbs in one frame.** `TargetLastTarget`'s memory
was sampled once a frame, so mid-frame it still named whatever preceded the
*previous* selection: the restore would have landed on the wrong unit even with a
synchronous read. The reference stamps inside the selection itself, so its memory
is always the thing being left behind; `selection_requests` now calls
`LastTarget::note` per request, and the per-frame sampler stays as the catch-all
for selection changes that are not requests (a stream-out, a death clear, a
click-off). The two compose: the test walks the whole probe-and-restore sequence
and checks both the mid-frame pick and the memory the frame ends with.
Fixes the `/target X` then `UnitName("target")` macro idiom generally, not just
unitscan — SuperCleveRoidMacros is built on it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn
|
||
|
gromandClaude Opus 5
|
0515274afc |
tests: four stale expectations this window's own commits left behind
Four benilla-ui tests were red and had been since the commits that changed what they pin. They were missed because the round's gate was run as `cargo test -p benilla-app -p benilla-ui` through a grep that only showed benilla-app's results; scripts/check.sh is the gate that scopes this correctly. - SetRaidTarget's third argument became a flag rather than the literal word "local" ( |
||
|
gromandClaude Opus 5
|
80648267d5 |
ui_aura: another unit's aura durations, learned from the GO that applies them
The target frame's debuff icons had no countdown. libdebuff was tracking fine — the probe run said `libdebuff guids: 2` after a fight, and the AURA_CAST_ON_OTHER events carried the right lengths (spell 772 Rend, dur 9000) — because the timers pfUI-for-ClassicAPI draws do not come through libdebuff at all. Its unit frames read duration/expirationTime straight off C_UnitAuras.UnitAuraBySlot (api/unitframes.lua:1680,1788), and libdebuff's own header says the plumbing "now runs on ClassicAPI's C_UnitAuras (which provides sourceUnit/sourceGUID and non-player expirationTime)". So the number came from us, and we answered 0 for every unit but the player. There is no packet to answer from: SMSG_UPDATE_AURA_DURATION is self-only, and SMSG_SET_EXTRA_AURA_INFO — which carries a caster's durations on others — is a 2.0 opcode this protocol does not have. Hence every vanilla duration addon keeping its own table. This is that table, kept client-side instead, because this client *is* the client: SMSG_SPELL_GO names the spell and its hit list, so the apply instant is the packet's own arrival rather than a combat-log line somebody has to parse, and every reader of the aura lists gets the timer at once instead of each inventing a tracker. Keyed by (guid, spell) so a re-cast refreshes in place and a recycled slot cannot inherit a stale stamp. Length comes from the same ui_cast_events::aura_duration_ms behind nampower's arg8 and GetSpellDuration, so the three cannot disagree, and its 0 — permanent, no SpellDuration.dbc row, or a combo-point/level-scaled one — stamps nothing: a timerless icon beats one counting to the wrong moment. The join is driven by the unit's own UNIT_FIELD_AURA, so a stamp for an aura the target resisted is never read, and a run-out stamp reports nothing rather than a negative remainder. Not covered, by construction: an aura already on a unit before we streamed it, and one applied out of our range. No GO, no timer — which is what the mods this stands in for also do. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
gromandClaude Opus 5
|
608b4b961b |
modelframe: a unit pane paints, so its OnUpdateModel fires
The paint that fires OnUpdateModel is gated on [widget+0x318] != 0 (0x76d24c) — the pane's loaded model. This read that as "has a file path", and a SetUnit pane keeps no path at all: its subject resolves per render, app-side, so SetUnit writes `unit` and nils `path`. Every unit pane's OnUpdateModel was therefore permanently inert. Not cosmetic. unitscan's alert model is made visible *only* from inside its own OnUpdateModel: reset() sets alpha 0 and arms the handler with a ten-paint settle delay, and the eleventh call is what sets alpha back to 1 (unitscan.lua:268-292). With the handler never firing, the alert drew a correctly placed, correctly sized, fully transparent model and the horn played over an empty card. The layout probe that said the model frame was a healthy 200x120 was right; it was just invisible. A unit token that names nothing still fires nothing, which is the "!= 0" half of the gate. Nothing in stock FrameXML or pfUI is woken by this: probed, none of CharacterModelFrame, DressUpModel, TabardModel or PetModelFrame registers an OnUpdateModel at all, and the stock consumer (Cooldown.lua) is file-backed and was already firing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
gromandClaude Opus 5
|
75432f7dde |
layout: a scroll child's re-anchor merges one point, as SetPoint does
A live ScrollFrame re-points its child's TOPLEFT to the frame's own TOPLEFT at the scroll offsets, which the reference does by calling SetPoint on the child (0x787100). This replaced the child's whole anchor list instead — and 1.12's SetPoint merges a point into the list, it does not clear it. The difference is not academic. AceGUI's scroll child is authored TOPLEFT *and* TOPRIGHT to its parent with an explicit height and no width (AceGUIContainer-ScrollFrame.lua:192-195), so the corner pair is where its width comes from. Dropping TOPRIGHT left it with one corner and no width, so it resolved to no rect at all — and with no rect for the child, nothing below it resolved either. That is what drew AI_VoiceOver's options window as chrome and tabs over an empty content area. The whole widget tree was there the entire time: probed through addon_harness, the panel builds TabGroup → ScrollFrame → four InlineGroups of CheckBoxes, Dropdowns, Sliders and Buttons, every one sized and IsVisible() true — and every one from the scroll child down answering GetLeft() == nil. After this, the same probe reads the scroll child at L=190 T=563 and its first checkbox at L=200 T=530 B=506, inside the viewport. The override's edges now include the authored anchors' targets as well, since the merge means the child reads both. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
gromandClaude Opus 5
|
0a90795555 |
compat: publish LOCALIZED_CLASS_NAMES_MALE/_FEMALE, the table tInvert exists to flip
Half this pair was already here. `ClassInfo::class_file`'s own doc says the token is what FrameXML keys `RAID_CLASS_COLORS` and `LOCALIZED_CLASS_NAMES_MALE` on, and `compat/classic_api/lua_ext.rs`'s `tInvert` names pfUI's `pfUI.lua:227` as the reason it exists — the line that inverts exactly this table. The table itself was never published, so that line inverted `nil`. Measured against the live chain: `pfUI.env.L["class"]` holds **0 entries**, and `L["class"]["Warrior"]` is nil. Every lookup through it then finds no class and skips, silently, because the consumer tests `if class then` — so the guild roster lost its name colours and class icons (`modules/socialmod.lua:18`), the friends list lost the same, and `modules/panel.lua:359` fell back to white. Chat stayed coloured throughout, which is what made it look like a pfUI setting rather than a missing global. Published from the class table the app already feeds for `C_CreatureInfo.GetClassInfo`, at that same feed, so there is one source for the names and no second reader of the DBC. Both tables carry the same string: 1.12's `ChrClasses.dbc` has one name column, so the gendered pair later clients ship cannot be derived from this data — a copy is what it honestly supports, and omitting `_FEMALE` would hand a caller nil where its sibling answers. A run that never feeds the table leaves both absent rather than empty. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
gromandClaude Opus 5
|
751cac062b |
compat: C_Spell.GetSpellEffectMechanics, the per-effect sibling
`SuperCleveRoidMacros/ClassicAPI.lua:267` calls it unguarded, so its absence was a raise a
session. It is not a second spelling of `GetSpellMechanicByID`, which this tree already answers:
that one reads `Spell.dbc`'s spell-level `Mechanic`, and vanilla stores some mechanics on an
*effect* instead. The consumer's own comment names the case — Rake's bleed is effect-level, so
the spell-level column says 0 while this answers `{0, 15, 0}` — and a macro conditional asking
whether its target is bleeding gets the wrong answer from the spell-level field alone.
`SpellDisplay::effect_mechanic` was already parsed for the cast-immunity scan, so this is a field
on the probe payload and a reader, not a new lookup. The three-way answer is its sibling's, for
the sibling's reason: nothing at all for an id no spell owns, three zeros for a real spell that
carries no mechanic anywhere, the ids otherwise — collapsing those two would reclassify crowd
control as ordinary damage, which is the hand-kept table this function exists to replace.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn
|
||
|
gromandClaude Opus 5
|
d25ef0f1d9 |
target: TargetLastTarget, a 1.12 verb we never registered
`TargetLastTarget` is in the reference's global table (`reference/1.12-globals.tsv:17678`, beside
`TargetLastEnemy`) and was simply missing. unitscan is what noticed: it probes for a watched rare
by targeting the name and reading `UnitName("target")` back, then calls this to put the old target
back (`unitscan.lua:84`). With the global absent the probe's target just stayed — which looks
exactly like the addon stealing your target every scan tick, and is what it was reported as.
Its memory is the selection held **before** the current one, any unit rather than only an
attackable one, so it is a separate stamp from `TargetLastEnemy`'s `[0xb4e2e8]` — sampled per
frame for the reason that one is, so that every writer is covered including the ones that are not
`SetSelection` calls here. The latch lives in the resource rather than a system `Local`: it is
part of the memory, and a `Local` is unreachable from a test that drives the system directly.
An empty or stale memory **deselects**. That is the one arm on this queue that does — the
reference's `0x489b00` reaches `0x493540(0, 0)` where `TargetLastEnemy`'s shim tests nothing and
returns, a difference `click.rs` already had written down — and it is what makes the verb a
toggle: probe something with nothing selected, and this puts you back to nothing.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn
|
||
|
gromandClaude Opus 5
|
fe00d16ec6 |
trainer: a service's skill line is the profession, not its group header
`GetTrainerServiceSkillLine` answered the group header text. At a class trainer the two are one string, so nothing showed; at a tradeskill trainer they are not. Every group there is labelled from `TRADESKILL_SERVICE_STEP`/`_LEARN` (`0x4d7786`), and `TRADESKILL_SERVICE_STEP` is the string "Development Skills" — so standing at the mining trainer, the stock confirmation dialog, which formats this return straight into `PROFESSION_CONFIRMATION2` (`Blizzard_TrainerUI.lua:28`), asked whether to learn **Development Skills** as a second profession. `TrainerService` now carries the taught spell's own line beside the group label, resolved through the same `SkillLineAbility` lookup the group key uses, and the getter prefers it. The header row is left alone: "Development Skills" there is the client's own label for a skill-stepping tradeskill group, not a lookup that went wrong. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
gromandClaude Opus 5
|
7098d64203 |
party: SetRaidTarget's third argument is a flag, not the word "local"
SuperWoW's extra argument marks the icon local-only, and its callers disagree on how to spell it: the wiki writes `SetRaidTarget(unit, i, "local")`, while SoloRaidTargetIcons passes the boolean `IsSuperWoWLoaded() and not IsPlayerInPartyOrRaid()` (`main.lua:51`). Typing the parameter as a `String` raised on that boolean — "bad argument #3: error converting Lua boolean to String", five times in one session's log — and it raised *only while solo*, which is the one case that addon exists for. So marking a target outside a group did nothing at all. Lua truthiness takes both spellings, which is what a flag means in this language. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
gromandClaude Opus 5
|
06011e5e5b |
ui_script: the LoadOnDemand log line stops claiming "not loaded"
A LoadOnDemand addon can already be in memory when the walk reaches its row, because an addon ahead of it loaded it from its own `ADDON_LOADED` — which is how every AI_VoiceOver data module actually arrives: 224 ms of Lua inside AI_VoiceOver's handler, before the walk gets to the `AI_VoiceOverData_*` rows at all. The line was printed unconditionally, so it went on saying "no LoadAddOn() yet" about addons that had loaded. That reads as a bug and is not one; it cost a session chasing a missing `GetAddOnEnableState` that was never the problem. Gated on a new `UiScript::addon_is_loaded`. `docs/ADDON_COMPAT.md` now records why that global stays unbuilt, since the evidence was gathered anyway: upstream ClassicAPI's own `docs/API.md` does not define it either, and AI_VoiceOver gates a modern-shaped `GetAddOnInfo` on the same `if not GetAddOnEnableState`, so defining it would leave `VoiceOver.lua:84` reading `loadable` where it means `reason` and pop the "delete your old folder" dialog on every login. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
gromandClaude Opus 5
|
75f04b4f28 |
compat: FontString:GetStringHeight, which AceGUI calls unguarded
1.12 registers no `GetStringHeight` (`script/button.rs`: `0x782390` reads the label's height instead) and ClassicAPI backports it, so `compat/classic_api/frame.rs` is its home. That file had it on its own not-built list under "needs per-line layout facts the host measurer is not asked for today" — true of `GetNumLines` and `GetWrappedWidth`, and not of this one: the whole-string height is the other half of the number `GetStringWidth` already reads. `measure::current_measure` now owns the staleness rule for both, so the two cannot disagree about when a measure has gone stale, and a changed string still reads 0 rather than the previous string's height. It is not cosmetic. AceGUI-3.0 calls it **unguarded** — `AceGUIWidget-Label.lua:54` is `height = label:GetStringHeight()` and `AceGUIWidget-CheckBox.lua:90` is `self:SetHeight(28 + self.desc:GetStringHeight())` — so every Ace options panel on this install drew its labels and checkbox descriptions at the library's own "avoid zero-height labels" fallback of one pixel: a window with all of its prose invisible. `addon_harness --probe AI_VoiceOver` builds that panel's full 32-widget tree with zero errors and every `Label` at `624x1`, which is what "can't see the actual options in VoiceOver" was. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
gromandClaude Opus 5
|
c1ef619295 |
trainer: a rank-less service subtexts empty, not nil
`GetTrainerServiceInfo`'s second return was nil whenever the spell had no rank. The client reads that field out of the spell record's string block, where an absent rank is the empty string at offset 0 rather than a null, so it is always a string — and the stock window depends on that: it formats the subtext unguarded at `Blizzard_TrainerUI.lua:275`, twenty lines before its own `not serviceSubText` fallback. So a nil raised inside `ClassTrainer_SelectFirstLearnableSkill`, which `ClassTrainerFrame_Show` calls on the line above `ClassTrainerFrame_Update` (l.48-49). The update never ran, so the list came up with no rows at all, and `SelectTrainerService` (l.277) never ran, so the Train button had nothing to act on. A trainer window that shows nothing and trains nothing, from one nil. Every rank-less service reaches that line: every profession recipe, and class abilities like Parry. The name beside it stays optional, because the stock window says so itself — it tests `serviceName` before using it as a header's (l.209) and falls back to `UNKNOWN` for a service's (l.287). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
gromandClaude Opus 5
|
81d96af35d |
Everwood: the HD/OctoWoW line of benilla, imported onto upstream
This checkout had no git history at all, so this one commit carries the whole
of the Everwood work as a single import on top of chairmanpow/benilla-octo's
`main` (
|
||
|
chairmanpow
|
706cd717a3 |
merge: absorb samwhosung/main on top of the upstream merge
Same fork policy: upstream structure wins, local-unique re-instated. Cargo.toml unions both measured dev-profile philosophies (codegen-units=1 + lto=off from samwhosung, debug=line-tables-only + split-debuginfo from the fork's disk measurements). glue_booth.rs takes samwhosung's typed GlueScene (token/of_race/from_model_path) and re-adds the fork's TurtleWoW races (Goblin=9, BloodElf=10) as variants; their glue fog stays on Human's row, exactly as the fork's scene_fog fallback did. ui script takes samwhosung's mouseover/hyperlink rework (world_mouseover, minimap_blip, hyperlink.rs) over the fork's inline hover_link/span_at/pointer path — the rework covers the same OnHyperlinkEnter/Leave behaviour. game_plugins.rs keeps the fork's re-measured ceilings (466 / 5,509); mail.rs keeps the explicit 13-tuple with a float daysLeft; MAP.md takes the superset. |
||
|
chairmanpow
|
26d714fd1d |
merge: absorb upstream/main into the Turtle fork
Fork policy (docs/METHOD.md): upstream structure wins; local-unique work re-applied on top. This commit is the upstream half of the reconciliation. Upstream is the same program at a rewritten history ( |
||
|
Sam SungandClaude Sonnet 5.5
|
2af917044d |
Stock login fetches the server alert; LaunchURL behind the gate (#1040)
The stock login's Breaking News box (`WOW_GLUE_STOCK=1`): - At glue init, as `0x46a400` does, the client reads the Lua global `SERVER_ALERT_URL`, which the player's own GlueStrings or a server's patch sets. It fetches the URL once on a background thread: plain HTTP, 5 s timeouts, redirects followed. - The body builds the reference's buffer (`0x46a580`: 0x7ff cap, appended chunks). A case-insensitive `SERVERALERT:` prefix fires `SHOW_SERVER_ALERT` once from the update (`0x46b939`). Every failure is silent. - `LaunchURL` opens the browser only for URLs that pass the reference's gate (`0x5abc10`): `http://`, a strict charset and the 10-host whitelist. An over-long host is refused rather than cut. - No fetch or browser ever runs in a test, capture or unattended run. Tests run against a local fake server: prefix, 404, redirects, chunked, timeouts, caps, and the frame shown end to end. Part of #1026 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> |