game/sticks.go was the largest under-tested file in the repo: 534 lines, 23 functions and a single test. It now has two test files, both written against the C reference (git show origin/c-master:sticks.c) rather than against the current Go code, so they can catch divergence instead of recording it. game/sticks_test.go covers every zap handler that had none — light in a room and in a corridor, drain-life's too-weak refusal (which returns before o_charges--), drain's hit-point split and its kill arm, drainReaches for all three of C's clauses, invisibility and the flytrap release, polymorph's detach/re-attach dance with the pack, under- character and delta-clobbering it does on the way, cancellation, both teleport wands, magic missile, haste/slow in both directions, fix_stick's damage and charge formulas, and charge_str. game/bolt_test.go covers fire_bolt: dirch for all eight directions, boltBounces including the door the hero stands on, an end-to-end flight asserting the path and resting square, bounces off both wall orientations, off a corner and diagonally off a wall (which pins C's rule that a bounce negates both components rather than reflecting), a bounced bolt striking the hero who fired it, the strike and miss arms, and the dragon that shrugs off a flame but not a lightning bolt. The tests read the flight path off the screen: fire_bolt paints its trail and then paints chat() back over every square it recorded, so on an otherwise blank screen the non-blank cells are exactly the squares the bolt occupied, and the walls it bounced off are absent because C undoes the record before the mvaddch. Determinism comes from a pinRng helper that searches for a seed whose next draw is the wanted value, and from a level the tests carve themselves with the generator's own drawRoom. The hero is fortified wherever a bolt can reach him, since death exits the process. No divergence from C was found. Two notes are recorded in the test comments: fire_bolt's "ch != 'M'" guard is a tautology, because winat is t_disguise whenever a monster stands there, and the door-under-hero exception can only be tested by the fact that the run terminates.
584 lines
40 KiB
Markdown
584 lines
40 KiB
Markdown
# Workflow
|
|
|
|
- branch (from `main`)
|
|
- do the work in Next Step
|
|
- move Next Step to the top of Completed Steps
|
|
- move the top item of Future Steps into Next Step
|
|
- commit (`TODO.md` changes in the same commit as the work)
|
|
- merge to `main` if the branch is not protected, otherwise open a PR
|
|
- push
|
|
|
|
# Status
|
|
|
|
pre-1.0
|
|
|
|
The port on main is complete and faithful (function-by-function from Rogue 5.4.4
|
|
C; reference sources on c-master/modern-rogue). Current phase: refactor from a
|
|
transliterated port into idiomatic Go — one feature branch per step below,
|
|
descriptive naming, real types, house style per
|
|
~/dev/prompts/prompts/CODE_STYLEGUIDE_GO.md.
|
|
|
|
Refactor ground rules:
|
|
|
|
- Behavior must not change unless a step says so. The full test suite (scripted
|
|
sessions, generation invariants, C-compatible RNG goldens) gates every step;
|
|
80x24 seed-compatible gameplay stays intact.
|
|
- Renames keep the C lineage greppable: doc comments retain their "(file.c
|
|
func_name)" breadcrumbs, and the docs refresh step adds a C-name → Go-name
|
|
table to ARCHITECTURE.md.
|
|
|
|
# Next Step
|
|
|
|
Broaden unit test coverage where playtesting finds thin spots (rings, sticks,
|
|
wizard commands).
|
|
|
|
# Completed Steps
|
|
|
|
- 2026-08-09 Wands and staffs under test (`test/sticks-coverage`, closes #6):
|
|
the second of the three thin spots the Next Step names. `game/sticks.go` was
|
|
the largest under-tested file in the repo — 534 lines, 23 functions, one test
|
|
— and now has `game/sticks_test.go` (the zap handlers, `drain`, `fix_stick`,
|
|
`charge_str`) and `game/bolt_test.go` (the `fire_bolt` geometry). Every
|
|
expectation was read out of `sticks.c` rather than off the Go code; **no
|
|
divergence from C was found**, and three things worth knowing came out of the
|
|
reading. (1) **The bolt trail is the test instrument.** `fire_bolt` paints
|
|
each square with `dirch` and then paints `chat()` back over every square it
|
|
recorded, so on a screen nothing else has drawn on, the non-blank cells
|
|
afterwards are exactly the squares the bolt occupied — and the walls it
|
|
bounced off are absent, because C undoes the record with `c1--` and `break`s
|
|
before the `mvaddch`. That gives an exact assertion of the path and the
|
|
resting place without touching game code, and it is why the tests fire from a
|
|
square that is not the hero's (which is what `chase.c` does for dragon
|
|
breath): with the hero off the ray the run produces one message and the screen
|
|
stays readable. (2) **A bounce reverses both components of the direction, not
|
|
one.** A bolt entering a wall at 45 degrees goes back the way it came instead
|
|
of reflecting off the surface, so the diagonal-into-a-vertical-wall case is
|
|
the one that separates C's rule from the plausible wrong one, and it is
|
|
tested. (3) **The `ch != 'M'` guard on the miss message is a tautology.** `ch`
|
|
comes from `winat`, and `winat` _is_ `t_disguise` when a monster stands there
|
|
(`rogue.h` 57), so `ch == 'M'` implies `t_disguise == 'M'` and the arm can
|
|
never go quiet; it is vestigial from when 'M' was the mimic, and the test pins
|
|
the port to speaking, so nobody "tidies" it into a real silence. The
|
|
door-under-hero exception has no assertion of its own because it cannot have
|
|
one: without it the bolt bounces on the hero's own square forever, recording
|
|
nothing, and `fire_bolt` never returns — the test for it hangs rather than
|
|
fails, which the comment on it says. Determinism comes from a `pinRng` helper
|
|
that searches for a seed whose next draw is the wanted value (running the real
|
|
`Rng`, never predicting it) and from a level the tests carve themselves
|
|
through `drawRoom`, since bounce geometry and `drain`'s room/passage/door
|
|
reach only mean something against known walls and a known passage number. All
|
|
21 mutations tried against the new tests were caught.
|
|
|
|
- 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
|
|
`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
|
|
port had no `'+'` anywhere, so the key fell through `dispatchKey`'s default to
|
|
`illcom` and answered "illegal command '+'". The password half of that arm was
|
|
dropped on purpose (wizard mode is `ROGUE_WIZARD` configuration) and is in
|
|
ARCHITECTURE.md §9; the leave half was lost silently and is not the same
|
|
decision — it does not touch the password machinery at all. **The substantive
|
|
part is `turn_see(TRUE)`**, not the flag: wizard sight draws every monster the
|
|
hero cannot see, so without the re-hide there is no way back to normal
|
|
visibility once wizard mode is on, and clearing the flag alone would have left
|
|
the screen lying. New `wizardToggleCommand` in `game/command.go`, registered
|
|
in `commandHandlers` between `'^'` and `Escape` — C's own switch order, and
|
|
note that C's arm sits in the **main** command switch under `#ifdef MASTER`,
|
|
not in the `if (wizard) switch (ch)` sub-switch that `wizardCommand` ports, so
|
|
it is reachable whether or not `wizard` is set. That makes the non-wizard case
|
|
a divergence too, and it resolves the way the dropped `passwd()` forces: a
|
|
password check that no longer exists can never succeed, so the else arm is
|
|
what C did on a wrong answer, the message "sorry" — no prompt, since nothing
|
|
typed into one could change the outcome, and none of the
|
|
`noscore`/`turn_see(FALSE)` bookkeeping of C's unreachable success branch. The
|
|
choice is stated in the function's doc comment and in §9, whose password row
|
|
now names the `'+'` enter arm and whose new paragraph records that the leave
|
|
arm is ported in full. Two tests in `game/wizard_test.go` drive `'+'` through
|
|
`g.dispatch`: the wizard one spawns a phantom (`ISINVIS` straight from the
|
|
monster table, so `seeMonst` is false and it is on screen only because wizard
|
|
sight put it there), asserts the precondition — monster glyph drawn in
|
|
standout at its cell, `SenseMonsters` set — and then asserts the flag cleared,
|
|
`SenseMonsters` cleared, the cell back to the map char under the monster with
|
|
standout off, the exact message, and `After` false; the non-wizard one pins
|
|
"sorry" and that `'+'` is no longer an illegal command. Mutation-proved:
|
|
deleting the `turnSee(true)` call fails the test on all three visibility
|
|
assertions, which is the half a flag-only test would have missed. No RNG call
|
|
is added — the `turn_off` arm of `turn_see` never reaches `rnd`, only the
|
|
turn-on arm does — and `TestSeedCompatItemTables` stays green against the
|
|
untouched golden. `Next Step` deliberately not rotated: out-of-band issue
|
|
work.
|
|
|
|
- 2026-08-09 Cleanups deferred from the PR #26 review (`cleanup/pr26-followups`,
|
|
closes #27): four items, no behaviour change. (1) The `sig-leave` entry below
|
|
still argued, in the present tense, that declining to save on SIGINT/SIGQUIT
|
|
was the safe choice because `AutoSave` encodes live state after removing the
|
|
file — both halves untrue since #24, and the entry read as a claim about how
|
|
the code works now rather than a record of what was weighed then. It is in the
|
|
past tense and marked superseded, pointing at the `fix/autosave-race` entry.
|
|
Nothing else in the file was touched — in particular the `err113` linter name
|
|
in the 2026-07-06 entry, which a `grep` for `113` still matches, and the "over
|
|
a hundred reports" wording the #26 rework had already corrected. Note for
|
|
anyone chasing this class of bug: the false claim was in the #12 entry, not
|
|
the #24 one, whose account of the old remove-then-write is correctly past
|
|
tense — find these by content, since `make fmt` reflows the file and cited
|
|
line numbers rot. (2) `encodeSnapshot` is `writeSnapshotFile`: it encodes,
|
|
fsyncs, chmods 0400 and closes, and the old name claimed only the first of
|
|
those. One call site (`saveFile`), and the doc comment now lists what it does
|
|
and why the fsync is there. (3) `TestAutoSaveOnSignalWhileInShellEscape` used
|
|
`t.Error` for its precondition, so a save that was never taken fell through
|
|
into `assertRestorable`, which can then only report a second, derived failure;
|
|
it is `t.Fatal`, matching the identical assertion in the blocked-on-input
|
|
test. (4) `serviceAutoSaveRequest`'s doc comment had a 24-column stub line
|
|
("The result is still a") left by an earlier edit — `gofmt` does not rewrap
|
|
comments, so `fmt-check` was legitimately green and nothing would ever have
|
|
caught it. Rewrapped to the block's width. `Next Step` deliberately not
|
|
rotated: out-of-band issue work.
|
|
|
|
- 2026-08-09 Signal-time autosave moved onto the game goroutine
|
|
(`fix/autosave-race`, closes #24): the SIGHUP/SIGTERM handler gob-encoded the
|
|
live game tree from the signal goroutine while the game goroutine was mid-turn
|
|
mutating it, and `AutoSave` **removed** the save file before encoding — so the
|
|
failure mode was not a stale save but a deleted one followed by a possibly
|
|
torn replacement, with a window in which the player had neither. `make test`
|
|
has run with `-race` since 2026-08-09 and was green, because no test had ever
|
|
driven the turn loop concurrently with a signal: evidence of untested, not of
|
|
safe. The handler now writes nothing itself. `AutoSaveOnSignal` posts a
|
|
request on a one-deep channel, wakes the input read, and waits up to
|
|
`signalSaveTimeout` (3s) for the game goroutine to take it; the encode happens
|
|
on the goroutine that owns the state. **The blocked-on-input case is the whole
|
|
point** — a dropped connection lands while the player is thinking, so a flag
|
|
checked only between turns would never be looked at — and it is handled by
|
|
making the read interruptible: `Terminal.ReadChar` returns `(byte, bool)` with
|
|
`ok == false` meaning "woken by `Interrupt`, no key", `term.Tcell.Interrupt`
|
|
posts a `tcell.EventInterrupt` onto tcell's own event queue to unpark
|
|
`PollEvent`, and `readchar` services the request and reads again, so no caller
|
|
sees the wake-up. The other unbounded park is the `!` shell escape, where a
|
|
hangup used to save and would otherwise have regressed to not saving: the
|
|
shell now runs on a helper goroutine and `runShellEscape` selects on {shell
|
|
finished, save request}, keeping the encode on the game goroutine while it
|
|
draws nothing. Between turns (`command`) covers a game that is busy rather
|
|
than parked. The wait is bounded so that a game goroutine wedged with no
|
|
service point can never stop a signal from getting the process out; giving up
|
|
costs nothing now that `saveFile` writes a temporary file in the save's own
|
|
directory, fsyncs it, and renames it over the target instead of truncating in
|
|
place — a failed or skipped save leaves the previous save whole. New
|
|
`game/autosave_test.go` drives the real turn loop while a second goroutine
|
|
asks for 25 saves (the interleaving that never existed before), plus the
|
|
parked-on-input case with a terminal fake that genuinely blocks, the shell
|
|
case, the deadline case (previous save byte-for-byte intact), the no-file-name
|
|
case, and the rename discipline — the last pinned by a handle opened before
|
|
the save, which still reads the old file whole after it. Each was
|
|
mutation-proved: reverting `AutoSaveOnSignal` to encode on the calling
|
|
goroutine (the pre-fix behavior) makes the turn-loop test fail under `-race`
|
|
with over a hundred reports, and removing each of the three service points
|
|
fails exactly the test for that park with its own message. `pendingSaver` now
|
|
reads the game out from under its mutex instead of delegating with it held,
|
|
because the delegated call blocks until the save is taken — the PR #23
|
|
review's N3 note, load-bearing rather than hypothetical, and pinned by a test.
|
|
The SIGINT/SIGQUIT no-save decision and the single-signal-read ordering
|
|
guarantee are untouched; `savesOnSignal`'s third ground ("safety") is
|
|
rewritten, since the corruption window it weighed no longer exists.
|
|
`MEMORY.md` stops listing signal-time autosave among the deliberate `_ =`
|
|
discards and states the new discipline; `ARCHITECTURE.md` §5.3, the `Terminal`
|
|
sketch, the C-to-Go mapping row and §9's SIGTSTP paragraph are corrected to
|
|
match. Two things review caught and this entry records so they are not undone:
|
|
moving the shell onto a helper goroutine also moved `term.Tcell.ShellEscape`'s
|
|
`panic` on a failed `Screen.Resume` there, and a panic at the top of any
|
|
goroutine kills the process without running the deferred calls of the others —
|
|
including `cmd/rogue/main.go`'s `defer t.Fini()`, so the tty would have been
|
|
left raw on exactly the path where the terminal is already broken (issue #12's
|
|
failure, reintroduced on a new path). `runShellEscape` recovers the helper's
|
|
panic and re-raises it on the game goroutine, pinned by
|
|
`TestShellEscapePanicUnwindsTheGameGoroutine`. And the doc comment took two
|
|
rounds to get right: the first version claimed in four places that nothing is
|
|
half-mutated at the `readchar` service point, and the revision that fixed that
|
|
claimed two of the three service points were between-commands. Both are false.
|
|
Only the check at the top of `command` is between commands — `readchar` is
|
|
reached from mid-command prompts, and `runShellEscape` is reached from
|
|
`shell`, an ordinary `'!'` command handler dispatched inside `command`, with
|
|
that turn's `DoDaemons(Before)`/`DoFuses(Before)` already fired and its AFTER
|
|
pass and ring effects not yet. What is actually guaranteed is that the encode
|
|
runs on the state-owning goroutine, so the snapshot is internally consistent
|
|
and restorable, though it may freeze a command half applied. `Next Step`
|
|
deliberately not rotated: out-of-band issue work.
|
|
|
|
- 2026-08-09 Signal-time terminal restore (`sig-leave`, closes #12): the port
|
|
handled only SIGHUP and SIGTERM, so SIGINT and SIGQUIT killed the process with
|
|
tcell still holding the tty, leaving the user at a shell with no echo. All
|
|
four signals now go to one `os/signal` channel read by one goroutine in
|
|
`cmd/rogue/main.go`, and every path calls `Terminal.Fini` before `os.Exit(0)`
|
|
— C's `leave()`, "leave quickly but curteously". **The decision** (written
|
|
into the `savesOnSignal` comment): SIGHUP/SIGTERM keep autosaving,
|
|
SIGINT/SIGQUIT restore and exit **without** saving. C never saves on INT or
|
|
QUIT anywhere — `leave()` is endwin-and-exit, `quit()` confirms/scores/exits,
|
|
`endit()` goes through `fatal()`, and `save.c auto_save` is reserved for
|
|
HUP/TERM — and the semantics agree: HUP/TERM are involuntary teardown worth
|
|
rescuing a game from, while INT/QUIT are a deliberate "stop now" that must not
|
|
become a one-keystroke checkpoint against a save discipline built to be
|
|
anti-save-scum. A third ground was weighed at the time and has since been
|
|
superseded: back then `AutoSave` gob-encoded live state that the main
|
|
goroutine was still mutating, after removing the old file, so declining to
|
|
save on the signals with nothing to rescue was also the option with no
|
|
corruption window. That window is gone as of the `fix/autosave-race` entry
|
|
above (#24) — the encode now runs on the game goroutine and `saveFile` renames
|
|
a temporary file into place — so nothing here should be read as a statement
|
|
about how saving works now; the split stands on C and on semantics alone, as
|
|
the current `savesOnSignal` comment says. The single-reader design closes the
|
|
window the issue warned about: a second signal arriving mid-save stays unread
|
|
in the buffer instead of exiting out from under the writer
|
|
(`TestLeaveOnSignalIgnoresLaterSignals` reproduces exactly that interleaving).
|
|
New `cmd/rogue/main_test.go` pins the membership of `handledSignals()` itself
|
|
(`TestHandledSignalsSet` — without it the rest of the file, which iterates
|
|
that set, would pass against a set that had silently lost SIGINT and SIGQUIT
|
|
again), and covers the ordering for each signal, the save/no-save split
|
|
against `savesOnSignal`, the mid-save-second-signal case, the pre-game
|
|
`pendingSaver` window, and real SIGINT/SIGQUIT/SIGHUP/SIGTERM delivered to the
|
|
test process through the same `notifySignals` wiring the game uses; the tty
|
|
leaving raw mode is the one step not checkable headlessly (it needs a
|
|
controlling terminal), and `term.Tcell.Fini` is a direct pass-through to
|
|
tcell's `Screen.Fini` that `myExit` already depends on. Two premises in the
|
|
issue turned out to be wrong and are recorded in ARCHITECTURE.md: `leave()` is
|
|
not installed on SIGINT/SIGQUIT during play (the wiring is in `mdport.c`, the
|
|
shipped build calls `md_onsignal_default()` and installs nothing, and
|
|
`leave()` appears only in the endgame paths of `rip.c`/`main.c`), and Ctrl-C
|
|
never generated SIGINT here anyway, since tcell's raw mode clears `ISIG` and
|
|
the key arrives as byte `0x03` — as it did in C, whose `setup()` calls curses
|
|
`raw()`. The real exposure is `kill -INT`/`kill -QUIT`, a SIGINT to the
|
|
process group while the `!` shell escape has the screen suspended, and the
|
|
window **after** `term.New()`: nothing is raw before it, and the handlers used
|
|
to be installed only once the game existed, leaving the restore path and
|
|
`-d`'s `DeathDemo()` — which never returns, blocking in `waitFor` inside
|
|
`death()` — running raw with no handler at all. The handlers are therefore
|
|
installed immediately after `term.New()`, with the game handed to them
|
|
afterwards via `pendingSaver`; a signal before the game exists restores the
|
|
terminal and exits with nothing to save, and the SIGHUP/SIGTERM autosave
|
|
behavior on the play path is unchanged. ARCHITECTURE.md §9 gained rows for
|
|
SIGTSTP/`tstp()` (dropped: raw mode means Ctrl-Z cannot reach us, a suspend
|
|
from the signal goroutine would race the drawing goroutine, and C armed `tstp`
|
|
only after a `restore()`; the `!` shell escape covers the need), for SIGINT
|
|
not routing to the interactive `quit()` prompt, and for `auto_save` on the
|
|
fault signals; §5.3's claim that tcell handles SIGTSTP was false — tcell
|
|
registers only SIGWINCH — and is corrected. `Next Step` deliberately not
|
|
rotated: out-of-band issue work.
|
|
|
|
- 2026-08-09 Wizard-create bounds fix (`fix/wizard-which-bounds`, closes #10):
|
|
`createObj` stored the raw `0-f` nibble as `Object.Which` with no bounds
|
|
check, so wizard mode -> `C` -> `/` -> `f` produced a wand numbered 15 against
|
|
a 14-entry table and panicked in `fixStick`. Input outside `0-f` overshoots
|
|
much further rather than going negative: `readchar` returns a `byte`, so the
|
|
`int(ch-'a') + 10` branch is byte arithmetic and wraps — `'A'` gives 234 and
|
|
`'!'` gives 202 — and panicked the same way. C's `create_obj()` was equally
|
|
unchecked, but every C consumer was either a `switch` (defined for any value)
|
|
or a static-array read past the end (undefined, and survivable in practice),
|
|
whereas since refactor step 8 one game is one process, so the Go panic kills
|
|
the game with the terminal still in raw mode. Fixed at the two boundaries a
|
|
bad `Which` can enter through: `createObj` now rejects an out-of-range choice
|
|
with a message built from C's own `type_name()` vocabulary and adds nothing to
|
|
the pack (a deliberate, commented divergence, since C had no defined behavior
|
|
here to be faithful to), and `Restore` refuses a snapshot describing such an
|
|
object (`ErrSaveCorrupt`) instead of loading a game that would explode later.
|
|
Behind those, `whichLimit`/`hasValidWhich` back defensive guards at every
|
|
dispatch named in the issue: the three effect tables (the new `quaffHandler`,
|
|
`readHandler`, and `zapHandler` accessors return no handler rather than
|
|
indexing — for wands that is exactly non-`MASTER` C, which matched no case and
|
|
still ran `o_charges--`), the `callIt` lore lookups, `identifyType` (whose
|
|
table is shorter than the scroll table keying it, though no scroll that can
|
|
reach `readIdentify` overshoots it, so that one is defensive rather than a
|
|
live bound), `armorClass` for the four `a_class[]` reads, `initWeapon` against
|
|
the missing `init_dam[]` row for `WeaponFlame`, `fixStick`'s `ws_type[]` read,
|
|
and `inventoryName`, hoisted so one check covers the scroll-title read the
|
|
issue listed plus its potion-color, ring-stone, wand-material, weapon and
|
|
armor siblings. `objectWorth` got the same hoisted guard, since the
|
|
death-screen appraisal reads the identical per-kind tables. No in-range input
|
|
changes behavior and no guard consumes a random number — the rejection
|
|
precedes every `rnd()` call, verified both by an explicit seed-unchanged test
|
|
and by `TestSeedCompatItemTables` staying green untouched. New
|
|
`game/wizard_test.go`: the exact reproducer, a rejection sweep over every
|
|
indexed kind including both wrapping-input forms, an acceptance sweep proving
|
|
valid choices still build the right item, one no-panic test per guarded family
|
|
(wand/potion/scroll/armor/weapon), the `fixStick` crash site, the corrupt-save
|
|
rejection over the wrapped values and a negative `Which` (a decoded snapshot
|
|
is the only source of one, so it is what exercises the `Which >= 0` arm of
|
|
`hasValidWhich`), and a check that `whichLimit` still agrees with the table
|
|
sizes. Each guard was confirmed load-bearing by reverting it and watching the
|
|
test panic. `Next Step` deliberately not rotated: this was out-of-band issue
|
|
work.
|
|
|
|
- 2026-08-09 Stale-docs correction (`docs-staleness`, closes #3): four claims in
|
|
`MEMORY.md`/`TODO.md`/`README.md` had gone false and were misdirecting agents
|
|
— the reviewer on PR #9 repeated one of them verbatim. Each was re-verified
|
|
against the tree before rewriting. (1) `MEMORY.md` described C's `exit()`
|
|
being unwound by a `gameEnd` panic recovered in `Run`; refactor step 8 deleted
|
|
that, `gameEnd` appears nowhere in the sources, and `myExit` (`game/rip.go`)
|
|
now calls `Terminal.Fini` then `os.Exit(0)` while `Run()` never returns — so
|
|
the section states the exit model and its testing consequence (a death exits
|
|
the test binary; hence `fortify()` in `game/run_test.go`). (2) `MEMORY.md`
|
|
said approved lint exceptions live in a "Repo-specific exceptions" block in
|
|
`.golangci.yml`; no such block exists and the config is byte-identical to
|
|
canonical (sha256 `021cc83f…46bcb`), the approvals having moved to in-code
|
|
`//nolint` directives carrying their dates — and `paralleltest` was listed as
|
|
an approved disable when it was in fact fixed (no `paralleltest` token in the
|
|
tree; 32 `t.Parallel()` calls against 32 tests). (3) `MEMORY.md` "Debugging"
|
|
and (4) `README.md` both told the reader to run `go test` directly, which
|
|
since PR #9 silently drops `-timeout 30s -race -cover`; both now point at
|
|
`make test`/`make check`. Also dropped the false "currently v2.12.2" host
|
|
linter claim from the 2026-08-07 entry (the host is v2.10.1 and nothing is
|
|
pinned; the pin question is tracked separately). Documentation only — no code,
|
|
`Makefile`, or config change; `Next Step` deliberately not rotated, since this
|
|
was out-of-band issue work.
|
|
|
|
- 2026-08-09 Policy-shaped `make test` (`make-test-policy-pattern`): the `test:`
|
|
target was a bare `go test $(GO_PKGS)` and now runs
|
|
`-timeout 30s -race -cover` with the mandated conditional verbose rerun (on
|
|
failure it reruns with `-v` and then `exit 1`, so a flaky pass on the second
|
|
attempt cannot rescue the build). `$(GO_PKGS)` is kept rather than hardcoding
|
|
`./...`. The substance was `-race`, not the Makefile edit: this is the first
|
|
time the suite has run under the race detector, and it is clean — no data
|
|
races across five consecutive uncached runs, including the tcell terminal
|
|
layer and the `os.Exit`-path playthrough tests. Wall clock 5.1s cold
|
|
(including the race build) and ~2.3s warm, against the 20s policy budget. The
|
|
failure path was exercised with a throwaway failing test to confirm the rerun
|
|
fires and `make` exits non-zero. Build tooling only; no game behavior change.
|
|
|
|
- 2026-08-07 Canonical linter config (`golangci-v2.12.2`): replaced
|
|
`.golangci.yml` with the shared canonical config (v2 schema; settings now live
|
|
under `linters.settings`, so the `lll`/`funlen`/`cyclop`/`dupl` thresholds
|
|
actually apply — the old top-level `linters-settings` block was silently
|
|
ignored). The four repo-specific disables (`mnd`, `exhaustive`,
|
|
`paralleltest`, `testpackage`) moved out of the config into targeted in-code
|
|
`//nolint` directives carrying the original approval dates, so the config
|
|
stays byte-identical to canonical. Real fixes: `t.Parallel()` in all 32 tests,
|
|
24 long lines wrapped or their comments tightened, control bytes in
|
|
`term/tcell.go` as character literals, and two `wsl_v5` defer cuddles. The
|
|
repo has no golangci-lint version pin to bump (no Dockerfile or CI;
|
|
`make lint` runs whatever `golangci-lint` is on the host).
|
|
|
|
- 2026-07-24 Seed compatibility — item tables (seed-compat): instrumented the C
|
|
reference on modern-rogue with a DUMP mode (testdata/c_seedcompat.patch) that
|
|
forces the RNG seed and prints the per-seed item appearance tables (potion
|
|
colors, scroll names, ring stones, wand/staff materials) before initscr, and
|
|
captured its output for four seeds as testdata/item_tables.golden.
|
|
TestSeedCompatItemTables regenerates the same tables from the Go port and they
|
|
match byte for byte — proving the LCG and its consumption order through the
|
|
whole init sequence agree with C. The remaining "same dungeon (map)" half
|
|
would need the harder headless-curses C dump (new_level draws to curses);
|
|
deferred — the item-table match already validates RNG-order faithfulness
|
|
through init, and the Go generation goldens guard determinism thereafter.
|
|
|
|
- 2026-07-23 Playtest hardening (playtest-hardening): added two death-safe
|
|
crash-sweep drives through the real turn loop, within the step-8 os.Exit
|
|
constraint (a fortify() helper pins HP/food/exp and clears the freeze/stuck
|
|
counters each turn so no death exits the test binary; fixed seeds keep them
|
|
deterministic). TestDeepPlaythrough uses quaff/read/zap through command
|
|
dispatch, then descends to depth 8 with a save/restore at depth 4;
|
|
TestTurnLoopCrashSweep mashes movement/search/rest for 200 turns on four
|
|
seeds. Neither surfaced a panic. The interactive "play several games at a real
|
|
tcell terminal" portion needs a human at an 80x24 terminal and is left to the
|
|
maintainer; the binary's non-interactive paths (`-s` scores) were
|
|
smoke-tested.
|
|
|
|
- 2026-07-23 Docs refresh (docs-refresh): rewrote ARCHITECTURE.md Part 2 (the
|
|
pre-implementation design sketch) to match the final code — current type/field
|
|
names (ObjectKind, DiceSpec, split o_arm, step-1 flag names, TrapCount, Level
|
|
list methods), the static tables now on the per-game gameData struct, the
|
|
daemon/effect handler tables, the MessageLine extraction, the Terminal
|
|
interface, the flat gob SaveState, and the New(Params) + os.Exit design. Added
|
|
§7.1, a C-name → Go-name rename table, and a README note on the make targets.
|
|
|
|
- 2026-07-23 Refactor step 8 (refactor/constructor-style): constructor and exit
|
|
pass. NewGame(Config) → New(Params) and Restore takes Params, so the package's
|
|
primary type gets the canonical New() constructor with a named-field Params
|
|
struct (styleguide 139/159). The gameEnd panic unwind is gone: one game run is
|
|
one process, so myExit restores the terminal (new Terminal.Fini) and calls
|
|
os.Exit(0), and Run() no longer returns; the four Run()-to-completion tests
|
|
were reworked/dropped since death (combat or starvation) now exits the process
|
|
(TestScoreRendersList and TestRunDownStairs preserve what is still drivable;
|
|
save/restore stays covered by TestSaveRestoreRoundTrip). The 77-column wrap
|
|
sweep was dropped per sneak (2026-07-23): line lengths left as-is (lll caps at
|
|
88 and passes).
|
|
|
|
- 2026-07-07 Refactor step 7 (refactor/effects-dispatch): effects dispatch
|
|
tables plus a full decomposition sweep — the quaff / readScroll / doZap
|
|
switches, the attack monster-power switch, the be_trapped switch, the daemon
|
|
d_func switch, and the command-key switch all became handler tables on
|
|
gameData (quaffHandlers, readHandlers, zapHandlers, hitHandlers, trapHandlers,
|
|
daemonHandlers, commandHandlers), one small named method per case. Every
|
|
remaining cyclop/gocognit/nestif hot spot was split into named helpers across
|
|
fight, misc (look), command, chase, move, passages, options, pack, things,
|
|
save, daemons, rooms, score, monsters, rings, rip, io, object, weapons,
|
|
wizard, and term/tcell, plus three test functions. Effect order and RNG call
|
|
sequence preserved throughout; the whole golangci-lint run is now 0 issues.
|
|
|
|
- 2026-07-07 Refactor step 6 (refactor/god-object-extraction): MessageLine (was
|
|
MsgLine) owns the msg/addmsg/endmsg machinery, wired to its screen/look/input
|
|
needs via attach(); RogueGame keeps one-line msg/addmsgf/endmsg shorthands so
|
|
call sites are unchanged. Player owns pack bookkeeping (nextPackChar,
|
|
removeFromPack — the state half of leave_pack; leavePack keeps only LastPick
|
|
tracking). Level owns object/monster list management and lookup (ObjectAt
|
|
replaces findObj; AddObject/RemoveObject/AddMonster/RemoveMonster replace
|
|
direct attachObj/detachObj/attachMon/detachMon on level lists).
|
|
Inventory/pickup UI flows stay on RogueGame deliberately: they are display and
|
|
turn orchestration, not state surgery.
|
|
|
|
- 2026-07-07 Refactor step 5 (refactor/item-combat-ui-renames, three commits,
|
|
one subsystem each): items — getItem→promptPackItem now returning (obj, ok),
|
|
invName→inventoryName, doPot→applyPotionFuse; combat — rollEm→rollAttacks,
|
|
attack/moveMonster/chaseStep return (removed bool) instead of C -1/0 int
|
|
codes; UI — getDir→promptDirection. C breadcrumbs kept; suite green.
|
|
|
|
- 2026-07-07 Refactor step 4 (refactor/movement-renames): movement/world renames
|
|
(doMove→moveHero, beTrapped→springTrap, rndmove→randomStep,
|
|
doRooms/doPassages/doMaze→digRooms/digPassages/digMaze, chgStr→changeStrength,
|
|
doRun→startRun, moveStuff→finishMove, turnref→turnRefresh,
|
|
moveMonst→moveMonster, doChase→chaseStep, setOldch→setOldChar, cansee→canSee,
|
|
roomin→roomIn, runto→runTo, conn→connectRooms, putpass→putPassage,
|
|
passnum→numberPassages, numpass→numberPassage, rndPos→randomPos,
|
|
rndRoom→randomRoom, treasRoom→treasureRoom, accntMaze→accountMaze); all
|
|
goto/label flows replaced with loops (moveHero retry loop + extracted
|
|
passageTurn, dispatch re-dispatch loop, chaseStep passage loop, saveGame
|
|
labeled prompt loop); C breadcrumbs kept in doc comments.
|
|
- 2026-07-07 Lint adoption finished (refactor/no-package-globals): all 37
|
|
package-level vars moved into `gameData` (built by `newGameData`, hung on
|
|
RogueGame as `g.data`, set in NewGame and Restore); ObjectKind
|
|
Glyph()/objectKindForGlyph became switches; the table-reading subtype
|
|
Stringers were removed; isMagic became a RogueGame method; goconst fixed with
|
|
named word constants (potionName, goldName, staffName, ripWall, ...);
|
|
testpackage and exhaustive disabled in .golangci.yml with sneak's approval
|
|
(2026-07-07); misspell's corruption of the "ther" scroll syllable reverted.
|
|
mnd disabled with sneak's approval (2026-07-07, follow-up commit). Remaining
|
|
red: cyclop (36), nestif (30), gocognit (23) stay until step 7 fixes them per
|
|
sneak's ruling.
|
|
- 2026-07-06 Lint adoption bulk (refactor/lint-adoption, 5ba9fe8): .golangci.yml
|
|
copied verbatim from the prompts repo (plus the sneak-approved paralleltest
|
|
exception, 2026-07-06); ~1,500 findings fixed (autofix formatting sweep,
|
|
errcheck/err113/noinlineerr error handling, forbidigo, funcorder, recvcheck
|
|
pointer receivers, goprintffuncname renames msg helpers to *f, revive doc
|
|
comments, gocritic switch rewrites, gosec real fixes plus justified nolints,
|
|
unparam signature tightening, C-faithful "missle" spellings restored after
|
|
misspell autofix changed game text).
|
|
- 2026-07-06 Module base path updated to git.eeqj.de/sneak/rgoue (go.mod, term/
|
|
and cmd/ imports, ARCHITECTURE.md, version string).
|
|
- 2026-07-06 Refactor step 3 (refactor/object-fields): Object.Arm split into
|
|
ArmorClass/Charges/GoldValue/Bonus (rings); Stats.Arm → ArmorClass; damage
|
|
strings parsed once into DiceSpec at table definition (ParseDice keeps C
|
|
roll_em parse semantics, incl. "%%%x0" and "000x0" edge cases,
|
|
regression-tested); save format 5.4.4-go3.
|
|
- 2026-07-06 Refactor step 2 (refactor/typed-kinds, b940cfc): ObjectKind
|
|
separates item category from map glyph (Object.Type byte → Kind ObjectKind
|
|
with Glyph()); PotionKind/ScrollKind/RingKind/
|
|
WandKind/WeaponKind/ArmorKind/TrapKind typed iota enums with Stringer; typed
|
|
accessors on Object; getItem/inventory/whatis filters take ObjectKind
|
|
(KindCallable/KindRingOrStick replace CALLABLE/R_OR_S); save format bumped to
|
|
5.4.4-go2. Suite green.
|
|
- 2026-07-06 Refactor step 1 (refactor/descriptive-constants): renamed all flag
|
|
bits, trap types, item subtype constants, and Max* counts to descriptive names
|
|
(IsHuh→Confused, SeeMonst→SenseMonsters, WsHasteM→WandHasteMonster,
|
|
MaxSticks→NumWandTypes, ...); Level.NTraps→TrapCount; C names kept as comment
|
|
breadcrumbs. Pure rename, suite green.
|
|
- 2026-07-06 Made the rgoue branch Go-only: removed C sources and the
|
|
autoconf/VS build system (they remain on master and modern-rogue), ported the
|
|
last wizard command (item-probability listing), rewrote README.md for the Go
|
|
port (c0b533e)
|
|
- 2026-07-06 Ported the command loop, save/restore, the tcell terminal layer,
|
|
and the playable binary at cmd/rogue (41fc104)
|
|
- 2026-07-06 Ported item effects: potions, scrolls, options, call_it (cdf9bf7)
|
|
- 2026-07-06 Ported combat, the chase driver, traps, zapping, death and scores
|
|
(3c5add8)
|
|
- 2026-07-06 Ported dungeon generation, base items, the pack, and monster
|
|
creation (a69ef7d)
|
|
- 2026-07-06 Ported the foundation: types, seed-compatible RNG, item tables,
|
|
daemon scheduler (7fa2048)
|
|
- 2026-07-06 Wrote ARCHITECTURE.md Parts 1 and 2: complete map of the C program
|
|
and the Go port design (91eeee0, 45dba95)
|
|
- Fork base: Davidslv/rogue C 5.4.4 with modernization fixes (C23 prototypes,
|
|
ncurses compat), preserved on master/modern-rogue
|
|
|
|
# Future Steps
|
|
|
|
1. Tag a release once a full game (Amulet retrieval and score entry) completes
|
|
without defects.
|
|
2. Full-terminal-size support (deferred by explicit decision 2026-07-06):
|
|
per-game dungeon dimensions instead of the 80x24 constants; open design
|
|
questions are resize policy, gameplay tuning at larger sizes, and a --classic
|
|
80x24 mode.
|
|
3. Note: this repo is exempt from the standard policy scaffold. A minimal dev
|
|
Makefile (fmt/fmt-check/lint/test/check targets) exists per sneak's
|
|
2026-07-07 request, but do not add a Dockerfile, CI config, or
|
|
REPO_POLICIES.md.
|