Feedback on Player Movement Code - Suggestions for Improvement and Best Practices

Hi, I’m 17 years old and have been working on my own game for a few months now. I want to make sure my code is well-structured to avoid possible errors in the future and make development easier. Here’s my code for player movement. I would really appreciate any feedback or improvement suggestions, especially regarding best practices and how I can maintain the code in the long run.

I’m looking forward to your tips and advice!

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

public class PlayerMovement : MonoBehaviour
{
    #region Variabeln
    [Header("Important References")]
    private Rigidbody2D playerRigidbody;  // Referenz auf den Rigidbody2D des Spielers für die Bewegung
    private Vector2 movementDirection;  // Richtungsvektor für die Bewegung des Spielers
    [HideInInspector] public static float movementDirectionValue;  // Eingabewert für die horizontale Bewegung

    [Header("Player Movement Settings")]
    [HideInInspector] public static bool isWalkingAllowed = true;  // Steuerung, ob der Spieler sich bewegen kann
    [SerializeField] private float walkingSpeed;  // Geschwindigkeit des Spielers

    [Header("Player Gravity Settings")]
    public static bool isGrounded = true;  // Überprüft, ob der Spieler den Boden berührt
    [SerializeField] private LayerMask groundLayer;  // Schichtmaske für den Boden
    [SerializeField] private Transform groundCheckPosition;  // Position, an der überprüft wird, ob der Spieler den Boden berührt
    [SerializeField] private float groundCheckRadius = 0.2f;  // Radius des Bodenkollisionschecks

    [SerializeField] private float maxGravityDelay = 1f;  // Maximale Verzögerung, bevor Schwerkraft angewendet wird
    private float gravityDelayTime;  // Aktuelle Verzögerungszeit für Schwerkraft
    private bool isGravityApplied = false;  // Ob Schwerkraft angewendet wurde

    [Header("Input Buffers")]
    [SerializeField] private float dashInputBufferTime = 2f;  // Eingabepufferzeit für Dash
    private float dashInputBuffer;  // Eingabepuffer für Dash

    [Header("Dash-Player-System")]
    [SerializeField] private float dashPower = 10f;  // Power des Dash
    [SerializeField] private float dashDuration;  // Dauer des Dash

    public bool isDashAllowed = true;  // Ob Dash aktiviert ist
    [HideInInspector] public static bool isDashing = false;  // Ob der Spieler gerade dashen wird
    private Coroutine dashCoroutine;  // Coroutine für den Dash

    [Header("Jump_System")]
    [SerializeField] private float jumpForce = 5f;  // Sprungkraft
    [SerializeField] private float maxJumpHoldTime = 0.1f;  // Maximale Zeit, in der der Sprung gehalten werden kann
    [SerializeField] private float maxJumpHeight = 6f;  // Maximale Sprunghöhe
    [SerializeField] private float fallGravityMultiplier = 2f;  // Multiplikator für die Schwerkraft beim Fallen
    private bool isJumping = false;  // Ob der Spieler gerade springt
    private float jumpTimer = 1.0f;  // Timer für den Sprung

    #endregion

    #region Awake
    // Wird beim Starten des Spiels aufgerufen
    void Awake()
    {
        playerRigidbody = GetComponent<Rigidbody2D>();  // Hole den Rigidbody2D des Spielers
        playerRigidbody.gravityScale = 0f;  // Setze die Schwerkraft auf null, bis sie manuell angewendet wird
        gravityDelayTime = maxGravityDelay;  // Initialisiere die Verzögerungszeit für die Schwerkraft
    }
    #endregion

    #region Update Methode
    // Wird jedes Frame aufgerufen
    void Update()
    {
        GetPlayerInputs();  // Hole die Eingaben des Spielers
        ClampValues();  // Beschränke Werte wie die Schwerkraftverzögerung
        HandleJump();  // Verwalte die Sprunglogik

        // Überprüfe, ob der Spieler einen Dash ausführen möchte
        if (Input.GetMouseButtonDown(1) && Mathf.Abs(movementDirectionValue) > 0 && dashCoroutine == null && isDashAllowed == true && dashInputBuffer < 0f)
        {
            dashInputBuffer = dashInputBufferTime;  // Setze den Dash-Eingabepuffer zurück
            StartCoroutine(HandleDash());  // Starte den Dash
        }
        else
        {
            dashInputBuffer -= Time.deltaTime;  // Reduziere den Puffer
        }
    }
    #endregion

