-
Notifications
You must be signed in to change notification settings - Fork 58
Add sort and limit support to listing lobbies #282
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Allows sorting the list() result on any property and adds support to limit how many lobbies should be returned.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds sorting and limiting capabilities to the lobby listing functionality. The changes enable clients to specify sort criteria and limit the number of results returned when requesting lobbies.
- Extends the
list()API to accept sort and limit parameters alongside existing filter functionality - Updates backend Go code to handle sort and limit parameters in database queries
- Refactors test files to use the new unified lobby request interface
Reviewed Changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| lib/types.ts | Adds sort and limit fields to ListPacket interface |
| lib/network.ts | Updates list method to accept sort and limit parameters |
| internal/signaling/types.go | Adds Sort and Limit fields to ListPacket struct |
| internal/signaling/stores/shared.go | Updates ListLobbies interface to include sort and limit parameters |
| internal/signaling/stores/postgres.go | Implements sort and limit logic in PostgreSQL query with default ordering |
| internal/signaling/peer.go | Updates HandleListPacket to pass new parameters and refactors error handling |
| go.mod | Updates mongodb-filter-to-postgres dependency to support sorting |
| features/support/steps/network.ts | Consolidates lobby request steps into unified interface |
| features/*.feature | Updates test scenarios to use new lobby request format |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
@codex review this please. |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. |
| } | ||
|
|
||
| async list (filter?: object): Promise<LobbyListEntry[]> { | ||
| async list (filter?: object, sort?: object, limit?: number): Promise<LobbyListEntry[]> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't it be nice to have better typings here? Enums or something
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's not possible, you can filter and sort on any of the fields in the random JSON object that you can store with a lobby.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
e.g. sort
type SortObject = Record<string, -1 | 1>;
const sort: SortObject = { playerCount: -1, leaderboard: 1 };
Allows sorting the list() result on any property and adds support to limit how many lobbies should be returned.