I’ll start by explaining what I am trying to accomplish, then explain what goes wrong, and then finally post the script, at which point, hopefully someone can tell me why it went wrong.
I’m designing a first-person shooter level where there are eight buildings, in each of them there will be either one enemy or one ammo pickup, for a total of four enemies and four pickups. The idea was to make it random which of those things would be in a building, and I attempted to do that with a script that uses two lists. It goes down a list of objects, and for each of them, selects a Vector3 from a list of Vector3s, then deletes that Vector3 from the list and then sets the enemy’s or pickup’s position to that Vector3. Repeat until all are deployed.
Unfortunately, this doesn’t totally seem to work. When run the scene, some of them work, but others deploy in places a bit off from the points I specified, and sometimes two objects will warp to the same point. Meanwhile, the console prints this error, which I’m assuming is related: ObjectDisposedException: SerializedProperty placesToDeploy.Array.data[0] has disappeared!
Now here’s my script:
using System.Collections;
using System.Collections.Generic;
using UnityEngine;
public class SpawnRandomizer : MonoBehaviour
{
public List<GameObject> objectsToPlace = new List<GameObject>();
public List<Vector3> placesToDeploy = new List<Vector3>();
public int placeInt;
public Vector3 currentPlaceToDeploy;
public bool clickToTestDeletion;
// Start is called before the first frame update
void Start()
{
foreach (GameObject objectToPlace in objectsToPlace)
{
ChooseDeploymentPlace();
objectToPlace.transform.position = currentPlaceToDeploy;
}
}
// Update is called once per frame
void Update()
{
if (clickToTestDeletion)
{
clickToTestDeletion = false;
placesToDeploy.Remove(placesToDeploy[placeInt]);
}
}
void ChooseDeploymentPlace()
{
placeInt = Random.Range(0, placesToDeploy.Count);
currentPlaceToDeploy = placesToDeploy[placeInt];
placesToDeploy.Remove(placesToDeploy[placeInt]);
}
}
If you’re curious about the reference to “Spawning” in the title but nowhere else, it’s because I originally wrote this as a script to spawn prefabs, but that caused the enemies’ NavMeshAI to malfunction since the way the game was set up, the player was just dragged into their target box. I didn’t really feel like spending all of the time needed to completely rewrite a lot of code to do a search for a type or tag, so I gave up on prefabs, put all of the objects in the scene from the start and rewrote the code to just warp them to the Vector3s at the start.
If the scene has X different spots in it, get those into a collection (such as a Transform[ ] array).
Easiest is a public field you drag them into:
public Transform[] AllSpawnpoints;
When you run, do these things:
shuffle the above list (see below for shuffle algorithm)
iterate through the shuffled items and place your four enemies, then your four gold
That’s it.
Shuffle and pick (take once shuffle):
Essentially:
// shuffle
for (int i = 0; i < things.Length; i++)
{
int j = Random.Range(i, things.Length);
var t = things[i];
things[i] = things[j];
things[j] = t;
}
Thank you, but I think I need more clarification. Would I keep the PlacesToDeploy list so I can drag items from there to AllSpawnPoints, or does AllSpawnPoints totally replace PlacesToDeploy. As for that Shuffle and Pick thing, I don’t understand it much. Based on the snippet you gave me, I don’t see anything that keeps it from choosing the same option more than once. Maybe there is, or maybe that’s contained in the longer script linked on Github. But I still don’t get it yet.
@Th0masThe0bvious so if you have a list like this [A, B, C, D, E, F, G, H]
let’s say E, F, G, H are defined as ammo pickups
you shuffle the list by swapping the elements randomly, i.e. [G, C, A, F, H, E, D, B]
then you step through it and assign each effect to all buildings in order
this would effectively distribute the following
[ammo, enemy, enemy, ammo, ammo, ammo, enemy, enemy]
So 4 of each distributed randomly, with the added benefit of much simpler logic and not having to remove anything.
Pardon my describing it quite this way but there are fundamental flaws in your approach in this code. What I mean is it isn’t “almost working if we adjust it a bit” but rather built on sand.
I think you can agree that randomizing some locations shouldn’t throw an an exception and that Unity and/or C# can surely assign Vector3 values, etc. So look at the assumptions you’ve made and work from there. A common problem that everyone encounters at some point is modifying a collection (i.e. changing its length or order) while iterating it. Don’t do that. The runtime error more or less said “the element is no longer here”.
That’s why shuffling was suggested as an alternative. You don’t remove anything you shuffle the locations and walk the list and you get random locations.
But there is more (if you care to read on). Why is currentPlaceToDeploy a property? Public on top of it. You needed a way to Choose and get a value an you seem to have opted for setting a property. That property lives on after Start runs and nobody needs it. So maybe GetDeploymentPlace and have it return the Vector3. And placeInt is local to the Choose method and again nobody else needs to know about it.
Seemingly (but it depends upon your use case) some property in the objectsToPlace array could contain the placeToDeploy. You wouldn’t have to insure that the number of objects equaled the number of objects for one benefit and each object would simply transform itself.
In any case I hope this helps (a bit). Step back once in awhile and ask “why does this seem so difficult to do” and you will find more often that not that you’ve made it difficult.
I redid the code to reflect Dekker’s suggestion, and now there’s no errors. I found out what was wrong with the positioning, too; the culprit was the NavMesh Agent component on the enemies. I solved this by setting them inactive in the editor, and then amending the code so it repositioned them and then set them active.
