diff --git a/src/core/SaveData.lua b/src/core/SaveData.lua index d8277c9f..0a92535f 100644 --- a/src/core/SaveData.lua +++ b/src/core/SaveData.lua @@ -109,11 +109,14 @@ local function makePortableFs(dir) } end -local function detectPortable() - if portableChecked then return portableBase end - portableChecked = true - portableBase = false - if not (love and love.filesystem) then return false end +-- Every folder a player might reasonably call "the game folder", best first: +-- the packaged-app container, the folder holding the executable, then the +-- source itself. Portable mode is the case where one of these holds the +-- marker; the list itself is just locations, marker or not, which is also +-- what the mods panel needs to notice a mod dropped beside the game by hand +-- (LauncherMods.strays). Empty on Android/iOS and outside LOVE. +function SaveData.gameFolders() + if not (love and love.filesystem) then return {} end -- Desktop only: portable mode carries the save (and, since issue #74, the -- ROM cache) in the game folder next to the executable/source. On -- Android/iOS the source is a read-only package with no such folder, so @@ -121,7 +124,7 @@ local function detectPortable() if love.system and love.system.getOS then local osName = love.system.getOS() if osName ~= "Windows" and osName ~= "Linux" and osName ~= "OS X" then - return false + return {} end end local src = love.filesystem.getSource and love.filesystem.getSource() @@ -149,15 +152,24 @@ local function detectPortable() -- Order: the packaged-app containing folder (macOS .app / Linux AppImage), -- then the source-base directory (next to a packaged .exe), then the source -- itself (a `love ` run drops portable.txt in the game folder). - -- First one holding the marker wins. Built by appending so a nil (e.g. no - -- .app in the path) never truncates the ipairs scan. + -- Built by appending so a nil (e.g. no .app in the path) never truncates + -- the ipairs scan. local candidates = {} local appDir = appContainer(src) or appContainer(sbd) or appImageContainer() if appDir then candidates[#candidates + 1] = appDir end - if sbd then candidates[#candidates + 1] = sbd end - if src then candidates[#candidates + 1] = src end - for _, base in ipairs(candidates) do - if base ~= "" and pathExists(base .. SEP .. PORTABLE_MARKER) then + if sbd and sbd ~= "" then candidates[#candidates + 1] = sbd end + if src and src ~= "" then candidates[#candidates + 1] = src end + return candidates +end + +-- The game folder carrying portable.txt, or false. First candidate holding +-- the marker wins. +local function detectPortable() + if portableChecked then return portableBase end + portableChecked = true + portableBase = false + for _, base in ipairs(SaveData.gameFolders()) do + if pathExists(base .. SEP .. PORTABLE_MARKER) then portableBase = base break end diff --git a/src/import/CacheFs.lua b/src/import/CacheFs.lua index c85ed8d0..ab69f39a 100644 --- a/src/import/CacheFs.lua +++ b/src/import/CacheFs.lua @@ -126,9 +126,9 @@ local function resolveMount() if okl and lib then local oks, fn = pcall(function() return lib.PHYSFS_mount end) if oks and fn then - physfsMountFn = function(d, append) + physfsMountFn = function(d, mountPoint, append) if append == nil then append = true end - local okr, ret = pcall(fn, d, "", append and 1 or 0) + local okr, ret = pcall(fn, d, mountPoint or "", append and 1 or 0) return okr and ret ~= 0 end break @@ -145,7 +145,7 @@ end local function mountReadable(dir, append) local fn = resolveMount() if not fn then return false end - return fn(dir, append) + return fn(dir, "", append) end -- PHYSFS_unmount, resolved the same way PHYSFS_mount is. Only @@ -385,4 +385,31 @@ function CacheFs.unmountVersion(version) return done end +-- Mount `dir` at `mountPoint` for the length of `fn()`, then take it back off +-- the read path and hand back whatever fn returned. +-- +-- Every other mount here is permanent and lands at the physfs root: this one +-- exists to *look* at a folder the game has deliberately not mounted, which +-- is a different job. The mods panel uses it to read a mods/ folder sitting +-- beside the executable of a non-portable install (LauncherMods.strays). +-- Because it unmounts again, and because a non-empty mountPoint keeps the +-- tree in its own corner of the namespace while it is up, a folder inspected +-- this way can never shadow a game file or change what the running game +-- resolves -- which is what makes it safe to point at a folder whose contents +-- nobody has validated. +-- +-- Returns nil when the mount is unavailable (no ffi, no PHYSFS symbol, or the +-- mount was refused), which callers must treat as "could not look", not as +-- "nothing there". An error inside fn still unmounts before it propagates. +function CacheFs.withMounted(dir, mountPoint, fn) + if not dir or dir == "" then return nil end + local mount, unmount = resolveMount(), resolveUnmount() + if not (mount and unmount) then return nil end + if not mount(dir, mountPoint, true) then return nil end + local ok, res = pcall(fn) + unmount(dir) + if not ok then error(res, 0) end + return res +end + return CacheFs diff --git a/src/import/RomImporter.lua b/src/import/RomImporter.lua index 99618938..b2235b0b 100644 --- a/src/import/RomImporter.lua +++ b/src/import/RomImporter.lua @@ -2697,6 +2697,30 @@ end -- so a still list costs nothing after the first paint. function RomImporter:_refreshMods() local LauncherMods = require("src.mods.LauncherMods") + -- Once per session, ahead of the first listing: pull in any mod the player + -- unzipped beside the executable, which an ordinary (non-portable) install + -- has no way to read. It happens here rather than behind a button because + -- the failure being fixed is one where nothing on screen suggests there is + -- anything to press -- the panel just comes up empty. Guarded so a toggle + -- or a delete does not re-scan; adoptStrays is idempotent regardless. + if not self.modStraysChecked then + self.modStraysChecked = true + local imported, failed = {}, {} + for _, s in ipairs(LauncherMods.adoptStrays() or {}) do + table.insert(s.err and failed or imported, s.id) + end + -- the failure wins the notice: an import that worked speaks for itself in + -- the list right below it, one that did not is the only word they get + if #imported > 0 then + self.modNotice = { ok = true, + text = "Imported from the game folder: " .. table.concat(imported, ", ") } + end + if #failed > 0 then + self.modNotice = { ok = false, + text = "Found beside the game but could not import: " + .. table.concat(failed, ", ") } + end + end self.mods = LauncherMods.list() or {} end diff --git a/src/mods/LauncherMods.lua b/src/mods/LauncherMods.lua index 2e81e0a2..84f35610 100644 --- a/src/mods/LauncherMods.lua +++ b/src/mods/LauncherMods.lua @@ -16,9 +16,19 @@ -- fused build), which is why those mods still loaded while landing in the -- wrong place. -- --- Split in two: the pure derivation (deriveList, locateRoot) has no love and --- no filesystem, so the engine tier can table-drive it; the discovery, --- install, and uninstall paths reach for love.filesystem and SaveData. +-- The same split decides where a mod is FOUND, and that has a sharp edge: a +-- non-portable install never reads the game folder at all, so a mod unzipped +-- next to the executable -- where most games would want it -- is not wrong so +-- much as invisible, with an empty panel and no error to explain it. +-- adoptStrays looks in those folders anyway (a scoped mount that comes down +-- again, CacheFs.withMounted) and copies what it finds into the tree the game +-- really reads, so the mistake costs a line of notice rather than a support +-- thread. +-- +-- Split in two: the pure derivation (deriveList, locateRoot, pickStrays) has +-- no love and no filesystem, so the engine tier can table-drive it; the +-- discovery, install, uninstall, and stray-scan paths reach for +-- love.filesystem and SaveData. local Manifest = require("src.mods.Manifest") local ManagerState = require("src.mods.ManagerState") @@ -136,6 +146,28 @@ function LauncherMods.locateRoot(paths) return nil, "no manifest.json found in the .zip" end +-- pickStrays(found, installed) -> the rows worth adopting, pure. +-- found is an array of { id, name, folder, path } in scan order (game folder +-- order, then directory order); installed is the id -> true set of what the +-- game can already see. An installed id is dropped -- the player has a +-- working copy and the loose folder is just where they first put it -- and a +-- duplicate id across two game folders keeps the first, the same first-wins +-- rule discover() uses. Sorted by id so the notice reads the same every time. +function LauncherMods.pickStrays(found, installed) + installed = installed or {} + local out, seen = {}, {} + for _, row in ipairs(found or {}) do + local id = row.id + if id and not installed[id] and not seen[id] then + seen[id] = true + out[#out + 1] = { id = id, name = row.name or id, + folder = row.folder, path = row.path } + end + end + table.sort(out, function(a, b) return a.id < b.id end) + return out +end + -- ------- discovery (love.filesystem) local function decodeManifest(raw, path) @@ -301,6 +333,109 @@ local function removeTree(path) fs.remove(path) end +-- ------- strays: mods dropped beside the game that it cannot see + +-- love.filesystem looks in two places for "mods/": the save directory, and -- +-- portable installs only -- the game folder, which CacheFs mounts. A player +-- who unzips a mod next to the executable of an ordinary install, which is +-- where very nearly every other game would want it, gets no error and no mod. +-- The MODS panel simply stays empty, and there is nothing on screen to +-- suggest the files are twenty centimetres away in the wrong folder. +-- +-- The scan mounts each game folder at a private mount point just long enough +-- to list mods/ inside it and drops it again (CacheFs.withMounted), so the +-- read path the game actually runs on is never touched and a stray can never +-- shadow a real file. +local STRAY_MOUNT = "stray_scan" + +-- Run fn(mountedModsRoot) for each game folder that has a readable mods/ +-- directory, one mount at a time. Folders that are already the physfs source +-- are skipped: their mods/ is discoverable by definition, so anything there is +-- installed already and not a stray (this is every `love ` dev run). +local function eachStrayRoot(fn) + local SaveData_ = require("src.core.SaveData") + local fs = love and love.filesystem + if not fs then return end + local source = fs.getSource and fs.getSource() + local seen = {} + for _, folder in ipairs(SaveData_.gameFolders() or {}) do + if not seen[folder] and folder ~= source then + seen[folder] = true + CacheFs.withMounted(folder, STRAY_MOUNT, function() + local root = STRAY_MOUNT .. "/mods" + if fs.getInfo(root) then fn(root, folder) end + end) + end + end +end + +-- Every valid mod folder sitting in a game folder's mods/, in scan order. +-- Only reads. The rows carry the mounted path, which is live for the length +-- of the mount and dead after it -- copying has to happen inside the same +-- scan, which is why adoption is a flag here rather than a second pass. +local function findStrays(fs, adopt, installed) + local found, adopted = {}, {} + eachStrayRoot(function(root, folder) + local batch = {} + for _, name in ipairs(fs.getDirectoryItems(root)) do + local path = root .. "/" .. name + local info = fs.getInfo(path) + if info and info.type == "directory" then + local raw = fs.read(path .. "/manifest.json") + local manifest = raw and decodeManifest(raw, path) + if manifest then + batch[#batch + 1] = { id = manifest.id, + name = manifest.name or manifest.id, + folder = folder, path = path } + end + end + end + -- filtered per mount, so a copy only ever runs for a row that survived + -- the pure rules -- and so the second game folder sees the first one's + -- ids as taken + for _, row in ipairs(LauncherMods.pickStrays(batch, installed)) do + if adopt then + -- same root pin installZip uses: the mods tree is shared by Red and + -- Blue, never version-prefixed (#330) + local savedPrefix = CacheFs.prefix + CacheFs.prefix = "" + local dest = "mods/" .. row.id + local copied, copyErr = copyTree(row.path, dest) + if not copied then removeTree(dest) end + CacheFs.prefix = savedPrefix + if not copied then row.err = copyErr or "could not copy the files" end + end + installed[row.id] = true + row.path = nil -- dead once this mount comes down + adopted[#adopted + 1] = row + found[#found + 1] = row + end + end) + return LauncherMods.pickStrays(found, {}) +end + +-- The strays, optionally adopted. A folder whose id the game can already see +-- is left out: the player has a working copy, and the loose one is just where +-- they first put it. Rows that failed to copy come back with .err set. +local function scanStrays(adopt) + local fs = love and love.filesystem + if not fs then return {} end + local installed = {} + for _, m in ipairs(discover()) do installed[m.id] = true end + return findStrays(fs, adopt, installed) +end + +-- strays() -> the rows, nothing copied. +function LauncherMods.strays() return scanStrays(false) end + +-- adoptStrays() -> the rows, each one copied into the mods tree the game +-- really reads (rows carrying .err failed). Idempotent: a second call finds +-- the ids installed and returns nothing, so the panel can run this on every +-- open without duplicating anything or nagging twice. The loose folder is +-- deliberately left where it is -- deleting files outside the save directory +-- on the player's behalf is not this function's call to make. +function LauncherMods.adoptStrays() return scanStrays(true) end + -- installZip(source) -> true, id | nil, errString -- source is an external path or a love DroppedFile. The archive is validated -- BEFORE anything is copied; every path unmounts and clears the staged temp diff --git a/tests/engine/launcher_mods_tests.lua b/tests/engine/launcher_mods_tests.lua index 9329944b..2f4044c5 100644 --- a/tests/engine/launcher_mods_tests.lua +++ b/tests/engine/launcher_mods_tests.lua @@ -230,4 +230,52 @@ do "the copy uses the literal picked path") end +-- ------- pickStrays: which mods dropped beside the game are worth adopting + +do + -- the case this exists for: a player unzipped a mod next to the executable + -- of a non-portable install, where the game has no way to read it + local rows = LauncherMods.pickStrays({ + { id = "b_mod", name = "B", folder = "/game", path = "m/b_mod" }, + { id = "a_mod", name = "A", folder = "/game", path = "m/a_mod" }, + }, {}) + eq(#rows, 2, "an uninstalled stray is worth adopting") + eq(rows[1].id, "a_mod", "rows come back sorted by id") + eq(rows[2].id, "b_mod", "both of them") + eq(rows[1].path, "m/a_mod", "carrying the path the copy reads from") + eq(rows[1].folder, "/game", "and the folder it was found in, for the notice") +end + +do + -- already installed: the player has a working copy and the loose folder is + -- just where they first put it. Silence is right -- adopting would make a + -- second copy, and warning would nag on every open. + local rows = LauncherMods.pickStrays({ + { id = "have", name = "Have" }, + { id = "want", name = "Want" }, + }, { have = true }) + eq(#rows, 1, "a stray the game can already see is not a stray") + eq(rows[1].id, "want", "only the one it cannot see is adopted") +end + +do + -- two game folders can both hold the same id (a launcher install plus an + -- older manual one). First wins, matching discover()'s duplicate rule. + local rows = LauncherMods.pickStrays({ + { id = "dup", name = "First", folder = "/a" }, + { id = "dup", name = "Second", folder = "/b" }, + }, {}) + eq(#rows, 1, "a duplicate id across two game folders is adopted once") + eq(rows[1].name, "First", "and the first one found wins") +end + +do + eq(#LauncherMods.pickStrays({}, {}), 0, "no candidates, nothing to adopt") + eq(#LauncherMods.pickStrays(nil, nil), 0, "and nil is not an error") + eq(#LauncherMods.pickStrays({ { name = "no id" } }, {}), 0, + "a row with no id is dropped rather than crashing the panel") + local rows = LauncherMods.pickStrays({ { id = "bare" } }, {}) + eq(rows[1].name, "bare", "a nameless row falls back to its id") +end + T.finish("launcher_mods")