Tiles Game Best Practices

Question

Coming from a software engineering background I don't have so much experience developing games. Below is a simple 2d tile connect game. What would people improve about it? What are some good practices that could be applied here and what would people suggest?

Board.cs - main script that handles setting up and resetting the board.

using System.Collections;
using System.Collections.Generic;
using UnityEngine;
public class Board : MonoBehaviour {
 // Board Lengths by Cells 
 public int width;
 public int height;
 public GameObject[] tilePrefabs; // array of different tiles which may be used (different colours)
 public GameObject[,] allTiles; // a 2d array of all the tiles on the board
 private TileAnimations tileAnimations;
 // Use this for initialization
 void Start () {
 tileAnimations = this.GetComponent<TileAnimations>();
 allTiles = new GameObject[width, height];
 Setup ();
 }
 // Generate board of random tiles
 private void Setup (){ 
 for (int i = 0; i < width; i++) {
 for (int j = 0; j < height; j++) {
 Vector2 tempPosition = new Vector2(i, j);
 // Create tiles
 int tileToUse = Random.Range(0, tilePrefabs.Length);
 GameObject tile = Instantiate(tilePrefabs[tileToUse], tempPosition, Quaternion.identity);
 tile.transform.parent = this.transform;
 tile.name = "(" + i + ", " + j + ")";
 // Add tile to tiles array
 allTiles[i, j] = tile;
 }
 }
 //bool deadlock = this.GetComponent<DeadlockChecker>().CheckDeadlock();
 }
 // check for null spaces and populate them
 public IEnumerator CollapseColumnCo(){
 int nullCount = 0;
 for (int i = 0; i < width; i++){
 for (int j = 0; j < height; j++){
 if(allTiles[i,j] == null){ // when you create a chain, there will be null spaces on the board (since the tiles disappear)
 nullCount++;
 }
 else if (nullCount > 0){
 allTiles[i, j].GetComponent<Tile>().tileY -= nullCount;
 StartCoroutine(tileAnimations.SlideDown(allTiles[i,j], nullCount)); // slide down tiles above null spaces
 allTiles[i, j - nullCount] = allTiles[i, j]; // update position in allTiles
 allTiles[i, j] = null;
 }
 }
 nullCount = 0;
 }
 yield return new WaitForSeconds(.3f);
 StartCoroutine(FillBoardCo()); // refills board
 }
 private void RefillBoard() {
 for (int i = 0; i < width; i++) {
 for (int j = 0; j < height; j++) {
 if (allTiles[i, j] == null) {
 Vector2 tempPosition = new Vector2(i, j);
 int tileToUse = Random.Range(0, tilePrefabs.Length);
 GameObject piece = Instantiate(tilePrefabs[tileToUse], tempPosition, Quaternion.identity);
 piece.transform.localScale = new Vector3(0f, 0f, 0);
 StartCoroutine(tileAnimations.ScaleTile(piece, "expand"));
 allTiles[i, j] = piece;
 }
 }
 }
 //bool deadlock = this.GetComponent<DeadlockChecker>().CheckDeadlock();
 }
 private IEnumerator FillBoardCo() {
 RefillBoard();
 yield return new WaitForSeconds(.3f);
 }
}

Link.cs - state of a chain

using System.Collections;
using System.Collections.Generic;
using UnityEngine;
public class Link : MonoBehaviour
{
 private Vector2 touchPosition;
 public List<GameObject> chain = new List<GameObject>();
 public GameObject chainHead;
 private Board board;
 private TileAnimations tileAnimations;
 public bool chaining = false; // is the player in the middle of creating a chain?
 // Start is called before the first frame update
 void Start(){
 board = FindObjectOfType<Board>();
 tileAnimations = this.GetComponent<TileAnimations>();
 }
 // Update is called once per frame
 void Update(){
 }
 // Pops tiles when chain is destroyed
 public void DestroyChain(List<GameObject> linkedChain){
 StartCoroutine(tileAnimations.PopIt(linkedChain)); 
 }
}

