diff --git a/Cargo.lock b/Cargo.lock index 263685c4..fac2bf34 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6189,7 +6189,9 @@ dependencies = [ "symphonia-bundle-mp3", "symphonia-codec-adpcm", "symphonia-codec-pcm", + "symphonia-codec-vorbis", "symphonia-core", + "symphonia-format-ogg", "symphonia-format-riff", "symphonia-metadata", ] @@ -6226,6 +6228,17 @@ dependencies = [ "symphonia-core", ] +[[package]] +name = "symphonia-codec-vorbis" +version = "0.5.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f025837c309cd69ffef572750b4a2257b59552c5399a5e49707cc5b1b85d1c73" +dependencies = [ + "log", + "symphonia-core", + "symphonia-utils-xiph", +] + [[package]] name = "symphonia-core" version = "0.5.5" @@ -6239,6 +6252,18 @@ dependencies = [ "log", ] +[[package]] +name = "symphonia-format-ogg" +version = "0.5.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2b4955c67c1ed3aa8ae8428d04ca8397fbef6a19b2b051e73b5da8b1435639cb" +dependencies = [ + "log", + "symphonia-core", + "symphonia-metadata", + "symphonia-utils-xiph", +] + [[package]] name = "symphonia-format-riff" version = "0.5.5" @@ -6263,6 +6288,16 @@ dependencies = [ "symphonia-core", ] +[[package]] +name = "symphonia-utils-xiph" +version = "0.5.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ee27c85ab799a338446b68eec77abf42e1a6f1bb490656e121c6e27bfbab9f16" +dependencies = [ + "symphonia-core", + "symphonia-metadata", +] + [[package]] name = "syn" version = "2.0.117" diff --git a/crates/benilla-app/Cargo.toml b/crates/benilla-app/Cargo.toml index cfb2c44e..93bb60ca 100644 --- a/crates/benilla-app/Cargo.toml +++ b/crates/benilla-app/Cargo.toml @@ -54,7 +54,20 @@ cosmic-text = "0.16" # `cosmic-text`'s font database, named by `ui_text`; must stay the version it depends on. fontdb = "0.23" # The mixer. Its `cpal` backend stays off on every target: `sound::output` is our own backend. -kira = { version = "0.12", default-features = false, features = ["mp3", "wav", "pcm", "adpcm"] } +# `ogg`/`vorbis` are the container and the codec, and both are needed for one `.ogg`: addon audio +# on this install is full of them (75 files across BigWigs, DoiteAuras, unitscan, HCWarn, +# TWThreat and Cleanup), and `PlaySoundFile` on one decoded to an error nobody saw — the play +# site logs at `debug!` — so every one of those alerts was simply silent. Stock 1.12 plays only +# wav and mp3; the Turtle/OctoWoW client those addons are written for plays ogg, which is why +# they ship it. `docs/ADDON_COMPAT.md` carries the row. +kira = { version = "0.12", default-features = false, features = [ + "mp3", + "wav", + "pcm", + "adpcm", + "ogg", + "vorbis", +] } # Aborts a debug build on an allocation in the audio IO callback or render pass; off in release. assert_no_alloc = "1.1" # `MediaSource`: our source raises kira's decode thread's QoS; must stay kira's version. diff --git a/crates/benilla-app/src/sound/mixer.rs b/crates/benilla-app/src/sound/mixer.rs index 75460f1d..ab9ef67c 100644 --- a/crates/benilla-app/src/sound/mixer.rs +++ b/crates/benilla-app/src/sound/mixer.rs @@ -776,6 +776,61 @@ mod tests { } } + /// An addon's `.ogg` decodes. `PlaySoundFile` is how every addon alert plays, and on this + /// install 75 of those files are Vorbis — BigWigs, DoiteAuras, unitscan, HCWarn, TWThreat and + /// Cleanup — so with kira built without the `ogg`/`vorbis` pair they all failed in + /// `sfx_from_bytes` and went silent. The play site logs that at `debug!`, so nothing said so. + /// + /// Gated on a local file the way the chain-backed tests are gated on a WoW install: the + /// addon folder is the player's and is not in the tree, so elsewhere this skips rather than + /// carrying someone else's audio into the repo as a fixture. + #[test] + fn an_addons_ogg_decodes() { + let Some(root) = std::env::var_os("CARGO_MANIFEST_DIR").map(std::path::PathBuf::from) + else { + return; + }; + let addons = root + .parent() + .and_then(|p| p.parent()) + .map(|p| p.join("benilla-config/AddOns")); + let Some(addons) = addons.filter(|p| p.is_dir()) else { + eprintln!("skipping: no benilla-config/AddOns to read a .ogg from"); + return; + }; + // The first `.ogg` under the folder, whichever addon ships it. + fn find_ogg(dir: &std::path::Path, depth: u32) -> Option { + if depth == 0 { + return None; + } + let mut dirs = Vec::new(); + for entry in std::fs::read_dir(dir).ok()?.flatten() { + let path = entry.path(); + if path.is_dir() { + dirs.push(path); + } else if path + .extension() + .is_some_and(|e| e.eq_ignore_ascii_case("ogg")) + { + return Some(path); + } + } + dirs.into_iter().find_map(|d| find_ogg(&d, depth - 1)) + } + let Some(ogg) = find_ogg(&addons, 4) else { + eprintln!("skipping: no .ogg under {}", addons.display()); + return; + }; + let bytes = std::fs::read(&ogg).expect("the file reads"); + let data = + sfx_from_bytes(bytes).unwrap_or_else(|e| panic!("decoding {}: {e:#}", ogg.display())); + assert!( + data.duration() > std::time::Duration::ZERO, + "{} decoded to no audio", + ogg.display() + ); + } + #[test] fn freeverb_projection_orders_the_presets() { // PRESET_GENERIC / PRESET_CAVE / PRESET_HANGAR: decay 1.49 / 3.0 / 10.05 s. diff --git a/docs/ADDON_COMPAT.md b/docs/ADDON_COMPAT.md index 6ab0bbe1..20c40c33 100644 --- a/docs/ADDON_COMPAT.md +++ b/docs/ADDON_COMPAT.md @@ -388,6 +388,7 @@ with the load checked after each. | Secure-attribute click dispatch (`type1`/`type2`/click-cast) | **built** | `SetAttribute`/`GetAttribute` were built as a bare key/value store, with this file's own header calling that "honest" about not claiming modern WoW's restricted-execution subsystem. The line was in the wrong place: a frame that stores attributes and does nothing when clicked is not a documented limitation, it is an inert frame. **pfUI drives every one of its unit frames this way whenever Clique is absent** — which is the default — setting `type1="target"`, `type2="menu"` and `EnableClickCast()` and installing **no `OnClick` script at all** (`api/unitframes.lua:1320-1326`). So the player frame could not be clicked to target, right-clicked for its menu, or used for click-casting, and neither could any other unit frame. Dispatched in `frame.rs`'s `dispatch_attribute_click`, called from `button.rs` **after** the `OnClick` handler for the same reason the nameplate selection is — a frame may have both and a raising hook must not eat the action. Handles `target`, `menu` (through `ClassicAPI_ToggleUnitMenu`, the same entry pfUI's own Lua path calls, so the two routes open one menu), `focus`, `spell` (cast on the frame's `unit`, which is what click-casting means) and `macro`/`macrotext`, with `shift-`/`ctrl-`/`alt-` prefixes falling back to the unmodified binding as the reference's lookup does. **What is dispatched is the action, not the security model**: no taint, no protected call, no combat lockdown — none of which 1.12 has | | nampower's `*_SELF` aura edges | **built** | `BUFF_ADDED_SELF`/`BUFF_REMOVED_SELF`/`DEBUFF_ADDED_SELF`/`DEBUFF_REMOVED_SELF`, over the diff that already produced the `*_OTHER` pair — the player was **explicitly skipped** there, with a comment saying this half was unbuilt. Its helpful half is diffed separately because the halves fire different events. Why it matters: DoiteAuras reads the whole aura block **once**, at `PLAYER_ENTERING_WORLD`, and then relies entirely on these four (`UpdateAuras` is called nowhere else), so with none fired its table froze at login and every buff gained afterwards read as missing. Upstream has **eight**; the `BUFF_*_OTHER` pair is deliberately not produced — it would mean diffing the helpful half of every streamed unit on every edge for no reader — and `AuraEdge`'s three variants say so rather than leaving a dead arm firing events the gate must then allow. **Gate note**: routing the names through a `&str` parameter compiled and left `reference_ui.rs`'s event walker blind (it reads the literal at the call site), including to the two `*_OTHER` names already declared; the literals are back at the fire sites for that reason | | Aura feed ordered before the `PLAYER_ENTERING_WORLD` fire | **fixed** | `feed_auras` and `feed_units` both sat in `UnitFeed` with **no ordering between them**, and `feed_units` is what fires `PLAYER_ENTERING_WORLD`. Whether an addon reading the aura block in that handler saw it was down to Bevy's ambiguous order — which is why DoiteAuras worked on a fresh login and broke after a `/reload`. A new `UnitFeedDispatch` set wraps `feed_units` alone so a sibling feed can order itself ahead of the fire; `feed_auras` now does. Same reasoning `feed_units` already applied internally to the XP pair ("ahead of the `PLAYER_ENTERING_WORLD` fire so its first update reads it") | +| `PlaySoundFile` on an addon's `.ogg` | **built** (decoder) | Not an API gap — the binding and the loose-file read were 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!`. Silent alerts with nothing in the log. **75 `.ogg` files on this install**: DoiteAuras (57), BigWigs (11), unitscan (3), HCWarn (2), TWThreat and Cleanup — so every one of those addons' alerts was dead, including HCWarn's hardcore death warnings. AI_VoiceOver ships `.mp3` and always worked, which is what made it look like an addon-specific problem. Stock 1.12 plays wav and mp3 only; the Turtle/OctoWoW client these addons are written for plays ogg, which is why they ship it — the deviation is from 1.12, toward the client the player actually runs. Covered by `sound::mixer::tests::an_addons_ogg_decodes`, gated on the player's own addon folder the way the chain tests are gated on a WoW install | | `LOCALIZED_CLASS_NAMES_MALE` / `_FEMALE` | **built** | Published by `script/class_info.rs` whenever the app feeds the class table, from the same two `ChrClasses` columns `C_CreatureInfo.GetClassInfo` answers from: keyed by `classFile` (`WARRIOR`), valued by the display name (`Warrior`). Half of this pair was already here and the other half missing — `ClassInfo::class_file`'s own doc names these tables as what FrameXML keys on, and `compat/classic_api/lua_ext.rs`'s `tInvert` exists *because* pfUI flips this one (`pfUI.lua:227`). With the table absent that flip inverted nil, so `pfUI.env.L["class"]` was **empty** (measured: 0 entries against the live chain) and every lookup through it silently found no class — the guild roster's name colours and class icons (`modules/socialmod.lua:18`), the friends list's, and `modules/panel.lua:359`. Nothing raised, because the module tests `if class then` and skips, which is why it read as "pfUI just doesn't colour those lists here". 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 — publishing `_FEMALE` as a copy is what this data honestly supports, and omitting it would hand a caller nil where its sibling answers. Absent, not empty, on a run that never fed the table | | `C_Spell.GetSpellMechanicByID` / `GetSpellEffectMechanics` / `GetSpellPowerCost` | **built** | One probe for all three (`SpellDbcExtras`): same `Spell.dbc` row, same install site, and a caller asking for either has already paid the lookup. Its own probe rather than columns on `SpellLookup`, which has eleven construction sites with no answer for them. **`GetSpellMechanicByID` is three-way** and the consumer spells it out: nothing for an id no spell owns, `0` for a real spell with no mechanic, the id otherwise — it replaces a hand-kept spellID → mechanic table, so collapsing "unknown" into "none" would reclassify crowd control as plain damage. The enUS name is the second return, from `SpellMechanic.dbc` (which this tree already parsed). **`GetSpellEffectMechanics` is the per-effect sibling**, `EffectMechanic[3]` as `{m1, m2, m3}`, and it is not the same question: vanilla puts some mechanics on an effect rather than on the spell, so Rake's bleed answers `{0, 15, 0}` where `GetSpellMechanicByID` answers 0 — a macro conditional asking whether its target is bleeding gets the wrong answer from the spell-level column alone. Same three-way shape (nothing for an unknown id, three zeros for a spell with no mechanic anywhere). `SpellDisplay` already parsed the column for the cast-immunity scan, so this cost a field on the probe payload and nothing else. Its absence was one raise a session from `SuperCleveRoidMacros/ClassicAPI.lua:267`, which calls it unguarded. **`GetSpellPowerCost` answers a LIST** (`costs[1].cost`), one row, since 1.12 has a single cost per spell; it is the flat `manaCost` column **without** the reference's per-level and percentage terms (`GetPowerCost` `0x6e31b0`), which need the caster's level and max power — stated because it is the base cost a macro reader wants, not what a scaling spell would actually spend | | nampower `GetUnitField` / `GetSpellRecField`, version 4.1.3 | **built** | `GetUnitField(unit, field)` is nampower v3+'s window onto the raw `UNIT_FIELD_AURA` block, and the reason it exists is that its slots are **stable**: every 1.12 aura getter walks the *compacted* list, so each index shifts the moment an earlier aura drops and nothing can be keyed on it across frames. Two fields: `"aura"` → slot → spell id, `"auraApplications"` → slot → **stacks minus one** (the wire's own encoding; pfUI reads it back as `(apps[slot] or 0) + 1`, so returning `stacks` would show every debuff one stack high). **Slots are reported 1-based** — debuffs 33..48, which both consumers loop literally — over benilla's 0-based wire index. Needed new state: `AuraState::slot`, since the VM's aura list was compacted and had dropped the raw index; filled at both app-side builders. Accepts a token or a raw GUID (pfUI passes `GetDebuffSlotMap(guid)`, DoiteAuras passes `"player"`), folding a guid back through `token_for_guid`. Reads the same filtered list every other getter does — hidden-from-aura-bar and tracking auras are already gone, which is what a debuff tracker wants, and the slot keying is unaffected because a dropped aura reads nil at its slot rather than shifting its neighbours. `GetSpellRecField(spellID, field)` is **partial on purpose**: `name` and `rank` are answered, `stackAmount`/`manaCost`/`spellIconID` are **nil rather than a plausible zero** — this client reaches no other column of that row, and a fabricated `manaCost` of 0 reads as "free", which is exactly the test DoiteAuras' Clearcasting rule turns on. `spellIconID` is not derivable at all: the icon is carried as an MPQ path, not as the numeric id that names it. With all three in place `GetNampowerVersion` moves 4.0.0 → **4.1.3**, which is the gate DoiteAuras gates on — bumped only *after* the machinery existed, per that constant's own standing rule not to claim what is absent. Measured: DoiteAuras goes from refusing to open to **0 load / 0 session errors**; pfUI's `libs/libdebuff.lua` ("GetUnitField Edition") stops falling back to manual slot shifting |