From f96a92b4d1e53a950a2ddd50cd4e5cbda0510559 Mon Sep 17 00:00:00 2001 From: Paul Kaplan Date: Tue, 25 Jul 2017 10:32:25 -0400 Subject: [PATCH] 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(); -});