a653cc76f223dec6f7e95b93726a4ddfc7416731
3 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
| 1142f43aed |
Restore three lost C behaviors: schtick message, forced redraw, greeting (closes #13)
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.
|
|||
clawbot
|
3a01283358 |
fix: take the signal-time autosave on the game goroutine (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. The suite has run under -race since 2026-08-09 and was green because nothing had ever driven the turn loop concurrently with a signal: evidence of untested, not of safe. The handler no longer writes anything. AutoSaveOnSignal posts a request, wakes the input read, and waits up to signalSaveTimeout for the game goroutine to take it; the encode runs on the goroutine that owns the state, at the three points where that goroutine can sit: between turns (command), on waking from a blocked readchar, and while parked in the `!` shell escape (runShellEscape, which now runs the shell on a helper goroutine so a hangup during it still rescues the game). Blocked on input is the case that matters — a dropped connection lands while the player is thinking, so a flag checked only between turns would never be looked at. Terminal.ReadChar therefore returns (byte, bool), with ok false meaning "woken by Interrupt, no key", and term.Tcell posts a tcell.EventInterrupt onto tcell's own event queue to unpark PollEvent. readchar services the request and reads again, so no caller sees it. Running the shell on a helper goroutine would also have moved term.Tcell.ShellEscape's panic on a failed Screen.Resume onto it, and a panic at the top of any goroutine terminates the process without running the deferred calls of the others — including cmd/rogue/main.go's `defer t.Fini()`. The tty would have been left raw on precisely the path where the terminal is already broken, which is issue #12's failure on a path this change created. runShellEscape therefore recovers the helper's panic and re-raises it on the game goroutine, whose stack has the restore in it, so "every path restores the terminal via Terminal.Fini before exiting" stays true. saveFile writes a temporary file in the save's own directory, fsyncs it and renames it over the target instead of truncating in place, so a save that fails — or never happens because the deadline ran out — leaves the player's previous save whole. What the handoff guarantees is stated exactly rather than flatteringly: the encode runs on the state-owning goroutine, so the snapshot is internally consistent and restorable, but it is not necessarily taken between commands. Only the check at the top of command is; the other two service points both sit inside a command call already under way. readchar is reached from mid-command prompts (--More--, askOverwrite, getStr, the direction and pack prompts) with the command's mutations already applied, and runShellEscape is reached from shell, an ordinary '!' command handler dispatched inside command, with that turn's DoDaemons(Before) and DoFuses(Before) already fired and its AFTER pass not yet. Restoring re-enters playit at the top of command, so either way the rest of that command is lost and a fresh BEFORE pass runs on top of the one in the snapshot. The SIGINT/SIGQUIT no-save decision and the single-signal-read ordering guarantee are untouched. pendingSaver reads the game out from under its mutex rather than delegating with it held, because the delegated call now blocks until the save is taken. |
||
| dfb34be1c4 |
fix: restore the terminal on SIGINT/SIGQUIT (closes #12)
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.
|
