From: Svjatoslav Agejenko <svjatoslav@svjatoslav.eu>
Date: Sat, 26 Sep 2026 12:31:48 +0000 (+0300)
Subject: fix(interp): treat DECLARE parameter names as placeholders per QB45
X-Git-Url: http://www2.svjatoslav.eu/gitweb/?a=commitdiff_plain;p=crtbasic.git

fix(interp): treat DECLARE parameter names as placeholders per QB45

- bindScope no longer registers the argument cell under a DECLARE-only
  parameter name; only the definition's names bind, matching QB45 where
  DECLARE names are placeholders. The declared parameter still
  contributes its type (by-value cell kind, array/asTyped flags).
  Previously a name mismatch aliased locals to byref arguments
  (truth.bas teeslg's x2 wrote into its caller's x2); verified
  pixel-identical against QB.EXE in DOSBox. Adds regression test
  declareParameterNamesArePlaceholders.
- GameDriver gains --textdump/--textcell: TextDump recognizes each 8xN
  screen cell against the built-in CP437 VGA font into diffable text
  (custom glyphs get stable private-use code points), written wherever
  a $ screenshot lands or at end of run.
---

diff --git a/src/main/java/eu/svjatoslav/crtbasic/interp/Interpreter.java b/src/main/java/eu/svjatoslav/crtbasic/interp/Interpreter.java
index def421b..2fd05d0 100644
--- a/src/main/java/eu/svjatoslav/crtbasic/interp/Interpreter.java
+++ b/src/main/java/eu/svjatoslav/crtbasic/interp/Interpreter.java
@@ -1412,7 +1412,14 @@ public final class Interpreter {
                 }
             }
             scope.put(headerKey, cell);
-            scope.put(declaredKey, cell);
+            // QB45 semantics: DECLARE parameter names are placeholders,
+            // only the definition's names are visible in the body. The
+            // declared param still contributes its TYPE (by-value cell
+            // kind above, array()/asTyped() below) — but the cell is NOT
+            // registered under a declared-only name. (Previously it was,
+            // which aliased locals to arguments when DECLARE and the
+            // definition disagreed on names — truth.bas teeslg's x2
+            // wrote into its caller's x2, unlike real QB45.)
             // An "AS type" parameter claims the bare name: body references
             // canonicalize through the DEFxxx default ("delayInSeconds" →
             // "…%" under DEFINT A-Z), so register the same cell under that
diff --git a/src/test/java/eu/svjatoslav/crtbasic/drivers/GameDriver.java b/src/test/java/eu/svjatoslav/crtbasic/drivers/GameDriver.java
index 0cac09e..bc01812 100644
--- a/src/test/java/eu/svjatoslav/crtbasic/drivers/GameDriver.java
+++ b/src/test/java/eu/svjatoslav/crtbasic/drivers/GameDriver.java
@@ -22,7 +22,7 @@ import java.util.List;
  * <pre>
  *   GameDriver program.bas [--steps=N] [--screenshot=out.png]
  *       [--keys=SCRIPT] [--mouse=SCRIPT] [--interval=MS] [--vars=a%,b%,c$]
- *       [--block]
+ *       [--block] [--textdump=out.txt] [--textcell=N]
  * </pre>
  *
  * <p>Key script: each character is one input event, delivered roughly every
@@ -42,6 +42,11 @@ import java.util.List;
  * so an end-of-run screenshot only catches screens reachable without
  * input.</p>
  *
+ * <p>{@code --textdump} additionally writes the screen as recognized text
+ * (see {@link TextDump}) wherever a screenshot is taken: at every {@code $}
+ * token with the same numbering, or at end of run when no {@code $} was
+ * scripted. {@code --textcell} sets the cell height in pixels (default 8).</p>
+ *
  * <p>Mouse script: space-separated tokens, one event per interval, feeding
  * the emulated qbext mouse TSR:</p>
  * <ul>
@@ -67,7 +72,8 @@ public final class GameDriver {
         if (args.length == 0) {
             System.err.println("Usage: GameDriver program.bas [--steps=N]"
                     + " [--screenshot=out.png] [--keys=SCRIPT] [--mouse=SCRIPT]"
-                    + " [--interval=MS] [--vars=a%,b%] [--block]");
+                    + " [--interval=MS] [--vars=a%,b%] [--block]"
+                    + " [--textdump=out.txt] [--textcell=N]");
             System.exit(1);
         }
         final Path programPath = Path.of(args[0]);
