diff --git a/src/engine/target.js b/src/engine/target.js index 7d0b40d8..9388937d 100644 --- a/src/engine/target.js +++ b/src/engine/target.js @@ -7,6 +7,7 @@ const uid = require('../util/uid'); const {Map} = require('immutable'); const log = require('../util/log'); const StringUtil = require('../util/string-util'); +const VariableUtil = require('../util/variable-util'); /** * @fileoverview @@ -514,13 +515,7 @@ class Target extends EventEmitter { // for all references for a given ID instead of doing the below..? this.blocks.getAllVariableAndListReferences()[idToBeMerged]; - referencesToChange.map(ref => { - ref.referencingField.id = idToMergeWith; - if (optNewName) { - ref.referencingField.value = optNewName; - } - return ref; - }); + VariableUtil.mergeVariables(idToBeMerged, idToMergeWith, referencesToChange, optNewName); } /** diff --git a/src/serialization/sb3.js b/src/serialization/sb3.js index 6c4a9ba8..ed2b5997 100644 --- a/src/serialization/sb3.js +++ b/src/serialization/sb3.js @@ -14,6 +14,8 @@ const StageLayering = require('../engine/stage-layering'); const log = require('../util/log'); const uid = require('../util/uid'); const MathUtil = require('../util/math-util'); +const StringUtil = require('../util/string-util'); +const VariableUtil = require('../util/variable-util'); const {loadCostume} = require('../import/load-costume.js'); const {loadSound} = require('../import/load-sound.js'); @@ -1075,6 +1077,12 @@ const deserializeMonitor = function (monitorData, runtime, targets, extensions) monitorBlockInfo && monitorBlockInfo.isSpriteSpecific) { monitorData.id = monitorBlockInfo.getId( monitorData.targetId, fields); + } else { + // Replace unsafe characters in monitor ID, if there are any. + // These would have come from projects that were originally 2.0 projects + // that had unsafe characters in the variable name (and then the name was + // used as part of the variable ID when importing the project). + monitorData.id = StringUtil.replaceUnsafeChars(monitorData.id); } // If the runtime already has a monitor block for this monitor's id, @@ -1128,6 +1136,35 @@ const deserializeMonitor = function (monitorData, runtime, targets, extensions) runtime.requestAddMonitor(MonitorRecord(monitorData)); }; +// Replace variable IDs throughout the project with +// xml-safe versions. +// This is to fix up projects imported from 2.0 where xml-unsafe names +// were getting added to the variable ids. +const replaceUnsafeCharsInVariableIds = function (targets) { + const allVarRefs = VariableUtil.getAllVarRefsForTargets(targets); + // Re-id the variables in the actual targets + targets.forEach(t => { + Object.keys(t.variables).forEach(id => { + const newId = StringUtil.replaceUnsafeChars(id); + if (newId === id) return; + t.variables[id].id = newId; + t.variables[newId] = t.variables[id]; + delete t.variables[id]; + }); + }); + + // Replace the IDs in the blocks refrencing variables or lists + for (const id in allVarRefs) { + const newId = StringUtil.replaceUnsafeChars(id); + if (id === newId) continue; // ID was already safe, skip + // We're calling this on the stage target because we need a + // target to call on but this shouldn't matter because we're passing + // in all the varRefs we want to operate on + VariableUtil.mergeVariables(id, newId, allVarRefs); + } + return targets; +}; + /** * Deserialize the specified representation of a VM runtime and loads it into the provided runtime instance. * @param {object} json - JSON representation of a VM runtime. @@ -1171,6 +1208,7 @@ const deserialize = function (json, runtime, zip, isSingleSprite) { delete t.targetPaneOrder; return t; })) + .then(targets => replaceUnsafeCharsInVariableIds(targets)) .then(targets => { monitorObjects.map(monitorDesc => deserializeMonitor(monitorDesc, runtime, targets, extensions)); return targets; diff --git a/src/util/variable-util.js b/src/util/variable-util.js new file mode 100644 index 00000000..80b59a4a --- /dev/null +++ b/src/util/variable-util.js @@ -0,0 +1,48 @@ +class VariableUtil { + static _mergeVarRefObjects (accum, obj2) { + for (const id in obj2) { + if (accum[id]) { + accum[id] = accum[id].concat(obj2[id]); + } else { + accum[id] = obj2[id]; + } + } + return accum; + } + + /** + * Get all variable/list references in the given list of targets + * in the project. + * @param {Array.} targets The list of targets to get the variable + * and list references from. + * @return {object} An object with variable ids as the keys and a list of block fields referencing + * the variable. + */ + static getAllVarRefsForTargets (targets) { + return targets + .map(t => t.blocks.getAllVariableAndListReferences()) + .reduce(VariableUtil._mergeVarRefObjects, {}); + } + + /** + * Merge variable references with another variable. + * @param {string} idToBeMerged ID of the variable whose references need to be updated + * @param {string} idToMergeWith ID of the variable that the old references should be replaced with + * @param {Array} referencesToUpdate Context of the change, the object containing variable + * references to update. + * @param {?string} optNewName New variable name to merge with. The old + * variable name in the references being updated should be replaced with this new name. + * If this parameter is not provided or is '', no name change occurs. + */ + static mergeVariables (idToBeMerged, idToMergeWith, referencesToUpdate, optNewName) { + referencesToUpdate.map(ref => { + ref.referencingField.id = idToMergeWith; + if (optNewName) { + ref.referencingField.value = optNewName; + } + return ref; + }); + } +} + +module.exports = VariableUtil;