From 11f938f8a9368cbb0a827950aa9fbf86763530ae Mon Sep 17 00:00:00 2001 From: Karishma Chadha <kchadha@scratch.mit.edu> Date: Wed, 18 May 2022 16:55:46 -0400 Subject: [PATCH] Update to latest version of storage which fixes issue where an HTML 404 page was being returned for missing asset data. Update VM to handle null assets properly. --- package-lock.json | 32 ++++++++++++++++++++++--- package.json | 2 +- src/import/load-costume.js | 6 ++--- src/import/load-sound.js | 10 ++++++-- src/serialization/deserialize-assets.js | 4 ++-- src/virtual-machine.js | 4 +++- test/integration/sb3_missing_svg.js | 9 +------ test/integration/sprite3_missing_svg.js | 8 +------ 8 files changed, 48 insertions(+), 27 deletions(-) diff --git a/package-lock.json b/package-lock.json index 2ddc3c957..10e6f2e05 100644 --- a/package-lock.json +++ b/package-lock.json @@ -15763,6 +15763,32 @@ "scratch-storage": "^1.0.0", "scratch-svg-renderer": "0.2.0-prerelease.20210727023023", "twgl.js": "4.4.0" + }, + "dependencies": { + "scratch-storage": { + "version": "1.3.6", + "resolved": "https://registry.npmjs.org/scratch-storage/-/scratch-storage-1.3.6.tgz", + "integrity": "sha512-L/7z7SB7cGANsgjyiE+qZNaPEqFHK1yPbNomizkgN3WHGcKRogLvmheR57kOxHNpQzodUTbG+pVVH6fR2ZY1Sg==", + "dev": true, + "requires": { + "arraybuffer-loader": "^1.0.3", + "base64-js": "1.3.0", + "fastestsmallesttextencoderdecoder": "^1.0.7", + "js-md5": "0.7.3", + "minilog": "3.1.0", + "worker-loader": "^2.0.0" + } + }, + "worker-loader": { + "version": "2.0.0", + "resolved": "https://registry.npmjs.org/worker-loader/-/worker-loader-2.0.0.tgz", + "integrity": "sha512-tnvNp4K3KQOpfRnD20m8xltE3eWh89Ye+5oj7wXEEHKac1P4oZ6p9oTj8/8ExqoSBnk9nu5Pr4nKfQ1hn2APJw==", + "dev": true, + "requires": { + "loader-utils": "^1.0.0", + "schema-utils": "^0.4.0" + } + } } }, "scratch-render-fonts": { @@ -15785,9 +15811,9 @@ } }, "scratch-storage": { - "version": "1.3.6", - "resolved": "https://registry.npmjs.org/scratch-storage/-/scratch-storage-1.3.6.tgz", - "integrity": "sha512-L/7z7SB7cGANsgjyiE+qZNaPEqFHK1yPbNomizkgN3WHGcKRogLvmheR57kOxHNpQzodUTbG+pVVH6fR2ZY1Sg==", + "version": "2.0.0", + "resolved": "https://registry.npmjs.org/scratch-storage/-/scratch-storage-2.0.0.tgz", + "integrity": "sha512-eLqI5bBWTS1d43BY3zSzJYerBfdwa2l5myLD+IASkGN8eBJtW+/CDsKQC0FtI6xV9Afb7req9eeikHlPYczIuw==", "dev": true, "requires": { "arraybuffer-loader": "^1.0.3", diff --git a/package.json b/package.json index 68d1ba16d..6fb90f499 100644 --- a/package.json +++ b/package.json @@ -76,7 +76,7 @@ "scratch-l10n": "3.14.20220510031559", "scratch-render": "0.1.0-prerelease.20211028200436", "scratch-render-fonts": "1.0.0-prerelease.20210401210003", - "scratch-storage": "1.3.6", + "scratch-storage": "2.0.0", "scratch-svg-renderer": "0.2.0-prerelease.20210727023023", "script-loader": "0.7.2", "stats.js": "0.17.0", diff --git a/src/import/load-costume.js b/src/import/load-costume.js index 0bf6ba653..6c1db499d 100644 --- a/src/import/load-costume.js +++ b/src/import/load-costume.js @@ -302,7 +302,7 @@ const loadCostumeFromAsset = function (costume, runtime, optVersion) { costume.assetId = costume.asset.assetId; const renderer = runtime.renderer; if (!renderer) { - log.error('No rendering module present; cannot load costume: ', costume.name); + log.warn('No rendering module present; cannot load costume: ', costume.name); return Promise.resolve(costume); } const AssetType = runtime.storage.AssetType; @@ -352,12 +352,12 @@ const loadCostume = function (md5ext, costume, runtime, optVersion) { // Need to load the costume from storage. The server should have a reference to this md5. if (!runtime.storage) { - log.error('No storage module present; cannot load costume asset: ', md5ext); + log.warn('No storage module present; cannot load costume asset: ', md5ext); return Promise.resolve(costume); } if (!runtime.storage.defaultAssetId) { - log.error(`No default assets found`); + log.warn(`No default assets found`); return Promise.resolve(costume); } diff --git a/src/import/load-sound.js b/src/import/load-sound.js index 639fcc2af..400251f80 100644 --- a/src/import/load-sound.js +++ b/src/import/load-sound.js @@ -14,7 +14,7 @@ const log = require('../util/log'); const loadSoundFromAsset = function (sound, soundAsset, runtime, soundBank) { sound.assetId = soundAsset.assetId; if (!runtime.audioEngine) { - log.error('No audio engine present; cannot load sound asset: ', sound.md5); + log.warn('No audio engine present; cannot load sound asset: ', sound.md5); return Promise.resolve(sound); } return runtime.audioEngine.decodeSoundPlayer(Object.assign( @@ -49,7 +49,7 @@ const loadSoundFromAsset = function (sound, soundAsset, runtime, soundBank) { */ const loadSound = function (sound, runtime, soundBank) { if (!runtime.storage) { - log.error('No storage module present; cannot load sound asset: ', sound.md5); + log.warn('No storage module present; cannot load sound asset: ', sound.md5); return Promise.resolve(sound); } const idParts = StringUtil.splitFirst(sound.md5, '.'); @@ -60,6 +60,12 @@ const loadSound = function (sound, runtime, soundBank) { (sound.asset && Promise.resolve(sound.asset)) || runtime.storage.load(runtime.storage.AssetType.Sound, md5, ext) ).then(soundAsset => { + if (!soundAsset) { + log.warn('Failed to find sound data: ', sound); + // TODO add missing sound error handling that adds the "gray question sound" + return sound; + } + sound.asset = soundAsset; return loadSoundFromAsset(sound, soundAsset, runtime, soundBank); }); diff --git a/src/serialization/deserialize-assets.js b/src/serialization/deserialize-assets.js index bf59f915f..568614c3d 100644 --- a/src/serialization/deserialize-assets.js +++ b/src/serialization/deserialize-assets.js @@ -18,7 +18,7 @@ const deserializeSound = function (sound, runtime, zip, assetFileName) { const fileName = assetFileName ? assetFileName : sound.md5; const storage = runtime.storage; if (!storage) { - log.error('No storage module present; cannot load sound asset: ', fileName); + log.warn('No storage module present; cannot load sound asset: ', fileName); return Promise.resolve(null); } @@ -81,7 +81,7 @@ const deserializeCostume = function (costume, runtime, zip, assetFileName, textL `${assetId}.${costume.dataFormat}`; if (!storage) { - log.error('No storage module present; cannot load costume asset: ', fileName); + log.warn('No storage module present; cannot load costume asset: ', fileName); return Promise.resolve(null); } diff --git a/src/virtual-machine.js b/src/virtual-machine.js index 4d24aa9e1..66a9c0fab 100644 --- a/src/virtual-machine.js +++ b/src/virtual-machine.js @@ -366,7 +366,9 @@ class VirtualMachine extends EventEmitter { const vm = this; const promise = storage.load(storage.AssetType.Project, id); promise.then(projectAsset => { - vm.loadProject(projectAsset.data); + if (projectAsset) { + return vm.loadProject(projectAsset.data); + } }); } diff --git a/test/integration/sb3_missing_svg.js b/test/integration/sb3_missing_svg.js index 150f97469..523f3459e 100644 --- a/test/integration/sb3_missing_svg.js +++ b/test/integration/sb3_missing_svg.js @@ -23,13 +23,6 @@ let vm; tap.beforeEach(() => { const storage = makeTestStorage(); - - // This line removes the webhelper from the list of available helpers. - // W/o the following line, this fails because storage doesn't handle the case - // where none of the tools have isGetSupported: true - // TODO: Remove this line when the related storage bug is resolved so that - // storage gracefully handles non-browser situations where assets are missing. - storage._helpers = [storage._helpers[0]]; vm = new VirtualMachine(); vm.attachStorage(storage); @@ -88,8 +81,8 @@ test('load and then save sb3 project with missing costume file', t => { test('serializeCostume does not save data for missing costume', t => { const costumeDescs = serializeCostumes(vm.runtime); + t.equal(costumeDescs.length, 1); // Should only have one costume, the backdrop - t.not(costumeDescs[0].fileName, `${missingCostumeAssetId}.svg`); t.end(); diff --git a/test/integration/sprite3_missing_svg.js b/test/integration/sprite3_missing_svg.js index 1df90dacc..6bd0985f8 100644 --- a/test/integration/sprite3_missing_svg.js +++ b/test/integration/sprite3_missing_svg.js @@ -27,13 +27,6 @@ let vm; tap.beforeEach(() => { const storage = makeTestStorage(); - - // This line removes the webhelper from the list of available helpers. - // W/o the following line, this fails because storage doesn't handle the case - // where none of the tools have isGetSupported: true - // TODO: Remove this line when the related storage bug is resolved so that - // storage gracefully handles non-browser situations where assets are missing. - storage._helpers = [storage._helpers[0]]; vm = new VirtualMachine(); vm.attachStorage(storage); @@ -86,6 +79,7 @@ test('load and then save sprite3 with missing vector costume file', t => { test('serializeCostume does not save data for missing costume', t => { const costumeDescs = serializeCostumes(vm.runtime, vm.runtime.targets[2].id); + t.equal(costumeDescs.length, 0); t.end();