790 lines
54 KiB
Markdown
790 lines
54 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
|
|
|
|
Tag a release once a full game (Amulet retrieval and score entry) completes
|
|
without defects. Promoted from Future Steps now that the coverage step above it
|
|
is finished.
|
|
|
|
# Completed Steps
|
|
|
|
- 2026-08-09 Wizard commands under test (`test/wizard-coverage`, closes #7): the
|
|
last of the three thin spots, so the coverage step is now closed rather than
|
|
narrowed. `game/wizard.go`'s eight functions had no tests of their own, and
|
|
the file is not purely a debug surface — `set_know` writes the per-game
|
|
discovered tables that name items in ordinary play, and `teleport` is what the
|
|
teleport ring calls every fiftieth turn. Package coverage 60.6% -> 62.4%.
|
|
Everything expected was transcribed from `wizard.c`, `command.c` (the
|
|
`CTRL('I')` kit), `extern.c` (`a_class[]`), `weapons.c` (`init_dam[]`) and
|
|
`rogue.h`; 30 mutations were tried and all 30 were caught.
|
|
|
|
Two findings came out of the reading. (1) **A wizard-created cursed weapon
|
|
is not cursed, in C or here.** `create_obj` sets `ISCURSED` and then calls
|
|
`init_weapon`, which _assigns_ `weap->o_flags = iwp->iw_flags` and so
|
|
overwrites the bit it just set; only the `o_hplus` penalty survives, and the
|
|
"cursed" weapon can still be dropped and unwielded. The port reproduces this
|
|
exactly. The test asserts the whole flag word comes back as the `init_dam[]`
|
|
row's value whatever blessing was answered, and deleting the `ISCURSED` line
|
|
from the port leaves every weapon test green — which is the evidence that
|
|
the line is dead for weapons. The armor arm has no such clobber and does
|
|
keep the curse. (2) **`show_map`'s standout is asymmetric in C and symmetric
|
|
here.** C tests `!(real & F_REAL)` before drawing and `!real` — the whole
|
|
flag word — after. `new_level` seeds every square with `p_flags = F_REAL`,
|
|
and exactly three sites clear that bit. `passages.c putpass` sets `F_PASS`
|
|
first, so its secret passage is left at `0x80`. `passages.c door`'s
|
|
secret-door arm clears it on a room-wall exit whose flags are still exactly
|
|
`F_REAL` (`rooms.c` writes no `p_flags` at all), leaving `p_flags == 0`; its
|
|
per-square gate is `rnd(5) == 0` against `putpass`'s `rnd(40) == 0`, and
|
|
`game/passages.go`'s `door` reproduces it. `new_level`'s trap loop then ORs
|
|
in `rnd(NTRAPS)`, which is `abs((int) RN) % 8` and so yields `0..7`, and
|
|
`T_DOOR` is `00` — an unsprung trapdoor square is also exactly zero
|
|
(`be_trapped` is what later ORs `F_SEEN` into it). So C _does_ turn standout
|
|
off again, at secret doors and unsprung trapdoors; what it gets wrong is
|
|
leaking the attribute forward from a secret passage or a non-trapdoor trap
|
|
until it reaches one of those. Intermittent bands of reverse video, not a
|
|
permanently reversed map. `game/wizard.go` tests `isReal` both times and
|
|
highlights the one square. That is a display-only difference in a
|
|
wizard-only command and was reported on the issue rather than changed here;
|
|
the test asserts the map characters unconditionally but the standout
|
|
attribute only up to the first secret square, so it pins nothing that C
|
|
contradicts.
|
|
|
|
Two things the tests had to be built around. The `insist` arm of `whatis` is
|
|
a loop whose only exits are picking a matching item and `n_objs == 0`, so a
|
|
script that runs dry hangs instead of failing — every sequence that can
|
|
re-prompt ends in an abort tail, the `n_objs == 0` exit is reached the way a
|
|
player reaches it (`*` for a list with nothing appropriate in the pack)
|
|
rather than by poking the counter, and the one mutation that deletes that
|
|
exit is the only one of the 30 that fails by timeout instead of fast,
|
|
necessarily so. And `show_map` does **not** mark squares seen — it writes
|
|
into `hw` and touches no `PLACE` at all — so the issue's wording for it
|
|
could not be tested as written; the loop bounds are asserted instead by
|
|
planting a marker in the rows C's loop excludes, since those rows are blank
|
|
on a real level and copying blanks over blanks would have made the bound
|
|
unfalsifiable.
|
|
|
|
- 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
|
|
27 mutations tried against the new tests were caught.
|
|
|
|
- 2026-08-09 Trap unit-test coverage (`test/traps-coverage`, closes #14):
|
|
`trapHandlers` had eight entries and **zero** direct tests, on the one
|
|
subsystem besides combat that can kill the hero outright. New
|
|
`game/traps_test.go` (19 tests, 15 subtests) covers all eight arms of
|
|
`move.c be_trapped`, the prologue every trap runs through, and the
|
|
`rust_armor` tail `T_RUST` calls. Package coverage 56.2% -> 57.9% measured on
|
|
`main` at `bf820e3`, the branch point, before the sticks tests landed. Every
|
|
expected value is transcribed from `origin/c-master` and quoted in the file.
|
|
**No divergence from C was found.**
|
|
|
|
The issue body's trap list was wrong and the correction is the first thing
|
|
worth recording: there is no separate "poison dart" trap — `T_DART` **is**
|
|
the poisoned dart, its death message being "a poisoned dart killed you" —
|
|
and the list omitted `T_MYST`, the mystery trap, whose arm is an eleven-way
|
|
`rnd(11)` message switch. `rogue.h` 192-200 is the authority
|
|
(`T_DOOR`/`T_ARROW`/`T_SLEEP`/`T_BEAR`/`T_TELEP`/`T_DART`/`T_RUST`/`T_MYST`,
|
|
`NTRAPS` 8) and the Go `TrapKind` iota matches it index-for-index.
|
|
|
|
Three C details the tests are built around. (1) `BEARTIME` and `SLEEPTIME`
|
|
are `spread(3)` and `spread(5)` (`rogue.h` 108-109), and `spread` is
|
|
`nm - nm/20 + rnd(nm/10)`; for both, `nm/10` is 0 and C's `rnd` short
|
|
circuits a zero range without touching the generator, so each is an exact
|
|
constant that costs **no** random number — and the tests assert the no-draw
|
|
half as well as the value, because a stray draw desynchronises the
|
|
seed-compatible stream. (2) `T_ARROW` swings at `s_lvl - 1` and `T_DART` at
|
|
`s_lvl + 1`: opposite signs, which is exactly the kind of detail a
|
|
transliterating port drops. (3) The strength loss is gated on
|
|
`!ISWEARING(R_SUSTSTR) && !save(VS_POISON)`, and the `&&` is load-bearing —
|
|
with the ring on, C never rolls the save, so the arm must spend two random
|
|
numbers and not three.
|
|
|
|
Two shapes worth keeping, both forced by mutation results rather than
|
|
foresight. Damage dice are checked by a **sweep**, not one shot: `rnd(n)` is
|
|
"raw value % n", so a single draw agrees between a d6 and a d5 five times in
|
|
six and leaves the generator identical either way — the first draft's
|
|
single-trial arrow test passed with `roll(1,6)` mutated to `roll(1,5)`.
|
|
Likewise the swing arguments are pinned by a 200-trial boundary sweep at a
|
|
mid-range to-hit target: a forced hit and a forced miss cannot see a wrong
|
|
`at_lvl` or a dropped `op_arm`, because both arms are reachable at any level
|
|
and swing spends one `rnd(20)` regardless.
|
|
|
|
Mutation-proved, 33 mutations, each reverted, and every one of them is now
|
|
caught. Three were **not** caught on the first pass and the tests were
|
|
strengthened until they were, which is the useful part of the record. (a)
|
|
Deleting `new_level()` from `T_DOOR` left the suite green: `be_trapped`'s
|
|
own prologue stamps the trap glyph into the cell the hero fell through, so
|
|
"the map changed" is true even with no new level dug. The test now counts
|
|
differing cells — exactly one can change that way — and also requires the
|
|
staircase to move and the hero to be re-placed. (b) The `roll(1,6)` case
|
|
above.
|
|
|
|
(c) **`be_trapped` takes a coordinate, and which coordinate decides whether
|
|
`T_TELEP`'s `mvaddch(tc, TRAP)` does anything.** Deleting that line first
|
|
left the suite green, and the first draft wrote that off as an unavoidable
|
|
redundancy — wrongly, because the test only exercised one of the two call
|
|
sites. `move.go` 105-108 (`case Floor`) springs a trap under the hero and
|
|
passes `p.Pos`; there `tc` **is** the hero's square, the prologue has
|
|
already set its `p_ch` to `TRAP`, and `teleport()` opens by drawing
|
|
`floor_at()` — which returns `chat(hero)` — over it, so the glyph is on
|
|
screen before the line runs. But `move.go` 94-98 (`case Trap`), the ordinary
|
|
walk onto a hidden trap, passes `nh`, the square being stepped **onto**,
|
|
with the hero still on the previous square: `teleport()`'s opening `mvaddch`
|
|
paints the old square, `leave_room` writes blanks and never `TRAP`, and
|
|
nothing calls `look()` afterwards because the `case Trap` arm returns before
|
|
`finishMove` for a teleporter. There `mvaddch(tc, TRAP)` is the only writer,
|
|
exactly as C's comment says.
|
|
`TestTrapTeleportDrawsTheTrapOnTheSquareSteppedOnto` springs the trap at a
|
|
floor square next to the hero and pins it: unmutated the screen at `tc`
|
|
reads `^`, with the line deleted it reads `.`.
|
|
|
|
The other 30 each failed their own test and only their own; two also moved
|
|
`TestAutoSaveOnSignalRacesTurnLoop`, which drives real turns and is
|
|
legitimately sensitive to `BEARTIME` and to armor rusting.
|
|
|
|
Deliberately uncovered: the two death messages, "an arrow killed you" and "a
|
|
poisoned dart killed you". Each is printed immediately before `death()`,
|
|
which reaches `myExit` and `os.Exit`, so provoking either would take the
|
|
test binary with it; the hero is pinned with `fortify()` and the damage
|
|
rolls are checked by replaying C's arithmetic instead of by letting HP reach
|
|
zero. They are the only two: `rust_armor`'s `|| ISWEARING(R_SUSTARM)`
|
|
operand and its `if (!to_death)` suppression of the rust-vanishes message,
|
|
the last predicates that had no assertion, are pinned by
|
|
`TestTrapRustHonoursTheRingAndTheToDeathFlag`. This entry does **not**
|
|
rotate `Next Step`: #14 was an out-of-band gap found while surveying, not
|
|
part of the rings/sticks/wizard step.
|
|
|
|
- 2026-08-09 Ring unit-test coverage (`test/rings-coverage`, closes #5): the
|
|
first third of the standing coverage step. `game/rings.go` had **zero** tests
|
|
— not one of the 32 in the suite touched wear, removal, hand choice, or the
|
|
ring contribution to the hunger clock. New `game/rings_test.go` (17 tests, 44
|
|
subtests) covers `ringOn`, `pickRingHand`, `ringOff`, `gethand`, `ringEat` and
|
|
`ringNum`, plus the ring arm of `things.c dropcheck` (`dropRing`), which is
|
|
what actually takes a ring off. Package coverage 53.7% -> 56.2%. `Next Step`
|
|
narrowed rather than rotated: #6 and #7 are the other two thirds.
|
|
|
|
Every expected value is transcribed from `origin/c-master` (`rings.c`,
|
|
`rogue.h`, `things.c`), never from what the port returns, and the C is
|
|
quoted in the file. **No divergence from C was found**, which is the result
|
|
and is worth recording as a negative: `ringEat` is the one function here
|
|
whose being wrong would be invisible — it feeds `daemons.c`'s hunger clock,
|
|
so a bad entry is a slow drift in when the hero starves rather than anything
|
|
a playtest would notice — and it now has all fourteen ring kinds pinned to
|
|
C's table.
|
|
|
|
Three C details the tests were written around. (1) `ring_eat`'s `uses[]`
|
|
holds negatives, and a negative is **not** a cost: C computes
|
|
`eat = (rnd(-eat) == 0)`, a one-in-n chance of a single unit. (2) `R_DIGEST`
|
|
then flips the sign, so slow digestion returns 0 or **-1** and is the only
|
|
ring that gives food back. (3) `ring_num`'s switch closes with the
|
|
`otherwise` macro, which `rogue.h` 53 defines as `break;default` — so its
|
|
four labels fall through to one `sprintf` and every other kind returns `""`
|
|
from a default arm, not by falling off the end. The `RingKind` iota matches
|
|
C's `R_` numbering index-for-index, so a `uses[]` index and a `RingKind` are
|
|
the same number; `R_ADDHIT` is `RingDexterity` and `R_ADDDAM` is
|
|
`RingIncreaseDamage`.
|
|
|
|
The chance rings are checked two ways at once. Each call snapshots the
|
|
generator, runs `ringEat`, and replays C's own expression from the identical
|
|
state — which pins the one-in-n denominator, the sign flip, and the fact
|
|
that exactly one `rnd` call is spent — and a frequency check over 4000
|
|
trials backs it with a number a human can read. The non-negative entries
|
|
assert the reverse: the generator must be **untouched**, because C never
|
|
reaches `rnd` on that path and a stray call there would desynchronise the
|
|
whole game's RNG stream from C's and cost seed compatibility. That assertion
|
|
is what caught the one real bug in this work, which was in the test and not
|
|
the game: `g.Rng` is a pointer, so the first draft's snapshots aliased
|
|
instead of copying.
|
|
|
|
Two shapes worth keeping. Scripted hand answers carry an abort tail (a space
|
|
for the reprompt's `--More--`, then ESCAPE): without it a port that stopped
|
|
accepting a key would loop forever on the headless terminal's filler input
|
|
and the test would die of the 30s timeout instead of failing on its
|
|
assertion — which is exactly what the first draft did, and it was only
|
|
visible because the mutation run was inspected rather than trusted. And the
|
|
"only one hand free" case scripts the _wrong_ hand key deliberately: a port
|
|
that asked anyway consumes it and lands the ring on the wrong side, so the
|
|
test fails on a hand rather than on a hang.
|
|
|
|
Mutation-proved, 23 mutations, each reverted: breaking `pickRingHand`'s
|
|
ask/auto/reject arms, `ring_on`'s type guard, `is_current` guard and all
|
|
three effect arms, `ring_off`'s no-rings message, hand selection and ESCAPE
|
|
abort, `gethand`'s uppercase keys, ESCAPE and reprompt, `dropRing`'s hand
|
|
clearing and both effect arms, `dropcheck`'s cursed gate, three `ringUses`
|
|
entries, the `R_DIGEST` sign flip, the one-in-n roll, the empty-hand zero,
|
|
and `ring_num`'s `ISKNOW` guard, label set and `RING`-vs-`WEAPON`
|
|
formatting. Each failed its own test and only its own; stripping all three
|
|
`ring_on` effect arms failed 3 of 3. All fourteen ring kinds are exercised;
|
|
the eleven with no wear-time effect in C are documented at the foot of the
|
|
file as deliberately not given a wear/remove test, with the files their
|
|
powers actually live in, and `ring_off`'s unreachable "not wearing such a
|
|
ring" arm is documented as unreachable rather than left looking untested.
|
|
|
|
- 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. 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.
|
|
2. 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.
|