Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

autohosts ignore FORCEJOINBATTLEFAILED #249

Closed
abma opened this issue Nov 23, 2014 · 29 comments
Closed

autohosts ignore FORCEJOINBATTLEFAILED #249

abma opened this issue Nov 23, 2014 · 29 comments

Comments

@abma
Copy link

abma commented Nov 23, 2014

they just randomly kick lobbies from battles depending on cpuid.
spring/uberserver#121

@Licho1
Copy link
Member

Licho1 commented Nov 23, 2014

Does FORCEJOINBATTLEFAILED work in all cases? Even in those where lobby decide to ignore command or fail due to being joined in other game?
Anyway even if it does, we need this information BEFORE we try to move people to know who to kick from queue. Knowing in retrospect that it failed is not useful...

@Licho1 Licho1 closed this as completed Nov 23, 2014
@abma
Copy link
Author

abma commented Nov 23, 2014

Does FORCEJOINBATTLEFAILED work in all cases? Even in those where lobby decide to ignore command or fail due to being joined in other game?

no. but clients already should get an error message when they try to join a battle but are already in a battle. FORCEJOINBATTLEFAILED works for clients which doesn't support FORCEJOINBATTLE.

it makes me sad that you even didn't investigate how FORCEJOINBATTLEFAILED works, its very few code: https://github.com/spring/uberserver/blob/master/protocol/Protocol.py#L1618 (and some more lines)

also if i read lobby dev meeting minutes right, it was your proposal.

@Licho1
Copy link
Member

Licho1 commented Nov 23, 2014

I read how it works, my question was rhetorical... My point was it's not reliable and even if it was its too late to learn after move attempt... For our scenario we need to know beforehand.. atm the only way is to rely on silly cpu and known list of complying lobbies..
For example SL does implement it but clients are not moved from queue to actual battle. Yes its a bug in SL but harm is done to all uses, not just SL, if they are kept in queue.

@abma
Copy link
Author

abma commented Nov 23, 2014

wrong, it leaves battle before joining:
https://github.com/springlobby/springlobby/blob/master/src/serverevents.cpp#L979

idk which client you are refering, without details... WHY DO I ALWAYS HAVE TO ASK YOU FOR DETAILS?

@Licho1
Copy link
Member

Licho1 commented Nov 23, 2014

Code looks fine, however people really were not moved .. if they didn't lie to me about lobby.
Also person with NOTAlobby wasnt moved.

@abma
Copy link
Author

abma commented Nov 23, 2014

lobby version? which user(s)?

@Licho1
Copy link
Member

Licho1 commented Nov 23, 2014

Can't remember users and no idea about version of their lobbies.. SL has identical code from february though..
GoogleFrog hammered me to block other lobbies because people were getting angry at failed "2v2" battles with 1v2 setup and one person left in queue...

@abma
Copy link
Author

abma commented Nov 23, 2014

this is not useful...

@Licho1
Copy link
Member

Licho1 commented Nov 23, 2014

Well there is simple server-side solution to all this..
in the meanwhile i dont know what to do except to ask people with those lobbies to test.. they can self test by joining springie and use !votemove

@abma
Copy link
Author

abma commented Nov 23, 2014

with springlobby 0.201 it works...

@Licho1
Copy link
Member

Licho1 commented Nov 23, 2014

Ok i will unban that if it uses cpu id now..

@cleanrock
Copy link
Contributor

I am implementing matchmaking support (FORCEJOINBATTLE) in flobby now.
I don't like the idea of using CPU id for this, i guess you require lobby to send 6667, correct?
How about instead improving the ADDUSER message?
Perhaps we can add this info at end, e.g. a new optional bitmap (uint32):
ADDUSER userName country cpu [accountID] [lobbyInfo]
This should be ok since the lobby will know if accountID is there depending on if it sent 'a' in LOGIN.

@cleanrock
Copy link
Contributor

Btw, lobbyInfo can probably be mandatory without breaking lobbies (current flobby would be fine and just ignore it):
ADDUSER userName country cpu [accountID] lobbyInfo

@cleanrock
Copy link
Contributor

The new field could also be the compFlags:
ADDUSER userName country cpu [accountID] {compFlags}
e.g. "cl sp p m"

@db81
Copy link
Contributor

db81 commented Dec 21, 2014

@cleanrock i guess you require lobby to send 6667, correct?

