Any ideas to optimize this scrip?

Questions & Answers
1

Hi i have just coded i script to control the basic movements of 2 elements that work with those constraints:

  • You cant control both elements(player1 and player 2) at the same time so when you start playing you control the player1, and pressing tab u can switch between the two players.
  • If u hold space the active player jump continually
  • A wall in the center divide the scenario in 2 halfs, the right for player1 and the left for player2

Actually this code works well, but i see it too rough, i hope someone can help me to optimize it

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

public class PlayersMoving : MonoBehaviour
{
    private ModeSwitcher modeScript;

    private Transform rightPlayer;
        private Rigidbody rb_R;

    private Transform leftPlayer;
        private Rigidbody rb_L;

    private bool jumpingLeft, auxLeft;
    private bool jumpingRight, auxRight;

    private float rightMove;
    private float leftMove;
    private float jumpForce;

    private LayerMask floor;

    void Awake()
    {
        modeScript = GameObject.FindObjectOfType<ModeSwitcher>();
        rightPlayer = GameObject.FindGameObjectWithTag("RightPlayer").transform;
        rb_R = rightPlayer.GetComponent<Rigidbody>();
        leftPlayer = GameObject.FindGameObjectWithTag("LeftPlayer").transform;
        rb_L = leftPlayer.GetComponent<Rigidbody>();
        floor = LayerMask.GetMask("Floor");

        rightMove = 5;
        leftMove = -5;
        jumpForce = 400;

        auxLeft = false;
        jumpingRight = false;
        auxRight = false;
        jumpingLeft = false;
    }

    void Update()
    {
        //Right Player On
        if (auxRight && jumpingRight && OnFloor(rightPlayer.GetComponent<Collider>(), Vector3.down))
        {
            jumpingRight = false;
            auxRight = false;
        }
        if (modeScript.GetMode())
        {
            //MOVING & JUMPING
            if (Input.GetKey(KeyCode.D))
            {
                Move(rightMove, rb_R, OnFloor(rightPlayer.GetComponent<Collider>(), Vector3.down));
            }
            if (Input.GetKey(KeyCode.A) && !OnFloor(rightPlayer.GetComponent<Collider>(), Vector3.left))
            {
                Move(leftMove, rb_R, OnFloor(rightPlayer.GetComponent<Collider>(), Vector3.down));
            }
            if ( (Input.GetKeyUp(KeyCode.D) || (Input.GetKeyUp(KeyCode.A))) && OnFloor(rightPlayer.GetComponent<Collider>(), Vector3.down))
            {
                rb_R.velocity = Vector3.zero;
            }
            if (Input.GetKey(KeyCode.Space) && OnFloor(rightPlayer.GetComponent<Collider>(), Vector3.down) && (Mathf.Abs(rb_R.velocity.y) == 0) && !jumpingRight)
            {
                jumpingRight = true;
                rb_R.AddForce(new Vector3(0, jumpForce, 0));
                StartCoroutine("JumpingWaitRight");
            }

            //Ground Check
            if (OnFloor(rightPlayer.GetComponent<Collider>(), Vector3.down)) Debug.Log("RIGHT ON FLOOR");
            else Debug.Log("RIGHT NOT ON FLOOR");
        }
        //Left Player On
        else
        {
            if (auxLeft && jumpingLeft && OnFloor(leftPlayer.GetComponent<Collider>(), Vector3.down))
            {
                jumpingLeft = false;
                auxLeft = false;
            }
            //MOVING & JUMPING
            if (Input.GetKey(KeyCode.D) && !OnFloor(leftPlayer.GetComponent<Collider>(), Vector3.right))
            {
                Move(rightMove, rb_L, OnFloor(leftPlayer.GetComponent<Collider>(), Vector3.down));
            }
            if (Input.GetKey(KeyCode.A))
            {
                Move(leftMove, rb_L, OnFloor(leftPlayer.GetComponent<Collider>(), Vector3.down));
            }
            if ((Input.GetKeyUp(KeyCode.D) || (Input.GetKeyUp(KeyCode.A))) && OnFloor(leftPlayer.GetComponent<Collider>(), Vector3.down))
            {
                rb_L.velocity = Vector3.zero;
            }
            if (Input.GetKey(KeyCode.Space) && OnFloor(leftPlayer.GetComponent<Collider>(), Vector3.down) && (Mathf.Abs(rb_L.velocity.y) == 0) && !jumpingLeft)
            {
                jumpingLeft = true;
                rb_L.AddForce(new Vector3(0, jumpForce, 0));
                StartCoroutine("JumpingWaitLeft");
            }
            //Ground Check
            if (OnFloor(leftPlayer.GetComponent<Collider>(), Vector3.down)) Debug.Log("LEFT ON FLOOR");
            else Debug.Log("LEFT NOT ON FLOOR");
        }
    }

