Saving allowed positions from a 3x3 matrix

I wrote the next code chunk to save all the allowed positions from a 3x3 matrix, it's working but I think it could be better. Besides something similar happens in my Randomizer class, although I admit it's messy.

public class MatrixRegistry : MonoBehaviour
{
private int matrixVolume;
private int matrixCount;

private GameObject[] matrix;

private List<GameObject> dynamicCubes = new List<GameObject>();
private List<Vector3> staticCubes = new List<Vector3>();
private List<Vector3> allowedPositions = new List<Vector3>();

public Func<List<GameObject>> GetDynamicCubes { get; private set; }
public Func<List<Vector3>> GetAllowedPositions { get; private set; }

private void Awake()
{
#region Getting all Matrix Cubes

matrixCount = gameObject.transform.childCount;

if (matrixCount != 0)
{
matrix = new GameObject[matrixCount];

for (int i = 0; i < matrixCount; i++)
{
matrix[i] = gameObject.transform.GetChild(i).gameObject;

if (matrix[i].GetComponent<CubeInfo>().cubeType == CubeSort.Dynamic)
{
}

if (matrix[i].GetComponent<CubeInfo>().cubeType == CubeSort.Static)
{
}
}
}

GetDynamicCubes = () => dynamicCubes;
#endregion

#region Setting Allowed Positions

matrixVolume = 3; // Hard Coding

for (int x = 0; x < matrixVolume; x++)
{
for (int y = 0; y < matrixVolume; y++)
{
for (int z = 0; z < matrixVolume; z++)
{
Vector3 position = new Vector3
{
x = x,
y = y,
z = z
};

}
}
}

for (int f = 0; f < staticCubes.Count; f++)
{
for (int a = 0; a < allowedPositions.Count; a++)
{
if (allowedPositions[a] == staticCubes[f])
{
allowedPositions.Remove(allowedPositions[a]);
}
}
}

GetAllowedPositions = () => allowedPositions;
#endregion
}
}

public class Randomizer
{
private MatrixRegistry matrixRegistry;
private EventManager eventManager;

public Randomizer(MatrixRegistry matrixRegistry, EventManager eventManager)
{
this.matrixRegistry = matrixRegistry;
this.eventManager = eventManager;

eventManager.OnStartGame += Randomize;
}

private void Randomize()
{
for (int i = 0; i < matrixRegistry.GetDynamicCubes().Count; i++)
{
Vector3 randomPosition = new Vector3
{
x = Random.Range(0, 3),
y = Random.Range(0, 3),
z = Random.Range(0, 3)
};

for (int j = 0; j < matrixRegistry.GetAllowedPositions().Count; j++)
{
for (int k = 0; k < matrixRegistry.GetDynamicCubes().Count; k++)
{
if (randomPosition == matrixRegistry.GetAllowedPositions()[j])
{
if (randomPosition != matrixRegistry.GetDynamicCubes()[k].transform.position)
{
matrixRegistry.GetDynamicCubes()[i].transform.position = randomPosition;
}
}
}
}
}
}
}

The randomizer class should check all the allowed positions to get in place all the cubes and avoid overlapping issues. I know it's not the best, and this is why I'm here.

• Why don't you post the complete class? I'd be much easier to review it having all the code rather then some random snippets. – t3chb0t Jan 14 at 7:24
• @t3chb0t I didn't think it was necessary, it's just a for loop, but already I did it. – Lobsang White Jan 14 at 7:39
• It looks much better now and it's much easier to understand. Thanks for updating ;-] – t3chb0t Jan 14 at 7:42

The way you are checking if any allowedPositions are in the staticCubes list seems very inefficient to me. I think it would be more efficient to check whether each position is in the staticCubes list as they are created then add them to the allowedPositions list.

Using a LINQ query for this wil also make it much more concise:

allowedPositions = (from int x in Enumerable.Range(0, matrixVolume)
from int y in Enumerable.Range(0, matrixVolume)
from int z in Enumerable.Range(0, matrixVolume)
let position = new Vector3(x, y, z)
where !staticCubes.Contains(position)
select position).ToList();
• I don't know much about LINQ features, I'm going to read about it. About Randomizer class, the Randomize method could be improve using LINQ. I still think is awful. – Lobsang White Jan 15 at 8:35
• @LobsangWhite - I added notes about the Randomizer class for you to look at. – tinstaafl Jan 15 at 14:20
• Yes, but it's not swap among their own positions, it's a 3x3 matrix, that's means 27 positions, what it does is shuffle among all the possible positions, but the static ones. – Lobsang White Jan 15 at 17:34
• @LobsangWhite - according to your code, the positions in dynamicCubes don't include the positions in staticCubes – tinstaafl Jan 15 at 18:08
• Exactly, this is why I use GetAllowedPositions method, because I stored all the matrix positions there, except static cubes positions, and I check if the new position could be the right one if is equal to an allowed position from the list returned by GetAllowedPositions method. – Lobsang White Jan 15 at 19:18