Add methods for missing variable and list monitors

Author: adazem009

include/scratchcpp/iengine.h

@@ -360,6 +360,12 @@ class LIBSCRATCHCPP_EXPORT IEngine
         /*! Sets the list of monitors. */
         virtual void setMonitors(const std::vector<std::shared_ptr<Monitor>> &newMonitors) = 0;
 
+        /*! Creates a monitor for the given variable if it's missing and returns it. */
+        virtual Monitor *createVariableMonitor(std::shared_ptr<Variable> var, const std::string &opcode, const std::string &varFieldName, int varFieldId) = 0;
+
+        /*! Creates a monitor for the given list if it's missing and returns it. */
+        virtual Monitor *createListMonitor(std::shared_ptr<List> list, const std::string &opcode, const std::string &listFieldName, int listFieldId) = 0;
+
         /*! Emits when a monitor is added. */
         virtual sigslot::signal<Monitor *> &monitorAdded() = 0;
 

src/engine/internal/engine.cpp

@@ -29,8 +29,6 @@
 #include "clock.h"
 #include "audio/iaudioengine.h"
 #include "blocks/standardblocks.h"
-#include "blocks/variableblocks.h"
-#include "blocks/listblocks.h"
 #include "scratch/monitor_p.h"
 
 using namespace libscratchcpp;
@@ -1231,9 +1229,6 @@ void Engine::setTargets(const std::vector<std::shared_ptr<Target>> &newTargets)
 
     // Sort the executable targets by layer order
     std::sort(m_executableTargets.begin(), m_executableTargets.end(), [](Target *t1, Target *t2) { return t1->layerOrder() < t2->layerOrder(); });
-
-    // Create missing monitors
-    createMissingMonitors();
 }
 
 Target *Engine::targetAt(int index) const
@@ -1401,9 +1396,38 @@ void Engine::setMonitors(const std::vector<std::shared_ptr<Monitor>> &newMonitor
         m_monitors.push_back(monitor);
         m_monitorAdded(monitor.get());
     }
+}
+
+Monitor *Engine::createVariableMonitor(std::shared_ptr<Variable> var, const std::string &opcode, const std::string &varFieldName, int varFieldId)
+{
+    if (var->monitor())
+        return var->monitor();
+    else {
+        auto monitor = std::make_shared<Monitor>(var->id(), opcode);
+        auto field = std::make_shared<Field>(varFieldName, var->name(), var);
+        field->setFieldId(varFieldId);
+        monitor->block()->addField(field);
+
+        addVarOrListMonitor(monitor, var->target());
+        var->setMonitor(monitor.get());
+        return monitor.get();
+    }
+}
 
-    // Create missing monitors
-    createMissingMonitors();
+Monitor *Engine::createListMonitor(std::shared_ptr<List> list, const std::string &opcode, const std::string &listFieldName, int listFieldId)
+{
+    if (list->monitor())
+        return list->monitor();
+    else {
+        auto monitor = std::make_shared<Monitor>(list->id(), opcode);
+        auto field = std::make_shared<Field>(listFieldName, list->name(), list);
+        field->setFieldId(listFieldId);
+        monitor->block()->addField(field);
+        list->setMonitor(monitor.get());
+
+        addVarOrListMonitor(monitor, list->target());
+        return monitor.get();
+    }
 }
 
 sigslot::signal<Monitor *> &Engine::monitorAdded()
@@ -1797,60 +1821,6 @@ void Engine::removeExecutableClones()
         m_executableTargets.erase(std::remove(m_executableTargets.begin(), m_executableTargets.end(), clone.get()), m_executableTargets.end());
 }
 
