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

Discord Rich Presence #721

Closed
EstebanQ33 opened this issue Dec 1, 2018 · 40 comments
Closed

Discord Rich Presence #721

EstebanQ33 opened this issue Dec 1, 2018 · 40 comments
Labels
enhancement New feature or request
Milestone

Comments

@EstebanQ33
Copy link

It would be good to know which server my friends are on, and to be able to enter through discord, and know the played time.

@botder
Copy link
Member

botder commented Dec 1, 2018

I tried implementing Discord Rich Presence, but Discord would still show the presence after quitting the game sometimes. Please use Feature request issue template next time.

@botder botder added the enhancement New feature or request label Dec 1, 2018
@Wolfee-J
Copy link

Wolfee-J commented Dec 1, 2018

You gotta send a signal that tells discord the game has ended when the client disconnects / game closes.

@kereis
Copy link

kereis commented Jan 31, 2019

Is there still something going on or is there a possibility to contribute to solving this issue?

@Fernando-A-Rocha
Copy link
Contributor

PLEASE implement Discord Rich Presence for MTA! It would be amazing. Definitely worth coding.

@kereis
Copy link

kereis commented Feb 3, 2019

PLEASE implement Discord Rich Presence for MTA! It would be amazing. Definitely worth coding.

Well, in case there's not much movement and as it hasn't high priority, I'd like to take a look at it. Can't say any ETA right now.

@qaisjp
Copy link
Contributor

qaisjp commented Feb 3, 2019

Go for it! Might be worth expanding on or taking a look at https://github.com/botder/mtasa-blue/tree/feature/discord-richpresence

It's also worth sharing your ideas here before implementing them.

@kereis
Copy link

kereis commented Feb 3, 2019

Go for it! Might be worth expanding on or taking a look at https://github.com/botder/mtasa-blue/tree/feature/discord-richpresence

It's also worth sharing your ideas here before implementing them.

Sure. Gotta take a look at the repo you've provided. Sorry in advance for my lacking experience in c++. Just in case you can take a look at it and suggest any improvements.

@kereis
Copy link

kereis commented Feb 5, 2019

Go for it! Might be worth expanding on or taking a look at https://github.com/botder/mtasa-blue/tree/feature/discord-richpresence

It's also worth sharing your ideas here before implementing them.

@qaisjp
Can I simply merge their branch to my fork?

@qaisjp
Copy link
Contributor

qaisjp commented Feb 5, 2019

Can I simply merge their branch to my fork?

@dopauthor: not exactly merging:

# Pull botder's branch into your own local repository
git fetch https://github.com/botder/mtasa-blue feature/discord-richpresence:feature/discord-richpresence

# Checkout the feature branch
git checkout feature/discord-richpresence

# Push that up to your own
git push

For help with pushing to your own fork (the final step), please read this page.

@kereis
Copy link

kereis commented Feb 6, 2019

Do I need any additional extension in order to solve these CMake errors?

image

Also, in core/CDiscordRichPresence.cpp

void CDiscordRichPresence::SetServerPresence(bool bOutdatedServer)
{
    if (bOutdatedServer)
    {
        // TODO: Read server name and player limit from server query protocol
    }

    m_PresenceState = ePresenceState::ON_SERVER;
    m_tPresenceTime = std::time(nullptr);
    UpdatePresence();
}

At TODO, isn't just using the last host and port and retrieving the server infomation through CServerBrowser enough?

@kereis
Copy link

kereis commented Feb 18, 2019

Any updates on this one or is this issue rather in the "backlog"?

@qaisjp qaisjp added this to the Backlog milestone Feb 18, 2019
@qaisjp
Copy link
Contributor

qaisjp commented Feb 18, 2019

I do not know why you are getting this error. Are you using Visual Studio or VS Code?

At TODO, isn't just using the last host and port and retrieving the server infomation through CServerBrowser enough?

As for your TODO, at the moment I do not know the best way to retrieve this information. Your suggestion might work, but this might also be synced to the client. Maybe there's a client command that pulls out this information (something like sinfo maybe)?

@kereis
Copy link

kereis commented Feb 22, 2019

I do not know why you are getting this error. Are you using Visual Studio or VS Code?

At TODO, isn't just using the last host and port and retrieving the server infomation through CServerBrowser enough?

As for your TODO, at the moment I do not know the best way to retrieve this information. Your suggestion might work, but this might also be synced to the client. Maybe there's a client command that pulls out this information (something like sinfo maybe)?

I was using Visual Studio. Could possibly be that some stuff of it is on an other directory as I do not have much space left on my main drive.

@patrikjuvonen
Copy link
Contributor

How are you trying to build MTA?
Also did you install some form of CMake with Visual Studio?
Curl should not contain CMakeLists.txt anymore so you must have an outdated branch or outdated local version. Try pulling the latest master into your branch.

@kereis
Copy link

kereis commented Feb 26, 2019

