diff --git a/pxtblocks/fields/field_scopedvalueselector.ts b/pxtblocks/fields/field_scopedvalueselector.ts index 0dd1b518cfdf..de1aab0d4811 100644 --- a/pxtblocks/fields/field_scopedvalueselector.ts +++ b/pxtblocks/fields/field_scopedvalueselector.ts @@ -2,6 +2,7 @@ import * as Blockly from "blockly"; import { FieldCustom, FieldCustomOptions, setBlockDataForField, getBlockDataForField } from "./field_utils"; +import { FieldVariable } from "../plugins/newVariableField/fieldVariable"; interface FieldScopedValueSelectorOptions extends FieldCustomOptions { defl?: string; @@ -107,7 +108,7 @@ export class FieldScopedValueSelector extends Blockly.FieldLabel implements Fiel if (!input) continue; const fieldRow = input.fieldRow; if (!fieldRow) continue; - const field = fieldRow.find(f => f.name === "VAR") as Blockly.FieldVariable; + const field = fieldRow.find(f => f.name === "VAR") as FieldVariable; if (!field) continue; const variable = field.getVariable(); if (!variable) continue; diff --git a/pxtblocks/legacyMutations.ts b/pxtblocks/legacyMutations.ts index d29cfd840b96..c76cdbebd0c7 100644 --- a/pxtblocks/legacyMutations.ts +++ b/pxtblocks/legacyMutations.ts @@ -5,6 +5,7 @@ import { Environment } from "./compiler/environment"; import { escapeVarName } from "./compiler/util"; import { compileExpression } from "./compiler/compiler"; import { setVarFieldValue } from "./loader"; +import { FieldVariable } from "./plugins/newVariableField/fieldVariable"; /** * This interface defines the optionally defined functions for mutations that Blockly @@ -462,7 +463,7 @@ class DestructuringMutator extends MutatorHelper { this.parameters.forEach(param => { if (this.currentlyVisible.indexOf(param) === -1) { const fieldValue = this.parameterRenames[param] || param; - dummyInput.appendField(new Blockly.FieldVariable(fieldValue), param); + dummyInput.appendField(new FieldVariable(fieldValue), param); } }); diff --git a/pxtblocks/loader.ts b/pxtblocks/loader.ts index 8b565d33e6b9..8009b9e58bab 100644 --- a/pxtblocks/loader.ts +++ b/pxtblocks/loader.ts @@ -25,6 +25,7 @@ import { renderCodeCard } from "./codecardRenderer"; import { FieldDropdown } from "./fields/field_dropdown"; import { setDraggableShadowBlocks, setDuplicateOnDrag, setDuplicateOnDragStrategy } from "./plugins/duplicateOnDrag"; import { initCopyPaste } from "./copyPaste"; +import { FieldVariable } from "./plugins/newVariableField/fieldVariable"; export const DRAGGABLE_PARAM_INPUT_PREFIX = "HANDLER_DRAG_PARAM_"; @@ -272,7 +273,7 @@ function initBlock(block: Blockly.Block, info: pxtc.BlocksInfo, fn: pxtc.SymbolI else { let i = block.appendDummyInput(); comp.handlerArgs.filter(a => !a.inBlockDef).forEach(arg => { - i.appendField(new Blockly.FieldVariable(arg.name), "HANDLER_" + arg.name); + i.appendField(new FieldVariable(arg.name), "HANDLER_" + arg.name); }); } } @@ -826,7 +827,7 @@ function removeOuterSpace(str: string) { * variable ID or set the value of the model and not the field */ export function setVarFieldValue(block: Blockly.Block, fieldName: string, newName: string) { - const varField = block.getField(fieldName) as Blockly.FieldVariable; + const varField = block.getField(fieldName) as FieldVariable; // Check for an existing model with this name; otherwise we'll create // a second variable with the same name and it will show up twice in the UI diff --git a/pxtblocks/plugins/flyout/blockInflater.ts b/pxtblocks/plugins/flyout/blockInflater.ts index 695ba3b6a74a..2656ff06dbf3 100644 --- a/pxtblocks/plugins/flyout/blockInflater.ts +++ b/pxtblocks/plugins/flyout/blockInflater.ts @@ -5,7 +5,6 @@ export const HIDDEN_CLASS_NAME = "pxtFlyoutHidden"; export class MultiFlyoutRecyclableBlockInflater extends Blockly.BlockFlyoutInflater { protected keyToBlock: Map = new Map(); protected blockToKey: Map = new Map(); - protected varRemapping: Map = new Map(); static register() { Blockly.registry.register( @@ -17,6 +16,13 @@ export class MultiFlyoutRecyclableBlockInflater extends Blockly.BlockFlyoutInfla } protected isBlockRecycleable(block: Blockly.BlockSvg): boolean { + switch (block.type) { + case "variables_get": + case "variables_set": + case "variables_change": + return false; + } + return true; } @@ -68,46 +74,6 @@ export class MultiFlyoutRecyclableBlockInflater extends Blockly.BlockFlyoutInfla block.moveBy(-xy.x, -xy.y); block.addClass(HIDDEN_CLASS_NAME); block.setDisabledReason(true, HIDDEN_CLASS_NAME); - - // blockly deletes all of the potential variables in the flyout after recycling blocks. - // this can end up deleting our cached blocks if one of the referenced variables was - // promoted from a potential variable to a real one, since it will detect all of the - // cached blocks as referencing that variable. we can remap any variable that exists - // as both a potential and non-potential variable to a new one with the same name to - // prevent its usages from being deleted - const potentialVariableMap = block.workspace.getPotentialVariableMap(); - - if (potentialVariableMap) { - const allBlocks = [block].concat(block.getChildren(false)); - - // loop over all child variable fields - for (const block of allBlocks) { - for (const input of block.inputList) { - for (const field of input.fieldRow) { - if (field instanceof Blockly.FieldVariable) { - const variable = field.getVariable(); - const id = variable.getId(); - - // check if this variable was promoted to a real variable - if (!block.workspace.getVariableMap().getVariableById(id)) { - continue; - } - - // if it was, replace with a new potential variable - if (!this.varRemapping.has(id)) { - // rename the old potential variable to avoid collisions - const name = variable.getName(); - variable.setName(Blockly.utils.idGenerator.getNextUniqueId()); - this.varRemapping.set(id, potentialVariableMap.createVariable(name).getId()); - } - - field.setValue(this.varRemapping.get(id)); - } - } - } - } - } - const key = this.blockToKey.get(block); this.keyToBlock.set(key, block); this.removeListeners(block.id); diff --git a/pxtblocks/plugins/functions/extensions.ts b/pxtblocks/plugins/functions/extensions.ts index 9f429038a9ff..6bd0460266a4 100644 --- a/pxtblocks/plugins/functions/extensions.ts +++ b/pxtblocks/plugins/functions/extensions.ts @@ -2,6 +2,7 @@ import * as Blockly from "blockly"; import { getDefinition } from "./utils"; import { MsgKey } from "./msg"; import { ADD_IMAGE_DATAURI, REMOVE_IMAGE_DATAURI } from "./svgs"; +import { FieldVariable } from "../newVariableField/fieldVariable"; export interface InlineSvgsExtensionBlock extends Blockly.Block { ADD_IMAGE_DATAURI: string; @@ -81,7 +82,7 @@ const variableReporterMixin = { enabled: !this.workspace.options.readOnly, callback: () => { const workspace = this.workspace; - const variable = (this.getField('VAR') as Blockly.FieldVariable).getVariable(); + const variable = (this.getField('VAR') as FieldVariable).getVariable(); Blockly.Variables.renameVariable(workspace, variable); } }; diff --git a/pxtblocks/plugins/newVariableField/fieldVariable.ts b/pxtblocks/plugins/newVariableField/fieldVariable.ts index 7b747097b99c..bdbae9f58c63 100644 --- a/pxtblocks/plugins/newVariableField/fieldVariable.ts +++ b/pxtblocks/plugins/newVariableField/fieldVariable.ts @@ -172,6 +172,24 @@ export class FieldVariable extends Blockly.FieldVariable { protected showEditor_(e?: MouseEvent): void { showEditorMixin.call(this, e); } + + getValue(): string | null { + const id = super.getValue(); + + // this is a workaround for to prevent blockly's flyout clearing behavior from + // deleting recycled blocks in the flyout. by returning a fake variable name, + // we get blockly to skip over this field's source block when it tries to delete + // all usages of the variable + if (this.sourceBlock_?.isInFlyout) { + const potentialMap = this.sourceBlock_.workspace?.getPotentialVariableMap(); + + if (potentialMap.getVariableById(id)) { + return "potential_" + id; + } + } + + return id; + } } // Override the default variable field