From 0b0b25222b3131ada79ff78c28eb8bf07d977deb Mon Sep 17 00:00:00 2001 From: historia <[not public]> Date: Tue, 15 Sep 2026 15:35:52 -0400 Subject: feat: single-owner game state, transactional lifecycle, durable money on server stop --- building_guide/concurrency.md | 122 ++++++++++ building_guide/foundation_review.md | 209 ----------------- cmd/thoi/main.go | 36 ++- internal/admin/api_helpers.go | 19 +- internal/admin/api_room_insert_remove.go | 5 + internal/admin/api_room_rename_test.go | 149 ++++++++++++ internal/admin/api_rooms.go | 149 ++++++++---- internal/admin/api_rooms_bulk_delete.go | 3 + internal/admin/server.go | 11 +- internal/admin/undo.go | 178 +++++++++++---- internal/admin/yaml_util.go | 15 +- internal/casino/baccarat.go | 45 +++- internal/casino/blackjack.go | 56 ++++- internal/casino/credits.go | 84 +++++++ internal/casino/credits_test.go | 74 ++++++ internal/casino/machine.go | 48 +++- internal/casino/machine_test.go | 4 +- internal/casino/manager.go | 122 ++++++++++ internal/casino/manager_test.go | 13 +- internal/casino/reconfig_test.go | 84 +++++++ internal/casino/table.go | 29 +++ internal/casino/types.go | 19 +- internal/engine/tick.go | 10 + internal/game/account_txn.go | 380 +++++++++++++++++++++++++++++++ internal/game/baseline_race_test.go | 132 +++++++++++ internal/game/casino.go | 67 +++++- internal/game/casino_test.go | 3 + internal/game/cmd_quit.go | 2 +- internal/game/cmd_registry.go | 10 +- internal/game/cmd_room_remove_test.go | 3 + internal/game/cmd_shutdown.go | 115 +++++----- internal/game/combat_attack.go | 4 +- internal/game/combat_mob.go | 4 +- internal/game/core_course.go | 6 + internal/game/core_login_account.go | 56 +++-- internal/game/core_login_char.go | 132 ++++++----- internal/game/economy.go | 289 +++++++++++++++++++++++ internal/game/economy_shutdown_test.go | 263 +++++++++++++++++++++ internal/game/exit_migrate.go | 39 +++- internal/game/game.go | 217 ++++++++++++++++-- internal/game/harness_test.go | 266 ++++++++++++++++++++++ internal/game/lifecycle_txn_test.go | 160 +++++++++++++ internal/game/migrate_rooms_test.go | 115 ++++++++++ internal/game/owner.go | 119 ++++++++++ internal/game/owner_concurrency_test.go | 154 +++++++++++++ internal/game/session_registry.go | 19 ++ internal/game/sys_combat.go | 2 +- internal/game/sys_escape_test.go | 3 + internal/game/sys_safespot.go | 2 +- internal/net/conn.go | 10 + internal/net/server.go | 202 ++++++++++++++-- internal/net/server_test.go | 11 +- internal/net/web.go | 1 + internal/net/wsconn.go | 8 + internal/persistence/txn.go | 335 +++++++++++++++++++++++++++ internal/persistence/txn_test.go | 186 +++++++++++++++ internal/player/player.go | 5 + internal/player/store.go | 32 ++- internal/world/mob.go | 24 ++ internal/world/world.go | 59 +++++ 60 files changed, 4356 insertions(+), 563 deletions(-) create mode 100644 building_guide/concurrency.md delete mode 100644 building_guide/foundation_review.md create mode 100644 internal/admin/api_room_rename_test.go create mode 100644 internal/casino/credits.go create mode 100644 internal/casino/credits_test.go create mode 100644 internal/casino/reconfig_test.go create mode 100644 internal/game/account_txn.go create mode 100644 internal/game/baseline_race_test.go create mode 100644 internal/game/economy.go create mode 100644 internal/game/economy_shutdown_test.go create mode 100644 internal/game/harness_test.go create mode 100644 internal/game/lifecycle_txn_test.go create mode 100644 internal/game/migrate_rooms_test.go create mode 100644 internal/game/owner.go create mode 100644 internal/game/owner_concurrency_test.go create mode 100644 internal/game/session_registry.go create mode 100644 internal/persistence/txn.go create mode 100644 internal/persistence/txn_test.go diff --git a/building_guide/concurrency.md b/building_guide/concurrency.md new file mode 100644 index 0000000..1a61670 --- /dev/null +++ b/building_guide/concurrency.md @@ -0,0 +1,122 @@ +# Concurrency & durability + +This guide documents the runtime architecture decisions that keep live game +state safe: the single-owner loop, the durable money journal, the account +lifecycle transactions, and the graceful-shutdown sequence. + +## Single-owner game state + +One goroutine — the **owner loop** (`internal/game/owner.go`) — executes all +mutation of live game state: session input handlers, connect/disconnect +transitions, every tick callback (the master per-tick system sequence and all +dynamically registered callbacks), and admin mutations. Network and admin +goroutines submit tasks; they never touch live players, sessions, Hub +membership, or game runtime maps directly. + +Rules: + +- `Game.RunOnOwner(fn)` (and `Game.HandleSessionOnOwner` for session input) + block the caller until the closure has executed on the owner. Connection + goroutines submit one task per input line, giving per-client backpressure + without ever blocking another client. +- Code already running on the owner must NOT call `Game.RunOnOwner` — it + would deadlock waiting for itself. Owner-executed code calls functions + directly. +- Task execution is FIFO, so tick callbacks serialize against session input + and admin mutations in submission order. Tick callbacks still run in + engine registration-ID order (see tick_order.md); the owner wrapper only + moves their execution onto the owner goroutine. +- Slow clients cannot stall the world: `Session` writes are queued + (`net.Session.StartWriter` puts a transport session into queued mode) and a + dedicated per-session writer goroutine performs the network I/O with + per-message write deadlines. A full queue disconnects only that client. +- Saves run on the owner and serialize a value snapshot; concurrent + read-modify-write cycles of the same files cannot happen. + +Unit tests construct Games without an owner; such Games run submitted tasks +inline on the caller's goroutine (legacy behavior), which is fine for +synthetic tests but production Games always have the owner (started by +`game.New`). + +## Durable money (casino wallet) + +Every wager and payout is journaled before the balance mutation +(`internal/game/economy.go`, `data/economy/journal.jsonl`): + +1. A durable journal record with balance checkpoints (before/after) is + appended and fsynced. +2. The in-memory balance mutates. +3. The character save makes the balance durable. +4. A settlement marker closes the record. + +A failed save reverts the in-memory mutation and voids the record — the +game must not proceed on a stake that could not be made durable, and the +player is told the house could not process the transaction (distinct from an +insufficient-funds message). Payouts keep their record open when the save +fails; startup recovery re-pays the owed settlement. + +At startup (`game.New`), open records are reduced (latest line per record ID +wins) and compensated: + +- open **wager** with the file at the pre-debit balance → void (nothing was + charged); +- open **wager** with the file at the post-debit balance → the stake is + **refunded** (the round state was lost in the crash); +- open **payout** with the file at the pre-payout balance → the payout is + **re-applied** (the player is owed it); +- otherwise the record settles, or is surfaced for operator inspection when + the balance is ambiguous. + +Currency arithmetic is checked (`internal/casino/credits.go`): explicit +`MaxCredits` cap, bounded `DefaultMaxBet` default (the explicit unlimited +sentinel is still honored), floor conversion for payout multipliers, and +overflow rejected instead of wrapping. + +## Account lifecycle transactions + +Account and character lifecycle operations (create, rename, delete, purge) +are single journal transactions (`internal/persistence/txn.go`): a journal +entry with before-images is written durably BEFORE any file changes, the +operations apply, and the journal is removed at the commit point. Any +failure rolls back every applied operation; a crash mid-commit is rolled +back at the next startup by `persistence.Recover`. + +Characters carry a stable UUID (`player.Player.ID`) and owner field +(`player.Player.Account`), assigned at creation and backfilled at first +startup. Cached membership authorizes by the character file's owner, not by +its renameable filename, so a stale cached name can never authorize +connecting, renaming, or deleting a character reassigned to another account. + +After any lifecycle mutation, `Game.publishAccountUpdate` republishes the +account state to every authenticated session of that account — including +sessions sitting in menus — resetting stale pending selections. The online +check and the file mutation run as one owner step, closing the +check-then-rename gap. + +## Graceful shutdown + +Signals (SIGINT/SIGTERM) and the in-game `shutdown` command converge on +`Game.RequestShutdown`, which runs the sequence exactly once: + +1. **Admissions stop:** session input and disconnect forfeit/save callbacks + become no-ops (server-initiated shutdown is not a player disconnect). +2. **Tick stop and join:** `Engine.StopAndWait` stops the ticker and waits + for the in-flight callback to finish. +3. **Drain and persist (on the owner):** remaining queued commands execute, + outstanding wagers are refunded (`Casino.RefundAll`), and every online + character is saved. Save failures are logged for the operator. +4. **Owner stop:** the owner loop drains and stops. +5. **Transports close:** HTTP(S) servers drain, listeners close, and + sessions flush their outbound queues before their sockets drop. + `ListenAndServe` returns and main exits normally — no `os.Exit`. + +## Room-ID migration + +Live room-ID changes (admin rename, swapid) execute as one mutation plan: +the renamed room's file (id field, filename, self-loop references) plus every +other room whose references moved, applied as a single journal transaction, +followed by the full runtime migration (`Game.MigrateRoomIDs`): player +state, Hub room buckets, casino room keys, world room-keyed state (ground +items, seeded markers, object states), and live mob instances. Undo/redo +store the whole plan (all before/after images plus the idMap) and apply the +inverse/forward runtime migration through the same wired callback. diff --git a/building_guide/foundation_review.md b/building_guide/foundation_review.md deleted file mode 100644 index b0afb88..0000000 --- a/building_guide/foundation_review.md +++ /dev/null @@ -1,209 +0,0 @@ -# Foundation review - -## Verdict - -The package boundaries are useful foundations: casino game logic is separate -from game presentation, multiplayer games share a small `tableGame` interface, -and tick system order has an explicit testable contract. Keep those boundaries; -a wholesale rewrite or a larger plugin framework is not needed yet. - -However, this is **not yet a safe foundation for expanding persistent casino -activity or live world editing**. Ownership and transaction boundaries matter -more than further extraction of command files. - -This review inspected session/tick entry points, account lifecycle and storage, -casino configuration/runtime/wallet integration, shutdown, and room rename/undo. -It is not a full audit of every combat, crafting, networking, or admin endpoint. - -## Fixed in this pass - -- **Account/character single-file saves:** replace direct truncating writes with - a same-directory temporary file, file sync, atomic rename, and directory sync - (`internal/persistence/atomic.go`, `internal/player/store.go`). Existing modes - are preserved; new player/account files use 0600. Account/character discovery - ignores non-YAML files, including leftover temporary files. This does not make - multi-file operations or concurrent read-modify-write cycles transactional. -- **Save-time live mutation:** god-mode serialization uses a value snapshot - instead of temporarily changing the live player's skills and HP. Maps still - require caller ownership during serialization; this is not a deep snapshot. -- **Casino RNG ownership:** the manager creates independent random streams for - each game under its own lock. Previously, different table/machine locks - protected the same `rand.Rand`, permitting concurrent access to its state. -- **Slot profile isolation:** `ScaleToRTP` copies the symbols/paytables it changes, - so scaling a reused profile no longer retunes other machines or its source. -- **Stale machine commands:** betting, spinning, or enabling autobet after a - disconnect rejects the requesting player without dereferencing a missing - occupant or messaging a replacement occupant. -- **Casino configuration:** validate raw values before defaulting; reject - invalid game/variant/ID/range/finite-value configurations; detect room-local - duplicate IDs. The room create/update API checks before writes, startup - validation reports errors, and runtime lookup rejects invalid definitions. - Legacy slots table entries remain supported. See `rooms.md` for bounds. -- **Tick lifecycle:** repeated `Stop` calls no longer close a closed channel; - ticker goroutines retain their own run's resources across quick restarts; - concurrent tick executions are serialized. `Stop` still does not wait for an - in-flight callback and is explicitly not a final-save barrier. - -## Remaining work, in recommended order - -### 1. High: make live game/session state single-owner - -Evidence: `internal/net/server.go` invokes `Game.HandleSession` from each -connection goroutine; `internal/engine/tick.go` invokes game callbacks from the -ticker goroutine. Disconnect callbacks also mutate players/game maps, while -admin callbacks such as `Game.RewriteRoomIDs` mutate online players directly. - -`Hub.AllSessions` copies a list of pointers, not immutable session snapshots. -Store/map locks similarly do not grant ownership over all pointed-to state. -The casino wallet mutates player inventory and credits under a casino lock, -which does not exclude other player commands or saves. Serializing tick -callbacks alone does not solve these cross-entry-point races. - -Recommended design: - -1. One owner loop executes session input, connect/disconnect transitions, all - tick callbacks (including dynamically registered ones), and admin mutations. -2. Network/admin goroutines submit bounded requests; they do not mutate live - player/session/world objects. Admin reads consume snapshots. -3. Preserve the documented tick order. Do not wrap only `RunTickSystems` and - assume dynamic callbacks or instant commands are covered. -4. Add bounded outbound queues/write deadlines before concentrating execution - in one owner: current session writes perform synchronous network I/O, so one - slow client could otherwise stall the entire world. -5. Put saves behind an explicit owned snapshot boundary. Avoid holding Hub - membership locks across filesystem work; `hardRemoveLocked` currently calls - the removal callback while holding the Hub mutex. - -Acceptance: concurrent login/input/disconnect/admin-mutation integration tests -under `-race`, plus a slow-client test proving other players keep progressing. - -### 2. High: transactional account lifecycle and authoritative membership - -The two outstanding account issues are still present: - -- `characterOnline` releases `charsMu` before rename/delete/purge touches files; - `connectCharacter` can load/register the character during that gap. -- Account rename changes only the initiating `sess.Account.Name`. Other - authenticated sessions retain old account objects. Character lists and - pending menu selections can also become stale after rename/delete/create. - -This is not just a cosmetic broadcast issue. A stale character name can be -reused by another account; cached membership must not authorize a later -connect/rename/delete against that reassigned name. - -Use the owner/lifecycle boundary for lookup, canonical-name resolution, -current membership validation, online check, mutation, and session publication. -Refresh or invalidate all affected authenticated sessions, including pending -menu selections. Do not simply broadcast pointer mutations from a connection - goroutine. Revalidate membership at operation commit, not only menu selection. - -Account creation, character creation, character rename/delete, and purge also -span multiple files and contain ignored save/remove errors in -`core_login_account.go` and `core_login_char.go`. Single-file atomic replacement -cannot prevent orphaned characters, stale ownership lists, or partial purges. -Choose a journaled lifecycle transaction or transactional player database. -Stable account/character IDs would decouple identity from renameable filenames. - -Acceptance: simultaneous same-account operations, stale menu selections, and -injected failure at each filesystem step must preserve identity/membership. - -### 3. High: durable money and graceful shutdown - -Evidence: - -- `internal/game/casino.go`: `casinoWallet.Wager` and `PayCredits` mutate balances - and ignore `SaveCharacter` errors. A successful wager can therefore exist only - in memory, and a reported win can be lost on restart. -- `internal/casino/types.go`: `Wallet.PayCredits` cannot report failure. -- Round/wager/free-spin state exists only in memory. A crash after a saved wager - debit but before settlement loses the player's stake without recovery. -- Currency uses machine-sized `int`, unchecked additions/multiplications, and - floating-point payout conversion. Unlimited bets remain supported. Large - balances or payout multipliers can overflow; finite config validation is not - sufficient protection. -- `cmd/thoi/main.go` exits on signals after requesting tick stop; - `internal/game/cmd_shutdown.go` calls `os.Exit` directly. Neither path drains - work or saves all online characters. Tick stop does not join running callbacks. - The countdown also reads a replaceable `g.shutdownCancel` field from its - goroutine; cancellation/restart needs a captured per-run channel. - -Recommended design: - -- Give each round/wager/settlement a stable transaction ID; debit plus wager - record, and payout plus settlement record, must be durable and idempotent. - A small transactional player/economy database is worth considering while - retaining YAML for authored world content. -- Define checked currency arithmetic and explicit rounding/balance limits. - Distinguish insufficient funds from persistence failure. A failed write must - not silently count as successful gameplay. -- Unify signals and the shutdown command into one shutdown request handled by - main. Stop admissions/admin mutations; quiesce input and autobet; drain the - owner and callbacks; resolve or persist outstanding wagers; save owned state; - then close transports and return. Surface save failures to the operator. -- Decide server-shutdown/crash policy separately from deliberate player - disconnect forfeiture. Do not blindly run disconnect-forfeit callbacks for a - server-initiated shutdown. - -Acceptance: fail saves during wager/payout; restart at each debit/settlement -boundary; test overflow boundaries; SIGTERM and scheduled shutdown must produce -identical recoverable state, without duplicate credits or lost settlements. - -### 4. High for live building: complete room-ID migration and undo - -`handleRoomRename` in `internal/admin/api_rooms.go` rewrites referencing rooms -and invokes player migration, but records only the renamed room in `ChangeDesc`. -Its `NewContent` is the original bytes, not necessarily the rewritten self-loop -content. Undo/redo therefore cannot restore all references or player state. -The reference loop also ignores some load/write failures. - -`Game.RewriteRoomIDs` changes player fields but does not remap Hub room buckets -or casino manager room keys. Live room-ID changes can therefore leave gameplay -and broadcasts attached to old locations. Other runtime room-keyed state must -be inventoried before declaring migration complete. - -`UndoStack` also ignores extra-file operation errors, so it can report success -with only part of a multi-file change restored. Restoring file snapshots alone -cannot update live players or runtime registries. - -Implement one mutation plan containing all file before/after images and explicit -runtime mappings, executed under the owner boundary. Undo/redo must use that -same transaction and propagate failures. A naive inverse ID map is not enough -when the destination had pre-existing stale references. Avoid live renumbering -until this is complete; back up player and world data together. - -Acceptance: rename/self-loop/external exits/triggers/offline and online player -state, Hub membership, active casino games, undo, redo, and injected partial -write failures all agree after execution and restart. - -### 5. Medium: explicit casino reconfiguration and profile contracts - -Manager `EnsureTable`/`EnsureMachine` returns the existing object for a room/ID. -Changing valid room config does not replace its live rules. Deleted/renumbered -rooms can leave cached games behind. Define whether changes take effect only -when idle, after draining rounds, or on restart; never replace an active game -and silently discard its wagers. - -Before custom slot profiles are exposed, validate their geometry and weights. -The current implementation has a fixed 3x5 grid despite configurable profile -fields. Public profile math/validation is not a general slot engine. Fractional -slot credits are discarded on leaving a seat, and crash recovery does not -preserve them. Keep these limits explicit rather than promising arbitrary -profiles or exact realized RTP for short sessions. - -## Verification and limits - -Added regression coverage for complete-file visibility, failed-rename cleanup, -permissions, read-only god-mode saves, file discovery, config rejection and API -non-mutation, independent concurrent casino activity, profile isolation, stale -machine commands, and repeated/concurrent tick lifecycle operations. - -Commands run successfully after the fixes: - -- `go test ./...` -- `go test -race ./...` -- `go vet ./...` - -The original suite also passed race mode before this work. Passing it is not -proof of safe live state ownership: it did not exercise the competing runtime -entry points described above. No production login/load test, process-kill -recovery test, or filesystem power-loss simulation was performed. diff --git a/cmd/thoi/main.go b/cmd/thoi/main.go index cc4785d..2031cb6 100644 --- a/cmd/thoi/main.go +++ b/cmd/thoi/main.go @@ -51,7 +51,9 @@ func main() { // body — the canonical per-tick system sequence — is enforced and tested in // internal/game/tick_systems.go and documented in // building_guide/tick_order.md. Reordering it is a breaking change. - g.Ticks.Subscribe(1, func() bool { + // SubscribeTicks executes the body on the game's owner goroutine so the + // whole tick sequence serializes against session input and admin tasks. + g.SubscribeTicks(1, func() bool { g.RunTickSystems() return true }) @@ -62,8 +64,23 @@ func main() { } g.SetHub(server.Hub()) + // Admin mutation callbacks execute on the game's owner goroutine: the + // admin HTTP goroutine must not touch live players or world state. + // MigrateRoomIDs is the complete room-ID migration (players, Hub + // membership, casino room keys, world state, mob instances). + adminRewriteRoomIDs := g.MigrateRoomIDs + adminRebuildCraftIndex := func() { + g.RunOnOwner(func() { g.RebuildCraftIndex() }) + } + adminEvictRoom := func(roomID int) { + g.RunOnOwner(func() { g.EvictCasinoRoom(roomID) }) + } + adminReloadCourses := func() { + g.RunOnOwner(func() { g.CourseStore.ReloadAll() }) + } + if cfg.AdminHTTP.Enabled { - adminSrv, err := admin.NewServer(cfg, false, g.AccountStore, g.World, g.ItemStore, g.ObjectStore, g.MobStore, dataDir, g.CourseStore.ReloadAll, g.RewriteRoomIDs, g.RebuildCraftIndex, *configPath) + adminSrv, err := admin.NewServer(cfg, false, g.AccountStore, g.World, g.ItemStore, g.ObjectStore, g.MobStore, dataDir, adminReloadCourses, adminRewriteRoomIDs, adminEvictRoom, adminRebuildCraftIndex, *configPath) if err != nil { log.Fatalf("failed to start admin HTTP server: %v", err) } @@ -76,7 +93,7 @@ func main() { } if cfg.AdminHTTPS.Enabled { - adminSrv, err := admin.NewServer(cfg, true, g.AccountStore, g.World, g.ItemStore, g.ObjectStore, g.MobStore, dataDir, g.CourseStore.ReloadAll, g.RewriteRoomIDs, g.RebuildCraftIndex, *configPath) + adminSrv, err := admin.NewServer(cfg, true, g.AccountStore, g.World, g.ItemStore, g.ObjectStore, g.MobStore, dataDir, adminReloadCourses, adminRewriteRoomIDs, adminEvictRoom, adminRebuildCraftIndex, *configPath) if err != nil { log.Fatalf("failed to start admin HTTPS server: %v", err) } @@ -105,14 +122,19 @@ func main() { sigCh := make(chan os.Signal, 1) signal.Notify(sigCh, syscall.SIGINT, syscall.SIGTERM) + // One shutdown request handler: signals and the in-game shutdown + // command both converge on RequestShutdown, which drains work, + // resolves outstanding wagers, saves owned state, then closes the + // transports. ListenAndServe returns after Shutdown, and main returns + // normally (no os.Exit; a final-save failure is logged and the exit + // code reports it via the transport error path). + g.SetTransportStop(server.Shutdown) go func() { <-sigCh - log.Println("Shutting down...") - g.Ticks.Stop() - os.Exit(0) + g.RequestShutdown("signal") }() - if err := server.ListenAndServe(g.HandleSession); err != nil { + if err := server.ListenAndServe(g.HandleSessionOnOwner); err != nil { log.Fatalf("server error: %v", err) } } diff --git a/internal/admin/api_helpers.go b/internal/admin/api_helpers.go index 109bca6..3224598 100644 --- a/internal/admin/api_helpers.go +++ b/internal/admin/api_helpers.go @@ -46,16 +46,27 @@ func stringifyMap(v any) any { } func writeMapAsYAML(path string, m map[string]any) ([]byte, error) { + content, err := marshalMapYAML(m) + if err != nil { + return nil, err + } + if err := os.WriteFile(path, content, 0644); err != nil { + return nil, err + } + return content, nil +} + +// marshalMapYAML serializes a generic map without writing: callers that +// only need the after-image (mutation-plan preflight) use this. +func marshalMapYAML(m map[string]any) ([]byte, error) { var buf bytes.Buffer enc := yaml.NewEncoder(&buf) enc.SetIndent(2) if err := enc.Encode(m); err != nil { return nil, err } - enc.Close() - content := buf.Bytes() - if err := os.WriteFile(path, content, 0644); err != nil { + if err := enc.Close(); err != nil { return nil, err } - return content, nil + return buf.Bytes(), nil } diff --git a/internal/admin/api_room_insert_remove.go b/internal/admin/api_room_insert_remove.go index 8549d76..812a3ae 100644 --- a/internal/admin/api_room_insert_remove.go +++ b/internal/admin/api_room_insert_remove.go @@ -349,6 +349,11 @@ func (s *AdminServer) handleRoomRemove(w http.ResponseWriter, r *http.Request) { if s.mobStore != nil { s.mobStore.RemoveMobsInRoom(bID) } + // Deleted room: evict its cached games (wagers refunded via the wired + // game callback) so gameplay never points at a removed room. + if s.evictRoom != nil { + s.evictRoom(bID) + } s.undoStack.Push(ChangeDesc{ Description: fmt.Sprintf("remove room %d %s (pull %d to %d)", bID, dir, cID, aID), FilePath: bPath, diff --git a/internal/admin/api_room_rename_test.go b/internal/admin/api_room_rename_test.go new file mode 100644 index 0000000..8d273f4 --- /dev/null +++ b/internal/admin/api_room_rename_test.go @@ -0,0 +1,149 @@ +package admin + +// Room-rename mutation-plan regressions: the rename, the reference rewrites +// (including self-loops), the runtime migration callback, undo, redo, and +// injected partial-write failures must all agree. + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "testing" +) + +func renameRoom(t *testing.T, s *AdminServer, id, newID int) *httptest.ResponseRecorder { + t.Helper() + body, _ := json.Marshal(map[string]int{"id": id, "new_id": newID}) + req := httptest.NewRequest(http.MethodPost, "/api/room/rename", strings.NewReader(string(body))) + rec := httptest.NewRecorder() + s.handleRoomRename(rec, req) + return rec +} + +func roomBody(t *testing.T, s *AdminServer, id int) string { + t.Helper() + path, ok := s.world.GetRoomPath(id) + if !ok { + return "" + } + data, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + return string(data) +} + +// TestRoomRenameFullPlanCoversReferencesAndSelfLoop: renaming a room that a +// neighbor references and that references itself rewrites the neighbor, the +// renamed room's own id field AND self-loop, moves the file, and records the +// whole plan for undo. +func TestRoomRenameFullPlan(t *testing.T) { + s := newTestAdminServer(t, map[int]string{ + 5: "id: 5\nname: Lab\nexits:\n north:\n room: 6\n east:\n room: 5\n", + 6: "id: 6\nname: Hall\nexits:\n south:\n room: 5\n", + }) + + var migrated []map[int]int + // The wired callback mirrors production (the game's MigrateRoomIDs): + // record the map and rebuild the world room index after migration. + migrate := func(idMap map[int]int) { + recorded := map[int]int{} + for k, v := range idMap { + recorded[k] = v + } + migrated = append(migrated, recorded) + s.world.RebuildRoomIndex(s.dataDir) + } + s.rewriteRoomIDs = migrate + s.undoStack.SetMigrateRoomIDs(migrate) + + rec := renameRoom(t, s, 5, 7) + if rec.Code != http.StatusOK { + t.Fatalf("rename failed: %d %s", rec.Code, rec.Body.String()) + } + + // Renamed room file: moved, id field rewritten, self-loop corrected. + selfBody := roomBody(t, s, 7) + if !strings.Contains(selfBody, "room: 7") { + t.Fatalf("renamed room self-loop not rewritten: %q", selfBody) + } + if _, err := os.Stat(filepath.Join(s.dataDir, "rooms", "5.yaml")); !os.IsNotExist(err) { + t.Fatalf("old room file still exists: %v", err) + } + // The referencing room's exit moved. + hallBody := roomBody(t, s, 6) + if !strings.Contains(hallBody, "room: 7") { + t.Fatalf("reference rewrite missing: %q", hallBody) + } + // The runtime migration callback ran with the forward map. + if len(migrated) != 1 { + t.Fatalf("runtime migration callback not invoked with one plan: %v", migrated) + } + + // Undo: everything returns to before-images, inverse migration runs. + if s.undoStack.Undo() == nil { + t.Fatal("undo failed") + } + if roomBody(t, s, 5) == "" { + t.Fatal("undo did not restore the renamed room at its old path") + } + if _, err := os.Stat(filepath.Join(s.dataDir, "rooms", "7.yaml")); !os.IsNotExist(err) { + t.Fatalf("undo left the new room file behind: %v", err) + } + if hall := roomBody(t, s, 6); strings.Contains(hall, "room: 7") { + t.Fatalf("undo did not restore the referencing room: %q", hall) + } + // The last migration callback must be the inverse map. + if last := migrated[len(migrated)-1]; !(len(last) == 1 && last[7] == 5) { + t.Fatalf("undo migration callback = %v, want inverse {7:5}", last) + } + + // Redo: forward again, whole plan re-applied. + if s.undoStack.Redo() == nil { + t.Fatal("redo failed") + } + if !strings.Contains(roomBody(t, s, 6), "room: 7") { + t.Fatal("redo did not re-apply the reference rewrite") + } + if roomBody(t, s, 5) != "" { + t.Fatal("redo resurrected the old room file") + } + if last := migrated[len(migrated)-1]; !(len(last) == 1 && last[5] == 7) { + t.Fatalf("redo migration callback = %v, want forward {5:7}", last) + } +} + +// TestRoomRenameAbortsOnWriteFailure: when the write side fails mid-plan, +// the journal transaction rolls back — the old room file and its references +// stay untouched, and no half-renamed state survives. +func TestRoomRenameAbortsOnWriteFailure(t *testing.T) { + s := newTestAdminServer(t, map[int]string{ + 5: "id: 5\nname: Lab\nexits:\n north:\n room: 6\n", + 6: "id: 6\nname: Hall\nexits:\n south:\n room: 5\n", + }) + + // A read-only rooms directory makes every room-file write fail. + roomsDir := filepath.Join(s.dataDir, "rooms") + if err := os.Chmod(roomsDir, 0500); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { os.Chmod(roomsDir, 0700) }) + + rec := renameRoom(t, s, 5, 7) + if rec.Code == http.StatusOK { + t.Fatalf("rename succeeded despite the write failure: %s", rec.Body.String()) + } + os.Chmod(roomsDir, 0700) + + // Nothing was renamed: room 5 still exists at its old path with its + // original content, and room 7 does not exist. + if body := roomBody(t, s, 5); !strings.Contains(body, "id: 5") { + t.Fatalf("room 5 damaged by failed rename: %q", body) + } + if roomExists(s, 7) { + t.Fatal("room 7 exists after the failed rename") + } +} diff --git a/internal/admin/api_rooms.go b/internal/admin/api_rooms.go index 2c251bb..4cfb467 100644 --- a/internal/admin/api_rooms.go +++ b/internal/admin/api_rooms.go @@ -12,6 +12,7 @@ import ( "strings" "thehouseoficarus/internal/behavior" + "thehouseoficarus/internal/persistence" "thehouseoficarus/internal/world" ) @@ -840,6 +841,18 @@ func (s *AdminServer) handleRoomMove(w http.ResponseWriter, r *http.Request) { writeJSON(w, map[string]any{"ok": true, "path": newPath}) } +// handleRoomRename executes a room-ID rename as one complete mutation plan: +// +// 1. Preflight reads every room file and computes after-images in memory +// (the renamed room — id field, filename, and self-loop references — plus +// every other room whose references moved). Load failures abort BEFORE +// any mutation; nothing is half-applied. +// 2. The file changes apply as a single journal transaction, rolling back +// on any write failure. +// 3. The wired runtime migration (players, Hub buckets, casino room keys, +// world room-keyed state, mob instances) runs through one owner task. +// 4. The undo record captures every before/after image and the idMap, so +// undo/redo restore the whole plan, not just the renamed file. func (s *AdminServer) handleRoomRename(w http.ResponseWriter, r *http.Request) { if r.Method != http.MethodPost { http.Error(w, "method not allowed", http.StatusMethodNotAllowed) @@ -862,12 +875,10 @@ func (s *AdminServer) handleRoomRename(w http.ResponseWriter, r *http.Request) { writeJSONError(w, "room not found", http.StatusBadRequest) return } - if _, ok := s.world.GetRoomPath(body.NewID); ok { writeJSONError(w, fmt.Sprintf("room %d already exists", body.NewID), http.StatusConflict) return } - dir := filepath.Dir(oldPath) newPath := filepath.Join(dir, strconv.Itoa(body.NewID)+".yaml") if _, err := os.Stat(newPath); err == nil { @@ -875,63 +886,125 @@ func (s *AdminServer) handleRoomRename(w http.ResponseWriter, r *http.Request) { return } - data, err := os.ReadFile(oldPath) - if err != nil { - writeJSONError(w, "read: "+err.Error(), http.StatusInternalServerError) + idMap := map[int]int{body.ID: body.NewID} + + // ---- Preflight: read every room file and compute after-images in + // memory before mutating anything. Load or marshal failures abort the + // request with no half-applied state. ---- + allIDs, _ := listRoomIDs(s.dataDir) + type roomImage struct { + path string + before []byte + after []byte + changed bool + } + images := make(map[int]*roomImage, len(allIDs)) + for _, roomID := range allIDs { + path, pathOk := s.world.GetRoomPath(roomID) + if !pathOk { + writeJSONError(w, fmt.Sprintf("room %d has no file path", roomID), http.StatusInternalServerError) + return + } + data, err := os.ReadFile(path) + if err != nil { + writeJSONError(w, "read: "+err.Error(), http.StatusInternalServerError) + return + } + img := &roomImage{path: path, before: data, after: data} + + room, loadErr := s.world.LoadRoom(roomID) + if loadErr != nil { + writeJSONError(w, fmt.Sprintf("load room %d: %v", roomID, loadErr), http.StatusInternalServerError) + return + } + moved := room.RewriteRoomIDs(idMap) + if roomID == body.ID { + // The renamed room's own id field becomes the new ID (the + // file index keys by filename, but the room's self-view must + // agree with it). + room.ID = body.NewID + moved = true + } + if moved { + after, err := marshalYAML(room) + if err != nil { + writeJSONError(w, "marshal: "+err.Error(), http.StatusInternalServerError) + return + } + img.after, img.changed = after, true + } + images[roomID] = img + } + renamed, ok := images[body.ID] + if !ok { + writeJSONError(w, "renamed room missing from plan", http.StatusInternalServerError) return } - if err := os.WriteFile(newPath, data, 0644); err != nil { - writeJSONError(w, "write: "+err.Error(), http.StatusInternalServerError) + + // ---- Apply the file changes as one journal transaction. ---- + txn := persistence.NewTransaction(filepath.Join(s.dataDir, ".journal")) + if err := txn.Move(oldPath, newPath); err != nil { + writeJSONError(w, "plan move: "+err.Error(), http.StatusInternalServerError) return } - if err := os.Remove(oldPath); err != nil { - writeJSONError(w, "remove old: "+err.Error(), http.StatusInternalServerError) + // The renamed room's rewritten content (new id field + corrected + // self-loop) lands via a replace on the new path right after the move. + if err := txn.Replace(newPath, renamed.after, 0644); err != nil { + writeJSONError(w, "plan self write: "+err.Error(), http.StatusInternalServerError) return } - - s.world.RebuildRoomIndex(s.dataDir) - s.world.ClearRoomState(body.ID) - - // Remap every genuine room-ID reference (exit targets, trigger/on-enter - // room+teleport+despawn_rooms, mob wander_rooms) from the old ID to the - // new one across all rooms — including the renamed room itself, so a - // self-loop exit is corrected too. Only rooms whose references actually - // moved are re-marshaled (preserving untouched files' formatting). - idMap := map[int]int{body.ID: body.NewID} - allIDs, _ := listRoomIDs(s.dataDir) var refUpdates int - for _, otherID := range allIDs { - otherRoom, loadErr := s.world.LoadRoom(otherID) - if loadErr != nil { - continue - } - if !otherRoom.RewriteRoomIDs(idMap) { + for roomID, img := range images { + if !img.changed || roomID == body.ID { continue } - otherPath, pathOk := s.world.GetRoomPath(otherID) - if !pathOk { - continue + if err := txn.Replace(img.path, img.after, 0644); err != nil { + writeJSONError(w, "plan write: "+err.Error(), http.StatusInternalServerError) + return } - writeYAMLFile(otherPath, otherRoom) refUpdates++ } + if err := txn.Commit(); err != nil { + writeJSONError(w, "commit rename: "+err.Error(), http.StatusInternalServerError) + return + } + + s.world.RebuildRoomIndex(s.dataDir) + s.world.ClearRoomState(body.ID) // Migrate room-ID-embedded state across all characters (discovered-exit - // player flags, RoomID, EnterSeqRoom, MapSymbols, RoomsVisited) so a - // rename does not strand players or leave stale hidden_exit__ - // flags pointing at the old room ID. + // player flags, RoomID, EnterSeqRoom, MapSymbols, RoomsVisited) plus + // Hub membership buckets, casino room keys, world room-keyed state, and + // live mob instances, so a rename leaves no gameplay, broadcast, or + // player attached to the old location. if s.rewriteRoomIDs != nil { s.rewriteRoomIDs(idMap) } - s.undoStack.Push(ChangeDesc{ + // ---- Undo record: the complete plan (renamed room before/after images + // including the rewritten id field and self-loop references, every + // rewritten reference room, and the idMap for inverse runtime + // migration). Undo/redo restore the whole plan, not just the renamed + // file. ---- + change := ChangeDesc{ Description: fmt.Sprintf("rename room %d to %d (updated %d refs)", body.ID, body.NewID, refUpdates), FilePath: oldPath, NewFilePath: newPath, - OldContent: data, - NewContent: data, - }) - writeJSON(w, map[string]any{"ok": true}) + OldContent: renamed.before, + NewContent: renamed.after, + IDMap: idMap, + } + for roomID, img := range images { + if img.changed && roomID != body.ID { + change.ExtraFiles = append(change.ExtraFiles, ExtraFile{ + FilePath: img.path, + OldContent: img.before, + NewContent: img.after, + }) + } + } + s.undoStack.Push(change) + writeJSON(w, map[string]any{"ok": true, "refs": refUpdates}) } func (s *AdminServer) handleRoomSwap(w http.ResponseWriter, r *http.Request) { diff --git a/internal/admin/api_rooms_bulk_delete.go b/internal/admin/api_rooms_bulk_delete.go index 6aa0181..18b10a5 100644 --- a/internal/admin/api_rooms_bulk_delete.go +++ b/internal/admin/api_rooms_bulk_delete.go @@ -72,6 +72,9 @@ func (s *AdminServer) handleBulkDelete(w http.ResponseWriter, r *http.Request) { s.world.RebuildRoomIndex(s.dataDir) for _, snap := range snapshots { s.world.ClearRoomState(snap.id) + if s.evictRoom != nil { + s.evictRoom(snap.id) + } } allIDs, _ := listRoomIDs(s.dataDir) diff --git a/internal/admin/server.go b/internal/admin/server.go index ccad6a2..41c1e0f 100644 --- a/internal/admin/server.go +++ b/internal/admin/server.go @@ -58,13 +58,18 @@ type AdminServer struct { // configPath is the on-disk config.yaml path, used by the color themer // to persist edited default_colors back to the file the game loaded. configPath string + // evictRoom evicts cached in-room games (tables/machines) when a room + // is deleted or renumbered: outstanding wagers are refunded through the + // wired game callback before the cache entries drop. + evictRoom func(roomID int) + // rebuildCraftIndex rebuilds the in-game craft index after item files // change (create/update/delete/rename/move/undo/redo). Wired from the // Game so the web editor's item edits stay consistent with gameplay. rebuildCraftIndex func() } -func NewServer(cfg *config.Config, useTLS bool, accountStore *player.AccountStore, w *world.World, is *item.ItemStore, os *object.ObjectStore, ms *world.MobStore, dataDir string, reloadCourses func(), rewriteRoomIDs func(map[int]int), rebuildCraftIndex func(), configPath string) (*AdminServer, error) { +func NewServer(cfg *config.Config, useTLS bool, accountStore *player.AccountStore, w *world.World, is *item.ItemStore, os *object.ObjectStore, ms *world.MobStore, dataDir string, reloadCourses func(), rewriteRoomIDs func(map[int]int), evictRoom func(roomID int), rebuildCraftIndex func(), configPath string) (*AdminServer, error) { secret := make([]byte, 32) if _, err := rand.Read(secret); err != nil { return nil, fmt.Errorf("cookie secret: %w", err) @@ -109,9 +114,13 @@ func NewServer(cfg *config.Config, useTLS bool, accountStore *player.AccountStor cookieSecret: secret, reloadCourses: reloadCourses, rewriteRoomIDs: rewriteRoomIDs, + evictRoom: evictRoom, rebuildCraftIndex: rebuildCraftIndex, configPath: configPath, } + // Undo/redo of room-ID changes applies the (inverse) runtime migration + // through the same wired callback the rename uses. + s.undoStack.SetMigrateRoomIDs(rewriteRoomIDs) mux := http.NewServeMux() diff --git a/internal/admin/undo.go b/internal/admin/undo.go index 5e1e6a3..0f9744c 100644 --- a/internal/admin/undo.go +++ b/internal/admin/undo.go @@ -29,6 +29,11 @@ type ChangeDesc struct { IsDelete bool `json:"is_delete"` IsCreate bool `json:"is_create"` ExtraFiles []ExtraFile `json:"extra_files,omitempty"` + // IDMap records a room-ID change (oldID -> newID) included in the + // change. Undo applies its inverse, redo the forward map, through the + // wired runtime migration so players, Hub membership, and casino room + // keys move back and forth with the file images. + IDMap map[int]int `json:"id_map,omitempty"` } func (c ChangeDesc) ShortDesc() string { @@ -63,6 +68,9 @@ type UndoStack struct { redo []ChangeDesc maxSize int filePath string + // migrateRoomIDs applies a room-ID map to runtime state (players, Hub + // membership, casino room keys) during undo/redo of room-ID changes. + migrateRoomIDs func(map[int]int) } func NewUndoStack(dataDir string) *UndoStack { @@ -73,6 +81,14 @@ func NewUndoStack(dataDir string) *UndoStack { } } +// SetMigrateRoomIDs wires the runtime room-ID migration used by undo/redo +// of room-ID changes. Called by the server after construction. +func (us *UndoStack) SetMigrateRoomIDs(fn func(map[int]int)) { + us.mu.Lock() + defer us.mu.Unlock() + us.migrateRoomIDs = fn +} + func (us *UndoStack) Push(desc ChangeDesc) { us.mu.Lock() defer us.mu.Unlock() @@ -98,6 +114,16 @@ func (us *UndoStack) Undo() *ChangeDesc { log.Printf("undo: failed %s: %v", last.ShortDesc(), err) return nil } + // Runtime migration moves with the file images: undo applies the + // inverse room-ID map so players, Hub membership, and casino room keys + // return to the old location together. + if len(last.IDMap) > 0 && us.migrateRoomIDs != nil { + inverse := make(map[int]int, len(last.IDMap)) + for from, to := range last.IDMap { + inverse[to] = from + } + us.migrateRoomIDs(inverse) + } us.history = us.history[:len(us.history)-1] us.redo = append(us.redo, last) @@ -118,6 +144,14 @@ func (us *UndoStack) Redo() *ChangeDesc { log.Printf("redo: failed %s: %v", last.ShortDesc(), err) return nil } + // Redo applies the forward room-ID map through the runtime migration. + if len(last.IDMap) > 0 && us.migrateRoomIDs != nil { + forward := make(map[int]int, len(last.IDMap)) + for from, to := range last.IDMap { + forward[from] = to + } + us.migrateRoomIDs(forward) + } us.redo = us.redo[:len(us.redo)-1] us.history = append(us.history, last) @@ -126,64 +160,80 @@ func (us *UndoStack) Redo() *ChangeDesc { return &last } +// applyUndo reverts a change: the primary file, then every extra file. +// Extra-file failures propagate: a multi-file undo that restored only part +// of a change must be reported as failed, never as success. func (us *UndoStack) applyUndo(c ChangeDesc) error { if c.NewFilePath != "" { - os.Remove(c.NewFilePath) + if err := os.Remove(c.NewFilePath); err != nil && !os.IsNotExist(err) { + return fmt.Errorf("remove %s: %w", c.NewFilePath, err) + } } if c.IsDelete { if c.OldContent == nil { - os.MkdirAll(c.FilePath, 0755) + if err := os.MkdirAll(c.FilePath, 0755); err != nil { + return fmt.Errorf("recreate dir %s: %w", c.FilePath, err) + } } else { - os.MkdirAll(filepath.Dir(c.FilePath), 0755) + if err := os.MkdirAll(filepath.Dir(c.FilePath), 0755); err != nil { + return fmt.Errorf("mkdir: %w", err) + } if err := os.WriteFile(c.FilePath, c.OldContent, 0644); err != nil { return fmt.Errorf("restore deleted %s: %w", c.FilePath, err) } } - for _, ef := range c.ExtraFiles { - applyUndoExtraFile(ef) - } } else if c.IsCreate { - os.Remove(c.FilePath) - for _, ef := range c.ExtraFiles { - applyUndoExtraFile(ef) + if err := os.Remove(c.FilePath); err != nil && !os.IsNotExist(err) { + return fmt.Errorf("remove created %s: %w", c.FilePath, err) } } else { - target := c.FilePath - content := c.OldContent - if c.NewFilePath != "" { - target = c.FilePath + if err := os.MkdirAll(filepath.Dir(c.FilePath), 0755); err != nil { + return fmt.Errorf("mkdir: %w", err) } - os.MkdirAll(filepath.Dir(target), 0755) - if err := os.WriteFile(target, content, 0644); err != nil { - return fmt.Errorf("revert %s: %w", target, err) + if err := os.WriteFile(c.FilePath, c.OldContent, 0644); err != nil { + return fmt.Errorf("revert %s: %w", c.FilePath, err) } - for _, ef := range c.ExtraFiles { - applyUndoExtraFile(ef) + } + for _, ef := range c.ExtraFiles { + if err := applyUndoExtraFile(ef); err != nil { + return err } } return nil } +// applyRedo re-applies a change: the primary file, then every extra file. +// Extra-file failures propagate like undo. func (us *UndoStack) applyRedo(c ChangeDesc) error { if c.NewFilePath != "" { - os.Remove(c.FilePath) + if err := os.Remove(c.FilePath); err != nil && !os.IsNotExist(err) { + return fmt.Errorf("remove %s: %w", c.FilePath, err) + } } if c.IsCreate { target := c.FilePath if c.NewFilePath != "" { target = c.NewFilePath } - os.MkdirAll(filepath.Dir(target), 0755) + if err := os.MkdirAll(filepath.Dir(target), 0755); err != nil { + return fmt.Errorf("mkdir: %w", err) + } if err := os.WriteFile(target, c.NewContent, 0644); err != nil { return fmt.Errorf("recreate %s: %w", target, err) } for _, ef := range c.ExtraFiles { - applyRedoExtraFile(ef) + if err := applyRedoExtraFile(ef); err != nil { + return err + } } } else if c.IsDelete { - os.Remove(c.FilePath) + if err := os.Remove(c.FilePath); err != nil && !os.IsNotExist(err) { + return fmt.Errorf("re-delete %s: %w", c.FilePath, err) + } for _, ef := range c.ExtraFiles { - applyRedoExtraFile(ef) + if err := applyRedoExtraFile(ef); err != nil { + return err + } } } else { target := c.FilePath @@ -191,49 +241,87 @@ func (us *UndoStack) applyRedo(c ChangeDesc) error { if c.NewFilePath != "" { target = c.NewFilePath } - os.MkdirAll(filepath.Dir(target), 0755) + if err := os.MkdirAll(filepath.Dir(target), 0755); err != nil { + return fmt.Errorf("mkdir: %w", err) + } if err := os.WriteFile(target, content, 0644); err != nil { return fmt.Errorf("reapply %s: %w", target, err) } for _, ef := range c.ExtraFiles { - applyRedoExtraFile(ef) + if err := applyRedoExtraFile(ef); err != nil { + return err + } } } return nil } -func applyUndoExtraFile(ef ExtraFile) { +func applyUndoExtraFile(ef ExtraFile) error { switch { case ef.IsCreate: - os.Remove(ef.FilePath) + if err := os.Remove(ef.FilePath); err != nil && !os.IsNotExist(err) { + return fmt.Errorf("undo remove %s: %w", ef.FilePath, err) + } case ef.IsDelete: - os.MkdirAll(filepath.Dir(ef.FilePath), 0755) - os.WriteFile(ef.FilePath, ef.OldContent, 0644) + if err := os.MkdirAll(filepath.Dir(ef.FilePath), 0755); err != nil { + return fmt.Errorf("undo mkdir: %w", err) + } + if err := os.WriteFile(ef.FilePath, ef.OldContent, 0644); err != nil { + return fmt.Errorf("undo restore %s: %w", ef.FilePath, err) + } case ef.NewFilePath != "": - os.Remove(ef.NewFilePath) - os.MkdirAll(filepath.Dir(ef.FilePath), 0755) - os.WriteFile(ef.FilePath, ef.OldContent, 0644) + if err := os.Remove(ef.NewFilePath); err != nil && !os.IsNotExist(err) { + return fmt.Errorf("undo remove %s: %w", ef.NewFilePath, err) + } + if err := os.MkdirAll(filepath.Dir(ef.FilePath), 0755); err != nil { + return fmt.Errorf("undo mkdir: %w", err) + } + if err := os.WriteFile(ef.FilePath, ef.OldContent, 0644); err != nil { + return fmt.Errorf("undo restore %s: %w", ef.FilePath, err) + } default: - os.MkdirAll(filepath.Dir(ef.FilePath), 0755) - os.WriteFile(ef.FilePath, ef.OldContent, 0644) + if err := os.MkdirAll(filepath.Dir(ef.FilePath), 0755); err != nil { + return fmt.Errorf("undo mkdir: %w", err) + } + if err := os.WriteFile(ef.FilePath, ef.OldContent, 0644); err != nil { + return fmt.Errorf("undo revert %s: %w", ef.FilePath, err) + } } + return nil } -func applyRedoExtraFile(ef ExtraFile) { +func applyRedoExtraFile(ef ExtraFile) error { switch { case ef.IsDelete: - os.Remove(ef.FilePath) + if err := os.Remove(ef.FilePath); err != nil && !os.IsNotExist(err) { + return fmt.Errorf("redo remove %s: %w", ef.FilePath, err) + } case ef.IsCreate: - os.MkdirAll(filepath.Dir(ef.FilePath), 0755) - os.WriteFile(ef.FilePath, ef.NewContent, 0644) + if err := os.MkdirAll(filepath.Dir(ef.FilePath), 0755); err != nil { + return fmt.Errorf("redo mkdir: %w", err) + } + if err := os.WriteFile(ef.FilePath, ef.NewContent, 0644); err != nil { + return fmt.Errorf("redo create %s: %w", ef.FilePath, err) + } case ef.NewFilePath != "": - os.Remove(ef.FilePath) - os.MkdirAll(filepath.Dir(ef.NewFilePath), 0755) - os.WriteFile(ef.NewFilePath, ef.NewContent, 0644) + if err := os.Remove(ef.FilePath); err != nil && !os.IsNotExist(err) { + return fmt.Errorf("redo remove %s: %w", ef.FilePath, err) + } + if err := os.MkdirAll(filepath.Dir(ef.NewFilePath), 0755); err != nil { + return fmt.Errorf("redo mkdir: %w", err) + } + if err := os.WriteFile(ef.NewFilePath, ef.NewContent, 0644); err != nil { + return fmt.Errorf("redo create %s: %w", ef.NewFilePath, err) + } default: - os.MkdirAll(filepath.Dir(ef.FilePath), 0755) - os.WriteFile(ef.FilePath, ef.NewContent, 0644) + if err := os.MkdirAll(filepath.Dir(ef.FilePath), 0755); err != nil { + return fmt.Errorf("redo mkdir: %w", err) + } + if err := os.WriteFile(ef.FilePath, ef.NewContent, 0644); err != nil { + return fmt.Errorf("redo reapply %s: %w", ef.FilePath, err) + } } + return nil } func (us *UndoStack) Info() UndoInfo { @@ -273,6 +361,7 @@ func (us *UndoStack) save() { IsDelete bool `json:"is_delete"` IsCreate bool `json:"is_create"` ExtraFiles []extraEntry `json:"extra_files,omitempty"` + IDMap map[int]int `json:"id_map,omitempty"` } type saveData struct { History []entry `json:"history"` @@ -302,6 +391,7 @@ func (us *UndoStack) save() { IsDelete: c.IsDelete, IsCreate: c.IsCreate, ExtraFiles: extra, + IDMap: c.IDMap, }) } return entries @@ -344,6 +434,7 @@ func (us *UndoStack) load() { IsDelete bool `json:"is_delete"` IsCreate bool `json:"is_create"` ExtraFiles []extraEntry `json:"extra_files,omitempty"` + IDMap map[int]int `json:"id_map,omitempty"` } var saveData struct { History []entry `json:"history"` @@ -377,6 +468,7 @@ func (us *UndoStack) load() { IsDelete: e.IsDelete, IsCreate: e.IsCreate, ExtraFiles: extra, + IDMap: e.IDMap, }) } return changes diff --git a/internal/admin/yaml_util.go b/internal/admin/yaml_util.go index 2d838ad..7157f15 100644 --- a/internal/admin/yaml_util.go +++ b/internal/admin/yaml_util.go @@ -17,15 +17,24 @@ func readYAMLFile(dataDir, subdir, id string) ([]byte, string, error) { return data, path, err } -func writeYAMLFile(path string, data any) ([]byte, error) { +func marshalYAML(data any) ([]byte, error) { var buf bytes.Buffer enc := yaml.NewEncoder(&buf) enc.SetIndent(2) if err := enc.Encode(data); err != nil { return nil, err } - enc.Close() - content := buf.Bytes() + if err := enc.Close(); err != nil { + return nil, err + } + return buf.Bytes(), nil +} + +func writeYAMLFile(path string, data any) ([]byte, error) { + content, err := marshalYAML(data) + if err != nil { + return nil, err + } if err := os.WriteFile(path, content, 0644); err != nil { return nil, err } diff --git a/internal/casino/baccarat.go b/internal/casino/baccarat.go index 800b5e6..d415901 100644 --- a/internal/casino/baccarat.go +++ b/internal/casino/baccarat.go @@ -2,7 +2,6 @@ package casino import ( "fmt" - "math" "math/rand" "sync" ) @@ -125,9 +124,9 @@ func (t *baccaratTable) bet(name, spot string, amount int, wallet Wallet) []Even if amount < t.Config.MinBet || amount > t.Config.MaxBet { return []Event{t.private(name, fmt.Sprintf("Baccarat bets must be %s.", betRangeText(t.Config.MinBet, t.Config.MaxBet)))} } - wager, ok := wallet.Wager(amount) - if !ok { - return []Event{t.private(name, "You don't have enough chips and credits for that bet.")} + wager, err := wallet.Wager(amount) + if err != nil { + return []Event{t.private(name, walletErrorMessage(err))} } p.HasBet, p.Wager, p.Wallet, p.lastBet, p.lastSpot = true, wager, wallet, amount, spot events := []Event{t.playerAction(name, @@ -329,11 +328,13 @@ func settleBaccaratBet(spot string, bet int, playerTotal, bankerTotal int, rules } case bankerTotal > playerTotal: if spot == baccaratSpotBanker { - return "win", bet + int(math.Floor(float64(bet)*rules.BankerPayout)) + bankerWin, _ := MulPayout(bet, rules.BankerPayout) + return "win", bet + bankerWin } default: if spot == baccaratSpotTie { - return "win", bet + int(math.Floor(float64(bet)*rules.TiePayout)) + tieWin, _ := MulPayout(bet, rules.TiePayout) + return "win", bet + tieWin } return "push", bet } @@ -361,3 +362,35 @@ func baccaratResultMessages(name, result string, payout int) (string, string) { return name + " loses.", "You lose!" } } + +// refundAll resolves every committed wager at this table (graceful +// shutdown): stakes return to their players through the wallet. +func (t *baccaratTable) refundAll() []Event { + t.mu.Lock() + defer t.mu.Unlock() + var events []Event + for _, p := range t.players.ordered() { + if !p.HasBet || p.Wallet == nil { + continue + } + name, refund := p.Name, p.Wager.Total + if refund <= 0 { + continue + } + if err := p.Wallet.PayCredits(refund); err != nil { + events = append(events, t.private(name, "Shutdown refund could not be persisted; the stake is recorded for recovery.")) + continue + } + events = append(events, t.playerAction(name, + fmt.Sprintf("%s is refunded %d credits as the casino closes.", name, refund), + fmt.Sprintf("The house refunds your %d credit stake for the shutdown.", refund))) + } + return events +} + +// setRoom updates the event-routing room after a room-ID rekey. Locked. +func (t *baccaratTable) setRoom(roomID int) { + t.mu.Lock() + defer t.mu.Unlock() + t.tableBase.setRoom(roomID) +} diff --git a/internal/casino/blackjack.go b/internal/casino/blackjack.go index 4901c31..7c01c34 100644 --- a/internal/casino/blackjack.go +++ b/internal/casino/blackjack.go @@ -2,7 +2,6 @@ package casino import ( "fmt" - "math" "math/rand" "strings" "sync" @@ -147,9 +146,9 @@ func (t *blackjackTable) bet(name, spot string, amount int, wallet Wallet) []Eve if amount < t.Config.MinBet || amount > t.Config.MaxBet { return []Event{t.private(name, fmt.Sprintf("Blackjack bets must be %s.", betRangeText(t.Config.MinBet, t.Config.MaxBet)))} } - wager, ok := wallet.Wager(amount) - if !ok { - return []Event{t.private(name, "You don't have enough chips and credits for that bet.")} + wager, err := wallet.Wager(amount) + if err != nil { + return []Event{t.private(name, walletErrorMessage(err))} } p.HasBet, p.Wager, p.Wallet, p.lastBet = true, wager, wallet, amount events := []Event{t.playerAction(name, fmt.Sprintf("%s bets %d credits.", name, amount), fmt.Sprintf("You bet %d credits.", amount))} @@ -263,8 +262,8 @@ func (t *blackjackTable) action(name, action string) []Event { if len(hand.cards) != 2 || hand.doubled || hand.surrendered || fromSplitAces(hand) || (hand.fromSplit && !t.Rules.AllowsDoubleAfterSplit()) { return []Event{t.private(name, "You can only double a qualifying hand.")} } - if _, ok := p.Wallet.Wager(hand.bet); !ok { - return []Event{t.private(name, "You don't have enough chips and credits to double that hand.")} + if _, err := p.Wallet.Wager(hand.bet); err != nil { + return []Event{t.private(name, walletErrorMessage(err, "You don't have enough chips and credits to double that hand."))} } hand.bet *= 2 hand.doubled = true @@ -305,8 +304,8 @@ func (t *blackjackTable) insuranceAction(name, action string) []Event { if amount <= 0 { return []Event{t.private(name, "Your bet is too small to insure.")} } - if _, ok := p.Wallet.Wager(amount); !ok { - return []Event{t.private(name, "You don't have enough chips and credits for insurance.")} + if _, err := p.Wallet.Wager(amount); err != nil { + return []Event{t.private(name, walletErrorMessage(err, "You don't have enough chips and credits for insurance."))} } p.insurance = amount events = append(events, t.playerAction(name, fmt.Sprintf("%s takes insurance.", name), fmt.Sprintf("You place %d credits on insurance.", amount))) @@ -351,8 +350,8 @@ func (t *blackjackTable) splitHand(name string, p *blackjackPlayer, hand *blackj if hand.cards[0].rank == "A" && hand.fromSplit && !t.Rules.AllowsResplitAces() { return []Event{t.private(name, "Aces cannot be re-split at this table.")} } - if _, ok := p.Wallet.Wager(hand.bet); !ok { - return []Event{t.private(name, "You don't have enough chips and credits to split that hand.")} + if _, err := p.Wallet.Wager(hand.bet); err != nil { + return []Event{t.private(name, walletErrorMessage(err, "You don't have enough chips and credits to split that hand."))} } left := &blackjackHand{cards: []card{hand.cards[0]}, bet: hand.bet, fromSplit: true} right := &blackjackHand{cards: []card{hand.cards[1]}, bet: hand.bet, fromSplit: true} @@ -560,7 +559,7 @@ func (t *blackjackTable) settle() []Event { if p.insurance > 0 { insurancePayout := 0 if dealerBlackjack { - insurancePayout = int(math.Floor(float64(p.insurance) * (1 + t.Rules.InsurancePayout))) + insurancePayout, _ = MulPayout(p.insurance, 1+t.Rules.InsurancePayout) } if insurancePayout > 0 { p.Wallet.PayCredits(insurancePayout) @@ -594,7 +593,8 @@ func settleHand(hand *blackjackHand, dealerTotal int, dealerBlackjack bool, rule } if playerBlackjack { // Winnings round down to whole credits; never bet less than two. - return "blackjack", hand.bet + int(math.Floor(float64(hand.bet)*rules.BlackjackPayout)) + blackjackWin, _ := MulPayout(hand.bet, rules.BlackjackPayout) + return "blackjack", hand.bet + blackjackWin } if total > 21 || dealerTotal > total && dealerTotal <= 21 { return "loss", 0 @@ -728,3 +728,35 @@ func (t *blackjackTable) hasParticipant(name string) bool { defer t.mu.Unlock() return t.players.has(name) } + +// refundAll resolves every committed wager at this table (graceful +// shutdown): stakes return to their players through the wallet. +func (t *blackjackTable) refundAll() []Event { + t.mu.Lock() + defer t.mu.Unlock() + var events []Event + for _, p := range t.players.ordered() { + if !p.HasBet || p.Wallet == nil { + continue + } + name, refund := p.Name, p.Wager.Total + if refund <= 0 { + continue + } + if err := p.Wallet.PayCredits(refund); err != nil { + events = append(events, t.private(name, "Shutdown refund could not be persisted; the stake is recorded for recovery.")) + continue + } + events = append(events, t.playerAction(name, + fmt.Sprintf("%s is refunded %d credits as the casino closes.", name, refund), + fmt.Sprintf("The house refunds your %d credit stake for the shutdown.", refund))) + } + return events +} + +// setRoom updates the event-routing room after a room-ID rekey. Locked. +func (t *blackjackTable) setRoom(roomID int) { + t.mu.Lock() + defer t.mu.Unlock() + t.tableBase.setRoom(roomID) +} diff --git a/internal/casino/credits.go b/internal/casino/credits.go new file mode 100644 index 0000000..ce47d41 --- /dev/null +++ b/internal/casino/credits.go @@ -0,0 +1,84 @@ +package casino + +import ( + "errors" + "fmt" + "math" +) + +// Currency limits and checked arithmetic. All balances and payouts flow +// through these helpers so overflow and unbounded multipliers fail loudly +// instead of silently wrapping or producing absurd balances. +const ( + // MaxCredits is the largest credit balance the economy supports. + // Configured bets are additionally bounded by each game's MaxBet. + MaxCredits = 1_000_000_000_000 // 1e12 + // DefaultMaxBet bounds wagers when a configuration does not set one. + // The explicit unlimited sentinel (MaxBet = MaxInt) remains accepted + // for authored configurations that deliberately want no cap. + DefaultMaxBet = 1_000_000 +) + +// maxBetUnlimited is the MaxBet sentinel meaning "no wager cap". +const maxBetUnlimited = int(^uint(0) >> 1) + +// Wallet errors. ErrInsufficientFunds is normal gameplay (the table reports +// it like a rejected bet); ErrWalletPersistence means the wager or payout +// could not be made durable and MUST NOT be presented as successful play. +var ( + ErrInsufficientFunds = errors.New("insufficient funds") + ErrWalletPersistence = errors.New("wallet persistence failure") +) + +// AddCredits adds two balances with an explicit cap, reporting overflow. +// A negative addend is rejected (payouts never decrease a balance). +func AddCredits(a, b int) (int, error) { + if b < 0 { + return a, errors.New("negative credit add") + } + sum := a + b + if b > 0 && sum < a || a > 0 && sum < b { + return MaxCredits, fmt.Errorf("credit overflow: %d + %d exceeds %d", a, b, MaxCredits) + } + if sum > MaxCredits { + return MaxCredits, fmt.Errorf("credit overflow: %d + %d exceeds %d", a, b, MaxCredits) + } + return sum, nil +} + +// MulPayout scales an amount by a payout multiplier and converts to whole +// credits (floor, like a house paying in chips). Overflow past MaxCredits +// is an error, not a silent wrap. A non-finite or negative multiplier is +// rejected. +func MulPayout(amount int, multiplier float64) (int, error) { + if multiplier < 0 || math.IsInf(multiplier, 0) || math.IsNaN(multiplier) { + return 0, fmt.Errorf("invalid payout multiplier %v", multiplier) + } + if amount <= 0 { + return 0, nil + } + // Bound the product before conversion: amount * multiplier must fit + // within MaxCredits (multiplier > 1 may grow the stake). + if float64(amount) > float64(MaxCredits)/multiplier { + return MaxCredits, fmt.Errorf("payout overflow: %d * %v exceeds %d", amount, multiplier, MaxCredits) + } + payout := int(math.Floor(float64(amount) * multiplier)) + if payout > MaxCredits { + return MaxCredits, fmt.Errorf("payout overflow: %d * %v exceeds %d", amount, multiplier, MaxCredits) + } + return payout, nil +} + +// walletErrorMessage renders a wallet error for a player: insufficient +// funds is normal gameplay; a persistence failure makes clear that the +// house could not process the wager rather than implying a short balance. +// context, when non-empty, is the game-specific insufficient-funds wording. +func walletErrorMessage(err error, context ...string) string { + if err == ErrWalletPersistence { + return "The house could not process that transaction (persistence failure). Nothing was charged; try again shortly." + } + if len(context) > 0 && context[0] != "" { + return context[0] + } + return "You don't have enough chips and credits for that bet." +} diff --git a/internal/casino/credits_test.go b/internal/casino/credits_test.go new file mode 100644 index 0000000..4094d72 --- /dev/null +++ b/internal/casino/credits_test.go @@ -0,0 +1,74 @@ +package casino + +import ( + "testing" +) + +func TestAddCreditsChecked(t *testing.T) { + sum, err := AddCredits(5, 7) + if err != nil || sum != 12 { + t.Fatalf("AddCredits(5,7) = %d, %v", sum, err) + } + if sum, err := AddCredits(MaxCredits, 1); err == nil || sum != MaxCredits { + t.Fatalf("overflow add: %d, %v", sum, err) + } + if _, err := AddCredits(MaxCredits, -1); err == nil { + t.Fatal("negative add must be rejected") + } +} + +func TestMulPayoutBoundsAndConverts(t *testing.T) { + payout, err := MulPayout(10, 1.5) + if err != nil || payout != 15 { + t.Fatalf("MulPayout(10,1.5) = %d, %v", payout, err) + } + // Floor conversion: 10 * 0.333 = 3.33 -> 3. + if payout, _ := MulPayout(10, 0.333); payout != 3 { + t.Fatalf("floor conversion: %d", payout) + } + if payout, err := MulPayout(MaxCredits, 2); err == nil || payout != MaxCredits { + t.Fatalf("overflow payout: %d, %v", payout, err) + } + if payout, err := MulPayout(MaxCredits, 0.5); err != nil || payout != MaxCredits/2 { + t.Fatalf("half of max: %d, %v", payout, err) + } + for _, bad := range []float64{-1, inf(), nan()} { + if payout, err := MulPayout(10, bad); err == nil || payout != 0 { + t.Fatalf("multiplier %v accepted: %d, %v", bad, payout, err) + } + } + if payout, _ := MulPayout(0, 2); payout != 0 { + t.Fatalf("zero-amount payout = %d", payout) + } +} + +func TestWalletErrorMessages(t *testing.T) { + if got := walletErrorMessage(ErrInsufficientFunds, "Custom short balance."); got != "Custom short balance." { + t.Fatalf("insufficient funds rendering: %q", got) + } + if got := walletErrorMessage(ErrWalletPersistence); !errContains(got, "persistence failure") || errContains(got, "insufficient") { + t.Fatalf("persistence rendering must be distinct: %q", got) + } + if got := walletErrorMessage(ErrInsufficientFunds); got == "" { + t.Fatal("default message missing") + } +} + +func inf() float64 { + var max float64 = 1e308 + return max * 10 +} +func nan() float64 { return inf() - inf() } + +func errContains(value, fragment string) bool { + return len(value) >= len(fragment) && (value == fragment || len(fragment) == 0 || indexOf(value, fragment) >= 0) +} + +func indexOf(haystack, needle string) int { + for i := 0; i+len(needle) <= len(haystack); i++ { + if haystack[i:i+len(needle)] == needle { + return i + } + } + return -1 +} diff --git a/internal/casino/machine.go b/internal/casino/machine.go index 64dcfd4..6a7b7aa 100644 --- a/internal/casino/machine.go +++ b/internal/casino/machine.go @@ -2,6 +2,7 @@ package casino import ( "fmt" + "math" "math/rand" "sync" ) @@ -100,8 +101,8 @@ func (m *Machine) start(name string, wallet Wallet) []Event { } func (m *Machine) beginSpinLocked(name string, amount int, wallet Wallet, publicMessage, privateMessage string) []Event { - if _, ok := wallet.Wager(amount); !ok { - return m.private("You don't have enough chips and credits for that bet.") + if _, err := wallet.Wager(amount); err != nil { + return m.private(walletErrorMessage(err, "You don't have enough chips and credits for that bet.")) } m.bet = amount m.spinBet = amount @@ -203,9 +204,9 @@ func (m *Machine) autoBetTick() []Event { m.autoBet = false return m.private("Autobet stopped: no wager is set.") } - if _, ok := m.player.Wallet.Wager(m.bet); !ok { + if _, err := m.player.Wallet.Wager(m.bet); err != nil { m.autoBet = false - return m.private("Autobet stopped: you don't have enough chips and credits for that bet.") + return m.private(walletErrorMessage(err, "Autobet stopped: you don't have enough chips and credits for that bet.")) } m.spinBet = m.bet m.spin = generateSlotSpinWithProfile(m.rng, m.spinBet, m.profile) @@ -220,7 +221,12 @@ func (m *Machine) autoBetTick() []Event { func (m *Machine) finishSpin() []Event { events := []Event{} result := m.spin.Payout + m.payoutRemainder - credits := int(result) + // Checked conversion: a runaway profile or overflow must clamp at the + // economy cap instead of wrapping through a machine-sized int. + if result > MaxCredits || result != result || result < 0 { + result = MaxCredits + } + credits := int(math.Floor(result)) m.payoutRemainder = result - float64(credits) if credits > 0 && m.player.Wallet != nil { m.player.Wallet.PayCredits(credits) @@ -380,3 +386,35 @@ func (m *Machine) privateMenu(prefix string) []Event { func (m *Machine) menuEvents(prefix string) []Event { return m.privateMenu(prefix) } + +// refundAll refunds any committed stake at graceful shutdown. Locked. +func (m *Machine) refundAll() []Event { + m.mu.Lock() + defer m.mu.Unlock() + if m.player == nil || m.player.Wallet == nil || m.spinBet <= 0 { + return nil + } + name, refund := m.player.Name, m.spinBet + wallet := m.player.Wallet + m.spinBet = 0 + m.bet = 0 + if err := wallet.PayCredits(refund); err != nil { + return []Event{{RoomID: m.RoomID, TableID: m.Config.ID, Type: EventResult, + PrivateTo: name, Player: name, + PrivateMessage: "Shutdown refund could not be persisted; the stake is recorded for recovery.", + Color: "casino_action", Public: false}} + } + return []Event{{RoomID: m.RoomID, TableID: m.Config.ID, Type: EventResult, + PrivateTo: name, Player: name, + Message: fmt.Sprintf("%s is refunded %d credits as the casino closes.", name, refund), + PrivateMessage: fmt.Sprintf("The house refunds your %d credit stake for the shutdown.", refund), + Color: "casino_action", Public: true}} +} + +// idle reports whether no player is seated: only an idle machine may have +// its live rules replaced by new room configuration. +func (m *Machine) idle() bool { + m.mu.Lock() + defer m.mu.Unlock() + return m.player == nil +} diff --git a/internal/casino/machine_test.go b/internal/casino/machine_test.go index 349a452..73687b2 100644 --- a/internal/casino/machine_test.go +++ b/internal/casino/machine_test.go @@ -14,11 +14,11 @@ func TestBetRangeTextFormatsUnlimitedMax(t *testing.T) { } } -func TestMachineSetBetUnlimitedMaxMessage(t *testing.T) { +func TestMachineSetBetUnsetDefaultMaxMessage(t *testing.T) { m := newMachine(1, MachineConfig{ID: "slots", Game: GameSlots, MinBet: 1}, rand.New(rand.NewSource(7))) m.player = &Participant{Name: "alice", Connected: true} events := m.setBet("alice", 0, &testWallet{credits: 100}) - if !hasMessage(events, "Slot machine bets must be 1 or more (no maximum).") { + if !hasMessage(events, "Slot machine bets must be between 1 and 1000000.") { t.Fatalf("unexpected bet-range message: %+v", events) } } diff --git a/internal/casino/manager.go b/internal/casino/manager.go index 38b754f..78e26b0 100644 --- a/internal/casino/manager.go +++ b/internal/casino/manager.go @@ -4,6 +4,7 @@ import ( "fmt" "math/rand" "sort" + "strings" "sync" ) @@ -27,6 +28,10 @@ func tableKey(roomID int, tableID string) string { } // EnsureMachine returns nil for an invalid or unsupported configuration. +// When a machine already exists for the room/ID, its live rules are +// replaced by the new configuration ONLY while it is idle (no seated +// player, so no wager can be discarded); an occupied machine keeps its +// live rules until it becomes idle again. func (m *Manager) EnsureMachine(roomID int, cfg MachineConfig) *Machine { if cfg.Validate() != nil { return nil @@ -36,6 +41,17 @@ func (m *Manager) EnsureMachine(roomID int, cfg MachineConfig) *Machine { m.mu.Lock() defer m.mu.Unlock() if machine := m.machines[key]; machine != nil { + if machine.Config == cfg { + return machine + } + if machine.idle() { + if next := newMachine(roomID, cfg, rand.New(rand.NewSource(m.rng.Int63()))); next != nil { + m.machines[key] = next + return next + } + // Fall through to keep the live machine rather than drop a + // functioning game for an invalid replacement. + } return machine } // Only the manager consumes its RNG, under m.mu. Each game owns an @@ -46,6 +62,9 @@ func (m *Manager) EnsureMachine(roomID int, cfg MachineConfig) *Machine { } // EnsureTable returns nil for an invalid or unsupported configuration. +// When a table already exists for the room/ID, its live rules are replaced +// only while the table is idle (no seated players, so no wagers can be +// silently discarded); an occupied table keeps its live rules until idle. func (m *Manager) EnsureTable(roomID int, cfg TableConfig) *Table { if cfg.Validate() != nil { return nil @@ -55,6 +74,18 @@ func (m *Manager) EnsureTable(roomID int, cfg TableConfig) *Table { m.mu.Lock() defer m.mu.Unlock() if table := m.tables[key]; table != nil { + if table.Config == cfg { + return table + } + if table.idle() { + next := newTable(roomID, cfg, rand.New(rand.NewSource(m.rng.Int63()))) + if next != nil { + m.tables[key] = next + return next + } + // Fall through to keep the live table rather than drop a + // functioning game for an invalid replacement. + } return table } table := newTable(roomID, cfg, rand.New(rand.NewSource(m.rng.Int63()))) @@ -257,3 +288,94 @@ func (m *Manager) MachineBusy(name string) bool { } return false } + +// RefundAll resolves every outstanding wager by refunding committed stakes +// to their players. Graceful shutdown uses this: rounds cannot settle, so +// no player may lose a stake to the shutdown itself. Refunds go through the +// wallet (durable, journaled) and return the refund events for delivery. +func (m *Manager) RefundAll() []Event { + var events []Event + for _, table := range m.snapshotTables() { + events = append(events, table.refundAll()...) + } + for _, machine := range m.snapshotMachines() { + events = append(events, machine.refundAll()...) + } + return events +} + +// RekeyRooms remaps every room-keyed table and machine per idMap +// (oldID -> newID): both the manager's map keys and each game's RoomID. +// Game state (wagers, rounds) is preserved — the games continue in the +// renamed room. Uses delete-all-then-set-all so a swap cannot clobber. +func (m *Manager) RekeyRooms(idMap map[int]int) { + if len(idMap) == 0 { + return + } + m.mu.Lock() + defer m.mu.Unlock() + rekey := func(key string) (string, bool) { + var roomID int + var tableID string + if _, err := fmt.Sscanf(key, "%d:%s", &roomID, &tableID); err != nil { + return "", false + } + newID, ok := idMap[roomID] + if !ok || newID == roomID { + return "", false + } + return fmt.Sprintf("%d:%s", newID, tableID), true + } + movedTables := make(map[string]*Table, len(m.tables)) + for key, table := range m.tables { + if newKey, ok := rekey(key); ok { + var roomID int + var tableID string + fmt.Sscanf(key, "%d:%s", &roomID, &tableID) + table.RoomID = idMap[roomID] + table.game.setRoom(table.RoomID) + movedTables[newKey] = table + delete(m.tables, key) + } + } + for newKey, table := range movedTables { + m.tables[newKey] = table + } + movedMachines := make(map[string]*Machine, len(m.machines)) + for key, machine := range m.machines { + if newKey, ok := rekey(key); ok { + var roomID int + var tableID string + fmt.Sscanf(key, "%d:%s", &roomID, &tableID) + machine.RoomID = idMap[roomID] + movedMachines[newKey] = machine + delete(m.machines, key) + } + } + for newKey, machine := range movedMachines { + m.machines[newKey] = machine + } +} + +// EvictRoom removes every cached table and machine in roomID (graceful +// shutdown semantics for deleted/renumbered rooms): outstanding wagers are +// refunded through each game's refundAll before the entries drop. +func (m *Manager) EvictRoom(roomID int) []Event { + prefix := fmt.Sprintf("%d:", roomID) + var events []Event + m.mu.Lock() + for key, table := range m.tables { + if strings.HasPrefix(key, prefix) { + events = append(events, table.refundAll()...) + delete(m.tables, key) + } + } + for key, machine := range m.machines { + if strings.HasPrefix(key, prefix) { + events = append(events, machine.refundAll()...) + delete(m.machines, key) + } + } + m.mu.Unlock() + return events +} diff --git a/internal/casino/manager_test.go b/internal/casino/manager_test.go index 134798a..50cf114 100644 --- a/internal/casino/manager_test.go +++ b/internal/casino/manager_test.go @@ -6,9 +6,9 @@ type testWallet struct { chips, credits, paid int } -func (w *testWallet) Wager(amount int) (Wager, bool) { +func (w *testWallet) Wager(amount int) (Wager, error) { if w.chips+w.credits < amount { - return Wager{}, false + return Wager{}, ErrInsufficientFunds } chips := amount if chips > w.chips { @@ -17,17 +17,18 @@ func (w *testWallet) Wager(amount int) (Wager, bool) { w.chips -= chips credits := amount - chips w.credits -= credits - return Wager{Total: amount, Chips: chips, Credits: credits}, true + return Wager{Total: amount, Chips: chips, Credits: credits}, nil } -func (w *testWallet) PayCredits(amount int) { +func (w *testWallet) PayCredits(amount int) error { w.paid += amount + return nil } func TestMixedWagerUsesChipsFirst(t *testing.T) { w := &testWallet{chips: 7, credits: 50} - wager, ok := w.Wager(12) - if !ok || wager.Chips != 7 || wager.Credits != 5 || w.chips != 0 || w.credits != 45 { + wager, err := w.Wager(12) + if err != nil || wager.Chips != 7 || wager.Credits != 5 || w.chips != 0 || w.credits != 45 { t.Fatalf("unexpected wager: %+v wallet=%+v", wager, w) } } diff --git a/internal/casino/reconfig_test.go b/internal/casino/reconfig_test.go new file mode 100644 index 0000000..37b3a65 --- /dev/null +++ b/internal/casino/reconfig_test.go @@ -0,0 +1,84 @@ +package casino + +// Reconfiguration contract regressions: live rules replace only when idle; +// occupied games keep their rules (and wagers); deleted rooms evict their +// cached games with refunds. + +import "testing" + +func TestEnsureMachineReplacesIdleRules(t *testing.T) { + m := NewManager(1) + first := m.EnsureMachine(1, MachineConfig{ID: "slots", Game: GameSlots, SpinTicks: 2}) + if first == nil { + t.Fatal("machine not created") + } + // Same config: same machine. + if again := m.EnsureMachine(1, MachineConfig{ID: "slots", Game: GameSlots, SpinTicks: 2}); again != first { + t.Fatal("same config returned a different machine") + } + // Changed config while idle: live rules replaced. + changed := m.EnsureMachine(1, MachineConfig{ID: "slots", Game: GameSlots, SpinTicks: 5}) + if changed == first { + t.Fatal("idle machine did not adopt the new configuration") + } + if changed.Config.SpinTicks != 5 { + t.Fatalf("replacement config spin ticks = %d, want 5", changed.Config.SpinTicks) + } +} + +func TestEnsureMachineKeepsOccupiedRules(t *testing.T) { + m := NewManager(1) + m.EnsureMachine(1, MachineConfig{ID: "slots", Game: GameSlots, SpinTicks: 2}) + m.JoinMachine(1, MachineConfig{ID: "slots", Game: GameSlots, SpinTicks: 2}, "alice") + + // Occupied machine: config changes do NOT replace live rules. + kept := m.EnsureMachine(1, MachineConfig{ID: "slots", Game: GameSlots, SpinTicks: 9}) + if kept.Config.SpinTicks != 2 { + t.Fatalf("occupied machine replaced live rules: spin ticks %d", kept.Config.SpinTicks) + } +} + +func TestEnsureTableKeepsOccupiedRules(t *testing.T) { + m := NewManager(1) + m.Join(1, TableConfig{ID: "bj", Game: GameBlackjack, MinBet: 1, BettingTicks: 50}, "alice") + + // Occupied table: changed config does not replace live rules. + kept := m.EnsureTable(1, TableConfig{ID: "bj", Game: GameBlackjack, MinBet: 1, BettingTicks: 3}) + if kept.Config.BettingTicks != 50 { + t.Fatalf("occupied table replaced live rules: betting ticks %d", kept.Config.BettingTicks) + } +} + +func TestEnsureTableReplacesIdleRules(t *testing.T) { + m := NewManager(1) + m.Join(1, TableConfig{ID: "bj", Game: GameBlackjack, MinBet: 1, BettingTicks: 50}, "alice") + m.Leave(1, "bj", "alice") + + changed := m.EnsureTable(1, TableConfig{ID: "bj", Game: GameBlackjack, MinBet: 1, BettingTicks: 3}) + if changed.Config.BettingTicks != 3 { + t.Fatalf("idle table did not adopt new rules: betting ticks %d", changed.Config.BettingTicks) + } +} + +func TestEvictRoomRefundsAndRemovesGames(t *testing.T) { + m := NewManager(1) + w := &testWallet{credits: 100} + m.JoinMachine(1, MachineConfig{ID: "slots", Game: GameSlots, MinBet: 1}, "alice") + m.SetMachineBet(1, "slots", "alice", 10, w) + if w.credits != 90 { + t.Fatalf("wallet before evict = %d, want 90", w.credits) + } + + events := m.EvictRoom(1) + if w.paid != 10 { + t.Fatalf("outstanding wager not refunded on evict: paid %d", w.paid) + } + if len(events) == 0 { + t.Fatal("eviction produced no refund events") + } + if m.Table(1, "any") != nil { + t.Fatal("stale table survived eviction") + } + // Re-entering the room creates a fresh game. + m.JoinMachine(1, MachineConfig{ID: "slots", Game: GameSlots, MinBet: 1}, "alice") +} diff --git a/internal/casino/table.go b/internal/casino/table.go index 80ae451..09c19de 100644 --- a/internal/casino/table.go +++ b/internal/casino/table.go @@ -16,6 +16,9 @@ type tableGame interface { rebet(name string) (spot string, amount int) markDisconnected(name string) []Event hasParticipant(name string) bool + refundAll() []Event + setRoom(roomID int) + idle() bool } // tableBase carries the identity every table game needs to build events. @@ -78,3 +81,29 @@ func (t *Table) rebet(name string) (string, int) { return t.game.rebet(name) func (t *Table) markDisconnected(name string) []Event { return t.game.markDisconnected(name) } func (t *Table) hasParticipant(name string) bool { return t.game.hasParticipant(name) } + +// refundAll resolves every committed wager at this table (graceful +// shutdown): stakes return to their players through the wallet. +func (t *Table) refundAll() []Event { return t.game.refundAll() } + +// setRoom updates the table game's room for event routing after a room-ID +// rekey. Games keep their state; only their location moves. +func (b *tableBase) setRoom(roomID int) { b.RoomID = roomID } + +// idle reports whether no player is seated (or mid-round): only an idle +// table may have its live rules replaced by new room configuration. +func (t *blackjackTable) idle() bool { + t.mu.Lock() + defer t.mu.Unlock() + return t.players.count() == 0 +} + +// idle reports whether no player is seated (or mid-round). +func (t *baccaratTable) idle() bool { + t.mu.Lock() + defer t.mu.Unlock() + return t.players.count() == 0 +} + +// idle on the Table wrapper delegates to the game. +func (t *Table) idle() bool { return t.game.idle() } diff --git a/internal/casino/types.go b/internal/casino/types.go index 4d5159a..75d041b 100644 --- a/internal/casino/types.go +++ b/internal/casino/types.go @@ -2,9 +2,6 @@ package casino import "fmt" -// maxBetUnlimited is the MaxBet sentinel meaning "no wager cap". -const maxBetUnlimited = int(^uint(0) >> 1) - // betRangeText describes the accepted wager range; the unlimited sentinel is // rendered as "no maximum" instead of a machine-sized integer. func betRangeText(minBet, maxBet int) string { @@ -126,8 +123,10 @@ func (c MachineConfig) Normalize() MachineConfig { if c.MinBet <= 0 { c.MinBet = 1 } + // An unset MaxBet defaults to the economy's bounded cap; the explicit + // unlimited sentinel is still honored for authored configurations. if c.MaxBet <= 0 { - c.MaxBet = maxBetUnlimited + c.MaxBet = DefaultMaxBet } if c.SpinTicks <= 0 { c.SpinTicks = 2 @@ -152,7 +151,7 @@ func (c TableConfig) Normalize() TableConfig { c.MinBet = 1 } if c.MaxBet <= 0 { - c.MaxBet = maxBetUnlimited + c.MaxBet = DefaultMaxBet } return c } @@ -164,8 +163,14 @@ type Wager struct { } type Wallet interface { - Wager(amount int) (Wager, bool) - PayCredits(amount int) + // Wager debits the stake durably. ErrInsufficientFunds is a normal + // rejected bet; ErrWalletPersistence means the debit could not be made + // durable and the game must not proceed as if the bet succeeded. + Wager(amount int) (Wager, error) + // PayCredits pays a settlement durably, returning an error when the + // credit could not be persisted (the game must not report a win that + // is not durable). + PayCredits(amount int) error } type EventType string diff --git a/internal/engine/tick.go b/internal/engine/tick.go index 00a9c60..74b1dcf 100644 --- a/internal/engine/tick.go +++ b/internal/engine/tick.go @@ -93,6 +93,16 @@ func (e *Engine) Stop() { } } +// StopAndWait stops the ticker AND joins any in-flight callback: it waits +// until the currently running tick has completed and the loop goroutine has +// exited. Use this for graceful shutdown, where the final tick must finish +// before owned state is saved. +func (e *Engine) StopAndWait() { + e.Stop() + e.tickMu.Lock() + e.tickMu.Unlock() +} + // Tick fires every due subscriber synchronously and returns. Subscribers // are invoked in ascending subscription-ID (registration) order. Tick calls // are serialized; a callback must not call Tick recursively. diff --git a/internal/game/account_txn.go b/internal/game/account_txn.go new file mode 100644 index 0000000..488a5f6 --- /dev/null +++ b/internal/game/account_txn.go @@ -0,0 +1,380 @@ +package game + +import ( + "crypto/rand" + "encoding/hex" + "fmt" + "log" + "os" + "path/filepath" + "strings" + + "thehouseoficarus/internal/net" + "thehouseoficarus/internal/persistence" + "thehouseoficarus/internal/player" +) + +// Account/character lifecycle operations. Each is one journal transaction +// over the affected files (account list, character files), executed with the +// game's single-owner guarantee: the online check, file mutation, registry +// update, and cross-session refresh happen without any interleaved session +// input, closing the check-then-rename gap. Save/remove errors are +// propagated to the caller, never ignored. +// +// All handlers here are invoked from session input (already on the owner). + +// journalDir is the persistence journal directory under the game data dir. +func (g *Game) journalDir() string { + return filepath.Join(g.DataDir, ".journal") +} + +// newTxn starts a lifecycle transaction. +func (g *Game) newTxn() *persistence.Transaction { + return persistence.NewTransaction(g.journalDir()) +} + +// adoptCharacterIdentity backfills stable identity fields on a loaded +// character: a UUID (assigned once at first load and persisted) and the +// owning account name. Legacy characters without these fields get them at +// first connection and are re-saved so subsequent authorizations can use +// them. Called from the owner goroutine only. +func (g *Game) adoptCharacterIdentity(p *player.Player, accountName string) { + if p.ID == "" { + p.ID = newCharacterID() + } + if p.Account == "" { + p.Account = accountName + } + if err := g.AccountStore.SaveCharacter(p); err != nil { + // Identity backfill is best-effort: the character remains usable, + // but membership validation falls back to the account list. + log.Printf("identity backfill for %s: %v", p.Name, err) + } +} + +// newCharacterID returns a random UUID (v4, formatted without dashes is +// unnecessary; standard hyphenated form is used). +func newCharacterID() string { + var b [16]byte + if _, err := rand.Read(b[:]); err != nil { + panic(fmt.Sprintf("character id generation: %v", err)) + } + b[6] = (b[6] & 0x0f) | 0x40 // version 4 + b[8] = (b[8] & 0x3f) | 0x80 // RFC 4122 variant + s := hex.EncodeToString(b[:]) + return s[0:8] + "-" + s[8:12] + "-" + s[12:16] + "-" + s[16:20] + "-" + s[20:32] +} + +// backfillCharacterIDs migrates every character file at startup: assigns +// missing IDs and owner fields (owner derived from account character lists +// for legacy files) and persists them. +func (g *Game) backfillCharacterIDs() { + names, err := g.AccountStore.ListCharacterNames() + if err != nil { + return + } + owners := map[string]string{} // character (lower) -> owning account + accounts, _ := g.AccountStore.ListAccountNames() + for _, accName := range accounts { + acc, err := g.AccountStore.LoadAccount(accName) + if err != nil { + continue + } + for _, c := range acc.Characters { + owners[strings.ToLower(c)] = acc.Name + } + } + for _, name := range names { + p, err := g.AccountStore.LoadCharacter(name) + if err != nil { + log.Printf("character migration load %s: %v", name, err) + continue + } + changed := false + if p.ID == "" { + p.ID = newCharacterID() + changed = true + } + if p.Account == "" { + if owner, ok := owners[strings.ToLower(p.Name)]; ok { + p.Account = owner + changed = true + } + } + if changed { + if err := g.AccountStore.SaveCharacter(p); err != nil { + log.Printf("character migration save %s: %v", p.Name, err) + } + } + } +} + +// canonicAccount returns the account name in its canonical on-disk form. +func (g *Game) canonicalAccount(name string) (string, error) { + return g.AccountStore.FindAccount(name) +} + +// canonicChar returns the character name in its canonical on-disk form. +func (g *Game) canonicalChar(name string) (string, error) { + return g.AccountStore.FindCharacter(name) +} + +// createCharacterTxn creates the character file and appends it to the +// owning account in a single transaction. Returns the created player. +func (g *Game) createCharacterTxn(accountName, charName string, startingRoom int) (*player.Player, error) { + p := player.New(charName) + p.RoomID = startingRoom + p.Stats.RecordRoomVisit(startingRoom) + p.ID = newCharacterID() + p.Account = accountName + data, err := g.AccountStore.MarshalCharacter(p) + if err != nil { + return nil, err + } + txn := g.newTxn() + if err := txn.Replace(g.AccountStore.CharPath(charName), data, 0600); err != nil { + return nil, err + } + acc, err := g.AccountStore.LoadAccount(accountName) + if err != nil { + return nil, err + } + acc.Characters = append(acc.Characters, charName) + accData, err := g.AccountStore.MarshalAccount(acc) + if err != nil { + return nil, err + } + if err := txn.Replace(g.AccountStore.AccountPath(accountName), accData, 0600); err != nil { + return nil, err + } + return p, txn.Commit() +} + +// renameCharacterTxn atomically renames the character file, persists the new +// name inside it, and updates the owning account's character list. +func (g *Game) renameCharacterTxn(accountName, oldName, newName string) error { + txn := g.newTxn() + if err := txn.Move(g.AccountStore.CharPath(oldName), g.AccountStore.CharPath(newName)); err != nil { + return err + } + p, err := g.AccountStore.LoadCharacter(oldName) + if err != nil { + return err + } + p.Name = newName + data, err := g.AccountStore.MarshalCharacter(p) + if err != nil { + return err + } + if err := txn.Replace(g.AccountStore.CharPath(newName), data, 0600); err != nil { + return err + } + acc, err := g.AccountStore.LoadAccount(accountName) + if err != nil { + return err + } + chars := make([]string, 0, len(acc.Characters)) + for _, c := range acc.Characters { + if c == oldName { + chars = append(chars, newName) + } else { + chars = append(chars, c) + } + } + acc.Characters = chars + accData, err := g.AccountStore.MarshalAccount(acc) + if err != nil { + return err + } + if err := txn.Replace(g.AccountStore.AccountPath(accountName), accData, 0600); err != nil { + return err + } + return txn.Commit() +} + +// deleteCharacterTxn removes the character file and updates the account list +// atomically. +func (g *Game) deleteCharacterTxn(accountName, charName string) error { + txn := g.newTxn() + if err := txn.Delete(g.AccountStore.CharPath(charName)); err != nil { + return err + } + acc, err := g.AccountStore.LoadAccount(accountName) + if err != nil { + return err + } + var chars []string + for _, c := range acc.Characters { + if c != charName { + chars = append(chars, c) + } + } + acc.Characters = chars + accData, err := g.AccountStore.MarshalAccount(acc) + if err != nil { + return err + } + if err := txn.Replace(g.AccountStore.AccountPath(accountName), accData, 0600); err != nil { + return err + } + return txn.Commit() +} + +// purgeAccountTxn removes the account file and every character file in one +// transaction. +func (g *Game) purgeAccountTxn(accountName string, characters []string) error { + txn := g.newTxn() + for _, name := range characters { + if err := txn.Delete(g.AccountStore.CharPath(name)); err != nil { + return err + } + } + if err := txn.Delete(g.AccountStore.AccountPath(accountName)); err != nil { + return err + } + return txn.Commit() +} + +// renameAccountTxn moves the account file and rewrites its embedded name in +// one transaction. +func (g *Game) renameAccountTxn(oldName, newName string) error { + txn := g.newTxn() + if err := txn.Move(g.AccountStore.AccountPath(oldName), g.AccountStore.AccountPath(newName)); err != nil { + return err + } + acc, err := g.AccountStore.LoadAccount(oldName) + if err != nil { + return err + } + acc.Name = newName + accData, err := g.AccountStore.MarshalAccount(acc) + if err != nil { + return err + } + if err := txn.Replace(g.AccountStore.AccountPath(newName), accData, 0600); err != nil { + return err + } + return txn.Commit() +} + +// isMemberOf reports whether characterName belongs to accountName by +// checking the character file's owner field (falling back to the account's +// character list for legacy files), case-insensitively. +func (g *Game) isMemberOf(accountName, characterName string) bool { + p, err := g.AccountStore.LoadCharacter(characterName) + if err != nil { + return false + } + if p.Account != "" { + return equalsFold(p.Account, accountName) + } + acc, err := g.AccountStore.LoadAccount(accountName) + if err != nil { + return false + } + for _, c := range acc.Characters { + if equalsFold(c, characterName) { + return true + } + } + return false +} + +// publishAccountUpdate reloads the account from disk and republishes its +// state to every session of that account, resetting stale pending menu +// selections (pending character names, menu choices) after lifecycle +// mutations. Called from the owner goroutine only. +func (g *Game) publishAccountUpdate(accountName string) { + for _, sess := range g.allSessionsOfAccount(accountName) { + acc, err := g.AccountStore.LoadAccount(accountName) + if err != nil { + // The account file vanished (purge from another session); the + // session is stale and must leave the menu. + sess.Account = nil + sess.PendingChar = "" + sess.PendingMenu = nil + sess.WriteLine("\nYour account no longer exists.") + sess.Close() + continue + } + sess.Account = &player.Account{ + Name: acc.Name, + PasswordHash: acc.PasswordHash, + Characters: acc.Characters, + Aliases: acc.Aliases, + Colors: acc.Colors, + Options: acc.Options, + Admin: acc.Admin, + } + if sess.Account.Aliases == nil { + sess.Account.Aliases = make(map[string]string) + } + if sess.Account.Colors == nil { + sess.Account.Colors = make(map[string]string) + } + if sess.Account.Options == nil { + sess.Account.Options = make(map[string]any) + } + // Any pending selection referencing a deleted/renamed character is + // stale now; revalidate before reuse. + sess.PendingMenu = nil + if sess.PendingChar != "" && sess.PendingChar != "connect" { + found := false + for _, c := range sess.Account.Characters { + if equalsFold(c, sess.PendingChar) { + found = true + break + } + } + if !found { + sess.PendingChar = "" + sess.State = net.StateMenu + g.showMenu(sess) + } + } + } +} + +// allSessionsOfAccount returns every session authenticated to accountName, +// including sessions sitting in menus (not just in-game characters). +func (g *Game) allSessionsOfAccount(accountName string) []*net.Session { + if g.Hub == nil { + return nil + } + var out []*net.Session + for _, sess := range g.Hub.AllSessions() { + if sess.Account != nil && equalsFold(sess.Account.Name, accountName) { + out = append(out, sess) + } + } + return out +} + +// equalsFold is a small case-insensitive string equality helper. +func equalsFold(a, b string) bool { + if len(a) != len(b) { + return false + } + for i := 0; i < len(a); i++ { + ca, cb := a[i], b[i] + if 'A' <= ca && ca <= 'Z' { + ca += 'a' - 'A' + } + if 'A' <= cb && cb <= 'Z' { + cb += 'a' - 'A' + } + if ca != cb { + return false + } + } + return true +} + +// recoverJournals rolls back leftover lifecycle journals at startup and +// surfaces unrecoverable ones. +func (g *Game) recoverJournals() { + if err := persistence.Recover(g.journalDir()); err != nil { + fmt.Fprintf(os.Stderr, "lifecycle journal recovery failed: %v\n", err) + os.Exit(1) + } +} diff --git a/internal/game/baseline_race_test.go b/internal/game/baseline_race_test.go new file mode 100644 index 0000000..de5b7b4 --- /dev/null +++ b/internal/game/baseline_race_test.go @@ -0,0 +1,132 @@ +package game + +// Owner-boundary concurrency regressions. These exercise the competing +// runtime entry points the way production does (connection input, ticks, +// disconnects, admin mutations, owner-routed wallet operations) and must +// pass under -race with the single-owner design in place. + +import ( + "fmt" + "sync" + "testing" + "thehouseoficarus/internal/net" +) + +// TestBaselineRaceSessionInputTickDisconnect exercises the three competing +// runtime entry points against each other: connection-goroutine input, the +// ticker goroutine, and hub disconnect callbacks. +func TestBaselineRaceSessionInputTickDisconnect(t *testing.T) { + h := newHarness(t) + h.seedAccount("alice", "pw", false) + h.seedAccount("bob", "pw", false) + h.seedCharacter("alice", "alice") + h.seedCharacter("bob", "bob") + + aSess, _ := h.connect("alice") + bSess, _ := h.connect("bob") + + aLines := h.goInput(aSess, 1) + bLines := h.goInput(bSess, 1) + + var wg sync.WaitGroup + wg.Add(2) + go func() { + defer wg.Done() + for i := 0; i < 200; i++ { + aLines <- "look" + } + close(aLines) + }() + go func() { + defer wg.Done() + for i := 0; i < 200; i++ { + bLines <- "say hi" + } + close(bLines) + }() + + // Disconnect bob from a third goroutine while everything else runs. + go func() { + for i := 0; i < 50; i++ { + h.hub.PlayersInRoom(1) + if i == 25 { + h.hub.Remove(bSess) + } + } + }() + + wg.Wait() +} + +// TestBaselineRaceAdminRewriteWhilePlaying hammers Game.RewriteRoomIDs (the +// admin entry point) while players keep moving and ticking. +func TestBaselineRaceAdminRewriteWhilePlaying(t *testing.T) { + h := newHarness(t) + h.seedAccount("carol", "pw", false) + h.seedCharacter("carol", "carol") + sess, _ := h.connect("carol") + lines := h.goInput(sess, 1) + + var wg sync.WaitGroup + wg.Add(1) + go func() { + defer wg.Done() + for i := 0; i < 200; i++ { + lines <- fmt.Sprintf("go north") + } + close(lines) + }() + + // Concurrent admin room-ID rewrites targeting the player's live state. + for i := 0; i < 20; i++ { + h.g.RewriteRoomIDs(map[int]int{2: 2 + i + 1, 2 + i: 2 + i + 1}) + } + + wg.Wait() +} + +// TestBaselineRaceCasinoWallet drives concurrent casino activity through +// the owner boundary (as production does): wallet debits/credits from +// multiple players' session tasks plus session input, all on one owner. +func TestBaselineRaceCasinoWallet(t *testing.T) { + h := newHarness(t) + for _, n := range []string{"dave", "erin", "frank"} { + h.seedAccount(n, "pw", false) + p := h.seedCharacter(n, n) + p.Credits = 100000 + } + sessions := make([]*net.Session, 0, 3) + for _, n := range []string{"dave", "erin", "frank"} { + s, _ := h.connect(n) + sessions = append(sessions, s) + } + + var wg sync.WaitGroup + for _, s := range sessions { + wallet := casinoWallet{g: h.g, p: s.Player} + wg.Add(1) + go func() { + defer wg.Done() + for i := 0; i < 100; i++ { + h.g.RunOnOwner(func() { + wallet.Wager(1) + wallet.PayCredits(1) + }) + } + }() + + // Session input competes with the wallet operations. + lines := h.goInput(s, 1) + wg.Add(1) + go func() { + defer wg.Done() + for i := 0; i < 100; i++ { + lines <- "look" + } + close(lines) + }() + } + + wg.Wait() + h.barrier() +} diff --git a/internal/game/casino.go b/internal/game/casino.go index eb6fa4a..7442178 100644 --- a/internal/game/casino.go +++ b/internal/game/casino.go @@ -18,29 +18,76 @@ type casinoWallet struct { p *player.Player } -func (w casinoWallet) Wager(amount int) (casino.Wager, bool) { +// Wager debits the stake durably. Order of operations: durable journal +// record FIRST (with balance checkpoints), then the in-memory mutation, +// then the character save, then the settlement marker. A failed save +// reverts the in-memory mutation and voids the record: no charge is kept +// in memory, and the error distinguishes persistence failure from +// insufficient funds. +func (w casinoWallet) Wager(amount int) (casino.Wager, error) { if amount <= 0 { - return casino.Wager{}, false + return casino.Wager{}, casino.ErrInsufficientFunds } - chips := countChips(w.p) + creditsBefore := w.p.Credits + chipsBefore := countChips(w.p) + chips := chipsBefore if chips > amount { chips = amount } credits := amount - chips - if w.p.Credits < credits { - return casino.Wager{}, false + if creditsBefore < credits { + return casino.Wager{}, casino.ErrInsufficientFunds + } + rec, err := w.g.Economy.start("wager", w.p, chips, credits, + creditsBefore, creditsBefore-credits, chipsBefore, chipsBefore-chips) + if err != nil { + return casino.Wager{}, casino.ErrWalletPersistence } if chips > 0 { w.p.RemoveItem("chips", chips) } w.p.Credits -= credits - w.g.AccountStore.SaveCharacter(w.p) - return casino.Wager{Total: amount, Chips: chips, Credits: credits}, true + if err := w.g.AccountStore.SaveCharacter(w.p); err != nil { + // Revert the in-memory mutation: the play must not proceed on a + // stake that could not be made durable. + if chips > 0 { + w.g.giveChips(w.p, chips) + } + w.p.Credits += credits + w.g.Economy.void(rec) + return casino.Wager{}, casino.ErrWalletPersistence + } + w.g.Economy.settle(rec) + return casino.Wager{Total: amount, Chips: chips, Credits: credits}, nil } -func (w casinoWallet) PayCredits(amount int) { - w.p.Credits += amount - w.g.AccountStore.SaveCharacter(w.p) +// PayCredits applies a payout durably. The journal record is written first; +// if the save fails, the record stays open and startup recovery re-pays it +// (the player is owed the settlement). +func (w casinoWallet) PayCredits(amount int) error { + if amount <= 0 { + return nil + } + creditsBefore := w.p.Credits + chipsBefore := countChips(w.p) + creditsAfter, err := addCreditsChecked(creditsBefore, amount) + if err != nil { + return err + } + rec, err := w.g.Economy.start("payout", w.p, 0, amount, + creditsBefore, creditsAfter, chipsBefore, chipsBefore) + if err != nil { + return casino.ErrWalletPersistence + } + w.p.Credits = creditsAfter + if err := w.g.AccountStore.SaveCharacter(w.p); err != nil { + w.p.Credits = creditsBefore + // Keep the record open: recovery re-pays on next startup, since a + // reported win must be durable. + return casino.ErrWalletPersistence + } + w.g.Economy.settle(rec) + return nil } // casinoAffordable is the most a player can wager: chips are spent first, diff --git a/internal/game/casino_test.go b/internal/game/casino_test.go index e43eb05..026415f 100644 --- a/internal/game/casino_test.go +++ b/internal/game/casino_test.go @@ -3,6 +3,7 @@ package game import ( "strings" "testing" + "time" "thehouseoficarus/internal/casino" "thehouseoficarus/internal/color" @@ -19,6 +20,8 @@ func (c *casinoTestConn) Write(b []byte) (int, error) { c.data = append(c.data, b...) return len(b), nil } +func (c *casinoTestConn) SetWriteDeadline(time.Time) error { return nil } + func (c *casinoTestConn) Close() error { return nil } func (c *casinoTestConn) SetEcho(bool) error { return nil } diff --git a/internal/game/cmd_quit.go b/internal/game/cmd_quit.go index 0eb8fb9..80ad351 100644 --- a/internal/game/cmd_quit.go +++ b/internal/game/cmd_quit.go @@ -30,7 +30,7 @@ func (g *Game) doQuit(sess *net.Session) { g.broadcastAction(sess, "%s sits down to rest.", p.Name) ticksLeft := 10 - id := g.Ticks.Subscribe(engine.ToTicks(1), func() bool { + id := g.SubscribeTicks(engine.ToTicks(1), func() bool { switch ticksLeft { case 10: sess.WriteLine("You sit down to rest...") diff --git a/internal/game/cmd_registry.go b/internal/game/cmd_registry.go index 0fee89e..9a42d1e 100644 --- a/internal/game/cmd_registry.go +++ b/internal/game/cmd_registry.go @@ -181,7 +181,10 @@ func classifyCommand(cmd string) CommandClass { if _, ok := verbAliases[cmd]; ok { return ClassActive } - if obstacleVerbs[cmd] { + obstacleVerbsMu.RLock() + isObstacle := obstacleVerbs[cmd] + obstacleVerbsMu.RUnlock() + if isObstacle { return ClassActive } return ClassUnknown @@ -211,7 +214,10 @@ func (g *Game) executeCommand(sess *net.Session, cmd string, args []string, rawI return } - if obstacleVerbs[cmd] { + obstacleVerbsMu.RLock() + isObstacle := obstacleVerbs[cmd] + obstacleVerbsMu.RUnlock() + if isObstacle { g.doObstacle(sess, cmd) return } diff --git a/internal/game/cmd_room_remove_test.go b/internal/game/cmd_room_remove_test.go index 8bd8470..cd9569d 100644 --- a/internal/game/cmd_room_remove_test.go +++ b/internal/game/cmd_room_remove_test.go @@ -6,6 +6,7 @@ import ( "strconv" "strings" "testing" + "time" "thehouseoficarus/internal/combat" "thehouseoficarus/internal/net" @@ -21,6 +22,8 @@ func (c *removeTestConn) Write(b []byte) (int, error) { c.buf = append(c.buf, b...) return len(b), nil } +func (c *removeTestConn) SetWriteDeadline(time.Time) error { return nil } + func (c *removeTestConn) Close() error { return nil } func (c *removeTestConn) SetEcho(bool) error { return nil } diff --git a/internal/game/cmd_shutdown.go b/internal/game/cmd_shutdown.go index 0bb0f8f..0030649 100644 --- a/internal/game/cmd_shutdown.go +++ b/internal/game/cmd_shutdown.go @@ -1,14 +1,24 @@ package game +// The shutdown command schedules a graceful server shutdown. Signals and +// this command converge on Game.RequestShutdown, so both paths drain work, +// resolve outstanding wagers, and save owned state before transports close. + import ( "fmt" - "os" "strconv" "time" "thehouseoficarus/internal/net" ) +// scheduledShutdown carries one countdown run's captured cancel channel, so +// a later run (or cancel) can never close or read a replacement run's +// channel. +type scheduledShutdown struct { + cancel chan struct{} +} + func (g *Game) executeShutdown(sess *net.Session, args []string, rawInput string) { if !g.checkAdmin(sess) { sess.WriteLine("Unknown command.") @@ -21,20 +31,17 @@ func (g *Game) executeShutdown(sess *net.Session, args []string, rawInput string if args[0] == "cancel" { g.shutdownMu.Lock() - if !g.shutdownActive { - g.shutdownMu.Unlock() + run := g.shutdownRun + g.shutdownMu.Unlock() + if run == nil { sess.WriteLine("No shutdown in progress.") return } - close(g.shutdownCancel) - g.shutdownActive = false + close(run.cancel) + g.shutdownMu.Lock() + g.shutdownRun = nil g.shutdownMu.Unlock() - - for _, other := range g.Hub.AllSessions() { - if other.Player != nil { - other.WriteLine("[Server] Shutdown cancelled.") - } - } + g.broadcastLoggedIn("[Server] Shutdown cancelled.") return } @@ -45,58 +52,62 @@ func (g *Game) executeShutdown(sess *net.Session, args []string, rawInput string } g.shutdownMu.Lock() - if g.shutdownActive { + if g.shutdownRun != nil { g.shutdownMu.Unlock() sess.WriteLine("A shutdown is already in progress.") return } - g.shutdownActive = true - g.shutdownCancel = make(chan struct{}) + run := &scheduledShutdown{cancel: make(chan struct{})} + g.shutdownRun = run g.shutdownMu.Unlock() - go func() { - remaining := minutes * 60 - ticker := time.NewTicker(30 * time.Second) - defer ticker.Stop() + go g.runShutdownCountdown(run, minutes) + sess.WriteLine(fmt.Sprintf("Shutdown scheduled in %d minute(s).", minutes)) +} - for _, other := range g.Hub.AllSessions() { - if other.Player != nil { - other.WriteLine(fmt.Sprintf("[Server] Shutting down in %d minute(s).", minutes)) - } - } +// runShutdownCountdown announces the countdown and requests the shutdown +// at expiry. Cancellation is handled through the run's captured channel; +// nothing reads a replaceable shared field here. +func (g *Game) runShutdownCountdown(run *scheduledShutdown, minutes int) { + remaining := minutes * 60 + ticker := time.NewTicker(30 * time.Second) + defer ticker.Stop() - for { - select { - case <-ticker.C: - remaining -= 30 - if remaining <= 0 { - for _, other := range g.Hub.AllSessions() { - if other.Player != nil { - other.WriteLine("[Server] Shutting down NOW.") - } - } - os.Exit(0) - } - mins := remaining / 60 - secs := remaining % 60 - if secs == 0 { - for _, other := range g.Hub.AllSessions() { - if other.Player != nil { - other.WriteLine(fmt.Sprintf("[Server] Shutting down in %d minute(s).", mins)) - } - } - } else { - for _, other := range g.Hub.AllSessions() { - if other.Player != nil { - other.WriteLine(fmt.Sprintf("[Server] Shutting down in %d minute(s) %d seconds.", mins, secs)) - } - } + g.broadcastLoggedIn(fmt.Sprintf("[Server] Shutting down in %d minute(s).", minutes)) + + for { + select { + case <-ticker.C: + remaining -= 30 + if remaining <= 0 { + g.broadcastLoggedIn("[Server] Shutting down NOW.") + if g.shutdownHook != nil { + g.shutdownHook("scheduled shutdown") } - case <-g.shutdownCancel: return } + mins := remaining / 60 + secs := remaining % 60 + if secs == 0 { + g.broadcastLoggedIn(fmt.Sprintf("[Server] Shutting down in %d minute(s).", mins)) + } else { + g.broadcastLoggedIn(fmt.Sprintf("[Server] Shutting down in %d minute(s) %d seconds.", mins, secs)) + } + case <-run.cancel: + return } - }() + } +} - sess.WriteLine(fmt.Sprintf("Shutdown scheduled in %d minute(s).", minutes)) +// broadcastLoggedIn writes one line to every logged-in player. Read-only +// over Hub membership (mutex-protected); writes go through outbound queues. +func (g *Game) broadcastLoggedIn(msg string) { + if g.Hub == nil { + return + } + for _, other := range g.Hub.AllSessions() { + if other.Player != nil { + other.WriteLine(msg) + } + } } diff --git a/internal/game/combat_attack.go b/internal/game/combat_attack.go index 3544fcc..bd16999 100644 --- a/internal/game/combat_attack.go +++ b/internal/game/combat_attack.go @@ -193,7 +193,7 @@ func (g *Game) startCombat(sess *net.Session, p *player.Player, mob *world.MobIn g.broadcastAction(sess, "%s attacks %s!", p.Name, mobDisplayName(mob, true)) } - g.Ticks.Subscribe(1, func() bool { + g.SubscribeTicks(1, func() bool { cs := g.Combat.Get(p.Name) if cs == nil || !cs.Active { return false @@ -255,7 +255,7 @@ func (g *Game) startCombat(sess *net.Session, p *player.Player, mob *world.MobIn }) if !isTask { - g.Ticks.Subscribe(engine.ToTicks(mob.Speed), func() bool { + g.SubscribeTicks(engine.ToTicks(mob.Speed), func() bool { cs := g.Combat.Get(p.Name) if cs == nil || !cs.Active { return false diff --git a/internal/game/combat_mob.go b/internal/game/combat_mob.go index dabb7d7..7d60a11 100644 --- a/internal/game/combat_mob.go +++ b/internal/game/combat_mob.go @@ -318,7 +318,7 @@ func (g *Game) awardKillDrops(sess *net.Session, p *player.Player, mob *world.Mo func (g *Game) scheduleMobRespawn(mob *world.MobInstance) { instanceID := mob.InstanceID if mob.SpawnedByTrigger { - g.Ticks.Subscribe(10, func() bool { + g.SubscribeTicks(10, func() bool { g.MobStore.RemoveInstance(instanceID) return false }) @@ -328,7 +328,7 @@ func (g *Game) scheduleMobRespawn(mob *world.MobInstance) { if respawnTicks <= 0 { respawnTicks = engine.ToTicks(30) } - g.Ticks.Subscribe(respawnTicks, func() bool { + g.SubscribeTicks(respawnTicks, func() bool { g.respawnMob(instanceID) return false }) diff --git a/internal/game/core_course.go b/internal/game/core_course.go index 232e6dc..15bd47e 100644 --- a/internal/game/core_course.go +++ b/internal/game/core_course.go @@ -70,6 +70,10 @@ type ObstacleInfo struct { var obstacleVerbs = map[string]bool{} +// obstacleVerbsMu guards the global obstacle-verb set: course loading +// rebuilds it, and command classification reads it from session tasks. +var obstacleVerbsMu sync.RWMutex + var verbGerund = map[string]string{ "scramble": "scrambling", "jump": "jumping", @@ -251,5 +255,7 @@ func (cs *CourseStore) loadAllLocked() { return nil }) + obstacleVerbsMu.Lock() obstacleVerbs = localVerbs + obstacleVerbsMu.Unlock() } diff --git a/internal/game/core_login_account.go b/internal/game/core_login_account.go index 77b72d3..a846f4d 100644 --- a/internal/game/core_login_account.go +++ b/internal/game/core_login_account.go @@ -2,7 +2,6 @@ package game import ( "fmt" - "os" "strings" "thehouseoficarus/internal/color" @@ -332,34 +331,19 @@ func (g *Game) handleRenameAccount(sess *net.Session, input string) { return } - oldPath := g.AccountStore.AccountPath(oldName) - newPath := g.AccountStore.AccountPath(newName) - if err := os.Rename(oldPath, newPath); err != nil { + // One journal transaction: file move + embedded name rewrite. Any + // failure rolls back, so the account is never half-renamed. + if err := g.renameAccountTxn(oldName, newName); err != nil { sess.WriteLine(fmt.Sprintf("Error renaming account: %v", err)) g.showMenu(sess) return } - // Persist the new name inside the account file itself; otherwise the next - // login adopts the old name again and account lookup drifts apart from - // the filename. Roll the rename back if the file cannot be updated so the - // account never ends up in a half-renamed state. - acc, err := g.AccountStore.LoadAccount(newName) - if err != nil { - sess.WriteLine(fmt.Sprintf("Error renaming account: %v", err)) - os.Rename(newPath, oldPath) - g.showMenu(sess) - return - } - acc.Name = newName - if err := g.AccountStore.SaveAccount(acc); err != nil { - sess.WriteLine(fmt.Sprintf("Error renaming account: %v", err)) - os.Rename(newPath, oldPath) - g.showMenu(sess) - return - } + // Republish account state to EVERY session of this account: other + // authenticated sessions must not retain stale account objects or + // pending menu selections bound to the old name. + g.publishAccountUpdate(newName) - sess.Account.Name = newName sess.WriteLine(fmt.Sprintf("Account renamed to %s.", newName)) g.showMenu(sess) } @@ -373,8 +357,10 @@ func (g *Game) handlePurgeAccount(sess *net.Session, input string) { return } - // Refuse to purge while any character is connected: a live session would - // keep playing (and saving) a character whose file is about to vanish. + // Refuse to purge while any character is connected: a live session + // would keep playing (and saving) a character whose file is about to + // vanish. The online check and the purge run as one owner step, so no + // connection can start a character between check and deletion. for _, name := range sess.Account.Characters { if g.characterOnline(name) { sess.WriteLine(fmt.Sprintf("%s is logged in. Log all characters out before purging.", name)) @@ -383,12 +369,24 @@ func (g *Game) handlePurgeAccount(sess *net.Session, input string) { } } - for _, name := range sess.Account.Characters { - os.Remove(g.AccountStore.CharPath(name)) + if err := g.purgeAccountTxn(sess.Account.Name, sess.Account.Characters); err != nil { + sess.WriteLine(fmt.Sprintf("Error purging account: %v", err)) + g.showMenu(sess) + return } - os.Remove(g.AccountStore.AccountPath(sess.Account.Name)) - sess.WriteLine(fmt.Sprintf("Account %s has been purged. Thanks for playing.", sess.Account.Name)) + // Other authenticated sessions of this account are stale now; tell + // them and close them (they cannot safely keep using cached state). + for _, other := range g.allSessionsOfAccount(sess.Account.Name) { + if other == sess { + continue + } + other.Account = nil + other.PendingChar = "" + other.PendingMenu = nil + other.WriteLine("\nYour account has been purged from another session.") + other.Close() + } sess.Close() } diff --git a/internal/game/core_login_char.go b/internal/game/core_login_char.go index 9d7c211..68a0d58 100644 --- a/internal/game/core_login_char.go +++ b/internal/game/core_login_char.go @@ -2,7 +2,6 @@ package game import ( "fmt" - "os" "strings" "thehouseoficarus/internal/net" @@ -34,33 +33,26 @@ func (g *Game) handleNewCharName(sess *net.Session, input string) { return } + // Everything below runs on the owner goroutine (session input): the + // existence check, file creation, and account publication are one + // transaction with no interleaved session input, so a concurrent + // creation of the same name cannot slip through the check. if g.AccountStore.CharacterExists(name) { sess.WriteLine("A character with that name already exists.") sess.Write("Character name: ") return } - p := player.New(name) - p.RoomID = g.StartingRoom - p.Stats.RecordRoomVisit(g.StartingRoom) - - if err := g.AccountStore.SaveCharacter(p); err != nil { - sess.WriteLine(fmt.Sprintf("Error creating character: %v", err)) - sess.State = net.StateMenu - g.showMenu(sess) - return - } - - acc, err := g.AccountStore.LoadAccount(sess.Account.Name) + p, err := g.createCharacterTxn(sess.Account.Name, name, g.StartingRoom) if err != nil { sess.WriteLine(fmt.Sprintf("Error creating character: %v", err)) sess.State = net.StateMenu g.showMenu(sess) return } - acc.Characters = append(acc.Characters, name) - g.AccountStore.SaveAccount(acc) - sess.Account.Characters = acc.Characters + _ = p + sess.Account.Characters = append(sess.Account.Characters, name) + g.publishAccountUpdate(sess.Account.Name) g.connectCharacter(sess, name) @@ -70,6 +62,29 @@ func (g *Game) handleNewCharName(sess *net.Session, input string) { } func (g *Game) connectCharacter(sess *net.Session, name string) { + // This whole function runs as one owner step (invoked from session + // input): membership validation, the online check, registry + // publication, and room entry are serialized against every other + // lifecycle operation, closing the check-then-register gap. + + // Authorize by ownership, not just existence: the character file's + // owner must match this account, so a stale cached name cannot connect + // a character reassigned to another account. + actualName, err := g.canonicalChar(name) + if err != nil { + sess.WriteLine(fmt.Sprintf("Error loading character: %v", err)) + sess.State = net.StateMenu + g.showMenu(sess) + return + } + if !g.isMemberOf(sess.Account.Name, actualName) { + sess.WriteLine("That character does not belong to this account.") + sess.State = net.StateMenu + g.showMenu(sess) + return + } + name = actualName + g.charsMu.Lock() if existing := g.loggedInChars[name]; existing != nil { g.charsMu.Unlock() @@ -87,6 +102,7 @@ func (g *Game) connectCharacter(sess *net.Session, name string) { g.showMenu(sess) return } + g.adoptCharacterIdentity(p, sess.Account.Name) sess.Player = p sess.State = net.StateGame @@ -171,49 +187,45 @@ func (g *Game) handleRenameCharName(sess *net.Session, input string) { g.showMenu(sess) return } - if g.AccountStore.CharacterExists(newName) { - sess.WriteLine("A character with that name already exists.") - sess.Write("New name: ") + + // Checks and mutation run as one owner step: no session input can + // interleave between the membership/online checks and the file + // rename, and the rename is a single journal transaction. + actualOld, err := g.canonicalChar(oldName) + if err != nil { + sess.WriteLine(fmt.Sprintf("Error renaming: %v", err)) + g.showMenu(sess) return } - if g.characterOnline(oldName) { - sess.WriteLine("That character is logged in elsewhere. Log them out first.") + if !g.isMemberOf(sess.Account.Name, actualOld) { + sess.WriteLine(fmt.Sprintf("No character named %s on this account.", oldName)) sess.PendingChar = "" g.showMenu(sess) return } - - oldPath := g.AccountStore.CharPath(oldName) - newPath := g.AccountStore.CharPath(newName) - if err := os.Rename(oldPath, newPath); err != nil { - sess.WriteLine(fmt.Sprintf("Error renaming: %v", err)) - g.showMenu(sess) + if newName != actualOld && g.AccountStore.CharacterExists(newName) { + sess.WriteLine("A character with that name already exists.") + sess.Write("New name: ") return } - - p, _ := g.AccountStore.LoadCharacter(newName) - if p != nil { - p.Name = newName - g.AccountStore.SaveCharacter(p) + if g.characterOnline(actualOld) { + sess.WriteLine("That character is logged in elsewhere. Log them out first.") + sess.PendingChar = "" + g.showMenu(sess) + return } - acc, err := g.AccountStore.LoadAccount(sess.Account.Name) - if err != nil { + if err := g.renameCharacterTxn(sess.Account.Name, actualOld, newName); err != nil { sess.WriteLine(fmt.Sprintf("Error renaming: %v", err)) g.showMenu(sess) return } - for i, c := range acc.Characters { - if c == oldName { - acc.Characters[i] = newName - break - } - } - g.AccountStore.SaveAccount(acc) - sess.Account.Characters = acc.Characters sess.PendingChar = "" - sess.WriteLine(fmt.Sprintf("%s renamed to %s.", oldName, newName)) + sess.WriteLine(fmt.Sprintf("%s renamed to %s.", actualOld, newName)) + // Refresh every session of this account so no stale character list or + // pending selection survives the rename. + g.publishAccountUpdate(sess.Account.Name) g.showMenu(sess) } @@ -256,39 +268,37 @@ func (g *Game) handleDeleteChar(sess *net.Session, input string) { return } - if g.characterOnline(sess.PendingChar) { - sess.WriteLine("That character is logged in elsewhere. Log them out first.") + // Checks and mutation run as one owner step; the delete is a single + // journal transaction (character file + account list). + actualName, err := g.canonicalChar(sess.PendingChar) + if err != nil { + sess.WriteLine(fmt.Sprintf("Error deleting: %v", err)) sess.PendingChar = "" g.showMenu(sess) return } - - charPath := g.AccountStore.CharPath(sess.PendingChar) - if err := os.Remove(charPath); err != nil { - sess.WriteLine(fmt.Sprintf("Error deleting: %v", err)) + if !g.isMemberOf(sess.Account.Name, actualName) { + sess.WriteLine(fmt.Sprintf("No character named %s on this account.", sess.PendingChar)) + sess.PendingChar = "" + g.showMenu(sess) + return + } + if g.characterOnline(actualName) { + sess.WriteLine("That character is logged in elsewhere. Log them out first.") sess.PendingChar = "" g.showMenu(sess) return } - acc, err := g.AccountStore.LoadAccount(sess.Account.Name) - if err != nil { + if err := g.deleteCharacterTxn(sess.Account.Name, actualName); err != nil { sess.WriteLine(fmt.Sprintf("Error deleting: %v", err)) sess.PendingChar = "" g.showMenu(sess) return } - var newChars []string - for _, c := range acc.Characters { - if c != sess.PendingChar { - newChars = append(newChars, c) - } - } - acc.Characters = newChars - g.AccountStore.SaveAccount(acc) - sess.Account.Characters = newChars - sess.WriteLine(fmt.Sprintf("%s has been deleted.", sess.PendingChar)) + sess.WriteLine(fmt.Sprintf("%s has been deleted.", actualName)) sess.PendingChar = "" + g.publishAccountUpdate(sess.Account.Name) g.showMenu(sess) } diff --git a/internal/game/economy.go b/internal/game/economy.go new file mode 100644 index 0000000..90c24ed --- /dev/null +++ b/internal/game/economy.go @@ -0,0 +1,289 @@ +package game + +// The economy journal: durable, idempotent money movement. Every casino +// wager and payout writes a journal record BEFORE the balance mutation, +// then the character save makes the balance durable, then the record is +// settled. A crash at any point leaves either: +// - an open record with the character file at its pre-transaction state +// (crash before save) — recovery settles it as void (debit) or applies +// it (payout, which the player is owed); or +// - an open record with the character file at its post-transaction state +// (crash after save) — recovery refunds open debit records (the round +// state was lost in the crash, so the stake returns) and voids payout +// records that are already durable. +// +// The reducer folds journal lines by record ID (later lines win), so +// settlement markers are idempotent appends. Recovery runs once at startup +// before any session can join; compaction rewrites the journal keeping only +// open records (normally none). + +import ( + "bufio" + "crypto/rand" + "encoding/hex" + "encoding/json" + "fmt" + "os" + "path/filepath" + "time" + + "thehouseoficarus/internal/player" +) + +type economyRecord struct { + ID string `json:"id"` + Player string `json:"player"` + Kind string `json:"kind"` // "wager" or "payout" + Chips int `json:"chips"` + Credit int `json:"credits"` + Status string `json:"status"` // "open", "settled", "void" + TS time.Time `json:"ts"` + + // Balance checkpoints used to decide recovery compensation. + CreditsBefore int `json:"credits_before"` + CreditsAfter int `json:"credits_after"` + ChipsBefore int `json:"chips_before"` + ChipsAfter int `json:"chips_after"` +} + +type economyJournal struct { + path string +} + +func newEconomyJournal(dataDir string) *economyJournal { + dir := filepath.Join(dataDir, "economy") + _ = os.MkdirAll(dir, 0700) + return &economyJournal{path: filepath.Join(dir, "journal.jsonl")} +} + +func newEconomyRecordID() string { + var b [8]byte + if _, err := rand.Read(b[:]); err != nil { + panic(fmt.Sprintf("economy record id: %v", err)) + } + return hex.EncodeToString(b[:]) +} + +// append writes one durable line. Errors are returned to the caller, which +// must treat the transaction as failed. +func (j *economyJournal) append(rec economyRecord) error { + rec.TS = time.Now() + data, err := json.Marshal(rec) + if err != nil { + return err + } + f, err := os.OpenFile(j.path, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0600) + if err != nil { + return fmt.Errorf("open economy journal: %w", err) + } + defer f.Close() + if _, err := f.Write(append(data, '\n')); err != nil { + return fmt.Errorf("append economy journal: %w", err) + } + if err := f.Sync(); err != nil { + return fmt.Errorf("sync economy journal: %w", err) + } + return nil +} + +// start opens a durable record for the in-flight transaction. +func (j *economyJournal) start(kind string, p *player.Player, chips, credits, creditsBefore, creditsAfter, chipsBefore, chipsAfter int) (*economyRecord, error) { + rec := &economyRecord{ + ID: newEconomyRecordID(), + Player: p.Name, + Kind: kind, + Chips: chips, + Credit: credits, + Status: "open", + CreditsBefore: creditsBefore, + CreditsAfter: creditsAfter, + ChipsBefore: chipsBefore, + ChipsAfter: chipsAfter, + } + if err := j.append(*rec); err != nil { + return nil, err + } + return rec, nil +} + +// settle marks a record settled (the balance mutation is durable). +func (j *economyJournal) settle(rec *economyRecord) { + rec.Status = "settled" + _ = j.append(*rec) +} + +// void marks a record void (the mutation was reverted in memory and never +// became durable; recovery must not compensate). +func (j *economyJournal) void(rec *economyRecord) { + rec.Status = "void" + _ = j.append(*rec) +} + +// loadOpen folds the journal and returns the still-open records. +func (j *economyJournal) loadOpen() []*economyRecord { + f, err := os.Open(j.path) + if err != nil { + return nil + } + defer f.Close() + latest := map[string]*economyRecord{} + scanner := bufio.NewScanner(f) + scanner.Buffer(make([]byte, 0, 64*1024), 1024*1024) + for scanner.Scan() { + line := scanner.Bytes() + if len(line) == 0 { + continue + } + var rec economyRecord + if err := json.Unmarshal(line, &rec); err != nil { + // A torn final line (crash mid-append) is not a valid record; + // skip it. Appends are small and single-line, so only the last + // line can be torn. + continue + } + latest[rec.ID] = &rec + } + var open []*economyRecord + for _, rec := range latest { + if rec.Status == "open" { + open = append(open, rec) + } + } + return open +} + +// compact rewrites the journal keeping only still-open records. +func (j *economyJournal) compact() { + open := j.loadOpen() + tmp := j.path + ".compact" + f, err := os.OpenFile(tmp, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0600) + if err != nil { + return + } + defer os.Remove(tmp) + w := bufio.NewWriter(f) + for _, rec := range open { + rec.TS = time.Now() + data, err := json.Marshal(rec) + if err != nil { + continue + } + w.Write(data) + w.WriteByte('\n') + } + if err := w.Flush(); err != nil { + f.Close() + return + } + if err := f.Sync(); err != nil { + f.Close() + return + } + f.Close() + // Best-effort atomic-ish replacement; a torn compaction leaves the + // original file intact. + if err := os.Rename(tmp, j.path); err != nil { + os.Remove(tmp) + } +} + +// giveChips re-credits chips to a player's inventory (used by recovery +// refunds). Stacks onto an existing chips slot when one exists. +func (g *Game) giveChips(p *player.Player, qty int) { + for _, slot := range p.Inventory { + if slot != nil && slot.ItemID == "chips" { + slot.Quantity += qty + return + } + } + slot := p.FirstFreeSlot() + if slot >= 0 { + p.SetInvSlot(slot, &player.InventorySlot{ItemID: "chips", Quantity: qty}) + } +} + +// payCreditsDurable applies a checked credit increase and persists it. +func (g *Game) payCreditsDurable(p *player.Player, amount int) error { + if amount < 0 { + return fmt.Errorf("negative payout %d", amount) + } + sum, err := addCreditsChecked(p.Credits, amount) + if err != nil { + return err + } + p.Credits = sum + return g.AccountStore.SaveCharacter(p) +} + +// addCreditsChecked is the game-side checked addition against the economy +// cap (mirrors casino.MaxCredits; kept local to avoid a dependency cycle). +func addCreditsChecked(a, b int) (int, error) { + const maxCredits = 1_000_000_000_000 + if b > 0 && a > maxCredits-b { + return maxCredits, fmt.Errorf("credit overflow: %d + %d exceeds %d", a, b, maxCredits) + } + if b < 0 { + return 0, fmt.Errorf("negative credit add %d", b) + } + return a + b, nil +} + +// recoverEconomy compensates open records from the last crash. It is called +// once from New, before any session connects. +func (g *Game) recoverEconomy() { + if g.Economy == nil { + return + } + records := g.Economy.loadOpen() + compensated := 0 + for _, rec := range records { + p, err := g.AccountStore.LoadCharacter(rec.Player) + if err != nil { + // Character file unreadable: leave the record open so a later + // attempt (or operator inspection) can resolve it; do not + // silently drop money. + fmt.Fprintf(os.Stderr, "economy recovery: load %s: %v\n", rec.Player, err) + continue + } + switch { + case rec.Kind == "wager" && p.Credits == rec.CreditsBefore: + // The debit never became durable: void it. + g.Economy.void(rec) + case rec.Kind == "wager" && p.Credits == rec.CreditsAfter: + // The debit is durable but the round was lost in the crash: + // refund the stake (approved refund-on-recovery policy). + chips, credits := rec.Chips, rec.Credit + p.Credits += credits + if chips > 0 { + g.giveChips(p, chips) + } + if err := g.AccountStore.SaveCharacter(p); err != nil { + fmt.Fprintf(os.Stderr, "economy recovery: refund save %s: %v\n", p.Name, err) + continue + } + g.Economy.settle(rec) + compensated++ + case rec.Kind == "wager": + // Balance moved past the recorded checkpoint: cannot compensate + // reliably; surface for operator inspection. + fmt.Fprintf(os.Stderr, "economy recovery: ambiguous wager %s for %s (credits %d, want before %d or after %d)\n", + rec.ID, rec.Player, p.Credits, rec.CreditsBefore, rec.CreditsAfter) + g.Economy.void(rec) + case rec.Kind == "payout" && p.Credits == rec.CreditsBefore: + // The payout was lost in the crash: the player is owed it. + if err := g.payCreditsDurable(p, rec.Credit); err != nil { + fmt.Fprintf(os.Stderr, "economy recovery: payout save %s: %v\n", p.Name, err) + continue + } + g.Economy.settle(rec) + compensated++ + case rec.Kind == "payout" && p.Credits == rec.CreditsAfter: + g.Economy.settle(rec) + case rec.Kind == "payout": + fmt.Fprintf(os.Stderr, "economy recovery: ambiguous payout %s for %s (credits %d, want before %d or after %d)\n", + rec.ID, rec.Player, p.Credits, rec.CreditsBefore, rec.CreditsAfter) + g.Economy.settle(rec) + } + } + g.Economy.compact() +} diff --git a/internal/game/economy_shutdown_test.go b/internal/game/economy_shutdown_test.go new file mode 100644 index 0000000..bd6c253 --- /dev/null +++ b/internal/game/economy_shutdown_test.go @@ -0,0 +1,263 @@ +package game + +// Durable-money and graceful-shutdown regressions: journal-first wagers +// with revert on save failure, startup recovery that refunds lost wagers +// and re-pays lost payouts, checked currency arithmetic, and a shutdown +// that persists online characters. + +import ( + "encoding/json" + "os" + "path/filepath" + "testing" + "time" + + "thehouseoficarus/internal/casino" + "thehouseoficarus/internal/config" + "thehouseoficarus/internal/net" + "thehouseoficarus/internal/player" + + "gopkg.in/yaml.v3" +) + +// harnessFromDir builds a Game over an existing directory (used when tests +// must pre-seed character files and journal records before startup). +func harnessFromDir(t *testing.T, dir string) *harness { + t.Helper() + colors := &config.ColorsConfig{} + g := New(dir, colors, colors, config.ValidationConfig{}, 1) + hub := net.NewHub() + g.SetHub(hub) + stopTick := make(chan struct{}) + done := make(chan struct{}) + go func() { + defer close(done) + tick := time.NewTicker(2 * time.Millisecond) + defer tick.Stop() + for { + select { + case <-tick.C: + g.Ticks.Tick() + case <-stopTick: + return + } + } + }() + h := &harness{ + t: t, dir: dir, g: g, hub: hub, + sessions: make(map[string]*net.Session), + conns: make(map[string]*chanConn), + } + h.stop = func() { close(stopTick); <-done } + t.Cleanup(h.stop) + return h +} + +func writeSeedFile(t *testing.T, path string, data []byte) { + t.Helper() + if err := os.MkdirAll(filepath.Dir(path), 0755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, data, 0600); err != nil { + t.Fatal(err) + } +} + +func writeEconomyJournal(t *testing.T, dir string, records []economyRecord) { + t.Helper() + if err := os.MkdirAll(filepath.Join(dir, "economy"), 0700); err != nil { + t.Fatal(err) + } + var data []byte + for _, rec := range records { + line, err := json.Marshal(rec) + if err != nil { + t.Fatal(err) + } + data = append(data, line...) + } + if err := os.WriteFile(filepath.Join(dir, "economy", "journal.jsonl"), data, 0600); err != nil { + t.Fatal(err) + } +} + +func TestEconomyWagerDebitsDurallyAndSettles(t *testing.T) { + h := newHarness(t) + h.seedAccount("alice", "pw", false) + p := h.seedCharacter("alice", "alice") + h.g.RunOnOwner(func() { + p.Credits = 100 + }) + w := casinoWallet{g: h.g, p: p} + + var wagerErr error + h.g.RunOnOwner(func() { + _, wagerErr = w.Wager(10) + }) + h.barrier() + if wagerErr != nil { + t.Fatalf("wager failed: %v", wagerErr) + } + if p.Credits != 90 { + t.Fatalf("credits after wager = %d, want 90", p.Credits) + } + // The journal must show the record settled (debit is durable). + if open := h.g.Economy.loadOpen(); len(open) != 0 { + t.Fatalf("open records remain after a successful wager: %v", open) + } +} + +func TestEconomyFailedSaveRevertsAndVoids(t *testing.T) { + h := newHarness(t) + h.seedAccount("alice", "pw", false) + p := h.seedCharacter("alice", "alice") + h.g.RunOnOwner(func() { + p.Credits = 100 + }) + + // Make character saves fail: read-only characters directory. + charDir := filepath.Join(h.dir, "players", "characters") + if err := os.Chmod(charDir, 0500); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { os.Chmod(charDir, 0700) }) + + var wagerErr error + h.g.RunOnOwner(func() { + _, wagerErr = casinoWallet{g: h.g, p: p}.Wager(10) + }) + h.barrier() + if wagerErr != casino.ErrWalletPersistence { + t.Fatalf("wager error = %v, want ErrWalletPersistence", wagerErr) + } + // The in-memory mutation must have been reverted: nothing was charged. + if p.Credits != 100 { + t.Fatalf("credits after failed wager = %d, want 100 (reverted)", p.Credits) + } + // The record is void, not open: recovery must not compensate. + os.Chmod(charDir, 0700) + if open := h.g.Economy.loadOpen(); len(open) != 0 { + t.Fatalf("void record remained open: %v", open) + } +} + +func TestEconomyRecoveryRefundsDurableLostWager(t *testing.T) { + dir := t.TempDir() + seedWorld(t, dir) + + // Character with the debit already durable (crash after save). + hash, _ := player.HashPassword("pw") + accData, _ := yaml.Marshal(&player.Account{Name: "alice", PasswordHash: hash}) + writeSeedFile(t, filepath.Join(dir, "players", "accounts", "alice.yaml"), accData) + p := player.New("alice") + p.Credits = 90 // post-debit balance + p.Account = "alice" + pData, _ := yaml.Marshal(p) + writeSeedFile(t, filepath.Join(dir, "players", "characters", "alice.yaml"), pData) + + // Open wager record: 10 credits debited (before=100, after=90). + writeEconomyJournal(t, dir, []economyRecord{{ + ID: "lost-wager", Player: "alice", Kind: "wager", + Chips: 0, Credit: 10, Status: "open", + CreditsBefore: 100, CreditsAfter: 90, + ChipsBefore: 0, ChipsAfter: 0, + }}) + + h := harnessFromDir(t, dir) + // Recovery must refund the stake: credits back to 100. + loaded, err := h.g.AccountStore.LoadCharacter("alice") + if err != nil { + t.Fatal(err) + } + if loaded.Credits != 100 { + t.Fatalf("credits after recovery = %d, want 100 (refund)", loaded.Credits) + } + if open := h.g.Economy.loadOpen(); len(open) != 0 { + t.Fatalf("recovered record left open: %v", open) + } +} + +func TestEconomyRecoveryAppliesLostPayout(t *testing.T) { + dir := t.TempDir() + seedWorld(t, dir) + + // Character at pre-payout balance; the payout was journaled but the + // save never happened (crash before save). The player is owed it. + hash, _ := player.HashPassword("pw") + accData, _ := yaml.Marshal(&player.Account{Name: "bob", PasswordHash: hash}) + writeSeedFile(t, filepath.Join(dir, "players", "accounts", "bob.yaml"), accData) + p := player.New("bob") + p.Credits = 50 + p.Account = "bob" + pData, _ := yaml.Marshal(p) + writeSeedFile(t, filepath.Join(dir, "players", "characters", "bob.yaml"), pData) + + writeEconomyJournal(t, dir, []economyRecord{{ + ID: "lost-payout", Player: "bob", Kind: "payout", + Chips: 0, Credit: 25, Status: "open", + CreditsBefore: 50, CreditsAfter: 75, + ChipsBefore: 0, ChipsAfter: 0, + }}) + + h := harnessFromDir(t, dir) + loaded, err := h.g.AccountStore.LoadCharacter("bob") + if err != nil { + t.Fatal(err) + } + if loaded.Credits != 75 { + t.Fatalf("credits after recovery = %d, want 75 (payout re-applied)", loaded.Credits) + } + if open := h.g.Economy.loadOpen(); len(open) != 0 { + t.Fatalf("recovered record left open: %v", open) + } +} + +func TestShutdownSavesOnlineCharactersAndRefundsWagers(t *testing.T) { + h := newHarness(t) + h.seedAccount("alice", "pw", false) + h.seedAccount("bob", "pw", false) + h.seedCharacter("alice", "alice") + h.seedCharacter("bob", "bob") + sA, _ := h.connect("alice") + pA := sA.Player + sB, _ := h.connect("bob") + pB := sB.Player + + // bob sits at a machine with a committed stake; shutdown must refund. + h.g.RunOnOwner(func() { + pB.Credits = 100 + h.g.Casino.JoinMachine(1, casino.MachineConfig{ID: "slots", Game: casino.GameSlots, MinBet: 1}, "bob") + h.g.Casino.SetMachineBet(1, "slots", "bob", 20, casinoWallet{g: h.g, p: pB}) + }) + h.barrier() + if pB.Credits != 80 { + t.Fatalf("bob's credits after wager = %d, want 80", pB.Credits) + } + + transportClosed := make(chan struct{}) + h.g.SetTransportStop(func() { close(transportClosed) }) + h.g.RequestShutdown("test") + // The shutdown goroutine closes the transports at the end of the + // ordered sequence; block for it. + select { + case <-transportClosed: + case <-time.After(5 * time.Second): + t.Fatal("transport stop was never invoked during shutdown") + } + + // Both characters must be saved with the wager refunded. + loadedA, err := h.g.AccountStore.LoadCharacter("alice") + if err != nil { + t.Fatal(err) + } + if loadedA.Credits != pA.Credits { + t.Fatalf("alice save mismatch: file %d, memory %d", loadedA.Credits, pA.Credits) + } + loadedB, err := h.g.AccountStore.LoadCharacter("bob") + if err != nil { + t.Fatal(err) + } + if loadedB.Credits != 100 { + t.Fatalf("bob's file = %d, want 100 (stake refunded at shutdown)", loadedB.Credits) + } +} diff --git a/internal/game/exit_migrate.go b/internal/game/exit_migrate.go index ff905f6..db70c8d 100644 --- a/internal/game/exit_migrate.go +++ b/internal/game/exit_migrate.go @@ -112,11 +112,11 @@ func rekeyIntMap[V any](m map[int]V, idMap map[int]int) bool { return true } -// RewriteRoomIDs is the shared entry point called by every room-ID change path -// (the swapid command and the admin web GUI rename) to migrate room-ID-embedded +// RewriteRoomIDs is the shared entry point called by every room-ID change +// path (the swapid command and the admin web GUI rename) to migrate room-ID-embedded // state across all characters. Online players are updated in memory (and -// re-saved); offline characters are loaded, rewritten, and saved. It follows -// the same cross-goroutine mutation precedent as cmd_undig/cmd_summon. +// re-saved); offline characters are loaded, rewritten, and saved. Callers +// must be on the owner goroutine (use MigrateRoomIDs from other goroutines). func (g *Game) RewriteRoomIDs(idMap map[int]int) { if len(idMap) == 0 { return @@ -150,3 +150,34 @@ func (g *Game) RewriteRoomIDs(idMap map[int]int) { } } } + +// MigrateRoomIDs is the complete room-ID migration entry point for other +// goroutines (the admin API and swapid command): player state, Hub room +// buckets, casino room keys, world room-keyed runtime state, and live mob +// instances all move together under one owner task. This is what makes a +// live rename leave NO gameplay or broadcast attached to the old location. +func (g *Game) MigrateRoomIDs(idMap map[int]int) { + if len(idMap) == 0 { + return + } + g.RunOnOwner(func() { + g.migrateRoomIDsLocked(idMap) + }) +} + +// migrateRoomIDsLocked performs the full runtime migration; caller must +// already be on the owner goroutine. +func (g *Game) migrateRoomIDsLocked(idMap map[int]int) { + if g.Hub != nil { + g.Hub.RekeyRooms(idMap) + } + if g.Casino != nil { + g.Casino.RekeyRooms(idMap) + } + g.World.RekeyRooms(idMap) + g.MobStore.RekeyRooms(idMap) + // The file paths changed under the world: rebuild its room index so + // lookups agree with the post-migration layout. + g.World.RebuildRoomIndex(g.DataDir) + g.RewriteRoomIDs(idMap) +} diff --git a/internal/game/game.go b/internal/game/game.go index 822042e..7f29d1f 100644 --- a/internal/game/game.go +++ b/internal/game/game.go @@ -2,6 +2,7 @@ package game import ( "fmt" + "log" "path/filepath" "sort" "strings" @@ -76,9 +77,26 @@ type Game struct { sequences map[string]*sequence globalSeqs []*sequence - shutdownCancel chan struct{} - shutdownActive bool - shutdownMu sync.Mutex + shutdownMu sync.Mutex + + // shutdownOnce guards RequestShutdown; transportStop is the registered + // hook that closes the network transports (set by main at startup). + shutdownOnce sync.Once + shutdownRun *scheduledShutdown + shutdownHook func(reason string) + transportStop func() + + // Economy is the durable money journal; see economy.go. + Economy *economyJournal + + // owner serializes all live-state mutations; see owner.go. + owner *owner + ownerStop sync.Once + + // shuttingDown disables disconnect forfeit/save callbacks while the + // graceful shutdown sequence owns final persistence. + shuttingDownMu sync.Mutex + shuttingDown bool } func New(dataDir string, colorConfig *config.ColorsConfig, constantColorConfig *config.ColorsConfig, valConfig config.ValidationConfig, startingRoom int) *Game { @@ -111,12 +129,25 @@ func New(dataDir string, colorConfig *config.ColorsConfig, constantColorConfig * hackingStates: make(map[string]*hacking.Session), sequences: make(map[string]*sequence), } + g.owner = newOwner() + // Runtime assignment (not initializer-level) so the command registry, + // which statically references executeShutdown, cannot form an + // initialization cycle with the shutdown coordinator. + g.shutdownHook = g.RequestShutdown + g.Economy = newEconomyJournal(dataDir) + // Compensate open money records from any prior crash before any + // session can connect. + g.recoverEconomy() g.CourseStore.SetWorld(g.World) g.CourseStore.LoadAll() g.LoadMods() g.LoadTechs() g.buildCraftIndex() g.loadAllFlagTriggers() + // Journal recovery before any store reads files a leftover transaction + // may reference, then one-time stable-identity migration. + g.recoverJournals() + g.backfillCharacterIDs() g.ValidateAndLog() return g } @@ -124,30 +155,141 @@ func New(dataDir string, colorConfig *config.ColorsConfig, constantColorConfig * func (g *Game) SetHub(hub *net.Hub) { g.Hub = hub hub.OnRemove(func(sess *net.Session) { - if p := sess.Player; p != nil { - g.restoreGodPlayer(p) - g.persistLockedOnDisconnect(p) - g.AccountStore.SaveCharacter(p) - p.DeactivateAllTechs() - g.charsMu.Lock() - delete(g.loggedInChars, p.Name) - g.charsMu.Unlock() - delete(g.hackingStates, p.Name) - if ss, ok := g.safespot.Get(p.Name); ok { - g.forceLeaveSafespot(sess, p, &ss, "") + // Runs on the connection/tick goroutine that removed the session; + // the callback itself mutates live game state, so it executes on + // the owner. During shutdown this callback is suppressed: the + // shutdown sequence has already persisted everyone. + g.run(func() { + if g.isShuttingDown() { + return } - } + if p := sess.Player; p != nil { + g.restoreGodPlayer(p) + g.persistLockedOnDisconnect(p) + g.AccountStore.SaveCharacter(p) + p.DeactivateAllTechs() + g.detachSession(p.Name) + delete(g.hackingStates, p.Name) + if ss, ok := g.safespot.Get(p.Name); ok { + g.forceLeaveSafespot(sess, p, &ss, "") + } + } + }) }) hub.OnDisconnect(func(sess *net.Session) { - if sess.Player != nil { - g.emitCasinoEvents(g.Casino.MarkDisconnected(sess.Player.Name)) - } + g.run(func() { + if sess.Player != nil { + g.emitCasinoEvents(g.Casino.MarkDisconnected(sess.Player.Name)) + } + }) }) g.GlobalFlags.OnChange(func(name string, value any) { + // Fire flag triggers synchronously on the caller; global-flag writes + // all happen on the owner (game commands) or via the admin API which + // is routed through the owner, so a nested run() would deadlock. g.fireGlobalFlagTriggers(name, value) }) } +// RequestShutdown begins the graceful shutdown sequence exactly once; both +// SIGINT/SIGTERM and the in-game shutdown command converge here. It returns +// immediately: the sequence runs on a fresh goroutine (never the owner, so +// its owner tasks cannot deadlock), performs the ordered drain, and closes +// the transports at the end. main then simply returns. +func (g *Game) RequestShutdown(reason string) { + g.shutdownOnce.Do(func() { + g.setShuttingDown() + go func() { + log.Printf("shutdown requested: %s", reason) + g.runShutdown() + }() + }) +} + +// SetTransportStop registers the hook that closes network transports during +// shutdown. main wires it to (*net.Server).Shutdown. +func (g *Game) SetTransportStop(stop func()) { + g.transportStop = stop +} + +// runShutdown executes the ordered shutdown sequence: +// +// 1. Admissions stop: setShuttingDown already makes session input and +// disconnect forfeit/save callbacks no-ops (all handlers are owner +// tasks; the owner still drains what was accepted). +// 2. Stop the tick engine AND join the in-flight callback. +// 3. On the owner: drain remaining queued commands, refund outstanding +// wagers, save every online character. Disconnect-forfeit does not run: +// this is a server-initiated shutdown, not a player disconnect. +// 4. Stop the owner loop. +// 5. Close transports (sessions flush their outbound queues first). +func (g *Game) runShutdown() { + g.broadcastLoggedIn("[Server] Shutting down. Your progress is being saved.") + + // Join the running tick so the last tick's mutations are complete. + g.Ticks.StopAndWait() + + var saveErr error + g.RunOnOwner(func() { + g.ProcessQueuedCommands() + if g.Casino != nil { + g.emitCasinoEvents(g.Casino.RefundAll()) + } + if err := g.saveAllOnlineCharacters(); err != nil { + saveErr = err + } + }) + if saveErr != nil { + log.Printf("shutdown: final save failed: %v", saveErr) + } + + g.stopOwner() + + if g.transportStop != nil { + g.transportStop() + } + log.Println("shutdown complete") +} + +// saveAllOnlineCharacters persists every online player exactly once under +// owner ownership. The first error is reported to the operator; remaining +// characters are still saved. +func (g *Game) saveAllOnlineCharacters() error { + if g.Hub == nil { + return nil + } + var firstErr error + for _, sess := range g.Hub.AllSessions() { + if p := sess.Player; p != nil { + g.restoreGodPlayer(p) + g.persistLockedOnDisconnect(p) + if err := g.AccountStore.SaveCharacter(p); err != nil && firstErr == nil { + firstErr = fmt.Errorf("save %s: %w", p.Name, err) + } + } + } + return firstErr +} + +// HandleSessionOnOwner is the entry point handed to the network server: +// connection goroutines submit one task per input line, so all game state +// mutation executes on the owner goroutine. +func (g *Game) HandleSessionOnOwner(sess *net.Session, input string) { + g.RunOnOwner(func() { + g.HandleSession(sess, input) + }) +} + +// RunOnOwner executes fn on the owner goroutine and waits for completion. +// Network and admin goroutines must route every live-state mutation through +// this; code already running on the owner must not call it (it would +// deadlock waiting for itself). Alias of Game.run with an exported name. +func (g *Game) RunOnOwner(fn func()) { + g.run(fn) +} + +// HandleSession dispatches one line of session input by state. Callers must +// already be on the owner goroutine (use HandleSessionOnOwner otherwise). func (g *Game) HandleSession(sess *net.Session, input string) { input = player.StripControlCharacters(input) switch sess.State { @@ -200,6 +342,45 @@ func (g *Game) HandleSession(sess *net.Session, input string) { } } +// SubscribeTicks registers a dynamic per-tick callback that executes on the +// owner goroutine, preserving registration-ID order relative to the master +// subscriber and other subscribers. Game code must use this instead of +// engine.Subscribe directly so tick callbacks cannot race session input. +func (g *Game) SubscribeTicks(interval int, cb func() bool) uint64 { + if g.owner == nil { + return g.Ticks.Subscribe(interval, cb) + } + return g.Ticks.Subscribe(interval, func() bool { + keep := false + g.run(func() { keep = cb() }) + return keep + }) +} + +// isShuttingDown reports whether the graceful shutdown sequence has taken +// ownership of final persistence. +func (g *Game) isShuttingDown() bool { + g.shuttingDownMu.Lock() + defer g.shuttingDownMu.Unlock() + return g.shuttingDown +} + +// setShuttingDown flips the shutdown ownership flag exactly once. +func (g *Game) setShuttingDown() { + g.shuttingDownMu.Lock() + g.shuttingDown = true + g.shuttingDownMu.Unlock() +} + +// EvictCasinoRoom evicts every cached table/machine in roomID (refund +// semantics per casino.Manager.EvictRoom). Callers must be on the owner +// goroutine. +func (g *Game) EvictCasinoRoom(roomID int) { + if g.Casino != nil { + g.emitCasinoEvents(g.Casino.EvictRoom(roomID)) + } +} + func (g *Game) writePrompt(sess *net.Session) { sess.WritePrompt(g.promptStr(sess)) } diff --git a/internal/game/harness_test.go b/internal/game/harness_test.go new file mode 100644 index 0000000..7898af2 --- /dev/null +++ b/internal/game/harness_test.go @@ -0,0 +1,266 @@ +package game + +import ( + "fmt" + "os" + "path/filepath" + "strings" + "sync" + "testing" + "time" + + "thehouseoficarus/internal/config" + "thehouseoficarus/internal/net" + "thehouseoficarus/internal/player" +) + +// chanConn is a test Conn whose reads come from a channel, so a session's +// input loop can run on its own goroutine exactly like the real server's +// connection goroutine. Writes are recorded in a mutex-guarded buffer and may +// optionally be gated to simulate a slow/stalled client. +type chanConn struct { + mu sync.Mutex + out []byte + in chan string + closed chan struct{} + once sync.Once + + // writeGate, when non-nil, blocks every Write until released. + writeGate chan struct{} +} + +func newChanConn() *chanConn { + return &chanConn{in: make(chan string, 64), closed: make(chan struct{})} +} + +func (c *chanConn) ReadMessage() (string, error) { + select { + case line := <-c.in: + return line, nil + case <-c.closed: + return "", fmt.Errorf("connection closed") + } +} + +func (c *chanConn) Write(b []byte) (int, error) { + if c.writeGate != nil { + select { + case <-c.writeGate: + case <-c.closed: + return 0, fmt.Errorf("write on closed conn") + } + } + c.mu.Lock() + defer c.mu.Unlock() + c.out = append(c.out, b...) + return len(b), nil +} + +func (c *chanConn) Close() error { + c.once.Do(func() { close(c.closed) }) + return nil +} + +func (c *chanConn) SetWriteDeadline(time.Time) error { return nil } + +func (c *chanConn) SetEcho(bool) error { return nil } + +func (c *chanConn) Bytes() []byte { + c.mu.Lock() + defer c.mu.Unlock() + return append([]byte(nil), c.out...) +} + +func (c *chanConn) Contains(substr string) bool { + return strings.Contains(string(c.Bytes()), substr) +} + +// send queues an input line for ReadMessage. No-op after close. +func (c *chanConn) send(line string) { + select { + case c.in <- line: + case <-c.closed: + } +} + +// harness wires a real Game (temp data dir) to fake transports driven from +// per-connection goroutines plus a competing tick goroutine, reproducing the +// production entry-point concurrency: session input, disconnect callbacks, and +// tick callbacks all touching shared state at once. +type harness struct { + t *testing.T + dir string + g *Game + hub *net.Hub + stop func() + + mu sync.Mutex + sessions map[string]*net.Session + conns map[string]*chanConn +} + +func newHarness(t *testing.T) *harness { + t.Helper() + dir := t.TempDir() + seedWorld(t, dir) + colors := &config.ColorsConfig{} + g := New(dir, colors, colors, config.ValidationConfig{}, 1) + hub := net.NewHub() + g.SetHub(hub) + + stopTick := make(chan struct{}) + done := make(chan struct{}) + go func() { + defer close(done) + tick := time.NewTicker(2 * time.Millisecond) + defer tick.Stop() + for { + select { + case <-tick.C: + g.Ticks.Tick() + case <-stopTick: + return + } + } + }() + + h := &harness{ + t: t, dir: dir, g: g, hub: hub, + sessions: make(map[string]*net.Session), + conns: make(map[string]*chanConn), + } + h.stop = func() { close(stopTick); <-done } + t.Cleanup(h.stop) + return h +} + +// seedWorld writes the minimal on-disk content a Game needs: three rooms. +func seedWorld(t *testing.T, dir string) { + t.Helper() + for _, d := range []string{"rooms", "players/accounts", "players/characters"} { + if err := os.MkdirAll(filepath.Join(dir, d), 0755); err != nil { + t.Fatalf("mkdir %s: %v", d, err) + } + } + files := map[string]string{ + "rooms/1.yaml": "id: 1\nname: Central\nexits:\n north:\n room: 2\n", + "rooms/2.yaml": "id: 2\nname: North\nexits:\n south:\n room: 1\n", + "rooms/3.yaml": "id: 3\nname: East\n", + } + for path, content := range files { + if err := os.WriteFile(filepath.Join(dir, path), []byte(content), 0644); err != nil { + t.Fatalf("seed %s: %v", path, err) + } + } +} + +// seedAccount writes an account file directly (bypassing the login menu). +func (h *harness) seedAccount(name, password string, admin bool) { + t := h.t + hash, err := player.HashPassword(password) + if err != nil { + t.Fatalf("hash: %v", err) + } + acc := &player.Account{Name: name, PasswordHash: hash, Admin: admin} + if err := h.g.AccountStore.SaveAccount(acc); err != nil { + t.Fatalf("seed account %s: %v", name, err) + } +} + +// seedCharacter writes a character file owned by the named account. +func (h *harness) seedCharacter(charName, accountName string) *player.Player { + t := h.t + p := player.New(charName) + p.RoomID = 1 + p.Account = accountName + if err := h.g.AccountStore.SaveCharacter(p); err != nil { + t.Fatalf("seed character %s: %v", charName, err) + } + acc, err := h.g.AccountStore.LoadAccount(accountName) + if err != nil { + t.Fatalf("load account %s: %v", accountName, err) + } + acc.Characters = append(acc.Characters, charName) + if err := h.g.AccountStore.SaveAccount(acc); err != nil { + t.Fatalf("save account: %v", err) + } + return p +} + +// connect registers a live in-game session directly, bypassing the login +// menu. The session is put into queued-write mode (like real transports) and +// its input is dispatched through the owner entry point, matching production +// concurrency exactly. Tests that exercise the menu drive handleAccountName +// themselves. +func (h *harness) connect(name string) (*net.Session, *chanConn) { + t := h.t + acc, err := h.g.AccountStore.LoadAccount(name) + if err != nil { + t.Fatalf("connect %s: %v", name, err) + } + p, err := h.g.AccountStore.LoadCharacter(name) + if err != nil { + t.Fatalf("connect %s: %v", name, err) + } + conn := newChanConn() + sess := &net.Session{Conn: conn, Account: acc, Player: p, State: net.StateGame} + sess.StartWriter() + h.hub.Add(sess) + h.hub.EnterRoom(sess, p.RoomID) + h.g.attachSession(sess) + h.mu.Lock() + h.sessions[name] = sess + h.conns[name] = conn + h.mu.Unlock() + return sess, conn +} + +// connectMenu registers a session authenticated to accountName sitting in +// the character menu (no character loaded). +func (h *harness) connectMenu(accountName string) *net.Session { + acc, err := h.g.AccountStore.LoadAccount(accountName) + if err != nil { + h.t.Fatalf("connectMenu %s: %v", accountName, err) + } + conn := newChanConn() + sess := &net.Session{Conn: conn, Account: acc, State: net.StateMenu} + sess.StartWriter() + h.hub.Add(sess) + h.mu.Lock() + h.sessions[accountName+"-menu"] = sess + h.conns[accountName+"-menu"] = conn + h.mu.Unlock() + return sess +} + +// barrier blocks until every task submitted so far has executed on the +// owner goroutine. +func (h *harness) barrier() { + h.g.RunOnOwner(func() {}) +} + +// input mirrors net.Server.handleSession: a per-connection goroutine reading +// lines and dispatching them to HandleSession until the channel closes. +func (h *harness) input(sess *net.Session, lines <-chan string) { + for line := range lines { + h.g.HandleSessionOnOwner(sess, line) + } +} + +// goInput starts input goroutine(s) for a session, returning the send channel. +func (h *harness) goInput(sess *net.Session, workers int) chan<- string { + lines := make(chan string, 64) + for i := 0; i < workers; i++ { + go h.input(sess, lines) + } + return lines +} + +const workers = 1 + +// sessionFor returns the registered session for a character name. +func (h *harness) sessionFor(name string) *net.Session { + h.mu.Lock() + defer h.mu.Unlock() + return h.sessions[name] +} diff --git a/internal/game/lifecycle_txn_test.go b/internal/game/lifecycle_txn_test.go new file mode 100644 index 0000000..f575298 --- /dev/null +++ b/internal/game/lifecycle_txn_test.go @@ -0,0 +1,160 @@ +package game + +// Account lifecycle transaction regressions: same-account races, stale +// session state, and injected filesystem failures must preserve +// identity/membership with no orphaned or duplicated characters. + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "thehouseoficarus/internal/net" +) + +// lifecycle setup: account "acct" with two characters and two menu-state +// sessions authenticated to it. +func setupAcct(t *testing.T, h *harness) (*net.Session, *net.Session) { + t.Helper() + h.seedAccount("acct", "pw", false) + h.seedCharacter("alice", "acct") + h.seedCharacter("bob", "acct") + s1 := h.connectMenu("acct") + s2 := h.connectMenu("acct") + return s1, s2 +} + +func assertCharFiles(t *testing.T, h *harness, want ...string) { + t.Helper() + names, err := h.g.AccountStore.ListCharacterNames() + if err != nil { + t.Fatalf("list characters: %v", err) + } + have := map[string]bool{} + for _, n := range names { + have[n] = true + } + for _, w := range want { + if !have[w] { + t.Fatalf("expected character file %s to exist; files: %v", w, names) + } + } + if len(names) != len(want) { + t.Fatalf("unexpected character files: got %v, want %v", names, want) + } +} + +// TestLifecycleConcurrentRenameNoOrphan: two sessions of the same account +// rename the same character at once. Exactly one succeeds; files and the +// account list stay consistent (no orphaned characters, no duplicate list +// entries). +func TestLifecycleConcurrentRenameNoOrphan(t *testing.T) { + h := newHarness(t) + s1, s2 := setupAcct(t, h) + + h.g.RunOnOwner(func() { h.g.handleRenameChar(s1, "alice") }) + h.g.RunOnOwner(func() { h.g.handleRenameChar(s2, "alice") }) + + // Both sessions now hold a pending rename of alice; drive them to + // submit new names concurrently. + h.g.RunOnOwner(func() { h.g.handleRenameCharName(s1, "alicia") }) + h.g.RunOnOwner(func() { h.g.handleRenameCharName(s2, "alicia") }) + + h.barrier() + // Alicia exists exactly once, alice does not. + assertCharFiles(t, h, "alicia", "bob") + if !h.g.AccountStore.CharacterExists("alicia") { + t.Fatal("renamed character missing") + } +} + +// TestLifecycleStaleMenuAfterDelete: deleting a character from one session +// refreshes the other session's pending selection (it cannot confirm-delete +// a stale name afterwards). +func TestLifecycleStaleMenuAfterDelete(t *testing.T) { + h := newHarness(t) + s1, s2 := setupAcct(t, h) + + // Session 2 selects alice for deletion (pending). + h.g.RunOnOwner(func() { + h.g.handleDeleteChar(s2, "alice") // asks for confirmation + }) + if s2.PendingChar != "alice" { + t.Fatalf("expected pending delete of alice, got %q", s2.PendingChar) + } + + // Session 1 confirms the delete first (its own pending selection is + // "alice", mirroring the same menu state). + h.g.RunOnOwner(func() { + s1.PendingChar = "alice" + h.g.handleDeleteChar(s1, "DELETE alice") + }) + h.barrier() + + // Session 2's pending selection must have been invalidated. + if s2.PendingChar == "alice" { + t.Fatalf("stale pending delete survived the delete from another session") + } + if s2.Account != nil { + for _, c := range s2.Account.Characters { + if strings.EqualFold(c, "alice") { + t.Fatalf("deleted character still in another session's account list") + } + } + } +} + +// TestLifecycleRenameBeatsConnect: a character cannot be renamed while it +// is connected (the online check holds and no stale-name rename goes +// through). +func TestLifecycleRenameBeatsConnect(t *testing.T) { + h := newHarness(t) + h.seedAccount("carl", "pw", false) + h.seedCharacter("carl", "carl") + sess, conn := h.connect("carl") + + h.g.RunOnOwner(func() { + // Drive the rename menu directly while the character is online. + sess.PendingChar = "carl" + sess.State = net.StateRenameCharName + h.g.handleRenameCharName(sess, "carla") + }) + h.barrier() + if h.g.AccountStore.CharacterExists("carla") { + t.Fatalf("rename succeeded while character was online") + } + if !h.g.AccountStore.CharacterExists("carl") { + t.Fatalf("original character file vanished") + } + _ = conn +} + +// TestLifecycleInjectedFailureRollsBack: a failure mid-transaction (the +// account file cannot be written because its directory is read-only... the +// journal lives elsewhere, so use a permissions failure on the account dir) +// must leave both files untouched. +func TestLifecycleInjectedFailureRollsBack(t *testing.T) { + h := newHarness(t) + s1, _ := setupAcct(t, h) + + accDir := filepath.Join(h.dir, "players", "accounts") + if err := os.Chmod(accDir, 0500); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { os.Chmod(accDir, 0700) }) + + // Rename via the transaction path (invoked from a menu session whose + // account dir is unwritable): the move succeeds on disk but the account + // list rewrite fails, so the whole transaction must roll back. + h.g.RunOnOwner(func() { + s1.PendingChar = "alice" + s1.State = net.StateRenameCharName + h.g.handleRenameCharName(s1, "alicia") + }) + h.barrier() + + os.Chmod(accDir, 0700) + // The character must be untouched: no alicia, alice still present. + assertCharFiles(t, h, "alice", "bob") +} diff --git a/internal/game/migrate_rooms_test.go b/internal/game/migrate_rooms_test.go new file mode 100644 index 0000000..eb4a44c --- /dev/null +++ b/internal/game/migrate_rooms_test.go @@ -0,0 +1,115 @@ +package game + +// Runtime room-ID migration regressions: Hub buckets, casino room keys, +// world room-keyed state, mob instances, and player state all move +// together through Game.MigrateRoomIDs (owner-executed). + +import ( + "testing" + + "thehouseoficarus/internal/casino" +) + +// TestMigrateRoomIDsMovesEverythingTogether: a live player in room 2, a +// casino machine in room 2, and seeded world state all migrate to room 4 +// in one owner task; nothing stays attached to room 2. +func TestMigrateRoomIDsMovesEverythingTogether(t *testing.T) { + h := newHarness(t) + h.seedAccount("alice", "pw", false) + p := h.seedCharacter("alice", "alice") + sess, _ := h.connect("alice") + p = sess.Player + + h.g.RunOnOwner(func() { + p.RoomID = 2 + h.hub.EnterRoom(sess, 2) + // A machine in room 2 with a committed stake. + h.g.Casino.JoinMachine(2, casino.MachineConfig{ID: "slots", Game: casino.GameSlots, MinBet: 1}, "alice") + }) + + // Migrate 2 -> 4 through the full runtime entry point. + h.g.MigrateRoomIDs(map[int]int{2: 4}) + h.barrier() + + h.g.RunOnOwner(func() { + if p.RoomID != 4 { + t.Fatalf("player room = %d, want 4", p.RoomID) + } + if room := h.hub.PlayersInRoom(4); len(room) != 1 || room[0] != sess { + t.Fatalf("hub bucket for room 4 = %v, want [alice]", len(room)) + } + if room := h.hub.PlayersInRoom(2); len(room) != 0 { + t.Fatalf("stale hub bucket for room 2: %d players", len(room)) + } + if _, playing := h.g.Casino.Participation("alice"); !playing { + t.Fatal("casino participation lost during migration") + } + // The machine's events must route to room 4 now: emit one and + // check the room target. + machine, ok := h.g.Casino.MachineFor("alice") + if !ok { + t.Fatal("machine lost during migration") + } + if machine.RoomID != 4 { + t.Fatalf("machine room = %d, want 4", machine.RoomID) + } + }) +} + +// TestMigrateRoomIDsRoundTrip preserves participation both directions. +func TestMigrateRoomIDsRoundTrip(t *testing.T) { + h := newHarness(t) + h.seedAccount("alice", "pw", false) + h.seedCharacter("alice", "alice") + sess, _ := h.connect("alice") + p := sess.Player + + h.g.RunOnOwner(func() { + p.RoomID = 3 + h.hub.EnterRoom(sess, 3) + }) + h.g.MigrateRoomIDs(map[int]int{3: 9}) + h.g.RunOnOwner(func() { + if p.RoomID != 9 { + t.Fatalf("forward migration: player room = %d, want 9", p.RoomID) + } + }) + h.g.MigrateRoomIDs(map[int]int{9: 3}) + h.g.RunOnOwner(func() { + if p.RoomID != 3 { + t.Fatalf("inverse migration: player room = %d, want 3", p.RoomID) + } + if room := h.hub.PlayersInRoom(3); len(room) != 1 { + t.Fatalf("inverse migration: hub bucket room 3 = %d players", len(room)) + } + }) +} + +// TestMigrateRoomIDsPersistsOnlinePlayer: the migrated player state is +// saved to disk (RoomID rewritten in the character file). +func TestMigrateRoomIDsPersistsOnlinePlayer(t *testing.T) { + h := newHarness(t) + h.seedAccount("alice", "pw", false) + h.seedCharacter("alice", "alice") + sess, _ := h.connect("alice") + p := sess.Player + + h.g.RunOnOwner(func() { + p.RoomID = 2 + p.Flags["hidden_exit_2_north"] = true + h.hub.EnterRoom(sess, 2) + }) + h.g.MigrateRoomIDs(map[int]int{2: 4}) + h.barrier() + + loaded, err := h.g.AccountStore.LoadCharacter("alice") + if err != nil { + t.Fatal(err) + } + if loaded.RoomID != 4 { + t.Fatalf("saved room = %d, want 4", loaded.RoomID) + } + if _, ok := loaded.Flags["hidden_exit_4_north"]; !ok { + t.Fatalf("discovered-exit flag not rewritten: %v", loaded.Flags) + } +} diff --git a/internal/game/owner.go b/internal/game/owner.go new file mode 100644 index 0000000..df87653 --- /dev/null +++ b/internal/game/owner.go @@ -0,0 +1,119 @@ +package game + +import "sync" + +// owner is the single execution authority for live game state. Every +// mutation of players, sessions, the Hub membership, runtime maps, and all +// tick callbacks runs on exactly one goroutine (the loop below). Network and +// admin goroutines submit tasks; they never touch live objects directly. +// +// Rules: +// - Game.run blocks the caller until the closure has executed on the +// owner goroutine. Connection goroutines use it per input line, which +// gives natural per-client backpressure without ever blocking another +// client: the owner stays free because network I/O happens outside +// (Session writes go through outbound queues, saves serialize copies). +// - Code already executing on the owner must NOT call Game.run (it would +// deadlock waiting for itself). Owner-executed code calls functions +// directly. +// - Task ordering is FIFO: submitted input lines, disconnect callbacks, +// and admin mutations execute in submission order. Tick callbacks +// serialize against these tasks the same way (the engine's ticker +// goroutine submits each callback), preserving the documented tick +// order relative to subscriber registration IDs. +type owner struct { + mu sync.Mutex + closed bool + tasks chan func() + done chan struct{} +} + +const ownerTaskQueue = 4096 + +func newOwner() *owner { + o := &owner{ + tasks: make(chan func(), ownerTaskQueue), + done: make(chan struct{}), + } + go o.loop() + return o +} + +func (o *owner) loop() { + defer close(o.done) + for task := range o.tasks { + task() + } +} + +// submit enqueues a task; it returns false once the owner has stopped. +// The closed flag is guarded by the mutex so a submit racing with stop +// cannot send on a closed channel. +func (o *owner) submit(fn func()) bool { + o.mu.Lock() + defer o.mu.Unlock() + if o.closed { + return false + } + select { + case o.tasks <- fn: + return true + default: + return false + } +} + +func (o *owner) stop() { + o.mu.Lock() + if !o.closed { + o.closed = true + close(o.tasks) + } + o.mu.Unlock() + <-o.done +} + +// run executes fn on the owner goroutine and waits for it. When no owner +// exists (synthetic Games in unit tests) fn runs inline on the caller's +// goroutine; production Games always have one, started by New. +func (g *Game) run(fn func()) { + o := g.owner + if o == nil { + fn() + return + } + task := make(chan struct{}) + if !o.submit(func() { + fn() + close(task) + }) { + // The owner loop has stopped (graceful shutdown in progress). + // Admissions are already closed, so only stragglers reach this: + // execute inline rather than block forever. + fn() + return + } + <-task +} + +// tryRun executes fn on the owner if it is currently idle-draining; it is +// not currently used for correctness paths but exists for future non-blocking +// administrative reads. It returns false when the queue is full. +func (g *Game) tryRun(fn func()) bool { + o := g.owner + if o == nil { + fn() + return true + } + return o.submit(fn) +} + +// stopOwner drains and stops the owner loop. Called once during graceful +// shutdown after all entry points have stopped submitting. +func (g *Game) stopOwner() { + g.ownerStop.Do(func() { + if g.owner != nil { + g.owner.stop() + } + }) +} diff --git a/internal/game/owner_concurrency_test.go b/internal/game/owner_concurrency_test.go new file mode 100644 index 0000000..58e426b --- /dev/null +++ b/internal/game/owner_concurrency_test.go @@ -0,0 +1,154 @@ +package game + +// Owner-loop concurrency regressions. These exercise the competing runtime +// entry points (connection input, ticks, disconnects, admin mutations) the +// way production does and must pass under -race now that all live-state +// mutation executes on the owner goroutine. The pre-owner baselines live in +// baseline_race_test.go behind the baselinerace build tag. + +import ( + "sync" + "testing" + "time" +) + +// TestOwnerSerializesEntryPoints runs session input, the ticker goroutine, +// and hub disconnects concurrently. +func TestOwnerSerializesEntryPoints(t *testing.T) { + h := newHarness(t) + h.seedAccount("alice", "pw", false) + h.seedAccount("bob", "pw", false) + h.seedCharacter("alice", "alice") + h.seedCharacter("bob", "bob") + + aSess, _ := h.connect("alice") + bSess, _ := h.connect("bob") + + aLines := h.goInput(aSess, 1) + bLines := h.goInput(bSess, 1) + + var wg sync.WaitGroup + wg.Add(2) + go func() { + defer wg.Done() + for i := 0; i < 200; i++ { + aLines <- "look" + } + close(aLines) + }() + go func() { + defer wg.Done() + for i := 0; i < 200; i++ { + bLines <- "say hi" + } + close(bLines) + }() + + go func() { + for i := 0; i < 50; i++ { + h.hub.PlayersInRoom(1) + if i == 25 { + h.hub.Remove(bSess) + } + } + }() + + wg.Wait() + h.barrier() +} + +// TestOwnerAdminRewriteWhilePlaying runs Game.RewriteRoomIDs (admin entry) +// concurrently with live movement and ticking. +func TestOwnerAdminRewriteWhilePlaying(t *testing.T) { + h := newHarness(t) + h.seedAccount("carol", "pw", false) + h.seedCharacter("carol", "carol") + sess, _ := h.connect("carol") + lines := h.goInput(sess, 1) + + var wg sync.WaitGroup + wg.Add(2) + go func() { + defer wg.Done() + for i := 0; i < 100; i++ { + lines <- "look" + } + close(lines) + }() + go func() { + defer wg.Done() + // Rewrite 2->4, then back; exercise both directions while the + // player commands run. + for i := 0; i < 10; i++ { + h.g.RunOnOwner(func() { h.g.RewriteRoomIDs(map[int]int{2: 4}) }) + h.g.RunOnOwner(func() { h.g.RewriteRoomIDs(map[int]int{4: 2}) }) + } + }() + wg.Wait() + h.barrier() +} + +// TestSlowClientDoesNotStallTheWorld proves the write-queue boundary: a +// session whose transport blocks forever on Write must not prevent other +// players from receiving output and progressing. +func TestSlowClientDoesNotStallTheWorld(t *testing.T) { + h := newHarness(t) + h.seedAccount("alice", "pw", false) + h.seedAccount("bob", "pw", false) + h.seedCharacter("alice", "alice") + h.seedCharacter("bob", "bob") + + // Alice's writer goroutine blocks at the transport (full socket buffer + // equivalent); her queue absorbs or disconnects her, but never blocks + // the owner goroutine. + aSess, aConn := h.connect("alice") + bSess, bConn := h.connect("bob") + aConn.writeGate = make(chan struct{}) + + bLines := h.goInput(bSess, 1) + + for i := 0; i < 300; i++ { + aSess.WriteLine("spam for the stalled client") + } + // Bob keeps playing and must still see output while alice is stuck. + bLines <- "say hello" + deadline := time.Now().Add(3 * time.Second) + for time.Now().Before(deadline) { + if bConn.Contains("hello") { + return + } + time.Sleep(5 * time.Millisecond) + } + t.Fatalf("bob's output never arrived while alice's client was stalled") +} + +// TestOwnerDisconnectDuringInput removes a session from a separate goroutine +// while its input goroutine is mid-stream, proving the remove callback +// (save + registry cleanup) serializes with session input on the owner. +func TestOwnerDisconnectDuringInput(t *testing.T) { + h := newHarness(t) + h.seedAccount("dave", "pw", false) + h.seedCharacter("dave", "dave") + sess, conn := h.connect("dave") + + lines := h.goInput(sess, 1) + done := make(chan struct{}) + go func() { + defer close(done) + for i := 0; i < 50; i++ { + lines <- "look" + } + close(lines) + }() + // Hard-remove while input is streaming; the onRemove callback mutates + // player state and must not race with HandleSession. + for i := 0; i < 50; i++ { + h.hub.PlayersInRoom(1) + if i == 10 { + h.hub.HardRemove(sess) + } + } + <-done + h.barrier() + _ = conn +} diff --git a/internal/game/session_registry.go b/internal/game/session_registry.go new file mode 100644 index 0000000..0e47f82 --- /dev/null +++ b/internal/game/session_registry.go @@ -0,0 +1,19 @@ +package game + +import "thehouseoficarus/internal/net" + +// attachSession registers a fully-logged-in session in the game's +// character registry. Menu flows call it through connectCharacter; the test +// harness uses it to skip the menu. +func (g *Game) attachSession(sess *net.Session) { + g.charsMu.Lock() + g.loggedInChars[sess.Player.Name] = sess + g.charsMu.Unlock() +} + +// detachSession removes a character's registry entry. +func (g *Game) detachSession(name string) { + g.charsMu.Lock() + delete(g.loggedInChars, name) + g.charsMu.Unlock() +} diff --git a/internal/game/sys_combat.go b/internal/game/sys_combat.go index b37459e..26dac2a 100644 --- a/internal/game/sys_combat.go +++ b/internal/game/sys_combat.go @@ -129,7 +129,7 @@ func (g *Game) checkAggro(sess *net.Session) { if !keepRolling() { break } - g.Ticks.Subscribe(1, keepRolling) + g.SubscribeTicks(1, keepRolling) break } } diff --git a/internal/game/sys_escape_test.go b/internal/game/sys_escape_test.go index a9842ba..0a64666 100644 --- a/internal/game/sys_escape_test.go +++ b/internal/game/sys_escape_test.go @@ -3,6 +3,7 @@ package game import ( "os" "testing" + "time" "thehouseoficarus/internal/combat" "thehouseoficarus/internal/engine" @@ -22,6 +23,8 @@ func (c *escapeTestConn) Write(b []byte) (int, error) { c.data = append(c.data, b...) return len(b), nil } +func (c *escapeTestConn) SetWriteDeadline(time.Time) error { return nil } + func (c *escapeTestConn) Close() error { return nil } func (c *escapeTestConn) SetEcho(bool) error { return nil } diff --git a/internal/game/sys_safespot.go b/internal/game/sys_safespot.go index d833b62..2529471 100644 --- a/internal/game/sys_safespot.go +++ b/internal/game/sys_safespot.go @@ -227,7 +227,7 @@ func (g *Game) scheduleSafespotRespawn(st *world.ObjState, objDef *object.Object } roomID := st.RoomID defID := objDef.ID - g.Ticks.Subscribe(engine.ToTicks(cfg.RespawnTicks), func() bool { + g.SubscribeTicks(engine.ToTicks(cfg.RespawnTicks), func() bool { respawnSt := g.World.GetObjState(roomID, defID, 0) if respawnSt == nil { return false diff --git a/internal/net/conn.go b/internal/net/conn.go index 2fff0bc..6d78d8f 100644 --- a/internal/net/conn.go +++ b/internal/net/conn.go @@ -6,12 +6,18 @@ import ( "io" "net" "strings" + "time" ) type Conn interface { ReadMessage() (string, error) io.Writer io.Closer + // SetWriteDeadline is optional for transports: sessions that support it + // enforce per-write deadlines so a stalled peer surfaces as a timeout + // instead of a hung writer goroutine. Implementations without deadline + // support return nil (deadlines are best-effort). + SetWriteDeadline(t time.Time) error SetEcho(bool) error } @@ -70,6 +76,10 @@ func (t *tcpConn) Write(b []byte) (int, error) { return t.conn.Write(b) } +func (t *tcpConn) SetWriteDeadline(ts time.Time) error { + return t.conn.SetWriteDeadline(ts) +} + func (t *tcpConn) Close() error { return t.conn.Close() } diff --git a/internal/net/server.go b/internal/net/server.go index 9b5be18..b0407fc 100644 --- a/internal/net/server.go +++ b/internal/net/server.go @@ -1,6 +1,7 @@ package net import ( + "context" "crypto/rand" "crypto/rsa" "crypto/tls" @@ -73,6 +74,19 @@ type Session struct { promptVisible bool writeMu sync.Mutex + // Outbound queueing. Transport sessions (telnet/TLS/websocket) are put + // into queued mode by startWriter before their input goroutine begins: + // Write* then enqueues into outbox, and the dedicated writer goroutine + // performs the network I/O with a per-message write deadline. A stalled + // or slow client therefore blocks at most its own queue, never the + // world; an overflowing queue disconnects only that client. Sessions + // without a writer (tests, synthetic conns) keep the legacy synchronous + // path so message construction is shared and behavior identical. + outbox chan string + writerOnce sync.Once + closeOnce sync.Once + writeDone chan struct{} + // seq is a monotonically increasing sequence number assigned when the // session is added to a Hub. Hub.AllSessions and Hub.PlayersInRoom sort // sessions by seq so per-tick consumers iterate players in a stable, @@ -97,6 +111,10 @@ type Server struct { httpsTLSConfig *tls.Config hub *Hub handler func(*Session, string) + + done chan struct{} + shutdownOnce sync.Once + listenErr error } type Hub struct { @@ -107,6 +125,64 @@ type Hub struct { onDisconnect func(*Session) } +// writeQueueBounds bound the outbound queue: maxQueuedMessages before a +// client is disconnected for falling behind, writeDeadline per message, and +// closeFlushTimeout to drain the queue when the session is closing. +const ( + maxQueuedMessages = 512 + writeDeadline = 5 * time.Second + closeFlushTimeout = 2 * time.Second +) + +// startWriter puts the session into queued mode: writes are enqueued and a +// dedicated goroutine drains the queue to the network with per-message write +// deadlines. The goroutine closes the transport after the queue drains, so a +// Close() during shutdown flushes pending output before the socket drops. +// Must be called before the session's input goroutine starts. +func (sess *Session) StartWriter() { + sess.writerOnce.Do(func() { + sess.outbox = make(chan string, maxQueuedMessages) + sess.writeDone = make(chan struct{}) + go func() { + defer close(sess.writeDone) + for msg := range sess.outbox { + if dl, ok := sess.Conn.(interface { + SetWriteDeadline(time.Time) error + }); ok { + _ = dl.SetWriteDeadline(time.Now().Add(writeDeadline)) + } + if _, err := sess.Conn.Write([]byte(msg)); err != nil { + log.Printf("session write error: %v", err) + // Abandon the queue and drop the transport; the input + // goroutine notices the closed connection on its next + // read and performs normal removal. + sess.Conn.Close() + return + } + } + // Queue closed and drained: flush any final bytes and close. + if dl, ok := sess.Conn.(interface { + SetWriteDeadline(time.Time) error + }); ok { + _ = dl.SetWriteDeadline(time.Now().Add(closeFlushTimeout)) + } + sess.Conn.Close() + }() + }) +} + +// enqueue appends msg to the outbound queue. When the client's queue is full +// the session is disconnected instead of blocking the caller: the world must +// never wait on a single slow client. +func (sess *Session) enqueue(msg string) { + select { + case sess.outbox <- msg: + default: + log.Printf("session write queue overflow; disconnecting slow client") + sess.Close() + } +} + func NewHub() *Hub { return &Hub{ sessions: make(map[*Session]bool), @@ -145,24 +221,34 @@ func (h *Hub) Remove(s *Session) { } return } - h.hardRemoveLocked(s) + callback := h.hardRemoveLocked(s) h.mu.Unlock() + // The removal callback runs outside the Hub mutex: it performs saves + // and game-state cleanup, and must never serialize behind (or hold up) + // Hub membership operations. + if callback != nil { + callback(s) + } } func (h *Hub) HardRemove(s *Session) { h.mu.Lock() - defer h.mu.Unlock() - h.hardRemoveLocked(s) + callback := h.hardRemoveLocked(s) + h.mu.Unlock() + if callback != nil { + callback(s) + } } -func (h *Hub) hardRemoveLocked(s *Session) { +// hardRemoveLocked removes the session from all membership structures and +// returns the removal callback for the caller to invoke AFTER releasing the +// Hub mutex (nil when none is registered). +func (h *Hub) hardRemoveLocked(s *Session) func(*Session) { delete(h.sessions, s) for _, room := range h.rooms { delete(room, s) } - if h.onRemove != nil { - h.onRemove(s) - } + return h.onRemove } func (h *Hub) EnterRoom(s *Session, roomID int) { @@ -187,6 +273,28 @@ func (h *Hub) leaveRoomLocked(s *Session) { } } +// RekeyRooms remaps Hub room membership buckets per idMap (oldID -> newID). +// Uses delete-all-then-set-all so a swap (A<->B) cannot clobber an entry +// mid-rekey. Sessions' own player state is migrated by the game layer; this +// only moves the membership buckets. +func (h *Hub) RekeyRooms(idMap map[int]int) { + h.mu.Lock() + defer h.mu.Unlock() + if len(idMap) == 0 { + return + } + moved := make(map[int]map[*Session]bool, len(h.rooms)) + for id, set := range h.rooms { + if newID, ok := idMap[id]; ok && newID != id { + moved[newID] = set + delete(h.rooms, id) + } + } + for newID, set := range moved { + h.rooms[newID] = set + } +} + func (h *Hub) AllSessions() []*Session { h.mu.Lock() defer h.mu.Unlock() @@ -215,6 +323,7 @@ func NewServer(cfg *config.Config) (*Server, error) { s := &Server{ config: cfg, hub: NewHub(), + done: make(chan struct{}), } if cfg.Telnet.Enabled { @@ -337,7 +446,38 @@ func (s *Server) ListenAndServe(handler func(*Session, string)) error { return fmt.Errorf("no listeners configured") } - select {} + // Block until Shutdown is called (or a fatal listener error occurs). + <-s.done + return s.listenErr +} + +// Shutdown stops all transports: HTTP(S) servers drain, listeners close, and +// every session is closed (its outbound queue drains first). ListenAndServe +// returns after Shutdown completes. It is safe to call more than once. +func (s *Server) Shutdown() { + s.shutdownOnce.Do(func() { + if s.httpServer != nil { + ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second) + defer cancel() + _ = s.httpServer.Shutdown(ctx) + } + if s.httpsServer != nil { + ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second) + defer cancel() + _ = s.httpsServer.Shutdown(ctx) + } + if s.telnetLn != nil { + s.telnetLn.Close() + } + if s.telnetTLSLn != nil { + s.telnetTLSLn.Close() + } + for _, sess := range s.hub.AllSessions() { + sess.Close() + } + close(s.done) + }) + <-s.done } func (s *Server) serveTelnet() { @@ -399,7 +539,7 @@ func (s *Server) handleSession(sess *Session, handler func(*Session, string)) { sess.Close() }() - sess.Conn.Write([]byte("\033[2J\033[H")) + sess.Write("\033[2J\033[H") sess.Write(WelcomeBanner()) sess.Write("What's your account name? ") @@ -439,15 +579,26 @@ func (sess *Session) ClearPrompt() { sess.promptVisible = false } +// dispatch delivers a fully-formatted message to the transport: enqueued in +// queued mode (the writer goroutine owns the network), written synchronously +// in direct mode (tests, synthetic conns). Caller holds writeMu. +func (sess *Session) dispatch(msg string) { + if sess.outbox != nil { + sess.enqueue(msg) + return + } + sess.Conn.Write([]byte(msg)) +} + // Write sends raw text with no wrapping. Used for prompts and partial lines. func (sess *Session) Write(msg string) { sess.writeMu.Lock() defer sess.writeMu.Unlock() if sess.promptVisible { - sess.Conn.Write([]byte("\r\n")) + sess.dispatch("\r\n") sess.promptVisible = false } - sess.Conn.Write([]byte(msg)) + sess.dispatch(msg) } func (sess *Session) WriteLine(msg string) { @@ -455,10 +606,10 @@ func (sess *Session) WriteLine(msg string) { sess.writeMu.Lock() defer sess.writeMu.Unlock() if sess.promptVisible { - sess.Conn.Write([]byte("\r\n")) + sess.dispatch("\r\n") sess.promptVisible = false } - sess.Conn.Write([]byte(msg + "\r\n")) + sess.dispatch(msg + "\r\n") } func (sess *Session) WriteLines(lines ...string) { @@ -466,10 +617,10 @@ func (sess *Session) WriteLines(lines ...string) { defer sess.writeMu.Unlock() for i, l := range lines { if i == 0 && sess.promptVisible { - sess.Conn.Write([]byte("\r\n")) + sess.dispatch("\r\n") sess.promptVisible = false } - sess.Conn.Write([]byte(sess.wrap(l) + "\r\n")) + sess.dispatch(sess.wrap(l) + "\r\n") } } @@ -478,10 +629,10 @@ func (sess *Session) Writef(format string, args ...interface{}) { sess.writeMu.Lock() defer sess.writeMu.Unlock() if sess.promptVisible { - sess.Conn.Write([]byte("\r\n")) + sess.dispatch("\r\n") sess.promptVisible = false } - sess.Conn.Write([]byte(msg)) + sess.dispatch(msg) } func (sess *Session) WritePrompt(prompt string) { @@ -490,19 +641,26 @@ func (sess *Session) WritePrompt(prompt string) { if sess.promptVisible { return } - sess.Conn.Write([]byte(prompt)) + sess.dispatch(prompt) sess.promptVisible = true } func (sess *Session) Reprompt(prompt string) { sess.writeMu.Lock() defer sess.writeMu.Unlock() - sess.Conn.Write([]byte(prompt)) + sess.dispatch(prompt) sess.promptVisible = true } +// Close stops the session. In queued mode it closes the queue so the writer +// goroutine drains pending messages and then closes the transport; in direct +// mode it closes the transport immediately. Idempotent. func (sess *Session) Close() error { - sess.writeMu.Lock() - defer sess.writeMu.Unlock() - return sess.Conn.Close() + if sess.outbox != nil { + sess.closeOnce.Do(func() { close(sess.outbox) }) + return nil + } + var err error + sess.closeOnce.Do(func() { err = sess.Conn.Close() }) + return err } diff --git a/internal/net/server_test.go b/internal/net/server_test.go index b106c53..c91c701 100644 --- a/internal/net/server_test.go +++ b/internal/net/server_test.go @@ -3,6 +3,7 @@ package net import ( "sync" "testing" + "time" ) func TestHubConcurrency(t *testing.T) { @@ -38,10 +39,12 @@ func TestHubConcurrency(t *testing.T) { type testConn struct{ data []byte } -func (c *testConn) ReadMessage() (string, error) { return "", nil } -func (c *testConn) Write(b []byte) (int, error) { c.data = append(c.data, b...); return len(b), nil } -func (c *testConn) Close() error { return nil } -func (c *testConn) SetEcho(bool) error { return nil } +func (c *testConn) ReadMessage() (string, error) { return "", nil } +func (c *testConn) Write(b []byte) (int, error) { c.data = append(c.data, b...); return len(b), nil } +func (c *testConn) SetWriteDeadline(time.Time) error { return nil } + +func (c *testConn) Close() error { return nil } +func (c *testConn) SetEcho(bool) error { return nil } func TestSessionWriteConcurrency(t *testing.T) { conn := &testConn{} diff --git a/internal/net/web.go b/internal/net/web.go index e573aba..233928f 100644 --- a/internal/net/web.go +++ b/internal/net/web.go @@ -64,6 +64,7 @@ func newHTTPMux(s *Server) *http.ServeMux { Conn: newWSConn(ws), State: StateAccountName, } + sess.StartWriter() s.hub.Add(sess) go s.handleSession(sess, s.handler) }) diff --git a/internal/net/wsconn.go b/internal/net/wsconn.go index 4990c0b..34663ba 100644 --- a/internal/net/wsconn.go +++ b/internal/net/wsconn.go @@ -1,6 +1,8 @@ package net import ( + "time" + "github.com/gorilla/websocket" ) @@ -25,6 +27,12 @@ func (w *wsConn) Write(b []byte) (int, error) { return len(b), err } +// SetWriteDeadline delegates to the underlying websocket connection, which +// supports deadlines natively. +func (w *wsConn) SetWriteDeadline(ts time.Time) error { + return w.conn.SetWriteDeadline(ts) +} + func (w *wsConn) Close() error { return w.conn.Close() } diff --git a/internal/persistence/txn.go b/internal/persistence/txn.go new file mode 100644 index 0000000..7d86cc8 --- /dev/null +++ b/internal/persistence/txn.go @@ -0,0 +1,335 @@ +package persistence + +// Multi-file journal transactions. A Transaction collects file operations +// (replace, delete, move), snapshots before-images, writes a journal entry +// BEFORE mutating anything, applies the operations, and removes the journal +// on success. If any operation fails, every applied operation is rolled back. +// If the process dies mid-commit, the leftover journal is rolled back at the +// next Recover call (identities revert to their pre-transaction state; no +// half-renamed accounts or orphaned characters survive a crash). +// +// Transactions do NOT serialize concurrent callers: like single-file saves, +// they must be executed by an owner that prevents concurrent mutations of +// the same files (the game's owner goroutine). + +import ( + "crypto/rand" + "encoding/hex" + "encoding/json" + "fmt" + "os" + "path/filepath" +) + +type fileOp struct { + // Replace / Delete / Move data + Path string `json:"path"` // file this record is keyed by (its before-image lives here) + Kind string `json:"kind"` // "replace", "delete", "move", "move-target" + MoveTo string `json:"move_to,omitempty"` // move: destination + New []byte `json:"new,omitempty"` // replace: after-image; nil means delete target + NewExists bool `json:"new_exists,omitempty"` + NewMode uint32 `json:"new_mode,omitempty"` + + // Before-image snapshot (for rollback), captured on first touch. + Old []byte `json:"old,omitempty"` + OldExists bool `json:"old_exists"` + OldMode uint32 `json:"old_mode,omitempty"` +} + +const ( + kindReplace = "replace" + kindDelete = "delete" + kindMove = "move" + kindMoveTarget = "move-target" +) + +type journalEntry struct { + Ops []fileOp `json:"ops"` +} + +type Transaction struct { + ops map[string]*fileOp + order []string + journalDir string +} + +func NewTransaction(journalDir string) *Transaction { + return &Transaction{ + ops: make(map[string]*fileOp), + journalDir: journalDir, + } +} + +// getOp returns (creating if needed) the operation record for path, capturing +// the before-image the first time the path is touched. +func (t *Transaction) getOp(path string) (*fileOp, error) { + if op, ok := t.ops[path]; ok { + return op, nil + } + mode := uint32(0600) + info, err := os.Stat(path) + oldExists := err == nil + if err != nil && !os.IsNotExist(err) { + return nil, err + } + if oldExists { + mode = uint32(info.Mode().Perm()) + } + var old []byte + if oldExists { + old, err = os.ReadFile(path) + if err != nil { + return nil, fmt.Errorf("snapshot %s: %w", path, err) + } + } + op := &fileOp{ + Path: path, + Old: old, + OldExists: oldExists, + OldMode: mode, + } + t.ops[path] = op + t.order = append(t.order, path) + return op, nil +} + +// Replace writes data over path (creating it if needed). When the path is +// also a move destination (move then replace content in one transaction), +// the replace augments the move-target record: apply writes the moved file, +// rollback restores the move-target before-image. +func (t *Transaction) Replace(path string, data []byte, perm os.FileMode) error { + op, err := t.getOp(path) + if err != nil { + return err + } + if op.Kind == kindMoveTarget { + op.New = data + op.NewExists = true + op.NewMode = uint32(perm) + return nil + } + op.Kind = kindReplace + op.New = data + op.NewExists = true + op.NewMode = uint32(perm) + return nil +} + +// Delete removes path. Deleting a missing path is a no-op. +func (t *Transaction) Delete(path string) error { + op, err := t.getOp(path) + if err != nil { + return err + } + op.Kind = kindDelete + op.New, op.NewExists = nil, false + return nil +} + +// Move relocates a file (rename semantics). If newPath already exists its +// before-image is captured in a separate move-target record for rollback. +func (t *Transaction) Move(oldPath, newPath string) error { + src, err := t.getOp(oldPath) + if err != nil { + return err + } + dst, err := t.getOp(newPath) + if err != nil { + return err + } + if src.Kind == kindMove && src.MoveTo != newPath { + return fmt.Errorf("conflicting move of %s", oldPath) + } + if dst.Kind != kindMoveTarget && dst.Kind != "" { + return fmt.Errorf("conflicting operation on %s", newPath) + } + // An existing replace/delete on the destination becomes a move-target + // (the destination's before-image governs rollback). + dst.Kind = kindMoveTarget + src.Kind = kindMove + src.MoveTo = newPath + return nil +} + +func (t *Transaction) MarshalJournal() ([]byte, error) { + ops := make([]fileOp, 0, len(t.order)) + for _, p := range t.order { + ops = append(ops, *t.ops[p]) + } + return json.Marshal(journalEntry{Ops: ops}) +} + +// Commit writes the journal (durable), applies all operations, and removes +// the journal. Any failure rolls back every already-applied operation and +// the journal itself, leaving the filesystem exactly as before the commit. +func (t *Transaction) Commit() error { + if len(t.order) == 0 { + return nil + } + journal, err := t.MarshalJournal() + if err != nil { + return fmt.Errorf("marshal journal: %w", err) + } + if err := os.MkdirAll(t.journalDir, 0700); err != nil { + return fmt.Errorf("journal dir: %w", err) + } + var id [8]byte + if _, err := rand.Read(id[:]); err != nil { + return fmt.Errorf("journal id: %w", err) + } + journalPath := filepath.Join(t.journalDir, "txn-"+hex.EncodeToString(id[:])+".json") + if err := WriteFile(journalPath, journal, 0600); err != nil { + return fmt.Errorf("write journal: %w", err) + } + + if err := t.apply(); err != nil { + if rbErr := t.rollback(); rbErr != nil { + err = fmt.Errorf("%v; rollback failed: %v", err, rbErr) + } + if rErr := os.Remove(journalPath); rErr != nil && !os.IsNotExist(rErr) { + err = fmt.Errorf("%v; journal cleanup failed: %v", err, rErr) + } + return err + } + + // Commit point: the journal is the record of applied operations. After + // it is removed the transaction is complete and unrecoverable. + if err := os.Remove(journalPath); err != nil && !os.IsNotExist(err) { + return fmt.Errorf("commit completed; remove journal: %w", err) + } + return nil +} + +func (t *Transaction) apply() error { + for _, p := range t.order { + op := t.ops[p] + switch op.Kind { + case kindReplace: + if err := WriteFile(op.Path, op.New, os.FileMode(op.NewMode)); err != nil { + return fmt.Errorf("write %s: %w", op.Path, err) + } + case kindDelete: + if err := os.Remove(op.Path); err != nil && !os.IsNotExist(err) { + return fmt.Errorf("delete %s: %w", op.Path, err) + } + case kindMove: + // Remove an existing destination first so the rename cannot fail + // on a non-empty target; the destination's before-image is + // captured in its move-target record for rollback. + if _, err := os.Stat(op.MoveTo); err == nil { + if err := os.Remove(op.MoveTo); err != nil { + return fmt.Errorf("move remove destination %s: %w", op.MoveTo, err) + } + } + if err := os.Rename(op.Path, op.MoveTo); err != nil { + return fmt.Errorf("move %s: %w", op.Path, err) + } + case kindMoveTarget: + // Content replace after the move delivered the source bytes. + if op.NewExists { + if err := WriteFile(op.Path, op.New, os.FileMode(op.NewMode)); err != nil { + return fmt.Errorf("write %s: %w", op.Path, err) + } + } + } + } + return nil +} + +func (t *Transaction) rollback() error { + var firstErr error + record := func(err error) { + if firstErr == nil { + firstErr = err + } + } + // Restore in reverse order so a move that overwrote a destination + // resurrects the destination before the source is restored. + for i := len(t.order) - 1; i >= 0; i-- { + op := t.ops[t.order[i]] + restore := func(path string) error { + if op.OldExists { + return WriteFile(path, op.Old, os.FileMode(op.OldMode)) + } + return os.Remove(path) + } + switch op.Kind { + case kindMove, kindMoveTarget: + if err := restore(op.Path); err != nil && !os.IsNotExist(err) { + record(fmt.Errorf("restore %s: %w", op.Path, err)) + } + case kindReplace: + if err := restore(op.Path); err != nil { + record(fmt.Errorf("restore %s: %w", op.Path, err)) + } + case kindDelete: + if err := restore(op.Path); err != nil && !os.IsNotExist(err) { + record(fmt.Errorf("restore %s: %w", op.Path, err)) + } + } + } + return firstErr +} + +// Recover rolls back every leftover journal in journalDir. Call it at +// startup before any store reads files the journals may reference. +func Recover(journalDir string) error { + if err := os.MkdirAll(journalDir, 0700); err != nil { + return err + } + entries, err := os.ReadDir(journalDir) + if err != nil { + return err + } + for _, e := range entries { + if e.IsDir() || !hasSuffix(e.Name(), ".json") { + continue + } + path := filepath.Join(journalDir, e.Name()) + data, err := os.ReadFile(path) + if err != nil { + return fmt.Errorf("read journal %s: %w", path, err) + } + var entry journalEntry + if err := json.Unmarshal(data, &entry); err != nil { + // An unparsable journal was likely mid-write and never applied + // any operation (journals are written atomically). Drop it. + if rmErr := os.Remove(path); rmErr != nil { + return fmt.Errorf("remove corrupt journal %s: %w", path, rmErr) + } + continue + } + t := &Transaction{ops: make(map[string]*fileOp), journalDir: journalDir} + for i := range entry.Ops { + op := entry.Ops[i] + t.ops[op.Path] = &op + t.order = append(t.order, op.Path) + } + if err := t.rollback(); err != nil { + return fmt.Errorf("recover %s: %w", path, err) + } + if err := os.Remove(path); err != nil { + return fmt.Errorf("remove recovered journal %s: %w", path, err) + } + } + return nil +} + +func hasSuffix(s, suffix string) bool { + return len(s) >= len(suffix) && s[len(s)-len(suffix):] == suffix +} + +// ListJournalFiles is a debugging helper: returns sorted journal file names. +func ListJournalFiles(journalDir string) ([]string, error) { + entries, err := os.ReadDir(journalDir) + if err != nil { + return nil, err + } + var out []string + for _, e := range entries { + if !e.IsDir() { + out = append(out, e.Name()) + } + } + return out, nil +} diff --git a/internal/persistence/txn_test.go b/internal/persistence/txn_test.go new file mode 100644 index 0000000..10a809d --- /dev/null +++ b/internal/persistence/txn_test.go @@ -0,0 +1,186 @@ +package persistence + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +func writeTestFile(t *testing.T, path, content string) { + t.Helper() + if err := os.MkdirAll(filepath.Dir(path), 0755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, []byte(content), 0644); err != nil { + t.Fatal(err) + } +} + +func readTestFile(t *testing.T, path string) string { + t.Helper() + data, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + return string(data) +} + +func TestTxnCommitAppliesAllOps(t *testing.T) { + dir := t.TempDir() + journalDir := filepath.Join(dir, ".journal") + writeTestFile(t, filepath.Join(dir, "a.yaml"), "old-a") + writeTestFile(t, filepath.Join(dir, "b.yaml"), "old-b") + writeTestFile(t, filepath.Join(dir, "c.yaml"), "old-c") + + txn := NewTransaction(journalDir) + if err := txn.Replace(filepath.Join(dir, "a.yaml"), []byte("new-a"), 0644); err != nil { + t.Fatal(err) + } + if err := txn.Delete(filepath.Join(dir, "b.yaml")); err != nil { + t.Fatal(err) + } + if err := txn.Move(filepath.Join(dir, "c.yaml"), filepath.Join(dir, "d.yaml")); err != nil { + t.Fatal(err) + } + if err := txn.Replace(filepath.Join(dir, "created.yaml"), []byte("created"), 0600); err != nil { + t.Fatal(err) + } + + if err := txn.Commit(); err != nil { + t.Fatalf("commit: %v", err) + } + if got := readTestFile(t, filepath.Join(dir, "a.yaml")); got != "new-a" { + t.Fatalf("replace failed: %q", got) + } + if _, err := os.Stat(filepath.Join(dir, "b.yaml")); !os.IsNotExist(err) { + t.Fatalf("delete failed: %v", err) + } + if _, err := os.Stat(filepath.Join(dir, "c.yaml")); !os.IsNotExist(err) { + t.Fatalf("move left source behind: %v", err) + } + if got := readTestFile(t, filepath.Join(dir, "d.yaml")); got != "old-c" { + t.Fatalf("move lost content: %q", got) + } + if got := readTestFile(t, filepath.Join(dir, "created.yaml")); got != "created" { + t.Fatalf("create failed: %q", got) + } + if entries, _ := ListJournalFiles(journalDir); len(entries) != 0 { + t.Fatalf("journal not cleaned: %v", entries) + } +} + +func TestTxnRollbackOnFailure(t *testing.T) { + dir := t.TempDir() + journalDir := filepath.Join(dir, ".journal") + writeTestFile(t, filepath.Join(dir, "a.yaml"), "old-a") + // A file inside a read-only subdirectory makes the delete fail. + sub := filepath.Join(dir, "sub") + writeTestFile(t, filepath.Join(sub, "gone.yaml"), "old-gone") + if err := os.Chmod(sub, 0500); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { os.Chmod(sub, 0700) }) + + txn := NewTransaction(journalDir) + if err := txn.Replace(filepath.Join(dir, "a.yaml"), []byte("new-a"), 0644); err != nil { + t.Fatal(err) + } + if err := txn.Delete(filepath.Join(sub, "gone.yaml")); err != nil { + t.Fatal(err) + } + if err := txn.Commit(); err == nil { + t.Fatal("expected commit to fail") + } + + if got := readTestFile(t, filepath.Join(dir, "a.yaml")); got != "old-a" { + t.Fatalf("rollback did not restore a.yaml: %q", got) + } + if got := readTestFile(t, filepath.Join(sub, "gone.yaml")); got != "old-gone" { + t.Fatalf("rollback did not restore deleted file: %q", got) + } + if entries, _ := ListJournalFiles(journalDir); len(entries) != 0 { + t.Fatalf("journal not cleaned after rollback: %v", entries) + } +} + +func TestRecoverRollsBackLeftoverJournal(t *testing.T) { + // Simulate a crash: journal exists (it says a.yaml becomes new-a and + // c.yaml is deleted) but the operations never ran. + dir := t.TempDir() + journalDir := filepath.Join(dir, ".journal") + writeTestFile(t, filepath.Join(dir, "a.yaml"), "old-a") + writeTestFile(t, filepath.Join(dir, "c.yaml"), "old-c") + + txn := NewTransaction(journalDir) + if err := txn.Replace(filepath.Join(dir, "a.yaml"), []byte("new-a"), 0644); err != nil { + t.Fatal(err) + } + if err := txn.Delete(filepath.Join(dir, "c.yaml")); err != nil { + t.Fatal(err) + } + journal, err := txn.MarshalJournal() + if err != nil { + t.Fatal(err) + } + if err := os.MkdirAll(journalDir, 0700); err != nil { + t.Fatal(err) + } + if err := WriteFile(filepath.Join(journalDir, "txn-crash.json"), journal, 0600); err != nil { + t.Fatal(err) + } + + if err := Recover(journalDir); err != nil { + t.Fatalf("recover: %v", err) + } + if got := readTestFile(t, filepath.Join(dir, "a.yaml")); got != "old-a" { + t.Fatalf("recovery did not roll back a.yaml: %q", got) + } + if got := readTestFile(t, filepath.Join(dir, "c.yaml")); got != "old-c" { + t.Fatalf("recovery resurrected a delete: %q", got) + } + if entries, _ := ListJournalFiles(journalDir); len(entries) != 0 { + t.Fatalf("recovered journal not removed: %v", entries) + } +} + +func TestRecoverDropsCorruptJournal(t *testing.T) { + dir := t.TempDir() + journalDir := filepath.Join(dir, ".journal") + if err := os.MkdirAll(journalDir, 0700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(journalDir, "txn-bad.json"), []byte("{not json"), 0600); err != nil { + t.Fatal(err) + } + if err := Recover(journalDir); err != nil { + t.Fatalf("recover corrupt: %v", err) + } + if entries, _ := ListJournalFiles(journalDir); len(entries) != 0 { + t.Fatalf("corrupt journal not removed: %v", entries) + } +} + +func TestTxnMoveOverwritesDestination(t *testing.T) { + dir := t.TempDir() + journalDir := filepath.Join(dir, ".journal") + writeTestFile(t, filepath.Join(dir, "src.yaml"), "src-content") + writeTestFile(t, filepath.Join(dir, "dst.yaml"), "dst-content") + + txn := NewTransaction(journalDir) + if err := txn.Move(filepath.Join(dir, "src.yaml"), filepath.Join(dir, "dst.yaml")); err != nil { + t.Fatal(err) + } + if err := txn.Commit(); err != nil { + t.Fatalf("commit: %v", err) + } + if got := readTestFile(t, filepath.Join(dir, "dst.yaml")); got != "src-content" { + t.Fatalf("move did not overwrite destination: %q", got) + } + if _, err := os.Stat(filepath.Join(dir, "src.yaml")); !os.IsNotExist(err) { + t.Fatalf("source not removed: %v", err) + } + if !strings.HasSuffix(readTestFile(t, filepath.Join(dir, "dst.yaml")), "content") { + t.Fatal("unreachable") + } +} diff --git a/internal/player/player.go b/internal/player/player.go index 81ef056..ae147b2 100644 --- a/internal/player/player.go +++ b/internal/player/player.go @@ -159,6 +159,11 @@ func GetOptionDef(name string) *OptionDef { type Player struct { Name string `yaml:"name"` + // ID is a stable identifier assigned at creation. Filenames are + // renameable; the ID is not, so cached references authorize by ID. + ID string `yaml:"id,omitempty"` + // Account is the owning account name (case-canonical at write time). + Account string `yaml:"account,omitempty"` Skills map[SkillName]int `yaml:"skills"` Inventory map[int]*InventorySlot `yaml:"inventory"` Bank map[int]*InventorySlot `yaml:"bank"` diff --git a/internal/player/store.go b/internal/player/store.go index d0dbd0f..d461485 100644 --- a/internal/player/store.go +++ b/internal/player/store.go @@ -37,12 +37,17 @@ func (s *AccountStore) LoadAccount(name string) (*Account, error) { } func (s *AccountStore) SaveAccount(acc *Account) error { - path := s.AccountPath(acc.Name) - data, err := yaml.Marshal(acc) + data, err := s.MarshalAccount(acc) if err != nil { return err } - return persistence.WriteFile(path, data, 0600) + return persistence.WriteFile(s.AccountPath(acc.Name), data, 0600) +} + +// MarshalAccount serializes an account; callers that own the account (the +// game owner, or transaction code holding it) may use this directly. +func (s *AccountStore) MarshalAccount(acc *Account) ([]byte, error) { + return yaml.Marshal(acc) } func (s *AccountStore) AccountExists(name string) bool { @@ -103,9 +108,18 @@ func (s *AccountStore) LoadCharacter(name string) (*Player, error) { } func (s *AccountStore) SaveCharacter(p *Player) error { - path := s.CharPath(p.Name) - // Serialize a value copy so saving god-mode stats never temporarily - // changes the live player. The caller still owns all referenced maps. + data, err := s.MarshalCharacter(p) + if err != nil { + return err + } + return persistence.WriteFile(s.CharPath(p.Name), data, 0600) +} + +// MarshalCharacter serializes a character for a same-directory temporary +// atomic write. It serializes a value snapshot so saving god-mode stats +// never temporarily changes the live player; the caller still owns all +// referenced maps (serialize on the owner goroutine). +func (s *AccountStore) MarshalCharacter(p *Player) ([]byte, error) { snapshot := *p if snapshot.GodMode && snapshot.GodBackup != nil { snapshot.Skills = snapshot.GodBackup @@ -113,11 +127,7 @@ func (s *AccountStore) SaveCharacter(p *Player) error { snapshot.HP = snapshot.MaxHP() } } - data, err := yaml.Marshal(&snapshot) - if err != nil { - return err - } - return persistence.WriteFile(path, data, 0600) + return yaml.Marshal(&snapshot) } func (s *AccountStore) CharacterExists(name string) bool { diff --git a/internal/world/mob.go b/internal/world/mob.go index f33b2f9..a794bdd 100644 --- a/internal/world/mob.go +++ b/internal/world/mob.go @@ -722,3 +722,27 @@ func (s *MobStore) SeedMobs(roomID int, entries []RoomMob) { s.instances[instID] = inst } } + +// RekeyRooms remaps every live mob instance's RoomID and HomeRoomID per +// idMap (oldID -> newID), including wander rooms. Uses delete-all-then- +// set-all so a swap cannot clobber an entry mid-rekey. +func (s *MobStore) RekeyRooms(idMap map[int]int) { + if len(idMap) == 0 { + return + } + s.mu.Lock() + defer s.mu.Unlock() + for _, inst := range s.instances { + if newID, ok := idMap[inst.RoomID]; ok && newID != inst.RoomID { + inst.RoomID = newID + } + if newID, ok := idMap[inst.HomeRoomID]; ok && newID != inst.HomeRoomID { + inst.HomeRoomID = newID + } + for i, room := range inst.WanderRooms { + if newID, ok := idMap[room]; ok && newID != room { + inst.WanderRooms[i] = newID + } + } + } +} diff --git a/internal/world/world.go b/internal/world/world.go index b582faf..c5d0841 100644 --- a/internal/world/world.go +++ b/internal/world/world.go @@ -4,6 +4,7 @@ import ( "fmt" "os" "path/filepath" + "strconv" "strings" "sync" @@ -175,3 +176,61 @@ func (w *World) ClearRoomState(roomID int) { } } } + +// RekeyRooms remaps room-keyed runtime state per idMap (oldID -> newID): +// ground items, seeded markers, and object-state keys ("roomID:..."). +// Uses delete-all-then-set-all so a swap cannot clobber an entry mid-rekey. +func (w *World) RekeyRooms(idMap map[int]int) { + if len(idMap) == 0 { + return + } + w.mu.Lock() + defer w.mu.Unlock() + movedGround := make(map[int][]*groundEntry, len(w.groundItems)) + for roomID, entries := range w.groundItems { + if newID, ok := idMap[roomID]; ok && newID != roomID { + movedGround[newID] = entries + delete(w.groundItems, roomID) + } + } + for newID, entries := range movedGround { + w.groundItems[newID] = entries + } + movedSeeded := make(map[int]bool, len(w.seeded)) + for roomID := range w.seeded { + if newID, ok := idMap[roomID]; ok && newID != roomID { + movedSeeded[newID] = true + delete(w.seeded, roomID) + } + } + for newID := range movedSeeded { + w.seeded[newID] = true + } + type objKeyMove struct { + oldKey, newKey string + st *ObjState + } + var moves []objKeyMove + for k, st := range w.objStates { + roomStr, _, ok := strings.Cut(k, ":") + if !ok { + continue + } + roomID, err := strconv.Atoi(roomStr) + if err != nil { + continue + } + newID, ok := idMap[roomID] + if !ok || newID == roomID { + continue + } + newKey := fmt.Sprintf("%d:%s", newID, k[len(roomStr)+1:]) + moves = append(moves, objKeyMove{k, newKey, st}) + } + for _, m := range moves { + delete(w.objStates, m.oldKey) + } + for _, m := range moves { + w.objStates[m.newKey] = m.st + } +} -- cgit v1.2.3