refactor: simplifcation, change how rooms are stored in player yaml, remove legacy code

6 files changed, 214 insertions, 140 deletions

diff --git a/internal/player/hardening_password_params_test.go b/internal/player/hardening_password_params_test.go
index 4c171f1..32b399b 100644
--- a/internal/player/hardening_password_params_test.go
+++ b/internal/player/hardening_password_params_test.go
@@ -5,12 +5,7 @@ package player
 // (e.g. argon2id:1:4294967295:...) must fail the check, not request a
 // multi-TiB allocation that kills the process.
 
-import (
-	"crypto/sha256"
-	"encoding/base64"
-	"encoding/hex"
-	"testing"
-)
+import "testing"
 
 func TestCheckPasswordRejectsOutOfRangeArgonParams(t *testing.T) {
 	// Enormous memory cost (0xFFFFFFFF KiB) with an otherwise well-formed
@@ -33,7 +28,7 @@ func TestCheckPasswordRejectsOutOfRangeArgonParams(t *testing.T) {
 	}
 }
 
-func TestCheckPasswordAcceptsNormalAndLegacyHashes(t *testing.T) {
+func TestCheckPasswordAcceptsNormalHashes(t *testing.T) {
 	hashed, err := HashPassword("s3cret")
 	if err != nil {
 		t.Fatal(err)
@@ -44,19 +39,4 @@ func TestCheckPasswordAcceptsNormalAndLegacyHashes(t *testing.T) {
 	if CheckPassword("wrong", hashed) {
 		t.Fatal("wrong password accepted")
 	}
-
-	// Legacy single-pass sha256 hashes must keep verifying so old accounts
-	// can still log in and be upgraded.
-	salt := []byte("sixteen-byte-slt")
-	sum := sha256.Sum256(append(append([]byte{}, salt...), []byte("pw")...))
-	legacy := "sha256:" + base64.RawStdEncoding.EncodeToString(salt) + ":" + hex.EncodeToString(sum[:])
-	if !CheckPassword("pw", legacy) {
-		t.Fatal("legacy sha256 hash rejected")
-	}
-	if CheckPassword("nope", legacy) {
-		t.Fatal("wrong password accepted against legacy sha256 hash")
-	}
-	if !NeedsRehash(legacy) {
-		t.Fatal("legacy sha256 hash not flagged for rehash")
-	}
 }
diff --git a/internal/player/password.go b/internal/player/password.go
index de5fe5f..706ef45 100644
--- a/internal/player/password.go
+++ b/internal/player/password.go
@@ -2,7 +2,6 @@ package player
 
 import (
 	"crypto/rand"
-	"crypto/sha256"
 	"crypto/subtle"
 	"encoding/base64"
 	"encoding/hex"
@@ -19,10 +18,8 @@ import (
 //
 //	argon2id:<time>:<memoryKiB>:<threads>:<b64salt>:<hexkey>
 //
-// Legacy accounts may still carry the old single-pass salted SHA-256 hash
-// ("sha256:<b64salt>:<hexhash>"); CheckPassword verifies both, and the
-// login paths rehash to Argon2id on successful verification (see
-// NeedsRehash).
+// Only Argon2id is accepted: the prototype has no password-hash migration and
+// accounts are recreated rather than upgraded.
 const (
 	argonTime    = 1
 	argonMemory  = 64 * 1024 // KiB
@@ -78,57 +75,33 @@ func HashPassword(password string) (string, error) {
 		base64.RawStdEncoding.EncodeToString(salt), hex.EncodeToString(key)), nil
 }
 
-// CheckPassword verifies a password against a stored hash in either the
-// current Argon2id format or the legacy salted SHA-256 format.
+// CheckPassword verifies a password against a stored Argon2id hash.
 func CheckPassword(password, stored string) bool {
 	parts := strings.SplitN(stored, ":", 6)
-	if len(parts) == 0 {
+	if len(parts) == 0 || parts[0] != "argon2id" {
 		return false
 	}
-	switch parts[0] {
-	case "argon2id":
-		if len(parts) != 6 {
-			return false
-		}
-		t, errT := strconv.Atoi(parts[1])
-		m, errM := strconv.Atoi(parts[2])
-		p, errP := strconv.Atoi(parts[3])
-		salt, errSalt := base64.RawStdEncoding.DecodeString(parts[4])
-		expected, errKey := hex.DecodeString(parts[5])
-		if errT != nil || errM != nil || errP != nil || errSalt != nil || errKey != nil {
-			return false
-		}
-		if t < argonMinTime || t > argonMaxTime ||
-			m < argonMinMemory || m > argonMaxMemory ||
-			p < argonMinThreads || p > argonMaxThreads ||
-			len(expected) < argonMinKeyLen || len(expected) > argonMaxKeyLen {
-			// Out-of-range parameters: fail the check instead of trusting the
-			// stored values and requesting an enormous allocation.
-			return false
-		}
-		acquireHashSlot()
-		key := argon2.IDKey([]byte(password), salt, uint32(t), uint32(m), uint8(p), uint32(len(expected)))
-		releaseHashSlot()
-		return subtle.ConstantTimeCompare(key, expected) == 1
-	case "sha256":
-		legacy := strings.SplitN(stored, ":", 3)
-		if len(legacy) != 3 {
-			return false
-		}
-		salt, err := base64.RawStdEncoding.DecodeString(legacy[1])
-		if err != nil {
-			return false
-		}
-		hash := sha256.Sum256(append(salt, []byte(password)...))
-		return subtle.ConstantTimeCompare([]byte(legacy[2]), []byte(fmt.Sprintf("%x", hash))) == 1
-	default:
+	if len(parts) != 6 {
 		return false
 	}
-}
-
-// NeedsRehash reports whether a stored hash uses the legacy single-pass
-// SHA-256 scheme. Callers rehash with HashPassword after a successful
-// verification and persist the new hash (transparent upgrade on login).
-func NeedsRehash(stored string) bool {
-	return strings.HasPrefix(stored, "sha256:")
+	t, errT := strconv.Atoi(parts[1])
+	m, errM := strconv.Atoi(parts[2])
+	p, errP := strconv.Atoi(parts[3])
+	salt, errSalt := base64.RawStdEncoding.DecodeString(parts[4])
+	expected, errKey := hex.DecodeString(parts[5])
+	if errT != nil || errM != nil || errP != nil || errSalt != nil || errKey != nil {
+		return false
+	}
+	if t < argonMinTime || t > argonMaxTime ||
+		m < argonMinMemory || m > argonMaxMemory ||
+		p < argonMinThreads || p > argonMaxThreads ||
+		len(expected) < argonMinKeyLen || len(expected) > argonMaxKeyLen {
+		// Out-of-range parameters: fail the check instead of trusting the
+		// stored values and requesting an enormous allocation.
+		return false
+	}
+	acquireHashSlot()
+	key := argon2.IDKey([]byte(password), salt, uint32(t), uint32(m), uint8(p), uint32(len(expected)))
+	releaseHashSlot()
+	return subtle.ConstantTimeCompare(key, expected) == 1
 }
diff --git a/internal/player/password_test.go b/internal/player/password_test.go
index 82b580b..2a7d596 100644
--- a/internal/player/password_test.go
+++ b/internal/player/password_test.go
@@ -1,9 +1,6 @@
 package player
 
 import (
-	"crypto/sha256"
-	"encoding/base64"
-	"fmt"
 	"strings"
 	"testing"
 )
@@ -29,30 +26,15 @@ func TestHashPasswordUsesArgon2idAndVerifies(t *testing.T) {
 	}
 }
 
-func TestCheckPasswordVerifiesLegacySHA256(t *testing.T) {
-	salt := make([]byte, 16)
-	for i := range salt {
-		salt[i] = byte(i)
-	}
-	sum := sha256.Sum256(append(salt, []byte("legacy pw")...))
-	legacy := fmt.Sprintf("sha256:%s:%x", base64.RawStdEncoding.EncodeToString(salt), sum)
-
-	if !CheckPassword("legacy pw", legacy) {
-		t.Fatal("correct legacy password rejected")
-	}
-	if CheckPassword("wrong", legacy) {
-		t.Fatal("wrong legacy password accepted")
-	}
-	if !NeedsRehash(legacy) {
-		t.Fatal("legacy hash not flagged for rehash")
-	}
-	if NeedsRehash("argon2id:1:65536:2:c2FsdA:x") {
-		t.Fatal("argon2id hash flagged for rehash")
-	}
-}
-
 func TestCheckPasswordRejectsGarbage(t *testing.T) {
-	for _, stored := range []string{"", "plain", "argon2id:1:2:3:no-salt:zz", "unknown:v1:salt:deadbeef"} {
+	for _, stored := range []string{
+		"",
+		"plain",
+		"argon2id:1:2:3:no-salt:zz",
+		"unknown:v1:salt:deadbeef",
+		// Legacy single-pass SHA-256 hashes are no longer accepted.
+		"sha256:AAAAAAAAAAAAAAAAAAAAAA:0011223344556677889900112233445566778899001122334455667788990011",
+	} {
 		if CheckPassword("anything", stored) {
 			t.Fatalf("garbage hash accepted: %q", stored)
 		}
diff --git a/internal/player/roomset.go b/internal/player/roomset.go
index 3565bb6..d7e1b47 100644
--- a/internal/player/roomset.go
+++ b/internal/player/roomset.go
@@ -2,75 +2,103 @@ package player
 
 import (
 	"encoding/base64"
-	"math/bits"
+	"encoding/binary"
+	"fmt"
+	"sort"
+	"strings"
 
 	"gopkg.in/yaml.v3"
 )
 
+// RoomSet is the set of room IDs a character has visited, used for the map
+// discovery view.
+//
+// It is stored sparsely: room IDs are filename-derived and may be large and
+// widely spaced (the shipped example rooms run from 1001 to 99999301), so an
+// ID-indexed bitset would grow with the *largest* ID rather than the number of
+// visited rooms. A sparse map keeps memory proportional to the visited count,
+// and the YAML encoding is a sorted, delta-encoded uvarint blob (see
+// MarshalYAML) which is a handful of bytes for a typical character regardless
+// of how high the IDs run.
+//
+// The zero value is a valid empty set. Copies share the underlying map, so a
+// RoomSet should not be copied after a concurrent mutation (the game layer only
+// touches it on the owner goroutine).
 type RoomSet struct {
-	bits []byte
+	ids map[int]struct{}
+}
+
+// roomSetV1Prefix tags the delta-encoded encoding. The colon is safe inside a
+// YAML plain scalar: ':' is only a mapping indicator when followed by a space,
+// and ':' is not in the base64 alphabet, so the prefix cannot be confused with
+// payload bytes.
+const roomSetV1Prefix = "v1:"
+
+// roomSetMaxID bounds decoded room IDs. IDs are filename-derived and fit
+// comfortably in an int32; rejecting anything larger keeps a corrupt delta
+// blob from synthesizing absurd IDs.
+const roomSetMaxID = 1<<31 - 1
+
+func (rs *RoomSet) ensure() {
+	if rs.ids == nil {
+		rs.ids = make(map[int]struct{})
+	}
 }
 
 func (rs *RoomSet) Add(roomID int) {
 	if roomID < 0 {
 		return
 	}
-	byteIdx := roomID / 8
-	if byteIdx >= len(rs.bits) {
-		nb := make([]byte, byteIdx+1)
-		copy(nb, rs.bits)
-		rs.bits = nb
-	}
-	rs.bits[byteIdx] |= 1 << (roomID % 8)
+	rs.ensure()
+	rs.ids[roomID] = struct{}{}
 }
 
 func (rs *RoomSet) Has(roomID int) bool {
-	if roomID < 0 {
+	if roomID < 0 || rs.ids == nil {
 		return false
 	}
-	byteIdx := roomID / 8
-	return byteIdx < len(rs.bits) && rs.bits[byteIdx]&(1<<(roomID%8)) != 0
+	_, ok := rs.ids[roomID]
+	return ok
 }
 
 func (rs *RoomSet) Len() int {
-	n := 0
-	for _, b := range rs.bits {
-		n += bits.OnesCount8(b)
+	if rs.ids == nil {
+		return 0
 	}
-	return n
+	return len(rs.ids)
 }
 
 func (rs RoomSet) IsZero() bool {
-	return len(rs.bits) == 0
+	return len(rs.ids) == 0
 }
 
+// Keys returns the visited room IDs in ascending order.
 func (rs *RoomSet) Keys() []int {
-	var keys []int
-	for byteIdx, b := range rs.bits {
-		if b == 0 {
-			continue
-		}
-		base := byteIdx * 8
-		for bitIdx := 0; bitIdx < 8; bitIdx++ {
-			if b&(1<<bitIdx) != 0 {
-				keys = append(keys, base+bitIdx)
-			}
-		}
+	if len(rs.ids) == 0 {
+		return nil
 	}
+	keys := make([]int, 0, len(rs.ids))
+	for id := range rs.ids {
+		keys = append(keys, id)
+	}
+	sort.Ints(keys)
 	return keys
 }
 
+// Rekey remaps every visited room through idMap, leaving unmapped IDs in place.
+// It returns the new set and whether any ID changed. The receiver is not
+// modified.
 func (rs *RoomSet) Rekey(idMap map[int]int) (RoomSet, bool) {
 	var next RoomSet
 	changed := false
-	for _, k := range rs.Keys() {
-		newK, ok := idMap[k]
+	for id := range rs.ids {
+		newID, ok := idMap[id]
 		if !ok {
-			newK = k
-		} else if newK != k {
+			newID = id
+		} else if newID != id {
 			changed = true
 		}
-		next.Add(newK)
+		next.Add(newID)
 	}
 	return next, changed
 }
@@ -83,13 +111,29 @@ func NewRoomSetFrom(ids ...int) RoomSet {
 	return rs
 }
 
+// MarshalYAML emits the current encoding: ascending IDs as delta-encoded
+// uvarints, base64-encoded behind the "v1:" tag. IDs come from filenames, so a
+// small delta usually costs one byte and the total is O(visited).
 func (rs RoomSet) MarshalYAML() (any, error) {
-	return base64.StdEncoding.EncodeToString(rs.bits), nil
+	if len(rs.ids) == 0 {
+		return "", nil
+	}
+	keys := rs.Keys()
+	buf := make([]byte, 0, len(keys)*2)
+	prev := 0
+	for _, id := range keys {
+		buf = binary.AppendUvarint(buf, uint64(id-prev))
+		prev = id
+	}
+	return roomSetV1Prefix + base64.StdEncoding.EncodeToString(buf), nil
 }
 
+// UnmarshalYAML accepts only the v1 delta encoding (plus null/empty for the
+// zero value). There is deliberately no legacy decoding: the game is a
+// prototype and character files are recreated rather than migrated.
 func (rs *RoomSet) UnmarshalYAML(value *yaml.Node) error {
-	if value.Kind == yaml.ScalarNode && (value.Tag == "!!null" || value.Value == "null") {
-		rs.bits = nil
+	rs.ids = nil
+	if value.Kind == yaml.ScalarNode && (value.Tag == "!!null" || value.Value == "null" || value.Value == "") {
 		return nil
 	}
 	var s string
@@ -97,13 +141,31 @@ func (rs *RoomSet) UnmarshalYAML(value *yaml.Node) error {
 		return err
 	}
 	if s == "" {
-		rs.bits = nil
 		return nil
 	}
-	b, err := base64.StdEncoding.DecodeString(s)
+	if !strings.HasPrefix(s, roomSetV1Prefix) {
+		return fmt.Errorf("rooms_visited: unrecognized encoding, want %q prefix", roomSetV1Prefix)
+	}
+	return rs.decodeDelta(s[len(roomSetV1Prefix):])
+}
+
+func (rs *RoomSet) decodeDelta(encoded string) error {
+	raw, err := base64.StdEncoding.DecodeString(encoded)
 	if err != nil {
-		return err
+		return fmt.Errorf("rooms_visited: %w", err)
+	}
+	prev := 0
+	for len(raw) > 0 {
+		delta, n := binary.Uvarint(raw)
+		if n <= 0 {
+			return fmt.Errorf("rooms_visited: corrupt delta encoding")
+		}
+		raw = raw[n:]
+		if delta > roomSetMaxID || prev > roomSetMaxID-int(delta) {
+			return fmt.Errorf("rooms_visited: room ID out of range")
+		}
+		prev += int(delta)
+		rs.Add(prev)
 	}
-	rs.bits = b
 	return nil
 }
diff --git a/internal/player/roomset_test.go b/internal/player/roomset_test.go
index 2b6b9cb..38bd993 100644
--- a/internal/player/roomset_test.go
+++ b/internal/player/roomset_test.go
@@ -1,6 +1,8 @@
 package player
 
 import (
+	"encoding/base64"
+	"encoding/binary"
 	"strings"
 	"testing"
 
@@ -85,3 +87,59 @@ func TestRoomSetMarshalEmpty(t *testing.T) {
 		t.Errorf("MarshalYAML empty = %q, want empty string", s)
 	}
 }
+
+// Sparse, high room IDs are the reason RoomSet is not a dense bitset: the
+// encoding must stay proportional to the visited count, not the largest ID.
+func TestRoomSetSparseEncodingIsCompact(t *testing.T) {
+	var rs RoomSet
+	for _, id := range []int{1001, 1002, 1003, 2001, 2006, 99999301} {
+		rs.Add(id)
+	}
+	data, err := yaml.Marshal(PlayerStats{RoomsVisited: rs})
+	if err != nil {
+		t.Fatalf("marshal: %v", err)
+	}
+	if len(data) > 200 {
+		t.Fatalf("encoding is not compact for sparse IDs: %d bytes\n%s", len(data), data)
+	}
+
+	var loaded PlayerStats
+	if err := yaml.Unmarshal(data, &loaded); err != nil {
+		t.Fatalf("unmarshal: %v", err)
+	}
+	for _, id := range []int{1001, 1002, 1003, 2001, 2006, 99999301} {
+		if !loaded.RoomsVisited.Has(id) {
+			t.Errorf("round trip lost room %d", id)
+		}
+	}
+	if loaded.RoomsVisited.Has(5000) {
+		t.Error("round trip invented room 5000")
+	}
+}
+
+// The prototype has no legacy formats: anything without the v1 prefix is a
+// hard error rather than a silently-decoded old bitset.
+func TestRoomSetRejectsUnknownEncoding(t *testing.T) {
+	var wrap struct {
+		Visited RoomSet `yaml:"rooms_visited"`
+	}
+	if err := yaml.Unmarshal([]byte("rooms_visited: AQIDBA==\n"), &wrap); err == nil {
+		t.Fatal("expected error for unprefixed encoding")
+	}
+	if err := yaml.Unmarshal([]byte("rooms_visited: [7, 42, 900]\n"), &wrap); err == nil {
+		t.Fatal("expected error for YAML sequence encoding")
+	}
+}
+
+func TestRoomSetRejectsOutOfRangeDelta(t *testing.T) {
+	// A valid uvarint delta above roomSetMaxID must fail rather than wrap or
+	// synthesize an absurd room ID.
+	buf := binary.AppendUvarint(nil, uint64(roomSetMaxID)+1)
+	encoded := roomSetV1Prefix + base64.StdEncoding.EncodeToString(buf)
+	var wrap struct {
+		Visited RoomSet `yaml:"rooms_visited"`
+	}
+	if err := yaml.Unmarshal([]byte("rooms_visited: "+encoded+"\n"), &wrap); err == nil {
+		t.Fatal("expected error for out-of-range delta")
+	}
+}
diff --git a/internal/player/skill_abbr_test.go b/internal/player/skill_abbr_test.go
new file mode 100644
index 0000000..81039f0
--- /dev/null
+++ b/internal/player/skill_abbr_test.go
@@ -0,0 +1,19 @@
+package player
+
+import "testing"
+
+// Every skill must have an abbreviation. A missing entry silently renders as an
+// empty XP drop and an empty column in the skills display, with no error, so
+// this must fail loudly when a skill is added without its abbreviation.
+func TestEverySkillHasAbbreviation(t *testing.T) {
+	for _, skill := range AllSkills {
+		if SkillAbbr[skill] == "" {
+			t.Errorf("skill %q has no abbreviation", skill)
+		}
+	}
+	for skill := range SkillAbbr {
+		if !ValidSkill(skill) {
+			t.Errorf("SkillAbbr contains unknown skill %q", skill)
+		}
+	}
+}