Looking for feedback on a 2D character controller I've written

Here’s a 2D movement script I’ve been working on for a while. I’ve been (for no particular reason) going for a sort of Smash Bros style controller but its really not even close and has a lot of functionality left to be implemented, but, with my coding experience, it’s the about the best I can do. I’d love to here any feedback anyone might have as well as any advice or examples of how to implement further functionality, such as lateral movement relative to ground slope, a way of keeping the player from sliding down slopes, any sort of functionality you might recommend or simply pointing out any any coding mistakes I may have made or better ways of doing the same things. Also, feel free to use the code and many thanks in advance:)

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

[RequireComponent(typeof(Rigidbody2D))]
public class PlayerMovement : MonoBehaviour
{
    // many of these variables are public for testing only and should be made private when an ideal value is determined
    //as there are a lot of variables, this controller might be difficult to get working properly but the default values should be ok for a rigidbody with a mass of one unit and a physics material with at or near zero drag
    and a fixed timestamp of 0.008
    [Header("Movement")]
    [Tooltip("This movement type is better (than Movement Type Two) for physics-based games")]
    public bool MoveTypeOne = false;
    [Tooltip("This movement type is less physically accurate but offers a somewhat more responsive feel.")]
    public bool MoveTypeTwo = false;
    [Tooltip("Walking speed.")]
    public float WalkSpeed = 5f;
    [Tooltip("Running speed.")]
    public float RunSpeed = 8f;
    [Tooltip("Controlls how quickly the rigidbody changes lateral velocity.")]
    public float MKp = 20f;
    [Tooltip("Sets the maximum force that can be applied to the rigidbody for lateral acceleration/deceleration.")]
    public float RbVChange = 30f;

    [Header("Jumping")]
    [Tooltip("Jump power.")]
    public float JumpPower = 9f;
    [Tooltip("The rate the player accelerates towards the Jump Power value.")]
    public float JKp = 1f;
    [Tooltip("The Time the player can accelerate upwards during a jump.")]
    public float jumpTime = 0.2f;

    [Header("Double Jump")]
    [Tooltip("Double jump power.")]
    public float DJPower = 9f;
    [Tooltip("The time the player can accelerate upwards during a double jump.")]
    public float DJTime = 0.15f;
    [Tooltip("The rate the player accelerates towards the Double Jump Power value.")]
    public float DJKp = 16f;

    [Header("Stomp")]
    [Tooltip("How quickly the player moves downwards during a stomp maneuver.")]
    public float StompSpeed = 1f;
    [Tooltip("How quickly the player accelerates towards the Stomp Speed value")]
    public float StmpKp = 1f;

    [Space(10)]
    [Tooltip("A force being applied downward on the player to mimic higher gravity the give the controller a snappier feel.")]
    public float DownForce = 2f;
    [Tooltip("How quickly the player accelerates downwards to the Down Force value.")]
    public float DFKp = 1f;
    public float DFClamp = 10f;
    [Tooltip("The amount of movement on a joystick required for input to register and make the player move.")]
    [Range(0,1)]public float JoyStickDeadZone = 0.01f;

    [Header("Ground Check")]
    [Tooltip("The layer(s) or objects that the player can jump off of.")]
    public LayerMask GroundLayer;
    [Tooltip("The transforms of two gameobjects located at the base of the player are used to determine whether or not the player is grounded and, thus, if it can jump or not.")]
    public Transform GroundCheck01;
    public Transform GroundCheck02;
  
    [Tooltip("The radius of the circle(s) drawn around the groundcheck(s) that check for collisions.")]
    public float GroundCheckRadius = 0.125f;

    [Header("Camera")]
    [Tooltip("I feel like the name describes this one pretty well...")]
    public GameObject MainCamera;
    [Tooltip("The rate the camera accelerates to reach the players position along the Y axis.")]
    public float CameraFollowSpeedY = 0.5f;
    [Tooltip("The rate the camera accelerates to reach the players position along the X axis.")]
    public float CameraFollowSpeedX = 1f;

    //a bunch of booleans for dynamic jump height and jumping and stuff
    private bool hasDoubleJumped = false;
    private bool IsGrounded = false;
    private bool isJumpButtonPressed = false;
    private bool isJumpButtonHeld = false;

    private Rigidbody2D rb;

    //default movement speed or the speed when not dashing
    private float DWS = 10f;

    //error is the difference between desired value and current value
    private float ErrorY = 0f;
    private float DJError = 0f;
    private float StmpErr = 0f;
    private float DFErr = 0f;

    //assuming that all moving objects have a rigidbody2D, this is the velocity of the platform the player is currently colliding with
    private Vector2 PlatformVol = Vector2.zero;

    void Start()
    {
        rb = GetComponent<Rigidbody2D>();
        DWS = WalkSpeed;
        //freeze rb rotation on the Z axis
        rb.constraints = RigidbodyConstraints2D.FreezeRotation;
    }

     void OnCollisionStay2D(Collision2D collisionInfo)
    {
        if (collisionInfo.gameObject.GetComponent<Rigidbody2D>() != null){
            PlatformVol = collisionInfo.gameObject.GetComponent<Rigidbody2D>().velocity;
        }
    }

