4
\$\begingroup\$

I am creating a .io style game through Unity and it's coming along nicely. This is my first time working with Unity, and my first time using C#; I have been a Java coder. Could you good people tell me what you think, and tell me if you see any major issues? I am hoping to improve the EnemyCode AI script, and if anyone has suggestions there please let me know.

How the game works - You drop towers that shoot everything that is not on your team, that are within a certain distance. You can also shoot other towers, and you can shoot enemy ships. When you get a kill, or drop a tower, your score goes up. There is a score board that I will make a script for eventually. Enemy ships place towers themselves and will have scores.

EnemyCode.cs

This is the code that the enemy ships run off of. When the enemy is created it randomly picks a color that is not already being used by other players in the game. This is done in the getRandomColor() method. They currently locate the closest tower, and move to it. When in range, the enemy shoots a bullet.cs. They check for a new closer tower every update of the physics engine. I would like to add features where enemy ships target other enemy ships, and the player (Controler.cs) as well. Every 10 seconds wherever the enemy ship is located it places a tower of its own color. I would like to change this up where the enemy code could have a better strategy when placing the towers. When the enemy dies, all towers of the same color die with it.

using System.Collections;
using System.Collections.Generic;
using UnityEngine;

public class EnemyCode : MonoBehaviour
{
    public GameObject tower;
    public GameObject shot;
    public float turnSpeed = 2;
    public float shootCooldown = 3;
    private float cooldownLeft = 0;
    public float shootRange = 10;
    public float toCloseRange = 2;
    public float speed;
    public string color;
    public float shotOffset;
    public int hp = 10;
    public float towerPlaceCooldown = 10;
    float towerCooldown = 0;
    Color32 c;
    TowerCode[] myTeam;

void Start()
{
    setColor();
}

void setColor()
{
    SpriteRenderer sr = GetComponent<SpriteRenderer>();
    c = getRandomColor();
    sr.color = c;
    color = c.ToString();
}

Color32 getRandomColor()
{
    Color32 temp = new Color32();
    int  randNum = (Random.Range(0, 255)/50)*50;

    switch (Random.Range(1, 6))
    {
        case 1: temp.r = (byte)randNum; temp.b = 0xFF; temp.g = 0x0; temp.a = 0xFF; break;
        case 2: temp.r = 0x0; temp.b = (byte)randNum; temp.g = 0xFF; temp.a = 0xFF; break;
        case 3: temp.r = 0xFF; temp.b = 0x0; temp.g = (byte)randNum; temp.a = 0xFF; break;
        case 4: temp.r = (byte)randNum; temp.b = 0x0; temp.g = 0xFF; temp.a = 0xFF; break;
        case 5: temp.r = 0xFF; temp.b = (byte)randNum; temp.g = 0x0; temp.a = 0xFF; break;
        case 6: temp.r = 0x0; temp.b = 0xFF; temp.g = (byte)randNum; temp.a = 0xFF; break;
    }

    Object[] g = FindObjectsOfType(typeof(EnemyCode));
    Object[] g2 = FindObjectsOfType(typeof(Controler));
    for (int c = 0; c < g.Length; c++)
    {
        EnemyCode e = (EnemyCode)g[c];
        if (temp.ToString().Equals(e.color))
        {

            return getRandomColor();
        }
    }
    for (int c = 0; c < g2.Length; c++)
    {
        Controler e = (Controler)g2[c];
        if (temp.ToString().Equals(e.color))
        {
            return getRandomColor();
        }
    }

    return temp;

}

TowerCode[] GetTowerCode()
{
    Object[] g = FindObjectsOfType(typeof(TowerCode));
    TowerCode[] t = new TowerCode[g.Length];
    for (int h = 0; h < g.Length; h++)
    {
        TowerCode temp = (TowerCode)g[h];
        if (!temp.color.Equals(this.color))
        {
            t[h] = temp;
        }
        else
        {
            t[h] = null;
        }
    }
    return t;
}

TowerCode getNextTarget()
{
    TowerCode[] t = GetTowerCode();
    TowerCode nextTarget = null;
    float dist = Mathf.Infinity;
    for (int h = 0; h < t.Length; h++)
    {
        if (t[h] != null)
        {
            float d = Vector3.Distance(this.transform.position, t[h].transform.position);

            if (nextTarget == null || d < dist)
            {
                nextTarget = t[h];
                dist = d;
            }
        }
    }
    return nextTarget;
}

void MoveTo(TowerCode nextTarget)
{
    Vector2 v = nextTarget.transform.position - this.transform.position;
    float angle = Mathf.Atan2(v.y, v.x) * Mathf.Rad2Deg;
    Quaternion rot = Quaternion.AngleAxis(angle - 90, Vector3.forward);
    transform.rotation = Quaternion.Slerp(transform.rotation, rot, turnSpeed * Time.deltaTime);

    if (Vector2.Distance(transform.position, nextTarget.transform.position) < toCloseRange)
    {
        transform.position = Vector2.MoveTowards(transform.position, nextTarget.transform.position, -speed * Time.deltaTime);
    }
    else if (Vector2.Distance(transform.position, nextTarget.transform.position) > shootRange)
    {
        transform.position = Vector2.MoveTowards(transform.position, nextTarget.transform.position, speed * Time.deltaTime);
    }
    else
    {
        transform.position = this.transform.position;
    }
}

void shoot()
{
    if (cooldownLeft <= 0)
    {
        Vector3 vec = transform.rotation * new Vector3(0, shotOffset, 0);
        Bullet b = shot.GetComponent<Bullet>();
        b.color = color;
        Instantiate(shot, transform.position + vec, transform.rotation);
        cooldownLeft = shootCooldown;
    }
    else
    {
        cooldownLeft -= Time.deltaTime;
    }
}

void buildTower()
{
    if (towerCooldown <= 0)
    {

        Transform tra = GetComponent<Transform>();
        float x = transform.position.x;
        float y = transform.position.y;
        tra.transform.position.Set(x, y, 0);

        TowerCode tc = tower.GetComponent<TowerCode>();
        tc.color = color;
        tc.col = c;
        tc.setColor();
        Instantiate(tower, this.transform.position, this.transform.rotation);
        towerCooldown = towerPlaceCooldown;
    }
    else
    {
        towerCooldown -= Time.deltaTime;
    }
}

void checkForDeath()
{
    if (hp <= 0)
    {
        killTeam();
        Destroy(gameObject);
    }
}

void killTeam()
{
    Object[] g = FindObjectsOfType(typeof(TowerCode));
    for (int h = 0; h < g.Length; h++)
    {
        TowerCode temp = (TowerCode)g[h];
        temp.teamDie(c);
    }
}

void FixedUpdate()
{
    TowerCode nextTarget = getNextTarget();
    if (!(nextTarget == null))
    {
        MoveTo(nextTarget);
        shoot();
    }
    buildTower();
    checkForDeath();
}

private void OnTriggerEnter2D(Collider2D collision)
{
    if (!(collision.gameObject.GetComponent<Bullet>().color == color))
    {
        hp--;
    }
}
}

