From 47a5a16d8a5f82ad2f0f95d98f93af09d5e6d77e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois?= Date: Fri, 11 Aug 2023 23:16:12 +0200 Subject: [PATCH] audio sinks don't need their custom drop anymore (#9336) # Objective - Fixes #9324 - Audio sinks used to have a custom drop implementation to detach the sinks because it was not required to keep a reference to it - With the new audio api, a reference is kept as a component of an entity ## Solution - Remove that custom drop implementation, and the option wrapping that was required for it. --- crates/bevy_audio/src/audio_output.rs | 41 +++++------------ crates/bevy_audio/src/sinks.rs | 66 ++++++++++----------------- 2 files changed, 35 insertions(+), 72 deletions(-) diff --git a/crates/bevy_audio/src/audio_output.rs b/crates/bevy_audio/src/audio_output.rs index fd2173f6726d3..2c8f2057dfea3 100644 --- a/crates/bevy_audio/src/audio_output.rs +++ b/crates/bevy_audio/src/audio_output.rs @@ -105,35 +105,25 @@ pub(crate) fn play_queued_audio_system( match settings.mode { PlaybackMode::Loop => { sink.append(audio_source.decoder().repeat_infinite()); - commands - .entity(entity) - .insert(SpatialAudioSink { sink: Some(sink) }); + commands.entity(entity).insert(SpatialAudioSink { sink }); } PlaybackMode::Once => { sink.append(audio_source.decoder()); - commands - .entity(entity) - .insert(SpatialAudioSink { sink: Some(sink) }); + commands.entity(entity).insert(SpatialAudioSink { sink }); } PlaybackMode::Despawn => { sink.append(audio_source.decoder()); commands .entity(entity) // PERF: insert as bundle to reduce archetype moves - .insert(( - SpatialAudioSink { sink: Some(sink) }, - PlaybackDespawnMarker, - )); + .insert((SpatialAudioSink { sink }, PlaybackDespawnMarker)); } PlaybackMode::Remove => { sink.append(audio_source.decoder()); commands .entity(entity) // PERF: insert as bundle to reduce archetype moves - .insert(( - SpatialAudioSink { sink: Some(sink) }, - PlaybackRemoveMarker, - )); + .insert((SpatialAudioSink { sink }, PlaybackRemoveMarker)); } }; } @@ -157,32 +147,25 @@ pub(crate) fn play_queued_audio_system( match settings.mode { PlaybackMode::Loop => { sink.append(audio_source.decoder().repeat_infinite()); - commands - .entity(entity) - .insert(AudioSink { sink: Some(sink) }); + commands.entity(entity).insert(AudioSink { sink }); } PlaybackMode::Once => { sink.append(audio_source.decoder()); - commands - .entity(entity) - .insert(AudioSink { sink: Some(sink) }); + commands.entity(entity).insert(AudioSink { sink }); } PlaybackMode::Despawn => { sink.append(audio_source.decoder()); commands .entity(entity) // PERF: insert as bundle to reduce archetype moves - .insert(( - AudioSink { sink: Some(sink) }, - PlaybackDespawnMarker, - )); + .insert((AudioSink { sink }, PlaybackDespawnMarker)); } PlaybackMode::Remove => { sink.append(audio_source.decoder()); commands .entity(entity) // PERF: insert as bundle to reduce archetype moves - .insert((AudioSink { sink: Some(sink) }, PlaybackRemoveMarker)); + .insert((AudioSink { sink }, PlaybackRemoveMarker)); } }; } @@ -215,24 +198,24 @@ pub(crate) fn cleanup_finished_audio( >, ) { for (entity, sink) in &query_nonspatial_despawn { - if sink.sink.as_ref().unwrap().empty() { + if sink.sink.empty() { commands.entity(entity).despawn(); } } for (entity, sink) in &query_spatial_despawn { - if sink.sink.as_ref().unwrap().empty() { + if sink.sink.empty() { commands.entity(entity).despawn(); } } for (entity, sink) in &query_nonspatial_remove { - if sink.sink.as_ref().unwrap().empty() { + if sink.sink.empty() { commands .entity(entity) .remove::<(AudioSourceBundle, AudioSink, PlaybackRemoveMarker)>(); } } for (entity, sink) in &query_spatial_remove { - if sink.sink.as_ref().unwrap().empty() { + if sink.sink.empty() { commands.entity(entity).remove::<( SpatialAudioSourceBundle, SpatialAudioSink, diff --git a/crates/bevy_audio/src/sinks.rs b/crates/bevy_audio/src/sinks.rs index d53e32db62927..f81b817d698f2 100644 --- a/crates/bevy_audio/src/sinks.rs +++ b/crates/bevy_audio/src/sinks.rs @@ -77,52 +77,44 @@ pub trait AudioSinkPlayback { /// that source is unchanged, that translates to the audio restarting. #[derive(Component)] pub struct AudioSink { - // This field is an Option in order to allow us to have a safe drop that will detach the sink. - // It will never be None during its life - pub(crate) sink: Option, -} - -impl Drop for AudioSink { - fn drop(&mut self) { - self.sink.take().unwrap().detach(); - } + pub(crate) sink: Sink, } impl AudioSinkPlayback for AudioSink { fn volume(&self) -> f32 { - self.sink.as_ref().unwrap().volume() + self.sink.volume() } fn set_volume(&self, volume: f32) { - self.sink.as_ref().unwrap().set_volume(volume); + self.sink.set_volume(volume); } fn speed(&self) -> f32 { - self.sink.as_ref().unwrap().speed() + self.sink.speed() } fn set_speed(&self, speed: f32) { - self.sink.as_ref().unwrap().set_speed(speed); + self.sink.set_speed(speed); } fn play(&self) { - self.sink.as_ref().unwrap().play(); + self.sink.play(); } fn pause(&self) { - self.sink.as_ref().unwrap().pause(); + self.sink.pause(); } fn is_paused(&self) -> bool { - self.sink.as_ref().unwrap().is_paused() + self.sink.is_paused() } fn stop(&self) { - self.sink.as_ref().unwrap().stop(); + self.sink.stop(); } fn empty(&self) -> bool { - self.sink.as_ref().unwrap().empty() + self.sink.empty() } } @@ -138,61 +130,52 @@ impl AudioSinkPlayback for AudioSink { /// that source is unchanged, that translates to the audio restarting. #[derive(Component)] pub struct SpatialAudioSink { - // This field is an Option in order to allow us to have a safe drop that will detach the sink. - // It will never be None during its life - pub(crate) sink: Option, -} - -impl Drop for SpatialAudioSink { - fn drop(&mut self) { - self.sink.take().unwrap().detach(); - } + pub(crate) sink: SpatialSink, } impl AudioSinkPlayback for SpatialAudioSink { fn volume(&self) -> f32 { - self.sink.as_ref().unwrap().volume() + self.sink.volume() } fn set_volume(&self, volume: f32) { - self.sink.as_ref().unwrap().set_volume(volume); + self.sink.set_volume(volume); } fn speed(&self) -> f32 { - self.sink.as_ref().unwrap().speed() + self.sink.speed() } fn set_speed(&self, speed: f32) { - self.sink.as_ref().unwrap().set_speed(speed); + self.sink.set_speed(speed); } fn play(&self) { - self.sink.as_ref().unwrap().play(); + self.sink.play(); } fn pause(&self) { - self.sink.as_ref().unwrap().pause(); + self.sink.pause(); } fn is_paused(&self) -> bool { - self.sink.as_ref().unwrap().is_paused() + self.sink.is_paused() } fn stop(&self) { - self.sink.as_ref().unwrap().stop(); + self.sink.stop(); } fn empty(&self) -> bool { - self.sink.as_ref().unwrap().empty() + self.sink.empty() } } impl SpatialAudioSink { /// Set the two ears position. pub fn set_ears_position(&self, left_position: Vec3, right_position: Vec3) { - let sink = self.sink.as_ref().unwrap(); - sink.set_left_ear_position(left_position.to_array()); - sink.set_right_ear_position(right_position.to_array()); + self.sink.set_left_ear_position(left_position.to_array()); + self.sink.set_right_ear_position(right_position.to_array()); } /// Set the listener position, with an ear on each side separated by `gap`. @@ -205,9 +188,6 @@ impl SpatialAudioSink { /// Set the emitter position. pub fn set_emitter_position(&self, position: Vec3) { - self.sink - .as_ref() - .unwrap() - .set_emitter_position(position.to_array()); + self.sink.set_emitter_position(position.to_array()); } }