How are you trying to build MTA?

I haven't build anything yet. I just simply opened the project with Visual Studio.

Also did you install some form of CMake with Visual Studio?

I'm not sure about CMake, because I haven't seen any options or anything during the installation where I could select it.

Curl should not contain CMakeLists.txt anymore so you must have an outdated branch or outdated local version. Try pulling the latest master into your branch.

I actually merged the latest master to my branch. Maybe I have to try that again.

//EDIT: Also, are there any must-have packages? I even get errors when compiling the project which seem like that for some reason the included libraries aren't recognized.

@mehrangta
Copy link

Is it gonna be added ?

@qaisjp
Copy link
Contributor

qaisjp commented May 10, 2019

It's in the Backlog, so it's something we would like to implement - we plan to include it in the future. It's not an urgent feature nor something that is guaranteed to happen.

@kereis
Copy link

kereis commented May 10, 2019 via email

@qaisjp
Copy link
Contributor

qaisjp commented May 10, 2019

Please can you join Discord and share your build issues in #development? We're happy to help you out there.

If you would prefer not to use Discord, feel free to open an issue describing your actions + errors, and we will try to lend you a hand.

@kereis
Copy link

kereis commented May 10, 2019 via email

@kereis
Copy link

kereis commented May 15, 2019

So appearently, my compiling problems just disappeared and everything seems working fine.
I just updated from VS 2017 to 2019 and seems it did the job.

I do not know why you are getting this error. Are you using Visual Studio or VS Code?

At TODO, isn't just using the last host and port and retrieving the server infomation through CServerBrowser enough?

As for your TODO, at the moment I do not know the best way to retrieve this information. Your suggestion might work, but this might also be synced to the client. Maybe there's a client command that pulls out this information (something like sinfo maybe)?

As far as I remember, using the command sinfo doesn't display the server host name.

About using CServerBrowser: If somebody connects to a server, is the server item added to the recent list even if connection was not successful? Perhaps you could iterate through the recent list and look up for the CServerListItem which matches with the address of the server we're trying to get the server name from.
But it doesn't really seem like an elegant solution as if your recent list is very huge, it will take up time to get the server name.

@0x416c69
Copy link
Contributor

0x416c69 commented Jul 15, 2019

Just so you know, I'm implementing it (well, it's done actually and will make PR at any moment)

Edit: I didn't notice that branch when I was making it and what I made is different, scripters are allowed to change parameters and it's unlike that branch, it's totally different.

@qaisjp
Copy link
Contributor

qaisjp commented Jul 15, 2019

That's great to hear, @0x416c69! Are you on the MTA Discord as well?

scripters are allowed to change parameters

Sounds good, but we may have to be a little careful here for a couple of reasons, one of which is that our client_id would end up being used and we can't moderate server scripts.

I look forward to your PR so we can discuss your work. In the future I'd recommend you to speak with us first — the last thing we want is you working on features and it not being accepted!

@botder
Copy link
Member

botder commented Jul 15, 2019

IMO, servers shouldn't be able to display any text they want in my Discord presence text.

@0x416c69
Copy link
Contributor

0x416c69 commented Jul 15, 2019

Yes. and I already announced it on #development
image

