spring / uberserver

uberserver, a matchmaking/chat lobby server for the spring rts project
https://springrts.com/wiki/Uberserver
Other
33 stars 38 forks source link

Add some parts of autohost commands to lobby protocol #366

Open PtaQQ opened 4 years ago

PtaQQ commented 4 years ago

From BAR Discord: PtaQ

it would be good to separate the player chat from host command chat and eventually get rid of the host chat

Bluestone:

+1, please make an uberserver ticket for this, to avoid horrible string passing it would require extending the lobby protocol a little but can def be done

This would make chat much more clearer, as autohost spam often pours in so fas that player messages may go without being read.

silentwings commented 4 years ago

Getting rid of the host using the chat isn't fully possible, because SPADS allows arbitrary extensions and customizations of its own protocol. What is possible is to standardise the central player<->autohost comms in way that allows lobbies to display them (and only them) in a dedicated GUI, if desired. This would allow a MP battle to run with much less host chat into the battles chat channel.

Admin/power user control of hosts would still go via text commands. These could be restricted to PM only but that part is a choice that would be made by SPADS, not by uberserver.

Likely best way to start is create a list of what info should be standardised as part of the lobby protocol & (by implication) GUI'ed:

It would need a per battle flag to state whether the new system was in use for that battle, users with clients unable to support this flag would not be permitted to join the battle.

@Yaribz, thoughts welcome.

Yaribz commented 4 years ago

Concerning specifically the separation of the autohost and player chat (to avoid autohost related messages flooding the chat): I think this can be done easily at lobby client side (and iirc it was done by TASClient long ago). You just need to filter all the battle lobby messages starting with "!" coming from the players and all the messages coming from the autohost in a separate (collapsable ?) tab/section dedicated to battle management. That way there would a "Battle chat" tab/section containing all the chat between players, and a "Battle management" tab/section containing all the autohost commands issued by players and the corresponding autohost messages/answers. Of course this is not ideal because there is still no real integration in the lobby client GUI (you need to watch the "Battle management" tab/section to follow votes for example), but at least players chat can be followed easily and this is doable right now, without changing anything on the infra side.

Yaribz commented 4 years ago

More generally, regarding the integration of the autohost battle management into lobby client GUI, here is the relevant part of my answer to PtaQ PM in Spring forums:

Concerning the ability to control SPADS through GUI there are two ways to do that: 1) you can simply implement buttons in your GUI which send commands in private to the autohost, like players would do (!ring, !start etc.). And then parse the answer sent by SPADS (like a player would read the response) to check if the command worked. This solution doesn't require any change in SPADS but it is not very clean though: ideally it requires hiding the answers sent by SPADS, but only when the command got triggered via the GUI... Also it requires parsing messages which are designed to be read by humans, so it can be tedious. And finally there is no guarantee these messages won't change in the future and break all the system... 2) A better way to implement that imo would be to implement a dedicated program-to-program interface, between SPADS and lobby clients. 2.1) This could be implemented through private messages starting with a specific tag (anything else than "!" which would be reserved for human interactions). Then it will be very easy for the lobby client to filter out these messages and SPADS could send answers which can be easily parsed and whose syntax won't change. 2.2) This could also be implemented through new dedicated lobby commands added in uberserver...

Basically:

(1)

(2.1)

(2.2)

Imho, a mix of (2.1) and (2.2), keeping backward compatibility, would be a great start: (2.2) for the vote system, which would really benefit from having a clean and generic integration in lobby protocol (2.1) for some other commonly used SPADS commands/functionalities

If (2.2) is implemented in a way that it breaks backward compatibility for lobby clients, then it might be the opportunity to clean up / make more generic other parts of the lobby protocol related to battle management (SETSCRIPTTAGS command, bitfields in BATTLESTATUS commands ?).

silentwings commented 4 years ago

You just need to filter all the battle lobby messages starting with "!" coming from the players and all the messages coming from the autohost in a separate (collapsable ?) tab/section dedicated to battle management.

Indeed, it can be done done via parsing the !xxx and host messages, but this is a bit hacky and I guess not reliable because SPADS plugins could change the commands/messages. I would rather add the few central bits of autohost interaction (above), which cause the most chat, into the lobby protocol, to encourage/support integration into clients GUIs.

So basically I favour your 2.2, and yes it needs a server compat flag, with entry to "new style" battlerooms denied to users with old clients. That way there is no need for SPADS or any lobby client to be compatible with both old & new. Which things were you thinking of as 2.1?

opportunity to clean up / make more generic other parts of the lobby protocol related to battle management (SETSCRIPTTAGS command, bitfields in BATTLESTATUS commands ?).

Agreed.

Yaribz commented 4 years ago

