Merge pull request #1680 from colsonrice/feat/gen2-rom-text

src/core/Game2.lua

@@ -943,6 +943,12 @@ function Game2:load()
   self.data.gen2Scripts = loadGenerated("data/generated/scripts.lua")
   self.data.gen2StdScripts = loadGenerated("data/generated/std_scripts.lua")
   self.data.gen2Text = loadGenerated("data/generated/text.lua")
+  -- The engine's own strings, keyed by the disassembly's label.  gen2Text
+  -- above is the script text and is keyed by bank:address for the overworld
+  -- VM, so the two are different tables and both are loaded.  This one is
+  -- what src/core/RomText.lua reads, which is why it lands on `text`: that
+  -- helper is shared with Gen 1 and looks up data.text[label].
+  self.data.text = loadGenerated("data/generated/rom_text.lua") or {}
   -- data/generated/events.lua: the side tables a script command NAMES rather
   -- than carries -- the phone book, the in-game trades, the elevator's floor
   -- labels, the decoration descriptions.  Keyed for World's own `eventTables`

src/import/RomExtractorGen2.lua

@@ -27,7 +27,7 @@ RomExtractorGen2.__index = RomExtractorGen2
 -- scripts, pokemon, moves, items, marts, encounters, trainers, pokedex,
 -- landmarks, intro movie, menu gfx, title, credits, diploma, trade animation,
 -- audio, stubs
