Merge branch 'develop' into pr/1652

# Conflicts: # CHANGELOG.md
chrismaltby · Nov 27, 2024 · 6627218 · 6627218
2 parents d964a56 + de1625e
commit 6627218
Show file tree

Hide file tree

Showing 9 changed files with 141 additions and 52 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -58,6 +58,8 @@ 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 issue preventing checkbox type working in engine fields [@pau-tomas](https://github.com/pau-tomas)
+- 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/appData/src/gb/engine.json b/appData/src/gb/engine.json
@@ -1,5 +1,5 @@
 {
-  "version": "4.2.0-e4",
+  "version": "4.2.0-e5",
   "fields": [
     {
       "key": "INPUT_PLATFORM_JUMP",

diff --git a/appData/src/gb/src/core/ui.c b/appData/src/gb/src/core/ui.c
@@ -389,7 +389,7 @@ UBYTE ui_draw_text_buffer_char(void) BANKED {
                 if (vwf_current_offset) ui_print_reset();
                 break;
             case 0x0b:
-                text_palette = (*++ui_text_ptr & 0x07);
+                text_palette = (((*++ui_text_ptr) - 1u) & 0x07u);
                 break;
             case '\r':  // 0x0d
                 // line feed

diff --git a/src/lib/project/ejectEngineChangelog.ts b/src/lib/project/ejectEngineChangelog.ts
@@ -793,6 +793,13 @@ const changes: EngineChange[] = [
       "src/core/vm_music.c",
     ],
   },
+  {
+    version: "4.2.0-e5",
+    description: createDescription("Fixes", [
+      "Fix UI palette text control code. Palette indices now go from 1 to 8, because zero byte is a string terminator",
+    ]),
+    modifiedFiles: ["src/core/ui.c"],
+  },
 ];
 
 export const isKnownEngineVersion = (currentVersion: string): boolean => {

diff --git 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<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);

diff --git 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<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;

diff --git 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<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");
+  });
+});
diff --git 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<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");
 });
diff --git a/yarn.lock b/yarn.lock
@@ -9101,19 +9101,19 @@ __metadata:
   linkType: hard
 
 "cross-spawn@npm:^6.0.0, cross-spawn@npm:^6.0.5":
-  version: 6.0.5
-  resolution: "cross-spawn@npm:6.0.5"
+  version: 6.0.6
+  resolution: "cross-spawn@npm:6.0.6"
   dependencies:
     nice-try: "npm:^1.0.4"
     path-key: "npm:^2.0.1"
     semver: "npm:^5.5.0"
     shebang-command: "npm:^1.2.0"
     which: "npm:^1.2.9"
-  checksum: 10c0/e05544722e9d7189b4292c66e42b7abeb21db0d07c91b785f4ae5fefceb1f89e626da2703744657b287e86dcd4af57b54567cef75159957ff7a8a761d9055012
+  checksum: 10c0/bf61fb890e8635102ea9bce050515cf915ff6a50ccaa0b37a17dc82fded0fb3ed7af5478b9367b86baee19127ad86af4be51d209f64fd6638c0862dca185fe1d
   languageName: node
   linkType: hard
 
-"cross-spawn@npm:^7.0.0, cross-spawn@npm:^7.0.2, cross-spawn@npm:^7.0.3":
+"cross-spawn@npm:^7.0.0, cross-spawn@npm:^7.0.1, cross-spawn@npm:^7.0.2, cross-spawn@npm:^7.0.3":
   version: 7.0.3
   resolution: "cross-spawn@npm:7.0.3"
   dependencies:
@@ -9124,17 +9124,6 @@ __metadata:
   languageName: node
   linkType: hard
 
-"cross-spawn@npm:^7.0.1":
-  version: 7.0.2
-  resolution: "cross-spawn@npm:7.0.2"
-  dependencies:
-    path-key: "npm:^3.1.0"
-    shebang-command: "npm:^2.0.0"
-    which: "npm:^2.0.1"
-  checksum: 10c0/3d4cfc2020cd7db56c20fe65ba780fdf3385ebe72850b098f3a21eaa9cc0142e4c0e10b7a8198d190bcbfe20faf288ffb99a6dd2422dbfb9422abeee9f34dee6
-  languageName: node
-  linkType: hard
-
 "cross-zip@npm:^4.0.0":
   version: 4.0.0
   resolution: "cross-zip@npm:4.0.0"
@@ -18716,9 +18705,9 @@ __metadata:
   linkType: hard
 
 "smol-toml@npm:^1.3.0":
-  version: 1.3.0
-  resolution: "smol-toml@npm:1.3.0"
-  checksum: 10c0/442b4d033236ff6dd05bf91d57695fd9070a8221af080a5b2782cb2d9fad8bc31f698c61de5308a351907c1200202ba3ee51d52c5704f5349149e7c374f5fe90
+  version: 1.3.1
+  resolution: "smol-toml@npm:1.3.1"
+  checksum: 10c0/bac5bf4f2655fd561fe41f9426d70ab68b486631beff97a7f127f5d2f811b5e247d50a06583be03d35a625dcb05b7984b94a61a81c68ea2810ac7a9bf4edc64d
   languageName: node
   linkType: hard