diff --git a/.gitignore b/.gitignore index ab7ceda5..59022aa0 100644 --- a/.gitignore +++ b/.gitignore @@ -16,6 +16,7 @@ __pycache__/ .* !.github/ !.gitignore +!.luacheckrc # Android build outputs / local SDK path / packaged payload (keep love-android sources) mobile/android/app/build/ diff --git a/.luacheckrc b/.luacheckrc new file mode 100644 index 00000000..3d7f91a1 --- /dev/null +++ b/.luacheckrc @@ -0,0 +1,43 @@ +-- Static-analysis config for `luacheck` (https://luacheck.readthedocs.io). +-- +-- Run it over the engine with: luacheck src (or scripts/lint.sh) +-- +-- The point is a high-signal baseline: the categories left on are the ones +-- that catch real defects -- undefined globals/locals (the class that hid a +-- `music.volume` crash: applyVolume read a `state` that was still the nil +-- global), unused values, unreachable code, redefinitions. The cosmetic +-- categories the codebase deliberately lives with (a `self`/`dt` an +-- interface requires but a given method ignores, documented empty +-- fall-through branches, the odd long line) are muted so they don't drown +-- the signal. + +std = "luajit" + +-- LÖVE exposes `love` as a mutable table: games assign their callbacks onto +-- it (love.wheelmoved, love.run, ...), so it is a regular global, not +-- read-only -- otherwise every callback registration reads as a violation. +globals = { "love" } + +read_globals = { + "jit", + -- LuaJIT 2.1 ships table.unpack even though the bare 5.1 `table` std lacks + -- it; without this, every `table.unpack` reads as an undefined field. + table = { fields = { "unpack" } }, +} + +-- Vendored/native trees and the test suites have their own conventions. +exclude_files = { + "mobile/", + "tests/", + "tools/save-editor/", +} + +ignore = { + "212", -- unused argument -- self/dt kept for a shared method signature + "213", -- unused loop variable -- `for _, v in` where only v is wanted + "421", -- shadowing a local -- deliberate re-use in a few tight scopes + "431", -- shadowing an upvalue + "432", -- shadowing an argument + "542", -- empty if branch -- documented fall-throughs, not gaps + "631", -- line is too long +} diff --git a/README.md b/README.md index a7291e32..f000776b 100644 --- a/README.md +++ b/README.md @@ -108,6 +108,17 @@ supported out of the box. COLORS, TILT, ZOOM, GBC FX, and VOID FILL are also in the Options menu and persist in `options.lua`. +### Low-end devices + +**OPTIONS → PERFORMANCE** scales the port's optional extras for weaker +hardware: **HIGH** (everything on), **BALANCED** (no 3D tilt or GBC FX), +**LOW** (also no survey zoom, FPS capped), or **AUTO** — the default, which +picks a tier from your device (ARM handhelds → LOW, phones → BALANCED, +normal desktops → HIGH, unchanged). It only scales presentation; the +fixed-step game logic is identical on every tier, and a lower tier hides +your tilt/zoom/GBC-FX preferences without forgetting them. Details in +[docs/new-features.md](docs/new-features.md#performance-tier-low-end-devices). + ### Rulesets **OPTIONS → RULESET** picks which set of Gen 1 battle behaviors to run. diff --git a/docs/anbernic-rg34xxsp.md b/docs/anbernic-rg34xxsp.md index 4081f4bc..441090ab 100644 --- a/docs/anbernic-rg34xxsp.md +++ b/docs/anbernic-rg34xxsp.md @@ -65,6 +65,13 @@ display option — COLORS, TILT, ZOOM, VOID FILL, MAX FPS — works normally. If your device turns out to handle the pass, launch with `POKEPORT_GBCFX=1` to put the row back. +**PERFORMANCE defaults to LOW here.** The OPTIONS → PERFORMANCE tier defaults +to AUTO, which reads this device as an ARM Linux handheld and resolves to +**LOW**: the 3D tilt and survey zoom stay off and the frame rate is capped, +so the overworld runs smoothly on the H700 out of the box. Bump it to +BALANCED or HIGH from OPTIONS if you want the extras and your device keeps +up; see [Performance tier](new-features.md#performance-tier-low-end-devices). + The pack bundles the LÖVE 11.5 aarch64 runtime from [PortMaster](https://portmaster.games/), so the device does not need a separate `love_11.5` runtime download on first launch. The launcher resolves diff --git a/docs/new-features.md b/docs/new-features.md index f600d9d8..afc5d3d5 100644 --- a/docs/new-features.md +++ b/docs/new-features.md @@ -143,6 +143,41 @@ effect, `=1` forces it available. The Anbernic handheld pack exports `0` from its launcher because the device reports `"Linux"` while its GPU is in the phone class (see [Anbernic RG34XXSP](anbernic-rg34xxsp.md)). +## Performance tier (low-end devices) + +The Options **PERFORMANCE** row scales the port's optional presentation +extras down for weaker hardware. The extras it governs are the three +heaviest things the port adds on top of the original -- the whole-screen 3D +**TILT** (transforms the entire map as a ground plane), the **GBC FX** +post-process shader (a fullscreen pass), and survey **ZOOM** (zooming out +renders the connected neighbor maps, a lot of extra overdraw) -- plus a hard +FPS ceiling. None of this touches game logic, which is fixed-step off `dt` +(`src/core/FixedStep.lua`), so every tier plays identically; they differ +only in how much eye-candy the renderer is allowed to do. + +| Tier | TILT | GBC FX | Survey ZOOM | Extra FPS ceiling | +| ------------ | ---- | ------ | ----------- | ----------------- | +| **HIGH** | on | on | on | none | +| **BALANCED** | off | off | on | none | +| **LOW** | off | off | off | 60 | +| **AUTO** | picks a default from the device (below) ||| + +- **AUTO** (the default) reads the device once at boot: ARM Linux handhelds + (e.g. the RG34XXSP) resolve to **LOW**, phones/tablets and very-low-core + desktops to **BALANCED**, and everything else -- a normal desktop, and + every existing `options.lua` that predates this option -- to **HIGH**, + so the common case is unchanged. See `src/core/Performance.detect`. +- AUTO only chooses the *default*; all four tiers are selectable, so a + wrong guess is one row away from being overridden. +- The clamps are applied **live** against your stored options and never + rewrite them (`Game:applyOptions`), so a lower tier hides your TILT / GBC + FX / ZOOM without forgetting them -- raising the tier restores exactly + what you had. (This is why the TILT / GBC FX / ZOOM rows still show your + saved choice on a clamped tier: it's your preference, waiting for a tier + that can afford it.) +- Persisted as `save.options.performance` (`auto` | `high` | `balanced` | + `low`); unit-tested in `tests/engine/performance_tiers.lua`. + ## Peer-to-peer link play (lua-enet) Trades and link battles connect two copies of the game directly over 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/scripts/lint.sh b/scripts/lint.sh new file mode 100755 index 00000000..bf2ece34 --- /dev/null +++ b/scripts/lint.sh @@ -0,0 +1,23 @@ +#!/usr/bin/env bash +# Static analysis for the engine Lua (config: .luacheckrc). +# +# Complements scripts/test.sh: the tests prove behavior, luacheck catches the +# defects that never run in a green test -- undefined globals/locals (the +# class that hid a music.volume crash: a hook read a `state` that was still +# the nil global), unused values, unreachable code. The .luacheckrc mutes the +# cosmetic categories the codebase lives with, so what prints is worth a look. +# +# scripts/lint.sh lint src/ +# scripts/lint.sh src tools lint specific paths +# +# Install once with: luarocks install luacheck + +set -uo pipefail +cd "$(dirname "$0")/.." + +if ! command -v luacheck >/dev/null 2>&1; then + echo "luacheck not found on PATH (install: luarocks install luacheck)" >&2 + exit 2 +fi + +luacheck "${@:-src}" diff --git a/src/core/Game.lua b/src/core/Game.lua index 2981060c..c2ab339f 100644 --- a/src/core/Game.lua +++ b/src/core/Game.lua @@ -634,6 +634,23 @@ function Game:applyOptions(opts) -- normalizes a nil/garbage cap to the 60 default, so old saves with no -- fpsCap key pace at the standard rate (issue #88) require("src.core.FrameCap").applyOptions(opts) + -- Scale the optional presentation extras to the device's performance + -- tier. Every heavy feature was just applied from the stored options + -- above; here we clamp the *live* state down for a weaker device without + -- rewriting what the player saved, so raising the tier later restores + -- their exact TILT / GBC FX / ZOOM / MAX FPS choices. A HIGH tier (the + -- default on a normal desktop, and every options.lua predating this + -- option) clamps nothing, so it is a no-op for the common case. + local caps = require("src.core.Performance").applyOptions(opts) + if not caps.tilt then require("src.render.Tilt").setLevel(0) end + if not caps.gbcfx then require("src.render.GBCFX").setLevel(0) end + local Zoom = require("src.render.Zoom") + Zoom.allowSurvey = caps.survey + if not caps.survey and Zoom.offset < 0 then Zoom.offset = 0 end + if caps.fpsMax then + local FrameCap = require("src.core.FrameCap") + if FrameCap.current > caps.fpsMax then FrameCap.apply(caps.fpsMax) end + end Input:applyBindings(opts.bindings) TouchControls:applyOptions(opts) -- heal soft-bricked APK installs that already saved gbcfx > 0 (#136) diff --git a/src/core/Music.lua b/src/core/Music.lua index e218658b..b5d54908 100644 --- a/src/core/Music.lua +++ b/src/core/Music.lua @@ -23,6 +23,13 @@ local volumeScale = 1 local FILTER_HIGHGAIN = { 0.4, 0.16, 0.064 } local filterLevel = 0 +-- Forward-declared here so applyVolume (below) closes over the real playback +-- state rather than a nil global: the table literal is assigned further down, +-- but a `local state = {}` there would leave every reference above it bound +-- to the global `state`. Before this, registering the `music.volume` mod +-- hook crashed applyVolume on `state.current` (a nil index). +local state + local function applyVolume(src) if not src then return end local vol = VOLUME * volumeScale @@ -63,7 +70,7 @@ local function applyFilter(src) end end -local state = { +state = { current = nil, -- song label chip = false, -- the playing song is a synthesized channel program source = nil, -- currently playing source diff --git a/src/core/Performance.lua b/src/core/Performance.lua new file mode 100644 index 00000000..a3d2945a --- /dev/null +++ b/src/core/Performance.lua @@ -0,0 +1,156 @@ +-- Graphics performance tier: one knob that scales the port's optional, +-- non-faithful presentation extras down for weaker hardware. +-- +-- The heavy extras are all things the Game Boy never had and this port +-- adds on top: the whole-screen 3D TILT (transforms the entire map as a +-- ground plane), the GBC FX post-process shader (a fullscreen pass), and +-- survey ZOOM (zooming out renders the connected neighbor maps -- a lot of +-- extra overdraw). A hard FPS ceiling caps present cost on top of that. +-- None of this touches game logic, which is fixed-step off dt +-- (src/core/FixedStep.lua), so every tier plays identically; they differ +-- only in how much optional eye-candy the renderer is allowed to do. +-- +-- The tier is persisted as save.options.performance: +-- auto pick a default from the device (see detect()) +-- high everything on -- the historical behavior +-- balanced no TILT, no GBC FX (kept: survey zoom, colors, uncapped FPS) +-- low no TILT, no GBC FX, no survey zoom, FPS capped +-- +-- AUTO only chooses the *default* for the current device; every tier is +-- selectable in OPTIONS, so a heuristic that guesses wrong is one row away +-- from being overridden. The clamps are applied live in Game:applyOptions +-- against the stored options without rewriting them, so raising the tier +-- restores exactly the TILT / GBC FX / ZOOM / FPS the player had. +-- +-- Zero requires: loads during love.conf and under plain Lua for tools and +-- tests, the same way src/core/GameVersion.lua does. + +local Performance = {} + +-- Option-row order (auto first, then most to least capable). +Performance.TIERS = { "auto", "high", "balanced", "low" } + +Performance.LABELS = { + auto = "AUTO", + high = "HIGH", + balanced = "BALANCED", + low = "LOW", +} + +-- What each concrete tier permits. `auto` is resolved to one of these +-- before caps are read, so it has no row here. fpsMax = false means no +-- extra ceiling (the player's own MAX FPS still applies). +Performance.CAPS = { + high = { tilt = true, gbcfx = true, survey = true, fpsMax = false }, + balanced = { tilt = false, gbcfx = false, survey = true, fpsMax = false }, + low = { tilt = false, gbcfx = false, survey = false, fpsMax = 60 }, +} + +-- Live resolved tier (never "auto"); Game:applyOptions sets it and the +-- renderer / options row read it back. Defaults to the unclamped tier so +-- a pre-boot launcher behaves exactly as it always did. +Performance.tier = "high" + +local function loveOS() + if love and love.system and love.system.getOS then + return love.system.getOS() + end + return nil +end + +local function processorCount() + if love and love.system and love.system.getProcessorCount then + return love.system.getProcessorCount() + end + return nil +end + +-- CPU architecture via LuaJIT's jit.arch when present: "arm"/"arm64" on +-- phones and PortMaster handhelds, "x86"/"x64" on desktops. nil under a +-- plain-Lua tool or test with no jit table, which resolves to HIGH (no +-- clamping) so tooling is never surprised. +local function cpuArch() + return (jit and jit.arch) or nil +end + +-- Default tier for the current device. Deliberately conservative: only +-- the platforms that are reliably weak drop below HIGH, and AUTO is always +-- overridable, so a wrong guess costs one OPTIONS row. A normal +-- multi-core desktop -- the common case, and every existing install whose +-- options.lua predates this option -- resolves to HIGH and behaves exactly +-- as before. +function Performance.detect() + local os = loveOS() + local arch = cpuArch() + local isArm = arch == "arm" or arch == "arm64" + local cores = processorCount() + + -- PortMaster-style ARM Linux handhelds (e.g. the RG34XXSP the project + -- already ships a build for): the weakest target here. + if isArm and os ~= "Android" and os ~= "iOS" then + return "low" + end + -- Phones and tablets: GBC FX is already force-disabled here (issue #136); + -- balanced additionally drops the 3D tilt, the heaviest remaining extra. + if os == "Android" or os == "iOS" then + return "balanced" + end + -- Desktop: a dual-core (or single-core) box is the low-end line. + if cores and cores <= 2 then + return "balanced" + end + return "high" +end + +-- Fold a stored option value to a concrete tier, resolving "auto" (and any +-- hand-edited garbage) through detect(). +function Performance.resolve(value) + if value == "high" or value == "balanced" or value == "low" then + return value + end + return Performance.detect() +end + +-- Normalize a stored value to a valid option id (keeps "auto"); a bad value +-- degrades to auto so a corrupt options.lua still lands on something sane. +function Performance.normalize(value) + for _, id in ipairs(Performance.TIERS) do + if value == id then return id end + end + return "auto" +end + +-- Row label for a stored value: the tier's own name (AUTO / HIGH / ...). +function Performance.label(value) + return Performance.LABELS[Performance.normalize(value)] +end + +-- Concrete caps for a stored option value (resolving auto). +function Performance.caps(value) + return Performance.CAPS[Performance.resolve(value)] +end + +-- Resolve the tier for these options, record it live, and return its caps. +-- Called from Game:applyOptions, which clamps the live presentation modules +-- against the returned caps. +function Performance.applyOptions(opts) + local tier = Performance.resolve(opts and opts.performance) + Performance.tier = tier + return Performance.CAPS[tier] +end + +-- Cycle the OPTIONS row: auto -> high -> balanced -> low -> auto (dir -1 +-- reverses). Lua's `%` is non-negative for a positive modulus, so a +-- negative dir wraps correctly. +function Performance.cycle(value, dir) + value = Performance.normalize(value) + local i = 1 + for idx, id in ipairs(Performance.TIERS) do + if id == value then i = idx break end + end + local n = #Performance.TIERS + local nextIdx = (i - 1 + (dir or 1)) % n + 1 + return Performance.TIERS[nextIdx] +end + +return Performance diff --git a/src/core/SaveData.lua b/src/core/SaveData.lua index 45f752ad..7d537f3c 100644 --- a/src/core/SaveData.lua +++ b/src/core/SaveData.lua @@ -241,6 +241,11 @@ function SaveData.defaultOptions() videoMode = "windowed", -- hard render frame-rate cap; render-only pacing (issue #88, FrameCap.lua) fpsCap = 60, + -- graphics performance tier: auto | high | balanced | low. "auto" + -- picks a default from the device (ARM handhelds/phones drop the heavy + -- extras); scales TILT / GBC FX / survey ZOOM / FPS but never game + -- logic. See src/core/Performance.lua. + performance = "auto", -- Per-pipeline display levels, keyed by render_pipelines id (see -- src/render/Pipelines.lua). A level for a mod that is not installed -- is kept rather than pruned, so re-enabling the mod restores the mode diff --git a/src/import/RomImporter.lua b/src/import/RomImporter.lua index 675aec20..eea9a618 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. -- v9: Yellow audio re-anchored on pokeyellow.sym (#522) -- stale caches -- carry Red's bank $1f header, wave-table, and CryData offsets. @@ -555,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. @@ -986,12 +1011,13 @@ 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 self.pickPending = true self.pickTimer = 0 + self.pickElapsed = 0 end return end @@ -1049,13 +1075,14 @@ 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." } else self.pickPending = true self.pickTimer = 0 + self.pickElapsed = 0 end return end @@ -1094,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 @@ -1137,7 +1165,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). @@ -1149,6 +1177,7 @@ function RomImporter:choose(version) else self.pickPending = true self.pickTimer = 0 + self.pickElapsed = 0 end return end @@ -1183,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 @@ -1200,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 } @@ -1217,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/src/inventory/ItemEffects.lua b/src/inventory/ItemEffects.lua index 3e34ed36..d60e16a3 100644 --- a/src/inventory/ItemEffects.lua +++ b/src/inventory/ItemEffects.lua @@ -10,7 +10,6 @@ -- "ball" caller must throw it (battle only) -- "learn", moveId caller must run the learn-move flow -local Pokemon = require("src.pokemon.Pokemon") local Flags = require("src.script.Flags") local Strings = require("src.core.Strings") diff --git a/src/link/Tournament.lua b/src/link/Tournament.lua index 233cf4e9..cfe663f4 100644 --- a/src/link/Tournament.lua +++ b/src/link/Tournament.lua @@ -384,9 +384,16 @@ function Tournament:update(dt) else self.pendingBattleOpts.seed = msg.seed end - local battle, why = self.isHost - and LinkBattle.newHost(self.game, self.net, self.pendingBattleOpts) - or LinkBattle.newGuest(self.game, self.net, self.pendingBattleOpts) + -- Split rather than `cond and newHost() or newGuest()`: the and/or + -- idiom truncates a call to its first result, so the second return + -- (the specific reason) was always dropped and every failure showed + -- the generic fallback instead of "same mods on both games" etc. + local battle, why + if self.isHost then + battle, why = LinkBattle.newHost(self.game, self.net, self.pendingBattleOpts) + else + battle, why = LinkBattle.newGuest(self.game, self.net, self.pendingBattleOpts) + end if not battle then self:exitWith(why or Strings("Link battle\ncan't start.")) return diff --git a/src/render/Zoom.lua b/src/render/Zoom.lua index 55a5f567..13b27cad 100644 --- a/src/render/Zoom.lua +++ b/src/render/Zoom.lua @@ -10,6 +10,13 @@ local Zoom = {} Zoom.offset = 0 +-- Survey zoom (zooming out past FIT, which renders connected neighbor maps) +-- is the port's most expensive optional extra. The performance tier sets +-- this false on LOW hardware (Game:applyOptions); offsetRange then floors +-- the range at FIT so the option row, hotkey, and mouse wheel all stop at +-- close-up. Nil/true keeps the historical full range. +Zoom.allowSurvey = true + -- legal offset range for a given fit scale (vanilla: survey at 1 px/world -- through 2× fit). zoom.range may widen or shrink the window. function Zoom.offsetRange(S) @@ -21,6 +28,9 @@ function Zoom.offsetRange(S) hi = math.floor(tonumber(hi) or S) if lo > hi then lo, hi = hi, lo end end + -- LOW performance tier: no survey (negative offsets), even if a mod's + -- zoom.range widened it. == false so nil/true stays permissive. + if Zoom.allowSurvey == false and lo < 0 then lo = 0 end return lo, hi end diff --git a/src/ui/OptionsMenu.lua b/src/ui/OptionsMenu.lua index 1d3443ca..847b1017 100644 --- a/src/ui/OptionsMenu.lua +++ b/src/ui/OptionsMenu.lua @@ -20,6 +20,7 @@ local GameSpeed = require("src.core.GameSpeed") local GameVersion = require("src.core.GameVersion") local VideoMode = require("src.core.VideoMode") local FrameCap = require("src.core.FrameCap") +local Performance = require("src.core.Performance") local Logger = require("src.core.Logger") local Runtime = require("src.mods.Runtime") local OptionRows = require("src.ui.OptionRows") @@ -202,6 +203,21 @@ local function buildRows(game) require("src.core.Music").setFilterLevel(o.musicFilter) return true end }, + -- Heads the port's display group: one tier that scales the heavy extras + -- (TILT / GBC FX / survey ZOOM) and the FPS ceiling for weaker devices. + -- AUTO picks a default from the hardware; every tier is overridable. + -- Re-applies live so the extras clamp (or, on a higher tier, restore to + -- the player's stored TILT / GBC FX / ZOOM) the moment the row changes. + { id = "performance", label = Strings("PERFORMANCE"), + value = function(g) + return Strings(Performance.label(g.save.options.performance)) + end, + step = function(g, dir) + local o = g.save.options + o.performance = Performance.cycle(o.performance, dir) + g:applyOptions(o) + return true + end }, { id = "colors", label = Strings("COLORS"), value = function(g) return PaletteFX.modeLabel(g.save.options.colors or "gbc") diff --git a/tests/engine/music_volume_hook_state.lua b/tests/engine/music_volume_hook_state.lua new file mode 100644 index 00000000..2b73e989 --- /dev/null +++ b/tests/engine/music_volume_hook_state.lua @@ -0,0 +1,65 @@ +-- Regression: the `music.volume` mod hook must not crash on Music's private +-- `state`. applyVolume (src/core/Music.lua) builds its hook context from +-- state.current/mapSong/onBike/... but was defined above the `local state` +-- table, so those reads bound to the nil global `state` instead -- a mod that +-- registered music.volume hit "attempt to index a nil value (global 'state')" +-- the first time any volume was applied. This drives a file-backed song +-- through Music with the hook installed and asserts the context resolves. +-- ROM-free: a fake audio source, no data/generated/. +-- luajit tests/engine/music_volume_hook_state.lua + +package.path = "./?.lua;./?/init.lua;" .. package.path + +local T = require("tests.harness") +love = require("tests.love_stub") + +-- minimal audio source: only the methods Music calls on a file song +local Source = {} +Source.__index = Source +function Source:play() self.playing = true end +function Source:stop() self.playing = false end +function Source:pause() self.playing = false end +function Source:isPlaying() return self.playing end +function Source:setLooping(v) self.looping = v end +function Source:setVolume(v) self.volume = v end +function Source:setFilter() end +love.audio = { + newSource = function(file) return setmetatable({ file = file }, Source) end, +} + +local Runtime = require("src.mods.Runtime") +local Music = require("src.core.Music") + +local events = require("src.mods.Events").new() +local hooks = require("src.mods.Hooks").new() +Runtime.install(events, hooks) + +-- one file-backed song is enough; the chip path would pull in `bit` +local data = { audio = { songs = { TEST = { file = "test.ogg" } } } } + +-- record every context the hook is handed, and scale the volume so the +-- return value is exercised too +local calls = {} +hooks:wrap("music.volume", function(_next, vol, ctx) + calls[#calls + 1] = ctx + return vol * 0.5 +end, nil, "voltest") + +T.check(Runtime.wantsHook("music.volume"), "the music.volume hook is registered") + +-- Before the fix this call raised inside applyVolume; reaching the next line +-- at all is the core of the regression. +Music.play(data, "TEST") +T.check(#calls >= 1, "applyVolume ran the hook during play without crashing") + +-- A second application, now that the song is current: state must resolve to +-- the real playback table, so the context carries the live song + scale. +local before = #calls +Music.setVolumeLevel(5) +T.check(#calls > before, "setVolumeLevel re-applies volume through the hook") +local ctx = calls[#calls] +T.check(type(ctx) == "table", "the hook receives a context table") +T.eq(ctx.song, "TEST", "ctx.song is the live song (state resolved, not nil)") +T.eq(ctx.optionScale, 5 / 7, "ctx.optionScale reflects the 0-7 volume level") + +T.finish("music_volume_hook_state") diff --git a/tests/engine/performance_tiers.lua b/tests/engine/performance_tiers.lua new file mode 100644 index 00000000..befdeb2f --- /dev/null +++ b/tests/engine/performance_tiers.lua @@ -0,0 +1,113 @@ +-- Graphics performance tier (src/core/Performance.lua): the AUTO device +-- heuristic, tier normalization/labels, per-tier caps, the option-row +-- cycle, and applyOptions recording the live tier. Pure logic over +-- stubbed love/jit globals, so it needs no ROM and runs in the T2 tier. + +package.path = "./?.lua;./?/init.lua;" .. package.path + +local T = require("tests.harness") +local Performance = require("src.core.Performance") + +-- detect() reads the live `love` and `jit` globals; swap them per scenario +-- and restore afterward so nothing leaks between checks (or into luajit). +local realLove, realJit = love, jit + +local function device(os, arch, cores) + love = { + system = { + getOS = function() return os end, + getProcessorCount = function() return cores end, + }, + } + jit = arch and { arch = arch } or nil +end + +local function restore() + love, jit = realLove, realJit +end + +-- ---------------------------------------------------------------- detect +device("Linux", "arm64", 4) +T.eq(Performance.detect(), "low", "ARM64 Linux handheld -> low") + +device("Linux", "arm", 2) +T.eq(Performance.detect(), "low", "32-bit ARM Linux handheld -> low") + +device("Android", "arm64", 8) +T.eq(Performance.detect(), "balanced", "Android phone -> balanced (not low)") + +device("iOS", "arm64", 6) +T.eq(Performance.detect(), "balanced", "iOS -> balanced") + +device("OS X", "x64", 8) +T.eq(Performance.detect(), "high", "8-core desktop -> high") + +device("Windows", "x64", 2) +T.eq(Performance.detect(), "balanced", "dual-core desktop -> balanced") + +device("Windows", "x86", 1) +T.eq(Performance.detect(), "balanced", "single-core desktop -> balanced") + +-- No love, no jit (a plain-Lua tool/test): nothing looks weak -> high, so +-- tooling that runs applyOptions is never surprised by a clamp. +love, jit = nil, nil +T.eq(Performance.detect(), "high", "no love/jit -> high (no clamping)") +restore() + +-- ---------------------------------------------------------------- resolve +T.eq(Performance.resolve("high"), "high", "resolve passes concrete high") +T.eq(Performance.resolve("balanced"), "balanced", "resolve passes balanced") +T.eq(Performance.resolve("low"), "low", "resolve passes low") + +device("Linux", "arm64", 4) +T.eq(Performance.resolve("auto"), "low", "resolve auto -> detect (handheld)") +T.eq(Performance.resolve(nil), "low", "resolve nil -> detect") +T.eq(Performance.resolve("bogus"), "low", "resolve garbage -> detect") +restore() + +-- --------------------------------------------------------------- normalize +T.eq(Performance.normalize("auto"), "auto", "normalize keeps auto") +T.eq(Performance.normalize("low"), "low", "normalize keeps low") +T.eq(Performance.normalize("xyz"), "auto", "normalize garbage -> auto") +T.eq(Performance.normalize(nil), "auto", "normalize nil -> auto") + +-- ------------------------------------------------------------------- label +T.eq(Performance.label("low"), "LOW", "label low") +T.eq(Performance.label("balanced"), "BALANCED", "label balanced") +T.eq(Performance.label(nil), "AUTO", "label nil -> AUTO") + +-- -------------------------------------------------------------------- caps +local hi, lo = Performance.CAPS.high, Performance.CAPS.low +T.check(hi.tilt and hi.gbcfx and hi.survey and not hi.fpsMax, "high caps: all on, no fps ceiling") +T.check((not lo.tilt) and (not lo.gbcfx) and (not lo.survey), "low caps: heavy extras off") +T.eq(lo.fpsMax, 60, "low caps: FPS ceiling of 60") +local bal = Performance.CAPS.balanced +T.check((not bal.tilt) and (not bal.gbcfx) and bal.survey and not bal.fpsMax, + "balanced caps: no tilt/gbcfx, survey kept, no fps ceiling") + +device("Linux", "arm64", 4) +T.eq(Performance.caps("auto"), Performance.CAPS.low, "caps(auto) resolves through detect") +restore() + +-- ------------------------------------------------------------- applyOptions +local caps = Performance.applyOptions({ performance = "low" }) +T.eq(caps, Performance.CAPS.low, "applyOptions returns the resolved caps") +T.eq(Performance.tier, "low", "applyOptions records the live tier") + +Performance.applyOptions({ performance = "high" }) +T.eq(Performance.tier, "high", "applyOptions high tier") + +device("Linux", "arm64", 4) +Performance.applyOptions(nil) +T.eq(Performance.tier, "low", "applyOptions(nil) resolves auto to the device tier") +restore() + +-- ------------------------------------------------------------------- cycle +T.eq(Performance.cycle("auto", 1), "high", "cycle auto -> high") +T.eq(Performance.cycle("high", 1), "balanced", "cycle high -> balanced") +T.eq(Performance.cycle("balanced", 1), "low", "cycle balanced -> low") +T.eq(Performance.cycle("low", 1), "auto", "cycle low wraps to auto") +T.eq(Performance.cycle("auto", -1), "low", "cycle back auto -> low") +T.eq(Performance.cycle("high", -1), "auto", "cycle back high -> auto") + +T.finish("performance_tiers") diff --git a/tests/mod_ui_tests.lua b/tests/mod_ui_tests.lua index f1aad5fd..acc34e9b 100644 --- a/tests/mod_ui_tests.lua +++ b/tests/mod_ui_tests.lua @@ -282,7 +282,8 @@ local function optGame() end local om = OptionsMenu.new(optGame()) local WANT_IDS = { "textSpeed", "animations", "battleStyle", "battleLayout", - "ruleset", "musicVol", "sfxVol", "musicFilter", "colors", + "ruleset", "musicVol", "sfxVol", "musicFilter", + "performance", "colors", "tilt", "gbcfx", "zoom", "voidFill", "videoMode", "fpsCap", "speed", "mods", "controls" } check(#om.rows == #WANT_IDS, "vanilla options row count (plus MODS/CONTROLS)") @@ -320,35 +321,36 @@ check(om.game.save.options.musicVol == 6, "music volume steps down") for _ = 1, 10 do om.rows[6].step(om.game, -1) end check(om.game.save.options.musicVol == 0, "music volume clamps at 0") --- ZOOM / VOID FILL rows +-- ZOOM / VOID FILL rows (indices shifted +1 by the PERFORMANCE row spliced +-- in ahead of COLORS) local Zoom = require("src.render.Zoom") local TileRenderer = require("src.render.TileRenderer") om.game.save.options.zoom = 0 Zoom.offset = 0 -check(om.rows[12].value(om.game) == "FIT", "ZOOM row shows FIT at offset 0") -om.rows[12].step(om.game, 1) +check(om.rows[13].value(om.game) == "FIT", "ZOOM row shows FIT at offset 0") +om.rows[13].step(om.game, 1) check(om.game.save.options.zoom == 1 and Zoom.offset == 1, "ZOOM row steps to IN1") -om.rows[13].step(om.game, 1) +om.rows[14].step(om.game, 1) check(om.game.save.options.voidFill == "water" and TileRenderer.voidFill == "water", "VOID FILL row cycles TREES → WATER") -om.rows[13].step(om.game, 1) +om.rows[14].step(om.game, 1) check(om.game.save.options.voidFill == "black", "VOID FILL steps to BLACK") -om.rows[13].step(om.game, 1) +om.rows[14].step(om.game, 1) check(om.game.save.options.voidFill == "trees", "VOID FILL wraps to TREES") -- the MAX FPS row cycles the render-cap steps and shows the value plain om.game.save.options.fpsCap = nil -check(om.rows[15].value(om.game) == "60", +check(om.rows[16].value(om.game) == "60", "MAX FPS row defaults to 60 with no saved cap") -om.rows[15].step(om.game, 1) +om.rows[16].step(om.game, 1) check(om.game.save.options.fpsCap == 75, "MAX FPS steps up from 60 to 75") -check(om.rows[15].value(om.game) == "75", "the MAX FPS row renders the cap") +check(om.rows[16].value(om.game) == "75", "the MAX FPS row renders the cap") om.game.save.options.fpsCap = 160 -om.rows[15].step(om.game, 1) +om.rows[16].step(om.game, 1) check(om.game.save.options.fpsCap == 30, "MAX FPS wraps past the ceiling to 30") -om.rows[15].step(om.game, -1) +om.rows[16].step(om.game, -1) check(om.game.save.options.fpsCap == 160, "MAX FPS wraps back down to the ceiling") -- ------- FrameCap normalize / cycle (issue #88) @@ -378,7 +380,7 @@ check(FrameCap.current == 60, "FrameCap.applyOptions defaults a missing key to 6 -- the MODS row is the manager's discoverable home local mgGame = optGame() om = OptionsMenu.new(mgGame) -om.rows[17].activate(mgGame) +om.rows[18].activate(mgGame) check(getmetatable(mgGame.stack:top()) == ManagerState, "the MODS row opens the manager") check(mgGame.stack:top().screenId == "ManagerState", @@ -388,7 +390,7 @@ check(mgGame.stack:top().screenId == "ManagerState", local BindingsMenu = require("src.ui.BindingsMenu") local cbGame = optGame() om = OptionsMenu.new(cbGame) -om.rows[18].activate(cbGame) +om.rows[19].activate(cbGame) local bm = cbGame.stack:top() check(getmetatable(bm) == BindingsMenu, "the CONTROLS row opens the rebind list") 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/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..59e826c3 100644 --- a/tests/run_tests.lua +++ b/tests/run_tests.lua @@ -3337,6 +3337,10 @@ 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" }) +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