Separate out monitors for blocks reporter blocks that have parameters.

src/blocks/scratch3_looks.js

@@ -3,6 +3,7 @@ const Clone = require('../util/clone');
 const RenderedTarget = require('../sprites/rendered-target');
 const uid = require('../util/uid');
 const StageLayering = require('../engine/stage-layering');
+const getMonitorIdForBlockWithArgs = require('../util/get-monitor-id');
 
 /**
  * @typedef {object} BubbleState - the bubble state associated with a particular target.
@@ -272,10 +273,10 @@ class Scratch3LooksBlocks {
             },
             looks_costumenumbername: {
                 isSpriteSpecific: true,
-                getId: targetId => `${targetId}_costumenumbername`
+                getId: (targetId, params) => getMonitorIdForBlockWithArgs(`${targetId}_costumenumbername`, params)
             },
             looks_backdropnumbername: {
-                getId: () => 'backdropnumbername'
+                getId: (_, params) => getMonitorIdForBlockWithArgs('backdropnumbername', params)
             }
         };
     }

src/blocks/scratch3_sensing.js

@@ -1,5 +1,6 @@
 const Cast = require('../util/cast');
 const Timer = require('../util/timer');
+const getMonitorIdForBlockWithArgs = require('../util/get-monitor-id');
 
 class Scratch3SensingBlocks {
     constructor (runtime) {
@@ -89,7 +90,7 @@ class Scratch3SensingBlocks {
                 // This is different from the default toolbox xml id in order to support
                 // importing multiple monitors from the same opcode from sb2 files,
                 // something that is not currently supported in scratch 3.
-                getId: (_, param) => `current_${param}`
+                getId: (_, params) => getMonitorIdForBlockWithArgs('current', params) // _${param}`
             }
         };
     }

src/engine/blocks.js

@@ -7,6 +7,7 @@ const {Map} = require('immutable');
 const BlocksExecuteCache = require('./blocks-execute-cache');
 const log = require('../util/log');
 const Variable = require('./variable');
+const getMonitorIdForBlockWithArgs = require('../util/get-monitor-id');
 
 /**
  * @fileoverview
@@ -523,11 +524,20 @@ class Blocks {
     changeBlock (args, optRuntime) {
         // Validate
         if (['field', 'mutation', 'checkbox'].indexOf(args.element) === -1) return;
-        const block = this._blocks[args.id];
+        let block = this._blocks[args.id];
         if (typeof block === 'undefined') return;
-        const wasMonitored = block.isMonitored;
         switch (args.element) {
         case 'field':
+            // TODO when the field of a monitored block changes,
+            // update the checkbox in the flyout based on whether
+            // a monitor for that current combination of selected parameters exists
+            // e.g.
+            // 1. check (current [v year])
+            // 2. switch dropdown in flyout block to (current [v minute])
+            // 3. the checkbox should become unchecked if we're not already
+            //    monitoring current minute
+
+
             // Update block value
             if (!block.fields[args.name]) return;
             if (args.name === 'VARIABLE' || args.name === 'LIST' ||
@@ -559,11 +569,36 @@ class Blocks {
             block.mutation = mutationAdapter(args.value);
             break;
         case 'checkbox': {
-            block.isMonitored = args.value;
             if (!optRuntime) {
                 break;
             }
 
+            // A checkbox usually has a one to one correspondence with the monitor
+            // block but in the case of monitored reporters that have arguments,
+            // map the old id to a new id, creating a new monitor block if necessary
+            if (block.fields && Object.keys(block.fields).length > 0 &&
+                block.opcode !== 'data_variable' && block.opcode !== 'data_listcontents') {
+
+                // This block has an argument which needs to get separated out into multiple monitor blocks
+                const params = Object.keys(block.fields).map(k => block.fields[k].value);
+                const newId = getMonitorIdForBlockWithArgs(block.id, params);
+                // Note: we're not just constantly creating a longer and longer id everytime we check
+                // the checkbox because we're using the id of the block in the flyout as the base
+
+                // check if a block withl the new id already exists, otherwise create
+                let newBlock = optRuntime.monitorBlocks.getBlock(newId);
+                if (!newBlock) {
+                    newBlock = JSON.parse(JSON.stringify(block));
+                    newBlock.id = newId;
+                    optRuntime.monitorBlocks.createBlock(newBlock);
+                }
+
+                block = newBlock; // Carry on through the rest of this code with newBlock
+            }
+
+            const wasMonitored = block.isMonitored;
+            block.isMonitored = args.value;
+
             // Variable blocks may be sprite specific depending on the owner of the variable
             let isSpriteLocalVariable = false;
             if (block.opcode === 'data_variable') {

src/serialization/sb2.js

@@ -297,7 +297,13 @@ const parseMonitorObject = (object, runtime, targets, extensions) => {
     } else if (object.cmd === 'contentsOfList:') {
         block.id = getVariableId(object.param, Variable.LIST_TYPE);
     } else if (runtime.monitorBlockInfo.hasOwnProperty(block.opcode)) {
-        block.id = runtime.monitorBlockInfo[block.opcode].getId(target.id, object.param);
+        block.id = runtime.monitorBlockInfo[block.opcode].getId(target.id, [object.param]);
+    } else {
+        // If the opcode can't be found in the runtime monitorBlockInfo,
+        // then default to using the block opcode as the id instead.
+        // This is for extension monitors, and assumes that extension monitors
+        // cannot be sprite specific.
+        block.id = block.opcode;
     }
 
     // Block needs a targetId if it is targetting something other than the stage
@@ -306,10 +312,19 @@ const parseMonitorObject = (object, runtime, targets, extensions) => {
     // Property required for running monitored blocks.
     block.isMonitored = object.visible;
 
-    // Blocks can be created with children, flatten and add to monitorBlocks.
-    const newBlocks = flatten([block]);
-    for (let i = 0; i < newBlocks.length; i++) {
-        runtime.monitorBlocks.createBlock(newBlocks[i]);
+    const existingMonitorBlock = runtime.monitorBlocks._blocks[block.id];
+    if (existingMonitorBlock) {
+        // A monitor block already exists if the toolbox has been loaded and
+        // the monitor block is not target specific (because the block gets recycled).
+        // Update the existing block with the relevant monitor information.
+        existingMonitorBlock.isMonitored = object.visible;
+        existingMonitorBlock.targetId = block.targetId;
+    } else {
+        // Blocks can be created with children, flatten and add to monitorBlocks.
+        const newBlocks = flatten([block]);
+        for (let i = 0; i < newBlocks.length; i++) {
+            runtime.monitorBlocks.createBlock(newBlocks[i]);
+        }
     }
 
     // Convert numbered mode into strings for better understandability.

src/util/get-monitor-id.js

@@ -0,0 +1,21 @@
+/**
+ * Returns a string representing a unique id for a monitored block
+ * where a single reporter block can have more than one monitor
+ * (and therefore more than one monitor block) associated
+ * with it (e.g. when reporter blocks have inputs).
+ * @param {string} baseId The base id to use for the different monitor blocks
+ * @param {string[]} params A list of strings representing selected
+ * parameters on the block.
+ */
+// TODO this function should eventually be the single place where all monitor
+// IDs are obtained given an opcode for the reporter block and the list of
+// selected parameters.
+const getMonitorIdForBlockWithArgs = function (id, params) {
+    let fieldString = '';
+    for (const param of params) {
+        fieldString += `_${param}`;
+    }
+    return `${id}${fieldString}`;
+};
+
+module.exports = getMonitorIdForBlockWithArgs;