Cover the wizard commands with C-verified unit tests (closes #7)
game/wizard.go had no tests of its own. It 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, so a defect in either leaks into a normal game. Adds coverage for createObj (pack filing and the gold arm), createWeaponArmor, createRing, showMap, whatis, whatisPick, setKnow, teleport and command.go's wizardKit. Package coverage 60.6% -> 62.4%. Expected values are transcribed from wizard.c, command.c, extern.c, weapons.c and rogue.h rather than read off the port; wizard mode is entered through Params.Wizard, the field main.go fills from ROGUE_WIZARD, so no test pokes the flag. Two notes from the C. A wizard-created "cursed" weapon is not cursed in either language: init_weapon assigns o_flags over the ISCURSED bit create_obj had just set, leaving only the o_hplus penalty, and the test pins the whole flag word to the init_dam[] row to say so. And show_map turns standout on for a square missing F_REAL but off only for a square whose whole flag word is zero. Exactly three sites clear F_REAL: putpass, which sets F_PASS first and so leaves 0x80; door's secret-door arm, on a room-wall exit still holding exactly F_REAL, leaving zero; and new_level's trap loop, whose rnd(NTRAPS) is 0..7, so the T_DOOR (00) case leaves zero as well. C's wstandend therefore does fire, 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. That display-only difference is reported on the issue and left alone here, and the test asserts standout only up to the first secret square. No game behavior is changed.
This commit is contained in:
67
TODO.md
67
TODO.md
@@ -29,12 +29,67 @@ Refactor ground rules:
|
||||
|
||||
# Next Step
|
||||
|
||||
Broaden unit test coverage where playtesting finds thin spots — wizard commands
|
||||
(#7). Rings and sticks, the first two thirds of this step, are done; see the top
|
||||
of Completed Steps.
|
||||
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
|
||||
@@ -638,13 +693,11 @@ of Completed Steps.
|
||||
|
||||
# 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):
|
||||
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.
|
||||
3. Note: this repo is exempt from the standard policy scaffold. A minimal dev
|
||||
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.
|
||||
|
||||
Reference in New Issue
Block a user