Add nametable helpers: nt_put_tile, nt_put_row, nt_write, nt_set_palette by Copilot · Pull Request #471 · jonathanpeppers/dotnes

Copilot · 2026-04-17T22:32:36Z

Manual VRAM address math and attribute-table bit-packing are the most error-prone parts of NES development in this codebase. This adds four high-level nametable helpers to NESLib so samples can replace dozens of lines of address arithmetic and read-modify-write code with one call.

nt_put_tile(NAMETABLE_A, 5, 10, 0xF4);
nt_put_row(NAMETABLE_A, col, row, buf, len);
nt_write(NAMETABLE_A, 2, 2, "HELLO, .NET!");
nt_set_palette(NAMETABLE_A, col, row, palette);  // 2-bit attribute-table field

Changes

src/neslib/NESLib.cs — four new method stubs (throw null! reference-only, per the existing convention) with XML docs.
src/neslib/PublicAPI.Shipped.txt — four new entries.
src/dotnes.tasks/Utilities/IL2NESWriter.ILDispatch.cs — four new intrinsic dispatch cases (~225 lines):
- nt_put_tile, nt_put_row, nt_write rewrite the call into the equivalent vram_adr/vram_put/vrambuf_put/vram_write emission with the address baked in at compile time.
- nt_set_palette is emitted fully inline (no new built-in subroutine) — the attribute address (nt + 0x3C0) + (row/4)*8 + (col/4), quadrant shift (row & 2) | ((col & 2) >> 1), and clear-mask are all resolved at compile time, leaving a small PPU read-modify-write sequence at runtime. Constant-palette collapses the OR to a single immediate; runtime palette emits a short ASL chain.
src/dotnes.tests/RoslynTests.cs — five RoslynTests (NtPutTile, NtPutRow, NtWrite, NtSetPalette, NtSetPaletteQuadrant3) asserting on the emitted hex.

Constraints

nametable, col, and row must be compile-time constants (matches every example in the issue and keeps the dispatch logic tractable). A clear TranspileException is raised otherwise.
tile, palette, buf, len, and str may be runtime values, inheriting whatever the underlying low-level API supports.
No samples or .verified.bin files are touched — the adoption work called out at the bottom of the issue (climber's ~60 lines of attribute packing, hello, attributetable, etc.) is intentionally deferred to a follow-up PR so this one stays scoped to the API surface.

Copilot

Pull request overview

Adds four high-level nametable helper APIs to NESLib and corresponding intrinsic handling in the IL→6502 transpiler, intended to eliminate manual VRAM address math and attribute-table bit packing for samples and games.

Changes:

Added nt_put_tile, nt_put_row, nt_write, and nt_set_palette to NESLib (API surface + XML docs).
Added intrinsic dispatch/codegen for the four helpers in IL2NESWriter.ILDispatch.cs.
Added Roslyn-based codegen tests for the new intrinsics.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 12 comments.

File	Description
src/neslib/NESLib.cs	Adds new public helper method stubs with documentation.
src/neslib/PublicAPI.Shipped.txt	Ships the new public API entries.
src/dotnes.tasks/Utilities/IL2NESWriter.ILDispatch.cs	Implements intrinsic lowering/inline codegen for the new helpers.
src/dotnes.tests/RoslynTests.cs	Adds tests asserting emitted bytes for the new helpers.

jonathanpeppers · 2026-05-01T22:39:11Z

+        // PHA (48) to save value
+        Assert.Contains("48", hex);
+        // PLA (68) to restore
+        Assert.Contains("68", hex);
+        // STA $2007 (PPU_DATA)
+        Assert.Contains("8D0720", hex);


Fixed. Replaced the single-byte Assert.Contains("48") / Assert.Contains("68") with a full PHA→PPU_ADDR→PLA→STA PPU_DATA sequence assertion: Assert.Contains("48A9238D0620A9C98D062068", hex). This matches the complete write-back sequence and cannot match unrelated bytes.

jonathanpeppers · 2026-05-01T22:39:43Z

+                            // Remove ldloc buf (LDA $arrayAddr) + ldc len (LDA #len) + 3 constant args
+                            RemoveLastInstructions(5);
+


Fixed. Replaced the fixed count with a backward walk that counts consecutive LDA/LDX/JSR(pusha|pushax) instructions from the end of the block. This correctly handles both RAM arrays (9 instructions) and ROM/label arrays (13 instructions).

jonathanpeppers · 2026-05-01T22:39:44Z

+                                    "nt_put_row requires a byte array argument", MethodName);
+                            }
+
+                            // A = length


