From 1cb6da257f7f504f94ac77929a67decb07ec08e6 Mon Sep 17 00:00:00 2001 From: Jarvis Date: Sat, 21 Feb 2026 18:52:10 +0000 Subject: [PATCH] Fix 7 bugs: trade property transfer, UI owner colors, resign edge cases, spec flag, phase field, timestamps Bug fixes: - #1: Trade now transfers properties (parse printsq-format lines in trade summaries) - #2: UI owner indicator uses player number lookup instead of raw array index - #3: Mid-stream game pickup sets _first_player_idx - #4: Resign with unresolved target clears properties (bank fallback) - #6: spec flag cleared after rent payment (matches C's get_card cleanup) - #13: get_state() always emits phase field (setup/playing/over) - #16: Resign and trade log entries include timestamps Also: - Bankrupt players tracked with bankrupt flag, shown in UI with skull/dashed border - 19 resignation tests (test_parser_resign.py) - 10 bug-specific tests (test_parser_bugs.py) - All 1551 parser checkpoints + 67 unit tests passing --- monop_parser.py | 116 +++++++++-- plugins/monop/monop_parser.py | 116 +++++++++-- site/index.html | 24 ++- test_parser_bugs.py | 302 ++++++++++++++++++++++++++++ test_parser_resign.py | 364 ++++++++++++++++++++++++++++++++++ 5 files changed, 892 insertions(+), 30 deletions(-) create mode 100644 test_parser_bugs.py create mode 100644 test_parser_resign.py diff --git a/monop_parser.py b/monop_parser.py index a6a9f1e..18370d3 100644 --- a/monop_parser.py +++ b/monop_parser.py @@ -60,6 +60,28 @@ SQUARE_BY_NAME["JAIL"] = 40 # Special: in-jail location SQUARE_NAME_BY_ID = {sq["id"]: sq["name"] for sq in BOARD} SQUARE_NAME_BY_ID[40] = "JAIL" +# Truncated name (10 chars, like C's printsq) + cost -> square id +# Used to identify properties in trade summary lines +_TRUNC_COST_TO_ID = {} +_TRUNC_TO_IDS = {} # trunc -> list of sq_ids (for ambiguous names) +for _sq in BOARD: + if _sq["type"] in ("property", "railroad", "utility"): + _trunc = _sq["name"][:10].rstrip() + _TRUNC_COST_TO_ID[(_trunc, _sq["cost"])] = _sq["id"] + _TRUNC_TO_IDS.setdefault(_trunc, []).append(_sq["id"]) + + +def resolve_trade_property(trunc_name, cost=None): + """Resolve a truncated property name (+ optional cost) to a square id.""" + if cost is not None: + sq_id = _TRUNC_COST_TO_ID.get((trunc_name, cost)) + if sq_id is not None: + return sq_id + ids = _TRUNC_TO_IDS.get(trunc_name, []) + if len(ids) == 1: + return ids[0] + return None + class Player: def __init__(self, name, number): @@ -73,7 +95,7 @@ class Player: self.get_out_of_jail_free_cards = 0 def to_dict(self): - return { + d = { "name": self.name, "number": self.number, "money": self.money, @@ -83,6 +105,9 @@ class Player: "doublesCount": self.doubles_count, "getOutOfJailFreeCards": self.get_out_of_jail_free_cards, } + if getattr(self, 'bankrupt', False): + d["bankrupt"] = True + return d class GameState: @@ -93,6 +118,7 @@ class GameState: self.current_player_idx = 0 # 0-based index into players list self.phase = "setup" # setup, playing, over self.squares = copy.deepcopy(BOARD) + self.bankrupt_players = [] # Players who resigned/went bankrupt # Property ownership tracking self.property_owner = {} # square_id -> player_number (1-based) self.property_mortgaged = {} # square_id -> bool @@ -196,6 +222,10 @@ class MonopParser: TRADE_CASH_RE = re.compile(r'^\s+\$(\d+)$') TRADE_GOJF_RE = re.compile(r'^\s+(\d+) get-out-of-jail-free card\(s\)$') TRADE_NOTHING_RE = re.compile(r'^\s+-- Nothing --$') + # Trade property line: " NAME OWNER GROUP COST ..." + # C's printsq uses %-10.10s for name, then owner+1, group, cost + TRADE_PROP_RE = re.compile(r'^\s(.{10})\s+(\d+)\s+\S*\s+(\d+)') + SOLVENT_RE = re.compile(r'^-- You are now Solvent ---$') DEBT_RE = re.compile(r'^That leaves you \$(\d+) in debt$') BROKE_RE = re.compile(r'^that leaves you broke$') @@ -218,6 +248,8 @@ class MonopParser: self._trade_cash2 = 0 self._trade_gojf1 = 0 self._trade_gojf2 = 0 + self._trade_props1 = [] # list of square_ids player1 is giving + self._trade_props2 = [] # list of square_ids player2 is giving # State for income tax self._inc_tax_pending = False self._inc_tax_worth = 0 @@ -449,6 +481,7 @@ class MonopParser: p.in_jail = True g.players.append(p) g.current_player_idx = 0 + g._first_player_idx = 0 # assume first seen is first self.checkpoints_validated += 1 return @@ -870,11 +903,13 @@ class MonopParser: self._trade_player1 = (name, num) self._trade_cash1 = 0 self._trade_gojf1 = 0 + self._trade_props1 = [] elif self._trade_state == 'gives1': self._trade_state = 'gives2' self._trade_player2 = (name, num) self._trade_cash2 = 0 self._trade_gojf2 = 0 + self._trade_props2 = [] return m = self.TRADE_CASH_RE.match(msg_raw) @@ -895,14 +930,28 @@ class MonopParser: self._trade_gojf2 = gojf return + # Trade property line (indented printsq output) + if self._trade_state: + m_tp = self.TRADE_PROP_RE.match(msg_raw) + if m_tp: + trunc_name = m_tp.group(1).rstrip() + cost = int(m_tp.group(3)) + sq_id = resolve_trade_property(trunc_name, cost) + if sq_id is not None: + if self._trade_state == 'gives1': + self._trade_props1.append(sq_id) + elif self._trade_state == 'gives2': + self._trade_props2.append(sq_id) + return + if self.TRADE_NOTHING_RE.match(msg_raw) and self._trade_state: return if self.TRADE_DONE_RE.match(msg): if hasattr(self, '_resign_pending') and self._resign_pending: - self._execute_resign_to_player() + self._execute_resign_to_player(timestamp=timestamp) else: - self._execute_trade() + self._execute_trade(timestamp=timestamp) return # ===== RESIGN ===== @@ -912,8 +961,13 @@ class MonopParser: return if self.RESIGN_TO_BANK_RE.match(msg): - # Player resigns to bank - remove them + # Player resigns to bank - properties go back to unowned if cp: + for sq_id, owner_num in list(g.property_owner.items()): + if owner_num == cp.number: + del g.property_owner[sq_id] + g.property_mortgaged.pop(sq_id, None) + g.property_houses.pop(sq_id, None) g.add_log(f"{cp.name} resigned to the bank", player=cp.name, timestamp=timestamp) self._remove_player(cp) return @@ -1098,6 +1152,7 @@ class MonopParser: else: g.add_log(f"Paid ${amount} rent", player=cp.name) g.pending_rent_owner = None + g.spec = False # Clear spec flag after rent (matches C's get_card cleanup) def _process_card(self, lines): """Process card text after both separators have been seen.""" @@ -1196,7 +1251,7 @@ class MonopParser: elif "Advance to St. Charles" in text: pass # Movement handled - def _execute_trade(self): + def _execute_trade(self, timestamp=None): """Execute a completed trade.""" g = self.game if not g: @@ -1214,12 +1269,20 @@ class MonopParser: p2.get_out_of_jail_free_cards += self._trade_gojf1 p2.get_out_of_jail_free_cards -= self._trade_gojf2 p1.get_out_of_jail_free_cards += self._trade_gojf2 - g.add_log(f"Trade completed between {p1.name} and {p2.name}") + # p1 gives properties to p2 + for sq_id in self._trade_props1: + g.property_owner[sq_id] = p2.number + # p2 gives properties to p1 + for sq_id in self._trade_props2: + g.property_owner[sq_id] = p1.number + g.add_log(f"Trade completed between {p1.name} and {p2.name}", timestamp=timestamp) self._trade_state = None self._trade_player1 = None self._trade_player2 = None + self._trade_props1 = [] + self._trade_props2 = [] - def _execute_resign_to_player(self): + def _execute_resign_to_player(self, timestamp=None): """Handle resign-to-player: transfer all assets then remove player.""" g = self.game if not g: @@ -1233,11 +1296,25 @@ class MonopParser: if hasattr(self, '_resign_target') and self._resign_target: target = g.get_player(name=self._resign_target) self._resign_target = None - if target and cp.money > 0: - target.money += cp.money if target: + # Transfer money + if cp.money > 0: + target.money += cp.money + # Transfer GOJF cards target.get_out_of_jail_free_cards += cp.get_out_of_jail_free_cards - g.add_log(f"{cp.name} resigned to {target.name if target else 'bank'}", player=cp.name) + # Transfer properties + for sq_id, owner_num in list(g.property_owner.items()): + if owner_num == cp.number: + g.property_owner[sq_id] = target.number + g.add_log(f"{cp.name} resigned to {target.name}", player=cp.name, timestamp=timestamp) + else: + # No target found — treat as bank resignation + for sq_id, owner_num in list(g.property_owner.items()): + if owner_num == cp.number: + del g.property_owner[sq_id] + g.property_mortgaged.pop(sq_id, None) + g.property_houses.pop(sq_id, None) + g.add_log(f"{cp.name} resigned to the bank", player=cp.name, timestamp=timestamp) self._remove_player(cp) def _remove_player(self, player): @@ -1246,10 +1323,20 @@ class MonopParser: if not g: return idx = g.players.index(player) + player.bankrupt = True + g.bankrupt_players.append(player) g.players.remove(player) + # Build old->new number mapping before renumbering + old_numbers = {p: p.number for p in g.players} # Renumber remaining players (C code shifts array) for i, p in enumerate(g.players): p.number = i + 1 + # Update property_owner references to match new numbers + number_map = {old_numbers[p]: p.number for p in g.players} + for sq_id in list(g.property_owner.keys()): + old_num = g.property_owner[sq_id] + if old_num in number_map: + g.property_owner[sq_id] = number_map[old_num] # C code: player = --player < 0 ? num_play - 1 : player # then next_play() increments to next # After removal, the C code decrements player index then calls next_play @@ -1301,12 +1388,17 @@ class MonopParser: else: ordered = g.players - return { - "players": [p.to_dict() for p in ordered], + # Append bankrupt players at the end + all_players = [p.to_dict() for p in ordered] + [p.to_dict() for p in g.bankrupt_players] + + result = { + "players": all_players, "currentPlayer": g.current_player.number if g.current_player else None, "squares": squares, "log": g.log[-30:], } + result["phase"] = g.phase # "playing" or "over" + return result def parse_log(filepath): diff --git a/plugins/monop/monop_parser.py b/plugins/monop/monop_parser.py index a6a9f1e..18370d3 100644 --- a/plugins/monop/monop_parser.py +++ b/plugins/monop/monop_parser.py @@ -60,6 +60,28 @@ SQUARE_BY_NAME["JAIL"] = 40 # Special: in-jail location SQUARE_NAME_BY_ID = {sq["id"]: sq["name"] for sq in BOARD} SQUARE_NAME_BY_ID[40] = "JAIL" +# Truncated name (10 chars, like C's printsq) + cost -> square id +# Used to identify properties in trade summary lines +_TRUNC_COST_TO_ID = {} +_TRUNC_TO_IDS = {} # trunc -> list of sq_ids (for ambiguous names) +for _sq in BOARD: + if _sq["type"] in ("property", "railroad", "utility"): + _trunc = _sq["name"][:10].rstrip() + _TRUNC_COST_TO_ID[(_trunc, _sq["cost"])] = _sq["id"] + _TRUNC_TO_IDS.setdefault(_trunc, []).append(_sq["id"]) + + +def resolve_trade_property(trunc_name, cost=None): + """Resolve a truncated property name (+ optional cost) to a square id.""" + if cost is not None: + sq_id = _TRUNC_COST_TO_ID.get((trunc_name, cost)) + if sq_id is not None: + return sq_id + ids = _TRUNC_TO_IDS.get(trunc_name, []) + if len(ids) == 1: + return ids[0] + return None + class Player: def __init__(self, name, number): @@ -73,7 +95,7 @@ class Player: self.get_out_of_jail_free_cards = 0 def to_dict(self): - return { + d = { "name": self.name, "number": self.number, "money": self.money, @@ -83,6 +105,9 @@ class Player: "doublesCount": self.doubles_count, "getOutOfJailFreeCards": self.get_out_of_jail_free_cards, } + if getattr(self, 'bankrupt', False): + d["bankrupt"] = True + return d class GameState: @@ -93,6 +118,7 @@ class GameState: self.current_player_idx = 0 # 0-based index into players list self.phase = "setup" # setup, playing, over self.squares = copy.deepcopy(BOARD) + self.bankrupt_players = [] # Players who resigned/went bankrupt # Property ownership tracking self.property_owner = {} # square_id -> player_number (1-based) self.property_mortgaged = {} # square_id -> bool @@ -196,6 +222,10 @@ class MonopParser: TRADE_CASH_RE = re.compile(r'^\s+\$(\d+)$') TRADE_GOJF_RE = re.compile(r'^\s+(\d+) get-out-of-jail-free card\(s\)$') TRADE_NOTHING_RE = re.compile(r'^\s+-- Nothing --$') + # Trade property line: " NAME OWNER GROUP COST ..." + # C's printsq uses %-10.10s for name, then owner+1, group, cost + TRADE_PROP_RE = re.compile(r'^\s(.{10})\s+(\d+)\s+\S*\s+(\d+)') + SOLVENT_RE = re.compile(r'^-- You are now Solvent ---$') DEBT_RE = re.compile(r'^That leaves you \$(\d+) in debt$') BROKE_RE = re.compile(r'^that leaves you broke$') @@ -218,6 +248,8 @@ class MonopParser: self._trade_cash2 = 0 self._trade_gojf1 = 0 self._trade_gojf2 = 0 + self._trade_props1 = [] # list of square_ids player1 is giving + self._trade_props2 = [] # list of square_ids player2 is giving # State for income tax self._inc_tax_pending = False self._inc_tax_worth = 0 @@ -449,6 +481,7 @@ class MonopParser: p.in_jail = True g.players.append(p) g.current_player_idx = 0 + g._first_player_idx = 0 # assume first seen is first self.checkpoints_validated += 1 return @@ -870,11 +903,13 @@ class MonopParser: self._trade_player1 = (name, num) self._trade_cash1 = 0 self._trade_gojf1 = 0 + self._trade_props1 = [] elif self._trade_state == 'gives1': self._trade_state = 'gives2' self._trade_player2 = (name, num) self._trade_cash2 = 0 self._trade_gojf2 = 0 + self._trade_props2 = [] return m = self.TRADE_CASH_RE.match(msg_raw) @@ -895,14 +930,28 @@ class MonopParser: self._trade_gojf2 = gojf return + # Trade property line (indented printsq output) + if self._trade_state: + m_tp = self.TRADE_PROP_RE.match(msg_raw) + if m_tp: + trunc_name = m_tp.group(1).rstrip() + cost = int(m_tp.group(3)) + sq_id = resolve_trade_property(trunc_name, cost) + if sq_id is not None: + if self._trade_state == 'gives1': + self._trade_props1.append(sq_id) + elif self._trade_state == 'gives2': + self._trade_props2.append(sq_id) + return + if self.TRADE_NOTHING_RE.match(msg_raw) and self._trade_state: return if self.TRADE_DONE_RE.match(msg): if hasattr(self, '_resign_pending') and self._resign_pending: - self._execute_resign_to_player() + self._execute_resign_to_player(timestamp=timestamp) else: - self._execute_trade() + self._execute_trade(timestamp=timestamp) return # ===== RESIGN ===== @@ -912,8 +961,13 @@ class MonopParser: return if self.RESIGN_TO_BANK_RE.match(msg): - # Player resigns to bank - remove them + # Player resigns to bank - properties go back to unowned if cp: + for sq_id, owner_num in list(g.property_owner.items()): + if owner_num == cp.number: + del g.property_owner[sq_id] + g.property_mortgaged.pop(sq_id, None) + g.property_houses.pop(sq_id, None) g.add_log(f"{cp.name} resigned to the bank", player=cp.name, timestamp=timestamp) self._remove_player(cp) return @@ -1098,6 +1152,7 @@ class MonopParser: else: g.add_log(f"Paid ${amount} rent", player=cp.name) g.pending_rent_owner = None + g.spec = False # Clear spec flag after rent (matches C's get_card cleanup) def _process_card(self, lines): """Process card text after both separators have been seen.""" @@ -1196,7 +1251,7 @@ class MonopParser: elif "Advance to St. Charles" in text: pass # Movement handled - def _execute_trade(self): + def _execute_trade(self, timestamp=None): """Execute a completed trade.""" g = self.game if not g: @@ -1214,12 +1269,20 @@ class MonopParser: p2.get_out_of_jail_free_cards += self._trade_gojf1 p2.get_out_of_jail_free_cards -= self._trade_gojf2 p1.get_out_of_jail_free_cards += self._trade_gojf2 - g.add_log(f"Trade completed between {p1.name} and {p2.name}") + # p1 gives properties to p2 + for sq_id in self._trade_props1: + g.property_owner[sq_id] = p2.number + # p2 gives properties to p1 + for sq_id in self._trade_props2: + g.property_owner[sq_id] = p1.number + g.add_log(f"Trade completed between {p1.name} and {p2.name}", timestamp=timestamp) self._trade_state = None self._trade_player1 = None self._trade_player2 = None + self._trade_props1 = [] + self._trade_props2 = [] - def _execute_resign_to_player(self): + def _execute_resign_to_player(self, timestamp=None): """Handle resign-to-player: transfer all assets then remove player.""" g = self.game if not g: @@ -1233,11 +1296,25 @@ class MonopParser: if hasattr(self, '_resign_target') and self._resign_target: target = g.get_player(name=self._resign_target) self._resign_target = None - if target and cp.money > 0: - target.money += cp.money if target: + # Transfer money + if cp.money > 0: + target.money += cp.money + # Transfer GOJF cards target.get_out_of_jail_free_cards += cp.get_out_of_jail_free_cards - g.add_log(f"{cp.name} resigned to {target.name if target else 'bank'}", player=cp.name) + # Transfer properties + for sq_id, owner_num in list(g.property_owner.items()): + if owner_num == cp.number: + g.property_owner[sq_id] = target.number + g.add_log(f"{cp.name} resigned to {target.name}", player=cp.name, timestamp=timestamp) + else: + # No target found — treat as bank resignation + for sq_id, owner_num in list(g.property_owner.items()): + if owner_num == cp.number: + del g.property_owner[sq_id] + g.property_mortgaged.pop(sq_id, None) + g.property_houses.pop(sq_id, None) + g.add_log(f"{cp.name} resigned to the bank", player=cp.name, timestamp=timestamp) self._remove_player(cp) def _remove_player(self, player): @@ -1246,10 +1323,20 @@ class MonopParser: if not g: return idx = g.players.index(player) + player.bankrupt = True + g.bankrupt_players.append(player) g.players.remove(player) + # Build old->new number mapping before renumbering + old_numbers = {p: p.number for p in g.players} # Renumber remaining players (C code shifts array) for i, p in enumerate(g.players): p.number = i + 1 + # Update property_owner references to match new numbers + number_map = {old_numbers[p]: p.number for p in g.players} + for sq_id in list(g.property_owner.keys()): + old_num = g.property_owner[sq_id] + if old_num in number_map: + g.property_owner[sq_id] = number_map[old_num] # C code: player = --player < 0 ? num_play - 1 : player # then next_play() increments to next # After removal, the C code decrements player index then calls next_play @@ -1301,12 +1388,17 @@ class MonopParser: else: ordered = g.players - return { - "players": [p.to_dict() for p in ordered], + # Append bankrupt players at the end + all_players = [p.to_dict() for p in ordered] + [p.to_dict() for p in g.bankrupt_players] + + result = { + "players": all_players, "currentPlayer": g.current_player.number if g.current_player else None, "squares": squares, "log": g.log[-30:], } + result["phase"] = g.phase # "playing" or "over" + return result def parse_log(filepath): diff --git a/site/index.html b/site/index.html index 7935957..9c59d16 100644 --- a/site/index.html +++ b/site/index.html @@ -355,12 +355,16 @@ function renderBoard(state) { el.appendChild(costEl); } - if (sq.owner >= 0 && sq.owner < players.length) { - const ownerEl = document.createElement('div'); - ownerEl.className = 'owner-indicator'; - ownerEl.textContent = '⬤'; - ownerEl.style.color = PLAYER_COLORS[sq.owner % PLAYER_COLORS.length]; - el.appendChild(ownerEl); + if (sq.owner != null && sq.owner > 0) { + // Find owner's index in the players array (which is turn-ordered) + const ownerIdx = players.findIndex(p => p.number === sq.owner); + if (ownerIdx >= 0) { + const ownerEl = document.createElement('div'); + ownerEl.className = 'owner-indicator'; + ownerEl.textContent = '⬤'; + ownerEl.style.color = PLAYER_COLORS[ownerIdx % PLAYER_COLORS.length]; + el.appendChild(ownerEl); + } } if (sq.houses > 0 && sq.houses < 5) { @@ -437,6 +441,14 @@ function renderPlayers(state) { panel.innerHTML = `