    void FixedUpdate()
    {
        float MoveX = Input.GetAxis("Horizontal") * WalkSpeed;
        //here the player velocity is calculated relative to any moving platforms the player is in contact with
        Vector2 PlayerSpeed = rb.velocity - PlatformVol;
        float ErrorX = MoveX - PlayerSpeed.x;
        ErrorY = JumpPower - PlayerSpeed.y;
        DJError = DJPower - PlayerSpeed.y;
        StmpErr = -StompSpeed - PlayerSpeed.y;
        DFErr = -DownForce - PlayerSpeed.y;

        //Here are two slightly different ways if moving left and right
        if (MoveTypeOne == true){
            if(Mathf.Abs(MoveX) > JoyStickDeadZone){
               if (MoveX > 0){
                   rb.AddForce(transform.right * Mathf.Clamp(ErrorX * MKp, 0, RbVChange));
                }
               if (MoveX < 0){
                   rb.AddForce(transform.right * Mathf.Clamp(ErrorX * MKp, -RbVChange, 0));
                }
            }
            MoveTypeTwo = false;
        }
        if (MoveTypeTwo == true){
            if (Mathf.Abs(MoveX) > JoyStickDeadZone){
                rb.AddForce(transform.right * Mathf.Clamp(ErrorX * MKp, -RbVChange, RbVChange));
            }
            MoveTypeOne = false;
        }

        if (Input.GetKeyDown(KeyCode.S) && IsGrounded == false){
            StartCoroutine(StompRoutine());
        }


        //simply increasing gravity gives a rather floaty feel to the jumping, so, instead, a proportional force is applied downwards
        //this should probably only be applied when the when the jump timer is at zero but i'm too lazy so instead it's clamped
        rb.AddForce(transform.up * Mathf.Clamp(DFErr * DFKp, -DFClamp, 0));

        //check is the player is grounded or not
        IsGrounded = Physics2D.OverlapCircle(GroundCheck01.position, GroundCheckRadius, GroundLayer) || Physics2D.OverlapCircle(GroundCheck02.position, GroundCheckRadius, GroundLayer);

        if ((isJumpButtonPressed == true || Input.GetButtonDown("Jump")) && IsGrounded == true){
            StartCoroutine(JumpRoutine());
        }
        if ((isJumpButtonPressed == true || Input.GetButtonDown("Jump")) && hasDoubleJumped == false && IsGrounded == false){
            StartCoroutine(DoubleJumpRoutine());
        }
        if (IsGrounded == true){
            hasDoubleJumped = false;
        }

         //Camera follow cause i'm too lazy to put it in a seperate script
        float CamErrY = transform.position.y - MainCamera.transform.position.y;
        float CamErrX = transform.position.x - MainCamera.transform.position.x;
        MainCamera.transform.Translate(Vector2.up * CamErrY * CameraFollowSpeedY);
        MainCamera.transform.Translate(Vector2.right * CamErrX * CameraFollowSpeedX);

        PlatformVol = Vector2.zero;
    }
    void Update()
    {
        // a dashing functionality
         if (Input.GetKey(KeyCode.LeftShift)){
            WalkSpeed = RunSpeed;
        }
        else{
            WalkSpeed = DWS;
        }
      
        //input is checked for twice, once in Update and again is FixedUpdate, because it seems to give the most consistent jump height
        isJumpButtonPressed = Input.GetButtonDown("Jump");
        isJumpButtonHeld = Input.GetButton("Jump");
    }

    IEnumerator JumpRoutine()
    {
        float timer = 0;
        while ((isJumpButtonHeld == true || Input.GetButton("Jump")) && timer < jumpTime)
        {
            rb.AddForce(transform.up * ErrorY * JKp);
            timer += Time.fixedDeltaTime;
            yield return null;
        }
    }

    IEnumerator DoubleJumpRoutine()
    {
        float timer = 0;
        while ((isJumpButtonHeld == true || Input.GetButton("Jump")) && timer < DJTime)
        {
            rb.AddForce(transform.up * DJError * DJKp);
            timer += Time.fixedDeltaTime;
            hasDoubleJumped = true;
            yield return null;
        }
    }
     // a stomping functionality because... well... stomp.
    IEnumerator StompRoutine()
    {
        while (IsGrounded == false)
        {
            rb.AddForce(transform.up * StmpErr * StmpKp);
            yield return null;
        }
    }
}

p.s. I was going for a drag-and-drop solution for testing purposes. Which is why all the public variables have a tooltip but in this format it makes it very difficult to read. My apologies.

I haven’t read the whole thing but I’ll start with a small improvement I can see off the bat:

    [Tooltip("This movement type is better (than Movement Type Two) for physics-based games")]
    public bool MoveTypeOne = false;
    [Tooltip("This movement type is less physically accurate but offers a somwhat more responsive feel.")]
    public bool MoveTypeTwo = false;

You have these two mutually exclusive options, and you’re controlling them with two individual boolean values. This leads you do to weird stuff like on line 130 when you set MoveTypeTwo = false inside the MoveTypeOne block. Imagine if you have 5 or 6 movement types. You’d have to set a lot of things to false there!

There’s a better way. You can use enums! First, define your Enum. you should put this in its own file or in the same file but outside of the class definition:

public enum MoveType {
  One,
  Two,
  Three
  // etc...
}

Then, instead of having MoveTypeOne and MoveTypeTwo bools, you can have just one field:public MoveType MyMoveType;

Then, in your code, you can use a switch statement to process the movement based on the MoveType:

switch (MyMoveType) {
  case MoveType.One:
    // Code for movement type 1
    break;
  case MoveType.Two:
    // Code for movement type 2
    break;
  // etc...
  default:
    // And this will make sure an error is thrown if you forgot to implement one of the movement types!
    throw new System.Exception("Unknown movement type " + MyMoveType.ToString());
}
1 Like

Awesome!! I’d wondered how to do this. I’ve just written it in and it works great