serverstf / python-valve

A Python Interface to various Valve products and services
Other
235 stars 71 forks source link

player_count incorrect and blank player names #42

Open danielunderwood opened 7 years ago

danielunderwood commented 7 years ago

I'm querying a CSGO server and the returned player_count is incorrect through both get_info() and get_players(). get_players()['players'] gets the players in the server, but also gets a number of players with blank names. If these blank names are removed from player_count, it's correct. Any idea of what's going on here?

danielunderwood commented 7 years ago

Well this is weird. The problem seems to be gone now during the times the server isn't as active. Perhaps some players were left in the query after leaving the server or something? They didn't show up with a status in the console while in the server though.

thel3l commented 7 years ago

@danielunderwood It appears that the original dev abandoned this project. I'm maintaining a fork and merging PRs and squashing bugs over at https://github.com/thel3l/python-valve. Could you please provide some more data, preferably at the other fork? Thanks :)

danielunderwood commented 7 years ago

@thel3l I'll switch to your fork and open an issue there if I encounter the issue again. The tricky thing is that I haven't been able to reproduce the issue so far. It's only happened once besides the time that I reported it.

thel3l commented 7 years ago

@danielunderwood awesome, thanks 😃

Holiverh commented 7 years ago

I must say I'm not too impressed with my own "maintenance" of python-valve. Recent changes in circumstances for me should free up some time. Hence I've been looking at investing a bit of that time back into python-valve recently. Specifically, I've been trying to get a bearing on the development that's gone on in the various forks and trying to see what can be merged back in.

@thel3l I must say I'm deeply appreciative of your voluntarily continuing the work I started. I'm yet to have a close look at it yet, but would you say that your fork is suitable for merging -- and, more importantly, would you be happy for me to do that? I could understand if you preferred, perhaps, to maintain your fork independently.

Either way, :+1: for the care you and others have shown this sorely neglected library!

@danielunderwood So ... I've always been acutely aware of the "empty player name" thing and how that can cause/relate to player counts being misleading. In fact, in a secondary project that depends on python-valve, I had an explicit work around for it. Would you say the assertion in the linked comment is consistent with the behaviour you're seeing? E.g., newly connected users show up blank. Note, I had mostly been observing that behaviour with TF2 servers circa 2015.

If that is the same sorta thing you're getting, then my personal instinct would be to say that it's not strictly a bug in python-valve. As long as the behaviour isn't exhibited due to a flaw in the way python-valve implements the A2S protocol.

My initial aim was for python-valve (at least the A2S bits referred to here) to be a relatively low-level interface for Source servers. An interface upon which higher abstractions could be built. Therefore, what the server returns is what the server returns. Python-valve isn't necessarily in a position to be too opinionated about that -- at least not at the valve.source.a2s level.

thel3l commented 7 years ago

@Holiverh - Yes! I would love to merge my fork back with the main project and I'll open a PR soon. Thanks for all the work that you've done, it's absolutely fantastic 😃

danielunderwood commented 7 years ago

I'm not so sure that what I was seeing was due to newly connected players not showing up, although it certainly was happening when the server was more active and people were joining. From what I could tell, all the people that were actually in the server were showing up in get_players()['players'], but there were blank entries in addition to them.

The closest thing that I thought of were maybe players were leaving the server and their names were being removed, but were somehow still represented in the list of players. I would have tried to dig further, but the problem seems to be pretty rare. I'm considering throwing the data in a database for my project, which may give some info as to what's going on. For now, I'm just throwing away any players where the name is blank, which seems to produce the correct player names at least. I could probably take the length of that to correct the player number too, but haven't gotten to that point yet.

Holiverh commented 7 years ago

So I've added various notices and a workaround about this in the docs with https://github.com/Holiverh/python-valve/commit/6a699bb673c09587c34836c755e0105999d4eb53.

Note sure what to do with it other than that though. Because, as I said, if a server is returning silly things then what position am I in to question that. Unless of course its an actual bug in python-valve that is; so I welcome any efforts you put into continuing to debug it -- hence I'm leaving this issue open.

Alternately, I'm tempted to look at extending PlayersResponse so that can optionally do the filtering out of the box as well. That satisfies my desire to continue to expose the raw values returned by the server, but it'd also provide a more sensible API for 99% of use cases. Not sure what said PlayersResponse changes would look like at this time though.

danielunderwood commented 7 years ago

I'll take a look at the a2s decoding code when I get a chance to see if I can find anything that may be wrong. I suppose there's a chance the a2s format could have changed, but I'd imagine that they'd have some type of version built in for backwards compatibility.

I found the following bug trackers for source and didn't see an issue like this in either: https://developer.valvesoftware.com/wiki/Source_Bug_Reports https://github.com/ValveSoftware/Source-1-Games

Yepoleb commented 6 years ago

I just noticed this while trying to reproduce #55. It happens very inconsistently. The message still parses correctly, so I'm pretty sure the server is sending garbage here. I think we should try to match the steam behavior, which is to ignore the empty players. I'll do some more testing tomorrow though to be sure about this.

Edit: The entries have a valid-looking but very short playtime. After some time the player name appears and continues to appear in every following response. I'm assuming these entries are players currently connecting to the server where the name is not yet known, but they still show up in the player count, so others know the spot is reserved.

0r3h commented 6 years ago

I've encountered a problem with player_count for Squad server yesterday. Usually number of blank names varies a bit but clears itself up somehow. Past few days it kept growing and growind and I ended up with constant 231 blank names in responses (server emptied meanwhile), meaning there was no more place for all actual (80 max) players in limited response of 255 players. Is that 255 response limit part of a2s protocol? Is there a way to circumvent it?

Yepoleb commented 6 years ago

Can you give us more details like server address or at least game name?

0r3h commented 6 years ago

Game is Squad, server IP is 79.136.73.35 and query port 27100. Server has since been restarted (12h ago). I'm not yet sure if blank names are accumulating again. It might be a buggy game implementation, as it's still in development and that particular server is running developer builds. Increase in blank names can be seen in this player count graph over a few days: https://cdn.discordapp.com/attachments/433594510493089792/454946689917255692/Screenshot_55.png (server filled up to the full each day, but because of increasing number of blank names, count was wrong).

Yepoleb commented 6 years ago

I just tested with the steam server browser, the server does send the names somehow but not in a format the client understands. I may look into it at some point but can't make any promises as this is clearly an implementation error on the game's side.