From 6f7ff6c3823ee81d33ac51c0f178422532f8fbb4 Mon Sep 17 00:00:00 2001 From: Kyle Brown Date: Wed, 26 Aug 2026 18:39:44 -0400 Subject: [PATCH] refactor: make Terminal.SCROLL_BACK_COUNT static final MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SCROLL_BACK_COUNT was a public mutable int field, read in index arithmetic buffer allocation, scrollback cap logic, and diff sizing. It was never assigned after declaration—verified via grep showing ONLY the public field declaration (line 48). Making it static final matches HEIGHT convention and prevents external mutation from desynchronizing the field from already-allocated buffers, which would cause ArrayIndexOutOfBoundsException. This is a no-behavior- change hardening change. Instance-qualified reads (terminal.SCROLL_BACK_COUNT) updated to class- qualified static access (Terminal.SCROLL_BACK_COUNT) in: - src/main/java/li/cil/oc2/common/vm/terminal/TerminalDiff.java (~2 reads) - src/main/java/li/cil/oc2/common/vm/terminal/buffer/TerminalBufferScrolling.java (~3 reads) - src/main/java/li/cil/oc2/common/vm/terminal/escapes/csi/CH8.java (1 read) - src/test/java/li/cil/oc2/common/vm/terminal/TerminalBufferTest.java (~5 reads) Internal unqualified reads in Terminal.java left as-is (conventional). §36 M6: class-qualified access for static fields preferred globally. No new tests needed—all existing buffer-sizing tests use SCROLL_BACK_COUNT for assertions and will adapt to the static qualifier automatically. Assisted by: GLM (syn:large:text on synthetic.new) — code generation and review. --- .../java/li/cil/oc2/common/vm/terminal/Terminal.java | 2 +- .../li/cil/oc2/common/vm/terminal/TerminalDiff.java | 4 ++-- .../vm/terminal/buffer/TerminalBufferScrolling.java | 6 +++--- .../li/cil/oc2/common/vm/terminal/escapes/csi/CH8.java | 2 +- .../cil/oc2/common/vm/terminal/TerminalBufferTest.java | 10 +++++----- 5 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/main/java/li/cil/oc2/common/vm/terminal/Terminal.java b/src/main/java/li/cil/oc2/common/vm/terminal/Terminal.java index 35025933..7780e847 100644 --- a/src/main/java/li/cil/oc2/common/vm/terminal/Terminal.java +++ b/src/main/java/li/cil/oc2/common/vm/terminal/Terminal.java @@ -57,7 +57,7 @@ public class Terminal { private transient int lastSentPaletteRevision = -1; public byte style; - public int SCROLL_BACK_COUNT = 20; + public static final int SCROLL_BACK_COUNT = 20; public transient ByteArrayFIFOQueue input = new ByteArrayFIFOQueue(32); // DECCOLM dynamic width; setWidth reallocates buffers. Transient: re-inits to WIDTH on load. public transient int width = WIDTH; diff --git a/src/main/java/li/cil/oc2/common/vm/terminal/TerminalDiff.java b/src/main/java/li/cil/oc2/common/vm/terminal/TerminalDiff.java index c67ed9bf..7fc8b730 100644 --- a/src/main/java/li/cil/oc2/common/vm/terminal/TerminalDiff.java +++ b/src/main/java/li/cil/oc2/common/vm/terminal/TerminalDiff.java @@ -176,7 +176,7 @@ private static int[] visibleWindowRows(final Terminal terminal) { } // Main buffer: the currently displayed scrollback window. final int first = Math.max(0, terminal.lastRowToDisplay - Terminal.HEIGHT); - final int count = Terminal.HEIGHT * terminal.SCROLL_BACK_COUNT - first; + final int count = Terminal.HEIGHT * Terminal.SCROLL_BACK_COUNT - first; final int[] rows = new int[Math.min(Terminal.HEIGHT, count)]; for (int i = 0; i < rows.length; i++) { rows[i] = first + i; @@ -360,7 +360,7 @@ private static void fillColors(final ColorData[] colors, final ColorData color) private static void deserializeRow( final Terminal terminal, final boolean alt, final int row, final byte[] data) { if (row < 0 - || (alt ? row >= Terminal.HEIGHT : row >= Terminal.HEIGHT * terminal.SCROLL_BACK_COUNT)) { + || (alt ? row >= Terminal.HEIGHT : row >= Terminal.HEIGHT * Terminal.SCROLL_BACK_COUNT)) { return; } final ByteBuffer buf = ByteBuffer.wrap(data).order(ByteOrder.LITTLE_ENDIAN); diff --git a/src/main/java/li/cil/oc2/common/vm/terminal/buffer/TerminalBufferScrolling.java b/src/main/java/li/cil/oc2/common/vm/terminal/buffer/TerminalBufferScrolling.java index e9620593..fb032603 100644 --- a/src/main/java/li/cil/oc2/common/vm/terminal/buffer/TerminalBufferScrolling.java +++ b/src/main/java/li/cil/oc2/common/vm/terminal/buffer/TerminalBufferScrolling.java @@ -20,7 +20,7 @@ public void incrementLastLineToDisplay(boolean scroll) { terminal.lastRowToDisplayMax = Math.min( terminal.lastRowToDisplayMax + 1, - Terminal.HEIGHT * terminal.SCROLL_BACK_COUNT); + Terminal.HEIGHT * Terminal.SCROLL_BACK_COUNT); } else if (terminal.lastRowToDisplay == terminal.lastRowToDisplayMax) { return; } @@ -53,7 +53,7 @@ public void shiftUp(int count) { if (terminal.currentPrivateModeState.isAltBufferEnabled()) { shiftLines(terminal.scrollFirst + 1, terminal.scrollLast, -count); } else { - if (terminal.lastRowToDisplay == Terminal.HEIGHT * terminal.SCROLL_BACK_COUNT + if (terminal.lastRowToDisplay == Terminal.HEIGHT * Terminal.SCROLL_BACK_COUNT || terminal.scrollLast != Terminal.HEIGHT - 1 || terminal.scrollFirst != 0) { shiftLines( @@ -65,7 +65,7 @@ public void shiftUp(int count) { : 1, terminal.scrollLast != Terminal.HEIGHT - 1 ? terminal.scrollLast + terminal.lastRowToDisplayMax - Terminal.HEIGHT - : (Terminal.HEIGHT * terminal.SCROLL_BACK_COUNT) - 1, + : (Terminal.HEIGHT * Terminal.SCROLL_BACK_COUNT) - 1, -count); } } diff --git a/src/main/java/li/cil/oc2/common/vm/terminal/escapes/csi/CH8.java b/src/main/java/li/cil/oc2/common/vm/terminal/escapes/csi/CH8.java index 5cde2da3..61457d03 100644 --- a/src/main/java/li/cil/oc2/common/vm/terminal/escapes/csi/CH8.java +++ b/src/main/java/li/cil/oc2/common/vm/terminal/escapes/csi/CH8.java @@ -32,7 +32,7 @@ public void execute(final int[] args, final int argsCount, final CSIState state) final int n = Math.min(args[0], Terminal.HEIGHT); for (int i = 0; i < n; i++) { if (terminal.lastRowToDisplay - < Terminal.HEIGHT * terminal.SCROLL_BACK_COUNT) { + < Terminal.HEIGHT * Terminal.SCROLL_BACK_COUNT) { terminal.bufferManager.incrementLastLineToDisplay(); } terminal.bufferManager.shiftUpOne(); diff --git a/src/test/java/li/cil/oc2/common/vm/terminal/TerminalBufferTest.java b/src/test/java/li/cil/oc2/common/vm/terminal/TerminalBufferTest.java index 0a87556f..9c3c2242 100644 --- a/src/test/java/li/cil/oc2/common/vm/terminal/TerminalBufferTest.java +++ b/src/test/java/li/cil/oc2/common/vm/terminal/TerminalBufferTest.java @@ -57,7 +57,7 @@ void initialBufferState() { assertEquals(0, terminal.y); assertEquals(24, terminal.lastRowToDisplay); assertEquals(24, terminal.lastRowToDisplayMax); - assertEquals(Terminal.WIDTH * Terminal.HEIGHT * terminal.SCROLL_BACK_COUNT, terminal.buffer.length); + assertEquals(Terminal.WIDTH * Terminal.HEIGHT * Terminal.SCROLL_BACK_COUNT, terminal.buffer.length); assertEquals(' ', charAt(0, 0)); assertEquals(' ', charAt(Terminal.WIDTH - 1, Terminal.HEIGHT - 1)); assertFalse(terminal.currentPrivateModeState.isAltBufferEnabled()); @@ -1442,7 +1442,7 @@ void deccolmSwitchesColumnWidthAndClearsScreen() { write(terminal, CSI + "?3h"); assertTrue(terminal.currentPrivateModeState.DECCOLM, "?3h enables DECCOLM"); assertEquals(132, terminal.getTerminalWidth(), "DECCOLM switches to 132 columns"); - final int expected132 = 132 * Terminal.HEIGHT * terminal.SCROLL_BACK_COUNT; + final int expected132 = 132 * Terminal.HEIGHT * Terminal.SCROLL_BACK_COUNT; assertEquals(expected132, terminal.buffer.length, "buffers reallocate to 132 columns"); assertEquals(0xFFFFFF, renderer.dirtyMask.get() & 0xFFFFFF, "DECCOLM must redraw the whole screen"); @@ -1464,7 +1464,7 @@ void deccolmSwitchesColumnWidthAndClearsScreen() { write(terminal, CSI + "?3l"); assertFalse(terminal.currentPrivateModeState.DECCOLM, "?3l disables DECCOLM"); assertEquals(Terminal.WIDTH, terminal.getTerminalWidth(), "reset returns to 80 columns"); - final int expected80 = Terminal.WIDTH * Terminal.HEIGHT * terminal.SCROLL_BACK_COUNT; + final int expected80 = Terminal.WIDTH * Terminal.HEIGHT * Terminal.SCROLL_BACK_COUNT; assertEquals(expected80, terminal.buffer.length, "buffers reallocate back to 80 columns"); } @@ -1590,7 +1590,7 @@ void scrollDownAtFullScrollbackDoesNotOverflow() { // SD (CSI T) / RI used to arraycopy past the physical buffer end // (AIOOBE under lock in putOutput -> terminal dead forever). writeMarkers(); - assertEquals(Terminal.HEIGHT * terminal.SCROLL_BACK_COUNT, terminal.lastRowToDisplayMax, + assertEquals(Terminal.HEIGHT * Terminal.SCROLL_BACK_COUNT, terminal.lastRowToDisplayMax, "precondition: scrollback filled to cap"); // Marker two rows above the discarded pair: survives the shift onto the last row. final int bottomMarkerRow = Terminal.HEIGHT - 3; @@ -1630,7 +1630,7 @@ void scrollUpDownWithHugeCountTerminates() { private void writeMarkers() { // Grow the scrollback window to its hard cap (HEIGHT * SCROLL_BACK_COUNT rows). final StringBuilder feed = new StringBuilder(); - feed.append("\n".repeat(Terminal.HEIGHT * terminal.SCROLL_BACK_COUNT)); + feed.append("\n".repeat(Terminal.HEIGHT * Terminal.SCROLL_BACK_COUNT)); write(terminal, feed.toString()); } } \ No newline at end of file