From 153acb7f644d755c45533b7c5311e3ffa47e8973 Mon Sep 17 00:00:00 2001 From: historia <[not public]> Date: Fri, 18 Sep 2026 05:31:09 -0400 Subject: fix: harden engine across casino, economy, concurrency, security, and content --- internal/game/account_txn.go | 90 ++++++- internal/game/act.go | 7 +- internal/game/act_burn.go | 6 + internal/game/act_clean.go | 5 +- internal/game/act_combine.go | 70 +++-- internal/game/act_farm.go | 77 ++++-- internal/game/act_fletch.go | 80 ++++-- internal/game/casino.go | 47 +++- internal/game/cmd_bank.go | 29 +- internal/game/cmd_color.go | 6 + internal/game/cmd_craft.go | 10 +- internal/game/cmd_get.go | 132 +++++++--- internal/game/cmd_option.go | 34 +-- internal/game/cmd_quit.go | 16 +- internal/game/cmd_remove.go | 24 +- internal/game/cmd_shop.go | 14 +- internal/game/cmd_swapid.go | 41 +-- internal/game/cmd_wear.go | 56 +++- internal/game/combat_mob.go | 38 ++- internal/game/core_admin.go | 20 +- internal/game/core_command_queue.go | 37 ++- internal/game/core_login_account.go | 82 +++++- internal/game/core_login_char.go | 37 ++- internal/game/correct_combine_skill_test.go | 100 +++++++ internal/game/correct_online_name_case_test.go | 85 ++++++ .../game/correct_owner_panic_subscription_test.go | 85 ++++++ internal/game/correct_quality_merge_test.go | 201 ++++++++++++++ .../game/correct_vanished_account_reset_test.go | 75 ++++++ internal/game/economy.go | 83 ++++-- internal/game/game.go | 131 +++++++++- .../game/hardening_account_name_validation_test.go | 201 ++++++++++++++ .../hardening_admin_player_file_online_test.go | 70 +++++ internal/game/hardening_ammo_unequip_test.go | 181 +++++++++++++ .../game/hardening_casino_entropy_wiring_test.go | 72 +++++ .../game/hardening_casino_quit_autobet_test.go | 75 ++++++ .../game/hardening_craft_combat_helpers_test.go | 40 +++ .../game/hardening_craft_combine_fletch_test.go | 260 ++++++++++++++++++ internal/game/hardening_craft_menu_skill_test.go | 91 +++++++ internal/game/hardening_craft_message_item_test.go | 85 ++++++ internal/game/hardening_credit_cap_test.go | 105 ++++++++ internal/game/hardening_economy_durability_test.go | 272 +++++++++++++++++++ internal/game/hardening_harvest_yield_test.go | 168 ++++++++++++ internal/game/hardening_login_validation_test.go | 58 ++++ internal/game/hardening_owner_panic_test.go | 162 ++++++++++++ internal/game/hardening_pickup_quality_test.go | 123 +++++++++ .../game/hardening_purge_session_reset_test.go | 50 ++++ internal/game/hardening_queue_cleanup_test.go | 139 ++++++++++ internal/game/hardening_quit_bet_warning_test.go | 79 ++++++ .../game/hardening_retribution_mob_death_test.go | 90 +++++++ internal/game/hardening_security_test.go | 100 +++++++ internal/game/hardening_shipped_content_test.go | 60 +++++ .../game/hardening_shipped_data_validation_test.go | 205 +++++++++++++++ .../game/hardening_shutdown_save_error_test.go | 63 +++++ internal/game/hardening_tick_lifecycle_test.go | 291 +++++++++++++++++++++ internal/game/owner.go | 60 ++++- internal/game/polish_craft_fail_delivery_test.go | 111 ++++++++ internal/game/polish_shop_merge_test.go | 55 ++++ internal/game/polish_wear_swap_quality_test.go | 140 ++++++++++ internal/game/production_engine.go | 102 ++++++-- internal/game/production_index.go | 23 +- internal/game/quit_casino_test.go | 8 +- internal/game/session_registry.go | 8 +- internal/game/sys_combat.go | 14 + internal/game/sys_triggers.go | 15 +- internal/game/tick.go | 34 ++- 65 files changed, 5046 insertions(+), 282 deletions(-) create mode 100644 internal/game/correct_combine_skill_test.go create mode 100644 internal/game/correct_online_name_case_test.go create mode 100644 internal/game/correct_owner_panic_subscription_test.go create mode 100644 internal/game/correct_quality_merge_test.go create mode 100644 internal/game/correct_vanished_account_reset_test.go create mode 100644 internal/game/hardening_account_name_validation_test.go create mode 100644 internal/game/hardening_admin_player_file_online_test.go create mode 100644 internal/game/hardening_ammo_unequip_test.go create mode 100644 internal/game/hardening_casino_entropy_wiring_test.go create mode 100644 internal/game/hardening_casino_quit_autobet_test.go create mode 100644 internal/game/hardening_craft_combat_helpers_test.go create mode 100644 internal/game/hardening_craft_combine_fletch_test.go create mode 100644 internal/game/hardening_craft_menu_skill_test.go create mode 100644 internal/game/hardening_craft_message_item_test.go create mode 100644 internal/game/hardening_credit_cap_test.go create mode 100644 internal/game/hardening_economy_durability_test.go create mode 100644 internal/game/hardening_harvest_yield_test.go create mode 100644 internal/game/hardening_login_validation_test.go create mode 100644 internal/game/hardening_owner_panic_test.go create mode 100644 internal/game/hardening_pickup_quality_test.go create mode 100644 internal/game/hardening_purge_session_reset_test.go create mode 100644 internal/game/hardening_queue_cleanup_test.go create mode 100644 internal/game/hardening_quit_bet_warning_test.go create mode 100644 internal/game/hardening_retribution_mob_death_test.go create mode 100644 internal/game/hardening_security_test.go create mode 100644 internal/game/hardening_shipped_content_test.go create mode 100644 internal/game/hardening_shipped_data_validation_test.go create mode 100644 internal/game/hardening_shutdown_save_error_test.go create mode 100644 internal/game/hardening_tick_lifecycle_test.go create mode 100644 internal/game/polish_craft_fail_delivery_test.go create mode 100644 internal/game/polish_shop_merge_test.go create mode 100644 internal/game/polish_wear_swap_quality_test.go (limited to 'internal/game') diff --git a/internal/game/account_txn.go b/internal/game/account_txn.go index cf31dea..13f7695 100644 --- a/internal/game/account_txn.go +++ b/internal/game/account_txn.go @@ -121,9 +121,36 @@ func (g *Game) canonicalChar(name string) (string, error) { return g.AccountStore.FindCharacter(name) } +// validateAccountName and validateCharacterName reject names that any +// AccountPath/CharPath join would interpret as a path (separators, dots) or +// that no account/character in this store may carry. Every lifecycle +// transaction validates its names BEFORE it touches the filesystem: a +// caller-supplied or account-file-supplied name such as "../../x" would +// otherwise make AccountPath/CharPath read or delete files outside +// players/, and a transaction is not the place to discover that. +func validateAccountName(name string) error { + if err := player.ValidName(name); err != nil { + return fmt.Errorf("invalid account name %q: %w", name, err) + } + return nil +} + +func validateCharacterName(name string) error { + if err := player.ValidName(name); err != nil { + return fmt.Errorf("invalid character name %q: %w", name, err) + } + return nil +} + // createCharacterTxn creates the character file and appends it to the // owning account in a single transaction. Returns the created player. func (g *Game) createCharacterTxn(accountName, charName string, startingRoom int) (*player.Player, error) { + if err := validateAccountName(accountName); err != nil { + return nil, err + } + if err := validateCharacterName(charName); err != nil { + return nil, err + } p := player.New(charName) p.RoomID = startingRoom p.Stats.RecordRoomVisit(startingRoom) @@ -155,6 +182,15 @@ func (g *Game) createCharacterTxn(accountName, charName string, startingRoom int // renameCharacterTxn atomically renames the character file, persists the new // name inside it, and updates the owning account's character list. func (g *Game) renameCharacterTxn(accountName, oldName, newName string) error { + if err := validateAccountName(accountName); err != nil { + return err + } + if err := validateCharacterName(oldName); err != nil { + return err + } + if err := validateCharacterName(newName); err != nil { + return err + } txn := g.newTxn() if err := txn.Move(g.AccountStore.CharPath(oldName), g.AccountStore.CharPath(newName)); err != nil { return err @@ -202,6 +238,12 @@ func (g *Game) renameCharacterTxn(accountName, oldName, newName string) error { // deleteCharacterTxn removes the character file and updates the account list // atomically. func (g *Game) deleteCharacterTxn(accountName, charName string) error { + if err := validateAccountName(accountName); err != nil { + return err + } + if err := validateCharacterName(charName); err != nil { + return err + } txn := g.newTxn() if err := txn.Delete(g.AccountStore.CharPath(charName)); err != nil { return err @@ -228,8 +270,19 @@ func (g *Game) deleteCharacterTxn(accountName, charName string) error { } // purgeAccountTxn removes the account file and every character file in one -// transaction. +// transaction. The account's character list arrives from the account file, +// so every name in it is validated before the first CharPath join: a +// hand-edited or corrupt entry must not turn a purge into a delete outside +// players/characters/. func (g *Game) purgeAccountTxn(accountName string, characters []string) error { + if err := validateAccountName(accountName); err != nil { + return err + } + for _, name := range characters { + if err := validateCharacterName(name); err != nil { + return err + } + } txn := g.newTxn() for _, name := range characters { if err := txn.Delete(g.AccountStore.CharPath(name)); err != nil { @@ -247,15 +300,30 @@ func (g *Game) purgeAccountTxn(accountName string, characters []string) error { // transaction. Without the character rewrite the renamed account loses // access to its characters: membership authorization compares the // character file's owner against the account name. +// +// Both names and every entry of the account's character list are validated +// up front: each one is joined into AccountPath/CharPath below, and a +// path-like name would escape players/. func (g *Game) renameAccountTxn(oldName, newName string) error { - txn := g.newTxn() - if err := txn.Move(g.AccountStore.AccountPath(oldName), g.AccountStore.AccountPath(newName)); err != nil { + if err := validateAccountName(oldName); err != nil { + return err + } + if err := validateAccountName(newName); err != nil { return err } acc, err := g.AccountStore.LoadAccount(oldName) if err != nil { return err } + for _, charName := range acc.Characters { + if err := validateCharacterName(charName); err != nil { + return err + } + } + txn := g.newTxn() + if err := txn.Move(g.AccountStore.AccountPath(oldName), g.AccountStore.AccountPath(newName)); err != nil { + return err + } acc.Name = newName accData, err := g.AccountStore.MarshalAccount(acc) if err != nil { @@ -311,7 +379,9 @@ func (g *Game) isMemberOf(accountName, characterName string) bool { // publishAccountUpdate reloads the account from disk and republishes its // state to every session of that account, resetting stale pending menu // selections (pending character names, menu choices) after lifecycle -// mutations. Called from the owner goroutine only. +// mutations. A session whose account no longer exists on disk is reset to the +// account-name prompt (resetToAccountName) and closed. Called from the owner +// goroutine only. // // oldName and newName differ only for an account rename: sessions // authenticated before the rename still carry the OLD cached account @@ -321,12 +391,14 @@ func (g *Game) publishAccountUpdate(oldName, newName string) { for _, sess := range g.allSessionsOfEitherAccount(oldName, newName) { acc, err := g.AccountStore.LoadAccount(newName) if err != nil { - // The account file vanished (purge from another session); the - // session is stale and must leave the menu. - sess.Account = nil - sess.PendingChar = "" - sess.PendingMenu = nil + // The account file vanished (purge or a raw admin delete from + // another session); the session is stale and must leave the menu. + // resetToAccountName clears PendingPass and the pending state + // alongside Account: a stale confirm-password still matching + // PendingPass would otherwise reach a handler that expects an + // account bound to the session. sess.WriteLine("\nYour account no longer exists.") + g.resetToAccountName(sess) sess.Close() continue } diff --git a/internal/game/act.go b/internal/game/act.go index 64d1848..1317df4 100644 --- a/internal/game/act.go +++ b/internal/game/act.go @@ -198,7 +198,10 @@ func (g *Game) AdvanceActions() { if g.Hub == nil { return } - for _, sess := range g.Hub.AllSessions() { + // One session snapshot per tick: the background-action pass below used to + // re-snapshot the whole hub. + sessions := g.Hub.AllSessions() + for _, sess := range sessions { p := sess.Player if p == nil || p.Action == nil { continue @@ -245,7 +248,7 @@ func (g *Game) AdvanceActions() { } } - for _, sess := range g.Hub.AllSessions() { + for _, sess := range sessions { p := sess.Player if p == nil || p.BackgroundAction == nil { continue diff --git a/internal/game/act_burn.go b/internal/game/act_burn.go index 2d03ac4..84f7f14 100644 --- a/internal/game/act_burn.go +++ b/internal/game/act_burn.go @@ -417,6 +417,12 @@ func (g *Game) broadcastDrop(p *player.Player, itemID, itemName string) { } func (g *Game) FireTick() { + // Mandatory tick-system guard: the master per-tick subscriber can fire + // before SetHub during startup, and burning out a fire reaches + // cancelStokers -> g.Hub.PlayersInRoom. See building_guide/tick_order.md. + if g.Hub == nil { + return + } for _, st := range g.World.AllFireStates() { st.Quality-- if st.Quality <= 0 { diff --git a/internal/game/act_clean.go b/internal/game/act_clean.go index 2872173..3525475 100644 --- a/internal/game/act_clean.go +++ b/internal/game/act_clean.go @@ -71,7 +71,10 @@ func (g *Game) advanceClean(sess *net.Session, p *player.Player) { } g.saveCharacter(p) - msg := g.resolveCraftMessage(sess, craft.SuccessMessage, "You clean the %i1.", craft, matchItem.ID, nil, p.HasItem) + // The grimy herb's slot was just overwritten with the clean product, so + // p.HasItem is false for it: resolve %i1 from the exact item consumed. + msg := g.resolveCraftMessage(sess, craft.SuccessMessage, "You clean the %i1.", craft, matchItem.ID, nil, + func(id string) bool { return id == consumeID }) if p.OptionBool("xp_drops") && craft.XP > 0 { msg += g.formatXpDropSingle(sess, p, player.Pharmacy, craft.XP) } diff --git a/internal/game/act_combine.go b/internal/game/act_combine.go index 014dbef..701aaec 100644 --- a/internal/game/act_combine.go +++ b/internal/game/act_combine.go @@ -46,6 +46,20 @@ func (g *Game) startCombine(sess *net.Session, p *player.Player, item *item.Item g.broadcastAction(sess, "%s starts combining.", p.Name) } +// combineSkill resolves the skill a combine recipe trains. Combine recipes +// declare type "combine", which is an action, not a skill: player.SkillName +// of it is the literal "combine", so p.Level would read a map miss as level 1 +// when rolling success and awardSkillXP would persist a phantom "combine" +// skill entry. A recipe that declares a real skill name is honored; anything +// else trains Crafting, the skill combine work belongs to. +func combineSkill(craft *item.CraftDef) player.SkillName { + skill := player.SkillName(craft.EffectiveSkill()) + if player.ValidSkill(skill) { + return skill + } + return player.Crafting +} + func (g *Game) advanceCombine(sess *net.Session, p *player.Player) { d, ok := p.Action.Data.(*behavior.CombineData) if !ok { @@ -100,10 +114,11 @@ func (g *Game) advanceCombine(sess *net.Session, p *player.Player) { return } - // A full inventory must never destroy materials: verify the output - // can be placed BEFORE collecting byproducts and consuming. + // A full inventory must never destroy materials: verify BOTH the + // output and any fail replacement can be placed BEFORE collecting + // byproducts and consuming. outDef, _ := g.ItemStore.Load(item.ID) - if !craftOutputFits(p, item.ID, outDef) { + if !craftOutputFits(p, item.ID, outDef) || !g.craftFailFits(p, craft) { sess.WriteLine("Your inventory is too full!") g.cancelAction(p) return @@ -113,31 +128,40 @@ func (g *Game) advanceCombine(sess *net.Session, p *player.Player) { // each ingredient entry to the alternative actually being consumed // (bucket vs jug of water), which requires the items still present. byproducts := g.collectByproducts(p, item) + hasItem := craftConsumedHasItem(craft, p.CountItem, p.HasItem) - consumeIngredients(craft, p.CountItem, p.RemoveItem) - - outputQty := craft.OutputQty - if outputQty <= 0 { - outputQty = 1 - } - - if !placeCraftOutput(p, item.ID, outDef, outputQty) { - // Defensive: the fits check above should have guaranteed a - // stack or slot. - sess.WriteLine("Your inventory is too full!") + if !consumeIngredients(craft, p.CountItem, p.RemoveItem) { g.cancelAction(p) return } - // Return the empty containers (and any other byproducts) alongside - // the output; without this, combine crafts silently discarded them. - for _, bp := range byproducts { - if slot := p.FirstFreeSlot(); slot >= 0 { - p.SetInvSlot(slot, &player.InventorySlot{ItemID: bp, Quantity: 1}) - } else { - // Never silently delete a byproduct: drop the remainder on - // the ground where the player can pick it up. - g.World.AddGroundItem(p.RoomID, bp, 1) + skill := combineSkill(craft) + if craftRollSuccess(craft, p.Level(skill)) { + outputQty := craft.OutputQty + if outputQty <= 0 { + outputQty = 1 + } + + if !placeCraftOutput(p, item.ID, outDef, outputQty) { + // Defensive: the fits check above should have guaranteed a + // stack or slot. + sess.WriteLine("Your inventory is too full!") + g.cancelAction(p) + return + } + + // Return the empty containers (and any other byproducts) alongside + // the output; without this, combine crafts silently discarded them. + g.placeCraftByproducts(p, byproducts) + + if craft.XP > 0 { + g.awardSkillXP(sess, p, skill, craft.XP) + } + } else { + g.placeCraftFail(p, craft) + msg := g.resolveCraftMessage(sess, craft.FailMessage, "", craft, item.ID, nil, hasItem) + if msg != "" { + sess.WriteLine(g.colorize(sess, "damage_taken", msg)) } } diff --git a/internal/game/act_farm.go b/internal/game/act_farm.go index 1e576c3..48fbec7 100644 --- a/internal/game/act_farm.go +++ b/internal/game/act_farm.go @@ -242,43 +242,68 @@ func (g *Game) advanceHarvest(sess *net.Session, p *player.Player) { bonus := p.Level(player.Farming) / 20 yield += bonus - freeSlots := p.FreeSlots() - if yield > freeSlots { - productDef, _ := g.ItemStore.Load(product) - if productDef == nil || !productDef.Stackable { + productDef, _ := g.ItemStore.Load(product) + stackable := productDef != nil && productDef.Stackable + + // The delivery target is chosen before the crop is consumed: a harvest + // that cannot be stored must leave the patch ready instead of being + // silently destroyed. A stackable product merges into an existing stack + // and therefore needs no free slot. + mergeSlot := -1 + freeSlot := -1 + noRoom := false + if stackable { + quality, maxQuality := stackProps(productDef) + for i := 0; i < 28; i++ { + s := p.InvSlot(i) + // Only a stack with the properties a fresh harvest stack gets + // may absorb it; merging into a differently qualified stack + // would silently destroy one of the two qualities. + if s != nil && s.ItemID == product && s.Quality == quality && s.MaxQuality == maxQuality { + mergeSlot = i + break + } + } + if mergeSlot < 0 { + freeSlot = p.FirstFreeSlot() + } + noRoom = mergeSlot < 0 && freeSlot < 0 + } else { + // One slot per unit: the clamp and the placement loop must count + // slots the same way (a nil 0..27 slot). + freeSlots := 0 + for i := 0; i < 28; i++ { + if p.InvSlot(i) == nil { + freeSlots++ + } + } + if yield > freeSlots { yield = freeSlots } + noRoom = yield <= 0 } - if yield <= 0 { + if noRoom { sess.WriteLine("Your inventory is too full!") p.Action = nil return } - productDef, _ := g.ItemStore.Load(product) - if productDef != nil && productDef.Stackable { - added := 0 - for i := 0; i < 28 && added < yield; i++ { - slot := p.InvSlot(i) - if slot != nil && slot.ItemID == product { - slot.Quantity += yield - added = yield - break - } - } - if added == 0 { - freeSlot := p.FirstFreeSlot() - if freeSlot >= 0 { - p.SetInvSlot(freeSlot, &player.InventorySlot{ItemID: product, Quantity: yield}) - } + placed := yield + if stackable { + if mergeSlot >= 0 { + p.InvSlot(mergeSlot).Quantity += yield + } else { + p.SetInvSlot(freeSlot, g.newInventorySlot(product, yield)) } } else { - for i := 0; i < yield; i++ { - freeSlot := p.FirstFreeSlot() - if freeSlot < 0 { + placed = 0 + for placed < yield { + fs := p.FirstFreeSlot() + if fs < 0 { break } - p.SetInvSlot(freeSlot, &player.InventorySlot{ItemID: product, Quantity: 1}) + p.SetInvSlot(fs, g.newInventorySlot(product, 1)) + placed++ } } @@ -302,7 +327,7 @@ func (g *Game) advanceHarvest(sess *net.Session, p *player.Player) { productName = g.itemColorize(sess, productDef, productDef.Name) } patchName := patchDisplayName(prefix) - msg := fmt.Sprintf("You harvest %d %s from the %s.", yield, productName, patchName) + msg := fmt.Sprintf("You harvest %d %s from the %s.", placed, productName, patchName) if int(xp) > 0 && p.OptionBool("xp_drops") { msg += g.formatXpDropSingle(sess, p, player.Farming, int(xp)) } diff --git a/internal/game/act_fletch.go b/internal/game/act_fletch.go index 9796bda..d4adff0 100644 --- a/internal/game/act_fletch.go +++ b/internal/game/act_fletch.go @@ -32,6 +32,7 @@ func (g *Game) doInstantFletch(sess *net.Session, p *player.Player, def *item.It batches := 0 totalOutput := 0 + failures := 0 skill := player.SkillName(c.EffectiveSkill()) tmpItem := &item.ItemDef{Craft: item.CraftList{*c}} inventoryFull := false @@ -39,24 +40,33 @@ func (g *Game) doInstantFletch(sess *net.Session, p *player.Player, def *item.It if !g.canDoFletchItem(p, tmpItem) { break } - // A full inventory must never destroy materials: check placement - // BEFORE consuming the batch. - if !craftOutputFits(p, def.ID, def) { + // A full inventory must never destroy materials: check placement of + // BOTH the output and any fail replacement BEFORE consuming. + if !craftOutputFits(p, def.ID, def) || !g.craftFailFits(p, c) { inventoryFull = true break } + byproducts := g.collectByproducts(p, def) + if !consumeIngredients(c, p.CountItem, p.RemoveItem) { break } - placeCraftOutput(p, def.ID, def, outputQty) - if c.XP > 0 { - g.awardSkillXP(sess, p, skill, c.XP) - } + if craftRollSuccess(c, p.Level(skill)) { + placeCraftOutput(p, def.ID, def, outputQty) + g.placeCraftByproducts(p, byproducts) - batches++ - totalOutput += outputQty + if c.XP > 0 { + g.awardSkillXP(sess, p, skill, c.XP) + } + + batches++ + totalOutput += outputQty + } else { + g.placeCraftFail(p, c) + failures++ + } if count > 0 { count-- @@ -69,6 +79,8 @@ func (g *Game) doInstantFletch(sess *net.Session, p *player.Player, def *item.It if batches == 0 { if inventoryFull { sess.WriteLine("Your inventory is too full!") + } else if failures > 0 { + sess.WriteLine("You fail to fletch anything.") } else { sess.WriteLine("You don't have the materials for that.") } @@ -153,38 +165,52 @@ func (g *Game) advanceFletch(sess *net.Session, p *player.Player) { outputQty = 1 } - // A full inventory must never destroy materials: verify the output can - // be placed BEFORE consuming the batch. - if !craftOutputFits(p, itemID, def) { + // A full inventory must never destroy materials: verify BOTH the output + // and any fail replacement can be placed BEFORE consuming the batch. + if !craftOutputFits(p, itemID, def) || !g.craftFailFits(p, c) { sess.WriteLine("Your inventory is too full!") g.cancelBgAction(p) return } skill := player.SkillName(c.EffectiveSkill()) + byproducts := g.collectByproducts(p, def) + hasItem := craftConsumedHasItem(c, p.CountItem, p.HasItem) if !consumeIngredients(c, p.CountItem, p.RemoveItem) { g.cancelBgAction(p) return } - if !placeCraftOutput(p, itemID, def, outputQty) { - // Defensive: the fits check above should have guaranteed a stack - // or slot. - sess.WriteLine("Your inventory is too full!") - g.cancelBgAction(p) - return - } - if c.XP > 0 { - g.awardSkillXP(sess, p, skill, c.XP) - } - g.saveCharacter(p) + if craftRollSuccess(c, p.Level(skill)) { + if !placeCraftOutput(p, itemID, def, outputQty) { + // Defensive: the fits check above should have guaranteed a + // stack or slot. + sess.WriteLine("Your inventory is too full!") + g.cancelBgAction(p) + return + } + g.placeCraftByproducts(p, byproducts) - msg := g.resolveCraftMessage(sess, c.SuccessMessage, "You fletch a %n.", c, def.ID, nil, p.HasItem) - if p.OptionBool("xp_drops") && c.XP > 0 { - msg += g.formatXpDropSingle(sess, p, skill, c.XP) + if c.XP > 0 { + g.awardSkillXP(sess, p, skill, c.XP) + } + g.saveCharacter(p) + + msg := g.resolveCraftMessage(sess, c.SuccessMessage, "You fletch a %n.", c, def.ID, byproducts, hasItem) + if p.OptionBool("xp_drops") && c.XP > 0 { + msg += g.formatXpDropSingle(sess, p, skill, c.XP) + } + sess.WriteLine(msg) + } else { + g.placeCraftFail(p, c) + g.saveCharacter(p) + + msg := g.resolveCraftMessage(sess, c.FailMessage, "", c, def.ID, nil, hasItem) + if msg != "" { + sess.WriteLine(g.colorize(sess, "damage_taken", msg)) + } } - sess.WriteLine(msg) if remaining > 0 { remaining-- diff --git a/internal/game/casino.go b/internal/game/casino.go index 133eb34..bc702e1 100644 --- a/internal/game/casino.go +++ b/internal/game/casino.go @@ -124,13 +124,15 @@ func (w casinoWallet) payCreditsFlagged(amount int, refund bool, stakeIDs []stri } w.p.Credits = creditsAfter // The character file is the only durable record, so it must also - // carry the resolution evidence for this player's open stakes: a - // crash before the games' own ResolveStakes call would otherwise - // leave the wager records open with checkpoint balances that no - // longer match, and recovery would refund chip stakes the paid - // settlement had already consumed. The single save below is the - // commit point for the payout AND the resolution. - resolved := w.recordStakeEvidence() + // carry the resolution evidence for the stakes this settlement + // resolves: a crash before the games' own ResolveStakes call would + // otherwise leave the wager records open with checkpoint balances + // that no longer match, and recovery would refund chip stakes the + // paid settlement had already consumed. Only the settled stakes are + // recorded (see recordStakeEvidence), so a sibling hand's stake stays + // refundable. The single save below is the commit point for the + // payout AND the resolution. + resolved := w.recordStakeEvidence(stakeIDs) if serr := w.g.AccountStore.SaveCharacter(w.p); serr != nil { w.p.Credits = creditsBefore if resolved > 0 { @@ -218,15 +220,25 @@ func (w casinoWallet) ResolveStake(stakeID string) { w.recordResolutionEvidence(unresolved) } -// recordStakeEvidence adds every currently open stake of the player to the +// recordStakeEvidence adds the stakes the current settlement resolves to the // file-side resolution evidence list and returns how many IDs were newly // recorded. Used when the economy journal is unavailable: the open wager // record IDs in the character file prove the round resolved even though no // settled marker (or payout record) could ever be journaled. The caller // must persist the character file for the evidence to count. -func (w casinoWallet) recordStakeEvidence() int { +// +// stakeIDs are the stakes the payout settles (a table hand lists its own +// hand's stakes). Only those are recorded: a sibling hand's stake is settled +// by its own payout, so marking it resolved here would suppress the refund a +// crash before that payout must still produce. An empty list (a slots payout +// links no stake) settles the round that consumed the stake, so every open +// stake of the player is evidence. +func (w casinoWallet) recordStakeEvidence(stakeIDs []string) int { added := 0 for _, rec := range w.g.Economy.openStakes(economyKey(w.p)) { + if len(stakeIDs) > 0 && !containsString(stakeIDs, rec.ID) { + continue + } if !resolvedStakeRecorded(w.p, rec.ID) { w.p.ResolvedStakes = append(w.p.ResolvedStakes, rec.ID) added++ @@ -315,11 +327,15 @@ func (g *Game) leaveCasinoGames(p *player.Player) bool { if table != nil { g.emitCasinoEvents(g.Casino.Leave(table.RoomID, table.Config.ID, p.Name)) } else if machine, ok := g.Casino.MachineFor(p.Name); ok { + // Stop autobet first, then take the machine's own leave path: a + // machine between spins has no committed stake, so one quit must + // free it (and must not warn about a forfeit that cannot happen). + // A machine mid-spin refuses the leave, the player stays seated, + // and the caller warns about the stake that is really committed. if events, stopped := g.Casino.StopMachineAutoBet(machine.RoomID, machine.Config.ID, p.Name); stopped { g.emitCasinoEvents(events) - } else { - g.emitCasinoEvents(g.Casino.LeaveMachine(machine.RoomID, machine.Config.ID, p.Name)) } + g.emitCasinoEvents(g.Casino.LeaveMachine(machine.RoomID, machine.Config.ID, p.Name)) } _, stillPlaying := g.Casino.Participation(p.Name) return !stillPlaying @@ -337,6 +353,11 @@ func (g *Game) inCasinoGame(name string) bool { return playing } +// casinoTable returns the room's table for the named game. A room addresses +// at most one table (or machine) per game name — "play blackjack" carries no +// table id — so a second table for the same game would be unreachable +// content; ValidateRoomConfigs rejects that configuration instead of silently +// dropping it. func (g *Game) casinoTable(roomID int, gameName string) (*casino.Table, bool) { room, err := g.World.LoadRoom(roomID) if err != nil { @@ -357,6 +378,10 @@ func (g *Game) casinoTable(roomID int, gameName string) (*casino.Table, bool) { return nil, false } +// casinoMachine returns the room's machine for the named game or variant. +// Like casinoTable, only the first match is reachable, which is why +// ValidateRoomConfigs rejects a room with two entries addressing the same +// game name or variant. func (g *Game) casinoMachine(roomID int, gameName string) (*casino.Machine, bool) { room, err := g.World.LoadRoom(roomID) if err != nil { diff --git a/internal/game/cmd_bank.go b/internal/game/cmd_bank.go index 6326700..468b173 100644 --- a/internal/game/cmd_bank.go +++ b/internal/game/cmd_bank.go @@ -2,6 +2,7 @@ package game import ( "fmt" + "log" "sort" "strconv" "strings" @@ -368,9 +369,13 @@ func (g *Game) doBankDepositCredits(sess *net.Session, parts []string) { sess.WriteLine("You don't have that many credits.") return } + creditsBefore, bankedBefore := p.Credits, p.BankedCredits p.Credits -= amount p.BankedCredits += amount - g.saveCharacter(p) + if !g.persistBankCredits(p, creditsBefore, bankedBefore) { + sess.WriteLine("The bank terminal could not record the transfer; nothing was moved.") + return + } sess.WriteLine(g.colorize(sess, "currency_pickup", fmt.Sprintf("You deposit %s credits. (Banked: %s)", formatInt(amount), formatInt(p.BankedCredits)))) g.showBankBrowse(sess) } @@ -396,13 +401,33 @@ func (g *Game) doBankWithdrawCredits(sess *net.Session, parts []string) { sess.WriteLine("Your wallet cannot hold that many credits.") return } + creditsBefore, bankedBefore := p.Credits, p.BankedCredits p.BankedCredits -= amount p.Credits += amount - g.saveCharacter(p) + if !g.persistBankCredits(p, creditsBefore, bankedBefore) { + sess.WriteLine("The bank terminal could not record the transfer; nothing was moved.") + return + } sess.WriteLine(g.colorize(sess, "currency_pickup", fmt.Sprintf("You withdraw %s credits.", formatInt(amount)))) g.showBankBrowse(sess) } +// persistBankCredits makes a credit move between the on-hand wallet and the +// bank durable, reverting it in memory when the save fails. A best-effort +// save here would leave the in-memory balance ahead of the durable file, and +// a wager taken from that phantom balance journals checkpoints the file can +// never match — recovery then leaves the stake permanently ambiguous instead +// of refunding or settling it. Reports whether the move may be announced. +func (g *Game) persistBankCredits(p *player.Player, creditsBefore, bankedBefore int) bool { + if err := g.AccountStore.SaveCharacter(p); err != nil { + log.Printf("bank credit transfer %s: %v", p.Name, err) + p.Credits = creditsBefore + p.BankedCredits = bankedBefore + return false + } + return true +} + func (g *Game) canUseBank(p *player.Player) bool { instances := g.World.FindObjInstances(p.RoomID, "bank booth") return len(instances) > 0 diff --git a/internal/game/cmd_color.go b/internal/game/cmd_color.go index 614583f..104d359 100644 --- a/internal/game/cmd_color.go +++ b/internal/game/cmd_color.go @@ -67,6 +67,12 @@ func (g *Game) doColor(sess *net.Session, input string) { } func (g *Game) saveAccountColor(sess *net.Session, target, value string) { + // The account can be gone (purged or republished from another session) + // while the player is still live; there is nothing to persist to then. + if sess.Account == nil { + sess.WriteLine("You must be logged into an account to set colors.") + return + } if value == "" { if sess.Account.Colors != nil { delete(sess.Account.Colors, target) diff --git a/internal/game/cmd_craft.go b/internal/game/cmd_craft.go index fe52511..a1e0c38 100644 --- a/internal/game/cmd_craft.go +++ b/internal/game/cmd_craft.go @@ -40,7 +40,7 @@ func (g *Game) doStationSkill(sess *net.Session, input string, recipeType string } if input != "" { - g.doWithInput(sess, p, filtered, input, skill, flagKey, label, verbCap) + g.doWithInput(sess, p, filtered, input, recipeType, skill, flagKey, label, verbCap) return } @@ -76,7 +76,7 @@ func (g *Game) doStationSkill(sess *net.Session, input string, recipeType string g.showProductionTable(sess, p, available, label, recipeType, flagKey, verbCap, false) } -func (g *Game) doWithInput(sess *net.Session, p *player.Player, items []*item.ItemDef, input string, skill player.SkillName, flagKey, label, verbCap string) { +func (g *Game) doWithInput(sess *net.Session, p *player.Player, items []*item.ItemDef, input, recipeType string, skill player.SkillName, flagKey, label, verbCap string) { qty, productName := parseQty(input) productName = strings.TrimSpace(productName) @@ -109,7 +109,11 @@ func (g *Game) doWithInput(sess *net.Session, p *player.Player, items []*item.It return } - g.showProductionTable(sess, p, available, label, label, flagKey, verbCap, false) + // The menu stores its skill as PendingSkill and handleProductChoice + // looks it up with player.SkillName: pass the recipe type ("crafting", + // "pharmacy", "construction"), exactly like the no-input path, never the + // capitalised display label (a map miss that resolves to level 1). + g.showProductionTable(sess, p, available, label, recipeType, flagKey, verbCap, false) } func (g *Game) itemVisible(p *player.Player, def *item.ItemDef) bool { diff --git a/internal/game/cmd_get.go b/internal/game/cmd_get.go index bc48394..a2ad137 100644 --- a/internal/game/cmd_get.go +++ b/internal/game/cmd_get.go @@ -4,6 +4,7 @@ import ( "fmt" "strings" + "thehouseoficarus/internal/casino" "thehouseoficarus/internal/net" "thehouseoficarus/internal/player" ) @@ -74,19 +75,21 @@ func (g *Game) doGet(sess *net.Session, input string) { // doGetAllNamed). See TestPickupMergesIntoFullInventory. if def != nil && def.Stackable { + quality, maxQuality := g.pickedStackProps(itemID) for i := 0; i < 28; i++ { slot := p.InvSlot(i) - if slot != nil && slot.ItemID == itemID { - removed, ok := g.World.RemoveReservedItem(p.RoomID, itemID, qty, p.Name) - if !ok { - sess.WriteLine("That's not yours!") - return - } - slot.Quantity += removed - g.saveCharacter(p) - sess.WriteLine(fmt.Sprintf("You pick up %d x %s. (now %d)", qty, g.itemColorize(sess, def, def.Name), slot.Quantity)) + if slot == nil || slot.ItemID != itemID || slot.Quality != quality || slot.MaxQuality != maxQuality { + continue + } + removed, ok := g.World.RemoveReservedItem(p.RoomID, itemID, qty, p.Name) + if !ok { + sess.WriteLine("That's not yours!") return } + slot.Quantity += removed + g.saveCharacter(p) + sess.WriteLine(fmt.Sprintf("You pick up %d x %s. (now %d)", qty, g.itemColorize(sess, def, def.Name), slot.Quantity)) + return } freeSlot := p.FirstFreeSlot() if freeSlot == -1 { @@ -116,7 +119,7 @@ func (g *Game) doGet(sess *net.Session, input string) { if _, ok := g.World.RemoveReservedItem(p.RoomID, itemID, 1, p.Name); !ok { break } - p.SetInvSlot(fs, &player.InventorySlot{ItemID: itemID, Quantity: 1}) + p.SetInvSlot(fs, g.newInventorySlot(itemID, 1)) picked++ } @@ -192,19 +195,21 @@ func (g *Game) doGetAllNamed(sess *net.Session, input string) { coloredName := g.itemColorize(sess, def, name) if def != nil && def.Stackable { + quality, maxQuality := g.pickedStackProps(itemID) for i := 0; i < 28; i++ { slot := p.InvSlot(i) - if slot != nil && slot.ItemID == itemID { - removed, ok := g.World.RemoveReservedItem(p.RoomID, itemID, wanted, p.Name) - if !ok { - sess.WriteLine("That's not yours!") - return - } - slot.Quantity += removed - g.saveCharacter(p) - sess.WriteLine(fmt.Sprintf("You pick up %d x %s. (now %d)", removed, coloredName, slot.Quantity)) + if slot == nil || slot.ItemID != itemID || slot.Quality != quality || slot.MaxQuality != maxQuality { + continue + } + removed, ok := g.World.RemoveReservedItem(p.RoomID, itemID, wanted, p.Name) + if !ok { + sess.WriteLine("That's not yours!") return } + slot.Quantity += removed + g.saveCharacter(p) + sess.WriteLine(fmt.Sprintf("You pick up %d x %s. (now %d)", removed, coloredName, slot.Quantity)) + return } freeSlot := p.FirstFreeSlot() if freeSlot == -1 { @@ -216,7 +221,7 @@ func (g *Game) doGetAllNamed(sess *net.Session, input string) { sess.WriteLine("That's not yours!") return } - p.SetInvSlot(freeSlot, &player.InventorySlot{ItemID: itemID, Quantity: removed}) + p.SetInvSlot(freeSlot, g.newInventorySlot(itemID, removed)) g.saveCharacter(p) if removed == 1 { sess.WriteLine(fmt.Sprintf("You pick up a %s.", coloredName)) @@ -236,7 +241,7 @@ func (g *Game) doGetAllNamed(sess *net.Session, input string) { if !ok { break } - p.SetInvSlot(fs, &player.InventorySlot{ItemID: itemID, Quantity: 1}) + p.SetInvSlot(fs, g.newInventorySlot(itemID, 1)) picked++ } @@ -265,15 +270,24 @@ func (g *Game) doGetAll(sess *net.Session) { } var picked []string + creditsCapped := false for itemID, qty := range ground { if qty <= 0 { continue } if itemID == "credits" { - // Same capped-removal rule as pickupCredits: the request is a - // sentinel above any possible stock, not an exact quantity. - taken := g.World.RemoveAvailableItem(p.RoomID, itemID, 999999, p.Name) + // Same capped-removal rule as pickupCredits: only the balance + // headroom is taken, so credits at the cap stay on the ground + // instead of being destroyed by the clamped grant. + room := creditHeadroom(p) + taken := 0 + if room > 0 { + taken = g.World.RemoveAvailableItem(p.RoomID, itemID, room, p.Name) + } if taken <= 0 { + if room <= 0 { + creditsCapped = true + } continue } g.grantCredits(p, taken) @@ -289,21 +303,23 @@ func (g *Game) doGetAll(sess *net.Session) { for qty > 0 { if def != nil && def.Stackable { stacked := false + quality, maxQuality := g.pickedStackProps(itemID) for i := 0; i < 28; i++ { slot := p.InvSlot(i) - if slot != nil && slot.ItemID == itemID { - _, ok := g.World.RemoveReservedItem(p.RoomID, itemID, qty, p.Name) - if !ok { - qty = 0 - stacked = true - break - } - slot.Quantity += qty - picked = append(picked, fmt.Sprintf("%s (now %d)", g.itemColorize(sess, def, def.Name), slot.Quantity)) - stacked = true + if slot == nil || slot.ItemID != itemID || slot.Quality != quality || slot.MaxQuality != maxQuality { + continue + } + _, ok := g.World.RemoveReservedItem(p.RoomID, itemID, qty, p.Name) + if !ok { qty = 0 + stacked = true break } + slot.Quantity += qty + picked = append(picked, fmt.Sprintf("%s (now %d)", g.itemColorize(sess, def, def.Name), slot.Quantity)) + stacked = true + qty = 0 + break } if stacked { break @@ -349,7 +365,7 @@ func (g *Game) doGetAll(sess *net.Session) { if !ok { break } - p.SetInvSlot(freeSlot, &player.InventorySlot{ItemID: itemID, Quantity: take}) + p.SetInvSlot(freeSlot, g.newInventorySlot(itemID, take)) name := itemID if def != nil { name = def.Name @@ -361,20 +377,28 @@ func (g *Game) doGetAll(sess *net.Session) { g.saveCharacter(p) - if len(picked) == 0 { - sess.WriteLine("Your inventory is full.") - } else { + if creditsCapped { + sess.WriteLine(g.colorize(sess, "currency_pickup", "Your purse can't hold any more credits.")) + } + switch { + case len(picked) > 0: sess.Write("You picked up: ") formatPickupList(sess, picked) + case !creditsCapped: + sess.WriteLine("Your inventory is full.") } } func (g *Game) pickupCredits(sess *net.Session, p *player.Player, itemID string) { - // Take whatever credits are accessible: a sentinel quantity far above - // any possible stock. RemoveReservedItem is all-or-nothing and would - // reject a request larger than the stack; RemoveAvailableItem caps the - // removal at what is actually on the ground. - qty := g.World.RemoveAvailableItem(p.RoomID, itemID, 999999, p.Name) + // The balance cap is a hard limit: credits the purse cannot absorb must + // stay on the ground instead of being removed and then clamped away by + // grantCredits. Only the headroom below the cap is taken. + room := creditHeadroom(p) + if room <= 0 { + sess.WriteLine(g.colorize(sess, "currency_pickup", "Your purse can't hold any more credits.")) + return + } + qty := g.World.RemoveAvailableItem(p.RoomID, itemID, room, p.Name) if qty <= 0 { return } @@ -387,6 +411,28 @@ func (g *Game) pickupCredits(sess *net.Session, p *player.Player, itemID string) } } +// creditHeadroom is how many credits p's balance can still absorb before the +// economy cap (casino.MaxCredits) is reached. +func creditHeadroom(p *player.Player) int { + room := casino.MaxCredits - p.Credits + if room < 0 { + return 0 + } + return room +} + +// pickedStackProps returns the Quality and MaxQuality a freshly picked-up +// stack receives (newInventorySlot stamps full quality onto quality-bearing +// items). A pickup only merges into a slot with matching properties, since +// merging a differently qualified stack would silently destroy the picked-up +// stack's quality - the same rule withdrawBankItem applies in cmd_bank.go. +// It reads the rule through stackProps so every other stack-creation site +// (craft, harvest, ammo removal) agrees on the same values. +func (g *Game) pickedStackProps(itemID string) (quality, maxQuality int) { + def, _ := g.ItemStore.Load(itemID) + return stackProps(def) +} + func (g *Game) collectMatches(input string, each func(func(itemID string, slot int) bool)) []itemMatch { var matches []itemMatch each(func(itemID string, slot int) bool { diff --git a/internal/game/cmd_option.go b/internal/game/cmd_option.go index 294ed5e..1bfd1b5 100644 --- a/internal/game/cmd_option.go +++ b/internal/game/cmd_option.go @@ -85,24 +85,28 @@ func (g *Game) doOption(sess *net.Session, input string) { p.Options["card_size"] = 5 } - if sess.Account.Options == nil { - sess.Account.Options = make(map[string]any) - } - sess.Account.Options[def.Name] = parsed - if def.Name == "unicode" && parsed == false { - sess.Account.Options["card_size"] = 5 - } - - acc, err := g.AccountStore.LoadAccount(sess.Account.Name) - if err == nil { - if acc.Options == nil { - acc.Options = make(map[string]any) + // The account can be gone (purged or republished from another session) + // while the player is still live: the option is applied to the player + // either way, but the account update needs a valid account. + if sess.Account != nil { + if sess.Account.Options == nil { + sess.Account.Options = make(map[string]any) } - acc.Options[def.Name] = parsed + sess.Account.Options[def.Name] = parsed if def.Name == "unicode" && parsed == false { - acc.Options["card_size"] = 5 + sess.Account.Options["card_size"] = 5 + } + + if acc, err := g.AccountStore.LoadAccount(sess.Account.Name); err == nil { + if acc.Options == nil { + acc.Options = make(map[string]any) + } + acc.Options[def.Name] = parsed + if def.Name == "unicode" && parsed == false { + acc.Options["card_size"] = 5 + } + g.saveAccount(acc) } - g.saveAccount(acc) } sess.WriteLine(fmt.Sprintf("%s set to %s.", def.Name, formatOptionValue(p, def))) } diff --git a/internal/game/cmd_quit.go b/internal/game/cmd_quit.go index f8075c1..bf46748 100644 --- a/internal/game/cmd_quit.go +++ b/internal/game/cmd_quit.go @@ -30,7 +30,14 @@ func (g *Game) doQuit(sess *net.Session) { // exits never forfeit a stake or reveal cards mid-hand. Forfeit-on-leave // stays reserved for genuine disconnects and "quit now". if !g.leaveCasinoGames(p) { - sess.WriteLine(g.colorize(sess, "warning", "Type \"quit now\" to log out anyway — your bet will be forfeited.")) + // A committed wager blocks the departure; whether it is actually lost + // depends on whether the game has dealt/started a round for it. A + // stake the game has not dealt for yet is refunded on disconnect + // (baccarat always, blackjack while the betting window is open), and + // a dealt hand or a spin in progress is forfeited. The refusal + // therefore states both outcomes instead of promising a forfeit that + // an undealt stake does not suffer. + sess.WriteLine(g.colorize(sess, "warning", "Type \"quit now\" to log out anyway — a bet still awaiting the deal is refunded; a hand or spin in progress is forfeited.")) return } @@ -86,6 +93,13 @@ func (g *Game) beginQuitCountdown(sess *net.Session) { g.charsMu.Lock() delete(g.loggedInChars, p.Name) g.charsMu.Unlock() + // The character is leaving the world: drop its queued free/ + // active/consume commands with the registry entry, or they + // retain this session and replay on a later login as the same + // character. + if g.queue != nil { + g.queue.ClearPlayer(p.Name) + } if g.Hub != nil { g.Hub.LeaveRoom(sess) } diff --git a/internal/game/cmd_remove.go b/internal/game/cmd_remove.go index 349617d..5bc02a7 100644 --- a/internal/game/cmd_remove.go +++ b/internal/game/cmd_remove.go @@ -97,9 +97,12 @@ func (g *Game) removeAmmo(sess *net.Session, p *player.Player, qty int) { removeQty = qty } + quality, maxQuality := g.pickedStackProps(ammoID) for i := 0; i < 28; i++ { s := p.InvSlot(i) - if s != nil && s.ItemID == ammoID { + // Only a stack with the properties a fresh stack gets may absorb the + // equipped stack (see pickedStackProps). + if s != nil && s.ItemID == ammoID && s.Quality == quality && s.MaxQuality == maxQuality { s.Quantity += removeQty p.AmmoQty -= removeQty if p.AmmoQty <= 0 { @@ -123,7 +126,7 @@ func (g *Game) removeAmmo(sess *net.Session, p *player.Player, qty int) { return } - p.SetInvSlot(freeSlot, &player.InventorySlot{ItemID: ammoID, Quantity: removeQty}) + p.SetInvSlot(freeSlot, g.newInventorySlot(ammoID, removeQty)) p.AmmoQty -= removeQty if p.AmmoQty <= 0 { delete(p.Equipment, item.SlotAmmo) @@ -193,10 +196,17 @@ func (g *Game) doRemoveAll(sess *net.Session) { g.saveCharacter(p) } +// removeAllAmmo moves the whole equipped ammo stack into the inventory. The +// equipment entry is only cleared once the stack has been merged into an +// existing stack or placed in a free slot, so a full inventory reports a +// failure instead of silently destroying the ammo. func (g *Game) removeAllAmmo(sess *net.Session, p *player.Player, itemID string) { + quality, maxQuality := g.pickedStackProps(itemID) for i := 0; i < 28; i++ { s := p.InvSlot(i) - if s != nil && s.ItemID == itemID { + // Only a stack with the properties a fresh stack gets may absorb the + // equipped stack (see pickedStackProps). + if s != nil && s.ItemID == itemID && s.Quality == quality && s.MaxQuality == maxQuality { s.Quantity += p.AmmoQty delete(p.Equipment, item.SlotAmmo) p.AmmoQty = 0 @@ -210,9 +220,13 @@ func (g *Game) removeAllAmmo(sess *net.Session, p *player.Player, itemID string) } } freeSlot := p.FirstFreeSlot() - if freeSlot >= 0 { - p.SetInvSlot(freeSlot, &player.InventorySlot{ItemID: itemID, Quantity: p.AmmoQty}) + if freeSlot < 0 { + // Nowhere to put the equipped stack: leave it equipped and report + // nothing removed. + sess.WriteLine("Nowhere to put that!") + return } + p.SetInvSlot(freeSlot, g.newInventorySlot(itemID, p.AmmoQty)) delete(p.Equipment, item.SlotAmmo) p.AmmoQty = 0 def, _ := g.ItemStore.Load(itemID) diff --git a/internal/game/cmd_shop.go b/internal/game/cmd_shop.go index 7299ad3..f3211e0 100644 --- a/internal/game/cmd_shop.go +++ b/internal/game/cmd_shop.go @@ -366,8 +366,13 @@ func (g *Game) doShopSell(sess *net.Session, mob *world.MobInstance, input strin // shopInvCapacity reports how many of an item the player can still carry. func (g *Game) shopInvCapacity(p *player.Player, itemID string, def *item.ItemDef) int { if def != nil && def.Stackable { + quality, maxQuality := stackProps(def) for i := 0; i < 28; i++ { - if slot := p.InvSlot(i); slot != nil && slot.ItemID == itemID { + // Capacity must agree with giveShopItems: only a stack with the + // properties a fresh stack gets can absorb a purchase (see + // stackProps). Counting a differently qualified stack here would + // let the caller charge for items the delivery path cannot place. + if slot := p.InvSlot(i); slot != nil && slot.ItemID == itemID && slot.Quality == quality && slot.MaxQuality == maxQuality { return 1 << 30 } } @@ -383,8 +388,13 @@ func (g *Game) shopInvCapacity(p *player.Player, itemID string, def *item.ItemDe // possible. Callers must ensure capacity via shopInvCapacity first. func (g *Game) giveShopItems(p *player.Player, itemID string, qty int, def *item.ItemDef) { if def != nil && def.Stackable { + // Merge only into a stack with the properties this purchase would + // itself receive (stackProps/newInventorySlot), matching craft, + // harvest, ammo return and pickup: a differently qualified stack is + // not a merge target. + quality, maxQuality := stackProps(def) for i := 0; i < 28; i++ { - if slot := p.InvSlot(i); slot != nil && slot.ItemID == itemID { + if slot := p.InvSlot(i); slot != nil && slot.ItemID == itemID && slot.Quality == quality && slot.MaxQuality == maxQuality { slot.Quantity += qty return } diff --git a/internal/game/cmd_swapid.go b/internal/game/cmd_swapid.go index bbcc79c..ef862b9 100644 --- a/internal/game/cmd_swapid.go +++ b/internal/game/cmd_swapid.go @@ -76,26 +76,35 @@ func (g *Game) executeSwapID(sess *net.Session, args []string, rawInput string) } } - // Swap the two files first. After this, path1 holds the old room id2's - // content and path2 holds the old room id1's content. Rebuilding the index - // then makes LoadRoom(id1)/LoadRoom(id2) return the swapped content, so the - // reference rewrite below operates on the correctly-positioned rooms and - // re-marshaling writes an `id:` field matching the new filename (keeping - // the file's id field consistent with its name, as the old regex did). - tempPath := path1 + ".swapid-tmp" - if rerr := os.Rename(path1, tempPath); rerr != nil { - sess.WriteLine(fmt.Sprintf("Error renaming room %d: %v", id1, rerr)) + // Swap the two files' contents first. After this, path1 holds the old room + // id2's content and path2 holds the old room id1's content. Rebuilding the + // index then makes LoadRoom(id1)/LoadRoom(id2) return the swapped content, + // so the reference rewrite below operates on the correctly-positioned rooms + // and re-marshaling writes an `id:` field matching the new filename. The + // swap runs as one journal transaction (like handleRoomRename/the account + // lifecycle), so a crash mid-swap is rolled back at the next startup + // instead of leaving both rooms at wrong paths with no recovery record. + data1, rerr := os.ReadFile(path1) + if rerr != nil { + sess.WriteLine(fmt.Sprintf("Error reading room %d: %v", id1, rerr)) return } - if rerr := os.Rename(path2, path1); rerr != nil { - os.Rename(tempPath, path1) - sess.WriteLine(fmt.Sprintf("Error renaming room %d: %v", id2, rerr)) + data2, rerr := os.ReadFile(path2) + if rerr != nil { + sess.WriteLine(fmt.Sprintf("Error reading room %d: %v", id2, rerr)) return } - if rerr := os.Rename(tempPath, path2); rerr != nil { - os.Rename(path2, path1) - os.Rename(tempPath, path1) - sess.WriteLine(fmt.Sprintf("Error finishing rename: %v", rerr)) + txn := g.newTxn() + if rerr := txn.Replace(path1, data2, 0644); rerr != nil { + sess.WriteLine(fmt.Sprintf("Error swapping rooms: %v", rerr)) + return + } + if rerr := txn.Replace(path2, data1, 0644); rerr != nil { + sess.WriteLine(fmt.Sprintf("Error swapping rooms: %v", rerr)) + return + } + if rerr := txn.Commit(); rerr != nil { + sess.WriteLine(fmt.Sprintf("Error swapping rooms: %v", rerr)) return } diff --git a/internal/game/cmd_wear.go b/internal/game/cmd_wear.go index a02fd3b..5a293de 100644 --- a/internal/game/cmd_wear.go +++ b/internal/game/cmd_wear.go @@ -101,7 +101,7 @@ func (g *Game) doWear(sess *net.Session, input string) { sess.WriteLine("Your inventory is full.") return } - p.SetInvSlot(freeSlot, &player.InventorySlot{ItemID: existingItemID, Quantity: 1}) + p.SetInvSlot(freeSlot, g.displacedEquipSlot(p, existingItemID)) } if slot.Quantity > 1 { @@ -127,6 +127,24 @@ func (g *Game) doWear(sess *net.Session, input string) { sess.WriteLine(fmt.Sprintf("You %s %s.", verb, g.itemColorize(sess, def, match.Name))) } +// displacedEquipSlot builds the inventory slot that receives an item +// displaced by a swap. The item's equipped quality (EquipQuality, kept +// because the equipment entry alone cannot carry a charge) is stamped onto +// the returned stack and cleared from the map, so the returned item keeps +// merging and charging like one carried normally; leaving the entry behind +// would let act_burn charge a stale record or a zeroed slot. +func (g *Game) displacedEquipSlot(p *player.Player, itemID string) *player.InventorySlot { + slot := &player.InventorySlot{ItemID: itemID, Quantity: 1} + if q, ok := p.EquipQuality[itemID]; ok { + slot.Quality = q + if def, _ := g.ItemStore.Load(itemID); def != nil { + slot.MaxQuality = def.MaxQuality + } + delete(p.EquipQuality, itemID) + } + return slot +} + func (g *Game) equipAmmo(sess *net.Session, p *player.Player, invIdx int, slot *player.InventorySlot, def *item.ItemDef, qty int) { transferQty := slot.Quantity if qty > 0 && qty < transferQty { @@ -135,7 +153,12 @@ func (g *Game) equipAmmo(sess *net.Session, p *player.Player, invIdx int, slot * existingAmmoID, hasAmmo := p.Equipment[item.SlotAmmo] if hasAmmo && existingAmmoID != slot.ItemID { - g.unequipAmmo(p) + // The equipped stack stays equipped when there is nowhere to put + // it; equipping the new stack would otherwise strand the old one. + if !g.unequipAmmo(p) { + sess.WriteLine("Your inventory is full.") + return + } } if slot.Quantity <= transferQty { @@ -156,26 +179,35 @@ func (g *Game) equipAmmo(sess *net.Session, p *player.Player, invIdx int, slot * sess.WriteLine(fmt.Sprintf("You equip %s (x%d).", displayName, p.AmmoQty)) } -func (g *Game) unequipAmmo(p *player.Player) { +// unequipAmmo returns the equipped ammo to the inventory and reports whether +// the equipped stack is now stored. With nowhere to put it the ammo stays +// equipped: the equipment entry is only cleared once the stack has actually +// been merged into an inventory stack or placed in a free slot. +func (g *Game) unequipAmmo(p *player.Player) bool { ammoID, ok := p.Equipment[item.SlotAmmo] if !ok || p.AmmoQty <= 0 { - return + return true } + quality, maxQuality := g.pickedStackProps(ammoID) for i := 0; i < 28; i++ { s := p.InvSlot(i) - if s != nil && s.ItemID == ammoID { + // Only a stack with the properties a fresh stack gets may absorb the + // equipped stack (see pickedStackProps). + if s != nil && s.ItemID == ammoID && s.Quality == quality && s.MaxQuality == maxQuality { s.Quantity += p.AmmoQty delete(p.Equipment, item.SlotAmmo) p.AmmoQty = 0 - return + return true } } freeSlot := p.FirstFreeSlot() - if freeSlot >= 0 { - p.SetInvSlot(freeSlot, &player.InventorySlot{ItemID: ammoID, Quantity: p.AmmoQty}) + if freeSlot < 0 { + return false } + p.SetInvSlot(freeSlot, g.newInventorySlot(ammoID, p.AmmoQty)) delete(p.Equipment, item.SlotAmmo) p.AmmoQty = 0 + return true } func (g *Game) doWearAll(sess *net.Session) { @@ -235,8 +267,10 @@ func (g *Game) doWearAll(sess *net.Session) { continue } existingID, hasAmmo := p.Equipment[item.SlotAmmo] - if hasAmmo && existingID != best.itemID { - g.unequipAmmo(p) + // No free slot for the equipped stack: leave it equipped and + // skip this item rather than delete the old stack. + if hasAmmo && existingID != best.itemID && !g.unequipAmmo(p) { + continue } transferQty := invSlot.Quantity p.SetInvSlot(best.invSlot, nil) @@ -256,7 +290,7 @@ func (g *Game) doWearAll(sess *net.Session) { if freeSlot == -1 { continue } - p.SetInvSlot(freeSlot, &player.InventorySlot{ItemID: existing, Quantity: 1}) + p.SetInvSlot(freeSlot, g.displacedEquipSlot(p, existing)) } invSlot := p.InvSlot(best.invSlot) diff --git a/internal/game/combat_mob.go b/internal/game/combat_mob.go index c53d847..ace02b5 100644 --- a/internal/game/combat_mob.go +++ b/internal/game/combat_mob.go @@ -213,16 +213,16 @@ func (g *Game) applyMobMiss(sess *net.Session, p *player.Player, mob *world.MobI } } -func (g *Game) endCombat(sess *net.Session, p *player.Player, mob *world.MobInstance) { - g.Combat.Leave(p.Name) - - if p.HP <= 0 { - g.killPlayer(sess, p, mob) - return - } - +// resolveMobDeath runs the shared mob-death path for a mob whose HP has +// reached zero: kill bookkeeping, the victory broadcast, drops, on-kill +// triggers, and respawn scheduling. It returns false when the mob is nil or +// still alive. Every path that can reduce a mob to zero HP — a player +// attack, a finishing blow, and Dead Man's Switch retribution — must route +// through here, or the zeroed instance lingers in MobStore, is filtered out +// of MobsInRoom (HP > 0), and never respawns. +func (g *Game) resolveMobDeath(sess *net.Session, p *player.Player, mob *world.MobInstance) bool { if mob == nil || mob.HP > 0 { - return + return false } isTask := mob.IsTask() @@ -236,6 +236,20 @@ func (g *Game) endCombat(sess *net.Session, p *player.Player, mob *world.MobInst // the player currently has that item equipped in a weapon-hand slot. g.runTrigger(sess, p, mob.OnKill, p.RoomID, p.IsWielding, false) g.scheduleMobRespawn(mob) + return true +} + +func (g *Game) endCombat(sess *net.Session, p *player.Player, mob *world.MobInstance) { + g.Combat.Leave(p.Name) + + if p.HP <= 0 { + g.killPlayer(sess, p, mob) + return + } + + if !g.resolveMobDeath(sess, p, mob) { + return + } if p.EscapeDir != "" { g.beginEscapeMove(p) @@ -360,6 +374,12 @@ func (g *Game) killPlayer(sess *net.Session, p *player.Player, mob *world.MobIns } } } + // Retribution can zero the mob: run the same death path the normal + // attack uses so it still drops loot and respawns instead of lingering + // at HP 0 and being permanently removed from the room. + if mob != nil && mob.HP <= 0 { + g.resolveMobDeath(sess, p, mob) + } p.DeactivateAllTechs() p.Stats.RecordDeath() sess.WriteLine(g.colorize(sess, "death", "Oh dear, you are dead!")) diff --git a/internal/game/core_admin.go b/internal/game/core_admin.go index e1236ab..848130f 100644 --- a/internal/game/core_admin.go +++ b/internal/game/core_admin.go @@ -2,6 +2,24 @@ package game import "thehouseoficarus/internal/net" +// checkAdmin reports whether the session belongs to an account with the +// on-disk admin flag set. The cached sess.Account copy is only refreshed on +// login or account republish, so it is NOT trusted as the source of truth: +// removing admin: true from an account file (or the admin API's reload) must +// demote an already-connected admin without requiring a relog. The account +// file is re-read on the owner goroutine, which serializes with logins and +// account saves, so the check cannot race a concurrent hash/admin change. +// Privilege checks fail closed when the account cannot be read. func (g *Game) checkAdmin(sess *net.Session) bool { - return sess.Account != nil && sess.Account.Admin + if sess.Account == nil { + return false + } + acc, err := g.AccountStore.LoadAccount(sess.Account.Name) + if err != nil || !acc.Admin { + // Drop the stale cached privilege too, so command gating and any + // code reading sess.Account.Admin agree with disk. + sess.Account.Admin = false + return false + } + return true } diff --git a/internal/game/core_command_queue.go b/internal/game/core_command_queue.go index 7afc75b..28b7a36 100644 --- a/internal/game/core_command_queue.go +++ b/internal/game/core_command_queue.go @@ -1,6 +1,7 @@ package game import ( + "sort" "sync" "time" @@ -18,8 +19,10 @@ type QueuedCommand struct { // CommandQueue manages the three per-player command queues (free, active, // consume) with proper mutex protection. Free commands stack and execute in // order; active commands replace each other (only one survives); consume -// commands are a single-slot eat/drink queue. All three are written from -// session goroutines and drained from the tick goroutine. +// commands are a single-slot eat/drink queue. Writers are the command +// handlers (owner goroutine); the consumer is ProcessQueuedCommands on the +// owner tick. The mutex is kept as a cheap guard for callers that reach a +// queue from outside that single-owner path. type CommandQueue struct { mu sync.Mutex freeQueue map[string][]QueuedCommand @@ -65,8 +68,8 @@ func (q *CommandQueue) DrainFree(name string) []QueuedCommand { return cmds } -// DrainActive returns all active commands sorted by timestamp (first-to-queue -// wins), then clears the map. +// DrainActive returns all active commands, then clears the map. It does not +// order them: ProcessQueuedCommands sorts by Timestamp (first-to-queue wins). func (q *CommandQueue) DrainActive() []QueuedCommand { q.mu.Lock() defer q.mu.Unlock() @@ -103,19 +106,37 @@ func (q *CommandQueue) DeleteActive(name string) { delete(q.activeQueue, name) } -// ActivePositions returns the sorted names of all players with active commands, -// and the position (1-indexed) of the named player among them. Used by the -// `queued` command to show queue order. +// ActivePositions returns the sorted names of every player holding an active +// command, and the position (1-indexed) of the named player among them, or 0 +// when that player has no active command. Used by the `queued` command to +// show queue order. func (q *CommandQueue) ActivePositions(name string) (names []string, pos int) { q.mu.Lock() defer q.mu.Unlock() for n := range q.activeQueue { names = append(names, n) } - // sort names (caller may want them sorted) + sort.Strings(names) + for i, n := range names { + if n == name { + return names, i + 1 + } + } return names, 0 } +// ClearPlayer drops every queued command for the player: free, active, and +// consume. Called when a session detaches (logout, quit, disconnect, or +// character removal) so leftovers cannot retain the departed session and +// player, and cannot replay against a later session for the same name. +func (q *CommandQueue) ClearPlayer(name string) { + q.mu.Lock() + defer q.mu.Unlock() + delete(q.freeQueue, name) + delete(q.activeQueue, name) + delete(q.consumeQueue, name) +} + // DrainConsume returns and removes the consume command for the player. func (q *CommandQueue) DrainConsume(name string) (*QueuedCommand, bool) { q.mu.Lock() diff --git a/internal/game/core_login_account.go b/internal/game/core_login_account.go index 921af82..a5106ea 100644 --- a/internal/game/core_login_account.go +++ b/internal/game/core_login_account.go @@ -16,6 +16,14 @@ func (g *Game) handleAccountName(sess *net.Session, input string) { sess.Write("Account name: ") return } + // Validate BEFORE any filesystem access: the name is joined into an + // account (and later character) path, so an invalid name must never + // reach FindAccount or anything downstream that reuses it. + if verr := player.ValidName(name); verr != nil { + sess.WriteLine(verr.Error()) + sess.Write("Account name: ") + return + } actualName, err := g.AccountStore.FindAccount(name) if err == nil { sess.Account = &player.Account{Name: actualName} @@ -24,11 +32,6 @@ func (g *Game) handleAccountName(sess *net.Session, input string) { sess.SetEcho(false) sess.Write("Password: ") } else { - if verr := player.ValidName(name); verr != nil { - sess.WriteLine(verr.Error()) - sess.Write("Account name: ") - return - } sess.Account = &player.Account{Name: name} sess.SetAuthName(name) sess.SetState(net.StateNewAccountPass) @@ -38,6 +41,22 @@ func (g *Game) handleAccountName(sess *net.Session, input string) { } } +// resetToAccountName returns a session that has no usable account left +// (purged, or republished out from under it) to the account-name prompt. A +// menu-state handler must call it instead of dereferencing sess.Account: +// a queued-mode Close only drops the transport after the writer drains, so +// an already-buffered input line can still be dispatched to the owner +// after the account is gone. +func (g *Game) resetToAccountName(sess *net.Session) { + sess.Account = nil + sess.PendingChar = "" + sess.PendingMenu = nil + sess.PendingPass = "" + sess.SetEcho(true) + sess.SetState(net.StateAccountName) + sess.Write("Account name: ") +} + func (g *Game) handlePassword(sess *net.Session, input string) { if input == "" { sess.Write("Password: ") @@ -171,6 +190,13 @@ func (g *Game) applyPasswordAuth(sess *net.Session, auth passwordAuth) { sess.SetState(net.StateAccountName) sess.Write("Account name: ") } + if sess.Account == nil { + // The account vanished while the password was being verified (a + // purge or republish from another session): there is nothing to + // authorize against, so start over from the account-name prompt. + g.resetToAccountName(sess) + return + } if auth.loadErr { reject("Error loading account.") return @@ -235,6 +261,10 @@ func (g *Game) handleNewPass(sess *net.Session, input string) { } func (g *Game) handleNewAccount(sess *net.Session, input string) { + if sess.Account == nil { + g.resetToAccountName(sess) + return + } input = strings.TrimSpace(input) if input == "" { sess.SetEcho(true) @@ -283,6 +313,13 @@ func (g *Game) handleNewAccountOffOwner(sess *net.Session, input string) { // applyNewAccount creates the account on the owner. The password was // already hashed by the caller (off the owner where possible). func (g *Game) applyNewAccount(sess *net.Session, hash string, hashErr error) { + if sess.Account == nil { + // The account vanished while the password was being hashed off the + // owner (a purge or republish from another session): there is nothing + // to create against, and sess.Account is dereferenced below. + g.resetToAccountName(sess) + return + } if hashErr != nil { sess.SetEcho(true) sess.WriteLine(fmt.Sprintf("Error creating account: %v", hashErr)) @@ -332,6 +369,10 @@ func (g *Game) applyNewAccount(sess *net.Session, hash string, hashErr error) { } func (g *Game) handleColorChoice(sess *net.Session, input string) { + if sess.Account == nil { + g.resetToAccountName(sess) + return + } input = strings.ToLower(strings.TrimSpace(input)) if input == "y" || input == "yes" { sess.Account.Options["color"] = "none" @@ -359,6 +400,13 @@ func menuColorMode(sess *net.Session) string { } func (g *Game) showMenu(sess *net.Session) { + if sess.Account == nil { + // The account is gone (purged or republished from another session) + // while this session was mid-flow: there is no menu to show for a + // deleted account. + g.resetToAccountName(sess) + return + } sess.SetState(net.StateMenu) mode := menuColorMode(sess) lines := []string{ @@ -387,6 +435,10 @@ func (g *Game) showMenu(sess *net.Session) { } func (g *Game) handleMenu(sess *net.Session, input string) { + if sess.Account == nil { + g.resetToAccountName(sess) + return + } switch strings.ToLower(strings.TrimSpace(input)) { case "": sess.Write("> ") @@ -483,6 +535,10 @@ func (g *Game) handleMenu(sess *net.Session, input string) { } func (g *Game) handleRenameAccount(sess *net.Session, input string) { + if sess.Account == nil { + g.resetToAccountName(sess) + return + } newName := strings.TrimSpace(input) if newName == "" { sess.Write("New account name: ") @@ -524,6 +580,10 @@ func (g *Game) handleRenameAccount(sess *net.Session, input string) { } func (g *Game) handlePurgeAccount(sess *net.Session, input string) { + if sess.Account == nil { + g.resetToAccountName(sess) + return + } input = strings.TrimSpace(input) expected := "PURGE " + sess.Account.Name if !strings.EqualFold(input, expected) { @@ -551,17 +611,19 @@ func (g *Game) handlePurgeAccount(sess *net.Session, input string) { } sess.WriteLine(fmt.Sprintf("Account %s has been purged. Thanks for playing.", sess.Account.Name)) - // Other authenticated sessions of this account are stale now; tell - // them and close them (they cannot safely keep using cached state). + // Every session authenticated to this account is stale now: the account + // file is gone, so none of them may keep a live protocol state (a menu + // handler dereferencing the cached account, or a buffered input line + // already inside the transport, would fault). Reset each to the + // account-name prompt before closing it. for _, other := range g.allSessionsOfAccount(sess.Account.Name) { if other == sess { continue } - other.Account = nil - other.PendingChar = "" - other.PendingMenu = nil other.WriteLine("\nYour account has been purged from another session.") + g.resetToAccountName(other) other.Close() } + g.resetToAccountName(sess) sess.Close() } diff --git a/internal/game/core_login_char.go b/internal/game/core_login_char.go index 0f57d3c..3a2e324 100644 --- a/internal/game/core_login_char.go +++ b/internal/game/core_login_char.go @@ -9,6 +9,10 @@ import ( ) func (g *Game) handleNewCharName(sess *net.Session, input string) { + if sess.Account == nil { + g.resetToAccountName(sess) + return + } name := strings.TrimSpace(input) if sess.PendingChar == "connect" && len(sess.Account.Characters) > 0 { @@ -62,6 +66,10 @@ func (g *Game) handleNewCharName(sess *net.Session, input string) { } func (g *Game) connectCharacter(sess *net.Session, name string) { + if sess.Account == nil { + g.resetToAccountName(sess) + return + } // This whole function runs as one owner step (invoked from session // input): membership validation, the online check, registry // publication, and room entry are serialized against every other @@ -139,6 +147,10 @@ func (g *Game) connectCharacter(sess *net.Session, name string) { } func (g *Game) handleRenameChar(sess *net.Session, input string) { + if sess.Account == nil { + g.resetToAccountName(sess) + return + } name := strings.TrimSpace(input) if idx, err := parseChoiceIndex(name); err == nil && idx > 0 && idx <= len(sess.Account.Characters) { sess.PendingChar = sess.Account.Characters[idx-1] @@ -168,13 +180,32 @@ func (g *Game) handleRenameChar(sess *net.Session, input string) { // through a live session. Menu operations that rename or delete character // files must refuse while the character is online, otherwise the in-memory // player later overwrites or resurrects the moved/removed file. +// +// The lookup is case-insensitive: loggedInChars is keyed by the canonical +// on-disk character name, but callers pass names taken from hand-editable +// account files and admin file paths ("ALICE"), and character names keep the +// case their owner chose. Character identity in this game is otherwise +// case-insensitive (see isMemberOf), so liveness must be too - an exact-match +// lookup lets an online character's file be edited or deleted. func (g *Game) characterOnline(name string) bool { g.charsMu.Lock() defer g.charsMu.Unlock() - return g.loggedInChars[name] != nil + if g.loggedInChars[name] != nil { + return true + } + for logged, sess := range g.loggedInChars { + if sess != nil && strings.EqualFold(logged, name) { + return true + } + } + return false } func (g *Game) handleRenameCharName(sess *net.Session, input string) { + if sess.Account == nil { + g.resetToAccountName(sess) + return + } newName := strings.TrimSpace(input) oldName := sess.PendingChar if newName == "" { @@ -239,6 +270,10 @@ func (g *Game) showDeleteConfirm(sess *net.Session) { } func (g *Game) handleDeleteChar(sess *net.Session, input string) { + if sess.Account == nil { + g.resetToAccountName(sess) + return + } if sess.PendingChar == "" { name := strings.TrimSpace(input) if idx, err := parseChoiceIndex(name); err == nil && idx > 0 && idx <= len(sess.Account.Characters) { diff --git a/internal/game/correct_combine_skill_test.go b/internal/game/correct_combine_skill_test.go new file mode 100644 index 0000000..d6de229 --- /dev/null +++ b/internal/game/correct_combine_skill_test.go @@ -0,0 +1,100 @@ +package game + +// Regression: combine recipes declare type "combine", which is an action and +// not a skill. Resolving the recipe type straight to a player.SkillName made +// p.Level read a map miss (level 1) for the success roll and made awardSkillXP +// persist a phantom "combine" skill entry. + +import ( + "path/filepath" + "testing" + + "thehouseoficarus/internal/item" + "thehouseoficarus/internal/net" + "thehouseoficarus/internal/player" +) + +const correctCombineYAML = `id: correct_combine_out +name: correct combine out +stackable: false +value: 5 +craft: + - type: combine + ingredients: + - items: + - correct_combine_part + quantity: 1 + xp: 7 + output_qty: 1 + ticks_per_cycle: 1 +` + +func TestCombineSkillResolvesARealSkill(t *testing.T) { + for _, tc := range []struct { + name string + typ string + want player.SkillName + }{ + {"combine is an action, not a skill", "combine", player.Crafting}, + {"empty type defaults to combine", "", player.Crafting}, + {"a declared real skill is honored", "smithing", player.Smithing}, + } { + t.Run(tc.name, func(t *testing.T) { + got := combineSkill(&item.CraftDef{Type: tc.typ}) + if got != tc.want { + t.Fatalf("combineSkill(type %q) = %q, want %q", tc.typ, got, tc.want) + } + if !player.ValidSkill(got) { + t.Fatalf("combineSkill(type %q) resolved to unknown skill %q", tc.typ, got) + } + }) + } +} + +func TestCombineXPGoesToCraftingNotAPhantomSkill(t *testing.T) { + h := newHarness(t) + h.seedAccount("alice") + p := h.seedCharacter("alice", "alice") + writeSeedFile(t, filepath.Join(h.dir, "items", "correct_combine_out.yaml"), []byte(correctCombineYAML)) + writeSeedFile(t, filepath.Join(h.dir, "items", "correct_combine_part.yaml"), + []byte("id: correct_combine_part\nname: correct combine part\nstackable: false\nvalue: 1\n")) + + conn := newChanConn() + sess := &net.Session{Conn: conn, Player: p} + sess.SetState(net.StateGame) + + startCrafting := p.Skills[player.Crafting] + h.g.RunOnOwner(func() { + h.g.ItemStore.Reload(h.dir) + p.SetInvSlot(0, &player.InventorySlot{ItemID: "correct_combine_part", Quantity: 1}) + def := mustLoadItem(t, h.g, "correct_combine_out") + h.g.startCombine(sess, p, def, def.FirstCraft(), 1) + if p.Action == nil { + t.Fatal("combine did not start") + } + h.g.advanceCombine(sess, p) // phase 0 -> 1 + h.g.advanceCombine(sess, p) // phase 1 -> 2 + h.g.advanceCombine(sess, p) // phase 2: consume + output + + if got := p.CountItem("correct_combine_out"); got != 1 { + t.Fatalf("combine output = %d, want 1", got) + } + if got := p.Skills[player.Crafting]; got != startCrafting+7 { + t.Errorf("crafting xp = %d, want %d", got, startCrafting+7) + } + if got, ok := p.Skills[player.SkillName("combine")]; ok { + t.Errorf("combine recipes persist a phantom skill: combine = %d", got) + } + }) + + saved, err := h.g.AccountStore.LoadCharacter("alice") + if err != nil { + t.Fatalf("load character: %v", err) + } + if got, ok := saved.Skills[player.SkillName("combine")]; ok { + t.Errorf("phantom combine skill was persisted: combine = %d", got) + } + if got := saved.Skills[player.Crafting]; got != startCrafting+7 { + t.Errorf("persisted crafting xp = %d, want %d", got, startCrafting+7) + } +} diff --git a/internal/game/correct_online_name_case_test.go b/internal/game/correct_online_name_case_test.go new file mode 100644 index 0000000..74c344c --- /dev/null +++ b/internal/game/correct_online_name_case_test.go @@ -0,0 +1,85 @@ +package game + +// Regression: character/account liveness must be matched case-insensitively, +// like the game's own membership check (isMemberOf). The admin raw file editor +// passes a file NAME ("players/characters/ALICE.yaml"), and an account file's +// character list is hand-editable, so an exact-match lookup both let an online +// character's file be overwritten and let an online character's account be +// purged. + +import ( + "strings" + "testing" + + "thehouseoficarus/internal/net" +) + +func TestAdminPlayerFileOnlineMatchesNamesCaseInsensitively(t *testing.T) { + h := newHarness(t) + h.seedAccount("alice") + h.seedCharacter("alice", "alice") + h.seedAccount("carol") + h.seedCharacter("carol", "carol") + + // The account file spells its character list differently from the live + // session's character name. + acc, err := h.g.AccountStore.LoadAccount("alice") + if err != nil { + t.Fatalf("load account: %v", err) + } + acc.Characters = []string{"ALICE"} + if err := h.g.AccountStore.SaveAccount(acc); err != nil { + t.Fatalf("save account: %v", err) + } + + h.connect("alice") + + cases := []struct { + name string + path string + want bool + }{ + {"online character, exact case", "players/characters/alice.yaml", true}, + {"online character, upper case", "players/characters/ALICE.yaml", true}, + {"online character, mixed case", "players/characters/Alice.yaml", true}, + {"account of an online character, list case differs", "players/accounts/alice.yaml", true}, + {"offline character, upper case", "players/characters/CAROL.yaml", false}, + {"offline account", "players/accounts/carol.yaml", false}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := h.g.AdminPlayerFileOnline(tc.path); got != tc.want { + t.Fatalf("AdminPlayerFileOnline(%q) = %v, want %v", tc.path, got, tc.want) + } + }) + } +} + +func TestPurgeRefusesCharacterListedInAnotherCase(t *testing.T) { + h := newHarness(t) + h.seedAccount("alice") + h.seedCharacter("alice", "alice") + + acc, err := h.g.AccountStore.LoadAccount("alice") + if err != nil { + t.Fatalf("load account: %v", err) + } + acc.Characters = []string{"ALICE"} + if err := h.g.AccountStore.SaveAccount(acc); err != nil { + t.Fatalf("save account: %v", err) + } + + h.connect("alice") + conn := newChanConn() + menu := &net.Session{Conn: conn, Account: acc} + menu.SetState(net.StateMenu) + + h.g.RunOnOwner(func() { h.g.handlePurgeAccount(menu, "PURGE alice") }) + + if out := string(conn.Bytes()); !strings.Contains(out, "is logged in") { + t.Fatalf("purge was not refused for a live character: %q", out) + } + if _, err := h.g.AccountStore.LoadAccount("alice"); err != nil { + t.Fatalf("account was purged while its character was online: %v", err) + } +} diff --git a/internal/game/correct_owner_panic_subscription_test.go b/internal/game/correct_owner_panic_subscription_test.go new file mode 100644 index 0000000..f535dcc --- /dev/null +++ b/internal/game/correct_owner_panic_subscription_test.go @@ -0,0 +1,85 @@ +package game + +// Regression: the owner loop contains a panic in a tick callback, so the tick +// engine no longer sees it. SubscribeTicks must therefore treat "the callback +// did not run to completion" as KEEP - a false return unsubscribes the system, +// and the master subscriber main.go registers (RunTickSystems) is what drains +// queued commands, settles casino rounds, and cleans up disconnects. Likewise +// RunAdminOnOwner must not report a contained panic as "game is shutting down". + +import ( + "testing" + + "thehouseoficarus/internal/engine" +) + +func TestSubscribeTicksKeepsSubscriptionWhenCallbackPanics(t *testing.T) { + g := &Game{owner: newOwner()} + g.Ticks = engine.New() + t.Cleanup(g.stopOwner) + + calls := 0 + g.SubscribeTicks(1, func() bool { + calls++ + if calls == 1 { + panic("tick system failure") + } + return false + }) + + g.Ticks.Tick() + // The panicking task's runTask (recover, log, counter) has not finished + // when the blocked caller is released, so drain the queue before reading + // the counter: the owner processes tasks FIFO. + g.run(func() {}) + if got := g.owner.panics.Load(); got != 1 { + t.Fatalf("contained panics = %d, want 1", got) + } + if calls != 1 { + t.Fatalf("callback calls = %d, want 1", calls) + } + + // The subscription must still be live: a deleted one never fires again, + // which would leave the world frozen while the process keeps running. + g.Ticks.Tick() + if calls != 2 { + t.Fatalf("a contained panic unsubscribed the system: calls = %d, want 2", calls) + } + + // An explicit false must still unsubscribe. + g.Ticks.Tick() + if calls != 2 { + t.Fatalf("explicit false did not unsubscribe: calls = %d, want 2", calls) + } +} + +func TestRunAdminOnOwnerReportsContainedPanicToCaller(t *testing.T) { + g := &Game{owner: newOwner()} + t.Cleanup(g.stopOwner) + + func() { + defer func() { + if r := recover(); r != adminTaskPanic { + t.Errorf("recovered %v, want the contained-task-panic value", r) + } + }() + if g.RunAdminOnOwner(func() { panic("editor task failure") }) { + t.Error("a panicking editor task was reported as accepted") + } + t.Fatal("a contained panic was reported as a normal result") + }() + + g.run(func() {}) + if got := g.owner.panics.Load(); got != 1 { + t.Errorf("contained panics = %d, want 1", got) + } + // The owner loop must still be draining, and the game must not have been + // flagged as shutting down by the panic. + ran := false + if !g.RunAdminOnOwner(func() { ran = true }) { + t.Fatal("the owner stopped accepting editor tasks after a contained panic") + } + if !ran { + t.Fatal("the owner loop did not execute later editor tasks") + } +} diff --git a/internal/game/correct_quality_merge_test.go b/internal/game/correct_quality_merge_test.go new file mode 100644 index 0000000..d54c8c9 --- /dev/null +++ b/internal/game/correct_quality_merge_test.go @@ -0,0 +1,201 @@ +package game + +// Regression: every stack-creation and stack-merge site must agree on the +// Quality/MaxQuality a fresh stack gets (newInventorySlot / stackProps). +// Pickups merge only into a stack with those exact properties, so a stack +// produced by crafting, harvesting, or ammo removal with Quality 0 could no +// longer be merged into - a full inventory then refused a pickup of an item +// the player already carried. + +import ( + "path/filepath" + "testing" + + "thehouseoficarus/internal/item" + "thehouseoficarus/internal/net" + "thehouseoficarus/internal/player" +) + +const correctQualityItemYAML = `id: correct_quality_gem +name: correct quality gem +stackable: true +max_quality: 5 +craft: + - type: crafting + ingredients: + - items: + - correct_quality_shard + quantity: 1 + xp: 1 + ticks_per_cycle: 1 +` + +func seedCorrectQualityItems(t *testing.T, h *harness) { + t.Helper() + writeSeedFile(t, filepath.Join(h.dir, "items", "correct_quality_gem.yaml"), []byte(correctQualityItemYAML)) + writeSeedFile(t, filepath.Join(h.dir, "items", "correct_quality_shard.yaml"), + []byte("id: correct_quality_shard\nname: correct quality shard\nstackable: true\nmax_quality: 5\nvalue: 1\n")) +} + +// fillInventoryExcept occupies every slot but slot 0 with filler. +func fillInventoryExcept(p *player.Player) { + for i := 0; i < 28; i++ { + if i != 0 { + p.SetInvSlot(i, &player.InventorySlot{ItemID: "filler", Quantity: 1}) + } + } +} + +// TestStackPropsMatchNewInventorySlot guards the one rule that stack creation +// and stack merging both depend on: the helper the craft/harvest/ammo sites +// use must stamp what newInventorySlot stamps. +func TestStackPropsMatchNewInventorySlot(t *testing.T) { + h := newHarness(t) + seedCorrectQualityItems(t, h) + h.g.RunOnOwner(func() { + h.g.ItemStore.Reload(h.dir) + gem := h.g.newInventorySlot("correct_quality_gem", 1) + if q, mq := stackProps(mustLoadItem(t, h.g, "correct_quality_gem")); q != gem.Quality || mq != gem.MaxQuality { + t.Errorf("stackProps = (%d,%d), newInventorySlot = (%d,%d)", q, mq, gem.Quality, gem.MaxQuality) + } + shard := h.g.newInventorySlot("correct_quality_shard", 1) + if q, mq := stackProps(mustLoadItem(t, h.g, "correct_quality_shard")); q != shard.Quality || mq != shard.MaxQuality { + t.Errorf("quality-free stackProps = (%d,%d), newInventorySlot = (%d,%d)", q, mq, shard.Quality, shard.MaxQuality) + } + }) +} + +func TestCraftOutputMergesIntoQualityStack(t *testing.T) { + h := newHarness(t) + h.seedAccount("alice") + p := h.seedCharacter("alice", "alice") + seedCorrectQualityItems(t, h) + + h.g.RunOnOwner(func() { + h.g.ItemStore.Reload(h.dir) + def := mustLoadItem(t, h.g, "correct_quality_gem") + p.SetInvSlot(0, &player.InventorySlot{ItemID: def.ID, Quantity: 10, Quality: 5, MaxQuality: 5}) + fillInventoryExcept(p) + + if !craftOutputFits(p, def.ID, def) { + t.Fatal("a full inventory must accept a craft output that merges into an identical stack") + } + if !placeCraftOutput(p, def.ID, def, 2) { + t.Fatal("craft output was not placed") + } + if slot := p.InvSlot(0); slot.Quantity != 12 || slot.Quality != 5 || slot.MaxQuality != 5 { + t.Errorf("merged stack = %+v, want quantity 12 at quality 5", slot) + } + + // A differently qualified stack is not a merge target: the same + // full inventory must report that nothing fits. + p.SetInvSlot(0, &player.InventorySlot{ItemID: def.ID, Quantity: 10, Quality: 1, MaxQuality: 5}) + if craftOutputFits(p, def.ID, def) { + t.Error("craft output would merge into a stack with a different quality") + } + if placeCraftOutput(p, def.ID, def, 2) { + t.Error("craft output was placed with no free slot and no matching stack") + } + }) +} + +func TestCraftOutputCreatesStackWithFreshProps(t *testing.T) { + h := newHarness(t) + h.seedAccount("alice") + p := h.seedCharacter("alice", "alice") + seedCorrectQualityItems(t, h) + + h.g.RunOnOwner(func() { + h.g.ItemStore.Reload(h.dir) + def := mustLoadItem(t, h.g, "correct_quality_gem") + if !placeCraftOutput(p, def.ID, def, 3) { + t.Fatal("craft output was not placed in an empty inventory") + } + slot := p.InvSlot(0) + if slot == nil || slot.ItemID != def.ID { + t.Fatalf("slot 0 = %+v", slot) + } + if q, mq := h.g.pickedStackProps(def.ID); slot.Quality != q || slot.MaxQuality != mq { + t.Fatalf("crafted stack %+v does not match what a pickup receives (%d,%d)", slot, q, mq) + } + }) +} + +func TestPickupMergesIntoHarvestedStackWhenFull(t *testing.T) { + h := newHarness(t) + h.seedAccount("alice") + p := h.seedCharacter("alice", "alice") + writeSeedFile(t, filepath.Join(h.dir, "items", hardeningCropID+".yaml"), + []byte("id: test_potato\nname: test potato\nstackable: true\nmax_quality: 5\n")) + + conn := newChanConn() + sess := &net.Session{Conn: conn, Player: p} + sess.SetState(net.StateGame) + + h.g.RunOnOwner(func() { + h.g.ItemStore.Reload(h.dir) + readyHarvest(p, 3, 3) + h.g.advanceHarvest(sess, p) + + stack := p.InvSlot(0) + if stack == nil || stack.ItemID != hardeningCropID { + t.Fatalf("harvested stack = %+v", stack) + } + quality, maxQuality := h.g.pickedStackProps(hardeningCropID) + if stack.Quality != quality || stack.MaxQuality != maxQuality { + t.Fatalf("harvested stack %+v does not match what a pickup receives (%d,%d)", stack, quality, maxQuality) + } + + // A full inventory must still merge a ground pickup into the stack + // the harvest just created. + fillInventoryExcept(p) + h.g.World.AddGroundItem(p.RoomID, hardeningCropID, 2) + h.g.doGet(sess, "test potato") + + if got := p.CountItem(hardeningCropID); got != 5 { + t.Errorf("potatoes carried = %d, want 5", got) + } + if got := h.g.World.GroundItems(p.RoomID)[hardeningCropID]; got != 0 { + t.Errorf("ground potatoes = %d, want 0 (they fit the existing stack)", got) + } + }) +} + +func TestPickupMergesIntoAmmoReturnedStack(t *testing.T) { + h := newHarness(t) + h.seedAccount("alice") + p := h.seedCharacter("alice", "alice") + seedCorrectQualityItems(t, h) + + conn := newChanConn() + sess := &net.Session{Conn: conn, Player: p} + sess.SetState(net.StateGame) + + h.g.RunOnOwner(func() { + h.g.ItemStore.Reload(h.dir) + p.Equipment[item.SlotAmmo] = "correct_quality_shard" + p.AmmoQty = 4 + if !h.g.unequipAmmo(p) { + t.Fatal("unequipAmmo refused an empty inventory") + } + stack := p.InvSlot(0) + if stack == nil || stack.ItemID != "correct_quality_shard" { + t.Fatalf("returned ammo stack = %+v", stack) + } + quality, maxQuality := h.g.pickedStackProps("correct_quality_shard") + if stack.Quality != quality || stack.MaxQuality != maxQuality { + t.Fatalf("returned ammo stack %+v does not match what a pickup receives (%d,%d)", stack, quality, maxQuality) + } + + fillInventoryExcept(p) + h.g.World.AddGroundItem(p.RoomID, "correct_quality_shard", 3) + h.g.doGet(sess, "correct quality shard") + + if got := p.CountItem("correct_quality_shard"); got != 7 { + t.Errorf("shards carried = %d, want 7", got) + } + if got := h.g.World.GroundItems(p.RoomID)["correct_quality_shard"]; got != 0 { + t.Errorf("ground shards = %d, want 0 (they fit the returned stack)", got) + } + }) +} diff --git a/internal/game/correct_vanished_account_reset_test.go b/internal/game/correct_vanished_account_reset_test.go new file mode 100644 index 0000000..d40f602 --- /dev/null +++ b/internal/game/correct_vanished_account_reset_test.go @@ -0,0 +1,75 @@ +package game + +// Regression: a session whose account file has vanished (purged, or deleted +// through the admin raw file editor) must be reset to the account-name prompt +// with no pending registration state left behind. Clearing Account alone left +// PendingPass set, so an already-buffered confirm-password line could still +// reach a handler that dereferences the account; applyNewAccount now also +// refuses to dereference a nil account. + +import ( + "strings" + "testing" + + "thehouseoficarus/internal/net" + "thehouseoficarus/internal/player" +) + +func TestPublishAccountUpdateVanishedAccountResetsSession(t *testing.T) { + h := newHarness(t) + + conn := newChanConn() + sess := &net.Session{Conn: conn, Account: &player.Account{Name: "bob"}} + sess.SetState(net.StateNewAccount) + sess.PendingPass = "pw" + h.hub.Add(sess) + + h.g.RunOnOwner(func() { h.g.publishAccountUpdate("bob", "bob") }) + + if got := sess.State(); got != net.StateAccountName { + t.Errorf("state = %v, want StateAccountName", got) + } + if sess.Account != nil { + t.Errorf("account not cleared: %+v", sess.Account) + } + if sess.PendingPass != "" { + t.Errorf("PendingPass = %q, want it cleared with the account", sess.PendingPass) + } + out := string(conn.Bytes()) + if !strings.Contains(out, "Your account no longer exists.") { + t.Errorf("missing vanished-account notice; output = %q", out) + } + if !strings.Contains(out, "Account name: ") { + t.Errorf("missing account-name prompt; output = %q", out) + } +} + +func TestApplyNewAccountResetsWhenAccountVanished(t *testing.T) { + h := newHarness(t) + + conn := newChanConn() + sess := &net.Session{Conn: conn} + sess.SetState(net.StateNewAccount) + sess.PendingPass = "pw" + + // Direct call: the menu handlers run on the owner goroutine, so the test + // goroutine stands in for it and an unguarded dereference fails loudly. + h.g.applyNewAccount(sess, "hash", nil) + + if got := sess.State(); got != net.StateAccountName { + t.Errorf("state = %v, want StateAccountName", got) + } + if sess.Account != nil { + t.Errorf("account not cleared: %+v", sess.Account) + } + if sess.PendingPass != "" { + t.Errorf("PendingPass = %q, want it cleared", sess.PendingPass) + } + out := string(conn.Bytes()) + if !strings.Contains(out, "Account name: ") { + t.Errorf("missing account-name prompt; output = %q", out) + } + if _, err := h.g.AccountStore.LoadAccount("bob"); err == nil { + t.Error("an account was created for a session with no account bound") + } +} diff --git a/internal/game/economy.go b/internal/game/economy.go index b39dbe4..b436a29 100644 --- a/internal/game/economy.go +++ b/internal/game/economy.go @@ -52,6 +52,7 @@ import ( "strings" "time" + "thehouseoficarus/internal/casino" "thehouseoficarus/internal/player" ) @@ -106,6 +107,12 @@ type economyRecord struct { type economyJournal struct { path string + // dirSynced records whether the directory entry of path has been made + // durable. A file's own fsync does not persist the entry that names it, + // so the first append must sync the directory (as persistence.WriteFile + // 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 // 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 @@ -132,6 +139,17 @@ func newEconomyJournal(dataDir string) *economyJournal { return &economyJournal{path: filepath.Join(dir, "journal.jsonl"), stakes: make(map[string][]*economyRecord), openPayouts: make(map[string]bool), openPayoutRecs: make(map[string][]*economyRecord)} } +// syncDir fsyncs a directory so an entry created or replaced in it survives a +// crash (the same durability step persistence.WriteFile performs). +func syncDir(dir string) error { + d, err := os.Open(dir) + if err != nil { + return err + } + defer d.Close() + return d.Sync() +} + func newEconomyRecordID() string { var b [8]byte if _, err := rand.Read(b[:]); err != nil { @@ -159,6 +177,12 @@ func (j *economyJournal) append(rec economyRecord) error { if err := f.Sync(); err != nil { return fmt.Errorf("sync economy journal: %w", err) } + if !j.dirSynced { + if err := syncDir(filepath.Dir(j.path)); err != nil { + return fmt.Errorf("sync economy journal dir: %w", err) + } + j.dirSynced = true + } return nil } @@ -497,11 +521,12 @@ func (j *economyJournal) loadOpen() ([]*economyRecord, error) { return open, nil } -// foldedRecord is the latest state of one record ID together with the -// order (line position of its latest write) it occupied in the journal. +// foldedRecord is the latest state of one record ID. Ordering decisions +// (which record came first) are made from economyRecord.Seq — the record's +// creation order — never from the record's line position, which a later +// rewrite of the same record moves. type foldedRecord struct { rec economyRecord - seq int } // loadFolded folds the journal (latest line per record ID wins) and @@ -556,9 +581,8 @@ func (j *economyJournal) loadFolded() ([]foldedRecord, error) { } if existing, ok := latest[rec.ID]; ok { existing.rec = rec - existing.seq = seq } else { - fr := &foldedRecord{rec: rec, seq: seq} + fr := &foldedRecord{rec: rec} latest[rec.ID] = fr order = append(order, fr) } @@ -617,9 +641,14 @@ func (j *economyJournal) compact() { } f.Close() // Best-effort atomic-ish replacement; a torn compaction leaves the - // original file intact. + // original file intact. The rename is made durable like the file's own + // sync, so a crash cannot lose the compacted journal entirely. if err := os.Rename(tmp, j.path); err != nil { os.Remove(tmp) + return + } + if err := syncDir(filepath.Dir(j.path)); err != nil { + fmt.Fprintf(os.Stderr, "economy compaction: sync journal dir: %v\n", err) } } @@ -698,22 +727,19 @@ func (g *Game) grantCredits(p *player.Player, amount int) { sum, err := addCreditsChecked(p.Credits, amount) if err != nil { fmt.Fprintf(os.Stderr, "credit grant for %s clamped at the economy cap: %v\n", p.Name, err) - p.Credits = maxCreditsValue + p.Credits = casino.MaxCredits return } p.Credits = sum } -// maxCreditsValue mirrors casino.MaxCredits; kept local to avoid a -// dependency cycle (see addCreditsChecked). -const maxCreditsValue = 1_000_000_000_000 - -// addCreditsChecked is the game-side checked addition against the economy -// cap (mirrors casino.MaxCredits; kept local to avoid a dependency cycle). +// addCreditsChecked is the game-side checked addition against the shared +// economy cap (casino.MaxCredits, mirrored by the casino wallet: a second, +// independently drifting cap would let the two sides of the money path +// disagree about what a balance may hold). func addCreditsChecked(a, b int) (int, error) { - const maxCredits = 1_000_000_000_000 - if b > 0 && a > maxCredits-b { - return maxCredits, fmt.Errorf("credit overflow: %d + %d exceeds %d", a, b, maxCredits) + if b > 0 && a > casino.MaxCredits-b { + return casino.MaxCredits, fmt.Errorf("credit overflow: %d + %d exceeds %d", a, b, casino.MaxCredits) } if b < 0 { return 0, fmt.Errorf("negative credit add %d", b) @@ -907,9 +933,16 @@ func (g *Game) recoverEconomy() { // instead marks a refund in progress: its credit part is re-applied by // phase 1 and the chip part (if the debit was durable) still comes // back in chips. - stakeEvidence := func(rec *economyRecord, seq int) (evidence bool, settlement bool) { + stakeEvidence := func(rec *economyRecord) (evidence bool, settlement bool) { for _, fr := range folded { - if fr.rec.Kind != "payout" || fr.rec.Status == "void" || fr.seq <= seq || + // Ordering is by the record's CREATION Seq, not by the line of + // its latest write: a payout created before this stake is not + // its settlement even when a later rewrite of that payout + // (a settle marker, or a recovery checkpoint re-anchor) lands + // after the stake's line. Line order would let an old payout + // mask a later stake and silently erase its refund (see + // stakeCovered, which orders the same way). + if fr.rec.Kind != "payout" || fr.rec.Status == "void" || fr.rec.Seq <= rec.Seq || !payoutIdentifies(fr.rec, *rec) { continue } @@ -1005,8 +1038,18 @@ func (g *Game) recoverEconomy() { // actually be paid from — without this, the balance a PAID // sibling moved reads as this record's own durability and // the liability is silently settled instead of paid. + // The re-anchored post-state must respect the cap: when it + // would exceed it, no reachable balance can pay the record, + // so the current balance is recorded as both checkpoints — + // the record then keeps reading as owed at this balance + // (retrying the re-application) instead of claiming an + // impossible post-state that a coincidental balance could + // be mistaken for as this record's own durability. other.CreditsBefore = p.Credits - other.CreditsAfter = p.Credits + other.Credit + other.CreditsAfter = p.Credits + if after, err := addCreditsChecked(p.Credits, other.Credit); err == nil { + other.CreditsAfter = after + } settled[other.ID] = true // handled for the rest of this run _ = g.Economy.append(*other) continue @@ -1110,7 +1153,7 @@ func (g *Game) recoverEconomy() { compensated++ continue } - resolvedByPayout, resolvedBySettlement := stakeEvidence(rec, folded[openWagers[k]].seq) + resolvedByPayout, resolvedBySettlement := stakeEvidence(rec) // Checkpoint comparisons use the DURABLE state, captured before any // in-memory chip restoration below. creditsNow, chipsNow := p.Credits, countChips(p) diff --git a/internal/game/game.go b/internal/game/game.go index ef48b63..de383a6 100644 --- a/internal/game/game.go +++ b/internal/game/game.go @@ -88,6 +88,13 @@ type Game struct { shutdownHook func(reason string) transportStop func() + // shutdownSaveMu guards shutdownSaveErr, the error of the final save + // runShutdown performs (nil when it succeeded or no shutdown ran). main + // reads it through ShutdownSaveError after the transports close, so a + // failed final save exits non-zero instead of only logging. + shutdownSaveMu sync.Mutex + shutdownSaveErr error + // Economy is the durable money journal; see economy.go. Economy *economyJournal @@ -117,9 +124,12 @@ func New(dataDir string, colorConfig *config.ColorsConfig, constantColorConfig * CardArt: cardart.Load(filepath.Join(dataDir, "cards.yaml")), DataDir: dataDir, }, - GlobalFlags: NewGlobalFlagStore(), - Combat: combat.NewTracker(), - Casino: casino.NewManager(time.Now().UnixNano()), + GlobalFlags: NewGlobalFlagStore(), + Combat: combat.NewTracker(), + // Entropy-seeded, not clock-seeded: a wall-clock seed makes every + // shoe and reel stream reconstructible. casino.NewManager(seed) stays + // the deterministic test seam. + Casino: casino.NewManagerEntropy(), flagIndex: newFlagTriggerIndex(), queue: NewCommandQueue(), safespot: NewSafespotManager(), @@ -228,6 +238,27 @@ func (g *Game) SetTransportStop(stop func()) { g.transportStop = stop } +// ShutdownSaveError reports the failure of the final online-character save +// performed by runShutdown. It is nil when that save succeeded or when no +// shutdown sequence ran. main calls it after (*net.Server).ListenAndServe +// returns (the transports close inside the shutdown sequence) and exits +// non-zero on a non-nil error: the transport return value reports "server +// stopped", never data loss. +func (g *Game) ShutdownSaveError() error { + g.shutdownSaveMu.Lock() + defer g.shutdownSaveMu.Unlock() + return g.shutdownSaveErr +} + +// 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. +func (g *Game) recordShutdownSaveErr(err error) { + g.shutdownSaveMu.Lock() + g.shutdownSaveErr = err + g.shutdownSaveMu.Unlock() +} + // runShutdown executes the ordered shutdown sequence: // // 1. Admissions stop: setShuttingDown already makes session input and @@ -256,6 +287,10 @@ func (g *Game) runShutdown() { } }) if saveErr != nil { + // Recorded as well as logged: main reads it after the transports + // close and exits non-zero, so an operator's supervisor can see + // that online characters were not persisted. + g.recordShutdownSaveErr(saveErr) log.Printf("shutdown: final save failed: %v", saveErr) } @@ -394,8 +429,25 @@ func (g *Game) SubscribeTicks(interval int, cb func() bool) uint64 { return g.Ticks.Subscribe(interval, cb) } return g.Ticks.Subscribe(interval, func() bool { - keep := false - g.run(func() { keep = cb() }) + // keep defaults to TRUE and is only read once the callback has run to + // completion. The owner loop's panic barrier (owner.runTask) contains + // a panic in cb, so returning false on a contained panic would tell + // the engine to DELETE this subscription - permanently unsubscribing + // the master subscriber main.go registers (RunTickSystems) on the + // first panic inside any tick system, leaving the process alive and + // accepting logins while commands never drain and casino rounds + // never settle. A subscription is only dropped on an explicit false. + // g.run also drops the task outright during shutdown; that is not a + // failure either. + ran := false + keep := true + g.run(func() { + keep = cb() + ran = true + }) + if !ran { + return true + } return keep }) } @@ -424,6 +476,67 @@ func (g *Game) EvictCasinoRoom(roomID int) { } } +// AdminPlayerFileOnline reports whether the data-relative player file path +// belongs to a character or account that is currently online, so the admin +// web editor (admin.AdminServer.SetPlayerFileOnlineCheck) refuses to +// overwrite or delete a file the game still owns in memory. relPath takes +// the form the editor passes, e.g. "players/characters/alice.yaml" or +// "players/accounts/alice.yaml"; any other path is not a player file and +// reports false. A missing account file also reports false (nothing is +// online under it). +// +// It reads the owner-safe registry rather than the owner-only fields of a +// session: character liveness comes from the mutex-guarded loggedInChars +// map (see characterOnline, which matches names case-insensitively) and +// account liveness from the account's on-disk +// character list, the same membership source the account purge uses. That +// keeps the hook safe whether the caller runs on the owner (where the admin +// file editor executes) or off it. +func (g *Game) AdminPlayerFileOnline(relPath string) bool { + name, kind, ok := playerDataFileName(relPath) + if !ok { + return false + } + switch kind { + case "characters": + return g.characterOnline(name) + case "accounts": + acc, err := g.AccountStore.LoadAccount(name) + if err != nil { + return false + } + // characterOnline compares case-insensitively, like the game's own + // membership check (isMemberOf): the account file's character list is + // hand-editable and need not spell a name the way the live session + // does, so "Alice" here must still find the online "alice". + for _, charName := range acc.Characters { + if g.characterOnline(charName) { + return true + } + } + } + return false +} + +// playerDataFileName splits a data-relative player file path into its base +// name and its kind ("characters" or "accounts"). It reports false for any +// path that is not players//.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/"))) + name, found := strings.CutSuffix(filepath.Base(clean), ".yaml") + if !found || name == "" { + return "", "", false + } + switch filepath.Dir(clean) { + case "players/characters": + return name, "characters", true + case "players/accounts": + return name, "accounts", true + } + return "", "", false +} + func (g *Game) writePrompt(sess *net.Session) { sess.WritePrompt(g.promptStr(sess)) } @@ -533,6 +646,14 @@ func (g *Game) ProcessQueuedCommands() { cmds := g.queue.DrainFree(p.Name) for _, qc := range cmds { + // The queued command records the session it was typed on, which + // can be a departed session by now (logout, disconnect, or a + // re-login on a new connection): never execute against it, or + // a nil Player dereference below kills the owner goroutine. + // Mirrors the nil guard in the actives loop. + if qc.Session == nil || qc.Session.Player == nil { + continue + } g.cancelRest(p.Name) raw := qc.Command if qc.Args != "" { diff --git a/internal/game/hardening_account_name_validation_test.go b/internal/game/hardening_account_name_validation_test.go new file mode 100644 index 0000000..7fca0ab --- /dev/null +++ b/internal/game/hardening_account_name_validation_test.go @@ -0,0 +1,201 @@ +package game + +// Regression coverage for name validation in the account/character lifecycle +// transactions: every caller-supplied (or account-file-supplied) name is +// joined into AccountPath/CharPath, so a path-like name must be rejected +// before the first join instead of deleting or moving a file outside the +// players/ tree. + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +// writeTraversalCanary creates a file that a path-traversal join would target, +// so a test can prove the transaction never touched it. +func writeTraversalCanary(t *testing.T, path string) { + t.Helper() + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, []byte("canary\n"), 0o644); err != nil { + t.Fatal(err) + } +} + +func assertTraversalCanary(t *testing.T, path string) { + t.Helper() + data, err := os.ReadFile(path) + if err != nil { + t.Fatalf("canary %s was removed: %v", path, err) + } + if string(data) != "canary\n" { + t.Fatalf("canary %s was overwritten: %q", path, data) + } +} + +func TestPurgeAccountTxnRejectsPathLikeNames(t *testing.T) { + h := newHarness(t) + h.seedAccount("alice") + + // CharPath("../../escape") would resolve to /escape.yaml. + charCanary := filepath.Join(h.dir, "escape.yaml") + writeTraversalCanary(t, charCanary) + + err := h.g.purgeAccountTxn("alice", []string{"../../escape"}) + if err == nil { + t.Fatal("purge accepted a path-traversing character name") + } + if !strings.Contains(err.Error(), "invalid character name") { + t.Fatalf("unclear purge error: %v", err) + } + assertTraversalCanary(t, charCanary) + if !h.g.AccountStore.AccountExists("alice") { + t.Fatal("purge ran (account file removed) despite the invalid character name") + } + + // AccountPath("../alice") would resolve to /players/alice.yaml. + accountCanary := filepath.Join(h.dir, "players", "alice.yaml") + writeTraversalCanary(t, accountCanary) + + err = h.g.purgeAccountTxn("../alice", nil) + if err == nil { + t.Fatal("purge accepted a path-traversing account name") + } + if !strings.Contains(err.Error(), "invalid account name") { + t.Fatalf("unclear purge error: %v", err) + } + assertTraversalCanary(t, accountCanary) + if !h.g.AccountStore.AccountExists("alice") { + t.Fatal("purge removed the real account despite the invalid account name") + } +} + +func TestRenameAccountTxnRejectsPathLikeNames(t *testing.T) { + h := newHarness(t) + h.seedAccount("dave") + + // AccountPath("../dave") would resolve to /players/dave.yaml. + accountCanary := filepath.Join(h.dir, "players", "dave.yaml") + writeTraversalCanary(t, accountCanary) + + err := h.g.renameAccountTxn("dave", "../dave") + if err == nil { + t.Fatal("rename accepted a path-traversing new account name") + } + if !strings.Contains(err.Error(), "invalid account name") { + t.Fatalf("unclear rename error: %v", err) + } + assertTraversalCanary(t, accountCanary) + if !h.g.AccountStore.AccountExists("dave") { + t.Fatal("rename moved the account file despite the invalid new name") + } + + // A path-like entry in the account's own character list is rejected too: + // the list is joined into CharPath while re-pointing owners. + acc, err := h.g.AccountStore.LoadAccount("dave") + if err != nil { + t.Fatal(err) + } + acc.Characters = []string{"../../escape"} + if err := h.g.AccountStore.SaveAccount(acc); err != nil { + t.Fatal(err) + } + charCanary := filepath.Join(h.dir, "escape.yaml") + writeTraversalCanary(t, charCanary) + + err = h.g.renameAccountTxn("dave", "dave2") + if err == nil { + t.Fatal("rename accepted an account whose character list is path-like") + } + if !strings.Contains(err.Error(), "invalid character name") { + t.Fatalf("unclear rename error: %v", err) + } + assertTraversalCanary(t, charCanary) + if !h.g.AccountStore.AccountExists("dave") { + t.Fatal("rename moved the account file despite the invalid character list") + } + if h.g.AccountStore.AccountExists("dave2") { + t.Fatal("rename created the new account name despite the invalid character list") + } +} + +func TestCharacterTxnsRejectPathLikeNames(t *testing.T) { + h := newHarness(t) + h.seedAccount("alice") + + // CharPath("../escape") resolves to /players/escape.yaml. + canary := filepath.Join(h.dir, "players", "escape.yaml") + writeTraversalCanary(t, canary) + + if _, err := h.g.createCharacterTxn("alice", "../escape", 1); err == nil { + t.Fatal("create accepted a path-traversing character name") + } else if !strings.Contains(err.Error(), "invalid character name") { + t.Fatalf("unclear create error: %v", err) + } + assertTraversalCanary(t, canary) + + if _, err := h.g.createCharacterTxn("../alice", "bob", 1); err == nil { + t.Fatal("create accepted a path-traversing account name") + } else if !strings.Contains(err.Error(), "invalid account name") { + t.Fatalf("unclear create error: %v", err) + } + assertTraversalCanary(t, canary) + + if err := h.g.renameCharacterTxn("alice", "../escape", "bob"); err == nil { + t.Fatal("rename accepted a path-traversing old character name") + } else if !strings.Contains(err.Error(), "invalid character name") { + t.Fatalf("unclear rename error: %v", err) + } + assertTraversalCanary(t, canary) + + if err := h.g.deleteCharacterTxn("alice", "../escape"); err == nil { + t.Fatal("delete accepted a path-traversing character name") + } else if !strings.Contains(err.Error(), "invalid character name") { + t.Fatalf("unclear delete error: %v", err) + } + assertTraversalCanary(t, canary) + + // Valid names still work: the validation must not block the normal path. + p, err := h.g.createCharacterTxn("alice", "bob", 1) + if err != nil { + t.Fatalf("create with a valid name failed: %v", err) + } + if p.Name != "bob" { + t.Fatalf("created character name = %q, want bob", p.Name) + } + acc, err := h.g.AccountStore.LoadAccount("alice") + if err != nil { + t.Fatal(err) + } + if len(acc.Characters) != 1 || acc.Characters[0] != "bob" { + t.Fatalf("account character list = %v, want [bob]", acc.Characters) + } + if err := h.g.deleteCharacterTxn("alice", "bob"); err != nil { + t.Fatalf("delete with a valid name failed: %v", err) + } + if _, err := h.g.AccountStore.LoadCharacter("bob"); err == nil { + t.Fatal("delete left the character file behind") + } +} + +// TestPurgeAccountTxnAcceptsSeededNames is the positive control: the existing +// login/session path (a genuinely registered account with its own character) +// still purges. +func TestPurgeAccountTxnAcceptsSeededNames(t *testing.T) { + h := newHarness(t) + h.seedAccount("alice") + h.seedCharacter("alice", "alice") + + if err := h.g.purgeAccountTxn("alice", []string{"alice"}); err != nil { + t.Fatalf("purge with valid names failed: %v", err) + } + if h.g.AccountStore.AccountExists("alice") { + t.Fatal("purge left the account file behind") + } + if _, err := h.g.AccountStore.LoadCharacter("alice"); err == nil { + t.Fatal("purge left the character file behind") + } +} diff --git a/internal/game/hardening_admin_player_file_online_test.go b/internal/game/hardening_admin_player_file_online_test.go new file mode 100644 index 0000000..f0c64f3 --- /dev/null +++ b/internal/game/hardening_admin_player_file_online_test.go @@ -0,0 +1,70 @@ +package game + +// Regression coverage for Game.AdminPlayerFileOnline, the hook the admin raw +// file editor uses (admin.AdminServer.SetPlayerFileOnlineCheck) to refuse +// editing a character/account file whose character is live. + +import "testing" + +func TestAdminPlayerFileOnlineRefusesLivePlayerFiles(t *testing.T) { + h := newHarness(t) + h.seedAccount("alice") + h.seedCharacter("alice", "alice") + // A second character of the same account, offline, so the account check + // cannot pass merely because the file exists. + h.seedCharacter("alicia", "alice") + h.seedAccount("carol") + h.seedCharacter("carol", "carol") + + sess, _ := h.connect("alice") + if sess.Player == nil || sess.Player.Name != "alice" { + t.Fatalf("alice is not registered as online: %+v", sess.Player) + } + + cases := []struct { + name string + path string + want bool + }{ + {"online character", "players/characters/alice.yaml", true}, + {"offline character of a live account", "players/characters/alicia.yaml", false}, + {"offline character", "players/characters/carol.yaml", false}, + {"unknown character", "players/characters/nobody.yaml", false}, + {"account of an online character", "players/accounts/alice.yaml", true}, + {"account with no online character", "players/accounts/carol.yaml", false}, + {"unknown account", "players/accounts/nobody.yaml", false}, + {"leading slash", "/players/characters/alice.yaml", true}, + {"data/ prefix", "data/players/accounts/alice.yaml", true}, + {"non-yaml file", "players/characters/alice.txt", false}, + {"bare directory", "players/characters", false}, + {"nested path", "players/characters/sub/alice.yaml", false}, + {"non-player file", "rooms/1.yaml", false}, + {"empty path", "", false}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := h.g.AdminPlayerFileOnline(tc.path); got != tc.want { + t.Fatalf("AdminPlayerFileOnline(%q) = %v, want %v", tc.path, got, tc.want) + } + }) + } +} + +// TestAdminPlayerFileOnlineIgnoresMenuOnlySession pins that an account sitting +// in the character menu is NOT treated as an online character: only a live +// character (a loggedInChars entry) makes its file, or its account's file, +// editable-refused for that reason. +func TestAdminPlayerFileOnlineIgnoresMenuOnlySession(t *testing.T) { + h := newHarness(t) + h.seedAccount("dave") + h.seedCharacter("dave", "dave") + h.connectMenu("dave") + + if h.g.AdminPlayerFileOnline("players/characters/dave.yaml") { + t.Fatal("a menu session was treated as a live character") + } + if h.g.AdminPlayerFileOnline("players/accounts/dave.yaml") { + t.Fatal("a menu session was treated as a live account character") + } +} diff --git a/internal/game/hardening_ammo_unequip_test.go b/internal/game/hardening_ammo_unequip_test.go new file mode 100644 index 0000000..2250b83 --- /dev/null +++ b/internal/game/hardening_ammo_unequip_test.go @@ -0,0 +1,181 @@ +package game + +// Regression: returning the equipped ammo stack to the inventory must never +// delete it. The equipment entry may only be cleared once the stack has been +// merged into an inventory stack or placed in a free slot, so a full +// inventory refuses the swap/removal instead of losing the ammo. The previous +// code deleted Equipment[SlotAmmo] and zeroed AmmoQty unconditionally. + +import ( + "path/filepath" + "strings" + "testing" + + "thehouseoficarus/internal/item" + "thehouseoficarus/internal/net" + "thehouseoficarus/internal/player" +) + +const ( + hardeningArrowsID = "test_arrows" + hardeningBoltsID = "test_bolts" +) + +// seedAmmoItems installs two stackable ammo items so a swap has a target. +func seedAmmoItems(t *testing.T, h *harness) { + t.Helper() + writeSeedFile(t, filepath.Join(h.dir, "items", hardeningArrowsID+".yaml"), + []byte("id: test_arrows\nname: test arrows\nstackable: true\nequipment:\n type: ammo\n slot: ammo\n")) + writeSeedFile(t, filepath.Join(h.dir, "items", hardeningBoltsID+".yaml"), + []byte("id: test_bolts\nname: test bolts\nstackable: true\nequipment:\n type: ammo\n slot: ammo\n")) +} + +// fillAllSlots occupies every inventory slot and returns the session used by +// the wear/remove paths. +func fillAllSlots(p *player.Player) *net.Session { + for i := 0; i < 28; i++ { + p.SetInvSlot(i, &player.InventorySlot{ItemID: "filler", Quantity: 1}) + } + sess := &net.Session{Conn: newChanConn(), Player: p} + sess.SetState(net.StateGame) + return sess +} + +func TestUnequipAmmoSwapKeepsStackWhenInventoryFull(t *testing.T) { + h := newHarness(t) + h.seedAccount("alice") + p := h.seedCharacter("alice", "alice") + seedAmmoItems(t, h) + h.g.RunOnOwner(func() { + h.g.ItemStore.Reload(h.dir) + sess := fillAllSlots(p) + p.SetInvSlot(3, &player.InventorySlot{ItemID: hardeningBoltsID, Quantity: 7}) + p.Equipment[item.SlotAmmo] = hardeningArrowsID + p.AmmoQty = 42 + + h.g.doWear(sess, "test bolts") + + if got := p.Equipment[item.SlotAmmo]; got != hardeningArrowsID { + t.Errorf("equipped ammo = %q, want %q: a refused swap must not clear equipment", got, hardeningArrowsID) + } + if p.AmmoQty != 42 { + t.Errorf("AmmoQty = %d, want 42", p.AmmoQty) + } + bolts := p.InvSlot(3) + if bolts == nil || bolts.ItemID != hardeningBoltsID || bolts.Quantity != 7 { + t.Errorf("source stack changed by a refused swap: %+v", bolts) + } + if out := sess.Conn.(*chanConn).Bytes(); !strings.Contains(string(out), "inventory is full") { + t.Errorf("expected a full-inventory message, got %q", out) + } + }) +} + +func TestUnequipAmmoSwapMovesStackToFreeSlot(t *testing.T) { + h := newHarness(t) + h.seedAccount("alice") + p := h.seedCharacter("alice", "alice") + seedAmmoItems(t, h) + h.g.RunOnOwner(func() { + h.g.ItemStore.Reload(h.dir) + sess := fillAllSlots(p) + p.SetInvSlot(0, nil) + p.SetInvSlot(3, &player.InventorySlot{ItemID: hardeningBoltsID, Quantity: 7}) + p.Equipment[item.SlotAmmo] = hardeningArrowsID + p.AmmoQty = 42 + + h.g.doWear(sess, "test bolts") + + if got := p.Equipment[item.SlotAmmo]; got != hardeningBoltsID { + t.Fatalf("equipped ammo = %q, want %q", got, hardeningBoltsID) + } + if p.AmmoQty != 7 { + t.Errorf("AmmoQty = %d, want 7", p.AmmoQty) + } + stored := p.InvSlot(0) + if stored == nil || stored.ItemID != hardeningArrowsID || stored.Quantity != 42 { + t.Fatalf("unequipped arrows not stored in the free slot: %+v", stored) + } + }) +} + +func TestRemoveAllAmmoKeepsStackWhenInventoryFull(t *testing.T) { + h := newHarness(t) + h.seedAccount("alice") + p := h.seedCharacter("alice", "alice") + seedAmmoItems(t, h) + h.g.RunOnOwner(func() { + h.g.ItemStore.Reload(h.dir) + sess := fillAllSlots(p) + p.Equipment[item.SlotAmmo] = hardeningArrowsID + p.AmmoQty = 42 + + h.g.removeAllAmmo(sess, p, hardeningArrowsID) + + if got := p.Equipment[item.SlotAmmo]; got != hardeningArrowsID { + t.Errorf("equipped ammo = %q, want %q", got, hardeningArrowsID) + } + if p.AmmoQty != 42 { + t.Errorf("AmmoQty = %d, want 42", p.AmmoQty) + } + out := string(sess.Conn.(*chanConn).Bytes()) + if strings.Contains(out, "You remove") { + t.Errorf("nothing was removed but the removal was reported: %q", out) + } + if !strings.Contains(out, "Nowhere to put that!") { + t.Errorf("expected a failure message, got %q", out) + } + }) +} + +func TestRemoveAllKeepsAmmoWhenInventoryFull(t *testing.T) { + h := newHarness(t) + h.seedAccount("alice") + p := h.seedCharacter("alice", "alice") + seedAmmoItems(t, h) + h.g.RunOnOwner(func() { + h.g.ItemStore.Reload(h.dir) + sess := fillAllSlots(p) + p.Equipment[item.SlotAmmo] = hardeningArrowsID + p.AmmoQty = 42 + + h.g.doRemoveAll(sess) + + if got := p.Equipment[item.SlotAmmo]; got != hardeningArrowsID || p.AmmoQty != 42 { + t.Errorf("remove all lost the equipped ammo: %q x%d", got, p.AmmoQty) + } + if got := p.CountItem(hardeningArrowsID); got != 0 { + t.Errorf("remove all invented %d arrows", got) + } + }) +} + +func TestRemoveAllAmmoMovesStackToFreeSlot(t *testing.T) { + h := newHarness(t) + h.seedAccount("alice") + p := h.seedCharacter("alice", "alice") + seedAmmoItems(t, h) + h.g.RunOnOwner(func() { + h.g.ItemStore.Reload(h.dir) + sess := fillAllSlots(p) + p.SetInvSlot(0, nil) + p.Equipment[item.SlotAmmo] = hardeningArrowsID + p.AmmoQty = 42 + + h.g.removeAllAmmo(sess, p, hardeningArrowsID) + + if _, ok := p.Equipment[item.SlotAmmo]; ok { + t.Error("equipment entry survived a successful removal") + } + if p.AmmoQty != 0 { + t.Errorf("AmmoQty = %d, want 0", p.AmmoQty) + } + stored := p.InvSlot(0) + if stored == nil || stored.ItemID != hardeningArrowsID || stored.Quantity != 42 { + t.Fatalf("removed arrows not stored in the free slot: %+v", stored) + } + if out := string(sess.Conn.(*chanConn).Bytes()); !strings.Contains(out, "You remove") { + t.Errorf("removal not reported: %q", out) + } + }) +} diff --git a/internal/game/hardening_casino_entropy_wiring_test.go b/internal/game/hardening_casino_entropy_wiring_test.go new file mode 100644 index 0000000..4f7c75e --- /dev/null +++ b/internal/game/hardening_casino_entropy_wiring_test.go @@ -0,0 +1,72 @@ +package game + +// Closure regressions for the casino RNG hardening: the entropy-seeded +// constructor must be the one PRODUCTION wires, not merely available. + +import ( + "io/fs" + "os" + "path/filepath" + "strings" + "testing" +) + +// TestProductionCasinoSeedIsEntropy pins the production wiring: game.New must +// build its casino manager with casino.NewManagerEntropy, and no production +// file may call casino.NewManager(seed). That constructor exists only as the +// deterministic test seam, because a wall-clock seed makes every shoe, reel, +// and spin stream reconstructible from the startup nanosecond. +// +// The manager's seed is unexported and deliberately not observable through the +// casino API (NewManagerEntropy and NewManager are otherwise +// indistinguishable), so this is a source-level guard: it is the only place +// the choice of constructor can be pinned. It fails on a revert to +// NewManager(time.Now().UnixNano()). +func TestProductionCasinoSeedIsEntropy(t *testing.T) { + root := filepath.Join("..", "..") + sawEntropyCall := false + err := filepath.WalkDir(root, func(path string, d fs.DirEntry, err error) error { + if err != nil { + return err + } + if d.IsDir() { + name := d.Name() + // Skip VCS internals, generated/test data, and the persisted + // game data (which is not Go source at all). + if path != root && (strings.HasPrefix(name, ".") || name == "testdata" || name == "vendor") { + return fs.SkipDir + } + return nil + } + if !strings.HasSuffix(d.Name(), ".go") || strings.HasSuffix(d.Name(), "_test.go") { + return nil + } + data, readErr := os.ReadFile(path) + if readErr != nil { + return readErr + } + rel, relErr := filepath.Rel(root, path) + if relErr != nil { + rel = path + } + for i, line := range strings.Split(string(data), "\n") { + trimmed := strings.TrimSpace(line) + if strings.HasPrefix(trimmed, "//") || strings.HasPrefix(trimmed, "*") { + continue // prose about the seam, not a call + } + if strings.Contains(trimmed, "casino.NewManager(") { + t.Errorf("%s:%d calls the deterministic casino.NewManager(seed) outside tests: %s", rel, i+1, trimmed) + } + if strings.Contains(trimmed, "casino.NewManagerEntropy()") { + sawEntropyCall = true + } + } + return nil + }) + if err != nil { + t.Fatalf("walk module: %v", err) + } + if !sawEntropyCall { + t.Fatal("no production call to casino.NewManagerEntropy() found: the casino RNG entropy fix is not wired") + } +} diff --git a/internal/game/hardening_casino_quit_autobet_test.go b/internal/game/hardening_casino_quit_autobet_test.go new file mode 100644 index 0000000..34f36e7 --- /dev/null +++ b/internal/game/hardening_casino_quit_autobet_test.go @@ -0,0 +1,75 @@ +package game + +import ( + "strings" + "testing" + + "thehouseoficarus/internal/casino" +) + +// Quitting at a slot machine must take one command: a machine between spins +// has no committed stake, so "quit" stops autobet AND frees the seat instead +// of reporting the player as still playing (the old flow needed a second +// quit and warned about a forfeit that could not happen). + +func TestQuitAtMachineStopsAutobetAndLeavesInOneAttempt(t *testing.T) { + g, sess := quitTestGame(t) + cfg := casino.MachineConfig{ID: "slots", Game: casino.GameSlots, MinBet: 1, MaxBet: 100, SpinTicks: 1} + g.Casino.JoinMachine(1, cfg, "alice") + wallet := casinoWallet{g: g, p: sess.Player} + if events := g.Casino.SetMachineBet(1, "slots", "alice", 10, wallet); len(events) == 0 { + t.Fatal("bet produced no events") + } + // Let the bet's spin finish: the machine is now between spins. + for i := 0; i < 30 && g.Casino.MachineBusy("alice"); i++ { + g.Casino.Tick() + } + if events := g.Casino.EnableMachineAutoBet(1, "slots", "alice", wallet); len(events) != 0 { + t.Fatalf("enabling autobet emitted events: %+v", events) + } + + conn := sess.Conn.(*casinoTestConn) + conn.data = nil + g.doQuit(sess) + out := string(conn.data) + + if !g.quitResting() { + t.Fatalf("quit between spins did not start the rest countdown: %q", out) + } + if _, playing := g.Casino.Participation("alice"); playing { + t.Fatalf("quit left the player at the machine, requiring a second quit: %q", out) + } + if !strings.Contains(out, "Autobet stopped") { + t.Fatalf("quit did not stop autobet: %q", out) + } + if strings.Contains(out, "WARNING") || strings.Contains(out, "forfeit") { + t.Fatalf("quit warned about a stake that does not exist: %q", out) + } +} + +// A machine mid-spin really does hold a committed stake, so quit must still +// refuse it (with the forfeit warning) rather than silently discard the spin. +func TestQuitMidSpinStillWarnsAboutTheCommittedStake(t *testing.T) { + g, sess := quitTestGame(t) + cfg := casino.MachineConfig{ID: "slots", Game: casino.GameSlots, MinBet: 1, MaxBet: 100, SpinTicks: 50} + g.Casino.JoinMachine(1, cfg, "alice") + wallet := casinoWallet{g: g, p: sess.Player} + if events := g.Casino.SetMachineBet(1, "slots", "alice", 10, wallet); len(events) == 0 { + t.Fatal("bet produced no events") + } + + conn := sess.Conn.(*casinoTestConn) + conn.data = nil + g.doQuit(sess) + out := string(conn.data) + + if g.quitResting() { + t.Fatal("quit started the countdown with a spin in flight") + } + if _, playing := g.Casino.Participation("alice"); !playing { + t.Fatal("quit freed the machine mid-spin") + } + if !strings.Contains(out, "forfeit") { + t.Fatalf("expected the forfeit warning for a committed stake, got %q", out) + } +} diff --git a/internal/game/hardening_craft_combat_helpers_test.go b/internal/game/hardening_craft_combat_helpers_test.go new file mode 100644 index 0000000..cfd860f --- /dev/null +++ b/internal/game/hardening_craft_combat_helpers_test.go @@ -0,0 +1,40 @@ +package game + +// Shared helpers for the crafting/combat hardening regressions. The tests +// build a real Game (owner loop, stores, craft index) over a temp data dir +// but deliberately do NOT start a background ticker, so each test drives +// production/combat cycles with explicit calls and asserts without racing +// Game.AdvanceActions. + +import ( + "os" + "path/filepath" + "testing" + + "thehouseoficarus/internal/config" + "thehouseoficarus/internal/net" +) + +// newHardeningGame builds a real Game over dir with an owner loop but no +// background ticker. stopOwner is registered for cleanup so the owner +// goroutine does not leak. +func newHardeningGame(t *testing.T, dir string) *Game { + t.Helper() + colors := &config.ColorsConfig{} + g := New(dir, colors, colors, config.ValidationConfig{}, 1) + g.SetHub(net.NewHub()) + t.Cleanup(g.stopOwner) + return g +} + +// writeDataFile writes subdir/name under dir, creating parent directories. +func writeDataFile(t *testing.T, dir, subdir, name, content string) { + t.Helper() + path := filepath.Join(dir, subdir, name) + if err := os.MkdirAll(filepath.Dir(path), 0755); err != nil { + t.Fatalf("mkdir %s: %v", subdir, err) + } + if err := os.WriteFile(path, []byte(content), 0644); err != nil { + t.Fatalf("write %s: %v", path, err) + } +} diff --git a/internal/game/hardening_craft_combine_fletch_test.go b/internal/game/hardening_craft_combine_fletch_test.go new file mode 100644 index 0000000..f3f6832 --- /dev/null +++ b/internal/game/hardening_craft_combine_fletch_test.go @@ -0,0 +1,260 @@ +package game + +// Regression: combine and fletch must honor the craft fields production +// already honors — roll craft.Success when defined, create craft.Fail on +// failure, award craft.XP, and place ingredient byproducts. Shipped combine +// and fletch recipes set xp:0 and define no fail/success, so these paths +// must be no-ops for them (asserted indirectly by the existing production +// regressions). + +import ( + "testing" + + "thehouseoficarus/internal/item" + "thehouseoficarus/internal/net" + "thehouseoficarus/internal/player" +) + +const combineSuccessYAML = `id: test_combine_out +name: test combine out +stackable: false +value: 5 +craft: + - type: combine + ingredients: + - items: + - combine_part + quantity: 1 + byproducts: + - combine_scrap + success: + base: 1.0 + per_level: 0 + cap: 1.0 + xp: 25 + output_qty: 1 + ticks_per_cycle: 1 +` + +const combineFailYAML = `id: test_combine_fail_out +name: test combine fail out +stackable: false +value: 5 +craft: + - type: combine + ingredients: + - items: + - combine_fail_part + quantity: 1 + success: + base: 0 + per_level: 0 + cap: 0 + fail: combine_ash + output_qty: 1 + ticks_per_cycle: 1 +` + +const fletchSuccessYAML = `id: test_fletch_out +name: test fletch out +stackable: false +value: 5 +craft: + - type: fletching + ingredients: + - items: + - fletch_part + quantity: 1 + byproducts: + - fletch_scrap + success: + base: 1.0 + per_level: 0 + cap: 1.0 + xp: 30 + output_qty: 1 + ticks_per_cycle: 1 +` + +const fletchFailYAML = `id: test_fletch_fail_out +name: test fletch fail out +stackable: false +value: 5 +craft: + - type: fletching + ingredients: + - items: + - fletch_fail_part + quantity: 1 + success: + base: 0 + per_level: 0 + cap: 0 + fail: fletch_ash + output_qty: 1 + ticks_per_cycle: 1 +` + +func seedCraftFieldItems(t *testing.T, dir string) { + t.Helper() + for _, id := range []string{ + "combine_part", "combine_scrap", "combine_fail_part", "combine_ash", + "fletch_part", "fletch_scrap", "fletch_fail_part", "fletch_ash", + } { + writeDataFile(t, dir, "items", id+".yaml", "id: "+id+"\nname: "+id+"\nstackable: false\nvalue: 1\n") + } + writeDataFile(t, dir, "items", "test_combine_out.yaml", combineSuccessYAML) + writeDataFile(t, dir, "items", "test_combine_fail_out.yaml", combineFailYAML) + writeDataFile(t, dir, "items", "test_fletch_out.yaml", fletchSuccessYAML) + writeDataFile(t, dir, "items", "test_fletch_fail_out.yaml", fletchFailYAML) +} + +func TestCombineHonorsSuccessXPAndByproducts(t *testing.T) { + dir := t.TempDir() + seedWorld(t, dir) + seedCraftFieldItems(t, dir) + g := newHardeningGame(t, dir) + + g.RunOnOwner(func() { + p := player.New("alice") + p.RoomID = 1 + p.SetInvSlot(0, &player.InventorySlot{ItemID: "combine_part", Quantity: 1}) + + sess := &net.Session{Conn: newChanConn(), Player: p} + sess.SetState(net.StateGame) + + def := mustLoadItem(t, g, "test_combine_out") + g.startCombine(sess, p, def, def.FirstCraft(), 1) + if p.Action == nil { + t.Fatal("combine did not start") + } + g.advanceCombine(sess, p) // phase 0 -> 1 + g.advanceCombine(sess, p) // phase 1 -> 2 + g.advanceCombine(sess, p) // phase 2: consume + output + + if got := p.CountItem("test_combine_out"); got != 1 { + t.Fatalf("combine output = %d, want 1", got) + } + if got := p.CountItem("combine_scrap"); got != 1 { + t.Errorf("combine byproduct = %d, want 1", got) + } + if got := p.CountItem("combine_part"); got != 0 { + t.Errorf("combine ingredient = %d, want 0 (consumed)", got) + } + if got := p.Skills[player.Crafting]; got != 25 { + t.Errorf("combine xp = %d, want 25 on crafting", got) + } + if _, ok := p.Skills[player.SkillName("combine")]; ok { + t.Error("combine recipes must not persist a phantom 'combine' skill") + } + }) +} + +func TestCombineCreatesFailOnFailure(t *testing.T) { + dir := t.TempDir() + seedWorld(t, dir) + seedCraftFieldItems(t, dir) + g := newHardeningGame(t, dir) + + g.RunOnOwner(func() { + p := player.New("alice") + p.RoomID = 1 + p.SetInvSlot(0, &player.InventorySlot{ItemID: "combine_fail_part", Quantity: 1}) + + sess := &net.Session{Conn: newChanConn(), Player: p} + sess.SetState(net.StateGame) + + def := mustLoadItem(t, g, "test_combine_fail_out") + g.startCombine(sess, p, def, def.FirstCraft(), 1) + g.advanceCombine(sess, p) + g.advanceCombine(sess, p) + g.advanceCombine(sess, p) + + if got := p.CountItem("test_combine_fail_out"); got != 0 { + t.Errorf("combine output = %d, want 0 on a guaranteed failure", got) + } + if got := p.CountItem("combine_ash"); got != 1 { + t.Errorf("combine fail product = %d, want 1", got) + } + if got := p.CountItem("combine_fail_part"); got != 0 { + t.Errorf("combine ingredient = %d, want 0 (consumed)", got) + } + }) +} + +func TestFletchHonorsSuccessXPAndByproducts(t *testing.T) { + dir := t.TempDir() + seedWorld(t, dir) + seedCraftFieldItems(t, dir) + g := newHardeningGame(t, dir) + + g.RunOnOwner(func() { + p := player.New("alice") + p.RoomID = 1 + p.SetInvSlot(0, &player.InventorySlot{ItemID: "fletch_part", Quantity: 1}) + + sess := &net.Session{Conn: newChanConn(), Player: p} + sess.SetState(net.StateGame) + + def := mustLoadItem(t, g, "test_fletch_out") + g.startFletchAction(sess, p, def, 1) + if p.BackgroundAction == nil { + t.Fatal("fletch did not start") + } + g.advanceFletch(sess, p) // phase 0 -> 1 + g.advanceFletch(sess, p) // phase 1: consume + output + + if got := p.CountItem("test_fletch_out"); got != 1 { + t.Fatalf("fletch output = %d, want 1", got) + } + if got := p.CountItem("fletch_scrap"); got != 1 { + t.Errorf("fletch byproduct = %d, want 1", got) + } + if got := p.CountItem("fletch_part"); got != 0 { + t.Errorf("fletch ingredient = %d, want 0 (consumed)", got) + } + if got := p.Skills[player.Fletching]; got != 30 { + t.Errorf("fletch xp = %d, want 30", got) + } + }) +} + +func TestFletchCreatesFailOnFailure(t *testing.T) { + dir := t.TempDir() + seedWorld(t, dir) + seedCraftFieldItems(t, dir) + g := newHardeningGame(t, dir) + + g.RunOnOwner(func() { + p := player.New("alice") + p.RoomID = 1 + p.SetInvSlot(0, &player.InventorySlot{ItemID: "fletch_fail_part", Quantity: 1}) + + sess := &net.Session{Conn: newChanConn(), Player: p} + sess.SetState(net.StateGame) + + def := mustLoadItem(t, g, "test_fletch_fail_out") + g.startFletchAction(sess, p, def, 1) + g.advanceFletch(sess, p) + g.advanceFletch(sess, p) + + if got := p.CountItem("test_fletch_fail_out"); got != 0 { + t.Errorf("fletch output = %d, want 0 on a guaranteed failure", got) + } + if got := p.CountItem("fletch_ash"); got != 1 { + t.Errorf("fletch fail product = %d, want 1", got) + } + if got := p.CountItem("fletch_fail_part"); got != 0 { + t.Errorf("fletch ingredient = %d, want 0 (consumed)", got) + } + }) +} + +func mustLoadItem(t *testing.T, g *Game, id string) *item.ItemDef { + t.Helper() + def, err := g.ItemStore.Load(id) + if err != nil { + t.Fatalf("load item %s: %v", id, err) + } + return def +} diff --git a/internal/game/hardening_craft_menu_skill_test.go b/internal/game/hardening_craft_menu_skill_test.go new file mode 100644 index 0000000..9b4e8c7 --- /dev/null +++ b/internal/game/hardening_craft_menu_skill_test.go @@ -0,0 +1,91 @@ +package game + +// Regression: the multi-match production menu (craft /mix/construct) +// must pass the real lowercase recipe type as PendingSkill. It previously +// passed the capitalised display label ("Crafting"), which +// handleProductChoice looks up with player.SkillName — a map miss that +// resolves to level 1, so every recipe above level 1 was refused even for a +// level-50 crafter. + +import ( + "testing" + + "thehouseoficarus/internal/net" + "thehouseoficarus/internal/player" +) + +const craftMenuIngotYAML = `id: test_ingot +name: test ingot +stackable: true +value: 1 +` + +const craftMenuSwordYAML = `id: test_iron_sword +name: iron sword +stackable: false +value: 5 +craft: + - type: crafting + ingredients: + - items: + - test_ingot + quantity: 1 + level: 1 + output_qty: 1 +` + +const craftMenuPlateYAML = `id: test_iron_plate +name: iron plate +stackable: false +value: 50 +craft: + - type: crafting + ingredients: + - items: + - test_ingot + quantity: 1 + level: 40 + output_qty: 1 +` + +func TestCraftMenuResolvesRealSkillForMultiMatch(t *testing.T) { + dir := t.TempDir() + seedWorld(t, dir) + writeDataFile(t, dir, "items", "test_ingot.yaml", craftMenuIngotYAML) + writeDataFile(t, dir, "items", "test_iron_sword.yaml", craftMenuSwordYAML) + writeDataFile(t, dir, "items", "test_iron_plate.yaml", craftMenuPlateYAML) + + g := newHardeningGame(t, dir) + + g.RunOnOwner(func() { + p := player.New("alice") + p.RoomID = 1 + p.Skills[player.Crafting] = player.XPForLevel(50) + p.SetInvSlot(0, &player.InventorySlot{ItemID: "test_ingot", Quantity: 1}) + + conn := newChanConn() + sess := &net.Session{Conn: conn, Player: p} + sess.SetState(net.StateGame) + + // "iron" matches both products, forcing the multi-match menu path. + g.doCraft(sess, "iron") + + if sess.PendingSkill != "crafting" { + t.Fatalf("PendingSkill = %q, want %q (a display label is not a skill)", + sess.PendingSkill, "crafting") + } + if len(sess.PendingMenu) != 2 { + t.Fatalf("menu entries = %d, want 2 (iron sword, iron plate)", len(sess.PendingMenu)) + } + + // Choice 2 is iron plate (level 40, the only row above level 1). + g.handleProductChoice(sess, "2") + + if p.Action == nil { + t.Fatalf("level-50 crafter could not start the level-40 recipe; output=%q", conn.Bytes()) + } + if p.Action.TargetID != "test_iron_plate" { + t.Fatalf("started recipe %q, want test_iron_plate; output=%q", p.Action.TargetID, conn.Bytes()) + } + }) +} diff --git a/internal/game/hardening_craft_message_item_test.go b/internal/game/hardening_craft_message_item_test.go new file mode 100644 index 0000000..b516852 --- /dev/null +++ b/internal/game/hardening_craft_message_item_test.go @@ -0,0 +1,85 @@ +package game + +// Regression: a %i success message must name the ingredient alternative +// the player actually used. Production resolved the message AFTER +// consumeIngredients, so p.HasItem was false for everything consumed and +// resolveCraftVar fell back to the entry's first alternative — naming the +// item the player never touched when an entry lists several (bucket vs jug +// of water). + +import ( + "strings" + "testing" + + "thehouseoficarus/internal/net" + "thehouseoficarus/internal/player" +) + +const craftMessageOutputYAML = `id: test_water_potion +name: test water potion +stackable: false +value: 3 +craft: + - type: crafting + ingredients: + - items: + - bucket_of_water + - jug_of_water + quantity: 1 + byproducts: + - empty_bucket + - empty_jug + success_message: "You mix the %i1 into a %n." + output_qty: 1 +` + +func TestCraftSuccessMessageNamesConsumedAlternative(t *testing.T) { + dir := t.TempDir() + seedWorld(t, dir) + writeDataFile(t, dir, "items", "bucket_of_water.yaml", "id: bucket_of_water\nname: bucket of water\nstackable: false\nvalue: 1\n") + writeDataFile(t, dir, "items", "jug_of_water.yaml", "id: jug_of_water\nname: jug of water\nstackable: false\nvalue: 1\n") + writeDataFile(t, dir, "items", "empty_bucket.yaml", "id: empty_bucket\nname: empty bucket\nstackable: false\nvalue: 1\n") + writeDataFile(t, dir, "items", "empty_jug.yaml", "id: empty_jug\nname: empty jug\nstackable: false\nvalue: 1\n") + writeDataFile(t, dir, "items", "test_water_potion.yaml", craftMessageOutputYAML) + + g := newHardeningGame(t, dir) + + g.RunOnOwner(func() { + p := player.New("alice") + p.RoomID = 1 + // Only the jug is present: the entered alternative is bucket_of_water, + // so a post-consumption p.HasItem fallback would name the bucket. + p.SetInvSlot(0, &player.InventorySlot{ItemID: "jug_of_water", Quantity: 1}) + + conn := newChanConn() + sess := &net.Session{Conn: conn, Player: p} + sess.SetState(net.StateGame) + + def, err := g.ItemStore.Load("test_water_potion") + if err != nil { + t.Fatalf("load recipe: %v", err) + } + + g.startProductionFromItem(sess, p, def, 1) + if p.Action == nil { + t.Fatalf("production did not start; output=%q", conn.Bytes()) + } + g.advanceProduction(sess, p) // phase 0 -> 1 + g.advanceProduction(sess, p) // phase 1: consume + produce + + if got := p.CountItem("test_water_potion"); got != 1 { + t.Fatalf("potion = %d, want 1; output=%q", got, conn.Bytes()) + } + if got := p.CountItem("empty_jug"); got != 1 { + t.Fatalf("empty_jug = %d, want 1 (byproduct for the consumed jug)", got) + } + + out := string(conn.Bytes()) + if !strings.Contains(out, "jug of water") { + t.Errorf("success message does not name the consumed jug of water: %q", out) + } + if strings.Contains(out, "bucket of water") { + t.Errorf("success message names the bucket the player never used: %q", out) + } + }) +} diff --git a/internal/game/hardening_credit_cap_test.go b/internal/game/hardening_credit_cap_test.go new file mode 100644 index 0000000..872c746 --- /dev/null +++ b/internal/game/hardening_credit_cap_test.go @@ -0,0 +1,105 @@ +package game + +// Regression: credit pickups must never destroy value at the economy cap. +// Only the balance headroom below casino.MaxCredits may be removed from the +// ground; the remainder stays there. The previous code removed the whole +// ground stack and then let grantCredits clamp the excess away. + +import ( + "strings" + "testing" + + "thehouseoficarus/internal/casino" + "thehouseoficarus/internal/net" +) + +func TestPickupCreditsAtCapLeavesRemainderOnGround(t *testing.T) { + h := newHarness(t) + h.seedAccount("alice") + p := h.seedCharacter("alice", "alice") + h.g.RunOnOwner(func() { + p.Credits = casino.MaxCredits - 5 + h.g.World.AddGroundItem(p.RoomID, "credits", 20) + sess := &net.Session{Conn: newChanConn(), Player: p} + sess.SetState(net.StateGame) + + h.g.pickupCredits(sess, p, "credits") + + if p.Credits != casino.MaxCredits { + t.Errorf("balance = %d, want the cap %d", p.Credits, casino.MaxCredits) + } + if got := h.g.World.GroundItems(p.RoomID)["credits"]; got != 15 { + t.Errorf("ground credits = %d, want 15 left behind", got) + } + }) +} + +func TestPickupCreditsAtCapDestroysNothing(t *testing.T) { + h := newHarness(t) + h.seedAccount("alice") + p := h.seedCharacter("alice", "alice") + h.g.RunOnOwner(func() { + p.Credits = casino.MaxCredits + h.g.World.AddGroundItem(p.RoomID, "credits", 20) + sess := &net.Session{Conn: newChanConn(), Player: p} + sess.SetState(net.StateGame) + + h.g.pickupCredits(sess, p, "credits") + + if p.Credits != casino.MaxCredits { + t.Errorf("balance = %d, want the cap %d", p.Credits, casino.MaxCredits) + } + if got := h.g.World.GroundItems(p.RoomID)["credits"]; got != 20 { + t.Errorf("ground credits = %d, want the full stack to survive", got) + } + if out := string(sess.Conn.(*chanConn).Bytes()); !strings.Contains(out, "can't hold any more credits") { + t.Errorf("expected a cap message, got %q", out) + } + }) +} + +func TestGetAllCreditsAtCapLeavesRemainderOnGround(t *testing.T) { + h := newHarness(t) + h.seedAccount("alice") + p := h.seedCharacter("alice", "alice") + h.g.RunOnOwner(func() { + p.Credits = casino.MaxCredits - 5 + h.g.World.AddGroundItem(p.RoomID, "credits", 20) + sess := &net.Session{Conn: newChanConn(), Player: p} + sess.SetState(net.StateGame) + + h.g.doGetAll(sess) + + if p.Credits != casino.MaxCredits { + t.Errorf("balance = %d, want the cap %d", p.Credits, casino.MaxCredits) + } + if got := h.g.World.GroundItems(p.RoomID)["credits"]; got != 15 { + t.Errorf("ground credits = %d, want 15 left behind", got) + } + }) +} + +func TestGetAllCreditsAtCapReportsShortfall(t *testing.T) { + h := newHarness(t) + h.seedAccount("alice") + p := h.seedCharacter("alice", "alice") + h.g.RunOnOwner(func() { + p.Credits = casino.MaxCredits + h.g.World.AddGroundItem(p.RoomID, "credits", 20) + sess := &net.Session{Conn: newChanConn(), Player: p} + sess.SetState(net.StateGame) + + h.g.doGetAll(sess) + + if got := h.g.World.GroundItems(p.RoomID)["credits"]; got != 20 { + t.Errorf("ground credits = %d, want the full stack to survive", got) + } + out := string(sess.Conn.(*chanConn).Bytes()) + if !strings.Contains(out, "can't hold any more credits") { + t.Errorf("expected a cap message, got %q", out) + } + if strings.Contains(out, "inventory is full") { + t.Errorf("cap shortfall misreported as a full inventory: %q", out) + } + }) +} diff --git a/internal/game/hardening_economy_durability_test.go b/internal/game/hardening_economy_durability_test.go new file mode 100644 index 0000000..55fff50 --- /dev/null +++ b/internal/game/hardening_economy_durability_test.go @@ -0,0 +1,272 @@ +package game + +// Persistence/economy durability regressions: the economy journal's directory +// entry must be synced, recovery must order payout evidence by the record's +// creation order (not by its last-written line), the recovery checkpoint +// re-anchor must respect the economy cap, the journal-unavailable settlement +// fallback must only resolve the stakes it settles, and a bank credit move +// whose save failed must be refused instead of leaving memory ahead of the +// durable file. + +import ( + "encoding/json" + "fmt" + "os" + "path/filepath" + "testing" + "time" + + "thehouseoficarus/internal/casino" + "thehouseoficarus/internal/player" + + "gopkg.in/yaml.v3" +) + +// waitForConnText blocks until the session's writer flushes text containing +// substr; the writer goroutine is asynchronous. +func waitForConnText(t *testing.T, conn *chanConn, substr string) { + t.Helper() + deadline := time.Now().Add(2 * time.Second) + for !conn.Contains(substr) && time.Now().Before(deadline) { + time.Sleep(time.Millisecond) + } + if !conn.Contains(substr) { + t.Errorf("expected %q in session output; got=%s", substr, conn.Bytes()) + } +} + +// TestRecoveryIgnoresRewrittenOlderPayoutAsStakeEvidence: payout evidence for +// a stake is ordered by the payout's CREATION sequence, not by the line of its +// latest write. A payout created before the stake that is rewritten afterwards +// (a settle marker, or a recovery checkpoint re-anchor) must not be read as +// the stake's settlement — otherwise recovery settles the stake without +// refunding it and the wager is silently lost. +func TestRecoveryIgnoresRewrittenOlderPayoutAsStakeEvidence(t *testing.T) { + h := newHarness(t) + h.seedAccount("alice") + p := h.seedCharacter("alice", "alice") + h.g.RunOnOwner(func() { + p.Credits = 1000 + if err := h.g.AccountStore.SaveCharacter(p); err != nil { + t.Error(err) + return + } + // An older payout record (created first) ... + old, err := h.g.Economy.start("payout", p, 0, 20, 1000, 1020, 0, 0) + if err != nil { + t.Error(err) + return + } + // ... then the stake ... + if _, err := h.g.Economy.start("wager", p, 0, 50, 1000, 950, 0, 0); err != nil { + t.Error(err) + return + } + // ... and finally a later write of the OLD payout lands after the + // stake's line, so line order alone would call it the stake's + // settlement. + if !h.g.Economy.settleDurable(old) { + t.Error("the older payout's settle marker could not be journaled") + return + } + // The stake's debit is durable; the crashed round is recoverable. + p.Credits = 950 + if err := h.g.AccountStore.SaveCharacter(p); err != nil { + t.Error(err) + return + } + h.g.recoverEconomy() + }) + loaded, err := h.g.AccountStore.LoadCharacter("alice") + if err != nil { + t.Fatal(err) + } + if loaded.Credits != 1000 { + t.Errorf("balance after recovery = %d, want 1000: a payout created before the stake must not settle it (the stake is refunded, not silently kept)", + loaded.Credits) + } + if open := h.g.Economy.mustLoadOpen(t); len(open) != 0 { + t.Errorf("records left open after recovery: %+v", open) + } +} + +// TestRecoveryReanchorKeepsCheckpointsWithinCreditCap: when the economy cap +// blocks a payout's re-application, the re-anchored checkpoints must stay +// within the cap. An impossible post-state above the cap can never be read as +// durability, and the record is owed at the current balance: it must keep +// reading that way across repeated recoveries instead of being settled as +// "already durable". Legacy records without the tracked-evidence flag take +// this checkpoint-compensating path. +func TestRecoveryReanchorKeepsCheckpointsWithinCreditCap(t *testing.T) { + dir := t.TempDir() + seedWorld(t, dir) + + hash, _ := player.HashPassword("pw") + accData, _ := yaml.Marshal(&player.Account{Name: "alice", PasswordHash: hash}) + writeSeedFile(t, filepath.Join(dir, "players", "accounts", "alice.yaml"), accData) + p := player.New("alice") + p.Credits = casino.MaxCredits - 30 + p.Account = "alice" + pData, _ := yaml.Marshal(p) + writeSeedFile(t, filepath.Join(dir, "players", "characters", "alice.yaml"), pData) + + // Two identical open payouts of 20 at the cap: the first can be + // re-applied, the second cannot. Each record is its own journal line. + var journal []byte + for i := 0; i < 2; i++ { + line, err := json.Marshal(economyRecord{ + ID: fmt.Sprintf("unpaid-%d", i), Player: "alice", Kind: "payout", + Credit: 20, Status: "open", + CreditsBefore: casino.MaxCredits - 30, CreditsAfter: casino.MaxCredits - 10, + }) + if err != nil { + t.Fatal(err) + } + journal = append(journal, line...) + journal = append(journal, '\n') + } + journalDir := filepath.Join(dir, "economy") + if err := os.MkdirAll(journalDir, 0700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(journalDir, "journal.jsonl"), journal, 0600); err != nil { + t.Fatal(err) + } + + // The first recovery runs in New. + h := harnessFromDir(t, dir) + for round := 1; round <= 3; round++ { + if round > 1 { + h.g.recoverEconomy() + } + open := h.g.Economy.mustLoadOpen(t) + if len(open) != 1 { + t.Fatalf("recovery %d: the unpaid liability must stay open exactly once, open=%d: %+v", round, len(open), open) + } + for _, rec := range open { + if rec.CreditsBefore > casino.MaxCredits || rec.CreditsAfter > casino.MaxCredits { + t.Fatalf("recovery %d re-anchored %s outside the economy cap: before=%d after=%d cap=%d", + round, rec.ID, rec.CreditsBefore, rec.CreditsAfter, casino.MaxCredits) + } + } + } + loaded, err := h.g.AccountStore.LoadCharacter("alice") + if err != nil { + t.Fatal(err) + } + if loaded.Credits != casino.MaxCredits-10 { + t.Fatalf("balance after repeated recovery = %d, want %d (one 20-credit payout applied, the other still owed)", + loaded.Credits, casino.MaxCredits-10) + } +} + +// TestJournalUnavailableSettlementSettlesOnlyItsOwnStakes: when the economy +// journal is unavailable, the character-file fallback records resolution +// evidence only for the stakes the payout settles. A sibling hand's stake is +// settled by its own payout, so a crash before that payout must still refund +// it. +func TestJournalUnavailableSettlementSettlesOnlyItsOwnStakes(t *testing.T) { + h := newHarness(t) + h.seedAccount("alice") + p := h.seedCharacter("alice", "alice") + h.g.RunOnOwner(func() { + p.Credits = 100 + w := casinoWallet{g: h.g, p: p} + // Two sibling stakes (a split, or two spots). The sibling is placed + // first so its post-debit balance is the durable one. + if _, err := w.Wager(10); err != nil { + t.Error(err) + return + } + settled, err := w.Wager(10) + if err != nil { + t.Error(err) + return + } + // The journal breaks for the second hand's push: the payout lands in + // the character file only, and may record evidence only for the + // stake it settles. + failJournalDuring(h, func() { + if err := w.PayStakeCover(10, []string{settled.ID}); err != nil { + t.Errorf("fallback settlement failed: %v", err) + return + } + }) + if p.Credits != 90 { + t.Errorf("after the fallback settlement: credits=%d, want 90", p.Credits) + return + } + h.g.recoverEconomy() + }) + loaded, err := h.g.AccountStore.LoadCharacter("alice") + if err != nil { + t.Fatal(err) + } + if loaded.Credits != 100 { + t.Errorf("balance after recovery = %d, want 100: the sibling hand's stake must be refunded, not resolved by the other hand's settlement", + loaded.Credits) + } + if open := h.g.Economy.mustLoadOpen(t); len(open) != 0 { + t.Errorf("records left open after recovery: %+v", open) + } +} + +// TestBankCreditTransferRefusedWhenSaveFails: a credit move between the wallet +// and the bank that cannot be made durable is reverted and refused. Leaving it +// in memory only would put the balance ahead of the character file, and a +// wager taken from that phantom balance journals checkpoints recovery can +// never match. +func TestBankCreditTransferRefusedWhenSaveFails(t *testing.T) { + h := newHarness(t) + h.seedAccount("alice") + p := h.seedCharacter("alice", "alice") + p.Credits = 100 + p.BankedCredits = 50 + if err := h.g.AccountStore.SaveCharacter(p); err != nil { + t.Fatal(err) + } + sess, conn := h.connect("alice") + + h.g.RunOnOwner(func() { + failingSaveDuring(h.g, h.dir, func() { + h.g.doBankDepositCredits(sess, []string{"deposit", "credits", "40"}) + }) + }) + if sess.Player.Credits != 100 || sess.Player.BankedCredits != 50 { + t.Errorf("a deposit whose save failed must be reverted in memory: credits=%d banked=%d, want 100/50", + sess.Player.Credits, sess.Player.BankedCredits) + } + waitForConnText(t, conn, "nothing was moved") + if loaded, err := h.g.AccountStore.LoadCharacter("alice"); err != nil { + t.Error(err) + } else if loaded.Credits != 100 || loaded.BankedCredits != 50 { + t.Errorf("durable balances after a refused deposit: credits=%d banked=%d, want 100/50", + loaded.Credits, loaded.BankedCredits) + } + + h.g.RunOnOwner(func() { + failingSaveDuring(h.g, h.dir, func() { + h.g.doBankWithdrawCredits(sess, []string{"withdraw", "credits", "10"}) + }) + }) + if sess.Player.Credits != 100 || sess.Player.BankedCredits != 50 { + t.Errorf("a withdrawal whose save failed must be reverted in memory: credits=%d banked=%d, want 100/50", + sess.Player.Credits, sess.Player.BankedCredits) + } + waitForConnText(t, conn, "nothing was moved") + + // A durable move still works. + h.g.RunOnOwner(func() { + h.g.doBankWithdrawCredits(sess, []string{"withdraw", "credits", "10"}) + }) + if sess.Player.Credits != 110 || sess.Player.BankedCredits != 40 { + t.Errorf("successful withdrawal: credits=%d banked=%d, want 110/40", + sess.Player.Credits, sess.Player.BankedCredits) + } + if loaded, err := h.g.AccountStore.LoadCharacter("alice"); err != nil { + t.Error(err) + } else if loaded.Credits != 110 || loaded.BankedCredits != 40 { + t.Errorf("durable balances after a successful withdrawal: credits=%d banked=%d, want 110/40", + loaded.Credits, loaded.BankedCredits) + } +} diff --git a/internal/game/hardening_harvest_yield_test.go b/internal/game/hardening_harvest_yield_test.go new file mode 100644 index 0000000..8bfdc6a --- /dev/null +++ b/internal/game/hardening_harvest_yield_test.go @@ -0,0 +1,168 @@ +package game + +// Regression: a harvest that cannot be stored must not consume the crop or +// report a yield that was never delivered. Stackable products merge into an +// existing stack (so they fit a full inventory) but need their own slot +// otherwise; non-stackable products need one slot per unit, and the reported +// yield must match what was actually placed. + +import ( + "path/filepath" + "strings" + "testing" + + "thehouseoficarus/internal/behavior" + "thehouseoficarus/internal/net" + "thehouseoficarus/internal/player" +) + +const ( + hardeningCropPrefix = "farm_allot_1" + hardeningCropSeed = "test_seed" + hardeningCropID = "test_potato" +) + +// readyHarvest installs the patch flags a ready crop has and an in-flight +// harvest action for hardeningCropPrefix, matching what cmd_farm.go queues. +func readyHarvest(p *player.Player, minYield, maxYield int) { + p.EnsureFlags() + p.Flags[hardeningCropPrefix+"_weeds"] = false + p.Flags[hardeningCropPrefix+"_seed"] = hardeningCropSeed + p.Flags[hardeningCropPrefix+"_stage"] = 4 + p.Flags[hardeningCropPrefix+"_watered"] = false + p.Flags[hardeningCropPrefix+"_diseased"] = false + p.Flags[hardeningCropPrefix+"_dead"] = false + p.Flags[hardeningCropPrefix+"_ready"] = true + p.Action = &behavior.Action{ + Type: "harvest", + Data: &behavior.FarmData{ + Prefix: hardeningCropPrefix, + SeedID: hardeningCropSeed, + Product: hardeningCropID, + MinYield: float64(minYield), + MaxYield: float64(maxYield), + }, + } +} + +func TestHarvestStackableFullInventoryKeepsCrop(t *testing.T) { + h := newHarness(t) + h.seedAccount("alice") + p := h.seedCharacter("alice", "alice") + writeSeedFile(t, filepath.Join(h.dir, "items", hardeningCropID+".yaml"), + []byte("id: test_potato\nname: test potato\nstackable: true\n")) + sess := &net.Session{Conn: newChanConn(), Player: p} + sess.SetState(net.StateGame) + h.g.RunOnOwner(func() { + h.g.ItemStore.Reload(h.dir) + for i := 0; i < 28; i++ { + p.SetInvSlot(i, &player.InventorySlot{ItemID: "filler", Quantity: 1}) + } + readyHarvest(p, 1, 1) + + h.g.advanceHarvest(sess, p) + + if got := p.CountItem(hardeningCropID); got != 0 { + t.Errorf("harvest stored %d potatoes with no room", got) + } + if ready, _ := p.Flags[hardeningCropPrefix+"_ready"].(bool); !ready { + t.Error("crop was consumed even though nothing could be stored") + } + if seed, _ := p.Flags[hardeningCropPrefix+"_seed"].(string); seed != hardeningCropSeed { + t.Errorf("patch seed = %q, want it left planted", seed) + } + out := string(sess.Conn.(*chanConn).Bytes()) + if strings.Contains(out, "You harvest") { + t.Errorf("reported a yield that was not delivered: %q", out) + } + if !strings.Contains(out, "too full") { + t.Errorf("expected an inventory-full message, got %q", out) + } + }) +} + +func TestHarvestStackableMergesIntoExistingStackWhenFull(t *testing.T) { + h := newHarness(t) + h.seedAccount("alice") + p := h.seedCharacter("alice", "alice") + writeSeedFile(t, filepath.Join(h.dir, "items", hardeningCropID+".yaml"), + []byte("id: test_potato\nname: test potato\nstackable: true\n")) + sess := &net.Session{Conn: newChanConn(), Player: p} + sess.SetState(net.StateGame) + h.g.RunOnOwner(func() { + h.g.ItemStore.Reload(h.dir) + for i := 0; i < 28; i++ { + p.SetInvSlot(i, &player.InventorySlot{ItemID: "filler", Quantity: 1}) + } + p.SetInvSlot(5, &player.InventorySlot{ItemID: hardeningCropID, Quantity: 4}) + readyHarvest(p, 1, 1) + + h.g.advanceHarvest(sess, p) + + if got := p.CountItem(hardeningCropID); got != 5 { + t.Errorf("stackable harvest did not merge into the existing stack: have %d, want 5", got) + } + if ready, _ := p.Flags[hardeningCropPrefix+"_ready"].(bool); ready { + t.Error("patch still ready after a successful harvest") + } + if out := string(sess.Conn.(*chanConn).Bytes()); !strings.Contains(out, "You harvest 1") { + t.Errorf("expected the delivered yield in the message, got %q", out) + } + }) +} + +func TestHarvestNonStackableReportsPlacedYield(t *testing.T) { + h := newHarness(t) + h.seedAccount("alice") + p := h.seedCharacter("alice", "alice") + writeSeedFile(t, filepath.Join(h.dir, "items", hardeningCropID+".yaml"), + []byte("id: test_potato\nname: test potato\n")) + sess := &net.Session{Conn: newChanConn(), Player: p} + sess.SetState(net.StateGame) + h.g.RunOnOwner(func() { + h.g.ItemStore.Reload(h.dir) + for i := 0; i < 26; i++ { + p.SetInvSlot(i, &player.InventorySlot{ItemID: "filler", Quantity: 1}) + } + readyHarvest(p, 5, 5) + + h.g.advanceHarvest(sess, p) + + if got := p.CountItem(hardeningCropID); got != 2 { + t.Fatalf("placed %d potatoes, want 2 (two free slots)", got) + } + out := string(sess.Conn.(*chanConn).Bytes()) + if !strings.Contains(out, "You harvest 2 ") { + t.Errorf("reported yield must match what was placed: %q", out) + } + }) +} + +func TestHarvestNonStackableFullInventoryKeepsCrop(t *testing.T) { + h := newHarness(t) + h.seedAccount("alice") + p := h.seedCharacter("alice", "alice") + writeSeedFile(t, filepath.Join(h.dir, "items", hardeningCropID+".yaml"), + []byte("id: test_potato\nname: test potato\n")) + sess := &net.Session{Conn: newChanConn(), Player: p} + sess.SetState(net.StateGame) + h.g.RunOnOwner(func() { + h.g.ItemStore.Reload(h.dir) + for i := 0; i < 28; i++ { + p.SetInvSlot(i, &player.InventorySlot{ItemID: "filler", Quantity: 1}) + } + readyHarvest(p, 5, 5) + + h.g.advanceHarvest(sess, p) + + if got := p.CountItem(hardeningCropID); got != 0 { + t.Errorf("harvest stored %d potatoes with no room", got) + } + if ready, _ := p.Flags[hardeningCropPrefix+"_ready"].(bool); !ready { + t.Error("crop was consumed even though nothing could be stored") + } + if out := string(sess.Conn.(*chanConn).Bytes()); strings.Contains(out, "You harvest") { + t.Errorf("reported a yield that was not delivered: %q", out) + } + }) +} diff --git a/internal/game/hardening_login_validation_test.go b/internal/game/hardening_login_validation_test.go new file mode 100644 index 0000000..fa60497 --- /dev/null +++ b/internal/game/hardening_login_validation_test.go @@ -0,0 +1,58 @@ +package game + +// Regression: the account name typed at the prompt must be validated with +// player.ValidName BEFORE it reaches the filesystem. The name is later stored +// on the session and reused to build account/character paths, so an unvalidated +// "../../" style name must never be joined into a path, even if such a path +// happens to resolve to an existing file. + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "thehouseoficarus/internal/net" +) + +func TestAccountNameValidatedBeforeFilesystemAccess(t *testing.T) { + h := newHarness(t) + h.seedAccount("alice") + + // A file that "../escape" resolves to once joined under players/accounts/ + // (filepath.Join cleans the traversal). Before the fix this name reached + // FindAccount first and was accepted into the password state. + escapePath := filepath.Join(h.dir, "players", "escape.yaml") + if err := os.WriteFile(escapePath, []byte("name: escape\npassword_hash: x\n"), 0600); err != nil { + t.Fatal(err) + } + + conn := newChanConn() + sess := &net.Session{Conn: conn} + sess.SetState(net.StateAccountName) + + h.g.RunOnOwner(func() { h.g.handleAccountName(sess, "../escape") }) + + if got := sess.State(); got != net.StateAccountName { + t.Fatalf("traversal account name accepted: state = %v, want StateAccountName", got) + } + if sess.Account != nil { + t.Fatalf("rejected name bound to the session: %+v", sess.Account) + } + if out := string(conn.Bytes()); !strings.Contains(out, "name must be") { + t.Fatalf("missing name-validation message; output = %q", out) + } + + // A valid existing name still reaches the password prompt. + conn2 := newChanConn() + valid := &net.Session{Conn: conn2} + valid.SetState(net.StateAccountName) + h.g.RunOnOwner(func() { h.g.handleAccountName(valid, "alice") }) + + if got := valid.State(); got != net.StatePassword { + t.Fatalf("valid account name rejected: state = %v, want StatePassword", got) + } + if valid.Account == nil || valid.Account.Name != "alice" { + t.Fatalf("valid account name not bound: %+v", valid.Account) + } +} diff --git a/internal/game/hardening_owner_panic_test.go b/internal/game/hardening_owner_panic_test.go new file mode 100644 index 0000000..aa1070b --- /dev/null +++ b/internal/game/hardening_owner_panic_test.go @@ -0,0 +1,162 @@ +package game + +// Crash-safety regressions for the owner goroutine and the account-menu +// handlers. The owner goroutine is the single execution authority for live +// game state, so an unrecovered panic there (from command parsing, a tick +// system, or a session whose account/player has gone away) takes down every +// session and all unsaved state. These tests pin both halves of the fix: the +// loop contains panics, and the menu handlers refuse to dereference a nil +// account. + +import ( + "strings" + "testing" + + "thehouseoficarus/internal/net" + "thehouseoficarus/internal/player" +) + +// TestOwnerLoopSurvivesPanickingTask proves a panicking task is contained and +// counted, and that the loop keeps draining the queue afterwards. +func TestOwnerLoopSurvivesPanickingTask(t *testing.T) { + h := newHarness(t) + + before := h.g.owner.panics.Load() + h.g.RunOnOwner(func() { panic("boom") }) + // Game.run releases its caller from the task's OWN defer (close(task)), + // which unwinds before runTask's deferred recover runs. The panic counter + // is incremented by that recover, so reading it right here could observe + // before+0. Submit a no-op task and wait for it: the owner drains tasks in + // FIFO order, so the panicking task's runTask — recover and counter + // increment included — has fully settled before the no-op task executes. + h.barrier() + if got := h.g.owner.panics.Load(); got != before+1 { + t.Fatalf("panic counter = %d, want %d", got, before+1) + } + + ran := false + h.g.RunOnOwner(func() { ran = true }) + if !ran { + t.Fatal("owner loop did not execute a task submitted after a panic") + } +} + +// TestOwnerSessionInputPanicDoesNotKillLoop reproduces the production crash +// path: a session in StateGame with no Player (a departed session whose +// connection goroutine is still dispatching buffered input) dereferences a nil +// Player. The owner must log and contain the panic instead of taking the +// process down with it. +func TestOwnerSessionInputPanicDoesNotKillLoop(t *testing.T) { + h := newHarness(t) + sess := &net.Session{Conn: newChanConn()} + sess.SetState(net.StateGame) + + h.g.HandleSessionOnOwner(sess, "look") + + if got := h.g.owner.panics.Load(); got == 0 { + t.Fatal("expected the nil-Player dispatch to panic and be contained") + } + h.barrier() // the owner loop is still alive and serving tasks +} + +// assertResetToAccountName requires a session to be back at the account-name +// prompt with no account bound. +func assertResetToAccountName(t *testing.T, sess *net.Session, conn *chanConn) { + t.Helper() + if got := sess.State(); got != net.StateAccountName { + t.Fatalf("state = %v, want StateAccountName", got) + } + if sess.Account != nil { + t.Fatalf("account not cleared: %+v", sess.Account) + } + if out := string(conn.Bytes()); !strings.Contains(out, "Account name: ") { + t.Fatalf("missing account-name prompt; output = %q", out) + } +} + +// TestNilAccountHandlersResetToAccountName drives every menu-state handler with +// a session whose account is nil (purged, or republished from another session) +// and requires each to fall back to the account-name prompt instead of +// dereferencing the nil account. The handlers run on the test goroutine so an +// unguarded dereference fails loudly here rather than being absorbed by the +// owner loop's panic barrier. +func TestNilAccountHandlersResetToAccountName(t *testing.T) { + h := newHarness(t) + + cases := []struct { + name string + state net.SessionState + input string + run func(*Game, *net.Session, string) + }{ + {"menu-connect", net.StateMenu, "c", (*Game).handleMenu}, + {"menu-list", net.StateMenu, "l", (*Game).handleMenu}, + {"menu-new", net.StateMenu, "n", (*Game).handleMenu}, + {"menu-rename-account", net.StateMenu, "a", (*Game).handleMenu}, + {"menu-rename-char", net.StateMenu, "r", (*Game).handleMenu}, + {"menu-delete-char", net.StateMenu, "d", (*Game).handleMenu}, + {"menu-purge", net.StateMenu, "p", (*Game).handleMenu}, + {"menu-default", net.StateMenu, "zzz", (*Game).handleMenu}, + {"color-choice", net.StateColorChoice, "y", (*Game).handleColorChoice}, + {"new-account-confirm", net.StateNewAccount, "pw", (*Game).handleNewAccount}, + {"new-char-name", net.StateNewCharName, "bob", (*Game).handleNewCharName}, + {"rename-account", net.StateRenameAccount, "bob", (*Game).handleRenameAccount}, + {"rename-char", net.StateRenameChar, "1", (*Game).handleRenameChar}, + {"rename-char-name", net.StateRenameCharName, "bob", (*Game).handleRenameCharName}, + {"delete-char", net.StateDeleteChar, "1", (*Game).handleDeleteChar}, + {"purge-account", net.StatePurgeAccount, "PURGE bob", (*Game).handlePurgeAccount}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + conn := newChanConn() + sess := &net.Session{Conn: conn} + sess.SetState(tc.state) + tc.run(h.g, sess, tc.input) + assertResetToAccountName(t, sess, conn) + }) + } + + t.Run("show-menu", func(t *testing.T) { + conn := newChanConn() + sess := &net.Session{Conn: conn} + sess.SetState(net.StateGame) + h.g.showMenu(sess) + assertResetToAccountName(t, sess, conn) + }) + + t.Run("connect-character", func(t *testing.T) { + conn := newChanConn() + sess := &net.Session{Conn: conn} + sess.SetState(net.StateMenu) + h.g.connectCharacter(sess, "bob") + assertResetToAccountName(t, sess, conn) + }) + + t.Run("apply-password-auth", func(t *testing.T) { + conn := newChanConn() + sess := &net.Session{Conn: conn} + sess.SetState(net.StatePassword) + h.g.applyPasswordAuth(sess, passwordAuth{ok: true, stored: "stale-hash"}) + assertResetToAccountName(t, sess, conn) + }) +} + +// TestCommandsTolerateNilAccount covers the in-game command handlers that +// persist to the account: with the account gone they must still work for the +// live player instead of faulting on the nil account. +func TestCommandsTolerateNilAccount(t *testing.T) { + h := newHarness(t) + sess := &net.Session{Conn: newChanConn(), Player: player.New("alice")} + sess.SetState(net.StateGame) + + h.g.doOption(sess, "unicode off") + if sess.Player.Options["unicode"] != false { + t.Fatalf("unicode option not applied to the player: %v", sess.Player.Options["unicode"]) + } + + h.g.doColor(sess, "room_name D0") + h.g.saveAccountColor(sess, "room_name", "D0") + h.g.doAlias(sess, []string{"x", "look"}) + h.g.doUnalias(sess, []string{"x"}) +} diff --git a/internal/game/hardening_pickup_quality_test.go b/internal/game/hardening_pickup_quality_test.go new file mode 100644 index 0000000..135b5a6 --- /dev/null +++ b/internal/game/hardening_pickup_quality_test.go @@ -0,0 +1,123 @@ +package game + +// Regression: a picked-up stack of a quality-bearing stackable item may only +// merge into a slot with the same Quality/MaxQuality. Otherwise it takes a +// free slot, mirroring withdrawBankItem in cmd_bank.go, so a merge can never +// silently destroy the picked-up stack's quality. + +import ( + "path/filepath" + "testing" + + "thehouseoficarus/internal/net" + "thehouseoficarus/internal/player" +) + +const hardeningGemID = "test_gem" + +func seedQualityGem(t *testing.T, h *harness) { + t.Helper() + writeSeedFile(t, filepath.Join(h.dir, "items", hardeningGemID+".yaml"), + []byte("id: test_gem\nname: test gem\nstackable: true\nmax_quality: 5\n")) +} + +// gemSlots returns every inventory slot holding the test gem. +func gemSlots(p *player.Player) map[int]*player.InventorySlot { + found := make(map[int]*player.InventorySlot) + for i := 0; i < 28; i++ { + if slot := p.InvSlot(i); slot != nil && slot.ItemID == hardeningGemID { + found[i] = slot + } + } + return found +} + +func TestPickupMismatchedQualityUsesFreeSlot(t *testing.T) { + h := newHarness(t) + h.seedAccount("alice") + p := h.seedCharacter("alice", "alice") + seedQualityGem(t, h) + sess := &net.Session{Conn: newChanConn(), Player: p} + sess.SetState(net.StateGame) + h.g.RunOnOwner(func() { + h.g.ItemStore.Reload(h.dir) + p.SetInvSlot(0, &player.InventorySlot{ItemID: hardeningGemID, Quantity: 10, Quality: 1, MaxQuality: 5}) + h.g.World.AddGroundItem(p.RoomID, hardeningGemID, 5) + + h.g.doGet(sess, "test gem") + + slots := gemSlots(p) + if len(slots) != 2 { + t.Fatalf("gem slots = %d, want the pickup in its own slot: %v", len(slots), slots) + } + if low := p.InvSlot(0); low.Quantity != 10 || low.Quality != 1 { + t.Errorf("existing low-quality stack changed: %+v", low) + } + found := false + for i, slot := range slots { + if i == 0 { + continue + } + found = true + if slot.Quantity != 5 || slot.Quality != 5 || slot.MaxQuality != 5 { + t.Errorf("picked-up stack lost its properties: %+v", slot) + } + } + if !found { + t.Error("pickup did not take a free slot") + } + if got := h.g.World.GroundItems(p.RoomID)[hardeningGemID]; got != 0 { + t.Errorf("ground gems = %d, want 0", got) + } + }) +} + +func TestPickupMatchingQualityMergesIntoStack(t *testing.T) { + h := newHarness(t) + h.seedAccount("alice") + p := h.seedCharacter("alice", "alice") + seedQualityGem(t, h) + sess := &net.Session{Conn: newChanConn(), Player: p} + sess.SetState(net.StateGame) + h.g.RunOnOwner(func() { + h.g.ItemStore.Reload(h.dir) + p.SetInvSlot(0, &player.InventorySlot{ItemID: hardeningGemID, Quantity: 10, Quality: 5, MaxQuality: 5}) + h.g.World.AddGroundItem(p.RoomID, hardeningGemID, 5) + + h.g.doGet(sess, "test gem") + + slots := gemSlots(p) + if len(slots) != 1 { + t.Fatalf("matching-quality pickup must merge, slots = %v", slots) + } + if slot := p.InvSlot(0); slot.Quantity != 15 || slot.Quality != 5 || slot.MaxQuality != 5 { + t.Errorf("merged stack = %+v, want quantity 15 at quality 5", slot) + } + }) +} + +func TestPickupMismatchedQualityFullInventoryRefused(t *testing.T) { + h := newHarness(t) + h.seedAccount("alice") + p := h.seedCharacter("alice", "alice") + seedQualityGem(t, h) + sess := &net.Session{Conn: newChanConn(), Player: p} + sess.SetState(net.StateGame) + h.g.RunOnOwner(func() { + h.g.ItemStore.Reload(h.dir) + for i := 0; i < 28; i++ { + p.SetInvSlot(i, &player.InventorySlot{ItemID: "filler", Quantity: 1}) + } + p.SetInvSlot(0, &player.InventorySlot{ItemID: hardeningGemID, Quantity: 10, Quality: 1, MaxQuality: 5}) + h.g.World.AddGroundItem(p.RoomID, hardeningGemID, 5) + + h.g.doGet(sess, "test gem") + + if slot := p.InvSlot(0); slot.Quantity != 10 || slot.Quality != 1 { + t.Errorf("a refused pickup must leave the mismatched stack alone: %+v", slot) + } + if got := h.g.World.GroundItems(p.RoomID)[hardeningGemID]; got != 5 { + t.Errorf("ground gems = %d, want the stack left in place", got) + } + }) +} diff --git a/internal/game/hardening_purge_session_reset_test.go b/internal/game/hardening_purge_session_reset_test.go new file mode 100644 index 0000000..c3462a6 --- /dev/null +++ b/internal/game/hardening_purge_session_reset_test.go @@ -0,0 +1,50 @@ +package game + +// Regression (P0): a purged account must leave every one of its sessions in a +// safe protocol state. A queued-mode Close only drops the transport after the +// writer drains its outbox, so input already buffered in the connection can +// still be dispatched to the owner after the purge; a session left in the +// character menu with a nil account then dereferences sess.Account and kills +// the owner goroutine (and with it every session and all unsaved state). + +import ( + "testing" + + "thehouseoficarus/internal/net" +) + +func TestPurgeResetsEveryAccountSession(t *testing.T) { + h := newHarness(t) + h.seedAccount("alice") + h.seedCharacter("alice", "alice") + + other := h.connectMenu("alice") + purger := h.connectMenu("alice") + + h.g.RunOnOwner(func() { h.g.handlePurgeAccount(purger, "PURGE alice") }) + + for name, sess := range map[string]*net.Session{"other": other, "purger": purger} { + if got := sess.State(); got != net.StateAccountName { + t.Errorf("%s session state = %v, want StateAccountName", name, got) + } + if sess.Account != nil { + t.Errorf("%s session retained the purged account: %+v", name, sess.Account) + } + } + + // Input already buffered in either transport is still routed to the owner: + // it must land on the account-name prompt instead of faulting. + before := h.g.owner.panics.Load() + h.g.RunOnOwner(func() { + h.g.handleMenu(other, "c") + h.g.handleMenu(other, "l") + h.g.handlePurgeAccount(purger, "PURGE alice") + h.g.handleNewCharName(purger, "bob") + }) + if got := h.g.owner.panics.Load(); got != before { + t.Fatalf("post-purge input panicked the owner (panic count %d -> %d)", before, got) + } + if got := other.State(); got != net.StateAccountName { + t.Errorf("state = %v after post-purge input, want StateAccountName", got) + } +} diff --git a/internal/game/hardening_queue_cleanup_test.go b/internal/game/hardening_queue_cleanup_test.go new file mode 100644 index 0000000..95b96bf --- /dev/null +++ b/internal/game/hardening_queue_cleanup_test.go @@ -0,0 +1,139 @@ +package game + +// Session-lifecycle regressions for the per-player command queues: a session +// that logs out, quits, or disconnects must not leave queued commands behind. +// Leftovers retain the departed *net.Session (and its transport) and replay +// against the next session that logs in as the same character — including the +// nil-Player crash path in ProcessQueuedCommands. + +import ( + "reflect" + "testing" + "time" + + "thehouseoficarus/internal/net" +) + +// TestClearPlayerDropsAllQueues proves ClearPlayer empties the free, active, +// and consume queues, and that detachSession applies it. +func TestClearPlayerDropsAllQueues(t *testing.T) { + h := newHarness(t) + h.seedAccount("alice") + h.seedCharacter("alice", "alice") + sess, _ := h.connect("alice") + + h.g.RunOnOwner(func() { + h.g.queue.EnqueueFree("alice", QueuedCommand{Session: sess, Command: "wear", Args: "sword"}) + h.g.queue.EnqueueActive("alice", QueuedCommand{Session: sess, Command: "wear", Args: "sword"}) + h.g.queue.EnqueueConsume("alice", QueuedCommand{Session: sess, Command: "eat", Args: "bread"}) + + h.g.detachSession("alice") + }) + + if cmds := h.g.queue.PeekFree("alice"); len(cmds) != 0 { + t.Errorf("free queue retained %d commands after detach", len(cmds)) + } + if _, ok := h.g.queue.PeekActive("alice"); ok { + t.Error("active queue retained a command after detach") + } + if _, ok := h.g.queue.DrainConsume("alice"); ok { + t.Error("consume queue retained a command after detach") + } +} + +// TestQuitCountdownDropsQueuedCommands covers the logout path in cmd_quit.go: +// the countdown drops the registry entry, so it must drop the queues too. The +// consume slot is asserted because nothing else ever drains it — a leftover +// there survives the logout entirely without the cleanup. +func TestQuitCountdownDropsQueuedCommands(t *testing.T) { + h := newHarness(t) + h.seedAccount("alice") + h.seedCharacter("alice", "alice") + sess, _ := h.connect("alice") + + h.g.RunOnOwner(func() { + h.g.queue.EnqueueConsume("alice", QueuedCommand{ + Session: sess, + Command: "eat", + Args: "bread", + Timestamp: time.Now(), + }) + }) + h.g.HandleSessionOnOwner(sess, "quit") + // "quit" is an active command: it executes on the next tick drain. The + // harness registers no master tick system, so drain it explicitly. + h.g.RunOnOwner(func() { h.g.ProcessQueuedCommands() }) + + deadline := time.Now().Add(5 * time.Second) + for time.Now().Before(deadline) && sess.State() != net.StateMenu { + time.Sleep(5 * time.Millisecond) + } + if got := sess.State(); got != net.StateMenu { + t.Fatalf("logout countdown never completed (state %v)", got) + } + + if qc, ok := h.g.queue.DrainConsume("alice"); ok { + t.Errorf("consume command survived logout and would replay on re-login: %+v", qc) + } + if cmds := h.g.queue.PeekFree("alice"); len(cmds) != 0 { + t.Errorf("free commands survived logout: %d", len(cmds)) + } + if _, ok := h.g.queue.PeekActive("alice"); ok { + t.Error("active command survived logout") + } +} + +// TestProcessQueuedCommandsSkipsStaleSession reproduces the crash path where a +// free command enqueued on a session that has since departed (Player nil) is +// drained while a live session for the same character is in the hub. +func TestProcessQueuedCommandsSkipsStaleSession(t *testing.T) { + h := newHarness(t) + h.seedAccount("alice") + h.seedCharacter("alice", "alice") + h.connect("alice") + + stale := &net.Session{Conn: newChanConn()} + stale.SetState(net.StateGame) // departed session: no Player + + h.g.RunOnOwner(func() { + h.g.queue.EnqueueFree("alice", QueuedCommand{ + Session: stale, + Command: "wear", + Args: "sword", + Timestamp: time.Now(), + }) + }) + + before := h.g.owner.panics.Load() + h.g.RunOnOwner(func() { h.g.ProcessQueuedCommands() }) + if got := h.g.owner.panics.Load(); got != before { + t.Fatalf("stale queued command panicked the owner (panic count %d -> %d)", before, got) + } + if cmds := h.g.queue.PeekFree("alice"); len(cmds) != 0 { + t.Errorf("free queue not drained for the live session: %d commands left", len(cmds)) + } +} + +// TestActivePositionsReturnsSortedNamesAndOneBasedPosition pins the documented +// contract of ActivePositions: names sorted, position 1-indexed, 0 when the +// player holds no active command. +func TestActivePositionsReturnsSortedNamesAndOneBasedPosition(t *testing.T) { + q := NewCommandQueue() + for _, name := range []string{"carol", "alice", "bob"} { + q.EnqueueActive(name, QueuedCommand{Command: "look", Timestamp: time.Now()}) + } + + names, pos := q.ActivePositions("bob") + if want := []string{"alice", "bob", "carol"}; !reflect.DeepEqual(names, want) { + t.Errorf("names = %v, want %v", names, want) + } + if pos != 2 { + t.Errorf("position of bob = %d, want 2", pos) + } + if _, pos := q.ActivePositions("alice"); pos != 1 { + t.Errorf("position of alice = %d, want 1", pos) + } + if _, pos := q.ActivePositions("nobody"); pos != 0 { + t.Errorf("position of an unqueued player = %d, want 0", pos) + } +} diff --git a/internal/game/hardening_quit_bet_warning_test.go b/internal/game/hardening_quit_bet_warning_test.go new file mode 100644 index 0000000..4daef49 --- /dev/null +++ b/internal/game/hardening_quit_bet_warning_test.go @@ -0,0 +1,79 @@ +package game + +// Regression coverage for the quit refusal warning: the departure is refused +// whenever a wager is committed, but only a dealt hand (or a spin in progress) +// is actually forfeited on disconnect -- a stake still in the betting window is +// refunded. The warning must not promise a forfeit in that case. + +import ( + "strings" + "testing" + + "thehouseoficarus/internal/casino" + "thehouseoficarus/internal/player" +) + +// TestQuitRefusalWarningMatchesRefundedBet places a bet with a second seat +// holding the round in the betting window (no card dealt for alice's stake), +// then runs a plain quit. The message must state both outcomes, and the +// refusal must still leave the player seated with no countdown running. +func TestQuitRefusalWarningMatchesRefundedBet(t *testing.T) { + g, sess := quitTestGame(t) + sess.Account = &player.Account{Name: "alice"} + cfg := casino.TableConfig{ID: "blackjack", Game: casino.GameBlackjack, MinBet: 1, MaxBet: 100} + g.Casino.Join(1, cfg, "alice") + g.Casino.Join(1, cfg, "bob") // bob never bets: the window stays open + g.Casino.Bet(1, "blackjack", "alice", "", 10, casinoWallet{g: g, p: sess.Player}) + + conn := sess.Conn.(*casinoTestConn) + conn.data = nil + g.doQuit(sess) + out := string(conn.data) + + if !strings.Contains(out, `"quit now"`) { + t.Fatalf("expected the quit-now reminder, got %q", out) + } + if strings.Contains(out, "your bet will be forfeited") { + t.Fatalf("warning still promises an unconditional forfeit for a betting-window stake: %q", out) + } + if !strings.Contains(out, "forfeit") { + t.Fatalf("warning does not mention the forfeit that a dealt hand suffers: %q", out) + } + if !strings.Contains(out, "refund") { + t.Fatalf("warning does not mention the refund a betting-window stake receives: %q", out) + } + + // Behavior is unchanged: the refusal keeps the seat and the wager. + if _, playing := g.Casino.Participation("alice"); !playing { + t.Fatal("refused quit removed the player from the table") + } + if g.quitResting() { + t.Fatal("refused quit started the rest countdown") + } +} + +// TestQuitNowBettingWindowStakeIsRefunded proves the refund half of the +// warning: after "quit now" runs its countdown to the logout tick, the +// betting-window stake is returned, so the old unconditional forfeit promise +// would have been wrong. +func TestQuitNowBettingWindowStakeIsRefunded(t *testing.T) { + g, sess := quitTestGame(t) + sess.Account = &player.Account{Name: "alice"} + cfg := casino.TableConfig{ID: "blackjack", Game: casino.GameBlackjack, MinBet: 1, MaxBet: 100} + g.Casino.Join(1, cfg, "alice") + g.Casino.Join(1, cfg, "bob") + g.Casino.Bet(1, "blackjack", "alice", "", 10, casinoWallet{g: g, p: sess.Player}) + wallet := sess.Player + + g.doQuitNow(sess) + for i := 0; i < 15 && sess.Player != nil; i++ { + g.Ticks.Tick() + } + + if _, playing := g.Casino.Participation("alice"); playing { + t.Fatal("quit now did not free the seat at the logout tick") + } + if wallet.Credits != 1000 { + t.Fatalf("betting-window stake was not refunded on logout: credits = %d, want 1000", wallet.Credits) + } +} diff --git a/internal/game/hardening_retribution_mob_death_test.go b/internal/game/hardening_retribution_mob_death_test.go new file mode 100644 index 0000000..cf05820 --- /dev/null +++ b/internal/game/hardening_retribution_mob_death_test.go @@ -0,0 +1,90 @@ +package game + +// Regression: Dead Man's Switch retribution can zero a mob inside killPlayer, +// after endCombat has already taken the player-death branch and returned. +// That left the mob at HP 0 in MobStore: filtered out of MobsInRoom (HP > 0) +// and skipped by SeedMobs, it vanished from the room until restart, with no +// drops and no respawn. Retribution kills must route through the same +// mob-death helper the normal attack path uses. + +import ( + "testing" + + "thehouseoficarus/internal/behavior" + "thehouseoficarus/internal/net" + "thehouseoficarus/internal/player" +) + +const retributionMobYAML = `id: retribution_target +name: test target +combat: + aggressive: false + attack_types: + - crush + kind: combat + respawn_ticks: 1 + size: medium + stats: + attack: 1 + defense: 1 + hp: 1 + max_melee_hit: 1 + speed: 5 + strength: 1 +drops: + remains: bones +` + +func TestRetributionKillRunsMobDeathPath(t *testing.T) { + dir := t.TempDir() + seedWorld(t, dir) + writeDataFile(t, dir, "items", "bones.yaml", "id: bones\nname: bones\nstackable: false\nvalue: 1\n") + writeDataFile(t, dir, "techs", "retribution.yaml", + "name: Dead Man's Switch\nlevel: 46\neffects:\n retribution_pct: 0.25\n") + writeDataFile(t, dir, "mobs", "retribution_target.yaml", retributionMobYAML) + + g := newHardeningGame(t, dir) + + g.RunOnOwner(func() { + g.MobStore.ReloadDefs(dir) + def, err := g.MobStore.LoadDef("retribution_target") + if err != nil { + t.Fatalf("load mob def: %v", err) + } + mob := g.MobStore.SpawnTransient(def, &behavior.SpawnMobConfig{ID: "retribution_target"}, 2, "") + // A normal seeded mob, not a trigger despawn: respawn must restore it. + mob.SpawnedByTrigger = false + mob.RespawnTicks = 1 + mob.Drops.Remains = "bones" + mob.HP = 1 + mob.MaxHP = 1 + + p := player.New("alice") + p.RoomID = 2 + p.HP = 0 + p.ActivateTech("retribution") + + conn := newChanConn() + sess := &net.Session{Conn: conn, Player: p} + sess.SetState(net.StateGame) + + g.endCombat(sess, p, mob) + + if mob.HP != 0 { + t.Fatalf("mob HP = %d, want 0 after retribution", mob.HP) + } + if ground := g.World.GroundItems(2); ground["bones"] != 1 { + t.Fatalf("retribution kill did not run the drops path: ground=%v", ground) + } + + // The death path schedules a respawn; one tick must restore the mob + // in its home room rather than leave it gone at HP 0. + g.Ticks.Tick() + if mob.HP != mob.MaxHP || mob.HP <= 0 { + t.Fatalf("mob did not respawn: HP=%d MaxHP=%d", mob.HP, mob.MaxHP) + } + if mob.RoomID != 2 { + t.Fatalf("mob respawned in room %d, want 2", mob.RoomID) + } + }) +} diff --git a/internal/game/hardening_security_test.go b/internal/game/hardening_security_test.go new file mode 100644 index 0000000..edf3c29 --- /dev/null +++ b/internal/game/hardening_security_test.go @@ -0,0 +1,100 @@ +package game + +// Regression tests for the security-hardening stage: +// - swapid swaps the two room files through a journal transaction and leaves +// no transient temp file behind +// - the admin privilege check re-reads the on-disk flag instead of trusting +// the cached sess.Account.Admin copy + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +func TestSwapIDSwapsRoomFilesTransactionally(t *testing.T) { + h := newHarness(t) + h.seedAccount("admin") + h.seedCharacter("admin", "admin") + acc, err := h.g.AccountStore.LoadAccount("admin") + if err != nil { + t.Fatal(err) + } + acc.Admin = true + if err := h.g.AccountStore.SaveAccount(acc); err != nil { + t.Fatal(err) + } + sess, _ := h.connect("admin") + + h.g.RunOnOwner(func() { h.g.executeSwapID(sess, []string{"1", "2"}, "swapid 1 2") }) + + r1, err := os.ReadFile(filepath.Join(h.dir, "rooms", "1.yaml")) + if err != nil { + t.Fatal(err) + } + r2, err := os.ReadFile(filepath.Join(h.dir, "rooms", "2.yaml")) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(r1), "name: North") { + t.Fatalf("room 1 not swapped:\n%s", r1) + } + if !strings.Contains(string(r2), "name: Central") { + t.Fatalf("room 2 not swapped:\n%s", r2) + } + + // The old implementation renamed through rooms/.yaml.swapid-tmp; the + // transaction-based swap must not leave one behind. + entries, err := os.ReadDir(filepath.Join(h.dir, "rooms")) + if err != nil { + t.Fatal(err) + } + for _, e := range entries { + if strings.Contains(e.Name(), "swapid-tmp") { + t.Fatalf("leftover swap temp file: %s", e.Name()) + } + } + + // A committed transaction removes its journal. + if jentries, err := os.ReadDir(filepath.Join(h.dir, ".journal")); err == nil { + for _, e := range jentries { + t.Fatalf("leftover journal after successful swap: %s", e.Name()) + } + } +} + +func TestCheckAdminReReadsDiskFlag(t *testing.T) { + h := newHarness(t) + h.seedAccount("alice") + h.seedCharacter("alice", "alice") + acc, err := h.g.AccountStore.LoadAccount("alice") + if err != nil { + t.Fatal(err) + } + acc.Admin = true + if err := h.g.AccountStore.SaveAccount(acc); err != nil { + t.Fatal(err) + } + sess, _ := h.connect("alice") + + var got bool + h.g.RunOnOwner(func() { got = h.g.checkAdmin(sess) }) + if !got { + t.Fatal("admin session not recognized") + } + + // Revoke admin on disk; the cached session must no longer be privileged. + revoked, err := h.g.AccountStore.LoadAccount("alice") + if err != nil { + t.Fatal(err) + } + revoked.Admin = false + if err := h.g.AccountStore.SaveAccount(revoked); err != nil { + t.Fatal(err) + } + h.g.RunOnOwner(func() { got = h.g.checkAdmin(sess) }) + if got { + t.Fatal("stale cached admin flag was trusted after disk revocation") + } +} diff --git a/internal/game/hardening_shipped_content_test.go b/internal/game/hardening_shipped_content_test.go new file mode 100644 index 0000000..9379395 --- /dev/null +++ b/internal/game/hardening_shipped_content_test.go @@ -0,0 +1,60 @@ +package game + +// Regression: pins the shipped content fixed alongside the validator work. +// The iron-bar smelt formula must improve with level (it previously capped at +// 50% at every level, destroying half the ore forever), and the pastry/pizza +// dough combines must return their empty containers like bread dough does. + +import ( + "path/filepath" + "testing" + + "thehouseoficarus/internal/item" +) + +func TestShippedIronBarSmeltFormulaImprovesWithLevel(t *testing.T) { + store := item.NewItemStore(filepath.Join("..", "..", "data")) + def, err := store.Load("iron_bar") + if err != nil { + t.Fatalf("load iron_bar: %v", err) + } + c := def.FirstCraft() + if c == nil { + t.Fatal("iron_bar has no craft recipe") + } + if c.Success == nil { + t.Fatal("iron_bar must define a success formula (fail_message/smelting failure otherwise unreachable)") + } + if c.Success.PerLevel <= 0 { + t.Errorf("iron_bar success per_level = %v, want > 0 (failure must become rarer with level)", c.Success.PerLevel) + } + if c.Success.Cap <= c.Success.Base { + t.Errorf("iron_bar success base=%v cap=%v, want cap > base (a permanent fixed chance is the defect)", + c.Success.Base, c.Success.Cap) + } +} + +func TestShippedDoughRecipesReturnEmptyContainers(t *testing.T) { + store := item.NewItemStore(filepath.Join("..", "..", "data")) + for _, id := range []string{"pastry_dough", "pizza_dough"} { + def, err := store.Load(id) + if err != nil { + t.Fatalf("load %s: %v", id, err) + } + c := def.FirstCraft() + if c == nil { + t.Fatalf("%s has no craft recipe", id) + } + got := map[string]bool{} + for _, e := range c.Ingredients { + for _, bp := range e.Byproducts { + got[bp] = true + } + } + for _, want := range []string{"empty_pot", "empty_bucket"} { + if !got[want] { + t.Errorf("%s must return %s as a byproduct so the container is not destroyed", id, want) + } + } + } +} diff --git a/internal/game/hardening_shipped_data_validation_test.go b/internal/game/hardening_shipped_data_validation_test.go new file mode 100644 index 0000000..dee464b --- /dev/null +++ b/internal/game/hardening_shipped_data_validation_test.go @@ -0,0 +1,205 @@ +package game + +// Regression: the authored data/ tree must pass the full startup validator in +// CI, not only when a server boots. Before this test the suite ran just +// validateKnownFields; validate.Run (the twelve semantic/reference checks +// behind game.ValidateAndLog) was reachable only from game.New, so broken +// content (dangling references, bad craft byproducts, unreachable fail +// branches, invalid coords) reached a running server before CI noticed. +// +// The gate fails on every ERROR, and on WARNs that are known-fatal dead data +// (see fatalShippedDataWarnings). It deliberately does NOT fail on every WARN: +// production only escalates ERROR (validate.LogIssues) and game.New never +// refuses to boot on a WARN, and many WARNs are by-design informational (an +// orphan room while content is authored, "has no name" placeholders, ignored +// shared-drop-table hints). Failing on all of them made CI stricter than the +// server and blocked legitimate content edits. + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "thehouseoficarus/internal/config" + "thehouseoficarus/internal/item" + "thehouseoficarus/internal/object" + "thehouseoficarus/internal/validate" + "thehouseoficarus/internal/world" +) + +// fatalShippedDataWarnings lists WARN message substrings that still fail the +// shipped-data gate. It keeps protection against dead fail data returning: a +// declared fail product or fail_message on a recipe with no success formula can +// never be produced or shown (see validate.craftFailureReachable). The second +// entry keeps protection against a failable recipe that declares no fail +// product, which consumes materials and yields nothing; every shipped failable +// recipe returns one of its own ingredients. +var fatalShippedDataWarnings = []string{ + "is unreachable", + "failure destroys materials with no fail product", +} + +// shippedDataIssueFails reports whether issue must fail the shipped-data gate: +// every ERROR, plus a WARN matching fatalShippedDataWarnings. +func shippedDataIssueFails(issue validate.Issue) bool { + if issue.Level == "ERROR" { + return true + } + for _, fatal := range fatalShippedDataWarnings { + if strings.Contains(issue.Message, fatal) { + return true + } + } + return false +} + +// TestShippedDataPassesStartupValidation runs validate.Run over ../../data +// using a Source equivalent to game.ValidateAndLog's (the same stores, techs +// and configured root rooms). +func TestShippedDataPassesStartupValidation(t *testing.T) { + dataDir := filepath.Join("..", "..", "data") + cfg, err := config.Load(filepath.Join("..", "..", "config.yaml")) + if err != nil { + t.Fatalf("load config.yaml: %v", err) + } + + // A minimal, side-effect-free Game: the stores and config the validator + // reads, without game.New's character migration / journal recovery (which + // would write to the shipped tree). + g := &Game{ + Deps: Deps{ + World: world.New(dataDir), + ObjectStore: object.NewObjectStore(dataDir), + ItemStore: item.NewItemStore(dataDir), + MobStore: world.NewMobStore(dataDir), + CourseStore: NewCourseStore(dataDir), + DataDir: dataDir, + }, + ValidationConfig: cfg.StartupValidation, + } + g.CourseStore.SetWorld(g.World) + g.CourseStore.LoadAll() + if err := g.LoadTechs(); err != nil { + t.Fatalf("load techs: %v", err) + } + + for _, issue := range validate.Run(g.validationSource()) { + if shippedDataIssueFails(issue) { + t.Errorf("[%s] %s: %s", issue.Level, issue.Type, issue.Message) + } + } +} + +// TestShippedDataGateCatchesDeadFailDataAndAllowsBenignWarns pins the gate's +// severity policy: an ERROR or dead craft-fail WARN fails, a benign +// content WARN (an orphan room) does not. +func TestShippedDataGateCatchesDeadFailDataAndAllowsBenignWarns(t *testing.T) { + errIssue := validate.Issue{Level: "ERROR", Type: "reference", Message: "Room 1: spawns nonexistent item \"ghost\""} + if !shippedDataIssueFails(errIssue) { + t.Error("an ERROR must fail the shipped-data gate") + } + + deadFail := validate.Issue{ + Level: "WARN", + Type: "semantic", + Message: `Item "x" (craft: cooking): fail product "burnt_fish" is unreachable — success is always 100% (add a success formula)`, + } + if !shippedDataIssueFails(deadFail) { + t.Error("dead fail data must still fail the shipped-data gate") + } + + orphan := validate.Issue{Level: "WARN", Type: "integrity", Message: "Orphan rooms (no exit leads to them): 1099"} + if shippedDataIssueFails(orphan) { + t.Error("a benign content WARN must not fail the shipped-data gate") + } +} + +// TestShippedDataGateTripsOnDeadFailRecipe proves the gate still trips on a +// real validator WARN emitted for a recipe whose fail branch can never run, +// not merely on a hand-built Issue. +func TestShippedDataGateTripsOnDeadFailRecipe(t *testing.T) { + dataDir := t.TempDir() + writeShippedDataFile(t, filepath.Join(dataDir, "items", "dead_fail.yaml"), `name: dead fail +craft: + - type: cooking + fail: dead_fail + fail_message: You burn it. + ingredients: + - items: [dead_fail] + quantity: 1 +`) + + src := validate.Source{ + DataDir: dataDir, + Items: item.NewItemStore(dataDir), + Objects: object.NewObjectStore(dataDir), + Mobs: world.NewMobStore(dataDir), + World: world.New(dataDir), + } + + found := false + for _, issue := range validate.Run(src) { + if !strings.Contains(issue.Message, "is unreachable") { + continue + } + found = true + if !shippedDataIssueFails(issue) { + t.Fatalf("dead-fail WARN must fail the shipped-data gate, got: %+v", issue) + } + } + if !found { + t.Fatal("expected a dead-fail WARN for a recipe with an unreachable fail branch") + } +} + +// TestShippedSmeltingBarsRecoverMaterialOnFailure verifies fix for the +// regression where the bars gained a success formula but no fail product: a +// lost roll consumed every ingredient and returned nothing. A smelt that can +// fail must return one of its own ingredient ids. +func TestShippedSmeltingBarsRecoverMaterialOnFailure(t *testing.T) { + dataDir := filepath.Join("..", "..", "data") + defs, err := item.NewItemStore(dataDir).LoadAll() + if err != nil { + t.Fatalf("load items: %v", err) + } + + smelts := 0 + for _, def := range defs { + for ci := range def.Craft { + c := &def.Craft[ci] + if c.Subtype != "smelting" || c.Success == nil { + continue + } + smelts++ + if c.Fail == "" { + t.Errorf("Item %q: a failing smelt consumes every ingredient but declares no fail product", def.ID) + continue + } + returned := false + for _, e := range c.Ingredients { + for _, id := range e.Items { + if id == c.Fail { + returned = true + } + } + } + if !returned { + t.Errorf("Item %q: fail product %q is not one of the recipe's own ingredients", def.ID, c.Fail) + } + } + } + if smelts == 0 { + t.Fatal("expected at least one smelting recipe with a success formula") + } +} + +func writeShippedDataFile(t *testing.T, path, body string) { + t.Helper() + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, []byte(body), 0o644); err != nil { + t.Fatal(err) + } +} diff --git a/internal/game/hardening_shutdown_save_error_test.go b/internal/game/hardening_shutdown_save_error_test.go new file mode 100644 index 0000000..c75f6fc --- /dev/null +++ b/internal/game/hardening_shutdown_save_error_test.go @@ -0,0 +1,63 @@ +package game + +// Regression coverage for the shutdown final-save report: runShutdown records +// a failed saveAllOnlineCharacters on the Game, and ShutdownSaveError exposes +// it so main can exit non-zero instead of losing online state silently. + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +// TestShutdownSaveErrorReportsFailedFinalSave makes the character save fail +// (the character file is replaced by a directory, so the atomic replace cannot +// land) and requires the shutdown sequence to record the failure on the Game. +func TestShutdownSaveErrorReportsFailedFinalSave(t *testing.T) { + h := newHarness(t) + h.seedAccount("alice") + h.seedCharacter("alice", "alice") + h.connect("alice") + + charPath := filepath.Join(h.dir, "players", "characters", "alice.yaml") + if err := os.Remove(charPath); err != nil { + t.Fatalf("remove character file: %v", err) + } + if err := os.Mkdir(charPath, 0o755); err != nil { + t.Fatalf("block character path: %v", err) + } + + h.g.runShutdown() + + err := h.g.ShutdownSaveError() + if err == nil { + t.Fatal("a failed final save was not recorded") + } + if !strings.Contains(err.Error(), "alice") { + t.Fatalf("shutdown save error does not name the unsaved character: %v", err) + } +} + +// TestShutdownSaveErrorNilWhenFinalSaveSucceeds pins the other direction: +// nothing is reported when every online character was persisted, so main does +// not exit non-zero on a clean shutdown. +func TestShutdownSaveErrorNilWhenFinalSaveSucceeds(t *testing.T) { + h := newHarness(t) + h.seedAccount("alice") + h.seedCharacter("alice", "alice") + h.connect("alice") + + if err := h.g.ShutdownSaveError(); err != nil { + t.Fatalf("ShutdownSaveError before any shutdown = %v, want nil", err) + } + + h.g.runShutdown() + + if err := h.g.ShutdownSaveError(); err != nil { + t.Fatalf("clean shutdown recorded a save failure: %v", err) + } + if _, err := h.g.AccountStore.LoadCharacter("alice"); err != nil { + t.Fatalf("online character was not saved: %v", err) + } +} diff --git a/internal/game/hardening_tick_lifecycle_test.go b/internal/game/hardening_tick_lifecycle_test.go new file mode 100644 index 0000000..b5b3bb9 --- /dev/null +++ b/internal/game/hardening_tick_lifecycle_test.go @@ -0,0 +1,291 @@ +package game + +// Tick and lifecycle liveness regressions: a pending aggro retry must never +// start combat for a departed player, an in-combat disconnect must not pin the +// character in the registry forever, per-tick systems must stay safe with a +// nil Hub and must advance in a reproducible order, and the per-tick session +// snapshots must not drop the wander broadcasts they replaced. + +import ( + "fmt" + "path/filepath" + "strings" + "testing" + + "thehouseoficarus/internal/behavior" + "thehouseoficarus/internal/net" + "thehouseoficarus/internal/player" +) + +// TestFireTickNilHubIsSafe guards the mandatory tick-system guard: FireTick +// reaches cancelStokers -> g.Hub.PlayersInRoom when a fire burns out, and the +// master tick subscriber can fire before SetHub during startup. +func TestFireTickNilHubIsSafe(t *testing.T) { + t.Run("nil_hub_burning_fire", func(t *testing.T) { + g := escapeTestGame() + // Quality 1 burns out on this tick, which is the path that touches + // the hub; without the guard it panics on g.Hub == nil. + g.World.AddObjInstance(1, "fire", 1) + g.Hub = nil + + defer func() { + if r := recover(); r != nil { + t.Fatalf("FireTick panicked with a nil Hub: %v", r) + } + }() + g.FireTick() + }) + + t.Run("hub_present_still_burns_out", func(t *testing.T) { + h := newHarness(t) + h.g.RunOnOwner(func() { + h.g.World.AddObjInstance(1, "fire", 1) + h.g.FireTick() + if fires := h.g.World.AllFireStates(); len(fires) != 0 { + t.Errorf("fire should have burned out, %d states remain", len(fires)) + } + }) + }) +} + +// TestSequenceTickFiresInDeterministicOrder guards the sorted iteration over +// g.sequences: map iteration randomly chose which simultaneous sequence +// advanced first, contradicting the determinism contract in +// building_guide/tick_order.md. +func TestSequenceTickFiresInDeterministicOrder(t *testing.T) { + g := escapeTestGame() + g.sequences = make(map[string]*sequence) + g.loggedInChars = make(map[string]*net.Session) + + const rounds = 32 + for round := 0; round < rounds; round++ { + conn := newChanConn() + p := player.New(fmt.Sprintf("seq_order_%d", round)) + p.RoomID = 1 + sess := &net.Session{Player: p, Conn: conn} + sess.SetState(net.StateGame) + g.loggedInChars[p.Name] = sess + + // Inserted in reverse key order: only sorting can make the advance + // order stable. + g.seqMu.Lock() + g.sequences["z_second"] = &sequence{ + sess: sess, player: p, playerName: p.Name, roomID: p.RoomID, + steps: []behavior.Step{{Messages: []string{"ORDER-MARKER-SECOND"}}}, + scope: scopePlayer, key: "z_second", + } + g.sequences["a_first"] = &sequence{ + sess: sess, player: p, playerName: p.Name, roomID: p.RoomID, + steps: []behavior.Step{{Messages: []string{"ORDER-MARKER-FIRST"}}}, + scope: scopePlayer, key: "a_first", + } + g.seqMu.Unlock() + + g.SequenceTick() + + out := string(conn.Bytes()) + first, second := strings.Index(out, "ORDER-MARKER-FIRST"), strings.Index(out, "ORDER-MARKER-SECOND") + if first < 0 || second < 0 { + t.Fatalf("round %d: both sequences should have fired, got %q", round, out) + } + if first > second { + t.Fatalf("round %d: sequences advanced in map order instead of key order: %q", round, out) + } + } +} + +// TestWanderTickBroadcastsWithSingleSessionSnapshot guards the hoisted +// AllSessions snapshot: the mob leave/enter broadcasts to other rooms must +// still fire after the per-move snapshot was replaced by one per system. +func TestWanderTickBroadcastsWithSingleSessionSnapshot(t *testing.T) { + h := newHarness(t) + writeSeedFile(t, filepath.Join(h.dir, "mobs", "wanderer.yaml"), []byte( + "id: wanderer\nname: wanderer\ncombat:\n kind: combat\n stats:\n hp: 1\n")) + + // Sessions without a write queue: broadcasts reach the transport + // synchronously with the tick, so the assertions need no polling. + aliceConn := wanderTestSession(h, "alice", 1) + bobConn := wanderTestSession(h, "bob", 2) + + var mobRoom int + h.g.RunOnOwner(func() { + // Room 1 has a north exit to room 2 (see seedWorld). + h.g.MobStore.ReloadDefs(h.dir) + def, err := h.g.MobStore.LoadDef("wanderer") + if err != nil { + t.Errorf("load seeded mob: %v", err) + return + } + inst := h.g.MobStore.SpawnTransient(def, &behavior.SpawnMobConfig{ID: "wanderer"}, 1, "") + if inst.HP <= 0 { + t.Errorf("seeded mob has no HP (%d); it would be skipped", inst.HP) + return + } + inst.WanderInterval = 1 + inst.WanderTickCounter = 1 + inst.WanderRooms = []int{2} + + h.g.WanderTick() + mobRoom = inst.RoomID + }) + + if mobRoom != 2 { + t.Fatalf("mob did not wander into room 2 (room %d)", mobRoom) + } + if sessions := h.g.Hub.AllSessions(); len(sessions) != 2 { + t.Fatalf("snapshot changed hub membership: %d sessions", len(sessions)) + } + + leaving := string(aliceConn.Bytes()) + entering := string(bobConn.Bytes()) + if !strings.Contains(leaving, "wanderer") || !strings.Contains(leaving, "moves away.") { + t.Errorf("departing room did not get the leave broadcast: %q", leaving) + } + if !strings.Contains(entering, "wanderer") || !strings.Contains(entering, "drifts in.") { + t.Errorf("arrival room did not get the enter broadcast: %q", entering) + } + if strings.Contains(leaving, "drifts in.") { + t.Errorf("enter broadcast leaked into the departing room: %q", leaving) + } + if strings.Contains(entering, "moves away.") { + t.Errorf("leave broadcast leaked into the arrival room: %q", entering) + } +} + +// wanderTestSession registers a live in-game session for a fresh character in +// roomID, bypassing the login menu and the write queue. +func wanderTestSession(h *harness, name string, roomID int) *chanConn { + h.t.Helper() + p := player.New(name) + p.RoomID = roomID + conn := newChanConn() + sess := &net.Session{Conn: conn, Player: p} + sess.SetState(net.StateGame) + h.hub.Add(sess) + h.hub.EnterRoom(sess, roomID) + return conn +} + +// TestAggroRetryStopsForDepartedPlayer guards keepRolling's liveness guard: a +// pending aggro timer outlives its session, and must never start combat for a +// player who has logged out, disconnected, or re-logged in elsewhere. +func TestAggroRetryStopsForDepartedPlayer(t *testing.T) { + t.Run("player_detached", func(t *testing.T) { + defer patchAggroRand(0.5)() + g := escapeTestGame() + spawnTestMob(g, 100) + p := player.New("aggro_ghost") + p.RoomID = 100 + sess := escapeTestSession(p) + + g.checkAggro(sess) + if g.Combat.Get(p.Name) != nil { + t.Fatal("inline roll should have missed (val=0.5)") + } + // The logout path nils the session's player before the retry fires. + sess.Player = nil + rollAggroRand = func() float64 { return 0.0 } + g.Ticks.Tick() + + if cs := g.Combat.Get(p.Name); cs != nil { + t.Fatalf("combat started for a departed player: %+v", cs) + } + }) + + t.Run("session_disconnecting", func(t *testing.T) { + defer patchAggroRand(0.5)() + g := escapeTestGame() + spawnTestMob(g, 150) + p := player.New("aggro_offline") + p.RoomID = 150 + sess := escapeTestSession(p) + + g.checkAggro(sess) + sess.StartDisconnect(10) + rollAggroRand = func() float64 { return 0.0 } + g.Ticks.Tick() + + if g.Combat.Get(p.Name) != nil { + t.Error("combat started for a session whose transport is gone") + } + }) + + t.Run("replaced_by_new_session", func(t *testing.T) { + defer patchAggroRand(0.5)() + g := escapeTestGame() + spawnTestMob(g, 200) + p := player.New("aggro_relogin") + p.RoomID = 200 + sess := escapeTestSession(p) + + g.checkAggro(sess) + // The same character logged in again on another connection. + g.loggedInChars = map[string]*net.Session{p.Name: escapeTestSession(p)} + rollAggroRand = func() float64 { return 0.0 } + g.Ticks.Tick() + + if g.Combat.Get(p.Name) != nil { + t.Error("combat started for the retired session of a re-logged-in character") + } + }) + + t.Run("live_session_still_rolls", func(t *testing.T) { + defer patchAggroRand(0.5)() + g := escapeTestGame() + spawnTestMob(g, 300) + p := player.New("aggro_live") + p.RoomID = 300 + sess := escapeTestSession(p) + g.loggedInChars = map[string]*net.Session{p.Name: sess} + + g.checkAggro(sess) + rollAggroRand = func() float64 { return 0.0 } + g.Ticks.Tick() + + if g.Combat.Get(p.Name) == nil { + t.Error("the live session's retry should still start combat") + } + }) +} + +// TestDisconnectTickCombatDeferralIsBounded guards the disconnect countdown: +// an unresolvable fight used to freeze it forever, so the session (and the +// character's "logged in elsewhere" lock) never cleared. +func TestDisconnectTickCombatDeferralIsBounded(t *testing.T) { + h := newHarness(t) + h.seedAccount("alice") + h.seedCharacter("alice", "alice") + sess, _ := h.connect("alice") + p := sess.Player + + var sessionsAfterFirstTick, remaining int + h.g.RunOnOwner(func() { + sess.StartDisconnect(1) + // A fight that never resolves (no subscriber ever ends it). + h.g.Combat.Enter(p.Name, "skeleton") + + h.g.DisconnectTick() + sessionsAfterFirstTick = len(h.g.Hub.AllSessions()) + + for i := 0; i < disconnectCombatGraceTicks+5 && len(h.g.Hub.AllSessions()) > 0; i++ { + h.g.DisconnectTick() + } + remaining = len(h.g.Hub.AllSessions()) + }) + + if sessionsAfterFirstTick != 1 { + t.Fatalf("in-combat session should be deferred on the first tick, %d sessions remain", sessionsAfterFirstTick) + } + if remaining != 0 { + t.Fatalf("in-combat disconnect never completed: %d sessions remain", remaining) + } + if _, err := h.g.AccountStore.LoadCharacter("alice"); err != nil { + t.Fatalf("character was not saved before the hard remove: %v", err) + } + h.g.charsMu.Lock() + _, stillRegistered := h.g.loggedInChars["alice"] + h.g.charsMu.Unlock() + if stillRegistered { + t.Error("character registry entry survived the hard remove") + } +} diff --git a/internal/game/owner.go b/internal/game/owner.go index 9c7be64..d4d3839 100644 --- a/internal/game/owner.go +++ b/internal/game/owner.go @@ -1,7 +1,9 @@ package game import ( + "log" "runtime" + "runtime/debug" "strconv" "strings" "sync" @@ -39,6 +41,10 @@ type owner struct { // detects itself — an "owner is busy" flag would wrongly let outside // callers bypass the queue while a task is in flight. goid atomic.Uint64 + // panics counts the task panics the loop has contained (see runTask). + // A nonzero value means a command, tick callback, or disconnect handler + // panicked and was logged instead of killing the process. + panics atomic.Uint64 } const ownerTaskQueue = 4096 @@ -57,13 +63,34 @@ func (o *owner) loop() { o.goid.Store(currentGoroutineID()) defer close(o.done) for task := range o.tasks { - task() + o.runTask(task) // A queue slot freed up; wake one blocked submitter waiting for - // room (see enqueue). + // room (see enqueue). The signal MUST be sent while holding o.mu: + // a submitter can otherwise observe a full queue, watch this loop + // free a slot and signal, and only then reach cond.Wait() — the + // signal is lost and the submitter sleeps with capacity available. + o.mu.Lock() o.cond.Signal() + o.mu.Unlock() } } +// runTask executes one queued task behind a panic barrier. A panic in a +// command, tick callback, or disconnect handler must not kill the owner +// goroutine: that goroutine is the only execution authority for live game +// state, so its death takes down every session and all unsaved state with +// it. The panic is logged with a stack trace and counted (see panics) so a +// storm is visible, and the loop keeps draining the queue. +func (o *owner) runTask(task func()) { + defer func() { + if r := recover(); r != nil { + o.panics.Add(1) + log.Printf("owner: recovered panic in task: %v\n%s", r, debug.Stack()) + } + }() + task() +} + // submit enqueues without blocking. It returns false when the owner has // stopped or the queue is full; callers that need guaranteed delivery use // enqueue instead. @@ -132,8 +159,10 @@ func (g *Game) run(fn func()) { } task := make(chan struct{}) if !o.enqueue(func() { + // Deferred so a panic contained by runTask still releases the + // caller blocked on <-task below instead of hanging it forever. + defer close(task) fn() - close(task) }) { // The owner loop has stopped (graceful shutdown in progress). // Admissions are closed: DROP the task. Executing it on the @@ -146,10 +175,25 @@ func (g *Game) run(fn func()) { <-task } +// adminTaskPanic is the value RunAdminOnOwner re-raises on its caller's +// goroutine when the owner loop contained a panic in an editor task. It is a +// distinct value so a caller can tell a contained task panic from the +// shutdown refusal that a false result means. +const adminTaskPanic = "owner: admin task panicked; the owner loop contained it, see the log for the stack trace" + // RunAdminOnOwner synchronously executes one complete editor request. Unlike // RunOnOwner it rejects work during shutdown and NEVER falls back to executing // on the HTTP goroutine when the owner has stopped. Call only from outside the // owner; runtime callbacks within fn must call owner-only methods directly. +// +// A panic inside fn is contained by the owner loop's panic barrier (runTask +// logs and counts it and the loop keeps draining), but it is NOT reported as a +// false result: false means "the owner refused the work", which the admin +// middleware renders as 503 "game is shutting down" - a wrong diagnosis for a +// task that panicked, and one that hides a real internal error. Instead the +// panic is re-raised on the CALLER's goroutine, so the admin HTTP handler's +// own panic recovery (admin.ownerMiddleware preserves net/http's) answers an +// internal error while the owner loop stays untouched. func (g *Game) RunAdminOnOwner(fn func()) bool { if g.isShuttingDown() { return false @@ -167,13 +211,21 @@ func (g *Game) RunAdminOnOwner(fn func()) bool { return accepted } done := make(chan struct{}) + // finished is written on the owner goroutine before close(done), so the + // caller reads it race-free after <-done. A deferred close(done) releases + // the caller whether the task completed or panicked. + finished := false if !g.owner.enqueue(func() { + defer close(done) task() - close(done) + finished = true }) { return false } <-done + if !finished { + panic(adminTaskPanic) + } return accepted } diff --git a/internal/game/polish_craft_fail_delivery_test.go b/internal/game/polish_craft_fail_delivery_test.go new file mode 100644 index 0000000..f16e2a9 --- /dev/null +++ b/internal/game/polish_craft_fail_delivery_test.go @@ -0,0 +1,111 @@ +package game + +// Regression: failed-craft delivery used FirstFreeSlot only, while the +// pre-roll fit gate (craftFailFits -> craftOutputFits) counts a matching +// stack. A stackable fail replacement could therefore be destroyed on a full +// inventory even though the gate promised it would fit. Delivery must merge +// like craft/harvest/pickup does. + +import ( + "path/filepath" + "testing" + + "thehouseoficarus/internal/behavior" + "thehouseoficarus/internal/item" + "thehouseoficarus/internal/net" + "thehouseoficarus/internal/player" +) + +const polishFailRecipeYAML = `id: polish_out +name: polish out +stackable: true +craft: + - type: smithing + subtype: smelting + fail: polish_ash + ingredients: + - items: + - polish_ore + quantity: 1 + success: + base: 0 + cap: 0 + per_level: 0 + output_qty: 1 + ticks_per_cycle: 1 +` + +func seedPolishFailItems(t *testing.T, dir string) { + t.Helper() + writeSeedFile(t, filepath.Join(dir, "items", "polish_out.yaml"), []byte(polishFailRecipeYAML)) + writeSeedFile(t, filepath.Join(dir, "items", "polish_ash.yaml"), + []byte("id: polish_ash\nname: polish ash\nstackable: true\nvalue: 1\n")) + writeSeedFile(t, filepath.Join(dir, "items", "polish_ore.yaml"), + []byte("id: polish_ore\nname: polish ore\nstackable: true\nvalue: 1\n")) +} + +func TestPlaceCraftFailMergesIntoMatchingStack(t *testing.T) { + h := newHarness(t) + seedPolishFailItems(t, h.dir) + p := player.New("alice") + + h.g.RunOnOwner(func() { + h.g.ItemStore.Reload(h.dir) + p.SetInvSlot(0, &player.InventorySlot{ItemID: "polish_ash", Quantity: 3}) + fillInventoryExcept(p) + + h.g.placeCraftFail(p, &item.CraftDef{Fail: "polish_ash"}) + if slot := p.InvSlot(0); slot == nil || slot.Quantity != 4 { + t.Fatalf("fail replacement was not merged into the matching stack: %+v", p.InvSlot(0)) + } + }) +} + +func TestCraftFailureMergesFailStackWhenFull(t *testing.T) { + h := newHarness(t) + h.seedAccount("alice") + p := h.seedCharacter("alice", "alice") + seedPolishFailItems(t, h.dir) + + sess := &net.Session{Conn: newChanConn(), Player: p} + sess.SetState(net.StateGame) + + h.g.RunOnOwner(func() { + h.g.ItemStore.Reload(h.dir) + // Full inventory: a matching output stack, a matching fail stack and + // the ingredient. Consuming the ingredient frees its slot, so the old + // FirstFreeSlot delivery landed the fail item there as a second stack. + p.SetInvSlot(0, &player.InventorySlot{ItemID: "polish_ash", Quantity: 1}) + p.SetInvSlot(1, &player.InventorySlot{ItemID: "polish_ore", Quantity: 1}) + p.SetInvSlot(2, &player.InventorySlot{ItemID: "polish_out", Quantity: 1}) + for i := 3; i < 28; i++ { + p.SetInvSlot(i, &player.InventorySlot{ItemID: "filler", Quantity: 1}) + } + + p.Action = &behavior.Action{ + Type: behavior.TypeSmelt, + TargetID: "polish_out", + Data: &behavior.ProductionData{ItemID: "polish_out", Phase: 1, Remaining: 1}, + } + h.g.advanceProduction(sess, p) + + if got := p.CountItem("polish_ore"); got != 0 { + t.Errorf("ingredient = %d, want 0 (consumed)", got) + } + if got := p.CountItem("polish_ash"); got != 2 { + t.Errorf("fail product = %d, want 2", got) + } + stacks := 0 + for i := 0; i < 28; i++ { + if s := p.InvSlot(i); s != nil && s.ItemID == "polish_ash" { + stacks++ + } + } + if stacks != 1 { + t.Errorf("fail product was delivered as %d stacks, want 1 merged stack", stacks) + } + if slot := p.InvSlot(0); slot == nil || slot.Quantity != 2 { + t.Errorf("matching fail stack = %+v, want quantity 2", p.InvSlot(0)) + } + }) +} diff --git a/internal/game/polish_shop_merge_test.go b/internal/game/polish_shop_merge_test.go new file mode 100644 index 0000000..8851464 --- /dev/null +++ b/internal/game/polish_shop_merge_test.go @@ -0,0 +1,55 @@ +package game + +// Regression: giveShopItems used to merge a purchase into any inventory slot +// holding the same item id, ignoring Quality/MaxQuality, while every other +// merge site (craft, harvest, ammo return, pickup) merges only into a stack +// with the properties a fresh stack gets (stackProps). A purchase could +// therefore merge into a differently qualified stack. shopInvCapacity had the +// same blind spot, which mattered because the buy path charges for the +// quantity capacity reports as placeable. + +import ( + "path/filepath" + "testing" + + "thehouseoficarus/internal/player" +) + +func TestGiveShopItemsDoesNotMergeIntoDifferentQualityStack(t *testing.T) { + h := newHarness(t) + h.seedAccount("alice") + p := h.seedCharacter("alice", "alice") + writeSeedFile(t, filepath.Join(h.dir, "items", "polish_shop_gem.yaml"), + []byte("id: polish_shop_gem\nname: polish shop gem\nstackable: true\nmax_quality: 5\nvalue: 1\n")) + + h.g.RunOnOwner(func() { + h.g.ItemStore.Reload(h.dir) + def := mustLoadItem(t, h.g, "polish_shop_gem") + + // A differently qualified stack is not a merge target: with no free + // slot, capacity must report 0 and the purchase must not be placed. + p.SetInvSlot(0, &player.InventorySlot{ItemID: def.ID, Quantity: 10, Quality: 1, MaxQuality: 5}) + fillInventoryExcept(p) + if got := h.g.shopInvCapacity(p, def.ID, def); got != 0 { + t.Fatalf("shopInvCapacity = %d, want 0 (the only stack has a different quality)", got) + } + h.g.giveShopItems(p, def.ID, 3, def) + if slot := p.InvSlot(0); slot.Quantity != 10 || slot.Quality != 1 || slot.MaxQuality != 5 { + t.Errorf("purchase merged into a differently qualified stack: %+v", slot) + } + if got := p.CountItem(def.ID); got != 10 { + t.Errorf("gems carried = %d, want 10 (nothing should have been delivered)", got) + } + + // The same full inventory accepts the purchase once the stack carries + // the properties a fresh stack gets. + p.SetInvSlot(0, &player.InventorySlot{ItemID: def.ID, Quantity: 10, Quality: 5, MaxQuality: 5}) + if got := h.g.shopInvCapacity(p, def.ID, def); got == 0 { + t.Fatalf("shopInvCapacity = %d, want room for a matching stack", got) + } + h.g.giveShopItems(p, def.ID, 3, def) + if slot := p.InvSlot(0); slot.Quantity != 13 || slot.Quality != 5 || slot.MaxQuality != 5 { + t.Errorf("matched stack = %+v, want quantity 13 at quality 5", slot) + } + }) +} diff --git a/internal/game/polish_wear_swap_quality_test.go b/internal/game/polish_wear_swap_quality_test.go new file mode 100644 index 0000000..bb9e996 --- /dev/null +++ b/internal/game/polish_wear_swap_quality_test.go @@ -0,0 +1,140 @@ +package game + +// Regression: swapping worn equipment used to return the displaced item as a +// raw Quality 0/MaxQuality 0 slot and leave its stale EquipQuality entry +// behind. Reachable with the shipped lighter (the only max_quality > 0 item): +// wielding a weapon after the lighter returned it zeroed, and act_burn charged +// the stale equipment record instead of the carried stack. The displaced +// item's equipped quality must move onto the returned slot and out of the map. + +import ( + "path/filepath" + "testing" + + "thehouseoficarus/internal/item" + "thehouseoficarus/internal/net" + "thehouseoficarus/internal/player" +) + +const polishLighterYAML = `id: polish_lighter +name: polish lighter +stackable: false +max_quality: 100 +equipment: + slot: main_hand + type: tool +tool: + type: fire +` + +const polishBladeYAML = `id: polish_blade +name: polish blade +stackable: false +equipment: + slot: main_hand + type: melee_weapon +` + +func seedPolishWearItems(t *testing.T, dir string) { + t.Helper() + writeSeedFile(t, filepath.Join(dir, "items", "polish_lighter.yaml"), []byte(polishLighterYAML)) + writeSeedFile(t, filepath.Join(dir, "items", "polish_blade.yaml"), []byte(polishBladeYAML)) +} + +// carriedInventSlot returns the first inventory slot holding itemID. +func carriedInventSlot(p *player.Player, itemID string) *player.InventorySlot { + for i := 0; i < 28; i++ { + if s := p.InvSlot(i); s != nil && s.ItemID == itemID { + return s + } + } + return nil +} + +func TestWearSwapReturnsEquippedQualityToInventory(t *testing.T) { + h := newHarness(t) + h.seedAccount("alice") + p := h.seedCharacter("alice", "alice") + seedPolishWearItems(t, h.dir) + + sess := &net.Session{Conn: newChanConn(), Player: p} + sess.SetState(net.StateGame) + + h.g.RunOnOwner(func() { + h.g.ItemStore.Reload(h.dir) + p.SetInvSlot(0, h.g.newInventorySlot("polish_lighter", 1)) + + h.g.doWear(sess, "lighter") + if got := p.Equipment[item.SlotMainHand]; got != "polish_lighter" { + t.Fatalf("lighter not equipped: %q", got) + } + // Burn two charges while the lighter is equipped. + for i := 0; i < 2; i++ { + if !h.g.consumeFireTool(sess, p, "polish_lighter") { + t.Fatal("consumeFireTool refused a fresh lighter") + } + } + if got := p.EquipQuality["polish_lighter"]; got != 98 { + t.Fatalf("equipped lighter charge = %d, want 98", got) + } + + // Wielding a weapon displaces the lighter back into the inventory. + p.SetInvSlot(1, h.g.newInventorySlot("polish_blade", 1)) + h.g.doWear(sess, "blade") + if got := p.Equipment[item.SlotMainHand]; got != "polish_blade" { + t.Fatalf("blade not equipped: %q", got) + } + + lighter := carriedInventSlot(p, "polish_lighter") + if lighter == nil { + t.Fatal("the displaced lighter was not returned to the inventory") + } + if lighter.Quality != 98 || lighter.MaxQuality != 100 { + t.Errorf("returned lighter = %+v, want quality 98/100", lighter) + } + if _, ok := p.EquipQuality["polish_lighter"]; ok { + t.Error("stale EquipQuality entry left behind after the swap") + } + + // act_burn must charge the carried stack, not a stale equipment record. + if !h.g.consumeFireTool(sess, p, "polish_lighter") { + t.Fatal("consumeFireTool refused the carried lighter stack") + } + if lighter.Quality != 97 { + t.Errorf("carried lighter charge = %d, want 97", lighter.Quality) + } + }) +} + +func TestWearAllSwapReturnsEquippedQualityToInventory(t *testing.T) { + h := newHarness(t) + h.seedAccount("alice") + p := h.seedCharacter("alice", "alice") + seedPolishWearItems(t, h.dir) + + sess := &net.Session{Conn: newChanConn(), Player: p} + sess.SetState(net.StateGame) + + h.g.RunOnOwner(func() { + h.g.ItemStore.Reload(h.dir) + p.SetInvSlot(0, h.g.newInventorySlot("polish_lighter", 1)) + h.g.doWear(sess, "lighter") + if got := p.EquipQuality["polish_lighter"]; got != 100 { + t.Fatalf("equipped lighter charge = %d, want 100", got) + } + + p.SetInvSlot(1, h.g.newInventorySlot("polish_blade", 1)) + h.g.doWearAll(sess) + + lighter := carriedInventSlot(p, "polish_lighter") + if lighter == nil { + t.Fatal("wear all did not return the displaced lighter") + } + if lighter.Quality != 100 || lighter.MaxQuality != 100 { + t.Errorf("returned lighter = %+v, want quality 100/100", lighter) + } + if _, ok := p.EquipQuality["polish_lighter"]; ok { + t.Error("stale EquipQuality entry left behind by wear all") + } + }) +} diff --git a/internal/game/production_engine.go b/internal/game/production_engine.go index 035b7e2..ddada6b 100644 --- a/internal/game/production_engine.go +++ b/internal/game/production_engine.go @@ -172,11 +172,11 @@ func (g *Game) loadCraftItem(itemID string) *item.ItemDef { return item } -func (g *Game) advanceProduction(sess *net.Session, p *player.Player) bool { +func (g *Game) advanceProduction(sess *net.Session, p *player.Player) { d, ok := p.Action.Data.(*behavior.ProductionData) if !ok { g.cancelAction(p) - return false + return } itemID := d.ItemID phase := d.Phase @@ -188,7 +188,7 @@ func (g *Game) advanceProduction(sess *net.Session, p *player.Player) bool { item := g.loadCraftItem(itemID) if item == nil { g.cancelAction(p) - return false + return } craft := item.FirstCraft() @@ -198,7 +198,7 @@ func (g *Game) advanceProduction(sess *net.Session, p *player.Player) bool { } d.Phase = 1 p.Action.WaitLeft = engine.ToTicks(wait) - return true + return } skill := craft.EffectiveSkill() @@ -222,9 +222,14 @@ func (g *Game) advanceProduction(sess *net.Session, p *player.Player) bool { if !craftOutputFits(p, itemID, item) || (failDef != nil && !craftOutputFits(p, craft.Fail, failDef)) { sess.WriteLine("Your inventory is too full!") g.cancelAction(p) - return false + return } + // Resolve %i names from the alternative that will actually be + // consumed: consumption empties the inventory, so p.HasItem can no + // longer identify it when the message is rendered below. + hasItem := craftConsumedHasItem(craft, p.CountItem, p.HasItem) + if rand.Float64() < chance { outputQty := craft.OutputQty if outputQty <= 0 { @@ -240,14 +245,14 @@ func (g *Game) advanceProduction(sess *net.Session, p *player.Player) bool { // Cannot happen after the availability gate above, but never // produce output from a failed consumption. g.cancelAction(p) - return false + return } if !placeCraftOutput(p, itemID, item, outputQty) { // Defensive: the fits check above should have guaranteed a // stack or slot. sess.WriteLine("Your inventory is too full!") g.cancelAction(p) - return false + return } for _, bp := range byproducts { @@ -273,7 +278,7 @@ func (g *Game) advanceProduction(sess *net.Session, p *player.Player) bool { } else { defaultSuccess = "You produce %n." } - msg := g.resolveCraftMessage(sess, craft.SuccessMessage, defaultSuccess, craft, item.ID, byproducts, p.HasItem) + msg := g.resolveCraftMessage(sess, craft.SuccessMessage, defaultSuccess, craft, item.ID, byproducts, hasItem) if p.OptionBool("xp_drops") && craft.XP > 0 { msg += g.formatXpDropSingle(sess, p, player.SkillName(skill), craft.XP) } @@ -281,18 +286,15 @@ func (g *Game) advanceProduction(sess *net.Session, p *player.Player) bool { } else { if !consumeIngredients(craft, p.CountItem, p.RemoveItem) { g.cancelAction(p) - return false + return } if craft.Fail != "" { - freeSlot := p.FirstFreeSlot() - if freeSlot >= 0 { - p.SetInvSlot(freeSlot, &player.InventorySlot{ItemID: craft.Fail, Quantity: 1}) - } + g.placeCraftFail(p, craft) } g.saveCharacter(p) - msg := g.resolveCraftMessage(sess, craft.FailMessage, info.FailMessage, craft, item.ID, nil, p.HasItem) + msg := g.resolveCraftMessage(sess, craft.FailMessage, info.FailMessage, craft, item.ID, nil, hasItem) if msg != "" { sess.WriteLine(g.colorize(sess, "damage_taken", msg)) } @@ -306,20 +308,19 @@ func (g *Game) advanceProduction(sess *net.Session, p *player.Player) bool { sess.WriteLine(endMsg) } g.cancelAction(p) - return false + return } } if g.canContinueProduction(p, item) { p.Action.WaitLeft = engine.ToTicks(wait) - return true + return } if endMsg != "" { sess.WriteLine(endMsg) } g.cancelAction(p) - return false } func (g *Game) collectByproducts(p *player.Player, item *item.ItemDef) []string { @@ -354,6 +355,73 @@ func (g *Game) collectByproducts(p *player.Player, item *item.ItemDef) []string return byproducts } +// craftConsumedHasItem returns a hasItem predicate that reports the +// ingredient alternatives consumeIngredients will remove right now — the +// same aggregate plan availability and consumption use. A %i message +// resolved AFTER consumption therefore still names the item the player +// actually used (p.HasItem is false for everything consumed). Falls back to +// the supplied predicate when no plan can be built. +func craftConsumedHasItem(craft *item.CraftDef, countItem func(string) int, fallback func(string) bool) func(string) bool { + plan := planIngredients(craft, countItem) + if plan == nil { + return fallback + } + consumed := make(map[string]bool, len(plan)) + for _, c := range plan { + consumed[c.id] = true + } + return func(id string) bool { return consumed[id] } +} + +// craftRollSuccess rolls one craft cycle's success: true when the recipe +// defines no success formula or the roll succeeds. +func craftRollSuccess(craft *item.CraftDef, level int) bool { + if craft == nil || craft.Success == nil { + return true + } + chance := behavior.SuccessChance(behavior.SuccessFormula(*craft.Success), level, craft.Level) + return rand.Float64() < chance +} + +// craftFailFits reports whether the recipe's fail replacement (if any) can +// be placed right now. Like the output fit check, it must run BEFORE +// consumption so a full inventory never destroys materials. +func (g *Game) craftFailFits(p *player.Player, craft *item.CraftDef) bool { + if craft == nil || craft.Fail == "" { + return true + } + def, _ := g.ItemStore.Load(craft.Fail) + if def == nil { + return true + } + return craftOutputFits(p, craft.Fail, def) +} + +// placeCraftFail delivers a failed craft's fail replacement into the +// inventory, merging into a matching stack when the fail item is stackable +// just like the success output does. FirstFreeSlot alone is not enough: the +// pre-roll gate counts a stack merge (craftFailFits), so a full inventory +// with a matching stack must not silently destroy the fail replacement. +func (g *Game) placeCraftFail(p *player.Player, craft *item.CraftDef) { + if craft == nil || craft.Fail == "" { + return + } + def, _ := g.ItemStore.Load(craft.Fail) + placeCraftOutput(p, craft.Fail, def, 1) +} + +// placeCraftByproducts delivers byproducts into free slots, dropping any +// remainder on the ground rather than silently deleting it. +func (g *Game) placeCraftByproducts(p *player.Player, byproducts []string) { + for _, bp := range byproducts { + if slot := p.FirstFreeSlot(); slot >= 0 { + p.SetInvSlot(slot, &player.InventorySlot{ItemID: bp, Quantity: 1}) + } else { + g.World.AddGroundItem(p.RoomID, bp, 1) + } + } +} + func (g *Game) canContinueProduction(p *player.Player, item *item.ItemDef) bool { craft := item.FirstCraft() if craft == nil { diff --git a/internal/game/production_index.go b/internal/game/production_index.go index ffacd8a..35466a7 100644 --- a/internal/game/production_index.go +++ b/internal/game/production_index.go @@ -272,6 +272,19 @@ func consumeIngredients(c *item.CraftDef, countItem func(string) int, removeItem return true } +// stackProps returns the Quality and MaxQuality a NEW inventory stack of def +// receives, mirroring newInventorySlot (core_utils.go): a quality-bearing +// definition is stamped at its full quality. Every stack-creation and +// stack-merge site must agree on these values, or a stack produced by one +// path (craft, harvest, ammo removal, shop) stops merging with the same item +// picked up from the ground while the inventory is full. +func stackProps(def *item.ItemDef) (quality, maxQuality int) { + if def != nil && def.MaxQuality > 0 { + return def.MaxQuality, def.MaxQuality + } + return 0, 0 +} + // craftOutputFits reports whether outputQty of an item can be placed into // the inventory right now: merged into an existing stack for stackable // definitions, otherwise a free slot. Crafters must check BEFORE consuming @@ -279,8 +292,10 @@ func consumeIngredients(c *item.CraftDef, countItem func(string) int, removeItem // producing the output. func craftOutputFits(p *player.Player, itemID string, outDef *item.ItemDef) bool { if outDef != nil && outDef.Stackable { + quality, maxQuality := stackProps(outDef) for i := 0; i < 28; i++ { - if slot := p.InvSlot(i); slot != nil && slot.ItemID == itemID { + slot := p.InvSlot(i) + if slot != nil && slot.ItemID == itemID && slot.Quality == quality && slot.MaxQuality == maxQuality { return true } } @@ -294,9 +309,11 @@ func craftOutputFits(p *player.Player, itemID string, outDef *item.ItemDef) bool // (consumption cannot free fewer slots than the check saw on the owner // goroutine, so a false return is defensive). func placeCraftOutput(p *player.Player, itemID string, outDef *item.ItemDef, outputQty int) bool { + quality, maxQuality := stackProps(outDef) if outDef != nil && outDef.Stackable { for i := 0; i < 28; i++ { - if slot := p.InvSlot(i); slot != nil && slot.ItemID == itemID { + slot := p.InvSlot(i) + if slot != nil && slot.ItemID == itemID && slot.Quality == quality && slot.MaxQuality == maxQuality { slot.Quantity += outputQty return true } @@ -306,6 +323,6 @@ func placeCraftOutput(p *player.Player, itemID string, outDef *item.ItemDef, out if freeSlot == -1 { return false } - p.SetInvSlot(freeSlot, &player.InventorySlot{ItemID: itemID, Quantity: outputQty}) + p.SetInvSlot(freeSlot, &player.InventorySlot{ItemID: itemID, Quantity: outputQty, Quality: quality, MaxQuality: maxQuality}) return true } diff --git a/internal/game/quit_casino_test.go b/internal/game/quit_casino_test.go index 50f9ac8..f373457 100644 --- a/internal/game/quit_casino_test.go +++ b/internal/game/quit_casino_test.go @@ -169,7 +169,9 @@ func TestQuitMidRoundMentionsQuitNow(t *testing.T) { // "quit now" starts the logout countdown while the player keeps their seat // and wager; the stake is forfeited and the seat freed only on the final -// tick, immediately before the disconnect. +// tick, immediately before the disconnect. The round must be IN FLIGHT for +// that: a stake placed in the betting window has had no card dealt for it and +// is refunded on disconnect (see markDisconnected). func TestQuitNowForfeitsOnlyAtLogout(t *testing.T) { g, sess := quitTestGame(t) sess.Account = &player.Account{Name: "alice"} @@ -177,6 +179,10 @@ func TestQuitNowForfeitsOnlyAtLogout(t *testing.T) { g.Casino.Join(1, cfg, "alice") g.Casino.Join(1, cfg, "bob") g.Casino.Bet(1, "blackjack", "alice", "", 10, casinoWallet{g: g, p: sess.Player}) + // Bob bets too so the round actually starts and alice holds a dealt hand. + bobWallet := player.New("bob") + bobWallet.Credits = 1000 + g.Casino.Bet(1, "blackjack", "bob", "", 10, casinoWallet{g: g, p: bobWallet}) wallet := sess.Player g.doQuitNow(sess) diff --git a/internal/game/session_registry.go b/internal/game/session_registry.go index 0e47f82..c4f5d67 100644 --- a/internal/game/session_registry.go +++ b/internal/game/session_registry.go @@ -11,9 +11,15 @@ func (g *Game) attachSession(sess *net.Session) { g.charsMu.Unlock() } -// detachSession removes a character's registry entry. +// detachSession removes a character's registry entry and drops every command +// queued under its name. Leftover queue entries retain the departed session +// (and its transport) and would otherwise replay against the next session +// that logs in as the same character. func (g *Game) detachSession(name string) { g.charsMu.Lock() delete(g.loggedInChars, name) g.charsMu.Unlock() + if g.queue != nil { + g.queue.ClearPlayer(name) + } } diff --git a/internal/game/sys_combat.go b/internal/game/sys_combat.go index 26dac2a..b2a2463 100644 --- a/internal/game/sys_combat.go +++ b/internal/game/sys_combat.go @@ -105,6 +105,20 @@ func (g *Game) checkAggro(sess *net.Session) { aggMob := mob keepRolling := func() (keep bool) { + // The retry subscription outlives the session that armed it (only + // mob/combat/room state ends it), so a player who has since logged + // out, disconnected, or logged in again on a new connection must + // not have combat started for them here: the mob's own subscribers + // would then keep fighting a character who is not in the world. + if sess.Player == nil || sess.IsDisconnecting() { + return false + } + g.charsMu.Lock() + current := g.loggedInChars[p.Name] + g.charsMu.Unlock() + if current != nil && current != sess { + return false + } if g.Combat.Get(p.Name) != nil { return false } diff --git a/internal/game/sys_triggers.go b/internal/game/sys_triggers.go index 2dbde9f..312fe50 100644 --- a/internal/game/sys_triggers.go +++ b/internal/game/sys_triggers.go @@ -4,6 +4,7 @@ import ( "fmt" "os" "path/filepath" + "sort" "strings" "sync" @@ -98,9 +99,17 @@ func (g *Game) startSequence(sess *net.Session, p *player.Player, steps []behavi // sequences are removed. func (g *Game) SequenceTick() { g.seqMu.Lock() - playerSeqs := make([]*sequence, 0, len(g.sequences)) - for _, s := range g.sequences { - playerSeqs = append(playerSeqs, s) + // g.sequences is a map: collect its keys and sort them so the per-tick + // order in which simultaneous sequences advance is reproducible (the + // determinism contract in building_guide/tick_order.md). + keys := make([]string, 0, len(g.sequences)) + for key := range g.sequences { + keys = append(keys, key) + } + sort.Strings(keys) + playerSeqs := make([]*sequence, 0, len(keys)) + for _, key := range keys { + playerSeqs = append(playerSeqs, g.sequences[key]) } globalSeqs := append([]*sequence(nil), g.globalSeqs...) g.seqMu.Unlock() diff --git a/internal/game/tick.go b/internal/game/tick.go index cd9b2fd..4181622 100644 --- a/internal/game/tick.go +++ b/internal/game/tick.go @@ -5,10 +5,25 @@ import ( "math/rand" "thehouseoficarus/internal/behavior" + "thehouseoficarus/internal/net" "thehouseoficarus/internal/player" "thehouseoficarus/internal/world" ) +// disconnectCombatGraceTicks bounds how many ticks DisconnectTick defers the +// hard remove of a session that is still in combat, so an unresolvable fight +// cannot keep the character's registry entry (and its "logged in elsewhere" +// lockout) forever. +const disconnectCombatGraceTicks = 5 + +// disconnectCombatDeferrals counts, per session, how many ticks its pending +// disconnect has already been deferred for combat. DisconnectTick is its only +// reader/writer and every tick system runs on the owner goroutine, so the map +// needs no lock. An entry is dropped when this path completes the hard remove; +// a session hard-removed elsewhere (a second Hub.Remove, shutdown) leaves at +// most one stale int behind. +var disconnectCombatDeferrals = map[*net.Session]int{} + func (g *Game) DisconnectTick() { if g.Hub == nil { return @@ -22,10 +37,20 @@ func (g *Game) DisconnectTick() { g.Hub.Remove(sess) continue } + // An unresolvable fight (a task/worksite mob that takes no progress, a + // zero-damage stalemate) must not postpone the hard remove forever: + // the character registry entry only drops with the remove, so an + // unbounded deferral locks the player out ("logged in elsewhere") + // until a restart. Defer while the fight can still resolve, but only + // for a bounded grace period, then let the countdown proceed. if g.Combat.Get(p.Name) != nil { - continue + if deferred := disconnectCombatDeferrals[sess]; deferred < disconnectCombatGraceTicks { + disconnectCombatDeferrals[sess] = deferred + 1 + continue + } } if sess.TickDisconnectCountdown() { + delete(disconnectCombatDeferrals, sess) g.restoreGodPlayer(p) g.saveCharacter(p) g.Hub.HardRemove(sess) @@ -76,6 +101,11 @@ func (g *Game) WanderTick() { return } + // One session snapshot per system invocation: the destination broadcast + // below used to re-snapshot every session (lock, allocate, sort) once per + // wandering mob/object. + sessions := g.Hub.AllSessions() + type moveEvent struct { name string fromRoom int @@ -134,7 +164,7 @@ func (g *Game) WanderTick() { } for _, m := range moves { - for _, sess := range g.Hub.AllSessions() { + for _, sess := range sessions { p := sess.Player if p == nil { continue -- cgit v1.2.3