Merge pull request #784 from fsih/perSpriteMonitors

Execute monitors on a given target ID when block is sprite-specific

src/blocks/scratch3_looks.js

@@ -242,6 +242,15 @@ class Scratch3LooksBlocks {
         };
     }
 
+    getMonitored () {
+        return {
+            looks_size: {isSpriteSpecific: true},
+            looks_costumeorder: {isSpriteSpecific: true},
+            looks_backdroporder: {},
+            looks_backdropname: {}
+        };
+    }
+
     say (args, util) {
         // @TODO in 2.0 calling say/think resets the right/left bias of the bubble
         this._updateBubble(util.target, 'say', String(args.MESSAGE));

src/blocks/scratch3_motion.js

@@ -38,6 +38,14 @@ class Scratch3MotionBlocks {
         };
     }
 
+    getMonitored () {
+        return {
+            motion_xposition: {isSpriteSpecific: true},
+            motion_yposition: {isSpriteSpecific: true},
+            motion_direction: {isSpriteSpecific: true}
+        };
+    }
+
     moveSteps (args, util) {
         const steps = Cast.toNumber(args.STEPS);
         const radians = MathUtil.degToRad(90 - util.target.direction);

src/blocks/scratch3_sensing.js

@@ -49,6 +49,16 @@ class Scratch3SensingBlocks {
         };
     }
 
+    getMonitored () {
+        return {
+            sensing_answer: {},
+            sensing_loudness: {},
+            sensing_timer: {},
+            sensing_of: {},
+            sensing_current: {}
+        };
+    }
+
     _onAnswer (answer) {
         this._answer = answer;
         const questionObj = this._questionList.shift();

src/blocks/scratch3_sound.js

@@ -103,6 +103,12 @@ class Scratch3SoundBlocks {
         };
     }
 