No, you keep using 4607052, 4607063, 4607053 and tell @Licho1 that you have implemented FORCEJOINBATTLE so that he can add those IDs to the whitelist.

@cleanrock
Copy link
Contributor

@db81, you can't know if all flobby clients support matchmaking (rebuilt with recent enough source).
We need something better than these CPU hacks to detect if matchmaking is supported.
Btw, i just pushed flobby support for this: cleanrock/flobby@f10181f .

@cleanrock
Copy link
Contributor

On the other hand if we don't want to change ADDUSER i could change the flobby CPU so Springie know if a particular flobby client is ok.

@Licho1
Copy link
Member

Licho1 commented Dec 21, 2014

feel free to modify protocol. Otherwise just invent some cpu number and let me know which cpu is it

@abma
Copy link
Author

abma commented Dec 21, 2014

https://github.com/spring/uberserver/blob/master/protocol/Protocol.py#L1813

(nobody read the initial bug report it seems)

@abma
Copy link
Author

abma commented Dec 21, 2014

i suggest to use zero-k lobby cpu id if you want it working with an other lobby.

@Licho1
Copy link
Member

Licho1 commented Dec 21, 2014

its too late when it reports after you try the command... thats why cpu id is still used

@cleanrock
Copy link
Contributor

After some thinking about this i wan't to go with the simple solution of db81 by just adding CPU ids 4607052, 4607063 and 4607053 as lobbies supporting matchmaking.
Please add the above so i can test flobby in the matchmaking rooms.
When i know it works i will inform the few flobby users in spring forum to make sure they update flobby (i am pretty sure flobby users update fairly regularly anyway).

@cleanrock
Copy link
Contributor

@abma,
I guess your disinterest in improving ADDUSER is because you don't see why FORCEJOINBATTLEFAILED is too late (i think you mentioned this a few times now), this may explain things:

  1. players join a match making battle room
  2. when the battle host thinks it got enough players for a good and balanced game it will move all (or some) players to a new battle room with FORCEJOINBATTLE and start the game

If some players are not moved in step 2 when uberserver sends FORCEJOINBATTLEFAILED the game is broken and the matchmaking room is useless.
Anarchid explanation to me: http://zero-k.info/Forum/Post/110708#110708

Btw, I am happy with flobby CPU ids being added, i don't think we must improve ADDUSER for matchmaking.

@cleanrock
Copy link
Contributor

Realized i don't need access to Queue rooms to test .. I just tested !votemove with flobby and it worked well so i will remind people to update.
Please add the CPU ids soon.

@abma
Copy link
Author

abma commented Dec 22, 2014

regarding "too late": there are always reasons why FORCEJOINBATTLE could fail: for example battle could be already full, client disconnected, battle doesn't exist, missing/wrong password, etcetc. the "matchmaking system" atm doesn't handle any error it seems. also there is still no clean feedback about which lobbies fails. for springlobby this command is implemented since ages, it wasn't touched by me and current version works and it still is getting blamed for not working but the "not working" feedback is way to inaccurate as it works for me.

also there are to many threads/places where this is discussed, this report here is about FORCEJOINBATTLEFAILED isn't used/implemented on the autohost (!!!)

for discussing this thread should be used:
http://springrts.com/phpbb/viewtopic.php?f=64&t=32800

i miss some feedback there.

@cleanrock
Copy link
Contributor

I think you are talking about unusual cases, matchmaking hosts needs to know up-front if a client supports matchmaking (which was the purpose of my proposal to add info in ADDUSER).
Anyway, i think all will be fine if allowed lobbies fully supports FORCEJOINBATTLE, let zk devs post bug reports if they see problems.

@abma
Copy link
Author

abma commented Dec 22, 2014

let zk devs post bug reports if they see problems.

i don't see that happen (without FORCEJOINBATTLEFAILED beeing implemented).

still off topic, please use http://springrts.com/phpbb/viewtopic.php?f=64&t=32800

@Licho1
Copy link
Member

Licho1 commented Dec 22, 2014

Abma it does create a brand new game and moves people there to be insta started (which is a proxy for missing "connect") .. there wont be any other "errors"...

@abma
Copy link
Author

abma commented Jan 4, 2015

i knew, still the same applies. FORCEJOINBATTLEFAILED should be implemented.

Licho1 added a commit that referenced this issue Jan 4, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants