ctm / mb2-doc

Mb2, poker software
https://devctm.com
7 stars 2 forks source link

PRP panic #675

Closed ctm closed 3 years ago

ctm commented 3 years ago
2021-07-25T00:41:26.727284490Z thread '<unnamed>' panicked at 'should not get here', lib/commands/src/game/choice.rs:74:9
2021-07-25T00:41:26.727325170Z stack backtrace:
[...]
2021-07-25T00:41:27.361382717Z   14:     0x55fb4c9bb839 - <mb2_commands::game::choice::ParadisePickEm as mb2_commands::game_group::GameGroup>::hand_winnings::h0df6f6c83e614db6
2021-07-25T00:41:27.361395657Z                                at /usr/src/poker/lib/commands/src/game/choice.rs:74:9
2021-07-25T00:41:27.436587228Z   15:     0x55fb4c91d47b - mb2::table::Table::showdown::h870ec0ca5ead0e28
2021-07-25T00:41:27.436662809Z                                at /usr/src/poker/mb2/src/table.rs:2087:40
2021-07-25T00:41:27.436781730Z   16:     0x55fb4c919b9b - mb2::table::Table::deal::hb0ec53dca91e875a
2021-07-25T00:41:27.436852661Z                                at /usr/src/poker/mb2/src/table.rs:1437:21
2021-07-25T00:41:27.466333704Z   17:     0x55fb4c7a6445 - <mb2::table::Table as mb2::table::NextActions>::continue_dealing::hef3bdafa9be4b4df
2021-07-25T00:41:27.466414635Z                                at /usr/src/poker/mb2/src/table.rs:4150:9
2021-07-25T00:41:27.466426436Z   18:     0x55fb4c7a6445 - mb2::tournament::RunningEvent::act::h9933c8b463c30143
2021-07-25T00:41:27.466432156Z                                at /usr/src/poker/mb2/src/tournament.rs:740:35
2021-07-25T00:41:27.470829330Z   19:     0x55fb4c7a5d3b - mb2::tournament::RunningEvent::run::habcb5283231a0bf3
2021-07-25T00:41:27.470907111Z                                at /usr/src/poker/mb2/src/tournament.rs:667:36

The hand involved was probably #202937:

Dealing #202937: 1000 2000 Paradise Road Pick'em

🐭GamboMouse blinds 500
AIYAH❗ blinds 775 and is all-in
1    V SuperKM        7900   0  [  ] [  ] [  ]
2 B   deadhead        3900   0  [  ] [  ] [  ]
3     🐭GamboMouse     6475 500  [  ] [  ] [  ]
4     AIYAH❗          0 775  [  ] [  ] [  ]
5   > pokerchimp🐵     12850   0  [  ] [  ] [  ]
6     💀smalltalkdan💀  29250   0  [  ] [  ] [  ]
7     JADC🚴           8350   0  [  ] [  ] [  ]
                     My cards are: [Qc] [5s] [Th]
pokerchimp🐵 asked for and gets 40 more seconds
The game is now 1000 2000 Texas Hold’em (High only)
1    V SuperKM        7900   0  [  ] [  ] [  ]
2 B   deadhead        3900   0  [  ] [  ] [  ]
3     🐭GamboMouse     6475 500  [  ] [  ] [  ]
4     AIYAH❗          0 775  [  ] [  ] [  ]
5   > pokerchimp🐵     12850   0  [  ] [  ] [  ]
6     💀smalltalkdan💀  29250   0  [  ] [  ] [  ]
7     JADC🚴           8350   0  [  ] [  ] [  ]
                     My cards are: [Qc] [5s] [Th]