    #region Fixed Update
    // Wird in festen Intervallen aufgerufen (ideal für Physikberechnungen)
    void FixedUpdate()
    {
        CheckIfGrounded();  // Überprüfe, ob der Spieler den Boden berührt

        if (isWalkingAllowed)
        {
            ApplyPlayerMovement();  // Wende die Bewegung auf den Spieler an
        }

        if (!isGrounded)
        {
            ApplyGravity();  // Wende Schwerkraft an, wenn der Spieler nicht am Boden ist
        }
        else
        {
            playerRigidbody.gravityScale = 0f;  // Deaktiviere die Schwerkraft, wenn der Spieler am Boden ist
            isGravityApplied = false;  // Setze Schwerkraft als nicht angewendet
        }
    }
    #endregion

    #region PlayerMethods

    // Holt die Eingaben des Spielers für die horizontale Bewegung
    private void GetPlayerInputs()
    {
        movementDirectionValue = Input.GetAxisRaw("Horizontal");  // Hole die Eingabe für horizontale Bewegung
    }

    // Verarbeitet den Sprung des Spielers
    private void HandleJump()
    {
        if (Input.GetButtonDown("Jump") && isGrounded == true)  // Wenn der Sprungknopf gedrückt wird und der Spieler am Boden ist
        {
            playerRigidbody.linearVelocity = new Vector2(playerRigidbody.linearVelocity.x, jumpForce);  // Setze die Y-Geschwindigkeit für den Sprung
            isJumping = true;  // Setze den Sprungstatus auf wahr
            jumpTimer = maxJumpHoldTime;  // Setze den Timer für das Halten des Sprungs
        }

        if (Input.GetButton("Jump") && isJumping == true)  // Wenn der Sprungknopf gehalten wird
        {
            if (jumpTimer > 0)  // Solange der Timer größer als 0 ist
            {
                float currentYVelocity = playerRigidbody.linearVelocity.y;
                float newYVelocity = Mathf.Lerp(currentYVelocity, maxJumpHeight, 0.1f);  // Berechne die neue Y-Geschwindigkeit
                playerRigidbody.linearVelocity = new Vector2(playerRigidbody.linearVelocity.x, newYVelocity);

                jumpTimer -= Time.deltaTime;  // Reduziere den Sprung-Timer
            }
            else
            {
                isJumping = false;  // Wenn der Timer abgelaufen ist, stoppe den Sprung
            }
        }

        if (Input.GetButtonUp("Jump"))
        {
            isJumping = false;  // Stoppe den Sprung, wenn der Knopf losgelassen wird
        }
    }

    // Beschränkt die Werte, z.B. die Schwerkraftverzögerung
    private void ClampValues()
    {
        gravityDelayTime = Mathf.Clamp(gravityDelayTime, 0f, maxGravityDelay);  // Beschränke die Schwerkraftverzögerung auf ein bestimmtes Maximum
    }

    // Wendet die horizontale Bewegung auf den Spieler an
    private void ApplyPlayerMovement()
    {
        Vector2 newVelocity = playerRigidbody.linearVelocity;  // Hole die aktuelle Geschwindigkeit
        newVelocity.x = movementDirectionValue * walkingSpeed;  // Setze die horizontale Geschwindigkeit
        playerRigidbody.linearVelocity = newVelocity;  // Wende die neue Geschwindigkeit an
    }

    // Wendet die Schwerkraft auf den Spieler an
    private void ApplyGravity()
    {
        if (!isGravityApplied)  // Wenn die Schwerkraft noch nicht angewendet wurde
        {
            gravityDelayTime -= 15f * Time.fixedDeltaTime;  // Reduziere die Schwerkraftverzögerung

            if (gravityDelayTime <= 0f)  // Wenn die Verzögerung abgelaufen ist
            {
                playerRigidbody.gravityScale = 1f;  // Setze die Schwerkraft
                isGravityApplied = true;  // Setze den Status auf "Schwerkraft angewendet"
                gravityDelayTime = maxGravityDelay;  // Setze die Verzögerung zurück
            }
        }

        if (playerRigidbody.linearVelocity.y < 0f)  // Wenn der Spieler fällt
        {
            playerRigidbody.gravityScale = Mathf.Lerp(playerRigidbody.gravityScale, fallGravityMultiplier, 0.1f);  // Erhöhe die Schwerkraft
        }
    }

    // Überprüft, ob der Spieler den Boden berührt
    private void CheckIfGrounded()
    {
        isGrounded = Physics2D.OverlapCircle(groundCheckPosition.position, groundCheckRadius, groundLayer);  // Überprüfe, ob der Spieler am Boden ist
    }

    #endregion

