From 0a15190b858bfe2d2ea7810fdb15af53a0a423c1 Mon Sep 17 00:00:00 2001 From: Karishma Chadha Date: Tue, 21 Nov 2017 16:48:48 -0500 Subject: [PATCH 1/5] Broadcast message functionality (works with creating new messages, and switching back and forth between them as well). This commit includes a temporary workaround for the issue where the default broadcast message, 'message1' wasn't triggering a var_create event (filed in LLK/scratch-blocks#1258). --- src/blocks/scratch3_event.js | 10 ++++++++-- src/engine/blocks.js | 3 ++- src/engine/execute.js | 13 +++++++++++++ src/engine/target.js | 19 ++++++++++++++++++- src/engine/variable.js | 11 +++++++++++ 5 files changed, 52 insertions(+), 4 deletions(-) diff --git a/src/blocks/scratch3_event.js b/src/blocks/scratch3_event.js index 8687141c..1bf798f4 100644 --- a/src/blocks/scratch3_event.js +++ b/src/blocks/scratch3_event.js @@ -56,14 +56,20 @@ class Scratch3EventBlocks { } broadcast (args, util) { - const broadcastOption = Cast.toString(args.BROADCAST_OPTION); + // TODO KARISHMA adding lookupOrCreate here temporarily (LLK/scratch-blocks#1258) + util.target.lookupOrCreateBroadcastMsg(args.BROADCAST_OPTION.id, + args.BROADCAST_OPTION.name); + const broadcastOption = Cast.toString(args.BROADCAST_OPTION.name); util.startHats('event_whenbroadcastreceived', { BROADCAST_OPTION: broadcastOption }); } broadcastAndWait (args, util) { - const broadcastOption = Cast.toString(args.BROADCAST_OPTION); + // TODO KARISHMA adding lookupOrCreate here temporarily (LLK/scratch-blocks#1258) + util.target.lookupOrCreateBroadcastMsg(args.BROADCAST_OPTION.id, + args.BROADCAST_OPTION.name); + const broadcastOption = Cast.toString(args.BROADCAST_OPTION.name); // Have we run before, starting threads? if (!util.stackFrame.startedThreads) { // No - start hats for this broadcast. diff --git a/src/engine/blocks.js b/src/engine/blocks.js index 8776c6f6..9117e991 100644 --- a/src/engine/blocks.js +++ b/src/engine/blocks.js @@ -365,7 +365,8 @@ class Blocks { case 'field': // Update block value if (!block.fields[args.name]) return; - if (args.name === 'VARIABLE' || args.name === 'LIST') { + if (args.name === 'VARIABLE' || args.name === 'LIST' || + args.name === 'BROADCAST_OPTION') { // Get variable name using the id in args.value. const variable = optRuntime.getEditingTarget().lookupVariableById(args.value); if (variable) { diff --git a/src/engine/execute.js b/src/engine/execute.js index f5acf909..3d2e5aa6 100644 --- a/src/engine/execute.js +++ b/src/engine/execute.js @@ -171,6 +171,19 @@ const execute = function (sequencer, thread) { if (!fields.hasOwnProperty(fieldName)) continue; if (fieldName === 'VARIABLE' || fieldName === 'LIST') { argValues[fieldName] = fields[fieldName].id; + } else if (fieldName === 'BROADCAST_OPTION') { + // TODO KARISHMA (scratch-blocks #1258) this will change when we + // upstream blockly issue #1395 is fixed. + + // we need both the id and the name here, because + // this object eventually gets passed to scratch3_event.broadcast + // which will create the message for the first time if one with the + // given id doesn't exist + // (e.g. default broadcast message, 'message1') + argValues[fieldName] = { + id: fields[fieldName].id, + name: fields[fieldName].value + }; } else { argValues[fieldName] = fields[fieldName].value; } diff --git a/src/engine/target.js b/src/engine/target.js index 6a5c8d12..9a6ad728 100644 --- a/src/engine/target.js +++ b/src/engine/target.js @@ -92,6 +92,23 @@ class Target extends EventEmitter { return newVariable; } + /** + * Look up a broadcast message object, and create it if one doesn't exist. + * @param {string} id Id of the variable. + * @param {string} name Name of the variable. + * @return {!Variable} Variable object. + */ + lookupOrCreateBroadcastMsg (id, name) { + debugger; + const broadcastMsg = this.lookupVariableById(id); + if (broadcastMsg) return broadcastMsg; + // No variable with this name exists - create it locally. + const newBroadcastMsg = new Variable(id, name, + Variable.BROADCAST_MESSAGE_TYPE, false); + this.variables[id] = newBroadcastMsg; + return newBroadcastMsg; + } + /** * Look up a variable object. * Search begins for local variables; then look for globals. @@ -134,7 +151,7 @@ class Target extends EventEmitter { * dictionary of variables. * @param {string} id Id of variable * @param {string} name Name of variable. - * @param {string} type Type of variable, '' or 'list' + * @param {string} type Type of variable, '', 'broadcast_msg', or 'list' */ createVariable (id, name, type) { if (!this.variables.hasOwnProperty(id)) { diff --git a/src/engine/variable.js b/src/engine/variable.js index d75c0b82..4aad9ffc 100644 --- a/src/engine/variable.js +++ b/src/engine/variable.js @@ -25,6 +25,9 @@ class Variable { case Variable.LIST_TYPE: this.value = []; break; + case Variable.BROADCAST_MESSAGE_TYPE: + this.value = this.name; + break; default: throw new Error(`Invalid variable type: ${this.type}`); } @@ -51,6 +54,14 @@ class Variable { static get LIST_TYPE () { return 'list'; } + + /** + * Type representation for list variables. + * @const {string} + */ + static get BROADCAST_MESSAGE_TYPE () { + return 'broadcast_msg'; + } } module.exports = Variable; From b674a0c047afb91083db2369ce9e9c3a3c6ade9b Mon Sep 17 00:00:00 2001 From: Karishma Chadha Date: Tue, 28 Nov 2017 22:33:27 -0500 Subject: [PATCH 2/5] Code cleanup and refactoring, and getting scratch3_event broadcast functionality to look up the variable name instead of getting it ad-hoc. --- src/blocks/scratch3_data.js | 33 ++++++++++++++++++++++----------- src/blocks/scratch3_event.js | 14 ++++++-------- src/engine/execute.js | 13 ++----------- src/engine/target.js | 1 - 4 files changed, 30 insertions(+), 31 deletions(-) diff --git a/src/blocks/scratch3_data.js b/src/blocks/scratch3_data.js index 8ec4f124..0a382e28 100644 --- a/src/blocks/scratch3_data.js +++ b/src/blocks/scratch3_data.js @@ -30,24 +30,28 @@ class Scratch3DataBlocks { } getVariable (args, util) { - const variable = util.target.lookupOrCreateVariable(args.VARIABLE); + const variable = util.target.lookupOrCreateVariable( + args.VARIABLE.id, args.VARIABLE.name); return variable.value; } setVariableTo (args, util) { - const variable = util.target.lookupOrCreateVariable(args.VARIABLE); + const variable = util.target.lookupOrCreateVariable( + args.VARIABLE.id, args.VARIABLE.name); variable.value = args.VALUE; } changeVariableBy (args, util) { - const variable = util.target.lookupOrCreateVariable(args.VARIABLE); + const variable = util.target.lookupOrCreateVariable( + args.VARIABLE.id, args.VARIABLE.name); const castedValue = Cast.toNumber(variable.value); const dValue = Cast.toNumber(args.VALUE); variable.value = castedValue + dValue; } getListContents (args, util) { - const list = util.target.lookupOrCreateList(args.LIST); + const list = util.target.lookupOrCreateList( + args.LIST.id, args.LIST.name); // Determine if the list is all single letters. // If it is, report contents joined together with no separator. // If it's not, report contents joined together with a space. @@ -68,12 +72,14 @@ class Scratch3DataBlocks { } addToList (args, util) { - const list = util.target.lookupOrCreateList(args.LIST); + const list = util.target.lookupOrCreateList( + args.LIST.id, args.LIST.name); list.value.push(args.ITEM); } deleteOfList (args, util) { - const list = util.target.lookupOrCreateList(args.LIST); + const list = util.target.lookupOrCreateList( + args.LIST.id, args.LIST.name); const index = Cast.toListIndex(args.INDEX, list.value.length); if (index === Cast.LIST_INVALID) { return; @@ -86,7 +92,8 @@ class Scratch3DataBlocks { insertAtList (args, util) { const item = args.ITEM; - const list = util.target.lookupOrCreateList(args.LIST); + const list = util.target.lookupOrCreateList( + args.LIST.id, args.LIST.name); const index = Cast.toListIndex(args.INDEX, list.value.length + 1); if (index === Cast.LIST_INVALID) { return; @@ -96,7 +103,8 @@ class Scratch3DataBlocks { replaceItemOfList (args, util) { const item = args.ITEM; - const list = util.target.lookupOrCreateList(args.LIST); + const list = util.target.lookupOrCreateList( + args.LIST.id, args.LIST.name); const index = Cast.toListIndex(args.INDEX, list.value.length); if (index === Cast.LIST_INVALID) { return; @@ -105,7 +113,8 @@ class Scratch3DataBlocks { } getItemOfList (args, util) { - const list = util.target.lookupOrCreateList(args.LIST); + const list = util.target.lookupOrCreateList( + args.LIST.id, args.LIST.name); const index = Cast.toListIndex(args.INDEX, list.value.length); if (index === Cast.LIST_INVALID) { return ''; @@ -114,13 +123,15 @@ class Scratch3DataBlocks { } lengthOfList (args, util) { - const list = util.target.lookupOrCreateList(args.LIST); + const list = util.target.lookupOrCreateList( + args.LIST.id, args.LIST.name); return list.value.length; } listContainsItem (args, util) { const item = args.ITEM; - const list = util.target.lookupOrCreateList(args.LIST); + const list = util.target.lookupOrCreateList( + args.LIST.id, args.LIST.name); if (list.value.indexOf(item) >= 0) { return true; } diff --git a/src/blocks/scratch3_event.js b/src/blocks/scratch3_event.js index 1bf798f4..5fbaa5b7 100644 --- a/src/blocks/scratch3_event.js +++ b/src/blocks/scratch3_event.js @@ -56,20 +56,18 @@ class Scratch3EventBlocks { } broadcast (args, util) { - // TODO KARISHMA adding lookupOrCreate here temporarily (LLK/scratch-blocks#1258) - util.target.lookupOrCreateBroadcastMsg(args.BROADCAST_OPTION.id, - args.BROADCAST_OPTION.name); - const broadcastOption = Cast.toString(args.BROADCAST_OPTION.name); + const broadcastVar = util.runtime.getTargetForStage().lookupOrCreateBroadcastMsg( + args.BROADCAST_OPTION.id, args.BROADCAST_OPTION.name); + const broadcastOption = broadcastVar.name; util.startHats('event_whenbroadcastreceived', { BROADCAST_OPTION: broadcastOption }); } broadcastAndWait (args, util) { - // TODO KARISHMA adding lookupOrCreate here temporarily (LLK/scratch-blocks#1258) - util.target.lookupOrCreateBroadcastMsg(args.BROADCAST_OPTION.id, - args.BROADCAST_OPTION.name); - const broadcastOption = Cast.toString(args.BROADCAST_OPTION.name); + const broadcastVar = util.runtime.getTargetForStage().lookupOrCreateBroadcastMsg( + args.BROADCAST_OPTION.id, args.BROADCAST_OPTION.name); + const broadcastOption = broadcastVar.name; // Have we run before, starting threads? if (!util.stackFrame.startedThreads) { // No - start hats for this broadcast. diff --git a/src/engine/execute.js b/src/engine/execute.js index 3d2e5aa6..4b1c0abf 100644 --- a/src/engine/execute.js +++ b/src/engine/execute.js @@ -169,17 +169,8 @@ const execute = function (sequencer, thread) { // Add all fields on this block to the argValues. for (const fieldName in fields) { if (!fields.hasOwnProperty(fieldName)) continue; - if (fieldName === 'VARIABLE' || fieldName === 'LIST') { - argValues[fieldName] = fields[fieldName].id; - } else if (fieldName === 'BROADCAST_OPTION') { - // TODO KARISHMA (scratch-blocks #1258) this will change when we - // upstream blockly issue #1395 is fixed. - - // we need both the id and the name here, because - // this object eventually gets passed to scratch3_event.broadcast - // which will create the message for the first time if one with the - // given id doesn't exist - // (e.g. default broadcast message, 'message1') + if (fieldName === 'VARIABLE' || fieldName === 'LIST' || + fieldName === 'BROADCAST_OPTION') { argValues[fieldName] = { id: fields[fieldName].id, name: fields[fieldName].value diff --git a/src/engine/target.js b/src/engine/target.js index 9a6ad728..39273fcc 100644 --- a/src/engine/target.js +++ b/src/engine/target.js @@ -99,7 +99,6 @@ class Target extends EventEmitter { * @return {!Variable} Variable object. */ lookupOrCreateBroadcastMsg (id, name) { - debugger; const broadcastMsg = this.lookupVariableById(id); if (broadcastMsg) return broadcastMsg; // No variable with this name exists - create it locally. From e5378d323d3eece585f8e48a356769eec529271a Mon Sep 17 00:00:00 2001 From: Karishma Chadha Date: Wed, 29 Nov 2017 10:43:07 -0500 Subject: [PATCH 3/5] Adding unit tests for new lookupOrCreate funciton, fixing a bug that came up during testing (using lookupVariableById on a null target). Skipping integration tests that do not work right now because SB2 import of broadcast message blocks has not been implemented yet. --- src/engine/blocks.js | 9 ++++++++- test/integration/data.js | 4 +++- test/integration/event.js | 4 +++- test/integration/motion.js | 4 +++- test/unit/blocks_event.js | 14 ++++++++------ test/unit/engine_target.js | 29 +++++++++++++++++++++++++++++ 6 files changed, 54 insertions(+), 10 deletions(-) diff --git a/src/engine/blocks.js b/src/engine/blocks.js index 9117e991..ed0cc4d8 100644 --- a/src/engine/blocks.js +++ b/src/engine/blocks.js @@ -304,7 +304,14 @@ class Blocks { // Check if this variable exists on the current target or stage. // If not, create it on the stage. // TODO create global and local variables when UI provides a way. - if (!optRuntime.getEditingTarget().lookupVariableById(e.varId)) { + if (optRuntime.getEditingTarget()) { + if (!optRuntime.getEditingTarget().lookupVariableById(e.varId)) { + stage.createVariable(e.varId, e.varName, e.varType); + } + } else if (!stage.lookupVariableById(e.varId)) { + // Since getEditingTarget returned null, we now need to + // explicitly check if the stage has the variable, and + // create one if not. stage.createVariable(e.varId, e.varName, e.varType); } break; diff --git a/test/integration/data.js b/test/integration/data.js index c6cd71d1..63048fc7 100644 --- a/test/integration/data.js +++ b/test/integration/data.js @@ -7,7 +7,9 @@ const VirtualMachine = require('../../src/index'); const uri = path.resolve(__dirname, '../fixtures/data.sb2'); const project = extract(uri); -test('data', t => { +// TODO (#828) Need to re-activate this test after sb2 import works for broadcast +// blocks +test('data', {todo: true}, t => { const vm = new VirtualMachine(); vm.attachStorage(makeTestStorage()); diff --git a/test/integration/event.js b/test/integration/event.js index c4f3ee58..648f8cdd 100644 --- a/test/integration/event.js +++ b/test/integration/event.js @@ -7,7 +7,9 @@ const VirtualMachine = require('../../src/index'); const uri = path.resolve(__dirname, '../fixtures/event.sb2'); const project = extract(uri); -test('event', t => { +// TODO (#828) Need to re-activate this test after sb2 import works for broadcast +// blocks +test('event', {todo: true}, t => { const vm = new VirtualMachine(); vm.attachStorage(makeTestStorage()); diff --git a/test/integration/motion.js b/test/integration/motion.js index 689b2187..dbb24a11 100644 --- a/test/integration/motion.js +++ b/test/integration/motion.js @@ -7,7 +7,9 @@ const VirtualMachine = require('../../src/index'); const uri = path.resolve(__dirname, '../fixtures/motion.sb2'); const project = extract(uri); -test('motion', t => { +// TODO (#828) Need to re-activate this test after sb2 import works for broadcast +// blocks +test('motion', {todo: true}, t => { const vm = new VirtualMachine(); vm.attachStorage(makeTestStorage()); diff --git a/test/unit/blocks_event.js b/test/unit/blocks_event.js index 2e62125b..35a1f0b4 100644 --- a/test/unit/blocks_event.js +++ b/test/unit/blocks_event.js @@ -12,7 +12,7 @@ test('#760 - broadcastAndWait', t => { id: 'broadcastAndWaitBlock', fields: { BROADCAST_OPTION: { - id: 'BROADCAST_OPTION', + id: 'testBroadcastID', value: 'message' } }, @@ -30,7 +30,7 @@ test('#760 - broadcastAndWait', t => { id: 'receiveMessageBlock', fields: { BROADCAST_OPTION: { - id: 'BROADCAST_OPTION', + id: 'testBroadcastID', value: 'message' } }, @@ -51,15 +51,17 @@ test('#760 - broadcastAndWait', t => { b.createBlock(broadcastAndWaitBlock); b.createBlock(receiveMessageBlock); const tgt = new Target(rt, b); + tgt.isStage = true; rt.targets.push(tgt); let th = rt._pushThread('broadcastAndWaitBlock', t); const util = new BlockUtility(); util.sequencer = rt.sequencer; util.thread = th; + util.runtime = rt; // creates threads - e.broadcastAndWait({BROADCAST_OPTION: 'message'}, util); + e.broadcastAndWait({BROADCAST_OPTION: {id: 'testBroadcastID', name: 'message'}}, util); t.strictEqual(rt.threads.length, 2); t.strictEqual(rt.threads[1].topBlock, 'receiveMessageBlock'); // yields when some thread is active @@ -76,18 +78,18 @@ test('#760 - broadcastAndWait', t => { // restarts done threads that are in runtime threads th = rt._pushThread('broadcastAndWaitBlock', tgt); util.thread = th; - e.broadcastAndWait({BROADCAST_OPTION: 'message'}, util); + e.broadcastAndWait({BROADCAST_OPTION: {id: 'testBroadcastID', name: 'message'}}, util); t.strictEqual(rt.threads.length, 3); t.strictEqual(rt.threads[1].status, Thread.STATUS_RUNNING); t.strictEqual(th.status, Thread.STATUS_YIELD); // yields when some restarted thread is active th.status = Thread.STATUS_RUNNING; - e.broadcastAndWait({BROADCAST_OPTION: 'message'}, util); + e.broadcastAndWait({BROADCAST_OPTION: {id: 'testBroadcastID', name: 'message'}}, util); t.strictEqual(th.status, Thread.STATUS_YIELD); // does not yield once all threads are done th.status = Thread.STATUS_RUNNING; rt.threads[1].status = Thread.STATUS_DONE; - e.broadcastAndWait({BROADCAST_OPTION: 'message'}, util); + e.broadcastAndWait({BROADCAST_OPTION: {id: 'testBroadcastID', name: 'message'}}, util); t.strictEqual(th.status, Thread.STATUS_RUNNING); t.end(); diff --git a/test/unit/engine_target.js b/test/unit/engine_target.js index 70bf46e9..4f663906 100644 --- a/test/unit/engine_target.js +++ b/test/unit/engine_target.js @@ -173,3 +173,32 @@ test('lookupOrCreateList returns list if one with given id exists', t => { t.end(); }); + +test('lookupOrCreateBroadcastMsg creates a var if one does not exist', t => { + const target = new Target(); + const variables = target.variables; + + t.equal(Object.keys(variables).length, 0); + const broadcastVar = target.lookupOrCreateBroadcastMsg('foo', 'bar'); + t.equal(Object.keys(variables).length, 1); + t.equal(broadcastVar.id, 'foo'); + t.equal(broadcastVar.name, 'bar'); + + t.end(); +}); + +test('lookupOrCreateBroadcastMsg returns the var with given id if exists', t => { + const target = new Target(); + const variables = target.variables; + + t.equal(Object.keys(variables).length, 0); + target.createVariable('foo', 'bar', Variable.BROADCAST_MESSAGE_TYPE); + t.equal(Object.keys(variables).length, 1); + + const broadcastMsg = target.lookupOrCreateBroadcastMsg('foo', 'bar'); + t.equal(Object.keys(variables).length, 1); + t.equal(broadcastMsg.id, 'foo'); + t.equal(broadcastMsg.name, 'bar'); + + t.end(); +}); From 80da989f0145d1d4ce8ef2cc2addcd40ab7a5035 Mon Sep 17 00:00:00 2001 From: Karishma Chadha Date: Fri, 1 Dec 2017 10:29:32 -0500 Subject: [PATCH 4/5] SB2 Import for Broadcast Blocks (not including the input field functionality for 'broadcast' and 'broadcast and wait '). Currently we also cannot import projects where messages and variables share names. --- src/serialization/sb2.js | 59 +++++++++++++++++++++++++++----- src/serialization/sb2_specmap.js | 15 ++++---- test/integration/data.js | 4 +-- test/integration/event.js | 4 +-- test/integration/motion.js | 4 +-- 5 files changed, 62 insertions(+), 24 deletions(-) diff --git a/src/serialization/sb2.js b/src/serialization/sb2.js index f6f03bd1..ae501bda 100644 --- a/src/serialization/sb2.js +++ b/src/serialization/sb2.js @@ -88,17 +88,18 @@ const flatten = function (blocks) { * a list of blocks in a branch (e.g., in forever), * or a list of blocks in an argument (e.g., move [pick random...]). * @param {Array.} blockList SB2 JSON-format block list. + * @param {Function} addBroadcastMsg function to update broadcast message name map * @param {Function} getVariableId function to retreive a variable's ID based on name * @param {ImportedExtensionsInfo} extensions - (in/out) parsed extension information will be stored here. * @return {Array.} Scratch VM-format block list. */ -const parseBlockList = function (blockList, getVariableId, extensions) { +const parseBlockList = function (blockList, addBroadcastMsg, getVariableId, extensions) { const resultingList = []; let previousBlock = null; // For setting next. for (let i = 0; i < blockList.length; i++) { const block = blockList[i]; // eslint-disable-next-line no-use-before-define - const parsedBlock = parseBlock(block, getVariableId, extensions); + const parsedBlock = parseBlock(block, addBroadcastMsg, getVariableId, extensions); if (typeof parsedBlock === 'undefined') continue; if (previousBlock) { parsedBlock.parent = previousBlock.id; @@ -115,16 +116,17 @@ const parseBlockList = function (blockList, getVariableId, extensions) { * This should only handle top-level scripts that include X, Y coordinates. * @param {!object} scripts Scripts object from SB2 JSON. * @param {!Blocks} blocks Blocks object to load parsed blocks into. + * @param {Function} addBroadcastMsg function to update broadcast message name map * @param {Function} getVariableId function to retreive a variable's ID based on name * @param {ImportedExtensionsInfo} extensions - (in/out) parsed extension information will be stored here. */ -const parseScripts = function (scripts, blocks, getVariableId, extensions) { +const parseScripts = function (scripts, blocks, addBroadcastMsg, getVariableId, extensions) { for (let i = 0; i < scripts.length; i++) { const script = scripts[i]; const scriptX = script[0]; const scriptY = script[1]; const blockList = script[2]; - const parsedBlockList = parseBlockList(blockList, getVariableId, extensions); + const parsedBlockList = parseBlockList(blockList, addBroadcastMsg, getVariableId, extensions); if (parsedBlockList[0]) { // Adjust script coordinates to account for // larger block size in scratch-blocks. @@ -166,6 +168,23 @@ const generateVariableIdGetter = (function () { }; }()); +const generateBroadcastMsgId = function (name) { + return `broadcastMsgId-${name}`; +}; + +const globalBroadcastMsgStateGenerator = (function () { + let broadcastMsgNameMap = {}; + return function (topLevel) { + if (topLevel) broadcastMsgNameMap = {}; + return { + broadcastMsgMapUpdater: function (name) { + broadcastMsgNameMap[name] = generateBroadcastMsgId(name); + }, + globalBroadcastMsgs: broadcastMsgNameMap + }; + }; +}()); + /** * Parse a single "Scratch object" and create all its in-memory VM objects. * TODO: parse the "info" section, especially "savedExtensions" @@ -227,6 +246,9 @@ const parseScratchObject = function (object, runtime, extensions, topLevel) { const getVariableId = generateVariableIdGetter(target.id, topLevel); + const globalBroadcastMsgObj = globalBroadcastMsgStateGenerator(topLevel); + const addBroadcastMsg = globalBroadcastMsgObj.broadcastMsgMapUpdater; + // Load target properties from JSON. if (object.hasOwnProperty('variables')) { for (let j = 0; j < object.variables.length; j++) { @@ -244,7 +266,7 @@ const parseScratchObject = function (object, runtime, extensions, topLevel) { // If included, parse any and all scripts/blocks on the object. if (object.hasOwnProperty('scripts')) { - parseScripts(object.scripts, blocks, getVariableId, extensions); + parseScripts(object.scripts, blocks, addBroadcastMsg, getVariableId, extensions); } if (object.hasOwnProperty('lists')) { @@ -317,6 +339,21 @@ const parseScratchObject = function (object, runtime, extensions, topLevel) { Promise.all( childrenPromises ).then(children => { + // Need create broadcast msgs as variables after + // all other targets have finished processing. + if (target.isStage) { + const allBroadcastMsgs = globalBroadcastMsgObj.globalBroadcastMsgs; + for (const msgName in allBroadcastMsgs) { + const msgId = allBroadcastMsgs[msgName]; + const newMsg = new Variable( + msgId, + msgName, + Variable.BROADCAST_MESSAGE_TYPE, + false + ); + target.variables[newMsg.id] = newMsg; + } + } let targets = [target]; for (let n = 0; n < children.length; n++) { targets = targets.concat(children[n]); @@ -349,11 +386,12 @@ const sb2import = function (json, runtime, optForceSprite) { /** * Parse a single SB2 JSON-formatted block and its children. * @param {!object} sb2block SB2 JSON-formatted block. + * @param {Function} addBroadcastMsg function to update broadcast message name map * @param {Function} getVariableId function to retrieve a variable's ID based on name * @param {ImportedExtensionsInfo} extensions - (in/out) parsed extension information will be stored here. * @return {object} Scratch VM format block, or null if unsupported object. */ -const parseBlock = function (sb2block, getVariableId, extensions) { +const parseBlock = function (sb2block, addBroadcastMsg, getVariableId, extensions) { // First item in block object is the old opcode (e.g., 'forward:'). const oldOpcode = sb2block[0]; // Convert the block using the specMap. See sb2specmap.js. @@ -404,10 +442,10 @@ const parseBlock = function (sb2block, getVariableId, extensions) { let innerBlocks; if (typeof providedArg[0] === 'object' && providedArg[0]) { // Block list occupies the input. - innerBlocks = parseBlockList(providedArg, getVariableId, extensions); + innerBlocks = parseBlockList(providedArg, addBroadcastMsg, getVariableId, extensions); } else { // Single block occupies the input. - innerBlocks = [parseBlock(providedArg, getVariableId, extensions)]; + innerBlocks = [parseBlock(providedArg, addBroadcastMsg, getVariableId, extensions)]; } let previousBlock = null; for (let j = 0; j < innerBlocks.length; j++) { @@ -493,6 +531,11 @@ const parseBlock = function (sb2block, getVariableId, extensions) { if (expectedArg.fieldName === 'VARIABLE' || expectedArg.fieldName === 'LIST') { // Add `id` property to variable fields activeBlock.fields[expectedArg.fieldName].id = getVariableId(providedArg); + } else if (expectedArg.fieldName === 'BROADCAST_OPTION') { + // add the name in this field to the broadcast msg name map + addBroadcastMsg(providedArg); + const broadcastId = generateBroadcastMsgId(providedArg); + activeBlock.fields[expectedArg.fieldName].id = broadcastId; } const varType = expectedArg.variableType; if (typeof varType === 'string') { diff --git a/src/serialization/sb2_specmap.js b/src/serialization/sb2_specmap.js index 9e1034bf..cc0a5696 100644 --- a/src/serialization/sb2_specmap.js +++ b/src/serialization/sb2_specmap.js @@ -656,7 +656,8 @@ const specMap = { argMap: [ { type: 'field', - fieldName: 'BROADCAST_OPTION' + fieldName: 'BROADCAST_OPTION', + variableType: Variable.BROADCAST_MESSAGE_TYPE } ] }, @@ -664,9 +665,9 @@ const specMap = { opcode: 'event_broadcast', argMap: [ { - type: 'input', - inputOp: 'event_broadcast_menu', - inputName: 'BROADCAST_OPTION' + type: 'field', + fieldName: 'BROADCAST_OPTION', + variableType: Variable.BROADCAST_MESSAGE_TYPE } ] }, @@ -674,9 +675,9 @@ const specMap = { opcode: 'event_broadcastandwait', argMap: [ { - type: 'input', - inputOp: 'event_broadcast_menu', - inputName: 'BROADCAST_OPTION' + type: 'field', + fieldName: 'BROADCAST_OPTION', + variableType: Variable.BROADCAST_MESSAGE_TYPE } ] }, diff --git a/test/integration/data.js b/test/integration/data.js index 63048fc7..c6cd71d1 100644 --- a/test/integration/data.js +++ b/test/integration/data.js @@ -7,9 +7,7 @@ const VirtualMachine = require('../../src/index'); const uri = path.resolve(__dirname, '../fixtures/data.sb2'); const project = extract(uri); -// TODO (#828) Need to re-activate this test after sb2 import works for broadcast -// blocks -test('data', {todo: true}, t => { +test('data', t => { const vm = new VirtualMachine(); vm.attachStorage(makeTestStorage()); diff --git a/test/integration/event.js b/test/integration/event.js index 648f8cdd..c4f3ee58 100644 --- a/test/integration/event.js +++ b/test/integration/event.js @@ -7,9 +7,7 @@ const VirtualMachine = require('../../src/index'); const uri = path.resolve(__dirname, '../fixtures/event.sb2'); const project = extract(uri); -// TODO (#828) Need to re-activate this test after sb2 import works for broadcast -// blocks -test('event', {todo: true}, t => { +test('event', t => { const vm = new VirtualMachine(); vm.attachStorage(makeTestStorage()); diff --git a/test/integration/motion.js b/test/integration/motion.js index dbb24a11..689b2187 100644 --- a/test/integration/motion.js +++ b/test/integration/motion.js @@ -7,9 +7,7 @@ const VirtualMachine = require('../../src/index'); const uri = path.resolve(__dirname, '../fixtures/motion.sb2'); const project = extract(uri); -// TODO (#828) Need to re-activate this test after sb2 import works for broadcast -// blocks -test('motion', {todo: true}, t => { +test('motion', t => { const vm = new VirtualMachine(); vm.attachStorage(makeTestStorage()); From aab4a45622c5448331306ea0f7b2304721d8e37d Mon Sep 17 00:00:00 2001 From: Karishma Chadha Date: Fri, 1 Dec 2017 11:27:54 -0500 Subject: [PATCH 5/5] Minor Refactoring/cleanup. --- src/serialization/sb2.js | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/serialization/sb2.js b/src/serialization/sb2.js index ae501bda..d19175dc 100644 --- a/src/serialization/sb2.js +++ b/src/serialization/sb2.js @@ -168,17 +168,14 @@ const generateVariableIdGetter = (function () { }; }()); -const generateBroadcastMsgId = function (name) { - return `broadcastMsgId-${name}`; -}; - const globalBroadcastMsgStateGenerator = (function () { let broadcastMsgNameMap = {}; return function (topLevel) { if (topLevel) broadcastMsgNameMap = {}; return { broadcastMsgMapUpdater: function (name) { - broadcastMsgNameMap[name] = generateBroadcastMsgId(name); + broadcastMsgNameMap[name] = `broadcastMsgId-${name}`; + return broadcastMsgNameMap[name]; }, globalBroadcastMsgs: broadcastMsgNameMap }; @@ -533,8 +530,7 @@ const parseBlock = function (sb2block, addBroadcastMsg, getVariableId, extension activeBlock.fields[expectedArg.fieldName].id = getVariableId(providedArg); } else if (expectedArg.fieldName === 'BROADCAST_OPTION') { // add the name in this field to the broadcast msg name map - addBroadcastMsg(providedArg); - const broadcastId = generateBroadcastMsgId(providedArg); + const broadcastId = addBroadcastMsg(providedArg); activeBlock.fields[expectedArg.fieldName].id = broadcastId; } const varType = expectedArg.variableType;