Skip to content

chore: multiplayer game examples#4203

Closed
NathanFlurry wants to merge 1 commit into02-13-chore_repo_ignore_openspecfrom
02-16-chore_multiplayer_game_examples
Closed

chore: multiplayer game examples#4203
NathanFlurry wants to merge 1 commit into02-13-chore_repo_ignore_openspecfrom
02-16-chore_multiplayer_game_examples

Conversation

@NathanFlurry
Copy link
Member

Description

Please include a summary of the changes and the related issue. Please also include relevant motivation and context.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

Copy link
Member Author

NathanFlurry commented Feb 16, 2026

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more


How to use the Graphite Merge Queue

Add the label merge-queue to this PR to add it to the merge queue.

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

@NathanFlurry NathanFlurry mentioned this pull request Feb 16, 2026
11 tasks
@claude
Copy link

claude bot commented Feb 16, 2026

PR Review: Multiplayer Game Patterns

This PR has been significantly revised since the last review. The current version adds a single comprehensive `examples/multiplayer-game-patterns` example showcasing 7 matchmaking and session patterns, along with several core framework improvements to rivetkit-typescript.

Core Framework Changes (Positive)

`c.aborted` Alias

The new `get aborted()` getter on `ActorContext` is a clean ergonomic improvement. `while (!c.aborted)` reads significantly better than `while (!c.abortSignal.aborted)` in run loops.

wa-sqlite Serializer (`db/mod.ts`)

This is a genuine correctness fix. The new `serialize` promise-chain queues all database calls so concurrent action processing cannot cause re-entrant wa-sqlite calls. It correctly handles the case where an action throws (the rejection passes through to the caller while the queue continues).

Port Sync in `runtime/index.ts`

The fix that re-writes `config.publicEndpoint` and `config.serverless.publicEndpoint` when a random port is chosen is correct. The guard avoids clobbering explicitly-configured endpoints.

`RawAccess` Re-export

Importing from `"rivetkit/db"` rather than navigating internal paths is better DX.


Issues

1. SQL String Interpolation Instead of Parameterized Queries

All matchmakers use the `sqlString`/`sqlInt` helper pattern from `shared/sql.ts` instead of parameterized `?` placeholders. Now that `db.execute` routes to `db.query`/`db.run` when args are passed, the helpers are unnecessary for new code.

The `sqlString` implementation is technically correct for the values used here (UUIDs and simple IDs). However, the pattern is non-idiomatic and acts as an anti-example for readers who may copy it into contexts where values are truly user-controlled. Consider migrating the examples to parameterized queries.

2. Race Condition in Turn-Based `processAcceptInvite`

Two concurrent `acceptInvite` messages for the same open invite can both pass the `status !== "open"` check and create two separate matches from the same invite. The read and status update are not wrapped in a transaction.

The `processJoinOpenPool` path correctly uses `withImmediateTransaction`, but `processAcceptInvite` does not. The fix is to wrap the read + update in a `BEGIN IMMEDIATE` transaction following the same pattern as `tryCreatePoolMatch`.

3. Committed Build Artifact

`examples/multiplayer-game-patterns/public/assets/index-9bY3DXxE.js` is a minified build artifact committed to the repository. Build artifacts should not be committed to source control — they are regenerated from source and the hash-based filename will change on rebuild, producing diff churn. Consider adding `public/assets/` to `.gitignore` for this example.

4. Battle Royale: Disconnect Does Not Eliminate Player

When a player disconnects, their entry is deleted from `state.players`, but if they were still `alive: true`, the match may not reach the win condition. If all players disconnect simultaneously, `aliveCount` reaches 0, `aliveCount <= 1` fires with `winnerPlayerId = undefined`, and the match ends without a winner. Worth handling explicitly — e.g., trigger `maybeFinish` after removal or mark the player as eliminated on disconnect.

5. Dead Code

These functions are defined but never called:

  • `countQueueByMode` in `competitive/matchmaker.ts`
  • `countQueue` in `battle-royale/matchmaker.ts`
  • `selectRoomByMatchId` in `party/matchmaker.ts`

6. `debugSetRating` in Ranked Matchmaker

This queue message type allows arbitrary ELO overrides with no authentication check. It is present to support tests, which is fine, but it should carry a prominent comment warning that it must be removed or access-gated before production use. As written, any client that can send queue messages could manipulate ratings.

7. Query-Type Heuristic in `db/mod.ts`

The routing between `db.query` and `db.run` based on inspecting the first 16 characters of the query string will mis-route queries that begin with SQL comments (`-- select...` or `/* ... */`). Since the current matchmaker examples never pass args to `execute` (they all use `sqlString`/`sqlInt` interpolation), this code path is untested in practice. A more robust approach would be to accept an explicit `{ returning: boolean }` option, or to always use `db.query` and handle zero-row DML results uniformly.

8. Competitive Duo Team Assignment

For `duo` mode (2 players per team), the round-robin `idx % teams` assignment means the oldest two players in the queue land on opposite teams. If two friends queue simultaneously expecting to be on the same team, this will split them. This may be intentional, but it warrants a comment since it is counterintuitive for the duo queue use case.


Minor Notes

  • `buildPartyCode` uses `Math.random()` — acceptable for party codes (32^6 is approximately 1 billion possibilities) but stands in contrast to `buildSecret` which uses `crypto.randomUUID()`. Worth a comment noting the intentional distinction.
  • `issuePlayerToken` is callable by any connected client — the matchmaker is expected to be the sole caller, but this is enforced only by key secrecy, not actor-level access control. Fine for an example, but worth a comment stating the assumption.
  • `to_player_id` stored as `""` instead of `NULL` in the turn-based matchmaker — the `.length > 0` check works but is non-idiomatic for SQL; an explicit NULL check would be clearer.
  • Open-world chunk key fallback parsing — the `rawKey.split("/")` fallback in `createState` produces `NaN` intermediate values for standard 3-part array keys before correctly falling through to `Number(c.key[1])`. The fallback logic is more confusing than it needs to be.

Test Coverage

The test suite covers all 7 patterns end-to-end including async queue-based flows (ranked, turn-based), cross-chunk movement (open world), and ELO update verification. The `waitFor` polling helper is a clean approach for testing message-queue-driven actors. The 15-second timeout is appropriate given the async nature.


Summary

The core framework changes (wa-sqlite serializer, `c.aborted`, port sync) are solid. The example demonstrates a good range of real-world multiplayer patterns with consistent structure across modes.

Before merge:

  1. Fix the race condition in `processAcceptInvite` (unguarded concurrent accept on open invites)
  2. Remove the committed build artifact (`public/assets/index-9bY3DXxE.js`)
  3. Clean up dead code (`countQueueByMode`, `countQueue`, `selectRoomByMatchId`)

Follow-up:
4. Migrate matchmakers to parameterized queries (now that `db.execute` supports them natively)
5. Add a production-warning comment to `debugSetRating`
6. Improve or document the `db/mod.ts` query-type heuristic

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

Comments