From e5723e299a687fb22642ffd2466b470df260567d Mon Sep 17 00:00:00 2001
From: Karishma Chadha <kchadha@media.mit.edu>
Date: Wed, 23 Jan 2019 18:18:38 -0500
Subject: [PATCH] Add runtime to blocks container since it was getting passed
in everywhere where it was being referenced. Update calls to blocks
constructor.
---
src/engine/blocks.js | 100 +++++++++++++++++++--------------------
src/engine/runtime.js | 4 +-
src/engine/target.js | 2 +-
src/serialization/sb2.js | 2 +-
src/serialization/sb3.js | 2 +-
src/sprites/sprite.js | 2 +-
src/virtual-machine.js | 9 ++--
7 files changed, 58 insertions(+), 63 deletions(-)
diff --git a/src/engine/blocks.js b/src/engine/blocks.js
index 85726c7f3..66cc9b953 100644
--- a/src/engine/blocks.js
+++ b/src/engine/blocks.js
@@ -17,11 +17,14 @@ const getMonitorIdForBlockWithArgs = require('../util/get-monitor-id');
/**
* Create a block container.
+ * @param {Runtime} runtime The runtime this block container operates within
* @param {boolean} optNoGlow Optional flag to indicate that blocks in this container
* should not request glows. This does not affect glows when clicking on a block to execute it.
*/
class Blocks {
- constructor (optNoGlow) {
+ constructor (runtime, optNoGlow) {
+ this.runtime = runtime;
+
/**
* All blocks in the workspace.
* Keys are block IDs, values are metadata about the block.
@@ -84,7 +87,6 @@ class Blocks {
* @type {boolean}
*/
this.forceNoGlow = optNoGlow || false;
-
}
/**
@@ -276,7 +278,7 @@ class Blocks {
}
duplicate () {
- const newBlocks = new Blocks(this.forceNoGlow);
+ const newBlocks = new Blocks(this.runtime, this.forceNoGlow);
newBlocks._blocks = Clone.simple(this._blocks);
newBlocks._scripts = Clone.simple(this._scripts);
return newBlocks;
@@ -288,23 +290,20 @@ class Blocks {
* serves as a generic adapter between the blocks, variables, and the
* runtime interface.
* @param {object} e Blockly "block" or "variable" event
- * @param {?Runtime} optRuntime Optional runtime to forward click events to.
*/
- blocklyListen (e, optRuntime) {
+ blocklyListen (e) {
// Validate event
if (typeof e !== 'object') return;
if (typeof e.blockId !== 'string' && typeof e.varId !== 'string' &&
typeof e.commentId !== 'string') {
return;
}
- const stage = optRuntime.getTargetForStage();
- const editingTarget = optRuntime.getEditingTarget();
+ const stage = this.runtime.getTargetForStage();
+ const editingTarget = this.runtime.getEditingTarget();
// UI event: clicked scripts toggle in the runtime.
if (e.element === 'stackclick') {
- if (optRuntime) {
- optRuntime.toggleScript(e.blockId, {stackClick: true});
- }
+ this.runtime.toggleScript(e.blockId, {stackClick: true});
return;
}
@@ -324,7 +323,7 @@ class Blocks {
element: e.element,
name: e.name,
value: e.newValue
- }, optRuntime);
+ });
break;
case 'move':
this.moveBlock({
@@ -337,19 +336,15 @@ class Blocks {
});
break;
case 'dragOutside':
- if (optRuntime) {
- optRuntime.emitBlockDragUpdate(e.isOutside);
- }
+ this.runtime.emitBlockDragUpdate(e.isOutside);
break;
case 'endDrag':
- if (optRuntime) {
- optRuntime.emitBlockDragUpdate(false /* areBlocksOverGui */);
+ this.runtime.emitBlockDragUpdate(false /* areBlocksOverGui */);
- // Drag blocks onto another sprite
- if (e.isOutside) {
- const newBlocks = adapter(e);
- optRuntime.emitBlockEndDrag(newBlocks, e.blockId);
- }
+ // Drag blocks onto another sprite
+ if (e.isOutside) {
+ const newBlocks = adapter(e);
+ this.runtime.emitBlockEndDrag(newBlocks, e.blockId);
}
break;
case 'delete':
@@ -360,8 +355,8 @@ class Blocks {
return;
}
// Inform any runtime to forget about glows on this script.
- if (optRuntime && this._blocks[e.blockId].topLevel) {
- optRuntime.quietGlow(e.blockId);
+ if (this._blocks[e.blockId].topLevel) {
+ this.runtime.quietGlow(e.blockId);
}
this.deleteBlock(e.blockId);
break;
@@ -379,7 +374,7 @@ class Blocks {
}
} else {
// Check for name conflicts in all of the targets
- const allTargets = optRuntime.targets.filter(t => t.isOriginal);
+ const allTargets = this.runtime.targets.filter(t => t.isOriginal);
for (const target of allTargets) {
if (target.lookupVariableByNameAndType(e.varName, e.varType, true)) {
return;
@@ -399,7 +394,7 @@ class Blocks {
// This is a global variable
stage.renameVariable(e.varId, e.newName);
// Update all blocks on all targets that use the renamed variable
- const targets = optRuntime.targets;
+ const targets = this.runtime.targets;
for (let i = 0; i < targets.length; i++) {
const currTarget = targets[i];
currTarget.blocks.updateBlocksAfterVarRename(e.varId, e.newName);
@@ -413,8 +408,8 @@ class Blocks {
break;
}
case 'comment_create':
- if (optRuntime && optRuntime.getEditingTarget()) {
- const currTarget = optRuntime.getEditingTarget();
+ if (this.runtime.getEditingTarget()) {
+ const currTarget = this.runtime.getEditingTarget();
currTarget.createComment(e.commentId, e.blockId, e.text,
e.xy.x, e.xy.y, e.width, e.height, e.minimized);
@@ -432,8 +427,8 @@ class Blocks {
}
break;
case 'comment_change':
- if (optRuntime && optRuntime.getEditingTarget()) {
- const currTarget = optRuntime.getEditingTarget();
+ if (this.runtime.getEditingTarget()) {
+ const currTarget = this.runtime.getEditingTarget();
if (!currTarget.comments.hasOwnProperty(e.commentId)) {
log.warn(`Cannot change comment with id ${e.commentId} because it does not exist.`);
return;
@@ -453,8 +448,8 @@ class Blocks {
}
break;
case 'comment_move':
- if (optRuntime && optRuntime.getEditingTarget()) {
- const currTarget = optRuntime.getEditingTarget();
+ if (this.runtime.getEditingTarget()) {
+ const currTarget = this.runtime.getEditingTarget();
if (currTarget && !currTarget.comments.hasOwnProperty(e.commentId)) {
log.warn(`Cannot change comment with id ${e.commentId} because it does not exist.`);
return;
@@ -466,8 +461,8 @@ class Blocks {
}
break;
case 'comment_delete':
- if (optRuntime && optRuntime.getEditingTarget()) {
- const currTarget = optRuntime.getEditingTarget();
+ if (this.runtime.getEditingTarget()) {
+ const currTarget = this.runtime.getEditingTarget();
if (!currTarget.comments.hasOwnProperty(e.commentId)) {
// If we're in this state, we have probably received
// a delete event from a workspace that we switched from
@@ -490,7 +485,7 @@ class Blocks {
// forceNoGlow is set to true on containers that don't affect the project serialization,
// e.g., the toolbox or monitor containers.
- if (optRuntime && !this.forceNoGlow) optRuntime.emitProjectChanged();
+ if (!this.forceNoGlow) this.runtime.emitProjectChanged();
}
// ---------------------------------------------------------------------
@@ -531,9 +526,8 @@ class Blocks {
/**
* Block management: change block field values
* @param {!object} args Blockly change event to be processed
- * @param {?Runtime} optRuntime Optional runtime to allow changeBlock to change VM state.
*/
- changeBlock (args, optRuntime) {
+ changeBlock (args) {
// Validate
if (['field', 'mutation', 'checkbox'].indexOf(args.element) === -1) return;
let block = this._blocks[args.id];
@@ -555,7 +549,7 @@ class Blocks {
if (args.name === 'VARIABLE' || args.name === 'LIST' ||
args.name === 'BROADCAST_OPTION') {
// Get variable name using the id in args.value.
- const variable = optRuntime.getEditingTarget().lookupVariableById(args.value);
+ const variable = this.runtime.getEditingTarget().lookupVariableById(args.value);
if (variable) {
block.fields[args.name].value = variable.name;
block.fields[args.name].id = args.value;
@@ -564,7 +558,8 @@ class Blocks {
// Changing the value in a dropdown
block.fields[args.name].value = args.value;
- if (!optRuntime){
+ if (!this.runtime){
+ log.warn('Runtime is not optional, it should get passed in when the block container is created.');
break;
}
// The selected item in the sensing of block menu needs to change based on the
@@ -576,12 +571,12 @@ class Blocks {
} else {
this._blocks[block.parent].fields.PROPERTY.value = 'x position';
}
- optRuntime.requestBlocksUpdate();
+ this.runtime.requestBlocksUpdate();
}
const flyoutBlock = block.shadow && block.parent ? this._blocks[block.parent] : block;
if (flyoutBlock.isMonitored) {
- optRuntime.requestUpdateMonitor(Map({
+ this.runtime.requestUpdateMonitor(Map({
id: flyoutBlock.id,
params: this._getBlockParams(flyoutBlock)
}));
@@ -592,7 +587,8 @@ class Blocks {
block.mutation = mutationAdapter(args.value);
break;
case 'checkbox': {
- if (!optRuntime) {
+ if (!this.runtime) {
+ log.warn('Runtime is not optional, it should get passed in when the block container is created.');
break;
}
@@ -609,11 +605,11 @@ class Blocks {
// the checkbox because we're using the id of the block in the flyout as the base
// check if a block with the new id already exists, otherwise create
- let newBlock = optRuntime.monitorBlocks.getBlock(newId);
+ let newBlock = this.runtime.monitorBlocks.getBlock(newId);
if (!newBlock) {
newBlock = JSON.parse(JSON.stringify(block));
newBlock.id = newId;
- optRuntime.monitorBlocks.createBlock(newBlock);
+ this.runtime.monitorBlocks.createBlock(newBlock);
}
block = newBlock; // Carry on through the rest of this code with newBlock
@@ -625,32 +621,32 @@ class Blocks {
// Variable blocks may be sprite specific depending on the owner of the variable
let isSpriteLocalVariable = false;
if (block.opcode === 'data_variable') {
- isSpriteLocalVariable = !(optRuntime.getTargetForStage().variables[block.fields.VARIABLE.id]);
+ isSpriteLocalVariable = !(this.runtime.getTargetForStage().variables[block.fields.VARIABLE.id]);
} else if (block.opcode === 'data_listcontents') {
- isSpriteLocalVariable = !(optRuntime.getTargetForStage().variables[block.fields.LIST.id]);
+ isSpriteLocalVariable = !(this.runtime.getTargetForStage().variables[block.fields.LIST.id]);
}
const isSpriteSpecific = isSpriteLocalVariable ||
- (optRuntime.monitorBlockInfo.hasOwnProperty(block.opcode) &&
- optRuntime.monitorBlockInfo[block.opcode].isSpriteSpecific);
+ (this.runtime.monitorBlockInfo.hasOwnProperty(block.opcode) &&
+ this.runtime.monitorBlockInfo[block.opcode].isSpriteSpecific);
if (isSpriteSpecific) {
// If creating a new sprite specific monitor, the only possible target is
// the current editing one b/c you cannot dynamically create monitors.
// Also, do not change the targetId if it has already been assigned
- block.targetId = block.targetId || optRuntime.getEditingTarget().id;
+ block.targetId = block.targetId || this.runtime.getEditingTarget().id;
} else {
block.targetId = null;
}
if (wasMonitored && !block.isMonitored) {
- optRuntime.requestHideMonitor(block.id);
+ this.runtime.requestHideMonitor(block.id);
} else if (!wasMonitored && block.isMonitored) {
// Tries to show the monitor for specified block. If it doesn't exist, add the monitor.
- if (!optRuntime.requestShowMonitor(block.id)) {
- optRuntime.requestAddMonitor(MonitorRecord({
+ if (!this.runtime.requestShowMonitor(block.id)) {
+ this.runtime.requestAddMonitor(MonitorRecord({
id: block.id,
targetId: block.targetId,
- spriteName: block.targetId ? optRuntime.getTargetById(block.targetId).getName() : null,
+ spriteName: block.targetId ? this.runtime.getTargetById(block.targetId).getName() : null,
opcode: block.opcode,
params: this._getBlockParams(block),
// @todo(vm#565) for numerical values with decimals, some countries use comma
diff --git a/src/engine/runtime.js b/src/engine/runtime.js
index f9a0178df..2f7131a72 100644
--- a/src/engine/runtime.js
+++ b/src/engine/runtime.js
@@ -185,14 +185,14 @@ class Runtime extends EventEmitter {
* These will execute on `_editingTarget.`
* @type {!Blocks}
*/
- this.flyoutBlocks = new Blocks(true /* force no glow */);
+ this.flyoutBlocks = new Blocks(this, true /* force no glow */);
/**
* Storage container for monitor blocks.
* These will execute on a target maybe
* @type {!Blocks}
*/
- this.monitorBlocks = new Blocks(true /* force no glow */);
+ this.monitorBlocks = new Blocks(this, true /* force no glow */);
/**
* Currently known editing target for the VM.
diff --git a/src/engine/target.js b/src/engine/target.js
index e34748fe2..7d0b40d81 100644
--- a/src/engine/target.js
+++ b/src/engine/target.js
@@ -25,7 +25,7 @@ class Target extends EventEmitter {
super();
if (!blocks) {
- blocks = new Blocks();
+ blocks = new Blocks(runtime);
}
/**
diff --git a/src/serialization/sb2.js b/src/serialization/sb2.js
index 8cc49cd8a..e925929fd 100644
--- a/src/serialization/sb2.js
+++ b/src/serialization/sb2.js
@@ -413,7 +413,7 @@ const parseScratchObject = function (object, runtime, extensions, topLevel, zip)
}
// Blocks container for this object.
- const blocks = new Blocks();
+ const blocks = new Blocks(runtime);
// @todo: For now, load all Scratch objects (stage/sprites) as a Sprite.
const sprite = new Sprite(blocks, runtime);
// Sprite/stage name from JSON.
diff --git a/src/serialization/sb3.js b/src/serialization/sb3.js
index 1b3e01729..31acdd6f0 100644
--- a/src/serialization/sb3.js
+++ b/src/serialization/sb3.js
@@ -834,7 +834,7 @@ const parseScratchObject = function (object, runtime, extensions, zip) {
return Promise.resolve(null);
}
// Blocks container for this object.
- const blocks = new Blocks();
+ const blocks = new Blocks(runtime);
// @todo: For now, load all Scratch objects (stage/sprites) as a Sprite.
const sprite = new Sprite(blocks, runtime);
diff --git a/src/sprites/sprite.js b/src/sprites/sprite.js
index 482d1b0ba..6874f91bf 100644
--- a/src/sprites/sprite.js
+++ b/src/sprites/sprite.js
@@ -18,7 +18,7 @@ class Sprite {
this.runtime = runtime;
if (!blocks) {
// Shared set of blocks for all clones.
- blocks = new Blocks();
+ blocks = new Blocks(runtime);
}
this.blocks = blocks;
/**
diff --git a/src/virtual-machine.js b/src/virtual-machine.js
index 599d23405..151815d75 100644
--- a/src/virtual-machine.js
+++ b/src/virtual-machine.js
@@ -1105,7 +1105,7 @@ class VirtualMachine extends EventEmitter {
*/
blockListener (e) {
if (this.editingTarget) {
- this.editingTarget.blocks.blocklyListen(e, this.runtime);
+ this.editingTarget.blocks.blocklyListen(e);
}
}
@@ -1114,7 +1114,7 @@ class VirtualMachine extends EventEmitter {
* @param {!Blockly.Event} e Any Blockly event.
*/
flyoutBlockListener (e) {
- this.runtime.flyoutBlocks.blocklyListen(e, this.runtime);
+ this.runtime.flyoutBlocks.blocklyListen(e);
}
/**
@@ -1125,7 +1125,7 @@ class VirtualMachine extends EventEmitter {
// Filter events by type, since monitor blocks only need to listen to these events.
// Monitor blocks shouldn't be destroyed when flyout blocks are deleted.
if (['create', 'change'].indexOf(e.type) !== -1) {
- this.runtime.monitorBlocks.blocklyListen(e, this.runtime);
+ this.runtime.monitorBlocks.blocklyListen(e);
}
}
@@ -1137,8 +1137,7 @@ class VirtualMachine extends EventEmitter {
// Filter events by type, since blocks only needs to listen to these
// var events.
if (['var_create', 'var_rename', 'var_delete'].indexOf(e.type) !== -1) {
- this.runtime.getTargetForStage().blocks.blocklyListen(e,
- this.runtime);
+ this.runtime.getTargetForStage().blocks.blocklyListen(e);
}
}