imjasonh/nescript
1
0
Fork 0
mirror of https://github.com/imjasonh/nescript synced 2026-07-08 17:06:04 +00:00
Code Activity
155 commits 29 branches 0 tags 4 MiB
Commit graph

6 commits

Author SHA1 Message Date
Claude
d6cb84a5bd
compiler: close out bug #4 (W0109 sprite-per-scanline) and bug #5 (real inlining) 
Fixes the last two deferred compiler bugs catalogued in
examples/war/COMPILER_BUGS.md, finishing the bug-cleanup arc on
the War branch.

Bug #5 — `inline fun` inliner
  Previously the `inline` keyword was parsed into `FunDecl.is_inline`
  and then dropped on the floor: every call site emitted a regular
  `JSR` through the $04-$07 transport slots. Now the IR lowerer
  captures inline function bodies up front in
  `LoweringContext::capture_inline_bodies` and rewrites call sites
  at lowering time. Two body shapes are supported:

    1. Single-return expression — the body is re-lowered in place
       of the `Call` op with the parameter names substituted to
       fresh IR temps for each argument.
    2. Void multi-statement body whose every statement is one of
       Assign/Call/Draw/Scroll/SetPalette/LoadBackground/WaitFrame/
       Play/StartMusic/StopMusic/InlineAsm/RawAsm/DebugLog/DebugAssert
       — the statements are spliced into the caller's block with
       the same parameter substitution machinery.

  Control-flow-heavy inline bodies (conditional early returns,
  loops, transitions) fall back to a regular out-of-line call with
  no diagnostic. That's predictable and documented in the bug-tracking
  doc. Nested inline expansion uses a substitution-frame stack so
  an inline calling another inline sees the right arguments.

  A codegen follow-up was needed because bug #3's scope-qualified
  local names broke `{result}` substitution in inline asm. The
  codegen now tracks `current_fn_scope_prefix` per function and the
  InlineAsm op tries the qualified name first before falling back
  to the bare name.

