Fix trade race condition: move trade/roll decision to -- Command: handler

The root cause: checkpoint handler queued .trade via say_delayed, but
monop sends '-- Command:' before processing the trade. The old handler
reset in_trade and queued .roll, so both commands got sent.

Fix: checkpoint handler no longer sends commands. The -- Command:
prompt is where the bot decides to trade or roll, matching how monop
actually works (-- Command: is the interactive prompt).

Also: no trades while in jail (jail has no -- Command: before roll).

Updated all tests to reflect the new flow.
This commit is contained in:
Jarvis 2026-02-21 10:55:33 +00:00
parent 8bbadba7d9
commit 7a4346a53f
5 changed files with 119 additions and 153 deletions

Binary file not shown.

View file

@ -224,16 +224,8 @@ class PlayerBot:
if self.is_my_turn(): if self.is_my_turn():
self.turns_played += 1 self.turns_played += 1
# ~10% chance to initiate a trade (after turn 5, not in debt) # Trade decision is deferred to -- Command: handler
if (self.turns_played > 5 and not self.in_debt # to avoid race conditions with delayed messages
and random.random() < 0.10):
self.in_trade = True
self.trade_props_offered = 0
self.log("Initiating a trade!")
self.say_delayed("trade")
else:
self.say_delayed("roll")
self.rolled_this_turn = True
return return
# ============================================================ # ============================================================
@ -444,11 +436,20 @@ class PlayerBot:
# COMMAND PROMPT # COMMAND PROMPT
# ============================================================ # ============================================================
if msg == "-- Command:": if msg == "-- Command:":
self.in_trade = False # trade ended (accepted, rejected, or cancelled) self.in_trade = False # any active trade is over
if self.is_my_turn(): if self.is_my_turn():
if self.in_debt: if self.in_debt:
self.say_delayed("mortgage") self.say_delayed("mortgage")
elif not self.rolled_this_turn: elif not self.rolled_this_turn:
# ~10% chance to initiate a trade (after turn 5, not in debt/jail)
if (self.turns_played > 5 and not self.in_debt
and not self.in_jail
and random.random() < 0.10):
self.in_trade = True
self.trade_props_offered = 0
self.log("Initiating a trade!")
self.say_delayed("trade")
else:
self.say_delayed("roll") self.say_delayed("roll")
self.rolled_this_turn = True self.rolled_this_turn = True
# else: already rolled, waiting for prompts to resolve # else: already rolled, waiting for prompts to resolve

View file

