diff --git a/include/scratchcpp/iengine.h b/include/scratchcpp/iengine.h index 262ebaa8..9aa60772 100644 --- a/include/scratchcpp/iengine.h +++ b/include/scratchcpp/iengine.h @@ -81,6 +81,9 @@ class LIBSCRATCHCPP_EXPORT IEngine /*! Calls the "when I start as a clone" blocks of the given sprite. */ virtual void initClone(Sprite *clone) = 0; + /*! Automatically called from clones that are being deleted. */ + virtual void deinitClone(Sprite *clone) = 0; + /*! * Runs the event loop and calls "when green flag clicked" blocks. * \note This function returns when all scripts finish.\n @@ -137,6 +140,12 @@ class LIBSCRATCHCPP_EXPORT IEngine /*! Sets the stage height. */ virtual void setStageHeight(unsigned int height) = 0; + /*! Returns the maximum number of clones (or -1 if the limit is disabled). */ + virtual int cloneLimit() const = 0; + + /*! Sets the maximum number of clones (use -1 or any negative number to disable the limit). */ + virtual void setCloneLimit(int limit) = 0; + /*! Returns true if there are any running script of the broadcast with the given index. */ virtual bool broadcastRunning(unsigned int index, VirtualMachine *sourceScript) = 0; diff --git a/src/engine/internal/engine.cpp b/src/engine/internal/engine.cpp index 372f9f6b..1235d235 100644 --- a/src/engine/internal/engine.cpp +++ b/src/engine/internal/engine.cpp @@ -32,11 +32,17 @@ Engine::Engine() : { } +Engine::~Engine() +{ + m_clones.clear(); +} + void Engine::clear() { m_sections.clear(); m_targets.clear(); m_broadcasts.clear(); + m_clones.clear(); } // Resolves ID references and sets pointers of entities. @@ -172,6 +178,11 @@ void Engine::frame() void Engine::start() { + if (m_running) + finalize(); + + deleteClones(); + m_timer->reset(); m_running = true; @@ -184,9 +195,8 @@ void Engine::start() void Engine::stop() { - m_runningScripts.clear(); - m_scriptsToRemove.clear(); - m_running = false; + finalize(); + deleteClones(); } void Engine::startScript(std::shared_ptr topLevelBlock, std::shared_ptr target) @@ -301,7 +311,7 @@ void Engine::stopTarget(Target *target, VirtualMachine *exceptScript) void Engine::initClone(Sprite *clone) { - if (!clone) + if (!clone || ((m_cloneLimit >= 0) && (m_clones.size() >= m_cloneLimit))) return; Sprite *source = clone->cloneParent(); @@ -328,6 +338,14 @@ void Engine::initClone(Sprite *clone) m_runningScripts.push_back(vm); } } + + assert(std::find(m_clones.begin(), m_clones.end(), clone) == m_clones.end()); + m_clones.push_back(clone); +} + +void Engine::deinitClone(Sprite *clone) +{ + m_clones.erase(std::remove(m_clones.begin(), m_clones.end(), clone), m_clones.end()); } void Engine::run() @@ -360,7 +378,7 @@ void Engine::run() lastFrameTime = currentTime; } - stop(); + finalize(); } bool Engine::isRunning() const @@ -463,6 +481,16 @@ void Engine::setStageHeight(unsigned int height) m_stageHeight = height; } +int Engine::cloneLimit() const +{ + return m_cloneLimit; +} + +void Engine::setCloneLimit(int limit) +{ + m_cloneLimit = limit < 0 ? -1 : limit; +} + bool Engine::broadcastRunning(unsigned int index, VirtualMachine *sourceScript) { if (index < 0 || index >= m_broadcasts.size()) @@ -879,6 +907,31 @@ BlockSectionContainer *Engine::blockSectionContainer(IBlockSection *section) con return nullptr; } +void Engine::finalize() +{ + m_runningScripts.clear(); + m_scriptsToRemove.clear(); + m_running = false; +} + +void Engine::deleteClones() +{ + m_clones.clear(); + + for (auto target : m_targets) { + Sprite *sprite = dynamic_cast(target.get()); + + if (sprite) { + std::vector> clones = sprite->children(); + + for (auto clone : clones) { + assert(clone); + clone->~Sprite(); + } + } + } +} + void Engine::updateFrameDuration() { m_frameDuration = std::chrono::milliseconds(static_cast(1000 / m_fps)); diff --git a/src/engine/internal/engine.h b/src/engine/internal/engine.h index e60cba19..9a14d8a7 100644 --- a/src/engine/internal/engine.h +++ b/src/engine/internal/engine.h @@ -21,6 +21,7 @@ class Engine : public IEngine public: Engine(); Engine(const Engine &) = delete; + ~Engine(); void clear() override; void resolveIds(); @@ -35,6 +36,7 @@ class Engine : public IEngine void stopScript(VirtualMachine *vm) override; void stopTarget(Target *target, VirtualMachine *exceptScript) override; void initClone(libscratchcpp::Sprite *clone) override; + void deinitClone(libscratchcpp::Sprite *clone) override; void run() override; bool isRunning() const override; @@ -61,6 +63,9 @@ class Engine : public IEngine unsigned int stageHeight() const override; void setStageHeight(unsigned int height) override; + int cloneLimit() const override; + void setCloneLimit(int limit) override; + bool broadcastRunning(unsigned int index, VirtualMachine *sourceScript) override; bool broadcastByPtrRunning(Broadcast *broadcast, VirtualMachine *sourceScript) override; @@ -109,6 +114,8 @@ class Engine : public IEngine BlockSectionContainer *blockSectionContainer(IBlockSection *section) const; private: + void finalize(); + void deleteClones(); std::shared_ptr getBlock(const std::string &id); std::shared_ptr getVariable(const std::string &id); std::shared_ptr getList(const std::string &id); @@ -142,6 +149,8 @@ class Engine : public IEngine bool m_mousePressed = false; unsigned int m_stageWidth = 480; unsigned int m_stageHeight = 360; + int m_cloneLimit = 300; + std::vector m_clones; bool m_running = false; bool m_breakFrame = false; diff --git a/src/scratch/sprite.cpp b/src/scratch/sprite.cpp index 3635f006..cb5351d7 100644 --- a/src/scratch/sprite.cpp +++ b/src/scratch/sprite.cpp @@ -23,6 +23,16 @@ Sprite::Sprite() : Sprite::~Sprite() { if (isClone()) { + IEngine *eng = engine(); + + if (eng) { + eng->deinitClone(this); + + auto children = allChildren(); + for (auto child : children) + eng->deinitClone(child.get()); + } + assert(impl->cloneParent); impl->cloneParent->impl->removeClone(this); } diff --git a/test/clone_limit.sb3 b/test/clone_limit.sb3 new file mode 100644 index 00000000..75338c6e Binary files /dev/null and b/test/clone_limit.sb3 differ diff --git a/test/engine/engine_test.cpp b/test/engine/engine_test.cpp index 0389708c..38dcada8 100644 --- a/test/engine/engine_test.cpp +++ b/test/engine/engine_test.cpp @@ -547,6 +547,58 @@ TEST(EngineTest, Clones) } } +TEST(EngineTest, CloneLimit) +{ + Project p("clone_limit.sb3"); + ASSERT_TRUE(p.load()); + auto engine = p.engine(); + ASSERT_EQ(engine->cloneLimit(), 300); + + // TODO: Set "infinite" FPS and remove this (#254) + engine->setFps(100000); + + Stage *stage = engine->stage(); + ASSERT_TRUE(stage); + + p.run(); + ASSERT_VAR(stage, "count"); + ASSERT_EQ(GET_VAR(stage, "count")->value().toInt(), 300); + ASSERT_VAR(stage, "delete_passed"); + ASSERT_TRUE(GET_VAR(stage, "delete_passed")->value().toBool()); + + engine->setCloneLimit(475); + ASSERT_EQ(engine->cloneLimit(), 475); + p.run(); + ASSERT_VAR(stage, "count"); + ASSERT_EQ(GET_VAR(stage, "count")->value().toInt(), 475); + ASSERT_VAR(stage, "delete_passed"); + ASSERT_TRUE(GET_VAR(stage, "delete_passed")->value().toBool()); + + engine->setCloneLimit(0); + ASSERT_EQ(engine->cloneLimit(), 0); + p.run(); + ASSERT_VAR(stage, "count"); + ASSERT_EQ(GET_VAR(stage, "count")->value().toInt(), 0); + ASSERT_VAR(stage, "delete_passed"); + ASSERT_TRUE(GET_VAR(stage, "delete_passed")->value().toBool()); + + engine->setCloneLimit(-1); + ASSERT_EQ(engine->cloneLimit(), -1); + p.run(); + ASSERT_VAR(stage, "count"); + ASSERT_GT(GET_VAR(stage, "count")->value().toInt(), 500); + ASSERT_VAR(stage, "delete_passed"); + ASSERT_TRUE(GET_VAR(stage, "delete_passed")->value().toBool()); + + engine->setCloneLimit(-5); + ASSERT_EQ(engine->cloneLimit(), -1); + p.run(); + ASSERT_VAR(stage, "count"); + ASSERT_GT(GET_VAR(stage, "count")->value().toInt(), 500); + ASSERT_VAR(stage, "delete_passed"); + ASSERT_TRUE(GET_VAR(stage, "delete_passed")->value().toBool()); +} + // TODO: Uncomment this after fixing #256 and #257 /*TEST(EngineTest, BackdropBroadcasts) { diff --git a/test/mocks/enginemock.h b/test/mocks/enginemock.h index d658dbf2..713c0017 100644 --- a/test/mocks/enginemock.h +++ b/test/mocks/enginemock.h @@ -23,6 +23,7 @@ class EngineMock : public IEngine MOCK_METHOD(void, stopScript, (VirtualMachine *), (override)); MOCK_METHOD(void, stopTarget, (Target *, VirtualMachine *), (override)); MOCK_METHOD(void, initClone, (Sprite *), (override)); + MOCK_METHOD(void, deinitClone, (Sprite *), (override)); MOCK_METHOD(void, run, (), (override)); MOCK_METHOD(bool, isRunning, (), (const, override)); @@ -49,6 +50,9 @@ class EngineMock : public IEngine MOCK_METHOD(unsigned int, stageHeight, (), (const, override)); MOCK_METHOD(void, setStageHeight, (unsigned int), (override)); + MOCK_METHOD(int, cloneLimit, (), (const, override)); + MOCK_METHOD(void, setCloneLimit, (int), (override)); + MOCK_METHOD(bool, broadcastRunning, (unsigned int, VirtualMachine *), (override)); MOCK_METHOD(bool, broadcastByPtrRunning, (Broadcast *, VirtualMachine *), (override)); diff --git a/test/scratch_classes/sprite_test.cpp b/test/scratch_classes/sprite_test.cpp index 05ee32ff..d8186a0f 100644 --- a/test/scratch_classes/sprite_test.cpp +++ b/test/scratch_classes/sprite_test.cpp @@ -27,36 +27,36 @@ TEST(SpriteTest, Visible) TEST(SpriteTest, Clone) { - Sprite sprite; - sprite.setName("Sprite1"); + std::unique_ptr sprite = std::make_unique(); + sprite->setName("Sprite1"); auto var1 = std::make_shared("a", "var1", "hello"); auto var2 = std::make_shared("b", "var2", "world"); - sprite.addVariable(var1); - sprite.addVariable(var2); + sprite->addVariable(var1); + sprite->addVariable(var2); auto c1 = std::make_shared("costume1", "", "svg"); - sprite.addCostume(c1); + sprite->addCostume(c1); auto list1 = std::make_shared("c", "list1"); list1->push_back("item1"); list1->push_back("item2"); auto list2 = std::make_shared("d", "list2"); list2->push_back("test"); - sprite.addList(list1); - sprite.addList(list2); - - sprite.addCostume(std::make_shared("", "", "")); - sprite.addCostume(std::make_shared("", "", "")); - sprite.setCurrentCostume(2); - sprite.setLayerOrder(5); - sprite.setVolume(50); - - sprite.setVisible(false); - sprite.setX(100.25); - sprite.setY(-45.43); - sprite.setSize(54.121); - sprite.setDirection(179.4); - sprite.setDraggable(true); - sprite.setRotationStyle(Sprite::RotationStyle::DoNotRotate); + sprite->addList(list1); + sprite->addList(list2); + + sprite->addCostume(std::make_shared("", "", "")); + sprite->addCostume(std::make_shared("", "", "")); + sprite->setCurrentCostume(2); + sprite->setLayerOrder(5); + sprite->setVolume(50); + + sprite->setVisible(false); + sprite->setX(100.25); + sprite->setY(-45.43); + sprite->setSize(54.121); + sprite->setDirection(179.4); + sprite->setDraggable(true); + sprite->setRotationStyle(Sprite::RotationStyle::DoNotRotate); auto checkCloneData = [](Sprite *clone) { ASSERT_TRUE(clone); @@ -97,42 +97,42 @@ TEST(SpriteTest, Clone) ASSERT_EQ(clone->rotationStyle(), Sprite::RotationStyle::DoNotRotate); }; - ASSERT_FALSE(sprite.isClone()); - ASSERT_EQ(sprite.cloneRoot(), nullptr); - ASSERT_EQ(sprite.cloneParent(), nullptr); + ASSERT_FALSE(sprite->isClone()); + ASSERT_EQ(sprite->cloneRoot(), nullptr); + ASSERT_EQ(sprite->cloneParent(), nullptr); - ASSERT_EQ(sprite.clone(), nullptr); - ASSERT_FALSE(sprite.isClone()); - ASSERT_EQ(sprite.cloneRoot(), nullptr); - ASSERT_EQ(sprite.cloneParent(), nullptr); + ASSERT_EQ(sprite->clone(), nullptr); + ASSERT_FALSE(sprite->isClone()); + ASSERT_EQ(sprite->cloneRoot(), nullptr); + ASSERT_EQ(sprite->cloneParent(), nullptr); EngineMock engine; - sprite.setEngine(&engine); + sprite->setEngine(&engine); Sprite *clone1; EXPECT_CALL(engine, initClone(_)).WillOnce(SaveArg<0>(&clone1)); - ASSERT_EQ(sprite.clone().get(), clone1); - ASSERT_FALSE(sprite.isClone()); - ASSERT_EQ(sprite.cloneRoot(), nullptr); - ASSERT_EQ(sprite.cloneParent(), nullptr); + ASSERT_EQ(sprite->clone().get(), clone1); + ASSERT_FALSE(sprite->isClone()); + ASSERT_EQ(sprite->cloneRoot(), nullptr); + ASSERT_EQ(sprite->cloneParent(), nullptr); ASSERT_TRUE(clone1->isClone()); - ASSERT_EQ(clone1->cloneRoot(), &sprite); - ASSERT_EQ(clone1->cloneParent(), &sprite); + ASSERT_EQ(clone1->cloneRoot(), sprite.get()); + ASSERT_EQ(clone1->cloneParent(), sprite.get()); checkCloneData(clone1); // Modify root sprite data to make sure parent is used - sprite.setLayerOrder(3); + sprite->setLayerOrder(3); Sprite *clone2; EXPECT_CALL(engine, initClone(_)).WillOnce(SaveArg<0>(&clone2)); ASSERT_EQ(clone1->clone().get(), clone2); ASSERT_TRUE(clone1->isClone()); - ASSERT_EQ(clone1->cloneRoot(), &sprite); - ASSERT_EQ(clone1->cloneParent(), &sprite); + ASSERT_EQ(clone1->cloneRoot(), sprite.get()); + ASSERT_EQ(clone1->cloneParent(), sprite.get()); ASSERT_TRUE(clone2->isClone()); - ASSERT_EQ(clone2->cloneRoot(), &sprite); + ASSERT_EQ(clone2->cloneRoot(), sprite.get()); ASSERT_EQ(clone2->cloneParent(), clone1); checkCloneData(clone2); @@ -143,10 +143,10 @@ TEST(SpriteTest, Clone) Sprite *clone4; EXPECT_CALL(engine, initClone(_)).WillOnce(SaveArg<0>(&clone4)); - ASSERT_EQ(sprite.clone().get(), clone4); + ASSERT_EQ(sprite->clone().get(), clone4); // children - const auto &children1 = sprite.children(); + const auto &children1 = sprite->children(); ASSERT_EQ(children1.size(), 2); ASSERT_EQ(children1[0].get(), clone1); ASSERT_EQ(children1[1].get(), clone4); @@ -163,7 +163,7 @@ TEST(SpriteTest, Clone) ASSERT_TRUE(clone4->children().empty()); // allChildren - auto allChildren = sprite.allChildren(); + auto allChildren = sprite->allChildren(); ASSERT_EQ(allChildren.size(), 4); ASSERT_EQ(allChildren[0].get(), clone1); ASSERT_EQ(allChildren[1].get(), clone2); @@ -181,10 +181,24 @@ TEST(SpriteTest, Clone) ASSERT_TRUE(clone4->allChildren().empty()); - ASSERT_EQ(clone2->costumes(), sprite.costumes()); + ASSERT_EQ(clone2->costumes(), sprite->costumes()); auto c2 = std::make_shared("costume2", "", "png"); clone2->addCostume(c2); - ASSERT_EQ(clone2->costumes(), sprite.costumes()); + ASSERT_EQ(clone2->costumes(), sprite->costumes()); + + EXPECT_CALL(engine, deinitClone(clone2)); + clone2->~Sprite(); + + EXPECT_CALL(engine, deinitClone(clone3)); + clone3->~Sprite(); + + EXPECT_CALL(engine, deinitClone(clone1)).Times(2); + clone1->~Sprite(); + + EXPECT_CALL(engine, deinitClone(clone4)).Times(2); + clone4->~Sprite(); + + sprite.reset(); } TEST(SpriteTest, X) diff --git a/test/target_interfaces/ispritehandler_test.cpp b/test/target_interfaces/ispritehandler_test.cpp index 75b69fc7..90feabf7 100644 --- a/test/target_interfaces/ispritehandler_test.cpp +++ b/test/target_interfaces/ispritehandler_test.cpp @@ -34,6 +34,10 @@ TEST_F(ISpriteHandlerTest, Clone) m_sprite.clone(); ASSERT_TRUE(clone); ASSERT_EQ(cloneArg, clone); + + // Engine is used during deletion, so let's just set it to nullptr (this is tested elsewhere) + m_sprite.setEngine(nullptr); + clone->setEngine(nullptr); } TEST_F(ISpriteHandlerTest, Visible)