Search Unity

  1. Megacity Metro Demo now available. Download now.
    Dismiss Notice
  2. Unity support for visionOS is now available. Learn more in our blog post.
    Dismiss Notice

Feedback Scripts for Basic Movement and Rotation

Discussion in 'Scripting' started by thmire2016, Jun 23, 2022.

  1. thmire2016

    thmire2016

    Joined:
    Jun 22, 2022
    Posts:
    1
    I am fairly new to Unity. I have created a partial game that currently solely consists of a player, a platform, and some cube obstacles.

    The player has a rigid body, capsule collider, and script component. There is also a camera object (player's perspective in first person) with a script attached.

    My biggest concern is am I approaching everything competently so far? Or am I setting myself up for future problems?

    Player script:
    Code (csharp):
    1.  
    2. using System.Collections;
    3. using System.Collections.Generic;
    4. using UnityEngine;
    5.  
    6. public class main : MonoBehaviour
    7. {
    8.     public Rigidbody body;
    9.     public CapsuleCollider collider;
    10.     public Camera perspective;
    11.     private const float movementSpeed = 100f;
    12.     private const KeyCode moveForward = KeyCode.W;
    13.     private const KeyCode moveBackward = KeyCode.S;
    14.     private const KeyCode moveRight = KeyCode.D;
    15.     private const KeyCode moveLeft = KeyCode.A;
    16.  
    17.     /**
    18.      * Teleport player's perspective/camera/view to player/self/this
    19.      */
    20.     private void teleportView()
    21.     {
    22.         // force perspective/camera/view at player's head
    23.         perspective.transform.position = new Vector3(transform.position.x, transform.position.y + collider.bounds.size.y, transform.position.z);
    24.     }
    25.  
    26.     /**
    27.      * Move this object's world coordinates relative to itself respective to the corresponding keyboard key presses
    28.      */
    29.     private void move()
    30.     {
    31.         Vector3 relativeForwardMovement = body.transform.forward * movementSpeed;
    32.         Vector3 relativeBackwardMovement = Quaternion.AngleAxis(180, Vector3.up) * body.transform.forward * movementSpeed;
    33.         Vector3 relativeRightMovement = Quaternion.AngleAxis(90, Vector3.up) * body.transform.forward * movementSpeed;
    34.         Vector3 relativeLeftMovement = Quaternion.AngleAxis(-90, Vector3.up) * body.transform.forward * movementSpeed;
    35.  
    36.         Vector3 desiredMovement = Vector3.zero;
    37.  
    38.         if (Input.GetKey(moveForward))
    39.             desiredMovement += relativeForwardMovement;
    40.         if (Input.GetKey(moveBackward))
    41.             desiredMovement += relativeBackwardMovement;
    42.         if (Input.GetKey(moveRight))
    43.             desiredMovement += relativeRightMovement;
    44.         if (Input.GetKey(moveLeft))
    45.             desiredMovement += relativeLeftMovement;
    46.  
    47.         body.MovePosition(body.transform.position + desiredMovement * Time.deltaTime);
    48.  
    49.         teleportView();
    50.     }
    51.  
    52.     void Start()
    53.     {
    54.         Debug.Log("Started");
    55.     }
    56.  
    57.     void Update()
    58.     {
    59.         move();
    60.     }
    61. }
    62.  
    Camera/player perspective script:
    Code (csharp):
    1.  
    2. using System.Collections;
    3. using System.Collections.Generic;
    4. using UnityEngine;
    5.  
    6. public class guidance : MonoBehaviour
    7. {
    8.     public Rigidbody playerBody;
    9.     private float mouseSensitivity = 100f;
    10.  
    11.     /**
    12.      * Rotate this object's view in response to the mouse movement relative to itself
    13.      */
    14.     private void rotateWithMouseMovement()
    15.     {
    16.         float xRotation = -Input.GetAxis("Mouse Y") * mouseSensitivity * Time.deltaTime;
    17.         float yRotation = Input.GetAxis("Mouse X") * mouseSensitivity * Time.deltaTime;
    18.         transform.localEulerAngles += new Vector3(xRotation, yRotation, 0);
    19.  
    20.         rotatePlayerY();
    21.     }
    22.  
    23.     /**
    24.      * Rotate the player object's y coordinate to match this object's y rotation
    25.      */
    26.     private void rotatePlayerY()
    27.     {
    28.         playerBody.transform.eulerAngles = new Vector3(0, transform.eulerAngles.y, 0);
    29.     }
    30.  
    31.     // TEMP
    32.     private void adjustSensitivity()
    33.     {
    34.         if (Input.GetKey(KeyCode.DownArrow))
    35.             mouseSensitivity = Mathf.Max(0, mouseSensitivity - 1);
    36.         if (Input.GetKey(KeyCode.UpArrow))
    37.             mouseSensitivity = Mathf.Min(100000, mouseSensitivity + 1);
    38.     }
    39.  
    40.     void Start()
    41.     {
    42.         Cursor.lockState = CursorLockMode.Locked; // cursor is invisible and anchored to the center of the screen
    43.     }
    44.  
    45.     void Update()
    46.     {
    47.         rotateWithMouseMovement();
    48.         adjustSensitivity();
    49.     }
    50. }
    51.  
    Any sort of feedback/criticism is welcome and appreciated.
     
  2. Kurt-Dekker

    Kurt-Dekker

    Joined:
    Mar 16, 2013
    Posts:
    38,514
    First, huge props for getting it working. I'll assume it works because obviously the first test is "does it work?" I'll assume the answer is "Yes" for now.

    Second, huge props for asking for feedback.

    Here's some feedback: you should follow typical C# guidelines for class names (capitalization), and come up with FAR better ones than you have. For instance
    main
    and
    guidance
    are useless names.

    How about
    PlayerMovementController
    and
    PlayerRotationController
    ?

    Also, are you actually teleporting? That looks like some kind of positional update, perhaps for the camera. How about either:

    - make a separate
    CameraController
    script (handy when your player dies), OR
    - name that teleport function
    UpdateCameraPosition
    (for instance)

    You call the guidance script "Camera/player perspective script:" but it has absolutely NOTHING to do with Camera. Even I can see that at a glance. Name things properly. It's only professional.

    Or else, use Cinemachine for camera stuff. It's free from Unity.

    Do you have a dog? If so, call him over and sit him down and explain to him what each part of your code does.

    If any part of it doesn't make sense, your dog will bark, then give him a treat and go fix that, then call him back when it's better. For one, your code will get better, for two you will learn a lot, and for three, it's quality time with your dog. (See my avatar picture).

    I would also use a LOT more intermediate variables. You have some horribly hairy long lines of code in there. If I was reviewing your code for inclusion, I'd make you rewrite any line that had more than one or two dots in it.

    If you have more than one or two dots (.) in a single statement, you're just being mean to yourself.

    How to break down hairy lines of code:

    http://plbm.com/?p=248

    Break it up, practice social distancing in your code, one thing per line please.

    "Programming is hard enough without making it harder for ourselves." - angrypenguin on Unity3D forums

    ALSO: never use eulerAngles directly. Just don't. Here's why:

    https://starmanta.gitbooks.io/unitytipsredux/content/second-question.html