From 2e966edc29fda58cf1c2739148e94ba3277257fc Mon Sep 17 00:00:00 2001 From: adazem009 <68537469+adazem009@users.noreply.github.com> Date: Wed, 21 Aug 2024 23:09:19 +0200 Subject: [PATCH 1/2] fix #555: Use int for index in IEngine::broadcast() --- include/scratchcpp/iengine.h | 2 +- src/engine/internal/engine.cpp | 2 +- src/engine/internal/engine.h | 2 +- test/mocks/enginemock.h | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/include/scratchcpp/iengine.h b/include/scratchcpp/iengine.h index 2e31b927..ef1735cc 100644 --- a/include/scratchcpp/iengine.h +++ b/include/scratchcpp/iengine.h @@ -60,7 +60,7 @@ class LIBSCRATCHCPP_EXPORT IEngine virtual VirtualMachine *startScript(std::shared_ptr topLevelBlock, Target *) = 0; /*! Starts the scripts of the broadcast with the given index. */ - virtual void broadcast(unsigned int index) = 0; + virtual void broadcast(int index) = 0; /*! Starts the scripts of the given broadcast. */ virtual void broadcastByPtr(Broadcast *broadcast) = 0; diff --git a/src/engine/internal/engine.cpp b/src/engine/internal/engine.cpp index dabd0920..d9a4bac6 100644 --- a/src/engine/internal/engine.cpp +++ b/src/engine/internal/engine.cpp @@ -415,7 +415,7 @@ VirtualMachine *Engine::startScript(std::shared_ptr topLevelBlock, Target return pushThread(topLevelBlock, target).get(); } -void Engine::broadcast(unsigned int index) +void Engine::broadcast(int index) { if (index < 0 || index >= m_broadcasts.size()) return; diff --git a/src/engine/internal/engine.h b/src/engine/internal/engine.h index 1d9efd82..d5522f79 100644 --- a/src/engine/internal/engine.h +++ b/src/engine/internal/engine.h @@ -35,7 +35,7 @@ class Engine : public IEngine void start() override; void stop() override; VirtualMachine *startScript(std::shared_ptr topLevelBlock, Target *target) override; - void broadcast(unsigned int index) override; + void broadcast(int index) override; void broadcastByPtr(Broadcast *broadcast) override; void startBackdropScripts(Broadcast *broadcast) override; void stopScript(VirtualMachine *vm) override; diff --git a/test/mocks/enginemock.h b/test/mocks/enginemock.h index bed2b592..4b322e07 100644 --- a/test/mocks/enginemock.h +++ b/test/mocks/enginemock.h @@ -17,7 +17,7 @@ class EngineMock : public IEngine MOCK_METHOD(void, start, (), (override)); MOCK_METHOD(void, stop, (), (override)); MOCK_METHOD(VirtualMachine *, startScript, (std::shared_ptr, Target *), (override)); - MOCK_METHOD(void, broadcast, (unsigned int), (override)); + MOCK_METHOD(void, broadcast, (int), (override)); MOCK_METHOD(void, broadcastByPtr, (Broadcast *), (override)); MOCK_METHOD(void, startBackdropScripts, (Broadcast *), (override)); MOCK_METHOD(void, stopScript, (VirtualMachine *), (override)); From 57967f2044a551f139190261a39cbc0061d1dec7 Mon Sep 17 00:00:00 2001 From: adazem009 <68537469+adazem009@users.noreply.github.com> Date: Wed, 21 Aug 2024 23:50:11 +0200 Subject: [PATCH 2/2] fix #560: Return a list in IEngine::findBroadcasts() --- include/scratchcpp/iengine.h | 4 +-- src/blocks/eventblocks.cpp | 50 ++++++++++++++++++++++++------- src/engine/internal/engine.cpp | 18 +++++------ src/engine/internal/engine.h | 2 +- test/blocks/event_blocks_test.cpp | 44 +++++++++++++++++---------- test/engine/engine_test.cpp | 22 +++++++------- test/mocks/enginemock.h | 2 +- 7 files changed, 93 insertions(+), 49 deletions(-) diff --git a/include/scratchcpp/iengine.h b/include/scratchcpp/iengine.h index ef1735cc..2372d42c 100644 --- a/include/scratchcpp/iengine.h +++ b/include/scratchcpp/iengine.h @@ -291,8 +291,8 @@ class LIBSCRATCHCPP_EXPORT IEngine /*! Returns the broadcast at index. */ virtual std::shared_ptr broadcastAt(int index) const = 0; - /*! Returns the index of the broadcast with the given name. */ - virtual int findBroadcast(const std::string &broadcastName) const = 0; + /*! Returns the list of indexes of the broadcasts with the given name (case insensitive). */ + virtual std::vector findBroadcasts(const std::string &broadcastName) const = 0; /*! Returns the index of the broadcast with the given ID. */ virtual int findBroadcastById(const std::string &broadcastId) const = 0; diff --git a/src/blocks/eventblocks.cpp b/src/blocks/eventblocks.cpp index 5a4c4fca..e4c61902 100644 --- a/src/blocks/eventblocks.cpp +++ b/src/blocks/eventblocks.cpp @@ -99,8 +99,12 @@ void EventBlocks::compileBroadcast(Compiler *compiler) auto input = compiler->input(BROADCAST_INPUT); if (input->type() != Input::Type::ObscuredShadow) { - compiler->addConstValue(compiler->engine()->findBroadcast(input->primaryValue()->value().toString())); - compiler->addFunctionCall(&broadcastByIndex); + std::vector broadcasts = compiler->engine()->findBroadcasts(input->primaryValue()->value().toString()); + + for (int index : broadcasts) { + compiler->addConstValue(index); + compiler->addFunctionCall(&broadcastByIndex); + } } else { compiler->addInput(input); compiler->addFunctionCall(&broadcast); @@ -112,11 +116,17 @@ void EventBlocks::compileBroadcastAndWait(Compiler *compiler) auto input = compiler->input(BROADCAST_INPUT); if (input->type() != Input::Type::ObscuredShadow) { - int index = compiler->engine()->findBroadcast(input->primaryValue()->value().toString()); - compiler->addConstValue(index); - compiler->addFunctionCall(&broadcastByIndexAndWait); - compiler->addConstValue(index); - compiler->addFunctionCall(&checkBroadcastByIndex); + std::vector broadcasts = compiler->engine()->findBroadcasts(input->primaryValue()->value().toString()); + + for (int index : broadcasts) { + compiler->addConstValue(index); + compiler->addFunctionCall(&broadcastByIndexAndWait); + } + + for (int index : broadcasts) { + compiler->addConstValue(index); + compiler->addFunctionCall(&checkBroadcastByIndex); + } } else { compiler->addInput(input); compiler->addFunctionCall(&broadcastAndWait); @@ -197,7 +207,11 @@ unsigned int EventBlocks::whenTouchingObjectPredicate(VirtualMachine *vm) unsigned int EventBlocks::broadcast(VirtualMachine *vm) { - vm->engine()->broadcast(vm->engine()->findBroadcast(vm->getInput(0, 1)->toString())); + std::vector broadcasts = vm->engine()->findBroadcasts(vm->getInput(0, 1)->toString()); + + for (int index : broadcasts) + vm->engine()->broadcast(index); + return 1; } @@ -209,7 +223,11 @@ unsigned int EventBlocks::broadcastByIndex(VirtualMachine *vm) unsigned int EventBlocks::broadcastAndWait(VirtualMachine *vm) { - vm->engine()->broadcast(vm->engine()->findBroadcast(vm->getInput(0, 1)->toString())); + std::vector broadcasts = vm->engine()->findBroadcasts(vm->getInput(0, 1)->toString()); + + for (int index : broadcasts) + vm->engine()->broadcast(index); + return 1; } @@ -221,8 +239,20 @@ unsigned int EventBlocks::broadcastByIndexAndWait(VirtualMachine *vm) unsigned int EventBlocks::checkBroadcast(VirtualMachine *vm) { - if (vm->engine()->broadcastRunning(vm->engine()->findBroadcast(vm->getInput(0, 1)->toString()))) + bool running = false; + + std::vector broadcasts = vm->engine()->findBroadcasts(vm->getInput(0, 1)->toString()); + + for (int index : broadcasts) { + if (vm->engine()->broadcastRunning(index)) { + running = true; + break; + } + } + + if (running) vm->stop(true, true, true); + return 1; } diff --git a/src/engine/internal/engine.cpp b/src/engine/internal/engine.cpp index d9a4bac6..216dc97b 100644 --- a/src/engine/internal/engine.cpp +++ b/src/engine/internal/engine.cpp @@ -1081,21 +1081,21 @@ std::shared_ptr Engine::broadcastAt(int index) const return m_broadcasts[index]; } -int Engine::findBroadcast(const std::string &broadcastName) const +std::vector Engine::findBroadcasts(const std::string &broadcastName) const { + std::vector ret; std::string lowercaseName = broadcastName; std::transform(lowercaseName.begin(), lowercaseName.end(), lowercaseName.begin(), ::tolower); - auto it = std::find_if(m_broadcasts.begin(), m_broadcasts.end(), [lowercaseName](std::shared_ptr broadcast) { - std::string name = broadcast->name(); + for (unsigned int i = 0; i < m_broadcasts.size(); i++) { + std::string name = m_broadcasts[i]->name(); std::transform(name.begin(), name.end(), name.begin(), ::tolower); - return name == lowercaseName; - }); - if (it == m_broadcasts.end()) - return -1; - else - return it - m_broadcasts.begin(); + if (name == lowercaseName) + ret.push_back(i); + } + + return ret; } int Engine::findBroadcastById(const std::string &broadcastId) const diff --git a/src/engine/internal/engine.h b/src/engine/internal/engine.h index d5522f79..d04f71f3 100644 --- a/src/engine/internal/engine.h +++ b/src/engine/internal/engine.h @@ -123,7 +123,7 @@ class Engine : public IEngine const std::vector> &broadcasts() const override; void setBroadcasts(const std::vector> &broadcasts) override; std::shared_ptr broadcastAt(int index) const override; - int findBroadcast(const std::string &broadcastName) const override; + std::vector findBroadcasts(const std::string &broadcastName) const override; int findBroadcastById(const std::string &broadcastId) const override; void addWhenTouchingObjectScript(std::shared_ptr hatBlock) override; diff --git a/test/blocks/event_blocks_test.cpp b/test/blocks/event_blocks_test.cpp index c7567a7d..5a6db769 100644 --- a/test/blocks/event_blocks_test.cpp +++ b/test/blocks/event_blocks_test.cpp @@ -373,8 +373,8 @@ TEST_F(EventBlocksTest, Broadcast) notBlock->setCompileFunction(&OperatorBlocks::compileNot); addObscuredInput(block2, "BROADCAST_INPUT", EventBlocks::BROADCAST_INPUT, notBlock); - EXPECT_CALL(m_engineMock, findBroadcast("test")).WillOnce(Return(0)); - EXPECT_CALL(m_engineMock, functionIndex(&EventBlocks::broadcastByIndex)).WillOnce(Return(0)); + EXPECT_CALL(m_engineMock, findBroadcasts("test")).WillOnce(Return(std::vector({ 0, 3 }))); + EXPECT_CALL(m_engineMock, functionIndex(&EventBlocks::broadcastByIndex)).Times(2).WillRepeatedly(Return(0)); EXPECT_CALL(m_engineMock, functionIndex(&EventBlocks::broadcast)).WillOnce(Return(1)); compiler.init(); @@ -384,9 +384,8 @@ TEST_F(EventBlocksTest, Broadcast) EventBlocks::compileBroadcast(&compiler); compiler.end(); - ASSERT_EQ(compiler.bytecode(), std::vector({ vm::OP_START, vm::OP_CONST, 0, vm::OP_EXEC, 0, vm::OP_NULL, vm::OP_NOT, vm::OP_EXEC, 1, vm::OP_HALT })); - ASSERT_EQ(compiler.constValues().size(), 1); - ASSERT_EQ(compiler.constValues()[0].toDouble(), 0); + ASSERT_EQ(compiler.bytecode(), std::vector({ vm::OP_START, vm::OP_CONST, 0, vm::OP_EXEC, 0, vm::OP_CONST, 1, vm::OP_EXEC, 0, vm::OP_NULL, vm::OP_NOT, vm::OP_EXEC, 1, vm::OP_HALT })); + ASSERT_EQ(compiler.constValues(), std::vector({ 0, 3 })); ASSERT_TRUE(compiler.variables().empty()); ASSERT_TRUE(compiler.lists().empty()); } @@ -402,8 +401,9 @@ TEST_F(EventBlocksTest, BroadcastImpl) vm.setFunctions(functions); vm.setConstValues(constValues); - EXPECT_CALL(m_engineMock, findBroadcast("test")).WillOnce(Return(1)); - EXPECT_CALL(m_engineMock, broadcast(1)).Times(1); + EXPECT_CALL(m_engineMock, findBroadcasts("test")).WillOnce(Return(std::vector({ 1, 4 }))); + EXPECT_CALL(m_engineMock, broadcast(1)); + EXPECT_CALL(m_engineMock, broadcast(4)); vm.setBytecode(bytecode1); vm.run(); @@ -432,9 +432,9 @@ TEST_F(EventBlocksTest, BroadcastAndWait) notBlock->setCompileFunction(&OperatorBlocks::compileNot); addObscuredInput(block2, "BROADCAST_INPUT", EventBlocks::BROADCAST_INPUT, notBlock); - EXPECT_CALL(m_engineMock, findBroadcast("test")).WillOnce(Return(0)); - EXPECT_CALL(m_engineMock, functionIndex(&EventBlocks::broadcastByIndexAndWait)).WillOnce(Return(0)); - EXPECT_CALL(m_engineMock, functionIndex(&EventBlocks::checkBroadcastByIndex)).WillOnce(Return(1)); + EXPECT_CALL(m_engineMock, findBroadcasts("test")).WillOnce(Return(std::vector({ 0, 3 }))); + EXPECT_CALL(m_engineMock, functionIndex(&EventBlocks::broadcastByIndexAndWait)).Times(2).WillRepeatedly(Return(0)); + EXPECT_CALL(m_engineMock, functionIndex(&EventBlocks::checkBroadcastByIndex)).Times(2).WillRepeatedly(Return(1)); EXPECT_CALL(m_engineMock, functionIndex(&EventBlocks::broadcastAndWait)).WillOnce(Return(2)); EXPECT_CALL(m_engineMock, functionIndex(&EventBlocks::checkBroadcast)).WillOnce(Return(3)); @@ -448,8 +448,9 @@ TEST_F(EventBlocksTest, BroadcastAndWait) ASSERT_EQ( compiler.bytecode(), std::vector( - { vm::OP_START, vm::OP_CONST, 0, vm::OP_EXEC, 0, vm::OP_CONST, 1, vm::OP_EXEC, 1, vm::OP_NULL, vm::OP_NOT, vm::OP_EXEC, 2, vm::OP_NULL, vm::OP_NOT, vm::OP_EXEC, 3, vm::OP_HALT })); - ASSERT_EQ(compiler.constValues(), std::vector({ 0, 0 })); + { vm::OP_START, vm::OP_CONST, 0, vm::OP_EXEC, 0, vm::OP_CONST, 1, vm::OP_EXEC, 0, vm::OP_CONST, 2, vm::OP_EXEC, 1, vm::OP_CONST, 3, vm::OP_EXEC, 1, + vm::OP_NULL, vm::OP_NOT, vm::OP_EXEC, 2, vm::OP_NULL, vm::OP_NOT, vm::OP_EXEC, 3, vm::OP_HALT })); + ASSERT_EQ(compiler.constValues(), std::vector({ 0, 3, 0, 3 })); ASSERT_TRUE(compiler.variables().empty()); ASSERT_TRUE(compiler.lists().empty()); } @@ -467,15 +468,16 @@ TEST_F(EventBlocksTest, BroadcastAndWaitImpl) vm.setFunctions(functions); vm.setConstValues(constValues); - EXPECT_CALL(m_engineMock, findBroadcast("test")).WillOnce(Return(1)); - EXPECT_CALL(m_engineMock, broadcast(1)).Times(1); + EXPECT_CALL(m_engineMock, findBroadcasts("test")).WillOnce(Return(std::vector({ 1, 4 }))); + EXPECT_CALL(m_engineMock, broadcast(1)); + EXPECT_CALL(m_engineMock, broadcast(4)); vm.setBytecode(bytecode1); vm.run(); ASSERT_EQ(vm.registerCount(), 0); - EXPECT_CALL(m_engineMock, findBroadcast("test")).WillOnce(Return(2)); + EXPECT_CALL(m_engineMock, findBroadcasts("test")).WillOnce(Return(std::vector({ 2, 3 }))); EXPECT_CALL(m_engineMock, broadcastRunning(2)).WillOnce(Return(true)); vm.setBytecode(bytecode2); @@ -484,8 +486,18 @@ TEST_F(EventBlocksTest, BroadcastAndWaitImpl) ASSERT_EQ(vm.registerCount(), 1); ASSERT_EQ(vm.atEnd(), false); - EXPECT_CALL(m_engineMock, findBroadcast("test")).WillOnce(Return(2)); + EXPECT_CALL(m_engineMock, findBroadcasts("test")).WillOnce(Return(std::vector({ 2, 3 }))); + EXPECT_CALL(m_engineMock, broadcastRunning(2)).WillOnce(Return(true)); + + vm.reset(); + vm.run(); + + ASSERT_EQ(vm.registerCount(), 1); + ASSERT_EQ(vm.atEnd(), false); + + EXPECT_CALL(m_engineMock, findBroadcasts("test")).WillOnce(Return(std::vector({ 2, 3 }))); EXPECT_CALL(m_engineMock, broadcastRunning(2)).WillOnce(Return(false)); + EXPECT_CALL(m_engineMock, broadcastRunning(3)).WillOnce(Return(false)); vm.run(); diff --git a/test/engine/engine_test.cpp b/test/engine/engine_test.cpp index 42b5d675..0d828a1b 100644 --- a/test/engine/engine_test.cpp +++ b/test/engine/engine_test.cpp @@ -1276,23 +1276,25 @@ TEST(EngineTest, Broadcasts) auto b1 = std::make_shared("a", "message1"); auto b2 = std::make_shared("b", "message2"); auto b3 = std::make_shared("c", "Test"); - engine.setBroadcasts({ b1, b2, b3 }); + auto b4 = std::make_shared("d", "TesT"); + engine.setBroadcasts({ b1, b2, b3, b4 }); - ASSERT_EQ(engine.broadcasts(), std::vector>({ b1, b2, b3 })); + ASSERT_EQ(engine.broadcasts(), std::vector>({ b1, b2, b3, b4 })); ASSERT_EQ(engine.broadcastAt(0), b1); ASSERT_EQ(engine.broadcastAt(1), b2); ASSERT_EQ(engine.broadcastAt(2), b3); - ASSERT_EQ(engine.broadcastAt(3), nullptr); + ASSERT_EQ(engine.broadcastAt(3), b4); + ASSERT_EQ(engine.broadcastAt(4), nullptr); ASSERT_EQ(engine.broadcastAt(-1), nullptr); - ASSERT_EQ(engine.findBroadcast("invalid"), -1); - ASSERT_EQ(engine.findBroadcast("message1"), 0); - ASSERT_EQ(engine.findBroadcast("message2"), 1); - ASSERT_EQ(engine.findBroadcast("Test"), 2); - ASSERT_EQ(engine.findBroadcast("MessAge2"), 1); - ASSERT_EQ(engine.findBroadcast("tEst"), 2); + ASSERT_TRUE(engine.findBroadcasts("invalid").empty()); + ASSERT_EQ(engine.findBroadcasts("message1"), std::vector({ 0 })); + ASSERT_EQ(engine.findBroadcasts("message2"), std::vector({ 1 })); + ASSERT_EQ(engine.findBroadcasts("Test"), std::vector({ 2, 3 })); + ASSERT_EQ(engine.findBroadcasts("MessAge2"), std::vector({ 1 })); + ASSERT_EQ(engine.findBroadcasts("tEst"), std::vector({ 2, 3 })); - ASSERT_EQ(engine.findBroadcastById("d"), -1); + ASSERT_EQ(engine.findBroadcastById("e"), -1); ASSERT_EQ(engine.findBroadcastById("a"), 0); ASSERT_EQ(engine.findBroadcastById("b"), 1); ASSERT_EQ(engine.findBroadcastById("c"), 2); diff --git a/test/mocks/enginemock.h b/test/mocks/enginemock.h index 4b322e07..c64691f6 100644 --- a/test/mocks/enginemock.h +++ b/test/mocks/enginemock.h @@ -103,7 +103,7 @@ class EngineMock : public IEngine MOCK_METHOD(const std::vector> &, broadcasts, (), (const, override)); MOCK_METHOD(void, setBroadcasts, (const std::vector> &), (override)); MOCK_METHOD(std::shared_ptr, broadcastAt, (int), (const, override)); - MOCK_METHOD(int, findBroadcast, (const std::string &), (const, override)); + MOCK_METHOD(std::vector, findBroadcasts, (const std::string &), (const, override)); MOCK_METHOD(int, findBroadcastById, (const std::string &), (const, override)); MOCK_METHOD(void, addWhenTouchingObjectScript, (std::shared_ptr), (override));