Merge pull request #1834 from kchadha/edge-hat-duplicate-sprite

Fix issue where edge-activated hats only run on one sprite after duplicating the sprite

src/engine/execute.js

@@ -63,11 +63,13 @@ const handleReport = function (resolvedValue, sequencer, thread, blockCached, la
             // true and used to be false, or the stack was activated explicitly
             // via stack click
             if (!thread.stackClick) {
-                const oldEdgeValue = sequencer.runtime.updateEdgeActivatedValue(
+                const hasOldEdgeValue = thread.target.hasEdgeActivatedValue(currentBlockId);
+                const oldEdgeValue = thread.target.updateEdgeActivatedValue(
                     currentBlockId,
                     resolvedValue
                 );
-                const edgeWasActivated = !oldEdgeValue && resolvedValue;
+
+                const edgeWasActivated = hasOldEdgeValue ? (!oldEdgeValue && resolvedValue) : resolvedValue;
                 if (!edgeWasActivated) {
                     sequencer.retireThread(thread);
                 }

src/engine/runtime.js

@@ -221,13 +221,6 @@ class Runtime extends EventEmitter {
          */
         this._hats = {};
 
-        /**
-         * Currently known values for edge-activated hats.
-         * Keys are block ID for the hat; values are the currently known values.
-         * @type {Object.<string, *>}
-         */
-        this._edgeActivatedHatValues = {};
-
         /**
          * A list of script block IDs that were glowing during the previous frame.
          * @type {!Array.<!string>}
@@ -1230,24 +1223,6 @@ class Runtime extends EventEmitter {
             this._hats[opcode].edgeActivated;
     }
 
-    /**
-     * Update an edge-activated hat block value.
-     * @param {!string} blockId ID of hat to store value for.
-     * @param {*} newValue Value to store for edge-activated hat.
-     * @return {*} The old value for the edge-activated hat.
-     */
-    updateEdgeActivatedValue (blockId, newValue) {
-        const oldValue = this._edgeActivatedHatValues[blockId];
-        this._edgeActivatedHatValues[blockId] = newValue;
-        return oldValue;
-    }
-
-    /**
-     * Clear all edge-activaed hat values.
-     */
-    clearEdgeActivatedValues () {
-        this._edgeActivatedHatValues = {};
-    }
 
     /**
      * Attach the audio engine
@@ -1687,7 +1662,7 @@ class Runtime extends EventEmitter {
         this.stopAll();
         this.emit(Runtime.PROJECT_START);
         this.ioDevices.clock.resetProjectTimer();
-        this.clearEdgeActivatedValues();
+        this.targets.forEach(target => target.clearEdgeActivatedValues());
         // Inform all targets of the green flag.
         for (let i = 0; i < this.targets.length; i++) {
             this.targets[i].onGreenFlag();

src/engine/target.js

@@ -63,6 +63,13 @@ class Target extends EventEmitter {
          */
         this._customState = {};
 
+        /**
+         * Currently known values for edge-activated hats.
+         * Keys are block ID for the hat; values are the currently known values.
+         * @type {Object.<string, *>}
+         */
+        this._edgeActivatedHatValues = {};
+
         if (this.runtime) {
             this.runtime.addExecutable(this);
         }
@@ -84,6 +91,29 @@ class Target extends EventEmitter {
         return this.id;
     }
 
+    /**
+     * Update an edge-activated hat block value.
+     * @param {!string} blockId ID of hat to store value for.
+     * @param {*} newValue Value to store for edge-activated hat.
+     * @return {*} The old value for the edge-activated hat.
+     */
+    updateEdgeActivatedValue (blockId, newValue) {
+        const oldValue = this._edgeActivatedHatValues[blockId];
+        this._edgeActivatedHatValues[blockId] = newValue;
+        return oldValue;
+    }
+
+    hasEdgeActivatedValue (blockId) {
+        return this._edgeActivatedHatValues.hasOwnProperty(blockId);
+    }
+
+    /**
+     * Clear all edge-activaed hat values.
+     */
+    clearEdgeActivatedValues () {
+        this._edgeActivatedHatValues = {};
+    }
+
     /**
      * Look up a variable object, first by id, and then by name if the id is not found.
      * Create a new variable if both lookups fail.

src/sprites/rendered-target.js

@@ -1,6 +1,7 @@
 const log = require('../util/log');
 const MathUtil = require('../util/math-util');
 const StringUtil = require('../util/string-util');
+const Clone = require('../util/clone');
 const Target = require('../engine/target');
 const StageLayering = require('../engine/stage-layering');
 
@@ -1015,8 +1016,9 @@ class RenderedTarget extends Target {
         newClone.size = this.size;
         newClone.currentCostume = this.currentCostume;
         newClone.rotationStyle = this.rotationStyle;
-        newClone.effects = JSON.parse(JSON.stringify(this.effects));
+        newClone.effects = Clone.simple(this.effects);
         newClone.variables = this.duplicateVariables();
+        newClone._edgeActivatedHatValues = Clone.simple(this._edgeActivatedHatValues);
         newClone.initDrawable(StageLayering.SPRITE_LAYER);
         newClone.updateAllDrawableProperties();
         // Place behind the current target.

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

@@ -5,6 +5,7 @@ const readFileToBuffer = require('../fixtures/readProjectFile').readFileToBuffer
 const VirtualMachine = require('../../src/index');
 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 project = readFileToBuffer(projectUri);
@@ -103,6 +104,108 @@ test('edge activated hat thread not added twice', t => {
     });
 });
 
+
+/**
+ * Duplicating a sprite should also track duplicated edge activated hat in
+ * runtime's _edgeActivatedHatValues map.
+ */
+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
+    // the sprite can be duplicated
+    const projectWithSpriteUri = path.resolve(__dirname, '../fixtures/edge-triggered-hat.sb3');
+    const projectWithSprite = readFileToBuffer(projectWithSpriteUri);
+
+    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;
+        vm.clear();
+        vm.setCompatibilityMode(false);
+        vm.setTurboMode(false);
+
+        vm.loadProject(projectWithSprite).then(() => {
+            t.equal(vm.runtime.threads.length, 0);
+
+            vm.runtime._step();
+            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);
+
+            vm.duplicateSprite(vm.runtime.targets[1].id).then(() => {
+                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);
+                t.end();
+            });
+
+        });
+    });
+});
+
+/**
+ * Cloning a sprite should also track cloned edge activated hat separately
+ * runtime's _edgeActivatedHatValues map.
+ */
+test('edge activated hat should trigger for both sprites when sprite is cloned', t => {
+
+    // Project that is similar to loudness-hat-block.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);
+
+    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;
+        vm.clear();
+        vm.setCompatibilityMode(false);
+        vm.setTurboMode(false);
+
+        vm.loadProject(projectWithSprite).then(() => {
+            t.equal(vm.runtime.threads.length, 0);
+
+            vm.runtime._step();
+            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);
+
+            vm.runtime.targets.push(vm.runtime.targets[1].makeClone());
+
+            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);
+            t.end();
+        });
+    });
+});
+
 /**
  * When adding a stack click thread first, make sure that the edge activated hat thread and
  * the stack click thread are both pushed and run (despite having the same top block)