Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Merged by Bors] - Add a way to toggle AudioSink #6321

Closed
wants to merge 2 commits into from
Closed

[Merged by Bors] - Add a way to toggle AudioSink #6321

wants to merge 2 commits into from

Conversation

lovelymono
Copy link
Contributor

Objective

Currently toggling an AudioSink (for example from a game menu) requires writing

if sink.is_paused() {
    sink.play();
} else {
    sink.pause();
}

It would be nicer if we could reduce this down to a single line

sink.toggle();

Solution

Add an AudioSink::toggle method which does exactly that.


Changelog

  • Added AudioSink::toggle which can be used to toggle state of a sink.

Signed-off-by: Lena Milizé <me@lvmn.org>
@alice-i-cecile alice-i-cecile added A-Audio Sounds playback and modification C-Usability A simple quality-of-life change that makes Bevy easier to use labels Oct 20, 2022
@alice-i-cecile
Copy link
Member

Code is correct, and docs are good. Unsure about adding non-idempotent methods, but users will want this.

Would you use this for e.g. "disable all dialogue" buttons in menus?

Signed-off-by: Lena Milizé <me@lvmn.org>
@lovelymono
Copy link
Contributor Author

Would you use this for e.g. "disable all dialogue" buttons in menus?

Right now in my game I have a persistent background music (similar to Stellaris' music player), and use this for a "Toggle music" button in options menu.

@harudagondi
Copy link
Member

I am okay with this change, but note that kira and oddio doesn't have a toggle method, so does bevy_kira_audio and bevy_oddio.

Copy link
Contributor

@Pietrek14 Pietrek14 left a comment

Choose a reason for hiding this comment

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

Maybe the use case isn't broad, but it won't hurt to have this feature.

@alice-i-cecile alice-i-cecile added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Oct 25, 2022
@alice-i-cecile
Copy link
Member

Sure, why not. This is a common pattern.

bors r+

bors bot pushed a commit that referenced this pull request Oct 31, 2022
# Objective

Currently toggling an `AudioSink` (for example from a game menu) requires writing

```rs
if sink.is_paused() {
    sink.play();
} else {
    sink.pause();
}
```

It would be nicer if we could reduce this down to a single line

```rs
sink.toggle();
```

## Solution

Add an `AudioSink::toggle` method which does exactly that.

---

## Changelog

- Added `AudioSink::toggle` which can be used to toggle state of a sink.
@bors bors bot changed the title Add a way to toggle AudioSink [Merged by Bors] - Add a way to toggle AudioSink Oct 31, 2022
@bors bors bot closed this Oct 31, 2022
@mockersf mockersf added the hacktoberfest-accepted A PR that was accepted for Hacktoberfest, an annual open source event label Oct 31, 2022
ItsDoot pushed a commit to ItsDoot/bevy that referenced this pull request Feb 1, 2023
# Objective

Currently toggling an `AudioSink` (for example from a game menu) requires writing

```rs
if sink.is_paused() {
    sink.play();
} else {
    sink.pause();
}
```

It would be nicer if we could reduce this down to a single line

```rs
sink.toggle();
```

## Solution

Add an `AudioSink::toggle` method which does exactly that.

---

## Changelog

- Added `AudioSink::toggle` which can be used to toggle state of a sink.
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 C-Usability A simple quality-of-life change that makes Bevy easier to use hacktoberfest-accepted A PR that was accepted for Hacktoberfest, an annual open source event S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

None yet

5 participants