diff options
Diffstat (limited to 'internal/player')
| -rw-r--r-- | internal/player/store.go | 39 | ||||
| -rw-r--r-- | internal/player/store_test.go | 81 |
2 files changed, 99 insertions, 21 deletions
diff --git a/internal/player/store.go b/internal/player/store.go index 9bfdf40..d0dbd0f 100644 --- a/internal/player/store.go +++ b/internal/player/store.go @@ -8,6 +8,7 @@ import ( "gopkg.in/yaml.v3" "thehouseoficarus/internal/item" + "thehouseoficarus/internal/persistence" ) type AccountStore struct { @@ -41,7 +42,7 @@ func (s *AccountStore) SaveAccount(acc *Account) error { if err != nil { return err } - return os.WriteFile(path, data, 0644) + return persistence.WriteFile(path, data, 0600) } func (s *AccountStore) AccountExists(name string) bool { @@ -61,7 +62,7 @@ func (s *AccountStore) FindAccount(name string) (string, error) { } lower := strings.ToLower(name) for _, e := range entries { - if e.IsDir() { + if e.IsDir() || !strings.HasSuffix(e.Name(), ".yaml") { continue } base := strings.TrimSuffix(e.Name(), ".yaml") @@ -103,23 +104,20 @@ func (s *AccountStore) LoadCharacter(name string) (*Player, error) { func (s *AccountStore) SaveCharacter(p *Player) error { path := s.CharPath(p.Name) - if p.GodMode && p.GodBackup != nil { - buffedSkills := p.Skills - buffedHP := p.HP - p.Skills = p.GodBackup - if p.HP > p.MaxHP() { - p.HP = p.MaxHP() + // Serialize a value copy so saving god-mode stats never temporarily + // changes the live player. The caller still owns all referenced maps. + snapshot := *p + if snapshot.GodMode && snapshot.GodBackup != nil { + snapshot.Skills = snapshot.GodBackup + if snapshot.HP > snapshot.MaxHP() { + snapshot.HP = snapshot.MaxHP() } - defer func() { - p.Skills = buffedSkills - p.HP = buffedHP - }() } - data, err := yaml.Marshal(p) + data, err := yaml.Marshal(&snapshot) if err != nil { return err } - return os.WriteFile(path, data, 0644) + return persistence.WriteFile(path, data, 0600) } func (s *AccountStore) CharacterExists(name string) bool { @@ -139,7 +137,7 @@ func (s *AccountStore) FindCharacter(name string) (string, error) { } lower := strings.ToLower(name) for _, e := range entries { - if e.IsDir() { + if e.IsDir() || !strings.HasSuffix(e.Name(), ".yaml") { continue } base := strings.TrimSuffix(e.Name(), ".yaml") @@ -150,10 +148,7 @@ func (s *AccountStore) FindCharacter(name string) (string, error) { return "", fmt.Errorf("character not found: %s", name) } -// ListCharacterNames returns the stem (character name) of every -// data/players/characters/*.yaml file. Used by room-ID migration to walk and -// rewrite offline characters. Online characters are excluded by the caller -// (their in-memory state is updated directly). +// ListAccountNames returns the stem of each account YAML file. func (s *AccountStore) ListAccountNames() ([]string, error) { dir := filepath.Join(s.dataDir, "players", "accounts") entries, err := os.ReadDir(dir) @@ -162,7 +157,7 @@ func (s *AccountStore) ListAccountNames() ([]string, error) { } var names []string for _, e := range entries { - if e.IsDir() { + if e.IsDir() || !strings.HasSuffix(e.Name(), ".yaml") { continue } name := strings.TrimSuffix(e.Name(), ".yaml") @@ -174,6 +169,8 @@ func (s *AccountStore) ListAccountNames() ([]string, error) { return names, nil } +// ListCharacterNames returns character YAML stems for offline migrations. +// Callers exclude online characters and update their in-memory state instead. func (s *AccountStore) ListCharacterNames() ([]string, error) { dir := filepath.Join(s.dataDir, "players", "characters") entries, err := os.ReadDir(dir) @@ -182,7 +179,7 @@ func (s *AccountStore) ListCharacterNames() ([]string, error) { } var names []string for _, e := range entries { - if e.IsDir() { + if e.IsDir() || !strings.HasSuffix(e.Name(), ".yaml") { continue } name := strings.TrimSuffix(e.Name(), ".yaml") diff --git a/internal/player/store_test.go b/internal/player/store_test.go new file mode 100644 index 0000000..593e9d1 --- /dev/null +++ b/internal/player/store_test.go @@ -0,0 +1,81 @@ +package player + +import ( + "os" + "path/filepath" + "reflect" + "sync" + "testing" +) + +func testAccountStore(t *testing.T) *AccountStore { + t.Helper() + dir := t.TempDir() + for _, kind := range []string{"accounts", "characters"} { + if err := os.MkdirAll(filepath.Join(dir, "players", kind), 0755); err != nil { + t.Fatal(err) + } + } + return NewAccountStore(dir) +} + +func TestSaveGodPlayerDoesNotMutateLiveState(t *testing.T) { + s := testAccountStore(t) + p := New("Alice") + p.GodBackup = p.Skills + p.Skills = map[SkillName]int{Hitpoints: 1000000} + p.GodMode = true + p.HP = 999 + expected := *p + expected.Skills = p.GodBackup + wantHP := expected.MaxHP() + + // Concurrent saves of an otherwise immutable player used to race while + // temporarily swapping Skills/HP. Saving must be observationally read-only. + var wg sync.WaitGroup + for i := 0; i < 2; i++ { + wg.Add(1) + go func() { + defer wg.Done() + if err := s.SaveCharacter(p); err != nil { + t.Error(err) + } + }() + } + wg.Wait() + if p.HP != 999 || p.Skills[Hitpoints] != 1000000 { + t.Fatal("save changed live god-mode state") + } + saved, err := s.LoadCharacter(p.Name) + if err != nil { + t.Fatal(err) + } + if saved.HP != wantHP || !reflect.DeepEqual(saved.Skills, p.GodBackup) { + t.Fatalf("saved god-mode stats instead of underlying character: %+v", saved) + } +} + +func TestAccountStoreIgnoresNonYAMLFiles(t *testing.T) { + s := testAccountStore(t) + for _, path := range []string{s.AccountPath("Alice"), s.CharPath("Alice")} { + for _, name := range []string{"Alice.yaml", ".save-stale", "Backup.yaml.bak"} { + if err := os.WriteFile(filepath.Join(filepath.Dir(path), name), nil, 0600); err != nil { + t.Fatal(err) + } + } + } + for _, list := range []func() ([]string, error){s.ListAccountNames, s.ListCharacterNames} { + got, err := list() + if err != nil || !reflect.DeepEqual(got, []string{"Alice"}) { + t.Fatalf("list = %v, %v", got, err) + } + } + for _, find := range []func(string) (string, error){s.FindAccount, s.FindCharacter} { + if got, err := find("alice"); err != nil || got != "Alice" { + t.Fatalf("case-insensitive lookup = %q, %v", got, err) + } + if _, err := find(".save-stale"); err == nil { + t.Fatal("temporary file treated as an account/character") + } + } +} |