Skip to content

feat(kvp-client): add client asynchronous kvp natives #3316

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

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

Conversation

outsider31000
Copy link
Contributor

Goal of this PR

just like the server we add client asynchronous natives.
using async natives reduces the load on the client as it's not always needed to wait for the operations to finish

How is this PR achieving the goal

the goal is to have it stream lined with the server natives that already have async natives

This PR applies to the following area(s)

FiveM/RedM

Successfully tested on

Game builds: ..

Platforms: Windows, Linux
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

@github-actions github-actions bot added the triage Needs a preliminary assessment to determine the urgency and required action label Apr 5, 2025
@outsider31000
Copy link
Contributor Author

outsider31000 commented Apr 5, 2025

@prikolium-cfx what you think how we should proceed ?
my changes would not be breaking scripts, I think everyone assumes the delete native on client is sync due to the name , when in the server is async and the docs is declared as shared with no mention of the client native being async despite the name.

So my question is:
Do we fix it with my changes (to correct it and avoid miss interpretation and keept it stream lined with the server) or leave it as it was ?
if you could give some advice on this would be appreciated.
thanks

@RealStonerGamer
Copy link

RealStonerGamer commented Apr 5, 2025

yeah I always thought the DeleteResourceKvp on the client was sync lol

@AvarianKnight
Copy link
Contributor

It should also be noted changing this from being async -> sync will also have performance implications since the sync versions are notably slower, and people who used this not thinking it had any performance implications, will suddenly have to deal with the overhead of it being synchronous.

@outsider31000
Copy link
Contributor Author

People who use this don't even know it's async in the first place, don't think a few ms per call would be more important to "keep" than correcting the mess and allow it to use it properly aligned with server natives and with no miss interpretations.

But that's just my opinion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
triage Needs a preliminary assessment to determine the urgency and required action
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants