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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

workaround for userid in player_connect #537

Merged
merged 2 commits into from May 16, 2024

Conversation

BestAwperEver
Copy link
Contributor

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.

@akiver
Copy link
Collaborator

akiver commented May 13, 2024

I think the root issue is how we parse stringtables since the version 4.1.0.
It's the same problem reported in #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
Copy link
Collaborator

akiver commented May 13, 2024

Just noticed that the change described in my previous comment may cause corrupted CMsgPlayerInfo messages as seen in #532 FYI @markus-wa

Edit: same for #525, it may corrupt parsing when players connect/disconnect

@markus-wa
Copy link
Owner

@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

@akiver
Copy link
Collaborator

akiver commented May 16, 2024

PR opened #540

@akiver
Copy link
Collaborator

akiver commented May 16, 2024

It looks like this PR is still required to fix #533 as this issue is not related to stringtables parsing.
Thanks @BestAwperEver!

@akiver akiver self-requested a review May 16, 2024 19:04
@akiver akiver force-pushed the playerConnect-set-raw-player branch from c1db754 to 919a289 Compare May 16, 2024 19:05
@akiver akiver merged commit 11dd3de into markus-wa:master May 16, 2024
3 checks passed
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

Successfully merging this pull request may close these issues.

None yet

3 participants