From 91de47f293e9eb315e9701d1f7a4443b2790f6fd Mon Sep 17 00:00:00 2001 From: adazem009 <68537469+adazem009@users.noreply.github.com> Date: Sat, 2 Mar 2024 21:51:44 +0100 Subject: [PATCH 1/6] fix #104: Properly clean up before loading another project --- src/projectloader.cpp | 49 +++++++++++++++++++---- src/projectloader.h | 1 + test/projectloader/projectloader_test.cpp | 21 +++++----- 3 files changed, 54 insertions(+), 17 deletions(-) diff --git a/src/projectloader.cpp b/src/projectloader.cpp index 9def881..356f348 100644 --- a/src/projectloader.cpp +++ b/src/projectloader.cpp @@ -64,6 +64,40 @@ void ProjectLoader::setFileName(const QString &newFileName) m_fileName = newFileName; + // Stop the project + if (m_engine) + m_engine->stop(); + + // 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 +232,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 +341,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 index 06731ae..198497c 100644 --- 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/test/projectloader/projectloader_test.cpp b/test/projectloader/projectloader_test.cpp index 23ae4a0..64e83da 100644 --- 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); } From 9b219150cbe353ff33ffaab5942b752f853b9c42 Mon Sep 17 00:00:00 2001 From: adazem009 <68537469+adazem009@users.noreply.github.com> Date: Sat, 2 Mar 2024 21:53:57 +0100 Subject: [PATCH 2/6] fix #105: Ignore removed sprites in MouseEventHandler --- src/mouseeventhandler.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/mouseeventhandler.cpp b/src/mouseeventhandler.cpp index a8db22e..18876ea 100644 --- 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) From 5b0054e576c43122edad3333bac63588f5d95895 Mon Sep 17 00:00:00 2001 From: adazem009 <68537469+adazem009@users.noreply.github.com> Date: Sat, 2 Mar 2024 22:17:49 +0100 Subject: [PATCH 3/6] fix #109: Reset stage model before loading another project --- src/projectloader.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/projectloader.cpp b/src/projectloader.cpp index 356f348..7171d59 100644 --- a/src/projectloader.cpp +++ b/src/projectloader.cpp @@ -68,6 +68,9 @@ void ProjectLoader::setFileName(const QString &newFileName) if (m_engine) m_engine->stop(); + // Reset stage model + m_stage.init(nullptr); + // Delete old sprites for (SpriteModel *sprite : m_sprites) sprite->deleteLater(); From 026716c973c1d2613f17960243ea63fcd869bd72 Mon Sep 17 00:00:00 2001 From: adazem009 <68537469+adazem009@users.noreply.github.com> Date: Sat, 2 Mar 2024 22:22:00 +0100 Subject: [PATCH 4/6] fix #106: Fix backdrop loading when loading another project --- src/renderedtarget.cpp | 16 ++++++++++++++++ src/stagemodel.cpp | 7 ++----- test/renderedtarget/renderedtarget_test.cpp | 4 ++-- test/target_models/stagemodel_test.cpp | 1 - 4 files changed, 20 insertions(+), 8 deletions(-) diff --git a/src/renderedtarget.cpp b/src/renderedtarget.cpp index bf2b5b0..20262ce 100644 --- 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 index 5c600ec..1379c80 100644 --- 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/renderedtarget/renderedtarget_test.cpp b/test/renderedtarget/renderedtarget_test.cpp index b595ba8..fb02831 100644 --- 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 index 8d73cda..66a50cc 100644 --- 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); From e18b4bc8811dc024f9f3cc5651e2e945e513653f Mon Sep 17 00:00:00 2001 From: adazem009 <68537469+adazem009@users.noreply.github.com> Date: Sat, 2 Mar 2024 22:30:24 +0100 Subject: [PATCH 5/6] ProjectLoader: Repaint stage before loading another project --- src/projectloader.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/projectloader.cpp b/src/projectloader.cpp index 7171d59..84a7544 100644 --- a/src/projectloader.cpp +++ b/src/projectloader.cpp @@ -71,6 +71,9 @@ void ProjectLoader::setFileName(const QString &newFileName) // 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(); From a586391572989627e73e4f128913e50050c6bf9b Mon Sep 17 00:00:00 2001 From: adazem009 <68537469+adazem009@users.noreply.github.com> Date: Sat, 2 Mar 2024 22:33:36 +0100 Subject: [PATCH 6/6] ProjectPlayer: Hide pen layer while loading --- src/ProjectPlayer.qml | 1 + 1 file changed, 1 insertion(+) diff --git a/src/ProjectPlayer.qml b/src/ProjectPlayer.qml index 4765be9..4e168d9 100644 --- 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 {