From 875ccf5b888f469f692a5e03c85a96b32a802b43 Mon Sep 17 00:00:00 2001 From: marisaleung Date: Thu, 15 Jun 2017 14:29:15 -0700 Subject: [PATCH] Updated variables and var_fire event listener. --- src/engine/adapter.js | 3 + src/engine/blocks.js | 22 +++++-- src/engine/execute.js | 6 +- src/engine/target.js | 55 ++++++++++++++--- src/engine/variable.js | 10 ++- src/playground/playground.js | 1 + src/serialization/sb2.js | 4 +- src/serialization/sb3.js | 4 +- src/virtual-machine.js | 23 ++++--- test/unit/engine_target.js | 116 +++++++++++++++++++++++++++++++++++ 10 files changed, 216 insertions(+), 28 deletions(-) create mode 100644 test/unit/engine_target.js diff --git a/src/engine/adapter.js b/src/engine/adapter.js index e664770a0..e9c7c3243 100644 --- a/src/engine/adapter.js +++ b/src/engine/adapter.js @@ -60,6 +60,8 @@ const domToBlock = function (blockDOM, blocks, isTopBlock, parent) { { // Add the field to this block. const fieldName = xmlChild.attribs.name; + // Add id in case it is a variable field + const fieldId = xmlChild.attribs.id; let fieldData = ''; if (xmlChild.children.length > 0 && xmlChild.children[0].data) { fieldData = xmlChild.children[0].data; @@ -70,6 +72,7 @@ const domToBlock = function (blockDOM, blocks, isTopBlock, parent) { } block.fields[fieldName] = { name: fieldName, + id: fieldId, value: fieldData }; break; diff --git a/src/engine/blocks.js b/src/engine/blocks.js index 451437c6e..1acc7774f 100644 --- a/src/engine/blocks.js +++ b/src/engine/blocks.js @@ -181,16 +181,19 @@ class Blocks { // --------------------------------------------------------------------- /** - * Create event listener for blocks. Handles validation and serves as a generic - * adapter between the blocks and the runtime interface. - * @param {Object} e Blockly "block" event + * Create event listener for blocks and variables. Handles validation and + * 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) { // Validate event if (typeof e !== 'object') return; - if (typeof e.blockId !== 'string') return; + if (typeof e.blockId !== 'string' && typeof e.varId !== 'string') { + return; + } + const stage = optRuntime.getTargetForStage(); // UI event: clicked scripts toggle in the runtime. if (e.element === 'stackclick') { @@ -243,6 +246,15 @@ class Blocks { id: e.blockId }); break; + case 'var_create': + stage.createVariable(e.varId, e.varName); + break; + case 'var_rename': + stage.renameVariable(e.varId, e.newName); + break; + case 'var_delete': + stage.deleteVariable(e.varId); + break; } } diff --git a/src/engine/execute.js b/src/engine/execute.js index 80c5b32ab..83530d380 100644 --- a/src/engine/execute.js +++ b/src/engine/execute.js @@ -137,7 +137,11 @@ const execute = function (sequencer, thread) { // Add all fields on this block to the argValues. for (const fieldName in fields) { if (!fields.hasOwnProperty(fieldName)) continue; - argValues[fieldName] = fields[fieldName].value; + if (fieldName === 'VARIABLE') { + argValues[fieldName] = fields[fieldName].id; + } else { + argValues[fieldName] = fields[fieldName].value; + } } // Recursively evaluate input blocks. diff --git a/src/engine/target.js b/src/engine/target.js index c4f690aa9..aac3a2052 100644 --- a/src/engine/target.js +++ b/src/engine/target.js @@ -72,24 +72,25 @@ class Target extends EventEmitter { /** * Look up a variable object, and create it if one doesn't exist. * Search begins for local variables; then look for globals. - * @param {!string} name Name of the variable. + * @param {string} id Id of the variable. + * @param {string} name Name of the variable. * @return {!Variable} Variable object. */ - lookupOrCreateVariable (name) { + lookupOrCreateVariable (id, name) { // If we have a local copy, return it. - if (this.variables.hasOwnProperty(name)) { - return this.variables[name]; + if (this.variables.hasOwnProperty(id)) { + return this.variables[id]; } // If the stage has a global copy, return it. if (this.runtime && !this.isStage) { const stage = this.runtime.getTargetForStage(); - if (stage.variables.hasOwnProperty(name)) { - return stage.variables[name]; + if (stage.variables.hasOwnProperty(id)) { + return stage.variables[id]; } } // No variable with this name exists - create it locally. - const newVariable = new Variable(name, 0, false); - this.variables[name] = newVariable; + const newVariable = new Variable(id, name, 0, false); + this.variables[id] = newVariable; return newVariable; } @@ -117,6 +118,44 @@ class Target extends EventEmitter { return newList; } + /** + * Creates a variable with the given id and name and adds it to the + * dictionary of variables. + * @param {string} id Id of variable + * @param {string} name Name of variable. + */ + createVariable (id, name) { + if (!this.variables.hasOwnProperty(id)) { + const newVariable = new Variable(id, name, 0, + false); + this.variables[id] = newVariable; + } + } + + /** + * Renames the variable with the given id to newName. + * @param {string} id Id of renamed variable. + * @param {string} newName New name for the variable. + */ + renameVariable (id, newName) { + if (this.variables.hasOwnProperty(id)) { + const variable = this.variables[id]; + if (variable.id === id) { + variable.name = newName; + } + } + } + + /** + * Removes the variable with the given id from the dictionary of variables. + * @param {string} id Id of renamed variable. + */ + deleteVariable (id) { + if (this.variables.hasOwnProperty(id)) { + delete this.variables[id]; + } + } + /** * Post/edit sprite info. * @param {object} data An object with sprite info data to set. diff --git a/src/engine/variable.js b/src/engine/variable.js index 69cb915c8..b79ab2975 100644 --- a/src/engine/variable.js +++ b/src/engine/variable.js @@ -3,21 +3,25 @@ * Object representing a Scratch variable. */ +const uid = require('../util/uid'); + class Variable { /** - * @param {!string} name Name of the variable. + * @param {string} id Id of the variable. + * @param {string} name Name of the variable. * @param {(string|number)} value Value of the variable. * @param {boolean} isCloud Whether the variable is stored in the cloud. * @constructor */ - constructor (name, value, isCloud) { + constructor (id, name, value, isCloud) { + this.id = id || uid(); this.name = name; this.value = value; this.isCloud = isCloud; } toXML () { - return `${this.name}`; + return `${this.name}`; } } diff --git a/src/playground/playground.js b/src/playground/playground.js index 814bfb628..d9aac1c30 100644 --- a/src/playground/playground.js +++ b/src/playground/playground.js @@ -90,6 +90,7 @@ window.onload = function () { // Attach scratch-blocks events to VM. workspace.addChangeListener(vm.blockListener); + workspace.addChangeListener(vm.variableListener); const flyoutWorkspace = workspace.getFlyout().getWorkspace(); flyoutWorkspace.addChangeListener(vm.flyoutBlockListener); flyoutWorkspace.addChangeListener(vm.monitorBlockListener); diff --git a/src/serialization/sb2.js b/src/serialization/sb2.js index a54b582f2..2946756de 100644 --- a/src/serialization/sb2.js +++ b/src/serialization/sb2.js @@ -191,11 +191,13 @@ const parseScratchObject = function (object, runtime, topLevel) { if (object.hasOwnProperty('variables')) { for (let j = 0; j < object.variables.length; j++) { const variable = object.variables[j]; - target.variables[variable.name] = new Variable( + const newVariable = new Variable( + null, variable.name, variable.value, variable.isPersistent ); + target.variables[newVariable.id] = newVariable; } } if (object.hasOwnProperty('lists')) { diff --git a/src/serialization/sb3.js b/src/serialization/sb3.js index 816b1af19..864a7aa25 100644 --- a/src/serialization/sb3.js +++ b/src/serialization/sb3.js @@ -98,11 +98,13 @@ const parseScratchObject = function (object, runtime) { if (object.hasOwnProperty('variables')) { for (let j = 0; j < object.variables.length; j++) { const variable = object.variables[j]; - target.variables[variable.name] = new Variable( + const newVariable = new Variable( + variable.id, variable.name, variable.value, variable.isPersistent ); + target.variables[newVariable.id] = newVariable; } } if (object.hasOwnProperty('lists')) { diff --git a/src/virtual-machine.js b/src/virtual-machine.js index 12e6e7a77..be1e443d8 100644 --- a/src/virtual-machine.js +++ b/src/virtual-machine.js @@ -62,6 +62,7 @@ class VirtualMachine extends EventEmitter { this.blockListener = this.blockListener.bind(this); this.flyoutBlockListener = this.flyoutBlockListener.bind(this); this.monitorBlockListener = this.monitorBlockListener.bind(this); + this.variableListener = this.variableListener.bind(this); } /** @@ -446,6 +447,19 @@ class VirtualMachine extends EventEmitter { } } + /** + * Handle a Blockly event for the variable map. + * @param {!Blockly.Event} e Any Blockly event. + */ + variableListener (e) { + // 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); + } + } + /** * Set an editing target. An editor UI can use this function to switch * between editing different targets, sprites, etc. @@ -550,15 +564,6 @@ class VirtualMachine extends EventEmitter { postSpriteInfo (data) { this.editingTarget.postSpriteInfo(data); } - - /** - * Create a variable by name. - * @todo this only creates global variables by putting them on the stage - * @param {string} name The name of the variable - */ - createVariable (name) { - this.runtime.getTargetForStage().lookupOrCreateVariable(name); - } } module.exports = VirtualMachine; diff --git a/test/unit/engine_target.js b/test/unit/engine_target.js new file mode 100644 index 000000000..cec5cf16d --- /dev/null +++ b/test/unit/engine_target.js @@ -0,0 +1,116 @@ +const test = require('tap').test; +const Target = require('../../src/engine/target'); + +test('spec', t => { + const target = new Target(); + + t.type(Target, 'function'); + t.type(target, 'object'); + t.ok(target instanceof Target); + + t.type(target.id, 'string'); + t.type(target.blocks, 'object'); + t.type(target.variables, 'object'); + t.type(target.lists, 'object'); + t.type(target._customState, 'object'); + + t.type(target.createVariable, 'function'); + t.type(target.renameVariable, 'function'); + + t.end(); +}); + +// Create Variable tests. +test('createVariable', t => { + const target = new Target(); + target.createVariable('foo', 'bar'); + + const variables = target.variables; + t.equal(Object.keys(variables).length, 1); + const variable = variables[Object.keys(variables)[0]]; + t.equal(variable.id, 'foo'); + t.equal(variable.name, 'bar'); + t.equal(variable.value, 0); + t.equal(variable.isCloud, false); + + t.end(); +}); + +// Create Same Variable twice. +test('createVariable2', t => { + const target = new Target(); + target.createVariable('foo', 'bar'); + target.createVariable('foo', 'bar'); + + const variables = target.variables; + t.equal(Object.keys(variables).length, 1); + + t.end(); +}); + +// Rename Variable tests. +test('renameVariable', t => { + const target = new Target(); + target.createVariable('foo', 'bar'); + target.renameVariable('foo', 'bar2'); + + const variables = target.variables; + t.equal(Object.keys(variables).length, 1); + const variable = variables[Object.keys(variables)[0]]; + t.equal(variable.id, 'foo'); + t.equal(variable.name, 'bar2'); + t.equal(variable.value, 0); + t.equal(variable.isCloud, false); + + t.end(); +}); + +// Rename Variable that doesn't exist. +test('renameVariable2', t => { + const target = new Target(); + target.renameVariable('foo', 'bar2'); + + const variables = target.variables; + t.equal(Object.keys(variables).length, 0); + + t.end(); +}); + +// Rename Variable that with id that exists as another variable's name. +// Expect no change. +test('renameVariable3', t => { + const target = new Target(); + target.createVariable('foo1', 'foo'); + target.renameVariable('foo', 'bar2'); + + const variables = target.variables; + t.equal(Object.keys(variables).length, 1); + const variable = variables[Object.keys(variables)[0]]; + t.equal(variable.id, 'foo1'); + t.equal(variable.name, 'foo'); + + t.end(); +}); + +// Delete Variable tests. +test('deleteVariable', t => { + const target = new Target(); + target.createVariable('foo', 'bar'); + target.deleteVariable('foo'); + + const variables = target.variables; + t.equal(Object.keys(variables).length, 0); + + t.end(); +}); + +// Delete Variable that doesn't exist. +test('deleteVariable2', t => { + const target = new Target(); + target.deleteVariable('foo'); + + const variables = target.variables; + t.equal(Object.keys(variables).length, 0); + + t.end(); +});