    // Zeichnet eine gelbe Gizmo-Kugel im Editor, um die Kollisionszone für den Boden anzuzeigen
    private void OnDrawGizmos()
    {
        Gizmos.color = Color.yellow;
        Gizmos.DrawWireSphere(groundCheckPosition.transform.position, groundCheckRadius);
    }

    #region Corutines

    // Verarbeitet den Dash des Spielers
    private IEnumerator HandleDash()
    {
        if (Mathf.Abs(movementDirectionValue) < 0.1f)  // Wenn keine Bewegung vorhanden ist, beende die Coroutine
        {
            yield break;
        }
        isDashing = true;  // Setze den Dash-Status auf aktiv
        movementDirection = new Vector2(movementDirectionValue, 0).normalized;  // Setze die Richtung für den Dash

        isWalkingAllowed = false;  // Deaktiviere die normale Bewegung

        playerRigidbody.linearVelocity = movementDirection * dashPower;  // Setze die Geschwindigkeit für den Dash

        yield return new WaitForSeconds(dashDuration);  // Warte für die Dauer des Dashes

        float lerpTime = 0.2f;
        while (lerpTime > 0f)  // Verlangsamen des Dash-Effekts
        {
            lerpTime -= Time.deltaTime;
            playerRigidbody.linearVelocity = Vector2.Lerp(playerRigidbody.linearVelocity, Vector2.zero, 0.2f);  // Lerp die Geschwindigkeit auf null
            yield return null;
        }

        isWalkingAllowed = true;  // Aktiviere die normale Bewegung wieder
        isDashing = false;  // Setze den Dash-Status zurück
        dashCoroutine = null;  // Setze die Dash-Coroutine auf null
    }

    #endregion
}

Personally I would eliminate 99% of those comments. They don’t help. Comments that simply restate what the variable name already tells you are not useful. The variable name already tells you what the value is, as it should. Comments beyond that are just visual noise.

Here’s how to evaluate your own code:

Does the code work and solve your problem?

If so then you’re done. Move on to the next problem.

If not, fix it. Is the code easy to fix and change?

Is it easy to set up? If it fails does it give good messages?

Do you understand all the parts and explain them in a technical sense? Is it easy for you to change out the parts and make them do different things??

1 Like

Thank you!!!

Yes, actually, it annoys me to write all those comments. I once created a script that I couldn’t understand afterward, which is why I’m afraid the same thing might happen again. But now I’m technically better and have learned that it’s best to write 99% of your code yourself, rather than relying on friends, for example. Thank you for your solution and your feedback.

1 Like

I agree that the comments are overkill at this moment. It is harder to read and you will find yourself adjusting comments as you refactor code. That GetPlayerInputs gets the player’s inputs is a given.

Prototyping can be an incredibly useful methodology BTW but you don’t have to intertwine comments and code. A separate doc can explain the mechanism (useful when there are lots of classes involved) and also large chunks of commentary can be appended to the bottom of the class file when related to a single class. I tend to limit inline comments to items I must remember but that would not be obvious months later.

// local only no serialization

There are some questionable items in my mind. GetPlayerInputs() doesn’t actually get the inputs but it “sets” movementDirectionValue. You force people to know that you must call this method prior to relying on the value. I don’t see any real value in wrapping the assignment in a method.

Similarly ClampValues() apparently is needed to clamp some values but only the gravityDelayTime. Rather than do this in Update (I think) it is better to clamp the value at the moment you modify the value. Values can’t be permitted to get out of range and shouldn’t require a separate action to keep them in range.

CheckIfGrounded() is odd as well as it sets a flag. The flag must be set prior to testing it. It is set in FixedUpdate() and indirectly referenced in Update() via HandleJump()

Finally I see you use the convention of == true for Booleans, e.g. isGrounded == true Some people do but I haven’t found it be any more clear. that isGrounded or isJumping alone.

And this is an odd one to picture but… notice in FixedUpdate() you set isGrounded, check isGrounded and either ApplyGravity() or set things that unapplies gravity. Surely you are applying Gravity in either case just the presence or absence of it. It reduces the number of places playerRigidbody.gravityScale and isGravityApplied settings are changed. Like why suddenly in FixedUpdate do two properties get called out?

That’s it from me :slight_smile:

1 Like

Thank you so much for your detailed feedback and suggestions! I really appreciate the time you took to review the code and point out areas for improvement. Your insights about reducing redundant comments, improving method clarity, and simplifying logic are incredibly helpful. I’ll definitely work on implementing these changes to make the code cleaner and more maintainable. Thanks again for your valuable advice—it means a lot! :blush: