diff --git a/src/import/RomImporter.lua b/src/import/RomImporter.lua index 0db7980c..cb3c005e 100644 --- a/src/import/RomImporter.lua +++ b/src/import/RomImporter.lua @@ -5,6 +5,25 @@ local HostShell = require("src.core.HostShell") local RomImporter = {} RomImporter.__index = RomImporter +-- love.system.pickFile is a NATIVE BRIDGE, not part of LÖVE: it exists only on +-- builds that compiled one (Android, and iOS builds patched by +-- mobile/ios/patch_love_src.py). A build without it must fall back to the +-- copy-it-into-the-save-folder flow that every caller below already has -- +-- calling the nil field instead took the whole app down the moment the player +-- pressed Import ROM: +-- +-- src/import/RomImporter.lua: attempt to call field 'pickFile' (a nil value) +-- +-- love.system.createFile was already guarded this way at its one call site; +-- these three were not. Every caller here treats `false` as "no picker +-- available" and shows its own notice, so a missing bridge now degrades to +-- exactly the path a picker-less Android device has always taken. +local function pickFile(...) + local fn = love.system.pickFile + if not fn then return false end + return fn(...) and true or false +end + -- Cache generation tag; bump to force every imported version to re-extract. local CACHE_FORMAT = "rom-cache-v8:" -- The completion marker is written under each version's cache prefix @@ -984,7 +1003,7 @@ function RomImporter:chooseMod() self.modNotice and self.modNotice.ok) return end - if not love.system.pickFile("mod") then + if not pickFile("mod") then self.modNotice = { ok = false, text = "Could not open the file picker. Copy a mod .zip via USB." } else @@ -1047,7 +1066,7 @@ function RomImporter:chooseSaveImport(version) return end self.androidPendingVersion = version - if not love.system.pickFile("sav") then + if not pickFile("sav") then self.androidPendingVersion = nil self.saveNotice[version] = { ok = false, text = "Could not open the file picker. Copy a .sav via USB." } @@ -1135,7 +1154,7 @@ function RomImporter:choose(version) self:startData(data, name) elseif consumePickedRomError(self) then return -- a rejected pick explains itself instead of silently reopening - elseif not love.system.pickFile() then + elseif not pickFile() then -- Picker unavailable (API < 19, or no document-picker app installed): -- fall back to the USB folder-drop path as a friendly notice, not an -- error (which would read as a rejected file). diff --git a/tests/rom_importer_no_picker_test.lua b/tests/rom_importer_no_picker_test.lua new file mode 100644 index 00000000..29dd6ae4 --- /dev/null +++ b/tests/rom_importer_no_picker_test.lua @@ -0,0 +1,97 @@ +-- #482: pressing Import ROM crashed on iOS with +-- +-- src/import/RomImporter.lua: attempt to call field 'pickFile' (a nil value) +-- +-- love.system.pickFile is a native bridge, not part of LÖVE, so it is absent +-- on any mobile build that did not compile one. The mobile path called it +-- unguarded, so a missing bridge took the app down instead of falling back to +-- the copy-into-the-save-folder flow each caller already had for a device with +-- no document picker. +-- +-- Self-contained: `luajit tests/rom_importer_no_picker_test.lua`; also +-- dofile'd by tests/run_tests.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 without a picker") +local eq = S.eq +local check = S.check + +local RomImporter = require("src.import.RomImporter") + +love.system = love.system or {} +local saved = { + getOS = love.system.getOS, + pickFile = love.system.pickFile, + getSaveDirectory = love.filesystem.getSaveDirectory, +} + +love.filesystem.getSaveDirectory = function() return "/tmp/pokemon-love2d" end +love.system.getOS = function() return "iOS" end +-- the condition the crash reports were in: a build with no native bridge +love.system.pickFile = nil + +local function freshImporter() + return setmetatable({ + android = true, -- RomImporter treats iOS as the mobile path + workState = nil, + ready = { red = false, blue = false, yellow = false }, + notice = nil, + modNotice = nil, + saveNotice = {}, + chooseVersion = nil, + startData = function(self, data, displayName) + self._started = { data = data, name = displayName } + end, + _installMod = function(self, name) self._mod = name end, + _importSave = function(self, version, name) self._save = name end, + }, RomImporter) +end + +-- ------- Import ROM + +local ri = freshImporter() +local ok, err = pcall(function() ri:choose("red") end) +check(ok, "Import ROM does not crash without a picker: " .. tostring(err)) +check(ri.notice ~= nil, "and it explains itself instead of doing nothing") +eq(ri.notice.detail, "/tmp/pokemon-love2d", + "pointing at the folder to copy the ROM into") +check(not ri.pickPending, "with no pick left pending on a picker that never opened") + +-- Yellow takes the same path: the crash was never version-specific. +ri = freshImporter() +ok = pcall(function() ri:choose("yellow") end) +check(ok, "Import ROM for Yellow does not crash either") + +-- ------- Import mod .zip + +ri = freshImporter() +ok, err = pcall(function() ri:chooseMod() end) +check(ok, "Import mod does not crash without a picker: " .. tostring(err)) +check(ri.modNotice ~= nil and ri.modNotice.ok == false, + "and reports that the picker could not open") + +-- ------- Import save + +ri = freshImporter() +ok, err = pcall(function() ri:chooseSaveImport("red") end) +check(ok, "Import save does not crash without a picker: " .. tostring(err)) +check(ri.saveNotice.red ~= nil and ri.saveNotice.red.ok == false, + "and reports that the picker could not open") +eq(ri.androidPendingVersion, nil, + "leaving no pending import for a pick that never happened") + +-- ------- and the picker is still used when the bridge IS there + +local pickCalls = 0 +love.system.pickFile = function() pickCalls = pickCalls + 1 return true end +ri = freshImporter() +ri:choose("red") +eq(pickCalls, 1, "a build WITH the bridge still opens the picker") +check(ri.pickPending, "and waits for the pick to come back") + +love.system.getOS = saved.getOS +love.system.pickFile = saved.pickFile +love.filesystem.getSaveDirectory = saved.getSaveDirectory + +S.finish() diff --git a/tests/run_tests.lua b/tests/run_tests.lua index b2809cfe..d75e356d 100644 --- a/tests/run_tests.lua +++ b/tests/run_tests.lua @@ -3337,6 +3337,9 @@ runSuites({ "tests/rom_importer_android_pick_test.lua" }) -- ---------------------------------------------- Android mod / save SAF pick runSuites({ "tests/rom_importer_android_mod_pick_test.lua" }) + +-- ---------------------------------------------- import with no picker (#482) +runSuites({ "tests/rom_importer_no_picker_test.lua" }) -- ---------------------------------------------- parity workstream tests -- Each tests/parity_*.lua is a self-contained file (own bootstrap + check, -- error()s if any assertion fails). Globbed, so dropping a new parity