Merge pull request #1379 from AverageConsumer/codex/mod-battle-intents

mods: add validated battle menu intents

docs/modding.md

@@ -264,6 +264,25 @@ Gold currently returns an empty `items` list rather than guessing at its
 pocketed PACK flow. Callers should ignore unknown fields and tolerate absent
 optional ones.
 
+## Battle menu intents
+
+`mod.battle:submit(intent)` applies a validated choice to the snapshot the mod
+just read. Every intent needs a mod-owned, strictly increasing positive
+integer `id` and the latest snapshot `revision`. Stale, replayed, covered, or
+invalid choices return `nil` plus a reason without changing the battle.
+
+The shared Red, Blue, Yellow, and Gold intents are:
+
+- `{ kind = "menu", choice = "fight" }` (`party`, `item`, and `run` are the
+  other accepted choices)
+- `{ kind = "move", slot = 1..4 }`
+- `{ kind = "back" }` while the move menu is active
+
+Menu choices and moves use the same engine methods as the native controls;
+`party` and `item` open the native screens rather than exposing or duplicating
+their mutable logic. Tutorial, link, Safari, forced, stale, and covered battle
+states refuse these core intents. Use `mod.input` for ordinary text advance.
+
 ## Rendering pipelines
 
 Most registries hand the engine *content*. `render_pipelines` hands it

src/battle/BattleAPI.lua

@@ -197,4 +197,61 @@ function BattleAPI:snapshot()
     mimicMoves = mimicCopies(game, battle), mimicIndex = battle.mimicIndex }
 end
 
+local MENU_CHOICES = { fight = true, party = true, item = true, run = true }
+
+local function validSlot(slot)
+  return type(slot) == "number" and slot % 1 == 0 and slot >= 1
+end
+
+function BattleAPI:submit(intent)
+  if type(intent) ~= "table" then return nil, "intent must be a table" end
+  if type(intent.id) ~= "number" or intent.id % 1 ~= 0 or intent.id < 1 then
+    return nil, "intent id must be a positive integer"
+  end
+  if self.lastIntentId and intent.id <= self.lastIntentId then
+    return nil, "replayed intent"
+  end
+
+  local battle, top = activeBattle(self.game)
+  if not battle then return nil, "no battle" end
+  if intent.revision ~= self:_revision(battle, top) then
+    return nil, "stale battle context"
+  end
+  local kind = battle:battleKind()
+  if kind == "oldman" or kind == "link" or kind == "safari" then
+    return nil, "battle kind is not controllable"
+  end
+  if top ~= battle then return nil, "battle menu is covered" end
+
+  local ok, err
+  if intent.kind == "menu" then
+    if battle.phase ~= "menu" then return nil, "battle menu is not active" end
+    if not MENU_CHOICES[intent.choice] then
+      return nil, "unknown battle menu choice"
+    end
+    ok, err = battle:chooseMenu(intent.choice)
+  elseif intent.kind == "move" then
+    if battle.phase ~= "moveSelect" then
+      return nil, "move menu is not active"
+    end
+    if battle.moveSwapIndex then return nil, "move reorder is active" end
+    local move = validSlot(intent.slot) and battle.player
+      and battle.player.curMoves[intent.slot]
+    if not move then return nil, "invalid move slot" end
+    if (move.pp or 0) <= 0 then return nil, "move has no PP" end
+    if battle.player.disabledSlot == intent.slot then
+      return nil, "move is disabled"
+    end
+    ok, err = battle:chooseMove(intent.slot)
+  elseif intent.kind == "back" then
+    ok, err = battle:cancelMove()
+  else
+    return nil, "unknown battle intent"
+  end
+  if not ok then return nil, err end
+  self.lastIntentId = intent.id
+  self.signature = nil
+  return true
+end
+
 return BattleAPI

src/battle/BattleState.lua

@@ -1951,6 +1951,77 @@ function BattleState:playerHasPP()
   return false
 end
 
