From f0f3e9634bdceacb155140de794da26c8395fa7a Mon Sep 17 00:00:00 2001 From: techmore Date: Sat, 1 Aug 2026 11:01:19 -0400 Subject: [PATCH 1/3] fix: restore PROF. OAK's PC intro, jingle timing and closing link CLOSES #576 The launcher skipped oaks_pc.asm's whole session -- the access text and the 'Want to get your #DEX rated?' YES/NO -- and played the Pokedex_Rating jingle the moment the entry was picked, before any text printed. Now the access text types out, the YES/NO pops, and only once the completion line and the rating tier have printed does the jingle sound (DisplayDexRating -> PlayPokedexRatingSfx, auto.wait hands the box to the A/B path), then the 'Closed link to PROF.OAK's PC.' tail closes the session. --- src/world/OverworldController.lua | 45 ++++++++- tests/engine/oaks_pc_flow.lua | 163 ++++++++++++++++++++++++++++++ 2 files changed, 204 insertions(+), 4 deletions(-) create mode 100644 tests/engine/oaks_pc_flow.lua diff --git a/src/world/OverworldController.lua b/src/world/OverworldController.lua index 3d282e17..394b9507 100644 --- a/src/world/OverworldController.lua +++ b/src/world/OverworldController.lua @@ -2576,8 +2576,7 @@ function OverworldState:openPC(onDone) table.insert(items, { label = Strings("PROF.OAK's PC"), onSelect = function() - self:dexRating() - done() + self:openOaksPC(done) end, }) end @@ -2604,11 +2603,41 @@ function OverworldState:openPC(onDone) noSound = true })) end +-- The PROF. OAK's PC session (engine/menus/oaks_pc.asm OpenOaksPC): the +-- access text, "Want to get your #DEX rated?" with a YES/NO, then the +-- rating, and "Closed link to PROF.OAK's PC." before control returns -- the +-- intro and closing links the launcher skipped, jingle ordering aside (#576). +function OverworldState:openOaksPC(onDone) + local done = onDone or function() end + local text = Game.data.text or {} + local accessed = text._AccessedOaksPCText + or Strings("Accessed PROF.\nOAK's PC.\fAccessed POKéDEX\nRating System.") + local rated = text._GetDexRatedText + or Strings("Want to get your\nPOKéDEX rated?") + local closed = text._ClosedOaksPCText + or Strings("Closed link to\nPROF.OAK's PC.") + local function close() + Game.stack:push(TextBox.new(Game, closed, done)) + end + Game.stack:push(TextBox.new(Game, accessed, function() + -- _GetDexRatedText ends with `done`, so the YES/NO pops as soon as the + -- text has typed out, with no button wait in between (YesNoChoice) + Game.stack:push(TextBox.new(Game, rated, nil, { + choice = function(yes) + if not yes then + close() + return + end + self:dexRating(close) + end, + })) + end)) +end + -- Prof. Oak's dex rating service (engine/events/pokedex_rating.asm): -- the completion line with seen AND owned counts, then the per-decade -- rating text. function OverworldState:dexRating(onDone) - require("src.core.Sound").play(Game.data, "Pokedex_Rating") local seen, owned = 0, 0 for _ in pairs(Game.save.pokedex.seen or {}) do seen = seen + 1 end for _ in pairs(Game.save.pokedex.owned or {}) do owned = owned + 1 end @@ -2625,7 +2654,15 @@ function OverworldState:dexRating(onDone) completion = completion :gsub("{NUM:hDexRatingNumMonsSeen[^}]*}", tostring(seen)) :gsub("{NUM:hDexRatingNumMonsOwned[^}]*}", tostring(owned)) - Game.stack:push(TextBox.new(Game, completion .. "\f" .. rating, onDone)) + -- DisplayDexRating prints the completion line, then the tier text, and + -- only then plays the rating jingle and waits for a button -- the fanfare + -- must not pre-empt the evaluation it celebrates (#576). auto.wait hands + -- the box to the plain A/B path once the jingle has sounded. + Game.stack:push(TextBox.new(Game, completion .. "\f" .. rating, onDone, { + auto = { wait = true, sound = function() + return require("src.core.Sound").play(Game.data, "Pokedex_Rating") + end }, + })) end -- AnimateHealingMachine (engine/overworld/healing_machine.asm): balls diff --git a/tests/engine/oaks_pc_flow.lua b/tests/engine/oaks_pc_flow.lua new file mode 100644 index 00000000..ec5864d5 --- /dev/null +++ b/tests/engine/oaks_pc_flow.lua @@ -0,0 +1,163 @@ +-- Prof. Oak's PC session (#576): engine/menus/oaks_pc.asm OpenOaksPC -- +-- the access text, "Want to get your #DEX rated?" with a YES/NO, the dex +-- rating (completion line + tier text), the rating jingle only once the +-- rating text has printed (DisplayDexRating -> PlayPokedexRatingSfx), and +-- the "Closed link to PROF.OAK's PC." tail before control returns. The +-- old flow played the jingle the moment the entry was picked and skipped +-- both the intro and the closing link. +-- ROM-free: uses the fixture dataset so CI (no data/generated/) stays green. +package.path = "./?.lua;./?/init.lua;" .. package.path + +local T = require("tests.modkit") +local Data = T.fixtures.load() + +-- the ROM-extracted strings the fixture text table does not carry; labels +-- and wording match pokered text/pokedex_ratings.asm + oaks_pc.asm +Data.text._AccessedOaksPCText = + "Accessed PROF.\nOAK's PC.\fAccessed #DEX\nRating System." +Data.text._GetDexRatedText = "Want to get your\n#DEX rated?" +Data.text._ClosedOaksPCText = "Closed link to\nPROF.OAK's PC." +Data.text._DexCompletionText = + "#DEX comp-\nletion is:\f{NUM:hDexRatingNumMonsSeen} #MON seen\n" .. + "{NUM:hDexRatingNumMonsOwned} #MON owned\fPROF.OAK's\nRating:" +Data.text._DexRatingText_Own50To59 = + "You finally got at\nleast 50 species!" + +local SaveData = require("src.core.SaveData") +local OW = require("src.world.OverworldController") + +local function setUpvalue(fn, name, val) + local i = 1 + while true do + local n = debug.getupvalue(fn, i) + if not n then return false end + if n == name then debug.setupvalue(fn, i, val); return true end + i = i + 1 + end +end + +local pushed = {} +local plays = {} +local stackStub = { + push = function(_, item) + pushed[#pushed + 1] = item + end, +} +local textBoxStub = { + new = function(_, text, onDone, opts) + return { kind = "text", text = text, onDone = onDone, opts = opts } + end, +} +local menuStub = { + new = function(_, items, opts) + return { kind = "menu", items = items, opts = opts or {} } + end, +} +-- Sound / Menu are required lazily at the call sites; stub via package.loaded +local realSound = package.loaded["src.core.Sound"] +package.loaded["src.core.Sound"] = { + play = function(_, name) + plays[#plays + 1] = name + end, + playCry = function() end, +} +local realMenu = package.loaded["src.ui.Menu"] +package.loaded["src.ui.Menu"] = menuStub + +local fakeGame = { + data = Data, + save = SaveData.newGame(), + stack = stackStub, +} +for _, name in ipairs({ "openOaksPC", "dexRating" }) do + T.check(setUpvalue(OW[name], "TextBox", textBoxStub), + ("TextBox upvalue on %s"):format(name)) + T.check(setUpvalue(OW[name], "Game", fakeGame), + ("Game upvalue on %s"):format(name)) +end +T.check(setUpvalue(OW.openPC, "Game", fakeGame), "Game upvalue on openPC") + +local fakeSelf = setmetatable({}, { __index = OW }) + +local function lastPush() + return pushed[#pushed] +end +local function reset() + fakeGame.save = SaveData.newGame() + fakeGame.save.flags.EVENT_GOT_POKEDEX = true + local seen, owned = {}, {} + for i = 1, 55 do seen[i] = true; owned[i] = true end + fakeGame.save.pokedex = { seen = seen, owned = owned } + pushed = {} + plays = {} +end +local function runChain() + -- A through every box that is up; the choice box answers YES + local guard = 0 + while lastPush() and lastPush().kind == "text" and guard < 10 do + guard = guard + 1 + local box = lastPush() + if box.opts and box.opts.choice then + box.opts.choice(true) + elseif box.onDone then + box.onDone() + else + break + end + end +end + +-- === full session from the launcher menu: intro, YES, rating, jingle, close +reset() +local done = false +fakeSelf:openPC(function() done = true end) +local menu = lastPush() +T.eq(menu.kind, "menu", "openPC pushes the PC menu") +local oak +for _, item in ipairs(menu.items) do + if item.label == "PROF.OAK's PC" then oak = item end +end +T.check(oak ~= nil, "PROF.OAK's PC is offered once the Pokédex is had") +plays = {} -- drop the menu's Turn_On_PC; the session's jingle is what counts +oak.onSelect() +T.eq(pushed[2].kind, "text", "selection opens the access text") +T.check(tostring(pushed[2].text):find("Accessed", 1, true) ~= nil, + "first session box is the access text") +runChain() +T.check(done, "session completes") +T.check(pushed[3].opts ~= nil and pushed[3].opts.choice ~= nil, + "the rated question carries the YES/NO choice") +T.check(tostring(pushed[3].text):find("rated", 1, true) ~= nil, + "second session box asks for the rating") +local ratingBox = pushed[4] +T.check(ratingBox.opts and ratingBox.opts.auto ~= nil + and ratingBox.opts.auto.wait ~= nil, + "rating box sounds the jingle then waits for a button") +T.check(tostring(ratingBox.text):find("55", 1, true) ~= nil, + "completion line carries the seen/owned counts") +T.check(tostring(ratingBox.text):find("least 50 species", 1, true) ~= nil, + "rating box carries the Own50To59 tier text") +T.eq(#plays, 0, "no jingle while the evaluation is printing") +ratingBox.opts.auto.sound() +T.eq(#plays, 1, "jingle fires once the rating text is printed") +T.eq(plays[1], "Pokedex_Rating", "jingle is the Pokedex_Rating fanfare") +T.check(tostring(pushed[5].text):find("Closed link", 1, true) ~= nil, + "the closing link prints at the end of the session") + +-- === declining the rating skips the evaluation but still closes the link +reset() +done = false +fakeSelf:openOaksPC(function() done = true end) +T.eq(pushed[1].kind, "text", "openOaksPC opens with the access text") +pushed[1].onDone() +T.check(pushed[2].opts and pushed[2].opts.choice ~= nil, + "rated question is a YES/NO") +pushed[2].opts.choice(false) +T.check(tostring(lastPush().text):find("Closed link", 1, true) ~= nil, + "NO skips the rating and closes the PC") +T.eq(#plays, 0, "declining plays no jingle") + +package.loaded["src.ui.Menu"] = realMenu +if realSound ~= nil then package.loaded["src.core.Sound"] = realSound end + +T.finish("oaks_pc_flow") From 2009df3dd13be9203f71bc9f55ad263d85fd7e32 Mon Sep 17 00:00:00 2001 From: bryanthaboi Date: Sat, 1 Aug 2026 12:56:13 -0400 Subject: [PATCH 2/3] CLOSES #575, CLOSES #578, CLOSES #584, CLOSES #589 --- docs/new-features.md | 12 ++ .../love/src/jni/love/src/common/android.cpp | 22 +++ .../love/src/jni/love/src/common/android.h | 9 + .../jni/love/src/modules/system/System.cpp | 9 + .../src/jni/love/src/modules/system/System.h | 8 + .../love/src/modules/system/wrap_System.cpp | 9 + .../java/org/love2d/android/GameActivity.java | 82 +++++++- src/core/Game.lua | 11 ++ src/core/HostShell.lua | 21 +++ src/import/RomImporter.lua | 77 +++++++- src/ui/BindingsMenu.lua | 178 +++++++++++++++--- tests/engine/host_restart_android_bug575.lua | 59 ++++++ tests/engine/launcher_text_input_bug578.lua | 148 +++++++++++++++ tests/engine/rebind_capture_bug510.lua | 36 +++- tests/engine/rebind_swap_clear_bug589.lua | 169 +++++++++++++++++ tests/mod_ui_tests.lua | 14 +- tools/save-editor/Kit.lua | 9 +- 17 files changed, 819 insertions(+), 54 deletions(-) create mode 100644 tests/engine/host_restart_android_bug575.lua create mode 100644 tests/engine/launcher_text_input_bug578.lua create mode 100644 tests/engine/rebind_swap_clear_bug589.lua diff --git a/docs/new-features.md b/docs/new-features.md index 9480a6af..70f7ff53 100644 --- a/docs/new-features.md +++ b/docs/new-features.md @@ -448,3 +448,15 @@ polls (better than a quarter of a second) and any direction in the mix cancels it, so it is hard to hit by accident -- including on the on-screen touch controls, where it would take four fingers held on four separate controls. + +## Controls rebinding (CONTROLS screen) + +OPTIONS -> CONTROLS lists every Game Boy button with its current keyboard +key and controller button side by side (Z/A). Press A on a row, then press +and release the key or pad button you want; the rebind commits on the +release. If that input already belongs to another row, the two rows swap, +so no button is ever stranded without an input and no input ever serves +two buttons. Holding a second key or pad button while the first is still +down backs out of the capture without touching a keyboard; Escape still +cancels too. SELECT clears one row back to its default, and START resets +every binding after a confirmation. diff --git a/mobile/android/love/src/jni/love/src/common/android.cpp b/mobile/android/love/src/jni/love/src/common/android.cpp index 7755f891..f295e527 100644 --- a/mobile/android/love/src/jni/love/src/common/android.cpp +++ b/mobile/android/love/src/jni/love/src/common/android.cpp @@ -231,6 +231,28 @@ bool syncHealthSteps() return result; } +bool restartApp() +{ + JNIEnv *env = (JNIEnv*) SDL_AndroidGetJNIEnv(); + jclass activity = env->FindClass("org/love2d/android/GameActivity"); + + // Old APK / new liblove skew: fail soft so HostShell.restart can fall + // back to a clean quit instead of aborting on a missing method (#575). + jmethodID method = env->GetStaticMethodID(activity, "restartApp", "()Z"); + if (method == nullptr) + { + env->ExceptionClear(); + env->DeleteLocalRef(activity); + return false; + } + + // Does not return on success: the Java side exits the process. + jboolean result = env->CallStaticBooleanMethod(activity, method); + + env->DeleteLocalRef(activity); + return result; +} + /* * Helper functions for the filesystem module */ diff --git a/mobile/android/love/src/jni/love/src/common/android.h b/mobile/android/love/src/jni/love/src/common/android.h index 63f93029..3f07e496 100644 --- a/mobile/android/love/src/jni/love/src/common/android.h +++ b/mobile/android/love/src/jni/love/src/common/android.h @@ -81,6 +81,15 @@ bool showCreateDocument(const char *suggestedName = nullptr); */ bool syncHealthSteps(); +/** + * Full process relaunch (GameActivity.restartApp): schedules the app's + * launch intent and kills the process, because the in-process + * quit("restart") loop double-inits physfs and crashes (#575). On success + * the process dies inside the Java call and this never returns; false + * means the relaunch could not be scheduled. + **/ +bool restartApp(); + /* * Helper functions for the filesystem module */ diff --git a/mobile/android/love/src/jni/love/src/modules/system/System.cpp b/mobile/android/love/src/jni/love/src/modules/system/System.cpp index 0ca2a484..0bbf214b 100644 --- a/mobile/android/love/src/jni/love/src/modules/system/System.cpp +++ b/mobile/android/love/src/jni/love/src/modules/system/System.cpp @@ -221,6 +221,15 @@ bool System::syncHealthSteps() const #endif } +bool System::restartApp() const +{ +#ifdef LOVE_ANDROID + return love::android::restartApp(); +#else + return false; +#endif +} + bool System::hasBackgroundMusic() const { #if defined(LOVE_ANDROID) diff --git a/mobile/android/love/src/jni/love/src/modules/system/System.h b/mobile/android/love/src/jni/love/src/modules/system/System.h index 8ef6aee4..c0fc76b4 100644 --- a/mobile/android/love/src/jni/love/src/modules/system/System.h +++ b/mobile/android/love/src/jni/love/src/modules/system/System.h @@ -133,6 +133,14 @@ public: */ virtual bool syncHealthSteps() const; + /** + * Relaunches the whole app with a fresh process (Android only; false + * elsewhere). The in-process love.event.quit("restart") double-inits + * physfs on Android and crashes, so src/core/HostShell.lua calls this + * instead (#575). Does not return on success -- the process exits. + **/ + virtual bool restartApp() const; + /** * Gets if the user is playing music on background. * Throws an exception on unsupported platforms. diff --git a/mobile/android/love/src/jni/love/src/modules/system/wrap_System.cpp b/mobile/android/love/src/jni/love/src/modules/system/wrap_System.cpp index 9127dbb2..8c4fb9cd 100644 --- a/mobile/android/love/src/jni/love/src/modules/system/wrap_System.cpp +++ b/mobile/android/love/src/jni/love/src/modules/system/wrap_System.cpp @@ -115,6 +115,14 @@ int w_syncHealthSteps(lua_State *L) return 1; } +int w_restartApp(lua_State *L) +{ + // Does not return on success: GameActivity.restartApp exits the process + // after scheduling the relaunch (#575). + luax_pushboolean(L, instance()->restartApp()); + return 1; +} + int w_hasBackgroundMusic(lua_State *L) { lua_pushboolean(L, instance()->hasBackgroundMusic()); @@ -133,6 +141,7 @@ static const luaL_Reg functions[] = { "pickFile", w_pickFile }, { "createFile", w_createFile }, { "syncHealthSteps", w_syncHealthSteps }, + { "restartApp", w_restartApp }, { "hasBackgroundMusic", w_hasBackgroundMusic }, { 0, 0 } }; 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 11c55125..5c0380b5 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 @@ -37,7 +37,9 @@ import java.util.List; import java.util.Map; import android.Manifest; +import android.app.AlarmManager; import android.app.AlertDialog; +import android.app.PendingIntent; import android.content.Context; import android.content.DialogInterface; import android.content.Intent; @@ -75,6 +77,7 @@ public class GameActivity extends SDLActivity { public static final int FILE_PICKER_REQUEST_CODE = 4; public static final int FILE_CREATE_REQUEST_CODE = 5; public static final int STEP_PERMISSION_REQUEST_CODE = 6; + public static final int RESTART_REQUEST_CODE = 7; /** @deprecated Prefer FILE_PICKER_REQUEST_CODE; kept for older call sites. */ public static final int ROM_PICKER_REQUEST_CODE = FILE_PICKER_REQUEST_CODE; // Mirrors conf.lua's t.identity ("pokemon-love2d"): where the picked file @@ -447,15 +450,23 @@ public class GameActivity extends SDLActivity { * Shows the system document picker (Storage Access Framework) so the * player can pick a ROM / mod / save from anywhere (Downloads, Drive, * etc.) without needing to know where the app's external files folder - * is. Requires API 19+ (ACTION_OPEN_DOCUMENT); the picked file (if any) - * arrives later in onActivityResult, not synchronously here. + * is. The picked file (if any) arrives later in onActivityResult, not + * synchronously here. + * + * API 21+ uses ACTION_OPEN_DOCUMENT; API 16-20 uses an ACTION_GET_CONTENT + * chooser instead. Below 19 OPEN_DOCUMENT does not exist, and on 19/20 + * the stock DocumentsUI is unreliable -- it launches and then hands back + * RESULT_CANCELED with no data, which onActivityResult cannot tell apart + * from the player cancelling (#584). GET_CONTENT lets any installed file + * manager serve the pick, and both intents return the same content:// or + * file:// URI shapes, so the result path in onActivityResult stays + * picker-agnostic and unchanged. * * @param destFilename basename under the app save identity (e.g. * picked_rom.gb, picked_mod.zip, picked_save.sav) */ @Keep public static boolean showFilePicker(String destFilename) { - if (android.os.Build.VERSION.SDK_INT < 19) return false; GameActivity self = (GameActivity) mSingleton; if (self == null) return false; if (destFilename == null || destFilename.length() == 0) { @@ -469,11 +480,26 @@ public class GameActivity extends SDLActivity { } self.pendingPickFilename = destFilename; - Intent intent = new Intent(Intent.ACTION_OPEN_DOCUMENT); + if (android.os.Build.VERSION.SDK_INT >= 21) { + Intent intent = new Intent(Intent.ACTION_OPEN_DOCUMENT); + intent.addCategory(Intent.CATEGORY_OPENABLE); + intent.setType("*/*"); + try { + self.startActivityForResult(intent, FILE_PICKER_REQUEST_CODE); + return true; + } catch (Exception e) { + // Some OEM / TV builds ship without DocumentsUI; fall through + // to the GET_CONTENT chooser below instead of giving up (#584). + Log.d("GameActivity", "could not open document picker: " + e.getMessage()); + } + } + Intent intent = new Intent(Intent.ACTION_GET_CONTENT); intent.addCategory(Intent.CATEGORY_OPENABLE); intent.setType("*/*"); try { - self.startActivityForResult(intent, FILE_PICKER_REQUEST_CODE); + self.startActivityForResult( + Intent.createChooser(intent, "Choose a file"), + FILE_PICKER_REQUEST_CODE); return true; } catch (Exception e) { Log.d("GameActivity", "could not open file picker: " + e.getMessage()); @@ -499,14 +525,60 @@ public class GameActivity extends SDLActivity { return showFilePicker(PICKED_SAVE_FILENAME); } + /** + * Relaunches the whole app for love.system.restartApp, used by + * src/core/HostShell.lua when a mod toggle needs a cold boot (#575). + * love.event.quit("restart") re-runs LOVE's boot inside the same + * process, and the second love.filesystem.init throws once physfs + * failed to deinit ("already initialized"), killing the app. Instead + * we hand our launch intent to AlarmManager and then exit the process: + * the alarm lives in system_server, so it survives our death and + * cannot race the exit the way a plain startActivity right before + * Runtime.exit can on some OEMs, and the dead process guarantees no + * native (physfs / SDL / JNI) state leaks into the fresh run. + */ + @Keep + public static boolean restartApp() { + GameActivity self = (GameActivity) mSingleton; + if (self == null) return false; + try { + Context context = self.getApplicationContext(); + Intent intent = context.getPackageManager() + .getLaunchIntentForPackage(context.getPackageName()); + if (intent == null) return false; + intent.addFlags(Intent.FLAG_ACTIVITY_CLEAR_TOP | Intent.FLAG_ACTIVITY_NEW_TASK); + int pendingFlags = PendingIntent.FLAG_CANCEL_CURRENT; + if (android.os.Build.VERSION.SDK_INT >= 23) { + // Mandatory mutability flag on API 31+; harmless from 23 up. + pendingFlags |= PendingIntent.FLAG_IMMUTABLE; + } + PendingIntent pending = PendingIntent.getActivity( + context, RESTART_REQUEST_CODE, intent, pendingFlags); + AlarmManager alarm = (AlarmManager) context.getSystemService(Context.ALARM_SERVICE); + if (alarm == null) return false; + alarm.set(AlarmManager.RTC, System.currentTimeMillis() + 250, pending); + } catch (Exception e) { + Log.d("GameActivity", "could not schedule restart: " + e.getMessage()); + return false; + } + Runtime.getRuntime().exit(0); + return true; // unreachable, but keeps the JNI signature honest + } + /** * Shows ACTION_CREATE_DOCUMENT so the player can save a staged export * (pending_export.sav in the app save identity) to Downloads / Drive / * etc. Suggested name is the dialog's default filename. + * + * CREATE_DOCUMENT does not exist below API 19 and has no pre-SAF + * equivalent, so unlike showFilePicker this stays 19+ (#584); the false + * return degrades on the Lua side (RomImporter export) to "Exported + * inside the app folder", which is the correct pre-KitKat behavior. */ @Keep public static boolean showCreateDocument(String suggestedName) { if (android.os.Build.VERSION.SDK_INT < 19) return false; + // (see showFilePicker for why the import side got a pre-19 path) GameActivity self = (GameActivity) mSingleton; if (self == null) return false; if (suggestedName == null || suggestedName.length() == 0) { diff --git a/src/core/Game.lua b/src/core/Game.lua index a4886323..101a91ed 100644 --- a/src/core/Game.lua +++ b/src/core/Game.lua @@ -491,8 +491,16 @@ function Game:restartWithMods() require("src.core.HostShell").restart() end +-- Releases reach Input even while a top state captures raw input: a +-- swallowed key-up would strand a held-state flag for a key Input saw go +-- down before the capture armed (the stuck-flag hazard Input:reset +-- exists for). The top state only OBSERVES the release afterwards, +-- unlike onKeyPressed above which owns the press, so BindingsMenu can +-- commit a capture on the key-up (#589). function Game:keyreleased(key) Input:keyreleased(key) + local top = self.stack and self.stack:top() + if top and top.onKeyReleased then top:onKeyReleased(key) end end function Game:gamepadpressed(joystick, button) @@ -509,7 +517,10 @@ function Game:gamepadpressed(joystick, button) end function Game:gamepadreleased(joystick, button) + -- same observe-after-Input contract as Game:keyreleased (#589) Input:gamepadreleased(joystick, button) + local top = self.stack and self.stack:top() + if top and top.onGamepadReleased then top:onGamepadReleased(button) end end function Game:gamepadaxis(joystick, axis, value) diff --git a/src/core/HostShell.lua b/src/core/HostShell.lua index 587a22f4..a6e3559e 100644 --- a/src/core/HostShell.lua +++ b/src/core/HostShell.lua @@ -26,9 +26,30 @@ end -- ("Failed to initialize filesystem: already initialized") and the relaunch -- crashes. So on an AppImage we relaunch the executable; the fresh process's -- Boot step mounts any downloaded update exactly as a manual relaunch would. +-- Android hits the same wall (#575): the vendored love.cpp loops runlove() +-- in-process on "restart", and PHYSFS_deinit in the old Filesystem module's +-- destructor fails ("files still open") whenever any physfs handle survives +-- lua_close, so the second PHYSFS_init throws the same "already initialized" +-- and the app dies. There we relaunch through the GameActivity.restartApp +-- JNI bridge (love.system.restartApp), which schedules our launch intent +-- and kills the process so no native state can leak into the fresh run. -- On every other platform the in-process restart works, so keep it. function HostShell.restart() if not (love and love.event and love.event.quit) then return end + + local osName = love.system and love.system.getOS and love.system.getOS() + if osName == "Android" then + -- restartApp kills the process on success, so a true return is never + -- observed; false means the bridge could not schedule the relaunch. + -- An older APK whose liblove predates the bridge (love.system.restartApp + -- is nil) has no crash-free in-process restart, so quit to the OS + -- cleanly and let the player relaunch by hand -- worse than restarting, + -- but better than the guaranteed crash of quit("restart") (#575). + if love.system.restartApp and love.system.restartApp() then return end + love.event.quit() + return + end + local appimage = os.getenv("APPIMAGE") if not appimage then love.event.quit("restart") diff --git a/src/import/RomImporter.lua b/src/import/RomImporter.lua index 90f2aa4a..8d6b7766 100644 --- a/src/import/RomImporter.lua +++ b/src/import/RomImporter.lua @@ -1386,6 +1386,7 @@ function RomImporter:_cycleTab(delta) self._slotPress = nil self._modPress = nil self._findSearchFocus = false + self:_disarmTextInput() end function RomImporter:_updatePadCursor(dt) @@ -2264,7 +2265,7 @@ function RomImporter:draw() col(PAL.bgBot, 0.72) love.graphics.rectangle("fill", 0, 0, fullW, fullH) local dw = math.min(appW - 32 * s, 520 * s) - local dh = 168 * s + local dh = 176 * s local dx = appX + (appW - dw) / 2 local dy = oy + (height - dh) / 2 local rr = 12 * s @@ -2308,6 +2309,14 @@ function RomImporter:draw() fy + 7 * s, math.max(1, 1.5 * s), fh - 14 * s) end + -- PASTE under the field: a touch screen has no ctrl+V, and an index URL + -- is not something anyone retypes on a soft keyboard (#578). This rect + -- is the one click mousepressed honors while the prompt is up; pinned so + -- page-scroll banding never eats the tap. + self._indexPasteRect = self:_chipButton(fx + fw - 84 * s, fy + fh + 8 * s, + Strings("Paste"), { w = 84 * s, h = 28 * s, kind = "accent" }) + self._indexPasteRect.pinned = true + love.graphics.setFont(self.hintFont) col(PAL.warning) printfB(Strings("Enter to add - Esc to cancel"), @@ -2649,7 +2658,14 @@ end function RomImporter:mousepressed(x, y, button) if self._rename then return end -- the rename modal swallows all clicks - if self._indexPrompt then return end -- and so does the add-index prompt + -- The add-index prompt swallows clicks too, except its PASTE button: a + -- touch screen has no ctrl+V, so the button is the only paste path (#578). + if self._indexPrompt then + if button == 1 and inside(self._indexPasteRect, x, y) then + self:_pasteIndexUrl() + end + return + end -- Mod confirm / versions / release-notes modals swallow clicks too. if self._modConfirm then if button ~= 1 then return end @@ -2756,6 +2772,7 @@ function RomImporter:mousepressed(x, y, button) self._modPress = nil -- and any half-started mod toggle press self._pagePress = nil -- and any half-started page pan self._findSearchFocus = false -- and the search caret, now off screen + self:_disarmTextInput() -- Each tab is its own column of a different length; carrying one tab's -- offset into another lands somewhere arbitrary. self.pageScroll = 0 @@ -2877,11 +2894,14 @@ function RomImporter:mousepressed(x, y, button) end if inside(self.findRefreshRect, x, y) then self._findSearchFocus = false + self:_disarmTextInput() self:_refreshFind(true) return end if inside(self.findSearchRect, x, y) then - self._findSearchFocus = true; return + self._findSearchFocus = true + self:_armTextInput() + return end for _, r in ipairs(self.findSourceRemoveRects or {}) do if inside(r, x, y) then self:_removeIndex(r.id); return end @@ -2909,7 +2929,10 @@ function RomImporter:mousepressed(x, y, button) end -- A press anywhere else on the tab drops the search caret, so the field does -- not silently keep eating keystrokes once the player has moved on. - if self.tab == "find" then self._findSearchFocus = false end + if self.tab == "find" and self._findSearchFocus then + self._findSearchFocus = false + self:_disarmTextInput() + end -- Nothing was hit. On a scrolling page that is a press on empty background, -- which is the natural place to grab and pan from. if armDrag and (self._pageMax or 0) > 0 then @@ -2925,6 +2948,7 @@ function RomImporter:keypressed(key) self:_commitRename() elseif key == "escape" then self._rename = nil + self:_disarmTextInput() end return end @@ -2935,13 +2959,11 @@ function RomImporter:keypressed(key) self:_commitAddIndex() elseif key == "escape" then self._indexPrompt = nil + self:_disarmTextInput() elseif key == "v" and (love.keyboard.isDown("lctrl", "rctrl", "lgui", "rgui")) then -- an index URL is long and comes from a browser: typing it out by hand -- is the difference between adding one and giving up - local ok, text = pcall(love.system.getClipboardText) - if ok and type(text) == "string" then - self._indexPrompt.text = self._indexPrompt.text .. text:gsub("%s", "") - end + self:_pasteIndexUrl() end return end @@ -2965,6 +2987,7 @@ function RomImporter:keypressed(key) self.findScroll = 0 elseif key == "escape" or key == "return" or key == "kpenter" then self._findSearchFocus = false + self:_disarmTextInput() end return end @@ -3518,6 +3541,27 @@ local MAX_SLOT_LABEL = 24 local MAX_INDEX_URL = 200 local MAX_FIND_QUERY = 48 +-- Mobile LOVE only delivers love.textinput while setTextInput(true) is armed, +-- and arming it is also what raises the soft keyboard, so a cabled USB +-- keyboard is just as dead without it (#578). Every site that opens one of +-- the launcher's three text fields (_rename, _indexPrompt, _findSearchFocus) +-- arms through here, and every site that closes one disarms. Desktop has +-- text input on by default and the save editor hosted from this launcher +-- depends on it staying on (tools/save-editor/Kit.lua, #529), so disarm only +-- lowers on mobile -- setTextInput is global SDL state, not per-widget. +function RomImporter:_armTextInput() + if love.keyboard and love.keyboard.setTextInput then + pcall(love.keyboard.setTextInput, true) + end +end + +function RomImporter:_disarmTextInput() + if not self.android then return end + if love.keyboard and love.keyboard.setTextInput then + pcall(love.keyboard.setTextInput, false) + end +end + function RomImporter:_beginRename(version, id) local label for _, slot in ipairs(self.slots[version] or {}) do @@ -3525,12 +3569,14 @@ function RomImporter:_beginRename(version, id) end self._rename = { version = version, id = id, text = label or "" } self._slotPress = nil -- cancel any armed click/drag on the list + self:_armTextInput() end function RomImporter:_commitRename() local r = self._rename if not r then return end self._rename = nil + self:_disarmTextInput() require("src.core.SaveData").renameSlot(r.version, r.id, r.text) self:_refreshSlots(r.version) end @@ -3552,6 +3598,19 @@ function RomImporter:textinput(text) self._rename.text = utf8Cap(self._rename.text .. text, MAX_SLOT_LABEL) end +-- Clipboard into the index prompt, shared by ctrl/cmd+V and the prompt's +-- on-screen PASTE button (#578). Same rule as typed input: URLs never +-- contain a literal space, and a pasted one usually arrives with a stray +-- newline attached. +function RomImporter:_pasteIndexUrl() + if not self._indexPrompt then return end + local ok, text = pcall(love.system.getClipboardText) + if ok and type(text) == "string" then + self._indexPrompt.text = + utf8Cap(self._indexPrompt.text .. text:gsub("%s", ""), MAX_INDEX_URL) + end +end + -- "+ New save slot": register an empty slot, make it active, relist, and pin the -- scroll to the bottom (clamped next draw) so the new row is on screen. function RomImporter:_newSlot(version) @@ -4595,11 +4654,13 @@ end -- would make the launcher's choice look like an endorsement. function RomImporter:_promptAddIndex() self._indexPrompt = { text = "" } + self:_armTextInput() end function RomImporter:_commitAddIndex() local prompt = self._indexPrompt self._indexPrompt = nil + self:_disarmTextInput() if not prompt then return end local ModIndex = require("src.mods.ModIndex") local row, err = ModIndex.addSource(prompt.text or "") diff --git a/src/ui/BindingsMenu.lua b/src/ui/BindingsMenu.lua index 291a640d..5cea86ba 100644 --- a/src/ui/BindingsMenu.lua +++ b/src/ui/BindingsMenu.lua @@ -6,6 +6,7 @@ local Font = require("src.render.Font") local ListMenu = require("src.ui.ListMenu") +local ChoiceBox = require("src.ui.ChoiceBox") local Input = require("src.core.Input") local Strings = require("src.core.Strings") @@ -13,16 +14,18 @@ local BindingsMenu = setmetatable({}, { __index = ListMenu }) BindingsMenu.__index = BindingsMenu -- Input.lua's map, primary key first where several keys share a button. --- `pad` is the default SDL gamecontroller button (see Input.lua); shown --- on the SELECT row so controller Back/View is discoverable (#73). +-- `pad` mirrors DEFAULT_GAMEPAD_BINDINGS in src/core/Input.lua row for +-- row; keep the two in sync. Every row shows its key and pad so the +-- controller side is discoverable (#73, #589), and the swap in +-- storeBinding leans on each row holding a value in both slots. local BUTTONS = { - { id = "up", label = "UP", key = "up" }, - { id = "down", label = "DOWN", key = "down" }, - { id = "left", label = "LEFT", key = "left" }, - { id = "right", label = "RIGHT", key = "right" }, - { id = "a", label = "A", key = "z" }, - { id = "b", label = "B", key = "x" }, - { id = "start", label = "START", key = "escape" }, + { id = "up", label = "UP", key = "up", pad = "dpup" }, + { id = "down", label = "DOWN", key = "down", pad = "dpdown" }, + { id = "left", label = "LEFT", key = "left", pad = "dpleft" }, + { id = "right", label = "RIGHT", key = "right", pad = "dpright" }, + { id = "a", label = "A", key = "z", pad = "a" }, + { id = "b", label = "B", key = "x", pad = "b" }, + { id = "start", label = "START", key = "escape", pad = "start" }, { id = "select", label = "SELECT", key = "tab", pad = "back" }, } @@ -41,14 +44,33 @@ local function boundPad(overlay, def) return def.pad end --- Key column for every row. SELECT also appends "/PAD" (default BACK) --- so controller Select/View is visible without opening a second legend. +-- The right column is KEY/PAD (e.g. "Z/A"). The row is 20 tiles and the +-- widest label ("SELECT") ends at x=64, so each half is clamped to 5 +-- glyphs: 5+1+5 right-aligned at x=152 starts no further left than x=64. +-- SDL names longer than 5 get a fixed short form before the clamp. +local KEY_SHORT = { + escape = "ESC", backspace = "BKSP", ["return"] = "ENTER", + kpenter = "ENTER", space = "SPACE", +} +local PAD_SHORT = { + dpup = "D-UP", dpdown = "D-DN", dpleft = "D-LT", dpright = "D-RT", + leftshoulder = "LB", rightshoulder = "RB", + leftstick = "LS", rightstick = "RS", guide = "GUIDE", +} +local function shortName(name, shorts) + local s = shorts[name] + if s then return s end + s = name:upper() + return #s > 5 and s:sub(1, 5) or s +end + +-- Right column for every row: effective key and pad together, so a +-- controller player can read the whole map without a second legend (#589). local function boundRight(overlay, def) - local key = boundKey(overlay, def) - if def.id ~= "select" then return key:upper() end + local key = shortName(boundKey(overlay, def), KEY_SHORT) local pad = boundPad(overlay, def) - if pad then return (key .. "/" .. pad):upper() end - return key:upper() + if pad then return key .. "/" .. shortName(pad, PAD_SHORT) end + return key end function BindingsMenu.new(game) @@ -61,9 +83,17 @@ function BindingsMenu.new(game) items[i] = { label = Strings(def.label), right = boundRight(overlay, def), button = def } end - local self = setmetatable(ListMenu.new(game, "CONTROLS", items, {}), - BindingsMenu) + local self = setmetatable(ListMenu.new(game, "CONTROLS", items, { + -- 6 rows leaves the bottom two lines free for the hint; a clear or + -- reset nobody can see on screen may as well not exist (#589) + rows = 6, + footer = Strings("SELECT:CLEAR ROW\nSTART:RESET ALL"), + }), BindingsMenu) self.onChoose = function(item) self:beginCapture(item) end + -- SELECT deletes one row's rebind: dropping the overlay entry is enough + -- because Input:applyBindings rebuilds the whole map from the defaults + -- on every call (#589) + self.onSelectKey = function(item) self:clearBinding(item) end -- A rebind reaches Input only when this screen closes (#510). The menu -- steers by the live map, so applying "B = Z" the instant it was captured -- turned the player's next confirm press into a cancel and shut the @@ -82,12 +112,28 @@ function BindingsMenu:commitBindings() if opts then Input:applyBindings(opts.bindings) end end --- the capture handlers are per-instance slots, so Game's raw-input --- routing only ever sees this screen while a capture is armed +-- The capture handlers are per-instance slots, so Game's raw-input +-- routing only ever sees this screen while a capture is armed. A capture +-- no longer commits on the press: it commits when that press is RELEASED, +-- and a second key or pad button going down while the first is still held +-- cancels instead. That gives a bare controller a way to back out of an +-- armed row, where Escape cannot help (#589). function BindingsMenu:beginCapture(item) self.capture = item + self.pending = nil self.onKeyPressed = BindingsMenu.captureKey self.onGamepadPressed = BindingsMenu.capturePad + self.onKeyReleased = BindingsMenu.captureKeyRelease + self.onGamepadReleased = BindingsMenu.capturePadRelease +end + +function BindingsMenu:endCapture() + self.capture = nil + self.pending = nil + self.onKeyPressed = nil + self.onGamepadPressed = nil + self.onKeyReleased = nil + self.onGamepadReleased = nil end -- Escape is the capture's way out, so it is never captured: every other @@ -95,23 +141,64 @@ end -- to bind something (#510). Escape stays START in Input's default map, -- which no rebind removes, so reserving it costs the player nothing. function BindingsMenu:captureKey(key) - if key == "escape" then return self:storeBinding("key", nil) end - self:storeBinding("key", key) + if key == "escape" or self.pending then return self:endCapture() end + self.pending = { slot = "key", value = key } end function BindingsMenu:capturePad(button) - self:storeBinding("pad", button) + if self.pending then return self:endCapture() end + self.pending = { slot = "pad", value = button } +end + +-- Game forwards every release to Input BEFORE these hooks (see +-- Game:keyreleased): the capture observes releases, it never owns them, +-- so Input's held-state stays honest for keys it saw go down before the +-- capture armed. A release that does not match the pending input (the +-- press that armed the row, a cancelled capture's stragglers) is noise. +function BindingsMenu:captureKeyRelease(key) + local p = self.pending + if p and p.slot == "key" and p.value == key then + self:storeBinding("key", key) + end +end + +function BindingsMenu:capturePadRelease(button) + local p = self.pending + if p and p.slot == "pad" and p.value == button then + self:storeBinding("pad", button) + end end function BindingsMenu:storeBinding(slot, value) local item = self.capture - self.capture = nil - self.onKeyPressed = nil - self.onGamepadPressed = nil + self:endCapture() local game = self.game if not (item and value and game.save and game.save.options) then return end local opts = game.save.options opts.bindings = opts.bindings or {} + -- Swap, never steal (#589): when the captured input is another row's + -- effective binding in this slot, that row inherits this row's previous + -- binding. Every BUTTONS row has a default in both slots, so `prev` + -- always exists: no row goes empty and no input serves two rows. + -- Default key ALIASES (W beside Up, Space beside Z; DEFAULT_BINDINGS in + -- Input.lua) are not effective bindings, so capturing one costs the + -- other row a spare alias, never its shown key. + local effective = (slot == "key") and boundKey or boundPad + local prev = effective(opts.bindings, item.button) + if value ~= prev then + for _, other in ipairs(self.items) do + if other ~= item and effective(opts.bindings, other.button) == value then + local ob = opts.bindings[other.button.id] + if type(ob) ~= "table" then + ob = { key = type(ob) == "string" and ob or nil } + end + ob[slot] = prev + opts.bindings[other.button.id] = ob + other.right = boundRight(opts.bindings, other.button) + break + end + end + end local b = opts.bindings[item.button.id] if type(b) ~= "table" then -- keep a direct-edited plain key string when only the pad changes @@ -123,18 +210,55 @@ function BindingsMenu:storeBinding(slot, value) if game.writeOptions then game:writeOptions() end end +-- SELECT: forget one row's rebind and fall back to the defaults. #510's +-- deferral still holds: only options change here, the live map catches up +-- in commitBindings on close. +function BindingsMenu:clearBinding(item) + local game = self.game + local opts = game and game.save and game.save.options + if not (opts and opts.bindings and opts.bindings[item.button.id]) then + return + end + opts.bindings[item.button.id] = nil + item.right = boundRight(opts.bindings, item.button) + if game.writeOptions then game:writeOptions() end +end + +-- START: confirm, then drop the whole overlay (#589). The footer doubles +-- as the prompt while the YES/NO box is up, the same bottom-line pattern +-- the mart and PC screens use. +function BindingsMenu:confirmReset() + local game = self.game + local hint = self.footer + self.footer = Strings("RESET ALL BINDINGS?") + game.stack:push(ChoiceBox.new(game, function(yes) + self.footer = hint + if not yes then return end + local opts = game.save and game.save.options + if opts then opts.bindings = nil end + for _, it in ipairs(self.items) do + it.right = boundRight(nil, it.button) + end + if game.writeOptions then game:writeOptions() end + end, { defaultNo = true })) +end + function BindingsMenu:update(dt) if self.capture then return end -- the raw capture owns the input + if self.game.input:wasPressed("start") then + return self:confirmReset() + end ListMenu.update(self, dt) end function BindingsMenu:draw() ListMenu.draw(self) if self.capture then - Font.drawBox(1, 6, 18, 5) + Font.drawBox(1, 6, 18, 6) love.graphics.setColor(0, 0, 0, 1) Font.draw(Strings("PRESS A BUTTON"), 24, 60) - Font.draw(Strings("ESC TO CANCEL"), 24, 72) + Font.draw(Strings("RELEASE TO SET"), 24, 72) + Font.draw(Strings("ESC/2ND CANCELS"), 24, 84) love.graphics.setColor(1, 1, 1, 1) end end diff --git a/tests/engine/host_restart_android_bug575.lua b/tests/engine/host_restart_android_bug575.lua new file mode 100644 index 00000000..82f1dca3 --- /dev/null +++ b/tests/engine/host_restart_android_bug575.lua @@ -0,0 +1,59 @@ +-- #575: HostShell.restart on Android must never reach love.event.quit +-- ("restart") -- the vendored love.cpp loops runlove() in-process on +-- "restart" and the second PHYSFS_init crashes ("already initialized"). +-- The fix prefers the love.system.restartApp JNI bridge (which kills the +-- process, so a true return is never observed live) and, on an old APK +-- whose liblove lacks the bridge, falls back to a CLEAN quit with no +-- argument. Desktop keeps the in-process quit("restart"). +-- luajit tests/engine/host_restart_android_bug575.lua + +package.path = "./?.lua;./?/init.lua;" .. package.path + +local T = require("tests.harness") +local check, eq = T.check, T.eq +love = love or require("tests.love_stub") + +local HostShell = require("src.core.HostShell") + +local quits = {} +love.event = { + -- record the argument distinctly from "called with none": quit() and + -- quit("restart") are the whole difference this test pins + quit = function(...) + quits[#quits + 1] = { n = select("#", ...), arg = (...) } + end, +} + +local osName = "Android" +local restartCalls = 0 +love.system = love.system or {} +love.system.getOS = function() return osName end + +-- bridge present and schedulable: restart goes through it, quit untouched +love.system.restartApp = function() restartCalls = restartCalls + 1 return true end +HostShell.restart() +eq(restartCalls, 1, "Android restart prefers the restartApp bridge (#575)") +eq(#quits, 0, "a scheduled relaunch never touches love.event.quit") + +-- bridge present but could not schedule: clean quit, never quit("restart") +love.system.restartApp = function() restartCalls = restartCalls + 1 return false end +HostShell.restart() +eq(restartCalls, 2, "the bridge is still tried first") +eq(#quits, 1, "a failed schedule falls back to one quit") +eq(quits[1].n, 0, "and it is a bare quit(), not quit(\"restart\")") + +-- old APK, no bridge compiled in: same clean quit fallback +love.system.restartApp = nil +HostShell.restart() +eq(#quits, 2, "a bridge-less APK quits cleanly instead of crashing") +eq(quits[2].n, 0, "again with no restart argument") + +-- desktop (no AppImage in a test environment) keeps the in-process restart +if not os.getenv("APPIMAGE") then + osName = "OS X" + HostShell.restart() + eq(quits[3] and quits[3].arg, "restart", + "non-Android still restarts in-process") +end + +T.finish("host_restart_android_bug575") diff --git a/tests/engine/launcher_text_input_bug578.lua b/tests/engine/launcher_text_input_bug578.lua new file mode 100644 index 00000000..2e8d7403 --- /dev/null +++ b/tests/engine/launcher_text_input_bug578.lua @@ -0,0 +1,148 @@ +-- #578: the launcher's "Add an index" prompt (and the rename / find-search +-- fields) accepted no typing on Android, because nothing ever called +-- love.keyboard.setTextInput(true) -- mobile LOVE only delivers +-- love.textinput while it is armed. Every site that opens a text field must +-- arm, every site that closes one must disarm, and disarm must be a no-op on +-- desktop where the hosted save editor depends on text input staying on +-- (tools/save-editor/Kit.lua, #529). A touch screen also has no ctrl+V, so +-- the prompt grew a PASTE chip; both paste paths share _pasteIndexUrl and +-- both honor the whitespace strip and the MAX_INDEX_URL cap. +-- luajit tests/engine/launcher_text_input_bug578.lua + +package.path = "./?.lua;./?/init.lua;" .. package.path + +local T = require("tests.harness") +local check, eq = T.check, T.eq +love = love or require("tests.love_stub") + +-- record every setTextInput transition; the assertions read this log +local textInputLog = {} +love.keyboard.setTextInput = function(on) + textInputLog[#textInputLog + 1] = on +end +local function lastArm() return textInputLog[#textInputLog] end + +local clipboard = "" +love.system = love.system or {} +love.system.getClipboardText = function() return clipboard end + +-- _commitAddIndex hands the typed URL to ModIndex.addSource; a canned +-- failure keeps the commit path off the network and out of options.lua +local addSourceUrl = nil +package.loaded["src.mods.ModIndex"] = { + addSource = function(url) addSourceUrl = url return nil, "offline" end, +} +-- _commitRename goes through SaveData.renameSlot; record the call +local renamed = nil +package.loaded["src.core.SaveData"] = { + renameSlot = function(version, id, text) renamed = { version, id, text } end, +} + +local RomImporter = require("src.import.RomImporter") + +local ri = setmetatable({ + android = true, workState = nil, tab = "find", + ready = {}, slots = { red = { { id = "s1", label = "OLD" } } }, + slotScroll = {}, activeSlot = {}, +}, RomImporter) +ri._refreshSlots = function() end -- rename commit relists; nothing to relist + +-- ---- index prompt: arm on open, disarm on escape and on commit ------------ + +ri:_promptAddIndex() +check(ri._indexPrompt ~= nil, "the add-index prompt opens") +eq(lastArm(), true, "opening the prompt arms setTextInput (#578)") + +-- typed input strips whitespace (URLs never contain a literal space) +ri:textinput("https://ex ample.com\n/idx") +eq(ri._indexPrompt.text, "https://example.com/idx", + "typed input lands with whitespace stripped") + +ri:keypressed("escape") +check(ri._indexPrompt == nil, "escape closes the prompt") +eq(lastArm(), false, "and disarms setTextInput") + +ri:_promptAddIndex() +ri._indexPrompt.text = "https://example.com/index.json" +ri:keypressed("return") +check(ri._indexPrompt == nil, "enter commits and closes the prompt") +eq(lastArm(), false, "commit disarms setTextInput too") +eq(addSourceUrl, "https://example.com/index.json", + "the committed text reaches ModIndex.addSource") +check(ri.findNotice and ri.findNotice.ok == false, + "a rejected source surfaces as a notice, not a crash") + +-- ---- PASTE chip: same entry point the touch screen uses ------------------- + +ri:_promptAddIndex() +-- the chip rect is what draw() published last frame (pinned: modal chrome +-- ignores the page-scroll band); mousepressed hit-tests it while the +-- prompt is up and everywhere else the prompt swallows the press +ri._indexPasteRect = { x = 10, y = 10, width = 60, height = 24, pinned = true } +clipboard = " https://example.com/mods/index.json\n" +ri:mousepressed(200, 200, 1) +eq(ri._indexPrompt.text, "", "a press outside the chip pastes nothing") +ri:mousepressed(20, 20, 1) +eq(ri._indexPrompt.text, "https://example.com/mods/index.json", + "the PASTE chip lands the clipboard with whitespace stripped (#578)") + +-- the cap holds through the button path: a 300-char clipboard cannot +-- overflow MAX_INDEX_URL (200) +ri._indexPrompt.text = "" +clipboard = string.rep("a", 300) +ri:mousepressed(20, 20, 1) +eq(#ri._indexPrompt.text, 200, "the PASTE chip enforces MAX_INDEX_URL") + +-- and through ctrl/cmd+V, which used to skip the cap entirely +ri._indexPrompt.text = "" +local savedIsDown = love.keyboard.isDown +love.keyboard.isDown = function() return true end +ri:keypressed("v") +love.keyboard.isDown = savedIsDown +eq(#ri._indexPrompt.text, 200, "ctrl+V routes through the same cap (#578)") +ri:keypressed("escape") + +-- ---- rename field: arm on open, disarm on escape and on commit ------------ + +ri:_beginRename("red", "s1") +check(ri._rename ~= nil, "the rename modal opens") +eq(lastArm(), true, "opening the rename arms setTextInput") +ri:keypressed("escape") +check(ri._rename == nil, "escape closes the rename") +eq(lastArm(), false, "and disarms setTextInput") + +ri:_beginRename("red", "s1") +ri:textinput("!") +ri:keypressed("return") +eq(lastArm(), false, "committing the rename disarms setTextInput") +eq(renamed and renamed[3], "OLD!", "the commit reaches SaveData.renameSlot") + +-- ---- find-search field: arm on rect press, disarm on escape --------------- + +ri.findSearchRect = { x = 100, y = 100, width = 80, height = 20 } +ri:mousepressed(110, 110, 1) +check(ri._findSearchFocus == true, "pressing the search field takes focus") +eq(lastArm(), true, "and arms setTextInput") +ri:keypressed("escape") +check(ri._findSearchFocus == false, "escape drops the search caret") +eq(lastArm(), false, "and disarms setTextInput") + +-- a press elsewhere on the find tab also drops the caret and disarms +ri:mousepressed(110, 110, 1) +eq(lastArm(), true, "refocus for the click-away case") +ri:mousepressed(400, 400, 1) +check(ri._findSearchFocus == false, "a click away drops the caret") +eq(lastArm(), false, "and disarms setTextInput") + +-- ---- desktop contract (#529): disarm never lowers off Android ------------- + +ri.android = false +ri:_promptAddIndex() +eq(lastArm(), true, "desktop still arms (harmless, already on)") +local before = #textInputLog +ri:keypressed("escape") +eq(#textInputLog, before, + "desktop disarm is a no-op: the hosted save editor keeps text input on " + .. "(#529)") + +T.finish("launcher_text_input_bug578") diff --git a/tests/engine/rebind_capture_bug510.lua b/tests/engine/rebind_capture_bug510.lua index c991cb93..e5d13d31 100644 --- a/tests/engine/rebind_capture_bug510.lua +++ b/tests/engine/rebind_capture_bug510.lua @@ -2,7 +2,10 @@ -- screen that captured it is still steering by that map (#510). Swapping A -- and B used to close the screen mid-swap, because Input:applyBindings ran -- inside BindingsMenu:storeBinding and turned the player's next confirm --- press into a cancel. No pokered cite: rebinding is port-only (gap C2). +-- press into a cancel. A capture commits on the RELEASE of its press, a +-- second held input cancels it, and a captured input that another row owns +-- swaps rather than steals (#589). No pokered cite: rebinding is +-- port-only (gap C2). -- luajit tests/engine/rebind_capture_bug510.lua package.path = "./?.lua;./?/init.lua;" .. package.path @@ -73,19 +76,29 @@ eq(game.wroteOptions, 0, "and does not touch options on disk") bm.index = ROW_B press(bm, "a") bm:onKeyPressed("z") -eq(game.save.options.bindings.b.key, "z", "the capture stores B = Z") -eq(bm.items[ROW_B].right, "Z", "the row shows the new key straight away") +check(game.save.options.bindings == nil, + "a capture holds its press; nothing stores before the release (#589)") +bm:onKeyReleased("z") +eq(game.save.options.bindings.b.key, "z", "releasing the press stores B = Z") +eq(bm.items[ROW_B].right, "Z/B", "the row shows the new key straight away") eq(game.wroteOptions, 1, "the choice persists immediately") eq(Input.keyBindings["z"], "a", "but the live map still reads Z as A while the screen is open (#510)") eq(#game.stack.states, 1, "capturing Z does not close the screen") +-- Z was the A row's effective key, so the steal became a swap: the A row +-- inherits B's previous key and no key serves two rows (#589) +eq(game.save.options.bindings.a.key, "x", + "capturing A's key for B hands A the old B key") +eq(bm.items[ROW_A].right, "X/A", "and the A row redraws with it") + -- the next Z the player presses is still confirm, so the A row can be armed bm.index = ROW_A press(bm, "a") eq(bm.capture, bm.items[ROW_A], "the A row arms instead of the screen closing") bm:onKeyPressed("x") -eq(game.save.options.bindings.a.key, "x", "the swap's other half stores") +bm:onKeyReleased("x") +eq(game.save.options.bindings.a.key, "x", "re-capturing A's own key keeps it") eq(Input.keyBindings["x"], "b", "and X is still cancel until the screen closes") -- closing commits both halves at once, through ListMenu's onCancel @@ -107,8 +120,23 @@ local padBm = openMenu(padGame) padBm.index = ROW_B press(padBm, "a") padBm:onGamepadPressed("y") +padBm:onGamepadReleased("y") eq(padGame.save.options.bindings.b.pad, "y", "a pad capture stores") eq(Input.padBindings["y"], nil, "and stays out of the live pad map until close") + +-- a second input going down while the first is held backs the capture out +-- with no keyboard in reach, the pad's Escape (#589) +padBm.index = ROW_A +press(padBm, "a") +padBm:onGamepadPressed("x") +padBm:onGamepadPressed("b") +check(padBm.capture == nil, "a second press cancels the armed capture") +-- Game only calls the hook while it is armed; the straggling release of +-- the first button reaches a disarmed menu and stores nothing +if padBm.onGamepadReleased then padBm:onGamepadReleased("x") end +eq(padGame.save.options.bindings.a, nil, + "a cancelled capture's release writes no binding") + press(padBm, "b") eq(Input.padBindings["y"], "b", "closing commits the pad half too") diff --git a/tests/engine/rebind_swap_clear_bug589.lua b/tests/engine/rebind_swap_clear_bug589.lua new file mode 100644 index 00000000..8efd19d0 --- /dev/null +++ b/tests/engine/rebind_swap_clear_bug589.lua @@ -0,0 +1,169 @@ +-- CONTROLS rebinding, the #589 feature set beyond the capture deferral that +-- tests/engine/rebind_capture_bug510.lua pins: a captured pad button another +-- row effectively owns SWAPS with that row (no input serves two rows, no row +-- goes empty), a second input of either kind cancels an armed capture, +-- SELECT forgets one row's rebind so it falls back to the default, and START +-- confirms then drops the whole overlay. Everything is driven through the +-- entry points Game routes raw input to (onKeyPressed/onKeyReleased/ +-- onGamepadPressed/onGamepadReleased) and through update() for the menu +-- keys. No pokered cite: rebinding is port-only (gap C2). +-- luajit tests/engine/rebind_swap_clear_bug589.lua + +package.path = "./?.lua;./?/init.lua;" .. package.path + +local T = require("tests.harness") +local check, eq = T.check, T.eq +love = love or require("tests.love_stub") + +local Input = require("src.core.Input") +local Strings = require("src.core.Strings") +local BindingsMenu = require("src.ui.BindingsMenu") + +-- same doubles as rebind_capture_bug510: a stack the menu can pop itself +-- off and an input whose queue is one fixed step of edges. data = {} keeps +-- ChoiceBox's un-guarded Sound.play on the headless no-audio path. +local function newGame() + local game = { save = { options = {} }, data = {}, wroteOptions = 0 } + game.stack = { + states = {}, + push = function(self, s) table.insert(self.states, s) end, + pop = function(self) return table.remove(self.states) end, + top = function(self) return self.states[#self.states] end, + } + game.input = { + queue = {}, + wasPressed = function(self, btn) return self.queue[btn] or false end, + isDown = function() return false end, + } + function game:writeOptions() self.wroteOptions = self.wroteOptions + 1 end + return game +end + +local function press(state, btn) + state.game.input.queue = { [btn] = true } + state:update(1 / 60) + state.game.input.queue = {} +end + +-- rows are BindingsMenu's BUTTONS order +local ROW_A, ROW_B, ROW_SELECT = 5, 6, 8 + +-- ---- (a) pad capture swaps with the row that owns the button -------------- + +Input:init() +local game = newGame() +local bm = BindingsMenu.new(game) +game.stack:push(bm) + +bm.index = ROW_A +press(bm, "a") +eq(bm.capture, bm.items[ROW_A], "A arms the A row") +bm:onGamepadPressed("b") +check(game.save.options.bindings == nil, + "a pad capture holds its press; nothing stores before the release") +bm:onGamepadReleased("b") +local bindings = game.save.options.bindings +eq(bindings.a.pad, "b", "releasing pad B stores it on the A row") +eq(bindings.b.pad, "a", + "and the B row, which owned pad B, inherits the A row's old pad (#589)") +eq(bm.items[ROW_A].right, "Z/B", "the A row redraws with the new pad") +eq(bm.items[ROW_B].right, "X/A", "so does the B row") + +-- after applying, every row's effective pad is unique and none is lost +Input:applyBindings(bindings) +eq(Input.padBindings["b"], "a", "applied: pad B is action A") +eq(Input.padBindings["a"], "b", "applied: pad A is action B") +local seen, actions = {}, {} +for button, action in pairs(Input.padBindings) do + check(not seen[action], "no action is reachable from two pad buttons: " + .. tostring(action)) + seen[action] = button + actions[#actions + 1] = action +end +eq(#actions, 8, "all eight actions still have exactly one pad button") +Input:init() + +-- ---- (b) a second input while the first is held cancels ------------------- + +-- key then key: the straggling release of the first press writes nothing +bm.index = ROW_SELECT +press(bm, "a") +bm:onKeyPressed("q") +bm:onKeyPressed("w") +check(bm.capture == nil, "a second key cancels the armed capture") +if bm.onKeyReleased then bm:onKeyReleased("q") end +check(bindings.select == nil, "the cancelled key capture wrote nothing") + +-- key then pad: cancel crosses input kinds too +press(bm, "a") +bm:onKeyPressed("q") +bm:onGamepadPressed("x") +check(bm.capture == nil, "a pad press cancels a held key capture") +if bm.onKeyReleased then bm:onKeyReleased("q") end +if bm.onGamepadReleased then bm:onGamepadReleased("x") end +check(bindings.select == nil, "and still nothing stored") +local writesAfterSwap = game.wroteOptions + +-- ---- (c) commit happens on release, not press ----------------------------- + +press(bm, "a") +bm:onKeyPressed("q") +check(bindings.select == nil, "press alone commits nothing (#589)") +eq(game.wroteOptions, writesAfterSwap, "and touches nothing on disk") +bm:onKeyReleased("q") +eq(bindings.select.key, "q", "the release is the commit") +eq(bm.items[ROW_SELECT].right, "Q/BACK", "the row shows the new key") + +-- ---- (d) SELECT on a row forgets its rebind ------------------------------- + +press(bm, "select") +check(bindings.select == nil, "SELECT drops the row's overlay entry (#589)") +eq(bm.items[ROW_SELECT].right, "TAB/BACK", "the row falls back to the default") +-- the swapped B row clears the same way; a second SELECT on the now +-- default row is a no-op, not a write +local writes = game.wroteOptions +bm.index = ROW_B +press(bm, "select") +eq(game.wroteOptions, writes + 1, "clearing the swapped B row writes once") +check(game.save.options.bindings.b == nil, "and drops its overlay entry") +press(bm, "select") +eq(game.wroteOptions, writes + 1, "clearing an already-default row writes nothing") + +-- ---- (e) START confirms, then drops the whole overlay --------------------- + +-- rebuild a dirty overlay to reset +bm.index = ROW_A +press(bm, "a") +bm:onKeyPressed("p") +bm:onKeyReleased("p") +eq(game.save.options.bindings.a.key, "p", "fixture rebind in place") + +press(bm, "start") +local box = game.stack:top() +check(box ~= bm, "START pushes the confirm box instead of resetting outright") +eq(bm.footer, Strings("RESET ALL BINDINGS?"), + "the footer doubles as the prompt") + +-- the box starts on NO: a bare A press must keep the overlay +press(box, "a") +eq(game.stack:top(), bm, "answering pops the box") +check(game.save.options.bindings ~= nil, "NO keeps the bindings (defaultNo)") +eq(bm.items[ROW_A].right, "P/B", "and the rows keep showing them") + +-- again, flip to YES: the overlay goes away and every row reads default +press(bm, "start") +box = game.stack:top() +press(box, "up") +press(box, "a") +check(game.save.options.bindings == nil, "YES clears options.bindings (#589)") +eq(bm.items[ROW_A].right, "Z/A", "the A row reads its default again") +eq(bm.items[ROW_B].right, "X/B", "so does the B row the swap had touched") + +-- closing after the reset leaves the live map at the defaults +press(bm, "b") +eq(#game.stack.states, 0, "B closes the screen") +eq(Input.keyBindings["z"], "a", "the live map is back to Z = A") +eq(Input.padBindings["b"], "b", "and pad B = B") + +Input:init() +T.finish("rebind_swap_clear_bug589") diff --git a/tests/mod_ui_tests.lua b/tests/mod_ui_tests.lua index acc34e9b..eac4d93c 100644 --- a/tests/mod_ui_tests.lua +++ b/tests/mod_ui_tests.lua @@ -397,11 +397,11 @@ check(getmetatable(bm) == BindingsMenu, check(bm.screenId == "BindingsMenu", "the pushed rebind screen carries its screen id") check(#bm.items == 8, "one row per logical button") -check(bm.items[1].label == "UP" and bm.items[1].right == "UP" - and bm.items[5].label == "A" and bm.items[5].right == "Z" - and bm.items[7].label == "START" and bm.items[7].right == "ESCAPE" +check(bm.items[1].label == "UP" and bm.items[1].right == "UP/D-UP" + and bm.items[5].label == "A" and bm.items[5].right == "Z/A" + and bm.items[7].label == "START" and bm.items[7].right == "ESC/START" and bm.items[8].label == "SELECT" and bm.items[8].right == "TAB/BACK", - "with no rebind the rows mirror the fixed map") + "with no rebind the rows mirror the fixed map, key and pad both (#589)") check(cbGame.save.options.bindings == nil, "opening the screen alone writes nothing") check(bm.onKeyPressed == nil and bm.onGamepadPressed == nil, @@ -412,18 +412,20 @@ check(bm.capture == bm.items[1] and bm.onKeyPressed ~= nil, local wroteOptions = false function cbGame:writeOptions() wroteOptions = true end bm:onKeyPressed("j") +bm:onKeyReleased("j") -- a capture commits on the press's release (#589) check(cbGame.save.options.bindings.up.key == "j", "a captured key lands in options.bindings") -check(bm.items[1].right == "J", "the row shows the new key") +check(bm.items[1].right == "J/D-UP", "the row shows the new key") check(wroteOptions, "a rebind persists through writeOptions") check(bm.capture == nil and bm.onKeyPressed == nil and bm.onGamepadPressed == nil, "the capture disarms after one input") bm.index = 5 press(bm, "a") bm:onGamepadPressed("y") +bm:onGamepadReleased("y") check(cbGame.save.options.bindings.a.pad == "y", "a captured pad button lands beside the key slot") -check(bm.items[5].right == "Z", "a pad rebind keeps the key column") +check(bm.items[5].right == "Z/Y", "a pad rebind keeps the key column") press(bm, "b") check(#cbGame.stack.states == 0, "B closes the rebind screen") diff --git a/tools/save-editor/Kit.lua b/tools/save-editor/Kit.lua index 355c50c1..8b99fc7f 100644 --- a/tools/save-editor/Kit.lua +++ b/tools/save-editor/Kit.lua @@ -29,10 +29,11 @@ local kbField = nil -- id of the field the OS soft keyboard is raised for -- Mobile LOVE only delivers love.textinput while setTextInput(true) is -- active, and that call is what raises the Android/iOS soft keyboard; the -- rect keeps the focused field visible above it. Desktop has text input on --- by default and the launcher hosting this editor depends on that -- nothing --- in src/import/RomImporter.lua (slot rename #205, ROM finder, mod index --- prompt) ever enables it -- so the editor only ever raises there and never --- lowers, since setTextInput is global SDL state, not per-widget (#529). +-- by default and the launcher hosting this editor depends on that -- the +-- launcher's own fields (slot rename #205, mod index prompt, find search) +-- follow the same rule since #578: arm on open, lower only on mobile -- so +-- neither side ever turns desktop text input off, since setTextInput is +-- global SDL state, not per-widget (#529). local function mobile() local osName = love and love.system and love.system.getOS and love.system.getOS() From 6bb2e078c05b774f614f0c3dcd1fef8c256919fd Mon Sep 17 00:00:00 2001 From: bryanthaboi Date: Sat, 1 Aug 2026 14:22:26 -0400 Subject: [PATCH 3/3] Scrub mod manifest strings to valid UTF-8 A manifest whose name, version, description, or category carries invalid UTF-8 (a BOM, Latin-1 bytes) crashed the launcher's MODS panel, since love.graphics.printf raises on invalid UTF-8. Manifest.validate now drops invalid bytes and a leading BOM from those strings, in place so the badge's raw.category read agrees. --- src/mods/Manifest.lua | 50 ++++++++++++++++++++++++++++ tests/engine/launcher_mods_tests.lua | 31 +++++++++++++++++ 2 files changed, 81 insertions(+) diff --git a/src/mods/Manifest.lua b/src/mods/Manifest.lua index d1f1545b..12ff83a7 100644 --- a/src/mods/Manifest.lua +++ b/src/mods/Manifest.lua @@ -86,8 +86,58 @@ local function mergeConflictLists(conflicts, incompatible) return out end +-- Drop bytes that are not valid UTF-8 (malformed sequences, overlongs, +-- surrogates, > U+10FFFF) and a leading BOM. LÖVE's text renderer raises +-- "Invalid UTF-8" from love.graphics.print/printf, so any manifest string a +-- panel may draw must be scrubbed here -- the one place every mod manifest +-- passes through -- or a single mangled description crashes the whole MODS +-- panel instead of misrendering one card. +local function scrubUtf8(s) + if type(s) ~= "string" then return s end + s = s:gsub("^\239\187\191", "") + local out, i, n = {}, 1, #s + while i <= n do + local b = s:byte(i) + local len + if b < 0x80 then len = 1 + elseif b >= 0xC2 and b <= 0xDF then len = 2 + elseif b >= 0xE0 and b <= 0xEF then len = 3 + elseif b >= 0xF0 and b <= 0xF4 then len = 4 + end + local ok = len ~= nil and i + len - 1 <= n + if ok and len > 1 then + for j = i + 1, i + len - 1 do + local c = s:byte(j) + if c < 0x80 or c > 0xBF then ok = false; break end + end + if ok then + -- boundary lead bytes narrow their second byte: no overlongs + -- (E0/F0), no surrogates (ED), nothing past U+10FFFF (F4) + local b2 = s:byte(i + 1) + if (b == 0xE0 and b2 < 0xA0) or (b == 0xED and b2 > 0x9F) + or (b == 0xF0 and b2 < 0x90) or (b == 0xF4 and b2 > 0x8F) then + ok = false + end + end + end + if ok then + out[#out + 1] = s:sub(i, i + len - 1) + i = i + len + else + i = i + 1 + end + end + return table.concat(out) +end + function Manifest.validate(raw, path) assert(type(raw) == "table", "manifest must be an object") + -- scrubbed in place so every later reader agrees, including the launcher's + -- badge derivation, which reads raw.category rather than the validated copy + raw.name = scrubUtf8(raw.name) + raw.version = scrubUtf8(raw.version) + raw.description = scrubUtf8(raw.description) + raw.category = scrubUtf8(raw.category) assert(type(raw.id) == "string" and raw.id:match("^[%w_%-]+$"), "manifest id must contain only letters, numbers, _ or -") assert(type(raw.name) == "string" and raw.name ~= "", "manifest name is required") diff --git a/tests/engine/launcher_mods_tests.lua b/tests/engine/launcher_mods_tests.lua index d2a2a5f0..02118482 100644 --- a/tests/engine/launcher_mods_tests.lua +++ b/tests/engine/launcher_mods_tests.lua @@ -297,4 +297,35 @@ do eq(rows[1].name, "bare", "a nameless row falls back to its id") end +-- ------- manifest strings are scrubbed to valid UTF-8 (MODS panel crash: +-- LÖVE's printf raises "Invalid UTF-8" on a mangled name/description, so +-- validate must drop bad bytes before any panel draws them) + +do + local m = mf({ id = "utf", entry = "m.lua", + -- BOM-prefixed name (a real manifest shipped this way), a Latin-1 e-acute + -- (\233, invalid as UTF-8) in the description, and a lone continuation + -- byte in the version + name = "\239\187\191Run Mode", + version = "1.0\128.0", + description = "caf\233 latt\233", + category = "UI\255" }) + eq(m.name, "Run Mode", "a leading BOM is stripped from the name") + eq(m.version, "1.0.0", "invalid bytes are dropped from the version") + eq(m.description, "caf latt", "Latin-1 bytes are dropped, not replaced") + eq(m.raw.category, "UI", "raw.category is scrubbed in place for the badge") + + local ok2 = mf({ id = "utf2", name = "Vers\195\163oVermelha", version = "1.0.0", + entry = "m.lua", description = "Pok\195\169mon \240\159\148\165" }) + eq(ok2.name, "Vers\195\163oVermelha", "valid two-byte sequences survive") + eq(ok2.description, "Pok\195\169mon \240\159\148\165", + "valid three- and four-byte sequences survive") + + -- surrogate half (ED A0 80) and overlong slash (C0 AF) are invalid even + -- though their lead bytes look plausible + local bad = mf({ id = "utf3", name = "a\237\160\128b\192\175c", + version = "1.0.0", entry = "m.lua" }) + eq(bad.name, "abc", "surrogates and overlongs are dropped") +end + T.finish("launcher_mods")