Missing conditional for Jump Hold Time in CanJumpInternal_Implementation

Development Programming & Scripting
1

This might be more of an oversight than a bug or maybe you had a reason, but I think I’ve categorized it accurately. I don’t know if this is specific to 4.12.

This is the current function body for ACharacter::CanJumpInternal_Implementation :

const bool bCanHoldToJumpHigher = (GetJumpMaxHoldTime() > 0.0f) && IsJumpProvidingForce();

return !bIsCrouched && CharacterMovement && (CharacterMovement->IsMovingOnGround() || bCanHoldToJumpHigher) && CharacterMovement->IsJumpAllowed() && !CharacterMovement->bWantsToCrouch;

This works fine if you have the JumpMaxHoldTime set to 0.f as that essentially disables it, however if you enable it you can continue jumping forever due to a missing conditional, now you could argue that the input should check if IsMovingOnGround() but when you add systems over the top you have to chuck all those into the check as well where appropriate, which shouldn’t be necessary.

The second line with the return should be this:

return !bIsCrouched && CharacterMovement && (CharacterMovement->IsMovingOnGround() || (bCanHoldToJumpHigher && JumpKeyHoldTime > JumpMaxHoldTime)) && CharacterMovement->IsJumpAllowed() && !CharacterMovement->bWantsToCrouch;

Note the extra parenthesis replacing the bCanHoldToJumpHigher with an extra conditional bCanHoldToJumpHigher && JumpKeyHoldTime > JumpMaxHoldTime

If you agree this is a bug I’m happy to submit a pull request if it would be convenient.

2

Vaei,

I’ll be happy to take a look at this, but I just want to make sure I understand exactly what the problem is here (I’ll start working on a repro either tomorrow, or early next week).

I’m guessing when you said the player can jump forever, you mean that as long as the player is holding the jump key their character just continues moving upward. If that’s the case, that does sound like a bug.

However, while the fix you have may seem to work, I don’t think it’s what we actually want. The biggest problem is that JumpyKeyHoldTime > JumpMaxHoldTime is that it implies the character cannot jump until the player has held the button for at least that long.

Further, if you expand everything out the logic actually makes sense:

bool ACharacter::CanJumpInternal_Implementation() const
{
        // I've included the IsJumpProvidingForce and GetJumpMaxHoldTime
        // methods below.
        // This is what the fully expanded conditional looks like:
        return !bIsCrouched &&
                 CharacterMovement && 
                 (
                     CharacterMovement->IsMovingOnGround() ||
                     (
                         JumpMaxHoldTime > 0.0f &&
                         bPressedJump && JumpKeyHoldTime > 0.0f &&
                         JumpKeyHoldTime < JumpMaxHoldTime
                     )
                 ) &&
                 CharacterMovement->IsJumpAllowed() &&
                 !CharacterMovement->bWantsToCrouch;
}

bool ACharacter::IsJumpProvidingForce() const
{
	return (bPressedJump && JumpKeyHoldTime > 0.0f && JumpKeyHoldTime < GetJumpMaxHoldTime());
}

float ACharacter::GetJumpMaxHoldTime() const
{
	return JumpMaxHoldTime;
}

Basically:

  1. The character isn’t crouching now.
  2. The character doesn’t want to crouch next frame.
  3. The character is allowed to jump.
  4. The character is moving on the ground OR they’ve held the jump key less than the maximum allowable time.

Where the problem may be occurring is if we’re not calculating the JumpKeyHoldTime properly. For example, if JumpKeyHoldTime is stuck at 0, then it’d give the same behavior described above, and your fix would seemingly mask the problem (both IsMovingOnGround() and 0 > JumpMaxHoldTime would evaluate to false).

Like I said, I’ll try to take more of a look into this issue early next week.

Thanks,
Jon N.

3

Hi Widmo, thanks for responding - I’ll hold off on responding to most of that since there’s a bit of a misunderstanding in the way. The issue isn’t being able to hold jump and go up forever, the issue is being able to repeatedly tap the jump key and continuously jump while in the air - similar to double jumping (except without a limit).

Yeah the > is misleading, it was the simplest fix on my end and the actual fix would involve something more readable.

By checking if IsMovingOnGround() in the PlayerController when calling jump from the input, it will prevent this occurring but the CanJump() technically should be doing that itself. By doing it in the PlayerController I have to go in and add all the conditionals for any other time it can jump for my own systems such as Wall Jumping and Ledge climbing (uses jump), whereas solving it in CanJump means it simply works and is out of the way.

EDIT Sorry - somehow I missed the IsJumpProvidingForce() function, up late at night programming with a headache and all, so basically - this method isn’t doing it’s job. It’s allowing us to repeatedly jump so long as there’s any max hold time set.

4

Vaei,

After talking with another engineer who works more closely with this system, it seems like this is a known issue. In fact there’s even an official video covering the work around(s).

However, we're going to work on actually putting some things like that into the engine itself so out of the box it works like expected.

Thanks,
Jon N.