Indeed, it can be done done via parsing the !xxx and host messages, but this is a bit hacky and I guess not reliable because SPADS plugins could change the commands/messages. I would rather add the few central bits of autohost interaction (above), which cause the most chat, into the lobby protocol, to encourage/support integration into clients GUIs.

Well if you just want to separate autohost related messages from the other messages, you don't really need parsing. The content of the messages isn't relevant.

Basically the filtering rule implemented at lobby client side for battle lobby messages would just be: message from autohost --> "Battle management" tab/section. message starting with "!" --> also "Battle management" tab/section. all other messages --> "Battle chat" tab/section.

silentwings commented 4 years ago

The content does matter when you try to make a GUI for it, though.

Yaribz commented 4 years ago

The content does matter when you try to make a GUI for it, though.

Indeed, my first post is only related to the "hard to follow players chat problem". That's what I meant by "Concerning specifically the separation of the autohost and player chat (to avoid autohost related messages flooding the chat)". My second post is the one which is related to GUI integration.

silentwings commented 4 years ago

Ok - so assuming we go for 2.2 for some parts, is there anything you'd suggest to be included/removed from my list in https://github.com/spring/uberserver/issues/366#issuecomment-585627849

On 2.1, for what remains, I would suggest keeping the current interface and the ! but restricting suitable parts of it to only respond to PMs.

Yaribz commented 4 years ago

A quick answer because I don't have much time right now: for me only the vote system would really benefit from 2.2. For other functionalities, 2.1 would be just as good. But actually I need to take some time to think more about it.

Yaribz commented 4 years ago

Actually the more I think about it, the more I think introducing a few new generic commands in lobby protocol for battle data/event communications would be ideal. This would allow new usages such as autohost vote management using lobby protocol, and at the same time it would make it possible to replace/improve easily other specific parts of the protocol later (SETSCRIPTTAGS, BATTLESTATUS...).

silentwings commented 4 years ago

If you let me know which lobby/spads commands you think would be suitable for addition/redesign (plus https://github.com/spring/uberserver/issues/153) then I'll draft a protocol for them

Yaribz commented 4 years ago

Concerning the other commands which would benefit from being included in lobby client GUI, I mostly agree with what you proposed. I guess it's the most used ones: !ring, !status (when a game is in progress), !list (maps, presets...), and (mainly through vote): !spec, !kick, !kickBan, !preset ...

However, for the lobby protocol evolution I was thinking about, there is no need to have a fixed list of commands before implementing the protocol extensions. If the commands added to lobby protocol are generic enough, they could be used for any autohost command (and still be somehow limited/configurable at uberserver level to avoid chaotic lobby developments). I have also started writing a proposal draft to explain better what I have in mind...

silentwings commented 4 years ago

Ok, I'll wait and see what you come up with.

Fwiw, I would rather that protocol commands had names that matched their individual purposes (e.g. no generic AUTOHOSTCOMMAND or suchlike), so that the basic structure of autohosting is hardcoded into the protocol.

Yaribz commented 4 years ago

But what about "AUTOHOSTCOMMAND command data" where the "command" parameter has to be in a whitelist in uberserver code or configuration file? That way you have the best of both worlds: you don't need to specify new commands at lobby protocol level to support new autohost functionalities, and at the same time you keep control of the basic structure of autohosting because new commands would need to get approved to be whitelisted? ("AUTOHOSTCOMMAND" is not really one of the commands I had in mind, I just use your example to discuss the idea of generic command with restricted parameters at uberserver level)

Yaribz commented 4 years ago

Here is a part of the protocol updates I have in mind: part 1

silentwings commented 4 years ago

Looks good so far, happy to implement that.

In case useful, I am not so familiar with this part of the protocol but I once sketched something similar in https://github.com/spring/uberserver/issues/58#issuecomment-427741044 (I am not suggesting to implement that ticket).

But what about "AUTOHOSTCOMMAND command data" where the "command" parameter has to be in a whitelist

It would be OK in a purely logical sense, but it would be inconvenient from the point of view of command stats / protocol documentation / server side implementation.

Yaribz commented 4 years ago

Looks good so far, happy to implement that.

Nice, FYI I added a section at the end about extensibility and backward compatibility. I guess it might be useful to have the opinions of lobby developers also.

In case useful, I am not so familiar with this part of the protocol but I once sketched something similar in #58 (comment) (I am not suggesting to implement that ticket).

Yeah, I plan to deal with these commands and more globally all the battle users / AI bots related commands in the next part of the proposal.

Basically here is what I planned for the proposal:

It would be OK in a purely logical sense, but it would be inconvenient from the point of view of command stats / protocol documentation / server side implementation.

Mmm, I guess some of the remaining parts of the proposal will have to be adapted then.

silentwings commented 4 years ago

Regarding backwards compatibility, it makes sense & most likely works. I'll think a bit longer about forwards compat & customizability.