Module-owns-placement migration + rolling-inventory engine - #32
Open
manz wants to merge 127 commits into
Open
Conversation
Adds the plumbing for routing battle inventory text rendering through the messages_vwf put_char dispatch: - New region in battle_render: init_inventory_region targets tile_id base 0xC0 (region_size * 4), distinct from the existing messages / monster / names / commands regions. - New messages_vwf.init_inventory wrapper that sets battle_flags = 0x02 before delegating to battle_render.init_inventory_region, mirroring the existing init_names / init_monsters wrappers. - draw_text_rolling_trampoline gains a BATTLE_ITEMS_VWF branch that sets the VWF flag and wraps the draw_text call with messages_vwf.init_inventory / messages_vwf.deinit. Flag stays OFF in config.i. The blocker before flipping it on: the init_X path calls clear_buffer which zeroes the WHOLE 2560-byte WRAM tile buffer, wiping monster + names + cmd render state alongside the inventory region. A region-local clear (only the inventory tile slice within buffer_ptr) is needed before this is safe to enable.
init_inventory_region now skips clear_buffer. Clearing from tile_id 0xC0 would write 768 bytes from buffer_ptr + 0xC00 = \$703C00+ and clobber pending_transfer_mask + region_dirty_bits + render_skipped + tilemap_pending_mask. The rolling-buffer overwrites each tile's bytes fully every render anyway, so the no-clear path is safe. The init still: - sets pending_transfer_mask to the inventory region offset - runs render_allocator.init_with_tile_id - resets prev_char (kerning), bits_left_on_tile, temp, counter Golden re-recorded ; extra pre-render cycles shifted the Zu animation by one frame, so the diff was pure BG drift not actual UI change. Next: confirm DMA path picks up the inventory region (tile_id 0xC0+, VRAM \$B800+) ; if not, extend messages_vwf.dma_transfer to cover it. SRAM space is plentiful per user note, so the region can be expanded or relocated if 48 tiles proves too tight.
The CHR transfer in messages_vwf.dma_transfer copied 0xC00 bytes from buffer_ptr to VRAM \$B000, which only reaches up to buffer tile 0xBF in the 16-byte-per-tile interleaved layout. The new inventory region sits at tile_ids 0xC0..0xEF, so its CHR was never flushed to VRAM. Bump the length to 0xF00 when BATTLE_ITEMS_VWF is on so the inventory tiles ride the same DMA. The tail lands at VRAM \$B000 + 0xF00 = \$BF00, still inside the BG3 CHR window (\$A000..\$BFFF), so nothing past the expected CHR range gets clobbered. The SMART branch stays as-is (still buggy per existing note in code).
Previous patch called init_inventory per draw_text_rolling_trampoline invocation, which reset the allocator to tile_id 0xC0 every item ; all 6 slots overwrote the same CHR range. Fix: hoist the reset to the rolling pass entry (init_inventory_text_buf_rolling) so the allocator increments naturally through items 0..5. With ~9 tiles per name * 6 items = ~54 tiles total, the inventory range stays inside 0xC0..0xF6 and never wraps into the messages region. The per-item trampoline now just flips the VWF battle flag for the duration of the DrawText call (no init / deinit on each call).
Two related fixes spotted by the inventory pre-render: - The inventory tile slice runs from buffer + 0xC00 to 0xEF0 in WRAM ($703C00..$703EF0), which collided with the gate state words at $703C00..$703C03. Move pending_transfer_mask / region_dirty_bits / render_skipped / tilemap_pending_mask to $703F00..$703F03 (past the inventory range) so rolling renders no longer stomp them. Updated the test + profiler scripts that poked the old addresses. - The rolling pre-render path was calling init_inventory but never the matching deinit, so the VWF battle flag stayed set after the pass and pending_transfer_mask bit 0 never got OR'd to fire the DMA. Add `jsr.l messages_vwf.deinit` at the end of init_inventory_text_buf_rolling.
Loads ff4-battle-ext.kss, settles 600 frames so Cecil's ATB fills, drives DOWN/DOWN/A to open the item menu, then snapshots the inventory rolling-buffer slots + WRAM CHR slice + gate state and diffs each subsequent frame against the prior to surface per-frame drift. Captures a framebuffer golden of the open inventory for visual reference. First runs surface two things: - VWF item names are rendering (Aiguille / Ether visible) so the init / DrawText / DMA chain is wired correctly end to end. - rolling_slot transitions from 04 -> 00 between frames without any input, and tile_id drifts from 0xCE -> 0xD1 in the same step. The rolling buffer keeps re-rendering slots, each re-render allocates more tile_ids past the inventory base, and the resulting tilemap entries reference shifted tile_ids every frame. That is the "scrolling VRAM" symptom: CHR is correct, tilemap refs walk. Next: figure out why the rolling buffer advances without input.
The pass-entry reset got wiped every frame because vanilla DrawInventoryItemText re-fires per row for quantity / palette refresh, advancing the allocator past 0xEF and overflowing into messages / monsters tile_ids. Move the reset INTO the per-call trampoline path: messages_vwf.init_inventory_for_current_slot reads rolling_slot_index from $7EEF82, computes 0xC0 + slot * 9, and re-arms the allocator + VWF battle flag before DrawText runs. Trampoline body shrunk back below the 131-byte bank-02 pool ceiling by keeping the slot math in bank-20. Still a tilemap-destination drift visible when the battle message window renders (Attaque preventive text leaks into inventory rows). That is a separate $EF52 / tilemap_offset routing issue.
Pumps battle-ext + DOWN/DOWN/A into the item menu then writes a hex dump of: - the 6 rolling-buffer slot tilemap entries at $7E97A6 - the VWF tile CHR buffer in WRAM ($703000..$703F00) - the moved gate state at $703F00 - the cross-bank allocator at $702F00 Surfaces: slot rows reference tile_ids 02..09 plus a few VWF-looking ids (50, 6d, 80, 85), NOT the expected 0xC0+ range. So init_inventory_for_current_slot's allocator reset isn't surviving into DrawText — something between the trampoline JSL and the DrawText body re-runs init_with_tile_id with a different base. Next: insert a runtime probe (write a marker byte to a scratch address before / after each candidate path) to find which JSL is clobbering the allocator.
init_inventory_for_current_slot computed the per-slot tile_id base in A then called set_vwf_render before storing it. set_vwf_render clobbered A, so the allocator ended up at whatever set_vwf_render left in A (low values, hence the messages-region tile_ids 0x02..0x09 we observed in the dump). Push/pull around the call. Inventory now renders distinct item names per row (Aiguille / Ether / Baguette / Collure / etc.) instead of all slots overwriting the same tile range. Digit / colon glyphs still leak through VWF and look garbled ; next step is routing those back through the fixed-width path so only the name itself is VWF-rendered. Region budget: 6 slots * 9 tile_ids = 54 tiles fits in 0xC0..0xF5. Plenty of room to grow to 11 or 12 tiles per slot if needed.
VWF put_char OR-stores glyph bits into the WRAM tile buffer ; without a pre-clear, stale CHR from prior renders bleeds into the new draw and produces the garbled column we saw between item names. Add a tight clear loop in init_inventory_for_current_slot that walks 9 tiles * 16 bytes from buffer_ptr + (tile_id_base * 16) writing the empty 4bpp pattern ($00FF as a 16-bit pair). Items now render with clean CHR (Aiguille / Ether / Baguette etc.) but a few rows still show artifacts where digit / colon glyphs flow through the VWF path. Next: route those back through fixed-width put_char, and grow the per-slot budget past 9 tiles so longer names like Collure stop truncating.
Slot N now allocates 10 tile_ids starting at 0xC0 + N * 10, so 6 slots span 0xC0..0xFB and leave 4 tiles of slack before the 8-bit allocator wraps. Names that used to truncate at "Coll" now render "Collure :61" cleanly. Open: monsters not rendering after inventory opens. Buffer offset \$703300..\$7035F0 (monsters region) dumps as the empty FF/00 pattern, so clear_buffer ran for that region but DrawText never filled it back. Tracking down the path that wipes monsters CHR without re-rendering. Region state at \$703F00 shows pending_transfer_mask = \$D2 (slot-2 base), region_dirty_bits = 0 (all clean), render_skipped = \$FF -- which means last gated init was a skip. Not yet pinned down when the prior clear fired.
Spaces (\$FF) inside a name now emit 0x03 0xFF in the format string so DrawText writes the menu_font space tile_id directly without consuming a VWF tile_id slot in the slot's budget. Combined with the prior colon escape and the existing digit escapes, only the actual name letters route through the VWF blitter, which matches the "VWF the name, fixed for everything else" intent. Open follow-ups from this session: - Pick / define a switch text code (one that flips the dispatch to the draw_letter dakuten path) instead of repeating 0x03 escapes for each fixed char. - Render item TYPE as a fixed-width string after the count (ARMOR / SWORD / etc.). Needs the item-type byte offset confirmed. - Given how much custom format handling we layer on draw_text, may be cleaner to own a full draw_text variant in the relocated bank and wire it into bank-02 (user's note). - Monsters region still renders empty after inventory opens ; needs pinning down the path that wipes monsters CHR without re-render.
…ry_text) Custom walker that interprets only the escapes we actually emit for inventory rows (0x00 terminator, 0x03 BB fixed tile, 0x0E PP palette attribute) and dispatches anything else as a VWF char through battle_render.display_char. Cuts the dependency on vanilla draw_text + battle_flags toggling. Setup mirrors the vanilla draw_text prologue: stashes src ptr ($30), row-1 dest ($32), row-2 dest ($34) and palette ($36) into DP so tilemap_write_no_inc's (\$32),y / (\$34),y indirect writes find the right rows. Y starts at 0 (relative offset into the slot tilemap buffer pointed to by $32 / $34), not the absolute slot address. init_inventory_for_current_slot_local runs at function entry to reset allocator + bits_left for the per-slot VWF run. Fixed-tile escapes (0x03 BB) write through the same ($32),y / ($34),y indirect pair so the layout stays consistent with VWF tiles in the slot. Wired in via the bank-02 trampoline so _render_inventory_item_circular calls draw_inventory_text instead of vanilla draw_text. Items now render with VWF names + fixed colon / digits across all 5 visible slots (Aiguille / Ether / Baguette / Collure / Baguette). Some artifacts at first char + middle column remain ; first char is the symbol tile, the middle artifacts come from spaces in the name still being VWF-rendered when the slot escape did not cover them.
Custom renderer in messages_vwf.draw_inventory_text picks up a new 0x0F escape that toggles a DP fixed-vs-VWF mode flag. Default is fixed so the row-leading symbol byte writes directly as a tile_id ; the format builder emits 0x0F before and after the 11-char name to flip into VWF and back. Eliminates per-char 0x03 escaping for colon / digits. Per-call setup also stopped permanently doubling $ef54 via asl ; we now lda + asl A so the line_length stays valid for subsequent slots. Open: name still garbles for some slots in the live render. Likely either an item-name byte that aliases an escape code (0x03 / 0x0E / 0x0F) or a Y / allocator interaction that I'm not yet handling correctly when toggling mid-format.
Two fixes combined unlock the full inventory render: - Drop the asl on \$ef54 in the renderer prologue. The vanilla draw_text path tolerated re-using line_length because every entry re-fetched it, but our reusable trampoline path called the prologue per slot and the in-place asl permanently doubled the stride after the first call. Use lda + asl A and leave \$ef54 alone. - Drop the ora.b #0x01 from _di_fixed and _di_fixed_char. That bit on the tilemap-entry high byte sets tile_id bit 8 (the +0x100 range used by VWF tiles). Fixed-font tiles live in 0x00..0xFF so the bit must stay clear ; vanilla wram.put_char does not set it either. Setting it shifted every fixed tile_id into the VWF window and rendered the wrong glyphs. Combined with the per-slot allocator reset and the 0x0F mode toggle introduced in the prior commits, the inventory now shows "Aiguille Or:05 / Ether :06 / Baguette :1 / Collyre :61 / Baguette :1" with VWF-rendered names and fixed colon + count.
The previous commit only landed the golden PNG ; pre-commit's stash / restore cycle swallowed the .s edits. Resend the real src/battle/message.s + inventory_rolling.s diffs (0x0F toggle escape, fixed-vs-VWF mode flag, drop spurious ora 0x01 on fixed tilemap entries, fix asl 0xef54 in the prologue).
The trampoline diff was the missing piece ; the previous commit landed the renderer + format builder but the bank-02 trampoline was still calling vanilla draw_text via init_inventory_for_current_slot. With this change the trampoline jsrs straight into the custom draw_inventory_text in bank-20 when BATTLE_ITEMS_VWF is on, so VWF actually fires for the name portion.
Three format builders in inventory_rolling.s now emit the same fixed -> VWF -> fixed shape: 0x0E palette / symbol / 0x0F / 0x0E palette / name chars / 0x0F / colon / digits / 0x00. Visual is still vanilla-looking because vanilla DrawInventoryItemText at \$02:9FA4 is not patched ; it rebuilds the format buffer with the old 0x03 escapes every frame on its own format-building pass, and the trampoline runs our renderer on THAT format (no 0x0F seen at runtime). The pre-render format builders we edited do emit 0x0F but their output is overwritten before draw_inventory_text reads it. Next: patch \$02:9FA4 to route through our format builder (or replace it outright with a renderer that takes item_id directly and skips the format-string intermediary).
Pre-commit format pass keeps swallowing unstaged source edits, so the previous commit landed only the golden PNG. Stage everything this time + the dump tool changes.
Char-value threshold matches what vanilla draw_letter already uses: codes < 0x42 are symbols / icons / small codes and render through the fixed-width path ; codes >= 0x42 are printable letters and route to battle_render.display_char (VWF). No 0x0F toggle needed in the format string ; the renderer makes the call from the char itself. Net effect: the existing vanilla DrawInventoryItemText format (0x0E pal / 0x03 symbol / 0x0E pal / 11 name chars / 0xC8 colon / 0x03 digit / 0x03 digit / 0x00) routes the name letters through VWF and everything else through fixed, with the colon falling naturally above 0x42 -- TODO: wrap colon in a 0x03 escape so it stays fixed-width. Renderer cleanup: drop the $37 mode flag, _di_toggle handler, and prologue init for it.
Routing now: < 0x42 fixed, 0x42..0xBF VWF, >= 0xC0 fixed. Symbols / icons / digits / colon / space all use the fixed-width path ; only the letter range routes through VWF, so the inventory tile_id budget stops being burned on non-letter glyphs. 0x0F treated as a no-op so the legacy toggle bytes the pre-render format builders still emit are silently consumed (no garbage tile).
Renderer now matches:
0xFE -> enable VWF dispatch for subsequent chars
0xFD -> disable VWF (chars route to the fixed tile_id path)
0xFC NN -> pad with space tiles until y reaches the NN-th tile
entry, then reset bits_left_on_tile to 8 so the next
VWF char starts on a clean tile boundary
Default mode at function entry is fixed (DP \$37 = 0x80) so the
leading symbol byte lands as a literal tile_id without an explicit
escape. The char-range heuristic (< 0x42 and >= 0xC0 -> fixed) is
removed ; the new control codes are explicit, no implicit routing.
Branches inside the dispatch grew past the 8-bit BRA range now that
the handler list is longer, so the per-handler `bra _di_loop` tails
are jmp.w. Tiny code-size hit, no runtime impact.
User follow-ups still open: turn 0xFC into a pure goto + pre-clear
each slot's tilemap to space tiles in init_inventory_for_current_slot
(only modify the cells we touch on render). Also off-screen slot
rolls don't yet re-render through the VWF region.
Renderer cleanup: - Drop 0xFD ; 0xFE now toggles fixed <-> VWF, single code per mode flip. - 0xFC NN is a pure goto (sets Y to NN * 2 plus a bits_left_on_tile reset). Intermediate slots stay at their pre-cleared values. - Pre-clear every slot's tilemap to space tiles ($FF) + palette in the renderer prologue. 15 tiles * 2 rows = 30 entries written before the format walk starts ; later goto / VWF / fixed writes only touch the cells they need. Format builders now emit 0xFE before the name region + 0xFE after, plus 0xFC 0x0C to align the colon at slot_length - 3 = column 12.
The custom renderer set battle_flags = 0x02 via set_vwf_render in the init helper but never cleared it on exit, so later non-inventory renders (monster HP refresh, status text) saw the flag still on and routed through put_fixed_char_dakuten_far. HP / count digits ended up in the VWF blitter, garbling the monster name area. Call battle_flags.clear_vwf_render before the rtl so the flag goes back to WRAM mode after the inventory pass.
Hidden-slot prefetch did not reach VRAM: draw_inventory_text exited without setting bit 0 of pending_transfer_mask so the NMI dma_transfer gated path skipped the CHR slice. Set the bit at _di_done so each scroll-edge render flushes its slot to VRAM. Quantity digits were wrong on scroll-edge renders. The format builder did pla in M=8 mode (1 byte = qty) then tax with X=16, copying leaked A.hi (residual from prior 16-bit arithmetic) into X.hi. With qty=1 and a leaked 0x06 in A.hi, hex_to_dec saw 0x0601 and emitted "37" instead of "1". Zero-extend A before the transfer. Allocator state leaked across regions when per-slot tile_id ranges overflowed. Add slot_limit_low at $702F02 that clamps render_allocator. increment; inventory slot init writes slot_base+9, non-inventory inits reset to 0xFF (no clamp). A too-long item name now keeps overwriting its last tile instead of bleeding into the next slot. Per-frame full rebuild of all six slots ran every frame inside tfr_inventory_list_rolling, clobbering the pre-rendered hidden slot before it could be revealed. Gate _refresh_visible_items_internal on inventory_needs_full_refresh at $7E:EF9D (clears after the pass). Init paint and item swap set it; steady-state scrolls leave the hidden slot intact. Earlier draft used $EF9B which collided with battle_monster_dirty and hid monster names. Scroll-up past item 0 underflowed cursor state. Vanilla UP handler at $02:B4F0 unconditionally runs dec EF86 / dec EF85 / dec $63 after jsr scroll_list_up_hook, so our top-of-list abort still clobbered $63 (cursor item index) to 0xFF. Pre-increment those three bytes in the abort path so vanilla's dec lands on the original values. Split _su_abort from _su_exit so the inc fires only on abort, not on the success-exit fall-through (an earlier draft put it on both and froze cursor state after a few scrolls).
The BG3 V-scroll HDMA chain (channel 2, indirect, src $7E:760B -> $7E:7ED2 chunk) had 7 entries at $7E:8068..$8081 hardcoded to scroll 0x02BB after our 5 body rows ; that scroll value lands in empty tilemap and rendered 2 rows of black between the last visible item and the inventory window's bottom border. Pin those entries per NMI inside dma_transfer when the inventory window is open (bit 2 of $4A). First 3 entries map to scroll 0x0193 (tilemap pixel +8 from the hidden-slot lock, the first border row), last 4 step to 0x019B (+16, second border row). Wrapped in php/plp and sep/rep #$30 to keep M/X state from leaking across the gate. Cmd / status menus share BG3 V-scroll but only see the override when the inventory flag is set, so they stay untouched.
The previous footer fix wrote our border-row scroll values (0x0193 / 0x019B) into the ch2 HDMA stream while inv was open but left them in place after close. The cmd / status menus share ch2 so they kept rendering inventory border tiles below the panel. Per-NMI hook now also writes the vanilla idle pattern when inv is closed: $7E:8068..$8070 -> 0x01F7 $7E:8074..$8080 -> 0x0026, 0x0025, 0x0024, 0x0023 (decreasing) Captured from a battle-settle HDMA dump rather than guessed. State-1 swap can't fight us anymore because both active and swap converge. Bumped tilemap_content_offset 0x44 -> 0x46 so item names render 1 tilemap column further right inside the inventory window.
The intended phase 2.4 (replace engine_init_rolling_buffer macro call with `jsr.l rolling_engine.rolling_engine_init`) renders only the menu chrome on screen ; per-slot hook dispatch through the new indirect-long path doesn't reach the same per-row tilemap state the macro's inline JSR.W achieves. Engine entries stay unit-tested + integration-tested independently against synthetic state. The hooks are armed in init_menu_rolling_buffer_impl so the JSL switch is one edit away once the discrepancy is chased. Going back to the macro keeps the field-menu rendering working while we figure out what the macro is doing that the engine isn't.
Root cause of the "items don't render" symptom : the swap replaced the macro's terminating `rtl` with `rts`. init_menu_rolling_buffer_impl is called via `jsr.l` from the bank-01 trampoline (JSL pushes 3 return bytes), so the impl must end with `rtl` (pops 3). The macro ended with `rtl` ; my replacement ended with `rts` (pops 2), which left the JSL bank byte stranded on the stack. Subsequent code ran with a corrupted stack, garbage PC, and eventually fell into bank-20 ROM padding executing the `brk #$00` and `bvc $50` chains visible in the kintsuki trace. Diagnosed using the existing `add_read_callback` API to observe that the field NMI hook never read $20 from $7E:1BAE on the engine path -- because the menu state machine never reached the post-init loop. Trace via `tracer_start` + `run_until_stp` then showed the exact `rtl` mismatch : engine_init's last sequence was `plp ; rts ; <garbage padding> ; brk` instead of returning cleanly to the bank-01 trampoline. With the rtl in place, field-menu items render. Some cosmetic display offset remains (right-edge border drawn slightly differently from the macro path) -- likely a side effect of engine_init zeroing + ordering minutiae that the macro skips. Will iterate. Three new golden test failures (border-position deltas) follow.
Engine path drives init through bank-20 rolling_engine_init successfully (items render, HDMA arms, dirty_mask gates work) but the resulting BG layout differs from the macro's pixel-for-pixel : whole inventory layer shifts ~2 tiles left, qty colon column lands at BG3 col 18 instead of 14, item names clip their first glyph against the window border, and the description-window content vanishes on first frame post-open. Goldens recorded against the macro render are correct ; the engine swap is what diverges. Keeping the macro as the active init path so the field-inventory suite stays green while the divergence is investigated separately. Config-arming + hook far-ptr writes stay in place so the engine integration tests still exercise live state.
FF4 menu code sets DP=$0100, so sty.b 0x90 wrote the hook-field offset to $0190 (BG1HOFS shadow) instead of zp $90; vanilla NMI then scrolled BG1 by the offset value each frame. Stash at $00:1F83 instead, next to the existing jmp-vec cell.
Drops the macro expansion at init_menu_rolling_buffer_impl in favor of the bank-20 engine entry. Hook far-ptrs were already armed in phase 2.1; the BG1HOFS clobber that blocked this swap is fixed in the prior commit.
Splits welcome / quantity / thank-you blocks so the three owner strings each get their own label. Adds bank-20 trampolines hooked at $01:C353 (welcome), $01:C442 / $01:C7E7 (quantity), and $01:C751 (thanks) ; vanilla menu-text engine still renders the action labels and Quantité + "1" digit. Carries a816 format normalisation for two unrelated files the pre-commit hook flagged.
Bumps SaveDlgGfx / RestoreDlgGfx slab from $1300 to $2000 bytes so the save/restore round-trip covers the full VWF region union $5000-$5FF0 (field items, drops, item description). Without the extension the menu glyph CHR at $5300+ stayed in VRAM forever (overworld never writes those addresses), so any BG tilemap entry referencing tile_ids $100+ rendered the leftover glyphs over the overworld map. SRAM buffer at $70:5000-$70:6FFF stays clear of the battle-magic region at $70:7000.
Adds _zero_vwf_description_region to the tail of clear_ram. DMAs $1000 bytes of zeros from freshly-cleared SRAM $70:0000 to VRAM byte $5000-$5FFF (Region 1 + Region 1B + Region D, the full VWF union) under force-blank. Breaks the orphan carry-forward where save/restore would otherwise round-trip whatever leftover glyph bytes happened to sit at those addresses at the very first SaveDlgGfx, preserving them forever in the SRAM save buffer.
Treasure state moves out of $1B00-$1BFF (vanilla menu / sprite code writes past $1BEB during normal frame rendering, which clobbers the engine struct's hook far-ptrs the macro path never wrote there). 35-byte struct now lives at clean $7E:9C00. init_treasure_rolling_buffer_impl drops the macro expansion in favor of config-arming RollingBufferState (visible_rows, slot_height_tiles, item_list_ptr, item_count, hdma_channel, vwf_cfg_ptr, three hook far-ptrs) then JSL into rolling_engine.rolling_engine_init. Adds three bank-20 RTL hook trampolines (render_slot, update_hdma, draw_window) so the engine's indirect-long dispatch can return cleanly. Hardcoded $1BDB / $1BD6 / $1BD1 refs in battle + field-menu code follow the relocation.
Drops state moves out of $1B00-$1BFF for the same reason as treasure : the engine struct extends past $1BEB which vanilla sprite code stomps on mid-frame. drops_scroll_pos follows to $9C5F (one byte past the struct) so it stays disjoint from the engine's visible_rows slot. drops_init_impl now config-arms RollingBufferState + JSLs into rolling_engine.rolling_engine_init ; three bank-20 RTL hook trampolines wrap the existing render / ensure-HDMA / draw-window helpers. drops_hdma_copy_pending follows the relocation.
Key-item picker state moves out of $1B00-$1BFF for the same collision reason ; struct lives at $9C60-$9C82 and key_item_scroll_pos at $9C8F. key_item_init_impl config-arms RollingBufferState (item_list points at $7E:0712, the filtered key-item buffer InitItemList builds) then JSLs into rolling_engine.rolling_engine_init. Three bank-20 RTL trampolines wrap the existing hook helpers.
All four callers (field-items, treasure, drops, key-item picker) config-arm RollingBufferState + JSL into rolling_engine_init now ; the macro has no live callers left.
Adds rolling_engine_refresh_slots in src/lib/rolling_inventory_engine.s. Caller passes state ptr in X and current scroll_pos in A ; engine derives buffer_slots = visible_rows + 1, iterates the slots calling fn_render_slot, stamps transfer_pending = 1. Treasure / drops / key-item picker callers swap from macro expansion to JSL. Macro definition deleted from rolling_buffer.s.
Adds rolling_engine_update_scroll_frame in the bank-20 engine. Reads scroll_direction / scroll_anim_offset / scroll_remaining from state, advances the animation one frame (pixels_per_frame hardcoded to 8 ; all four profiles use that value), bumps the vanilla cursor sprite at $0311 when $1B19 is non-zero, dispatches the per-menu HDMA shadow rebuild via menu_id (new RollingBufferState field, $9C0F-style trampolines in each menu), then drains tfr_sprites + tfr_bg2 to VRAM. Field / treasure / drops / key-item callers swap macro expansion for JSL. engine_update_scroll_frame macro deleted from rolling_buffer.s.
Both macros defined `*_scroll_down_prepare` / `*_scroll_up_prepare` labels in each menu file but nothing in the codebase ever called those labels. engine_start_scroll_down / engine_start_scroll_up inline their own prep logic. Drop the macros + the dead per-menu labels.
Adds rolling_engine_start_scroll_down + rolling_engine_start_scroll_up
in the bank-20 engine. Caller passes scroll_pos in A, state ptr in X ;
engine fires fn_update_hdma (lazy ensure), updates buffer_pos with wrap,
computes slot_index + edge_row, fires fn_render_slot for the new edge,
configures scroll FSM (state = 1, remaining = 16, direction = +8 down /
$FE up, anim_offset = -16 down / +16 up, transfer_pending = 1), then
dispatches the per-menu update_scroll_hdma via menu_id.
pixels_per_frame (= 8) + total_pixels (= 16) hardcoded ; all four
profiles use the same values.
Moves scroll_pos / buffer_slots scratch from $1F84 / $1F85 to
$1F88 / $1F89 to avoid the $1F84 clobber from `_engine_call_hook`'s
16-bit offset stash at $1F83. The same move applies to
rolling_engine_refresh_slots.
Field / treasure / drops / key-item callers swap the macro expansion
for `jsr.l rolling_engine.rolling_engine_start_scroll_{down,up}` after
loading scroll_pos in A. engine_start_scroll_down +
engine_start_scroll_up macro definitions deleted from rolling_buffer.s.
Adds rolling_engine_finish_scroll. Caller passes scroll_pos in A, state ptr in X ; engine reads scroll_direction to pick the post-animation prefetch direction, computes slot_index + edge_row, fires fn_render_slot (skipping when past list end / before list head), clears scroll_state + scroll_anim_offset, dispatches per-menu update_scroll_hdma via menu_id, calls draw_item_cursors + update_ctrl_after_scroll trampolines. Field / treasure / drops / key-item callers swap macro expansion for JSL. engine_finish_scroll macro deleted from rolling_buffer.s.
Adds rolling_engine_swap_redraw. Caller passes scroll_pos in A, state ptr in X ; engine fires fn_update_hdma, clears scroll FSM scratch, iterates buffer_slots = visible+1 times, computes slot_index + edge_row, fires fn_render_slot when item index in range. Slots past the tail get the field-items clear path (gated on menu_id == 0 ; other profiles' clear stubs were RTS no-ops). Stamps transfer_pending = 1 and dispatches per-menu update_scroll_hdma. Field / treasure / drops callers swap macro expansion for JSL. engine_swap_redraw macro deleted.
Last engine_* macro standing. Unlike the other operations, its per-row writes use compile-time state_base + a compile-time hdma_shadow_addr, so routing through the bank-20 engine would need menu_id dispatch on every state read. Inlining the body ~80 lines × 4 menus wins over that complexity. Each menu's update_*_scroll_hdma function now carries the previously macro-generated body verbatim. engine_update_scroll_hdma deleted from rolling_buffer.s — that file is now just the shared INVENTORY_SCROLL_* constants and a docstring. No more engine_* macros anywhere in src/.
`treasure_scroll_cooldown := treasure_rolling + 35` collided with the new `menu_id` field at offset 35. `treasure_ensure_hdma_initialized` STZs cooldown on first call, wiping my menu_id back to 0 and routing the engine's update_scroll_hdma dispatch to field-items instead of treasure. Bump to +36 (one past the end of RollingBufferState).
Key-item picker triggers via EventCmd_f7 mid-map ; the room's BG3 CHR is still live underneath. Engine's rolling-buffer init would write the picker's VWF glyphs straight over the map tiles. Field menu gets away with this because SaveDlgGfx round-trips $4000-$5FFF on every entry/exit ; the picker doesn't get that wrapper. Engine code (key_item_render_all + key_item_init_impl + trampolines) stays in place but unreachable. Wire-up needs either picker-scoped save/restore or a dedicated VRAM region the room never touches — follow-up commit.
Captures the rewrite shape we landed on after auditing ff5decomp +
ff6decomp:
- BG layer split (text on BG1, borders on BG2, cursor/desc on BG3) :
dissolves the FF4 "anchor border while items scroll" HDMA gymnastics
that currently force per-menu header/footer/signal hooks.
- FF6-style HDMA scroll table = static ROM table + one runtime
`ADC scroll_pos` patch on the variable slice. Drops the per-frame
buffer_pos / buffer_slots modular rebuild.
- Single BG3 CHR window at $C000 shared by menu / dialog / picker.
Kills the BG34NBA-per-context flip.
- Wholesale function-replacement strategy : each vanilla menu routine
(DrawWindow, UpdateItemText, DrawItemSlot, ShowItemWindow,
TfrBG*TilesVblank, main loop) gets a bank-20 reimplementation ;
vanilla site becomes a 4-byte thunk, original code dies.
Ordered migration list with acceptance gates per step.
Restructures ff4 to land cleanly on a816 master. Each module
that used to ride the `*=0x208100 / .import module` chain now
declares its own placement via `.alloc <name>_block in
bank20_reloc { ... }`.
Changes:
- New `src/bank20.i` with shared `.pool bank20_reloc { range
0x208000 0x20FFFF strategy order }` decl. Linker dedups
identical pool ranges per `_merge_one_pool_decl`.
- 23 imported modules wrapped:
libmz, dialog, kerning, vwf, dakuten, intro, small_vwf/init,
ingame/items_menu_vwf, ingame/places_names_window, menus/*,
battle/sram, battle/graphics, battle/monsters_reloc,
battle/magic_reloc, battle/redraw_gates,
battle/commands_reloc, battle/equip_window, battle/math_reloc,
battle/inventory_rolling, ingame/init_bg_scroll_hdma.
- 5 `.include`d source files also wrapped:
ingame/inventory_rolling, lib/rolling_inventory_engine,
ingame/treasure_rolling, ingame/drops_rolling,
ingame/key_item_picker.
- ff4.s: dropped `*=0x208100` chain anchor. Converted remaining
`*=` hijacks ($00:FFC0, $00:FFD6, $00:FFE0/E6/FE,
$00:8031, $00:B463, $23:8000) to `.alloc at ADDR { ... }`
blocks. Conditional `*=` patches inside `.if` blocks
similarly wrapped.
- build.py: `overlap_mode="warn"` argument to
`build_with_imports_direct` so the 19 latent overlaps
(battle_messages overrun into battle_text region, etc.) keep
building while audited separately.
Status:
- Build: succeeds on a816 master. 267KB IPS, exit 0.
- Tests: 113/162 pass (up from 90 pre-migration), 37 fail,
9 setup-error. Most remaining failures are
inventory-rolling/treasure/kerning where label
resolution in alloc bodies still drifts vs a17 baseline.
- Known a816 bug uncovered: label inside `.alloc body` can
resolve off-by-1 (initialize_snes lands on previous fn's
RTS instead of the sep #$30 that follows it). Boot STP's
because of this. Tracked separately in a816.
- All 527 legacy `*= ADDR` rewritten to `.alloc at ADDR { ... }` via
a816's UP001 autofix (latest nested-container + brace-pair fixes).
- 9 modules: added missing real `.include "bank20.i"` where only
docstring fakes existed (commands_reloc, monsters_reloc, dakuten,
graphics, magic_reloc, math_reloc, inventory_rolling, redraw_gates,
equip_window).
- 10+ modules using `.if FLAG` got `.include "config.i"` so flags
bind at module compile time (replaces the dropped `prelude_file=`
prepend).
- ff4.s top: prepended `.include "config.i"` + 25 `.import` lines
before any `.include`d patches so extern stubs are visible when the
patch files compile.
- build.py: switched build_with_imports_direct → build_with_imports,
removed `prelude_file=`, added `Path(".")` to include_paths so
config.i resolves from anywhere.
pytest delta vs direct-mode baseline: identical pass/fail
(28 failed, 90 passed, 15 skipped, 1 xfailed, 9 errors). Module
pipeline is functionally equivalent for ff4's test suite. Existing
failures are pre-existing branch issues, not migration regressions.
…ss-module symbols
Replace per-field := alias chains (drops_rolling_top_row, ...) with a
single (addr as RollingBufferState) cast per menu, referencing fields as
<base>.<field>. Add the field_menu_rolling typed view in items.i and move
all four rolling bases into the clean $7E:9C00 arena.
Surface cross-module labels so .extern resolves: drop the leading _ from
cross-bank trampolines/entries (draw_text_battle_far,
draw_battle_command_window_relocated, msg_*_window_gated, battle_ext_seed,
tfr_bg{3,4}_tiles_vblank_trampoline, item_desc_back, first_window, ...)
and hoist .extern declarations out of .alloc/.scope bodies to root scope,
where they actually register.
Extract the battle redraw dirty-bit / tilemap-pending constants into
render_defs.i, shared by message.s and redraw_gates.s via .include.
Add a816.toml for the module pipeline build.
Replace em-dashes with ASCII hyphens in touched comment lines.
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.
Lands the a816 module pipeline migration on top of the rolling-inventory engine work. 127 commits; large by design (full migration vs mainline
master).What
build_with_imports/ module-owns-placement; adda816.toml.$7E:9C00arena and ported to engine entries; per-field:=alias chains replaced with(addr as RollingBufferState)typed casts._from cross-bank trampolines/entries and hoist.externout of.alloc/.scopebodies to root scope (a816 now mangles alloc-body privates per-alloc, so underscore locals no longer resolve cross-module).render_defs.i, shared bymessage.sandredraw_gates.s.1.1.0a30.Risk / things CI can't see
$7E:9C00overlaps spell-list text buffers freed by magic-direct-render; relies on menus being mutually exclusive with battle and init re-seeding state per open.