From 921f565f1db04c05a9ec24242c60ffb97c086ab6 Mon Sep 17 00:00:00 2001 From: Colson Rice Date: Fri, 21 Aug 2026 22:21:41 -0400 Subject: [PATCH] Import the cart Choose ROM asked for findPendingRom answered with the first dump in the save directory whose SHA-1 mapped to any not-yet-ready version. On a device with no file picker that scan IS the import, so with four dumps in the folder, choosing Red imported and decoded Blue (#1274). It now takes an optional version and narrows to it. The two Choose paths pass self.chooseVersion, since a Choose names the cart it is for. The two Android USB-drop scans pass nothing and still take the first pending cart of any version, which is what they are for. A chosen version with no dump present now imports nothing rather than the wrong cart, and falls through to the notice that says where to put the file. --- src/import/RomImporter.lua | 21 +++- tests/rom_importer_choose_version_test.lua | 126 +++++++++++++++++++++ tests/run_tests.lua | 2 + 3 files changed, 143 insertions(+), 6 deletions(-) create mode 100644 tests/rom_importer_choose_version_test.lua diff --git a/src/import/RomImporter.lua b/src/import/RomImporter.lua index 2c303fac..ca051683 100644 --- a/src/import/RomImporter.lua +++ b/src/import/RomImporter.lua @@ -1086,13 +1086,19 @@ end -- naive "first ROM wins" scan would re-import Red when the player tries to -- add Blue (issue #167). Yellow and Gold carts are typically .gbc (Gold is -- 2 MiB). -local function findPendingRom(ready) +-- `wanted` narrows the scan to one version. A Choose names the cart it is +-- for, so without it several dumps sitting in the save directory answered in +-- listing order and the selection was dropped: picking Red imported Blue +-- (#1274). The callers that are not a Choose, the Android USB-drop scans, +-- pass nothing and still take the first pending cart of any version. +local function findPendingRom(ready, wanted) for _, name in ipairs(love.filesystem.getDirectoryItems("")) do if name:lower():match("%.gbc?$") and love.filesystem.getInfo(name, "file") then local data = love.filesystem.read(name) if type(data) == "string" and isAcceptedRomSize(#data) then local version = GameVersion.forSha1(sha1(data)) - if version and not ready[version] then + if version and not ready[version] + and (wanted == nil or version == wanted) then return name, data end end @@ -2575,8 +2581,9 @@ function RomImporter:choose(version) if self.android then -- Prefer a not-yet-imported .gb/.gbc already in the save dir (USB copy, or -- a fresh SAF pick). Never reuse an already-imported cart's file -- that - -- was the #167 failure mode (second Choose just re-extracted Red). - local name, data = findPendingRom(self.ready) + -- was the #167 failure mode (second Choose just re-extracted Red). This + -- is a Choose, so it takes the cart that was asked for and nothing else. + local name, data = findPendingRom(self.ready, self.chooseVersion) if name then self:startData(data, name) elseif consumePickedRomError(self) then @@ -2604,8 +2611,10 @@ function RomImporter:choose(version) -- Handheld Linux (Anbernic stock OS / PortMaster) rarely has zenity or -- kdialog. Fall back to the same "drop a .gb/.gbc next to the game" scan -- used on Android, which works when the game is launched as an unpacked - -- directory (see build-rg34xxsp.sh). - local name, data = findPendingRom(self.ready) + -- directory (see build-rg34xxsp.sh). Narrowed to the chosen version: with + -- four dumps in the folder the unnarrowed scan answered in listing order, + -- so Choose Red imported and decoded Blue (#1274). + local name, data = findPendingRom(self.ready, self.chooseVersion) if name then self:startData(data, name) return diff --git a/tests/rom_importer_choose_version_test.lua b/tests/rom_importer_choose_version_test.lua new file mode 100644 index 00000000..35b5bacc --- /dev/null +++ b/tests/rom_importer_choose_version_test.lua @@ -0,0 +1,126 @@ +-- #1274: with several cartridge dumps sitting in the save directory, Choose +-- ROM imported whichever version was not yet ready rather than the one the +-- player picked. Select Red, get Blue. +-- +-- The fallback exists for devices with no file picker (handheld Linux with +-- no zenity or kdialog, and the Android USB-drop path), where chooseRom +-- returns nil and the importer scans the save directory instead. That scan, +-- findPendingRom, answered with the first dump whose SHA-1 mapped to any +-- not-yet-ready version, so the selection was dropped on the floor. +-- +-- Self-contained: `luajit tests/rom_importer_choose_version_test.lua` +package.path = "./?.lua;./?/init.lua;" .. package.path +if not _G.love then _G.love = require("tests.love_stub") end + +local S = require("tests.harness").suite("rom importer honours the chosen version") +local eq = S.eq +local check = S.check + +local RomImporter = require("src.import.RomImporter") +local GameVersion = require("src.core.GameVersion") + +local ROM_BYTES = 1024 * 1024 + +-- One dump per version, each a blob of the right size whose first byte says +-- which cart it is. love.data.hash is stubbed to answer with the real SHA-1 +-- from GameVersion, so GameVersion.forSha1 does its own lookup unchanged. +local MARK = { R = "red", B = "blue", Y = "yellow" } +local FILES = { + ["blue.gb"] = string.rep("B", ROM_BYTES), + ["red.gb"] = string.rep("R", ROM_BYTES), + ["yellow.gbc"] = string.rep("Y", ROM_BYTES), +} +-- Deliberately not alphabetical and not selection order: the bug returned +-- whatever getDirectoryItems listed first. +local LISTING = { "blue.gb", "red.gb", "yellow.gbc" } + +love.system = love.system or {} +love.data = love.data or {} + +local saved = { + getOS = love.system.getOS, + hash = love.data.hash, + encode = love.data.encode, + getDirectoryItems = love.filesystem.getDirectoryItems, + getInfo = love.filesystem.getInfo, + read = love.filesystem.read, + getSaveDirectory = love.filesystem.getSaveDirectory, +} + +-- Not OS X, Windows or Linux, so chooseRom returns nil without shelling out +-- to a dialog that may or may not exist on the machine running the suite. +-- The fallback under test is the same one every pickerless device takes. +love.system.getOS = function() return "Unknown" end +love.filesystem.getSaveDirectory = function() return "/tmp/pokemon-love2d" end +love.filesystem.getDirectoryItems = function() return LISTING end +love.filesystem.getInfo = function(name, filter) + if FILES[name] then return { type = "file", size = #FILES[name] } end + return nil +end +love.filesystem.read = function(name) return FILES[name] end +-- RomImporter's sha1 helper is hash then encode-to-hex, so the pair is +-- stubbed together: hash answers with the digest already in the form +-- GameVersion stores, and encode hands it back untouched. +love.data.hash = function(_, data) + local version = MARK[data:sub(1, 1)] + return version and GameVersion.info(version).sha1 or "0" +end +love.data.encode = function(_, _, digest) return digest end + +local function freshImporter() + return setmetatable({ + android = false, + nativePicker = false, + workState = nil, + ready = { red = false, blue = false, yellow = false }, + notice = nil, + modNotice = nil, + saveNotice = {}, + baseRoms = {}, + chooseVersion = nil, + startData = function(self, data, displayName) + self._started = { data = data, name = displayName } + end, + startPath = function(self, path) self._startedPath = path end, + }, RomImporter) +end + +-- ------- the report: pick Red, get Red + +local ri = freshImporter() +ri:choose("red") +check(ri._started ~= nil, "a pickerless Choose still imports from the save dir") +eq(ri._started and ri._started.name, "red.gb", + "and it imports the cart that was chosen, not the first pending one") + +-- ------- the same for a version listed after another pending dump + +ri = freshImporter() +ri:choose("yellow") +eq(ri._started and ri._started.name, "yellow.gbc", + "Yellow is imported for Yellow, with Blue and Red still pending") + +-- ------- a chosen version with no dump present imports nothing + +ri = freshImporter() +ri.ready = { red = false, blue = false, yellow = false, gold = false } +ri:choose("gold") +check(ri._started == nil, + "choosing a version whose dump is absent imports no other cart") + +-- ------- an already-imported cart is still never re-extracted (#167) + +ri = freshImporter() +ri.ready = { red = true, blue = false, yellow = false } +ri:choose("red") +check(ri._started == nil, + "and a version already imported is not extracted a second time") + +for name, fn in pairs(saved) do + if name == "getOS" then love.system.getOS = fn + elseif name == "hash" then love.data.hash = fn + elseif name == "encode" then love.data.encode = fn + else love.filesystem[name] = fn end +end + +S.finish() diff --git a/tests/run_tests.lua b/tests/run_tests.lua index 4ba9ccff..3b32b255 100644 --- a/tests/run_tests.lua +++ b/tests/run_tests.lua @@ -3623,6 +3623,8 @@ runSuites({ "tests/rom_importer_android_mod_pick_test.lua" }) -- ---------------------------------------------- import with no picker (#482) runSuites({ "tests/rom_importer_no_picker_test.lua" }) runSuites({ "tests/rom_importer_double_pick_test.lua" }) +-- the same pickerless scan, asked for one version in particular (#1274) +runSuites({ "tests/rom_importer_choose_version_test.lua" }) -- ---------------------------------------------- Switch platform capabilities -- platform_nx_* / rom_importer_nx_* live in tests/engine/ (ROM-free T2) so -- CI's headless lane runs them without data/generated/.