Fix transition state pop, Gen 2 Bide/Sleep Talk/exp bugs, pp repair, TM pocket capacity and Bill's PC arrows

CLOSES #1663, CLOSES #1664, CLOSES #1665, CLOSES #1666, CLOSES #1667, CLOSES #1668, CLOSES #1670, CLOSES #1672

src/battle/gen2/Battle.lua

@@ -903,8 +903,12 @@ function Battle:movePriority(moveId)
   return (def and Battle.PRIORITY[def.effect]) or 0
 end
 
+-- engine/battle/effect_commands.asm:192-197 (enemy twin :383-390)
+Battle.SLEEP_BYPASS_MOVES = { SNORE = true, SLEEP_TALK = true }
+
 -- Can this mon act?  Returns true, or false plus the message the cart prints.
-function Battle:canAct(mon)
+-- `moveId` is wCurPlayerMove / wCurEnemyMove (effect_commands.asm:193).
+function Battle:canAct(mon, moveId)
   local name = self:monName(mon)
   -- SUBSTATUS_RECHARGE, and it is checked BEFORE status: CheckPlayerTurn reads
   -- it first, clears it, prints MustRechargeText and jumps to EndTurn, so a mon
@@ -925,7 +929,11 @@ function Battle:canAct(mon)
   local beforeMove = record and record.beforeMove
   if beforeMove
       and (record.beforeMovePriority or 0) > Battle.VOLATILE_PRIORITY then
-    return beforeMove(self, mon, name) and true or false
+    -- engine/battle/effect_commands.asm:188-200
+    local bypass = mon.status == "sleep" and Battle.SLEEP_BYPASS_MOVES[moveId]
+    local acted = beforeMove(self, mon, name) and true or false
+    if acted or not bypass then return acted end
+    beforeMove = nil
   end
   -- SUBSTATUS_FLINCHED, read and cleared right after the freeze check
   -- (CheckPlayerTurn / CheckEnemyTurn `.not_frozen`).  Set this turn by the
@@ -1389,7 +1397,14 @@ function Battle:useMove(attacker, defender, moveId)
   -- free of PP and obedience in exactly the same way.
   local rolling = state.rolloutLock == moveId
 
-  if not (charging or rampaging or rolling) then
+  -- engine/battle/effect_commands.asm:977-979, data/moves/effects.asm:795-800,
+  -- engine/battle/move_effects/bide.asm:62-68
+  local biding = def.effect == "EFFECT_BIDE" and state.bideTurns ~= nil
+
+  -- engine/battle/effect_commands.asm:6222-6234, :949-951
+  local called = (self.copyDepth or 0) > 0
+
+  if not (charging or rampaging or rolling or biding or called) then
     if move and (move.pp or 0) <= 0 then
       self:emit({ kind = "message", text = "No PP left for this move!" })
       return
@@ -1471,9 +1486,38 @@ function Battle:useMove(attacker, defender, moveId)
     return
   end
 
