The new one has two bugs:
- buffer isn't assigned to the sound object (you'll need a sound.setBuffer(buffer) in there when it starts playing)
- auto buffer = soundBuffers.get(soundToPlay); will cause a similar scope destruction issue as before
For the second one, soundBuffers.get(soundToPlay) will return a reference to the SoundBuffer object. But the auto keyword ignores references. So buffer will be a SoundBuffer (a clone of the one in the resourceholder), not a reference to one. This clone is a local variable, and will go out of scope at the end of the function. SoundBuffers, on destruction, remove themselves from any Sound object playing them.
Changing the line to auto &buffer = soundBuffers.get(soundToPlay); should fix it. (So buffer will be a reference to a SoundBuffer that keeps existing)
Although another problem will arise. You need one Sound object for every simultaneous playing sound. If your manager has just the one Sound member, you can only play one sound at a time.
In my own sound manager, I create a new Sound object every time I play a sound. I store it in a collection. Every frame I scan the collection to find all sounds that have finished playing, and delete them.
It's a bit crappy, but it works for what I needed.
sound_manager.h:
#pragma once
#include <SFML/Audio.hpp>
namespace kage
{
namespace SoundManager
{
sf::Sound *playSound(const std::string &filename);
void update();
void preload(const std::string& filename);
}
}
sound_manager.cpp
#include <map>
#include <string>
#include "kage/sound_manager.h"
namespace kage
{
namespace SoundManager
{
std::map<std::string, sf::SoundBuffer*>& getSoundBuffers()
{
static std::map<std::string, sf::SoundBuffer*> s_soundBuffers;
return s_soundBuffers;
}
std::vector<sf::Sound*>& getSounds()
{
static std::vector<sf::Sound*> s_sounds;
return s_sounds;
}
sf::Sound *playSound(const std::string &filename)
{
sf::SoundBuffer *sb;
std::map<std::string, sf::SoundBuffer *>::iterator it = getSoundBuffers().find(filename);
if (it == getSoundBuffers().end())
{
sb = new sf::SoundBuffer;
if (!sb->loadFromFile(filename))
{
delete sb;
return 0;
}
getSoundBuffers()[filename] = sb;
}
else
{
sb = it->second;
}
sf::Sound *s = new sf::Sound(*sb);
s->play();
return s;
}
void update()
{
for (int i = 0; i < getSounds().size(); ++i)
{
if (getSounds()[i]->getStatus() == sf::SoundSource::Stopped)
{
delete getSounds()[i];
getSounds().erase(getSounds().begin() + i);
--i;
}
}
}
void preload(const std::string& filename)
{
sf::SoundBuffer* sb;
std::map<std::string, sf::SoundBuffer*>::iterator it = getSoundBuffers().find(filename);
if (it == getSoundBuffers().end())
{
sb = new sf::SoundBuffer;
if (!sb->loadFromFile(filename))
{
delete sb;
return;
}
getSoundBuffers()[filename] = sb;
}
}
}
}
(Before anybody mentions, yes, preload and playSound duplicate a big chunk of code. I added preload later and didn't get around to refactoring the code)
When a non-looping sound has a status of sf::SoundSource::Stopped, it has finished playing and can be removed. Or you could use a pool system, reuse sounds that have stopped instead of making new ones.
This is an older version, my newer code uses the Entt (Entity Component System) to handle sounds and check if they are playing. But that's still a work in progress, and would be confusing if you aren't familiar with Entt (it's awesome, works great with SFML).
