server icon indicating copy to clipboard operation
server copied to clipboard

Things to change for a protocol v2

Open Askaholic opened this issue 4 years ago • 6 comments

I'd like to start keeping track of things we want to change in a new protocol.

Stuff to remove

  • [x] Remove QDataStreamProtocol #816 It is pointless, everyone hates it.
  • [x] Remove all modvault functionality #816 Only the legacy client still uses it. All clients should use the API instead.
  • [x] Remove create_account command completely #816
  • [x] Remove id and login from the welcome message. #816 Use corresponding attributes on the me object instead
  • [x] Remove mod from game_matchmaking command. #816 Use queue_name instead.
  • [x] Remove faction from game_matchmaking command. #816 Use set_party_factions instead.
  • [ ] Remove password based authentication Use token instead
  • [x] Remove init_mode from game_launch command. #816 Use game_type instead.
  • [ ] Remove kick and kill styles from notice messages Use a dedicated administrative command instead
  • [x] Remove search boundaries from matchmaker_info message #816
  • [x] Remove irc_password command IRC authentication will use tokens from the API
  • [ ] Remove global_rating, ladder_rating, and number_of_games from player_info message. Use ratings instead

Stuff to change

  • [ ] Rework command_avatar
    • [ ] Rename action list_avatar to list
    • [ ] Return avatar id's in avatar list
    • [ ] Select avatar via id instead of url
  • [x] Move from UTF-16 to UTF-8 #629 We are just wasting bytes
  • [ ] Move the protocol to websocket, choose a websocket subprotocol (e.g. STOMP?) This would allow more web interactions
  • [ ] Guarantee that every message is a json dictionary (not a list) Simplifies parsing
  • [ ] Change all unix timestamps to ISO timestamps. For example launched_at in game_info
  • [ ] Remove or rework ping and pong commands? Maybe only send them if no other messages have been sent
  • [ ] Rework game_info so that messages are not sent sometimes as a list and sometimes as individual games
  • [ ] Use player id instead of username in game_info teams section #812
  • [x] Don't send game_launch message when match is cancelled #816 Handle match_cancelled instead
  • [ ] Rework command_ask_session What is the point? What does the policy server use this for? Can it be removed?

Stuff to add

  • [ ] Add message Id to every message (or a message envelope)
  • [ ] Add request Id to messages that are a direct response to other messages.
  • [x] ~A way to tell when a player has left an ongoing game (player dies but game is not over).~ #831
  • [ ] Matchmaker match confirmation #607
  • [ ] Add a way to check supported protocol version of clients

Fancy features

  • [ ] Auto-generate documentation or describe protocol in a formal language that allows creation of code in multiple languages Use dataclasses to describe message fields

Askaholic avatar Apr 11 '20 05:04 Askaholic

  • Move from UTF-16 to UTF-8
  • Add messageId to every message (or a message envelope)
  • Move the protocol to websocket, choose a websocket subprotocol (e.g. STOMP?)
  • Auto-generate documentation of the new protocol or describe protocol in a formal language that allows creation of code from it in multiple languages
  • Some messages are either Object or List.
    • every message should be a json object
    • if it needs a list, make it contain a list
    • if the list only contains one item, it's still a list

Brutus5000 avatar Apr 11 '20 06:04 Brutus5000

Which messages are sent as a list?

Askaholic avatar Apr 11 '20 07:04 Askaholic

I think it's the list of open games.

Also documenting the new protocol is not a fancy feature, it should be mandatory. Otherwise in 3 years the next dev sits here and tries to find all the details of the protocol

Brutus5000 avatar Apr 11 '20 08:04 Brutus5000

Additional things I'd like to see:

  • Publish all events on RabbitMQ as well
  • New "a player logged in" event (skip if there is already an open session from another connection)
  • New "a player logged of" event (only if the last connection is closed)
  • New "player joined game" event
  • New "player left game" event
  • New "game started" event

Brutus5000 avatar Nov 21 '21 08:11 Brutus5000

I think that could just be a normal issue as it doesn’t require breaking changes to the protocol.

Askaholic avatar Nov 21 '21 18:11 Askaholic

Saying now that we shouldn't remove ping and pong as I am now going to use them to check if the connection is still alive client side as I realized that the client will only say the connection is closed due to internet outage when it fails on write which right now all write actions require user action so the client doesn't know a connection is dead until the user takes action.

Sheikah45 avatar Apr 29 '22 21:04 Sheikah45