bazinga

src/battle/BattleState.lua

@@ -1240,6 +1240,31 @@ local function clearTrapping(battler)
   battler.trapDamage = nil
 end
 
+-- core.asm:297-300: both sides' FLINCHED bits are cleared as a turn's move
+-- selection opens, but the clear is skipped for a mon that must recharge or
+-- is locked into Rage (core.asm:293-295 -- the Hyper Beam flinch-recharge
+-- glitch).
+--
+-- A method rather than three lines inside the menu branch, because two
+-- other places need the identical rule at the identical point in the turn
+-- and both got it wrong by not having it:
+--
+--   * guarded PER BATTLER, not off self.player alone.  This runs on
+--     whichever machine is looking at its own menu, and in a lockstep link
+--     battle "self.player" is the host's mon on one peer and the guest's on
+--     the other, so one shared guard let one peer clear both flags while
+--     the other cleared neither -- a bogus desync draw in a winnable match.
+--   * a tournament spectator never enters the menu phase at all (it has no
+--     decision to make), so nothing cleared a flinch in its replay: the
+--     flag survived into the next turn, ate a move the real players saw
+--     land, and from there the replay was watching a different battle.
+--     LinkBattle.newSpectator calls this at the head of every turn.
+function BattleState:clearTurnFlinches()
+  for _, b in ipairs({ self.player, self.enemy }) do
+    if b and not (b.mustRecharge or b.rageMove) then b.flinched = false end
+  end
+end
+
 -- Actions that skip DisplayBattleMenu entirely (core.asm:300-310):
 -- recharge, Rage, thrash, charge.  Bide / trapping / being held do NOT
 -- skip the menu -- the player can still item/switch (and must press
@@ -1260,11 +1285,17 @@ function BattleState:fightLockedAction(battler)
   end
   if battler.bideTurns then return { special = "bide" } end
   -- held while the OPPONENT's trapping bit is set (live mirror so a
-  -- trap ended early by paralysis/faint frees the victim immediately)
+  -- trap ended early by paralysis/faint frees the victim immediately).
+  -- Read, not written: executeAction refreshes battler.boundTurns from the
+  -- same expression when the action actually runs, and that site runs on
+  -- both peers of a link battle.  Storing it here instead wrote a hashed
+  -- field on whichever machine happened to open its own FIGHT menu, which
+  -- left the two peers holding boundTurns=0 against nil for the same
+  -- battler and ended the match as a desync over a mirror of a mirror.
   local opp = battler.isPlayer and self.enemy or self.player
-  battler.boundTurns = opp and opp.trappingTurns
-                       and math.max(1, opp.trappingTurns) or nil
-  if battler.boundTurns then
+  local bound = opp and opp.trappingTurns
+                and math.max(1, opp.trappingTurns) or nil
+  if bound then
     return { special = "bound" }
   end
   return nil
@@ -1301,9 +1332,23 @@ function BattleState:swapMoves(i, j)
   require("src.core.Sound").play(self.data, "Swap")
 end
 
-function BattleState:update(dt)
+-- One frame of the presentational clock: the BGP flash sequences, the
+-- per-battler pic slide/hide programs, the send-out grow-in, the intro
+-- slide and the screen-shake programs all advance in updateFx and nowhere
+-- else.  It lives behind its own entry point because a caller that has to
+-- skip the rest of update() for a frame must still tick this, and a link
+-- battle does exactly that on two hot paths -- waiting on the peer's action,
+-- and draining a resolved lockstep turn.  Skipping it there froze whatever
+-- was mid-flight: a flash stuck on its inverted BGP step repainted the whole
+-- UI in inverted shades, and a pic part-way through a slide-off or a grow-in
+-- simply stayed gone -- for as long as the opponent took to choose.
+function BattleState:tickFx()
   self.frame = self.frame + 1
   self:updateFx()
