1
\$\begingroup\$

My goal was to create a big invisible cube and inside that spawn small random cubes.

These small cubes would have:

  • A random position within the big cube.
  • A random colour.
  • A random size.
  • A random lifetime.
  • A higher chance to spawn with offset of 0 on y-axis than any other offset.
  • Ability to rotate in sync with the big cube.
  • Ability to fade away as it ages.
  • To be replaced with new random cubes as they expire.

To achieve this, here is the code I wrote:

SpawnCube.cs

    public GameObject smallCubePrefab;
    public float rateOfSpawn = 1;
    private float nextSpawn = 0;

    // Update is called once per frame
    void Update()
    {
        // Spawn new cubes at specified spawn rate
        if (Time.time > nextSpawn)
        {
            nextSpawn = Time.time + rateOfSpawn;
            StartCoroutine(FadeOutCube());
        }
    }

    public List<GameObject> SpawnSmallCubes()
    {
        // Create an empty list of cubes
        List<GameObject> cubesList = new List<GameObject>();

        // Spawn cube at random position within the big cube's transform position
        Vector3 randPosition = new Vector3(Random.Range(-1f, 1f), 0, Random.Range(-1f, 1f));

        // Generate higher chance (2 chances in 3) of spawning small cubes with offset = 0 on Y-axis 
        List<float> randomY = new List<float>() {0, 0, Random.Range(-1f, 1f)};
        randPosition.y = randomY[Random.Range(0, 3)];
        randPosition = transform.TransformPoint(randPosition * .5f);

        // Spawn small cube
        GameObject smallCube = Instantiate(smallCubePrefab, randPosition, transform.rotation);

        // Give random color
        smallCube.GetComponent<Renderer>().material.color = Random.ColorHSV(0f, 1f, 1f, 1f, 0.5f, 1f);

        // Give random size
        int randSize = Random.Range(1, 10);
        smallCube.transform.localScale = new Vector3(randSize, randSize, randSize);

        // Add spawned cube to the list of cubes
        cubesList.Add(smallCube);

        return cubesList;
    }

    public IEnumerator FadeOutCube()
    {
        // Give random lifetime
        float fadeSpeed = Random.Range(0.01f, 0.05f);
 
        List<GameObject> smallCubes = SpawnSmallCubes();

        foreach (GameObject cube in smallCubes.ToArray())
        {
            while (cube.GetComponent<Renderer>().material.color.a > 0)
            {
                Color cubeColor = cube.GetComponent<Renderer>().material.color;
                float fadeAmount = cubeColor.a - (fadeSpeed * Time.deltaTime);

                cubeColor = new Color(cubeColor.r, cubeColor.g, cubeColor.b, fadeAmount);
                cube.GetComponent<Renderer>().material.color = cubeColor;
                yield return null;
            }
            if(cube.GetComponent<Renderer>().material.color.a <= 0)
            {
                Destroy(cube);
                smallCubes.Remove(cube);
            }
        }
    }  

Rotate.cs

public Vector3 rotationSpeed;

    // Update is called once per frame
    void Update()
    {
        transform.Rotate(rotationSpeed * Time.deltaTime);
    }

Any suggestions for improvement or changes to make it better in any way?

Thanks

\$\endgroup\$
3
  • 1
    \$\begingroup\$ Please do not update the code in your question to incorporate feedback from answers, doing so goes against the Question + Answer style of Code Review. This is not a forum where you should keep the most updated version in your question. Please see what you may and may not do after receiving answers. With a new question isn't meant to just copy & paste the questions context but to explain what you have changed after receiving an answer. \$\endgroup\$ – Heslacher Jan 26 at 16:37
  • \$\begingroup\$ @Heslacher Ah I understand. I actually have now quite a bit updated my code for SpawnCube.cs so would it be okay for me to edit that on this? \$\endgroup\$ – AaySquare Jan 26 at 17:04
  • 1
    \$\begingroup\$ No, because answers could be invalidated. I would suggest to wait at least another day because answers on Code Review will come not at a fast rate because it takes some time to dig through the code and write an answer. If by tomorrow there had been no additional answer, feel free to ask a follow-up question in which you should link back to this question. In the follow-up question you should implement the answers to which you agree \$\endgroup\$ – Heslacher Jan 26 at 17:14
2
\$\begingroup\$
  • Why does SpawnSmallCubes() generate only a single cube?
  • if(cube.GetComponent<Renderer>().material.color.a <= 0) this line is unecessary
  • GetComponent() calls can be reduced by holding onto the Renderer instance, assuming you aren't swapping it out
  • smallCubes.ToArray() conversion is unecessary
  • FadeOutCube() also spawns the cubes. This isn't obvious from the naming
\$\endgroup\$
5
  • \$\begingroup\$ SpawnSmallCubes() generates single cube as I am currently spawning cubes at a specified spawn rate in the Update function as you can see. However, if I were to remove spawn rate, the cubes would just spawn at a rapid rate. I was thinking I could maybe change this so that for example 5 random cubes spawns at the same time, and then once those 5 cubes disappear, I'd spawn another 5 and this would keep repeating... Not sure how to go about that though. \$\endgroup\$ – AaySquare Jan 24 at 18:29
  • \$\begingroup\$ I am doing the smallCubes.ToArray() conversion because if I don't, Unity throws an error "Collection was modified; enumeration operation may not execute." at foreach (GameObject cube in smallCubes) in the FadeOutCube() function. \$\endgroup\$ – AaySquare Jan 24 at 18:31
  • \$\begingroup\$ smallCubes.Remove(cube); is unecessary, as you only iterate over the list once anyways \$\endgroup\$ – Ted Brownlow Jan 24 at 18:42
  • \$\begingroup\$ Ah I understand. So about the SpawnSmallCubes(), how would you recommend I make the change such that 5 random cubes get spawned at the start rather than just one. And once they all expire, another set of 5 random cubes should spawn. \$\endgroup\$ – AaySquare Jan 24 at 19:01
  • \$\begingroup\$ You're already using a list, so putting 5 elements in the list with a for loop should work \$\endgroup\$ – Ted Brownlow Jan 24 at 19:55
0
\$\begingroup\$

You should break out your Random Generators into their own objects and use the Next() method.

    // Spawn cube at random position within the big cube's transform position
    Vector3 randPosition = new Vector3(Random.Range(-1f, 1f), 0, Random.Range(-1f, 1f));

Then becomes something like this

var randomVectorValue = new Random.Range(-1f, 1f);
var randPosition = new Vector3(randomVectorValue.Next(), 0, randomVectorValue.Next())

please bear in mind that I am not using an IDE to write this code so the syntax may be slightly off

Doing it like this makes the values of the Random Number generator more maintainable, what if you were calculating random numbers for several things all with the same range, you wouldn't want to go through the entire code and change it each time it would be easier to change it in one location, in this case it would be randomVectorValue (unlikely, because of the way that vectors work, but if it were for something else that had a range of 1-10 and you wanted to change it to 5-25...)

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.