Skip to content

feat(state)!: add compression dictionary for RedM #3372

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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

feat(state)!: add compression dictionary for RedM #3372

wants to merge 1 commit into from

Conversation

AvarianKnight
Copy link
Contributor

Goal of this PR

Having a compression dictionary for RedM reduces the net traffic from 18-30kb/s to 16-24kb/s in testing, depending on the amount of players in the area this could result in a lot less data sent over svNetwork

This is protocol breaking since our compression dict will no longer be the same one used for FiveM, this adds a protocol version file that can be shared between server/client to the bump easier if its ever done again

This PR applies to the following area(s)

RedM

Successfully tested on

Game builds: ..

Platforms: Windows, Linux

Checklist

  • Code compiles and has been tested successfully.
  • Code explains itself well and/or is documented.
  • My commit message explains what the changes do and what they are for.
  • No extra compilation warnings are added by these changes.

@github-actions github-actions bot added the invalid Requires changes before it's considered valid and can be (re)triaged label Apr 29, 2025
@AvarianKnight AvarianKnight force-pushed the feat/rdr/compression-dict branch from 6a9a365 to 5e95c6a Compare April 29, 2025 19:40
@github-actions github-actions bot added RedM Issues/PRs related to RedM triage Needs a preliminary assessment to determine the urgency and required action and removed invalid Requires changes before it's considered valid and can be (re)triaged labels Apr 29, 2025
@prikolium-cfx prikolium-cfx added manual-review PRs that need manual review and work before merging. needs manual verification PRs that need manual verification by testing the change locally labels Apr 30, 2025
@AvarianKnight
Copy link
Contributor Author

For an explanation on how this was generated:

There's a function in CloneManagerLocal::SendUpdates to write chunks to a file, you can enable by setting #define WRITE_BYTEHUNK true (you'll have to modify the relative path to data/cache since this was used before the cache file was moved)

Afterwards just play in the game for a little bit and it'll write all the update packets it sends out and you can use zstd --train -r dirname outfile --maxdict=64KB to generate the raw dictionary file

After that you can use this python script to convert the dictionary file to a header file of bytes

convert_dict_to_header.zip

FabianTerhorst
FabianTerhorst reviewed Apr 30, 2025
View reviewed changes
code/components/citizen-server-impl/src/InitConnectMethod.cpp
@@ -564,7 +565,7 @@ static InitFunction initFunction([]()
auto gameBuildField = (gameBuildIt != postMap.end()) ? gameBuildIt->second : "0";
auto gameName = (gameNameIt != postMap.end()) ? gameNameIt->second : "";

if (protocol < 12)
if (protocol < kNetworkProtocolVersion)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this breaking the compatibility when the current clients still send 12, because of the NETWORK_PROTOCOL macro?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I put the wrong protocol number here, will fix it when I get home this should be 12 and 13

@AvarianKnight
Copy link
Contributor Author

AvarianKnight commented Apr 30, 2025 •
edited
Loading

It should be noted that it would be best to merge this with the object id patch when it's finished (and train it on that patch too) since it is also protocol breaking, it would be better to break all at once than to do two separate breakages.

@prikolium-cfx prikolium-cfx added the has-dependencies Blocked by or requires another change, read PR description. label May 1, 2025
@AvarianKnight
feat(state): add compression dictionary for RedM
e92d2fa 
!! THIS IS A BREAKING CHANGE !!

Having a compression dictionary for RedM reduces the net traffic from 18-30kb/s to
16-24kb/s in testing

This is protocol breaking since our compression dict will no longer be the
same one used for FiveM, this adds a protocol version file that can be shared
between server/client to the bump easier if its ever done again
@AvarianKnight AvarianKnight force-pushed the feat/rdr/compression-dict branch from 5e95c6a to e92d2fa Compare May 1, 2025 22:44
FabianTerhorst
FabianTerhorst approved these changes May 2, 2025
View reviewed changes
Copy link
Contributor

@FabianTerhorst FabianTerhorst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes itself are looking good to me. But backward compatibility needs to be evaluated independently.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@FabianTerhorst FabianTerhorst FabianTerhorst approved these changes

Assignees
No one assigned
Labels
has-dependencies Blocked by or requires another change, read PR description. manual-review PRs that need manual review and work before merging. needs manual verification PRs that need manual verification by testing the change locally RedM Issues/PRs related to RedM triage Needs a preliminary assessment to determine the urgency and required action
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@AvarianKnight @FabianTerhorst @prikolium-cfx