Tile.cs

using System.Collections;
using System.Collections.Generic;
using UnityEngine;
public class Tile : MonoBehaviour
{
 public int tileX;
 public int tileY;
 private Board board;
 public bool inChain = false;
 // Start is called before the first frame update
 void Start(){
 board = FindObjectOfType<Board>();
 tileX = (int)transform.position.x; // column number
 tileY = (int)transform.position.y; // row number
 }
 // Update is called once per frame
 void Update() {
 if (inChain == true) {
 this.gameObject.transform.localScale = new Vector3(1.5f, 1.5f, 0);
 } else {
 this.gameObject.transform.localScale = new Vector3(1.0f, 1.0f, 0);
 }
 }
 // Checks adjacency so you can only make chains with tiles which are next to each other
 public bool IsAdjacentToHead() {
 // if on left of head
 if (tileX == (board.GetComponent<Link>().chainHead.GetComponent<Tile>().tileX - 1)
 && tileY == board.GetComponent<Link>().chainHead.GetComponent<Tile>().tileY) {
 return true;
 } else if (tileX == (board.GetComponent<Link>().chainHead.GetComponent<Tile>().tileX + 1)
 && tileY == board.GetComponent<Link>().chainHead.GetComponent<Tile>().tileY) {
 // if on right of head
 return true;
 } else if (tileX == (board.GetComponent<Link>().chainHead.GetComponent<Tile>().tileX)
 && tileY == (board.GetComponent<Link>().chainHead.GetComponent<Tile>().tileY + 1)) {
 // if on top of head
 return true;
 } else if (tileX == (board.GetComponent<Link>().chainHead.GetComponent<Tile>().tileX)
 && tileY == (board.GetComponent<Link>().chainHead.GetComponent<Tile>().tileY - 1)) {
 // if on bottom of head
 return true;
 } else {
 return false;
 }
 }
 private void OnMouseDown(){
 board.GetComponent<Link>().chaining = true;
 inChain = true;
 board.GetComponent<Link>().chainHead = this.gameObject;
 board.GetComponent<Link>().chain.Add(this.gameObject);
 }
 private void OnMouseUp(){
 board.GetComponent<Link>().chaining = false;
 if (board.GetComponent<Link>().chain.Count >= 3){
 board.GetComponent<Link>().DestroyChain(board.GetComponent<Link>().chain);
 board.GetComponent<Link>().chain.Clear();
 } else {
 // Remove all from chain
 foreach (GameObject x in board.GetComponent<Link>().chain){
 x.GetComponent<Tile>().inChain = false;
 }
 board.GetComponent<Link>().chain.Clear();
 }
 }
 private void OnMouseEnter(){
 // Only if we are chaining AND the tile does not alread exist in the chain
 if(board.GetComponent<Link>().chaining == true && !board.GetComponent<Link>().chain.Contains(gameObject)){
 // Make sure the tile to be connected is adjacent to the head AND have same colour (tag)
 if (IsAdjacentToHead() && this.CompareTag(board.GetComponent<Link>().chainHead.tag)) {
 board.GetComponent<Link>().chain.Add(this.gameObject);
 board.GetComponent<Link>().chainHead = this.gameObject;
 inChain = true;
 }
 }
 // If the next tile is the one previous in the chain (penultimate element) - remove last element
 if (board.GetComponent<Link>().chaining == true) {
 if (this.gameObject == board.GetComponent<Link>().chain[board.GetComponent<Link>().chain.Count - 2]) {
 board.GetComponent<Link>().chainHead = this.gameObject;
 // remove last element from chain
 board.GetComponent<Link>().chain[board.GetComponent<Link>().chain.Count - 1].GetComponent<Tile>().inChain = false;
 board.GetComponent<Link>().chain.RemoveAt(board.GetComponent<Link>().chain.Count-1);
 }
 }
 }
}

TileAnimation.cs

