Is there a better way of doing this?

I was wondering if there was a better way of writing this code?

using System.Collections;

using System.Collections.Generic;

using UnityEngine;

using UnityEngine.InputSystem;

public class PlayerMovement : MonoBehaviour

{

[SerializeField] float runSpeed = 7f;

[SerializeField] float jumpSpeed = 20f;

[SerializeField] float climbSpeed = 5f;

[SerializeField] bool isAlive = true;

[SerializeField] GameObject fireBall;

[SerializeField] Transform castFireBall;

CapsuleCollider2D playerBodyCollider;

BoxCollider2D playerFeetCollider;

float gravityScaleAtStart;

Vector2 moveInput;

Rigidbody2D playerRigidbody;

Animator playerAnimator;

    void Start()

{

    playerRigidbody = GetComponent<Rigidbody2D>();

    playerAnimator = GetComponent<Animator>();

    playerBodyCollider = GetComponent<CapsuleCollider2D>();

    gravityScaleAtStart = playerRigidbody.gravityScale;

    playerFeetCollider = GetComponent<BoxCollider2D>();

}



    void Update()

{

    if(!isAlive)

    {

        return;

    }

    Run();

    FlipSprite();

    IsJumping();

    Climbing();

    Die();

}

    void OnFire(InputValue value)

{  

    playerAnimator.SetTrigger("isCastingFire");

}

public void FireBallAttack()

{

   

    if(!isAlive)

    {

        return;

    }

    Instantiate(fireBall, castFireBall.position, transform.rotation);

}

    void OnMove(InputValue value)

{

    if(!isAlive)

    {

        return;

    }

    moveInput = value.Get<Vector2>();

}

    void OnJump(InputValue value)

{

    if(!isAlive)

    {

        return;

    }

   if (!playerFeetCollider.IsTouchingLayers(LayerMask.GetMask("Ground")))

   {

       return;

   }

    if (value.isPressed)

    {

        playerRigidbody.velocity += new Vector2 (0f, jumpSpeed);

    }

}

    void Run()

{

    Vector2 playerVelocity = new Vector2 (moveInput.x * runSpeed, playerRigidbody.velocity.y);

    playerRigidbody.velocity = playerVelocity;

    bool playerHasHorizontalSpeed = Mathf.Abs(playerRigidbody.velocity.x) > Mathf.Epsilon; //Find the velocity of your movement

   

    if (playerHasHorizontalSpeed)

    {

        playerAnimator.SetBool("isRunning", true);

    }

    else

    {

        playerAnimator.SetBool("isRunning", false);

    }

}

    void FlipSprite()

{

    bool playerHasHorizontalSpeed = Mathf.Abs(playerRigidbody.velocity.x) > Mathf.Epsilon; //Find the velocity of your movement

    if (playerHasHorizontalSpeed)

    {

    transform.localScale = new Vector2 (Mathf.Sign(playerRigidbody.velocity.x),1f);

    }

}

    void Climbing()

{

    if (!playerFeetCollider.IsTouchingLayers(LayerMask.GetMask("Climbing")))

    {

        playerRigidbody.gravityScale = gravityScaleAtStart;

        return;

    }

    Vector2 climbVelocity = new Vector2 (playerRigidbody.velocity.x, moveInput.y * climbSpeed);

    playerRigidbody.velocity = climbVelocity;

    playerRigidbody.gravityScale = 0f;

   

}

    void IsJumping()

{

    if (!playerFeetCollider.IsTouchingLayers(LayerMask.GetMask("Ground")))

   {

       playerAnimator.SetBool("isJumping", true);

   }

   else

   {

       playerAnimator.SetBool("isJumping", false);

   }

}

    void Die()

{

    if (playerBodyCollider.IsTouchingLayers(LayerMask.GetMask("Enemies", "Hazards")))

    {

        playerBodyCollider.enabled = false;

        isAlive = false;

        playerAnimator.SetTrigger("Dying");

        //FindObjectOfType<GameSession>().ProcessPlayerDeath();

    }

}