When it's your turn to act, you'll fold
pokerchimp🐵 folds
💀smalltalkdan💀 raises 1000 to 2000
💀smalltalkdan💀 discards 1
JADC🚴 folds
SuperKM discards 1
SuperKM folds due to being on vacation
deadhead folds
🐭GamboMouse folds
🐭GamboMouse: glai
AIYAH❗ checks
1    [SuperKM]        7900[   0] 
2 B  [deadhead]       3900[   0] 
3    [🐭GamboMouse]    6475[ 500] 
4     AIYAH❗          0  775  7d 3h Td
5    [pokerchimp🐵]    12850[   0] 
6     💀smalltalkdan💀  27250 2000  6s As
7    [JADC🚴]          8350[   0] 
1    [SuperKM]        7900[   0] 
2 B  [deadhead]       3900[   0] 
3    [🐭GamboMouse]    6475[ 500] 
4     AIYAH❗          0  775  7d 3h Td
5    [pokerchimp🐵]    12850[   0] 
6     💀smalltalkdan💀  27250 2000  6s As
7    [JADC🚴]          8350[   0] 
Board: Kd Ks Tc
AIYAH❗: ooh can I play all 3 cards?
1    [SuperKM]        7900[   0] 
2 B  [deadhead]       3900[   0] 
3    [🐭GamboMouse]    6475[ 500] 
4     AIYAH❗          0  775  7d 3h Td
5    [pokerchimp🐵]    12850[   0] 
6     💀smalltalkdan💀  27250 2000  6s As
7    [JADC🚴]          8350[   0] 
Board: Kd Ks Tc 8h
🐭GamboMouse: sure
1    [SuperKM]        7900[   0] 
2 B  [deadhead]       3900[   0] 
3    [🐭GamboMouse]    6475[ 500] 
4     AIYAH❗          0  775  7d 3h Td
5    [pokerchimp🐵]    12850[   0] 
6     💀smalltalkdan💀  27250 2000  6s As
7    [JADC🚴]          8350[   0] 
Board: Kd Ks Tc 8h 5c
Uncalled 1225 returned to 💀smalltalkdan💀
🐭GamboMouse: ruh roh
AIYAH❗: BOOM goes the dynamite!
💀smalltalkdan💀: um
🐭GamboMouse: Bus Error.  Core Dumped
kidZee: paging Mr. DH to the white courtesy phone
💀smalltalkdan💀: I have too many chips!
🐭GamboMouse: segmentation violation at line 105 of kermod
deadhead: I'm trying to get into the server, but ssh is failing ...
gerdog: you're in a cave with many passages
deadhead: Yeah, it died.
🐭GamboMouse: heh
deadhead: Sorry folks. It won't count against (or for) the standings.
💀smalltalkdan💀: sigh. OK.
deadhead: I've got a lot of work to do on this game. I'll start posting in the lobby so others can see.

In addition to the crash, AIYAH didn't have to discard. That may actually be a separate issue. If so, I'll split it off...

ctm commented 3 years ago

Hmmm... This appears to be that part of mb2 is still thinking the "game" is "PRP" while other parts of mb2 are thinking the "game" is "Hold'em". So PRP itself doesn't have a functioning hand_winnings method, yet that's what's getting called.

This should be trivial to find and fix.

ctm commented 3 years ago

Sure enough:

                    self.upcoming_game = Some(game);
                    self.betting_rounds = betting_rounds;
                    self.betting_rounds.set_index(betting_round_index);
                    if self.drawing.is_some() {
                        self.table_message(TableMessage::ChosenGame(desc, true));
                    } else {
                        self.table_message(TableMessage::ChosenGame(desc, false));
                        // FWIW, this may be unnecessary once we adjust
                        // game_keeper, because we may wind up sending
                        // status at the beginning of each betting round
                        // anyway.  I don't rememember.
                        self.send_status(None);
                        self.choices = None;
                    }

Mb2 sets up the betting rounds without actually telling the GameKeeper that we're playing a new game.

I can't remember why I deferred updating GameKeeper. I'll poke around a bit and see if anything jars my memory. I should have written a comment, but I'm sure it was obvious at the time (maybe if I get a few more decades of programming under my belt I'll learn my lesson!). Anyway, there are two solutions. The cleanest is to not defer the assignment. The other is to also add an assignment where all-ins short-circuit the path that normally has the deferred assignment. Ugh!

ctm commented 3 years ago

Git to the rescue:

