Merge pull request #652 from paulkaplan/fix-asset-naming

Use unused names when adding assets

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

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();
     }
 

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,

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();
+});

test/unit/virtual-machine.js

@@ -110,44 +110,6 @@ test('renameSprite does not increment when renaming to the same name', t => {
     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();
-});
-
 test('emitWorkspaceUpdate', t => {
     const vm = new VirtualMachine();
     vm.runtime.targets = [