Make the field item inventories work - #34
Merged
Merged
Conversation
The rolling state structs moved to $7E:9C00 and every hardcoded test address rotted silently; the a816 casts already export per-field symbols into the .adbg.
A bare 16-bit constant sent every lda.l/sta.l to bank $00, i.e. ROM, so drops never scrolled; the swap-index site has no room for the wider opcode.
Only refresh_slots stamped transfer_pending, so drops rendered at menu open sat in WRAM behind vanilla's partial window upload.
The picker loop spins ~20 times in the frame that ends a scroll, draining the debounce; blocking input also cost the loop vanilla's frame pacing, so an aborted trigger now burns a WaitVblank.
The engine derives buffer_slots itself, so passing the slot count gave it one slot too many and the prefetch row wiped the window border.
The one-shot DMA left its descriptor in ch6, which treasure arms for BG3VOFS, so the scroll table was never read again once a flush ran; ch3 is HDMA-free in field, treasure and battle.
Same bank-$00 aliasing the drops scroll position had.
Groundwork: nothing in the ROM reaches this yet, and glyph cells still do not land in the staging page.
Previous captures predate the VWF render and carry the scroll, border and window-push bugs fixed above.
The list loop keeps the item id in $5D, which the VWF hijack reads as a slot index, so a high id asked for tiles past the CHR buffer and rows overwrote each other.
The list loop draws digits and the Gils suffix before the name, so the bottom-row pre-fill has to stop short of those columns.
The pre-fill cleared the old fixed-width 16-char field, but no name exceeds its 10-tile budget; the surplus cells belonged to whatever the caller drew beside the name, which is how the shop lost its price digits.
SelectItemSell is untouched: it lists the player's inventory through a different draw path.
Vanilla's UpdateCtrl uses $1D as scratch, so an NMI landing mid-render sent the rest of a name's tilemap cells to $7E:0000; the menus never saw it because they run with NMI disabled, but the key-item picker overlays the live map.
The engine dispatches hooks with DB inherited from the vanilla caller, so the picker's absolute accesses silently missed; it also has to copy the scroll table itself, since the field NMI hook only runs while a menu owns the screen.
Arming ch4 overrode the scroll vanilla uses to place this window and left BG3VOFS parked at our value afterwards.
The menu renderer scratches direct-page bytes the field engine owns, so the picker snapshots the page around its render and keeps NMI off across it; the push covers only the window's rows, since BG3 plane 1 is the map's own tilemap either side.
Vanilla owns the scroll position and the cursor; the engine owns the row contents, so the per-frame hook renders on the scroll edge as well as the open edge.
Vanilla drew the whole list into the window band and walked $BB over it; the engine keeps five slots, so the scroll routines now step $BB back onto the ring and redraw it, and vanilla's own list draw is suppressed.
The slide marker survives the picker closing, so a later open could reach the scroll edge without init having run and dispatch stale far-pointers straight into unrelated code.
The window is drawn by the V-IRQ, so clearing NMITIMEN outright blanked it for the render frames: the window appeared to close and reopen on every scroll and the cursor lost its per-frame draw.
Hand-picked addresses kept colliding with live state; the pool places them and checks overlap. Bank $70 SRAM would be the better home, but a816's low_rom bus has no region for it so a bss pool there fails the map check.
Nothing is emitted for a bss reservation, so the bank only needed declaring; SRAM is cleared at boot and untouched by vanilla, unlike the WRAM holes we had been picking by hand.
The picker renders glyphs into BG3 tile ids $100+, which is map graphics on maps that use them, and nothing saved that slice; the close animation's tail restores it from SRAM.
The picker's transfers ran on channel 0 and reprogrammed channel 3 without putting it back, stealing whichever the map had armed for HDMA: a mosaic effect loses its table mid-animation and never recovers it.
Direct page $77 is free in the menus but is the field engine's MOSAIC shadow, so glyph codes written there as prev_char reached $2106 and pixelated the map for as long as a picker render took.
The picker renders through vanilla's DrawItemSlot chain, which scratches a wide set of direct-page bytes that are free in the menus but live field state at $0600. Snapshotting the page and restoring it could not work: the window V-IRQ keeps running across the render, so the restore also rewound whatever it advanced, and the frame it missed dragged the window band across the map. Copy the field page into $1D00 and render there instead. Reads still see the caller's values, writes are dropped, and the field page is never touched. $1D00 is unassigned in the decomp RAM map ($1BEC-$1DFF) and sees no access in any of our savestates; both interrupt handlers load their own D, so neither is affected by ours. Open/scroll/close recording: 0 of 486 frames show map corruption, against 88 before.
Cell $AF was empty, so "Flûte Gysahl" rendered as "Flte" everywhere the menu font is used. The rest of the circumflex family (â ê î ô) is already there; this takes the accent and blank row from ô and the body from u.
The renderer wrote only the cells it draws - the name, the colon, the two quantity digits - and left every other cell in the row holding whatever the field had in the BG3 tilemap underneath. In a dark room that passed for a window body; in a bright one the map's own tiles show straight through the list, most visibly in the gap between a short name and its quantity. Wipe the slot's two rows to the window's blank cell first and let the draw fill them back in.
The sell list drew through vanilla DrawInventoryList, which walks the inventory two items at a time and lays all 48 into the BG3 buffer at 16 bytes each, then scrolls BG3VOFS over the result. Our globally patched DrawItemSlot renders one item per 16px row at 128 bytes a slot, so the two disagreed: the list showed every second item, stopped after six, and never scrolled. Sell now gets its own profile, in the shape of the other three: a 9-slot ring in the BG3 buffer with BG3VOFS driven per scanline by HDMA channel 5, which is free - the shop arms no HDMA of its own. Each of vanilla's blocking 8-frame scroll loops becomes one engine-driven animation of the same length, so the input loop's timing is unchanged. Two shared pieces needed extending: the menu NMI hook copies the sell shadow table alongside the field and drops ones, and the engine's per-menu HDMA dispatch gets a branch for the new menu id - its tail case is the key-item builder, so without one the sell list rebuilt the picker's table.
Vanilla's DrawItemSelectCursor computes the hand's VRAM address from $BA + $8C - scroll plus cursor row - because the filtered list was one tall strip that the window scrolled over, so an item's address grew without bound. The ring re-renders in place and wraps $BB instead, so past the first screenful the address walked off the end of the window and drew the hand into unrelated tilemap rows. Fold it into the ring: slot = (scroll + row) mod KEY_ITEM_BUFFER_SLOTS. The push was also a slot short - 0x200 covers four of the ring's five slots, and the fifth is exactly the one a scroll rotates into view, so it reached VRAM holding whatever was there before. The cursor now sits in that slot's row on every frame. Note the patch site: `lda $ba` is at $00:B13D in our ROM, not the $00:B13B the ff4decomp notes give - bank $00 there runs two bytes behind ours from $0089ED on. Patched at the notes' address it landed a byte into `sta $2119` and collapsed the window.
Both lists draw their hand as a sprite whose X is picked from a two-column choice - `lda $1bb4 / beq + / lda #$70` - where the left column's hand sits at X = 0, because vanilla's col-0 names started hard against the window border. Single-column VWF rows put the item symbol at x=24 and the name at x=32, so the hand belonged at x=16 and instead floated against the frame. Both branches now load it, which also makes the column byte irrelevant. The drops hand was five pixels above its own row as well: its Y base was retuned to $28 when the rolling profile moved the band to BG4VOFS -24, but the inventory hand lands exactly on its glyph row ($86 = 134), so drops takes $2D for the same relationship. Measured off the framebuffer and OAM: drops rows now 45 + 16r, the inventory 134 + 16r, both at x = 16.
Vanilla compared the scroll position against a hardcoded 17, sized for the longest list it could draw; the picker builds its list per save, so a 16-item list scrolled five rows of blanks into view before it stopped.
Verified byte-identical output: the only bytes that move are the BUILD_DATE stamp the assembler embeds on every run.
Picks up the block-move formatter fix: `mvn 0x7E, 0x7E` came back from a format pass as `mvn 0x7E`, which the assembler then rejects. The key-item picker's direct-page copy is a block move, so running the formatter over it used to break the build.
`RollingBufferState` is declared in items.i, which ff4.s includes ahead of every rolling module, so the assembler resolves these casts. The lint reads one file at a time and cannot see that, and reported all five as targeting an unknown type. Suppressed rather than papered over by adding an include to each module: the build is correct as it stands, and re-declaring a header the program already composes would be editing working source to satisfy a tool. The reason sits above each marker because noqa codes are comma-separated - trailing prose after `; noqa: S001` is parsed as part of the code and silently stops matching.
Four modules carried their description as a comment above the includes, where DOC001 could not see it, and redraw_gates.s explained its `battle_render` scope from outside the body. Same text, moved into docstrings, with the small VWF renderer's expanded to say what it actually does and why its scratch sits in SRAM.
These are assembled standalone into a flat WRAM image the test then single-steps. There is no ROM layout to place code into, so the bare origin is the point rather than something to modernise into `.alloc at`.
Two defects, both measured against a Palom / Cecil / Porom party. The walk read which character a row shows out of CharOrderTbl and wrote a tilemap row per display index. Neither holds. `DrawCharNames` ($02:A20C) takes both from `$02:A1CD`, the table of battle-struct pointers it indexes by display position: a first byte of zero is the empty slot it skips, and it skips without taking a row, so the names compact upward. The order table disagrees with those pointers - it calls display 0 slot 1 while the pointer reads $2080, slot 2 - which is what lit the wrong name. Slot now comes from the pointer, (base - $2000) / $40, and rows count only the names actually drawn. It also ran from the writer shim, at the instant the engine stores $1822. That is too early to ask what is on screen, and it never came back to check: measured there, four of five slots report hidden, while the same bytes read clear once the battle is running. The shim now just invalidates a key and a per-frame refresh re-applies when the active character or the drawn-name set moves - a compare on an idle frame, still a ~300-cycle palette flip when it fires, never a re-render. Verified by forcing $1822 and reading the ink colour off the framebuffer: Cecil highlights Cecil's row, Palom highlights Palom's.
|
🎮 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.
Treasure, drops, key-item picker and both shop lists now render single-column VWF names through the rolling-buffer engine.
The picker was the hard one. It overlays a live map, so unlike the menus it shares its buffers and its direct page with the field engine, and every wrong assumption showed up as corruption rather than a wrong pixel:
prev_charaliased the MOSAIC shadow,$2106, so scrolling text pixelated the screen) moved to cart SRAM, mapped and pooled through a816.map/.reserveThe shop sell list drew every second item and stopped after six: vanilla keeps all 48 items in the BG3 buffer at 16 bytes each and scrolls over them, which single-column 128-byte rows cannot fit. It gets its own 9-slot ring with BG3VOFS on HDMA channel 5.
Also here: the missing u-circumflex in the 8x8 font (
Flûte Gysahlrendered asFlte), the shop price/cursor geometry, treasure hand-cursor alignment, a Makefile, and the tree formatted and lint-clean (make checkexits 0).