Skip to content

Fix loading bugs #112

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Mar 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/ProjectPlayer.qml
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ ProjectScene {
id: projectPenLayer
engine: loader.engine
anchors.fill: parent
visible: !priv.loading
}

Component {
Expand Down
8 changes: 8 additions & 0 deletions src/mouseeventhandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
55 changes: 47 additions & 8 deletions src/projectloader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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();
}

Expand Down
1 change: 1 addition & 0 deletions src/projectloader.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
16 changes: 16 additions & 0 deletions src/renderedtarget.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

Expand Down
7 changes: 2 additions & 5 deletions src/stagemodel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -101,7 +99,6 @@ void StageModel::setRenderedTarget(IRenderedTarget *newRenderedTarget)
return;

m_renderedTarget = newRenderedTarget;
loadCostume();

emit renderedTargetChanged();
}
Expand Down
21 changes: 12 additions & 9 deletions test/projectloader/projectloader_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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());
Expand All @@ -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());
}
};
Expand Down Expand Up @@ -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();
Expand All @@ -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);
}

Expand Down
4 changes: 2 additions & 2 deletions test/renderedtarget/renderedtarget_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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());

Expand All @@ -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));
Expand Down
1 change: 0 additions & 1 deletion test/target_models/stagemodel_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down