Merge pull request #1742 from kchadha/multi-monitors

Multi monitors

package-lock.json

@@ -11899,9 +11899,9 @@
       }
     },
     "scratch-render": {
-      "version": "0.1.0-prerelease.20181024220305",
-      "resolved": "https://registry.npmjs.org/scratch-render/-/scratch-render-0.1.0-prerelease.20181024220305.tgz",
-      "integrity": "sha512-wbCVQW6qNJUsuUv6YDTfsKfmXCwI2r0W6wWqstNS2HojNigpYeBVG2SHFPQp/zDOzp/6q/uQkTN/pBOuDoEJpw==",
+      "version": "0.1.0-prerelease.20181102130522",
+      "resolved": "https://registry.npmjs.org/scratch-render/-/scratch-render-0.1.0-prerelease.20181102130522.tgz",
+      "integrity": "sha512-r6V3zzwXalhWR5xWN4miNht7FbYqF6On+p3n2USK4r2D6oaE5vh2aMwKSjt9OfEf/hfFJK63mJd+qeuJ1slKVQ==",
       "dev": true,
       "requires": {
         "grapheme-breaker": "0.3.2",

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, fields) => getMonitorIdForBlockWithArgs(`${targetId}_costumenumbername`, fields)
             },
             looks_backdropnumbername: {
-                getId: () => 'backdropnumbername'
+                getId: (_, fields) => getMonitorIdForBlockWithArgs('backdropnumbername', fields)
             }
         };
     }

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: (_, fields) => getMonitorIdForBlockWithArgs('current', fields) // _${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 with ids based on the selected argument
+                const newId = getMonitorIdForBlockWithArgs(block.id, block.fields);
+                // 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 with 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, block.fields);
+    } 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.
@@ -1013,6 +1028,15 @@ const parseBlock = function (sb2block, addBroadcastMsg, getVariableId, extension
                 value: providedArg
             };
 
+            if (expectedArg.fieldName === 'CURRENTMENU') {
+                // In 3.0, the field value of the `sensing_current` block
+                // is in all caps.
+                activeBlock.fields[expectedArg.fieldName].value = providedArg.toUpperCase();
+                if (providedArg === 'day of week') {
+                    activeBlock.fields[expectedArg.fieldName].value = 'DAYOFWEEK';
+                }
+            }
+
             if (expectedArg.fieldName === 'VARIABLE') {
                 // Add `id` property to variable fields
                 activeBlock.fields[expectedArg.fieldName].id = getVariableId(providedArg, Variable.SCALAR_TYPE);

src/util/get-monitor-id.js

@@ -0,0 +1,33 @@
+/**
+ * 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 {object} fields The monitor block's fields object.
+ */
+// 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, fields) {
+    let fieldString = '';
+    for (const fieldKey in fields) {
+        let fieldValue = fields[fieldKey].value;
+        if (fieldKey === 'CURRENTMENU') {
+            // The 'sensing_current' block has field values in all caps.
+            // However, when importing from scratch 2.0, these
+            // could have gotten imported as lower case field values.
+            // Normalize the field value here so that we don't ever
+            // end up with a different monitor ID representing the same
+            // block configuration
+            // Note: we are not doing this for every block field that comes into
+            // this function so as not to make the faulty assumption that block
+            // field values coming in would be unique after being made lower case
+            fieldValue = fieldValue.toLowerCase();
+        }
+        fieldString += `_${fieldValue}`;
+    }
+    return `${id}${fieldString}`;
+};
+
+module.exports = getMonitorIdForBlockWithArgs;

test/integration/monitors.js

@@ -17,10 +17,10 @@ test('importing sb2 project with monitors', t => {
         // All monitors should create threads that finish during the step and
         // are revoved from runtime.threads.
         t.equal(threads.length, 0);
-        t.equal(vm.runtime._lastStepDoneThreads.length, 5);
+        t.equal(vm.runtime._lastStepDoneThreads.length, 8);
         // There should be one additional hidden monitor that is in the monitorState but
         // does not start a thread.
-        t.equal(vm.runtime._monitorState.size, 6);
+        t.equal(vm.runtime._monitorState.size, 9);
 
         const stage = vm.runtime.targets[0];
         const target = vm.runtime.targets[1];
@@ -56,11 +56,13 @@ test('importing sb2 project with monitors', t => {
         t.equal(monitorRecord.opcode, 'data_listcontents');
         t.equal(monitorRecord.mode, 'list');
         t.equal(monitorRecord.visible, true);
-        t.equal(monitorRecord.width, 102); // Make sure these are imported from lists.
-        t.equal(monitorRecord.height, 202);
+        t.equal(monitorRecord.width, 104); // Make sure these are imported from lists.
+        t.equal(monitorRecord.height, 204);
 
         // Backdrop name monitor is visible, not sprite specific
-        monitorRecord = vm.runtime._monitorState.get('backdropnumbername');
+        // should get imported with id that references the name parameter
+        // via '_name' at the end since the 3.0 block has a dropdown.
+        monitorRecord = vm.runtime._monitorState.get('backdropnumbername_name');
         t.equal(monitorRecord.opcode, 'looks_backdropnumbername');
         t.equal(monitorRecord.mode, 'default');
         t.equal(monitorRecord.visible, true);
@@ -75,6 +77,45 @@ test('importing sb2 project with monitors', t => {
         t.equal(monitorRecord.spriteName, 'Sprite1');
         t.equal(monitorRecord.targetId, target.id);
 
+
+        let monitorId;
+        let monitorBlock;
+
+        // The monitor IDs for the sensing_current block should be unique
+        // to the parameter that is selected on the block being monitored.
+        // The paramater portion of the id should be lowercase even
+        // though the field value on the block is uppercase.
+
+        monitorId = 'current_date';
+        monitorRecord = vm.runtime._monitorState.get(monitorId);
+        t.equal(monitorRecord.opcode, 'sensing_current');
+        monitorBlock = vm.runtime.monitorBlocks.getBlock(monitorId);
+        t.equal(monitorBlock.fields.CURRENTMENU.value, 'DATE');
+        t.equal(monitorRecord.mode, 'default');
+        t.equal(monitorRecord.visible, true);
+        t.equal(monitorRecord.spriteName, null);
+        t.equal(monitorRecord.targetId, null);
+
+        monitorId = 'current_minute';
+        monitorRecord = vm.runtime._monitorState.get(monitorId);
+        t.equal(monitorRecord.opcode, 'sensing_current');
+        monitorBlock = vm.runtime.monitorBlocks.getBlock(monitorId);
+        t.equal(monitorBlock.fields.CURRENTMENU.value, 'MINUTE');
+        t.equal(monitorRecord.mode, 'default');
+        t.equal(monitorRecord.visible, true);
+        t.equal(monitorRecord.spriteName, null);
+        t.equal(monitorRecord.targetId, null);
+
+        monitorId = 'current_dayofweek';
+        monitorRecord = vm.runtime._monitorState.get(monitorId);
+        t.equal(monitorRecord.opcode, 'sensing_current');
+        monitorBlock = vm.runtime.monitorBlocks.getBlock(monitorId);
+        t.equal(monitorBlock.fields.CURRENTMENU.value, 'DAYOFWEEK');
+        t.equal(monitorRecord.mode, 'default');
+        t.equal(monitorRecord.visible, true);
+        t.equal(monitorRecord.spriteName, null);
+        t.equal(monitorRecord.targetId, null);
+
         t.end();
         process.nextTick(process.exit);
     });