mirror of
https://github.com/bryanthaboi/gen1recomp.git
synced 2026-08-12 00:10:56 +02:00
Fix #482: guard love.system.pickFile so Import ROM cannot crash
Pressing Import ROM on iOS takes the whole app down: src/import/RomImporter.lua: attempt to call field 'pickFile' (a nil value) love.system.pickFile is a NATIVE BRIDGE, not part of LOVE. It exists only on builds that compiled one -- Android, and iOS builds patched by mobile/ios/patch_love_src.py -- so on a build without it the field is simply nil. RomImporter:546 routes iOS down the same path as Android (`mobileOS == "Android" or mobileOS == "iOS"`), and all three mobile pick sites called the field unguarded. That is why the reports say "any version": nothing about it is version specific. Red, Blue and Yellow all reach the same call. Every one of those call sites already handles a device with no document picker -- Choose falls back to "No picker available, copy your ROM into:" plus the save directory, and the mod / save rows have their own notices -- and love.system.createFile at its single call site was already guarded this way. These three were not, so the fallback that was written for exactly this case could never be reached. Route them through one small helper that answers false when the bridge is absent. A build without a picker now degrades to the copy-into-the-save-folder flow, which on iOS is a working path: the Files app exposes the app's Documents folder and GRBootstrap sweeps what lands there into the save dir. tests/rom_importer_no_picker_test.lua covers Import ROM, Import mod and Import save with the bridge missing, and asserts the picker is still used when it is present. Reverting the fix reproduces the reported error exactly. Reported in #482 (confirmed by three people) and #512.
This commit is contained in:
@@ -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).
|
||||
|
||||
@@ -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()
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user