Skip to content

Commit

Permalink
audio sinks don't need their custom drop anymore (#9336)
Browse files Browse the repository at this point in the history 
# 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.
  • Loading branch information
@mockersf
mockersf authored Aug 11, 2023
1 parent 6a8fd54 commit 47a5a16
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 72 deletions.
41 changes: 12 additions & 29 deletions crates/bevy_audio/src/audio_output.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,35 +105,25 @@ pub(crate) fn play_queued_audio_system<Source: Asset + Decodable>(
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));
}
};
}
Expand All @@ -157,32 +147,25 @@ pub(crate) fn play_queued_audio_system<Source: Asset + Decodable>(
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));
}
};
}
Expand Down Expand Up @@ -215,24 +198,24 @@ pub(crate) fn cleanup_finished_audio<T: Decodable + Asset>(
>,
) {
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<T>, 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<T>,
SpatialAudioSink,
Expand Down
66 changes: 23 additions & 43 deletions crates/bevy_audio/src/sinks.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<Sink>,
}

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()
}
}

Expand All @@ -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<SpatialSink>,
}

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`.
Expand All @@ -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());
}
}

0 comments on commit 47a5a16

Please sign in to comment.