ctm / mb2-doc

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

Demos created before a restart can't be started #1434

Closed ctm closed 1 month ago

ctm commented 1 month ago

Figure out what to do about demos that are created before a restart

Currently, NickMapper has a demo_info_for_player field that is a HashMap<PlayerId, DemoInfo> that is populated as when NickMapper gets a Login::Demo message. However, after that's been processed, the demo user is given a LobbySessionId which—like any other LobbySessionId—continues to work after a restart. However, demo_info_for_player is not repopulated on restart. Perhaps it should be.

This is a fairly unimportant edge case. AFAIK, it's never come up IRL, because there had been a bug where if it happens, mb2 would panic (#1433). Today, during testing, is the first time I ever hit it. I've since fixed that bug so that if someone tries to start a demo that was created before a restart, we get an error in our logs. However, we already have this comment:

            Login::Demo(demo_params, client) => {
                // Do we want to log demo requests? ... PROBABLY
                MessageResult(self.demo(demo_params, client, _ctx))
            }

where we currently don't log demo requests, which means that debugging demo problems is that much harder, but I can't recall ever having a demo problem to investigate, much less one where the investigation would have benefitted from logging there.

So, although it would be fairly easy to log enough info to reconstitute demo_info_for_player after a restart, it would be functionality that would be used so infrequently that it might fail to work at all and nobody would notice it.

So, I think for now the thing to do is nothing, although I'll first take a super quick stab at invalidating at restart LobbySesionId tht belong to demo users.

ctm commented 1 month ago

One line fix:

diff --git a/mb2/src/models.rs b/mb2/src/models.rs
index d862981e..3d44bcd4 100644
--- a/mb2/src/models.rs
+++ b/mb2/src/models.rs
@@ -1690,6 +1690,7 @@ impl LobbySession {

         let sessions = lobby_sessions
             .inner_join(players)
+            .filter(demo.is_null())
             .select((self::lobby_sessions::dsl::id, player_id, nick))
             .load::<LobbySessionNick>(conn)
             .expect("Could not load Lobby Sessions");

Deploying now.

ctm commented 1 month ago

Turns out, that fix does indeed get demo users logged out after a restart, but they then can't use the demo button. Oops.