1608 Commits

This Branch

This Branch

All Branches

Author	SHA1	Message	Date
gromandClaude Opus 5	fcdf14c0dc	app: a held press cannot time out while something still guards it ... The queue stamped each entry's deadline when the press was made. That reads right and has a hole in it: a cast pushed back twice (`SMSG_SPELL_DELAYED`) ends after the window a press made at its start was given, so the entry times out while the thing it is waiting for is still running — and the press vanishes with no packet, no error line and no cast. That is the one failure mode a player cannot describe, and it is exactly the class of bug the press queue was supposed to remove rather than introduce. The same hole swallows a press held through a cast whose global cooldown then outlasts the window, which is the ordinary case rather than an edge one: a 500 ms window, a cast that ends, and 1.5 s of global cooldown behind it. So a slot now keeps the window and the base it is measured from, and the drain ratchets that base forward — only ever forward — past every live guard: the cast's reopening, the channel's trimmed end, and the global cooldown's. An entry gives up when nothing is holding it back any more and the window has run out since, which is what "how long past the guard's end" meant all along. Scripts keep a fixed deadline. Their contract is "after queued spells", and a script whose spells never fired has nothing left to run after. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn	2026-10-25 00:30:46 -06:00
gromandClaude Opus 5	39932b12da	app: nampower's channel latency trim — the last default-on queue row ... `NP_ChannelLatencyReductionPercentage` ships 75, not 0, so a real install really does take three quarters of the round trip off a channel's end. Without it a press held through a channel is sent a round trip late, which is the cost the whole press queue exists to remove — leaving this stored would have left the queue's worst case exactly where it started. **Why sending early here is conservative and not reckless**, which is the part worth writing down. The server starts the channel at T and ends it at T + duration; we only learn of it half a trip later, so our own end sits at T + L/2 + duration — half a trip *after* the server is already free. Trimming 75% of L puts our end at T + duration − L/4, and a packet sent then reaches the server at T + duration + L/4: still after its own end, by a quarter of a trip. Same arithmetic as the cast buffer, and the round trip it removes was never real work. It reads the channel's **named** end, not the slack-inflated one. That slack is two seconds of belt-and-braces for a lost `MSG_CHANNEL_UPDATE(0)`, and reading it here would make every channel look longer than it is — the pop gate was doing exactly that, so the press would have fired a round trip and then some after the channel finished. With no pong measured yet nothing is trimmed, which is the safe direction to be wrong in. The lock is recovered rather than unwrapped, as every reader of it is, and that matters more here than at the latency meter: the cast path reads it now, and a poisoned lock must not take the ability to cast with it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn	2026-10-25 00:03:10 -06:00
gromandClaude Opus 5	a9bb9624e9	app+ui: nampower's press-interception queue — the half that changes how keys feel ... The half of upstream's queue that `queue.rs` has said for three rounds it deliberately left out. A press the client would refuse for a **timing** reason is now held in the same slot an explicit `QueueSpellByName` uses, and the drain sends it the moment the guard clears: no packet, no global cooldown, no error line while it waits. One queue, because upstream has one — a second set of slots beside the first would be a different feature wearing the same name. **The guard reopens early, and that is the headline.** `PendingCast` kept one timestamp and now keeps two. The guard reopens at the cast's own already-known length plus `NP_MinBufferTimeMs`; the record still lives for the cast plus two seconds, so a resolution arriving a round trip later is still recognised as the answer to that cast. That split is not tidiness — the `modalNextSpell` chain that starts a hunter's Auto Shot reads the record, and a record that died with the guard would break shooting. The buffer is not a latency figure, which is why upstream's default is 55 ms and not a ping: our packet reaches the server half a trip after the send, the server finishes its cast half a trip after our own cast time elapses, so a next cast sent at exactly `cast_time` arrives just as the server frees up and the latency cancels out of the arithmetic. The buffer is jitter margin on top. `NP_NonGcdBufferTimeMs` is wider because a non-GCD spell has nothing *but* the guard between two presses. **`NP_RetryServerRejectedSpells` is the mitigation for that**, and that is why upstream defaults it on. If the margin is ever wrong the server refuses the next cast and the press is simply gone — the dropped cast the early guard buys its speed with. It goes back once, held a buffer's worth, on exactly two codes: 0x3c not-ready and 0x61 another-action. Every other refusal is the server telling the player something true, and re-sending one would be a packet a second that never succeeds. **What a window measures, and how that was decided.** Upstream publishes four and no sentence saying. Two readings fit: how long an entry survives, or how close to the guard's end a press must land. This takes the first, because `queue.rs` already reads `NP_SpellQueueWindowMs` that way and one number meaning two things in one client is a trap nobody finds twice; because `Replacing queued nonGcd spell X with Y` means the last press wins, so holding every press costs nothing in staleness; and because the other reading would make the 250 ms cooldown window refuse almost every press made during a global cooldown, which is the complaint nampower exists to answer. Two rows do read a window as remaining time, and both say so themselves. `NP_InterruptChannelsOutsideQueueWindow`'s name states one. And `NP_CooldownQueueWindowMs` has to be one: a cast ends in a second, a Hearthstone cooldown is half an hour, and holding that press means either firing it half an hour later or swallowing it with no error line. A quarter of a second is an allowance for pressing a touch early, not a queue. **Where the rung sits.** Five rungs stay above it — the profession opener, the auto-repeat re-press, the dead caster, the targeting cursor's clear and the on-swing re-press — because each is what the press *means* whatever is in flight. Everything below is a real refusal a player should read: no reagent, out of range, no power, stunned, moving, wrong form. Upstream hooks the cast entry and holds those too, so its error arrives a second later about something the player stopped doing. Refusing them now is the deviation and the better half of the trade. Fourteen CVar rows went live. What stays stored is named with its cost: the dynamic buffer optimizer and its ceiling (off in upstream's own defaults), `NP_OnSwingBufferCooldownMs` (nothing to rate-limit — a re-press of the queued strike already takes the reference's silent bail), the channel latency trim, and the three quickcast rows, all off by default and all needing the world point under the mouse in the cast path. `CastSpellNoQueue`/`CastSpellByNameNoQueue` became meaningful: they take `CastLadder::send_now`, which skips the rung. The queue's own pop takes it too — a pop that could be re-held is a loop that never casts anything. The test worlds now seed the **registered** defaults rather than `Default`'s all-zeroes, which is a configuration nobody runs: three ladder tests were passing only because the queue was off in them. Each now asserts the queue behaviour and keeps the reference's 0x61 in a world where a player unticked the queue. And one test drives the whole cycle through the real drain — press held, nothing on the wire, guard cleared, one packet out — because the failure this guards against is a keypress that produced no packet, no error and no cast, which is the one a player cannot describe. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn	2026-10-24 23:46:42 -06:00
gromandClaude Opus 5	94c571e3ea	app: nampower's mount block at the buff cap — the first live press refusal ... `NP_PreventMountingWhenBuffCapped` was the one gap in the nampower table named as a gap rather than a shrug: upstream defaults it ON, so a real install refuses a mount cast with all 32 positive aura slots taken instead of sending one the server accepts and then drops on the floor. With the slots full the mount aura has nowhere to land; vmangos runs the cooldown and the global cooldown and applies nothing, so the player stands still having spent a press, with no error to read because from the server's side nothing failed. The rule is `spell/np_press.rs`, a new module for nampower's press side — the rules that decide what a keypress does when the client would refuse it. It is separate from `queue.rs` because its inputs are not the queue's, and it is where the press-interception half will land. Three things worth naming: **The aura number is measured, not quoted.** `SPELL_AURA_MOUNTED = 78` is asserted against this realm's own `Spell.dbc` for Brown Horse, Summon Warhorse and Summon Felsteed, plus the converse on Frostbolt. A wrong constant here makes the refusal silently never fire, which no amount of play distinguishes from the row being off. **One deviation, and upstream names the bug itself.** Its settings label reads "Preventing mounting due to buff cap (breaks dismount)" — in 1.12 you dismount by casting the mount spell again, so a blanket block strands a buff-capped player on their mount. A press while `UNIT_FIELD_MOUNTDISPLAYID` is live is exempt here. One descriptor read, and it removes the only way this rule can make things worse. **Refused silently**, as upstream is: its own report is a line in its debug log, and 1.12's `SpellCastResult` has no code for "too many auras", so the only error line available would be a sentence about something else. What the player gets instead is the GCD and the packet both unspent, which is the whole point of refusing above the commit rather than below it. The ladder-level test asserts the fixture really reaches the cap before it asserts anything about the refusal — the id dwords alone leave every slot reading empty without the `AURAFLAGS` liveness nibble, and a fixture that never filled the slots would have passed for the wrong reason. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn	2026-10-24 22:45:57 -06:00
gromandClaude Opus 5	152788d894	app: seal a password only if it reads back, and keep tests out of the store ... This cost grom his stored password once. Two unit tests ran at the same time: one redirected the config directory through `BENILLA_HOME`, which is process-wide, while the other stood up `LoginPlugin`, whose `build` had just gained a call to `migrate_plaintext`. So his real `autologin.toml` was upgraded against a key file that was not the one beside it — the plaintext was cleared and the blob replacing it does not decrypt. He types the password once more and `remember` stores it properly, since a blob that answers empty does not match and is resealed. Two fixes, and the second is the one that matters. The migration is now `#[cfg(not(test))]`: it was the only line in that plugin's build that writes to the player's own config directory, and no test has any business there. `autologin::sealed` encrypts and then **reads the blob back**, and refuses to clear the plaintext unless the round trip holds. That would have prevented this on its own, and it closes the general hole rather than this instance of it: `credential.rs` already carries a note about a scheme that round-trips with itself while being wrong, and this is the same mistake one level up — the cipher was right, the *key* was what disagreed, and the only readable copy of the password was thrown away before anything checked. A test drives the keep-it branch through the hermetic capture flag, the one state where sealing honestly cannot work, and asserts the plaintext is still there afterwards. An ignored `live_check` test reads the real store by hand and prints lengths only, never the password; it is what caught this. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn	2026-10-24 17:33:26 -06:00
gromandClaude Opus 5	fc4eb2c827	app: the rest of nampower's CVar table — 49/49, five of them live ... `strings nampower.dll | grep -oE '\bNP_[A-Za-z]+\b'` is the DLL's whole CVar table and `comm -23` against `cvars.rs` now prints nothing. Fifteen names were missing, for the same reason the function list started wrong: the first pass took them off `NampowerSettings`' writes rather than off the binary. Live: `NP_ChatBubbleDistance` (the same `BubbleConfig::range_sq` SuperWoW's `ChatBubbleRange` writes — both DLLs hook one cull, so last write wins there too), the three `NP_ChatBubbles*` kind toggles, and `NP_EnableLocalSetRaidTarget`, which routes a solo `SetRaidTarget` to the local board instead of a packet the server has nobody to echo to. Two deliberate deviations, both about what a player who never touched a setting gets: `NP_ChatBubbleDistance` ships 20 yd, the reference's own cull, not upstream's 60. Tripling the distance bubbles appear at is a visible change to the game nobody asked for; writing 60 gets 60. The six `NP_EnableUnitEvents*` rows keep a default of 0, where upstream's README says 1 and its own `EVENTS.md` says 0. The disagreement costs nothing under the registration rule the event family uses: a family is wanted when its CVar is set or something registered the event. Nine stored-only rows each say what building them would take. One is a real gap rather than a shrug: `NP_PreventMountingWhenBuffCapped` defaults ON upstream, so a real install refuses a mount cast at 32 buffs instead of letting the server eat it. Everything it needs is here; the test belongs inside `send_spell_cast`, which already takes 22 parameters and none of them is the player's own descriptor — and it refuses a press, so it wants a live test rather than being bolted on at the end of a round. `KEY_DOWN`/`KEY_UP` stay unbuilt, which leaves nampower's event table at 57 of 59 with both absences explained. They need the client's numeric `KEY_*` enum, a table in `WoW.exe` this round did not read: the DLL publishes four of its values, enough to see the shape (ASCII in bank 0, specials at 0x200, function keys at 0x300) and not enough to name a key in bank 2 past the first. A guessed code is worse than no event, since an addon switching on arg1 would silently never match — and they are the keyboard hook the press-interception queue uses, so they belong with that decision. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn	2026-10-24 17:19:35 -06:00
gromandClaude Opus 5	68f397af72	app+protocol: nampower's combat-log family — 35 events off one decode ... Every packet this family reports was already decoded: `combat_log/` builds its chat lines from exactly these structs. `ui_np_events.rs` is a second consumer of that decode, not a second decoder, which is also why its arguments cannot drift from the chat log's. Built: `SPELL_DAMAGE_EVENT_*`, `AUTO_ATTACK_*`, the three `SPELL_HEAL_*`, the three `SPELL_ENERGIZE_*`, `SPELL_MISS_*`, `DAMAGE_SHIELD_*`, `ENVIRONMENTAL_DMG_*`, `SPELL_DISPEL_BY_*`, `UNIT_DIED`, the `*_UPDATE_DURATION_SELF` pair, the thirteen `UNIT_*_GUID`/ `PLAYER_GUILD_UPDATE_GUID` rows and `UNIT_COMBAT_GUID`. **A registration is the switch.** Upstream's README calls every `NP_Enable*Events` CVar deprecated — "registering the event is preferred" — and three of the eleven families have no CVar at all, so a CVar-only reading would leave `SPELL_DAMAGE_EVENT_*` unreachable for every addon and `SPELL_ENERGIZE_*` unreachable for all of them, since nothing installed writes its CVar. `refresh_listeners` reads the VM's own registration table (the same one `GetFramesRegisteredForEvent` reads), so a family is wanted when its CVar is set OR something is listening. That is also what keeps the cost honest: with nobody registered a damage packet queues nothing, rather than building eight arguments for a fire that reaches no frame. Nine `NP_*` CVars move from stored-only to LIVE, and `NP_EnableSpellEnergizeEvents` is registered for the first time — the first pass took the names off `NampowerSettings`' writes rather than off the DLL's own CVar table. The two reasons written at the auto-attack and heal rows were both about the *cast* path, and both were answered by the combat-log decode that was already there. The descriptor indices the GUID-keyed family watches are written out and then cross-checked against the two tables the tree already keeps — `unit_rec::FIELDS` and the `FIELD_UNIT_*` constants. A hand-written descriptor index is exactly the number that is plausibly wrong and silently so: the event simply never fires, which no amount of play distinguishes from "the server never sent one". `AttackerState` gains `sub_damage_count`, which is `AUTO_ATTACK_*`'s arg6; vmangos writes one block, but a server splitting a swing across schools sends more. Two test notes worth keeping: the edge tests run through the schedule, because `run_system_once` builds a fresh system each call and the `MessageReader` cursor restarts — a test written that way passes whatever the system does with the second edge. And the data-gated suite held at exactly 269, after the event gate correctly named all 35 new names. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn	2026-10-24 16:53:28 -06:00
gromandClaude Opus 5	a093f3c476	app: the Remember Password box writes the encrypted blob ... Asked for explicitly, a round after the machinery landed. `AutoLogin:: remember` is now the one write path the login screen uses, and it stores `password_encrypted` — the authenticated blob of `credential.rs`, keyed on 32 random bytes in a separate 0600 file. `migrate_plaintext` upgrades a file written by an older build on the next launch, and the plaintext field stays *readable* so that upgrade cannot lock anyone out of a store they already had. Two decisions inside it, both places where the convenient answer is the wrong one: A password that cannot be encrypted is **not stored at all** — there is no fallback to the clear field. The player cannot see the difference between a box that wrote a blob and one that quietly wrote plaintext, so a silent downgrade is worse than a box that visibly did not take. A stale plaintext field beside a blob is **cleared, not encrypted**: `effective_password` never read it, so encrypting it would promote a password the player has already replaced. What changed is the file. What did not is the threat model: the blob answers the file being read by another account, copied into a backup or pasted into a bug report, and cannot answer anything running as this user — the point of the feature is logging in without typing a secret, so no secret of the player's is in the chain to key it on. Both files are 0600 from their first byte. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn	2026-10-24 15:43:06 -06:00
gromandClaude Opus 5	7c1ef274a3	docs+cvars: four status rows the last two rounds made stale ... The event bucket table still listed `SPELL_QUEUE_EVENT` as deliberately absent; it has been fired from `ui_spellbook.rs` since the queue landed. Three nampower rows and two code comments carried counts from before the cast-event and queue rounds: one of five `NP_Enable*` switches live (now three), twenty-two queue CVars stored only (now twenty), and the cast and buff/debuff event families listed as fired by nothing here. Rows are marked superseded rather than rewritten, per the doc's own convention — the earlier reason is why the later one is a decision. The two names still genuinely unfired from that row, `AUTO_ATTACK_*` and `SPELL_HEAL_*`, are named as combat-log producers, which is the block they actually belong with. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn	2026-10-24 14:40:34 -06:00
gromandClaude Opus 5	702eb475a4	app+ui: the queue, the disenchant sequencer — 62/62 ... QueueSpellByName, QueueScript, ChannelStopCastingNextTick, DisenchantAll. Every one of nampower's 62 registered Lua functions is now defined; the `comm -23` in the doc prints nothing. **Upstream's queue has two halves and one is deliberately not built.** The Lua surface — remember this spell, send it when the client will accept it — is `spell/queue.rs`. The other half intercepts *every ordinary press*, so a keypress during a cast is held rather than refused, and it is governed by NP_QueueCastTimeSpells, NP_QueueInstantSpells, the buffer rows and the quickcast pair. That half is not a binding: it changes how every key in the game feels, its bugs are double-casts and dropped casts, and it is not something to switch on as a side effect of porting three function names. Those twenty CVars stay stored-only, each row saying so; the machinery they would sit on is now here, and turning it on is its own piece of work with its own live test. Two of the twenty-two went live, because they are the explicit queue's own knobs: NP_SpellQueueWindowMs and NP_QueueSpellsOnCooldown. Three slots, because upstream has three and its own event says so: SPELL_QUEUE_EVENT's code is 0/1 on-swing, 2/3 normal, 4/5 non-GCD, so the queue is keyed by what kind of cast a spell is — and the kind comes off its own Spell.dbc row through the same three tests the send path makes. Each pops on its own condition: non-GCD and on-swing as soon as no cast guards (neither uses the global cooldown), normal when the GCD has run out too. Three decisions at the queue, each pinned: - A replacement raises no popped event. A second queue of one kind replaces the first — upstream's own log line is `Replacing queued nonGcd spell X with Y` — and the replaced entry was never sent, so a consumer counting pops against queues would see one too many. - An expiry raises nothing either. `*_POPPED` means "this was sent". - The expiry is measured from the end of the cast in flight, not from the moment of queueing, so queueing early during a long cast is not punished. A lapsed deadline cannot shorten the window. **`DisenchantAll` is specified completely by the DLL's own strings**, which is why it is built this way rather than designed. `0x93087` onward carries the argument note, the precondition, the three quality keywords, every stop reason and the three chat lines — one of them as a Lua chunk that builds the item link itself. "move during cast to cancel" is the abort, and it needed nothing new: moving already cancels a cast here, the cast then fails, and a failed cast stops the run. The one piece of machinery a sequencer usually needs is the client's own, and the player's way out is the one upstream documents. How a step's outcome is read: the item is gone, or it is not. There is no "disenchant succeeded" packet — the item leaves the bag. Each step remembers the guid it cast at and waits for it to vanish with a ten-second deadline. An observable test with no new wire plumbing, right for the same reason the cast bar's own edges are. The queue's drain and the sequencer both live inside `drain_spell_casts`, which that file's own note explains: only one system may hold `CastLadder`, because a second would share its resources with every other cast-send system in an order the ambiguity gate cannot know is safe. `spell/queue.rs` names no VM — the first draft mentioned `UiScript` in one comment and `ui_script_wall.rs` counted it as gameplay code naming the VM, which is the gate working as intended: that module really is gameplay state and not a feed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn	2026-10-24 13:25:27 -06:00
gromandClaude Opus 5	b652c90a4b	app+ui+protocol: the four measured-out names, and the unit descriptor by name ... Each of these had a reason for being out. Each reason turned out to be answerable, and two of them were simply wrong. `GetSpellPower(school)` is **delegated to `GetSpellBonusDamage`**, which this client already answered — one number under two mods' names, as SuperCleveRoidMacros says itself ("Reads the same client field nampower's GetSpellPower does"). Delegating is the only way to guarantee they cannot drift, and the test asserts equality across all seven schools and the off-range values rather than a literal. The earlier "needs a descriptor band fed" was wrong: it was already fed. `GetRaidTargets()` is the 8-slot **board**, now pushed beside the party roster. The earlier reason was right about the scan it rejected and wrong about the alternative: `GroupState::raid_targets` is the reference's own table and copying it costs eight words a push. A mark with nothing on it is absent, because a zero GUID is a string that passes `if t[4]`. `LearnTalentRank` is one `LearnTalent` press per missing rank, which is what a player clicking the button does. The earlier reason — the burst cannot be checked before it is sent — is true and is not a reason to withhold it: the check that matters is the server's and the client has never had another. What can be checked locally is, so a rank already held queues nothing and one past the maximum is clamped. `GetUnitData` is the whole descriptor, from a new raw block on `UnitState`. The earlier reason — "a decision about which fields" — is now a table of seventy columns under the DLL's own names, read off its string pool in UnitFields order; `GetUnitField`'s scalar half reads the same block, so the two cannot answer differently for one name, and a test walks the table asserting exactly that. **The indices are derived and the derivation is checked.** Only thirteen `FIELD_UNIT_*` constants existed here; the rest come from 1.12's block sizes. Those thirteen span index 22 to 147 and every one lands where the layout puts it — thirteen independent confirmations, run as a test rather than asserted in a comment, with two more that no columns overlap and that nothing falls inside the aura blocks' 47..=124. Three decisions at that mapping: - An unstreamed column is absent, not zero. `ObjectFields::at` is `get_raw`'s strictness rather than `get_u32`'s absent-is-zero convenience: `health = 0` for a unit whose health has not arrived is a number a caller divides by. - `aura`/`auraApplications` come from the filtered per-unit aura list, not the raw slots — the same place `GetUnitField` has always answered them from. An unfiltered second answer would be a different unit by the same name. - `status` and `zoneId` are left out of nampower's seven computed columns: nothing says what `status` is a status *of*, and a zone is only knowable for the player. The other five are filled by the caller, which knows them. `SpellRecValue` is now `RecValue`: it serves both record readers, and a second copy of the same four shapes would drift. 58 of 62 registered names. The four left are `DisenchantAll` and the queue trio. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn	2026-10-24 12:08:46 -06:00
gromandClaude Opus 5	e716709bca	app+ui: the nine file and credential verbs, fenced and keyed ... Built on grom's explicit say-so, after a round where they were withheld and the reason recorded. Both halves of that decision stay in the doc, because the reasoning is what makes the second half a decision rather than a drift. ReadCustomFile/ImportFile (one operation, two upstream names), WriteCustomFile/ExportFile, CustomFileExists, ExecuteCustomLuaFile, StartItemExport, EncryptPassword, EncryptedServerLogin. **The fence is app-side and it is not a `..` scan.** `local_state::custom_file_path` admits one relative name whose every component is an ordinary name, inside `benilla-config/CustomFiles/`, and refuses a leaf that is already a symlink. Each escape a string check misses has its own rule and its own test row: an absolute path (`join` *replaces* the base rather than appending), a Windows prefix, a `..` nested inside a longer path, and a planted symlink — the one no amount of sanitising the string will catch. A subdirectory rather than the config root, because a verb that could create `config.toml` or `autologin.toml` by name would let an addon rewrite the client's own settings; that is a different power from "keep a file of your own". The VM side holds three closures and no paths, so no spelling of a `filename` reaches outside that directory. **The credential pair's deviation is forced, not chosen.** Both are registered by nampower under "Registering *Glue* Lua functions" and this client has no glue Lua VM at all — login, character-select and character-create are Rust. `EncryptPassword` is pure and works the same anywhere. `EncryptedServerLogin` cannot mean "submit the login form" in the only VM there is, so it stores the credential for the next login: `enabled` on, the plaintext field cleared, the blob kept, and `AutoLogin::effective_password` reads it back at the refill. What that protects is an `autologin.toml` or a SavedVariables file read by someone else or pasted into a forum: the blob is authenticated and keyed on 32 random bytes in a different file, mode 0600. What it does not protect is anything running as this user, which can read the key as easily as the blob — there is no user secret in the chain and there cannot be, because the point is logging in without typing one. Both halves are at the module. Encrypt-then-MAC with HMAC-SHA1 as the PRF for both halves, from the two primitives the tree already had (sha1 moves from a dev-dependency to a real one). The HMAC is tested against RFC 2202's own vectors, because a hand-written one still round-trips with itself when it is wrong — and a first draft of the key reader did exactly that kind of silent wrong: it returned raw bytes on creation and hex text on read, and the caller guessed between them with "32 bytes and not all hex digits". It guessed wrong the moment a random key came out all-hex. Returning one thing is the fix, and the note is at the function. **The existing plaintext path is untouched.** Storing the password in plaintext was grom's own explicit choice when the alternatives were a keyring or nothing, so this is additive and nothing migrates by itself. The machinery to make the Remember Password tick-box write a blob instead is now all here — one line — and that is his call. Base64 was missing from the whole tree; it is in `benilla-ui`'s `script/encoding.rs` with RFC 4648's own vectors, and `C_EncodingUtil.EncodeBase64` will want the same one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn	2026-10-24 11:30:49 -06:00
gromandClaude Opus 5	78e4e02cfe	docs: the registered list's buckets add up to 62 ... `StartItemExport` was in two buckets. It writes a file of its own, so it belongs with the withheld — not with the names measured out for want of a consumer. 45 built + 9 withheld + 4 not ours + 4 measured out = 62, and the `comm -23` already in that section is the check: the seventeen it prints are exactly the last three rows. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn	2026-10-24 10:26:08 -06:00
gromandClaude Opus 5	f87ef148b8	app+ui: the trinket cooldown — and a stock verb that always said "cold" ... Built: GetTrinketCooldown, UseTrinket, GetSpellRangeData, SetLocalRaidTargetIndex, GetQuestLogQuestIds, GetQuestDialogQuestId, CombatLogFlush. The find on the way through is not a nampower verb. **`GetInventoryItemCooldown` is a stock 1.12 verb and it answered `(0, 0, 1)` unconditionally** — right shape, always "cold", so nothing errored and no worn trinket ever drew its cooldown swirl on the character pane. Stock PaperDollFrame.lua:691 calls it on every slot update. Feeding an equipped slot's cooldown for GetTrinketCooldown fixes that one off the same column, and the test runs `CooldownFrame_SetTimer`'s own arithmetic against it. The feed keys its query on the (use spell, item entry) pair exactly as the bag feed does, so one trinket reads the same cooldown from the doll and from a bag. **The risk was the gate, and my first test for it was vacuous.** `feed_char` sits behind a change gate and no descriptor moves when a cooldown is armed, so `Cooldowns::feed_epoch` had to become a gate input. The first test passed with that input deleted: `run_system_once` builds a fresh system each call, so the gate's `Local` memo resets and `vm_reset` opens the gate every time. A test for a gate input has to run the system the way the schedule does. Rewritten with `add_systems` + `update`, it fails with the input removed — verified by probe, and the reason is written at the test. `feed_char` also hit Bevy's sixteen-parameter ceiling, which was a fair signal its parameters wanted grouping by what they are for: `ItemCooldownInputs` and `RepairInputs` replace six. Two decisions recorded at the code: - `GetSpellRangeData(rangeIndex)` answers three values, not a table. Every table-returning verb in this DLL has its key names in the string pool and this row's are not there. `flags` bit 0 is the melee family, where the engine takes the combat-reach sum instead of the authored pair — a caller ignoring the bit reads 0..5 for every melee ability. - `CombatLogFlush()` is a deliberate no-op. Upstream buffers its combat-log writes and this empties the buffer, so what an addon asks for is "my lines are on disk now" — and `ui_chat/logging.rs` writes each line to an unbuffered File as the window shows it, so that holds at all times. Defining it is the honest answer: the capability is present, and a missing global makes `HasFeature` report it absent. Also fed, both one wire column each: `QuestState::quest_id` (the wire names the quest on all three giver panels and 1.12's Lua names it nowhere — a title is not a key) and `SpellDisplay::stack_amount`'s neighbour work from the previous commit. Five names stay out, each now with the field list that makes it a decision rather than a shrug — `GetUnitData` (80 raw descriptor columns, read off the pool, zero call sites, and a partial answer is worse than none), `GetRaidTargets` (the reference keeps a board; this client feeds the mark per unit, and a scan over named tokens would silently omit a marked unit), `DisenchantAll` (a server-paced sequencer, which is the queue), `GetSpellPower`, `StartItemExport` (writes a file — it belongs with the withheld seven). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn	2026-10-24 10:21:31 -06:00
gromandClaude Opus 5	f06caa9968	ui+app+formats: the Spell.dbc row, and seven columns that answered nil ... `GetSpellRecField` knew three names. Counting what the corpus actually asks it for: `name` (4 sites), `school` (3), `stackAmount` (2), `spellIconID` (2), then one each of `rank`, `rangeIndex`, `mechanic`, `manaCost`, `effectMechanic`, `effectApplyAuraName`. Seven of those ten answered nil, and each was a feature quietly off rather than an error — DoiteAuras caps its stack display on `stackAmount` (DoitePlayerAuras.lua:539) and SuperCleveRoidMacros classifies a crowd-control effect off the two arrays (Utility.lua:7776-7794). The eighty field names are the DLL's own, read off its pool in Spell.dbc column order. The mapping to `SpellDisplay` lives in `ui_script/spell_rec.rs`, beside the struct it reads — a copy on the VM side would be free to drift — and `GetSpellRec(spellId)` is that same field list walked through that same per-field reader, so the two verbs cannot answer differently for one name. A test asserts exactly that, key by key, over one row. The array columns are a 1-based table of three, which is measured, not guessed: SuperCleveRoidMacros walks `effectMechanic` as `for i = 1, 3 do local em = effectMechanics[i]`. `totem`, `reagent` and `reagentCount` take the same shape, two and eight long. One new DBC column — `StackAmount`, 39, bracketed by two constants this tree already had (38 ModalNextSpell, 40 Totem[0]). It is the only one added: fifteen further names upstream answers are left nil rather than putting fifteen unused u32s on a struct the spell catalog holds one of per row. They are listed at `FIELDS` with that reason, so the gap is a decision rather than an omission. `GetSpellRecField`'s three string columns keep the spellbook fallback the verb had before the probe, so a VM with a book and no catalog — a capture, or the boot VM before the catalog loads — still answers a learnt spell's name and rank. Every other column is a DBC row and has nowhere else to come from, which is why the fallback is exactly three names wide and says so. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn	2026-10-24 09:28:05 -06:00
gromandClaude Opus 5	cf0e157f34	ui+app: the bag walk, the modifier fold, and two shapes the DLL's pool corrected ... Built: `GetBagItems`, `GetSpellModifiers`, `GetSpellIdCooldown` — plus fixes to two verbs that already existed and had the wrong *shape*, found the same way `GetItemIconTexture`'s wrong argument was: by comparing what we answer against nampower's own string pool. `GetBagItems()` hands back every occupied bag slot as `{ [bagIndex] = { [slot] = row } }`. DoiteAuras asks seven times over and walks both levels with `pairs`, reading `itemId` and `stackCount` — it is how that addon answers "do I have this item" without 100 `GetContainerItemInfo` calls per condition evaluation. Empty slots and empty bags are absent, because a nil value in a Lua table *is* absence and `pairs` is how both levels are read. `GetSpellModifiers(spellId, modifierType)` answers `flat, percent, hasModifier`, where percent is the final multiplier (90 means 90% of the original) — the consumer says so in its own words and multiplies by `percent / 100`. The 26 modifier types come from the DLL's usage string. The fold is `spell/mods.rs`' existing reader, reached through a new `spell_modifier_probe` the lifecycle reinstalls whenever the wire moves the tables: a VM-side reimplementation of a four-gate 64-bit fold would be free to drift from the mana cost the action bar greys a button on. The third return is the point — the consumer tests it before touching either number, and the duration path multiplies, so `0, 0` for an unmodified spell would zero every debuff timer that asks. `GetSpellIdCooldown(spellId)` answers the twenty-key table, from a new `Cooldowns::legs`: `info`'s three legs kept apart, same walk and same arms, so the folded number a swirl draws and the legs an addon reads cannot describe different timers. **`GetBagItem`'s row had the wrong key names** — `bag`, `slot`, `count`, `quality`, `link`, `locked`, this client's own `GetContainerItemInfo` spellings. nampower's row is contiguous at `0xa8a68` and is the block `GetEquippedItem` already used, so a caller reading `.stackCount` got nil and no error. One `bag_slot_table` now serves both, with three columns absent rather than zeroed: this client resolves a bag slot's enchants to names at the app boundary and keeps no random-properties id for a container slot, and a zeroed enchant id would be a claim. **`GetItemIdCooldown` returned the wrong kind of value** — the stock `start, duration, enable` triple. At the version we report it is a table, and the addon layer settles it: `API.GetItemCooldownInfo` and `API.GetSpellCooldownInfo` return these calls' results *directly* and their callers index them. `cd.gcdCategoryRemainingMs` is the primary path of `API.GetGCDRemainingMs`, which the `[gcd]`/`[nogcd]` macro conditionals divide by 1000 — against a triple that is `(0.0).gcdCategoryRemainingMs`, which raises. Latent rather than live (nothing calls those wrappers today), still wrong. Two things the schedule gates caught and improved: - The probe refresher started as an exclusive system, which is ambiguous against everything in Update it is not ordered against: +274 pairs over the ceiling. `NonSendMut` is all it needs. - Its memo started as a bare `Local`, which `session.rs`' own gate refuses — a memo about what this VM has been told must die with the VM. As a `VmMemo` it now reinstalls on the first frame after a `/reload`, which is better than the reasoning I had written for the `Local`: a fresh VM has no closure at all until something installs one. One count the earlier sweep got wrong, corrected in the doc: `GetSpellPower`'s "7 call sites" are BetterCharacterStats' own method `BCS:GetSpellPower(school)`, not nampower's global. Nothing in the corpus calls the global at all. A bare name match is not a call site. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn	2026-10-24 09:00:17 -06:00
gromandClaude Opus 5	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	2026-10-24 08:00:19 -06:00
gromandClaude Opus 5	ecee994ab6	app+ui: the player's own cast state, and SPELL_CAST_EVENT off the commit ... `GetCurrentCastingInfo` and `GetCastInfo` are the two most-called nampower functions in grom's corpus — 25 and 20 call sites outside `NampowerAPI.lua` — and one of them calls with no existence guard at all: `local _,_,_,_,_,_,autoattack = GetCurrentCastingInfo()` (SuperCleveRoidMacros/Core.lua:94), in the frame that keeps clearing the target while an autoattack is queued. On this client that line raised. The earlier doc row said the client had "a spell id, a deadline and a guard bit — no start instant, no channel state, no GCD end". That was true of `PendingCast` and wrong about the tree: `ui_cast_events.rs` already folds a cast window per caster off the same packets, with a start, an end, a channel bit and the accumulated pushback, behind `C_Spell.UnitCastingInfo`. What was actually missing was the caster's own machinery — the reference's cast globals, which have no descriptor field anywhere — and the GCD, which `spell/cooldowns.rs` has run correctly all along and never published. So `GetCastInfo` folds two halves: its timing columns come from the player's own window, its identity and GCD columns from a new per-tick `SelfCast` snapshot. The feed declares itself after the window drain, so the two halves can never describe different casts. Added: - `SelfCast` (benilla-ui) — the inflight spell `0xceca88`, the bar's spell `0xceac48`, the auto-repeat key `0xceac30`, casting/channeling/ onswing/autoattack, the cast counter, the cast type, the target, and the GCD's start and end on the GetTime clock. - `ui_self_cast.rs` (benilla-app) — the push, plus `SPELL_CAST_EVENT`. - `Cooldowns::gcd` — the running global cooldown as one timer. `info` still answers the per-category question every cooldown swirl asks; this answers "when does the GCD end", which has no spell to key on because its consumers divide it (`API.GetGCDRemainingMs`) or compare it against seconds (`[gcd:<1]`). - `PendingCast`'s cast counter, in the one place every class of commit already passes through. Three decisions where the tempting answer was the wrong one: - `GetCastInfo` is **nil** when nothing is casting, and an instant's GCD does not count. Answering a table whenever the GCD is up would make the consumer's fast path always hit — and `API.IsCasting()` *is* `GetCastInfo() ~= nil`, so `[casting]` would read true for a second and a half after every instant. The wrapper's own comment says what to do instead ("GetSpellIdCooldown … works for instant casts"). - `castId` is two different numbers under one upstream name: a counter in `GetCastInfo`'s table, beside a separate `spellId`; a **spell id** in `GetCurrentCastingInfo`'s returns, which is how both of its readers treat it (Init.lua:89 stores it as castingSpellId, Core.lua:5427 hands it to C_Spell.GetSpellName). Each is implemented as what its own consumer reads, and both are spelled out at the binding. - `SPELL_CAST_EVENT` fires for commits, not refusals. Upstream's `success` can be 0; this client's local refusals funnel through `CastErrors`, which carries a spell id and a reason byte and neither a cast type nor a target — a row from there would invent two of five fields. Nothing in the corpus reads the failing branch (Core.lua:5177 is one `if success == 1 then` with no else). Written down at the type rather than left to be found. Tests run the consumers' own bodies transcribed, as `cast_info.rs` next door does: `API.GetCastInfo` with its `GetCurrentCastingInfo` fallback arm, `API.IsCasting`, `API.GetGCDRemainingMs`, `API.GetCastRemainingMs` and `CleveRoids.UpdateCastingState`. A positional seven-return shape and a ten-key table are only correct relative to the code that reads them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn	2026-10-24 07:16:30 -06:00
gromandClaude Opus 5	aab2130c62	ui+docs: nampower's own registration table, and the icon id it takes ... `scripts/dll-surface.py` answers "what might this DLL publish". For nampower.dll there is a better question with an exact answer: the names it registers sit contiguously in the string pool, in registration order, bracketed by the DLL's own log lines — `Registering Custom Lua functions` immediately before the first, `Registering Glue Lua functions` before the last two. That is 60 functions plus 2 glue ones, with no inference at all, where the pointer filter gives a candidate list a reader still has to judge. The event table (59 names) and the CVar names sit the same way, just before it. A compiler emits one string literal per `lua_register(L, "<name>", fn)` and pools them in source order, so one registration function's names land in one run, and the log line that opens it is the literal printed immediately before the first call. Look for the log line, then read forward until the strings stop looking like identifiers. Two things fell out of the same pool that corrected code already written: - The 44 `Usage:` strings are the argument contract, printed verbatim when a call's arguments are wrong. `GetSpellRangeData(rangeIndex)` takes a SpellRange.dbc row index, not a spell id, which this doc had wrong; `GetSpellModifiers(spellId, modifierType)` enumerates all 26 modifier constants. - `GetItemIconTexture(displayInfoId)` takes an ItemDisplayInfo row id, the exact sibling of `GetSpellIconTexture(spellIconId)`. Ours took an *item* id and resolved the display id itself, which is a different function: upstream's addon layer keeps that hop in a wrapper of its own (`NampowerAPI.lua:2648` is the pass-through, `:2665` is `GetItemIconTextureByItemId`). So a caller handing us a real display id got "Item not found in DBCache", and one handing an item id got an icon the real DLL would not have answered. Fixed, and the test now pins that an item id answers nothing — it populates the item cache on purpose so the wrong reading would be visible if it came back. With the table read, the surface is no longer a count of what a heuristic flagged: 62 names, each in exactly one bucket (32 built, 8 withheld pending a decision that is not mine, 4 not ours to add, 18 to build), with the `comm -23` that reproduces it. The two at the head of the last bucket are the most-called nampower functions in the whole corpus, and they are coupled: SuperCleveRoidMacros registers SPELL_CAST_EVENT gated on `GetCurrentCastingInfo` existing, so defining the function without firing the event would move it from never registered to registered and silent. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn	2026-10-24 06:26:11 -06:00
gromandClaude Opus 5	7861ab2710	ui: ClassicAPI's GetCraftReagentItemID ... The one column 1.12's `GetCraftReagentInfo` does not answer: it gives the reagent's name, icon and the needed/held counts, and no item id at all, so nothing that keys a table on an item — pfUI's reagent counter — can use the stock verb. Same `(craftIndex, reagentIndex)` pair and the same rows that verb walks, so the two cannot disagree about which reagent is meant. Out of range on either index is nil, matching the stock verb's own nil-first return. All nine gates green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn	2026-10-23 21:53:10 -06:00
gromandClaude Opus 5	de3a3411e7	ui: ClassicAPI's GUID player info, no-toggle cast and stable-id skill rank ... Third DLL tranche, from ClassicAPI's side of the work list. Its 70 corpus-called names are mostly **not** functions — `Clear`, `Fire`, `Size`, `Armor`, `Nature`, `Normal`, `None` and the rest are widget methods and enum keys its DLL carries as strings, and several more (`GetSpellSchool`, `RunMacro`, `GetSpellLink`) the corpus defines for itself. Reading the call sites left nine genuine gaps; three are landed here and the rest need state this client does not feed yet. - `GetPlayerInfoByGUID(guid)` -> `localizedClass, englishClass, localizedRace, englishRace, sex, name, realm`. 1.12 has no verb that takes a GUID at all. Answered through the same `Model::unit` as the whole unit surface, so a raw GUID, a `0x`-prefixed one and every other token spelling resolve alike; an unknown GUID is seven nils, not a raise. `realm` is **nil** on purpose: the field means the realm the *named* character is from, which 1.12 has no concept of, and our own realm name there would be a guess that reads as data. - `CastSpellNoToggle(spellId)`: the plain cast by id. The toggle lives in the client's cast path and benilla's queue toggles nothing — a press is a send — so this is what the caller asked for, on the same queue a bar press uses. Same reasoning as nampower's `CastSpellNoQueue`. - `C_SpellBook.GetSkillLineRank(skillLineID)` -> `rank, maxRank`. 1.12 reaches a skill only by its index into the skills *window*, which moves as lines are learnt and categories collapse — a `SkillLine.dbc` id is stable, which is why pfQuest asks by one. It reuses `skills::displayed_ranks`, so the stable-id getter and `GetSkillLineInfo` fold bonuses and the mono override identically and cannot disagree. A line the character has no row for is `nil, nil`, not `0, 0`: untrained and unknown are different answers. All nine gates green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn	2026-10-23 21:45:42 -06:00
gromandClaude Opus 5	801bc09c05	ui+docs: SuperWoW's quest-log getters, and the DLL work list worked name by name ... `GetQuestIDForLogIndex(index)` and `GetQuestLinkForLogIndex(index)`: 1.12's `GetQuestLogTitle` answers the title and never the id, so neither the id nor a chat link for a log row is derivable from the stock API. The id reads the same `quest_log.entries` ClassicAPI's `C_QuestLog.GetQuestIDForLogIndex` does, so an addon that feature-detects one and falls back to the other gets one answer; a header row is id `0` and **no** link, rather than a link to quest 0. `docs/ADDON_COMPAT.md` now carries the whole work list with a status per name, and the most useful finding in it is not a missing function: `SuperCleveRoidMacros/NampowerAPI.lua` is a capability registry whose `HasFeature` checks the version **and** `_G[name] ~= nil`, so nothing on the list was broken and raising `GetNampowerVersion` would not have helped. Defining the global is the only honest way to make one live. Counts, so the list cannot be read as more alarming than it is: 16 done across this tranche and the two before it; 12 that were already present and only looked absent because the script compares against `dump_globals` on a bare VM (and because an *event* is never a global); 6 that need a feed, each with the specific thing missing; 3 that are the spell queue this client deliberately does not have; and 7 withheld. The seven withheld are `ExportFile`, `ImportFile`, `WriteCustomFile`, `ReadCustomFile`, `CustomFileExists`, `ExecuteCustomLuaFile` and `EncryptPassword`. Six of those give addon Lua read, write and **execute** on files under the client folder and the seventh takes a password. They are real in nampower and used for config export, but they are a different kind of surface from a getter and the right default for a client that did not have them yesterday is not to grow them quietly. `GetCastInfo` and `GetCurrentCastingInfo` are left out for a reason worth keeping: their consumers divide by the cast and GCD timings, and this client has a spell id, a deadline and a guard bit — no start instant, no channel flag, no GCD end. A table of three real fields and seven zeros is worse than the nil the addon's own fallback already handles correctly. 269 data-gated failures, identical to the baseline. All nine gates green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn	2026-10-23 21:32:37 -06:00
gromandClaude Opus 5	5555bf9c89	ui+app: nampower's bag group, aura cancels, no-queue casts, unit-event CVars ... Second tranche. Eight more verbs and six CVar rows, all from state this client already owns. The bag group — `FindPlayerItemSlot(idOrName)`, `GetBagItem(bag, slot)`, `GetItemIdCooldown(itemId)`, `UseItemIdOrName(idOrName)` — shares one lookup, deliberately: the three that search must agree about *which* of two identical stacks they mean, and they answer the first, reading bags and slots ascending, as the reference's own auto-store search walks (`0x4f8030`) and as a player reads their bags. A name matches through the item cache, case-insensitively, since a bag slot carries no name of its own. `GetItemIdCooldown` answers the stock `0, 0, 1` for a cold or uncarried item rather than nil, because the consumer does arithmetic on the triple. `UseItemIdOrName` calls `UseContainerItem` itself rather than reaching past it, so the cursor-clear and repair-mode rules the stock verb applies still happen. `CancelPlayerAuraSpellId(spellId, [ignoreMissing])` and `CancelPlayerAuraSlot(auraSlot)`: the two keys into the `CMSG_CANCEL_AURA` queue `CancelPlayerBuff` already fills, so the three cannot disagree. The slot is the **raw 0-based** descriptor slot upstream documents and the buff events report, not the Lua index. `ignoreMissing` is accepted and changes nothing — it asks upstream not to complain, and nothing here would have complained. `CastSpellNoQueue(spellId, spellBook [, unit])` and `CastSpellByNameNoQueue(name [, unit])`: the plain cast, which is the honest mapping and not a stub — benilla has no spell queue to bypass, so "cast this now, do not queue it" is exactly what happens. The point of them is the first argument: a **spell id** where stock `CastSpell` takes a book slot. `"mouseover"` routes to the queue SuperWoW's `CastSpellByName(name, "CLICK")` already uses. The six `NP_EnableUnitEventsX` switches are registered stored-only, one step further back than the four `NP_Enable*` rows beside them: benilla raises no GUID-keyed unit event at all, so there is no stream for a token filter to narrow. Registering them stops the unknown-CVar warning and lets `SuperCleveRoidMacros`'s writes persist; it claims nothing about the events arriving, and `cvars.rs`'s own reader gate carries that reason. All nine gates green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn	2026-10-23 21:23:41 -06:00
gromandClaude Opus 5	f120e8003d	ui: six of nampower's reads, and the item-icon probe behind two of them ... First tranche off the DLL work list. The method that produced it is worth recording with the code: `SuperCleveRoidMacros`'s `NampowerAPI.lua` is a capability registry — a `VERSION_REQUIREMENTS` table of every nampower name with its minimum version, reached through `HasFeature`, which tests the version **and** `_G[name] ~= nil`. So none of this was broken, and raising `GetNampowerVersion` would not have helped: the only honest way to make one of those features live is to define the global. - `PlayerIsMoving()`, `PlayerIsRooted()`, `PlayerIsSwimming()` — `1` or nil, as upstream's own doc lines state, never a boolean. All three read the per-tick posture snapshot `SelfMovement` already carries, which gains `rooted` (`MOVEFLAG_ROOT`) and `moving` (any of the six movement bits, the keyboard turns included — they are input to the same word the client tests, and a turn in place does interrupt a cast). - `IsAuraHidden(spellId)` — from a new `SpellLookup::hidden_aura`, filled with exactly `ui_aura::shown_in_aura_ui`'s predicate inverted, so it cannot contradict the aura list an addon reads beside it. An unknown spell is nil: "not hidden" is the honest answer about an aura this client knows nothing about, and the lists show a catalog miss too. - `GetItemIconTexture(itemId)` — two hops, because an item template carries a display id and not an icon: the cache row's `displayInfoID`, then `ItemDisplayInfo`'s `Icon` through a new `ItemIconProbe`. A known item with no display row is nil (a data gap); an item the cache has never seen raises and records the ask, as every `item_record` verb here does. ClassicAPI's `GetItemIcon` wants the same probe, which is why it is a probe and not a closure. - `GetTrinkets()` — slots 13 and 14 in `GetEquippedItem`'s own row shape, an empty slot absent rather than nil-valued, since the consumer walks it with `if t[i] then`. Each is in `reference_surface.rs`'s `allowed_beyond_1_12` with why 1.12 has no spelling to prefer — the client reads its own movement word and tells Lua nothing, applies the aura attribute test inside `UnitBuff`/`UnitDebuff` without exposing either, and can only reach an item icon through a bag slot it already holds. All nine gates green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn	2026-10-23 21:06:41 -06:00
gromandClaude Opus 5	ada08720bb	ui+docs: the GUID token's real spelling, 40-yard interact, and the whole patch map ... Working down the 29 `.tdata` hooks from the realm's patched `WoW.exe`. Most turned out to be already ported; two were wrong here, and one of those was wrong in a way that mattered. **The GUID token was the wrong string.** `unit::guid_token` wrote 16 lowercase hex digits and no prefix. The realm writes `0x` + 16 upper-case digits: its formatter at `0xd06500` — which `UnitExists`'s second return goes through — lays down `0x30 0x78` then sixteen nibbles each `+0x37` above 9, and its parser at `0xd06650` demands the `0x` and takes upper case only. grom's own addons agree from the other end: BigWigs decides a string is a GUID with `string.sub(guidOrName, 1, 2) == "0x"` (`MageTools.lua:963`), and AI_VoiceOver reads fields from position 3 and mints its own with `format("0x%04X%06X%06X", …)`. Same number, wrong string, and wrong where it mattered most: `UNIT_CASTEVENT` and the aura events carry these tokens, so every such test read our GUIDs as unit *names* and every `sub(3, …)` field read came out two characters off. Now written the realm's way; both spellings and either case are accepted on the way in, since refusing one buys nothing. Pinned literally rather than by round trip — a round trip held fine through the whole bug. **`CheckInteractDistance` type 4 is 40 yards, not 30.** `0x48bafd` jumps to `0xd0686c`, which tests the type against 4 and compares against the float the section carries at `0xd06868`: `1600.0`, squared thresholds. Stock 1.12's `900.0` is still what the other three types use. `docs/ADDON_COMPAT.md` now carries all 39 hook sites of both sections with a status each: 10 in `.octoql` (one feature, the quest log, ported), and 29 in `.tdata` — the aura ids, the master looter, `UnitExists`'s GUID, the char-create mask and `CHAT_MSG_HARDCORE` already done; the two challenge-mode functions needing nothing, because the realm's own `ChallengeList.lua` defines them in Lua for exactly our case; four read but unported with the reason (unnamed struct fields, no consumer in the corpus); and three that cannot apply to a client that loads no DLLs. Also `scripts/dll-surface.py`, which asks the injected DLLs the same question. Their export tables are one `DllMain` each, but a DLL that registers a Lua function must point at the name: collecting NUL-delimited identifier strings the image holds a pointer to cuts `ClassicAPI.dll` from 2872 CamelCase candidates to 706, and subtracting the 1.12 census, Lua's own names and benilla's real surface leaves a candidate list the addon corpus can rank. It is a list to read, not a specification — statically linked libraries contribute their own strings, and `UnitXP_SP3.dll` yields nothing. It found 38 names in `nampower.dll` that grom's addons call and this file did not know about; they are written down rather than guessed at. 269 data-gated failures, identical to the baseline, all nine gates green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn	2026-10-23 19:57:26 -06:00
gromandClaude Opus 5	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	2026-10-23 19:27:36 -06:00
gromandClaude Opus 5	56bdd15cfe	protocol: read the realm's quest-log slots 21-25 ... The offsets came out of the realm's own patched `WoW.exe`, which carries the patch in a PE section it names `.octoql` — 488 bytes used of 20 KB at VA 0xd26000. `GetNumQuestLogEntries` (0x4df8b0, from `reference/1.12-shapes.tsv`) begins `jmp 0xd26145`, and the routine the detour calls at 0xd26000 is the entire contract in eight instructions: slot against 20, 12 bytes per slot, `+0x28` for the first twenty and `+0x10C8` for the next five, against the same `%edx`, with a `cmp $0x5` that is the 25 the realm's Lua also declares. Both arms index one dword array, so the step between the blocks is `(0x10C8 - 0x28) / 4` = 1064 and slot 20 is field 198 + 1064 = 1262. That checks out twice independently: 198 × 4 = 0x318, so `%edx` is the field array plus 188 dwords, which is exactly `UNIT_END` — the player block's own base, with `0x28` as slot 0 inside it; and 1262 is `PLAYER_FIELD_COMBAT_RATING_1`, 1.12's dead twenty-field block, which is what their `QuestLogFrame.lua` says in prose. Five slots take 15 of those 20 fields, which is also exactly the run the detour's own change-poll walks (`cmp $0xf`) before calling the client's quest-log-changed path itself, those fields raising nothing. `player_quest_log_field` holds both blocks and every caller already iterates `0..PLAYER_QUEST_LOG_SLOTS`, so the quest log, the markers and the probes pick the five up unchanged. The test pins each boundary and the 1064 step against the disassembly, because every number here came off a listing and a one-field slip reads a neighbour's counters. Stated in the doc rather than guarded: a server that wrote real combat ratings there would show slot 21 as a phantom quest. None does — the block is dead in 1.12 and vmangos never writes it — and gating this would mean asking the interface what the protocol should parse. 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	2026-10-23 18:16:02 -06:00
gromandClaude Opus 5	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	2026-10-23 17:44:21 -06:00
gromandClaude Opus 5	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	2026-10-23 16:52:40 -06:00
gromandClaude Opus 5	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	2026-10-23 16:27:03 -06:00
gromandClaude Opus 5	cf0e56a4cf	ui: give the worst-call ordering test margins a stall cannot invert ... `the_worst_call_is_one_calls_cost_and_not_the_mean` reads both its orderings off a wall clock, and the margins were too thin to survive the full suite: it failed about one run in three, on an idle box as well as a loaded one. One of the steady handler's ten calls would pick up a ~0.9 ms scheduler stall and overtake the spike it is meant to rank below — `self_us` 5109 / `worst_us` 1405 against the spike's 1398 / 1361. Twenty steady calls instead of ten raise the window cost without raising any single call's, and an 8x spike instead of 3x puts the worst-call margin about four times beyond the stall that broke it. Six consecutive full-suite runs clean, then a full gate chain. The doc comment carries the measured numbers and says to keep both ratios, since the absolute durations are not what the test rests on. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn	2026-10-23 15:06:11 -06:00
gromandClaude Opus 5	9ce0fe7b3f	app: load the five FrameXML rows this client update added ... benilla loads its own curated `benilla.toc`, not the chain's `FrameXML.toc`, so a row the realm adds is silently absent: no "not found", no warning, just a missing global. grom updated the client on 2026-10-23 and `/xpboost` did nothing. The chain itself was fine — every archive mounted, no DBC field-count mismatch, and the two sessions' warning sets differ by two lines *fewer* after the update. The command was simply undefined. Diffing the chain's toc against ours found five rows, all added here in the chain's own order: - `OctoXpBoost`: `/xpboost`, the bonus-experience opt-out. Slash commands never reach the server, so it sends the server's `.xpboost` as a SAY, which the server runs as a command and never broadcasts. This is the reported bug. - `OctoTurtleBridge`: Turtle_General's addon-message dispatcher, for the realm's inspect replies and `RESPONSE_PLAYER_CHALLENGES`. - `Turtle_GroupUI` + `OctoGroupUIFix`: the compact group/raid frames and three fixes to them. The fix stays immediately after, as the chain has it; the xml pulls its own .lua and templates. - `OctoTurtleStore`: transmog outfits and LFT presets kept on the realm over AIO, FrameXML having no SavedVariables. Verified with the real data: `b_opens_the_backpack_and_shift_b_opens_every_bag` (the only test that reads a new row's OnLoads) stays green, and the producer gate goes from red to green with it. Through the addon harness against grom's own corpus: `SLASH_OCTOXPBOOST1` is `/xpboost` with a function behind it, the bridge and all nine group frames exist, no session errors — and the group frames are hidden with no OnUpdate armed while solo, so they cost nothing until a group fills them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn	2026-10-23 15:06:02 -06:00
gromandClaude Opus 5	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	2026-10-23 15:05:50 -06:00
gromandClaude Opus 5	d3d2103919	app: publish the world-map feed from the frame's own area ... `SetMapToCurrentZone()` answers out of the `(continent, zone)` pair `feed_world_map` pushes into the VM, because the VM cannot query the world — the reference's resolver `0x4a6650` reads the player's area live. That makes the schedule part of the verb's contract, and only half of it was declared: the feed ordered `.after(UiInput)` and nothing else, so it ran before `terrain_stream::update_current_area` wrote the area under the player and published the previous frame's zone. At a crossing the feed therefore named the zone just left, `area`'s `feed_zone_events` fired `ZONE_CHANGED_NEW_AREA` on the new one, and both addons that answer it while the map is closed reselected where the map already was. From grom's 03:57 log at the Ironforge gate: `ZONE_CHANGED_NEW_AREA 10/10 handler(s) · MINIMAP_ZONE_CHANGED 8/8 handler(s) · the UI asked to follow the player 3× · WorldMapFrame shown=Some(false)` — every handler reached, the verb called three times, the selection unmoved. Every map-derived overlay then sits a zone behind, `GetPlayerMapPosition` and so pfQuest's minimap pins included, until the player opens the world map by hand (pfQuest reselects on close), which is the workaround he reported. Ordered after `AreaAuthoritySet`, whose own doc already required it of every consumer of the area, and pinned by a schedule test over the declared graph: remove the edge and it fails naming both systems. The watchdog's zero-ask arm said "nothing has called `SetMapToCurrentZone()` since the player crossed" through all of this. The count was right and the window was not: it starts where this feed first publishes the new zone, which the stale feed made *later* than the addons' answer, so three real calls read as none. That one wrong fork cost two sessions; the wording now names the window it measures. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn	2026-10-23 04:22:49 -06:00
gromandClaude Opus 5	49b4f7b55e	ui+app: say what a fired event reached, and whether the UI answered ... grom's stale-minimap report survived the same-interior-skip fix, and his log says why that fix was the wrong suspect: at the gate the zone-text interior claim reads `outdoors`, so the skip was never consulted there. The module doc no longer claims otherwise. What his run leaves is sharper and stranger. The election cannot have declined — leaf 1537 → 809 "Gates of Ironforge" takes the zone from 1537 to 1 and the subzone text with it, so both `ZONE_CHANGED_NEW_AREA` and `MINIMAP_ZONE_CHANGED` fire — and two separate addons call `SetMapToCurrentZone()` on those while the map is closed (`pfQuest/map.lua:1466`, `pfUI/modules/minimap.lua:183`). Zero calls arrived, with nothing raised anywhere in the session. Offline, against the same addons and saved variables, both events reach every registered handler and are answered every time, so the difference is live-only and the engine cannot currently see which of three things it is. So it can now. `UiScript::fire_event_reach` answers an `EventReach` — frames registered when the dispatch began, frames whose handler ran, and the name of the frame a cut-short walk stopped on (our documented deviation: a handler that unregisters the walk's saved next ends the dispatch, and every later registrant silently never hears the event). `UiScript::frame_shown` reads the shown bit addons branch on, which no amount of engine-side logging could otherwise explain. `fire_event` is unchanged for its nineteen callers. `area::feed_zone_events` logs the whole handshake at each fire: what was elected, reached/registered per event, the `SetMapToCurrentZone` delta, and `WorldMapFrame`'s shown bit. `0/0` is nothing registered, `reached` below `registered` is a cut-short walk, and every handler run with `0×` asked is the guard both answerers share. `addon_harness --fire` reports the reach too. Gates green (459 s). His corpus offline: 10/10, 8/8 and 4/4 handlers reached. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn	2026-10-22 20:12:03 -06:00
gromandClaude Opus 5	38fd2543b4	app: the same-interior skip must not swallow a zone change ... grom's minimap report, found. Walking out through Ironforge's gate tunnel fires no `ZONE_CHANGED_NEW_AREA`, so nothing tells pfQuest to call `SetMapToCurrentZone()`, so `GetPlayerMapPosition` keeps projecting onto Ironforge and every minimap pin sits wherever that lands — until the world map's `OnShow` calls the verb itself, which is exactly "it fixes itself when I open and close the map". The zone updater's same-interior skip (`[0x868608]`, `[0x86860c]`) returned out of the whole function while the player stayed inside one group of one building. But the zone it elects on comes from the **terrain leaf**, and that can change underneath an unbroken group: the gate tunnel spans the boundary, so the leaf goes 1537 → 809 "Gates of Ironforge", whose zone is Dun Morogh, with the group never changing. The election never ran, and the cache kept saying Ironforge for as long as he stood there. So the skip now covers the naming only: it carries the previous frame's names forward explicitly and falls through. The texts then compare equal, so the only thing that can elect is the zone id — which is the case that was being lost, and nothing else changes. Three tests pin it, including the gate crossing itself and that a different group of the same building still renames. Read as a skip of the whole updater the reference would lose the "Dun Morogh" splash on the way out, which it plainly does not, so this is recorded in the module doc as a deviation chosen on that evidence rather than one traced out of the binary. The dedup key is a separate question left open there: the doc claims `(WMOID, MOGP uniqueID)` and the code has only the group's `WMOAreaTable` key. Two log lines, because this was invisible from a log twice over: the zone-text interior claim now prints on change (`sound::interior` prints the *render* claim, a different ray, and the two disagree exactly at the aprons where it matters), and an election that happens inside one unbroken group says so. Measured first, against grom's own addons: `--fire ZONE_CHANGED_NEW_AREA` in the probe harness has the loaded corpus call `SetMapToCurrentZone` 1× and move the selection, which is what ruled the addon side out and left the client. `--fire` is new to the harness for that. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn	2026-10-22 18:21:11 -06:00
gromandClaude Opus 5	9e13856cfd	app: say in the log when the world map stops following the player ... The stale-minimap report needs one fact: did the selection follow the player into the new zone? Everything map-derived rests on it, because `GetPlayerMapPosition` projects onto the **displayed** map — a selection left a zone behind seats pfQuest's minimap pins against the wrong rect, and plausibly rather than obviously wrong wherever two rects overlap, as Ironforge's does inside Dun Morogh's. The `zone:` line could not answer it even after it started printing both pairs, because the pairs disagree constantly and mostly correctly: for a frame or two after every crossing, since this feed publishes the new zone and the `ZONE_CHANGED*` an addon answers by reselecting fires after it; and for as long as the map is open on another zone, which is what browsing is. A marker on that line would have cried wolf on both. So the rule is its own thing, `SelectionLag::step`: an episode starts only on a frame where the *player* moved to a zone the map is not on, and only an episode outliving a 1 s grace is reported — once, at WARN, with the catch-up line when something finally reselects. A selection that wanders under a standing player is never an episode. Five tests, including the 600-frame browse that must stay silent. The line also forks the cause, which is the part that saves a round trip: `UiScript::world_map_resolver_calls` counts Lua's `SetMapToCurrentZone` calls, so the report can say whether the UI never asked to follow — the event did not reach the addon — or asked and still landed wrong, which is the verb reading a zone the feed had not published. Same symptom, different fixes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn	2026-10-22 17:20:28 -06:00
gromandClaude Opus 5	e282410938	Seat the map catalog in the harness, and print the real map selection ... Two instruments, both because a diagnosis went wrong on this data today. The zone line said "world map shows" while printing the player's *resolved* zone, not the selection. Those are not the same fact, and conflating them cost a whole diagnosis: GetPlayerMapPosition projects onto the **displayed** map, so a selection sitting a zone behind is exactly what leaves every map-derived overlay stale — and from the log it looked identical to a selection that had followed. Both pairs are now printed, with an explicit "STALE: the map is not on the player's zone" marker, so a plain run answers it with no flags. The harness never seated the world-map catalog, so C_Map.GetMapWorldSize had no rects there. pfQuest's minimap_sizes metatable caches a false per zone on a miss, so a probe of its pin gate reported "no zone sizes" for Dun Morogh, Elwynn, Orgrimmar, Thunder Bluff, Darnassus and Loch Modan — and the four zones that did answer were the four whose sizes the addon still ships. Every number was real and the conclusion was false. With the catalog seated (and the C_TaxiMap table beside it, for the same reason) all six answer and SearchFlightNodes adds 69 nodes. seat_a_session already carried the area-name column against exactly this fault class; this is the next feed in the same wall. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn	2026-10-22 07:42:33 -06:00
gromandClaude Opus 5	ce8992bc3e	Draw flight masters: C_TaxiMap.GetTaxiNodesForMap over TaxiNodes.dbc ... grom: "can't see flight master location marks, even tho I have them on." Nothing was broken — the layer was silently empty. pfQuest-octo has deleted its shipped pfDB.meta.flight table in favour of this call (db/meta.lua:181 is a stub whose comment says the data "always comes live from the client (ClassicAPI)"), and SearchFlightNodes opens with if not (C_TaxiMap and C_TaxiMap.GetTaxiNodesForMap) then return maps end so with the namespace absent the option stays ticked, no error is raised, and no icon is ever drawn. Same silent-by-construction shape as C_Map.GetAreas, same consumer. The contract is half read off the bundled ClassicAPI.dll's string pool (C_TaxiMap, GetTaxiNodesForMap, nodeID, faction, reachable, state and the FlightPathFaction/FlightPathState enum names) and half off the caller: the areaID/mapX/mapY triple is not separately visible there, since the pool holds one copy of each string and C_Map.GetAreaTriggerInfo already uses all three, so for those SearchFlightNodes itself is the authority. It reads six fields, gates on `node.areaID and node.mapX and node.mapY`, and feeds the pair into pfMap:AddNode as meta.x/meta.y, which UpdateNodes divides by 100 — so they are percentages of the zone rect. Both files say which half is which. Faction comes from the two MountCreatureID columns, the reference's own gate (0x4a2650 asks for the mount of the player's half): one column filled is that faction, both is neutral, and neither means the row is not a flight master at all — on this install those are exactly the eight Transport/Ferry/zeppelin endpoints, and passing them on would put "Flight Master" pins on the Menethil docks. Attributing a node to a zone needed the file's own names. Geometry over WorldMapArea boxes proposes — these are axis-aligned boxes around irregular zones, so a node near a border sits inside two or three of them — but no purely geometric rule works: "smallest box" misfiled a dozen of ~70 rows (Crossroads under Mulgore, Aerie Peak under Alterac Mountains), and "most central box", much better and what zones_at now sorts by, still loses Talonbranch Glade to Darkshore by about one percent of inset. TaxiNodes.dbc names its rows "<place>, <zone>" and a city row leads with the city, which is itself a zone, so trying each comma-separated part against the candidates gets Ironforge over Dun Morogh *and* Felwood over Darkshore from one rule. zones_at also reaches the instance-map list, so Alterac Valley's two nodes place on the battleground map instead of nowhere. Seeded before the interface loads, after the map catalog it reads the rects from. That deadline is tighter than file scope: pfQuest replays its saved tracking selections once on PLAYER_ENTERING_WORLD and then unregisters every event (database.lua:131-151), so a push one frame later is a push after the only read. Verified against the real addon offline, not just by unit test: SearchFlightNodes now adds 69 nodes through the probe harness. Three tests pin the rule against the DBC itself — the capitals and the border nodes by hand, and every named row held to the zone its own name claims, which is the test that rejected both earlier geometric rules. It also pins the four rows that name no zone, three of which are this realm's own additions (Northshire Abbey and two Darkshore canoe routes), so the next custom node shows up as a failing assertion rather than a pin in the wrong zone. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn	2026-10-22 07:42:22 -06:00
gromandClaude Opus 5	f434cc9916	Survive a net entity despawned before attach's queue is applied ... grom's 06:23 session died 183 s in on first world entry (Diagnostics/crash-1792671984.txt): insert::<GroundShade> on 4431v0, "its index now has generation 1", one frame after a 4301 ms world-entry hitch during which the server sent a burst of creates and destroys. The mechanism is structural, not a one-off. A command queue is applied at the schedule's next sync point, not when the system returns, so a destroy handled by a system that ran *earlier* in the same frame is still only queued while attach_entity_visuals' query runs: the row looks alive, the whole visual is built for it, and the despawn lands first at the sync point. The panicking command form then kills the client for an entity the server had already taken away. So every write this system makes onto the net entity root is now a try_ command. add_child was the one that needed more than a rename: it queues its command on the *parent*, so the conform node and RigPose::anchor_for's bone anchors are related at spawn with ChildOf instead — the relationship hook tolerates a missing target where a command on it does not. The test builds a CommandQueue the way the existing rig test does, despawns the root before applying it, and asserts the entity stays gone. It fails with grom's exact panic text when any single write is put back, which is how I know it bites. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn	2026-10-22 07:41:59 -06:00
gromandClaude Opus 5	b4a5d3b38f	ui: "do I know this spell?" asks the known set, not the spellbook ... Every "Already known" check read `spellbook.slots`, and `SpellDisplay::in_spellbook` filters out `SPELL_ATTR_IS_TRADESKILL` — correctly, since recipes live in the trade-skill window, not the spellbook. So the question "do I know this spell?" answered **no for every crafting recipe in the game**: a recipe item never showed its `ITEM_SPELL_KNOWN` red line, and nothing stood between the player and consuming it a second time. grom learned Plans: Copper Chain Vest (item 3609) twice. `PlayerReqState` gains `known_spells`, filled from `PlayerActions::spells` — `SMSG_INITIAL_SPELLS` plus every learn, which is the set the reference's own check reads (`0xb700b0`). It belongs there: that struct is already "the player state the tooltip's red lines and the usable gate compare against", already change-gated on the action/spell inputs, and already pushed by the same feed. Both readers move over: the taught-spell line and `item_usable_by_id`'s spell requirement, which had the same hole. The existing `verified_families_signable_locked_resists_known` only passed because it made the recipe known by putting it in the spellbook, which no real session does — it now sets the known set instead, and that is the change's own regression story. The new test leaves the spellbook deliberately empty, which is the real shape and what fails if the check ever reads it again; it asserts the not-yet-learned case too, so it cannot pass by always reading red. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn	2026-10-21 22:17:15 -06:00
gromandClaude Opus 5	a2da3e4da7	ui: a skill lock you can open stops reading red ... The GameObject tooltip's skill-lock line took the opener-*unknown* arm unconditionally — `LOCKED_WITH_SPELL` in red — whatever the player's skill. The known arm was simply not built, and the code said so where it was missing. The reference forks on whether any known spell opens the slot's LockType (`0x52abcb`): known takes `LOCKED_WITH_SPELL_KNOWN`, which carries the requirement as a `%d` and the colour of the skill-difficulty ramp `0x529fa0`. Built now, both halves. grom reported it on a copper vein at Mining 70, which under the ramp is +69 over the vein's requirement of 1 — the green rung, not the grey one he expected; grey starts at +100, so a copper vein greys at Mining 101. Either way not red, and red is what it showed. The resolve is hoisted out of the `flag_locked` branch, because the "Locked" line's colour and this line's known arm are two readings of one answer and must not disagree with the click. `lock::skill_margin` recomputes the two numbers from the opener the resolver already matched, with the same two functions its own value test uses (`0x5f850f`), so hover and click cannot diverge on which opener is in play. Deviation, recorded at the constants: the three new rung RGBs are the standard trade-skill palette, not values read out of the binary's table. Only the ramp's shape is traced — five rungs, bands at +0/25/50/100 — and the rung a line lands on is what the player reads. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn	2026-10-21 22:17:04 -06:00
gromandClaude Opus 5	92b2e31577	app: order the zone events after the world-map feed ... `feed_zone_events` fires `ZONE_CHANGED`, `ZONE_CHANGED_NEW_AREA` and `MINIMAP_ZONE_CHANGED` inline into the VM. pfQuest answers all three by calling `SetMapToCurrentZone()` whenever the map is closed (`pfQuest/map.lua:1466`), and that verb reads `worldmap.player_zone` — which `ui_world_map::feed_world_map` publishes, in a system that was ordered only `.after(UiInput)`, exactly as this one was. Nothing ordered the two against each other. Run the wrong way round, `SetMapToCurrentZone()` reselects the zone the player has just left, because the feed has not published the new one yet. Every map-derived overlay then stays a zone behind until something else reselects — opening the world map, say, whose `OnShow` calls the same verb a frame later when the feed has caught up. There is nothing to order in the reference: `SetMapToCurrentZone` calls the resolver `0x4a6650`, which reads the player's current area live. Reading a per-frame feed instead is ours, so the ordering is ours to state. Stated as a set rather than a bare `.after`, since the next consumer of the published zone will owe the same edge. Found while chasing grom's report that pfQuest's minimap icons do not re-seat on a zone change until the world map is opened — which is this shape. Not confirmed as that bug's cause: it is an unpinned order, so which way it fell before this commit is not something the logs record. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn	2026-10-21 21:37:12 -06:00
gromandClaude Opus 5	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	2026-10-21 17:47:07 -06:00
gromandClaude Opus 5	e6ae2e0f67	ui: a scroll names the child it moved instead of giving up on the graph ... `SetVerticalScroll` and `SetHorizontalScroll` called `Model::touch_layout` on the line before `reanchor_child`. That gave up naming for the whole graph and then *erased* the precise touch that followed: `reanchor_child` re-anchors the scroll child through `write_anchor`, which reads an offsets-only change (same point, same target) and names the child node — but `touch_layout_node` will not un-say an imprecise touch, so the give-up won on every call. Naming the child is the whole truth here. A scroll offset reaches the layout only as that one anchor: anchor targets are ids fixed at `SetPoint`, so no edge moves and the roster is unchanged, and the clip a scroll child draws within is the ScrollFrame's resolved rect rather than the offset — `clip::scroll_clip_sources` rebuilds it per call and caches nothing. Measured on grom's 2026-10-21 17:08 session, which was a map-and-panel run. These two lines were **472 of the run's 806 whole-graph derivations**, the largest single source by a factor of two, and the cost lands on the Lua that reads a frame edge afterwards rather than on the pass's own `resolve` phase: | | n | fps | UI pass | `frame:GetLeft` | |---|---|---|---|---| | windows with >= 10 scroll derivations | 12 | 68.5 (min 53) | 5.93 ms/f | 2.18 ms/f | | windows with no derivations at all | 11 | 92.4 (min 77) | 1.62 ms/f | 0.05 ms/f | `r(scroll derivations, GetLeft us/f) = +0.907` over 57 in-play windows, against `+0.014` for every other derivation site. `[ui-verbs]` has `frame:GetLeft` at 94-376 us a call in those windows and 0.6-1.5 us in the quiet ones, on ~14 calls a frame, because stock FrameXML reads frame edges constantly and each read past the clean epoch re-derived. That is why a scrolling panel — the world map, the quest log, a profession list — was the expensive thing on screen. The test scrolls both axes on the shipped interface and asserts zero derivations, with a positive control so it cannot pass on a dead counter. Put one `touch_layout` back and it reads exactly 10 for 10 scrolls. `WOW_LAYOUT_VERIFY=1` over the real 92-addon interface gives `frames 13379 -> 13379, regions 29597 -> 29645` — identical to the recorded pre-existing baseline, so this narrowing introduced nothing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn	2026-10-21 17:46:55 -06:00
gromandClaude Opus 5	754d577eaf	ui: a trade-skill list mutator signals inside its own call ... `ExpandTradeSkillSubClass`, `CollapseTradeSkillSubClass`, `SetTradeSkillSubClassFilter` and `SetTradeSkillInvSlotFilter` fire `TRADE_SKILL_UPDATE` from inside the call now, as the one helper all four tail-call in the reference does (`0x4fd710`, `0x4fd730`, `0x4fd750`; `reference/1.12-verb-events.tsv`). It used to be a `trade_skill_touched` flag drained by `ui_tradeskill::drain_trade_skill` and answered the following frame. Inline is load-bearing. These four verbs are called *from* a `TRADE_SKILL_UPDATE` handler: a profession library reads the recipe list by unfolding every header, walking it and folding back what was folded. The signal is therefore re-entrant by construction, and every addon that does it carries its own guard — LibCraftingProfessions has `trade_skill_scan_in_progress`, ATSW an `atsw_scans < 2` counter. Those guards are written against an inline signal and only an inline signal lands inside the window they cover. Deferred to the next frame it landed after the guard had cleared: the library rescanned, the rescan called the verb, the verb re-armed the flag, and the two kept each other alive at one fire a frame for as long as the window was open. grom's 2026-10-21 session shows it directly — `[ui-events]` reporting `TRADE_SKILL_UPDATE · 1.01 fires/f · ~900 call(s)` across 37 consecutive windows, three unbroken minutes, at 0.58-0.84 ms a frame in handlers over four addons rebuilding their lists for nothing. It is the 118-second and 61-second stretches at 55-70 fps in a session whose in-play median was 86. Two tests: each mutator's fire is read with its own `run` still on the stack, so a deferred signal fails; and a library-shaped in-progress guard turns one real update into exactly one scan, with nothing owed to a later tick. The bench's `[events]` block also fires the five non-quest events the live session named. They cost ~0.2-0.5 ms there against 12-72 ms live, because this harness has no NPC in front of it, no trainer list and no open map — recorded as a gap, not an answer. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn	2026-10-21 14:47:57 -06:00
gromandClaude Opus 5	e269e4dbb4	ui: name the event, not the library frame that dispatched it ... `WOW_UI_EVENTS=<secs>` (`script/event_prof.rs`): per event, what one fire costs across every handler it reaches, and which frame took the most of the worst fire. The gap it fills is a dispatcher. `WOW_UI_HANDLERS` keys on `(frame, script)`, which is the right key for an `OnUpdate` and the wrong one for an `OnEvent`, because every addon library registers *one* frame for *every* event it wants and fans out in Lua. So the largest identifiable cost in grom's 2026-10-21 session, eleven separate times, read as [ui-handlers] 312.5 312.5 0.00 72522.3 AceEvent30Frame:OnEvent — a true row that names a library's shared frame and no addon, no event and no cause. `[sys-timing]` saw the same cost as `ui_quest::feed_quest` at 85.7 ms for one run in 76, because every handler runs synchronously inside whichever system fired, so the system holding the stack gets the bill. Two conventions the report sticks to: - `self` is nest-adjusted, in both columns. A handler's own time has any event it fired taken back out (`event::fire` differences `event_prof::nested_ns` across the call), or every event in a cascade reports the cascade and the row naming the real cost is the one that looks innocent. `total` keeps it, which is what a `[sys-timing]` span sees. - A fire is one `fire_event`, however wide it fans out, because that is the unit a dropped frame is measured in. `calls` is beside it for the width. Measured on the player's own 92-addon interface (`tick_bench`'s new `[events]` row, three fires per event from a settled start): us/fire worst event 5068 14759 QUEST_DETAIL <- HCQuest.lua:139 took 14,097 us 764 1324 QUEST_LOG_UPDATE 662 1564 QUEST_FINISHED Read `us/fire` against `worst` and that 14.8 ms is **one** fire of three: the other two cost ~0.1 ms, so it is HCQuest building something once and keeping it, not a cost paid per quest. With that read correctly, no quest event costs more than ~1.5 ms recurring offline — so the bench does not reproduce the live 57-73 ms and says so, rather than offering a number that looks like an answer. Hence the instrument, armed in `./measure.sh`. Also adds `./measure.sh --frames`, which arms two meters that already existed and have never been armed: `WOW_FRAME_PHASES=50` and `WOW_RENDER_MS=1`. A frame's wall time is `max(main app, render app)` since bevy pipelines the render app onto its own thread, so when the render app is the longer, `wall` climbs while `cpu`, `main`, `gpu` and every per-system `worst` all read healthy — exactly what that session's **161 ms frame in a second whose costliest system worst was 2.9 ms over 105 runs** looks like, and what 43 of its 64 spike-seconds look like. Its own mode because both perturb what they measure. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn	2026-10-21 06:43:01 -06:00
gromandClaude Opus 5	0c862838da	ui: a SetScale names its subtree instead of giving up on the graph ... `[ui-phases]`'s new write-site tally answered the combat-fps question on its first armed session: 463 whole-graph layout derivations across 34 windows, and **232 of them** at `object/frame_state.rs:427` — the `touch_layout()` inside `SetScale`. Splitting the per-window count by site and regressing against resolve time over 43 in-world windows, resolve correlates with `SetScale` derivations at r = +0.996 and with every other derivation at r = -0.041. The four costliest windows of the run are 36-64 `SetScale` derivations each and almost nothing else. That also corrects the flat "~15.5 ms per derivation" of the round before: a mint-triggered derivation costs ~1.1 ms, a `SetScale`-triggered one 12-13 ms. The difference is that `SetScale` gave up *two* ledgers, not one. The caller is pfUI's action-button press animation (`pfUI/modules/actionbar.lua:182`, `:207`, `:233`, `:259`): four styles that all run `this:SetScale(this:GetScale() + fade)` on `OnUpdate` for the ~60 frames a button's fade lasts. Pressing an ability starts it, which is why a settled interface never reproduced this and a fight always did. Both give-ups had precise versions already, written for `SetParent`: - `touch_layout()` -> `touch_layout_subtree_scale(h)`. A scale write moves the subtree's `effective_scale` and nothing else: anchor targets are ids fixed at `SetPoint`, so no edge moves and the roster is unchanged. - `touch_measure_all()` -> the same walk now names the subtree's regions on the measure ledger. This was the bigger half. `measure_dirty = None` makes the next `fontstrings_needing_measure` walk all ~43,000 regions of a real interface to issue, typically, zero requests; `stale_measure_key` reads only a region's own data plus *its owner frame's* `effective_scale`, so the subtree's regions are exactly the ones that can go stale. Measured on the player's own 92-addon interface, 43,195 regions, one `SetScale` per frame for 240 frames (`tick_bench`'s new `[scale]` row): before changing resolve 0.601 ms/f · 0.996 derivations/f (239/240) after changing resolve 0.005 ms/f · 0.000 derivations/f ( 0/240) control same resolve 0.005 ms/f · 0.000 derivations/f ( 0/240) `changing` is now indistinguishable from a control that writes the value it already holds. The harness understates the live cost (its measure answers are a 6px-per-char stub), so the live saving is to be confirmed in a play-test, not predicted from that row. Correctness is left to `WOW_LAYOUT_VERIFY`, which `cfg!(test)` forces on for all 1975 benilla-ui tests: it re-derives the whole graph after every `resolve()` and panics on any rect the incremental pass got wrong, and runs the full measure roster beside the ledger drain and panics on any stale region the ledger missed. So the two new tests assert only that the cheap path was taken, on a graph three frames deep with a FontString on the leaf. The real-interface verify run fails identically to its documented pre-existing red (`frames 13379 -> 13379, regions 29597 -> 29645`), so this change adds nothing to it. Also repairs an assert message in `layout_gate.rs` whose line continuations had been eaten by a heredoc, leaving runs of spaces mid-sentence. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn	2026-10-20 20:57:15 -06:00
gromandClaude Opus 5	c1a56f98ab	layout: name the write that asks for a whole-graph derivation ... `[ui-phases]` could say a window paid for 61 whole-graph derivations. It could not say which of the forty-odd `touch_layout` sites asked for them, and that is the half a fix has to be aimed at. Measured on the player's interface, one derivation costs ~15.5 ms — from two directions: the derivation count explains total resolve time at r = +0.977 across twenty windows (solve count: r = -0.33, so solves are free and derivations are everything), and 26 of 78 worst-single-call figures over 1 ms land in the 15-16 ms bin across eleven unrelated handlers by six authors, two of which fired exactly once for their whole window. Eleven handlers cannot each independently be 15.5 ms slow; it is one shared charge landing on whoever reads the graph first, which is also why the same session shows `resolve` p99 at 14.4 ms and worst frames "mostly tick". One defect, two accounts. So `#[track_caller]` down the naming chain and a tally of `file:line`, printed in the report. Not a backtrace: `WOW_LAYOUT_DERIVE_TRACE` already captures those and has to be rationed to `n` prints because of what they cost. This is a `(&'static str, u32)` key off `Location::caller()`, cheap enough to leave armed, and take-and-rearm so a window reports its own rows and a VM replaced by `/rl` re-arms without anybody noticing it was replaced. Also a `[churn]` window in tick_bench, which records a negative: a plate roster turning over 4-in-40 every ten frames costs **zero** derivations on the player's own interface. `sync_nameplates` pools the plate frames, so a new key rewrites an existing plate and mints nothing. With `resolve_bench`'s hover-sweep, tooltip-content and moving-region tests all asserting zero, four candidate triggers are now ruled out offline and the live tally is what names the real one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn	2026-10-20 19:17:44 -06:00
gromandClaude Opus 5	4393bde4ec	sys-timing: a stall report names the thread and how long it was stuck ... Two durations were being conflated. How long the thread has not moved is the defect; how long the span it is inside has been open is only where it is stuck, and for an enclosing span that is the whole session — which is why the live log said "has been running 360380 ms" and then "was in there at least 360406 ms" about a 150 ms hiccup. So both, labelled, and the thread's name beside them: the span it is stuck in may be unnamed, since an enclosing `info_span!` carries no `name` field for `on_new_span` to record, and then the thread is the only thing the report can name. Before this, such a stall printed `span #2473901162497` and nothing else. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn	2026-10-20 19:17:44 -06:00