From 7eab582d8a8ee2af1a6baae7baf0696c7005de2e Mon Sep 17 00:00:00 2001 From: historia <[not public]> Date: Fri, 18 Sep 2026 15:16:44 -0400 Subject: fix: harden triggers, economy, casino, world data, and admin/net paths --- internal/admin/api_files.go | 16 ++ internal/admin/api_map.go | 10 ++ internal/admin/server.go | 50 +++++- internal/admin/undo.go | 8 +- internal/casino/baccarat_test.go | 4 +- internal/casino/blackjack.go | 35 +++- internal/casino/cards.go | 8 + internal/casino/config_validate.go | 31 ++++ internal/casino/credits.go | 6 + internal/casino/hardening_fixes_batch2_test.go | 108 ++++++++++++ internal/casino/slots.go | 3 - internal/casino/types.go | 5 + internal/game/act_effects.go | 86 +++++++--- internal/game/act_farm.go | 3 + internal/game/act_gather.go | 37 +---- internal/game/act_steal.go | 19 +-- internal/game/act_talk.go | 47 ++++-- internal/game/casino.go | 2 +- internal/game/cmd_bank.go | 7 +- internal/game/cmd_consume.go | 4 +- internal/game/cmd_drop.go | 61 ++++--- internal/game/cmd_estate_directory.go | 10 +- internal/game/cmd_quit.go | 11 +- internal/game/cmd_swapid.go | 13 +- internal/game/cmd_trigger_utility.go | 62 ++++--- internal/game/core_validate.go | 33 ++-- internal/game/economy.go | 68 ++++++++ internal/game/exit_migrate.go | 7 +- internal/game/game.go | 18 ++ internal/game/hardening_fixes_batch2_test.go | 203 +++++++++++++++++++++++ internal/game/production_menu.go | 3 + internal/game/sys_combat.go | 21 ++- internal/net/conn.go | 15 +- internal/player/hardening_removeitem_test.go | 23 +++ internal/player/player.go | 6 + internal/validate/checks.go | 106 ++++++++++++ internal/validate/hardening_fixes_batch2_test.go | 89 ++++++++++ internal/validate/validate.go | 10 +- 38 files changed, 1071 insertions(+), 177 deletions(-) create mode 100644 internal/casino/hardening_fixes_batch2_test.go create mode 100644 internal/game/hardening_fixes_batch2_test.go create mode 100644 internal/player/hardening_removeitem_test.go create mode 100644 internal/validate/hardening_fixes_batch2_test.go (limited to 'internal') diff --git a/internal/admin/api_files.go b/internal/admin/api_files.go index 96b5e81..2aa0e5f 100644 --- a/internal/admin/api_files.go +++ b/internal/admin/api_files.go @@ -108,6 +108,22 @@ func (s *AdminServer) handleFileRename(w http.ResponseWriter, r *http.Request) { return } + // A raw rename changes the ID-bearing filename. Rebuild the room index + // and drop the old room's cached state (mirroring the DELETE path), and + // refresh item/mob/drop caches, or the old ID resolves to a deleted file + // and the new one resolves to nothing until a restart. + s.evictDeletedRoomFile(relPath) + s.invalidateEntityCacheForPath(newAbs) + // Hazards and courses have their own runtime caches that a raw rename + // must refresh (the undo path does the same). Techs and triggers keep the + // documented in-game `reload` requirement. + if pathUnderDir(s.dataDir, "hazards", newAbs) { + s.world.ClearHazardCache() + } + if pathUnderDir(s.dataDir, "courses", newAbs) { + s.reloadCoursesIfWired() + } + newRelPath := filepath.Join(filepath.Dir(relPath), newName) if filepath.Dir(relPath) == "." { newRelPath = newName diff --git a/internal/admin/api_map.go b/internal/admin/api_map.go index 5eac385..d0372df 100644 --- a/internal/admin/api_map.go +++ b/internal/admin/api_map.go @@ -24,6 +24,16 @@ func (s *AdminServer) handleMap(w http.ResponseWriter, r *http.Request) { } dir := r.URL.Query().Get("dir") + if dir != "" { + // Route the map's directory through the same guard every other room + // API uses, so ?dir=../../.. cannot read/walk outside data/rooms. + clean, err := s.resolveRoomSubdir(dir) + if err != nil { + failJSON(w, err.Error()) + return + } + dir = clean + } seed := s.cfg.StartingRoom if dir != "" { if low, ok := findLowestLoadableRoom(s, dir); ok { diff --git a/internal/admin/server.go b/internal/admin/server.go index 4a54c78..4ea22d8 100644 --- a/internal/admin/server.go +++ b/internal/admin/server.go @@ -365,6 +365,20 @@ func (s *AdminServer) authMiddleware(next http.Handler) http.Handler { }) } +// UndoStack returns the server's undo history. Two admin transports over the +// same data directory must share one stack: each server otherwise loads and +// rewrites the same history file independently, so records made through one +// are silently dropped by the next write from the other. +func (s *AdminServer) UndoStack() *UndoStack { return s.undoStack } + +// SetUndoStack replaces the server's undo history with a shared stack. Call +// before serving requests. +func (s *AdminServer) SetUndoStack(us *UndoStack) { + if us != nil { + s.undoStack = us + } +} + func (s *AdminServer) checkAuth(r *http.Request) bool { cookie, err := r.Cookie("admin_session") if err != nil { @@ -389,8 +403,16 @@ func (s *AdminServer) checkAuth(r *http.Request) bool { return false } + // Bind the signature to the current password hash as well as the issue + // time, so changing an admin password invalidates every previously issued + // cookie instead of leaving it valid for the full TTL. The admin check + // above already loaded the account; reload it here for the hash. + acc, err := s.accountStore.LoadAccount(account) + if err != nil || !acc.Admin { + return false + } mac := hmac.New(sha256.New, s.cookieSecret) - mac.Write([]byte(account + ":" + parts[1])) + mac.Write([]byte(account + ":" + parts[1] + ":" + acc.PasswordHash)) expected := hex.EncodeToString(mac.Sum(nil)) return hmac.Equal([]byte(sig), []byte(expected)) } @@ -552,9 +574,15 @@ func clientIP(remoteAddr string) string { } func (s *AdminServer) setAuthCookie(w http.ResponseWriter, account string) { + // Include the current password hash in the MAC so a password change + // revokes outstanding cookies (see checkAuth). + hash := "" + if acc, err := s.accountStore.LoadAccount(account); err == nil { + hash = acc.PasswordHash + } issued := strconv.FormatInt(time.Now().Unix(), 10) mac := hmac.New(sha256.New, s.cookieSecret) - mac.Write([]byte(account + ":" + issued)) + mac.Write([]byte(account + ":" + issued + ":" + hash)) sig := hex.EncodeToString(mac.Sum(nil)) http.SetCookie(w, &http.Cookie{ Name: "admin_session", @@ -607,6 +635,16 @@ func (s *AdminServer) handleLogin(w http.ResponseWriter, r *http.Request) { account := strings.TrimSpace(r.FormValue("account")) password := r.FormValue("password") + // Validate before the name is joined into a filesystem path: LoadAccount + // does no validation, so an unvalidated name is an arbitrary-*.yaml read + // primitive ("account=../../rooms/1"). Every other account-name entry + // point validates first. + if err := player.ValidName(account); err != nil { + limiter.fail(ip) + s.renderPage(w, r, "login.html", map[string]any{"Error": "Invalid account name or password."}) + return + } + acc, err := s.accountStore.LoadAccount(account) if err != nil || !player.CheckPassword(password, acc.PasswordHash) { limiter.fail(ip) @@ -1004,6 +1042,14 @@ func (s *AdminServer) invalidateEntityCacheForPath(path string) { } } +// pathUnderDir reports whether path lies inside dataDir/subdir (and is not the +// directory itself). Used by the raw-rename path to refresh caches that the +// shared invalidateEntityCacheForPath does not own. +func pathUnderDir(dataDir, subdir, path string) bool { + rel, err := filepath.Rel(filepath.Join(dataDir, subdir), path) + return err == nil && rel != "." && !strings.HasPrefix(rel, "..") +} + func changeFiles(change *ChangeDesc) []string { files := []string{change.FilePath} if change.NewFilePath != "" { diff --git a/internal/admin/undo.go b/internal/admin/undo.go index 7bfbe09..24ce9c1 100644 --- a/internal/admin/undo.go +++ b/internal/admin/undo.go @@ -9,6 +9,8 @@ import ( "strings" "sync" "time" + + "thehouseoficarus/internal/persistence" ) type ExtraFile struct { @@ -537,12 +539,12 @@ func (us *UndoStack) save() { log.Printf("undo: failed to marshal history: %v", err) return } - if err := os.WriteFile(us.filePath, b, 0600); err != nil { + if err := persistence.WriteFile(us.filePath, b, 0600); err != nil { log.Printf("undo: failed to write history file: %v", err) return } - // os.WriteFile does not change an existing file's mode: tighten a history - // file that a previous version created world-readable. + // persistence.WriteFile preserves an existing file's mode: tighten a + // history file that a previous version created world-readable. if err := os.Chmod(us.filePath, 0600); err != nil { log.Printf("undo: failed to secure history file: %v", err) } diff --git a/internal/casino/baccarat_test.go b/internal/casino/baccarat_test.go index 522f875..c5565b1 100644 --- a/internal/casino/baccarat_test.go +++ b/internal/casino/baccarat_test.go @@ -108,8 +108,8 @@ func TestBaccaratSettlement(t *testing.T) { if result, payout := settleBaccaratBet(baccaratSpotBanker, 100, 4, 6, rules); result != "win" || payout != 195 { t.Fatalf("banker win: result=%s payout=%d, want win/195 (5%% commission)", result, payout) } - if result, payout := settleBaccaratBet(baccaratSpotBanker, 1, 4, 6, rules); result != "win" || payout != 1 { - t.Fatalf("banker win on bet 1: result=%s payout=%d, want win/1 (commission rounds up)", result, payout) + if result, payout := settleBaccaratBet(baccaratSpotBanker, 1, 4, 6, rules); result != "win" || payout != 2 { + t.Fatalf("banker win on bet 1: result=%s payout=%d, want win/2 (a winning stake never pays zero)", result, payout) } if result, payout := settleBaccaratBet(baccaratSpotTie, 100, 7, 7, rules); result != "win" || payout != 900 { t.Fatalf("tie win: result=%s payout=%d, want win/900 (8:1)", result, payout) diff --git a/internal/casino/blackjack.go b/internal/casino/blackjack.go index 7b30699..16a480c 100644 --- a/internal/casino/blackjack.go +++ b/internal/casino/blackjack.go @@ -75,8 +75,14 @@ type blackjackTable struct { ceiling countdown activePlayer int activeHand int - dealer blackjackHand - shoe *shoe + // turnName/turnHand identify the current turn so the absolute turn + // ceiling is restarted only when the turn actually changes. A third party + // joining or leaving mid-round re-prompts the same turn and must not + // extend a hung bettor's deadline. + turnName string + turnHand int + dealer blackjackHand + shoe *shoe } // loneTurnGrace multiplies the action window into an absolute per-turn @@ -94,7 +100,7 @@ func newBlackjackTable(roomID int, cfg TableConfig, rng *rand.Rand) *blackjackTa return &blackjackTable{ tableBase: tableBase{RoomID: roomID, Config: cfg}, Rules: rules, players: newSeatSet[*blackjackPlayer](), phase: blackjackWaiting, - shoe: newShoe(rules.Decks, rules.ShuffleAt, rng), + shoe: newShoe(rules.Decks, rules.ShuffleAt, rng), turnHand: -1, } } @@ -465,6 +471,7 @@ func (t *blackjackTable) beginPlay() { t.phase = blackjackPlaying t.timer.start(t.Rules.ActionTicks) t.activePlayer, t.activeHand = 0, 0 + t.turnName, t.turnHand = "", -1 } // roundCardCeiling is the heuristic number of cards this table's round is @@ -475,19 +482,29 @@ func (t *blackjackTable) beginPlay() { // guaranteed to hold it and draw() may still rebuild an exhausted shoe // mid-round (a deliberate shuffle-through). func (t *blackjackTable) roundCardCeiling() int { - return blackjackRoundCards(t.Config.MaxPlayers, t.Rules.MaxSplitHands) + // Size the boundary reshuffle from the seats actually in the round, not + // the configured maximum: using MaxPlayers made ensureForRound dominate + // the cut card, so shuffle_at never fired on a table whose maximum seats + // implied more cards than the cut card allowed. + return blackjackRoundCards(t.bettorCount(), t.Rules.MaxSplitHands) } func (t *blackjackTable) prompt() []Event { // Every freshly-prompted turn gets a full action window; rejected inputs - // and timeouts can never extend the deadline. The turn ceiling restarts - // with the window for the same reason. + // and timeouts can never extend the deadline. t.timer.start(t.Rules.ActionTicks) - t.ceiling.start(t.Rules.ActionTicks * loneTurnGrace) name := t.activeName() if name == "" { return t.settle() } + // The absolute turn ceiling belongs to the turn, not the prompt: restart + // it only when the active (player, hand) changes. A seat event that + // re-prompts the same turn must not reset a hung bettor's deadline. + if name != t.turnName || t.activeHand != t.turnHand { + t.ceiling.start(t.Rules.ActionTicks * loneTurnGrace) + t.turnName = name + t.turnHand = t.activeHand + } p := t.players.get(name) h := p.hands[t.activeHand] ranks := make([]string, len(h.cards)) @@ -710,6 +727,7 @@ func (t *blackjackTable) resetRound() { t.window.close() t.timer.stop() t.ceiling.stop() + t.turnName, t.turnHand = "", -1 for _, p := range t.players.ordered() { p.HasBet, p.Wager, p.hands, p.insurance, p.insuranceStakeID, p.declinedInsurance = false, Wager{}, nil, 0, "", false } @@ -837,7 +855,8 @@ func (t *blackjackTable) markDisconnectedLocked(name string) []Event { } switch { case index < t.activePlayer: - // The same player stays active, one seat earlier. + // The same player stays active, one seat earlier. prompt() sees the + // unchanged turn and keeps the existing ceiling. t.activePlayer-- case index == t.activePlayer: // The next bettor slides into the active seat. diff --git a/internal/casino/cards.go b/internal/casino/cards.go index 27da8aa..2d53b4a 100644 --- a/internal/casino/cards.go +++ b/internal/casino/cards.go @@ -73,6 +73,14 @@ func (s *shoe) remaining() int { return len(s.cards) } func (s *shoe) draw() card { if len(s.cards) == 0 { + if s.decks <= 0 { + // A synthetic, explicitly-scripted shoe (tests) has no decks to + // rebuild from and no cards left; return a visibly-invalid sentinel + // card rather than panicking. Production shoes always have Decks > + // 0 (Normalize), so this only surfaces a test fixture that drew + // past its script. + return card{rank: "?", suit: "?"} + } // Shuffle-through: if the shoe runs out mid-round it is rebuilt from // all decks, so a card already dealt this round can reappear. This // matches real blackjack, where an exhausted shoe is shuffled through diff --git a/internal/casino/config_validate.go b/internal/casino/config_validate.go index ee8e8c9..38d426d 100644 --- a/internal/casino/config_validate.go +++ b/internal/casino/config_validate.go @@ -141,6 +141,37 @@ func (p SlotProfile) Validate() error { return fmt.Errorf("unsupported slot geometry %dx%d: the runtime evaluator supports %dx%d only", profile.Rows, profile.Reels, slotGridRows, slotGridReels) } + // A named trigger must exist, or the feature silently never fires while + // the RTP model also computes a zero trigger probability and reports green. + if profile.FreeSpinTrigger != "" { + found := false + positiveWeight := false + for _, symbol := range profile.Symbols { + if symbol.ID == profile.FreeSpinTrigger { + found = true + positiveWeight = symbol.Weight > 0 + break + } + } + if !found { + return fmt.Errorf("free_spin_trigger %q names no symbol in symbols", profile.FreeSpinTrigger) + } + if !positiveWeight { + return fmt.Errorf("free_spin_trigger %q names a symbol with no reel weight, so the feature can never trigger", profile.FreeSpinTrigger) + } + } + // A paytable must not have a gap: evaluateSlot pays only the longest run, + // so Payout{3: 0.5} with no Payout[4] makes every 4-run pay zero — less + // than the 3-run — while the RTP model agrees and nothing else flags it. + // A symbol whose shorter runs also pay zero (e.g. only a 5-run pays) is a + // legitimate shape and is not flagged. + for _, symbol := range profile.Symbols { + for length := 4; length <= 5; length++ { + if symbol.Payout[length] <= 0 && symbol.Payout[length-1] > 0 { + return fmt.Errorf("symbol %q pays for a run of %d but not %d: a longer run would pay zero", symbol.ID, length-1, length) + } + } + } return nil } diff --git a/internal/casino/credits.go b/internal/casino/credits.go index 8b7cb7a..631a4bc 100644 --- a/internal/casino/credits.go +++ b/internal/casino/credits.go @@ -47,6 +47,12 @@ func MulPayout(amount int, multiplier float64) (int, error) { return MaxCredits, fmt.Errorf("payout overflow: %d * %v exceeds %d", amount, multiplier, MaxCredits) } payout := int(math.Floor(float64(amount) * multiplier)) + if payout == 0 && multiplier > 0 { + // A positive stake on a winning outcome must never pay nothing: a + // fractional multiplier (e.g. a 5% baccarat commission) at a 1-credit + // bet floors to zero, turning a win into a total loss. + payout = 1 + } if payout > MaxCredits { return MaxCredits, fmt.Errorf("payout overflow: %d * %v exceeds %d", amount, multiplier, MaxCredits) } diff --git a/internal/casino/hardening_fixes_batch2_test.go b/internal/casino/hardening_fixes_batch2_test.go new file mode 100644 index 0000000..1391143 --- /dev/null +++ b/internal/casino/hardening_fixes_batch2_test.go @@ -0,0 +1,108 @@ +package casino + +import "testing" + +// TestMulPayoutPositiveBetNeverZero: a positive stake on a winning outcome +// must not floor to zero (a 1-credit banker win at 0.95 paid nothing). +func TestMulPayoutPositiveBetNeverZero(t *testing.T) { + if got, err := MulPayout(1, 0.95); err != nil || got != 1 { + t.Fatalf("MulPayout(1, 0.95) = %d, %v; want 1", got, err) + } + if got, err := MulPayout(1, 0.5); err != nil || got != 1 { + t.Fatalf("MulPayout(1, 0.5) = %d, %v; want 1", got, err) + } + // A zero amount is still zero. + if got, err := MulPayout(0, 0.95); err != nil || got != 0 { + t.Fatalf("MulPayout(0, 0.95) = %d, %v; want 0", got, err) + } +} + +// TestCalculateRTPIncludesTriggerPayout: the runtime pays a paying scatter +// like any other symbol, so the RTP model must include it. +func TestCalculateRTPIncludesTriggerPayout(t *testing.T) { + base := DefaultHuffAndPuffProfile() + withPay := DefaultHuffAndPuffProfile() + for i := range withPay.Symbols { + if withPay.Symbols[i].ID == withPay.FreeSpinTrigger { + withPay.Symbols[i].Payout = map[int]float64{3: 1, 4: 2, 5: 5} + } + } + if CalculateRTP(withPay) <= CalculateRTP(base) { + t.Fatalf("paying trigger did not raise RTP: %v <= %v", CalculateRTP(withPay), CalculateRTP(base)) + } +} + +// TestSlotProfileRejectsMissingTriggerSymbol: a named trigger that matches no +// symbol silently disables the feature. +func TestSlotProfileRejectsMissingTriggerSymbol(t *testing.T) { + p := DefaultHuffAndPuffProfile() + p.FreeSpinTrigger = "ghost" + if err := p.Validate(); err == nil { + t.Fatal("accepted a free_spin_trigger that names no symbol") + } +} + +// TestSlotProfileRejectsPaytableGap: a positive payout for a shorter run with +// no payout for a longer run makes the longer run pay zero. +func TestSlotProfileRejectsPaytableGap(t *testing.T) { + p := DefaultHuffAndPuffProfile() + for i := range p.Symbols { + if p.Symbols[i].ID == "straw" { + p.Symbols[i].Payout = map[int]float64{3: 0.5} + } + } + if err := p.Validate(); err == nil { + t.Fatal("accepted a paytable with a gap") + } +} + +// TestSlotProfileRejectsWeightlessTrigger: a trigger symbol with no reel +// weight never appears, so the feature silently never fires. +func TestSlotProfileRejectsWeightlessTrigger(t *testing.T) { + p := DefaultHuffAndPuffProfile() + for i := range p.Symbols { + if p.Symbols[i].ID == p.FreeSpinTrigger { + p.Symbols[i].Weight = 0 + } + } + if err := p.Validate(); err == nil { + t.Fatal("accepted a free_spin_trigger with no reel weight") + } +} + +// TestDisconnectingNonBettorDoesNotResetLoneTurnCeiling: a third party's +// seat event re-prompts the same turn and must not extend a hung lone +// bettor's absolute deadline. +func TestDisconnectingNonBettorDoesNotResetLoneTurnCeiling(t *testing.T) { + table := newBlackjackTable(1, TableConfig{ID: "blackjack", Game: GameBlackjack, MaxPlayers: 3, + Blackjack: BlackjackConfig{ActionTicks: 2}}, newTestRand()) + table.shoe = tableauShoe( + card{rank: "10"}, card{rank: "7"}, // dealer 17 + card{rank: "10"}, card{rank: "6"}, // alice 16 + ) + wallet := &testWallet{credits: 100} + table.join("alice") + table.join("bob") + table.bet("alice", "", 10, wallet) + + // Let the betting window expire so the round deals with alice the only + // bettor and bob seated as a non-bettor. + for i := 0; i < 200 && table.phase != blackjackPlaying; i++ { + table.tick() + } + if table.phase != blackjackPlaying { + t.Fatalf("setup: round did not start (phase=%s)", table.phase) + } + for i := 0; i < 3; i++ { + table.tick() + } + before := table.ceiling.clock + if before == 0 { + t.Fatal("setup: turn ceiling did not advance") + } + + table.markDisconnected("bob") + if table.ceiling.clock != before { + t.Fatalf("a non-bettor's disconnect reset the turn ceiling: %d -> %d", before, table.ceiling.clock) + } +} diff --git a/internal/casino/slots.go b/internal/casino/slots.go index 64caf85..6be5004 100644 --- a/internal/casino/slots.go +++ b/internal/casino/slots.go @@ -182,9 +182,6 @@ func CalculateRTP(profile SlotProfile) float64 { if symbol.ID == p.FreeSpinTrigger { scatterProbability = probability } - if symbol.ID == p.FreeSpinTrigger { - continue - } for length := 3; length <= 5; length++ { ways := math.Pow(float64(p.Rows), float64(length)) nextReelNoMatch := 1.0 diff --git a/internal/casino/types.go b/internal/casino/types.go index 080e60a..71da01c 100644 --- a/internal/casino/types.go +++ b/internal/casino/types.go @@ -162,6 +162,11 @@ func (c TableConfig) Normalize() TableConfig { if c.MaxBet < c.MinBet { c.MaxBet = c.MinBet } + // Normalize the nested game rule blocks too: a new game that reads + // t.Config.Blackjack/Baccarat directly (rather than only t.Rules) would + // otherwise see zero values and build an empty shoe. + c.Blackjack = c.Blackjack.Normalize() + c.Baccarat = c.Baccarat.Normalize() return c } diff --git a/internal/game/act_effects.go b/internal/game/act_effects.go index cae54a5..a107320 100644 --- a/internal/game/act_effects.go +++ b/internal/game/act_effects.go @@ -51,9 +51,9 @@ const ( // 12. despawn_mob (player scope: owner-filtered; global scope: all) // 13. teleport (player scope; re-runs look + on_enter of target) // 14. aps_node (player scope; marks "aps_node_" player flag) -func (g *Game) applyStep(sess *net.Session, p *player.Player, step *behavior.Step, roomID int, sc effectScope, flagValue any) { +func (g *Game) applyStep(sess *net.Session, p *player.Player, step *behavior.Step, roomID int, sc effectScope, flagValue any) bool { if step == nil { - return + return true } var playerName string @@ -61,6 +61,20 @@ func (g *Game) applyStep(sess *net.Session, p *player.Player, step *behavior.Ste playerName = p.Name } + // Player-scope effects are transactional: preflight every cost (a required + // take_item, give_item placement, and negative credits) before applying + // anything, so a trade can never take/destroy an item or charge credits + // without delivering its reward. Returns false when the whole step was + // aborted; callers that show follow-up success text must honour it. + if sc == scopePlayer && p != nil { + if reason := g.stepPreflight(p, step); reason != "" { + if sess != nil { + sess.WriteLine(reason) + } + return false + } + } + // 1. messages — direct to the triggering session. if sc != scopeGlobal { for _, msg := range step.Messages { @@ -103,11 +117,11 @@ func (g *Game) applyStep(sess *net.Session, p *player.Player, step *behavior.Ste if step.DespawnMob != "" { g.despawnTriggerMobs(step.DespawnMob, "") } - return + return true } if p == nil { - return + return true } // 5. set_player_flags (and globals, folded in so the SetAll happens once). @@ -124,16 +138,15 @@ func (g *Game) applyStep(sess *net.Session, p *player.Player, step *behavior.Ste } } - // 7. give_item + // 7. give_item (placement was preflighted above; merges stackables and + // stamps quality so the received stack matches every other placement path). if step.GiveItem != "" { - slot := p.FirstFreeSlot() - if slot == -1 { - if sess != nil { - sess.WriteLine("Your inventory is too full to receive that.") - } - } else { - p.SetInvSlot(slot, &player.InventorySlot{ItemID: step.GiveItem, Quantity: 1}) + def, _ := g.ItemStore.Load(step.GiveItem) + if placeCraftOutput(p, step.GiveItem, def, 1) { g.saveCharacter(p) + } else if sess != nil { + // Defensive: preflight verified placement on the owner goroutine. + sess.WriteLine("Your inventory is too full to receive that.") } } @@ -159,15 +172,12 @@ func (g *Game) applyStep(sess *net.Session, p *player.Player, step *behavior.Ste } } - // 10. credits (signed; negative is gated). + // 10. credits (signed). A negative cost was preflighted for affordability, + // so the debit is unconditional here. if step.Credits != 0 { if step.Credits < 0 { - if p.Credits >= -step.Credits { - p.Credits += step.Credits - g.saveCharacter(p) - } else if sess != nil { - sess.WriteLine(fmt.Sprintf("You don't have enough credits. (Need %d, have %d)", -step.Credits, p.Credits)) - } + p.Credits += step.Credits + g.saveCharacter(p) } else { g.grantCredits(p, step.Credits) g.saveCharacter(p) @@ -197,6 +207,35 @@ func (g *Game) applyStep(sess *net.Session, p *player.Player, step *behavior.Ste g.setPlayerFlag(p, "aps_node_"+strconv.Itoa(p.RoomID), true) g.saveCharacter(p) } + return true +} + +// stepPreflight reports why a player-scope step cannot be applied in full, or +// "" when every cost can be met. It is the transaction gate for trigger/talk +// effects: flags, items, and credits are all-or-nothing, so a step whose +// take_item is missing, whose give_item cannot be placed, or whose negative +// credits are unaffordable must apply nothing at all. +func (g *Game) stepPreflight(p *player.Player, step *behavior.Step) string { + if p == nil { + return "" + } + if step.TakeItem != "" && !p.HasItem(step.TakeItem) { + name := step.TakeItem + if def, err := g.ItemStore.Load(step.TakeItem); err == nil && def.Name != "" { + name = def.Name + } + return fmt.Sprintf("You don't have %s.", name) + } + if step.GiveItem != "" { + def, _ := g.ItemStore.Load(step.GiveItem) + if !craftOutputFits(p, step.GiveItem, def) { + return "Your inventory is too full to receive that." + } + } + if step.Credits < 0 && p.Credits < -step.Credits { + return fmt.Sprintf("You don't have enough credits. (Need %d, have %d)", -step.Credits, p.Credits) + } + return "" } // writePlayerMessage expands the message template (player name, flag value), @@ -253,13 +292,14 @@ func (g *Game) giveTriggerDrop(sess *net.Session, p *player.Player, drop *behavi } // applyTalkStep fires a talk node/option's Action (a single Step) -// synchronously at scopePlayer, scoped to the player's current room. -func (g *Game) applyTalkStep(sess *net.Session, step *behavior.Step) { +// synchronously at scopePlayer, scoped to the player's current room. It +// returns false when the step's preflight failed and no effect was applied. +func (g *Game) applyTalkStep(sess *net.Session, step *behavior.Step) bool { p := sess.Player if p == nil || step == nil { - return + return true } - g.applyStep(sess, p, step, p.RoomID, scopePlayer, nil) + return g.applyStep(sess, p, step, p.RoomID, scopePlayer, nil) } // runTrigger runs the first matching entry in a trigger block. itemMatch diff --git a/internal/game/act_farm.go b/internal/game/act_farm.go index 48fbec7..6725351 100644 --- a/internal/game/act_farm.go +++ b/internal/game/act_farm.go @@ -238,6 +238,9 @@ func (g *Game) advanceHarvest(sess *net.Session, p *player.Player) { return } + if maxYield < minYield { + maxYield = minYield + } yield := int(minYield) + rand.Intn(int(maxYield)-int(minYield)+1) bonus := p.Level(player.Farming) / 20 yield += bonus diff --git a/internal/game/act_gather.go b/internal/game/act_gather.go index a1c87b9..08ffbb1 100644 --- a/internal/game/act_gather.go +++ b/internal/game/act_gather.go @@ -110,10 +110,6 @@ func (g *Game) startGather(sess *net.Session, p *player.Player, obj *object.Obje } } - if p.FirstFreeSlot() == -1 { - sess.WriteLine("Your inventory is too full!") - return - } verb := cfg.Skill switch cfg.Skill { case "woodcutting": @@ -162,11 +158,6 @@ func (g *Game) advanceGather(sess *net.Session, p *player.Player) { shared := d.DepleteTimer if step == 0 { - if p.FirstFreeSlot() == -1 { - sess.WriteLine(g.colorize(sess, "error", "Your inventory is too full!")) - g.cancelAction(p) - return - } if cfg.Bait != "" { if !p.HasItem(cfg.Bait) { baitName := cfg.Bait @@ -216,13 +207,6 @@ func (g *Game) advanceGather(sess *net.Session, p *player.Player) { eligible = filterDropsByTool(eligible, d.ToolType, len(cfg.Tools)) drop := behavior.ResolveDrop(g.DataDir, eligible) if drop != nil { - freeSlot := p.FirstFreeSlot() - if freeSlot == -1 { - sess.WriteLine(g.colorize(sess, "error", "Your inventory is too full!")) - g.cancelAction(p) - return - } - qty := drop.Quantity if qty <= 0 { qty = 1 @@ -234,7 +218,14 @@ func (g *Game) advanceGather(sess *net.Session, p *player.Player) { itemName = itemDef.Name } - p.SetInvSlot(freeSlot, &player.InventorySlot{ItemID: drop.ItemID, Quantity: qty}) + // Stack-aware placement: a stackable drop merges into an existing + // stack, so a full inventory with a matching stack is not a failure + // (matching doGet/shop/bank withdrawal). + if !placeCraftOutput(p, drop.ItemID, itemDef, qty) { + sess.WriteLine(g.colorize(sess, "error", "Your inventory is too full!")) + g.cancelAction(p) + return + } xp := drop.XP if xp > 0 { g.awardSkillXP(sess, p, player.SkillName(cfg.Skill), xp) @@ -254,7 +245,7 @@ func (g *Game) advanceGather(sess *net.Session, p *player.Player) { } sess.WriteLine(msg) - g.checkBirdNest(sess, p, cfg, freeSlot) + g.checkBirdNest(sess, p, cfg, -1) if shared { st := g.World.GetObjStateByKey(instanceKey) @@ -306,11 +297,6 @@ func (g *Game) advanceGather(sess *net.Session, p *player.Player) { return } - if p.FirstFreeSlot() == -1 { - sess.WriteLine(g.colorize(sess, "error", "Your inventory is too full!")) - g.cancelAction(p) - return - } d.Step = 0 p.Action.WaitLeft = engine.ToTicks(wait) return @@ -318,11 +304,6 @@ func (g *Game) advanceGather(sess *net.Session, p *player.Player) { } sess.WriteLine(cfg.FailMessage) - if p.FirstFreeSlot() == -1 { - sess.WriteLine(g.colorize(sess, "error", "Your inventory is too full!")) - g.cancelAction(p) - return - } d.Step = 0 p.Action.WaitLeft = engine.ToTicks(wait) } diff --git a/internal/game/act_steal.go b/internal/game/act_steal.go index 32889ce..5a7f93e 100644 --- a/internal/game/act_steal.go +++ b/internal/game/act_steal.go @@ -42,7 +42,7 @@ var stallGuardTalk = &behavior.TalkConfig{ }, "jail": { Messages: []string{"Off to the detention cell with you!"}, - Action: &behavior.Step{Teleport: 162}, + Action: &behavior.Step{Teleport: 99999162}, Options: []behavior.TalkOption{{Text: "(You are dragged away)"}}, }, "fight": { @@ -254,11 +254,6 @@ func (g *Game) startSteal(sess *net.Session, p *player.Player, targetType string } } - if p.FirstFreeSlot() == -1 { - sess.WriteLine("Your inventory is too full!") - return - } - sess.WriteLine(fmt.Sprintf("You attempt to steal from the %s...", targetName)) g.broadcastAction(sess, "%s glances around the room.", p.Name) @@ -379,12 +374,6 @@ func (g *Game) advanceSteal(sess *net.Session, p *player.Player) { g.saveCharacter(p) - if p.FirstFreeSlot() == -1 { - sess.WriteLine("Your inventory is full.") - g.cancelAction(p) - return - } - p.Action.WaitLeft = engine.ToTicks(stealSpeed) } else { if targetType == "mob" { @@ -429,14 +418,14 @@ func (g *Game) giveStealLoot(sess *net.Session, p *player.Player, drop *behavior name = lootDef.Name } - freeSlot := p.FirstFreeSlot() - if freeSlot == -1 { + // Stack-aware placement: a stackable steal merges into an existing stack, + // so a full inventory with a matching stack is not a failure. + if !placeCraftOutput(p, drop.ItemID, lootDef, qty) { g.World.AddGroundItem(p.RoomID, drop.ItemID, qty) sess.WriteLine(fmt.Sprintf("You steal %s but your inventory is full. It falls to the ground.", g.itemColorize(sess, lootDef, name))) return } - p.SetInvSlot(freeSlot, &player.InventorySlot{ItemID: drop.ItemID, Quantity: qty}) sess.WriteLine(fmt.Sprintf("You steal %s.", g.itemColorize(sess, lootDef, name))) } diff --git a/internal/game/act_talk.go b/internal/game/act_talk.go index ef3c6e5..9ae9959 100644 --- a/internal/game/act_talk.go +++ b/internal/game/act_talk.go @@ -55,8 +55,8 @@ func (g *Game) startTalk(sess *net.Session, p *player.Player, obj *object.Object } } -func (g *Game) handleNodeAction(sess *net.Session, na *behavior.Step) (intercepted bool) { - g.applyTalkStep(sess, na) +func (g *Game) handleNodeAction(sess *net.Session, na *behavior.Step) (intercepted bool, applied bool) { + applied = g.applyTalkStep(sess, na) if v, ok := g.GlobalFlags.Get("guard_hostile"); ok && v != nil { g.GlobalFlags.Delete("guard_hostile") @@ -70,11 +70,11 @@ func (g *Game) handleNodeAction(sess *net.Session, na *behavior.Step) (intercept guardMob.Protected = false g.startCombat(sess, p, guardMob) } - return true + return true, applied } } } - return false + return false, applied } func (g *Game) showTalkNode(sess *net.Session, node behavior.TalkNode) { @@ -90,12 +90,19 @@ func (g *Game) showTalkNode(sess *net.Session, node behavior.TalkNode) { for { if node.Condition == nil || g.checkCondition(sess, node.Condition) { - if len(node.Messages) > 0 { - if node.Action != nil { - if g.handleNodeAction(sess, node.Action) { - return - } + // Apply the node's action first. A failed preflight (missing item, + // full inventory, unaffordable cost) applies nothing, so skip the + // node's success messages rather than narrating a trade that did + // not happen; the player still gets the node's options. + actionApplied := true + if node.Action != nil { + intercepted, applied := g.handleNodeAction(sess, node.Action) + if intercepted { + return } + actionApplied = applied + } + if actionApplied && len(node.Messages) > 0 { td.SeqIndex = 0 msg := node.Messages[0] if msg != "" { @@ -107,12 +114,6 @@ func (g *Game) showTalkNode(sess *net.Session, node behavior.TalkNode) { sess.Write(g.colorize(sess, "dialog_options", "[enter to continue]\n")) return } - } else { - if node.Action != nil { - if g.handleNodeAction(sess, node.Action) { - return - } - } } visible := g.visibleOptions(sess, node.Options) if len(visible) > 0 { @@ -313,7 +314,21 @@ func (g *Game) advanceTalk(sess *net.Session, p *player.Player) { td.PendingChoice = "" td.PendingAction = nil if action != nil { - if g.handleNodeAction(sess, action) { + intercepted, applied := g.handleNodeAction(sess, action) + if intercepted { + return + } + if !applied { + // The option's cost could not be met and no effect was applied. + // Re-show the current node's options instead of advancing to the + // success node and its misleading success text. + if node, ok := td.Cfg.Nodes[td.Node]; ok { + g.finishSequence(sess, node) + } else { + g.cancelAction(p) + sess.SetState(net.StateGame) + g.writePrompt(sess) + } return } } diff --git a/internal/game/casino.go b/internal/game/casino.go index bc702e1..4dfced6 100644 --- a/internal/game/casino.go +++ b/internal/game/casino.go @@ -568,7 +568,7 @@ func (g *Game) CasinoTick() { func (g *Game) emitCasinoEvents(events []casino.Event) { for _, event := range events { - if event.Message == "" && event.Menu == nil && event.Blackjack == nil && event.Baccarat == nil && event.Slot == nil { + if event.Message == "" && event.PrivateMessage == "" && event.Menu == nil && event.Blackjack == nil && event.Baccarat == nil && event.Slot == nil { continue } category := event.Color diff --git a/internal/game/cmd_bank.go b/internal/game/cmd_bank.go index 468b173..0bfc2a3 100644 --- a/internal/game/cmd_bank.go +++ b/internal/game/cmd_bank.go @@ -236,8 +236,11 @@ func (g *Game) doBankWithdraw(sess *net.Session, input string) { entries = append(entries, entryRef{slot: i, pos: len(entries) + 1, name: itemName, id: bs.ItemID, qty: bs.Quantity}) } - idx, err := parseChoiceIndex(input) - if err == nil && idx > 0 && idx <= len(entries) { + // Only a pure number selects a bank slot by position: parseChoiceIndex + // accepts a leading integer and ignores trailing text, so "withdraw 2 + // chips" would otherwise withdraw display entry #2 instead of the named + // item. + if idx, err := strconv.Atoi(strings.TrimSpace(input)); err == nil && idx > 0 && idx <= len(entries) { e := entries[idx-1] g.withdrawBankItem(sess, p, e.slot) return diff --git a/internal/game/cmd_consume.go b/internal/game/cmd_consume.go index a49d897..2e05620 100644 --- a/internal/game/cmd_consume.go +++ b/internal/game/cmd_consume.go @@ -228,8 +228,8 @@ func (g *Game) applyPotion(sess *net.Session, p *player.Player, itemID string, d case "battery": batteryRestore := def.PotionBonus p.Battery += float64(batteryRestore) - if p.Battery > 100 { - p.Battery = 100 + if max := p.MaxBattery(); p.Battery > max { + p.Battery = max } sess.WriteLine(fmt.Sprintf("You drink the %s. Your battery is restored by %d%%.", g.itemColorize(sess, def, def.Name), batteryRestore)) diff --git a/internal/game/cmd_drop.go b/internal/game/cmd_drop.go index 5920696..2e27f91 100644 --- a/internal/game/cmd_drop.go +++ b/internal/game/cmd_drop.go @@ -51,15 +51,24 @@ func (g *Game) doDropAllNamed(sess *net.Session, input string) { coloredName := g.itemColorize(sess, def, name) if def != nil && def.Stackable { - slot := p.InvSlot(matches[0].Slot) - qty := slot.Quantity - p.SetInvSlot(matches[0].Slot, nil) - g.World.AddGroundItem(p.RoomID, itemID, qty) + // Drop every matching stack, not just the first: multiple stacks of + // one stackable item are normal (trigger/gather/steal placement does + // not always merge), so "drop all " must span them. + total := 0 + for _, match := range matches { + slot := p.InvSlot(match.Slot) + if slot == nil || slot.ItemID != itemID || slot.Quantity <= 0 { + continue + } + total += slot.Quantity + p.SetInvSlot(match.Slot, nil) + g.World.AddGroundItem(p.RoomID, itemID, slot.Quantity) + } g.saveCharacter(p) - if qty == 1 { + if total == 1 { sess.WriteLine(fmt.Sprintf("You drop a %s.", coloredName)) } else { - sess.WriteLine(fmt.Sprintf("You drop %d x %s.", qty, coloredName)) + sess.WriteLine(fmt.Sprintf("You drop %d x %s.", total, coloredName)) } return } @@ -110,11 +119,11 @@ func (g *Game) doDrop(sess *net.Session, input string) { if qty == 0 { if def != nil && def.Stackable { + // "drop " means the whole item: sum every matching stack. for i := 0; i < 28; i++ { slot := p.InvSlot(i) if slot != nil && slot.ItemID == itemID && slot.Quantity > 0 { - qty = slot.Quantity - break + qty += slot.Quantity } } } else { @@ -128,21 +137,35 @@ func (g *Game) doDrop(sess *net.Session, input string) { } if def != nil && def.Stackable { - slot := p.InvSlot(matches[0].Slot) - if qty > slot.Quantity { - qty = slot.Quantity - } - if slot.Quantity > qty { - slot.Quantity -= qty - } else { - p.SetInvSlot(matches[0].Slot, nil) + // Consume the requested quantity across every matching stack, not just + // the first one. + remaining := qty + for _, match := range matches { + if remaining <= 0 { + break + } + slot := p.InvSlot(match.Slot) + if slot == nil || slot.ItemID != itemID || slot.Quantity <= 0 { + continue + } + take := slot.Quantity + if take > remaining { + take = remaining + } + if slot.Quantity > take { + slot.Quantity -= take + } else { + p.SetInvSlot(match.Slot, nil) + } + g.World.AddGroundItem(p.RoomID, itemID, take) + remaining -= take } - g.World.AddGroundItem(p.RoomID, itemID, qty) + dropped := qty - remaining g.saveCharacter(p) - if qty == 1 { + if dropped == 1 { sess.WriteLine(fmt.Sprintf("You drop a %s.", g.itemColorize(sess, def, name))) } else { - sess.WriteLine(fmt.Sprintf("You drop %d x %s.", qty, g.itemColorize(sess, def, name))) + sess.WriteLine(fmt.Sprintf("You drop %d x %s.", dropped, g.itemColorize(sess, def, name))) } return } diff --git a/internal/game/cmd_estate_directory.go b/internal/game/cmd_estate_directory.go index 3da2c18..9966e52 100644 --- a/internal/game/cmd_estate_directory.go +++ b/internal/game/cmd_estate_directory.go @@ -182,7 +182,7 @@ func (g *Game) getOwnedHouseRoom(p *player.Player, flagKey string) int { } houseMap := map[string]int{ - "local_neighborhood": 300, + "local_neighborhood": 99999300, } return houseMap[v] } @@ -196,6 +196,14 @@ func (g *Game) teleportPlayer(sess *net.Session, p *player.Player, roomID int) { } } + // A teleport changes rooms just like a walk, so it must clear any safespot + // cover: isSafespotted/safespotBlocksMelee/safespotBlocksHazard compare no + // room, so leaving the state active grants melee and hazard immunity in the + // destination and keeps the player as a phantom occupant of the old cover. + if ss, ok := g.safespot.Get(p.Name); ok { + g.forceLeaveSafespot(sess, p, &ss, "") + } + p.RoomID = roomID p.Stats.RecordRoomVisit(roomID) if g.Hub != nil { diff --git a/internal/game/cmd_quit.go b/internal/game/cmd_quit.go index bf46748..249df80 100644 --- a/internal/game/cmd_quit.go +++ b/internal/game/cmd_quit.go @@ -74,7 +74,16 @@ func (g *Game) beginQuitCountdown(sess *net.Session) { g.broadcastAction(sess, "%s sits down to rest.", p.Name) ticksLeft := 10 - id := g.SubscribeTicks(engine.ToTicks(1), func() bool { + var id uint64 + id = g.SubscribeTicks(engine.ToTicks(1), func() bool { + // The engine snapshots its subscribers before firing them, so this + // callback can still run once after cancelRest unsubscribes it. Bail + // out unless this is still the registered countdown, so a quit + // cancelled on the final tick never force-disconnects the player (and + // never forfeits a committed casino wager). + if g.restTimers[p.Name] != id { + return false + } switch ticksLeft { case 10: sess.WriteLine("You sit down to rest...") diff --git a/internal/game/cmd_swapid.go b/internal/game/cmd_swapid.go index ef862b9..9ee832a 100644 --- a/internal/game/cmd_swapid.go +++ b/internal/game/cmd_swapid.go @@ -141,12 +141,13 @@ func (g *Game) executeSwapID(sess *net.Session, args []string, rawInput string) updated++ } - // Migrate room-ID-embedded state across all characters (online + offline): - // discovered-exit player flags, current RoomID, EnterSeqRoom, MapSymbols, - // and RoomsVisited. This is what the player asked for — without it, - // swapid leaves every character's hidden_exit__ flags (and more) - // pointing at stale room IDs. - g.RewriteRoomIDs(idMap) + // Full runtime migration: Hub room buckets, casino room keys, world + // room-keyed state (ground items, seeded markers, object states), live + // mob instances, the room index, and every character's embedded room-ID + // state. Without this, a swap leaves live gameplay attached to the old + // locations until restart. This runs on the owner (swapid is executed + // from the owner command loop) and must not dispatch again. + g.MigrateRoomIDsOnOwner(idMap) sess.WriteLine(fmt.Sprintf("Swapped room IDs %d and %d (%d files updated).", id1, id2, updated)) } diff --git a/internal/game/cmd_trigger_utility.go b/internal/game/cmd_trigger_utility.go index b5a168b..f9e3692 100644 --- a/internal/game/cmd_trigger_utility.go +++ b/internal/game/cmd_trigger_utility.go @@ -107,12 +107,9 @@ func (g *Game) triggerEmGrab(sess *net.Session, p *player.Player, mod *ModDef, t return } - if p.FirstFreeSlot() < 0 { - sess.WriteLine("Your inventory is full.") - return - } - - groundItems := g.World.GroundItems(p.RoomID) + // Reservation-aware: only items this player may take are considered, so + // em grab cannot magnetize kill loot reserved for another player. + groundItems := g.World.AccessibleGround(p.RoomID, p.Name) var matchedID string for itemID := range groundItems { def, _ := g.ItemStore.Load(itemID) @@ -143,41 +140,58 @@ func (g *Game) triggerEmGrab(sess *net.Session, p *player.Player, mod *ModDef, t } // Credits are currency, not an inventory item: collect them the same way - // the get command does. + // the get command does, respecting the balance cap so the excess stays on + // the ground instead of being removed and clamped away. if matchedID == "credits" { - g.World.RemoveGroundItem(p.RoomID, matchedID, qty) - g.grantCredits(p, qty) + room := creditHeadroom(p) + if room <= 0 { + sess.WriteLine(g.colorize(sess, "currency_pickup", "Your purse can't hold any more credits.")) + return + } + taken := g.World.RemoveAvailableItem(p.RoomID, matchedID, room, p.Name) + if taken <= 0 { + return + } + g.grantCredits(p, taken) + g.saveCharacter(p) g.triggerModReward(sess, p, mod, 1.0) - sess.WriteLine(fmt.Sprintf("You magnetically pull %d credits toward you.", qty)) + sess.WriteLine(fmt.Sprintf("You magnetically pull %d credits toward you.", taken)) return } if def != nil && def.Stackable { - g.World.RemoveGroundItem(p.RoomID, matchedID, qty) - if !placeCraftOutput(p, matchedID, def, qty) { - g.World.AddGroundItem(p.RoomID, matchedID, qty) + taken := g.World.RemoveAvailableItem(p.RoomID, matchedID, qty, p.Name) + if taken <= 0 { + sess.WriteLine("You don't see that here.") + return + } + if !placeCraftOutput(p, matchedID, def, taken) { + g.World.AddGroundItem(p.RoomID, matchedID, taken) sess.WriteLine("Your inventory is full.") return } } else { // Non-stackable items take one slot each; leave any remainder on the - // ground instead of cramming N items into a single slot. - taken := 0 - for taken < qty { + // ground instead of cramming N items into a single slot. Remove from + // the ground first (reservation-aware) so a placement failure cannot + // duplicate stock. + taken, ok := g.World.RemoveReservedItem(p.RoomID, matchedID, qty, p.Name) + if !ok || taken == 0 { + sess.WriteLine("Your inventory is full.") + return + } + placed := 0 + for placed < taken { freeSlot := p.FirstFreeSlot() if freeSlot < 0 { break } p.SetInvSlot(freeSlot, &player.InventorySlot{ItemID: matchedID, Quantity: 1}) - taken++ - } - if taken == 0 { - sess.WriteLine("Your inventory is full.") - return + placed++ } - g.World.RemoveGroundItem(p.RoomID, matchedID, taken) - if taken < qty { - sess.WriteLine(fmt.Sprintf("Your inventory fills up; %d %s remain on the ground.", qty-taken, name)) + if placed < taken { + g.World.AddGroundItem(p.RoomID, matchedID, taken-placed) + sess.WriteLine(fmt.Sprintf("Your inventory fills up; %d %s remain on the ground.", taken-placed, name)) } } diff --git a/internal/game/core_validate.go b/internal/game/core_validate.go index 0e302c0..b6cb3b9 100644 --- a/internal/game/core_validate.go +++ b/internal/game/core_validate.go @@ -37,16 +37,29 @@ func (g *Game) validationSource() validate.Source { techIDs[t.ID] = true } + moduleElements := make(map[string]bool) + moduleDestinations := make(map[string]int) + for _, m := range AllMods { + if m.Element != "" { + moduleElements[m.Element] = true + } + if m.Destination > 0 { + moduleDestinations[m.ID] = m.Destination + } + } + return validate.Source{ - DataDir: g.DataDir, - Items: g.ItemStore, - Objects: g.ObjectStore, - Mobs: g.MobStore, - World: g.World, - Courses: courseViews, - Techs: techViews, - TechIDs: techIDs, - RootRooms: g.ValidationConfig.RootRooms, - IgnoreUnreachable: g.ValidationConfig.IgnoreUnreachable, + DataDir: g.DataDir, + Items: g.ItemStore, + Objects: g.ObjectStore, + Mobs: g.MobStore, + World: g.World, + Courses: courseViews, + Techs: techViews, + TechIDs: techIDs, + RootRooms: g.ValidationConfig.RootRooms, + IgnoreUnreachable: g.ValidationConfig.IgnoreUnreachable, + ModuleElements: moduleElements, + ModuleDestinations: moduleDestinations, } } diff --git a/internal/game/economy.go b/internal/game/economy.go index b436a29..3505a86 100644 --- a/internal/game/economy.go +++ b/internal/game/economy.go @@ -43,6 +43,7 @@ package game import ( "bufio" + "bytes" "crypto/rand" "encoding/hex" "encoding/json" @@ -113,6 +114,12 @@ type economyJournal struct { // does) or a crash can lose the whole journal — including the record of // a debit that is already durable in the character file. dirSynced bool + // tailChecked records whether the journal tail has been verified to end on + // a newline before the first append of this process. A torn final line + // (short write) must be truncated before a new record is appended, or the + // two merge into one malformed mid-file line that permanently aborts + // recovery. + tailChecked bool // stakes indexes outstanding wager records per player identity key // (economyKey: the character's stable ID when known, else its name). // All journal mutations happen on the game owner (or during startup @@ -166,6 +173,12 @@ func (j *economyJournal) append(rec economyRecord) error { if err != nil { return err } + if !j.tailChecked { + if err := j.repairTornTail(); err != nil { + return fmt.Errorf("repair economy journal tail: %w", err) + } + j.tailChecked = true + } f, err := os.OpenFile(j.path, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0600) if err != nil { return fmt.Errorf("open economy journal: %w", err) @@ -186,6 +199,61 @@ func (j *economyJournal) append(rec economyRecord) error { return nil } +// repairTornTail guarantees the journal ends on a newline before a new record +// is appended. A short write (ENOSPC, a flaky filesystem) can leave a partial +// JSON suffix; appending a full record after it merges the two into one +// malformed mid-file line, which loadFolded treats as fatal and which then +// permanently disables recovery and compaction. Truncating the torn suffix is +// safe: a partial record was never a committed transaction. This runs at most +// once per process (append is otherwise hot). +func (j *economyJournal) repairTornTail() error { + f, err := os.OpenFile(j.path, os.O_RDWR, 0600) + if err != nil { + if os.IsNotExist(err) { + return nil + } + return err + } + defer f.Close() + + fi, err := f.Stat() + if err != nil { + return err + } + size := fi.Size() + if size == 0 { + return nil + } + + last := make([]byte, 1) + if _, err := f.ReadAt(last, size-1); err != nil { + return err + } + if last[0] == '\n' { + return nil + } + + // Scan backwards in bounded chunks for the last newline and truncate the + // torn suffix after it. If none exists the whole file is one torn line. + const chunk = 64 * 1024 + buf := make([]byte, chunk) + for end := size; end > 0; { + start := end - chunk + if start < 0 { + start = 0 + } + n := int(end - start) + if _, err := f.ReadAt(buf[:n], start); err != nil { + return err + } + if idx := bytes.LastIndexByte(buf[:n], '\n'); idx >= 0 { + return f.Truncate(start + int64(idx) + 1) + } + end = start + } + return f.Truncate(0) +} + // appendRetry appends one durable line, retrying a bounded number of // times: a transient write failure must not permanently lose a marker // that money correctness depends on. diff --git a/internal/game/exit_migrate.go b/internal/game/exit_migrate.go index 1fceca9..2c56d27 100644 --- a/internal/game/exit_migrate.go +++ b/internal/game/exit_migrate.go @@ -130,7 +130,10 @@ func (g *Game) RewriteRoomIDs(idMap map[int]int) { if rewritePlayerRoomIDs(sess.Player, idMap) { g.saveCharacter(sess.Player) } - online[sess.Player.Name] = true + // Key case-insensitively: a character file stem can differ in case + // from the loaded Player.Name, and a case-sensitive set would miss it + // and remap that online character a second time on disk. + online[strings.ToLower(sess.Player.Name)] = true } } names, err := g.AccountStore.ListCharacterNames() @@ -138,7 +141,7 @@ func (g *Game) RewriteRoomIDs(idMap map[int]int) { return } for _, name := range names { - if online[name] { + if online[strings.ToLower(name)] { continue } p, err := g.AccountStore.LoadCharacter(name) diff --git a/internal/game/game.go b/internal/game/game.go index de383a6..3ffc72e 100644 --- a/internal/game/game.go +++ b/internal/game/game.go @@ -87,6 +87,11 @@ type Game struct { shutdownRun *scheduledShutdown shutdownHook func(reason string) transportStop func() + // shutdownDone is closed after runShutdown's ordered sequence (drain, + // refund, final save, transport close) completes. main waits on it on the + // transport-failure path before reading ShutdownSaveError, so it cannot + // exit while the final save is still in flight. + shutdownDone chan struct{} // shutdownSaveMu guards shutdownSaveErr, the error of the final save // runShutdown performs (nil when it succeeded or no shutdown ran). main @@ -140,6 +145,7 @@ func New(dataDir string, colorConfig *config.ColorsConfig, constantColorConfig * guardWatchTimers: make(map[string]int), hackingStates: make(map[string]*hacking.Session), sequences: make(map[string]*sequence), + shutdownDone: make(chan struct{}), } g.owner = newOwner() // Runtime assignment (not initializer-level) so the command registry, @@ -250,6 +256,15 @@ func (g *Game) ShutdownSaveError() error { return g.shutdownSaveErr } +// ShutdownDone returns a channel that is closed once the graceful shutdown +// sequence (including the final online-character save and the transport +// close) has completed. A caller that starts shutdown itself (the +// transport-failure path) waits on it before reading ShutdownSaveError and +// exiting, so the save is not skipped. +func (g *Game) ShutdownDone() <-chan struct{} { + return g.shutdownDone +} + // recordShutdownSaveErr stores the final-save failure for ShutdownSaveError. // The shutdown sequence runs on its own goroutine, so the field is mutex // guarded against the reader in main. @@ -300,6 +315,9 @@ func (g *Game) runShutdown() { g.transportStop() } log.Println("shutdown complete") + // Publish completion last: main waits on this before reading + // ShutdownSaveError on the transport-failure path. + close(g.shutdownDone) } // saveAllOnlineCharacters persists every online player exactly once under diff --git a/internal/game/hardening_fixes_batch2_test.go b/internal/game/hardening_fixes_batch2_test.go new file mode 100644 index 0000000..e81dd9c --- /dev/null +++ b/internal/game/hardening_fixes_batch2_test.go @@ -0,0 +1,203 @@ +package game + +// Regression tests for the second hardening pass: transactional trigger +// effects, ammo preservation on death, safespot clearing on teleport, and +// economy-journal torn-tail repair. + +import ( + "os" + "path/filepath" + "testing" + + "thehouseoficarus/internal/behavior" + "thehouseoficarus/internal/item" + "thehouseoficarus/internal/player" +) + +func seedBatchItems(t *testing.T, dir string) { + t.Helper() + writeSeedFile(t, filepath.Join(dir, "items", "cowhide.yaml"), + []byte("id: cowhide\nname: cowhide\nvalue: 1\n")) + writeSeedFile(t, filepath.Join(dir, "items", "leather.yaml"), + []byte("id: leather\nname: leather\nvalue: 1\nstackable: true\n")) + writeSeedFile(t, filepath.Join(dir, "items", "rock.yaml"), + []byte("id: rock\nname: rock\nvalue: 1\n")) + writeSeedFile(t, filepath.Join(dir, "items", "arrow.yaml"), + []byte("id: arrow\nname: arrow\nvalue: 1\nstackable: true\n")) + writeSeedFile(t, filepath.Join(dir, "items", "helm.yaml"), + []byte("id: helm\nname: helm\nvalue: 100\n")) + writeSeedFile(t, filepath.Join(dir, "items", "body.yaml"), + []byte("id: body\nname: body\nvalue: 100\n")) + writeSeedFile(t, filepath.Join(dir, "items", "sword.yaml"), + []byte("id: sword\nname: sword\nvalue: 100\n")) +} + +// TestApplyStepUnaffordablePurchaseIsAtomic: a step that both sets a flag and +// charges credits must apply nothing when the player cannot afford it. +func TestApplyStepUnaffordablePurchaseIsAtomic(t *testing.T) { + h := newHarness(t) + seedBatchItems(t, h.dir) + h.g.ItemStore.Reload(h.dir) + h.seedAccount("alice") + h.seedCharacter("alice", "alice") + sess, _ := h.connect("alice") + p := sess.Player + p.Credits = 0 + p.Flags = map[string]any{} + + step := &behavior.Step{Credits: -10, SetPlayerFlags: map[string]any{"bought": true}} + if h.g.applyStep(sess, p, step, p.RoomID, scopePlayer, nil) { + t.Fatal("unaffordable purchase reported success") + } + if _, ok := p.Flags["bought"]; ok { + t.Fatal("flag granted on a failed purchase") + } + if p.Credits != 0 { + t.Fatalf("credits changed on a failed purchase: %d", p.Credits) + } +} + +// TestApplyStepFullInventoryTradeIsAtomic: a take+give+charge step with no +// room for the reward must consume nothing and charge nothing. +func TestApplyStepFullInventoryTradeIsAtomic(t *testing.T) { + h := newHarness(t) + seedBatchItems(t, h.dir) + h.g.ItemStore.Reload(h.dir) + h.seedAccount("alice") + h.seedCharacter("alice", "alice") + sess, _ := h.connect("alice") + p := sess.Player + p.Inventory = make(map[int]*player.InventorySlot) + p.SetInvSlot(0, &player.InventorySlot{ItemID: "cowhide", Quantity: 1}) + for i := 1; i < 28; i++ { + p.SetInvSlot(i, &player.InventorySlot{ItemID: "rock", Quantity: 1}) + } + p.Credits = 100 + + step := &behavior.Step{TakeItem: "cowhide", GiveItem: "leather", Credits: -10} + if h.g.applyStep(sess, p, step, p.RoomID, scopePlayer, nil) { + t.Fatal("full-inventory trade reported success") + } + if !p.HasItem("cowhide") { + t.Fatal("cowhide consumed on a failed trade") + } + if p.HasItem("leather") { + t.Fatal("leather delivered on a failed trade") + } + if p.Credits != 100 { + t.Fatalf("credits charged on a failed trade: %d", p.Credits) + } +} + +// TestApplyStepStackableRewardMergesWhenFull: a stackable reward that fits an +// existing stack must still be delivered when every slot is occupied. +func TestApplyStepStackableRewardMergesWhenFull(t *testing.T) { + h := newHarness(t) + seedBatchItems(t, h.dir) + h.g.ItemStore.Reload(h.dir) + h.seedAccount("alice") + h.seedCharacter("alice", "alice") + sess, _ := h.connect("alice") + p := sess.Player + p.Inventory = make(map[int]*player.InventorySlot) + p.SetInvSlot(0, &player.InventorySlot{ItemID: "leather", Quantity: 1}) + for i := 1; i < 28; i++ { + p.SetInvSlot(i, &player.InventorySlot{ItemID: "rock", Quantity: 1}) + } + + if !h.g.applyStep(sess, p, &behavior.Step{GiveItem: "leather"}, p.RoomID, scopePlayer, nil) { + t.Fatal("stackable reward should merge into a full inventory") + } + if slot := p.InvSlot(0); slot == nil || slot.Quantity != 2 { + t.Fatalf("reward did not merge into the existing stack: %+v", slot) + } +} + +// TestDropItemsOnDeathPreservesAmmoStack: an equipped ammo stack must be +// dropped in full (and AmmoQty cleared), not silently reduced to one unit. +func TestDropItemsOnDeathPreservesAmmoStack(t *testing.T) { + h := newHarness(t) + seedBatchItems(t, h.dir) + h.g.ItemStore.Reload(h.dir) + + p := player.New("alice") + p.RoomID = 1 + p.Credits = 0 + p.Equipment = map[item.EquipSlot]string{ + item.SlotAmmo: "arrow", + item.SlotHead: "helm", + item.SlotTorso: "body", + item.SlotMainHand: "sword", + } + p.AmmoQty = 50 + + h.g.RunOnOwner(func() { + h.g.dropItemsOnDeath(p) + }) + + if p.AmmoQty != 0 { + t.Fatalf("AmmoQty not cleared after the stack dropped: %d", p.AmmoQty) + } + ground := h.g.World.GroundItems(p.RoomID) + if ground["arrow"] != 50 { + t.Fatalf("ammo stack not preserved on the ground: %+v", ground) + } +} + +// TestTeleportPlayerClearsSafespot: cover immunity must not follow a teleport +// into a room with no cover. +func TestTeleportPlayerClearsSafespot(t *testing.T) { + h := newHarness(t) + h.seedAccount("alice") + h.seedCharacter("alice", "alice") + sess, _ := h.connect("alice") + p := sess.Player + h.g.safespot.Set(p.Name, SafespotState{Active: true, RoomID: p.RoomID, ObjectDefID: "rock"}) + + h.g.RunOnOwner(func() { + h.g.teleportPlayer(sess, p, 2) + }) + + if h.g.safespot.Has(p.Name) { + t.Fatal("safespot survived a teleport") + } + if p.RoomID != 2 { + t.Fatalf("room after teleport = %d, want 2", p.RoomID) + } +} + +// TestEconomyJournalRepairsTornTail: a torn final line must be truncated +// before the next append, so the journal does not become permanently +// unreadable (which would disable recovery and compaction). +func TestEconomyJournalRepairsTornTail(t *testing.T) { + dir := t.TempDir() + j := newEconomyJournal(dir) + if err := os.MkdirAll(filepath.Join(dir, "economy"), 0700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(j.path, []byte(`{"id":"torn","kind":"payout","status":"open"`), 0600); err != nil { + t.Fatal(err) + } + + if err := j.append(economyRecord{ID: "good", Player: "alice", Kind: "payout", Status: "open", Credit: 5}); err != nil { + t.Fatalf("append after torn tail: %v", err) + } + open, err := j.loadOpen() + if err != nil { + t.Fatalf("loadOpen after torn-tail repair: %v", err) + } + for _, rec := range open { + if rec.ID == "torn" { + t.Fatal("torn record survived repair") + } + } + found := false + for _, rec := range open { + if rec.ID == "good" { + found = true + } + } + if !found { + t.Fatal("valid record missing after torn-tail repair") + } +} diff --git a/internal/game/production_menu.go b/internal/game/production_menu.go index 4f013e7..9b9f8e6 100644 --- a/internal/game/production_menu.go +++ b/internal/game/production_menu.go @@ -107,6 +107,9 @@ func (g *Game) formatMaterials(sess *net.Session, item *item.ItemDef) string { } var parts []string for _, e := range craft.Ingredients { + if len(e.Items) == 0 { + continue + } itemName := e.Items[0] if def, err := g.ItemStore.Load(e.Items[0]); err == nil { itemName = g.itemColorize(sess, def, def.Name) diff --git a/internal/game/sys_combat.go b/internal/game/sys_combat.go index b2a2463..af8cc75 100644 --- a/internal/game/sys_combat.go +++ b/internal/game/sys_combat.go @@ -5,6 +5,7 @@ import ( "math/rand" "sort" + "thehouseoficarus/internal/item" "thehouseoficarus/internal/net" "thehouseoficarus/internal/player" ) @@ -36,13 +37,23 @@ func (g *Game) dropItemsOnDeath(p *player.Player) { } for eqSlot, itemID := range p.Equipment { + quantity := 1 + // Ammo keeps its stack count in AmmoQty, not in an inventory slot; + // dropping the equipment entry without moving that count would destroy + // the rest of the stack and leave a stale ammo_qty on disk. + if eqSlot == item.SlotAmmo { + quantity = p.AmmoQty + if quantity <= 0 { + quantity = 1 + } + } val := 0 if def, err := g.ItemStore.Load(itemID); err == nil { - val = def.Value + val = def.Value * quantity } items = append(items, deathDrop{ itemID: itemID, - quantity: 1, + quantity: quantity, totalValue: val, isEquip: true, equipSlot: eqSlot, @@ -61,6 +72,12 @@ func (g *Game) dropItemsOnDeath(p *player.Player) { it := items[i] if it.isEquip { delete(p.Equipment, it.equipSlot) + // Equipped quality is keyed by item id and must not outlive the + // equipment entry, or a later drop/wear reads a stale quality. + delete(p.EquipQuality, it.itemID) + if it.equipSlot == item.SlotAmmo { + p.AmmoQty = 0 + } } else { p.SetInvSlot(it.invSlot, nil) } diff --git a/internal/net/conn.go b/internal/net/conn.go index 2a250d8..42b7c41 100644 --- a/internal/net/conn.go +++ b/internal/net/conn.go @@ -9,6 +9,7 @@ import ( "io" "net" "strings" + "sync/atomic" "time" ) @@ -25,9 +26,11 @@ type Conn interface { } type tcpConn struct { - conn net.Conn - reader *bufio.Reader - echoOff bool + conn net.Conn + reader *bufio.Reader + // echoOff is written by the owner goroutine (SetEcho/SetEchoState) and + // read by this connection's reader goroutine, so it must be atomic. + echoOff atomic.Bool } func newTCPConn(conn net.Conn) *tcpConn { @@ -69,7 +72,7 @@ func (t *tcpConn) ReadMessage() (string, error) { } text := strings.TrimRight(string(line), "\r\n") text = stripIAC(text) - if t.echoOff { + if t.echoOff.Load() { t.conn.Write([]byte("\r\n")) } return text, nil @@ -88,7 +91,7 @@ func (t *tcpConn) Close() error { } func (t *tcpConn) SetEcho(on bool) error { - t.echoOff = !on + t.echoOff.Store(!on) if on { return t.writeTelnetCmd(wont, echo) } @@ -109,7 +112,7 @@ func (t *tcpConn) EchoSequence(on bool) []byte { // SetEchoState records the echo state without writing to the network; used // when the negotiation bytes are queued instead of written inline. func (t *tcpConn) SetEchoState(on bool) { - t.echoOff = !on + t.echoOff.Store(!on) } const ( diff --git a/internal/player/hardening_removeitem_test.go b/internal/player/hardening_removeitem_test.go new file mode 100644 index 0000000..5cb1e4c --- /dev/null +++ b/internal/player/hardening_removeitem_test.go @@ -0,0 +1,23 @@ +package player + +import "testing" + +// TestRemoveItemRejectsNonPositive: a non-positive quantity must never mutate +// a stack; a negative value used to increase it. +func TestRemoveItemRejectsNonPositive(t *testing.T) { + p := New("alice") + p.SetInvSlot(0, &InventorySlot{ItemID: "coin", Quantity: 5}) + + if p.RemoveItem("coin", -3) { + t.Fatal("negative quantity accepted") + } + if slot := p.InvSlot(0); slot == nil || slot.Quantity != 5 { + t.Fatalf("stack mutated by a negative removal: %+v", slot) + } + if p.RemoveItem("coin", 0) { + t.Fatal("zero quantity accepted") + } + if slot := p.InvSlot(0); slot == nil || slot.Quantity != 5 { + t.Fatalf("stack mutated by a zero removal: %+v", slot) + } +} diff --git a/internal/player/player.go b/internal/player/player.go index f8d9a68..efa906b 100644 --- a/internal/player/player.go +++ b/internal/player/player.go @@ -510,6 +510,12 @@ func (p *Player) CountItem(itemID string) int { } func (p *Player) RemoveItem(itemID string, qty int) bool { + // A non-positive quantity must never mutate a stack: with qty < 0 the + // subtraction below would increase it. Callers pass positive quantities; + // reject anything else instead of creating items from a data typo. + if qty <= 0 { + return false + } remaining := qty for i, slot := range p.Inventory { if slot == nil || slot.ItemID != itemID { diff --git a/internal/validate/checks.go b/internal/validate/checks.go index eb24721..a6eeee8 100644 --- a/internal/validate/checks.go +++ b/internal/validate/checks.go @@ -427,6 +427,16 @@ func validateMobs(s Source) []Issue { }) } + // A weakness that matches no module element is dead data: the +weakness + // bonus can never trigger against any science module. + if s.ModuleElements != nil && def.Combat != nil && def.Combat.Stats.Defenses.Weakness != "" && !s.ModuleElements[def.Combat.Stats.Defenses.Weakness] { + issues = append(issues, Issue{ + Level: "WARN", + Type: "reference", + Message: fmt.Sprintf("Mob %q: weakness %q matches no science module element (the weakness can never trigger)", id, def.Combat.Stats.Defenses.Weakness), + }) + } + if !def.Protected && (def.Combat == nil || def.Combat.Stats.HP <= 0) { hp := 0 if def.Combat != nil { @@ -761,6 +771,15 @@ func validateCraftBlocks(s Source) []Issue { } for _, e := range c.Ingredients { + if len(e.Items) == 0 { + issues = append(issues, Issue{ + Level: "ERROR", + Type: "reference", + Message: fmt.Sprintf("Item %q (craft: %s): ingredient entry has no items; the recipe can never be satisfied", + item.ID, craftLabel), + }) + continue + } // Byproducts pair with the alternatives by index // (collectByproducts reads Byproducts[j] for Items[j]), so a // short or reordered list silently returns the wrong @@ -1165,6 +1184,93 @@ func validateRoomWiring(s Source) []Issue { }) } + // Reachability: an orphan check only catches rooms with zero inbound + // edges, so an entire disconnected component (every member linked only + // within the component) passes silently. BFS the directed exits from the + // configured root rooms and warn about anything with no path in, honouring + // ignore_unreachable. + reachable := make(map[int]bool) + var queue []int + for _, src := range s.RootRooms { + if roomIndex[src] && !reachable[src] { + reachable[src] = true + queue = append(queue, src) + } + } + // Transport module destinations are also legitimate entry points: a + // player can reach them with a module even when no exit path leads there, + // so they must not be reported as unreachable. + for _, dest := range s.ModuleDestinations { + if dest > 0 && roomIndex[dest] && !reachable[dest] { + reachable[dest] = true + queue = append(queue, dest) + } + } + for len(queue) > 0 { + id := queue[0] + queue = queue[1:] + room, err := s.World.LoadRoom(id) + if err != nil { + continue + } + for _, exit := range room.Exits { + next := exit.Room + if next <= 0 || reachable[next] || !roomIndex[next] { + continue + } + reachable[next] = true + queue = append(queue, next) + } + } + var unreachable []int + for id := range roomIndex { + if reachable[id] || ignoreTo[id] || sourceSet[id] { + continue + } + unreachable = append(unreachable, id) + } + if len(unreachable) > 0 { + sort.Ints(unreachable) + var strs []string + for _, id := range unreachable { + strs = append(strs, fmt.Sprintf("%d", id)) + } + issues = append(issues, Issue{ + Level: "WARN", + Type: "integrity", + Message: fmt.Sprintf("Unreachable rooms (no exit path from a configured root room): %s", + strings.Join(strs, ", ")), + }) + } + + return issues +} + +// validateModuleDestinations flags a transport module whose destination room +// does not exist: the game assigns the id unconditionally, so the player is +// saved into a void with no exits. +func validateModuleDestinations(s Source) []Issue { + if len(s.ModuleDestinations) == 0 { + return nil + } + roomIndex := s.World.RoomIndex() + ids := make([]string, 0, len(s.ModuleDestinations)) + for id := range s.ModuleDestinations { + ids = append(ids, id) + } + sort.Strings(ids) + var issues []Issue + for _, id := range ids { + dest := s.ModuleDestinations[id] + if dest <= 0 || roomIndex[dest] { + continue + } + issues = append(issues, Issue{ + Level: "ERROR", + Type: "reference", + Message: fmt.Sprintf("Module %q: destination room %d does not exist", id, dest), + }) + } return issues } diff --git a/internal/validate/hardening_fixes_batch2_test.go b/internal/validate/hardening_fixes_batch2_test.go new file mode 100644 index 0000000..f88c924 --- /dev/null +++ b/internal/validate/hardening_fixes_batch2_test.go @@ -0,0 +1,89 @@ +package validate + +import ( + "path/filepath" + "strings" + "testing" + + "thehouseoficarus/internal/item" + "thehouseoficarus/internal/object" + "thehouseoficarus/internal/world" +) + +// TestValidateModuleDestinationMissingRoom: a transport destination that is +// not a real room soft-locks the player, so it must be a startup ERROR. +func TestValidateModuleDestinationMissingRoom(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "rooms", "1.yaml"), "id: 1\nname: Root\n") + + src := Source{ + DataDir: dir, + World: world.New(dir), + ModuleDestinations: map[string]int{"transport_bad": 999}, + } + issues := validateModuleDestinations(src) + if len(issues) != 1 { + t.Fatalf("issues = %+v, want exactly one", issues) + } + if issues[0].Level != "ERROR" || !strings.Contains(issues[0].Message, "999") { + t.Fatalf("unexpected issue: %+v", issues[0]) + } +} + +// TestValidateReachabilityFlagsDisconnectedComponent: a component whose rooms +// only link to each other has no inbound edge from outside, so the orphan +// check misses it; the reachability BFS must warn. +func TestValidateReachabilityFlagsDisconnectedComponent(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "rooms", "1.yaml"), "id: 1\nname: Root\n") + writeFile(t, filepath.Join(dir, "rooms", "2.yaml"), "id: 2\nname: A\nexits:\n north:\n room: 3\n") + writeFile(t, filepath.Join(dir, "rooms", "3.yaml"), "id: 3\nname: B\nexits:\n south:\n room: 2\n") + + src := Source{DataDir: dir, World: world.New(dir), RootRooms: []int{1}} + issues := validateRoomWiring(src) + + found := false + for _, issue := range issues { + if strings.Contains(issue.Message, "Unreachable rooms") && + strings.Contains(issue.Message, "2") && strings.Contains(issue.Message, "3") { + found = true + if issue.Level != "WARN" { + t.Fatalf("reachability issue must be a WARN: %+v", issue) + } + } + if strings.Contains(issue.Message, "Orphan") { + t.Fatalf("disconnected component must not be reported as an orphan: %+v", issue) + } + } + if !found { + t.Fatalf("expected an unreachable-rooms warning, got %+v", issues) + } +} + +// TestValidateCraftEmptyIngredientIsError: an ingredient entry with no items +// makes formatMaterials index out of range at runtime, so it must be an ERROR. +func TestValidateCraftEmptyIngredientIsError(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "items", "thing.yaml"), + "name: thing\ncraft:\n - type: cooking\n ingredients:\n - quantity: 2\n") + + src := Source{ + DataDir: dir, + Items: item.NewItemStore(dir), + Objects: object.NewObjectStore(dir), + Mobs: world.NewMobStore(dir), + World: world.New(dir), + } + found := false + for _, issue := range validateCraftBlocks(src) { + if strings.Contains(issue.Message, "has no items") { + found = true + if issue.Level != "ERROR" { + t.Fatalf("empty ingredient must be an ERROR: %+v", issue) + } + } + } + if !found { + t.Fatal("expected an ERROR for an ingredient entry with no items") + } +} diff --git a/internal/validate/validate.go b/internal/validate/validate.go index 8ca4555..6a05eb6 100644 --- a/internal/validate/validate.go +++ b/internal/validate/validate.go @@ -57,6 +57,13 @@ type Source struct { TechIDs map[string]bool RootRooms []int IgnoreUnreachable []int + // ModuleElements is the set of science module elements present in the data + // (solar/hydro/eco/bio/chaos). A mob weakness matching none can never + // trigger. Nil skips the check (synthetic test sources). + ModuleElements map[string]bool + // ModuleDestinations maps module id to destination room for transport + // modules. A destination that is not a real room soft-locks the player. + ModuleDestinations map[string]int } // Run executes all checks and returns the collected issues. @@ -67,7 +74,7 @@ func Run(s Source) []Issue { {"items", "item"}, {"rooms", "room"}, {"mobs", "mob"}, {"objects", "object"}, {"drops", "drop table"}, {"courses", "course"}, {"modules", "module"}, {"techs", "tech"}, {"help", "help topic"}, - {"triggers", "trigger"}, + {"triggers", "trigger"}, {"hazards", "hazard"}, } { issues = append(issues, validateIDs(s.DataDir, dir.sub, dir.label)...) } @@ -86,6 +93,7 @@ func Run(s Source) []Issue { issues = append(issues, validateRoomTriggers(s)...) issues = append(issues, validateTechs(s)...) issues = append(issues, validateRoomWiring(s)...) + issues = append(issues, validateModuleDestinations(s)...) issues = append(issues, validateRoomGrid(s)...) issues = append(issues, validateExitReciprocity(s)...) -- cgit v1.2.3