From 00f6e3a7b4ee00e22a60126d2050305bc231e50a Mon Sep 17 00:00:00 2001 From: bryanthaboi Date: Thu, 6 Aug 2026 14:36:01 -0400 Subject: [PATCH] CLOSES #889 --- src/import/RomExtractor.lua | 24 ++ src/import/RomImporter.lua | 6 +- src/save_convert/GenSave.lua | 39 ++++ src/save_convert/MapContext.lua | 263 ++++++++++++++++++++++ src/save_convert/SaveConvert.lua | 11 +- tests/drivers/export_sav_bug889_test.lua | 51 +++++ tests/engine/save_import_retry_bug420.lua | 9 +- tests/engine/save_map_context_bug889.lua | 201 +++++++++++++++++ 8 files changed, 600 insertions(+), 4 deletions(-) create mode 100644 src/save_convert/MapContext.lua create mode 100644 tests/drivers/export_sav_bug889_test.lua create mode 100644 tests/engine/save_map_context_bug889.lua diff --git a/src/import/RomExtractor.lua b/src/import/RomExtractor.lua index 88cad0fa..d9b25c77 100644 --- a/src/import/RomExtractor.lua +++ b/src/import/RomExtractor.lua @@ -198,6 +198,12 @@ function RomExtractor:extractTilesets() out[constName] = { id = constName, source = ("ROM:Tilesets[%d]"):format(index - 1), + -- The raw Tilesets row, verbatim. A .sav export has to reproduce what + -- LoadTilesetHeader (engine/overworld/tilesets.asm) would have left in + -- wTilesetBank..wGrassTile, because a Continue never re-runs it -- see + -- src/save_convert/MapContext.lua (#889). Byte 12 is the tile + -- animation id, which rides in sTileAnimations. + header = self.rom:bytes(headers.bank, rowAddress, 12), image = "assets/generated/tilesets/" .. base .. ".png", imageWidth = spec.imageWidth, imageHeight = spec.imageHeight, @@ -268,6 +274,11 @@ function RomExtractor:extractMaps() assert(tilesetId < #tilesets, constName .. ": unknown tileset id") local blockPointer = self.rom:word(header.bank, address + 3) local connectionFlags = self.rom:byte(header.bank, address + 9) + -- wCurMapHeader verbatim (tileset, height, width, data/text/script + -- pointers, connection flags). A save restores this window instead of + -- rebuilding it, so an export has to carry the real bytes (#889). + local headerBytes = self.rom:bytes(header.bank, address, 10) + local connectionStart = address + 10 address = address + 10 local connections = {} @@ -289,6 +300,8 @@ function RomExtractor:extractMaps() end assert(bit.band(connectionFlags, 0xF0) == 0, constName .. ": unknown connection flags") + local connectionBytes = self.rom:bytes( + header.bank, connectionStart, address - connectionStart) local objectPointer = self.rom:word(header.bank, address) local objectAddress = objectPointer local borderBlock = self.rom:byte(header.bank, objectAddress) @@ -376,6 +389,17 @@ function RomExtractor:extractMaps() width = width, height = height, blocks = blocks, borderBlock = borderBlock, connections = connections, warps = warps, signs = signs, objects = objects, + -- Raw ROM bytes a .sav export replays through LoadMapHeader's WRAM + -- writes (src/save_convert/MapContext.lua, #889). Kept as the original + -- bytes rather than re-encoded from the decoded tables above: the + -- pointers in them (wCurMapDataPtr, the connection strip src/dest + -- addresses, sign text ids) have no equivalent in the port's own model. + sram = { + header = headerBytes, + connections = connectionBytes, + objects = self.rom:bytes( + header.bank, objectPointer, objectAddress - objectPointer), + }, } self:tick("Maps", mapIndex, #keys) end diff --git a/src/import/RomImporter.lua b/src/import/RomImporter.lua index 3544c7f4..71fa08aa 100644 --- a/src/import/RomImporter.lua +++ b/src/import/RomImporter.lua @@ -30,7 +30,11 @@ end -- Cache generation tag; bump to force every imported version to re-extract. -- v9: Yellow audio re-anchored on pokeyellow.sym (#522) -- stale caches -- carry Red's bank $1f header, wave-table, and CryData offsets. -local CACHE_FORMAT = "rom-cache-v9:" +-- v10: maps carry their raw map-header/connection/object bytes and tilesets +-- their Tilesets row (#889), which a .sav export replays so a Continue on +-- real hardware has a map to load; a v9 cache has none of them and exports +-- the same unbootable save as before. +local CACHE_FORMAT = "rom-cache-v10:" -- The completion marker is written under each version's cache prefix -- (rom-cache.complete for Red, blue/rom-cache.complete for Blue). local MARKER_PATH = "rom-cache.complete" diff --git a/src/save_convert/GenSave.lua b/src/save_convert/GenSave.lua index 6a465acf..0f754359 100644 --- a/src/save_convert/GenSave.lua +++ b/src/save_convert/GenSave.lua @@ -27,6 +27,7 @@ -- game regenerates all of it from wCurMap on the next map load anyway. local bit = require("bit") +local MapContext = require("src.save_convert.MapContext") local GenSave = {} @@ -1023,6 +1024,44 @@ function GenSave.encode(save, data, template) setByte(buf, O.lastMap, cw.mapsIndex[save.lastOutdoor.id] or 0) end + -- Current-map engine state (see src/save_convert/MapContext.lua). A + -- Continue restores this window from the save and never rebuilds it, so a + -- zero-filled one boots into a garbled map on a silent hang (#889). + -- + -- Rebuilt when there is no template at all (a save that began as a New Game + -- in this port), and when the template was saved on a DIFFERENT map than the + -- one the player is standing on now -- an imported save that has since been + -- played carries the old map's header, which is just as unbootable. A + -- template still on its own map keeps its bytes untouched: they are the + -- game's own, including live NPC positions, and preserving them is what + -- makes import -> export byte-identical. + local mapId = save.player and save.player.map + if mapId then + local rebuild = true + if src then + -- compare the way the byte was written (masked), and rebuild when the + -- map has no index at all rather than trusting a stale template + local index = cw.mapsIndex[mapId] + rebuild = index == nil or u8(src, O.curMap) ~= bit.band(index, 0xFF) + end + if rebuild then + local ctx = MapContext.build(data, mapId, + (save.player and save.player.x) or 0, (save.player and save.player.y) or 0) + if ctx then + for offset, values in pairs(ctx.writes) do + for i, value in ipairs(values) do + setByte(buf, O.mainData + offset + i - 1, value) + end + end + for i, value in ipairs(ctx.spriteData) do + setByte(buf, O.spriteData + i - 1, value) + end + -- sTileAnimations, the byte between sCurBoxData and the checksum + setByte(buf, O.checksumEnd - 1, ctx.tileAnimations) + end + end + end + -- play time: split save.playTime (seconds) back into H/M/S/F. The real -- game freezes the clock at 255h and sets wPlayTimeMaxed once past it, so -- mirror that cap rather than letting hours overflow a single byte. diff --git a/src/save_convert/MapContext.lua b/src/save_convert/MapContext.lua new file mode 100644 index 00000000..0443a992 --- /dev/null +++ b/src/save_convert/MapContext.lua @@ -0,0 +1,263 @@ +-- MapContext -- the current map's engine state, as a vanilla Gen1 save has to +-- carry it (#889). +-- +-- Continuing a real save never re-loads the map header. LoadMainData +-- (engine/menus/save.asm) copies sMainData back into WRAM and then sets +-- BIT_NO_PREVIOUS_MAP in wCurMapTileset; LoadMapHeader (home/overworld.asm) +-- clears that bit and RETURNS IMMEDIATELY when it was set, so every byte it +-- would otherwise have written -- the map header, connections, warps, signs, +-- sprites, the tileset header, the map's music -- comes straight out of the +-- save file. A cartridge save always has them because the game saved them. +-- +-- An export from this port did not: GenSave models the player's progress, not +-- the engine scratch around it, and left that whole window zero-filled unless +-- it had an imported SRAM image to copy from. The game then continued into +-- tileset 0 with a $0000 map-data pointer and sound id 0, which is the +-- garbled overworld / silent hang users reported after exporting a save that +-- began as a New Game in the port. +-- +-- This module rebuilds the window from the extracted ROM data, replaying the +-- same writes LoadMapHeader makes, so a port-origin save boots on hardware. +-- The offsets are all relative to wMainDataStart (= sMainData in a .sav) and +-- were summed from ram/wram.asm, then checked byte-for-byte against a real +-- cartridge save: rebuilding its current map reproduces the bytes it already +-- held, everywhere except the stale tails of disabled connection slots (which +-- the engine never reads once the connected-map byte is $FF). +-- +-- Pure Lua, no love.*: shared by the runtime exporter, the CLI and the tests. + +local MapContext = {} + +-- wMainDataStart-relative offsets. Anchors: wCurMap is +103 (the offset +-- GenSave already decodes the player's map from), and the run from there is +-- wCurMap, wCurrentTileBlockMapViewPointer(2), wYCoord, wXCoord, wYBlockCoord, +-- wXBlockCoord, wLastMap, wUnusedLastMapWidth, wCurMapHeader -- so the header +-- lands at +112, and every later label follows from the declaration sizes in +-- ram/wram.asm (MAX_WARP_EVENTS 32, MAX_BG_EVENTS 16, MAX_OBJECT_EVENTS 16, +-- SPRITE_SET_LENGTH 11). +local O = { + mapMusicSoundID = 100, -- wMapMusicSoundID + mapMusicROMBank = 101, -- wMapMusicROMBank + viewPointer = 104, -- wCurrentTileBlockMapViewPointer (2) + yCoord = 106, + xCoord = 107, + yBlockCoord = 108, + xBlockCoord = 109, + curMapHeader = 112, -- wCurMapHeader (10) + connectionHeaders = 122, -- wNorth/South/West/EastConnectionHeader (4 x 11) + mapBackgroundTile = 182, + numberOfWarps = 183, + warpEntries = 184, -- MAX_WARP_EVENTS x 4 + numSigns = 441, + signCoords = 442, -- MAX_BG_EVENTS x 2 + signTextIDs = 474, -- MAX_BG_EVENTS + numSprites = 490, + mapSpriteData = 493, -- MAX_OBJECT_EVENTS x 2 + mapSpriteExtra = 525, -- MAX_OBJECT_EVENTS x 2 + currentMapHeight2 = 557, + currentMapWidth2 = 558, + tilesetHeader = 564, -- wTilesetBank .. wGrassTile (11) +} +MapContext.OFFSETS = O + +local MAX_WARP_EVENTS = 32 +local MAX_BG_EVENTS = 16 +local MAX_OBJECT_EVENTS = 16 +local CONNECTION_STRUCT = 11 -- map_connection_struct (macros/ram.asm) +local SPRITE_STRUCT = 16 -- one wSpriteStateData1/2 entry +local NUM_SPRITE_STRUCTS = 16 + +-- wOverworldMap, the tile-block map the view pointer indexes. Recovered from +-- the event_displacement formula below applied to a real save's coordinates, +-- and it is the same $C6E8 every Gen1 reference quotes. +local OVERWORLD_MAP = 0xC6E8 + +-- Audio header tables start at $4000 in each of the three audio banks, and +-- constants/music_constants.asm derives every song id arithmetically from +-- that base ("Song ids are calculated by address", music_const: +-- (Music_X - SFX_Headers_1) / 3). So the extracted header address IS the id, +-- and no separate MapSongBanks extraction is needed. +local SFX_HEADERS_BASE = 0x4000 + +local function soundId(address) + if type(address) ~= "number" then return nil end + local delta = address - SFX_HEADERS_BASE + if delta < 0 or delta % 3 ~= 0 then return nil end + local id = delta / 3 + if id > 0xFF then return nil end + return id +end + +-- LoadMapHeader parses the map's object data (data/maps/objects/*.asm) into +-- five separate WRAM arrays. Walk it exactly as the engine does; `bytes` is +-- the 1-based array of raw object-data bytes the extractor captured. +local function parseObjects(bytes) + local pos = 1 + local function take() + local b = bytes[pos] + pos = pos + 1 + return b + end + local out = { warps = {}, signCoords = {}, signTexts = {}, + spriteData = {}, spriteExtra = {}, sprites = {} } + + out.backgroundTile = take() + + local warpCount = take() + if warpCount == nil then return nil end + out.warpCount = warpCount + for _ = 1, math.min(warpCount, MAX_WARP_EVENTS) * 4 do + out.warps[#out.warps + 1] = take() + end + + local signCount = take() + if signCount == nil then return nil end + out.signCount = signCount + for _ = 1, math.min(signCount, MAX_BG_EVENTS) do + out.signCoords[#out.signCoords + 1] = take() -- Y + out.signCoords[#out.signCoords + 1] = take() -- X + out.signTexts[#out.signTexts + 1] = take() + end + + local spriteCount = take() + if spriteCount == nil then return nil end + out.spriteCount = spriteCount + for _ = 1, math.min(spriteCount, MAX_OBJECT_EVENTS) do + local picture, mapY, mapX = take(), take(), take() + local movement1, movement2, textId = take(), take(), take() + if textId == nil then return nil end + out.sprites[#out.sprites + 1] = { + picture = picture, mapY = mapY, mapX = mapX, movement1 = movement1, + } + -- wMapSpriteData: movement byte 2, then the text id with its trainer/item + -- flag bits masked off (LoadMapHeader's `and $3f`). + out.spriteData[#out.spriteData + 1] = movement2 + out.spriteData[#out.spriteData + 1] = textId % 0x40 + -- BIT_TRAINER ($40) is tested before BIT_ITEM ($80), as LoadMapHeader does + if textId % 0x80 >= 0x40 then -- trainer: class, then party id + out.spriteExtra[#out.spriteExtra + 1] = take() + out.spriteExtra[#out.spriteExtra + 1] = take() + elseif textId >= 0x80 then -- item ball: item id, second byte unused + out.spriteExtra[#out.spriteExtra + 1] = take() + out.spriteExtra[#out.spriteExtra + 1] = 0 + else + out.spriteExtra[#out.spriteExtra + 1] = 0 + out.spriteExtra[#out.spriteExtra + 1] = 0 + end + end + return out +end + +-- build(data, mapId, x, y) -> ctx, err +-- +-- ctx.writes [wMainDataStart-relative offset] = array of bytes +-- ctx.spriteData 512 bytes for sSpriteData (wSpriteStateData1 then 2) +-- ctx.tileAnimations the byte sTileAnimations holds +-- +-- Returns nil plus a reason when the data set predates the extractor fields +-- this needs (an older ROM cache), so callers can fall back rather than fail. +function MapContext.build(data, mapId, x, y) + local map = data and data.maps and data.maps[mapId] + if not map then return nil, "unknown map " .. tostring(mapId) end + local sram = map.sram + if not (sram and sram.header and sram.objects) then + return nil, "map cache has no saved-map bytes (re-import the ROM)" + end + x, y = math.floor(tonumber(x) or 0), math.floor(tonumber(y) or 0) + + local writes = {} + local header = sram.header + writes[O.curMapHeader] = header + local height, width = header[2], header[3] + local connectionFlags = header[10] + + -- Connections: LoadMapHeader writes $FF over all four connected-map bytes + -- and then copies 11 bytes per present direction, north/south/west/east. + -- The disabled slots' remaining bytes are never read. + local conn = {} + for i = 1, 4 * CONNECTION_STRUCT do conn[i] = 0 end + for slot = 0, 3 do conn[slot * CONNECTION_STRUCT + 1] = 0xFF end + local pos = 1 + for slot, flag in ipairs({ 0x08, 0x04, 0x02, 0x01 }) do + if math.floor(connectionFlags / flag) % 2 == 1 then + for i = 0, CONNECTION_STRUCT - 1 do + conn[(slot - 1) * CONNECTION_STRUCT + 1 + i] = (sram.connections or {})[pos + i] or 0 + end + pos = pos + CONNECTION_STRUCT + end + end + writes[O.connectionHeaders] = conn + + local parsed = parseObjects(sram.objects) + if not parsed then return nil, "malformed object data for " .. tostring(mapId) end + writes[O.mapBackgroundTile] = { parsed.backgroundTile } + writes[O.numberOfWarps] = { parsed.warpCount } + writes[O.warpEntries] = parsed.warps + writes[O.numSigns] = { parsed.signCount } + writes[O.signCoords] = parsed.signCoords + writes[O.signTextIDs] = parsed.signTexts + writes[O.numSprites] = { parsed.spriteCount } + writes[O.mapSpriteData] = parsed.spriteData + writes[O.mapSpriteExtra] = parsed.spriteExtra + + -- "map height/width in 2x2 tile blocks", doubled at the end of LoadMapHeader + writes[O.currentMapHeight2] = { (height * 2) % 256 } + writes[O.currentMapWidth2] = { (width * 2) % 256 } + + -- The tileset header, as predef LoadTilesetHeader would have copied it. + local tilesets = data.tilesets or {} + local tilesetDef = tilesets[map.tileset] + local tileAnimations = 0 + if tilesetDef and tilesetDef.header then + local row = {} + for i = 1, 11 do row[i] = tilesetDef.header[i] end + writes[O.tilesetHeader] = row + tileAnimations = tilesetDef.header[12] or 0 + end + + -- MapSongBanks: without it the game continues with sound id 0 and audio + -- bank 0, which is what actually hangs a Continue on a white screen. + local audio = data.audio + local songLabel = audio and audio.mapSongs and audio.mapSongs[mapId] + local song = songLabel and audio.songs and audio.songs[songLabel] + local id = song and soundId(song.address) + if id and song.bank then + writes[O.mapMusicSoundID] = { id } + writes[O.mapMusicROMBank] = { song.bank % 256 } + end + + -- Player position within its block, and the upper-left corner of the view. + -- The pointer is the same expression the warp_to tables are assembled with + -- (macros/coords.asm event_displacement). + writes[O.yBlockCoord] = { y % 2 } + writes[O.xBlockCoord] = { x % 2 } + local view = OVERWORLD_MAP + 7 + width + + (width + 6) * math.floor(y / 2) + math.floor(x / 2) + writes[O.viewPointer] = { view % 256, math.floor(view / 256) % 256 } + + -- sSpriteData. LoadMapHeader zeroes structs 1-15, sets their image index to + -- $ff (off screen until the engine places them), then fills in each map + -- object's picture id and its map Y/X plus movement byte 1. Struct 0 is the + -- player, which SpecialEnterMap rebuilds through ResetPlayerSpriteData. + local spriteData = {} + for i = 1, NUM_SPRITE_STRUCTS * SPRITE_STRUCT * 2 do spriteData[i] = 0 end + local data2 = NUM_SPRITE_STRUCTS * SPRITE_STRUCT + for slot = 1, NUM_SPRITE_STRUCTS - 1 do + spriteData[slot * SPRITE_STRUCT + 2 + 1] = 0xFF + end + for index, sprite in ipairs(parsed.sprites) do + local base = index * SPRITE_STRUCT + spriteData[base + 1] = sprite.picture + spriteData[data2 + base + 4 + 1] = sprite.mapY + spriteData[data2 + base + 5 + 1] = sprite.mapX + spriteData[data2 + base + 6 + 1] = sprite.movement1 + end + + return { + writes = writes, + spriteData = spriteData, + tileAnimations = tileAnimations, + } +end + +return MapContext diff --git a/src/save_convert/SaveConvert.lua b/src/save_convert/SaveConvert.lua index b4dc9d34..3df6122f 100644 --- a/src/save_convert/SaveConvert.lua +++ b/src/save_convert/SaveConvert.lua @@ -42,11 +42,17 @@ local DATA_MODULES = { moves = { "data.generated.moves", "data/generated/moves.lua" }, items = { "data.generated.items", "data/generated/items.lua" }, maps = { "data.generated.maps", "data/generated/maps.lua" }, + -- tilesets/audio are only read by src/save_convert/MapContext.lua, to + -- rebuild the current map's engine state on export (#889) + tilesets = { "data.generated.tilesets", "data/generated/tilesets.lua" }, + audio = { "data.generated.audio", "data/generated/audio.lua" }, charmap = { "src.save_convert.data.charmap", "src/save_convert/data/charmap.lua" }, eventFlags = { "src.save_convert.data.event_flags", "src/save_convert/data/event_flags.lua" }, toggleObjects = { "src.save_convert.data.toggle_objects", "src/save_convert/data/toggle_objects.lua" }, } +local OPTIONAL_MODULES = { tilesets = true, audio = true } + -- Yellow renumbers wEventFlags bits: pokeyellow's constants/event_constants.asm -- inserts events pokered does not have (the Jessie & James fights, catch -- training, the Officer Jenny Squirtle) and shifts the Mt Moon 3 / Silph Co @@ -132,7 +138,10 @@ local function ensureData(gameVersion) if not mod then local e mod, e = loadTable(spec[1], spec[2]) - if not mod then return nil, e end + -- tilesets/audio only sharpen the export (MapContext); a cache + -- without them still imports and exports, just without the + -- rebuilt map window, so they must not fail the whole load + if not mod and not OPTIONAL_MODULES[name] then return nil, e end end data[name] = mod end diff --git a/tests/drivers/export_sav_bug889_test.lua b/tests/drivers/export_sav_bug889_test.lua new file mode 100644 index 00000000..f258f318 --- /dev/null +++ b/tests/drivers/export_sav_bug889_test.lua @@ -0,0 +1,51 @@ +-- Driver (#889): export a .sav from a save that never came from a ROM import, +-- the case that used to write a save with no map context at all -- no map +-- header, no tileset header, sound id 0 -- so a real Game Boy continued into a +-- garbled map and hung on a white screen. +-- +-- Saves the game in two places (an interior and an outdoor map with +-- connections), exports each, and prints the bytes the engine reads back on +-- Continue so a human can eyeball them, plus the export path to load in an +-- emulator. +return function(game) + local U = dofile("tests/drivers/util.lua") + local SaveFileIO = require("src.import.SaveFileIO") + local SaveData = require("src.core.SaveData") + local GameVersion = require("src.core.GameVersion") + + local version = GameVersion.get() + local spots = { + { "REDS_HOUSE_2F", 3, 6 }, + { "PALLET_TOWN", 5, 6 }, + } + + for _, spot in ipairs(spots) do + local map, x, y = spot[1], spot[2], spot[3] + U.teleport(game, map, x, y, "down") + U.wait(30) + -- the same sync the in-game SAVE does before writing (OverworldState: + -- captureSave), so the slot on disk holds the position we just walked to + local top = game.stack:top() + if top and top.captureSave then top:captureSave(game.save) end + SaveData.save(game.save) + local ok, pathOrErr = SaveFileIO.exportActiveSlot(version) + if not ok then + U.log(("export_sav_bug889: %s FAILED: %s"):format(map, tostring(pathOrErr))) + else + local bytes = love.filesystem.read( + ("exports/%s/gen1recomp-%s-%s.sav"):format( + version, version, SaveData.activeSlot(version) or "save")) + local main = 0x2598 + 11 + local function u8(off) return bytes:byte(main + off + 1) end + local hdr = {} + for i = 0, 9 do hdr[#hdr + 1] = ("%02X"):format(u8(112 + i)) end + U.log(("export_sav_bug889: %s -> %s"):format(map, pathOrErr)) + U.log((" wCurMap=%02X wCurMapHeader=%s music=%02X/%02X tilesetBank=%02X"): + format(u8(103), table.concat(hdr, " "), u8(100), u8(101), u8(564))) + end + end + + U.log("export_sav_bug889: done") + love.event.quit() + while true do coroutine.yield() end +end diff --git a/tests/engine/save_import_retry_bug420.lua b/tests/engine/save_import_retry_bug420.lua index 0ae47ec1..46b955e3 100644 --- a/tests/engine/save_import_retry_bug420.lua +++ b/tests/engine/save_import_retry_bug420.lua @@ -131,7 +131,12 @@ package.loaded["src.import.CacheFs"] = fakeCache local SaveConvert = require("src.save_convert.SaveConvert") -local GENERATED = { "pokemon", "moves", "items", "maps" } +-- tilesets/audio joined the set with the #889 map-context rebuild, which +-- reads the current map's tileset row and song out of the same cache. +-- The audio entry is single-quoted on purpose: gate_meta_coverage.lua treats a +-- double-quoted registry name anywhere in the test corpus as that registry's +-- unit test, and this suite is not the mod audio registry's. +local GENERATED = { "pokemon", "moves", "items", "maps", "tilesets", 'audio' } local function prefixes() local seen = {} @@ -148,7 +153,7 @@ do name .. " comes out of Blue's cache, not the un-prefixed read path") end eq(prefixes()[GameVersion.VERSIONS.blue.cachePrefix], #GENERATED, - "all four generated tables are read under Blue's cache prefix") + "every generated table is read under Blue's cache prefix") eq(fakeCache.prefix, SENTINEL, "CacheFs.prefix is launcher-owned state and is put back after the read") check(data and data.eventFlags ~= nil, diff --git a/tests/engine/save_map_context_bug889.lua b/tests/engine/save_map_context_bug889.lua new file mode 100644 index 00000000..f0b9f6f3 --- /dev/null +++ b/tests/engine/save_map_context_bug889.lua @@ -0,0 +1,201 @@ +-- #889: a .sav exported from a save that never came from a ROM import used to +-- carry no current-map state at all. A Continue restores that window from the +-- save and never rebuilds it (LoadMainData sets BIT_NO_PREVIOUS_MAP and +-- LoadMapHeader returns early on it), so the game continued into tileset 0, +-- a $0000 map-data pointer and sound id 0 -- a garbled map and a silent hang +-- on real hardware. +-- +-- src/save_convert/MapContext.lua replays LoadMapHeader's WRAM writes from the +-- extracted ROM bytes instead. This suite pins the layout it writes, on a +-- synthetic map whose header/object bytes are chosen so every field is +-- distinguishable, and then checks the encoder's three cases: no template +-- (rebuild), a template saved on the same map (leave the game's own bytes +-- alone, which is what keeps import -> export byte-identical), and a template +-- saved on a different map (rebuild, or the export carries the wrong map's +-- header). +package.path = "./?.lua;./?/init.lua;" .. package.path + +local T = require("tests.modkit") +local MapContext = require("src.save_convert.MapContext") +local GenSave = require("src.save_convert.GenSave") + +local O = MapContext.OFFSETS +local SAVE = GenSave.OFFSETS + +-- ---------------------------------------------------------------- fixtures + +-- tileset 0, 4 blocks tall, 5 wide, data/text/script pointers, north|west +local HEADER = { 0x00, 0x04, 0x05, 0x21, 0x43, 0x78, 0x56, 0x89, 0x67, 0x0A } +-- two 11-byte map_connection_structs, north first then west (the order +-- LoadMapHeader copies them in), each tagged with its own filler +local CONNECTIONS = { + 0x11, 0xA1, 0xA2, 0xA3, 0xA4, 0xA5, 0xA6, 0xA7, 0xA8, 0xA9, 0xAA, + 0x22, 0xB1, 0xB2, 0xB3, 0xB4, 0xB5, 0xB6, 0xB7, 0xB8, 0xB9, 0xBA, +} +local OBJECTS = { + 0x0E, -- wMapBackgroundTile + 2, -- warps + 0x04, 0x05, 0x00, 0x26, + 0x06, 0x07, 0x01, 0x27, + 2, -- signs + 0x08, 0x09, 0x03, -- Y, X, text id + 0x0A, 0x0B, 0x04, + 3, -- sprites + 0x01, 0x14, 0x15, 0xFF, 0xD0, 0x05, -- plain NPC + 0x02, 0x16, 0x17, 0xFE, 0x01, 0x47, -- trainer ($40): class, party + 0x33, 0x44, + 0x03, 0x18, 0x19, 0xFF, 0xD3, 0x85, -- item ball ($80): item id + 0x14, +} +local TILESET_HEADER = { 0x0C, 0x11, 0x40, 0x22, 0x40, 0x33, 0x40, + 0x44, 0x55, 0x66, 0x77, 0x02 } + +local function fixtureData() + local maps = {} + for id, map in pairs(dofile("tests/fixture_data/maps.lua")) do + local copy = {} + for k, v in pairs(map) do copy[k] = v end + maps[id] = copy + end + maps.FIX_TOWN.sram = { + header = HEADER, connections = CONNECTIONS, objects = OBJECTS, + } + return { + maps = maps, + pokemon = dofile("tests/fixture_data/pokemon.lua"), + moves = dofile("tests/fixture_data/moves.lua"), + items = dofile("tests/fixture_data/items.lua"), + tilesets = { [maps.FIX_TOWN.tileset] = { header = TILESET_HEADER } }, + audio = { + -- Song ids are computed from the header address + -- (constants/music_constants.asm): (address - $4000) / 3. + mapSongs = { FIX_TOWN = "Music_Fixture" }, + songs = { Music_Fixture = { address = 0x4000 + 3 * 0xBD, bank = 2 } }, + }, + } +end + +local data = fixtureData() + +-- ------------------------------------------------------------ the window + +local ctx = assert(MapContext.build(data, "FIX_TOWN", 7, 4)) +local w = ctx.writes + +local function eqBytes(got, want, msg) + got = got or {} + T.eq(#got, #want, msg .. " (length)") + for i = 1, #want do + T.eq(got[i], want[i], ("%s (byte %d)"):format(msg, i)) + end +end + +eqBytes(w[O.curMapHeader], HEADER, "wCurMapHeader is the ROM header verbatim") + +-- north and west are present; south and east must read $FF, or LoadTileBlockMap +-- walks a connection that is not there +local conn = w[O.connectionHeaders] +T.eq(#conn, 44, "all four connection structs are written") +T.eq(conn[1], 0x11, "north takes the first connection struct") +T.eq(conn[11], 0xAA, "north keeps all 11 of its bytes") +T.eq(conn[12], 0xFF, "south is disabled with $FF") +T.eq(conn[23], 0x22, "west takes the second connection struct") +T.eq(conn[34], 0xFF, "east is disabled with $FF") + +eqBytes(w[O.mapBackgroundTile], { 0x0E }, "wMapBackgroundTile") +eqBytes(w[O.numberOfWarps], { 2 }, "wNumberOfWarps") +eqBytes(w[O.warpEntries], { 0x04, 0x05, 0x00, 0x26, 0x06, 0x07, 0x01, 0x27 }, + "wWarpEntries are the raw 4-byte rows") +eqBytes(w[O.numSigns], { 2 }, "wNumSigns") +eqBytes(w[O.signCoords], { 0x08, 0x09, 0x0A, 0x0B }, + "wSignCoords are split out of the 3-byte sign rows") +eqBytes(w[O.signTextIDs], { 0x03, 0x04 }, "wSignTextIDs") +eqBytes(w[O.numSprites], { 3 }, "wNumSprites") +-- movement byte 2 and the text id with its flag bits masked ("and $3f") +eqBytes(w[O.mapSpriteData], { 0xD0, 0x05, 0x01, 0x07, 0xD3, 0x05 }, + "wMapSpriteData is (movement byte 2, text id & $3f) per sprite") +eqBytes(w[O.mapSpriteExtra], { 0, 0, 0x33, 0x44, 0x14, 0 }, + "wMapSpriteExtraData holds trainer class/party and item id") +eqBytes(w[O.currentMapHeight2], { 8 }, "map height doubled into 2x2 blocks") +eqBytes(w[O.currentMapWidth2], { 10 }, "map width doubled into 2x2 blocks") + +local tilesetRow = {} +for i = 1, 11 do tilesetRow[i] = TILESET_HEADER[i] end +eqBytes(w[O.tilesetHeader], tilesetRow, + "wTilesetBank..wGrassTile is the Tilesets row (11 bytes)") +T.eq(ctx.tileAnimations, 0x02, "the 12th tileset byte rides in sTileAnimations") + +eqBytes(w[O.mapMusicSoundID], { 0xBD }, "the song id is derived from its header address") +eqBytes(w[O.mapMusicROMBank], { 2 }, "the song's audio bank comes with it") + +-- x=7,y=4: block coords are the odd/even halves, and the view pointer is +-- macros/coords.asm event_displacement over the map width. +eqBytes(w[O.yBlockCoord], { 0 }, "wYBlockCoord is y & 1") +eqBytes(w[O.xBlockCoord], { 1 }, "wXBlockCoord is x & 1") +local view = 0xC6E8 + 7 + 5 + (5 + 6) * 2 + 3 +eqBytes(w[O.viewPointer], { view % 256, math.floor(view / 256) }, + "wCurrentTileBlockMapViewPointer") + +-- sSpriteData: picture ids in structs 1..3 of page 1, positions in page 2, +-- and every non-player struct's image index disabled at $ff +T.eq(#ctx.spriteData, 512, "sSpriteData is the full 512-byte window") +T.eq(ctx.spriteData[16 + 1], 0x01, "sprite 1 picture id") +T.eq(ctx.spriteData[16 + 3], 0xFF, "sprite 1 image index starts disabled") +T.eq(ctx.spriteData[256 + 16 + 5], 0x14, "sprite 1 map Y") +T.eq(ctx.spriteData[256 + 16 + 6], 0x15, "sprite 1 map X") +T.eq(ctx.spriteData[256 + 16 + 7], 0xFF, "sprite 1 movement byte 1") +T.eq(ctx.spriteData[3 * 16 + 1], 0x03, "sprite 3 picture id") +T.eq(ctx.spriteData[1], 0, "the player's struct is left to ResetPlayerSpriteData") + +-- a map the cache has no bytes for degrades instead of raising +T.eq(MapContext.build(data, "FIX_ROUTE", 0, 0), nil, + "a map with no extracted bytes returns nil, not an error") + +-- ------------------------------------------------------- through the codec + +GenSave.setCharmap(loadfile("src/save_convert/data/charmap.lua")()) + +local save = { + player = { name = "RED", rival = "BLUE", map = "FIX_TOWN", x = 7, y = 4 }, + money = 3000, inventory = {}, pokedex = { seen = {}, owned = {} }, + flags = {}, party = {}, boxes = {}, +} + +local function byteAt(bytes, mainOffset) + return bytes:byte(SAVE.mainData + mainOffset + 1) +end + +local fresh = GenSave.encode(save, data, nil) +T.eq(byteAt(fresh, O.curMapHeader + 1), 0x04, + "a templateless export carries the map header") +T.eq(byteAt(fresh, O.tilesetHeader), 0x0C, + "a templateless export carries the tileset header") +T.eq(byteAt(fresh, O.mapMusicSoundID), 0xBD, + "a templateless export carries the map's music, not sound id 0") +T.eq(fresh:byte(SAVE.checksumEnd - 1 + 1), 0x02, + "sTileAnimations is written before the checksum covers it") +T.eq(GenSave.mainChecksumValid(fresh), true, + "the rebuilt window is inside a valid main-data checksum") + +-- a template still on its own map keeps the game's own bytes: those include +-- live NPC positions, and preserving them is the round-trip invariant +local template = {} +for i = 1, #fresh do template[i] = fresh:sub(i, i) end +template[SAVE.mainData + O.curMapHeader + 1] = string.char(0x99) +template[SAVE.spriteData + 17] = string.char(0x77) +local sameMap = GenSave.encode(save, data, table.concat(template)) +T.eq(byteAt(sameMap, O.curMapHeader), 0x99, + "a template saved on this map keeps its map header untouched") +T.eq(sameMap:byte(SAVE.spriteData + 17), 0x77, + "a template saved on this map keeps its live sprite data") + +-- a template saved somewhere else is stale: it holds the OTHER map's header, +-- which is exactly as unbootable as an empty one +template[SAVE.mainData + SAVE.curMap - SAVE.mainData + 1] = nil +local other = {} +for i = 1, #fresh do other[i] = fresh:sub(i, i) end +other[SAVE.curMap + 1] = string.char(0xFE) -- some map this save is not on +other[SAVE.mainData + O.curMapHeader + 1] = string.char(0x99) +local moved = GenSave.encode(save, data, table.concat(other)) +T.eq(byteAt(moved, O.curMapHeader), HEADER[1], + "a template saved on another map is rebuilt for the map the save is on")