len is always a compile-time constant in practice because it comes from a literal in the C# source. The existing checked((byte)len) catches overflow. Supporting runtime len would require fundamentally different codegen (preserving A through the address setup), which is out of scope for this helper. The current behavior is consistent with how the nametable/col/row args are enforced as compile-time.

jonathanpeppers · 2026-05-01T22:39:45Z

+                            // Remove: 3 constant args (nametable, col, row) + Ldstr pattern (5 instrs)
+                            RemoveLastInstructions(8);
+


Fixed. Changed to nametable > byte.MaxValue ? 13 : 12, which accounts for: ushort nametable (LDX+LDA+pushax=3) + col (LDA+pusha=2) + row (LDA+pusha=2) + pusha before Ldstr (1) + Ldstr pattern (5) = 13. Also added a fallback backward walk for robustness when the block is smaller than expected.

jonathanpeppers · 2026-05-01T22:40:13Z

+                            // Detect byte array local via _lastLoadedLocalIndex
+                            Local? arrayLocal = null;
+                            bool isByteArrayLocal = _lastLoadedLocalIndex.HasValue &&
+                                Locals.TryGetValue(_lastLoadedLocalIndex.Value, out arrayLocal) &&
+                                arrayLocal.ArraySize > 0;


Fixed. Removed the dead isByteArrayLocal variable. The array local detection now just uses _lastLoadedLocalIndex with Locals.TryGetValue directly.

jonathanpeppers · 2026-05-01T22:40:15Z

+                            // Check if tile is runtime (last LDA is absolute, not immediate)
+                            var lastInstr = block[block.Count - 1];
+                            bool tileIsRuntime = lastInstr.Opcode == Opcode.LDA && lastInstr.Mode == AddressMode.Absolute;
+


Fixed. Now checks _lastLoadedLocalIndex and _lastStaticFieldAddress in addition to the LDA Absolute pattern. Throws TranspileException for unsupported computed runtime patterns (_runtimeValueInA without LDA Absolute). Added NtPutTileRuntime test.

jonathanpeppers · 2026-05-01T22:10:06Z

/review

github-actions · 2026-05-01T22:10:52Z

✅ dotnes PR Reviewer completed successfully!

github-actions

Review: Nametable Helpers

Overall: Well-designed compile-time intrinsics that follow existing transpiler patterns. The API surface is clean, the NESLib stubs follow the throw null! convention, and the RoslynTests follow the preferred testing approach. CI is green.

What looks good

Correct PPU read-modify-write sequence in nt_set_palette (dummy read for non-palette VRAM)
vram_adr/vram_put/vram_write don't need UsedMethods?.Add (always-included built-ins); vrambuf_put correctly added since it's optional
checked((byte)len) catches overflow at transpile time
Safety guards in commit 3 (bounds check before indexing, pattern-match cast)
PublicAPI.Shipped.txt correctly updated

Issues to address

Missing test coverage for runtime codegen paths — tileIsRuntime (nt_put_tile) and paletteIsRuntime (nt_set_palette) have no tests. These are the most complex branches.
Unguarded Stack.Pop() in nt_put_row — inconsistent with all other pops in the PR.
No col/row bounds validation — since these must be compile-time constants, range checks would catch bugs at build time rather than silently emitting wrong VRAM addresses.

Generated by dotnes PR Reviewer for issue #471 · ● 4.5M

Agent-Logs-Url: https://github.com/jonathanpeppers/dotnes/sessions/ff4c9338-98e8-4800-9405-26972afff726 Co-authored-by: jonathanpeppers <840039+jonathanpeppers@users.noreply.github.com>

