ctm / mb2-doc

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

Initial Level message claims to be final #1281

Closed ctm closed 6 months ago

ctm commented 6 months ago

Fix so the initial level knows when it will be over.

With full messages on in chat, I see:

We're on the final level: Level 1

as the first message. I also noticed in the graphical interface (but didn't screenshot) that it only said "Level 1" in the upper right and didn't have the time that it would end.

This might be a regression, but I'm not sufficiently confident that it is to add the regression label.

ctm commented 6 months ago

The broadcast messages all have the duration, but in addition to them, various player-specific messages were sent that don't have the duration.

mb2=> select id, received_at at time zone 'america/denver', message from public_table_messages where table_id = 5584 and variant = 43 and player_id is null order by received_at;
    id    |          timezone          |                                     message                                      
----------+----------------------------+----------------------------------------------------------------------------------
 10335527 | 2024-01-14 18:05:00.27197  | {"OnLevel": ["Level 1", [299999, "2024-01-15T01:10:00.196440708Z"], false, 0]}
 10335760 | 2024-01-14 18:10:00.199149 | {"OnLevel": ["Level 2", [299999, "2024-01-15T01:15:00.197640772Z"], false, 1]}
 10336005 | 2024-01-14 18:15:00.209706 | {"OnLevel": ["Level 3", [299999, "2024-01-15T01:20:00.198630061Z"], false, 2]}
...
mb2=> select id, player_id, received_at at time zone 'america/denver', message from public_table_messages where table_id = 5584 and variant = 43 and player_id is not null order by received_at;
    id    | player_id |          timezone          |                                    message                                     
----------+-----------+----------------------------+--------------------------------------------------------------------------------
 10335530 |         3 | 2024-01-14 18:05:01.117798 | {"OnLevel": ["Level 1", null, false, 0]}
 10335532 |         3 | 2024-01-14 18:05:01.128184 | {"OnLevel": ["Level 1", [299069, "2024-01-15T01:10:00.196440708Z"], false, 0]}
 10335541 |        10 | 2024-01-14 18:05:02.308467 | {"OnLevel": ["Level 1", null, false, 0]}
 10335550 |        11 | 2024-01-14 18:05:02.504408 | {"OnLevel": ["Level 1", null, false, 0]}
 10335559 |        13 | 2024-01-14 18:05:03.623382 | {"OnLevel": ["Level 1", null, false, 0]}
 10335571 |        14 | 2024-01-14 18:05:37.537343 | {"OnLevel": ["Level 1", null, false, 0]}
 10335583 |        17 | 2024-01-14 18:05:39.5628   | {"OnLevel": ["Level 1", null, false, 0]}
 10337226 |         6 | 2024-01-14 18:42:53.37508  | {"OnLevel": ["Level 8", null, false, 7]}
(8 rows)

It looks like the level, but without the duration is sent specifically to players as part of them being seated, which is silly if the replay already contains an OnLevel message and is also poor form to not have the duration. My guess is I'll find that the OnLevel is being sent from NickMapper, which doesn't necessarily know the duration, but the solution would be to just have NickMapper as the tournament to send the complete message.

But there's one more curiosity: Player 3 got a personalized message that did contain the duration. Where the heck did that come from?

ctm commented 6 months ago

The player-specific OnLevel without the duration comes from Table::replay. The broadcast OnLevel that has a duration comes from RunningEvent::anounce_level. I do not see any OnLevel-specific code for sending a player-specific OnLevel, so my guess is it's a replay of the initial OnLevel and that only player 3 got that replay because the captured messages had rolled over for everyone else.

I try not to work after 8pm, so I'll look at this more tomorrow.

ctm commented 6 months ago

AFAICT, the only reason Table::replay doesn't specify the duration is that RunningEvent has it in its current_level_expires_at field and a Table doesn't have access to the RunningEvent that it belongs to. Putting that field in an Rc<RefCell> and then sharing it with Table seems like a reasonable and easy thing to do. That would solve this bug.

There's a separate issue that certain messages that are generated upon connection may already be in the replay buffer and so we send them more than once. That's inefficient and ugly, but doesn't result in any badness except for the lack of duration. So, I'll make a new issue for that before closing out this one.

ctm commented 6 months ago

FWIW, current_level_expires_at is already passed in to advance_to, so if we share with an Rc, we can get rid of that parameter, or, if we simply add a current_level_expires_at field to Table we can just clone and save it for reuse later.

ctm commented 6 months ago

Turns out replay wasn't in Table, but was in ToNickMapper, so in addition to stashing current_level_expires_at in Table, I had to add another parameter to replay, which I did.

This fix works and has been deployed.