@ -1,21 +1,11 @@
{ {
"players": [ "players": [
{
"name": "charlie",
"number": 3,
"money": 985,
"location": 40,
"inJail": true,
"jailTurns": 1,
"doublesCount": 0,
"getOutOfJailFreeCards": 0
},
{ {
"name": "alice", "name": "alice",
"number": 1, "number": 1,
"money": 1375, "money": 1280,
"location": 40, "location": 21,
"inJail": true, "inJail": false,
"jailTurns": 0, "jailTurns": 0,
"doublesCount": 0, "doublesCount": 0,
"getOutOfJailFreeCards": 0 "getOutOfJailFreeCards": 0
@ -23,8 +13,18 @@
{ {
"name": "bob", "name": "bob",
"number": 2, "number": 2,
"money": 800, "money": 1305,
"location": 31, "location": 16,
"inJail": false,
"jailTurns": 0,
"doublesCount": 0,
"getOutOfJailFreeCards": 0
},
{
"name": "charlie",
"number": 3,
"money": 1260,
"location": 13,
"inJail": false, "inJail": false,
"jailTurns": 0, "jailTurns": 0,
"doublesCount": 0, "doublesCount": 0,
@ -81,7 +81,7 @@
"id": 6, "id": 6,
"name": "Oriental ave. (L)", "name": "Oriental ave. (L)",
"type": "property", "type": "property",
"owner": 2, "owner": 3,
"mortgaged": false, "mortgaged": false,
"group": "lightblue", "group": "lightblue",
"cost": 100, "cost": 100,
@ -96,7 +96,7 @@
"id": 8, "id": 8,
"name": "Vermont ave. (L)", "name": "Vermont ave. (L)",
"type": "property", "type": "property",
"owner": 1, "owner": null,
"mortgaged": false, "mortgaged": false,
"group": "lightblue", "group": "lightblue",
"cost": 100, "cost": 100,
@ -121,7 +121,7 @@
"id": 11, "id": 11,
"name": "St. Charles pl. (V)", "name": "St. Charles pl. (V)",
"type": "property", "type": "property",
"owner": 3, "owner": null,
"mortgaged": false, "mortgaged": false,
"group": "violet", "group": "violet",
"cost": 140, "cost": 140,
@ -131,7 +131,7 @@
"id": 12, "id": 12,
"name": "Electric Co.", "name": "Electric Co.",
"type": "utility", "type": "utility",
"owner": 2, "owner": null,
"mortgaged": false, "mortgaged": false,
"group": "utility", "group": "utility",
"cost": 150 "cost": 150
@ -140,7 +140,7 @@
"id": 13, "id": 13,
"name": "States ave. (V)", "name": "States ave. (V)",
"type": "property", "type": "property",
"owner": null, "owner": 3,
"mortgaged": false, "mortgaged": false,
"group": "violet", "group": "violet",
"cost": 140, "cost": 140,
@ -169,7 +169,7 @@
"id": 16, "id": 16,
"name": "St. James pl. (O)", "name": "St. James pl. (O)",
"type": "property", "type": "property",
"owner": null, "owner": 2,
"mortgaged": false, "mortgaged": false,
"group": "orange", "group": "orange",
"cost": 180, "cost": 180,
@ -194,7 +194,7 @@
"id": 19, "id": 19,
"name": "New York ave. (O)", "name": "New York ave. (O)",
"type": "property", "type": "property",
"owner": 3, "owner": null,
"mortgaged": false, "mortgaged": false,
"group": "orange", "group": "orange",
"cost": 200, "cost": 200,
@ -209,7 +209,7 @@
"id": 21, "id": 21,
"name": "Kentucky ave. (R)", "name": "Kentucky ave. (R)",
"type": "property", "type": "property",
"owner": null, "owner": 1,
"mortgaged": false, "mortgaged": false,
"group": "red", "group": "red",
"cost": 220, "cost": 220,
@ -244,7 +244,7 @@
"id": 25, "id": 25,
"name": "B&O RR", "name": "B&O RR",
"type": "railroad", "type": "railroad",
"owner": 3, "owner": null,
"mortgaged": false, "mortgaged": false,
"group": "railroad", "group": "railroad",
"cost": 200 "cost": 200
@ -273,7 +273,7 @@
"id": 28, "id": 28,
"name": "Water Works", "name": "Water Works",
"type": "utility", "type": "utility",
"owner": 2, "owner": null,
"mortgaged": false, "mortgaged": false,
"group": "utility", "group": "utility",
"cost": 150 "cost": 150
@ -297,7 +297,7 @@
"id": 31, "id": 31,
"name": "Pacific ave. (G)", "name": "Pacific ave. (G)",
"type": "property", "type": "property",
"owner": 2, "owner": null,
"mortgaged": false, "mortgaged": false,
"group": "green", "group": "green",
"cost": 300, "cost": 300,
@ -370,153 +370,99 @@
], ],
"log": [ "log": [
{ {
"text": "Landed on Community Chest ii", "text": "roll is 4, 6",
"player": "alice", "player": "alice",
"timestamp": "2026-02-21 10:41:53" "timestamp": "2026-02-21 10:55:12"
}, },
{ {
"text": "You are Assessed for street repairs.", "text": "Landed on Just Visiting",
"player": "alice" "player": "alice",
"timestamp": "2026-02-21 10:55:13"
}, },
{ {
"text": "bob's turn \u2014 $1400 on Oriental ave. (L)", "text": "bob's turn \u2014 $1500 on === GO ===",
"player": "bob", "player": "bob",
"timestamp": "2026-02-21 10:41:57" "timestamp": "2026-02-21 10:55:14"
}, },
{ {
"text": "roll is 1, 5", "text": "roll is 5, 2",
"player": "bob", "player": "bob",
"timestamp": "2026-02-21 10:41:58" "timestamp": "2026-02-21 10:55:15"
}, },
{ {
"text": "Landed on Electric Co.", "text": "Landed on Chance i",
"player": "bob", "player": "bob",
"timestamp": "2026-02-21 10:41:58" "timestamp": "2026-02-21 10:55:15"
}, },
{ {
"text": "charlie's turn \u2014 $1160 on New York ave. (O)", "text": "Pay Poor Tax of $15",
"player": "bob"
},
{
"text": "charlie's turn \u2014 $1500 on === GO ===",
"player": "charlie", "player": "charlie",
"timestamp": "2026-02-21 10:42:00" "timestamp": "2026-02-21 10:55:17"
}, },
{ {
"text": "roll is 3, 3", "text": "roll is 3, 3",
"player": "charlie", "player": "charlie",
"timestamp": "2026-02-21 10:42:01" "timestamp": "2026-02-21 10:55:19"
}, },
{ {
"text": "Landed on B&O RR", "text": "Landed on Oriental ave. (L)",
"player": "charlie", "player": "charlie",
"timestamp": "2026-02-21 10:42:01" "timestamp": "2026-02-21 10:55:19"
}, },
{ {
"text": "charlie's turn \u2014 $960 on B&O RR", "text": "charlie's turn \u2014 $1400 on Oriental ave. (L)",
"player": "charlie", "player": "charlie",
"timestamp": "2026-02-21 10:42:04" "timestamp": "2026-02-21 10:55:22"
}, },
{ {
"text": "roll is 4, 1", "text": "roll is 1, 6",
"player": "charlie", "player": "charlie",
"timestamp": "2026-02-21 10:42:05" "timestamp": "2026-02-21 10:55:23"
}, },
{ {
"text": "Landed on GO TO JAIL!", "text": "Landed on States ave. (V)",
"player": "charlie", "player": "charlie",
"timestamp": "2026-02-21 10:42:05" "timestamp": "2026-02-21 10:55:23"
}, },
{ {
"text": "alice's turn \u2014 $1400 on Community Chest ii", "text": "alice's turn \u2014 $1500 on Just Visiting",
"player": "alice", "player": "alice",
"timestamp": "2026-02-21 10:42:06" "timestamp": "2026-02-21 10:55:25"
}, },
{ {
"text": "roll is 6, 2", "text": "roll is 5, 6",
"player": "alice", "player": "alice",
"timestamp": "2026-02-21 10:42:07" "timestamp": "2026-02-21 10:55:26"
}, },
{ {
"text": "Landed on B&O RR", "text": "Landed on Kentucky ave. (R)",
"player": "alice", "player": "alice",
"timestamp": "2026-02-21 10:42:08" "timestamp": "2026-02-21 10:55:27"
}, },
{ {
"text": "Paid $25 rent to charlie", "text": "bob's turn \u2014 $1485 on Chance i",
"player": "alice"
},
{
"text": "bob's turn \u2014 $1250 on Electric Co.",
"player": "bob", "player": "bob",
"timestamp": "2026-02-21 10:42:09" "timestamp": "2026-02-21 10:55:29"
}, },
{ {
"text": "roll is 4, 4", "text": "roll is 3, 6",
"player": "bob", "player": "bob",
"timestamp": "2026-02-21 10:42:10" "timestamp": "2026-02-21 10:55:30"
}, },
{ {
"text": "Landed on Free Parking", "text": "Landed on St. James pl. (O)",
"player": "bob", "player": "bob",
"timestamp": "2026-02-21 10:42:11" "timestamp": "2026-02-21 10:55:30"
}, },
{ {
"text": "bob's turn \u2014 $1250 on Free Parking", "text": "charlie's turn \u2014 $1260 on States ave. (V)",
"player": "bob",
"timestamp": "2026-02-21 10:42:12"
},
{
"text": "roll is 6, 2",
"player": "bob",
"timestamp": "2026-02-21 10:42:13"
},
{
"text": "Landed on Water Works",
"player": "bob",
"timestamp": "2026-02-21 10:42:14"
},
{
"text": "charlie's turn \u2014 $985 on JAIL",
"player": "charlie", "player": "charlie",
"timestamp": "2026-02-21 10:42:16" "timestamp": "2026-02-21 10:55:32"
},
{
"text": "roll is 6, 5",
"player": "charlie",
"timestamp": "2026-02-21 10:42:17"
},
{
"text": "alice's turn \u2014 $1375 on B&O RR",
"player": "alice",
"timestamp": "2026-02-21 10:42:18"
},
{
"text": "roll is 2, 3",
"player": "alice",
"timestamp": "2026-02-21 10:42:20"
},
{
"text": "Landed on GO TO JAIL!",
"player": "alice",
"timestamp": "2026-02-21 10:42:20"
},
{
"text": "bob's turn \u2014 $1100 on Water Works",
"player": "bob",
"timestamp": "2026-02-21 10:42:21"
},
{
"text": "roll is 1, 2",
"player": "bob",
"timestamp": "2026-02-21 10:42:22"
},
{
"text": "Landed on Pacific ave. (G)",
"player": "bob",
"timestamp": "2026-02-21 10:42:22"
},
{
"text": "charlie's turn \u2014 $985 on JAIL",
"player": "charlie",
"timestamp": "2026-02-21 10:42:24"
} }
], ],
"lastUpdated": "2026-02-21T10:42:27.015680+00:00" "lastUpdated": "2026-02-21T10:55:32.750315+00:00"
} }

View file

@ -115,8 +115,13 @@ class TestTurns(unittest.TestCase):
return bot return bot
def test_checkpoint_triggers_roll(self): def test_checkpoint_triggers_roll(self):
"""Checkpoint sets up turn, -- Command: triggers the roll."""
bot = self._make_bot("alice") bot = self._make_bot("alice")
msgs = bot.feed("alice (1) (cash $1500) on === GO ===") msgs = bot.feed("alice (1) (cash $1500) on === GO ===")
self.assertEqual(msgs, [], "Checkpoint should not send commands")
# Roll happens at -- Command:
with patch("random.random", return_value=0.99): # no trade
msgs = bot.feed("-- Command:")
self.assertIn("roll", msgs) self.assertIn("roll", msgs)
self.assertTrue(bot.rolled_this_turn) self.assertTrue(bot.rolled_this_turn)
@ -224,11 +229,12 @@ class TestTrading(unittest.TestCase):
@patch("monop_players.random") @patch("monop_players.random")
def test_trade_initiated(self, mock_random): def test_trade_initiated(self, mock_random):
"""With random < 0.10, bot initiates trade instead of rolling.""" """With random < 0.10, bot initiates trade at -- Command:."""
mock_random.random.return_value = 0.05 # < 0.10 mock_random.random.return_value = 0.05 # < 0.10
bot = self._make_bot("alice") bot = self._make_bot("alice")
bot.turns_played = 10 # past turn 5 bot.turns_played = 10 # past turn 5
msgs = bot.feed("alice (1) (cash $1500) on === GO ===") bot.feed("alice (1) (cash $1500) on === GO ===")
msgs = bot.feed("-- Command:")
self.assertIn("trade", msgs) self.assertIn("trade", msgs)
self.assertTrue(bot.in_trade) self.assertTrue(bot.in_trade)
self.assertNotIn("roll", msgs) self.assertNotIn("roll", msgs)
@ -239,7 +245,8 @@ class TestTrading(unittest.TestCase):
mock_random.random.return_value = 0.05 mock_random.random.return_value = 0.05
bot = self._make_bot("alice") bot = self._make_bot("alice")
bot.turns_played = 2 bot.turns_played = 2
msgs = bot.feed("alice (1) (cash $1500) on === GO ===") bot.feed("alice (1) (cash $1500) on === GO ===")
msgs = bot.feed("-- Command:")
self.assertIn("roll", msgs) self.assertIn("roll", msgs)
self.assertFalse(bot.in_trade) self.assertFalse(bot.in_trade)
@ -332,21 +339,18 @@ class TestTradeInJail(unittest.TestCase):
self.assertNotIn("roll", msgs, self.assertNotIn("roll", msgs,
"Should not roll while in_trade is True") "Should not roll while in_trade is True")
def test_trade_then_jail_turn_sequence(self): def test_no_trade_from_jail(self):
"""Simulate the exact sequence: checkpoint initiates trade, then jail prompt arrives.""" """Can't trade while in jail — must roll first."""
bot = self._make_bot() bot = self._make_bot()
# Force trade to trigger (patch random)
with patch("random.random", return_value=0.01): # < 0.10 → triggers trade
msgs = bot.feed("charlie (3) (cash $985) on JAIL") msgs = bot.feed("charlie (3) (cash $985) on JAIL")
self.assertTrue(bot.in_trade, "Trade should be initiated")
self.assertIn("trade", msgs, "Should send .trade")
self.assertNotIn("roll", msgs, "Should not also roll")
# Now jail turn prompt arrives
msgs = bot.feed("(This is your 2nd turn in JAIL)") msgs = bot.feed("(This is your 2nd turn in JAIL)")
self.assertNotIn("roll", msgs, self.assertIn("roll", msgs, "Should roll in jail")
"Jail handler must not roll during active trade")
self.assertTrue(bot.in_trade, "Trade should still be active") # Even if random triggers, no trade while in_jail
with patch("random.random", return_value=0.01):
msgs = bot.feed("-- Command:")
self.assertFalse(bot.in_trade, "Should not trade while in jail")
self.assertIn("roll", msgs)
def test_trade_which_player_prompt(self): def test_trade_which_player_prompt(self):
"""Bot should pick a trade partner when asked.""" """Bot should pick a trade partner when asked."""
@ -371,16 +375,31 @@ class TestTradeInJail(unittest.TestCase):
msgs = bot.feed("(This is your 2nd turn in JAIL)") msgs = bot.feed("(This is your 2nd turn in JAIL)")
self.assertIn("roll", msgs, "Should roll when not trading") self.assertIn("roll", msgs, "Should roll when not trading")
def test_command_prompt_after_trade_allows_roll(self): def test_trade_done_triggers_roll(self):
"""After trade ends (-- Command:), bot should roll normally.""" """Trade is done! resets in_trade and triggers roll."""
bot = self._make_bot() bot = self._make_bot()
bot.current_player = "charlie" bot.current_player = "charlie"
bot.in_trade = True bot.in_trade = True
bot.rolled_this_turn = False bot.rolled_this_turn = False
msgs = bot.feed("-- Command:") msgs = bot.feed("Trade is done!")
self.assertFalse(bot.in_trade) self.assertFalse(bot.in_trade)
self.assertIn("roll", msgs) self.assertIn("roll", msgs)
def test_trade_decision_at_command_prompt(self):
"""Trade/roll decision happens at -- Command:, not at checkpoint."""
bot = self._make_bot()
# Checkpoint just records the turn
msgs = bot.feed("charlie (3) (cash $1500) on === GO ===")
self.assertNotIn("trade", msgs, "Checkpoint must not send trade")
self.assertNotIn("roll", msgs, "Checkpoint must not send roll")
# -- Command: is where the decision happens
with patch("random.random", return_value=0.01): # triggers trade
msgs = bot.feed("-- Command:")
self.assertTrue(bot.in_trade)
self.assertIn("trade", msgs)
self.assertNotIn("roll", msgs)
class TestValidInputs(unittest.TestCase): class TestValidInputs(unittest.TestCase):
def test_picks_first_option(self): def test_picks_first_option(self):