Bug #4 — W0109 sprite-per-scanline static check
  Adds a new warning code W0109 and an analyzer pass
  `check_sprite_scanline_budget` that walks each state's `on_frame`
  handler, collects literal-coordinate `draw` statements (including
  metasprite expansion via dx/dy offsets), and iterates scanlines
  0..240 to count how many 8x8 sprites overlap each line. When a
  scanline has > 8, the analyzer emits W0109 with labels pointing
  at each offending draw site plus a help message about staggering
  y-rows and a note explaining the hardware dropout. Non-literal
  coordinates are skipped (static analysis can't resolve them).
  Nested `if`/`while`/`for`/`loop` blocks are unioned conservatively.

Tests added
  src/ir/tests.rs
    - inline_fun_expression_body_emits_no_call_at_use_site
    - inline_fun_void_body_statements_are_spliced
    - inline_fun_with_conditional_return_compiles_as_regular_call
    - inline_fun_nested_inlines_substitute_correctly
  src/analyzer/tests.rs
    - analyze_sprite_scanline_budget_warns_over_eight
    - analyze_sprite_scanline_budget_ok_when_staggered
    - analyze_sprite_scanline_budget_skips_dynamic_coords
    - analyze_sprite_scanline_budget_expands_metasprites
    - analyze_sprite_scanline_budget_recurses_into_if

COMPILER_BUGS.md
  Bugs #4 and #5 marked **FIXED** in the status table, with full
  reproduction/root-cause/fix/regression-test write-ups updated in
  place. All seven catalogued bugs now have shipped fixes.

Artifact churn
  - examples/war.nes and examples/inline_asm_demo.nes rebuild
    byte-shifted (different JSR targets post-inliner).
  - tests/emulator/goldens/war.audio.hash shifts from 143660f to
    13443e28 — the inliner removes JSRs to set_phase, which nudges
    NMI sampling timing. No pixel diff; behavior is unchanged.

https://claude.ai/code/session_0143dTgh3UeRrtfHgQwzcv5z
 2026-04-15 21:33:00 +00:00
Claude
76dd8eacb0
compiler: fix three scoping bugs; war: revert all local/param workarounds 
Three related scoping bugs from examples/war/COMPILER_BUGS.md,
all fixed in one pass because they're different layer
manifestations of the same "flat global namespace" problem:

## §3: function-local `var` declarations lived in one namespace

`src/analyzer/mod.rs::register_var` inserted every `var` it
saw — top-level, state-local, AND function-body local — into
the same `self.symbols: HashMap<String, Symbol>`. Two different
functions declaring `var i` collided on E0501, which is why
every local in war/*.ne had a function-prefix like `dfa_card`
or `dwp_px`.

Fix: add a `current_scope_prefix: Option<String>` to the
Analyzer, set it to `Some("<fn_name>")` when checking a
function body (or `Some("Title__frame")` for state handler
bodies), and have `register_var` store the declaration under
an internal key `"__local__{prefix}__{name}"`. New
`resolve_symbol` / `resolve_key` helpers try the
scope-qualified key first and fall back to the bare key for
globals / consts / enum variants / state-level vars / function
names. Every existing `self.symbols.get(name)` inside
body-checking code was swapped over.

Two `var i` declarations inside the SAME function body still
collide with E0501 — we scoped per function body, not per
nested block. Per-block scoping would require live-range
analysis to reuse RAM slots.

## §1b: same-named params across functions shared VarIds

`src/ir/lowering.rs::get_or_create_var` looked up names in a
single global `var_map`, so two functions both with a `card:
u8` parameter resolved to the same `VarId`. Whichever function
was lowered last won the zero-page slot mapping, silently
rerouting the other function's param reads to the wrong slot.

Fix: the IR lowerer now mirrors the analyzer's scope logic.
`LoweringContext` gains a `current_scope_prefix` field that
gets set in `lower_function` / `lower_handler`, and
`get_or_create_var` uses a new `scoped_key` helper that
prepends `"__local__{prefix}__"` when the qualified key exists
in `var_map` or `var_types`. Each function's parameters and
locals therefore get distinct VarIds, and the codegen's
`var_addrs` map naturally has no collisions.

## §2: param transport slots $04-$07 clobbered across nested JSRs

Parameters were passed AND kept in `$04-$07` for the lifetime
of a function. Any nested call overwrote those slots with its
own arguments, so the caller's params were silently corrupted
as soon as it invoked anything. Every war helper that took
params and called other helpers (draw_card_face, push_back_a,
etc) snapshotted its params into fresh locals at the top of
the body.

Fix: in `codegen/ir_codegen.rs::IrCodeGen::new`, every
function-local — including parameters — now gets a dedicated
per-function RAM slot at `$0300+`. Parameters are still passed
via the zero-page transport slots `$04-$07` as the calling
convention, but `gen_function` now emits a **prologue** at
every function entry:

    LDA $04
    STA <param_0_addr>
    LDA $05
    STA <param_1_addr>
    ... etc, up to 4 ...

By the time the body runs, every parameter lives in the
function's dedicated RAM slot, so any nested call can freely
clobber $04-$07 (writing its own arguments there) without
corrupting the caller's saved parameters. Costs 4 LDA/STA
pairs (≈ 20 bytes of ROM, 16 cycles) at every function entry
— worth it to make the calling convention sound.

## War cleanup

With all three fixes in place, every workaround prefix in
`examples/war/*.ne` is gone:

- `card_rank(card)` instead of `card_rank(crk_c)` — bug #1b
- `compare_cards(a, b)` instead of `compare_cards(cmp_a, cmp_b)`
- `push_back_a(card)` instead of `push_back_a(pba_in)` — bug #1b
- `var card: u8 = draw_front_a()` in bury_from_* — bug #3
- `var i: u8 = 0` freely in multiple functions — bug #3
- `fun push_back_a(card)` body no longer snapshots `card` into
  `pba_card` before calling wrap52 — bug #2
- `fun draw_card_face` body no longer snapshots x/y/card into
  locals before calling card_rank/card_suit — bug #2
- `draw_word_player` steps its own x without needing a
  `dwp_px` accumulator to avoid the `x + N` arg compilation
  quirk — that quirk was a downstream symptom of bug #2 and
  is also gone

The source is now about 300 lines shorter and significantly
more readable.

## Regression tests

Seven new tests nail these bugs down:

- `analyzer::tests::analyze_allows_same_local_name_in_two_functions`
- `analyzer::tests::analyze_allows_same_param_name_in_two_functions`
- `analyzer::tests::analyze_allows_same_local_name_in_two_state_handlers`
- `analyzer::tests::analyze_still_rejects_duplicate_local_in_same_function`
- `codegen::ir_codegen::gen_function_prologue_spills_params_to_local_ram`

Plus the four param-arity tests from the earlier E0506 fix
and the wide_hi-leak regression test from the previous
compiler fix. Total suite: 591 unit tests, all passing.

## Golden drift

The prologue change adds a few cycles to every function entry,
which shifts NMI sampling by a handful of cycles and flips
the audio-hash of any example that plays sfx or music
(platformer, war). `arrays_and_functions.png` also picks up a
1-pixel shift in its enemy positions due to the same timing
drift. All three golden updates are pure "compiler produces
different but functionally-identical output" — no game
behavior changed.

## What's still open in COMPILER_BUGS.md

- §4: 8-sprites-per-scanline hardware limit is invisible to
  user code. A static analyzer hint could help; deferred.
- §5: `inline` keyword is silently declined for short
  functions that the optimizer's inliner doesn't recognize
  (it only removes empty functions). Deferred pending a real
  single-return-expression inlining pass.

https://claude.ai/code/session_0143dTgh3UeRrtfHgQwzcv5z
 2026-04-15 20:33:41 +00:00
Claude
e10d09db76
examples/war: redesign card art — opaque bodies, 16x16 pips, checkerboard back 
The first-pass card tiles were riddled with palette-0 transparent
pixels that let the felt background bleed through every rank
glyph, small suit, and big pip. At arm's length the cards looked
like they had green specks eating them. This commit rewrites all
of the card art from scratch:

- **Opaque card bodies.** Every pixel inside a card tile is now
  either white (palette 2), red (1), or black (3). No `.` values
  anywhere on a face or back tile. The green felt only shows
  outside the card rectangle, where it should.

- **Readable rank glyphs.** The 13 rank tiles (A, 2-9, 10, J, Q, K)
  are now drawn as bold black strokes on a solid white body. The
  "holes" of the letters (e.g. the triangle inside an "A") are
  white, not transparent.

- **16x16 big pips.** The big centre pip is now a 4-tile (16x16)
  shape split into TL/TR/BL/BR quadrants per suit. Previously it
  was a 2-tile (16x8) half-height strip that looked cramped. The
  TL/TR quadrants kept their existing tile indices (28-35) so the
  shift is local; the new BL/BR quadrants are appended after the
  BIG WAR letters at tiles 88-95 to avoid renumbering the entire
  alphabet / digits / UI tile range.

- **Distinct suit shapes.** Spade is a smooth teardrop with a
  short stem and base; heart is two symmetric lobes with a V
  bottom; diamond is a clean rhombus; club is three circles
  joined over a stem. Side-by-side they are unmistakable.

- **Clean checkerboard card back.** The old card back was a
  diamond lattice that had the same transparent-bleed problem
  and looked noisy anyway. Replaced with a crisp 2-pixel black-
  and-white checkerboard that tiles seamlessly across the card
  back's 16x24 footprint.

- **draw_card_face now emits 6 sprites in a rank/suit + 4-tile
  big-pip layout.** The previous 6-sprite layout was
  `[rank][ssuit] / [pipL][pipR] / [blankL][blankR]`; the new
  one is `[rank][ssuit] / [pipTL][pipTR] / [pipBL][pipBR]` with
  the bottom row carrying the bottom half of the big pip
  instead of being wasted blank tiles.

Also:
- New constants TILE_PIP_TL_BASE / TR / BL / BR replace the old
  TILE_PIP_L_BASE / TILE_PIP_R_BASE in constants.ne.
- Refreshed war.nes and the goldens. Every emulator harness
  test still passes (31/31).

https://claude.ai/code/session_0143dTgh3UeRrtfHgQwzcv5z
 2026-04-15 18:31:15 +00:00
Claude
9137b1f713
examples/war: polish pass + README entry + plan close-out 
End-of-implementation polish for the War example after the
compiler bugs were fixed:

- Title state now calls draw_big_war_banner instead of inlining
  12 draws — same pixel output, fewer lines.
- P_WAR_BURY redraws the previous round's face-up cards while
  the noise thumps fire so the table doesn't look empty for
  24 frames between the WAR banner and the new face-ups.
- Drop draw_word_war from render.ne (orphaned by the BIG WAR
  metasprite).
- Refresh comments in background.ne (now references the real
  felt tile) and deal_state.ne (drop the stale FRAMES_DEAL_STEP
  reference now that the deal pace is hard-coded at 2 frames).
- README.md and examples/README.md gain a war row.
- PLAN.md marks every implementation step complete and records
  the design revisions made along the way.
- Refresh the war audio hash to match the new ROM (the title
  screen helper change shifts one frame of pulse-2 timing
  enough to flip the FNV-1a). The frame-180 PNG is unchanged.

https://claude.ai/code/session_0143dTgh3UeRrtfHgQwzcv5z
 2026-04-15 16:08:03 +00:00
Claude
4e8e349d7c
ir: clear wide_hi between functions to fix 16-bit op aliasing 
The IrLowerer's wide_hi map records "this u8 temp's high byte
lives at this other temp" pairs whenever a 16-bit value is
produced. Both lower_function and lower_handler reset next_temp
to 0 at the start of each function, but neither cleared wide_hi
— so stale (low_id -> high_id) entries from earlier functions
leaked into subsequent ones.

When a fresh function reused those temp IDs for unrelated u8
expressions, is_wide() returned spurious true and widen() handed
back stale (lo, hi) pairs whose hi happened to coincide with the
*next* temp ID fresh_temp() was about to allocate. The result
was 16-bit IR ops (CmpEq16 in particular) where the destination
temp aliased one of the source operand high bytes — for War this
made `match phase` arms past P_WIN_B impossible to enter and the
game would freeze with both face-up cards on the table forever.

Fix: clear wide_hi alongside the next_temp reset in both
lower_function and lower_handler. Adds a regression test
(ir::tests::wide_hi_does_not_leak_between_functions) that
constructs a function whose body has no u16 ops but follows a
function that does, and asserts no CmpEq16 op aliases its dest
with an operand high byte.

Also:
- Convert the war Playing state's phase machine from an
  if-chain to a `match`, which is what tripped this bug to the
  surface (it was lurking in earlier ROMs too but their layouts
  never produced the dest/source collision shape).
- Refactor begin_draw_a/b to set fly_card / fly_face_up via
  globals before calling arm_fly, since arm_fly only takes 4
  params (the v0.1 ABI limit, now diagnosed by E0506).
- Hoist the P_RESOLVE comparison result to the global pf_result
  to dodge the param-clobbering issue documented in
  examples/war/COMPILER_BUGS.md §2.
- Document the bug as item #6 in COMPILER_BUGS.md with a
  minimal repro and reproducer-test pointer.
- Refresh the war golden + audio hash to match the new ROM.

https://claude.ai/code/session_0143dTgh3UeRrtfHgQwzcv5z
 2026-04-15 15:57:26 +00:00
Claude
8ababdcec4
examples/war: working end-to-end War card game 
A complete, playable port of the card game War: title screen with
0/1/2 player menu, animated deal, sliding cards, deck-count HUD, a
"WAR!" tie-break with buried cards, and a victory screen with a
fanfare. Source split across examples/war/*.ne (constants, assets,
audio, deck/queue logic, RNG, render helpers, and one state file
per game state) and pulled in via examples/war.ne.

Drives nearly every NEScript subsystem at once: custom 88-tile
sprite sheet (card frames, ranks, suits, font, BIG WAR letters);
felt background nametable; pulse-1 / pulse-2 / noise sfx; looping
march on pulse 2; an 8-bit Galois LFSR PRNG; queue-based decks
that conserve cards across rounds; a phase machine inside the
Playing state that handles draw/reveal/win/war/check; and an
autopilot that boots straight into 0-PLAYERS mode so the headless
jsnes harness captures real gameplay at frame 180.

While building this I uncovered five compiler bugs / limitations
in the v0.1 implementation; each is documented with a minimal
reproduction, root cause, current workaround, and proposed fix in
examples/war/COMPILER_BUGS.md. The most painful was the
parameter-VarId aliasing one (#1b) — two functions sharing a
parameter NAME end up sharing a single zero-page slot mapping
across the whole program. Once those compiler bugs are fixed, the
workarounds in war/*.ne should be reverted in the same PR.

https://claude.ai/code/session_0143dTgh3UeRrtfHgQwzcv5z
 2026-04-15 15:22:20 +00:00