Merge pull request #560 from fsih/updateMonitorsAction

Fire update monitors action for checked monitors

src/engine/blocks.js

@@ -215,7 +215,7 @@ class Blocks {
                 element: e.element,
                 name: e.name,
                 value: e.newValue
-            });
+            }, optRuntime);
             break;
         case 'move':
             this.moveBlock({
@@ -270,13 +270,15 @@ class Blocks {
     /**
      * Block management: change block field values
      * @param {!object} args Blockly change event to be processed
+     * @param {?Runtime} optRuntime Optional runtime to allow changeBlock to change VM state.
      */
-    changeBlock (args) {
+    changeBlock (args, optRuntime) {
         // Validate
         if (['field', 'mutation', 'checkbox'].indexOf(args.element) === -1) return;
         const block = this._blocks[args.id];
         if (typeof block === 'undefined') return;
 
+        let wasMonitored = block.isMonitored;
         switch (args.element) {
         case 'field':
             // Update block value
@@ -288,6 +290,26 @@ class Blocks {
             break;
         case 'checkbox':
             block.isMonitored = args.value;
+            if (optRuntime && wasMonitored && !block.isMonitored) {
+                optRuntime.requestRemoveMonitor(block.id);
+            } else if (optRuntime && !wasMonitored && block.isMonitored) {
+                optRuntime.requestAddMonitor(
+                    // Ensure that value is not undefined, since React requires it
+                    {
+                        // @todo(vm#564) this will collide if multiple sprites use same block
+                        id: block.id,
+                        category: 'data',
+                        // @todo(vm#565) how to handle translation here?
+                        label: block.opcode,
+                        // @todo(vm#565) for numerical values with decimals, some countries use comma
+                        value: '',
+                        x: 0,
+                        // @todo(vm#566) Don't require sending x and y when instantiating a
+                        // monitor. If it's not preset the GUI should decide.
+                        y: 0
+                    }
+                );
+            }
             break;
         }
     }
@@ -357,7 +379,7 @@ class Blocks {
         Object.keys(this._blocks).forEach(blockId => {
             if (this.getBlock(blockId).isMonitored) {
                 // @todo handle specific targets (e.g. apple x position)
-                runtime.toggleScript(blockId);
+                runtime.toggleScript(blockId, {updateMonitor: true});
             }
         });
     }

src/engine/execute.js

@@ -30,7 +30,12 @@ const execute = function (sequencer, thread) {
     const currentBlockId = thread.peekStack();
     const currentStackFrame = thread.peekStackFrame();
 
-    let blockContainer = target.blocks;
+    let blockContainer;
+    if (thread.updateMonitor) {
+        blockContainer = runtime.monitorBlocks;
+    } else {
+        blockContainer = target.blocks;
+    }
     let block = blockContainer.getBlock(currentBlockId);
     if (typeof block === 'undefined') {
         blockContainer = runtime.flyoutBlocks;
@@ -60,6 +65,8 @@ const execute = function (sequencer, thread) {
      * or after a promise resolves.
      * @param {*} resolvedValue Value eventually returned from the primitive.
      */
+     // @todo move this to callback attached to the thread when we have performance
+     // metrics (dd)
     const handleReport = function (resolvedValue) {
         thread.pushReportedValue(resolvedValue);
         if (isHat) {
@@ -85,8 +92,16 @@ const execute = function (sequencer, thread) {
         } else {
             // In a non-hat, report the value visually if necessary if
             // at the top of the thread stack.
-            if (typeof resolvedValue !== 'undefined' && thread.showVisualReport && thread.atStackTop()) {
-                runtime.visualReport(currentBlockId, resolvedValue);
+            if (typeof resolvedValue !== 'undefined' && thread.atStackTop()) {
+                if (thread.showVisualReport) {
+                    runtime.visualReport(currentBlockId, resolvedValue);
+                }
+                if (thread.updateMonitor) {
+                    runtime.requestUpdateMonitor({
+                        id: currentBlockId,
+                        value: String(resolvedValue)
+                    });
+                }
             }
             // Finished any yields.
             thread.status = Thread.STATUS_RUNNING;

src/engine/runtime.js

@@ -93,10 +93,10 @@ class Runtime extends EventEmitter {
         this._scriptGlowsPreviousFrame = [];
 
         /**
-         * Number of threads running during the previous frame
+         * Number of non-monitor threads running during the previous frame.
          * @type {number}
          */
-        this._threadCount = 0;
+        this._nonMonitorThreadCount = 0;
 
         /**
          * Currently known number of clones, used to enforce clone limit.
@@ -111,6 +111,11 @@ class Runtime extends EventEmitter {
          */
         this._refreshTargets = false;
 
+        /**
+         * List of all monitors.
+         */
+        this._monitorState = {};
+
         /**
          * Whether the project is in "turbo mode."
          * @type {Boolean}
@@ -240,6 +245,14 @@ class Runtime extends EventEmitter {
         return 'TARGETS_UPDATE';
     }
 
+    /**
+     * Event name for monitors update.
+     * @const {string}
+     */
+    static get MONITORS_UPDATE () {
+        return 'MONITORS_UPDATE';
+    }
+
     /**
      * How rapidly we try to step threads by default, in ms.
      */
@@ -376,13 +389,22 @@ class Runtime extends EventEmitter {
      * Create a thread and push it to the list of threads.
      * @param {!string} id ID of block that starts the stack.
      * @param {!Target} target Target to run thread on.
-     * @param {?boolean} optShowVisualReport true if the script should show speech bubble for its value
+     * @param {?object} opts optional arguments
+     * @param {?boolean} opts.showVisualReport true if the script should show speech bubble for its value
+     * @param {?boolean} opts.updateMonitor true if the script should update a monitor value
      * @return {!Thread} The newly created thread.
      */
-    _pushThread (id, target, optShowVisualReport) {
+    _pushThread (id, target, opts) {
+        opts = Object.assign({
+            showVisualReport: false,
+            updateMonitor: false
+        }, opts);
+
         const thread = new Thread(id);
         thread.target = target;
-        thread.showVisualReport = optShowVisualReport;
+        thread.showVisualReport = opts.showVisualReport;
+        thread.updateMonitor = opts.updateMonitor;
+
         thread.pushStack(id);
         this.threads.push(thread);
         return thread;
@@ -411,6 +433,8 @@ class Runtime extends EventEmitter {
     _restartThread (thread) {
         const newThread = new Thread(thread.topBlock);
         newThread.target = thread.target;
+        newThread.showVisualReport = thread.showVisualReport;
+        newThread.updateMonitor = thread.updateMonitor;
         newThread.pushStack(thread.topBlock);
         const i = this.threads.indexOf(thread);
         if (i > -1) {
@@ -435,8 +459,14 @@ class Runtime extends EventEmitter {
      * @param {?object} opts optional arguments to toggle script
      * @param {?string} opts.target target ID for target to run script on. If not supplied, uses editing target.
      * @param {?boolean} opts.showVisualReport true if the speech bubble should pop up on the block, false if not.
+     * @param {?boolean} opts.updateMonitor true if the monitor for this block should get updated.
      */
     toggleScript (topBlockId, opts) {
+        opts = Object.assign({
+            target: this._editingTarget,
+            showVisualReport: false,
+            updateMonitor: false
+        }, opts);
         // Remove any existing thread.
         for (let i = 0; i < this.threads.length; i++) {
             if (this.threads[i].topBlock === topBlockId) {
@@ -445,10 +475,7 @@ class Runtime extends EventEmitter {
             }
         }
         // Otherwise add it.
-        this._pushThread(
-            topBlockId,
-            opts && opts.target ? opts.target : this._editingTarget,
-            opts ? opts.showVisualReport : false);
+        this._pushThread(topBlockId, opts.target, opts);
     }
 
     /**
@@ -657,15 +684,39 @@ class Runtime extends EventEmitter {
         this._pushMonitors();
         const doneThreads = this.sequencer.stepThreads();
         this._updateGlows(doneThreads);
-        this._setThreadCount(this.threads.length + doneThreads.length);
+        // Add done threads so that even if a thread finishes within 1 frame, the green
+        // flag will still indicate that a script ran.
+        this._emitProjectRunStatus(
+            this.threads.length + doneThreads.length -
+                this._getMonitorThreadCount([...this.threads, ...doneThreads]));
         if (this.renderer) {
             // @todo: Only render when this.redrawRequested or clones rendered.
             this.renderer.draw();
         }
+
         if (this._refreshTargets) {
             this.emit(Runtime.TARGETS_UPDATE);
             this._refreshTargets = false;
         }
+
+        // @todo(vm#570) only emit if monitors has changed since last time.
+        this.emit(Runtime.MONITORS_UPDATE,
+            Object.keys(this._monitorState).map(key => this._monitorState[key])
+        );
+    }
+
+    /**
+     * Get the number of threads in the given array that are monitor threads (threads
+     * that update monitor values, and don't count as running a script).
+     * @param {!Array.<Thread>} threads The set of threads to look through.
+     * @return {number} The number of monitor threads in threads.
+     */
+    _getMonitorThreadCount (threads) {
+        let count = 0;
+        threads.forEach(thread => {
+            if (thread.updateMonitor) count++;
+        });
+        return count;
     }
 
     /**
@@ -760,16 +811,16 @@ class Runtime extends EventEmitter {
      * Emit run start/stop after each tick. Emits when `this.threads.length` goes
      * between non-zero and zero
      *
-     * @param {number} threadCount The new threadCount
+     * @param {number} nonMonitorThreadCount The new nonMonitorThreadCount
      */
-    _setThreadCount (threadCount) {
-        if (this._threadCount === 0 && threadCount > 0) {
+    _emitProjectRunStatus (nonMonitorThreadCount) {
+        if (this._nonMonitorThreadCount === 0 && nonMonitorThreadCount > 0) {
             this.emit(Runtime.PROJECT_RUN_START);
         }
-        if (this._threadCount > 0 && threadCount === 0) {
+        if (this._nonMonitorThreadCount > 0 && nonMonitorThreadCount === 0) {
             this.emit(Runtime.PROJECT_RUN_STOP);
         }
-        this._threadCount = threadCount;
+        this._nonMonitorThreadCount = nonMonitorThreadCount;
     }
 
     /**
@@ -820,6 +871,35 @@ class Runtime extends EventEmitter {
         this.emit(Runtime.VISUAL_REPORT, {id: blockId, value: String(value)});
     }
 
+    /**
+     * Add a monitor to the state. If the monitor already exists in the state,
+     * overwrites it.
+     * @param {!object} monitor Monitor to add.
+     */
+    requestAddMonitor (monitor) {
+        this._monitorState[monitor.id] = monitor;
+    }
+
+    /**
+     * Update a monitor in the state. Does nothing if the monitor does not already
+     * exist in the state.
+     * @param {!object} monitor Monitor to update.
+     */
+    requestUpdateMonitor (monitor) {
+        if (this._monitorState.hasOwnProperty(monitor.id)) {
+            this._monitorState[monitor.id] = Object.assign({}, this._monitorState[monitor.id], monitor);
+        }
+    }
+
+    /**
+     * Removes a monitor from the state. Does nothing if the monitor already does
+     * not exist in the state.
+     * @param {!object} monitorId ID of the monitor to remove.
+     */
+    requestRemoveMonitor (monitorId) {
+        delete this._monitorState[monitorId];
+    }
+
     /**
      * Get a target by its id.
      * @param {string} targetId Id of target to find.

src/virtual-machine.js

@@ -55,6 +55,9 @@ class VirtualMachine extends EventEmitter {
         this.runtime.on(Runtime.TARGETS_UPDATE, () => {
             this.emitTargetsUpdate();
         });
+        this.runtime.on(Runtime.MONITORS_UPDATE, monitorList => {
+            this.emit(Runtime.MONITORS_UPDATE, monitorList);
+        });
 
         this.blockListener = this.blockListener.bind(this);
         this.flyoutBlockListener = this.flyoutBlockListener.bind(this);