@@ -78,6 +84,8 @@ public final class GameDriver {
         long interval = 12;
         boolean block = false;
         String[] vars = new String[0];
+        Path textdump = null;
+        int textCell = 8;
         for (int i = 1; i < args.length; i++) {
             final String arg = args[i];
             if (arg.startsWith("--steps=")) {
@@ -94,6 +102,10 @@ public final class GameDriver {
                 vars = arg.substring("--vars=".length()).split(",");
             } else if (arg.equals("--block")) {
                 block = true;
+            } else if (arg.startsWith("--textdump=")) {
+                textdump = Path.of(arg.substring("--textdump=".length()));
+            } else if (arg.startsWith("--textcell=")) {
+                textCell = Integer.parseInt(arg.substring("--textcell=".length()));
             } else {
                 System.err.println("Unknown option: " + arg);
                 System.exit(1);
@@ -130,12 +142,19 @@ public final class GameDriver {
             }
             if (i < keys.length()) {
                 final char c = keys.charAt(i);
-                if (c == '$' && screenshot != null) {
+                if (c == '$' && (screenshot != null || textdump != null)) {
                     // Mid-run screenshot: number the path (out-1.png, ...).
                     shotCount++;
-                    final Path target = numbered(screenshot, shotCount);
-                    interpreter.video().dumpPng(target);
-                    System.out.println("Screenshot: " + target);
+                    if (screenshot != null) {
+                        final Path target = numbered(screenshot, shotCount);
+                        interpreter.video().dumpPng(target);
+                        System.out.println("Screenshot: " + target);
+                    }
+                    if (textdump != null) {
+                        final Path textTarget = numbered(textdump, shotCount);
+                        TextDump.dump(interpreter.video(), textTarget, textCell);
+                        System.out.println("Text dump: " + textTarget);
+                    }
                 } else {
                     pushKey(queue, c);
                 }
@@ -151,6 +170,10 @@ public final class GameDriver {
             interpreter.video().dumpPng(screenshot);
             System.out.println("Screenshot: " + screenshot);
         }
+        if (textdump != null && shotCount == 0) {
+            TextDump.dump(interpreter.video(), textdump, textCell);
+            System.out.println("Text dump: " + textdump);
+        }
         System.out.println("Statements executed: " + interpreter.statementsExecuted());
         for (final String variable : vars) {
             System.out.println(variable + " = " + interpreter.variable(variable));
diff --git a/src/test/java/eu/svjatoslav/crtbasic/drivers/TextDump.java b/src/test/java/eu/svjatoslav/crtbasic/drivers/TextDump.java
new file mode 100644
index 0000000..d7ec985
--- /dev/null
+++ b/src/test/java/eu/svjatoslav/crtbasic/drivers/TextDump.java
@@ -0,0 +1,118 @@
+package eu.svjatoslav.crtbasic.drivers;
+
+import eu.svjatoslav.crtbasic.video.VgaDevice;
+import eu.svjatoslav.crtbasic.video.VgaFont;
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.HashMap;
+import java.util.Map;
+
+/**
+ * Dumps a graphics-mode screen as recognized text. Programs that render
+ * their own text with pixel primitives (truth.bas printText) bypass the
+ * text console, so a PNG screenshot is the only output — this reverses
+ * each 8x8 (or 8xN) cell back to a character by matching its pixel
+ * bitmap against the built-in CP437 VGA font, giving tests a
+ * diffable, byte-stable rendering of the screen.
+ *
+ * <p>Per cell the most frequent palette index is taken as the
+ * background; the remaining pixels form the glyph bitmap. Bitmaps that
+ * match no font glyph (custom LINE-drawn symbols, e.g. truth.bas
+ * operator glyphs) are emitted as a stable private-use code point
+ * (U+E000..U+EFFF) derived from the bitmap hash, so dumps of identical
+ * screens stay byte-identical across runs while different bitmaps
+ * virtually never collide.
+ */
+public final class TextDump {
+
+    private TextDump() {
+        // Utility class.
+    }
+
+    /**
+     * Writes the screen contents as text rows of recognized cells.
+     *
+     * @param vga        the video device to read pixels from
+     * @param output     destination text file (UTF-8)
+     * @param cellHeight cell height in pixels (8 for programs that draw
+     *                   their own 8x8 text; 14/16 for console rows)
+     */
+    public static void dump(final VgaDevice vga, final Path output, final int cellHeight)
+            throws IOException {
+        final VgaFont font = VgaFont.forCellHeight(cellHeight);
+        final Map<Long, Character> byBitmap = glyphMap(font);
+        final int width = vga.mode().pixelWidth();
+        final int height = vga.mode().pixelHeight();
+        final StringBuilder text = new StringBuilder();
+        for (int cellY = 0; cellY + cellHeight <= height; cellY += cellHeight) {
+            for (int cellX = 0; cellX + 8 <= width; cellX += 8) {
+                text.append(recognize(vga, cellX, cellY, cellHeight, byBitmap));
+            }
+            text.append('\n');
+        }
+        Files.writeString(output, text.toString());
+    }
+
+    /** Recognizes one cell: background = most frequent color, rest = glyph. */
+    private static char recognize(final VgaDevice vga, final int cellX, final int cellY,
+                                  final int cellHeight, final Map<Long, Character> byBitmap) {
+        final int[] counts = new int[256];
+        for (int y = 0; y < cellHeight; y++) {
+            for (int x = 0; x < 8; x++) {
+                counts[vga.point(cellX + x, cellY + y) & 0xFF]++;
+            }
+        }
+        int background = 0;
+        for (int i = 1; i < counts.length; i++) {
+            if (counts[i] > counts[background]) {
+                background = i;
+            }
+        }
+        long bitmap = 0;
+        for (int y = 0; y < cellHeight; y++) {
+            for (int x = 0; x < 8; x++) {
+                if ((vga.point(cellX + x, cellY + y) & 0xFF) != background) {
+                    bitmap |= 1L << (y * 8 + x);
+                }
+            }
+        }
+        if (bitmap == 0) {
+            return ' ';
+        }
+        final Character glyph = byBitmap.get(bitmap);
+        if (glyph != null) {
+            return glyph;
+        }
+        // Unknown (custom-drawn) glyph: stable private-use code point.
+        return (char) (0xE000 + (Long.hashCode(bitmap) & 0x0FFF));
+    }
+
+    /**
+     * Bitmap-to-character map of the whole font. Printable ASCII is
+     * registered first and wins bitmap collisions (some extended glyphs
+     * duplicate ASCII ones).
+     */
+    private static Map<Long, Character> glyphMap(final VgaFont font) {
+        final Map<Long, Character> byBitmap = new HashMap<>();
+        for (int glyph = 32; glyph < 127; glyph++) {
+            byBitmap.putIfAbsent(bitmapOf(font, glyph), (char) glyph);
+        }
+        for (int glyph = 0; glyph < VgaFont.GLYPH_COUNT; glyph++) {
+            byBitmap.putIfAbsent(bitmapOf(font, glyph), (char) glyph);
+        }
+        return byBitmap;
+    }
+
+    private static long bitmapOf(final VgaFont font, final int glyph) {
+        long bitmap = 0;
+        for (int y = 0; y < font.glyphHeight(); y++) {
+            for (int x = 0; x < 8; x++) {
+                if (font.pixelSet(glyph, x, y)) {
+                    bitmap |= 1L << (y * 8 + x);
+                }
+            }
+        }
+        return bitmap;
+    }
+}
diff --git a/src/test/java/eu/svjatoslav/crtbasic/interp/InterpreterFeaturesTest.java b/src/test/java/eu/svjatoslav/crtbasic/interp/InterpreterFeaturesTest.java
index 4848b08..771ae94 100644
--- a/src/test/java/eu/svjatoslav/crtbasic/interp/InterpreterFeaturesTest.java
+++ b/src/test/java/eu/svjatoslav/crtbasic/interp/InterpreterFeaturesTest.java
@@ -664,4 +664,29 @@ class InterpreterFeaturesTest {
         assertEquals(0.0, interpreter.variable("e"));
         assertEquals(5.0, interpreter.variable("m"));
     }
+
+    @Test
+    void declareParameterNamesArePlaceholders() {
+        // QB45 semantics: DECLARE SUB parameter names are irrelevant,
+        // only the definition's names exist in the body. A body reference
+        // to a name that appears ONLY in the DECLARE is a plain local and
+        // must NOT alias the byref argument. (Regression: crtbasic used to
+        // register the argument cell under the declared name too, so
+        // truth.bas teeslg's x2 wrote into its caller's x2.)
+        final Interpreter interpreter = run("""
+                DECLARE SUB inner (zz!)
+                DIM SHARED probe
+                zz = 10
+                inner zz
+                probe = zz
+                SYSTEM
+                SUB inner (argName)
+                    argName = argName + 1
+                    zz = 999
+                END SUB
+                """);
+        // Byref through the definition's name works (+1), but the body's
+        // zz write is a local and must not clobber the argument (not 999).
+        assertEquals(11.0, interpreter.variable("probe"));
+    }
 }