From f2a3e5f05bd363c15acb32104d4fa58601e835e0 Mon Sep 17 00:00:00 2001 From: bryanthaboi Date: Fri, 31 Jul 2026 22:39:01 -0400 Subject: [PATCH] mobile updates CLOSES #482, CLOSES #553 --- .../java/org/love2d/android/GameActivity.java | 23 ++++ scripts/build_ios.sh | 55 +++++++- src/import/RomImporter.lua | 51 ++++++-- tests/rom_importer_double_pick_test.lua | 119 ++++++++++++++++++ tests/run_tests.lua | 1 + 5 files changed, 235 insertions(+), 14 deletions(-) create mode 100644 tests/rom_importer_double_pick_test.lua diff --git a/mobile/android/love/src/main/java/org/love2d/android/GameActivity.java b/mobile/android/love/src/main/java/org/love2d/android/GameActivity.java index f8374566..abe2bad9 100644 --- a/mobile/android/love/src/main/java/org/love2d/android/GameActivity.java +++ b/mobile/android/love/src/main/java/org/love2d/android/GameActivity.java @@ -84,7 +84,15 @@ public class GameActivity extends SDLActivity { // instead of leaving the player on "No ROM imported" (issue #442). private static final String PICK_ERROR_FILENAME = "pick_error.flag"; // Destination basename for the in-flight SAF pick (set by showFilePicker). + // Saved/restored across instance state: the picker is a separate activity + // and Android may destroy this one while it is up (memory pressure, or + // "Don't keep activities"). A recreated instance still receives + // onActivityResult, so without this a mod or save pick came back with the + // field reset and was filed as picked_rom.gb, which Lua then rejected as a + // bad ROM instead of installing it (#553). private String pendingPickFilename = PICKED_ROM_FILENAME; + private static final String STATE_PENDING_PICK = "pendingPickFilename"; + private static final String STATE_PENDING_CREATE = "pendingCreateSuggestedName"; // Suggested download name for the in-flight SAF create (set by showCreateDocument). private String pendingCreateSuggestedName = "export.sav"; private static boolean immersiveActive = false; @@ -149,6 +157,14 @@ public class GameActivity extends SDLActivity { } super.onCreate(savedInstanceState); + if (savedInstanceState != null) { + // Restore the in-flight SAF destinations, so a pick that returns to + // a recreated activity still lands under the basename it asked for. + String pick = savedInstanceState.getString(STATE_PENDING_PICK); + if (pick != null) pendingPickFilename = pick; + String create = savedInstanceState.getString(STATE_PENDING_CREATE); + if (create != null) pendingCreateSuggestedName = create; + } metrics = getResources().getDisplayMetrics(); // Set low-latency audio values @@ -539,6 +555,13 @@ public class GameActivity extends SDLActivity { } } + @Override + protected void onSaveInstanceState(Bundle outState) { + super.onSaveInstanceState(outState); + outState.putString(STATE_PENDING_PICK, pendingPickFilename); + outState.putString(STATE_PENDING_CREATE, pendingCreateSuggestedName); + } + @Override protected void onActivityResult(int requestCode, int resultCode, Intent data) { super.onActivityResult(requestCode, resultCode, data); diff --git a/scripts/build_ios.sh b/scripts/build_ios.sh index 7ca1dc8d..30d3d08f 100755 --- a/scripts/build_ios.sh +++ b/scripts/build_ios.sh @@ -243,6 +243,7 @@ pack_game_love() { (cd "$ROOT" && zip -q -9 -r "$LOVE_FILE" \ main.lua conf.lua src data assets tools/save-editor \ tools/rom_manifest.json tools/rom_manifest_blue.json \ + tools/rom_manifest_yellow.json \ -x '*.DS_Store' -x '*/.git/*' -x '*/.DS_Store' \ -x 'data/generated/*' -x 'assets/generated/*') # NOTE: grep -q here would race pipefail — it exits on first match, unzip @@ -252,8 +253,19 @@ pack_game_love() { | grep -E '^(data|assets)/generated/[^/]+|^(data|assets)/generated/.+/' >/dev/null; then fail "game.love unexpectedly contains generated ROM data" fi - unzip -Z1 "$LOVE_FILE" | grep -x 'tools/save-editor/App.lua' >/dev/null \ - || fail "game.love is missing the save editor (Edit on a save row would crash)" + # Same required-file gate as scripts/build.sh and scripts/build_android.sh. + # iOS only checked App.lua, which is why the Yellow manifest shipped missing + # in 0.1.45 through 0.1.47: decodeManifest (src/import/RomImporter.lua) errors + # outright when a version's manifest is absent, so Import ROM on Yellow died + # in the built app while dev, which reads the source tree, stayed green. + archive_entries="$(unzip -Z1 "$LOVE_FILE")" + for required in tools/save-editor/App.lua tools/save-editor/Kit.lua \ + tools/save-editor/panels/Party.lua \ + tools/rom_manifest.json tools/rom_manifest_blue.json \ + tools/rom_manifest_yellow.json; do + printf '%s\n' "$archive_entries" | grep -qx "$required" \ + || fail "game.love is missing $required" + done say "game.love: $(du -h "$LOVE_FILE" | cut -f1) -> $LOVE_FILE" } @@ -348,6 +360,43 @@ PY } # --------------------------------------------------------------- xcodebuild +# love.system.pickFile and createFile are a native bridge compiled in by +# mobile/ios/patch_love_src.py, not part of LÖVE. A build that skipped the +# patch still links and still runs, then finds the field nil the moment anyone +# taps Import ROM (#482). #539 made that degrade to the copy-into-Files flow +# rather than crash, which is the right floor, but a build with no picker at all +# is a silent downgrade, so fail here instead of shipping one. +# +# Checked against the built binary rather than the source, because patching +# love-src proves nothing about what Xcode actually compiled: the shipped +# 0.1.45/0.1.46/0.1.47 IPAs all DO carry the bridge, so the reports that blamed +# a missing patch step were self-built IPAs, exactly the case this catches. +verify_native_bridge() { + local app="$1" + local exe bin missing="" + exe="$(/usr/libexec/PlistBuddy -c 'Print :CFBundleExecutable' \ + "$app/Info.plist" 2>/dev/null || true)" + bin="$app/${exe:-love}" + [ -f "$bin" ] || bin="$app/love" + if [ ! -f "$bin" ]; then + warn "no executable inside $(basename "$app"); skipping native bridge check" + return 0 + fi + # grep -q here would race pipefail the same way pack_game_love documents: + # it exits on first match, strings dies of SIGPIPE, the pipeline "fails" + # nondeterministically. >/dev/null keeps grep reading the whole stream. + for sym in pickFile createFile; do + strings -a "$bin" | grep -x "$sym" >/dev/null || missing="$missing $sym" + done + if [ -n "$missing" ]; then + fail "built app has no native bridge (missing:$missing). + Import ROM would fall back to copy-into-Files instead of opening the picker. + mobile/ios/patch_love_src.py did not take. Re-run: + scripts/build_ios.sh --fetch && scripts/build_ios.sh" + fi + say "native bridge present (pickFile, createFile)" +} + run_xcodebuild() { local config sdk destination if $RELEASE; then @@ -455,6 +504,8 @@ run_xcodebuild() { cp "$LOVE_FILE" "$app/game.love" fi + verify_native_bridge "$app" + local dist_dir="$DIST/${config}-${sdk}" rm -rf "$dist_dir" mkdir -p "$dist_dir" diff --git a/src/import/RomImporter.lua b/src/import/RomImporter.lua index fe0abe57..eea9a618 100644 --- a/src/import/RomImporter.lua +++ b/src/import/RomImporter.lua @@ -574,10 +574,16 @@ function RomImporter.new(onComplete, opts) onEditTouchControls = opts.onEditTouchControls, android = android, ios = mobileOS == "iOS", - -- One startup poll pass: files dropped through the Files app are swept - -- into the save dir before Lua boots (GRBootstrap), but no love.focus - -- event necessarily follows, so consume them via the first poll tick. - pickPending = mobileOS == "iOS" or nil, + -- One startup poll pass on both mobiles. iOS: files dropped through the + -- Files app are swept into the save dir before Lua boots (GRBootstrap) with + -- no love.focus event necessarily following. Android: the SAF picker is a + -- separate activity, and Android is free to destroy GameActivity while it + -- is up (memory pressure, or "Don't keep activities"), so the app RESTARTS + -- instead of resuming and the love.focus(true) that would have consumed the + -- pick never arrives. The file is sitting in the save dir either way, so + -- boot armed and let the first poll tick consume it, rather than making the + -- player tap Import a second time to trigger the scan by hand (#553). + pickPending = android or nil, -- Android drag: the launcher is handed no move events at all (main.lua -- forwards neither touchmoved nor mousemoved while it is up), and its mouse -- emulation is what "no reliable pointer polling" below refers to. @@ -1011,6 +1017,7 @@ function RomImporter:chooseMod() else self.pickPending = true self.pickTimer = 0 + self.pickElapsed = 0 end return end @@ -1075,6 +1082,7 @@ function RomImporter:chooseSaveImport(version) else self.pickPending = true self.pickTimer = 0 + self.pickElapsed = 0 end return end @@ -1113,6 +1121,7 @@ function RomImporter:exportSave(version) if love.system.createFile and love.system.createFile(suggested) then self.pickPending = true self.pickTimer = 0 + self.pickElapsed = 0 self.saveNotice[version] = { ok = true, text = "Pick where to save " .. suggested .. "..." } else @@ -1168,6 +1177,7 @@ function RomImporter:choose(version) else self.pickPending = true self.pickTimer = 0 + self.pickElapsed = 0 end return end @@ -1202,14 +1212,31 @@ function RomImporter:choose(version) end end --- iOS: the document picker is an in-process modal sheet, so unlike Android's --- separate SAF activity there is no love.focus(true) when it dismisses. --- While a pick is outstanding, poll the save dir for the bridge's delivered --- file (picked_rom.gb / picked_mod.zip / picked_save.sav / export_done.flag) --- and run the same refocus import path Android uses. +-- Poll the save dir for a delivered pick (picked_rom.gb / picked_mod.zip / +-- picked_save.sav / export_done.flag) and run the same import path a refocus +-- runs. Both mobiles need this, for different reasons: +-- +-- iOS the document picker is an in-process modal sheet, so there is no +-- love.focus(true) when it dismisses -- nothing else would consume it. +-- Android the SAF picker IS a separate activity and normally does refocus, +-- but Android may destroy GameActivity while it is up, in which case +-- the app restarts and that focus event never comes. Polling makes +-- the outcome the same either way instead of leaving the pick on disk +-- for the next tap to find, which is what made users import twice and +-- what made it look random: it depends on memory pressure (#553). +-- +-- Disarms after PICK_TIMEOUT so a cancelled picker (which delivers nothing, +-- ever) does not leave this scanning the save directory for the whole session. +local PICK_TIMEOUT = 120 + function RomImporter:_pollPickedFiles(dt) - if not (self.ios and self.pickPending) then return end + if not self.pickPending then return end if self.workState == "working" then return end + self.pickElapsed = (self.pickElapsed or 0) + dt + if self.pickElapsed > PICK_TIMEOUT then + self.pickPending, self.pickElapsed = nil, nil + return + end self.pickTimer = (self.pickTimer or 0) + dt if self.pickTimer < 0.5 then return end self.pickTimer = 0 @@ -1219,7 +1246,7 @@ function RomImporter:_pollPickedFiles(dt) local pickError = love.filesystem.read("pick_error.txt") if pickError then love.filesystem.remove("pick_error.txt") - self.pickPending = nil + self.pickPending, self.pickElapsed = nil, nil self.modNotice = { ok = false, text = pickError } self.notice = { version = self.chooseVersion or "red", status = "File import failed:", detail = pickError } @@ -1236,7 +1263,7 @@ function RomImporter:_pollPickedFiles(dt) end end if found then - self.pickPending = nil + self.pickPending, self.pickElapsed = nil, nil self:focus(true) end end diff --git a/tests/rom_importer_double_pick_test.lua b/tests/rom_importer_double_pick_test.lua new file mode 100644 index 00000000..56f7e3a2 --- /dev/null +++ b/tests/rom_importer_double_pick_test.lua @@ -0,0 +1,119 @@ +-- #553: "App makes user import both game and/or mods twice before installing." +-- +-- Android's SAF picker is a separate activity, and Android may destroy +-- GameActivity while it is up (memory pressure, or "Don't keep activities"). +-- When that happens the app RESTARTS rather than resuming, so the +-- love.focus(true) that RomImporter:focus consumes a pick on never arrives. +-- GameActivity has already written picked_rom.gb / picked_mod.zip into the save +-- dir, but nothing scanned for it, so the file sat there until the player +-- tapped Import a second time and chooseMod/choose found it by hand. Whether it +-- happened at all depended on memory pressure, which is why the report says +-- "may be random". +-- +-- _pollPickedFiles was the fix that already existed, gated to iOS. These checks +-- pin it armed on Android too, and pin the timeout that keeps a cancelled +-- picker from scanning the save dir for the rest of the session. +-- +-- Self-contained: `luajit tests/rom_importer_double_pick_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 double pick (#553)") +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, + getDirectoryItems = love.filesystem.getDirectoryItems, + getInfo = love.filesystem.getInfo, + read = love.filesystem.read, + remove = love.filesystem.remove, +} + +-- A fake save dir we can drop a delivered pick into. +local saveDir = {} +love.filesystem.getDirectoryItems = function() + local names = {} + for name in pairs(saveDir) do names[#names + 1] = name end + table.sort(names) + return names +end +love.filesystem.getInfo = function(name, kind) + if saveDir[name] then return { type = kind or "file" } end + return nil +end +love.filesystem.read = function(name) return saveDir[name] end +love.filesystem.remove = function(name) saveDir[name] = nil; return true end + +local function importer(os) + love.system.getOS = function() return os end + local ri = RomImporter.new(function() end, { launcher = true }) + ri.ready = { red = false, blue = false, yellow = false } + return ri +end + +-- 1. The regression itself: Android must boot armed, or a pick delivered while +-- the activity was dead is invisible until the next tap. +local android = importer("Android") +check(android.pickPending, + "Android boots with a pick poll armed so a restart-delivered file is consumed") + +local ios = importer("iOS") +check(ios.pickPending, "iOS still boots armed (unchanged by this fix)") + +love.system.getOS = function() return "OS X" end +local desktop = RomImporter.new(function() end, { launcher = true }) +check(not desktop.pickPending, "desktop does not poll: it has no save-dir picks") + +-- 2. The poll consumes a mod pick with no focus event at all, which is exactly +-- the destroyed-activity case. Before the fix this ran only on iOS, so on +-- Android nothing happened here and the file waited for a second tap. +local ri = importer("Android") +local consumed = false +ri.focus = function(self, f) if f then consumed = true end end +saveDir["picked_mod.zip"] = "PK\003\004 pretend mod" +ri:_pollPickedFiles(0.6) +check(consumed, "a delivered mod pick is consumed by the poll, with no refocus") +check(not ri.pickPending, "and the poll disarms once it has fired") + +-- 3. A ROM pick goes the same way. +local ri2 = importer("Android") +local consumed2 = false +ri2.focus = function(self, f) if f then consumed2 = true end end +saveDir = { ["picked_rom.gb"] = "not a real cart" } +ri2:_pollPickedFiles(0.6) +check(consumed2, "a delivered ROM pick is consumed by the poll too") + +-- 4. Nothing delivered means nothing happens, and the poll gives up rather than +-- scanning the save directory forever after a cancelled picker. +saveDir = {} +local ri3 = importer("Android") +local fired = false +ri3.focus = function(self, f) if f then fired = true end end +ri3:_pollPickedFiles(0.6) +check(not fired, "an empty save dir consumes nothing") +check(ri3.pickPending, "and stays armed while it is still within the timeout") +ri3:_pollPickedFiles(200) +check(not ri3.pickPending, "a cancelled pick disarms instead of polling forever") + +-- 5. A pick that is still importing must not be double-started. +saveDir = { ["picked_mod.zip"] = "PK\003\004" } +local ri4 = importer("Android") +local fired4 = false +ri4.focus = function(self, f) if f then fired4 = true end end +ri4.workState = "working" +ri4:_pollPickedFiles(0.6) +check(not fired4, "the poll stands down while an import is already running") + +love.system.getOS = saved.getOS +love.system.pickFile = saved.pickFile +love.filesystem.getDirectoryItems = saved.getDirectoryItems +love.filesystem.getInfo = saved.getInfo +love.filesystem.read = saved.read +love.filesystem.remove = saved.remove + +S.finish() diff --git a/tests/run_tests.lua b/tests/run_tests.lua index d75e356d..59e826c3 100644 --- a/tests/run_tests.lua +++ b/tests/run_tests.lua @@ -3340,6 +3340,7 @@ 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" }) -- ---------------------------------------------- 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