Skip to content

Conversation

@21Joakim
Copy link
Contributor

This PR is based on #563 but I have removed the client checks (as they are not necessary) which makes this easier to maintain.

I have tested and validated that everything works correctly on Linux, the core ReplicateConVar code is also identical to the implementation in CS2Fixes (https://github.com/Source2ZE/CS2Fixes/blob/e9dfd9a08b7158c313221019d34e8586a6396d5a/src/playermanager.cpp#L567-L580) and uses all the same features which UserMessage already relies on, no additional signatures or offsets.

I have tested this with sv_autobunnyhopping and sv_enablebunnyhopping using the plugin below to enable bhop on a per player basis and I have not encountered any issues, I tested this with a friend to ensure it was properly setting per player.

Here is a simple bhop example plugin https://gist.github.com/21Joakim/480f2bae588df54d2f34faba8b2861bc, simply use the bhop console command to toggle bhop on and off for your player only.

I did some additional tests to ensure it is safe to use

  • ✅ Passing a convar which is not replicated does not cause any issues
  • ✅ Passing an invalid convarname (i.e. a convar which does not exist) does not cause any issues
  • ✅ Passing an invalid convarvalue (e.g. a string for a bool value) for the provided convarname convar does not cause any issues
  • ✅ Replicating the convar for a bot does not cause any issues
  • ✅ Passing an invalid slot (i.e. a negative slot or a slot which does not have a player) does not cause any issues
  • ✅ Passing an empty value for either (or both) of convarname or convarvalue does not cause any issues
  • ✅ Creating a UserMessage hook for SetConvar works as expected, it sees the messages emitted by ReplicateConVar and there are no issues with sending the UserMessage from within the hook
  • ❌ Passing a null value for convarname or convarvalue will cause a segfault, this happens for other native functions as well (e.g. player.ExecuteClientCommand(null)) so probably more of a systemic problem if anything

I have been waiting for the original PR to be merged for almost 5 months so I am hoping that by cutting the feature down to the core and testing it thoroughly we can have this new PR merged quickly!

@21Joakim 21Joakim requested a review from roflmuffin as a code owner January 27, 2025 20:41
@KillStr3aK
Copy link
Contributor

LGTM

@roflmuffin roflmuffin enabled auto-merge (squash) January 27, 2025 22:03
@roflmuffin roflmuffin merged commit f72e6d5 into roflmuffin:main Jan 27, 2025
5 checks passed
Dliix66 pushed a commit to Dliix66/CounterStrikeSharp that referenced this pull request Feb 6, 2025
@Dliix66 Dliix66 mentioned this pull request Feb 8, 2025
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.

3 participants