Add move semantics to 'AlResource'

include/SFML/Audio/AlResource.hpp

@@ -31,6 +31,15 @@
 #include <SFML/Audio/Export.hpp>
 
 
+namespace sf::priv
+{
+////////////////////////////////////////////////////////////
+/// \brief Return OpenAL resource count
+///
+////////////////////////////////////////////////////////////
+SFML_AUDIO_API unsigned int getOpenAlResourceCount();
+}
+
 namespace sf
 {
 ////////////////////////////////////////////////////////////
@@ -52,6 +61,48 @@ protected:
     ///
     ////////////////////////////////////////////////////////////
     ~AlResource();
+
+    ////////////////////////////////////////////////////////////
+    /// \brief Copy constructor
+    ///
+    ////////////////////////////////////////////////////////////
+    AlResource(const AlResource& other);
+
+    ////////////////////////////////////////////////////////////
+    /// \brief Move constructor
+    ///
+    ////////////////////////////////////////////////////////////
+    AlResource(AlResource&& other) noexcept;
+
+    ////////////////////////////////////////////////////////////
+    /// \brief Copy assignment operator
+    ///
+    ////////////////////////////////////////////////////////////
+    AlResource& operator=(const AlResource& other);
+
+    ////////////////////////////////////////////////////////////
+    /// \brief Move assignment operator
+    ///
+    ////////////////////////////////////////////////////////////
+    AlResource& operator=(AlResource&& other) noexcept;
+
+private:
+    ////////////////////////////////////////////////////////////
+    /// \brief Increment resource counter
+    ///
+    ////////////////////////////////////////////////////////////
+    void incrementResourceCounter();
+
+    ////////////////////////////////////////////////////////////
+    /// \brief Decrement resource counter
+    ///
+    ////////////////////////////////////////////////////////////
+    void decrementResourceCounter();
+
+    ////////////////////////////////////////////////////////////
+    // Member data
+    ////////////////////////////////////////////////////////////
+    bool m_active; //!< Whether the resource is active or not
 };
 
 } // namespace sf

src/SFML/Audio/AlResource.cpp

@@ -29,11 +29,13 @@
 #include <SFML/Audio/AudioDevice.hpp>
 #include <memory>
 #include <mutex>
+#include <utility>
+#include <cassert>
 
 
 namespace
 {
-    // OpenAL resources counter and its mutex
+    // OpenAL resource counter and its mutex
     unsigned int count = 0;
     std::recursive_mutex mutex;
 
@@ -44,10 +46,122 @@ namespace
 }
 
 
