Merge pull request #155 from TurboWarp/hats-revisit

Significantly improve hat blocks and event blocks

src/compiler/compile.js

@@ -29,7 +29,8 @@ const compile = thread => {
 
     return {
         startingFunction: entry,
-        procedures
+        procedures,
+        executableHat: ir.entry.executableHat
     };
 };
 

src/compiler/intermediate.js

@@ -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;
     }
 }
 

src/compiler/irgen.js

@@ -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}
@@ -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;
     }
 }

src/compiler/jsexecute.js

@@ -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)) {
@@ -606,5 +614,7 @@ const scopedEval = source => {
 
 execute.scopedEval = scopedEval;
 execute.runtimeFunctions = runtimeFunctions;
+execute.saveGlobalState = saveGlobalState;
+execute.restoreGlobalState = restoreGlobalState;
 
 module.exports = execute;

src/compiler/jsgen.js

@@ -385,6 +385,7 @@ class JSGenerator {
         this._setupVariables = {};
 
         this.descendedIntoModulo = false;
+        this.isInHat = false;
 
         this.debug = this.target.runtime.debug;
     }
@@ -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);
             }
@@ -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();

src/engine/execute.js

@@ -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 {
@@ -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 {

src/engine/runtime.js

@@ -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');
@@ -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();
             }

src/engine/thread.js

@@ -214,6 +214,7 @@ class Thread {
          * @type {Object.<string, import('../compiler/compile').CompiledScript>}
          */
         this.procedures = null;
+        this.executableHat = false;
     }
 
     /**
@@ -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;
@@ -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;
             }
@@ -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;

src/extension-support/extension-manager.js

@@ -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);

test/integration/execute.js

@@ -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}`);
                 });
             }
 

test/integration/hat-execution-order.js

@@ -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
@@ -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