0
\$\begingroup\$

I created 10000 array and assigned value 1 and 0 to it, then used gizmos to generate 10000 cubes. I paint it black or white depending on the assigned value.

After that I use the bubble sort algorithm to sort the cubes, expecting all black cubes pieled on the bottom but when I run it, the sorting does not happen and I can't spot where my mistake is.

    using UnityEngine;

public class NewBehaviourScript : MonoBehaviour
{
    int x;
    int y;
    int z;
   
    int[,] array1 = new int[100, 100];
    int temp;
    // Start is called before the first frame update
    void Start()
    {



        for (x = 0; x < 100; x++)

        {
            for (y = 0; y < 100; y++)
            {
                array1[x, y] = Random.Range(0, 2);

                Debug.Log(string.Format("{0},{1},{2}", x, y, array1[x, y]));

            }
        }
        OnDrawGizmos();


    }
    void OnDrawGizmos()
    {
        
        for (x = 0; x < 100; x++)
        {
            for (y = 0; y < 100; y++)
            {
                Vector2 pos1 = new Vector2(0 + x, 0 + y);
                Gizmos.DrawCube(pos1, transform.position);
                Gizmos.color = (array1[x, y] == 1) ? Color.black : Color.white;
            }
        }
    }

    void update()
    {
        for (z = 0; z < 100; z++)
        {
            for (x = 0; x < 100; x++)
            {
                for (y = 0; y < 100 - z; y++)
                {
                    if (array1[x, y] > array1[x, y + 1])
                        temp = array1[x, y];
                    array1[x + 1, y] = array1[x, y];
                    array1[x, y] = temp;

                }
            }
        }
        OnDrawGizmos();
    }
}
\$\endgroup\$
3
\$\begingroup\$

You have multiple obvious errors in this code.

  1. You are setting the colour to draw the cube after you have drawn it. That will apply its colour to the next cube you draw, not the one you just drew. Set the colour before you draw the cube.

  2. Your bubble sort is comparing the cube with its neighbour above (x, y + 1), but then copying its value to the neighbour beside (x + 1, y).

  3. You're copying the value and overwriting what was in the neighbouring spot, not swapping it as a bubble sort should.

  4. You're using bubble sort. You're spending 100 * 100 * 100 = 1 000 000 loop iterations on a problem that you can solve a hundred times faster. You're also repeating this sort every frame, when (if you'd done the sort correctly), the results won't change after you've done it once.

  5. Or rather, you would be repeating the sort every frame, if you'd spelled Update correctly with an uppercase U. With a lowercase u, this method won't get called by the engine.

  6. You're calling OnDrawGizmos manually. This is called by the engine - you should not call it yourself.

  7. You're using member variables for things that can easily be temporaries.

You can do this much more efficiently:

using UnityEngine;

public class CubeSorter : MonoBehaviour
{
    bool[,] cubeIsBlack = new bool[100, 100];


    // Start is called before the first frame update
    void Start()
    {    
        for (int x = 0; x < 100; x++)    
        {
            for (int y = 0; y < 100; y++)
            {
                cubeIsBlack[x, y] = (Random.Range(0, 2) > 0);
    
                Debug.Log($"{x}, {y}, {cubeIsBlack[x, y]}");    
            }
        }

        // We'll sort just once, not every frame.
        Sort();
    }


    // This sorts all the black cubes to the bottom of the column.
    // You can swap x & y to sort along rows instead.
    void Sort() {
        for (int x = 0; x < 100; x++) {
            // Count how many black cubes are in the column, and remove them.
            int blackCount = 0;
            for (int y = 0; y < 100; y++) {
                blackCount += cubeIsBlack[x, y] ? 1 : 0;
                cubeIsBlack[x, y] = false;
            }

            // Replace the black cubes all together at the bottom of the column.
            for (int y = 0; y < blackCount; y++) {
                cubeIsBlack[x, y] = true;
            }
        }
    }

    void OnDrawGizmos()
    {
        
        for (int x = 0; x < 100; x++)
        {
            for (int y = 0; y < 100; y++)
            {
                Vector2 position = new Vector2(x, y);
                Gizmos.color = cubeIsBlack[x, y] ? Color.black : Color.white;
                Gizmos.DrawCube(position, Vector3.One);
            }
        }
    }    
}
\$\endgroup\$
  • \$\begingroup\$ Thanks for your answer, please allow me to have some time to digest it :) \$\endgroup\$ – chuackt Jan 10 at 8:23
1
\$\begingroup\$

I think it should be Update() instead of update(). Btw your code in update can be quite slow, so I'd rather run it once instead of every frame.

\$\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.