Need help with c# script!

So i am developing a parkour game.
I need a script which i can use to detect the closest object to the player with the "parkourable" tag
Here is my code:

Code (CSharp):

1. using UnityEngine;
2. using System.Collections;
3.  
4. public class Vault : MonoBehaviour {
5.     private float distance;
6.     public GameObject closest;
7.     private GameObject gos;
8.     private GameObject closestObject;
9.  
10. void Awake(){
11.     InvokeRepeating("FindClosestObject", 0.5f,0.5f);
12. }
13. GameObject FindClosestObject() {
14.     gos = GameObject.FindGameObjectsWithTag("Vaultable");
15.     distance = Mathf.Infinity;
16.     foreach (GameObject go in gos) {
17.         Vector3 diff = go.transform.position - transform.position;
18.         float curDistance = diff.sqrMagnitude;
19.         if (curDistance < distance) {
20.             closest = go;
21.             distance = curDistance;
22.         }
23.     }
24.     return closestObject;
25.   }
26.  
27. }

 

Sounds like fun. But, do you have a question?

 

The script is not working it gives the error:

 

Ah. First, you should be able to select and copy the error as text, without having to take a screen shot. That'll save some time in the future.

So let's look at the error. It's telling you that on line 14, it can't convert a GameObject[] (i.e., a GameObject array). into a GameObject. So look at line 14:

Code (CSharp):

1.     gos = GameObject.FindGameObjectsWithTag("Vaultable");

FindGameObjectsWithTag returns an array of GameObjects. But you've declared gos as a (single) GameObject. Thus the error.

So you can fix that by simply changing your declaration of gos to GameObject[], which is the type suggested in the error.

While you're at it, you probably shouldn't have gos be a property. It would work fine as a local variable. Or you could use no variable at all... just change line 16 to:

Code (CSharp):

1.     foreach (GameObject go in GameObject.FindGameObjectsWithTag("Vaultable")) {

and then delete gos entirely.

Cheers,
- Joe

 

Thank you for the help but now it gives:
Assets/Scripts/Vault.cs(16,29): error CS0136: A local variable named `go' cannot be declared in this scope because it would give a different meaning to `go', which is already used in a `parent' scope to denote something else

 

Alright i fixed that but i have another error:
Assets/Scripts/Vault.cs(12,12): error CS0161: `Vault.FindClosestObject()': not all code paths return a value

Code (CSharp):

1. using UnityEngine;
2. using System.Collections;
3.  
4. public class Vault : MonoBehaviour {
5.     private float distance;
6.     public GameObject closest;
7.     private GameObject closestObject;
8.  
9. void Awake(){
10.     InvokeRepeating("FindClosestObject", 0.5f,0.5f);
11. }
12. GameObject FindClosestObject() {
13.         foreach (GameObject go in GameObject.FindGameObjectsWithTag("Vaultable")) {
14.     distance = Mathf.Infinity;
15.         Vector3 diff = go.transform.position - transform.position;
16.         float curDistance = diff.sqrMagnitude;
17.         if (curDistance < distance) {
18.             closest = go;
19.             distance = curDistance;
20.         }
21.     return closestObject;
22.   }
23. }
24. }

 

Y

You probably messed up your code a bit.

1) You always return on the first run in your loop, that's probably not what you want.
(That's also why you get the latest error, you do not have anything to return when the array is empty/loop is not run)
2) you never (re-)assign 'closestObject', maybe you wanted to return 'closest' ?
3) foreach has some cons, consider using normal for-loops (not a must, but many people recommend that due to how it works 'internally / behind the scenes')
4) undo the FindGameObjectsWithTag(...) change you've made and get the array before you start the loop

 

OK, I hate to quibble, but I can't let folks get confused by misinformation here... point 4 is just wrong; there is no benefit to getting the array before you start the loop, as opposed to doing it in the foreach (which does, in fact, get the array before the loop; it just does it with cleaner code and less opportunity for error).

And I don't believe point 3 either. The difference between for and foreach is so tiny, it's really not worth worrying about. But if you feel you must worry about it, then check out this post, which does a decent job explaining and measuring the difference. Note that the for loop is ever so slightly faster in a trivial loop that does nothing but add up a bunch of numbers (which you could probably do better using Sum), but in an only slightly more realistic loop, where you access the array item twice instead of once, foreach is actually (slightly) faster. The code we're considering here also accesses the array item twice, so foreach will probably be faster.

But again, the difference is so slight that to worry about it either way is a textbook case of premature optimization. Except when the performance difference is great, you should always write your code in the way that is clearest, simplest, and hardest to screw up. That's where foreach is clearly superior in most cases. If benchmarking shows a performance hotspot that can be improved with more complex code, then you complicate that code. But not before.

My $0.02,
- Joe

P.S. Points 1 and 2 are spot-on though!

 

As for 3, as already stated in my post it's just meant to be a hint, other people recommend that, i even use it myself sometimes for quick and easy loops.
Although foreach has been improved alot in performance, it can still be a good idea to go with the for loop because you've got 'total freedom' with the direct access of the elements, e.g. you cannot re-assign the iteration variable in a foreach loop which is sometimes pretty annoying when you get back to your script and find out you have to re-write everything again.

As for 4, it was meant to be for the case he switched to the for loop. However, personal preference or not when using foreach in this case, you cannot tell it's not clean code if you set a reference in a situation like this as you may want to have it sooner or later when something does not work as expected or if you need to do anything else with it anyways.

 

Thanks for the help from both of you!