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

Fix issue where players added or removed from friend or foe lists did not have games updated #1026

Closed benjamin-lawson closed 3 weeks ago

benjamin-lawson commented 2 months ago

Changes

Potential Issues

TODO

Closes #1002

Askaholic commented 2 months ago

Hey thanks for the PR! I added the issue link to the description so it shows up in GitHub nicely.

I think you can avoid much of the DOS concerns by sending only the 1 game_info message that actually would change instead of the entire game list. Since the visibility will only change for the game that the player doing the social add/remove is currently hosting. Hopefully the way I said that makes sense.

Thanks for taking this on! I have so little time for FAF development anymore but more than happy to give reviews and answer any questions you may have about the code!

benjamin-lawson commented 2 months ago

Hey thanks for the PR! I added the issue link to the description so it shows up in GitHub nicely.

I think you can avoid much of the DOS concerns by sending only the 1 game_info message that actually would change instead of the entire game list. Since the visibility will only change for the game that the player doing the social add/remove is currently hosting. Hopefully the way I said that makes sense.

Okay! So if I send the client a game_info message with just the single game that would be affected by the friend or foe change, the client would update only that single game? I would think it would remove all the other games in that case.

Thanks for taking this on! I have so little time for FAF development anymore but more than happy to give reviews and answer any questions you may have about the code!

No worries at all! Trying to knock out some of the low hanging fruit on the server side to understand the codebase better.

Askaholic commented 2 months ago

So the way the protocol is designed is that the client only removes a game from the game list when it sees a game_info message with "state": "closed". That way we can send updates on a per game basis without having to send the entire game list. The main code that handles this is in the broadcast service: https://github.com/FAForever/server/blob/645d001c08228f8f30f031f30f44a8eca624e9a5/server/broadcast_service.py#L89

You can see there that only the “dirty” (i.e. games that had a state change that hasn’t been sent to the connected clients yet) games are added to the list. Any games that haven’t seen any state changes are ignored.

Hope that helps!