diff --git a/package-lock.json b/package-lock.json index 0791258428a..2cfc11a5829 100644 --- a/package-lock.json +++ b/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", diff --git a/src/blocks/scratch3_looks.js b/src/blocks/scratch3_looks.js index 21e56b376d4..ebf40e5552c 100644 --- a/src/blocks/scratch3_looks.js +++ b/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) } }; } diff --git a/src/blocks/scratch3_sensing.js b/src/blocks/scratch3_sensing.js index 2dcb23ed650..6307b4c6a2c 100644 --- a/src/blocks/scratch3_sensing.js +++ b/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}` } }; } diff --git a/src/engine/blocks.js b/src/engine/blocks.js index 858871f699c..00cc5176131 100644 --- a/src/engine/blocks.js +++ b/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') { diff --git a/src/serialization/sb2.js b/src/serialization/sb2.js index ac8eda01995..d45fbffe166 100644 --- a/src/serialization/sb2.js +++ b/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); diff --git a/src/util/get-monitor-id.js b/src/util/get-monitor-id.js new file mode 100644 index 00000000000..8582db23184 --- /dev/null +++ b/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; diff --git a/test/fixtures/monitors.sb2 b/test/fixtures/monitors.sb2 index d4117a83ca2..2b9ede37694 100644 Binary files a/test/fixtures/monitors.sb2 and b/test/fixtures/monitors.sb2 differ diff --git a/test/integration/monitors.js b/test/integration/monitors.js index 38e89b43e36..dc9a9e9e46c 100644 --- a/test/integration/monitors.js +++ b/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); });