From 3019d05181594360b0838775b9916abaf1be2f6d Mon Sep 17 00:00:00 2001 From: Paul Kaplan Date: Thu, 26 Apr 2018 13:03:57 -0400 Subject: [PATCH 1/2] Fix issue where say bubble flickers during conversation. The fix for this involves something of hack where we decide whether to use the optimization that prevents "typewriter" style projects (say for 0.05s in a loop) based on the amount of time the block is going to run for. A full fix of this can only be achieved if the text bubble renders synchronously. --- src/blocks/scratch3_looks.js | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/src/blocks/scratch3_looks.js b/src/blocks/scratch3_looks.js index b561682b..cb124c23 100644 --- a/src/blocks/scratch3_looks.js +++ b/src/blocks/scratch3_looks.js @@ -58,6 +58,14 @@ class Scratch3LooksBlocks { return 'Scratch.looks'; } + /** + * Max block time we use the "soft clear" optimization to prevent flickering in "typewriter" projects. + * @type {string} + */ + static get MAX_SOFT_CLEAR_TIME () { + return 0.02; // seconds + } + /** * @param {Target} target - collect bubble state for this target. Probably, but not necessarily, a RenderedTarget. * @returns {BubbleState} the mutable bubble state associated with that target. This will be created if necessary. @@ -282,12 +290,17 @@ class Scratch3LooksBlocks { this.say(args, util); const target = util.target; const usageId = this._getBubbleState(target).usageId; + const secs = args.SECS; return new Promise(resolve => { this._bubbleTimeout = setTimeout(() => { this._bubbleTimeout = null; // Clear say bubble if it hasn't been changed and proceed. if (this._getBubbleState(target).usageId === usageId) { - this._updateBubble(target, 'say', ''); + if (secs < this.MAX_SOFT_CLEAR_TIME) { + this._updateBubble(target, 'say', ''); + } else { + this._onTargetWillExit(target); + } } resolve(); }, 1000 * args.SECS); @@ -302,12 +315,17 @@ class Scratch3LooksBlocks { this.think(args, util); const target = util.target; const usageId = this._getBubbleState(target).usageId; + const secs = args.SECS; return new Promise(resolve => { this._bubbleTimeout = setTimeout(() => { this._bubbleTimeout = null; // Clear think bubble if it hasn't been changed and proceed. if (this._getBubbleState(target).usageId === usageId) { - this._updateBubble(target, 'think', ''); + if (secs < this.MAX_SOFT_CLEAR_TIME) { + this._updateBubble(target, 'think', ''); + } else { + this._onTargetWillExit(target); + } } resolve(); }, 1000 * args.SECS); From 3f97ccc8ce250846ea675e2dafdb3f1a80f02842 Mon Sep 17 00:00:00 2001 From: Paul Kaplan Date: Mon, 30 Apr 2018 13:40:36 -0400 Subject: [PATCH 2/2] Remove the "optimization" for --- src/blocks/scratch3_looks.js | 45 +++++------------------------------- 1 file changed, 6 insertions(+), 39 deletions(-) diff --git a/src/blocks/scratch3_looks.js b/src/blocks/scratch3_looks.js index ba3e16f0..bd273fd4 100644 --- a/src/blocks/scratch3_looks.js +++ b/src/blocks/scratch3_looks.js @@ -7,8 +7,6 @@ const uid = require('../util/uid'); * @typedef {object} BubbleState - the bubble state associated with a particular target. * @property {Boolean} onSpriteRight - tracks whether the bubble is right or left of the sprite. * @property {?int} drawableId - the ID of the associated bubble Drawable, null if none. - * @property {Boolean} drawableVisible - false if drawable has been hidden by blank text. - * See _renderBubble for explanation of this optimization. * @property {string} text - the text of the bubble. * @property {string} type - the type of the bubble, "say" or "think" * @property {?string} usageId - ID indicating the most recent usage of the say/think bubble. @@ -43,7 +41,6 @@ class Scratch3LooksBlocks { static get DEFAULT_BUBBLE_STATE () { return { drawableId: null, - drawableVisible: true, onSpriteRight: true, skinId: null, text: '', @@ -60,14 +57,6 @@ class Scratch3LooksBlocks { return 'Scratch.looks'; } - /** - * Max block time we use the "soft clear" optimization to prevent flickering in "typewriter" projects. - * @type {string} - */ - static get MAX_SOFT_CLEAR_TIME () { - return 0.02; // seconds - } - /** * @param {Target} target - collect bubble state for this target. Probably, but not necessarily, a RenderedTarget. * @returns {BubbleState} the mutable bubble state associated with that target. This will be created if necessary. @@ -106,7 +95,6 @@ class Scratch3LooksBlocks { this.runtime.renderer.destroySkin(bubbleState.skinId); bubbleState.drawableId = null; bubbleState.skinId = null; - bubbleState.drawableVisible = true; // Reset back to default value this.runtime.requestRedraw(); } target.removeListener(RenderedTarget.EVENT_TARGET_VISUAL_CHANGE, this._onTargetChanged); @@ -181,27 +169,16 @@ class Scratch3LooksBlocks { if (!this.runtime.renderer) return; const bubbleState = this._getBubbleState(target); - const {drawableVisible, type, text, onSpriteRight} = bubbleState; + const {type, text, onSpriteRight} = bubbleState; - // Remove the bubble if target is not visible, or text is being set to blank - // without being initialized. See comment below about blank text optimization. - if (!target.visible || (text === '' && !bubbleState.skinId)) { + // Remove the bubble if target is not visible, or text is being set to blank. + if (!target.visible || text === '') { this._onTargetWillExit(target); return; } if (bubbleState.skinId) { - // Optimization: if text is set to blank, hide the drawable instead of - // getting rid of it. This prevents flickering in "typewriter" projects - if ((text === '' && drawableVisible) || (text !== '' && !drawableVisible)) { - bubbleState.drawableVisible = text !== ''; - this.runtime.renderer.updateDrawableProperties(bubbleState.drawableId, { - visible: bubbleState.drawableVisible - }); - } - if (bubbleState.drawableVisible) { - this.runtime.renderer.updateTextSkin(bubbleState.skinId, type, text, onSpriteRight, [0, 0]); - } + this.runtime.renderer.updateTextSkin(bubbleState.skinId, type, text, onSpriteRight, [0, 0]); } else { target.addListener(RenderedTarget.EVENT_TARGET_VISUAL_CHANGE, this._onTargetChanged); @@ -292,17 +269,12 @@ class Scratch3LooksBlocks { this.say(args, util); const target = util.target; const usageId = this._getBubbleState(target).usageId; - const secs = args.SECS; return new Promise(resolve => { this._bubbleTimeout = setTimeout(() => { this._bubbleTimeout = null; // Clear say bubble if it hasn't been changed and proceed. if (this._getBubbleState(target).usageId === usageId) { - if (secs < this.MAX_SOFT_CLEAR_TIME) { - this._updateBubble(target, 'say', ''); - } else { - this._onTargetWillExit(target); - } + this._onTargetWillExit(target); } resolve(); }, 1000 * args.SECS); @@ -317,17 +289,12 @@ class Scratch3LooksBlocks { this.think(args, util); const target = util.target; const usageId = this._getBubbleState(target).usageId; - const secs = args.SECS; return new Promise(resolve => { this._bubbleTimeout = setTimeout(() => { this._bubbleTimeout = null; // Clear think bubble if it hasn't been changed and proceed. if (this._getBubbleState(target).usageId === usageId) { - if (secs < this.MAX_SOFT_CLEAR_TIME) { - this._updateBubble(target, 'think', ''); - } else { - this._onTargetWillExit(target); - } + this._onTargetWillExit(target); } resolve(); }, 1000 * args.SECS);