diff --git a/TODO.md b/TODO.md index 7269bea..496fbb5 100644 --- a/TODO.md +++ b/TODO.md @@ -29,14 +29,21 @@ Refactor ground rules: # Next Step -Refactor step 5: method renames, items/combat/UI subsystems -(invName→inventoryName, rollEm→rollAttacks, doPot→applyPotionFuse, -getItem→promptPackItem returning (obj, ok), getDir→promptDirection); -int status codes (attack returning -1) become named results. Two or -three commits, one subsystem each. +Refactor step 6: extract types from the god object — MessageLine +owns the msg/addmsg/endmsg machinery; pack/inventory operations move +onto *Player; monster/object list management and map queries +consolidate onto *Level; RogueGame keeps turn orchestration and +cross-system effects only. # Completed Steps +- 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, @@ -110,36 +117,31 @@ three commits, one subsystem each. # Future Steps -1. Refactor step 6: extract types from the god object — MessageLine - owns the msg/addmsg/endmsg machinery; pack/inventory operations move - onto *Player; monster/object list management and map queries - consolidate onto *Level; RogueGame keeps turn orchestration and - cross-system effects only. -2. Refactor step 7: effects dispatch — the giant quaff/readScroll/doZap +1. Refactor step 7: effects dispatch — the giant quaff/readScroll/doZap switches become per-kind handler tables of small named methods, keeping effect order and RNG call sequence identical. This step also clears the outstanding cyclop/gocognit/nestif lint findings. -3. Refactor step 8: constructor and style pass per the house +2. Refactor step 8: constructor and style pass per the house styleguide — game.New(game.Params{...}) replacing NewGame(Config); replace the gameEnd panic unwind with error-based turn results where feasible; 77-column wrap sweep. -4. Docs refresh: update ARCHITECTURE.md Part 2 and README.md for the +3. Docs refresh: update ARCHITECTURE.md Part 2 and README.md for the post-refactor names; add the C name → Go name rename table. -5. Playtest hardening pass: play several full games with the tcell +4. Playtest hardening pass: play several full games with the tcell binary and extend run_test.go to script a deeper multi-level playthrough (descend past level 5, use potions, scrolls, zapping, save/restore). Fix any panics, message mismatches, or divergences from the C behavior that this uncovers, with regression tests. -6. Verify the seed-compatibility claim against the C reference on +5. Verify the seed-compatibility claim against the C reference on c-master: same seed, same dungeon, same item tables, for several seeds. -7. Broaden unit test coverage where playtesting finds thin spots +6. Broaden unit test coverage where playtesting finds thin spots (rings, sticks, wizard commands). -8. Tag a release once a full game (Amulet retrieval and score entry) +7. Tag a release once a full game (Amulet retrieval and score entry) completes without defects. -9. Full-terminal-size support (deferred by explicit decision +8. 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. -10. Note: this repo is exempt from the standard policy scaffold. Do not - add Makefile, Dockerfile, or REPO_POLICIES.md. +9. Note: this repo is exempt from the standard policy scaffold. Do not + add Makefile, Dockerfile, or REPO_POLICIES.md. diff --git a/game/armor.go b/game/armor.go index c57aea7..d8f71bc 100644 --- a/game/armor.go +++ b/game/armor.go @@ -6,8 +6,8 @@ package game func (g *RogueGame) wear() { p := &g.Player - obj := g.getItem("wear", KindArmor) - if obj == nil { + obj, ok := g.promptPackItem("wear", KindArmor) + if !ok { return } @@ -32,7 +32,7 @@ func (g *RogueGame) wear() { g.wasteTime() obj.Flags.Set(Known) - sp := g.invName(obj, true) + sp := g.inventoryName(obj, true) p.CurArmor = obj if !g.Options.Terse { @@ -70,7 +70,7 @@ func (g *RogueGame) takeOff() { g.addmsgf("you used to be") } - g.msg(" wearing %c) %s", obj.PackCh, g.invName(obj, true)) + g.msg(" wearing %c) %s", obj.PackCh, g.inventoryName(obj, true)) } // wasteTime does nothing but let other things happen (armor.c waste_time). diff --git a/game/chase.go b/game/chase.go index 5e6f5cf..0ab92bb 100644 --- a/game/chase.go +++ b/game/chase.go @@ -13,12 +13,12 @@ func (g *RogueGame) runners(int) { origPos := tp.Pos wastarget := tp.On(Targeted) - if g.moveMonster(tp) == -1 { + if removed := g.moveMonster(tp); removed { continue } if tp.On(Flying) && distCp(g.Player.Pos, tp.Pos) >= 3 { - if g.moveMonster(tp) == -1 { + if removed := g.moveMonster(tp); removed { continue } } @@ -38,23 +38,24 @@ func (g *RogueGame) runners(int) { } // moveMonster executes a single turn of running for a monster (chase.c -// move_monst). Returns -1 if the monster died or left the level. -func (g *RogueGame) moveMonster(tp *Monster) int { +// move_monst). removed reports that the monster died or left the level +// (the C -1 return). +func (g *RogueGame) moveMonster(tp *Monster) (removed bool) { if !tp.On(Slowed) || tp.Turn { - if g.chaseStep(tp) == -1 { - return -1 + if g.chaseStep(tp) { + return true } } if tp.On(Hasted) { - if g.chaseStep(tp) == -1 { - return -1 + if g.chaseStep(tp) { + return true } } tp.Turn = !tp.Turn - return 0 + return false } // relocate makes the monster's new location be the specified one, updating @@ -86,9 +87,10 @@ func (g *RogueGame) relocate(th *Monster, newLoc Coord) { } } -// chaseStep makes one thing chase another (chase.c do_chase). Returns -1 -// if the chaser died in the attempt. -func (g *RogueGame) chaseStep(th *Monster) int { +// chaseStep makes one thing chase another (chase.c do_chase). removed +// reports that the chaser died or left the level in the attempt (the C +// -1 return). +func (g *RogueGame) chaseStep(th *Monster) (removed bool) { p := &g.Player stoprun := false // true means we are there mindist := 32767 @@ -153,7 +155,7 @@ func (g *RogueGame) chaseStep(th *Monster) int { g.Kamikaze = false } - return 0 + return false } } @@ -188,7 +190,7 @@ func (g *RogueGame) chaseStep(th *Monster) int { } } else { if th.Type == 'F' { - return 0 + return false } } @@ -198,7 +200,7 @@ func (g *RogueGame) chaseStep(th *Monster) int { th.Flags.Clear(Awake) } - return 0 + return false } // chase finds the spot for the chaser to move closer to the chasee diff --git a/game/command.go b/game/command.go index 70e1349..f5dbf1b 100644 --- a/game/command.go +++ b/game/command.go @@ -240,7 +240,7 @@ func (g *RogueGame) dispatch(ch byte) { g.Kamikaze = true } - if !g.getDir() { + if !g.promptDirection() { g.After = false break @@ -269,7 +269,7 @@ func (g *RogueGame) dispatch(ch byte) { continue // the C goto over: fight by running at it } case 't': - if !g.getDir() { + if !g.promptDirection() { g.After = false } else { g.missile(g.Delta.Y, g.Delta.X) @@ -334,7 +334,7 @@ func (g *RogueGame) dispatch(ch byte) { case 's': g.search() case 'z': - if g.getDir() { + if g.promptDirection() { g.doZap() } else { g.After = false @@ -360,7 +360,7 @@ func (g *RogueGame) dispatch(ch byte) { g.After = false // "legal" illegal command case '^': g.After = false - if g.getDir() { + if g.promptDirection() { g.Delta.Y += p.Pos.Y g.Delta.X += p.Pos.X @@ -386,7 +386,7 @@ func (g *RogueGame) dispatch(ch byte) { g.Again = false case 'm': g.MoveOn = true - if !g.getDir() { + if !g.promptDirection() { g.After = false } else { ch = g.DirCh @@ -453,7 +453,7 @@ func (g *RogueGame) wizardCommand(ch byte) { case CTRL('X'): g.turnSee(p.On(SenseMonsters)) case CTRL('~'): - if item := g.getItem("charge", KindWand); item != nil { + if item, ok := g.promptPackItem("charge", KindWand); ok { item.Charges = 10000 } case CTRL('I'): @@ -738,9 +738,9 @@ func (g *RogueGame) levitCheck() bool { // call allows a user to call a potion, scroll, or ring something // (command.c call). func (g *RogueGame) call() { - obj := g.getItem("call", KindCallable) + obj, ok := g.promptPackItem("call", KindCallable) // Make certain that it is something that we want to name - if obj == nil { + if !ok { return } @@ -820,7 +820,7 @@ func (g *RogueGame) current(cur *Object, how, where string) { } g.InvDescribe = false - g.addmsgf("%c) %s", cur.PackCh, g.invName(cur, true)) + g.addmsgf("%c) %s", cur.PackCh, g.inventoryName(cur, true)) g.InvDescribe = true if where != "" { diff --git a/game/fight.go b/game/fight.go index b607a05..c491e54 100644 --- a/game/fight.go +++ b/game/fight.go @@ -67,7 +67,7 @@ func (g *RogueGame) fight(mp Coord, weap *Object, thrown bool) bool { didHit := false g.HasHit = g.Options.Terse && !g.ToDeath - if g.rollEm(&p.Creature, &tp.Creature, weap, thrown) { + if g.rollAttacks(&p.Creature, &tp.Creature, weap, thrown) { didHit = false if thrown { @@ -104,9 +104,10 @@ func (g *RogueGame) fight(mp Coord, weap *Object, thrown bool) bool { return didHit } -// attack has the monster attack the player (fight.c attack). Returns -1 if -// the monster removed itself from the level during its own attack. -func (g *RogueGame) attack(mp *Monster) int { +// attack has the monster attack the player (fight.c attack). removed +// reports that the monster took itself off the level during its own +// attack (the C -1 return). +func (g *RogueGame) attack(mp *Monster) (removed bool) { p := &g.Player // Since this is an attack, stop running and any healing that was // going on at the time. @@ -128,9 +129,8 @@ func (g *RogueGame) attack(mp *Monster) int { mname := g.setMname(mp) oldhp := p.Stats.HP - removed := false - if g.rollEm(&mp.Creature, &p.Creature, nil, false) { + if g.rollAttacks(&mp.Creature, &p.Creature, nil, false) { if mp.Type != 'I' { if g.HasHit { g.addmsgf(". ") @@ -290,7 +290,7 @@ func (g *RogueGame) attack(mp *Monster) int { removed = true g.leavePack(steal, false, false) - g.msg("she stole %s!", g.invName(steal, true)) + g.msg("she stole %s!", g.inventoryName(steal, true)) } } } @@ -317,11 +317,7 @@ func (g *RogueGame) attack(mp *Monster) int { g.Count = 0 g.status() - if removed { - return -1 - } - - return 0 + return removed } // swing returns true if the swing hits (fight.c swing). @@ -332,8 +328,8 @@ func (g *RogueGame) swing(atLvl, opArm, wplus int) bool { return res+wplus >= need } -// rollEm rolls several attacks (fight.c roll_em). -func (g *RogueGame) rollEm(thatt, thdef *Creature, weap *Object, hurl bool) bool { +// rollAttacks rolls several attacks (fight.c roll_em). +func (g *RogueGame) rollAttacks(thatt, thdef *Creature, weap *Object, hurl bool) bool { p := &g.Player att := &thatt.Stats def := &thdef.Stats diff --git a/game/fight_test.go b/game/fight_test.go index bd72056..6157bf1 100644 --- a/game/fight_test.go +++ b/game/fight_test.go @@ -32,7 +32,7 @@ func TestRollEmParsesMultiAttackDice(t *testing.T) { // With attacker level 20 vs armor 10, swing always hits // (rnd(20)+wplus >= (20-20)-10 is always true), so three attacks of // 1x4 + str bonus 1 each must deal between 6 and 15 damage. - if !g.rollEm(att, def, nil, false) { + if !g.rollAttacks(att, def, nil, false) { t.Fatal("attack with guaranteed swing missed") } diff --git a/game/misc.go b/game/misc.go index 2390efe..98aefa3 100644 --- a/game/misc.go +++ b/game/misc.go @@ -236,8 +236,8 @@ func (g *RogueGame) findObj(y, x int) *Object { // eat lets her try to eat something (misc.c eat). func (g *RogueGame) eat() { - obj := g.getItem("eat", KindFood) - if obj == nil { + obj, ok := g.promptPackItem("eat", KindFood) + if !ok { return } @@ -406,9 +406,9 @@ func (g *RogueGame) isCurrent(obj *Object) bool { return false } -// getDir sets up the direction coordinate for use in various "prefix" -// commands (misc.c get_dir). -func (g *RogueGame) getDir() bool { +// promptDirection sets up the direction coordinate for use in various +// "prefix" commands (misc.c get_dir). +func (g *RogueGame) promptDirection() bool { if g.Again && g.LastDir != 0 { g.Delta = g.lastDelt g.DirCh = g.LastDir diff --git a/game/pack.go b/game/pack.go index 4e2727f..6120c3e 100644 --- a/game/pack.go +++ b/game/pack.go @@ -137,7 +137,7 @@ func (g *RogueGame) addPack(obj *Object, silent bool) { g.addmsgf("you now have ") } - g.msg("%s (%c)", g.invName(obj, !g.Options.Terse), obj.PackCh) + g.msg("%s (%c)", g.inventoryName(obj, !g.Options.Terse), obj.PackCh) } } @@ -236,7 +236,7 @@ func (g *RogueGame) inventory(list []*Object, kind ObjectKind) bool { g.NObjs++ g.Msgs.MsgEsc = true - line := string(item.PackCh) + ") " + g.invName(item, false) + line := string(item.PackCh) + ") " + g.inventoryName(item, false) if g.addLine("%s", line) == Escape { g.Msgs.MsgEsc = false g.msg("") @@ -323,7 +323,7 @@ func (g *RogueGame) moveMsg(obj *Object) { g.addmsgf("you ") } - g.msg("moved onto %s", g.invName(obj, true)) + g.msg("moved onto %s", g.inventoryName(obj, true)) } // pickyInven allows the player to inventory a single item (pack.c @@ -337,7 +337,7 @@ func (g *RogueGame) pickyInven() { } if len(p.Pack) == 1 { - g.msg("a) %s", g.invName(p.Pack[0], false)) + g.msg("a) %s", g.inventoryName(p.Pack[0], false)) return } @@ -354,7 +354,7 @@ func (g *RogueGame) pickyInven() { for _, obj := range p.Pack { if mch == obj.PackCh { - g.msg("%c) %s", mch, g.invName(obj, false)) + g.msg("%c) %s", mch, g.inventoryName(obj, false)) return } @@ -363,23 +363,24 @@ func (g *RogueGame) pickyInven() { g.msg("'%s' not in pack", unctrl(mch)) } -// getItem picks something out of a pack for a purpose (pack.c get_item). -func (g *RogueGame) getItem(purpose string, kind ObjectKind) *Object { +// promptPackItem picks something out of a pack for a purpose (pack.c +// get_item); ok reports whether the player chose an item. +func (g *RogueGame) promptPackItem(purpose string, kind ObjectKind) (obj *Object, ok bool) { p := &g.Player if len(p.Pack) == 0 { g.msg("you aren't carrying anything") - return nil + return nil, false } if g.Again { if g.LastPick != nil { - return g.LastPick + return g.LastPick, true } g.msg("you ran out") - return nil + return nil, false } for { @@ -402,7 +403,7 @@ func (g *RogueGame) getItem(purpose string, kind ObjectKind) *Object { g.After = false g.msg("") - return nil + return nil, false } g.NObjs = 1 // normal case: person types one char @@ -411,7 +412,7 @@ func (g *RogueGame) getItem(purpose string, kind ObjectKind) *Object { if !g.inventory(p.Pack, kind) { g.After = false - return nil + return nil, false } continue @@ -419,7 +420,7 @@ func (g *RogueGame) getItem(purpose string, kind ObjectKind) *Object { for _, obj := range p.Pack { if obj.PackCh == ch { - return obj + return obj, true } } diff --git a/game/potions.go b/game/potions.go index 64bf973..bf6ffc3 100644 --- a/game/potions.go +++ b/game/potions.go @@ -16,9 +16,9 @@ type pact struct { // quaff drinks a potion from the pack (potions.c quaff). func (g *RogueGame) quaff() { p := &g.Player - obj := g.getItem("quaff", KindPotion) + obj, ok := g.promptPackItem("quaff", KindPotion) // Make certain that it is something that we want to drink - if obj == nil { + if !ok { return } @@ -43,7 +43,7 @@ func (g *RogueGame) quaff() { switch obj.PotionKind() { case PotionConfusion: - g.doPot(PotionConfusion, !trip) + g.applyPotionFuse(PotionConfusion, !trip) case PotionPoison: g.Items.Potions[PotionPoison].Know = true if p.IsWearing(RingSustainStrength) { @@ -118,11 +118,11 @@ func (g *RogueGame) quaff() { g.SeenStairs = g.seenStairs() } - g.doPot(PotionLSD, true) + g.applyPotionFuse(PotionLSD, true) case PotionSeeInvisible: show := p.On(CanSeeInvisible) - g.doPot(PotionSeeInvisible, false) + g.applyPotionFuse(PotionSeeInvisible, false) if !show { g.invisOn() @@ -177,9 +177,9 @@ func (g *RogueGame) quaff() { g.msg("hey, this tastes great. It make you feel warm all over") case PotionBlindness: - g.doPot(PotionBlindness, true) + g.applyPotionFuse(PotionBlindness, true) case PotionLevitation: - g.doPot(PotionLevitation, true) + g.applyPotionFuse(PotionLevitation, true) } g.status() @@ -194,9 +194,9 @@ func (g *RogueGame) raiseLevel() { g.checkLevel() } -// doPot does a potion with standard setup: it uses a fuse and turns on a -// flag (potions.c do_pot). -func (g *RogueGame) doPot(kind PotionKind, knowit bool) { +// applyPotionFuse does a potion with standard setup: it uses a fuse and +// turns on a flag (potions.c do_pot). +func (g *RogueGame) applyPotionFuse(kind PotionKind, knowit bool) { pp := &g.data.pActions[kind] if !g.Items.Potions[kind].Know { g.Items.Potions[kind].Know = knowit diff --git a/game/rings.go b/game/rings.go index de988ae..c62a00f 100644 --- a/game/rings.go +++ b/game/rings.go @@ -7,9 +7,9 @@ import "fmt" // ringOn puts a ring on a hand (rings.c ring_on). func (g *RogueGame) ringOn() { p := &g.Player - obj := g.getItem("put on", KindRing) + obj, ok := g.promptPackItem("put on", KindRing) // Make certain that it is something that we want to wear - if obj == nil { + if !ok { return } @@ -65,7 +65,7 @@ func (g *RogueGame) ringOn() { g.addmsgf("you are now wearing ") } - g.msg("%s (%c)", g.invName(obj, true), obj.PackCh) + g.msg("%s (%c)", g.inventoryName(obj, true), obj.PackCh) } // ringOff takes off a ring (rings.c ring_off). @@ -103,7 +103,7 @@ func (g *RogueGame) ringOff() { } if g.dropCheck(obj) { - g.msg("was wearing %s(%c)", g.invName(obj, true), obj.PackCh) + g.msg("was wearing %s(%c)", g.inventoryName(obj, true), obj.PackCh) } } diff --git a/game/rip.go b/game/rip.go index 6c6f31e..5e504cb 100644 --- a/game/rip.go +++ b/game/rip.go @@ -189,7 +189,7 @@ func (g *RogueGame) totalWinner() { } g.scr.Std.MvPrintwf(line, 0, "%c) %5d %s", obj.PackCh, worth, - g.invName(obj, false)) + g.inventoryName(obj, false)) line++ p.Purse += worth } diff --git a/game/scrolls.go b/game/scrolls.go index fbcdfd1..d73eb9c 100644 --- a/game/scrolls.go +++ b/game/scrolls.go @@ -7,8 +7,8 @@ package game func (g *RogueGame) readScroll() { p := &g.Player - obj := g.getItem("read", KindScroll) - if obj == nil { + obj, ok := g.promptPackItem("read", KindScroll) + if !ok { return } diff --git a/game/sticks.go b/game/sticks.go index 20ac1d6..44a6710 100644 --- a/game/sticks.go +++ b/game/sticks.go @@ -14,8 +14,8 @@ const ( func (g *RogueGame) doZap() { p := &g.Player - obj := g.getItem("zap with", KindWand) - if obj == nil { + obj, ok := g.promptPackItem("zap with", KindWand) + if !ok { return } diff --git a/game/tables.go b/game/tables.go index 1bb644c..bb2c4fc 100644 --- a/game/tables.go +++ b/game/tables.go @@ -94,7 +94,7 @@ type gameData struct { initWeaps [NumWeaponTypes]weaponSetup // pActions is potions.c p_actions[]. The P_SEEINVIS message is dynamic - // (it names the fruit) and is computed in doPot. + // (it names the fruit) and is computed in applyPotionFuse. pActions [NumPotionTypes]pact // idType maps identify scrolls to the kind of item they identify diff --git a/game/things.go b/game/things.go index a74173f..d0186c0 100644 --- a/game/things.go +++ b/game/things.go @@ -9,7 +9,7 @@ import ( // invName returns the name of something as it would appear in an inventory // (things.c inv_name). -func (g *RogueGame) invName(obj *Object, drop bool) string { +func (g *RogueGame) inventoryName(obj *Object, drop bool) string { var pb strings.Builder which := obj.Which @@ -142,8 +142,8 @@ func (g *RogueGame) dropIt() { return } - obj := g.getItem("drop", KindNone) - if obj == nil { + obj, ok := g.promptPackItem("drop", KindNone) + if !ok { return } @@ -162,7 +162,7 @@ func (g *RogueGame) dropIt() { g.HasAmulet = false } - g.msg("dropped %s", g.invName(obj, true)) + g.msg("dropped %s", g.inventoryName(obj, true)) } // dropCheck does special checks for dropping or unwielding|unwearing| @@ -392,7 +392,7 @@ func (g *RogueGame) printDisc(typ byte) { if info[order[i]].Know || info[order[i]].Guess != "" { obj.Kind = objectKindForGlyph(typ) obj.Which = order[i] - g.addLine("%s", g.invName(&obj, false)) + g.addLine("%s", g.inventoryName(&obj, false)) numFound++ } diff --git a/game/weapons.go b/game/weapons.go index 8158a51..515a4b6 100644 --- a/game/weapons.go +++ b/game/weapons.go @@ -9,8 +9,8 @@ const noWeapon WeaponKind = -1 // missile fires a missile in a given direction (weapons.c missile). func (g *RogueGame) missile(ydelta, xdelta int) { // Get which thing we are hurling - obj := g.getItem("throw", KindWeapon) - if obj == nil { + obj, ok := g.promptPackItem("throw", KindWeapon) + if !ok { return } @@ -113,8 +113,8 @@ func (g *RogueGame) wield() { p.CurWeapon = oweapon - obj := g.getItem("wield", KindWeapon) - if obj == nil { + obj, ok := g.promptPackItem("wield", KindWeapon) + if !ok { g.After = false return @@ -133,7 +133,7 @@ func (g *RogueGame) wield() { return } - sp := g.invName(obj, true) + sp := g.inventoryName(obj, true) p.CurWeapon = obj if !g.Options.Terse { diff --git a/game/wizard.go b/game/wizard.go index dedb1f4..d954421 100644 --- a/game/wizard.go +++ b/game/wizard.go @@ -119,7 +119,7 @@ func (g *RogueGame) whatis(insist bool, kind ObjectKind) { var obj *Object for { - obj = g.getItem("identify", kind) + obj, _ = g.promptPackItem("identify", kind) if !insist { break @@ -161,7 +161,7 @@ func (g *RogueGame) whatis(insist bool, kind ObjectKind) { setKnow(obj, g.Items.Rings[:]) } - g.msg("%s", g.invName(obj, false)) + g.msg("%s", g.inventoryName(obj, false)) } // setKnow sets things up when we really know what a thing is (wizard.c