fix(game3): gate the level-up stat window on the EXP sequence (#2324)

The FRLG level-up stat window opened and the sequence kept pumping, so the
box outlived its own step: it stopped taking input (routing is phase-gated),
never closed, and drew over the win/money text.

- exp_seq: update() now waits while the window is open (mirroring the
  already-correct busy()), and finish()/reset() tear the window down silently
  so a stale onDone cannot advance a sequence that no longer owns it.
- stat_growth: close(opts) gains opts.silent for teardown callers.
- init: the input-routing phase list moves into STAT_WINDOW_PHASES behind
  Battle.statWindowPhase(), and Battle.update() tears down a window whose
  phase can no longer dismiss it.
- ui: the stat-window draw keys off the same predicate, so input and drawing
  can never disagree.

Tests: the "In-Battle Level Up Stat Growth Window" block in
tests/game3_battle_switch_and_faint_test.lua now pumps ExpSeq.update() while
the box is open, and a new CI-covered tests/engine suite pins the same
regression.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
Shane McGovern
2026-09-19 13:26:02 +01:00
parent da6fe8877f
commit 7ec1d9be0c
6 changed files with 221 additions and 13 deletions
+24 -8
View File
@@ -7,6 +7,17 @@ local Pokemon = require("src.core.game3.pokemon")
local ExpSeq = {}
local function stat_growth()
local ok, SG = pcall(require, "src.ui.game3.stat_growth")
if ok and SG then return SG end
return nil
end
local function stat_window_open()
local SG = stat_growth()
return (SG and SG.isOpen and SG.isOpen()) and true or false
end
ExpSeq._steps = nil
ExpSeq._i = 1
ExpSeq._waiting = false
@@ -34,18 +45,13 @@ function ExpSeq.reset()
if okA and okS and Audio.stopSe and SE and SE.SE_EXP then
Audio.stopSe(SE.SE_EXP)
end
local okSG, StatGrowth = pcall(require, "src.ui.game3.stat_growth")
if okSG and StatGrowth and StatGrowth.close then
StatGrowth.close()
end
local StatGrowth = stat_growth()
if StatGrowth and StatGrowth.close then StatGrowth.close({ silent = true }) end
LearnMove.reset()
end
function ExpSeq.busy()
local okSG, StatGrowth = pcall(require, "src.ui.game3.stat_growth")
if okSG and StatGrowth and StatGrowth.isOpen and StatGrowth.isOpen() then
return true
end
if stat_window_open() then return true end
return ExpSeq._steps ~= nil or LearnMove.busy()
end
@@ -58,6 +64,10 @@ local function finish()
ExpSeq._i = 1
ExpSeq._waiting = false
ExpSeq._waitingMsg = false
-- The step that owned the stat window is over; drop it without letting its
-- stale callback advance a sequence that has already ended (#2324).
local StatGrowth = stat_growth()
if StatGrowth and StatGrowth.close then StatGrowth.close({ silent = true }) end
local okA, Audio = pcall(require, "src.core.game3.audio")
local okS, SE = pcall(require, "src.core.game3.se_ids")
if okA and okS and Audio.stopSe and SE and SE.SE_EXP then
@@ -334,6 +344,12 @@ function ExpSeq.update()
if LearnMove.busy() then
return false
end
-- The level-up stat window waits for the player; its onDone callback clears
-- _waiting and advances. Without this the sequence ran straight past the
-- open window, leaving it on screen (#2324).
if stat_window_open() then
return false
end
if not Anim.busy() then
advance()
else
+24 -2
View File
@@ -44,6 +44,21 @@ Battle._residualStepState = nil
local D = {}
-- Phases in which the level-up stat window can still be dismissed by the
-- player. Input routing and drawing both key off this, so the window can never
-- linger somewhere it can no longer be dismissed (#2324).
local STAT_WINDOW_PHASES = {
awarding = true,
evolving = true,
switching = true,
shift_prompt = true,
catch_nickname_prompt = true,
}
function Battle.statWindowPhase()
return STAT_WINDOW_PHASES[Battle._phase] == true
end
-- pokefirered/src/battle_interface.c:2168
local function hp_bar_red(hp, maxHp)
local ok, BattleChrome = pcall(require, "src.ui.game3.battle_chrome")
@@ -2140,6 +2155,14 @@ Battle.finishCatchFlow = finish_catch_flow
function Battle.update(dt, game)
if not Battle._active then return end
-- A stat window whose phase can no longer dismiss it must not linger (#2324).
if not Battle.statWindowPhase() then
local StatGrowth = package.loaded["src.ui.game3.stat_growth"]
if StatGrowth and StatGrowth.isOpen and StatGrowth.isOpen() then
StatGrowth.close({ silent = true })
end
end
local input = game and game.input
local Pokedex = package.loaded["src.ui.game3.pokedex"]
if Pokedex and Pokedex.isOpen and Pokedex.isOpen() then
@@ -2206,8 +2229,7 @@ function Battle.update(dt, game)
end
-- Choice input during award / shift prompt / evolution learn-move prompts / catch nickname prompt / evolving
if (Battle._phase == "awarding" or Battle._phase == "evolving" or Battle._phase == "switching"
or Battle._phase == "shift_prompt" or Battle._phase == "catch_nickname_prompt")
if Battle.statWindowPhase()
and not Battle._auto and game and game.input then
local EvolutionScene = package.loaded["src.ui.game3.evolution_scene"]
if EvolutionScene and EvolutionScene.isOpen and EvolutionScene.isOpen() then
+11 -1
View File
@@ -24,6 +24,15 @@ local BallOpen = require("src.core.game3.battle.ball_open")
local Ui = {}
-- The stat window may only be on screen while the battle is in a phase that can
-- still dismiss it; init.lua owns the list (#2324). Resolved lazily because
-- init.lua requires this module.
local function stat_window_phase()
local Battle = package.loaded["src.core.game3.battle.init"]
if not (Battle and Battle.statWindowPhase) then return true end
return Battle.statWindowPhase()
end
Ui._queue = {}
Ui._showing = false
Ui._headless = false
@@ -2032,7 +2041,8 @@ function Ui.draw(w, h)
end
local StatGrowth = package.loaded["src.ui.game3.stat_growth"]
if StatGrowth and StatGrowth.isOpen and StatGrowth.isOpen() and StatGrowth.draw then
if StatGrowth and StatGrowth.isOpen and StatGrowth.isOpen() and StatGrowth.draw
and stat_window_phase() then
StatGrowth.draw()
end
+4 -2
View File
@@ -39,7 +39,9 @@ function StatGrowth.isOpen()
return StatGrowth._open
end
function StatGrowth.close()
--- opts.silent drops the window without firing its onDone callback, for callers
--- tearing down a step that no longer exists (#2324).
function StatGrowth.close(opts)
local wasOpen = StatGrowth._open
StatGrowth._open = false
StatGrowth._mon = nil
@@ -48,7 +50,7 @@ function StatGrowth.close()
StatGrowth._page = 1
local cb = StatGrowth._onDone
StatGrowth._onDone = nil
if wasOpen and cb then cb() end
if wasOpen and cb and not (opts and opts.silent) then cb() end
end
function StatGrowth.handleInput(input)
@@ -0,0 +1,127 @@
-- #2324: the level-up stat window gates the EXP sequence; it is never run past.
--
-- pokefirered draws the lvlup box from Cmd_drawlvlupbox and then *waits* for the
-- player (battle_script_commands.c: Cmd_drawlvlupbox ends the command, the
-- controller only returns after LvlUpBoxInput sees A/B). The FRLG rewrite
-- opened the box and kept pumping, so the box outlived its own step: it could
-- no longer be dismissed (input routing is phase-gated) and sat on screen over
-- the win/money text.
--
-- luajit tests/engine/battle_levelup_statwindow_bug2324.lua
package.path = "./?.lua;./?/init.lua;" .. package.path
local T = require("tests.modkit")
local check, eq = T.check, T.eq
local Experience = require("src.core.game3.battle.experience")
local ExpSeq = require("src.core.game3.battle.exp_seq")
local StatGrowth = require("src.ui.game3.stat_growth")
local Ui = require("src.core.game3.battle.ui")
local Anim = require("src.core.game3.battle.anim")
local Battle = require("src.core.game3.battle.init")
local A_PRESS = { wasPressed = function(_, key) return key == "a" end }
local NO_PRESS = { wasPressed = function() return false end }
-- A mon one award away from a level, so the sequence always grows stats.
local function levelUpMon()
return {
species = 1,
level = 5,
hp = 20,
maxHp = 20,
attack = 10,
defense = 10,
spAtk = 12,
spDef = 12,
speed = 9,
exp = Experience.expForLevel(3, 5),
growthRate = 3,
}
end
--- Runs the sequence up to the point the stat window opens.
--- Returns the pushMsg log and the ExpSeq step index at that moment.
local function pumpToStatWindow()
local mon = levelUpMon()
local res = Experience.apply(mon, 100)
local messages = {}
local awards = { { mon = mon, result = res, partyIndex = 1 } }
Ui.reset({ headless = false })
check(ExpSeq.begin(awards, function(t) messages[#messages + 1] = t end, nil,
{ headless = false }), "the level-up award starts an EXP sequence")
-- exp-gain text, then the bar, then the grew-to-LV text
for _ = 1, 4 do
ExpSeq.update()
Ui._showing = false
Ui._queue = {}
Anim.reset({ headless = false })
end
check(StatGrowth.isOpen(), "the stat window opened on level up")
eq(StatGrowth._page, 1, "the stat window starts on Page 1 (diffs)")
return messages, ExpSeq._i, mon, res
end
do
local messages, stepBefore = pumpToStatWindow()
local msgsBefore = #messages
-- The box waits for the player: idle pumps must not move the sequence.
for i = 1, 20 do
eq(ExpSeq.update(), false, "pump " .. i .. " reports busy while the box is open")
end
eq(ExpSeq._i, stepBefore, "idle pumps did not advance the sequence")
eq(#messages, msgsBefore, "idle pumps pushed no further battle text")
check(StatGrowth.isOpen(), "the box is still open after 20 idle pumps")
eq(StatGrowth._page, 1, "the box is still on Page 1 after 20 idle pumps")
-- Page 2 is still a wait.
check(StatGrowth.handleInput(A_PRESS), "A consumed on Page 1")
eq(StatGrowth._page, 2, "the box flipped to Page 2 (new values)")
check(StatGrowth.isOpen(), "the box is still open on Page 2")
for _ = 1, 20 do
eq(ExpSeq.update(), false, "pump reports busy while Page 2 is open")
end
eq(ExpSeq._i, stepBefore, "Page 2 idle pumps did not advance the sequence")
eq(#messages, msgsBefore, "Page 2 idle pumps pushed no further battle text")
-- The second A press is what releases the sequence.
check(StatGrowth.handleInput(A_PRESS), "A consumed on Page 2")
check(not StatGrowth.isOpen(), "the second A press closed the box")
eq(ExpSeq._i, stepBefore + 1, "the sequence advanced only once the box closed")
-- And it can be driven to the end from there without the box coming back.
local guard = 0
while not ExpSeq.update() and guard < 200 do
guard = guard + 1
Ui._showing = false
Ui._queue = {}
Anim.reset({ headless = false })
end
check(guard < 200, "the sequence finishes after the box is dismissed")
check(not StatGrowth.isOpen(), "no stat window survives the finished sequence")
check(ExpSeq.busy() == false, "ExpSeq.busy() is false once the sequence is done")
end
do
-- A box whose phase can no longer dismiss it must not linger: input routing is
-- phase-gated, so a box left open outside those phases would be undismissable
-- and would draw over the win/money text.
local _, _, mon, res = pumpToStatWindow()
local savedActive, savedPhase, savedHeadless = Battle._active, Battle._phase, Battle._headless
Battle._active, Battle._phase, Battle._headless = true, nil, true
StatGrowth.open(mon, res.steps[1].oldStats, res.steps[1].newStats, function()
error("a torn-down stat window must not fire its onDone callback")
end)
check(StatGrowth.isOpen(), "a stale box is open before Battle.update")
Battle.update(1 / 60, { input = NO_PRESS })
check(not StatGrowth.isOpen(), "Battle.update tears down a box outside its phases")
Battle._active, Battle._phase, Battle._headless = savedActive, savedPhase, savedHeadless
end
T.finish("battle_levelup_statwindow_bug2324")
@@ -568,6 +568,17 @@ do
check(StatGrowth.isOpen(), "StatGrowth window opened on level up")
eq(StatGrowth._page, 1, "StatGrowth starts on Page 1 (diffs)")
-- #2324: the sequence must WAIT on the window, not run past it.
local stepBefore = ExpSeq._i
local msgsBefore = #messages
for _ = 1, 20 do
eq(ExpSeq.update(), false, "ExpSeq.update() reports busy while the stat window is open")
end
eq(ExpSeq._i, stepBefore, "ExpSeq did not advance past the open stat window")
eq(#messages, msgsBefore, "no battle text pushed while the stat window is open")
check(StatGrowth.isOpen(), "StatGrowth still open after idle pumps")
eq(StatGrowth._page, 1, "StatGrowth still on Page 1 after idle pumps")
-- Advance to Page 2
local fakeInput = {
wasPressed = function(self, key) return key == "a" end,
@@ -576,9 +587,29 @@ do
check(StatGrowth.isOpen(), "StatGrowth still open on Page 2")
eq(StatGrowth._page, 2, "StatGrowth on Page 2 (new values)")
-- #2324: Page 2 is still a wait
for _ = 1, 20 do
eq(ExpSeq.update(), false, "ExpSeq.update() reports busy on stat window Page 2")
end
eq(ExpSeq._i, stepBefore, "ExpSeq still did not advance on Page 2")
check(StatGrowth.isOpen(), "StatGrowth still open after idle pumps on Page 2")
eq(#messages, msgsBefore, "still no battle text while Page 2 is up")
-- Confirm Page 2 -> closes window and advances sequence
StatGrowth.handleInput(fakeInput)
check(not StatGrowth.isOpen(), "StatGrowth closed after confirmation")
eq(ExpSeq._i, stepBefore + 1, "ExpSeq advanced only once the window closed")
-- #2324: a window whose phase can no longer dismiss it must be torn down.
local savedActive, savedPhase, savedHeadless = Battle._active, Battle._phase, Battle._headless
Battle._active, Battle._phase, Battle._headless = true, nil, true
StatGrowth.open(mon, res.steps[1].oldStats, res.steps[1].newStats, function()
error("[FAIL] a torn-down stat window must not fire its onDone callback")
end)
check(StatGrowth.isOpen(), "stale stat window open before Battle.update")
Battle.update(1 / 60, { input = fakeInput })
check(not StatGrowth.isOpen(), "Battle.update closed a stat window outside its phases")
Battle._active, Battle._phase, Battle._headless = savedActive, savedPhase, savedHeadless
end
print("\nALL BATTLE SWITCH & FAINT TESTS PASSED! (100%)")