diff --git a/blocks/procedures.js b/blocks/procedures.js index a6800f934..bc53990f0 100644 --- a/blocks/procedures.js +++ b/blocks/procedures.js @@ -59,6 +59,7 @@ Blockly.Blocks['procedures_defnoreturn'] = { this.setTooltip(Blockly.Msg.PROCEDURES_DEFNORETURN_TOOLTIP); this.setHelpUrl(Blockly.Msg.PROCEDURES_DEFNORETURN_HELPURL); this.arguments_ = []; + this.argumentVarModels_ = []; this.setStatements_(true); this.statementConnection_ = null; }, @@ -153,9 +154,14 @@ Blockly.Blocks['procedures_defnoreturn'] = { */ domToMutation: function(xmlElement) { this.arguments_ = []; + this.argumentVarModels_ = []; for (var i = 0, childNode; childNode = xmlElement.childNodes[i]; i++) { if (childNode.nodeName.toLowerCase() == 'arg') { - this.arguments_.push(childNode.getAttribute('name')); + var varName = childNode.getAttribute('name'); + this.arguments_.push(varName); + var variable = Blockly.Variables.getOrCreateVariable(this.workspace, + null, varName, ''); + this.argumentVarModels_.push(variable); } } this.updateParams_(); @@ -206,9 +212,14 @@ Blockly.Blocks['procedures_defnoreturn'] = { // Parameter list. this.arguments_ = []; this.paramIds_ = []; + this.argumentVarModels_ = []; var paramBlock = containerBlock.getInputTargetBlock('STACK'); while (paramBlock) { - this.arguments_.push(paramBlock.getFieldValue('NAME')); + var varName = paramBlock.getFieldValue('NAME'); + this.arguments_.push(varName); + var variable = Blockly.Variables.getOrCreateVariable(this.workspace, null, + varName, ''); + this.argumentVarModels_.push(variable); this.paramIds_.push(paramBlock.id); paramBlock = paramBlock.nextConnection && paramBlock.nextConnection.targetBlock(); @@ -265,46 +276,67 @@ Blockly.Blocks['procedures_defnoreturn'] = { * @this Blockly.Block */ getVarModels: function() { - var vars = []; - // Not fully initialized. - if (!this.arguments_) { - return vars; - } - for (var i = 0, argName; argName = this.arguments_[i]; i++) { - // TODO (#1494): When we switch to tracking procedure arguments by ID, - // update this. - var model = this.workspace.getVariable(argName, ''); - if (model) { - vars.push(model); - } - } - return vars; + return this.argumentVarModels_; }, /** * Notification that a variable is renaming. * If the name matches one of this block's variables, rename it. - * @param {string} oldName Previous name of variable. - * @param {string} newName Renamed variable. + * @param {string} oldId ID of variable to rename. + * @param {string} newId ID of new variable. May be the same as oldId, but + * with an updated name. Guaranteed to be the same type as the old + * variable. * @this Blockly.Block */ - renameVar: function(oldName, newName) { + renameVarById: function(oldId, newId) { + var oldVariable = this.workspace.getVariableById(oldId); + if (oldVariable.type != '') { + // Procedure arguments always have the empty type. + return; + } + var oldName = oldVariable.name; + var newVar = this.workspace.getVariableById(newId); + var change = false; - for (var i = 0; i < this.arguments_.length; i++) { - if (Blockly.Names.equals(oldName, this.arguments_[i])) { + for (var i = 0; i < this.argumentVarModels_.length; i++) { + if (this.argumentVarModels_[i].getId() == oldId) { + this.arguments_[i] = newVar.name; + this.argumentVarModels_[i] = newVar; + change = true; + } + } + if (change) { + this.displayRenamedVar_(oldName, newVar.name); + } + }, + updateVarName: function(variable) { + var newName = variable.name; + var change = false; + for (var i = 0; i < this.argumentVarModels_.length; i++) { + if (this.argumentVarModels_[i].getId() == variable.getId()) { + var oldName = this.arguments_[i]; this.arguments_[i] = newName; change = true; } } if (change) { - this.updateParams_(); - // Update the mutator's variables if the mutator is open. - if (this.mutator.isVisible()) { - var blocks = this.mutator.workspace_.getAllBlocks(); - for (var i = 0, block; block = blocks[i]; i++) { - if (block.type == 'procedures_mutatorarg' && - Blockly.Names.equals(oldName, block.getFieldValue('NAME'))) { - block.setFieldValue(newName, 'NAME'); - } + this.displayRenamedVar_(oldName, newName); + } + }, + /** + * Update the display to reflect a newly renamed argument. + * @param {string} oldName The old display name of the argument. + * @param {string} newName The new display name of the argument. + * @private + */ + displayRenamedVar_: function(oldName, newName) { + this.updateParams_(); + // Update the mutator's variables if the mutator is open. + if (this.mutator.isVisible()) { + var blocks = this.mutator.workspace_.getAllBlocks(); + for (var i = 0, block; block = blocks[i]; i++) { + if (block.type == 'procedures_mutatorarg' && + Blockly.Names.equals(oldName, block.getFieldValue('NAME'))) { + block.setFieldValue(newName, 'NAME'); } } } @@ -376,6 +408,7 @@ Blockly.Blocks['procedures_defreturn'] = { this.setTooltip(Blockly.Msg.PROCEDURES_DEFRETURN_TOOLTIP); this.setHelpUrl(Blockly.Msg.PROCEDURES_DEFRETURN_HELPURL); this.arguments_ = []; + this.argumentVarModels_ = []; this.setStatements_(true); this.statementConnection_ = null; }, @@ -398,7 +431,9 @@ Blockly.Blocks['procedures_defreturn'] = { }, getVars: Blockly.Blocks['procedures_defnoreturn'].getVars, getVarModels: Blockly.Blocks['procedures_defnoreturn'].getVarModels, - renameVar: Blockly.Blocks['procedures_defnoreturn'].renameVar, + renameVarById: Blockly.Blocks['procedures_defnoreturn'].renameVarById, + updateVarName: Blockly.Blocks['procedures_defnoreturn'].updateVarName, + displayRenamedVar_: Blockly.Blocks['procedures_defnoreturn'].displayRenamedVar_, customContextMenu: Blockly.Blocks['procedures_defnoreturn'].customContextMenu, callType_: 'procedures_callreturn' }; @@ -493,6 +528,7 @@ Blockly.Blocks['procedures_callnoreturn'] = { // Tooltip is set in renameProcedure. this.setHelpUrl(Blockly.Msg.PROCEDURES_CALLNORETURN_HELPURL); this.arguments_ = []; + this.argumentVarModels_ = []; this.quarkConnections_ = {}; this.quarkIds_ = null; }, @@ -591,6 +627,14 @@ Blockly.Blocks['procedures_callnoreturn'] = { } // Rebuild the block's arguments. this.arguments_ = [].concat(paramNames); + // And rebuild the argument model list. + this.argumentVarModels_ = []; + for (var i = 0; i < this.arguments_.length; i++) { + var variable = Blockly.Variables.getOrCreateVariable(this.workspace, null, + this.arguments_[i], ''); + this.argumentVarModels_.push(variable); + } + this.updateShape_(); this.quarkIds_ = paramIds; // Reconnect any child blocks. @@ -695,18 +739,30 @@ Blockly.Blocks['procedures_callnoreturn'] = { /** * Notification that a variable is renaming. * If the name matches one of this block's variables, rename it. - * @param {string} oldName Previous name of variable. - * @param {string} newName Renamed variable. + * @param {string} oldId ID of variable to rename. + * @param {string} newId ID of new variable. May be the same as oldId, but + * with an updated name. Guaranteed to be the same type as the old + * variable. * @this Blockly.Block */ - renameVar: function(oldName, newName) { + renameVarById: function(oldId, newId) { + var newVar = this.workspace.getVariableById(newId); for (var i = 0; i < this.arguments_.length; i++) { - if (Blockly.Names.equals(oldName, this.arguments_[i])) { - this.arguments_[i] = newName; - this.getField('ARGNAME' + i).setValue(newName); + if (oldId == this.argumentVarModels_[i].getId()) { + this.arguments_[i] = newVar.name; + this.argumentVarModels_[i] = newVar; + this.getField('ARGNAME' + i).setValue(newVar.name); } } }, + /** + * Return all variables referenced by this block. + * @return {!Array.} List of variable models. + * @this Blockly.Block + */ + getVarModels: function() { + return this.argumentVarModels_; + }, /** * Procedure calls cannot exist without the corresponding procedure * definition. Enforce this link whenever an event is fired. @@ -816,7 +872,7 @@ Blockly.Blocks['procedures_callreturn'] = { updateShape_: Blockly.Blocks['procedures_callnoreturn'].updateShape_, mutationToDom: Blockly.Blocks['procedures_callnoreturn'].mutationToDom, domToMutation: Blockly.Blocks['procedures_callnoreturn'].domToMutation, - renameVar: Blockly.Blocks['procedures_callnoreturn'].renameVar, + getVarModels: Blockly.Blocks['procedures_callnoreturn'].getVarModels, onchange: Blockly.Blocks['procedures_callnoreturn'].onchange, customContextMenu: Blockly.Blocks['procedures_callnoreturn'].customContextMenu, diff --git a/core/field_variable.js b/core/field_variable.js index 5a5ab5def..5e654fde2 100644 --- a/core/field_variable.js +++ b/core/field_variable.js @@ -137,6 +137,18 @@ Blockly.FieldVariable.prototype.getText = function() { return this.variable_ ? this.variable_.name : ''; }; +/** + * Get the variable model for the selected variable. + * Not guaranteed to be in the variable map on the workspace (e.g. if accessed + * after the variable has been deleted). + * @return {?Blockly.VariableModel} the selected variable, or null if none was + * selected. + * @package + */ +Blockly.FieldVariable.prototype.getVariable = function() { + return this.variable_; +}; + /** * Set the variable ID. * @param {string} id New variable ID, which must reference an existing diff --git a/core/workspace.js b/core/workspace.js index 29dfb860d..e0bf956b2 100644 --- a/core/workspace.js +++ b/core/workspace.js @@ -144,6 +144,8 @@ Blockly.Workspace.prototype.addTopBlock = function(block) { var variableNames = Blockly.Variables.allUsedVariables(block); for (var i = 0, name; name = variableNames[i]; i++) { if (!this.getVariable(name)) { + // TODO (fenichel): Is this still necessary? Is allUsedVariables still + // necessary? this.createVariable(name); } } diff --git a/core/xml.js b/core/xml.js index 347e0eb8a..a3e42bc54 100644 --- a/core/xml.js +++ b/core/xml.js @@ -89,19 +89,20 @@ Blockly.Xml.blockToDomWithXY = function(block, opt_noId) { /** * Encode a variable field as XML. * @param {!Blockly.Field} field The field to encode. - * @param {!Blockly.Workspace} workspace The workspace that the field is in. * @return {?Element} XML element, or null if the field did not need to be * serialized. * @private */ -Blockly.Xml.fieldToDomVariable_ = function(field, workspace) { +Blockly.Xml.fieldToDomVariable_ = function(field) { var id = field.getValue(); // The field had not been initialized fully before being serialized. if (id == null) { field.initModel(); id = field.getValue(); } - var variable = Blockly.Variables.getVariable(workspace, id); + // Get the variable directly from the field, instead of doing a lookup. This + // will work even if the variable has already been deleted. + var variable = field.getVariable(); if (!variable) { throw Error('Tried to serialize a variable field with no variable.'); @@ -121,10 +122,10 @@ Blockly.Xml.fieldToDomVariable_ = function(field, workspace) { * serialized. * @private */ -Blockly.Xml.fieldToDom_ = function(field, workspace) { +Blockly.Xml.fieldToDom_ = function(field) { if (field.name && field.EDITABLE) { if (field instanceof Blockly.FieldVariable) { - return Blockly.Xml.fieldToDomVariable_(field, workspace); + return Blockly.Xml.fieldToDomVariable_(field); } else { var container = goog.dom.createDom('field', null, field.getValue()); container.setAttribute('name', field.name); @@ -143,10 +144,9 @@ Blockly.Xml.fieldToDom_ = function(field, workspace) { * @private */ Blockly.Xml.allFieldsToDom_ = function(block, element) { - var workspace = block.workspace; for (var i = 0, input; input = block.inputList[i]; i++) { for (var j = 0, field; field = input.fieldRow[j]; j++) { - var fieldDom = Blockly.Xml.fieldToDom_(field, workspace); + var fieldDom = Blockly.Xml.fieldToDom_(field); if (fieldDom) { element.appendChild(fieldDom); } @@ -728,8 +728,8 @@ Blockly.Xml.domToBlockHeadless_ = function(xmlBlock, workspace) { 'Shadow block not allowed non-shadow child.'); } // Ensure this block doesn't have any variable inputs. - goog.asserts.assert(block.getVars().length == 0, - 'Shadow blocks cannot have variable fields.'); + goog.asserts.assert(block.getVarModels().length == 0, + 'Shadow blocks cannot have variable references.'); block.setShadow(true); } return block; diff --git a/generators/dart.js b/generators/dart.js index 1d3c93b75..e89c160c8 100644 --- a/generators/dart.js +++ b/generators/dart.js @@ -108,7 +108,7 @@ Blockly.Dart.init = function(workspace) { // Add user variables. var variables = workspace.getAllVariables(); for (var i = 0; i < variables.length; i++) { - defvars[i] = Blockly.Dart.variableDB_.getName(variables[i].name, + defvars[i] = Blockly.Dart.variableDB_.getName(variables[i].getId(), Blockly.Variables.NAME_TYPE); } diff --git a/generators/javascript.js b/generators/javascript.js index 864ae50c3..c7396591b 100644 --- a/generators/javascript.js +++ b/generators/javascript.js @@ -158,7 +158,7 @@ Blockly.JavaScript.init = function(workspace) { // Add user variables. var variables = workspace.getAllVariables(); for (var i = 0; i < variables.length; i++) { - defvars[i] = Blockly.JavaScript.variableDB_.getName(variables[i].name, + defvars[i] = Blockly.JavaScript.variableDB_.getName(variables[i].getId(), Blockly.Variables.NAME_TYPE); } diff --git a/generators/php.js b/generators/php.js index 6dc81ba62..3f999528e 100644 --- a/generators/php.js +++ b/generators/php.js @@ -152,11 +152,9 @@ Blockly.PHP.init = function(workspace) { Blockly.PHP.variableDB_.setVariableMap(workspace.getVariableMap()); var defvars = []; - var varName; var variables = workspace.getAllVariables(); for (var i = 0, variable; variable = variables[i]; i++) { - varName = variable.name; - defvars[i] = Blockly.PHP.variableDB_.getName(varName, + defvars[i] = Blockly.PHP.variableDB_.getName(variable.getId(), Blockly.Variables.NAME_TYPE) + ';'; } diff --git a/generators/python.js b/generators/python.js index ab927d13b..4b88a6fad 100644 --- a/generators/python.js +++ b/generators/python.js @@ -165,7 +165,7 @@ Blockly.Python.init = function(workspace) { var defvars = []; var variables = workspace.getAllVariables(); for (var i = 0; i < variables.length; i++) { - defvars[i] = Blockly.Python.variableDB_.getName(variables[i].name, + defvars[i] = Blockly.Python.variableDB_.getName(variables[i].getId(), Blockly.Variables.NAME_TYPE) + ' = None'; }