Script seems to only test distance against game objects in <List> that were added in inspector

Hi everyone. I'm stumped on this one. I have a script that is checking for overlapping gameobjects by testing position of a Vector3 against the current positions of all the existing game objects. If the new point's position is too close to an existing game object it will not instantiate a game object and try another point, if it's not close to any other existing gameobjects it will instantiate the game object. Once the new game object is instantiated, it will add that object's transform.position to the list of transforms.

The list changes after each successful instantiation and the list gets resorted so the closest game object's position should in theory be in the list[0] slot. It's working up to a point but the problem is that the code only seems to want to test the transforms that I've added in the inspector and not the transforms that were added via instantiation of game objects.

The list gets sorted correctly but I'm getting no love for the picking of the closest object's transform. I've been looking at this for awhile and I think a fresh set of eyes might see what I'm not.

Here's the code. Project file was too large to upload, but here's a link to it:

https://drive.google.com/open?id=19T5NkgRCu8YR7dGm1YkjnvBzIRmMirti

Code (CSharp):

1. using UnityEngine;
2. using System.Collections;
3. using System.Collections.Generic;
4.  
5. public class SortTest : MonoBehaviour
6. {
7.  
8.     public GameObject sphere;
9.     public List<Transform> spawnPoints;
10.  
11.     public Vector3 currentPosition;
12.     public float minX, maxX, minZ, maxZ;
13.     public float dist;
14.  
15.     public float minSpawnDistance;  //the minimum amount of space needed to spawn object so they don't intersect.
16.  
17.     public int currentNumberSpawned;
18.     public int maxNumberSpawned;
19.  
20.     public int count;//for adding a number to the end of the enemy spawns so I can tell the difference
21.  
22.     bool distanceToSmall = false;  //flag for the loop that will not generate a new random number if the distance was too small on the previous try
23.  
24.  
25.     void Start()
26.     {
27.         //I added in a couple of starter transforms to populate the list
28.         //spawnPoints = new List<Transform>();
29.  
30.         SpawnInitialEnemy();  //get the list started with one transform
31.  
32.         maxNumberSpawned = 15;
33.         count = 0;
34.  
35.         for(int i=0; i<maxNumberSpawned; i++)
36.         {
37.            GenerateSpawnPoint();
38.            Sort();
39.         }
40.     }
41.  
42.     public void GenerateSpawnPoint()
43.     {
44.         float spawnX = Random.Range(minX, maxX);  //create a random float for Xposition and...
45.         float spawnZ = Random.Range(minZ, maxZ);  //...Zposition
46.         currentPosition = new Vector3(spawnX, 0, spawnZ); //this is current spawn position to be tested against the rest of the spawn positions      
47.     }
48.  
49.     void Sort()
50.     {
51.         spawnPoints.Sort(delegate (Transform a, Transform b)  //compare all of the spawnPoints and...
52.         {
53.             return Vector3.Distance(currentPosition, a.transform.position).CompareTo(Vector3.Distance(currentPosition, b.transform.position));
54.         });
55.  
56.         foreach (Transform point in spawnPoints)
57.        
58.             Debug.Log(point.name);
59.  
60.             Vector3 tempLowValue = spawnPoints[0].position;  //...get the first value from the list which should be the closest object
61.             Debug.Log("Closest point is at" + tempLowValue);
62.             Debug.Log("Closest point name is" + spawnPoints[0]);
63.  
64.             dist = Vector3.Distance(currentPosition, tempLowValue); //check the distance between the currentPosition (position to test to see if it's OK to spawn at) and the closest already instantiated game object.
65.             Debug.Log("The low value is" + dist);
66.        
67.  
68.         if (dist > minSpawnDistance)  //if the distance is greater than the mindistance needed to spawn...
69.         {
70.             var newEnemy =Instantiate(sphere, currentPosition, Quaternion.identity); //...spawn in a new enemy
71.             newEnemy.name = "Top" + count; //rename the enemy
72.             Debug.Log("+++++++++++++++++++++++++++++++++++++++++++++++++++++++"+newEnemy.name);
73.  
74.             Transform addMeToList = sphere.transform;  //create a new transform so the newly instantiated enemy's transform can be added to the list
75.             spawnPoints.Add(addMeToList); //add the transform to the list.
76.             currentNumberSpawned++;  //add one to the current number of enemies spawned.
77.             count++;
78.         }
79.  
80.         else if (dist <= minSpawnDistance)
81.         {
82.             Debug.Log("__________________Can't spawn here, try again____________________");
83.             GenerateSpawnPoint();  //generate a new spawn point
84.            
85.         }
86.     }
87.  
88.     public void SpawnInitialEnemy()
89.     {
90.         var newEnemy = Instantiate(sphere, currentPosition, Quaternion.identity);
91.         newEnemy.name = "Top" + count;
92.         Debug.Log("+++++++++++++++++++++++++++++++++++++++++++++++++++++++" + newEnemy.name);
93.         Transform initialEnemy = sphere.transform;
94.         spawnPoints.Add(initialEnemy);
95.     }
96.  
97.     private void Update()
98.     {
99.        
100.  
101.     }
102.  
103.  
104. }
105.  

 

Line 74 looks problematic:

Transform addMeToList = sphere.transform;  //create a new transform so the newly instantiated enemy's transform can be added to the list

The comment suggests you are creating a new transform, but this code simply gets a reference to the transform on the variable "sphere", which seems to be the original prefab you are instantiating to create new enemies. I assume what you actually want is "newEnemy.transform".

Other notes:

A little above that, you have a foreach loop that iterates over the spawnPoints, but it has no curly braces {} so the loop only applies to the next line. Despite that, you have a bunch of lines under it indented. This is highly misleading and error-prone.

