From f214d3a19112bd5e2e760397e14ef270d359a8c6 Mon Sep 17 00:00:00 2001 From: picklesrus Date: Tue, 20 Nov 2018 16:32:08 -0500 Subject: [PATCH 1/6] VM changes for the sensing_of block. This handles lists properly (by ignoring them like Scratch2 and makes the attribute menu update based on what was chosen in the target menu. --- src/blocks/scratch3_sensing.js | 9 ++++----- src/engine/blocks.js | 10 ++++++++++ src/engine/runtime.js | 15 +++++++++++++++ src/virtual-machine.js | 3 +++ test/unit/blocks_sensing.js | 27 +++++++++++++++++++++++++++ 5 files changed, 59 insertions(+), 5 deletions(-) diff --git a/src/blocks/scratch3_sensing.js b/src/blocks/scratch3_sensing.js index 6307b4c6a..9f3ed7420 100644 --- a/src/blocks/scratch3_sensing.js +++ b/src/blocks/scratch3_sensing.js @@ -313,12 +313,11 @@ class Scratch3SensingBlocks { } } - // Variables + // Local variables (not lists). const varName = args.PROPERTY; - for (const id in attrTarget.variables) { - if (attrTarget.variables[id].name === varName) { - return attrTarget.variables[id].value; - } + const variable = attrTarget.lookupVariableByNameAndType(varName, '', true); + if (variable) { + return variable.value; } // Otherwise, 0 diff --git a/src/engine/blocks.js b/src/engine/blocks.js index f2d792d6b..abb706538 100644 --- a/src/engine/blocks.js +++ b/src/engine/blocks.js @@ -567,6 +567,16 @@ class Blocks { if (!optRuntime){ break; } + // The selected item in the sensing of block menu needs to change based on the + // selected target. Set it to the first item in the menu list. + if (block.opcode === 'sensing_of_object_menu') { + if (block.fields.OBJECT.value === '_stage_') { + this._blocks[block.parent].fields.PROPERTY.value = 'backdrop #'; + } else { + this._blocks[block.parent].fields.PROPERTY.value = 'x position'; + } + optRuntime.requestBlocksUpdate(); + } const flyoutBlock = block.shadow && block.parent ? this._blocks[block.parent] : block; if (flyoutBlock.isMonitored) { diff --git a/src/engine/runtime.js b/src/engine/runtime.js index c1f9c90fc..f59b5dd59 100644 --- a/src/engine/runtime.js +++ b/src/engine/runtime.js @@ -613,6 +613,14 @@ class Runtime extends EventEmitter { return 'RUNTIME_STARTED'; } + /** + * Event name for reporting that a block was updated and needs to be rerendered. + * @const {string} + */ + static get BLOCKS_NEED_UPDATE () { + return 'BLOCKS_NEED_UPDATE'; + } + /** * How rapidly we try to step threads by default, in ms. */ @@ -2169,6 +2177,13 @@ class Runtime extends EventEmitter { this._refreshTargets = true; } + /** + * Emit an event that indicate that the blocks on the workspace need updating. + */ + requestBlocksUpdate () { + this.emit(Runtime.BLOCK_NEED_UPDATE); + } + /** * Set up timers to repeatedly step in a browser. */ diff --git a/src/virtual-machine.js b/src/virtual-machine.js index 29b0be289..757e351c8 100644 --- a/src/virtual-machine.js +++ b/src/virtual-machine.js @@ -108,6 +108,9 @@ class VirtualMachine extends EventEmitter { this.runtime.on(Runtime.BLOCKSINFO_UPDATE, blocksInfo => { this.emit(Runtime.BLOCKSINFO_UPDATE, blocksInfo); }); + this.runtime.on(Runtime.BLOCKS_NEED_UPDATE, () => { + this.emitWorkspaceUpdate(); + }); this.runtime.on(Runtime.PERIPHERAL_LIST_UPDATE, info => { this.emit(Runtime.PERIPHERAL_LIST_UPDATE, info); }); diff --git a/test/unit/blocks_sensing.js b/test/unit/blocks_sensing.js index 58b9510c8..d60b8fa68 100644 --- a/test/unit/blocks_sensing.js +++ b/test/unit/blocks_sensing.js @@ -229,6 +229,33 @@ test('loud? boolean', t => { t.end(); }); +test('get attribute of sprite variable', t => { + const rt = new Runtime(); + const sensing = new Sensing(rt); + const s = new Sprite(); + const target = new RenderedTarget(s, rt); + const thing = { + name: 'cars', + value: 'trucks' + }; + rt.getSpriteTargetByName = () => target; + target.lookupVariableByNameAndType = () => thing; + t.equal(sensing.getAttributeOf({PROPERTY: 'cars'}), 'trucks'); + + t.end(); +}); +test('get attribute of variable that does not exist', t => { + const rt = new Runtime(); + const sensing = new Sensing(rt); + const s = new Sprite(); + const stage = new RenderedTarget(s, rt); + rt.getTargetForStage = () => stage; + stage.lookupVariableByNameAndType = () => null; + t.equal(sensing.getAttributeOf({PROPERTY: 'stage'}), 0); + + t.end(); +}); + test('username block', t => { const rt = new Runtime(); const sensing = new Sensing(rt); From 4a542b455da53ed0868fa57753d64f57d0d6f1e7 Mon Sep 17 00:00:00 2001 From: picklesrus Date: Wed, 21 Nov 2018 11:08:22 -0500 Subject: [PATCH 2/6] fix code review comments --- src/blocks/scratch3_sensing.js | 2 +- src/engine/runtime.js | 2 +- test/unit/blocks_sensing.js | 12 ++++++------ 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/blocks/scratch3_sensing.js b/src/blocks/scratch3_sensing.js index 9f3ed7420..ee4a009b4 100644 --- a/src/blocks/scratch3_sensing.js +++ b/src/blocks/scratch3_sensing.js @@ -313,7 +313,7 @@ class Scratch3SensingBlocks { } } - // Local variables (not lists). + // Target variables. const varName = args.PROPERTY; const variable = attrTarget.lookupVariableByNameAndType(varName, '', true); if (variable) { diff --git a/src/engine/runtime.js b/src/engine/runtime.js index f59b5dd59..b53014c19 100644 --- a/src/engine/runtime.js +++ b/src/engine/runtime.js @@ -2181,7 +2181,7 @@ class Runtime extends EventEmitter { * Emit an event that indicate that the blocks on the workspace need updating. */ requestBlocksUpdate () { - this.emit(Runtime.BLOCK_NEED_UPDATE); + this.emit(Runtime.BLOCKS_NEED_UPDATE); } /** diff --git a/test/unit/blocks_sensing.js b/test/unit/blocks_sensing.js index d60b8fa68..bb08cabb4 100644 --- a/test/unit/blocks_sensing.js +++ b/test/unit/blocks_sensing.js @@ -234,12 +234,12 @@ test('get attribute of sprite variable', t => { const sensing = new Sensing(rt); const s = new Sprite(); const target = new RenderedTarget(s, rt); - const thing = { + const variable = { name: 'cars', value: 'trucks' }; rt.getSpriteTargetByName = () => target; - target.lookupVariableByNameAndType = () => thing; + target.lookupVariableByNameAndType = () => variable; t.equal(sensing.getAttributeOf({PROPERTY: 'cars'}), 'trucks'); t.end(); @@ -248,10 +248,10 @@ test('get attribute of variable that does not exist', t => { const rt = new Runtime(); const sensing = new Sensing(rt); const s = new Sprite(); - const stage = new RenderedTarget(s, rt); - rt.getTargetForStage = () => stage; - stage.lookupVariableByNameAndType = () => null; - t.equal(sensing.getAttributeOf({PROPERTY: 'stage'}), 0); + const target = new RenderedTarget(s, rt); + rt.getTargetForStage = () => target; + target.lookupVariableByNameAndType = () => null; + t.equal(sensing.getAttributeOf({PROPERTY: 'variableThatDoesNotExist'}), 0); t.end(); }); From e64cd5f34b66b55afa4533da97718fdfa0b32033 Mon Sep 17 00:00:00 2001 From: picklesrus Date: Tue, 27 Nov 2018 23:59:07 -0500 Subject: [PATCH 3/6] stop mocking method in the test. --- test/unit/blocks_sensing.js | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/test/unit/blocks_sensing.js b/test/unit/blocks_sensing.js index bb08cabb4..a38695360 100644 --- a/test/unit/blocks_sensing.js +++ b/test/unit/blocks_sensing.js @@ -4,6 +4,8 @@ const Runtime = require('../../src/engine/runtime'); const Sprite = require('../../src/sprites/sprite'); const RenderedTarget = require('../../src/sprites/rendered-target'); const BlockUtility = require('../../src/engine/block-utility'); +const Variable = require('../../src/engine/variable'); + test('getPrimitives', t => { const rt = new Runtime(); @@ -236,10 +238,12 @@ test('get attribute of sprite variable', t => { const target = new RenderedTarget(s, rt); const variable = { name: 'cars', - value: 'trucks' + value: 'trucks', + type: '' }; + // Add variable to set the map (it should be empty before this). + target.variables.anId = variable; rt.getSpriteTargetByName = () => target; - target.lookupVariableByNameAndType = () => variable; t.equal(sensing.getAttributeOf({PROPERTY: 'cars'}), 'trucks'); t.end(); @@ -250,7 +254,6 @@ test('get attribute of variable that does not exist', t => { const s = new Sprite(); const target = new RenderedTarget(s, rt); rt.getTargetForStage = () => target; - target.lookupVariableByNameAndType = () => null; t.equal(sensing.getAttributeOf({PROPERTY: 'variableThatDoesNotExist'}), 0); t.end(); From d19848b7122fe34268e725f509fe25db017cd0b6 Mon Sep 17 00:00:00 2001 From: picklesrus Date: Wed, 28 Nov 2018 11:55:02 -0500 Subject: [PATCH 4/6] remove unused import --- test/unit/blocks_sensing.js | 1 - 1 file changed, 1 deletion(-) diff --git a/test/unit/blocks_sensing.js b/test/unit/blocks_sensing.js index a38695360..85b45d44d 100644 --- a/test/unit/blocks_sensing.js +++ b/test/unit/blocks_sensing.js @@ -4,7 +4,6 @@ const Runtime = require('../../src/engine/runtime'); const Sprite = require('../../src/sprites/sprite'); const RenderedTarget = require('../../src/sprites/rendered-target'); const BlockUtility = require('../../src/engine/block-utility'); -const Variable = require('../../src/engine/variable'); test('getPrimitives', t => { From eefe02142507a93ead63ac20c41bd022ff672ec9 Mon Sep 17 00:00:00 2001 From: picklesrus Date: Wed, 28 Nov 2018 12:33:04 -0500 Subject: [PATCH 5/6] add todo and remove a newline --- src/engine/blocks.js | 1 + test/unit/blocks_sensing.js | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/src/engine/blocks.js b/src/engine/blocks.js index abb706538..aaa52bf6e 100644 --- a/src/engine/blocks.js +++ b/src/engine/blocks.js @@ -569,6 +569,7 @@ class Blocks { } // The selected item in the sensing of block menu needs to change based on the // selected target. Set it to the first item in the menu list. + // TODO: https://github.com/LLK/scratch-vm/issues/1787 if (block.opcode === 'sensing_of_object_menu') { if (block.fields.OBJECT.value === '_stage_') { this._blocks[block.parent].fields.PROPERTY.value = 'backdrop #'; diff --git a/test/unit/blocks_sensing.js b/test/unit/blocks_sensing.js index 85b45d44d..130b27d0b 100644 --- a/test/unit/blocks_sensing.js +++ b/test/unit/blocks_sensing.js @@ -5,7 +5,6 @@ const Sprite = require('../../src/sprites/sprite'); const RenderedTarget = require('../../src/sprites/rendered-target'); const BlockUtility = require('../../src/engine/block-utility'); - test('getPrimitives', t => { const rt = new Runtime(); const s = new Sensing(rt); From 90b627982c2adfe2829aa8410678ae255c1fc5d1 Mon Sep 17 00:00:00 2001 From: picklesrus Date: Wed, 28 Nov 2018 12:34:49 -0500 Subject: [PATCH 6/6] fix todo --- src/engine/blocks.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/engine/blocks.js b/src/engine/blocks.js index aaa52bf6e..8b8575b94 100644 --- a/src/engine/blocks.js +++ b/src/engine/blocks.js @@ -569,7 +569,7 @@ class Blocks { } // The selected item in the sensing of block menu needs to change based on the // selected target. Set it to the first item in the menu list. - // TODO: https://github.com/LLK/scratch-vm/issues/1787 + // TODO: (#1787) if (block.opcode === 'sensing_of_object_menu') { if (block.fields.OBJECT.value === '_stage_') { this._blocks[block.parent].fields.PROPERTY.value = 'backdrop #';