Fix issue where migrating old projects could cause gbvm symbols to be…

…come empty, preventing build from completing

CHANGELOG.md

@@ -58,6 +58,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Fix line numbers in code editor for GBVM scripts with more than 999 lines
 - Fix issue preventing projects being created with the name "project"
 - Fix UI palette text control code. Palette indices now go from 1 to 8, because zero byte is a string terminator [@untoxa](https://github.com/untoxa)
+- Fix issue where migrating old projects could cause gbvm symbols to become empty, preventing build from completing (opening a broken project will now automatically fix this issue)
 
 ## [4.1.3] - 2024-09-16
 

src/shared/lib/entities/entitiesHelpers.ts

@@ -714,52 +714,58 @@ export const defaultLocalisedPaletteName = (paletteIndex: number) =>
 
 const extractEntitySymbols = (
   entities: EntityState<{ symbol?: string }, string>
-) => {
-  return Object.values(entities.entities).map(
-    (entity) => entity?.symbol
-  ) as string[];
+): Set<string> => {
+  return new Set(
+    Object.values(entities.entities).map((entity) => entity?.symbol ?? "")
+  );
 };
 
-const extractEntityStateSymbols = (state: EntitiesState) => {
-  return [
-    ...extractEntitySymbols(state.scenes),
-    ...extractEntitySymbols(state.actors),
-    ...extractEntitySymbols(state.triggers),
-    ...extractEntitySymbols(state.backgrounds),
-    ...extractEntitySymbols(state.spriteSheets),
-    ...extractEntitySymbols(state.emotes),
-    ...extractEntitySymbols(state.tilesets),
-    ...extractEntitySymbols(state.fonts),
-    ...extractEntitySymbols(state.variables),
-    ...extractEntitySymbols(state.constants),
-    ...extractEntitySymbols(state.customEvents),
-    ...extractEntitySymbols(state.music),
-    ...extractEntitySymbols(state.sounds),
-  ];
+const extractEntityStateSymbols = (state: EntitiesState): Set<string> => {
+  const allSymbols = new Set<string>();
+
+  const addSymbols = (symbols: Set<string>) => {
+    symbols.forEach((symbol) => allSymbols.add(symbol));
+  };
+
+  addSymbols(extractEntitySymbols(state.scenes));
+  addSymbols(extractEntitySymbols(state.actors));
+  addSymbols(extractEntitySymbols(state.triggers));
+  addSymbols(extractEntitySymbols(state.backgrounds));
+  addSymbols(extractEntitySymbols(state.spriteSheets));
+  addSymbols(extractEntitySymbols(state.emotes));
+  addSymbols(extractEntitySymbols(state.tilesets));
+  addSymbols(extractEntitySymbols(state.fonts));
+  addSymbols(extractEntitySymbols(state.variables));
+  addSymbols(extractEntitySymbols(state.constants));
+  addSymbols(extractEntitySymbols(state.customEvents));
+  addSymbols(extractEntitySymbols(state.music));
+  addSymbols(extractEntitySymbols(state.sounds));
+
+  return allSymbols;
 };
 
 export const genEntitySymbol = (state: EntitiesState, name: string) => {
   return genSymbol(name, extractEntityStateSymbols(state));
 };
 
-const ensureEntitySymbolsUnique = (
+export const ensureEntitySymbolsUnique = (
   entities: EntityState<{ symbol?: string }, string>,
-  seenSymbols: string[]
+  seenSymbols: Set<string>
 ) => {
   for (const entity of Object.values(entities.entities)) {
-    if (entity && entity.symbol) {
-      entity.symbol = toValidSymbol(entity.symbol);
-      if (seenSymbols.includes(entity.symbol)) {
+    if (entity) {
+      entity.symbol = toValidSymbol(entity.symbol ?? "");
+      if (seenSymbols.has(entity.symbol)) {
         const newSymbol = genSymbol(entity.symbol, seenSymbols);
         entity.symbol = newSymbol;
       }
-      seenSymbols.push(entity.symbol);
+      seenSymbols.add(entity.symbol);
     }
   }
 };
 
 export const ensureSymbolsUnique = (state: EntitiesState) => {
-  const symbols: string[] = [];
+  const symbols: Set<string> = new Set();
   ensureEntitySymbolsUnique(state.scenes, symbols);
   ensureEntitySymbolsUnique(state.actors, symbols);
   ensureEntitySymbolsUnique(state.triggers, symbols);

src/shared/lib/helpers/symbols.ts

@@ -26,11 +26,14 @@ export const toValidSymbol = (inputSymbol: string) => {
  * @param existingSymbols Array of existing symbols
  * @returns unique C symbol
  */
-export const genSymbol = (inputSymbol: string, existingSymbols: string[]) => {
+export const genSymbol = (
+  inputSymbol: string,
+  existingSymbols: Set<string>
+) => {
   const initialSymbol = toValidSymbol(inputSymbol);
   let symbol = initialSymbol;
   let count = 0;
-  while (existingSymbols.includes(symbol)) {
+  while (existingSymbols.has(symbol)) {
     symbol = `${initialSymbol.replace(/_[0-9]+/, "")}_${count++}`;
   }
   return symbol;

test/helpers/entitiesHelpers.test.ts

@@ -1,6 +1,8 @@
+import { EntityState } from "@reduxjs/toolkit";
 import {
   isActorPrefabEqual,
   isTriggerPrefabEqual,
+  ensureEntitySymbolsUnique,
 } from "shared/lib/entities/entitiesHelpers";
 import {
   ActorPrefabNormalized,
@@ -199,3 +201,83 @@ describe("isTriggerPrefabEqual", () => {
     expect(result).toBe(false);
   });
 });
+
+describe("ensureEntitySymbolsUnique", () => {
+  test("Should ensure unique symbols for entities", () => {
+    const state: EntityState<{ id: string; symbol?: string }, string> = {
+      ids: ["e1", "e2"],
+      entities: {
+        e1: {
+          id: "e1",
+          symbol: "entity",
+        },
+        e2: {
+          id: "e1",
+          symbol: "entity",
+        },
+      },
+    };
+    const seenSymbols = new Set<string>();
+    ensureEntitySymbolsUnique(state, seenSymbols);
+    expect(state.entities.e1.symbol).toBe("entity");
+    expect(state.entities.e2.symbol).toBe("entity_0");
+  });
+
+  test("Should not modify symbols that are already unique", () => {
+    const state: EntityState<{ id: string; symbol?: string }, string> = {
+      ids: ["e1", "e2"],
+      entities: {
+        e1: {
+          id: "e1",
+          symbol: "entity1",
+        },
+        e2: {
+          id: "e1",
+          symbol: "entity2",
+        },
+      },
+    };
+    const seenSymbols = new Set<string>();
+    ensureEntitySymbolsUnique(state, seenSymbols);
+    expect(state.entities.e1.symbol).toBe("entity1");
+    expect(state.entities.e2.symbol).toBe("entity2");
+  });
+
+  test("Should ensure unique symbols for entities when current symbol isn't defined", () => {
+    const state: EntityState<{ id: string; symbol?: string }, string> = {
+      ids: ["e1", "e2"],
+      entities: {
+        e1: {
+          id: "e1",
+        },
+        e2: {
+          id: "e1",
+        },
+      },
+    };
+    const seenSymbols = new Set<string>();
+    ensureEntitySymbolsUnique(state, seenSymbols);
+    expect(state.entities.e1.symbol).toBe("symbol");
+    expect(state.entities.e2.symbol).toBe("symbol_0");
+  });
+
+  test("Should ensure unique symbols for entities when current symbol is an empty string", () => {
+    const state: EntityState<{ id: string; symbol?: string }, string> = {
+      ids: ["e1", "e2"],
+      entities: {
+        e1: {
+          id: "e1",
+          symbol: "",
+        },
+        e2: {
+          id: "e1",
+          symbol: "",
+        },
+      },
+    };
+    const seenSymbols = new Set<string>();
+    ensureEntitySymbolsUnique(state, seenSymbols);
+    expect(state.entities.e1.symbol).toBe("symbol");
+    expect(state.entities.e2.symbol).toBe("symbol_0");
+  });
+});

test/helpers/symbols.test.ts

@@ -36,18 +36,18 @@ test("Should crop to 27 characters total", () => {
 
 test("Should return input name if valid and not already in use", () => {
   const input = "actor_0";
-  const existing: string[] = [];
+  const existing: Set<string> = new Set();
   expect(genSymbol(input, existing)).toBe(input);
 });
 
 test("Should return input incremented if already exists", () => {
   const input = "actor_0";
-  const existing: string[] = ["actor_0"];
+  const existing: Set<string> = new Set(["actor_0"]);
   expect(genSymbol(input, existing)).toBe("actor_1");
 });
 
 test("Should convert input to valid name before checking for existance", () => {
   const input = "Actor|0";
-  const existing: string[] = ["actor_0"];
+  const existing: Set<string> = new Set(["actor_0"]);
   expect(genSymbol(input, existing)).toBe("actor_1");
 });