From 1902848ea4e67683bc4371ede0e54c6e223041ac Mon Sep 17 00:00:00 2001
From: DD Liu <liudi@media.mit.edu>
Date: Wed, 24 May 2017 15:42:29 -0400
Subject: [PATCH] Switch to ordered maps of monitor records for monitor state

---
 src/engine/blocks.js        |  4 ++--
 src/engine/runtime.js       | 14 ++++++++------
 test/unit/engine_runtime.js | 13 ++++++-------
 3 files changed, 16 insertions(+), 15 deletions(-)

diff --git a/src/engine/blocks.js b/src/engine/blocks.js
index b26f6f529..19921f832 100644
--- a/src/engine/blocks.js
+++ b/src/engine/blocks.js
@@ -1,7 +1,7 @@
 const adapter = require('./adapter');
 const mutationAdapter = require('./mutation-adapter');
 const xmlEscape = require('../util/xml-escape');
-const {Map} = require('immutable');
+const MonitorRecord = require('./records');
 
 /**
  * @fileoverview
@@ -294,7 +294,7 @@ class Blocks {
             if (optRuntime && wasMonitored && !block.isMonitored) {
                 optRuntime.requestRemoveMonitor(block.id);
             } else if (optRuntime && !wasMonitored && block.isMonitored) {
-                optRuntime.requestAddMonitor(Map({
+                optRuntime.requestAddMonitor(MonitorRecord({
                     // @todo(vm#564) this will collide if multiple sprites use same block
                     id: block.id,
                     category: 'data',
diff --git a/src/engine/runtime.js b/src/engine/runtime.js
index 8ecbb6b1f..26dc4d354 100644
--- a/src/engine/runtime.js
+++ b/src/engine/runtime.js
@@ -2,7 +2,7 @@ const EventEmitter = require('events');
 const Sequencer = require('./sequencer');
 const Blocks = require('./blocks');
 const Thread = require('./thread');
-const {Map} = require('immutable');
+const {OrderedMap} = require('immutable');
 
 // Virtual I/O devices.
 const Clock = require('../io/clock');
@@ -113,14 +113,14 @@ class Runtime extends EventEmitter {
         this._refreshTargets = false;
 
         /**
-         * List of all monitors.
+         * Ordered map of all monitors, which are MonitorReporter objects.
          */
-        this._monitorState = Map({});
+        this._monitorState = OrderedMap({});
 
         /**
          * Monitor state from last tick
          */
-        this._prevMonitorState = Map({});
+        this._prevMonitorState = OrderedMap({});
 
         /**
          * Whether the project is in "turbo mode."
@@ -880,7 +880,7 @@ class Runtime extends EventEmitter {
     /**
      * Add a monitor to the state. If the monitor already exists in the state,
      * overwrites it.
-     * @param {!Map} monitor Monitor to add.
+     * @param {!MonitorRecord} monitor Monitor to add.
      */
     requestAddMonitor (monitor) {
         this._monitorState = this._monitorState.set(monitor.get('id'), monitor);
@@ -889,7 +889,9 @@ class Runtime extends EventEmitter {
     /**
      * Update a monitor in the state. Does nothing if the monitor does not already
      * exist in the state.
-     * @param {!Map} 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.has(monitor.get('id'))) {
diff --git a/test/unit/engine_runtime.js b/test/unit/engine_runtime.js
index dd60c5eda..8087bab63 100644
--- a/test/unit/engine_runtime.js
+++ b/test/unit/engine_runtime.js
@@ -1,5 +1,6 @@
 const test = require('tap').test;
 const Runtime = require('../../src/engine/runtime');
+const MonitorRecord = require('../../src/engine/records');
 const {Map} = require('immutable');
 
 test('spec', t => {
@@ -15,7 +16,7 @@ test('spec', t => {
 test('monitorStateEquals', t => {
     const r = new Runtime();
     const id = 'xklj4#!';
-    const prevMonitorState = Map({
+    const prevMonitorState = MonitorRecord({
         id,
         category: 'data',
         label: 'turtle whereabouts',
@@ -38,13 +39,11 @@ test('monitorStateEquals', t => {
 test('monitorStateDoesNotEqual', t => {
     const r = new Runtime();
     const id = 'xklj4#!';
-    const prevMonitorState = Map({
+    const prevMonitorState = MonitorRecord({
         id,
         category: 'data',
         label: 'turtle whereabouts',
-        value: '25',
-        x: 0,
-        y: 0
+        value: '25'
     });
 
     // Value change
@@ -61,13 +60,13 @@ test('monitorStateDoesNotEqual', t => {
     // Prop change
     newMonitorDelta = Map({
         id: 'xklj4#!',
-        moose: 7
+        x: 7
     });
     r.requestUpdateMonitor(newMonitorDelta);
 
     t.equals(false, prevMonitorState.equals(r._monitorState.get(id)));
     t.equals(String(24), r._monitorState.get(id).get('value'));
-    t.equals(7, r._monitorState.get(id).get('moose'));
+    t.equals(7, r._monitorState.get(id).get('x'));
 
     t.end();
 });