Skip to content

Implement lobby leave functionality #241

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

erikdubbelboer
Copy link
Member

Summary

  • support leave() in Network API
  • emit leave events from signaling
  • add leave feature tests
  • implement leaveLobby packet on server
  • document lobby leaving in API docs
  • add leave lobby button to example

Testing

  • go test ./...
  • yarn cucumber (fails: cucumber-js not found)

https://chatgpt.com/codex/tasks/task_e_6867d29d6d448320bb7eea6347085ce3

@erikdubbelboer erikdubbelboer marked this pull request as ready for review July 6, 2025 04:42
Copy link
Collaborator

@koenbollen koenbollen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks good. I have some thoughts on the new event/packet names (commented inline).

And it could also use a feature tests where a client leaves a lobby and joins a new one (that's the actually functionality we want to support).

@erikdubbelboer erikdubbelboer force-pushed the codex/determine-priority-for-lobby-functionality branch from a571dd6 to 3df6a8a Compare July 8, 2025 02:52
@erikdubbelboer erikdubbelboer force-pushed the codex/determine-priority-for-lobby-functionality branch from c34ffa0 to 2374d32 Compare July 8, 2025 03:39
@@ -94,6 +94,16 @@ type LobbyUpdatedPacket struct {
LobbyInfo stores.Lobby `json:"lobbyInfo"`
}

type LeaveLobbyPacket struct {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick: The "Lobby" part could go here

Type string `json:"type"`
}

type LeftLobbyPacket struct {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick: the "Lobby" part could go here

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

Successfully merging this pull request may close these issues.

2 participants