From fe31293566decb2459946dbf48ea892c93cf5750 Mon Sep 17 00:00:00 2001 From: Colson Rice Date: Wed, 26 Aug 2026 14:52:44 -0400 Subject: [PATCH] Refuse a Gen 2 cart save as a Gen 2 save, not as a broken Gen 1 one #1832 reports a Crystal .sav failing to import with "save data checksum invalid (main data checksum mismatch)". The save is fine. It is being measured with the wrong generation's ruler. SaveFileIO.importToSlot sends anything that is not exactly SAVE_SIZE into mainChecksumValid, which is pokered's main-data checksum. Gen 2 carts are MBC3+TIMER, so a real Gold, Silver or Crystal battery save carries an RTC footer: 32786 bytes, never 32768. It misses the size test, falls into the checksum branch, and is told it is damaged by a rule written for a different game. Every real Gen 2 cart save takes that path, every time. The generation check already exists, it just lives too late, inside importSav and behind the size gate that has already rejected the save. This lifts it into SaveConvert.importSupported and exportSupported and asks before the bytes are measured. Both existing guards route through the same predicates now, so the early caller and the late one cannot describe the same game two different ways. Nothing changes for Gen 1, and Gen 2 is still refused, for the real reason and in a sentence a player can do something with. tests/gen2_save_import_message_test.lua covers it. Against the code as it stands it is 20/26, and the failures are the report itself. Co-Authored-By: Claude Opus 5 --- src/import/SaveFileIO.lua | 6 ++ src/save_convert/SaveConvert.lua | 41 ++++++++--- tests/gen2_save_import_message_test.lua | 96 +++++++++++++++++++++++++ tests/run_tests.lua | 1 + 4 files changed, 136 insertions(+), 8 deletions(-) create mode 100644 tests/gen2_save_import_message_test.lua diff --git a/src/import/SaveFileIO.lua b/src/import/SaveFileIO.lua index 0a36fa1a..8723fe0a 100644 --- a/src/import/SaveFileIO.lua +++ b/src/import/SaveFileIO.lua @@ -78,6 +78,12 @@ function SaveFileIO.importToSlot(source, version, force) version = version or GameVersion.get() local bytes, readErr = readSource(source) if not bytes then return false, readErr end + -- The GAME decides before the BYTES do. Everything below this line judges a + -- save by Gen 1's rules -- the size test, and mainChecksumValid, which is + -- pokered's checksum -- so a Gen 2 cart save reaching it is measured against + -- a rule that cannot match and comes back "checksum invalid" (#1832). + local supported, unsupportedWhy = SaveConvert.importSupported(version) + if not supported then return false, unsupportedWhy end if #bytes ~= SAVE_SIZE then local check = SaveConvert.mainChecksumValid(bytes) if check == nil then diff --git a/src/save_convert/SaveConvert.lua b/src/save_convert/SaveConvert.lua index e9d11bd0..449cab82 100644 --- a/src/save_convert/SaveConvert.lua +++ b/src/save_convert/SaveConvert.lua @@ -235,6 +235,35 @@ local function gen2CartName(gameVersion) return GameVersion.info(gameVersion).displayName end +-- Can a cart save for this game cross in or out at all? Public because the +-- launcher has to ask about the GAME before it measures the BYTES. +-- +-- SaveFileIO.importToSlot judges anything that is not exactly SAVE_SIZE with +-- mainChecksumValid, which is pokered's main-data checksum. A Gen 2 cart is +-- MBC3+TIMER, so a real Gold/Silver/Crystal .sav carries an RTC footer and is +-- 32786 bytes: it misses the size test and is then measured against a rule +-- that was never going to match it. The player is told "save data checksum +-- invalid" about a save that is perfectly good (#1832). +-- +-- Returns true, or false plus the same sentence importSav/exportSav would have +-- answered with, so a caller that asks early and a caller that does not cannot +-- describe the same game two different ways. +function SaveConvert.importSupported(gameVersion) + local gen2Name = gen2CartName(gameVersion) + if gen2Name then + return false, gen2Name .. " uses a Gen 2 cart save; importing one is not supported yet." + end + return true +end + +function SaveConvert.exportSupported(gameVersion) + local gen2Name = gen2CartName(gameVersion) + if gen2Name then + return false, gen2Name .. " uses a Gen 2 cart save; exporting one is not supported yet." + end + return true +end + -- importSav(bytes, version, gameVersion) -> saveTable, err -- bytes: the raw 32768-byte SRAM string. Validates size and the main-data -- checksum, decodes through GenSave, and returns a save table fully merged @@ -247,10 +276,8 @@ function SaveConvert.importSav(bytes, version, gameVersion) if type(bytes) ~= "string" then return nil, "expected raw save bytes as a string" end - local gen2Name = gen2CartName(gameVersion) - if gen2Name then - return nil, gen2Name .. " uses a Gen 2 cart save; importing one is not supported yet." - end + local supported, unsupportedWhy = SaveConvert.importSupported(gameVersion) + if not supported then return nil, unsupportedWhy end if #bytes ~= GenSave.SAVE_SIZE then return nil, ("save must be %d bytes, got %d"):format(GenSave.SAVE_SIZE, #bytes) end @@ -283,10 +310,8 @@ function SaveConvert.exportSav(saveTable, gameVersion) if type(saveTable) ~= "table" then return nil, "expected a save table" end - local gen2Name = gen2CartName(gameVersion) - if gen2Name then - return nil, gen2Name .. " uses a Gen 2 cart save; exporting one is not supported yet." - end + local supported, unsupportedWhy = SaveConvert.exportSupported(gameVersion) + if not supported then return nil, unsupportedWhy end local data, derr = ensureData(gameVersion) if not data then return nil, derr end local ok, bytes = pcall(GenSave.encode, saveTable, data, nil) diff --git a/tests/gen2_save_import_message_test.lua b/tests/gen2_save_import_message_test.lua new file mode 100644 index 00000000..652479f2 --- /dev/null +++ b/tests/gen2_save_import_message_test.lua @@ -0,0 +1,96 @@ +-- A Gen 2 cart save must be refused as a Gen 2 cart save, not as a corrupt +-- Gen 1 one (#1832). +-- luajit tests/gen2_save_import_message_test.lua +-- Also dofile'd by tests/run_tests.lua. +-- +-- SaveFileIO.importToSlot judges anything that is not exactly SAVE_SIZE with +-- SaveConvert.mainChecksumValid, which is pokered's main-data checksum. Gen 2 +-- carts are MBC3+TIMER, so a real Gold/Silver/Crystal battery save carries an +-- RTC footer and is 32786 bytes: it misses the size test, is then measured +-- against a checksum rule written for a different generation, and the launcher +-- tells the player their save is corrupt. It is not -- there is simply no Gen +-- 2 codec yet, which is a different sentence and an actionable one. +package.path = "./?.lua;./?/init.lua;" .. package.path + +love = love or require("tests.love_stub") + +local S = require("tests.harness").suite("gen2 save import message") +local check = S.check + +local SaveConvert = require("src.save_convert.SaveConvert") +local SaveFileIO = require("src.import.SaveFileIO") + +-- The size a real Gen 2 cart save actually is: 32768 bytes of SRAM plus the +-- 18-byte RTC footer an MBC3+TIMER cart writes. +local GEN2_CART_SAVE_SIZE = 32786 + +local function blob(n) return string.rep("\0", n) end + +-- readSource only takes a raw string when it is EXACTLY 32768 bytes; anything +-- else is treated as a picker path (its own comment says so). A real Gen 2 +-- cart save is 32786, so it can only ever reach importToSlot as a FILE -- which +-- is exactly how the player in #1832 supplied theirs. Write one and hand over +-- the path, so this exercises the route the report came from. +local function savFile(n) + local path = os.tmpname() + local f = assert(io.open(path, "wb")) + f:write(blob(n)) + f:close() + return path +end + +-- ------------------------------------------------------------------ +-- The report: a real Gen 2 save is not "checksum invalid" +-- ------------------------------------------------------------------ + +for _, version in ipairs({ "gold", "silver", "crystal" }) do + local ok, err = SaveFileIO.importToSlot(savFile(GEN2_CART_SAVE_SIZE), version, true) + check(ok == false, version .. ": a Gen 2 cart save is still refused") + check(type(err) == "string" and err:find("Gen 2 cart save", 1, true) ~= nil, + version .. ": refused AS a Gen 2 save -- got: " .. tostring(err)) + check(type(err) == "string" and err:find("checksum", 1, true) == nil, + version .. ": never blamed on a checksum it was never measured by -- got: " + .. tostring(err)) +end + +-- ------------------------------------------------------------------ +-- The predicate both callers share +-- ------------------------------------------------------------------ + +for _, version in ipairs({ "red", "blue", "yellow" }) do + check(SaveConvert.importSupported(version) == true, + version .. ": Gen 1 import is unaffected") + check(SaveConvert.exportSupported(version) == true, + version .. ": Gen 1 export is unaffected") +end + +for _, version in ipairs({ "gold", "silver", "crystal" }) do + local impOk, impWhy = SaveConvert.importSupported(version) + local expOk, expWhy = SaveConvert.exportSupported(version) + check(impOk == false and expOk == false, version .. ": both directions say no") + -- One sentence per direction, wherever it is asked from: the early gate in + -- SaveFileIO and the late one inside importSav must not describe the same + -- game two different ways. + local _, lateWhy = SaveConvert.importSav(blob(32768), version, version) + check(impWhy == lateWhy, + version .. ": the early gate and importSav answer identically") + check(expWhy:find("exporting", 1, true) ~= nil, + version .. ": the export sentence is about exporting") +end + +-- ------------------------------------------------------------------ +-- Gen 1 keeps its own diagnosis +-- ------------------------------------------------------------------ +-- +-- A Gen 1 save that really is the wrong size AND fails pokered's checksum must +-- still say so: this fix moves the generation check in front of that test, it +-- does not remove it. + +do + local ok, err = SaveFileIO.importToSlot(savFile(GEN2_CART_SAVE_SIZE), "red", true) + check(ok == false, "red: a corrupt oversize save is still refused") + check(type(err) == "string" and err:find("checksum", 1, true) ~= nil, + "red: still diagnosed by pokered's checksum -- got: " .. tostring(err)) +end + +S.finish() diff --git a/tests/run_tests.lua b/tests/run_tests.lua index b778adaf..8a39a350 100644 --- a/tests/run_tests.lua +++ b/tests/run_tests.lua @@ -3615,6 +3615,7 @@ runSuites(orderedGlob( -- name resolution, and the .sav converter refusing a Gen 2 save table. "tests/gen2_sound_alias_test.lua", "tests/gen2_save_convert_cli_test.lua", + "tests/gen2_save_import_message_test.lua", -- The wall radios (`special MapRadio`). gen2_save_export_test cannot share a -- process (LEAKS_SAVE_SLOT_STATE above); tests/run_gen2.lua runs it alone. "tests/gen2_map_radio_test.lua",