Update looks implementation

src/blocks/scratch3_looks.js

@@ -21,10 +21,11 @@ class Scratch3LooksBlocks {
 
         this._onTargetMoved = this._onTargetMoved.bind(this);
         this._onResetBubbles = this._onResetBubbles.bind(this);
+        this._onTargetWillExit = this._onTargetWillExit.bind(this);
 
         // Reset all bubbles on start/stop
-        this.runtime.on('PROJECT_RUN_START', this._onResetBubbles);
-        this.runtime.on('PROJECT_RUN_STOP', this._onResetBubbles);
+        this.runtime.on('PROJECT_STOP_ALL', this._onResetBubbles);
+        this.runtime.on('targetWasRemoved', this._onTargetWillExit);
     }
 
     /**
@@ -74,29 +75,54 @@ class Scratch3LooksBlocks {
     }
 
     /**
-     * Handle a target which has moved. This only fires when the bubble is visible.
+     * Handle a target which has moved.
      * @param {RenderedTarget} target - the target which has moved.
      * @private
      */
     _onTargetMoved (target) {
         const bubbleState = this._getBubbleState(target);
 
-        if (bubbleState.drawableId) {
+        if (bubbleState.drawableId && bubbleState.visible) {
             this._checkBubbleBounds(target);
             this._positionBubble(target);
         }
     }
 
-    _onResetBubbles () {
-        // for (let n = 0; n < this.runtime.targets.length; n++) {
-        //     const target = this.runtime.targets[n];
-        //     const bubbleState = this._getBubbleState(target);
-        //     if (bubbleState.drawableId) {
-        //         this._clearBubble(target);
-        //     }
-        // }
+    /**
+     * Handle a target which has moved.
+     * @param {RenderedTarget} target - the target which has moved.
+     * @private
+     */
+    _onTargetWillExit (target) {
+        const bubbleState = this._getBubbleState(target);
+        if (bubbleState.drawableId) {
+            this.runtime.renderer.destroyDrawable(bubbleState.drawableId);
+        }
+        if (bubbleState.skinId) {
+            this.runtime.renderer.destroySkin(bubbleState.skinId);
+        }
+
     }
 
+    /**
+     * Handle project start/stop by clearing all visible bubbles.
+     * @private
+     */
+    _onResetBubbles () {
+        for (let n = 0; n < this.runtime.targets.length; n++) {
+            const target = this.runtime.targets[n];
+            const bubbleState = this._getBubbleState(target);
+            if (bubbleState.drawableId) {
+                this._clearBubble(target);
+            }
+        }
+    }
+
+    /**
+     * Position the bubble of a target.
+     * @param {!Target} target Target whose bubble needs positioning.
+     * @private
+     */
     _positionBubble (target) {
         const bubbleState = this._getBubbleState(target);
         const [bubbleWidth, bubbleHeight] = this.runtime.renderer.getSkinSize(bubbleState.drawableId);
@@ -113,6 +139,12 @@ class Scratch3LooksBlocks {
         this.runtime.requestRedraw();
     }
 
+    /**
+     * Check whether a bubble needs to be flipped. If so, flip the `onSpriteRight` state
+     * and call the bubble render again.
+     * @param {!Target} target Target whose bubble needs positioning.
+     * @private
+     */
     _checkBubbleBounds (target) {
         const bubbleState = this._getBubbleState(target);
         const [bubbleWidth, _] = this.runtime.renderer.getSkinSize(bubbleState.drawableId);
@@ -127,6 +159,13 @@ class Scratch3LooksBlocks {
         }
     }
 
+    /**
+     * Create a visible bubble for a target. If a bubble exists for the target,
+     * just set it to visible and update the type/text. Otherwise create a new
+     * bubble and update the relevant custom state.
+     * @param {!Target} target Target who needs a bubble.
+     * @private
+     */
     _renderBubble (target) {
         const bubbleState = this._getBubbleState(target);
         const {type, text, onSpriteRight} = bubbleState;
@@ -163,29 +202,38 @@ class Scratch3LooksBlocks {
         this._positionBubble(target);
     }
 
+    /**
+     * The entry point for say/think blocks. Clears existing bubble if the text is empty.
+     * Set the bubble custom state and then call _renderBubble.
+     * @param {!Target} target Target that say/think blocks are being called on.
+     * @param {!string} type Either "say" or "think"
+     * @param {!string} text The text for the bubble, empty string clears the bubble.
+     * @private
+     */
     _updateBubble (target, type, text) {
-        // Say/think empty string should clear any bubble
         if (text === '') {
             this._clearBubble(target);
         } else {
             const bubbleState = this._getBubbleState(target);
             bubbleState.type = type;
             bubbleState.text = text;
-
             this._renderBubble(target);
         }
     }
 
+    /**
+     * Hide the bubble for a given target.
+     * @param {!Target} target Target that say/think blocks are being called on.
+     * @private
+     */
     _clearBubble (target) {
         const bubbleState = this._getBubbleState(target);
         bubbleState.visible = false;
-        this.runtime.renderer.updateDrawableProperties(bubbleState.drawableId, {
-            visible: bubbleState.visible
-        });
-
-        // @TODO is this safe? It could have been already removed?
-        target.removeListener(RenderedTarget.EVENT_TARGET_MOVED, this._onTargetMoved);
-
+        if (bubbleState.drawableId) {
+            this.runtime.renderer.updateDrawableProperties(bubbleState.drawableId, {
+                visible: bubbleState.visible
+            });
+        }
         this.runtime.requestRedraw();
     }
 

src/engine/runtime.js

@@ -280,7 +280,8 @@ class Runtime extends EventEmitter {
     }
 
     /**
-     * Event name for glowing the green flag
+     * Event name when threads start running.
+     * Used by the UI to indicate running status.
      * @const {string}
      */
     static get PROJECT_RUN_START () {
@@ -288,13 +289,23 @@ class Runtime extends EventEmitter {
     }
 
     /**
-     * Event name for unglowing the green flag
+     * Event name when threads stop running
+     * Used by the UI to indicate not-running status.
      * @const {string}
      */
     static get PROJECT_RUN_STOP () {
         return 'PROJECT_RUN_STOP';
     }
 
+    /**
+     * Event name for project being stopped or restarted by the user.
+     * Used by blocks that need to reset state.
+     * @const {string}
+     */
+    static get PROJECT_STOP_ALL () {
+        return 'PROJECT_STOP_ALL';
+    }
+
     /**
      * Event name for visual value report.
      * @const {string}
@@ -905,6 +916,9 @@ class Runtime extends EventEmitter {
      * Stop "everything."
      */
     stopAll () {
+        // Emit stop event to allow blocks to clean up any state.
+        this.emit(Runtime.PROJECT_STOP_ALL);
+
         // Dispose all clones.
         const newTargets = [];
         for (let i = 0; i < this.targets.length; i++) {
@@ -1227,6 +1241,16 @@ class Runtime extends EventEmitter {
         this.emit('targetWasCreated', newTarget, sourceTarget);
     }
 
+    /**
+     * Report that a new target has been created, possibly by cloning an existing target.
+     * @param {Target} newTarget - the newly created target.
+     * @param {Target} [sourceTarget] - the target used as a source for the new clone, if any.
+     * @fires Runtime#targetWasCreated
+     */
+    fireTargetWasRemoved (newTarget, sourceTarget) {
+        this.emit('targetWasRemoved', newTarget, sourceTarget);
+    }
+
     /**
      * Get a target representing the Scratch stage, if one exists.
      * @return {?Target} The target, if found.

src/sprites/sprite.js

@@ -71,6 +71,7 @@ class Sprite {
      * @param {!RenderedTarget} clone - the clone to be removed.
      */
     removeClone (clone) {
+        this.runtime.fireTargetWasRemoved(clone);
         const cloneIndex = this.clones.indexOf(clone);
         if (cloneIndex >= 0) {
             this.clones.splice(cloneIndex, 1);