ctm / mb2-doc

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

Link to tournament gives white screen #1420

Closed ctm closed 1 month ago

ctm commented 1 month ago

Fix event/player links to work again.

This link should work, but it doesn't right now. This used to work.

ctm commented 1 month ago

Ugh. It started working as I was trying to figure out what is going on. It worked fine locally, so I retried the link in the description and that worked, too. D'oh!

ctm commented 1 month ago

Turns out I can reproduce it. I just have to use a tournament that ended while server was still running. The reason the link in the description works is because I had upgraded the server.

ctm commented 1 month ago

This looks to be a regression I created with my fix for #1386. I should be able to fix it early tomorrow morning.

ctm commented 1 month ago

There are a few issues here that should be examined, the first being that attempting a reconnect after the event is over should result in a tournament summary, just like it would if it weren't a reconnect. The second is that we shouldn't be attempting a reconnect if the tournament is over, but we do so because in #1386, I made it so the table session id isn't deleted when its read. I think I should also make it be deleted if we close the the table after the tournament is over. There's a chance that mb2 is already doing that. That's only two issues, but I think the first one might get split into a non-existent table session id being passed in vs. a currently open table window being passed in. It may wind up being the exact same code path regardless, but I need to make sure.

IOW, I didn't do enough testing at the end of my day yesterday to get all the nuances down, but I think I know all the areas of the code that need to be scrutinized.

ctm commented 1 month ago

The fix for the first issue was trivial. As such, I don't think there's any reason to try to clean up the table session id earlier than we're already doing. I can't think of a problem that its presence causes other than this one which is no longer an issue.

diff --git a/mb2/src/mb2/nick_mapper.rs b/mb2/src/mb2/nick_mapper.rs
index 93a100cf..baafbd13 100644
--- a/mb2/src/mb2/nick_mapper.rs
+++ b/mb2/src/mb2/nick_mapper.rs
@@ -2182,6 +2182,22 @@ impl Handler<Sit> for NickMapper {
         let Sit(SitRequest(to_track, table_session_id), client) = msg;
         self.log_sit_request(&to_track, table_session_id);

+        // If we're trying to track an EventPlayer for an event that
+        // is not running, that may be an instance of someone using
+        // the event summary link while accidentally submitting a
+        // table_session_id.  So, we don't even check the
+        // table_session_id; we just observe.  See #1420 for more
+        // details.
+
+        match &to_track {
+            ToTrack::EventPlayer(event_id, _) => {
+                if !self.tournaments.contains_key(event_id) {
+                    return self.observe(&to_track, Some(client));
+                }
+            }
+            ToTrack::Table(..) => {}
+        }
+
         match table_session_id {
             None => self.observe(&to_track, Some(client)),
             Some(table_session_id) => self.reconnect(table_session_id, client, ctx),

Deploying now.

ctm commented 1 month ago

With that diff staring me in the face, I decided to change the code slightly to must make table_session_id mutable and set it to None when we detect that event_id refers to an event no longer running.

diff --git a/mb2/src/mb2/nick_mapper.rs b/mb2/src/mb2/nick_mapper.rs
index baafbd13..f060bba9 100644
--- a/mb2/src/mb2/nick_mapper.rs
+++ b/mb2/src/mb2/nick_mapper.rs
@@ -2179,20 +2179,19 @@ impl Handler<Sit> for NickMapper {
     type Result = MessageResult<Sit>;

     fn handle(&mut self, msg: Sit, ctx: &mut Self::Context) -> Self::Result {
-        let Sit(SitRequest(to_track, table_session_id), client) = msg;
+        let Sit(SitRequest(to_track, mut table_session_id), client) = msg;
         self.log_sit_request(&to_track, table_session_id);

         // If we're trying to track an EventPlayer for an event that
         // is not running, that may be an instance of someone using
         // the event summary link while accidentally submitting a
-        // table_session_id.  So, we don't even check the
-        // table_session_id; we just observe.  See #1420 for more
-        // details.
+        // table_session_id.  So, we completely ignore the
+        // table_session_id.  See #1420 for more details.

         match &to_track {
             ToTrack::EventPlayer(event_id, _) => {
                 if !self.tournaments.contains_key(event_id) {
-                    return self.observe(&to_track, Some(client));
+                    table_session_id = None;
                 }
             }
             ToTrack::Table(..) => {}