aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--FIXES.md26
-rw-r--r--internal/admin/correct_audit_fixes_test.go33
-rw-r--r--internal/admin/undo.go99
-rw-r--r--internal/casino/core.go3
-rw-r--r--internal/casino/pai_gow.go66
-rw-r--r--internal/game/account_txn.go7
-rw-r--r--internal/game/act_talk.go4
-rw-r--r--internal/game/casino_house_edge.go16
-rw-r--r--internal/game/correct_audit_fixes_test.go94
-rw-r--r--internal/game/exit_migrate.go13
-rw-r--r--internal/game/harness_test.go33
-rw-r--r--internal/ui/table.go7
-rw-r--r--internal/world/room_migrate.go13
13 files changed, 311 insertions, 103 deletions
diff --git a/FIXES.md b/FIXES.md
index 45e0fab..f89cce9 100644
--- a/FIXES.md
+++ b/FIXES.md
@@ -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
}
}