From 0b56ac8019abfa957c5c7c2def07dac48ba63f9f Mon Sep 17 00:00:00 2001 From: sneak Date: Tue, 7 Jul 2026 02:42:18 +0200 Subject: [PATCH] Extract MessageLine, Player pack ops, and Level list management Refactor step 6. MessageLine (was MsgLine) owns the msg/addmsg/endmsg machinery, wired to its screen, pre---More-- redraw, and input via attach(); RogueGame keeps one-line msg/addmsgf/endmsg shorthands so the ~400 call sites are unchanged. Player gains nextPackChar and removeFromPack (the state half of pack.c leave_pack); leavePack keeps only the LastPick repeat-command tracking. Level gains ObjectAt (misc.c find_obj) and AddObject/RemoveObject/AddMonster/RemoveMonster, replacing direct attach/detach calls on the level lists. Inventory and pickup UI flows stay on RogueGame: display and orchestration, not state surgery. Behavior and RNG order unchanged; suite green. --- TODO.md | 41 ++++++++++++--------- game/chase.go | 2 +- game/creature.go | 41 +++++++++++++++++++++ game/fight.go | 2 +- game/game.go | 3 +- game/io.go | 93 +++++++++++++++++++++++++++++++++--------------- game/level.go | 23 ++++++++++++ game/misc.go | 11 ------ game/monsters.go | 2 +- game/newlevel.go | 6 ++-- game/pack.go | 51 ++++++-------------------- game/rooms.go | 2 +- game/save.go | 1 + game/scrolls.go | 2 +- game/sticks.go | 2 +- game/things.go | 2 +- game/weapons.go | 2 +- 17 files changed, 177 insertions(+), 109 deletions(-) diff --git a/TODO.md b/TODO.md index 496fbb5..8a09b43 100644 --- a/TODO.md +++ b/TODO.md @@ -29,14 +29,25 @@ Refactor ground rules: # Next Step -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. +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. # Completed Steps +- 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; @@ -117,31 +128,27 @@ cross-system effects only. # Future Steps -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. -2. Refactor step 8: constructor and style pass per the house +1. 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. -3. Docs refresh: update ARCHITECTURE.md Part 2 and README.md for the +2. Docs refresh: update ARCHITECTURE.md Part 2 and README.md for the post-refactor names; add the C name → Go name rename table. -4. Playtest hardening pass: play several full games with the tcell +3. 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. -5. Verify the seed-compatibility claim against the C reference on +4. Verify the seed-compatibility claim against the C reference on c-master: same seed, same dungeon, same item tables, for several seeds. -6. Broaden unit test coverage where playtesting finds thin spots +5. Broaden unit test coverage where playtesting finds thin spots (rings, sticks, wizard commands). -7. Tag a release once a full game (Amulet retrieval and score entry) +6. Tag a release once a full game (Amulet retrieval and score entry) completes without defects. -8. Full-terminal-size support (deferred by explicit decision +7. 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. -9. Note: this repo is exempt from the standard policy scaffold. Do not +8. Note: this repo is exempt from the standard policy scaffold. Do not add Makefile, Dockerfile, or REPO_POLICIES.md. diff --git a/game/chase.go b/game/chase.go index 51bae32..d686994 100644 --- a/game/chase.go +++ b/game/chase.go @@ -169,7 +169,7 @@ func (g *RogueGame) chaseStep(th *Monster) (removed bool) { } else if this == *th.Dest { for _, obj := range g.Level.Objects { if th.Dest == &obj.Pos { - detachObj(&g.Level.Objects, obj) + g.Level.RemoveObject(obj) attachObj(&th.Pack, obj) if th.Room.Flags.Has(Gone) { diff --git a/game/creature.go b/game/creature.go index 2211a47..5388014 100644 --- a/game/creature.go +++ b/game/creature.go @@ -50,6 +50,47 @@ func (p *Player) IsWearing(ring RingKind) bool { return p.IsRing(Left, ring) || p.IsRing(Right, ring) } +// nextPackChar claims and returns the next unused pack character (pack.c +// pack_char). +func (p *Player) nextPackChar() byte { + for i := range p.PackUsed { + if !p.PackUsed[i] { + p.PackUsed[i] = true + + return byte(i) + 'a' + } + } + + return byte(len(p.PackUsed)) + 'a' // C would walk off the array here +} + +// removeFromPack takes an item out of the pack: the whole entry, or one +// of a stack when all is false (the bookkeeping half of pack.c +// leave_pack). It returns the object that left the pack — a copy when +// newobj asks for a split. +func (p *Player) removeFromPack(obj *Object, newobj, all bool) *Object { + p.Inpack-- + + nobj := obj + if obj.Count > 1 && !all { + obj.Count-- + if obj.Group != 0 { + p.Inpack++ + } + + if newobj { + copied := *obj + nobj = &copied + nobj.Count = 1 + } + } else { + p.PackUsed[obj.PackCh-'a'] = false + detachObj(&p.Pack, obj) + } + + return nobj +} + // attachMon pushes a monster onto the front of a list (list.c attach). func attachMon(list *[]*Monster, item *Monster) { *list = append([]*Monster{item}, *list...) diff --git a/game/fight.go b/game/fight.go index c491e54..53c6caf 100644 --- a/game/fight.go +++ b/game/fight.go @@ -549,7 +549,7 @@ func (g *RogueGame) removeMon(mp Coord, tp *Monster, waskill bool) { g.Level.SetMonsterAt(mp.Y, mp.X, nil) g.mvaddch(mp.Y, mp.X, tp.OldCh) - detachMon(&g.Level.Monsters, tp) + g.Level.RemoveMonster(tp) if tp.On(Targeted) { g.Kamikaze = false diff --git a/game/game.go b/game/game.go index ecaa426..a70c60c 100644 --- a/game/game.go +++ b/game/game.go @@ -110,7 +110,7 @@ type RogueGame struct { // screen / messages scr *Screen - Msgs MsgLine + Msgs MessageLine statusCache statusCache invPage invPage // things.c discovery-list pagination statics @@ -169,6 +169,7 @@ func NewGame(cfg Config) *RogueGame { g.InvDescribe = true g.Msgs.SaveMsg = true g.scr = NewScreen(cfg.Term) + g.Msgs.attach(g.scr, g.look, g.readchar) g.FileName = cfg.Home + "/rogue.save" g.rogueOpts = cfg.RogueOpts diff --git a/game/io.go b/game/io.go index b21fa49..eb487dc 100644 --- a/game/io.go +++ b/game/io.go @@ -10,9 +10,11 @@ import ( // maxMsg is io.c MAXMSG: how much message fits before --More--. const maxMsg = NumCols - len("--More--") - 1 -// MsgLine is the io.c message machinery: the static msgbuf/newpos pair plus -// the related globals (mpos, huh, and the message-behavior flags). -type MsgLine struct { +// MessageLine is the io.c message machinery: the static msgbuf/newpos +// pair plus the related globals (mpos, huh, and the message-behavior +// flags). It owns the top line of the screen; attach wires in the +// display and input it needs. +type MessageLine struct { buf strings.Builder // msgbuf newpos int Mpos int // where cursor is on top line @@ -20,49 +22,52 @@ type MsgLine struct { SaveMsg bool // remember last msg LowerMsg bool // messages should start w/lower case MsgEsc bool // check for ESC from msg's --More-- + + scr *Screen // the top line lives on scr.Std + look func(wakeup bool) // redraw before a --More-- (misc.c look) + readChar func() byte // input for --More-- prompts } // Msg displays a message at the top of the screen (io.c msg). It returns -// Escape if the player escaped out of a --More--, ^Escape otherwise (the C -// convention: callers compare against ESCAPE). -func (g *RogueGame) msg(format string, a ...any) int { +// Escape if the player escaped out of a --More--, ^Escape otherwise (the +// C convention: callers compare against ESCAPE). +func (m *MessageLine) Msg(format string, a ...any) int { // if the string is "", just clear the line if format == "" { - g.move(0, 0) - g.clrtoeol() - g.Msgs.Mpos = 0 + m.scr.Std.Move(0, 0) + m.scr.Std.Clrtoeol() + m.Mpos = 0 return ^Escape } // otherwise add to the message and flush it out - g.doaddf(format, a...) + m.doaddf(format, a...) - return g.endmsg() + return m.End() } -// addmsgf adds things to the current message (io.c addmsg). -func (g *RogueGame) addmsgf(format string, a ...any) { - g.doaddf(format, a...) +// Addf adds things to the current message (io.c addmsg). +func (m *MessageLine) Addf(format string, a ...any) { + m.doaddf(format, a...) } -// endmsg displays a new msg, giving the player a chance to see the previous +// End displays a new msg, giving the player a chance to see the previous // one if it is up there with the --More-- (io.c endmsg). -func (g *RogueGame) endmsg() int { - m := &g.Msgs +func (m *MessageLine) End() int { if m.SaveMsg { m.Huh = m.buf.String() } if m.Mpos != 0 { - g.look(false) - g.mvaddstr(0, m.Mpos, "--More--") - g.refresh() + m.look(false) + m.scr.Std.MvAddStr(0, m.Mpos, "--More--") + m.scr.Refresh() if !m.MsgEsc { - g.waitFor(' ') + m.waitForSpace() } else { for { - ch := g.readchar() + ch := m.readChar() if ch == ' ' { break } @@ -85,30 +90,60 @@ func (g *RogueGame) endmsg() int { out = string(toUpper(out[0])) + out[1:] } - g.mvaddstr(0, 0, out) - g.clrtoeol() + m.scr.Std.MvAddStr(0, 0, out) + m.scr.Std.Clrtoeol() m.Mpos = m.newpos m.newpos = 0 m.buf.Reset() - g.refresh() + m.scr.Refresh() return ^Escape } -// doaddf performs an add onto the message buffer (io.c doadd). -func (g *RogueGame) doaddf(format string, a ...any) { - m := &g.Msgs +// attach wires the message line to its display and input; NewGame and +// Restore call it once the screen and game exist. +func (m *MessageLine) attach(scr *Screen, look func(bool), readChar func() byte) { + m.scr = scr + m.look = look + m.readChar = readChar +} +// waitForSpace absorbs input until the player types a space: the +// --More-- acknowledgement (io.c wait_for). +func (m *MessageLine) waitForSpace() { + for { + if m.readChar() == ' ' { + return + } + } +} + +// doaddf performs an add onto the message buffer (io.c doadd). +func (m *MessageLine) doaddf(format string, a ...any) { s := fmt.Sprintf(format, a...) if len(s)+m.newpos >= maxMsg { - g.endmsg() + m.End() } m.buf.WriteString(s) m.newpos = m.buf.Len() } +// msg, addmsgf, and endmsg are the game-side shorthands for the message +// line; the machinery lives on MessageLine. +func (g *RogueGame) msg(format string, a ...any) int { + return g.Msgs.Msg(format, a...) +} + +func (g *RogueGame) addmsgf(format string, a ...any) { + g.Msgs.Addf(format, a...) +} + +func (g *RogueGame) endmsg() { + g.Msgs.End() +} + // stepOk returns true if it is ok to step on ch (io.c step_ok). func stepOk(ch byte) bool { switch ch { diff --git a/game/level.go b/game/level.go index 5eea628..ad93d4a 100644 --- a/game/level.go +++ b/game/level.go @@ -51,6 +51,29 @@ func (l *Level) VisibleChar(y, x int) byte { return l.Char(y, x) } +// ObjectAt finds the unclaimed object at (y, x) (misc.c find_obj). +func (l *Level) ObjectAt(y, x int) *Object { + for _, obj := range l.Objects { + if obj.Pos.Y == y && obj.Pos.X == x { + return obj + } + } + + return nil +} + +// AddObject puts an object on the level (list.c attach on lvl_obj). +func (l *Level) AddObject(obj *Object) { attachObj(&l.Objects, obj) } + +// RemoveObject takes an object off the level (list.c detach on lvl_obj). +func (l *Level) RemoveObject(obj *Object) { detachObj(&l.Objects, obj) } + +// AddMonster puts a monster on the level (list.c attach on mlist). +func (l *Level) AddMonster(m *Monster) { attachMon(&l.Monsters, m) } + +// RemoveMonster takes a monster off the level (list.c detach on mlist). +func (l *Level) RemoveMonster(m *Monster) { detachMon(&l.Monsters, m) } + // goldCalc is the GOLDCALC macro: how much a gold pile is worth at depth. func (g *RogueGame) goldCalc() int { return g.rnd(50+10*g.Depth) + 2 diff --git a/game/misc.go b/game/misc.go index 98aefa3..f9ad13f 100644 --- a/game/misc.go +++ b/game/misc.go @@ -223,17 +223,6 @@ func (g *RogueGame) showFloor() bool { return true } -// findObj finds the unclaimed object at (y, x) (misc.c find_obj). -func (g *RogueGame) findObj(y, x int) *Object { - for _, obj := range g.Level.Objects { - if obj.Pos.Y == y && obj.Pos.X == x { - return obj - } - } - - return nil -} - // eat lets her try to eat something (misc.c eat). func (g *RogueGame) eat() { obj, ok := g.promptPackItem("eat", KindFood) diff --git a/game/monsters.go b/game/monsters.go index 7cf2611..06c81fa 100644 --- a/game/monsters.go +++ b/game/monsters.go @@ -31,7 +31,7 @@ func (g *RogueGame) randMonster(wander bool) byte { func (g *RogueGame) newMonster(tp *Monster, typ byte, cp Coord) { levAdd := max(g.Depth-AmuletLevel, 0) - attachMon(&g.Level.Monsters, tp) + g.Level.AddMonster(tp) tp.Type = typ tp.Disguise = typ tp.Pos = cp diff --git a/game/newlevel.go b/game/newlevel.go index 3d162de..a549aba 100644 --- a/game/newlevel.go +++ b/game/newlevel.go @@ -105,7 +105,7 @@ func (g *RogueGame) putThings() { if g.rnd(100) < 36 { // Pick a new object and link it in the list obj := g.newThing() - attachObj(&g.Level.Objects, obj) + g.Level.AddObject(obj) // Put it somewhere obj.Pos, _ = g.findFloor(false) g.Level.SetChar(obj.Pos.Y, obj.Pos.X, obj.Kind.Glyph()) @@ -115,7 +115,7 @@ func (g *RogueGame) putThings() { // yet, put it somewhere on the ground if g.Depth >= AmuletLevel && !g.HasAmulet { obj := newObject() - attachObj(&g.Level.Objects, obj) + g.Level.AddObject(obj) obj.Damage = dice("0x0") obj.HurlDmg = dice("0x0") obj.ArmorClass = 11 @@ -137,7 +137,7 @@ func (g *RogueGame) treasureRoom() { mp, _ := g.findFloorIn(rp, 2*maxTries, false) tp := g.newThing() tp.Pos = mp - attachObj(&g.Level.Objects, tp) + g.Level.AddObject(tp) g.Level.SetChar(mp.Y, mp.X, tp.Kind.Glyph()) } diff --git a/game/pack.go b/game/pack.go index 6120c3e..faeac49 100644 --- a/game/pack.go +++ b/game/pack.go @@ -9,7 +9,7 @@ func (g *RogueGame) addPack(obj *Object, silent bool) { fromFloor := false if obj == nil { - if obj = g.findObj(p.Pos.Y, p.Pos.X); obj == nil { + if obj = g.Level.ObjectAt(p.Pos.Y, p.Pos.X); obj == nil { return } @@ -18,7 +18,7 @@ func (g *RogueGame) addPack(obj *Object, silent bool) { // Check for and deal with scare monster scrolls if obj.Kind == KindScroll && obj.ScrollKind() == ScrollScareMonster && obj.Flags.Has(WasFound) { - detachObj(&g.Level.Objects, obj) + g.Level.RemoveObject(obj) g.mvaddch(p.Pos.Y, p.Pos.X, g.floorCh()) if p.Room.Flags.Has(Gone) { @@ -34,7 +34,7 @@ func (g *RogueGame) addPack(obj *Object, silent bool) { if len(p.Pack) == 0 { p.Pack = append(p.Pack, obj) - obj.PackCh = g.packChar() + obj.PackCh = p.nextPackChar() p.Inpack++ } else { // Walk the pack looking for the insertion point, keeping items of @@ -112,7 +112,7 @@ func (g *RogueGame) addPack(obj *Object, silent bool) { return } - obj.PackCh = g.packChar() + obj.PackCh = p.nextPackChar() p.Pack = append(p.Pack[:lp+1], append([]*Object{obj}, p.Pack[lp+1:]...)...) } @@ -168,7 +168,7 @@ func (g *RogueGame) packRoom(fromFloor bool, obj *Object) bool { } if fromFloor { - detachObj(&g.Level.Objects, obj) + g.Level.RemoveObject(obj) g.mvaddch(p.Pos.Y, p.Pos.X, g.floorCh()) if p.Room.Flags.Has(Gone) { @@ -181,46 +181,17 @@ func (g *RogueGame) packRoom(fromFloor bool, obj *Object) bool { return true } -// leavePack takes an item out of the pack (pack.c leave_pack). +// leavePack takes an item out of the pack (pack.c leave_pack), keeping +// the repeat-command bookkeeping; the pack surgery is +// Player.removeFromPack. func (g *RogueGame) leavePack(obj *Object, newobj, all bool) *Object { - p := &g.Player - p.Inpack-- - - nobj := obj if obj.Count > 1 && !all { g.LastPick = obj - - obj.Count-- - if obj.Group != 0 { - p.Inpack++ - } - - if newobj { - copied := *obj - nobj = &copied - nobj.Count = 1 - } } else { g.LastPick = nil - p.PackUsed[obj.PackCh-'a'] = false - detachObj(&p.Pack, obj) } - return nobj -} - -// packChar returns the next unused pack character (pack.c pack_char). -func (g *RogueGame) packChar() byte { - p := &g.Player - for i := range p.PackUsed { - if !p.PackUsed[i] { - p.PackUsed[i] = true - - return byte(i) + 'a' - } - } - - return byte(len(p.PackUsed)) + 'a' // C would walk off the array here + return g.Player.removeFromPack(obj, newobj, all) } // inventory lists what is in the pack; returns true if there is something @@ -277,7 +248,7 @@ func (g *RogueGame) pickUp(ch byte) { return } - obj := g.findObj(p.Pos.Y, p.Pos.X) + obj := g.Level.ObjectAt(p.Pos.Y, p.Pos.X) if g.MoveOn { g.moveMsg(obj) @@ -291,7 +262,7 @@ func (g *RogueGame) pickUp(ch byte) { } g.money(obj.GoldValue) - detachObj(&g.Level.Objects, obj) + g.Level.RemoveObject(obj) p.Room.GoldVal = 0 default: diff --git a/game/rooms.go b/game/rooms.go index d78a5cf..7dc9e36 100644 --- a/game/rooms.go +++ b/game/rooms.go @@ -105,7 +105,7 @@ func (g *RogueGame) digRooms() { gold.Flags = Stackable gold.Group = goldGrp gold.Kind = KindGold - attachObj(&g.Level.Objects, gold) + g.Level.AddObject(gold) } // Put the monster in prob := 25 diff --git a/game/save.go b/game/save.go index 5452d79..3a648ce 100644 --- a/game/save.go +++ b/game/save.go @@ -618,6 +618,7 @@ func Restore(path string, cfg Config) (*RogueGame, error) { restored: true, } g.scr = NewScreen(cfg.Term) + g.Msgs.attach(g.scr, g.look, g.readchar) g.applySnapshot(&st) // defeat multiple restarting from the same place diff --git a/game/scrolls.go b/game/scrolls.go index d73eb9c..f1eea9e 100644 --- a/game/scrolls.go +++ b/game/scrolls.go @@ -104,7 +104,7 @@ func (g *RogueGame) readScroll() { // Or anything else nasty if ch := g.Level.VisibleChar(y, x); stepOk(ch) { if ch == Scroll { - if fo := g.findObj(y, x); fo != nil && fo.ScrollKind() == ScrollScareMonster { + if fo := g.Level.ObjectAt(y, x); fo != nil && fo.ScrollKind() == ScrollScareMonster { continue } } diff --git a/game/sticks.go b/game/sticks.go index 44a6710..0b2d51a 100644 --- a/game/sticks.go +++ b/game/sticks.go @@ -85,7 +85,7 @@ func (g *RogueGame) doZap() { } case WandPolymorph: pp := tp.Pack - detachMon(&g.Level.Monsters, tp) + g.Level.RemoveMonster(tp) if g.seeMonst(tp) { g.mvaddch(y, x, g.Level.Char(y, x)) diff --git a/game/things.go b/game/things.go index d0186c0..d7a9d78 100644 --- a/game/things.go +++ b/game/things.go @@ -153,7 +153,7 @@ func (g *RogueGame) dropIt() { obj = g.leavePack(obj, true, !obj.Kind.MergesInPack()) // Link it into the level object list - attachObj(&g.Level.Objects, obj) + g.Level.AddObject(obj) g.Level.SetChar(p.Pos.Y, p.Pos.X, obj.Kind.Glyph()) g.Level.FlagsAt(p.Pos.Y, p.Pos.X).Set(FDropped) diff --git a/game/weapons.go b/game/weapons.go index 515a4b6..de56d1d 100644 --- a/game/weapons.go +++ b/game/weapons.go @@ -78,7 +78,7 @@ func (g *RogueGame) fall(obj *Object, pr bool) { } } - attachObj(&g.Level.Objects, obj) + g.Level.AddObject(obj) return }