From bd92b6cb342ee862ddcf87e26bb19e2d51fc4d8e Mon Sep 17 00:00:00 2001 From: Shane McGovern Date: Sat, 19 Sep 2026 23:14:59 +0100 Subject: [PATCH] feat(modkit): add gen3check for FireRed mods There was no way to ask "will this mod load on FireRed?". `validate`, `lint` and `pack` only reason about Gen 1, and `gen2check` refuses a Gen 3 mod at the manifest gate. `gen3check` is the Gen 3 counterpart of `gen2check`: python3 tools/modkit.py gen3check mods/ [--notes] [--json] [--strict] - Factor the shared compat analysis behind a `Generation` descriptor, with `GEN2`/`GEN3` module-level singletons holding everything that differs: the compat facade name, the generation's own directory (`gen2` vs `game3`), the legacy manifest flag, the screen-twin prefix, and the sibling spelling. Every compat helper now takes a trailing `gen`. - MK400-MK410 stay shared by design -- they are the same questions asked of a different facade -- and the verdict line (`on gen 3: ...`) disambiguates. - Gen 3 sibling spelling: `src/ui/game3/` is snake_case while `src/world/game3/WorldAPI.lua` is not, so MK403 tries both spellings and stops at the first sibling that exists. Only 8 Gen 1 modules are reachable this way (`BattleAPI`, `BagMenu`, `HallOfFame`, `IntroMovie`, `OptionRows`, `ShopMenu`, `SummaryMenu`, `TrainerCard`), so MK403 on Gen 3 is live but narrow. - MK409's screen-twin half is switched off on Gen 3 -- there is no `Screens.GEN2_IDS` equivalent -- while its version-string half still runs, since a Gen 1 version id in a mod that declares only FireRed is a real bug. - Rename `GEN2_IDS_DUMP` to `VERSION_IDS_DUMP`, now parameterized by generation. Gen 2 behaviour is unchanged: `diff -r` of `gen2check --notes` and `gen2check --notes --json` output over all 10 shipped mods, captured before and after this change, is empty. Tests: `tests/modkit/cases/gen3check.lua` grows from 504 to 542 checks. Every fixture is derived from the engine at run time -- the coverage table, `GEN1_ONLY_MODULES`, and the `game3/` directory listings -- so the suite cannot drift away from the tables it is asserting against. It covers MK400's Gen 3 wording, MK402 naming `Gen3Compat` and never `Gen2Compat`, MK403 naming the snake_case path, MK404 quoting the member, MK409's version-string half, MK410 counted once at file scope, the `--json` verdict shape, and that no adapted module is ever MK402. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- tests/modkit/cases/gen3check.lua | 368 ++++++++++++++++++++++++++++++ tools/modkit.py | 378 +++++++++++++++++++------------ 2 files changed, 601 insertions(+), 145 deletions(-) diff --git a/tests/modkit/cases/gen3check.lua b/tests/modkit/cases/gen3check.lua index eec0721f..3e774892 100644 --- a/tests/modkit/cases/gen3check.lua +++ b/tests/modkit/cases/gen3check.lua @@ -70,4 +70,372 @@ local worldRow = Gen3Compat.coverage("src.world.WorldAPI") T.eq(worldRow and worldRow.target, "src.world.game3.WorldAPI", "the WorldAPI coverage row names the FireRed module") +-- ------- the command itself. `gen3check` is the same checks as `gen2check` +-- run against src/mods/Gen3Compat.lua, so what this section has to prove is +-- that the generation reached every one of them and that the one rule which +-- only makes sense on Gold was switched off. Every fixture is derived from +-- the engine, for the reason this file's header gives. + +local isWindows = package.config:sub(1, 1) == "\\" + +-- luajit's pclose drops the exit status, so the shell reports it in-band +-- (tests/modkit_tests.lua uses the same shape) +local function run(command) + if isWindows then + command = 'cmd /v:on /c "' .. command .. ' 2>&1 & echo EXIT:!errorlevel!"' + else + command = command .. ' 2>&1; echo "EXIT:$?"' + end + local pipe = io.popen(command) + local output = pipe:read("*a") + pipe:close() + return output, tonumber(output:match("EXIT:(%d+)%s*$")) or -1 +end + +local python = isWindows and "python" or "python3" +if not run(python .. " --version"):find("Python 3", 1, true) then + T.check(true, "python3 is absent: the gen3check command is not exercised") + T.finish("gen3check") + return +end + +local function gen3check(dir, extra) + return run(("%s tools/modkit.py gen3check %q %s") + :format(python, dir, extra or "")) +end + +-- ------- what to write the fixtures against, taken from the engine + +local names = {} +for name in pairs(Gen3Compat.ADAPTERS) do names[#names + 1] = name end +table.sort(names) + +-- written onto the running game and never onto the module table, which is +-- what MK410 is about; read from the Gen 1 source, as the tool does +local function instanceField(module, member) + local handle = io.open(module:gsub("%.", "/") .. ".lua", "r") + if not handle then return false end + local body = handle:read("*a") + handle:close() + if not body:find("self%." .. member .. "%s*=[^=]") then return false end + if body:find("function%s+%w+[%.:]" .. member .. "%s*%(") then return false end + for owner in body:gmatch("[\n%s]([%a_][%w_]*)%." .. member .. "%s*=[^=]") do + if owner ~= "self" then return false end + end + return true +end + +local aliasName, backedMember, absentName, absentMember, liveName, liveMember +for _, name in ipairs(names) do + local row = Gen3Compat.coverage and Gen3Compat.coverage(name) + local members = row and row.members or {} + local sorted = {} + for member in pairs(members) do sorted[#sorted + 1] = member end + table.sort(sorted) + for _, member in ipairs(sorted) do + if not member:find("[%.%s]") then + if members[member] == "absent" and not absentMember then + absentName, absentMember = name, member + end + if members[member] == "backed" and row.kind == "alias" + and not backedMember then + aliasName, backedMember = name, member + end + if members[member] == "backed" and row.kind == "facade" + and not liveMember and instanceField(name, member) then + liveName, liveMember = name, member + end + end + end +end +T.check(absentMember ~= nil, "the FireRed coverage table names at least one " + .. "absent member") + +-- the Gen 1 modules a FireRed boot never instantiates (src/mods/Loader.lua), +-- at least one of which the coverage table refuses to adapt: MK402 +local unservedName +do + local handle = io.open("src/mods/Loader.lua", "r") + local body = handle and handle:read("*a") or "" + if handle then handle:close() end + local block = body:match("GEN1_ONLY_MODULES%s*=%s*{(.-)\n}") + for name in (block or ""):gmatch('%["([^"]+)"%]') do + if not Gen3Compat.serves(name) then + unservedName = name + break + end + end +end +T.check(unservedName ~= nil, + "at least one Gen 1 module has no FireRed adapter, which is MK402") + +-- the shape only FireRed has: a module a FireRed boot really runs from a +-- snake_case file under a game3/ directory, on a Gen 1 module nothing adapts, +-- so the sibling search has to try its second spelling. The directory comes +-- from a coverage target, so nothing here restates the tree. +local function listLua(dir) + local files = {} + local pipe = io.popen(isWindows and ('dir /b "%s\\*.lua" 2>nul'):format(dir) + or ('ls "%s"/*.lua 2>/dev/null'):format(dir)) + if not pipe then return files end + for line in pipe:lines() do + local base = line:match("([^/\\]+)%.lua$") + if base and base ~= "init" then files[#files + 1] = base end + end + pipe:close() + table.sort(files) + return files +end + +local siblingName, siblingPath, siblingCamel +for _, name in ipairs(Gen3Compat.modules()) do + if not siblingName then + local row = Gen3Compat.coverage(name) + local dir = row and row.target and row.target:match("^(.*%.)") + if dir and dir:find("%.game3%.$") then + local gen1 = dir:gsub("%.game3%.$", ".") + local gen1dir = gen1:gsub("%.", "/") + for _, file in ipairs(listLua(dir:gsub("%.$", ""):gsub("%.", "/"))) do + local camel = file:gsub("_(%a)", string.upper):gsub("^%l", string.upper) + local handle = io.open(gen1dir .. camel .. ".lua", "r") + if handle then handle:close() end + if handle and camel ~= file and not Gen3Compat.serves(gen1 .. camel) + and not siblingName then + siblingName = gen1 .. camel -- src.ui.BagMenu, what a mod requires + siblingPath = dir .. file -- src.ui.game3.bag_menu, what runs + siblingCamel = dir .. camel -- src.ui.game3.BagMenu, not on disk + end + end + end + end +end +T.check(siblingName ~= nil, + "a snake_case game3 sibling with no adapter exists, which is MK403") + +-- ------- fixtures on disk, because the tool reads a mod directory + +local tmp = os.tmpname() +os.remove(tmp) +local root = (isWindows and tmp:gsub("\\", "/") or tmp) .. "_gen3check" +run((isWindows and "mkdir " or "mkdir -p ") .. ("%q"):format(root)) + +local function write(dir, files) + run((isWindows and "mkdir " or "mkdir -p ") + .. ("%q"):format(root .. "/" .. dir)) + for name, body in pairs(files) do + local handle = assert(io.open(root .. "/" .. dir .. "/" .. name, "w")) + handle:write(body) + handle:close() + end + return root .. "/" .. dir +end + +local function manifest(id, extra) + return ('{ "id": "%s", "name": "%s", "version": "1.0.0", "api": 2, ' + .. '"entry": "main.lua", "description": "gen3check fixture"%s }') + :format(id, id, extra or "") +end + +local FIRERED = ', "games": ["firered"]' + +-- claims FireRed and nothing else, so gen2check has to fail it while +-- gen3check passes it: the same checks against two generations +local firered = write("gen3_firered", { + ["manifest.json"] = manifest("gen3_firered", FIRERED), + ["main.lua"] = "local mod = ...\n", +}) + +-- claims no generation at all +local unclaimed = write("gen3_unclaimed", { + ["manifest.json"] = manifest("gen3_unclaimed"), + ["main.lua"] = "local mod = ...\n", +}) + +-- only reads what the FireRed adapter backs +local clean = aliasName and write("gen3_clean", { + ["manifest.json"] = manifest("gen3_clean", FIRERED), + ["main.lua"] = ([[ +local mod = ... +local M = require("%s") +local held = M.%s +mod.exports.held = held ~= nil +]]):format(aliasName, backedMember), +}) + +-- requires a module a FireRed boot never instantiates and nothing adapts +local unserved = write("gen3_unserved", { + ["manifest.json"] = manifest("gen3_unserved", FIRERED), + ["main.lua"] = ([[ +local mod = ... +local M = require("%s") +M.thing(mod) +]]):format(unservedName), +}) + +-- requires a module a FireRed boot runs out of another file entirely +local sibling = siblingName and write("gen3_sibling", { + ["manifest.json"] = manifest("gen3_sibling", FIRERED), + ["main.lua"] = ([[ +local mod = ... +local M = require("%s") +M.open(mod) +]]):format(siblingName), +}) + +-- calls a member the FireRed coverage table refuses to invent +local absent = write("gen3_absent", { + ["manifest.json"] = manifest("gen3_absent", FIRERED), + ["main.lua"] = ([[ +local mod = ... +local M = require("%s") +M.%s(mod) +]]):format(absentName, absentMember), +}) + +-- an entry chunk holding a member of a game that is not up yet +local held = liveMember and write("gen3_held", { + ["manifest.json"] = manifest("gen3_held", FIRERED), + ["main.lua"] = ([[ +local mod = ... +local G = require("%s") +local captured = G.%s +mod.events:on("game.ready", function() + mod.exports.live = G.%s ~= nil +end) +mod.exports.captured = captured ~= nil +]]):format(liveName, liveMember, liveMember), +}) + +-- the Gen 2 screen-twin half of MK409 is off on FireRed, so the version +-- string is the only MK409 a FireRed boot can raise +local version = write("gen3_version", { + ["manifest.json"] = manifest("gen3_version", FIRERED), + ["main.lua"] = ([[ +local mod = ... +if mod.game.version == "red" then mod.exports.gen1 = true end +]]), +}) + +-- ------- a mod claiming FireRed, run through both commands + +local out, code = gen3check(firered) +T.eq(code, 0, "a mod claiming FireRed exits 0 from gen3check: " .. out) +T.check(out:find("will load", 1, true) ~= nil, + "and its verdict is 'will load': " .. out) +T.check(out:find("on gen 3", 1, true) ~= nil, + "and the verdict line names the generation it checked: " .. out) +T.check(out:find("MK400", 1, true) == nil, + "a FireRed mod is never told it claims no FireRed game: " .. out) + +out, code = run(("%s tools/modkit.py gen2check %q"):format(python, firered)) +T.eq(code, 1, "the same mod fails gen2check, which is the point: " .. out) +T.check(out:find("MK400", 1, true) ~= nil, + "on the manifest gate: " .. out) +T.check(out:find("on gen 2", 1, true) ~= nil, + "with a verdict naming Gen 2: " .. out) + +-- ------- the manifest gate, in FireRed's words + +out, code = gen3check(unclaimed) +T.eq(code, 1, "a mod claiming no Gen 3 game fails the check") +T.check(out:find("MK400", 1, true) ~= nil, "MK400 names the manifest: " .. out) +T.check(out:find("Gen 3", 1, true) ~= nil, + "and the message is the FireRed one, not Gold's: " .. out) +T.check(out:find("will not work", 1, true) ~= nil, + "and the verdict says so: " .. out) + +-- ------- a mod that only reads what the adapter backs + +if clean then + out, code = gen3check(clean) + T.eq(code, 0, "a mod inside the FireRed adapter's coverage exits 0: " .. out) + T.check(out:find("MK40", 1, true) == nil, + "and raises nothing: " .. out) +end + +-- ------- a module FireRed never instantiates and nothing adapts + +out, code = gen3check(unserved) +T.eq(code, 1, "requiring it fails the check: " .. out) +T.check(out:find("MK402", 1, true) ~= nil, "MK402 names it: " .. out) +T.check(out:find(unservedName, 1, true) ~= nil, + "and quotes the module by name: " .. out) +T.check(out:find("Gen3Compat", 1, true) ~= nil, + "and points at the FireRed adapter file: " .. out) +T.check(out:find("Gen2Compat", 1, true) == nil, + "with no mention of the Gold one: " .. out) + +-- ------- a module FireRed runs, from a file with another spelling + +if sibling then + out, code = gen3check(sibling) + T.eq(code, 0, "MK403 is a warning, so it does not fail the gate: " .. out) + T.check(out:find("MK403", 1, true) ~= nil, + "a module FireRed runs from another file is MK403: " .. out) + T.check(out:find(siblingPath, 1, true) ~= nil, + "and the sibling it names is the snake_case one: " .. out) + T.check(out:find(siblingCamel, 1, true) == nil, + "never the CamelCase spelling that is not on disk: " .. out) +end + +-- ------- a member the coverage table refuses to invent + +out, code = gen3check(absent) +T.eq(code, 1, "calling an unbacked member fails the check: " .. out) +T.check(out:find("MK404", 1, true) ~= nil, "MK404 names the member: " .. out) +T.check(out:find(absentMember, 1, true) ~= nil, + "and quotes it by name: " .. out) +T.check(out:find("Gen 3 backing", 1, true) ~= nil, + "and says which generation has no backing for it: " .. out) + +-- ------- the entry chunk holding a member of a game that is not up yet + +if held then + out = gen3check(held) + T.check(out:find("MK410", 1, true) ~= nil, + "MK410 names the file-scope read: " .. out) + T.check(select(2, out:gsub("MK410", "")) == 1, + "and only the file-scope one, not the read inside the handler: " .. out) +end + +-- ------- MK409's screen half is a Gen 2 fact and is off here + +out = gen3check(version) +T.check(out:find("MK409", 1, true) ~= nil, + "a Gen 1 version string is MK409 on FireRed too: " .. out) +T.check(out:find("a Gen 3 game by construction", 1, true) ~= nil, + "and the message is the FireRed one: " .. out) + +-- ------- the machine-readable form one CI step reads + +out, code = run(("%s tools/modkit.py --json gen3check %q"):format(python, + firered)) +T.eq(code, 0, "the JSON run of a passing mod exits 0") +T.check(out:find('"verdict": "will load"', 1, true) ~= nil, + "the JSON carries a verdict per mod: " .. out) +T.check(out:find('"ok": true', 1, true) ~= nil, + "and one ok for the batch: " .. out) + +out, code = run(("%s tools/modkit.py --json gen3check %q %q") + :format(python, firered, unclaimed)) +T.eq(code, 1, "the batch fails when any mod in it fails") +T.check(out:find('"ok": false', 1, true) ~= nil, + "and the batch ok follows: " .. out) + +-- ------- every adapted name is served, so requiring one is never MK402 + +local requires = { "local mod = ..." } +for _, name in ipairs(names) do + requires[#requires + 1] = ("require(%q)"):format(name) +end +local served = write("gen3_served", { + ["manifest.json"] = manifest("gen3_served", FIRERED), + ["main.lua"] = table.concat(requires, "\n") .. "\n", +}) +out = gen3check(served) +T.check(out:find("MK402", 1, true) == nil, + "no adapted module is reported as unserved: " .. out) + +run((isWindows and "rmdir /s /q " or "rm -rf ") .. ("%q"):format(root)) + T.finish("gen3check") diff --git a/tools/modkit.py b/tools/modkit.py index bae76b15..6f5effab 100644 --- a/tools/modkit.py +++ b/tools/modkit.py @@ -10,6 +10,7 @@ Subcommands: [--refresh] [--dest DIR] [--pixel-font] validate [--strict] [--base auto|fixture|imported] gen2check [...] [--strict] [--notes] + gen3check [...] [--strict] [--notes] lint pack [-o out.modpkg] bounce [--seconds N] [--out DIR] @@ -31,14 +32,17 @@ fixture that rule is reported as skipped rather than guessed at. lint is the no-ROM-content distribution gate (MK3xx); pack runs both at --strict, so any finding -- warning included -- refuses the package. -gen2check (MK4xx) answers whether a mod runs on a Gen 2 game and how far it -gets: the manifest gate, then a static read of the mod's Lua against what -src/mods/Gen2Compat.lua actually backs, member by member. It is a scan, not +gen2check and gen3check (MK4xx) answer whether a mod runs on a Gen 2 or a +Gen 3 (FireRed) game and how far it gets: the manifest gate, then a static +read of the mod's Lua against what src/mods/Gen2Compat.lua or +src/mods/Gen3Compat.lua actually backs, member by member. It is a scan, not an interpreter -- what it could not follow is listed as unresolved rather than guessed at -- and it exits non-zero on a finding it calls fatal. Mods named together are read as one install set, so a mod and its dependencies answer each other; --notes adds the adapter's own line for every backed -member the mod touches. +member the mod touches. The rule ids are shared by both commands: MK400 +means the same thing either way, and the verdict line names the game that +answered. """ import argparse @@ -860,7 +864,7 @@ def run_loader(repo, mod_dir, findings, base="fixture", notes=None, if base != "imported": skipped.add("MK103") continue - if gen2_routed and declares_gen2(repo, manifest): + if gen2_routed and declares_generation(repo, manifest, GEN2): # tools/build_data.py never writes a Gen 2 cache, so the # imported dataset has no Gold/Crystal ground truth either. skipped.add("MK103") @@ -2275,8 +2279,8 @@ def lua_api(path): def gen1_only_modules(repo): - """The Gen 1 modules a Gold boot never instantiates, read from the loader - so this tool and the require shim cannot disagree (Loader.lua).""" + """The Gen 1 modules a Gen 2 or Gen 3 boot never instantiates, read from + the loader so this tool and the require shim cannot disagree (Loader.lua).""" try: src = open(os.path.join(repo, "src", "mods", "Loader.lua"), encoding="utf-8").read() @@ -2286,12 +2290,64 @@ def gen1_only_modules(repo): return set(re.findall(r'\["([^"]+)"\]', block.group(1))) if block else set() -def _adapters_from_source(repo): +class Generation: + """One target generation, as the MK4xx checks need to see it. gen2check + and gen3check are the same checks run against two of these, so a rule that + only makes sense on one of them is switched off on the descriptor rather + than duplicated inside the check.""" + + def __init__(self, number, compat, doc, own_dir, legacy_flag=None, + screen_prefix=None, snake_files=False): + self.number = number + self.label = "Gen %d" % number + self.compat = compat # the layer a boot of it resolves through + self.compat_file = compat.replace(".", "/") + ".lua" + self.doc = doc # the doc the findings cite + self.own_dir = own_dir # where this generation's modules live + self.legacy_flag = legacy_flag # the pre-`games` manifest flag + self.screen_prefix = screen_prefix # this generation's screen twins + self.snake_files = snake_files # whether its files are snake_case + + def siblings(self, module): + """Where this generation runs the module a mod named the Gen 1 way: + src.ui.StartMenu is src.ui.gen2.StartMenu on Gold and + src.ui.game3.start_menu on FireRed. Both spellings are offered for a + snake_case generation, because src/world/game3 keeps the CamelCase + basename that src/ui/game3 does not.""" + parts = module.split(".") + if len(parts) < 3: + return [] + names = [parts[-1]] + if self.snake_files: + alt = _snake(parts[-1]) + if alt != parts[-1]: + names.append(alt) + return [".".join(parts[:-1] + [self.own_dir, name]) + for name in names] + + +def _snake(name): + """StartMenu -> start_menu, the spelling src/ui/game3 writes its files in.""" + return re.sub(r"(? alias target ("" for a built facade), for the checkout where the coverage accessor cannot be run.""" try: - src = strip_lua(open(os.path.join(repo, "src", "mods", - "Gen2Compat.lua"), + src = strip_lua(open(os.path.join(repo, *gen.compat_file.split("/")), encoding="utf-8").read()) except OSError: return {} @@ -2302,14 +2358,15 @@ def _adapters_from_source(repo): r'\["([^"]+)"\]\s*=\s*(?:"([^"]+)"|\w+)', block.group(1))} -def gen2_coverage(repo, notes): +def compat_coverage(repo, notes, gen): """name -> {kind, target, members, notes, declared}, straight off - Gen2Compat.coverage. `members` is None where nothing could answer, which - every check below treats as "unknown", never as "backed".""" + Gen2Compat.coverage or Gen3Compat.coverage. `members` is None where + nothing could answer, which every check below treats as "unknown", never + as "backed".""" rows = [] with tempfile.NamedTemporaryFile("w", suffix=".lua", delete=False, encoding="utf-8") as handle: - handle.write(COVERAGE_DUMP) + handle.write(_probe(COVERAGE_DUMP, gen)) dump_path = handle.name try: proc = subprocess.run([LUAJIT, dump_path], cwd=repo, @@ -2342,7 +2399,7 @@ def gen2_coverage(repo, notes): elif parts[0] == "NOTE" and len(parts) >= 4: coverage[parts[1]]["notes"][parts[2]] = "\t".join(parts[3:]) if not coverage: - for name, alias in _adapters_from_source(repo).items(): + for name, alias in _adapters_from_source(repo, gen).items(): coverage[name] = {"kind": "alias" if alias else "facade", "target": alias, "members": None, "notes": {}, "declared": False} @@ -2950,62 +3007,67 @@ def module_at(sites, offset): # ------------------------------------------------------------- the checks -GEN2_IDS_DUMP = '''\ +VERSION_IDS_DUMP = '''\ package.path = "./?.lua;./?/init.lua;" .. package.path -print(table.concat(require("src.mods.ModTargets").generationVersions(2), " ")) +print(table.concat(require("src.mods.ModTargets").generationVersions(%d), " ")) ''' -_GEN2_IDS = None +_VERSION_IDS = {} -def gen2_version_ids(repo): - """The Gen 2 version ids, read out of the engine (src/mods/ModTargets.lua) - rather than restated here. Empty when luajit cannot answer, which leaves - the "gen2"/"all" tokens to decide alone.""" - global _GEN2_IDS - if _GEN2_IDS is None: - _GEN2_IDS = [] +def version_ids(repo, gen): + """This generation's version ids, read out of the engine + (src/mods/ModTargets.lua) rather than restated here. Empty when luajit + cannot answer, which leaves the "genN"/"all" tokens to decide alone.""" + if gen.number not in _VERSION_IDS: + _VERSION_IDS[gen.number] = [] try: - proc = subprocess.run([LUAJIT, "-e", GEN2_IDS_DUMP], cwd=repo, - capture_output=True, text=True, timeout=30) + proc = subprocess.run( + [LUAJIT, "-e", VERSION_IDS_DUMP % gen.number], cwd=repo, + capture_output=True, text=True, timeout=30) if proc.returncode == 0: - _GEN2_IDS = proc.stdout.split() + _VERSION_IDS[gen.number] = proc.stdout.split() except (OSError, subprocess.SubprocessError): pass - return _GEN2_IDS + return _VERSION_IDS[gen.number] -def declares_gen2(repo, manifest): - """Does this manifest claim a Gen 2 game: the `games` list, or the legacy - gen2compat flag it is derived from (src/mods/Manifest.lua).""" +def declares_generation(repo, manifest, gen): + """Does this manifest claim a game of this generation: the `games` list, + or the legacy gen2compat flag it is derived from (src/mods/Manifest.lua). + Only Gen 2 has a legacy flag -- `games` is the only way to claim a Gen 3 + game, and a Gold mod that never named one still answers yes through the + flag.""" if not manifest: return False - if manifest.get("gen2compat"): + if gen.legacy_flag and manifest.get(gen.legacy_flag): return True games = manifest.get("games") if not isinstance(games, list): return False - ids = set(gen2_version_ids(repo)) + ids = set(version_ids(repo, gen)) for token in games: if isinstance(token, str) and ( - token.strip().lower() in ("gen2", "all") + token.strip().lower() in ("gen%d" % gen.number, "all") or token.strip().lower() in ids): return True return False -def check_gen2_manifest(repo, mod_dir, manifest, named): +def check_compat_manifest(repo, mod_dir, manifest, named, gen): """MK400/MK401: what the loader decides before a line of the mod runs (src/mods/Loader.lua's generation gate). `named` is every mod on this command line, so checking a mod together with its dependencies reads them as one install set.""" findings, notes = [], [] - if not declares_gen2(repo, manifest): + if not declares_generation(repo, manifest, gen): findings.append(Finding( "MK400", "error", - "no Gen 2 game in \"games\" (and no gen2compat), so a Gen 2 boot " - "skips this mod; the rest of this report is what it would hit " - "once it claims one", + "no %s game in \"games\"%s, so a %s boot skips this mod; the rest " + "of this report is what it would hit once it claims one" + % (gen.label, + " (and no %s)" % gen.legacy_flag if gen.legacy_flag else "", + gen.label), "manifest.json")) deps = manifest.get("dependencies") or [] for dep in deps if isinstance(deps, list) else []: @@ -3016,17 +3078,20 @@ def check_gen2_manifest(repo, mod_dir, manifest, named): g_list = dep.get("games") if isinstance(g_list, str): g_list = [g_list] - if isinstance(g_list, list) and not any(g in ["gen2", "gold", "silver", "crystal", "all"] for g in g_list): + if isinstance(g_list, list) and not any( + g in ["gen%d" % gen.number, "all"] + version_ids(repo, gen) + for g in g_list): continue found = named.get(dep_id) or find_mod_by_id(repo, mod_dir, dep_id) if found is None: notes.append("unresolved: dependency %s is not installed beside " "this mod, so its games list could not be read" % dep_id) - elif not declares_gen2(repo, found): + elif not declares_generation(repo, found, gen): findings.append(Finding( "MK401", "error", - f"depends on {dep_id}, which claims no Gen 2 game; the " - f"loader disables a mod whose dependency a Gen 2 boot skipped", + f"depends on {dep_id}, which claims no {gen.label} game; the " + f"loader disables a mod whose dependency a {gen.label} boot " + f"skipped", "manifest.json")) return findings, notes @@ -3056,12 +3121,12 @@ def find_mod_by_id(repo, mod_dir, mod_id): return None -def check_gen2_requires(repo, coverage, requires): - """MK402: a Gen 1 module a Gen 2 boot never instantiates and no adapter - backs -- the require succeeds, the patch lands on dead code, and the - loader says so in the manager's error feed. MK403: the same silence - without the loader's warning, spotted from the gen2/ sibling that runs - instead.""" +def check_compat_requires(repo, coverage, requires, gen): + """MK402: a Gen 1 module this generation's boot never instantiates and no + adapter backs -- the require succeeds, the patch lands on dead code, and + the loader says so in the manager's error feed. MK403: the same silence + without the loader's warning, spotted from the gen2/ or game3/ sibling + that runs instead.""" findings, notes = [], [] gen1_only = gen1_only_modules(repo) seen = set() @@ -3077,30 +3142,28 @@ def check_gen2_requires(repo, coverage, requires): if module in gen1_only: findings.append(Finding( "MK402", "error", - f"requires {module}, which a Gen 2 boot never runs and " - f"src/mods/Gen2Compat.lua has no adapter for; take the game " + f"requires {module}, which a {gen.label} boot never runs and " + f"{gen.compat_file} has no adapter for; take the game " f"from the game.ready payload and mod.world instead", f"{rel}:{line}")) continue - parts = module.split(".") - if len(parts) < 3: - continue - sibling = ".".join(parts[:-1] + ["gen2", parts[-1]]) - if os.path.isfile(module_path(repo, sibling)): - findings.append(Finding( - "MK403", "warn", - f"requires {module}, but a Gen 2 game runs {sibling}; the " - f"require succeeds and hands back a module nothing " - f"instantiates", - f"{rel}:{line}")) + for sibling in gen.siblings(module): + if os.path.isfile(module_path(repo, sibling)): + findings.append(Finding( + "MK403", "warn", + f"requires {module}, but a {gen.label} game runs " + f"{sibling}; the require succeeds and hands back a module " + f"nothing instantiates", + f"{rel}:{line}")) + break return findings, notes -def check_gen2_members(repo, coverage, uses, advise=False): - """MK404: a member the adapter says has no Gen 2 backing, so the read is - nil and the call raises. MK405: one that is there and degrades, in the - adapter's own words. MK406: one whose parameters moved under it -- the - trap an alias sets, because it runs and means something else.""" +def check_compat_members(repo, coverage, uses, gen, advise=False): + """MK404: a member the adapter says has no backing, so the read is nil and + the call raises. MK405: one that is there and degrades, in the adapter's + own words. MK406: one whose parameters moved under it -- the trap an + alias sets, because it runs and means something else.""" findings, notes = [], [] owned = {(use.module, use.member) for use in uses if use.kind == "write"} for use in uses: @@ -3122,7 +3185,7 @@ def check_gen2_members(repo, coverage, uses, advise=False): api = lua_api(module_path(repo, record["target"])) or {} \ if record["target"] else {} if use.chain[0] in api or (use.module, use.chain[0]) in owned: - continue # the Gen 2 module carries it, or the mod put it there + continue # this generation's module carries it, or the mod put it there if use.chain[0] not in gen1: continue # the mod's own field on a table it did not declare notes.append("unresolved: %s.%s is a Gen 1 member the coverage " @@ -3131,11 +3194,11 @@ def check_gen2_members(repo, coverage, uses, advise=False): if status == ABSENT: findings.append(Finding( "MK404", "warn" if use.guarded else "error", - "%s.%s has no Gen 2 backing: %s" - % (use.ident, use.member, note or "%s has no %s" - % (target, member)) + "%s.%s has no %s backing: %s" + % (use.ident, use.member, gen.label, + note or "%s has no %s" % (target, member)) + ("; the guarded branch never runs" if use.guarded - else "; nothing on a Gen 2 boot reads this write" + else "; nothing on a %s boot reads this write" % gen.label if use.kind == "write" else "; this reads nil" + (" and the call raises" if use.kind == "call" else "")), use.where())) @@ -3143,19 +3206,19 @@ def check_gen2_members(repo, coverage, uses, advise=False): if status != "backed": findings.append(Finding( "MK405", "warn", - "%s.%s is %s on a Gen 2 boot: %s" - % (use.ident, use.member, status, + "%s.%s is %s on a %s boot: %s" + % (use.ident, use.member, status, gen.label, note or "it answers nil and names itself once in the log"), use.where())) continue - held = _held_at_file_scope(repo, record, use) + held = _held_at_file_scope(repo, record, use, gen) if held: findings.append(held) continue - shapes = _signature_diff(repo, record, use) + shapes = _signature_diff(repo, record, use, gen) if shapes: - # an alias hands the mod the Gen 2 module itself: no shim stands - # between this call and the parameters that moved under it + # an alias hands the mod the generation's module itself: no shim + # stands between this call and the parameters that moved under it findings.append(Finding( "MK406", "warn", shapes + ("; " + note if note else ""), use.where())) @@ -3164,13 +3227,13 @@ def check_gen2_members(repo, coverage, uses, advise=False): return findings, notes -def _held_at_file_scope(repo, record, use): +def _held_at_file_scope(repo, record, use, gen): """MK410: the entry chunk reading a member the Gen 1 module only ever writes onto the running game. A facade resolves against the live instance at read time and there is none yet while the mod is loading, so the value captured is nil for the life of the process; the same read from inside a - hook or an event is correct (docs/mod-api-gen2-compat.md, "live, never a - snapshot").""" + hook or an event is correct (docs/mod-api-gen2-compat.md and + docs/mod-api-gen3-compat.md, "live, never a snapshot").""" if not use.top or use.kind == "write" or record["kind"] != "facade": return None entry = (lua_api(module_path(repo, use.module)) or {}).get(use.chain[0]) @@ -3178,9 +3241,9 @@ def _held_at_file_scope(repo, record, use): return None return Finding( "MK410", "warn", - f"reads {use.ident}.{use.member} at file scope, where a Gen 2 boot " - f"has no game yet: the facade answers nil until one exists, so take " - f"this from the game.ready payload instead of the entry chunk", + f"reads {use.ident}.{use.member} at file scope, where a {gen.label} " + f"boot has no game yet: the facade answers nil until one exists, so " + f"take this from the game.ready payload instead of the entry chunk", use.where()) @@ -3202,15 +3265,15 @@ def _resolve_member(members, chain): return None, None -def _signature_diff(repo, record, use): - """The sentence for a call whose parameters moved: the Gen 2 module spells - them in an order the Gen 1 call site cannot survive, or takes a different - number of them. Equal shape with different names is a rename as often as - a change, and this tool does not guess between the two. +def _signature_diff(repo, record, use, gen): + """The sentence for a call whose parameters moved: the generation's module + spells them in an order the Gen 1 call site cannot survive, or takes a + different number of them. Equal shape with different names is a rename as + often as a change, and this tool does not guess between the two. An alias only: a facade is free to override the member with the Gen 1 shape (src/mods/Gen2Compat.lua's Boxes.deposit does exactly that), so the - Gen 2 module's parameters are not what the mod would be calling.""" + generation's own parameters are not what the mod would be calling.""" if (use.kind != "call" or record["kind"] != "alias" or not record["target"] or len(use.chain) != 1): return None @@ -3220,8 +3283,9 @@ def _signature_diff(repo, record, use): use.member, {}).get("params") if want is None or have is None or want == have: return None - shapes = ("%s.%s is (%s) on a Gen 2 boot and (%s) on Gen 1" - % (use.ident, use.member, ", ".join(want), ", ".join(have))) + shapes = ("%s.%s is (%s) on a %s boot and (%s) on Gen 1" + % (use.ident, use.member, ", ".join(want), gen.label, + ", ".join(have))) if _reordered(want, have): return shapes + "; the shared parameters sit in different places" if (use.argc is not None and not use.varargs @@ -3271,18 +3335,21 @@ end _UPVALUE_CACHE = {} -def gen2_upvalues(repo, queries): +def compat_upvalues(repo, queries, gen): """(status, upvalue names) for each (module, member) a mod reaches, taken by resolving the adapter the way src/mods/Loader.lua does and enumerating the function's real upvalues. A pair luajit could not answer for stays out - of the table, which the caller reports as unknown and never as landing.""" + of the table, which the caller reports as unknown and never as landing. + One table per generation: the same name answers differently on either + side.""" + cache = _UPVALUE_CACHE.setdefault(gen.number, {}) wanted = sorted({pair for pair in queries - if pair[0] and pair not in _UPVALUE_CACHE}) + if pair[0] and pair not in cache}) if not wanted: - return _UPVALUE_CACHE + return cache with tempfile.NamedTemporaryFile("w", suffix=".lua", delete=False, encoding="utf-8") as handle: - handle.write(UPVALUE_DUMP) + handle.write(_probe(UPVALUE_DUMP, gen)) dump_path = handle.name try: proc = subprocess.run( @@ -3293,24 +3360,23 @@ def gen2_upvalues(repo, queries): for row in proc.stdout.splitlines(): parts = row.split("\t") if len(parts) >= 4: - _UPVALUE_CACHE[(parts[0], parts[1])] = ( - parts[2], parts[3].split()) + cache[(parts[0], parts[1])] = (parts[2], parts[3].split()) except (OSError, subprocess.SubprocessError): pass finally: os.unlink(dump_path) - return _UPVALUE_CACHE + return cache -def check_gen2_upvalues(repo, coverage, upvalues): +def check_compat_upvalues(repo, coverage, upvalues, gen): """MK407/MK408: reaching an engine function's file-local with debug.setupvalue. The function is resolved through the adapter and its - upvalues enumerated, so a member the Gen 2 arm does not carry is the error - it is at runtime and a local that is not an upvalue of it never reads as - landing.""" + upvalues enumerated, so a member the generation's arm does not carry is + the error it is at runtime and a local that is not an upvalue of it never + reads as landing.""" findings, notes = [], [] - table = gen2_upvalues(repo, [(module, member) for _, _, module, member, _ - in upvalues if module in coverage]) + table = compat_upvalues(repo, [(module, member) for _, _, module, member, _ + in upvalues if module in coverage], gen) lands = {} for rel, line, module, member, upvalue in upvalues: record = coverage.get(module) @@ -3323,15 +3389,15 @@ def check_gen2_upvalues(repo, coverage, upvalues): findings.append(Finding( "MK408", "warn", f"reaches the upvalue {upvalue!r} on {member}; this scan could " - f"not resolve {module}.{member} on a Gen 2 boot, so whether " - f"the surgery lands is unknown", + f"not resolve {module}.{member} on a {gen.label} boot, so " + f"whether the surgery lands is unknown", f"{rel}:{line}")) continue if status != "ok": findings.append(Finding( "MK407", "error", - f"reaches the upvalue {upvalue!r} on {member}, but a Gen 2 " - f"boot resolves {module}.{member} to " + f"reaches the upvalue {upvalue!r} on {member}, but a " + f"{gen.label} boot resolves {module}.{member} to " + ("nil" if status == "nomember" else "a value that is not a " "function") + f" ({target} carries no such function), so the " @@ -3347,26 +3413,28 @@ def check_gen2_upvalues(repo, coverage, upvalues): if record["target"] else {} findings.append(Finding( "MK407", "error", - f"reaches the upvalue {upvalue!r} on {member}, but on a Gen 2 boot " - f"{module}.{member} closes over " + f"reaches the upvalue {upvalue!r} on {member}, but on a " + f"{gen.label} boot {module}.{member} closes over " + (", ".join(sorted(names)[:6]) if names else "nothing") + ", so the surgery lands on nothing" + (f"; {target.split('.')[-1]}.{setter} is the supported route" if setter in api else ""), f"{rel}:{line}")) for (upvalue, module, member), places in sorted(lands.items()): - notes.append("%s.%s closes over %r on a Gen 2 boot, so the upvalue " + notes.append("%s.%s closes over %r on a %s boot, so the upvalue " "surgery at %s lands as it does on Gen 1" - % (module, member, upvalue, _places(places))) + % (module, member, upvalue, gen.label, _places(places))) return findings, notes -def check_gen2_patterns(repo, mod_dir): +def check_compat_patterns(repo, mod_dir, gen): """MK409: the two shapes no adapter is allowed to fix, because the mod - decided something about the game and a Gen 2 boot answers differently - (docs/mod-api-gen2-compat.md, "what the facades cannot fix").""" + decided something about the game and this generation's boot answers + differently (the compat doc, "what the facades cannot fix"). The screen + twin half is Gen 2 only: it is a fact about Screens.GEN2_IDS, and Gen 3 + has no such table.""" findings = [] - twins = gen2_screen_twins(repo) + twins = gen2_screen_twins(repo) if gen.screen_prefix else set() for rel in mod_files(mod_dir): if os.path.splitext(rel)[1].lower() != ".lua": continue @@ -3375,9 +3443,9 @@ def check_gen2_patterns(repo, mod_dir): for match in VERSION_MATCH.finditer(body): findings.append(Finding( "MK409", "warn", - "allow-lists a Gen 1 version string, which excludes this mod " - "from a Gen 2 game by construction; test for the capability " - "the code needs instead of the version", + f"allow-lists a Gen 1 version string, which excludes this mod " + f"from a {gen.label} game by construction; test for the " + f"capability the code needs instead of the version", "%s:%d" % (rel, _line_of(body, match.start())))) # the id itself, not a word in the line around it: `if id == "BoxMenu"` # carries no screen-shaped word and is the shape the docs warn about @@ -3388,10 +3456,10 @@ def check_gen2_patterns(repo, mod_dir): line = _line_of(body, match.start()) findings.append(Finding( "MK409", "warn", - f"{name!r} is a Gen 1 screen id; a Gen 2 boot builds " - f"'Gen2{name}' (Screens.GEN2_IDS in src/ui/Screens.lua), so a " - f"screen compared or opened by this literal matches nothing " - f"there", + f"{name!r} is a Gen 1 screen id; a {gen.label} boot builds " + f"'{gen.screen_prefix}{name}' (Screens.GEN2_IDS in " + f"src/ui/Screens.lua), so a screen compared or opened by this " + f"literal matches nothing there", "%s:%d" % (rel, line))) return findings @@ -3418,13 +3486,13 @@ def _count(total, word): "" if total == 1 else "s") -def gen2_verdict(findings): +def compat_verdict(findings): if any(f.severity == "error" for f in findings): return "will not work" return "will load but degrade" if findings else "will load" -def report_gen2(results, args): +def report_compat(results, args, gen): """report()'s shape plus the per-mod verdict this command exists to give. One JSON document covers every mod named, so a CI step reads one object however many it gated on.""" @@ -3434,7 +3502,7 @@ def report_gen2(results, args): [f for f in findings if f.severity == "error"] if errors: ok = False - payload.append({"id": mod_id, "verdict": gen2_verdict(findings), + payload.append({"id": mod_id, "verdict": compat_verdict(findings), "errors": len(errors), "manifest": facts, "findings": [f.as_dict() for f in findings], "notes": notes}) @@ -3454,15 +3522,16 @@ def report_gen2(results, args): counts = ", ".join(part for part in ( _count(len(findings) - warns, "error"), _count(warns, "warning")) if part) - print("%s %s on gen 2: %s%s" + print("%s %s on gen %d: %s%s" % ("FAIL" if payload[index]["errors"] else "ok", mod_id, - payload[index]["verdict"], " (%s)" % counts if counts else "")) + gen.number, payload[index]["verdict"], + " (%s)" % counts if counts else "")) return 0 if ok else 1 -def cmd_gen2check(args, repo): +def cmd_compat_check(args, repo, gen): shared = [] - coverage = gen2_coverage(repo, shared) + coverage = compat_coverage(repo, shared, gen) results = [] dirs, named = [], {} for target in args.mod: @@ -3479,27 +3548,35 @@ def cmd_gen2check(args, repo): if problem: findings.append(problem) else: - manifest_findings, manifest_notes = check_gen2_manifest( - repo, mod_dir, manifest, named) + manifest_findings, manifest_notes = check_compat_manifest( + repo, mod_dir, manifest, named, gen) findings.extend(manifest_findings) notes.extend(manifest_notes) requires, uses, upvalues, scan_notes = scan_module_uses(mod_dir) - require_findings, require_notes = check_gen2_requires( - repo, coverage, requires) + require_findings, require_notes = check_compat_requires( + repo, coverage, requires, gen) findings.extend(require_findings) - member_findings, member_notes = check_gen2_members( - repo, coverage, uses, args.notes) + member_findings, member_notes = check_compat_members( + repo, coverage, uses, gen, args.notes) findings.extend(member_findings) - upvalue_findings, upvalue_notes = check_gen2_upvalues( - repo, coverage, upvalues) + upvalue_findings, upvalue_notes = check_compat_upvalues( + repo, coverage, upvalues, gen) findings.extend(upvalue_findings) - findings.extend(check_gen2_patterns(repo, mod_dir)) + findings.extend(check_compat_patterns(repo, mod_dir, gen)) notes.extend(scan_notes + require_notes + member_notes + upvalue_notes) mod_id = manifest.get("id") if manifest else os.path.basename(mod_dir) results.append((mod_id, _order(_dedupe(findings)), _dedupe_notes(notes), _facts(manifest))) - return report_gen2(results, args) + return report_compat(results, args, gen) + + +def cmd_gen2check(args, repo): + return cmd_compat_check(args, repo, GEN2) + + +def cmd_gen3check(args, repo): + return cmd_compat_check(args, repo, GEN3) def _dedupe(findings): @@ -3580,8 +3657,9 @@ def main(argv): p.add_argument("--experimental", action="store_true", help="mark the mod experimental (off until confirmed)") p.add_argument("--games", default="gen1", - help="games this mod is for: gen1, gen2, all, or a " - "comma-separated list of version ids (red,gold,...)") + help="games this mod is for: gen1, gen2, gen3, all, or a " + "comma-separated list of version ids " + "(red,gold,firered,...)") p.add_argument("--dest") p.add_argument("--force", action="store_true") @@ -3599,6 +3677,15 @@ def main(argv): help="also print the adapter's note for every backed " "member the mod touches") + p = sub.add_parser("gen3check", parents=[shared], + help="will this mod run on a Gen 3 (FireRed) game, " + "and how far") + p.add_argument("mod", nargs="+") + p.add_argument("--strict", action="store_true") + p.add_argument("--notes", action="store_true", + help="also print the adapter's note for every backed " + "member the mod touches") + p = sub.add_parser("lint", parents=[shared]) p.add_argument("mod") @@ -3675,6 +3762,7 @@ def main(argv): "scaffold": cmd_scaffold, "validate": cmd_validate, "gen2check": cmd_gen2check, + "gen3check": cmd_gen3check, "lint": cmd_lint, "pack": cmd_pack, "bounce": cmd_bounce,