Sorting the entire list is overkill when all you care about is testing whether any element is within a given distance, but if you're only making 15 points it probably doesn't matter very much.

Your Sort() function is highly misleadingly-named.

GenerateSpawnPoint() probably shouldn't be public, since it doesn't look like it makes sense to call it except from inside this class. (Also, if I were writing it, I would have it return a value rather than setting a class variable.)

 

Thanks for taking a look at it Antistone! Some of this I wrote and some of it I borrowed from:

https://answers.unity.com/questions/1286957/sorting-a-list-by-distance-to-an-object.html

I saw the missing curly braces after the for loop but honestly didn't know what to make of it so I just left it. Didn't realize that not having the braces makes it only apply to the next line.

Would you just do a Vector3.Distance with all of the spawn points in the list and then check to see if any are less than the minimum distance, and if so, kill the loop and generate another point to test against the spawn points until a suitable point is found?

 

Thanks to Antistone's eyes and help it is now working. I'll post it here just in case anyone else has the same problem or need for a game.

Code (CSharp):

1. using UnityEngine;
2. using System.Collections;
3. using System.Collections.Generic;
4.  
5. public class SortTest : MonoBehaviour
6. {
7.  
8.     public GameObject enemy;
9.     public List<Transform> spawnPoints;
10.  
11.     public Vector3 currentPosition;
12.     public float minX, maxX, minZ, maxZ;
13.     public float dist;
14.  
15.     public float minSpawnDistance;  //the minimum amount of space needed to spawn object so they don't intersect.
16.  
17.     public int currentNumberSpawned;
18.     public int maxNumberSpawned;
19.  
20.     public int count;//for adding a number to the end of the enemy spawns so I can tell the difference between tops in hierarchy
21.  
22.     bool distanceToSmall = false;  //flag for the loop that will not generate a new random number if the distance was too small on the previous try
23.  
24.  
25.     void Start()
26.     {
27.         //If you want to manually start the list add a couple of transforms in the inspector, otherwise leave following line...
28.         spawnPoints = new List<Transform>();
29.  
30.         SpawnInitialEnemy();  //get the list started with one transform
31.        
32.         maxNumberSpawned = 25;
33.         count = 0;
34.  
35.         while (currentNumberSpawned < maxNumberSpawned)
36.         {
37.             GenerateSpawnPoint();
38.             Sort();
39.         }      
40.  
41.     }
42.  
43.     public void GenerateSpawnPoint()
44.     {
45.         float spawnX = Random.Range(minX, maxX);  //create a random float for Xposition and...
46.         float spawnZ = Random.Range(minZ, maxZ);  //...Zposition
47.         currentPosition = new Vector3(spawnX, 0, spawnZ); //this is current spawn position to be tested against the rest of the spawn positions      
48.     }
49.  
50.     void Sort()
51.     {
52.         spawnPoints.Sort(delegate (Transform a, Transform b)  //compare all of the spawnPoints and...
53.         {
54.             return Vector3.Distance(currentPosition, a.transform.position).CompareTo(Vector3.Distance(currentPosition, b.transform.position));
55.         });
56.  
57.             Vector3 tempLowValue = spawnPoints[0].position;  //...get the first value from the list which should be the closest object
58.             dist = Vector3.Distance(currentPosition, tempLowValue); //check the distance between the currentPosition (position to test to see if it's OK to spawn at) and the closest already instantiated game object.
59.        
60.         if (dist > minSpawnDistance)  //if the distance is greater than the mindistance needed to spawn...
61.         {
62.             var newEnemy = Instantiate(enemy, currentPosition, Quaternion.identity); //...spawn in a new enemy
63.             newEnemy.name = "Top" + count; //rename the enemy
64.            
65.             Transform addMeToList = newEnemy.transform;  //create a new transform so the newly instantiated enemy's transform can be added to the list
66.             spawnPoints.Add(addMeToList); //add the transform to the list.
67.             currentNumberSpawned++;  //add one to the current number of enemies spawned.
68.             count++;
69.         }
70.  
71.         else if (dist <= minSpawnDistance)
72.         {
73.             GenerateSpawnPoint();  //generate a new spawn point          
74.         }
75.     }
76.  
77.     public void SpawnInitialEnemy()
78.     {
79.         var newEnemy = Instantiate(enemy, currentPosition, Quaternion.identity);
80.         newEnemy.name = "Top" + count;
81.         Transform initialEnemy = newEnemy.transform;
82.         spawnPoints.Add(initialEnemy);
83.         currentNumberSpawned++;  //add one to the current number of enemies spawned.
84.     }
85.  
86.     private void Update()
87.     {
88.        
89.  
90.     }
91.  
92.  
93. }

 

Conceptually, yes. Though I'd probably call Find() instead of writing my own loop. Something like:

Code (CSharp):

1. Transform firstPointThatIsTooClose = spawnPoints.Find(
2.     (point) => (currentPosition - point.transform).sqrMagnitude < minimumDistanceSquared
3. );
4. if (firstPointThatIsTooClose != null)
5. {
6.     // something was too close
7. }
8. else
9. {
10.     // everything is far enough away
11. }

Sorting the list is a O(n log n) operation, whereas searching it is only O(n), which means searching it is faster, and the speed difference gets bigger the longer the list is. Also, sorting the list changes it, whereas searching it leaves it the same...and the less you change, the less chance you accidentally mess up some other piece of code using the same data structures.

Square distance is used instead of regular distance because it's faster to calculate. (Calculating the actual distance requires a square root function, which is expensive compared to most arithmetic.)

Those changes shouldn't affect the end result in this particular case, though.