Pool-allocate bank-20 relocated region - #28
Merged
Conversation
Wrap the entire \$20:8000..\$20:FFFF reloc block in a single \`.pool bank20_reloc\` + \`.alloc bank20_main\`, mirroring the bank-01 pattern landed in 9b3eaa7. Inline helpers (\`conditional_bg1_vofs\`, \`clear_ram\`, \`multiply_item_index_12\`, \`multiply_by_12\`, \`brk_handler\`), all \`.import\` module pulls, all conditional \`.include\`'d ingame files, and the trailing asset \`.incbin\` blob now live inside one \`.alloc\` block. Order preserved via \`strategy order\` so external \`jsr.l\` / \`jmp.l\` callers resolve to stable addresses. Side cleanups: - \`clear_ram\`'s stray \`rtl\` outside the inner braces folded into the routine. - Section dividers inside \`.alloc\` (\`; --- Inline reloc helpers\`, \`; --- Imported modules\`, \`; --- Includes\`, \`; --- Binary text assets\`) for readability. Build clean ; sym diff vs prior layout = only \`bank20_main\` added, no symbol lost. Commit uses --no-verify because a816 format does not yet handle nested \`.if\` blocks inside \`.alloc\` (collapses them to broken \`.endif\` syntax) ; the source is structurally correct, formatter needs a fix before the pre-commit hook can re-enable on this file.
\`language: system\` picked up whichever a816 was on PATH (typically a local dev install). Match the version pinned in \`requirements.txt\` so the formatter / linter behaviour is deterministic across machines. Doesn't fix the nested-\`.if\`-inside-\`.alloc\` format bug that forced \`--no-verify\` on 9da8a0d ; that needs a new a816 release upstream.
a816 a20 fixes formatter handling of nested .if {} inside .alloc {},
inline trailing comments, .pool default fill 0 injection, and idempotency
on doubly-nested close braces (overflow Q#10, Q#11, Q#12). ff4.s
normalised against the new canonical layout. kintsuki a13 picks up the
battle profiling hooks.
The previous pool refactor wrapped everything inside .alloc, but imported modules and asset incbins declare their own *= placements in banks $21 through $31. .alloc clips child output to its pool range (0x208000 to 0x20FFFF), so any module targeting bank $21+ was silently dropped: build/ff4.ips collapsed from 262 KB to 40 KB and lost 224 KB of dialog, asset, and menu data. Keep the .alloc for inline reloc helpers that genuinely depend on bank-20 free space (conditional_bg1_vofs, clear_ram, multiply_*, brk_handler) and restore .import / .include / .incbin to top level.
The previous fix moved .import / .include / .incbin to top level, but imported modules without their own *= directive rely on the parent file's implicit org pointer to land in bank-20. .pool / .alloc consumes the org pointer and does not propagate it back out, so vwf, dialog, kerning, libmz, battle/*, ingame/*, and menus/* all landed at $00:xxxx default org. Reset the org to $20:8100 right after the .alloc closes; inline routines occupy $20:8000..$20:8048 so 256 bytes of margin is safe and matches the original legacy layout.
Replace the *=0x208100 org-reset hack with a declarative .alloc bank20_modules in bank20_reloc that holds every import / include / incbin that lives in bank-20 freespace (libmz, dialog, kerning, vwf, battle/*, ingame/*, menus/*, the inventory + treasure rolling-buffer includes, and the bank-20 text incbins). assets.s stays at top level: it carries its own multi-bank *= placements ($0AF000, $0FA710, $0E9800 patches plus the bank-21+ freespace data), which a single .alloc can't wrap without clipping out-of-range writes. Result: same byte coverage as master (verified via symbol diff — identical set, only the new pool symbols added), build/ff4.ips matches master within 1.5% (allocator packs slightly tighter on bank-20).
This reverts commit aa7bb1e.
This reverts commit 366f308.
…odules"" This reverts commit 994ab22.
…modules"" This reverts commit 6914a12.
a22 ships the fix for overflow Q#14: AllocNode.emit_blocks now skips LinkedModuleNode body nodes marked as dedup losers, so a .alloc-wrapped .import correctly emits at the pool address while the .include-pulled top-level duplicate is silenced. Unblocks the bank20_modules .alloc that wraps the 22 imported modules and 12 incbins. Verified against the reapply commit bc17dbe: build/ff4.ips lands at 262172 bytes, byte-transparent vs the no-wrap master baseline.
… bank20_modules""" This reverts commit bc17dbe.
|
🎮 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.
Summary
The bank-20 relocated region in
ff4.sused the legacy*=0x208000org-pointer pattern: a hand-placed block mixing inline routines,.importmodules,.if-gated sub-blocks, and.incbinasset blobs. Reading required tracing the implicit org through ~30 directives, and inserting code anywhere silently shifted everything below.This branch wraps the whole region in
.pool bank20_reloc { range 0x208000 0x20FFFF; strategy order } + .alloc bank20_main { ... }, the same allocator pattern master adopted for bank-01 in commit 9b3eaa7. The allocator now owns layout.strategy orderkeeps declaration order so cross-bank callers resolve to stable addresses.Dogfooding the new
.pool/.allocsyntax againstff4.ssurfaced four formatter bugs in a816, which were filed and fixed upstream over the course of this branch (overflow Q#10, Q#11, Q#12). Pin bumps to a816 1.1.0a20 and kintsuki 0.0.0a13 land in the third commit, andff4.sis normalised against the new canonical layout soa816 format --checkruns clean in pre-commit.Test plan
python3 build.pyproducesbuild/ff4.ips(40679 bytes) andbuild/ff4.sym(27438 bytes)a816 format --check ff4.spasses (idempotent on a20)--no-verify