Commit Graph
11 Commits
Author SHA1 Message Date
gromandClaude Opus 5 ed92574cea tooltip: SetOwner hides the plate, so a rebuild gives the addon its OnShow back
Reported from play with a video: hovering a bag item draws the enhanced tooltip
— MasterTradeSkills' recipe block, AdvancedTradeSkillWindow2's Bags/Bank/Total
— for about nine frames of sixty, then the plate falls back to the bare one and
stays bare while the cursor sits still.

The loop is stock. `ContainerFrameItemButton_OnUpdate` re-enters `..._OnEnter`
every `TOOLTIP_UPDATE_TIME` (`ContainerFrame.lua:693` in his own patch-4.mpq;
pfUI's bag slots are `ContainerFrameItemButtonTemplate`, so they run it too),
and each turn is `SetOwner` + `SetBagItem` — a clear that wipes every line an
addon appended. What puts them back in the reference is a hide/show pair, and
we had neither half: the `SetOwner` binding hides the plate before it even reads
the owner argument (`0x531179` zeroes the content gate at `+0xd0`, `0x531183`
calls `[vtable+0x84]` = `0x530a60`, which is the `SetOwner(NULL)` core plus the
base hide `0x76ad50`), and the content `Set*` re-shows it through the tooltip's
own Show override (`0x530a80`). So every rebuild raises `OnHide` then `OnShow`.
Ours cleared in place, and the base show early-outs on an already-visible frame
(`0x76ae3b`), so the plate never hid and the `OnShow` never came.

That is why exactly two addons lost their lines while others kept theirs:
`LibItemTooltip-1.0`, the tooltip contract of MasterTradeSkills and
Gromissingcrafts, hangs entirely on a scratch child frame of `GameTooltip`
listening for `OnShow`, where pfUI's ItemID and sell-price lines hook the `Set*`
methods themselves and so re-run on every refresh.

The reference also clears twice per `SetOwner` (once inside the hide, once in
the core) and so fires `OnTooltipCleared` twice; that falls out of calling the
same hide, and the existing lifecycle test already allowed for it.

Addresses read off the player's own `WoW.exe` with objdump, not from memory.
The new test is the hover loop in miniature — three turns of `SetOwner` + a
content `Set*` under a `LibItemTooltip`-shaped child probe — and fails on the
old behaviour with one `OnShow` instead of three.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn
2026-10-17 15:38:36 -06:00
gromandClaude Opus 5 1287c328e0 creature_anim: the loot sparkle's switch starts where its CVar does
Reported from play: no sparkle on a mining vein, while lootable mobs sparkled
fine. That split is the diagnosis — only the GameObject arm reads the switch.

`LootSparkleMode` derived `Default`, so it booted `false` while its row
registered `"1"`. A boot fires `CvarChanged` only for rows `config.toml`
actually carries, and writing `"1"` over a value already `"1"` is a no-op that
fires nothing either, so nothing could ever tell the resource its real default —
not the boot, and not the player toggling the CVar. The feature was inert for
everyone who had never written that row.

`vplates.rs`'s `VPlateMode::default` carries this warning in as many words,
from `NameplateOcclusion` shipping inert the same way, and this is the same
mistake one feature later. So: a hand-written `Default`, and the twin of that
module's test, reading the registered row rather than restating it. Swept the
rest — of the 32 resources a `CvarChanged` observer writes, 6 derive `Default`
and every one of their rows registers `"0"`, so this was the only one.

The doc row kept two stale claims and both are corrected: the test has had two
arms (the activate bit *or* `GAMEOBJECT_TYPE_CHEST`) since it was widened, and
`FxStage` is immaterial here — `Particles\LootFX.m2`, the `"HARDCODED Loot Art"`
row's own model, holds one 2400 ms sequence with `flags = 0`, and bit 0 clear
loops, so `FxAnimLife::arm` repeats it whatever the stage says.

Still unverified in world: whether veins, herbs and chests sparkle, and whether
anything sparkles that should not.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn
2026-10-17 13:41:47 -06:00
gromandClaude Opus 5 aeba81ec2e creature_anim: SuperWoW's LootSparkle, the sparkle on lootable treasure
1.12 sparkles a lootable corpse and nothing else — a chest, a herb, a vein or a
quest object is found by eye. Sparkling those arrived in 2.3.0 ("interactable
game objects sparkle and display their names") and SuperWoW backports it as
`LootSparkle`, which upstream describes as "the sparkling effect on lootable
treasure". So on this install the sparkle appears on mobs and not on objects,
which is what was reported.

`arm_loot_fx` grows a GameObject arm beside its unit and corpse ones, hanging the
same "HARDCODED Loot Art" row (0x13) at the same tag, driven by the same field
edges. The three-way decision is a named predicate with a test, because a corpse
falling into the unit arm would read its CORPSE_* bytes as a live unit's, and
because the switch being off has to answer *exactly* 1.12 rather than
approximately it.

The object test has two arms, because the mod's own phrase is "lootable treasure"
and neither arm alone is that:

- `GO_DYNFLAG_LO_ACTIVATE`, the server's per-viewer bit, documented on its own
  field reader as "the sparkle". But vmangos raises it through
  `GameObject::ActivateToQuest`, so on a chest it means *quest* loot: an ordinary
  chest, herb or vein need never carry it, and keying on it alone risks lighting
  up nothing but quest objects.
- `GAMEOBJECT_TYPE_CHEST`, which is exactly chests, herbs and mining nodes —
  treasure by construction, and a type the client reads without the server's
  help. Excluded when `GO_FLAG_IN_USE` or `NO_INTERACT` is set, so an emptied or
  disabled chest stops sparkling.

Both arms are a judgement about what the mod does rather than a trace of it —
there is no source and no published default — so they are written to be read and
corrected, and the predicate's own doc says which symptom indicts which arm: only
quest objects lighting up means the first is doing all the work, doors and signs
lighting up means the second is too wide.

Defaulted on: upstream publishes no default, and the sparkle is present on this
install with SuperWoW loaded, which is what "as if its own default config loaded"
means for this compat layer. `SetCVar("LootSparkle", "0")` is stock 1.12.

**Not verified in world.** The harness has no scene, so the test covers the
decision and not the rendering.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn
2026-10-17 09:04:37 -06:00
gromandClaude Opus 5 0515274afc tests: four stale expectations this window's own commits left behind
Four benilla-ui tests were red and had been since the commits that changed what
they pin. They were missed because the round's gate was run as `cargo test -p
benilla-app -p benilla-ui` through a grep that only showed benilla-app's results;
scripts/check.sh is the gate that scopes this correctly.

- SetRaidTarget's third argument became a flag rather than the literal word
  "local" (7098d642), so the test asserting the word is the only form now
  states the contract it replaced: any truthy value is the local mark, which is
  what SoloRaidTargetIcons passes.
- GetStringHeight is installed deliberately (75f04b4f), so the regions surface
  check that asserted its absence had to flip.
- The LOCALIZED_CLASS_NAMES pair (0a907955) was published only once class data
  was fed, leaving the globals absent on a bare VM — which the
  allowed-beyond-1.12 list said we exposed and the creature_info test asserted
  we did not, while its own comment described the behaviour we wanted. Both
  tables are now seeded empty at install, as ClassicAPI publishes them at load:
  tInvert(nil) raises where tInvert({}) is an empty map, and pfUI inverts one in
  its own file scope.
- The AceGUI label test's Lua used a one-argument SetPoint, which 1.12 correctly
  raises Usage: on (LUA_TNONE is not LUA_TNIL — script/tests/anchors.rs pins it,
  and the 1.12 AceGUI backport had every one-argument call rewritten for exactly
  that reason).

And a correction in docs/ADDON_COMPAT.md that the last of those turned up:
GetStringHeight was credited with the empty VoiceOver options, on the strength of
AceGUIWidget-Label.lua's call site and a probe showing every Label at 624x1.
Both were wrong. AI_VoiceOver ships four copies of AceGUI and its TOC loads
1.12\embeds.xml — and the 1.12 tree reads label:GetHeight(), never
GetStringHeight. The 624x1 was the harness's own missing text measurer, the trap
its notes already warn about. The method stays as ClassicAPI parity surface with
the claim corrected; the real cause of that symptom is the scroll-child anchor,
fixed in 75432f7d. The lesson, written into the row: an addon shipping several
library copies has to be read through its own TOC chain before any call site in
it is evidence.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn
2026-10-17 07:56:52 -06:00
gromandClaude Opus 5 80648267d5 ui_aura: another unit's aura durations, learned from the GO that applies them
The target frame's debuff icons had no countdown. libdebuff was tracking fine —
the probe run said `libdebuff guids: 2` after a fight, and the AURA_CAST_ON_OTHER
events carried the right lengths (spell 772 Rend, dur 9000) — because the timers
pfUI-for-ClassicAPI draws do not come through libdebuff at all. Its unit frames
read duration/expirationTime straight off C_UnitAuras.UnitAuraBySlot
(api/unitframes.lua:1680,1788), and libdebuff's own header says the plumbing
"now runs on ClassicAPI's C_UnitAuras (which provides sourceUnit/sourceGUID and
non-player expirationTime)". So the number came from us, and we answered 0 for
every unit but the player.

There is no packet to answer from: SMSG_UPDATE_AURA_DURATION is self-only, and
SMSG_SET_EXTRA_AURA_INFO — which carries a caster's durations on others — is a
2.0 opcode this protocol does not have. Hence every vanilla duration addon
keeping its own table. This is that table, kept client-side instead, because
this client *is* the client: SMSG_SPELL_GO names the spell and its hit list, so
the apply instant is the packet's own arrival rather than a combat-log line
somebody has to parse, and every reader of the aura lists gets the timer at once
instead of each inventing a tracker.

Keyed by (guid, spell) so a re-cast refreshes in place and a recycled slot
cannot inherit a stale stamp. Length comes from the same
ui_cast_events::aura_duration_ms behind nampower's arg8 and GetSpellDuration, so
the three cannot disagree, and its 0 — permanent, no SpellDuration.dbc row, or a
combo-point/level-scaled one — stamps nothing: a timerless icon beats one
counting to the wrong moment. The join is driven by the unit's own
UNIT_FIELD_AURA, so a stamp for an aura the target resisted is never read, and a
run-out stamp reports nothing rather than a negative remainder.

Not covered, by construction: an aura already on a unit before we streamed it,
and one applied out of our range. No GO, no timer — which is what the mods this
stands in for also do.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn
2026-10-17 07:56:35 -06:00
gromandClaude Opus 5 3d4c242c09 sound: decode addon .ogg, which is most of the addon audio on this install
unitscan's rare alert was reported as silent. The binding and the loose-file read were both
already right: kira was built without the `ogg` container and `vorbis` codec features, so
`sfx_from_bytes` returned an error for every Vorbis file and `sound/ui.rs` logged it at `debug!`.
A silent alert with nothing in the log to say why.

It is not one horn. **75 `.ogg` files ship with the installed addons** — DoiteAuras 57, BigWigs
11, unitscan 3, HCWarn 2, TWThreat and Cleanup one each — so every alert any of them raises by
`PlaySoundFile` was dead, HCWarn's hardcore death warnings included. AI_VoiceOver ships `.mp3`
and always played, which is exactly what made this look like an addon-specific fault rather than
a missing decoder.

Stock 1.12 plays wav and mp3 only. The deviation is from 1.12 and toward the client the player
actually runs: these are Turtle/OctoWoW addons shipping ogg because that client plays it.
`docs/ADDON_COMPAT.md` carries the row.

The test decodes a real `.ogg` out of the player's own addon folder and skips when there is none,
the same gate the chain-backed tests use for a WoW install — rather than carrying someone else's
audio into the tree as a fixture.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn
2026-10-16 18:45:25 -06:00
gromandClaude Opus 5 0a90795555 compat: publish LOCALIZED_CLASS_NAMES_MALE/_FEMALE, the table tInvert exists to flip
Half this pair was already here. `ClassInfo::class_file`'s own doc says the token is what
FrameXML keys `RAID_CLASS_COLORS` and `LOCALIZED_CLASS_NAMES_MALE` on, and
`compat/classic_api/lua_ext.rs`'s `tInvert` names pfUI's `pfUI.lua:227` as the reason it exists —
the line that inverts exactly this table. The table itself was never published, so that line
inverted `nil`.

Measured against the live chain: `pfUI.env.L["class"]` holds **0 entries**, and
`L["class"]["Warrior"]` is nil. Every lookup through it then finds no class and skips, silently,
because the consumer tests `if class then` — so the guild roster lost its name colours and class
icons (`modules/socialmod.lua:18`), the friends list lost the same, and `modules/panel.lua:359`
fell back to white. Chat stayed coloured throughout, which is what made it look like a pfUI
setting rather than a missing global.

Published from the class table the app already feeds for `C_CreatureInfo.GetClassInfo`, at that
same feed, so there is one source for the names and no second reader of the DBC. Both tables
carry the same string: 1.12's `ChrClasses.dbc` has one name column, so the gendered pair later
clients ship cannot be derived from this data — a copy is what it honestly supports, and omitting
`_FEMALE` would hand a caller nil where its sibling answers. A run that never feeds the table
leaves both absent rather than empty.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn
2026-10-16 18:45:25 -06:00
gromandClaude Opus 5 751cac062b compat: C_Spell.GetSpellEffectMechanics, the per-effect sibling
`SuperCleveRoidMacros/ClassicAPI.lua:267` calls it unguarded, so its absence was a raise a
session. It is not a second spelling of `GetSpellMechanicByID`, which this tree already answers:
that one reads `Spell.dbc`'s spell-level `Mechanic`, and vanilla stores some mechanics on an
*effect* instead. The consumer's own comment names the case — Rake's bleed is effect-level, so
the spell-level column says 0 while this answers `{0, 15, 0}` — and a macro conditional asking
whether its target is bleeding gets the wrong answer from the spell-level field alone.

`SpellDisplay::effect_mechanic` was already parsed for the cast-immunity scan, so this is a field
on the probe payload and a reader, not a new lookup. The three-way answer is its sibling's, for
the sibling's reason: nothing at all for an id no spell owns, three zeros for a real spell that
carries no mechanic anywhere, the ids otherwise — collapsing those two would reclassify crowd
control as ordinary damage, which is the hand-kept table this function exists to replace.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn
2026-10-16 18:45:25 -06:00
gromandClaude Opus 5 06011e5e5b ui_script: the LoadOnDemand log line stops claiming "not loaded"
A LoadOnDemand addon can already be in memory when the walk reaches its row, because an addon
ahead of it loaded it from its own `ADDON_LOADED` — which is how every AI_VoiceOver data module
actually arrives: 224 ms of Lua inside AI_VoiceOver's handler, before the walk gets to the
`AI_VoiceOverData_*` rows at all. The line was printed unconditionally, so it went on saying "no
LoadAddOn() yet" about addons that had loaded. That reads as a bug and is not one; it cost a
session chasing a missing `GetAddOnEnableState` that was never the problem. Gated on a new
`UiScript::addon_is_loaded`.

`docs/ADDON_COMPAT.md` now records why that global stays unbuilt, since the evidence was gathered
anyway: upstream ClassicAPI's own `docs/API.md` does not define it either, and AI_VoiceOver gates a
modern-shaped `GetAddOnInfo` on the same `if not GetAddOnEnableState`, so defining it would leave
`VoiceOver.lua:84` reading `loadable` where it means `reason` and pop the "delete your old folder"
dialog on every login.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn
2026-10-16 18:45:25 -06:00
gromandClaude Opus 5 75f04b4f28 compat: FontString:GetStringHeight, which AceGUI calls unguarded
1.12 registers no `GetStringHeight` (`script/button.rs`: `0x782390` reads the label's height
instead) and ClassicAPI backports it, so `compat/classic_api/frame.rs` is its home. That file had
it on its own not-built list under "needs per-line layout facts the host measurer is not asked for
today" — true of `GetNumLines` and `GetWrappedWidth`, and not of this one: the whole-string height
is the other half of the number `GetStringWidth` already reads. `measure::current_measure` now owns
the staleness rule for both, so the two cannot disagree about when a measure has gone stale, and a
changed string still reads 0 rather than the previous string's height.

It is not cosmetic. AceGUI-3.0 calls it **unguarded** — `AceGUIWidget-Label.lua:54` is
`height = label:GetStringHeight()` and `AceGUIWidget-CheckBox.lua:90` is
`self:SetHeight(28 + self.desc:GetStringHeight())` — so every Ace options panel on this install
drew its labels and checkbox descriptions at the library's own "avoid zero-height labels" fallback
of one pixel: a window with all of its prose invisible. `addon_harness --probe AI_VoiceOver` builds
that panel's full 32-widget tree with zero errors and every `Label` at `624x1`, which is what
"can't see the actual options in VoiceOver" was.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn
2026-10-16 18:45:25 -06:00
gromandClaude Opus 5 81d96af35d Everwood: the HD/OctoWoW line of benilla, imported onto upstream
This checkout had no git history at all, so this one commit carries the whole
of the Everwood work as a single import on top of chairmanpow/benilla-octo's
`main` (706cd71). The two trees have genuinely diverged — each has files the
other does not — so this is a fork point, not a catch-up.

Today's work, which is the reason a baseline exists at all:

- Booth previews no longer cast world shadows (`NO_BOOTH_SHADOW`). Portrait
  characters were entering the sun's shadow cascades, so every skinned-rig
  shadow specialization compiled live mid-frame; the hitch detector logged
  "main thread blocked 1316 ms" beside a `compiled LIVE` warning. This was
  the Fashion/Tmog stutter.
- Per-pane dress-up: each `DressUpModel` pane assembles and stages its own
  tried-on look instead of every pane mirroring the live player.
- Item template cache is append-only JSONL written on the IO pool, replacing
  an 11.6 MB whole-map JSON rewrite on the main thread every 10 s.
- `WOW_SYSTEM_TIMING` rebuilt per-thread: it took 2 mutexes per system enter
  and 4 per exit, costing 15 ms a frame (590 fps -> 60) and corrupting the
  measurements it was taken to produce.
- The visibility-churn sweep moved to its own `WOW_FLASH_CHURN` switch, so a
  frame-cost reading no longer pays 2.7 ms a frame for it.

Local state (`benilla-config/`, `logs/`, `target/`) is excluded by the
existing .gitignore; the account credential it holds is not in this commit.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn
2026-10-15 15:01:14 -06:00