Need Advice on my Scripting Practice

Ok, so I've been working on something new and in the past few weeks of learning and going through tutorials I've been overwhelmed by how much I've forgotten about since I was in school. What I'm asking from you vets is if my scripts seem to be in the direction I need to be going. I've been working on my character movement and attacking script one little bit at a time. The script is to move the character with user input, play sounds on attacking and animate if he is close enough to the victim npc. Can someone who is good at this stuff just give my code a quick look and tell me if I'm doing things well enough? All of the tutorials I look at show little scripts all being put together to make things happen in their games. I can't figure out a way to do it on this one. If there are ways to simplify what I'm doing I want to find out now before I get really involved with this project so I won't have to go back redo everything.

Here's a video of what the player does using this script:

Code (CSharp):

1. using UnityEngine;
2. using System.Collections;
3.  
4. public class CharacterMovementScript : MonoBehaviour {
5.  
6.    public float playerSpeed = 10f;     //sets player's speed
7.  
8.     private Vector3 moveDirection = Vector3.zero;   //initialized player movement direction values
9.     private Vector3 fallDirection = Vector3.zero;
10.     public float gravityValue = 1; //sets value for gravity for when the player is falling off edges 1 is slower, 5+ is very fast
11.  
12.     private bool canMove = true;          //used to disable movement while attacking.
13.     private bool attacking = false;        //for timer during attack
14.  
15.     private float WaitToAttack = 1.0f;     //0.917f;    //this MUST be set to length of attacking animation for smooth player input
16.  
17.     private float AttackTime = 0f;          //used for determining length of grabbing animation
18.     private float attackSpeedBoost = 1.5f;
19.     private bool eatVictim = false;         //used to determine code that enables eating actions
20.     private float eatingTime = 2.375f;       //match this with time it takes Player Zombie eating animation to finish
21.     private float distanceFromVictim = 5f;
22.     private float eatingAnimationTime = 2.2f;   //MUST match the time length of the eating people animation for smooth play
23.  
24.     Animator anim;  //animator object
25.     CharacterController controller;
26.     AudioSource audio;
27.     public AudioClip[] zombieTalk;  //initialize audio clips from inspector.
28.     private int clipNumbers;
29.  
30.     private int rayhits = 0;
31.     // Use this for initialization
32.  
33.     void Start () {
34.  
35.         clipNumbers = zombieTalk.Length - 2;    //reason for -2 is because the last 2 in the list are for eating people
36.  
37.         controller = GetComponent<CharacterController>();
38.         anim = GetComponent<Animator>();
39.         audio = GetComponent<AudioSource>();
40.  
41.         anim.SetBool("isWalking", false);
42.     }
43.  
44.     // Update is called once per frame
45.  
46.     void Update () {
47.  
48.         if(Input.GetKeyDown("escape")) Application.Quit(); // Quits the game when pressing Escape
49.  
50.         if (canMove) MovePlayer();       //calls function to move player in the direction of user input
51.      
52.         Attack();           //checks to see if player is attacking
53.  
54.     }
55.  
56.     void Attack()
57.     {
58.         if (Input.GetButtonUp("Fire1") && eatVictim == false && !attacking)
59.         {
60.             attacking = true;
61.             anim.SetBool("isAttacking", true);  //set isAttacking true so animation plays right
62.             PlayerAudio();
63.         }
64.  
65.        
66.  
67.         if (attacking)           //determine if currently attacking
68.         {
69.             //determines if still attacking within the timeframe of the animation (WaitToAttack)
70.             //WaitToAttack must be in sync with time it takes to animate the actual attack
71.             if (AttackTime < WaitToAttack)
72.             {
73.                 AttackTime += Time.deltaTime;
74.             }
75.             if (AttackTime >= WaitToAttack && eatVictim == false)
76.             {
77.                 anim.SetBool("isAttacking", false);
78.                 print("Attack finished");
79.                 canMove = true;
80.                 attacking = false;
81.                 AttackTime = 0f;
82.             }
83.  
84.             //check to see if InnocentHuman victim is within grab range. If so, begin eating
85.             Ray ray = new Ray(transform.position, transform.forward);
86.             RaycastHit hit;
87.             if (Physics.Raycast(ray, out hit, 0.4f))    //set 3rd variable to distance player has to be from victim before grabbing
88.             {
89.                 if (hit.collider.gameObject.tag == "InnocentHuman") //check if innocent human is in grasp
90.                 {
91.                     canMove = false;    //disable movement during attack
92.                     eatVictim = true;   //enable eating of the victim
93.                     int eatingAudioClipNumber = Random.Range(0, 2);   //get random one of the 2 eating clips
94.  
95.                     hit.collider.gameObject.GetComponent<BlondeGirlAI>().GettingEaten(); //call script on Blonde Girl to be eaten    
96.                     audio.Stop();   //stop audio currently playing
97.                     audio.PlayOneShot(zombieTalk[clipNumbers + eatingAudioClipNumber]); //play audio for eating victim
98.                     eatingTime = 0f;    //reset eatingTime to 0 for timer
99.                 }            
100.             }
101.  
102.             if (eatVictim)
103.             {
104.                 anim.SetBool("isEating", true);            
105.                 eatingTime += Time.deltaTime;
106.             }
107.  
108.             //timer to determine if eating animation has finished before proceeding
109.             if (eatingTime >= eatingAnimationTime && eatVictim == true)
110.             {
111.                 eatingTime = 0f;
112.                 eatVictim = false;
113.                 anim.SetBool("isEating", false);
114.             }
115.         }
116.     }
117.  
118.     void PlayerAudio()
119.     {
120.         if (!eatVictim)
121.         {
122.             //cycles through audio clip for the zombie attack
123.             int clipNumber = Random.Range(0, clipNumbers);
124.             audio.PlayOneShot(zombieTalk[clipNumber]);  //play currently selected audio clip for attacking
125.         }
126.     }
127.  
128.     void MovePlayer()
129.     {
130.         //grab movement direction depending on player input
131.         moveDirection = new Vector3(Input.GetAxis("Horizontal"), 0, Input.GetAxis("Vertical")); //get direction of movement
132.  
133.         if (moveDirection != Vector3.zero)
134.             //(moveHorizontal > 0 || moveVertical > 0 || moveHorizontal < 0 || moveVertical < 0)
135.         {
136.             anim.SetBool("isWalking", true);
137.             transform.LookAt(transform.position + moveDirection);  //look towards movement direction
138.  
139.             //move player in the direction retrieved
140.             controller.Move(moveDirection * Time.deltaTime * playerSpeed);
141.         }
142.         else
143.         {
144.             anim.SetBool("isWalking", false);
145.         }
146.  
147.         //check if player is not on the ground. if player is not on the ground, make player fall
148.         if(!controller.isGrounded)
149.         {
150.             fallDirection = new Vector3(0, -(gravityValue), 0);  //change movement direction to fall down
151.             controller.Move(fallDirection * Time.deltaTime);    //make player fall to the ground
152.         }
153.  
154.     }
155.  
156. }

 

