demoinfocs-golang icon indicating copy to clipboard operation
demoinfocs-golang copied to clipboard

workaround for `userid` in `player_connect`

Open BestAwperEver opened this issue 1 year ago • 2 comments
trafficstars

Since the last update, I've been encountering many demos in which after a player has disconnected, and then reconnected, they wouldn't be registered in the rawPlayers field. Thus, in many cases, there were nil players in some events (e. g. all grenade events).

I investigated this a little and found out that seemingly the only place we could get information to add the player back to the rawPlayers (at least in particular demos I've been using) is inside the player_connect handler, which was disabled for s2 demos in #499. I'm assuming this was done because the user ids for players have been changed in the February, 6 update from being in the lower digits to 652xx.

I'm not sure what exactly has been changed in the last update (my guess is the userinfo string table is not parsed as often or not at all in some situations anymore?), but I've implemented a workaround that allows us to use the player_connect again by noticing that the old used ids are still present in the new ones -- they are the least significant byte of the two, and the other one is just set to 255.

Here's an example demo. On tick number 48430 kefu_. disconnects, then at tick 50731 we get a player_connect event, and then at tick 51237, when the m_iConnected is updated, PlayerConnect is dispatched (but, notably, we don't know the new user id at this point so the rawPlayers field doesn't get updated). This leads to Thrower being nil in the dispatched HeExplode event on tick 53754, which breaks at least my application 😄

This PR is more of an attempt to get some attention to the problem. There probably is a proper way to handle the situation without resorting to this hacky method, which might or might not work in the future. But I've already tested this implementation (and by "tested" I mean I've been using it in the production since yesterday) and was able to lower the error count in the parsed matches tenfold, so maybe that would help someone else in a similar situation.

BestAwperEver avatar May 07 '24 07:05 BestAwperEver

I think the root issue is how we parse stringtables since the version 4.1.0. It's the same problem reported in https://github.com/markus-wa/demoinfocs-golang/issues/522 where many events now contain nil players.

I tried your demo against this branch https://github.com/markus-wa/demoinfocs-golang/tree/csdm which is v4.0.5 + updated proto messages and the thrower at tick 51237 is not nil.

If that can help it seems that the diff related to this issue is here. We used to read the bit that indicates if there is a value even if there is no key but since v4.1.0 (for POV support) we read the value bit only if there is a key. Reverting this changes fix the problem.

akiver avatar May 13 '24 20:05 akiver

Just noticed that the change described in my previous comment may cause corrupted CMsgPlayerInfo messages as seen in https://github.com/markus-wa/demoinfocs-golang/issues/532 FYI @markus-wa

Edit: same for https://github.com/markus-wa/demoinfocs-golang/issues/525, it may corrupt parsing when players connect/disconnect

akiver avatar May 13 '24 20:05 akiver

@akiver would you be able to propose an MR? I'm not really sure what the best approach is since a few people rely on the POV features right now

markus-wa avatar May 16 '24 09:05 markus-wa

PR opened https://github.com/markus-wa/demoinfocs-golang/pull/540

akiver avatar May 16 '24 18:05 akiver

It looks like this PR is still required to fix https://github.com/markus-wa/demoinfocs-golang/issues/533 as this issue is not related to stringtables parsing. Thanks @BestAwperEver!

akiver avatar May 16 '24 19:05 akiver