expose audio rewind and add finished function to SoundInstance

[puppetmaster-]

For sound that does not loop it is important to know when it has finished playing. This allows the following sound to be played or game events to be coordinated.

I used it in the latest game to play the background music again after a certain time.

[17cupsofcoffee]

:+1: This seems like a sensible thing to add!

Before I merge, I need to make sure that just checking rewind on its own is the right thing to do here - I think it is, but it's been so long since I worked on the audio backend that I'm doubting myself.

[17cupsofcoffee]

Ah, okay, I remember a bit better now - the rewind flag is a way for the code to signal that playback has been stopped and the sound woukd need to start over if it is played again. There's actually two circumstances where this can occur:

• When playback of the sound finishes
• When stop is called on the sound instance

With that in mind, I'm not sure if finished is the right name for this method as-is - it'd maybe make sense for it to be called stopped (which would maybe imply that I should add playing and paused methods too, hmmm....)

For your use case, is it important to distinguish between a sound 'finishing' naturally and a sound being manually stopped? Or do you just want to check that it's ended, regardless of how it ended?

[puppetmaster-]

For my use case I only need to know if it was finished naturally.

some of my use cases:

• don't play two background music at the same time
• do scene changes only after the effect music has been played
• play background music again after a certain time

[17cupsofcoffee]

I guess what I'm trying to work out is, would it cause you issues if the new method also returns true when you manually stop the sound instance? Because Tetra's audio API currently doesn't make a distinction between a sound that's been stopped manually and a sound that's been stopped by reaching the end of playback.

Based on your use cases, I think this'd be okay, I just don't want to add a new API that isn't actually going to solve your problem :p

[puppetmaster-]

Then i got you wrong, for my use-cases it doesn't matter if the sound stopped by itself or was stopped manually. I do not need to distinguish. I just thought that stop is not important because you have to trigger it manually, so you know that it is stopped.

[17cupsofcoffee]

Cool! Then sounds like this shouldn't be too tricky a change.

I'm not sure if I'm going to use this branch as a starting point or just implement the changes locally (seems to be the same amount of effort either way), but I'll try to get this added tonight 😄

[17cupsofcoffee]

Done in https://github.com/17cupsofcoffee/tetra/commit/4adf23cca617597e9557702a789264325f9ceda5 - I went with a slightly different API in the end, inspired by how this works in XNA.

Rather than having individual methods for is_playing, is_paused and is_stopped, I added a new enum called SoundState and a method that returns it. The benefit of this is that if you want to do different things in your code depending on what state a sound is in, you can just match on the enum like this, without having to repeat the atomic operations in AudioControls for each branch:

match self.sound.state() {
    SoundState::Playing => { /* do something */ },
    SoundState::Paused  => { /* do something else */ },
    SoundState::Stopped => { /* or something else? */ },
}

This is (probably?) a bit cheaper, and it ensures that there's no race conditions (e.g. if there were seperate is_playing and is_stopped methods, could the sound's state change between calling the two methods?).

For the case where you just want to check one, you can do:

if let SoundState::Stopped = self.sound.state() {
    // just react to a single state
}

This also allowed me to add a set_state API, which might be nicer than using play/pause/stop in some circumstances (e.g. if you want to pull out some logic around playback states into a function).

Does this sound okay to you? Bit of a change from the original PR, but hopefully solves the same problem :)

[puppetmaster-]

Perfect, I tried your adjustment and everything works as I expected. Thank you.

[17cupsofcoffee]

:+1: Thank you for the PR!