9.7 KiB
Bugs & Fixes Log
This document tracks bugs found, their root cause, the fix applied, and which tests guard against regression. Refer to this before making changes to the parser, UI, or bridge.
Fixed Bugs
Bug #1: Trade doesn't transfer properties
Found: 2026-02-21
Severity: High — property ownership map becomes permanently wrong after any trade
Root cause: _execute_trade() transferred cash and GOJF cards but ignored property lists entirely. Trade summary lines from printsq() (e.g. Tennessee 3 Orange 180 * 14) were swallowed by catch-all returns.
C behavior: do_trade() → move_em() iterates from->prop_list, calls add_list/del_list to transfer each property's ownership (sqr->owner = to->trader).
Fix: Added TRADE_PROP_RE regex to parse printsq-format lines during trade state. Built resolve_trade_property() using truncated-name+cost lookup (handles the "Pennsylvan" ambiguity between RR and ave via cost). Properties tracked in _trade_props1/_trade_props2 and transferred in _execute_trade().
Tests: test_parser_bugs.py::TestBug1_TradeProperties (3 tests including real log replay at line 789)
Bug #2: UI owner indicator color off-by-one
Found: 2026-02-21
Severity: Medium — wrong colors on board squares
Root cause: sq.owner is a 1-based player number, but the UI used it directly as an array index into PLAYER_COLORS. Additionally, after turn-order rotation, player numbers don't correspond to array positions.
Fix: Changed board rendering to players.findIndex(p => p.number === sq.owner) — looks up the player by number in the turn-ordered array, ensuring the owner dot color matches the player token color.
Tests: test_parser_bugs.py::TestBug2_OwnerColorIndex (data contract verification)
Bug #3: _first_player_idx not set on mid-stream pickup
Found: 2026-02-21
Severity: Low — turn order display wrong when parser picks up an in-progress game
Root cause: _first_player_idx is only set in the "goes first" setup path. When a game is detected from a checkpoint line (mid-stream), this attribute is never set, so get_state() can't rotate the player list correctly.
Fix: Set g._first_player_idx = 0 when creating a game from a checkpoint.
Tests: test_parser_bugs.py::TestBug3_FirstPlayerIdx
Bug #4: Resign-to-player with unresolved target doesn't clear properties
Found: 2026-02-21
Severity: Medium — ghost ownership if resign target name can't be matched
Root cause: In _execute_resign_to_player, the else branch (no target found) logged "resigned to the bank" but didn't clear property_owner, property_mortgaged, or property_houses — unlike the explicit bank resignation path.
Fix: Added bank-style property cleanup in the else branch.
Tests: test_parser_bugs.py::TestBug4_ResignNoTarget
Bug #5: Resign-to-bank doesn't clear properties
Found: 2026-02-21
Severity: High — properties stay "owned" by a removed player after bank resignation
Root cause: _remove_player() removed the player from the list but left their entries in property_owner. The explicit bank resignation handler also didn't clear properties, mortgages, or houses.
C behavior: resign() for bank case iterates cur_p->own_list, sets sqp->owner = -1, sqp->desc->morg = FALSE, sqp->desc->houses = 0.
Fix: Bank resignation path now deletes ownership, mortgage, and house entries. Also fixed property owner renumbering after _remove_player() shifts remaining player numbers.
Tests: test_parser_resign.py::TestResignToBank (7 tests)
Bug #6: Resign-to-player doesn't transfer properties
Found: 2026-02-21
Severity: High — transferred assets incomplete
Root cause: _execute_resign_to_player transferred money and GOJF cards but not properties.
C behavior: resign() for player case uses do_trade() with trades[1].prop_list = cur_p->own_list, which transfers all properties via move_em().
Fix: Added property transfer loop in the target branch of _execute_resign_to_player.
Tests: test_parser_resign.py::TestResignToPlayer (6 tests), test_parser_resign.py::TestResignWithChosenTarget
Bug #7: Property owner numbers stale after player removal
Found: 2026-02-21
Severity: High — after resignation, property_owner references point to wrong players
Root cause: _remove_player() renumbers remaining players (C code shifts the array), but property_owner entries still reference old numbers.
C behavior: After removing a player: for (i = 0; i < N_SQRS; i++) if (board[i].owner > player) --board[i].owner;
Fix: Added renumber pass in _remove_player() that builds old→new number mapping and updates all property_owner values.
Tests: test_parser_resign.py::TestResignToBank::test_remaining_players_renumbered, test_parser_resign.py::TestResignToPlayer::test_other_player_properties_preserved
Bug #8: spec flag never cleared
Found: 2026-02-21
Severity: Low — could affect rent calculation display for railroads/utilities
Root cause: spec is set when "Advance to nearest Railroad/Utility" Chance cards are drawn (doubles RR rent, forces 10x utility multiplier), but never cleared afterward.
C behavior: spec = FALSE; at the end of get_card(), after movement and rent have been processed.
Fix: Clear g.spec = False in _pay_rent().
Tests: test_parser_bugs.py::TestBug6_SpecFlag
Bug #9: get_state() doesn't emit phase: "playing"
Found: 2026-02-21
Severity: Low — UI had to infer playing state by absence of phase field
Root cause: get_state() only added phase for "setup" and "over", not "playing".
Fix: Changed to always emit result["phase"] = g.phase.
Tests: test_parser_bugs.py::TestBug13_PhasePlaying
Bug #10: Resign and trade log entries missing timestamps
Found: 2026-02-21
Severity: Low — log entries for trades/resignations had no timestamp in the game log
Root cause: _execute_resign_to_player() and _execute_trade() called g.add_log() without passing timestamp.
Fix: Threaded timestamp parameter from the "Trade is done!" handler through to both methods.
Tests: test_parser_bugs.py::TestBug16_ResignTimestamp
Bug #11: House counts from rent lines not tracked
Found: 2026-02-21
Severity: Medium — UI always showed 0 houses on properties
Root cause: When "with N houses, rent is X" or "with a hotel, rent is X" appeared, the parser extracted rent but didn't update property_houses.
Fix: Update property_houses for the current player's location when these rent lines appear.
Note: This is a partial fix — houses are synced when rent is charged, but not at the moment of purchase (see Known Limitations).
Tests: Covered by existing parser checkpoint tests (1551 checkpoints)
Known Limitations
These are behaviors we can't fully track from parser output alone:
GOJF card usage invisible
When a player types .card in jail, the C code silently decrements num_gojf, sets loc = 10, and in_jail = 0 — with no output. The parser can't detect this, so getOutOfJailFreeCards count stays wrong. Location self-corrects at the next checkpoint.
House counts not tracked at purchase time
The interactive house buying dialog ("PropName (N): " prompts) is ignored by the parser. We only learn house counts when rent is later charged on that property. A full fix would require parsing the entire house buy/sell dialog, matching truncated property names to squares.
Holdings display ignored
printhold() outputs detailed property info (owner, group, cost, mortgage flag, house count) that could be used to periodically resync state. Currently the parser discards these lines. Parsing them would provide self-correction for houses, mortgages, and ownership.
Unmortgage tracking incomplete
"That cost you $X" for unmortgage is handled for money deduction, but property_mortgaged isn't cleared because we can't determine which property was unmortgaged from the cost alone. Mortgage status self-corrects if a holdings display is parsed (not yet implemented) or when the property appears in a rent line (mortgaged properties show "The thing is mortgaged").
printsq property name truncation
The C code truncates property names to 10 characters (%-10.10s). This creates one ambiguity: "Pennsylvan" maps to both Pennsylvania RR (cost $200) and Pennsylvania ave. (cost $320). Resolved by cost in trade parsing, but could be an issue if future code matches by name alone.
Test Coverage Summary
| Test File | Count | Coverage |
|---|---|---|
test_parser.py |
1551 checkpoints | Full log replay — money, location, player list at every checkpoint |
test_parser_resign.py |
19 tests | Both resign types, property/money/GOJF transfer, renumbering, game over, real log replay |
test_parser_bugs.py |
10 tests | Trade property transfer, owner data contract, mid-stream pickup, spec flag, phase field, timestamps |
test_players.py |
38 tests | Player bot behavior — setup, prompts, jail, trading, debt, doubles |
Acceptable mismatches: 2 checkpoints in test_parser.py (line ~11682) caused by IRC disconnects in the original log, not parser bugs.
How to Verify
cd /tmp/monop-state
# Full test suite
python3 test_parser.py # Log replay (1551 checkpoints)
python3 test_parser_resign.py # Resignation tests (19)
python3 test_parser_bugs.py # Bug regression tests (10)
python3 test_players.py # Player bot tests (38)
All tests should pass. If any fail after a change, check this document for the relevant bug — the regression likely reintroduced it.