The port installed handlers for SIGHUP and SIGTERM only, so SIGINT and
SIGQUIT killed the process with tcell still holding the tty and dropped
the user into a shell with no echo and a scrambled screen. All four
signals now go to one os/signal channel read by one goroutine, and every
path calls Terminal.Fini before os.Exit(0) -- C's leave(), "leave
quickly but curteously" (main.c).
The handlers are installed immediately after term.New(), the call that
raises raw mode, rather than after the game exists. Everything between
those two points ran raw with no handler at all: the save-restore path,
and -d's DeathDemo(), which never returns -- death() blocks in
waitFor('\n') (game/rip.go) -- so a kill -INT during the death demo left
exactly the scrambled terminal this fixes. The game is handed to the
handler afterwards through pendingSaver, whose AutoSave is a no-op until
then: a signal before the game is built restores the terminal and exits
with nothing to save. SIGHUP/SIGTERM autosave on the play path is
unchanged.
The save decision, written into the savesOnSignal comment: SIGHUP and
SIGTERM keep autosaving; SIGINT and SIGQUIT restore and exit without
saving. No path in C saves on INT or QUIT (leave() is endwin-and-exit,
quit() confirms/scores/exits, endit() goes through fatal(), and
save.c auto_save is reserved for HUP/TERM), the semantics agree
(involuntary teardown is worth rescuing a game from; a deliberate "stop
now" must not become a one-keystroke checkpoint against an anti-save-scum
save discipline), and it is the safe choice, since AutoSave gob-encodes
live state the main goroutine is still mutating after removing the old
file.
One reader of one signal is also what closes the corruption window: a
second signal arriving while a SIGHUP's AutoSave is mid-write stays
unread in the buffer instead of exiting out from under the writer.
cmd/rogue/main_test.go pins the membership of handledSignals() itself --
the rest of the file iterates that set, so without that assertion the
suite would pass against a set that had lost SIGINT and SIGQUIT again,
which is the regression this issue exists to prevent -- and covers the
ordering per signal, the save/no-save split (driven from the expectation
table so every entry is read), the mid-save second-signal interleaving,
the pre-game pendingSaver window, and real SIGINT/SIGQUIT/SIGHUP/SIGTERM
delivered to the test process through the same notifySignals wiring the
game uses.
Two premises behind the report were wrong and are recorded rather than
silently fixed: 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 and 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()
described above. Nothing is raw before term.New(), so there was never
anything to cover there.
ARCHITECTURE.md section 9 gains rows for SIGTSTP/tstp() (deliberately
dropped: raw mode means Ctrl-Z cannot reach the process, suspending the
screen from the signal goroutine is a logical race against the drawing
goroutine -- not a data race, since tcell guards Suspend/Resume and Fini
alike -- and C armed tstp only after a successful restore(); the ! shell
escape covers the need), for SIGINT not routing to the interactive
quit() prompt, and for auto_save on the fault signals. Section 5.3's
claim that tcell handles SIGTSTP was false -- tcell registers only
SIGWINCH -- and is corrected, and its "every path restores the terminal"
claim now holds because of the install ordering above.
22 KiB
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.mdchanges in the same commit as the work) - merge to
mainif 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 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 oneos/signalchannel read by one goroutine incmd/rogue/main.go, and every path callsTerminal.Finibeforeos.Exit(0)— C'sleave(), "leave quickly but curteously". The decision (written into thesavesOnSignalcomment): 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 throughfatal(), andsave.c auto_saveis 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. It is also the safe choice:AutoSavegob-encodes live state that the main goroutine is still mutating, after removing the old file, so on the signals with nothing to rescue the port takes the option with no corruption window. 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 (TestLeaveOnSignalIgnoresLaterSignalsreproduces exactly that interleaving). Newcmd/rogue/main_test.gopins the membership ofhandledSignals()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 againstsavesOnSignal, the mid-save-second-signal case, the pre-gamependingSaverwindow, and real SIGINT/SIGQUIT/SIGHUP/SIGTERM delivered to the test process through the samenotifySignalswiring the game uses; the tty leaving raw mode is the one step not checkable headlessly (it needs a controlling terminal), andterm.Tcell.Finiis a direct pass-through to tcell'sScreen.FinithatmyExitalready 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 inmdport.c, the shipped build callsmd_onsignal_default()and installs nothing, andleave()appears only in the endgame paths ofrip.c/main.c), and Ctrl-C never generated SIGINT here anyway, since tcell's raw mode clearsISIGand the key arrives as byte0x03— as it did in C, whosesetup()calls cursesraw(). The real exposure iskill -INT/kill -QUIT, a SIGINT to the process group while the!shell escape has the screen suspended, and the window afterterm.New(): nothing is raw before it, and the handlers used to be installed only once the game existed, leaving the restore path and-d'sDeathDemo()— which never returns, blocking inwaitForinsidedeath()— running raw with no handler at all. The handlers are therefore installed immediately afterterm.New(), with the game handed to them afterwards viapendingSaver; 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 armedtstponly after arestore(); the!shell escape covers the need), for SIGINT not routing to the interactivequit()prompt, and forauto_saveon the fault signals; §5.3's claim that tcell handles SIGTSTP was false — tcell registers only SIGWINCH — and is corrected.Next Stepdeliberately not rotated: out-of-band issue work. -
2026-08-09 Wizard-create bounds fix (
fix/wizard-which-bounds, closes #10):createObjstored the raw0-fnibble asObject.Whichwith no bounds check, so wizard mode ->C->/->fproduced a wand numbered 15 against a 14-entry table and panicked infixStick. Input outside0-fovershoots much further rather than going negative:readcharreturns abyte, so theint(ch-'a') + 10branch is byte arithmetic and wraps —'A'gives 234 and'!'gives 202 — and panicked the same way. C'screate_obj()was equally unchecked, but every C consumer was either aswitch(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 badWhichcan enter through:createObjnow rejects an out-of-range choice with a message built from C's owntype_name()vocabulary and adds nothing to the pack (a deliberate, commented divergence, since C had no defined behavior here to be faithful to), andRestorerefuses a snapshot describing such an object (ErrSaveCorrupt) instead of loading a game that would explode later. Behind those,whichLimit/hasValidWhichback defensive guards at every dispatch named in the issue: the three effect tables (the newquaffHandler,readHandler, andzapHandleraccessors return no handler rather than indexing — for wands that is exactly non-MASTERC, which matched no case and still rano_charges--), thecallItlore lookups,identifyType(whose table is shorter than the scroll table keying it, though no scroll that can reachreadIdentifyovershoots it, so that one is defensive rather than a live bound),armorClassfor the foura_class[]reads,initWeaponagainst the missinginit_dam[]row forWeaponFlame,fixStick'sws_type[]read, andinventoryName, hoisted so one check covers the scroll-title read the issue listed plus its potion-color, ring-stone, wand-material, weapon and armor siblings.objectWorthgot 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 everyrnd()call, verified both by an explicit seed-unchanged test and byTestSeedCompatItemTablesstaying green untouched. Newgame/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), thefixStickcrash site, the corrupt-save rejection over the wrapped values and a negativeWhich(a decoded snapshot is the only source of one, so it is what exercises theWhich >= 0arm ofhasValidWhich), and a check thatwhichLimitstill agrees with the table sizes. Each guard was confirmed load-bearing by reverting it and watching the test panic.Next Stepdeliberately not rotated: this was out-of-band issue work. -
2026-08-09 Stale-docs correction (
docs-staleness, closes #3): four claims inMEMORY.md/TODO.md/README.mdhad 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.mddescribed C'sexit()being unwound by agameEndpanic recovered inRun; refactor step 8 deleted that,gameEndappears nowhere in the sources, andmyExit(game/rip.go) now callsTerminal.Finithenos.Exit(0)whileRun()never returns — so the section states the exit model and its testing consequence (a death exits the test binary; hencefortify()ingame/run_test.go). (2)MEMORY.mdsaid 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 (sha256021cc83f…46bcb), the approvals having moved to in-code//nolintdirectives carrying their dates — andparalleltestwas listed as an approved disable when it was in fact fixed (noparalleltesttoken in the tree; 32t.Parallel()calls against 32 tests). (3)MEMORY.md"Debugging" and (4)README.mdboth told the reader to rungo testdirectly, which since PR #9 silently drops-timeout 30s -race -cover; both now point atmake 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 Stepdeliberately not rotated, since this was out-of-band issue work. -
2026-08-09 Policy-shaped
make test(make-test-policy-pattern): thetest:target was a barego test $(GO_PKGS)and now runs-timeout 30s -race -coverwith the mandated conditional verbose rerun (on failure it reruns with-vand thenexit 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 theos.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 andmakeexits non-zero. Build tooling only; no game behavior change. -
2026-08-07 Canonical linter config (
golangci-v2.12.2): replaced.golangci.ymlwith the shared canonical config (v2 schema; settings now live underlinters.settings, so thelll/funlen/cyclop/duplthresholds actually apply — the old top-levellinters-settingsblock was silently ignored). The four repo-specific disables (mnd,exhaustive,paralleltest,testpackage) moved out of the config into targeted in-code//nolintdirectives 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 interm/tcell.goas character literals, and twowsl_v5defer cuddles. The repo has no golangci-lint version pin to bump (no Dockerfile or CI;make lintruns whatevergolangci-lintis 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 (
-sscores) 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 bynewGameData, hung on RogueGame asg.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
- Tag a release once a full game (Amulet retrieval and score entry) completes without defects.
- 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.
- 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.