Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Significantly improve hat blocks and event blocks #155

Merged
merged 6 commits into from
Aug 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/compiler/compile.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@ const compile = thread => {

return {
startingFunction: entry,
procedures
procedures,
executableHat: ir.entry.executableHat
};
};

Expand Down
6 changes: 6 additions & 0 deletions src/compiler/intermediate.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,12 @@ class IntermediateScript {
* @type {Function|null}
*/
this.cachedCompileResult = null;

/**
* Whether the top block of this script is an executable hat.
* @type {boolean}
*/
this.executableHat = false;
}
}

Expand Down
84 changes: 69 additions & 15 deletions src/compiler/irgen.js
Original file line number Diff line number Diff line change
Expand Up @@ -1453,6 +1453,58 @@ class ScriptTreeGenerator {
}
}

/**
* @param {Block} hatBlock
*/
walkHat (hatBlock) {
const nextBlock = hatBlock.next;
const opcode = hatBlock.opcode;
const hatInfo = this.runtime._hats[opcode];

if (this.thread.stackClick) {
// We still need to treat the hat as a normal block (so executableHat should be false) for
// interpreter parity, but the reuslt is ignored.
const opcodeFunction = this.runtime.getOpcodeFunction(opcode);
if (opcodeFunction) {
return [
this.descendCompatLayer(hatBlock),
...this.walkStack(nextBlock)
];
}
return this.walkStack(nextBlock);
}

if (hatInfo.edgeActivated) {
// Edge-activated HAT
this.script.yields = true;
this.script.executableHat = true;
return [
{
kind: 'hat.edge',
id: hatBlock.id,
condition: this.descendCompatLayer(hatBlock)
},
...this.walkStack(nextBlock)
];
}

const opcodeFunction = this.runtime.getOpcodeFunction(opcode);
if (opcodeFunction) {
// Predicate-based HAT
this.script.yields = true;
this.script.executableHat = true;
return [
{
kind: 'hat.predicate',
condition: this.descendCompatLayer(hatBlock)
},
...this.walkStack(nextBlock)
];
}

return this.walkStack(nextBlock);
}

