Merge pull request #456 from johnjohto/fix-yellow-npc-trades

Fix Yellow NPC trades

README.md

@@ -1,6 +1,6 @@
 # Gen1Recomp
 
-A native LÖVE2D recreation of Poke Red and Blue. The engine and map
+A native LÖVE2D recreation of Poke Red, Blue and Yellow. The engine and map
 behavior are hand-written Lua; game data and graphics are decoded from a ROM
 supplied by the player.
 
@@ -9,6 +9,9 @@ supplied by the player.
 **SUPPORT / ANNOUNCEMENTS / MODS:** [Discord](https://bois.icu)
 
 <p align="center"> <a href="https://www.polygon.com/pokemon-red-blue-3d-voxel-mod-battle-pixels-gameplay-footage-remake/"> <img src="https://img.shields.io/badge/AS%20SEEN%20ON-POLYGON-ea2e49?style=for-the-badge" alt="As seen on Polygon"> </a> </p>
+<p align="center"> <a href="https://kotaku.com/pokemon-red-blue-recompilation-project-voxel-3d-mod-2000720281"> <img src="https://img.shields.io/badge/AS%20SEEN%20ON-KOTAKU-ea2e49?style=for-the-badge" alt="As seen on KOTAKU"> </a> </p>
+
+
 
 ### Watch the latest update video
 

mobile/ANDROID.md

@@ -70,7 +70,10 @@ The APK lands under `app/build/outputs/apk/embedNoRecord/debug/`.
 ### Payload path
 
 `app/src/embed/assets/game.love` - zip of `main.lua`, `conf.lua`, `src/`,
-`data/`, `assets/`, and `tools/rom_manifest.json`. Generated game data,
+`data/`, `assets/`, and the Red, Blue, and Yellow ROM manifests. The Android
+packer verifies the Yellow manifest before it packages; if a partial source
+export omitted it, it restores the file from this checkout's Git data and then
+falls back to the project's GitHub copy. Generated game data,
 scripts, tests, and mobile build sources are excluded.
 
 ## Branding (applied by the build script)

scripts/build_android.sh

@@ -26,6 +26,11 @@ APP_NAME="gen1recomp"
 APPLICATION_ID="com.theboisclub.pokemonred"
 LOVE_ANDROID_VERSION="11.5a"
 NDK_VERSION="25.2.9519653"
+YELLOW_MANIFEST_RELATIVE="tools/rom_manifest_yellow.json"
+# A fresh source checkout normally supplies this through Git.  This URL is
+# deliberately only a last resort for incomplete source exports: the manifest
+# contains extraction metadata, never a ROM or extracted game data.
+YELLOW_MANIFEST_URL="${YELLOW_MANIFEST_URL:-https://raw.githubusercontent.com/bryanthaboi/gen1recomp/main/tools/rom_manifest_yellow.json}"
 
 VERSION=""
 PACKAGE_ONLY=false
@@ -72,6 +77,59 @@ if [ ! -d "$ANDROID_DIR/love/src/jni/love/src" ]; then
   Re-clone or 'git checkout -- mobile/android'. See mobile/ANDROID.md."
 fi
 
+# ------------------------------------------------------- Yellow import metadata
+# Android packages game.love itself rather than reusing scripts/build.sh's
+# archive.  Keep a partial source export from silently shipping an APK that can
+# list Yellow but cannot import it.  Prefer the exact manifest from this
+# checkout's Git object database; only then fall back to the public repository.
+yellow_manifest_is_valid() {
+  local path="$1"
+  python3 - "$path" <<'PY'
+import json, pathlib, sys
+
+try:
+    manifest = json.loads(pathlib.Path(sys.argv[1]).read_text())
+except (OSError, ValueError):
+    raise SystemExit(1)
+
+raise SystemExit(0 if manifest.get("romSha1") ==
+                 "cc7d03262ebfaf2f06772c1a480c7d9d5f4a38e1" else 1)
+PY
+}
+
+ensure_yellow_manifest() {
+  local manifest="$ROOT/$YELLOW_MANIFEST_RELATIVE"
+  local staged
+  staged="$(mktemp)"
+
+  if yellow_manifest_is_valid "$manifest"; then
+    rm -f "$staged"
+    return
+  fi
+
+  warn "Yellow import manifest is missing or invalid; recovering it before packaging"
+  if git -C "$ROOT" show "HEAD:$YELLOW_MANIFEST_RELATIVE" > "$staged" 2>/dev/null \
+      && yellow_manifest_is_valid "$staged"; then
+    mkdir -p "$(dirname "$manifest")"
+    mv "$staged" "$manifest"
+    say "restored Yellow import manifest from this checkout's Git data"
+    return
+  fi
+
+  if command -v curl >/dev/null 2>&1 \
+      && curl --fail --location --retry 2 --connect-timeout 15 \
+          --output "$staged" "$YELLOW_MANIFEST_URL" \
+      && yellow_manifest_is_valid "$staged"; then
+    mkdir -p "$(dirname "$manifest")"
+    mv "$staged" "$manifest"
+    say "downloaded Yellow import manifest from the project repository"
+    return
+  fi
+
+  rm -f "$staged"
+  fail "Yellow import manifest is unavailable. Git recovery failed and could not download $YELLOW_MANIFEST_URL"
+}
+
 # --------------------------------------------------------------- branding
 # love-android 11.5+ reads app id / name / orientation from gradle.properties.
 # Manifest still gets permission trims. Re-applied every build so refreshing
@@ -135,6 +193,7 @@ PY
 # --------------------------------------------------------------- game.love
 pack_game_love() {
   say "packing game.love for love-android embed flavor"
+  ensure_yellow_manifest
   mkdir -p "$EMBED_ASSETS"
   rm -f "$LOVE_FILE"
   # tools/save-editor ships with the app: the launcher's Edit button on a save
@@ -142,14 +201,21 @@ pack_game_love() {
   (cd "$ROOT" && zip -q -9 -r "$LOVE_FILE" \
     main.lua conf.lua src data assets tools/save-editor \
     tools/rom_manifest.json tools/rom_manifest_blue.json \
+    tools/rom_manifest_yellow.json \
     -x '*.DS_Store' -x '*/.git/*' -x '*/.DS_Store' \
     -x 'data/generated/*' -x 'assets/generated/*')
   if unzip -Z1 "$LOVE_FILE" \
       | grep -Eq '^(data|assets)/generated/[^/]+|^(data|assets)/generated/.+/'; then
     fail "game.love unexpectedly contains generated ROM data"
   fi
-  unzip -Z1 "$LOVE_FILE" | grep -qx 'tools/save-editor/App.lua' \
+  # Do not pipe unzip straight into grep here: on a large archive grep can
+  # finish early and make unzip report SIGPIPE under `set -o pipefail`.
+  local archive_entries
+  archive_entries="$(unzip -Z1 "$LOVE_FILE")"
+  grep -qx 'tools/save-editor/App.lua' <<< "$archive_entries" \
     || fail "game.love is missing the save editor (Edit on a save row would crash)"
+  grep -qx "$YELLOW_MANIFEST_RELATIVE" <<< "$archive_entries" \
+    || fail "game.love is missing the Yellow ROM import manifest"
   say "game.love: $(du -h "$LOVE_FILE" | cut -f1) -> $LOVE_FILE"
 
   # This script packs its own game.love (it does not reuse build.sh's), so it

src/battle/BattleState.lua

@@ -188,6 +188,27 @@ local function namedPalette(data, name)
   return { name = key, colors = colors }
 end
 
+-- Custom trainer portraits can opt into the same Advanced OBJ palette source
+-- as their overworld walker. Vanilla trainers preserve the hardware-faithful
+-- MEWMON fallback used during the battle introduction.
+function BattleState.trainerPalette(data, trainer)
+  local source = trainer and trainer.paletteSource
+  if source then
+    local PaletteFX = require("src.render.PaletteFX")
+    local colors, group = PaletteFX.spriteObp({ paletteSource = source }, trainer.id)
+    if colors then
+      return { name = "trainer:" .. source .. ":" .. tostring(group), colors = colors }
+    end
+  end
+  return namedPalette(data, "MEWMON")
+end
+
+function BattleState.trainerPicPath(data, trainer)
+  if trainer and trainer.pic then return trainer.pic end
+  local base = trainer and trainer.basePic and data.trainers[trainer.basePic]
+  return base and base.pic or nil
+end
+
 -- The battle-BGP fade variant of a pic (AnimationFlashScreen and the
 -- SetAnimationBGPalette effects remap the four BG shades; on the SGB
 -- the colorizer then colors the REMAPPED shade, so a faded pic shows
@@ -608,7 +629,8 @@ function BattleState.newTrainer(game, oppClass, partyIndex)
   -- MonsterPalettes[0] = PAL_MEWMON -- InitBattleCommon zeroes
   -- wEnemyMonSpecies2 before the intro's SET_PAL_BATTLE
   -- (engine/battle/core.asm:6682, engine/gfx/palettes.asm SetPal_Battle)
-  self.trainerPic = getImage(self.trainer.pic, namedPalette(game.data, "MEWMON"))
+  self.trainerPic = getImage(BattleState.trainerPicPath(game.data, self.trainer),
+    BattleState.trainerPalette(game.data, self.trainer))
   self.introText = Strings("%s wants\nto fight!", self.trainer.name)
   return self
 end

src/core/Data.lua

@@ -38,6 +38,22 @@ local CONSTANT_DEFAULTS = {
   },
 }
 
+-- data/events/trades.asm in Pokemon Yellow has its own TradeMons table.
+-- Old Yellow imports were built from the Red manifest, so correct that
+-- table after loading as well as in the fixed import manifest below.
+local YELLOW_TRADES = {
+  { give = "LICKITUNG", get = "DUGTRIO", dialogset = 1, nickname = "GURIO" },
+  { give = "CLEFAIRY", get = "MR_MIME", dialogset = 1, nickname = "MILES" },
+  { give = "BUTTERFREE", get = "BEEDRILL", dialogset = 3, nickname = "STINGER" },
+  { give = "KANGASKHAN", get = "MUK", dialogset = 1, nickname = "STICKY" },
+  { give = "MEW", get = "MEW", dialogset = 3, nickname = "BART" },
+  { give = "TANGELA", get = "PARASECT", dialogset = 1, nickname = "SPIKE" },
+  { give = "PIDGEOT", get = "PIDGEOT", dialogset = 2, nickname = "MARTY" },
+  { give = "GOLDUCK", get = "RHYDON", dialogset = 2, nickname = "BUFFY" },
+  { give = "GROWLITHE", get = "DEWGONG", dialogset = 3, nickname = "CEZANNE" },
+  { give = "CUBONE", get = "MACHOKE", dialogset = 3, nickname = "RICKY" },
+}
+
 -- field.boot is the total-conversion override point for the new game; the
 -- values match what SaveData.newGame and the Oak speech used to inline.
 local BOOT_DEFAULTS = {
@@ -58,6 +74,12 @@ local function copy(value)
   return out
 end
 
+function Data:applyVersionedFieldData()
+  if require("src.core.GameVersion").isYellow() then
+    self.field.trades = copy(YELLOW_TRADES)
+  end
+end
+
 -- Fills only what the cache is missing, so an importer that learns to
 -- stamp one of these keys silently takes over from the engine.
 function Data:seedDefaults()
@@ -77,6 +99,7 @@ function Data:seedDefaults()
   if constants.dexDigits == nil then
     constants.dexDigits = math.max(3, #tostring(constants.dexSize))
   end
+  self:applyVersionedFieldData()
   local boot = self.field.boot
   if boot == nil then
     boot = {}

src/mods/Schemas.lua

@@ -561,6 +561,11 @@ R.trainers = {
     index = f.opt(f.int(0, 255)),
     -- unused vanilla classes ship without a pic, so it cannot be required
     pic = f.opt(f.path),
+    -- Optional Advanced-mode OBJ palette source for a custom trainer portrait.
+    -- It follows the same ROM crosswalk form as sprites.paletteSource.
+    paletteSource = f.opt(f.str),
+    -- Reuse a base trainer class's portrait without redistributing its asset.
+    basePic = f.opt(f.id("trainers")),
     baseMoney = f.opt(f.int(0)),
     parties = f.list(f.list(f.rec{ level = f.int(1),
                                    species = f.id("pokemon") })),
@@ -580,6 +585,10 @@ R.sprites = {
     frames = f.int(1),
     walker = f.opt(f.bool),
     trueColor = f.opt(f.bool),
+    -- Mod art can opt into an existing ROM sprite's Advanced-mode OBJ
+    -- palette assignment without claiming that the image itself came from
+    -- the ROM (which is what `source` documents on imported records).
+    paletteSource = f.opt(f.str),
   },
   example = 'mod.content.sprites:register("SPRITE_HERO", { image = "...", frames = 6 })',
 }

src/render/PaletteFX.lua

@@ -537,7 +537,7 @@ end
 function PaletteFX.spriteObp(spriteDef, seed)
   local pack = PaletteFX.gbcPack()
   local w = pack and pack.world
-  local src = spriteDef and spriteDef.source
+  local src = spriteDef and (spriteDef.paletteSource or spriteDef.source)
   if not (w and src) then return nil end
   local idx = tonumber(src:match("%[(%d+)%]"))
   -- RedBikeSprite loads outside SpriteSheetPointerTable

tests/mod_battle_tests.lua

@@ -11,6 +11,7 @@ local Font = require("src.render.Font")
 Font.load(Data)
 
 local BattleState = require("src.battle.BattleState")
+local PaletteFX = require("src.render.PaletteFX")
 local Catching = require("src.battle.Catching")
 local Damage = require("src.battle.Damage")
 local Events = require("src.mods.Events")
@@ -327,6 +328,25 @@ end
 
 -- ------- ai_classes: brains and layer records
 
+-- ------- custom trainer portraits: palette sources and base portraits
+
+do
+  local oldMode = PaletteFX.mode
+  PaletteFX.mode = "redpp"
+  local custom = { id = "TEST_TRAINER",
+                   paletteSource = "ROM:SpriteSheetPointerTable[21]" }
+  local pal = BattleState.trainerPalette(Data, custom)
+  local expected = PaletteFX.spriteObp({ paletteSource = custom.paletteSource }, custom.id)
+  PaletteFX.mode = oldMode
+  check(pal and expected and pal.colors[3][1] == expected[3][1]
+        and pal.colors[3][2] == expected[3][2]
+        and pal.colors[3][3] == expected[3][3],
+        "a custom trainer portrait resolves its Advanced OBJ palette source")
+  check(BattleState.trainerPicPath(Data, { basePic = "OPP_ENGINEER" })
+        == Data.trainers.OPP_ENGINEER.pic,
+        "a custom trainer can reuse a base trainer portrait by id")
+end
+
 do
   Data.ai_classes = { OPP_YOUNGSTER = { brain = function(battle)
     return { id = "SPLASH", pp = 1, brained = true }

tests/parity_yellow_trades.lua

@@ -0,0 +1,59 @@
+-- Pokemon Yellow has a different TradeMons table from Red and Blue.
+-- The import manifest must agree, and Data:seedDefaults repairs Yellow
+-- caches made before that manifest carried the Yellow table (#453).
+
+package.path = "./?.lua;./?/init.lua;" .. package.path
+if not _G.love then _G.love = require("tests.love_stub") end
+
+local Data = require("src.core.Data")
+if not (Data.field and Data.field.trades) then Data:load() end
+local GameVersion = require("src.core.GameVersion")
+local S = require("tests.harness").suite("parity Yellow trades")
+local check, eq = S.check, S.eq
+
+local oldVersion = GameVersion.get()
+local oldTrades = {}
+for i, trade in ipairs(Data.field.trades) do
+  oldTrades[i] = {}
+  for key, value in pairs(trade) do oldTrades[i][key] = value end
+end
+
+GameVersion.set("yellow")
+Data:applyVersionedFieldData()
+
+local expected = {
+  { "LICKITUNG", "DUGTRIO", "GURIO" },
+  { "CLEFAIRY", "MR_MIME", "MILES" },
+  { "BUTTERFREE", "BEEDRILL", "STINGER" },
+  { "KANGASKHAN", "MUK", "STICKY" },
+  { "MEW", "MEW", "BART" },
+  { "TANGELA", "PARASECT", "SPIKE" },
+  { "PIDGEOT", "PIDGEOT", "MARTY" },
+  { "GOLDUCK", "RHYDON", "BUFFY" },
+  { "GROWLITHE", "DEWGONG", "CEZANNE" },
+  { "CUBONE", "MACHOKE", "RICKY" },
+}
+
+local manifestFile = assert(io.open("tools/rom_manifest_yellow.json", "r"))
+local manifest = manifestFile:read("*a")
+manifestFile:close()
+
+eq(#Data.field.trades, #expected, "Yellow has ten in-game trade rows")
+for i, want in ipairs(expected) do
+  local got = Data.field.trades[i]
+  check(got.give == want[1] and got.get == want[2] and got.nickname == want[3],
+    ("Yellow trade %d is %s for %s (%s)"):format(i, want[1], want[2], want[3]))
+  local row = ('"get": "%s",%%s+"give": "%s",%%s+"nickname": "%s"')
+    :format(want[2], want[1], want[3])
+  check(manifest:find(row) ~= nil,
+    ("Yellow manifest carries trade %d"):format(i))
+end
+
+local ricky = Data.field.trades[10]
+check(ricky.give == "CUBONE" and ricky.get == "MACHOKE",
+  "the Underground Path trade is Cubone for Machoke")
+
+Data.field.trades = oldTrades
+GameVersion.set(oldVersion)
+
+return S:finish()

tests/run_tests.lua

@@ -3277,6 +3277,7 @@ runSuites(orderedGlob("tests/parity_*.lua", {
   "tests/parity_flavor.lua", "tests/parity_trainer_sight.lua",
   "tests/parity_static.lua", "tests/parity_trashcans.lua",
   "tests/parity_hof.lua", "tests/parity_trade_gift.lua",
+  "tests/parity_yellow_trades.lua",
   "tests/parity_intro.lua", "tests/parity_tilt.lua",
   "tests/parity_gbcfx.lua",
 }))

tools/make_yellow_manifest.py

@@ -382,6 +382,7 @@ def derive(red, pokeyellow, symbols_path):
         except SystemExit as exc:
             print(f"warning: parse_credits failed ({exc}); keeping Red credits")
             # TODO: hand-author a Yellow credits banner if pret layout drifts.
+        yellow["field"]["trades"] = field.parse_trades(pokeyellow)
     finally:
         util.ASM_DEFINES = saved
 

tools/rom_manifest_yellow.json

@@ -10552,63 +10552,63 @@
     "trades": [
       {
         "dialogset": 1,
-        "get": "NIDORINA",
-        "give": "NIDORINO",
-        "nickname": "TERRY"
+        "get": "DUGTRIO",
+        "give": "LICKITUNG",
+        "nickname": "GURIO"
       },
       {
         "dialogset": 1,
         "get": "MR_MIME",
-        "give": "ABRA",
-        "nickname": "MARCEL"
+        "give": "CLEFAIRY",
+        "nickname": "MILES"
       },
       {
         "dialogset": 3,
         "get": "BEEDRILL",
         "give": "BUTTERFREE",
-        "nickname": "CHIKUCHIKU"
+        "nickname": "STINGER"
       },
       {
         "dialogset": 1,
-        "get": "SEEL",
-        "give": "PONYTA",
-        "nickname": "SAILOR"
+        "get": "MUK",
+        "give": "KANGASKHAN",
+        "nickname": "STICKY"
       },
       {
         "dialogset": 3,
-        "get": "FARFETCHD",
-        "give": "SPEAROW",
-        "nickname": "DUX"
+        "get": "MEW",
+        "give": "MEW",
+        "nickname": "BART"
       },
       {
         "dialogset": 1,
-        "get": "LICKITUNG",
-        "give": "SLOWBRO",
-        "nickname": "MARC"
+        "get": "PARASECT",
+        "give": "TANGELA",
+        "nickname": "SPIKE"
       },
       {
         "dialogset": 2,
-        "get": "JYNX",
-        "give": "POLIWHIRL",
-        "nickname": "LOLA"
+        "get": "PIDGEOT",
+        "give": "PIDGEOT",
+        "nickname": "MARTY"
       },
       {
         "dialogset": 2,
-        "get": "ELECTRODE",
-        "give": "RAICHU",
-        "nickname": "DORIS"
+        "get": "RHYDON",
+        "give": "GOLDUCK",
+        "nickname": "BUFFY"
       },
       {
         "dialogset": 3,
-        "get": "TANGELA",
-        "give": "VENONAT",
-        "nickname": "CRINKLES"
+        "get": "DEWGONG",
+        "give": "GROWLITHE",
+        "nickname": "CEZANNE"
       },
       {
         "dialogset": 3,
-        "get": "NIDORAN_F",
-        "give": "NIDORAN_M",
-        "nickname": "SPOT"
+        "get": "MACHOKE",
+        "give": "CUBONE",
+        "nickname": "RICKY"
       }
     ],
     "warpCarpets": {