From 396c3facb8287b9a34076cae8f3d7b300d366d99 Mon Sep 17 00:00:00 2001 From: historia <[not public]> Date: Tue, 15 Sep 2026 14:18:14 -0400 Subject: fix: atomic saves, casino RNG/config isolation, tick lifecycle --- internal/admin/api_rooms.go | 9 +++ internal/admin/casino_validate.go | 33 ++++++++ internal/admin/casino_validate_test.go | 52 +++++++++++++ internal/casino/config_validate.go | 119 ++++++++++++++++++++++++++++ internal/casino/config_validate_test.go | 124 ++++++++++++++++++++++++++++++ internal/casino/machine.go | 12 ++- internal/casino/machine_ownership_test.go | 24 ++++++ internal/casino/manager.go | 22 ++++-- internal/casino/slots.go | 7 +- internal/engine/lifecycle_test.go | 40 ++++++++++ internal/engine/tick.go | 16 +++- internal/game/casino.go | 14 +++- internal/persistence/atomic.go | 53 +++++++++++++ internal/persistence/atomic_test.go | 75 ++++++++++++++++++ internal/player/store.go | 39 +++++----- internal/player/store_test.go | 81 +++++++++++++++++++ internal/validate/casino_test.go | 29 +++++++ internal/validate/checks.go | 7 ++ 18 files changed, 719 insertions(+), 37 deletions(-) create mode 100644 internal/admin/casino_validate.go create mode 100644 internal/admin/casino_validate_test.go create mode 100644 internal/casino/config_validate.go create mode 100644 internal/casino/config_validate_test.go create mode 100644 internal/casino/machine_ownership_test.go create mode 100644 internal/engine/lifecycle_test.go create mode 100644 internal/persistence/atomic.go create mode 100644 internal/persistence/atomic_test.go create mode 100644 internal/player/store_test.go create mode 100644 internal/validate/casino_test.go (limited to 'internal') diff --git a/internal/admin/api_rooms.go b/internal/admin/api_rooms.go index 84c3348..2c251bb 100644 --- a/internal/admin/api_rooms.go +++ b/internal/admin/api_rooms.go @@ -45,6 +45,11 @@ func (s *AdminServer) handleRooms(w http.ResponseWriter, r *http.Request) { m = map[string]any{} } + if err := validateRoomCasinoMap(m); err != nil { + writeJSONError(w, "invalid casino configuration: "+err.Error(), http.StatusBadRequest) + return + } + var linkFrom *int var linkDir *string var linkOneWay bool @@ -272,6 +277,10 @@ func (s *AdminServer) handleRoomByID(w http.ResponseWriter, r *http.Request) { m = map[string]any{} } m["id"] = id + if err := validateRoomCasinoMap(m); err != nil { + writeJSONError(w, "invalid casino configuration: "+err.Error(), http.StatusBadRequest) + return + } newContent, writeErr := writeMapAsYAML(path, m) if writeErr != nil { diff --git a/internal/admin/casino_validate.go b/internal/admin/casino_validate.go new file mode 100644 index 0000000..5b6902c --- /dev/null +++ b/internal/admin/casino_validate.go @@ -0,0 +1,33 @@ +package admin + +import ( + "bytes" + + "gopkg.in/yaml.v3" + "thehouseoficarus/internal/casino" +) + +// Validate before any room/link writes. Only casino fields are decoded here; +// the room editor's unrelated fields retain their existing representation. +func validateRoomCasinoMap(m map[string]any) error { + fields := map[string]any{} + for _, key := range []string{"casino_tables", "casino_machines"} { + if value, ok := m[key]; ok { + fields[key] = value + } + } + data, err := yaml.Marshal(fields) + if err != nil { + return err + } + var cfg struct { + Tables []casino.TableConfig `yaml:"casino_tables"` + Machines []casino.MachineConfig `yaml:"casino_machines"` + } + decoder := yaml.NewDecoder(bytes.NewReader(data)) + decoder.KnownFields(true) + if err := decoder.Decode(&cfg); err != nil { + return err + } + return casino.ValidateRoomConfigs(cfg.Tables, cfg.Machines) +} diff --git a/internal/admin/casino_validate_test.go b/internal/admin/casino_validate_test.go new file mode 100644 index 0000000..a0f4451 --- /dev/null +++ b/internal/admin/casino_validate_test.go @@ -0,0 +1,52 @@ +package admin + +import ( + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "testing" + + "thehouseoficarus/internal/world" +) + +func TestRoomUpdateRejectsInvalidCasinoBeforeWriting(t *testing.T) { + dir := t.TempDir() + rooms := filepath.Join(dir, "rooms") + if err := os.MkdirAll(rooms, 0755); err != nil { + t.Fatal(err) + } + path := filepath.Join(rooms, "1.yaml") + original := "name: Original\n" + if err := os.WriteFile(path, []byte(original), 0644); err != nil { + t.Fatal(err) + } + s := &AdminServer{dataDir: dir, world: world.New(dir), undoStack: NewUndoStack(dir)} + for _, cfg := range []string{ + `{"id":"slots","game":"slots","payout":-1}`, + `{"id":"slots","game":"slots","spin_tickz":2}`, + } { + r := httptest.NewRequest(http.MethodPut, "/api/rooms/1", strings.NewReader(`{"casino_machines":[`+cfg+`]}`)) + w := httptest.NewRecorder() + s.handleRoomByID(w, r) + if w.Code != http.StatusBadRequest { + t.Fatalf("status %d: %s", w.Code, w.Body.String()) + } + data, err := os.ReadFile(path) + if err != nil || string(data) != original || s.undoStack.Info().CanUndo { + t.Fatal("invalid update changed room or undo history") + } + } +} + +func TestRoomCreateValidatesBeforeAllocatingOrLinking(t *testing.T) { + // No world/config needed: rejection must precede all mutation work. + s := &AdminServer{} + r := httptest.NewRequest(http.MethodPost, "/api/rooms", strings.NewReader(`{"casino_tables":[{"id":"cards","game":"typo"}],"link_from":1,"link_dir":"n"}`)) + w := httptest.NewRecorder() + s.handleRooms(w, r) + if w.Code != http.StatusBadRequest { + t.Fatalf("status %d: %s", w.Code, w.Body.String()) + } +} diff --git a/internal/casino/config_validate.go b/internal/casino/config_validate.go new file mode 100644 index 0000000..13229bc --- /dev/null +++ b/internal/casino/config_validate.go @@ -0,0 +1,119 @@ +package casino + +import ( + "errors" + "fmt" + "math" + "strings" +) + +// Validate checks raw configuration before Normalize supplies omitted (zero) +// defaults. Invalid explicit values must not silently become house rules. +func (c MachineConfig) Validate() error { + if err := validateIdentityAndBets(c.ID, c.MinBet, c.MaxBet); err != nil { + return err + } + if c.Game != GameSlots { + return fmt.Errorf("unsupported machine game %q", c.Game) + } + if c.Variant != "" && c.Variant != "huff_and_even_more_puff" { + return fmt.Errorf("unsupported slots variant %q", c.Variant) + } + if c.SpinTicks < 0 { + return fmt.Errorf("spin_ticks must not be negative") + } + if !finiteNonnegative(c.Payout) || c.Payout > 1 { + return fmt.Errorf("payout must be finite and between 0 and 1 (0 uses the default)") + } + return nil +} + +func (c TableConfig) Validate() error { + if err := validateIdentityAndBets(c.ID, c.MinBet, c.MaxBet); err != nil { + return err + } + if c.MaxPlayers < 0 || c.BettingTicks < 0 { + return fmt.Errorf("max_players and betting_ticks must not be negative") + } + switch c.Game { + case GameBlackjack: + b := c.Blackjack + if err := validateShoe(b.Decks, b.ShuffleAt); err != nil { + return err + } + if b.MaxSplitHands < 0 || b.ActionTicks < 0 { + return fmt.Errorf("max_split_hands and action_ticks must not be negative") + } + if !finiteNonnegative(b.BlackjackPayout) || !finiteNonnegative(b.InsurancePayout) { + return fmt.Errorf("blackjack_payout and insurance_payout must be finite and nonnegative") + } + case GameBaccarat: + b := c.Baccarat + if err := validateShoe(b.Decks, b.ShuffleAt); err != nil { + return err + } + if !finiteNonnegative(b.TiePayout) || !finiteNonnegative(b.BankerPayout) { + return fmt.Errorf("tie_payout and banker_payout must be finite and nonnegative") + } + default: + return fmt.Errorf("unsupported table game %q", c.Game) + } + return nil +} + +func validateIdentityAndBets(id string, minBet, maxBet int) error { + if id == "" || strings.TrimSpace(id) != id { + return fmt.Errorf("id must be nonempty with no surrounding whitespace") + } + if minBet < 0 || maxBet < 0 { + return fmt.Errorf("min_bet and max_bet must not be negative") + } + if maxBet > 0 && maxBet < max(1, minBet) { + return fmt.Errorf("max_bet must be at least min_bet") + } + return nil +} + +func finiteNonnegative(n float64) bool { + return !math.IsNaN(n) && !math.IsInf(n, 0) && n >= 0 +} + +func validateShoe(decks int, shuffleAt float64) error { + // A generous resource limit prevents malformed content from allocating an + // unbounded shoe. Zero retains the existing eight-deck default. + if decks < 0 || decks > 64 { + return fmt.Errorf("decks must be between 0 and 64 (0 uses the default)") + } + if !finiteNonnegative(shuffleAt) || shuffleAt >= 1 { + return fmt.Errorf("shuffle_at must be finite and in [0, 1)") + } + return nil +} + +// ValidateRoomConfigs checks the shared room-local ID namespace as well as +// individual definitions. Legacy slots under casino_tables remain supported. +func ValidateRoomConfigs(tables []TableConfig, machines []MachineConfig) error { + seen := make(map[string]bool) + var errs []error + check := func(kind, id string, err error) { + if seen[id] { + errs = append(errs, fmt.Errorf("duplicate casino id %q", id)) + } + seen[id] = true + if err != nil { + errs = append(errs, fmt.Errorf("%s %q: %w", kind, id, err)) + } + } + for _, c := range tables { + if c.Game == GameSlots { + legacy := MachineConfig{ID: c.ID, Game: GameSlots, MinBet: c.MinBet, MaxBet: c.MaxBet} + check("legacy slots", c.ID, legacy.Validate()) + } else { + check("table", c.ID, c.Validate()) + } + } + for _, c := range machines { + check("machine", c.ID, c.Validate()) + } + return errors.Join(errs...) +} diff --git a/internal/casino/config_validate_test.go b/internal/casino/config_validate_test.go new file mode 100644 index 0000000..f0f87f5 --- /dev/null +++ b/internal/casino/config_validate_test.go @@ -0,0 +1,124 @@ +package casino + +import ( + "fmt" + "math" + "reflect" + "sync" + "testing" +) + +func TestMachineConfigValidation(t *testing.T) { + valid := MachineConfig{ID: "slots", Game: GameSlots} + if err := valid.Validate(); err != nil { + t.Fatal(err) + } + for name, change := range map[string]func(*MachineConfig){ + "missing ID": func(c *MachineConfig) { c.ID = "" }, + "ID whitespace": func(c *MachineConfig) { c.ID = " slots " }, + "unknown game": func(c *MachineConfig) { c.Game = "roulette" }, + "unknown variant": func(c *MachineConfig) { c.Variant = "typo" }, + "negative bet": func(c *MachineConfig) { c.MinBet = -1 }, + "negative max": func(c *MachineConfig) { c.MaxBet = -1 }, + "inverted bets": func(c *MachineConfig) { c.MinBet, c.MaxBet = 10, 5 }, + "negative timer": func(c *MachineConfig) { c.SpinTicks = -1 }, + "negative payout": func(c *MachineConfig) { c.Payout = -1 }, + "excess payout": func(c *MachineConfig) { c.Payout = 1.01 }, + "NaN payout": func(c *MachineConfig) { c.Payout = math.NaN() }, + "infinite payout": func(c *MachineConfig) { c.Payout = math.Inf(1) }, + } { + t.Run(name, func(t *testing.T) { + c := valid + change(&c) + if err := c.Validate(); err == nil { + t.Fatalf("accepted invalid config: %+v", c) + } + m := NewManager(1) + if m.EnsureMachine(1, c) != nil { + t.Fatal("invalid machine instantiated") + } + if events := m.JoinMachine(1, c, "alice"); len(events) != 1 || !contains(events[0].Message, "not available") { + t.Fatalf("invalid machine join: %+v", events) + } + }) + } +} + +func TestTableConfigValidation(t *testing.T) { + for _, game := range []GameName{GameBlackjack, GameBaccarat} { + if err := (TableConfig{ID: "table", Game: game}).Validate(); err != nil { + t.Fatal(err) + } + } + for name, c := range map[string]TableConfig{ + "unknown game": {ID: "table", Game: "roulette"}, + "missing ID": {Game: GameBlackjack}, + "negative timer": {ID: "table", Game: GameBlackjack, BettingTicks: -1}, + "negative seats": {ID: "table", Game: GameBlackjack, MaxPlayers: -1}, + "inverted bets": {ID: "table", Game: GameBlackjack, MinBet: 10, MaxBet: 5}, + "negative decks": {ID: "table", Game: GameBlackjack, Blackjack: BlackjackConfig{Decks: -1}}, + "huge shoe": {ID: "table", Game: GameBaccarat, Baccarat: BaccaratConfig{Decks: 1000000000}}, + "NaN shuffle": {ID: "table", Game: GameBlackjack, Blackjack: BlackjackConfig{ShuffleAt: math.NaN()}}, + "shuffle endpoint": {ID: "table", Game: GameBaccarat, Baccarat: BaccaratConfig{ShuffleAt: 1}}, + "infinite payout": {ID: "table", Game: GameBlackjack, Blackjack: BlackjackConfig{BlackjackPayout: math.Inf(1)}}, + "negative insurance": {ID: "table", Game: GameBlackjack, Blackjack: BlackjackConfig{InsurancePayout: -1}}, + "NaN tie": {ID: "table", Game: GameBaccarat, Baccarat: BaccaratConfig{TiePayout: math.NaN()}}, + "negative banker": {ID: "table", Game: GameBaccarat, Baccarat: BaccaratConfig{BankerPayout: -1}}, + "negative splits": {ID: "table", Game: GameBlackjack, Blackjack: BlackjackConfig{MaxSplitHands: -1}}, + "negative action timer": {ID: "table", Game: GameBlackjack, Blackjack: BlackjackConfig{ActionTicks: -1}}, + } { + t.Run(name, func(t *testing.T) { + if c.Validate() == nil || NewManager(1).EnsureTable(1, c) != nil { + t.Fatalf("accepted invalid table: %+v", c) + } + }) + } +} + +func TestRoomCasinoIDsAndLegacySlots(t *testing.T) { + tables := []TableConfig{{ID: "slots", Game: GameSlots}} + machines := []MachineConfig{{ID: "slots", Game: GameSlots}} + if err := ValidateRoomConfigs(tables, nil); err != nil { + t.Fatalf("legacy slots rejected: %v", err) + } + if ValidateRoomConfigs(tables, machines) == nil || ValidateRoomConfigs(nil, append(machines, machines...)) == nil { + t.Fatal("duplicate casino IDs accepted") + } +} + +func TestManagerGamesHaveIndependentRandomStreams(t *testing.T) { + m := NewManager(123) + var wg sync.WaitGroup + for i := 0; i < 8; i++ { + wg.Add(1) + go func(i int) { + defer wg.Done() + // Creation/shuffling can overlap spins on already-created games. + id := fmt.Sprintf("slots%d", i) + machine := m.EnsureMachine(1, MachineConfig{ID: id, Game: GameSlots}) + m.EnsureTable(1, TableConfig{ID: fmt.Sprintf("cards%d", i), Game: GameBlackjack}) + machine.join(id) + wallet := &testWallet{credits: 100} + for j := 0; j < 50; j++ { + machine.setBet(id, 1, wallet) + for machine.isBusy(id) { + machine.tick() + } + } + }(i) + } + wg.Wait() +} + +func TestScaleToRTPDoesNotChangeSourceProfile(t *testing.T) { + source := DefaultHuffAndPuffProfile() + want := DefaultHuffAndPuffProfile() + first := ScaleToRTP(source, .9) + second := ScaleToRTP(source, .97) + if !reflect.DeepEqual(source, want) { + t.Fatal("scaling mutated the source profile") + } + if math.Abs(CalculateRTP(first)-.9) > 1e-10 || math.Abs(CalculateRTP(second)-.97) > 1e-10 { + t.Fatal("profiles share mutable paytables") + } +} diff --git a/internal/casino/machine.go b/internal/casino/machine.go index 15c612d..64dcfd4 100644 --- a/internal/casino/machine.go +++ b/internal/casino/machine.go @@ -63,7 +63,7 @@ func (m *Machine) setBet(name string, amount int, wallet Wallet) []Event { m.mu.Lock() defer m.mu.Unlock() if !m.owns(name) { - return m.private("You aren't playing at a slot machine.") + return m.privateTo(name, "You aren't playing at a slot machine.") } if m.autoBet { return m.private("Autobet is active. Type \"stop\" to stop playing.") @@ -83,7 +83,7 @@ func (m *Machine) start(name string, wallet Wallet) []Event { m.mu.Lock() defer m.mu.Unlock() if !m.owns(name) { - return m.private("You aren't playing at a slot machine.") + return m.privateTo(name, "You aren't playing at a slot machine.") } if m.autoBet { return m.private("Autobet is active. Type \"stop\" to stop playing.") @@ -116,7 +116,7 @@ func (m *Machine) enableAutoBet(name string, wallet Wallet) []Event { m.mu.Lock() defer m.mu.Unlock() if !m.owns(name) { - return m.private("You aren't playing at a slot machine.") + return m.privateTo(name, "You aren't playing at a slot machine.") } if m.autoBet { return m.private("Autobet is already active.") @@ -319,7 +319,11 @@ func (m *Machine) isBusy(name string) bool { func (m *Machine) owns(name string) bool { return m.player != nil && m.player.Name == name } func (m *Machine) private(message string) []Event { - return []Event{{RoomID: m.RoomID, TableID: m.Config.ID, Type: EventResult, PrivateTo: m.player.Name, Message: message, Color: "casino_action"}} + return m.privateTo(m.player.Name, message) +} + +func (m *Machine) privateTo(name, message string) []Event { + return []Event{{RoomID: m.RoomID, TableID: m.Config.ID, Type: EventResult, PrivateTo: name, Message: message, Color: "casino_action"}} } func (m *Machine) public(message string) []Event { diff --git a/internal/casino/machine_ownership_test.go b/internal/casino/machine_ownership_test.go new file mode 100644 index 0000000..47e068a --- /dev/null +++ b/internal/casino/machine_ownership_test.go @@ -0,0 +1,24 @@ +package casino + +import "testing" + +func TestMachineRejectsStaleAndNonOwnerCommands(t *testing.T) { + m := NewManager(1) + cfg := MachineConfig{ID: "slots", Game: GameSlots} + m.JoinMachine(1, cfg, "alice") + m.MarkDisconnected("alice") + for _, occupied := range []bool{false, true} { + if occupied { + m.JoinMachine(1, cfg, "bob") + } + for _, events := range [][]Event{ + m.SetMachineBet(1, "slots", "alice", 10, nil), + m.StartMachine(1, "slots", "alice", nil), + m.EnableMachineAutoBet(1, "slots", "alice", nil), + } { + if len(events) != 1 || events[0].PrivateTo != "alice" || events[0].Public { + t.Fatalf("rejection misrouted (occupied=%v): %+v", occupied, events) + } + } + } +} diff --git a/internal/casino/manager.go b/internal/casino/manager.go index a9bab92..38b754f 100644 --- a/internal/casino/manager.go +++ b/internal/casino/manager.go @@ -26,7 +26,11 @@ func tableKey(roomID int, tableID string) string { return fmt.Sprintf("%d:%s", roomID, tableID) } +// EnsureMachine returns nil for an invalid or unsupported configuration. func (m *Manager) EnsureMachine(roomID int, cfg MachineConfig) *Machine { + if cfg.Validate() != nil { + return nil + } cfg = cfg.Normalize() key := tableKey(roomID, cfg.ID) m.mu.Lock() @@ -34,14 +38,18 @@ func (m *Manager) EnsureMachine(roomID int, cfg MachineConfig) *Machine { if machine := m.machines[key]; machine != nil { return machine } - machine := newMachine(roomID, cfg, m.rng) + // Only the manager consumes its RNG, under m.mu. Each game owns an + // independent stream protected by that game's lock. + machine := newMachine(roomID, cfg, rand.New(rand.NewSource(m.rng.Int63()))) m.machines[key] = machine return machine } -// EnsureTable returns the table for cfg, or nil when cfg.Game has no -// multiplayer table implementation. +// EnsureTable returns nil for an invalid or unsupported configuration. func (m *Manager) EnsureTable(roomID int, cfg TableConfig) *Table { + if cfg.Validate() != nil { + return nil + } cfg = cfg.Normalize() key := tableKey(roomID, cfg.ID) m.mu.Lock() @@ -49,7 +57,7 @@ func (m *Manager) EnsureTable(roomID int, cfg TableConfig) *Table { if table := m.tables[key]; table != nil { return table } - table := newTable(roomID, cfg, m.rng) + table := newTable(roomID, cfg, rand.New(rand.NewSource(m.rng.Int63()))) if table == nil { return nil } @@ -122,7 +130,11 @@ func (m *Manager) withMachine(roomID int, machineID string, fn func(*Machine) [] } func (m *Manager) JoinMachine(roomID int, cfg MachineConfig, name string) []Event { - return m.EnsureMachine(roomID, cfg).join(name) + machine := m.EnsureMachine(roomID, cfg) + if machine == nil { + return []Event{{RoomID: roomID, TableID: cfg.ID, Type: EventResult, PrivateTo: name, Message: "That game is not available here."}} + } + return machine.join(name) } func (m *Manager) SetMachineBet(roomID int, machineID, name string, amount int, wallet Wallet) []Event { diff --git a/internal/casino/slots.go b/internal/casino/slots.go index aba8f91..c519293 100644 --- a/internal/casino/slots.go +++ b/internal/casino/slots.go @@ -234,10 +234,15 @@ func ScaleToRTP(profile SlotProfile, target float64) SlotProfile { return profile } scale := target / actual + // SlotProfile is a value API, but its slice and paytable maps are shared + // references. Clone both so tuning one machine cannot retune another. + profile.Symbols = append([]SlotSymbol(nil), profile.Symbols...) for i := range profile.Symbols { + payouts := make(map[int]float64, len(profile.Symbols[i].Payout)) for length, payout := range profile.Symbols[i].Payout { - profile.Symbols[i].Payout[length] = payout * scale + payouts[length] = payout * scale } + profile.Symbols[i].Payout = payouts } profile.TargetRTP = target return profile diff --git a/internal/engine/lifecycle_test.go b/internal/engine/lifecycle_test.go new file mode 100644 index 0000000..8cea97f --- /dev/null +++ b/internal/engine/lifecycle_test.go @@ -0,0 +1,40 @@ +package engine + +import ( + "sync" + "testing" +) + +func TestConcurrentTicksAreSerialized(t *testing.T) { + e := New() + count := 0 + e.Subscribe(1, func() bool { + count++ // safe only when callbacks have a single owner + return true + }) + var wg sync.WaitGroup + for i := 0; i < 8; i++ { + wg.Add(1) + go func() { + defer wg.Done() + for j := 0; j < 100; j++ { + e.Tick() + } + }() + } + wg.Wait() + if count != 800 { + t.Fatalf("lost ticks: got %d, want 800", count) + } +} + +func TestStopIsIdempotentAndRestartable(t *testing.T) { + e := New() + e.Stop() + defer e.Stop() + for i := 0; i < 100; i++ { + e.Start(60000) + e.Stop() + e.Stop() + } +} diff --git a/internal/engine/tick.go b/internal/engine/tick.go index 7b9666b..00a9c60 100644 --- a/internal/engine/tick.go +++ b/internal/engine/tick.go @@ -17,6 +17,7 @@ type subscriber struct { } type Engine struct { + tickMu sync.Mutex // serializes manual ticks and successive ticker runs mu sync.Mutex subscribers map[uint64]*subscriber nextID uint64 @@ -61,19 +62,24 @@ func (e *Engine) Start(tickLengthMs int) { } e.ticker = time.NewTicker(time.Duration(tickLengthMs) * time.Millisecond) e.stopCh = make(chan struct{}) + // Capture this run's resources. A quick Stop/Start must not make the + // old goroutine consume the replacement ticker or stop channel. + ticker, stopCh := e.ticker, e.stopCh go func() { for { select { - case <-e.ticker.C: + case <-ticker.C: e.processTick() - case <-e.stopCh: + case <-stopCh: return } } }() } +// Stop requests that the ticker loop exit. It is idempotent, but does not +// wait for an in-flight callback; it is not a graceful-shutdown save barrier. func (e *Engine) Stop() { e.mu.Lock() defer e.mu.Unlock() @@ -83,14 +89,18 @@ func (e *Engine) Stop() { e.running = false if e.stopCh != nil { close(e.stopCh) + e.stopCh = nil } } // Tick fires every due subscriber synchronously and returns. Subscribers -// are invoked in ascending subscription-ID (registration) order. +// are invoked in ascending subscription-ID (registration) order. Tick calls +// are serialized; a callback must not call Tick recursively. func (e *Engine) Tick() { e.processTick() } func (e *Engine) processTick() { + e.tickMu.Lock() + defer e.tickMu.Unlock() e.mu.Lock() snapshot := make(map[uint64]*subscriber, len(e.subscribers)) for id, sub := range e.subscribers { diff --git a/internal/game/casino.go b/internal/game/casino.go index 2e80bf3..eb6fa4a 100644 --- a/internal/game/casino.go +++ b/internal/game/casino.go @@ -62,6 +62,9 @@ func (g *Game) casinoTable(roomID int, gameName string) (*casino.Table, bool) { if err != nil { return nil, false } + if casino.ValidateRoomConfigs(room.CasinoTables, room.CasinoMachines) != nil { + return nil, false + } for _, cfg := range room.CasinoTables { if strings.EqualFold(string(cfg.Game), gameName) { table := g.Casino.EnsureTable(roomID, cfg) @@ -79,9 +82,13 @@ func (g *Game) casinoMachine(roomID int, gameName string) (*casino.Machine, bool if err != nil { return nil, false } + if casino.ValidateRoomConfigs(room.CasinoTables, room.CasinoMachines) != nil { + return nil, false + } for _, cfg := range room.CasinoMachines { if strings.EqualFold(string(cfg.Game), gameName) || strings.EqualFold(cfg.Variant, gameName) { - return g.Casino.EnsureMachine(roomID, cfg), true + machine := g.Casino.EnsureMachine(roomID, cfg) + return machine, machine != nil } } // Migrate older room definitions that declared slots under casino_tables; @@ -89,9 +96,10 @@ func (g *Game) casinoMachine(roomID int, gameName string) (*casino.Machine, bool if strings.EqualFold(gameName, "slots") { for _, cfg := range room.CasinoTables { if strings.EqualFold(string(cfg.Game), "slots") { - return g.Casino.EnsureMachine(roomID, casino.MachineConfig{ + machine := g.Casino.EnsureMachine(roomID, casino.MachineConfig{ ID: cfg.ID, Game: casino.GameSlots, MinBet: cfg.MinBet, MaxBet: cfg.MaxBet, - }), true + }) + return machine, machine != nil } } } diff --git a/internal/persistence/atomic.go b/internal/persistence/atomic.go new file mode 100644 index 0000000..a3cc06a --- /dev/null +++ b/internal/persistence/atomic.go @@ -0,0 +1,53 @@ +// Package persistence provides crash-resistant single-file replacement. +// It does not serialize callers or make multi-file operations transactional. +package persistence + +import ( + "fmt" + "os" + "path/filepath" +) + +// WriteFile writes and syncs a temporary file in the destination directory, +// atomically renames it over path, then syncs the directory. Existing file +// permissions are preserved; perm is used for new files. Callers must own the +// data being serialized and coordinate competing read-modify-write operations. +// A directory-sync error means replacement occurred but durability is uncertain. +func WriteFile(path string, data []byte, perm os.FileMode) error { + if info, err := os.Stat(path); err == nil { + perm = info.Mode().Perm() + } else if !os.IsNotExist(err) { + return err + } + dir := filepath.Dir(path) + f, err := os.CreateTemp(dir, ".save-*") + if err != nil { + return err + } + defer os.Remove(f.Name()) + defer f.Close() + if _, err := f.Write(data); err != nil { + return err + } + if err := f.Chmod(perm); err != nil { + return err + } + if err := f.Sync(); err != nil { + return err + } + if err := f.Close(); err != nil { + return err + } + if err := os.Rename(f.Name(), path); err != nil { + return err + } + d, err := os.Open(dir) + if err != nil { + return fmt.Errorf("replacement completed; open directory for sync: %w", err) + } + defer d.Close() + if err := d.Sync(); err != nil { + return fmt.Errorf("replacement completed; sync directory: %w", err) + } + return nil +} diff --git a/internal/persistence/atomic_test.go b/internal/persistence/atomic_test.go new file mode 100644 index 0000000..05710f1 --- /dev/null +++ b/internal/persistence/atomic_test.go @@ -0,0 +1,75 @@ +package persistence + +import ( + "bytes" + "os" + "path/filepath" + "sync" + "testing" +) + +func TestWriteFileReplacesAndPreservesPermissions(t *testing.T) { + path := filepath.Join(t.TempDir(), "save.yaml") + if err := WriteFile(path, []byte("old"), 0600); err != nil { + t.Fatal(err) + } + if err := WriteFile(path, []byte("new"), 0644); err != nil { + t.Fatal(err) + } + got, err := os.ReadFile(path) + if err != nil || string(got) != "new" { + t.Fatalf("read: %q, %v", got, err) + } + info, err := os.Stat(path) + if err != nil || info.Mode().Perm() != 0600 { + t.Fatalf("permissions: %v, %v", info, err) + } + entries, err := os.ReadDir(filepath.Dir(path)) + if err != nil || len(entries) != 1 { + t.Fatalf("temporary files left behind: %v, %v", entries, err) + } +} + +func TestFailedRenameCleansTempFile(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "destination") + if err := os.Mkdir(path, 0755); err != nil { + t.Fatal(err) + } + if err := WriteFile(path, []byte("new"), 0600); err == nil { + t.Fatal("expected rename over directory to fail") + } + entries, err := os.ReadDir(dir) + if err != nil || len(entries) != 1 || !entries[0].IsDir() { + t.Fatalf("destination damaged or temporary file leaked: %v, %v", entries, err) + } +} + +func TestReadersSeeOnlyCompleteSaves(t *testing.T) { + path := filepath.Join(t.TempDir(), "save.yaml") + a, b := bytes.Repeat([]byte("a"), 8192), bytes.Repeat([]byte("b"), 16384) + if err := WriteFile(path, a, 0600); err != nil { + t.Fatal(err) + } + var wg sync.WaitGroup + for _, data := range [][]byte{a, b} { + wg.Add(1) + go func(data []byte) { + defer wg.Done() + for i := 0; i < 30; i++ { + if err := WriteFile(path, data, 0600); err != nil { + t.Error(err) + return + } + } + }(data) + } + for i := 0; i < 300; i++ { + got, err := os.ReadFile(path) + if err != nil || (!bytes.Equal(got, a) && !bytes.Equal(got, b)) { + t.Errorf("reader saw partial or missing save: length=%d, err=%v", len(got), err) + break + } + } + wg.Wait() +} 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") + } + } +} diff --git a/internal/validate/casino_test.go b/internal/validate/casino_test.go new file mode 100644 index 0000000..9eb94db --- /dev/null +++ b/internal/validate/casino_test.go @@ -0,0 +1,29 @@ +package validate + +import ( + "path/filepath" + "testing" +) + +func TestRoomCasinoValidation(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "rooms", "1.yaml"), `name: Casino +casino_tables: + - id: cards + game: blackjack + min_bet: 10 + max_bet: 5 +casino_machines: + - id: slots + game: slots + payout: .nan + - id: slots + game: slots +`) + issues := validateRooms(newSource(dir)) + for _, message := range []string{"Room 1", "max_bet", "payout", "duplicate casino id"} { + if !containsMsg(issues, message) { + t.Errorf("missing %q in %+v", message, issues) + } + } +} diff --git a/internal/validate/checks.go b/internal/validate/checks.go index 527b558..344df3a 100644 --- a/internal/validate/checks.go +++ b/internal/validate/checks.go @@ -8,6 +8,7 @@ import ( "strings" "thehouseoficarus/internal/behavior" + "thehouseoficarus/internal/casino" "thehouseoficarus/internal/color" "thehouseoficarus/internal/combat" "thehouseoficarus/internal/world" @@ -31,6 +32,12 @@ func validateRooms(s Source) []Issue { }) continue } + if err := casino.ValidateRoomConfigs(room.CasinoTables, room.CasinoMachines); err != nil { + issues = append(issues, Issue{ + Level: "ERROR", Type: "casino", + Message: fmt.Sprintf("Room %d: %v", id, err), + }) + } if room.Name == "" { issues = append(issues, Issue{ Level: "WARN", -- cgit v1.2.3