Release the front-end wherever the game ends, and let the browser see the ended screen - #175
Merged
Merged
Conversation
cleanup() had exactly one production caller, on the Pyodide path, so a run that finished under any other front-end released nothing: the pygame window closed without pygame.quit(), and the server-backed web front-end exited without publishing its ended screen - the tab kept polling a dead process and told a player who had just retired that the connection was lost. play() now calls it once, in a finally, so every ending (retiring, quitting, an unhandled error) goes through the same path. "Quit" in the save-file menu clears running instead of calling exit(0), which used to kill the interpreter from inside __init__ before any front-end could be released; the Pyodide entry point's SystemExit handling and its own cleanup() call go with it. The ended screen also has to survive being published: the browser only discovers it on its next poll, and cleanup() closed the socket immediately after. The server is now held open until the page has actually collected that screen, or a short timeout expires when nothing is listening, and the client stops polling once it renders it. Closes #153 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every other internal helper on WebUserInterface (_present, _awaitInput, _header) carries the underscore; only get_state/submit_input/ record_state_delivered are public, because the request handler calls them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Member
Author
Self-reviewThe full diff was read against this repo's conventions (front-end parity, the schema/reader-writer contract, test coverage of new public methods, doc drift). One finding was acted on before this comment was posted; the rest are notes on deliberate choices, and nothing is left outstanding. Acted on
Reviewed and judged correct
Parity, schemas and docs
This review was performed during a Gardener session (https://github.com/Stephenson-Software/gardener). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
cleanup()is now called once, from afinallyinFishE.play(), rather than from the Pyodide entry point alone. Every way a run can finish — retiring, quitting, an unhandled error — is covered, for all four front-ends. It stays a no-op for the console.exit(0)in the save-file menu's "Quit" branch has been replaced by clearingrunning. Ending the interpreter from inside__init__meant no front-end was ever released;__init__now returns early andplay()does nothing but clean up.web/pyodide_main.py'sSystemExithandling and its owncleanup()call have been dropped as redundant.Test plan
python3 -m compileall -q src tests web examplespython3 -m pytest --cov=src --cov-report=term-missing --cov-report=xml:cov.xml— 815 passed,src/ui/webUserInterface.pyat 97%FishE.play), so console, pygame,UIType.WEBandUIType.PYODIDEall take the same path. Console and pygamecleanup()were read and are unaffected beyond now being called (pygame'spygame.quit()is idempotent, and its own QUIT handlers still call it). The delivery wait is skipped when no server is running, which is how the Pyodide subclass inherits it.play()cleans up on a normal end and on an exception; "Quit" from the save-file menu ends the run withoutSystemExitand claims no slot; the ended screen is published with or without a server; the server is held open until that screen is fetched and gives up when nothing fetches it; delivery bookkeeping ignores a late overlapping poll; the client's poll loop stops onended.README.md,PLANNING.mdand the threeschemas/*.jsonfiles make no claim touched by this change (noPlayer/Stats/TimeServicefield was added, renamed or retyped).Closes #153
This PR description was drafted during a Gardener session (https://github.com/Stephenson-Software/gardener).