From fe6b07accaf737eecd433546f1b7e9cca40ab687 Mon Sep 17 00:00:00 2001
From: adroitwhiz <adroitwhiz@protonmail.com>
Date: Wed, 29 May 2019 02:14:38 -0400
Subject: [PATCH] Consistently format text bubbles

---
 src/blocks/scratch3_looks.js | 36 +++++++++++++++++++++++--------
 test/unit/blocks_looks.js    | 41 ++++++++++++++++++++++++++----------
 2 files changed, 57 insertions(+), 20 deletions(-)

diff --git a/src/blocks/scratch3_looks.js b/src/blocks/scratch3_looks.js
index 3be4fcfc5..3b65c35e3 100644
--- a/src/blocks/scratch3_looks.js
+++ b/src/blocks/scratch3_looks.js
@@ -233,6 +233,29 @@ class Scratch3LooksBlocks {
         this._positionBubble(target);
     }
 
+    /**
+     * Properly format text for a text bubble.
+     * @param {string} text The text to be formatted
+     * @return {string} The formatted text
+     * @private
+     */
+    _formatBubbleText (text) {
+        if (text === '') return text;
+
+        // Non-integers should be rounded to 2 decimal places (no more, no less), unless they're small enough that
+        // rounding would display them as 0.00. This matches 2.0's behavior:
+        // https://github.com/LLK/scratch-flash/blob/2e4a402ceb205a042887f54b26eebe1c2e6da6c0/src/scratch/ScratchSprite.as#L579-L585
+        if (typeof text === 'number' &&
+            Math.abs(text) >= 0.01 && text % 1 !== 0) {
+            text = text.toFixed(2);
+        }
+
+        // Limit the length of the string.
+        text = String(text).substr(0, Scratch3LooksBlocks.SAY_BUBBLE_LIMIT);
+
+        return text;
+    }
+
     /**
      * The entry point for say/think blocks. Clears existing bubble if the text is empty.
      * Set the bubble custom state and then call _renderBubble.
@@ -244,7 +267,7 @@ class Scratch3LooksBlocks {
     _updateBubble (target, type, text) {
         const bubbleState = this._getBubbleState(target);
         bubbleState.type = type;
-        bubbleState.text = text;
+        bubbleState.text = this._formatBubbleText(text);
         bubbleState.usageId = uid();
         this._renderBubble(target);
     }
@@ -300,12 +323,7 @@ class Scratch3LooksBlocks {
 
     say (args, util) {
         // @TODO in 2.0 calling say/think resets the right/left bias of the bubble
-        let message = args.MESSAGE;
-        if (typeof message === 'number') {
-            message = parseFloat(message.toFixed(2));
-        }
-        message = String(message).substr(0, Scratch3LooksBlocks.SAY_BUBBLE_LIMIT);
-        this.runtime.emit('SAY', util.target, 'say', message);
+        this.runtime.emit('SAY', util.target, 'say', args.MESSAGE);
     }
 
     sayforsecs (args, util) {
@@ -325,7 +343,7 @@ class Scratch3LooksBlocks {
     }
 
     think (args, util) {
-        this._updateBubble(util.target, 'think', String(args.MESSAGE).substr(0, Scratch3LooksBlocks.SAY_BUBBLE_LIMIT));
+        this.runtime.emit('SAY', util.target, 'think', args.MESSAGE);
     }
 
     thinkforsecs (args, util) {
@@ -418,7 +436,7 @@ class Scratch3LooksBlocks {
                     const lowerBound = 0;
                     const upperBound = numCostumes - 1;
                     const costumeToExclude = stage.currentCostume;
-                    
+
                     const nextCostume = MathUtil.inclusiveRandIntWithout(lowerBound, upperBound, costumeToExclude);
 
                     stage.setCostume(nextCostume);
diff --git a/test/unit/blocks_looks.js b/test/unit/blocks_looks.js
index 599e7e49a..0a772f04e 100644
--- a/test/unit/blocks_looks.js
+++ b/test/unit/blocks_looks.js
@@ -15,7 +15,9 @@ const util = {
                 {name: 'second name'},
                 {name: 'third name'}
             ]
-        }
+        },
+        _customState: {},
+        getCustomState: () => util.target._customState
     }
 };
 
@@ -159,7 +161,7 @@ test('switch backdrop block runs correctly', t => {
     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();
 });
 
@@ -195,21 +197,38 @@ test('getBackdropNumberName can return costume name', t => {
     t.end();
 });
 
-test('numbers should be rounded to two decimals in say', t => {
+test('numbers should be rounded properly in say/think', t => {
     const rt = new Runtime();
     const looks = new Looks(rt);
 
-    const args = {MESSAGE: 3.14159};
-    const expectedSayString = '3.14';
+    let expectedSayString;
 
-    rt.removeAllListeners('SAY'); // Prevent say blocks from executing
-
-    rt.addListener('SAY', (target, type, sayString) => {
-        t.strictEqual(sayString, expectedSayString);
-        t.end();
+    rt.addListener('SAY', () => {
+        const bubbleState = util.target.getCustomState(Looks.STATE_KEY);
+        t.strictEqual(bubbleState.text, expectedSayString);
     });
 
-    looks.say(args, util);
+    expectedSayString = '3.14';
+    looks.say({MESSAGE: 3.14159}, util, 'say bubble should round to 2 decimal places');
+    looks.think({MESSAGE: 3.14159}, util, 'think bubble should round to 2 decimal places');
+
+    expectedSayString = '3';
+    looks.say({MESSAGE: 3}, util, 'say bubble should not add decimal places to integers');
+    looks.think({MESSAGE: 3}, util, 'think bubble should not add decimal places to integers');
+
+    expectedSayString = '3.10';
+    looks.say({MESSAGE: 3.1}, util, 'say bubble should round to 2 decimal places, even if only 1 is needed');
+    looks.think({MESSAGE: 3.1}, util, 'think bubble should round to 2 decimal places, even if only 1 is needed');
+
+    expectedSayString = '0.00125';
+    looks.say({MESSAGE: 0.00125}, util, 'say bubble should not round if it would display small numbers as 0');
+    looks.think({MESSAGE: 0.00125}, util, 'think bubble should not round if it would display small numbers as 0');
+
+    expectedSayString = '1.99999';
+    looks.say({MESSAGE: '1.99999'}, util, 'say bubble should not round strings');
+    looks.think({MESSAGE: '1.99999'}, util, 'think bubble should not round strings');
+
+    t.end();
 });
 
 test('clamp graphic effects', t => {