How can I optimize Unity C# code for my character movement?

0
votes

I've been trying to figure out how to get this specific movement system working, and I got it working (yay!), but the bad news is that it is SUPER unoptimized. I have lots of copypaste and honestly I'm kind of embarrassed to post this, but here it is. Please let me know what I'm doing wrong if you see any obvious mistakes (you will). I'm still pretty new to programming so I want to catch any bad habits before I let them develop.

For reference, the code posted below is for my player character. It is referencing a float called camDirection from another script which is currently attached to the camera, it goes from -3 to +3 in increments of 1.

The basic idea here is that I wanted the player's input for movement to switch directions on the keyboard when the player presses one of two buttons to move the camera 90 degrees around the player, one for clockwise rotation and another for counterclockwise rotation. When the camera is facing north, we have standard WASD controls(up is W, left is A, down is S and right is D), which I assigned to an action map called PlayerN.

Now, if the camera is facing east, the actionmap gets changed to PlayerW where up is D, left is W, right is S and down is A. If that sounds weird to you, rotate your keyboard 90 degrees counter-clockwise and look at your WASD keys and you might see where I'm coming from. I have no idea how optimal this is, but this was just what made sense to me at the time so I went with it, I couldn't get unity's ApplyBindingOverride to work or anything similar to that, but I'm certain I was using them incorrectly. Along with PlayerS and PlayerE I have 4 action maps altogether, one for each direction.

Anyways, here it is. I'm gonna break down some of this mess below the post:

using System;
using System.Collections;
using System.Collections.Generic;
using UnityEditor;
using UnityEngine;
using UnityEngine.InputSystem;

public class Player : MonoBehaviour
{
    public InputMaster controls;
    private float playerSpeed = 4f;

    Vector2 moveN;
    Vector2 moveE;
    Vector2 moveS;
    Vector2 moveW;

    Vector2 moveW2;
    Vector2 moveS2;
    Vector2 moveE2;

    public CameraRotate refScript;

    void Awake()
    {
        controls = new InputMaster();

        controls.PlayerN.Movement.performed += ctx => moveN = ctx.ReadValue<Vector2>();
        controls.PlayerN.Movement.canceled += ctx => moveN = Vector2.zero;

        controls.PlayerE.Movement.performed += ctx => moveE = ctx.ReadValue<Vector2>();
        controls.PlayerE.Movement.canceled += ctx => moveE = Vector2.zero;
        controls.PlayerE.Movement.performed += ctx => moveE2 = ctx.ReadValue<Vector2>();
        controls.PlayerE.Movement.canceled += ctx => moveE2 = Vector2.zero;

        controls.PlayerS.Movement.performed += ctx => moveS = ctx.ReadValue<Vector2>();
        controls.PlayerS.Movement.canceled += ctx => moveS = Vector2.zero;
        controls.PlayerS.Movement.performed += ctx => moveS2 = ctx.ReadValue<Vector2>();
        controls.PlayerS.Movement.canceled += ctx => moveS2 = Vector2.zero;

        controls.PlayerW.Movement.performed += ctx => moveW = ctx.ReadValue<Vector2>();
        controls.PlayerW.Movement.canceled += ctx => moveW = Vector2.zero;
        controls.PlayerW.Movement.performed += ctx => moveW2 = ctx.ReadValue<Vector2>();
        controls.PlayerW.Movement.canceled += ctx => moveW2 = Vector2.zero;
    }

    void Update()
    {
        Vector2 inputVectorN = controls.PlayerN.Movement.ReadValue<Vector2>();
        Vector2 inputVectorE = controls.PlayerE.Movement.ReadValue<Vector2>();
        Vector2 inputVectorS = controls.PlayerS.Movement.ReadValue<Vector2>();
        Vector2 inputVectorW = controls.PlayerW.Movement.ReadValue<Vector2>();

        Vector2 inputVectorE2 = controls.PlayerE.Movement.ReadValue<Vector2>();
        Vector2 inputVectorS2 = controls.PlayerS.Movement.ReadValue<Vector2>();
        Vector2 inputVectorW2 = controls.PlayerW.Movement.ReadValue<Vector2>();

        switch (refScript.camDirection)
        {
            case -3:
                Debug.Log("Facing East");
                inputVectorE = new Vector2(moveE.x, moveE.y) * Time.deltaTime * playerSpeed;
                Vector3 finalVectorE = new Vector3();
                finalVectorE.x = inputVectorE.x;
                finalVectorE.z = inputVectorE.y;
                transform.Translate(finalVectorE, Space.World);
                break;
            case -2:
                Debug.Log("Facing South");
                inputVectorS = new Vector2(moveS.x, moveS.y) * Time.deltaTime * playerSpeed;
                Vector3 finalVectorS = new Vector3();
                finalVectorS.x = inputVectorS.x;
                finalVectorS.z = inputVectorS.y;
                transform.Translate(finalVectorS, Space.World);
                break;
            case -1:
                Debug.Log("Facing West");
                inputVectorW = new Vector2(moveW.x, moveW.y) * Time.deltaTime * playerSpeed;
                Vector3 finalVectorW = new Vector3();
                finalVectorW.x = inputVectorW.x;
                finalVectorW.z = inputVectorW.y;
                transform.Translate(finalVectorW, Space.World);
                break;
            case 0:
                Debug.Log("Facing North");
                inputVectorN = new Vector2(moveN.x, moveN.y) * Time.deltaTime * playerSpeed;
                Vector3 finalVectorN = new Vector3();
                finalVectorN.x = inputVectorN.x;
                finalVectorN.z = inputVectorN.y;
                transform.Translate(finalVectorN, Space.World);
                break;
            case 1:
                Debug.Log("Facing East");
                inputVectorE2 = new Vector2(moveE2.x, moveE2.y) * Time.deltaTime * playerSpeed;
                Vector3 finalVectorE2 = new Vector3();
                finalVectorE2.x = inputVectorE2.x;
                finalVectorE2.z = inputVectorE2.y;
                transform.Translate(finalVectorE2, Space.World);
                break;
            case 2:
                Debug.Log("Facing South");
                inputVectorS2 = new Vector2(moveS2.x, moveS2.y) * Time.deltaTime * playerSpeed;
                Vector3 finalVectorS2 = new Vector3();
                finalVectorS2.x = inputVectorS2.x;
                finalVectorS2.z = inputVectorS2.y;
                transform.Translate(finalVectorS2, Space.World);
                break;
            case 3:
                Debug.Log("Facing West");
                inputVectorW2 = new Vector2(moveW2.x, moveW2.y) * Time.deltaTime * playerSpeed;
                Vector3 finalVectorW2 = new Vector3();
                finalVectorW2.x = inputVectorW2.x;
                finalVectorW2.z = inputVectorW2.y;
                transform.Translate(finalVectorW2, Space.World);
                break;
        }
     
    }

    private void OnEnable()
    {
        controls.Enable();
    }

    private void OnDisable()
    {
        controls.Disable();
    }

}

I think the Switch might be the worst offender here, I couldn't figure out how to condense my code for movement so I just copypasted it a bunch of times for each case. However, once I did this 4 times (for cases -3, -2 -1, and 0) it started giving me errors, and that's because it was conflicting with the code I did in an earlier case.

For instance, when case is -3 OR 1, the player will be facing east, which means I should run the same code. HOWEVER it started giving me errors, and that's when I noticed that I couldn't have identical variable names in a Switch statement (it was my first time using one). So I quickly made variables named moveE2 inputVectorE2 and finalVectorE2 and I did this for every direction except for north since it only shows up once on the switch statement.

One of the big reasons I'm making this post is because I don't necessarily want this to be the final movement system for my character, and I don't know how to easily create something more modular for this code, or for when I do something similar to this in the future. Currently if I want to change the movement at all I have to go into the cases and change it 7 times. Not very efficient.

Anyways thanks for reading, I'd really appreciate any pointers or tips you might have to slim down this code and make it easier to work with in the future.

1
c#unity3d
Aside from the solution required, you need to create field called thisTransform inside every MonoBehavior class. Use this reference instead of transform. This will avoid reflection calls for every access. Initialize the thisTransform = transform inside Awake() or Start() methods. The same goes for any similar commonly used properties like rigidBody, collider or any component you want to use in Update() or FixedUpdate() - Kumar C
create an enum of N,E,S,W,E2,S2,W2 combine the moveXYZ fields into a dictionary where you can use the enum to index into it. Do the same with inputVectorXYZ locals. Then you can make a method that does the thing for each case statement depending on the key - Ruzihm
tbh if caching transform make a noticeable difference, best to just use DOTS and ditch monobehaviours. - Ruzihm
First, use "sealed" keyword before "class" wherever possible to prevent callvirt-calls generated by the C#-compiler :-) - Martin.Martinsson

1 Answers

0
votes

Alright, I optimized it a bit. This mostly to help anyone who finds this via google or something. I learned how to use lists and added them. This made my script easier to work with and also afforded me the ability to get rid of the extra directions (E2,S2,W2) which is nice.

