From 8e48bb4e322088a5ebbe1286625afac7e08a5f14 Mon Sep 17 00:00:00 2001 From: thibautbus <310327033+thibautbus@users.noreply.github.com> Date: Tue, 11 Aug 2026 12:07:07 +0200 Subject: [PATCH 1/6] Play the museum-guy cue during Oak's lab walk in Yellow Yellow's Pallet Town intro (professor catches the wild Pikachu, then walks the player to the lab) played the map's default Pallet Town theme the whole time instead of the dedicated escort cue. Per pokeyellow's actual scripts: PlayDefaultMusicFadeOutCurrent (run on every battle exit) legitimately restores Pallet Town's theme after the Pikachu demo battle for the Whew.../Come with me lines; the escort cue (MUSIC_MUSEUM_GUY, the same "led by an NPC" theme Pewter's museum guide uses) only starts in PalletMovementScript_OakMoveLeft, the first function of the escort script -- but only in pokeyellow's copy. pokered's copy of that same shared Red/Yellow function only sets BIT_NO_MAP_MUSIC and leaves whatever was already playing (MUSIC_MEET_PROF_OAK) running uninterrupted into the lab. Start Music_MuseumGuy at the top of escortToLab, gated on Yellow so Red/Blue keeps its unchanged behavior, and fix the Oak-escort warp's keepMusic comment to say which song rides the warp in each version. --- data/scripts/story2.lua | 13 +++++++++++++ src/world/OverworldController.lua | 9 ++++++--- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/data/scripts/story2.lua b/data/scripts/story2.lua index b7e40f3d..958ba2a2 100644 --- a/data/scripts/story2.lua +++ b/data/scripts/story2.lua @@ -206,6 +206,19 @@ M.PALLET_TOWN = { end local function escortToLab(oak) + -- PalletMovementScript_OakMoveLeft (engine/overworld/auto_movement + -- .asm) is shared by Red and Yellow, but only Yellow's copy starts + -- MUSIC_MUSEUM_GUY there (the instant the movement script is + -- armed, before Oak or the player takes a single step); pokered's + -- copy only sets BIT_NO_MAP_MUSIC and leaves whatever was already + -- playing (MUSIC_MEET_PROF_OAK, started when Oak first appears) + -- running uninterrupted all the way into the lab. Until Yellow's + -- switch fires (including the Whew.../Come with me lines right + -- after the Pikachu battle) the map's default Pallet Town theme + -- plays, restored by the battle's own exit path. + 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() diff --git a/src/world/OverworldController.lua b/src/world/OverworldController.lua index a345ba6c..9330f879 100644 --- a/src/world/OverworldController.lua +++ b/src/world/OverworldController.lua @@ -453,9 +453,12 @@ 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: the Oak-escort warp keeps whatever cutscene song is + -- already playing going into the lab instead of cutting to it + -- (BIT_NO_MAP_MUSIC in wStatusFlags7) -- MUSIC_MUSEUM_GUY in Yellow + -- (started in escortToLab when Oak begins walking), MUSIC_MEET_PROF_OAK + -- in Red/Blue (pokered never switches songs for this walk); keepMusicOnce + -- is the play_music opts.keep one-shot of the same bit local keepMusic = (opts and opts.keepMusic) or self.keepMusicOnce self.keepMusicOnce = nil if not keepMusic then From 6d841526b16a0a21166a66722543c0602951d1e0 Mon Sep 17 00:00:00 2001 From: thibautbus <310327033+thibautbus@users.noreply.github.com> Date: Tue, 11 Aug 2026 14:00:59 +0200 Subject: [PATCH 2/6] Hold two frames on Oak's turn before the Pikachu battle starts oak.facing was set and the battle pushed in the same synchronous callback, so the overworld never rendered a frame of Oak already turned toward the grass before the screen cut to battle. Traced home/overworld.asm: PalletTownOakGreetsPlayerScript (the turn) and PalletTownPikachuBattleScript (arming wCurOpponent) are separate script ticks, one main-loop iteration apart. OverworldLoopLessDelay burns two DelayFrame calls per iteration, calls RunMapScript (via JoypadOverworld) first, and only then checks wCurOpponent to jump into the battle -- so the turn from iteration A is on screen for the two DelayFrame calls that open iteration B, before that same iteration's RunMapScript arms wCurOpponent and falls straight into the battle check. hold(2, ...) restores exactly that beat. --- data/scripts/story2.lua | 27 ++++++++++++++++++++------- 1 file changed, 20 insertions(+), 7 deletions(-) diff --git a/data/scripts/story2.lua b/data/scripts/story2.lua index 958ba2a2..a84bffcc 100644 --- a/data/scripts/story2.lua +++ b/data/scripts/story2.lua @@ -262,14 +262,27 @@ 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). + -- PalletTownOakGreetsPlayerScript (the turn) and + -- PalletTownPikachuBattleScript (arming wCurOpponent) are + -- separate script ticks in pokeyellow, one main-loop iteration + -- apart: OverworldLoopLessDelay (home/overworld.asm) burns two + -- DelayFrame calls per iteration, calls RunMapScript (via + -- JoypadOverworld) first, and only then checks wCurOpponent to + -- jump into the battle -- so the turn from iteration A is on + -- screen for the two DelayFrame calls that open iteration B, + -- before that same iteration's RunMapScript arms wCurOpponent + -- and falls straight into the battle check. Two frames, not + -- zero and not a deliberate pause. 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 + game.stack:push(battle) + end) end)) end From d4dc72d0f4989417dcbcd1b789db1d41dc32d6c9 Mon Sep 17 00:00:00 2001 From: thibautbus <310327033+thibautbus@users.noreply.github.com> Date: Tue, 11 Aug 2026 11:55:41 +0200 Subject: [PATCH 3/6] Fix the old-man demo bag to show 1 Poke Ball in Yellow, 50 in Red/Blue 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. --- src/battle/BattleState.lua | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/battle/BattleState.lua b/src/battle/BattleState.lua index 6cee785a..fc9480a3 100644 --- a/src/battle/BattleState.lua +++ b/src/battle/BattleState.lua @@ -2194,8 +2194,14 @@ function BattleState:openOldManBag() self.afterQueue = "menu" self:ui(function() local list + -- The canned one-item bag (POKE_BALL, neither reading from the real + -- inventory) differs by version: pokered's OldManItemList (core.asm + -- :2212-2214) is quantity 50; pokeyellow's SimulatedInputBattleItemList + -- (core.asm:2316-2319), shared by both the Viridian old man's demo and + -- Oak's Pikachu catch, dropped that to quantity 1. + 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 From cc43bd77d2e4744425bd7efa59385509d08c5afe Mon Sep 17 00:00:00 2001 From: thibautbus <310327033+thibautbus@users.noreply.github.com> Date: Tue, 11 Aug 2026 14:01:19 +0200 Subject: [PATCH 4/6] Give Oak's Pikachu catch the battle-start flash/wipe transition story2.lua pushed the demo battle straight onto the stack, skipping the flash + wipe that every other wild battle gets. InitWildBattle calls DoBattleTransitionAndInitBattleVariables unconditionally (core.asm:6699) -- there is no BATTLE_TYPE_OLD_MAN or BATTLE_TYPE_PIKACHU special case -- so the old-man tutorial and Oak's Pikachu catch get the wipe like any other wild battle, same as every scripted trainer. Commands.old_man_demo already routed the Viridian old man's tutorial catch through OverworldState:pushBattle for exactly this reason; the fallback it used for a pushBattle-less overworld (ctx.overworld and ctx.overworld.pushBattle then ... else game.stack:push(battle)) was duplicated verbatim in Commands .start_battle and would have been a third copy in story2.lua, so it's pulled into one Commands.pushBattle(ctx, battle) helper instead, used by all three call sites; the fallback branch now logs a warning rather than silently dropping the transition and battle-theme start. --- data/scripts/story2.lua | 7 ++++++- src/script/Commands.lua | 42 +++++++++++++++++++++++------------------ 2 files changed, 30 insertions(+), 19 deletions(-) diff --git a/data/scripts/story2.lua b/data/scripts/story2.lua index a84bffcc..30d78750 100644 --- a/data/scripts/story2.lua +++ b/data/scripts/story2.lua @@ -281,7 +281,12 @@ M.PALLET_TOWN = { battle.onFinish = function() afterPikaBattle() end - game.stack:push(battle) + -- InitWildBattle calls DoBattleTransitionAndInitBattleVariables + -- unconditionally (core.asm:6699) -- no BATTLE_TYPE_PIKACHU + -- special case -- so Oak's catch gets the flash + wipe like + -- any other wild battle (Commands.old_man_demo already does + -- this for the Viridian old man's tutorial catch). + Commands.pushBattle(ctx, battle) end) end)) end diff --git a/src/script/Commands.lua b/src/script/Commands.lua index dba4844b..45a77e6b 100644 --- a/src/script/Commands.lua +++ b/src/script/Commands.lua @@ -280,6 +280,28 @@ function Commands.save_end_battle_text(ctx, textId) ctx.endBattleText = TextBox.substitute(ctx.game, text or textId) 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. InitWildBattle calls DoBattleTransitionAndInitBattleVariables +-- unconditionally (core.asm:6699) -- there is no BATTLE_TYPE_OLD_MAN or +-- BATTLE_TYPE_PIKACHU special case -- so the old-man tutorial and Oak's +-- Pikachu catch get the wipe like any other wild battle, same as every +-- scripted trainer (gym leaders, the rival, Giovanni). ctx.overworld can +-- be a test double without the full OverworldState metatable, so this +-- falls back to a bare push; that fallback silently skips the wipe and +-- the battle-theme start, so it's worth a log rather than a quiet +-- behavior change. +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 +334,11 @@ 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 + Commands.pushBattle(ctx, battle) runner:yield() end @@ -818,15 +832,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 From ab94b96a79e8721fb321292dc55a0137bb1c7191 Mon Sep 17 00:00:00 2001 From: thibautbus <310327033+thibautbus@users.noreply.github.com> Date: Tue, 11 Aug 2026 15:23:32 +0200 Subject: [PATCH 5/6] Simplify comments --- data/scripts/story2.lua | 38 ++++++++++--------------------- src/battle/BattleState.lua | 9 ++++---- src/script/Commands.lua | 25 +++++++------------- src/world/OverworldController.lua | 10 ++++---- 4 files changed, 28 insertions(+), 54 deletions(-) diff --git a/data/scripts/story2.lua b/data/scripts/story2.lua index 30d78750..96904d35 100644 --- a/data/scripts/story2.lua +++ b/data/scripts/story2.lua @@ -206,16 +206,10 @@ M.PALLET_TOWN = { end local function escortToLab(oak) - -- PalletMovementScript_OakMoveLeft (engine/overworld/auto_movement - -- .asm) is shared by Red and Yellow, but only Yellow's copy starts - -- MUSIC_MUSEUM_GUY there (the instant the movement script is - -- armed, before Oak or the player takes a single step); pokered's - -- copy only sets BIT_NO_MAP_MUSIC and leaves whatever was already - -- playing (MUSIC_MEET_PROF_OAK, started when Oak first appears) - -- running uninterrupted all the way into the lab. Until Yellow's - -- switch fires (including the Whew.../Come with me lines right - -- after the Pikachu battle) the map's default Pallet Town theme - -- plays, restored by the battle's own exit path. + -- 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 @@ -263,17 +257,13 @@ M.PALLET_TOWN = { -- 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). - -- PalletTownOakGreetsPlayerScript (the turn) and - -- PalletTownPikachuBattleScript (arming wCurOpponent) are - -- separate script ticks in pokeyellow, one main-loop iteration - -- apart: OverworldLoopLessDelay (home/overworld.asm) burns two - -- DelayFrame calls per iteration, calls RunMapScript (via - -- JoypadOverworld) first, and only then checks wCurOpponent to - -- jump into the battle -- so the turn from iteration A is on - -- screen for the two DelayFrame calls that open iteration B, - -- before that same iteration's RunMapScript arms wCurOpponent - -- and falls straight into the battle check. Two frames, not - -- zero and not a deliberate pause. + -- 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 hold(2, nil, function() local battle = BattleState.newWild(game, "PIKACHU", 5) @@ -281,11 +271,7 @@ M.PALLET_TOWN = { battle.onFinish = function() afterPikaBattle() end - -- InitWildBattle calls DoBattleTransitionAndInitBattleVariables - -- unconditionally (core.asm:6699) -- no BATTLE_TYPE_PIKACHU - -- special case -- so Oak's catch gets the flash + wipe like - -- any other wild battle (Commands.old_man_demo already does - -- this for the Viridian old man's tutorial catch). + -- Use the standard wild-battle entry transition. Commands.pushBattle(ctx, battle) end) end)) diff --git a/src/battle/BattleState.lua b/src/battle/BattleState.lua index fc9480a3..fcd33523 100644 --- a/src/battle/BattleState.lua +++ b/src/battle/BattleState.lua @@ -2194,11 +2194,10 @@ function BattleState:openOldManBag() self.afterQueue = "menu" self:ui(function() local list - -- The canned one-item bag (POKE_BALL, neither reading from the real - -- inventory) differs by version: pokered's OldManItemList (core.asm - -- :2212-2214) is quantity 50; pokeyellow's SimulatedInputBattleItemList - -- (core.asm:2316-2319), shared by both the Viridian old man's demo and - -- Oak's Pikachu catch, dropped that to quantity 1. + -- 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 = qty }, diff --git a/src/script/Commands.lua b/src/script/Commands.lua index 45a77e6b..89a35a2d 100644 --- a/src/script/Commands.lua +++ b/src/script/Commands.lua @@ -280,19 +280,13 @@ function Commands.save_end_battle_text(ctx, textId) ctx.endBattleText = TextBox.substitute(ctx.game, text or textId) 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. InitWildBattle calls DoBattleTransitionAndInitBattleVariables --- unconditionally (core.asm:6699) -- there is no BATTLE_TYPE_OLD_MAN or --- BATTLE_TYPE_PIKACHU special case -- so the old-man tutorial and Oak's --- Pikachu catch get the wipe like any other wild battle, same as every --- scripted trainer (gym leaders, the rival, Giovanni). ctx.overworld can --- be a test double without the full OverworldState metatable, so this --- falls back to a bare push; that fallback silently skips the wipe and --- the battle-theme start, so it's worth a log rather than a quiet --- behavior change. +-- 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) @@ -334,10 +328,7 @@ function Commands.start_battle(ctx, kind, a, b) end runner:resume() end - -- 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. + -- A direct stack push would skip the battle-entry transition. Commands.pushBattle(ctx, battle) runner:yield() end diff --git a/src/world/OverworldController.lua b/src/world/OverworldController.lua index 9330f879..87b40ae3 100644 --- a/src/world/OverworldController.lua +++ b/src/world/OverworldController.lua @@ -453,12 +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 whatever cutscene song is - -- already playing going into the lab instead of cutting to it - -- (BIT_NO_MAP_MUSIC in wStatusFlags7) -- MUSIC_MUSEUM_GUY in Yellow - -- (started in escortToLab when Oak begins walking), MUSIC_MEET_PROF_OAK - -- in Red/Blue (pokered never switches songs for this walk); 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 From cef286b17006989a25f58109a8aa944f92c1d470 Mon Sep 17 00:00:00 2001 From: thibautbus <310327033+thibautbus@users.noreply.github.com> Date: Tue, 11 Aug 2026 16:55:06 +0200 Subject: [PATCH 6/6] Add unit tests Regression coverage for the four Yellow Pallet Town fixes on this branch: - tests/parity_J.lua: the old-man-style demo bag shows x1 in Yellow (SimulatedInputBattleItemList), not just the pre-existing x50 (pokered's OldManItemList) case. - tests/engine/push_battle_transition.lua: Commands.pushBattle calls ctx.overworld:pushBattle when available and falls back with a logged warning otherwise. Logger.warn is spied (pcall-safe, always restored) rather than read off the shared Logger.history ring buffer, so the fallback checks don't leave noise behind for whatever else runs in the same process. - tests/parity_yellow_pallet_pikachu.lua: drives the real onStep closure through the real StateStack/OverworldState, with no mocked battle -- Red never plays Music_MuseumGuy for this escort (Blue shares the same code path: onStep only branches on GameVersion.isYellow(), never isBlue(), so a separate Blue run would exercise nothing new), and Yellow's hold before the Pikachu battle is armed for exactly 2 frames before handing off to BattleTransition rather than a bare stack push. Stops there rather than also driving the demo battle to completion just to assert Music_MuseumGuy fires in Yellow (that fix's own coverage): the extra coupling to unrelated battle menu/bag/throw frame budgets wasn't worth it for one more assertion. That side stays manually verified. scenario() restores Game/GameVersion and re-inits StateStack on the way out. All three pass standalone (luajit tests/). The Yellow E2E test hits the same pre-existing "Music.playMap is nil" gap three other map-warping parity tests already hit inside the aggregated tests/run_tests.lua run (missing data/generated/audio.lua in this dev environment) -- confirmed by diffing the identical error text against parity_warp_after_warp_step.lua, which also passes clean standalone. --- tests/engine/push_battle_transition.lua | 76 +++++++++ tests/parity_J.lua | 46 ++++++ tests/parity_yellow_pallet_pikachu.lua | 201 ++++++++++++++++++++++++ 3 files changed, 323 insertions(+) create mode 100644 tests/engine/push_battle_transition.lua create mode 100644 tests/parity_yellow_pallet_pikachu.lua 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()