StartUpCode.cs

This code randomly spawns 27 EnemyCode.cs at the start of the game in the range of the variables. Every graphics update it makes sure there are still 27. If there aren't, it spawns more.

public class StartUpCode : MonoBehaviour
{
    public GameObject enemyPlayer;
    public float xMaxRange;
    public float xMinRange;
    public float yMaxRange;
    public float yMinRange;

void Start()
{
    // max enemy count that current color system can handle is 27
    for (int v = 0; v < 27; v++)
    {
       summonEnemy();
    }
}

void Update()
{
    if (GetEnemyNum() < 27)
    {
       summonEnemy();
    }
}

void summonEnemy()
{
    Vector2 pos = new Vector2(Random.Range(xMinRange, xMaxRange), Random.Range(yMinRange, yMaxRange));
    Instantiate(enemyPlayer, pos, Quaternion.identity);
}

int GetEnemyNum()
{
    Object[] g = FindObjectsOfType(typeof(EnemyCode));
    return g.Length;
}

}

TowerCode.cs

Very similar to EnemyCode.cs, other than it doesn't move or place towers. I would like to add functionality where the TowerCode.cs can target other towers and the Controler.cs, but I keep running into problems and going back to this stable version.

using System.Collections;
using System.Collections.Generic;
using UnityEngine;

