[SOLVED] No sound is played but in debug mode is played

[winsett]

Hello!

I'm developing a small game and get to the point audio to be introduced to the game. But when I run the game and when sound should be played the game play no sound. The strange thing is when I'm in debug a sound is played when I go stepping through the code.  Nothing as error is shown to the console.

Here is a simplified scheme how I organized the audio system so far:

// singelton class for holding of all of the resources
class ResourcesManager
{
public:
    static ResourcesManager* getInstance(); // return single instance
    std::vector<sf::SoundBuffer> getSoundBuffers();
    ...
private:
    ResourcesManager() {}
    ...
    std::vector<sf::SoundBuffer> soundBuffers;
}

// another singelton class for playing the sounds
class SoundsPlayer
{
public:
        static std::shared_ptr<SoundsPlayer> getInstance();
        void play(const sf::SoundBuffer& buffer)
        {
                bool soundPlayed = false;
                // get next available sound
                for (auto sound : m_sounds)
                {
                        if (sound.getStatus() != sf::SoundSource::Status::Playing)
                        {
                                sound.setBuffer(buffer);
                                sound.play();
                                soundPlayed = true;
                                break;
                        }
                }
                if (!soundPlayed) // no sounds available
                {
                        sf::Sound sound;
                        m_sounds.push_back(sound);
                        size_t last = m_sounds.size() - 1;
                        m_sounds[last].setBuffer(buffer);
                        m_sounds[last].play();
                }
        }
private:
        SoundsPlayer() = default;
        std::vector<sf::Sound> m_sounds;
};


class PlayingCharacter
{
    ...
    void playAttackingSound()
        {
                extern ResourcesManager *resMan;

                SoundBuffersHolder soundBuffersHolder;
                std::shared_ptr<SoundsPlayer> soundsPly = SoundsPlayer::getInstance();
                bool isErr;
                soundsPly->play(resMan->getSoundBuffers().soundBuffers[0]);
        }
}
 

Some explanations on the code above:

• ResourcesManager is taking care of holding all of the resources and it is a singelton class. So far it works the way I organized it for the textures and other resources - I invoke
  extern ResourcesManager *resMan;
  and then I get what I need from it, i.e.
  sf::texture tex = resMan->getTexture() )
• First I simply tried to play a sound directly in the PlayingCharacter::playAttackingSound() method but there was no sound produced and I decided that probably the sf::Sound variable goes out of scope and that's why I made this SoundsPlayer singelton
• The PlayingCharacter::playAttackingSound() is invoked on more than a seconds (timer is taking care of it) and the sound is less than a second long so no overlapping happens
• Of course, these classes are in different files each

What am I missing? 

[eXpl0it3r]

Either your posted code doesn't match what you've implemented or this line shouldn't compile.

soundsPly->play(resMan->getSoundBuffers().soundBuffers[0]);

getSoundBuffers() returns a vector, so you can't be accessing a property called soundBuffers.

Hard to say now, what your actual code is, but if getSoundBuffers really does return a std::vector<sf::SoundBuffer>, you're always return a copy of the the whole vector, which then only exists temporarily and will be deleted after the call to play() is finished.
So the temporary sound buffer copy gets destroyed, before it's actually played.

To fix this, you'd need to return a reference to the sound buffer.

Additionally, I highly recommend to not misuse the singleton pattern for glorified globals. Instead use dependency injection, meaning to pass the resource manager and sound play via constructor to as few classes as possible.

[winsett]

Hi @eXpl0it3r and thanks for your answer!

Either your posted code doesn't match what you've implemented...

Yes, the real code is a little bit complicated than what I presented here and that's why simplified it a bit in order to present the issue in a more clear way but I think logically it presents what I wrote on my real code.

or this line shouldn't compile.

soundsPly->play(resMan->getSoundBuffers().soundBuffers[0]);

getSoundBuffers() returns a vector, so you can't be accessing a property called soundBuffers.

