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

Resolve issues with Discord RPC while on Do Not Disturb #27354

Merged
merged 6 commits into from Feb 28, 2024

Conversation

jvyden
Copy link
Contributor

@jvyden jvyden commented Feb 24, 2024

While experimenting with osu!'s Discord integration, I noticed that when you set your user status to 'Do not disturb' then osu! will stop updating things on Discord and simply show that the user is idle.

The reason this happens is osu! explicitly looks for the Online status instead of including DoNotDisturb. This is likely unintentional, since right above this there's another separate check to stop the presence update entirely if the user is considered to be Offline.

To fix this, we simply remove the check for the user status entirely from that if statement since it's not necessary.

@bdach
Copy link
Collaborator

bdach commented Feb 26, 2024

Looking at blame the behaviour in question looks to have been implemented intentionally. There was no specific rationale given, though.

I can see where such a decision could come from (not wanting people to join your game or whatever) but I'd probably treat UserStatus.DoNotDisturb the same way as DiscordRichPresenceMode.Limited, i.e. hide identifiable information maybe?

@jvyden
Copy link
Contributor Author

jvyden commented Feb 26, 2024

Hmm, well if the problem is that you might get bothered while in DND, I feel like the better solution would just be to hide such interactions instead of going all-out hiding everything - privacy is a different issue than not wanting to be joined/spectated.

osu!'s Discord integration doesn't seem to currently support game invites/spectating/the interactivity stable currently has so I think it's best to discuss that whenever that functionality gets implemented rather than right now... On that note, Discord RPC is something I'm interested in working on improving - let me know if you would like to see more PRs related to it :)

@frenzibyte
Copy link
Member

I feel like the better solution would just be to hide such interactions instead of going all-out hiding everything - privacy is a different issue than not wanting to be joined/spectated.

That's what's being proposed here, to change the code so that setting to DoNotDisturb will make rich presence still work but in "limited" mode. In that mode, when the user is in a room, it does not show the room title in that user's discord.

@peppy
Copy link
Sponsor Member

peppy commented Feb 27, 2024

Indifferent on this change.

@jvyden
Copy link
Contributor Author

jvyden commented Feb 27, 2024

Cool, I've made that happen. It'll now hide the room name while in multiplayer and do not disturb is active, but will retain all details in all other scenarios.

@jvyden jvyden changed the title Apply Discord RPC changes regardless of user's status Resolve issues with Discord RPC while on Do Not Disturb Feb 27, 2024
Comment on lines 97 to 98
bool hideIdentifiableInformation = privacyMode.Value == DiscordRichPresenceMode.Limited;
bool hideInteractions = status.Value == UserStatus.DoNotDisturb && activity.Value is UserActivity.InLobby;
Copy link
Member

Choose a reason for hiding this comment

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

I do not see why these are separate variables, the intention is that the DnD status should behave exactly like "Limited" mode.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought that was what we agreed we didn't want? I'm confused now. 😅

Copy link
Member

Choose a reason for hiding this comment

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

You may have misunderstood my comment, I meant that DnD and "Limited" mode should be the same.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you mind clarifying why this is your stance over my proposed solution? I honestly find that kind of an extreme solution for the problem in-hand.

Copy link
Member

Choose a reason for hiding this comment

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

I wouldn't say that, since the "DnD" status hasn't been given any special behaviour yet, and this looks like one candidate that the player might benefit from (hiding identifiable information when player is in DnD mode, as to not be disturbed).

In addition, "interactions" and "identifiable information" really are one and the same for the existing cases of user statuses, the entire point of the "limited" mode is to prevent the user/player from being disturbed.

I'll try to keep things clear next time before your contribution goes in.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for the clarification, it does help provide some insight.

I wouldn't say that, since the "DnD" status hasn't been given any special behaviour yet, and this looks like one candidate that the player might benefit from (hiding identifiable information when player is in DnD mode, as to not be disturbed).

In addition, "interactions" and "identifiable information" really are one and the same for the existing cases of user statuses, the entire point of the "limited" mode is to prevent the user/player from being disturbed.

Hmm, I suppose we have different definitions of "disturbed" then. I see limited mode as more of a privacy feature, as in "I don't want people to see my profile".

I've seen other players get self-conscious and end up playing offline or otherwise hide their profile whether that be from some goofy top plays or them having their real name as their username, so that's what I figured limited mode was for.

In general I tried to think ahead about what DND would probably mean for osu! in the future, and to me I came up with "please don't DM me and please don't join my lobbies, etc." So I suppose that's where that miscommunication originated from.

I'll try to keep things clear next time before your contribution goes in.

Sweet. It was mostly confusing for me because I was like "wait, why are my changes getting overwritten?" xD

Communication is always good.

@bdach bdach disabled auto-merge February 28, 2024 10:37
@bdach bdach merged commit 5ccd53e into ppy:master Feb 28, 2024
11 of 17 checks passed
@jvyden jvyden deleted the discord-dnd-fix branch February 28, 2024 20:46
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.

None yet

4 participants