Release the front-end wherever the game ends, and let the browser see the ended screen

src/fishE.py

@@ -59,6 +59,14 @@ def __init__(self, interfaceType=INTERFACE_TYPE):
         # Show save file selection menu (uses the UI above)
         self._selectSaveFile()
 
+        # "Quit" from that menu clears running rather than ending the process,
+        # so the front-end still gets its cleanup() (see _selectSaveFile and
+        # play()). Nothing below can run without a chosen slot - get_save_path()
+        # raises when none was selected - and there is no run to build anyway,
+        # so construction stops here and play() returns immediately.
+        if not self.running:
+            return
+
         # Load the chosen slot over the defaults if it has data.
         #
         # Existence is the only condition: a file that is present but empty is a
@@ -153,7 +161,10 @@ def _selectSaveFile(self):
         """Display the save-file menu through the UI and let the player choose.
 
         Slots and actions are presented as numbered options (so the menu renders
-        and reads input through the active front-end — console or pygame)."""
+        and reads input through the active front-end — console or pygame).
+
+        Returns once a slot is selected, or with self.running cleared if the
+        player chose "Quit" without picking one."""
         while True:  # loop instead of recursion to avoid stack overflow
             save_files = self.saveFileManager.list_save_files()
 
@@ -217,7 +228,16 @@ def _selectSaveFile(self):
                 self._deleteSaveFile(save_files)
                 # loop to show the refreshed menu either way
             elif kind == "quit":
-                exit(0)
+                # Ending the run rather than the process. exit(0) killed the
+                # interpreter from inside __init__, so cleanup() was never
+                # reached: the pygame window vanished without pygame.quit(),
+                # and both browser front-ends left the tab on the save-file
+                # menu with no ended screen (the Pyodide entry point had to
+                # catch SystemExit and post one itself). __init__ returns
+                # early on a cleared running flag, and play() then does
+                # nothing but clean up - one exit path for every front-end.
+                self.running = False
+                return
             elif kind == "damaged":
                 # A conforming front-end refuses to return an unavailable
                 # option's number, so this should be unreachable. It is handled
@@ -266,6 +286,21 @@ def _deleteSaveFile(self, save_files):
         return False
 
     def play(self):
+        """Run the game loop, releasing the front-end however it ends.
+
+        The cleanup() call is here, once, rather than at each of the places a
+        run can finish (retiring, quitting, an unhandled error): every
+        front-end needs it and only this method sees all of those endings.
+        Without it the pygame window closed without pygame.quit(), and the
+        browser front-ends never published their ended screen - the tab kept
+        polling a server that had exited and told a player who had just retired
+        that the connection was lost. It is a no-op for the console."""
+        try:
+            self._runGameLoop()
+        finally:
+            self.userInterface.cleanup()
+
+    def _runGameLoop(self):
         while self.running:
             # show the current location and goal progress in the UI header
             self.userInterface.currentLocationName = self.currentLocation.capitalize()

src/ui/webUserInterface.py

@@ -21,6 +21,19 @@
     os.path.join(os.path.dirname(os.path.abspath(__file__)), "..", "..", "web")
 )
 
+# How long cleanup() gives the browser to collect the ended screen before it
+# closes the server, and how often it checks whether that has happened.
+#
+# The client only discovers a new screen on its next poll, so shutting the
+# socket the instant the ended screen is published closes it before the screen
+# can be fetched: a player who had just retired was shown "Lost connection"
+# instead of the end of their run. The wait ends as soon as the screen has
+# actually gone out (typically within one poll interval), so the timeout is
+# only ever paid when nobody is listening - a closed tab, or a game driven by
+# something other than a browser.
+ENDED_SCREEN_DELIVERY_TIMEOUT_SECONDS = 2.0
+ENDED_SCREEN_DELIVERY_POLL_SECONDS = 0.02
+
 
 def _readWebAsset(name):
     """Read a shared browser-client file from web/, or explain why it could not."""
@@ -99,6 +112,10 @@ def htmlPage():
     failures = 0;
     if (recovered) version = -1;  // force a re-render to clear the disconnect banner
     if (state.version !== version) { version = state.version; FisheClient.render(state.screen); }
+    // The game shuts its server down once this screen has gone out, so there
+    // is nothing left to poll for: stop, rather than spend the rest of the
+    // tab's life failing to reach a process that has finished.
+    if (state.screen && state.screen.type === "ended") { return; }
   } catch (e) {
     failures++;
     // Don't clobber the intentional "game ended" screen with a scary banner.
@@ -135,8 +152,12 @@ def do_GET(self):
             if self.path in ("/", "/index.html"):
                 self._send(200, "text/html; charset=utf-8", htmlPage().encode("utf-8"))
             elif self.path.startswith("/state"):
-                body = json.dumps(ui.get_state()).encode("utf-8")
-                self._send(200, "application/json", body)
+                state = ui.get_state()
+                self._send(200, "application/json", json.dumps(state).encode("utf-8"))
+                # Recorded only once the bytes are on the wire, so cleanup()
+                # cannot close the server out from under a response it is
+                # still writing (see record_state_delivered).
+                ui.record_state_delivered(state["version"])
             else:
                 self._send(404, "text/plain", b"Not found")
 
@@ -184,11 +205,17 @@ def __init__(
         host="127.0.0.1",
         port=8000,
         start_server=True,
+        endedScreenTimeoutSeconds=ENDED_SCREEN_DELIVERY_TIMEOUT_SECONDS,
     ):
         super().__init__(currentPrompt, timeService, player)
         self._lock = threading.Lock()
         self._screen = {"type": "loading"}
         self._version = 0
+        # The newest screen version the browser has been handed. Starts below
+        # the first version so "nothing has been collected yet" is a state
+        # cleanup() can tell apart from "the current screen has been seen".
+        self._deliveredVersion = -1
+        self._endedScreenTimeoutSeconds = endedScreenTimeoutSeconds
         self._inputQueue = queue.Queue()
         self._server = None
         if start_server:
@@ -211,6 +238,29 @@ def submit_input(self, value):
         """Deliver the player's browser response to the waiting game thread."""
         self._inputQueue.put(value)
 
