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

Deprecated messages related to privileges? #1025

Closed
mathiascode opened this issue Mar 17, 2021 · 5 comments
Closed

Deprecated messages related to privileges? #1025

mathiascode opened this issue Mar 17, 2021 · 5 comments

Comments

@mathiascode
Copy link
Member

mathiascode commented Mar 17, 2021

It seems like a few server messages related to privileges are no longer in use. When I purchased privileges and attempted a few transfers, I never saw the following messages, even with the official client:

  • AddToPrivileged (code 91)
  • UserPrivileged (code 122)
  • NotifyPrivileges (code 124)
  • AckNotifyPrivileges (code 125)

It would appear that most of these messages are redundant, either replaced with private messages both when receiving and gifting privileges, or the PrivilegedUsers (code 69) and GetUserStatus (code 7) messages for registering the change. The privileged boolean was apparently added to the GetUserStatus message in 2006: https://github.com/Nicotine-Plus/nicotine-plus/blob/master/NEWS.md#version-125-september-17th-2006

The documentation for NotifyPrivileges and AckNotifyPrivileges seems to be quite inconsistent and incorrect in some cases, and Nicotine+ previously sent a NotifyPrivileges message to the server, while the official client didn't.

Looking at the message list in the Soulfind server software, there's a comment next to code 69 about the server switching to a new system, but ultimately it seems like this user list is still the preferred method, as it's sent each time you log into the server. https://github.com/seeschloss/soulfind/blob/master/src/message_codes.d#L75

@jpdillingham Is this something you've previously looked into?

@jpdillingham
Copy link
Contributor

Yep, a lot of this has been deprecated. I coded it all up based on the museek documentation the Nicotine+ source, then bought privileges to test and found out that not much of it actually works.

You can check your own privileges with code 92, someone else with 122, and gift privileges with code 123. The behavior of the grant operation is a pretty big departure from the rest of the API (all of this functionality is, really); my notes are here.

It still seems odd to me that the server doesn't bother sending a message to notify everyone when new privileges are granted, yet retains the initial PrivilegedUsers list upon login, but I just tested it by gifting a few days to a couple clients and nothing is sent to the recipient or anyone else.

@mathiascode
Copy link
Member Author

It still seems odd to me that the server doesn't bother sending a message to notify everyone when new privileges are granted, yet retains the initial PrivilegedUsers list upon login.

It's strange indeed. I can see why AddToPrivileged was deprecated, as it doesn't provide a way of notifying clients of expired privileges. I'm assuming 122, 124 and 125 were mostly failed experiments that were eventually embedded into the AddUser watch system.

@mathiascode
Copy link
Member Author

mathiascode commented Mar 18, 2021

The AddUser reponse contains no boolean flag for privileges, but there seems to be a special case where a GetUserStatus message is sent in addition to the AddUser response if the user is privileged.

Edit: seems like this actually isn't the case. I'll have to rethink this.

@mathiascode
Copy link
Member Author

mathiascode commented Mar 18, 2021

I ended up sending a GetUserStatus message in addition to AddUser when watching a user. This way, I can be informed of the user's privilege status even if their privileges were granted after the initial PrivilegedUsers list was received, The server should automatically send GetUserStatus updates in the future.

SoulseekQt also seems to send a AddUser/GetUserStatus combo when watching users.

@mathiascode
Copy link
Member Author

Nicotine+ previously sent a NotifyPrivileges message to the server, while the official client didn't.

Turns out Nicotine+ followed the behavior of Soulseek NS in this case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants