diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml new file mode 100644 index 0000000..a95932b --- /dev/null +++ b/.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 diff --git a/README.md b/README.md index b4d0b49..6c40a74 100644 --- a/README.md +++ b/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=` 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() 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=`; 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. diff --git a/pom.xml b/pom.xml index 17f2dbc..ca2efca 100644 --- a/pom.xml +++ b/pom.xml @@ -6,7 +6,7 @@ io.github.turtleisaac PokEditor-Core - 1.0-SNAPSHOT + 1.0.0 20 @@ -16,6 +16,22 @@ + + + com.fasterxml.jackson.core + jackson-databind + 2.15.2 + + + + com.fasterxml.jackson.core + jackson-core + 2.15.2 + + org.junit.jupiter junit-jupiter @@ -32,18 +48,11 @@ com.google.inject guice 7.0.0 - - - junit - junit - 4.13.2 + test - - com.fasterxml.jackson.dataformat - jackson-dataformat-xml - 2.15.2 - org.antlr @@ -55,7 +64,7 @@ io.github.turtleisaac Nds4j - 0.1.0 + 1.0.0 diff --git a/src/main/antlr4/io/github/turtleisaac/pokeditor/formats/trainers/SmogonTeam.g4 b/src/main/antlr4/io/github/turtleisaac/pokeditor/formats/trainers/SmogonTeam.g4 index 85bd7a3..ca6ea67 100644 --- a/src/main/antlr4/io/github/turtleisaac/pokeditor/formats/trainers/SmogonTeam.g4 +++ b/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 ; diff --git a/src/main/java/io/github/turtleisaac/pokeditor/formats/BytesDataContainer.java b/src/main/java/io/github/turtleisaac/pokeditor/formats/BytesDataContainer.java index abe661f..d99b0ba 100644 --- a/src/main/java/io/github/turtleisaac/pokeditor/formats/BytesDataContainer.java +++ b/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 m = super.get(file); + return m != null && m.containsKey(Objects.requireNonNullElse(key, Default.NO_GROUPING)); } public interface PatternIndex { diff --git a/src/main/java/io/github/turtleisaac/pokeditor/formats/FieldWidth.java b/src/main/java/io/github/turtleisaac/pokeditor/formats/FieldWidth.java new file mode 100644 index 0000000..c430019 --- /dev/null +++ b/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. + *

+ * 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. + *

+ * These methods turn that silent loss into a failure that names the field and the value. + * They are deliberately called on the write 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; + } +} diff --git a/src/main/java/io/github/turtleisaac/pokeditor/formats/encounters/GenericEncounterData.java b/src/main/java/io/github/turtleisaac/pokeditor/formats/encounters/GenericEncounterData.java index 98bb6ae..97945b1 100644 --- a/src/main/java/io/github/turtleisaac/pokeditor/formats/encounters/GenericEncounterData.java +++ b/src/main/java/io/github/turtleisaac/pokeditor/formats/encounters/GenericEncounterData.java @@ -113,6 +113,7 @@ 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) { @@ -120,6 +121,7 @@ public static class WaterEncounterSet { minLevels = new int[numSlots]; maxLevels = new int[numSlots]; species = new int[numSlots]; + slotPadding = new byte[numSlots][]; } public int getNumSlots() diff --git a/src/main/java/io/github/turtleisaac/pokeditor/formats/encounters/JohtoEncounterData.java b/src/main/java/io/github/turtleisaac/pokeditor/formats/encounters/JohtoEncounterData.java index 3d710be..989c000 100644 --- a/src/main/java/io/github/turtleisaac/pokeditor/formats/encounters/JohtoEncounterData.java +++ b/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; diff --git a/src/main/java/io/github/turtleisaac/pokeditor/formats/encounters/SinnohEncounterData.java b/src/main/java/io/github/turtleisaac/pokeditor/formats/encounters/SinnohEncounterData.java index 1664ae2..b93f3f7 100644 --- a/src/main/java/io/github/turtleisaac/pokeditor/formats/encounters/SinnohEncounterData.java +++ b/src/main/java/io/github/turtleisaac/pokeditor/formats/encounters/SinnohEncounterData.java @@ -17,6 +17,10 @@ public class SinnohEncounterData extends GenericEncounterData int[][] dualSlotSpecies; + // 0x2C bytes sitting between the surf encounters and the old rod encounters - contents are not + // understood yet, so they are preserved verbatim rather than zeroed out on save + byte[] unknown; + public SinnohEncounterData(BytesDataContainer files) { super(files); @@ -24,6 +28,24 @@ public SinnohEncounterData(BytesDataContainer files) private SinnohEncounterData() { super(); + fieldSpecies = new int[1][NUM_BASE_FIELD_ENCOUNTER_SLOTS]; + swarmSpecies = new int[NUM_SWARM_DAY_NIGHT_ENCOUNTER_SLOTS]; + daySpecies = new int[NUM_SWARM_DAY_NIGHT_ENCOUNTER_SLOTS]; + nightSpecies = new int[NUM_SWARM_DAY_NIGHT_ENCOUNTER_SLOTS]; + radarSpecies = new int[NUM_RADAR_ENCOUNTER_SLOTS]; + formProbability = new byte[NUM_FORM_PROBABILITY_BYTES]; + dualSlotSpecies = new int[NUM_DUAL_SLOT_GAMES][NUM_DUAL_SLOT_ENCOUNTER_SLOTS]; + unknown = new byte[NUM_UNKNOWN_BYTES]; + + for (int i = 0; i < waterEncounters.length; i++) + { + WaterEncounterSet set = new WaterEncounterSet(WaterEncounterSet.NUM_WATER_SLOTS); + for (int slot = 0; slot < WaterEncounterSet.NUM_WATER_SLOTS; slot++) + { + set.slotPadding[slot] = new byte[NUM_WATER_SLOT_PADDING_BYTES]; + } + waterEncounters[i] = set; + } } public static SinnohEncounterData create() @@ -82,7 +104,7 @@ public void setData(BytesDataContainer files) } //todo really read the form probability table - formProbability = reader.readBytes(24); + formProbability = reader.readBytes(NUM_FORM_PROBABILITY_BYTES); //todo really read the dual slot mons dualSlotSpecies = new int[NUM_DUAL_SLOT_GAMES][NUM_DUAL_SLOT_ENCOUNTER_SLOTS]; @@ -97,8 +119,8 @@ public void setData(BytesDataContainer files) surfRate = reader.readInt(); waterEncounters[0] = readWaterEncounterSet(reader); - //todo figure out wtf is going on here - reader.skip(0x2C); + //todo figure out wtf is going on here - preserved verbatim so a save round-trips + unknown = reader.readBytes(NUM_UNKNOWN_BYTES); oldRodRate = reader.readInt(); waterEncounters[1] = readWaterEncounterSet(reader); @@ -117,7 +139,7 @@ private WaterEncounterSet readWaterEncounterSet(MemBuf.MemBufReader reader) { set.maxLevels[i] = reader.readUInt8(); set.minLevels[i] = reader.readUInt8(); - reader.skip(2); + set.slotPadding[i] = reader.readBytes(NUM_WATER_SLOT_PADDING_BYTES); set.species[i] = reader.readInt(); } return set; @@ -173,8 +195,8 @@ public BytesDataContainer save() writeWaterEncounterSet(writer, surfRate, waterEncounters[0]); - //todo figure out wtf is going on here - writer.skip(0x2C); + //todo figure out wtf is going on here - preserved verbatim so a save round-trips + writer.write(unknown); writeWaterEncounterSet(writer, oldRodRate, waterEncounters[1]); writeWaterEncounterSet(writer, goodRodRate, waterEncounters[2]); @@ -189,7 +211,7 @@ private void writeWaterEncounterSet(MemBuf.MemBufWriter writer, int rate, WaterE for (int i = 0; i < set.getNumSlots(); i++) { writer.writeBytes(set.maxLevels[i], set.minLevels[i]); - writer.skip(2); + writer.write(set.slotPadding[i] != null ? set.slotPadding[i] : new byte[NUM_WATER_SLOT_PADDING_BYTES]); writer.writeInt(set.species[i]); } } @@ -248,6 +270,9 @@ public void setRadarSpecies(int idx, int species) private static final int NUM_RADAR_ENCOUNTER_SLOTS = 4; private static final int NUM_DUAL_SLOT_ENCOUNTER_SLOTS = 2; private static final int NUM_DUAL_SLOT_GAMES = 5; + private static final int NUM_FORM_PROBABILITY_BYTES = 24; + private static final int NUM_UNKNOWN_BYTES = 0x2C; + private static final int NUM_WATER_SLOT_PADDING_BYTES = 2; private static final int PLAT_FIELD_ENCOUNTER_SET_IDX = 0; enum DualSlot { diff --git a/src/main/java/io/github/turtleisaac/pokeditor/formats/evolutions/EvolutionData.java b/src/main/java/io/github/turtleisaac/pokeditor/formats/evolutions/EvolutionData.java index cd21aac..95e52da 100644 --- a/src/main/java/io/github/turtleisaac/pokeditor/formats/evolutions/EvolutionData.java +++ b/src/main/java/io/github/turtleisaac/pokeditor/formats/evolutions/EvolutionData.java @@ -2,6 +2,7 @@ import io.github.turtleisaac.nds4j.framework.MemBuf; import io.github.turtleisaac.pokeditor.formats.BytesDataContainer; +import io.github.turtleisaac.pokeditor.formats.FieldWidth; import io.github.turtleisaac.pokeditor.gamedata.GameFiles; import io.github.turtleisaac.pokeditor.formats.GenericFileData; @@ -11,6 +12,8 @@ public class EvolutionData extends ArrayList implements GenericFileData { + private int fileSize = FIXED_FILE_SIZE; + public EvolutionData(BytesDataContainer files) { super(); @@ -30,9 +33,16 @@ public void setData(BytesDataContainer files) MemBuf dataBuf = MemBuf.create(file); MemBuf.MemBufReader reader = dataBuf.reader(); - for (int i = 0; i < file.length / 6; i++) + fileSize = Math.max(file.length, FIXED_FILE_SIZE); + + // everything the file holds is read - dropping entries here would silently discard them on save + int numEntries = file.length / ENTRY_SIZE; + for (int i = 0; i < numEntries; i++) { - add(new EvolutionEntry(reader.readShort(), reader.readShort(), reader.readShort())); + // unsigned: these are species and item IDs, which run past 0x7FFF in expanded ROMs. + // readShort() sign extended them, so a species above 32767 came back negative and the + // sheet's declared 0..65535 range was a value the read path could not reproduce. + add(new EvolutionEntry(reader.readUInt16(), reader.readUInt16(), reader.readUInt16())); } } @@ -42,15 +52,34 @@ public BytesDataContainer save() MemBuf dataBuf = MemBuf.create(); MemBuf.MemBufWriter writer = dataBuf.writer(); + // the cap is what THIS file can hold, not what a retail one holds. setData deliberately + // reads every entry present so an expanded table is not silently truncated, and fileSize + // is kept at the input's length for the same reason - refusing to write those entries back + // would parse a hacked ROM cleanly and then abort the whole NARC on save. + int capacity = (fileSize - TERMINATOR_SIZE) / ENTRY_SIZE; + if (size() > capacity) + { + throw new RuntimeException("This evolution file holds " + capacity + " entries (" + + fileSize + " bytes). Provided: " + size() + + ". Remove an evolution, or expand the file first."); + } + for(EvolutionEntry entry : this) { - writer.writeShort((short) entry.getMethod()); - writer.writeShort((short) entry.getRequirement()); - writer.writeShort((short) entry.getResultSpecies()); + writer.writeShort((short) FieldWidth.u16(entry.getMethod(), "Evolution method")); + writer.writeShort((short) FieldWidth.u16(entry.getRequirement(), "Evolution requirement")); + writer.writeShort((short) FieldWidth.u16(entry.getResultSpecies(), "Evolution result species")); } writer.writeShort((short) 0); + // the game reads a fixed-size record regardless of how many evolutions are actually populated, + // so the subfile must always be emitted at its full length + if (writer.getPosition() < fileSize) + { + writer.writeByteNumTimes((byte) 0, fileSize - writer.getPosition()); + } + return new BytesDataContainer(GameFiles.EVOLUTIONS, null, dataBuf.reader().getBuffer()); } @@ -105,5 +134,14 @@ public void setResultSpecies(int resultSpecies) } } + /** bytes per entry: method, requirement and result species, each a u16 */ + public static final int ENTRY_SIZE = 6; + + /** the trailing u16 terminator every evolution file carries */ + public static final int TERMINATOR_SIZE = 2; + + /** how many entries a retail-sized file holds; an expanded file holds more */ public static final int MAX_NUM_ENTRIES = 7; + + public static final int FIXED_FILE_SIZE = MAX_NUM_ENTRIES * ENTRY_SIZE + TERMINATOR_SIZE; } diff --git a/src/main/java/io/github/turtleisaac/pokeditor/formats/items/ItemData.java b/src/main/java/io/github/turtleisaac/pokeditor/formats/items/ItemData.java index f64c069..2a7db03 100644 --- a/src/main/java/io/github/turtleisaac/pokeditor/formats/items/ItemData.java +++ b/src/main/java/io/github/turtleisaac/pokeditor/formats/items/ItemData.java @@ -27,6 +27,7 @@ public class ItemData implements GenericFileData int fieldFunction; // u8 int battleFunction; // u8 int workType; // u8 + int unknownPad; // u8 - unused byte at 0x0D, preserved verbatim //sleep, poison, burn, freeze, paralyze, confuse, attract, guard spec boolean[] statusRecoveries; // bitfield in this u8 @@ -50,6 +51,9 @@ public class ItemData implements GenericFileData int[] friendshipChangeAmounts; + int unknownBitfield2Bits; // upper nibble of the byte at 0x14, preserved verbatim + byte[] trailingPad; // trailing bytes of the entry, preserved verbatim + public ItemData(BytesDataContainer files) { setData(files); @@ -84,7 +88,7 @@ public void setData(BytesDataContainer files) fieldFunction = reader.readUInt8(); battleFunction = reader.readUInt8(); workType = reader.readUInt8(); - reader.skip(1); + unknownPad = reader.readUInt8(); statusRecoveries = new boolean[NUM_STATUS_RECOVERIES]; int recovery = reader.readUInt8(); @@ -135,6 +139,8 @@ public void setData(BytesDataContainer files) int bitfield2 = reader.readUInt8(); evYieldToggles[NUM_EV_YIELDS - 1] = (bitfield2 & 1) == 1; + unknownBitfield2Bits = bitfield2 & 0xf0; + friendshipChangeToggles = new boolean[NUM_FRIENDSHIP_CHANGE_FIELDS]; for (int i = 0; i < NUM_FRIENDSHIP_CHANGE_FIELDS; i++) { @@ -144,17 +150,20 @@ public void setData(BytesDataContainer files) evYields = new int[NUM_EV_YIELDS]; for (int i = 0; i < NUM_EV_YIELDS; i++) { - evYields[i] = reader.readByte(); // s8 + evYields[i] = (byte) reader.readByte(); // s8 } - hpRecoveryAmount = reader.readByte(); - ppRecoveryAmount = reader.readByte(); + hpRecoveryAmount = reader.readByte() & 0xFF; + ppRecoveryAmount = reader.readByte() & 0xFF; friendshipChangeAmounts = new int[NUM_FRIENDSHIP_CHANGE_FIELDS]; for (int i = 0; i < NUM_FRIENDSHIP_CHANGE_FIELDS; i++) { - friendshipChangeAmounts[i] = reader.readByte(); + friendshipChangeAmounts[i] = (byte) reader.readByte(); // s8: bitter berries lower friendship } + + // getBuffer() returns everything between the read and write positions - i.e. the unread remainder + trailingPad = reader.getBuffer(); } @Override @@ -175,7 +184,7 @@ public BytesDataContainer save() writer.writeShort((short) composite); writer.writeBytes(fieldFunction, battleFunction, workType); - writer.skip(1); + writer.writeBytes(unknownPad); composite = 0; for (int i = 0; i < statusRecoveries.length; i++) @@ -194,11 +203,11 @@ public BytesDataContainer save() for (int i = 1; i < 4; i += 2) { - composite = (statBoosts[i] & 0xf) | ((statBoosts[i + 1] & 0x3) << 4); + composite = (statBoosts[i] & 0xf) | ((statBoosts[i + 1] & 0xf) << 4); writer.writeBytes(composite); } - composite = (statBoosts[5] & 0xf) | ((statBoosts[6] & 0xf) << 4); + composite = (statBoosts[5] & 0xf) | ((statBoosts[6] & 0x3) << 4); composite |= (ppUpEffects[0] ? 1 : 0) << 6; composite |= (ppUpEffects[1] ? 1 : 0) << 7; writer.writeBytes(composite); @@ -221,13 +230,21 @@ public BytesDataContainer save() { composite |= (friendshipChangeToggles[i] ? 1 : 0) << i + 1; } + composite |= unknownBitfield2Bits & 0xf0; writer.writeBytes(composite); writer.writeBytes(evYields); - writer.writeBytes(hpRecoveryAmount, ppRecoveryAmount); + writer.writeBytes(hpRecoveryAmount & 0xFF, ppRecoveryAmount & 0xFF); writer.writeBytes(friendshipChangeAmounts); - writer.writeByteNumTimes((byte) 0, 2); + if (trailingPad != null && trailingPad.length != 0) + { + writer.write(trailingPad); + } + else + { + writer.writeByteNumTimes((byte) 0, 2); + } return new BytesDataContainer(GameFiles.ITEMS, null, dataBuf.reader().getBuffer()); } @@ -417,6 +434,20 @@ public int[] getStatBoosts() public void setStatBoosts(int[] statBoosts) { + if (statBoosts == null || statBoosts.length != NUM_STAT_BOOSTS) + { + throw new IllegalArgumentException("Stat boosts array must contain exactly " + NUM_STAT_BOOSTS + " entries"); + } + + for (int i = 0; i < NUM_STAT_BOOSTS; i++) + { + int max = (i == NUM_STAT_BOOSTS - 1) ? 0x3 : 0xf; + if (statBoosts[i] < 0 || statBoosts[i] > max) + { + throw new IllegalArgumentException("Stat boost at index " + i + " must be between 0 and " + max + " (got " + statBoosts[i] + ")"); + } + } + this.statBoosts = statBoosts; } diff --git a/src/main/java/io/github/turtleisaac/pokeditor/formats/learnsets/LearnsetData.java b/src/main/java/io/github/turtleisaac/pokeditor/formats/learnsets/LearnsetData.java index e95bb9c..68f0801 100644 --- a/src/main/java/io/github/turtleisaac/pokeditor/formats/learnsets/LearnsetData.java +++ b/src/main/java/io/github/turtleisaac/pokeditor/formats/learnsets/LearnsetData.java @@ -2,6 +2,7 @@ import io.github.turtleisaac.nds4j.framework.MemBuf; import io.github.turtleisaac.pokeditor.formats.BytesDataContainer; +import io.github.turtleisaac.pokeditor.formats.FieldWidth; import io.github.turtleisaac.pokeditor.gamedata.GameFiles; import io.github.turtleisaac.pokeditor.formats.GenericFileData; @@ -51,10 +52,22 @@ public void setData(BytesDataContainer files) // reader.setPosition(0); short combinedValue; - while ((combinedValue = reader.readShort()) != (short) 0xFFFF) + boolean terminated = false; + while (reader.getPosition() + 2 <= file.length) { + combinedValue = reader.readShort(); + if (combinedValue == (short) 0xFFFF) + { + terminated = true; + break; + } add(new LearnsetEntry(getMoveId(combinedValue), getLevelLearned(combinedValue))); } + + if (!terminated) + { + throw new RuntimeException("Level-up learnset is missing its 0xFFFF terminator (file is " + file.length + " bytes long)"); + } } @Override @@ -93,7 +106,24 @@ private static int getLevelLearned(short x) private static int produceLearnData(LearnsetEntry entry) { - return ((entry.getLevel() & 0x7f) << 9) | (entry.getMoveID() & 0x1ff); + // the two fields share one 16 bit word: 7 bits of level, 9 of move. masking instead of + // checking is what turned move 512 into move 0 and level 200 into level 72, silently. + int level = FieldWidth.bits(entry.getLevel(), 7, "Level"); + int moveID = FieldWidth.bits(entry.getMoveID(), 9, "Move ID"); + int packed = (level << 9) | moveID; + + // both fields are individually in range here, but this one combination packs to the + // same 0xFFFF the file uses to mark the end of the list. saving it writes a + // terminator, so the entry - and every entry after it - vanishes on reload. + if (packed == TERMINATOR) + { + throw new IllegalArgumentException(String.format( + "Move %d at level %d cannot be stored: the two together are indistinguishable " + + "from the end-of-learnset marker, so the entry would disappear when the " + + "file is read back. Use a different move or level.", + moveID, level)); + } + return packed; } public void sortLearnset() @@ -154,4 +184,7 @@ public void setLevel(int level) } public static final int MAX_NUM_ENTRIES = 20; + + /** the 16 bit value marking the end of the entry list; no real entry may pack to it */ + private static final int TERMINATOR = 0xFFFF; } diff --git a/src/main/java/io/github/turtleisaac/pokeditor/formats/learnsets/LearnsetParser.java b/src/main/java/io/github/turtleisaac/pokeditor/formats/learnsets/LearnsetParser.java index b153a9c..aa97acc 100644 --- a/src/main/java/io/github/turtleisaac/pokeditor/formats/learnsets/LearnsetParser.java +++ b/src/main/java/io/github/turtleisaac/pokeditor/formats/learnsets/LearnsetParser.java @@ -47,9 +47,17 @@ public List generateDataList(Map narcs, Map data = new ArrayList<>(); - for (byte[] subfile : learnsets.getFiles()) + List subfiles = learnsets.getFiles(); + for (int idx = 0; idx < subfiles.size(); idx++) { - data.add(new LearnsetData(new BytesDataContainer(GameFiles.LEVEL_UP_LEARNSETS, null, subfile))); + try + { + data.add(new LearnsetData(new BytesDataContainer(GameFiles.LEVEL_UP_LEARNSETS, null, subfiles.get(idx)))); + } + catch (RuntimeException e) + { + throw new RuntimeException("Failed to parse level-up learnset entry " + idx + ": " + e.getMessage(), e); + } } return data; diff --git a/src/main/java/io/github/turtleisaac/pokeditor/formats/moves/MoveData.java b/src/main/java/io/github/turtleisaac/pokeditor/formats/moves/MoveData.java index 424171c..987fed1 100644 --- a/src/main/java/io/github/turtleisaac/pokeditor/formats/moves/MoveData.java +++ b/src/main/java/io/github/turtleisaac/pokeditor/formats/moves/MoveData.java @@ -21,6 +21,7 @@ import io.github.turtleisaac.nds4j.framework.MemBuf; import io.github.turtleisaac.pokeditor.formats.BytesDataContainer; +import io.github.turtleisaac.pokeditor.formats.FieldWidth; import io.github.turtleisaac.pokeditor.formats.learnsets.LearnsetData; import io.github.turtleisaac.pokeditor.gamedata.GameFiles; import io.github.turtleisaac.pokeditor.formats.GenericFileData; @@ -92,7 +93,7 @@ public void setData(BytesDataContainer files) target = reader.readUInt16(); - priority = reader.readByte(); + priority = (byte) reader.readByte(); // s8: negative for Trick Room, Roar, Whirlwind... flags = new boolean[NUM_MOVE_FLAGS]; int composite = reader.readUInt8(); @@ -111,19 +112,24 @@ public BytesDataContainer save() MemBuf dataBuf = MemBuf.create(); MemBuf.MemBufWriter writer = dataBuf.writer(); - writer.writeShort((short) effect); - writer.writeBytes(category, power, type, accuracy, pp, effectChance); + // every one of these is narrowed on the way out - a short cast or writeBytes' i2b - + // so a value that does not fit is stored as a different value with nothing reported + writer.writeShort((short) FieldWidth.u16(effect, "Effect")); + writer.writeBytes(FieldWidth.u8(category, "Category"), FieldWidth.u8(power, "Power"), + FieldWidth.u8(type, "Type"), FieldWidth.u8(accuracy, "Accuracy"), + FieldWidth.u8(pp, "PP"), FieldWidth.u8(effectChance, "Effect chance")); - writer.writeShort((short) target); + writer.writeShort((short) FieldWidth.u16(target, "Target")); - writer.writeBytes(priority); + writer.writeBytes(FieldWidth.s8(priority, "Priority")); int composite = 0; for (int i = 0; i < NUM_MOVE_FLAGS; i++) { composite |= ((flags[i] ? 1 : 0) << i); } - writer.writeBytes(composite, contestEffect, contestType, 0, 0); + writer.writeBytes(composite, FieldWidth.u8(contestEffect, "Contest effect"), + FieldWidth.u8(contestType, "Contest type"), 0, 0); return new BytesDataContainer(GameFiles.MOVES, null, dataBuf.reader().getBuffer()); } diff --git a/src/main/java/io/github/turtleisaac/pokeditor/formats/personal/PersonalData.java b/src/main/java/io/github/turtleisaac/pokeditor/formats/personal/PersonalData.java index 91cc7be..4d25d53 100644 --- a/src/main/java/io/github/turtleisaac/pokeditor/formats/personal/PersonalData.java +++ b/src/main/java/io/github/turtleisaac/pokeditor/formats/personal/PersonalData.java @@ -2,6 +2,7 @@ import io.github.turtleisaac.nds4j.framework.MemBuf; import io.github.turtleisaac.pokeditor.formats.BytesDataContainer; +import io.github.turtleisaac.pokeditor.formats.FieldWidth; import io.github.turtleisaac.pokeditor.gamedata.GameFiles; import io.github.turtleisaac.pokeditor.formats.GenericFileData; @@ -40,6 +41,8 @@ public class PersonalData implements GenericFileData private int runChance; // u8 private int dexColor; // u8:7 + private byte[] padding; // 2 bytes of padding, preserved verbatim so a save round-trips + private int unknownEvYieldBits; // bits 12-15 of the ev yield halfword, preserved verbatim private boolean flip; // u8:1 private boolean[] tmCompatibility; // u8[16], each TM is a single bit @@ -106,6 +109,7 @@ public void setData(BytesDataContainer files) baseExp = reader.readUInt8(); int evYields = reader.readUInt16(); + unknownEvYieldBits = evYields & 0xF000; hpEvYield = getHpEv(evYields); atkEvYield = getAtkEv(evYields); defEvYield = getDefEv(evYields); @@ -131,7 +135,7 @@ public void setData(BytesDataContainer files) dexColor = colorFlip & 0x7F; flip = ((colorFlip & 0x80) >> 7) == 1; - reader.skip(2); // 2 bytes padding + padding = reader.readBytes(NUMBER_PADDING_BYTES); // 2 bytes padding, preserved verbatim byte[] tmLearnset = reader.readBytes(16); this.tmCompatibility = new boolean[NUMBER_TM_HM_BITS]; @@ -147,12 +151,23 @@ public BytesDataContainer save() MemBuf dataBuf = MemBuf.create(); MemBuf.MemBufWriter writer = dataBuf.writer(); - writer.writeBytes(hp, atk, def, speed, spAtk, spDef, type1, type2, catchRate, baseExp); + // the setters guard only the upper bound, so a negative reached this point and was + // narrowed by writeBytes into a large positive one - setHp(-1) was stored as 255 + writer.writeBytes(FieldWidth.u8(hp, "HP"), FieldWidth.u8(atk, "Attack"), + FieldWidth.u8(def, "Defense"), FieldWidth.u8(speed, "Speed"), + FieldWidth.u8(spAtk, "Sp. Attack"), FieldWidth.u8(spDef, "Sp. Defense"), + FieldWidth.u8(type1, "Type 1"), FieldWidth.u8(type2, "Type 2"), + FieldWidth.u8(catchRate, "Catch rate"), FieldWidth.u8(baseExp, "Base experience")); writer.writeShort(getCombinedEvShort()); - writer.writeShort((short)uncommonItem); - writer.writeShort((short)rareItem); - writer.writeBytes(genderRatio,hatchMultiplier,baseHappiness,expRate,eggGroup1,eggGroup2,ability1,ability2,runChance,getCombinedColorFlip()); - writer.skip(2); + writer.writeShort((short) FieldWidth.u16(uncommonItem, "Uncommon held item")); + writer.writeShort((short) FieldWidth.u16(rareItem, "Rare held item")); + writer.writeBytes(FieldWidth.u8(genderRatio, "Gender ratio"), + FieldWidth.u8(hatchMultiplier, "Hatch multiplier"), + FieldWidth.u8(baseHappiness, "Base happiness"), FieldWidth.u8(expRate, "Exp rate"), + FieldWidth.u8(eggGroup1, "Egg group 1"), FieldWidth.u8(eggGroup2, "Egg group 2"), + FieldWidth.u8(ability1, "Ability 1"), FieldWidth.u8(ability2, "Ability 2"), + FieldWidth.u8(runChance, "Run chance"), getCombinedColorFlip()); + writer.write(padding != null ? padding : new byte[NUMBER_PADDING_BYTES]); int[] tmLearnsetData = new int[16]; for (int i = 0; i < NUMBER_TM_HM_BITS; i++) @@ -224,19 +239,20 @@ public BytesDataContainer save() private short getCombinedEvShort() { int val = 0; - val |= hpEvYield; - val |= (atkEvYield << 2); - val |= (defEvYield << 4); - val |= (speedEvYield << 6); - val |= (spAtkEvYield << 8); - val |= (spDefEvYield << 10); + val |= (hpEvYield & 0x03); + val |= ((atkEvYield & 0x03) << 2); + val |= ((defEvYield & 0x03) << 4); + val |= ((speedEvYield & 0x03) << 6); + val |= ((spAtkEvYield & 0x03) << 8); + val |= ((spDefEvYield & 0x03) << 10); + val |= (unknownEvYieldBits & 0xF000); return (short) val; } private int getCombinedColorFlip() { - return dexColor | (flip ? 0x80 : 0); + return (dexColor & 0x7F) | (flip ? 0x80 : 0); } public int getHp() @@ -311,6 +327,17 @@ public void setSpDef(int spDef) this.spDef = spDef; } + /** + * How many Pokemon types an unmodified Generation 4 game defines: 18, numbered 0 to 17. + *

+ * This is what retail data contains, not a limit on what the field can hold - the type is + * stored in a whole byte, and ROM hacks with more than 18 types are exactly the thing this + * library exists to edit. The setters therefore do not enforce it; only the byte width is + * enforced, at save. A user interface that can only name or colour 18 types should say so + * itself rather than have the data layer refuse the value. + */ + public static final int NUMBER_OF_TYPES = 18; + public int getType1() { return type1; @@ -318,8 +345,6 @@ public int getType1() public void setType1(int type1) { - if (type1 >= 19) // TODO have a way to know for certain the number of types - throw new RuntimeException("Maximum type value is 18. Provided: " + type1); this.type1 = type1; } @@ -330,8 +355,6 @@ public int getType2() public void setType2(int type2) { - if (type2 >= 19) // TODO have a way to know for certain the number of types - throw new RuntimeException("Maximum type value is 18. Provided: " + type2); this.type2 = type2; } @@ -366,8 +389,8 @@ public int getHpEvYield() public void setHpEvYield(int hpEvYield) { - if (hpEvYield >= 5) - throw new RuntimeException("Maximum HP EV yield value is 4. Provided: " + hpEvYield); + if (hpEvYield < 0 || hpEvYield > 3) + throw new RuntimeException("Maximum HP EV yield value is 3. Provided: " + hpEvYield); this.hpEvYield = hpEvYield; } @@ -378,8 +401,8 @@ public int getAtkEvYield() public void setAtkEvYield(int atkEvYield) { - if (atkEvYield >= 5) - throw new RuntimeException("Maximum Attack EV yield value is 4. Provided: " + atkEvYield); + if (atkEvYield < 0 || atkEvYield > 3) + throw new RuntimeException("Maximum Attack EV yield value is 3. Provided: " + atkEvYield); this.atkEvYield = atkEvYield; } @@ -390,8 +413,8 @@ public int getDefEvYield() public void setDefEvYield(int defEvYield) { - if (defEvYield >= 5) - throw new RuntimeException("Maximum Defense EV yield value is 4. Provided: " + defEvYield); + if (defEvYield < 0 || defEvYield > 3) + throw new RuntimeException("Maximum Defense EV yield value is 3. Provided: " + defEvYield); this.defEvYield = defEvYield; } @@ -402,8 +425,8 @@ public int getSpeedEvYield() public void setSpeedEvYield(int speedEvYield) { - if (speedEvYield >= 5) - throw new RuntimeException("Maximum Speed EV yield value is 4. Provided: " + speedEvYield); + if (speedEvYield < 0 || speedEvYield > 3) + throw new RuntimeException("Maximum Speed EV yield value is 3. Provided: " + speedEvYield); this.speedEvYield = speedEvYield; } @@ -414,8 +437,8 @@ public int getSpAtkEvYield() public void setSpAtkEvYield(int spAtkEvYield) { - if (spAtkEvYield >= 5) - throw new RuntimeException("Maximum Special Attack EV yield value is 4. Provided: " + spAtkEvYield); + if (spAtkEvYield < 0 || spAtkEvYield > 3) + throw new RuntimeException("Maximum Special Attack EV yield value is 3. Provided: " + spAtkEvYield); this.spAtkEvYield = spAtkEvYield; } @@ -426,8 +449,8 @@ public int getSpDefEvYield() public void setSpDefEvYield(int spDefEvYield) { - if (spDefEvYield >= 5) - throw new RuntimeException("Maximum Special Defense EV yield value is 4. Provided: " + spDefEvYield); + if (spDefEvYield < 0 || spDefEvYield > 3) + throw new RuntimeException("Maximum Special Defense EV yield value is 3. Provided: " + spDefEvYield); this.spDefEvYield = spDefEvYield; } @@ -570,8 +593,8 @@ public int getDexColor() public void setDexColor(int dexColor) { - if (dexColor >= 129) - throw new RuntimeException("Maximum dex color value is 128. Provided: " + dexColor); + if (dexColor < 0 || dexColor > 0x7F) + throw new RuntimeException("Maximum dex color value is 127. Provided: " + dexColor); this.dexColor = dexColor; } @@ -596,6 +619,7 @@ public void setTmCompatibility(boolean[] tmCompatibility) } private static final int NUMBER_TM_HM_BITS = 128; + private static final int NUMBER_PADDING_BYTES = 2; protected static final int NUMBER_TMS_HMS = 100; private static int getHpEv(int x) diff --git a/src/main/java/io/github/turtleisaac/pokeditor/formats/personal/PersonalParser.java b/src/main/java/io/github/turtleisaac/pokeditor/formats/personal/PersonalParser.java index 99041b6..55322fb 100644 --- a/src/main/java/io/github/turtleisaac/pokeditor/formats/personal/PersonalParser.java +++ b/src/main/java/io/github/turtleisaac/pokeditor/formats/personal/PersonalParser.java @@ -38,10 +38,45 @@ public class PersonalParser implements GenericParser { - public static final int[] tmMoveIdNumbers = new int[PersonalData.NUMBER_TMS_HMS]; - public static final int[] tmMoveTypes = new int[PersonalData.NUMBER_TMS_HMS]; - private static boolean hasReadTmMoveIds = false; - private static boolean hasReadTmMoveTypes = false; + // Instance state rather than static, which is where per-parser state belongs. Note this + // buys no isolation between ROMs on its own: the parser is bound as a Guice singleton, so + // there is exactly one instance for the lifetime of the process and instance fields are as + // process-global as static ones were. What actually keeps one ROM's data out of another is + // DataManager discarding its caches when the ROM changes. + private final int[] tmMoveIdNumbers = new int[PersonalData.NUMBER_TMS_HMS]; + private final int[] tmMoveTypes = new int[PersonalData.NUMBER_TMS_HMS]; + // the palette index exactly as it was read, so that indices this editor does not recognize survive a save + private final int[] tmMovePaletteIndices = new int[PersonalData.NUMBER_TMS_HMS]; + private boolean hasReadTmMoveIds = false; + private boolean hasReadTmMoveTypes = false; + + /** + * Gets the move ID assigned to each TM/HM in the currently loaded ROM + * @return an int[] + */ + public int[] getTmMoveIdNumbers() + { + return tmMoveIdNumbers; + } + + /** + * Gets the move ID assigned to the given TM/HM in the currently loaded ROM + * @param tmID an int + * @return an int + */ + public int getTmMoveIdNumber(int tmID) + { + return tmMoveIdNumbers[tmID]; + } + + /** + * Gets the type of the move assigned to each TM/HM in the currently loaded ROM + * @return an int[] + */ + public int[] getTmMoveTypes() + { + return tmMoveTypes; + } @Override public List generateDataList(Map narcs, Map codeBinaries) @@ -84,7 +119,8 @@ public List generateDataList(Map narcs, Map processDataList(List data, Map 5; // rock case 0x19D -> 2; // flying case 0x262 -> 6; // bug + // an unrecognized index is reported as Normal, but the raw index is kept by the caller so that + // it can be written straight back out rather than rewritten to Normal's index default -> 0; //todo figure out how to handle ???/fairy }; @@ -242,13 +285,18 @@ private static int typeToTmHmPaletteIndex(int type) case 5 -> 0x19C; // rock case 2 -> 0x19D; // flying case 6 -> 0x262; // bug - case 9 -> 0x191; // ???/fairy - default -> 0x192; + // NOTE: this mapping is deliberately NOT a bijection. Type 9 (???/fairy) has no TM/HM palette of + // its own, so it shares Psychic's index and therefore reads back as Psychic (14). The same is + // true of every unrecognized type, which shares Normal's index. Callers which need to preserve + // an index they did not change must write the original index back instead of round-tripping it + // through these two methods. + case 9 -> 0x191; // ???/fairy - shares Psychic's palette, reads back as Psychic + default -> 0x192; // shares Normal's palette, reads back as Normal //todo figure out how to handle ???/fairy }; } - public static void updateTmType(int tmID, int moveID, List moves) + public void updateTmType(int tmID, int moveID, List moves) { tmMoveIdNumbers[tmID] = moveID; tmMoveTypes[tmID] = moves.get(moveID).getType(); diff --git a/src/main/java/io/github/turtleisaac/pokeditor/formats/pokemon_sprites/PokemonSpriteData.java b/src/main/java/io/github/turtleisaac/pokeditor/formats/pokemon_sprites/PokemonSpriteData.java index 5368212..7dd46d4 100644 --- a/src/main/java/io/github/turtleisaac/pokeditor/formats/pokemon_sprites/PokemonSpriteData.java +++ b/src/main/java/io/github/turtleisaac/pokeditor/formats/pokemon_sprites/PokemonSpriteData.java @@ -5,8 +5,11 @@ import io.github.turtleisaac.nds4j.images.Palette; import io.github.turtleisaac.pokeditor.formats.GenericFileData; import io.github.turtleisaac.pokeditor.formats.BytesDataContainer; +import io.github.turtleisaac.pokeditor.gamedata.Game; import io.github.turtleisaac.pokeditor.gamedata.GameFiles; +import java.util.Objects; + public class PokemonSpriteData implements GenericFileData { public static final int BATTLE_SPRITE_HEIGHT = 80; @@ -40,11 +43,57 @@ public class PokemonSpriteData implements GenericFileData private int partyIconPaletteIndex = 0; + private final boolean scanFrontToBack; + + private boolean femaleBackOffsetEmpty; + private boolean maleBackOffsetEmpty; + private boolean femaleFrontOffsetEmpty; + private boolean maleFrontOffsetEmpty; + + private boolean paletteEmpty; + private boolean shinyPaletteEmpty; + + /** + * Creates a PokemonSpriteData for a game which scans its sprites front-to-back + * (Platinum, HeartGold and SoulSilver - the games this module supports) + * @param files a BytesDataContainer + */ public PokemonSpriteData(BytesDataContainer files) { + this(files, true); + } + + /** + * Creates a PokemonSpriteData, deriving the sprite scan direction from the given game + * @param files a BytesDataContainer + * @param game a Game + */ + public PokemonSpriteData(BytesDataContainer files, Game game) + { + this(files, scanDirectionFor(game)); + } + + private PokemonSpriteData(BytesDataContainer files, boolean scanFrontToBack) + { + this.scanFrontToBack = scanFrontToBack; setData(files); } + /** + * Gets whether the given game scans its sprite data front-to-back + * @param game a Game + * @return a boolean + */ + public static boolean scanDirectionFor(Game game) + { + Objects.requireNonNull(game, "A game must be provided in order to determine the sprite scan direction"); + return switch (game) { + case Platinum, HeartGold, SoulSilver -> true; + // Diamond and Pearl seed their scanned sprite decoding from the last halfword rather than the first + case Diamond, Pearl -> false; + }; + } + @Override public void setData(BytesDataContainer files) { @@ -62,25 +111,37 @@ public void setData(BytesDataContainer files) byte[] femaleFrontHeightOffsetFile = files.get(GameFiles.BATTLE_SPRITE_HEIGHT, BattleSpriteHeightOffsetsPattern.FEMALE_FRONT_Y); byte[] maleFrontHeightOffsetFile = files.get(GameFiles.BATTLE_SPRITE_HEIGHT, BattleSpriteHeightOffsetsPattern.MALE_FRONT_Y); - palette = new Palette(paletteFile, 4); - shinyPalette = new Palette(shinyPaletteFile, 4); + paletteEmpty = paletteFile.length == 0; + shinyPaletteEmpty = shinyPaletteFile.length == 0; + + // 0 = read the file's own bit depth. Forcing 4 mislabels the occasional 8bpp palette + // (its 0x4 marker gets re-emitted as 0x3), corrupting it on a plain load/save round-trip. + if (!paletteEmpty) + palette = new Palette(paletteFile, 0); + if (!shinyPaletteEmpty) + shinyPalette = new Palette(shinyPaletteFile, 0); if (femaleBackFile.length != 0) - femaleBack = new IndexedImage(femaleBackFile, 0, 0, 1, 1, true); + femaleBack = new IndexedImage(femaleBackFile, 0, 0, 1, 1, scanFrontToBack); if (maleBackFile.length != 0) - maleBack = new IndexedImage(maleBackFile, 0, 0, 1, 1, true); + maleBack = new IndexedImage(maleBackFile, 0, 0, 1, 1, scanFrontToBack); if (femaleFrontFile.length != 0) - femaleFront = new IndexedImage(femaleFrontFile, 0, 0, 1, 1, true); + femaleFront = new IndexedImage(femaleFrontFile, 0, 0, 1, 1, scanFrontToBack); if (maleFrontFile.length != 0) - maleFront = new IndexedImage(maleFrontFile, 0, 0, 1, 1, true); + maleFront = new IndexedImage(maleFrontFile, 0, 0, 1, 1, scanFrontToBack); + + femaleBackOffsetEmpty = femaleBackHeightOffsetFile.length == 0; + maleBackOffsetEmpty = maleBackHeightOffsetFile.length == 0; + femaleFrontOffsetEmpty = femaleFrontHeightOffsetFile.length == 0; + maleFrontOffsetEmpty = maleFrontHeightOffsetFile.length == 0; - if (femaleBackHeightOffsetFile.length != 0) + if (!femaleBackOffsetEmpty) femaleBackOffset = -(femaleBackHeightOffsetFile[0] & 0xff); - if (maleBackHeightOffsetFile.length != 0) + if (!maleBackOffsetEmpty) maleBackOffset = -(maleBackHeightOffsetFile[0] & 0xff); - if (femaleFrontHeightOffsetFile.length != 0) + if (!femaleFrontOffsetEmpty) femaleFrontOffset = -(femaleFrontHeightOffsetFile[0] & 0xff); - if (maleFrontHeightOffsetFile.length != 0) + if (!maleFrontOffsetEmpty) maleFrontOffset = -(maleFrontHeightOffsetFile[0] & 0xff); MemBuf buffer = MemBuf.create(metadata); @@ -91,11 +152,11 @@ public void setData(BytesDataContainer files) unknownSection1 = reader.readBytes(42); //bytes 2-43 backMovement = reader.readUInt8(); //byte 44 unknownSection2 = reader.readBytes(41); //bytes 45-85 - globalFrontYOffset = reader.readByte(); //byte 86 - shadowXOffset = reader.readByte(); //byte 87 + globalFrontYOffset = (byte) reader.readByte(); //byte 86, s8 + shadowXOffset = (byte) reader.readByte(); //byte 87, s8 shadowSize = reader.readUInt8(); //byte 88 - partyIcon = new IndexedImage(partyIconFile, 4, 0, 1, 1, true); + partyIcon = new IndexedImage(partyIconFile, 4, 0, 1, 1, scanFrontToBack); } @Override @@ -106,22 +167,15 @@ public BytesDataContainer save() container.insert(GameFiles.BATTLE_SPRITES, BattleSpriteNarcPattern.MALE_BACK, maleBack != null ? maleBack.save() : new byte[] {}); container.insert(GameFiles.BATTLE_SPRITES, BattleSpriteNarcPattern.FEMALE_FRONT, femaleFront != null ? femaleFront.save() : new byte[] {}); container.insert(GameFiles.BATTLE_SPRITES, BattleSpriteNarcPattern.MALE_FRONT, maleFront != null ? maleFront.save() : new byte[] {}); - container.insert(GameFiles.BATTLE_SPRITES, BattleSpriteNarcPattern.PALETTE, palette.save()); - container.insert(GameFiles.BATTLE_SPRITES, BattleSpriteNarcPattern.SHINY_PALETTE, shinyPalette.save()); + container.insert(GameFiles.BATTLE_SPRITES, BattleSpriteNarcPattern.PALETTE, palette != null ? palette.save() : new byte[] {}); + container.insert(GameFiles.BATTLE_SPRITES, BattleSpriteNarcPattern.SHINY_PALETTE, shinyPalette != null ? shinyPalette.save() : new byte[] {}); - container.insert(GameFiles.PARTY_ICONS, null, partyIcon.save()); + container.insert(GameFiles.PARTY_ICONS, null, partyIcon != null ? partyIcon.save() : new byte[] {}); - byte[] femaleBackHeightOffsetFile = new byte[] {(byte) Math.abs(femaleBackOffset)}; - container.insert(GameFiles.BATTLE_SPRITE_HEIGHT, BattleSpriteHeightOffsetsPattern.FEMALE_BACK_Y, femaleBackHeightOffsetFile); - - byte[] maleBackHeightOffsetFile = new byte[] {(byte) Math.abs(maleBackOffset)}; - container.insert(GameFiles.BATTLE_SPRITE_HEIGHT, BattleSpriteHeightOffsetsPattern.MALE_BACK_Y, maleBackHeightOffsetFile); - - byte[] femaleFrontHeightOffsetFile = new byte[] {(byte) Math.abs(femaleFrontOffset)}; - container.insert(GameFiles.BATTLE_SPRITE_HEIGHT, BattleSpriteHeightOffsetsPattern.FEMALE_FRONT_Y, femaleFrontHeightOffsetFile); - - byte[] maleFrontHeightOffsetFile = new byte[] {(byte) Math.abs(maleFrontOffset)}; - container.insert(GameFiles.BATTLE_SPRITE_HEIGHT, BattleSpriteHeightOffsetsPattern.MALE_FRONT_Y, maleFrontHeightOffsetFile); + container.insert(GameFiles.BATTLE_SPRITE_HEIGHT, BattleSpriteHeightOffsetsPattern.FEMALE_BACK_Y, heightOffsetFile(femaleBackOffset, femaleBackOffsetEmpty)); + container.insert(GameFiles.BATTLE_SPRITE_HEIGHT, BattleSpriteHeightOffsetsPattern.MALE_BACK_Y, heightOffsetFile(maleBackOffset, maleBackOffsetEmpty)); + container.insert(GameFiles.BATTLE_SPRITE_HEIGHT, BattleSpriteHeightOffsetsPattern.FEMALE_FRONT_Y, heightOffsetFile(femaleFrontOffset, femaleFrontOffsetEmpty)); + container.insert(GameFiles.BATTLE_SPRITE_HEIGHT, BattleSpriteHeightOffsetsPattern.MALE_FRONT_Y, heightOffsetFile(maleFrontOffset, maleFrontOffsetEmpty)); MemBuf buffer = MemBuf.create(); MemBuf.MemBufWriter writer = buffer.writer(); @@ -138,6 +192,22 @@ public BytesDataContainer save() return container; } + /** + * Produces the height offset subfile for the given offset. A zero-length entry in the ROM has to stay + * zero-length, and the stored byte is the exact inverse of what setData reads, which + * negates it. + * + * @param offset an int containing the height offset + * @param wasEmpty a boolean containing whether the entry this came from was zero-length + * @return a byte[] + */ + private static byte[] heightOffsetFile(int offset, boolean wasEmpty) + { + if (wasEmpty) + return new byte[0]; + return new byte[] {(byte) (-offset)}; + } + public Palette getPalette() { diff --git a/src/main/java/io/github/turtleisaac/pokeditor/formats/pokemon_sprites/PokemonSpriteParser.java b/src/main/java/io/github/turtleisaac/pokeditor/formats/pokemon_sprites/PokemonSpriteParser.java index eb95bfd..6107a57 100644 --- a/src/main/java/io/github/turtleisaac/pokeditor/formats/pokemon_sprites/PokemonSpriteParser.java +++ b/src/main/java/io/github/turtleisaac/pokeditor/formats/pokemon_sprites/PokemonSpriteParser.java @@ -20,9 +20,15 @@ public class PokemonSpriteParser implements GenericParser { - private static final int MAX_PARTY_ICON_PALETTE_VALUE = 2; + public static final int MAX_PARTY_ICON_PALETTE_VALUE = 2; - private static List partyIconStartingFiles; + private static final int NUM_PARTY_ICON_STARTING_FILES = 7; + + // Instance state rather than static. As in PersonalParser, this is not what isolates one + // ROM from another - the parser is a Guice singleton - it is simply where the state belongs. + private List partyIconStartingFiles; + private List partyIconTrailingFiles; + private int partyIconPaletteTableLength = -1; @Override public List generateDataList(Map narcs, Map codeBinaries) @@ -58,11 +64,6 @@ public List generateDataList(Map narcs, Map< } MainCodeFile arm9 = (MainCodeFile) codeBinaries.get(GameCodeBinaries.ARM9); - MemBuf.MemBufReader arm9Reader = arm9.getPhysicalAddressBuffer().reader(); - arm9Reader.setPosition(Tables.PARTY_ICON_PALETTE.getPointerOffset()); - int offset = arm9Reader.readInt(); - arm9Reader.setPosition(offset - arm9.getRamStartAddress()); - Narc sprites = narcs.get(GameFiles.BATTLE_SPRITES); Narc spriteHeights = narcs.get(GameFiles.BATTLE_SPRITE_HEIGHT); @@ -70,9 +71,30 @@ public List generateDataList(Map narcs, Map< Narc partyIcons = narcs.get(GameFiles.PARTY_ICONS); ArrayList data = new ArrayList<>(); - Palette partyIconPalette = new Palette(partyIcons.getFile(0), 4); + PokemonSpriteData.BattleSpriteNarcPattern[] spritesNarcPattern = PokemonSpriteData.BattleSpriteNarcPattern.values(); + PokemonSpriteData.BattleSpriteHeightOffsetsPattern[] spriteHeightOffsetsPattern = PokemonSpriteData.BattleSpriteHeightOffsetsPattern.values(); + + int numSpecies = sprites.getFiles().size() / spritesNarcPattern.length; + + // read the entire party icon palette index table up front so the shared arm9 buffer is only + // touched while its lock is held + int[] partyIconPaletteIndices; + arm9.lock(); + try { + MemBuf.MemBufReader arm9Reader = arm9.getPhysicalAddressBuffer().reader(); + arm9Reader.setPosition(Tables.PARTY_ICON_PALETTE.getPointerOffset()); + int offset = arm9Reader.readInt(); + arm9Reader.setPosition(offset - arm9.getRamStartAddress()); + partyIconPaletteIndices = arm9Reader.readBytesI(numSpecies); + } + finally { + arm9.unlock(); + } + partyIconPaletteTableLength = partyIconPaletteIndices.length; + + Palette partyIconPalette = new Palette(partyIcons.getFile(0), 0); // read the file's own bit depth partyIconStartingFiles = new ArrayList<>(); - for (int i = 0; i < 7; i++) + for (int i = 0; i < NUM_PARTY_ICON_STARTING_FILES; i++) { partyIconStartingFiles.add(partyIcons.getFile(i)); } @@ -80,16 +102,14 @@ public List generateDataList(Map narcs, Map< MemBuf spriteMetadataBuffer = MemBuf.create(spriteMetadata.getFile(0)); MemBuf.MemBufReader spriteMetadataReader = spriteMetadataBuffer.reader(); - PokemonSpriteData.BattleSpriteNarcPattern[] spritesNarcPattern = PokemonSpriteData.BattleSpriteNarcPattern.values(); - PokemonSpriteData.BattleSpriteHeightOffsetsPattern[] spriteHeightOffsetsPattern = PokemonSpriteData.BattleSpriteHeightOffsetsPattern.values(); - for (int i = 0; i < sprites.getFiles().size() / spritesNarcPattern.length; i++) + for (int i = 0; i < numSpecies; i++) { BytesDataContainer container = new BytesDataContainer(); for (PokemonSpriteData.BattleSpriteNarcPattern entry : spritesNarcPattern) { container.insert(GameFiles.BATTLE_SPRITES, entry, sprites.getFile((i*spritesNarcPattern.length) + entry.getIndex())); } - container.insert(GameFiles.PARTY_ICONS, null, partyIcons.getFile(i + 7)); + container.insert(GameFiles.PARTY_ICONS, null, partyIcons.getFile(i + NUM_PARTY_ICON_STARTING_FILES)); container.insert(GameFiles.BATTLE_SPRITE_METADATA, null, spriteMetadataReader.readBytes(89)); for (PokemonSpriteData.BattleSpriteHeightOffsetsPattern entry : spriteHeightOffsetsPattern) { @@ -98,13 +118,22 @@ public List generateDataList(Map narcs, Map< PokemonSpriteData species = new PokemonSpriteData(container); species.getPartyIcon().setPalette(partyIconPalette); - int val = arm9Reader.readUInt8(); - if (val <= MAX_PARTY_ICON_PALETTE_VALUE) - species.setPartyIconPaletteIndex(val); + // the raw value is stored even when it is out of the range the editor knows about, otherwise + // simply loading and saving a ROM would rewrite the table + species.setPartyIconPaletteIndex(partyIconPaletteIndices[i]); data.add(species); } + // Party icons after the per-species block (alternate forms, egg/substitute icons, etc.) are not part + // of the per-species editing model. Preserve them verbatim, as with the leading files, so that loading + // and saving a ROM round-trips the whole narc instead of dropping them. + partyIconTrailingFiles = new ArrayList<>(); + for (int i = NUM_PARTY_ICON_STARTING_FILES + numSpecies; i < partyIcons.getFiles().size(); i++) + { + partyIconTrailingFiles.add(partyIcons.getFile(i)); + } + return data; } @@ -140,11 +169,15 @@ public Map processDataList(List data, Map processDataList(List data, Map commandID >= 0x16 && commandID <= 0x1D && commandID != 0x1B; + private static final int GOTO_IF_TRAINER_DEFEATED = 225; + + // 225 (goto_if_trainer_defeated) is a relative branch, so its destination has to be registered as a label + private static final IntPredicate isCallCommand = commandID -> (commandID >= 0x16 && commandID <= 0x1D && commandID != 0x1B) || commandID == GOTO_IF_TRAINER_DEFEATED; + // 0x2 (End), 0x16 (Jump) and 0x1B (Return) terminate the current run of commands. + // 0x15 (endstd/LocalScript) does NOT: verified against retail HeartGold and SoulSilver, treating it as a + // terminator drops the commands that follow it (round-trips one fewer script and re-emits shorter files). private static final IntPredicate isEndCommand = commandId -> commandId == 0x2 || commandId == 0x16 || commandId == 0x1B; - private static final IntPredicate isDoIfCommand = commandID -> commandID == 28 || commandID == 29 || commandID == 225; + // 225 is NOT a comparator command - per Scrcmd_Hg.txt it takes a single relative destination and no comparator byte + private static final IntPredicate isDoIfCommand = commandID -> commandID == 28 || commandID == 29; private static final IntPredicate isMovementCommand = commandID -> commandID == 0x5E; private static final IntPredicate isEndMovementCommand = commandID -> commandID == 0xFE; // private static final IntPredicate isOverworldObjectCommand @@ -37,19 +44,62 @@ public class FieldScriptData extends GenericScriptData comparators.put(5, "DIFFERENT"); } + /** + * Determines whether the given macro parameter name identifies a branch destination which has to be + * resolved to a label + * @param parameterName a String containing the name of a parameter as declared by its macro + * @return a boolean + */ + private static boolean isLabelParameterName(String parameterName) + { + return parameterName.contains("dest") || parameterName.contains("sub") || parameterName.equals("arg0"); + } + private List scripts; + private int fileIndex = -1; + public FieldScriptData(BytesDataContainer files) { super(files); } + public FieldScriptData(BytesDataContainer files, int fileIndex) + { + super(); + this.fileIndex = fileIndex; + setData(files); + } + public FieldScriptData() { super(); scripts = new ArrayList<>(); } + /** + * Gets the index of this script file within the field scripts narc, or -1 if it is not known + * @return an int + */ + public int getFileIndex() + { + return fileIndex; + } + + /** + * Sets the index of this script file within the field scripts narc (used purely for error reporting) + * @param fileIndex an int + */ + public void setFileIndex(int fileIndex) + { + this.fileIndex = fileIndex; + } + + private String describeFile() + { + return "script file " + (fileIndex >= 0 ? String.valueOf(fileIndex) : "?"); + } + @Override public void setData(BytesDataContainer files) { @@ -93,7 +143,7 @@ public void setData(BytesDataContainer files) for (int i = 0; i < labelOffsets.size(); i++) { if (!actionOffsets.contains(labelOffsets.get(i))) - readAtOffset(dataBuf, globalScriptOffsets, labelOffsets, actionOffsets, visitedOffsets, labelOffsets.get(i), labelMap, false); + readAtOffset(dataBuf, globalScriptOffsets, labelOffsets, actionOffsets, visitedOffsets, labelOffsets.get(i), labelMap, actionMap, false); } } while (lastSize != labelOffsets.size()); @@ -102,7 +152,7 @@ public void setData(BytesDataContainer files) for (int i = 0; i < labelOffsets.size(); i++) { if (!actionOffsets.contains(labelOffsets.get(i))) - readAtOffset(dataBuf, globalScriptOffsets, labelOffsets, actionOffsets, visitedOffsets, labelOffsets.get(i), labelMap, true); + readAtOffset(dataBuf, globalScriptOffsets, labelOffsets, actionOffsets, visitedOffsets, labelOffsets.get(i), labelMap, actionMap, true); else readActionAtOffset(dataBuf, actionOffsets, visitedOffsets, actionMap, labelOffsets.get(i)); } @@ -127,7 +177,7 @@ else if (component instanceof ActionLabel actionLabel) scripts.sort(Comparator.comparingInt(ScriptLabel::getScriptID)); if (scripts.size() != globalScriptOffsets.size()) - throw new RuntimeException(String.format("the expected number of scripts (%d) does not match the actual located amount (%d)", globalScriptOffsets.size(), globalScriptOffsets.size())); + throw new RuntimeException(String.format("%s: the expected number of scripts (%d) does not match the actual located amount (%d)", describeFile(), globalScriptOffsets.size(), scripts.size())); for (ScriptComponent component : this) { @@ -138,7 +188,7 @@ else if (component instanceof ActionLabel actionLabel) for (int i = 0; i < command.parameters.length; i++) { String paramName = command.commandMacro.getParameters()[i]; - if ((paramName.contains("dest") || paramName.contains("sub"))) + if (isLabelParameterName(paramName)) { command.parameters[i] = "label_" + labels.indexOf(labelMap.get((Integer) command.parameters[i])); } @@ -149,7 +199,7 @@ else if (isMovementCommand.test(command.commandMacro.getId())) for (int i = 0; i < command.parameters.length; i++) { String paramName = command.commandMacro.getParameters()[i]; - if ((paramName.contains("dest") || paramName.contains("sub"))) + if (isLabelParameterName(paramName)) { command.parameters[i] = "action_" + actions.indexOf(actionMap.get((Integer) command.parameters[i])); } @@ -176,7 +226,7 @@ else if (paramName.contains("overworld")) // } } - private void readAtOffset(MemBuf dataBuf, ArrayList globalScriptOffsets, ArrayList labelOffsets, ArrayList actionOffsets, ArrayList visitedOffsets, int offset, HashMap labelMap, boolean finalRun) + private void readAtOffset(MemBuf dataBuf, ArrayList globalScriptOffsets, ArrayList labelOffsets, ArrayList actionOffsets, ArrayList visitedOffsets, int offset, HashMap labelMap, HashMap actionMap, boolean finalRun) { MemBuf.MemBufReader reader = dataBuf.reader(); if (visitedOffsets.contains(offset)) { @@ -186,8 +236,10 @@ private void readAtOffset(MemBuf dataBuf, ArrayList globalScriptOffsets reader.setPosition(offset); int currentPosition; + int end = dataBuf.writer().getPosition(); - while (reader.getPosition() < dataBuf.writer().getPosition()) + // a command is at least a 2 byte id, so stop as soon as a whole id no longer fits + while (reader.getPosition() + 2 <= end) { currentPosition = reader.getPosition(); if (finalRun && !visitedOffsets.contains(currentPosition)) @@ -219,6 +271,7 @@ else if (labelOffsets.contains(currentPosition)) else if (actionOffsets.contains(currentPosition)) { ActionLabel actionLabel = new ActionLabel("action_" + Integer.toHexString(currentPosition)); + actionMap.putIfAbsent(currentPosition, actionLabel); add(actionLabel); } } @@ -230,13 +283,18 @@ else if (actionOffsets.contains(currentPosition)) // System.err.println(commandID); CommandMacro commandMacro = FieldScriptParser.nativeCommands.get(commandID); if (commandMacro == null) { - throw new RuntimeException("Invalid command ID: " + commandID); + throw new RuntimeException(describeFile() + ": Invalid command ID: " + commandID + " at offset 0x" + Integer.toHexString(currentPosition)); } ScriptCommand command = new ScriptCommand(commandMacro); command.name = commandMacro.getName(); - command.parameters = commandMacro.readParameters(reader); + try { + command.parameters = commandMacro.readParameters(reader); + } + catch (IllegalStateException e) { + throw new RuntimeException(String.format("%s: the command \"%s\" at offset 0x%s runs past the end of the file", describeFile(), commandMacro.getName(), Integer.toHexString(currentPosition)), e); + } // if (command.parameters != null) // { @@ -307,7 +365,11 @@ private void readActionAtOffset(MemBuf dataBuf, ArrayList actionOffsets reader.setPosition(offset); - while (reader.getPosition() < dataBuf.writer().getPosition()) + int end = dataBuf.writer().getPosition(); + boolean terminated = false; + + // each movement is a 2 byte id plus a 2 byte parameter, so stop as soon as a whole record no longer fits + while (reader.getPosition() + 4 <= end) { if (!visitedOffsets.contains(reader.getPosition())) { @@ -331,6 +393,7 @@ private void readActionAtOffset(MemBuf dataBuf, ArrayList actionOffsets if (isEndMovementCommand.test(commandID)) { + terminated = true; break; } @@ -344,6 +407,11 @@ private void readActionAtOffset(MemBuf dataBuf, ArrayList actionOffsets // // command.parameters = commandMacro.readParameters(reader); } + + if (!terminated) + { + throw new RuntimeException(String.format("%s: the action sequence at offset 0x%s runs off the end of the file without an end-movement command", describeFile(), Integer.toHexString(offset))); + } } private void findAndReplaceSequencesWithConvenienceCommands() @@ -454,7 +522,7 @@ else if (component instanceof ActionLabel actionLabel) idx++; } - return 0; + throw new RuntimeException(String.format("%s: the label \"%s\" could not be resolved to an offset - it is referenced by a branch but never defined", describeFile(), labelName)); }; for (ScriptComponent component : this) @@ -753,9 +821,11 @@ public String[] getParameterStrings() { if (parameters[i] instanceof Integer val) { - if (val >= 0x4000) + // ScriptFile.g4's NUMBER token has no sign, so a negative value has to be emitted in + // hexadecimal or it cannot be lexed back in + if (val >= 0x4000 || val < 0) { - parameterStrings[i] = "0x" + Integer.toHexString((int) parameters[i]); + parameterStrings[i] = "0x" + Integer.toHexString(val); } else { @@ -841,9 +911,20 @@ public ActionCommand(String name, int id, int parameter) public ActionCommand(int id, int parameter) { this.name = String.valueOf(id); + this.id = id; this.parameter = parameter; } + public int getId() + { + return id; + } + + public int getParameter() + { + return parameter; + } + @Override public String toString() { diff --git a/src/main/java/io/github/turtleisaac/pokeditor/formats/scripts/FieldScriptParser.java b/src/main/java/io/github/turtleisaac/pokeditor/formats/scripts/FieldScriptParser.java index bf6a8e5..19ca16f 100644 --- a/src/main/java/io/github/turtleisaac/pokeditor/formats/scripts/FieldScriptParser.java +++ b/src/main/java/io/github/turtleisaac/pokeditor/formats/scripts/FieldScriptParser.java @@ -130,9 +130,10 @@ public List generateDataList(Map narcs, Map< Narc scripts = narcs.get(GameFiles.FIELD_SCRIPTS); ArrayList data = new ArrayList<>(); -// int i = 0; - for (byte[] subfile : scripts.getFiles()) + List subfiles = scripts.getFiles(); + for (int fileIndex = 0; fileIndex < subfiles.size(); fileIndex++) { + byte[] subfile = subfiles.get(fileIndex); // System.out.print(i); // if (i == 271) { // System.currentTimeMillis(); @@ -145,9 +146,8 @@ public List generateDataList(Map narcs, Map< else { // System.out.println(" (Normal)"); - data.add(new FieldScriptData(new BytesDataContainer(GameFiles.FIELD_SCRIPTS, null, subfile))); + data.add(new FieldScriptData(new BytesDataContainer(GameFiles.FIELD_SCRIPTS, null, subfile), fileIndex)); } -// i++; } return data; diff --git a/src/main/java/io/github/turtleisaac/pokeditor/formats/scripts/LevelScriptData.java b/src/main/java/io/github/turtleisaac/pokeditor/formats/scripts/LevelScriptData.java index 620713a..7018bd5 100644 --- a/src/main/java/io/github/turtleisaac/pokeditor/formats/scripts/LevelScriptData.java +++ b/src/main/java/io/github/turtleisaac/pokeditor/formats/scripts/LevelScriptData.java @@ -6,8 +6,8 @@ import java.util.ArrayList; import java.util.Arrays; +import java.util.List; import java.util.Objects; -import java.util.TreeSet; import java.util.stream.Collectors; // this class only exists because AdAstra is awesome and wrote the only tool (until now) which could work with them @@ -17,11 +17,14 @@ */ public class LevelScriptData extends GenericScriptData { - private boolean hasPadding = true; + // NOTE: no initializer - instance initializers run after the superclass constructor, which is what + // parses the file, so an initializer here would overwrite whatever setData() worked out + private boolean hasPadding; public LevelScriptData() { super(); + hasPadding = true; } public LevelScriptData(BytesDataContainer files) @@ -37,7 +40,8 @@ public void setData(BytesDataContainer files) throw new RuntimeException("Script file not provided to editor"); } - MemBuf dataBuf = MemBuf.create(files.get(GameFiles.FIELD_SCRIPTS, null)); + byte[] file = files.get(GameFiles.FIELD_SCRIPTS, null); + MemBuf dataBuf = MemBuf.create(file); MemBuf.MemBufReader reader = dataBuf.reader(); ArrayList temp = new ArrayList<>(); @@ -85,6 +89,7 @@ public void setData(BytesDataContainer files) if (reader.readUInt16() == 0 && dataBuf.writer().getPosition() < SMALLEST_TRIGGER_SIZE) { //todo come back here // LSTrigger.customInfo("This level script does nothing.", "Interesting..."); + hasPadding = false; return; } } @@ -107,6 +112,34 @@ public void setData(BytesDataContainer files) } } } + + // whether this file carries trailing padding is a property of the file, not a constant + hasPadding = file.length != getUnpaddedLength(); + } + + /** + * Calculates the number of bytes save() emits before any trailing padding is applied + * @return the unpadded length in bytes of this level script + */ + private int getUnpaddedLength() + { + if (isEmpty()) + return 4; + + int mapScreenLoadCount = 0; + int variableCount = 0; + for (ScriptComponent component : this) + { + if (component instanceof VariableValueTrigger) + variableCount++; + else if (component instanceof MapScreenLoadTrigger) + mapScreenLoadCount++; + } + + int length = mapScreenLoadCount * 5; + if (variableCount != 0) + length += 6 + variableCount * 6; + return length + 2; } @Override @@ -115,8 +148,8 @@ public BytesDataContainer save() MemBuf dataBuf = MemBuf.create(); MemBuf.MemBufWriter writer = dataBuf.writer(); - TreeSet tsMapScreenLoad = new TreeSet<>(); - TreeSet tsVariable = new TreeSet<>(); + List tsMapScreenLoad = new ArrayList<>(); + List tsVariable = new ArrayList<>(); if (!isEmpty()) { @@ -133,7 +166,7 @@ public BytesDataContainer save() for (LevelScriptTrigger lstm : tsMapScreenLoad) { writer.writeByte((byte) lstm.getTriggerType()); - writer.writeUInt32((byte) lstm.getScriptTriggered()); + writer.writeUInt32(lstm.getScriptTriggered()); } if (!tsVariable.isEmpty()) { diff --git a/src/main/java/io/github/turtleisaac/pokeditor/formats/scripts/antlr4/CommandDiscoverer.java b/src/main/java/io/github/turtleisaac/pokeditor/formats/scripts/antlr4/CommandDiscoverer.java index b6ef08e..5466f22 100644 --- a/src/main/java/io/github/turtleisaac/pokeditor/formats/scripts/antlr4/CommandDiscoverer.java +++ b/src/main/java/io/github/turtleisaac/pokeditor/formats/scripts/antlr4/CommandDiscoverer.java @@ -55,7 +55,7 @@ else if (child instanceof MacrosParser.Id_lineContext) macro.setId(child.accept(new CommandMacroVisitor<>() { @Override - protected Integer idLineAction(int idNumber) + protected Integer idLineAction(int idNumber, int dataType) { return idNumber; } diff --git a/src/main/java/io/github/turtleisaac/pokeditor/formats/scripts/antlr4/CommandMacro.java b/src/main/java/io/github/turtleisaac/pokeditor/formats/scripts/antlr4/CommandMacro.java index fb4b2e1..aaf30af 100644 --- a/src/main/java/io/github/turtleisaac/pokeditor/formats/scripts/antlr4/CommandMacro.java +++ b/src/main/java/io/github/turtleisaac/pokeditor/formats/scripts/antlr4/CommandMacro.java @@ -46,10 +46,24 @@ public void write(MemBuf memBuf, CommandWriter.LabelOffsetObtainer offsetObtaine if (parameters.length != 0) { + int providedCount = parameterValues == null ? 0 : parameterValues.length; + if (providedCount != parameters.length) + { + throw new RuntimeException(String.format("The command \"%s\" expects %d parameter(s) %s but %d were provided", name, parameters.length, Arrays.toString(parameters), providedCount)); + } + int idx = 0; for (String parameter : parameters) { Object param = parameterValues[idx++]; - if (param instanceof Number number) + if (param == null) + { + // A conditional/variable-length macro (.if-guarded, defaulted args) legitimately leaves + // some declared parameters unread, so a null here is not necessarily an error. The writer + // only visits a parameter when its branch is actually taken, and raises a precise error at + // that point if a genuinely-required parameter turns out to be missing. See CommandWriter. + continue; + } + else if (param instanceof Number number) parameterToValueMap.put(parameter, number); else if (param instanceof String str) { @@ -73,10 +87,18 @@ else if (param instanceof String str) throw new RuntimeException(String.format("An invalid parameter was provided (%s) in \"%s\"", str, this)); } } + else + { + throw new RuntimeException(String.format("The parameter \"%s\" of the command \"%s\" was provided a value of an unsupported type (%s): %s", parameter, name, param.getClass().getName(), param)); + } } } + else if (parameterValues != null && parameterValues.length != 0) + { + throw new RuntimeException(String.format("The command \"%s\" takes no parameters but %d were provided", name, parameterValues.length)); + } - CommandWriter commandWriter = new CommandWriter(memBuf.writer(), offsetObtainer, parameterToValueMap); + CommandWriter commandWriter = new CommandWriter(memBuf.writer(), offsetObtainer, parameterToValueMap, this); commandWriter.visitEntry(entryContext); } diff --git a/src/main/java/io/github/turtleisaac/pokeditor/formats/scripts/antlr4/CommandMacroVisitor.java b/src/main/java/io/github/turtleisaac/pokeditor/formats/scripts/antlr4/CommandMacroVisitor.java index b3bb04a..5675404 100644 --- a/src/main/java/io/github/turtleisaac/pokeditor/formats/scripts/antlr4/CommandMacroVisitor.java +++ b/src/main/java/io/github/turtleisaac/pokeditor/formats/scripts/antlr4/CommandMacroVisitor.java @@ -15,7 +15,7 @@ public abstract class CommandMacroVisitor extends MacrosBaseVisitor @Override public T visitId_line(MacrosParser.Id_lineContext ctx) { - boolean foundShort = false; + int idDataType = -1; boolean foundValue = false; int idNumber = -1; for (int i = 0; i < ctx.getChildCount(); i++) @@ -25,7 +25,7 @@ public T visitId_line(MacrosParser.Id_lineContext ctx) int type = ((TerminalNodeImpl) c).symbol.getType(); if (type == MacrosLexer.SHORT || type == MacrosLexer.WORD) { - foundShort = true; + idDataType = type; } else if (type == MacrosLexer.NUMBER) { foundValue = true; @@ -34,14 +34,14 @@ else if (type == MacrosLexer.NUMBER) { } } - if (foundShort && foundValue) { - return idLineAction(idNumber); + if (idDataType != -1 && foundValue) { + return idLineAction(idNumber, idDataType); } return null; } - protected T idLineAction(int idNumber) { + protected T idLineAction(int idNumber, int dataType) { return null; } diff --git a/src/main/java/io/github/turtleisaac/pokeditor/formats/scripts/antlr4/CommandReader.java b/src/main/java/io/github/turtleisaac/pokeditor/formats/scripts/antlr4/CommandReader.java index 902ff1c..6772ac0 100644 --- a/src/main/java/io/github/turtleisaac/pokeditor/formats/scripts/antlr4/CommandReader.java +++ b/src/main/java/io/github/turtleisaac/pokeditor/formats/scripts/antlr4/CommandReader.java @@ -120,7 +120,7 @@ public Integer visitTerminal(TerminalNode node) return (int) parameterToValueMap.get(terminalNode.getText().substring(1)); } } else if (terminalNode.symbol.getType() == MacrosLexer.NUMBER) { - return Integer.parseInt(terminalNode.getText()); + return Integer.decode(terminalNode.getText()); } else if (terminalNode.symbol.getType() == MacrosLexer.CURRENT_OFFSET) { return reader.getPosition() - 4; } diff --git a/src/main/java/io/github/turtleisaac/pokeditor/formats/scripts/antlr4/CommandWriter.java b/src/main/java/io/github/turtleisaac/pokeditor/formats/scripts/antlr4/CommandWriter.java index 5bbbfbe..13ad53c 100644 --- a/src/main/java/io/github/turtleisaac/pokeditor/formats/scripts/antlr4/CommandWriter.java +++ b/src/main/java/io/github/turtleisaac/pokeditor/formats/scripts/antlr4/CommandWriter.java @@ -27,17 +27,29 @@ public class CommandWriter extends CommandMacroVisitor private int parameterType; private boolean compareMode; - public CommandWriter(MemBuf.MemBufWriter writer, LabelOffsetObtainer offsetObtainer, Map parameterToValueMap) + public CommandWriter(MemBuf.MemBufWriter writer, LabelOffsetObtainer offsetObtainer, Map parameterToValueMap, CommandMacro macro) { this.writer = writer; this.parameterToValueMap = parameterToValueMap; this.offsetObtainer = offsetObtainer; + this.macro = macro; + } + + private String commandName() + { + return macro != null ? macro.getName() : "?"; } @Override - protected Integer idLineAction(int idNumber) + protected Integer idLineAction(int idNumber, int dataType) { - writer.writeShort((short) idNumber); + // the macro declares the width of its own id line - AI script macros use .word, field script + // macros use .short - so it has to be honoured rather than assumed to be a short + switch (dataType) { + case MacrosLexer.SHORT -> writer.writeShort((short) idNumber); + case MacrosLexer.WORD -> writer.writeInt(idNumber); + default -> throw new IllegalStateException("Unexpected command id width: " + dataType); + } return null; } @@ -67,6 +79,12 @@ public Integer visitTerminal(TerminalNode node) if (!compareMode) { // writing arg value to file Object value = parameterToValueMap.get(text); + if (value == null) + { + // This parameter is actually being emitted but the reader never gave it a value - + // a genuinely required parameter is missing (as opposed to one a .if branch skips). + throw new RuntimeException(String.format("The parameter \"%s\" of the command \"%s\" was not provided a value", text, commandName())); + } int valueToWrite; if (value instanceof Number number) @@ -84,10 +102,15 @@ public Integer visitTerminal(TerminalNode node) } else // this is a calculation which requires an already read value { - return (int) parameterToValueMap.get(text); + Object value = parameterToValueMap.get(text); + if (value == null) + { + throw new RuntimeException(String.format("The parameter \"%s\" of the command \"%s\" was used in a condition but not provided a value", text, commandName())); + } + return (int) value; } } else if (terminalNode.symbol.getType() == MacrosLexer.NUMBER) { - return Integer.parseInt(terminalNode.getText()); + return Integer.decode(terminalNode.getText()); } else if (terminalNode.symbol.getType() == MacrosLexer.CURRENT_OFFSET) { return writer.getPosition(); } diff --git a/src/main/java/io/github/turtleisaac/pokeditor/formats/scripts/antlr4/ScriptDataProducer.java b/src/main/java/io/github/turtleisaac/pokeditor/formats/scripts/antlr4/ScriptDataProducer.java index 0dd911f..d65025f 100644 --- a/src/main/java/io/github/turtleisaac/pokeditor/formats/scripts/antlr4/ScriptDataProducer.java +++ b/src/main/java/io/github/turtleisaac/pokeditor/formats/scripts/antlr4/ScriptDataProducer.java @@ -263,8 +263,10 @@ private Object visitCommandParameterHelper(ScriptFileParser.ParameterContext ctx if (type == ScriptFileLexer.NUMBER) { + // negative word parameters are emitted as their unsigned hexadecimal representation, + // which does not fit in a signed parse if (text.contains("0x")) - return Integer.parseInt(text.substring(2), 16); + return Integer.parseUnsignedInt(text.substring(2), 16); else return Integer.parseInt(text); } @@ -273,7 +275,7 @@ else if (type == ScriptFileLexer.NAME) if (text.startsWith("0x")) { try { - return Integer.parseInt(text.substring(2), 16); + return Integer.parseUnsignedInt(text.substring(2), 16); } catch(NumberFormatException ignored) {} } return text; @@ -291,6 +293,66 @@ else if (child instanceof ScriptFileParser.LabelContext labelContext) @Override public Void visitAction_command(ScriptFileParser.Action_commandContext ctx) { + List parameters = new ArrayList<>(); + + for (ParseTree child : ctx.children) + { + if (child instanceof ScriptFileParser.Action_parametersContext actionParametersContext) + { + if (actionParametersContext.children == null) + continue; + + for (ParseTree parametersChild : actionParametersContext.children) + { + if (parametersChild instanceof ScriptFileParser.Action_parameterContext parameterContext) + parameters.add(parameterContext.getText().trim()); + } + } + } + + if (parameters.size() != 2) + { + scriptCompilationException.addSuppressed(new ScriptCompilationException(String.format("An action requires a movement and a duration, but %d parameter(s) were provided: \"%s\"", parameters.size(), ctx.getText().trim()))); + return null; + } + + String movement = parameters.get(0); + + Integer id = null; + String name = null; + for (Map.Entry entry : FieldScriptParser.movementNames.entrySet()) + { + if (entry.getValue().equalsIgnoreCase(movement)) + { + id = entry.getKey(); + name = entry.getValue(); + break; + } + } + + if (id == null) + { + // Movements_Hg.json does not name every opcode, so a movement may legitimately be written as a number + try { + id = Integer.decode(movement); + } + catch (NumberFormatException e) { + scriptCompilationException.addSuppressed(new ScriptCompilationException(String.format("\"%s\" is not a valid movement name or id", movement))); + return null; + } + } + + int parameter; + try { + parameter = Integer.decode(parameters.get(1)); + } + catch (NumberFormatException e) { + scriptCompilationException.addSuppressed(new ScriptCompilationException(String.format("\"%s\" is not a valid duration for the movement \"%s\"", parameters.get(1), movement))); + return null; + } + + data.add(name != null ? new FieldScriptData.ActionCommand(name, id, parameter) : new FieldScriptData.ActionCommand(id, parameter)); + return super.visitAction_command(ctx); } diff --git a/src/main/java/io/github/turtleisaac/pokeditor/formats/text/TextBankData.java b/src/main/java/io/github/turtleisaac/pokeditor/formats/text/TextBankData.java index 786ff8c..d33068f 100644 --- a/src/main/java/io/github/turtleisaac/pokeditor/formats/text/TextBankData.java +++ b/src/main/java/io/github/turtleisaac/pokeditor/formats/text/TextBankData.java @@ -42,12 +42,44 @@ public class TextBankData extends ArrayList implements Gen private int seed; + private int bankIndex = -1; + public TextBankData(BytesDataContainer files) { super(); setData(files); } + public TextBankData(BytesDataContainer files, int bankIndex) + { + super(); + this.bankIndex = bankIndex; + setData(files); + } + + /** + * Gets the index of this text bank within the text narc, or -1 if it is not known + * @return an int + */ + public int getBankIndex() + { + return bankIndex; + } + + /** + * Sets the index of this text bank within the text narc (used purely for error reporting) + * @param bankIndex an int + */ + public void setBankIndex(int bankIndex) + { + this.bankIndex = bankIndex; + } + + private String describe(int messageIdx) + { + return "text bank " + (bankIndex >= 0 ? String.valueOf(bankIndex) : "?") + ", message " + messageIdx; + } + @Override public void setData(BytesDataContainer files) { @@ -56,7 +88,8 @@ public void setData(BytesDataContainer files) throw new RuntimeException("Text file not provided to editor"); } - MemBuf dataBuf = MemBuf.create(files.get(GameFiles.TEXT, null)); + byte[] file = files.get(GameFiles.TEXT, null); + MemBuf dataBuf = MemBuf.create(file); MemBuf.MemBufReader reader = dataBuf.reader(); int numEntries = reader.readUInt16(); @@ -73,6 +106,19 @@ public void setData(BytesDataContainer files) key = ((seed * (i + 1) * 0x2fd) & 0xffff) | ((seed * (i + 1) * 0x2fd0000) & 0xffff0000); offsets[i] = reader.readInt() ^ key; sizes[i] = reader.readInt() ^ key; + + if (sizes[i] < 0) + { + throw new TextEncodingException(describe(i) + " declares a negative length (" + sizes[i] + ")"); + } + if (offsets[i] < 0 || offsets[i] > file.length) + { + throw new TextEncodingException(describe(i) + " declares an out of bounds offset (" + offsets[i] + ", file is " + file.length + " bytes)"); + } + if (offsets[i] + sizes[i] * 2L > file.length) + { + throw new TextEncodingException(describe(i) + " runs past the end of the file (offset " + offsets[i] + ", length " + sizes[i] + " halfwords, file is " + file.length + " bytes)"); + } } @@ -108,13 +154,26 @@ public void setData(BytesDataContainer files) else if (c == 0xfffe) { // if the character is 0xfffe, then it's a special VAR case + int[] binaryString = binaryStrings[messageIdx]; + + if (j + 2 >= binaryString.length) + { + throw new TextEncodingException(describe(messageIdx) + " contains a truncated VAR() - the variable id and argument count run past the end of the message"); + } + text.append("VAR("); StringBuilder args = new StringBuilder(); - args.append(binaryStrings[messageIdx][++j]); - int argNum = binaryStrings[messageIdx][++j]; + args.append(binaryString[++j]); + int argNum = binaryString[++j]; + + if (argNum < 0 || j + argNum >= binaryString.length) + { + throw new TextEncodingException(describe(messageIdx) + " contains a VAR() declaring " + argNum + " argument(s), which runs past the end of the message"); + } + for (int k = 0; k < argNum; k++) { args.append(", "); - args.append(binaryStrings[messageIdx][++j]); + args.append(binaryString[++j]); } args.append(")"); text.append(args); @@ -145,8 +204,9 @@ public BytesDataContainer save() ArrayList> binaryStrings = new ArrayList<>(); // encode text and write it - for (Message msg : this) + for (int messageIdx = 0; messageIdx < size(); messageIdx++) { + Message msg = get(messageIdx); String message = msg.text; tmpBinaryString = new ArrayList<>(); @@ -156,13 +216,29 @@ public BytesDataContainer save() String sub = message.substring(j, Math.min(j + 4, message.length())); if (message.charAt(j) == '\\') { + if (j + 1 >= message.length()) + { + throw new TextEncodingException(describe(messageIdx) + " ends with a lone '\\' escape character"); + } + switch (message.charAt(j + 1)) { case 'r' -> tmpBinaryString.add(0x25bc); case 'n' -> tmpBinaryString.add(0xe000); case 'f' -> tmpBinaryString.add(0x25bd); default -> { - tmpBinaryString.add(Integer.parseInt(message.substring(j + 2, j + 6), 16)); + if (j + 6 > message.length()) + { + throw new TextEncodingException(describe(messageIdx) + " contains a truncated escape sequence \"" + message.substring(j) + "\" - four hexadecimal digits are required"); + } + + String hex = message.substring(j + 2, j + 6); + try { + tmpBinaryString.add(Integer.parseInt(hex, 16)); + } + catch (NumberFormatException e) { + throw new TextEncodingException(describe(messageIdx) + " contains a malformed escape sequence \"" + message.substring(j, j + 6) + "\"", e); + } j += 4; } } @@ -172,38 +248,47 @@ else if (sub.equals("VAR(")) { int endOfVar = message.indexOf(')', j); if (endOfVar == -1) { - throw new RuntimeException("Could not find end of VAR()"); + throw new TextEncodingException(describe(messageIdx) + ": could not find end of VAR()"); } String[] args = message.substring(j + 4, endOfVar).split(","); tmpBinaryString.add(0xfffe); - tmpBinaryString.add(Integer.parseInt(args[0].trim())); - tmpBinaryString.add(args.length - 1); + try { + tmpBinaryString.add(Integer.parseInt(args[0].trim())); + tmpBinaryString.add(args.length - 1); - for (int x = 1; x < args.length; x++) - { - tmpBinaryString.add(Integer.parseInt(args[x].trim())); + for (int x = 1; x < args.length; x++) + { + tmpBinaryString.add(Integer.parseInt(args[x].trim())); + } + } + catch (NumberFormatException e) { + throw new TextEncodingException(describe(messageIdx) + " contains a VAR() with a non-numeric argument: \"" + message.substring(j, endOfVar + 1) + "\"", e); } j = endOfVar; } else { - int val = 0; - try { - val = characters.get("getInt").get(String.valueOf(message.charAt(j))).asInt(); - } - catch(NullPointerException e) { - e.printStackTrace(); + char c = message.charAt(j); + JsonNode node = characters.get("getInt").get(String.valueOf(c)); + if (node == null) + { + throw new TextEncodingException(String.format("%s contains the character '%c' (U+%04X) at index %d, which has no Pokemon Gen 4 character encoding", describe(messageIdx), c, (int) c, j)); } - tmpBinaryString.add(val); + tmpBinaryString.add(node.asInt()); } } if (msg.compressed) { - binaryStrings.add(compress(tmpBinaryString.toArray(Integer[]::new))); + try { + binaryStrings.add(compress(tmpBinaryString.toArray(Integer[]::new))); + } + catch (RuntimeException e) { + throw new TextEncodingException(describe(messageIdx) + " is marked as 9-bit compressed but " + e.getMessage(), e); + } } else { @@ -366,9 +451,12 @@ protected static ArrayList compress(Integer[] uncompressedString) throw for (int c : uncompressedString) { - if ( (c >> 9) == 1) + // only 9 bits are available per character, and all-ones (0x1ff) is reserved as the terminator + // by decompress(), so anything outside 0x000-0x1fe would silently overwrite the characters + // which follow it + if (c < 0 || c >= 0x1ff) { - throw new RuntimeException(String.format("%04x cannot be compressed", c)); + throw new RuntimeException(String.format("character %04x cannot be represented in the 9-bit compressed encoding", c)); } container |= c << bitshift; @@ -418,6 +506,23 @@ public List getStringList() return output; } + /** + * Thrown when a text bank cannot be decoded from or encoded to the Pokemon Gen 4 text format. + * The message names the offending bank and message so the caller can report it to the user. + */ + public static class TextEncodingException extends RuntimeException + { + public TextEncodingException(String message) + { + super(message); + } + + public TextEncodingException(String message, Throwable cause) + { + super(message, cause); + } + } + public static class Message { String text; boolean compressed; diff --git a/src/main/java/io/github/turtleisaac/pokeditor/formats/text/TextBankParser.java b/src/main/java/io/github/turtleisaac/pokeditor/formats/text/TextBankParser.java index df8a3de..2310778 100644 --- a/src/main/java/io/github/turtleisaac/pokeditor/formats/text/TextBankParser.java +++ b/src/main/java/io/github/turtleisaac/pokeditor/formats/text/TextBankParser.java @@ -46,9 +46,10 @@ public List generateDataList(Map narcs, Map data = new ArrayList<>(); - for (byte[] subfile : personal.getFiles()) + List subfiles = personal.getFiles(); + for (int idx = 0; idx < subfiles.size(); idx++) { - data.add(new TextBankData(new BytesDataContainer(GameFiles.TEXT, null, subfile))); + data.add(new TextBankData(new BytesDataContainer(GameFiles.TEXT, null, subfiles.get(idx)), idx)); } return data; diff --git a/src/main/java/io/github/turtleisaac/pokeditor/formats/trainers/TrainerData.java b/src/main/java/io/github/turtleisaac/pokeditor/formats/trainers/TrainerData.java index 6c96c4b..b4e0242 100644 --- a/src/main/java/io/github/turtleisaac/pokeditor/formats/trainers/TrainerData.java +++ b/src/main/java/io/github/turtleisaac/pokeditor/formats/trainers/TrainerData.java @@ -41,6 +41,8 @@ public class TrainerData implements GenericFileData int[] items; boolean[] ai; + int unknownFlagBits; + long unknownAiBits; int battleType2; short unknown1; short unknown2; @@ -82,6 +84,7 @@ public void setData(BytesDataContainer files) int flag = reader.readUInt8(); movesEnabled = (flag & 1) == 1; itemEnabled = ((flag >> 1) & 1) == 1; + unknownFlagBits = flag & ~0b11; trainerClass = reader.readUInt8(); battleType = reader.readUInt8(); @@ -101,6 +104,7 @@ public void setData(BytesDataContainer files) { ai[i] = ((aiComposite >> i) & 1) == 1; } + unknownAiBits = aiComposite & ~((1L << NUMBER_AI_FLAGS) - 1) & 0xffffffffL; battleType2 = reader.readUInt8(); unknown1 = reader.readUInt8(); @@ -121,7 +125,7 @@ public void setData(BytesDataContainer files) int combinedMon = reader.readUInt16(); entry.setSpecies(combinedMon & 0x3ff); - entry.setAltForm(combinedMon >> 10); + entry.setAltForm((combinedMon >> 10) & 0x3f); if (itemEnabled) { @@ -144,7 +148,19 @@ public void setData(BytesDataContainer files) public void setTeamFromSmogon(String text, BiFunction stringReplacementFunction) { - trainerPartyEntries = SmogonTeamImporter.importSmogonTeam(text, stringReplacementFunction); + ArrayList imported = SmogonTeamImporter.importSmogonTeam(text, stringReplacementFunction); + + if (imported.size() > MAX_NUMBER_TRAINER_MONS) + { + throw new SmogonTeamImporter.SmogonImportException(String.format("A trainer can have at most %d Pokemon, but the provided team contains %d", MAX_NUMBER_TRAINER_MONS, imported.size())); + } + + if (imported.size() < MIN_NUMBER_TRAINER_MONS) + { + throw new SmogonTeamImporter.SmogonImportException(String.format("A trainer must have at least %d Pokemon, but the provided team contains %d", MIN_NUMBER_TRAINER_MONS, imported.size())); + } + + trainerPartyEntries = imported; } @Override @@ -154,7 +170,7 @@ public BytesDataContainer save() MemBuf.MemBufWriter writer = dataBuf.writer(); // trdata - int compositeFlags = (movesEnabled ? 1 : 0) | (itemEnabled ? 0b10 : 0); + int compositeFlags = (movesEnabled ? 1 : 0) | (itemEnabled ? 0b10 : 0) | (unknownFlagBits & ~0b11); writer.writeBytes(compositeFlags, trainerClass, battleType, trainerPartyEntries.size()); for (int i = 0; i < NUMBER_TRAINER_ITEMS; i++) { @@ -166,6 +182,7 @@ public BytesDataContainer save() { compositeAiFlags |= ((ai[i] ? 1 : 0) << i); } + compositeAiFlags |= (int) unknownAiBits; writer.writeInt(compositeAiFlags); writer.writeBytes(battleType2, unknown1, unknown2, unknown3); @@ -179,7 +196,7 @@ public BytesDataContainer save() { writer.writeBytes(entry.getDifficultyValue(), entry.getAbility()); writer.writeShort((short) entry.getLevel()); - writer.writeShort((short)( ( (entry.getAltForm() & 0x7) << 10) | (entry.getSpecies() & 0x3ff) ) ); + writer.writeShort((short)( ( (entry.getAltForm() & 0x3f) << 10) | (entry.getSpecies() & 0x3ff) ) ); if (itemEnabled) { @@ -388,8 +405,8 @@ public String toSmogonString(BiFunction importSmogonTeam(String t { SmogonTeamImporter importer = new SmogonTeamImporter(stringReplacementFunction); - SmogonTeamLexer lexer = new SmogonTeamLexer(CharStreams.fromString(text)); + // the grammar requires each species entry to start on a fresh line and the paste to end with one + String normalized = text; + if (!normalized.startsWith("\n") && !normalized.startsWith("\r")) + normalized = "\n" + normalized; + if (!normalized.endsWith("\n")) + normalized = normalized + "\n"; + + SmogonTeamLexer lexer = new SmogonTeamLexer(CharStreams.fromString(normalized)); CommonTokenStream tokens = new CommonTokenStream(lexer); SmogonTeamParser parser = new SmogonTeamParser(tokens); - importer.visitTeam(parser.team()); + SmogonTeamParser.TeamContext team = parser.team(); + + int syntaxErrors = parser.getNumberOfSyntaxErrors(); + if (syntaxErrors != 0) + { + throw new SmogonImportException(String.format("The provided team could not be understood - %d syntax error(s) were found in it", syntaxErrors)); + } + + importer.visitTeam(team); + + if (importer.trainerPartyEntries.size() > TrainerData.MAX_NUMBER_TRAINER_MONS) + { + throw new SmogonImportException(String.format("A trainer can have at most %d Pokemon, but the provided team contains %d", TrainerData.MAX_NUMBER_TRAINER_MONS, importer.trainerPartyEntries.size())); + } return importer.trainerPartyEntries; } @@ -124,6 +144,34 @@ public Void visitEffortValues(SmogonTeamParser.EffortValuesContext ctx) return super.visitEffortValues(ctx); } + @Override + public Void visitIndividualValues(SmogonTeamParser.IndividualValuesContext ctx) + { + // the gen 4 trainer format has a single "difficulty" byte rather than per-stat IVs, and the exporter + // writes the same value out for every stat, so the first entry is the one which matters + for (ParseTree child : ctx.children) + { + if (child instanceof SmogonTeamParser.EffortValueEntryContext entryContext) + { + for (ParseTree entryChild : entryContext.children) + { + if (entryChild instanceof TerminalNodeImpl terminalNode && terminalNode.symbol.getType() == SmogonTeamLexer.NUMBER) + { + int iv = Integer.parseInt(terminalNode.getText()); + if (iv < 0 || iv > MAX_IV) + { + throw new SmogonImportException("An IV must be between 0 and " + MAX_IV + ". Provided: " + iv); + } + current.setDifficultyValue(iv * MAX_DIFFICULTY_VALUE / MAX_IV); + return null; + } + } + } + } + + return null; + } + @Override public Void visitNature(SmogonTeamParser.NatureContext ctx) { @@ -151,6 +199,27 @@ public Void visitMove(SmogonTeamParser.MoveContext ctx) return null; } + /** + * The largest value an individual value can hold + */ + public static final int MAX_IV = 31; + + /** + * The largest value a trainer party entry's difficulty byte can hold + */ + public static final int MAX_DIFFICULTY_VALUE = 255; + + /** + * Thrown when a pasted Smogon team cannot be turned into a trainer's party + */ + public static class SmogonImportException extends RuntimeException + { + public SmogonImportException(String message) + { + super(message); + } + } + public enum SmogonStringSources { SPECIES, diff --git a/src/main/java/io/github/turtleisaac/pokeditor/gamedata/Game.java b/src/main/java/io/github/turtleisaac/pokeditor/gamedata/Game.java index 8535f17..3daa42e 100755 --- a/src/main/java/io/github/turtleisaac/pokeditor/gamedata/Game.java +++ b/src/main/java/io/github/turtleisaac/pokeditor/gamedata/Game.java @@ -24,7 +24,6 @@ public enum Game public final String[] sheetList; public final String[] editorList; - private Region region; Game(String[] sheetList, String[] editorList) { @@ -32,12 +31,16 @@ public enum Game this.editorList= editorList; } - public Region getRegion() - { - return region; - } - public static Game parseBaseRom(String baseRomGameCode) + /** + * The game and region identified by a base ROM's game code + * + * @param game a Game + * @param region a Region + */ + public record BaseRomInfo(Game game, Region region) {} + + public static BaseRomInfo parseBaseRom(String baseRomGameCode) { Game game = switch (baseRomGameCode.substring(0, 3)) { case "ADA" -> Game.Diamond; @@ -48,8 +51,9 @@ public static Game parseBaseRom(String baseRomGameCode) default -> throw new RuntimeException("Invalid game"); }; - game.region = Region.getRegion(baseRomGameCode.charAt(3)); - return game; + // the region travels with the result rather than being stashed anywhere: a field on the + // enum, static or per-constant, is shared by every ROM opened in the process + return new BaseRomInfo(game, Region.getRegion(baseRomGameCode.charAt(3))); } public enum Region @@ -63,7 +67,7 @@ public enum Region EUROPE, SPAIN; - static Region getRegion(char c) + public static Region getRegion(char c) { return switch (c) { diff --git a/src/main/java/io/github/turtleisaac/pokeditor/gamedata/GameCodeBinaries.java b/src/main/java/io/github/turtleisaac/pokeditor/gamedata/GameCodeBinaries.java index c1541c4..20b5edb 100644 --- a/src/main/java/io/github/turtleisaac/pokeditor/gamedata/GameCodeBinaries.java +++ b/src/main/java/io/github/turtleisaac/pokeditor/gamedata/GameCodeBinaries.java @@ -23,6 +23,7 @@ public static void initialize(Game baseROM) case HeartGold, SoulSilver -> { BATTLE.id = 12; } + default -> throw new UnsupportedOperationException("Unsupported base ROM: " + baseROM); } } diff --git a/src/main/java/io/github/turtleisaac/pokeditor/gamedata/GameFiles.java b/src/main/java/io/github/turtleisaac/pokeditor/gamedata/GameFiles.java index fcb0d39..d9cd01a 100644 --- a/src/main/java/io/github/turtleisaac/pokeditor/gamedata/GameFiles.java +++ b/src/main/java/io/github/turtleisaac/pokeditor/gamedata/GameFiles.java @@ -100,6 +100,7 @@ public static void initialize(Game baseROM) FIELD_SCRIPTS.path = "a/0/1/2"; TRAINER_AI_SCRIPTS.path = "a/0/9/9"; } + default -> throw new UnsupportedOperationException("Unsupported base ROM: " + baseROM); } } } diff --git a/src/main/java/io/github/turtleisaac/pokeditor/gamedata/Tables.java b/src/main/java/io/github/turtleisaac/pokeditor/gamedata/Tables.java index d3546f9..3e61eff 100644 --- a/src/main/java/io/github/turtleisaac/pokeditor/gamedata/Tables.java +++ b/src/main/java/io/github/turtleisaac/pokeditor/gamedata/Tables.java @@ -1,5 +1,7 @@ package io.github.turtleisaac.pokeditor.gamedata; +import java.util.Objects; + public enum Tables { PARTY_ICON_PALETTE, @@ -9,21 +11,43 @@ public enum Tables TM_HM_MOVES ; + private static final int UNSET_POINTER_OFFSET = -1; + private GameCodeBinaries pointerLocation; - private int pointerOffset; + private int pointerOffset = UNSET_POINTER_OFFSET; public GameCodeBinaries getPointerLocation() { + if (pointerLocation == null) + { + throw new IllegalStateException("The code binary containing the " + name() + " table is not known for the base ROM which is currently loaded"); + } return pointerLocation; } public int getPointerOffset() { + if (pointerOffset == UNSET_POINTER_OFFSET) + { + throw new IllegalStateException("The offset of the " + name() + " table is not known for the base ROM which is currently loaded"); + } return pointerOffset; } - public static void initialize(Game baseROM) + + public static void initialize(Game baseROM, Game.Region region) { + Objects.requireNonNull(baseROM, "A base ROM must be provided in order to initialize the table pointers"); + Objects.requireNonNull(region, "The region of the base ROM must be provided in order to initialize the table pointers - it comes from Game.parseBaseRom()"); + + // every constant is reset first, otherwise switching between ROMs leaves the previous ROM's + // pointers in place for any table the new ROM does not assign + for (Tables table : values()) + { + table.pointerLocation = null; + table.pointerOffset = UNSET_POINTER_OFFSET; + } + switch (baseROM) { case Platinum -> { PARTY_ICON_PALETTE.pointerLocation = GameCodeBinaries.ARM9; @@ -31,7 +55,7 @@ public static void initialize(Game baseROM) TRAINER_CLASS_PRIZE_MONEY.pointerLocation = GameCodeBinaries.BATTLE; TM_HM_MOVES.pointerLocation = GameCodeBinaries.ARM9; ITEMS.pointerLocation = GameCodeBinaries.ARM9; - switch (baseROM.getRegion()) { + switch (region) { case USA -> { PARTY_ICON_PALETTE.pointerOffset = 0x079f80; TRAINER_CLASS_PRIZE_MONEY.pointerOffset = 0x816c; @@ -88,7 +112,7 @@ public static void initialize(Game baseROM) TRAINER_CLASS_GENDER.pointerLocation = GameCodeBinaries.ARM9; TM_HM_MOVES.pointerLocation = GameCodeBinaries.ARM9; ITEMS.pointerLocation = GameCodeBinaries.ARM9; - switch (baseROM.getRegion()) { + switch (region) { case USA -> { PARTY_ICON_PALETTE.pointerOffset = 0x074408; TRAINER_CLASS_GENDER.pointerOffset = 0x073600; @@ -126,7 +150,7 @@ public static void initialize(Game baseROM) TRAINER_CLASS_GENDER.pointerLocation = GameCodeBinaries.ARM9; TM_HM_MOVES.pointerLocation = GameCodeBinaries.ARM9; ITEMS.pointerLocation = GameCodeBinaries.ARM9; - switch (baseROM.getRegion()) { + switch (region) { case USA -> { PARTY_ICON_PALETTE.pointerOffset = 0x074408; TRAINER_CLASS_GENDER.pointerOffset = 0x073600; @@ -177,6 +201,7 @@ public static void initialize(Game baseROM) } } } + default -> throw new UnsupportedOperationException("Unsupported base ROM: " + baseROM); } } } diff --git a/src/main/java/io/github/turtleisaac/pokeditor/gamedata/TextFiles.java b/src/main/java/io/github/turtleisaac/pokeditor/gamedata/TextFiles.java index f0bfb11..794b85f 100644 --- a/src/main/java/io/github/turtleisaac/pokeditor/gamedata/TextFiles.java +++ b/src/main/java/io/github/turtleisaac/pokeditor/gamedata/TextFiles.java @@ -13,9 +13,19 @@ public enum TextFiles TRAINER_TEXT, TYPE_NAMES; + // 0 is a valid bank index, so an unknown bank has to be represented by something which isn't + static final int UNKNOWN_BANK = -1; + private int value; - public int getValue() {return value;} + public int getValue() + { + if (value == UNKNOWN_BANK) + { + throw new IllegalStateException("The " + name() + " text bank index is not known for the base ROM which is currently loaded"); + } + return value; + } public static void initialize(Game baseROM) { @@ -50,6 +60,7 @@ public static void initialize(Game baseROM) TRAINER_TEXT.value = HGSS_TRAINER_TEXT.value; TYPE_NAMES.value = HGSS_TYPE_NAMES.value; } + default -> throw new UnsupportedOperationException("Unsupported base ROM: " + baseROM); } } @@ -83,11 +94,11 @@ enum SpecificTextBanks PLAT_TRAINER_CLASS_NAMES(619), HGSS_TRAINER_CLASS_NAMES(730), - DP_TRAINER_TEXT(0), //TODO change + DP_TRAINER_TEXT(TextFiles.UNKNOWN_BANK), //TODO find the real bank index PLAT_TRAINER_TEXT(617), //TODO change HGSS_TRAINER_TEXT(728), - DP_TYPE_NAMES(0), //TODO change + DP_TYPE_NAMES(TextFiles.UNKNOWN_BANK), //TODO find the real bank index PLAT_TYPE_NAMES(624), HGSS_TYPE_NAMES(735); diff --git a/src/main/resources/data/characters.json b/src/main/resources/data/characters.json index 2abad1c..bdf2cf2 100755 --- a/src/main/resources/data/characters.json +++ b/src/main/resources/data/characters.json @@ -244,7 +244,7 @@ "242": "⊗", "243": "⊘", "244": "＝", - "245": "ｚ", + "245": "～", "246": "：", "247": "；", "248": "．", @@ -3099,6 +3099,7 @@ "ｘ": "221", "ｙ": "222", "ｚ": "223", + "～": "245", "！": "225", "？": "226", "、": "227", diff --git a/src/test/java/io/github/turtleisaac/pokeditor/formats/FieldWidthTest.java b/src/test/java/io/github/turtleisaac/pokeditor/formats/FieldWidthTest.java new file mode 100644 index 0000000..e52c093 --- /dev/null +++ b/src/test/java/io/github/turtleisaac/pokeditor/formats/FieldWidthTest.java @@ -0,0 +1,247 @@ +package io.github.turtleisaac.pokeditor.formats; + +import io.github.turtleisaac.pokeditor.formats.evolutions.EvolutionData; +import io.github.turtleisaac.pokeditor.formats.learnsets.LearnsetData; +import io.github.turtleisaac.pokeditor.gamedata.GameFiles; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatCode; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +/** + * A field that does not fit its storage must fail loudly rather than be narrowed into a + * different value. + *

+ * The bounds asserted here are derived from the file layout, not from what the code + * currently does: a level occupies 7 bits and a move ID 9 bits of one shared 16-bit word, + * so the representable values are exactly 0..127 and 0..511. Anything outside that cannot + * be stored, and storing it anyway is data loss - move 512 became move 0, which reads back + * as "this Pokemon learns nothing at this level". + */ +class FieldWidthTest +{ + @Nested + @DisplayName("the width check itself") + class Widths + { + @Test + void everyValueThatFitsIsReturnedUnchanged() + { + // the check is the identity on its whole domain - it must not clamp, wrap or + // otherwise alter a value that was always fine + for (int i = 0; i <= 0xFF; i++) + assertThat(FieldWidth.u8(i, "f")).isEqualTo(i); + for (int i = 0; i <= 0xFFFF; i++) + assertThat(FieldWidth.u16(i, "f")).isEqualTo(i); + for (int i = Byte.MIN_VALUE; i <= Byte.MAX_VALUE; i++) + assertThat(FieldWidth.s8(i, "f")).isEqualTo(i); + } + + @Test + void theBoundariesThemselvesFit() + { + // an off-by-one in a bounds check shows up first at the bounds + assertThatCode(() -> { + FieldWidth.u8(0, "f"); + FieldWidth.u8(255, "f"); + FieldWidth.u16(0, "f"); + FieldWidth.u16(65535, "f"); + FieldWidth.s8(-128, "f"); + FieldWidth.s8(127, "f"); + FieldWidth.bits(0, 7, "f"); + FieldWidth.bits(127, 7, "f"); + FieldWidth.bits(511, 9, "f"); + }).doesNotThrowAnyException(); + } + + @Test + void oneStepPastEachBoundaryIsRejected() + { + assertThatThrownBy(() -> FieldWidth.u8(256, "f")).isInstanceOf(IllegalArgumentException.class); + assertThatThrownBy(() -> FieldWidth.u8(-1, "f")).isInstanceOf(IllegalArgumentException.class); + assertThatThrownBy(() -> FieldWidth.u16(65536, "f")).isInstanceOf(IllegalArgumentException.class); + assertThatThrownBy(() -> FieldWidth.u16(-1, "f")).isInstanceOf(IllegalArgumentException.class); + assertThatThrownBy(() -> FieldWidth.s8(128, "f")).isInstanceOf(IllegalArgumentException.class); + assertThatThrownBy(() -> FieldWidth.s8(-129, "f")).isInstanceOf(IllegalArgumentException.class); + assertThatThrownBy(() -> FieldWidth.bits(128, 7, "f")).isInstanceOf(IllegalArgumentException.class); + assertThatThrownBy(() -> FieldWidth.bits(512, 9, "f")).isInstanceOf(IllegalArgumentException.class); + } + + @Test + void theFailureNamesTheFieldTheValueAndTheLimit() + { + // a message that omits any of the three cannot be acted on: the user needs to know + // which cell, what they typed, and what would have been acceptable + assertThatThrownBy(() -> FieldWidth.bits(512, 9, "Move ID")) + .hasMessageContaining("Move ID") + .hasMessageContaining("512") + .hasMessageContaining("511"); + } + } + + @Nested + @DisplayName("expanded files") + class Expanded + { + /** + * Users of this tool routinely open ROMs with tables larger than retail. setData reads + * every entry a file contains, on purpose, so save has to be able to write them back. + * A cap fixed at the retail count parses such a file cleanly and then aborts the whole + * NARC write, which loses the edit and every other file in it. + */ + private EvolutionData evolutionsFromFileOf(int entries) + { + byte[] file = new byte[entries * EvolutionData.ENTRY_SIZE + EvolutionData.TERMINATOR_SIZE]; + return new EvolutionData(new BytesDataContainer(GameFiles.EVOLUTIONS, null, file)); + } + + @Test + void aRetailSizedEvolutionFileRoundTrips() + { + EvolutionData data = evolutionsFromFileOf(EvolutionData.MAX_NUM_ENTRIES); + assertThat(data).hasSize(EvolutionData.MAX_NUM_ENTRIES); + assertThatCode(data::save).doesNotThrowAnyException(); + } + + @Test + void anExpandedEvolutionFileSavesEveryEntryItWasReadWith() + { + // the property: whatever setData accepted, save must be able to write. Anything else + // means the file could be opened but not closed again. + for (int entries : new int[] {8, 10, 20, 50}) + { + EvolutionData data = evolutionsFromFileOf(entries); + assertThat(data).as("a file sized for %d entries", entries).hasSize(entries); + assertThatCode(data::save) + .as("a file sized for %d entries must save the %d entries it was read with", + entries, entries) + .doesNotThrowAnyException(); + } + } + + @Test + void addingMoreEntriesThanTheFileHoldsIsStillRefused() + { + // the cap is not gone, it is derived: a retail file still cannot take an eighth + // evolution, and the message says how many it holds rather than quoting a constant + EvolutionData data = evolutionsFromFileOf(EvolutionData.MAX_NUM_ENTRIES); + data.add(new EvolutionData.EvolutionEntry(1, 2, 3)); + + assertThatThrownBy(data::save) + .isInstanceOf(RuntimeException.class) + .hasMessageContaining(String.valueOf(EvolutionData.MAX_NUM_ENTRIES)); + } + + @Test + void anExpandedFileRefusesOnlyPastItsOwnCapacity() + { + EvolutionData data = evolutionsFromFileOf(20); + data.add(new EvolutionData.EvolutionEntry(1, 2, 3)); + + assertThatThrownBy(data::save).isInstanceOf(RuntimeException.class); + } + } + + @Nested + @DisplayName("learnset packing") + class LearnsetPacking + { + /** an empty level-up learnset: just the 0xFFFF terminator and its padding */ + private LearnsetData empty() + { + return new LearnsetData(new BytesDataContainer(GameFiles.LEVEL_UP_LEARNSETS, null, + new byte[] {(byte) 0xFF, (byte) 0xFF, 0, 0})); + } + + private LearnsetData withOneEntry(int moveID, int level) + { + LearnsetData data = empty(); + data.add(new LearnsetData.LearnsetEntry(moveID, level)); + return data; + } + + @Test + void anEntryThatFitsSurvivesSaveAndReload() + { + // the property that matters: what was set is what comes back. asserted at the + // extremes of both fields, where a mask would bite first + // (511, 127) is deliberately absent: it is individually in range but packs to + // 0xFFFF, the terminator, and is rejected. It is covered by its own test below. + for (int[] pair : new int[][] {{0, 0}, {511, 126}, {510, 127}, {1, 1}, {350, 100}}) + { + LearnsetData saved = withOneEntry(pair[0], pair[1]); + LearnsetData reloaded = empty(); + reloaded.setData(saved.save()); + + assertThat(reloaded).as("learnset holding move %d at level %d", pair[0], pair[1]) + .hasSize(1); + assertThat(reloaded.get(0).getMoveID()).isEqualTo(pair[0]); + assertThat(reloaded.get(0).getLevel()).isEqualTo(pair[1]); + } + } + + @Test + void aMoveIdPastNineBitsIsRefusedRatherThanTruncated() + { + // 512 & 0x1FF == 0, so this used to save as "no move" with nothing reported. + // the test asserts the failure, and that the message names the value the user typed + assertThatThrownBy(() -> withOneEntry(512, 5).save()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("512"); + } + + @Test + void aLevelPastSevenBitsIsRefusedRatherThanTruncated() + { + // 200 & 0x7F == 72 + assertThatThrownBy(() -> withOneEntry(1, 200).save()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("200"); + } + + @Test + void theCombinationThatCollidesWithTheTerminatorIsRefused() + { + // (127 << 9) | 511 == 0xFFFF, which is exactly the end-of-list marker. both fields + // are individually legal, so only the packed value reveals the problem - saving it + // wrote a terminator and the entry silently disappeared on reload + assertThatThrownBy(() -> withOneEntry(511, 127).save()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("511") + .hasMessageContaining("127"); + } + + @Test + void everyOtherInRangeCombinationRoundTrips() + { + // exhaustive over both fields: the terminator collision must be the ONLY hole. + // a sampled test would have missed it, and would miss a second one just as easily + for (int level = 0; level <= 127; level++) + { + for (int move = 0; move <= 511; move++) + { + if (((level << 9) | move) == 0xFFFF) + continue; + + LearnsetData reloaded = empty(); + reloaded.setData(withOneEntry(move, level).save()); + assertThat(reloaded).as("move %d at level %d", move, level).hasSize(1); + assertThat(reloaded.get(0).getMoveID()).as("move %d at level %d", move, level).isEqualTo(move); + assertThat(reloaded.get(0).getLevel()).as("move %d at level %d", move, level).isEqualTo(level); + } + } + } + + @Test + void aNegativeMoveIdIsRefused() + { + // reachable from the sheet: a combo box with nothing selected reports -1, and + // -1 & 0x1FF is 511 - a real move, silently substituted for an empty selection + assertThatThrownBy(() -> withOneEntry(-1, 5).save()) + .isInstanceOf(IllegalArgumentException.class); + } + } +} diff --git a/src/test/java/io/github/turtleisaac/pokeditor/formats/GenericParserTest.java b/src/test/java/io/github/turtleisaac/pokeditor/formats/GenericParserTest.java index 4190978..335f7c4 100644 --- a/src/test/java/io/github/turtleisaac/pokeditor/formats/GenericParserTest.java +++ b/src/test/java/io/github/turtleisaac/pokeditor/formats/GenericParserTest.java @@ -4,30 +4,66 @@ import io.github.turtleisaac.nds4j.NintendoDsRom; import io.github.turtleisaac.nds4j.binaries.CodeBinary; import io.github.turtleisaac.pokeditor.gamedata.*; +import org.junit.jupiter.api.Assumptions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import java.nio.file.Files; +import java.nio.file.Path; import java.util.*; import static org.assertj.core.api.Assertions.assertThat; abstract class GenericParserTest { + /** + * The directory the test ROMs live in. These are copyrighted files which cannot be committed, so the + * suite skips rather than fails when they are not present. + */ + static final String ROM_DIRECTORY_PROPERTY = "rom.dir"; + + static final String PLATINUM_ROM_PROPERTY = "rom.platinum"; + static final String HEARTGOLD_ROM_PROPERTY = "rom.heartgold"; + + static final String DEFAULT_PLATINUM_ROM = "Platinum.nds"; + static final String DEFAULT_HEARTGOLD_ROM = "HeartGold.nds"; + protected GenericParser parser; protected NintendoDsRom rom; protected abstract GenericParser createParser(); + /** + * The name of the ROM file this test needs. Override this to run against a different game. + * @return a String + */ + protected String romFileName() + { + return System.getProperty(PLATINUM_ROM_PROPERTY, DEFAULT_PLATINUM_ROM); + } + @BeforeEach protected void setup() { parser = createParser(); - rom = NintendoDsRom.fromFile("Platinum.nds"); - Game game = Game.parseBaseRom(rom.getGameCode()); - GameFiles.initialize(game); - TextFiles.initialize(game); - GameCodeBinaries.initialize(game); - Tables.initialize(game); + } + + /** + * Loads the ROM this test needs, skipping the test if it is not present rather than failing the build + */ + protected void loadRom() + { + String fileName = romFileName(); + Path path = Path.of(System.getProperty(ROM_DIRECTORY_PROPERTY, ".")).resolve(fileName); + Assumptions.assumeTrue(Files.exists(path), + () -> "Skipping - no ROM at " + path.toAbsolutePath() + ". Provide one with -D" + ROM_DIRECTORY_PROPERTY + "="); + + rom = NintendoDsRom.fromFile(path.toString()); + Game.BaseRomInfo baseRomInfo = Game.parseBaseRom(rom.getGameCode()); + GameFiles.initialize(baseRomInfo.game()); + TextFiles.initialize(baseRomInfo.game()); + GameCodeBinaries.initialize(baseRomInfo.game()); + Tables.initialize(baseRomInfo.game(), baseRomInfo.region()); } @Test @@ -38,6 +74,8 @@ void parserNotNull() { @Test void outputMatchesInput() { + loadRom(); + HashMap map = new HashMap<>(); for (GameFiles gameFile : parser.getRequirements()) { map.put(gameFile, new Narc(rom.getFileByName(gameFile.getPath()))); @@ -52,8 +90,18 @@ void outputMatchesInput() { for (GameFiles gameFile : parser.getRequirements()) { Narc originalNarc = map.get(gameFile); Narc outputNarc = output.get(gameFile); + + assertThat(outputNarc) + .as("no output narc was produced for %s", gameFile) + .isNotNull(); + + assertThat(outputNarc.getFiles().size()) + .as("the number of subfiles in the %s narc changed", gameFile) + .isEqualTo(originalNarc.getFiles().size()); + for (int idx = 0; idx < originalNarc.getFiles().size(); idx++) { assertThat(outputNarc.getFile(idx)) + .as("subfile %d of the %s narc", idx, gameFile) .isEqualTo(originalNarc.getFile(idx)); } } diff --git a/src/test/java/io/github/turtleisaac/pokeditor/formats/ParserTests.java b/src/test/java/io/github/turtleisaac/pokeditor/formats/ParserTests.java index bf861aa..0d4e81d 100644 --- a/src/test/java/io/github/turtleisaac/pokeditor/formats/ParserTests.java +++ b/src/test/java/io/github/turtleisaac/pokeditor/formats/ParserTests.java @@ -3,7 +3,6 @@ import com.google.inject.Key; import com.google.inject.TypeLiteral; import io.github.turtleisaac.nds4j.Narc; -import io.github.turtleisaac.nds4j.NintendoDsRom; import io.github.turtleisaac.nds4j.binaries.CodeBinary; import io.github.turtleisaac.pokeditor.formats.encounters.JohtoEncounterData; import io.github.turtleisaac.pokeditor.formats.encounters.SinnohEncounterData; @@ -12,18 +11,15 @@ import io.github.turtleisaac.pokeditor.formats.learnsets.LearnsetData; import io.github.turtleisaac.pokeditor.formats.moves.MoveData; import io.github.turtleisaac.pokeditor.formats.personal.PersonalData; +import io.github.turtleisaac.pokeditor.formats.pokemon_sprites.PokemonSpriteData; import io.github.turtleisaac.pokeditor.formats.scripts.GenericScriptData; -import io.github.turtleisaac.pokeditor.formats.scripts.LevelScriptData; -import io.github.turtleisaac.pokeditor.formats.scripts.FieldScriptData; import io.github.turtleisaac.pokeditor.formats.scripts.FieldScriptParser; import io.github.turtleisaac.pokeditor.formats.text.TextBankData; import io.github.turtleisaac.pokeditor.formats.trainers.TrainerData; import io.github.turtleisaac.pokeditor.gamedata.*; -import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; -import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -33,7 +29,8 @@ public class ParserTests { - public static class PersonalTests extends GenericParserTest + @Nested + class PersonalTests extends GenericParserTest { @Override protected GenericParser createParser() @@ -42,7 +39,8 @@ protected GenericParser createParser() } } - public static class LearnsetsTests extends GenericParserTest + @Nested + class LearnsetsTests extends GenericParserTest { @Override protected GenericParser createParser() @@ -51,7 +49,8 @@ protected GenericParser createParser() } } - public static class EvolutionsTests extends GenericParserTest + @Nested + class EvolutionsTests extends GenericParserTest { @Override protected GenericParser createParser() @@ -60,7 +59,8 @@ protected GenericParser createParser() } } - public static class TrainersTests extends GenericParserTest + @Nested + class TrainersTests extends GenericParserTest { @Override protected GenericParser createParser() @@ -69,7 +69,8 @@ protected GenericParser createParser() } } - public static class MovesTests extends GenericParserTest + @Nested + class MovesTests extends GenericParserTest { @Override protected GenericParser createParser() @@ -78,7 +79,8 @@ protected GenericParser createParser() } } - public static class SinnohEncountersTests extends GenericParserTest + @Nested + class SinnohEncountersTests extends GenericParserTest { @Override protected GenericParser createParser() @@ -87,7 +89,8 @@ protected GenericParser createParser() } } - public static class JohtoEncountersTests extends GenericParserTest + @Nested + class JohtoEncountersTests extends GenericParserTest { @Override protected GenericParser createParser() @@ -95,21 +98,15 @@ protected GenericParser createParser() return injector.getInstance(Key.get(new TypeLiteral<>() {})); } - @BeforeEach @Override - protected void setup() + protected String romFileName() { - parser = createParser(); - rom = NintendoDsRom.fromFile("HeartGold.nds"); - Game game = Game.parseBaseRom(rom.getGameCode()); - GameFiles.initialize(game); - TextFiles.initialize(game); - GameCodeBinaries.initialize(game); - Tables.initialize(game); + return System.getProperty(HEARTGOLD_ROM_PROPERTY, DEFAULT_HEARTGOLD_ROM); } } - public static class ItemsTests extends GenericParserTest + @Nested + class ItemsTests extends GenericParserTest { @Override protected GenericParser createParser() @@ -118,7 +115,8 @@ protected GenericParser createParser() } } - public static class TextBankTests extends GenericParserTest + @Nested + class TextBankTests extends GenericParserTest { @Override protected GenericParser createParser() @@ -127,6 +125,16 @@ protected GenericParser createParser() } } + @Nested + class PokemonSpriteTests extends GenericParserTest + { + @Override + protected GenericParser createParser() + { + return injector.getInstance(Key.get(new TypeLiteral<>() {})); + } + } + @Nested class FieldScriptsTests extends GenericParserTest { @@ -136,23 +144,18 @@ protected GenericParser createParser() return new FieldScriptParser(); } - @BeforeEach @Override - protected void setup() + protected String romFileName() { - parser = createParser(); - rom = NintendoDsRom.fromFile("HeartGold.nds"); - Game game = Game.parseBaseRom(rom.getGameCode()); - GameFiles.initialize(game); - TextFiles.initialize(game); - GameCodeBinaries.initialize(game); - Tables.initialize(game); + return System.getProperty(HEARTGOLD_ROM_PROPERTY, DEFAULT_HEARTGOLD_ROM); } @Test @Override void outputMatchesInput() { + loadRom(); + HashMap map = new HashMap<>(); for (GameFiles gameFile : parser.getRequirements()) { map.put(gameFile, new Narc(rom.getFileByName(gameFile.getPath()))); @@ -164,49 +167,30 @@ void outputMatchesInput() List data = parser.generateDataList(map, codeBinaries); Map output = parser.processDataList(data, codeBinaries); - int nonExactMatches = 0; - for (GameFiles gameFile : parser.getRequirements()) { Narc originalNarc = map.get(gameFile); Narc outputNarc = output.get(gameFile); + + assertThat(outputNarc) + .as("no output narc was produced for %s", gameFile) + .isNotNull(); + + assertThat(outputNarc.getFiles().size()) + .as("the number of script files changed") + .isEqualTo(originalNarc.getFiles().size()); + for (int idx = 0; idx < originalNarc.getFiles().size(); idx++) { byte[] outputFile = outputNarc.getFile(idx); - if (Arrays.equals(originalNarc.getFile(idx), outputFile)) - { - assertThat(outputFile) - .isEqualTo(originalNarc.getFile(idx)); - } - else if (outputFile.length != 0) - { - BytesDataContainer container = new BytesDataContainer(); - container.insert(GameFiles.FIELD_SCRIPTS, null, outputFile); - - GenericScriptData scriptData; - if (FieldScriptParser.testFileIsLevelScript(originalNarc.getFile(idx))) - scriptData = new LevelScriptData(container); - else - scriptData = new FieldScriptData(container); - - container = scriptData.save(); - byte[] rebuiltResult = container.get(GameFiles.FIELD_SCRIPTS, null); - - if (Arrays.equals(rebuiltResult, outputFile)) - { - System.out.println("Valid but non-1:1 Match: File " + idx); - nonExactMatches++; - } - else - { - System.err.println("File did not match original, attempted conditional rebuild but failed"); - } - - assertThat(rebuiltResult) - .isEqualTo(outputFile); - } + + assertThat(outputFile) + .as("script file %d serialized to nothing", idx) + .isNotEmpty(); + + assertThat(outputFile) + .as("script file %d", idx) + .isEqualTo(originalNarc.getFile(idx)); } } - - System.out.printf("In total, there were %d valid but non-1:1 matching rebuilt field script files (%d 1:1 matches).\n", nonExactMatches, data.size()-nonExactMatches); } } } diff --git a/src/test/java/io/github/turtleisaac/pokeditor/formats/SignedFieldTest.java b/src/test/java/io/github/turtleisaac/pokeditor/formats/SignedFieldTest.java new file mode 100644 index 0000000..a89fdd0 --- /dev/null +++ b/src/test/java/io/github/turtleisaac/pokeditor/formats/SignedFieldTest.java @@ -0,0 +1,145 @@ +package io.github.turtleisaac.pokeditor.formats; + +import io.github.turtleisaac.pokeditor.formats.items.ItemData; +import io.github.turtleisaac.pokeditor.formats.moves.MoveData; +import io.github.turtleisaac.pokeditor.gamedata.GameFiles; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Sign handling for the fields the Gen IV formats define as signed 8-bit. + *

+ * The underlying buffer returns an unsigned byte, so any field that the format defines + * as signed has to be converted at the point of the read. Getting this wrong does not corrupt the + * file — every write path narrows back through a {@code byte}, so the bytes round-trip + * either way — but it corrupts the value: a move priority of -7 becomes 249, which + * is what the editor then displays and range-checks. + *

+ * These tests build records from raw bytes rather than from a ROM, so they run everywhere. Each + * asserts both halves of the property: the parsed value carries the right sign, and the bytes + * still survive a round trip. + */ +@DisplayName("Signed 8-bit fields keep their sign") +class SignedFieldTest +{ + /** Byte 0xF9 is -7 as a signed value and 249 as an unsigned one. */ + private static final int RAW = 0xF9; + private static final int SIGNED = -7; + + private static BytesDataContainer container(GameFiles file, byte[] data) + { + return new BytesDataContainer(file, null, data); + } + + private static byte[] moveRecordWithPriority(int rawPriority) + { + // The moves record is a fixed 16-byte struct; priority sits at offset 0x0A, after + // effect(2), category(1), power(1), type(1), accuracy(1), pp(1), effectChance(1), + // target(2). + byte[] record = new byte[16]; + record[0x0A] = (byte) rawPriority; + return record; + } + + @Test + @DisplayName("move priority parses negative, as Trick Room and Roar require") + void movePriorityIsSigned() + { + MoveData move = new MoveData(container(GameFiles.MOVES, moveRecordWithPriority(RAW))); + + assertThat(move.getPriority()) + .as("0x%02X in the priority field is %d, not %d", RAW, SIGNED, RAW) + .isEqualTo(SIGNED); + } + + @Test + @DisplayName("move priority survives a round trip regardless of sign") + void movePriorityRoundTrips() + { + for (int raw = 0; raw <= 0xFF; raw++) + { + byte[] original = moveRecordWithPriority(raw); + MoveData move = new MoveData(container(GameFiles.MOVES, original)); + byte[] written = move.save().get(GameFiles.MOVES, null); + + assertThat(written[0x0A]) + .as("priority byte 0x%02X must survive parse then save", raw) + .isEqualTo((byte) raw); + } + } + + @Test + @DisplayName("the full signed byte domain parses to the right value") + void movePriorityDomain() + { + for (int raw = 0; raw <= 0xFF; raw++) + { + MoveData move = new MoveData(container(GameFiles.MOVES, moveRecordWithPriority(raw))); + assertThat(move.getPriority()) + .as("raw byte 0x%02X", raw) + .isEqualTo((int) (byte) raw) + .isBetween(-128, 127); + } + } + + @Test + @DisplayName("item EV yields and friendship changes keep their sign across a save") + void itemSignedFieldsSurviveRoundTrip() + { + // Expressed without reference to field offsets: set the signed fields, save, reload, and + // require the values to come back as they went in. That is the property a user cares + // about, and it stays true if the record layout is ever revised. + ItemData item = new ItemData(container(GameFiles.ITEMS, new byte[ITEM_RECORD_SIZE])); + item.setEvYields(new int[]{SIGNED, SIGNED, SIGNED, SIGNED, SIGNED, SIGNED}); + item.setFriendshipChangeAmounts(new int[]{SIGNED, SIGNED, SIGNED}); + + ItemData reloaded = new ItemData( + container(GameFiles.ITEMS, item.save().get(GameFiles.ITEMS, null))); + + assertThat(reloaded.getEvYields()) + .as("EV-reducing berries yield negative EVs, so the sign has to survive") + .containsOnly(SIGNED); + assertThat(reloaded.getFriendshipChangeAmounts()) + .as("bitter berries lower friendship, so the sign has to survive") + .containsOnly(SIGNED); + } + + @Test + @DisplayName("every signed byte value survives an item save/reload") + void itemSignedDomain() + { + for (int raw = 0; raw <= 0xFF; raw++) + { + int signed = (byte) raw; + ItemData item = new ItemData(container(GameFiles.ITEMS, new byte[ITEM_RECORD_SIZE])); + item.setFriendshipChangeAmounts(new int[]{signed, signed, signed}); + + ItemData reloaded = new ItemData( + container(GameFiles.ITEMS, item.save().get(GameFiles.ITEMS, null))); + + assertThat(reloaded.getFriendshipChangeAmounts()) + .as("friendship change %d (raw 0x%02X)", signed, raw) + .containsOnly(signed); + } + } + + @Test + @DisplayName("an item record round trips byte-for-byte") + void itemRecordRoundTrips() + { + // Whatever the sign handling, the bytes themselves must be preserved exactly. + ItemData item = new ItemData(container(GameFiles.ITEMS, new byte[ITEM_RECORD_SIZE])); + byte[] first = item.save().get(GameFiles.ITEMS, null); + byte[] second = new ItemData(container(GameFiles.ITEMS, first)) + .save().get(GameFiles.ITEMS, null); + + assertThat(second) + .as("parse then save must be a fixed point") + .isEqualTo(first); + } + + /** The fixed portion of an items record is 34 bytes; anything beyond is trailing padding. */ + private static final int ITEM_RECORD_SIZE = 36; +} diff --git a/src/test/java/io/github/turtleisaac/pokeditor/formats/TestsInjector.java b/src/test/java/io/github/turtleisaac/pokeditor/formats/TestsInjector.java index 55b9024..646731c 100644 --- a/src/test/java/io/github/turtleisaac/pokeditor/formats/TestsInjector.java +++ b/src/test/java/io/github/turtleisaac/pokeditor/formats/TestsInjector.java @@ -12,6 +12,8 @@ import io.github.turtleisaac.pokeditor.formats.moves.MoveParser; import io.github.turtleisaac.pokeditor.formats.personal.PersonalData; import io.github.turtleisaac.pokeditor.formats.personal.PersonalParser; +import io.github.turtleisaac.pokeditor.formats.pokemon_sprites.PokemonSpriteData; +import io.github.turtleisaac.pokeditor.formats.pokemon_sprites.PokemonSpriteParser; import io.github.turtleisaac.pokeditor.formats.text.TextBankData; import io.github.turtleisaac.pokeditor.formats.text.TextBankParser; import io.github.turtleisaac.pokeditor.formats.trainers.TrainerData; @@ -109,5 +111,15 @@ protected void configure() } } - public static final Injector injector = Guice.createInjector(new PersonalModule(), new LearnsetsModule(), new EvolutionsModule(), new TrainersModule(), new MovesModule(), new SinnohEncountersModule(), new JohtoEncountersModule(), new ItemsModule(), new TextBankModule()); + static class PokemonSpriteModule extends AbstractModule { + @Override + protected void configure() + { + bind(new TypeLiteral>() {}) + .to(PokemonSpriteParser.class) + .in(Scopes.SINGLETON); + } + } + + public static final Injector injector = Guice.createInjector(new PersonalModule(), new LearnsetsModule(), new EvolutionsModule(), new TrainersModule(), new MovesModule(), new SinnohEncountersModule(), new JohtoEncountersModule(), new ItemsModule(), new TextBankModule(), new PokemonSpriteModule()); }