diff --git a/data/scripts/story2.lua b/data/scripts/story2.lua index b7e40f3d..96904d35 100644 --- a/data/scripts/story2.lua +++ b/data/scripts/story2.lua @@ -206,6 +206,13 @@ M.PALLET_TOWN = { end local function escortToLab(oak) + -- PalletMovementScript_OakMoveLeft + -- (engine/overworld/auto_movement.asm) starts MUSIC_MUSEUM_GUY + -- when the escort begins in Yellow. Until then, Pallet Town plays + -- after the battle; Red/Blue leave MUSIC_MEET_PROF_OAK playing. + if yellow then + Music.play(game.data, "Music_MuseumGuy") + end local numSteps = x - 10 if oak and numSteps > 0 then ow:scriptMove(oak, "left", numSteps, function() @@ -249,14 +256,24 @@ M.PALLET_TOWN = { function() -- Oak turns toward the horizontally adjacent grass (left exit -- looks right, right exit looks left -- the - -- EVENT_PLAYER_AT_RIGHT_EXIT_TO_PALLET_TOWN branch) + -- EVENT_PLAYER_AT_RIGHT_EXIT_TO_PALLET_TOWN branch). + -- In pokeyellow, PalletTownOakGreetsPlayerScript turns Oak and + -- PalletTownPikachuBattleScript arms the battle on the next + -- overworld iteration. OverworldLoopLessDelay + -- (home/overworld.asm) burns two DelayFrame calls at the top + -- of each iteration and calls RunMapScript before checking + -- wCurOpponent, so those two DelayFrame calls are what keep + -- Oak's turn on screen before the battle check fires. if oak then oak.facing = x == 10 and "right" or "left" end - local battle = BattleState.newWild(game, "PIKACHU", 5) - battle:makeOldManDemo("PROF.OAK") - battle.onFinish = function() - afterPikaBattle() - end - game.stack:push(battle) + hold(2, nil, function() + local battle = BattleState.newWild(game, "PIKACHU", 5) + battle:makeOldManDemo("PROF.OAK") + battle.onFinish = function() + afterPikaBattle() + end + -- Use the standard wild-battle entry transition. + Commands.pushBattle(ctx, battle) + end) end)) end diff --git a/src/battle/BattleState.lua b/src/battle/BattleState.lua index 4d809bc1..99af2a74 100644 --- a/src/battle/BattleState.lua +++ b/src/battle/BattleState.lua @@ -2203,8 +2203,13 @@ function BattleState:openOldManBag() self.afterQueue = "menu" self:ui(function() local list + -- The canned bag (POKE_BALL, not read from the player's real + -- inventory) differs by version: pokered's OldManItemList has 50 + -- POKé BALLs; pokeyellow's SimulatedInputBattleItemList, shared by + -- the Viridian tutorial and Oak's catch, has one. + local qty = require("src.core.GameVersion").isYellow() and "x1" or "x50" list = ListMenu.new(game, "ITEMS", { - { value = "POKE_BALL", label = Strings("POKé BALL"), right = "x50" }, + { value = "POKE_BALL", label = Strings("POKé BALL"), right = qty }, }, { script = function(l) l.scriptTimer = (l.scriptTimer or 0) + 1 diff --git a/src/script/Commands.lua b/src/script/Commands.lua index dba4844b..89a35a2d 100644 --- a/src/script/Commands.lua +++ b/src/script/Commands.lua @@ -280,6 +280,22 @@ function Commands.save_end_battle_text(ctx, textId) ctx.endBattleText = TextBox.substitute(ctx.game, text or textId) end +-- Route scripted battles through the standard entry transition. In the +-- originals, InitWildBattle (engine/battle/init_battle.asm) always calls +-- DoBattleTransitionAndInitBattleVariables (engine/battle/core.asm), with +-- no old-man or Pikachu-demo exception; BattleTransition then selects the +-- wipe for the battle kind. Some tests provide only a partial overworld +-- double, so retain a logged fallback even though it skips the transition +-- and battle music. +function Commands.pushBattle(ctx, battle) + if ctx.overworld and ctx.overworld.pushBattle then + ctx.overworld:pushBattle(battle) + else + Logger.warn("pushBattle: no overworld:pushBattle, skipping the transition wipe") + ctx.game.stack:push(battle) + end +end + -- start_battle "wild" species level | start_battle "trainer" OPP_CLASS partyIndex function Commands.start_battle(ctx, kind, a, b) local BattleState = require("src.battle.BattleState") @@ -312,19 +328,8 @@ function Commands.start_battle(ctx, kind, a, b) end runner:resume() end - -- Every battle enters through the transition wipe, script-driven ones - -- included: BattleTransition (engine/battle/battle_transitions.asm:1) runs - -- from DoBattleTransitionAndInitBattleVariables for all of them, and - -- GetBattleTransitionID_WildOrTrainer picks the style from the battle kind. - -- Pushing the BattleState straight onto the stack skipped the wipe - -- entirely, so every scripted trainer -- gym leaders, the rival, Giovanni -- - -- and every scripted wild battle simply cut to the battle screen. The - -- trainer-sight path already went through pushBattle; this one did not. - if ctx.overworld and ctx.overworld.pushBattle then - ctx.overworld:pushBattle(battle) - else - ctx.game.stack:push(battle) - end + -- A direct stack push would skip the battle-entry transition. + Commands.pushBattle(ctx, battle) runner:yield() end @@ -818,15 +823,7 @@ function Commands.old_man_demo(ctx, outcome) local battle = BattleState.newWild(ctx.game, om.species, om.level) battle:makeOldManDemo(nil, outcome == "fail") battle.onFinish = function() runner:resume() end - -- InitWildBattle calls DoBattleTransitionAndInitBattleVariables - -- unconditionally (core.asm:6699) -- there is no BATTLE_TYPE_OLD_MAN - -- special case -- so the catch tutorial gets the wipe like any other - -- wild battle - if ctx.overworld and ctx.overworld.pushBattle then - ctx.overworld:pushBattle(battle) - else - ctx.game.stack:push(battle) - end + Commands.pushBattle(ctx, battle) runner:yield() end diff --git a/src/world/OverworldController.lua b/src/world/OverworldController.lua index a345ba6c..87b40ae3 100644 --- a/src/world/OverworldController.lua +++ b/src/world/OverworldController.lua @@ -453,9 +453,10 @@ function OverworldState:setMap(mapId, x, y, facing, opts) -- walks out of the warp, not beside him (#863) require("src.world.PikachuFollower").onMapEntered(Game, self, opts, true) - -- opts.keepMusic: the Oak-escort warp keeps MUSIC_MEET_PROF_OAK - -- playing into the lab (BIT_NO_MAP_MUSIC in wStatusFlags7); - -- keepMusicOnce is the play_music opts.keep one-shot of the same bit + -- opts.keepMusic preserves the Oak-escort song across the lab warp, + -- matching BIT_NO_MAP_MUSIC: MUSIC_MUSEUM_GUY in Yellow and + -- MUSIC_MEET_PROF_OAK in Red/Blue. keepMusicOnce is the equivalent + -- one-shot set by play_music opts.keep. local keepMusic = (opts and opts.keepMusic) or self.keepMusicOnce self.keepMusicOnce = nil if not keepMusic then diff --git a/tests/engine/push_battle_transition.lua b/tests/engine/push_battle_transition.lua new file mode 100644 index 00000000..1e03eb2f --- /dev/null +++ b/tests/engine/push_battle_transition.lua @@ -0,0 +1,76 @@ +-- Commands.pushBattle (src/script/Commands.lua): the shared entry point +-- for start_battle, old_man_demo, and the PALLET_TOWN Pikachu catch +-- (data/scripts/story2.lua). Every one of those call sites used to carry +-- its own copy of "if ctx.overworld.pushBattle then ... else +-- ctx.game.stack:push(battle) end"; this locks the dedup in so a future +-- edit to one call site can't silently drop the transition wipe for the +-- others. +-- luajit tests/engine/push_battle_transition.lua + +package.path = "./?.lua;./?/init.lua;" .. package.path +love = love or require("tests.love_stub") + +local T = require("tests.harness") +local check = T.check +local eq = T.eq + +local Commands = require("src.script.Commands") +local Logger = require("src.core.Logger") + +local battle = { id = "the battle" } + +-- Runs fn() with Logger.warn spied instead of hitting the real +-- print()/Logger.history ring buffer, and returns the last formatted +-- warning (or nil if none). pcall-wrapped so an error inside fn() still +-- restores Logger.warn before propagating -- a leaked spy would swallow +-- every later warning in the same process silently. +local function withWarnSpy(fn) + local warned + local origWarn = Logger.warn + Logger.warn = function(fmt, ...) warned = string.format(fmt, ...) end + local ok, err = pcall(fn) + Logger.warn = origWarn + if not ok then error(err, 0) end + return warned +end + +-- ctx.overworld has a real pushBattle: it must be used, not a bare stack +-- push, so the flash/wipe transition and the battle-theme start survive. +do + local pushed + local ow = { pushBattle = function(self, b) pushed = b end } + local stackPushed + local ctx = { overworld = ow, game = { stack = { + push = function(_, b) stackPushed = b end } } } + Commands.pushBattle(ctx, battle) + eq(pushed, battle, "ctx.overworld:pushBattle is called with the battle") + check(stackPushed == nil, "the bare stack push is not also taken") +end + +-- ctx.overworld without a pushBattle method (a partial test double, per +-- BattleState:finish's "no live children" contract) falls back to a +-- bare stack push and logs, rather than silently skipping the transition. +do + local stackPushed + local ctx = { overworld = {}, game = { stack = { + push = function(_, b) stackPushed = b end } } } + local warned = withWarnSpy(function() Commands.pushBattle(ctx, battle) end) + eq(stackPushed, battle, "falls back to ctx.game.stack:push") + check(warned ~= nil, "the fallback logs a warning") + check(warned and warned:find("pushBattle") ~= nil, + "the warning names pushBattle") +end + +-- ctx.overworld absent entirely (headless script tests that never set +-- one up): same fallback, no crash on the ctx.overworld.pushBattle read +-- -- and still spied, since this also takes the warning path. +do + local stackPushed + local ctx = { game = { stack = { + push = function(_, b) stackPushed = b end } } } + local warned = withWarnSpy(function() Commands.pushBattle(ctx, battle) end) + eq(stackPushed, battle, "falls back to ctx.game.stack:push with no overworld") + check(warned ~= nil, "this fallback also logs a warning") +end + +T.finish("push_battle_transition") diff --git a/tests/parity_J.lua b/tests/parity_J.lua index 5316ca07..00d513cd 100644 --- a/tests/parity_J.lua +++ b/tests/parity_J.lua @@ -452,4 +452,50 @@ do "the caught Weedle is NOT added to the dex") end +-- (5b) Yellow's SimulatedInputBattleItemList (core.asm:2316-2319) drops +-- the same canned bag to a single POKé BALL, x1 -- pokered's +-- OldManItemList (core.asm:2212-2214, checked above) stays x50. Only +-- the item count changes; the scripted no-input menu flow is identical +-- and already covered above, so this jumps straight to the bag. +do + local GameVersion = require("src.core.GameVersion") + local oldVersion = GameVersion.get() + GameVersion.set("yellow") + local ok, err = pcall(function() + local pressed = {} + local stack = { states = {} } + function stack:push(s) table.insert(self.states, s) end + function stack:pop() return table.remove(self.states) end + function stack:top() return self.states[#self.states] end + local fg = { + data = Data, + save = require("src.core.SaveData").newGame(), + input = { wasPressed = function(_, k) return pressed[k] or false end, + isDown = function(_, k) return pressed[k] or false end }, + stack = stack, + } + fg.save.party = { Pokemon.new(Data, "BULBASAUR", 20) } + local demo = BattleState.newWild(fg, "PIKACHU", 5) + demo:makeOldManDemo("PROF.OAK") + stack:push(demo) + demo:enter() + for _ = 1, 300 do + if demo.phase == "menu" then break end + pressed.a = true + demo:update(1 / 60) + end + pressed.a = false + eq(demo.phase, "menu", "Yellow: the demo reaches the battle menu") + for _ = 1, 200 do + if stack:top() ~= demo then break end + demo:update(1 / 60) + end + local bag = stack:top() + eq(bag.items and bag.items[1] and bag.items[1].right, "x1", + "Yellow's old-man-style bag lists x1 (SimulatedInputBattleItemList)") + end) + GameVersion.set(oldVersion) + if not ok then error(err, 0) end +end + S.finish() diff --git a/tests/parity_yellow_pallet_pikachu.lua b/tests/parity_yellow_pallet_pikachu.lua new file mode 100644 index 00000000..57a2c910 --- /dev/null +++ b/tests/parity_yellow_pallet_pikachu.lua @@ -0,0 +1,201 @@ +-- Parity test for Yellow's Pallet Town intro (Oak stops the player, +-- catches a wild Pikachu, walks them to the lab). Drives the real +-- onStep closure in data/scripts/story2.lua through the real +-- StateStack/OverworldState, with no mocked battle or overworld, up +-- through the point Oak's Pikachu catch is armed: covers two of the +-- four fixes on this branch -- Music_MuseumGuy staying off in +-- Red/Blue, and the 2-frame hold before the Pikachu battle handing +-- off to BattleTransition rather than a bare push. +-- +-- Deliberately stops there rather than also driving the demo battle +-- to completion to assert Music_MuseumGuy fires in Yellow (the third +-- fix): that would mean re-deriving frame budgets for the battle +-- menu/bag/throw sequence tests/parity_J.lua already exercises, on +-- top of everything already driven here, for one more assertion -- +-- more coupling to unrelated timing than the fix is worth. That side +-- is manually verified instead (see the PR description). +-- +-- Sources: scripts/PalletTown.asm, engine/overworld/auto_movement.asm, +-- home/overworld.asm, engine/battle/core.asm (see the commits on this +-- branch for the exact citations). +-- luajit tests/parity_yellow_pallet_pikachu.lua +package.path = "./?.lua;./?/init.lua;" .. package.path +if not _G.love then _G.love = require("tests.love_stub") end + +local Data = require("src.core.Data") +if not (Data.maps and Data.maps.PALLET_TOWN) then Data:load() end + +local S = require("tests.harness").suite("parity Yellow Pallet Town Pikachu") +local check, eq = S.check, S.eq + +local GameVersion = require("src.core.GameVersion") +local SaveData = require("src.core.SaveData") +local Game = require("src.core.Game") +local StateStack = require("src.core.StateStack") +local OverworldState = require("src.world.OverworldController") +local BattleTransition = require("src.render.BattleTransition") +local TextBox = require("src.render.TextBox") +local Music = require("src.core.Music") +local mapScripts = require("data.scripts.init") + +local pallet = mapScripts.get("PALLET_TOWN") +check(pallet and pallet.onStep, "PALLET_TOWN exposes onStep") + +local oldVersion = GameVersion.get() +local prevGame = { data = Game.data, save = Game.save, stack = Game.stack, + input = Game.input, renderer = Game.renderer, + overworld = Game.overworld } + +local function freshGame(mapX, mapY) + Game.data = Data + Game.save = SaveData.newGame(Data) + Game.save.player.name = "RED" + StateStack:init() + Game.stack = StateStack + local pressed = {} + Game.input = { + isDown = function() return false end, + wasPressed = function(_, b) return pressed[b] or false end, + step = function() end, state = {}, pressQueue = {}, + } + Game.renderer = { + beginWorldPass = function() end, endWorldPass = function() end, + beginUIPass = function() end, endUIPass = function() end, + worldViewSize = function() return 160, 144 end, + setSGBZones = function() end, + } + StateStack:push(OverworldState, "PALLET_TOWN", mapX, mapY, "up") + Game.overworld = OverworldState + return pressed +end + +-- mash "a" whenever anything but the overworld is on top (dismisses +-- text boxes; scriptMove/hold/BattleTransition/BattleState's own +-- scripted-menu phases all ignore it) +local function pump(pressed, n) + for _ = 1, n do + pressed.a = Game.stack:top() ~= OverworldState + Game.stack:update(1 / 60) + end +end + +local function mashUntil(pressed, cond, cap) + for _ = 1, cap do + if cond() then return true end + pressed.a = Game.stack:top() ~= OverworldState + Game.stack:update(1 / 60) + end + return false +end + +-- Every scenario mutates the Game/GameVersion/StateStack singletons the +-- rest of the aggregate tests/run_tests.lua run depends on; a genuine +-- error partway through (plausible -- this drives real scriptMove, +-- pathfinding, TextBox paging and BattleState code, not test doubles) +-- must not skip the restore, or it cascades into every later suite in +-- the same process. StateStack:init() leaves it empty rather than +-- pointed at this scenario's pushed OverworldState/TextBox/ +-- BattleTransition instances. +local function scenario(fn) + local ok, err = pcall(fn) + GameVersion.set(oldVersion) + for k, v in pairs(prevGame) do Game[k] = v end + StateStack:init() + if not ok then error(err, 0) end +end + +-- ===================================================================== +-- (A) Red/Blue: MUSIC_MUSEUM_GUY must never play. Only "red" is driven +-- here -- story2.lua's onStep branches on GameVersion.isYellow() alone +-- and never calls isBlue(), so Red and Blue share this exact code path +-- and a separate Blue run would exercise nothing new. pokered's copy of +-- PalletMovementScript_OakMoveLeft only sets BIT_NO_MAP_MUSIC and never +-- calls PlayMusic -- MUSIC_MEET_PROF_OAK (started when Oak appears) +-- keeps running straight into the lab. +-- ===================================================================== +scenario(function() + GameVersion.set("red") + local pressed = freshGame(8, 2) + local played = {} + local origPlay = Music.play + Music.play = function(data, song, ...) + table.insert(played, song) + return origPlay(data, song, ...) + end + local ok, err = pcall(function() + local ow = OverworldState + check(pallet.onStep(Game, ow, 8, 1) == true, + "Red: onStep claims the trigger tile") + -- HeyWaitDontGoOutText (auto) -> "!" bubble hold(50) -> Oak + -- approaches -> "It's unsafe!" text -> escortToLab -> the walk to + -- the lab door. 3000 frames (50s of game time) covers it; the + -- negative Music_MuseumGuy checks below only mean something once + -- the escort has actually finished, not merely started. + pump(pressed, 3000) + check(Game.save.flags.EVENT_FOLLOWED_OAK_INTO_LAB == true, + "Red: the escort actually reaches the lab within the frame budget") + end) + Music.play = origPlay + if not ok then error(err, 0) end + local sawMuseumGuy = false + for _, s in ipairs(played) do + if s == "Music_MuseumGuy" then sawMuseumGuy = true end + end + check(not sawMuseumGuy, "Red/Blue: Music_MuseumGuy never plays for this escort") + check(played[1] == "Music_MeetProfOak", + "Red/Blue: Music_MeetProfOak is still the only cutscene cue played") +end) + +-- ===================================================================== +-- (B) Yellow: the turn-then-battle timing and the transition wipe. +-- ===================================================================== +scenario(function() + GameVersion.set("yellow") + local pressed = freshGame(10, 1) + local ok, err = pcall(function() + local ow = OverworldState + check(pallet.onStep(Game, ow, 10, 0) == true, + "Yellow: onStep claims the north-exit tile") + local heyWaitBox = Game.stack:top() + check(getmetatable(heyWaitBox) == TextBox, + "Yellow: onStep opens with a text box (HeyWaitDontGoOutText)") + + -- HeyWaitDontGoOutText (auto, ~20 frames) -> "!" bubble hold(50) -> + -- Oak approaches (findPath(10,4,10,1): 3 steps) -> hold(6) -> the + -- next real text box is ThatWasClose (button-dismissed, unlike the + -- first). + local sawThatWasClose = mashUntil(pressed, function() + local top = Game.stack:top() + return top ~= heyWaitBox and getmetatable(top) == TextBox + end, 3000) + check(sawThatWasClose, "Yellow: reaches the ThatWasClose text") + + local oak + for _, n in ipairs(ow.npcs) do + if n.def and n.def.name == "PALLETTOWN_OAK" then oak = n end + end + check(oak ~= nil, "Yellow: PALLETTOWN_OAK is spawned") + + -- dismiss ThatWasClose (a multi-page text: mash through the + -- typewriter cadence and the page turn); oak.facing flips + -- synchronously in the close callback (x == 10 -> "right"), then + -- hold(2) starts + local sawHold = mashUntil(pressed, function() return ow.emote ~= nil end, 2000) + check(sawHold, "Yellow: ThatWasClose closes and the hold before battle arms") + eq(oak and oak.facing, "right", "Oak turns to face the grass (x==10 -> right)") + check(ow.emote ~= nil and ow.emote.frames == 2, + "the hold before the battle is armed for exactly 2 frames") + + -- tick 1: 2 -> 1, battle not pushed yet + Game.stack:update(1 / 60) + check(getmetatable(Game.stack:top()) ~= BattleTransition, + "1 frame in: the battle has not started yet") + -- tick 2: 1 -> 0, the demo battle is pushed through the transition + Game.stack:update(1 / 60) + check(getmetatable(Game.stack:top()) == BattleTransition, + "2 frames in: Oak's catch enters through BattleTransition, not a bare push") + end) + if not ok then error(err, 0) end +end) + +S.finish()