Merge pull request #637 from fsih/timerBlock

Fix monitor not updating every frame

src/engine/blocks.js

@@ -393,7 +393,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, {updateMonitor: true});
+                runtime.addMonitorScript(blockId);
             }
         });
     }

src/engine/runtime.js

@@ -467,14 +467,12 @@ class Runtime extends EventEmitter {
      * @param {!string} topBlockId ID of block that starts the script.
      * @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.updateMonitor true if the monitor for this block should get updated.
      * @param {?boolean} opts.stackClick true if the user activated the stack by clicking, false if not. This
      *     determines whether we show a visual report when turning on the script.
      */
     toggleScript (topBlockId, opts) {
         opts = Object.assign({
             target: this._editingTarget,
-            updateMonitor: false,
             stackClick: false
         }, opts);
         // Remove any existing thread.
@@ -497,6 +495,24 @@ class Runtime extends EventEmitter {
         this._pushThread(topBlockId, opts.target, opts);
     }
 
+    /**
+     * Enqueue a script that when finished will update the monitor for the block.
+     * @param {!string} topBlockId ID of block that starts the script.
+     * @param {?string} optTarget target ID for target to run script on. If not supplied, uses editing target.
+     */
+    addMonitorScript (topBlockId, optTarget) {
+        if (!optTarget) optTarget = this._editingTarget;
+        for (let i = 0; i < this.threads.length; i++) {
+            // Don't re-add the script if it's already running
+            if (this.threads[i].topBlock === topBlockId && this.threads[i].status !== Thread.STATUS_DONE &&
+                    this.threads[i].updateMonitor) {
+                return;
+            }
+        }
+        // Otherwise add it.
+        this._pushThread(topBlockId, optTarget, {updateMonitor: true});
+    }
+
     /**
      * Run a function `f` for all scripts in a workspace.
      * `f` will be called with two parameters:

test/integration/monitor-threads-run-every-frame.js

@@ -0,0 +1,117 @@
+const path = require('path');
+const test = require('tap').test;
+const makeTestStorage = require('../fixtures/make-test-storage');
+const extract = require('../fixtures/extract');
+const VirtualMachine = require('../../src/index');
+const Thread = require('../../src/engine/thread');
+const Runtime = require('../../src/engine/runtime');
+
+const projectUri = path.resolve(__dirname, '../fixtures/default.sb2');
+const project = extract(projectUri);
+
+const checkMonitorThreadPresent = (t, threads) => {
+    t.equal(threads.length, 1);
+    const monitorThread = threads[0];
+    t.equal(monitorThread.stackClick, false);
+    t.equal(monitorThread.updateMonitor, true);
+    t.equal(monitorThread.topBlock.toString(), 'sensing_timer');
+};
+
+/**
+ * Creates a monitor and then checks if it gets run every frame.
+ */
+/* TODO: when loadProject loads monitors, we can create a project with a monitor and will
+ * not have to do the create monitor step manually.
+ */
+test('monitor thread runs every frame', t => {
+    const vm = new VirtualMachine();
+    vm.attachStorage(makeTestStorage());
+
+    // Start VM, load project, and run
+    t.doesNotThrow(() => {
+        // Note: don't run vm.start(), we handle calling _step() manually in this test
+        vm.runtime.currentStepTime = Runtime.THREAD_STEP_INTERVAL;
+
+        // Manually populate the monitor block and set its isMonitored to true.
+        vm.runtime.monitorBlocks.createBlock({
+            id: 'sensing_timer',
+            opcode: 'sensing_timer',
+            inputs: {},
+            fields: {},
+            next: null,
+            topLevel: true,
+            parent: null,
+            shadow: false,
+            isMonitored: true,
+            x: '0',
+            y: '0'
+        });
+
+        vm.clear();
+        vm.setCompatibilityMode(false);
+        vm.setTurboMode(false);
+
+        vm.loadProject(project).then(() => {
+            t.equal(vm.runtime.threads.length, 0);
+
+            vm.runtime._step();
+            checkMonitorThreadPresent(t, vm.runtime.threads);
+            t.assert(vm.runtime.threads[0].status === Thread.STATUS_DONE);
+
+            // Check that both are added again when another step is taken
+            vm.runtime._step();
+            checkMonitorThreadPresent(t, vm.runtime.threads);
+            t.assert(vm.runtime.threads[0].status === Thread.STATUS_DONE);
+            t.end();
+        });
+    });
+});
+
+/**
+ * If the monitor doesn't finish evaluating within one frame, it shouldn't be added again
+ * on the next frame. (We skip execution by setting the step time to 0)
+ */
+test('monitor thread not added twice', t => {
+    const vm = new VirtualMachine();
+    vm.attachStorage(makeTestStorage());
+
+    // Start VM, load project, and run
+    t.doesNotThrow(() => {
+        // Note: don't run vm.start(), we handle calling _step() manually in this test
+        vm.runtime.currentStepTime = 0;
+
+        // Manually populate the monitor block and set its isMonitored to true.
+        vm.runtime.monitorBlocks.createBlock({
+            id: 'sensing_timer',
+            opcode: 'sensing_timer',
+            inputs: {},
+            fields: {},
+            next: null,
+            topLevel: true,
+            parent: null,
+            shadow: false,
+            isMonitored: true,
+            x: '0',
+            y: '0'
+        });
+
+        vm.clear();
+        vm.setCompatibilityMode(false);
+        vm.setTurboMode(false);
+
+        vm.loadProject(project).then(() => {
+            t.equal(vm.runtime.threads.length, 0);
+
+            vm.runtime._step();
+            checkMonitorThreadPresent(t, vm.runtime.threads);
+            t.assert(vm.runtime.threads[0].status === Thread.STATUS_RUNNING);
+            const prevThread = vm.runtime.threads[0];
+
+            // Check that both are added again when another step is taken
+            vm.runtime._step();
+            checkMonitorThreadPresent(t, vm.runtime.threads);
+            t.equal(vm.runtime.threads[0], prevThread);
+            t.end();
+        });
+    });
+});