Merge pull request #633 from paulkaplan/fix-sound-indexing

Fix sound indexing and add tests

src/blocks/scratch3_sound.js

@@ -135,18 +135,20 @@ class Scratch3SoundBlocks {
             return -1;
         }
 
-        let index;
-
-        // try to convert to a number and use that as an index
-        const num = parseInt(soundName, 10);
-        if (!isNaN(num)) {
-            index = MathUtil.wrapClamp(num, 0, len - 1);
+        // look up by name first
+        const index = this.getSoundIndexByName(soundName, util);
+        if (index !== -1) {
             return index;
         }
 
-        // return the index for the sound of that name
-        index = this.getSoundIndexByName(soundName, util);
-        return index;
+        // then try using the sound name as a 1-indexed index
+        const oneIndexedIndex = parseInt(soundName, 10);
+        if (!isNaN(oneIndexedIndex)) {
+            return MathUtil.wrapClamp(oneIndexedIndex - 1, 0, len - 1);
+        }
+
+        // could not be found as a name or converted to index, return -1
+        return -1;
     }
 
     getSoundIndexByName (soundName, util) {
@@ -181,7 +183,7 @@ class Scratch3SoundBlocks {
         let drum = Cast.toNumber(args.DRUM);
         drum -= 1; // drums are one-indexed
         if (typeof this.runtime.audioEngine === 'undefined') return;
-        drum = MathUtil.wrapClamp(drum, 0, this.runtime.audioEngine.numDrums);
+        drum = MathUtil.wrapClamp(drum, 0, this.runtime.audioEngine.numDrums - 1);
         let beats = Cast.toNumber(args.BEATS);
         beats = this._clampBeats(beats);
         if (util.target.audioPlayer === null) return;
@@ -204,7 +206,7 @@ class Scratch3SoundBlocks {
         let instNum = Cast.toNumber(args.INSTRUMENT);
         instNum -= 1; // instruments are one-indexed
         if (typeof this.runtime.audioEngine === 'undefined') return;
-        instNum = MathUtil.wrapClamp(instNum, 0, this.runtime.audioEngine.numInstruments);
+        instNum = MathUtil.wrapClamp(instNum, 0, this.runtime.audioEngine.numInstruments - 1);
         soundState.currentInstrument = instNum;
         return this.runtime.audioEngine.instrumentPlayer.loadInstrument(soundState.currentInstrument);
     }

test/unit/blocks_sounds.js

@@ -0,0 +1,111 @@
+const test = require('tap').test;
+const Sound = require('../../src/blocks/scratch3_sound');
+let playedSound;
+let playedDrum;
+let playedInstrument;
+const runtime = {
+    audioEngine: {
+        numDrums: 3,
+        numInstruments: 3,
+        instrumentPlayer: {
+            loadInstrument: instrument => (playedInstrument = instrument)
+        }
+    }
+};
+const blocks = new Sound(runtime);
+const util = {
+    target: {
+        sprite: {
+            sounds: [
+                {name: 'first name', md5: 'first md5'},
+                {name: 'second name', md5: 'second md5'},
+                {name: 'third name', md5: 'third md5'},
+                {name: '6', md5: 'fourth md5'}
+            ]
+        },
+        audioPlayer: {
+            playSound: md5 => (playedSound = md5),
+            playDrumForBeats: drum => (playedDrum = drum)
+        }
+    }
+};
+
+test('playSound with a name string works', t => {
+    const args = {SOUND_MENU: 'second name'};
+    blocks.playSound(args, util);
+    t.strictEqual(playedSound, 'second md5');
+    t.end();
+});
+
+test('playSound with a number string works 1-indexed', t => {
+    let args = {SOUND_MENU: '5'};
+    blocks.playSound(args, util);
+    t.strictEqual(playedSound, 'first md5');
+
+    args = {SOUND_MENU: '1'};
+    blocks.playSound(args, util);
+    t.strictEqual(playedSound, 'first md5');
+
+    args = {SOUND_MENU: '0'};
+    blocks.playSound(args, util);
+    t.strictEqual(playedSound, 'fourth md5');
+    t.end();
+});
+
+test('playSound with a number works 1-indexed', t => {
+    let args = {SOUND_MENU: 5};
+    blocks.playSound(args, util);
+    t.strictEqual(playedSound, 'first md5');
+
+    args = {SOUND_MENU: 1};
+    blocks.playSound(args, util);
+    t.strictEqual(playedSound, 'first md5');
+
+    args = {SOUND_MENU: 0};
+    blocks.playSound(args, util);
+    t.strictEqual(playedSound, 'fourth md5');
+    t.end();
+});
+
+test('playSound prioritizes sound index if given a number', t => {
+    const args = {SOUND_MENU: 6};
+    blocks.playSound(args, util);
+    // Ignore the sound named '6', wrapClamp to the second instead
+    t.strictEqual(playedSound, 'second md5');
+    t.end();
+});
+
+test('playSound prioritizes sound name if given a string', t => {
+    const args = {SOUND_MENU: '6'};
+    blocks.playSound(args, util);
+    // Use the sound named '6', which is the fourth
+    t.strictEqual(playedSound, 'fourth md5');
+    t.end();
+});
+
+test('playDrum uses 1-indexing and wrap clamps', t => {
+    let args = {DRUM: 1};
+    blocks.playDrumForBeats(args, util);
+    t.strictEqual(playedDrum, 0);
+
+    args = {DRUM: runtime.audioEngine.numDrums + 1};
+    blocks.playDrumForBeats(args, util);
+    t.strictEqual(playedDrum, 0);
+
+    t.end();
+});
+
+test('setInstrument uses 1-indexing and wrap clamps', t => {
+    // Stub getSoundState
+    blocks._getSoundState = () => ({});
+
+    let args = {INSTRUMENT: 1};
+    blocks.setInstrument(args, util);
+    t.strictEqual(playedInstrument, 0);
+
+    args = {INSTRUMENT: runtime.audioEngine.numInstruments + 1};
+    blocks.setInstrument(args, util);
+    t.strictEqual(playedInstrument, 0);
+
+    t.end();
+});