Skip to content
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

Fix spectator mode not showing when a spectated user quits correctly #25548

Closed
wants to merge 11 commits into from
Prev Previous commit
Next Next commit
Reduce access to various fields and events in SpectatorClient
  • Loading branch information
peppy committed Nov 22, 2023
commit a972ef9f617ed2cdc32591f9877b50cf2d3bbe8b
2 changes: 1 addition & 1 deletion osu.Game/Online/Spectator/OnlineSpectatorClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ public partial class OnlineSpectatorClient : SpectatorClient

private IHubClientConnector? connector;

public override IBindable<bool> IsConnected { get; } = new BindableBool();
protected override IBindable<bool> IsConnected { get; } = new BindableBool();

private HubConnection? connection => connector?.CurrentConnection;

Expand Down
30 changes: 15 additions & 15 deletions osu.Game/Online/Spectator/SpectatorClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -33,42 +33,42 @@ 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; }
Copy link
Collaborator

Choose a reason for hiding this comment

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

Any particular reason for encapsulating this? I rely on this being public in #25522.

Copy link
Member Author

Choose a reason for hiding this comment

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

No reason, just wasn't being used.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I guess I can just re-expose it again in mine when resolving merge conflicts.


/// <summary>
/// The states of all users currently being watched.
/// </summary>
public virtual IBindableDictionary<int, SpectatorState> WatchedUserStates => watchedUserStates;
public IBindableDictionary<int, SpectatorState> WatchedUserStates => watchedUserStates;

/// <summary>
/// A global list of all players currently playing.
/// </summary>
public IBindableList<int> PlayingUsers => playingUsers;

/// <summary>
/// Whether the local user is playing.
/// Whether the spectated user is playing.
/// </summary>
protected internal bool IsPlaying { get; private set; }
private bool isPlaying { get; set; }

/// <summary>
/// Called whenever new frames arrive from the server.
/// </summary>
public virtual event Action<int, FrameDataBundle>? OnNewFrames;
public event Action<int, FrameDataBundle>? OnNewFrames;

/// <summary>
/// Called whenever a user starts a play session, or immediately if the user is being watched and currently in a play session.
/// </summary>
public virtual event Action<int, SpectatorState>? OnUserBeganPlaying;
public event Action<int, SpectatorState>? OnUserBeganPlaying;

/// <summary>
/// Called whenever a user finishes a play session.
/// </summary>
public virtual event Action<int, SpectatorState>? OnUserFinishedPlaying;
public event Action<int, SpectatorState>? OnUserFinishedPlaying;

/// <summary>
/// Called whenever a user-submitted score has been fully processed.
/// </summary>
public virtual event Action<int, long>? OnUserScoreProcessed;
public event Action<int, long>? OnUserScoreProcessed;

/// <summary>
/// A dictionary containing all users currently being watched, with the number of watching components for each user.
Expand Down Expand Up @@ -114,7 +114,7 @@ private void load()
}

// re-send state in case it wasn't received
if (IsPlaying)
if (isPlaying)
// TODO: this is likely sent out of order after a reconnect scenario. needs further consideration.
BeginPlayingInternal(currentScoreToken, currentState);
}
Expand Down Expand Up @@ -179,10 +179,10 @@ public void BeginPlaying(long? scoreToken, GameplayState state, Score score)
// This schedule is only here to match the one below in `EndPlaying`.
Schedule(() =>
{
if (IsPlaying)
if (isPlaying)
throw new InvalidOperationException($"Cannot invoke {nameof(BeginPlaying)} when already playing");

IsPlaying = true;
isPlaying = true;

// transfer state at point of beginning play
currentState.BeatmapID = score.ScoreInfo.BeatmapInfo!.OnlineID;
Expand All @@ -202,7 +202,7 @@ public void BeginPlaying(long? scoreToken, GameplayState state, Score score)

public void HandleFrame(ReplayFrame frame) => Schedule(() =>
{
if (!IsPlaying)
if (!isPlaying)
{
Logger.Log($"Frames arrived at {nameof(SpectatorClient)} outside of gameplay scope and will be ignored.");
return;
Expand All @@ -224,7 +224,7 @@ public void EndPlaying(GameplayState state)
// We probably need to find a better way to handle this...
Schedule(() =>
{
if (!IsPlaying)
if (!isPlaying)
return;

// Disposal can take some time, leading to EndPlaying potentially being called after a future play session.
Expand All @@ -235,7 +235,7 @@ public void EndPlaying(GameplayState state)
if (pendingFrames.Count > 0)
purgePendingFrames();

IsPlaying = false;
isPlaying = false;
currentBeatmap = null;

if (state.HasPassed)
Expand All @@ -249,7 +249,7 @@ public void EndPlaying(GameplayState state)
});
}

public virtual void WatchUser(int userId)
public void WatchUser(int userId)
{
Debug.Assert(ThreadSafety.IsUpdateThread);

Expand Down
2 changes: 1 addition & 1 deletion osu.Game/Tests/Visual/Spectator/TestSpectatorClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public partial class TestSpectatorClient : SpectatorClient

public int FrameSendAttempts { get; private set; }

public override IBindable<bool> IsConnected { get; } = new Bindable<bool>(true);
protected override IBindable<bool> IsConnected { get; } = new Bindable<bool>(true);

public IReadOnlyDictionary<int, ReplayFrame> LastReceivedUserFrames => lastReceivedUserFrames;

Expand Down