From cb6cfb5556134f03f094d2697d785f6269529560 Mon Sep 17 00:00:00 2001 From: bryanthaboi Date: Thu, 6 Aug 2026 06:36:12 -0400 Subject: [PATCH] CLOSES #883, CLOSES #887, CLOSES #894 --- README.md | 13 ++ docs/new-features.md | 8 +- main.lua | 35 +++- src/core/HostShell.lua | 36 +++- src/core/LaunchOptions.lua | 16 +- src/core/Platform.lua | 17 ++ src/import/RomImporter.lua | 45 ++--- src/inventory/ItemEffects.lua | 10 +- src/ui/BagMenu.lua | 8 +- tests/engine/evo_stone_cancel_bug883_test.lua | 166 ++++++++++++++++++ tests/parity_ai_switch_rate.lua | 123 +++++++++++++ tests/parity_picker_pointer_grab.lua | 28 ++- tests/run_tests.lua | 10 ++ 13 files changed, 476 insertions(+), 39 deletions(-) create mode 100644 tests/engine/evo_stone_cancel_bug883_test.lua create mode 100644 tests/parity_ai_switch_rate.lua diff --git a/README.md b/README.md index c1d4d08e..cd2a3bc6 100644 --- a/README.md +++ b/README.md @@ -205,6 +205,19 @@ even on a different computer, as long as the same folder comes along. already written to either location is touched automatically, so copy files over yourself if you want to carry existing progress across the switch. +## Launch Options + +By default the app opens the launcher so you can pick a game. Launch options +skip it and start one game directly, which is what you want for a one-click +entry: a desktop shortcut per game, a Steam entry, or a handheld frontend. + +| Option | Effect | +| --- | --- | +| `--game=red` | boot Red, skipping the launcher (`blue` and `yellow` too, or just `r` / `b` / `y`) | +| `--slot=2` | load that save slot; takes a slot number or a slot id | +| `--launcher` | open the launcher anyway, so you can edit a shortcut you already made | + + ## iOS Every release ships `gen1recomp-*-ios.ipa`. Sideload it with AltStore diff --git a/docs/new-features.md b/docs/new-features.md index 77552928..3a19f226 100644 --- a/docs/new-features.md +++ b/docs/new-features.md @@ -625,10 +625,12 @@ layout. Both ask twice. ## Launch options: boot straight into a game -`love . --game red` skips the launcher and starts that game; `--slot ` picks the save slot to load, and `--launcher` forces the launcher -anyway. `POKEPORT_GAME` / `POKEPORT_SLOT` do the same for shortcuts that can -only pass environment variables. This is for one-click entries: a desktop +anyway. Spell these with an `=`: LÖVE reads the command line first and takes a +bare word as a path to a game, so `--game red` fails looking for a folder +called `red`. `POKEPORT_GAME` / `POKEPORT_SLOT` do the same for shortcuts that +can only pass environment variables. This is for one-click entries: a desktop shortcut per game, a Steam entry, or a handheld frontend. Asking for a game whose ROM has not been imported opens the launcher on that game's tab rather than failing. diff --git a/main.lua b/main.lua index ab3c8623..cb57bc37 100644 --- a/main.lua +++ b/main.lua @@ -30,6 +30,21 @@ end local Game, EditorApp, Importer, TouchEditor +-- #887: quit-to-launcher state, shared by love.load and love.quit (both need +-- it, so it is declared here rather than next to love.quit). +-- * launchedIntoGame -- a --game / POKEPORT_GAME shortcut booted this +-- session straight into a game, so there is no launcher behind it and a +-- window close must exit. Restarting instead re-read the same shortcut +-- and came right back into the game, and the next close did it again: +-- the app could not be closed at all (macOS feels this worst, where the +-- red X, Cmd+Q and the Dock's Quit are all the same quit event). +-- * RELAUNCH_MARKER -- written in the save dir just before the #785 +-- restart, so the fresh boot ignores any boot-straight-into-a-game +-- option exactly once and keeps #785's promise of landing in the +-- launcher, whatever put the game on screen this time. +local launchedIntoGame = false +local RELAUNCH_MARKER = "relaunch_to_launcher.txt" + local autopilot -- optional scripted-input dev tool (tests/autopilot.lua) local driverCo -- optional frame-driver (POKEPORT_DRIVER=file.lua): a -- coroutine that receives `Game` and yields once per @@ -334,10 +349,19 @@ function love.load(args) -- EmulationStation needs: one click into the game the player wants, with no -- menu in between. A game that is not imported falls through to the -- launcher on its tab rather than booting into nothing. + -- A window close that restarted us into the launcher (#785) leaves the + -- marker behind: consume it and stay on the launcher, or the shortcut below + -- would boot the same game again and that close would restart again, + -- forever (#887). Consumed on read, so the very next launch is normal. + local relaunched = love.filesystem.getInfo(RELAUNCH_MARKER) ~= nil + if relaunched then pcall(love.filesystem.remove, RELAUNCH_MARKER) end + local launchGame, launchSlot = LaunchOptions.resolve(arg) - if launchGame and not LaunchOptions.forceLauncher(arg) then + if launchGame and not relaunched and not LaunchOptions.forceLauncher(arg) then if RomImporter.isReady(launchGame) then if launchSlot then LaunchOptions.selectSlot(launchGame, launchSlot) end + -- No launcher behind this session: love.quit must exit, not restart. + launchedIntoGame = true bootGame(launchGame) return end @@ -793,8 +817,15 @@ function love.quit() -- restart path must be no worse than that, not quietly better. local scripted = os.getenv("POKEPORT_AUTOPILOT") or os.getenv("POKEPORT_DRIVER") or os.getenv("POKEPORT_IMPORT_ONLY") == "1" or os.getenv("POKEPORT_IMPORT_ROM") - if Game and not Importer and not quitToLauncher and not scripted then + -- #887: a shortcut session (--game / POKEPORT_GAME) has no launcher to go + -- back to and the restart would re-read the shortcut, so it exits instead. + if Game and not Importer and not quitToLauncher and not scripted + and not launchedIntoGame then quitToLauncher = true + -- Tell the fresh boot to ignore any boot-straight-into-a-game option this + -- once, so the restart really does land in the launcher (#887). A failed + -- write only costs that suppression, so it must never block the restart. + pcall(love.filesystem.write, RELAUNCH_MARKER, "1") require("src.core.HostShell").restart() return true -- abort this quit; the restart lands back in the launcher end diff --git a/src/core/HostShell.lua b/src/core/HostShell.lua index d7a9d0f2..448db31e 100644 --- a/src/core/HostShell.lua +++ b/src/core/HostShell.lua @@ -62,8 +62,35 @@ function HostShell.hideHostConsole() return consoleHidden end +-- #254 was fixed inside the launcher and nowhere else: a native dialog opened +-- while a mouse button is still down blocks the whole loop in io.popen, so SDL +-- never processes the button-up and never drops the pointer capture it took +-- for the press (on X11 an XGrabPointer with owner_events). The grab outlives +-- the click, every pointer event over the child dialog is still routed to our +-- window, and the dialog draws and keyboard-navigates but ignores the mouse. +-- src/import/RomImporter.lua owns the launcher's copy; hoisting it here means +-- every host spawn inherits it, including one a mod reaches through HostShell. +-- Pump until nothing is held so SDL sees the release first; bounded, so a +-- stuck button costs a moment and never the game. pump() drains OS events +-- into LOVE's queue and dispatches nothing, so there is no reentry. Worker +-- threads load neither love.mouse nor love.event, so the guard below makes +-- this a no-op off the main thread. +function HostShell.releasePointerGrab() + if not (love and love.mouse and love.mouse.isDown and love.event + and love.event.pump and love.timer) then + return + end + local deadline = love.timer.getTime() + 1 + while love.mouse.isDown(1, 2, 3) do + love.event.pump() + if love.timer.getTime() > deadline then break end + love.timer.sleep(0.005) + end +end + -- Wraps io.popen with the AppImage env fix applied and lua errors swallowed function HostShell.popen(command, mode) + HostShell.releasePointerGrab() local ok, pipe = pcall(io.popen, HostShell.envPrefix() .. command, mode or "r") if not ok or not pipe then return nil end return pipe @@ -154,8 +181,15 @@ local function haveBridge() if not (love and love.system and type(love.system.httpDownload) == "function") then return false end + -- The OS allowlist is deliberate: the bridge is a per-port native addition, + -- not part of LOVE, so a build that exports the name on a platform we never + -- wired one for is a name collision, not a transport. UWP is listed because + -- Xbox has no curl and no way to spawn one (Platform.canSpawnProcess is + -- false there), so the bridge is its only possible transport (#876). Its + -- LOVE backend does not export it today and this still returns false, but + -- the gate is no longer the thing in the way. local osName = love.system.getOS and love.system.getOS() - return osName == "Android" or osName == "iOS" + return osName == "Android" or osName == "iOS" or osName == "UWP" end -- Is any transport available at all? Callers gate on this, never on curl. diff --git a/src/core/LaunchOptions.lua b/src/core/LaunchOptions.lua index 101785ac..216e26df 100644 --- a/src/core/LaunchOptions.lua +++ b/src/core/LaunchOptions.lua @@ -1,10 +1,16 @@ -- Launch options: boot straight into a game, skipping the launcher. -- --- love . --game red -- boot Red --- love . --game yellow --slot 2 -- boot Yellow on save slot 2 --- love . --game red --launcher -- open the launcher anyway (a shortcut --- the player wants to edit) --- POKEPORT_GAME=blue love . -- same, for launchers that only pass env +-- love . --game=red -- boot Red +-- love . --game=yellow --slot=2 -- boot Yellow on save slot 2 +-- love . --game=red --launcher -- open the launcher anyway (a shortcut +-- the player wants to edit) +-- POKEPORT_GAME=blue love . -- same, for launchers that only pass env +-- +-- The "--flag value" spelling parses here (argValue reads argv[i + 1]), but it +-- does not survive LOVE: boot.lua takes the first bare argument as a path to a +-- game to run, so `--game red` dies with "Cannot load game at path .../red" +-- before love.load is ever called, fused or not. Only the "=" spelling is +-- reachable, so that is the one the docs quote. -- -- This exists for the click-once cases: a desktop shortcut per game, a Steam -- entry, an EmulationStation/Playnite entry, a handheld frontend. Those all diff --git a/src/core/Platform.lua b/src/core/Platform.lua index 29e030ea..2d1adc4a 100644 --- a/src/core/Platform.lua +++ b/src/core/Platform.lua @@ -12,6 +12,8 @@ local function compute() local mobile = osName == "Android" or osName == "iOS" local nativePicker = love and love.system and type(love.system.pickFile) == "function" + local nativeHttp = love and love.system + and type(love.system.httpDownload) == "function" return { os = osName, nx = nx, @@ -24,6 +26,17 @@ local function compute() or (nativePicker and "native-picker") or "desktop", networkValidated = not nx and not uwp, + -- networkValidated is the self-updater's gate and stays a per-platform + -- policy call: a console package cannot replace itself on disk, so that + -- answer never depends on whether a transport exists. Fetching a mod + -- index or a mod zip is the narrower question, and #876 showed the two + -- had been conflated, so Xbox lost the mod catalog for the updater's + -- reason. Desktop answers it with curl through HostShell; the mobile and + -- console ports answer it with the native love.system.httpDownload bridge + -- (#597). The UWP LOVE backend does not export that bridge yet, so this + -- still resolves false on Xbox and the launcher still says so, but the + -- day the backend grows one, nothing here or in RomImporter has to change. + canFetchRemote = (not nx and not uwp) or nativeHttp, } end @@ -52,6 +65,10 @@ function Platform.networkValidated() return Platform.detect().networkValidated end +function Platform.canFetchRemote() + return Platform.detect().canFetchRemote +end + -- Tests may swap love.system between cases. function Platform._resetForTests() cached = nil diff --git a/src/import/RomImporter.lua b/src/import/RomImporter.lua index 50cae927..0b4570e0 100644 --- a/src/import/RomImporter.lua +++ b/src/import/RomImporter.lua @@ -315,27 +315,16 @@ end -- keyboard-navigates (keyboard focus is a separate grab) but ignores the -- mouse entirely -- issue #254 on Linux. Whether it bites is a race with how -- long the click was held, which is why the same build picks one ROM fine and --- then hangs the mouse on the next. So pump until no button is held, letting --- SDL see the release and let go first; bounded, so a stuck button costs a --- moment and never the launcher. pump() only drains OS events into LOVE's --- queue -- it dispatches nothing -- so there is no reentry into mousepressed --- and the release is still delivered normally on the next frame. -local function releasePointerGrab() - if not (love.mouse and love.mouse.isDown and love.event and love.event.pump - and love.timer) then - return - end - local deadline = love.timer.getTime() + 1 - while love.mouse.isDown(1, 2, 3) do - love.event.pump() - if love.timer.getTime() > deadline then break end - love.timer.sleep(0.005) - end -end +-- then hangs the mouse on the next. +-- +-- The release itself now lives in HostShell.releasePointerGrab, called from +-- HostShell.popen, so every host spawn inherits it and not just the three +-- pickers here. It stays a single release point on purpose: this file used +-- to run its own copy first, and each copy carries its own one-second bound, +-- so keeping both made a stuck button cost two seconds instead of one. local function commandOutput(command) if not Platform.canSpawnProcess() then return nil end - releasePointerGrab() local pipe = HostShell.popen(command) if not pipe then return nil end local result = pipe:read("*a") @@ -2917,9 +2906,14 @@ end -- Update button: when a newer release is known, confirm then install; when -- already current, force-refresh the 6h cache and report / offer update. function RomImporter:_modGithubAction(id, action) - if not Platform.networkValidated() then + -- canFetchRemote, not networkValidated: the self-updater's gate used to + -- stand in for this one, which cost Xbox the whole mod catalog rather than + -- just the self-update it actually cannot do (#876). Say what still works + -- while we are here, since the native picker is live on every platform that + -- lands in this branch. + if not Platform.canFetchRemote() then self.modNotice = { ok = false, - text = "Remote mod download is unavailable on this platform." } + text = "Remote mod download is unavailable on this platform. Install a mod .zip from storage instead." } return end local ModUpdate = require("src.mods.ModUpdate") @@ -3223,9 +3217,18 @@ end -- never ran. The fetch now starts here and completes across later frames in -- _pumpFindFetch; the loader overlay is up for the whole flight. function RomImporter:_refreshFind(force) - if not Platform.networkValidated() then + -- The notice is the fix, not the gate (#876). This branch used to return an + -- empty listing silently, and because the player had by then added a source, + -- the panel skipped its "No mod index added" card and rendered the merged + -- listing empty state instead: a valid feed reported as "This index lists no + -- mods yet." Every other failure on this panel surfaces through findNotice, + -- and this one has to as well, or adding an index looks like it worked and + -- the index looks empty. + if not Platform.canFetchRemote() then self.findLoaded = true self.findIndex = { mods = {}, categories = {} } + self.findNotice = { ok = false, + text = "Mod indexes cannot be fetched on this platform. Install a mod .zip from storage instead." } return end local ModIndex = require("src.mods.ModIndex") diff --git a/src/inventory/ItemEffects.lua b/src/inventory/ItemEffects.lua index 81a5362a..63f74228 100644 --- a/src/inventory/ItemEffects.lua +++ b/src/inventory/ItemEffects.lua @@ -51,6 +51,10 @@ local STONES = { local VITAMINS = { HP_UP = "hp", PROTEIN = "attack", IRON = "defense", CARBOS = "speed", CALCIUM = "special" } +-- REPEL / SUPER_REPEL / MAX_REPEL all funnel through ItemUseRepelCommon, +-- which refuses mid-battle before writing wRepelRemainingSteps (#894) +local REPELS = { REPEL = true, SUPER_REPEL = true, MAX_REPEL = true } + ItemEffects.BALLS = BALLS function ItemEffects.isBall(id) return BALLS[id] or false end @@ -144,9 +148,11 @@ function ItemEffects.use(data, save, itemId, target, battle, moveIndex, ow) local name = itemDef and itemDef.name or itemId -- ItemUseVitamin / ItemUsePPUp / ItemUseEvoStone / ItemUseCoinCase / - -- ItemUseTMHM all refuse mid-battle (jp nz, ItemUseNotTime) + -- ItemUseTMHM / ItemUseRepelCommon all refuse mid-battle + -- (jp nz, ItemUseNotTime) if battle and (VITAMINS[itemId] or STONES[itemId] or itemId == "PP_UP" or itemId == "RARE_CANDY" or itemId == "COIN_CASE" + or REPELS[itemId] or (itemDef and itemDef.machine)) then return "failed", { notTime(data, save) } end @@ -518,7 +524,7 @@ function ItemEffects.use(data, save, itemId, target, battle, moveIndex, ow) return "failed", { romText(data, "_CoinCaseNumCoinsText", "Coin count:\n%d", save.coins or 0) } end - if itemId == "REPEL" or itemId == "SUPER_REPEL" or itemId == "MAX_REPEL" then + if REPELS[itemId] then local steps = itemId == "REPEL" and 100 or itemId == "SUPER_REPEL" and 200 or 250 save.repelSteps = steps return "consumed", { Strings("%s used\n%s!", save.player.name, name) } diff --git a/src/ui/BagMenu.lua b/src/ui/BagMenu.lua index b0c5c738..650b364d 100644 --- a/src/ui/BagMenu.lua +++ b/src/ui/BagMenu.lua @@ -267,7 +267,13 @@ local function useOn(game, battle, id, target, list, moveIndex, picker) if extra and extra.evolveTo then list:close() local Evolution = require("src.pokemon.Evolution") - Evolution.evolve(game, target, extra.evolveTo) + -- item_effects.asm ItemUseEvoStone sets wForceEvolution before + -- TryEvolvingMon, so a stone evolution's B press is read and + -- discarded (EvolutionState.lua's cancelable check). via = "ITEM" + -- is what makes that non-cancelable here, same as the RARE_CANDY + -- call below; without it the stone (already consumed above) could + -- be cancelled out from under the player (#883) + Evolution.evolve(game, target, extra.evolveTo, nil, "ITEM") return end -- RARE CANDY: after the level text, the stat window, any level-up diff --git a/tests/engine/evo_stone_cancel_bug883_test.lua b/tests/engine/evo_stone_cancel_bug883_test.lua new file mode 100644 index 00000000..6a04f155 --- /dev/null +++ b/tests/engine/evo_stone_cancel_bug883_test.lua @@ -0,0 +1,166 @@ +-- A stone evolution started from the bag must not be cancelable (#883). +-- +-- engine/items/item_effects.asm ItemUseEvoStone sets wForceEvolution before +-- `call TryEvolvingMon`, and engine/movie/evolution.asm +-- Evolution_CheckForCancel reads the joypad but throws the B press away while +-- that flag is set (#290). So the B abort is a level-up/rare-candy behavior +-- only: a stone is removed from the bag the moment it is used, and an +-- evolution the player can cancel out of would eat the stone for nothing. +-- +-- src/ui/EvolutionState.lua encodes the flag as `via`: cancelable is +-- (via ~= "TRADE" and via ~= "ITEM"). The bag's stone branch omitted the +-- argument entirely, so `via` arrived nil and the movie accepted B. The +-- assertion here is on the value that reaches the screen, which is the only +-- thing standing between the two behaviors. +-- +-- ROM-free: the fixture dataset plus a registry-supplied EvolutionState, so +-- the real Screens.push resolution runs and no sprite is ever loaded. +-- luajit tests/engine/evo_stone_cancel_bug883_test.lua + +package.path = "./?.lua;./?/init.lua;" .. package.path + +local T = require("tests.harness") +local check, eq = T.check, T.eq +love = love or require("tests.love_stub") + +-- Lazily required inside the use branches; seeding package.loaded first keeps +-- the suite silent and free of a real Font atlas. +package.loaded["src.core.Sound"] = { + play = function() end, + playCry = function() end, +} +package.loaded["src.render.TextBox"] = { + new = function(_, text, done) return { textBox = true, text = text, done = done } end, +} +-- BagMenu and PartyMenu bind TextBox at require time, so they load against the +-- stub; Screens caches its factory per id and must be told to forget. +package.loaded["src.ui.BagMenu"] = nil +package.loaded["src.ui.PartyMenu"] = nil +local BagMenu = require("src.ui.BagMenu") +local PartyMenu = require("src.ui.PartyMenu") +local Screens = require("src.ui.Screens") +Screens.invalidate() + +local Fixtures = require("tests.modkit.fixtures") +local Bag = require("src.inventory.Bag") +local Pokemon = require("src.pokemon.Pokemon") +local EvolutionState = require("src.ui.EvolutionState") + +local Data = Fixtures.fresh() +-- The fixture item table carries no stone, and ItemEffects keys its stone +-- branch on the id; BagMenu only reads name/keyItem off the def. +Data.items.THUNDER_STONE = { + id = "THUNDER_STONE", index = 33, name = "THUNDERSTONE", price = 2100, + tossable = true, +} +-- and no fixture species evolves, so give A the stone evolution the branch +-- looks for (evo.method == "ITEM" and evo.item == the stone used). +Data.pokemon.FIXMON_A.evolutions = { + { method = "ITEM", item = "THUNDER_STONE", species = "FIXMON_B" }, +} + +-- The seam: Screens resolves an id through game.data.screens before falling +-- back to the builtin module, which is the same path a mod-replaced screen +-- takes. Recording the factory here catches exactly what Evolution.evolve +-- forwards, with no monkeypatching of Screens itself. +local pushed +Data.screens = Data.screens or {} +Data.screens.EvolutionState = function(game, mon, newSpecies, onDone, via) + pushed = { game = game, mon = mon, newSpecies = newSpecies, + onDone = onDone, via = via } + return { evoRecorder = true } +end +Screens.invalidate() + +local function freshGame() + local mon = Pokemon.new(Data, "FIXMON_A", 20) + local game = { + data = Data, + save = { + party = { mon }, + player = { name = "RED", id = 1 }, + inventory = {}, + options = {}, + flags = {}, + money = 0, + }, + } + game.stack = { + states = {}, + push = function(self, s) table.insert(self.states, s) end, + pop = function(self) return table.remove(self.states) end, + top = function(self) return self.states[#self.states] end, + } + -- one button edge per update, the way Input reports a fixed step + game.input = { pressed = nil } + function game.input:wasPressed(b) return self.pressed == b end + Bag.add(game.save, "THUNDER_STONE", 1) + return game, mon +end + +local function isPicker(s) return getmetatable(s) == PartyMenu end + +local function rowFor(list, id) + for i, r in ipairs(list.items) do + if r.value == id then return i end + end + return nil +end + +-- Open the bag, put the cursor on the stone, choose it, take USE off the +-- USE/TOSS box, then press A on the party picker. +local function useStone(game) + local list = BagMenu.new(game, {}) + game.stack:push(list) + local row = rowFor(list, "THUNDER_STONE") + if not row then return nil, "no THUNDER_STONE row in the bag" end + list.index = row + list.onChoose(list.items[row], list) + local sub = game.stack:top() + if sub and sub.items and sub.items[1] and sub.items[1].onSelect then + game.stack:pop() -- the USE/TOSS Menu pops itself on select + sub.items[1].onSelect() + end + local picker = game.stack:top() + if not isPicker(picker) then return nil, "party picker never opened" end + game.input.pressed = "a" + picker:update(1 / 60) + game.input.pressed = nil + return list +end + +do + local game, mon = freshGame() + local list, why = useStone(game) + if check(list ~= nil, "the bag opened and reached the picker: " .. tostring(why)) then + if check(pushed ~= nil, "the stone use pushed the evolution screen") then + eq(pushed.newSpecies, "FIXMON_B", "and it is the stone's evolution") + eq(pushed.mon, mon, "for the mon the stone was used on") + eq(pushed.via, "ITEM", + "the evolution runs as via = \"ITEM\" (wForceEvolution), which is " + .. "what makes it non-cancelable (#883)") + end + eq(game.save.inventory.THUNDER_STONE, nil, + "the stone is already gone by then, so a cancel would cost it for " + .. "nothing") + end +end + +-- The value only matters because of what EvolutionState does with it, so +-- assert that half against the real constructor rather than trusting the +-- comment. new() loads sprites through pcall and plays music through the +-- stubbed Sound, so it is safe headless. +do + local game = freshGame() + local mon = game.save.party[1] + local stoneEvo = EvolutionState.new(game, mon, "FIXMON_B", nil, "ITEM") + check(stoneEvo.cancelable == false, + "EvolutionState refuses B for a stone evolution (evolution.asm " + .. "Evolution_CheckForCancel with wForceEvolution set)") + local levelEvo = EvolutionState.new(game, mon, "FIXMON_B", nil, "LEVEL") + check(levelEvo.cancelable == true, + "and still honours B for a level-up evolution, so the fix did not " + .. "silently disable the cancel everywhere (#290, #213)") +end + +T.finish() diff --git a/tests/parity_ai_switch_rate.lua b/tests/parity_ai_switch_rate.lua new file mode 100644 index 00000000..d792a3e7 --- /dev/null +++ b/tests/parity_ai_switch_rate.lua @@ -0,0 +1,123 @@ +-- Parity test: the per-class trainer switch rolls (#890). +-- +-- Reports keep landing that Jugglers and Agatha "never switch". The rolls +-- are exact byte compares in pokered, so they are machine-assertable: sweep +-- every one of the 256 random bytes through TrainerAI.classAction and count +-- the switch outcomes. +-- +-- JugglerAI (engine/battle/trainer_ai.asm:324-327) +-- cp 25 percent + 1 / ret nc / jp AISwitchIfEnoughMons +-- `percent` is `* $ff / 100` (macros/data.asm:3), so the threshold is +-- 25 * 255 / 100 + 1 = 64 and the switch fires on rolls 0..63. +-- AgathaAI (engine/battle/trainer_ai.asm:429-437) +-- cp 8 percent / jp c, AISwitchIfEnoughMons -> 8 * 255 / 100 = 20, so +-- rolls 0..19 switch; the SAME byte then feeds cp 50 percent + 1 = 128 +-- for the SUPER POTION branch, which is why the two outcomes partition +-- the byte range instead of rolling twice. +-- +-- Self-contained; run via `luajit tests/parity_ai_switch_rate.lua`. +-- Also picked up by tests/run_tests.lua's parity_* glob. +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.pokemon and Data.pokemon.RATTATA) then Data:load() end + +local Pokemon = require("src.pokemon.Pokemon") +local TrainerAI = require("src.battle.TrainerAI") +local BattleState = require("src.battle.BattleState") +local S = require("tests.harness").suite("parity ai switch rate") +local check, eq = S.check, S.eq + +-- Just the fields classAction reads: the class lookup goes through +-- trainer.id, the HP fraction through enemy.mon, the reserve scan through +-- enemyParty/enemyIndex. hpFrac is current/max for the item branches. +local function stubBattle(id, roll, hpFrac) + local maxHp = 100 + return { + kind = "trainer", trainer = { id = id, name = id }, data = Data, + aiUses = 3, + enemy = { mon = { hp = math.floor(maxHp * hpFrac), stats = { hp = maxHp } }, + stages = {}, name = "MON" }, + enemyParty = { { hp = maxHp }, { hp = maxHp }, { hp = maxHp } }, + enemyIndex = 1, + rng = function() return roll end, + } +end + +-- Sweep the whole byte range: the counts ARE the thresholds. +local function sweep(id, hpFrac) + local switches, items = 0, 0 + for roll = 0, 255 do + local act = TrainerAI.classAction(stubBattle(id, roll, hpFrac)) + if act and act.special == "aiSwitch" then switches = switches + 1 + elseif act and act.special == "aiItem" then items = items + 1 end + end + return switches, items +end + +do + local sw, it = sweep("OPP_JUGGLER", 1.0) + eq(sw, 64, "Juggler switches on 64 of 256 rolls (cp 25 percent + 1)") + eq(it, 0, "Juggler never reaches for an item") + local swLow = sweep("OPP_JUGGLER", 0.05) + eq(swLow, 64, "the Juggler roll does not depend on the enemy's HP") +end + +do + -- above 1/4 max HP the item branch is refused, so only the switch fires + local sw, it = sweep("OPP_AGATHA", 1.0) + eq(sw, 20, "Agatha switches on 20 of 256 rolls (cp 8 percent)") + eq(it, 0, "Agatha holds the SUPER POTION above 1/4 HP") + -- below 1/4 the shared byte splits: 0..19 switch, 20..127 potion + local swLow, itLow = sweep("OPP_AGATHA", 0.1) + eq(swLow, 20, "the switch roll still wins the low rolls at low HP") + eq(itLow, 108, "the same byte leaves 20..127 for the SUPER POTION") +end + +-- AISwitchIfEnoughMons (engine/battle/trainer_ai.asm:554-582) counts every +-- unfainted party mon including the active one and needs 2 or more, so a +-- one-mon roster never switches however low the roll lands. +do + local b = stubBattle("OPP_JUGGLER", 0, 1.0) + b.enemyParty = { { hp = 100 } } + check(TrainerAI.classAction(b) == nil, + "a lone enemy mon never switches (cp 2 / jp nc)") + local b2 = stubBattle("OPP_JUGGLER", 0, 1.0) + b2.enemyParty = { { hp = 100 }, { hp = 0 }, { hp = 100 } } + local act = TrainerAI.classAction(b2) + check(act and act.index == 3, + "the switch takes the first living reserve, skipping the fainted slot") +end + +-- End to end through the real battle: the action a Juggler picks has to +-- reach executeAction and actually swap the active mon plus print +-- _AIBattleWithdrawText, otherwise a correct roll is invisible in play. +do + local Game = { + data = Data, + save = { party = { Pokemon.new(Data, "BULBASAUR", 50) }, + player = { name = "RED" }, inventory = {}, + options = { battleStyle = "set" }, + pokedex = { seen = {}, owned = {} }, flags = {}, money = 0 }, + stack = { push = function() end, pop = function() end, top = function() end }, + } + -- Juggler party 2 is the four-mon Victory Road roster + local b = BattleState.newTrainer(Game, "OPP_JUGGLER", 2) + eq(b.aiUses, 3, "wAICount seeded from the class record on send-out") + b.rng = function(lo) return lo end -- roll 0: inside every threshold + local act = b:enemyAction() + check(act and act.special == "aiSwitch", "the enemy turn resolves to a switch") + local outgoing = b.enemy.name + b:executeAction(b.enemy, b.player, act) + eq(b.enemyIndex, 2, "the active enemy slot moved to the reserve") + check(b.enemy.name ~= outgoing, "a different mon is out") + eq(b.aiUses, 3, "EnemySendOutFirstMon reseeds wAICount (core.asm:1305-1307)") + local withdrew = false + for _, item in ipairs(b.queue) do + if item.text and item.text:find("with%-\ndrew") then withdrew = true end + end + check(withdrew, "_AIBattleWithdrawText is queued for the player to read") +end + +S.finish() diff --git a/tests/parity_picker_pointer_grab.lua b/tests/parity_picker_pointer_grab.lua index f7b1a4e2..77b7e877 100644 --- a/tests/parity_picker_pointer_grab.lua +++ b/tests/parity_picker_pointer_grab.lua @@ -14,8 +14,29 @@ local check, eq = S.check, S.eq local RomImporter = require("src.import.RomImporter") -- ---------------------------------------------------------------- the funnel --- The release lives in commandOutput because all three pickers reach popen --- through it; a fourth picker calling io.popen directly would bring #254 back. +-- The release lives in HostShell.popen because every host spawn reaches the +-- OS through it; a caller reaching for io.popen directly would bring #254 +-- back. This assertion used to count io.popen calls in RomImporter, which is +-- where the release started out, and it went red the day the call was hoisted +-- into HostShell and nobody moved the check with it: RomImporter has held +-- zero io.popen calls since, so the count could never be the 1 it wanted. +-- Point it at the funnel that actually exists now. +-- Matched as pcall(io.popen rather than io.popen( because the spawn is +-- wrapped to swallow lua errors, so the call form never appears bare. +do + local f = io.open("src/core/HostShell.lua", "rb") + check(f ~= nil, "HostShell source is readable") + if f then + local src = f:read("*a") + f:close() + local calls = 0 + for _ in src:gmatch("pcall%(io%.popen") do calls = calls + 1 end + eq(calls, 1, "every host spawn still funnels through the one io.popen" + .. " call, which is where the pointer grab is released (#254)") + end +end + +-- RomImporter must not grow a picker that goes around HostShell. do local f = io.open("src/import/RomImporter.lua", "rb") check(f ~= nil, "RomImporter source is readable") @@ -24,8 +45,7 @@ do f:close() local calls = 0 for _ in src:gmatch("io%.popen%(") do calls = calls + 1 end - eq(calls, 1, "every desktop picker still funnels through the one io.popen" - .. " call, which is where the pointer grab is released (#254)") + eq(calls, 0, "no picker calls io.popen behind HostShell's back (#254)") end end diff --git a/tests/run_tests.lua b/tests/run_tests.lua index a5d5517d..92b21cd2 100644 --- a/tests/run_tests.lua +++ b/tests/run_tests.lua @@ -283,6 +283,16 @@ do eq(ItemEffects.use(Data, save, "HM_SURF", pikachu, {}), "failed", "HM refuses mid-battle") end +do + local rRepel, repelMsg = ItemEffects.use(Data, save, "MAX_REPEL", nil, {}) + eq(rRepel, "failed", "Max Repel refuses mid-battle (#894)") + check(repelMsg and repelMsg[1] and repelMsg[1]:find("isn't the", 1, true), + "Max Repel mid-battle Oak text") + eq(ItemEffects.use(Data, save, "REPEL", nil, {}), "failed", + "Repel refuses mid-battle") + eq(ItemEffects.use(Data, save, "SUPER_REPEL", nil, {}), "failed", + "Super Repel refuses mid-battle") +end local r5, _, extra = ItemEffects.use(Data, save, "THUNDER_STONE", pikachu) eq(r5, "consumed", "Thunder Stone works on Pikachu") eq(extra.evolveTo, "RAICHU", "Thunder Stone evolves Pikachu to Raichu")