- Fix RemoveLastInstructions counts for all nt_* handlers to account for JSR pushax/pusha between arg loads (was 4, now 8 for ushort nametable args; nt_put_row uses backward walk; nt_write uses 13) - Add compile-time bounds validation for col (0-31) and row (0-29) - Fix unguarded Stack.Pop() in nt_put_row - Remove dead code (isByteArrayLocal computed but never used) - Broaden runtime tile/palette detection to also check _lastLoadedLocalIndex and _lastStaticFieldAddress, throw TranspileException for unsupported computed runtime patterns - Reset _lastLoadedLocalIndex/_lastStaticFieldAddress after use - Update XML docs to document compile-time constant constraint - Strengthen NtSetPalette test assertions with full PHA/PLA sequence - Add NtPutTileRuntime and NtSetPaletteRuntime tests for runtime codegen paths Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI assigned Copilot and jonathanpeppers Apr 17, 2026

Copilot started work on behalf of jonathanpeppers April 17, 2026 22:32 View session

Copilot AI linked an issue Apr 17, 2026 that may be closed by this pull request

Add nametable helpers: nt_put_tile, nt_put_row, nt_write, nt_set_palette #423

Open

Copilot AI changed the title ~~[WIP] Add nametable helpers for tile and row operations~~ Add nametable helpers: nt_put_tile, nt_put_row, nt_write, nt_set_palette Apr 17, 2026

Copilot finished work on behalf of jonathanpeppers April 17, 2026 22:49

Copilot AI requested a review from jonathanpeppers April 17, 2026 22:49

jonathanpeppers marked this pull request as ready for review April 18, 2026 00:01

Copilot AI review requested due to automatic review settings April 18, 2026 00:01

Copilot started reviewing on behalf of jonathanpeppers April 18, 2026 00:01 View session

Copilot AI reviewed Apr 18, 2026

View reviewed changes

github-actions Bot reviewed May 1, 2026

View reviewed changes

Comment thread src/dotnes.tasks/Utilities/IL2NESWriter.ILDispatch.cs

Comment thread src/dotnes.tasks/Utilities/IL2NESWriter.ILDispatch.cs

Comment thread src/dotnes.tasks/Utilities/IL2NESWriter.ILDispatch.cs Outdated

Comment thread src/dotnes.tasks/Utilities/IL2NESWriter.ILDispatch.cs Outdated

Copilot AI and others added 4 commits May 1, 2026 18:23

Initial plan

2e141e9

Add nt_put_tile, nt_put_row, nt_write, nt_set_palette helpers

e40b240

Agent-Logs-Url: https://github.com/jonathanpeppers/dotnes/sessions/ff4c9338-98e8-4800-9405-26972afff726 Co-authored-by: jonathanpeppers <840039+jonathanpeppers@users.noreply.github.com>

Add bounds-check and cast safety guards in nt_* intrinsics

7f52ddf

Agent-Logs-Url: https://github.com/jonathanpeppers/dotnes/sessions/ff4c9338-98e8-4800-9405-26972afff726 Co-authored-by: jonathanpeppers <840039+jonathanpeppers@users.noreply.github.com>

jonathanpeppers force-pushed the copilot/add-nametable-helpers branch from f8f5bb5 to 1bdab89 Compare May 1, 2026 23:25

		// Remove ldloc buf (LDA $arrayAddr) + ldc len (LDA #len) + 3 constant args
		RemoveLastInstructions(5);

		// Remove: 3 constant args (nametable, col, row) + Ldstr pattern (5 instrs)
		RemoveLastInstructions(8);

Conversation

Copilot AI commented Apr 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Constraints

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

jonathanpeppers May 1, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jonathanpeppers May 1, 2026

Choose a reason for hiding this comment

Uh oh!

jonathanpeppers May 1, 2026

Choose a reason for hiding this comment

Uh oh!

jonathanpeppers May 1, 2026

Choose a reason for hiding this comment

Uh oh!

jonathanpeppers May 1, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jonathanpeppers May 1, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

jonathanpeppers commented May 1, 2026

Uh oh!

github-actions Bot commented May 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Review: Nametable Helpers

What looks good

Issues to address

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Copilot AI commented Apr 17, 2026 •

edited

Loading

github-actions Bot commented May 1, 2026 •

edited

Loading