+    def record_state_delivered(self, version):
+        """Note that the browser has been sent the screen at this version.
+
+        Only cleanup() reads this, to know the ended screen reached the page
+        before the server is closed. Kept as the highest version seen so an
+        overlapping poll that finishes late cannot walk it backwards."""
+        with self._lock:
+            self._deliveredVersion = max(self._deliveredVersion, version)
+
+    def _awaitScreenDelivery(self, timeout):
+        """Block until the current screen has been sent to the browser.
+
+        Returns True if it went out, False if the timeout ran out first -
+        which is the ordinary outcome when no page is polling."""
+        deadline = time.monotonic() + timeout
+        while True:
+            with self._lock:
+                if self._deliveredVersion >= self._version:
+                    return True
+            if time.monotonic() >= deadline:
+                return False
+            time.sleep(ENDED_SCREEN_DELIVERY_POLL_SECONDS)
+
     # --- transport seams ---------------------------------------------------
     # Every screen below is built once and shared by both web front-ends; only
     # how a screen reaches the browser (_present) and how the response comes
@@ -303,6 +353,10 @@ def timedKeyPress(self, message):
     def cleanup(self):
         self._present({"type": "ended"})
         if self._server is not None:
+            # Hold the server open until the page has the ended screen;
+            # otherwise the run's last screen is never fetched and the browser
+            # reports a lost connection instead.
+            self._awaitScreenDelivery(self._endedScreenTimeoutSeconds)
             self._server.shutdown()
             self._server.server_close()
             self._server = None

tests/test_fishE.py

