Skip to content

Commit

Permalink
Do not re-evaluate procedure arguments after stop this script or return
Browse files Browse the repository at this point in the history
Fixes #201
  • Loading branch information
GarboMuffin committed Mar 30, 2024
1 parent 064155f commit 14a837a
Show file tree
Hide file tree
Showing 6 changed files with 117 additions and 5 deletions.
18 changes: 14 additions & 4 deletions src/engine/thread.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down
Binary file not shown.
Original file line number Diff line number Diff line change
@@ -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 "";
}; })
Original file line number Diff line number Diff line change
@@ -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 "";
}; })
2 changes: 1 addition & 1 deletion test/unit/engine_thread.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});
Expand Down
52 changes: 52 additions & 0 deletions test/unit/tw_stop_this_script.js
Original file line number Diff line number Diff line change
@@ -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();
});

0 comments on commit 14a837a

Please sign in to comment.