Kazel13
Kazel13

Reputation: 21

How to avoid multiple hard coded method calls

I'm currently working on a little game of mine which I'm creating with Unity. Before I get to my question, let me give you a little overview on the situation:
At the moment there are two players and each of them has the same set of pieces (five types, but ten in total) . Before the scene is build, each player chooses a colour (which will later equal to some sort of "race" of the pieces). At the moment there are three colours so all in all I have 15 different prefabs, because lets say a "blue apple" will have a bit of a different mesh then a "red apple".
At the moment i have something like this:

using UnityEngine;
using System.Collections;

public class Board : MonoBehavior
{
public Piece[,] pieces = new Piece[5,5];
public GameObject redApplePrefab;
public GameObject redBananaPrefab;
public GameObject redBerryPrefab;

public GameObject blueApplePrefab;
public GameObject blueBananaPrefab;
public GameObject blueBerryPrefab;

public GameObject whiteApplePrefab;
public GameObject whiteBananaPrefab;
public GameObject whiteBerryPrefab;


private void GenerateBoard()
{
    if(white) //don't let the missing declaration of the if statement bother you, just wanted to keep it short here)
    {
        GeneratePiece(whiteApplePrefab, 0, 0);
        GeneratePiece(whiteApplePrefab, 0, 1);
        GeneratePiece(whiteApplePrefab, 0, 2);
        GeneratePiece(whiteBananaPrefab, 1, 0);
        GeneratePiece(whiteBananaPrefab, 1, 1);
        GeneratePiece(whiteBananaPrefab, 2, 0);
        GeneratePiece(whiteBerryPrefab, 2, 2);
    }
    if(blue) //don't let the missing declaration of the if statement bother you, just wanted to keep it short here)
    {
        GeneratePiece(blueApplePrefab, 0, 0);
        GeneratePiece(blueApplePrefab, 0, 1);
        GeneratePiece(blueeApplePrefab, 0, 2);
        GeneratePiece(blueeBananaPrefab, 1, 0);
        GeneratePiece(blueeBananaPrefab, 1, 1);
        GeneratePiece(blueeBananaPrefab, 2, 0);
        GeneratePiece(blueeBerryPrefab, 2, 2);
    }
    if(red) //don't let the missing declaration of the if statement bother you, just wanted to keep it short here)
    {
        GeneratePiece(redApplePrefab, 0, 0);
        GeneratePiece(redApplePrefab, 0, 1);
        GeneratePiece(redApplePrefab, 0, 2);
        GeneratePiece(redBananaPrefab, 1, 0);
        GeneratePiece(redBananaPrefab, 1, 1);
        GeneratePiece(redBananaPrefab, 2, 0);
        GeneratePiece(redBerryPrefab, 2, 2);
    }
}

GeneratePiece(Gameobject prefab, int x, int y)
{
    GameObject go = Instantiate(prefab) as GameObject;
    go.transform.SetParent(transform);
    Piece p = go.getComponent<Piece>();
    pieces[x, y] = p;
}
}

And to be blunt. It's kinda ugly. I really want to avoid manually calling the same method over and over just with different values. But I am currently a bit hard stuck at what is an elegant way to make this work. I want to call the prefab with just colour and kind of piece, and I dont want to change the GeneratePiece() method if not necessary. All calculations as to which prefab and where shall happen in the GenerateBoard() method.
When the Game is built, all pieces have fixed places, compare it with chess, but I just have more colours then only two to choose from.
I want to have some kind of loop, which I can give the two colours picked and it geneartes them with the correct prefabs and places them on the appropriate fields.

I was thinking of something like a double keys dictonary

Dictionary<string colour, Dictionary<string kindOfPiece, GameObject prefab>>

Or is it better to have some kind of two dimensional array of prefabs like this:

GameObject[numberOfColours, numberOfPieces] prefabs;

and then have some kind of mapper that translates colour "white" to 0, "blue" to 1, etc. and same for the kind of pieces, so I can access the specific prefab in the array?

GameObject prefabToGenerate = prefabs[colourPlay1.ColourToNumber(), pieceToBeCreated.PieceToNumber()];

Upvotes: 2

Views: 128

Answers (3)

Programmer
Programmer

Reputation: 125305

Use enum to represent the color:

