ctm / mb2-doc

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

Allow people to choose the game in advance in Dealer's Choice #1262

Closed ctm closed 5 months ago

ctm commented 8 months ago

Make it so people playing Dealer's Choice can choose the game in advance of their time to actually choose it.

ctm commented 6 months ago

I just added high priority to this, although I don't plan on working on it soon. I do, however, want to get it implemented and tested well before May 30th, which is when craftpoker.com will be dealing Dealer's Choice for the first time in the 2024 WSOP-Style series. Unfortunately, if I blink twice, it's May 30th. That's my super power.

ctm commented 5 months ago

Branch choice-1262 has the server code present, but untested. I should be able to hack in text support with an hour or two of work, although I don't know if I have that much programming time left in the day.

Once the code is working in text mode, my guess is I can it to the GUI in another day or so of full-time work, which might wind up being done over several days.

ctm commented 5 months ago

The text interface appears to work. The choice, however, is a value 0 through 20 which corresponds to the order of the games inside the structure, which isn't visible to players. That works fine for testing but is frustrating for users, especially since no attempt is made to tell the player that the advanced choice has been accepted, much less which game was chosen.

My guess is I'll do a mini code review and deploy later today, perhaps changing how the argument to choose is interpreted. My reason for deploying even before the GUI has the functionality is to potentially catch bugs sooner rather than later, because I definitely had to do some minor refactoring of the choice code.

ctm commented 5 months ago

The GUI choice picker contains this code (in Table::game_choser):

                let mut choices = info.choices.iter().enumerate().collect::<Vec<_>>();
                if choices.len() > 10 {
                    choices[1..].sort_by_key(|c| c.1);
                }

So, to accept 1-21, I need to abstract out that sorting so it's only done in one place. I also haven't tested Paradise Road Pick'em, and there the choices are 1-5 (but there's no sorting).

I can use the above info to do some sanity checking and have the text interface actually state what game has been chosen so that people can't, for example, submit 30 only to get a complaint when it's finally their turn to choose. I do—of course—have to continue checking at the server level so that people can't hack the protocol and cause badness.

ctm commented 5 months ago

I'm deploying a version now where the text interface works, but gives no feedback. Later today, I'll see if I can quickly hack in graphical functionality and once that's done, I'll go back and make the text interface provide feedback.

We're playing Dealer's Choice this evening, so I'm hoping I can get the graphical interface functionality in place by then.

ctm commented 5 months ago

The pull-down menu appears to work. There's a little problem in that if you choose in advance and then close the window, the pull-down will not yet reflect that you made that choice. This is a known problem that I believe I've opened as an issue before, but I can't find it.

ctm commented 5 months ago

The issue that I was thinking about in my last comment was #1192 and it's closed, which is why I couldn't find it, so I'll add another checkbox to the things to do in the description so as not to overlook the problem.

ctm commented 5 months ago

Yesterday evening's test worked well from my perspective, however, there were times when other people didn't appear to have chosen in advance that could have been a bug. I did, just now, see a bug when I was exploring the problem with "Dealer's Choice" being announced as the game choice (#1359). Specifically, I created a 2 player demo with WSOP 2023 05 Dealer's Choice as the structure and two players. I had the first player choose the game manually and after it was chosen (perhaps with a hand or two finishing), I set the second player's advance choice to Badugi. However, when it came time for the second player to choose, instead of us moving directly to Badugi, the second player got the choice dialog with Badugi pre-chosen.

I do not yet know if I can trivially reproduce the problem I just saw. I'm documenting it now, before trying to reproduce it so that I get as many details written down before I forget. If I can trivially reproduce it, it'll be trivial to fix. Otherwise…

ctm commented 5 months ago

Unfortunately, I tried twice to reproduce the problem and was unsuccessful. In my first try I accidentally let player one time out when choosing, which is why I tried a second time. I can, however, check the db logs for the one that showed the glitch and see more, so I'll do that now.

ctm commented 5 months ago

Local db FTW.

This is an action synchronizer issue, which makes sense, but is a little disappointing.

To reproduce it, have a player make an advanced choice in the time between a hand ends and a new hand is dealt.

I added the action synchronizer to make sure people's advanced actions applied to the action that their client was on. However, when I added choice at time-to-choose, I included an ActionSynchronizer field in the new table::Request::Choose variant. That worked fine and I thought nothing more of it. However, when I added advance choice, I kept that field and didn't think about the fact that there may be times when the GUI and server disagree, like the interval between the end of a hand and the start of a new hand.

I'll spend a little time thinking about this before I solve it. Do I really want to use the action synchronizer for game choice? If so, I need to make sure it's correct even when there's no action…

ctm commented 5 months ago

All the functionality is done. Much of it was tested in yesterday's evening game, but a few issues were found. I've fixed the issues except the for the announcing that the game that has been selected is "Dealer's Choice". I created a separate issue (#1359) for that and will work on it next.

Deploying now.