+-- One semantic path for the native command menu and mod.battle intents.
+function BattleState:chooseMenu(choice)
+  if self.phase ~= "menu" then return nil, "battle menu is not active" end
+  if not self.player or not self.player.mon or self.player.mon.hp <= 0
+      or self:menuLockedAction(self.player) then
+    return nil, "battle menu is not ready"
+  end
+  self:clearTurnFlinches()
+  if choice == "fight" and self.ghost then
+    self:say(Strings("%s is too\nscared to move!", self.player.name))
+    self.phase = "messages"
+    self.afterQueue = "menu"
+    self:act(function()
+      self:executeAction(self.enemy, self.player, self:enemyAction())
+    end)
+    -- A scared turn still ticks the player's residual effects.
+    self:queueResidual(self.player, self.enemy)
+    self:act(function() self:endOfTurn() end)
+  elseif choice == "fight" then
+    -- Trapping, Bide, and similar locks skip the move list.
+    local fightLock = self:fightLockedAction(self.player)
+    if fightLock then
+      self:resolveTurn(fightLock)
+    elseif not self:playerHasPP() then
+      -- No usable PP goes straight to Struggle.
+      self:say(Strings("%s has no\nmoves left!", self.player.name))
+      self:resolveTurn({ id = "STRUGGLE", pp = 1, struggle = true })
+    else
+      self.phase = "moveSelect"
+      self.moveIndex = math.min(self.moveIndex, #self.player.curMoves)
+      self.moveSwapIndex = nil
+    end
+  elseif choice == "run" then
+    self:tryRun()
+  elseif choice == "item" then
+    self:openItems()
+  elseif choice == "party" then
+    self:openParty()
+  else
+    return nil, "unknown battle menu choice"
+  end
+  return true
+end
+
+function BattleState:chooseMove(index)
+  if self.phase ~= "moveSelect" then return nil, "move menu is not active" end
+  local move = self.player.curMoves[index]
+  if not move then return nil, "invalid move slot" end
+  self.moveIndex = index
+  if self.player.disabledSlot == index then
+    self:say(self:romText("_MoveDisabledText", "The move is\ndisabled!"))
+    self.phase = "messages"
+    self.afterQueue = "menu"
+  elseif move.pp <= 0 then
+    self:say(self:romText("_MoveNoPPText", "No PP left for\nthis move!"))
+    self.phase = "messages"
+    self.afterQueue = "menu"
+  else
+    self.playerMoveListIndex = index
+    self:resolveTurn(move)
+  end
+  return true
+end
+
+function BattleState:cancelMove()
+  if self.phase ~= "moveSelect" then return nil, "move menu is not active" end
+  self.moveSwapIndex = nil
+  self.phase = "menu"
+  return true
+end
+
 function BattleState:swapMoves(i, j)
   if i == j then return end
   local moves = self.player.curMoves
@@ -2120,42 +2191,7 @@ function BattleState:update(dt)
     self.menuIndex = row * 2 + col + 1
     if input:wasPressed("a") then
       require("src.core.Sound").play(self.data, "Press_AB")
-      local choice = ({ "fight", "pkmn", "item", "run" })[self.menuIndex]
-      if choice == "fight" and self.ghost then
-        self:say(Strings("%s is too\nscared to move!", self.player.name))
-        self.phase = "messages"
-        self.afterQueue = "menu"
-        self:act(function()
-          self:executeAction(self.enemy, self.player, self:enemyAction())
-        end)
-        -- the scared turn still ticks the player's residual (PrintGhostText
-        -- -> ExecutePlayerMoveDone, core.asm:3056, 3275-3279)
-        self:queueResidual(self.player, self.enemy)
-        self:act(function() self:endOfTurn() end)
-      elseif choice == "fight" then
-        -- After the menu: own trapping/Bide or foe Wrap skips the move
-        -- list and forces the locked action (core.asm:320-329)
-        local fightLock = self:fightLockedAction(self.player)
-        if fightLock then
-          self:resolveTurn(fightLock)
-          return
-        end
-        if not self:playerHasPP() then
-          -- _NoMovesLeftText, then Struggle engages
-          self:say(Strings("%s has no\nmoves left!", self.player.name))
-          self:resolveTurn({ id = "STRUGGLE", pp = 1, struggle = true })
-          return
-        end
-        self.phase = "moveSelect"
-        self.moveIndex = math.min(self.moveIndex, #self.player.curMoves)
-        self.moveSwapIndex = nil
-      elseif choice == "run" then
-        self:tryRun()
-      elseif choice == "item" then
-        self:openItems()
-      else
-        self:openParty()
-      end
+      self:chooseMenu(({ "fight", "party", "item", "run" })[self.menuIndex])
     end
     return
   end
@@ -2184,8 +2220,7 @@ function BattleState:update(dt)
       end
     elseif input:wasPressed("b") then
       require("src.core.Sound").play(self.data, "Press_AB")
-      self.moveSwapIndex = nil
-      self.phase = "menu"
+      self:cancelMove()
     elseif input:wasPressed("a") then
       require("src.core.Sound").play(self.data, "Press_AB")
       if self.moveSwapIndex then
@@ -2193,19 +2228,7 @@ function BattleState:update(dt)
         self.moveSwapIndex = nil
         return
       end
-      local mv = moves[self.moveIndex]
-      if self.player.disabledSlot == self.moveIndex then
-        self:say(self:romText("_MoveDisabledText", "The move is\ndisabled!"))
-        self.phase = "messages"
-        self.afterQueue = "menu"
-      elseif mv.pp <= 0 then
-        self:say(self:romText("_MoveNoPPText", "No PP left for\nthis move!"))
-        self.phase = "messages"
-        self.afterQueue = "menu"
-      else
-        self.playerMoveListIndex = self.moveIndex
-        self:resolveTurn(mv)
-      end
+      self:chooseMove(self.moveIndex)
     end
     return
   end

src/battle/gen2/BattleAPI.lua

@@ -116,4 +116,57 @@ function BattleAPI:snapshot()
     items = {} }
 end
 
