e3ab4aba8bebca0503315ccbce4f69a7e2dae08b
4 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
| 6f409bda9e |
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. |
|||
| 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.
|
|||
| c95f98ffe5 |
Port the '+' wizard-mode toggle-off (closes #11)
C's command.c 317-338 has a `when '+'` arm in the main command switch, under #ifdef MASTER, that toggles wizard mode. The port had no '+' at all, so the key fell through dispatchKey's default to illcom and answered "illegal command '+'". The password half of that arm was dropped deliberately (wizard mode is ROGUE_WIZARD configuration) and is recorded in ARCHITECTURE.md section 9. The leave half was lost silently, and it is a different decision: it does not touch the password machinery. The substantive part of it is turn_see(TRUE) rather than the flag -- wizard sight draws every monster the hero cannot see, so without the re-hide there is no way back to normal visibility, and clearing the flag alone would leave the screen lying. New wizardToggleCommand, registered in commandHandlers between '^' and Escape, which is C's own switch order. Because C's arm sits in the main switch rather than the `if (wizard) switch (ch)` sub-switch that wizardCommand ports, it is reachable whether or not wizard is set, so the non-wizard case was a divergence too. 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", with 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 doc comment and in section 9. Two tests 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, then asserts the flag cleared, SenseMonsters cleared, the cell restored to the map char under the monster with standout off, the exact message text, and After false. Deleting the turnSee(true) call fails it on all three visibility assertions. The other pins "sorry". No RNG call is added: the turn_off arm of turn_see never reaches rnd. TestSeedCompatItemTables is green against the untouched golden. |
|||
| af3050b187 |
fix: bound wizard-created Which against its item table (closes #10)
createObj stored the raw 0-f nibble as Object.Which with no bounds check, so wizard mode -> C -> / -> f made 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, giving 234 for 'A' and 202 for '!', and panicked the same way. C's create_obj() was equally unchecked, but its consumers were either switches (defined for any value) or static-array reads past the end (undefined, and survivable in practice). Since one game is now one process, the Go panic kills the game outright and leaves the terminal in raw mode. Reject at the two boundaries a bad Which can enter through. createObj now refuses an out-of-range choice with a message drawn from C's own type_name() vocabulary and adds nothing to the pack, a deliberate divergence recorded in a comment because C had no defined behavior here to be faithful to. Restore refuses a snapshot describing such an object (ErrSaveCorrupt) rather than loading a game that would explode later. A decoded snapshot is also the only source of a genuinely negative Which, Which being a plain int off the wire, so it is what the Which >= 0 arm of hasValidWhich defends against. Behind those, whichLimit/hasValidWhich back defensive guards at every dispatch the issue names: the quaffHandler/readHandler/zapHandler accessors return no handler instead of indexing (for wands that is exactly what non-MASTER C did, matching no case and still running o_charges--), the callIt lore lookups, identifyType, armorClass for the a_class[] reads, initWeapon against the missing init_dam[] row for WeaponFlame, fixStick's ws_type[] read, and inventoryName and objectWorth, hoisted so one check each covers the whole family of per-kind name and appraisal tables. identifyType's bound is defensive rather than live: readHandlers registers readIdentify only for the identify scrolls, all of which sit inside the shorter idType table. No in-range input changes behavior and no guard consumes a random number: the rejection precedes every rnd() call. TestSeedCompatItemTables stays green untouched. New game/wizard_test.go covers the exact reproducer, a rejection sweep over every indexed kind including the wrapped values from input outside 0-f, an acceptance sweep proving valid choices still build the right item, one no-panic test per guarded family, the fixStick crash site, the corrupt-save rejection over both the wrapped values and a negative Which, and a check that whichLimit still agrees with the table sizes. Each guard was confirmed load-bearing by reverting it and watching the test fail. TODO.md records the step; Next Step is deliberately left alone, since this arrived out of band via an issue. |
