From add50ba105a270df920de96b205155237e70b9db Mon Sep 17 00:00:00 2001 From: Thomas Weber Date: Thu, 4 Dec 2025 21:19:35 -0600 Subject: [PATCH] Fix negative zero in monitors --- src/engine/runtime.js | 11 +--- src/util/tw-immutable-util.js | 48 ++++++++++++++ test/fixtures/tw-monitors.sb3 | Bin 0 -> 2613 bytes test/integration/tw_monitor_update.js | 74 +++++++++++++++++++++ test/unit/tw_immutable_util.js | 91 ++++++++++++++++++++++++++ 5 files changed, 216 insertions(+), 8 deletions(-) create mode 100644 src/util/tw-immutable-util.js create mode 100644 test/fixtures/tw-monitors.sb3 create mode 100644 test/integration/tw_monitor_update.js create mode 100644 test/unit/tw_immutable_util.js 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 0000000000000000000000000000000000000000..637a3b68f7e774727c8e53994403b16aaba7b792 GIT binary patch literal 2613 zcma);c`zGz8^_}a;x2I$>Q<_eh6tr@;;5r&D5dT?f{nU0#96e6tHe=66?Mi@v`Sko zYaP2buAsH*SV!Gb*0FZpKX$h>@67(5dFGktH}gC*-@U}{|)*f(AU#FMA;V`h-tO`WJ!f`pWe!(HKQ(m@y-t65`kuBPU^Ja zEm#8TH=aRIqEk2SP3+@7ITOlj+m`16e?UQ1c_o*eZP=K-O7v5NrPhU*OSgM6PEw+RS1b=hG(h;C zv;;Y{Qn;|SiE@Mz{{deICal9g&#}73edXp#gNLBO9A-~M=%~8A8=GQn(bp8#xLo6t zyrU&Azhbk(Hk62|YAJugC&bxPWTyNpdhEwv!9A{Kuyn-Trz4Vp8c8&{(OtDB(UpvK zT(mF?qk#LSr+&kkvXNzC9nBPv#X$ZlN7uitx__H^*dkNckU$6JhQ=-cImr_9@zVWx zv~;!-QMOuK_U=#RaJP6RmNsOx0?kD0`D#)f(aSeS9fOMqvhQYYG(-`B(6mZL03d0R{qZ2Nj@N2wEz`4(O(1%VF-k$mw!_8j#xlIc;F!8`VxG^v1QUhrkR@&Q@=Jp@{_l)Q z0~N&5GkkNsUsKm}w%CLBf=;`?F%&=`fiLgR-zWw$86!>^wUNI)!+~7Q1vY4AIKaO} zZAh19ssaIkRJQ+#Lc$OpY6ut-rs|1sLn6Hp>MBTq{8Bn1t!wRxhgLkk-ioMS6uEF>%Gh|Zi@q|Ec6prq!Z*eEMeHM2tXMCyB z>DQ`&-NspSM6LY*Y5+xD@|wBx$-LiipwgnpyD!mk$eN0XN3Bw=N`YMxSVkM7AI} z*ShTfn14`s#{qZUUuBTG0f&^X?_=8gxHNE_%?_oQ@sA5fyEQSMnoiEeMlbD0M(js6 z4w}vyMQFs+gGQk%L@lGHxiU)Ee#n@={YFC^K3mZAt3Q}W<&CbBNfGSX0$lDPN0&{? zbE$l&;yrpz<@eQTy@!M+%#zCitIqN_c&zpNWcFDFLQNgq^;g~YQh56AjS`X|0?6@~ zQ_J!kpM%%~0lau*;^>|kY~!fPU(0P#5A=W_rXW{jcz7v}bW|y-u6jb?eXvY&nZ_|k zyb%%?@S)MVX6Y~M%;dn9w%}`t#oe#NktRkDJb^%L=xW7?zKbZzqLJHyyD(V!qJ;h$}URjFIQw3vE6Fu6X>skna zZ1WWgw~U-VLNQ{Ydl%wBWSR8j>;;?Z&n*h3X*)2zH<4B5k~J8Y98d5buC!k61?42j zBfWLQFN4hlxTUMnaJtS)Dce%@H;saBsO+a}>#5-%%n7`bK#}~1ub%uewI|Ecki-Sw zK}ay7NTm19Tc9yFI!aFrwEqZI&R7x`gfGzy{wcA8pyqamE{{a&4d)p5{C!j&kREE( z!;(Y?A?=%U+qVf_g{z?#vVhrk0@so~lC5Gb0j!J?fzh0gb)w!@7vdR5Qw$gq8hx(Ce{3&yjpAZN~yh?|*nL=XciJ7gwdC(_IZDQzqCAO@F47XB%yk zN+dVEUu%hl#C00gMH+xPytN z{2a30nqsP)I!N1q2x^cy$DJD{g5HzyN^VkxtxQ>5=zO}udU~cH#zdJS#`|tepiBb* zA(W{hNQaf5rJl?$FMJBg;w^E{pm*}Wz*)?poz2;1*Ho=zhguQ*lIh=7nqEB#lz8sS z$tOQLu+nhfyJGpdk<;zH^j!WjHX~y#PumZKk-XlNL2cjJ$R?O^o!Giu;18%ayJ%-2 z8Ob4HkIPyT_VmDXctBq-XB5raMi}5{ui1YlyP`(1Fp-RNK`*hj#at5y*pSB#3$niD zVg+lw4ZiGVANG@%r+yA2`;k4guSZz-x`mJVcONaGz2ap~BQ@TJ7YBV { + 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(); +});