Add battle-init smoke test and pre-push pytest hook - #30
Merged
Conversation
Loads ff4-battle-ext.kss at the Battle_ext seed point, settles 120 frames, asserts the CPU never STPs, and pixel-matches the framebuffer against a PNG golden. Catches the regression that the bank-20 reloc iteration shipped: build + format hooks passed but the engine BRK-trapped during DrawText on first menu interaction. Wire pytest as a pre-push hook so the full kintsuki suite runs once before any push, catching similar codegen regressions before they reach the remote.
|
🎮 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. |
120 frames left the slice-2 redraw gates clean: names and the cmd list intentionally do not draw on the first ATB tick. Bump to 600 frames so ATB fills the first character's command list and the golden actually exercises the rendering path the bank-20 reloc regression broke.
Slice-2 redraw gates stayed clean through the 600-frame settle, so the golden captured battle with empty name + monster headers and only the cmd list visible. Poke battle_menu_dirty ($7E:EF9A) and battle_monster_dirty ($7E:EF9B) to $FF plus the slice-2 region_dirty_bits queue at $70:3C01 before running frames, so DrawCharNames + DrawMonsterNames fire on the first frame. Golden now shows monster sprite + label, char names, cmd list and HP/MP boxes.
Space ($FF) glyph bytes are all zero so the row loop only OR-stores zeros into the tile buffer — visually a no-op, but it still pays the 8 `lda assets_menu_font_dat, x` + `inx` round-trip plus the shift / store dance when bits_left_on_tile is unaligned. Bump X past the glyph rows up front and jump straight to brk_bits_left so the width byte still drives the tilemap advance.
Loads ff4-battle-ext.kss, pokes every redraw-gate dirty bit so the
char names, monster names and command list re-render on every
profiled frame, then runs profile_start over the 4 KB neighbourhood
around battle_render.display_char and prints per-function call
counts plus inclusive / exclusive cycles. Sorts by exclusive so the
hot inner loops surface first. Use as the baseline before / after
any change to the 8x8 VWF blit or the kerning binary search:
python3 tests/_profile/profile_battle_vwf.py > /tmp/vwf.before
# apply change, rebuild
python3 tests/_profile/profile_battle_vwf.py > /tmp/vwf.after
diff /tmp/vwf.before /tmp/vwf.after
Current numbers on master: _display_char 22 calls, 140k excl cycles
over 60 frames; _get_kerning_adjustment_binary_search 22 calls,
84k cycles ; _adjust_bits_left_for_kerning 103k inclusive.
prev_char or current_char of \$FF (space) has no kerning entries in any font's pair table, so the binary search always reports "not found" after walking the full table. Early-out at the top of _adjust_bits_left_for_kerning and fall through to _finalize so prev_char still advances for the next call. Profile delta on the battle VWF bench (60 frames, all redraw gates forced dirty): binary search calls: 22 -> 15 (-32%) binary search cycles: 84792 -> 57670 (-32%) _adjust_bits_left incl: 103824 -> 74088 (-29%) total incl (60f): 3107570 -> 2931590 (-5.7%)
Tier-2 optimisation. The original char_line_loop ran the cmp #8 + jmp-table dispatch + asl ladder per row even though bits_left_on_tile is invariant across the 8 rows of a single glyph. Hoist the classification to glyph entry, dispatch once into one of 8 loops (aligned + shift_0..shift_7) whose asl chain is baked into the body via a .for macro. Aligned drops the redundant ora/sta on the high bitplane (always a noop) for an extra ~80 cy/glyph. X holds the font pointer at entry and the shift dispatch clobbers it for the indirect jump, so the font pointer is stashed in temp first and each shift loop restores it via ldx.b temp on entry. Aligned bypasses save / restore since it never touches X. Profile delta on the battle VWF bench (60 frames, all redraw gates forced dirty): _display_char excl: 139954 -> 97588 (-30%) _display_char avg: 6361 -> 4435 (-30%) total inclusive: 2931590 -> 2416816 (-17.6%) total exclusive: 819092 -> 773366 (-5.6%) Cumulative vs master (space-skip + kerning early-out + tier 2): total inclusive: 3107570 -> 2416816 (-22.2%) _display_char avg: 6365 -> 4435 (-30.3%) ROM cost: +375 bytes in bank-20 reloc (build/ff4.ips 262313 -> 262686). Bank-20 still well under capacity.
Push the space-pair early-out down into each of the three kerning binary searches (battle_render, dialog VWF, small_vwf). The previous patch only short-circuited at _adjust_bits_left_for_kerning, so external callers like battle_msg_kerning_binary_ext and the dialog + small_vwf entry points still walked the full table for space pairs. Now every binary search bails before touching the table when prev_char or current_char is \$FF. Costs ~22 cy per non-space call on the path that already had the adjust-level early-out (battle bench shows +14k inclusive cycles in 60 frames, well within noise) but recovers the win uniformly across all three VWF surfaces and all external callers.
The binary search itself now bails on space pairs, so the adjust-level check is duplicate work. Keep the early-out at the single load-bearing site (the binary search entry) and let the adjust wrapper fall through to its normal jsr.w get_kerning_adjustment which returns carry-set immediately for space pairs.
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 relocation iteration shipped a build that passed format / lint but BRK-trapped during battle redraw on the first menu interaction. Build + format hooks could not see it, and the kintsuki suite never ran on the broken iteration because I pushed the fix before testing locally.
This branch adds two layers of protection.
Layer one is a battle-init smoke test (
tests/test_battle_init.py). It loadsff4-battle-ext.kssat theBattle_extseed point, settles 120 frames, asserts the CPU did not execute STP, and pixel-matches the framebuffer against a PNG golden. The savestate sits at the exact entry the bank-20 reloc work touches (sram_patches + redraw_writer_patches both seed gate state atBattle_ext), so any regression in the bank-02 trampolines or bank-20 module placement shows up either as a STP halt or a screen diff.Layer two is a pytest pre-push hook in
.pre-commit-config.yaml. Tests stay out of the per-commit critical path (134 tests, kintsuki spin-up) but run once before any push, so a broken patch can no longer escape the local checkout.Test plan
pytest tests/test_battle_init.pypasses on the current master treeUPDATE_GOLDENS=1and replays cleanpre-commit run --hook-stage pre-push --all-filestriggers the pytest hook