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

Add more functionality to right clicking in the chat window #32446

Merged
merged 4 commits into from
Mar 19, 2025

Conversation

GioSDA
Copy link
Contributor

@GioSDA GioSDA commented Mar 19, 2025

Based off of what was requested in #32423

Implements a button to both spectate and invite to multiplayer lobby.

Spectate
https://github.com/user-attachments/assets/d0858736-9ed0-4f49-9ab4-5cfd333579a0

Invite to Lobby
https://github.com/user-attachments/assets/8d4148e2-6a5f-48e7-9cb2-add112fa3173

Code is basically just slightly modified copy/paste from UserPanel.cs

@peppy peppy self-requested a review March 19, 2025 03:13
peppy
peppy previously approved these changes Mar 19, 2025
@peppy peppy enabled auto-merge March 19, 2025 03:29
@GAMIS65
Copy link
Contributor

GAMIS65 commented Mar 19, 2025

user.IsOnline doesn't get updated in real-time, so whenever someone goes online after you, it won't work.

image

@smoogipoo smoogipoo disabled auto-merge March 19, 2025 05:22
@peppy
Copy link
Member

peppy commented Mar 19, 2025

user.IsOnline doesn't get updated in real-time, so whenever someone goes online after you, it won't work.

I noticed this while testing, but don't see this as the end of the world.

Using the MetadataClient flow isn't possible here because we aren't watching for user presences. I'd be very hesitant to turn this on every time chat is open, so think this should be addresses in the future (for reference, stable gets around this by always sending online/offline packets, with extra metadata being on-request as we already have it in MetadataClient).

I've added documentation around this and renamed some variable to hopefully make things more explicit.

@GAMIS65
Copy link
Contributor

GAMIS65 commented Mar 19, 2025

Using the MetadataClient flow isn't possible here because we aren't watching for user presences.

Friend presences do update in real-time, so at least adding a check will eliminate some cases where the button should appear but doesn’t, and vice versa.

@peppy
Copy link
Member

peppy commented Mar 19, 2025

I'm not too sure what you're saying. I don't see how we can check against both. In the case a friend is offline it's going to return a null, which is the same as the unknown case. Unless you mean also checking the friend flag. Which is not going to happen (too complicated for what it is).

@bdach
Copy link
Collaborator

bdach commented Mar 19, 2025

Can we just... not check the online flag at all in any capacity? I feel like while suboptimal, it's better what this currently is going to likely be which is state being frozen at the point of login more or less.

@peppy peppy self-requested a review March 19, 2025 07:17
@peppy
Copy link
Member

peppy commented Mar 19, 2025

Sounds good, have removed.

@GAMIS65
Copy link
Contributor

GAMIS65 commented Mar 19, 2025

I may have worded it poorly, what I meant is just checking the MetadataClient for the user presence.

if (user.IsOnline || metadataClient?.GetPresence(user.OnlineID) != null)
2025-03-19.08-19-44.mp4

@bdach
Copy link
Collaborator

bdach commented Mar 19, 2025

@GAMIS65 you're not understanding what you're being told. checking MetadataClient is useless here, because you have to call MetadataClient.BeginWatchingUserPresence() for that to do what you think it does reliably which is annoying to infeasible in the context of chat

@bdach bdach self-requested a review March 19, 2025 11:27
@bdach bdach merged commit 8b566d2 into ppy:master Mar 19, 2025
6 of 10 checks passed
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.

4 participants