Four message families in OverworldController.lua were plain Lua
literals instead of their already-extracted, already-translated ROM
text labels:
- applyFieldPoison()'s faint message: the third of three collapsed
"%s\nfainted!" ROM strings (the other two, in BattleState.lua, are
fixed in the previous commit) -- routed through _PokemonFaintedText.
- useSoftboiledFieldMove()'s two outcome messages: _ItemUseNoEffectText
and _PotionText, the exact labels ItemEffects.lua's real potion
message already uses, including _PotionText's second slot (the
actual amount healed) the old literal never showed at all.
- tryHiddenObject()'s two hidden-item finds: _FoundHiddenItemText.
- The normal item-ball pickup path's two finds (one Yellow-only
bag-full variant): a comment already named _FoundItemText
("FoundItemText: text_far, sound_get_item_1, text_end").
Both found-item labels lead with a {PLAYER} token that romText
auto-fills from a 2-arg call (player name, item name) in the same
order the literal already used. The fallback text for both is plain
"%s found\n%s!", matching the original literal's shape exactly --
an earlier version of this fix used a "{PLAYER} found\n%s!" fallback
that relied on TextBox.new's later TextBox.substitute pass to resolve
{PLAYER}, which works but needlessly made the fallback path depend on
a downstream call instead of being self-contained.
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.
A fixed cutoff (e.g. "<=200 is ink") only makes sense for sprites with a light background to split against; a mostly-opaque 16x16 icon has almost no pixel above that cutoff, so every such icon collapsed onto the same "all ink" hash and was flagged as a near-duplicate of anything else that also collapsed -- which was most of them, boulder.png included. Thresholding against the image's own mean keeps the split meaningful (and roughly balanced) no matter how light or dark the source is.
BoxMenu.lua's release() pushes both its "Once released...OK?" prompt
and its Yellow-only "Pikachu looks unhappy" message through
TextBox.new(game, (t._X or Strings(...)):gsub(...)) -- gsub returns
two values (the text and a substitution count), and since the gsub
call is the last argument in the TextBox.new(...) call with nothing
after it, Lua expands both into the call: the count lands in
TextBox.new's third parameter, onDone. TextBox.lua later calls
onDone() once the box is dismissed; a number is not callable, so
every release of your own caught Pikachu in Yellow crashed --
regardless of its nickname (unlike the separate %-escape gsub bug,
this one needs no special save content, ordinary play reaches it
every time).
Fixed by wrapping the gsub call in an extra pair of parens, which
truncates it to its first return value only -- the same fix already
applied to the neighboring _OnceReleasedText/_MonWasReleasedText
lines on the (separate, unmerged) fix/route-more-messages-through-romtext
branch, where this exact bug shape was first noticed while adding a
third callsite with the same pattern.
tests/engine/pikachu_unhappy_release_crash.lua: registers a fake
Data.pokemon.PIKACHU cloned from the fixture species (ROM-free) so
the species == "PIKACHU" check can be exercised, drives the real
interactive release flow in Yellow on a mon owned by the player, and
confirms the crash. Verified failing pre-fix (exact same
"attempt to call field 'onDone' (a number value)" error) and passing
post-fix.
Neither tests/parity_status_true_color.lua (SGB recolor rectangle) nor
tests/parity_party_icon_mirror.lua (icon mirroring) check the drawn
status text, so this fix had no coverage. Drive SummaryMenu:draw() and
PartyMenu:draw() with a mod-patched statuses registry and check the
patched label reaches Font.draw instead of the raw status id, plus a
vanilla case confirming the no-mod fallback is unchanged.
Also cover the hudLabel-shadowing bug directly through the real
Registry:patch (not a hand-built table): a label-only patch, the exact
shape a translation mod would send, must reach Status.hudLabelFor for
all five vanilla ids. Confirmed both regressions: reverting
src/ui/*.lua and src/battle/*.lua to dev's pre-fix content fails 2 of
the draw-site checks; reverting only the vanilla hudLabel removal in
Status.lua fails the 3 checks whose French label differs from English
(FRZ/BRN/SLP).
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.
The previous drawCellBottom calls fired only for cells containing a
tracked entity. While walking this worked acceptably because the
sprite's sub-pixel tween kept the visual overlap plausible, but while
standing still the sprite is pixel-aligned with the cell and the opaque
leaf-edge pixels in the grass bottom row paint over the player's feet.
This change removes the per-entity isGrassCell checks and replaces them
with a single post-sprite pass that overdraws every visible grass cell.
TileRenderer:
- ensureWindow now builds grassCells (all paths) and grassBatch (DMG/SGB
shader path) alongside winBatch during the existing tile scan loop.
A grassSeen table deduplicates cells so each cx/cy pair is only
recorded once despite having two bottom-row tiles.
- drawGrassOverdraw: DMG/SGB draws the grassBatch under color0KeyShader
in one call; GBC iterates grassCells and calls drawCellBottomRaw per
cell (pre-keyed images can't share a SpriteBatch).
- markGrassOverdrawRedraw: iterates grassCells and calls
markCellBottomRedraw for the post-zone OBP-replay pass (GBC only).
- releaseBatches cleans up grassBatch and grassCells.
OverworldController (flat path):
- Entity loop draws sprites only; grass overdraw fires once after the
loop via drawGrassOverdraw + markGrassOverdrawRedraw.
OverworldController (tilt path):
- Grass cells are injected into the billboard sort queue keyed on the
world-pixel foot of each cell's bottom tile row (cy*16+16), so they
depth-sort correctly against entities at different y positions. Each
grass cell billboards via drawCellBottomRaw inside the upright pass.
Fixes: standing-in-tall-grass feet overdraw (Gen 2 confirmed, Gen 1
improved); NPCs and Pikachu follower in grass benefit automatically.
Parity test: tests/parity_grass_seam.lua 10/10, engine 228/228.
The takeover stop in Sound.playMove moved after the new source starts in
cfa84063, but an equal-id replay reuses the cached source, so the stop
killed the sound it had just restarted.
thibautbus
bryanthaboi
Dorian Burton
Adrian Castro
1jamie
AverageConsumer