diff --git a/src/engine/runtime.js b/src/engine/runtime.js index 503a1fbc449..a7e79370926 100644 --- a/src/engine/runtime.js +++ b/src/engine/runtime.js @@ -23,6 +23,7 @@ const ScratchLinkWebSocket = require('../util/scratch-link-websocket'); const FontManager = require('./tw-font-manager'); const fetchWithTimeout = require('../util/fetch-with-timeout'); const platform = require('./tw-platform.js'); +const {compareImmutableMaps, mergeMaps} = require('../util/tw-immutable-util'); // Virtual I/O devices. const Clock = require('../io/clock'); @@ -2577,7 +2578,7 @@ class Runtime extends EventEmitter { this._refreshTargets = false; } - if (!this._prevMonitorState.equals(this._monitorState)) { + if (!compareImmutableMaps(this._prevMonitorState, this._monitorState)) { this.emit(Runtime.MONITORS_UPDATE, this._monitorState); this._prevMonitorState = this._monitorState; } @@ -3112,13 +3113,7 @@ class Runtime extends EventEmitter { const id = monitor.get('id'); if (this._monitorState.has(id)) { this._monitorState = - // Use mergeWith here to prevent undefined values from overwriting existing ones - this._monitorState.set(id, this._monitorState.get(id).mergeWith((prev, next) => { - if (typeof next === 'undefined' || next === null) { - return prev; - } - return next; - }, monitor)); + this._monitorState.set(id, mergeMaps(this._monitorState.get(id), monitor)); return true; } return false; diff --git a/src/util/tw-immutable-util.js b/src/util/tw-immutable-util.js new file mode 100644 index 00000000000..d706e0c2081 --- /dev/null +++ b/src/util/tw-immutable-util.js @@ -0,0 +1,48 @@ +/** + * @fileoverview Utilities for immutable.js objects. + */ + +/** + * Determine if two maps have identical keys and content (Object.is equality) + * Unlike the regular immutable.js comparison functions, this one considers 0 and -0 to be different. + * @param {OrderedMap} a + * @param {OrderedMap} b + * @returns {boolean} true if a and b have the same keys and values + */ +const compareImmutableMaps = (a, b) => { + if (a.size !== b.size) { + return false; + } + + for (const key of a.keys()) { + const aValue = a.get(key); + const bValue = b.get(key); + if (!Object.is(aValue, bValue)) { + return false; + } + } + + return true; +}; + +/** + * Merge map B into map A. Values of undefined or null in B will default to B. + * Unlike the regular immutable.js comparison functions, this one considers 0 and -0 to be different. + * @param {OrderedMap} a + * @param {OrderedMap} b + * @returns {OrderedMap} + */ +const mergeMaps = (a, b) => b + .filter(value => value === 0) + .merge(a) + .mergeWith((prev, next) => { + if (typeof next === 'undefined' || next === null) { + return prev; + } + return next; + }, b); + +module.exports = { + compareImmutableMaps, + mergeMaps +}; diff --git a/test/fixtures/tw-monitors.sb3 b/test/fixtures/tw-monitors.sb3 new file mode 100644 index 00000000000..637a3b68f7e Binary files /dev/null and b/test/fixtures/tw-monitors.sb3 differ diff --git a/test/integration/tw_monitor_update.js b/test/integration/tw_monitor_update.js new file mode 100644 index 00000000000..8cff4734d47 --- /dev/null +++ b/test/integration/tw_monitor_update.js @@ -0,0 +1,74 @@ +const fs = require('node:fs'); +const path = require('node:path'); +const {test} = require('tap'); +const VM = require('../../src/virtual-machine'); + +test('monitor update', t => { + const fixture = path.join(__dirname, '../fixtures/tw-monitors.sb3'); + const vm = new VM(); + vm.loadProject(fs.readFileSync(fixture)).then(() => { + const variable = vm.runtime.getTargetForStage().lookupVariableByNameAndType('variable', ''); + const list = vm.runtime.getTargetForStage().lookupVariableByNameAndType('list', 'list'); + + const updates = []; + vm.runtime.on('MONITORS_UPDATE', monitors => { + const values = {}; + for (const monitor of monitors.values()) { + const name = Object.values(monitor.get('params'))[0]; + const value = monitor.get('value'); + values[name] = value; + } + updates.push(values); + }); + + // Baseline start + updates.length = 0; + vm.runtime._step(); + t.equal(updates.length, 1); + t.equal(updates[0].variable, 0); + t.same(updates[0].list, []); + + // Change variable to 5 + updates.length = 0; + variable.value = 5; + vm.runtime._step(); + t.equal(updates.length, 1); + t.equal(updates[0].variable, 5); + t.same(updates[0].list, []); + + // Change variable to -0 + updates.length = 0; + variable.value = -0; + vm.runtime._step(); + t.equal(updates.length, 1); + t.ok(Object.is(updates[0].variable, -0)); + t.same(updates[0].list, []); + + // Change variable to 0 + updates.length = 0; + variable.value = 0; + vm.runtime._step(); + t.equal(updates.length, 1); + t.ok(Object.is(updates[0].variable, 0)); + t.same(updates[0].list, []); + + // Replace list entirely + updates.length = 0; + list.value = [1, 2, 3]; + vm.runtime._step(); + t.equal(updates.length, 1); + t.equal(updates[0].variable, 0); + t.same(updates[0].list, [1, 2, 3]); + + // Append to list + updates.length = 0; + list.value.push(4); + list.value._monitorUpToDate = false; + vm.runtime._step(); + t.equal(updates.length, 1); + t.equal(updates[0].variable, 0); + t.same(updates[0].list, [1, 2, 3, 4]); + + t.end(); + }); +}); diff --git a/test/unit/tw_immutable_util.js b/test/unit/tw_immutable_util.js new file mode 100644 index 00000000000..832bc6a1252 --- /dev/null +++ b/test/unit/tw_immutable_util.js @@ -0,0 +1,91 @@ +const {test} = require('tap'); +const {OrderedMap} = require('immutable'); +const {compareImmutableMaps, mergeMaps} = require('../../src/util/tw-immutable-util'); + +test('compareImmutableMaps', t => { + t.ok(compareImmutableMaps(OrderedMap(), OrderedMap())); + t.ok(compareImmutableMaps(OrderedMap({ + a: 'hello' + }), OrderedMap({ + a: 'hello' + }))); + t.ok(compareImmutableMaps(OrderedMap({ + what: 0, + why: 'how' + }), OrderedMap({ + why: 'how', + what: 0 + }))); + + t.notOk(compareImmutableMaps(OrderedMap({ + a: 0 + }), OrderedMap({ + a: -0 + }))); + t.notOk(compareImmutableMaps(OrderedMap(), OrderedMap({ + a: 0 + }))); + t.notOk(compareImmutableMaps(OrderedMap({ + a: 0 + }), OrderedMap())); + + const arr = []; + t.ok(compareImmutableMaps(OrderedMap({ + arr + }), OrderedMap({ + arr + }))); + t.notOk(compareImmutableMaps(OrderedMap({ + arr: [] + }), OrderedMap({ + arr: [] + }))); + + t.end(); +}); + +test('mergeMaps', t => { + t.ok(compareImmutableMaps(mergeMaps( + OrderedMap({ + a: 'hello', + b: 'bye', + c: 'ok' + }), + OrderedMap({ + b: '!!', + c: null, + d: 'e', + e: undefined + }) + ), OrderedMap({ + a: 'hello', + b: '!!', + c: 'ok', + d: 'e', + e: undefined + }))); + + t.ok(compareImmutableMaps(mergeMaps( + OrderedMap({ + a: 0 + }), + OrderedMap({ + a: -0 + }) + ), OrderedMap({ + a: -0 + }))); + + t.ok(compareImmutableMaps(mergeMaps( + OrderedMap({ + a: -0 + }), + OrderedMap({ + a: 0 + }) + ), OrderedMap({ + a: 0 + }))); + + t.end(); +});