Build the patch from modules instead of one inlined translation unit - #36
Merged
Merged
Conversation
Structs do not reach a module through `.include`: the declaration registers in the including translation unit, and a module compiled by build_with_imports is not that unit. Proved with a three-line probe - `.include` of the header fails the cast, `.import` of a module that declares it succeeds. dq6 has been doing the latter for a while; its config.s shares pools, structs and constants with every module that imports it. So items.i becomes item_layout.s, imported by its five consumers. That unblocks the rest of the module migration, where each module has to carry the headers it uses rather than inheriting them from ff4.s. Named item_layout rather than items because `.import "items"` resolves against the module paths and finds src/ingame/items.s, the patch file, which then fails to link against labels it never imported. Output-neutral: the IPS differs only in the BUILD_DATE stamp, which moves between any two builds of identical source.
The four rolling-menu surfaces - treasure, drops, key-item picker and the shop sell list - become imported modules rather than files ff4.s inlines. Each now names what it uses: `.import "item_layout"` for the struct, `.extern` for the trampolines it borrows from its neighbours. Two things do not travel through `.import`, and both cost a build to find: `.map` is per translation unit. A module that reserves from the SRAM pool or allocs into bank $20 needs those banks described where it can see them, so the maps now sit beside the pools they describe, in rolling_state.i and bank20.i. The linker dedupes identical decls. Constants shared by inlining had the same problem: INVENTORY_SCROLL_PIXELS_PER_FRAME lived in a .s that ff4.s included, so rolling_buffer.s becomes rolling_buffer.i and the modules that want the constant include it. The S001 suppressions come out with this: the struct arrives by import, so the lint resolves it the way the assembler does.
The engine dispatches into each profile's HDMA builder and the profiles call back into the engine, so the two sides import each other's names and the linker ties them together. Also pins down why the shared header had to be renamed: a module importing a bare name resolves it against its own directory first, so `.import "items"` from src/ingame picked up src/ingame/items.s - the ROM patch file, which declares no struct - and compiled it as the module. The failure surfaced far away, as an unknown struct type.
a816 1.1.0a33 resolves `.import` against the configured module paths at both sites that do the resolving, so a file no longer shadows a project-wide module with a same-named neighbour. The header can carry the name that belongs to it; `src/ingame/items.s`, the ROM patch file, is addressed as `ingame/items`.
places_names, new_game and credits are pure `.alloc at` patch files. Absolute placement resolves against the maps in scope, and a module does not inherit the patch-main's, so the ROM map moves into its own header for them to include; bank20.i takes it from there too rather than repeating the declaration. Each names the labels it reaches for: the window data next door, the text-module entry points behind the menu-text macro, and the asset symbols the build generates.
items_menu, the two rolling-menu patch files and the BG-scroll HDMA patches become imports. Each names the implementation labels its `.alloc at` sites jump to, plus the asset symbols the build generates. Macros are compile-time and do not link, so the patch files that expand `pad_nop` include libmz.i for it - the failure a missing macro produces says only `Build failed: 'pad_nop'`, with no file or line. Drops four `\$` escapes from a docstring in message.s while here. a816 parses docstrings with `ast.literal_eval`, so an invalid Python escape in one surfaces as `<unknown>:1: SyntaxWarning` on every build, pointing at nothing.
Everything ff4.s used to inline is now imported: the battle patch set (math, graphics, magic, commands, monsters, items, message, sram, the redraw writers, inventory rolling, the drop debug hook), the field-menu trampolines and minimal_vwf_patches. ff4.s keeps five includes, all headers: build config, the libmz macros, the rolling-buffer constants, the menu-text macro and the menus header. The trampolines needed one thing an `.extern` cannot express. A typed cast is compile-time, so `treasure_rolling.scroll_state` does not resolve through a declaration: the module binds its own view over the same address, which is what battle/inventory_rolling.s was already doing. Scopes, by contrast, do import by name - `.extern shops` brings the whole `shops.` namespace with it.
|
🎮 FF4 IPS Build Ready! Your patch has been built successfully! Download the IPS file from the artifacts: 📦 Artifact: The artifact will be available for 30 days. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ff4.s went from 50 imports and 32 includes to 77 imports and 5 includes. The five that remain are headers, which is where they belong: build config, the libmz macros, the rolling-buffer constants, the menu-text macro, the menus header.
Everything else is now a module that names what it uses, rather than a file that inherits everything because ff4.s happened to inline it first: the rolling menus and their engine, the battle patch set, the field-menu trampolines, the event-script patches, minimal_vwf_patches.
Four things do not travel through
.import, and each cost a build to find:.mapis per translation unit. A module placing code at an absolute address, or reserving from a pool, needs the mapping in scope. The ROM map moves tosrc/rom_map.i; the SRAM map sits beside the pool it describes inrolling_state.i.treasure_rolling.scroll_statecannot resolve through a declaration, so each module binds its own view over the same address - whichbattle/inventory_rolling.swas already doing. Scopes are different:.extern shopsbrings the whole namespace.pad_nopincludeslibmz.i. The failure says onlyBuild failed: 'pad_nop'- no file, no line.rolling_buffer.sbecamerolling_buffer.i.Requires a816 1.1.0a33, pinned here. Two fixes went in upstream for this:
.importwas resolving against the importing file's own directory, so a neighbour shadowed a project-wide module (manz/a816#119, manz/a816#120) -.import "items"fromsrc/ingame/compiledsrc/ingame/items.sand failed later with the shared module's structs simply missing.The five
S001suppressions come out. The struct now arrives by import, so the lint resolves it exactly as the assembler does; the noqa was masking the coupling this removes.Behaviour is unchanged - same IPS, bar the BUILD_DATE stamp that moves between any two builds. Verified with
build/objcleared.