Reference/resolve variables by ID instead of name

src/BlockInput.ts

@@ -41,12 +41,18 @@ export interface Broadcast extends Base {
 
 export interface Variable extends Base {
   type: "variable";
-  value: string;
+  value: {
+    id: string;
+    name: string;
+  };
 }
 
 export interface List extends Base {
   type: "list";
-  value: string;
+  value: {
+    id: string;
+    name: string;
+  };
 }
 
 export interface Block extends Base {

src/__tests__/__snapshots__/compilesb3.test.ts.snap

@@ -168,7 +168,7 @@ export default class Stage extends StageBase {
     yield* this.askAndWait(this.loudness);
     yield* this.askAndWait(this.timer);
     yield* this.askAndWait(this.stage.costumeNumber);
-    yield* this.askAndWait(this.stage.vars[\\"CloudVar\\"]);
+    yield* this.askAndWait(this.stage.vars.CloudVar);
     yield* this.askAndWait(new Date().getDay() + 1);
     yield* this.askAndWait(
       ((new Date().getTime() - new Date(2000, 0, 1)) / 1000 / 60 +

src/io/leopard/toLeopard.ts

@@ -1,6 +1,6 @@
 import Project from "../../Project";
 import Script from "../../Script";
-import Block from "../../Block";
+import Block, { BlockBase } from "../../Block";
 import * as BlockInput from "../../BlockInput";
 import { OpCode } from "../../OpCode";
 
@@ -99,7 +99,7 @@ export default function toLeopard(
 
   let targetNameMap = {};
   let customBlockArgNameMap: Map<Script, { [key: string]: string }> = new Map();
-  let variableNameMap: Map<Target, { [key: string]: string }> = new Map();
+  let variableNameMap: { [id: string]: string } = {}; // ID to unique (Leopard) name
 
   for (const target of [project.stage, ...project.sprites]) {
     const newTargetName = uniqueName(camelCase(target.name, true));
@@ -108,19 +108,9 @@ export default function toLeopard(
 
     let uniqueVariableName = uniqueName.branch();
 
-    const varNameMap = {};
-    variableNameMap.set(target, varNameMap);
-
-    for (const list of target.lists) {
-      const newName = uniqueVariableName(camelCase(list.name));
-      varNameMap[list.name] = newName;
-      list.setName(newName);
-    }
-
-    for (const variable of target.variables) {
-      const newName = uniqueVariableName(camelCase(variable.name));
-      varNameMap[variable.name] = newName;
-      variable.setName(newName);
+    for (const { id, name } of [...target.lists, ...target.variables]) {
+      const newName = uniqueVariableName(camelCase(name));
+      variableNameMap[id] = newName;
     }
 
     const uniqueScriptName = uniqueNameGenerator([
@@ -276,24 +266,25 @@ export default function toLeopard(
 
       // If the block contains a variable or list dropdown,
       // get the code to grab that variable now for convenience
-      let varName: string = null;
       let selectedVarSource: string = null;
       let selectedWatcherSource: string = null;
+      let varInputId: string = null;
+      let isSpriteVar: boolean = null;
       if ("VARIABLE" in block.inputs) {
-        varName = block.inputs.VARIABLE.value.toString();
+        varInputId = (block.inputs.VARIABLE.value as { id: string }).id;
+        isSpriteVar = target.variables.some(({ id }) => id === varInputId);
+      } else if ("LIST" in block.inputs) {
+        varInputId = (block.inputs.LIST.value as { id: string }).id;
+        isSpriteVar = target.lists.some(({ id }) => id === varInputId);
       }
-      if ("LIST" in block.inputs) {
-        varName = block.inputs.LIST.value.toString();
-      }
-      if (varName !== null) {
-        const spriteVars = variableNameMap.get(target);
-        if (varName in spriteVars) {
-          selectedVarSource = `this.vars.${spriteVars[varName]}`;
-          selectedWatcherSource = `this.watchers.${spriteVars[varName]}`;
+      if (varInputId) {
+        const newName = variableNameMap[varInputId];
+        if (isSpriteVar) {
+          selectedVarSource = `this.vars.${newName}`;
+          selectedWatcherSource = `this.watchers.${newName}`;
         } else {
-          const stageVars = variableNameMap.get(project.stage);
-          selectedVarSource = `this.stage.vars.${stageVars[varName]}`;
-          selectedWatcherSource = `this.stage.watchers.${stageVars[varName]}`;
+          selectedVarSource = `this.stage.vars.${newName}`;
+          selectedWatcherSource = `this.stage.watchers.${newName}`;
         }
       }
 
@@ -643,7 +634,10 @@ export default function toLeopard(
               if (block.inputs.OBJECT.value !== "_stage_") {
                 varOwner = project.sprites.find(sprite => sprite.name === targetNameMap[block.inputs.OBJECT.value]);
               }
-              propName = `vars[${JSON.stringify(variableNameMap.get(varOwner)[block.inputs.PROPERTY.value])}]`;
+              // "of" block gets variables by name, not ID, using lookupVariableByNameAndType in scratch-vm.
+              const variable = varOwner.variables.find(variable => variable.name === block.inputs.PROPERTY.value);
+              const newName = variableNameMap[variable.id];
+              propName = `vars.${newName}`;
               break;
             }
           }
@@ -976,7 +970,7 @@ export default function toLeopard(
     // Some watchers start invisible but appear later, so this code builds a list of
     // watchers that appear in "show variable" and "show list" blocks. The list is
     // actually *used* later, by some other code.
-    let shownWatchers = new Set();
+    let shownWatchers: Set<string> = new Set();
     let targetsToCheckForShowBlocks: Target[];
     if (target.isStage) {
       targetsToCheckForShowBlocks = [project.stage, ...project.sprites];
@@ -986,11 +980,15 @@ export default function toLeopard(
     for (const checkTarget of targetsToCheckForShowBlocks) {
       for (const script of checkTarget.scripts) {
         for (const block of script.blocks) {
-          if (block.opcode === OpCode.data_showvariable) {
-            shownWatchers.add(block.inputs.VARIABLE.value);
+          if (block.opcode === OpCode.data_showvariable || block.opcode === OpCode.data_hidevariable) {
+            shownWatchers.add(
+              (block as BlockBase<OpCode.data_showvariable, { VARIABLE: BlockInput.Variable }>).inputs.VARIABLE.value.id
+            );
           }
-          if (block.opcode === OpCode.data_showlist) {
-            shownWatchers.add(block.inputs.LIST.value);
+          if (block.opcode === OpCode.data_showlist || block.opcode === OpCode.data_hidelist) {
+            shownWatchers.add(
+              (block as BlockBase<OpCode.data_showlist, { LIST: BlockInput.List }>).inputs.LIST.value.id
+            );
           }
         }
       }
@@ -1051,32 +1049,29 @@ export default function toLeopard(
 
           ${target.volume !== 100 ? `this.audioEffects.volume = ${target.volume};` : ""}
 
-          ${[...target.variables, ...target.lists]
-            .map(variable => `this.vars.${variable.name} = ${toOptimalJavascriptRepresentation(variable.value)};`)
-            .join("\n")}
-
           ${[...target.variables, ...target.lists]
             .map(
               variable =>
-                [
-                  variable,
-                  Object.entries(variableNameMap.get(target)).find(([, newName]) => newName === variable.name)[0]
-                ] as [Variable | List, string]
+                `this.vars.${variableNameMap[variable.id]} = ${toOptimalJavascriptRepresentation(variable.value)};`
             )
-            .filter(([variable, oldName]) => variable.visible || shownWatchers.has(oldName))
-            .map(([variable, oldName]) => {
-              return `this.watchers.${variable.name} = new Watcher({
-              label: ${JSON.stringify((target.isStage ? "" : `${target.name}: `) + oldName)},
+            .join("\n")}
+
+          ${[...target.variables, ...target.lists]
+            .map(variable => [variable, variableNameMap[variable.id]] as [Variable | List, string])
+            .filter(([variable]) => variable.visible || shownWatchers.has(variable.id))
+            .map(([variable, newName]) => {
+              return `this.watchers.${newName} = new Watcher({
+              label: ${JSON.stringify((target.isStage ? "" : `${target.name}: `) + variable.name)},
               style: ${JSON.stringify(
                 variable instanceof List
                   ? "normal"
                   : { default: "normal", large: "large", slider: "slider" }[variable.mode]
               )},
               visible: ${JSON.stringify(variable.visible)},
-              value: () => this.vars.${variable.name},
+              value: () => this.vars.${newName},
               ${
                 variable instanceof Variable && variable.mode === "slider"
-                  ? `setValue: (value) => { this.vars.${variable.name} = value; },\n`
+                  ? `setValue: (value) => { this.vars.${newName} = value; },\n`
                   : ""
               }x: ${JSON.stringify(variable.x + 240)},
               y: ${JSON.stringify(180 - variable.y)},

src/io/sb3/fromSb3.ts

@@ -8,7 +8,7 @@ import * as BlockInput from "../../BlockInput";
 import Costume from "../../Costume";
 import Project from "../../Project";
 import Sound from "../../Sound";
-import { Sprite, Stage, TargetOptions } from "../../Target";
+import Target, { Sprite, Stage, TargetOptions } from "../../Target";
 import { List, Variable } from "../../Data";
 import Script from "../../Script";
 
@@ -216,7 +216,7 @@ function getBlockScript(blocks: { [key: string]: sb3.Block }) {
                 type: "block",
                 value: new BlockBase({
                   opcode: OpCode.data_variable,
-                  inputs: { VARIABLE: { type: "variable", value: value[1] } },
+                  inputs: { VARIABLE: { type: "variable", value: { id: value[2], name: value[1] } } },
                   parent: blockId
                 }) as Block
               });
@@ -227,7 +227,7 @@ function getBlockScript(blocks: { [key: string]: sb3.Block }) {
                 type: "block",
                 value: new BlockBase({
                   opcode: OpCode.data_listcontents,
-                  inputs: { LIST: { type: "list", value: value[1] } },
+                  inputs: { LIST: { type: "list", value: { id: value[2], name: value[1] } } },
                   parent: blockId
                 }) as Block
               });
@@ -268,7 +268,11 @@ function getBlockScript(blocks: { [key: string]: sb3.Block }) {
       let result = {};
       for (const [fieldName, values] of Object.entries(fields)) {
         const type = sb3.fieldTypeMap[opcode][fieldName];
-        result[fieldName] = { type, value: values[0] };
+        if (fieldName === "VARIABLE" || fieldName === "LIST") {
+          result[fieldName] = { type, value: { id: values[1], name: values[0] } };
+        } else {
+          result[fieldName] = { type, value: values[0] };
+        }
       }
 
       return result;
@@ -396,7 +400,7 @@ export async function fromSb3JSON(json: sb3.ProjectJSON, options: { getAsset: Ge
     };
   }
 
-  return new Project({
+  const project = new Project({
     stage: new Stage(await getTargetOptions(stage)),
     sprites: await Promise.all(
       json.targets
@@ -423,6 +427,70 @@ export async function fromSb3JSON(json: sb3.ProjectJSON, options: { getAsset: Ge
     videoOn: stage.videoState === "on",
     videoAlpha: stage.videoTransparency
   });
+
+  const targetToBlocks: Map<Target, Array<Block>> = new Map();
+  for (const target of [project.stage, ...project.sprites]) {
+    targetToBlocks.set(target, flattenBlocks(target.scripts.flatMap(script => script.blocks)));
+  }
+  const allBlocks = Array.from(targetToBlocks.values()).flat();
+
+  // Run an extra pass on variables (and lists). Only those which are actually
+  // referenced in blocks or monitors should be kept.
+  for (const target of [project.stage, ...project.sprites]) {
+    let relevantBlocks: Array<Block> = null;
+    if (target === project.stage) {
+      relevantBlocks = allBlocks;
+    } else {
+      relevantBlocks = targetToBlocks.get(target);
+    }
+
+    const usedVariableIds: Set<string> = new Set();
+    for (const block of relevantBlocks) {
+      let id: string = null;
+      if ((block.inputs as { VARIABLE: BlockInput.Variable }).VARIABLE) {
+        id = (block.inputs as { VARIABLE: BlockInput.Variable }).VARIABLE.value.id;
+      } else if ((block.inputs as { LIST: BlockInput.List }).LIST) {
+        id = (block.inputs as { LIST: BlockInput.List }).LIST.value.id;
+      } else {
+        continue;
+      }
+      usedVariableIds.add(id);
+    }
+
+    for (const varList of [target.variables, target.lists]) {
+      for (let i = 0, variable; (variable = varList[i]); i++) {
+        if (variable.visible) {
+          continue;
+        }
+        if (usedVariableIds.has(variable.id)) {
+          continue;
+        }
+
+        varList.splice(i, 1);
+        i--;
+        continue;
+      }
+    }
+  }
+
+  return project;
+}
+
+function flattenBlocks(blocks: Array<Block>) {
+  return blocks.flatMap(block => [
+    block,
+    ...flattenBlocks(
+      Object.values(block.inputs).flatMap(input => {
+        if (input.type === "block") {
+          return [input.value];
+        } else if (input.type === "blocks") {
+          return input.value;
+        } else {
+          return [];
+        }
+      })
+    )
+  ]);
 }
 
 export default async function fromSb3(fileData: Parameters<typeof JSZip.loadAsync>[0]): Promise<Project> {

src/io/sb3/toSb3.ts

@@ -93,19 +93,20 @@ export default function toSb3(options: Partial<ToSb3Options> = {}): ToSb3Output
     for (const key of Object.keys(fieldEntries)) {
       const input = inputs[key];
       // Fields are stored as a plain [value, id?] pair.
+      let valueOrName;
       let id: string;
       switch (input.type) {
         case "variable":
-          id = getVariableId(input.value, target, stage);
-          break;
         case "list":
-          id = getListId(input.value, target, stage);
+          valueOrName = input.value.name;
+          id = input.value.id;
           break;
         default:
+          valueOrName = input.value;
           id = null;
           break;
       }
-      fields[key] = [input.value, id];
+      fields[key] = [valueOrName, id];
     }
 
     return fields;
@@ -397,14 +398,12 @@ export default function toSb3(options: Partial<ToSb3Options> = {}): ToSb3Output
 
           switch (input.value.opcode) {
             case OpCode.data_variable: {
-              const variableName = input.value.inputs.VARIABLE.value;
-              const variableId = getVariableId(variableName, target, stage);
+              const { id: variableId, name: variableName } = input.value.inputs.VARIABLE.value;
               obscuringBlockValue = [BIS.VAR_PRIMITIVE, variableName, variableId];
               break;
             }
             case OpCode.data_listcontents: {
-              const listName = input.value.inputs.LIST.value;
-              const listId = getListId(listName, target, stage);
+              const { id: listId, name: listName } = input.value.inputs.LIST.value;
               obscuringBlockValue = [BIS.LIST_PRIMITIVE, listName, listId];
               break;
             }

src/io/scratchblocks/toScratchblocks.ts

@@ -61,6 +61,8 @@ export default function toScratchblocks(
 
       case "variable":
       case "list":
+        return `[${escape(inp.value.name)} v]`;
+
       case "rotationStyle":
       case "scrollAlignment":
       case "stopMenu":
@@ -422,7 +424,7 @@ export default function toScratchblocks(
 
       // data -------------------------------------------------------- //
       case OpCode.data_variable:
-        return `(${block.inputs.VARIABLE.value} :: variables)`;
+        return `(${block.inputs.VARIABLE.value.name} :: variables)`;
       case OpCode.data_setvariableto:
         return `set ${i("VARIABLE")} to ${i("VALUE")}`;
       case OpCode.data_changevariableby:
@@ -432,7 +434,7 @@ export default function toScratchblocks(
       case OpCode.data_hidevariable:
         return `hide variable ${i("VARIABLE")}`;
       case OpCode.data_listcontents:
-        return `(${block.inputs.LIST.value} :: list)`;
+        return `(${block.inputs.LIST.value.name} :: list)`;
       case OpCode.data_addtolist:
         return `add ${i("ITEM")} to ${i("LIST")}`;
       case OpCode.data_deleteoflist: