From 9ccabbeb3e323ffaf692320cfc8c840de96e1397 Mon Sep 17 00:00:00 2001 From: Chris Maltby Date: Mon, 25 Nov 2024 14:04:05 +0000 Subject: [PATCH] Fix issue where migrating old projects could cause gbvm symbols to become empty, preventing build from completing --- CHANGELOG.md | 1 + src/shared/lib/entities/entitiesHelpers.ts | 60 +++++++++------- src/shared/lib/helpers/symbols.ts | 7 +- test/helpers/entitiesHelpers.test.ts | 82 ++++++++++++++++++++++ test/helpers/symbols.test.ts | 6 +- 5 files changed, 124 insertions(+), 32 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a5bf19ae6..a052e3647 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/src/shared/lib/entities/entitiesHelpers.ts b/src/shared/lib/entities/entitiesHelpers.ts index a918065ec..57d8afc52 100644 --- a/src/shared/lib/entities/entitiesHelpers.ts +++ b/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 => { + 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 => { + const allSymbols = new Set(); + + const addSymbols = (symbols: Set) => { + 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 ) => { 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 = new Set(); ensureEntitySymbolsUnique(state.scenes, symbols); ensureEntitySymbolsUnique(state.actors, symbols); ensureEntitySymbolsUnique(state.triggers, symbols); diff --git a/src/shared/lib/helpers/symbols.ts b/src/shared/lib/helpers/symbols.ts index a5cc2c284..938d34513 100644 --- a/src/shared/lib/helpers/symbols.ts +++ b/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 +) => { 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; diff --git a/test/helpers/entitiesHelpers.test.ts b/test/helpers/entitiesHelpers.test.ts index b18a77e9b..321d07b77 100644 --- a/test/helpers/entitiesHelpers.test.ts +++ b/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(); + 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(); + 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(); + 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(); + ensureEntitySymbolsUnique(state, seenSymbols); + expect(state.entities.e1.symbol).toBe("symbol"); + expect(state.entities.e2.symbol).toBe("symbol_0"); + }); +}); diff --git a/test/helpers/symbols.test.ts b/test/helpers/symbols.test.ts index c232729c1..b5ba92efe 100644 --- a/test/helpers/symbols.test.ts +++ b/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 = 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 = 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 = new Set(["actor_0"]); expect(genSymbol(input, existing)).toBe("actor_1"); });