-local STAGE_COUNT = 26
+local STAGE_COUNT = 27
 local Opcodes = require("src.script.gen2.Opcodes")
 
 -- BG palette slots inside one loaded 8-palette set (constants/tileset_constants.asm
@@ -136,6 +136,18 @@ local TEXT_NO_GLYPH = {
   [0x13] = true, [0x15] = true,
 }
 
+-- The three runtime name slots.  PlaceMoveUsersName, PlaceMoveTargetsName and
+-- PlaceEnemysName (home/text.asm:302, :307, :327) swap these for a battler's
+-- own name as the line prints, so they are markers rather than glyphs.  They
+-- decode to the same shape Gen 1 uses, which src/core/RomText.lua already
+-- fills in argument order.  Dropped, SubTookDamageText read "took damage
+-- for" with nothing after it.
+local NAME_SLOT = {
+  ["<USER>"] = "{USER}",
+  ["<TARGET>"] = "{TARGET}",
+  ["<ENEMY>"] = "{ENEMY}",
+}
+
 local ROOF_TILES = 9
 local SPRITEDATA_LENGTH = 6
 
@@ -2596,6 +2608,8 @@ function RomExtractorGen2:decodeGen2Text(bank, address, charmap, buffers)
         out[#out + 1] = ch
       elseif ch == "<……>" or b == 0x56 then
         out[#out + 1] = "……"
+      elseif NAME_SLOT[ch] then
+        out[#out + 1] = NAME_SLOT[ch]
       elseif not ch then
         out[#out + 1] = ("{BYTE:%02X}"):format(b)
       end
@@ -3698,6 +3712,37 @@ function RomExtractorGen2:splashGfx()
   }
 end
 
+-- The engine's own strings, keyed by the label the disassembly gives them.
+--
+-- This is what extractOakSpeech has always done for _OakText1-7: resolve the
+-- label, decode from the cart, key by name.  What is new is that the list of
+-- labels comes from the manifest instead of being written out here, so all of
+-- data/text/ arrives rather than seven strings.  Gen 1 has had the same table
+-- since RomExtractor:extractText; this is the Gen 2 side of it, and it is
+-- what lets src/core/RomText.lua work on Gold and Silver at all.
+--
+-- Written as `rom_text` rather than `text`: data/generated/text.lua is
+-- already the script text, keyed by bank:address for the overworld VM, and
+-- these are a different table with different keys.
+function RomExtractorGen2:extractText()
+  self:beginStage("Dialogue")
+  local charmap = self.manifest.charmap or {}
+  local labels = (self.manifest.text or {}).labels or {}
+  local texts = {}
+  for index, label in ipairs(labels) do
+    local location = self.symbols[label]
+    -- A label the manifest names but the symbol table does not carry would
+    -- be a generator bug, not a cart difference: make_gold_manifest.py
+    -- resolves every one of these before it writes the list.
+    if location then
+      texts[label] = self:decodeGen2Text(location[1], location[2], charmap)
+    end
+    self:tick("Dialogue", index, #labels)
+  end
+  self:write("rom_text", texts)
+  return texts
+end
+
 -- OakSpeech (engine/menus/intro_menu.asm): named _OakText* strings plus the
 -- POKEMON_PROF / CAL trainer pics shown before NamePlayer.  Also pulls
 -- Shrink1/2 pics and the GameFreak splash sheets for the boot cinema.
@@ -6283,6 +6328,7 @@ function RomExtractorGen2:run()
   results.sprites = self:extractSprites()
   results.stdScripts = self:extractStdScripts()
   results.scripts = self:extractScriptsAndText(results.maps, results.stdScripts)
+  results.text = self:extractText()
   results.pokemon = self:extractPokemon()
   results.moves = self:extractMoves()
   results.items = self:extractItems()

src/import/RomImporter.lua

@@ -104,6 +104,12 @@ local VERSION_REQUIRED_FILES_OVERRIDE = {
     "data/generated/sprites.lua", -- OW sheets (Chris + NPCs)
     "data/generated/scripts.lua", -- disassembled map scripts
     "data/generated/text.lua", -- decoded Gen 2 dialogue strings
+    -- The engine's own strings, keyed by label rather than by address.  A
+    -- cache built before RomExtractorGen2:extractText has none, and every
+    -- line that reads through src/core/RomText.lua would silently keep
+    -- printing its Lua fallback, so this re-imports those caches rather than
+    -- bumping CACHE_FORMAT and dragging Red, Blue and Yellow through it too.
+    "data/generated/rom_text.lua",
     "data/generated/pokemon.lua",
     "data/generated/tilesets.lua",
     "data/generated/audio.lua",

tests/gen2_diploma_test.lua

@@ -42,7 +42,7 @@ check(type(RomExtractorGen2.extractDiploma) == "function",
   "RomExtractorGen2:extractDiploma exists")
 check(extractorSource:find("results.diploma = self:extractDiploma()", 1, true)
   ~= nil, "and RomExtractorGen2:run calls it")
-check(extractorSource:find("local STAGE_COUNT = 26", 1, true) ~= nil,
+check(extractorSource:find("local STAGE_COUNT = 27", 1, true) ~= nil,
   "STAGE_COUNT counts the new stage, so the progress bar still ends at 1")
 
 -- The three symbols the stage reads have to be in the curated manifest set or

tests/gen2_rom_text_test.lua

@@ -0,0 +1,139 @@
+-- Gen 2's engine text, the counterpart to Gen 1's data/generated/text.lua.
+--
+-- Gold and Silver had no label-keyed string table at all: the manifests
+-- carried no `text` section, RomExtractorGen2 had no extractText, and
+-- game.data.text was never assigned, so every call through
+-- src/core/RomText.lua fell back to the literal written beside it.  These
+-- cover the three halves of closing that: the manifest names the labels and
+-- resolves every one, the decoder emits the runtime name slots rather than
+-- dropping them, and RomText fills those slots.
+--
+--   GOLD_CACHE="..." luajit tests/gen2_rom_text_test.lua
+--
+-- ROM-free apart from the last section, which reads an imported cache's
+-- rom_text.lua and skips cleanly when there is none.
+package.path = "./?.lua;./?/init.lua;" .. package.path
+
+local S = require("tests.harness").suite("gen2 rom text")
+local check, eq = S.check, S.eq
+
+love = require("tests.love_stub")
+
+local Json = require("src.link.Json")
+local romText = require("src.core.RomText")
+
+local function manifest(path)
+  local file = assert(io.open(path, "r"))
+  local data = assert(Json.decode(file:read("*a")))
+  file:close()
+  return data
+end
+
+-- ---- the label list, and that every label resolves ------------------------
+-- A label the list names but the symbol table cannot place would fail the
+-- import at the Dialogue stage rather than at generation time, so the pairing
+-- is asserted here instead.
+for _, edition in ipairs({ "gold", "silver" }) do
+  local data = manifest("tools/rom_manifest_" .. edition .. ".json")
+  local labels = (data.text or {}).labels or {}
+  check((data.text or {}).labels ~= nil,
+    edition .. " carries a text section")
+  check(#labels > 800,
+    ("%s names %d text labels"):format(edition, #labels))
+
+  local unresolved = {}
+  for _, label in ipairs(labels) do
+    if not data.symbols[label] then unresolved[#unresolved + 1] = label end
+  end
+  eq(#unresolved, 0,
+    ("every %s text label resolves to a symbol (%s)")
+      :format(edition, table.concat(unresolved, ", "):sub(1, 60)))
+
+  local named = {}
+  for _, label in ipairs(labels) do named[label] = true end
+
+  -- data/text/ also holds keyboard layouts and kana tables.  Decoded as text
+  -- they come out as keyboard rows, so make_gold_manifest.TEXT_SOURCES leaves
+  -- their files out.  `BattleText::` is excluded for a different reason: it
+  -- is a bank anchor sharing an address with the first real label under it,
+  -- and its own comment in the disassembly says so.
+  for _, excluded in ipairs({ "NameInputLower", "MailEntry_Uppercase",
+      "Dakutens", "Gen1TrainerClassNames", "BattleText" }) do
+    check(not named[excluded],
+      edition .. " leaves " .. excluded .. " out of the text list")
+  end
+end
+
+-- Both editions describe the same strings; only the addresses move.
+do
+  local gold = (manifest("tools/rom_manifest_gold.json").text or {}).labels or {}
+  local silver =
+    (manifest("tools/rom_manifest_silver.json").text or {}).labels or {}
+  eq(#gold, #silver, "Gold and Silver name the same number of labels")
+  local mismatch
+  for index, label in ipairs(gold) do
+    if silver[index] ~= label then mismatch = label; break end
+  end
+  eq(mismatch, nil, "and the same labels in the same order")
+end
+
+-- ---- the slots RomText fills ----------------------------------------------
+-- decodeGen2Text emits {USER}, {TARGET} and {ENEMY} for the three names
+-- PlaceMoveUsersName / PlaceMoveTargetsName / PlaceEnemysName write at
+-- runtime (home/text.asm:302, :307, :327).  Dropped, the line printed with a
+-- hole where the name belongs.
+do
+  local data = { text = {
+    SubTookDamageText = "The SUBSTITUTE\ntook damage for\v{TARGET}!",
+    WantsToBattleText = "{ENEMY}\nwants to battle!",
+    ConfusedNoMoreText = "{USER}'s\nconfused no more!",
+    SuperEffectiveText = "It's super-\neffective!",
+  } }
+
+  eq(romText(data, "SubTookDamageText", "fallback", "GEODUDE"),
+    "The SUBSTITUTE\ntook damage for\vGEODUDE!",
+    "a {TARGET} slot takes the name the caller passes")
+  eq(romText(data, "WantsToBattleText", "fallback", "FALKNER"),
+    "FALKNER\nwants to battle!", "and so does {ENEMY}")
+  eq(romText(data, "ConfusedNoMoreText", "fallback", "CYNDAQUIL"),
+    "CYNDAQUIL's\nconfused no more!", "and {USER}")
+  eq(romText(data, "SuperEffectiveText", "It's super effective!"),
+    "It's super-\neffective!",
+    "a line with no slot comes back as the cart wrote it")
+  eq(romText(data, "NoSuchLabel", "the engine's own wording"),
+    "the engine's own wording",
+    "and a label the cache does not carry falls back")
+end
+
+-- ---- against a real imported cache ----------------------------------------
+do
+  local cache = os.getenv("GOLD_CACHE")
+  if not cache then
+    local home = os.getenv("HOME") or ""
+    cache = home .. "/Library/Application Support/LOVE/gold-dev/gold"
+  end
+  local path = cache .. "/data/generated/rom_text.lua"
+  local file = io.open(path, "r")
+  if not file then
+    print("  (skipped: no rom_text.lua at " .. path .. ")")
+  else
+    file:close()
+    local texts = assert(loadfile(path))()
+    check(next(texts) ~= nil, "the imported cache carries strings")
+    -- Wording taken from pokegold's data/text/battle.asm, with \n for `line`
+    -- and \v for `cont`, which is what RomExtractorGen2 decodes those to.
+    eq(texts.SuperEffectiveText, "It's super-\neffective!",
+      "SuperEffectiveText comes off the cart hyphenated and broken")
+    eq(texts.NotVeryEffectiveText, "It's not very\neffective…",
+      "NotVeryEffectiveText ends on the ellipsis glyph")
+    eq(texts.StartPerishText, "Both POKéMON will\nfaint in 3 turns!",
+      "StartPerishText names both sides")
+    eq(texts.ButItFailedText, "But it failed!", "and a one-row line is one row")
+    eq(texts.SubTookDamageText, "The SUBSTITUTE\ntook damage for\v{TARGET}!",
+      "SpikesText's neighbour keeps its cont row and its target slot")
+    eq(texts.PlayerHitTimesText, "Hit {NUM} times!",
+      "a text_decimal reads back as {NUM}")
+  end
+end
+
+S.finish()

tests/gen2_trade_gfx_test.lua

@@ -45,7 +45,7 @@ check(type(RomExtractorGen2.extractTrade) == "function",
   "RomExtractorGen2:extractTrade exists")
 check(extractorSource:find("results.trade = self:extractTrade()", 1, true)
   ~= nil, "and RomExtractorGen2:run calls it")
-check(extractorSource:find("local STAGE_COUNT = 26", 1, true) ~= nil,
+check(extractorSource:find("local STAGE_COUNT = 27", 1, true) ~= nil,
   "STAGE_COUNT counts the new stage, so the progress bar still ends at 1")
 
 -- pokegold.sym, bank $0a.  These are also what the cache is checked against

tests/run_tests.lua

@@ -3526,6 +3526,7 @@ runSuites({
   "tests/gen2_decorations_test.lua",
   "tests/gen2_pokerus_test.lua",
   "tests/gen2_common_text_test.lua",
+  "tests/gen2_rom_text_test.lua",
   "tests/gen2_magnet_train_test.lua",
   "tests/gen2_bank_of_mom_test.lua",
   "tests/gen2_trainerhouse_test.lua",

tools/make_gold_manifest.py

@@ -907,9 +907,62 @@ REQUIRED_SYMBOLS = {
 }
 
 
-def embedded_symbols(symbols, pokemon_labels, song_labels=()):
-    """Resolve REQUIRED_SYMBOLS + pic labels + Music_* song headers."""
-    names = set(REQUIRED_SYMBOLS) | set(pokemon_labels) | set(song_labels)
+# The engine's own text, the counterpart to make_rom_manifest.text_metadata.
+#
+# Only these five carry dialogue.  data/text/'s other files are character
+# tables rather than strings: dakutens.asm and name_input_chars.asm /
+# mail_input_chars.asm are keyboard layouts, and unused_gen1_trainer_names.asm
+# is a dead Gen 1 leftover.  Decoding those as text yields keyboard rows and
+# kana runs, so they are left out by name rather than filtered afterwards.
+#
+# None of the five carries an IF DEF(_GOLD) / IF DEF(_SILVER) arm, so the
+# label set is one list for both editions and make_silver_manifest.py inherits
+# it with the addresses re-resolved from pokesilver.sym.
+TEXT_SOURCES = (
+    "battle.asm",
+    "common_1.asm",
+    "common_2.asm",
+    "common_3.asm",
+    "std_text.asm",
+)
+
+
+def text_labels(pokegold):
+    """Every text label in TEXT_SOURCES, in sorted order.
+
+    Unlike Gen 1 there is no `dynamic` map beside this.  pokered's decoder is
+    told which runtime token each label carries; RomExtractorGen2's reads the
+    cart's own TX_RAM / TX_DECIMAL command bytes and emits {STRBUF} / {NUM}
+    itself, so the label alone is enough.
+    """
+    labels = set()
+    for name in TEXT_SOURCES:
+        path = os.path.join(pokegold, "data/text", name)
+        pending = None
+        for _, line in read_asm(path):
+            stripped = line.strip()
+            if not stripped:
+                continue
+            match = re.match(r"(\w+)::?\s*$", stripped)
+            if match:
+                # A label whose next line is another label owns no string of
+                # its own.  `BattleText::` is the one in this set: its own
+                # comment says "used only for BANK(BattleText)", and it shares
+                # an address with the first real label under it, so taking it
+                # would decode that neighbour's string a second time.
+                pending = match.group(1)
+                continue
+            if pending:
+                labels.add(pending)
+                pending = None
+    return sorted(labels)
+
+
+def embedded_symbols(symbols, pokemon_labels, song_labels=(),
+                     text_label_names=()):
+    """Resolve REQUIRED_SYMBOLS + pic labels + songs + text labels."""
+    names = (set(REQUIRED_SYMBOLS) | set(pokemon_labels)
+             | set(song_labels) | set(text_label_names))
     for symbol_name in symbols.by_name:
         # Pokedex entries are split across four banks and the game derives the
         # bank arithmetically from the species id (radio.asm's rlca/maskbits
@@ -1111,6 +1164,7 @@ def generate(pokegold, symbols_path):
             pokemon_labels.append(asset["backLabel"])
 
     songs = music_order(pokegold)
+    text_label_names = text_labels(pokegold)
     sfx = sfx_order(pokegold)
 
     # Index 0 is NO_ITEM, so the parsed list is already 1-based on item id.
@@ -1202,6 +1256,9 @@ def generate(pokegold, symbols_path):
             "battleAnimBgPaletteOrder": battle_anim_bg_pals,
             "battleAnimObPaletteOrder": battle_anim_ob_pals,
         },
+        # Label -> decoded string is built at import time from these, the
+        # same way Gen 1 builds data/generated/text.lua from its own list.
+        "text": {"labels": text_label_names},
         "charmap": charmap(pokegold),
         "fontCharmap": font_extract.parse_charmap(pokegold),
         "pokemonAssets": assets,
@@ -1210,7 +1267,8 @@ def generate(pokegold, symbols_path):
         "maps": {name: map_groups[name] for name in map_order},
         "tilesets": {name: {} for name in tilesets},
     }
-    data["symbols"] = embedded_symbols(symbols, pokemon_labels, songs)
+    data["symbols"] = embedded_symbols(
+        symbols, pokemon_labels, songs, text_label_names)
     return data