From 119aa269a5ac7c2b051c22fbfde1e2700b34593c Mon Sep 17 00:00:00 2001 From: adazem009 <68537469+adazem009@users.noreply.github.com> Date: Sat, 19 Aug 2023 11:25:42 +0200 Subject: [PATCH 1/7] Use parent block to determine whether a block is top level --- include/scratchcpp/block.h | 1 - src/internal/scratch3reader.cpp | 4 ---- src/scratch/block.cpp | 10 +--------- src/scratch/block_p.h | 1 - 4 files changed, 1 insertion(+), 15 deletions(-) diff --git a/include/scratchcpp/block.h b/include/scratchcpp/block.h index ce3ecf02..13f4a1cd 100644 --- a/include/scratchcpp/block.h +++ b/include/scratchcpp/block.h @@ -53,7 +53,6 @@ class LIBSCRATCHCPP_EXPORT Block : public Entity void setShadow(bool newShadow); bool topLevel() const; - void setTopLevel(bool newTopLevel); void setEngine(IEngine *newEngine); diff --git a/src/internal/scratch3reader.cpp b/src/internal/scratch3reader.cpp index 504ab3fe..ee125990 100644 --- a/src/internal/scratch3reader.cpp +++ b/src/internal/scratch3reader.cpp @@ -177,10 +177,6 @@ bool Scratch3Reader::load() READER_STEP(step, "target -> block -> shadow"); block->setShadow(blockInfo["shadow"]); - // topLevel - READER_STEP(step, "target -> block -> topLevel"); - block->setTopLevel(blockInfo["topLevel"]); - target->addBlock(block); } diff --git a/src/scratch/block.cpp b/src/scratch/block.cpp index 3af45f8d..14a4b44b 100644 --- a/src/scratch/block.cpp +++ b/src/scratch/block.cpp @@ -222,15 +222,7 @@ void Block::setShadow(bool newShadow) /*! Returns true if this is a top level block. */ bool Block::topLevel() const { - // TODO: Return true if parentId() == "" - // and remove the setter - return impl->topLevel; -} - -/*! Toggles whether this block is a top level block. */ -void Block::setTopLevel(bool newTopLevel) -{ - impl->topLevel = newTopLevel; + return (impl->parentId == "" && !impl->parent); } /*! Sets the Engine. */ diff --git a/src/scratch/block_p.h b/src/scratch/block_p.h index 0e80fdb1..0c0daff3 100644 --- a/src/scratch/block_p.h +++ b/src/scratch/block_p.h @@ -29,7 +29,6 @@ struct BlockPrivate std::vector> fields; std::unordered_map fieldMap; bool shadow = false; - bool topLevel = false; IEngine *engine = nullptr; Target *target = nullptr; BlockPrototype mutationPrototype; From 2c7eb8a10c288562a65c8d760597a1047968201f Mon Sep 17 00:00:00 2001 From: adazem009 <68537469+adazem009@users.noreply.github.com> Date: Sat, 19 Aug 2023 11:42:54 +0200 Subject: [PATCH 2/7] Block: Set next and parent ID properly --- src/scratch/block.cpp | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/src/scratch/block.cpp b/src/scratch/block.cpp index 14a4b44b..0219358d 100644 --- a/src/scratch/block.cpp +++ b/src/scratch/block.cpp @@ -66,16 +66,18 @@ std::shared_ptr Block::next() const /*! Returns the ID of the next block. */ std::string Block::nextId() const { - if (impl->next) - return impl->next->id(); - else - return impl->nextId; + return impl->nextId; } /*! Sets the next block. */ void Block::setNext(std::shared_ptr block) { impl->next = block; + + if (block) + impl->nextId = block->id(); + else + impl->nextId = ""; } /*! Sets the next block by ID. */ @@ -94,16 +96,18 @@ std::shared_ptr Block::parent() const /*! Returns the ID of the parent block. */ std::string Block::parentId() const { - if (impl->parent) - return impl->parent->id(); - else - return impl->parentId; + return impl->parentId; } /*! Sets the parent block. */ void Block::setParent(std::shared_ptr block) { impl->parent = block; + + if (block) + impl->parentId = block->id(); + else + impl->parentId = ""; } /*! Sets the parent block by ID. */ From c5bfcde9e4a970bc482c386587666d5ebfd7d828 Mon Sep 17 00:00:00 2001 From: adazem009 <68537469+adazem009@users.noreply.github.com> Date: Sat, 19 Aug 2023 12:01:28 +0200 Subject: [PATCH 3/7] Block: Do not add existing inputs and fields --- src/scratch/block.cpp | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/scratch/block.cpp b/src/scratch/block.cpp index 0219358d..f964a10f 100644 --- a/src/scratch/block.cpp +++ b/src/scratch/block.cpp @@ -126,6 +126,11 @@ std::vector> Block::inputs() const /*! Adds an input and returns its index. */ int Block::addInput(std::shared_ptr input) { + auto it = std::find(impl->inputs.begin(), impl->inputs.end(), input); + + if (it != impl->inputs.end()) + return it - impl->inputs.begin(); + impl->inputs.push_back(input); return impl->inputs.size() - 1; } @@ -173,6 +178,11 @@ std::vector> Block::fields() const /*! Adds a field and returns its index. */ int Block::addField(std::shared_ptr field) { + auto it = std::find(impl->fields.begin(), impl->fields.end(), field); + + if (it != impl->fields.end()) + return it - impl->fields.begin(); + impl->fields.push_back(field); return impl->fields.size() - 1; } From c7dcab718420caa26364b7a7c2103ba37f72899f Mon Sep 17 00:00:00 2001 From: adazem009 <68537469+adazem009@users.noreply.github.com> Date: Sat, 19 Aug 2023 12:04:23 +0200 Subject: [PATCH 4/7] Block: Add missing range checks --- src/scratch/block.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/scratch/block.cpp b/src/scratch/block.cpp index f964a10f..bb293e85 100644 --- a/src/scratch/block.cpp +++ b/src/scratch/block.cpp @@ -138,6 +138,9 @@ int Block::addInput(std::shared_ptr input) /*! Returns the input at index. */ std::shared_ptr Block::inputAt(int index) const { + if (index < 0 || index >= impl->inputs.size()) + return nullptr; + return impl->inputs[index]; } @@ -190,6 +193,9 @@ int Block::addField(std::shared_ptr field) /*! Returns the field at index. */ std::shared_ptr Block::fieldAt(int index) const { + if (index < 0 || index >= impl->fields.size()) + return nullptr; + return impl->fields[index]; } From 688609fcf2c539a092156eb8d15f6fd7c1a38050 Mon Sep 17 00:00:00 2001 From: adazem009 <68537469+adazem009@users.noreply.github.com> Date: Sat, 19 Aug 2023 12:30:59 +0200 Subject: [PATCH 5/7] Block: Add getters for engine and target --- include/scratchcpp/block.h | 2 ++ src/scratch/block.cpp | 12 ++++++++++++ 2 files changed, 14 insertions(+) diff --git a/include/scratchcpp/block.h b/include/scratchcpp/block.h index 13f4a1cd..4288f808 100644 --- a/include/scratchcpp/block.h +++ b/include/scratchcpp/block.h @@ -55,8 +55,10 @@ class LIBSCRATCHCPP_EXPORT Block : public Entity bool topLevel() const; void setEngine(IEngine *newEngine); + IEngine *engine() const; void setTarget(Target *newTarget); + Target *target() const; BlockComp compileFunction() const; void setCompileFunction(BlockComp newCompileFunction); diff --git a/src/scratch/block.cpp b/src/scratch/block.cpp index bb293e85..868651f7 100644 --- a/src/scratch/block.cpp +++ b/src/scratch/block.cpp @@ -251,8 +251,20 @@ void Block::setEngine(IEngine *newEngine) impl->engine = newEngine; } +/*! Returns the Engine. */ +IEngine *Block::engine() const +{ + return impl->engine; +} + /*! Sets the Target. */ void Block::setTarget(Target *newTarget) { impl->target = newTarget; } + +/*! Returns the Target. */ +Target *Block::target() const +{ + return impl->target; +} From 77a6f27d4d7cd7c8111339ac20d2014267a43595 Mon Sep 17 00:00:00 2001 From: adazem009 <68537469+adazem009@users.noreply.github.com> Date: Sat, 19 Aug 2023 12:32:59 +0200 Subject: [PATCH 6/7] Block: Add null check to compile() --- src/scratch/block.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/scratch/block.cpp b/src/scratch/block.cpp index 868651f7..92e1ee8e 100644 --- a/src/scratch/block.cpp +++ b/src/scratch/block.cpp @@ -18,7 +18,8 @@ Block::Block(const std::string &id, const std::string &opcode) : /*! Calls the compile function. */ void Block::compile(Compiler *compiler) { - return impl->compileFunction(compiler); + if (impl->compileFunction) + return impl->compileFunction(compiler); } /*! Returns the opcode. */ From 706665a2a8b143a535debbdda85a6319edcfa608 Mon Sep 17 00:00:00 2001 From: adazem009 <68537469+adazem009@users.noreply.github.com> Date: Sat, 19 Aug 2023 12:38:31 +0200 Subject: [PATCH 7/7] Add Block test --- test/scratch_classes/CMakeLists.txt | 16 ++ test/scratch_classes/block_test.cpp | 243 ++++++++++++++++++++++++++++ 2 files changed, 259 insertions(+) create mode 100644 test/scratch_classes/block_test.cpp diff --git a/test/scratch_classes/CMakeLists.txt b/test/scratch_classes/CMakeLists.txt index f23fbd0a..8fc62437 100644 --- a/test/scratch_classes/CMakeLists.txt +++ b/test/scratch_classes/CMakeLists.txt @@ -153,3 +153,19 @@ target_link_libraries( ) gtest_discover_tests(field_test) + +# block_test +add_executable( + block_test + block_test.cpp +) + +target_link_libraries( + block_test + GTest::gtest_main + GTest::gmock_main + scratchcpp + scratchcpp_mocks +) + +gtest_discover_tests(block_test) diff --git a/test/scratch_classes/block_test.cpp b/test/scratch_classes/block_test.cpp new file mode 100644 index 00000000..0dcf626b --- /dev/null +++ b/test/scratch_classes/block_test.cpp @@ -0,0 +1,243 @@ +#include +#include +#include +#include +#include +#include + +#include "../common.h" + +using namespace libscratchcpp; + +class BlockTestMock +{ + public: + MOCK_METHOD(void, compileTest, (Compiler *), ()); +}; + +class BlockTest : public testing::Test +{ + public: + void SetUp() override { m_mockPtr = &m_mock; } + + static void compileTest(Compiler *compiler) + { + ASSERT_TRUE(m_mockPtr); + m_mockPtr->compileTest(compiler); + } + + static inline BlockTestMock *m_mockPtr = nullptr; + BlockTestMock m_mock; +}; + +TEST_F(BlockTest, Constructors) +{ + Block block("abc", "motion_movesteps"); + ASSERT_EQ(block.id(), "abc"); + ASSERT_EQ(block.opcode(), "motion_movesteps"); +} + +TEST_F(BlockTest, Next) +{ + Block block("", ""); + ASSERT_EQ(block.next(), nullptr); + ASSERT_EQ(block.nextId(), ""); + + block.setNextId("hello"); + ASSERT_EQ(block.next(), nullptr); + ASSERT_EQ(block.nextId(), "hello"); + + auto nextBlock = std::make_shared("abc", ""); + block.setNext(nextBlock); + ASSERT_EQ(block.next(), nextBlock); + ASSERT_EQ(block.nextId(), "abc"); + + block.setNextId("def"); + ASSERT_EQ(block.next(), nullptr); + ASSERT_EQ(block.nextId(), "def"); + + block.setNext(nullptr); + ASSERT_EQ(block.next(), nullptr); + ASSERT_EQ(block.nextId(), ""); +} + +TEST_F(BlockTest, Parent) +{ + Block block("", ""); + ASSERT_EQ(block.parent(), nullptr); + ASSERT_EQ(block.parentId(), ""); + + block.setParentId("hello"); + ASSERT_EQ(block.parent(), nullptr); + ASSERT_EQ(block.parentId(), "hello"); + + auto parentBlock = std::make_shared("abc", ""); + block.setParent(parentBlock); + ASSERT_EQ(block.parent(), parentBlock); + ASSERT_EQ(block.parentId(), "abc"); + + block.setParentId("def"); + ASSERT_EQ(block.parent(), nullptr); + ASSERT_EQ(block.parentId(), "def"); + + block.setParent(nullptr); + ASSERT_EQ(block.parent(), nullptr); + ASSERT_EQ(block.parentId(), ""); +} + +TEST_F(BlockTest, Inputs) +{ + auto i1 = std::make_shared("VALUE1", Input::Type::Shadow); + i1->setInputId(11); + + auto i2 = std::make_shared("VALUE2", Input::Type::Shadow); + i2->setInputId(12); + + auto i3 = std::make_shared("VALUE3", Input::Type::Shadow); + i3->setInputId(15); + + Block block("", ""); + ASSERT_EQ(block.addInput(i1), 0); + ASSERT_EQ(block.addInput(i2), 1); + ASSERT_EQ(block.addInput(i3), 2); + ASSERT_EQ(block.addInput(i2), 1); // add existing input + + ASSERT_EQ(block.inputs(), std::vector>({ i1, i2, i3 })); + ASSERT_EQ(block.inputAt(0), i1); + ASSERT_EQ(block.inputAt(1), i2); + ASSERT_EQ(block.inputAt(2), i3); + ASSERT_EQ(block.inputAt(3), nullptr); + ASSERT_EQ(block.inputAt(-1), nullptr); + + ASSERT_EQ(block.findInput("invalid"), -1); + ASSERT_EQ(block.findInput("VALUE1"), 0); + ASSERT_EQ(block.findInput("VALUE2"), 1); + ASSERT_EQ(block.findInput("VALUE3"), 2); + + block.updateInputMap(); + ASSERT_EQ(block.findInputById(5), nullptr); + ASSERT_EQ(block.findInputById(11), i1.get()); + ASSERT_EQ(block.findInputById(12), i2.get()); + ASSERT_EQ(block.findInputById(15), i3.get()); +} + +TEST_F(BlockTest, Fields) +{ + auto f1 = std::make_shared("VARIABLE1", Value()); + f1->setFieldId(11); + + auto f2 = std::make_shared("VARIABLE2", Value()); + f2->setFieldId(12); + + auto f3 = std::make_shared("VARIABLE3", Value()); + f3->setFieldId(15); + + Block block("", ""); + ASSERT_EQ(block.addField(f1), 0); + ASSERT_EQ(block.addField(f2), 1); + ASSERT_EQ(block.addField(f3), 2); + ASSERT_EQ(block.addField(f2), 1); // add existing field + + ASSERT_EQ(block.fields(), std::vector>({ f1, f2, f3 })); + ASSERT_EQ(block.fieldAt(0), f1); + ASSERT_EQ(block.fieldAt(1), f2); + ASSERT_EQ(block.fieldAt(2), f3); + ASSERT_EQ(block.fieldAt(3), nullptr); + ASSERT_EQ(block.fieldAt(-1), nullptr); + + ASSERT_EQ(block.findField("invalid"), -1); + ASSERT_EQ(block.findField("VARIABLE1"), 0); + ASSERT_EQ(block.findField("VARIABLE2"), 1); + ASSERT_EQ(block.findField("VARIABLE3"), 2); + + block.updateFieldMap(); + ASSERT_EQ(block.findFieldById(5), nullptr); + ASSERT_EQ(block.findFieldById(11), f1.get()); + ASSERT_EQ(block.findFieldById(12), f2.get()); + ASSERT_EQ(block.findFieldById(15), f3.get()); +} + +TEST_F(BlockTest, Shadow) +{ + Block block("", ""); + ASSERT_FALSE(block.shadow()); + + block.setShadow(true); + ASSERT_TRUE(block.shadow()); +} + +TEST_F(BlockTest, TopLevel) +{ + Block block("", ""); + ASSERT_TRUE(block.topLevel()); + + block.setParentId("hello"); + ASSERT_FALSE(block.topLevel()); + + auto parentBlock = std::make_shared("abc", ""); + block.setParent(parentBlock); + ASSERT_FALSE(block.topLevel()); + + block.setParentId("def"); + ASSERT_FALSE(block.topLevel()); + + block.setParent(nullptr); + ASSERT_TRUE(block.topLevel()); +} + +TEST_F(BlockTest, Engine) +{ + Block block("", ""); + ASSERT_EQ(block.engine(), nullptr); + + EngineMock engine; + block.setEngine(&engine); + ASSERT_EQ(block.engine(), &engine); +} + +TEST_F(BlockTest, Target) +{ + Block block("", ""); + ASSERT_EQ(block.target(), nullptr); + + Target target; + block.setTarget(&target); + ASSERT_EQ(block.target(), &target); +} + +TEST_F(BlockTest, CompileFunction) +{ + Block block("", ""); + ASSERT_EQ(block.compileFunction(), nullptr); + + block.setCompileFunction(&compileTest); + ASSERT_EQ(block.compileFunction(), &compileTest); +} + +TEST_F(BlockTest, Compile) +{ + Block block("", ""); + EngineMock engine; + Compiler compiler(&engine); + + block.compile(&compiler); // test with null compile function + + block.setCompileFunction(&compileTest); + EXPECT_CALL(m_mock, compileTest(&compiler)).Times(1); + block.compile(&compiler); +} + +TEST_F(BlockTest, MutationHasNext) +{ + Block block("", ""); + ASSERT_TRUE(block.mutationHasNext()); + + block.setMutationHasNext(false); + ASSERT_FALSE(block.mutationHasNext()); +} + +TEST_F(BlockTest, MutationPrototype) +{ + Block block("", ""); + ASSERT_TRUE(block.mutationPrototype()); +}