HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Spell and cooldown system for an Unity game

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
spellunitysystemcooldowngameforand

Problem

So I have doubts about my code and if it is any good. I have a parent class Spell:

using UnityEngine;
using System.Collections;

public class Spell : MonoBehaviour {

    public enum SpellType { AttackType, AreaType, BuffType, DebuffType };

    public float damage;
    public float speed;
    public float cooldowntime;
    public SpellType type;

    public Spell()
    {

    }

    void Start () {

    }

    protected void Update () {
        if(type == SpellType.AttackType)
        {
            updateSpell();
        }
    }

    protected virtual void updateSpell()
    {

    }

    protected void deleteSpell()
    {
        Destroy(gameObject);
    }

    void OnTriggerEnter2D(Collider2D other)
    {
        if(other.tag == "Monster")
        {
            Interface.IMonster inter = other.GetComponent();
            inter.takeDamage(damage);
            deleteSpell();
        }
    }
}


Then one of the child classes:

using UnityEngine;
using System.Collections;

class WaterBolt : Spell
{
    private Vector3 mouseposition;
    private Vector3 screenpoint;
    private Vector2 offset;
    private float angle;

    void Start()
    {
        mouseposition = Input.mousePosition;
        screenpoint = Camera.main.WorldToScreenPoint(transform.localPosition);
        offset = new Vector2(mouseposition.x - screenpoint.x, mouseposition.y - screenpoint.y);
        angle = Mathf.Atan2(offset.y, offset.x) * Mathf.Rad2Deg;
        transform.rotation = Quaternion.Euler(0, 0, angle);

        Invoke("deleteSpell", 5f);
    }

    protected override void updateSpell()
    {
        transform.position += transform.right * speed * Time.deltaTime;
    }

}


Then this is what pretty much displays and drives the spell system with cooldown. Its a bit long and I would like to shorten but I can't think of something that would help.
using UnityEngine;
using System.Collections;

```
using UnityEngine.UI;
using System.Collections.Generic;

public class GuiSpellHandler

Solution

You can get rid of a lot of repetition in activateSpell() by storing things in arrays and accessing them by the activeNumber. You can set up your arrays like so:

private GameObject[] spells;
private float[] spellCooldowns;
private float[] spellCooldownNumbers;

void Start () {
    player = GameObject.FindWithTag("Player");
    spells = new[] { spell1, spell2...
    spellCooldowns = new[] { spellcooldown1, spellcooldown2...
    spellCooldownNumbers = new[] { spellcooldownnumber1, spellcooldownnumber2...
}


You can then write activateSpell like this:

public void activateSpell(Transform trans)
{
    int i = activenumber;
    if (spellCooldowns[i] == 0 || spellCooldowns[i] ();
        spellCooldowns[i] = Time.time + spell.cooldowntime;
        spellCooldownNumber[i] = spell.cooldowntime;
    }
}


If you want, you can create a class to store a spell, its cooldown, and its cooldown number, so then you would only need to create one array.

You can use modulo arithmetic to make sure a number is within a certain range. scrollSpells() can be written like this:

void scrollSpells()
{
    if (Input.GetAxis("Mouse ScrollWheel") > 0f) // forward
    {
        activenumber = (activenumber + 1) % 7;
    }
    else if (Input.GetAxis("Mouse ScrollWheel") < 0f) // backwards
    {
        activenumber = (activenumber + 7 - 1) % 7;
    }
}


Instead of hard coding in 7 you can use the number of spells in the array.

void scrollSpells()
{
    if (Input.GetAxis("Mouse ScrollWheel") > 0f) // forward
    {
        activenumber = (activenumber + 1) % spells.Count();
    }
    else if (Input.GetAxis("Mouse ScrollWheel") < 0f) // backwards
    {
        activenumber = (activenumber + spells.Count() - 1) % spells.Count();
    }
}


Variable names should be lowerCamelCase, so activenumber should be activeNumber. Method names should be UpperCamelCase, so activateSpell() should be ActivateSpell().

Code Snippets

private GameObject[] spells;
private float[] spellCooldowns;
private float[] spellCooldownNumbers;

void Start () {
    player = GameObject.FindWithTag("Player");
    spells = new[] { spell1, spell2...
    spellCooldowns = new[] { spellcooldown1, spellcooldown2...
    spellCooldownNumbers = new[] { spellcooldownnumber1, spellcooldownnumber2...
}
public void activateSpell(Transform trans)
{
    int i = activenumber;
    if (spellCooldowns[i] == 0 || spellCooldowns[i] < Time.time)
    {
        Instantiate(spells[i], trans.position, trans.rotation);
        Spell spell = spells[i].GetComponent<Spell>();
        spellCooldowns[i] = Time.time + spell.cooldowntime;
        spellCooldownNumber[i] = spell.cooldowntime;
    }
}
void scrollSpells()
{
    if (Input.GetAxis("Mouse ScrollWheel") > 0f) // forward
    {
        activenumber = (activenumber + 1) % 7;
    }
    else if (Input.GetAxis("Mouse ScrollWheel") < 0f) // backwards
    {
        activenumber = (activenumber + 7 - 1) % 7;
    }
}
void scrollSpells()
{
    if (Input.GetAxis("Mouse ScrollWheel") > 0f) // forward
    {
        activenumber = (activenumber + 1) % spells.Count();
    }
    else if (Input.GetAxis("Mouse ScrollWheel") < 0f) // backwards
    {
        activenumber = (activenumber + spells.Count() - 1) % spells.Count();
    }
}

Context

StackExchange Code Review Q#132517, answer score: 2

Revisions (0)

No revisions yet.