using System.Collections;
using System.Collections.Generic;
using System;
using UnityEngine;
public class TileAnimations : MonoBehaviour
{
 public GameObject destroyEffect;
 private Board board;
 void Start() {
 board = FindObjectOfType<Board>();
 }
 // After chain disappears, tiles above slide down
 public IEnumerator SlideDown(GameObject tile, int nullCount) {
 var originalPos = tile.transform.position;
 var finalPos = tile.transform.position -= new Vector3(0, nullCount, 0);
 float t = 0f;
 float timeToMove = 0.2f;
 while (t < 1) {
 t += Time.deltaTime / timeToMove;
 tile.transform.position = Vector3.Lerp(originalPos, finalPos, t);
 yield return null;
 }
 }
 // Destroy each tile with a delay
 public IEnumerator PopIt(List<GameObject> linkedChain) {
 float destroyDelay = 0.1f;
 foreach (GameObject tile in linkedChain) {
 int tileX = tile.GetComponent<Tile>().tileX;
 int tileY = tile.GetComponent<Tile>().tileY;
 StartCoroutine(PopOut(tile, destroyDelay));
 board.allTiles[tileX, tileY] = null;
 destroyDelay += 0.1f;
 }
 yield return new WaitForSeconds((linkedChain.Count + 1f) * 0.1f);
 StartCoroutine(board.CollapseColumnCo());
 }
 // Shrink tile down + Particle Effect
 public IEnumerator PopOut(GameObject tile, float delay) {
 yield return new WaitForSeconds(delay);
 GameObject particle = Instantiate(destroyEffect, tile.transform.position, Quaternion.identity);
 Destroy(particle, .6f);
 StartCoroutine(ScaleTile(tile, "shrink"));
 Destroy(tile,0.2f); // wait for shrink animation before destroying
 }
 public IEnumerator ScaleTile(GameObject tile, string action) {
 var originalScale = tile.transform.localScale;
 Vector3 finalScale = new Vector3(0f,0f,0f);
 if (String.Equals(action, "shrink")) {
 finalScale = new Vector3(0f, 0f, 0f);
 } else if (String.Equals(action, "expand")) {
 finalScale = new Vector3(1f, 1f, 1f);
 } else {
 Debug.LogError("Only expand and shrink actions can be used with ScaleTile script.");
 }
 float t = 0f;
 float timeToMove = 0.2f;
 while (t < 1) {
 t += Time.deltaTime / timeToMove;
 tile.transform.localScale = Vector3.Lerp(originalScale, finalScale, t);
 yield return null;
 }
 yield return null;
 }
}

Any tips/help/advice would be greatly appreciated!

• ggorlen: Just curious--why did you remove the cool screen capture showing how the app works?

Answer 1

I have some tips regarding C# conventions.

• Public fields should be replaced with public properties and should be title-cased. For insance Board.width, Board.height.
• Think about whether certain fields should be encapsulated within the instance and can only be mutated by methods on the instance. Tile.tileX, Tile.tileY are good candidates to encapsulate to avoid letting other classes changing these values out of bounds.
• Don't repeat class name in variable names. Tile.tileX should be called Tile.X.
• Use formatted string where possible: $"({i},{j})". In this particular, case you could have used a ValueTuple (i, j).ToString() to get the same output.
• Be consistent with white space, parentheses: at one moment you have 3 consecutive methods with 3 different styles of white space Setup (){, CollapseColumnCo(){, RefillBoard() {.
• I don't get why you have an empty method void Update(){ which is not an interface implementation.
• Some if statements could be replaced with ternary operators to avoid redundant code: see this.gameObject.transform.localScale in method Update and condition inChain == true.
• Conditions should not be checked against true/false: if (inChain == true) should be written as if (inChain).
• Cache redundant calls in a local variable: board.GetComponent<Link>().chainHead.GetComponent<Tile>() should be cachec before all the if statements in IsAdjacentToHead.
• Some of your loops could be refactored to use LINQ: foreach (GameObject x in board.GetComponent<Link>().chain) x.GetComponent<Tile>().inChain = false;. See IList<T>.ForEach(Action<T> action) extension method.