Merge pull request #112 from scratchcpp/fix_loading_bugs

adazem009 · web-flow · commit 4c2f1aa056e3 · 2024-03-02T22:51:09.000+01:00
Fix loading bugs
diff --git a/src/ProjectPlayer.qml b/src/ProjectPlayer.qml
@@ -133,6 +133,7 @@ ProjectScene {
             id: projectPenLayer
             engine: loader.engine
             anchors.fill: parent
+            visible: !priv.loading
         }
 
         Component {
diff --git a/src/mouseeventhandler.cpp b/src/mouseeventhandler.cpp
@@ -113,6 +113,14 @@ void scratchcpprender::MouseEventHandler::getSprites()
         Q_ASSERT(sprite->scratchTarget());
         m_sprites.push_back(sprite);
     }
+
+    // Make sure the clicked and hovered item pointers are in the list
+    // If not, make them nullptr
+    if (std::find(m_sprites.begin(), m_sprites.end(), m_clickedItem) == m_sprites.end())
+        m_clickedItem = nullptr;
+
+    if (std::find(m_sprites.begin(), m_sprites.end(), m_hoveredItem) == m_sprites.end())
+        m_hoveredItem = nullptr;
 }
 
 void scratchcpprender::MouseEventHandler::addClone(SpriteModel *model)
diff --git a/src/projectloader.cpp b/src/projectloader.cpp
@@ -64,6 +64,46 @@ void ProjectLoader::setFileName(const QString &newFileName)
 
     m_fileName = newFileName;
 
+    // Stop the project
+    if (m_engine)
+        m_engine->stop();
+
+    // Reset stage model
+    m_stage.init(nullptr);
+
+    if (m_stage.renderedTarget())
+        m_stage.renderedTarget()->update();
+
+    // Delete old sprites
+    for (SpriteModel *sprite : m_sprites)
+        sprite->deleteLater();
+
+    m_sprites.clear();
+    emit spritesChanged();
+
+    // Delete old clones
+    for (SpriteModel *clone : m_clones)
+        deleteCloneObject(clone);
+
+    m_clones.clear();
+    emit clonesChanged();
+
+    // Delete old monitors
+    for (MonitorModel *monitor : m_monitors) {
+        emit monitorRemoved(monitor);
+        monitor->deleteLater();
+    }
+
+    m_monitors.clear();
+    emit monitorsChanged();
+
+    // Clear the engine
+    if (m_engine)
+        m_engine->clear();
+
+    m_engine = nullptr;
+    emit engineChanged();
+
     m_project.setScratchVersion(ScratchVersion::Scratch3);
     m_project.setFileName(m_fileName.toStdString());
     m_loadStatus = false;
@@ -198,12 +238,6 @@ void ProjectLoader::load()
     m_engineMutex.lock();
     m_engine = m_project.engine().get();
 
-    // Delete old sprites
-    for (SpriteModel *sprite : m_sprites)
-        sprite->deleteLater();
-
-    m_sprites.clear();
-
     if (!m_engine || m_stopLoading) {
         m_engineMutex.unlock();
         emit fileNameChanged();
@@ -313,13 +347,18 @@ void ProjectLoader::addClone(SpriteModel *model)
     emit clonesChanged();
 }
 
-void ProjectLoader::deleteClone(SpriteModel *model)
+void ProjectLoader::deleteCloneObject(SpriteModel *model)
 {
-    m_clones.removeAll(model);
     emit cloneDeleted(model);
     Q_ASSERT(model->renderedTarget());
     model->renderedTarget()->deinitClone();
     model->deleteLater();
+}
+
+void ProjectLoader::deleteClone(SpriteModel *model)
+{
+    m_clones.removeAll(model);
+    deleteCloneObject(model);
     emit clonesChanged();
 }
 
diff --git a/src/projectloader.h b/src/projectloader.h
@@ -120,6 +120,7 @@ class ProjectLoader : public QObject
         void initTimer();
         void redraw();
         void addClone(SpriteModel *model);
+        void deleteCloneObject(SpriteModel *model);
         void deleteClone(SpriteModel *model);
         void addMonitor(libscratchcpp::Monitor *monitor);
         void removeMonitor(libscratchcpp::Monitor *monitor, libscratchcpp::IMonitorHandler *iface);
diff --git a/src/renderedtarget.cpp b/src/renderedtarget.cpp
@@ -199,6 +199,22 @@ void RenderedTarget::setEngine(IEngine *newEngine)
         return;
 
     m_engine = newEngine;