public class TowerCode : MonoBehaviour
{
public GameObject shot;
public int hp = 5;
public float turnSpeed = 5f;
public float shootRange = 20;
public float shootCooldown = 0.5f;
float cooldownLeft = 0;
public string color;
public float shotOffset;
public Color col;


public void teamDie(Color deadTeamCol)
{
    if (this.col == deadTeamCol)
    {
        Destroy(gameObject);
    }
}

public void setColor()
{
    SpriteRenderer sr = GetComponent<SpriteRenderer>();
    sr.color = col;
}

EnemyCode[] GetEnemyCode()
{
    Object[] g = FindObjectsOfType(typeof(EnemyCode));
    EnemyCode[] t = new EnemyCode[g.Length];
    for (int h = 0; h < g.Length; h++)
    {
        EnemyCode temp = (EnemyCode)g[h];
        if (!temp.color.Equals(this.color))
        {
            t[h] = temp;
        }
        else
        {
            t[h] = null;
        }
    }
    return t;
}

EnemyCode getNextTarget()
{

    EnemyCode[] t = GetEnemyCode();
    EnemyCode nextTarget = null;
    float dist = Mathf.Infinity;
    for (int h = 0; h < t.Length; h++)
    {
        if (t[h] != null)
        {
            float d = Vector3.Distance(this.transform.position, t[h].transform.position);

            if (nextTarget == null || d < dist)
            {
                nextTarget = t[h];
                dist = d;
            }
        }
    }
    return nextTarget;
}

void pointTo(EnemyCode nextTarget)
{
    Vector2 v = nextTarget.transform.position - this.transform.position;
    float angle = Mathf.Atan2(v.y, v.x) * Mathf.Rad2Deg;
    Quaternion rot = Quaternion.AngleAxis(angle - 90, Vector3.forward);
    transform.rotation = Quaternion.Slerp(transform.rotation, rot, turnSpeed * Time.deltaTime);
}

void shoot()
{
    if (cooldownLeft <= 0)
    {
        Vector3 vec = transform.rotation * new Vector3(0, shotOffset, 0);
        Bullet b = shot.GetComponent<Bullet>();
        b.color = color;
        Instantiate(shot, transform.position + vec, transform.rotation);
        cooldownLeft = shootCooldown;
    }
    else
    {
        cooldownLeft -= Time.deltaTime;
    }
}

void checkForDeath()
{
    if (hp <= 0)
    {
        Destroy(gameObject);
    }
}

void FixedUpdate()
{
    EnemyCode nextTarget = getNextTarget();

    if (nextTarget != null)
    {
        pointTo(nextTarget);
        shoot();
    }

    checkForDeath();
}

private void OnTriggerEnter2D(Collider2D collision)
{
    if (!(collision.gameObject.GetComponent<Bullet>().color == color))
    {
        hp--;
    }
}

}

Controler.cs

This is my player script. It works exactly as I want other than for two things. The towers should spawn behind the player instead of disrupting them when moving. I'm not sure how to implement that.

using System.Collections;
using System.Collections.Generic;
using UnityEngine;

public class Controler : MonoBehaviour
{

public float speed = 3f;
float buildCoolDown = 0f;
public float BuildCoolDown = 10;
float shootCooldown = 0;
public float ShootCooldown = 0.5f;
public float turnSensitivity = -2f;
public GameObject tower;
public GameObject shot;
public int hp = 10;
public float shotOffset;
public string color;
Color32 c;
Rigidbody2D rb;

void Start()
{
    rb = GetComponent<Rigidbody2D>();
    getColor();
}

void getColor()
{
    SpriteRenderer sr = GetComponent<SpriteRenderer>();
    c = new Color32();
    int randNum = (Random.Range(0, 255) / 50) * 50;

    switch (Random.Range(1, 6))
    {
        case 1: c.r = (byte)randNum; c.b = 0xFF; c.g = 0x0; c.a = 0xFF; break;
        case 2: c.r = 0x0; c.b = (byte)randNum; c.g = 0xFF; c.a = 0xFF; break;
        case 3: c.r = 0xFF; c.b = 0x0; c.g = (byte)randNum; c.a = 0xFF; break;
        case 4: c.r = (byte)randNum; c.b = 0x0; c.g = 0xFF; c.a = 0xFF; break;
        case 5: c.r = 0xFF; c.b = (byte)randNum; c.g = 0x0; c.a = 0xFF; break;
        case 6: c.r = 0x0; c.b = 0xFF; c.g = (byte)randNum; c.a = 0xFF; break;
    }
    sr.color = c;
    color = c.ToString();
}

void move()
{
    if (Input.GetButton("Vertical"))
    {
        rb.AddForce(transform.up * speed * Input.GetAxis("Vertical"));
    }
    rb.AddTorque(Input.GetAxis("Horizontal") * turnSensitivity);
}

void shoot()
{
    if (Input.GetButton("Fire1") && shootCooldown <= 0)
    {
        Vector3 vec = transform.rotation * new Vector3(0, shotOffset, 0);
        Bullet b = shot.GetComponent<Bullet>();
        b.color = color;
        Instantiate(shot, transform.position + vec, transform.rotation);
        shootCooldown = ShootCooldown;
    }
    else
    {
        shootCooldown -= Time.deltaTime;
    }
}

void placeTower()
{
    if (Input.GetButton("Fire3") && buildCoolDown <= 0)
    {
        Transform t = GetComponent<Transform>();
        float x = rb.transform.position.x;
        float y = rb.transform.position.y;
        t.transform.position.Set(x, y, 0);

        TowerCode tc = tower.GetComponent<TowerCode>();
        tc.color = color;
        tc.col = c;
        tc.setColor();
        Instantiate(tower, this.transform.position, this.transform.rotation);
        buildCoolDown = BuildCoolDown;
    }
    else
    {
        buildCoolDown -= Time.deltaTime;
    }
}

void FixedUpdate()
{
    move();

    shoot();

    placeTower();

    if (hp <= 0)
    {
        Destroy(gameObject);
    }
}

private void OnTriggerEnter2D(Collider2D collision)
{
    if (!(collision.gameObject.GetComponent<Bullet>().color == color))
    {
        hp--;
    }
}

}