+end
+
+function BattleState:update(dt)
+  self:tickFx()
   local input = self.game.input
 
   -- safety net: HP/status changed outside a queued drain (level-up heals,
@@ -1386,13 +1431,7 @@ function BattleState:update(dt)
       end
       return
     end
-    -- core.asm:297-300: both sides' FLINCHED bits are cleared during
-    -- move selection, but the clear is skipped while the player must
-    -- recharge or is locked into Rage (core.asm:293-295 -- the Hyper
-    -- Beam flinch-recharge glitch)
-    if not (self.player.mustRecharge or self.player.rageMove) then
-      self.player.flinched, self.enemy.flinched = false, false
-    end
+    self:clearTurnFlinches()
     -- only recharge/Rage/thrash/charge skip DisplayBattleMenu; trapping
     -- victims (and wrappers) still get FIGHT/PKMN/ITEM/RUN (core.asm:312)
     local locked = self:menuLockedAction(self.player)

src/core/Game.lua

@@ -191,6 +191,15 @@ end
 -- or screenshot run does not depend on whatever the player last chose.
 function Game:logicSpeed()
   local GameSpeed = require("src.core.GameSpeed")
+  -- Link play is always 1X on both machines, and this wins over every other
+  -- source including POKEPORT_SPEED.  Fast-forward multiplies the logic
+  -- clock, so a peer at 10X burned a tournament shot clock ten times faster
+  -- than the opponent it is racing, and drove its own animation/message
+  -- queue at a different rate than the peer it is locked to.  Nothing about
+  -- a match should depend on what either player set this to.
+  if self.linkSession or (self.linkNet and not self.linkNet.closed) then
+    return 1
+  end
   if self.speedOverride then return GameSpeed.clamp(self.speedOverride) end
   local opts = self.save and self.save.options
   return GameSpeed.clamp(opts and opts.speed or GameSpeed.DEFAULT)

src/link/LinkBattle.lua

@@ -112,6 +112,19 @@ local function scalar(v)
   return tostring(v)
 end
 
+-- A volatile slot is "off" three interchangeable ways -- absent, false, or
+-- a counter sitting at 0 -- because every reader tests `if b.key then` or
+-- `key > 0`.  They have to hash the same, or a match ends over a difference
+-- that does not exist.  They routinely disagree: the menu-phase flinch
+-- clear (BattleState:update) and the FIGHT-branch boundTurns mirror
+-- (fightLockedAction) are written by whichever machine is sitting at ITS
+-- OWN menu, and in a link battle that is a different battler on each side,
+-- so one peer holds flinched=false / boundTurns=0 where the other still
+-- holds nil with two identical simulations underneath.
+local function off(v)
+  return v == nil or v == false or v == 0
+end
+
 local function stageStr(b)
   local out = {}
   for i, stat in ipairs(STAGES) do
@@ -131,7 +144,7 @@ end
 local function volStr(b)
   local out = {}
   for _, key in ipairs(VOLATILE) do
-    if b[key] ~= nil then
+    if not off(b[key]) then
       out[#out + 1] = key .. "=" .. scalar(b[key])
     end
   end
@@ -168,6 +181,17 @@ end
 
 local PARTS = { "actives", "volatile", "bench" }
 
+-- Which components are allowed to end a match.  `actives` and `bench` carry
+-- what decides one -- species, HP, status, stat stages, PP, the rest of the
+-- party -- so a divergence there is a real split between the two
+-- simulations and stays a draw.  `volatile` is per-turn bookkeeping that
+-- both sides recompute from the authoritative state every turn (see `off`
+-- above): it can disagree for a turn without either side being wrong, and
+-- when it does mean something real it lands in `actives` as damage or
+-- status within a turn or two, where it is caught.  Ending a match on it
+-- alone cost players games they were winning, over nothing.
+local FATAL_PART = { actives = true, bench = true }
+
 -- opts: { myParty = packed, theirParty = packed, theirName, role =
 -- "host"/"guest", seed, verdict, strict }.  Returns nil plus a reason when
 -- the handshake says the two link surfaces don't match: a lockstep
@@ -315,12 +339,23 @@ function LinkBattle.new(game, net, opts)
     Logger.warn("link: desync turn %s component=%s (%s vs %s)",
                 tostring(turn), component, tostring(localH), tostring(remoteH))
     Runtime.emit("link.desync", { turn = turn, component = component,
-                                  localHash = localH, remoteHash = remoteH })
+                                  localHash = localH, remoteHash = remoteH,
+                                  fatal = true })
     endAsDraw(s, Strings(
       "Link desync!\n%s differs.\fAre both games\nrunning the same\nmods?",
       component))
   end
 
