TODO.md

@@ -34,6 +34,27 @@ wizard commands).
 
 # Completed Steps
 
+- 2026-08-09 Command dispatch audit (`audit/command-switch-coverage`, closes
+  #31): checked every case label in C's `command.c` against this port's
+  dispatch, and left the audit behind as a standing test
+  (`game/dispatch_test.go`) so the two cannot silently drift again. **No further
+  missing keys were found** — `'+'` (#11) was the only one. That is the result,
+  and it is worth recording as a negative: the class of bug exists, it has now
+  been searched for exhaustively rather than stumbled upon, and the search came
+  back empty. Three tables transcribe C's labels with their line numbers:
+  main-switch keys answered from `commandHandlers`, main-switch keys whose arms
+  need `dispatchKey`'s own switch (the `goto over` re-dispatches, `F`-to-`f`,
+  `a`, `m`), and the `if (wizard)` sub-switch. `commandHandlers` is pinned by
+  set equality in **both** directions: a missing key is the `'+'` bug, and an
+  extra key is the same bug mirrored — a MASTER debug command leaking into
+  ordinary play. Two traps make this audit harder than it sounds and are
+  documented in the file: `rogue.h` 52-53 defines `when` as `break;case`, so a
+  grep for `case ` finds ten of the eighty labels; and the main/wizard split is
+  load-bearing, since `'+'` was a divergence in ordinary play precisely because
+  it is a main-switch key. Confirms the port targets the MASTER build — all four
+  `#ifdef MASTER` sites in `command.c` are ported unconditionally, as is
+  `sticks.c` 237.
+
 - 2026-08-09 Three small lost C behaviors (`fix/lost-c-behaviors`, closes #13):
   grouped because each is a few lines and all are "restore something the port
   dropped silently". (1) **"what a bizarre schtick!"**, `sticks.c` 237 — the

game/dispatch_test.go

@@ -0,0 +1,248 @@
+//nolint:testpackage // white-box tests reach unexported state (approved 2026-07-07)
+package game
+
+import (
+	"strings"
+	"testing"
+)
+
+// This file is the standing form of the issue #31 audit: every case
+// label in C's command switch against this port's dispatch. It exists
+// because a missing dispatch entry is the one porting error that leaves
+// no trace at build time. The port is function-by-function, so every C
+// function has a Go counterpart and a dropped key dangles nothing and
+// fails to compile nowhere; it simply answers "illegal command" the
+// first time a player presses it. That is how '+' (issue #11) survived
+// until PR #30 found it by accident.
+//
+// The tables below are transcribed from origin/c-master:command.c, with
+// the C line numbers alongside. Read them there with rogue.h 52-53 in
+// hand:
+//
+//	#define when		break;case
+//	#define otherwise	break;default
+//
+// The labels are therefore written "when 'x':", and a grep for "case "
+// finds ten of the eighty. CTRL is extern.h:113, (c & 037); ESCAPE is
+// rogue.h:121, 27.
+//
+// There are two switches and the split between them is load-bearing. A
+// key C answers from the main switch (151-427) must be answered here
+// whether or not wizard mode is on. A key C answers only from the
+// "if (wizard) switch (ch)" sub-switch (369-423) must not be reachable
+// outside it. '+' was a divergence in ordinary play, not just in wizard
+// mode, precisely because it is a main-switch key.
+//
+// The whole sub-switch, and '+' with it, is #ifdef MASTER. This port
+// targets the MASTER build: all four #ifdef MASTER sites in command.c
+// (67, 128, 317, 368) are ported unconditionally, as is sticks.c 237.
+
+// cMainSwitchTableKeys are the main-switch labels whose arms are a plain
+// call, and which this port therefore answers from commandHandlers.
+func cMainSwitchTableKeys() []byte {
+	return []byte{
+		',',                                    // 153
+		'!',                                    // 180
+		'h', 'j', 'k', 'l', 'y', 'u', 'b', 'n', // 181-188  do_move
+		'H', 'J', 'K', 'L', 'Y', 'U', 'B', 'N', // 189-196  do_run
+		't',                                    // 241
+		'q', 'Q', 'i', 'I', 'd', 'r', 'e', 'w', // 258-269
+		'W', 'T', 'P', 'R', 'o', 'c', // 270-275
+		'>', '<', '?', '/', 's', 'z', 'D', // 276-286
+		CTRL('P'), CTRL('R'), // 287-291
+		'v',           // 292
+		'S',           // 295
+		'.',           // 298  rest
+		' ',           // 299  "legal" illegal command
+		'^',           // 300
+		'+',           // 318  (#ifdef MASTER)
+		Escape,        // 339
+		')', ']', '=', // 354-360
+		'@', // 361
+	}
+}
+
+// cMainSwitchMultiStepKeys are the main-switch labels whose arms need
+// more than a call — C's "goto over" re-dispatch, or the F-to-f
+// fallthrough — and which this port therefore answers from dispatchKey's
+// own switch rather than from commandHandlers. They are main-switch keys
+// all the same, and a player reaches them without wizard mode.
+func cMainSwitchMultiStepKeys() []byte {
+	return []byte{
+		CTRL('H'), CTRL('J'), CTRL('K'), CTRL('L'), // 197
+		CTRL('Y'), CTRL('U'), CTRL('B'), CTRL('N'), // 198
+		'F', // 214  sets kamikaze, then falls through
+		'f', // 217
+		'a', // 246
+		'm', // 344
+	}
+}
+
+// cWizardSwitchKeys are the labels of the "if (wizard) switch (ch)"
+// sub-switch, which sits inside the main switch's otherwise: arm.
+func cWizardSwitchKeys() []byte {
+	return []byte{
+		'|',                  // 371
+		'C',                  // 372
+		'$',                  // 373
+		CTRL('G'), CTRL('W'), // 374-375
+		CTRL('D'), CTRL('A'), // 376-377
+		CTRL('F'), CTRL('T'), // 378-379
+		CTRL('E'), CTRL('C'), // 380-381
+		CTRL('X'), // 382
+		CTRL('~'), // 383
+		CTRL('I'), // 390
+		'*',       // 419
+	}
+}
+
+// assertNotIllegalCommand fails if the top line reports the key as
+// illegal. illcom is the only thing that writes that message, so it is a
+// reliable "the dispatch had no arm for this key" probe: it holds
+// whether the arm printed its own message, printed nothing, or cleared
+// the line on the way out.
+func assertNotIllegalCommand(t *testing.T, g *RogueGame, ch byte) {
+	t.Helper()
+
+	// End() upper-cases the first letter, so match from the second.
+	if line := g.scr.Std.Line(0); strings.Contains(line, "llegal command") {
+		t.Errorf("dispatching '%s' reached illcom (top line %q); C answers "+
+			"it from command.c's switch", unctrl(ch), strings.TrimSpace(line))
+	}
+}
+
+// TestCommandHandlersMatchCMainSwitch pins commandHandlers to exactly the
+// set of main-switch keys C answers with a plain call. It is checked in
+// both directions on purpose. A missing key is the '+' bug. An extra key
+// is the same bug mirrored: the most likely way to acquire one is to
+// promote a key out of the wizard sub-switch, which would make a MASTER
+// debug command available in ordinary play.
+func TestCommandHandlersMatchCMainSwitch(t *testing.T) {
+	t.Parallel()
+
+	handlers := newGameData().commandHandlers
+	keys := cMainSwitchTableKeys()
+
+	want := make(map[byte]bool, len(keys))
+
+	for _, ch := range keys {
+		want[ch] = true
+
+		if _, ok := handlers[ch]; !ok {
+			t.Errorf("commandHandlers has no entry for '%s'; C answers it "+
+				"from the main command.c switch, so this port says "+
+				"\"illegal command\" where C does not", unctrl(ch))
+		}
+	}
+
+	for ch := range handlers {
+		if !want[ch] {
+			t.Errorf("commandHandlers has an entry for '%s' that C's main "+
+				"switch does not; if C answers it only under if (wizard), "+
+				"it belongs in wizardCommand", unctrl(ch))
+		}
+	}
+
+	if len(want) != len(keys) {
+		t.Errorf("cMainSwitchTableKeys lists %d keys, %d of them distinct; "+
+			"a duplicate hides a missing key", len(keys), len(want))
+	}
+}
+
+// TestDispatchKeyAnswersCMultiStepKeys covers the main-switch keys that
+// commandHandlers cannot hold, which the set-equality test above cannot
+// see. Removing one of these from dispatchKey's switch is just as silent
+// as removing a map entry: it falls into the default arm and lands on
+// illcom, so that is what is checked.
+func TestDispatchKeyAnswersCMultiStepKeys(t *testing.T) {
+	t.Parallel()
+
+	for _, ch := range cMainSwitchMultiStepKeys() {
+		t.Run(unctrl(ch), func(t *testing.T) {
+			t.Parallel()
+
+			g := mkGameInput(t)
+			// Not a wizard: these are ordinary-play keys, so the default
+			// arm they must not reach is illcom itself.
+			g.Wizard = false
+			g.Options.Terse = false
+			// 'a' replays the last command; give it one to replay so it
+			// takes its re-dispatch arm rather than its complaint arm.
+			g.LastComm = '.'
+			// F, f and m prompt for a direction. Escape backs out of the
+			// prompt, which keeps them from moving the hero or starting a
+			// fight while still proving their arm ran.
+			setInput(t, g, Escape, Escape, Escape)
+
+			next, again := g.dispatchKey(ch)
+			t.Logf("dispatchKey(%s) = %s, again=%v",
+				unctrl(ch), unctrl(next), again)
+
+			assertNotIllegalCommand(t, g, ch)
+		})
+	}
+}
+
+// TestDispatchKeyRedispatchesCtrlDirections is the positive half of the
+// test above for the eight ctrl-directions: C's arm converts the key to
+// its upper-case run command and does "goto over" (command.c 197-213),
+// which this port spells as a true second result. Checking the returned
+// key, and not merely that illcom was missed, is what would catch the
+// arm being present but wired to the wrong direction.
+func TestDispatchKeyRedispatchesCtrlDirections(t *testing.T) {
+	t.Parallel()
+
+	for _, ch := range []byte{
+		CTRL('H'), CTRL('J'), CTRL('K'), CTRL('L'),
+		CTRL('Y'), CTRL('U'), CTRL('B'), CTRL('N'),
+	} {
+		t.Run(unctrl(ch), func(t *testing.T) {
+			t.Parallel()
+
+			g := mkGameInput(t)
+
+			// C's "ch += ('A' - CTRL('A'))": ctrl-h becomes 'H'.
+			wantCh := ch + 'A' - CTRL('A')
+
+			next, again := g.dispatchKey(ch)
+			if !again {
+				t.Fatalf("dispatchKey(%s) did not ask to re-dispatch; C's "+
+					"arm ends in goto over", unctrl(ch))
+			}
+
+			if next != wantCh {
+				t.Errorf("dispatchKey(%s) re-dispatched as %q, want %q",
+					unctrl(ch), next, wantCh)
+			}
+		})
+	}
+}
+
+// TestWizardDispatchAnswersCWizardSwitch is the same guard for the
+// MASTER sub-switch, driven through dispatchKey rather than through
+// wizardCommand directly so that the routing is covered too: these keys
+// must be answered because wizard mode is on, not because they leaked
+// into commandHandlers.
+func TestWizardDispatchAnswersCWizardSwitch(t *testing.T) {
+	t.Parallel()
+
+	for _, ch := range cWizardSwitchKeys() {
+		t.Run(unctrl(ch), func(t *testing.T) {
+			t.Parallel()
+
+			g := mkGameInput(t)
+			g.Wizard = true
+			// ctrl-a is "level--; new_level()", so start deep enough for
+			// it to have somewhere to go.
+			g.Depth = 5
+			// Escape backs out of the item and type prompts that ctrl-w,
+			// ctrl-~, 'C' and '*' put up. testTerm keeps answering after
+			// the script runs out, so nothing here can block.
+			setInput(t, g, Escape, Escape, Escape, Escape)
+
+			g.dispatchKey(ch)
+
+			assertNotIllegalCommand(t, g, ch)
+		})
+	}
+}