mobile updates CLOSES #482, CLOSES #553

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);

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"

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

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()

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