+  -- a non-fatal component split: both sides log it and carry on, so the
+  -- match is decided by the battle rather than by bookkeeping
+  local function noteDrift(s, turn, component, localH, remoteH)
+    Logger.warn("link: %s drift on turn %s (%s vs %s) -- match continues",
+                component, tostring(turn), tostring(localH), tostring(remoteH))
+    Runtime.emit("link.desync", { turn = turn, component = component,
+                                  localHash = localH, remoteHash = remoteH,
+                                  fatal = false })
+  end
+
   -- a verified turn stays recorded: consuming it here left a finished
   -- battle holding 0-1 entries, so the whole-battle sweep the link suite
   -- runs over localHashes had nothing left to compare
@@ -333,8 +368,11 @@ function LinkBattle.new(game, net, opts)
         if mine and theirs then
           for _, component in ipairs(PARTS) do
             if mine[component] ~= theirs[component] then
-              reportDesync(s, turn, component, mine[component], theirs[component])
-              return
+              if FATAL_PART[component] then
+                reportDesync(s, turn, component, mine[component], theirs[component])
+                return
+              end
+              noteDrift(s, turn, component, mine[component], theirs[component])
             end
           end
         end
@@ -565,10 +603,19 @@ function LinkBattle.new(game, net, opts)
     if net.closed and not s.linkEnded and not s.result then
       endAsDraw(s)
     end
+    -- Both early returns below skip baseUpdate, which is where the
+    -- presentational clock normally advances -- so tick it here, in the same
+    -- order baseUpdate would (fx, then the queue).  Without this an
+    -- animation caught mid-flight froze for the whole wait: a flash stopped
+    -- on its inverted BGP step and repainted the UI in inverted shades, and
+    -- a pic part-way through a slide or grow-in stayed off screen, which is
+    -- the "the screen went inverted" / "a Pokemon just vanished" pair.
     if s.phase == "waitRemote" then
+      s:tickFx()
       return -- the other side is still choosing
     end
     if s.phase == "messages" and s.afterQueue == "linkNext" then
+      s:tickFx()
       if not s:updateQueue() then
         s.afterQueue = "menu"
         s.phase = "menu"
@@ -742,6 +789,13 @@ function LinkBattle.newSpectator(game, net, opts)
     end
 
     s:act(function()
+      -- the two real players cleared their flinch flags when their move
+      -- menu opened; a spectator has no menu, so it does it here instead,
+      -- at the same point in the turn (see BattleState:clearTurnFlinches).
+      -- Without this a flinch survived into the next turn and ate a move
+      -- that landed in the real match, and the replay -- sharing the RNG
+      -- stream -- was watching a different battle from that point on.
+      s:clearTurnFlinches()
       local hostAction = hostMsg.kind ~= "switch" and hostMsg.kind ~= "run"
                           and decodeWireAction(s, hostMsg, s.player) or nil
       local guestAction = guestMsg.kind ~= "switch" and guestMsg.kind ~= "run"
@@ -837,7 +891,11 @@ function LinkBattle.newSpectator(game, net, opts)
     if net.closed and not s.linkEnded and not s.result then
       endSpectate(s)
     end
+    -- same as the real-participant loop: every path that skips baseUpdate
+    -- still has to advance the presentational clock, and a spectator sits in
+    -- waitBoth between every single turn
     if s.phase == "messages" and s.afterQueue == "linkNext" then
+      s:tickFx()
       if not s:updateQueue() then
         s.afterQueue = "waitBoth"
         s.phase = "waitBoth"
@@ -845,6 +903,7 @@ function LinkBattle.newSpectator(game, net, opts)
       return
     end
     if s.phase == "menu" or s.phase == "waitBoth" then
+      s:tickFx()
       return -- frozen between resolved turns; never a real decision here
     end
     baseUpdate(s, dt)

src/link/LinkState.lua

@@ -73,6 +73,9 @@ end
 function LinkState.new(game)
   local self = setmetatable({}, LinkState)
   self.game = game
+  -- a link session runs at 1X on both machines whatever either player set
+  -- GAME SPEED to (see Game:logicSpeed); cleared in exitWith
+  game.linkSession = true
   self.stage = "menu"
   self.index = 1
   self.addr = ipDigits(Net.lanIP())
@@ -98,6 +101,7 @@ end
 
 function LinkState:exitWith(message, reason)
   DiscordPresence.setJoinCode(nil)
+  self.game.linkSession = nil -- back to the player's own GAME SPEED
   Runtime.emit("link.ended", { reason = reason or (message and "error" or "bye") })
   if self.net then self.net:close() end
   self.game.stack:pop()
@@ -106,6 +110,46 @@ function LinkState:exitWith(message, reason)
   end
 end
 
+-- Online play meets strangers, so it requires vanilla on both ends
+-- (Handshake.onlineAllowed).  Mods merge into the shared Data registries at
+-- boot and there is no unmerge, so switching them off has to go through a
+-- relaunch -- but the player should not have to go find the mod manager and
+-- work out which mods count.  This turns every enabled mod off, records
+-- them so the mod manager can put them back, and relaunches.  The restart
+-- is confirmed rather than silent: it drops unsaved progress.
+function LinkState:offerVanillaRestart()
+  local game = self.game
+  local loader = game.mods
+  local mods = Handshake.mods(game)
+  local names = {}
+  for i, mod in ipairs(mods) do
+    if i > 2 then break end
+    names[#names + 1] = tostring(mod.id):upper():sub(1, 12)
+  end
+  local list = table.concat(names, ", ")
+  if #mods > #names then list = list .. (" +%d"):format(#mods - #names) end
+  local text = Strings(
+    "Online play runs\nvanilla for both\nplayers.\fTurn off %s\nand restart?", list)
+  self.game.linkSession = nil
+  Runtime.emit("link.ended", { reason = "error" })
+  if self.net then self.net:close() end
+  game.stack:pop()
+  game.stack:push(TextBox.new(game, text, nil, { choice = function(yes)
+    if not yes then return end
+    -- setEnabled persists the toggle itself (Loader:_saveState), so the
+    -- relaunch comes up vanilla and the mod manager lists them as disabled
+    -- for the player to switch back on afterwards
+    for _, mod in ipairs(mods) do
+      if loader and loader.setEnabled then loader:setEnabled(mod.id, false) end
+    end
+    if game.restartWithMods then
+      game:restartWithMods()
+    elseif love.event and love.event.quit then
+      love.event.quit("restart")
+    end
+  end }))
+end
+
 -- -------------------------------------------------------------------
 -- handshake v2 (D8): both peers announce engine version, api version and
 -- a fingerprint of their link surface, and the verdict comes from the two
@@ -202,7 +246,7 @@ function LinkState:update(dt)
         self.index = 1
       elseif self.index == 2 or self.index == 3 then
         if not Handshake.onlineAllowed(self.game) then
-          self:exitWith(Strings("Online play needs\nno mods enabled.\fDisable them in\nSTART > MODS."))
+          self:offerVanillaRestart()
           return
         end
         if self.index == 2 then
@@ -509,6 +553,7 @@ function LinkState:updateTrade(input)
     if self.game.writeSave then self.game:writeSave() end
     local name = received.nickname or self.game.data.pokemon[received.species].name
     Runtime.emit("link.ended", { reason = "done" })
+    self.game.linkSession = nil -- this path pops without exitWith
     self.net:close()
     self.game.stack:pop()
     local game = self.game

src/link/Tournament.lua

@@ -91,6 +91,11 @@ function Tournament.new(game)
   self.settingsIndex = 1
   self.roster = {}
   self.spectatorRoster = {}
+  -- everything from here to exitWith runs at 1X regardless of the GAME
+  -- SPEED option (see Game:logicSpeed): a tournament's shot clock counts
+  -- down on the logic step, so fast-forward would hand one player less
+  -- real time to choose than the opponent they are racing
+  game.linkSession = true
   Sound.startLoop(game.data, MUSIC)
   return self
 end
@@ -113,6 +118,7 @@ end
 
 function Tournament:exitWith(message)
   DiscordPresence.setJoinCode(nil)
+  self.game.linkSession = nil -- back to the player's own GAME SPEED
   Sound.stopLoop(MUSIC)
   Runtime.emit("link.ended", { reason = message and "error" or "bye" })
   if self.net then self.net:close() end