Replace download progress callback with a signal

Author: adazem009

include/scratchcpp/project.h

@@ -7,6 +7,7 @@
 #include "spimpl.h"
 
 #include "global.h"
+#include "signal.h"
 
 namespace libscratchcpp
 {
@@ -42,7 +43,7 @@ class LIBSCRATCHCPP_EXPORT Project
 
         std::shared_ptr<IEngine> engine() const;
 
-        void setDownloadProgressCallback(const std::function<void(unsigned int, unsigned int)> &&f);
+        sigslot::signal<unsigned int, unsigned int> &downloadProgressChanged();
 
     private:
         spimpl::unique_impl_ptr<ProjectPrivate> impl;

src/internal/iprojectdownloader.h

@@ -5,6 +5,7 @@
 #include <string>
 #include <vector>
 #include <functional>
+#include <scratchcpp/signal.h>
 
 namespace libscratchcpp
 {
@@ -18,7 +19,7 @@ class IProjectDownloader
         virtual bool downloadAssets(const std::vector<std::string> &assetIds) = 0;
         virtual void cancel() = 0;
 
-        virtual void setDownloadProgressCallback(const std::function<void(unsigned int, unsigned int)> &f) = 0;
+        virtual sigslot::signal<unsigned int, unsigned int> &downloadProgressChanged() = 0;
 
         virtual const std::string &json() const = 0;
         virtual const std::vector<std::string> &assets() const = 0;

src/internal/projectdownloader.cpp

@@ -149,13 +149,7 @@ bool ProjectDownloader::downloadAssets(const std::vector<std::string> &assetIds)
                 m_assets[index] = downloader->text();
                 m_downloadedAssetCount++;
                 std::cout << "Downloaded assets: " << m_downloadedAssetCount << " of " << count << std::endl;
-
-                m_downloadProgressCallbackMutex.lock();
-
-                if (m_downloadProgressCallback)
-                    m_downloadProgressCallback(m_downloadedAssetCount, count);
-
-                m_downloadProgressCallbackMutex.unlock();
+                m_downloadProgressChanged(m_downloadedAssetCount, count);
                 m_assetsMutex.unlock();
             }
         }
@@ -182,11 +176,9 @@ void ProjectDownloader::cancel()
     m_downloadedAssetCount = 0;
 }
 
-void ProjectDownloader::setDownloadProgressCallback(const std::function<void(unsigned int, unsigned int)> &f)
+sigslot::signal<unsigned int, unsigned int> &ProjectDownloader::downloadProgressChanged()
 {
-    m_downloadProgressCallbackMutex.lock();
-    m_downloadProgressCallback = f;
-    m_downloadProgressCallbackMutex.unlock();
+    return m_downloadProgressChanged;
 }
 
 const std::string &ProjectDownloader::json() const

src/internal/projectdownloader.h

@@ -23,7 +23,7 @@ class ProjectDownloader : public IProjectDownloader
         bool downloadAssets(const std::vector<std::string> &assetIds) override;
         void cancel() override;
 
-        void setDownloadProgressCallback(const std::function<void(unsigned int, unsigned int)> &f) override;
+        sigslot::signal<unsigned int, unsigned int> &downloadProgressChanged() override;
 
         const std::string &json() const override;
         const std::vector<std::string> &assets() const override;
@@ -38,8 +38,7 @@ class ProjectDownloader : public IProjectDownloader
         std::atomic<unsigned int> m_downloadedAssetCount = 0;
         bool m_cancel = false;
         std::mutex m_cancelMutex;
-        std::function<void(unsigned int, unsigned int)> m_downloadProgressCallback = nullptr;
-        std::mutex m_downloadProgressCallbackMutex;
+        sigslot::signal<unsigned int, unsigned int> m_downloadProgressChanged;
 };
 
 } // namespace libscratchcpp

src/project.cpp

@@ -100,10 +100,10 @@ std::shared_ptr<IEngine> Project::engine() const
 }
 
 /*!
- * Sets the function which will be called when the asset download progress changes.
+ * Emits when the asset download progress changes.
  * \note The first parameter is the number of downloaded assets and the latter is the number of all assets to download.
  */
-void Project::setDownloadProgressCallback(const std::function<void(unsigned int, unsigned int)> &&f)
+sigslot::signal<unsigned int, unsigned int> &Project::downloadProgressChanged()
 {
-    impl->setDownloadProgressCallback(f);
+    return impl->downloadProgressChanged();
 }

src/project_p.cpp

@@ -183,7 +183,7 @@ void ProjectPrivate::setScratchVersion(ScratchVersion version)
         std::cerr << "Unsupported Scratch version: " << static_cast<int>(version) << std::endl;
 }
 
-void ProjectPrivate::setDownloadProgressCallback(const std::function<void(unsigned int, unsigned int)> &f)
+sigslot::signal<unsigned int, unsigned int> &ProjectPrivate::downloadProgressChanged()
 {
-    downloader->setDownloadProgressCallback(f);
+    return downloader->downloadProgressChanged();
 }

src/project_p.h

@@ -28,7 +28,7 @@ struct ProjectPrivate
         void detectScratchVersion();
         void setScratchVersion(ScratchVersion version);
 
-        void setDownloadProgressCallback(const std::function<void(unsigned int, unsigned int)> &f);
+        sigslot::signal<unsigned int, unsigned int> &downloadProgressChanged();
 
         ScratchVersion scratchVersion = ScratchVersion::Invalid;
         std::string fileName;

test/mocks/projectdownloadermock.h

@@ -12,7 +12,7 @@ class ProjectDownloaderMock : public IProjectDownloader
         MOCK_METHOD(bool, downloadAssets, (const std::vector<std::string> &), (override));
         MOCK_METHOD(void, cancel, (), (override));
 
-        MOCK_METHOD(void, setDownloadProgressCallback, (const std::function<void(unsigned int, unsigned int)> &), (override));
+        MOCK_METHOD((sigslot::signal<unsigned int, unsigned int> &), downloadProgressChanged, (), (override));
 
         MOCK_METHOD(const std::string &, json, (), (const, override));
         MOCK_METHOD(const std::vector<std::string> &, assets, (), (const, override));

test/project/project_test.cpp

@@ -9,6 +9,7 @@
 using namespace libscratchcpp;
 
 using ::testing::Return;
+using ::testing::ReturnRef;
 
 class ProjectTest : public testing::Test
 {
@@ -158,7 +159,7 @@ TEST_F(ProjectTest, ScratchVersion)
     ASSERT_EQ(p.scratchVersion(), ScratchVersion::Scratch3);
 }
 
-TEST(LoadProjectTest, DownloadProgressCallback)
+TEST(LoadProjectTest, DownloadProgressChanged)
 {
     ProjectDownloaderFactoryMock factory;
     auto downloader = std::make_shared<ProjectDownloaderMock>();
@@ -168,8 +169,7 @@ TEST(LoadProjectTest, DownloadProgressCallback)
     ProjectPrivate p;
     ProjectPrivate::downloaderFactory = nullptr;
 
-    auto lambda = [](unsigned int, unsigned int) {};
-    // TODO: Check the function parameter, if possible (std::function doesn't have operator== for this)
-    EXPECT_CALL(*downloader, setDownloadProgressCallback);
-    p.setDownloadProgressCallback(lambda);
+    sigslot::signal<unsigned int, unsigned int> signal;
+    EXPECT_CALL(*downloader, downloadProgressChanged).WillOnce(ReturnRef(signal));
+    ASSERT_EQ(&p.downloadProgressChanged(), &signal);
 }