0
\$\begingroup\$

I managed to make my 3D player character jump (hurrah!) but I get an unwanted jump upon landing.

It's not a mid-air double-jump. If I press space mid air, then after it get backs to ground it immediately jumps again.

How can I prevent this?

using System.Collections;
using System.Collections.Generic;
using UnityEngine;

public class PlayerMovement : MonoBehaviour
{
    bool jumpKeyIsPressed;
    float horizontalMovement;
    bool isGrounded ;
    Rigidbody rb;
    [SerializeField] float playerJumpForce;
    [SerializeField] float movementSpeed;
    
    void Start()
    {   //sustituye GetComponent<Rigidbody>() por rb!!!
         rb = GetComponent<Rigidbody>();
    }

    void Update()
    {
        if (Input.GetKeyDown(KeyCode.Space))
        {
            jumpKeyIsPressed = true;    
        }

        horizontalMovement = Input.GetAxis("Horizontal");
    }

    private void OnCollisionEnter(Collision collision)
    {
        if (collision.gameObject.CompareTag("Ground"))
        {
            isGrounded = true;
        }
    }

    
    private void FixedUpdate()
    {
        rb.velocity = new Vector3(horizontalMovement * movementSpeed, rb.velocity.y, 0);

        if (jumpKeyIsPressed && isGrounded)
        {
            rb.AddForce(Vector3.up * playerJumpForce, ForceMode.VelocityChange);
            isGrounded = false;
            jumpKeyIsPressed = false;
        }       
    }
}
\$\endgroup\$
1
\$\begingroup\$

What you have here is called input queuing. When the player presses the jump button, if you can't act on it right away, you store-up that button press to fire again when it's applicable.

This can be a good thing in many games. Celeste, as one example, queues a jump input up to a few frames before you land, so if you press the jump button just a moment too early it "snaps" to a perfect moment-of-landing rebound, improving game feel and making it easier to pull off some of the fast-paced precision platforming challenges in that game.

It's also important when you're gathering your input in Unity's Update() method, but acting on it in FixedUpdate(). If the player is running at a very high framerate, you might not get a physics step the first frame after a button press, so if we clear jumpKeyIsPressed back to false in the next Update() then we might erase it before we had a chance to apply it.

The trick is that you don't want to queue the input infinitely! Right now you keep jumpKeyIsPressed == true eternally until we touch the ground and are allowed to jump again and finally consume that input.

A quick and simple fix if you don't want to keep the input queuing at all is to act on the jump input as soon as it's received:

void Update()
{
    if (Input.GetKeyDown(KeyCode.Space) && isGrounded)
    {
        rb.AddForce(Vector3.up * playerJumpVelocity, ForceMode.VelocityChange);
        isGrounded = false;
    }

    horizontalMovement = Input.GetAxis("Horizontal");
}

(I changed your variable name from playerJumpForce to playerJumpVelocity because the way you're using it, it's not actually a force measured in Newtons. It's a delta-v measured in m/s)

You want to apply any continuous movement changes in FixedUpdate() to make sure they're always consistent, no matter how the rhythm of display frames and physics steps interleave. But for an instantaneous change like a jump, there's no harm in applying it eagerly in Update() - it won't kick in until the next physics step anyway, the same as if we queued it up.

If you'd like to keep the input queuing behaviour, we'll just want to give it a time window it's valid for. Instead of tracking a Boolean for whether it's pressed or not, we'll keep a float tracking how long ago it was pressed.

float jumpQueueWindow = 2.0f/30.0f; // Queue for up to 2 frames at 30 FPS.
float secondsSinceJumpPressed = float.PositiveInfinity;

void Update()
{
    secondsSinceJumpPressed += Time.deltaTime;

    if (Input.GetKeyDown(KeyCode.Space))
    {
        secondsSinceJumpPressed = 0f;
    }

    horizontalMovement = Input.GetAxis("Horizontal");
}

void FixedUpdate() {
    rb.velocity = new Vector3(horizontalMovement * movementSpeed, rb.velocity.y, 0);

    if (isGrounded && secondsSinceJumpPressed <= jumpQueueWindow )
    {
        rb.AddForce(Vector3.up * playerJumpVelocity, ForceMode.VelocityChange);
        isGrounded = false;
        secondsSinceJumpPressed = float.PositiveInfinity;
    }
}

You could also store a timestamp when jump is pressed and compare it against the current time. That saves accumulating deltaTime into your variable each frame, but it means the precision of your jump window is limited to the precision of your timestamp, and the precision of Time.time in Unity loses precision if your game's been running for many hours continuously. So I like to err on the side of using frame deltas which keep their precision no matter how long your game's been running.

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Wouldn't it be easier and cleaner to clear the queued inputs the first time after (or at least not before) they Could have been acted on, at the end of FixedUpdate()? \$\endgroup\$ – Weckar E. Oct 17 at 5:37
  • \$\begingroup\$ That gives you behaviour equivalent to the first code snippet I showed, with no queueing (you jump on the next physics step if you were already on the ground, otherwise nothing happens), but slightly more complicated code because you're splitting the work between Update and FixedUpdate unnecessarily. \$\endgroup\$ – DMGregory Oct 17 at 11:50
  • \$\begingroup\$ Just bad form to be adding any forces in Update(), especially if that depends on a collision event. \$\endgroup\$ – Weckar E. Oct 17 at 13:09
  • \$\begingroup\$ That is indeed a common superstition about Unity, but it stems from a misunderstanding of why we focus on physics in FixedUpdate. As I explain in the answer, keeping physics changes in FixedUpdate is important for continuous changes that need to be applied consistently every tick. Doubling up or skipping one because of variances in Update/FixedUpdate interleaving would cause divergence in the simulation. But that doesn't apply to an instantaneous change like the jump. We want that to take effect on just the next physics step after input was received, and this accomplishes that. \$\endgroup\$ – DMGregory Oct 17 at 13:13
  • \$\begingroup\$ Hmm... Only if we can be sure isGrounded won't be set again before the physics motion resolves. \$\endgroup\$ – Weckar E. Oct 17 at 14:16

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.