Bullet.cs

The only problem I have here is that my bullets start out with a speed of almost nothing, and it speeds up as it goes. This causes the problem of the bullets appearing behind moving players/enemy's instead of in front of them. Also there is a weird glitch that causes console errors that don't effect game play. In java I would solve this with a throws exception, but that isn't in C# so I used try catch blocks. Kind of bad code, but it works.

public class Bullet : MonoBehaviour
{
public float bulletSpeed = 10;
public float damage = 1f;
float bulletLife = 3;
public string color;
Rigidbody2D rb;

void Start()
{
    rb = GetComponent<Rigidbody2D>();
}

void FixedUpdate()
{
    rb.AddForce(transform.up * bulletSpeed);
    bulletLifeTracker();
}

void bulletLifeTracker()
{
    if (bulletLife <= 0)
    {
        Destroy(gameObject);
    }
    bulletLife -= Time.deltaTime;
}

private void OnTriggerEnter2D(Collider2D collision)
{
    try
    {
        if (!(collision.gameObject.GetComponent<Bullet>().color == color))
        {
            Destroy(gameObject);

        }
    }
    catch (Exception e) { }
    try
    {
        if (!(collision.gameObject.GetComponent<TowerCode>().color == color))
        {
            Destroy(gameObject);

        }
    }
    catch (Exception e) { }
    try
    {
        if (!(collision.gameObject.GetComponent<EnemyCode>().color == color))
        {
            Destroy(gameObject);

        }
    }
    catch (Exception e) { }
    try
    {
        if (!(collision.gameObject.GetComponent<Controler>().color == color))
        {
            Destroy(gameObject);

        }
    }
    catch (Exception e) { }
}

}
\$\endgroup\$
  • 2
    \$\begingroup\$ Could you expand the description above the code to include more details about the game? Perhaps an explanation of the game flow would be useful to reviewers \$\endgroup\$ – Sᴀᴍ Onᴇᴌᴀ Nov 11 '18 at 20:15
  • \$\begingroup\$ @SᴀᴍOnᴇᴌᴀ Alright, sure! \$\endgroup\$ – Clint Nov 11 '18 at 20:20
2
\$\begingroup\$

This isn't a complete review: I'm going to start by focusing on just one function. And I may come across as pretty critical, so let me also say that this all looks pretty cool. What follows are my (pretty strong) opinions but bear in mind that I haven't coded any games lately. You have, so you have my admiration.

EnemyCode.getRandomColor()

  • Why is this class called EnemyCode? Most classes contain code. Surely simply Enemy is sufficient.

  • The color you're going to return is called temp. This is better (barely) than a single-letter variable name, in that I know the variable is "temporary"... But in fact, I already know it's temporary, because it was declared inside of a function, and will soon go out of scope. For a more useful name, tell me what the variable represents; how about generatedColor?

  • You have an int variable called randNum that is never used as an int. Cast it to a byte at declaration, so that I have a better idea what you're doing with it.

  • randNum is generated semi-randomly; a random number is passed through a filter, so that I have to perform arithmetic to figure out what's going on with it. Why not pull this out into a function (and simplify it as you do so)?

