Simplify teletext hold handling (#470)

Fixes #469 by being simpler too.
mattgodbolt · Jan 5, 2025 · f3a68ce · f3a68ce
1 parent 09dbaa4
commit f3a68ce
Show file tree

Hide file tree

Showing 4 changed files with 18 additions and 9 deletions.
diff --git a/src/teletext.js b/src/teletext.js
@@ -5,7 +5,6 @@ import { makeFast32 } from "./utils.js";
 export class Teletext {
     constructor() {
         this.prevCol = 0;
-        this.holdOff = false;
         this.col = 7;
         this.bg = 0;
         this.sep = false;
@@ -149,7 +148,6 @@ export class Teletext {
     }
 
     handleControlCode(data) {
-        this.holdOff = false;
         const wasGfx = this.gfx;
         const wasHoldChar = this.holdChar;
 
@@ -208,7 +206,7 @@ export class Teletext {
                 this.holdChar = true;
                 break;
             case 31:
-                this.holdOff = true;
+                this.holdChar = false;
                 break;
         }
         if (wasGfx && (wasHoldChar || this.holdChar) && this.dbl === this.oldDbl) {
@@ -322,7 +320,7 @@ export class Teletext {
                 this.heldGlyphs = this.curGlyphs;
             }
         } else {
-            this.holdOff = true;
+            this.heldChar = 32;
         }
 
         if (this.oldDbl) {
@@ -346,10 +344,5 @@ export class Teletext {
                 chardef >>>= 2;
             }
         }
-
-        if (this.holdOff) {
-            this.holdChar = false;
-            this.heldChar = 32;
-        }
     }
 }
diff --git a/tests/integration/teletext.js b/tests/integration/teletext.js
@@ -110,4 +110,17 @@ describe("Test other teletext test pages", () => {
         await testMachine.runUntilInput();
         await compare(video, testMachine, `expected_hoglet_held_char.png`);
     });
+    it("should work with the alternative engineer test page bug 469", async () => {
+        const video = new CapturingVideo();
+        const testMachine = new TestMachine(null, { video: video });
+        await testMachine.initialise();
+        await testMachine.runUntilInput();
+        // https://github.com/mattgodbolt/jsbeeb/issues/469
+        // Taken from the 7th line of the engineer test page from b2.
+        await testMachine.type(
+            "CLS:VDU &81,&80,&81,&A0,&80,&A0,&81,&9E,&A0,&9E,&A0,&97,&AC,&93,&93,&96,&96,&92,&92,&92,&95,&95,&91,&91,&94,&94,&94,&A0,&A0,&94,&80,&81,&80,&81,&80,&81,&80,&81,&B0,&B7",
+        );
+        await testMachine.runUntilInput();
+        await compare(video, testMachine, `expected_bug_469.png`);
+    });
 });
diff --git a/tests/integration/teletext/expected_bug_469.png b/tests/integration/teletext/expected_bug_469.png
diff --git a/tests/test-machine.js b/tests/test-machine.js
@@ -138,6 +138,9 @@ export class TestMachine {
                 case ";":
                     ch = utils.keyCodes.SEMICOLON;
                     break;
+                case ":":
+                    ch = utils.keyCodes.APOSTROPHE;
+                    break;
                 case ",":
                     ch = utils.keyCodes.COMMA;
                     break;