+    m_costume = nullptr;
+    m_costumesLoaded = false;
+
+    if (!m_skinsInherited) {
+        for (const auto &[costume, skin] : m_skins)
+            delete skin;
+
+        m_skins.clear();
+    }
+
+    m_skin = nullptr;
+    m_texture = Texture();
+    m_oldTexture = Texture();
+    clearGraphicEffects();
+    m_hullPoints.clear();
+
     emit engineChanged();
 }
 
diff --git a/src/stagemodel.cpp b/src/stagemodel.cpp
@@ -79,10 +79,8 @@ void StageModel::onBubbleTextChanged(const std::string &text)
 
 void StageModel::loadCostume()
 {
-    if (m_renderedTarget && m_stage) {
-        if (m_stage)
-            m_renderedTarget->updateCostume(m_stage->currentCostume().get());
-    }
+    if (m_renderedTarget && m_stage)
+        m_renderedTarget->updateCostume(m_stage->currentCostume().get());
 }
 
 libscratchcpp::Stage *StageModel::stage() const
@@ -101,7 +99,6 @@ void StageModel::setRenderedTarget(IRenderedTarget *newRenderedTarget)
         return;
 
     m_renderedTarget = newRenderedTarget;
-    loadCostume();
 
     emit renderedTargetChanged();
 }
diff --git a/test/projectloader/projectloader_test.cpp b/test/projectloader/projectloader_test.cpp
@@ -29,6 +29,7 @@ class ProjectLoaderTest : public testing::Test
             QSignalSpy engineSpy(loader, &ProjectLoader::engineChanged);
             QSignalSpy stageSpy(loader, &ProjectLoader::stageChanged);
             QSignalSpy spritesSpy(loader, &ProjectLoader::spritesChanged);
+            QSignalSpy clonesSpy(loader, &ProjectLoader::clonesChanged);
             QSignalSpy monitorsSpy(loader, &ProjectLoader::monitorsChanged);
             QSignalSpy monitorAddedSpy(loader, &ProjectLoader::monitorAdded);
 
@@ -37,10 +38,11 @@ class ProjectLoaderTest : public testing::Test
             ASSERT_EQ(fileNameSpy.count(), 1);
             ASSERT_EQ(loadStatusSpy.count(), 1);
             ASSERT_TRUE(loadingFinishedSpy.empty());
-            ASSERT_TRUE(engineSpy.empty());
+            ASSERT_EQ(engineSpy.count(), 1);
             ASSERT_TRUE(stageSpy.empty());
-            ASSERT_TRUE(spritesSpy.empty());
-            ASSERT_TRUE(monitorsSpy.empty());
+            ASSERT_EQ(spritesSpy.count(), 1);
+            ASSERT_EQ(clonesSpy.count(), 1);
+            ASSERT_EQ(monitorsSpy.count(), 1);
             ASSERT_TRUE(monitorAddedSpy.empty());
             ASSERT_EQ(loader->fileName(), fileName);
             ASSERT_FALSE(loader->loadStatus());
@@ -52,10 +54,11 @@ class ProjectLoaderTest : public testing::Test
             ASSERT_EQ(fileNameSpy.count(), 1);
             ASSERT_EQ(loadStatusSpy.count(), 2);
             ASSERT_EQ(loadingFinishedSpy.count(), 1);
-            ASSERT_EQ(engineSpy.count(), 1);
+            ASSERT_EQ(engineSpy.count(), 2);
             ASSERT_EQ(stageSpy.count(), 1);
-            ASSERT_EQ(spritesSpy.count(), 1);
-            ASSERT_EQ(monitorsSpy.count(), loader->monitorList().size());
+            ASSERT_EQ(spritesSpy.count(), 2);
+            ASSERT_EQ(clonesSpy.count(), 1);
+            ASSERT_EQ(monitorsSpy.count(), loader->monitorList().size() + 1);
             ASSERT_EQ(monitorAddedSpy.count(), loader->monitorList().size());
         }
 };
@@ -113,13 +116,13 @@ TEST_F(ProjectLoaderTest, Clones)
     load(&loader, "clones.sb3");
     ASSERT_TRUE(cloneCreatedSpy.empty());
     ASSERT_TRUE(cloneDeletedSpy.empty());
-    ASSERT_TRUE(clonesChangedSpy.empty());
+    ASSERT_EQ(clonesChangedSpy.count(), 1);
 
     auto engine = loader.engine();
     engine->run();
     ASSERT_EQ(cloneCreatedSpy.count(), 3);
     ASSERT_EQ(cloneDeletedSpy.count(), 0);
-    ASSERT_EQ(clonesChangedSpy.count(), 3);
+    ASSERT_EQ(clonesChangedSpy.count(), 4);
 
     const auto &sprites = loader.spriteList();
     const auto &clones = loader.cloneList();
