From 14a837a0f2c9ff36d4fe3de25032bf1a195ea9dd Mon Sep 17 00:00:00 2001 From: Muffin Date: Sat, 30 Mar 2024 17:39:32 -0500 Subject: [PATCH] Do not re-evaluate procedure arguments after stop this script or return Fixes https://github.com/TurboWarp/scratch-vm/issues/201 --- src/engine/thread.js | 18 ++++-- ...p-script-does-not-reevaluate-arguments.sb3 | Bin 0 -> 2494 bytes ...s-not-reevaluate-arguments.sb3.tw-snapshot | 25 +++++++++ ...s-not-reevaluate-arguments.sb3.tw-snapshot | 25 +++++++++ test/unit/engine_thread.js | 2 +- test/unit/tw_stop_this_script.js | 52 ++++++++++++++++++ 6 files changed, 117 insertions(+), 5 deletions(-) create mode 100644 test/fixtures/execute/tw-gh-201-stop-script-does-not-reevaluate-arguments.sb3 create mode 100644 test/snapshot/__snapshots__/tw-gh-201-stop-script-does-not-reevaluate-arguments.sb3.tw-snapshot create mode 100644 test/snapshot/__snapshots__/warp-timer/tw-gh-201-stop-script-does-not-reevaluate-arguments.sb3.tw-snapshot create mode 100644 test/unit/tw_stop_this_script.js diff --git a/src/engine/thread.js b/src/engine/thread.js index c8cf6cb1c13..70ecf302e71 100644 --- a/src/engine/thread.js +++ b/src/engine/thread.js @@ -316,12 +316,22 @@ class Thread { let blockID = this.peekStack(); while (blockID !== null) { const block = this.target.blocks.getBlock(blockID); - if ( - (typeof block !== 'undefined' && block.opcode === 'procedures_call') || - this.peekStackFrame().waitingReporter - ) { + + // Reporter form of procedures_call + if (this.peekStackFrame().waitingReporter) { break; } + + // Command form of procedures_call + if (typeof block !== 'undefined' && block.opcode === 'procedures_call') { + // By definition, if we get here, the procedure is done, so skip ahead so + // the arguments won't be re-evaluated and then discarded as frozen state + // about which arguments have been evaluated is lost. + // This fixes https://github.com/TurboWarp/scratch-vm/issues/201 + this.goToNextBlock(); + break; + } + this.popStack(); blockID = this.peekStack(); } diff --git a/test/fixtures/execute/tw-gh-201-stop-script-does-not-reevaluate-arguments.sb3 b/test/fixtures/execute/tw-gh-201-stop-script-does-not-reevaluate-arguments.sb3 new file mode 100644 index 0000000000000000000000000000000000000000..9e8b051d197aef02634bc43c8a8badeb6eb26616 GIT binary patch literal 2494 zcma);c{CJy8^?#TGo-<|gv4tyXpAuiHMt=%_Dr@RW=oV^WNA>AE4z?nU&774rG^qC zvNuZEm#o)5F-J7R25oXgsV`TKO`AR zTMLkSH&S%T0dMvjr?K|7HJd4;sGX7e>{c95Loi!bX=k!EaEYM`w^QtML$pv0aTY@6 z;74FAoM!=bSG{6Xy5V))ydA8j)sP*keJD!J7BJ4}f7q=;_&8P>50boro&C{J}9 z#PeZb()_8f7iw9_Tqz^DScoxLRgCWUR{ZP|lGEGQnP-H*+_!zCOuCa%_!P)h*qHK( zsv}uA&oS6tL2l2=?{Czh**H&YyAXW_u9)6X3?ZD7MDfmP`0GP+!)9FEitQ!VfQ~}f z1b`Fn1dTI4(iv7t?Ha7!E~o80+8bcwojmSY@a6cgV3WFx6fuRk;L!bV|J!D%#S5X2UCI%-ejptYwCUes8kK)0_|<1 zQ~AX4Pg;vh&KV>X1TN=;v@>Oje|fIs5XTl66z-n0xYVm=7{l11Ykgh_vMC{na7l%{ zYA#irR?1_;w|zgQ(e%+W{ncUiyAb22$9d%aii*R}R(R^-GIUlna-68ySE<>pZ9yJs z-NiU2!iowL)>zTKj-);Bsm#IjCStsGY-0a-upkg|TpndV);CqEFv<4YB1Q-mJ>U9% z%5k4w#KX6{tyE{o2BuCDTO>+q9JxOn8M`8C(~A;q}WmaadXf_z@gsp^Vt_J8sZ z{x)u(K8THg9vukTxZEoZR^YJ=RqYaXt$B==)Ndvk6t0@wXsly7hn-s{AU1?`t4&mM z$7R2}{vjm)d~qcFPNdaG;aRbu`PxmxA`u%}Q0gz^Jqh2H4)!|}sIc%&2q%XgwtxSn z+@|hhO@VUts9ZsAKCSDLOpaBTD;0JJe4l}G`n4xl7+B(2Yh>ut^`UYYArcW0K( zKh}p;Exyl-dqE6@?#x`)@63*Y*v60CNm{p=RVZiQM}*$6GVOP;IT0^%?x_klahNE2 zN4&&+E1a$}>N443%o7=i>N5j06e6v9=_@7A@OW*FdIInYE%+-NgT zXBFagp|Qo#@dlH7Hndm24~>sHWSWuHv%I86Ij$TpWYk~iQM}-L&OS;kQq!}hf&Ml+OOqiF59QVO4osl+-HLjnBL)GMak3hcjY02ps$L|dE|l5v;wV{pL_ z6vxQ&n5s^v&C+Gcrlt5w(CEs^)1gDl+x^t6*&zS(=QoCma>!c1fUeVnL-YdKMV;{w z3bmI(Uoo&qO{A+y#MesiLl-v613R8L8hFO{WYPt5a7)a9QYe4EGQ6SLvzZz0_7-d_ zub(uKJC$e=&;{#AiKTzdmKAx#=A*YYfv8^>`6z3?2fO1BDOT5)Irw4D z>bdQuG?o}6eJr(*8d!OP91=6tddiD!5Lfqk%)DuPV>q(cReG@`c|FF5j!+<^JAmv>AftD6~ z_*wf4S7zJoCb}e*LBGpV(svwX5`&j-<_OJieKIFRlg}sQ4TL9n5pNHY#FGk@p+M>YALbk*S#^5+g;kMZv-Up zsLJH)n!UUtyelj9(#R=?rG_ZN_F$r-EDNrRdGQrFxRGG`_vm#Hp)bi+J-w-?Un#RSk@fgtT;Kv147`^{kdzwR?7y&U&?tqHXZ)G1 z4okaM$}D`CO1JHYH)6Wp#VO4njI1&E=_a%R$$3o}vKUMOY&Eic@-{P)Y=b6ZEWTz_ zI4m(7oFKse&oYPC_qY0cviVv0GlKq--95}k{{~1uZ~XcG|JrEB1pxdT8dzeuxc^ig PIqa#!_w6A`0D%7jw&h^a literal 0 HcmV?d00001 diff --git a/test/snapshot/__snapshots__/tw-gh-201-stop-script-does-not-reevaluate-arguments.sb3.tw-snapshot b/test/snapshot/__snapshots__/tw-gh-201-stop-script-does-not-reevaluate-arguments.sb3.tw-snapshot new file mode 100644 index 00000000000..757a9053056 --- /dev/null +++ b/test/snapshot/__snapshots__/tw-gh-201-stop-script-does-not-reevaluate-arguments.sb3.tw-snapshot @@ -0,0 +1,25 @@ +// TW Snapshot +// Input SHA-256: fd981742e0e4299bad5a89349635d3a7d0467d8163ae77ba4bafe43c97849bae + +// Sprite1 script +(function factoryXYZ(thread) { const target = thread.target; const runtime = target.runtime; const stage = runtime.getTargetForStage(); +const b0 = runtime.getOpcodeFunction("looks_say"); +return function* genXYZ () { +yield* executeInCompatibilityLayer({"MESSAGE":"plan 0",}, b0, false, false, "d", null); +thread.procedures["Zfoo %s"](""); +yield* executeInCompatibilityLayer({"MESSAGE":"end",}, b0, false, false, "g", null); +retire(); return; +}; }) + +// Sprite1 foo %s +(function factoryXYZ(thread) { const target = thread.target; const runtime = target.runtime; const stage = runtime.getTargetForStage(); +return function funXYZ_foo_ (p0) { +return ""; +return ""; +}; }) + +// Sprite1 no op +(function factoryXYZ(thread) { const target = thread.target; const runtime = target.runtime; const stage = runtime.getTargetForStage(); +return function funXYZ_no_op () { +return ""; +}; }) diff --git a/test/snapshot/__snapshots__/warp-timer/tw-gh-201-stop-script-does-not-reevaluate-arguments.sb3.tw-snapshot b/test/snapshot/__snapshots__/warp-timer/tw-gh-201-stop-script-does-not-reevaluate-arguments.sb3.tw-snapshot new file mode 100644 index 00000000000..757a9053056 --- /dev/null +++ b/test/snapshot/__snapshots__/warp-timer/tw-gh-201-stop-script-does-not-reevaluate-arguments.sb3.tw-snapshot @@ -0,0 +1,25 @@ +// TW Snapshot +// Input SHA-256: fd981742e0e4299bad5a89349635d3a7d0467d8163ae77ba4bafe43c97849bae + +// Sprite1 script +(function factoryXYZ(thread) { const target = thread.target; const runtime = target.runtime; const stage = runtime.getTargetForStage(); +const b0 = runtime.getOpcodeFunction("looks_say"); +return function* genXYZ () { +yield* executeInCompatibilityLayer({"MESSAGE":"plan 0",}, b0, false, false, "d", null); +thread.procedures["Zfoo %s"](""); +yield* executeInCompatibilityLayer({"MESSAGE":"end",}, b0, false, false, "g", null); +retire(); return; +}; }) + +// Sprite1 foo %s +(function factoryXYZ(thread) { const target = thread.target; const runtime = target.runtime; const stage = runtime.getTargetForStage(); +return function funXYZ_foo_ (p0) { +return ""; +return ""; +}; }) + +// Sprite1 no op +(function factoryXYZ(thread) { const target = thread.target; const runtime = target.runtime; const stage = runtime.getTargetForStage(); +return function funXYZ_no_op () { +return ""; +}; }) diff --git a/test/unit/engine_thread.js b/test/unit/engine_thread.js index 0a7c3f3e420..0864fa9e47d 100644 --- a/test/unit/engine_thread.js +++ b/test/unit/engine_thread.js @@ -223,7 +223,7 @@ test('stopThisScript', t => { th.pushStack('arbitraryString'); th.pushStack('secondString'); th.stopThisScript(); - t.strictEquals(th.peekStack(), 'secondString'); + t.strictEquals(th.peekStack(), null); t.end(); }); diff --git a/test/unit/tw_stop_this_script.js b/test/unit/tw_stop_this_script.js new file mode 100644 index 00000000000..1f00096dd74 --- /dev/null +++ b/test/unit/tw_stop_this_script.js @@ -0,0 +1,52 @@ +const {test} = require('tap'); +const Thread = require('../../src/engine/thread'); +const Runtime = require('../../src/engine/runtime'); +const Target = require('../../src/engine/target'); + +test('stopThisScript procedures_call reporter form', t => { + const rt = new Runtime(); + const target = new Target(rt, null); + + target.blocks.createBlock({ + id: 'reporterCall', + opcode: 'procedures_call', + inputs: {}, + fields: {}, + mutation: { + return: '1' + }, + shadow: false, + topLevel: true, + parent: null, + next: 'afterReporterCall' + }); + target.blocks.createBlock({ + id: 'afterReporterCall', + opcode: 'motion_ifonedgebounce', + inputs: {}, + fields: {}, + mutation: null, + shadow: false, + topLevel: false, + parent: null, + next: null + }); + + const thread = new Thread('reporterCall'); + thread.target = target; + + // pretend to run reporterCall + thread.pushStack('reporterCall'); + thread.peekStackFrame().waitingReporter = true; + + // pretend to run scripts inside of the procedure + thread.pushStack('fakeBlock'); + + // stopping or returning should always return to reporterCall, not the block after + thread.stopThisScript(); + t.same(thread.stack, [ + 'reporterCall' + ]); + + t.end(); +});