ResourcesManager::getSoundBuffers() returns a vector to the private member in the same class:
std::vector<sf::SoundBuffer> soundBuffers
so I don't see why it shouldn't compile. But anyway that is not the issue here... 

Hard to say now, what your actual code is, but if getSoundBuffers really does return a std::vector<sf::SoundBuffer>, you're always return a copy of the the whole vector, which then only exists temporarily and will be deleted after the call to play() is finished.
So the temporary sound buffer copy gets destroyed, before it's actually played.

To fix this, you'd need to return a reference to the sound buffer.

OK, changed the declaration to
std::vector<sf::SoundBuffer>& getSoundBuffers(); and now the getter returns a reference to the vector of buffers but still no sound...

Additionally, I highly recommend to not misuse the singleton pattern for glorified globals. Instead use dependency injection, meaning to pass the resource manager and sound play via constructor to as few classes as possible.

OK, I'll try to refactor this but it will take some time as the Resource manager is used on many places... It's highly possible I to have some question to your instruction and will ask you later...

Thanks again for your directions!

[eXpl0it3r]

One thing with using std::vector is that any push_back can cause a reallocation of all elements and change in memory address, which for sf::SoundBuffers will break the connection to the sf::Sound and for sf::Sound the playback will be stopped.

My personally recommendation is to use a std::map or std::unordered_map for the sound buffers (see also the resource holder implementation) and then use a std::deque for the sf::Sound. That way you can push_back new sounds to play and pop_front sounds that finished playing (assuming they're all of about the same short play time).

[winsett]

OK, I've found finally the bug. It was in how I get each element from the vector in the for each loop in the SoundsPlayer::paly() method:

                for (auto sound : m_sounds)
                {
                        if (sound.getStatus() != sf::SoundSource::Status::Playing)
                        {
                                sound.setBuffer(buffer);
                                sound.play();
                                soundPlayed = true;
                                break;
                        }
                }

where I have forgotten to get the elements by reference:
for (auto& sound : m_sounds)
but not
for (auto sound : m_sounds).

It is a dumb mistake I've made it in the past too and I shall finally take notes about 

One thing with using std::vector is that any push_back can cause a reallocation of all elements and change in memory address, which for sf::SoundBuffers will break the connection to the sf::Sound and for sf::Sound the playback will be stopped.

My personally recommendation is to use a std::map or std::unordered_map for the sound buffers (see also the resource holder implementation) and then use a std::deque for the sf::Sound. That way you can push_back new sounds to play and pop_front sounds that finished playing (assuming they're all of about the same short play time).

Yes, your point makes sense but for now the sounds in my game will not be a big number and I think the vector will not rearrange addresses. But I will take it into account in the future if a problem arises.

[kojack]

Yes, your point makes sense but for now the sounds in my game will not be a big number and I think the vector will not rearrange addresses.

Actually the fewer items in the vector the more often it rearranges memory.
In visual studio pushing 10 items into a vector will rearrange the entire vector's memory 6 times (it rearranges all items after pushes 2, 3, 4, 5, 7, 10). GCC has a slightly different pattern, but same idea.
You can call reserve() on the vector to give it a bigger capacity before it rearranges again, but that's just delaying potential trouble.
Or use a vector of sound buffer pointers instead of sound buffers and allocate them explicitly, then reallocating the vector is harmless since the buffers themselves are fixed on the heap.

[winsett]

Actually the fewer items in the vector the more often it rearranges memory.
In visual studio pushing 10 items into a vector will rearrange the entire vector's memory 6 times (it rearranges all items after pushes 2, 3, 4, 5, 7, 10). GCC has a slightly different pattern, but same idea.
You can call reserve() on the vector to give it a bigger capacity before it rearranges again, but that's just delaying potential trouble.
Or use a vector of sound buffer pointers instead of sound buffers and allocate them explicitly, then reallocating the vector is harmless since the buffers themselves are fixed on the heap.

Next step is to add more sounds to the game and I'll see if there will be issues with the vector... If there are any I'll take into account your advice.