From 24d0c6528da1c22608d69743e3ef8086d27d1adc Mon Sep 17 00:00:00 2001 From: thibautbus <310327033+thibautbus@users.noreply.github.com> Date: Sun, 16 Aug 2026 20:36:41 +0200 Subject: [PATCH 1/5] Translate the stat name in RBY's X-item and vitamin rose! messages Both the player-side and AI-trainer stat-rise messages (X ATTACK/ DEFENSE/etc. and the vitamins) passed the raised stat's name as a raw uppercase Lua string (stat:upper()), bypassing Strings() entirely, so it always rendered in English regardless of the active language even though the surrounding sentence template was already translated. Wrap the substituted stat name in Strings() at every call site (src/inventory/ItemEffects.lua's two player-side messages and src/battle/TrainerAI.lua's AI-trainer X-item message, found in review), reusing the same "ATTACK"/"DEFENSE"/"SPEED"/"SPECIAL"/"HP" keys SummaryMenu.lua's stat labels already look up the same way. --- src/battle/TrainerAI.lua | 2 +- src/inventory/ItemEffects.lua | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/battle/TrainerAI.lua b/src/battle/TrainerAI.lua index 9654029a..7eb4bb32 100644 --- a/src/battle/TrainerAI.lua +++ b/src/battle/TrainerAI.lua @@ -124,7 +124,7 @@ function TrainerAI.useItem(battle, item) elseif X_STAT[item] then local stat = X_STAT[item] enemy.stages[stat] = math.min(6, (enemy.stages[stat] or 0) + 1) - table.insert(msgs, Strings("%s's\n%s rose!", displayName(enemy), stat:upper())) + table.insert(msgs, Strings("%s's\n%s rose!", displayName(enemy), Strings(stat:upper()))) elseif item == "GUARD_SPEC" then enemy.mist = true table.insert(msgs, Strings("%s's\nprotected against\nstat changes!", displayName(enemy))) diff --git a/src/inventory/ItemEffects.lua b/src/inventory/ItemEffects.lua index a900fec9..678cc517 100644 --- a/src/inventory/ItemEffects.lua +++ b/src/inventory/ItemEffects.lua @@ -294,7 +294,7 @@ function ItemEffects.use(data, save, itemId, target, battle, moveIndex, ow) "Nothing happened!") } end b.stages[stat] = cur + 1 - return "consumed", { Strings("%s's\n%s rose!", b.name, stat:upper()) } + return "consumed", { Strings("%s's\n%s rose!", b.name, Strings(stat:upper())) } end -- ItemUseDireHit/ItemUseGuardSpec always set the bit and consume -- the item, even when it is already active @@ -493,7 +493,7 @@ function ItemEffects.use(data, save, itemId, target, battle, moveIndex, ow) -- Spanish ROM puts the stat before the name), so the extracted line -- cannot be filled positionally; the engine wording stands return "consumed", { Strings("%s's %s\nrose!", monName(data, target), - vitaminStat == "hp" and "HP" or vitaminStat:upper()) } + Strings(vitaminStat == "hp" and "HP" or vitaminStat:upper())) } end -- PP UP boosts the move the player picked (ItemUsePPUp's move menu) From c280119d03d1f4bf167cd7efb8bba83ebbf8730d Mon Sep 17 00:00:00 2001 From: thibautbus <310327033+thibautbus@users.noreply.github.com> Date: Sun, 16 Aug 2026 20:36:46 +0200 Subject: [PATCH 2/5] Translate Gold's Light Screen / Reflect rose! messages Same theme as the RBY fix, found while checking whether Gold had the same gap: EFFECT_LIGHT_SCREEN and EFFECT_REFLECT built their "'s SPCL.DEF/DEFENSE rose!" message by raw string concatenation, bypassing Strings() entirely -- unlike most other messages in this file (e.g. "%s\nused %s!" a few lines up), which already go through it. Wrap the whole message template in Strings(), matching that existing pattern; the substituted name still comes from monName() as before. Gold's gen2/Battle.lua has many more messages built the same unwrapped way (fainted!, learned..., missed!, and so on) -- that is the much larger "Battle messages" gap already tracked separately and deliberately left out of this change. --- src/battle/gen2/Battle.lua | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/battle/gen2/Battle.lua b/src/battle/gen2/Battle.lua index 05ea1c2e..e2d5a47e 100644 --- a/src/battle/gen2/Battle.lua +++ b/src/battle/gen2/Battle.lua @@ -2429,7 +2429,7 @@ Battle.MOVE_EFFECTS.EFFECT_LIGHT_SCREEN = function(self, attacker) if (side.lightScreen or 0) > 0 then return fail(self) end side.lightScreen = Battle.SCREEN_TURNS self:emit({ kind = "message", - text = self:monName(attacker) .. "'s SPCL.DEF rose!" }) + text = Strings("%s's SPCL.DEF rose!", self:monName(attacker)) }) end Battle.MOVE_EFFECTS.EFFECT_REFLECT = function(self, attacker) @@ -2437,7 +2437,7 @@ Battle.MOVE_EFFECTS.EFFECT_REFLECT = function(self, attacker) if (side.reflect or 0) > 0 then return fail(self) end side.reflect = Battle.SCREEN_TURNS self:emit({ kind = "message", - text = self:monName(attacker) .. "'s DEFENSE rose!" }) + text = Strings("%s's DEFENSE rose!", self:monName(attacker)) }) end -- engine/battle/move_effects/safeguard.asm:1 From 9423337bcc4d23f36f78e2b46565cd0fdb4cda46 Mon Sep 17 00:00:00 2001 From: thibautbus <310327033+thibautbus@users.noreply.github.com> Date: Sun, 16 Aug 2026 20:48:28 +0200 Subject: [PATCH 3/5] Make the rose! messages' stat name harvestable by the mod catalog tool Strings(stat:upper()) is a dynamic argument -- tools/modkit.py's STRINGS_CALL harvester only matches a literal string right after Strings(/Strings.source(, so it can't discover "ATTACK"/"DEFENSE"/etc. from these call sites. Translation coverage happened to still work only because the same literals are independently harvested from unrelated call sites (MoveEffects.lua's STAT_LABEL, BattleState.lua's literal Strings("ATTACK") calls) -- real but fragile, found in review. Reuse the codebase's existing pattern for exactly this situation (MoveEffects.lua's STAT_LABEL): a local table built at require time with Strings.source(...), which the harvester can see, resolved to a translated label at use time with Strings(TABLE[key]). Adds one such table to ItemEffects.lua (covering its X-item and vitamin call sites, including "hp") and one to TrainerAI.lua. --- src/battle/TrainerAI.lua | 11 ++++++++++- src/inventory/ItemEffects.lua | 14 ++++++++++++-- 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/src/battle/TrainerAI.lua b/src/battle/TrainerAI.lua index 7eb4bb32..e623b61c 100644 --- a/src/battle/TrainerAI.lua +++ b/src/battle/TrainerAI.lua @@ -33,6 +33,15 @@ end local HEAL_AMOUNT = { POTION = 20, SUPER_POTION = 50, HYPER_POTION = 200 } local X_STAT = { X_ATTACK = "attack", X_DEFEND = "defense", X_SPEED = "speed" } +-- Strings.source, not Strings: harvested at require time so the catalog +-- generator can see the literal, same pattern as MoveEffects.lua's +-- STAT_LABEL (#811) -- Strings(stat:upper()) alone is a dynamic argument +-- the harvester can't discover. +local STAT_LABEL = { + attack = Strings.source("ATTACK"), defense = Strings.source("DEFENSE"), + speed = Strings.source("SPEED"), +} + -- The trainer's ai_classes record from the merged registry; the direct -- require covers battles built without a loader. A trainer record's -- aiClass field picks a record other than its own id. @@ -124,7 +133,7 @@ function TrainerAI.useItem(battle, item) elseif X_STAT[item] then local stat = X_STAT[item] enemy.stages[stat] = math.min(6, (enemy.stages[stat] or 0) + 1) - table.insert(msgs, Strings("%s's\n%s rose!", displayName(enemy), Strings(stat:upper()))) + table.insert(msgs, Strings("%s's\n%s rose!", displayName(enemy), Strings(STAT_LABEL[stat]))) elseif item == "GUARD_SPEC" then enemy.mist = true table.insert(msgs, Strings("%s's\nprotected against\nstat changes!", displayName(enemy))) diff --git a/src/inventory/ItemEffects.lua b/src/inventory/ItemEffects.lua index 678cc517..86aa814d 100644 --- a/src/inventory/ItemEffects.lua +++ b/src/inventory/ItemEffects.lua @@ -59,6 +59,16 @@ local STONES = { LEAF_STONE = true, MOON_STONE = true, } +-- Strings.source, not Strings: harvested at require time so the catalog +-- generator can see the literal, same pattern as MoveEffects.lua's +-- STAT_LABEL (#811) -- Strings(stat:upper()) alone is a dynamic argument +-- the harvester can't discover. +local STAT_LABEL = { + hp = Strings.source("HP"), attack = Strings.source("ATTACK"), + defense = Strings.source("DEFENSE"), speed = Strings.source("SPEED"), + special = Strings.source("SPECIAL"), accuracy = Strings.source("ACCURACY"), +} + -- vitamins: stat-exp boosters (ItemUseVitamin) local VITAMINS = { HP_UP = "hp", PROTEIN = "attack", IRON = "defense", CARBOS = "speed", CALCIUM = "special" } @@ -294,7 +304,7 @@ function ItemEffects.use(data, save, itemId, target, battle, moveIndex, ow) "Nothing happened!") } end b.stages[stat] = cur + 1 - return "consumed", { Strings("%s's\n%s rose!", b.name, Strings(stat:upper())) } + return "consumed", { Strings("%s's\n%s rose!", b.name, Strings(STAT_LABEL[stat])) } end -- ItemUseDireHit/ItemUseGuardSpec always set the bit and consume -- the item, even when it is already active @@ -493,7 +503,7 @@ function ItemEffects.use(data, save, itemId, target, battle, moveIndex, ow) -- Spanish ROM puts the stat before the name), so the extracted line -- cannot be filled positionally; the engine wording stands return "consumed", { Strings("%s's %s\nrose!", monName(data, target), - Strings(vitaminStat == "hp" and "HP" or vitaminStat:upper())) } + Strings(STAT_LABEL[vitaminStat])) } end -- PP UP boosts the move the player picked (ItemUsePPUp's move menu) From d6ddf23f976de5e37e2d0e3d0b2c2e4831a57eae Mon Sep 17 00:00:00 2001 From: thibautbus <310327033+thibautbus@users.noreply.github.com> Date: Sun, 16 Aug 2026 21:30:52 +0200 Subject: [PATCH 4/5] Add targeted coverage for the stat-rise message translation fix No existing test could tell a translated stat name apart from a raw stat:upper() that never went through Strings() at all: every rose! message assertion in the suite runs with no catalog loaded, where Strings() is an identity function either way. Loads a real catalog (Strings.load) that translates one stat name at a time and checks it actually reaches the X-item, vitamin, and AI-trainer X-item messages -- ROM-free, over tests/fixture_data. Confirmed this catches the regression: reverting src/inventory/ItemEffects.lua and src/battle/TrainerAI.lua to their pre-fix state fails 5 of 6 checks. --- .../stat_rise_message_translation_test.lua | 73 +++++++++++++++++++ 1 file changed, 73 insertions(+) create mode 100644 tests/engine/stat_rise_message_translation_test.lua diff --git a/tests/engine/stat_rise_message_translation_test.lua b/tests/engine/stat_rise_message_translation_test.lua new file mode 100644 index 00000000..5b8ff08c --- /dev/null +++ b/tests/engine/stat_rise_message_translation_test.lua @@ -0,0 +1,73 @@ +-- The stat name substituted into X-item/vitamin "rose!" messages must +-- itself reach a translation catalog, not just the surrounding sentence +-- template (src/inventory/ItemEffects.lua, src/battle/TrainerAI.lua). +-- With no catalog loaded it stays English (the existing baseline); with +-- one loaded that translates e.g. "ATTACK", the substituted word must +-- change too -- that is the actual bug this suite guards against, which +-- passing/failing sentences alone (as other suites already check) +-- cannot tell apart from a raw stat:upper() that was never wrapped in +-- Strings() at all. +package.path = "./?.lua;./?/init.lua;" .. package.path + +local T = require("tests.modkit") +local Data = T.fixtures.fresh() +local Pokemon = require("src.pokemon.Pokemon") +local SaveData = require("src.core.SaveData") +local ItemEffects = require("src.inventory.ItemEffects") +local TrainerAI = require("src.battle.TrainerAI") +local Strings = require("src.core.Strings") + +local function withCatalog(catalog, fn) + Strings.load({ strings = catalog }) + local ok, err = pcall(fn) + Strings.load(nil) + if not ok then error(err, 0) end +end + +-- ------------------------------------------------------- player X-item + +local save = SaveData.newGame() +local player = { name = "FIXMON", stages = {} } +local xBattle = { player = player, kind = "wild" } + +local _, baseline = ItemEffects.use(Data, save, "X_ATTACK", nil, xBattle) +T.check(baseline[1]:find("ATTACK", 1, true) ~= nil, + "X ATTACK's rose! message names the stat in English with no catalog") + +withCatalog({ ATTACK = "ATTAQUE" }, function() + player.stages.attack = nil + local _, msgs = ItemEffects.use(Data, save, "X_ATTACK", nil, xBattle) + T.check(msgs[1]:find("ATTAQUE", 1, true) ~= nil, + "a catalog translating ATTACK reaches the X ATTACK rose! message") + T.check(msgs[1]:find("ATTACK", 1, true) == nil, + "...and the untranslated English stat name is gone") +end) + +-- --------------------------------------------------------- player vitamin + +local target = Pokemon.new(Data, "FIXMON_A", 10) +withCatalog({ DEFENSE = "DEFENSE_FR" }, function() + local _, msgs = ItemEffects.use(Data, save, "IRON", target) + T.check(msgs[1]:find("DEFENSE_FR", 1, true) ~= nil, + "a catalog translating DEFENSE reaches the IRON (vitamin) rose! message") +end) + +local hpTarget = Pokemon.new(Data, "FIXMON_A", 10) +withCatalog({ HP = "PV" }, function() + local _, msgs = ItemEffects.use(Data, save, "HP_UP", hpTarget) + T.check(msgs[1]:find("PV", 1, true) ~= nil, + "a catalog translating HP reaches the HP UP rose! message") +end) + +-- ------------------------------------------------------- AI trainer X-item + +local enemy = { name = "FOE", stages = {} } +local aiBattle = { enemy = enemy, trainer = { name = "TRAINER" }, data = Data } + +withCatalog({ SPEED = "VITESSE" }, function() + local msgs = TrainerAI.useItem(aiBattle, "X_SPEED") + T.check(msgs[2]:find("VITESSE", 1, true) ~= nil, + "a catalog translating SPEED reaches the AI trainer's X SPEED rose! message") +end) + +T.finish("stat rise message translation") From 467566c7996b1d1fee76668765253c851d0cf896 Mon Sep 17 00:00:00 2001 From: thibautbus <310327033+thibautbus@users.noreply.github.com> Date: Sun, 16 Aug 2026 21:53:30 +0200 Subject: [PATCH 5/5] Cover Gold's Light Screen / Reflect fix in the same test The targeted test only covered the RBY side (ItemEffects.lua, TrainerAI.lua); src/battle/gen2/Battle.lua's EFFECT_LIGHT_SCREEN/ EFFECT_REFLECT fix had no test at all, spotted when asked whether the Gold changes were covered. Adds a minimal MACHOP/TACKLE gen2 fixture (same shape as tests/gen2_move_effects_test.lua's), calls both MOVE_EFFECTS directly, and checks a loaded catalog reaches the whole message template (Gold wraps the full sentence, not just the stat name). Confirmed it catches the regression: reverting Battle.lua to its pre-fix state fails 2 of the suite's now 8 checks. --- .../stat_rise_message_translation_test.lua | 86 +++++++++++++++++-- 1 file changed, 77 insertions(+), 9 deletions(-) diff --git a/tests/engine/stat_rise_message_translation_test.lua b/tests/engine/stat_rise_message_translation_test.lua index 5b8ff08c..83b4da0c 100644 --- a/tests/engine/stat_rise_message_translation_test.lua +++ b/tests/engine/stat_rise_message_translation_test.lua @@ -1,12 +1,13 @@ --- The stat name substituted into X-item/vitamin "rose!" messages must --- itself reach a translation catalog, not just the surrounding sentence --- template (src/inventory/ItemEffects.lua, src/battle/TrainerAI.lua). --- With no catalog loaded it stays English (the existing baseline); with --- one loaded that translates e.g. "ATTACK", the substituted word must --- change too -- that is the actual bug this suite guards against, which --- passing/failing sentences alone (as other suites already check) --- cannot tell apart from a raw stat:upper() that was never wrapped in --- Strings() at all. +-- The stat name substituted into X-item/vitamin "rose!" messages, and +-- Gold's whole Light Screen / Reflect "rose!" messages, must reach a +-- translation catalog, not just the surrounding sentence template (RBY: +-- src/inventory/ItemEffects.lua, src/battle/TrainerAI.lua; Gold: +-- src/battle/gen2/Battle.lua). With no catalog loaded the message stays +-- English (the existing baseline); with one loaded that translates the +-- relevant word(s), the substitution must change too -- that is the +-- actual bug this suite guards against, which passing/failing sentences +-- alone (as other suites already check) cannot tell apart from text that +-- never reached Strings() at all. package.path = "./?.lua;./?/init.lua;" .. package.path local T = require("tests.modkit") @@ -70,4 +71,71 @@ withCatalog({ SPEED = "VITESSE" }, function() "a catalog translating SPEED reaches the AI trainer's X SPEED rose! message") end) +-- ------------------------------------------------------- Gold: Light Screen / Reflect + +local Gen2Battle = require("src.battle.gen2.Battle") +local Gen2Mon = require("src.battle.gen2.Mon") + +local GEN2_DATA = { + pokemon = { + MACHOP = { + id = "MACHOP", index = 66, name = "MACHOP", + baseStats = { hp = 70, attack = 80, defense = 50, speed = 35, + specialAttack = 35, specialDefense = 35 }, + types = { "NORMAL", "NORMAL" }, catchRate = 180, baseExp = 75, + growthRate = "GROWTH_MEDIUM_FAST", genderRatio = 63, + levelMoves = { { level = 1, move = "TACKLE" } }, evolutions = {}, + }, + }, + moves = { + TACKLE = { id = "TACKLE", name = "TACKLE", power = 35, type = "NORMAL", + accuracy = 95, pp = 35, effect = "EFFECT_NORMAL_HIT" }, + }, + type_chart = { types = { NORMAL = { id = "NORMAL", index = 0, + category = "physical" } }, matchups = {} }, + items = {}, +} +local perfectDvs = { attack = 15, defense = 15, speed = 15, special = 15 } +perfectDvs.hp = Gen2Mon.hpDV(perfectDvs) + +local function newGen2Battle() + local player = Gen2Mon.new(GEN2_DATA, "MACHOP", 15, { dvs = perfectDvs }) + player.moves = { { id = "TACKLE", pp = 35, maxPp = 35 } } + local wild = Gen2Mon.new(GEN2_DATA, "MACHOP", 15, { dvs = perfectDvs }) + wild.moves = { { id = "TACKLE", pp = 35, maxPp = 35 } } + return Gen2Battle.new({ data = GEN2_DATA, party = { player }, wild = wild }) +end + +withCatalog({ ["%s's SPCL.DEF rose!"] = "%s voit sa DEF.SPÉ augmenter !" }, + function() + local lsBattle = newGen2Battle() + Gen2Battle.MOVE_EFFECTS.EFFECT_LIGHT_SCREEN(lsBattle, lsBattle.player) + local events = lsBattle:takeEvents() + local found = false + for _, event in ipairs(events) do + if event.kind == "message" + and event.text:find("DEF.SPÉ augmenter", 1, true) then + found = true + end + end + T.check(found, + "a catalog translating Light Screen's rose! message reaches it") + end) + +withCatalog({ ["%s's DEFENSE rose!"] = "%s voit sa DEFENSE augmenter !" }, + function() + local refBattle = newGen2Battle() + Gen2Battle.MOVE_EFFECTS.EFFECT_REFLECT(refBattle, refBattle.player) + local events = refBattle:takeEvents() + local found = false + for _, event in ipairs(events) do + if event.kind == "message" + and event.text:find("DEFENSE augmenter", 1, true) then + found = true + end + end + T.check(found, + "a catalog translating Reflect's rose! message reaches it") + end) + T.finish("stat rise message translation")