You also need to be a little careful with sounds and sound buffers, due to the way they are linked to each other internally (sounds know which buffer they are using, buffers know every sound using them).
For example, if you had this code:
std::vector<sf::SoundBuffer> sbv;
std::vector<sf::Sound> sv;
{
sf::SoundBuffer sb;
sb.loadFromFile("data/NFF-soft-confirm.wav");
sf::Sound s(sb);
sbv.push_back(sb);
sv.push_back(s);
sv.back().play();
}
This will start trying to play the sound, then immediately stop (usually too fast to tell, if you aren't debug stepping through the code).
The reason is there are 2 sounds and 2 buffers alive at once, the local ones (s and sb) and the copies in the vectors. When a sound is pushed into a vector, it is duplicated. The duplicate connects itself to the buffer sb, since that's what sound s is connected to.
When buffer sb goes out of scope at the end of the braces, it's destructor tells every sound using that buffer to remove their buffer connection. Both sounds (s and the one in the vector) lose their buffer.
So the connection (and any play operations) needs to happen after pushing the sound and sound buffer, otherwise it will be lost.
And as eXpl0it3r says, vectors themselves can shuffle around (as they fill up, they will move everything to larger blocks of memory) which will break the connection, so something like a deque will help.
Using any complex class (like sounds, sprites, textures, etc) as a value in an std::vector or other container is always something to be careful of, since the duplication while inserting/pushing might mess things up on cleanup. If I'm not familiar with the internals of a class, I'd always heap allocate it and push the address instead.
The erase function requires an iterator to tell it which one to erase. In your code:
m_SoundV.erase(m_SoundV[i]);
m_BufferV.erase(m_BufferV[i]);
the value you are passing to erase is the actual sound or soundbuffer object, not an iterator to it.
An iterator is like a pointer, it's an object made to move through containers. STD likes them a lot.
Given an index number i, you can turn it into an iterator by adding it to another iterator, like begin(), which is the iterator of the first thing in the deque:
m_SoundV.erase(m_SoundV.begin()+i);
m_BufferV.erase(m_BufferV.begin()+i);
I find iterators to be annoying, but there are some containers (maps, lists) that can't be used with an index number lookup the iterator has extra logic for them to step 1 at a time.
Something that can go wrong with erasing is values move down to fill the gap. If you erase at position 5, whatever was at position 6 will now be position 5 and so on. Your code is ok, since it returns immediately after the erasing, it doesn't keep stepping through the deque. But it does mean you can only stop one sound each time. If several stopped, it takes one pass for each. The way around that is to make it take a step back if it erases. So if you erase at position 5, you need to check 5 again since a new item is there now. A bit like this:
for(int i=0;i<m_soundV.size();++i)
{
if (m_SoundV[i].getStatus() == sf::Sound::Stopped) {
m_SoundV.erase(m_SoundV.begin()+i);
m_BufferV.erase(m_BufferV.begin()+i);
--i;
}
}
This way the --i takes a step back, to counteract the ++i step forwards in the loop, so if anything is erased, you won't skip the next one, they all get checked.
The parts about templates you've written look like you meant typedefs. There's no need to make any templates for an std::deque, it already is one.
A typedef lets you give an alternative name to a type, usually to make it easier to use.
typedef sf::Sound S;
std::deque<S> m_SoundV;
In that code, S is another name for sf::Sound. Or you could shorten it even further:
typedef std::deque<sf::Sound> S;
S m_soundV;
Now S is the entire deque type.
But none of these are actually needed, they just reduce your typing if you use std::deque<sf::Sound> a lot.
I think that's what you meant there.
Anyway, one little SFML optimisation tip. SoundBuffers are large, they could take megabytes each (they are raw uncompressed audio, like a WAV file). Sounds are tiny. You can have many Sounds all sharing a single SoundBuffer. If 100 gunshots are playing at once, you can have 100 Sounds and 1 SoundBuffer. So usually you don't remove a SoundBuffer, you keep it around, since making a new one uses a lot of memory and slows down to read the file from disk again.
The same with sprites and textures, you can have hundreds of Sprites all using 1 Texture.
Update: I've marked this as Un-SOLVED but I'm putting it back as SOLVED because I've implemented a satisfactory buffer cleanup! I'll leave this lingering question just in case anybody has a good answer. However, I'm not stuck on it so we're good now :D
Okay, I tried to figure this out for myself but I'm not sure yet... maybe somebody has already been down this path and knows the answer?
Is there a way to check if the buffer is unused (no longer being used)? It looks like OpenAL can check within itself but I don't think the API exposes that state publicly.
I can implement a lazy cleanup, I'm not entirely against it. The first thing I usually do is try to break the system once I've written it; it breaks when I play long lightning/thunder roll sounds. The buffers sometimes get swept up prematurely. Not a game breaker, so meh, I'll keep notes on it.
If I figure it out I'll see if I can do it within the scope of SFML (and not the scope of OpenAL) and I'll request it be merged. I might not get to it for another week though, I have old tasks I need to check off my list :) I probably WON'T do this but if I ever do I won't be shy to request to merge it Just not high on my priority list. :)
This is basically what I did, just for reference to anybody new who might need to do it. I basically time the longest sound, double that time, and do a full clear of the buffers if the time maxes out. I'm sure an even BETTER implementation COULD be made, sure, but this works for me.
bool SoundMix::play(const sf::String& l_name, float volume, float pitch) {
// End function doing nothing if no name given
if (l_name.isEmpty()) { return true; }
// Create sound and sound buffer
sf::Sound sound;
sf::SoundBuffer buffer;
// If unable to open from file
if (!buffer.loadFromFile(l_name)) {
// Print to console
cout << "Unable to load Sound Effect" << endl;
// Error opening file
return false;
}
// Set volume, pitch and loop
sound.setVolume(volume);
sound.setPitch(pitch);
sound.setLoop(false);
// If sound duration is more than current max time
if (buffer.getDuration() > m_timeSEMax) {
// Restart timer and set time max duration
m_timeSE.restart();
m_timeSEMax = (buffer.getDuration());
// Give the buffers some extra time by doubling it
m_timeSEMax += (buffer.getDuration());
}
// Add to vectors
m_SoundV.push_back(sound);
m_BufferV.push_back(buffer);
// Set the last buffer on the stack
m_SoundV[m_SoundV.size() - 1].setBuffer(m_BufferV[m_BufferV.size() - 1]);
// Play the sound
m_SoundV[m_SoundV.size() - 1].play();
cout << "Playing SE : " << m_SoundV.size() << endl;
// Opened file successfully
return true;
}
void Audio::updateBuffers() {
// End function if no buffers
if (m_BufferV.empty()) { return; }
// Clear buffers if enough time has passed by
if (m_timeSE.getElapsedTime() >= m_timeSEMax) {
cout << "Erasing Buffers : " << m_BufferV.size();
// Clear buffers and restart timer
m_BufferV.clear();
m_timeSE.restart();
cout << " ; Buffer Size " << m_BufferV.size() << endl;
}
}[code]