+namespace sf::priv
+{
+////////////////////////////////////////////////////////////
+/// \brief Return OpenAL resource count
+///
+////////////////////////////////////////////////////////////
+unsigned int getOpenAlResourceCount()
+{
+    std::scoped_lock lock(mutex);
+    return count;
+}
+
+}
+
+
 namespace sf
 {
 ////////////////////////////////////////////////////////////
-AlResource::AlResource()
+AlResource::AlResource() :
+m_active(true)
+{
+    incrementResourceCounter();
+}
+
+
+////////////////////////////////////////////////////////////
+AlResource::~AlResource()
+{
+    if (m_active)
+        decrementResourceCounter();
+}
+
+
+////////////////////////////////////////////////////////////
+AlResource::AlResource(const AlResource& other) :
+m_active(other.m_active)
+{
+    if (m_active)
+        incrementResourceCounter();
+}
+
+
+////////////////////////////////////////////////////////////
+AlResource::AlResource(AlResource&& other) noexcept :
+m_active(std::exchange(other.m_active, false))
+{
+}
+
+
+////////////////////////////////////////////////////////////
+AlResource& AlResource::operator=(const AlResource& other)
+{
+    if (&other == this)
+        return *this;
+
+    if (m_active && other.m_active)
+    {
+        // Both resources are already accounted for, nothing to do
+    }
+    else if (!m_active && other.m_active)
+    {
+        // '*this' was previously moved-from
+        m_active = true;
+        incrementResourceCounter();
+    }
+    else if (m_active && !other.m_active)
+    {
+        // 'other' was previously moved-from
+        m_active = false;
+        decrementResourceCounter();
+    }
+    else
+    {
+        // Both resources are inactive, nothing to do
+        assert(!m_active && !other.m_active);
+    }
+
+    return *this;
+}
+
+
+////////////////////////////////////////////////////////////
+AlResource& AlResource::operator=(AlResource&& other) noexcept
+{
+    if (&other == this)
+        return *this;
+
+    if (m_active && other.m_active)
+    {
+        other.m_active = false;
+        decrementResourceCounter();
+    }
+    else if (!m_active && other.m_active)
+    {
+        // '*this' was previously moved-from
+        m_active = true;
+        other.m_active = false;
+    }
+    else if (m_active && !other.m_active)
+    {
+        // 'other' was previously moved-from
+        m_active = false;
+        decrementResourceCounter();
+    }
+    else
+    {
+        // Both resources are inactive, nothing to do
+        assert(!m_active && !other.m_active);
+    }
+
+    return *this;
+}
+
+
+////////////////////////////////////////////////////////////
+void AlResource::incrementResourceCounter()
 {
     // Protect from concurrent access
     std::scoped_lock lock(mutex);
@@ -57,20 +171,20 @@ AlResource::AlResource()
         globalDevice = std::make_unique<priv::AudioDevice>();
 
     // Increment the resources counter
-    count++;
+    ++count;
 }
 
 
 ////////////////////////////////////////////////////////////
-AlResource::~AlResource()
+void AlResource::decrementResourceCounter()
 {
     // Protect from concurrent access
     std::scoped_lock lock(mutex);
 
     // Decrement the resources counter
-    count--;
+    --count;
 
-    // If there's no more resource alive, we can destroy the device
+    // If there are no more resources alive, we can destroy the device
     if (count == 0)
         globalDevice.reset();
 }

test/Audio/AlResource.cpp

@@ -0,0 +1,217 @@
+#include <SFML/Audio/AlResource.hpp>
+#include <utility>
+
+#include <doctest.h>
+
+namespace {
+
+struct TestAlResource : sf::AlResource { };
+
+}
+
+TEST_CASE("sf::AlResource class - [audio]")
+{
+    SUBCASE("Never used")
+    {
+        CHECK(sf::priv::getOpenAlResourceCount() == 0u);
+    }
+
+    SUBCASE("Basic ctor/dtor")
+    {
+        CHECK(sf::priv::getOpenAlResourceCount() == 0u);
+
+        {
+            TestAlResource r0;
+            CHECK(sf::priv::getOpenAlResourceCount() == 1u);
+        }
+
+        CHECK(sf::priv::getOpenAlResourceCount() == 0u);
+    }
+
+    SUBCASE("Nested ctor/dtor")
+    {
+        CHECK(sf::priv::getOpenAlResourceCount() == 0u);
+
+        {
+            TestAlResource r0;
+            CHECK(sf::priv::getOpenAlResourceCount() == 1u);
+
+            {
+                TestAlResource r1;
+                CHECK(sf::priv::getOpenAlResourceCount() == 2u);
+            }
+
+            CHECK(sf::priv::getOpenAlResourceCount() == 1u);
+        }
+
+        CHECK(sf::priv::getOpenAlResourceCount() == 0u);
+    }
+
+    SUBCASE("Copy ctor")
+    {
+        CHECK(sf::priv::getOpenAlResourceCount() == 0u);
+
+        {
+            TestAlResource r0;
+            CHECK(sf::priv::getOpenAlResourceCount() == 1u);
+
+            {
+                TestAlResource r1 = r0;
+                CHECK(sf::priv::getOpenAlResourceCount() == 2u);
+            }
+
+            CHECK(sf::priv::getOpenAlResourceCount() == 1u);
+        }
+
+        CHECK(sf::priv::getOpenAlResourceCount() == 0u);
+    }
+
+    SUBCASE("Move ctor")
+    {
+        CHECK(sf::priv::getOpenAlResourceCount() == 0u);
+
+        {
+            TestAlResource r0;
+            CHECK(sf::priv::getOpenAlResourceCount() == 1u);
+
+            {
+                TestAlResource r1 = std::move(r0);
+                CHECK(sf::priv::getOpenAlResourceCount() == 1u);
+            }
+
+            CHECK(sf::priv::getOpenAlResourceCount() == 0u);
+        }
+
+        CHECK(sf::priv::getOpenAlResourceCount() == 0u);
+    }
+
+    SUBCASE("Copy assignment from active to active")
+    {
+        CHECK(sf::priv::getOpenAlResourceCount() == 0u);
+
+        {
+            TestAlResource r0;
+            CHECK(sf::priv::getOpenAlResourceCount() == 1u);
+
+            TestAlResource r1;
+            CHECK(sf::priv::getOpenAlResourceCount() == 2u);
+
+            r1 = r0;
+            CHECK(sf::priv::getOpenAlResourceCount() == 2u);
+        }
+
+        CHECK(sf::priv::getOpenAlResourceCount() == 0u);
+    }
+
+    SUBCASE("Copy assignment from inactive to active")
+    {
+        CHECK(sf::priv::getOpenAlResourceCount() == 0u);
+
+        {
+            TestAlResource r0;
+            CHECK(sf::priv::getOpenAlResourceCount() == 1u);
+
+            TestAlResource r1 = std::move(r0);
+            CHECK(sf::priv::getOpenAlResourceCount() == 1u);
+
+            r1 = r0;
+            CHECK(sf::priv::getOpenAlResourceCount() == 0u);
+        }
+
+        CHECK(sf::priv::getOpenAlResourceCount() == 0u);
+    }
+
+    SUBCASE("Move assignment from active to active")
+    {
+        CHECK(sf::priv::getOpenAlResourceCount() == 0u);
+
+        {
+            TestAlResource r0;
+            CHECK(sf::priv::getOpenAlResourceCount() == 1u);
+
+            TestAlResource r1;
+            CHECK(sf::priv::getOpenAlResourceCount() == 2u);
+
+            r1 = std::move(r0);
+            CHECK(sf::priv::getOpenAlResourceCount() == 1u);
+        }
+
+        CHECK(sf::priv::getOpenAlResourceCount() == 0u);
+    }
+
+    SUBCASE("Move assignment from inactive to active")
+    {
+        CHECK(sf::priv::getOpenAlResourceCount() == 0u);
+
+        {
+            TestAlResource r0;
+            CHECK(sf::priv::getOpenAlResourceCount() == 1u);
+
+            TestAlResource r1 = std::move(r0);
+            CHECK(sf::priv::getOpenAlResourceCount() == 1u);
+
+            r1 = std::move(r0);
+            CHECK(sf::priv::getOpenAlResourceCount() == 0u);
+        }
+
+        CHECK(sf::priv::getOpenAlResourceCount() == 0u);
+    }
+
+    SUBCASE("Copy assignment from active to inactive")
+    {
+        CHECK(sf::priv::getOpenAlResourceCount() == 0u);
+
+        {
+            TestAlResource r0;
+            CHECK(sf::priv::getOpenAlResourceCount() == 1u);
+
+            TestAlResource r1 = std::move(r0);
+            CHECK(sf::priv::getOpenAlResourceCount() == 1u);
+
+            r1 = r0;
+            CHECK(sf::priv::getOpenAlResourceCount() == 0u);
+        }
+
+        CHECK(sf::priv::getOpenAlResourceCount() == 0u);
+    }
+
+    SUBCASE("Move assignment from active to inactive")
+    {
+        CHECK(sf::priv::getOpenAlResourceCount() == 0u);
+
+        {
+            TestAlResource r0;
+            CHECK(sf::priv::getOpenAlResourceCount() == 1u);
+
+            TestAlResource r1 = std::move(r0);
+            CHECK(sf::priv::getOpenAlResourceCount() == 1u);
+
+            r1 = std::move(r0);
+            CHECK(sf::priv::getOpenAlResourceCount() == 0u);
+        }
+
+        CHECK(sf::priv::getOpenAlResourceCount() == 0u);
+    }
+
+    SUBCASE("Copy assignment from inactive to inactive")
+    {
+        CHECK(sf::priv::getOpenAlResourceCount() == 0u);
+
+        {
+            TestAlResource r0;
+            TestAlResource r1;
+            CHECK(sf::priv::getOpenAlResourceCount() == 2u);
+
+            TestAlResource r2 = std::move(r0);
+            CHECK(sf::priv::getOpenAlResourceCount() == 2u);
+
+            TestAlResource r3 = std::move(r1);
+            CHECK(sf::priv::getOpenAlResourceCount() == 2u);
+
+            r0 = r1;
+            CHECK(sf::priv::getOpenAlResourceCount() == 2u);
+        }
+
+        CHECK(sf::priv::getOpenAlResourceCount() == 0u);
+    }
+}

test/CMakeLists.txt

@@ -53,7 +53,7 @@ endif()
 
 if(SFML_BUILD_AUDIO)
     SET(AUDIO_SRC
-        "${SRCROOT}/Audio/Dummy.cpp" # TODO: Remove when there are real tests
+        "${SRCROOT}/Audio/AlResource.cpp"
     )
     sfml_add_test(test-sfml-audio "${AUDIO_SRC}" SFML::Audio)
     target_link_libraries(test-sfml-audio PRIVATE sfml-test-main)