diff --git a/crates/benilla-app/src/cvars.rs b/crates/benilla-app/src/cvars.rs index c15e8148..951ca410 100644 --- a/crates/benilla-app/src/cvars.rs +++ b/crates/benilla-app/src/cvars.rs @@ -4698,6 +4698,9 @@ mod tests { let mut app = App::new(); app.add_plugins(bevy::MinimalPlugins) .insert_resource(SoundConfig::default()) + // `sound::on_cvar` arms the `MasterSoundEffects` cut here, as it does in the real + // app; the observer is registered below and the resource has to exist for it. + .init_resource::() .insert_resource(UiScaleCvar(DEFAULT_UI_SCALE)) .insert_resource(ViewDistance { farclip: 350.0, diff --git a/crates/benilla-app/src/game_plugins.rs b/crates/benilla-app/src/game_plugins.rs index 77a8cbf2..4f7407e8 100644 --- a/crates/benilla-app/src/game_plugins.rs +++ b/crates/benilla-app/src/game_plugins.rs @@ -897,7 +897,17 @@ pub(crate) mod schedule_tests { /// to declare an order *about*: it reads one window's `RawHandleWrapper` and writes one /// `NonSend` handle neither of them can name, and its `run_if` is the handle's own /// "unresolved" bit, so it runs at most once in a process and then never again. - const UPDATE_ACTIONABLE_CEILING: usize = 4_954; + /// +1 for `sound::kit::cut_effects_on_reenable` (addon compat, `docs/ADDON_COMPAT.md`): the + /// `MasterSoundEffects` re-enable stop, which 1.12 addons use as their only way to cut a + /// playing sound. Its one pair is against the collider finisher, the same exclusive + /// `&mut World` shape `log_new_corpses` and `textinput::open_early` describe above. Its own + /// two accesses name nothing actionable: `SoundOutput` is a `NonSend` the [`Classes`] pass + /// already explains for every system that holds it, and `EffectCutArmed` exists precisely so + /// the flag is not a `SoundConfig` field — as a field it needed `ResMut` against every + /// `Res` reader in the frame, which measured 43 pairs under + /// `WOW_AMBIGUITY_DUMP=1`. The orders that *could* decide something are declared: after the + /// Lua pass that arms the edge, before the sound drain that plays what that pass queued. + const UPDATE_ACTIONABLE_CEILING: usize = 4_955; const UPDATE_ACTIONABLE_SLACK: usize = 40; fn ratchet(what: &str, n: usize, ceiling: usize, slack: usize) { diff --git a/crates/benilla-app/src/sound/kit.rs b/crates/benilla-app/src/sound/kit.rs index 3c5e6ceb..4558f6bc 100644 --- a/crates/benilla-app/src/sound/kit.rs +++ b/crates/benilla-app/src/sound/kit.rs @@ -992,6 +992,71 @@ fn evict_kit_cache( } } +/// The edge `0x7a6570` fires the stop on, and only it: the store is edge-triggered +/// (`7a657f: je` returns when the flag is unchanged) and `0x7a67a0` reaches its stop path only +/// when the new value is 1 (`7a67eb: test cl,cl` / `je`). Spent by [`super::on_cvar`], which is +/// the one place that sees every write. +pub(super) fn arms_effect_cut(was: bool, now: bool) -> bool { + now && !was +} + +/// Which categories the enabling edge stops. `Sfx` alone: music and ambience are the reference's +/// `flags & 0x4000008` class, muted rather than stopped (`0x7a65a0`), so a toggle must not cost +/// the player their music — the point of the toggle is to cut a bark, not to restart the score. +fn stopped_by_reenable(category: SoundCategory) -> bool { + matches!(category, SoundCategory::Sfx) +} + +/// Spend an armed `MasterSoundEffects` enabling edge: stop every playing effect, as the +/// reference does. +/// +/// `0x457500` (the CVar callback) → `0x7a6570` (the edge-triggered flag store) → `0x7a67a0`, +/// which walks the live instance list and, **on the enabling edge only** +/// (`7a67eb: test cl,cl` / `je`), stops each node: `0x7a5950` for a `flags & 0x4` node, and +/// `0x7a4200` plus the list removal `0x7604f0` for the rest — `0x7a5950` being a stop on the +/// node's own stream handle (`+0x34`) and channel id (`+0x30`). Two classes survive: a +/// `flags & 0x2` node, the cinematic bypass already named in [`super::cinematic`], and the +/// `flags & 0x4000008` class, which is muted rather than stopped (`0x7a65a0`). The *disabling* +/// edge stops nothing at all — channels run on silently, which is what +/// [`SoundConfig::category_amp`] already does, and why only this edge needs a system. +/// +/// Here that reads as: stop every `SoundCategory::Sfx` channel. Music and ambience are the +/// mute-not-stop class, and cinematic audio never enters [`SoundOutput::channels`] at all — it +/// goes to `play_stream` direct. +/// +/// **Not cosmetic.** `SetCVar("MasterSoundEffects", 0)` then `SetCVar("MasterSoundEffects", 1)` +/// is the *only* way a 1.12 addon can cut a sound that is already playing, and addons depend on +/// it: SoftSpoken's `SoundUtils:StopSound` is exactly those two lines — its `MuteChannel` is a +/// documented no-op on this client ("Intentionally unsupported"), so the toggle is its whole +/// repertoire. Its `SoundQueue` spends it twice over, to duck the game's own NPC dialogue while +/// it speaks (`MuteGameDialogue`: "on 1.12 … what the client can do is cut a bark already +/// playing") and to stop its own clip before replaying one ("Silence the head but keep its +/// place: it replays from the start when Advance next"). With the toggle inert, both failed in +/// the way grom reported: the NPC gossip line it meant to silence played on, and a clip it meant +/// to stop kept going while the queue started the same line again over the top of itself. +pub(super) fn cut_effects_on_reenable( + mut armed: ResMut, + mut out: NonSendMut, +) { + if !std::mem::take(&mut armed.0) { + return; + } + let before = out.channels.len(); + out.channels.retain_mut(|c| { + if !stopped_by_reenable(c.category) { + return true; + } + c.handle.stop(mixer::declick()); + false + }); + let stopped = before - out.channels.len(); + if stopped > 0 { + info!( + "sound: MasterSoundEffects re-enabled — {stopped} effect channel(s) stopped (0x7a67a0)" + ); + } +} + /// Registration hook for [`super::SoundPlugin`]. pub(super) fn plugin(app: &mut App) { app.add_systems(Startup, load_sound_kits.after(AssetSet::Open)) @@ -1007,12 +1072,75 @@ pub(super) fn plugin(app: &mut App) { OnExit(crate::char_select::ClientState::InWorld), stop_all_channels, ); + // Ordered between the Lua pass that arms the edge and the drain that plays what that same + // pass queued, because the reference's stop lands in exactly that gap: SoftSpoken toggles the + // CVar and *then* calls `PlaySoundFile` for the line it is about to read, so a cut applied + // after the drain would kill the new clip instead of the old one. + app.add_systems( + Update, + cut_effects_on_reenable + .after(crate::ui_script::UiInput) + .before(super::ui::drain_ui_sounds), + ); } #[cfg(test)] mod tests { use super::*; + /// `MasterSoundEffects` cuts playing effects on the way **back on**, never on the way off and + /// never at boot. An addon's `SetCVar(0)` / `SetCVar(1)` pair is the whole point: the first + /// write silences, the second is what stops — so getting this edge backwards would make the + /// only cut primitive 1.12 offers an addon do nothing at all, which is the bug this fixes. + #[test] + fn only_the_reenabling_edge_cuts_playing_effects() { + assert!( + arms_effect_cut(false, true), + "off then on: the `0x7a67a0` stop" + ); + assert!( + !arms_effect_cut(true, false), + "on then off: the reference stops nothing — channels run on silently, which is what \ + `category_amp` already does" + ); + assert!( + !arms_effect_cut(true, true), + "an unchanged write is not an edge (`7a657f: je`) — an addon that re-asserts \"1\" \ + every frame must not cut anything" + ); + assert!(!arms_effect_cut(false, false), "no edge at all"); + } + + /// The edge is armed by the CVar write, never sampled from the value. SoftSpoken writes `0` + /// and `1` inside one Lua call, so a per-frame sampler reads `true` both sides of it and sees + /// nothing — which is the whole reason `SoundConfig::effect_cut_armed` exists. Arming is also + /// what keeps boot quiet: the flag starts `false`, and `MasterSoundEffects` is registered + /// "1", so a session that never toggles never cuts. + #[test] + fn the_toggle_pair_arms_exactly_one_cut() { + let mut armed = false; + let mut enabled = true; + for write in [false, true] { + if arms_effect_cut(enabled, write) { + armed = true; + } + enabled = write; + } + assert!(armed, "`SetCVar(0)` then `SetCVar(1)` has to arm the cut"); + assert!(enabled, "and leave sound effects on"); + } + + /// The toggle cuts effects and leaves the score alone. + #[test] + fn the_reenabling_edge_spares_music_and_ambience() { + assert!(stopped_by_reenable(SoundCategory::Sfx)); + assert!( + !stopped_by_reenable(SoundCategory::Music), + "music is the reference's mute-not-stop class (`flags & 0x4000008`, `0x7a65a0`)" + ); + assert!(!stopped_by_reenable(SoundCategory::Ambience)); + } + /// Pinned as the 13 dwords at `.data` `0x87ce60`. #[test] fn the_cap_table_is_the_bytes_at_0x87ce60() { diff --git a/crates/benilla-app/src/sound/mod.rs b/crates/benilla-app/src/sound/mod.rs index 2bbc89dd..ec7861ed 100644 --- a/crates/benilla-app/src/sound/mod.rs +++ b/crates/benilla-app/src/sound/mod.rs @@ -59,13 +59,32 @@ pub(crate) use mixer::Mixer; pub(crate) use ui::{AutoEquipSound, LootPickupSound}; pub(crate) use zone::ExplorationSounds; +/// A `MasterSoundEffects` enabling edge waiting to be spent: armed by [`on_cvar`], consumed by +/// [`kit::cut_effects_on_reenable`]. +/// +/// **An armed flag, not a watched value.** The addon idiom writes `0` and then `1` *inside one +/// Lua call*, so a system sampling [`SoundConfig::enabled`] once a frame reads `true` on both +/// sides of the pair and sees no edge at all. The reference has no such problem: its stop runs +/// synchronously inside the `SetCVar` (`0x457500`), before the `PlaySoundFile` that follows. +/// +/// **Its own resource, not a `SoundConfig` field.** Spending the flag needs `ResMut`, and every +/// sound system in `Update` holds `Res` — as a field it put the cut in an undeclared +/// order against all of them, 43 pairs over `game_plugins`' ambiguity ceiling. Nothing but the +/// observer and the one consumer touches this, and an observer is not in the schedule. +#[derive(Resource, Default)] +pub(crate) struct EffectCutArmed(pub bool); + /// The sound CVars. Defaults are the reference's registrations (`0x456fe0`, `0x460a60`): /// `MusicVolume` 0.4 and `AmbienceVolume` 0.6, every other volume 1.0. #[derive(Resource)] pub(crate) struct SoundConfig { - /// `MasterSoundEffects` (default "1", `0x45737a`). The reference pauses the engine - /// (`0x457500` → `0x7a6570`); benilla zeroes every category in [`Self::category_amp`] and - /// channels run on silently, which sounds the same. + /// `MasterSoundEffects` (default "1", `0x45737a`). While it is off, the reference stops + /// nothing — channels run on silently, which is what [`Self::category_amp`] does by zeroing + /// every category. The edge that matters is the way **back**: `0x457500` → `0x7a6570` → + /// `0x7a67a0` stops every live effect instance on the *enabling* edge, which + /// [`kit::stop_effects_on_reenable`] carries and whose doc has the reason it is not optional. + /// An earlier revision of this comment called the pair "the same", which it is not: the + /// difference is the only sound-cutting primitive 1.12 gives an addon. pub enabled: bool, /// The dev chord + `M` mute: zeroes the main track only, so selection and channels run on and /// unmute is instant. Starts `false`; unattended runs open no device instead ([`SoundPlugin`]). @@ -321,14 +340,26 @@ pub(crate) struct SoundPlugin; /// The sound CVars' change callback: volumes clamp to `[0, 1]`, enables are `!= 0` as the /// reference parses them (`0x4574d0`: `setne al`). -pub(crate) fn on_cvar(ev: On, mut sound: ResMut) { +pub(crate) fn on_cvar( + ev: On, + mut sound: ResMut, + mut cut: ResMut, +) { let v = ev.num(); match ev.key().as_str() { "mastervolume" => sound.master = v.clamp(0.0, 1.0), "soundvolume" => sound.sfx = v.clamp(0.0, 1.0), "musicvolume" => sound.music = v.clamp(0.0, 1.0), "ambiencevolume" => sound.ambience = v.clamp(0.0, 1.0), - "mastersoundeffects" => sound.enabled = v != 0.0, + "mastersoundeffects" => { + // The reference's flag store is edge-triggered (`7a657f: je` returns on an unchanged + // value) and only the enabling edge reaches the stop (`7a67eb: test cl,cl`). + let now = v != 0.0; + if kit::arms_effect_cut(sound.enabled, now) { + cut.0 = true; + } + sound.enabled = now; + } "enablemusic" => sound.music_enabled = v != 0.0, "enableambience" => sound.ambience_enabled = v != 0.0, "enableerrorspeech" => sound.error_speech = v != 0.0, @@ -395,6 +426,7 @@ impl Plugin for SoundPlugin { copies_dropped: 0, }) .init_resource::() + .init_resource::() .init_resource::() .add_systems( Startup, diff --git a/crates/benilla-app/src/sound/ui.rs b/crates/benilla-app/src/sound/ui.rs index 304ba119..796e22cf 100644 --- a/crates/benilla-app/src/sound/ui.rs +++ b/crates/benilla-app/src/sound/ui.rs @@ -25,7 +25,7 @@ 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. -fn drain_ui_sounds( +pub(super) fn drain_ui_sounds( script: Option>, kits: Option>, assets: Option>, diff --git a/docs/ADDON_COMPAT.md b/docs/ADDON_COMPAT.md index 5b37764f..7b6de43b 100644 --- a/docs/ADDON_COMPAT.md +++ b/docs/ADDON_COMPAT.md @@ -1541,3 +1541,94 @@ Still deviating, and now written where it lives: the final `UNIT_MOD_CAST_SPEED` (`6e342c`, taken only when `Attributes & 0x30` is clear) is not applied, because no site streams unit field 139. It is 1.0 without a casting-speed aura, so it moves nothing a physical ability does — a Curse of Tongues target's cast lines are the case that still reads short. + +## SoftSpoken: one inert CVar toggle, two symptoms + +grom reported SoftSpoken "play a voice line over and over", and that its **Silence NPC Voices** +option did nothing although it was on. Both are one deviation, and it is ours. + +### The only cut primitive 1.12 gives an addon + +SoftSpoken's own 1.12 shim says what it cannot do: + +```lua +function SoundUtils:MuteChannel(channel, muted, fadeOut) + -- Intentionally unsupported on this client. +end +function SoundUtils:StopSound(clip, fadeMs) + SetCVar("MasterSoundEffects", 0) + SetCVar("MasterSoundEffects", 1) + clip.handle = nil +end +``` + +There is no Dialog channel to mute on 1.12, so the whole repertoire is that CVar pair. It is not a +hack the addon invented: toggling `MasterSoundEffects` off and straight back on is how a vanilla +addon stops a sound that is already playing, and the reference makes it work on the **enabling** +edge: + +``` +0x457500 the CVar callback: cl = !!value, tail-calls… +0x7a6570 …the flag store, edge-triggered (7a657f: je on an unchanged flag) +0x7a67a0 walks the live instance list; per node: + flags & 0x2 → skipped (the cinematic bypass) + flags & 0x4000008 → muted, not stopped (0x7a65a0) + otherwise, and ONLY when enabling (7a67eb: test cl,cl / je): + flags & 0x4 → 0x7a5950 stop + else → 0x7a4200 stop, then unlink (0x7604f0) +``` + +`0x7a5950` is unmistakable: it takes the node's stream handle (`+0x34`) and channel id (`+0x30`) +and calls the FMOD stop thunks. The *disabling* edge stops nothing at all — sounds run on, gated +silent at the pump, which is exactly what `SoundConfig::category_amp` already did by zeroing every +category. So our deviation was only ever half of the CVar, and the half we had was the half that +does not matter. + +An earlier revision of `SoundConfig::enabled`'s comment said benilla's gain-zeroing "sounds the +same". It does, for a player moving the slider. It does not for an addon, and that difference is +the only sound-cutting primitive the era offers. + +### The two symptoms + +`SoundQueue:MuteGameDialogue` is gated on the player's option and is, on this client, nothing but +the toggle: + +```lua +if not Addon.db.profile.Audio.AutoToggleDialog then return end +if Version.IsLegacyVanilla then + if speakingOn then + SetCVar("MasterSoundEffects", 0) + SetCVar("MasterSoundEffects", 1) + end + return +end +``` + +`AutoToggleDialog` defaults to `(Version.IsLegacyVanilla or …)` — **true** here — which is why his +saved profile has no `Audio` block at all and the option was nonetheless on. The gate passed, the +toggle ran, nothing was cut: the NPC greeting played straight over the line being read. + +The repeat is the same toggle spent on SoftSpoken's own clip. `SoundQueue:PlaySound` cuts *before* +it plays (`-- On Vanilla, interrupt NPC audio BEFORE playback`), and `StopKeeping` is documented as +"silence the head but keep its place: it replays from the start when Advance next reaches it, since +the client has no seek". With the cut inert the head never stopped, so the replay layered a second +copy on a first that was still going — a line over and over, over itself. + +### What changed + +`kit::cut_effects_on_reenable` stops every `SoundCategory::Sfx` channel when the edge is spent. +Music and ambience are left alone, matching the reference's mute-not-stop class; cinematic audio +never enters `SoundOutput::channels` at all, matching the `flags & 0x2` bypass. + +Two details are load-bearing and both are pinned by tests: + +- **The edge is armed by the CVar write, never sampled from the value.** The addon writes `0` and + `1` inside one Lua call, so a system reading `SoundConfig::enabled` once a frame sees `true` on + both sides and no edge at all. `EffectCutArmed` is set in the CVar observer, which is the one + place that sees every write — and it is its own resource rather than a `SoundConfig` field + because spending it needs `ResMut`, which as a field cost 43 ambiguous pairs against every + `Res` reader. +- **The cut is spent between the Lua pass and the sound drain.** The reference stops synchronously + inside `SetCVar`, which is *before* the `PlaySoundFile` that follows it in the same addon + function. A cut applied after the drain would stop the new clip instead of the old one — the + symptom inverted rather than fixed.