A few concerns with my code...

Unity Engine
1

Hi! I need help cleaning up my code so I can be sure that it works properly and efficiently. I have been doing research and kind of grabbing things from various examples and it isn’t the best right now. I think most of my confusion comes from arrays and how to interact with them. They are probably very simple answers.

I currently have a node based cover system that works pretty well. The AI finds the game objects tagged as “cover” and does various tasks such as shooting raycasts to targets and measuring distances then assigning point values to pick the “best” one.

The problem is that I have to manually place each cover node by hand.

I decided to try and have the AI spawn a grid of nodes around them.

I am making a grid of points like this

using UnityEngine;
using System.Collections;

public class GridMaker : MonoBehaviour 
{
    public int x = 5;
    public int z = 5;

    public float spacing = 5f;

    public float upDistance = 2f;

    public GameObject prefab;


    void Start()
    {
        Vector3 pos = (transform.position);
        GameObject[,] my2DArray = new GameObject[x,z] ;
        for(int i = 0;i <x;i++)
        {
            for(int j = 0;j < z;j++)
            {
                my2DArray[i,j] = Instantiate(prefab, new Vector3(j * spacing + pos.x + spacing, pos.y + upDistance, i * spacing + pos.z + spacing), transform.rotation) as GameObject;
                my2DArray[i,j] = Instantiate(prefab, new Vector3(j * -spacing + pos.x, pos.y + upDistance, i * -spacing + pos.z), transform.rotation) as GameObject;
                my2DArray[i,j] = Instantiate(prefab, new Vector3(j * spacing + pos.x + spacing, pos.y + upDistance, i * -spacing + pos.z), transform.rotation) as GameObject;
                my2DArray[i,j] = Instantiate(prefab, new Vector3(j * -spacing + pos.x, pos.y + upDistance, i * spacing + pos.z + spacing), transform.rotation) as GameObject;
            }
        }
    }
}

I want them to be along the ground so I made a separate script that shoots rays and moves the position to the impact. I attached this script to the cover node game object and execute it during the start function.

it just looks like this

void Start()
    {
        Vector3 dir = transform.TransformDirection(-Vector3.up);

        RaycastHit hit;
        if (Physics.Raycast(transform.position, dir, out hit, downRange))
        {
            transform.position = hit.point;
        }

        StartCoroutine (Kill ());
    }

The first thing I am wondering is if there is a way I can just make the objects do this without having to do it on a separate script.

The next thing I am wondering is this.
I plan on merging the grid creator script within my current GetCover.

After the grid is created I am using FindObjectsWithTag to create an array that is used for “sorting” and finding the best one.

Two things concern me. I am wondering if I have to keep the scripts above separate (the generator and the ground finder) if when the array goes to sort the nodes, it will execute before the nodes have found the ground, shooting raycasts and measuring distances from the incorrect spot. I want to make sure this does not happen.

The second issue that I have is the FindObjectsWithTag, I am wondering how I would go about it so the AI only uses its own generated grid and doesn’t use another’s if multiple AI’s are trying to find cover at the same time. I have the nodes set up to only last five seconds but that is more than enough time to possibly kill performance.

I know it’s a lot of questions, but can anyone help me out please?

2

First, I’m just gonna give you scattershot advice as I notice things in your first code block (the second one looks fine).

First thing I notice about your code: you are spawning four nodes for every spot on the grid! That can’t be right, can it? I honestly can’t say whether it is or not because I don’t have any idea what the other three are intended to do. However, I’m guessing something isn’t right, because in your 2D array, you’re overwriting the same grid slot 4 times, and the first three are going to be lost to the ether. (Though, since the array itself is a local variable, that’s going to be lost to the ether anyway as soon as Start() finishes. In which case, why do you have an array at all?)

Second thing: Make your code more self-documenting by using good variable names. “Prefab” is not a good variable name; “nodePrefab” is better. “my2DArray” is bad; “nodeArray” would be better. (I’m mostly guessing that that’s what those things are, but you get the idea.

Third: Use

nodeArray[i,j] =Instantiate<GameObject>(....);

Fourth: Fewer lines is not automatically better. It looks like you tried to cram everything into one line for those Instantiate calls, and it makes it very difficult to read and to spot problems. Go ahead and separate out the Vector3 declaration to its own line (possibly with a descriptive variable name!), or in this case, maybe even put it on several lines:

Vector3 newPosition = new Vector3(
j * spacing + pos.x + spacing, 
pos.y + upDistance, 
i * spacing + pos.z + spacing);
my2DArray[i,j] = Instantiate<GameObject>(prefab, newPosition, transform.rotation);

Or, at the very least, insert tabs in between the parameters of the Vector3() constructor, so that your X’s, Y’s, and Z’s all line up.

Space is not your enemy. Tabs and newlines are free. Use them well to make your code more organized!

Answers to your specific questions in the next comment…

3

You’ll need a script to do that, but you could put that in any script - that functionality could be put into any script. In general, though, it’s best to put functionality that doesn’t need to interact with the rest of a script into its own script, so that your code can be more modular.

For starters, FindObjectsWithTag is pretty slow and not especially reliable. Instead, I’d use a List and create the array as you spawn the nodes.

...
using System.Collections.Generic; // add this to the top of your file
...
List<YourNodeClass> allNodes = new List<YourNodeClass>();
...
GameObject spawnedNodeObject = Instantiate<GameObject>(prefab, newPosition, transform.rotation);
YourNodeClass thisNode = spawnedNodeObject.GetComponent<YourNodeClass>();
allNodes.Add(thisNode);

And after that, you’ll have a ready-made list of Node objects. This is faster, and gives you the node object directly, not just the GameObject it’s on.

(I don’t know for sure, but I’m assuming that you have a Node class which is attached to your node prefab. If not, you should do that! Even if the nodes are idle and inactive, custom classes are a great way to store information, plus a convenient way to reliably find certain types of objects. Also, and this is largely personal taste I admit, I hate tags.)

And this is a prime reason not to use tags! (Full disclosure: I typed the above paragraph before I actually read this part of your question. Hating tags is so smart it’s prophetic.) If you have a Node class, this class can contain a reference to the enemy that created it. Better yet, the enemy should contain a List of the nodes that it spawned, so that you can loop through only those.

4

One final note: It’s almost always inefficient to have each enemy do its own “finding cover” algorithm, ignorant of what the other enemies are doing. Most such systems will have a global “here are some spots where cover is” manager class, and the enemies will reference this. That way your enemies are processing the same area multiple times. That kind of logic is a bit too complex to get into here though; for now, let’s just perfect the algorithm you came up with.

5

Thank you so much for responding StarManta! Im going to have to rest awhile before chewing through your answers because right now my brain is fried lol.

I just wanted to address the 4 node spawning thing. I made it spawn 4 times by multiplying by the -spacing variable in some and the normal spacing in others. This was to create one large grid with the enemy in the center by spawning 4 small grids around him. Without doing this it only creates a large grid with the enemy in one corner of it.

Is my code still spawning multiple nodes per grid point or is there a better way of doing this?

6

There are about a million ways of doing this, and some of them are better. It’s tough to say more without knowing more about your game. Have you considered a radial spawning pattern centered around the enemy? You could generate the X and Y coords use Mathf.Sin and .Cos. The grid would surround your enemy in a pretty organic way.