grom/benilla-octo
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

app+ui: nampower's press-interception queue — the half that changes how keys feel

Browse Source 
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
This commit is contained in:
grom
2026-10-24 23:46:42 -06:00
co-authored by Claude Opus 5
parent 94c571e3ea
commit a9bb9624e9
16 changed files with 1848 additions and 217 deletions
Download Patch File Download Diff File Expand all files Collapse all files
docs/ADDON_COMPAT.md
+93 -16
View File
@@ -896,19 +896,16 @@ Three decisions at that mapping:
**Built**, on grom's "62/62": `QueueSpellByName`, `QueueScript`, `ChannelStopCastingNextTick` and
`DisenchantAll`.
**Upstream's queue has two halves, and one of them is deliberately not built.** The Lua surface —
"remember this spell and 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;
it is governed by `NP_QueueCastTimeSpells`, `NP_QueueInstantSpells`, `NP_QueueTargetingSpells`,
the buffer rows and the quickcast pair. That half is not a binding: it changes how every key in the
game feels, it is the one piece whose 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. If grom wants press-interception, it is its own piece of work
with its own live test — and the machinery it would sit on is now all here.
> **Superseded by "The press-interception half, built" below.** The paragraph that stood here said
> the press half was deliberately unbuilt and its twenty CVars stored-only. It is built now, on
> grom's say-so and with a live test to follow; the slots are still the ones described in this
> section, because upstream has one queue.
Two of the twenty-two went **live**, because they are the explicit queue's own knobs:
`NP_SpellQueueWindowMs` (how long past the in-flight cast's end an entry is held before it gives
up) and `NP_QueueSpellsOnCooldown` (whether an entry may wait out the spell's own cooldown).
**Upstream's queue has two halves, and both are built now.** The Lua surface — "remember this spell
and send it when the client will accept it" — is `spell/queue.rs`, below. The half that intercepts
**every ordinary press** is `spell/np_press.rs`, and the section after this one covers it. They
share the three slots and the one `SPELL_QUEUE_EVENT`: upstream has one queue, and a second set of
slots beside it would be a different feature wearing the same name.
**Three slots, because upstream has three and its own event says so.** `SPELL_QUEUE_EVENT`'s first
argument is `0`/`1` on-swing queued/popped, `2`/`3` normal, `4`/`5` non-GCD — so the queue is keyed
@@ -966,9 +963,88 @@ comm -23 /tmp/np /tmp/ours
| **Built** | **62** | `CancelPlayerAuraSlot`, `CancelPlayerAuraSpellId`, `CastSpellByNameNoQueue`, `CastSpellNoQueue`, `ChannelStopCastingNextTick`, `CombatLogFlush`, `CustomFileExists`, `DisenchantAll`, `EncryptPassword`, `EncryptedServerLogin`, `ExecuteCustomLuaFile`, `ExportFile`, `FindPlayerItemSlot`, `GetBagItem`, `GetBagItems`, `GetCastInfo`, `GetCurrentCastingInfo`, `GetEquippedItem`, `GetEquippedItems`, `GetItemIconTexture`, `GetItemIdCooldown`, `GetItemLevel`, `GetItemStats`, `GetItemStatsField`, `GetNampowerVersion`, `GetPlayerAuraDuration`, `GetQuestDialogQuestId`, `GetQuestLogQuestIds`, `GetRaidTargets`, `GetSpellDuration`, `GetSpellIconTexture`, `GetSpellIdCooldown`, `GetSpellIdForName`, `GetSpellModifiers`, `GetSpellNameAndRankForId`, `GetSpellPower`, `GetSpellRangeData`, `GetSpellRec`, `GetSpellRecField`, `GetSpellSlotTypeIdForName`, `GetTrinketCooldown`, `GetTrinkets`, `GetUnitData`, `GetUnitField`, `GetUnitGUID`, `ImportFile`, `IsAuraHidden`, `IsSpellInRange`, `IsSpellUsable`, `LearnTalentRank`, `PlayerIsMoving`, `PlayerIsRooted`, `PlayerIsSwimming`, `QueueScript`, `QueueSpellByName`, `ReadCustomFile`, `SetLocalRaidTargetIndex`, `SetMouseoverUnit`, `StartItemExport`, `UseItemIdOrName`, `UseTrinket`, `WriteCustomFile` |
**62 of 62.** The `comm -23` above is the check, and it now prints nothing. What remains unbuilt is
not a *name* — it is upstream's press-interception half of the queue, which has no Lua surface of
its own and whose twenty CVars say at each row that they are stored-only and why.
**62 of 62.** The `comm -23` above is the check, and it prints nothing.
`CastSpellNoQueue` and `CastSpellByNameNoQueue` became meaningful with the press half: they take
the ladder entry that skips the queue (`CastLadder::send_now`), so a macro that has already decided
*when* to cast is not held by the client deciding to cast later instead. Before the press half they
were the plain cast, which was the honest mapping at the time and is no longer the whole story.
### The press-interception half, built
`spell/np_press.rs`. A press that the client would refuse for a **timing** reason is held in the
same slot an explicit `QueueSpellByName` would use, and the drain sends it the moment the guard
clears. No packet, no global cooldown, no error line while it waits.
**Where the rung sits, and the one deviation that comes of it.** 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, and holding them
would change the meaning. Everything below it is a real refusal the player should read: no reagent,
no weapon, out of range, no power, stunned, moving, wrong form. Upstream hooks the client's cast
entry and so holds presses that would fail for those reasons too, which means its error arrives
when the entry pops, a second later, about something the player has stopped doing. Refusing them
immediately is the deviation, and it is the better half of the trade.
**What a window measures.** Upstream publishes four and no sentence saying. Two readings fit the
names: how long an entry survives, or how close to the guard's end a press must land. This takes
the first, for three reasons — `spell/queue.rs` already reads `NP_SpellQueueWindowMs` that way, and
one number meaning two things in one client is a trap nobody finds twice; upstream's own `Replacing
queued nonGcd spell X with Y` means the last press wins, so holding every press costs nothing in
staleness; and the admission reading would make the 250 ms cooldown window refuse almost every
press made during a global cooldown, which is the exact complaint nampower exists to answer.
Two rows do read a window as a threshold on remaining time, and both say so themselves:
`NP_InterruptChannelsOutsideQueueWindow`, whose name states one, and `NP_CooldownQueueWindowMs`,
which has to be one. A cast or a global cooldown ends in a second or two; a spell's own cooldown can
be half an hour. Holding that press means either firing it half an hour later or swallowing it with
no error line, and a Hearthstone pressed on cooldown has to still say "Item is not ready yet" — so
a quarter of a second, which is what the row defaults to, is an allowance for pressing a touch early
rather than a queue.
**The guard reopens early, and that is the headline.** `PendingCast` now keeps two timestamps where
it kept one. 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 `SMSG_SPELL_GO`
or `SMSG_CAST_RESULT` arriving a round trip later is still recognised as the answer to that cast.
Splitting them 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 about half a trip after the send; the server starts its cast then and
finishes half a trip after our own cast time elapses. A next cast sent at exactly `cast_time`
therefore arrives just as the server frees up, and the latency cancels out of the arithmetic
entirely. The buffer is jitter margin on top. `NP_NonGcdBufferTimeMs` is wider (100 ms) because a
non-GCD spell has nothing *but* the guard between two presses.
**`NP_RetryServerRejectedSpells` is the mitigation for that early guard**, and that is why upstream
defaults it on. If the margin is ever wrong — jitter, or a server cast time that differs from ours —
the server refuses the next cast and without a retry the press is simply gone. It goes back in its
slot **once**, held a buffer's worth, on exactly two reason codes: `0x3c` not-ready and `0x61`
another-action, the two an early guard can cause. Every other refusal is the server telling the
player something true, and re-sending one would be a packet a second that never succeeds. Upstream's
own set is not published and its log line (`" spell queued + ignored code "`) hints at a wider one;
a narrower set is the safe direction to be wrong in.
| Row | State | Note |
|---|---|---|
| `NP_QueueCastTimeSpells` | **LIVE** | Holds a press of a spell with a cast bar. Also the addon's presence probe |
| `NP_QueueInstantSpells` | **LIVE** | The same for an instant |
| `NP_QueueChannelingSpells` | **LIVE** | Whether a press made while a channel runs is held |
| `NP_QueueTargetingSpells` | **LIVE** | The terrain-cursor class, named by `cast_target::wants_terrain_click` — implicit target 16, or a location word with no unit bit. Deliberately *not* the item/locked/GameObject cursors, which want a click on a thing rather than a patch of ground |
| `NP_QueueOnSwingSpells` | **LIVE** | Default 0, as upstream's is "changed with 1.17.2 due to changes to on swing spells" |
| `NP_SpellQueueWindowMs` / `ChannelQueueWindowMs` / `CooldownQueueWindowMs` / `TargetingQueueWindowMs` | **LIVE** | See "What a window measures" above. The general one covers both a cast in flight and a global cooldown, which is what its unqualified name can mean |
| `NP_MinBufferTimeMs`, `NP_NonGcdBufferTimeMs` | **LIVE** | The guard's reopening margin |
| `NP_InterruptChannelsOutsideQueueWindow` | **LIVE** | A press while a channel has more than `NP_ChannelQueueWindowMs` left ends the channel instead of being held. Off by default |
| `NP_ReplaceMatchingNonGcdCategory` | **LIVE** | On, a non-GCD press takes a held slot only when the two share a `Spell.dbc` category — and a category of 0 means "no shared cooldown", so two of those do not match. Off by default, which is the plain always-replace |
| `NP_RetryServerRejectedSpells` | **LIVE** | Above |
| `NP_MaxBufferIncreaseMs`, `NP_OptimizeBufferUsingPacketTimings` | stored | Upstream's *dynamic* tuning of the buffer from observed packet timings. **Off in its own defaults**, so nothing changes by not having it; building it means a model of this realm's jitter, which is measurement rather than a binding |
| `NP_OnSwingBufferCooldownMs` | stored | A rate limit on re-queueing an on-swing strike. The slot is live; what is missing is anything to rate-limit, since a re-press of the queued strike already takes the reference's own silent bail (`6e4d43`) and so cannot queue twice |
| `NP_ChannelLatencyReductionPercentage` | stored | Trims a percentage of latency off a channel's end. The channel's true end is now kept apart from its slack (`ActiveChannel::ends_at`), so the input exists; what it needs is the measured latency in the cast path, which lives behind `PingShared` and is read only by the net-stats feed today |
**One behaviour worth knowing before testing.** Pressing a spell during a long cast holds it for the
whole cast and fires it at the end — the last press wins, so pressing three different spells during
a Pyroblast fires the third. That is upstream's `Replacing queued nonGcd spell X with Y` applied to
presses, and it is what "queue the next spell" means in practice. What it is *not* is a queue of
several casts in a row.
### The event table, accounted for
@@ -1049,7 +1125,8 @@ reason is the same mistake the function list started from: the first pass took t
| `NP_ChatBubblesWhisper` / `Raid` / `Battleground` | **LIVE** | Three kinds the reference bubbles never. `Raid` is the raid-**leader** and **warning** kinds, which is upstream's own wording — reading it as "raid chat" would bubble every `/raid` line |
| `NP_EnableLocalSetRaidTarget` | **LIVE** | Solo, the wire leg marks nothing at all: the server answers by echoing to a group that is not there. This switch routes it to the local board instead — the path SuperWoW's `"local"` argument already built |
| `NP_EnableSpellEnergizeEvents` | **LIVE** | The seventh `NP_Enable*`; nothing installed writes it, which is why it was missed |
| `NP_SpamProtectionEnabled`, `NP_QuickcastOnDoubleCast` | stored | The press-interception half of the queue, which is a separate decision |
| `NP_SpamProtectionEnabled` | **LIVE** | A repeat press of the cast already in flight is swallowed rather than held — which is also the reference's own silent bail (`6e4d43`), so the two agree. Off, such a press may replace itself in its slot |
| `NP_QuickcastOnDoubleCast`, `NP_QuickcastTargetingSpells`, `NP_DoubleCastToEndChannelEarly` | stored | The double-press family, **all three off in upstream's own defaults**. Each needs the world point under the mouse *at press time*, which this client resolves in the click path (`spell/targeting/world.rs`) and not in the cast path, so wiring them means handing the ladder a camera ray — a real change whose feel wants its own test |
| `NP_PreventRightClickPvPAttack` | stored | Needs a hook in the right-click interact path, which decides target, interact and attack in one pass; its sibling `NP_PreventRightClickTargetChange` is stored for the same reason |
| `NP_PreventMountingWhenBuffCapped` | **live** | A mount press with all 32 positive aura slots taken is refused above the commit, so neither the packet nor the global cooldown is spent — `spell/np_press.rs`'s `mount_refusal`. The aura number (`SPELL_AURA_MOUNTED` = 78) is asserted against this realm's own `Spell.dbc` for three mount spells rather than taken from mangos' header, because a wrong one would make the refusal silently never fire. **One deliberate 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 re-casting the mount — so a press while `UNIT_FIELD_MOUNTDISPLAYID` is live is exempt here. Refused **silently**, as upstream's own report is a line in its debug log and 1.12's `SpellCastResult` has no code for "too many auras" |
| `NP_EnableEnhancedTooltips` | stored | Authored content rather than a binding: the numbers are in `Spell.dbc` columns this client reads, but what the lines say and where they sit is upstream's design, and guessing it produces a tooltip that is neither ours nor theirs |