   void CheckLives()

{   // This function is set as an event after the animation of dying is over. it's a key frame

    // this makes it so the animation plays out befor reloading the secne and taking a life from the "GameSession" script

    FindObjectOfType<GameSession>().ProcessPlayerDeath();

}

    void OnTriggerEnter2D(Collider2D other)

{

   if (other.tag == "Exit")

   {

    //StartCoroutine(LoadNextLevel(1f));

    LoadingNextLevel();

   }

}

public void LoadingNextLevel()

{

    playerAnimator.SetTrigger("PlayerWin");

    FindObjectOfType<LevelExit>().test();

}

}

new C# code
using System.Collections;

using System.Collections.Generic;

using UnityEngine;

using UnityEngine.SceneManagement;

public class LevelExit : MonoBehaviour

{

[SerializeField] float levelLoadDelay = 1f;

void OnTriggerEnter2D(Collider2D other)

{

   if (other.tag == "Player")

   {

    ///StartCoroutine(LoadNextLevel(10f));

   }

}

public void test()

{

   StartCoroutine(LoadNextLevel(10f));

}

IEnumerator LoadNextLevel(float waitTime)

{

Debug.Log("TEST");

yield return new WaitForSecondsRealtime(levelLoadDelay);

int currentSceneIndex = SceneManager.GetActiveScene().buildIndex;

int netxSceneIndex = currentSceneIndex +1;

if (netxSceneIndex == SceneManager.sceneCountInBuildSettings)

{

    netxSceneIndex = 0;

}



FindObjectOfType<ScenePersist>().ResetScenePersist();

SceneManager.LoadScene(netxSceneIndex);

}

}

this actually works, the thing I want to do is have my character d \o a winning animation then load to the next level. I just think it’s a little bit too much creating more classes just to call other classes.

basically in the Player Move class it’s this code .

void OnTriggerEnter2D(Collider2D other)

{

   if (other.tag == "Exit")

   {

    //StartCoroutine(LoadNextLevel(1f));

    LoadingNextLevel();

   }

}

public void LoadingNextLevel()

{

    playerAnimator.SetTrigger("PlayerWin");

    FindObjectOfType<LevelExit>().test();

}

Hi Bdragon,

In this course, we follow the principles of object-oriented programming (OOP) if possible. The single responsibility principle is one of the most important principles. That’s why you often see one class for each task/topic. In large projects, this approach will make debugging easier because you usually would not have to guess or read walls of code to find a problem.

In many cases, the structure of the code is just a matter of personal preference. If you prefer less classes, feel free to refactor your code. :slight_smile:


See also:

I’m sorry I misspoke, I meant to say there seems to be a lot of creating more functions to call other functions. as I provided in the above code. Like for an “OnTriggerEnter2D” do you need to create another function to call for an animation to be over? also the other functions you can not call directly Like “OnFire” , “OnMove” just seems like more Functions need to be created in order to call from one place of the application.

OnFire and OnMove are very likely event methods. I can see that because of the “On” prefix. These methods get called when a specific event happens. We need those methods.

As far as I know, there is no event method for the end of an animation. However, you could create your own animation event and a method which gets called.

HI Nina,
you have referred me to this topic before. what I am saying is that you do have to create your own method to call a certain method like “Onfire” from my experience. I just want to know is there a cleaner way of doing this?

Thank you
Trevor

That’s right. If you want to call a method, you need to create it first since non-existent methods cannot be invoked. That’s how C# works. :slight_smile:

However, “clean” does not make much sense in this context. If you want to figure out if there is a “cleaner” way for your current approach, you’ll have to define your requirements for “clean” because “cleaner” presupposes a “clean” way. Without a definition of “clean”, it is impossible to understand “cleaner”. In most cases, if you define your ideas/concepts/problems, you will be able to find a solution based on your definition. The vaguer your questions are, the less likely it is that you’ll find an answer.

1 Like

forget it neverminded.

If you would like to discuss this subject, please feel free to join our helpful community of students over on our Discord chat server.

Privacy & Terms