From 59a865ef470d726837253d21055e481f65f6dc1c Mon Sep 17 00:00:00 2001
From: Karishma Chadha <kchadha@media.mit.edu>
Date: Mon, 14 Jan 2019 11:48:11 -0500
Subject: [PATCH] Create a new `addTarget` function on the runtime which
 populates the targets list as well as the executable targets list.

---
 src/blocks/scratch3_control.js                  |  3 +--
 src/engine/runtime.js                           | 11 +++++++----
 src/virtual-machine.js                          | 13 +++++--------
 test/integration/hat-threads-run-every-frame.js |  3 +--
 test/unit/blocks_event.js                       |  5 ++---
 test/unit/blocks_looks.js                       |  2 +-
 test/unit/engine_target.js                      |  8 ++++----
 7 files changed, 21 insertions(+), 24 deletions(-)

diff --git a/src/blocks/scratch3_control.js b/src/blocks/scratch3_control.js
index df426ea2a..00c72e549 100644
--- a/src/blocks/scratch3_control.js
+++ b/src/blocks/scratch3_control.js
@@ -165,8 +165,7 @@ class Scratch3ControlBlocks {
         // Create clone
         const newClone = cloneTarget.makeClone();
         if (newClone) {
-            this.runtime.targets.push(newClone);
-            this.runtime.addExecutable(newClone);
+            this.runtime.addTarget(newClone);
 
             // Place behind the original target.
             newClone.goBehindOther(cloneTarget);
diff --git a/src/engine/runtime.js b/src/engine/runtime.js
index 778761845..f9a0178df 100644
--- a/src/engine/runtime.js
+++ b/src/engine/runtime.js
@@ -1560,11 +1560,14 @@ class Runtime extends EventEmitter {
     }
 
     /**
-     * Add a target to the execution order.
-     * @param {Target} executableTarget target to add
+     * Add a target to the runtime. This tracks the sprite pane
+     * ordering of the target. The target still needs to be put
+     * into the correct execution order after calling this function.
+     * @param {Target} target target to add
      */
-    addExecutable (executableTarget) {
-        this.executableTargets.push(executableTarget);
+    addTarget (target) {
+        this.targets.push(target);
+        this.executableTargets.push(target);
     }
 
     /**
diff --git a/src/virtual-machine.js b/src/virtual-machine.js
index e2ee1fa66..599d23405 100644
--- a/src/virtual-machine.js
+++ b/src/virtual-machine.js
@@ -495,17 +495,15 @@ class VirtualMachine extends EventEmitter {
 
         return Promise.all(extensionPromises).then(() => {
             targets.forEach(target => {
-                this.runtime.targets.push(target);
+                this.runtime.addTarget(target);
                 (/** @type RenderedTarget */ target).updateAllDrawableProperties();
                 // Ensure unique sprite name
                 if (target.isSprite()) this.renameSprite(target.id, target.getName());
             });
-            // Initialize executable targets sorted by the layerOrder.
+            // Sort the executable targets by layerOrder.
             // Remove layerOrder property after use.
