diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index a30a70d..2006ef8 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -1488,6 +1488,8 @@ type Terminal interface { ReadChar() (byte, bool) // event loop → C char codes (arrows→hjkl, ^C→quit); // ok is false when Interrupt woke the read Interrupt() // wake a blocked ReadChar (signal goroutine only) + Repaint() // forced full redraw for CTRL-R + // (curses clearok(curscr,TRUE)+wrefresh(curscr)) Fini() // restore the device (curses endwin) on exit } @@ -1507,13 +1509,22 @@ func (w *Window) Printwf(format string, a ...any); func (w *Window) Inch(y, x in func (w *Window) Clear(); func (w *Window) Clrtoeol(); func (w *Window) Standout(on bool) func (s *Screen) Refresh() // blit stdscr to the device func (s *Screen) RefreshWin(w *Window) // blit any window +func (s *Screen) Repaint() // force a full redraw of the device func (s *Screen) Fini() // tear the device down ``` Ported drawing code keeps its structure: `mvaddch(y, x, ch)` → `g.scr.Std.MvAddCh(y, x, ch)`. Curses `mvinch` reads come from the Window buffer, preserving the "screen is a data structure" idiom without touching the -real terminal. `md_readchar`'s escape decoding is deleted; tcell's `EventKey` +real terminal. `Repaint` is the one drawing call that is not a blit: it is the +`CTRL('R')` command's `clearok(curscr, TRUE)` plus `wrefresh(curscr)`, +implemented as tcell's `Screen.Sync`, and it exists because `Render` cannot do +its job. Both diff a new frame against the device's record of the old one and +send nothing where they agree, which is the wrong answer precisely when the +screen has been corrupted by something else's output — the only reason a player +types `CTRL('R')`. Like C's, it repaints what was last drawn rather than +re-blitting `stdscr`, so it takes no window. `md_readchar`'s escape decoding is +deleted; tcell's `EventKey` provides decoded keys and we translate to the byte codes `command()` already handles (KeyUp → 'k', etc.). Resize is handled by tcell, which registers only SIGWINCH — SIGTSTP is not among the signals it takes, and is dropped (§9). diff --git a/TODO.md b/TODO.md index 39efd7d..5ce90c8 100644 --- a/TODO.md +++ b/TODO.md @@ -34,6 +34,58 @@ wizard commands). # Completed Steps +- 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 + `otherwise` arm that closes `do_zap`'s switch, which `doZap` had turned into + doing nothing at all. Two things about it are easy to get wrong and are why + the fix is not one line. It is under `#ifdef MASTER`, **not** under a + `wizard` test, so in the MASTER build this port is it printed for every + player — gating it on `g.Wizard` would be issue #11's trap in reverse. And + `WS_NOP` is a case of that switch in its own right (`when WS_NOP: break;`), + so "no handler ran" cannot be the trigger: the wand of nothing does nothing + _quietly_, and only a kind C had no case for is bizarre. Since C's switch + covers all 14 `WS_` values, its `otherwise` is reachable only for an + `o_which` outside the table, which is exactly what `Object.hasValidWhich` + already screens for — so the split needed no new state, just a three-way + switch on handler / valid-Which / neither. All three arms fall through to + `obj.Charges--`, as C's do: even the bizarre schtick costs a charge. Replaces + the deferral comment PR #20 left there. (2) **`CTRL('R')` now actually + redraws.** C is `after = FALSE; clearok(curscr, TRUE); wrefresh(curscr);` + (`command.c` 288-291); the port called `g.refresh()`, the ordinary diffing + blit, **which cannot fix the only situation the command exists for** — a + screen corrupted by something else's output leaves the game's record of it + still correct, so the diff sends nothing and the corruption stays. New + `Terminal.Repaint` (tcell `Screen.Sync`, which discards tcell's record of the + terminal instead of diffing against it), `Screen.Repaint`, `g.repaint()`; + three implementations to update, the same shape as PR #26's `ReadChar` + change, so no split was needed. Named for the curses operation, not for + tcell: the interface is the game's abstraction. It repaints what was last + rendered — C repainted `curscr`, not `stdscr` — so it takes no window, and + the arm drops the `refresh()` C never had there (`command` refreshes before + the next key read anyway). (3) **The startup greeting**, `main.c` 107-113, + which existed nowhere in the tree. New `game.Greeting`, printed by + `cmd/rogue/main.go` before `term.New()` — the port's `initscr()`. Only the + wizard wording is `#ifdef MASTER`; the other is unconditional. The `%d` is + `dnum`, which `main.c` has just assigned to `seed`, so it is `Params.Seed`. + Two placement details the issue did not mention and the tests now pin: the + printf sits **after** `parse_opts`, so a ROGUEOPTS `name=` is what the player + is greeted by and the account name is only the fallback (`Greeting` re-runs + `ParseOpts`, which does nothing but assign into fields — no RNG, no screen); + and it sits after the `-s`/`-d` handling and after `restore()`, which never + returns, so a resumed game does not announce that a dungeon is being dug + (`digsNewDungeon`). No RNG call is added on any path and nothing under + `game/testdata/` moved; `TestSeedCompatItemTables` is green against the + untouched golden. Mutation-proved, each new behaviour deleted in turn and + only its own test failing: dropping the message arm fails + `TestZapUnhandledWandSaysBizarreSchtick`; extending it to `WandNothing` fails + `TestZapWandOfNothingIsSilent`; putting `g.refresh()` back fails + `TestRedrawCommandForcesFullRepaint`; swapping the two wordings, or the + ROGUEOPTS name for the account name, fails `TestGreeting`; greeting on the + restore path fails `TestDigsNewDungeon`. ARCHITECTURE.md §5.3 gains `Repaint` + and the paragraph on why a blit cannot substitute for it. `Next Step` + deliberately not rotated: out-of-band issue work. + - 2026-08-09 The `'+'` wizard-mode toggle (`fix/wizard-toggle-off`, closes #11): C's `command.c` 317-338 has a `when '+'` arm that leaves wizard mode — `wizard = FALSE`, `turn_see(TRUE)`, `msg("not wizard any more")` — and the diff --git a/cmd/rogue/main.go b/cmd/rogue/main.go index d4d4392..6be426f 100644 --- a/cmd/rogue/main.go +++ b/cmd/rogue/main.go @@ -40,6 +40,13 @@ func run() int { return 0 } + // C printed its greeting just before initscr(); here that means just + // before the tcell screen takes the terminal, and on stdout, exactly + // as C did (main.c main). + if digsNewDungeon(*deathDemo, flag.Args()) { + fmt.Print(game.Greeting(params)) + } + t, err := term.New() if err != nil { fmt.Fprintln(os.Stderr, err) @@ -87,6 +94,23 @@ func run() int { return 0 } +// digsNewDungeon reports whether this invocation is the one that digs a +// fresh dungeon, and so the only one that greets. +// +// C's printf is the last statement before initscr(), and everything that +// does something else has already left by then: -s scores and exits, -d +// runs the death demo and exits, and restore() — the argc == 2 case that +// is neither — never returns. So a saved game resumes without a greeting, +// which is right: nothing is being dug. +// +// The restore test is duplicated from run's own, deliberately. Keeping +// them as one predicate would mean deciding the startup path before the +// terminal exists and carrying it past the error returns, which is more +// rearrangement of run than a greeting is worth. +func digsNewDungeon(deathDemo bool, args []string) bool { + return !deathDemo && len(args) != 1 +} + // loadParams gathers the game parameters from the environment: home // directory, ROGUEOPTS, user name, wizard mode, and the dungeon seed // (main.c's startup). diff --git a/cmd/rogue/main_test.go b/cmd/rogue/main_test.go index 38c41e9..252616d 100644 --- a/cmd/rogue/main_test.go +++ b/cmd/rogue/main_test.go @@ -380,3 +380,40 @@ func (s *stuckSaver) AutoSaveOnSignal(time.Duration) bool { return true } + +// TestDigsNewDungeon pins which invocations reach C's greeting. In main.c +// the printf is the last statement before initscr(), so -s and -d, which +// exit earlier, never see it, and neither does a restored game, because +// restore() does not return. The saved-game case is the one worth having +// a test for: resuming a dungeon must not announce that one is being dug. +func TestDigsNewDungeon(t *testing.T) { + t.Parallel() + + cases := []struct { + name string + deathDemo bool + args []string + want bool + }{ + {name: "new game", args: nil, want: true}, + {name: "restore a save", args: []string{"rogue.save"}, want: false}, + {name: "death demo", deathDemo: true, want: false}, + { + name: "death demo wins over a save argument", + deathDemo: true, + args: []string{"rogue.save"}, + want: false, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + if got := digsNewDungeon(tc.deathDemo, tc.args); got != tc.want { + t.Errorf("digsNewDungeon(%v, %v) = %v, want %v", + tc.deathDemo, tc.args, got, tc.want) + } + }) + } +} diff --git a/game/autosave_test.go b/game/autosave_test.go index 5f94942..9641161 100644 --- a/game/autosave_test.go +++ b/game/autosave_test.go @@ -457,6 +457,10 @@ func newBlockingTerm() *blockingTerm { func (t *blockingTerm) Render(*Window) {} +// Repaint has nothing to redraw: this terminal exists for its input +// behaviour, and no autosave test types CTRL-R. +func (t *blockingTerm) Repaint() {} + func (t *blockingTerm) Fini() {} // Interrupt wakes a blocked ReadChar; called from the saving goroutine. diff --git a/game/command_test.go b/game/command_test.go new file mode 100644 index 0000000..415f36c --- /dev/null +++ b/game/command_test.go @@ -0,0 +1,37 @@ +//nolint:testpackage // white-box tests reach unexported state (approved 2026-07-07) +package game + +import "testing" + +// TestRedrawCommandForcesFullRepaint pins CTRL-R to a forced repaint +// rather than an ordinary refresh. C's arm is "after = FALSE; +// clearok(curscr, TRUE); wrefresh(curscr);" (command.c), and the +// clearok is the command: a diffing refresh compares the new frame +// against the device's record of the old one and sends nothing when they +// agree, which is exactly the situation after some other program has +// scribbled on the terminal. Only the terminal can tell the difference, +// so the test watches the terminal rather than the window contents. +func TestRedrawCommandForcesFullRepaint(t *testing.T) { + t.Parallel() + + g := mkGameInput(t) + + term, ok := g.scr.term.(*testTerm) + if !ok { + t.Fatal("game terminal is not a testTerm") + } + + before := term.repaints + g.After = true + + g.dispatch(CTRL('R')) + + if term.repaints != before+1 { + t.Errorf("terminal repainted %d times, want %d: CTRL-R did not force "+ + "a full redraw", term.repaints-before, 1) + } + + if g.After { + t.Error("CTRL-R consumed a turn; C sets after = FALSE") + } +} diff --git a/game/effects_test.go b/game/effects_test.go index 6c054b2..fdc043a 100644 --- a/game/effects_test.go +++ b/game/effects_test.go @@ -194,6 +194,74 @@ func TestZapSlowMonster(t *testing.T) { } } +// bizarreSchtick is C's message for a zap that matched no case at all +// (sticks.c do_zap, the "otherwise" arm). Shared by the pair of tests +// below so that the one asserting it appears and the one asserting it +// does not can never drift apart. +const bizarreSchtick = "what a bizarre schtick!" + +// TestZapUnhandledWandSaysBizarreSchtick pins the closing arm of C's zap +// switch. Every WS_ kind has a case, so the arm is reachable only for an +// o_which outside the table — here a wand one past the end, the state a +// corrupt save file can still describe. C's message is not gated on the +// wizard flag, only on the MASTER build this port is, so no test setup +// turns it on. +func TestZapUnhandledWandSaysBizarreSchtick(t *testing.T) { + t.Parallel() + + g := mkGameInput(t) + wand := malformed(KindWand) + wand.Charges = 3 + ch := give(g, wand) + + setInput(t, g, ch) + g.Msgs.Huh = "" + + g.doZap() + + if g.Msgs.Huh != bizarreSchtick { + t.Errorf("message = %q, want %q", g.Msgs.Huh, bizarreSchtick) + } + + // C falls out of the switch into o_charges-- from the otherwise arm + // as much as from any other. + if wand.Charges != 2 { + t.Errorf("charges = %d after zapping, want 2", wand.Charges) + } +} + +// TestZapWandOfNothingIsSilent is the other half, and the reason the +// message cannot simply be attached to "no handler ran". WS_NOP is a case +// of C's switch in its own right — "when WS_NOP: break;" — so the wand +// that does nothing does it quietly, and only a kind C had no case for +// is bizarre. +func TestZapWandOfNothingIsSilent(t *testing.T) { + t.Parallel() + + g := mkGameInput(t) + stick := newObject() + stick.Kind = KindWand + stick.Which = int(WandNothing) + g.fixStick(stick) + ch := give(g, stick) + + setInput(t, g, ch) + + charges := stick.Charges + g.Msgs.Huh = "" + + g.doZap() + + if g.Msgs.Huh == bizarreSchtick { + t.Errorf("the wand of nothing said %q; WS_NOP is a case of C's "+ + "switch, not an unhandled kind", bizarreSchtick) + } + + if stick.Charges != charges-1 { + t.Error("zap did not use a charge") + } +} + func TestParseOpts(t *testing.T) { t.Parallel() diff --git a/game/game.go b/game/game.go index 5ec9f62..7f95009 100644 --- a/game/game.go +++ b/game/game.go @@ -1,6 +1,8 @@ //nolint:mnd // C-faithful literals; names hurt C-greppability (approved 2026-07-07) package game +import "fmt" + // ItemLore is the per-game item identity state: the randomized appearance // names and the seven mutable ObjInfo tables (extern.c/init.c). type ItemLore struct { @@ -151,6 +153,43 @@ type RogueGame struct { data *gameData } +// Greeting is the line C printed on stdout while the player waited for +// the dungeon to be dug, immediately before initscr() (main.c main). The +// caller prints it before the terminal package takes the screen, which is +// where initscr() sat; there is no trailing newline in either wording, +// because C followed the printf with fflush and let curses have the +// display. +// +// Only the wizard wording is #ifdef MASTER in C, and it carries the +// dungeon number, which is the seed (main.c assigns seed = dnum right +// after choosing dnum). The other wording is unconditional. +// +// The name is C's whoami, resolved the way main.c resolves it: parse_opts +// runs before the printf, so a ROGUEOPTS "name=" setting is what the +// player is greeted by, and the account name is only the fallback. New +// does the same parse a moment later; doing it here too is safe because +// ParseOpts does nothing but assign into the fields it is handed — no +// RNG, no screen — so it cannot disturb the item tables the seed-compat +// golden pins. +func Greeting(params Params) string { + whoami := params.Name + + if params.RogueOpts != "" { + opts := &RogueGame{Whoami: params.Name} + opts.ParseOpts(params.RogueOpts) + + whoami = opts.Whoami + } + + if params.Wizard { + return fmt.Sprintf("Hello %s, welcome to dungeon #%d", + whoami, params.Seed) + } + + return fmt.Sprintf( + "Hello %s, just a moment while I dig the dungeon...", whoami) +} + // New builds a game from params, seeds the RNG, and randomizes the item // appearance tables (the front half of main.c main(); the player roll-up // and first level arrive with later porting phases). diff --git a/game/greeting_test.go b/game/greeting_test.go new file mode 100644 index 0000000..1421033 --- /dev/null +++ b/game/greeting_test.go @@ -0,0 +1,64 @@ +//nolint:testpackage // white-box tests reach unexported state (approved 2026-07-07) +package game + +import ( + "strings" + "testing" +) + +// TestGreeting pins both wordings of main.c's pre-initscr printf byte for +// byte. The wizard one carries dnum, which main.c has just assigned to +// seed, so it is the seed the player sees. Neither ends in a newline: C +// printed, flushed, and handed the display to curses. +func TestGreeting(t *testing.T) { + t.Parallel() + + cases := []struct { + name string + params Params + want string + }{ + { + name: "normal", + params: Params{Name: "conan", Seed: 4242}, + want: "Hello conan, just a moment while I dig the dungeon...", + }, + { + name: "wizard names the dungeon", + params: Params{Name: "conan", Seed: 4242, Wizard: true}, + want: "Hello conan, welcome to dungeon #4242", + }, + { + // parse_opts runs before the printf in main.c, and whoami + // falls back to the account name only when ROGUEOPTS left it + // empty, so the option is what the player is greeted by. + name: "ROGUEOPTS name wins over the account name", + params: Params{ + Name: "conan", Seed: 7, RogueOpts: "name=Rodney", + }, + want: "Hello Rodney, just a moment while I dig the dungeon...", + }, + { + name: "ROGUEOPTS without a name keeps the account name", + params: Params{ + Name: "conan", Seed: 7, RogueOpts: "terse,fruit=mango", + }, + want: "Hello conan, just a moment while I dig the dungeon...", + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + got := Greeting(tc.params) + if got != tc.want { + t.Errorf("Greeting() = %q, want %q", got, tc.want) + } + + if strings.HasSuffix(got, "\n") { + t.Error("greeting ends in a newline; C's printf did not") + } + }) + } +} diff --git a/game/screen.go b/game/screen.go index 9064988..2c02a54 100644 --- a/game/screen.go +++ b/game/screen.go @@ -23,6 +23,15 @@ type Terminal interface { // the one Terminal method called from another goroutine, so an // implementation must be safe to call concurrently with ReadChar. Interrupt() + // Repaint forces the device to redraw every cell it is showing, the + // redraw command's whole point (curses clearok(curscr, TRUE) followed + // by wrefresh(curscr)). Render cannot stand in for it: a device that + // diffs against its own idea of what is on screen will do nothing at + // all when the screen has been corrupted by something else's output, + // which is the case the player types CTRL-R for. It repaints what was + // last rendered — C repainted curscr, not stdscr — so it neither + // needs nor takes a window. + Repaint() // Fini restores the device to its pre-game state (curses endwin). The // game calls it on its way out, since one game run is one process. Fini() @@ -223,6 +232,14 @@ func (s *Screen) Refresh() { } } +// Repaint forces the device to redraw everything it is showing, if there +// is a device (curses clearok(curscr, TRUE) + wrefresh(curscr)). +func (s *Screen) Repaint() { + if s.term != nil { + s.term.Repaint() + } +} + // Fini restores the terminal device, if there is one (curses endwin). func (s *Screen) Fini() { if s.term != nil { @@ -263,3 +280,4 @@ func (g *RogueGame) standend() { g.scr.Std.Standout(false) } func (g *RogueGame) clear() { g.scr.Std.Clear() } func (g *RogueGame) clrtoeol() { g.scr.Std.Clrtoeol() } func (g *RogueGame) refresh() { g.scr.Refresh() } +func (g *RogueGame) repaint() { g.scr.Repaint() } diff --git a/game/sticks.go b/game/sticks.go index e93bc8c..67dabce 100644 --- a/game/sticks.go +++ b/game/sticks.go @@ -31,14 +31,30 @@ func (g *RogueGame) doZap() { return } - // A malformed Which yields no handler, which is exactly what C did in a - // non-MASTER build: its do_zap switch matched no case, fell out, and - // still ran o_charges--. (The MASTER-only "what a bizarre schtick!" - // message for that arm is issue #13, not this bounds fix.) - if h := g.data.zapHandler(obj); h != nil { + // C's switch has a case for every one of the 14 WS_ kinds, so its + // closing "otherwise: msg(...)" arm is reachable only for an o_which + // outside the table — the malformed objects hasValidWhich screens + // for, which is why no handler and a legal Which can only mean WS_NOP. + // + // The message is under #ifdef MASTER, not under a wizard test: C + // printed it for every player of a MASTER build, which is the build + // this port is (see the '+' command, issue #11). Do not gate it on + // g.Wizard. + // + // WS_NOP is a case of its own ("when WS_NOP: break;"): the wand that + // deliberately does nothing says nothing either. All three arms fall + // out of the switch into o_charges--, so even the bizarre schtick + // costs a charge. + h := g.data.zapHandler(obj) + + switch { + case h != nil: if !h(g, obj) { return // the zap aborted; no charge is used } + case obj.hasValidWhich(): // WS_NOP + default: + g.msg("what a bizarre schtick!") } obj.Charges-- diff --git a/game/tables.go b/game/tables.go index 31201a9..22d0a18 100644 --- a/game/tables.go +++ b/game/tables.go @@ -785,7 +785,10 @@ func newGameData() *gameData { }, CTRL('R'): func(g *RogueGame) { g.After = false - g.refresh() + // C forces a full repaint here (clearok + wrefresh on + // curscr), not the diffing refresh: the command exists + // for screens the game's own record no longer matches. + g.repaint() }, 'v': func(g *RogueGame) { g.After = false diff --git a/game/term_test.go b/game/term_test.go index 951931a..f25fa8b 100644 --- a/game/term_test.go +++ b/game/term_test.go @@ -8,10 +8,18 @@ type testTerm struct { input []byte pos int tick int + // repaints counts forced full redraws. Rendering is a no-op here, so + // counting is the only way a headless test can tell that CTRL-R asked + // for a repaint rather than an ordinary refresh — the two are + // indistinguishable in the window contents, which is the whole reason + // the bug this replaces went unnoticed. + repaints int } func (t *testTerm) Render(*Window) {} +func (t *testTerm) Repaint() { t.repaints++ } + func (t *testTerm) Fini() {} // Interrupt has nothing to wake: this terminal's ReadChar never blocks. diff --git a/game/wizard_test.go b/game/wizard_test.go index e65ef9d..55645f5 100644 --- a/game/wizard_test.go +++ b/game/wizard_test.go @@ -187,8 +187,9 @@ func malformed(kind ObjectKind) *Object { } // TestZapMalformedWandDoesNotPanic covers the sticks.go dispatch. C's -// non-MASTER do_zap matched no case and still ran o_charges--, so the -// charge must be spent even though nothing happens. +// do_zap matched no case and still ran o_charges--, so the charge must be +// spent even though the zap did nothing. What it says while doing nothing +// belongs to TestZapUnhandledWandSaysBizarreSchtick. func TestZapMalformedWandDoesNotPanic(t *testing.T) { t.Parallel() diff --git a/term/tcell.go b/term/tcell.go index 79d33dc..2e02310 100644 --- a/term/tcell.go +++ b/term/tcell.go @@ -78,6 +78,16 @@ func (t *Tcell) Render(w *game.Window) { t.screen.Show() } +// Repaint redraws the whole physical screen from tcell's content buffer +// — which holds what Render last blitted, so this is C's clearok(curscr, +// TRUE) + wrefresh(curscr) (command.c, the CTRL('R') arm) rather than a +// fresh draw of stdscr. Sync throws away tcell's record of what the +// terminal is showing, so unlike Show it repaints cells it believes are +// already correct, which is what makes it fix a corrupted screen. +func (t *Tcell) Repaint() { + t.screen.Sync() +} + // ReadChar blocks for the next key, translated to the byte codes the C // game reads: arrows become hjkl, control keys their C0 codes. ok is // false when Interrupt woke the read instead of a key arriving.