Spring RTS Engine

Posted: **18 Jun 2020, 17:14**

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.

Posted: **18 Jun 2020, 23:13**

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.

Posted: **19 Jun 2020, 07:44**

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

Posted: **19 Jun 2020, 07:45**

The others should agree though

Posted: **19 Jun 2020, 08:18**

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.

Posted: **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. 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.

Posted: **19 Jun 2020, 11:59**

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.

Posted: **20 Jun 2020, 08:51**

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).

Spring RTS Engine

Can we please simplify the protocol

Can we please simplify the protocol

Re: Can we please simplify the protocol

Re: Can we please simplify the protocol

Re: Can we please simplify the protocol

Re: Can we please simplify the protocol

Re: Can we please simplify the protocol

Re: Can we please simplify the protocol

Re: Can we please simplify the protocol