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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refine net time sync code for OneSync #2381

Merged
merged 1 commit into from
Mar 5, 2024

Conversation

Disquse
Copy link
Contributor

@Disquse Disquse commented Feb 9, 2024

Goal of this PR

The goal of this PR is to improve the readability of rage::netTimeSync related code. Additionally, it aims to improve the maintainability of CloneExperiments.cpp by logically splitting certain parts of the file into appropriate chunks. The code refine isn't intended to be a full fledged refactor but rather a first step towards facilitating future refactoring efforts.

I invite everyone to participate in reviewing and suggesting improvements. Please remember that on the current stage I'm not planning to make it more complex than it is right now (i.e. adding abstraction layers, reworking the inner or depended code, etc).

How is this PR achieving the goal

By moving the related code from CloneExperiments.cpp to separate .cpp and .h files, and making some minor tweaks.

This PR applies to the following area(s)

FiveM, RedM

Successfully tested on

Game builds:
FiveM: 1604, 2060, 2189, 2372. 2545, 2612, 2699, 2802, 2944, 3095
RedM: 1311, 1355, 1436, 1491

Platforms: Windows

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.

Fixes issues

-

@Disquse Disquse changed the title tweak(onesync): refine net time sync code Refine net time sync code for OneSync Feb 9, 2024
@Disquse Disquse added the onesync label Feb 9, 2024
@gottfriedleibniz
Copy link
Contributor

gottfriedleibniz commented Feb 15, 2024

Besides a few small nitpicks to make myself feel better, I won't look too hard on this as its mostly moving code around.

Does our style guideline include rules on names/ordering/grouping of includes? How netTimeSync is currently structured is a bit unappealing to the eye.

@gottfriedleibniz
Copy link
Contributor

One benefit here is that as we start refining/decoupling things - we'll figure out a nice convention that allows us to structure functionality a bit better (e.g., gamecode structure vs. hooking vs. onesync protocols, etc).

@github-actions github-actions bot added RedM Issues/PRs related to RedM triage Needs a preliminary assessment to determine the urgency and required action labels Feb 15, 2024
The goal of this commit is to improve the readability of `rage::netTimeSync` related code. Additionally, it aims to improve the maintainability of `CloneExperiments.cpp` by logically splitting certain parts of the file into appropriate chunks. The code refine isn't intended to be a full fledged refactor but rather a first step towards facilitating future refactoring efforts.
@Disquse Disquse force-pushed the shared/net-time-sync-refine branch from 5d710f7 to 751a76c Compare March 4, 2024 10:45
@gottfriedleibniz gottfriedleibniz added ready-to-merge This PR is enqueued for merging and removed triage Needs a preliminary assessment to determine the urgency and required action labels Mar 4, 2024
@thorium-cfx thorium-cfx merged commit 5a7de00 into citizenfx:master Mar 5, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
onesync ready-to-merge This PR is enqueued for merging RedM Issues/PRs related to RedM
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants