From 28dae39ddd7034bbdd4f27a34962deacc01b990e Mon Sep 17 00:00:00 2001 From: Ray Schamp Date: Fri, 12 May 2017 19:48:03 -0400 Subject: [PATCH 01/21] Use stages instead of travis-after-all Also, parallelize tap tests --- .travis.yml | 49 +++++++++++++++++++++++++------------------------ package.json | 3 ++- 2 files changed, 27 insertions(+), 25 deletions(-) diff --git a/.travis.yml b/.travis.yml index 29c45a807..0e68449f0 100644 --- a/.travis.yml +++ b/.travis.yml @@ -2,6 +2,9 @@ language: node_js node_js: - 6 - node +env: + - NPM_SCRIPT=tap:unit -- --jobs=4 + - NPM_SCRIPT=tap:integration -- --jobs=4 sudo: false cache: directories: @@ -9,27 +12,25 @@ cache: install: - npm install - npm update -after_script: -- | - # RELEASE_BRANCHES and NPM_TOKEN defined in Travis settings panel - declare exitCode - $(npm bin)/travis-after-all - exitCode=$? - if [[ - # Execute after all jobs finish successfully - $exitCode = 0 && - # Only release on release branches - $RELEASE_BRANCHES =~ $TRAVIS_BRANCH && - # Don't release on PR builds - $TRAVIS_PULL_REQUEST = "false" - ]]; then - # Authenticate NPM - echo "//registry.npmjs.org/:_authToken=\${NPM_TOKEN}" > .npmrc - # Set version to timestamp - npm --no-git-tag-version version $($(npm bin)/json -f package.json version)-prerelease.$(date +%s) - npm publish - # Publish to gh-pages as most recent committer - git config --global user.email $(git log --pretty=format:"%ae" -n1) - git config --global user.name $(git log --pretty=format:"%an" -n1) - npm run --silent deploy -- -x -r $GH_PAGES_REPO - fi +script: npm run $NPM_SCRIPT +jobs: + include: + - env: NPM_SCRIPT=lint + node_js: 6 + - stage: release + node_js: 6 + script: true + before_deploy: npm --no-git-tag-version version $($(npm bin)/json -f package.json version)-prerelease.$(date +%s) + deploy: + - provider: script + on: + condition: $RELEASE_BRANCHES =~ $TRAVIS_BRANCH + skip_cleanup: true + script: + - git config --global user.email $(git log --pretty=format:"%ae" -n1) + - git config --global user.name $(git log --pretty=format:"%an" -n1) + - npm run --silent deploy -- -x -r $GH_PAGES_REPO + - provider: npm + on: + condition: $RELEASE_BRANCHES =~ $TRAVIS_BRANCH + api_key: $NPM_TOKEN diff --git a/package.json b/package.json index d51b27e2e..fe7347371 100644 --- a/package.json +++ b/package.json @@ -19,6 +19,8 @@ "prepublish-watch": "npm run watch", "start": "./node_modules/.bin/webpack-dev-server", "tap": "./node_modules/.bin/tap ./test/{unit,integration}/*.js", + "tap:unit": "./node_modules/.bin/tap ./test/unit/*.js", + "tap:integration": "./node_modules/.bin/tap ./test/integration/*.js", "test": "npm run lint && npm run tap", "watch": "./node_modules/.bin/webpack --progress --colors --watch", "version": "./node_modules/.bin/json -f package.json -I -e \"this.repository.sha = '$(git log -n1 --pretty=format:%H)'\"" @@ -50,7 +52,6 @@ "socket.io-client": "1.7.3", "stats.js": "^0.17.0", "tap": "^10.2.0", - "travis-after-all": "^1.4.4", "webpack": "^2.4.1", "webpack-dev-server": "^2.4.1" } From 3431fb39c8461e5bffc23b67f9a1c3278b65d6df Mon Sep 17 00:00:00 2001 From: Ray Schamp Date: Sat, 13 May 2017 20:25:34 -0400 Subject: [PATCH 02/21] Don't run prepublish on install --- package.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/package.json b/package.json index fe7347371..d20b86728 100644 --- a/package.json +++ b/package.json @@ -15,8 +15,7 @@ "coverage": "./node_modules/.bin/tap ./test/{unit,integration}/*.js --coverage --coverage-report=lcov", "deploy": "touch playground/.nojekyll && ./node_modules/.bin/gh-pages -t -d playground -m \"Build for $(git log --pretty=format:%H -n1)\"", "lint": "./node_modules/.bin/eslint .", - "prepublish": "npm run build", - "prepublish-watch": "npm run watch", + "prepublish": "in-publish && npm run build || not-in-publish", "start": "./node_modules/.bin/webpack-dev-server", "tap": "./node_modules/.bin/tap ./test/{unit,integration}/*.js", "tap:unit": "./node_modules/.bin/tap ./test/unit/*.js", @@ -40,6 +39,7 @@ "highlightjs": "^9.8.0", "htmlparser2": "3.9.2", "immutable": "3.8.1", + "in-publish": "^2.0.0", "json": "^9.0.4", "lodash.defaultsdeep": "4.6.0", "minilog": "3.1.0", From 3f53c3fd5c7d3a93146d9cecfcaa7661c4735e40 Mon Sep 17 00:00:00 2001 From: Ray Schamp Date: Sun, 14 May 2017 15:48:21 -0400 Subject: [PATCH 03/21] Allow condition alone to control release --- .travis.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.travis.yml b/.travis.yml index 0e68449f0..e4be6aad9 100644 --- a/.travis.yml +++ b/.travis.yml @@ -24,6 +24,7 @@ jobs: deploy: - provider: script on: + all_branches: true condition: $RELEASE_BRANCHES =~ $TRAVIS_BRANCH skip_cleanup: true script: @@ -32,5 +33,6 @@ jobs: - npm run --silent deploy -- -x -r $GH_PAGES_REPO - provider: npm on: + all_branches: true condition: $RELEASE_BRANCHES =~ $TRAVIS_BRANCH api_key: $NPM_TOKEN From e173a0ae2cc59884831b9a19c8241ef6ea1c9720 Mon Sep 17 00:00:00 2001 From: Ray Schamp Date: Sun, 14 May 2017 15:56:06 -0400 Subject: [PATCH 04/21] Fix NPM_SCRIPT vars --- .travis.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index e4be6aad9..fab21efd1 100644 --- a/.travis.yml +++ b/.travis.yml @@ -3,8 +3,8 @@ node_js: - 6 - node env: - - NPM_SCRIPT=tap:unit -- --jobs=4 - - NPM_SCRIPT=tap:integration -- --jobs=4 + - NPM_SCRIPT="tap:unit -- --jobs=4" + - NPM_SCRIPT="tap:integration -- --jobs=4" sudo: false cache: directories: From 1b6baf8114c4bc7ca1c27f6bd36509a5007bb812 Mon Sep 17 00:00:00 2001 From: Paul Kaplan Date: Thu, 6 Jul 2017 14:28:11 -0400 Subject: [PATCH 05/21] 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 a9361f230..1b28d6bd3 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 000000000..c616519ae --- /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 06/21] 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 1b28d6bd3..02347aa4c 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 c616519ae..2d0312913 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 07/21] 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 2d0312913..e6c4bda91 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); From a83c0808a04ae050e2dfc13b1bd3e702a4f82edf Mon Sep 17 00:00:00 2001 From: Paul Kaplan Date: Fri, 7 Jul 2017 08:19:32 -0400 Subject: [PATCH 08/21] Add public API for costume and sound renaming with tests --- src/virtual-machine.js | 26 ++++++++++++++++++++++++ test/unit/virtual-machine.js | 38 ++++++++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+) diff --git a/src/virtual-machine.js b/src/virtual-machine.js index 2ca4e21d8..0cc3b7bb6 100644 --- a/src/virtual-machine.js +++ b/src/virtual-machine.js @@ -287,6 +287,19 @@ class VirtualMachine extends EventEmitter { }); } + /** + * Rename a costume on the current editing target. + * @param {int} costumeIndex - the index of the costume to be renamed. + * @param {string} newName - the desired new name of the costume (will be modified if already in use). + */ + renameCostume (costumeIndex, newName) { + const usedNames = this.editingTarget.sprite.costumes + .filter((costume, index) => costumeIndex !== index) + .map(costume => costume.name); + this.editingTarget.sprite.costumes[costumeIndex].name = StringUtil.unusedName(newName, usedNames); + this.emitTargetsUpdate(); + } + /** * Delete a costume from the current editing target. * @param {int} costumeIndex - the index of the costume to be removed. @@ -307,6 +320,19 @@ class VirtualMachine extends EventEmitter { }); } + /** + * Rename a sound on the current editing target. + * @param {int} soundIndex - the index of the sound to be renamed. + * @param {string} newName - the desired new name of the sound (will be modified if already in use). + */ + renameSound (soundIndex, newName) { + const usedNames = this.editingTarget.sprite.sounds + .filter((sound, index) => soundIndex !== index) + .map(sound => sound.name); + this.editingTarget.sprite.sounds[soundIndex].name = StringUtil.unusedName(newName, usedNames); + this.emitTargetsUpdate(); + } + /** * Delete a sound from the current editing target. * @param {int} soundIndex - the index of the sound to be removed. diff --git a/test/unit/virtual-machine.js b/test/unit/virtual-machine.js index 37a62ff69..9361ab918 100644 --- a/test/unit/virtual-machine.js +++ b/test/unit/virtual-machine.js @@ -109,3 +109,41 @@ test('renameSprite does not increment when renaming to the same name', t => { t.equal(vm.runtime.targets[0].sprite.name, 'this name'); t.end(); }); + +test('renameSound sets the sound name', t => { + const vm = new VirtualMachine(); + vm.editingTarget = { + sprite: { + sounds: [{name: 'first'}, {name: 'second'}] + } + }; + vm.renameSound(0, 'hello'); + t.equal(vm.editingTarget.sprite.sounds[0].name, 'hello'); + t.equal(vm.editingTarget.sprite.sounds[1].name, 'second'); + // Make sure renaming to same name doesn't increment + vm.renameSound(0, 'hello'); + t.equal(vm.editingTarget.sprite.sounds[0].name, 'hello'); + // But renaming to used name does increment + vm.renameSound(1, 'hello'); + t.equal(vm.editingTarget.sprite.sounds[1].name, 'hello2'); + t.end(); +}); + +test('renameCostume sets the costume name', t => { + const vm = new VirtualMachine(); + vm.editingTarget = { + sprite: { + costumes: [{name: 'first'}, {name: 'second'}] + } + }; + vm.renameCostume(0, 'hello'); + t.equal(vm.editingTarget.sprite.costumes[0].name, 'hello'); + t.equal(vm.editingTarget.sprite.costumes[1].name, 'second'); + // Make sure renaming to same name doesn't increment + vm.renameCostume(0, 'hello'); + t.equal(vm.editingTarget.sprite.costumes[0].name, 'hello'); + // But renaming to used name does increment + vm.renameCostume(1, 'hello'); + t.equal(vm.editingTarget.sprite.costumes[1].name, 'hello2'); + t.end(); +}); From 7f1f9079e36d92d061f23b5d38edbc44b6a1f85a Mon Sep 17 00:00:00 2001 From: DD Liu Date: Mon, 10 Jul 2017 18:29:25 -0400 Subject: [PATCH 09/21] support rename variable --- src/engine/blocks.js | 2 +- src/engine/target.js | 13 +++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/src/engine/blocks.js b/src/engine/blocks.js index 93b865770..30558d267 100644 --- a/src/engine/blocks.js +++ b/src/engine/blocks.js @@ -298,7 +298,7 @@ class Blocks { if (!block.fields[args.name]) return; if (args.name === 'VARIABLE') { // Get variable name using the id in args.value. - const variable = optRuntime.getEditingTarget().lookupVariableById(args.value); + const variable = optRuntime.getEditingTarget().lookupVariableById(args.id); if (variable) { block.fields[args.name].value = variable.name; block.fields[args.name].id = args.value; diff --git a/src/engine/target.js b/src/engine/target.js index da00074b8..92bd8296d 100644 --- a/src/engine/target.js +++ b/src/engine/target.js @@ -4,6 +4,7 @@ const Blocks = require('./blocks'); const Variable = require('../engine/variable'); const List = require('../engine/list'); const uid = require('../util/uid'); +const {Map} = require('immutable'); /** * @fileoverview @@ -161,6 +162,17 @@ class Target extends EventEmitter { const variable = this.variables[id]; if (variable.id === id) { variable.name = newName; + const blocks = this.runtime.monitorBlocks; + blocks.changeBlock({ + id: id, + element: 'field', + name: 'VARIABLE', + value: newName + }, this.runtime); + this.runtime.requestUpdateMonitor(Map({ + id: id, + params: blocks._getBlockParams(blocks.getBlock(variable.id)) + })); } } } @@ -172,6 +184,7 @@ class Target extends EventEmitter { deleteVariable (id) { if (this.variables.hasOwnProperty(id)) { delete this.variables[id]; + this.runtime.requestRemoveMonitor(id); } } From 4b1307bb11bc1da9f06f9a7943ccfcef44bb937d Mon Sep 17 00:00:00 2001 From: DD Liu Date: Tue, 11 Jul 2017 11:51:47 -0400 Subject: [PATCH 10/21] fix test --- src/engine/target.js | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/src/engine/target.js b/src/engine/target.js index 92bd8296d..9d135687e 100644 --- a/src/engine/target.js +++ b/src/engine/target.js @@ -162,17 +162,21 @@ class Target extends EventEmitter { const variable = this.variables[id]; if (variable.id === id) { variable.name = newName; - const blocks = this.runtime.monitorBlocks; - blocks.changeBlock({ - id: id, - element: 'field', - name: 'VARIABLE', - value: newName - }, this.runtime); - this.runtime.requestUpdateMonitor(Map({ - id: id, - params: blocks._getBlockParams(blocks.getBlock(variable.id)) - })); + + if (this.runtime) { + const blocks = this.runtime.monitorBlocks; + blocks.changeBlock({ + id: id, + element: 'field', + name: 'VARIABLE', + value: newName + }, this.runtime); + this.runtime.requestUpdateMonitor(Map({ + id: id, + params: blocks._getBlockParams(blocks.getBlock(variable.id)) + })); + } + } } } @@ -184,7 +188,9 @@ class Target extends EventEmitter { deleteVariable (id) { if (this.variables.hasOwnProperty(id)) { delete this.variables[id]; - this.runtime.requestRemoveMonitor(id); + if (this.runtime) { + this.runtime.requestRemoveMonitor(id); + } } } From 01db5da8ce0c6fc7cc516b4e79e695b8772dea00 Mon Sep 17 00:00:00 2001 From: DD Liu Date: Tue, 11 Jul 2017 13:08:18 -0400 Subject: [PATCH 11/21] fix value --- src/engine/blocks.js | 2 +- src/engine/target.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/engine/blocks.js b/src/engine/blocks.js index 30558d267..93b865770 100644 --- a/src/engine/blocks.js +++ b/src/engine/blocks.js @@ -298,7 +298,7 @@ class Blocks { if (!block.fields[args.name]) return; if (args.name === 'VARIABLE') { // Get variable name using the id in args.value. - const variable = optRuntime.getEditingTarget().lookupVariableById(args.id); + const variable = optRuntime.getEditingTarget().lookupVariableById(args.value); if (variable) { block.fields[args.name].value = variable.name; block.fields[args.name].id = args.value; diff --git a/src/engine/target.js b/src/engine/target.js index 9d135687e..2a6be81a6 100644 --- a/src/engine/target.js +++ b/src/engine/target.js @@ -169,7 +169,7 @@ class Target extends EventEmitter { id: id, element: 'field', name: 'VARIABLE', - value: newName + value: id }, this.runtime); this.runtime.requestUpdateMonitor(Map({ id: id, From bd3a29650b73f3c668342c3c69dd14a5d7e58688 Mon Sep 17 00:00:00 2001 From: Paul Kaplan Date: Mon, 17 Jul 2017 12:02:48 -0400 Subject: [PATCH 12/21] Fix variable import scoping --- src/serialization/sb2.js | 61 ++++++++++++++++++++++++++------- src/virtual-machine.js | 7 ++-- test/fixtures/data.sb2 | Bin 55147 -> 55296 bytes test/unit/serialization_sb2.js | 17 +++++++++ test/unit/virtual-machine.js | 39 +++++++++++++++++++++ 5 files changed, 110 insertions(+), 14 deletions(-) diff --git a/src/serialization/sb2.js b/src/serialization/sb2.js index 2946756de..48e89f448 100644 --- a/src/serialization/sb2.js +++ b/src/serialization/sb2.js @@ -77,15 +77,16 @@ const flatten = function (blocks) { * a list of blocks in a branch (e.g., in forever), * or a list of blocks in an argument (e.g., move [pick random...]). * @param {Array.} blockList SB2 JSON-format block list. + * @param {Function} getVariableId function to retreive a variable's ID based on name * @return {Array.} Scratch VM-format block list. */ -const parseBlockList = function (blockList) { +const parseBlockList = function (blockList, getVariableId) { const resultingList = []; let previousBlock = null; // For setting next. for (let i = 0; i < blockList.length; i++) { const block = blockList[i]; // eslint-disable-next-line no-use-before-define - const parsedBlock = parseBlock(block); + const parsedBlock = parseBlock(block, getVariableId); if (typeof parsedBlock === 'undefined') continue; if (previousBlock) { parsedBlock.parent = previousBlock.id; @@ -102,14 +103,15 @@ const parseBlockList = function (blockList) { * This should only handle top-level scripts that include X, Y coordinates. * @param {!object} scripts Scripts object from SB2 JSON. * @param {!Blocks} blocks Blocks object to load parsed blocks into. + * @param {Function} getVariableId function to retreive a variable's ID based on name */ -const parseScripts = function (scripts, blocks) { +const parseScripts = function (scripts, blocks, getVariableId) { for (let i = 0; i < scripts.length; i++) { const script = scripts[i]; const scriptX = script[0]; const scriptY = script[1]; const blockList = script[2]; - const parsedBlockList = parseBlockList(blockList); + const parsedBlockList = parseBlockList(blockList, getVariableId); if (parsedBlockList[0]) { // Adjust script coordinates to account for // larger block size in scratch-blocks. @@ -127,6 +129,30 @@ const parseScripts = function (scripts, blocks) { } }; +/** + * Create a callback for assigning fixed IDs to imported variables + * Generator stores the global variable mapping in a closure + * @param {!string} targetId the id of the target to scope the variable to + * @return {string} variable ID + */ +const generateVariableIdGetter = (function () { + let globalVariableNameMap = {}; + const namer = (targetId, name) => `${targetId}-${name}`; + return function (targetId, topLevel) { + // Reset the global variable map if topLevel + if (topLevel) globalVariableNameMap = {}; + return function (name) { + if (topLevel) { // Store the name/id pair in the globalVariableNameMap + globalVariableNameMap[name] = namer(targetId, name); + return globalVariableNameMap[name]; + } + // Not top-level, so first check the global name map + if (globalVariableNameMap[name]) return globalVariableNameMap[name]; + return namer(targetId, name); + }; + }; +}()); + /** * Parse a single "Scratch object" and create all its in-memory VM objects. * @param {!object} object From-JSON "Scratch object:" sprite, stage, watcher. @@ -180,19 +206,18 @@ const parseScratchObject = function (object, runtime, topLevel) { soundPromises.push(loadSound(sound, runtime)); } } - // If included, parse any and all scripts/blocks on the object. - if (object.hasOwnProperty('scripts')) { - parseScripts(object.scripts, blocks); - } + // Create the first clone, and load its run-state from JSON. const target = sprite.createClone(); + const getVariableId = generateVariableIdGetter(target.id, topLevel); + // Load target properties from JSON. if (object.hasOwnProperty('variables')) { for (let j = 0; j < object.variables.length; j++) { const variable = object.variables[j]; const newVariable = new Variable( - null, + getVariableId(variable.name), variable.name, variable.value, variable.isPersistent @@ -200,6 +225,12 @@ const parseScratchObject = function (object, runtime, topLevel) { target.variables[newVariable.id] = newVariable; } } + + // If included, parse any and all scripts/blocks on the object. + if (object.hasOwnProperty('scripts')) { + parseScripts(object.scripts, blocks, getVariableId); + } + if (object.hasOwnProperty('lists')) { for (let k = 0; k < object.lists.length; k++) { const list = object.lists[k]; @@ -294,9 +325,10 @@ const sb2import = function (json, runtime, optForceSprite) { /** * Parse a single SB2 JSON-formatted block and its children. * @param {!object} sb2block SB2 JSON-formatted block. + * @param {Function} getVariableId function to retreive a variable's ID based on name * @return {object} Scratch VM format block. */ -const parseBlock = function (sb2block) { +const parseBlock = function (sb2block, getVariableId) { // First item in block object is the old opcode (e.g., 'forward:'). const oldOpcode = sb2block[0]; // Convert the block using the specMap. See sb2specmap.js. @@ -341,10 +373,10 @@ const parseBlock = function (sb2block) { let innerBlocks; if (typeof providedArg[0] === 'object' && providedArg[0]) { // Block list occupies the input. - innerBlocks = parseBlockList(providedArg); + innerBlocks = parseBlockList(providedArg, getVariableId); } else { // Single block occupies the input. - innerBlocks = [parseBlock(providedArg)]; + innerBlocks = [parseBlock(providedArg, getVariableId)]; } let previousBlock = null; for (let j = 0; j < innerBlocks.length; j++) { @@ -426,6 +458,11 @@ const parseBlock = function (sb2block) { name: expectedArg.fieldName, value: providedArg }; + + if (expectedArg.fieldName === 'VARIABLE') { + // Add `id` property to variable fields + activeBlock.fields[expectedArg.fieldName].id = getVariableId(providedArg); + } } } // Special cases to generate mutations. diff --git a/src/virtual-machine.js b/src/virtual-machine.js index 0cc3b7bb6..c3a0ba191 100644 --- a/src/virtual-machine.js +++ b/src/virtual-machine.js @@ -545,8 +545,11 @@ class VirtualMachine extends EventEmitter { * of the current editing target's blocks. */ emitWorkspaceUpdate () { - // @todo Include variables scoped to editing target also. - const variableMap = this.runtime.getTargetForStage().variables; + const variableMap = Object.assign({}, + this.runtime.getTargetForStage().variables, + this.editingTarget.variables + ); + const variables = Object.keys(variableMap).map(k => variableMap[k]); const xmlString = ` diff --git a/test/fixtures/data.sb2 b/test/fixtures/data.sb2 index a1a815efaa467375fbb1e5ec83c8ce0c5408d13f..e4b8c0b1c8658629cb8b77eb4afc10ac3bf0de7d 100644 GIT binary patch delta 1743 zcmaF8j=5n2bA5m}GYc030|N(xQQ||dWJ~4#a8?Ee0RaXE9-wGJQGQlxa*1A6aem(0 zaPQ*VW&(Dn_cuM&ocVEug2#u4^I~>;nr@usbH{UXpsC~p230kYgN&M%dB^_m zyj^t3InS4mP5k$Vb8BBquFQg zoHBUoq%mz~lGgVwmkIy$W|_yBxnBOXinYk^blc&hccU_O8tpDEv#Z{$In^y$O-uF8 z$I8zNJyxH1&L&rX=(MXUmS_ED5wQ1yoZ+W65&o{9S$^6&KZxGjDSsuXK_Rl)6^e0i^C#O~I7{>=Qly833(s@>J6 z7G`i&=qhY2vVXEi@OM}5q}5}8#r~ui`+%~Uq_i6*I(T7`|gfw zJgFKJ-WX3_^sQ&>6th2F_lj;>zP+!kr*HjU+1``$xCp!5N#}PTdDJ`QRpX=*?sFfi zjLog8b=|%AYxKR2qWfZtq>L0@y+xlg$#tLpuCSkOv@w4pII6a|${)HKju6#G1^`$Oqr``*{;M3(>e|$YFwC`|V zfrfVrO^XJe6DVyYJNIusJ)Ir~R|YC^>&5 zIf7cTDoiKOR)PDeV82H>>7L8NX4z|M2*Ub6ji@OJDyjtz!)= zce|nxUH(l{zw-3@hUY#veyg{Bx&L+J2CKDO+N2UDbT}qQzxiSQlsEI+6y|B^PsKtH zr&hb&?tFjk64$L=NlD+Gy5?4jyg&J%uVVWNU%!TM-nx3Ovb6$#KKkg%ySzQba&nvf z&i<9HQIhF*cco9cW#Fc?QshGMmg{F{vjtp`zOv5XJ#=&b=tjo<}d4_vcIxbdD~ibtZgR+e>=A&KmPrH!@I>PLhELyXI^w?x@_A_Ii z97CUYd*PjU$qNT11hbR4tPclNG1&4SJYG|E?Tbk59`+RD>z`YyW{L|=D>}pZ*rubv zC+S$tkv#^!&(9_rzp!a@=uZj|u&rC}&>uo_dF$2FotwMH(rb=i*ddXY2EB;qTJw+Z%UH zU8T@m&rmjfQl44J`jKW{tXi!!HRK+j+A{809~T|I=Q@>Jp`5?8JW0;)af!ET zPy5NFb&KvBy57nNFgY^IYuD_&=k>cn4>AiqYWY$))#T~ZDTV4cG$egFty^Nmu4=^2 za@k+vy(0FTKKBWo{N;{!ZzrDI__w6v)BkuT`)|_^EdFW|`Fq~F6&LS)Upvbv>-?OJ zXZm&YWhSWCGl)7D&-}aDz31ITAMPlTml<0{<|oLMG++37h(+)X>xrv>%zs*cO}V?G zpq`(R({%6c-SOd%-Alf2aG9I`$I``(@`O6I}iy1&= z@#dc_{H)9%1~=CuNigGsvY!N);bIdHP!b!jH; PE0g`Mi?h{T16cw9Wby_L delta 1611 zcmZvcdpOg39LIkdVTIg6ildgLmKiZhv1~cb0eZSA=dEVdm`}usI_g}B;&mdDzAm9lbh$IvM z02v^MwG;nRzo@ zBwQl|3QwTCW53I%yS? zu{9LF+nto$j*8w6?^K5w-r=WEradyr9NoQ3tl=XI4K{II3VTfWNApYFtqnyDfutxP3fbW)C!Z%=C@U zEyWwbWq}VoJK-yg=h@gtEGwk55%FkImRnm&I3l;9@BNwi?sw>Y_oDQeG2GGNHmS=! zB%jl~gkvRsqt}vl#;6Di3J_<+>J|1AF`I1?i97ch0#i43)F)?iPkcNyk zn|j5mG9tj0dv5+5a$n%p^s+;9OD-I*ocRQ5YRS|~yYV>Hk16$i7xPx?{RS@RFv3*( zaqeJgA2xHm;Y4I_5of$9F`jHtJ2rXGj~c$T$?5e_(>VHCmoL0nCE~tK37h-PwAapW zS`~~IB9~C*SyD2r0XKTG~keh#+8O0%i~!m!GkwY%06!C&vP(U;|C!Gl4C6fMv#`^7}f}3 zP@Tjxde!rz^~@Qstw9_|l}2r&=)EKS-ulfi}RZIi3o%m}%w zO`e|+a?2)A+Af~;@4dIPb*G&S9#dZ^;7pri=p0!^XvX|zSY4}}E)$`q7f%N9N{+B% z1RP7bA!}e$hVTgYu#};9y;9Qny4|z>x_rk%BiPjSr_xVDMrGRdM0=kNPEF~S(IlN= z6(UTq9U`y%n%f_Yo-mhELN;MDUS2&P9KA&?M?g8O8?sY3WG@qY?)De~=E~NvL>RZm zg{RYFnXtwWr)hfTeaDx`2H@F8&Aju;L1gHTFB*x>9sy3n?ag0h0dQ7(BlmHsDgo#7 z<^?yTDnD>PyoyU&&^Av8RR-d=-j-&B2TFmEvKa4^5+=|gt$ zcpB!gs}Z|U^>V_;HR7tpDeee?*wvV`@*uNZBQo?&FpWF()ymavvjxZ@KIL0siMZNT zwW-lgt*-tDMPy0jbs^P%n!@*0^F@VL1-k&i z@JFFHPZK;~4gf|!_!f^@7|B2+0f2bEZ(@xRHwp6z{ZA8o_5$^ZfrKa&&G##*J>q`r VD^eT9#id2};syZNIPp&W{0m!}$D9BF diff --git a/test/unit/serialization_sb2.js b/test/unit/serialization_sb2.js index cd3294f0a..68535b408 100644 --- a/test/unit/serialization_sb2.js +++ b/test/unit/serialization_sb2.js @@ -51,3 +51,20 @@ test('default', t => { t.end(); }); }); + +test('data scoping', t => { + // Get SB2 JSON (string) + const uri = path.resolve(__dirname, '../fixtures/data.sb2'); + const file = extract(uri); + const json = JSON.parse(file); + + // Create runtime instance & load SB2 into it + const rt = new Runtime(); + sb2.deserialize(json, rt).then(targets => { + const globalVariableIds = Object.keys(targets[0].variables); + const localVariableIds = Object.keys(targets[1].variables); + t.equal(targets[0].variables[globalVariableIds[0]].name, 'foo'); + t.equal(targets[1].variables[localVariableIds[0]].name, 'local'); + t.end(); + }); +}); diff --git a/test/unit/virtual-machine.js b/test/unit/virtual-machine.js index 9361ab918..94958569e 100644 --- a/test/unit/virtual-machine.js +++ b/test/unit/virtual-machine.js @@ -147,3 +147,42 @@ test('renameCostume sets the costume name', t => { t.equal(vm.editingTarget.sprite.costumes[1].name, 'hello2'); t.end(); }); + +test('emitWorkspaceUpdate', t => { + const vm = new VirtualMachine(); + vm.runtime.targets = [ + { + isStage: true, + variables: { + global: { + toXML: () => 'global' + } + } + }, { + variables: { + unused: { + toXML: () => 'unused' + } + } + }, { + variables: { + local: { + toXML: () => 'local' + } + }, + blocks: { + toXML: () => 'blocks' + } + } + ]; + vm.editingTarget = vm.runtime.targets[2]; + + let xml = null; + vm.emit = (event, data) => (xml = data.xml); + vm.emitWorkspaceUpdate(); + t.notEqual(xml.indexOf('global'), -1); + t.notEqual(xml.indexOf('local'), -1); + t.equal(xml.indexOf('unused'), -1); + t.notEqual(xml.indexOf('blocks'), -1); + t.end(); +}); From 0e11ff1c236ff56e1e129f5d22efecfe9e3c7eee Mon Sep 17 00:00:00 2001 From: Ray Schamp Date: Wed, 19 Jul 2017 10:28:54 -0400 Subject: [PATCH 13/21] Fix provider 'on' key being converted to 'true' --- .travis.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index fab21efd1..8ebf98bfb 100644 --- a/.travis.yml +++ b/.travis.yml @@ -23,7 +23,7 @@ jobs: before_deploy: npm --no-git-tag-version version $($(npm bin)/json -f package.json version)-prerelease.$(date +%s) deploy: - provider: script - on: + "on": all_branches: true condition: $RELEASE_BRANCHES =~ $TRAVIS_BRANCH skip_cleanup: true @@ -32,7 +32,7 @@ jobs: - git config --global user.name $(git log --pretty=format:"%an" -n1) - npm run --silent deploy -- -x -r $GH_PAGES_REPO - provider: npm - on: + "on": all_branches: true condition: $RELEASE_BRANCHES =~ $TRAVIS_BRANCH api_key: $NPM_TOKEN From 55d0b2650ccc90eeb1255c8ac5c7ccef8f7e88dd Mon Sep 17 00:00:00 2001 From: Ray Schamp Date: Wed, 19 Jul 2017 11:54:51 -0400 Subject: [PATCH 14/21] Convert multi-entry script to single scalar Multi-entries aren't supported https://docs.travis-ci.com/user/deployment/script --- .travis.yml | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/.travis.yml b/.travis.yml index 8ebf98bfb..7294016ca 100644 --- a/.travis.yml +++ b/.travis.yml @@ -27,10 +27,7 @@ jobs: all_branches: true condition: $RELEASE_BRANCHES =~ $TRAVIS_BRANCH skip_cleanup: true - script: - - git config --global user.email $(git log --pretty=format:"%ae" -n1) - - git config --global user.name $(git log --pretty=format:"%an" -n1) - - npm run --silent deploy -- -x -r $GH_PAGES_REPO + script: git config --global user.email $(git log --pretty=format:"%ae" -n1) && git config --global user.name $(git log --pretty=format:"%an" -n1) && npm run --silent deploy -- -x -r $GH_PAGES_REPO - provider: npm "on": all_branches: true From 7eae7e19576accbb8a02309946619c6022c6acbf Mon Sep 17 00:00:00 2001 From: Ray Schamp Date: Wed, 19 Jul 2017 12:04:22 -0400 Subject: [PATCH 15/21] Build the vm so we can deploy it Whoops. Also move the git deploy setup to before_deploy. --- .travis.yml | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/.travis.yml b/.travis.yml index 7294016ca..87bd1a1bd 100644 --- a/.travis.yml +++ b/.travis.yml @@ -19,15 +19,18 @@ jobs: node_js: 6 - stage: release node_js: 6 - script: true - before_deploy: npm --no-git-tag-version version $($(npm bin)/json -f package.json version)-prerelease.$(date +%s) + env: NPM_SCRIPT=build + before_deploy: + - npm --no-git-tag-version version $($(npm bin)/json -f package.json version)-prerelease.$(date +%s) + - git config --global user.email $(git log --pretty=format:"%ae" -n1) + - git config --global user.name $(git log --pretty=format:"%an" -n1) deploy: - provider: script "on": all_branches: true condition: $RELEASE_BRANCHES =~ $TRAVIS_BRANCH skip_cleanup: true - script: git config --global user.email $(git log --pretty=format:"%ae" -n1) && git config --global user.name $(git log --pretty=format:"%an" -n1) && npm run --silent deploy -- -x -r $GH_PAGES_REPO + script: npm run --silent deploy -- -x -r $GH_PAGES_REPO - provider: npm "on": all_branches: true From e0e10b61c529299d5e78720f8eafd645fa0fdfc3 Mon Sep 17 00:00:00 2001 From: Ray Schamp Date: Wed, 19 Jul 2017 13:48:55 -0400 Subject: [PATCH 16/21] Finish configuring npm deploy More followup fixes on #567, wasn't possible to test npm deployments really. --- .travis.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.travis.yml b/.travis.yml index 87bd1a1bd..955aa5bae 100644 --- a/.travis.yml +++ b/.travis.yml @@ -35,4 +35,6 @@ jobs: "on": all_branches: true condition: $RELEASE_BRANCHES =~ $TRAVIS_BRANCH + skip_cleanup: true + email: $NPM_EMAIL api_key: $NPM_TOKEN From ef16a7d574dd4a629dc8a94c8d89962434b220f6 Mon Sep 17 00:00:00 2001 From: Paul Kaplan Date: Tue, 25 Jul 2017 10:31:15 -0400 Subject: [PATCH 17/21] Use name instead of costumeName when using vm#addCostume --- test/integration/complex.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/integration/complex.js b/test/integration/complex.js index 113928483..331f2a982 100644 --- a/test/integration/complex.js +++ b/test/integration/complex.js @@ -43,7 +43,7 @@ test('complex', t => { vm.addCostume( 'f9a1c175dbe2e5dee472858dd30d16bb.svg', { - costumeName: 'costume1', + name: 'costume1', baseLayerID: 0, baseLayerMD5: 'f9a1c175dbe2e5dee472858dd30d16bb.svg', bitmapResolution: 1, @@ -79,7 +79,7 @@ test('complex', t => { vm.addBackdrop( '6b3d87ba2a7f89be703163b6c1d4c964.png', { - costumeName: 'baseball-field', + name: 'baseball-field', baseLayerID: 26, baseLayerMD5: '6b3d87ba2a7f89be703163b6c1d4c964.png', bitmapResolution: 2, From f96a92b4d1e53a950a2ddd50cd4e5cbda0510559 Mon Sep 17 00:00:00 2001 From: Paul Kaplan Date: Tue, 25 Jul 2017 10:32:25 -0400 Subject: [PATCH 18/21] Move renaming and adding into rendered-target with tests --- src/sprites/rendered-target.js | 45 ++++++++++++++++++++++++++++ src/virtual-machine.js | 14 +++------ test/unit/sprites_rendered-target.js | 40 +++++++++++++++++++++++++ test/unit/virtual-machine.js | 38 ----------------------- 4 files changed, 89 insertions(+), 48 deletions(-) diff --git a/src/sprites/rendered-target.js b/src/sprites/rendered-target.js index 7f7fbe0d6..1951700d3 100644 --- a/src/sprites/rendered-target.js +++ b/src/sprites/rendered-target.js @@ -1,5 +1,6 @@ const log = require('../util/log'); const MathUtil = require('../util/math-util'); +const StringUtil = require('../util/string-util'); const Target = require('../engine/target'); /** @@ -391,6 +392,28 @@ class RenderedTarget extends Target { this.runtime.requestTargetsUpdate(this); } + /** + * Add a costume, taking care to avoid duplicate names. + * @param {!object} costumeObject Object representing the costume. + */ + addCostume (costumeObject) { + const usedNames = this.sprite.costumes.map(costume => costume.name); + costumeObject.name = StringUtil.unusedName(costumeObject.name, usedNames); + this.sprite.costumes.push(costumeObject); + } + + /** + * Rename a costume, taking care to avoid duplicate names. + * @param {int} costumeIndex - the index of the costume to be renamed. + * @param {string} newName - the desired new name of the costume (will be modified if already in use). + */ + renameCostume (costumeIndex, newName) { + const usedNames = this.sprite.costumes + .filter((costume, index) => costumeIndex !== index) + .map(costume => costume.name); + this.sprite.costumes[costumeIndex].name = StringUtil.unusedName(newName, usedNames); + } + /** * Delete a costume by index. * @param {number} index Costume index to be deleted @@ -414,6 +437,28 @@ class RenderedTarget extends Target { this.runtime.requestTargetsUpdate(this); } + /** + * Add a sound, taking care to avoid duplicate names. + * @param {!object} soundObject Object representing the sound. + */ + addSound (soundObject) { + const usedNames = this.sprite.sounds.map(sound => sound.name); + soundObject.name = StringUtil.unusedName(soundObject.name, usedNames); + this.sprite.sounds.push(soundObject); + } + + /** + * Rename a sound, taking care to avoid duplicate names. + * @param {int} soundIndex - the index of the sound to be renamed. + * @param {string} newName - the desired new name of the sound (will be modified if already in use). + */ + renameSound (soundIndex, newName) { + const usedNames = this.sprite.sounds + .filter((sound, index) => soundIndex !== index) + .map(sound => sound.name); + this.sprite.sounds[soundIndex].name = StringUtil.unusedName(newName, usedNames); + } + /** * Delete a sound by index. * @param {number} index Sound index to be deleted diff --git a/src/virtual-machine.js b/src/virtual-machine.js index 0cc3b7bb6..3b338b974 100644 --- a/src/virtual-machine.js +++ b/src/virtual-machine.js @@ -280,7 +280,7 @@ class VirtualMachine extends EventEmitter { */ addCostume (md5ext, costumeObject) { loadCostume(md5ext, costumeObject, this.runtime).then(() => { - this.editingTarget.sprite.costumes.push(costumeObject); + this.editingTarget.addCostume(costumeObject); this.editingTarget.setCostume( this.editingTarget.sprite.costumes.length - 1 ); @@ -293,10 +293,7 @@ class VirtualMachine extends EventEmitter { * @param {string} newName - the desired new name of the costume (will be modified if already in use). */ renameCostume (costumeIndex, newName) { - const usedNames = this.editingTarget.sprite.costumes - .filter((costume, index) => costumeIndex !== index) - .map(costume => costume.name); - this.editingTarget.sprite.costumes[costumeIndex].name = StringUtil.unusedName(newName, usedNames); + this.editingTarget.renameCostume(costumeIndex, newName); this.emitTargetsUpdate(); } @@ -315,7 +312,7 @@ class VirtualMachine extends EventEmitter { */ addSound (soundObject) { return loadSound(soundObject, this.runtime).then(() => { - this.editingTarget.sprite.sounds.push(soundObject); + this.editingTarget.addSound(soundObject); this.emitTargetsUpdate(); }); } @@ -326,10 +323,7 @@ class VirtualMachine extends EventEmitter { * @param {string} newName - the desired new name of the sound (will be modified if already in use). */ renameSound (soundIndex, newName) { - const usedNames = this.editingTarget.sprite.sounds - .filter((sound, index) => soundIndex !== index) - .map(sound => sound.name); - this.editingTarget.sprite.sounds[soundIndex].name = StringUtil.unusedName(newName, usedNames); + this.editingTarget.renameSound(soundIndex, newName); this.emitTargetsUpdate(); } diff --git a/test/unit/sprites_rendered-target.js b/test/unit/sprites_rendered-target.js index ecb4a0899..470afbfd9 100644 --- a/test/unit/sprites_rendered-target.js +++ b/test/unit/sprites_rendered-target.js @@ -355,3 +355,43 @@ test('#toJSON returns the sounds and costumes', t => { t.same(a.toJSON().costumes, costumes); t.end(); }); + +test('#addSound does not duplicate names', t => { + const spr = new Sprite(); + const a = new RenderedTarget(spr, null); + a.sprite.sounds = [{name: 'first'}]; + a.addSound({name: 'first'}); + t.deepEqual(a.sprite.sounds, [{name: 'first'}, {name: 'first2'}]); + t.end(); +}); + +test('#addCostume does not duplicate names', t => { + const spr = new Sprite(); + const a = new RenderedTarget(spr, null); + a.sprite.costumes = [{name: 'first'}]; + a.addCostume({name: 'first'}); + t.deepEqual(a.sprite.costumes, [{name: 'first'}, {name: 'first2'}]); + t.end(); +}); + +test('#renameSound does not duplicate names', t => { + const spr = new Sprite(); + const a = new RenderedTarget(spr, null); + a.sprite.sounds = [{name: 'first'}, {name: 'second'}]; + a.renameSound(0, 'first'); // Shouldn't increment the name, noop + t.deepEqual(a.sprite.sounds, [{name: 'first'}, {name: 'second'}]); + a.renameSound(1, 'first'); + t.deepEqual(a.sprite.sounds, [{name: 'first'}, {name: 'first2'}]); + t.end(); +}); + +test('#renameCostume does not duplicate names', t => { + const spr = new Sprite(); + const a = new RenderedTarget(spr, null); + a.sprite.costumes = [{name: 'first'}, {name: 'second'}]; + a.renameCostume(0, 'first'); // Shouldn't increment the name, noop + t.deepEqual(a.sprite.costumes, [{name: 'first'}, {name: 'second'}]); + a.renameCostume(1, 'first'); + t.deepEqual(a.sprite.costumes, [{name: 'first'}, {name: 'first2'}]); + t.end(); +}); diff --git a/test/unit/virtual-machine.js b/test/unit/virtual-machine.js index 9361ab918..37a62ff69 100644 --- a/test/unit/virtual-machine.js +++ b/test/unit/virtual-machine.js @@ -109,41 +109,3 @@ test('renameSprite does not increment when renaming to the same name', t => { t.equal(vm.runtime.targets[0].sprite.name, 'this name'); t.end(); }); - -test('renameSound sets the sound name', t => { - const vm = new VirtualMachine(); - vm.editingTarget = { - sprite: { - sounds: [{name: 'first'}, {name: 'second'}] - } - }; - vm.renameSound(0, 'hello'); - t.equal(vm.editingTarget.sprite.sounds[0].name, 'hello'); - t.equal(vm.editingTarget.sprite.sounds[1].name, 'second'); - // Make sure renaming to same name doesn't increment - vm.renameSound(0, 'hello'); - t.equal(vm.editingTarget.sprite.sounds[0].name, 'hello'); - // But renaming to used name does increment - vm.renameSound(1, 'hello'); - t.equal(vm.editingTarget.sprite.sounds[1].name, 'hello2'); - t.end(); -}); - -test('renameCostume sets the costume name', t => { - const vm = new VirtualMachine(); - vm.editingTarget = { - sprite: { - costumes: [{name: 'first'}, {name: 'second'}] - } - }; - vm.renameCostume(0, 'hello'); - t.equal(vm.editingTarget.sprite.costumes[0].name, 'hello'); - t.equal(vm.editingTarget.sprite.costumes[1].name, 'second'); - // Make sure renaming to same name doesn't increment - vm.renameCostume(0, 'hello'); - t.equal(vm.editingTarget.sprite.costumes[0].name, 'hello'); - // But renaming to used name does increment - vm.renameCostume(1, 'hello'); - t.equal(vm.editingTarget.sprite.costumes[1].name, 'hello2'); - t.end(); -}); From 44298fd71bcabea561965c1a570751f3fa29dd4d Mon Sep 17 00:00:00 2001 From: Paul Kaplan Date: Tue, 25 Jul 2017 12:17:20 -0400 Subject: [PATCH 19/21] Use soundId instead of md5 for playing sounds --- src/blocks/scratch3_sound.js | 8 ++++---- src/import/load-sound.js | 5 ++++- test/unit/blocks_sounds.js | 28 ++++++++++++++-------------- 3 files changed, 22 insertions(+), 19 deletions(-) diff --git a/src/blocks/scratch3_sound.js b/src/blocks/scratch3_sound.js index 02347aa4c..6bd19ede2 100644 --- a/src/blocks/scratch3_sound.js +++ b/src/blocks/scratch3_sound.js @@ -113,18 +113,18 @@ class Scratch3SoundBlocks { playSound (args, util) { const index = this._getSoundIndex(args.SOUND_MENU, util); if (index >= 0) { - const md5 = util.target.sprite.sounds[index].md5; + const soundId = util.target.sprite.sounds[index].soundId; if (util.target.audioPlayer === null) return; - util.target.audioPlayer.playSound(md5); + util.target.audioPlayer.playSound(soundId); } } playSoundAndWait (args, util) { const index = this._getSoundIndex(args.SOUND_MENU, util); if (index >= 0) { - const md5 = util.target.sprite.sounds[index].md5; + const soundId = util.target.sprite.sounds[index].soundId; if (util.target.audioPlayer === null) return; - return util.target.audioPlayer.playSound(md5); + return util.target.audioPlayer.playSound(soundId); } } diff --git a/src/import/load-sound.js b/src/import/load-sound.js index 295b87ebe..8f4f9edff 100644 --- a/src/import/load-sound.js +++ b/src/import/load-sound.js @@ -31,7 +31,10 @@ const loadSound = function (sound, runtime) { {data: soundAsset.data} )); }) - .then(() => sound); + .then(soundId => { + sound.soundId = soundId; + return sound; + }); }; module.exports = loadSound; diff --git a/test/unit/blocks_sounds.js b/test/unit/blocks_sounds.js index e6c4bda91..753f46f0c 100644 --- a/test/unit/blocks_sounds.js +++ b/test/unit/blocks_sounds.js @@ -17,14 +17,14 @@ 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'} + {name: 'first name', soundId: 'first soundId'}, + {name: 'second name', soundId: 'second soundId'}, + {name: 'third name', soundId: 'third soundId'}, + {name: '6', soundId: 'fourth soundId'} ] }, audioPlayer: { - playSound: md5 => (playedSound = md5), + playSound: soundId => (playedSound = soundId), playDrumForBeats: drum => (playedDrum = drum) } } @@ -33,37 +33,37 @@ const util = { test('playSound with a name string works', t => { const args = {SOUND_MENU: 'second name'}; blocks.playSound(args, util); - t.strictEqual(playedSound, 'second md5'); + t.strictEqual(playedSound, 'second soundId'); 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'); + t.strictEqual(playedSound, 'first soundId'); args = {SOUND_MENU: '1'}; blocks.playSound(args, util); - t.strictEqual(playedSound, 'first md5'); + t.strictEqual(playedSound, 'first soundId'); args = {SOUND_MENU: '0'}; blocks.playSound(args, util); - t.strictEqual(playedSound, 'fourth md5'); + t.strictEqual(playedSound, 'fourth soundId'); 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'); + t.strictEqual(playedSound, 'first soundId'); args = {SOUND_MENU: 1}; blocks.playSound(args, util); - t.strictEqual(playedSound, 'first md5'); + t.strictEqual(playedSound, 'first soundId'); args = {SOUND_MENU: 0}; blocks.playSound(args, util); - t.strictEqual(playedSound, 'fourth md5'); + t.strictEqual(playedSound, 'fourth soundId'); t.end(); }); @@ -71,7 +71,7 @@ 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.strictEqual(playedSound, 'second soundId'); t.end(); }); @@ -79,7 +79,7 @@ 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.strictEqual(playedSound, 'fourth soundId'); t.end(); }); From 25a0e2ffaff936187397b1ec384ecdb979cebc59 Mon Sep 17 00:00:00 2001 From: Paul Kaplan Date: Tue, 25 Jul 2017 12:39:30 -0400 Subject: [PATCH 20/21] Use new audio engine api to retreive and set sound buffers --- src/virtual-machine.js | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/src/virtual-machine.js b/src/virtual-machine.js index 0cc3b7bb6..29d35f3f1 100644 --- a/src/virtual-machine.js +++ b/src/virtual-machine.js @@ -333,6 +333,32 @@ class VirtualMachine extends EventEmitter { this.emitTargetsUpdate(); } + /** + * Get a sound buffer from the audio engine. + * @param {int} soundIndex - the index of the sound to be got. + * @return {AudioBuffer} the sound's audio buffer. + */ + getSoundBuffer (soundIndex) { + const id = this.editingTarget.sprite.sounds[soundIndex].soundId; + if (id && this.runtime && this.runtime.audioEngine) { + return this.runtime.audioEngine.getSoundBuffer(id); + } + return null; + } + + /** + * Update a sound buffer. + * @param {int} soundIndex - the index of the sound to be updated. + * @param {AudioBuffer} newBuffer - new audio buffer for the audio engine. + */ + updateSoundBuffer (soundIndex, newBuffer) { + const id = this.editingTarget.sprite.sounds[soundIndex].soundId; + if (id && this.runtime && this.runtime.audioEngine) { + this.runtime.audioEngine.updateSoundBuffer(id, newBuffer); + } + this.emitTargetsUpdate(); + } + /** * Delete a sound from the current editing target. * @param {int} soundIndex - the index of the sound to be removed. From 936ff04b0f204094f0f6d28c57de48800a069975 Mon Sep 17 00:00:00 2001 From: TheBrokenRail Date: Wed, 26 Jul 2017 17:44:02 -0400 Subject: [PATCH 21/21] Fix setting Infinity to direction (#612) * Create rendered-target.js * Update math-util.js * Update rendered-target.js * Update math-util.js * Update rendered-target.js --- src/sprites/rendered-target.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/sprites/rendered-target.js b/src/sprites/rendered-target.js index 1951700d3..bbf85231c 100644 --- a/src/sprites/rendered-target.js +++ b/src/sprites/rendered-target.js @@ -231,6 +231,9 @@ class RenderedTarget extends Target { if (this.isStage) { return; } + if (!isFinite(direction)) { + return; + } // Keep direction between -179 and +180. this.direction = MathUtil.wrapClamp(direction, -179, 180); if (this.renderer) {