diff --git a/src/blocks/scratch3_control.js b/src/blocks/scratch3_control.js index d0572c412..00c72e549 100644 --- a/src/blocks/scratch3_control.js +++ b/src/blocks/scratch3_control.js @@ -165,7 +165,10 @@ class Scratch3ControlBlocks { // Create clone const newClone = cloneTarget.makeClone(); if (newClone) { - this.runtime.targets.push(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/engine/target.js b/src/engine/target.js index 2ac3a367f..e34748fe2 100644 --- a/src/engine/target.js +++ b/src/engine/target.js @@ -69,10 +69,6 @@ class Target extends EventEmitter { * @type {Object.} */ this._edgeActivatedHatValues = {}; - - if (this.runtime) { - this.runtime.addExecutable(this); - } } /** diff --git a/src/serialization/sb2.js b/src/serialization/sb2.js index 8482789f0..8cc49cd8a 100644 --- a/src/serialization/sb2.js +++ b/src/serialization/sb2.js @@ -781,14 +781,17 @@ const reorderParsedTargets = function (targets) { // Reorder parsed targets based on the temporary targetPaneOrder property // and then delete it. - targets.sort((a, b) => a.targetPaneOrder - b.targetPaneOrder); + const reordered = targets.map((t, index) => { + t.layerOrder = index; + return t; + }).sort((a, b) => a.targetPaneOrder - b.targetPaneOrder); // Delete the temporary target pane ordering since we shouldn't need it anymore. - targets.forEach(t => { + reordered.forEach(t => { delete t.targetPaneOrder; }); - return targets; + return reordered; }; diff --git a/src/serialization/sb3.js b/src/serialization/sb3.js index 254257069..1b3e01729 100644 --- a/src/serialization/sb3.js +++ b/src/serialization/sb3.js @@ -1157,12 +1157,18 @@ const deserialize = function (json, runtime, zip, isSingleSprite) { parseScratchObject(target, runtime, extensions, zip)) ) .then(targets => targets // Re-sort targets back into original sprite-pane ordering + .map((t, i) => { + // Add layer order property to deserialized targets. + // This property is used to initialize executable targets in + // the correct order and is deleted in VM's installTargets function + t.layerOrder = i; + return t; + }) .sort((a, b) => a.targetPaneOrder - b.targetPaneOrder) .map(t => { // Delete the temporary properties used for // sprite pane ordering and stage layer ordering delete t.targetPaneOrder; - delete t.layerOrder; return t; })) .then(targets => { diff --git a/src/sprites/rendered-target.js b/src/sprites/rendered-target.js index 019513f22..78243a269 100644 --- a/src/sprites/rendered-target.js +++ b/src/sprites/rendered-target.js @@ -1021,8 +1021,6 @@ class RenderedTarget extends Target { newClone._edgeActivatedHatValues = Clone.simple(this._edgeActivatedHatValues); newClone.initDrawable(StageLayering.SPRITE_LAYER); newClone.updateAllDrawableProperties(); - // Place behind the current target. - newClone.goBehindOther(this); return newClone; } @@ -1046,7 +1044,6 @@ class RenderedTarget extends Target { newTarget.effects = JSON.parse(JSON.stringify(this.effects)); newTarget.variables = this.duplicateVariables(newTarget.blocks); newTarget.updateAllDrawableProperties(); - newTarget.goBehindOther(this); return newTarget; }); } diff --git a/src/virtual-machine.js b/src/virtual-machine.js index c19098ec8..599d23405 100644 --- a/src/virtual-machine.js +++ b/src/virtual-machine.js @@ -495,11 +495,18 @@ 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()); }); + // Sort the executable targets by layerOrder. + // Remove layerOrder property after use. + this.runtime.executableTargets.sort((a, b) => a.layerOrder - b.layerOrder); + targets.forEach(target => { + delete target.layerOrder; + }); + // Select the first target for editing, e.g., the first sprite. if (wholeProject && (targets.length > 1)) { this.editingTarget = targets[1]; @@ -1016,7 +1023,8 @@ 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.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 a31f260dd..32b282ff8 100644 --- a/test/integration/hat-threads-run-every-frame.js +++ b/test/integration/hat-threads-run-every-frame.js @@ -192,7 +192,8 @@ test('edge activated hat should trigger for both sprites when sprite is cloned', val + Object.keys(target._edgeActivatedHatValues).length, 0); t.equal(numTargetEdgeHats, 1); - vm.runtime.targets.push(vm.runtime.targets[1].makeClone()); + const cloneTarget = vm.runtime.targets[1].makeClone(); + 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 56dd10f1f..363eab66b 100644 --- a/test/unit/blocks_event.js +++ b/test/unit/blocks_event.js @@ -54,7 +54,8 @@ test('#760 - broadcastAndWait', t => { const tgt = new Target(rt, b); tgt.isStage = true; tgt.createVariable('testBroadcastID', 'message', Variable.BROADCAST_MESSAGE_TYPE); - rt.targets.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'); diff --git a/test/unit/serialization_sb3.js b/test/unit/serialization_sb3.js index 9e9785b2a..5ef1ac80d 100644 --- a/test/unit/serialization_sb3.js +++ b/test/unit/serialization_sb3.js @@ -1,6 +1,7 @@ const test = require('tap').test; const path = require('path'); const VirtualMachine = require('../../src/index'); +const Runtime = require('../../src/engine/runtime'); const sb3 = require('../../src/serialization/sb3'); const readFileToBuffer = require('../fixtures/readProjectFile').readFileToBuffer; const exampleProjectPath = path.resolve(__dirname, '../fixtures/clone-cleanup.sb2'); @@ -147,10 +148,10 @@ test('deserialize sb3 project with comments - no duplicate id serialization', t }); }); -test('serialize sb3 preserves sprite layer order', t => { +test('serializing and deserializing sb3 preserves sprite layer order', t => { const vm = new VirtualMachine(); vm.attachRenderer(new FakeRenderer()); - vm.loadProject(readFileToBuffer(path.resolve(__dirname, '../fixtures/ordering.sb2'))) + return vm.loadProject(readFileToBuffer(path.resolve(__dirname, '../fixtures/ordering.sb2'))) .then(() => { // Target get layer order needs a renderer, // fake the numbers we would get back from the @@ -182,8 +183,28 @@ test('serialize sb3 preserves sprite layer order', t => { t.equal(result.targets[2].layerOrder, 1); t.equal(result.targets[3].layerOrder, 3); - t.end(); - }); + return result; + }) + .then(serializedObject => + sb3.deserialize( + JSON.parse(JSON.stringify(serializedObject)), new Runtime(), null, false) + .then(({targets}) => { + // First check that the sprites are ordered correctly (as they would + // appear in the target pane) + t.equal(targets[0].sprite.name, 'Stage'); + t.equal(targets[1].sprite.name, 'First'); + t.equal(targets[2].sprite.name, 'Second'); + t.equal(targets[3].sprite.name, 'Third'); + + // Check that they are in the correct layer order (as they would render + // back to front on the stage) + t.equal(targets[0].layerOrder, 0); + t.equal(targets[1].layerOrder, 2); + t.equal(targets[2].layerOrder, 1); + t.equal(targets[3].layerOrder, 3); + + t.end(); + })); }); test('serializeBlocks', t => {