+local MENU_CHOICES = { fight = true, party = true, item = true, run = true }
+
+local function validSlot(slot)
+  return type(slot) == "number" and slot % 1 == 0 and slot >= 1
+end
+
+function BattleAPI:submit(intent)
+  if type(intent) ~= "table" then return nil, "intent must be a table" end
+  if type(intent.id) ~= "number" or intent.id % 1 ~= 0 or intent.id < 1 then
+    return nil, "intent id must be a positive integer"
+  end
+  if self.lastIntentId and intent.id <= self.lastIntentId then
+    return nil, "replayed intent"
+  end
+
+  local screen, top = activeBattle(self.game)
+  if not screen or not screen.battle then return nil, "no battle" end
+  if intent.revision ~= self:_revision(screen, top) then
+    return nil, "stale battle context"
+  end
+  if screen.tutorial then return nil, "battle kind is not controllable" end
+  if top ~= screen then return nil, "battle menu is covered" end
+
+  local battle = screen.battle
+  local ok, err
+  if intent.kind == "menu" then
+    if screen.phase ~= "menu" then return nil, "battle menu is not active" end
+    if not MENU_CHOICES[intent.choice] then
+      return nil, "unknown battle menu choice"
+    end
+    ok, err = screen:chooseMenu(intent.choice)
+  elseif intent.kind == "move" then
+    if screen.phase ~= "moves" then return nil, "move menu is not active" end
+    if screen.moveSwapIndex then return nil, "move reorder is active" end
+    local move = validSlot(intent.slot) and battle.player
+      and battle.player.moves and battle.player.moves[intent.slot]
+    if not move then return nil, "invalid move slot" end
+    if (move.pp or 0) <= 0 then return nil, "move has no PP" end
+    if battle:moveDisabled(battle.player, move.id) then
+      return nil, "move is disabled"
+    end
+    ok, err = screen:chooseMove(intent.slot)
+  elseif intent.kind == "back" then
+    ok, err = screen:cancelMove()
+  else
+    return nil, "unknown battle intent"
+  end
+  if not ok then return nil, err end
+  self.lastIntentId = intent.id
+  self.signature = nil
+  return true
+end
+
 return BattleAPI

src/ui/gen2/BattleState.lua

