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

app+ui: the player's own cast state, and SPELL_CAST_EVENT off the commit

Browse Source 
`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
This commit is contained in:
grom
2026-10-24 07:16:30 -06:00
co-authored by Claude Opus 5
parent aab2130c62
commit ecee994ab6
19 changed files with 1252 additions and 31 deletions
Download Patch File Download Diff File Expand all files Collapse all files
docs/ADDON_COMPAT.md
+50 -14
View File
@@ -573,12 +573,10 @@ config export, but they are a different kind of surface from a getter, and the r
client that did not have them yesterday is not to grow them silently. `CombatLogFlush` is here only
because it is in the same family — it would flush our own `WoWCombatLog.txt`, which is benign.
**Needs a feed this client does not have, with what it would take (6).**
**Needs a feed this client does not have, with what it would take (4).**
| Name | What is missing |
|---|---|
| `GetCastInfo()` | Its table is `castId, spellId, guid, castType, castStartS, castEndS, castRemainingMs, castDurationMs, gcdEndS, gcdRemainingMs` (`NampowerAPI.lua:1859`). `PendingCast` (`spell/inflight.rs`) carries a spell id, a deadline and a guard bit — no start instant, no channel state, no GCD end. The consumer **divides by these** (`API.GetGCDRemainingMs`), so a table of three real fields and seven zeros is worse than the nil its own fallback already handles correctly. Needs `SMSG_SPELL_START`'s cast time kept as a start/end pair, a channel flag, and the GCD end the action bar already computes but does not publish |
| `GetCurrentCastingInfo()` | `castId, visId, autoId, casting, channeling, onswing, autoattack`. Same gap: `model.casting` is one bool that explicitly excludes channels, and `visId`/`autoId`/`onswing` are queue state this client has none of |
| `GetSpellRangeData(rangeIndex)` | A `SpellRange.dbc` **row index**, per the DLL's own usage string — not a spell id, which is how this row read until the usage strings were compared against it. `IsSpellInRange` answers the *verdict* from the live usable map, never the yards, so the table is genuinely absent. One more probe beside `install_spell_extras_probes` |
| `GetSpellModifiers(spellId)` | `spell/mods.rs` stores the server's `SPELLMOD_*` cells but publishes no reader |
| `GetUnitData(unit)` | The whole descriptor as a table, where `GetUnitField` answers one field. Needs a decision about which fields: the real one returns GUID-typed columns (charm, summon, charmedBy) this client's field reader does not type |
@@ -591,6 +589,49 @@ index, rank)` is left out for a narrower reason: `LearnTalent` here learns the *
reaching a named rank means queueing the pair once per missing rank, and whether the server accepts
that burst is not something this client can check before sending.
### The cast feed — `GetCurrentCastingInfo`, `GetCastInfo` and `SPELL_CAST_EVENT`
**Built**, and the reason this pair came first is measurement rather than taste: they are the
most-called nampower functions in grom's corpus (25 and 20 sites outside `NampowerAPI.lua`), and
one of those sites calls `GetCurrentCastingInfo` with **no existence guard at all** —
`local _,_,_,_,_,_,autoattack = GetCurrentCastingInfo()` (`SuperCleveRoidMacros/Core.lua:94`), in
the deferred-stop frame that keeps clearing the target while an autoattack is queued. On a client
without the function that line raises.
What the earlier "needs a feed" row said was true of `PendingCast` and wrong about the client as a
whole: this tree already folds a **cast window per caster** off the same packets (`UnitCastState`
in `ui_cast_events.rs`, behind `C_Spell.UnitCastingInfo`), with a start, an end, a channel bit and
the accumulated pushback. What was genuinely missing was the *caster's own machinery* — the
reference's cast globals — and the GCD, which `spell/cooldowns.rs` has run correctly all along and
never published.
| Piece | Where |
|---|---|
| `SelfCast`, eleven columns: the inflight spell `0xceca88`, the bar's spell `0xceac48`, the auto-repeat key `0xceac30`, four flags, the cast counter, the cast type, the target, and the GCD's start/end on the `GetTime()` clock | `benilla-ui`'s `script/model.rs` + `script/self_cast.rs` |
| The per-tick push, and `SPELL_CAST_EVENT` fired off the same commit | `benilla-app`'s `ui_self_cast.rs`, in `UnitFeed` after the cast-window drain so the two halves of `GetCastInfo`'s table cannot describe different casts |
| `Cooldowns::gcd` — the running global cooldown as one timer | `spell/cooldowns.rs`, the longest running GCD leg, which is the one a player is waiting on; `info` still answers the per-category question every cooldown swirl asks |
| `PendingCast`'s cast counter | `spell/inflight.rs`; the one place every class of commit passes through, which is why it is there and not in the send path |
| The commit rows (`castType`, target, item) | `spell/cast_send.rs`, at the two commits — the plain send and the targeting-cursor click, which is the only arm that produces `TARGETING`/`TARGETING_NON_GCD` |
Three decisions worth having written down, because each had a tempting wrong answer:
- **`GetCastInfo` is nil when nothing is casting, and an instant's GCD does not count.** The
shortcut is to answer a table whenever the GCD is up, so the consumer's fast path always hits.
But `API.IsCasting()` *is* `GetCastInfo() ~= nil` (`NampowerAPI.lua:1888`), and `[casting]` is a
macro conditional players gate abilities on — a table there would make it true for a second and
a half after every instant. The wrapper's own fallback comment says what to do instead
("`GetSpellIdCooldown` … works for instant casts").
- **`castId` means two different numbers under one upstream name.** In `GetCastInfo`'s table it is
a counter, beside a separate `spellId` column. In `GetCurrentCastingInfo`'s returns it is a
*spell id*, 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`). Both are implemented as
what their own consumers read.
- **`SPELL_CAST_EVENT` fires for commits, not for refusals.** Upstream's `success` column can be
0; this client's local refusals all funnel through `CastErrors`, which carries a spell id and a
reason byte and neither a cast type nor a target, so 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.
### The whole registered list, accounted for
With the registration table read (the section above), the surface is no longer a count of what a
@@ -607,18 +648,13 @@ comm -23 /tmp/np /tmp/ours
| Bucket | Count | Names |
|---|---|---|
| **Built** | 32 | `CancelPlayerAuraSlot`, `CancelPlayerAuraSpellId`, `CastSpellByName`, `CastSpellByNameNoQueue`, `CastSpellNoQueue`, `FindPlayerItemSlot`, `GetBagItem`, `GetEquippedItem`, `GetEquippedItems`, `GetItemIconTexture`, `GetItemIdCooldown`, `GetItemLevel`, `GetItemStats`, `GetItemStatsField`, `GetNampowerVersion`, `GetPlayerAuraDuration`, `GetSpellDuration`, `GetSpellIconTexture`, `GetSpellIdForName`, `GetSpellNameAndRankForId`, `GetSpellRecField`, `GetSpellSlotTypeIdForName`, `GetTrinkets`, `GetUnitField`, `GetUnitGUID`, `IsAuraHidden`, `IsSpellInRange`, `IsSpellUsable`, `PlayerIsMoving`, `PlayerIsRooted`, `PlayerIsSwimming`, `SetMouseoverUnit`, `UseItemIdOrName` |
| **Built** | 34 | `CancelPlayerAuraSlot`, `CancelPlayerAuraSpellId`, `CastSpellByName`, `CastSpellByNameNoQueue`, `CastSpellNoQueue`, `FindPlayerItemSlot`, `GetBagItem`, `GetEquippedItem`, `GetEquippedItems`, `GetItemIconTexture`, `GetItemIdCooldown`, `GetItemLevel`, `GetItemStats`, `GetItemStatsField`, `GetNampowerVersion`, `GetPlayerAuraDuration`, `GetSpellDuration`, `GetSpellIconTexture`, `GetSpellIdForName`, `GetSpellNameAndRankForId`, `GetSpellRecField`, `GetSpellSlotTypeIdForName`, `GetTrinkets`, `GetUnitField`, `GetUnitGUID`, `IsAuraHidden`, `IsSpellInRange`, `IsSpellUsable`, `PlayerIsMoving`, `PlayerIsRooted`, `PlayerIsSwimming`, `SetMouseoverUnit`, `UseItemIdOrName` |
| **Withheld pending grom's say-so** | 8 | `WriteCustomFile`, `ReadCustomFile`, `CustomFileExists`, `ImportFile`, `ExportFile`, `ExecuteCustomLuaFile`, `StartItemExport` (writes a file of its own), `EncryptPassword` + `EncryptedServerLogin` |
| **Not ours to add** | 4 | `QueueSpellByName`, `QueueScript`, `ChannelStopCastingNextTick`, `LearnTalentRank` |
| **To build, in corpus-call order** | 18 | `GetCurrentCastingInfo` (25 call sites outside the API layer), `GetCastInfo` (20), `GetSpellPower` (7), `GetBagItems` (7), `GetSpellModifiers` (6), `DisenchantAll` (5), `GetSpellIdCooldown` (3), then `GetSpellRec`, `GetTrinketCooldown`, `UseTrinket`, `GetUnitData`, `GetSpellRangeData`, `CombatLogFlush`, `GetRaidTargets`, `SetLocalRaidTargetIndex`, `GetQuestLogQuestIds`, `GetQuestDialogQuestId` (0 each — present for completeness, not need) |
| **To build, in corpus-call order** | 16 | `GetSpellPower` (7 call sites outside the API layer), `GetBagItems` (7), `GetSpellModifiers` (6), `DisenchantAll` (5), `GetSpellIdCooldown` (3), then `GetSpellRec`, `GetTrinketCooldown`, `UseTrinket`, `GetUnitData`, `GetSpellRangeData`, `CombatLogFlush`, `GetRaidTargets`, `SetLocalRaidTargetIndex`, `GetQuestLogQuestIds`, `GetQuestDialogQuestId` (0 each — present for completeness, not need) |
The two at the top of that last bucket are worth the ordering note: `GetCurrentCastingInfo` and
`GetCastInfo` are the **most-called nampower functions in the whole corpus**, and one of
`GetCurrentCastingInfo`'s sites (`SuperCleveRoidMacros/Core.lua:94`) calls it with no existence
guard at all, so on this client that path raises today. They are also coupled: `Core.lua:4899`
registers `SPELL_CAST_EVENT` **gated on `GetCurrentCastingInfo` existing**, so defining the function
without firing the event would move that addon from "never registered" to "registered and silent",
which is worse. Implement the pair with the event.
The two that were at the head of that last bucket are now built — see "The cast feed" above for
why they came first and what each decision cost.
### The event table, accounted for
@@ -629,9 +665,9 @@ from the consumer side — it destructures `arg2`=spellId, `arg3`=casterGuid, `a
| Bucket | Names |
|---|---|
| **Fired** | `BUFF_ADDED_SELF`, `BUFF_REMOVED_SELF`, `DEBUFF_ADDED_SELF`, `DEBUFF_REMOVED_SELF`, `DEBUFF_ADDED_OTHER`, `DEBUFF_REMOVED_OTHER`, `AURA_CAST_ON_SELF`, `AURA_CAST_ON_OTHER` (`ui_aura.rs`, `ui_cast_events.rs`) |
| **Fired** | `BUFF_ADDED_SELF`, `BUFF_REMOVED_SELF`, `DEBUFF_ADDED_SELF`, `DEBUFF_REMOVED_SELF`, `DEBUFF_ADDED_OTHER`, `DEBUFF_REMOVED_OTHER`, `AURA_CAST_ON_SELF`, `AURA_CAST_ON_OTHER` (`ui_aura.rs`, `ui_cast_events.rs`), `SPELL_CAST_EVENT` (`ui_self_cast.rs`) |
| **Deliberately absent** | `BUFF_ADDED_OTHER`/`BUFF_REMOVED_OTHER` — diffing the helpful half of every streamed unit, which nothing registers (the `AuraEdge` doc says so at the enum); `SPELL_QUEUE_EVENT` — the queue; `SPELL_DELAYED_OTHER` — upstream's own note is that the server only sends the packet to the affected player, so it never fires there either |
| **To fire, off packets already parsed** | `SPELL_START_SELF`/`_OTHER`, `SPELL_GO_SELF`/`_OTHER`, `SPELL_FAILED_SELF`/`_OTHER`, `SPELL_DELAYED_SELF`, `SPELL_CHANNEL_START`, `SPELL_CHANNEL_UPDATE`, `SPELL_CAST_EVENT` — all of these are edges `ui_cast_events.rs`'s one queue already carries for `UNIT_CASTEVENT`; what each needs beyond today's row is named in that file |
| **To fire, off packets already parsed** | `SPELL_START_SELF`/`_OTHER`, `SPELL_GO_SELF`/`_OTHER`, `SPELL_FAILED_SELF`/`_OTHER`, `SPELL_DELAYED_SELF`, `SPELL_CHANNEL_START`, `SPELL_CHANNEL_UPDATE` — all of these are edges `ui_cast_events.rs`'s one queue already carries for `UNIT_CASTEVENT`; what each needs beyond today's row is named in that file |
| **To fire, needs its own producer** | `UNIT_DIED`, the thirteen `UNIT_*_GUID`/`PLAYER_GUILD_UPDATE_GUID` rows (one shape: `guid, isPlayer, isTarget, isMouseover, isPet, partyIndex, raidIndex`, gated by the `NP_EnableUnitEvents*` CVars already registered), `UNIT_COMBAT_GUID`, `SPELL_DAMAGE_EVENT_SELF`/`_OTHER`, `SPELL_HEAL_BY_SELF`/`_BY_OTHER`/`_ON_SELF`, `SPELL_ENERGIZE_*`, `SPELL_MISS_SELF`/`_OTHER`, `SPELL_DISPEL_BY_*`, `DAMAGE_SHIELD_*`, `ENVIRONMENTAL_DMG_*`, `AUTO_ATTACK_SELF`/`_OTHER`, `BUFF_UPDATE_DURATION_SELF`, `DEBUFF_UPDATE_DURATION_SELF`, `KEY_DOWN`/`KEY_UP` |