@@ -900,13 +900,14 @@ def test_selectSaveFile_delete_then_quit():
     # Delete submenu: Delete Slot 1 / Cancel -> "2" (Cancel)
     # Menu again: same options -> "4" (Quit)
     game.userInterface.showOptions.side_effect = ["3", "2", "4"]
+    game.running = True
 
-    # call/check - Quit calls exit(0), which raises SystemExit
-    try:
-        game._selectSaveFile()
-        assert False, "expected SystemExit"
-    except SystemExit as e:
-        assert e.code == 0
+    # call - Quit returns rather than raising SystemExit; ending the process
+    # here skipped the front-end's cleanup() (see FishE.play)
+    game._selectSaveFile()
+
+    # check - the run is over and no slot was claimed
+    assert game.running is False
     game.saveFileManager.select_save_slot.assert_not_called()
 
 
@@ -1160,3 +1161,72 @@ def test_play_appends_the_fleet_report_and_the_eviction_together():
     # check
     assert "The Marauder landed 12 fish." in game.prompt.text
     assert housing.EVICTION_MESSAGE in game.prompt.text
+
+
+def test_play_cleans_up_the_front_end_when_the_run_ends():
+    # Retiring or quitting used to return out of play() with nothing released:
+    # the pygame window closed without pygame.quit(), and neither browser
+    # front-end ever published its ended screen.
+    game = createGameForPlay()
+    game.locations[LocationType.HOME].run.return_value = LocationType.NONE
+
+    # call
+    game.play()
+
+    # check
+    game.userInterface.cleanup.assert_called_once_with()
+
+
+def test_play_cleans_up_the_front_end_when_the_loop_raises():
+    # An error mid-run is exactly when a leftover window or a still-bound
+    # server is hardest to explain, so cleanup happens on the way out either way.
+    game = createGameForPlay()
+    game.locations[LocationType.HOME].run.side_effect = RuntimeError("kraken")
+
+    # call/check - the error still reaches the caller
+    try:
+        game.play()
+        assert False, "expected RuntimeError"
+    except RuntimeError as error:
+        assert str(error) == "kraken"
+    game.userInterface.cleanup.assert_called_once_with()
+
+
+def test_quit_from_the_save_file_menu_ends_the_run_without_exiting():
+    # "Quit" called exit(0) from inside __init__, ending the process before
+    # any front-end could clean up - which is why the Pyodide entry point had
+    # to catch SystemExit and publish an ended screen of its own. It now
+    # clears running, __init__ stops early, and play() does nothing but hand
+    # the front-end the same cleanup() every other ending gets.
+    with tempfile.TemporaryDirectory() as data_directory:
+        fishE.Player = Player
+        fishE.Stats = Stats
+        fishE.TimeService = TimeService
+        fishE.Prompt = Prompt
+        fishE.PlayerJsonReaderWriter = PlayerJsonReaderWriter
+        fishE.StatsJsonReaderWriter = StatsJsonReaderWriter
+        fishE.TimeServiceJsonReaderWriter = TimeServiceJsonReaderWriter
+        fishE.SaveFileManager = SaveFileManager
+
+        config = Config()
+        config.dataDirectory = data_directory
+
+        userInterface = MagicMock()
+        # An empty save directory offers "Create New Save (Slot 1)" then "Quit".
+        userInterface.showOptions.return_value = "2"
+        factory = MagicMock()
+        factory.create_user_interface.return_value = userInterface
+
+        # call
+        with patch.object(fishE, "Config", return_value=config), patch.object(
+            fishE, "UserInterfaceFactory", factory
+        ):
+            game = fishE.FishE()
+            game.play()
+
+        # check - no slot was claimed, nothing was written, and the front-end
+        # was released
+        assert game.running is False
+        assert game.saveFileManager.selected_save_slot is None
+        assert os.listdir(data_directory) == []
+        userInterface.cleanup.assert_called_once_with()

tests/ui/test_webUserInterface.py

@@ -20,13 +20,21 @@
 from world.timeService import TimeService
 
 
