Merge pull request #1943 from ktbee/clock-timer-compat-fix

Start executing hats before other threads change values

src/engine/block-utility.js

@@ -205,9 +205,18 @@ class BlockUtility {
      * @return {Array.<Thread>} List of threads started by this function.
      */
     startHats (requestedHat, optMatchFields, optTarget) {
-        return (
+        // Store thread and sequencer to ensure we can return to the calling block's context.
-            this.sequencer.runtime.startHats(requestedHat, optMatchFields, optTarget)
+        // startHats may execute further blocks and dirty the BlockUtility's execution context
-        );
+        // and confuse the calling block when we return to it.
+        const callerThread = this.thread;
+        const callerSequencer = this.sequencer;
+        const result = this.sequencer.runtime.startHats(requestedHat, optMatchFields, optTarget);
+
+        // Restore thread and sequencer to prior values before we return to the calling block.
+        this.thread = callerThread;
+        this.sequencer = callerSequencer;
+
+        return result;
     }
 
     /**

src/engine/runtime.js

@@ -7,6 +7,7 @@ const Blocks = require('./blocks');
 const BlockType = require('../extension-support/block-type');
 const Profiler = require('./profiler');
 const Sequencer = require('./sequencer');
+const execute = require('./execute.js');
 const ScratchBlocksConstants = require('./scratch-blocks-constants');
 const TargetType = require('../extension-support/target-type');
 const Thread = require('./thread');
@@ -316,7 +317,7 @@ class Runtime extends EventEmitter {
         // I/O related data.
         /** @type {Object.<string, Object>} */
         this.ioDevices = {
-            clock: new Clock(),
+            clock: new Clock(this),
             cloud: new Cloud(this),
             keyboard: new Keyboard(this),
             mouse: new Mouse(this),
@@ -1631,6 +1632,12 @@ class Runtime extends EventEmitter {
             // Start the thread with this top block.
             newThreads.push(instance._pushThread(topBlockId, target));
         }, optTarget);
+        // For compatibility with Scratch 2, edge triggered hats need to be processed before
+        // threads are stepped. See ScratchRuntime.as for original implementation
+        newThreads.forEach(thread => {
+            execute(this.sequencer, thread);
+            thread.goToNextBlock();
+        });
         return newThreads;
     }
 

src/engine/sequencer.js

@@ -138,7 +138,6 @@ class Sequencer {
                     activeThread.status === Thread.STATUS_DONE) {
                     // Finished with this thread.
                     stoppedThread = true;
-                    this.runtime.updateCurrentMSecs();
                 }
             }
             // We successfully ticked once. Prevents running STATUS_YIELD_TICK

src/io/clock.js

@@ -2,7 +2,7 @@ const Timer = require('../util/timer');
 
 class Clock {
     constructor (runtime) {
-        this._projectTimer = new Timer();
+        this._projectTimer = new Timer({now: () => runtime.currentMSecs});
         this._projectTimer.start();
         this._pausedTime = null;
         this._paused = false;

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

@@ -7,7 +7,7 @@ const Thread = require('../../src/engine/thread');
 const Runtime = require('../../src/engine/runtime');
 const execute = require('../../src/engine/execute.js');
 
-const projectUri = path.resolve(__dirname, '../fixtures/loudness-hat-block.sb2');
+const projectUri = path.resolve(__dirname, '../fixtures/timer-greater-than-hat.sb2');
 const project = readFileToBuffer(projectUri);
 
 const checkIsHatThread = (t, vm, hatThread) => {
@@ -24,8 +24,8 @@ const checkIsStackClickThread = (t, vm, stackClickThread) => {
 };
 
 /**
- * loudness-hat-block.sb2 contains a single stack
+ * timer-greater-than-hat.sb2 contains a single stack
- *     when loudness > 10
+ *     when timer > -1
  *         change color effect by 25
  * The intention is to make sure that the hat block condition is evaluated
  * on each frame.
@@ -111,7 +111,7 @@ test('edge activated hat thread not added twice', t => {
  */
 test('edge activated hat should trigger for both sprites when sprite is duplicated', t => {
 
-    // Project that is similar to loudness-hat-block.sb2, but has code on the sprite so that
+    // Project that is similar to timer-greater-than-hat.sb2, but has code on the sprite so that
     // the sprite can be duplicated
     const projectWithSpriteUri = path.resolve(__dirname, '../fixtures/edge-triggered-hat.sb3');
     const projectWithSprite = readFileToBuffer(projectWithSpriteUri);
@@ -134,9 +134,6 @@ test('edge activated hat should trigger for both sprites when sprite is duplicat
             t.equal(vm.runtime.threads.length, 1);
             checkIsHatThread(t, vm, vm.runtime.threads[0]);
             t.assert(vm.runtime.threads[0].status === Thread.STATUS_RUNNING);
-            // Run execute on the thread to populate the runtime's
-            // _edgeActivatedHatValues object
-            execute(vm.runtime.sequencer, vm.runtime.threads[0]);
             let numTargetEdgeHats = vm.runtime.targets.reduce((val, target) =>
                 val + Object.keys(target._edgeActivatedHatValues).length, 0);
             t.equal(numTargetEdgeHats, 1);
@@ -145,7 +142,6 @@ test('edge activated hat should trigger for both sprites when sprite is duplicat
                 vm.runtime._step();
                 // Check that the runtime's _edgeActivatedHatValues object has two separate keys
                 // after execute is run on each thread
-                vm.runtime.threads.forEach(thread => execute(vm.runtime.sequencer, thread));
                 numTargetEdgeHats = vm.runtime.targets.reduce((val, target) =>
                     val + Object.keys(target._edgeActivatedHatValues).length, 0);
                 t.equal(numTargetEdgeHats, 2);

test/unit/io_clock.js

@@ -23,12 +23,15 @@ test('cycle', t => {
     setTimeout(() => {
         c.resetProjectTimer();
         setTimeout(() => {
-            t.ok(c.projectTimer() > 0);
+            // The timer shouldn't advance until all threads have been stepped
+            t.ok(c.projectTimer() === 0);
             c.pause();
-            t.ok(c.projectTimer() > 0);
+            t.ok(c.projectTimer() === 0);
             c.resume();
-            t.ok(c.projectTimer() > 0);
+            t.ok(c.projectTimer() === 0);
             t.end();
         }, 100);
     }, 100);
+    rt._step();
+    t.ok(c.projectTimer() > 0);
 });