Fix loading broadcast messages with special characters for projects that were converted from 2.0.

src/engine/blocks.js

@@ -823,11 +823,12 @@ class Blocks {
      * @param {Array<object>} 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;

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 => {

src/util/variable-util.js

@@ -15,12 +15,13 @@ class VariableUtil {
      * in the project.
      * @param {Array.<Target>} 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, {});
     }
 

test/fixtures/events.json

@@ -97,6 +97,12 @@
             "outerHTML": "<block type='data_variable' id='a block' x='0' y='0'><field name='VARIABLE' id='mock var id' variabletype=''>a mock variable</field></block>"
         }
     },
+    "mockBroadcastBlock": {
+        "name": "block",
+        "xml": {
+            "outerHTML": "<block type='event_broadcast' id='a broadcast block' x='0' y='0'><value name='BROADCAST_INPUT' id='mock broadcast input'><shadow id='boadcast shadow' type='event_broadcast_menu'><field name='BROADCAST_OPTION' id='mock broadcast message id' variabletype='broadcast-msg'>my message</field></shadow></value></block>"
+        }
+    },
     "mockListBlock": {
         "name": "block",
         "xml": {

test/integration/broadcast_special_chars_sb2.js

@@ -10,15 +10,13 @@ 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 variable names', t => {
+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);
-        // 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.targets.length, 2);
@@ -32,7 +30,7 @@ test('importing sb2 project with special chars in variable names', t => {
         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 variable ID does not have any unsafe characters
+        // 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
@@ -46,17 +44,16 @@ test('importing sb2 project with special chars in variable names', t => {
         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 variable ID does not have any unsafe characters
+        // 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
+        // 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 this variable, and verify they have the correct ID
-        // // There should be one
+        // 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]);

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);
+        });
+    });
+});

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();
+});