Merge pull request #1818 from picklesrus/monitor-vars-project-load

Clear out the blocks in dispose. Fixes #1758 where old monitored vari…

src/engine/runtime.js

@@ -1549,11 +1549,17 @@ class Runtime extends EventEmitter {
         return newThreads;
     }
 
+
     /**
      * Dispose all targets. Return to clean state.
      */
     dispose () {
         this.stopAll();
+        // Deleting each target's variable's monitors.
+        this.targets.forEach(target => {
+            if (target.isOriginal) target.deleteMonitors();
+        });
+
         this.targets.map(this.disposeTarget, this);
         this._monitorState = OrderedMap({});
         this.emit(Runtime.RUNTIME_DISPOSED);

src/engine/target.js

@@ -336,6 +336,26 @@ class Target extends EventEmitter {
         }
     }
 
+    /**
+     * Remove this target's monitors from the runtime state and remove the
+     * target-specific monitored blocks (e.g. local variables, global variables for the stage, x-position).
+     * NOTE: This does not delete any of the stage monitors like backdrop name.
+     */
+    deleteMonitors () {
+        this.runtime.requestRemoveMonitorByTargetId(this.id);
+        let targetSpecificMonitorBlockIds;
+        if (this.isStage) {
+            // This only deletes global variables and not other stage monitors like backdrop number.
+            targetSpecificMonitorBlockIds = Object.keys(this.variables);
+        } else {
+            targetSpecificMonitorBlockIds = Object.keys(this.runtime.monitorBlocks._blocks)
+                .filter(key => this.runtime.monitorBlocks._blocks[key].targetId === this.id);
+        }
+        for (const blockId of targetSpecificMonitorBlockIds) {
+            this.runtime.monitorBlocks.deleteBlock(blockId);
+        }
+    }
+
     /**
      * Create a clone of the variable with the given id from the dictionary of
      * this target's variables.

src/virtual-machine.js

@@ -939,12 +939,7 @@ class VirtualMachine extends EventEmitter {
             const restoreSprite = () => spritePromise.then(spriteBuffer => this.addSprite(spriteBuffer));
             // Remove monitors from the runtime state and remove the
             // target-specific monitored blocks (e.g. local variables)
-            this.runtime.requestRemoveMonitorByTargetId(targetId);
-            const targetSpecificMonitorBlockIds = Object.keys(this.runtime.monitorBlocks._blocks)
-                .filter(key => this.runtime.monitorBlocks._blocks[key].targetId === targetId);
-            for (const blockId of targetSpecificMonitorBlockIds) {
-                this.runtime.monitorBlocks.deleteBlock(blockId);
-            }
+            target.deleteMonitors();
             const currentEditingTarget = this.editingTarget;
             for (let i = 0; i < sprite.clones.length; i++) {
                 const clone = sprite.clones[i];

test/integration/monitor-threads-run-every-frame.js

@@ -6,7 +6,7 @@ const VirtualMachine = require('../../src/index');
 const Thread = require('../../src/engine/thread');
 const Runtime = require('../../src/engine/runtime');
 
-const projectUri = path.resolve(__dirname, '../fixtures/default.sb2');
+const projectUri = path.resolve(__dirname, '../fixtures/timer-monitor.sb3');
 const project = readFileToBuffer(projectUri);
 
 const checkMonitorThreadPresent = (t, threads) => {
@@ -14,15 +14,12 @@ const checkMonitorThreadPresent = (t, threads) => {
     const monitorThread = threads[0];
     t.equal(monitorThread.stackClick, false);
     t.equal(monitorThread.updateMonitor, true);
-    t.equal(monitorThread.topBlock.toString(), 'sensing_timer');
+    t.equal(monitorThread.topBlock.toString(), 'timer');
 };
 
 /**
  * Creates a monitor and then checks if it gets run every frame.
  */
-/* TODO: when loadProject loads monitors, we can create a project with a monitor and will
- * not have to do the create monitor step manually.
- */
 test('monitor thread runs every frame', t => {
     const vm = new VirtualMachine();
     vm.attachStorage(makeTestStorage());
@@ -31,22 +28,6 @@ test('monitor thread runs every frame', t => {
     t.doesNotThrow(() => {
         // Note: don't run vm.start(), we handle calling _step() manually in this test
         vm.runtime.currentStepTime = Runtime.THREAD_STEP_INTERVAL;
-
-        // Manually populate the monitor block and set its isMonitored to true.
-        vm.runtime.monitorBlocks.createBlock({
-            id: 'sensing_timer',
-            opcode: 'sensing_timer',
-            inputs: {},
-            fields: {},
-            next: null,
-            topLevel: true,
-            parent: null,
-            shadow: false,
-            isMonitored: true,
-            x: '0',
-            y: '0'
-        });
-
         vm.clear();
         vm.setCompatibilityMode(false);
         vm.setTurboMode(false);
@@ -86,21 +67,6 @@ test('monitor thread not added twice', t => {
         // Note: don't run vm.start(), we handle calling _step() manually in this test
         vm.runtime.currentStepTime = 0;
 
-        // Manually populate the monitor block and set its isMonitored to true.
-        vm.runtime.monitorBlocks.createBlock({
-            id: 'sensing_timer',
-            opcode: 'sensing_timer',
-            inputs: {},
-            fields: {},
-            next: null,
-            topLevel: true,
-            parent: null,
-            shadow: false,
-            isMonitored: true,
-            x: '0',
-            y: '0'
-        });
-
         vm.clear();
         vm.setCompatibilityMode(false);
         vm.setTurboMode(false);

test/integration/variable_monitor_reset.js

@@ -0,0 +1,42 @@
+const path = require('path');
+const test = require('tap').test;
+const makeTestStorage = require('../fixtures/make-test-storage');
+const readFileToBuffer = require('../fixtures/readProjectFile').readFileToBuffer;
+const VirtualMachine = require('../../src/index');
+
+const projectUri = path.resolve(__dirname, '../fixtures/monitored_variables.sb3');
+const project = readFileToBuffer(projectUri);
+
+const anotherProjectUri = path.resolve(__dirname, '../fixtures/default.sb2');
+const anotherProject = readFileToBuffer(anotherProjectUri);
+
+test('importing one project after the other resets monitored variables', t => {
+    const vm = new VirtualMachine();
+    vm.attachStorage(makeTestStorage());
+
+    // Start VM, load project, and run
+    vm.start();
+    vm.clear();
+    vm.setCompatibilityMode(false);
+    vm.setTurboMode(false);
+    vm.loadProject(project).then(() => {
+        const refSprite = vm.runtime.targets[1];
+        const refVarId = Object.keys(refSprite.variables)[0];
+        const refBlock = vm.runtime.monitorBlocks.getBlock(refVarId);
+        t.ok(refBlock);
+        const jamalSprite = vm.runtime.targets[2];
+        const jamalVarId = Object.keys(jamalSprite.variables)[0];
+        const jamalBlock = vm.runtime.monitorBlocks.getBlock(jamalVarId);
+        t.ok(jamalBlock);
+        vm.loadProject(anotherProject).then(() => {
+            // Blocks from the previously loaded project should be gone.
+            const refVarBlock = vm.runtime.monitorBlocks.getBlock(refVarId);
+            t.notOk(refVarBlock);
+            const jamalVarBlock = vm.runtime.monitorBlocks.getBlock(jamalVarId);
+            t.notOk(jamalVarBlock);
+
+            t.end();
+            process.nextTick(process.exit);
+        });
+    });
+});