Fix read/write asymmetries, script serialization and global state; revive the test suite

.github/workflows/build.yml

@@ -0,0 +1,65 @@
+name: Build
+
+on:
+  push:
+    branches: ['**']
+  pull_request:
+
+concurrency:
+  group: ${{ github.workflow }}-${{ github.ref }}
+  cancel-in-progress: true
+
+jobs:
+  test:
+    name: Test
+    runs-on: ubuntu-latest
+
+    steps:
+      - uses: actions/checkout@v4
+
+      - name: Set up JDK
+        uses: actions/setup-java@v4
+        with:
+          distribution: temurin
+          # This module compiles at source/target 20.
+          java-version: '21'
+          cache: maven
+
+      # Nds4j is not on Maven Central at the version this module depends on, so it has to be
+      # built from source. Prefer a branch of the same name when one exists: a change that
+      # spans both repositories is developed on matching branches, and testing this module
+      # against a stale Nds4j would report a failure that does not exist (or hide a real one).
+      - name: Resolve Nds4j branch
+        id: nds4j
+        run: |
+          ref="${{ github.head_ref || github.ref_name }}"
+          if git ls-remote --exit-code --heads https://github.com/turtleisaac/Nds4j.git "$ref" >/dev/null 2>&1; then
+            echo "Building against matching Nds4j branch: $ref"
+          else
+            echo "No Nds4j branch named '$ref'; falling back to main"
+            ref=main
+          fi
+          echo "ref=$ref" >> "$GITHUB_OUTPUT"
+
+      - name: Check out Nds4j
+        uses: actions/checkout@v4
+        with:
+          repository: turtleisaac/Nds4j
+          ref: ${{ steps.nds4j.outputs.ref }}
+          path: .upstream/Nds4j
+
+      - name: Install Nds4j
+        run: mvn -B -ntp -q install -DskipTests -f .upstream/Nds4j/pom.xml
+
+      - name: Build and test
+        # Round-trip tests against real game data need a retail ROM, which cannot be committed.
+        # They skip here and are run locally with -Drom.dir. Everything else runs.
+        run: mvn -B -ntp verify
+
+      - name: Upload test reports
+        if: failure()
+        uses: actions/upload-artifact@v4
+        with:
+          name: surefire-reports
+          path: '**/target/surefire-reports/**'
+          if-no-files-found: ignore

README.md

