diff --git a/src/engine/blocks.js b/src/engine/blocks.js index 05de4e7e65d..e22122edd0d 100644 --- a/src/engine/blocks.js +++ b/src/engine/blocks.js @@ -823,11 +823,12 @@ class Blocks { * @param {Array} optBlocks Optional list of blocks to constrain the search to. * This is useful for getting variable/list references for a stack of blocks instead * of all blocks on the workspace + * @param {?boolean} optIncludeBroadcast Optional whether to include broadcast fields. * @return {object} A map of variable ID to a list of all variable references * for that ID. A variable reference contains the field referencing that variable * and also the type of the variable being referenced. */ - getAllVariableAndListReferences (optBlocks) { + getAllVariableAndListReferences (optBlocks, optIncludeBroadcast) { const blocks = optBlocks ? optBlocks : this._blocks; const allReferences = Object.create(null); for (const blockId in blocks) { @@ -839,6 +840,9 @@ class Blocks { } else if (blocks[blockId].fields.LIST) { varOrListField = blocks[blockId].fields.LIST; varType = Variable.LIST_TYPE; + } else if (optIncludeBroadcast && blocks[blockId].fields.BROADCAST_OPTION) { + varOrListField = blocks[blockId].fields.BROADCAST_OPTION; + varType = Variable.BROADCAST_MESSAGE_TYPE; } if (varOrListField) { const currVarId = varOrListField.id; diff --git a/src/serialization/sb2.js b/src/serialization/sb2.js index e606f5a69f0..f8d1b9e6511 100644 --- a/src/serialization/sb2.js +++ b/src/serialization/sb2.js @@ -227,7 +227,7 @@ const globalBroadcastMsgStateGenerator = (function () { if (name === '') { name = emptyStringName; } - broadcastMsgNameMap[name] = `broadcastMsgId-${name}`; + broadcastMsgNameMap[name] = `broadcastMsgId-${StringUtil.replaceUnsafeChars(name)}`; allBroadcastFields.push(field); return broadcastMsgNameMap[name]; }, diff --git a/src/serialization/sb3.js b/src/serialization/sb3.js index ccc5dfb094a..5b4d5a76e1f 100644 --- a/src/serialization/sb3.js +++ b/src/serialization/sb3.js @@ -1141,7 +1141,7 @@ const deserializeMonitor = function (monitorData, runtime, targets, extensions) // This is to fix up projects imported from 2.0 where xml-unsafe names // were getting added to the variable ids. const replaceUnsafeCharsInVariableIds = function (targets) { - const allVarRefs = VariableUtil.getAllVarRefsForTargets(targets); + const allVarRefs = VariableUtil.getAllVarRefsForTargets(targets, true); // Re-id the variables in the actual targets targets.forEach(t => { Object.keys(t.variables).forEach(id => { diff --git a/src/util/variable-util.js b/src/util/variable-util.js index c1dab8f8e31..3e34a732775 100644 --- a/src/util/variable-util.js +++ b/src/util/variable-util.js @@ -15,12 +15,13 @@ class VariableUtil { * in the project. * @param {Array.} targets The list of targets to get the variable * and list references from. + * @param {boolean} shouldIncludeBroadcast Whether to include broadcast message fields. * @return {object} An object with variable ids as the keys and a list of block fields referencing * the variable. */ - static getAllVarRefsForTargets (targets) { + static getAllVarRefsForTargets (targets, shouldIncludeBroadcast) { return targets - .map(t => t.blocks.getAllVariableAndListReferences()) + .map(t => t.blocks.getAllVariableAndListReferences(null, shouldIncludeBroadcast)) .reduce(VariableUtil._mergeVarRefObjects, {}); } diff --git a/test/fixtures/broadcast_special_chars.sb2 b/test/fixtures/broadcast_special_chars.sb2 new file mode 100644 index 00000000000..afaf4ba25b0 Binary files /dev/null and b/test/fixtures/broadcast_special_chars.sb2 differ diff --git a/test/fixtures/broadcast_special_chars.sb3 b/test/fixtures/broadcast_special_chars.sb3 new file mode 100644 index 00000000000..9a840b0ef3d Binary files /dev/null and b/test/fixtures/broadcast_special_chars.sb3 differ diff --git a/test/fixtures/events.json b/test/fixtures/events.json index dbddd277751..a6b42cf8186 100644 --- a/test/fixtures/events.json +++ b/test/fixtures/events.json @@ -97,6 +97,12 @@ "outerHTML": "a mock variable" } }, + "mockBroadcastBlock": { + "name": "block", + "xml": { + "outerHTML": "my message" + } + }, "mockListBlock": { "name": "block", "xml": { diff --git a/test/integration/broadcast_special_chars_sb2.js b/test/integration/broadcast_special_chars_sb2.js new file mode 100644 index 00000000000..eec5d6b5e08 --- /dev/null +++ b/test/integration/broadcast_special_chars_sb2.js @@ -0,0 +1,83 @@ +const path = require('path'); +const test = require('tap').test; +const makeTestStorage = require('../fixtures/make-test-storage'); +const readFileToBuffer = require('../fixtures/readProjectFile').readFileToBuffer; +const VirtualMachine = require('../../src/index'); +const Variable = require('../../src/engine/variable'); +const StringUtil = require('../../src/util/string-util'); +const VariableUtil = require('../../src/util/variable-util'); + +const projectUri = path.resolve(__dirname, '../fixtures/broadcast_special_chars.sb2'); +const project = readFileToBuffer(projectUri); + +test('importing sb2 project with special chars in message names', t => { + const vm = new VirtualMachine(); + vm.attachStorage(makeTestStorage()); + + // Evaluate playground data and exit + vm.on('playgroundData', e => { + const threads = JSON.parse(e.threads); + t.equal(threads.length, 0); + + t.equal(vm.runtime.targets.length, 2); + + const stage = vm.runtime.targets[0]; + const cat = vm.runtime.targets[1]; + + const allBroadcastFields = VariableUtil.getAllVarRefsForTargets(vm.runtime.targets, true); + + const abMessageId = Object.keys(stage.variables).filter(k => stage.variables[k].name === 'a&b')[0]; + const abMessage = stage.variables[abMessageId]; + // Check for unsafe characters, replaceUnsafeChars should just result in the original string + // (e.g. there was nothing to replace) + // Check that the message ID does not have any unsafe characters + t.equal(StringUtil.replaceUnsafeChars(abMessageId), abMessageId); + + // Check that the message still has the correct info + t.equal(StringUtil.replaceUnsafeChars(abMessage.id), abMessage.id); + t.equal(abMessage.id, abMessageId); + t.equal(abMessage.type, Variable.BROADCAST_MESSAGE_TYPE); + t.equal(abMessage.value, 'a&b'); + + + const ltPerfectMessageId = Object.keys(stage.variables).filter(k => stage.variables[k].name === '< perfect')[0]; + const ltPerfectMessage = stage.variables[ltPerfectMessageId]; + // Check for unsafe characters, replaceUnsafeChars should just result in the original string + // (e.g. there was nothing to replace) + // Check that the message ID does not have any unsafe characters + t.equal(StringUtil.replaceUnsafeChars(ltPerfectMessageId), ltPerfectMessageId); + + // Check that the message still has the correct info + t.equal(StringUtil.replaceUnsafeChars(ltPerfectMessage.id), ltPerfectMessage.id); + t.equal(ltPerfectMessage.id, ltPerfectMessageId); + t.equal(ltPerfectMessage.type, Variable.BROADCAST_MESSAGE_TYPE); + t.equal(ltPerfectMessage.value, '< perfect'); + + // Find all the references for these messages, and verify they have the correct ID + t.equal(allBroadcastFields[ltPerfectMessageId].length, 1); + t.equal(allBroadcastFields[abMessageId].length, 1); + const catBlocks = Object.keys(cat.blocks._blocks).map(blockId => cat.blocks._blocks[blockId]); + const catMessageBlocks = catBlocks.filter(block => block.fields.hasOwnProperty('BROADCAST_OPTION')); + t.equal(catMessageBlocks.length, 2); + t.equal(catMessageBlocks[0].fields.BROADCAST_OPTION.id, ltPerfectMessageId); + t.equal(catMessageBlocks[1].fields.BROADCAST_OPTION.id, abMessageId); + + t.end(); + process.nextTick(process.exit); + }); + + // Start VM, load project, and run + t.doesNotThrow(() => { + vm.start(); + vm.clear(); + vm.setCompatibilityMode(false); + vm.setTurboMode(false); + vm.loadProject(project).then(() => { + vm.greenFlag(); + setTimeout(() => { + vm.getPlaygroundData(); + vm.stopAll(); + }, 100); + }); + }); +}); diff --git a/test/integration/broadcast_special_chars_sb3.js b/test/integration/broadcast_special_chars_sb3.js new file mode 100644 index 00000000000..62e0770c731 --- /dev/null +++ b/test/integration/broadcast_special_chars_sb3.js @@ -0,0 +1,83 @@ +const path = require('path'); +const test = require('tap').test; +const makeTestStorage = require('../fixtures/make-test-storage'); +const readFileToBuffer = require('../fixtures/readProjectFile').readFileToBuffer; +const VirtualMachine = require('../../src/index'); +const Variable = require('../../src/engine/variable'); +const StringUtil = require('../../src/util/string-util'); +const VariableUtil = require('../../src/util/variable-util'); + +const projectUri = path.resolve(__dirname, '../fixtures/broadcast_special_chars.sb3'); +const project = readFileToBuffer(projectUri); + +test('importing sb3 project with special chars in message names', t => { + const vm = new VirtualMachine(); + vm.attachStorage(makeTestStorage()); + + // Evaluate playground data and exit + vm.on('playgroundData', e => { + const threads = JSON.parse(e.threads); + t.equal(threads.length, 0); + + t.equal(vm.runtime.targets.length, 2); + + const stage = vm.runtime.targets[0]; + const cat = vm.runtime.targets[1]; + + const allBroadcastFields = VariableUtil.getAllVarRefsForTargets(vm.runtime.targets, true); + + const abMessageId = Object.keys(stage.variables).filter(k => stage.variables[k].name === 'a&b')[0]; + const abMessage = stage.variables[abMessageId]; + // Check for unsafe characters, replaceUnsafeChars should just result in the original string + // (e.g. there was nothing to replace) + // Check that the message ID does not have any unsafe characters + t.equal(StringUtil.replaceUnsafeChars(abMessageId), abMessageId); + + // Check that the message still has the correct info + t.equal(StringUtil.replaceUnsafeChars(abMessage.id), abMessage.id); + t.equal(abMessage.id, abMessageId); + t.equal(abMessage.type, Variable.BROADCAST_MESSAGE_TYPE); + t.equal(abMessage.value, 'a&b'); + + + const ltPerfectMessageId = Object.keys(stage.variables).filter(k => stage.variables[k].name === '< perfect')[0]; + const ltPerfectMessage = stage.variables[ltPerfectMessageId]; + // Check for unsafe characters, replaceUnsafeChars should just result in the original string + // (e.g. there was nothing to replace) + // Check that the message ID does not have any unsafe characters + t.equal(StringUtil.replaceUnsafeChars(ltPerfectMessageId), ltPerfectMessageId); + + // Check that the message still has the correct info + t.equal(StringUtil.replaceUnsafeChars(ltPerfectMessage.id), ltPerfectMessage.id); + t.equal(ltPerfectMessage.id, ltPerfectMessageId); + t.equal(ltPerfectMessage.type, Variable.BROADCAST_MESSAGE_TYPE); + t.equal(ltPerfectMessage.value, '< perfect'); + + // Find all the references for these messages, and verify they have the correct ID + t.equal(allBroadcastFields[ltPerfectMessageId].length, 1); + t.equal(allBroadcastFields[abMessageId].length, 1); + const catBlocks = Object.keys(cat.blocks._blocks).map(blockId => cat.blocks._blocks[blockId]); + const catMessageBlocks = catBlocks.filter(block => block.fields.hasOwnProperty('BROADCAST_OPTION')); + t.equal(catMessageBlocks.length, 2); + t.equal(catMessageBlocks[0].fields.BROADCAST_OPTION.id, ltPerfectMessageId); + t.equal(catMessageBlocks[1].fields.BROADCAST_OPTION.id, abMessageId); + + t.end(); + process.nextTick(process.exit); + }); + + // Start VM, load project, and run + t.doesNotThrow(() => { + vm.start(); + vm.clear(); + vm.setCompatibilityMode(false); + vm.setTurboMode(false); + vm.loadProject(project).then(() => { + vm.greenFlag(); + setTimeout(() => { + vm.getPlaygroundData(); + vm.stopAll(); + }, 100); + }); + }); +}); diff --git a/test/integration/variable_special_chars_sb3.js b/test/integration/variable_special_chars_sb3.js index 1c506406d01..b599415afe3 100644 --- a/test/integration/variable_special_chars_sb3.js +++ b/test/integration/variable_special_chars_sb3.js @@ -10,7 +10,7 @@ const VariableUtil = require('../../src/util/variable-util'); const projectUri = path.resolve(__dirname, '../fixtures/variable_characters.sb3'); const project = readFileToBuffer(projectUri); -test('importing sb2 project with special chars in variable names', t => { +test('importing sb3 project with special chars in variable names', t => { const vm = new VirtualMachine(); vm.attachStorage(makeTestStorage()); diff --git a/test/unit/engine_blocks.js b/test/unit/engine_blocks.js index 3b3ecedc181..c25d5b42ba8 100644 --- a/test/unit/engine_blocks.js +++ b/test/unit/engine_blocks.js @@ -809,3 +809,35 @@ test('getAllVariableAndListReferences returns references when variable blocks ex t.end(); }); + +test('getAllVariableAndListReferences does not return broadcast blocks if the flag is left out', t => { + const b = new Blocks(new Runtime()); + b.createBlock(adapter(events.mockBroadcastBlock)[0]); + b.createBlock(adapter(events.mockBroadcastBlock)[1]); + + t.equal(Object.keys(b.getAllVariableAndListReferences()).length, 0); + t.end(); +}); + +test('getAllVariableAndListReferences returns broadcast when we tell it to', t => { + const b = new Blocks(new Runtime()); + + b.createBlock(adapter(events.mockVariableBlock)[0]); + // Make the broadcast block and its shadow (which includes the actual broadcast field). + b.createBlock(adapter(events.mockBroadcastBlock)[0]); + b.createBlock(adapter(events.mockBroadcastBlock)[1]); + + const varListRefs = b.getAllVariableAndListReferences(null, true); + + t.equal(Object.keys(varListRefs).length, 2); + t.equal(Array.isArray(varListRefs['mock var id']), true); + t.equal(varListRefs['mock var id'].length, 1); + t.equal(varListRefs['mock var id'][0].type, Variable.SCALAR_TYPE); + t.equal(varListRefs['mock var id'][0].referencingField.value, 'a mock variable'); + t.equal(Array.isArray(varListRefs['mock broadcast message id']), true); + t.equal(varListRefs['mock broadcast message id'].length, 1); + t.equal(varListRefs['mock broadcast message id'][0].type, Variable.BROADCAST_MESSAGE_TYPE); + t.equal(varListRefs['mock broadcast message id'][0].referencingField.value, 'my message'); + + t.end(); +});