Fix spectator mode not showing when a spectated user quits correctly #25548
Conversation
This file contains 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
Spectator relies on the `SpectatorScreen` (which importantly, is not the `SpectatorPlayer` but a separate screen) to be alive and present in order to function correctly, due to `Schedule`d operations. This was true for multiplayer but not for solo play, so it seems logical to standardise things here. Closes ppy#25535.
1 task
bdach
reviewed
Nov 23, 2023
| @@ -33,7 +33,7 @@ public abstract partial class SpectatorClient : Component, ISpectatorClient | |||
| /// Whether the <see cref="SpectatorClient"/> is currently connected. | |||
| /// This is NOT thread safe and usage should be scheduled. | |||
| /// </summary> | |||
| public abstract IBindable<bool> IsConnected { get; } | |||
| protected abstract IBindable<bool> IsConnected { get; } | |||
There was a problem hiding this comment.
Any particular reason for encapsulating this? I rely on this being public in #25522.
There was a problem hiding this comment.
No reason, just wasn't being used.
There was a problem hiding this comment.
I guess I can just re-expose it again in mine when resolving merge conflicts.
|
Transition on retry is a tad broken: 2023-11-23.17-42-46.mp4osu! logo should not be hanging around like that there. |
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.
This PR switches
SoloSpectatorScreento use a nested screen for gameplay.Spectator relies on the
SpectatorScreen(which importantly, is not theSpectatorPlayerbut a separate screen) to be alive and present in order to function correctly, due toScheduled operations).This was already true for multiplayer but not for solo play, so it seems logical to standardise things here.
Closes #25535.
CleanShot.2023-11-23.at.03.38.06.mp4