commit e60909f501d900e6347b16feb663818b5f487742
Author: Clifford T. Matthews <ctm@devctm.com>
Date:   Sun Jun 27 12:09:08 2021 -0600

    [Bug] allows UTG raises in Paradise Road Pick'em Stud.  Closes #641.

diff --git a/mb2/src/table.rs b/mb2/src/table.rs
index d110d794..a63028fa 100644
--- a/mb2/src/table.rs
+++ b/mb2/src/table.rs
@@ -394,6 +394,7 @@ pub struct Table {
     button: Button,
     betting_rounds: BettingRounds,
     pub(crate) game: Box<dyn GameGroup + Send + Sync>,
+    upcoming_game: Option<Box<dyn GameGroup + Send + Sync>>,
...

We used to switch the game immediately, but that precluded UTG raises in stud. I'll add a comment to the code and then add the appropriate clean-up assignment to game when we do the short-circuit.

ctm commented 3 years ago

This bug is sufficiently embarrassing (it crashed yesterday's evening tournament) and the issue is sufficiently tricky that I'm going to wear my underpants on the outside just to show that I've cleaned them up and to encourage me to soil them less frequently.

diff --git a/mb2/src/table.rs b/mb2/src/table.rs
index 8d6610db..87fe3be1 100644
--- a/mb2/src/table.rs
+++ b/mb2/src/table.rs
@@ -420,6 +420,11 @@ pub struct Table {
     button: Button,
     betting_rounds: BettingRounds,
     pub(crate) game: Box<dyn GameGroup + Send + Sync>,
+    // upcoming_game is used by Paradise Road Pick'em to defer
+    // changing the raising semantics until after the cleanup betting
+    // round has been completed because stud during cleanup allows the
+    // first to act to raise (and there's no concept of a bring-in or
+    // completion).
     upcoming_game: Option<Box<dyn GameGroup + Send + Sync>>,
     pub(crate) on_break: bool,
     to_nick_mapper: ToNickMapper,
@@ -891,6 +896,12 @@ impl Table {
         self.awaiting_redraw = true;
     }

+    pub(crate) fn switch_to_upcoming_game(&mut self) {
+        if let Some(game) = self.upcoming_game.take() {
+            self.game = game;
+        }
+    }
+
     #[allow(clippy::too_many_arguments)]
     pub(crate) fn new(
         id: TableId,
@@ -3196,6 +3207,16 @@ impl Table {
         true
     }

+    // The switch_to_upcoming_game calls are slightly tricky, in that
+    // currently we could just put a single call to
+    // switch_to_upcoming_game in continue_dealing, because currently
+    // our only use of upcoming_game is in the cleanup portion of
+    // Paradise Road Pick'em and that portion uses combined
+    // card-manipulation and action.  However, a previous version of
+    // Paradise Road Pick'em used Mixed Deal where card-manipulation
+    // and action were separate and were we ever to allow such a
+    // thing, we *wouldn't* want to switch to the upcoming game until
+    // after the manipulation was done.
     fn proceed_to_next_player_after(&mut self, seat: SeatIndex) {
         use NextAction::*;

@@ -3203,8 +3224,10 @@ impl Table {
         self.action_timer = None;
         self.timer_sequence = self.timer_sequence.wrapping_add(1);
         let message = if self.n_not_folded < 2 {
+            self.switch_to_upcoming_game();
             AwardUncontested(self.id)
         } else if self.n_not_all_in < 1 && self.drawing.is_none() {
+            self.switch_to_upcoming_game();
             ContinueDealing(self.id)
         } else {
             let next_to_act = self.next_actionable_seat(seat);
@@ -3223,9 +3246,7 @@ impl Table {
                     || self.choices.is_some())
             {
                 if self.drawing.is_none() {
-                    if let Some(game) = self.upcoming_game.take() {
-                        self.game = game;
-                    }
+                    self.switch_to_upcoming_game();
                     ContinueDealing(self.id)
                 } else {
                     self.choices = None;
ctm commented 3 years ago

Deploying now.