Decompose pack.go (refactor step 7)

addPack splits into pickupScareScroll and packInsert with
packScanKind/packScanWhich/packMatch/packMatchGroup for the C
linked-list walk; promptPackItem gains repeatLastItem and
promptItemPurpose; inventory's empty-handed messages flatten via
chooseTerse. pack.go is complexity-clean. Behavior unchanged.

game/pack.go

@@ -17,107 +17,15 @@ 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) {
+	if g.pickupScareScroll(obj) {
-		g.Level.RemoveObject(obj)
-		g.mvaddch(p.Pos.Y, p.Pos.X, g.floorCh())
-
-		if p.Room.Flags.Has(Gone) {
-			g.Level.SetChar(p.Pos.Y, p.Pos.X, Passage)
-		} else {
-			g.Level.SetChar(p.Pos.Y, p.Pos.X, Floor)
-		}
-
-		g.msg("the scroll turns to dust as you pick it up")
-
 		return
 	}
 
-	if len(p.Pack) == 0 {
+	obj, ok := g.packInsert(obj, fromFloor)
-		p.Pack = append(p.Pack, obj)
+	if !ok {
-		obj.PackCh = p.nextPackChar()
-		p.Inpack++
-	} else {
-		// Walk the pack looking for the insertion point, keeping items of
-		// one type together and merging stackable/grouped items — a direct
-		// translation of the C linked-list walk. lp is the index to insert
-		// after; -1 after a merge means no insertion.
-		lp := -1
-		merged := false
-
-		for i := 0; i < len(p.Pack); i++ {
-			if p.Pack[i].Kind != obj.Kind {
-				lp = i
-
-				continue
-			}
-			// found the group of our type: scan for matching subtype
-			for p.Pack[i].Kind == obj.Kind && p.Pack[i].Which != obj.Which {
-				lp = i
-				if i+1 >= len(p.Pack) {
-					break
-				}
-
-				i++
-			}
-
-			op := p.Pack[i]
-			if op.Kind == obj.Kind && op.Which == obj.Which {
-				switch {
-				case op.Kind.MergesInPack():
-					if !g.packRoom(fromFloor, obj) {
 		return
 	}
 
-					op.Count++
-					obj = op
-					lp = -1
-					merged = true
-				case obj.Group != 0:
-					lp = i
-					for p.Pack[i].Kind == obj.Kind &&
-						p.Pack[i].Which == obj.Which &&
-						p.Pack[i].Group != obj.Group {
-						lp = i
-						if i+1 >= len(p.Pack) {
-							break
-						}
-
-						i++
-					}
-
-					op = p.Pack[i]
-					if op.Kind == obj.Kind && op.Which == obj.Which &&
-						op.Group == obj.Group {
-						op.Count += obj.Count
-						p.Inpack--
-
-						if !g.packRoom(fromFloor, obj) {
-							return
-						}
-
-						obj = op
-						lp = -1
-						merged = true
-					}
-				default:
-					lp = i
-				}
-			}
-
-			break
-		}
-
-		if !merged && lp != -1 {
-			if !g.packRoom(fromFloor, obj) {
-				return
-			}
-
-			obj.PackCh = p.nextPackChar()
-			p.Pack = append(p.Pack[:lp+1],
-				append([]*Object{obj}, p.Pack[lp+1:]...)...)
-		}
-	}
-
 	obj.Flags.Set(WasFound)
 
 	// If this was the object of something's desire, that monster will get
@@ -141,6 +49,163 @@ func (g *RogueGame) addPack(obj *Object, silent bool) {
 	}
 }
 
+// pickupScareScroll crumbles a found scare monster scroll when it is
+// picked up again; it reports whether it did (pack.c add_pack).
+func (g *RogueGame) pickupScareScroll(obj *Object) bool {
+	if obj.Kind != KindScroll || obj.ScrollKind() != ScrollScareMonster ||
+		!obj.Flags.Has(WasFound) {
+		return false
+	}
+
+	p := &g.Player
+
+	g.Level.RemoveObject(obj)
+	g.mvaddch(p.Pos.Y, p.Pos.X, g.floorCh())
+
+	if p.Room.Flags.Has(Gone) {
+		g.Level.SetChar(p.Pos.Y, p.Pos.X, Passage)
+	} else {
+		g.Level.SetChar(p.Pos.Y, p.Pos.X, Floor)
+	}
+
+	g.msg("the scroll turns to dust as you pick it up")
+
+	return true
+}
+
+// packInsert places the object in the pack, keeping items of one type
+// together and merging stackable and grouped items — a translation of
+// the C linked-list walk in pack.c add_pack. It returns the pack entry
+// the object ended up as; ok is false when the pack has no room.
+func (g *RogueGame) packInsert(obj *Object, fromFloor bool) (*Object, bool) {
+	p := &g.Player
+	if len(p.Pack) == 0 {
+		p.Pack = append(p.Pack, obj)
+		obj.PackCh = p.nextPackChar()
+		p.Inpack++
+
+		return obj, true
+	}
+
+	merged := false
+
+	// lp is the index to insert after; -1 after a merge means no
+	// insertion.
+	i, lp := packScanKind(p.Pack, obj.Kind)
+	if i < len(p.Pack) {
+		i, lp = packScanWhich(p.Pack, obj, i, lp)
+
+		if op := p.Pack[i]; op.Kind == obj.Kind && op.Which == obj.Which {
+			var ok bool
+
+			obj, lp, merged, ok = g.packMatch(obj, op, i, lp, fromFloor)
+			if !ok {
+				return nil, false
+			}
+		}
+	}
+
+	if !merged && lp != -1 {
+		if !g.packRoom(fromFloor, obj) {
+			return nil, false
+		}
+
+		obj.PackCh = p.nextPackChar()
+		p.Pack = append(p.Pack[:lp+1],
+			append([]*Object{obj}, p.Pack[lp+1:]...)...)
+	}
+
+	return obj, true
+}
+
+// packScanKind scans to the first pack entry of this kind, returning
+// its index (or the pack length) and the entry to insert after (the
+// outer scan of pack.c add_pack).
+func packScanKind(pack []*Object, kind ObjectKind) (int, int) {
+	lp := -1
+
+	i := 0
+	for ; i < len(pack); i++ {
+		if pack[i].Kind == kind {
+			break
+		}
+
+		lp = i
+	}
+
+	return i, lp
+}
+
+// packScanWhich scans within the kind group for the matching subtype
+// (the inner scan of pack.c add_pack).
+func packScanWhich(pack []*Object, obj *Object, i, lp int) (int, int) {
+	for pack[i].Kind == obj.Kind && pack[i].Which != obj.Which {
+		lp = i
+		if i+1 >= len(pack) {
+			break
+		}
+
+		i++
+	}
+
+	return i, lp
+}
+
+// packMatch merges the object with a matching pack entry when possible:
+// stackables merge counts, grouped missiles rejoin their bundle, and
+// anything else marks the insertion point (the matched-subtype switch
+// of pack.c add_pack). It returns the resulting entry, the insert-after
+// index, whether a merge happened, and ok false when the pack is full.
+func (g *RogueGame) packMatch(obj, op *Object, i, lp int, fromFloor bool) (*Object, int, bool, bool) {
+	switch {
+	case op.Kind.MergesInPack():
+		if !g.packRoom(fromFloor, obj) {
+			return nil, 0, false, false
+		}
+
+		op.Count++
+
+		return op, -1, true, true
+	case obj.Group != 0:
+		return g.packMatchGroup(obj, i, fromFloor)
+	default:
+		return obj, i, false, true
+	}
+}
+
+// packMatchGroup rejoins a grouped missile bundle with its group entry
+// (the o_group arm of pack.c add_pack).
+func (g *RogueGame) packMatchGroup(obj *Object, i int, fromFloor bool) (*Object, int, bool, bool) {
+	p := &g.Player
+
+	lp := i
+	for p.Pack[i].Kind == obj.Kind &&
+		p.Pack[i].Which == obj.Which &&
+		p.Pack[i].Group != obj.Group {
+		lp = i
+		if i+1 >= len(p.Pack) {
+			break
+		}
+
+		i++
+	}
+
+	op := p.Pack[i]
+	if op.Kind == obj.Kind && op.Which == obj.Which &&
+		op.Group == obj.Group {
+		op.Count += obj.Count
+		p.Inpack--
+
+		if !g.packRoom(fromFloor, obj) {
+			return nil, 0, false, false
+		}
+
+		return op, -1, true, true
+	}
+
+	return obj, lp, false, true
+}
+
 // packRoom sees if there's room in the pack; if not, prints an appropriate
 // message (pack.c pack_room).
 func (g *RogueGame) packRoom(fromFloor bool, obj *Object) bool {
@@ -219,18 +284,11 @@ func (g *RogueGame) inventory(list []*Object, kind ObjectKind) bool {
 	}
 
 	if g.NObjs == 0 {
-		if g.Options.Terse {
 		if kind == KindNone {
-				g.msg("empty handed")
+			g.msg("%s", g.chooseTerse("empty handed", "you are empty handed"))
 		} else {
-				g.msg("nothing appropriate")
+			g.msg("%s", g.chooseTerse("nothing appropriate",
-			}
+				"you don't have anything appropriate"))
-		} else {
-			if kind == KindNone {
-				g.msg("you are empty handed")
-			} else {
-				g.msg("you don't have anything appropriate")
-			}
 		}
 
 		return false
@@ -345,27 +403,12 @@ func (g *RogueGame) promptPackItem(purpose string, kind ObjectKind) (obj *Object
 	}
 
 	if g.Again {
-		if g.LastPick != nil {
+		return g.repeatLastItem()
-			return g.LastPick, true
-		}
-
-		g.msg("you ran out")
-
-		return nil, false
 	}
 
 	for {
-		if !g.Options.Terse {
+		g.promptItemPurpose(purpose)
-			g.addmsgf("which object do you want to ")
-		}
 
-		g.addmsgf("%s", purpose)
-
-		if g.Options.Terse {
-			g.addmsgf(" what")
-		}
-
-		g.msg("? (* for list): ")
 		ch := g.readchar()
 		g.Msgs.Mpos = 0
 		// Give the poor player a chance to abort the command
@@ -399,6 +442,34 @@ func (g *RogueGame) promptPackItem(purpose string, kind ObjectKind) (obj *Object
 	}
 }
 
+// repeatLastItem replays the previous selection for the repeat command
+// (pack.c get_item).
+func (g *RogueGame) repeatLastItem() (*Object, bool) {
+	if g.LastPick != nil {
+		return g.LastPick, true
+	}
+
+	g.msg("you ran out")
+
+	return nil, false
+}
+
+// promptItemPurpose prints the "which object do you want to ...?"
+// prompt (pack.c get_item).
+func (g *RogueGame) promptItemPurpose(purpose string) {
+	if !g.Options.Terse {
+		g.addmsgf("which object do you want to ")
+	}
+
+	g.addmsgf("%s", purpose)
+
+	if g.Options.Terse {
+		g.addmsgf(" what")
+	}
+
+	g.msg("? (* for list): ")
+}
+
 // money adds or subtracts gold from the pack (pack.c money).
 func (g *RogueGame) money(value int) {
 	p := &g.Player