Can we please simplify the protocol

Can we please simplify the protocol

Discuss development of lobby clients, server, autohosts and auto-download software.

Moderators: Moderators, Lobby Developers

Post Reply
User avatar
MasterBel
Posts: 271
Joined: 18 Mar 2018, 07:48

Can we please simplify the protocol

Post by MasterBel »

There are so many individual success/failure commands in the lobby server protocol now that it's kind of silly trying to implement it. I strongly suggest we go a different way: using the generic "Failed" command and adding a generic "Success" command. Something like…

Success [COMMAND] (command argument is optional – preferably clients would be using message IDs. wouldn't be optional in the final spec – would be there or not)
Failure [Command] {Reason}

Having this as a standard response to *all* fail/success commands would also make a nice alternative to the commands which *may* get a ServerMessage response (e.g. ChangePassword) and make it easier to deliberately display client behaviour based on responses of an expected format that we know we can count on, without having to try to "guess" whether it's a failure or a success.

Similarly, why are these all separate commands?

Is there room to update these commands? I'm willing to do the work, if you can assure me it would be considered.

I'm very sorry about the last link but I had one spare and had to do something with it.
abma
Spring Developer
Posts: 3798
Joined: 01 Jun 2009, 00:08

Re: Can we please simplify the protocol

Post by abma »

the issue was, that changing the protocol mostly broke clients. thats why mostly new commands where added when some feature was missing...

the main issue i guess is, that lobby server and lobby client development doesn't happen in sync.

if you really want to improve stuff i guess you have to start client and server development.
User avatar
PicassoCT
Journeywar Developer & Mapper
Posts: 10450
Joined: 24 Jan 2006, 21:12

Re: Can we please simplify the protocol

Post by PicassoCT »

I say, sir, those who demand a lower protocol level, stoop to be lower class.

Srsly, protocols are always a first draft expanded on and then walled in. Licho and
User avatar
PicassoCT
Journeywar Developer & Mapper
Posts: 10450
Joined: 24 Jan 2006, 21:12

Re: Can we please simplify the protocol

Post by PicassoCT »

The others should agree though
User avatar
MasterBel
Posts: 271
Joined: 18 Mar 2018, 07:48

Re: Can we please simplify the protocol

Post by MasterBel »

abma wrote: 18 Jun 2020, 23:13 the main issue i guess is, that lobby server and lobby client development doesn't happen in sync.

if you really want to improve stuff i guess you have to start client and server development.
I still can't get SpringLobby compiling on my machine. Setting up WxWidgets stuff is permanently a pain. (It hurts more that I've got it working once before in the past, and can't work it out now).

I should be able to (at least mostly) deal with things with Chobby/ Uberserver, but I'd need someone's help with SpringLobby. But if you (or someone else) is interested in helping out on lobby side, and there are no complaints from a protocol quality standpoint, I'll start doing some testing.
User avatar
Silentwings
Posts: 3720
Joined: 25 Oct 2008, 00:23

Re: Can we please simplify the protocol

Post by Silentwings »

Potentially all of SL, Chobby, SPADS, SLDB, Maddox's autohosts, theIRC bridge, the matrix bridge, and Notalobby suffer fully breaking changes here, so you're looking at a potentially huge amount of favours/PRs. All you're gaining is merging some commands - which on its own is imo a questionable idea of simplification, it would move lots of "if" statements from server<->client code and vice versa, but afaics without any reduction in the complexity of what information has to be communicated.
Having this as a standard response to *all* fail/success commands would ...
Wanting "all" requests to be sent a generic pass/fail response simply doesn't work e.g. successful commands mostly need non-binary responses, in the form of state change messages to several clients, after which a generic success message would be pointless. There are also failure cases on server side where the server cannot know if the client would consider the result a success or not (e.g. request to set team id to current team id, which is denied because client does not have battle host rights). A generic FAILED command already exists, used in >50 places, often to inform client devs of invalid/ill-formatted requests that they should have known not to ask (if backwards compat was forgotten, it could be used more widely). It's unclear to me why you mention messageIDs - do you see logical inconsistencies if they remained optional? Do you know that there wouldn't be logical inconsistencies if they didn't?
Similarly, why are these all separate commands?
Bandwidth, older parts of the protocol try harder to minimize it.
commands which *may* get a ServerMessage response (e.g. ChangePassword) and make it easier to deliberately display client behaviour based on responses
This seems like a more genuine issue, if you can simply list the troublesome cases that would be better.
User avatar
MasterBel
Posts: 271
Joined: 18 Mar 2018, 07:48

Re: Can we please simplify the protocol

Post by MasterBel »

Silentwings wrote: 19 Jun 2020, 09:58 Potentially all of SL, Chobby, SPADS, SLDB, Maddox's autohosts, theIRC bridge, the matrix bridge, and Notalobby suffer fully breaking changes here, so you're looking at a potentially huge amount of favours/PRs.
Good point. Thanks for the heads up.
Silentwings wrote: 19 Jun 2020, 09:58 it would move lots of "if" statements from server<->client code and vice versa, but afaics without any reduction in the complexity of what information has to be communicated.
With my protocol implementation I'd only see simplifications on clientside. I don't think it's reducing complexity, but it's reducing repetition and (in theory) increasing extensibility.
Silentwings wrote: 19 Jun 2020, 09:58 Wanting "all" pass/fail replies to switch to a generic version simply doesn't work e.g. successful commands mostly need non-binary responses, in the form of state change messages to several clients, after which a generic success message would be pointless. There are cases (e.g. request to kick a non-existing user) where the server does not know if the client would consider the state change a success or not. A generic FAILED command already exists, used in >50 places, often to inform client devs of invalid/ill-formatted requests (if backwards compat was forgotten, it could be used more widely). It's unclear to me why you mention messageIDs - do you see logical inconsistencies if they remained optional? Do you know that there wouldn't be logical inconsistencies if they didn't?
commands which *may* get a ServerMessage response (e.g. ChangePassword) and make it easier to deliberately display client behaviour based on responses
This seems like a more genuine issue, if you can simply list the troublesome cases that would be better.
MessageIDs are useful for ascertaining which success/fail replies to which request, and would provide extra opportunity for simplifying the protocol… in theory.

I'm primarily thinking of messages that get the Succeeded/Failed(with a reason) responses that don't correlate to state changes which would be sent out to everyone.

And now for the lists of commands that may benefit:

Login commands:
  • Register
  • Login (Questionable because Agreement-related commands are also in there)
  • ConfirmAgreement
Account-related commands (unless otherwise specified, these all have their own Denied/Accepted commands which all follow the same format):
  • RenameAccount (currently ServerMSG response)
  • ChangePassword (currently ServerMSG response)
  • ChangeEmailRequest
  • ChangeEmail
  • ResendVerification
  • ResetPasswordRequest
  • ResetPassword
General commands that use Accepted/Failed format, plus some other maybe(maybe not, let me know)-redundant information that would better work with messageIDs:
  • Join (possibly could remove the Join response from the server and just rely on the "Joined" which would now also be sent to the new client. – like how JoinFrom appears to work.)
  • BridgeClientFrom (Already uses the Failed command)
  • UnBridgeClientFrom (Already uses the Failed command)
General commands that use Failed, plus a state-updating command which indicates success that is broadcasted to everyone who needs to know about it:
  • Join (Kind of. Also in the above category)
  • JoinFrom
  • LeaveFrom
  • SayFrom
Commands that use a custom <CommandName>Failed, plus a state-updating command which indicates success that is broadcasted to everyone who needs to know about it:
  • OpenBattle
  • JoinBattle
Also, is this an error in the documentation? "Response" description for JoinFrom links to UNBRIDGECLIENTFROM:server – which doesn't exist. Is it supposed to be UNBRIDGEDCLIENTFROM:server ?

Also somewhat less related, the JSON command could use more description. I had a look into it, I think I won't be qualified to write the description until I've actually implemented a few JSON commands on the client-side.
User avatar
Silentwings
Posts: 3720
Joined: 25 Oct 2008, 00:23

Re: Can we please simplify the protocol

Post by Silentwings »

After some thought, a generic "SUCCESS" command is not sensible, better that the server just informs the client of what changed, and leaves open the option that the server may not react exactly as the client requested/expected. FAILED is ok, to be used in cases where the server changes nothing, and sends back a human readable error/explanation instead. That is what's most extensible/future proof.

Re your list, the login sequence & JOIN/**BATTLE & messageIDs are not worth touching for this, breaks far too much for too little gain / are not suitable. The **FROM set already uses the nicest format for "simple" commands imo. That leaves the user management stuff

Code: Select all

RenameAccount 
ChangePassword 
ChangeEmailRequest 
ChangeEmail 
ResendVerification 
ResetPasswordRequest 
ResetPassword
Two possible changes here:
(1) Add ACCEPTED/DENIED replies to RENAMEACCOUNT/CHANGEPASSWORD, to remove the (only) amibiguity on client side. This only requires a tiny PR to uberserver & the LobbyProtocol docs.
(2) Move the failure replies of all the above to generic FAILED command, and have the successful replies put into the reflection style with RENAMEDACCOUNT/CHANGEDPASSWORD/CHANGEDEMAILREQUEST/etc and add suitable reflected args specifying what changed (ie. consistent with **FROM syntax). I guess that requires
- probably a compat flag
- PRs to uberserver and the LobbyProtocol docs
- PR to SL (iirc it knows only CHANGEPASSWORD and RENAMEACCOUNT)
- possible PRs/favour requests to Chobby, NotaLobby and Bibim's IrcBridge (if they use any of them, likely they do but I don't remember)

I probably should have done (2) myself, at the point where I added half of those commands, but I don't plan to touch it rn.
error in the documentation
https://github.com/spring/LobbyProtocol/issues/new
Join (Kind of. Also in the above category)
There is some minor oddity in the reflection of JOIN/JOINED, no idea what history caused it, but trying to change it is not worth the trouble (I once removed this oddity by accident and broke almost everything that tried to connect).
Post Reply

Return to “Lobby Clients & Server”