Merge refactor/god-object-extraction (refactor step 6)

This commit is contained in:
2026-07-07 02:42:18 +02:00
17 changed files with 177 additions and 109 deletions

41
TODO.md
View File

@@ -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.

View File

@@ -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) {

View File

@@ -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...)

View File

@@ -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

View File

@@ -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

View File

@@ -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 {

View File

@@ -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

View File

@@ -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)

View File

@@ -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

View File

@@ -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())
}

View File

@@ -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:

View File

@@ -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

View File

@@ -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

View File

@@ -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
}
}

View File

@@ -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))

View File

@@ -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)

View File

@@ -78,7 +78,7 @@ func (g *RogueGame) fall(obj *Object, pr bool) {
}
}
attachObj(&g.Level.Objects, obj)
g.Level.AddObject(obj)
return
}