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

feat(extra-natives/five): GET_VEHICLE_GEAR_RATIO and SET_VEHICLE_GEAR_RATIO #2409

Merged
merged 2 commits into from
May 28, 2024

Conversation

RickyB505
Copy link
Contributor

@RickyB505 RickyB505 commented Mar 2, 2024

Goal of this PR

These new natives will allow developers to create scripts to create custom gear ratios for vehicles.

How is this PR achieving the goal

I added the memory editing stuff to VehicleExtraNatives.cpp to allow me to create the 2 new natives.

The natives

float GetVehicleGearRatio(Vehicle vehicle, int gear);
void SetVehicleGearRatio(Vehicle vehicle, int gear, float ratio);

This PR applies to the following area(s)

FiveM, Vehicle, Lua, JS, C#

Successfully tested on

Game builds: 3095,2944,2802,2699,2612,2545,2372,2189,2060,1604

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 triage Needs a preliminary assessment to determine the urgency and required action label Mar 2, 2024
@thorium-cfx
Copy link
Contributor

Hey Ricky,

Thank you for the PR, seems like you got it working, good job! Some prechecks: your commit titles don't follow our format, see https://github.com/citizenfx/fivem/actions/runs/8118915693/job/22194013573?pr=2409#step:4:83 could you resolve these?

When you are at it can you also squash the commits? Guide: https://docs.fivem.net/docs/contributing/git/squash-guide/

@thorium-cfx thorium-cfx added invalid Requires changes before it's considered valid and can be (re)triaged and removed triage Needs a preliminary assessment to determine the urgency and required action labels Mar 5, 2024
@RickyB505 RickyB505 changed the title feat(extra-natives/five) GET_VEHICLE_GEAR_RATIO and SET_VEHICLE_GEAR_RATIO feat(extra-natives/five): GET_VEHICLE_GEAR_RATIO and SET_VEHICLE_GEAR_RATIO Mar 5, 2024
@github-actions github-actions bot added 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 Mar 5, 2024
@RickyB505
Copy link
Contributor Author

Hey Ricky,

Thank you for the PR, seems like you got it working, good job! Some prechecks: your commit titles don't follow our format, see https://github.com/citizenfx/fivem/actions/runs/8118915693/job/22194013573?pr=2409#step:4:83 could you resolve these?

When you are at it can you also squash the commits? Guide: https://docs.fivem.net/docs/contributing/git/squash-guide/

should be all good now just need to let the thing run

ext/native-decls/GetVehicleGearRatio.md Show resolved Hide resolved
ext/native-decls/GetVehicleGearRatio.md Show resolved Hide resolved
ext/native-decls/GetVehicleGearRatio.md Outdated Show resolved Hide resolved
ext/native-decls/SetVehicleGearRatio.md Show resolved Hide resolved
ext/native-decls/SetVehicleGearRatio.md Show resolved Hide resolved
ext/native-decls/SetVehicleGearRatio.md Outdated Show resolved Hide resolved
@gottfriedleibniz
Copy link
Contributor

gottfriedleibniz commented Mar 6, 2024

Should we handle or document MODIFY_VEHICLE_TOP_SPEED, SET_VEHICLE_MOD, REMOVE_VEHICLE_MOD (VMT_GEARBOX) potentially recalculating this gear ratio array and overwriting changes made by SET_VEHICLE_GEAR_RATIO? Additionally b3095 introduced 0x337EF33DA3DDB990 which also repopulates the array while allowing different constants, e.g., 0.9 vs. 0.8 and 1.1 vs 1.03 in its calculations (native is tentatively named _SET_TRANSMISSION_REDUCED_GEAR_RATIO).

@github-actions github-actions bot added invalid Requires changes before it's considered valid and can be (re)triaged and removed triage Needs a preliminary assessment to determine the urgency and required action labels Mar 6, 2024
@RickyB505
Copy link
Contributor Author

Should we handle or document MODIFY_VEHICLE_TOP_SPEED, SET_VEHICLE_MOD, REMOVE_VEHICLE_MOD (VMT_GEARBOX) potentially recalculating this gear ratio array and overwriting changes made by SET_VEHICLE_GEAR_RATIO? Additionally b3095 introduced 0x337EF33DA3DDB990 which also repopulates the array while allowing different constants, e.g., 0.9 vs. 0.8 and 1.1 vs 1.03 in its calculations (native is tentatively named _SET_TRANSMISSION_REDUCED_GEAR_RATIO).

possibly might want to add to the corresponding decals that they might interfere with each other

@github-actions github-actions bot added 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 Mar 6, 2024
@RickyB505 RickyB505 requested a review from Disquse March 8, 2024 03:07
@RickyB505
Copy link
Contributor Author

is there anything else i need to change about this?

Copy link
Contributor

@Disquse Disquse left a comment

Choose a reason for hiding this comment

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

A few more small changes and it's good to go!

ext/native-decls/SetVehicleGearRatio.md Show resolved Hide resolved
ext/native-decls/GetVehicleGearRatio.md Show resolved Hide resolved
@RickyB505
Copy link
Contributor Author

@Disquse there you go all done lmk if you notice anything else that needs changing

@RickyB505 RickyB505 requested a review from Disquse March 13, 2024 12:23
@RickyB505
Copy link
Contributor Author

any updates for changes needed?

@RickyB505
Copy link
Contributor Author

i assume this is ready to merge then?

@Disquse Disquse self-assigned this Apr 23, 2024
@Disquse
Copy link
Contributor

Disquse commented May 27, 2024

Tested this code in-game, works great. Thanks for your contribution, this is a nice feature to have. Apologies for the very slow review process, I was busy with a lot of other stuff. I'm attaching a debug script I made to test this feature, just in case anyone is interested in testing it out before there are any proper resources: https://pastebin.com/yDYj0sqA

@Disquse Disquse 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 May 27, 2024
@prikolium-cfx prikolium-cfx merged commit fce24af into citizenfx:master May 28, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge This PR is enqueued for merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants