diff --git a/lib/continue_singleplayer.lua b/lib/continue_singleplayer.lua new file mode 100644 index 00000000..07f77376 --- /dev/null +++ b/lib/continue_singleplayer.lua @@ -0,0 +1,28 @@ +-- Pure decision for G.FUNCS:continue_in_singleplayer (ui/game/functions.lua). +-- +-- Context: vanilla save_run() synchronously fills in G.ARGS.save_run, but the +-- actual save.jkr write is dispatched to the async SAVE_MANAGER worker thread +-- on a later Game:update tick. The old "continue in singleplayer" handler read +-- save.jkr back off disk immediately after calling save_run(), racing that +-- write. Because G.F_NO_SAVING is true for an entire MP match, there is also no +-- prior save on disk to fall back on -- the read reliably came back nil and +-- G:start_run silently began a brand-new run instead of resuming the match. +-- +-- The fix captures the in-memory snapshot (G.ARGS.save_run, deep-copied before +-- G:delete_run() can mutate the live G.GAME table it still references) instead +-- of round-tripping through disk. This function is the pure "what do we do +-- with the captured snapshot" decision, so it's testable without any I/O. +-- +-- savetext: the captured save table, or nil if save_run() didn't produce one. +-- Returns a plain-data command: +-- { action = "continue", savetext = } +-- { action = "abort", reason = } +function MP.UTILS.decide_continue_singleplayer(savetext) + if type(savetext) ~= "table" then + return { + action = "abort", + reason = "continue_in_singleplayer: no save snapshot captured, aborting to avoid starting a fresh run", + } + end + return { action = "continue", savetext = savetext } +end diff --git a/tests/test_continue_singleplayer.lua b/tests/test_continue_singleplayer.lua new file mode 100644 index 00000000..0a3778df --- /dev/null +++ b/tests/test_continue_singleplayer.lua @@ -0,0 +1,94 @@ +--[[ + "Continue in singleplayer" resume decision. + + Bug: the end-of-match "Continue in singleplayer" handler + (G.FUNCS:continue_in_singleplayer, ui/game/functions.lua) used to call + save_run() then immediately read save.jkr back off disk and hand that to + G:start_run(). Vanilla save_run() only synchronously fills in + G.ARGS.save_run -- the actual disk write is dispatched to the async + SAVE_MANAGER worker thread on a later Game:update tick. Reading the file + back immediately raced that write, and because G.F_NO_SAVING is true for + the whole MP match there was no prior save on disk to fall back on either, + so the read reliably came back nil and G:start_run silently began a brand + new run instead of resuming. + + The fix (lib/continue_singleplayer.lua) captures G.ARGS.save_run in memory + right after save_run() returns and skips the disk round trip entirely. + MP.UTILS.decide_continue_singleplayer is the pure "what do we do with the + captured snapshot" decision extracted out of that handler: given a captured + save table (or nil), decide whether to continue with it or abort rather + than silently starting a fresh run. + + This test asserts the new decision function's behavior (green), and -- since + the function is newly extracted and has no pre-fix equivalent to run + unmodified -- also runs an inlined reproduction of the OLD racy-disk-read + logic through the same assertion to demonstrate it fails for the documented + reason (red control). + + Run from the repo root: + luajit tests/test_continue_singleplayer.lua +]] + +MP = { UTILS = {} } + +dofile("lib/continue_singleplayer.lua") +local decide = assert(MP.UTILS.decide_continue_singleplayer, "MP.UTILS.decide_continue_singleplayer not defined after load") + +-- ─── New (fixed) logic: table in vs continue-with-that-table out ─────────── + +do + local savetext = { GAME = { dollars = 4 }, BLIND = { name = "Small Blind" } } + local decision = decide(savetext) + assert(decision.action == "continue", "expected continue, got " .. tostring(decision.action)) + assert(decision.savetext == savetext, "decision must carry through the exact captured table") +end + +-- ─── New (fixed) logic: nil guard -- abort, never silently start fresh ───── + +do + local decision = decide(nil) + assert(decision.action == "abort", "expected abort for nil savetext, got " .. tostring(decision.action)) + assert(type(decision.reason) == "string" and #decision.reason > 0, "abort must carry a non-empty reason") + assert(decision.savetext == nil, "abort must not carry a savetext to start_run with") +end + +-- ─── New (fixed) logic: non-table savetext also aborts (defensive) ───────── + +do + local decision = decide(false) + assert(decision.action == "abort", "expected abort for non-table savetext, got " .. tostring(decision.action)) +end + +print("continue_singleplayer (fixed): all assertions passed") + +-- ─── Regression control: the OLD racy-disk-read logic, inlined ──────────── +-- +-- Mirrors the removed handler body exactly: +-- G.SAVED_GAME = get_compressed(save_path) +-- if G.SAVED_GAME ~= nil then G.SAVED_GAME = STR_UNPACK(G.SAVED_GAME) end +-- G:start_run({ savetext = G.SAVED_GAME }) +-- +-- It always proceeds to start_run (no abort path existed), with whatever the +-- disk read produced. disk_read_result models get_compressed()+STR_UNPACK(): +-- nil when the async save.jkr write from THIS run hasn't landed yet -- which, +-- per the bug, is every time, since G.F_NO_SAVING held for the entire MP +-- match and there's no earlier save.jkr to fall back on. +local function decide_continue_singleplayer_OLD(disk_read_result) + return { action = "continue", savetext = disk_read_result } +end + +do + local ok, err = pcall(function() + local disk_read_result = nil -- the async write hasn't flushed; no prior save.jkr exists either + local decision = decide_continue_singleplayer_OLD(disk_read_result) + assert(decision.savetext ~= nil, "BUG: continuing with a nil savetext starts a brand-new run instead of resuming") + end) + assert(not ok, "control: OLD logic was expected to fail this assertion (nil savetext -> fresh run bug), but it passed") + assert( + tostring(err):find("BUG: continuing with a nil savetext starts a brand-new run instead of resuming", 1, true), + "control failed for the wrong reason: " .. tostring(err) + ) + print("continue_singleplayer (OLD, control): failed as expected -- " .. tostring(err)) +end + +print("continue_singleplayer: all checks complete (fix green, pre-fix control red)") diff --git a/ui/game/functions.lua b/ui/game/functions.lua index cff22a8e..a6df165f 100644 --- a/ui/game/functions.lua +++ b/ui/game/functions.lua @@ -247,7 +247,11 @@ end function G.FUNCS:continue_in_singleplayer(e) -- Detach from the multiplayer lobby (the API owns leave now; the legacy -- MP.ACTIONS.leave_lobby() is a no-op here), then update UI. The run itself is kept - -- and reloaded below so the player continues solo. + -- and reloaded below so the player continues solo. lobby:leave() is async + -- (server round trip) for a live lobby, but its completion only clears mod + -- focus/UI state (see BalatroMultiplayerAPI on_lobby_disconnected) -- it + -- never touches G.GAME/G.ARGS, so it's safe regardless of when it lands + -- relative to the reload below. MP.LOBBY.code = nil local lobby = MPAPI.get_current_lobby() if lobby then @@ -255,22 +259,32 @@ function G.FUNCS:continue_in_singleplayer(e) end MP.UI.update_connection_status() - -- Allow saving, save the run, and set up for continuation + -- Allow saving and build the save table. save_run() synchronously fills in + -- G.ARGS.save_run; the disk write it queues happens later, off-thread, so we + -- don't wait for it (see MP.UTILS.decide_continue_singleplayer for why the + -- old disk round trip was racy). Deep-copy the snapshot now, before + -- G:delete_run() mutates the live G.GAME table it still references + -- (e.g. G.GAME.won = false). G.F_NO_SAVING = false G.SETTINGS.current_setup = "Continue" - G.FUNCS.wipe_on() save_run() + local savetext = G.ARGS and G.ARGS.save_run and copy_table(G.ARGS.save_run) or nil + + local decision = MP.UTILS.decide_continue_singleplayer(savetext) + if decision.action == "abort" then + sendWarnMessage(decision.reason, "MULTIPLAYER") + return + end + + G.FUNCS.wipe_on() G:delete_run() - -- Load the saved game and start a new run in singleplayer + -- Resume the captured run in-memory -- no disk round trip. G.E_MANAGER:add_event(Event({ trigger = "immediate", no_delete = true, func = function() - local profile = G.SETTINGS.profile - local save_path = profile .. "/save.jkr" - G.SAVED_GAME = get_compressed(save_path) - if G.SAVED_GAME ~= nil then G.SAVED_GAME = STR_UNPACK(G.SAVED_GAME) end + G.SAVED_GAME = decision.savetext G:start_run({ savetext = G.SAVED_GAME }) return true end,