Bugfix for scratch-gui issue #994, where executing a broadcast block from the flyout was creating a conflicting variable, causing a fatal error.

src/blocks/scratch3_event.js

@@ -56,17 +56,20 @@ class Scratch3EventBlocks {
     }
 
     broadcast (args, util) {
-        const broadcastVar = util.runtime.getTargetForStage().lookupOrCreateBroadcastMsg(
+        const broadcastVar = util.runtime.getTargetForStage().lookupBroadcastMsg(
             args.BROADCAST_OPTION.id, args.BROADCAST_OPTION.name);
+        if (broadcastVar) {
             const broadcastOption = broadcastVar.name;
             util.startHats('event_whenbroadcastreceived', {
                 BROADCAST_OPTION: broadcastOption
             });
         }
+    }
 
     broadcastAndWait (args, util) {
-        const broadcastVar = util.runtime.getTargetForStage().lookupOrCreateBroadcastMsg(
+        const broadcastVar = util.runtime.getTargetForStage().lookupBroadcastMsg(
             args.BROADCAST_OPTION.id, args.BROADCAST_OPTION.name);
+        if (broadcastVar) {
             const broadcastOption = broadcastVar.name;
             // Have we run before, starting threads?
             if (!util.stackFrame.startedThreads) {
@@ -88,6 +91,7 @@ class Scratch3EventBlocks {
                 util.yield();
             }
         }
+    }
 }
 
 module.exports = Scratch3EventBlocks;

src/engine/target.js

@@ -4,6 +4,7 @@ const Blocks = require('./blocks');
 const Variable = require('../engine/variable');
 const uid = require('../util/uid');
 const {Map} = require('immutable');
+const log = require('../util/log');
 
 /**
  * @fileoverview
@@ -93,19 +94,25 @@ class Target extends EventEmitter {
     }
 
     /**
-     * Look up a broadcast message object, and create it if one doesn't exist.
+     * Look up a broadcast message object with the given id and return it
+     * if it exists.
      * @param {string} id Id of the variable.
      * @param {string} name Name of the variable.
      * @return {!Variable} Variable object.
      */
-    lookupOrCreateBroadcastMsg (id, name) {
+    lookupBroadcastMsg (id, name) {
         const broadcastMsg = this.lookupVariableById(id);
-        if (broadcastMsg) return broadcastMsg;
+        if (broadcastMsg) {
-        // No variable with this name exists - create it locally.
+            if (broadcastMsg.name !== name) {
-        const newBroadcastMsg = new Variable(id, name,
+                log.error(`Found broadcast message with id: ${id}, but` +
-            Variable.BROADCAST_MESSAGE_TYPE, false);
+                    `its name, ${broadcastMsg.name} did not match expected name ${name}.`);
-        this.variables[id] = newBroadcastMsg;
+            }
-        return newBroadcastMsg;
+            if (broadcastMsg.type !== Variable.BROADCAST_MESSAGE_TYPE) {
+                log.error(`Found variable with id: ${id}, but its type ${broadcastMsg.type}` +
+                    `did not match expected type ${Variable.BROADCAST_MESSAGE_TYPE}`);
+            }
+            return broadcastMsg;
+        }
     }
 
     /**

test/unit/blocks_event.js

@@ -6,6 +6,7 @@ const Event = require('../../src/blocks/scratch3_event');
 const Runtime = require('../../src/engine/runtime');
 const Target = require('../../src/engine/target');
 const Thread = require('../../src/engine/thread');
+const Variable = require('../../src/engine/variable');
 
 test('#760 - broadcastAndWait', t => {
     const broadcastAndWaitBlock = {
@@ -52,6 +53,7 @@ test('#760 - broadcastAndWait', t => {
     b.createBlock(receiveMessageBlock);
     const tgt = new Target(rt, b);
     tgt.isStage = true;
+    tgt.createVariable('testBroadcastID', 'message', Variable.BROADCAST_MESSAGE_TYPE);
     rt.targets.push(tgt);
 
     let th = rt._pushThread('broadcastAndWaitBlock', t);
@@ -67,12 +69,12 @@ test('#760 - broadcastAndWait', t => {
     // yields when some thread is active
     t.strictEqual(th.status, Thread.STATUS_YIELD);
     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);
 
     // restarts done threads that are in runtime threads

test/unit/engine_target.js

@@ -174,20 +174,7 @@ test('lookupOrCreateList returns list if one with given id exists', t => {
     t.end();
 });
 
-test('lookupOrCreateBroadcastMsg creates a var if one does not exist', t => {
+test('lookupBroadcastMsg returns the var with given id if exists', 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;
 
@@ -195,7 +182,7 @@ test('lookupOrCreateBroadcastMsg returns the var with given id if exists', t =>
     target.createVariable('foo', 'bar', Variable.BROADCAST_MESSAGE_TYPE);
     t.equal(Object.keys(variables).length, 1);
 
-    const broadcastMsg = target.lookupOrCreateBroadcastMsg('foo', 'bar');
+    const broadcastMsg = target.lookupBroadcastMsg('foo', 'bar');
     t.equal(Object.keys(variables).length, 1);
     t.equal(broadcastMsg.id, 'foo');
     t.equal(broadcastMsg.name, 'bar');