/// <summary>Returns one of { 0, 51, 102, 153, 204, 255 }</summary>
byte getRandomByte()
{
    return (byte)(Random.Range(0,5)*51);
}
  • getRandomColor() is actually divided into two parts. First, generate a random color. Second, recursively ensure this color is unused. And one function that does two things will be clearer when it's written as two functions doing one thing each: perhaps getRandomColor() and getRandomUnusedColor().

  • You'll note that I'm not matching behavior exactly with my implementation of getRandomByte(). Your code as originally posted goes by increments of 50 and not 51. The advantage of 51 is that it can generate 255 exactly; this prevents the possibility of generating (0, 255, 250) for one enemy and (0, 250, 255) for another. Those would pass the "duplicate color check" code, but would appear identical to the player. There is also the issue of weighting; both case 2 and case 6 can produce R,G,B = 0,0,255, so solid blue is twice as likely as some of the other colors.

  • Now that getRandomColor() only generates the color, and doesn't perform any additional logic, you can return straight from the switch branch. I prefer case x: return y to Foo answer... case x: answer=y; break... return answer. That's if you keep that case statement in your code at all; more on that later.

  • I am dismayed to find that Unity does not have a generic FindObjectsOfType<T>. The first thing I would do is write one:

IEnumerable<T> FindObjectsOfType<T>()
{
    return FindObjectsOfType(typeof(T)).Cast<T>();
}
  • You've got two for loops, and at first I wasn't sure what they were for. As a rule, foreach is clearer (and usually easier to use!) than for, and LINQ is usually clearer (and even easier!) than foreach. With our new helper function, we can write:
Color32 getRandomUnusedColor()
{
    var enemyColors = FindObjectsOfType<EnemyCode>().Select(enemy => enemy.color);
    var playerColors = FindObjectsOfType<Controler>().Select(player => player.color);
    var usedColors = enemyColors.Concatenate(playerColors).ToHashSet();

    Color32 generatedColor;

    do
    {
        generatedColor = getRandomColor();
    } while (usedColors.Contains(generatedColor));

    return generatedColor;
}
  • That code has the advantage of not using .ToString() to compare structs, which feels pretty odd. Based on my testing I believe it will work as expected, but that would be a good thing to verify on your own (if it does not, you could fix it by writing a Color32Comparer and supplying it as the second parameter to Contains).

  • That code also has the advantage of only calling FindObjectsOfType twice per call, as opposed to 2*(N+1) times per call, where N is the number of color collisions. Another possible optimization would be to statically maintain the set of in-use colors, so that you don't need to call FindObjectsOfType at all.

  • One way to make your color generation code simpler (and less weighted toward primary colors):

private const byte MAXIMUM_OPACITY = 0xFF;
Color32 getRandomColor()
{
    return new Color32(
        r: getRandomByte(),
        g: getRandomByte(),
        b: getRandomByte(),
        a: MAXIMUM_OPACITY);
}
  • This introduces the possibility of black and white, which you may not want to see. One way to rectify that is to manually add black and white to the usedColors blacklist. However, some of these colors are barely distinguishable... perhaps you would like to be even more restrictive about possible outputs from getRandomByte()? Or perhaps you would like even more control over which colors are generated? You might replace all of the color generation code with a static "palette" collection of colors, and remove the randomness. "First enemy will be purple, second enemy will be dark green"... and so on.

  • Switching to a fixed palette would have another advantage: a clear exception state. Currently, if you have (by my math) 33 enemies on screen and try to spawn a 34th, the duplicate color check code will loop infinitely (or blow the call stack). That sets off alarm bells for me. On the other hand, if you've got a static readonly Queue<Color32> ENEMY_PALETTE, you can easily check for that:

Color32 getRandomColor() // better yet, something like "getNextEnemyColor()"
{
    if (!ENEMY_PALETTE.TryDequeue(out var nextColor))
    {
        throw new InvalidOperationException("Failed to assign color: Too many enemies");
    }
    return nextColor;
}

Not directly related, but possibly useful: Here is the jsfiddle I whipped up to see what different color generation algorithms might do, and here is the DotNetFiddle I whipped up to test FindObjectsOfType<T> and .Contains(structInstance).

\$\endgroup\$
  • 1
    \$\begingroup\$ Thanks you! You have some great ideas. Some of the things you talked about like the ENEMY_PALETTE I had thought about, but had not decided yet. Coming from java, i'm not 100% sure how the var data type works, I get the basic idea, but have not used it. I really like the IEnumerable<T> FindObjectsOfType<T>() function. Some C# syntax is unknown to me (Time for some research) such as the LINQ , throw new InvalidOperationException, and Select(enemy => enemy.color). Probably due to my heavy java usage throughout the past few years. Anyway, great (partial) review, and thank you for your help! \$\endgroup\$ – Clint Nov 13 '18 at 1:28

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.