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",