@@ -121,6 +121,8 @@ local TEXT_ASK_FORGET_MOVE = Strings.source(
 -- Gen 1 uses.  The second label is the two-glyph <PK><MN> ligature (charmap
 -- $e1/$e2), which is what makes it fit a six-tile column.
 local MENU = { "FIGHT", "<PK><MN>", "PACK", "RUN" }
+local MENU_ACTION = { FIGHT = "fight", ["<PK><MN>"] = "party",
+  PACK = "item", RUN = "run" }
 local MENU_BOX_X = 8
 local MENU_COL_SPACING = 6
 
@@ -1660,6 +1662,64 @@ function BattleState:playerMoves()
   return (self.battle and self.battle.player and self.battle.player.moves) or {}
 end
 
+-- One semantic path for the native command menu and mod.battle intents.
+function BattleState:chooseMenu(choice)
+  if self.phase ~= "menu" then return nil, "battle menu is not active" end
+  if choice == "fight" then
+    -- CheckPlayerHasUsableMoves skips MoveSelectionScreen and uses Struggle.
+    local fighter = self.battle and self.battle.player
+    if fighter and #self:playerMoves() > 0
+        and not self.battle:hasUsableMoves(fighter) then
+      self:submit({ kind = "move", move = Battle.STRUGGLE })
+    else
+      self.phase = "moves"
+      -- MoveSelectionScreen reopens on the last used move, clamped if the
+      -- moveset shrank since then.
+      local moves = self:playerMoves()
+      self.moveIndex = math.max(1,
+        math.min(self.moveIndex or 1, math.max(1, #moves)))
+    end
+  elseif choice == "run" then
+    self:submit({ kind = "run" })
+  elseif choice == "item" then
+    if self.tutorial then
+      self:openTutorialPack()
+    elseif self.contest then
+      self:throwParkBall()
+    else
+      self:openPack()
+    end
+  elseif choice == "party" then
+    self:openParty()
+  else
+    return nil, "unknown battle menu choice"
+  end
+  return true
+end
+
+function BattleState:chooseMove(index)
+  if self.phase ~= "moves" then return nil, "move menu is not active" end
+  local move = self:playerMoves()[index]
+  if not move then return nil, "invalid move slot" end
+  self.moveIndex = index
+  self.moveSwapIndex = nil
+  if (move.pp or 0) <= 0 then
+    self:refuseMove(TEXT_NO_PP_LEFT)
+  elseif self.battle:moveDisabled(self.battle.player, move.id) then
+    self:refuseMove(TEXT_MOVE_DISABLED)
+  else
+    self:submit({ kind = "move", move = move.id })
+  end
+  return true
+end
+
+function BattleState:cancelMove()
+  if self.phase ~= "moves" then return nil, "move menu is not active" end
+  self.moveSwapIndex = nil
+  self.phase = "menu"
+  return true
+end
+
 -- MoveSelectionScreen's `.pressed_select` (engine/battle/core.asm:5320-5374).
 -- SELECT marks a slot, SELECT again swaps the marked slot with the one under
 -- the cursor, and A or B clears the mark without swapping (the A arm opens
@@ -1833,37 +1893,7 @@ function BattleState:update(_dt)
         or self.menuIndex - 2
     elseif input:wasPressed("a") then
       self:playSfx("Sfx_ReadText2")
-      local choice = MENU[self.menuIndex]
-      if choice == "FIGHT" then
-        -- `call .CheckPlayerHasUsableMoves / ret z` (engine/battle/core.asm
-        -- :5058-5059): a mon with nothing to spend never sees the list.
-        local fighter = self.battle and self.battle.player
-        if fighter and #self:playerMoves() > 0
-            and not self.battle:hasUsableMoves(fighter) then
-          return self:submit({ kind = "move", move = Battle.STRUGGLE })
-        end
-        self.phase = "moves"
-        -- MoveSelectionScreen seeds wMenuCursorY from wCurMoveNum + 1
-        -- (engine/battle/core.asm:5111) and the A-press writes the picked row
-        -- back, so the list reopens on the move used last turn; only
-        -- SendOutPlayerMon and CleanUpBattleRAM zero it.  Clamp rather than
-        -- reset, for a moveset that shrank (Mimic, a forgotten slot).
-        local moves = self:playerMoves()
-        self.moveIndex = math.max(1,
-          math.min(self.moveIndex or 1, math.max(1, #moves)))
-      elseif choice == "RUN" then
-        self:submit({ kind = "run" })
-      elseif choice == "PACK" then
-        if self.tutorial then
-          self:openTutorialPack()
-        elseif self.contest then
-          self:throwParkBall()
-        else
-          self:openPack()
-        end
-      else
-        self:openParty()
-      end
+      self:chooseMenu(MENU_ACTION[MENU[self.menuIndex]])
     end
     return
   end
@@ -1884,22 +1914,12 @@ function BattleState:update(_dt)
     elseif input:wasPressed("b") then
       -- B leaves the list, and a mark never survives it
       self:playSfx("Sfx_ReadText2")
-      self.moveSwapIndex = nil
-      self.phase = "menu"
+      self:cancelMove()
     elseif input:wasPressed("a") then
       -- `xor a / ld [wSwappingMove], a` opens the A arm: choosing a move
       -- cancels a pending swap rather than performing it
       self:playSfx("Sfx_ReadText2")
-      self.moveSwapIndex = nil
-      local move = moves[self.moveIndex]
-      if not move then return end
-      -- `.no_pp_left` and `.move_disabled` both end on `jp MoveSelectionScreen`
-      -- (engine/battle/core.asm:5213-5246): neither spends the turn.
-      if (move.pp or 0) <= 0 then return self:refuseMove(TEXT_NO_PP_LEFT) end
-      if self.battle:moveDisabled(self.battle.player, move.id) then
-        return self:refuseMove(TEXT_MOVE_DISABLED)
-      end
-      self:submit({ kind = "move", move = move.id })
+      self:chooseMove(self.moveIndex)
     end
     return
   end

tests/mod_battle_snapshot_test.lua

@@ -4,7 +4,8 @@ love = love or require("tests.love_stub")
 local S = require("tests.harness").suite("mod battle snapshot")
 local check, eq = S.check, S.eq
 
-check(require("src.battle.BattleState").isBattleState == true,
+local Gen1BattleState = require("src.battle.BattleState")
+check(Gen1BattleState.isBattleState == true,
   "Gen 1 battle states carry the discovery marker")
 
 local TypeChart = require("src.battle.TypeChart")
@@ -50,6 +51,18 @@ local battle = {
 function battle:battleKind() return "wild" end
 function battle:effectRecord() return { accuracyChecked = true } end
 function battle:visibleText() return { "Wild TESTMON appeared!" } end
+function battle:menuLockedAction() return nil end
+function battle:chooseMenu(choice)
+  self.chosenMenu = choice
+  if choice == "fight" then self.phase = "moveSelect" end
+  return true
+end
+function battle:chooseMove(slot)
+  self.chosenMove = slot
+  self.phase = "messages"
+  return true
+end
+function battle:cancelMove() self.phase = "menu" return true end
 function battle:catchChance(ball)
   return require("src.battle.Catching").chance(ball, self.enemy.mon,
     game.data.pokemon[self.enemy.mon.species])
@@ -81,6 +94,35 @@ game.stack.states = {}
 check(api:snapshot() == nil, "Gen 1 returns nil outside a battle")
 game.stack.states = { battle }
 
+local menu = api:snapshot()
+local ok, err = api:submit({ id = 1, revision = menu.revision - 1,
+  kind = "menu", choice = "fight" })
+check(not ok and err == "stale battle context",
+  "Gen 1 rejects a stale intent")
+ok, err = api:submit({ id = 1, revision = menu.revision,
+  kind = "menu", choice = "missing" })
+check(not ok and err == "unknown battle menu choice",
+  "Gen 1 rejects an unknown menu choice")
+check(api:submit({ id = 1, revision = menu.revision,
+  kind = "menu", choice = "fight" }), "Gen 1 accepts a menu intent")
+eq(battle.chosenMenu, "fight", "Gen 1 uses the semantic menu path")
+ok, err = api:submit({ id = 1, revision = menu.revision,
+  kind = "menu", choice = "fight" })
+check(not ok and err == "replayed intent", "Gen 1 rejects a replayed intent")
+local moveMenu = api:snapshot()
+ok, err = api:submit({ id = 2, revision = moveMenu.revision,
+  kind = "move", slot = 9 })
+check(not ok and err == "invalid move slot",
+  "Gen 1 rejects an invalid move slot")
+check(api:submit({ id = 2, revision = moveMenu.revision,
+  kind = "move", slot = 1 }), "Gen 1 accepts a valid move")
+eq(battle.chosenMove, 1, "Gen 1 uses the semantic move path")
+battle.phase = "moveSelect"
+local back = api:snapshot()
+check(api:submit({ id = 3, revision = back.revision, kind = "back" }),
+  "Gen 1 accepts move-menu back")
+eq(battle.phase, "menu", "Gen 1 back restores the command menu")
+
 local player2 = { species = "CHIKORITA", level = 5, hp = 20,
   maxHp = 21, moves = { { id = "TACKLE", pp = 35, maxPp = 35 } } }
 local enemy2 = { species = "RATTATA", level = 3, hp = 12, maxHp = 12,
@@ -90,6 +132,17 @@ local battle2 = { player = player2, enemy = enemy2, party = { player2 },
 function battle2:moveDisabled() return false end
 local screen2 = { screenId = "Gen2BattleState", battle = battle2,
   phase = "menu", menuIndex = 1, moveIndex = 1 }
+function screen2:chooseMenu(choice)
+  self.chosenMenu = choice
+  if choice == "fight" then self.phase = "moves" end
+  return true
+end
+function screen2:chooseMove(slot)
+  self.chosenMove = slot
+  self.phase = "resolving"
+  return true
+end
+function screen2:cancelMove() self.phase = "menu" return true end
 local game2 = {
   data = {
     pokemon = { CHIKORITA = { name = "CHIKORITA" },
@@ -121,6 +174,60 @@ game2.stack.states = {}
 check(api2:snapshot() == nil, "Gold returns nil outside a battle")
 game2.stack.states = { screen2 }
 
+screen2.message = nil
+screen2.phase = "menu"
+local menu2 = api2:snapshot()
+ok, err = api2:submit({ id = 1, revision = menu2.revision - 1,
+  kind = "menu", choice = "fight" })
+check(not ok and err == "stale battle context",
+  "Gold rejects a stale intent")
+ok, err = api2:submit({ id = 1, revision = menu2.revision,
+  kind = "menu", choice = "missing" })
+check(not ok and err == "unknown battle menu choice",
+  "Gold rejects an unknown menu choice")
+check(api2:submit({ id = 1, revision = menu2.revision,
+  kind = "menu", choice = "fight" }), "Gold accepts a menu intent")
+eq(screen2.chosenMenu, "fight", "Gold uses the semantic menu path")
+local moveMenu2 = api2:snapshot()
+ok, err = api2:submit({ id = 2, revision = moveMenu2.revision,
+  kind = "move", slot = 9 })
+check(not ok and err == "invalid move slot",
+  "Gold rejects an invalid move slot")
+check(api2:submit({ id = 2, revision = moveMenu2.revision,
+  kind = "move", slot = 1 }), "Gold accepts a valid move")
+eq(screen2.chosenMove, 1, "Gold uses the semantic move path")
+screen2.phase = "moves"
+local back2 = api2:snapshot()
+check(api2:submit({ id = 3, revision = back2.revision, kind = "back" }),
+  "Gold accepts move-menu back")
+eq(screen2.phase, "menu", "Gold back restores the command menu")
+
+do
+  local Data = require("tests.modkit").fixtures.fresh()
+  local Pokemon = require("src.pokemon.Pokemon")
+  local SaveData = require("src.core.SaveData")
+  local save = SaveData.newGame()
+  save.party = { Pokemon.new(Data, "FIXMON_A", 20) }
+  local pressed = {}
+  local game3 = { data = Data, save = save, input = {
+    wasPressed = function(_, key) return pressed[key] == true end,
+    isDown = function() return false end,
+  }, stack = { states = {} } }
+  function game3.stack:top() return self.states[#self.states] end
+  function game3.stack:push(state) self.states[#self.states + 1] = state end
+  local real = Gen1BattleState.newWild(game3, "FIXMON_B", 12)
+  real.phase, real.queue, real.introSlide = "menu", {}, nil
+  game3.stack.states = { real }
+  pressed.a = true
+  real:update(1 / 60)
+  pressed.a = nil
+  eq(real.phase, "moveSelect", "native Gen 1 FIGHT uses the semantic path")
+  pressed.b = true
+  real:update(1 / 60)
+  pressed.b = nil
+  eq(real.phase, "menu", "native Gen 1 move-menu back still works")
+end
+
 local Loader = require("src.mods.Loader")
 local fs = { read = function() end, getInfo = function() end,
   getDirectoryItems = function() return {} end }