Remove Sound default constructor

so that a Sound instance is always bound to a SoundBuffer

include/SFML/Audio/Sound.hpp

@@ -46,12 +46,6 @@ class SoundBuffer;
 class SFML_AUDIO_API Sound : public SoundSource
 {
 public:
-    ////////////////////////////////////////////////////////////
-    /// \brief Default constructor
-    ///
-    ////////////////////////////////////////////////////////////
-    Sound();
-
     ////////////////////////////////////////////////////////////
     /// \brief Construct the sound with a buffer
     ///
@@ -213,18 +207,27 @@ public:
     ////////////////////////////////////////////////////////////
     Sound& operator=(const Sound& right);
 
-    ////////////////////////////////////////////////////////////
-    /// \brief Reset the internal buffer of the sound
-    ///
-    /// This function is for internal use only, you don't have
-    /// to use it. It is called by the sf::SoundBuffer that
-    /// this sound uses, when it is destroyed in order to prevent
-    /// the sound from using a dead buffer.
-    ///
-    ////////////////////////////////////////////////////////////
-    void resetBuffer();
-
 private:
+    friend class SoundBuffer;
+
+    ////////////////////////////////////////////////////////////
+    /// \brief Detach sound from its internal buffer
+    ///
+    /// This allows the sound buffer to temporarily detach the
+    /// sounds that use it when the sound buffer gets updated.
+    ///
+    ////////////////////////////////////////////////////////////
+    void detachBuffer();
+
+    ////////////////////////////////////////////////////////////
+    /// \brief Re-attach sound to its internal buffer
+    ///
+    /// This allows the sound buffer to attach back the sounds
+    /// that use it after the sound buffer has been updated.
+    ///
+    ////////////////////////////////////////////////////////////
+    void reattachBuffer();
+
     ////////////////////////////////////////////////////////////
     // Member data
     ////////////////////////////////////////////////////////////
@@ -252,7 +255,7 @@ private:
 ///
 /// In order to work, a sound must be given a buffer of audio
 /// data to play. Audio data (samples) is stored in sf::SoundBuffer,
-/// and attached to a sound with the setBuffer() function.
+/// and attached to a sound when it is created or with the setBuffer() function.
 /// The buffer object attached to a sound must remain alive
 /// as long as the sound uses it. Note that multiple sounds
 /// can use the same sound buffer at the same time.
@@ -265,8 +268,7 @@ private:
 ///     // Handle error...
 /// }
 ///
-/// sf::Sound sound;
+/// sf::Sound sound(buffer);
-/// sound.setBuffer(buffer);
 /// sound.play();
 /// \endcode
 ///

include/SFML/Audio/SoundBuffer.hpp

@@ -331,16 +331,14 @@ private:
 ///     // error...
 /// }
 ///
-/// // Create a sound source and bind it to the buffer
+/// // Create a sound source bound to the buffer
-/// sf::Sound sound1;
+/// sf::Sound sound1(buffer);
-/// sound1.setBuffer(buffer);
 ///
 /// // Play the sound
 /// sound1.play();
 ///
 /// // Create another sound source bound to the same buffer
-/// sf::Sound sound2;
+/// sf::Sound sound2(buffer);
-/// sound2.setBuffer(buffer);
 ///
 /// // Play it with a higher pitch -- the first sound remains unchanged
 /// sound2.setPitch(2);

src/SFML/Audio/Sound.cpp

@@ -36,21 +36,18 @@
 namespace sf
 {
 ////////////////////////////////////////////////////////////
-Sound::Sound() = default;
+Sound::Sound(const SoundBuffer& buffer) : m_buffer(&buffer)
-
-
-////////////////////////////////////////////////////////////
-Sound::Sound(const SoundBuffer& buffer)
 {
-    setBuffer(buffer);
+    m_buffer->attachSound(this);
+    alCheck(alSourcei(m_source, AL_BUFFER, static_cast<ALint>(m_buffer->m_buffer)));
 }
 
 
 ////////////////////////////////////////////////////////////
-Sound::Sound(const Sound& copy) : SoundSource(copy)
+Sound::Sound(const Sound& copy) : SoundSource(copy), m_buffer(copy.m_buffer)
 {
-    if (copy.m_buffer)
+    m_buffer->attachSound(this);
-        setBuffer(*copy.m_buffer);
+    alCheck(alSourcei(m_source, AL_BUFFER, static_cast<ALint>(m_buffer->m_buffer)));
     setLoop(copy.getLoop());
 }
 
@@ -59,8 +56,7 @@ Sound::Sound(const Sound& copy) : SoundSource(copy)
 Sound::~Sound()
 {
     stop();
-    if (m_buffer)
+    m_buffer->detachSound(this);
-        m_buffer->detachSound(this);
 }
 
 
@@ -89,11 +85,8 @@ void Sound::stop()
 void Sound::setBuffer(const SoundBuffer& buffer)
 {
     // First detach from the previous buffer
-    if (m_buffer)
+    stop();
-    {
+    m_buffer->detachSound(this);
-        stop();
-        m_buffer->detachSound(this);
-    }
 
     // Assign and use the new buffer
     m_buffer = &buffer;
@@ -164,17 +157,8 @@ Sound& Sound::operator=(const Sound& right)
     // Delegate to base class, which copies all the sound attributes
     SoundSource::operator=(right);
 
-    // Detach the sound instance from the previous buffer (if any)
-    if (m_buffer)
-    {
-        stop();
-        m_buffer->detachSound(this);
-        m_buffer = nullptr;
-    }
-
     // Copy the remaining sound attributes
-    if (right.m_buffer)
+    setBuffer(*right.m_buffer);
-        setBuffer(*right.m_buffer);
     setLoop(right.getLoop());
 
     return *this;
@@ -182,18 +166,20 @@ Sound& Sound::operator=(const Sound& right)
 
 
 ////////////////////////////////////////////////////////////
-void Sound::resetBuffer()
+void Sound::detachBuffer()
 {
     // First stop the sound in case it is playing
     stop();
 
     // Detach the buffer
-    if (m_buffer)
+    alCheck(alSourcei(m_source, AL_BUFFER, 0));
-    {
+}
-        alCheck(alSourcei(m_source, AL_BUFFER, 0));
+
-        m_buffer->detachSound(this);
+
-        m_buffer = nullptr;
+////////////////////////////////////////////////////////////
-    }
+void Sound::reattachBuffer()
+{
+    alCheck(alSourcei(m_source, AL_BUFFER, static_cast<ALint>(m_buffer->m_buffer)));
 }
 
 } // namespace sf

src/SFML/Audio/SoundBuffer.cpp

@@ -38,6 +38,8 @@
 #include <memory>
 #include <ostream>
 
+#include <cassert>
+
 #if defined(__APPLE__)
 #pragma GCC diagnostic ignored "-Wdeprecated-declarations"
 #endif
@@ -68,15 +70,7 @@ SoundBuffer::SoundBuffer(const SoundBuffer& copy) : m_samples(copy.m_samples), m
 ////////////////////////////////////////////////////////////
 SoundBuffer::~SoundBuffer()
 {
-    // To prevent the iterator from becoming invalid, move the entire buffer to another
+    assert(m_sounds.empty() && "sf::SoundBuffer must not be destructed while it is used by a sf::Sound");
-    // container. Otherwise calling resetBuffer would result in detachSound being
-    // called which removes the sound from the internal list.
-    SoundList sounds;
-    sounds.swap(m_sounds);
-
-    // Detach the buffer from the sounds that use it (to avoid OpenAL errors)
-    for (Sound* soundPtr : sounds)
-        soundPtr->resetBuffer();
 
     // Destroy the buffer
     if (m_buffer)
@@ -213,7 +207,13 @@ SoundBuffer& SoundBuffer::operator=(const SoundBuffer& right)
     std::swap(m_samples, temp.m_samples);
     std::swap(m_buffer, temp.m_buffer);
     std::swap(m_duration, temp.m_duration);
-    std::swap(m_sounds, temp.m_sounds); // swap sounds too, so that they are detached when temp is destroyed
+
+    // Reattach sounds that use this buffer so they get bound to the new OpenAL buffer
+    for (Sound* soundPtr : m_sounds)
+    {
+        soundPtr->stop();
+        soundPtr->reattachBuffer();
+    }
 
     return *this;
 }
@@ -258,12 +258,9 @@ bool SoundBuffer::update(unsigned int channelCount, unsigned int sampleRate)
         return false;
     }
 
-    // First make a copy of the list of sounds so we can reattach later
-    const SoundList sounds(m_sounds);
-
     // Detach the buffer from the sounds that use it (to avoid OpenAL errors)
-    for (Sound* soundPtr : sounds)
+    for (Sound* soundPtr : m_sounds)
-        soundPtr->resetBuffer();
+        soundPtr->detachBuffer();
 
     // Fill the buffer
     const auto size = static_cast<ALsizei>(m_samples.size() * sizeof(std::int16_t));
@@ -274,8 +271,8 @@ bool SoundBuffer::update(unsigned int channelCount, unsigned int sampleRate)
         static_cast<float>(m_samples.size()) / static_cast<float>(sampleRate) / static_cast<float>(channelCount));
 
     // Now reattach the buffer to the sounds that use it
-    for (Sound* soundPtr : sounds)
+    for (Sound* soundPtr : m_sounds)
-        soundPtr->setBuffer(*this);
+        soundPtr->reattachBuffer();
 
     return true;
 }

test/install/Install.cpp

@@ -11,7 +11,6 @@ int main()
     [[maybe_unused]] const sf::InputSoundFile      inputSoundFile;
     [[maybe_unused]] const sf::SoundBufferRecorder soundBufferRecorder;
     [[maybe_unused]] const sf::Music               music;
-    [[maybe_unused]] const sf::Sound               sound;
 
     // Graphics
     [[maybe_unused]] const sf::Color          color;