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

future proof(?) shotgun checking logic #3653

Merged
merged 5 commits into from
Feb 17, 2024

Conversation

holysnipz
Copy link
Contributor

@holysnipz holysnipz commented Feb 17, 2024

no more hardcoded weapon hashes (even this is used by only 2 effects) 😅

from SHV's natives definition, seems old enough to not cause compatibility issues in earlier game versions
NATIVE_DECL Hash GET_WEAPONTYPE_GROUP(Hash weaponHash) { return invoke<Hash>(0xC3287EE3050FB74C, weaponHash); } // 0xC3287EE3050FB74C 0x5F2DE833 b323

@Rylxnd
Copy link
Contributor

Rylxnd commented Feb 17, 2024

Okay, how does this perform compared to the “old” method with the switch case? Probably worse considering the speed of switch compared with the extra overhead of calling a native function.

@ShufflePerson
Copy link

Okay, how does this perform compared to the “old” method with the switch case? Probably worse considering the speed of switch compared with the extra overhead of calling a native function.

Maybe adding caching could be the best option of both worlds?
First call with each ID will be slower, the rest will be as fast as it was with switch.

@holysnipz
Copy link
Contributor Author

holysnipz commented Feb 17, 2024

it was way slower, but still unnoticeable in normal gameplay. so i didn't cared much.

should be better now

@pongo1231
Copy link
Member

The overhead from the native call is very negligible, especially considering the already existing frequency of native calls by the mod. The additional optimization is appreciated regardless.

@pongo1231 pongo1231 merged commit 4bc9ab0 into gta-chaos-mod:master Feb 17, 2024
2 checks passed
@holysnipz holysnipz deleted the sg-checks branch February 18, 2024 00:32
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.

4 participants