From 1142f43aed2bd14df683e50b929a3cbefb640375 Mon Sep 17 00:00:00 2001 From: sneak Date: Sun, 9 Aug 2026 09:55:38 +0000 Subject: [PATCH] Restore three lost C behaviors: schtick message, forced redraw, greeting (closes #13) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three behaviors from 5.4.4 that the port dropped silently. Each is a few lines; grouped because they are all "restore something C did". 1. sticks.c 237: the "otherwise" arm closing do_zap's switch printed "what a bizarre schtick!", and doZap had turned it into doing nothing. The arm is under #ifdef MASTER, not under a runtime wizard test, so in the MASTER build this port is it printed for every player and must not be gated on g.Wizard. 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. C's switch covers all 14 WS_ values, so its otherwise is reachable only for an o_which outside the table, which is what Object.hasValidWhich already screens for. All three arms fall through to obj.Charges--, as C's do. 2. command.c 288-291: CTRL('R') is "after = FALSE; clearok(curscr, TRUE); wrefresh(curscr);" — a forced full repaint. The port called g.refresh(), the ordinary diffing blit, which cannot fix the only situation the command exists for: a screen corrupted by another program's output leaves the game's record of it still correct, so the diff sends nothing. New Terminal.Repaint (tcell Screen.Sync, which discards tcell's record of the terminal rather than diffing against it), Screen.Repaint and g.repaint(), implemented in term.Tcell and in both headless test terminals. Named for the curses operation: the interface is the game's abstraction, not tcell's. It repaints what was last rendered — C repainted curscr, not stdscr — so it takes no window. 3. main.c 107-113: the startup greeting existed nowhere in the tree. New game.Greeting, printed on stdout 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. Neither wording ends in a newline. Two placement details the tests pin: the printf sits after parse_opts, so a ROGUEOPTS name= is what the player is greeted by; 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). Greeting parses ROGUEOPTS into a throwaway game built the way New builds the real one, tables and home directory included: ParseOpts handles every option, not just the one the greeting reads, and inven= is matched against inv_t_name[], which lives on the game. All three message strings verified byte-for-byte against origin/c-master sticks.c and main.c. No RNG call is added on any path and nothing under game/testdata/ changed; TestSeedCompatItemTables is green against the untouched golden. Mutation-proved, each behavior removed in turn with only its own test failing: dropping the message arm fails TestZapUnhandledWandSaysBizarreSchtick; extending the message to WS_NOP fails TestZapWandOfNothingIsSilent; putting g.refresh() back fails TestRedrawCommandForcesFullRepaint; swapping the two wordings, and ignoring the ROGUEOPTS name, both fail TestGreeting; greeting on the restore path fails TestDigsNewDungeon. ARCHITECTURE.md 5.3 gains Repaint and why a blit cannot substitute for it; nothing here is deliberately dropped, so section 9 is unchanged. TODO.md gets a Completed Steps entry; Next Step deliberately not rotated, this being out-of-band issue work. --- ARCHITECTURE.md | 19 +++++++--- TODO.md | 56 +++++++++++++++++++++++++++++ cmd/rogue/main.go | 26 ++++++++++++++ cmd/rogue/main_test.go | 37 +++++++++++++++++++ game/autosave_test.go | 4 +++ game/command_test.go | 37 +++++++++++++++++++ game/effects_test.go | 68 +++++++++++++++++++++++++++++++++++ game/game.go | 50 ++++++++++++++++++++++++++ game/greeting_test.go | 80 ++++++++++++++++++++++++++++++++++++++++++ game/screen.go | 18 ++++++++++ game/sticks.go | 26 +++++++++++--- game/tables.go | 5 ++- game/term_test.go | 8 +++++ game/wizard_test.go | 5 +-- term/tcell.go | 10 ++++++ 15 files changed, 437 insertions(+), 12 deletions(-) create mode 100644 game/command_test.go create mode 100644 game/greeting_test.go diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index a30a70d..f2c8049 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,16 +1509,25 @@ 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` -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). +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). Signals are handled in `cmd/rogue/main.go`: one `os/signal` channel read by one goroutine that reads exactly one signal, so a second signal can never call diff --git a/TODO.md b/TODO.md index 39efd7d..2c65bf9 100644 --- a/TODO.md +++ b/TODO.md @@ -34,6 +34,62 @@ 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`). The game `Greeting` parses + into is a throwaway but is built the way `New` builds the real one, tables and + home directory included, because `ParseOpts` handles every option and not just + the one the greeting reads: `inven=` is matched against `inv_t_name[]`, which + lives on the game, so a bare `&RogueGame{}` turned a legal `ROGUEOPTS` into a + nil dereference before the player saw a character. 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..e61e301 100644 --- a/cmd/rogue/main.go +++ b/cmd/rogue/main.go @@ -40,6 +40,15 @@ 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). C followed the printf with fflush because + // its stdout was buffered; os.Stdout is not, so the write is the + // flush. + if digsNewDungeon(*deathDemo, flag.Args()) { + _, _ = fmt.Fprint(os.Stdout, game.Greeting(params)) // CLI output + } + t, err := term.New() if err != nil { fmt.Fprintln(os.Stderr, err) @@ -87,6 +96,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..f00494e 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,54 @@ 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. +// +// The game it parses into is a throwaway, but it is built the way New +// builds the real one, because ParseOpts handles every option and not +// just the one this function reads: "inven=" is matched against the +// inv_t_name[] table and "file=~/..." against the home directory, both +// of which live on the game. A greeting that skimped on them faulted on +// a perfectly legal ROGUEOPTS before the player saw a single character. +func Greeting(params Params) string { + whoami := params.Name + + if params.RogueOpts != "" { + opts := &RogueGame{ + data: newGameData(), + Whoami: params.Name, + Home: params.Home, + } + 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..c4af3e7 --- /dev/null +++ b/game/greeting_test.go @@ -0,0 +1,80 @@ +//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() + + // The account name main.c copies into whoami when ROGUEOPTS does not + // name the player itself. + const account = "conan" + + cases := []struct { + name string + params Params + want string + }{ + { + name: "normal", + params: Params{Name: account, Seed: 4242}, + want: "Hello conan, just a moment while I dig the dungeon...", + }, + { + name: "wizard names the dungeon", + params: Params{Name: account, 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: account, 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: account, Seed: 7, RogueOpts: "terse,fruit=mango", + }, + want: "Hello conan, just a moment while I dig the dungeon...", + }, + { + // ParseOpts reaches every option, not just name=, and the + // inventory style is matched against a table (options.c + // parse_opts, inv_t_name[]) that lives in the game data. A + // greeting parsed on a game without those tables faulted on + // this ROGUEOPTS before it could print anything at all. + name: "ROGUEOPTS inventory style parses without a fault", + params: Params{ + Name: account, Seed: 7, RogueOpts: "inven=slow,name=Rodney", + }, + want: "Hello Rodney, 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. -- 2.49.1