public enum PieceColor
{
    white, blue, red
}

Then use Dictionary enum and Action as key and value to represent the function to call:

Dictionary<PieceColor, Action> colorToAction = new Dictionary<PieceColor, Action>();

Map the color enums to the functions. You should map them manually instead of loop because there are some inconsistencies in the x, y values. Trying to map it with a for loop will result to confusion when reading the code in the future.

void Init()
{
    colorToAction.Add(PieceColor.white,
    delegate
    {
        GeneratePiece(whiteApplePrefab, 0, 0);
        GeneratePiece(whiteApplePrefab, 0, 1);
        GeneratePiece(whiteApplePrefab, 0, 2);
        GeneratePiece(whiteBananaPrefab, 1, 0);
        GeneratePiece(whiteBananaPrefab, 1, 1);
        GeneratePiece(whiteBananaPrefab, 2, 0);
        GeneratePiece(whiteBerryPrefab, 2, 2);
    });

    colorToAction.Add(PieceColor.blue,
    delegate
    {
        GeneratePiece(blueApplePrefab, 0, 0);
        GeneratePiece(blueApplePrefab, 0, 1);
        GeneratePiece(blueApplePrefab, 0, 2);
        GeneratePiece(blueBananaPrefab, 1, 0);
        GeneratePiece(blueBananaPrefab, 1, 1);
        GeneratePiece(blueBananaPrefab, 2, 0);
        GeneratePiece(blueBerryPrefab, 2, 2);
    });

    colorToAction.Add(PieceColor.red,
    delegate
    {
        GeneratePiece(redApplePrefab, 0, 0);
        GeneratePiece(redApplePrefab, 0, 1);
        GeneratePiece(redApplePrefab, 0, 2);
        GeneratePiece(redBananaPrefab, 1, 0);
        GeneratePiece(redBananaPrefab, 1, 1);
        GeneratePiece(redBananaPrefab, 2, 0);
        GeneratePiece(redBerryPrefab, 2, 2);
    });
}

Below is your new GenerateBoard function. Each time you need to call a function, get the color and use TryGetValue to retrieve the appropriate function then use the Invoke function to call the functions.

PieceColor color = PieceColor.white;

private void GenerateBoard()
{
    Action action;
    if (colorToAction.TryGetValue(color, out action))
        action.Invoke();
}

Upvotes: 5

Chopi
Chopi

Reputation: 1143

You could try to avoid all the copy and paste code a little bit in this way...

Inside your GenerateBoard you could have something like this, inside appleWithColor and the others you could have a public GameObject[] appleWithCOlor and fill it from the inspector with the colors. The same for the others fruits

for(int color = 0;color < MAX_COLORS; color++)
{
    GameObject Apple = appleWithColor[color];//set depeding on color
    GameObject Bannana = bannanaWithColor[color];
    GameObject Berry = berryWithColor[color];

    for(int i = 0; i <3;i++)
    {
        GeneratePiece(Apple, 0,i);
    }
    for(int i = 0; i <2;i++)
    {
        GeneratePiece(Bannana, 1,i);
    }
    GeneratePiece(Bannana, 2,0);        
    GeneratePiece(Berry,2,2);
}

Upvotes: 1

Dan Scott
Dan Scott

Reputation: 564

...
    if(red) //don't let the missing declaration of the if statement bother you, just wanted to keep it short here)
    {
        GeneratePieces(redApplePrefab, new List<Vector2>() { new Vector2(0,0), new Vector2(0,1), new Vector2(0,2)});
        GeneratePieces(redBananaPrefab, new List<Vector2>() { new Vector2(1,0), new Vector2(1,1), new Vector2(2,0)});
        GeneratePiece(redBerryPrefab, 2, 2);
    }
}

GeneratePieces(Gameobject prefab, List<Vector2> coords) 
{
    foreach (Vector2 vec in coords) 
    {
        GeneratePiece(prefab, vec.X, vec.Y);
    }
}

GeneratePiece(Gameobject prefab, int x, int y)
{
    GameObject go = Instantiate(prefab) as GameObject;
    go.transform.SetParent(transform);
    Piece p = go.getComponent<Piece>();
    pieces[x, y] = p;
}
}

Upvotes: 1

Related Questions