From 62bf790d8f309b16cd7a8c3ab7fbe996151a25b2 Mon Sep 17 00:00:00 2001
From: Karishma Chadha <kchadha@scratch.mit.edu>
Date: Wed, 22 Aug 2018 14:44:06 -0400
Subject: [PATCH] Fix stack glows so that stacks glow as soon as the first
 block in the stack starts running as opposed to after the first block has
 finished. Make sure that monitored reporters in the flyout are not glowing.

---
 src/engine/blocks.js  | 20 ++++++++++++++++++--
 src/engine/execute.js | 15 ++++++++-------
 src/engine/runtime.js |  4 ++--
 3 files changed, 28 insertions(+), 11 deletions(-)

diff --git a/src/engine/blocks.js b/src/engine/blocks.js
index d40c32795..61493f040 100644
--- a/src/engine/blocks.js
+++ b/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;
diff --git a/src/engine/execute.js b/src/engine/execute.js
index a6958aabd..f0ed1f3dd 100644
--- a/src/engine/execute.js
+++ b/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
diff --git a/src/engine/runtime.js b/src/engine/runtime.js
index 5847f6ebc..c12809c64 100644
--- a/src/engine/runtime.js
+++ b/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.