diff options
Diffstat (limited to 'building_guide')
| -rw-r--r-- | building_guide/concurrency.md | 122 | ||||
| -rw-r--r-- | building_guide/foundation_review.md | 209 |
2 files changed, 122 insertions, 209 deletions
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. |