fix: harden engine across casino, economy, concurrency, security, and content

1 files changed, 47 insertions, 23 deletions

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))
 			}
 		}