Merge #222

222: Improve transaction logic, fix SimpleRatio underflow r=charleskawczynski a=charleskawczynski

This PR improves the transaction logic and fixes SimpleRatio underflow. Closes #220, #200 and #223.

Co-authored-by: Charles Kawczynski <[email protected]>

Project.toml

@@ -1,7 +1,7 @@
 name = "TexasHoldem"
 uuid = "6cef90fc-eb55-4a2a-97d0-7ecce2b738fe"
 authors = ["Charles Kawczynski <[email protected]>"]
-version = "0.3.2"
+version = "0.4.0"
 
 [deps]
 Logging = "56ddb016-857b-54e1-b83d-db4d58db5568"

src/chips.jl

@@ -5,6 +5,11 @@ export SimpleRatio
 struct SimpleRatio
     num::Int
     den::Int
+    function SimpleRatio(num, den)
+        # https://github.com/charleskawczynski/TexasHoldem.jl/issues/223
+        _den = num == 0 ? 1 : den
+        return new(num, _den)
+    end
 end
 
 Base.:(+)(x::Int, y::SimpleRatio) = SimpleRatio(x*y.den + y.num, y.den)

src/game.jl

@@ -330,7 +330,7 @@ function _deal_and_play!(game::Game, sf::StartFrom)
     @cdebug logger "initial_∑brs = $(initial_∑brs)"
     @cdebug logger "sum(bank_roll.(players)) = $(sum(bank_roll.(players)))"
     @cdebug logger "initial_brs = $(initial_brs)"
-    @cdebug logger "bank_roll.(players) = $(bank_roll.(players))"
+    @cdebug logger "bank_roll_chips.(players) = $(bank_roll_chips.(players))"
 
     if sf.game_point isa StartOfGame
         if !(logger isa ByPassLogger)
@@ -346,12 +346,14 @@ function _deal_and_play!(game::Game, sf::StartFrom)
         prof = bank_roll_chips(player) - initial_br
         br = map(x->bank_roll_chips(x), players)
         # TODO: this is broken due to https://github.com/charleskawczynski/TexasHoldem.jl/issues/200
-        # @assert prof ≤ mpp string("Over-winning occurred:\n",
-        #       "    Player $(name(player))\n",
-        #       "    Initial BRs $(initial_brs)\n",
-        #       "    BRs $br\n",
-        #       "    profit $prof\n",
-        #       "    max possible profit $mpp")
+        @assert prof ≤ mpp string("Over-winning occurred:\n",
+              "    Player $(name(player))\n",
+              "    Initial BRs $(initial_brs)\n",
+              "    BRs $br\n",
+              "    profit $prof\n",
+              "    profit.n $(prof.n)\n",
+              "    cond $(prof.n ≤ mpp.n)\n",
+              "    max possible profit $mpp")
     end
 
     @cinfo logger "Final bank roll summary: $(bank_roll_chips.(players))"

src/transactions.jl

@@ -157,6 +157,7 @@ of the (sorted) players at the start of the game.
 function contribute!(table, player, amt, call=false)
     tm = table.transactions
     logger = table.logger
+    players = players_at_table(table)
     @cdebug logger "$(name(player))'s bank roll (pre-contribute) = $(bank_roll(player))"
     if !(0 ≤ amt ≤ bank_roll(player))
         msg1 = "$(name(player)) has insufficient bank"
@@ -175,16 +176,19 @@ function contribute!(table, player, amt, call=false)
 
     @inbounds for i in 1:length(tm.side_pots)
         @assert 0 ≤ amt_remaining
-        cap_i = cap(tm.side_pots[i])
-        cond = amt_remaining < cap_i
-        amt_contrib = cond ? amt_remaining : cap_i
-        contributing = !side_pot_full(tm, i) && !(cap_i == 0)
-        # This is a bit noisy:
-        # @cdebug logger "$(name(player)) potentially contributing $amt_contrib to side-pot $(i) ($cond). cap_i=$cap_i, amt_remaining=$amt_remaining"
-        @cdebug logger "contributing, amt_contrib = $contributing, $amt_contrib"
-        contributing || continue
-        @assert !(amt_contrib == 0)
-        tm.side_pots[i].amts[seat_number(player)] += amt_contrib
+        sp = tm.side_pots[i]
+        sn = seat_number(player)
+        if is_player_contribution_to_side_pot_full(sp, sn)
+            continue
+        end
+        @assert 0 < amt_remaining "amt_remaining: $amt_remaining"
+        amt_contrib = if contribution_fits_in_sidepot(sp, sn, amt_remaining)
+            amt_remaining
+        else
+            contribution_that_fits_in_sidepot(sp, sn)
+        end
+        @assert 0 < amt_contrib
+        sp.amts[sn] += amt_contrib
         player.bank_roll -= amt_contrib
         amt_remaining -= amt_contrib
         amt_remaining == 0 && break
