From 90b2ba1859b60a16ec2304d74b4f9c8314194677 Mon Sep 17 00:00:00 2001 From: mgi388 <135186256+mgi388@users.noreply.github.com> Date: Tue, 17 Dec 2024 06:28:24 +1100 Subject: [PATCH] Rename AudioSinkPlayback::toggle to toggle_playback (#16837) # Objective - #16813 added the ability to mute sinks and added a new method `toggle_mute()`. - Leaving `toggle()` as is creates inconsistency and a bit of confusion about what is being toggled. ## Solution - Rename `toggle()` to `toggle_playback()`. - The choice to use the `_playback` suffix was easy because the method comment was already telling us what is being toggled: `Toggles playback of the sink.` - [Raised in Discord] and got the OK from Alice. [Raised in Discord]: https://discord.com/channels/691052431525675048/749430447326625812/1318000355824504905 ## Testing - I ran the example and also updated the instruction text to make it clear `Space` is toggling the playback not just pausing. - I added a unit test for `toggle_playback()` because why not. --- ## Showcase Example instructions: image ## Migration Guide - `AudioSinkPlayback`'s `toggle` method has been renamed to `toggle_playback`. This was done to create consistency with the `toggle_mute` method added in https://github.com/bevyengine/bevy/pull/16813. Change instances of `toggle` to `toggle_playback`. E.g.: Before: ```rust fn pause(keyboard_input: Res>, sink: Single<&AudioSink>) { if keyboard_input.just_pressed(KeyCode::Space) { sink.toggle(); } } ``` After: ```rust fn pause(keyboard_input: Res>, sink: Single<&AudioSink>) { if keyboard_input.just_pressed(KeyCode::Space) { sink.toggle_playback(); } } ``` --- crates/bevy_audio/src/sinks.rs | 18 +++++++++++++----- examples/audio/audio_control.rs | 4 ++-- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/crates/bevy_audio/src/sinks.rs b/crates/bevy_audio/src/sinks.rs index dcd8fb01f3d1d..d4be43261fde0 100644 --- a/crates/bevy_audio/src/sinks.rs +++ b/crates/bevy_audio/src/sinks.rs @@ -58,10 +58,11 @@ pub trait AudioSinkPlayback { /// A paused sink can be resumed with [`play`](Self::play). fn pause(&self); - /// Toggles the playback of this sink. + /// Toggles playback of the sink. /// - /// Will pause if playing, and will be resumed if paused. - fn toggle(&self) { + /// If the sink is paused, toggling playback resumes it. If the sink is + /// playing, toggling playback pauses it. + fn toggle_playback(&self) { if self.is_paused() { self.play(); } else { @@ -69,7 +70,7 @@ pub trait AudioSinkPlayback { } } - /// Is this sink paused? + /// Returns true if the sink is paused. /// /// Sinks can be paused and resumed using [`pause`](Self::pause) and [`play`](Self::play). fn is_paused(&self) -> bool; @@ -338,13 +339,20 @@ mod tests { audio_sink.set_speed(1.0); assert_eq!(audio_sink.speed(), 1.0); - // Test pause + // Test playback assert!(!audio_sink.is_paused()); // default pause state audio_sink.pause(); assert!(audio_sink.is_paused()); audio_sink.play(); assert!(!audio_sink.is_paused()); + // Test toggle playback + audio_sink.pause(); // start paused + audio_sink.toggle_playback(); + assert!(!audio_sink.is_paused()); + audio_sink.toggle_playback(); + assert!(audio_sink.is_paused()); + // Test mute assert!(!audio_sink.is_muted()); // default mute state audio_sink.mute(); diff --git a/examples/audio/audio_control.rs b/examples/audio/audio_control.rs index c13509266a201..2091ac8011966 100644 --- a/examples/audio/audio_control.rs +++ b/examples/audio/audio_control.rs @@ -18,7 +18,7 @@ fn setup(mut commands: Commands, asset_server: Res) { // example instructions commands.spawn(( - Text::new("-/=: Volume Down/Up\nSpace: Pause Playback\nM: Toggle Mute"), + Text::new("-/=: Volume Down/Up\nSpace: Toggle Playback\nM: Toggle Mute"), Node { position_type: PositionType::Absolute, bottom: Val::Px(12.0), @@ -40,7 +40,7 @@ fn update_speed(sink: Single<&AudioSink, With>, time: Res