search

FAForever / server

The servercode for the Forged Alliance Forever lobby
http://www.faforever.com
GNU General Public License v3.0
66 stars 61 forks source link

Issue/#946 Update achievements over RabbitMQ #949

Closed Askaholic closed 1 year ago

Askaholic commented 1 year ago

Reworks the achievements updating code to send messages to rabbitmq instead of trying to post them to the API over http. This allows us to get rid of a whole bunch of broken boilerplate code for handling API requests. The only thing that becomes more annoying is handling the responses as it needs to function even when the other side is temporarily down, so we need to handle responses asynchronously and can't assume they will come in within any particular time frame.

Currently, if the player disconnects before getting a response from the API, the response will just be ignored when it does eventually come in. We could make things more complex by adding logic to allow those message to come in even after re-logs. It would just require more memory and optionally also a hard time out after which we would ignore the responses.

Closes #946

Sheikah45 commented 1 year ago

What even are the messages the server sends to the client about achievements?

When we reworked the code for the messages in the client. I don't think any achievement handling was even implemented. I think it all just queries the api now.

Askaholic commented 1 year ago

Idk. If it’s not used in the client I’d be happy to remove it from the server. I assume it would have been for showing a notification when you unlock an achievement.

Sheikah45 commented 1 year ago

If I knew the message command name I could check if we even would recognize it

Askaholic commented 1 year ago

It’s called updated_achievements https://github.com/FAForever/server/blob/dee639f4e271662b593f44f449169eb06a8d3ec1/server/lobbyconnection.py#L306-L309

Askaholic commented 1 year ago

Nope. The nice thing about rabbitmq is that if nothing is listening on the other side, the messages just disappear into the void. We could preemptively configure a queue to capture the messages, and then once the API implementation is deployed, anything sitting in the queue will start to be processed. Up to @Brutus5000 really if he wants to do that.

Brutus5000 commented 1 year ago

I have no pain in deployment order because right now everything is broke anyway. However, before merging we should make sure that it is compatible (or can be made compatible) with api code.

Askaholic commented 1 year ago

playerId is in there. See the integration test.