3
\$\begingroup\$

I have a mesh generation script which initialises a mesh with vertices that represent my map tiles as quads. Then I pass the tiles that I wish to display on the mesh by setting my triangles for the vertices related to the tile.

I was wondering if there is a more optimised way to approach this for Unity. I'm still learning about mesh generation but am unsure if I have designed it efficiently.

This is my initial generation of the mesh before I create any tiles:

private void Awake(){
  Generate();
}
private void Generate ()
{
    vertices = new Vector3[mapLength * mapWidth * 4];
    int v = -1;

    for (int j = 0; j < mapLength; j++)
    {
        for (int i = 0; i < mapWidth; i++)
        {
            vertices[++v] = new Vector3(i * gridSize, 0, j * gridSize); // bottom left 
            vertices[++v] = new Vector3(i * gridSize + gridSize, 0, j * gridSize); // bottom right
            vertices[++v] = new Vector3(i * gridSize, 0, j * gridSize + gridSize); // top left
            vertices[++v] = new Vector3(i * gridSize + gridSize, 0, j * gridSize + gridSize); //top right
        }
    }
    GetComponent<MeshFilter>().mesh = mesh = new Mesh();
    mesh.name = "Floor Grid";
    mesh.MarkDynamic();
    mesh.vertices = vertices;
}

So, now I have created the mesh with the vertices for my quads. I now apply triangles to it when the user wishes to draw tiles I obtain the triangles array and add to it like so:

private void Add(List<Vector3> tiles)
{
    int[] triangles;
    if (mesh.triangles.Length == 0)
    {
        // there are currently no tiles so create new array match tile count
        triangles = new int[tiles.Count * 6];
    }
    else
    {
        // we already have some tiles so expand the existing array
        triangles = new int[mesh.triangles.Length + (tiles.Count * 6)];
    }

    // get the triangle indices from mesh.triangle and apply them to new array
    int index = 0;
    for (int i = 0; i < mesh.triangles.Length; i++)
    {
        trianglesList.AddRange(mesh.triangles); 
        triangles[i] = mesh.triangles[i];
        index = i+1;
    }

    //apply the new triangles for the tiles the user wants
    for (int i = 0; i < tiles.Count; i++)
    {
        tri = (int)(tiles[i].z * mapWidth + tiles[i].x) * 4;
        triangles[index++] = tri;
        triangles[index++] = tri + 2;
        triangles[index++] = tri + 1;

        triangles[index++] = tri + 2;
        triangles[index++] = tri + 3;
        triangles[index++] = tri + 1;
    }

    mesh.triangles = triangles;
    mesh.RecalculateNormals();
}

This code works, but I am no expert at mesh generation and have basically picked up knowledge from others and example scripts over time.

I was wondering if there is any optimising I can do here. Or have I understood how it is done and this is pretty much as good at it gets?

\$\endgroup\$
2
\$\begingroup\$

I can't comment on the specifics of how efficient your algorithm is, but i can comment on something that is probably more important - assuming you have enough computing power:

Readability and Understandability

if you didn't know anything about Mesh code above, it is very difficult to understand. this means that if you were to look at it in 12 months - would you understand it? this is very important when writing code.

Code

Generally speaking be very suspicious of "if" statements. Whenever you see one you gotta ask: why is that there? if you don't have a good reason, then it might be better of to extract a class and use polymorphism to do the same thing. that way, conditional statements are somewhat isolated from behaviour. that makes things easier to change and somewhat easier to code, but it does come at a price.

Methods

You can hide away some implementation in methods. you've got some methods that are very long that it's not easy for someone unfamiliar with what you're trying to do to make sense of it.

int[] triangles;
if (mesh.triangles.Length == 0)
{
    // there are currently no tiles so create new array match tile count
    triangles = new int[tiles.Count * 6];
}
else
{
    // we already have some tiles so expand the existing array
    triangles = new int[mesh.triangles.Length + (tiles.Count * 6)];
}

you can turn the above into this:

int[] triangles = GetTriangles();

and hide everything in that method. it would already dramatically improve readability

Update

A picture is worth a million words! I can sort of conceptualise what you are trying to do.

Unfortunately i can't comment on the best OOP way because i have a very incomplete pictures but something like this would certainly give you ideas on how to structure code to make it more readable. Without altering what it does.

Secondly - tests - i'm gonna guess that you have written tests for this? if you did then tests will force you to write code more systematically so i highly recommend adding tests.

