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

app+ui: the trinket cooldown — and a stock verb that always said "cold"

Browse Source 
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
This commit is contained in:
grom
2026-10-24 10:21:31 -06:00
co-authored by Claude Opus 5
parent f06caa9968
commit f87ef148b8
16 changed files with 835 additions and 27 deletions
Download Patch File Download Diff File Expand all files Collapse all files
docs/ADDON_COMPAT.md
+51 -6
View File
@@ -476,7 +476,7 @@ real when something else agrees — an addon calling it, or the mod's own docs.
What it found that this file did not already have: **38 names in `nampower.dll` that grom's addons
call**, on top of the families the nampower section above already tracks. Unported, and listed
rather than guessed at — `CancelPlayerAuraSlot`, `CancelPlayerAuraSpellId`,
rather than guessed at — `CancelPlayerAuraSlot`, `CancelPlayerAuraSpellId`, `CombatLogFlush`,
`CastSpellByNameNoQueue`, `CastSpellNoQueue`, `ChannelStopCastingNextTick`, `CombatLogFlush`,
`CustomFileExists`, `EncryptPassword`, `ExecuteCustomLuaFile`, `FindPlayerItemSlot`, `GetBagItem`,
`GetCastInfo`, `GetCurrentCastingInfo`, `GetItemIconTexture`, `GetItemIdCooldown`,
@@ -573,13 +573,11 @@ 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 (4).**
**Needs a feed this client does not have, with what it would take (2).**
| Name | What is missing |
|---|---|
| `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` |
| `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 |
| `GetTrinketCooldown()` | Equipped-slot cooldowns. `container_cooldowns` is keyed `(bag, slot)`; the equipped slots are not in it |
**Not ours to add (3).** `QueueSpellByName`, `QueueScript` and `ChannelStopCastingNextTick` are the
spell queue itself, which this client deliberately does not have (see the nampower rows above:
@@ -763,6 +761,53 @@ runs — still answers a learnt spell's name and rank, which is what `GetSpellNa
beside it does from the same `spell_display`. Every other column is a DBC row and has nowhere else
to come from.
### The trinket cooldown — and a stock verb that had been answering "cold" since it was written
**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** — the shape was right and the number was 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 too, off the same column, and the test runs
`CooldownFrame_SetTimer`'s own arithmetic against it.
The feed is `ui_char::slot_view`, keyed on the (use spell, item entry) pair exactly as the bag
feed's is — `Cooldowns::info` matches an item record on both — so one trinket reads the same
cooldown from the doll and from a bag.
**The risk in that change was the gate, and the first test for it was vacuous.** `feed_char` is
behind a change gate, and no descriptor moves when a cooldown is armed, so `Cooldowns::feed_epoch`
had to become a gate input or the swirl would appear only on the next frame something unrelated
changed. 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 through the schedule (`add_systems` + `update`), or it proves
nothing. Rewritten that way, it fails with the input removed — verified by probe.
Two also worth recording:
- **`GetSpellRangeData(rangeIndex)` answers three values, not a table.** Every table-returning verb
in this DLL has its key names in the string pool; this row's are not there, so a flat row is what
it answers. `flags` bit 0 is the melee family, where the engine takes the combat-reach sum
instead of the authored pair — a caller that ignores the bit reads 0..5 for every melee ability,
which is why the binding says so.
- **`CombatLogFlush()` is a no-op, deliberately defined.** Upstream buffers its combat-log writes
and this empties the buffer, so what an addon asks for is "my lines are on disk now".
`ui_chat/logging.rs` writes each line to an **unbuffered** `std::fs::File` as the window shows
it, so that postcondition already holds at all times. Defining it is the honest answer: the
capability is present, and a missing global makes `HasFeature` report it absent when it is not.
**Still out, now with the field lists that make each a decision rather than a shrug:**
| Name | Why not, and what it would take |
|---|---|
| `GetUnitData(unitToken, [copy])` / `GetUnitField`'s full surface | The names are read: 80 of them at `0x92620` — `charm summon charmedBy summonedBy createdBy persuaded channelObject health power1..5 maxHealth maxPower1..5 level factionTemplate bytes0 auraState baseAttackTime … aura auraFlags auraLevels auraApplications powerCostModifier powerCostMultiplier`, plus five computed extras (`guid status zoneId posX posY`). They are the **raw descriptor**, which lives app-side in `ObjectStore` and not in the VM's resolved `UnitState`, so the full surface is a live per-unit feed. `GetUnitField` already answers the only two names anything reads (`aura`, `auraApplications`) and `GetUnitData` has **zero** corpus call sites, so the feed would be built for nothing. The partial alternative is worse: a caller reading `bytes0` and getting nil while `health` works cannot tell which names are real |
| `GetRaidTargets()` | The reference keeps a raid-target **board** (`0xb712a8`); this client feeds the mark per unit (`UnitState::raid_target`), which `GetRaidTargetIndex` answers from. A scan over currently-named tokens would silently omit a marked unit no token names — plausible and wrong, which is worse than absent. Zero corpus call sites |
| `DisenchantAll(itemIdOrName \| quality, [includeSoulbound])` | **Queue machinery, not a getter.** It picks the next matching bag item, casts Disenchant, waits for the resolution, repeats — a server-paced sequencer with aborts on failure and on a bag change, which is the same thing `QueueSpellByName` is and which this client deliberately does not have. pfUI binds `/disenchantall` to it (`modules/nampower.lua:140`), so the *user-visible* cost is one slash command. Say the word and the sequencer is its own piece of work; it is not a binding |
| `GetSpellPower(school)` | The player's spell damage bonus by school, which needs the `PLAYER_FIELD_MOD_DAMAGE_DONE_POS` band fed per school. Nothing in the corpus calls it — the "7 call sites" an earlier sweep counted are BetterCharacterStats' own method of the same name |
| `StartItemExport` | Writes a file of its own; it belongs with the seven withheld, not with the getters |
### The whole registered list, accounted for
With the registration table read (the section above), the surface is no longer a count of what a
@@ -779,10 +824,10 @@ comm -23 /tmp/np /tmp/ours
| Bucket | Count | Names |
|---|---|---|
| **Built** | 38 | `CancelPlayerAuraSlot`, `CancelPlayerAuraSpellId`, `CastSpellByName`, `CastSpellByNameNoQueue`, `CastSpellNoQueue`, `FindPlayerItemSlot`, `GetBagItem`, `GetBagItems`, `GetEquippedItem`, `GetEquippedItems`, `GetItemIconTexture`, `GetItemIdCooldown`, `GetItemLevel`, `GetItemStats`, `GetItemStatsField`, `GetNampowerVersion`, `GetPlayerAuraDuration`, `GetSpellDuration`, `GetSpellIconTexture`, `GetSpellIdCooldown`, `GetSpellIdForName`, `GetSpellModifiers`, `GetSpellNameAndRankForId`, `GetSpellRec`, `GetSpellRecField`, `GetSpellSlotTypeIdForName`, `GetTrinkets`, `GetUnitField`, `GetUnitGUID`, `IsAuraHidden`, `IsSpellInRange`, `IsSpellUsable`, `PlayerIsMoving`, `PlayerIsRooted`, `PlayerIsSwimming`, `SetMouseoverUnit`, `UseItemIdOrName` |
| **Built** | 45 | `CancelPlayerAuraSlot`, `CancelPlayerAuraSpellId`, `CastSpellByName`, `CastSpellByNameNoQueue`, `CastSpellNoQueue`, `FindPlayerItemSlot`, `GetBagItem`, `GetBagItems`, `GetEquippedItem`, `GetEquippedItems`, `GetItemIconTexture`, `GetItemIdCooldown`, `GetItemLevel`, `GetItemStats`, `GetItemStatsField`, `GetNampowerVersion`, `GetPlayerAuraDuration`, `GetQuestDialogQuestId`, `GetQuestLogQuestIds`, `GetSpellDuration`, `GetSpellIconTexture`, `GetSpellIdCooldown`, `GetSpellIdForName`, `GetSpellModifiers`, `GetSpellNameAndRankForId`, `GetSpellRangeData`, `GetSpellRec`, `GetSpellRecField`, `GetSpellSlotTypeIdForName`, `GetTrinketCooldown`, `GetTrinkets`, `GetUnitField`, `GetUnitGUID`, `IsAuraHidden`, `IsSpellInRange`, `IsSpellUsable`, `PlayerIsMoving`, `PlayerIsRooted`, `PlayerIsSwimming`, `SetLocalRaidTargetIndex`, `SetMouseoverUnit`, `UseItemIdOrName`, `UseTrinket` |
| **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** | 12 | `DisenchantAll` (5 call sites outside the API layer), then `GetSpellPower`, `GetTrinketCooldown`, `UseTrinket`, `GetUnitData`, `GetSpellRangeData`, `CombatLogFlush`, `GetRaidTargets`, `SetLocalRaidTargetIndex`, `GetQuestLogQuestIds`, `GetQuestDialogQuestId`, `StartItemExport` (0 each — present for completeness, not need) |
| **Measured out, with the reason at each** | 5 | `DisenchantAll` (queue machinery — see the table above), `GetUnitData`, `GetRaidTargets`, `GetSpellPower`, `StartItemExport` |
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.