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 <noreply@anthropic.com>
This commit is contained in:
Colson Rice
2026-08-26 14:52:44 -04:00
parent 017eafc51b
commit fe31293566
4 changed files with 136 additions and 8 deletions
+6
View File
@@ -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
+33 -8
View File
@@ -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)
+96
View File
@@ -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()
+1
View File
@@ -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",