Delay closing tournament rooms for 30min after last user left#466
Merged
peppy merged 5 commits intoppy:masterfrom Apr 16, 2026
Merged
Delay closing tournament rooms for 30min after last user left#466peppy merged 5 commits intoppy:masterfrom
peppy merged 5 commits intoppy:masterfrom
Conversation
The way this works is: 1. When the last user leaves a tournament mode room, the room controller will set an end date for the room in the database to 30 minutes in the future. This is persisted to the database, which makes the rooms disappear on their own in the client-side room listing because that's handled by osu-web which will respect the new end date. 2. If any user joins the room before its end date elapses, the end date is cleared to `null` again. 3. If point (2) never occurs before the end date elapses, `ServerMultiplayerRoom.CanJoinRoom()` now checks the end date, which ensures that after that date passes, no user can join the room anymore. 4. A new job, `MultiplayerRoomLifetimeBackgroundService()`, is called into existence. Every minute, it looks for tournament rooms with end dates in the past. If it finds any, it destroys their server-side memory state, and also dissociates any referees from said rooms to complete the cleanup. Of particular note, this took some non-minor reshuffling of how referee user state is managed because of the expectations here. - Referees join refereeing rights: - On creation and subsequent joining of a tournament room. - On being added as a referee by another tournament room's host. - Referees lose refereeing rights: - On being removed as a referee by the tournament room's host. - On the room being force-closed by themselves or another referee. - On the room being cleaned up by the background job. The above lists are exhaustive, e.g. those are the *only* condition that cause granting / losing referee rights. No other interaction causes either. Obviously, the referee privileges are still server-side memory state and will get lost on a redeploy / restart. Additionally note that the referee hub does not provide a host change operation and I'm kind of fine with that until someone really needs it for whatever reason - adding it *is* possible, but would require further safeties in terms of per-user open room quotas and similar that I'd rather not get into if I don't need to.
Now that it doesn't shed referee permissions, it's kind of no real use anymore. It was being very awkward in terms of behaviours given the revised concept of referee permission management so I just removed it.
This is partly to match bancho, partly to reduce undesirable scenarios that could cause weird breakage. For instance, imagine this sequence of steps: 1. User A creates room 2. User A adds user B as referee 3. User B removes user A as referee from room Now A is in a sticky state as the room is chalked up to them, goes on their open tournament room quota, but they're locked out of it (they can't join the room, as removal of a referee revokes rights). So just disallow this to begin with. With this change only A can add or remove referees.
Provided for convenience / information as to which rooms can be recovered on a reconnection.
Member
Sounds good to me (haven't read the code). |
|
Looks good to me as well |
smoogipoo
approved these changes
Apr 16, 2026
Contributor
smoogipoo
left a comment
There was a problem hiding this comment.
LGTM, looks like this turned out well in the end.
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.
Closes #460
Add support for delayed closing of tournament mode multiplayer rooms
The way this works is:
nullagain.ServerMultiplayerRoom.CanJoinRoom()now checks the end date, which ensures that after that date passes, no user can join the room anymore.MultiplayerRoomLifetimeBackgroundService, is called into existence. Every minute, it looks for tournament rooms with end dates in the past. If it finds any, it destroys their server-side memory state, and also dissociates any referees from said rooms to complete the cleanup.Of particular note, this took some non-minor reshuffling of how referee user state is managed because of the expectations here.
The above lists are exhaustive, e.g. those are the only conditions that cause granting / losing referee rights. No other interaction causes either.
Obviously, the referee privileges are still server-side memory state and will get lost on a redeploy / restart.
Additionally note that the referee hub does not provide a host change operation and I'm kind of fine with that until someone really needs it for whatever reason - adding it is possible, but would require further safeties in terms of per-user open room quotas and similar that I'd rather not get into if I don't need to.
Remove
LeaveRoom()referee hub operationNow that it doesn't shed referee permissions, it's kind of no real use anymore. It was being very awkward in terms of behaviours given the revised concept of referee permission management so I just removed it.
Require being room host for adding / removing referees
This is partly to match bancho, partly to reduce undesirable scenarios that could cause weird breakage. For instance, imagine this sequence of steps:
Now A is in a sticky state as the room is chalked up to them, goes on their open tournament room quota, but they're locked out of it (they can't join the room, as removal of a referee revokes rights). So just disallow this to begin with. With this change only A can add or remove referees.
Add primitive for listing all refereed rooms
Provided for convenience / information as to which rooms can be recovered on a reconnection.
@ILW8 @ThePooN would appreciate if you read through the above description of the changes and see if there's anything you'd find objectionable.