Is there a reason why scripters should not control discord rich presence? Is there any terms about it on discord to violate by rpc texts? We can also allow them to have their own client_id which is already done by some other mods. (But I don't recommend this because scripters can go entirely incognito and some people won't find out it's even MTA:SA unless we take "details" and set it to Multi Theft Auto everytime they update the parameters)

As MTA:SA is highly scriptable I think it's best to let scripters control some texts, what I think is scripters should be able to configure these parameters:
image
Images are controlled by MTA, also the Rich Presence Text cannot be set.

Also you should know that there is more to Discord Game SDK but I haven't implemented it all.

Edit: @botder How about some user settings to 1. disallow certain servers from controlling your Discord Rich Presence (Something like user blacklist) 2. just disallow every server and use MTA's default 3. just disable the whole system?
Also instead of making a blacklist we can make a whitelist so the server asks the user to allow changing rich presence?

@kereis
Copy link

kereis commented Jul 15, 2019

Any chance that you're allowed to view the changes you made before PR or is it private?

@0x416c69
Copy link
Contributor

@humenius local repo, not yet pushed

@kereis
Copy link

kereis commented Jul 15, 2019

Ah, okay. Well, I was trying to do something there too but well, it sort of went into background. Gotta look for something else then to work on.

@qaisjp
Copy link
Contributor

qaisjp commented Jul 15, 2019

I think initially it would be good to have basic MTA rich presence and then we can iterate / see where to go from there.

What do you have in mind for the "joinSecret" feature?

Note that spectate will not be possible (esp. during our first iteration) because it requires special approval from Discord, and there are a couple Discord rules there.

Obviously we wouldn't want servers to abuse the details/state feature—maybe sometime in the future we could an approval process for servers, but there does not seem to be a consensus right now.

@0x416c69
Copy link
Contributor

0x416c69 commented Jul 15, 2019

Discord users can click "Join" on the invitation. Their game will then launch, and the joinGame() callback will fire in their client with the inviting player's joinSecret. The client should reverse hash or otherwise unencrypt this secret and match the players together.

Well there should be a client-sided function which allows scripters to set joinSecret containing some data that they need in order to handle clients who is invited by a user (or clicked on Join button of that user in Discord), joinSecret should be internally combined with ip address to connect when MTA:SA launches, this can be useful for some scripters who got groups or party or sections in their server, for example a PUBG server can use it to invite users and when they accept his invite or join him the server will find out which team the invited player should be joined to using that joinSecret.

Yes, I'm aware that spectate feature needs approval which is not that hard to achieve though. My friend got his game approved easily and fast but as this is a modification and not an entirely a game I'm not sure about it but I implemented it anyways.

Alright then, I'll revert those features and will make a PR of a basic rich presence just like how it's done in https://github.com/botder/mtasa-blue/tree/feature/discord-richpresence

Edit: I need your approval before merging that joinSecret feature thing, it will require an event and a function in client side.

@0x416c69
Copy link
Contributor

0x416c69 commented Jul 16, 2019

That basic rich presence implementation, there's an issue with backward compatibility on server.
MTA:SA does not share players slot (maxPlayers) within clients, I've handled server name with querying server within join (Which made me to clean up CServerListItem and making a new class called CQueryReceiver) but there is no way to receive maxPlayers for only once, it's modifiable using setMaxPlayers and there are couple of ways to handle this:

  1. Query the server every 15sec and receive data from there which is NOT RECOMMENDED
  2. RakPeer::GetMaximumNumberOfPeers from netc.dll (It's possible from the client right? I'm not sure I didn't test it)
  3. Check the server version, if it's updated enough it won't query the server every 15sec and instead server will notify the client for maxPlayers update, else query every 15sec.

Second way is the best way if it's possible.

@qaisjp
Copy link
Contributor

qaisjp commented Jul 16, 2019

The joinSecret feature seems good, and I give preliminary approval (I like the idea), but we need more information about the event and function

Yes, I'm aware that spectate feature needs approval which is not that hard to achieve though. My friend got his game approved easily and fast but as this is a modification and not an entirely a game I'm not sure about it but I implemented it anyways.

Okay. I'll get in touch with Discord. In this case I would rather we ask for permission from Discord rather than forgiveness. It may be that we need to introduce a second approval layer (so that only certain servers can use the spectate feature).

That basic rich presence implementation, there's an issue with backward compatibility on server. [...]

I think the best solution is to just:

  • make the server send this max player information to the client on connect/join
  • make setMaxPlayers resend this information

This would make it possible to implement getMaxPlayers client side. We could maybe get this simple feature (getMaxPlayers) in before 1.5.7, which is just around the corner.

For "new" clients connecting to "old" servers (i.e. connecting to servers that don't send the max player information), we could just omit partySize and partyMax?

@0x416c69
Copy link
Contributor

The event should be simple: onClientDiscordJoin(secretData) and the function: setJoinInfo(secretData, partySize, partyMax)

partySize and partyMax should be controlled by joining/inviting feature and player count is what I'm putting in "state" parameter, but yeah we can omit it.

@qaisjp
Copy link
Contributor

qaisjp commented Jul 16, 2019

Can you elaborate on why onClientDiscordJoin(secretData) should be a client event?

I'm thinking only the server needs to know (and server scripts can trigger events to clients as necessary).

@0x416c69
Copy link
Contributor

It shouldn't have to be a client event, but it's going through client, we can do shared events or just server but yeah in any way client needs to communicate with server about this secretData.

@qaisjp
Copy link
Contributor

qaisjp commented Jul 16, 2019

but it's going through client

is client here the user in the sidebar, or the user that has clicked "join game"?

@0x416c69
Copy link
Contributor

0x416c69 commented Jul 16, 2019

The user that has clicked "join game".
Also partyID needs to be set.
https://discordapp.com/developers/docs/game-sdk/activities#activity-action-field-requirements-requirements

@qaisjp
Copy link
Contributor

qaisjp commented Jul 16, 2019

I was thinking that the MTA client will decode this secret data to decide which server to join, and the remaining data will be sent to the server, accessible via onPlayerJoin (and/or onPlayerConnect).

What are your thoughts on that?

@0x416c69
Copy link
Contributor

Yeah, that's perfect.

I'll get onto it then.

@qaisjp
Copy link
Contributor

qaisjp commented Mar 30, 2020

Done in #1031.

@qaisjp qaisjp closed this as completed Mar 30, 2020
@qaisjp qaisjp modified the milestones: Backlog, 1.6 Mar 30, 2020
@Deihim007 Deihim007 mentioned this issue Mar 22, 2022
18 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

9 participants