Update storage and update 'loadSound' to handle null asset from storage. Track additional metadata for broken costumes: 'bitmapResolution' and 'dataFormat'.

package-lock.json

@@ -15811,9 +15811,9 @@
       }
     },
     "scratch-storage": {
-      "version": "2.0.0",
-      "resolved": "https://registry.npmjs.org/scratch-storage/-/scratch-storage-2.0.0.tgz",
-      "integrity": "sha512-eLqI5bBWTS1d43BY3zSzJYerBfdwa2l5myLD+IASkGN8eBJtW+/CDsKQC0FtI6xV9Afb7req9eeikHlPYczIuw==",
+      "version": "2.0.1",
+      "resolved": "https://registry.npmjs.org/scratch-storage/-/scratch-storage-2.0.1.tgz",
+      "integrity": "sha512-1Z4sR6jwhpcaFeOY9W5l/u3KKzGKDQcV0WH77OVQ6FFpxXZuKcE/PcXukKnu/PozB/l6QvX2fSADjoXNJ6hbOQ==",
       "dev": true,
       "requires": {
         "arraybuffer-loader": "^1.0.3",

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": "2.0.0",
+    "scratch-storage": "2.0.1",
     "scratch-svg-renderer": "0.2.0-prerelease.20210727023023",
     "script-loader": "0.7.2",
     "stats.js": "0.17.0",

src/import/load-costume.js

@@ -256,6 +256,8 @@ const handleCostumeLoadError = function (costume, runtime) {
     const oldAssetId = costume.assetId;
     const oldRotationX = costume.rotationCenterX;
     const oldRotationY = costume.rotationCenterY;
+    const oldBitmapResolution = costume.bitmapResolution;
+    const oldDataFormat = costume.dataFormat;
 
     const AssetType = runtime.storage.AssetType;
     const isVector = costume.dataFormat === AssetType.ImageVector.runtimeFormat;
@@ -265,7 +267,7 @@ const handleCostumeLoadError = function (costume, runtime) {
         runtime.storage.defaultAssetId.ImageVector :
         runtime.storage.defaultAssetId.ImageBitmap;
     costume.asset = runtime.storage.get(costume.assetId);
-    costume.md5 = `${costume.assetId}.${costume.dataFormat}`;
+    costume.md5 = `${costume.assetId}.${costume.asset.dataFormat}`;
     
     const defaultCostumePromise = (isVector) ?
         loadVector_(costume, runtime) : loadBitmap_(costume, runtime);
@@ -273,13 +275,15 @@ const handleCostumeLoadError = function (costume, runtime) {
     return defaultCostumePromise.then(loadedCostume => {
         loadedCostume.broken = {};
         loadedCostume.broken.assetId = oldAssetId;
-        loadedCostume.broken.md5 = `${oldAssetId}.${costume.dataFormat}`;
+        loadedCostume.broken.md5 = `${oldAssetId}.${oldDataFormat}`;
 
         // Should be null if we got here because the costume was missing
         loadedCostume.broken.asset = oldAsset;
+        loadedCostume.broken.dataFormat = oldDataFormat;
         
         loadedCostume.broken.rotationCenterX = oldRotationX;
         loadedCostume.broken.rotationCenterY = oldRotationY;
+        loadedCostume.broken.bitmapResolution = oldBitmapResolution;
         return loadedCostume;
     });
 };

src/import/load-sound.js

@@ -38,6 +38,40 @@ const loadSoundFromAsset = function (sound, soundAsset, runtime, soundBank) {
     });
 };
 
+// Handle sound loading errors by replacing the runtime sound with the
+// default sound from storage, but keeping track of the original sound metadata
+// in a `broken` field
+const handleSoundLoadError = function (sound, runtime, soundBank) {
+    // Keep track of the old asset information until we're done loading the default sound
+    const oldAsset = sound.asset; // could be null
+    const oldAssetId = sound.assetId;
+    const oldSample = sound.sampleCount;
+    const oldRate = sound.rate;
+    const oldFormat = sound.format;
+    const oldDataFormat = sound.dataFormat;
+                
+    // Use default asset if original fails to load
+    sound.assetId = runtime.storage.defaultAssetId.Sound;
+    sound.asset = runtime.storage.get(sound.assetId);
+    sound.md5 = `${sound.assetId}.${sound.asset.dataFormat}`;
+
+    return loadSoundFromAsset(sound, sound.asset, runtime, soundBank).then(loadedSound => {
+        loadedSound.broken = {};
+        loadedSound.broken.assetId = oldAssetId;
+        loadedSound.broken.md5 = `${oldAssetId}.${oldDataFormat}`;
+
+        // Should be null if we got here because the sound was missing
+        loadedSound.broken.asset = oldAsset;
+        
+        loadedSound.broken.sampleCount = oldSample;
+        loadedSound.broken.rate = oldRate;
+        loadedSound.broken.format = oldFormat;
+        loadedSound.broken.dataFormat = oldDataFormat;
+        
+        return loadedSound;
+    });
+};
+
 /**
  * Load a sound's asset into memory asynchronously.
  * @param {!object} sound - the Scratch sound object.
@@ -60,13 +94,13 @@ 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;
+
+        if (!soundAsset) {
+            log.warn('Failed to find sound data: ', sound.md5);
+            return handleSoundLoadError(sound, runtime, soundBank);
+        }
+
         return loadSoundFromAsset(sound, soundAsset, runtime, soundBank);
     });
 };

src/serialization/sb3.js

@@ -346,29 +346,24 @@ const serializeBlocks = function (blocks) {
 const serializeCostume = function (costume) {
     const obj = Object.create(null);
     obj.name = costume.name;
-    obj.bitmapResolution = costume.bitmapResolution;
-    obj.dataFormat = costume.dataFormat.toLowerCase();
-    if (costume.broken) {
-        obj.assetId = costume.broken.assetId;
-        
-        // serialize this property with the name 'md5ext' because that's
-        // what it's actually referring to. TODO runtime objects need to be
-        // updated to actually refer to this as 'md5ext' instead of 'md5'
-        // but that change should be made carefully since it is very
-        // pervasive
-        obj.md5ext = (costume.broken.md5);
-        
-        obj.rotationCenterX = costume.broken.rotationCenterX;
-        obj.rotationCenterY = costume.broken.rotationCenterY;
-    } else {
-        obj.assetId = costume.assetId;
-        
-        // See related comment above
-        obj.md5ext = costume.md5;
-        
-        obj.rotationCenterX = costume.rotationCenterX;
-        obj.rotationCenterY = costume.rotationCenterY;
-    }
+
+    const costumeToSerialize = costume.broken || costume;
+
+    obj.bitmapResolution = costumeToSerialize.bitmapResolution;
+    obj.dataFormat = costumeToSerialize.dataFormat.toLowerCase();
+    
+    obj.assetId = costumeToSerialize.assetId;
+    
+    // serialize this property with the name 'md5ext' because that's
+    // what it's actually referring to. TODO runtime objects need to be
+    // updated to actually refer to this as 'md5ext' instead of 'md5'
+    // but that change should be made carefully since it is very
+    // pervasive
+    obj.md5ext = costumeToSerialize.md5;
+    
+    obj.rotationCenterX = costumeToSerialize.rotationCenterX;
+    obj.rotationCenterY = costumeToSerialize.rotationCenterY;
+
     return obj;
 };
 
@@ -379,18 +374,21 @@ const serializeCostume = function (costume) {
  */
 const serializeSound = function (sound) {
     const obj = Object.create(null);
-    obj.assetId = sound.assetId;
     obj.name = sound.name;
-    obj.dataFormat = sound.dataFormat.toLowerCase();
-    obj.format = sound.format;
-    obj.rate = sound.rate;
-    obj.sampleCount = sound.sampleCount;
+    
+    const soundToSerialize = sound.broken || sound;
+
+    obj.assetId = soundToSerialize.assetId;
+    obj.dataFormat = soundToSerialize.dataFormat.toLowerCase();
+    obj.format = soundToSerialize.format;
+    obj.rate = soundToSerialize.rate;
+    obj.sampleCount = soundToSerialize.sampleCount;
     // serialize this property with the name 'md5ext' because that's
     // what it's actually referring to. TODO runtime objects need to be
     // updated to actually refer to this as 'md5ext' instead of 'md5'
     // but that change should be made carefully since it is very
     // pervasive
-    obj.md5ext = sound.md5;
+    obj.md5ext = soundToSerialize.md5;
     return obj;
 };
 

src/virtual-machine.js

@@ -366,9 +366,11 @@ class VirtualMachine extends EventEmitter {
         const vm = this;
         const promise = storage.load(storage.AssetType.Project, id);
         promise.then(projectAsset => {
-            if (projectAsset) {
-                return vm.loadProject(projectAsset.data);
+            if (!projectAsset) {
+                log.error("Failed to fetch project with id: ", id);
+                return null;
             }
+            return vm.loadProject(projectAsset.data);
         });
     }
 

test/integration/sb3_missing_sound.js

@@ -0,0 +1,87 @@
+/**
+ * This test ensures that the VM gracefully handles an sb3 project with
+ * a missing sound. The project should load without error.
+ * TODO: handle missing or corrupted sounds by replacing the missing sound data
+ * with the empty sound file but keeping the info about the original missing / corrupted sound
+ * so that user data does not get overwritten / lost.
+ */
+const path = require('path');
+const tap = require('tap');
+const makeTestStorage = require('../fixtures/make-test-storage');
+const readFileToBuffer = require('../fixtures/readProjectFile').readFileToBuffer;
+const VirtualMachine = require('../../src/index');
+const {serializeSounds} = require('../../src/serialization/serialize-assets');
+
+const projectUri = path.resolve(__dirname, '../fixtures/missing_sound.sb3');
+const project = readFileToBuffer(projectUri);
+
+const missingSoundAssetId = '78618aadd225b1db7bf837fa17dc0568';
+
+let vm;
+
+tap.beforeEach(() => {
+    const storage = makeTestStorage();
+
+    vm = new VirtualMachine();
+    vm.attachStorage(storage);
+
+    return vm.loadProject(project);
+});
+
+const test = tap.test;
+
+test('loading sb3 project with missing sound file', t => {
+    t.equal(vm.runtime.targets.length, 2);
+    
+    const stage = vm.runtime.targets[0];
+    t.ok(stage.isStage);
+
+    const catSprite = vm.runtime.targets[1];
+    t.equal(catSprite.getSounds().length, 1);
+    
+    const missingSound = catSprite.getSounds()[0];
+    t.equal(missingSound.name, 'Boop Sound Recording');
+    // Sound should have original data but no asset
+    const defaultSoundAssetId = vm.runtime.storage.defaultAssetId.Sound;
+    t.equal(missingSound.assetId, defaultSoundAssetId);
+    t.equal(missingSound.dataFormat, 'wav');
+
+    // Runtime should have info about broken asset
+    t.ok(missingSound.broken);
+    t.equal(missingSound.broken.assetId, missingSoundAssetId);
+
+    t.end();
+});
+
+test('load and then save sb3 project with missing sound file', t => {
+    const resavedProject = JSON.parse(vm.toJSON());
+
+    t.equal(resavedProject.targets.length, 2);
+    
+    const stage = resavedProject.targets[0];
+    t.ok(stage.isStage);
+
+    const catSprite = resavedProject.targets[1];
+    t.equal(catSprite.name, 'Sprite1');
+    t.equal(catSprite.sounds.length, 1);
+    
+    const missingSound = catSprite.sounds[0];
+    t.equal(missingSound.name, 'Boop Sound Recording');
+    // Costume should have both default sound data (e.g. "Gray Question Sound" ^_^) and original data
+    t.equal(missingSound.assetId, missingSoundAssetId);
+    t.equal(missingSound.dataFormat, 'wav');
+    // Test that we didn't save any data about the costume being broken
+    t.notOk(missingSound.broken);
+
+    t.end();
+});
+
+test('serializeCostume does not save data for missing costume', t => {
+    const soundDescs = serializeSounds(vm.runtime);
+    
+    t.equal(soundDescs.length, 1); // Should only have one sound, the pop sound for the stage
+    t.not(soundDescs[0].fileName, `${missingSoundAssetId}.wav`);
+
+    t.end();
+    process.nextTick(process.exit);
+});