here is just some basic cutting and pasting - i hope it helps.

Performance

btw - your algorithm looks good enough. but be warned, this type of looping can slow things done if mapLength and width gets too large.

for (int j = 0; j < mapLength; j++)
    {
        for (int i = 0; i < mapWidth; i++)
        {
.....}}

using System.Collections.Generic;
namespace practiceTest
{
    internal class UnityMesh
    {
        private Mesh mesh;
    private void Awake()
    {
        Mesh mesh = CreateMesh();
        this.mesh = mesh;
    }

    /// <summary>
    /// Generates this instance - i don't like the name of this method. it's way too vague
    /// </summary>
    /// <returns></returns>
    private Mesh CreateMesh()
    {
        Vector3d vertices = GetVertices();
        Mesh mesh = SetVerticesToNewMesh(vertices);
        return mesh;
    }

    private Mesh SetVerticesToNewMesh(Vector3d vertices)
    {
        GetComponent<MeshFilter>().mesh = mesh = new Mesh();
        mesh.name = "Floor Grid";
        mesh.MarkDynamic();
        mesh.vertices = vertices;

        return mesh;
    }

    /// <summary>
    /// Gets the vertices.
    /// </summary>
    /// <returns></returns>
    private Vector3d GetVertices()
    {
        vertices = new Vector3[mapLength * mapWidth * 4];
        int v = -1;

        for (int j = 0; j < mapLength; j++)
        {
            for (int i = 0; i < mapWidth; i++)
            {
                vertices[++v] = new Vector3(i * gridSize, 0, j * gridSize); // bottom left
                vertices[++v] = new Vector3(i * gridSize + gridSize, 0, j * gridSize); // bottom right
                vertices[++v] = new Vector3(i * gridSize, 0, j * gridSize + gridSize); // top left
                vertices[++v] = new Vector3(i * gridSize + gridSize, 0, j * gridSize + gridSize); //top right
            }
        }
    }

    private void Add(List<Vector3> tiles)
    {
        int[] triangles = GetTriangles();

        int index = ApplyTriangleIndices(triangles);

        //apply the new triangles for the tiles the user wants
        index = ApplyNewTriangles(tiles, triangles, index);

        mesh.triangles = triangles;
        mesh.RecalculateNormals();
    }

    private static int ApplyNewTriangles(List<Vector3> tiles, int[] triangles, int index)
    {
        for (int i = 0; i < tiles.Count; i++)
        {
            tri = (int)(tiles[i].z * mapWidth + tiles[i].x) * 4;
            triangles[index++] = tri;
            triangles[index++] = tri + 2;
            triangles[index++] = tri + 1;

            triangles[index++] = tri + 2;
            triangles[index++] = tri + 3;
            triangles[index++] = tri + 1;
        }
        return index;
    }

    private int ApplyTriangleIndices(int[] triangles)
    {
        // get the triangle indices from mesh.triangle and apply them to new array
        int index = 0;
        for (int i = 0; i < mesh.triangles.Length; i++)
        {
            trianglesList.AddRange(mesh.triangles);
            triangles[i] = mesh.triangles[i];
            index = i + 1;
        }
        return index;
    }
}
}
\$\endgroup\$
  • \$\begingroup\$ Should GetTriangles be getting both the new triangles for the tiles and the previously existing tiles, or just the tiles that already exist in triangles? \$\endgroup\$ – Sir Mar 28 '17 at 4:35
  • \$\begingroup\$ @Sir that's a question of design principle. i'll have a serious look at this later tonight. basic point i was making is that you can hide a lot of things in methods which makes things easier to read and understand as a programmer reading it. secondly, are you able to post some data or a picture or something (via a github gist or otherwise) so i can run it and see the results -- because atm i have no idea what is going on so it's kinda hard to conceptualise what this code is trying to do? \$\endgroup\$ – BKSpurgeon Mar 28 '17 at 4:47
  • \$\begingroup\$ I did a quick change to this: hastebin.com/yixanimuwe.cs and used ref to get the index aswell. \$\endgroup\$ – Sir Mar 28 '17 at 4:50
  • \$\begingroup\$ here is a visual of the mesh generation i.imgur.com/7TMHUu6.gif it's hideous pink since i have not applied uv's for textures yet but that will come later. \$\endgroup\$ – Sir Mar 28 '17 at 4:53
  • \$\begingroup\$ have updated some ideas for you \$\endgroup\$ – BKSpurgeon Mar 28 '17 at 9:17

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.