Commit Graph
4 Commits
Author SHA1 Message Date
gromandClaude Opus 5 5a824f1625 app: the press queue was switched off in play — a derived Default is the live config
`cvars.rs` announces a `CvarChanged` only when a value *moves*, and `config.toml`
holds only the rows that left their defaults ("CVar values that moved off their
defaults" — its own header). So a row the player never touched never reaches an
`on_cvar` observer, and the resource's own `Default` is what the client runs with
for the whole session.

`PressRules` and `SpellQueue` both derived `Default`. Every toggle was false, every
window 0 and every buffer 0, so the press-interception queue held nothing, the mount
refusal never fired, and `SpellQueue`'s zero window expired each explicit entry on
the frame it was queued — the day after all of it was committed. Fourteen of the
sixteen rows have a non-zero registered default.

grom's own `WoWCombatLog.txt` is what named it, with no live run to ask for: 147
`Sunder Armor: Not yet recovered` against 163 landed Sunders, in bursts 0.17-0.23 s
apart — every press refused, none held. Sunder Armor rank 1 on this realm is
`rec=0 cat=0 catrec=0 gcdcat=133 gcd=1500`, read off its own `Spell.dbc`: a global
cooldown and nothing else, which is precisely the case the queue must hold.

Both now hand-write `Default` with the registered values, which is the pattern
`cvars.rs::registered_defaults_mirror_the_code_truths` already welds for the engine
knobs; `np_press.rs::the_default_is_the_registry` welds these sixteen the same way.
The test-only `defaults_for_tests()` constructors are gone — they spelled out the
registered numbers for tests to read while the client could not reach them, which is
the whole shape of the bug. A sweep of the other nine CVar-cached resources (list
every `fn on_cvar`, take its `ResMut` types, check for a hand-written `Default`, look
up each CVar it names in `REGISTERED`) found no further victims.

Two things so the next session can speak for itself:

- `a_press_made_during_the_global_cooldown_is_held_and_sent_when_it_ends` — the
  end-to-end cycle with the GCD as the only guard, which is what a class with no cast
  bar presses into all day. The existing end-to-end test arms `PendingCast` by hand
  and so never starts a GCD at all; this one presses through the real ladder, so the
  commit arms it the way a cast does.
- one `info!` a minute from the drain, and only when something moved: held, sent,
  dropped, retried, fell-through, channels cut — plus the live windows and buffers.
  Every other line this half writes is `debug!` and the default filter is INFO, so a
  67-minute session could not say one word about the queue. A line of zeroed knobs
  now diagnoses this class of fault by itself.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn
2026-10-25 09:46:50 -06:00
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 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 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