+  -- engine/battle/move_effects/sleep_talk.asm:2, :16-19, :61
+  if def.effect == "EFFECT_SLEEP_TALK" then
+    local picked
+    if attacker.status == "sleep" and (self.copyDepth or 0) == 0 then
+      -- engine/battle/move_effects/sleep_talk.asm:40-44, :117-141
+      local pool = {}
+      for _, own in ipairs(attacker.moves or {}) do
+        local ownDef = self:moveDef(own.id)
+        local effect = ownDef and ownDef.effect
+        if own.id ~= moveId and not self:moveDisabled(attacker, own.id)
+            and not Effects.CHARGE[effect] and effect ~= "EFFECT_BIDE" then
+          pool[#pool + 1] = own.id
+        end
+      end
+      if #pool > 0 then picked = pool[rand(self.random, #pool) + 1] end
+    end
+    if not picked then
+      self:markMissed()
+      self:emit({ kind = "message", text = "But it failed!" })
+      return
+    end
+    state.lastMove = nil
+    self.copyDepth = (self.copyDepth or 0) + 1
+    self:useMove(attacker, defender, picked)
+    self.copyDepth = self.copyDepth - 1
+    return
+  end
+
   -- Everything past here counts as "the user's last move" for Mirror Move,
-  -- Encore and Disable.
-  state.lastMove = moveId
+  -- Encore and Disable.  A called move skips the write
+  -- (engine/battle/used_move_text.asm:30-36).
+  if (self.copyDepth or 0) == 0 then state.lastMove = moveId end
   state.turnsTaken = (state.turnsTaken or 0) + 1
   state.usedMoves = state.usedMoves or {}
   local seen = false
@@ -1516,6 +1560,13 @@ function Battle:useMove(attacker, defender, moveId)
     return
   end
 
+  -- BattleCommand_Snore (engine/battle/move_effects/snore.asm:1-9)
+  if def.effect == "EFFECT_SNORE" and attacker.status ~= "sleep" then
+    self:markMissed()
+    self:emit({ kind = "message", text = "But it failed!" })
+    return
+  end
+
   -- Counter and Mirror Coat answer what the user took this turn, at double,
   -- and fail outright when nothing of the right kind landed.
   local counterKind = Effects.COUNTER[def.effect]
@@ -2133,6 +2184,8 @@ Battle.MOVE_EFFECTS.EFFECT_BIDE = function(self, attacker, defender, def, moveId
   if not state.bideTurns then
     state.bideTurns = Effects.bideTurns(self.random)
     state.bideStored = 0
+    -- engine/battle/core.asm:574-576
+    state.bideMove = moveId
     self:emit({ kind = "message",
       text = self:monName(attacker) .. " is storing energy!" })
     return
@@ -2144,7 +2197,7 @@ Battle.MOVE_EFFECTS.EFFECT_BIDE = function(self, attacker, defender, def, moveId
     return
   end
   local damage = Effects.bideDamage(state.bideStored)
-  state.bideTurns, state.bideStored = nil, nil
+  state.bideTurns, state.bideStored, state.bideMove = nil, nil, nil
   self:emit({ kind = "message",
     text = self:monName(attacker) .. " unleashed energy!" })
   if damage <= 0 then return fail(self) end
@@ -3092,6 +3145,12 @@ end
 
 -- Faint bookkeeping and experience.  Returns true when the battle ended.
 function Battle:resolveFaints()
+  -- engine/battle/core.asm:2551-2556, :7116-7130, :3033-3037
+  if (self.player.hp or 0) <= 0 and self.participantsCleared ~= self.player then
+    self.participantsCleared = self.player
+    if self.playerIndex then self.participants[self.playerIndex] = nil end
+  end
+
   if (self.enemy.hp or 0) <= 0 then
     self:emit({ kind = "faint", side = "enemy",
       text = (self.wild and "Wild " or "") .. self:monName(self.enemy)
@@ -3547,6 +3606,7 @@ function Battle:switch(index)
   -- A mon that comes back (a REVIVE, or a second battle) has to be able to
   -- announce its own faint again; see resolveFaints.
   self.faintAnnounced = nil
+  self.participantsCleared = nil
   -- ForcePlayerMonChoice has been answered, so the next faint may ask again.
   self.pendingSwitch = nil
   self.player = mon
@@ -3784,19 +3844,37 @@ function Battle:lockedInMove(mon)
   return nil
 end
 
+-- ParsePlayerAction's bide arm (engine/battle/core.asm:569-576), enemy twin
+-- at :5650
+function Battle:fightLockedMove(mon)
+  local state = self:volatile(mon)
+  if state.bideTurns then return state.bideMove end
+  return nil
+end
+
+-- engine/battle/core.asm:627-629
+function Battle:cancelBide(mon)
+  local state = self:volatile(mon)
+  state.bideTurns, state.bideStored, state.bideMove = nil, nil, nil
+end
+
 -- Encore forces the move; Disable forbids one.  Both are read by the screen
 -- (to grey out the move list) and by the enemy's own choice below.
 function Battle:forcedMove(mon)
   local locked = self:lockedInMove(mon)
   if locked then return locked end
   local state = self:volatile(mon)
-  if not state.encore then return nil end
-  for _, move in ipairs(mon.moves or {}) do
-    if move.id == state.encore and (move.pp or 0) > 0 then return state.encore end
+  -- ParsePlayerAction reads SUBSTATUS_ENCORED ahead of the bide arm
+  -- (engine/battle/core.asm:561-566).
+  if state.encore then
+    for _, move in ipairs(mon.moves or {}) do
+      if move.id == state.encore and (move.pp or 0) > 0 then
+        return state.encore
+      end
+    end
+    state.encore, state.encoreTurns = nil, nil
   end
-  -- Encore ends early when the move runs out of PP.
-  state.encore, state.encoreTurns = nil, nil
-  return nil
+  return self:fightLockedMove(mon)
 end
 
 function Battle:moveDisabled(mon, moveId)
@@ -3810,8 +3888,9 @@ function Battle:usableMoves(mon)
   -- .CheckPlayerHasUsableMoves (core.asm:533-556), so a Rollout or a rampage
   -- that spent its last PP on the opening turn keeps running: no later turn
   -- of the lock spends any.  Encore is not in this exemption -- forcedMove
-  -- ends it the moment the encored move runs dry.
-  local locked = self:lockedInMove(mon)
+  -- ends it the moment the encored move runs dry.  Bide is exempt too:
+  -- .CheckPlayerHasUsableMoves lives inside MoveSelectionScreen (core.asm:5058).
+  local locked = self:lockedInMove(mon) or self:fightLockedMove(mon)
   local out = {}
   for _, move in ipairs(mon.moves or {}) do
     local ok = (move.pp or 0) > 0 and not self:moveDisabled(mon, move.id)
@@ -4105,9 +4184,14 @@ function Battle:vanillaEnemyMove()
   -- move answers "TYPHLOSION's attack missed!", so Hitmonlee cannot be damaged
   -- by anything, at any level.  Fifteen straight attempts at the Elite Four
   -- died there, and no amount of grinding could ever have got past it.
-  local charged = self:volatile(self.enemy).chargeMove
+  local enemyState = self:volatile(self.enemy)
+  local charged = enemyState.chargeMove
   if charged then return charged end
 
+  -- engine/battle/core.asm:5650, :5532-5533.  Straight off the volatile, the
+  -- way `charged` above is, so the charge-lock test's bare stub still drives it.
+  if enemyState.bideTurns then return enemyState.bideMove end
+
   -- Encore and Disable narrow the pool before the AI ever scores it.
   local moves = self:usableMoves(self.enemy)
   if #moves == 0 then
@@ -4201,6 +4285,7 @@ local function runTurn(self, action)
     -- (engine/battle/core.asm:5035-5038), which reopens the 2x2 menu with the
     -- turn unspent -- so a refused RUN never bought the enemy a free attack.
     if self.runRefused then return self:takeEvents() end
+    self:cancelBide(self.player)
     action = { kind = "skip" }
   end
 
@@ -4216,6 +4301,9 @@ local function runTurn(self, action)
   if action.kind == "item" and Battle.X_ITEMS[action.item] then
     Happiness.change(self.player, "USEDXITEM")
   end
+  -- engine/battle/core.asm:572-573 into :627-629; a switch takes :570-571
+  -- instead and keeps the store.
+  if action.kind == "item" then self:cancelBide(self.player) end
 
   -- AI_SwitchOrTryItem runs BEFORE the move is chosen: a trainer that decides
   -- to rotate or drink a potion spends its whole turn on it.
@@ -4264,7 +4352,6 @@ local function runTurn(self, action)
 
   local function playerAttack()
     if action.kind ~= "move" then return end
-    if not self:canAct(self.player) then return end
     local move = action.move
     -- An encored mon has no choice, whatever the menu said.
     local forced = self:forcedMove(self.player)
@@ -4284,13 +4371,20 @@ local function runTurn(self, action)
     -- player's own mon spends the rest of the battle underground.
     local stored = self:volatile(self.player).chargeMove
     if stored then move = stored end
+    -- engine/battle/core.asm:558-598 settles wCurPlayerMove before
+    -- engine/battle/effect_commands.asm:193 reads it.
+    if not self:canAct(self.player, move) then return end
     -- CheckPlayerLockedIn quits before .CheckPlayerHasUsableMoves and before
     -- checkobedience, so a locked Rollout or Thrash is exempt from the
     -- Struggle substitution and the obedience roll the same way the second
     -- half of a charge move is.
     local charging = self:volatile(self.player).chargeMove == move
       or self:lockedInMove(self.player) == move
-    if not charging and not self:hasUsableMoves(self.player) then
+    -- engine/battle/core.asm:5058, and data/moves/effects.asm:796 keeps
+    -- `checkobedience`.
+    local bideLocked = self:fightLockedMove(self.player) == move
+    if not charging and not bideLocked
+        and not self:hasUsableMoves(self.player) then
       self:emit({ kind = "message",
         text = self:monName(self.player) .. " has no moves left!" })
       move = Battle.STRUGGLE
@@ -4328,7 +4422,7 @@ local function runTurn(self, action)
       -- against a trainer, could not be escaped either.
       enemyMoveId = Battle.STRUGGLE
     end
-    if not self:canAct(self.enemy) then return end
+    if not self:canAct(self.enemy, enemyMoveId) then return end
     -- CheckEnemyTurn's disabled arm (engine/battle/effect_commands.asm:562-574):
     -- the AI chose before the player's Disable landed, so the turn is spent here.
     if self:moveDisabled(self.enemy, enemyMoveId) then

src/core/SaveData.lua

@@ -1700,6 +1700,14 @@ local function clamp(n, lo, hi, fallback)
   return n
 end
 
+-- PP and the PP Up count are unsigned bit fields of one byte
+-- (constants/pokemon_data_constants.asm:101-102)
+local function ppInt(v, fallback)
+  local n = tonumber(v)
+  if n == nil or n ~= n or n == math.huge or n == -math.huge then return fallback end
+  return math.max(0, math.floor(n))
+end
+
 local function ensureOrphaned(save)
   if not save.orphaned then
     save.orphaned = { mons = {}, items = {} }
@@ -1763,7 +1771,7 @@ local function scrubKnownMon(mon, data)
   -- (status_screen.asm:66-76, add_mon.asm _MoveMon); deriving once here means
   -- every later reader (menus, battle, items, SGB bar zones, the link
   -- fingerprint) sees a party-shaped mon.  Runs after the level clamp above
-  -- so the derived stats use a sane level.  A save that already has stats is
+  -- so the derived stats use a sane level.  A complete stat block is
   -- untouched.
   Stats.ensure(data.pokemon and data.pokemon[mon.species], mon)
   local moves = mon.moves
@@ -1783,6 +1791,18 @@ local function scrubKnownMon(mon, data)
       moves[1] = { id = fallback, pp = def.pp }
     end
   end
+  -- the replacement PP mirrors AddBonusPP (engine/items/item_effects.asm:2418);
+  -- Mimic rewrites the move id and not the PP (engine/battle/effects.asm:1266)
+  for j = 1, #moves do
+    local slot = moves[j]
+    if type(slot) == "table" then
+      local mdef = data.moves and data.moves[slot.id]
+      local base = ppInt(mdef and mdef.pp, 0)
+      local ppUps = math.min(3, ppInt(slot.ppUps, 0))
+      if slot.ppUps ~= nil then slot.ppUps = ppUps end
+      slot.pp = ppInt(slot.pp, base + ppUps * math.floor(base / 5))
+    end
+  end
 end
 
 local function scrubMonList(list, where, save, data, report)

src/import/RomImporter.lua

@@ -146,8 +146,7 @@ local VERSION_REQUIRED_FILES_OVERRIDE = {
     -- complete and SlotMachine crashed on its labelled-cell fallback.
     "assets/generated/slots/gold_slots_1.png",
     "assets/generated/card_flip/card_flip_1.png",
-    -- PCMailGFX (engine/pokemon/bills_pc.asm:2170-2173): Bill's PC has no
-    -- held-item or mail icon in a cache built before the symbol was listed.
+    -- PCMailGFX (engine/pokemon/bills_pc.asm:2170-2173)
     "assets/generated/pc/mail_item.png",
   },
 }

src/inventory/Bag.lua

@@ -10,13 +10,14 @@
 local Bag = {}
 
 -- MAX_ITEMS / MAX_BALLS / MAX_KEY_ITEMS, and the TM/HM pocket holds one of
--- every TM plus the seven HMs (NUM_TMS + NUM_HMS).  A `mods` bagSize override
+-- every TM plus the seven HMs (NUM_TMS + NUM_HMS -- ram/wram.asm:2421 is
+-- `wTMsHMs:: ds NUM_TMS + NUM_HMS`, 57 bytes).  A `mods` bagSize override
 -- replaces the ITEM pocket only, the way the Gen 1 single-bag config did.
 local POCKET_CAPACITY = {
   ITEM = 20,
   BALL = 12,
   KEY_ITEM = 25,
-  TM_HM = 64,
+  TM_HM = 57,
 }
 local DEFAULT_CAPACITY = 20
 

src/mods/Gen2Compat.lua

@@ -811,19 +811,21 @@ local function buildOverworld()
   local FieldMoves = rawRequire("src.world.gen2.FieldMoves")
 
   -- home/map.asm:1869
-  local function stepAllowed(world, entity, dir)
+  local function stepAllowed(world, entity, dir, cx, cy)
     local d = Map2.DELTA[dir]
-    if not (world.map and d and entity and entity.cellX) then return true end
+    if not (world.map and d and entity) then return true end
+    cx, cy = cx or entity.cellX, cy or entity.cellY
+    if not (cx and cy) then return true end
     if not Permissions.stepPermitted(
-         function(cx, cy) return world:cellCollisionAcross(world.map, cx, cy) end,
-         entity.cellX, entity.cellY, dir) then
+         function(px, py) return world:cellCollisionAcross(world.map, px, py) end,
+         cx, cy, dir) then
       return false
     end
     local map = world.map
     if entity == world.player and FieldMoves.isSurfing(world.playerState) then
       map = world:surfMap(world.map)
     end
-    local tx, ty = entity.cellX + d[1], entity.cellY + d[2]
+    local tx, ty = cx + d[1], cy + d[2]
     if not map:inBounds(tx, ty) or not map:isWalkable(tx, ty) then return false end
     for _, e in ipairs(world.entities or {}) do
       if e ~= entity and not e.passable then
@@ -1048,9 +1050,17 @@ local function buildOverworld()
         .. "mapped object (def.index) can be moved"
     end
     local n = math.max(0, tiles or 1)
-    if opts and opts.collide and n > 0 and not stepAllowed(world, entity, dir) then
-      entity.facing = dir
-      n = 0
+    if opts and opts.collide and n > 0 then
+      local d = Map2.DELTA[dir]
+      local cx, cy = entity.cellX, entity.cellY
+      local allowed = 0
+      for _ = 1, n do
+        if not stepAllowed(world, entity, dir, cx, cy) then break end
+        allowed = allowed + 1
+        if d and cx and cy then cx, cy = cx + d[1], cy + d[2] end
+      end
+      if allowed < n then entity.facing = dir end
+      n = allowed
     end
     local bytes = {}
     for _ = 1, n do bytes[#bytes + 1] = step end
@@ -1431,7 +1441,8 @@ COVERAGE[OW] = {
       .. "objectId 1 is wLastTalked, not object zero",
     scriptMove = "the player maps to objectId 0 and a mapped object to "
       .. "def.index + 1; an entity with neither (a mod's own guest) is "
-      .. "REFUSED with a reason rather than moving the last-talked NPC",
+      .. "REFUSED with a reason rather than moving the last-talked NPC; "
+      .. "opts.collide truncates the walk at the first blocked step",
     connectionLanding = "Gen 1's five values (destDef, tilesetDef, x, y, "
       .. "conn); `conn` is Gold's connection record, keyed map/mapId + offset",
     timeOfDay = "recomputed from the clock every call and never cached, so a "

src/pokemon/Stats.lua

@@ -52,7 +52,9 @@ end
 -- CalcStats) and when one is moved back into the party
 -- (engine/pokemon/add_mon.asm _MoveMon tail).  The stored current HP is
 -- kept (box_struct does hold it) but clamped to the recalculated maximum.
--- CalcStats writes all NUM_STATS stats or none (home/move_mon.asm:33-48).
+-- A complete stat block is returned untouched; an incomplete one is rebuilt,
+-- because CalcStats writes all NUM_STATS stats or none
+-- (home/move_mon.asm:33-48).  #233, #304, #1517
 local function statsComplete(stats)
   for _, key in ipairs(ORDER) do
     if type(stats[key]) ~= "number" then return false end

src/render/Transition.lua

@@ -85,7 +85,9 @@ function Transition.new(game, onMidpoint, onDone, warp)
 end
 
 function Transition:finish()
-  self.game.stack:pop()
+  if self.done then return end
+  self.done = true
+  if not self.offStack then self.game.stack:pop() end
   if self.onDone then self.onDone() end
 end
 
@@ -96,10 +98,16 @@ function Transition:update(dt)
     self.t = 0
     if self.phase == "out" then
       self.phase = "in"
-      if self.onMidpoint then self.onMidpoint() end
       -- LoadGBPal restores the palettes in one write, so with no fade in the
       -- map is simply there on the next frame
-      if (self.framesIn or 0) <= 0 then self:finish() end
+      if (self.framesIn or 0) <= 0 then
+        self.offStack = true
+        self.game.stack:pop()
+        if self.onMidpoint then self.onMidpoint() end
+        self:finish()
+      elseif self.onMidpoint then
+        self.onMidpoint()
+      end
     else
       self:finish()
     end

src/ui/gen2/BoxMenu.lua

@@ -66,7 +66,7 @@ local PIC_X, PIC_Y = 1, 4
 -- 7-size blank tiles per column (engine/gfx/load_pics.asm:342-386).
 local PIC_PAD = { [7] = { 0, 0 }, [6] = { 1, 1 }, [5] = { 1, 2 } }
 
--- gfx/pc/orange.pal, for a cache that predates menu_gfx.billsPc.
+-- gfx/pc/orange.pal
 local BILLS_PC_ORANGE = {
   { 255, 123, 0 }, { 189, 99, 0 }, { 123, 58, 0 }, { 0, 0, 0 },
 }
@@ -75,6 +75,12 @@ local BILLS_PC_ORANGE = {
 -- (engine/pokemon/bills_pc.asm:1093).
 local ICON_X, ICON_Y = 7, 12
 
+-- $5f at hlcoord 8, 1 and $5e at hlcoord 19, 1, off a PCMailGFX sheet that
+-- starts at $5c (engine/pokemon/bills_pc.asm:957-963).
+local ARROW_ROW = 1
+local ARROW_LEFT = { 3, 8 }
+local ARROW_RIGHT = { 2, 19 }
+
 -- wBillsPC_LoadedBox: 0 is the PARTY, 1..NUM_BOXES are the boxes.  Only the
 -- MOVE screen ever loads box 0; the withdraw and deposit lists are one list
 -- each (BillsPC_BoxName reads the same byte for all three).
@@ -550,7 +556,7 @@ function BoxMenu:playSfx(name)
   if sfx and sfx[Sound.resolve(data, name)] then Sound.play(data, name) end
 end
 
--- PlayMonCry: `call GetCryIndex / jr c, .done` (home/pokemon.asm:101)
+-- PlayMonCry: `call GetCryIndex / jr c, .done` (home/pokemon.asm:113-114)
 function BoxMenu:playMonCry(mon)
   local data = self.game and self.game.data
   if not (data and mon and mon.species) or mon.isEgg then return end
@@ -765,6 +771,35 @@ function BoxMenu:drawHeldIcon(mon)
   G.setColor(1, 1, 1, 1)
 end
 
+-- _MovePKMNWithoutMail only (engine/pokemon/bills_pc.asm:545, :698)
+function BoxMenu:drawBoxArrows()
+  if self.mode ~= "move" then return end
+  local gfx = (self.menuGfx or {}).billsPc
+  local image = self:image(gfx and gfx.icons)
+  if not image then return end
+  local G = love.graphics
+  local quads = {}
+  for _, arrow in ipairs({ ARROW_LEFT, ARROW_RIGHT }) do
+    local ok, quad = pcall(love.graphics.newQuad, arrow[1] * 8, 0, 8, 8,
+      image:getDimensions())
+    if not ok then return end
+    quads[#quads + 1] = { quad, arrow[2] }
+  end
+  G.setColor(1, 1, 1, 1)
+  local function body()
+    for _, entry in ipairs(quads) do
+      G.draw(image, entry[1], entry[2] * 8, ARROW_ROW * 8)
+    end
+  end
+  local colors = gfx and gfx.palette
+  if colors and GbcPalette.available() then
+    GbcPalette.with(colors, body)
+  else
+    body()
+  end
+  G.setColor(1, 1, 1, 1)
+end
+
 -- The PC does not mark the selected row with a ▶: BillsPC_UpdateSelectionCursor
 -- lays 20 OBJs as a frame *around* the row -- ten tiles wide by two tall, top
 -- left at pixel (71, 25), stepping 16 pixels per row.  Those cursor tiles are
@@ -811,6 +846,7 @@ function BoxMenu:drawPanel()
   -- Textbox at (8,0) with a 10x1 interior and the name at (10,1).
   Chrome.box(8, 0, 12, 3)
   Chrome.print(self:title(), 10, 1)
+  self:drawBoxArrows()
   Chrome.box(8, 2, 12, 12)
   -- BillsPC_RefreshTextboxes overwrites its own top corners with '└'/'┘'
   -- (engine/pokemon/bills_pc.asm:1204-1211) so the list reads as hanging

src/ui/gen2/ItemPcMenu.lua

@@ -521,9 +521,7 @@ function ItemPcMenu:drawList()
       local entry = self.rows[i]
       if i == self.listIndex then Chrome.cursor(5, ty) end
       Chrome.print(entry.name, 6, ty)
-      -- PlaceMenuItemQuantity (engine/menus/menu_2.asm:24): the xNN is the
-      -- entry's second line, right-aligned in a blank-padded 2-digit field,
-      -- and an item with no quantity draws none at all (menu_2.asm:18).
+      -- PlaceMenuItemQuantity (engine/menus/menu_2.asm:18, :24)
       if not self:cantToss(entry.id) then
         Chrome.print(TIMES .. Chrome.number(entry.count, 2), 7, ty + 1)
       end

src/ui/gen2/PackMenu.lua

@@ -16,6 +16,9 @@ local PackGfx = require("src.ui.gen2.PackGfx")
 local Screens = require("src.ui.Screens")
 local Strings = require("src.core.Strings")
 
+-- constants/sfx_constants.asm:3, :28
+local SFX_DEX_FANFARE_50_79, SFX_WRONG = 0, 25
+
 local PackMenu = {}
 PackMenu.__index = PackMenu
 PackMenu.isOpaque = true
@@ -247,8 +250,7 @@ function PackMenu:rebuild()
       }
     end
   end
-  -- engine/items/tmhm.asm:341 -- wTMsHMs is walked 1..57, so the TM/HM pocket
-  -- has no acquisition order to preserve.
+  -- engine/items/tmhm.asm:341
   if pocket == "TM_HM" then
     table.sort(rows, function(a, b)
       local ka, kb = self:tmhmKey(a.id), self:tmhmKey(b.id)
@@ -377,7 +379,7 @@ function PackMenu:useSelected()
       -- all four rows at once, the way OAK_THIS_ISNT_THE_TIME's three fit.
       -- World has already set the decoration's flag and taken the box.
       if world.playSfxNamed then
-        world:playSfxNamed("Sfx_DexFanfare5079")
+        world:playSfxNamed("Sfx_DexFanfare5079", SFX_DEX_FANFARE_50_79)
       end
       self.message = { "There was a trophy", "inside!",
                        "{PLAYER} sent the", "trophy home." }
@@ -663,7 +665,9 @@ function PackMenu:openTeachParty(row)
       if not allowed then
         -- engine/items/tmhm.asm:131
         local world = game.world
-        if world and world.playSfxNamed then world:playSfxNamed("Sfx_Wrong") end
+        if world and world.playSfxNamed then
+          world:playSfxNamed("Sfx_Wrong", SFX_WRONG)
+        end
         if game.say then
           game:say(("%s can't learn %s!"):format(
             require("src.battle.gen2.Mon").displayName(mon), moveName))

src/world/gen2/World.lua

@@ -2184,8 +2184,8 @@ function World:playSfxNamed(want, fallbackId)
   self:playSfx(self:sfxIdNamed(want, fallbackId))
 end
 
--- .BumpSound (engine/overworld/player_movement.asm:771): `call CheckSFX /
--- ret c` is the whole rate limit (home/audio.asm:477), not a frame counter.
+-- .BumpSound (engine/overworld/player_movement.asm:771), CheckSFX at
+-- home/audio.asm:477
 function World:bumpSound()
   if Sound.sfxBusy() then return end
   self:playSfxNamed("Sfx_Bump", SFX.BUMP)