From 74e747434903b9582a58c90a94942735a5300d6f Mon Sep 17 00:00:00 2001 From: Andrew Quenehen Date: Sat, 1 Aug 2026 05:58:17 -0300 Subject: [PATCH] fix(nx-mods): skip ROM AppleDouble sidecars and surface mixed failures Ignore hidden ._*.gb in the ROM inbox like zips, keep mixed-rescan success notices while appending sibling errors, and cover FileData mount / PK rejection in unit tests. Co-authored-by: Cursor --- docs/switch-development.md | 2 +- src/import/RomImporter.lua | 30 +++- tests/launcher_mods_install_zip_test.lua | 197 ++++++++++++++++++++++ tests/rom_importer_nx_mods_inbox_test.lua | 23 ++- tests/run_tests.lua | 1 + 5 files changed, 243 insertions(+), 10 deletions(-) create mode 100644 tests/launcher_mods_install_zip_test.lua diff --git a/docs/switch-development.md b/docs/switch-development.md index b1a04d2c..d211a335 100644 --- a/docs/switch-development.md +++ b/docs/switch-development.md @@ -245,7 +245,7 @@ Community mods install from a **separate** MTP inbox (not mixed into the ROM `im Do **not** commit third-party mod zip bytes into git. Drop the zip over MTP, rescan, enable in MODS, then Play. -**MTP tip (Mac):** OpenMTP/Finder often creates AppleDouble sidecars named `._Something.zip`. Those are not real archives — the launcher ignores hidden `.*` names. If install still fails with “could not be opened” / “not a zip file”, delete any `._*.zip` under `imports/mods/` and confirm the real zip starts with the `PK` magic (re-copy the release asset if unsure). +**MTP tip (Mac):** OpenMTP/Finder often creates AppleDouble sidecars named `._Something.zip` / `._cart.gb`. Those are not real archives or ROMs — the launcher ignores hidden `.*` names under both `imports/` and `imports/mods/`. If install still fails with “could not be opened” / “not a zip file”, delete any `._*` under the inbox and confirm the real zip starts with the `PK` magic (re-copy the release asset if unsure). **Example zip source:** [DramaticShape VoxelMod releases](https://github.com/DramaticShape/DramaticShapeVoxelMod/releases) — download a release `.zip`, copy into `imports/mods/`, rescan, enable. diff --git a/src/import/RomImporter.lua b/src/import/RomImporter.lua index 1f4ae426..64d9cc48 100644 --- a/src/import/RomImporter.lua +++ b/src/import/RomImporter.lua @@ -388,11 +388,15 @@ end local function listRomPaths(dir) local paths = {} - for _, name in ipairs(love.filesystem.getDirectoryItems(dir)) do - local path = (dir == "" or dir == "/") and name or (dir .. "/" .. name) - if name:lower():match("%.gbc?$") - and love.filesystem.getInfo(path, "file") then - paths[#paths + 1] = path + for _, name in ipairs(love.filesystem.getDirectoryItems(dir) or {}) do + -- Skip AppleDouble / hidden junk from Mac MTP (._cart.gb ends in .gb + -- but is not a ROM — rescan would try it first and block the real dump). + if name:sub(1, 1) ~= "." then + local path = (dir == "" or dir == "/") and name or (dir .. "/" .. name) + if name:lower():match("%.gbc?$") + and love.filesystem.getInfo(path, "file") then + paths[#paths + 1] = path + end end end return paths @@ -447,6 +451,7 @@ function RomImporter:rescanModsAction() local anyOk = false local lastOk = nil local lastFail = nil + local failCount = 0 for _, path in ipairs(candidates) do -- Reuse _installMod carefully: it must not remove the inbox source. self:_installMod(path) @@ -454,12 +459,21 @@ function RomImporter:rescanModsAction() anyOk = true lastOk = self.modNotice else + failCount = failCount + 1 lastFail = self.modNotice end end - -- Prefer success when at least one zip installed (a leftover MTP - -- AppleDouble / corrupt sibling must not hide a good install). - if anyOk then + -- Success wins overall ok=true so a leftover MTP junk sibling cannot hide + -- a good install; still append the last failure so a real broken zip is + -- visible beside the success line. + if anyOk and lastFail then + local okText = (lastOk and lastOk.text) or "Installed" + local failText = (lastFail and lastFail.text) or "unknown error" + self.modNotice = { + ok = true, + text = Strings("%s\n(%d failed: %s)", okText, failCount, failText), + } + elseif anyOk then self.modNotice = lastOk elseif lastFail then self.modNotice = lastFail diff --git a/tests/launcher_mods_install_zip_test.lua b/tests/launcher_mods_install_zip_test.lua new file mode 100644 index 00000000..0fb9d1d0 --- /dev/null +++ b/tests/launcher_mods_install_zip_test.lua @@ -0,0 +1,197 @@ +-- LauncherMods.installZip: PK gate, FileData mount preference, path fallback. +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("launcher mods installZip mount") +local eq = S.eq +local check = S.check + +local MOD_ID = "mount_probe" +local ARCHIVE = { + [MOD_ID .. "/manifest.json"] = + ('{"id":"%s","name":"Mount Probe","version":"1.0.0","entry":"main.lua"}') + :format(MOD_ID), + [MOD_ID .. "/main.lua"] = "return function() end\n", +} + +local files, dirs, arch = {}, {}, {} +local fileDataMounts, pathMounts, stagedTemps = 0, 0, {} +local stagedEver = false + +local function resetFs() + for k in pairs(files) do files[k] = nil end + for k in pairs(dirs) do dirs[k] = nil end + for k in pairs(arch) do arch[k] = nil end + fileDataMounts, pathMounts = 0, 0 + stagedTemps = {} + stagedEver = false +end + +local function dirChild(key, name) + if name == nil or name == "" then return key:match("^[^/]+") end + local prefix = name .. "/" + if key:sub(1, #prefix) ~= prefix then return nil end + return key:sub(#prefix + 1):match("^[^/]+") +end + +local function mapInfo(map, name, kind) + if map[name] ~= nil then return { type = kind or "file" } end + for key in pairs(map) do + if dirChild(key, name) then return { type = "directory" } end + end + return nil +end + +local vfs = {} + +function vfs.write(name, data) + files[name] = data + if name:match("^mod_import_") then + stagedTemps[name] = true + stagedEver = true + end + return true +end + +function vfs.read(name) + if arch[name] ~= nil then return arch[name] end + return files[name] +end + +function vfs.remove(name) + files[name] = nil + dirs[name] = nil + stagedTemps[name] = nil + return true +end + +function vfs.createDirectory(name) + dirs[name] = true + return true +end + +function vfs.getInfo(name, kind) + local info = mapInfo(arch, name) + or mapInfo(files, name) + or mapInfo(dirs, name, "directory") + if info and kind and info.type ~= kind then return nil end + return info +end + +function vfs.getDirectoryItems(name) + local seen, items = {}, {} + local function add(child) + if child and not seen[child] then + seen[child] = true + items[#items + 1] = child + end + end + for key in pairs(arch) do add(dirChild(key, name)) end + for key in pairs(files) do add(dirChild(key, name)) end + for key in pairs(dirs) do add(dirChild(key, name)) end + table.sort(items) + return items +end + +function vfs.mount(archive, point) + if type(archive) == "table" and archive.__filedata then + fileDataMounts = fileDataMounts + 1 + else + pathMounts = pathMounts + 1 + end + for rel, body in pairs(ARCHIVE) do + arch[point .. "/" .. rel] = body + end + return true +end + +function vfs.unmount() + for k in pairs(arch) do arch[k] = nil end + return true +end + +function vfs.newFileData(data, name) + return { __filedata = true, data = data, name = name } +end + +function vfs.getSaveDirectory() + return "/tmp/pokeport-install-zip-test" +end + +function vfs.getSource() + return nil +end + +local savedFs = love.filesystem +local savedCacheFs = package.loaded["src.import.CacheFs"] +local savedLauncherMods = package.loaded["src.mods.LauncherMods"] +local savedSaveDataPortable = nil + +local SaveData = require("src.core.SaveData") +savedSaveDataPortable = SaveData.portableBaseDir + +local function freshMods() + package.loaded["src.import.CacheFs"] = nil + package.loaded["src.mods.LauncherMods"] = nil + SaveData.portableBaseDir = function() return nil end + return require("src.mods.LauncherMods") +end + +love.filesystem = vfs +local LauncherMods = freshMods() + +-- Reject non-PK / empty / AppleDouble-shaped bytes before mount +resetFs() +files["imports/mods/junk.zip"] = "\0\5\22\7AppleDouble" +local ok, err = LauncherMods.installZip("imports/mods/junk.zip") +check(not ok, "non-PK bytes are rejected") +check(tostring(err):find("not a zip file", 1, true), + "rejection names not a zip file") +eq(fileDataMounts + pathMounts, 0, "invalid zip never mounts") + +resetFs() +files["imports/mods/empty.zip"] = "" +ok, err = LauncherMods.installZip("imports/mods/empty.zip") +check(not ok, "empty file is rejected") +check(tostring(err):find("not a zip file", 1, true), + "empty rejection is not a zip file") + +-- Prefer FileData / in-memory mount for relative save-dir zips +resetFs() +files["imports/mods/good.zip"] = "PK\3\4relative-inbox" +ok, err = LauncherMods.installZip("imports/mods/good.zip") +check(ok == true, "PK zip installs via relative love.filesystem path (" + .. tostring(err) .. ")") +eq(err, MOD_ID, "install reports manifest id") +eq(fileDataMounts, 1, "relative zip prefers FileData mount") +eq(pathMounts, 0, "relative zip does not fall back to path mount when FileData works") +local staged = 0 +for _ in pairs(stagedTemps) do staged = staged + 1 end +eq(staged, 0, "FileData path leaves no staged temp zip") +check(files["mods/" .. MOD_ID .. "/manifest.json"] ~= nil, + "install wrote manifest into mods/") + +-- Fallback: no newFileData → stage temp + path mount +resetFs() +vfs.newFileData = nil +package.loaded["src.mods.LauncherMods"] = nil +package.loaded["src.import.CacheFs"] = nil +LauncherMods = freshMods() +files["imports/mods/fallback.zip"] = "PK\3\4fallback" +ok, err = LauncherMods.installZip("imports/mods/fallback.zip") +check(ok == true, "install still works without newFileData (" + .. tostring(err) .. ")") +eq(fileDataMounts, 0, "no FileData mounts when API absent") +eq(pathMounts, 1, "falls back to path mount") +check(stagedEver, "fallback stages a mod_import_*.zip temp") +local leftover = 0 +for _ in pairs(stagedTemps) do leftover = leftover + 1 end +eq(leftover, 0, "fallback cleans staged temp after install") + +-- Restore +love.filesystem = savedFs +SaveData.portableBaseDir = savedSaveDataPortable +package.loaded["src.import.CacheFs"] = savedCacheFs +package.loaded["src.mods.LauncherMods"] = savedLauncherMods + +S.finish() diff --git a/tests/rom_importer_nx_mods_inbox_test.lua b/tests/rom_importer_nx_mods_inbox_test.lua index b7b429e9..283e0194 100644 --- a/tests/rom_importer_nx_mods_inbox_test.lua +++ b/tests/rom_importer_nx_mods_inbox_test.lua @@ -197,7 +197,11 @@ check(not removed["imports/mods/a-bad.zip"], "mixed: bad zip retained") check(not removed["imports/mods/b-good.zip"], "mixed: good zip retained") check(love.filesystem.read("imports/mods/a-bad.zip") ~= nil, "mixed bad still present") check(love.filesystem.read("imports/mods/b-good.zip") ~= nil, "mixed good still present") -check(ri.modNotice and ri.modNotice.ok, "mixed prefers success notice over sibling fail") +check(ri.modNotice and ri.modNotice.ok, "mixed keeps overall success when one zip installs") +check(ri.modNotice.text:find("failed", 1, true), + "mixed success notice still surfaces sibling failure") +check(ri.modNotice.text:find("no manifest", 1, true), + "mixed success notice includes the failure reason") -- Mac MTP AppleDouble (._*.zip) must not be install candidates ri = freshImporter() @@ -210,6 +214,23 @@ eq(#installCalls, 1, "AppleDouble ._*.zip is skipped") eq(installCalls[1], "imports/mods/DRAMATIC_SHAPE-1.4.0.zip", "only the real zip is installed") check(ri.modNotice and ri.modNotice.ok, "AppleDouble skip still shows install success") +check(not (ri.modNotice.text or ""):find("failed", 1, true), + "AppleDouble-only sibling does not invent a mixed failure line") + +-- Mac MTP AppleDouble ROM sidecar must not be ROM inbox candidates +ri = freshImporter() +love.filesystem.write("imports/._cart.gb", string.rep("X", 16)) +love.filesystem.write("imports/cart.gb", string.rep("G", 16)) +roms = ri:scanInbox(ri.ready) +local sawHidden, sawReal = false, false +for _, path in ipairs(roms) do + if path:find("._cart", 1, true) then sawHidden = true end + if path == "imports/cart.gb" then sawReal = true end +end +check(not sawHidden, "ROM scanInbox skips AppleDouble ._*.gb") +check(sawReal, "ROM scanInbox still finds the real .gb") +love.filesystem.remove("imports/._cart.gb") +love.filesystem.remove("imports/cart.gb") -- NXMOD-05: chooseMod on NX routes to inbox rescan; no HostShell/chooseZip local hostShellCalls = 0 diff --git a/tests/run_tests.lua b/tests/run_tests.lua index d8a85a42..a907ddef 100644 --- a/tests/run_tests.lua +++ b/tests/run_tests.lua @@ -3376,6 +3376,7 @@ runSuites({ "tests/platform_nx_network_gate_test.lua" }) runSuites({ "tests/rom_importer_nx_flags_test.lua" }) runSuites({ "tests/rom_importer_nx_inbox_test.lua" }) runSuites({ "tests/rom_importer_nx_mods_inbox_test.lua" }) +runSuites({ "tests/launcher_mods_install_zip_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