Mtasa-blue: setPedWearingJetpack fails to remove if setPedAnimation is ran shortly before

Created on 16 Oct 2020  路  4Comments  路  Source: multitheftauto/mtasa-blue

Describe the bug
setPedWearingJetpack fails to remove if setPedAnimation is ran shortly before setPedWearingJetpack. It works fine when giving, but not removing.

To reproduce
jetpack-bug.zip

Colshape is located at -2422.6799316406, -608.91955566406

Expected behaviour
The jetpack should be able to be removed regardless of setPedAnimation.

Screenshots
https://youtu.be/ZXm2OWkB05w

Version
1.5.8

bug good first issue

Most helpful comment

Great work both of you. Thanks for the detailed investigation. I will implement :)

All 4 comments

Simpler reproduction:

  1. Observe this works fine: run me.jetpack = true
  2. Observe this does not work (and returns false): run me:setAnimation(); iprint(me:setWearingJetpack(false))
  3. Observe this DOES work (returns true): iprint(me:setWearingJetpack(false))

setPedWearingJetpack returns false, which means the server knows something is going on. This is what setPedWearingJetpack looks like:

https://github.com/multitheftauto/mtasa-blue/blob/c610ff3a74f1563d3355b5699b50b96a44b59068/Server/mods/deathmatch/logic/CStaticFunctionDefinitions.cpp#L3899-L3919

So the function returns false if the ped is not spawned, or if you're not changing the jetpack state. Lets see if setPedAnimation changes any of these properties.

https://github.com/multitheftauto/mtasa-blue/blob/c610ff3a74f1563d3355b5699b50b96a44b59068/Server/mods/deathmatch/logic/CStaticFunctionDefinitions.cpp#L4219-L4266

So lines 4231-4233 are taking away the jetpack, as a fix to @ArranTuna's comment in mantis issue 9522 (pr submitted by @patrikjuvonen in #229, merged by @saml1er):

similar problem with setElementPosition, setElementModel and setPedAnimation the jetpack is removed but jetpack function (maybe just server side) still think a jetpack is being worn.

Except the game won't remove the player's jetpack when cancelling an animation (because there's no animation to cancel). So, until the next sync, the server thinks the player is not wearing a jetpack.

Note that the player will sync basically instantly, but not within the same frame, so you'll find that this prints false:

run setPedAnimation(me); iprint(me.jetpack)

But this prints true:

run setPedAnimation(me)
run me.jetpack

So we need to only set the jetpack to false IF setting the animation to something different.

The real logic would be something like if (playerIsCurrentlyAnimated && !cancelAnimationRequested) { player->jetpack = false; } but it should be basically impossible for the player to be animated and wearing a jetpack, so we don't need to worry about checking if the player is currently animated, as it would already be set to false in that case.


TL;DR change this:

            // Remove jetpack now so it doesn't stay on (#9522#c25612)
            if (pPed->HasJetPack())
                pPed->SetHasJetPack(false);

            // Remove choking state
            if (pPed->IsChoking())
                pPed->SetChoking(false);

            // TODO: save their animation?

            // Tell the players
            CBitStream BitStream;
            if (!blockName.empty() && !animName.empty())
            {
                BitStream.pBitStream->WriteString<unsigned char>(blockName);
                BitStream.pBitStream->WriteString<unsigned char>(animName);
                BitStream.pBitStream->Write(iTime);
                BitStream.pBitStream->WriteBit(bLoop);
                BitStream.pBitStream->WriteBit(bUpdatePosition);
                BitStream.pBitStream->WriteBit(bInterruptable);
                BitStream.pBitStream->WriteBit(bFreezeLastFrame);
                BitStream.pBitStream->Write(iBlend);
                BitStream.pBitStream->WriteBit(bTaskToBeRestoredOnAnimEnd);
            }

to this:

            // Remove choking state
            if (pPed->IsChoking())
                pPed->SetChoking(false);

            // TODO: save their animation?

            CBitStream BitStream;
            if (!blockName.empty() && !animName.empty())
            {
                // Remove jetpack now so it doesn't stay on (mantis#9522#c25612 and GitHub #1724)
                if (pPed->HasJetPack())
                    pPed->SetHasJetPack(false);

                // Tell the players
                BitStream.pBitStream->WriteString<unsigned char>(blockName);
                BitStream.pBitStream->WriteString<unsigned char>(animName);
                BitStream.pBitStream->Write(iTime);
                BitStream.pBitStream->WriteBit(bLoop);
                BitStream.pBitStream->WriteBit(bUpdatePosition);
                BitStream.pBitStream->WriteBit(bInterruptable);
                BitStream.pBitStream->WriteBit(bFreezeLastFrame);
                BitStream.pBitStream->Write(iBlend);
                BitStream.pBitStream->WriteBit(bTaskToBeRestoredOnAnimEnd);
            }

Note that #229 also affects choking, so there might be a similar issue there. Also, someone needs to test to make sure that this suggested fix doesn't cause a regression of that mantis issue. There's a nice set of tests to follow in #229. Thanks for documenting that @patrikjuvonen!!!

Great work both of you. Thanks for the detailed investigation. I will implement :)

Submitted PR #1733 to close this issue.

Was this page helpful?
0 / 5 - 0 ratings