From 1b6baf8114c4bc7ca1c27f6bd36509a5007bb812 Mon Sep 17 00:00:00 2001 From: Paul Kaplan Date: Thu, 6 Jul 2017 14:28:11 -0400 Subject: [PATCH 1/3] Make playSound use one indexing --- src/blocks/scratch3_sound.js | 20 +++++----- test/unit/blocks_sounds.js | 72 ++++++++++++++++++++++++++++++++++++ 2 files changed, 83 insertions(+), 9 deletions(-) create mode 100644 test/unit/blocks_sounds.js diff --git a/src/blocks/scratch3_sound.js b/src/blocks/scratch3_sound.js index a9361f23..1b28d6bd 100644 --- a/src/blocks/scratch3_sound.js +++ b/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) { diff --git a/test/unit/blocks_sounds.js b/test/unit/blocks_sounds.js new file mode 100644 index 00000000..c616519a --- /dev/null +++ b/test/unit/blocks_sounds.js @@ -0,0 +1,72 @@ +const test = require('tap').test; +const Sound = require('../../src/blocks/scratch3_sound'); +const blocks = new Sound(null); +let playedSound = null; +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) + } + } +}; + +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(); +}); From 2ddb6215fd3c9e34b0447a9853184277b6478a9b Mon Sep 17 00:00:00 2001 From: Paul Kaplan Date: Thu, 6 Jul 2017 14:56:46 -0400 Subject: [PATCH 2/3] Add tests and fixes for set instrument and play drum --- src/blocks/scratch3_sound.js | 4 ++-- test/unit/blocks_sounds.js | 43 +++++++++++++++++++++++++++++++++--- 2 files changed, 42 insertions(+), 5 deletions(-) diff --git a/src/blocks/scratch3_sound.js b/src/blocks/scratch3_sound.js index 1b28d6bd..02347aa4 100644 --- a/src/blocks/scratch3_sound.js +++ b/src/blocks/scratch3_sound.js @@ -183,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; @@ -206,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); } diff --git a/test/unit/blocks_sounds.js b/test/unit/blocks_sounds.js index c616519a..2d031291 100644 --- a/test/unit/blocks_sounds.js +++ b/test/unit/blocks_sounds.js @@ -1,7 +1,16 @@ const test = require('tap').test; const Sound = require('../../src/blocks/scratch3_sound'); -const blocks = new Sound(null); -let playedSound = null; +let playedSound, playedDrum, playedInstrument; +const runtime = { + audioEngine: { + numDrums: 3, + numInstruments: 3, + instrumentPlayer: { + loadInstrument: instrument => (playedInstrument = instrument) + } + } +}; +const blocks = new Sound(runtime); const util = { target: { sprite: { @@ -13,7 +22,8 @@ const util = { ] }, audioPlayer: { - playSound: md5 => (playedSound = md5) + playSound: md5 => (playedSound = md5), + playDrumForBeats: drum => (playedDrum = drum) } } }; @@ -70,3 +80,30 @@ test('playSound prioritizes sound name if given a string', t => { 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(); +}); From 7ec45d9519a35406b8f8f959ac0a4947aa92e963 Mon Sep 17 00:00:00 2001 From: Paul Kaplan Date: Thu, 6 Jul 2017 17:23:25 -0400 Subject: [PATCH 3/3] Satisfy linter --- test/unit/blocks_sounds.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test/unit/blocks_sounds.js b/test/unit/blocks_sounds.js index 2d031291..e6c4bda9 100644 --- a/test/unit/blocks_sounds.js +++ b/test/unit/blocks_sounds.js @@ -1,6 +1,8 @@ const test = require('tap').test; const Sound = require('../../src/blocks/scratch3_sound'); -let playedSound, playedDrum, playedInstrument; +let playedSound; +let playedDrum; +let playedInstrument; const runtime = { audioEngine: { numDrums: 3, @@ -95,7 +97,7 @@ test('playDrum uses 1-indexing and wrap clamps', t => { test('setInstrument uses 1-indexing and wrap clamps', t => { // Stub getSoundState - blocks._getSoundState = () => ({}) + blocks._getSoundState = () => ({}); let args = {INSTRUMENT: 1}; blocks.setInstrument(args, util);