search

ZeroK-RTS / Zero-K-Infrastructure

Website, lobby launcher and server, steam deployment, .NET based tools and other vital parts of Zero-K infrastructure
GNU General Public License v3.0
53 stars 52 forks source link

Clamp voter count to the room maxPlayers #2815

Closed Legomenon-gh closed 2 years ago

Legomenon-gh commented 3 years ago

This is a bandaid fix for waiting list players being included in the quorum for polls. A full fix would require also preventing waiting list players from starting or participating in polls.

Partly addresses https://github.com/ZeroK-RTS/Zero-K-Infrastructure/issues/2780

Legomenon-gh commented 3 years ago

I'd like to know if I'm correct on the behavior of the waiting list before I dig too much deeper, or if I missed something and it's easier than I think it is.

If my understanding is correct, is it worth having a full fix, and do you have any pointers to the safest/easiest way to deal with the waiting list in polls?

sprunk commented 3 years ago

I'd like to know if I'm correct on the behavior of the waiting list

The comment on your PR says "The lobby is responsible for determining who gets to play" but actually the server does: https://github.com/ZeroK-RTS/Zero-K-Infrastructure/blob/26698dadedc96bc2515ad15a00bef9b26c24cbb8/ZkLobbyServer/ServerBattle.cs#L535-L538

It's still not obvious on who should vote because the queue order is set once when you join the room and doesn't reset when you spec/unspec. Consider this situation:

Legomenon-gh commented 3 years ago

Indeed, this part of the comment is misleading, I've updated it and clarified that this does not affect ingame polls. Feel free to reword it further for clarity too.

(I don't think it would be too arduous to filter out voters by whether they will get to play in the current state of affairs, but it's quite a bit of extra code that shouldn't be hit very often, so curious if it's worth doing)

sprunk commented 3 years ago

Feel free to, it would prevent some cases that are clearly bad (for example in a 1v1 you shouldn't be able to decide the map / modoptions and then start just because you brought a homie). This PR is already a standalone improvement though so no hurry.

Legomenon-gh commented 3 years ago

Yeah I'm probably not filtering out wait list players from polls until/unless there's examples of users gaming that system, since it's relatively painful to test locally and ensure that the fix is very low risk (i.e. I don't want to break the teams all welcome room in a way I couldn't replicate in dev). Odds are there will be a change to the wait list feature one day and it can be handled then.

(An uglier but safer alternative is to have Chobby block poll related comments from wait list users too)

sprunk commented 3 years ago

Given that you can vote "as a spec" anyway (unspec, cast a vote, spec back) and people are using that power fairly responsibly it's probably fine to let the waiting list vote for now.