diff --git a/crates/benilla-app/src/sound/kit.rs b/crates/benilla-app/src/sound/kit.rs index daa03d7b..cb72b5a3 100644 --- a/crates/benilla-app/src/sound/kit.rs +++ b/crates/benilla-app/src/sound/kit.rs @@ -1009,21 +1009,59 @@ pub(super) fn arms_effect_cut(was: bool, now: bool) -> bool { /// of the toggle is to cut a bark, not to restart the score. /// /// `kit == 0`, the `PlaySoundFile` path ([`play_file`], whose doc records that id as its own -/// marker), is **measured, not derived**, and the distinction is worth keeping honest. Stopping -/// those too is what the first revision of this did, and it silenced SoftSpoken completely: -/// grom's session carries ten `MasterSoundEffects re-enabled` lines and he heard none of the ten -/// voice lines that produced them. It has to be excluded, because **every 1.12 voice-over addon -/// toggles this CVar and then plays its own file** — a cut that reaches file playback cuts the -/// very thing the toggle was spent to make room for. +/// marker), depends on **where the cut comes from**, and that is the whole subtlety of this CVar. +/// +/// A cut that arrives in order with the addon's own calls — [`cut_effects_in_order`], driven by +/// `SoundRequest::CutEffects` — stops files too, because the addon wrote the stop *before* the +/// play and the queue preserves that: `SoundQueue.StopKeeping` spends the toggle to stop its own +/// clip before replaying one, and that is a file. +/// +/// A cut that arrives from the deferred observer ([`cut_effects_on_reenable`]) spares them. That +/// path is a frame behind the Lua pass that armed it, so by the time it runs the drain has +/// already started the clip the toggle was spent to make room for; stopping files there is what +/// the first revision of this did, and it silenced SoftSpoken completely — grom's session carried +/// ten `MasterSoundEffects re-enabled` lines and he heard none of the ten voice lines that +/// produced them. /// /// Which of the reference's two flag classes a streamed file falls in is not settled here; -/// `0x4000008`'s meaning is unreversed, so this is a player's measurement standing in for a fact -/// we do not have. What it costs is the other half of SoftSpoken's use of the toggle: -/// `SoundQueue.StopKeeping` spends it to stop its *own* clip before replaying one, and that does -/// nothing again. If the overlapping-replay symptom returns, this is why — and the per-kit line -/// the stop now logs is what will say so. -fn stopped_by_reenable(kit: u32, category: SoundCategory) -> bool { - matches!(category, SoundCategory::Sfx) && kit != 0 +/// `0x4000008`'s meaning is unreversed. What is settled is the ordering, and the ordering is what +/// decides whether stopping a file is a stop or a self-inflicted silence. +fn stopped_by_reenable(kit: u32, category: SoundCategory, files: bool) -> bool { + matches!(category, SoundCategory::Sfx) && (files || kit != 0) +} + +/// Stop every playing effect and say what went, shared by the two cut paths. +fn cut_effects(out: &mut SoundOutput, files: bool, how: &str) { + let before = out.channels.len(); + // Named, not merely counted: which kits a toggle cuts is the only evidence separating "the + // bark died, as intended" from "we cut the line the addon was about to speak". + let mut stopped: Vec = Vec::new(); + let mut spared_files = 0usize; + out.channels.retain_mut(|c| { + if !stopped_by_reenable(c.kit, c.category, files) { + spared_files += usize::from(c.kit == 0 && c.category == SoundCategory::Sfx); + return true; + } + stopped.push(c.kit); + c.handle.stop(mixer::declick()); + false + }); + if !stopped.is_empty() || spared_files > 0 { + info!( + "sound: MasterSoundEffects re-enabled ({how}) — stopped {} of {before} channel(s), \ + kits {:?}; spared {spared_files} PlaySoundFile channel(s) (0x7a67a0)", + stopped.len(), + stopped + ); + } +} + +/// The in-order cut: `SoundRequest::CutEffects`, drained by [`super::ui::drain_ui_sounds`] from +/// the same queue as the plays, so a toggle the addon wrote before a `PlaySoundFile` is spent +/// before that file starts — which is where the reference's own stop lands (`0x457500`, inside +/// the `SetCVar`). This is the path that lets a voice addon stop its own clip. +pub(super) fn cut_effects_in_order(out: &mut SoundOutput) { + cut_effects(out, true, "in order with the addon's own calls"); } /// Spend an armed `MasterSoundEffects` enabling edge: stop every playing effect, as the @@ -1060,28 +1098,7 @@ pub(super) fn cut_effects_on_reenable( if !std::mem::take(&mut armed.0) { return; } - let before = out.channels.len(); - // Named, not merely counted: which kits a toggle cuts is the only evidence separating "the - // bark died, as intended" from "we cut the line the addon was about to speak". - let mut stopped: Vec = Vec::new(); - let mut spared_files = 0usize; - out.channels.retain_mut(|c| { - if !stopped_by_reenable(c.kit, c.category) { - spared_files += usize::from(c.kit == 0 && c.category == SoundCategory::Sfx); - return true; - } - stopped.push(c.kit); - c.handle.stop(mixer::declick()); - false - }); - if !stopped.is_empty() || spared_files > 0 { - info!( - "sound: MasterSoundEffects re-enabled — stopped {} of {before} channel(s), kits {:?}; \ - spared {spared_files} PlaySoundFile channel(s) (0x7a67a0)", - stopped.len(), - stopped - ); - } + cut_effects(&mut out, false, "deferred, from the CVar observer"); } /// Registration hook for [`super::SoundPlugin`]. @@ -1157,24 +1174,46 @@ mod tests { assert!(enabled, "and leave sound effects on"); } - /// The toggle cuts barks, leaves the score alone, and — the one grom's silence paid for — - /// never touches an addon's own `PlaySoundFile` audio. + /// The toggle cuts barks and leaves the score alone, on either path. #[test] - fn the_reenabling_edge_spares_music_ambience_and_addon_files() { + fn the_reenabling_edge_spares_music_and_ambience() { + for files in [false, true] { + assert!( + stopped_by_reenable(1, SoundCategory::Sfx, files), + "a kit-played effect is the bark the toggle exists to cut" + ); + assert!( + !stopped_by_reenable(1, SoundCategory::Music, files), + "music is the reference's mute-not-stop class (`flags & 0x4000008`, `0x7a65a0`)" + ); + assert!(!stopped_by_reenable(1, SoundCategory::Ambience, files)); + } + } + + /// Whether the cut reaches an addon's own `PlaySoundFile` audio is decided by **when** the + /// cut arrives, and both answers were paid for by a report. + /// + /// In order with the addon's calls it must: `SoundQueue.StopKeeping` spends the toggle to + /// stop its own clip before replaying it, and the clip is a file. grom, 2026-11-05: + /// *"softspoken was repeating zone readouts over and over unless I manually removed them + /// from queue."* That is the silenced stop — the old line never stopped, so each replay + /// layered over it. + /// + /// A frame late, from the deferred observer, it must not: by then the drain has already + /// started the clip the toggle made room for. That cut silenced SoftSpoken outright — ten + /// `MasterSoundEffects re-enabled` lines in grom's 2026-10-30 session and none of the ten + /// voice lines heard. + #[test] + fn only_the_in_order_cut_reaches_an_addons_own_file() { assert!( - stopped_by_reenable(1, SoundCategory::Sfx), - "a kit-played effect is the bark the toggle exists to cut" + stopped_by_reenable(0, SoundCategory::Sfx, true), + "in order: the stop the addon wrote BEFORE its play, so the file it stops is the \ + old clip and not the new one" ); assert!( - !stopped_by_reenable(1, SoundCategory::Music), - "music is the reference's mute-not-stop class (`flags & 0x4000008`, `0x7a65a0`)" - ); - assert!(!stopped_by_reenable(1, SoundCategory::Ambience)); - assert!( - !stopped_by_reenable(0, SoundCategory::Sfx), - "kit 0 is `play_file`'s marker, so this is an addon's own `PlaySoundFile`. Cutting \ - it silenced SoftSpoken outright: every 1.12 voice-over addon toggles this CVar and \ - then plays its own file, so the cut would take the line the toggle made room for" + !stopped_by_reenable(0, SoundCategory::Sfx, false), + "a frame late: kit 0 is `play_file`'s marker, and the only file alive by then is \ + the one the toggle was spent to make room for" ); } diff --git a/crates/benilla-app/src/sound/ui.rs b/crates/benilla-app/src/sound/ui.rs index 796e22cf..c648627d 100644 --- a/crates/benilla-app/src/sound/ui.rs +++ b/crates/benilla-app/src/sound/ui.rs @@ -25,6 +25,11 @@ use super::{SoundConfig, SoundOutput}; /// Drain the VM's queued `PlaySound` intents into the kit player; drained even with no catalog, /// so the queue cannot grow unbounded. +/// +/// The queue is walked **in call order** and the catalog is checked per play rather than once up +/// front, because one of the requests is not a play: `SoundRequest::CutEffects` is an addon's +/// stop, it needs no catalog, and a stop dropped for want of one leaves the clip it meant to +/// stop playing underneath the next one. pub(super) fn drain_ui_sounds( script: Option>, kits: Option>, @@ -39,22 +44,25 @@ pub(super) fn drain_ui_sounds( if requests.is_empty() { return; } - let (Some(mut kits), Some(assets)) = (kits, assets) else { - debug!( - "sound(ui): {} play(s) dropped — no kit catalog", - requests.len() - ); - return; - }; + let mut catalog = kits.zip(assets); + let mut dropped = 0usize; for req in requests { let kit_ref = match &req { + SoundRequest::CutEffects => { + kit::cut_effects_in_order(&mut out); + continue; + } SoundRequest::KitId(id) => KitRef::Id(*id), SoundRequest::KitName(name) => KitRef::Name(name), SoundRequest::File(path) => { + let Some((kits, assets)) = catalog.as_mut() else { + dropped += 1; + continue; + }; // `PlaySoundFile`: by path, with no kit gates or variation. if let Err(e) = kit::play_file( - &mut kits, - &assets, + kits, + assets, &mut out, &config, path, @@ -65,10 +73,14 @@ pub(super) fn drain_ui_sounds( continue; } }; + let Some((kits, assets)) = catalog.as_mut() else { + dropped += 1; + continue; + }; // 2D, on the SFX slider (the client's SoundVolume bucket). if let Err(e) = kit::play_kit( - &mut kits, - &assets, + kits, + assets, &mut out, &config, Vec3::ZERO, @@ -79,6 +91,9 @@ pub(super) fn drain_ui_sounds( debug!("sound(ui): {req:?} — {e:#}"); } } + if dropped > 0 { + debug!("sound(ui): {dropped} play(s) dropped — no kit catalog"); + } } /// The `ItemGroupSounds.dbc` catalog: the pickup, put-down and use kits per item sound group. diff --git a/crates/benilla-ui/src/script/cvars.rs b/crates/benilla-ui/src/script/cvars.rs index 398727fd..8f87d7ab 100644 --- a/crates/benilla-ui/src/script/cvars.rs +++ b/crates/benilla-ui/src/script/cvars.rs @@ -275,8 +275,32 @@ pub(super) fn set_from_engine(model: &mut Model, name: &str, value: String) { /// The one store every write goes through; returns the registered spelling when the value moved /// and the change was queued. A latched slot stages the write, as the reference's `Set 0x63df50` /// stores `latchedValue` and skips `InternalSet`; writing back the applied value unstages it. +/// The three registered spellings of the master sound enable, which is the one CVar an addon +/// writes for its *effect* rather than for its value: 1.12's `MasterSoundEffects` and the +/// later-era `Sound_EnableAllSound`/`Sound_EnableSFX` that the ported addons ask for. Each is its +/// own registered row, so the edge is per row, which is what an addon toggling one of them means. +fn is_master_sound_enable(key: &str) -> bool { + matches!( + key, + "mastersoundeffects" | "sound_enableallsound" | "sound_enablesfx" + ) +} + +/// Does this write turn the master sound enable back **on**? The reference's flag store is +/// edge-triggered and only this edge reaches the stop (`0x7a67eb`); the disabling edge stops +/// nothing. A value that does not parse reads as 0, as `atoi` does. +fn is_sound_reenable(key: &str, was: &str, now: &str) -> bool { + let num = |s: &str| s.trim().parse::().unwrap_or(0.0); + is_master_sound_enable(key) && num(was) == 0.0 && num(now) != 0.0 +} + fn store(model: &mut Model, key: &str, value: String) -> Option { let slot = model.cvars.get_mut(key)?; + // Decided here, where both the old and the new value are in hand, and queued below onto the + // *sound* queue rather than left to the `CvarChanged` observer — see + // [`crate::script::SoundRequest::CutEffects`] for why the observer is a frame too late to be + // the one that cuts. + let mut reenabled = false; if slot.latched { let staged = (value != slot.value).then_some(value.clone()); if slot.pending == staged && staged.is_some() { @@ -290,10 +314,16 @@ fn store(model: &mut Model, key: &str, value: String) -> Option { if slot.value == value { return None; } + reenabled = is_sound_reenable(key, &slot.value, &value); slot.value = value.clone(); } let registered = slot.name.clone(); model.cvar_changes.push((registered.clone(), value)); + if reenabled { + model + .sound_queue + .push(super::sound::SoundRequest::CutEffects); + } Some(registered) } diff --git a/crates/benilla-ui/src/script/sound.rs b/crates/benilla-ui/src/script/sound.rs index f33ac992..3b4c69b0 100644 --- a/crates/benilla-ui/src/script/sound.rs +++ b/crates/benilla-ui/src/script/sound.rs @@ -25,6 +25,18 @@ pub enum SoundRequest { KitName(String), /// `PlaySoundFile("path")`: a file path, no kit. File(String), + /// The enabling edge of `MasterSoundEffects` (and its two later-era spellings): stop every + /// playing effect, the reference's `0x457500` -> `0x7a6570` -> `0x7a67a0`. + /// + /// **Why it is a queued request and not just the `CvarChanged` observer's job.** The pair + /// `SetCVar("MasterSoundEffects", 0)` then `..., 1)` is the only way a 1.12 addon can stop a + /// sound that is already playing, and every voice-over addon spends it and then calls + /// `PlaySoundFile` for the line it is about to read. The reference stops *inside* the second + /// write, so the cut lands between the two calls. The observer cannot: it fires at a command + /// flush after the Lua pass, by which time the drain has already started the new clip, and a + /// cut there kills the very thing the toggle was spent to make room for. Riding this queue + /// puts the cut and the play in one stream, in the order the addon wrote them. + CutEffects, } /// One queued `PlayMusic` or `StopMusic`, for the app's Lua music slot. @@ -160,6 +172,50 @@ mod tests { assert!(s.take_sounds().is_empty()); } + /// `SetCVar("MasterSoundEffects", 0)` then `..., 1)` is the only way a 1.12 addon can stop a + /// sound already playing, and it rides this queue so that it keeps its place **in front of + /// the `PlaySoundFile` the addon calls next**. This is SoftSpoken's `SoundUtils:StopSound` + /// followed by its `PlaySound`, written out: the stop must be request 1 and the play request + /// 2, because the app spends them in that order and a stop spent afterwards kills the new + /// clip instead of the old one. + #[test] + fn the_master_sound_toggle_queues_its_cut_ahead_of_the_file_that_follows_it() { + let mut s = UiScript::new().unwrap(); + // The row as the host registers it (`0x45737a`): on by default, so an addon's first + // write is the disabling one. + s.register_cvars([("MasterSoundEffects", "1")]); + s.run( + "SetCVar(\"MasterSoundEffects\", 0)\n\ + SetCVar(\"MasterSoundEffects\", 1)\n\ + PlaySoundFile(\"Interface\\\\AddOns\\\\SpokenZonesAudio\\\\Sounds\\\\dm.mp3\")", + ) + .unwrap(); + assert_eq!( + s.take_sounds(), + vec![ + SoundRequest::CutEffects, + SoundRequest::File("Interface\\AddOns\\SpokenZonesAudio\\Sounds\\dm.mp3".into()), + ], + "the stop keeps its place ahead of the play" + ); + + // Only the enabling edge, and only on a move: the disabling write stops nothing in the + // reference (`0x7a67eb`), and a re-assert of the value it already holds is not an edge. + s.run("SetCVar(\"MasterSoundEffects\", 0)").unwrap(); + s.run("SetCVar(\"MasterSoundEffects\", 0)").unwrap(); + assert!( + s.take_sounds().is_empty(), + "switching sound off cuts nothing" + ); + s.run("SetCVar(\"MasterSoundEffects\", 1)").unwrap(); + s.run("SetCVar(\"MasterSoundEffects\", 1)").unwrap(); + assert_eq!( + s.take_sounds(), + vec![SoundRequest::CutEffects], + "one cut for the edge, none for the re-assert" + ); + } + #[test] fn the_music_pair_queues_in_call_order_and_answers_nothing() { let mut s = UiScript::new().unwrap();