Good news is that it is MUCH more modular now which is great. I'm a little worried that I have too much going on in void update() but we'll see I guess.

I also condensed the switch statements down a bit since I noticed that I have identical things happening in them now.

Even with these improvements I am certain it can be optimized more, but for now this does what I need it to :D Hope this helps somebody.

using System;
using System.Collections;
using System.Collections.Generic;
using UnityEditor;
using UnityEngine;
using UnityEngine.InputSystem;

public class Player : MonoBehaviour
{  
    private Transform _transform;
    //this is the new transform object I added to help with optimization. 
    public InputMaster controls;
    public CameraRotate refScript;
    private float playerSpeed = 4f;

    Vector2 _move;
    Vector2 _inputVector;
    Vector3 _finalVector;

    public List<Vector2> moveDir = new List<Vector2>();
    public List<Vector2> inputVectorDir = new List<Vector2>();
    public List<Vector3> finalVectorDir = new List<Vector3>();

    Vector2 moveN;
    Vector2 moveE;
    Vector2 moveS;
    Vector2 moveW;

    void Awake()
    {
        moveDir.Add(moveN);
        moveDir.Add(moveE);
        moveDir.Add(moveS);
        moveDir.Add(moveW);

        _transform = transform;

        controls = new InputMaster();

        //north
        controls.PlayerN.Movement.performed += ctx => moveDir[0] = ctx.ReadValue<Vector2>();
        controls.PlayerN.Movement.canceled += ctx => moveDir[0] = Vector2.zero;
        //east
        controls.PlayerE.Movement.performed += ctx => moveDir[1] = ctx.ReadValue<Vector2>();
        controls.PlayerE.Movement.canceled += ctx => moveDir[1] = Vector2.zero;
        //south
        controls.PlayerS.Movement.performed += ctx => moveDir[2] = ctx.ReadValue<Vector2>();
        controls.PlayerS.Movement.canceled += ctx => moveDir[2] = Vector2.zero;
        //west
        controls.PlayerW.Movement.performed += ctx => moveDir[3] = ctx.ReadValue<Vector2>();
        controls.PlayerW.Movement.canceled += ctx => moveDir[3] = Vector2.zero;
    }

    void MovementScript()
    {
        _inputVector = new Vector2(_move.x, _move.y) * Time.deltaTime * playerSpeed;
        _finalVector.x = _inputVector.x;
        _finalVector.z = _inputVector.y;
        _transform.Translate(_finalVector, Space.World);
    }

    void Update()
    {
        Vector2 inputVectorN = controls.PlayerN.Movement.ReadValue<Vector2>();
        Vector2 inputVectorE = controls.PlayerE.Movement.ReadValue<Vector2>();
        Vector2 inputVectorS = controls.PlayerS.Movement.ReadValue<Vector2>();
        Vector2 inputVectorW = controls.PlayerW.Movement.ReadValue<Vector2>();
        inputVectorDir.Add(inputVectorN);
        inputVectorDir.Add(inputVectorE);
        inputVectorDir.Add(inputVectorS);
        inputVectorDir.Add(inputVectorW);

        Vector3 finalVectorN = new Vector3();
        Vector3 finalVectorE = new Vector3();
        Vector3 finalVectorS = new Vector3();
        Vector3 finalVectorW = new Vector3();
        finalVectorDir.Add(finalVectorN);
        finalVectorDir.Add(finalVectorE);
        finalVectorDir.Add(finalVectorS);
        finalVectorDir.Add(finalVectorW);

        switch (refScript.camDirection)
        {
            case 0:
                Debug.Log("Facing North");
                _move = moveDir[0];
                _inputVector = inputVectorDir[0];
                _finalVector = finalVectorDir[0];
                MovementScript();
                break;
            case -3:
            case 1:
                Debug.Log("Facing East");
                _move = moveDir[1];
                _inputVector = inputVectorDir[1];
                _finalVector = finalVectorDir[1];
                MovementScript();
                break;
            case -2:
            case 2:
                Debug.Log("Facing South");
                _move = moveDir[2];
                _inputVector = inputVectorDir[2];
                _finalVector = finalVectorDir[2];
                MovementScript();
                break;
            case -1:
            case 3:
                Debug.Log("Facing West");
                _move = moveDir[3];
                _inputVector = inputVectorDir[3];
                _finalVector = finalVectorDir[3];
                MovementScript();
                break;
        }
    }

    private void OnEnable()
    {
        controls.Enable();
    }

    private void OnDisable()
    {
        controls.Disable();
    }

}