From c81f7ee9a6852612799414ebdd7e511499f8630e Mon Sep 17 00:00:00 2001 From: jokebookservice1 Date: Wed, 22 Aug 2018 20:38:46 +0300 Subject: [PATCH 1/9] Create unit tests for SWITCH COSTUME and SWITCH BACKDROP --- test/unit/blocks_looks.js | 142 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 142 insertions(+) diff --git a/test/unit/blocks_looks.js b/test/unit/blocks_looks.js index a9602a706..74d4a6fe8 100644 --- a/test/unit/blocks_looks.js +++ b/test/unit/blocks_looks.js @@ -1,6 +1,8 @@ const test = require('tap').test; const Looks = require('../../src/blocks/scratch3_looks'); const Runtime = require('../../src/engine/runtime'); +const Sprite = require('../../src/sprites/sprite.js'); +const RenderedTarget = require('../../src/sprites/rendered-target.js'); const util = { target: { currentCostume: 0, // Internally, current costume is 0 indexed @@ -23,6 +25,146 @@ const fakeRuntime = { }; const blocks = new Looks(fakeRuntime); +test('switch costume block runs correctly', t => { + /** + * Test which costume index the `switch costume` + * block will jump to given an argument and array + * of costume names. + * + * @param {string[]} costumes List of costume names as strings + * @param {string|number|boolean} arg The argument to provide to the block. + * @param {number} [currentCostume=1] The 1-indexed default costume for the sprite to start at. + * @return {number} The 1-indexed costume index on which the sprite lands. + */ + const testCostume = (costumes, arg, currentCostume = 1) => { + const rt = new Runtime(); + const looks = new Looks(rt); + + const sprite = new Sprite(null, rt); + const target = new RenderedTarget(sprite, rt); + + sprite.costumes = costumes.map(name => ({name: name})); + target.currentCostume = currentCostume - 1; // Convert to 0-indexed. + + looks.switchCostume({COSTUME: arg}, {target}); + + return target.currentCostume + 1; // Convert to 1-indexed. + }; + + // Non-existant costumes do nothing + t.strictEqual(testCostume(['a', 'b', 'c', 'd'], 'e', 3), 3); + + // Difference between string and numeric arguments + t.strictEqual(testCostume(['a', 'b', 'c', '2'], 2), 2); + t.strictEqual(testCostume(['a', 'b', 'c', '2'], '2'), 4); + + // 'previous costume' and 'next costume' increment/decrement + t.strictEqual(testCostume(['a', 'b', 'c', 'd'], 'previous costume', 3), 2); + t.strictEqual(testCostume(['a', 'b', 'c', 'd'], 'next costume', 2), 3); + + // 'previous costume' and 'next costume' can be overriden + t.strictEqual(testCostume(['a', 'previous costume', 'c', 'd'], 'previous costume'), 2); + t.strictEqual(testCostume(['next costume', 'b', 'c', 'd'], 'next costume'), 1); + + // NaN, Infinity, and true are the first costume + t.strictEqual(testCostume(['a', 'b', 'c', 'd'], NaN, 2), 1); + t.strictEqual(testCostume(['a', 'b', 'c', 'd'], true, 2), 1); + t.strictEqual(testCostume(['a', 'b', 'c', 'd'], Infinity, 2), 1); + t.strictEqual(testCostume(['a', 'b', 'c', 'd'], -Infinity, 2), 1); + + // 'previous backdrop' and 'next backdrop' have no effect + t.strictEqual(testCostume(['a', 'b', 'c', 'd'], 'previous backdrop', 3), 3); + t.strictEqual(testCostume(['a', 'b', 'c', 'd'], 'next backdrop', 3), 3); + + // Strings with no digits are not numeric + t.strictEqual(testCostume(['a', 'b', 'c', 'd'], ' ', 2), 2); + + // False is 0 (the last costume) + t.strictEqual(testCostume(['a', 'b', 'c', 'd'], false), 4); + + // Booleans are costume names where possible. + t.strictEqual(testCostume(['a', 'true', 'false', 'd'], false), 3); + t.strictEqual(testCostume(['a', 'true', 'false', 'd'], true), 2); + + // Costume indices should wrap around. + t.strictEqual(testCostume(['a', 'b', 'c', 'd'], -1), 3); + t.strictEqual(testCostume(['a', 'b', 'c', 'd'], -4), 4); + t.strictEqual(testCostume(['a', 'b', 'c', 'd'], 10), 2); + + t.end(); +}); + +test('switch backdrop block runs correctly', t => { + /** + * Test which backdrop index the `switch backdrop` + * block will jump to given an argument and array + * of backdrop names. + * + * @param {string[]} backdrops List of backdrop names as strings + * @param {string|number|boolean} arg The argument to provide to the block. + * @param {number} [currentCostume=1] The 1-indexed default backdrop for the stage to start at. + * @return {number} The 1-indexed backdrop index on which the stage lands. + */ + const testBackdrop = (backdrops, arg, currentCostume = 1) => { + const rt = new Runtime(); + const looks = new Looks(rt); + + const stage = new Sprite(null, rt); + const target = new RenderedTarget(stage, rt); + + stage.costumes = backdrops.map(name => ({name: name})); + target.currentCostume = currentCostume - 1; // Convert to 0-indexed. + target.isStage = true; + rt.targets.push(target); + + looks.switchBackdrop({BACKDROP: arg}, {target}); + + return target.currentCostume + 1; // Convert to 1-indexed. + }; + + // Non-existant backdrops do nothing + t.strictEqual(testBackdrop(['a', 'b', 'c', 'd'], 'e', 3), 3); + + // Difference between string and numeric arguments + t.strictEqual(testBackdrop(['a', 'b', 'c', '2'], 2), 2); + t.strictEqual(testBackdrop(['a', 'b', 'c', '2'], '2'), 4); + + // 'previous backdrop' and 'next backdrop' increment/decrement + t.strictEqual(testBackdrop(['a', 'b', 'c', 'd'], 'previous backdrop', 3), 2); + t.strictEqual(testBackdrop(['a', 'b', 'c', 'd'], 'next backdrop', 2), 3); + + // 'previous backdrop' and 'next backdrop' can't be overriden + t.strictEqual(testBackdrop(['a', 'previous backdrop', 'c', 'd'], 'previous backdrop', 4), 3); + t.strictEqual(testBackdrop(['next backdrop', 'b', 'c', 'd'], 'next backdrop'), 2); + + // NaN, Infinity, and true are the first costume + t.strictEqual(testBackdrop(['a', 'b', 'c', 'd'], NaN, 2), 1); + t.strictEqual(testBackdrop(['a', 'b', 'c', 'd'], true, 2), 1); + t.strictEqual(testBackdrop(['a', 'b', 'c', 'd'], Infinity, 2), 1); + t.strictEqual(testBackdrop(['a', 'b', 'c', 'd'], -Infinity, 2), 1); + + // 'previous costume' and 'next costume' have no effect + t.strictEqual(testBackdrop(['a', 'b', 'c', 'd'], 'previous costume', 3), 3); + t.strictEqual(testBackdrop(['a', 'b', 'c', 'd'], 'next costume', 3), 3); + + // Strings with no digits are not numeric + t.strictEqual(testBackdrop(['a', 'b', 'c', 'd'], ' ', 2), 2); + + // False is 0 (the last costume) + t.strictEqual(testBackdrop(['a', 'b', 'c', 'd'], false), 4); + + // Booleans are backdrop names where possible. + t.strictEqual(testBackdrop(['a', 'true', 'false', 'd'], false), 3); + t.strictEqual(testBackdrop(['a', 'true', 'false', 'd'], true), 2); + + // Backdrop indices should wrap around. + t.strictEqual(testBackdrop(['a', 'b', 'c', 'd'], -1), 3); + t.strictEqual(testBackdrop(['a', 'b', 'c', 'd'], -4), 4); + t.strictEqual(testBackdrop(['a', 'b', 'c', 'd'], 10), 2); + + t.end(); +}); + test('getCostumeNumberName returns 1-indexed costume number', t => { util.target.currentCostume = 0; // This is 0-indexed. const args = {NUMBER_NAME: 'number'}; From 3ef61e22489f49b1cf277a3a835669fe1260efd9 Mon Sep 17 00:00:00 2001 From: jokebookservice1 Date: Wed, 22 Aug 2018 20:39:15 +0300 Subject: [PATCH 2/9] Make SWITCH COSTUME and SWITCH BACKDROP compatible with 2.0 --- src/blocks/scratch3_looks.js | 97 +++++++++++++++++++++------------- src/sprites/rendered-target.js | 2 + 2 files changed, 61 insertions(+), 38 deletions(-) diff --git a/src/blocks/scratch3_looks.js b/src/blocks/scratch3_looks.js index cc2f2ac4c..92f4995b5 100644 --- a/src/blocks/scratch3_looks.js +++ b/src/blocks/scratch3_looks.js @@ -344,65 +344,86 @@ class Scratch3LooksBlocks { } /** - * Utility function to set the costume or backdrop of a target. + * Utility function to set the costume of a target. * Matches the behavior of Scratch 2.0 for different types of arguments. - * @param {!Target} target Target to set costume/backdrop to. + * @param {!Target} target Target to set costume to. * @param {Any} requestedCostume Costume requested, e.g., 0, 'name', etc. * @param {boolean=} optZeroIndex Set to zero-index the requestedCostume. * @return {Array.} Any threads started by this switch. */ - _setCostumeOrBackdrop (target, - requestedCostume, optZeroIndex) { + _setCostume (target, requestedCostume, optZeroIndex) { if (typeof requestedCostume === 'number') { - target.setCostume(optZeroIndex ? - requestedCostume : requestedCostume - 1); + target.setCostume(optZeroIndex ? requestedCostume : requestedCostume - 1); } else { - const costumeIndex = target.getCostumeIndexByName(requestedCostume); - if (costumeIndex > -1) { + const costumeIndex = target.getCostumeIndexByName(requestedCostume.toString()); + + if (costumeIndex !== -1) { target.setCostume(costumeIndex); - } else if (requestedCostume === 'previous costume' || - requestedCostume === 'previous backdrop') { - target.setCostume(target.currentCostume - 1); - } else if (requestedCostume === 'next costume' || - requestedCostume === 'next backdrop') { + } else if (requestedCostume === 'next costume') { target.setCostume(target.currentCostume + 1); - } else if (requestedCostume === 'random backdrop') { - const numCostumes = target.getCostumes().length; - if (numCostumes > 1) { - let selectedIndex = Math.floor(Math.random() * (numCostumes - 1)); - if (selectedIndex === target.currentCostume) selectedIndex += 1; - target.setCostume(selectedIndex); - } - } else { - const forcedNumber = Number(requestedCostume); - if (!isNaN(forcedNumber)) { - target.setCostume(optZeroIndex ? - forcedNumber : forcedNumber - 1); - } + } else if (requestedCostume === 'previous costume') { + target.setCostume(target.currentCostume - 1); + } else if (!isNaN(requestedCostume) && + (typeof requestedCostume !== 'string' || /\d/g.test(requestedCostume))) { + target.setCostume(optZeroIndex ? Number(requestedCostume) : Number(requestedCostume) - 1); } } - if (target === this.runtime.getTargetForStage()) { - // Target is the stage - start hats. - const newName = target.getCostumes()[target.currentCostume].name; - return this.runtime.startHats('event_whenbackdropswitchesto', { - BACKDROP: newName - }); - } + + // Per 2.0, 'switch costume' can't start threads even in the Stage. return []; } + /** + * Utility function to set the backdrop of a target. + * Matches the behavior of Scratch 2.0 for different types of arguments. + * @param {!Target} stage Target to set backdrop to. + * @param {Any} requestedBackdrop Backdrop requested, e.g., 0, 'name', etc. + * @param {boolean=} optZeroIndex Set to zero-index the requestedBackdrop. + * @return {Array.} Any threads started by this switch. + */ + _setBackdrop (stage, requestedBackdrop, optZeroIndex) { + if (typeof requestedBackdrop === 'number') { + stage.setCostume(optZeroIndex ? requestedBackdrop : requestedBackdrop - 1); + } else if (requestedBackdrop === 'next backdrop') { + stage.setCostume(stage.currentCostume + 1); + } else if (requestedBackdrop === 'previous backdrop') { + stage.setCostume(stage.currentCostume - 1); + } else { + const costumeIndex = stage.getCostumeIndexByName(requestedBackdrop.toString()); + + if (costumeIndex !== -1) { + stage.setCostume(costumeIndex); + } else if (requestedBackdrop === 'random backdrop') { + const numCostumes = stage.getCostumes().length; + if (numCostumes > 1) { + let selectedIndex = Math.floor(Math.random() * (numCostumes - 1)); + if (selectedIndex === stage.currentCostume) selectedIndex += 1; + stage.setCostume(selectedIndex); + } + } else if (!isNaN(requestedBackdrop) && + (typeof requestedBackdrop !== 'string' || /\d/g.test(requestedBackdrop))) { + stage.setCostume(optZeroIndex ? Number(requestedBackdrop) : Number(requestedBackdrop) - 1); + } + } + + const newName = stage.getCostumes()[stage.currentCostume].name; + return this.runtime.startHats('event_whenbackdropswitchesto', { + BACKDROP: newName + }); + } + switchCostume (args, util) { - this._setCostumeOrBackdrop(util.target, args.COSTUME); + this._setCostume(util.target, args.COSTUME); } nextCostume (args, util) { - this._setCostumeOrBackdrop( + this._setCostume( util.target, util.target.currentCostume + 1, true ); } switchBackdrop (args) { - this._setCostumeOrBackdrop(this.runtime.getTargetForStage(), args.BACKDROP); + this._setBackdrop(this.runtime.getTargetForStage(), args.BACKDROP); } switchBackdropAndWait (args, util) { @@ -410,7 +431,7 @@ class Scratch3LooksBlocks { if (!util.stackFrame.startedThreads) { // No - switch the backdrop. util.stackFrame.startedThreads = ( - this._setCostumeOrBackdrop( + this._setBackdrop( this.runtime.getTargetForStage(), args.BACKDROP ) @@ -440,7 +461,7 @@ class Scratch3LooksBlocks { nextBackdrop () { const stage = this.runtime.getTargetForStage(); - this._setCostumeOrBackdrop( + this._setBackdrop( stage, stage.currentCostume + 1, true ); } diff --git a/src/sprites/rendered-target.js b/src/sprites/rendered-target.js index d21280831..52b9bb850 100644 --- a/src/sprites/rendered-target.js +++ b/src/sprites/rendered-target.js @@ -463,6 +463,8 @@ class RenderedTarget extends Target { setCostume (index) { // Keep the costume index within possible values. index = Math.round(index); + if ([Infinity, -Infinity, NaN].includes(index)) index = 0; + this.currentCostume = MathUtil.wrapClamp( index, 0, this.sprite.costumes.length - 1 ); From c90e0331484445cc778ad02c9aa5e8473638cee7 Mon Sep 17 00:00:00 2001 From: jokebookservice1 Date: Wed, 10 Oct 2018 18:16:58 +0100 Subject: [PATCH 3/9] test: add more comments, add additional test --- test/unit/blocks_looks.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/test/unit/blocks_looks.js b/test/unit/blocks_looks.js index 74d4a6fe8..6cc9f7c9c 100644 --- a/test/unit/blocks_looks.js +++ b/test/unit/blocks_looks.js @@ -54,9 +54,12 @@ test('switch costume block runs correctly', t => { // Non-existant costumes do nothing t.strictEqual(testCostume(['a', 'b', 'c', 'd'], 'e', 3), 3); - // Difference between string and numeric arguments + // Numeric arguments are always the costume index + // String arguments are treated as costume names, and coerced to + // a costume index as a fallback t.strictEqual(testCostume(['a', 'b', 'c', '2'], 2), 2); t.strictEqual(testCostume(['a', 'b', 'c', '2'], '2'), 4); + t.strictEqual(testCostume(['a', 'b', 'c'], '2'), 2); // 'previous costume' and 'next costume' increment/decrement t.strictEqual(testCostume(['a', 'b', 'c', 'd'], 'previous costume', 3), 2); From df78a8345e33c3bbec0948a4cd3b1ec7977e920f Mon Sep 17 00:00:00 2001 From: jokebookservice1 Date: Wed, 10 Oct 2018 18:36:06 +0100 Subject: [PATCH 4/9] refactor: use utility for whitespace-testing --- src/blocks/scratch3_looks.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/blocks/scratch3_looks.js b/src/blocks/scratch3_looks.js index 92f4995b5..dbf4be7a3 100644 --- a/src/blocks/scratch3_looks.js +++ b/src/blocks/scratch3_looks.js @@ -353,6 +353,7 @@ class Scratch3LooksBlocks { */ _setCostume (target, requestedCostume, optZeroIndex) { if (typeof requestedCostume === 'number') { + // Numbers should be treated as comments, always target.setCostume(optZeroIndex ? requestedCostume : requestedCostume - 1); } else { const costumeIndex = target.getCostumeIndexByName(requestedCostume.toString()); @@ -363,8 +364,10 @@ class Scratch3LooksBlocks { target.setCostume(target.currentCostume + 1); } else if (requestedCostume === 'previous costume') { target.setCostume(target.currentCostume - 1); - } else if (!isNaN(requestedCostume) && - (typeof requestedCostume !== 'string' || /\d/g.test(requestedCostume))) { + // Try to cast the string to a number (and treat it as a costume index) + // Pure whitespace should not be treated as a number (JS casts this to 0) + // Note: isNaN will cast the string to a number before checking if it's NaN + } else if (!(isNaN(requestedCostume) || Cast.isWhiteSpace(requestedCostume))) { target.setCostume(optZeroIndex ? Number(requestedCostume) : Number(requestedCostume) - 1); } } From f2a46098aba7acc2cd0fc593c7777cfea0c75951 Mon Sep 17 00:00:00 2001 From: jokebookservice1 Date: Wed, 10 Oct 2018 19:20:03 +0100 Subject: [PATCH 5/9] refactor: move helper functions for tests Move the helper functions `testCostume` and `testBackdrop` outside of their test definitions. In addition, define the latter in terms of the former. --- test/unit/blocks_looks.js | 87 ++++++++++++++++++--------------------- 1 file changed, 40 insertions(+), 47 deletions(-) diff --git a/test/unit/blocks_looks.js b/test/unit/blocks_looks.js index 6cc9f7c9c..4c89dd711 100644 --- a/test/unit/blocks_looks.js +++ b/test/unit/blocks_looks.js @@ -25,32 +25,52 @@ const fakeRuntime = { }; const blocks = new Looks(fakeRuntime); -test('switch costume block runs correctly', t => { - /** - * Test which costume index the `switch costume` - * block will jump to given an argument and array - * of costume names. - * - * @param {string[]} costumes List of costume names as strings - * @param {string|number|boolean} arg The argument to provide to the block. - * @param {number} [currentCostume=1] The 1-indexed default costume for the sprite to start at. - * @return {number} The 1-indexed costume index on which the sprite lands. - */ - const testCostume = (costumes, arg, currentCostume = 1) => { - const rt = new Runtime(); - const looks = new Looks(rt); +/** + * Test which costume index the `switch costume` + * block will jump to given an argument and array + * of costume names. Works for backdrops if isStage is set. + * + * @param {string[]} costumes List of costume names as strings + * @param {string|number|boolean} arg The argument to provide to the block. + * @param {number} [currentCostume=1] The 1-indexed default costume for the sprite to start at. + * @param {boolean} [isStage=false] Whether the sprite is the stage + * @return {number} The 1-indexed costume index on which the sprite lands. + */ +const testCostume = (costumes, arg, currentCostume = 1, isStage = false) => { + const rt = new Runtime(); + const looks = new Looks(rt); - const sprite = new Sprite(null, rt); - const target = new RenderedTarget(sprite, rt); + const sprite = new Sprite(null, rt); + const target = new RenderedTarget(sprite, rt); - sprite.costumes = costumes.map(name => ({name: name})); - target.currentCostume = currentCostume - 1; // Convert to 0-indexed. + sprite.costumes = costumes.map(name => ({name: name})); + target.currentCostume = currentCostume - 1; // Convert to 0-indexed. + if(isStage) { + target.isStage = true; + rt.targets.push(target); + looks.switchBackdrop({BACKDROP: arg}, {target}); + } else { looks.switchCostume({COSTUME: arg}, {target}); + } - return target.currentCostume + 1; // Convert to 1-indexed. - }; + return target.currentCostume + 1; // Convert to 1-indexed. +}; + +/** + * Test which backdrop index the `switch backdrop` + * block will jump to given an argument and array + * of backdrop names. + * + * @param {string[]} backdrops List of backdrop names as strings + * @param {string|number|boolean} arg The argument to provide to the block. + * @param {number} [currentCostume=1] The 1-indexed default backdrop for the stage to start at. + * @return {number} The 1-indexed backdrop index on which the stage lands. + */ +const testBackdrop = (backdrops, arg, currentCostume = 1) => testCostume(backdrops, arg, currentCostume, true) + +test('switch costume block runs correctly', t => { // Non-existant costumes do nothing t.strictEqual(testCostume(['a', 'b', 'c', 'd'], 'e', 3), 3); @@ -98,33 +118,6 @@ test('switch costume block runs correctly', t => { }); test('switch backdrop block runs correctly', t => { - /** - * Test which backdrop index the `switch backdrop` - * block will jump to given an argument and array - * of backdrop names. - * - * @param {string[]} backdrops List of backdrop names as strings - * @param {string|number|boolean} arg The argument to provide to the block. - * @param {number} [currentCostume=1] The 1-indexed default backdrop for the stage to start at. - * @return {number} The 1-indexed backdrop index on which the stage lands. - */ - const testBackdrop = (backdrops, arg, currentCostume = 1) => { - const rt = new Runtime(); - const looks = new Looks(rt); - - const stage = new Sprite(null, rt); - const target = new RenderedTarget(stage, rt); - - stage.costumes = backdrops.map(name => ({name: name})); - target.currentCostume = currentCostume - 1; // Convert to 0-indexed. - target.isStage = true; - rt.targets.push(target); - - looks.switchBackdrop({BACKDROP: arg}, {target}); - - return target.currentCostume + 1; // Convert to 1-indexed. - }; - // Non-existant backdrops do nothing t.strictEqual(testBackdrop(['a', 'b', 'c', 'd'], 'e', 3), 3); From 1a7ce093c36f407405f0d729f5e6c006090ca7de Mon Sep 17 00:00:00 2001 From: jokebookservice1 Date: Wed, 10 Oct 2018 19:39:57 +0100 Subject: [PATCH 6/9] style(blocks_looks.js): add semicolon/space --- test/unit/blocks_looks.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/unit/blocks_looks.js b/test/unit/blocks_looks.js index 4c89dd711..ab0552a08 100644 --- a/test/unit/blocks_looks.js +++ b/test/unit/blocks_looks.js @@ -46,7 +46,7 @@ const testCostume = (costumes, arg, currentCostume = 1, isStage = false) => { sprite.costumes = costumes.map(name => ({name: name})); target.currentCostume = currentCostume - 1; // Convert to 0-indexed. - if(isStage) { + if (isStage) { target.isStage = true; rt.targets.push(target); looks.switchBackdrop({BACKDROP: arg}, {target}); @@ -68,7 +68,7 @@ const testCostume = (costumes, arg, currentCostume = 1, isStage = false) => { * @param {number} [currentCostume=1] The 1-indexed default backdrop for the stage to start at. * @return {number} The 1-indexed backdrop index on which the stage lands. */ -const testBackdrop = (backdrops, arg, currentCostume = 1) => testCostume(backdrops, arg, currentCostume, true) +const testBackdrop = (backdrops, arg, currentCostume = 1) => testCostume(backdrops, arg, currentCostume, true); test('switch costume block runs correctly', t => { // Non-existant costumes do nothing From 0c18b4952da10feadc5826417014aaeeeb929561 Mon Sep 17 00:00:00 2001 From: jokebookservice1 Date: Wed, 10 Oct 2018 19:45:36 +0100 Subject: [PATCH 7/9] refactor: use better whitespace-test for backdrops too --- src/blocks/scratch3_looks.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/blocks/scratch3_looks.js b/src/blocks/scratch3_looks.js index dbf4be7a3..6addc629a 100644 --- a/src/blocks/scratch3_looks.js +++ b/src/blocks/scratch3_looks.js @@ -403,8 +403,10 @@ class Scratch3LooksBlocks { if (selectedIndex === stage.currentCostume) selectedIndex += 1; stage.setCostume(selectedIndex); } - } else if (!isNaN(requestedBackdrop) && - (typeof requestedBackdrop !== 'string' || /\d/g.test(requestedBackdrop))) { + // Try to cast the string to a number (and treat it as a costume index) + // Pure whitespace should not be treated as a number (JS casts this to 0) + // Note: isNaN will cast the string to a number before checking if it's NaN + } else if (!(isNaN(requestedBackdrop) || Cast.isWhiteSpace(requestedBackdrop))) { stage.setCostume(optZeroIndex ? Number(requestedBackdrop) : Number(requestedBackdrop) - 1); } } From 341bd8f3d38d6e30895730c81e8319e67fb6c013 Mon Sep 17 00:00:00 2001 From: jokebookservice1 Date: Wed, 10 Oct 2018 20:06:54 +0100 Subject: [PATCH 8/9] feat: Allow switching to specially named backdrops The `switch costume` block accepts special values like "next costume" and "previous costume". If you create a costume with these names, these take priority over the special values. However, the `switch backdrop` block keeps these special values for values like "next backdrop", "previous backdrop", "random backdrop". It is impossible to navigate to such a backdrop by name via block. This commit also modifies tests to allow for this. BREAKING CHANGE: specially-named backdrops can now be navigated --- src/blocks/scratch3_looks.js | 11 ++++++----- test/unit/blocks_looks.js | 8 +++++--- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/src/blocks/scratch3_looks.js b/src/blocks/scratch3_looks.js index 6addc629a..bd885dbf5 100644 --- a/src/blocks/scratch3_looks.js +++ b/src/blocks/scratch3_looks.js @@ -353,7 +353,7 @@ class Scratch3LooksBlocks { */ _setCostume (target, requestedCostume, optZeroIndex) { if (typeof requestedCostume === 'number') { - // Numbers should be treated as comments, always + // Numbers should be treated as costume indices, always target.setCostume(optZeroIndex ? requestedCostume : requestedCostume - 1); } else { const costumeIndex = target.getCostumeIndexByName(requestedCostume.toString()); @@ -386,16 +386,17 @@ class Scratch3LooksBlocks { */ _setBackdrop (stage, requestedBackdrop, optZeroIndex) { if (typeof requestedBackdrop === 'number') { + // Numbers should be treated as backdrop indices, always stage.setCostume(optZeroIndex ? requestedBackdrop : requestedBackdrop - 1); - } else if (requestedBackdrop === 'next backdrop') { - stage.setCostume(stage.currentCostume + 1); - } else if (requestedBackdrop === 'previous backdrop') { - stage.setCostume(stage.currentCostume - 1); } else { const costumeIndex = stage.getCostumeIndexByName(requestedBackdrop.toString()); if (costumeIndex !== -1) { stage.setCostume(costumeIndex); + } else if (requestedBackdrop === 'next backdrop') { + stage.setCostume(stage.currentCostume + 1); + } else if (requestedBackdrop === 'previous backdrop') { + stage.setCostume(stage.currentCostume - 1); } else if (requestedBackdrop === 'random backdrop') { const numCostumes = stage.getCostumes().length; if (numCostumes > 1) { diff --git a/test/unit/blocks_looks.js b/test/unit/blocks_looks.js index ab0552a08..779f36109 100644 --- a/test/unit/blocks_looks.js +++ b/test/unit/blocks_looks.js @@ -129,9 +129,11 @@ test('switch backdrop block runs correctly', t => { t.strictEqual(testBackdrop(['a', 'b', 'c', 'd'], 'previous backdrop', 3), 2); t.strictEqual(testBackdrop(['a', 'b', 'c', 'd'], 'next backdrop', 2), 3); - // 'previous backdrop' and 'next backdrop' can't be overriden - t.strictEqual(testBackdrop(['a', 'previous backdrop', 'c', 'd'], 'previous backdrop', 4), 3); - t.strictEqual(testBackdrop(['next backdrop', 'b', 'c', 'd'], 'next backdrop'), 2); + // 'previous backdrop', 'previous backdrop', 'random backdrop' can be overriden + // Test is deterministic since 'random backdrop' will not pick the same backdrop as currently selected + t.strictEqual(testBackdrop(['a', 'previous backdrop', 'c', 'd'], 'previous backdrop', 4), 2); + t.strictEqual(testBackdrop(['next backdrop', 'b', 'c', 'd'], 'next backdrop', 3), 1); + t.strictEqual(testBackdrop(['random backdrop', 'b', 'c', 'd'], 'random backdrop'), 1); // NaN, Infinity, and true are the first costume t.strictEqual(testBackdrop(['a', 'b', 'c', 'd'], NaN, 2), 1); From 9c128db723d78afeccf713aa1c1579c7008c5b26 Mon Sep 17 00:00:00 2001 From: jokebookservice1 Date: Wed, 17 Oct 2018 18:32:01 +0100 Subject: [PATCH 9/9] docs: Provide more/clearer comments We ensure that all code whose purpose may be confusing to grasp is commented; and we remove information that is no longer required. --- src/blocks/scratch3_looks.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/blocks/scratch3_looks.js b/src/blocks/scratch3_looks.js index bd885dbf5..a750a7cc7 100644 --- a/src/blocks/scratch3_looks.js +++ b/src/blocks/scratch3_looks.js @@ -356,6 +356,7 @@ class Scratch3LooksBlocks { // Numbers should be treated as costume indices, always target.setCostume(optZeroIndex ? requestedCostume : requestedCostume - 1); } else { + // Strings should be treated as costume names, where possible const costumeIndex = target.getCostumeIndexByName(requestedCostume.toString()); if (costumeIndex !== -1) { @@ -365,7 +366,7 @@ class Scratch3LooksBlocks { } else if (requestedCostume === 'previous costume') { target.setCostume(target.currentCostume - 1); // Try to cast the string to a number (and treat it as a costume index) - // Pure whitespace should not be treated as a number (JS casts this to 0) + // Pure whitespace should not be treated as a number // Note: isNaN will cast the string to a number before checking if it's NaN } else if (!(isNaN(requestedCostume) || Cast.isWhiteSpace(requestedCostume))) { target.setCostume(optZeroIndex ? Number(requestedCostume) : Number(requestedCostume) - 1); @@ -389,6 +390,7 @@ class Scratch3LooksBlocks { // Numbers should be treated as backdrop indices, always stage.setCostume(optZeroIndex ? requestedBackdrop : requestedBackdrop - 1); } else { + // Strings should be treated as backdrop names where possible const costumeIndex = stage.getCostumeIndexByName(requestedBackdrop.toString()); if (costumeIndex !== -1) { @@ -398,6 +400,8 @@ class Scratch3LooksBlocks { } else if (requestedBackdrop === 'previous backdrop') { stage.setCostume(stage.currentCostume - 1); } else if (requestedBackdrop === 'random backdrop') { + // Don't pick the current backdrop, so that the block + // will always have an observable effect. const numCostumes = stage.getCostumes().length; if (numCostumes > 1) { let selectedIndex = Math.floor(Math.random() * (numCostumes - 1)); @@ -405,7 +409,7 @@ class Scratch3LooksBlocks { stage.setCostume(selectedIndex); } // Try to cast the string to a number (and treat it as a costume index) - // Pure whitespace should not be treated as a number (JS casts this to 0) + // Pure whitespace should not be treated as a number // Note: isNaN will cast the string to a number before checking if it's NaN } else if (!(isNaN(requestedBackdrop) || Cast.isWhiteSpace(requestedBackdrop))) { stage.setCostume(optZeroIndex ? Number(requestedBackdrop) : Number(requestedBackdrop) - 1);