Skip to content

Conversation

@hankjordan
Copy link
Contributor

@hankjordan hankjordan commented Mar 21, 2023

Objective

Exposes empty() method for AudioSink.
Based on 0.10.0, should be a non-breaking change.


Changelog

  • Expose empty() method for AudioSink
  • Add AudioSink::empty() example

@github-actions
Copy link
Contributor

Welcome, new contributor!

Please make sure you've read our contributing guide and we look forward to reviewing your pull request shortly ✨

@hankjordan hankjordan added the A-Audio Sounds playback and modification label Mar 21, 2023
@cart cart force-pushed the audiosink-empty branch from d3b0315 to 33b9906 Compare March 21, 2023 18:34
Copy link
Member

@cart cart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I rebased on main to fix the diff (it was including changelog and publish changes)

@@ -0,0 +1,35 @@
use bevy::prelude::*;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Definitely a matter of preference, but I don't think we need an example just to illustrate the empty method. With that criteria we'd have thousands of examples.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could update one of the existing audio example, like audio_control, to exit once the sink is empty

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just think its solving a problem that doesn't need to be solved. I think we should wait until the demand for such an example presents itself.

@cart cart enabled auto-merge April 5, 2023 21:35
@cart cart added this pull request to the merge queue Apr 5, 2023
Merged via the queue into bevyengine:main with commit 28046d5 Apr 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-Audio Sounds playback and modification

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

3 participants