Your script looks alright in my opinion.

A few tweaks would make it neater but what matters is it does what it needs to do.

 

I've been re-reading Terry Norton's Learning C# by Developing Games with Unity 3D. He takes a different approach, implementing a State Machine with an Interface and an implementing class for each state in the game. Update() and OnGUI() - yes, it's a bit dated - simply delegate to methods of the interface on whichever state is active at any time. It's a much more OO approach which I like a lot better. Well worth having a look at if you're interested in structuring your code.

 

Thanks for the replies. It seems I'm heading towards where I need to be.

 

Just wanted to pop in to say I love the audio.

 

I'd like to give a feedback as well. Personally, I think you should move your attack logic into another script. It seems easier to have everything in one script for convenience, but making, lets say, AttackScript with all your attacking/eating logic will allow you to make your code readable and reusable. It would look something like:

Code (CSharp):

1. public AttackScript attackBehaviour;
2.  
3. void Update () {
4.         if(Input.GetKeyDown("escape")) Application.Quit(); // Quits the game when pressing Escape
5.         if (canMove) MovePlayer();       //calls function to move player in the direction of user input
6.    
7.        if (attackBehaviour != null)
8.        {
9.               attackBehaviour.Attack();
10.        }
11.     }

You could then give your AttackScript to those running NPCs who were unfortunate to turn into zombies and make them attack other non-zombie NPCs. Think of the possibilities Also, you could easily turn on/off attacking just by enabling/disabling your attack component, let's say you have hunger meter; if you're full you can't attack other NPCs or something.