Merge pull request #1517 from joker314/costume-compatibility

Make "switch costume" and "switch backdrop" blocks compatible with 2.0

src/blocks/scratch3_looks.js

@@ -337,65 +337,96 @@ 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.<!Thread>} Any threads started by this switch.
      */
-    _setCostumeOrBackdrop (target,
+    _setCostume (target, requestedCostume, optZeroIndex) {
-        requestedCostume, optZeroIndex) {
         if (typeof requestedCostume === 'number') {
-            target.setCostume(optZeroIndex ?
+            // Numbers should be treated as costume indices, always
-                requestedCostume : requestedCostume - 1);
+            target.setCostume(optZeroIndex ? requestedCostume : requestedCostume - 1);
         } else {
-            const costumeIndex = target.getCostumeIndexByName(requestedCostume);
+            // Strings should be treated as costume names, where possible
-            if (costumeIndex > -1) {
+            const costumeIndex = target.getCostumeIndexByName(requestedCostume.toString());
+
+            if (costumeIndex !== -1) {
                 target.setCostume(costumeIndex);
-            } else if (requestedCostume === 'previous costume' ||
+            } else if (requestedCostume === 'next costume') {
-                       requestedCostume === 'previous backdrop') {
-                target.setCostume(target.currentCostume - 1);
-            } else if (requestedCostume === 'next costume' ||
-                       requestedCostume === 'next backdrop') {
                 target.setCostume(target.currentCostume + 1);
-            } else if (requestedCostume === 'random backdrop') {
+            } else if (requestedCostume === 'previous costume') {
-                const numCostumes = target.getCostumes().length;
+                target.setCostume(target.currentCostume - 1);
-                if (numCostumes > 1) {
+            // Try to cast the string to a number (and treat it as a costume index)
-                    let selectedIndex = Math.floor(Math.random() * (numCostumes - 1));
+            // Pure whitespace should not be treated as a number
-                    if (selectedIndex === target.currentCostume) selectedIndex += 1;
+            // Note: isNaN will cast the string to a number before checking if it's NaN
-                    target.setCostume(selectedIndex);
+            } else if (!(isNaN(requestedCostume) || Cast.isWhiteSpace(requestedCostume))) {
-                }
+                target.setCostume(optZeroIndex ? Number(requestedCostume) : Number(requestedCostume) - 1);
-            } else {
-                const forcedNumber = Number(requestedCostume);
-                if (!isNaN(forcedNumber)) {
-                    target.setCostume(optZeroIndex ?
-                        forcedNumber : forcedNumber - 1);
-                }
             }
         }
-        if (target === this.runtime.getTargetForStage()) {
+
-            // Target is the stage - start hats.
+        // Per 2.0, 'switch costume' can't start threads even in the Stage.
-            const newName = target.getCostumes()[target.currentCostume].name;
-            return this.runtime.startHats('event_whenbackdropswitchesto', {
-                BACKDROP: newName
-            });
-        }
         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.<!Thread>} Any threads started by this switch.
+     */
+    _setBackdrop (stage, requestedBackdrop, optZeroIndex) {
+        if (typeof requestedBackdrop === 'number') {
+            // 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) {
+                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') {
+                // 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));
+                    if (selectedIndex === stage.currentCostume) selectedIndex += 1;
+                    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
+            // 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);
+            }
+        }
+
+        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) {
@@ -403,7 +434,7 @@ class Scratch3LooksBlocks {
         if (!util.stackFrame.startedThreads) {
             // No - switch the backdrop.
             util.stackFrame.startedThreads = (
-                this._setCostumeOrBackdrop(
+                this._setBackdrop(
                     this.runtime.getTargetForStage(),
                     args.BACKDROP
                 )
@@ -438,7 +469,7 @@ class Scratch3LooksBlocks {
 
     nextBackdrop () {
         const stage = this.runtime.getTargetForStage();
-        this._setCostumeOrBackdrop(
+        this._setBackdrop(
             stage, stage.currentCostume + 1, true
         );
     }

src/sprites/rendered-target.js

@@ -471,6 +471,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
         );

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,144 @@ const fakeRuntime = {
 };
 const blocks = new Looks(fakeRuntime);
 
+/**
+ * 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);
+
+    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.
+};
+
+
+/**
+ * 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);
+
+    // 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);
+    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 => {
+    // 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', '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);
+    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'};