From 88e44c65eb65b9d39cf747630627bdb32ab49026 Mon Sep 17 00:00:00 2001 From: Christopher Willis-Ford Date: Thu, 8 Jun 2017 12:47:56 -0700 Subject: [PATCH 1/5] Remember the data format when an asset is imported --- src/import/load-costume.js | 2 +- src/import/load-sound.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/import/load-costume.js b/src/import/load-costume.js index 54962ea1f..d435129b2 100644 --- a/src/import/load-costume.js +++ b/src/import/load-costume.js @@ -32,7 +32,7 @@ const loadCostume = function (md5ext, costume, runtime) { let promise = runtime.storage.load(assetType, md5, ext).then(costumeAsset => { costume.assetId = costumeAsset.assetId; - costume.assetType = assetType; + costume.dataFormat = ext; return costumeAsset; }); diff --git a/src/import/load-sound.js b/src/import/load-sound.js index 80b595987..295b87ebe 100644 --- a/src/import/load-sound.js +++ b/src/import/load-sound.js @@ -24,7 +24,7 @@ const loadSound = function (sound, runtime) { return runtime.storage.load(runtime.storage.AssetType.Sound, md5, ext) .then(soundAsset => { sound.assetId = soundAsset.assetId; - sound.assetType = runtime.storage.AssetType.Sound; + sound.dataFormat = ext; return runtime.audioEngine.decodeSound(Object.assign( {}, sound, From cd1c72c7cc3610710cb0cd4ce40f5284652411f2 Mon Sep 17 00:00:00 2001 From: Christopher Willis-Ford Date: Fri, 9 Jun 2017 14:54:10 -0700 Subject: [PATCH 2/5] Convert `attachTestStorage` to `makeTestStorage` Rather than assuming that the storage instance will be attached to a VM, just return it. Callers may attach it to a `VM` or (in the case of `import_sb2.js`) to a `Runtime`. --- .../{attach-test-storage.js => make-test-storage.js} | 10 +++++----- test/integration/clone-cleanup.js | 4 ++-- test/integration/complex.js | 4 ++-- test/integration/control.js | 4 ++-- test/integration/data.js | 4 ++-- test/integration/event.js | 4 ++-- test/integration/hat-execution-order.js | 4 ++-- test/integration/import_sb2.js | 4 ++-- test/integration/looks.js | 4 ++-- test/integration/motion.js | 4 ++-- test/integration/pen.js | 4 ++-- test/integration/procedure.js | 4 ++-- test/integration/sensing.js | 4 ++-- test/integration/sound.js | 4 ++-- 14 files changed, 31 insertions(+), 31 deletions(-) rename test/fixtures/{attach-test-storage.js => make-test-storage.js} (79%) diff --git a/test/fixtures/attach-test-storage.js b/test/fixtures/make-test-storage.js similarity index 79% rename from test/fixtures/attach-test-storage.js rename to test/fixtures/make-test-storage.js index bb0cc3a08..0179ca34a 100644 --- a/test/fixtures/attach-test-storage.js +++ b/test/fixtures/make-test-storage.js @@ -33,15 +33,15 @@ const getAssetUrl = function (asset) { }; /** - * Construct a new instance of ScratchStorage, provide it with default web sources, and attach it to the provided VM. - * @param {VirtualMachine} vm - the VM which will own the new ScratchStorage instance. + * Construct a new instance of ScratchStorage and provide it with default web sources. + * @returns {ScratchStorage} - an instance of ScratchStorage, ready to be used for tests. */ -const attachTestStorage = function (vm) { +const makeTestStorage = function () { const storage = new ScratchStorage(); const AssetType = storage.AssetType; storage.addWebSource([AssetType.Project], getProjectUrl); storage.addWebSource([AssetType.ImageVector, AssetType.ImageBitmap, AssetType.Sound], getAssetUrl); - vm.attachStorage(storage); + return storage; }; -module.exports = attachTestStorage; +module.exports = makeTestStorage; diff --git a/test/integration/clone-cleanup.js b/test/integration/clone-cleanup.js index bfecd6d77..8b801a00e 100644 --- a/test/integration/clone-cleanup.js +++ b/test/integration/clone-cleanup.js @@ -1,6 +1,6 @@ const path = require('path'); const test = require('tap').test; -const attachTestStorage = require('../fixtures/attach-test-storage'); +const makeTestStorage = require('../fixtures/make-test-storage'); const extract = require('../fixtures/extract'); const VirtualMachine = require('../../src/index'); @@ -9,7 +9,7 @@ const project = extract(projectUri); test('clone-cleanup', t => { const vm = new VirtualMachine(); - attachTestStorage(vm); + vm.attachStorage(makeTestStorage()); /** * Track which step of the project is currently under test. diff --git a/test/integration/complex.js b/test/integration/complex.js index 39bc0c667..113928483 100644 --- a/test/integration/complex.js +++ b/test/integration/complex.js @@ -1,7 +1,7 @@ const fs = require('fs'); const path = require('path'); const test = require('tap').test; -const attachTestStorage = require('../fixtures/attach-test-storage'); +const makeTestStorage = require('../fixtures/make-test-storage'); const extract = require('../fixtures/extract'); const VirtualMachine = require('../../src/index'); @@ -13,7 +13,7 @@ const sprite = fs.readFileSync(spriteUri, 'utf8'); test('complex', t => { const vm = new VirtualMachine(); - attachTestStorage(vm); + vm.attachStorage(makeTestStorage()); // Evaluate playground data and exit vm.on('playgroundData', e => { diff --git a/test/integration/control.js b/test/integration/control.js index 4b65a0166..66b4c0283 100644 --- a/test/integration/control.js +++ b/test/integration/control.js @@ -1,6 +1,6 @@ const path = require('path'); const test = require('tap').test; -const attachTestStorage = require('../fixtures/attach-test-storage'); +const makeTestStorage = require('../fixtures/make-test-storage'); const extract = require('../fixtures/extract'); const VirtualMachine = require('../../src/index'); @@ -9,7 +9,7 @@ const project = extract(uri); test('control', t => { const vm = new VirtualMachine(); - attachTestStorage(vm); + vm.attachStorage(makeTestStorage()); // Evaluate playground data and exit vm.on('playgroundData', e => { diff --git a/test/integration/data.js b/test/integration/data.js index ba263fec1..c6cd71d1e 100644 --- a/test/integration/data.js +++ b/test/integration/data.js @@ -1,6 +1,6 @@ const path = require('path'); const test = require('tap').test; -const attachTestStorage = require('../fixtures/attach-test-storage'); +const makeTestStorage = require('../fixtures/make-test-storage'); const extract = require('../fixtures/extract'); const VirtualMachine = require('../../src/index'); @@ -9,7 +9,7 @@ const project = extract(uri); test('data', t => { const vm = new VirtualMachine(); - attachTestStorage(vm); + vm.attachStorage(makeTestStorage()); // Evaluate playground data and exit vm.on('playgroundData', () => { diff --git a/test/integration/event.js b/test/integration/event.js index 34e21c63b..c4f3ee58a 100644 --- a/test/integration/event.js +++ b/test/integration/event.js @@ -1,6 +1,6 @@ const path = require('path'); const test = require('tap').test; -const attachTestStorage = require('../fixtures/attach-test-storage'); +const makeTestStorage = require('../fixtures/make-test-storage'); const extract = require('../fixtures/extract'); const VirtualMachine = require('../../src/index'); @@ -9,7 +9,7 @@ const project = extract(uri); test('event', t => { const vm = new VirtualMachine(); - attachTestStorage(vm); + vm.attachStorage(makeTestStorage()); // Evaluate playground data and exit vm.on('playgroundData', e => { diff --git a/test/integration/hat-execution-order.js b/test/integration/hat-execution-order.js index 44efcfa02..c8d2dcabf 100644 --- a/test/integration/hat-execution-order.js +++ b/test/integration/hat-execution-order.js @@ -1,6 +1,6 @@ const path = require('path'); const test = require('tap').test; -const attachTestStorage = require('../fixtures/attach-test-storage'); +const makeTestStorage = require('../fixtures/make-test-storage'); const extract = require('../fixtures/extract'); const VirtualMachine = require('../../src/index'); @@ -9,7 +9,7 @@ const project = extract(projectUri); test('complex', t => { const vm = new VirtualMachine(); - attachTestStorage(vm); + vm.attachStorage(makeTestStorage()); // Evaluate playground data and exit vm.on('playgroundData', e => { diff --git a/test/integration/import_sb2.js b/test/integration/import_sb2.js index 3b375a44c..f9a0d2d0d 100644 --- a/test/integration/import_sb2.js +++ b/test/integration/import_sb2.js @@ -1,6 +1,6 @@ const path = require('path'); const test = require('tap').test; -const attachTestStorage = require('../fixtures/attach-test-storage'); +const makeTestStorage = require('../fixtures/make-test-storage'); const extract = require('../fixtures/extract'); const renderedTarget = require('../../src/sprites/rendered-target'); @@ -20,7 +20,7 @@ test('default', t => { // Create runtime instance & load SB2 into it const rt = new runtime(); - attachTestStorage(rt); + rt.attachStorage(makeTestStorage()); sb2.deserialize(json, rt).then(targets => { // Test t.type(file, 'string'); diff --git a/test/integration/looks.js b/test/integration/looks.js index d251d4f85..874646496 100644 --- a/test/integration/looks.js +++ b/test/integration/looks.js @@ -1,6 +1,6 @@ const path = require('path'); const test = require('tap').test; -const attachTestStorage = require('../fixtures/attach-test-storage'); +const makeTestStorage = require('../fixtures/make-test-storage'); const extract = require('../fixtures/extract'); const VirtualMachine = require('../../src/index'); @@ -9,7 +9,7 @@ const project = extract(uri); test('looks', t => { const vm = new VirtualMachine(); - attachTestStorage(vm); + vm.attachStorage(makeTestStorage()); // Evaluate playground data and exit vm.on('playgroundData', e => { diff --git a/test/integration/motion.js b/test/integration/motion.js index d8b1fc8d7..689b21871 100644 --- a/test/integration/motion.js +++ b/test/integration/motion.js @@ -1,6 +1,6 @@ const path = require('path'); const test = require('tap').test; -const attachTestStorage = require('../fixtures/attach-test-storage'); +const makeTestStorage = require('../fixtures/make-test-storage'); const extract = require('../fixtures/extract'); const VirtualMachine = require('../../src/index'); @@ -9,7 +9,7 @@ const project = extract(uri); test('motion', t => { const vm = new VirtualMachine(); - attachTestStorage(vm); + vm.attachStorage(makeTestStorage()); // Evaluate playground data and exit vm.on('playgroundData', e => { diff --git a/test/integration/pen.js b/test/integration/pen.js index f4da55fd1..24d86e953 100644 --- a/test/integration/pen.js +++ b/test/integration/pen.js @@ -1,6 +1,6 @@ const path = require('path'); const test = require('tap').test; -const attachTestStorage = require('../fixtures/attach-test-storage'); +const makeTestStorage = require('../fixtures/make-test-storage'); const extract = require('../fixtures/extract'); const VirtualMachine = require('../../src/index'); @@ -9,7 +9,7 @@ const project = extract(uri); test('pen', t => { const vm = new VirtualMachine(); - attachTestStorage(vm); + vm.attachStorage(makeTestStorage()); // Evaluate playground data and exit vm.on('playgroundData', () => { diff --git a/test/integration/procedure.js b/test/integration/procedure.js index 4abcf1e41..e62f2b108 100644 --- a/test/integration/procedure.js +++ b/test/integration/procedure.js @@ -1,6 +1,6 @@ const path = require('path'); const test = require('tap').test; -const attachTestStorage = require('../fixtures/attach-test-storage'); +const makeTestStorage = require('../fixtures/make-test-storage'); const extract = require('../fixtures/extract'); const VirtualMachine = require('../../src/index'); @@ -9,7 +9,7 @@ const project = extract(uri); test('procedure', t => { const vm = new VirtualMachine(); - attachTestStorage(vm); + vm.attachStorage(makeTestStorage()); // Evaluate playground data and exit vm.on('playgroundData', e => { diff --git a/test/integration/sensing.js b/test/integration/sensing.js index ca8fa1bb5..1d7d30852 100644 --- a/test/integration/sensing.js +++ b/test/integration/sensing.js @@ -1,6 +1,6 @@ const path = require('path'); const test = require('tap').test; -const attachTestStorage = require('../fixtures/attach-test-storage'); +const makeTestStorage = require('../fixtures/make-test-storage'); const extract = require('../fixtures/extract'); const VirtualMachine = require('../../src/index'); @@ -9,7 +9,7 @@ const project = extract(uri); test('sensing', t => { const vm = new VirtualMachine(); - attachTestStorage(vm); + vm.attachStorage(makeTestStorage()); // Evaluate playground data and exit vm.on('playgroundData', e => { diff --git a/test/integration/sound.js b/test/integration/sound.js index 7ab101593..50100d70f 100644 --- a/test/integration/sound.js +++ b/test/integration/sound.js @@ -1,6 +1,6 @@ const path = require('path'); const test = require('tap').test; -const attachTestStorage = require('../fixtures/attach-test-storage'); +const makeTestStorage = require('../fixtures/make-test-storage'); const extract = require('../fixtures/extract'); const VirtualMachine = require('../../src/index'); @@ -9,7 +9,7 @@ const project = extract(uri); test('sound', t => { const vm = new VirtualMachine(); - attachTestStorage(vm); + vm.attachStorage(makeTestStorage()); // Evaluate playground data and exit vm.on('playgroundData', e => { From 4b1aad5099853c65d47a87743b8a438d51b8b9a0 Mon Sep 17 00:00:00 2001 From: Christopher Willis-Ford Date: Mon, 12 Jun 2017 11:27:51 -0700 Subject: [PATCH 3/5] Add test for SB3 serialization round trip --- test/integration/sb3-roundtrip.js | 110 ++++++++++++++++++++++++++++++ 1 file changed, 110 insertions(+) create mode 100644 test/integration/sb3-roundtrip.js diff --git a/test/integration/sb3-roundtrip.js b/test/integration/sb3-roundtrip.js new file mode 100644 index 000000000..bc5da9d0f --- /dev/null +++ b/test/integration/sb3-roundtrip.js @@ -0,0 +1,110 @@ +const test = require('tap').test; + +const Blocks = require('../../src/engine/blocks'); +const Clone = require('../../src/util/clone'); +const loadCostume = require('../../src/import/load-costume'); +const loadSound = require('../../src/import/load-sound'); +const makeTestStorage = require('../fixtures/make-test-storage'); +const Runtime = require('../../src/engine/runtime'); +const sb3 = require('../../src/serialization/sb3'); +const Sprite = require('../../src/sprites/sprite'); + +const defaultCostumeInfo = { + bitmapResolution: 1, + rotationCenterX: 0, + rotationCenterY: 0 +}; + +const defaultSoundInfo = { +}; + +test('sb3-roundtrip', t => { + const runtime1 = new Runtime(); + runtime1.attachStorage(makeTestStorage()); + + const runtime2 = new Runtime(); + runtime2.attachStorage(makeTestStorage()); + + const testRuntimeState = (label, runtime) => { + t.strictEqual(runtime.targets.length, 2, `${label}: target count`); + const [stageClone, spriteClone] = runtime.targets; + + t.strictEqual(stageClone.isOriginal, true); + t.strictEqual(stageClone.isStage, true); + + const stage = stageClone.sprite; + t.strictEqual(stage.name, 'Stage'); + t.strictEqual(stage.clones.length, 1); + t.strictEqual(stage.clones[0], stageClone); + + t.strictEqual(stage.costumes.length, 1); + const [building] = stage.costumes; + t.strictEqual(building.assetId, 'fe5e3566965f9de793beeffce377d054'); + t.strictEqual(building.dataFormat, 'jpg'); + + t.strictEqual(stage.sounds.length, 0); + + t.strictEqual(spriteClone.isOriginal, true); + t.strictEqual(spriteClone.isStage, false); + + const sprite = spriteClone.sprite; + t.strictEqual(sprite.name, 'Sprite'); + t.strictEqual(sprite.clones.length, 1); + t.strictEqual(sprite.clones[0], spriteClone); + + t.strictEqual(sprite.costumes.length, 2); + const [cat, squirrel] = sprite.costumes; + t.strictEqual(cat.assetId, 'f88bf1935daea28f8ca098462a31dbb0'); + t.strictEqual(cat.dataFormat, 'svg'); + t.strictEqual(squirrel.assetId, '7e24c99c1b853e52f8e7f9004416fa34'); + t.strictEqual(squirrel.dataFormat, 'png'); + + t.strictEqual(sprite.sounds.length, 1); + const [meow] = sprite.sounds; + t.strictEqual(meow.md5, '83c36d806dc92327b9e7049a565c6bff.wav'); + }; + + const loadThings = Promise.all([ + loadCostume('fe5e3566965f9de793beeffce377d054.jpg', Clone.simple(defaultCostumeInfo), runtime1), + loadCostume('f88bf1935daea28f8ca098462a31dbb0.svg', Clone.simple(defaultCostumeInfo), runtime1), + loadCostume('7e24c99c1b853e52f8e7f9004416fa34.png', Clone.simple(defaultCostumeInfo), runtime1), + loadSound(Object.assign({md5: '83c36d806dc92327b9e7049a565c6bff.wav'}, defaultSoundInfo), runtime1) + ]); + + const installThings = loadThings.then(results => { + const [building, cat, squirrel, meow] = results; + + const stageBlocks = new Blocks(); + const stage = new Sprite(stageBlocks, runtime1); + stage.name = 'Stage'; + stage.costumes = [building]; + stage.sounds = []; + const stageClone = stage.createClone(); + stageClone.isStage = true; + + const spriteBlocks = new Blocks(); + const sprite = new Sprite(spriteBlocks, runtime1); + sprite.name = 'Sprite'; + sprite.costumes = [cat, squirrel]; + sprite.sounds = [meow]; + const spriteClone = sprite.createClone(); + + runtime1.targets = [stageClone, spriteClone]; + + testRuntimeState('original', runtime1); + }); + + const serializeAndDeserialize = installThings.then(() => { + // `Clone.simple` here helps in two ways: + // 1. it ensures that any non-serializable data is thrown away, and + // 2. `sb3.deserialize` does some `hasOwnProperty` checks which fail on the object returned by `sb3.serialize` + // due to its inheritance structure. + const serializedState = Clone.simple(sb3.serialize(runtime1)); + return sb3.deserialize(serializedState, runtime2); + }); + + return serializeAndDeserialize.then(targets => { + runtime2.targets = targets; + testRuntimeState('copy', runtime2); + }); +}); From 1bf180882654605b9c32001f07e5727d23b6c915 Mon Sep 17 00:00:00 2001 From: Christopher Willis-Ford Date: Tue, 13 Jun 2017 13:58:34 -0700 Subject: [PATCH 4/5] Clarify "flattening" step in SB3 roundtrip test --- test/integration/sb3-roundtrip.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/integration/sb3-roundtrip.js b/test/integration/sb3-roundtrip.js index bc5da9d0f..c401248bd 100644 --- a/test/integration/sb3-roundtrip.js +++ b/test/integration/sb3-roundtrip.js @@ -95,11 +95,11 @@ test('sb3-roundtrip', t => { }); const serializeAndDeserialize = installThings.then(() => { - // `Clone.simple` here helps in two ways: + // Doing a JSON `stringify` and `parse` here more accurately simulate a save/load cycle. In particular: // 1. it ensures that any non-serializable data is thrown away, and - // 2. `sb3.deserialize` does some `hasOwnProperty` checks which fail on the object returned by `sb3.serialize` - // due to its inheritance structure. - const serializedState = Clone.simple(sb3.serialize(runtime1)); + // 2. `sb3.deserialize` and its helpers do some `hasOwnProperty` checks which fail on the object returned by + // `sb3.serialize` but succeed if that object is "flattened" in this way. + const serializedState = JSON.parse(JSON.stringify(sb3.serialize(runtime1))); return sb3.deserialize(serializedState, runtime2); }); From 884ba19aab57611da74bceeee09c04f98349f7df Mon Sep 17 00:00:00 2001 From: Christopher Willis-Ford Date: Tue, 13 Jun 2017 14:56:06 -0700 Subject: [PATCH 5/5] Add support for older SB3 JSON If the `dataFormat` field is absent, fall back on the older `assetType` field. That can at least help us tell if something is a vector or bitmap image, though it doesn't tell us which kind of bitmap. --- src/serialization/sb3.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/serialization/sb3.js b/src/serialization/sb3.js index 816b1af19..3ffac8ead 100644 --- a/src/serialization/sb3.js +++ b/src/serialization/sb3.js @@ -75,7 +75,11 @@ const parseScratchObject = function (object, runtime) { rotationCenterX: costumeSource.rotationCenterX, rotationCenterY: costumeSource.rotationCenterY }; - const costumeMd5 = `${costumeSource.assetId}.${costumeSource.dataFormat}`; + const dataFormat = + costumeSource.dataFormat || + (costumeSource.assetType && costumeSource.assetType.runtimeFormat) || // older format + 'png'; // if all else fails, guess that it might be a PNG + const costumeMd5 = `${costumeSource.assetId}.${dataFormat}`; return loadCostume(costumeMd5, costume, runtime); }); // Sounds from JSON