diff options
| -rw-r--r-- | FIXES.md | 26 | ||||
| -rw-r--r-- | internal/admin/correct_audit_fixes_test.go | 33 | ||||
| -rw-r--r-- | internal/admin/undo.go | 99 | ||||
| -rw-r--r-- | internal/casino/core.go | 3 | ||||
| -rw-r--r-- | internal/casino/pai_gow.go | 66 | ||||
| -rw-r--r-- | internal/game/account_txn.go | 7 | ||||
| -rw-r--r-- | internal/game/act_talk.go | 4 | ||||
| -rw-r--r-- | internal/game/casino_house_edge.go | 16 | ||||
| -rw-r--r-- | internal/game/correct_audit_fixes_test.go | 94 | ||||
| -rw-r--r-- | internal/game/exit_migrate.go | 13 | ||||
| -rw-r--r-- | internal/game/harness_test.go | 33 | ||||
| -rw-r--r-- | internal/ui/table.go | 7 | ||||
| -rw-r--r-- | internal/world/room_migrate.go | 13 |
13 files changed, 311 insertions, 103 deletions
@@ -107,14 +107,30 @@ Nothing was staged or committed. The audit artifacts (`AUDIT.md`, `reports/`) ar - **Test coverage**: added `TestGatherYieldsDrop`, an end-to-end gather test (seed object + pickaxe, start the action, tick to a drop) — the first coverage for the gather system the audit flagged. - **Investigated (no change)**: the craps Fire bet. Settling it at the 4th/5th point as the audit suggested would cap the payout at the lower tier and under-pay; the bet must ride to the seven-out to pay the highest tier reached. Left as-is and noted here. +## Tranche 8 (this pass) + +- **Undo**: added an explicit before/after snapshot-existence flag (`ChangeDesc.OldExists`/`NewExists`, carried through `ExtraFile` and the persisted `undoEntry`). `requireSnapshot` now keys on existence, not `len == 0`, so an intentionally empty file can be undone after a restart while a missing (failed-read) image is still refused. Older history files infer the flag from non-empty content. Test: `TestUndoRestoresIntentionallyEmptyFile`. +- **Refactor**: the duplicated `world.remapInt` and `game.remapScalar` are now one exported `world.RemapInt`, used by both the room-file and player-state migrations. + +## Tranche 9 (this pass) + +- **Test coverage**: added `TestBurnCreatesFire`, an end-to-end firemaking test (seed logs + tinderbox, start the action, tick until a fire object appears) — the burn coverage the audit called out. +- **Test infrastructure**: added `newHarnessWithTicks`, a harness variant that registers the production master tick subscription, plus `TestHarnessWithTicksCompletesDisconnect`, which exercises the real `Hub.Remove → DisconnectTick → onRemove` countdown end-to-end (the gap the audit found in the disconnect race tests). +- **Refactor**: exported `ui.FitWidths` (the `Table.Render` shrink algorithm) and replaced the duplicated shrink loop in `casino_house_edge.go`, so the two cannot disagree on which column yields. + +## Tranche 10 (this pass) + +- **Bug fix (previously missed)**: a terminal talk node (messages, no options, no goto) now clears the talk action in `showTalkNode`, so it no longer lingers as a stale action that suppresses the prompt and no-ops every tick. Test: `TestTerminalTalkNodeClearsAction`. +- **Test coverage**: added `TestObjectStealYieldsLoot`, an end-to-end object-steal test (seed a stealable stall, start the action, tick to loot). Talk and steal are now covered (gather and burn were in earlier tranches). +- **Refactor**: `paiGowTable.phase` is now the typed `tablePhase` with `phaseSetting`/`phaseSettling` constants, removing the 29 raw phase-string literals (a safe subset of the `tableCore` consistency work). +- **Docs**: `recoverJournals` now documents that it must run only at startup or on the owner goroutine, making the mid-game `Recover` invariant explicit. + ## Still open -- **Explicit undo snapshot-existence flag** (the current fix safely refuses an intentionally empty before-image). -- **Persistence** `Recover` mid-game misuse. - **Casino**: craps Fire bet forfeiture on disconnect (design decision; shutdown refunds). -- **Structural refactors** remaining from `AUDIT.md` §3–4 (`ui.Table` shrink consolidation, `tableCore` for pai gow, `refs` struct in validate, `remapInt`/`remapScalar`). -- **Test infrastructure**: CI wiring for `test-race`, a master-tick harness option, and coverage for steal/burn/talk. +- **Structural refactors** remaining from `AUDIT.md` §3–4 (`tableCore` for pai gow — now partly addressed by the typed phase — `refs` struct in validate, flex-aware predictions table). +- **CI**: wire `make test-race` into a workflow. ## Suggested next step -Tranche 8: the remaining structural refactors and the steal/burn/talk test coverage. Keep `make lint`, `go test ./...`, and `go test -race` green. +The remaining structural refactors are cosmetic DRY with real regression risk and no functional benefit; the correctness backlog is now empty. Suggest stopping here unless a specific item is wanted. Keep `make lint`, `go test ./...`, and `go test -race` green. diff --git a/internal/admin/correct_audit_fixes_test.go b/internal/admin/correct_audit_fixes_test.go index d7b713e..282e943 100644 --- a/internal/admin/correct_audit_fixes_test.go +++ b/internal/admin/correct_audit_fixes_test.go @@ -89,3 +89,36 @@ func TestResolveDataPathRejectsSymlinkEscape(t *testing.T) { t.Fatalf("normal path rejected: %v", err) } } + +// Regression: an intentionally empty before-image must be restorable after a +// restart. The existence flag distinguishes it from a missing (failed-read) +// image, which both persist as "". +func TestUndoRestoresIntentionallyEmptyFile(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "rooms", "1.yaml") + if err := os.MkdirAll(filepath.Dir(path), 0755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, []byte("id: 1\n"), 0644); err != nil { + t.Fatal(err) + } + us := NewUndoStack(dir) + us.Push(ChangeDesc{ + Description: "write to an originally empty file", + FilePath: path, + OldContent: []byte{}, // exists, but empty + NewContent: []byte("id: 1\n"), + }) + // Reload from disk to exercise the persistence round trip. + us2 := NewUndoStack(dir) + if ch := us2.Undo(); ch == nil { + t.Fatalf("undo of an empty-but-existing before-image was refused: %v", us2.LastError()) + } + data, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + if len(data) != 0 { + t.Fatalf("file not restored to empty: %q", data) + } +} diff --git a/internal/admin/undo.go b/internal/admin/undo.go index c3231e3..d9ef263 100644 --- a/internal/admin/undo.go +++ b/internal/admin/undo.go @@ -21,15 +21,23 @@ type ExtraFile struct { IsDelete bool `json:"is_delete,omitempty"` IsCreate bool `json:"is_create,omitempty"` IsDirDelete bool `json:"is_dir_delete,omitempty"` + OldExists bool `json:"-"` + NewExists bool `json:"-"` } type ChangeDesc struct { - Time string `json:"time"` - Description string `json:"description"` - FilePath string `json:"file_path"` - NewFilePath string `json:"new_file_path,omitempty"` - OldContent []byte `json:"old_content"` - NewContent []byte `json:"new_content"` + Time string `json:"time"` + Description string `json:"description"` + FilePath string `json:"file_path"` + NewFilePath string `json:"new_file_path,omitempty"` + OldContent []byte `json:"old_content"` + NewContent []byte `json:"new_content"` + // OldExists/NewExists record whether the before/after image was actually + // captured. A failed read leaves nil content, but nil and an empty file + // both persist as ""; the flag distinguishes them so undo can restore an + // intentionally empty file yet still refuse a missing snapshot. + OldExists bool `json:"old_exists,omitempty"` + NewExists bool `json:"new_exists,omitempty"` IsDelete bool `json:"is_delete"` IsCreate bool `json:"is_create"` IsDirDelete bool `json:"is_dir_delete,omitempty"` @@ -164,6 +172,23 @@ func (us *UndoStack) Push(desc ChangeDesc) { us.mu.Lock() defer us.mu.Unlock() desc.Time = time.Now().Format(time.RFC3339) + // Infer the existence flags from the captured content: a non-nil image + // (including an empty file's []byte{}) means it exists; nil means the read + // failed or the file was absent. Callers may set the flags explicitly. + if desc.OldContent != nil { + desc.OldExists = true + } + if desc.NewContent != nil { + desc.NewExists = true + } + for i := range desc.ExtraFiles { + if desc.ExtraFiles[i].OldContent != nil { + desc.ExtraFiles[i].OldExists = true + } + if desc.ExtraFiles[i].NewContent != nil { + desc.ExtraFiles[i].NewExists = true + } + } us.history = append(us.history, desc) us.redo = nil if len(us.history) > us.maxSize { @@ -235,12 +260,13 @@ func (us *UndoStack) Redo() *ChangeDesc { return &last } -// requireSnapshot rejects a nil before/after image on a change that must -// restore real content. A failed before-image read leaves nil, and writing -// it would truncate the file to zero bytes (which parses as a zero-value -// def), so a missing image must fail the undo/redo instead. -func requireSnapshot(content []byte, path, action string) error { - if content == nil { +// requireSnapshot rejects a change whose before/after image was never +// recorded (a failed read). Writing a missing image would truncate the file to +// zero bytes, which parses as a zero-value def. The existence flag (not the +// content length) is the source of truth, since nil and an empty file both +// persist as "". +func requireSnapshot(exists bool, path, action string) error { + if !exists { return fmt.Errorf("%s %s: no snapshot recorded", action, path) } return nil @@ -277,7 +303,7 @@ func (us *UndoStack) applyUndo(c ChangeDesc) error { return fmt.Errorf("recreate dir %s: %w", c.FilePath, err) } } else { - if c.OldContent == nil { + if !c.OldExists { return fmt.Errorf("restore deleted %s: no snapshot recorded", c.FilePath) } if err := us.writeSnapshot(c.FilePath, restored); err != nil { @@ -289,7 +315,7 @@ func (us *UndoStack) applyUndo(c ChangeDesc) error { return fmt.Errorf("remove created %s: %w", c.FilePath, err) } } else { - if err := requireSnapshot(c.OldContent, c.FilePath, "revert"); err != nil { + if err := requireSnapshot(c.OldExists, c.FilePath, "revert"); err != nil { return err } if err := us.writeSnapshot(c.FilePath, restored); err != nil { @@ -354,7 +380,7 @@ func (us *UndoStack) applyRedo(c ChangeDesc) error { if c.NewFilePath != "" { target = c.NewFilePath } - if err := requireSnapshot(c.NewContent, target, "reapply"); err != nil { + if err := requireSnapshot(c.NewExists, target, "reapply"); err != nil { return err } if err := us.writeSnapshot(target, restored); err != nil { @@ -380,14 +406,14 @@ func (us *UndoStack) applyUndoExtraFile(ef ExtraFile) error { return fmt.Errorf("undo mkdir: %w", err) } case ef.IsDelete: - if ef.OldContent == nil { + if !ef.OldExists { return fmt.Errorf("undo restore %s: no snapshot recorded", ef.FilePath) } if err := us.writeRestored(ef.FilePath, ef.OldContent, ef.NewFilePath); err != nil { return fmt.Errorf("undo restore %s: %w", ef.FilePath, err) } case ef.NewFilePath != "": - if err := requireSnapshot(ef.OldContent, ef.FilePath, "undo restore"); err != nil { + if err := requireSnapshot(ef.OldExists, ef.FilePath, "undo restore"); err != nil { return err } if err := os.Remove(ef.NewFilePath); err != nil && !os.IsNotExist(err) { @@ -397,7 +423,7 @@ func (us *UndoStack) applyUndoExtraFile(ef ExtraFile) error { return fmt.Errorf("undo restore %s: %w", ef.FilePath, err) } default: - if err := requireSnapshot(ef.OldContent, ef.FilePath, "undo revert"); err != nil { + if err := requireSnapshot(ef.OldExists, ef.FilePath, "undo revert"); err != nil { return err } if err := us.writeRestored(ef.FilePath, ef.OldContent); err != nil { @@ -418,7 +444,7 @@ func (us *UndoStack) applyRedoExtraFile(ef ExtraFile) error { return fmt.Errorf("redo create %s: %w", ef.FilePath, err) } case ef.NewFilePath != "": - if err := requireSnapshot(ef.NewContent, ef.NewFilePath, "redo create"); err != nil { + if err := requireSnapshot(ef.NewExists, ef.NewFilePath, "redo create"); err != nil { return err } if err := os.Remove(ef.FilePath); err != nil && !os.IsNotExist(err) { @@ -428,7 +454,7 @@ func (us *UndoStack) applyRedoExtraFile(ef ExtraFile) error { return fmt.Errorf("redo create %s: %w", ef.NewFilePath, err) } default: - if err := requireSnapshot(ef.NewContent, ef.FilePath, "redo reapply"); err != nil { + if err := requireSnapshot(ef.NewExists, ef.FilePath, "redo reapply"); err != nil { return err } if err := us.writeRestored(ef.FilePath, ef.NewContent); err != nil { @@ -561,6 +587,8 @@ type undoExtraEntry struct { NewFilePath string `json:"new_file_path,omitempty"` OldContent string `json:"old_content"` NewContent string `json:"new_content"` + OldExists bool `json:"old_exists,omitempty"` + NewExists bool `json:"new_exists,omitempty"` IsDelete bool `json:"is_delete,omitempty"` IsCreate bool `json:"is_create,omitempty"` IsDirDelete bool `json:"is_dir_delete,omitempty"` @@ -573,6 +601,8 @@ type undoEntry struct { NewFilePath string `json:"new_file_path,omitempty"` OldContent string `json:"old_content"` NewContent string `json:"new_content"` + OldExists bool `json:"old_exists,omitempty"` + NewExists bool `json:"new_exists,omitempty"` IsDelete bool `json:"is_delete"` IsCreate bool `json:"is_create"` IsDirDelete bool `json:"is_dir_delete,omitempty"` @@ -596,6 +626,8 @@ func (us *UndoStack) save() { NewFilePath: ef.NewFilePath, OldContent: string(us.redactContent(ef.OldContent, ef.FilePath, ef.NewFilePath)), NewContent: string(us.redactContent(ef.NewContent, ef.FilePath, ef.NewFilePath)), + OldExists: ef.OldExists, + NewExists: ef.NewExists, IsDelete: ef.IsDelete, IsCreate: ef.IsCreate, IsDirDelete: ef.IsDirDelete, @@ -608,6 +640,8 @@ func (us *UndoStack) save() { NewFilePath: c.NewFilePath, OldContent: string(us.redactContent(c.OldContent, c.FilePath, c.NewFilePath)), NewContent: string(us.redactContent(c.NewContent, c.FilePath, c.NewFilePath)), + OldExists: c.OldExists, + NewExists: c.NewExists, IsDelete: c.IsDelete, IsCreate: c.IsCreate, IsDirDelete: c.IsDirDelete, @@ -762,37 +796,32 @@ func (us *UndoStack) load() { toChanges := func(entries []undoEntry) []ChangeDesc { out := make([]ChangeDesc, 0, len(entries)) for _, e := range entries { - // A before-image recorded as nil is persisted as "" and would - // reload as a non-nil empty slice, defeating requireSnapshot and - // letting undo truncate the file to zero bytes. Map it back to nil - // so the guard fires. An intentionally empty before-image is also - // refused, which is safer than zeroing a live file. - oldContent := []byte(e.OldContent) - if e.OldContent == "" { - oldContent = nil - } + // The existence flag distinguishes a missing before-image (a failed + // read, persisted as "") from an intentionally empty file. Older + // history files predate the flag, so infer it from non-empty content; + // an old empty image stays missing (the safe behaviour). c := ChangeDesc{ Time: e.Time, Description: e.Description, FilePath: e.FilePath, NewFilePath: e.NewFilePath, - OldContent: oldContent, + OldContent: []byte(e.OldContent), NewContent: []byte(e.NewContent), + OldExists: e.OldExists || e.OldContent != "", + NewExists: e.NewExists || e.NewContent != "", IsDelete: e.IsDelete, IsCreate: e.IsCreate, IsDirDelete: e.IsDirDelete, IDMap: e.IDMap, } for _, ef := range e.ExtraFiles { - oldExtra := []byte(ef.OldContent) - if ef.OldContent == "" { - oldExtra = nil - } c.ExtraFiles = append(c.ExtraFiles, ExtraFile{ FilePath: ef.FilePath, NewFilePath: ef.NewFilePath, - OldContent: oldExtra, + OldContent: []byte(ef.OldContent), NewContent: []byte(ef.NewContent), + OldExists: ef.OldExists || ef.OldContent != "", + NewExists: ef.NewExists || ef.NewContent != "", IsDelete: ef.IsDelete, IsCreate: ef.IsCreate, IsDirDelete: ef.IsDirDelete, diff --git a/internal/casino/core.go b/internal/casino/core.go index 5631c0f..d04e046 100644 --- a/internal/casino/core.go +++ b/internal/casino/core.go @@ -15,6 +15,9 @@ const ( phaseBetting tablePhase = "betting" phaseInsurance tablePhase = "insurance" phasePlaying tablePhase = "playing" + // Pai gow's extra phases (it is not built on tableCore yet). + phaseSetting tablePhase = "setting" + phaseSettling tablePhase = "settling" ) // tableCore is the plumbing every multiplayer table game shares: the seat set, diff --git a/internal/casino/pai_gow.go b/internal/casino/pai_gow.go index 8f581db..55dc601 100644 --- a/internal/casino/pai_gow.go +++ b/internal/casino/pai_gow.go @@ -83,7 +83,7 @@ type paiGowTable struct { mu sync.Mutex rng *rand.Rand players []*pgPlayer // stable seat/deal/settlement order - phase string + phase tablePhase ticks int dealer []card dealerSplit pgSplit @@ -91,7 +91,7 @@ type paiGowTable struct { } func newPaiGowTable(room int, c TableConfig, rng *rand.Rand) tableGame { - return &paiGowTable{tableBase: &tableBase{RoomID: room, Config: c}, rng: rng, phase: "waiting"} + return &paiGowTable{tableBase: &tableBase{RoomID: room, Config: c}, rng: rng, phase: phaseWaiting} } func (t *paiGowTable) base() *tableBase { return t.tableBase } func (t *paiGowTable) player(name string) *pgPlayer { @@ -133,7 +133,7 @@ func (t *paiGowTable) seats() []string { func (t *paiGowTable) state() string { t.mu.Lock() defer t.mu.Unlock() - return t.phase + return string(t.phase) } func (t *paiGowTable) hintsLocked(name string) []MenuCommand { p := t.player(name) @@ -143,7 +143,7 @@ func (t *paiGowTable) hintsLocked(name string) []MenuCommand { commands := []MenuCommand{} faceUp := t.Config.PaiGow.Variant == PaiGowFaceUp switch t.phase { - case "waiting", "betting": + case phaseWaiting, phaseBetting: if p.main.Total == 0 { // Side bets are placed before the main wager: the main bet is what // deals, so there is no later chance to add them. @@ -158,7 +158,7 @@ func (t *paiGowTable) hintsLocked(name string) []MenuCommand { if p.main.Total > 0 || p.fortune.Total > 0 || p.pushace.Total > 0 { commands = append(commands, MenuCommand{Command: "unbet", Desc: "Take back all your wagers before the deal"}) } - case "setting": + case phaseSetting: if p.main.Total > 0 && !p.ready { if p.pending.high != nil { commands = append(commands, MenuCommand{Command: "set", Desc: "Lock this arrangement and reveal the dealer"}) @@ -173,7 +173,7 @@ func (t *paiGowTable) hintsLocked(name string) []MenuCommand { commands = append(commands, MenuCommand{Command: "autoset on", Desc: "Set a legal arrangement immediately"}) } } - case "settling": + case phaseSettling: commands = append(commands, MenuCommand{Command: "settle", Desc: "Retry the same pending payment"}) } return append(commands, MenuCommand{Command: "table", Desc: "View the felt"}, MenuCommand{Command: "rules", Desc: "Learn Pai Gow"}, MenuCommand{Command: "stop", Desc: "Leave after finishing your wager"}) @@ -187,7 +187,7 @@ func (t *paiGowTable) wagers(name string) []WagerRow { t.mu.Lock() defer t.mu.Unlock() p := t.player(name) - if p == nil || t.phase == "setting" || t.phase == "settling" { + if p == nil || t.phase == phaseSetting || t.phase == phaseSettling { return nil } rows := []WagerRow{{Bet: "main", Payout: "See the felt", Win: "Beat both dealer hands"}, {Bet: "fortune", Payout: "See Fortune felt", Win: "Best seven-card bonus; independent of main result"}} @@ -241,7 +241,7 @@ func pgCardNums(hand, all []card) []int { } func (t *paiGowTable) statusLocked(name string, showFelt bool) []Event { - s := &PaiGowSnapshot{Config: t.Config, Phase: t.phase, ShowFelt: showFelt} + s := &PaiGowSnapshot{Config: t.Config, Phase: string(t.phase), ShowFelt: showFelt} if p := t.player(name); p != nil { // The dealt hand only appears before the player has arranged anything; // afterwards the board shows the arrangement or the waiting message. @@ -266,14 +266,14 @@ func (t *paiGowTable) statusLocked(name string, showFelt bool) []Event { s.PreviewLowNums = pgCardNums(split.low, p.cards) } s.Result = p.result - if p.ready && t.phase == "setting" && p.showdown == nil { + if p.ready && t.phase == phaseSetting && p.showdown == nil { s.Result = "Hand set — waiting for the other players." } - if t.phase == "settling" && !p.paid && p.main.Total > 0 { + if t.phase == phaseSettling && !p.paid && p.main.Total > 0 { s.Result += "; PAYMENT PENDING (not yet credited)" } } - if t.phase == "settling" || t.phase == "waiting" || t.phase == "setting" && t.Config.PaiGow.Variant == PaiGowFaceUp { + if t.phase == phaseSettling || t.phase == phaseWaiting || t.phase == phaseSetting && t.Config.PaiGow.Variant == PaiGowFaceUp { s.DealerHigh = pgViews(t.dealerSplit.high) s.DealerLow = pgViews(t.dealerSplit.low) if len(t.dealerSplit.high) > 0 { @@ -323,7 +323,7 @@ func (t *paiGowTable) leave(name string) []Event { return nil } // Cards in flight must finish; before the deal the wagers are returned. - if t.phase == "setting" || t.phase == "settling" { + if t.phase == phaseSetting || t.phase == phaseSettling { return []Event{t.private(name, "You're in the middle of a Pai Gow round!")} } if p.wallet != nil && (p.main.Total > 0 || p.fortune.Total > 0 || p.pushace.Total > 0) { @@ -353,7 +353,7 @@ func (t *paiGowTable) bet(name, spot string, amount int, w Wallet) []Event { return nil } fail := func(s string) []Event { return []Event{t.private(name, s)} } - if t.phase != "waiting" && t.phase != "betting" { + if t.phase != phaseWaiting && t.phase != phaseBetting { return fail("Betting is closed.") } if spot == "" { @@ -406,8 +406,8 @@ func (t *paiGowTable) bet(name, spot string, amount int, w Wallet) []Event { } else { p.fortune = stake } - if spot == "main" && t.phase == "waiting" { - t.phase = "betting" + if spot == "main" && t.phase == phaseWaiting { + t.phase = phaseBetting t.ticks = t.Config.BettingTicks t.dealer = nil t.dealerSplit = pgSplit{} @@ -475,7 +475,7 @@ func (t *paiGowTable) dealLocked() []Event { t.dealer = packets[6] t.dealerSplit = pgHouseRules(t.dealer, t.a2345High()) t.tie = [2]card{deck.draw(), deck.draw()} - t.phase = "setting" + t.phase = phaseSetting t.ticks = t.Config.PaiGow.ActionTicks // Face Up ace-high settles immediately: there is no decision to make. // Still assign every player's seven cards for independent Fortune/Envy. @@ -511,11 +511,11 @@ func (t *paiGowTable) action(name, action string) []Event { if action == "paytable" || action == "status" { return t.statusLocked(name, false) } - if t.phase == "settling" && action == "settle" { + if t.phase == phaseSettling && action == "settle" { return t.payLocked() } if action == "leave_after" { - if t.phase == "setting" || t.phase == "settling" { + if t.phase == phaseSetting || t.phase == phaseSettling { p.leaveAfter = true return []Event{t.private(name, "You'll leave after this round. Your current hand still needs your actions.")} } @@ -525,7 +525,7 @@ func (t *paiGowTable) action(name, action string) []Event { return []Event{t.private(name, "Pai Gow has no sit-out; use stop to leave after the round.")} } if action == "autoset" || strings.HasPrefix(action, "autoset ") { - if t.phase != "waiting" && t.phase != "betting" && t.phase != "setting" { + if t.phase != phaseWaiting && t.phase != phaseBetting && t.phase != phaseSetting { return []Event{t.private(name, "Autoset can only change before the showdown.")} } rest := strings.TrimSpace(strings.TrimPrefix(action, "autoset")) @@ -548,13 +548,13 @@ func (t *paiGowTable) action(name, action string) []Event { if strings.HasPrefix(action, "unbet") { return t.unbetLocked(p) } - if action == "deal" && (t.phase == "waiting" || t.phase == "betting") && p.main.Total > 0 { + if action == "deal" && (t.phase == phaseWaiting || t.phase == phaseBetting) && p.main.Total > 0 { if t.allBetLocked() { return t.dealLocked() } return []Event{t.private(name, "Waiting for the other bettors; the window closes on the timer.")} } - if t.phase != "setting" || p.main.Total == 0 { + if t.phase != phaseSetting || p.main.Total == 0 { return []Event{t.private(name, "Place your bets, then deal; set cards only after they are dealt.")} } if p.ready { @@ -600,7 +600,7 @@ func (t *paiGowTable) action(name, action string) []Event { return t.statusLocked(name, false) } func (t *paiGowTable) prepareSettlementLocked() []Event { - t.phase = "settling" + t.phase = phaseSettling for _, p := range t.players { if p.main.Total == 0 { continue @@ -720,7 +720,7 @@ func (t *paiGowTable) payLocked() []Event { } return withSettlementReceipts(events, receipts) } - t.phase = "waiting" + t.phase = phaseWaiting kept := t.players[:0] for _, p := range t.players { p.main = Wager{} @@ -741,9 +741,9 @@ func (t *paiGowTable) tick() []Event { t.mu.Lock() defer t.mu.Unlock() switch t.phase { - case "settling": + case phaseSettling: return t.payLocked() - case "betting": + case phaseBetting: if t.allBetLocked() { return t.dealLocked() } @@ -759,7 +759,7 @@ func (t *paiGowTable) tick() []Event { return nil } return t.finishBettingLocked() - case "setting": + case phaseSetting: // A lone player has no time limit; wait for them to set. if t.activePlayersLocked() <= 1 { return nil @@ -791,7 +791,7 @@ func (t *paiGowTable) markDisconnected(name string) []Event { } // Before the deal nothing is revealed, so a disconnect returns the // wagers and frees the seat; the remaining bettors may then be dealt. - if t.phase == "waiting" || t.phase == "betting" { + if t.phase == phaseWaiting || t.phase == phaseBetting { if p.wallet != nil { // Nothing has been dealt, so the stake is refunded; a failed refund // leaves it pending for startup recovery, but the seat is still freed. @@ -803,7 +803,7 @@ func (t *paiGowTable) markDisconnected(name string) []Event { } p.disconnected = true p.ready = true - if t.phase == "setting" { + if t.phase == phaseSetting { if p.split.high == nil { p.split = pgHouseRules(p.cards, t.a2345High()) } @@ -818,7 +818,7 @@ func (t *paiGowTable) markDisconnected(name string) []Event { // or disconnects: the cards deal when every remaining active player has bet, // and a table with no main wager returns to waiting. func (t *paiGowTable) afterSeatChangeLocked() []Event { - if t.phase != "betting" { + if t.phase != phaseBetting { return nil } if t.allBetLocked() { @@ -879,7 +879,7 @@ func (t *paiGowTable) refundSideBetsLocked(p *pgPlayer) []Event { // resetTableLocked clears a round that closed without a main wager and // returns the table to waiting. Callers have already refunded any side bets. func (t *paiGowTable) resetTableLocked() { - t.phase = "waiting" + t.phase = phaseWaiting t.ticks = 0 t.dealer = nil t.dealerSplit = pgSplit{} @@ -920,7 +920,7 @@ func (t *paiGowTable) seatedPlayersLocked() int { // PayCredits with the original stake handles so the ledger entries resolve in // the same save; wagers are only cleared on success. func (t *paiGowTable) unbetLocked(p *pgPlayer) []Event { - if t.phase != "waiting" && t.phase != "betting" { + if t.phase != phaseWaiting && t.phase != phaseBetting { return []Event{t.private(p.name, "You can only take back bets before the cards are dealt.")} } total, ids := 0, []string{} @@ -943,8 +943,8 @@ func (t *paiGowTable) unbetLocked(p *pgPlayer) []Event { } } p.main, p.fortune, p.pushace = Wager{}, Wager{}, Wager{} - if !t.anyMainLocked() && t.phase == "betting" { - t.phase = "waiting" + if !t.anyMainLocked() && t.phase == phaseBetting { + t.phase = phaseWaiting t.ticks = 0 } return append([]Event{t.private(p.name, fmt.Sprintf("You take back %d credits.", total))}, t.statusLocked(p.name, false)...) diff --git a/internal/game/account_txn.go b/internal/game/account_txn.go index a1a5cf0..77a4219 100644 --- a/internal/game/account_txn.go +++ b/internal/game/account_txn.go @@ -482,8 +482,11 @@ func equalsFold(a, b string) bool { return true } -// recoverJournals rolls back leftover lifecycle journals at startup and -// surfaces unrecoverable ones. +// recoverJournals rolls back leftover lifecycle journals and surfaces +// unrecoverable ones. It scans every journal in the directory, so it must run +// only where no other transaction can be in flight: at startup, or on the game +// owner goroutine (which serializes every transaction). The mid-game caller in +// video_poker_banks.go is on the owner. func (g *Game) recoverJournals() { if err := persistence.Recover(g.journalDir()); err != nil { fmt.Fprintf(os.Stderr, "lifecycle journal recovery failed: %v\n", err) diff --git a/internal/game/act_talk.go b/internal/game/act_talk.go index fa93435..8f47bf5 100644 --- a/internal/game/act_talk.go +++ b/internal/game/act_talk.go @@ -139,6 +139,10 @@ func (g *Game) showTalkNode(sess *net.Session, node behavior.TalkNode) { } } + // A terminal node (no options, no goto) ends the conversation; clear the + // action or it lingers as a stale talk action (status shows "talking", + // isBusy suppresses the prompt, and advanceTalk no-ops every tick). + g.cancelAction(p) sess.SetState(net.StateGame) g.writePrompt(sess) } diff --git a/internal/game/casino_house_edge.go b/internal/game/casino_house_edge.go index de0036b..ff3ce59 100644 --- a/internal/game/casino_house_edge.go +++ b/internal/game/casino_house_edge.go @@ -158,19 +158,9 @@ func (g *Game) showCasinoListTable(sess *net.Session, betHeader string, rows []c } } budget := max(len(headers), width-3*len(headers)-1) - for { - sum, widest := 0, 0 - for i, w := range widths { - sum += w - if w > widths[widest] { - widest = i - } - } - if sum <= budget || widths[widest] <= 1 { - break - } - widths[widest]-- - } + // Size the pre-wrapped cells with the same shrink algorithm Table.Render + // uses, instead of a second copy that can disagree on which column yields. + ui.FitWidths(widths, budget) table := ui.Table{} // Headers wrap too, so narrow terminals never lose a column label. all := append([][]string{headers}, cells...) diff --git a/internal/game/correct_audit_fixes_test.go b/internal/game/correct_audit_fixes_test.go index c32b1e5..03facc0 100644 --- a/internal/game/correct_audit_fixes_test.go +++ b/internal/game/correct_audit_fixes_test.go @@ -166,6 +166,100 @@ func TestGatherYieldsDrop(t *testing.T) { }) } +// End-to-end burn: the missing coverage the audit called out. Start a fire +// with a tinderbox and tick until it catches. +func TestBurnCreatesFire(t *testing.T) { + h := newHarness(t) + h.seedAccount("alice") + p := h.seedCharacter("alice", "alice") + writeSeedFile(t, filepath.Join(h.dir, "items", "logs.yaml"), + []byte("id: logs\nname: logs\nburn_ticks: 100\nfire_level: 1\nfire_xp: 10\n")) + writeSeedFile(t, filepath.Join(h.dir, "items", "tinderbox.yaml"), + []byte("id: tinderbox\nname: tinderbox\ntool:\n type: fire\n speed: 1\n")) + sess := &net.Session{Conn: newChanConn(), Player: p} + sess.SetState(net.StateGame) + h.g.RunOnOwner(func() { + h.g.ItemStore.Reload(h.dir) + p.SetInvSlot(0, &player.InventorySlot{ItemID: "logs", Quantity: 5}) + p.SetInvSlot(1, &player.InventorySlot{ItemID: "tinderbox", Quantity: 1}) + h.g.startBurn(sess, p, "logs", false) + if p.Action == nil || p.Action.Type != behavior.TypeBurn { + t.Fatalf("burn action did not start: %s", sess.Conn.(*chanConn).Bytes()) + } + // Each attempt is two calls (phase 1 then the roll); 200 calls is ~100 + // attempts at >=5% success, so failure is effectively impossible. + for i := 0; i < 200 && p.Action != nil; i++ { + h.g.advanceBurn(sess, p) + } + if p.Action != nil { + t.Fatal("burn never completed") + } + if !h.g.hasFireObject(1) { + t.Fatalf("no fire created (output: %s)", sess.Conn.(*chanConn).Bytes()) + } + }) +} + +// Regression: a terminal talk node (messages, no options, no goto) must clear +// the talk action, or it lingers as a stale action that suppresses the prompt +// and no-ops every tick. +func TestTerminalTalkNodeClearsAction(t *testing.T) { + h := newHarness(t) + h.seedAccount("alice") + p := h.seedCharacter("alice", "alice") + writeSeedFile(t, filepath.Join(h.dir, "objects", "statue.yaml"), + []byte("id: statue\nname: statue\ntalk:\n nodes:\n start:\n messages: [The statue says nothing more.]\n")) + sess := &net.Session{Conn: newChanConn(), Player: p} + sess.SetState(net.StateGame) + h.g.RunOnOwner(func() { + h.g.ObjectStore.Reload(h.dir) + obj, err := h.g.ObjectStore.Load("statue") + if err != nil { + t.Fatal(err) + } + h.g.startTalk(sess, p, obj) + if p.Action != nil { + t.Fatalf("terminal talk node left a stale action: %+v", p.Action) + } + if sess.State() != net.StateGame { + t.Fatalf("state = %v, want StateGame", sess.State()) + } + }) +} + +// End-to-end object steal: the missing coverage the audit called out. +func TestObjectStealYieldsLoot(t *testing.T) { + h := newHarness(t) + h.seedAccount("alice") + p := h.seedCharacter("alice", "alice") + writeSeedFile(t, filepath.Join(h.dir, "objects", "market_stall.yaml"), + []byte("id: market_stall\nname: market stall\nsteal:\n level: 1\n xp: 10\n speed: 1\n drops:\n - item_id: stolen_coins\n weight: 100\n")) + writeSeedFile(t, filepath.Join(h.dir, "items", "stolen_coins.yaml"), + []byte("id: stolen_coins\nname: stolen coins\nstackable: true\n")) + sess := &net.Session{Conn: newChanConn(), Player: p} + sess.SetState(net.StateGame) + h.g.RunOnOwner(func() { + h.g.ItemStore.Reload(h.dir) + h.g.ObjectStore.Reload(h.dir) + h.g.World.EnsureObjectStates(1, []string{"market_stall"}) + obj, err := h.g.ObjectStore.Load("market_stall") + if err != nil { + t.Fatal(err) + } + h.g.startSteal(sess, p, "object", nil, obj) + if p.Action == nil || p.Action.Type != behavior.TypeSteal { + t.Fatalf("steal action did not start: %s", sess.Conn.(*chanConn).Bytes()) + } + // ~50% success per attempt; 200 attempts makes failure negligible. + for i := 0; i < 200 && p.CountItem("stolen_coins") == 0; i++ { + h.g.advanceSteal(sess, p) + } + if got := p.CountItem("stolen_coins"); got == 0 { + t.Fatalf("steal yielded no loot (output: %s)", sess.Conn.(*chanConn).Bytes()) + } + }) +} + func TestPlayerDataFileNameCanonicalizesPrefixes(t *testing.T) { for _, in := range []string{ "players/characters/alice.yaml", diff --git a/internal/game/exit_migrate.go b/internal/game/exit_migrate.go index f6df05d..4edab95 100644 --- a/internal/game/exit_migrate.go +++ b/internal/game/exit_migrate.go @@ -6,6 +6,7 @@ import ( "strings" "thehouseoficarus/internal/player" + "thehouseoficarus/internal/world" ) // rewritePlayerRoomIDs remaps room-ID-embedded state in a single player per @@ -21,10 +22,10 @@ func rewritePlayerRoomIDs(p *player.Player, idMap map[int]int) bool { return false } changed := false - if remapScalar(idMap, &p.RoomID) { + if world.RemapInt(idMap, &p.RoomID) { changed = true } - if p.PendingSequence != nil && p.PendingSequence.RoomID != 0 && remapScalar(idMap, &p.PendingSequence.RoomID) { + if p.PendingSequence != nil && p.PendingSequence.RoomID != 0 && world.RemapInt(idMap, &p.PendingSequence.RoomID) { changed = true } if len(p.Flags) > 0 && rewriteDiscoveredExitFlags(p.Flags, idMap) { @@ -41,14 +42,6 @@ func rewritePlayerRoomIDs(p *player.Player, idMap map[int]int) bool { return changed } -func remapScalar(idMap map[int]int, n *int) bool { - if newID, ok := idMap[*n]; ok && newID != *n { - *n = newID - return true - } - return false -} - // rewriteDiscoveredExitFlags rewrites hidden_exit_<roomID>_<dir> flag keys, // remapping the embedded roomID per idMap. Flag values are preserved. func rewriteDiscoveredExitFlags(flags map[string]any, idMap map[int]int) bool { diff --git a/internal/game/harness_test.go b/internal/game/harness_test.go index 3bdce0f..c5377b4 100644 --- a/internal/game/harness_test.go +++ b/internal/game/harness_test.go @@ -131,6 +131,39 @@ func newHarness(t *testing.T) *harness { return h } +// newHarnessWithTicks is newHarness with the production master tick +// subscription registered, so tick-driven systems (disconnect countdown, +// action advance, casino tick) run exactly as they do in production. Most +// harness tests drive the systems directly and do not need this. +func newHarnessWithTicks(t *testing.T) *harness { + t.Helper() + h := newHarness(t) + h.g.SubscribeTicks(1, func() bool { + h.g.RunTickSystems() + return true + }) + return h +} + +// Regression: the harness must be able to run the real master tick sequence, +// which is the only way the disconnect countdown (Hub.Remove -> DisconnectTick +// -> onRemove) is exercised end-to-end. +func TestHarnessWithTicksCompletesDisconnect(t *testing.T) { + h := newHarnessWithTicks(t) + h.seedAccount("alice") + h.seedCharacter("alice", "alice") + sess, _ := h.connect("alice") + h.hub.Remove(sess) + h.barrier() + deadline := time.Now().Add(2 * time.Second) + for time.Now().Before(deadline) && len(h.hub.AllSessions()) > 0 { + time.Sleep(5 * time.Millisecond) + } + if got := len(h.hub.AllSessions()); got != 0 { + t.Fatalf("session not hard-removed after the countdown: %d", got) + } +} + // seedWorld writes the minimal on-disk content a Game needs: three rooms. func seedWorld(t *testing.T, dir string) { t.Helper() diff --git a/internal/ui/table.go b/internal/ui/table.go index 8a056fb..437c445 100644 --- a/internal/ui/table.go +++ b/internal/ui/table.go @@ -38,6 +38,13 @@ type Table struct { Rows [][]string } +// FitWidths shrinks colWidths in place so their sum fits budget, using the +// same fair-share algorithm Table.Render uses. Exported so callers that +// pre-wrap cells can size them consistently instead of reimplementing it. +func FitWidths(colWidths []int, budget int) { + rebalanceWidths(colWidths, budget) +} + // rebalanceWidths shrinks column widths in place so their sum fits budget. // Rather than crushing only the last column, every column wider than the // fair share (budget/nCols) is capped first, then columns are trimmed one diff --git a/internal/world/room_migrate.go b/internal/world/room_migrate.go index 42cd1ae..12859f2 100644 --- a/internal/world/room_migrate.go +++ b/internal/world/room_migrate.go @@ -25,7 +25,7 @@ func (r *Room) RewriteRoomIDs(idMap map[int]int) bool { // Exit targets + their on_traverse steps + exit/desc conditions. for dir, exit := range r.Exits { var exitChanged bool - if remapInt(idMap, &exit.Room) { + if RemapInt(idMap, &exit.Room) { exitChanged = true } if remapTriggers(idMap, exit.OnTraverse) { @@ -166,7 +166,7 @@ func remapStepList(idMap map[int]int, steps []behavior.Step) bool { changed := false for i := range steps { s := &steps[i] - if remapInt(idMap, &s.Teleport) { + if RemapInt(idMap, &s.Teleport) { changed = true } if s.SpawnMob != nil && remapIntSlice(idMap, s.SpawnMob.DespawnRooms) { @@ -186,7 +186,7 @@ func remapCondition(idMap map[int]int, c *behavior.Condition) bool { return false } changed := false - if c.Room != 0 && remapInt(idMap, &c.Room) { + if c.Room != 0 && RemapInt(idMap, &c.Room) { changed = true } for i := range c.AllOf { @@ -202,7 +202,10 @@ func remapCondition(idMap map[int]int, c *behavior.Condition) bool { return changed } -func remapInt(idMap map[int]int, n *int) bool { +// RemapInt rewrites *n through idMap (oldID -> newID) and reports whether it +// changed. Shared by the room-file and player-state migrations so they cannot +// disagree on the remap semantics. +func RemapInt(idMap map[int]int, n *int) bool { if newID, ok := idMap[*n]; ok && newID != *n { *n = newID return true @@ -213,7 +216,7 @@ func remapInt(idMap map[int]int, n *int) bool { func remapIntSlice(idMap map[int]int, slice []int) bool { changed := false for i := range slice { - if remapInt(idMap, &slice[i]) { + if RemapInt(idMap, &slice[i]) { changed = true } } |
