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

Movement script - horribly written?

Discussion in '2D' started by wlewis13, Jan 27, 2020.

  1. wlewis13

    wlewis13

    Joined:
    Jan 27, 2020
    Posts:
    3
    Here's a movement script I wrote for my Unity 2D project. Just wondering if there's a better way of doing this (in terms of gameplay and CPU usage).

    Code (CSharp):
    1. using System.Collections;
    2. using System.Collections.Generic;
    3. using UnityEngine;
    4.  
    5. public class Movement : MonoBehaviour {
    6.  
    7.     public float moveSpeed = 5f;
    8.     // A public constant moveSpeed to dictate how fast a player moves - bigger = faster. Public so it can be edited in Unity's inspector
    9.  
    10.     private bool facingRight;
    11.     // Indicates if the player is facing right
    12.  
    13.     void Start()  {
    14.             facingRight = true;
    15.     }
    16.     // Start is called before the first frame update
    17.  
    18.     public bool isGrounded = false;
    19.     // Whether or not the player is touching the ground - determines whether or not if the player can jump
    20.  
    21.     Rigidbody2D rb;
    22.     // References the player's rigid body 2D values
    23.  
    24.  
    25.     void Awake(){
    26.         rb = GetComponent<Rigidbody2D>();
    27.     }
    28.     // This hooks up the rb variable to the values received from the RigidBody2D component - called before first frame
    29.  
    30.     public float jumpVelocity;
    31.     // How fast the player jumps up
    32.  
    33.     public int jumpCounter;
    34.     // This allows for a maximum amount of height to be gained in one jump
    35.  
    36.     public Animator animator;
    37.  
    38.     // Update is called once per frame
    39.     void Update() {
    40.      
    41.     }
    42.  
    43.     void FixedUpdate() {
    44.         if (Input.GetButton("Jump")  && jumpCounter < 25) {
    45.             // If the player is pressing space and they still have jumping capacity left
    46.                 GetComponent<Rigidbody2D>().AddForce(Vector2.up * jumpVelocity, ForceMode2D.Impulse);
    47.                 // Manipulates the position of the player
    48.                 jumpCounter = jumpCounter + 1;
    49.                 // Reduces the amount the player can jump in later frames as they are jumping in this frame
    50.                 animator.SetBool("isJumping", true);
    51.                 // Sets the isJumping boolean to true if the player is jumping
    52.         } else if (isGrounded){
    53.             jumpCounter = 0;
    54.             // Resets jump counter to allow the player to jump again
    55.             jumpVelocity = 5f;
    56.             // Resets jumpVelocity so the next jump will have the same 'impulse' effect
    57.             animator.SetBool("isJumping", false);
    58.             // Sets the isJumping boolean to false if the player is falls back to the ground
    59.         }
    60.         if (Input.GetButton("Jump")) {
    61.             jumpVelocity = jumpVelocity - 0.3f;
    62.             // Decrementsthe jumping velocity because the player is being decelerated and pulled back down by gravity
    63.             // Resets the jumpCounter and jumpVelocity as soon as the player touches the ground again
    64.         }
    65.  
    66.         if (jumpCounter > 22 || (!isGrounded && !Input.GetButton("Jump")) && jumpCounter > 8) {
    67.             animator.SetBool("isFalling", true);
    68.         } else {
    69.             animator.SetBool("isFalling", false);
    70.         }
    71.      
    72.         float horizontal = Input.GetAxis("Horizontal");
    73.  
    74.         Flip(horizontal);
    75.  
    76.         Vector3 movement = new Vector3(Input.GetAxis("Horizontal"), 0f, 0f);
    77.         // This code constantly checks whether player is moving (every frame - because that's the most it needs to do)
    78.         // Receives input (A or D keys) for X axis (horizontal), creates a new Vector3 each frame update
    79.  
    80.         transform.position += movement * Time.deltaTime * moveSpeed;
    81.         // This adds the movement variable to the X axis value of the player, multiplied by Time.deltaTime (constant speed for varying framerate) & moveSpeed
    82.  
    83.         horizontal = horizontal * horizontal;
    84.         // Ensures horizontal is always positive
    85.  
    86.         animator.SetFloat("Speed", horizontal);
    87.         // Assigns the Float Speed the value held by horizontal
    88.         // Stationary: Speed = 0 / Moving: Speed > 0
    89.  
    90.     }
    91.     private void Flip(float horizontal){
    92.         if (horizontal > 0 && !facingRight || horizontal < 0 && facingRight) {
    93.         // If the horizontal > 0 (moving to the right) and facingRight is false OR horizontal < 0 and facingRight is true (moving to the left)
    94.         // During the first frame the player inputs A or D and they were previously facing the other way, this code is run
    95.             Vector3 refScale = transform.localScale;
    96.  
    97.             refScale.x *= -1;
    98.             // Times by negative one should flip the x values
    99.  
    100.             transform.localScale = refScale;
    101.             // Assigns the player's transform variables the refScale - the same but flipped
    102.          
    103.             facingRight = !facingRight;
    104.             // The facingRight flag is inverted as the character is now facing the opposite direction
    105.         }
    106.     }
    107. }
     
    Last edited: Jan 29, 2020
  2. MelvMay

    MelvMay

    Unity Technologies

    Joined:
    May 24, 2013
    Posts:
    11,321
    Please use code-tags when posting code otherwise it just becomes a wall fo text hard to read, thanks.

    Straight away though I can see problems:
    • You should use Input per-frame in "Update" and not "FixedUpdate".
    • You're using Time.timeDelta in FixedUpdate expecting per-frame time. Unity secretly makes this the fixed-time so it works when you call it from FixedUpdate (never liked this personally) but you should understand what's going on with your times.
    • You're modifying Transform.position which stomps over the whole reason having a Rigidbody2D one it which is to update the Transform pose. Never do this.
    • Even worse, you're dynamically scaling the whole GameObject with local scale. Consider if you can make your collider(s) symmetric along the flip axis and flip the rendering only.
     
    wlewis13 likes this.
  3. wlewis13

    wlewis13

    Joined:
    Jan 27, 2020
    Posts:
    3
    Thanks for the feedback, I understand what you're saying I'm just not sure on how to implement these improvements.

    I tried to change the movement script to use rb2d instead of transform, but the player became sticky on walls. Other stuff I don't know how to fix. As 2D platformer, the script works ok, even though under the hood it is very poorly written and assembled, as you pointed out.
    Screenshot (285).png Screenshot (286).png
     
  4. Cornysam

    Cornysam

    Joined:
    Feb 8, 2018
    Posts:
    1,452
    This will fix sticky walls:
     
    wlewis13 likes this.
  5. Cornysam

    Cornysam

    Joined:
    Feb 8, 2018
    Posts:
    1,452
    Also, to help with improving the structure, move all of your global variables to the top of your class under the Movement : MonoBehavior line (Where your moveSpeed and facingRight variables are). So move the isGrounded, Rigidbody2D, Animator, jumpCounter and jumpVelocity near moveSpeed and facingRight. DO NOT move your variables within the functions like FixedUpdate or Start, etc. because that is where you manipulate them, outside of functions is where you declare them. What you did isnt wrong, its just confusing to read. You want all of your variables at the top so you dont have to dig to find them when you come back to this script in 2 weeks, 2 months or 2 years.
     
    wlewis13 likes this.