From 2e02e7d190b7c91bbf76edc40547e0ba5d9ddd80 Mon Sep 17 00:00:00 2001 From: sneak Date: Sun, 9 Aug 2026 14:17:21 +0000 Subject: [PATCH] test: pin command dispatch to C's command.c switch (closes #31) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Audits every case label in C's command.c against this port's dispatch and leaves the audit behind as a standing test, so the two lists cannot drift again. 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, fails to compile nowhere, and simply answers "illegal command" the first time a player presses it. That is how '+' (#11) survived until PR #30 found it by accident. Result: no further missing keys. '+' was the only one. Recording that as a negative result — the class of bug is real, it has now been searched for exhaustively rather than stumbled upon, and the search came back empty. Eighty labels: sixty-five in the main switch, fifteen in the wizard sub-switch. commandHandlers is pinned by set equality in both directions. A missing key is the '+' bug; an extra key is the same bug mirrored, since the likeliest way to acquire one is promoting a key out of the wizard sub-switch, which would expose a MASTER debug command in ordinary play. The main-switch keys that commandHandlers cannot hold - the goto-over re-dispatches, F-to-f, 'a' and 'm' - are covered separately through dispatchKey, because dropping one of those is just as silent as dropping a map entry. Two traps 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: '+' was a divergence in ordinary play, not just wizard mode, precisely because it is a main-switch key. Confirms this port targets the MASTER build: all four #ifdef MASTER sites in command.c are ported unconditionally, as is sticks.c 237. --- TODO.md | 21 ++++ game/dispatch_test.go | 248 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 269 insertions(+) create mode 100644 game/dispatch_test.go diff --git a/TODO.md b/TODO.md index 2c65bf9..e4dd3f7 100644 --- a/TODO.md +++ b/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 diff --git a/game/dispatch_test.go b/game/dispatch_test.go new file mode 100644 index 0000000..4283169 --- /dev/null +++ b/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) + }) + } +} -- 2.49.1