ctm / mb2-doc

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

Table not displayed immediately on joining / refreshing #1273

Closed ctm closed 7 months ago

ctm commented 7 months ago

Fix so that creating a new table window gets the table displayed immdiately.

Currently, there's no guarantee that message replay will have a Status message in it, so if someone opens a table window during a pause, they'll get an empty window. This is trivial to reproduce by forcing table_remains_after_rebalance to pause waiting for a non-existant redraw:

     fn table_remains_after_rebalance(&mut self, table_id: TableId) -> bool {
+        // DO NOT COMMIT
+        self.mut_table_at(table_id).unwrap().pause_for_redraw(); // DO NOT COMMIT
+        return true; // DO NOT COMMIT
+        // DO NOT COMMIT
+        

Add that code, play a hand, the close the table window and the problem can be seen.

Currently, Table::replay already prepends an OnLevel and also rummages through the stored messages looking for PlayerIds, so while it's looking through each of the messages for PlayerIds we can also determine if there's already a Status message that will be replayed and if there isn't, we can prepend one along with the OnLevel.

ctm commented 7 months ago

Oops. Table doesn't implement replay, ToNickMapper does, which doesn't have access to the Table's state.

I think a refactor is in order here. I can add a HashSet<PlayerId> and add to that each time a message is pushed to messages, then we don't need to recompute that set each time we do a replay. Similarly, we can just hold onto the last Status that was sent and keep a bool around that tells us if messages contains a Status and prepend the one we hold onto if it doesn't.

I'm also curious as to why we clear the messages when we receive an EndOfHand rather than a Dealing message. FWIW, EndOfHand's variant is 30 and Dealing is 38, so there's a possibility that I added the clear before we even were sending a Dealing.

ctm commented 7 months ago

Although Dealing existed when I added that clear, the shuffle pause wasn't present, nor did we have late registration, so there wasn't much difference between EndOfHand and the immediately following Dealing. It seems to me like a better way to do it now is to keep track of the hand number and once we see a Dealing for a new hand number, go back and prune away all the messages at the front until, but not including the Dealing for that new hand number. Then there's no need to even cache a separate Status, since there will always be one.

ctm commented 7 months ago

My algorithm in the above comment is wrong. I meant to say that when we see a Status, we prune back to the most recent Dealing. That's what I've implemented and am deploying now.