diff --git a/src/engine/blocks.js b/src/engine/blocks.js index 787f03116..218e9271d 100644 --- a/src/engine/blocks.js +++ b/src/engine/blocks.js @@ -263,6 +263,7 @@ class Blocks { return; } const stage = optRuntime.getTargetForStage(); + const editingTarget = optRuntime.getEditingTarget(); // UI event: clicked scripts toggle in the runtime. if (e.element === 'stackclick') { @@ -330,25 +331,39 @@ class Blocks { this.deleteBlock(e.blockId); break; case 'var_create': - // New variables being created by the user are all global. - // Check if this variable exists on the current target or stage. - // If not, create it on the stage. - // TODO create global and local variables when UI provides a way. - if (optRuntime.getEditingTarget()) { - if (!optRuntime.getEditingTarget().lookupVariableById(e.varId)) { - stage.createVariable(e.varId, e.varName, e.varType); + // Check if the variable being created is global or local + // If local, create a local var on the current editing target, as long + // as there are no conflicts, and the current target is actually a sprite + // If global or if the editing target is not present or we somehow got + // into a state where a local var was requested for the stage, + // create a stage (global) var after checking for name conflicts + // on all the sprites. + if (e.isLocal && editingTarget && !editingTarget.isStage) { + if (!editingTarget.lookupVariableById(e.varId)) { + editingTarget.createVariable(e.varId, e.varName, e.varType); + } + } else { + // Check for name conflicts in all of the targets + const allTargets = optRuntime.targets.filter(t => t.isOriginal); + for (const target of allTargets) { + if (target.lookupVariableByNameAndType(e.varName, e.varType, true)) { + return; + } } - } else if (!stage.lookupVariableById(e.varId)) { - // Since getEditingTarget returned null, we now need to - // explicitly check if the stage has the variable, and - // create one if not. stage.createVariable(e.varId, e.varName, e.varType); } break; case 'var_rename': - stage.renameVariable(e.varId, e.newName); - // Update all the blocks that use the renamed variable. - if (optRuntime) { + if (editingTarget && editingTarget.hasOwnProperty(e.varId)) { + // This is a local variable, rename on the current target + editingTarget.renameVariable(e.varId, e.newName); + // Update all the blocks on the current target that use + // this variable + editingTarget.blocks.updateBlocksAfterVarRename(e.varId, e.newName); + } else { + // 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; for (let i = 0; i < targets.length; i++) { const currTarget = targets[i]; @@ -356,9 +371,12 @@ class Blocks { } } break; - case 'var_delete': - stage.deleteVariable(e.varId); + case 'var_delete': { + const target = (editingTarget && editingTarget.hasOwnProperty(e.varId)) ? + editingTarget : stage; + target.deleteVariable(e.varId); break; + } case 'comment_create': if (optRuntime && optRuntime.getEditingTarget()) { const currTarget = optRuntime.getEditingTarget(); diff --git a/src/engine/runtime.js b/src/engine/runtime.js index 517ae48d9..1ba19570b 100644 --- a/src/engine/runtime.js +++ b/src/engine/runtime.js @@ -1715,6 +1715,15 @@ class Runtime extends EventEmitter { return this._editingTarget; } + getAllVarNamesOfType (varType) { + let varNames = []; + for (const target of this.targets) { + const targetVarNames = target.getAllVariableNamesInScopeByType(varType, true); + varNames = varNames.concat(targetVarNames); + } + return varNames; + } + /** * Tell the runtime to request a redraw. * Use after a clone/sprite has completed some visible operation on the stage. diff --git a/src/engine/target.js b/src/engine/target.js index 69620c91b..89cb27d06 100644 --- a/src/engine/target.js +++ b/src/engine/target.js @@ -83,17 +83,19 @@ class Target extends EventEmitter { /** * Get the names of all the variables of the given type that are in scope for this target. * For targets that are not the stage, this includes any target-specific - * variables as well as any stage variables. + * variables as well as any stage variables unless the skipStage flag is true. * For the stage, this is all stage variables. * @param {string} type The variable type to search for; defaults to Variable.SCALAR_TYPE + * @param {?bool} skipStage Optional flag to skip the stage. * @return {Array} A list of variable names */ - getAllVariableNamesInScopeByType (type) { + getAllVariableNamesInScopeByType (type, skipStage) { if (typeof type !== 'string') type = Variable.SCALAR_TYPE; + skipStage = skipStage || false; const targetVariables = Object.values(this.variables) .filter(v => v.type === type) .map(variable => variable.name); - if (this.isStage || !this.runtime) { + if (skipStage || this.isStage || !this.runtime) { return targetVariables; } const stage = this.runtime.getTargetForStage(); @@ -194,11 +196,13 @@ class Target extends EventEmitter { * was not found. * @param {string} name Name of the variable. * @param {string} type Type of the variable. Defaults to Variable.SCALAR_TYPE. + * @param {?bool} skipStage Optional flag to skip checking the stage * @return {?Variable} Variable object if found, or null if not. */ - lookupVariableByNameAndType (name, type) { + lookupVariableByNameAndType (name, type, skipStage) { if (typeof name !== 'string') return; if (typeof type !== 'string') type = Variable.SCALAR_TYPE; + skipStage = skipStage || false; for (const varId in this.variables) { const currVar = this.variables[varId]; @@ -207,7 +211,7 @@ class Target extends EventEmitter { } } - if (this.runtime && !this.isStage) { + if (!skipStage && this.runtime && !this.isStage) { const stage = this.runtime.getTargetForStage(); if (stage) { for (const varId in stage.variables) { diff --git a/src/engine/variable.js b/src/engine/variable.js index 4aad9ffc2..c56a3cf01 100644 --- a/src/engine/variable.js +++ b/src/engine/variable.js @@ -33,8 +33,9 @@ class Variable { } } - toXML () { - return `${this.name}`; + toXML (isLocal) { + isLocal = (isLocal === true); + return `${this.name}`; } /** diff --git a/src/virtual-machine.js b/src/virtual-machine.js index 940991e49..922c2fc20 100644 --- a/src/virtual-machine.js +++ b/src/virtual-machine.js @@ -1074,19 +1074,21 @@ class VirtualMachine extends EventEmitter { const id = messageIds[i]; delete this.runtime.getTargetForStage().variables[id]; } - const variableMap = Object.assign({}, - this.runtime.getTargetForStage().variables, - this.editingTarget.variables - ); + const globalVarMap = Object.assign({}, this.runtime.getTargetForStage().variables); + const localVarMap = this.editingTarget.isStage ? + Object.create(null) : + Object.assign({}, this.editingTarget.variables); - const variables = Object.keys(variableMap).map(k => variableMap[k]); + const globalVariables = Object.keys(globalVarMap).map(k => globalVarMap[k]); + const localVariables = Object.keys(localVarMap).map(k => localVarMap[k]); const workspaceComments = Object.keys(this.editingTarget.comments) .map(k => this.editingTarget.comments[k]) .filter(c => c.blockId === null); const xmlString = ` - ${variables.map(v => v.toXML()).join()} + ${globalVariables.map(v => v.toXML()).join()} + ${localVariables.map(v => v.toXML(true)).join()} ${workspaceComments.map(c => c.toXML()).join()} ${this.editingTarget.blocks.toXML(this.editingTarget.comments)}