@@ -8,7 +8,7 @@ Feel free to use this library for your own projects relating to the Gen 4 Pokém
 ## Dependencies
 [Nds4j](https://github.com/turtleisaac/Nds4j), my library for interacting with, reading, editing, and writing the general proprietary Nintendo DS file formats, is required.
 
-If you go to build PokEditor-Core yourself, please place your own **legally obtained** Pokémon HeartGold ROM in the root directory of the repo and name it "HeartGold.nds", and a Platinum ROM named "Platinum.nds". These will be accessed as part of the unit tests.
+PokEditor-Core builds without a ROM. Some tests exercise round-tripping against real game data and are skipped when none is available; to run them, pass `-Drom.dir=<directory>` pointing at your own **legally obtained** HeartGold and Platinum ROMs.
 
 ## Usage
 As this is a library, there is nothing for you to run in terms of a JAR file or Main method. You need to write all the code to make use of my library yourself.
@@ -17,11 +17,16 @@ When you go to use PokEditor-Core, the first thing your program needs to do is d
 ```java
 // create a NintendoDsRom object using something like NintendoDsRom.fromFile(<path to rom>)
 NintendoDsRom rom = NintendoDsRom.fromFile("path to rom");
-Game game = Game.parseBaseRom(rom.getGameCode());
+
+// parseBaseRom returns the game together with its region, rather than storing the
+// region on the Game enum constant where every ROM in the process would share it
+Game.BaseRomInfo baseRomInfo = Game.parseBaseRom(rom.getGameCode());
+Game game = baseRomInfo.game();
+
 GameFiles.initialize(game);
 TextFiles.initialize(game);
 GameCodeBinaries.initialize(game);
-Tables.initialize(game);
+Tables.initialize(game, baseRomInfo.region());
 ```
 Once you have ran the four `initialize` methods, your code can make use of the library without much further pain. Here is an example which works with species personal data:
 
@@ -51,7 +56,7 @@ rom.saveToFile("output rom path", false);
 
 ## Troubleshooting
 
-If you are having trouble building PokEditor-Core, make sure you have a "HeartGold.nds" and "Platinum.nds" ROM in the root directory of your local repo, as these are needed for the unit tests.
+PokEditor-Core builds and its test suite passes without any ROM present — the tests that need one are skipped rather than failed. To run those too, point the build at a directory holding your own **legally obtained** ROMs with `-Drom.dir=<directory>`; the file names default to `HeartGold.nds` and `Platinum.nds` and can be overridden with `-Drom.heartgold` and `-Drom.platinum`.
 
 Additionally, there are some sources which need to be generated by maven/antlr4 which don't always get generated and compiled for some reason, so try running `mvn generate-sources` in the terminal/command-line from within the repo root to get those to appear.
 You may need to mark the directory `src/main/generated-sources` as a generated sources root afterwards.

pom.xml

@@ -6,7 +6,7 @@
 
     <groupId>io.github.turtleisaac</groupId>
     <artifactId>PokEditor-Core</artifactId>
-    <version>1.0-SNAPSHOT</version>
+    <version>1.0.0</version>
 
     <properties>
         <maven.compiler.source>20</maven.compiler.source>
@@ -16,6 +16,22 @@
     </properties>
 
     <dependencies>
+        <!-- Declared because this module imports it. It used to arrive through
+             jackson-dataformat-xml, which nothing here used - so removing that dead entry took
+             Jackson with it and the build stopped compiling. Declare what is used; do not declare
+             what is not. -->
+        <dependency>
+            <groupId>com.fasterxml.jackson.core</groupId>
+            <artifactId>jackson-databind</artifactId>
+            <version>2.15.2</version>
+        </dependency>
+
+        <dependency>
+            <groupId>com.fasterxml.jackson.core</groupId>
+            <artifactId>jackson-core</artifactId>
+            <version>2.15.2</version>
+        </dependency>
+
         <dependency>
             <groupId>org.junit.jupiter</groupId>
             <artifactId>junit-jupiter</artifactId>
@@ -32,18 +48,11 @@
             <groupId>com.google.inject</groupId>
             <artifactId>guice</artifactId>
             <version>7.0.0</version>
-        </dependency>
-        <dependency>
-            <groupId>junit</groupId>
-            <artifactId>junit</artifactId>
-            <version>4.13.2</version>
+            <!-- Test only: nothing in src/main imports Guice - the parsers are bound by the
+                 consumer's injector, and this module's own tests build one. At compile scope it
+                 landed on every downstream module's compile classpath for no reason. -->
             <scope>test</scope>
         </dependency>
-        <dependency>
-            <groupId>com.fasterxml.jackson.dataformat</groupId>
-            <artifactId>jackson-dataformat-xml</artifactId>
-            <version>2.15.2</version>
-        </dependency>
 
         <dependency>
             <groupId>org.antlr</groupId>
@@ -55,7 +64,7 @@
         <dependency>
             <groupId>io.github.turtleisaac</groupId>
             <artifactId>Nds4j</artifactId>
-            <version>0.1.0</version>
+            <version>1.0.0</version>
         </dependency>
     </dependencies>
 

src/main/antlr4/io/github/turtleisaac/pokeditor/formats/trainers/SmogonTeam.g4

@@ -8,7 +8,7 @@ team: (speciesEntry)+ EOF?;
 
 move: '-' WHITESPACE+ nameWithSpace WHITESPACE*? NEWLINE? ;
 
-speciesEntry : NEWLINE*? species ability? level? shiny? effortValues? nature? individualValues? move move? move? move? NEWLINE*? ;
+speciesEntry : NEWLINE*? species ability? level? shiny? effortValues? nature? individualValues? move? move? move? move? NEWLINE*? ;
 
 species: NEWLINE NAME WHITESPACE+? item? WHITESPACE*? NEWLINE ;
 item: '@' WHITESPACE+ nameWithSpace ;

src/main/java/io/github/turtleisaac/pokeditor/formats/BytesDataContainer.java

@@ -39,7 +39,8 @@ public boolean containsKey(GameFiles key)
 
     public boolean containsPatternKey(GameFiles file, PatternIndex key)
     {
-        return get(file).containsKey(key);
+        Map<PatternIndex, byte[]> m = super.get(file);
+        return m != null && m.containsKey(Objects.requireNonNullElse(key, Default.NO_GROUPING));
     }
 
     public interface PatternIndex {

src/main/java/io/github/turtleisaac/pokeditor/formats/FieldWidth.java

@@ -0,0 +1,74 @@
+package io.github.turtleisaac.pokeditor.formats;
+
+/**
+ * Checks that a value fits the field it is about to be written into.
+ * <p>
+ * Every format in this package writes its fields by narrowing: {@code (short) value},
+ * {@code writeBytes(value)}, or an explicit mask such as {@code value & 0x1FF}. Narrowing
+ * silently discards the high bits, so a value the editor accepted could be written back as
+ * a completely different one - a learnset move of 512 was stored as move 0 (no move at all),
+ * and a level of 200 as level 72. Nothing reported it, and the only way to notice was to
+ * reopen the file and read the wrong value back.
+ * <p>
+ * These methods turn that silent loss into a failure that names the field and the value.
+ * They are deliberately called on the <em>write</em> path rather than in the setters: the
+ * setters are also fed by the load path in some formats, and a file already containing an
+ * unusual value must still open. What must not happen is writing a value back out as
+ * something other than what was set.
+ */
+public final class FieldWidth
+{
+    private FieldWidth() {}
+
+    /**
+     * @param value the value about to be written
+     * @param numBits the number of bits the field actually occupies
+     * @param fieldName the field's name, for the failure message
+     * @return {@code value}, when it fits
+     * @throws IllegalArgumentException when it does not
+     */
+    public static int bits(int value, int numBits, String fieldName)
+    {
+        int max = (1 << numBits) - 1;
+        if (value < 0 || value > max)
+        {
+            throw new IllegalArgumentException(String.format(
+                    "%s is %d, which does not fit in the %d bits the file gives it (allowed: 0 to %d). "
+                            + "Writing it would silently store a different value.",
+                    fieldName, value, numBits, max));
+        }
+        return value;
+    }
+
+    /**
+     * An unsigned byte field, written through {@code writeBytes}.
+     */
+    public static int u8(int value, String fieldName)
+    {
+        return bits(value, 8, fieldName);
+    }
+
+    /**
+     * An unsigned 16-bit field, written through {@code writeShort}.
+     */
+    public static int u16(int value, String fieldName)
+    {
+        return bits(value, 16, fieldName);
+    }
+
+    /**
+     * A signed byte field. Distinct from {@link #u8} because the range is -128 to 127, not
+     * 0 to 255 - move priority is the field this exists for.
+     */
+    public static int s8(int value, String fieldName)
+    {
+        if (value < Byte.MIN_VALUE || value > Byte.MAX_VALUE)
+        {
+            throw new IllegalArgumentException(String.format(
+                    "%s is %d, which does not fit in the signed byte the file gives it "
+                            + "(allowed: %d to %d). Writing it would silently store a different value.",
+                    fieldName, value, Byte.MIN_VALUE, Byte.MAX_VALUE));
+        }
+        return value;
+    }
+}

src/main/java/io/github/turtleisaac/pokeditor/formats/encounters/GenericEncounterData.java

@@ -113,13 +113,15 @@ public static class WaterEncounterSet {
         int[] minLevels;
         int[] maxLevels;
         int[] species;
+        byte[][] slotPadding; // bytes between the levels and the species of each slot, preserved verbatim
 
         WaterEncounterSet(int numSlots)
         {
             this.numSlots = numSlots;
             minLevels = new int[numSlots];
             maxLevels = new int[numSlots];
             species = new int[numSlots];
+            slotPadding = new byte[numSlots][];
         }
 
         public int getNumSlots()

src/main/java/io/github/turtleisaac/pokeditor/formats/encounters/JohtoEncounterData.java

@@ -18,11 +18,35 @@ public class JohtoEncounterData extends GenericEncounterData
 
     int[] swarmSpecies;
 
+    // 2 bytes of padding after the rates - preserved verbatim so a save round-trips
+    byte[] ratePadding;
+
     public JohtoEncounterData(BytesDataContainer files)
     {
         super(files);
     }
 
+    private JohtoEncounterData()
+    {
+        super();
+        fieldSpecies = new int[NUM_FIELD_ENCOUNTER_SETS][NUM_BASE_FIELD_ENCOUNTER_SLOTS];
+        hoennSoundSpecies = new int[NUM_SOUND_SMASH_ENCOUNTER_SLOTS];
+        sinnohSoundSpecies = new int[NUM_SOUND_SMASH_ENCOUNTER_SLOTS];
+        swarmSpecies = new int[NUM_SWARM_ENCOUNTER_SLOTS];
+        smashEncounterSet = new WaterEncounterSet(NUM_SMASH_SLOTS);
+        ratePadding = new byte[NUM_RATE_PADDING_BYTES];
+
+        for (int i = 0; i < waterEncounters.length; i++)
+        {
+            waterEncounters[i] = new WaterEncounterSet(WaterEncounterSet.NUM_WATER_SLOTS);
+        }
+    }
+
+    public static JohtoEncounterData create()
+    {
+        return new JohtoEncounterData();
+    }
+
     @Override
     public void setData(BytesDataContainer files)
     {
@@ -42,7 +66,7 @@ public void setData(BytesDataContainer files)
         oldRodRate = reader.readUInt8();
         goodRodRate = reader.readUInt8();
         superRodRate = reader.readUInt8();
-        reader.skip(2);
+        ratePadding = reader.readBytes(NUM_RATE_PADDING_BYTES);
 
         for (int i = 0; i < NUM_BASE_FIELD_ENCOUNTER_SLOTS; i++)
         {
@@ -103,7 +127,7 @@ public BytesDataContainer save()
         MemBuf.MemBufWriter writer = dataBuf.writer();
 
         writer.writeBytes(fieldRate, surfRate, smashRate, oldRodRate, goodRodRate, superRodRate);
-        writer.skip(2);
+        writer.write(ratePadding != null ? ratePadding : new byte[NUM_RATE_PADDING_BYTES]);
 
         writer.writeBytes(fieldLevels);
 
@@ -149,6 +173,7 @@ private void writeWaterEncounterSet(MemBuf.MemBufWriter writer, WaterEncounterSe
         }
     }
 
+    private static final int NUM_RATE_PADDING_BYTES = 2;
     private static final int NUM_SOUND_SMASH_ENCOUNTER_SLOTS = 2;
     private static final int NUM_SMASH_SLOTS = 2;
     private static final int NUM_SWARM_ENCOUNTER_SLOTS = 4;