Merge pull request #574 from fsih/immutableState

Only send update monitor events when changes happen

package.json

@@ -37,6 +37,7 @@
     "got": "5.7.1",
     "highlightjs": "^9.8.0",
     "htmlparser2": "3.9.2",
+    "immutable": "3.8.1",
     "json": "^9.0.4",
     "lodash.defaultsdeep": "4.6.0",
     "minilog": "3.1.0",

src/engine/blocks.js

@@ -1,6 +1,7 @@
 const adapter = require('./adapter');
 const mutationAdapter = require('./mutation-adapter');
 const xmlEscape = require('../util/xml-escape');
+const MonitorRecord = require('./monitor-record');
 
 /**
  * @fileoverview
@@ -293,14 +294,14 @@ class Blocks {
             if (optRuntime && wasMonitored && !block.isMonitored) {
                 optRuntime.requestRemoveMonitor(block.id);
             } else if (optRuntime && !wasMonitored && block.isMonitored) {
-                optRuntime.requestAddMonitor({
+                optRuntime.requestAddMonitor(MonitorRecord({
                     // @todo(vm#564) this will collide if multiple sprites use same block
                     id: block.id,
                     opcode: block.opcode,
                     params: this._getBlockParams(block),
                     // @todo(vm#565) for numerical values with decimals, some countries use comma
                     value: ''
-                });
+                }));
             }
             break;
         }

src/engine/execute.js

@@ -1,5 +1,6 @@
 const log = require('../util/log');
 const Thread = require('./thread');
+const {Map} = require('immutable');
 
 /**
  * Utility function to determine if a value is a Promise.
@@ -97,10 +98,10 @@ const execute = function (sequencer, thread) {
                     runtime.visualReport(currentBlockId, resolvedValue);
                 }
                 if (thread.updateMonitor) {
-                    runtime.requestUpdateMonitor({
+                    runtime.requestUpdateMonitor(Map({
                         id: currentBlockId,
                         value: String(resolvedValue)
-                    });
+                    }));
                 }
             }
             // Finished any yields.

src/engine/monitor-record.js

@@ -0,0 +1,10 @@
+const {Record} = require('immutable');
+
+const MonitorRecord = Record({
+    id: null,
+    opcode: null,
+    value: null,
+    params: null
+});
+
+module.exports = MonitorRecord;

src/engine/runtime.js

@@ -2,6 +2,7 @@ const EventEmitter = require('events');
 const Sequencer = require('./sequencer');
 const Blocks = require('./blocks');
 const Thread = require('./thread');
+const {OrderedMap} = require('immutable');
 
 // Virtual I/O devices.
 const Clock = require('../io/clock');
@@ -113,9 +114,14 @@ class Runtime extends EventEmitter {
         this._refreshTargets = false;
 
         /**
-         * List of all monitors.
+         * Ordered map of all monitors, which are MonitorReporter objects.
          */
-        this._monitorState = {};
+        this._monitorState = OrderedMap({});
+
+        /**
+         * Monitor state from last tick
+         */
+        this._prevMonitorState = OrderedMap({});
 
         /**
          * Whether the project is in "turbo mode."
@@ -701,10 +707,10 @@ class Runtime extends EventEmitter {
             this._refreshTargets = false;
         }
 
-        // @todo(vm#570) only emit if monitors has changed since last time.
-        this.emit(Runtime.MONITORS_UPDATE,
-            Object.keys(this._monitorState).map(key => this._monitorState[key])
-        );
+        if (!this._prevMonitorState.equals(this._monitorState)) {
+            this.emit(Runtime.MONITORS_UPDATE, this._monitorState);
+            this._prevMonitorState = this._monitorState;
+        }
     }
 
     /**
@@ -876,30 +882,33 @@ class Runtime extends EventEmitter {
     /**
      * Add a monitor to the state. If the monitor already exists in the state,
      * overwrites it.
-     * @param {!object} monitor Monitor to add.
+     * @param {!MonitorRecord} monitor Monitor to add.
      */
     requestAddMonitor (monitor) {
-        this._monitorState[monitor.id] = monitor;
+        this._monitorState = this._monitorState.set(monitor.id, monitor);
     }
 
     /**
      * Update a monitor in the state. Does nothing if the monitor does not already
      * exist in the state.
-     * @param {!object} monitor Monitor to update.
+     * @param {!Map} monitor Monitor values to update. Values on the monitor with overwrite
+     *     values on the old monitor with the same ID. If a value isn't defined on the new monitor,
+     *     the old monitor will keep its old value.
      */
     requestUpdateMonitor (monitor) {
-        if (this._monitorState.hasOwnProperty(monitor.id)) {
-            this._monitorState[monitor.id] = Object.assign({}, this._monitorState[monitor.id], monitor);
+        if (this._monitorState.has(monitor.get('id'))) {
+            this._monitorState =
+                this._monitorState.set(monitor.get('id'), this._monitorState.get(monitor.get('id')).merge(monitor));
         }
     }
 
     /**
      * Removes a monitor from the state. Does nothing if the monitor already does
      * not exist in the state.
-     * @param {!object} monitorId ID of the monitor to remove.
+     * @param {!string} monitorId ID of the monitor to remove.
      */
     requestRemoveMonitor (monitorId) {
-        delete this._monitorState[monitorId];
+        this._monitorState = this._monitorState.delete(monitorId);
     }
 
     /**

test/unit/engine_runtime.js

@@ -1,5 +1,7 @@
 const test = require('tap').test;
 const Runtime = require('../../src/engine/runtime');
+const MonitorRecord = require('../../src/engine/monitor-record');
+const {Map} = require('immutable');
 
 test('spec', t => {
     const r = new Runtime();
@@ -10,3 +12,58 @@ test('spec', t => {
 
     t.end();
 });
+
+test('monitorStateEquals', t => {
+    const r = new Runtime();
+    const id = 'xklj4#!';
+    const prevMonitorState = MonitorRecord({
+        id,
+        opcode: 'turtle whereabouts',
+        value: '25'
+    });
+    const newMonitorDelta = Map({
+        id,
+        value: String(25)
+    });
+    r.requestAddMonitor(prevMonitorState);
+    r.requestUpdateMonitor(newMonitorDelta);
+
+    t.equals(true, prevMonitorState === r._monitorState.get(id));
+    t.equals(String(25), r._monitorState.get(id).get('value'));
+    t.end();
+});
+
+test('monitorStateDoesNotEqual', t => {
+    const r = new Runtime();
+    const id = 'xklj4#!';
+    const params = {seven: 7};
+    const prevMonitorState = MonitorRecord({
+        id,
+        opcode: 'turtle whereabouts',
+        value: '25'
+    });
+
+    // Value change
+    let newMonitorDelta = Map({
+        id,
+        value: String(24)
+    });
+    r.requestAddMonitor(prevMonitorState);
+    r.requestUpdateMonitor(newMonitorDelta);
+    
+    t.equals(false, prevMonitorState.equals(r._monitorState.get(id)));
+    t.equals(String(24), r._monitorState.get(id).get('value'));
+
+    // Prop change
+    newMonitorDelta = Map({
+        id: 'xklj4#!',
+        params: params
+    });
+    r.requestUpdateMonitor(newMonitorDelta);
+
+    t.equals(false, prevMonitorState.equals(r._monitorState.get(id)));
+    t.equals(String(24), r._monitorState.get(id).value);
+    t.equals(params, r._monitorState.get(id).params);
+
+    t.end();
+});