-            const executableTargets = targets.slice()
-                .sort((a, b) => a.layerOrder - b.layerOrder);
-            executableTargets.forEach(target => {
-                this.runtime.addExecutable(target);
+            this.runtime.executableTargets.sort((a, b) => a.layerOrder - b.layerOrder);
+            targets.forEach(target => {
                 delete target.layerOrder;
             });
 
@@ -1025,8 +1023,7 @@ class VirtualMachine extends EventEmitter {
             throw new Error('No sprite associated with this target.');
         }
         return target.duplicate().then(newTarget => {
-            this.runtime.targets.push(newTarget);
-            this.runtime.addExecutable(newTarget);
+            this.runtime.addTarget(newTarget);
             newTarget.goBehindOther(target);
             this.setEditingTarget(newTarget.id);
         });
diff --git a/test/integration/hat-threads-run-every-frame.js b/test/integration/hat-threads-run-every-frame.js
index 957624be7..32b282ff8 100644
--- a/test/integration/hat-threads-run-every-frame.js
+++ b/test/integration/hat-threads-run-every-frame.js
@@ -193,8 +193,7 @@ test('edge activated hat should trigger for both sprites when sprite is cloned',
             t.equal(numTargetEdgeHats, 1);
 
             const cloneTarget = vm.runtime.targets[1].makeClone();
-            vm.runtime.targets.push(cloneTarget);
-            vm.runtime.addExecutable(cloneTarget);
+            vm.runtime.addTarget(cloneTarget);
 
             vm.runtime._step();
             // Check that the runtime's _edgeActivatedHatValues object has two separate keys
diff --git a/test/unit/blocks_event.js b/test/unit/blocks_event.js
index b52bc951b..363eab66b 100644
--- a/test/unit/blocks_event.js
+++ b/test/unit/blocks_event.js
@@ -54,9 +54,8 @@ test('#760 - broadcastAndWait', t => {
     const tgt = new Target(rt, b);
     tgt.isStage = true;
     tgt.createVariable('testBroadcastID', 'message', Variable.BROADCAST_MESSAGE_TYPE);
-    // Need to add to both runtime.targets as well as runtime.executableTargets here
-    rt.targets.push(tgt);
-    rt.executableTargets.push(tgt);
+
+    rt.addTarget(tgt);
 
     let th = rt._pushThread('broadcastAndWaitBlock', t);
     const util = new BlockUtility();
diff --git a/test/unit/blocks_looks.js b/test/unit/blocks_looks.js
index 779f36109..61b6ddd58 100644
--- a/test/unit/blocks_looks.js
+++ b/test/unit/blocks_looks.js
@@ -48,7 +48,7 @@ const testCostume = (costumes, arg, currentCostume = 1, isStage = false) => {
 
     if (isStage) {
         target.isStage = true;
-        rt.targets.push(target);
+        rt.addTarget(target);
         looks.switchBackdrop({BACKDROP: arg}, {target});
     } else {
         looks.switchCostume({COSTUME: arg}, {target});
diff --git a/test/unit/engine_target.js b/test/unit/engine_target.js
index 5d899399f..a656d5de7 100644
--- a/test/unit/engine_target.js
+++ b/test/unit/engine_target.js
@@ -188,7 +188,7 @@ test('renameVariable calls cloud io device\'s requestRenameVariable function', t
     target.isStage = true;
     const mockCloudVar = new Variable('foo', 'bar', Variable.SCALAR_TYPE, true);
     target.variables[mockCloudVar.id] = mockCloudVar;
-    runtime.targets.push(target);
+    runtime.addTarget(target);
 
     target.renameVariable('foo', 'bar2');
 
@@ -215,7 +215,7 @@ test('renameVariable does not call cloud io device\'s requestRenameVariable func
     const target = new Target(runtime);
     const mockCloudVar = new Variable('foo', 'bar', Variable.SCALAR_TYPE, true);
     target.variables[mockCloudVar.id] = mockCloudVar;
-    runtime.targets.push(target);
+    runtime.addTarget(target);
 
     target.renameVariable('foo', 'bar2');
 
@@ -266,7 +266,7 @@ test('deleteVariable calls cloud io device\'s requestRenameVariable function', t
     target.isStage = true;
     const mockCloudVar = new Variable('foo', 'bar', Variable.SCALAR_TYPE, true);
     target.variables[mockCloudVar.id] = mockCloudVar;
-    runtime.targets.push(target);
+    runtime.addTarget(target);
 
     target.deleteVariable('foo');
 
@@ -288,7 +288,7 @@ test('deleteVariable calls cloud io device\'s requestRenameVariable function', t
     const target = new Target(runtime);
     const mockCloudVar = new Variable('foo', 'bar', Variable.SCALAR_TYPE, true);
     target.variables[mockCloudVar.id] = mockCloudVar;
-    runtime.targets.push(target);
+    runtime.addTarget(target);
 
     target.deleteVariable('foo');