Unwanted jump when landing

Question

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;
        }       
    }
}

Answer 1

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.