+    getMonitored () {
+        return {
+            sound_volume: {}
+        };
+    }
+
     playSound (args, util) {
         const index = this._getSoundIndex(args.SOUND_MENU, util);
         if (index >= 0) {

src/engine/blocks.js

@@ -381,12 +381,19 @@ class Blocks {
             break;
         case 'checkbox':
             block.isMonitored = args.value;
+            if (optRuntime) {
+                const isSpriteSpecific = optRuntime.monitorBlockInfo.hasOwnProperty(block.opcode) &&
+                    optRuntime.monitorBlockInfo[block.opcode].isSpriteSpecific;
+                block.targetId = isSpriteSpecific ? optRuntime.getEditingTarget().id : null;
+            }
             if (optRuntime && wasMonitored && !block.isMonitored) {
                 optRuntime.requestRemoveMonitor(block.id);
             } else if (optRuntime && !wasMonitored && block.isMonitored) {
                 optRuntime.requestAddMonitor(MonitorRecord({
                     // @todo(vm#564) this will collide if multiple sprites use same block
                     id: block.id,
+                    targetId: block.targetId,
+                    spriteName: block.targetId ? optRuntime.getTargetById(block.targetId).getName() : null,
                     opcode: block.opcode,
                     params: this._getBlockParams(block),
                     // @todo(vm#565) for numerical values with decimals, some countries use comma
@@ -464,8 +471,8 @@ class Blocks {
     runAllMonitored (runtime) {
         Object.keys(this._blocks).forEach(blockId => {
             if (this.getBlock(blockId).isMonitored) {
-                // @todo handle specific targets (e.g. apple x position)
+                const targetId = this.getBlock(blockId).targetId;
-                runtime.addMonitorScript(blockId);
+                runtime.addMonitorScript(blockId, targetId ? runtime.getTargetById(targetId) : null);
             }
         });
     }

src/engine/execute.js

@@ -80,8 +80,14 @@ const handleReport = function (
                 sequencer.runtime.visualReport(currentBlockId, resolvedValue);
             }
             if (thread.updateMonitor) {
+                const targetId = sequencer.runtime.monitorBlocks.getBlock(currentBlockId).targetId;
+                if (targetId && !sequencer.runtime.getTargetById(targetId)) {
+                    // Target no longer exists
+                    return;
+                }
                 sequencer.runtime.requestUpdateMonitor(Map({
                     id: currentBlockId,
+                    spriteName: targetId ? sequencer.runtime.getTargetById(targetId).getName() : null,
                     value: String(resolvedValue)
                 }));
             }

src/engine/monitor-record.js

@@ -2,6 +2,10 @@ const {Record} = require('immutable');
 
 const MonitorRecord = Record({
     id: null,
+    /** Present only if the monitor is sprite-specific, such as x position */
+    spriteName: null,
+    /** Present only if the monitor is sprite-specific, such as x position */
+    targetId: null,
     opcode: null,
     value: null,
     params: null

src/engine/runtime.js

@@ -195,6 +195,13 @@ class Runtime extends EventEmitter {
          */
         this._refreshTargets = false;
 
+        /**
+         * Map to look up all monitor block information by opcode.
+         * @type {object}
+         * @private
+         */
+        this.monitorBlockInfo = {};
+
         /**
          * Ordered map of all monitors, which are MonitorReporter objects.
          */
@@ -423,6 +430,10 @@ class Runtime extends EventEmitter {
                         }
                     }
                 }
+                // Collect monitored from package.
+                if (packageObject.getMonitored) {
+                    this.monitorBlockInfo = Object.assign({}, this.monitorBlockInfo, packageObject.getMonitored());
+                }
             }
         }
     }
@@ -870,7 +881,7 @@ class Runtime extends EventEmitter {
     /**
      * Enqueue a script that when finished will update the monitor for the block.
      * @param {!string} topBlockId ID of block that starts the script.
-     * @param {?string} optTarget target ID for target to run script on. If not supplied, uses editing target.
+     * @param {?Target} optTarget target Target to run script on. If not supplied, uses editing target.
      */
     addMonitorScript (topBlockId, optTarget) {
         if (!optTarget) optTarget = this._editingTarget;
@@ -1321,7 +1332,7 @@ class Runtime extends EventEmitter {
      * @param {!MonitorRecord} monitor Monitor to add.
      */
     requestAddMonitor (monitor) {
-        this._monitorState = this._monitorState.set(monitor.id, monitor);
+        this._monitorState = this._monitorState.set(monitor.get('id'), monitor);
     }
 
     /**
@@ -1332,9 +1343,10 @@ class Runtime extends EventEmitter {
      *     the old monitor will keep its old value.
      */
     requestUpdateMonitor (monitor) {
-        if (this._monitorState.has(monitor.get('id'))) {
+        const id = monitor.get('id');
+        if (this._monitorState.has(id)) {
             this._monitorState =
-                this._monitorState.set(monitor.get('id'), this._monitorState.get(monitor.get('id')).merge(monitor));
+                this._monitorState.set(id, this._monitorState.get(id).merge(monitor));
         }
     }
 
@@ -1347,6 +1359,15 @@ class Runtime extends EventEmitter {
         this._monitorState = this._monitorState.delete(monitorId);
     }
 
+    /**
+     * Removes all monitors with the given target ID from the state. Does nothing if
+     * the monitor already does not exist in the state.
+     * @param {!string} targetId Remove all monitors with given target ID.
+     */
+    requestRemoveMonitorByTargetId (targetId) {
+        this._monitorState = this._monitorState.filterNot(value => value.targetId === targetId);
+    }
+
     /**
      * Get a target by its id.
      * @param {string} targetId Id of target to find.

src/virtual-machine.js

@@ -484,6 +484,7 @@ class VirtualMachine extends EventEmitter {
             if (!sprite) {
                 throw new Error('No sprite associated with this target.');
             }
+            this.runtime.requestRemoveMonitorByTargetId(targetId);
             const currentEditingTarget = this.editingTarget;
             for (let i = 0; i < sprite.clones.length; i++) {
                 const clone = sprite.clones[i];