Drop Scratch version API

Resolves #496

Author: adazem009

include/scratchcpp/global.h

@@ -23,12 +23,6 @@
 namespace libscratchcpp
 {
 
-enum class ScratchVersion
-{
-    Invalid = 0, /*!< An unsupported version. */
-    Scratch3 = 3 /*!< Scratch 3.0 */
-};
-
 class VirtualMachine;
 class Compiler;
 class Block;

include/scratchcpp/project.h

@@ -26,7 +26,6 @@ class LIBSCRATCHCPP_EXPORT Project
     public:
         Project();
         Project(const std::string &fileName);
-        Project(const std::string &fileName, ScratchVersion scratchVersion);
         Project(const Project &) = delete;
 
         bool load();
@@ -38,9 +37,6 @@ class LIBSCRATCHCPP_EXPORT Project
         const std::string &fileName() const;
         void setFileName(const std::string &newFileName);
 
-        ScratchVersion scratchVersion() const;
-        void setScratchVersion(const ScratchVersion &version);
-
         std::shared_ptr<IEngine> engine() const;
 
         sigslot::signal<unsigned int, unsigned int> &downloadProgressChanged();

src/project.cpp

@@ -21,15 +21,6 @@ Project::Project(const std::string &fileName) :
 {
 }
 
-/*!
- * Constructs ScratchProjects with the given file name, for the given Scratch version.
- * \note You shouldn't normally use this constructor because the Scratch version is determined automatically.
- */
-Project::Project(const std::string &fileName, ScratchVersion scratchVersion) :
-    impl(spimpl::make_unique_impl<ProjectPrivate>(fileName, scratchVersion))
-{
-}
-
 /*! Loads the project. Returns true if successful. */
 bool Project::load()
 {
@@ -75,22 +66,6 @@ const std::string &Project::fileName() const
 void Project::setFileName(const std::string &newFileName)
 {
     impl->fileName = newFileName;
-    impl->detectScratchVersion();
-}
-
-/*! Returns the version of Scratch used for the project. */
-ScratchVersion Project::scratchVersion() const
-{
-    return impl->scratchVersion;
-}
-
-/*!
- * Sets the version of Scratch.
- * \note You shouldn't use this as the Scratch version is automatically determined.
- */
-void Project::setScratchVersion(const ScratchVersion &version)
-{
-    impl->setScratchVersion(version);
 }
 
 /*! Returns the engine of the loaded project. \see IEngine */

src/project_p.cpp

@@ -25,33 +25,28 @@ ProjectPrivate::ProjectPrivate() :
 }
 
 ProjectPrivate::ProjectPrivate(const std::string &fileName) :
-    ProjectPrivate()
-{
-    this->fileName = fileName;
-
-    // Auto detect Scratch version
-    detectScratchVersion();
-}
-
-ProjectPrivate::ProjectPrivate(const std::string &fileName, ScratchVersion scratchVersion) :
     fileName(fileName),
     engine(std::make_shared<Engine>())
 {
-    setScratchVersion(scratchVersion);
+    this->fileName = fileName;
 }
 
 bool ProjectPrivate::load()
 {
     std::shared_ptr<IProjectReader> reader;
-    switch (scratchVersion) {
-        case ScratchVersion::Invalid:
-            std::cerr << "Could not read the project because version is set to invalid." << std::endl;
-            return false;
-        case ScratchVersion::Scratch3:
-            reader = std::make_shared<Scratch3Reader>();
-            break;
-    }
 
+    // Scratch 3
+    reader = std::make_shared<Scratch3Reader>();
+
+    if (tryLoad(reader.get()))
+        return true;
+
+    std::cerr << "Unsupported Scratch version." << std::endl;
+    return false;
+}
+
+bool ProjectPrivate::tryLoad(IProjectReader *reader)
+{
     // Load from URL
     ProjectUrl url(fileName);
 
@@ -114,7 +109,6 @@ bool ProjectPrivate::load()
         // Load from file
         reader->setFileName(fileName);
         if (!reader->isValid()) {
-            scratchVersion = ScratchVersion::Invalid;
             std::cerr << "Could not read the project." << std::endl;
             return false;
         }
@@ -149,40 +143,6 @@ void ProjectPrivate::runEventLoop()
     engine->runEventLoop();
 }
 
-void ProjectPrivate::detectScratchVersion()
-{
-    ProjectUrl url(fileName);
-
-    scratchVersion = ScratchVersion::Invalid;
-    Scratch3Reader scratch3;
-
-    if (url.isProjectUrl()) {
-        if (!downloader->downloadJson(url.projectId())) {
-            std::cerr << "Failed to download the project file." << std::endl;
-            return;
-        }
-
-        scratch3.loadData(downloader->json());
-    } else
-        scratch3.setFileName(fileName);
-
-    if (scratch3.isValid())
-        scratchVersion = ScratchVersion::Scratch3;
-
-    if (scratchVersion == ScratchVersion::Invalid)
-        std::cerr << "Unable to determine Scratch version." << std::endl;
-}
-
-void ProjectPrivate::setScratchVersion(ScratchVersion version)
-{
-    // TODO: Use this when more versions become supported
-    // if((version >= Version::Scratch3) && (version <= Version::Scratch3))
-    if (version == ScratchVersion::Scratch3)
-        scratchVersion = version;
-    else
-        std::cerr << "Unsupported Scratch version: " << static_cast<int>(version) << std::endl;
-}
-
 sigslot::signal<unsigned int, unsigned int> &ProjectPrivate::downloadProgressChanged()
 {
     return downloader->downloadProgressChanged();

src/project_p.h

@@ -9,28 +9,25 @@ namespace libscratchcpp
 {
 
 class IEngine;
+class IProjectReader;
 class IProjectDownloaderFactory;
 class IProjectDownloader;
 
 struct ProjectPrivate
 {
         ProjectPrivate();
         ProjectPrivate(const std::string &fileName);
-        ProjectPrivate(const std::string &fileName, ScratchVersion scratchVersion);
         ProjectPrivate(const ProjectPrivate &) = delete;
 
         bool load();
+        bool tryLoad(IProjectReader *reader);
 
         void start();
         void run();
         void runEventLoop();
 
-        void detectScratchVersion();
-        void setScratchVersion(ScratchVersion version);
-
         sigslot::signal<unsigned int, unsigned int> &downloadProgressChanged();
 
-        ScratchVersion scratchVersion = ScratchVersion::Invalid;
         std::string fileName;
         std::shared_ptr<IEngine> engine = nullptr;
 

test/compiler/compiler_test.cpp

@@ -159,7 +159,7 @@ TEST_F(CompilerTest, Lists)
 
 TEST_F(CompilerTest, EmptyProject)
 {
-    ProjectPrivate p("empty_project.sb3", ScratchVersion::Scratch3);
+    ProjectPrivate p("empty_project.sb3");
     ASSERT_TRUE(p.load());
     auto engine = p.engine;
     ASSERT_EQ(engine->targets().size(), 1);
@@ -171,7 +171,7 @@ TEST_F(CompilerTest, EmptyProject)
 
 TEST_F(CompilerTest, ResolveIds)
 {
-    ProjectPrivate p("resolve_id_test.sb3", ScratchVersion::Scratch3);
+    ProjectPrivate p("resolve_id_test.sb3");
     ASSERT_TRUE(p.load());
     auto engine = p.engine;
     ASSERT_EQ(engine->targets().size(), 2);

test/load_project/load_project_test.cpp

@@ -22,7 +22,6 @@
 
 using namespace libscratchcpp;
 
-static const std::vector<ScratchVersion> scratchVersions = { ScratchVersion::Scratch3 };
 static const std::vector<std::string> fileExtensions = { ".sb3" };
 
 using ::testing::Return;
@@ -31,8 +30,8 @@ using ::testing::ReturnRef;
 TEST(LoadProjectTest, EmptyProject)
 {
     int i = 0;
-    for (auto version : scratchVersions) {
-        Project p("empty_project" + fileExtensions[i], version);
+    for (auto ext : fileExtensions) {
+        Project p("empty_project" + ext);
 
         ASSERT_TRUE(p.load());
 
@@ -99,11 +98,6 @@ TEST(LoadProjectTest, DownloadDefaultProject)
     };
     static const std::vector<std::string> assetData = { "a", "b", "c", "d", "e" };
 
-    EXPECT_CALL(*downloader, downloadJson("ABCDEFGH")).WillOnce(Return(true));
-    EXPECT_CALL(*downloader, json()).WillOnce(ReturnRef(data));
-    p.detectScratchVersion();
-    ASSERT_EQ(p.scratchVersion, ScratchVersion::Scratch3);
-
     EXPECT_CALL(*downloader, downloadJson("ABCDEFGH")).WillOnce(Return(true));
     EXPECT_CALL(*downloader, json()).WillOnce(ReturnRef(data));
     EXPECT_CALL(*downloader, downloadAssets(assets)).WillOnce(Return(true));
@@ -168,15 +162,8 @@ TEST(LoadProjectTest, DownloadDefaultProjectFailure)
     static const std::vector<std::string> assetData = { "a", "b", "c", "d", "e" };
 
     EXPECT_CALL(*downloader, downloadJson("ABCDEFGH")).WillOnce(Return(false));
-    p.detectScratchVersion();
-    ASSERT_EQ(p.scratchVersion, ScratchVersion::Invalid);
     ASSERT_FALSE(p.load());
 
-    EXPECT_CALL(*downloader, downloadJson("ABCDEFGH")).WillOnce(Return(true));
-    EXPECT_CALL(*downloader, json()).WillOnce(ReturnRef(data));
-    p.detectScratchVersion();
-    ASSERT_EQ(p.scratchVersion, ScratchVersion::Scratch3);
-
     EXPECT_CALL(*downloader, downloadJson("ABCDEFGH")).WillOnce(Return(false));
     ASSERT_FALSE(p.load());
 
@@ -195,8 +182,8 @@ TEST(LoadProjectTest, DownloadDefaultProjectFailure)
 TEST(LoadProjectTest, LoadTestProject)
 {
     int i = 0;
-    for (auto version : scratchVersions) {
-        Project p("load_test" + fileExtensions[i], version);
+    for (auto ext : fileExtensions) {
+        Project p("load_test" + ext);
 
         ASSERT_TRUE(p.load());
 
@@ -647,8 +634,8 @@ TEST(LoadProjectTest, LoadTestProject)
 TEST(LoadProjectTest, LoadTopLevelReporterProject)
 {
     int i = 0;
-    for (auto version : scratchVersions) {
-        Project p("top_level_reporter" + fileExtensions[i], version);
+    for (auto ext : fileExtensions) {
+        Project p("top_level_reporter" + ext);
 
         ASSERT_TRUE(p.load());
 
@@ -688,11 +675,10 @@ TEST(LoadProjectTest, LoadTopLevelReporterProject)
 TEST(LoadProjectTest, ProjectTest)
 {
     int i = 0;
-    for (auto version : scratchVersions) {
-        std::string name = "load_test" + fileExtensions[i];
-        Project p(name, version);
+    for (auto ext : fileExtensions) {
+        std::string name = "load_test" + ext;
+        Project p(name);
         ASSERT_EQ(p.fileName(), name);
-        ASSERT_EQ(p.scratchVersion(), version);
         ASSERT_TRUE(p.load());
 
         auto engine = p.engine();
@@ -709,15 +695,6 @@ TEST(LoadProjectTest, ProjectTest)
     }
 }
 
-TEST(LoadProjectTest, ProjectInvalidTest)
-{
-    std::string name = "load_test.sb3";
-    Project p(name, ScratchVersion::Invalid);
-    ASSERT_EQ(p.fileName(), name);
-    ASSERT_EQ(p.scratchVersion(), ScratchVersion::Invalid);
-    ASSERT_FALSE(p.load());
-}
-
 TEST(LoadProjectTest, LoadNullDimensionMonitor)
 {
     // Regtest for #448