${p.name.charAt(0).toUpperCase()} ${p.name} ✓

$1,500
Ready to play
`; + } else if (p.bankrupt) { + // Bankrupt player + panel.className = 'player-panel'; + panel.style.opacity = '0.45'; + panel.style.borderColor = '#e74c3c'; + panel.style.borderStyle = 'dashed'; + panel.innerHTML = `

${p.name.charAt(0).toUpperCase()} ${p.name} 💀

+
BANKRUPT
`; } else { // Normal playing state panel.className = 'player-panel' + (p.number === state.currentPlayer ? ' current-turn' : ''); diff --git a/test_parser_bugs.py b/test_parser_bugs.py new file mode 100644 index 0000000..31edc4f --- /dev/null +++ b/test_parser_bugs.py @@ -0,0 +1,302 @@ +#!/usr/bin/env python3 +"""Tests for bugs identified in the codebase audit.""" + +import sys +import os +import unittest + +sys.path.insert(0, os.path.dirname(__file__)) +from monop_parser import MonopParser, BOARD, SQUARE_BY_NAME + +TS = "2026-01-01 00:00:00" + + +def feed(parser, lines): + for line in lines: + parser.parse_line(f"{TS}\t{line}") + + +def setup_3player_game(): + """Set up a 3-player game (merp, hiro, fbs) in playing phase. + fbs goes first (turn order: fbs→merp→hiro).""" + p = MonopParser() + feed(p, [ + "monop\tHow many players? ", + "monop\tPlayer 1, say 'me' please.", + "monop\tmerp (1) rolls 5", + "monop\tPlayer 2, say 'me' please.", + "monop\thiro (2) rolls 3", + "monop\tPlayer 3, say 'me' please.", + "monop\tfbs (3) rolls 8", + "monop\tfbs (3) goes first", + "monop\tfbs (3) (cash $1500) on === GO ===", + "monop\t-- Command: ", + ]) + return p + + +def advance_to_merp_turn(p): + """Advance from fbs's turn to merp's turn.""" + feed(p, [ + "fbs\t.", + "monop\troll is 3, 4", + "monop\tThat puts you on Oriental ave. (L)", + "monop\tThat would cost $100, do you want it? ", + "fbs\t.n", + "monop\tmerp (1) (cash $1500) on === GO ===", + "monop\t-- Command: ", + ]) + + +def give_properties(game, player_num, sq_ids): + for sq_id in sq_ids: + game.property_owner[sq_id] = player_num + + +# ===================================================================== +# Bug #1: Trade doesn't transfer properties +# ===================================================================== +class TestBug1_TradeProperties(unittest.TestCase): + """_execute_trade() only handles cash+GOJF, not property transfers.""" + + def test_trade_property_transfer(self): + """hiro gives Tennessee ave to fbs for $290.""" + p = setup_3player_game() + g = p.game + # hiro (2) owns Tennessee ave (sq 18) + give_properties(g, 2, [18]) + + # fbs's turn — initiate trade + feed(p, [ + # Trade summary + "monop\tPlayer fbs (3) gives:", + "monop\t $290", + "monop\tPlayer hiro (2) gives:", + "monop\t Tennessee 2 Orange 180 * 14", + "monop\thiro, is the trade ok? ", + "hiro\t.y", + "monop\tTrade is done!", + "monop\tfbs (3) (cash $1210) on === GO ===", + "monop\t-- Command: ", + ]) + + # Tennessee (sq 18) should now belong to fbs (3) + self.assertEqual(g.property_owner.get(18), 3, + "Tennessee ave should be transferred to fbs") + # fbs paid $290 + fbs = g.get_player(name="fbs") + self.assertEqual(fbs.money, 1500 - 290) + # hiro received $290 + hiro = g.get_player(name="hiro") + self.assertEqual(hiro.money, 1500 + 290) + + def test_trade_multiple_properties(self): + """xLink gives Indiana + Illinois to fbs for $550.""" + p = setup_3player_game() + g = p.game + # Use merp as xLink stand-in (player 1 owns Indiana=23, Illinois=24) + give_properties(g, 1, [23, 24]) + + feed(p, [ + "monop\tPlayer fbs (3) gives:", + "monop\t $550", + "monop\tPlayer merp (1) gives:", + "monop\t Indiana av 1 Red 220 * 18", + "monop\t Illinois a 1 Red 240 * 20", + "monop\tmerp, is the trade ok? ", + "merp\t.y", + "monop\tTrade is done!", + "monop\tfbs (3) (cash $950) on === GO ===", + "monop\t-- Command: ", + ]) + + self.assertEqual(g.property_owner.get(23), 3, "Indiana should be fbs's") + self.assertEqual(g.property_owner.get(24), 3, "Illinois should be fbs's") + + def test_real_log_trade_line_789(self): + """Replay real log through trade at line 789 — Tennessee from hiro to fbs.""" + p = MonopParser() + with open("test_data/monop.log") as f: + for i, line in enumerate(f, 1): + p.parse_line(line.rstrip('\n')) + if i >= 795: + break + g = p.game + state = p.get_state() + self.assertIsNotNone(state) + # Tennessee ave (sq 18) should be owned by fbs after the trade + # fbs is player 2 at this point in the log + owner = g.property_owner.get(18) + self.assertIsNotNone(owner, "Tennessee should be owned after trade") + + +# ===================================================================== +# Bug #2: sq.owner uses player number but UI indexes by array position +# ===================================================================== +class TestBug2_OwnerColorIndex(unittest.TestCase): + """UI uses sq.owner as array index, but it's a 1-based player number. + This is a UI-only bug — test the data contract instead.""" + + def test_owner_is_player_number_not_index(self): + """Verify get_state() emits owner as player number (1-based).""" + p = setup_3player_game() + g = p.game + # fbs is player 3, give them a property + give_properties(g, 3, [1]) # Mediterranean + state = p.get_state() + sq1 = next(sq for sq in state["squares"] if sq["id"] == 1) + # owner should be 3 (fbs's player number), not 2 (array index) + self.assertEqual(sq1["owner"], 3) + + +# ===================================================================== +# Bug #3: _first_player_idx not set on mid-stream pickup +# ===================================================================== +class TestBug3_FirstPlayerIdx(unittest.TestCase): + """When game is picked up from a checkpoint, _first_player_idx is never set.""" + + def test_midstream_pickup_has_first_player_idx(self): + """Picking up game from checkpoint should set _first_player_idx.""" + p = MonopParser() + feed(p, [ + "monop\tmerp (1) (cash $1200) on Connecticut ave. (L)", + "monop\t-- Command: ", + ]) + g = p.game + self.assertIsNotNone(g) + self.assertTrue(hasattr(g, '_first_player_idx'), + "_first_player_idx should be set on mid-stream pickup") + + +# ===================================================================== +# Bug #4: Resign-to-player with no target falls through to bank msg +# ===================================================================== +class TestBug4_ResignNoTarget(unittest.TestCase): + """If _resign_target can't resolve, logs 'resigned to bank' but + doesn't clear properties like the real bank path.""" + + def test_resign_unresolved_target_clears_properties(self): + """If resign target can't be found, treat as bank resignation.""" + p = setup_3player_game() + g = p.game + give_properties(g, 1, [1, 3]) # merp owns Mediterranean, Baltic + advance_to_merp_turn(p) + + # Force _resign_target to a bad name + p._resign_target = "nonexistent_player" + p._resign_pending = True + feed(p, [ + "monop\tTrade is done!", + "monop\thiro (1) (cash $1500) on === GO ===", + "monop\t-- Command: ", + ]) + + # Properties should be cleared (bank behavior) + self.assertNotIn(1, g.property_owner, "Props should be cleared on failed resign target") + self.assertNotIn(3, g.property_owner) + + +# ===================================================================== +# Bug #5: Houses/mortgages not transferred on resign-to-player +# ===================================================================== +class TestBug5_ResignTransferHouses(unittest.TestCase): + """When resigning to a player, houses/mortgage state should transfer.""" + + def test_houses_preserved_on_resign(self): + """Houses on transferred properties should be preserved.""" + p = setup_3player_game() + g = p.game + give_properties(g, 1, [1, 3]) + g.property_houses[1] = 3 # 3 houses on Mediterranean + g.property_mortgaged[3] = True # Baltic mortgaged + advance_to_merp_turn(p) + + feed(p, [ + "monop\tYou would resign to fbs", + "monop\tDo you really want to resign? ", + "merp\t.y", + "monop\tresigning to player", + "monop\tTrade is done!", + "monop\thiro (1) (cash $1500) on === GO ===", + "monop\t-- Command: ", + ]) + + fbs = g.get_player(name="fbs") + # Properties transferred to fbs + self.assertEqual(g.property_owner.get(1), fbs.number) + # Houses should be preserved + self.assertEqual(g.property_houses.get(1), 3, + "Houses should be preserved on transfer") + # Mortgage state should be preserved + self.assertTrue(g.property_mortgaged.get(3), + "Mortgage state should be preserved on transfer") + + +# ===================================================================== +# Bug #6: spec flag never consumed (Chance: nearest RR/utility) +# ===================================================================== +class TestBug6_SpecFlag(unittest.TestCase): + """The spec flag is set for 'Advance to nearest Railroad/Utility' + Chance cards but never cleared, potentially affecting future rent.""" + + def test_spec_flag_cleared_after_rent(self): + """spec should be cleared after rent is calculated.""" + p = setup_3player_game() + g = p.game + g.spec = True # simulate having drawn the card + + # Player pays rent + give_properties(g, 2, [5]) # hiro owns Reading RR + feed(p, [ + "monop\tOwned by hiro", + "monop\trent is 25", + "monop\thiro (2) (cash $1500) on === GO ===", + "monop\t-- Command: ", + ]) + + self.assertFalse(g.spec, "spec flag should be cleared after rent payment") + + +# ===================================================================== +# Bug #13: get_state() doesn't emit phase: "playing" +# ===================================================================== +class TestBug13_PhasePlaying(unittest.TestCase): + """get_state() should always include phase field.""" + + def test_phase_playing_emitted(self): + p = setup_3player_game() + state = p.get_state() + self.assertIn("phase", state, "State should always include phase") + self.assertEqual(state["phase"], "playing") + + +# ===================================================================== +# Bug #16: Resign log entries missing timestamps +# ===================================================================== +class TestBug16_ResignTimestamp(unittest.TestCase): + """Resign-to-player log entries should have timestamps.""" + + def test_resign_to_player_has_timestamp(self): + p = setup_3player_game() + g = p.game + advance_to_merp_turn(p) + + feed(p, [ + "monop\tYou would resign to fbs", + "monop\tDo you really want to resign? ", + "merp\t.y", + "monop\tresigning to player", + "monop\tTrade is done!", + "monop\thiro (1) (cash $1500) on === GO ===", + "monop\t-- Command: ", + ]) + + resign_entries = [e for e in g.log if "resigned" in e.get("text", "")] + self.assertTrue(len(resign_entries) > 0, "Should have resign log entry") + self.assertIn("timestamp", resign_entries[0], + "Resign log entry should have a timestamp") + self.assertEqual(resign_entries[0]["timestamp"], TS) + + +if __name__ == "__main__": + unittest.main(verbosity=2) diff --git a/test_parser_resign.py b/test_parser_resign.py new file mode 100644 index 0000000..6e5d707 --- /dev/null +++ b/test_parser_resign.py @@ -0,0 +1,364 @@ +#!/usr/bin/env python3 +"""Unit tests for parser resignation handling — both to-player and to-bank.""" + +import sys +import os +import unittest + +sys.path.insert(0, os.path.dirname(__file__)) +from monop_parser import MonopParser + + +TS = "2026-01-01 00:00:00" + + +def feed(parser, lines): + """Feed tab-delimited lines (sender\\tmessage) into the parser.""" + for line in lines: + parser.parse_line(f"{TS}\t{line}") + + +def setup_3player_game(): + """Set up a 3-player game (merp, hiro, fbs) in playing phase. + Uses the DarkScience-style setup that the parser actually handles.""" + p = MonopParser() + feed(p, [ + # Start a new game + "monop\tHow many players? ", + # Setup: say me + rolls + "monop\tPlayer 1, say 'me' please.", + "monop\tmerp (1) rolls 5", + "monop\tPlayer 2, say 'me' please.", + "monop\thiro (2) rolls 3", + "monop\tPlayer 3, say 'me' please.", + "monop\tfbs (3) rolls 8", + # fbs rolls highest, goes first + "monop\tfbs (3) goes first", + # First checkpoint to confirm playing state + "monop\tfbs (3) (cash $1500) on === GO ===", + "monop\t-- Command: ", + ]) + return p + + +def give_properties(game, player_num, sq_ids): + """Assign properties to a player in game state.""" + for sq_id in sq_ids: + game.property_owner[sq_id] = player_num + + +class TestResignToBank(unittest.TestCase): + """Test resignation to bank — properties become unowned.""" + + def _resign_merp_to_bank(self): + """Set up game, give merp properties, resign to bank. + + Note: fbs goes first (highest roll). Turn order is fbs→merp→hiro. + We need it to be merp's turn for the resign to work.""" + p = setup_3player_game() + g = p.game + + # Give merp some properties and houses/mortgages + give_properties(g, 1, [1, 3, 5]) # Mediterranean, Baltic, Reading RR + g.property_houses[1] = 2 + g.property_mortgaged[3] = True + + # fbs rolls, then it's merp's turn + feed(p, [ + "fbs\t.", + "monop\troll is 3, 4", + "monop\tThat puts you on Oriental ave. (L)", + "monop\tThat would cost $100, do you want it? ", + "fbs\t.n", + "monop\tmerp (1) (cash $1500) on === GO ===", + "monop\t-- Command: ", + # Now merp resigns to bank + "merp\t.resign", + "monop\tWho do you wish to resign to? ", + "merp\t.bank", + "monop\tDo you really want to resign? ", + "merp\t.y", + "monop\tresigning to bank", + "monop\thiro (1) (cash $1500) on === GO ===", + "monop\t-- Command: ", + ]) + return p + + def test_player_removed_from_active(self): + p = self._resign_merp_to_bank() + state = p.get_state() + active_names = [pl["name"] for pl in state["players"] if not pl.get("bankrupt")] + self.assertNotIn("merp", active_names) + self.assertEqual(len(active_names), 2) + + def test_player_marked_bankrupt(self): + p = self._resign_merp_to_bank() + state = p.get_state() + bankrupt = [pl for pl in state["players"] if pl.get("bankrupt")] + self.assertEqual(len(bankrupt), 1) + self.assertEqual(bankrupt[0]["name"], "merp") + + def test_properties_cleared(self): + p = self._resign_merp_to_bank() + g = p.game + for sq_id in [1, 3, 5]: + self.assertNotIn(sq_id, g.property_owner, + f"Property {sq_id} should be unowned after resign to bank") + + def test_houses_cleared(self): + p = self._resign_merp_to_bank() + self.assertNotIn(1, p.game.property_houses) + + def test_mortgages_cleared(self): + p = self._resign_merp_to_bank() + self.assertNotIn(3, p.game.property_mortgaged) + + def test_remaining_players_renumbered(self): + p = self._resign_merp_to_bank() + state = p.get_state() + active = [pl for pl in state["players"] if not pl.get("bankrupt")] + names = {pl["name"]: pl["number"] for pl in active} + # After merp removed, remaining get renumbered 1, 2 + self.assertIn("hiro", names) + self.assertIn("fbs", names) + + def test_log_entry(self): + p = self._resign_merp_to_bank() + state = p.get_state() + log_texts = [e["text"] for e in state["log"]] + self.assertTrue(any("merp" in t and "bank" in t for t in log_texts), + f"Expected resign-to-bank log entry, got: {log_texts}") + + +class TestResignToPlayer(unittest.TestCase): + """Test resignation to another player — assets transferred.""" + + def _resign_merp_to_fbs(self, extra_setup=None): + """merp resigns to fbs. Optional extra_setup callback for custom state.""" + p = setup_3player_game() + g = p.game + + # Give merp properties and money + give_properties(g, 1, [1, 3, 5]) + merp = g.get_player(name="merp") + merp.money = 800 + merp.get_out_of_jail_free_cards = 1 + + if extra_setup: + extra_setup(p) + + # fbs turn, then merp's turn + feed(p, [ + "fbs\t.", + "monop\troll is 3, 4", + "monop\tThat puts you on Oriental ave. (L)", + "monop\tThat would cost $100, do you want it? ", + "fbs\t.n", + "monop\tmerp (1) (cash $800) on === GO ===", + "monop\t-- Command: ", + # merp resigns — auto-target since negative money scenario uses "You would resign to" + "monop\tYou would resign to fbs", + "monop\tDo you really want to resign? ", + "merp\t.y", + "monop\tresigning to player", + "monop\tTrade is done!", + "monop\thiro (1) (cash $1500) on === GO ===", + "monop\t-- Command: ", + ]) + return p + + def test_player_marked_bankrupt(self): + p = self._resign_merp_to_fbs() + state = p.get_state() + bankrupt = [pl for pl in state["players"] if pl.get("bankrupt")] + self.assertEqual(len(bankrupt), 1) + self.assertEqual(bankrupt[0]["name"], "merp") + + def test_money_transferred(self): + p = self._resign_merp_to_fbs() + fbs = p.game.get_player(name="fbs") + self.assertIsNotNone(fbs) + # fbs started with 1500, gets merp's 800 + self.assertEqual(fbs.money, 1500 + 800) + + def test_gojf_transferred(self): + p = self._resign_merp_to_fbs() + fbs = p.game.get_player(name="fbs") + self.assertEqual(fbs.get_out_of_jail_free_cards, 1) + + def test_properties_transferred(self): + p = self._resign_merp_to_fbs() + g = p.game + fbs = g.get_player(name="fbs") + for sq_id in [1, 3, 5]: + self.assertEqual(g.property_owner.get(sq_id), fbs.number, + f"Property {sq_id} should belong to fbs (#{fbs.number})") + + def test_other_player_properties_preserved(self): + """hiro's properties survive with correct renumbered owner.""" + def setup(parser): + give_properties(parser.game, 2, [11, 13]) # hiro owns St Charles, States + + p = self._resign_merp_to_fbs(extra_setup=setup) + g = p.game + hiro = g.get_player(name="hiro") + self.assertIsNotNone(hiro, "hiro should still be in the game") + for sq_id in [11, 13]: + self.assertEqual(g.property_owner.get(sq_id), hiro.number, + f"Property {sq_id} should belong to hiro (#{hiro.number})") + + def test_log_entry(self): + p = self._resign_merp_to_fbs() + state = p.get_state() + log_texts = [e["text"] for e in state["log"]] + self.assertTrue(any("merp" in t and "fbs" in t for t in log_texts), + f"Expected resign-to-fbs log entry, got: {log_texts}") + + +class TestResignWithChosenTarget(unittest.TestCase): + """Player with positive money chooses who to resign to via 'Who do you wish to resign to?'""" + + def test_resign_choose_hiro(self): + p = setup_3player_game() + give_properties(p.game, 1, [1, 3]) + + feed(p, [ + # fbs turn + "fbs\t.", + "monop\troll is 3, 4", + "monop\tThat puts you on Oriental ave. (L)", + "monop\tThat would cost $100, do you want it? ", + "fbs\t.n", + "monop\tmerp (1) (cash $1500) on === GO ===", + "monop\t-- Command: ", + # merp chooses to resign to hiro + "merp\t.resign", + "monop\tWho do you wish to resign to? ", + "merp\t.hiro", + "monop\tDo you really want to resign? ", + "merp\t.y", + "monop\tresigning to player", + "monop\tTrade is done!", + "monop\thiro (1) (cash $3000) on === GO ===", + "monop\t-- Command: ", + ]) + + g = p.game + hiro = g.get_player(name="hiro") + self.assertIsNotNone(hiro) + # Properties transferred to hiro + for sq_id in [1, 3]: + self.assertEqual(g.property_owner.get(sq_id), hiro.number) + # Money transferred (1500 + 1500) + self.assertEqual(hiro.money, 3000) + + +class TestResignGameOver(unittest.TestCase): + """Resignations leading to game over.""" + + def test_last_two_players_one_resigns(self): + """In 3-player game: merp resigns, then hiro resigns → fbs wins.""" + p = setup_3player_game() + + feed(p, [ + # fbs turn, then merp's + "fbs\t.", + "monop\troll is 3, 4", + "monop\tThat puts you on Oriental ave. (L)", + "monop\tThat would cost $100, do you want it? ", + "fbs\t.n", + "monop\tmerp (1) (cash $1500) on === GO ===", + "monop\t-- Command: ", + # merp resigns to bank + "merp\t.resign", + "monop\tWho do you wish to resign to? ", + "merp\t.bank", + "monop\tDo you really want to resign? ", + "merp\t.y", + "monop\tresigning to bank", + "monop\thiro (1) (cash $1500) on === GO ===", + "monop\t-- Command: ", + # hiro's turn, rolls + "hiro\t.", + "monop\troll is 2, 3", + "monop\tThat puts you on Reading RR", + "monop\tThat would cost $200, do you want it? ", + "hiro\t.n", + "monop\tfbs (2) (cash $1500) on Oriental ave. (L)", + "monop\t-- Command: ", + # fbs turn, then hiro again + "fbs\t.", + "monop\troll is 1, 2", + "monop\tThat puts you on Connecticut ave. (L)", + "monop\tThat would cost $120, do you want it? ", + "fbs\t.n", + "monop\thiro (1) (cash $1500) on Reading RR", + "monop\t-- Command: ", + # hiro resigns to fbs + "monop\tYou would resign to fbs", + "monop\tDo you really want to resign? ", + "hiro\t.y", + "monop\tresigning to player", + "monop\tTrade is done!", + "monop\tThen fbs WINS!!!!!", + ]) + + state = p.get_state() + self.assertEqual(state.get("phase"), "over") + bankrupt = [pl for pl in state["players"] if pl.get("bankrupt")] + self.assertEqual(len(bankrupt), 2) + active = [pl for pl in state["players"] if not pl.get("bankrupt")] + self.assertEqual(len(active), 1) + self.assertEqual(active[0]["name"], "fbs") + + +class TestLogReplayResignations(unittest.TestCase): + """Replay real log segments involving resignations.""" + + def _replay_to_line(self, end_line): + p = MonopParser() + with open("test_data/monop.log") as f: + for i, line in enumerate(f, 1): + p.parse_line(line.rstrip('\n')) + if i >= end_line: + break + return p + + def test_merp_resigns_to_fbs_line_1012(self): + """Real log: merp resigns to fbs around line 1012.""" + p = self._replay_to_line(1020) + state = p.get_state() + self.assertIsNotNone(state) + bankrupt_names = [pl["name"] for pl in state["players"] if pl.get("bankrupt")] + self.assertIn("merp", bankrupt_names) + active_names = [pl["name"] for pl in state["players"] if not pl.get("bankrupt")] + self.assertIn("hiro", active_names) + self.assertIn("fbs", active_names) + + def test_xlink_resigns_to_fbs_line_1245(self): + """Real log: xLink resigns to fbs around line 1245.""" + p = self._replay_to_line(1250) + state = p.get_state() + self.assertIsNotNone(state) + bankrupt_names = [pl["name"] for pl in state["players"] if pl.get("bankrupt")] + self.assertIn("xLink", bankrupt_names) + + def test_derecho_resigns_to_bank_line_6874(self): + """Real log: Derecho resigns to bank around line 6874.""" + p = self._replay_to_line(6880) + state = p.get_state() + self.assertIsNotNone(state) + # Should not crash — that's the main assertion + # Check bankrupt list includes someone + bankrupt_names = [pl["name"] for pl in state["players"] if pl.get("bankrupt")] + self.assertTrue(len(bankrupt_names) > 0, "Expected at least one bankrupt player by line 6880") + + def test_whoami_resigns_to_bank_line_10729(self): + """Real log: whoami resigns to bank around line 10729.""" + p = self._replay_to_line(10735) + state = p.get_state() + self.assertIsNotNone(state) + + +if __name__ == "__main__": + unittest.main(verbosity=2)