mirror of
https://github.com/bryanthaboi/gen1recomp.git
synced 2026-08-21 05:00:43 +02:00
Route a handful of pokered dialogue labels through game.data.text
Several hand-ported scripts carry pokered dialogue as inline English literals instead of reading game.data.text, because the real ROM label was never reachable from data/generated/text.lua: ViridianCityYoungster2OkThenText/CaterpieAndWeedleDescriptionText, TMNotebookText, the SS Anne kitchen cook's three dish lines, and the Viridian fisher's pre-gift line (data/scripts/story5.lua's gift() already read t[label] here, just had a stale comment and a missing fallback). Traced the actual cause carefully -- there are two independent, differently-behaved label scanners in this codebase: - tools/extract/text.py's parse_text_file() requires a label to start with "_" to be collected. This is a real bug (confirmed against a real pret/pokered checkout), but this function has no callers anywhere in the tree and no __main__ entry point -- it looks like dead code left over from an earlier version of the pipeline. - The function that actually produces the shipped label list is text_metadata() in tools/make_rom_manifest.py, which feeds manifest["text"]["labels"], which build_rom_data.py's extract_text() iterates to decode each label straight from the ROM. text_metadata() already uses the permissive regex (no "_" requirement) since commit0f581e2f. So the actual blocker is that the committed tools/rom_manifest.json was stale relative to text_metadata()'s current code, not a source bug. Verified by rebuilding pret/pokered from source with RGBDS (reproducible -- the resulting pokered.gbc/pokeblue.gbc hash to the same canonical SHA-1s gen1recomp already pins, so no cartridge dump was involved anywhere here) and running the real, unmodified make_rom_manifest.py against it: 2595 labels against the committed manifest's 2585, a clean superset containing everything these scripts need. SilphCo2FSilphWorkerFPleaseTakeThisText is the one exception already in the manifest -- confirmed by commit0f581e2f("so many bugs i cannot even breathe") that it was hand-patched in exactly this same targeted way, for issue #393. Fix: - tools/extract/text.py: relaxed parse_text_file()'s regex to match text_metadata()'s, for consistency (no effect on what ships, since nothing calls this function, but no reason to leave a legacy copy of the same scanner out of sync). - Four scripts read the real label first (t[label] or fallback, the established pattern): celadon_eevee.lua, ss_anne_kitchen.lua, viridian_city.lua, story5.lua (comment/fallback only, lookup was already correct). - tools/rom_manifest.json and tools/rom_manifest_blue.json: regenerated for real -- both files are the direct, unedited output of running make_rom_manifest.py/make_blue_manifest.py against a real pret/pokered checkout at POKERED_REVISION, not hand-assembled or reverse-engineered to match. Only safe because of the two fixes below, which exist specifically so a real run doesn't regress anything the previously-committed files had. Diffing a real make_rom_manifest.py run against that previous file (1143 lines out of 45253) found exactly what a naive "just regenerate against whatever pokered HEAD is handy" would have silently broken: - pret/pokered commit 079d1cc92fc3b0ec82bc1418c2b4045bfca84620 (PR #596, 2026-08-06) renamed _SilphCo10FGiovanniILostAgainText/_SilphCo10FPorygonText to _SilphCo11F... (they live in text/SilphCo11F.asm, Giovanni's floor). data/scripts/victories.lua:183 still hardcodes the old name, and extracting under pokered's new name would silently blank Giovanni's "I lost again!?" rematch line. Fix is the pin below: POKERED_REVISION is pinned to the last commit before this rename, so today's generator output matches victories.lua natively, no engine code touched, no generator-side workaround either. Advancing the pin past this commit is a real, welcome future upgrade -- it just needs victories.lua's labels (and anything else's) fixed up in the same change. - trainerPartyOverrides.OPP_CHIEF (Giovanni's Celadon gym team) wasn't produced by any code under tools/ at all. Traced why: pret/pokered's data/trainers/parties.asm has "ChiefData: ; none" -- the Celadon Chief's battle is unused/cut content in the original game, and RomExtractor.lua's own comment confirms gen1recomp reimplements it as a real fight using a hand-authored party for exactly that reason -- no pokered commit, old or new, will ever produce this data. Added a TRAINER_PARTY_OVERRIDES constant to make_rom_manifest.py so this survives every future regeneration automatically; verified it reproduces the committed value byte-for-byte and flows through to Blue/Yellow for free via their existing derive-from-Red path. - trainerHeaders.MtMoonB2F's Super Nerd slot is pre-existing fabricated data, not pokered drift: his event name, EVENT_BEAT_MT_MOON_3_SUPER_NERD, has never existed in pokered at any point in its history (the real name, EVENT_BEAT_MT_MOON_EXIT_SUPER_NERD, has been stable since 2015), nor in gen1recomp's own event_flags.lua; trainerDefeated() checks defeatedTrainers[npc.id] first, the same pattern already used for the Fighting Dojo's Karate Master, so this entry is very likely already inert. field.seafoam also differs from a fresh regeneration (two showObject boulder-toggle IDs in the B3F puzzle, a live gameplay system nobody has verified either value against), and field.tradeArt is new content a fresh extraction produces that was never shipped. None of those three are this PR's problem to fix, but a real generator run has to do something with them regardless -- so make_rom_manifest.py gets a new apply_known_nonreproducible_overrides(), called right after text_metadata()/field_metadata(), that pins MtMoonB2F and seafoam back to what was already shipped and drops tradeArt, each with a comment explaining why and what the real fix looks like (MtMoonB2F needs a Data:seedMtMoonB2FSuperNerd()-style engine seed, not manifest data). Verified this override function was complete and correct -- diffed a real run against the previously-committed file first, empty -- before trusting it to write tools/rom_manifest.json/_blue.json directly; both are now literally that generator's output, not hand-assembled. tools/rom_manifest_yellow.json isn't touched by this PR at all: it already had all ten labels, and make_yellow_manifest.py has no matching override yet for its own field.oldManBattle outlier, so regenerating it for real isn't safe the same way yet. - tools/make_rom_manifest.py: added a POKERED_REVISION pin -- cf621a76d4941c93c078eb38e0880fe8db48ef40, the last pret/pokered commit before the Silph Co rename above, chosen deliberately rather than current HEAD -- and a check_pokered_revision() guard main() calls before generating: fails loudly if --pokered isn't at that commit instead of silently absorbing whatever upstream renames or restructures since, with an explicit --allow-revision-mismatch escape hatch for a deliberate pin bump. That's the intended way this pin moves forward: diff a fresh run against the committed manifest, fix up whatever engine code depends on by exact name, and bump POKERED_REVISION in the same change -- a conscious, reviewable decision instead of a silent contributor default. Wired the same guard into make_blue_manifest.py and make_yellow_manifest.py for their own --pokered/--pokeyellow checkouts; make_yellow_manifest.py gets its own POKEYELLOW_REVISION (e6ba56989b0f2694f393e6924820be11dcc1fbb8, verified here). The pin and its guard live entirely in the generator source, not also embedded as a field in the shipped manifests -- that would be redundant with the .py constant next to it in the same commit, for no protection the guard doesn't already give. - tools/make_rom_manifest.py, make_blue_manifest.py, make_yellow_manifest.py: switched json.dump(..., ensure_ascii= False, ...) to ensure_ascii=True to match how the committed manifests were actually encoded (escaped \uXXXX rather than literal UTF-8). Purely cosmetic -- json.load parses both identically -- but needed so that a fresh make_rom_manifest.py run at POKERED_REVISION now produces tools/rom_manifest.json byte-for-byte (plain diff empty), not just content-equal. One small, pre-existing cosmetic mismatch remains and wasn't chased: a single nested dict, field.cardKeyDoors.doors, has its SILPH_CO_10F/11F keys in natural floor order in the committed file instead of the sort_keys=True lexicographic order everything else in the file uses -- same content either way. - tests/rom_manifest_generator_test.py: new ROM-free unit tests (same style as the existing tests/build_rom_data_cli_test.py, wired into scripts/test.sh as a T0 tier) for check_pokered_revision() and apply_known_nonreproducible_overrides() -- matching/mismatched/ bypassed/unresolvable-checkout revision cases, and that the MtMoonB2F/seafoam/tradeArt overrides land correctly (including alongside a populated map entry, and without erroring when tradeArt is already absent). Doesn't replace the manual real-ROM verification above, which needs an actual pokered/RGBDS toolchain -- but a future typo or logic slip in either function now fails immediately instead of only surfacing next time someone happens to redo that manual check. - Left data/scripts/flavor/silph_co_9f.lua's nurse dialogue (labels also added to both manifests here) untouched code-wise: static command table, not a function, needs its face_player/heal_party/fade state machine restructured to use t[label] safely, and show_text's un-resolved-label fallback prints the label name literally rather than English -- not safe without interactive testing. Checked Yellow's equivalent case (Melanie's House) since it looked like the same shape: it isn't actually broken. tools/make_yellow_manifest.py's YELLOW_EXTRA_TEXT_LABELS already force-includes those eight labels, and a real built dialogue_yellow.lua already has correct French translations for them. tools/rom_manifest_yellow.json also already carries all ten labels this PR adds to Red/Blue. No label changes needed there. Tested: patched parse_text_file() against a real pret/pokered checkout (+11 labels, 0 removed, all clearly dialogue-shaped); rebuilt pokered.gbc/pokeblue.gbc with RGBDS at current pret/pokered HEAD (confirmed hashes to gen1recomp's own canonical SHA-1s) and diffed a fresh make_rom_manifest.py run there against the committed manifest to map out what a full regen at HEAD would need; re-checked out the same pokered checkout at cf621a76 (POKERED_REVISION), rebuilt both ROMs again (same canonical hashes); with apply_known_nonreproducible_overrides() and the ensure_ascii fix in place, ran both generators to a scratch path first and diffed against the then-committed manifests -- empty -- before running them again writing tools/rom_manifest.json/_blue.json directly, so both files are now the generator's literal, unedited output (git diff on that final write: 18 lines moved in rom_manifest.json, exactly the pre-existing SILPH_CO_10F/11F ordering quirk; zero lines changed in rom_manifest_blue.json); rebuilt pokeyellow.gbc the same way, confirmed its canonical hash, and confirmed make_yellow_manifest.py's symbols/ text/trainerHeaders/trainerPartyOverrides also come out byte-for- byte identical to the committed tools/rom_manifest_yellow.json; ran the real build_rom_data.py --only text against all three rebuilt ROMs and confirmed every added label decodes from real ROM bytes matching the English fallback literals exactly; verified check_pokered_revision() actually raises on a deliberate mismatch before relying on it to gate the runs above; python3 tests/rom_manifest_generator_test.py: 9/9 pass; luajit tests/run_engine.lua: 250/250 suites pass.
This commit is contained in:
thibautbus
+121
-1
@@ -12,6 +12,7 @@ import argparse
|
||||
import json
|
||||
import os
|
||||
import re
|
||||
import subprocess
|
||||
import sys
|
||||
import tempfile
|
||||
|
||||
@@ -21,6 +22,43 @@ from extract import (battle_anims, constants, field, font, items, maps, # noqa:
|
||||
pokemon, sprites, text, tilesets, trainers, util)
|
||||
from rom_data import CANONICAL_RED_SHA1, SymbolTable # noqa: E402
|
||||
|
||||
# The pret/pokered commit this manifest was last verified against. Pinned
|
||||
# deliberately just *before* commit 079d1cc92fc3b0ec82bc1418c2b4045bfca84620
|
||||
# (PR #596, 2026-08-06), which renamed
|
||||
# _SilphCo10FGiovanniILostAgainText/_SilphCo10FPorygonText to _SilphCo11F...
|
||||
# -- data/scripts/victories.lua:183 still hardcodes the old name, so
|
||||
# generating against a newer pokered would silently blank Giovanni's
|
||||
# "I lost again!?" rematch line. Advancing this pin past that commit is a
|
||||
# real, welcome upgrade (it picks up everything pokered has fixed since) --
|
||||
# it just needs a fresh run diffed against the committed manifest first, and
|
||||
# victories.lua's (and anything else's) label references fixed up to match
|
||||
# pokered's new names in the same change, same as any other dependency
|
||||
# bump. Without a pin at all, running this generator against whatever a
|
||||
# contributor's checkout happens to be at would silently absorb unaudited
|
||||
# upstream renames like this one with nobody noticing.
|
||||
POKERED_REVISION = "cf621a76d4941c93c078eb38e0880fe8db48ef40"
|
||||
|
||||
|
||||
def _checkout_revision(path):
|
||||
try:
|
||||
return subprocess.run(
|
||||
["git", "rev-parse", "HEAD"], cwd=path, check=True,
|
||||
capture_output=True, text=True).stdout.strip()
|
||||
except (OSError, subprocess.CalledProcessError):
|
||||
return None
|
||||
|
||||
|
||||
def check_pokered_revision(pokered, expected, *, allow_mismatch=False):
|
||||
actual = _checkout_revision(pokered)
|
||||
if actual is None or actual == expected or allow_mismatch:
|
||||
return
|
||||
raise SystemExit(
|
||||
f"{pokered} is at {actual}, but this generator is pinned to "
|
||||
f"{expected}. Diff a fresh run against the committed manifest and "
|
||||
"fix up anything engine code depends on by exact name before "
|
||||
"bumping the revision constant, or pass --allow-revision-mismatch "
|
||||
"to generate anyway.")
|
||||
|
||||
|
||||
def simple_constants(pokered, relpath, stop_at=None):
|
||||
return util.parse_const_block(
|
||||
@@ -628,6 +666,80 @@ def embedded_symbols(data, symbols):
|
||||
}
|
||||
|
||||
|
||||
# Trainer parties this project reimplements that have no data to extract:
|
||||
# CeladonChiefHouse's CHIEF is a talk-only NPC in the ROM (the Celadon Chief
|
||||
# battle is unused/cut content -- pokered's data/trainers/parties.asm has
|
||||
# `ChiefData:` followed by `; none`, zero Pokemon). gen1recomp gives the
|
||||
# CHIEF a real fight after the Hall of Fame (see
|
||||
# data/scripts/celadon_chief_house.lua and src/import/RomExtractor.lua's
|
||||
# `trainerPartyOverrides` handling), so this party is hand-authored rather
|
||||
# than decoded, and belongs in the generator itself rather than only in the
|
||||
# committed manifest, so it survives a regeneration instead of needing to be
|
||||
# manually reapplied every time.
|
||||
TRAINER_PARTY_OVERRIDES = {
|
||||
"OPP_CHIEF": [
|
||||
{"species": "MACHOKE", "level": 41},
|
||||
{"species": "GOLBAT", "level": 41},
|
||||
{"species": "MAROWAK", "level": 43},
|
||||
{"species": "WEEZING", "level": 43},
|
||||
{"species": "PERSIAN", "level": 46},
|
||||
],
|
||||
}
|
||||
|
||||
|
||||
def apply_known_nonreproducible_overrides(texts, field_data):
|
||||
"""Pin a handful of fields a fresh extraction gets right but that this
|
||||
manifest hasn't shipped, or gets differently than what's shipped, and
|
||||
that nobody has verified in-game yet. These are deliberately NOT fixed
|
||||
here -- only kept stable so this generator's output matches the
|
||||
committed manifest byte-for-byte. Each one is a real, separate,
|
||||
diagnosed-but-unresolved discrepancy; drop the matching override once
|
||||
it's actually investigated and the manifest is updated to match.
|
||||
"""
|
||||
# MtMoonB2F's Super Nerd (object index 1) isn't part of pokered's real
|
||||
# def_trainers/trainer table for this map at all -- he's handled by
|
||||
# bespoke script logic (MtMoonB2FDefeatedSuperNerdScript and friends),
|
||||
# so a fresh extraction never produces this slot. This manifest has
|
||||
# always shipped one anyway, under an event name
|
||||
# (EVENT_BEAT_MT_MOON_3_SUPER_NERD) that has never existed in pokered
|
||||
# at any point in its history (the real flag is
|
||||
# EVENT_BEAT_MT_MOON_EXIT_SUPER_NERD, gen1recomp's own
|
||||
# src/save_convert/data/event_flags.lua flag 1401) -- fabricated data,
|
||||
# not pokered drift. OverworldState:trainerDefeated() checks
|
||||
# Game.save.defeatedTrainers[npc.id] before ever consulting this
|
||||
# header (the same pattern already used for the Fighting Dojo's Karate
|
||||
# Master, Data:seedFightingDojoKarateMaster()), so this entry is very
|
||||
# likely already inert -- but removing it outright is a real behavior
|
||||
# change nobody has verified in-game, so it's preserved as shipped
|
||||
# rather than silently dropped. The real fix is a
|
||||
# Data:seedMtMoonB2FSuperNerd()-style engine seed, not manifest data.
|
||||
texts["trainerHeaders"].setdefault("MtMoonB2F", {})[1] = {
|
||||
"after": "_MtMoonB2FSuperNerdTheresAPokemonLabText",
|
||||
"battle": "_MtMoonB2FSuperNerdTheyreBothMineText",
|
||||
"event": "EVENT_BEAT_MT_MOON_3_SUPER_NERD",
|
||||
"won": "_MtMoonB2FSuperNerdOkIllShareText",
|
||||
}
|
||||
|
||||
# Seafoam Islands B3F's cross-floor boulder-toggle IDs (which B3F
|
||||
# object shows/hides when a boulder falls in from B2F above) come out
|
||||
# of a fresh extraction as TOGGLE_SEAFOAM_ISLANDS_B3F_BOULDER_3/_4;
|
||||
# this manifest has always shipped _5/_6 instead. Not yet diagnosed
|
||||
# which is actually correct against a real Seafoam Islands playthrough
|
||||
# (a live gameplay puzzle, not something to change on a guess) --
|
||||
# preserved as shipped pending that investigation.
|
||||
plugged = (
|
||||
field_data["seafoam"]["SEAFOAM_ISLANDS_B3F"]["pluggedByHolesOn"])
|
||||
plugged["holes"][0]["showObject"] = "TOGGLE_SEAFOAM_ISLANDS_B3F_BOULDER_5"
|
||||
plugged["holes"][1]["showObject"] = "TOGGLE_SEAFOAM_ISLANDS_B3F_BOULDER_6"
|
||||
|
||||
# tradeArt is new content the current extractor can already produce
|
||||
# (trade-animation sprite sheets) but that this manifest has never
|
||||
# shipped -- no engine feature consumes it yet. Dropped here rather
|
||||
# than silently included, so this PR's diff stays about text labels;
|
||||
# drop this line once a real trade feature actually needs the field.
|
||||
field_data.pop("tradeArt", None)
|
||||
|
||||
|
||||
def generate(pokered, symbols_path):
|
||||
symbols = SymbolTable(symbols_path)
|
||||
map_order, map_dims = constants.extract_map_constants(pokered)
|
||||
@@ -700,6 +812,7 @@ def generate(pokered, symbols_path):
|
||||
pokered, constants_data["speciesOrder"])
|
||||
texts = text_metadata(pokered)
|
||||
field_data = field_metadata(pokered)
|
||||
apply_known_nonreproducible_overrides(texts, field_data)
|
||||
audio_data = audio_metadata(pokered, symbols, map_order)
|
||||
|
||||
data = {
|
||||
@@ -737,6 +850,7 @@ def generate(pokered, symbols_path):
|
||||
"field": field_data,
|
||||
"audio": audio_data,
|
||||
"battleAnimations": battle_animation_metadata(pokered),
|
||||
"trainerPartyOverrides": TRAINER_PARTY_OVERRIDES,
|
||||
}
|
||||
data["symbols"] = embedded_symbols(data, symbols)
|
||||
return data
|
||||
@@ -749,14 +863,20 @@ def main():
|
||||
parser.add_argument(
|
||||
"--out",
|
||||
default=os.path.join(os.path.dirname(__file__), "rom_manifest.json"))
|
||||
parser.add_argument(
|
||||
"--allow-revision-mismatch", action="store_true",
|
||||
help="generate even if --pokered isn't at POKERED_REVISION "
|
||||
"(intentional pin bumps; audit the diff before committing)")
|
||||
args = parser.parse_args()
|
||||
|
||||
pokered = os.path.abspath(args.pokered)
|
||||
if not os.path.isfile(os.path.join(pokered, "main.asm")):
|
||||
raise SystemExit(f"{pokered} is not a pokered checkout")
|
||||
check_pokered_revision(
|
||||
pokered, POKERED_REVISION, allow_mismatch=args.allow_revision_mismatch)
|
||||
data = generate(pokered, os.path.abspath(args.symbols))
|
||||
with open(args.out, "w", encoding="utf-8", newline="\n") as f:
|
||||
json.dump(data, f, ensure_ascii=False, indent=2, sort_keys=True)
|
||||
json.dump(data, f, ensure_ascii=True, indent=2, sort_keys=True)
|
||||
f.write("\n")
|
||||
print(f"wrote {args.out}")
|
||||
|
||||
|
||||
Reference in New Issue
Block a user