Nine message families in BattleState.lua were plain Lua literals,
bypassing already-extracted, already-translated ROM text labels --
some with a comment right next to them already naming the real label:
- storeCaughtMon(): the new-Pokedex-data line (_ItemUseBallText06) and
the box-transfer line, which used a hardcoded "BILL's PC"/"someone's
PC" as if it were a substituted argument in one shared template --
_ItemUseBallText07/08 are two full, independently translated ROM
strings, not a template with a substituted PC name.
- throwBall(): the dodged-ball and can't-be-caught lines were two
separate Strings() calls; _ItemUseBallText00 is one \f-paged ROM
label covering both. Unlike TextBox.new() (which splits \f itself),
sayNext() goes through the battle queue's own startMessage(), which
only splits on \n/\v -- confirmed live in a real build (the second
sentence overflowed off the box instead of starting a fresh page).
Resolves the label once, splits it the same way TextBox.lua does,
and queues one sayNext per page.
- onFaint(): displayName(battler) runs the enemy name through a
separate Strings("Enemy %s", ...) call, then the shared "%s\nfainted!"
literal added the rest -- but _EnemyMonFaintedText already carries
its own "Enemy" wording, so this passes the raw battler.name and
picks _PlayerMonFaintedText/_EnemyMonFaintedText by battler.isPlayer.
- enter()'s pre-battle black-out message (_PlayerBlackedOutText2, a
\f-paged pair like _ItemUseBallText00 above).
- The AI switch-in withdraw/send-out line and the enemy trainer's
first send-out (3 callsites, one shared by the link-battle intro
path): _AIBattleWithdrawText and _TrainerSentOutText.
Also investigated folding _TrainerAboutToUseText's SHIFT-switch offer
(say() then sayChoice(), both plain Strings(), which the label also
\f-pages) into one romText + sayChoice call the same way. That does
NOT work: tests/engine/trainer_shift_prompt_bug565.lua caught that the
battle queue's own text renderer pages a sayChoice string differently
from TextBox.lua's \f handling that the say()+say() merges above rely
on. Left as two calls, unchanged, with a comment explaining why.
src/ui/SummaryMenu.lua:148 and src/ui/PartyMenu.lua:824 drew mon.status
(PSN/PAR/BRN/FRZ/SLP) as a bare literal, bypassing translation. Unlike
plain text, a mod translates status labels through the statuses content
registry (mod.content.statuses:patch(id, { label = value }), the same
registry src/battle/BattleState.lua:statusLabel already reads in battle.
Route both screens through the same lookup, extracted as
Status.hudLabelFor(statuses, id) and shared with BattleState:statusLabel
so the hudLabel-or-label fallback rule lives in one place, with the raw
status id kept as the fallback when no record overrides it.
Found along the way: Status.RECORDS' five vanilla entries duplicated
hudLabel = label ("FRZ", hudLabel = "FRZ", ...) for no functional
reason. Since hudLabelFor reads hudLabel before label, and
Registry:patch only overrides fields a mod actually passes, a
translation mod's label-only patch (the natural shape for a status
catalog carrying one string per id, with no separate hudLabel data to
patch) was silently shadowed by the untouched vanilla hudLabel -- the
translation was stored but never displayed, in or out of battle. This
affected BattleState:statusLabel too, before this change and
independently of it. Dropped the redundant hudLabel field from all
five vanilla records: it's declared optional in the schema, and
nothing in this codebase ever gives it a value different from label --
setting it here only recreated the shadowing trap for no observed
benefit. Left a comment above Status.RECORDS warning against
re-adding it.
stepHPDrain counts drainHold down to 0 as the last step of every phase
(pixel slide, HP-number step, closing frames) but never let go of the
field afterward, so it sat at 0 -- not nil -- for the rest of the
battle. BattleSafety.inspect uses drainHold ~= nil as its
settled-presentation gate for checkpoint capture, so the very first HP
change in a battle permanently refused every checkpoint after it with
battle_phase_busy, even once the bar had long since caught up.
Only nil the field when the whole drain is actually over (bar pixel,
HP number and the closing-frame hold all settled), not on every
mid-sequence 0 -- a fresh HP change still needs drainHold to read as
busy so BattleSafety keeps refusing captures until that one settles
too.
Confirmed against pokeyellow's engine/battle/core.asm: the Viridian
old man's demo and Oak's Pikachu catch share the same canned
one-item bag (SimulatedInputBattleItemList), quantity 1 -- pokered's
equivalent (OldManItemList, old man only, no Pikachu battle type) is
quantity 50. The port hardcoded x50 for both versions.
GameSpeed is a single fast-forward multiplier applied uniformly to the
whole logic clock -- overworld walking, menu navigation and battle turns
all scale together. A player who wants 4X battles but 1X overworld (so a
cutscene or NPC dialogue doesn't blur past) has no way to get both.
Splits save.options.speed into speedOverworld/speedBattle/speedMenu, each
cycling independently, with an automatic migration so an existing save's
speed choice carries over. Game.speedCategoryInStack resolves which
category is active by walking the state stack (the same idiom
wideBattleInStack/fillScaleInStack already use), so a menu opened mid-
battle inherits battle speed rather than resetting to whatever "menu"
defaults to. Adds a new core.logic_speed hook so a mod can read or
override the resolved multiplier for the current frame regardless of
which category produced it, sitting after the link-play and run-argument
overrides so neither is a seam a mod can defeat.
RFC 0007 status: Proposed.
Wrap the battle stat box, the PC quantity footer and the dex metric
labels in Strings() (SummaryMenu -- and MoveEffects since #811 --
already do this); give the battle menu a 'battle' lookup context so a
translation can shorten FIGHT/ITEM/RUN independently of the pause menu;
align the metric dex rows with the imperial ones and make the No.
prefix translatable; make the title menu's recolor zone follow Menu's
auto-grown width; honor the declared-but-unread boot.title seam,
drawing an explicit versionRibbon as one centered piece. With an empty
catalog every path is pixel-identical to vanilla.
trainers.battleTheme validated and merged onto the trainer record but was
never read: battle music came solely from data.audio.battle[kind] where
kind is computeMusicKind()'s final/gym/trainer/wild. Route both battle-
theme start sites through a single choke point:
- BattleState:playBattleTheme() cues Music.playBattle with the override
(self.trainer.battleTheme via battleTheme()), defaulting to the kind
when unset, so vanilla fights and #782's non-gym Giovanni are unchanged.
- BattleState:enter() and OverworldController:pushBattle() both call it.
- Music.playBattle gains an optional 4th song arg that overrides the kind
default, and real call sites now populate the music.select trainerId.
- Victory jingles stay kind-based: a custom battle theme has no derivable
win-variant.
New ROM-free T2 suite tests/engine/trainer_battle_theme_bug945.lua covers
mod load, override resolution, the choke point, and the nil-override
parity gate.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
bryanthaboi
MaxTomahawk
thibautbus
James Hall
sanjinpepic
AverageConsumer
MaxTomahawk
anxiousintrovert
david
Juan Heredia
Shane McGovern