diff options
Diffstat (limited to 'internal/game')
| -rw-r--r-- | internal/game/cmd_get.go | 11 | ||||
| -rw-r--r-- | internal/game/economy.go | 61 | ||||
| -rw-r--r-- | internal/game/economy_recovery_identity_test.go | 128 | ||||
| -rw-r--r-- | internal/game/game.go | 28 | ||||
| -rw-r--r-- | internal/game/hacking/liars_dice.go | 10 | ||||
| -rw-r--r-- | internal/game/hacking/liars_dice_test.go | 68 | ||||
| -rw-r--r-- | internal/game/inventory_pickup_test.go | 55 | ||||
| -rw-r--r-- | internal/game/owner.go | 9 | ||||
| -rw-r--r-- | internal/game/owner_regression_test.go | 14 | ||||
| -rw-r--r-- | internal/game/shutdown_admission_test.go | 55 |
10 files changed, 400 insertions, 39 deletions
diff --git a/internal/game/cmd_get.go b/internal/game/cmd_get.go index d538afc..515d2ab 100644 --- a/internal/game/cmd_get.go +++ b/internal/game/cmd_get.go @@ -68,11 +68,10 @@ func (g *Game) doGet(sess *net.Session, input string) { qty = available } - freeSlot := p.FirstFreeSlot() - if freeSlot == -1 { - sess.WriteLine("Your inventory is full.") - return - } + // No premature free-slot gate here: a stackable pickup may merge into an + // existing stack even when every slot is occupied, so capacity is only + // required when a new slot is actually needed (matching doGetAll and + // doGetAllNamed). See TestPickupMergesIntoFullInventory. if def != nil && def.Stackable { for i := 0; i < 28; i++ { @@ -89,7 +88,7 @@ func (g *Game) doGet(sess *net.Session, input string) { return } } - freeSlot = p.FirstFreeSlot() + freeSlot := p.FirstFreeSlot() if freeSlot == -1 { sess.WriteLine("Your inventory is full.") return diff --git a/internal/game/economy.go b/internal/game/economy.go index e917fa6..be3f5ef 100644 --- a/internal/game/economy.go +++ b/internal/game/economy.go @@ -234,6 +234,24 @@ func (rec *economyRecord) key() string { return rec.Player } +// payoutIdentifies reports whether two journal records belong to the same +// character identity. ID-carrying records match only on the stable ID: a +// different character that reused the display name must never supply (or +// consume) another character's resolution evidence. Legacy records without +// IDs keep the historical name-based match among themselves. +func payoutIdentifies(a, b economyRecord) bool { + if a.PlayerID != "" && b.PlayerID != "" { + return a.PlayerID == b.PlayerID + } + if a.PlayerID != "" || b.PlayerID != "" { + // Mixed legacy/ID records: refuse to match on name alone so an ID + // can never be resolved through a name that another character may + // have reused. + return false + } + return strings.EqualFold(a.Player, b.Player) +} + // reloadStakes rebuilds the outstanding-stake index from the journal file: // open wager records remain tracked (and refundable/resolvable) after // recovery compaction. @@ -435,16 +453,14 @@ func (g *Game) recoverEconomy() { } loadPlayer := func(rec *economyRecord) *player.Player { if rec.PlayerID != "" { + // A record carrying a stable ID resolves to THAT identity only. + // Never fall back to the display name: a different character + // that later reused the name would collect the original + // character's payout and the original liability would be + // settled as paid. See TestRecoveryDoesNotPayReusedCharacterName. if p, ok := playersByID[rec.PlayerID]; ok { return p } - } - if rec.Player != "" { - if p, ok := playersByName[strings.ToLower(rec.Player)]; ok { - return p - } - } - if rec.PlayerID != "" { // A character file added since the index was built: load every // file once and match by stable ID. if names, err := g.AccountStore.ListCharacterNames(); err == nil { @@ -456,11 +472,20 @@ func (g *Game) recoverEconomy() { } } } + // Identity not found (character deleted or unreadable): leave + // the record open so a later attempt (or operator inspection) + // can resolve it; do not silently drop or reassign money. + fmt.Fprintf(os.Stderr, "economy recovery: load %s (id %q): character not found\n", rec.Player, rec.PlayerID) + return nil + } + // Legacy record written before stable IDs existed: the (renameable) + // name is the only identity available. + if rec.Player != "" { + if p, ok := playersByName[strings.ToLower(rec.Player)]; ok { + return p + } } - // 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 (id %q): character not found\n", rec.Player, rec.PlayerID) + fmt.Fprintf(os.Stderr, "economy recovery: load %s: character not found\n", rec.Player) return nil } // laterPayoutEvidence reports whether any non-void payout record was @@ -476,9 +501,10 @@ func (g *Game) recoverEconomy() { // instead marks a refund in progress: its credit part is re-applied by // phase 1 and the chip part (if the debit was durable) still comes // back in chips. - laterPayoutEvidence := func(name string, seq int) (resolved bool, settlement bool) { + laterPayoutEvidence := func(rec *economyRecord, seq int) (resolved bool, settlement bool) { for _, fr := range folded { - if fr.rec.Kind == "payout" && fr.rec.Player == name && fr.rec.Status != "void" && fr.seq > seq { + if fr.rec.Kind == "payout" && fr.rec.Status != "void" && fr.seq > seq && + payoutIdentifies(fr.rec, *rec) { resolved = true if !fr.rec.Refund { settlement = true @@ -499,6 +525,9 @@ func (g *Game) recoverEconomy() { // re-application as its own durability and silently drop the owed // payout — the group is settled together instead. type payoutSig struct { + // player is the record's stable identity key (PlayerID when known, + // else the name): grouping by display name alone would merge two + // different characters that reused the same name. player string before, after int credit int @@ -507,7 +536,7 @@ func (g *Game) recoverEconomy() { for i := range folded { rec := &folded[i].rec if rec.Kind == "payout" && rec.Status == "open" { - sig := payoutSig{rec.Player, rec.CreditsBefore, rec.CreditsAfter, rec.Credit} + sig := payoutSig{rec.key(), rec.CreditsBefore, rec.CreditsAfter, rec.Credit} siblings[sig] = append(siblings[sig], rec) } } @@ -521,7 +550,7 @@ func (g *Game) recoverEconomy() { if p == nil { continue } - sig := payoutSig{rec.Player, rec.CreditsBefore, rec.CreditsAfter, rec.Credit} + sig := payoutSig{rec.key(), rec.CreditsBefore, rec.CreditsAfter, rec.Credit} group := siblings[sig] switch { case p.Credits == rec.CreditsBefore: @@ -589,7 +618,7 @@ func (g *Game) recoverEconomy() { } for k := len(openWagers) - 1; k >= 0; k-- { rec := &folded[openWagers[k]].rec - resolvedByPayout, resolvedBySettlement := laterPayoutEvidence(rec.Player, folded[openWagers[k]].seq) + resolvedByPayout, resolvedBySettlement := laterPayoutEvidence(rec, folded[openWagers[k]].seq) p := loadPlayer(rec) if p == nil { continue diff --git a/internal/game/economy_recovery_identity_test.go b/internal/game/economy_recovery_identity_test.go new file mode 100644 index 0000000..eceeecc --- /dev/null +++ b/internal/game/economy_recovery_identity_test.go @@ -0,0 +1,128 @@ +package game + +// Identity-resolution regressions for economy recovery: a record carrying a +// stable character ID must never be resolved — paid or resolved — through a +// display name that a different character later reused, and money recovery +// must run only AFTER lifecycle-journal rollback (a rolled-back rename +// restores an older character before-image that must not erase compensation +// already applied). + +import ( + "os" + "path/filepath" + "testing" + + "thehouseoficarus/internal/persistence" +) + +// TestRecoveryDoesNotPayReusedCharacterName: an owed payout journaled for +// identity A (named "alice") must not be paid to identity B after B reuses +// the name, and A's unresolved liability must stay open for operator +// inspection instead of being silently settled against B's balance. +func TestRecoveryDoesNotPayReusedCharacterName(t *testing.T) { + h := newHarness(t) + h.seedAccount("alice", "pw", false) + p := h.seedCharacter("alice", "alice") + h.g.RunOnOwner(func() { + p.ID = "original-character" + p.Credits = 90 + if err := h.g.AccountStore.SaveCharacter(p); err != nil { + t.Error(err) + return + } + // A failed payout save left the owed money journaled for recovery. + if _, err := h.g.Economy.start("payout", p, 0, 20, 90, 110, 0, 0); err != nil { + t.Error(err) + return + } + // Delete the original character, then reuse its name for a + // different identity. + p.ID = "replacement-character" + if err := h.g.AccountStore.SaveCharacter(p); err != nil { + t.Error(err) + return + } + h.g.recoverEconomy() + }) + loaded, err := h.g.AccountStore.LoadCharacter("alice") + if err != nil { + t.Fatal(err) + } + if loaded.Credits != 90 { + t.Errorf("replacement identity received someone else's payout: credits=%d, want 90", loaded.Credits) + } + if open := h.g.Economy.loadOpen(); len(open) != 1 { + t.Errorf("original identity's unresolved liability was discarded: open records=%d", len(open)) + } +} + +// TestEconomyRecoveryRunsAfterLifecycleRollback: a leftover lifecycle +// transaction (a rename that never reached its commit point) restores the +// character's before-image at startup, and must do so BEFORE money +// compensation — otherwise the compensation's settled/compacted records are +// gone and the rollback silently erases a payout that was already applied. +func TestEconomyRecoveryRunsAfterLifecycleRollback(t *testing.T) { + h := newHarness(t) + h.seedAccount("alice", "pw", false) + p := h.seedCharacter("alice", "alice") + h.g.RunOnOwner(func() { + p.ID = "stable-character" + p.Credits = 90 + if err := h.g.AccountStore.SaveCharacter(p); err != nil { + t.Error(err) + return + } + // A failed payout save left the owed money journaled for recovery. + if _, err := h.g.Economy.start("payout", p, 0, 20, 90, 110, 0, 0); err != nil { + t.Error(err) + return + } + // Simulate a crash mid-rename: apply the move and the renamed + // character file directly, then leave the lifecycle journal on + // disk so the next startup rolls it back. + oldPath := h.g.AccountStore.CharPath("alice") + newPath := h.g.AccountStore.CharPath("renamed") + txn := persistence.NewTransaction(h.g.journalDir()) + if err := txn.Move(oldPath, newPath); err != nil { + t.Error(err) + return + } + p.Name = "renamed" + data, err := h.g.AccountStore.MarshalCharacter(p) + if err != nil { + t.Error(err) + return + } + if err := txn.Replace(newPath, data, 0600); err != nil { + t.Error(err) + return + } + journal, err := txn.MarshalJournal() + if err != nil { + t.Error(err) + return + } + if err := os.WriteFile(filepath.Join(h.g.journalDir(), "txn-audit.json"), journal, 0600); err != nil { + t.Error(err) + return + } + if err := os.Rename(oldPath, newPath); err != nil { + t.Error(err) + return + } + if err := h.g.AccountStore.SaveCharacter(p); err != nil { + t.Error(err) + return + } + }) + // A fresh Game over the saved files simulates the restart. + restarted := harnessFromDir(t, h.dir) + loaded, err := restarted.g.AccountStore.LoadCharacter("alice") + if err != nil { + t.Fatal(err) + } + if loaded.Credits != 110 { + t.Errorf("lifecycle rollback erased recovered payout: credits=%d, want 110; open liabilities=%d", + loaded.Credits, len(restarted.g.Economy.loadOpen())) + } +} diff --git a/internal/game/game.go b/internal/game/game.go index 7f29d1f..ca38e27 100644 --- a/internal/game/game.go +++ b/internal/game/game.go @@ -135,6 +135,15 @@ func New(dataDir string, colorConfig *config.ColorsConfig, constantColorConfig * // initialization cycle with the shutdown coordinator. g.shutdownHook = g.RequestShutdown g.Economy = newEconomyJournal(dataDir) + // Lifecycle-journal rollback MUST run before economy recovery: a rolled- + // back rename transaction restores an older character before-image, and + // if money compensation ran first its settled/compacted records would no + // longer be recoverable — the rollback would silently erase a payout + // that was already re-applied. Rollback also precedes every store read + // below (courses, mods, techs, craft index) that a leftover transaction + // may reference. See the TestEconomyRecoveryRunsAfterLifecycleRollback + // regression. + g.recoverJournals() // Compensate open money records from any prior crash before any // session can connect. g.recoverEconomy() @@ -144,9 +153,7 @@ func New(dataDir string, colorConfig *config.ColorsConfig, constantColorConfig * 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() + // One-time stable-identity migration. g.backfillCharacterIDs() g.ValidateAndLog() return g @@ -273,9 +280,22 @@ func (g *Game) saveAllOnlineCharacters() error { // 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. +// mutation executes on the owner goroutine. Once shutdown has begun, new +// input is REJECTED (not queued and not executed inline): the shutdown +// sequence owns final persistence, and the transports keep draining until +// they close, so an accepted line could mutate characters after the final +// save or race the refund/save barrier. func (g *Game) HandleSessionOnOwner(sess *net.Session, input string) { + if g.isShuttingDown() { + return + } g.RunOnOwner(func() { + // Re-checked at task execution: the flag may flip between + // submission and execution, and RunOnOwner must never fall back + // to inline execution after the owner has stopped. + if g.isShuttingDown() { + return + } g.HandleSession(sess, input) }) } diff --git a/internal/game/hacking/liars_dice.go b/internal/game/hacking/liars_dice.go index 1dd27b6..8e21b94 100644 --- a/internal/game/hacking/liars_dice.go +++ b/internal/game/hacking/liars_dice.go @@ -175,7 +175,7 @@ func (l *LiarsDiceGame) doCall(player bool) (string, bool, bool) { } } - return l.checkGameOver(sb) + return l.checkGameOver(&sb) } func (l *LiarsDiceGame) doExact() (string, bool, bool) { @@ -202,10 +202,14 @@ func (l *LiarsDiceGame) doExact() (string, bool, bool) { l.playerLost++ } - return l.checkGameOver(sb) + return l.checkGameOver(&sb) } -func (l *LiarsDiceGame) checkGameOver(sb strings.Builder) (string, bool, bool) { +// checkGameOver resolves a completed challenge/exact exchange. The builder is +// passed BY POINTER: it already holds the resolution text, and copying a +// non-zero strings.Builder by value panics — a panic here runs on the game +// owner goroutine, which has no recovery, so it would terminate the server. +func (l *LiarsDiceGame) checkGameOver(sb *strings.Builder) (string, bool, bool) { if len(l.playerDice) == 0 { sb.WriteString("\nYou've lost all your fragments. Connection terminated.") l.gameOver = true diff --git a/internal/game/hacking/liars_dice_test.go b/internal/game/hacking/liars_dice_test.go new file mode 100644 index 0000000..59ebb92 --- /dev/null +++ b/internal/game/hacking/liars_dice_test.go @@ -0,0 +1,68 @@ +package hacking + +import ( + "strings" + "testing" +) + +// Regression: round resolution (challenge and exact calls) passed an +// already-used strings.Builder BY VALUE into checkGameOver, panicking with +// "illegal use of non-zero Builder copied by value" on ordinary input — a +// forced challenge resolves on the very first bid. A panic here runs on the +// game owner goroutine, which has no recovery, so it terminated the server. +// See building_guide/concurrency.md for the owner's no-recovery guarantee. + +func TestLiarsDiceForcedChallengeDoesNotPanic(t *testing.T) { + g := NewLiarsDiceGame() + g.Init(1) + // An impossible bid deterministically forces an immediate AI challenge. + out, _, _ := g.HandleInput("bid 100 6") + if !strings.Contains(out, "Actual count") { + t.Fatalf("challenge output missing the reveal: %q", out) + } + if !strings.Contains(out, "You lose a fragment") && !strings.Contains(out, "System loses a fragment") { + t.Fatalf("challenge did not resolve a fragment loss: %q", out) + } +} + +func TestLiarsDiceCallAndExactResolveWithoutPanic(t *testing.T) { + // Call path: bid, let the system raise, then challenge it. + g := NewLiarsDiceGame() + g.Init(1) + if out, _, _ := g.HandleInput("bid 2 3"); out == "" { + t.Fatal("opening bid produced no output") + } + if out, _, _ := g.HandleInput("call"); out == "" { + t.Fatal("call produced no output") + } + + // Exact path: bid, let the system raise, then claim exact. + g = NewLiarsDiceGame() + g.Init(1) + if out, _, _ := g.HandleInput("bid 2 3"); out == "" { + t.Fatal("opening bid produced no output") + } + if out, _, _ := g.HandleInput("exact"); out == "" { + t.Fatal("exact produced no output") + } +} + +func TestLiarsDiceFullLossGameResolvesWithoutPanic(t *testing.T) { + // Bid high enough that the system always calls and the player always + // loses a fragment, driving the game to its terminal resolution. + g := NewLiarsDiceGame() + g.Init(1) + for i := 0; i < 10; i++ { + out, done, won := g.HandleInput("bid 100 6") + if done { + if won { + t.Fatal("all-in losing bids cannot win") + } + return + } + if out == "" { + t.Fatal("round produced no output") + } + } + t.Fatal("game did not terminate within 10 forced-loss rounds") +} diff --git a/internal/game/inventory_pickup_test.go b/internal/game/inventory_pickup_test.go new file mode 100644 index 0000000..a0d267e --- /dev/null +++ b/internal/game/inventory_pickup_test.go @@ -0,0 +1,55 @@ +package game + +// Regression: picking up a stackable item must merge into an existing stack +// even when every inventory slot is occupied — capacity is required only +// when a NEW slot is needed, matching the get-all paths. + +import ( + "path/filepath" + "testing" + + "thehouseoficarus/internal/net" + "thehouseoficarus/internal/player" +) + +func TestPickupMergesIntoFullInventory(t *testing.T) { + h := newHarness(t) + h.seedAccount("alice", "pw", false) + p := h.seedCharacter("alice", "alice") + writeSeedFile(t, filepath.Join(h.dir, "items", "chips.yaml"), []byte("id: chips\nname: chips\nstackable: true\n")) + sess := &net.Session{Conn: newChanConn(), Player: p, State: net.StateGame} + h.g.RunOnOwner(func() { + h.g.ItemStore.Reload(h.dir) + for i := 0; i < 28; i++ { + p.SetInvSlot(i, &player.InventorySlot{ItemID: "filler", Quantity: 1}) + } + p.SetInvSlot(0, &player.InventorySlot{ItemID: "chips", Quantity: 10}) + h.g.World.AddGroundItem(1, "chips", 5) + h.g.doGet(sess, "chips") + if got := countChips(p); got != 15 { + t.Errorf("pickup rejected despite existing chip stack: have %d, want 15", got) + } + }) +} + +func TestPickupFullInventoryWithoutStackStillRefused(t *testing.T) { + h := newHarness(t) + h.seedAccount("alice", "pw", false) + p := h.seedCharacter("alice", "alice") + writeSeedFile(t, filepath.Join(h.dir, "items", "chips.yaml"), []byte("id: chips\nname: chips\nstackable: true\n")) + sess := &net.Session{Conn: newChanConn(), Player: p, State: net.StateGame} + h.g.RunOnOwner(func() { + h.g.ItemStore.Reload(h.dir) + for i := 0; i < 28; i++ { + p.SetInvSlot(i, &player.InventorySlot{ItemID: "filler", Quantity: 1}) + } + h.g.World.AddGroundItem(1, "chips", 5) + h.g.doGet(sess, "chips") + if got := countChips(p); got != 0 { + t.Errorf("pickup without an existing stack must be refused, have %d", got) + } + if got := h.g.World.GroundItems(1)["chips"]; got != 5 { + t.Errorf("refused pickup must leave the ground stack intact, have %d", got) + } + }) +} diff --git a/internal/game/owner.go b/internal/game/owner.go index 8546d10..ce1f687 100644 --- a/internal/game/owner.go +++ b/internal/game/owner.go @@ -136,10 +136,11 @@ func (g *Game) run(fn func()) { 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. A full queue never - // takes this path: enqueue applies backpressure instead. - fn() + // Admissions are closed: DROP the task. Executing it on the + // caller's goroutine would mutate live state concurrently with the + // shutdown sequence and after the final-save barrier (network + // transports are still draining until transportStop runs). See + // TestShutdownRejectsSessionInput. return } <-task diff --git a/internal/game/owner_regression_test.go b/internal/game/owner_regression_test.go index 1ccd25f..3ba4bd4 100644 --- a/internal/game/owner_regression_test.go +++ b/internal/game/owner_regression_test.go @@ -52,17 +52,19 @@ func TestFullOwnerQueueAppliesBackpressure(t *testing.T) { } } -// TestStoppedOwnerRunsStragglersInline: after the owner stops (graceful -// shutdown), straggler submissions execute inline rather than blocking -// forever — the documented admissions-closed behavior. -func TestStoppedOwnerRunsStragglersInline(t *testing.T) { +// TestStoppedOwnerDropsStragglers: after the owner stops (graceful +// shutdown), straggler submissions are dropped, never executed inline on the +// caller's goroutine (which would mutate live state concurrently with the +// shutdown sequence and after the final-save barrier) and never block +// forever. +func TestStoppedOwnerDropsStragglers(t *testing.T) { g := &Game{owner: newOwner()} g.stopOwner() ran := make(chan struct{}) go g.run(func() { close(ran) }) select { case <-ran: - case <-time.After(5 * time.Second): - t.Fatal("straggler task blocked forever after the owner stopped") + t.Fatal("straggler task executed after the owner stopped; it must be dropped") + case <-time.After(100 * time.Millisecond): } } diff --git a/internal/game/shutdown_admission_test.go b/internal/game/shutdown_admission_test.go new file mode 100644 index 0000000..6aa4be5 --- /dev/null +++ b/internal/game/shutdown_admission_test.go @@ -0,0 +1,55 @@ +package game + +// Shutdown-admission regression: once shutdown has begun, session input must +// never mutate game state — not during the drain window, and especially not +// after the owner has stopped (the transports keep draining until they +// close, so an accepted line would execute concurrently with the shutdown +// sequence or after the final-save barrier). + +import ( + "testing" + + "thehouseoficarus/internal/net" +) + +func TestShutdownRejectsSessionInput(t *testing.T) { + h := newHarness(t) + h.seedAccount("alice", "pw", false) + h.seedCharacter("alice", "alice") + sess, _ := h.connect("alice") + sess.State = net.StateGame + h.g.setShuttingDown() + h.g.stopOwner() + + // The network listeners are closed only AFTER the owner stops, so + // connections can still submit lines through the production entry + // point. Every one of them must be rejected. + h.g.HandleSessionOnOwner(sess, "look") + + // A durable mutation gives an unambiguous admission check: description + // input would rewrite the character. + sess.State = net.StateChangeDesc + h.g.HandleSessionOnOwner(sess, "Input after final save") + if sess.Player.Description == "Input after final save" { + t.Error("session input still mutates character after shutdown/owner stop") + } +} + +func TestShutdownRejectsInputDuringDrain(t *testing.T) { + h := newHarness(t) + h.seedAccount("alice", "pw", false) + h.seedCharacter("alice", "alice") + sess, _ := h.connect("alice") + sess.State = net.StateGame + h.g.setShuttingDown() + + // During the drain window the owner is still alive, but new input is + // already rejected: only work accepted before shutdown executes. + h.g.HandleSessionOnOwner(sess, "look") + sess.State = net.StateChangeDesc + h.g.HandleSessionOnOwner(sess, "Input during drain") + if sess.Player.Description == "Input during drain" { + t.Error("session input accepted after shutdown began") + } + h.barrier() +} |
