diff --git a/src/engine/blocks.js b/src/engine/blocks.js index ebe113f89..b26f6f529 100644 --- a/src/engine/blocks.js +++ b/src/engine/blocks.js @@ -1,6 +1,7 @@ const adapter = require('./adapter'); const mutationAdapter = require('./mutation-adapter'); const xmlEscape = require('../util/xml-escape'); +const {Map} = require('immutable'); /** * @fileoverview @@ -293,22 +294,19 @@ class Blocks { if (optRuntime && wasMonitored && !block.isMonitored) { optRuntime.requestRemoveMonitor(block.id); } else if (optRuntime && !wasMonitored && block.isMonitored) { - optRuntime.requestAddMonitor( - // Ensure that value is not undefined, since React requires it - { - // @todo(vm#564) this will collide if multiple sprites use same block - id: block.id, - category: 'data', - // @todo(vm#565) how to handle translation here? - label: block.opcode, - // @todo(vm#565) for numerical values with decimals, some countries use comma - value: '', - x: 0, - // @todo(vm#566) Don't require sending x and y when instantiating a - // monitor. If it's not preset the GUI should decide. - y: 0 - } - ); + optRuntime.requestAddMonitor(Map({ + // @todo(vm#564) this will collide if multiple sprites use same block + id: block.id, + category: 'data', + // @todo(vm#565) how to handle translation here? + label: block.opcode, + // @todo(vm#565) for numerical values with decimals, some countries use comma + value: '', // Ensure that value is not undefined, since React requires it + x: 0, + // @todo(vm#566) Don't require sending x and y when instantiating a + // monitor. If it's not preset the GUI should decide. + y: 0 + })); } break; } diff --git a/src/engine/execute.js b/src/engine/execute.js index 34e2e91be..3dfa7c1a9 100644 --- a/src/engine/execute.js +++ b/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. diff --git a/src/engine/runtime.js b/src/engine/runtime.js index bae3d0bae..bdc6c29a4 100644 --- a/src/engine/runtime.js +++ b/src/engine/runtime.js @@ -706,7 +706,8 @@ class Runtime extends EventEmitter { } if (!this._prevMonitorState.equals(this._monitorState)) { - this.emit(Runtime.MONITORS_UPDATE, this._monitorState.toArray()); + const monitorStateObj = this._monitorState.toJS(); + this.emit(Runtime.MONITORS_UPDATE, Object.keys(monitorStateObj).map(key => monitorStateObj[key])); } this._prevMonitorState = this._monitorState; } @@ -880,51 +881,28 @@ 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 {!Map} monitor Monitor to add. */ requestAddMonitor (monitor) { - this._monitorState = this._monitorState.set(monitor.id, monitor); + this._monitorState = this._monitorState.set(monitor.get('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 to update. */ requestUpdateMonitor (monitor) { - if (this._monitorState.has(monitor.id) && - this._monitorWouldChange(this._monitorState.get(monitor.id), monitor)) { + if (this._monitorState.has(monitor.get('id'))) { this._monitorState = - this._monitorState.set(monitor.id, Object.assign({}, this._monitorState.get(monitor.id), monitor)); + this._monitorState.set(monitor.get('id'), this._monitorState.get(monitor.get('id')).merge(monitor)); } } - /** - * Whether or not the monitor would change when the new state is applied. New state has its properties - * applied to the current state, it does not completely overwrite the current state. For this to return - * accurately, the changes being applied should be on primitive types, otherwise it may false-positive. - * @param {!object} currentMonitorState the state of a monitor, the values in this._monitorState - * @param {!object} newMonitorDelta the state to be applied to a monitor. The IDs of current and new monitor - * should match. - * @return {boolean} whether the new state, when applied to the current state, would change the current state. - */ - _monitorWouldChange (currentMonitorState, newMonitorDelta) { - for (const prop in newMonitorDelta) { - if (currentMonitorState.hasOwnProperty(prop)) { - // Strict equals to check if properties change; switch this to deep equals if - // we expect monitors to have changing state that is not primative. - if (currentMonitorState[prop] !== newMonitorDelta[prop]) return true; - } else { - return true; // The new state would add a new property to the monitor - } - } - return false; - } - /** * 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) { this._monitorState = this._monitorState.delete(monitorId); diff --git a/test/unit/engine_runtime.js b/test/unit/engine_runtime.js index 6fd04eefd..dd60c5eda 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 {Map} = require('immutable'); test('spec', t => { const r = new Runtime(); @@ -11,48 +12,62 @@ test('spec', t => { t.end(); }); -test('monitorWouldChange_false', t => { +test('monitorStateEquals', t => { const r = new Runtime(); - const currentMonitorState = { - id: 'xklj4#!', + const id = 'xklj4#!'; + const prevMonitorState = Map({ + id, category: 'data', label: 'turtle whereabouts', value: '25', x: 0, y: 0 - }; - const newMonitorDelta = { - id: 'xklj4#!', + }); + const newMonitorDelta = Map({ + id, value: String(25) - }; - t.equals(false, r._monitorWouldChange(currentMonitorState, newMonitorDelta)); + }); + r.requestAddMonitor(prevMonitorState); + r.requestUpdateMonitor(newMonitorDelta); + + t.equals(true, prevMonitorState.equals(r._monitorState.get(id))); + t.equals(String(25), r._monitorState.get(id).get('value')); t.end(); }); -test('monitorWouldChange_true', t => { +test('monitorStateDoesNotEqual', t => { const r = new Runtime(); - const currentMonitorState = { - id: 'xklj4#!', + const id = 'xklj4#!'; + const prevMonitorState = Map({ + id, category: 'data', label: 'turtle whereabouts', value: '25', x: 0, y: 0 - }; + }); // Value change - let newMonitorDelta = { - id: 'xklj4#!', + let newMonitorDelta = Map({ + id, value: String(24) - }; - t.equal(true, r._monitorWouldChange(currentMonitorState, newMonitorDelta)); + }); + 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 = { + newMonitorDelta = Map({ id: 'xklj4#!', moose: 7 - }; - t.equal(true, r._monitorWouldChange(currentMonitorState, newMonitorDelta)); + }); + 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.end(); });