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

The MasterSoundEffects cut has to ride the play queue, not an observer a frame later

Browse Source 
grom, 2026-11-05: *"softspoken was repeating zone readouts over and over unless I manually
removed them from queue."*

1.12 has no verb that stops a playing sound, so every voice-over addon uses the one
primitive there is: `SetCVar("MasterSoundEffects", 0)` then `..., 1)`, whose ENABLING edge
walks the live instance list and stops each node (`0x457500` -> `0x7a6570` -> `0x7a67a0`).
SoftSpoken's `SoundUtils:StopSound` is exactly those two lines, and its `SoundQueue` spends
them twice over: to duck the game's NPC dialogue while it speaks, and — this report — to
stop its OWN clip before replaying one, which `StopKeeping` does and comments on ("it
replays from the start when Advance next reaches it, since the client has no seek").

`bb9eb084` had to exempt `PlaySoundFile` audio from the cut because stopping it silenced
SoftSpoken outright: the addon spends the toggle and THEN plays its own file, so a cut that
reached files took the very line the toggle made room for. Its doc said what that cost —
*"if the overlapping-replay symptom returns, this is why"*. It returned.

The ordering was the thing, not the exemption. `cut_effects_on_reenable` is already
`.after(UiInput).before(drain_ui_sounds)`, which looks like the right gap and is not: the
flag is armed by the `CvarChanged` observer, `CvarChanged` goes out through
`commands.trigger`, and so the observer fires at a command flush a frame AFTER the Lua pass
that wrote it — by which time the drain has started the new clip. The consumer's ordering
cannot fix a producer that is a frame late.

So the cut now rides the sound queue, in call order with the plays:

  * `SoundRequest::CutEffects`, pushed from `script::cvars::store` — the one write path
    every `SetCVar`, `ConsoleExec` and engine write goes through, and the only place that
    holds both the old value and the new, which an edge needs.
  * `drain_ui_sounds` walks the queue in order and spends it where the addon wrote it:
    stop, then play. Its catalog guard moved inside the loop, because the cut needs neither
    a catalog nor assets and a stop dropped for want of one leaves the clip it meant to
    stop playing under the next one.
  * `cut_effects` is now shared by both paths with a `files` flag. In order with the
    addon's calls it stops files, because the stop was written before the play; a frame
    late, from the observer, it still spares them, because by then the only file alive is
    the one the toggle made room for. Both answers were paid for by a report, and
    `stopped_by_reenable`'s doc says which is which.

Verified: `the_master_sound_toggle_queues_its_cut_ahead_of_the_file_that_follows_it` writes
SoftSpoken's own two calls and asserts the queue is `[CutEffects, File(...)]` — the stop
keeping its place in front of the play — with no cut for the disabling edge and none for a
re-assert. `only_the_in_order_cut_reaches_an_addons_own_file` pins both answers.
benilla-ui 2128/0.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn
This commit is contained in:
grom
2026-11-05 10:37:43 -07:00
co-authored by Claude Opus 5
parent fc78e52c25
commit 8fcab8517b
4 changed files with 200 additions and 60 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/benilla-app/src/sound/kit.rs
+88 -49
View File
@@ -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<u32> = 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<u32> = 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"
);
}
crates/benilla-app/src/sound/ui.rs
+26 -11
View File
@@ -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<NonSendMut<UiScript>>,
kits: Option<ResMut<SoundKits>>,
@@ -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.
crates/benilla-ui/src/script/cvars.rs
+30
View File
@@ -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::<f64>().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<String> {
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<String> {
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)
}
crates/benilla-ui/src/script/sound.rs
+56
View File
@@ -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();