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

Don't show multiplayer channels in chat overlay #16109

Merged
merged 1 commit into from Dec 16, 2021

Conversation

smoogipoo
Copy link
Contributor

Resolves #16099

channelManager.JoinedChannels.CollectionChanged += joinedChannelsChanged;

foreach (Channel channel in channelManager.JoinedChannels)
ChannelTabControl.AddChannel(channel);
channelManager.JoinedChannels.BindCollectionChanged(joinedChannelsChanged, true);
Copy link
Contributor Author

@smoogipoo smoogipoo Dec 16, 2021

Choose a reason for hiding this comment

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

There's no special reason why this was this way. BindCollectionChanged was added after the event.

@@ -436,12 +433,19 @@ private void joinedChannelsChanged(object sender, NotifyCollectionChangedEventAr
{
case NotifyCollectionChangedAction.Add:
foreach (Channel channel in args.NewItems.Cast<Channel>())
ChannelTabControl.AddChannel(channel);
{
if (channel.Type != ChannelType.Multiplayer)
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

we're gonna want to add spectator here at some point, but i guess that's not relevant until it's implemented.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Didn't realise we had a spectator type, can be done later. Also, what's a temporary channel?

Copy link
Sponsor Member

Choose a reason for hiding this comment

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

I believe it's an unused legacy type. Or maybe it is used but won't be relevant to lazer.

@smoogipoo
Copy link
Contributor Author

Not sure why this comment was removed but I'll repost it here:

image

It's a good point, but I don't feel comfortable with doing that for the time being, because it seems there's no notion of whether a user has left the channel other than the client explicitly saying they have. For example if the user just Alt+F4s, they'll remain joined to the channel indefinitely:

image

Can probably be addressed later / if it's a big issue, but I would hope that the new All players (round robin) queueing mode removes this sort of pressure of needing to look at the chat all the time.

@peppy
Copy link
Sponsor Member

peppy commented Dec 16, 2021

Was removed because they were watching my stream, where i said something along the lines of: the multiplayer chat display is intended to always be visible (ie. it will be added somewhere on song select / results), which is going to be the intended UX going forward. The global chat won't house the multiplayer/spectator chats, similar to how osu-web also doesn't show these.

Bit of a change from how stable does it, but I believe it will be a better flow.

@peppy peppy merged commit aa0813f into ppy:master Dec 16, 2021
@smoogipoo smoogipoo deleted the chat-overlay-multiplayer-removal branch September 11, 2023 02:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

No chat access in the multiplayer room
2 participants