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

[turtleisaac]

Part of a four-repo audit of the PokEditor toolchain. Depends on turtleisaac/Nds4j#2 — merge that first.

⚠️ Contains breaking public API changes — see the bottom. PokEditor's companion PR is already updated for them.

The governing defect

A field's parse path and its serialize path disagreed. These need no user edit to bite: loading a ROM and saving it is enough. Every fix below was verified with a byte-exact round-trip harness over randomized inputs.

Field	Read as	Written as
Trainer alt-form	6 bits (>> 10)	3 bits (& 0x7)
Item stat boosts	& 0xf	& 0x3
Item crit stage	& 0x3	& 0xf — overflow bled into the PP Up flags
EV yields	2-bit fields	setters accepted 4, write never masked
DPPt encounters	skip(0x2C)	skip(0x2C) — 44 bytes re-emitted as zeroes

Concretely: every Unown letter past H and every Arceus plate form was silently remapped or reverted; Speed and Sp. Def stat stages lost their top two bits; a crit stage of 4 turned an item into a PP Up; and ~600 pl_enc_data subfiles lost a rate word plus a five-slot water encounter table on every save.

Also in this class: trainer type flags and the AI bitfield were re-encoded from only the bits the tool understood, zeroing everything else; the upper nibble at item offset 0x14 and bits 12-15 of the personal EV halfword were read into nothing and written as zero; level-script trigger numbers were narrowed through a signed byte cast, so any script above 127 sign-extended.

Scripts

• Command 225 is goto_if_trainer_defeated, not a comparator. It was listed in isDoIfCommand, so loading any script that uses it threw IllegalStateException and aborted the entire NARC. Its jump target is now registered as a label and recomputed on save instead of being emitted from the old file's absolute address.
• ActionCommand(int, int) never assigned its id — the fallback taken whenever a movement name lookup misses. Movements_Hg.json omits real HGSS opcodes 96-103 and 106, so those movements re-serialized as movement 0.
• visitAction_command only called super, so recompiling a script dropped every movement command while keeping the action_N: labels pointing at them.
• An unresolvable label serialized as a branch to absolute offset 0.
• Trainer AI opcodes were written 16-bit though every macro declares .word and the reader reads readUInt32().
• Hex literals crashed both algebra visitors — and Convenience_Hg.txt:92 already contains .if \item < 0x4000. The macro files are user-editable, so one added hex constant was a load-time crash for every script in the ROM.

Text

9-bit compression only guarded characters up to 0x3FF, so a newline (0xE000) or a VAR placeholder overran its slot and overwrote following characters. Unmapped characters were silently written as code 0 — paste dialogue with a typographic apostrophe and it became a null glyph in-game, reported only to a console with no window attached. Bank tables, VAR sequences and escape decoding are now bounds-checked, so a message ending in a stray backslash no longer aborts the whole ROM save.

Global state

• PersonalParser's TM/HM tables were static and never reset: open ROM A, then save ROM B, and ROM A's TM list was written into ROM B's ARM9.
• The sprite parser's party-icon header cache was static on a Guice singleton; when unset the write silently skipped, shifting the entire party-icon NARC by seven files.
• Game.region was mutable state on a global enum constant.
• Diamond and Pearl fell through every initialize switch with no default, so a D/P ROM half-initialized and failed far from the cause with an unrelated NPE.
• The sprite parser mutated the shared ARM9 buffer without holding its lock, while processDataList in the same class does.

The test suite never ran

Nine of the ten parser test classes were declared public static class — which Surefire's default **/*$* exclude drops and JUnit only descends into for @Nested. Only one ran. Proven by converting them: the suite went from 2 discovered tests to 22.

And the one surviving round-trip test could not fail:

if (Arrays.equals(originalNarc.getFile(idx), outputFile)) {
    assertThat(outputFile).isEqualTo(originalNarc.getFile(idx));  // tautology
}
else if (outputFile.length != 0) {                                // empty ⇒ nothing asserted
    assertThat(rebuiltResult).isEqualTo(outputFile);              // output vs output
}

A parser returning zero bytes for every subfile passed this green — which is exactly how TrainerAiData.save(), an empty stub returning a zero-length buffer, survived. It now compares against the original and asserts the file count.

The suite also hard-required a copyrighted ROM beside the pom and failed the build without one; ROMs are now located via -Drom.dir and missing ones skip. Added the sprite parser binding and test class, which had no coverage at all.

SignedFieldTest is new and needs no ROM: it pins the fields the format defines as s8 across the whole 0..255 domain, asserting both that the sign survives and that the bytes round-trip.

Breaking API changes

// Game — region is no longer mutable state on the enum constant
public record BaseRomInfo(Game game, Region region) {}
public static BaseRomInfo parseBaseRom(String)     // was: static Game parseBaseRom(String)
@Deprecated public Region getRegion()              // still works

// Tables
public static void initialize(Game, Game.Region)   // NEW, preferred
@Deprecated public static void initialize(Game)    // delegates
public int getPointerOffset()                      // now THROWS when unset (was: returned 0)

// PersonalParser — static -> instance, so two ROMs cannot share TM tables
public int[] getTmMoveIdNumbers()                  // was: public static final int[] tmMoveIdNumbers
public int getTmMoveIdNumber(int tmID)             // NEW
public void updateTmType(int, int, List<MoveData>) // was: static

Also newly throwing where they previously corrupted silently: PersonalData.set*EvYield (rejects > 3), setDexColor (rejects > 0x7F), ItemData.setStatBoosts, and TextBankData (new TextEncodingException).

Verification

mvn test → BUILD SUCCESS, 28 tests discovered, 0 failures, 11 skipped (ROM-dependent).

🤖 Generated with Claude Code

https://claude.ai/code/session_01RqYGZhtVb8KrPA9ftp5bob

---

Generated by Claude Code