Fix the room leak, redesign the UI, and add a computer opponent - #6
Merged
Merged
Conversation
Server.deleteGame existed but was never called from anywhere, so every "New Game" click leaked a room for the lifetime of the process. Games live only in memory, so nothing else cleaned them up either. Left alone this is not just a slow memory leak. Game ids are drawn from rnd.Next(1, 1000000) inside a do/while that retries on collision, so as the dictionary fills, collisions rise and the loop degenerates; once the id space is exhausted it spins forever and hangs the request thread. Two further latent crashes turned up next to it: startGame and removePlayer both locked on the game without the null check every other endpoint performs, so any stale game id threw a NullReferenceException instead of returning "gameid not found". Changes: - Track last-activity time on Game, refreshed by every endpoint that touches it, so a table someone is still polling is never considered idle - Delete the game as soon as its last player leaves - Sweep games idle for more than 30 minutes from a hosted background service running every 5 minutes - Bound id allocation and cap concurrent games, returning a clear "too many games in progress" response rather than looping - Add the missing null checks to startGame and removePlayer, and reject an unknown username instead of removing null Nine tests cover the reclamation paths, including the two calls that used to throw. Full suite: 40 tests, all passing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The stylesheet was 67 lines built around a global "div { width: 50% }" rule,
and the page had no viewport meta tag, so the game was unusable on a phone.
The leaderboard was absolutely positioned and overlapped the board on narrow
screens.
Presentation:
- Rebuild the stylesheet on CSS custom properties, keeping the cream and
orange identity but adding depth, spacing and a consistent type scale
- Size the board and type with clamp() so it adapts from phone to desktop
- Add the viewport meta tag, without which none of the above applies on mobile
- Give buttons hover, active, focus-visible and disabled states
- Keep the leaderboard out of the flow only on wide screens, so it can no
longer cover the board
- Add a meta description
Gameplay feedback, all additive:
- Show the word as it is traced. Previously the only feedback was highlighted
tiles, so players could not read back what they were about to submit
- Disable Guess and Reset while nothing is selected, initialised when the
board is drawn rather than on first click
- Select tiles via a CSS class instead of an inline background colour, so the
selected state can carry more than one property
Rules, scoring, the API and the request flow are untouched. Verified through
the UI: traced GEM on a real board, submitted with the Guess button, validated
against the dictionary at round end, scored 1 point. A non-word was correctly
rejected. No console errors. 40 tests passing.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b6cab4178d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+223
to
+225
| if (abandoned) | ||
| { | ||
| srv.deleteGame(gameId); |
There was a problem hiding this comment.
When the last existing player leaves while another request is joining the same game, abandoned is computed under lock (g) but deletion happens after that lock is released. The joining request can obtain the game, add its player between lines 219 and 225, and then have the now-nonempty game removed by this unconditional call; its login succeeds, but its next poll returns gameid not found. Make the empty check and removal atomic with respect to both game membership updates and the server dictionary.
Useful? React with 👍 / 👎.
Comment on lines
+123
to
+125
| List<int> stale = games | ||
| .Where(kv => kv.Value.getLastActivityUtc() < cutoff) | ||
| .Select(kv => kv.Key) |
There was a problem hiding this comment.
When a client resumes an idle game at the same time as a sweep, an endpoint can retrieve the Game from getGame and then be waiting to lock/touch it while this predicate still observes the old timestamp and removes it. That request subsequently succeeds against the detached object, but every following request reports that the game is missing. The stale check/removal must be coordinated with acquiring and touching the game so a request already using the room prevents its deletion.
Useful? React with 👍 / 👎.
There was no way to play alone: a solo game was just a room with one person in
it and nothing to play against.
The opponent is built on a board solver. A depth-first walk from every tile
follows the same adjacency rules players get and never reuses a die, pruning a
branch as soon as the letters collected cannot begin any dictionary word. That
prefix check is what makes it cheap; without it the walk explores every path on
the grid regardless of whether it could spell anything. Prefix lookup reuses
the existing sorted word list via binary search, so no extra structure is held
in memory.
Die faces can be two characters ("Qu"), so the solver builds words from faces
rather than single characters, matching how a guess is assembled.
The opponent itself is a difficulty policy over the solution set. Easy plays
about five short words, Medium about twelve, Hard takes the best two dozen.
Easy and Medium are capped by word length so the computer stays beatable, and
picks are drawn randomly through the eligible set so the same board does not
always produce the same game.
Moves are planned up front and released on a clock rather than by a timer
thread: the game hands over whatever is due whenever a client checks in. That
keeps it deterministic and testable, costs nothing for an idle game, and needs
no extra concurrency around the existing lock.
It also composes with what was already there. The computer is an ordinary
player, so duplicate cancellation applies between it and the human, and the
existing masking hides its words until the round ends.
Fourteen tests cover prefix lookup, solver correctness, die reuse, difficulty
scaling, plan ordering and release timing. Suite: 54 tests, all passing.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The client is plain files with unversioned names, so a browser that had already loaded them kept running the old JavaScript and CSS after a deploy. This was not theoretical: during testing the browser held a cached apis.js and could not see a newly added function even though the server was serving it correctly. A returning player would have hit exactly that. Markup, script and styles are now sent with no-cache so the browser revalidates; static files already carry ETags, so the check is a cheap 304 rather than a re-download. Images keep a long cache since their contents do not change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Ssavan99
changed the title
Nothing told a crawler what this page is, and crawling it had a side effect: a GET to /Server/newGame creates a real game room, so an indexer walking the API would have generated rooms on every pass. - robots.txt allowing the site but disallowing /Server/ - sitemap.xml, referenced from robots.txt - A descriptive title and description, a canonical link, and Open Graph and Twitter card tags so a shared link previews properly - A marked spot for the Search Console verification tag Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Fills the placeholder left in the head so the property can be verified via the HTML tag method. This token is a public site identifier, not a secret. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
1 participant
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.
Game rooms were never reclaimed
UI redesign
Single-player against the computer
Stale client code after deploys