From 627fc1d9c346cb95c71d6cc8aea379779f1c380f Mon Sep 17 00:00:00 2001 From: Paul Kaplan Date: Mon, 20 Mar 2017 11:13:24 -0400 Subject: [PATCH 1/4] Add tests for existing vm.renameSprite functionality --- test/unit/virtual-machine.js | 51 ++++++++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) create mode 100644 test/unit/virtual-machine.js diff --git a/test/unit/virtual-machine.js b/test/unit/virtual-machine.js new file mode 100644 index 000000000..75c857965 --- /dev/null +++ b/test/unit/virtual-machine.js @@ -0,0 +1,51 @@ +var test = require('tap').test; +var VirtualMachine = require('../../src/virtual-machine.js'); + +test('renameSprite throws when there is no sprite with that id', function (t) { + var vm = new VirtualMachine(); + vm.runtime.getTargetById = () => null; + t.throws( + (() => vm.renameSprite('id', 'name')), + new Error('No target with the provided id.') + ); + t.end(); +}); + +test('renameSprite throws when used on a non-sprite target', function (t) { + var vm = new VirtualMachine(); + var fakeTarget = { + isSprite: () => false + }; + vm.runtime.getTargetById = () => (fakeTarget); + t.throws( + (() => vm.renameSprite('id', 'name')), + new Error('Cannot rename non-sprite targets.') + ); + t.end(); +}); + +test('renameSprite throws when there is no sprite for given target', function (t) { + var vm = new VirtualMachine(); + var fakeTarget = { + sprite: null, + isSprite: () => true + }; + vm.runtime.getTargetById = () => (fakeTarget); + t.throws( + (() => vm.renameSprite('id', 'name')), + new Error('No sprite associated with this target.') + ); + t.end(); +}); + +test('renameSprite sets the sprite name', function (t) { + var vm = new VirtualMachine(); + var fakeTarget = { + sprite: {name: 'original'}, + isSprite: () => true + }; + vm.runtime.getTargetById = () => (fakeTarget); + vm.renameSprite('id', 'not-original'); + t.equal(fakeTarget.sprite.name, 'not-original'); + t.end(); +}); From 5d2352e47187b182bb11588e977c1800564f8cc2 Mon Sep 17 00:00:00 2001 From: Paul Kaplan Date: Mon, 20 Mar 2017 11:14:25 -0400 Subject: [PATCH 2/4] Add test for skipping rename on blank input --- src/virtual-machine.js | 4 +++- test/unit/virtual-machine.js | 12 ++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/src/virtual-machine.js b/src/virtual-machine.js index 1431640fb..aae77937d 100644 --- a/src/virtual-machine.js +++ b/src/virtual-machine.js @@ -204,7 +204,9 @@ VirtualMachine.prototype.renameSprite = function (targetId, newName) { if (!sprite) { throw new Error('No sprite associated with this target.'); } - sprite.name = newName; + if (newName) { + sprite.name = newName; + } this.emitTargetsUpdate(); } else { throw new Error('No target with the provided id.'); diff --git a/test/unit/virtual-machine.js b/test/unit/virtual-machine.js index 75c857965..28bcdd764 100644 --- a/test/unit/virtual-machine.js +++ b/test/unit/virtual-machine.js @@ -49,3 +49,15 @@ test('renameSprite sets the sprite name', function (t) { t.equal(fakeTarget.sprite.name, 'not-original'); t.end(); }); + +test('renameSprite does not set sprite names to an empty string ', function (t) { + var vm = new VirtualMachine(); + var fakeTarget = { + sprite: {name: 'original'}, + isSprite: () => true + }; + vm.runtime.getTargetById = () => (fakeTarget); + vm.renameSprite('id', ''); + t.equal(fakeTarget.sprite.name, 'original'); + t.end(); +}); From 052ecef91c356552c98023c4f666a8753b5e0fed Mon Sep 17 00:00:00 2001 From: Paul Kaplan Date: Mon, 20 Mar 2017 12:12:38 -0400 Subject: [PATCH 3/4] Add string utils with tests --- src/util/string-util.js | 17 ++++++++++++ test/unit/util_string.js | 57 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 74 insertions(+) create mode 100644 src/util/string-util.js create mode 100644 test/unit/util_string.js diff --git a/src/util/string-util.js b/src/util/string-util.js new file mode 100644 index 000000000..6270cd885 --- /dev/null +++ b/src/util/string-util.js @@ -0,0 +1,17 @@ +var StringUtil = function () {}; + +StringUtil.withoutTrailingDigits = function (s) { + var i = s.length - 1; + while ((i >= 0) && ('0123456789'.indexOf(s.charAt(i)) > -1)) i--; + return s.slice(0, i + 1); +}; + +StringUtil.unusedName = function (name, existingNames) { + if (existingNames.indexOf(name) < 0) return name; + name = StringUtil.withoutTrailingDigits(name); + var i = 2; + while (existingNames.indexOf(name + i) >= 0) i++; + return name + i; +}; + +module.exports = StringUtil; diff --git a/test/unit/util_string.js b/test/unit/util_string.js new file mode 100644 index 000000000..fdcd439d3 --- /dev/null +++ b/test/unit/util_string.js @@ -0,0 +1,57 @@ +var test = require('tap').test; +var StringUtil = require('../../src/util/string-util'); + +test('withoutTrailingDigits', function (t) { + t.strictEqual(StringUtil.withoutTrailingDigits('boeing747'), 'boeing'); + t.strictEqual(StringUtil.withoutTrailingDigits('boeing747 '), 'boeing747 '); + t.strictEqual(StringUtil.withoutTrailingDigits('boeing𝟨'), 'boeing𝟨'); + t.strictEqual(StringUtil.withoutTrailingDigits('boeing 747'), 'boeing '); + t.strictEqual(StringUtil.withoutTrailingDigits('747'), ''); + t.end(); +}); + +test('unusedName', function (t) { + t.strictEqual( + StringUtil.unusedName( + 'name', + ['not the same name'] + ), + 'name' + ); + t.strictEqual( + StringUtil.unusedName( + 'name', + ['name'] + ), + 'name2' + ); + t.strictEqual( + StringUtil.unusedName( + 'name', + ['name30'] + ), + 'name' + ); + t.strictEqual( + StringUtil.unusedName( + 'name', + ['name', 'name2'] + ), + 'name3' + ); + t.strictEqual( + StringUtil.unusedName( + 'name', + ['name', 'name3'] + ), + 'name2' + ); + t.strictEqual( + StringUtil.unusedName( + 'boeing747', + ['boeing747'] + ), + 'boeing2' // Yup, this matches scratch-flash... + ); + t.end(); +}); From 8bbb395b35d2532b0b20e40d94fa4798f5b10fba Mon Sep 17 00:00:00 2001 From: Paul Kaplan Date: Mon, 20 Mar 2017 12:52:57 -0400 Subject: [PATCH 4/4] Add sprite name incrementing and reserved naming --- src/virtual-machine.js | 13 +++++++++++-- test/integration/complex.js | 2 +- test/unit/virtual-machine.js | 35 ++++++++++++++++++++++++++++++++++- 3 files changed, 46 insertions(+), 4 deletions(-) diff --git a/src/virtual-machine.js b/src/virtual-machine.js index aae77937d..576e3a78a 100644 --- a/src/virtual-machine.js +++ b/src/virtual-machine.js @@ -3,6 +3,9 @@ var util = require('util'); var Runtime = require('./engine/runtime'); var sb2import = require('./import/sb2import'); +var StringUtil = require('./util/string-util'); + +var RESERVED_NAMES = ['_mouse_', '_stage_', '_edge_', '_myself_', '_random_']; /** * Handles connections between blocks, stage, and extensions. @@ -204,8 +207,14 @@ VirtualMachine.prototype.renameSprite = function (targetId, newName) { if (!sprite) { throw new Error('No sprite associated with this target.'); } - if (newName) { - sprite.name = newName; + if (newName && RESERVED_NAMES.indexOf(newName) === -1) { + var names = this.runtime.targets.filter(function (runtimeTarget) { + return runtimeTarget.isSprite(); + }).map(function (runtimeTarget) { + return runtimeTarget.sprite.name; + }); + + sprite.name = StringUtil.unusedName(newName, names); } this.emitTargetsUpdate(); } else { diff --git a/test/integration/complex.js b/test/integration/complex.js index dea8d40bc..f8dbd2cc2 100644 --- a/test/integration/complex.js +++ b/test/integration/complex.js @@ -26,7 +26,7 @@ test('complex', function (t) { var targets = data.targetList; for (var i in targets) { if (targets[i].isStage === true) continue; - if (targets[i].name === 'test') continue; + if (targets[i].name.match(/test/)) continue; vm.setEditingTarget(targets[i].id); vm.renameSprite(targets[i].id, 'test'); diff --git a/test/unit/virtual-machine.js b/test/unit/virtual-machine.js index 28bcdd764..eed537dd3 100644 --- a/test/unit/virtual-machine.js +++ b/test/unit/virtual-machine.js @@ -50,7 +50,7 @@ test('renameSprite sets the sprite name', function (t) { t.end(); }); -test('renameSprite does not set sprite names to an empty string ', function (t) { +test('renameSprite does not set sprite names to an empty string', function (t) { var vm = new VirtualMachine(); var fakeTarget = { sprite: {name: 'original'}, @@ -61,3 +61,36 @@ test('renameSprite does not set sprite names to an empty string ', function (t) t.equal(fakeTarget.sprite.name, 'original'); t.end(); }); + +test('renameSprite does not set sprite names to reserved names', function (t) { + var vm = new VirtualMachine(); + var fakeTarget = { + sprite: {name: 'original'}, + isSprite: () => true + }; + vm.runtime.getTargetById = () => (fakeTarget); + vm.renameSprite('id', '_mouse_'); + t.equal(fakeTarget.sprite.name, 'original'); + t.end(); +}); + +test('renameSprite increments from existing sprite names', function (t) { + var vm = new VirtualMachine(); + vm.emitTargetsUpdate = () => {}; + vm.runtime.targets = [{ + id: 'id1', + isSprite: () => true, + sprite: { + name: 'this name' + } + }, { + id: 'id2', + isSprite: () => true, + sprite: { + name: 'that name' + } + }]; + vm.renameSprite('id1', 'that name'); + t.equal(vm.runtime.targets[0].sprite.name, 'that name2'); + t.end(); +});