Fix instabilities in channel join logic

This commit is contained in:
Dean Herbert 2018-11-21 17:15:10 +09:00
parent 4cde66240d
commit 88f82eb722

View File

@ -1,4 +1,4 @@
// Copyright (c) 2007-2018 ppy Pty Ltd <contact@ppy.sh>. // Copyright (c) 2007-2018 ppy Pty Ltd <contact@ppy.sh>.
// Licensed under the MIT Licence - https://raw.githubusercontent.com/ppy/osu/master/LICENCE // Licensed under the MIT Licence - https://raw.githubusercontent.com/ppy/osu/master/LICENCE
using System; using System;
@ -223,13 +223,11 @@ namespace osu.Game.Online.Chat
{ {
foreach (var channel in channels) foreach (var channel in channels)
{ {
// add as available if not already var ch = getChannel(channel, addToAvailable: true);
if (AvailableChannels.All(c => c.Id != channel.Id))
AvailableChannels.Add(channel);
// join any channels classified as "defaults" // join any channels classified as "defaults"
if (joinDefaults && defaultChannels.Any(c => c.Equals(channel.Name, StringComparison.OrdinalIgnoreCase))) if (joinDefaults && defaultChannels.Any(c => c.Equals(channel.Name, StringComparison.OrdinalIgnoreCase)))
JoinChannel(channel); JoinChannel(ch);
} }
}; };
req.Failure += error => req.Failure += error =>
@ -262,38 +260,68 @@ namespace osu.Game.Online.Chat
api.Queue(fetchInitialMsgReq); api.Queue(fetchInitialMsgReq);
} }
public void JoinChannel(Channel channel) /// <summary>
/// Find an existing channel instance for the provided channel. Lookup is performed basd on ID.
/// The provided channel may be used if an existing instance is not found.
/// </summary>
/// <param name="lookup">A candidate channel to be used for lookup or permanently on lookup failure.</param>
/// <param name="addToAvailable">Whether the channel should be added to <see cref="AvailableChannels"/> if not already.</param>
/// <param name="addToJoined">Whether the channel should be added to <see cref="JoinedChannels"/> if not already.</param>
/// <returns>The found channel.</returns>
private Channel getChannel(Channel lookup, bool addToAvailable = false, bool addToJoined = false)
{ {
if (channel == null) return; Channel found = null;
// ReSharper disable once AccessToModifiedClosure var available = AvailableChannels.FirstOrDefault(c => c.Id == lookup.Id);
var existing = JoinedChannels.FirstOrDefault(c => c.Id == channel.Id); if (available != null)
found = available;
if (existing != null) var joined = JoinedChannels.FirstOrDefault(c => c.Id == lookup.Id);
if (found == null && joined != null)
found = joined;
if (found == null)
{ {
// if we already have this channel loaded, we don't want to make a second one. found = lookup;
channel = existing;
} // if we're using a channel object from the server, we want to remove ourselves from the users list.
else // this is because we check the first user in the channel to display a name/icon on tabs for now.
{ var foundSelf = found.Users.FirstOrDefault(u => u.Id == api.LocalUser.Value.Id);
var foundSelf = channel.Users.FirstOrDefault(u => u.Id == api.LocalUser.Value.Id);
if (foundSelf != null) if (foundSelf != null)
channel.Users.Remove(foundSelf); found.Users.Remove(foundSelf);
}
JoinedChannels.Add(channel); if (joined == null && addToJoined) JoinedChannels.Add(found);
if (available == null && addToAvailable) AvailableChannels.Add(found);
if (channel.Type == ChannelType.Public && !channel.Joined) return found;
}
/// <summary>
/// Joins a channel if it has not already been joined.
/// </summary>
/// <param name="channel">The channel to join.</param>
/// <param name="alreadyJoined">Whether the channel has already been joined server-side. Will skip a join request.</param>
/// <returns>The joined channel. Note that this may not match the parameter channel as it is a backed object.</returns>
public Channel JoinChannel(Channel channel, bool alreadyJoined = false)
{
if (channel == null) return null;
channel = getChannel(channel, addToJoined: true);
// ensure we are joined to the channel
if (!channel.Joined.Value)
{
if (!alreadyJoined && channel.Type == ChannelType.Public)
{ {
var req = new JoinChannelRequest(channel, api.LocalUser); var req = new JoinChannelRequest(channel, api.LocalUser);
req.Success += () => req.Success += () => JoinChannel(channel, true);
{
channel.Joined.Value = true;
JoinChannel(channel);
};
req.Failure += ex => LeaveChannel(channel); req.Failure += ex => LeaveChannel(channel);
api.Queue(req); api.Queue(req);
return; return channel;
} }
channel.Joined.Value = true;
} }
if (CurrentChannel.Value == null) if (CurrentChannel.Value == null)
@ -304,6 +332,8 @@ namespace osu.Game.Online.Chat
// let's fetch a small number of messages to bring us up-to-date with the backlog. // let's fetch a small number of messages to bring us up-to-date with the backlog.
fetchInitalMessages(channel); fetchInitalMessages(channel);
} }
return channel;
} }
public void LeaveChannel(Channel channel) public void LeaveChannel(Channel channel)
@ -353,13 +383,8 @@ namespace osu.Game.Online.Chat
{ {
foreach (var channel in updates.Presence) foreach (var channel in updates.Presence)
{ {
if (!channel.Joined.Value) // we received this from the server so should mark the channel already joined.
{ JoinChannel(channel, true);
// we received this from the server so should mark the channel already joined.
channel.Joined.Value = true;
JoinChannel(channel);
}
} }