Merge pull request #1515 from kchadha/fix-stack-glow

Fix stack glow

src/engine/blocks.js

@@ -14,8 +14,13 @@ const Variable = require('./variable');
  * and handle updates from Scratch Blocks events.
  */
 
+/**
+ * Create a block container.
+ * @param {boolean} optNoGlow Optional flag to indicate that blocks in this container
+ * should not request glows. This does not affect glows when clicking on a block to execute it.
+ */
 class Blocks {
-    constructor () {
+    constructor (optNoGlow) {
         /**
          * All blocks in the workspace.
          * Keys are block IDs, values are metadata about the block.
@@ -61,6 +66,17 @@ class Blocks {
             _executeCached: {}
         };
 
+        /**
+         * Flag which indicates that blocks in this container should not glow.
+         * Blocks will still glow when clicked on, but this flag is used to control
+         * whether the blocks in this container can request a glow as part of
+         * a running stack. E.g. the flyout block container and the monitor block container
+         * should not be able to request a glow, but blocks containers belonging to
+         * sprites should.
+         * @type {boolean}
+         */
+        this.forceNoGlow = optNoGlow || false;
+
     }
 
     /**
@@ -241,7 +257,7 @@ class Blocks {
     }
 
     duplicate () {
-        const newBlocks = new Blocks();
+        const newBlocks = new Blocks(this.forceNoGlow);
         newBlocks._blocks = Clone.simple(this._blocks);
         newBlocks._scripts = Clone.simple(this._scripts);
         return newBlocks;

src/engine/execute.js

@@ -475,6 +475,14 @@ const execute = function (sequencer, thread) {
 
         // Fields are set during opCached initialization.
 
+        // Blocks should glow when a script is starting,
+        // not after it has finished (see #1404).
+        // Only blocks in blockContainers that don't forceNoGlow
+        // should request a glow.
+        if (!blockContainer.forceNoGlow) {
+            thread.requestScriptGlowInFrame = true;
+        }
+
         // Inputs are set during previous steps in the loop.
 
         let primitiveReportedValue = null;
@@ -530,13 +538,6 @@ const execute = function (sequencer, thread) {
             break;
         } else if (thread.status === Thread.STATUS_RUNNING) {
             if (lastOperation) {
-                if (typeof primitiveReportedValue === 'undefined') {
-                    // No value reported - potentially a command block.
-                    // Edge-activated hats don't request a glow; all
-                    // commands do.
-                    thread.requestScriptGlowInFrame = true;
-                }
-
                 handleReport(primitiveReportedValue, sequencer, thread, opCached, lastOperation);
             } else {
                 // By definition a block that is not last in the list has a

src/engine/runtime.js

@@ -127,14 +127,14 @@ class Runtime extends EventEmitter {
          * These will execute on `_editingTarget.`
          * @type {!Blocks}
          */
-        this.flyoutBlocks = new Blocks();
+        this.flyoutBlocks = new Blocks(true /* force no glow */);
 
         /**
          * Storage container for monitor blocks.
          * These will execute on a target maybe
          * @type {!Blocks}
          */
-        this.monitorBlocks = new Blocks();
+        this.monitorBlocks = new Blocks(true /* force no glow */);
 
         /**
          * Currently known editing target for the VM.