e17d7dff04647b296657f3bc97104625ffbe7efe
663
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
17ee517602 |
An unpainted loot-roll name is the reference's own placeholder, not nothing
`nothing_repaints_a_frame_that_opened_before_its_snapshot` asserted that a frame
which never painted has no name text. It has one: `LootFrame.xml` authors
`text="Item Name"` on `$parentName` as a design-time placeholder, and every copy
of that file carries it — the realm's `patch-4`, `patch.MPQ` and stock
`interface.MPQ` alike. So "stays blank" means **unchanged**, which is what the
test is actually about, and it now reads the untouched label once and compares
against that rather than against nil. The realm owns its own placeholder.
Leaves the five other `group_loot_tests` failures, and they are a different
thing worth stating: the realm has **rewritten `GroupLootFrame_OnShow`**. It now
opens with
local texture, name, ... = GetLootRollItemInfo(this.rollID)
if ( not name ) then this:Hide() return end
where the stock body had no such guard and called `SetText(name)` with a nil.
So on this realm an in-flight roll does not open at all until its name is known,
and `in_flight_roll_opens_without_a_name_or_icon` encodes the older behaviour in
its name as well as its assertions. That one needs a probe of what
`GetLootRollItemInfo` answers in that state before anything is rewritten — the
visibility assertion above it passes, which does not fit either reading yet.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn
|
||
|
|
cb8eb7f01d |
The chat dock indexes the video window unguarded, so the kit owes it
`not OptionsFrame:IsShown()` at `FloatingChatFrame.lua:862`, inside the hover pass `UIParent`'s OnUpdate drives — an unguarded index of a global the kit only seated from `IsOptionFrameOpen` and `ToggleGameMenu`, neither of which that path calls. Every kit that drove a chat hover raised on it. Given as a prerequisite rather than a stand-in, because the realm's toc loads the stock video window 29 rows above the chat one (row 29 against 58): that is the toc's own order, and it hands over the real frame instead of something that would shadow it the way the `ChatFrame1` stand-in did. `chat_resize_tests` 7 → 0, `chat_options_tests` 7 → 0, `chat_tests` 6 → 1. 116 → 97, nothing newly failing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
|
1aa3418fdc |
Read the test that reports errors first: 20 more, from four causes
The guild and friends clusters — 26 tests between them — were one nil function.
`guild_tests::driving_the_guild_windows_raises_no_script_errors` exists to name
its errors, and all four of them were the same line: `attempt to call global
'GetDifficultyColor' (a nil value)` at `FriendsFrame.lua:523`, reached from four
entry points. `GetDifficultyColor` is FrameXML's, not a client global
(`reference/1.12-globals.tsv` types it so), and `QuestLogFrame.lua` defines it —
ten toc rows above `FriendsFrame.xml`. The raise took the rest of the roster
update with it, including the `GuildPlayerStatusFrame:Show()` that ends that
branch, which is how it read as "the guild tab opens and the player pane is not
visible": an assertion about visibility with no hint of a nil function behind it.
One prerequisite row took guild 17 → 1 and friends 9 → 1.
The reputation, honor and skills pages (20 more) shared a fixture gap: their
player `UnitState` had a level, a race and a class and **no name**. The realm's
`CharacterFrame_OnShow` formats `gsub(UnitPVPName("player"), "\n.+", "")`, and
`UnitPVPName` answers nil for a nameless unit, so every one of those pages raised
`bad argument #1 to 'gsub'` on the way up and never finished showing.
The inspect page's arena tab raised three more: its team buttons inherit
`ArenaTeamTemplate`, which is a FrameXML row of the realm's own
(`Turtle_ArenaUI\Turtle_ArenaUITemplates.xml`) and not in the addon's templates
file, so the inherits was skipped, `$parentText` was never built, and the OnLoad's
`_G[this:GetName().."Text"]:SetText("(2v2)")` had nothing to index. That fixture
already said "Before the inspect addon, as `inherits=` resolves at load" — it was
simply missing this one.
And five expected a rank title without its trailing space. The realm's
`GlobalStrings.lua` gives every rank one — `PVP_RANK_8_1 = "Master Sergeant "`,
where stock 1.12's copy has none — because its own callers concatenate a name
after the title. The titles are now read off `PVP_RANK_<rank>_<team>` through one
helper, so the realm owns its own strings; that also turned up a wrong index
hiding behind the stale spelling (Grand Marshal is rank 18, not 14).
136 → 116, nothing newly failing.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn
|
||
|
|
b39d901257 |
Twelve more, all of them numbers the realm moved out from under
Both clusters left after the chat-window shadow were stale numbers written into tests rather than anything of ours, and both are stale because the realm's FrameXML moved — which is the thing to suspect first in this crate. Seven in `battlefield_minimap_tests`: the manifest was expected to warn exactly once, about `gxRefresh`. This client does not register that CVar because no target offers the exclusive mode-set a refresh rate needs, and the only reader was stock `OptionsFrame.lua`'s `OptionsFrameRefreshDropDown_OnLoad` — which the realm replaced with a rewrite that has no refresh-rate dropdown at all: three mentions of the CVar in `patch.MPQ`'s stock copy, none in `patch-4.mpq`'s. The list is now empty rather than gone, so a realm that restores the stock window brings the row back and that is where it shows. Five in `questlog_tests`, from three separate numbers in the one file: `"Quests: 8/20"` — the realm sets `MAX_QUESTLOG_QUESTS = 25`, and this file had already learned that once: another test reads the cap off the global with a comment saying why. This call site had been missed, so it is read the same way. A wheel notch was asserted to scroll the list, with eight fixture quests described as "two more than the list shows". The realm shows `QUESTS_DISPLAYED = 24` where 1.12 shows six, so eight fit with room to spare and the notch had nothing to move — the `assert_ne!` was passing only while the row count read as six, which is to say it stopped testing anything a while ago. The log is re-seated past the window for that leg, sized off the live global, and put back to the eight the rest of the test reads. And `frame_rect(quads, 300.0, 261.0)` / `(384.0, 512.0)` hunted the quad list for a bare frame of a given size, which is a number the realm can change and did. `named_rect` asks the frame where it is instead. Nothing any of those tests asserts was ever about the size: they clip reward plates and text against the pane and compare the window's own edges. 269 → 160, still nothing newly failing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
|
eeebb5abf0 |
A stand-in shadowed the real chat window, and took 96 tests with it
`benilla_seat` — the test kit's stop-gap for frames a one-window kit does not
load — seats one for every row of `UIPARENT_MANAGED_FRAME_POSITIONS`, and
`ChatFrame1` and `ChatFrame2` are rows of it. The pass that triggers the seeding
runs **at load**, from `ShapeshiftBar_OnLoad` (`BonusActionBarFrame.lua:109` →
`:157`), twelve toc rows before `FloatingChatFrame.xml` builds the real windows.
A frame's publish to `_G` is non-overwriting (`0x701bd0`), faithfully, so the
stand-in stood and the real window — correctly built, correctly parented — was
unreachable by name for the rest of the VM's life.
`FloatingChatFrame.lua` then read the stand-in back three times: `FCF_SetWindowName`
concatenating its nil `GetName` (l.693), `FCF_SaveDock` handing
`SetChatWindowDocked` window 0 (l.1284), and `FCF_UpdateButtonSide` doing
arithmetic on its unresolvable `GetRight` (l.1019). Every kit that seats any part
of the chat UI died on the first of those, which is **96 of this crate's tests** —
27 bag, 26 guild, 18 raid, 18 friends and the rest — all reported as one
`assert!(report.errors.is_empty())`.
The two rows are now excluded from the seeding, and the exclusion is the point
rather than a special case: the pass reads a managed row *only* through
`frame = _G[index]; if ( frame )` (`UIParent.lua:1962`) and compares at l.2024
with `frame == ChatFrame1`, which a nil global answers without raising — so these
two never needed a stand-in at all. The rest of the table still seats, because the
custom-positioning tail does read some rows unguarded, and one of those
(`MultiBarBottomRight`, l.2070) is not in the helper's hardcoded list.
Two smaller things on the way, each right on its own and neither enough alone.
Stand-ins are created **named and with the id their name implies** — a bare
`CreateFrame("Frame")` answers `GetName()` nil and `GetID()` 0, the publish is
`setglobal` either way, so the naming costs nothing and it is what a stand-in is
for. And `floatingchatframe.xml` gains `ChatFrame.xml` as a prerequisite, which
the realm's toc puts immediately above it.
Four tests in `loader/tests.rs` are what ruled the loader out, each killing a
hypothesis about template `parent=` inheritance: it reaches the frame that
inherits it one level down, two levels down, with the chain's base in an earlier
document, and for a `<ScrollingMessageFrame>`, which is the kind `ChatFrame1` is.
None of them was the bug — the frame was always built with its parent, and only
its name never reached `_G` — but they pin behaviour the realm's chat windows
depend on and they are how the next one of these gets cheaper.
269 → 173, with no test newly failing.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn
|
||
|
|
f4960efc53 |
The realm update lands clean, and three of the "269 known red" were real
His 2026-10-28 20:18 update rewrote `Data/patch-4.mpq` (1.37 GB) and
`patch-5.mpq`
(230 MB). Verified rather than assumed, and nothing of ours broke: the realm's
`FrameXML.toc` went 146 → 151 rows and patch-5 holds the richest copy, so
`chain::demote_stale_snapshots` fires and it wins the manifest; the five new
rows
are realm modules (`OctoChallengesSession`, `OctoHardcoreBGMenu`, `OctoReplay`,
`OctoReplayBar`, `OctoReport`) that `unnamed_chain_rows` picks up by design and
that load with **0 load errors and 0 session errors**, every global they name
resolving; DBC field counts are unchanged everywhere, so no parser is at risk,
while record counts moved (Spell 27992 → 28026, CreatureDisplayInfo 13423 →
13451), which is the proof the new data is live; a `WOW_CAPTURE=vista` at
Ironforge prints **0 ERROR lines**; and both golden baselines held exactly, 269
with an identical failing name set and 80, `goldshire_cell` still at 1874.
Two examples answer "did it land", because "the file is in the archive" never
does. The reference mounts `patch-?.MPQ` by case-folded name, so every
`patch-A..Z` outranks `patch-4`/`patch-5`, and the toc-subset demotion cannot
see
an archive that carries no toc at all. `chain_report` prints the `FrameXML.toc`
copies in priority order and, with `--toc`, the winner of every row of the
realm's manifest with a tally — 68 to patch-4, 43 to patch-5, 32 to `patch.MPQ`,
7 to `interface.MPQ`, 1 to patch-3, and **none to a stale letter patch**.
`chain_shadow` answers from the archive's side: patch-4 carries 20326 paths of
which 10220 are shadowed, patch-5's 1494 are not shadowed at all.
Then, grouping the red bucket by panic *site* rather than trusting its label:
252 of the 269 are shipped-UI tests and **136 share one site**. Three of the
rest
were real, and are fixed here.
Six `OptionsFrame_*` enable/disable helpers were missing outright. `benilla.toc`
is the only thing that loads two of the reference's settings windows — the
realm's own toc lists neither `SoundOptionsFrame.xml` nor `UIOptionsFrame.xml` —
stock 1.12 keeps those six beside them in its own `OptionsFrame.lua`, and the
realm replaced that file with Turtle's rewrite, which drops them. Checked
against
all 151 interface rows, Lua and XML: nothing defines one. So those windows held
controls that could not grey out, with the first raise waiting on the click that
reached one. `assets/ui/OptionsFrameAdapters.lua` carries the reference's own
bodies, read out of `patch.MPQ` rather than from memory.
`CHARACTERFRAME_SUBFRAMES` was asserted to hold exactly five. The realm's
`CharacterFrame.lua` carries a sixth, `ArenaFrame` — which this client already
stubs (`CharacterFrameAdapters.xml`), so the count was the only thing wrong. It
now checks the reference's five by membership and order, which is what the test
is for; a realm may append.
And the kit's stand-ins were anonymous. `CreateFrame("Frame")` with no name
answers `GetName()` nil and `GetID()` 0, the publish is `setglobal` either way,
so naming them costs nothing and the absence cost two of the 136's three errors:
`FCF_SetWindowName` concatenating a nil name and `SetChatWindowDocked` refusing
window 0. `ChatFrame1`/`ChatFrame2` are no longer seated at all — every row of
`UIPARENT_MANAGED_FRAME_POSITIONS` is read through `frame = _G[index]; if
( frame )`, so a missing one is skipped, and seating them only shadowed the real
windows for good. The pass runs at load, from `ShapeshiftBar_OnLoad`, twelve toc
rows before the chat files, which is why the stand-ins were always first.
That leaves 136 on one cause, and the two tests added to `loader/tests.rs` are
what ruled out the first hypothesis: a template's `parent="UIParent"` does reach
the frame that inherits it, one level and two, which is how `ChatFrame1` is
built. The next suspects are written down in the gates memory.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn
|
||
|
|
ee9cfdcacf |
Two settings for things arriving: a foliage distance row, and a ramp for lights
**Foliage Distance** (`foliageDistance`, Advanced Graphics, 70..300 yd). The reference draws ground clutter to a hardcoded 70 yd (`[0x867958]`) and we inherited it, so grass, reeds and small bushes arrive at a fixed ring around the player — which is exactly where the eye is drawn to them appearing. The row moves the ring; it changes nothing about what grass looks like. Measured on an Elwynn fence line (`fence-shadowline-day`, clutter density 2), p50 frame time / main thread: 70 yd **3.25/3.26 ms**, 140 **3.32/3.24**, 200 **3.48/3.38**, 300 **3.69/3.42**, with the GPU flat throughout. Doubling it is free; past 140 it costs main thread, which is the budget this client is short of. The registered default IS the reference's own 70 and the row is in no preset ladder, so nothing moves until a player asks it to. It commits on Apply: `fade_far` is baked into the clutter material key, so a change drops every built patch to be re-cut (338 meshes on one measured write) and a drag would do that thirty times a second. `clutter::remesh_on_cutout_change` is now `remesh_on_material_change` and watches both the alpha ref and the fade for that reason. What it buys in a STILL is small — 70 -> 300 moves 0.6% of a frame, the far tufts being sub-pixel — and that is the wrong instrument: the complaint is grass ARRIVING, which no static capture can show. **Light arrival fade** (`lighting::global_light`). No setting: a correction, not a taste. A fixture enters the packed light table for several unrelated reasons — the portal walk reaching its room, its WMO group streaming in, the camera crossing `INTERIOR_NEAR_ADMIT` or `POINT_PACK_RADIUS`, the 255-entry capacity cut re-sorting — and every one of them was a hard edge that switched a light on at full brightness between one frame and the next. Measured in Ironforge by diffing the admitted set between static vantages: walking **3 yd exchanged 2 of 172 admitted lights, and 10 yd exchanged 8** — about one arriving per yard, which at a walk is several a second. The table ran 110-196 across five vantages and never reached the cap, so this is admission churn and NOT capacity; the cap does bind transiently during a world load (305 candidates seen once), which is why the boundary fade below is kept. Arrivals now ramp over 0.35 s, whichever rule admitted them. The scale lands on the light's colour before the pack, so the GPU table, the CPU mirror, the fog lamps and the froxel volume all follow from one write. Deliberately NOT symmetric: a departing fixture still goes at once, because keeping it alive to dim means keeping its room claim alive with it, and a stale claim gates a live light with a dead building's identity. **Spell lights are exempt**, and that is the exemption that matters: a fireball's light lives about as long as the ramp does, so ramping it would spend the whole effect at half strength. Beside it the capacity cut itself is softened — the last 24 admitted ranks scale toward zero across the distance band they span — and nothing fades while the table fits, which is what makes the control possible: with the ramp on against off, the Goldshire inn and the Great Forge both settle BYTE-IDENTICAL. Also recorded in docs/GRAPHICS.md: captures force `WOW_CLUTTER_DENSITY=0` for byte-stable baselines, so a run that sets only `WOW_CLUTTER_FADE` measures a scene with no grass in it and reports every horizon as free. Three rounds of this measurement were worthless before the starved control caught it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
|
33fc0c2023 |
Froxel volumetrics: the haze stops painting shadowed air the colour of sunlit air
`volumetricFog` gains tier 3, "Volume", with a "Shadowed Air" dial beside it; the Ultra rung of Lighting Quality takes it. A view-space 160x90x64 froxel grid — a fixed BUDGET, so every resolution costs the same — marched one compute thread per screen COLUMN, front to back, with a reprojected eight-frame history. Inject and integrate in one pass, because a froxel past the last shadow cascade has no evidence either way and the honest answer is the mean of what the rest of its column could see; a thread that owns the column can carry that and a froxel-per-invocation dispatch cannot. It publishes two things: - **the lit share** — how much of the air in front of each pixel the sun actually reaches, weighted by each slice's own contribution to what the eye sees through it. `volumetric_fog`'s haze multiplies its colour by it. The haze below this rung is the reference's: a zone colour mixed in by distance, the SAME colour whether the air it tints stands in open sun or a mountain's shadow, and no screen pass can tell it otherwise because a shaft march only ever ADDS light. On an Elwynn overlook at noon, 74% of the frame is air the sun does not fully reach. - **the lamps' in-scatter**, free once a volume exists: the phase lobe evaluated at every point of the air rather than once per ray at its closest approach, attenuated by the medium it crosses and strengthened when the medium is. `lampFog`'s analytic halos retire into this solve at tier 3, and that row still chooses which lamps and how many. The gain is derived, `0.9 / density(clear night)`, so the two lanes match on a clear night. At the Goldshire inn, isolating each by rendering lampFog 0 against 2 on both (the bases are byte-identical, which is what makes it a comparison): the volume's are 1.30x and wider. The sun's SHAFTS deliberately stay on `volumetric_light`'s per-pixel march, and that is a measurement. A volume bands in hard horizontal steps at 160x90, 320x180 and 534x300 alike, because one froxel column hands the same value to about a hundred pixels and a canopy's shadow is finer than any grid worth paying for. Spreading each column's samples across its own screen footprint (R2 down the column) turns the steps into a gradient and is worth knowing for any volume, but the march still wins on detail. docs/GRAPHICS.md has the sweep. Safety is one number: fully lit air multiplies by EXACTLY 1, so at Shadowed Air 0 the lane renders byte-identical to High over all 1,440,000 pixels with every pass still running. Getting that exact needed `1 - (1-floor)*(1-lit)` and not the `mix(floor, 1, lit)` it equals — `mix` is a fused multiply-add and lands a last ulp below 1.0, which moved five of nine tier 0-2 control pairs with the rung switched OFF. The shipped floor, 0.85, is deliberately half the depth of the first one tried, and the dial reaches twice it. Control: nine tier 0-2 pairs against the previous build, five byte-identical and four differing by one or two PIXELS out of 1,440,000 at 1-2 of 255 — every tier-0 pair identical, and tier 0 is the tier that switches the fog pass off, so the remainder is the fog shader's instruction schedule changing because the shader did. Cost is under the noise of both instruments: the frame probe puts Volume inside High's own run-to-run spread, and the GPU pass timer puts `gpu_other` at 0.75 -> 0.76 ms. Also recorded: `water-lake` leaves the deterministic row once shadows are forced on — it differs from a build of ITSELF by up to 88/255 on 0.03% of pixels. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
|
11dda4f9cf |
Two cheap lanes: RCAS sharpening, and a field of view a player can move
Both were named in docs/GRAPHICS.md as nearly free and neither had been built. They are unrelated to each other; they land together because each is small. ## Sharpening (`sharpening`, 0 Off / 1 Low / 2 Medium / 3 High) RCAS, FSR 1.0's sharpening half taken on its own. It does not resample, so it needs none of the upscaler's motion vectors, jitter or history — which is the whole reason it can land while §7 cannot. One 5-tap cross; the lobe is not a constant but the widest one that cannot push the result outside the ring's own range, solved per channel against a peak of 1.0 and taken as the most conservative of the three. Flat regions get nothing and nothing rings. The seat is most of the design. `Core3d`, after `FfxGlowLabel` and before `Node3d::Bloom`: - After the glow, not before. `FfxGlowNode` builds the blur chain the glow and the underwater haze read, and a sharpened bloom source is how an unsharp mask turns a lit window into a ringed halo. - The world view only. The world's last reader is the FFXGlow combine drawn inside the UI camera's main pass, which binds `main_texture_view()` at node-run time per frame, precisely so a `post_process_write` upstream lands. The interface is drawn afterwards into a different target — and sharpening text is the one thing this must never do. - Every tap saturates, as the combine's own scene read does one pass later: the limiter is solved against a peak of 1.0, so a super-white tap would invert the bright side's sign. Nothing is lost, the blur chain already took its copy. - No denoise term. The deband dither it would exist to suppress is added downstream, in the combine's exit. Measured, 600 frames: Redridge overlook 2.95 -> 3.02 (Medium) -> 3.01 (High); Goldshire inn 3.46 -> 3.51 -> 3.29. Inside the noise, the inn reading *faster* at High being how you know. CPU flat. What it buys: 3.8% / 19.9% / 48.6% of the frame moved by more than 2/255. **The first run hit the trap this repo already has a note about.** `max(4.0 * mx, 1e-5)` where `mx` is a `vec3` is `error: inconsistent type passed as argument #2 to max`. naga refused the shader, the pass drew nothing, and the capture came out looking perfectly normal. One `grep -ciE 'error:'` on the capture log answered it before a single image was diffed. ## Field of view (`fieldOfView`, vertical degrees, 40..70, default 45) Field of View on the Graphics page under Render Scale, live as you drag. It is the BASE lens, not a fourth factor. `player::scoped_view` already multiplied two things in and neither is a field of view: SuperWoW's `FoV` is the reference's `[camera+0x40]` as a ratio, the spyglass is a zoom off normal. Both answer "how much wider than normal"; nothing a player could move answered what normal IS, and the answer was a `const` in `benilla-world`. So the term order is `base × FoV × zoom`, and an addon writing `FoV` still means the same fraction of the lens the player chose. Vertical degrees, not horizontal, which keeps the reference's Hor+ framing: a fixed vertical lens widens horizontally as the window does, so an ultrawide player gains picture at the sides instead of losing it off the top. The default is `CAM_FOVY` to the bit — `45f32.to_radians()` and `FRAC_PI_4` are both `0x3f490fdb` — which the defaults-mirror test now asserts on `to_bits()` rather than against a tolerance. An ulp of projection is a pixel in a capture diff, and a row that reframes the world for a player who never opened Options is not a row, it is a regression. ## Both lanes off: byte-identical Against a `git stash`ed build of the same tree: `overlook-noon` and `water-lake` identical, and `fieldOfView=45` written explicitly identical to the row never touched (40 and 65 move 97.6% and 99.3% of the frame). `inn-noon` is excluded WITH PROOF rather than quietly: two runs of the *same* binary differ on 37.8% of their pixels by up to 199, because that scene's flame emitters do not reproduce. Its own before/after delta is 0.00% of pixels at a maximum of 1. A scenario that cannot reproduce itself cannot be a control, and the number to check first is always the baseline against itself. ## Seats Six Options edits each, per the recipe, including the silent one (`OPTIONS_PAGE_ROWS`). Sharpening is a dropdown on Advanced Graphics with the other screen effects and reuses the existing `qOff/qLow/qMedium/qHigh` labels; Field of View is a `raw`-format slider on Graphics, whose label is literal because that page is not localised. Preset ladder: `sharpening` takes `ambientOcclusion`'s shape (`0 0 1 2 2`) because a look change starts at Medium in this file's convention; `fieldOfView` is on no rung at all, because it changes how the game feels to play and that is the player's call. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
|
280c2e886f |
Planar water: the mirror works, and ladders in at waterQuality 3 (Ultra)
The starburst was three defects, and none of them was the oblique projection I blamed it on in the last commit. What found them was looking at the mirror's own render target instead of reasoning about the composited frame — the water reads it through a fresnel weight, a rippled UV, a plane-match band and an SSR march that paints over it, so from the outside a broken mirror and a broken water shader look identical. WOW_MIRROR_DUMP=1 now writes that target beside the capture. (Bevy's own save_to_disk cannot: try_into_dynamic knows four 8-bit formats and answers UnsupportedFormat for Rgba16Float, logging and writing nothing.) - The oblique row's scale. Replacing the near plane with the water plane costs the FAR plane: `z >= 0` stops meaning "nearer than CAM_FAR" and becomes `depth >= a*d`, a plane through the eye tilted by a. Lengyel's own scale puts that plane exactly through the frustum's far corner, and the sky dome is pinned just inside the far plane, so it straddled the boundary and was sliced along its own tessellation. Scale by the supremum of d/depth over the frustum instead, with 2% of margin. - The water surface lay ON its own clip plane. This was the starburst and it was fatal rather than untidy: depth now means "distance above the plane", so a fragment at d=0 reads z/w=1, the nearest value reverse-Z has, and beats every other surface in the frame. The mesh is wave-displaced, so its triangles straddled the plane instead of lying in it, and each was clipped to a sliver that kept the winning depth. SURFACE_CLEARANCE lifts the plane a yard: the swell sums to 0.34 and the probed height answers for the surface under the eye, not this fragment. - The mirror was stored upside down. A true reflection is what a camera on the far side sees with its up vector reflected too — an improper basis no Transform can hold. The honest right-handed camera sees the same surfaces through the same frustum and stores the image flipped, so the shader samples 1 - v; that costs no winding flip, so backface culling stays correct. It read as a lake reflecting the ridge the RIGHT way up. Measured: Lake Everstill 2.87 -> 3.36 ms mean over 600 frames (+17%), the Stormwind canal inside the noise, CPU +0.2 to +1.1 ms. It is the first lane in docs/GRAPHICS.md that spends main thread, so it is a rung of its own above High and the default stays 2. Seated as a setting, per the standing rule that no graphics lane hides behind an env var: waterQuality gains tier 3, the Ultra preset takes it, and the Advanced Graphics dropdown gains a fourth entry in both locales. With the lane off, both scenarios are byte-identical to the build before it — which took two findings of its own: - The system could not live in Update at all. `&mut Transform` on a camera is shared with everything that moves anything; declaring it put 59 undeclared orders into Update and tripped the schedule ratchet, which is what kept this behind an env var. It belongs in PostUpdate with the skybox and the billboards, after propagation and before visibility, where it also reads the eye's settled pose instead of a frame-old one. (The comment blaming ResMut<Assets<Image>> for those 59 was wrong — dropping the asset handle changed the count by zero.) - A system that runs and returns on its first line is still not free. Holding &mut Camera and &mut GlobalTransform for a frame is enough to change which undeclared orders the executor picks: the canal moved 0.75% of its pixels by up to 96/255 with the lane switched off. So the whole system sits behind a run condition rather than an early return — and the same effect killed a .before(UpdateDirectionalLightCascades) order that was correct in itself. PostUpdate's ambiguity ceiling goes 466 -> 495 with the reason (the pairs are structurally impossible: nothing else in the schedule names the mirror camera); Update's ratchets down 4973 -> 4952. Also restores scripts/gates.sh's `where` example, which had gone missing. Its absence was silent — cargo's error went to /dev/null and `|| true` made it read as "no install" — so the memo key and the player-tests gate saw none even when WOW_DATA was set. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
|
77fce9210e |
The water already reflected the scenery; it was switched off
`docs/GRAPHICS.md` said we had refraction and foam but no mirror. That was
wrong. `waterQuality` 2 has carried a depth-marched screen-space
reflection of the scenery all along (`enhanced_water.wgsl`, "HIGH"), with
bisection refinement against the real scene depth rather than the fixed
probe its WarcraftXL source took. What was missing was the switch: the
registered default was Enhanced (sky only), and of the five lighting rungs
only Ultra reached High.
The line holding it back said exactly what it was waiting for —
"Mirror reflections stay opt-in until their cost is measured." So I
measured it, `WOW_FPS_PROBE=300`:
Classic Enhanced High
Lake Everstill 1.91 ms 1.84 ms 1.90 ms
open ocean 1.89 ms 1.88 ms 1.89 ms
Every tier is inside the run-to-run noise — Enhanced lands FASTER than
Classic on the lake, which is how you know you are reading noise rather
than a cost. What it buys is not noise: the far bank and the mountains
appear in the water, 18.5% of the lake frame and 35.1% of the ocean frame.
So High becomes the default and the High rung. That is a weld across four
places the tests hold together, and each one bit in turn and was fixed as
it asked: `REGISTERED`, `VideoConfig`'s literal, `WaterQuality::default`,
and the rung itself — the High rung IS the registered defaults, which
`the_high_preset_is_the_registered_defaults` states and enforces.
Low and Medium keep Enhanced deliberately, and that needed saying in the
one test that assumed every rung between Off and Ultra sits AT its
default: free on the machine it was measured on is an argument for the
default, not for the rung whose whole job is hardware that cannot afford
what this one can. `water_and_lava_observers_clamp_and_water_high_is_opt_in`
asserted the old decision in its name and its body, so it is renamed and
now pins the pairing the other way: High reflects, Medium does not.
The honest remaining gap is written into the doc rather than claimed as
done: SSR gives up when the ray leaves the screen and fades toward the top
edge, which is where a shallow-angle lake wants its far bank. A planar
pass would fill that — and it is the first item on the list that draws the
scene twice, so it costs main thread and gets priced against item 1's
view-distance table before anyone builds it.
Verified: ALL GATES GREEN (492 s); `benilla-app --lib` against the install
at the exact 269-failure baseline, set-identical by `comm`.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn
|
||
|
|
88b6998453 |
Auto exposure: dark rooms read at their own level
The programme's item 1 was terrain horizon occlusion, ranked first on the reasoning that it would make the frame cheaper on the thread that is actually full. **I measured it before building it, and the premise does not hold for this renderer**, so this is item 2 instead. The numbers are in `docs/GRAPHICS.md`: cutting the view distance from 777 yd to 177 — removing four fifths of the visible world — saves 0.44 ms of a 3.6 ms frame, p95 is flat at 6.4 ms at every distance, no main-world system in a steady vista exceeds 0.12 ms/run, and the model cull already takes 6,016 submeshes to 29 drawn. Horizon occlusion can only remove the fraction of that which sits behind a hill. The reference needed it on a 2004 CPU with no GPU-side culling; we do not. What the same harness said about item 2 was the opposite. Before anything was written: the Elwynn vista renders at a gamma median of 87.6/255 with 1.4% of its pixels under 32, the Goldshire inn at noon at 31.9 with 50.1% under 32, and the inn's kitchen at **12.8 with 94.5% under 32**. That is not a dim room, it is a black screen — and the reference has the same problem, because it has no metering either. A histogram-and-reduce over the HDR view target, smoothed in the log domain, applied ahead of the shafts, bloom and glow chain so FFXGlow's blur — which is built from the scene — sees the same exposed image everything else does. Two departures from the textbook version, both forced by what this renderer's pixels mean (the world is drawn in the reference's gamma convention, which the combine clamps to 1.0): - The gain has a **floor of 1.0**: it only ever brightens. Aiming at the exterior's own measured level is what lands it at 1.0 outdoors with no special case — the vista's median moves 87.6 -> 87.5, and 0.12% of its pixels with it. Daylight is the one look this client is measured against and nothing here was worth moving it. - It has a **ceiling**, which is the tier. The kitchen meters to roughly 24x; uncapped it comes out as daylight, which erases the room instead of making it readable. Subtle and Full are 2x and 4x: "a dark room you can see in" and "one you can read in", neither "a lit room". Measured after: inn noon goes 50.1% -> 4.6% of pixels under 32, the kitchen 94.5% -> 40.3%, and the vista does not move. **+0.08 ms of GPU**, nothing on the main thread. The calibration is the one thing to get right if this is ever re-derived, and it is written down because I got it wrong first: the aim level is the level of the scene THIS PASS SEES, not of the screenshot. The glow adds `w·blur²` downstream, so the finished vista log-averages to 0.0931 linear and the pass's own input to 0.060. Calibrating against the former aimed the lane 1.5x high and brightened the daylight exterior it exists to leave alone — which the vista capture caught immediately. Two rows on Advanced Graphics, both locales, the blend greyed while there is no gain to blend; on the Graphics Preset ladder from Medium, in the shape the other post lanes take. Classic keeps it off, as the reference has no metering at all. Verified: ALL GATES GREEN (550 s); `benilla-app --lib` against the install at the exact 269-failure baseline, set-identical by `comm`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
|
50fd1f158e |
The interior lighting rig gets its ten rows, and the audit closes
The last ten CVars `video::on_cvar` answers that a player could not reach. With these the sweep is done: every graphics and video row the observer takes now has a seat, bar the three that are not settings — `frillDensity` (World Detail already drives it, and two dials on one knob is worse than none), `nearclip` (the projection near plane: changing it is a bug, not a preference) and `interiorDebug` (a visualiser). Seven shape the room — the light budget, the ambient floor under every indoor surface, the indoor daylight floor, the bounce gain, the reach past each fixture's authored window, the share of painted-in lighting kept where nothing reaches, and the room gate — and three are the indoor shadow budgets. Named for what they do to a room, not for the quantity they scale, because the quantity is the renderer's word and the room is the player's: "Unlit Room Floor", not "interior bake floor"; "Lights Stay in Their Room", not "room gate". Each description says what the setting is FOR rather than what it multiplies, and three of them carry the thing a player would otherwise misread: - `interiorAttenScale` at 0 is not darkness, it is the authored window switched OFF, so every fixture reaches the whole room. A left end that looks like "no light" and means "all light" needs saying. - `interiorRoomGate` is the one setting whose job is diagnosis: a room the gate leaves on ambient alone looks exactly like a bug, and one click tells the two apart. That belongs in the options window and not only in `/console`. - `interiorShadowEntityRate` is the only row in the indoor shadow block that actually costs frame time — neither count dial above it moved it, which is why it exists as its own row — so its description says to lower it first. Two grooves deliberately offer less than their CVar's clamp, as `RowRenderScale` already does (50-200% inside a 0.25-4 clamp): ambient and indoor daylight run 0..0.1 of a 0..1 clamp, because the shipped value is 0.015 and past about 0.1 a room is not lit, it is fogged. `/console` still reaches the rest. One offers less than its *registration* says: `interiorShadowDynamic`'s help line reads 0..16, but the observer raises 0 to 1 and clamps to `MAX_TORCH_DYNAMIC` (8), so a groove to 16 would be seven stops that all mean 8 and a zero that means 1. The gating is two-level: every row needs the live-fixture lane, and the three shadow budgets need the shadows on top of it. A budget row left live while its shadows are off is a dial that does nothing, which is how settings earn the reputation these ones had. Russian for all ten rows and their descriptions is written to match the register of the column already there; a native reader would still improve it. Verified: ALL GATES GREEN (346 s); `benilla-app --lib` against the install at the exact 269-failure baseline, set-identical by `comm`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
|
2d1dbd5639 |
Seven more graphics settings get a seat: the filter pair, MSAA, the shadow rig
The sweep behind this: every CVar `video::on_cvar` answers, against every CVar the Options window names. Twenty of fifty-five had no seat at all, so the only way to them was `/console` — which is not a setting. Seven of them here; the ten interior-lighting rows follow. **Graphics page.** Trilinear Filtering under Anisotropic, because they are one policy: aniso at 2 or more writes filter mode 5 and trilinear writes mode 4 only while that bit is clear (`0x688d0d`), so the row is GREYED while anisotropy is on — not hidden, and not rewritten. It keeps the value a player set, and turning anisotropy off hands it straight back. The gating reads the STAGED anisotropy where there is one, since both rows defer: otherwise a player who moves one and has not pressed Apply sees the row it is about to gate stay live for the rest of the visit. Multisampling beside Vertical Sync and Resolution, the latched `gx*` rows it commits with. 1.12 had this row and we did not. Backed by the index verbs its own dropdown used (`0x48c580` / `0x48c640`) rather than by `gxMultisample`, because picking a format writes the colour and depth rows with it as one record — and it rebuilds its list on open, like Resolution, since at OnLoad the engine has published no formats yet. Only the sample count is named: the colour and depth halves of each triple describe our swapchain and steer nothing, so offering them would offer a choice that does not exist. Its description says plainly that multisampling does less here than in a modern game, because it misses the alpha-tested cutouts most of this game's foliage and fencing is made of. **Advanced Graphics.** The shadow rig's four unreachable rows — Soft Shadow Edges (the 9-tap gaussian against the card's own 2x2; Bevy's third mode is deliberately not offered, since it only resolves under TAA this camera does not run), Shadow Caster Range, and the two update-rate caps — plus Torch Shadow Darkness beside the softness dial it shares a shadow with. The rate caps needed their own readout. `0` there is **every frame**, the opposite of what it looks like, so a bare "0 per second" would read as the shadows being switched off — which is what the two toggles above them are for. They now say "Every frame", in both locales. Gating is per-lane where the lane is what matters: each rate cap is dead only while its own lane is, while the filter and the reach ride the cascade and need just one sun lane left. Russian for all five rows and their descriptions is written to match the register of the column already there; it would still benefit from a native reader. One thing worth recording for the next row: `OPTIONS_PAGE_ROWS` has positional contracts in its tests, and the list's own comments say "appended so the other rows keep their indices". It drives refresh, search and Restore Defaults — never layout, which is the frames' own anchors — so appending costs nothing and seats each row where it belongs. Verified: ALL GATES GREEN (322 s); `benilla-app --lib` against the install at the exact 269-failure baseline, set-identical by `comm`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
|
1c786b7ecb |
Anisotropic Filtering takes its seat on the Graphics page
The CVar has been registered and read since the filter policy landed, and last commit put it on the Graphics Preset ladder — but a player had no way to reach it. The reference's own `OptionsFrameSliders` declares the row at index 7; ours left that index a hole, so the only route to anisotropy was `/console anisotropic 16`, and a player whose preset reads Custom (the seeder never overwrites one) never met the setting at all. Seated where the reference's own slider order puts it: after Brightness, before Weather Intensity. Three things it does differently, each stated beside it: - The groove runs 1 to 16, the CVar's own `ANISO_RANGE`, not the stock table's 1 to 4. That ceiling was the reference *slider's*, not the CVar's — its callback parses 1-16 (`0x689110`) — and both the High and the Ultra rung ask for 16. So the row carries literal bounds and stays out of `BENILLA_GRAPHICS_SLIDER_ROWS`, exactly as `RowRenderScale` does for a CVar the stock table never had, which leaves `OptionsFrameSliders` verbatim for any addon that reads it. - `1` reads out as "Off", not "1x": the reference writes filter mode 5 only at 2 and above, so 1 is the feature off and not its weakest stop. - The description is ours. 1.12's own OPTION_TOOLTIP_ANISOTROPIC describes a four-stop slider, and this one is sixteen. Deferred to Apply, as Vertical Sync and Render Scale are: a sampler is baked into each `Image` at load, so the write cannot reach a texture that is already resident. The tooltip says so in the row rather than leaving a player to wonder why the ground did not change — the same thing the reference's UI means by "enabled upon restart". Verified: `benilla-app --lib` at the exact 269-failure data-gated baseline, set-identical by `comm`, with the new row's own test and the two census counts (tooltip keys, raised plates) moved 116 -> 117 as the row asks. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
|
89ae4699bb |
Re-seat two picks the #1730 revert and the realm's interface displaced
Two follow-ups to the series, both consequences of the same root cause. **#2150's OptionsFrame.xml hunk, re-ported.** It cherry-picked with no conflict only because the botched #1730 resolution had already replaced this fork's `OptionsFrame.xml` with upstream's, so it applied against upstream's file. The revert then restored ours and took it with it. Ported by hand here: our rows write `self.applyFunc` where upstream's write `this.applyFunc`, and inherit `OptionsCheckboxRowTemplate`/`OptionsDropdownRowTemplate` where upstream's inherit the `Benilla`-prefixed names. Both `TargetofTarget_Update` applyFuncs are gone and the reference note comes along. **#1972's blanket dialog test, scoped to our two dialogs.** The fix — this page's two confirms declaring `timeout`, `whileDead`, `showAlert` — is right and stands. Its test asserted a numeric `timeout` on *every* entry in `StaticPopupDialogs`, which holds for stock 1.12.1 FrameXML but not here: this fork loads the realm's own interface, whose `StaticPopup.lua` leaves `timeout` off 73 of its dialogs. Those are the realm's to declare, and failing on them says nothing about this fix. benilla-app --lib is back to the 269-failure baseline exactly. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
|
73f4bf808d |
The target-of-target rows only write their globals, as 1.12's options do (#2150)
1.12's options declare Show Target of Target as a bare uvar (UIOptionsFrame.lua:65) and the mode is read on save (UIOptionsFrame.lua:365); neither calls TargetofTarget_Update. The frame follows from TargetFrame_OnUpdate and TargetofTarget_OnUpdate on the next frame (TargetFrame.lua:492-501). Our rows called it as their applyFunc, with the row as `this`; the function refreshes `this`'s debuffs (RefreshBuffs(this, 0, "targettarget"), TargetFrame.lua:538), so with a target's target up the switch raised "BuffFrame.lua:281: attempt to index local 'debuffBorder' (a nil value)" (seen on a Linux client). Verified: the_target_of_target_switch_leaves_the_frame_to_its_own_updates (with a target's target up: no error, the frame shows after one tick on and hides after one tick off) fails with the applyFunc put back, with exactly that error, and passes without it; the options and tot_frame tests pass on a deDE install; cargo test -p benilla-app --lib 5145, clippy -D warnings, fmt. Built on macOS only. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit 4d6f9ecc8a0d99e6d3ecf7fa4a8da229ebad92c2) |
||
|
|
8f658cb516 |
Key-binding confirm dialogs no longer raise a Lua error each frame (#1972)
Unticking Character Specific Key Bindings, or pressing Defaults, raised `StaticPopup.lua:1713: attempt to compare number with nil` every frame the confirm stayed up. 1.12.1: `StaticPopup_Show` copies an entry's `timeout` into `dialog.timeleft`, and `StaticPopup_OnUpdate` compares it with 0 each frame; every stock entry declares one, the stock key-binding confirm included (`Blizzard_BindingUI.lua:10-24`, `timeout = 0`, `whileDead = 1`, `showAlert = 1`). Our key-bindings page's two confirms declared none. Both now do, the character-set one with the stock entry's three fields. The checkbox test now runs a frame with the confirm up, and a new test fails on any dialog the loaded interface defines without a numeric timeout; both fail without the fix. Fixes #1971 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit 881152bda6d48cc3fd1a47679a7465add212ae05) |
||
|
|
0c247396b7 |
The ESC menu's Support rung opens the help window (#1807)
The ESC menu's Support rung was greyed with nothing behind it. The 1.15 era menu that the layer's ladder follows opens the help window from Support. In 1.12 that window is the stock `HelpFrame`, opened by the "?" micro button's `ToggleHelpFrame()` (`MainMenuBarMicroButtons.xml:217`). Support is now live: it plays the menu click, closes the menu as the Macros rung does, and toggles the help window. Edit Mode stays greyed. Test: `support_closes_the_menu_and_toggles_the_help_window` clicks the rung and checks the toggle and the closed menu; the live-entries test now lists Support. Fixes #1800 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit bb434a181f6a3815ec6fe3e5ace89c864fa1808c) Absorbed into the octo fork. Ported by hand: upstream wires Support in GameMenuAdapters.xml, a Lua adapter layered over stock FrameXML that this fork does not carry — our menu declares its own rungs in GameMenuFrame.xml, where GameMenuButtonSupport was GameMenuButton_Pending (SetText + Disable). It now sets its text on load and, on click, plays igMainMenuOption, hides the menu as the Macros rung does and calls ToggleHelpFrame(). Our rungs carry no Benilla prefix, so upstream's test names are retargeted; our own narrowing_the_frame_keeps_the_button_column_centred test is kept. The 6 FloatingChatFrame failures in this module are pre-existing baseline (10 passed/6 failed before, 11/6 after). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
|
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
|
||
|
|
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 |
||
|
|
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
|
||
|
|
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 |
||
|
|
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 |
||
|
|
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 |
||
|
|
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
|
||
|
|
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 |
||
|
|
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 |
||
|
|
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
|
||
|
|
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 |
||
|
|
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 |
||
|
|
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 |
||
|
|
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
|
||
|
|
dc59594389 |
app+protocol: nampower's ten cast events, off the queue that already had them
All ten ride `ui_cast_events.rs`'s existing `CastEventFeed` rows — the same rows `UNIT_CASTEVENT` and the cast windows are folded from — with a new `NpCastColumns` carrying the fields nampower's shapes want and SuperWoW's one word has no place for. One row per packet, folded once, which is this file's own rule: an addon woken by one of these and then asking a getter must not be able to get a contradicting answer, and a parallel queue over the same packets is exactly how that happens. `NP_EnableSpellStartEvents` and `NP_EnableSpellGoEvents` go from stored-only to LIVE. Their rows said the shapes wanted "itemId/spellType/corpseOwnerGuid columns this client does not carry"; two of those three turned out to be a Spell.dbc lookup and a guid hop the item-cooldown path already made. `itemId` is the cast item's entry through `item_entry` (now shared with `item_cooldown`, which had the same two hops inline) — `SessionEvent::SpellStart` was dropping the guid the GO always carried, so it carries it now. `spellType` is `AttributesEx & 0x44` for a channel and the auto-repeat bit for a shot. `channel_ms` is `SpellDuration.dbc`. The GO's two counts are the packet's own lists, capped at 255 because the server writes both as `u8`s. `SPELL_FAILED_SELF`'s result byte is the failing `SMSG_CAST_RESULT`'s reason — the number 1.12 turns into a red line and then discards, so no addon on that surface can see why a cast failed. `corpseOwnerGuid` is the one column that really is absent, and it is not sent at all: a trailing nil in Lua is indistinguishable from an absent return, and upstream sends nil for every cast without a corpse target. For the handful that have one the gap is bounded and written down — the targets block reads the corpse guid and drops it, and the owner is a further hop through that corpse's own CORPSE_FIELD_OWNER. Two subtleties, both pinned by tests: - A channel's START is queued although its wire cast time is 0, and `UNIT_CASTEVENT`'s START is still suppressed for it. Without the row, `SPELL_START_SELF` would never fire for Mind Flay or Blizzard — pfUI's reader expects exactly that row and falls back to arg7 "only for true channels like Blizzard where arg6 is 0/nil". With the word, every cast-tracking addon on this realm would get a SuperWoW START whose arg5 is 0, which is a cast bar told to run for no time. So the suppression moved from the producer to the fire, and SuperWoW's word behaves exactly as it did before the row existed. - The `_SELF`/`_OTHER` split is by caster here and by *target* for the aura family next door. Neither is a guess: upstream's wording for the aura pair is "casts that land on the active player", and for this one pfUI handles both names in a single arm and then reads arg3 to learn the caster, which only makes sense if the name already settled it. A gate hole closed on the way past: `AURA_CAST_ON_SELF`/`_OTHER` were fired through one `&str` variable, and `reference_ui.rs`'s beyond-1.12 event gate reads the string literal *at the call* — so it had been blind to that pair for as long as they had existed. Both are literals now, with their rows, and the probe confirms it: renaming either row makes the gate name the event. Tests read pfUI's own `local` bindings back out rather than a list of values in push order, as the aura-cast block beside them does. A positional shape got backwards here fails in silence — the handler binds arg3 to casterGuid whatever arg3 happens to be — and this family is where that would be most expensive, since pfUI's swing timer resets off the START and its whole debuff bookkeeping gates on the GO's two counts. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
|
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
|
||
|
|
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 |
||
|
|
cae59cdc11 |
app+protocol: fire CHAT_MSG_HARDCORE, the realm's own chat type
`reference_ui`'s `UNPRODUCED` carried `CHAT_MSG_HARDCORE` as a chain event nothing here fires: the window registered for it and benilla routed no such message, so the realm's hardcore-death broadcasts simply never arrived. Three of grom's addons listen for it too. The missing piece was one number, and the realm's patched `WoW.exe` has it in the PE section it names `.tdata`, beside the string `"HARDCORE"` at `0xd06000`. Two detours do the whole job: `0x49f7aa`, in the chat type-name lookup, jumps to `0xd06020`, which sets the name pointer to that string and the type to `$0x5e`; and `0x49b193`, inside the `CHAT_MSG_*` fire helper `0x49b0b0` whose format string this file already cites, jumps to `0xd06040`, which tests the type against `$0x5e` and swaps in the event-name index `0x21f`. So wire type `0x5E` is `ChatEventKind::Hardcore`. Nothing in 1.12 claims it — its last type is `CHAT_MSG_BATTLEGROUND_LEADER` at `0x5D` and its `CHAT_MSG_BG_SYSTEM_*` block sits at `0x52`-`0x54` — so there is no collision to arbitrate, and the test pins that adjacency along with the decode, since a wrong id would look exactly like the silence it replaces. The chain's `ChatFrame.xml` registers the event and its `GlobalStrings.lua` has the `[Hardcore] %s: ` format, so the window needed nothing from us but the fire; it takes the window's default colour, as `FILTERED` does, the realm adding no `ChatTypeInfo` row. Not covered: sending. The ruRU locale carries `SLASH_HARDCORE1 = "/h"` and `CHAT_HARDCORE_SEND` where enUS has only the receive format, so the send side needs its own look. 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 |
||
|
|
02e6fc4208 |
formats+app: a stale snapshot archive mounts below the realm's patches
grom is keeping `patch-A.mpq` — it is almost certainly textures — so the shadowing it causes is handled in the chain instead of on his disk. `mount_order` stays the reference's law, letter above digit and all, and `demote_stale_snapshots` runs after it: an archive whose `FrameXML.toc` is a **strict subset** of another's is an earlier edition of the realm's manifest, so it mounts just below the patch set — above `patch.MPQ` and the base archives, so anything only it carries still resolves. A toc that differs by *removal* is a different manifest rather than an older one and is left exactly where its name puts it. `WOW_CHAIN_DEMOTE=0` keeps the reference's order for comparison. On his install `patch-A.mpq` (Aug 7) now loses to `patch-5.mpq`, so the realm's current data is what loads: its `FrameXML.toc`, its `GlobalStrings.lua` and a `Spell.dbc` with **28019** rows against the 27992 his last session reported — 27 spells the update added that were invisible. Reading the realm's real files then broke ten tests, all of which had pinned August's behaviour, and each is now pinned the realm's way: - `AIOHandlers.lua` claimed the `OctoRadio` block as a no-op on the evidence that the radio file carried no AIO call. The update built the radio out into a file twice the size whose `RegisterNowPlayingFeed` registers that name at `VARIABLES_LOADED`, and `AIO.RegisterEvent` asserts on a duplicate, so our shim raised through seven tests. The row is gone, and the shim now claims at `PLAYER_LOGIN` through `pcall` so a realm that implements one of these takes the name and we step aside. `no_aio_block_we_shim_is_one_the_realm_now_handles` fails a row the realm has implemented — verified by putting `OctoRadio` back. - `BATTLEFIELDS_CLOSED` no longer hides the battlefield list, by the realm's decision: the server stamps `SMSG_BATTLEFIELD_LIST`'s battlemaster guid with the player's own, so the client anchors the window to the spot it opened and closes it when you step away, on foot or on a boat (bug tracker #330 and its follow-up). Pinned that way, with the close button proving the list still closes. - `MAX_QUESTLOG_QUESTS` is 25, not 1.12's 20. The test reads the realm's global now, so the next bump is data and not a failure. - The realm's `OctoQuestLogPoller` redraws the log every 0.2 s, which legitimately costs a layout gate walk. `POLLS_ON_A_TIMER` stands it down by name before the quiet-frame bench measures, rather than raising the threshold and blunting the one instrument that catches an every-frame writer. A timed realm handler that is not listed fails. 269 data-gated failures, identical to the baseline at HEAD name for name, and all nine gates green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
|
73bc9a9e8e |
app+formats: name the realm files a shadowing archive holds stale
The archive found holding an old copy of the realm's own `FrameXML.toc` is holding an old snapshot of the realm's interface, not one old file. `Chain::copies` answers which archives hold a name and at what size off the hash and block tables alone, decompressing nothing, so the boot log can now name every row of the realm's toc whose winning copy comes from that archive while the realm's own has a different one. On grom's install that is ten, and it is the better half of the finding: `GlobalStrings.lua`, five `OctoLocale` files, `AIO.lua` — the client and server's whole transport — the calendar, the challenge tips and the radio all load as an August `patch-A.mpq` has them, because a letter patch outranks the realm's `patch-5.mpq`. `Spell.dbc` is shadowed the same way, 20 KB behind. Reported and not repaired, deliberately: shadowing a file with different content is what a patch archive is *for*, so outside the toc — where "holds every row the winner does, plus more" is a decidable relation — this client cannot tell a stale copy from an intended override. The real 1.12 client reads the same copies, so this is a fix on disk, and the line is what makes it findable instead of invisible. `richest_toc` splits the superset rule out of `realm_toc_rows` so it is testable without client data, which is where it ships from: the winner stands unless a shadowed copy drops nothing, a longer copy that drops a row is passed over, and the caller is told which archive it read so it can say so. The staleness report is checked against the real chain and every row it names must really differ in size between the two archives, so it cannot be noise. The 269 data-gated failures remain identical to the baseline at HEAD, name for name. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
|
eaadc9f73b |
app+formats: take the realm's own FrameXML list, not a curated copy of it
`benilla.toc` enumerated every interface file, so a row the realm adds in a client update was *silently* absent: no "not found", no warning, just a global nobody defined. That is what `/xpboost` was, and the only way to find it was to diff two MPQ archives by hand. The manifest now decides order and substitution, and the realm's own `FrameXML.toc` decides membership: every row it carries that the manifest does not name loads after it, in the chain's order, and the boot log names them. `DECLINED` is the short list of rows this client must not run (`StatsFrame.xml`, whose OnUpdate is the only reader of a `GetDebugStats` we do not have), each with its reason; a row we merely have no verb for still loads and raises where it is used, which beats silence. `WOW_FRAMEXML_TAIL=0` opts out and says what it is leaving out. Finding that out turned up the reason the row was missing in the first place, which is worse and is now fixed too. `Chain::open` mounts `patch-?.MPQ` in the reference's order — case-folded, last wins — so `patch-A.mpq` mounts **above** `patch-5.mpq`. The realm ships its interface in `patch-5`; this update added six rows to that toc; and the 140-row copy inside an older `patch-A.mpq` kept winning. The interface was loading August's list of files. So `Chain::read_all` returns every archive's copy of a name in ascending priority, and `realm_toc_rows` reads a shadowed copy when it holds every row the winner does *plus* more. That case is unambiguous — nothing was removed, so nothing can be lost — and it is a packaging accident with no intent behind it either way. A copy that disagrees by removal is a real change of mind we cannot arbitrate: the winner stands and the log says what was passed over. Both paths say which archive, so the clean fix on disk is one line of log away. Verified against grom's own install and his addon corpus: with the `OctoXpBoost` row deleted from the manifest — tomorrow's update, simulated — the tail finds it, resolves it to a readable chain path, and `/xpboost` is a live slash command with a function behind it. The tail is empty with the manifest as it stands, which is the point: the mechanism is armed and changes nothing today. Tests: the merge takes both lists as arguments so it runs where there is no client data, since that is the machine where a regression in it would otherwise skip unseen; the letter-above-digit mount order is pinned on its own, because that fact is what cost the six rows; `read_all`'s ordering contract is checked against a real chain; and a `DECLINED` row that the realm stops shipping now fails as stale. The 269 data-gated failures are the same 269, name for name, as at HEAD. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
|
ef7f823d9a |
ui: teach the producer gates the reach-reporting fire
`every_event_a_chain_file_registers_has_a_producer` finds producers by scanning our own `.rs` for `fire_event(`, `fire_event_into(` and `pending_events.push((` followed by a string literal. `49b4f7b5` moved `area.rs`'s minimap fire to `fire_event_reach(`, which matches none of them, so the gate began reporting that `Minimap.xml` listens for `MINIMAP_ZONE_CHANGED` and nothing fires it. Both scanners here now match that spelling too. The break rode green through a full gate run and a commit because this test and its sibling need `WOW_DATA`, which `scripts/gates.sh` does not set — on a machine without the client data 1609 tests skip silently. Found by running `WOW_DATA=<Data> cargo test -p benilla-app --lib` and diffing the failures against a baseline at HEAD: 3 before, 2 after, and the two that remain fail at HEAD as well. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
|
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
|
||
|
|
cd6731799b |
ui: memoise the texture-existence probe, as its two neighbours already are
`set_texture_probe`'s closure answers every path form of `SetTexture`, and it was the only one of the three probes installed beside it without a cache — `set_texture_size_probe` and `set_font_probe` both memoise, and the font one's comment already gives the reason that applies here verbatim: a path's answer cannot change, and an addon may ask every frame. A miss is the expensive answer. An addon texture is never in the MPQ chain, so both candidates fall through to `loose_addon_file`, which walks the path one component at a time and, whenever a component's real spelling is not the lower-cased one it was handed, enumerates that whole directory to compare case-insensitively. `Interface\AddOns` holds 94 folders here and `pfquest` never matches `pfQuest` on a case-sensitive filesystem, so an uncached call read that directory twice over, on the main thread, between two systems. grom's 2026-10-21 17:08 session priced it: `[ui-verbs]` reports `texture:SetTexture` at **45-59 us a call across all 33 windows** — stable enough to be a fixed charge rather than a tail — against the ~0.2 us a cached existence answer costs. 1.75 ms a frame over 34 calls while the world map was up, because `WorldMapFrame_Update` re-textures every overlay tile and POI on each `WORLD_MAP_UPDATE`. Replaying the same walk over the real AddOns tree reproduces 26-42 us for a single candidate. Misses are cached too, as the size probe's are: an addon probing for optional art is the common case and it is the one that pays for the walk. This is the smaller half of the map's cost — `scrollframe.rs`' derivations were the larger — and it is a ~1-2% of frame fix, not the dip. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
|
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 |
||
|
|
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 |
||
|
|
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 |
||
|
|
e4851df7df |
ui: WOW_UI_PHASES — the UI pass's phase split, windowed
`WOW_UI_TICK` measures `UiScript::tick`. `[sys-timing]` measures the Bevy system around it, `ui_script::extract::tick_script`, which also runs the FontString measure round-trip and `UiScript::resolve`. In a combat session the first read 38 ms/s — its lowest of the whole run — while the second read 157 ms/s. Neither instrument was wrong, and 76% of the cost was in the space between them, which is where the stutter players report lives. `UiFrameCost` has carried the whole split since the hover recorder needed it, and `WOW_UI_COST=1` prints it one line per frame: at this client's framerate, twenty thousand synchronous stderr writes a minute from the thread being measured, which is the trap `sys_timing`'s header records costing 15 ms a frame. The split existed and was unusable in exactly the situation it was for. So a window, and a worst column — a phase that is ruinous on one frame in a hundred has a mean near nothing and is the only thing a player feels. `derives` is in the per-frame line because `UiFrameCost::derives` is documented "expected zero", so a nonzero one names a whole-graph re-derivation on sight. `UiFrameCost::stamped` comes with it: the paint can return before any of its three arms wrote this frame's phases, and a reader that cannot tell counts the previous frame twice. Armed in ./measure.sh at 5 s, with the per-frame form left alone for the hover recorder. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
|
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 |
||
|
|
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 |
||
|
|
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 |