From 0147e3d87b28807a809f834ee53f7ec1a787449b Mon Sep 17 00:00:00 2001 From: Bo Layer Date: Sun, 23 Aug 2026 03:17:27 -0600 Subject: [PATCH 1/4] feat(mod-api): add composable map occupancy seam --- data/scripts/story3.lua | 33 ++++- docs/modding.md | 50 +++++++ .../0013-map-occupancy-and-active-block.md | 131 ++++++++++++++++++ src/world/WorldAPI.lua | 30 ++++ 4 files changed, 237 insertions(+), 7 deletions(-) create mode 100644 docs/rfcs/0013-map-occupancy-and-active-block.md diff --git a/data/scripts/story3.lua b/data/scripts/story3.lua index d2d4dbd5..787b0d99 100644 --- a/data/scripts/story3.lua +++ b/data/scripts/story3.lua @@ -2,6 +2,8 @@ -- ghost, elevators, the Game Corner coins/prizes, the SS Anne departure -- and the Hall of Fame record. Each cites its pokered source. +local Runtime = require("src.mods.Runtime") + local M = {} -- ------------------------------------------------------------------- @@ -909,17 +911,34 @@ M.VERMILION_DOCK = { local f = game.save.flags if Flags.get(game.save, "EVENT_SS_ANNE_LEFT") then -- the ship is long gone: erase her right away, and anyone who - -- still lands here is sent back out past the guard + -- still lands here is sent back out past the guard unless a mod + -- explicitly permits this occupied map state. This hook surrounds + -- only the ejection decision; the map's complete onEnter chain has + -- already run and the departed ship remains erased. for _, b in ipairs(DOCK_SHIP_BLOCKS) do ow.map:setBlock(b.bx, b.by, b.water) end ow.map.renderer:rebuild() - local TextBox = require("src.render.TextBox") - game.stack:push(TextBox.new(game, - game.data.text._VermilionCitySailor1ShipSetSailText - or "The ship set sail.", function() - ow:startWarpTo("VERMILION_CITY", 18, 29, "up") - end)) + local occupancyAllowed = false + if Runtime.wantsHook("map.occupancy_allowed") then + local player = ow.player or {} + occupancyAllowed = Runtime.call("map.occupancy_allowed", + function() return false end, game, { + mapId = "VERMILION_DOCK", + reason = "ss_anne_departed", + gameVersion = game.save and game.save.version, + x = player.cellX, + y = player.cellY, + }) == true + end + if not occupancyAllowed then + local TextBox = require("src.render.TextBox") + game.stack:push(TextBox.new(game, + game.data.text._VermilionCitySailor1ShipSetSailText + or "The ship set sail.", function() + ow:startWarpTo("VERMILION_CITY", 18, 29, "up") + end)) + end elseif f.EVENT_GOT_HM01 and ow.player.cellY == 2 then -- VermilionDockSSAnneLeavesScript: only stepping OFF the ship -- triggers the departure (wDestinationWarpID == 1 in pokered) -- diff --git a/docs/modding.md b/docs/modding.md index 7c724404..73c8d853 100644 --- a/docs/modding.md +++ b/docs/modding.md @@ -228,6 +228,56 @@ optional visual `tileRows` at 2x resolution, and optional `tileDetailRows` at read-only snapshots; mods choose which layers to render. Red and Gold expose the same contract while applying their own object and event visibility rules. +### Active Gen 1 block checks + +Red, Blue, and Yellow expose +`mod.world:activeBlockAt(mapId, blockX, blockY)`. It returns the numeric block +ID at one zero-based block coordinate only when `mapId` is the active map. +The value is a scalar snapshot: changing it cannot change the map. This lets a +mod compare a small runtime map signature before it applies a lawful authored +replacement, without reading the mutable map or ROM cache through engine +internals. + +The method fails closed. Before an overworld exists it returns +`nil, "no overworld"`; for a different active map it returns +`nil, "map is not active"`; non-numeric, non-finite, or fractional coordinates +return `nil, "invalid block coordinates"`; and negative or out-of-range +coordinates return `nil, "block coordinates out of bounds"`. An unavailable +or malformed active block returns `nil, "block unavailable"`. The caller must +require every expected cell to match before changing presentation. This method +is Gen 1-only; Gold callers receive no parity promise for it. + +### Conditional map occupancy + +`map.occupancy_allowed` is a narrow Gen 1 hook around a map script's vanilla +decision to eject the player from an otherwise valid loaded map. Its first +call site is the post-departure `VERMILION_DOCK` branch. The ship has already +been erased when the hook runs, and the hook does not replace or suppress any +base or peer map handler. + +The wrapper receives `(next, game, context)`. The dock context is a copied +`{ mapId = "VERMILION_DOCK", reason = "ss_anne_departed", gameVersion, x, y }` +record. Vanilla returns `false`. Return exactly `true` to allow the player to +remain; every other value denies occupancy and preserves the normal message +and warp. A composable wrapper calls downstream first and only adds its own +permission: + +```lua +mod.hooks:wrap("map.occupancy_allowed", function(next, game, ctx) + local allowed = next(game, ctx) + local mine = ctx.mapId == "VERMILION_DOCK" + and ctx.reason == "ss_anne_departed" + and myPublicEligibilityCheck(game) + return allowed == true or mine == true +end) +``` + +With no wrapper, the hook allocates no context and vanilla behavior is byte-for- +byte unchanged. A throwing wrapper is isolated by the normal hook bus. A nil, +string, number, table, or other malformed final answer fails closed. Disabling +or uninstalling the permitting mod therefore restores vanilla ejection without +changing the S.S. Anne story flag or restoring the ship. + ## Party ordering Companion UIs and alternate party screens can call diff --git a/docs/rfcs/0013-map-occupancy-and-active-block.md b/docs/rfcs/0013-map-occupancy-and-active-block.md new file mode 100644 index 00000000..1066ddc3 --- /dev/null +++ b/docs/rfcs/0013-map-occupancy-and-active-block.md @@ -0,0 +1,131 @@ +# RFC 0013: Conditional map occupancy and active-block reads + +## Status + +Proposed. + +## Motivation + +The Gen 1 Vermilion Dock script correctly ejects a player who enters after the +S.S. Anne has departed. A content mod can add a city-side route back to that +empty harbor, but it cannot preserve the dock visit: replacing the complete +dock handler would discard vanilla behavior and peer handlers, while an added +handler cannot cancel the base handler's ejection. + +A mod that changes one active map block also needs to prove that it is looking +at the expected Red, Blue, or Yellow layout before it acts. `mapOverview()` is +intentionally presentation-oriented and does not expose block identity. +Requiring internal `Map` state or generated ROM data would cross the public mod +boundary and make a wrong-version edit difficult to fail closed. + +## Decision and plan extended + +This extends Route B in `CONTRIBUTING-mods.md`: new behavior is additive, +ordinary hook composition remains the authority, an empty hook chain is a +provable no-op, and mods receive copied or scalar data rather than mutable +engine state. It supports the approved Mew-under-the-truck implementation plan +without adding any Mew-specific rule, asset, flag, or content to the engine. + +## Exact API delta + +### `map.occupancy_allowed` + +The post-departure `VERMILION_DOCK` script calls this hook after it replaces the +ship blocks with water and immediately before it would display the departure +message and warp the player to Vermilion City. + +A wrapper has this shape: + +```lua +function(next, game, context) -> boolean +``` + +The context is a new table with these fields: + +| Field | Meaning | +|---|---| +| `mapId` | `"VERMILION_DOCK"` at this call site | +| `reason` | Stable reason key `"ss_anne_departed"` | +| `gameVersion` | Active save version (`red`, `blue`, or `yellow`) when present | +| `x`, `y` | Current player cell coordinates when present | + +Vanilla returns `false`. The player remains only when the final chain result is +exactly `true`. Absent, throwing, or malformed wrappers therefore preserve +ejection. A wrapper composes by calling `next(game, context)` and returning +true when either downstream or its own narrow rule permits occupancy. The hook +does not replace `MapScripts` registration, merging, or dispatch, and does not +change the departure flag or reconstruct the ship. + +The call is guarded by `Runtime.wantsHook`, so an empty chain allocates no +context and follows the prior branch exactly. + +### `WorldAPI:activeBlockAt` + +Gen 1's public `mod.world` facade adds: + +```lua +activeBlockAt(mapId, blockX, blockY) -> blockId + | nil, reason +``` + +`mapId` must equal the active map ID. Coordinates are finite, integral, +zero-based block coordinates. A successful result is a numeric scalar copied +from the active runtime map. The method never returns the map's mutable block +array and never writes game or save state. + +Failure reasons are stable: + +| Condition | Reason | +|---|---| +| No active overworld map | `no overworld` | +| `mapId` differs from the active map | `map is not active` | +| Coordinate has the wrong type, is non-finite, or is fractional | `invalid block coordinates` | +| Coordinate is negative or outside the active map | `block coordinates out of bounds` | +| Active block data is absent or malformed | `block unavailable` | + +Requiring the expected map ID and rejecting all ambiguous input lets a mod +compare every cell in its version-specific signature before it calls an +existing mutation API. Red, Blue, and Yellow each use their own loaded map +data. Gold does not gain this method in this RFC. + +## Migration and compatibility + +Existing mods change nothing. No hook, event, registry, manifest field, save +field, map handler, or WorldAPI method is removed or renamed. With no hook +subscriber the departed-dock behavior is unchanged. Existing callers cannot +invoke the new WorldAPI method accidentally. + +The occupancy answer is not persisted by the engine. Disabling or uninstalling +a mod removes its wrapper through normal owner cleanup, so a later dock entry +uses vanilla ejection. The engine writes no new save state and neither API +returns or serializes ROM or save data. + +## Verification requirements + +The parity gate must prove: + +- vanilla departed-dock message and warp remain with no subscriber; +- one permission wrapper can allow occupancy without removing the ship-erasure + work or any map handler; +- multiple cooperative wrappers preserve downstream permission; +- absent, throwing, nil, false, and malformed hook answers fail closed; +- Red, Blue, and Yellow contexts keep their version identity separate; +- disabling or removing the owner restores vanilla behavior without a save + migration; +- `activeBlockAt` accepts only the active map and valid in-range integral + coordinates, returns a scalar, and never mutates map or save state; and +- the test fixtures and resulting changes contain no ROM or save payload. + +The hook must be driven through a real public `hooks:wrap` chain. The block API +must be exercised through a public `WorldAPI` instance. Tests must not replace +the production map handler with a test-only implementation. + +## Docs with the change + +`docs/modding.md` documents both contracts, their failure behavior, the +cooperative wrapper pattern, and the Gen 1-only block method. No registry or +schema changes occur, so generated registry documentation is unchanged. + +## Deprecation etiquette + +Nothing is removed, superseded, or deprecated. diff --git a/src/world/WorldAPI.lua b/src/world/WorldAPI.lua index 69b9eea2..a7abd4f3 100644 --- a/src/world/WorldAPI.lua +++ b/src/world/WorldAPI.lua @@ -117,6 +117,36 @@ function WorldAPI:current() facing = p and p.facing } end +local function validBlockCoordinate(value) + return type(value) == "number" and value == value + and value ~= math.huge and value ~= -math.huge + and value == math.floor(value) +end + +-- Read one block from the active Gen 1 map without exposing the mutable block +-- array. Requiring the expected map id makes a stale signature fail closed +-- if a warp or reload moved the player before the caller completed its check. +-- Block coordinates are zero-based, matching replaceBlock. +function WorldAPI:activeBlockAt(mapId, bx, by) + local ow = self:overworld() + if not ow or not ow.map then return nil, NO_OVERWORLD end + local map = ow.map + if map.id ~= mapId then return nil, "map is not active" end + if not validBlockCoordinate(bx) or not validBlockCoordinate(by) then + return nil, "invalid block coordinates" + end + local def = map.def + if not def or type(def.width) ~= "number" or type(def.height) ~= "number" + or bx < 0 or by < 0 or bx >= def.width or by >= def.height then + return nil, "block coordinates out of bounds" + end + local blockId = map:blockAt(bx, by) + if not validBlockCoordinate(blockId) or blockId < 0 then + return nil, "block unavailable" + end + return blockId +end + -- Companion UIs may offer party ordering while the player is in free roam. -- The same guard that makes opening a menu safe keeps scripts, transitions, -- movement and screens above the overworld from observing a mid-action swap. From c9221daa90d7fba96b4ba4a6443e4910499cc130 Mon Sep 17 00:00:00 2001 From: Bo Layer Date: Sun, 23 Aug 2026 03:18:17 -0600 Subject: [PATCH 2/4] fix(mod-api): fail closed on malformed map blocks --- docs/modding.md | 11 +++++++++-- docs/rfcs/0013-map-occupancy-and-active-block.md | 9 ++++++++- src/world/WorldAPI.lua | 14 ++++++++++---- 3 files changed, 27 insertions(+), 7 deletions(-) diff --git a/docs/modding.md b/docs/modding.md index 73c8d853..8687c7a2 100644 --- a/docs/modding.md +++ b/docs/modding.md @@ -272,12 +272,19 @@ mod.hooks:wrap("map.occupancy_allowed", function(next, game, ctx) end) ``` -With no wrapper, the hook allocates no context and vanilla behavior is byte-for- -byte unchanged. A throwing wrapper is isolated by the normal hook bus. A nil, +With no wrapper, the hook allocates no context and vanilla behavior is +unchanged. A throwing wrapper is isolated by the normal hook bus. A nil, string, number, table, or other malformed final answer fails closed. Disabling or uninstalling the permitting mod therefore restores vanilla ejection without changing the S.S. Anne story flag or restoring the ship. +Normal hook-chain ownership applies: a wrapper that does not call `next` +intentionally owns the final answer and does not run lower-priority wrappers. +Permission wrappers must call `next` as shown above to compose. A noncompliant +wrapper that returns false without calling `next` safely denies occupancy and +can suppress downstream permission by this standard rule. A malformed answer +also fails closed and cannot force occupancy. + ## Party ordering Companion UIs and alternate party screens can call diff --git a/docs/rfcs/0013-map-occupancy-and-active-block.md b/docs/rfcs/0013-map-occupancy-and-active-block.md index 1066ddc3..cfb796db 100644 --- a/docs/rfcs/0013-map-occupancy-and-active-block.md +++ b/docs/rfcs/0013-map-occupancy-and-active-block.md @@ -56,6 +56,12 @@ true when either downstream or its own narrow rule permits occupancy. The hook does not replace `MapScripts` registration, merging, or dispatch, and does not change the departure flag or reconstruct the ship. +As with every wrapper hook, a callback that does not call `next` intentionally +owns the final answer and does not run lower-priority callbacks. Permission +wrappers must call downstream to compose. A false, non-forwarding wrapper +safely denies occupancy and can suppress downstream permission by this normal +rule. A malformed final result also fails closed and cannot permit occupancy. + The call is guarded by `Runtime.wantsHook`, so an empty chain allocates no context and follows the prior branch exactly. @@ -108,7 +114,8 @@ The parity gate must prove: - one permission wrapper can allow occupancy without removing the ship-erasure work or any map handler; - multiple cooperative wrappers preserve downstream permission; -- absent, throwing, nil, false, and malformed hook answers fail closed; +- absent, throwing, nil, false, malformed, and non-forwarding hook answers fail + closed according to the normal wrapper-chain rule; - Red, Blue, and Yellow contexts keep their version identity separate; - disabling or removing the owner restores vanilla behavior without a save migration; diff --git a/src/world/WorldAPI.lua b/src/world/WorldAPI.lua index a7abd4f3..e204d635 100644 --- a/src/world/WorldAPI.lua +++ b/src/world/WorldAPI.lua @@ -136,12 +136,18 @@ function WorldAPI:activeBlockAt(mapId, bx, by) return nil, "invalid block coordinates" end local def = map.def - if not def or type(def.width) ~= "number" or type(def.height) ~= "number" - or bx < 0 or by < 0 or bx >= def.width or by >= def.height then + if not def or not validBlockCoordinate(def.width) or def.width <= 0 + or not validBlockCoordinate(def.height) or def.height <= 0 then + return nil, "block unavailable" + end + if bx < 0 or by < 0 or bx >= def.width or by >= def.height then return nil, "block coordinates out of bounds" end - local blockId = map:blockAt(bx, by) - if not validBlockCoordinate(blockId) or blockId < 0 then + if type(def.blocks) ~= "table" or type(map.blockAt) ~= "function" then + return nil, "block unavailable" + end + local ok, blockId = pcall(map.blockAt, map, bx, by) + if not ok or not validBlockCoordinate(blockId) or blockId < 0 then return nil, "block unavailable" end return blockId From 3b7fc428585ff4ce2361dfd29e16a25f489b983b Mon Sep 17 00:00:00 2001 From: Bo Layer Date: Sun, 23 Aug 2026 03:20:23 -0600 Subject: [PATCH 3/4] test(mod-api): gate Mew dock seams --- .../engine/mew_dock_private_artifact_gate.lua | 49 ++ tests/engine/mew_dock_seam_contract.lua | 428 ++++++++++++++++++ 2 files changed, 477 insertions(+) create mode 100644 tests/engine/mew_dock_private_artifact_gate.lua create mode 100644 tests/engine/mew_dock_seam_contract.lua diff --git a/tests/engine/mew_dock_private_artifact_gate.lua b/tests/engine/mew_dock_private_artifact_gate.lua new file mode 100644 index 00000000..1402aefd --- /dev/null +++ b/tests/engine/mew_dock_private_artifact_gate.lua @@ -0,0 +1,49 @@ +-- Static privacy gate for the narrow Mew dock engine branch. It checks the +-- Git publication set, not ignored local imports: user ROMs and progress +-- saves may exist on a developer machine but must never become tracked files. + +package.path = "./?.lua;./?/init.lua;" .. package.path +local T = require("tests.harness") + +local pipe = io.popen("git ls-files 2>" .. (package.config:sub(1, 1) == "\\" and "nul" or "/dev/null")) +local paths = {} +if pipe then + for path in pipe:lines() do paths[#paths + 1] = path:gsub("\\", "/") end + pipe:close() +end + +T.check(#paths > 100, + "privacy gate inspects a real Git publication set instead of passing vacuously") + +local forbiddenExtensions = { + gb = true, gbc = true, gba = true, sav = true, srm = true, + rom = true, z64 = true, v64 = true, n64 = true, nds = true, + sfc = true, smc = true, +} +local forbiddenRuntimeRoots = { + ["save.lua"] = true, + ["save_blue.lua"] = true, + ["save_yellow.lua"] = true, + ["save_gold.lua"] = true, + ["options.lua"] = true, +} + +local binaryLeaks, runtimeLeaks = {}, {} +for _, path in ipairs(paths) do + local lower = path:lower() + local ext = lower:match("%.([^./\\]+)$") + if forbiddenExtensions[ext] then binaryLeaks[#binaryLeaks + 1] = path end + if forbiddenRuntimeRoots[lower] + or lower:match("^saves/") + or lower:match("^imports/") + or lower:match("^mods%-data/") then + runtimeLeaks[#runtimeLeaks + 1] = path + end +end + +T.eq(#binaryLeaks, 0, + "tracked publication contains no ROM/save binaries: " .. table.concat(binaryLeaks, ", ")) +T.eq(#runtimeLeaks, 0, + "tracked publication contains no runtime save/import data: " .. table.concat(runtimeLeaks, ", ")) + +T.finish("mew dock private artifact gate") diff --git a/tests/engine/mew_dock_seam_contract.lua b/tests/engine/mew_dock_seam_contract.lua new file mode 100644 index 00000000..627dfca6 --- /dev/null +++ b/tests/engine/mew_dock_seam_contract.lua @@ -0,0 +1,428 @@ +-- Contract gate for the two narrow Gen 1 seams added for a composable +-- post-departure S.S. Anne dock mod. The suite is ROM-free: it drives the +-- real hook bus and WorldAPI against hand-written maps and save snapshots. + +package.path = "./?.lua;./?/init.lua;" .. package.path +love = love or require("tests.love_stub") + +local T = require("tests.harness") +local Hooks = require("src.mods.Hooks") +local Runtime = require("src.mods.Runtime") +local WorldAPI = require("src.world.WorldAPI") + +local oldHooks = Runtime.hooks +local oldTextBox = package.loaded["src.render.TextBox"] + +package.loaded["src.render.TextBox"] = { + new = function(_, text, done) return { text = text, done = done } end, +} + +local story = dofile("data/scripts/story3.lua") +local VERSIONS = { "red", "blue", "yellow" } + +local function newDock(version, wrappers) + local blocks, pushes, warps = {}, {}, {} + local rebuilds = 0 + local save = { + version = version, + flags = { EVENT_SS_ANNE_LEFT = true }, + marker = "save-must-not-change", + } + local map = { + id = "VERMILION_DOCK", + setBlock = function(_, bx, by, block) + blocks[bx .. "," .. by] = block + end, + renderer = { rebuild = function() rebuilds = rebuilds + 1 end }, + } + local game = { + save = save, + data = { text = { + _VermilionCitySailor1ShipSetSailText = "The ship set sail.", + } }, + stack = { push = function(_, value) pushes[#pushes + 1] = value end }, + } + local ow = { + map = map, + player = { cellX = 14, cellY = 2, facing = "down" }, + startWarpTo = function(_, ...) + warps[#warps + 1] = { ... } + end, + } + + local hooks = Hooks.new() + Runtime.hooks = hooks + for _, entry in ipairs(wrappers or {}) do + hooks:wrap("map.occupancy_allowed", entry.fn, entry.priority or 0, + entry.owner) + end + + story.VERMILION_DOCK.onEnter(game, ow) + if pushes[1] and pushes[1].done then pushes[1].done() end + return { + blocks = blocks, pushes = pushes, warps = warps, rebuilds = rebuilds, + save = save, map = map, game = game, ow = ow, hooks = hooks, + } +end + +local function allowedWrapper(owner, inspect) + return { + owner = owner, + fn = function(nextFn, game, ctx) + if inspect then inspect(game, ctx) end + local downstream = nextFn(game, ctx) + local ownClaim = true + return downstream == true or ownClaim == true + end, + } +end + +-- With no subscriber, the post-departure branch remains byte-for-byte +-- vanilla in effect: erase the ship, show its line, and eject the player. +do + local run = newDock("red") + T.eq(run.rebuilds, 1, "vanilla re-entry rebuilds the erased dock") + T.eq(run.blocks["5,1"], 1, "vanilla re-entry erases the upper hull") + T.eq(run.blocks["8,2"], 13, "vanilla re-entry erases the lower hull") + T.eq(#run.pushes, 1, "vanilla re-entry shows the ship-set-sail line") + T.eq(run.pushes[1].text, "The ship set sail.", + "vanilla re-entry preserves its dialogue") + T.eq(run.warps[1] and run.warps[1][1], "VERMILION_CITY", + "vanilla re-entry ejects to Vermilion City") +end + +-- The new permission seam is post-departure only. An ordinary HM01 exit +-- must not invoke it or change the existing departure-script path. +do + local hookCalls, queued = 0, nil + local hooks = Hooks.new() + Runtime.hooks = hooks + hooks:wrap("map.occupancy_allowed", function(nextFn, game, ctx) + hookCalls = hookCalls + 1 + return nextFn(game, ctx) + end, 0, "must_not_run") + + local savedMusic = package.loaded["src.core.Music"] + package.loaded["src.core.Music"] = { + stop = function() end, + play = function() end, + } + local game = { + save = { version = "red", flags = { EVENT_GOT_HM01 = true } }, + data = {}, + } + local ow = { + player = { cellX = 14, cellY = 2 }, + startDustAnim = function(_, _, _, done) if done then done() end end, + queueScript = function(_, rows) queued = rows end, + } + story.VERMILION_DOCK.onEnter(game, ow) + package.loaded["src.core.Music"] = savedMusic + + T.eq(hookCalls, 0, "normal HM01 departure never calls the occupancy seam") + T.eq(game.save.flags.EVENT_SS_ANNE_LEFT, true, + "normal HM01 departure still sets the vanilla event flag") + T.check(type(queued) == "table" and #queued > 0, + "normal HM01 departure still queues its sail-away script") +end + +-- One cooperative claimant may permit occupancy. The callback receives a +-- detached data snapshot, not the live overworld, map, player, or save. +do + local seenGame, seenCtx + local run = newDock("red", { allowedWrapper("mew_fixture", function(game, ctx) + seenGame = game + seenCtx = { + mapId = ctx.mapId, reason = ctx.reason, gameVersion = ctx.gameVersion, + x = ctx.x, y = ctx.y, + } + ctx.mapId, ctx.x, ctx.y = "MUTATED", -1, -1 + end) }) + T.eq(seenGame, run.game, "occupancy callback receives the live game explicitly") + T.same(seenCtx, { + mapId = "VERMILION_DOCK", reason = "ss_anne_departed", + gameVersion = "red", x = 14, y = 2, + }, "occupancy context is the exact detached Red dock snapshot") + T.eq(run.ow.map.id, "VERMILION_DOCK", "context mutation cannot change the map") + T.eq(run.ow.player.cellX, 14, "context mutation cannot change player X") + T.eq(run.ow.player.cellY, 2, "context mutation cannot change player Y") + T.eq(run.save.marker, "save-must-not-change", "permission check does not mutate save") + T.same(run.save, { + version = "red", flags = { EVENT_SS_ANNE_LEFT = true }, + marker = "save-must-not-change", + }, "permission check preserves the complete save snapshot") + T.eq(#run.pushes, 0, "an exact true suppresses the vanilla rejection dialog") + T.eq(#run.warps, 0, "an exact true permits post-departure dock occupancy") + T.eq(run.blocks["5,1"], 1, "permitted occupancy still erases the departed ship") + T.eq(run.rebuilds, 1, "permitted occupancy still rebuilds the water layout") +end + +-- Standard hook composition also means a non-cooperative false/no-next +-- wrapper can suppress downstream claims. This remains safe because false +-- is denial; it cannot accidentally grant occupancy. +do + local downstreamCalls = 0 + local run = newDock("red", { + { + owner = "denier", priority = 10, + fn = function() return false end, + }, + { + owner = "unreached_claimant", priority = 0, + fn = function() + downstreamCalls = downstreamCalls + 1 + return true + end, + }, + }) + T.eq(downstreamCalls, 0, "no-next denial suppresses downstream by hook semantics") + T.eq(run.warps[1] and run.warps[1][1], "VERMILION_CITY", + "non-cooperative false remains fail-closed") +end + +-- Cooperative peers all run through next(). A lower-priority peer claim is +-- preserved by a higher-priority peer that has no claim of its own. +do + local calls, contextIdentity = {}, nil + local run = newDock("blue", { + { + owner = "peer_high", priority = 10, + fn = function(nextFn, game, ctx) + calls[#calls + 1] = "high-before" + contextIdentity = ctx + local allowed = nextFn(game, ctx) + calls[#calls + 1] = "high-after" + return allowed == true or false + end, + }, + { + owner = "peer_low", priority = 0, + fn = function(nextFn, game, ctx) + calls[#calls + 1] = "low" + T.eq(ctx, contextIdentity, "peer wrappers share one detached snapshot instance") + local allowed = nextFn(game, ctx) + local ownClaim = true + return allowed == true or ownClaim == true + end, + }, + }) + T.same(calls, { "high-before", "low", "high-after" }, + "multiple peer handlers preserve hook-chain order") + T.eq(#run.warps, 0, "a cooperative peer claim survives the whole chain") +end + +-- Absent, throwing, or malformed callbacks fail closed. Only boolean true +-- can turn off ejection; truthy strings/tables/numbers do not grant access. +do + local malformed = { + { label = "nil", value = nil }, + { label = "false", value = false }, + { label = "string", value = "yes" }, + { label = "number", value = 1 }, + { label = "table", value = {} }, + } + for _, case in ipairs(malformed) do + local run = newDock("red", { { + owner = "malformed_" .. case.label, + fn = function() return case.value end, + } }) + T.eq(run.warps[1] and run.warps[1][1], "VERMILION_CITY", + "malformed " .. case.label .. " permission fails closed") + end + + local beforeNext = newDock("red", { { + owner = "throws_before_next", + fn = function() error("fixture throws before next", 0) end, + } }) + T.eq(beforeNext.warps[1] and beforeNext.warps[1][1], "VERMILION_CITY", + "throwing callback before next fails closed") + + local afterNext = newDock("red", { { + owner = "throws_after_next", + fn = function(nextFn, game, ctx) + nextFn(game, ctx) + error("fixture throws after next", 0) + end, + } }) + T.eq(afterNext.warps[1] and afterNext.warps[1][1], "VERMILION_CITY", + "throwing callback after next keeps the downstream denial") +end + +-- The context carries one version only. A Red-only claimant must not leak +-- access into Blue or Yellow, and each call gets its own snapshot. +do + local contexts = {} + for _, version in ipairs(VERSIONS) do + local run = newDock(version, { { + owner = "red_only", + fn = function(nextFn, game, ctx) + contexts[#contexts + 1] = ctx + local downstream = nextFn(game, ctx) + return downstream == true or ctx.gameVersion == "red" + end, + } }) + T.eq(#run.warps == 0, version == "red", + version .. " occupancy is decided only by its own version context") + end + T.eq(contexts[1].gameVersion, "red", "Red context stays Red") + T.eq(contexts[2].gameVersion, "blue", "Blue context stays Blue") + T.eq(contexts[3].gameVersion, "yellow", "Yellow context stays Yellow") + T.check(contexts[1] ~= contexts[2] and contexts[2] ~= contexts[3], + "Red, Blue, and Yellow calls do not share context tables") +end + +-- Removing the owner is the engine's disable/uninstall path. It restores +-- vanilla denial immediately and leaves no save flag or serialized state. +do + local run = newDock("yellow", { allowedWrapper("removable") }) + T.eq(#run.warps, 0, "installed owner may grant Yellow dock occupancy") + run.hooks:removeOwner("removable") + local pushes, warps = {}, {} + run.game.stack.push = function(_, value) pushes[#pushes + 1] = value end + run.ow.startWarpTo = function(_, ... ) warps[#warps + 1] = { ... } end + story.VERMILION_DOCK.onEnter(run.game, run.ow) + if pushes[1] and pushes[1].done then pushes[1].done() end + T.eq(warps[1] and warps[1][1], "VERMILION_CITY", + "disabling the owner restores vanilla ejection") + T.eq(run.hooks.chains["map.occupancy_allowed"], nil, + "uninstall removes the occupancy chain itself") + T.eq(run.save.marker, "save-must-not-change", + "disable/uninstall writes no persistent permission state") + T.same(run.save, { + version = "yellow", flags = { EVENT_SS_ANNE_LEFT = true }, + marker = "save-must-not-change", + }, "disable/uninstall preserves the complete Yellow save snapshot") +end + +-- activeBlockAt is read-only and fail-closed. A successful call exposes +-- only one scalar from the active runtime layout, never its backing table. +local function blockApi(version, blockAt) + local backing = { 4, 5, 6, 8, 9, 10 } + local map = { + id = "VERMILION_DOCK", + def = { width = 3, height = 2, blocks = backing }, + blockAt = blockAt or function(_, bx, by) + return backing[by * 3 + bx + 1] + end, + } + local world = { isOverworld = true, map = map } + local game = { + save = { version = version }, + stack = { states = { world } }, + overworld = world, + } + return WorldAPI.new(game, "contract_fixture"), backing, game, map +end + +do + for _, version in ipairs(VERSIONS) do + local api, backing = blockApi(version) + local block, err = api:activeBlockAt("VERMILION_DOCK", 1, 0) + T.eq(block, 5, version .. " reads its active dock block") + T.eq(err, nil, version .. " valid active block has no error") + block = 99 + T.eq(backing[2], 5, version .. " scalar result cannot mutate the map") + end + + local api = blockApi("red") + local wrong, wrongErr = api:activeBlockAt("VERMILION_CITY", 1, 0) + T.eq(wrong, nil, "wrong map has no block result") + T.eq(wrongErr, "map is not active", "wrong map fails closed explicitly") + + local invalid = { + { "nil x", nil, 0 }, { "string x", "1", 0 }, { "table x", {}, 0 }, + { "fraction x", 0.5, 0 }, { "negative infinity x", -math.huge, 0 }, + { "infinity y", 0, math.huge }, { "NaN y", 0, 0 / 0 }, + } + for _, case in ipairs(invalid) do + local value, err = api:activeBlockAt("VERMILION_DOCK", case[2], case[3]) + T.eq(value, nil, case[1] .. " returns no block") + T.eq(err, "invalid block coordinates", case[1] .. " is rejected by type") + end + for _, coords in ipairs({ { -1, 0 }, { 0, -1 }, { 3, 0 }, { 0, 2 } }) do + local value, err = api:activeBlockAt("VERMILION_DOCK", coords[1], coords[2]) + T.eq(value, nil, "out-of-bounds coordinate returns no block") + T.eq(err, "block coordinates out of bounds", "bounds fail closed explicitly") + end +end + +do + local malformed = { + { label = "nil", get = function() return nil end }, + { label = "negative", get = function() return -1 end }, + { label = "fractional", get = function() return 1.5 end }, + { label = "infinite", get = function() return math.huge end }, + { label = "NaN", get = function() return 0 / 0 end }, + { label = "string", get = function() return "4" end }, + { label = "table", get = function() return {} end }, + { label = "throwing", get = function() error("bad map", 0) end }, + } + for _, case in ipairs(malformed) do + local api = blockApi("red", case.get) + local block, err = api:activeBlockAt("VERMILION_DOCK", 0, 0) + T.eq(block, nil, "malformed active block " .. case.label .. " returns no value") + T.eq(err, "block unavailable", + "malformed active block " .. case.label .. " fails closed") + end +end + + +-- Invalid map shapes are untrusted runtime data too. None may escape as a +-- block or raise through the mod facade. +do + local badDefs = { + { label = "missing def", value = nil }, + { label = "missing width", value = { height = 2, blocks = {} } }, + { label = "string width", value = { width = "3", height = 2, blocks = {} } }, + { label = "fractional width", value = { width = 1.5, height = 2, blocks = {} } }, + { label = "nonpositive width", value = { width = 0, height = 2, blocks = {} } }, + { label = "infinite height", value = { width = 3, height = math.huge, blocks = {} } }, + { label = "missing blocks", value = { width = 3, height = 2 } }, + { label = "scalar blocks", value = { width = 3, height = 2, blocks = 4 } }, + } + for _, case in ipairs(badDefs) do + local api, _, _, map = blockApi("red") + map.def = case.value + local block, err = api:activeBlockAt("VERMILION_DOCK", 0, 0) + T.eq(block, nil, case.label .. " returns no block") + T.eq(err, "block unavailable", case.label .. " fails closed") + end + + local api, _, _, map = blockApi("red") + map.blockAt = nil + local block, err = api:activeBlockAt("VERMILION_DOCK", 0, 0) + T.eq(block, nil, "missing blockAt returns no block") + T.eq(err, "block unavailable", "missing blockAt fails closed") + map.blockAt = "not a function" + block, err = api:activeBlockAt("VERMILION_DOCK", 0, 0) + T.eq(block, nil, "malformed blockAt returns no block") + T.eq(err, "block unavailable", "malformed blockAt fails closed") + + api, _, _, map = blockApi("red") + map.def.blocks = {} + block, err = api:activeBlockAt("VERMILION_DOCK", 0, 0) + T.eq(block, nil, "sparse stored block slot returns no block") + T.eq(err, "block unavailable", "sparse stored block slot fails closed") + + api, _, _, map = blockApi("red") + map.def.blocks[1] = "4" + block, err = api:activeBlockAt("VERMILION_DOCK", 0, 0) + T.eq(block, nil, "malformed stored block slot returns no block") + T.eq(err, "block unavailable", "malformed stored block slot fails closed") + + api, _, _, map = blockApi("red", function() return 5 end) + block, err = api:activeBlockAt("VERMILION_DOCK", 0, 0) + T.eq(block, nil, "stored/accessor mismatch returns no block") + T.eq(err, "block unavailable", "stored/accessor mismatch fails closed") +end + +do + local api = WorldAPI.new({ stack = { states = {} } }, "contract_fixture") + local block, err = api:activeBlockAt("VERMILION_DOCK", 0, 0) + T.eq(block, nil, "no-overworld lookup returns no block") + T.eq(err, "no overworld", "no-overworld lookup reports its state") +end + +Runtime.hooks = oldHooks +package.loaded["src.render.TextBox"] = oldTextBox +T.finish("mew dock seam contract") From d17f0725d80481e47b74ea406a4c4bccbd574295 Mon Sep 17 00:00:00 2001 From: Bo Layer Date: Sun, 23 Aug 2026 03:20:43 -0600 Subject: [PATCH 4/4] fix(mod-api): validate active block storage --- data/scripts/story3.lua | 4 ++-- docs/modding.md | 3 +++ docs/rfcs/0013-map-occupancy-and-active-block.md | 4 ++++ src/world/WorldAPI.lua | 7 ++++++- 4 files changed, 15 insertions(+), 3 deletions(-) diff --git a/data/scripts/story3.lua b/data/scripts/story3.lua index 787b0d99..826a7f96 100644 --- a/data/scripts/story3.lua +++ b/data/scripts/story3.lua @@ -913,8 +913,8 @@ M.VERMILION_DOCK = { -- the ship is long gone: erase her right away, and anyone who -- still lands here is sent back out past the guard unless a mod -- explicitly permits this occupied map state. This hook surrounds - -- only the ejection decision; the map's complete onEnter chain has - -- already run and the departed ship remains erased. + -- only the ejection decision; map-script registration and dispatch + -- stay unchanged, and the departed ship remains erased. for _, b in ipairs(DOCK_SHIP_BLOCKS) do ow.map:setBlock(b.bx, b.by, b.water) end diff --git a/docs/modding.md b/docs/modding.md index 8687c7a2..ab0ba7e9 100644 --- a/docs/modding.md +++ b/docs/modding.md @@ -247,6 +247,9 @@ or malformed active block returns `nil, "block unavailable"`. The caller must require every expected cell to match before changing presentation. This method is Gen 1-only; Gold callers receive no parity promise for it. +The same unavailable result covers missing or sparse active block storage and +an accessor result that does not match its validated active block slot. + ### Conditional map occupancy `map.occupancy_allowed` is a narrow Gen 1 hook around a map script's vanilla diff --git a/docs/rfcs/0013-map-occupancy-and-active-block.md b/docs/rfcs/0013-map-occupancy-and-active-block.md index cfb796db..64e6aa04 100644 --- a/docs/rfcs/0013-map-occupancy-and-active-block.md +++ b/docs/rfcs/0013-map-occupancy-and-active-block.md @@ -89,6 +89,10 @@ Failure reasons are stable: | Coordinate is negative or outside the active map | `block coordinates out of bounds` | | Active block data is absent or malformed | `block unavailable` | +The block slot and the active map accessor must both contain the same valid +nonnegative integer. Missing or sparse storage and inconsistent accessor data +return `block unavailable`. + Requiring the expected map ID and rejecting all ambiguous input lets a mod compare every cell in its version-specific signature before it calls an existing mutation API. Red, Blue, and Yellow each use their own loaded map diff --git a/src/world/WorldAPI.lua b/src/world/WorldAPI.lua index e204d635..bdab92a3 100644 --- a/src/world/WorldAPI.lua +++ b/src/world/WorldAPI.lua @@ -146,8 +146,13 @@ function WorldAPI:activeBlockAt(mapId, bx, by) if type(def.blocks) ~= "table" or type(map.blockAt) ~= "function" then return nil, "block unavailable" end + local stored = def.blocks[by * def.width + bx + 1] + if not validBlockCoordinate(stored) or stored < 0 then + return nil, "block unavailable" + end local ok, blockId = pcall(map.blockAt, map, bx, by) - if not ok or not validBlockCoordinate(blockId) or blockId < 0 then + if not ok or not validBlockCoordinate(blockId) or blockId < 0 + or blockId ~= stored then return nil, "block unavailable" end return blockId