-def makeWebUI(start_server=False, port=0):
+def makeWebUI(start_server=False, port=0, endedScreenTimeoutSeconds=0.1):
     prompt = Prompt("What would you like to do?")
     player = Player()
     stats = Stats()
     timeService = TimeService(player, stats)
+    # No browser polls these servers, so cleanup()'s wait for the ended screen
+    # to be collected always runs to its timeout; the production default (two
+    # seconds) would be paid by every test that starts one.
     return WebUserInterface(
-        prompt, timeService, player, port=port, start_server=start_server
+        prompt,
+        timeService,
+        player,
+        port=port,
+        start_server=start_server,
+        endedScreenTimeoutSeconds=endedScreenTimeoutSeconds,
     )
 
 
@@ -308,3 +316,79 @@ def test_showOptions_refuses_an_unavailable_choice():
     ui.submit_input("2")  # available
     thread.join(timeout=2)
     assert box["result"] == "2"
+
+
+def test_cleanup_publishes_the_ended_screen():
+    # check - the run's last screen says the game is over, whether or not a
+    # server is involved (the Pyodide front-end inherits this path)
+    ui = makeWebUI()
+    ui.cleanup()
+
+    assert ui.get_state()["screen"] == {"type": "ended"}
+
+
+def test_cleanup_holds_the_server_open_until_the_ended_screen_is_fetched():
+    # The browser only learns the game finished on its next poll, so closing
+    # the socket the instant the ended screen is published loses it: the page
+    # would show "Lost connection" to a player who had just retired.
+    ui = makeWebUI(start_server=True, port=0, endedScreenTimeoutSeconds=2.0)
+    host, port = ui.address
+    base = "http://127.0.0.1:%d" % port
+
+    thread, box = runInThread(ui.cleanup)
+    try:
+        waitForScreen(ui, "ended")
+        # Still serving: the ended screen has not been collected yet.
+        assert thread.is_alive()
+        state = json.loads(urllib.request.urlopen(base + "/state", timeout=2).read())
+        assert state["screen"] == {"type": "ended"}
+    finally:
+        thread.join(timeout=3)
+
+    # check - once the page has it, the server is closed rather than left running
+    assert not thread.is_alive()
+    assert ui.address is None
+
+
+def test_cleanup_stops_waiting_when_nothing_is_listening():
+    # A closed tab (or a game driven by anything other than a browser) never
+    # fetches the ended screen, so the wait has to end on its own.
+    ui = makeWebUI(start_server=True, port=0, endedScreenTimeoutSeconds=0.2)
+
+    startTime = time.time()
+    ui.cleanup()
+    elapsed = time.time() - startTime
+
+    assert 0.2 <= elapsed < 2.0
+    assert ui.address is None
+
+
+def test_record_state_delivered_keeps_the_highest_version_seen():
+    # Two polls can overlap, and the older one can finish last; the newer
+    # screen must not be reported as uncollected because of it.
+    ui = makeWebUI()
+    ui._present({"type": "dialogue", "text": "Caught a fish!"})
+    version = ui.get_state()["version"]
+
+    ui.record_state_delivered(version)
+    ui.record_state_delivered(version - 1)
+
+    assert ui._awaitScreenDelivery(timeout=0) is True
+
+
+def test_waiting_for_delivery_reports_an_uncollected_screen():
+    # check - a screen published after the last delivery is not treated as seen
+    ui = makeWebUI()
+    ui.record_state_delivered(ui.get_state()["version"])
+    ui._present({"type": "dialogue", "text": "Caught a fish!"})
+
+    assert ui._awaitScreenDelivery(timeout=0) is False
+
+
+def test_client_stops_polling_once_the_game_has_ended():
+    # check - the server is gone by the time the page renders the ended
+    # screen, so the poll loop stops rather than failing every 300ms for the
+    # rest of the tab's life
+    page = webUserInterface.htmlPage()
+
+    assert 'state.screen.type === "ended"' in page