@@ -138,7 +141,7 @@ TEST_F(ProjectLoaderTest, Clones)
     clones[1]->sprite()->deleteClone();
     ASSERT_EQ(cloneCreatedSpy.count(), 3);
     ASSERT_EQ(cloneDeletedSpy.count(), 1);
-    ASSERT_EQ(clonesChangedSpy.count(), 4);
+    ASSERT_EQ(clonesChangedSpy.count(), 5);
     ASSERT_EQ(clones.size(), 2);
 }
 
diff --git a/test/renderedtarget/renderedtarget_test.cpp b/test/renderedtarget/renderedtarget_test.cpp
@@ -56,6 +56,8 @@ TEST_F(RenderedTargetTest, UpdateMethods)
     createContextAndSurface(&context, &surface);
     RenderedTarget parent; // a parent item is needed for setVisible() to work
     RenderedTarget target(&parent);
+    EngineMock engine;
+    target.setEngine(&engine);
     QSignalSpy mirrorHorizontallySpy(&target, &RenderedTarget::mirrorHorizontallyChanged);
     ASSERT_FALSE(target.costumesLoaded());
 
@@ -73,8 +75,6 @@ TEST_F(RenderedTargetTest, UpdateMethods)
     stage.addCostume(costume);
     target.loadCostumes();
     ASSERT_TRUE(target.costumesLoaded());
-    EngineMock engine;
-    target.setEngine(&engine);
 
     EXPECT_CALL(engine, stageWidth()).WillOnce(Return(480));
     EXPECT_CALL(engine, stageHeight()).WillOnce(Return(360));
diff --git a/test/target_models/stagemodel_test.cpp b/test/target_models/stagemodel_test.cpp
@@ -123,7 +123,6 @@ TEST(StageModelTest, RenderedTarget)
 
     RenderedTargetMock renderedTarget;
     QSignalSpy spy(&model, &StageModel::renderedTargetChanged);
-    EXPECT_CALL(renderedTarget, updateCostume(c2.get()));
     model.setRenderedTarget(&renderedTarget);
     ASSERT_EQ(spy.count(), 1);
     ASSERT_EQ(model.renderedTarget(), &renderedTarget);

Original file line number	Diff line number	Diff line change
`@@ -133,6 +133,7 @@ ProjectScene {`
`133`	`133`	`id: projectPenLayer`
`134`	`134`	`engine: loader.engine`
`135`	`135`	`anchors.fill: parent`
	`136`	`+ visible: !priv.loading`
`136`	`137`	`}`
`137`	`138`
`138`	`139`	`Component {`
Original file line number	Diff line number	Diff line change
`@@ -113,6 +113,14 @@ void scratchcpprender::MouseEventHandler::getSprites()`
`113`	`113`	`Q_ASSERT(sprite->scratchTarget());`
`114`	`114`	`m_sprites.push_back(sprite);`
`115`	`115`	`}`
	`116`	`+`
	`117`	`+ // Make sure the clicked and hovered item pointers are in the list`
	`118`	`+ // If not, make them nullptr`
	`119`	`+ if (std::find(m_sprites.begin(), m_sprites.end(), m_clickedItem) == m_sprites.end())`
	`120`	`+ m_clickedItem = nullptr;`
	`121`	`+`
	`122`	`+ if (std::find(m_sprites.begin(), m_sprites.end(), m_hoveredItem) == m_sprites.end())`
	`123`	`+ m_hoveredItem = nullptr;`
`116`	`124`	`}`
`117`	`125`
`118`	`126`	`void scratchcpprender::MouseEventHandler::addClone(SpriteModel *model)`
Original file line number	Diff line number	Diff line change
`@@ -79,10 +79,8 @@ void StageModel::onBubbleTextChanged(const std::string &text)`
`79`	`79`
`80`	`80`	`void StageModel::loadCostume()`
`81`	`81`	`{`
`82`		`- if (m_renderedTarget && m_stage) {`
`83`		`- if (m_stage)`
`84`		`- m_renderedTarget->updateCostume(m_stage->currentCostume().get());`
`85`		`- }`
	`82`	`+ if (m_renderedTarget && m_stage)`
	`83`	`+ m_renderedTarget->updateCostume(m_stage->currentCostume().get());`
`86`	`84`	`}`
`87`	`85`
`88`	`86`	`libscratchcpp::Stage *StageModel::stage() const`
`@@ -101,7 +99,6 @@ void StageModel::setRenderedTarget(IRenderedTarget *newRenderedTarget)`
`101`	`99`	`return;`
`102`	`100`
`103`	`101`	`m_renderedTarget = newRenderedTarget;`
`104`		`- loadCostume();`
`105`	`102`
`106`	`103`	`emit renderedTargetChanged();`
`107`	`104`	`}`