    private void Move(float direction, Rigidbody player, bool onFloor)
    {
        if (onFloor) player.velocity = new Vector3(direction, player.velocity.y, 0);
        else player.velocity = new Vector3(direction/3, player.velocity.y, 0);
    }

    private bool OnFloor(Collider player, Vector3 direction)
    {
        return Physics.Raycast(player.bounds.center, direction, player.bounds.extents.y + 0.1f, floor);
    }

    //***I coded those coroutine because when i hold the space bar down to jump continually 
    //sometimes the ball made an unwanted superjump***
    IEnumerator JumpingWaitRight()
    {
        //We keep the jumping value true for the first seconds right after doing the jump
        yield return new WaitForSeconds(.5f);
        //jumping = false;
        auxRight = true;
        StopCoroutine("JumpingWaitRight");
    }
    IEnumerator JumpingWaitLeft()
    {
        //We keep the jumping value true for the first seconds right after doing the jump
        yield return new WaitForSeconds(.5f);
        //jumping = false;
        auxLeft = true;
        StopCoroutine("JumpingWaitLeft");
    }
}

HERE IS THE SWITCHER CLASS

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

public class ModeSwitcher : MonoBehaviour
{
    private bool rightMode;

    void Start()
    {
        rightMode = true;    
    }

    
    void Update()
    {
        if(Input.GetKeyDown(KeyCode.Tab))
        {
            rightMode = !rightMode;
        }
    }

    public bool GetMode()
    {
        return rightMode;
    }
}
2

When refactoring code, the first step is to look at duplicate code, and try to apply the DRY (Do Not Repeat Yourself) method as best as possible without complexifying the code.

For example, your code is duplicated for the left and right player, despite using the same logic, this can be fixed like this:

    public class Player
    {
        public float movement = 5;
        public float jumpForce = 400f;

        private bool jumping;
        private bool aux;
        private Rigidbody rb;
        private Collider collider;

        void Start()
        {
            movement = 5;
            jumpForce = 400;
            rb = GetComponent<Rigidbody>();
            collider = GetComponent<Collider>();
        }

        static public bool OnFloor(Collider player, Vector3 direction)
        {
            return Physics.Raycast(player.bounds.center, direction, player.bounds.extents.y + 0.1f, LayerMask.GetMask("Floor"));
        }

        private bool OnFloor()
        {
            return OnFloor(collider, Vector3.down);
        }

        public void PlayerMovement()
        {
            bool onFloor = OnFloor();
            if (aux && jumping && onFloor)
            {
                jumping = false;
                aux = false;
            }
            //MOVING & JUMPING
            if (Input.GetKey(KeyCode.D))
            {
                Move(movement, onFloor);
            }
            if (Input.GetKey(KeyCode.A))
            {
                Move(-movement, onFloor);
            }
            if ( (Input.GetKeyUp(KeyCode.D) || (Input.GetKeyUp(KeyCode.A))) && onFloor)
            {
                rb.velocity = Vector3.zero;
            }
            if (Input.GetKey(KeyCode.Space) && onFloor && !jumping)
            {
                jumping = true;
                rb.AddForce(new Vector3(0, jumpForce, 0));
                StartCoroutine("JumpingWait");
            }
        }

        private void Move(float direction, bool onFloor)
        {
            if (onFloor == false)
                direction /= 3f;
            rb.velocity = new Vector3(direction, rb.velocity.y, 0);
        }

        //***I coded those coroutine because when i hold the space bar down to jump continually 
        //sometimes the ball made an unwanted superjump***
        IEnumerator JumpingWait()
        {
            //We keep the jumping value true for the first seconds right after doing the jump
            yield return new WaitForSeconds(.5f);
            //jumping = false;
            aux = true;
        }
    }

    public class PlayersMoving : MonoBehaviour
    {
        private ModeSwitcher modeScript;

        private Player rightPlayer;
        private Player leftPlayer;

        void Awake()
        {
            modeScript = GameObject.FindObjectOfType<ModeSwitcher>();
            rightPlayer = GameObject.FindGameObjectWithTag("RightPlayer")
                .GetComponent<Player>();
            leftPlayer = GameObject.FindGameObjectWithTag("LeftPlayer")
                .GetComponent<Player>();
        }

        void Update()
        {
            //Right Player On
            if (modeScript.GetMode())
            {
                rightPlayer.PlayerMovement();
            }
            //Left Player On
            else
            {
                leftPlayer.PlayerMovement();
            }
        }
    }

Do be aware that this code is not tested, and might not work as you intended. I removed the side raycast, hoping that you are using rigidbodies and colliders instead.

Now for a bit of advice. It’s weird that you chose to abstract away the very simple mode switching, and not the complex player logic. You should consider using Input.GetAxis instead of key events. The super jumping is caused by multiple Updates happening between FixedUpdates, since RigidBodies won’t move at all unless a FixedUpdate has been reached, and they will stay grounded for multiple updates. Basically simply make the jump code set the y velocity instead of incrementing it with AddForce.

3

Thak you so much, you helped me a lot