Is this code any good?

Question

I recently starting working on a 2D turn based game like the first final fantasy. I created this script to create a random chance of battle when you are walking. It works, but I am still new to unity and programming and would like a second opinion on it and if you think there is a more optimal way to do it.

 using UnityEngine;
 using System.Collections;
 
 public class BattleStart : MonoBehaviour {
 
 
     public bool isWalking;
     public double currentTime;
     public float randomNumber;
     double newNumber;
 
 
     // Use this for initialization
     void Start () {
         newNumber = 1;
     
     }
     
     // Update is called once per frame
     void Update () {
         
         currentTime = Time.time;
 
         //Creates a new random number between 1 and 100 every half second
         if (currentTime > newNumber) 
         {
             randomNumber = Random.Range (1,100);
             newNumber = newNumber + 0.5;
         }
 
 
         // Determines if the player is walking
         if ((Input.GetKey (KeyCode.W)) || (Input.GetKey (KeyCode.A)) || (Input.GetKey (KeyCode.S)) || (Input.GetKey (KeyCode.D))) 
         {
             isWalking = true;
         } 
         else 
         {
             isWalking = false;
         }
 
 
         // When the player is walking, there is a chance of battle
         if ((isWalking == true) & (randomNumber >= 90)) // 10% of a battle
         {
             Application.LoadLevel (1); //Loads the battle scene
         }
         
     }
 }

Answer 1

Overall it looks good. A few things to consider;

Move your input check to a method that returns bool.
You dont need to generate and check random numbers when you are not Walking.
Use SceneManager instead. Application Load is the old method.

Here is my untested code:

 using System.Collections;
 using UnityEngine;
 using UnityEngine.SceneManagement;
 
 public class BattleStart : MonoBehaviour
 {
     private float nextCheck = 1f;
     private float interval = 0.5f;
 
     private void Update()
     {
         if (IsWalking())
             BattleStartCheck();
     }
 
     private void BattleStartCheck()
     {
         if (Time.time > nextCheck)
         {
             nextCheck = Time.time + interval;
 
             if (Random.Range(1, 100) >= 90)
                 SceneManager.LoadScene(1, LoadSceneMode.Single);
         }
     }
 
     private bool IsWalking()
     {
         if (Input.GetKey(KeyCode.W) ||
             Input.GetKey(KeyCode.A) ||
             Input.GetKey(KeyCode.S) ||
             Input.GetKey(KeyCode.D)
             )
             return true;
 
         return false;
     }
 }

Is this code any good?

1 Reply

Your answer

Follow this Question

Related Questions