-void Engine::createMissingMonitors()
-{
-    // This is called when setting targets and monitors because we never know in which order they're set
-    // If there aren't any targets yet, quit
-    if (m_targets.empty())
-        return;
-
-    for (auto target : m_targets) {
-        // Read all variables
-        const auto &variables = target->variables();
-
-        for (auto variable : variables) {
-            // Find the monitor for this variable
-            auto it = std::find_if(m_monitors.begin(), m_monitors.end(), [variable](std::shared_ptr<Monitor> monitor) {
-                // TODO: Move the opcode out of Engine
-                return monitor->opcode() == "data_variable" && monitor->id() == variable->id();
-            });
-
-            // If it doesn't exist, create it
-            if (it == m_monitors.end()) {
-                auto monitor = std::make_shared<Monitor>(variable->id(), "data_variable");
-                // TODO: Move field information out of Engine
-                auto field = std::make_shared<Field>("VARIABLE", variable->name(), variable);
-                field->setFieldId(VariableBlocks::VARIABLE);
-                monitor->block()->addField(field);
-
-                addVarOrListMonitor(monitor, target.get());
-            }
-        }
-
-        // Read all lists
-        const auto &lists = target->lists();
-
-        for (auto list : lists) {
-            // Find the monitor for this list
-            auto it = std::find_if(m_monitors.begin(), m_monitors.end(), [list](std::shared_ptr<Monitor> monitor) {
-                // TODO: Move the opcode out of Engine
-                return monitor->opcode() == "data_listcontents" && monitor->id() == list->id();
-            });
-
-            // If it doesn't exist, create it
-            if (it == m_monitors.end()) {
-                auto monitor = std::make_shared<Monitor>(list->id(), "data_listcontents");
-                // TODO: Move field information out of Engine
-                auto field = std::make_shared<Field>("LIST", list->name(), list);
-                field->setFieldId(ListBlocks::LIST);
-                monitor->block()->addField(field);
-
-                addVarOrListMonitor(monitor, target.get());
-            }
-        }
-    }
-}
-
 void Engine::addVarOrListMonitor(std::shared_ptr<Monitor> monitor, Target *target)
 {
     if (!target->isStage())

src/engine/internal/engine.h

@@ -149,6 +149,8 @@ class Engine : public IEngine
 
         const std::vector<std::shared_ptr<Monitor>> &monitors() const override;
         void setMonitors(const std::vector<std::shared_ptr<Monitor>> &newMonitors) override;
+        Monitor *createVariableMonitor(std::shared_ptr<Variable> var, const std::string &opcode, const std::string &varFieldName, int varFieldId) override;
+        Monitor *createListMonitor(std::shared_ptr<List> list, const std::string &opcode, const std::string &listFieldName, int listFieldId) override;
         sigslot::signal<Monitor *> &monitorAdded() override;
         sigslot::signal<Monitor *, IMonitorHandler *> &monitorRemoved() override;
 
@@ -199,7 +201,6 @@ class Engine : public IEngine
         void finalize();
         void deleteClones();
         void removeExecutableClones();
-        void createMissingMonitors();
         void addVarOrListMonitor(std::shared_ptr<Monitor> monitor, Target *target);
         std::shared_ptr<Block> getBlock(const std::string &id, Target *target);
         std::shared_ptr<Variable> getVariable(const std::string &id, Target *target);

test/engine/engine_test.cpp

@@ -29,8 +29,6 @@
 #include "engine/internal/clock.h"
 
 // TODO: Remove this
-#include "blocks/variableblocks.h"
-#include "blocks/listblocks.h"
 #include "blocks/eventblocks.h"
 
 using namespace libscratchcpp;
@@ -1708,6 +1706,14 @@ TEST(EngineTest, Monitors)
 
 TEST(EngineTest, CreateMissingMonitors)
 {
+    static constexpr const char *VARIABLE_OPCODE = "data_variable";
+    static constexpr const char *VARIABLE_FIELD_NAME = "VARIABLE";
+    static constexpr int VARIABLE_FIELD_ID = 1;
+
+    static constexpr const char *LIST_OPCODE = "data_listcontents";
+    static constexpr const char *LIST_FIELD_NAME = "LIST";
+    static constexpr int LIST_FIELD_ID = 2;
+
     auto var1 = std::make_shared<Variable>("a", "var1");
     auto var2 = std::make_shared<Variable>("b", "var2");
     auto var3 = std::make_shared<Variable>("c", "var3");
@@ -1730,6 +1736,8 @@ TEST(EngineTest, CreateMissingMonitors)
     auto m3 = std::make_shared<Monitor>(list2->id(), "data_listcontents");
 
     auto checkVariableMonitor = [](std::shared_ptr<Monitor> monitor, std::shared_ptr<Variable> var) {
+        ASSERT_EQ(var->monitor(), monitor.get());
+
         auto block = monitor->block();
         ASSERT_EQ(monitor->id(), var->id());
         ASSERT_EQ(monitor->opcode(), "data_variable");
@@ -1738,8 +1746,8 @@ TEST(EngineTest, CreateMissingMonitors)
         ASSERT_EQ(block->fields().size(), 1);
 
         auto field = block->fieldAt(0);
-        ASSERT_EQ(field->name(), "VARIABLE");
-        ASSERT_EQ(field->fieldId(), VariableBlocks::VARIABLE);
+        ASSERT_EQ(field->name(), VARIABLE_FIELD_NAME);
+        ASSERT_EQ(field->fieldId(), VARIABLE_FIELD_ID);
         ASSERT_EQ(field->value(), var->name());
         ASSERT_EQ(field->valuePtr(), var);
 
@@ -1750,6 +1758,8 @@ TEST(EngineTest, CreateMissingMonitors)
     };
 
     auto checkListMonitor = [](std::shared_ptr<Monitor> monitor, std::shared_ptr<List> list) {
+        ASSERT_EQ(list->monitor(), monitor.get());
+
         auto block = monitor->block();
         ASSERT_EQ(monitor->id(), list->id());
         ASSERT_EQ(monitor->opcode(), "data_listcontents");
@@ -1758,8 +1768,8 @@ TEST(EngineTest, CreateMissingMonitors)
         ASSERT_EQ(block->fields().size(), 1);
 
         auto field = block->fieldAt(0);
-        ASSERT_EQ(field->name(), "LIST");
-        ASSERT_EQ(field->fieldId(), ListBlocks::LIST);
+        ASSERT_EQ(field->name(), LIST_FIELD_NAME);
+        ASSERT_EQ(field->fieldId(), LIST_FIELD_ID);
         ASSERT_EQ(field->value(), list->name());
         ASSERT_EQ(field->valuePtr(), list);
 
@@ -1769,40 +1779,34 @@ TEST(EngineTest, CreateMissingMonitors)
             ASSERT_EQ(monitor->sprite(), dynamic_cast<Sprite *>(list->target()));
     };
 
-    // Set monitors after setting targets
     {
         Engine engine;
         engine.setTargets({ target1, target2 });
         engine.setMonitors({ m1, m2, m3 });
 
         const auto &monitors = engine.monitors();
-        ASSERT_EQ(monitors.size(), 7);
+        ASSERT_EQ(monitors.size(), 3);
         ASSERT_EQ(monitors[0], m1);
         ASSERT_EQ(monitors[1], m2);
         ASSERT_EQ(monitors[2], m3);
-        checkVariableMonitor(monitors[3], var2);
-        checkListMonitor(monitors[4], list1);
-        checkVariableMonitor(monitors[5], var4);
-        checkVariableMonitor(monitors[6], var5);
-    }
 
-    // Set monitors before setting targets
-    {
-        Engine engine;
-        engine.setMonitors({ m1, m2, m3 });
-        engine.setTargets({ target1, target2 });
+        engine.createVariableMonitor(var2, VARIABLE_OPCODE, VARIABLE_FIELD_NAME, VARIABLE_FIELD_ID);
+        engine.createListMonitor(list1, LIST_OPCODE, LIST_FIELD_NAME, LIST_FIELD_ID);
+        engine.createVariableMonitor(var4, VARIABLE_OPCODE, VARIABLE_FIELD_NAME, VARIABLE_FIELD_ID);
+        engine.createVariableMonitor(var5, VARIABLE_OPCODE, VARIABLE_FIELD_NAME, VARIABLE_FIELD_ID);
 
-        const auto &monitors = engine.monitors();
         ASSERT_EQ(monitors.size(), 7);
-        ASSERT_EQ(monitors[0], m1);
-        ASSERT_EQ(monitors[1], m2);
-        ASSERT_EQ(monitors[2], m3);
         checkVariableMonitor(monitors[3], var2);
         checkListMonitor(monitors[4], list1);
         checkVariableMonitor(monitors[5], var4);
         checkVariableMonitor(monitors[6], var5);
     }
 
+    var2->setMonitor(nullptr);
+    list1->setMonitor(nullptr);
+    var4->setMonitor(nullptr);
+    var5->setMonitor(nullptr);
+
     {
         Engine engine;
         AddRemoveMonitorMock addMonitorMock;
@@ -1816,7 +1820,11 @@ TEST(EngineTest, CreateMissingMonitors)
 
         Monitor *m4, *m5, *m6, *m7;
         EXPECT_CALL(addMonitorMock, monitorAdded(_)).WillOnce(SaveArg<0>(&m4)).WillOnce(SaveArg<0>(&m5)).WillOnce(SaveArg<0>(&m6)).WillOnce(SaveArg<0>(&m7));
-        engine.setTargets({ target1, target2 });
+
+        engine.createVariableMonitor(var2, VARIABLE_OPCODE, VARIABLE_FIELD_NAME, VARIABLE_FIELD_ID);
+        engine.createListMonitor(list1, LIST_OPCODE, LIST_FIELD_NAME, LIST_FIELD_ID);
+        engine.createVariableMonitor(var4, VARIABLE_OPCODE, VARIABLE_FIELD_NAME, VARIABLE_FIELD_ID);
+        engine.createVariableMonitor(var5, VARIABLE_OPCODE, VARIABLE_FIELD_NAME, VARIABLE_FIELD_ID);
 
         const auto &monitors = engine.monitors();
         ASSERT_EQ(monitors.size(), 7);

test/load_project/load_project_test.cpp

@@ -454,7 +454,7 @@ TEST(LoadProjectTest, LoadTestProject)
 
         // Monitors
         const auto &monitors = engine->monitors();
-        ASSERT_EQ(monitors.size(), 11);
+        ASSERT_EQ(monitors.size(), 10);
 
         // list1
         auto monitor = monitors[0];

test/mocks/enginemock.h

@@ -128,6 +128,8 @@ class EngineMock : public IEngine
 
         MOCK_METHOD(const std::vector<std::shared_ptr<Monitor>> &, monitors, (), (const, override));
         MOCK_METHOD(void, setMonitors, (const std::vector<std::shared_ptr<Monitor>> &), (override));
+        MOCK_METHOD(Monitor *, createVariableMonitor, (std::shared_ptr<Variable>, const std::string &, const std::string &, int), (override));
+        MOCK_METHOD(Monitor *, createListMonitor, (std::shared_ptr<List>, const std::string &, const std::string &, int), (override));
         MOCK_METHOD(sigslot::signal<Monitor *> &, monitorAdded, (), (override));
         MOCK_METHOD((sigslot::signal<Monitor *, IMonitorHandler *> &), monitorRemoved, (), (override));