fix(gta-streaming-five): sanitize ragdoll component based in attach bone

[DaniGP17]

Goal of this PR

Validate that the component obtained from the attachment index is not -1, which represents an invalid position in the component array.

There are some peds that don't have the same attachment bones as the common ones and when the ragdoll is activated with a weapon it causes it to crash near people.

How is this PR achieving the goal

Patching a part of the CPedWeaponManager::SwitchToRagdoll function to validate that the r13 register representing the component value is not -1.

This PR applies to the following area(s)

FiveM

Successfully tested on

Game builds: 1, 1604, 3407

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

fixes #3185

[iridium-cfx]

The index is used a few times before this in the same function.
Would it be better to just return from the function if it's -1, right after getting the index?

[DaniGP17]

The index is used a few times before this in the same function. Would it be better to just return from the function if it's -1, right after getting the index?

I assume you mean jumping directly here, right?

Is it better to calculate that position with an offset from the current pattern or to get a new pattern for that location?

[iridium-cfx]

The index is used a few times before this in the same function. Would it be better to just return from the function if it's -1, right after getting the index?

I assume you mean jumping directly here, right? Is it better to calculate that position with an offset from the current pattern or to get a new pattern for that location?

Yeah, and uh, it's quite a large function, so maybe sig scan with an assert that the distance is vaguely correct.

[DaniGP17]

Yeah, and uh, it's quite a large function, so maybe sig scan with an assert that the distance is vaguely correct.

This is the code looking for the pattern of the fail location:

auto location = hook::get_pattern("48 8B 91 ? ? ? ? 48 8D 8D");
hook::nop(location, 7);
const auto successPtr = reinterpret_cast<intptr_t>(location) + 7;
const auto failPtr = reinterpret_cast<intptr_t>(hook::get_pattern("4C 8D 85 ? ? ? ? E8 ? ? ? ? 4C 8D 9C 24 ? ? ? ? 49 8B 5B ? 41 0F 28 73", 12));
trace("CPedWeaponManager::SwitchToRagdoll: %lld\n", static_cast<long long>(failPtr - reinterpret_cast<intptr_t>(location))); // 1375
patchStub.Init(successPtr, failPtr);
hook::jump(location, patchStub.GetCode());

But I didn't understand the assert, do you mean something like this?

assert((failPtr - reinterpret_cast<intptr_t>(location)) < 2000);

[iridium-cfx]

Yeah, and uh, it's quite a large function, so maybe sig scan with an assert that the distance is vaguely correct.

This is the code looking for the pattern of the fail location:

auto location = hook::get_pattern("48 8B 91 ? ? ? ? 48 8D 8D");
hook::nop(location, 7);
const auto successPtr = reinterpret_cast<intptr_t>(location) + 7;
const auto failPtr = reinterpret_cast<intptr_t>(hook::get_pattern("4C 8D 85 ? ? ? ? E8 ? ? ? ? 4C 8D 9C 24 ? ? ? ? 49 8B 5B ? 41 0F 28 73", 12));
trace("CPedWeaponManager::SwitchToRagdoll: %lld\n", static_cast<long long>(failPtr - reinterpret_cast<intptr_t>(location))); // 1375
patchStub.Init(successPtr, failPtr);
hook::jump(location, patchStub.GetCode());

But I didn't understand the assert, do you mean something like this?

assert((failPtr - reinterpret_cast<intptr_t>(location)) < 2000);

Yeah, that's fine.

[DaniGP17]

I think it's better to leave it as it was before because now it seems to skip some function in charge of cleaning the weapon, and it stays floating when I get run over.

[iridium-cfx]

Oh right, I mean return right after reading the index, 4C 63 2C 88 48 8B 82, hopefully it won't break with that?

[DaniGP17]

It should be good now, tested and seems to be working as expected

[what11-ctrl]

When will it be updated