/**
* @param {string} topBlockId The ID of the top block of the script.
* @returns {IntermediateScript}
Expand All @@ -1475,24 +1527,26 @@ class ScriptTreeGenerator {
this.readTopBlockComment(topBlock.comment);
}

// If the top block is a hat, advance to its child.
let entryBlock;
if (this.runtime.getIsHat(topBlock.opcode) || topBlock.opcode === 'procedures_definition') {
if (this.runtime.getIsEdgeActivatedHat(topBlock.opcode)) {
throw new Error(`Not compiling an edge-activated hat: ${topBlock.opcode}`);
}
entryBlock = topBlock.next;
// We do need to evaluate empty hats
const hatInfo = this.runtime._hats[topBlock.opcode];
const isHat = !!hatInfo;
if (isHat) {
this.script.stack = this.walkHat(topBlock);
} else {
entryBlock = topBlockId;
}

if (!entryBlock) {
// This is an empty script.
return this.script;
// We don't evaluate the procedures_definition top block as it never does anything
// We also don't want it to be treated like a hat block
let entryBlock;
if (topBlock.opcode === 'procedures_definition') {
entryBlock = topBlock.next;
} else {
entryBlock = topBlockId;
}

if (entryBlock) {
this.script.stack = this.walkStack(entryBlock);
}
}

this.script.stack = this.walkStack(entryBlock);

return this.script;
}
}
Expand Down
10 changes: 10 additions & 0 deletions src/compiler/jsexecute.js
Original file line number Diff line number Diff line change
Expand Up @@ -578,6 +578,14 @@ const execute = thread => {
thread.generator.next();
};

const threadStack = [];
const saveGlobalState = () => {
threadStack.push(globalState.thread);
};
const restoreGlobalState = () => {
globalState.thread = threadStack.pop();
};

const insertRuntime = source => {
let result = baseRuntime;
for (const functionName of Object.keys(runtimeFunctions)) {
Expand Down Expand Up @@ -606,5 +614,7 @@ const scopedEval = source => {

execute.scopedEval = scopedEval;
execute.runtimeFunctions = runtimeFunctions;
execute.saveGlobalState = saveGlobalState;
execute.restoreGlobalState = restoreGlobalState;

module.exports = execute;
30 changes: 29 additions & 1 deletion src/compiler/jsgen.js
Original file line number Diff line number Diff line change
Expand Up @@ -385,6 +385,7 @@ class JSGenerator {
this._setupVariables = {};

this.descendedIntoModulo = false;
this.isInHat = false;

this.debug = this.target.runtime.debug;
}
Expand Down Expand Up @@ -650,7 +651,8 @@ class JSGenerator {
const joinedArgs = args.join(',');

const yieldForRecursion = !this.isWarp && procedureCode === this.script.procedureCode;
if (yieldForRecursion) {
const yieldForHat = this.isInHat;
if (yieldForRecursion || yieldForHat) {
const runtimeFunction = procedureData.yields ? 'yieldThenCallGenerator' : 'yieldThenCall';
return new TypedInput(`(yield* ${runtimeFunction}(${procedureReference}, ${joinedArgs}))`, TYPE_UNKNOWN);
}
Expand Down Expand Up @@ -848,6 +850,32 @@ class JSGenerator {
this.source += `}\n`;
break;

case 'hat.edge':
this.isInHat = true;
this.source += '{\n';
// For exact Scratch parity, evaluate the input before checking old edge state.
// Can matter if the input is not instantly evaluated.
this.source += `const resolvedValue = ${this.descendInput(node.condition).asBoolean()};\n`;
this.source += `const id = "${sanitize(node.id)}";\n`;
this.source += 'const hasOldEdgeValue = target.hasEdgeActivatedValue(id);\n';
this.source += `const oldEdgeValue = target.updateEdgeActivatedValue(id, resolvedValue);\n`;
this.source += `const edgeWasActivated = hasOldEdgeValue ? (!oldEdgeValue && resolvedValue) : resolvedValue;\n`;
this.source += `if (!edgeWasActivated) {\n`;
this.retire();
this.source += '}\n';
this.source += 'yield;\n';
this.source += '}\n';
this.isInHat = false;
break;
case 'hat.predicate':
this.isInHat = true;
this.source += `if (!${this.descendInput(node.condition).asBoolean()}) {\n`;
this.retire();
this.source += '}\n';
this.source += 'yield;\n';
this.isInHat = false;
break;

case 'event.broadcast':
this.source += `startHats("event_whenbroadcastreceived", { BROADCAST_OPTION: ${this.descendInput(node.broadcast).asString()} });\n`;
this.resetVariableInputs();
Expand Down
39 changes: 20 additions & 19 deletions src/engine/execute.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,28 +58,29 @@ const handleReport = function (resolvedValue, sequencer, thread, blockCached, la
thread.pushReportedValue(resolvedValue);
if (isHat) {
// Hat predicate was evaluated.
if (sequencer.runtime.getIsEdgeActivatedHat(opcode)) {
if (thread.stackClick) {
thread.status = Thread.STATUS_RUNNING;
} else if (sequencer.runtime.getIsEdgeActivatedHat(opcode)) {
// If this is an edge-activated hat, only proceed if the value is
// true and used to be false, or the stack was activated explicitly
// via stack click
if (!thread.stackClick) {
const hasOldEdgeValue = thread.target.hasEdgeActivatedValue(currentBlockId);
const oldEdgeValue = thread.target.updateEdgeActivatedValue(
currentBlockId,
resolvedValue
);

const edgeWasActivated = hasOldEdgeValue ? (!oldEdgeValue && resolvedValue) : resolvedValue;
if (edgeWasActivated) {
// TW: Resume the thread if we were paused for a promise.
thread.status = Thread.STATUS_RUNNING;
} else {
sequencer.retireThread(thread);
}
const hasOldEdgeValue = thread.target.hasEdgeActivatedValue(currentBlockId);
const oldEdgeValue = thread.target.updateEdgeActivatedValue(
currentBlockId,
resolvedValue
);

const edgeWasActivated = hasOldEdgeValue ? (!oldEdgeValue && resolvedValue) : resolvedValue;
if (edgeWasActivated) {
thread.status = Thread.STATUS_RUNNING;
} else {
sequencer.retireThread(thread);
}
} else if (!resolvedValue) {
// Not an edge-activated hat: retire the thread
// if predicate was false.
} else if (resolvedValue) {
// Predicate returned true: allow the script to run.
thread.status = Thread.STATUS_RUNNING;
} else {
// Predicate returned false: do not allow script to run
sequencer.retireThread(thread);
}
} else {
Expand Down Expand Up @@ -118,7 +119,7 @@ const handlePromise = (primitiveReportedValue, sequencer, thread, blockCached, l
// If it's a command block or a top level reporter in a stackClick.
// TW: Don't mangle the stack when we just finished executing a hat block.
// Hat block is always the top and first block of the script. There are no loops to find.
if (lastOperation && !blockCached._isHat) {
if (lastOperation && (!blockCached._isHat || thread.stackClick)) {
let stackFrame;
let nextBlockId;
do {
Expand Down
12 changes: 10 additions & 2 deletions src/engine/runtime.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ const BlockType = require('../extension-support/block-type');
const Profiler = require('./profiler');
const Sequencer = require('./sequencer');
const execute = require('./execute.js');
const compilerExecute = require('../compiler/jsexecute');
const ScratchBlocksConstants = require('./scratch-blocks-constants');
const TargetType = require('../extension-support/target-type');
const Thread = require('./thread');
Expand Down Expand Up @@ -2116,8 +2117,15 @@ class Runtime extends EventEmitter {
// For compatibility with Scratch 2, edge triggered hats need to be processed before
// threads are stepped. See ScratchRuntime.as for original implementation
newThreads.forEach(thread => {
// tw: do not step compiled threads, the hat block can't be executed
if (!thread.isCompiled) {
if (thread.isCompiled) {
if (thread.executableHat) {
// It is quite likely that we are currently executing a block, so make sure
// that we leave the compiler's state intact at the end.
compilerExecute.saveGlobalState();
compilerExecute(thread);
compilerExecute.restoreGlobalState();
}
} else {
execute(this.sequencer, thread);
thread.goToNextBlock();
}
Expand Down
18 changes: 15 additions & 3 deletions src/engine/thread.js
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,7 @@ class Thread {
* @type {Object.<string, import('../compiler/compile').CompiledScript>}
*/
this.procedures = null;
this.executableHat = false;
}

/**
Expand Down Expand Up @@ -462,10 +463,15 @@ class Thread {

this.triedToCompile = true;

// stackClick === true disables hat block generation
// It would be great to cache these separately, but for now it's easiest to just disable them to avoid
// cached versions of scripts breaking projects.
const canCache = !this.stackClick;

const topBlock = this.topBlock;
// Flyout blocks are stored in a special block container.
const blocks = this.blockContainer.getBlock(topBlock) ? this.blockContainer : this.target.runtime.flyoutBlocks;
const cachedResult = blocks.getCachedCompileResult(topBlock);
const cachedResult = canCache && blocks.getCachedCompileResult(topBlock);
// If there is a cached error, do not attempt to recompile.
if (cachedResult && !cachedResult.success) {
return;
Expand All @@ -477,10 +483,14 @@ class Thread {
} else {
try {
result = compile(this);
blocks.cacheCompileResult(topBlock, result);
if (canCache) {
blocks.cacheCompileResult(topBlock, result);
}
} catch (error) {
log.error('cannot compile script', this.target.getName(), error);
blocks.cacheCompileError(topBlock, error);
if (canCache) {
blocks.cacheCompileError(topBlock, error);
}
this.target.runtime.emitCompileError(this.target, error);
return;
}
Expand All @@ -493,6 +503,8 @@ class Thread {

this.generator = result.startingFunction(this)();

this.executableHat = result.executableHat;

if (!this.blockContainer.forceNoGlow) {
this.blockGlowInFrame = this.topBlock;
this.requestScriptGlowInFrame = true;
Expand Down
4 changes: 4 additions & 0 deletions src/extension-support/extension-manager.js
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,10 @@ class ExtensionManager {
this.runtime.compilerRegisterExtension(extensionId, extensionInstance);
}

addBuiltinExtension (extensionId, extensionClass) {
builtinExtensions[extensionId] = () => extensionClass;
}

_isValidExtensionURL (extensionURL) {
try {
const parsedURL = new URL(extensionURL);
Expand Down
Binary file added test/fixtures/tw-add-builtin-extension.sb3
Binary file not shown.
Binary file added test/fixtures/tw-hats-and-events.sb3
Binary file not shown.
5 changes: 1 addition & 4 deletions test/integration/execute.js
Original file line number Diff line number Diff line change
Expand Up @@ -114,10 +114,7 @@ fs.readdirSync(executeDir)
// TW: Script compilation errors should fail.
if (enableCompiler) {
vm.on('COMPILE_ERROR', (target, error) => {
// Edge-activated hats are a known error.
if (!`${error}`.includes('edge-activated hat')) {
throw new Error(`Could not compile script in ${target.getName()}: ${error}`);
}
throw new Error(`Could not compile script in ${target.getName()}: ${error}`);
});
}

Expand Down
18 changes: 16 additions & 2 deletions test/integration/hat-execution-order.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,22 @@ const VirtualMachine = require('../../src/index');
const projectUri = path.resolve(__dirname, '../fixtures/hat-execution-order.sb2');
const project = readFileToBuffer(projectUri);

test('complex', t => {
const compilerAndInterpreter = (name, callback) => {
test(`${name} - interpreted`, t => {
callback(t, {
enabled: false
});
});
test(`${name} - compiled`, t => {
callback(t, {
enabled: true
});
});
};

compilerAndInterpreter('complex', (t, co) => {
const vm = new VirtualMachine();
vm.setCompilerOptions(co);
vm.attachStorage(makeTestStorage());

// Evaluate playground data and exit
Expand All @@ -21,7 +35,7 @@ test('complex', t => {
t.deepEqual(results, ['3', '2', '1', 'stage']);

t.end();
process.nextTick(process.exit);
vm.stop();
});

// Start VM, load project, and run
Expand Down
Loading
Loading