@@ -196,24 +200,27 @@ function contribute!(table, player, amt, call=false)
         player.action_required = false
     end
 
-    if is_side_pot_full(tm, table, player, call)
+    if is_side_pot_full(tm, players)
         increment_pot_id!(tm)
     end
     @cdebug logger "$(name(player))'s bank roll (post-contribute) = $(bank_roll(player))"
     @cdebug logger "all_in($(name(player))) = $(all_in(player))"
     @cdebug logger "post-contribute side-pots: $(tm.side_pots)"
 end
 
-function is_side_pot_full(tm::TransactionManager, table, player, call)
-    players = players_at_table(table)
-    # To switch from pot_id = 1 to pot_id = 2, then exactly 1 player  should be all-in:
-    # To switch from pot_id = 2 to pot_id = 3, then exactly 2 players should be all-in:
-    # ...
-    return @inbounds count(x->all_in(x), players) == tm.pot_id[1] && last_action_of_round(table, player, call)
+increment_pot_id!(tm::TransactionManager) = (tm.pot_id[1]+=1)
+function is_side_pot_full(tm, players, ith_sidepot = tm.pot_id[1])
+    sp = tm.side_pots[ith_sidepot]
+    all(x->is_player_contribution_to_side_pot_full(sp, seat_number(x)), players)
 end
+is_player_contribution_to_side_pot_full(side_pot, sn::Int) =
+    side_pot.amts[sn] == cap(side_pot)
 
-increment_pot_id!(tm::TransactionManager) = (tm.pot_id[1]+=1)
-side_pot_full(tm::TransactionManager, i) = i < tm.pot_id[1]
+contribution_fits_in_sidepot(side_pot, sn::Int, contribution) =
+    side_pot.amts[sn] + contribution ≤ cap(side_pot)
+
+contribution_that_fits_in_sidepot(side_pot, sn::Int) =
+    cap(side_pot) - side_pot.amts[sn]
 
 Base.@propagate_inbounds function sidepot_winnings(tm::TransactionManager, id::Int)
     mapreduce(i->sum(tm.side_pots[i].amts), +, 1:id; init=0)

test/chips.jl

@@ -11,4 +11,5 @@ end
     @test a + b == Chips(4, SimpleRatio(1, 2))
     @test Chips(12, SimpleRatio(0, 1)) == Chips(12, SimpleRatio(0, 2))
     @test Chips(12, SimpleRatio(1, 3)) ≠ Chips(12, SimpleRatio(1, 2))
+    @test Chips(94, SimpleRatio(-518400, 1036800)) ≤ Chips(175, SimpleRatio(0, 4031078400000))
 end

test/fuzz_play.jl

@@ -1,5 +1,7 @@
 #=
 using Revise; include("test/fuzz_utils.jl")
+@eval Main using TexasHoldem # don't qualify types in log
+@eval Main using TexasHoldem:SidePot # don't qualify types in log
 
 to debug cases, use (for example):
 fuzz_debug(;fun=tournament!,n_players=10,bank_roll=30,n_games=3788)
@@ -12,6 +14,7 @@ players = (
     Player(Bot5050(), 3; bank_roll=4),
 )
 fuzz_given_players_debug(;fun=play!, players, n_games=138)
+fuzz_debug(; fun = tournament!, n_players = 10, bank_roll = 30, n_games = 1310)
 fuzz_debug(; fun = tournament!, n_players = 2, bank_roll = 6, n_games = 1)
 fuzz_debug(; fun = tournament!, n_players = 3, bank_roll = 6, n_games = 38)
 fuzz_debug(; fun = play!, n_players = 3, bank_roll = 200, n_games = 2373)

test/transactions.jl

@@ -370,9 +370,12 @@ end
     @test TH.amounts.(tm.side_pots) == [[4, 1, 2], [1, 0, 0], [0, 0, 0]]
 
     call!(table, players[2]) # call
-    @test_broken TH.amounts.(tm.side_pots) == [10, 2, 0]
+    @test TH.amounts.(tm.side_pots) == [[4, 4, 2], [1, 1, 0], [0, 0, 0]]
 
     call!(table, players[3]) # call
-    @test_broken TH.amounts.(tm.side_pots) == [12, 2, 0]
-    @test_broken TH.side_pot_full(tm, 1) == true
+    @test TH.amounts.(tm.side_pots) == [[4, 4, 4], [1, 1, 0], [0, 0, 0]]
+    @test TH.is_side_pot_full(tm, players, 1)
+    # TODO: all but one player is all-in, is it okay that this
+    #       side-pot is considered not full?
+    @test !TH.is_side_pot_full(tm, players, 2)
 end