patterncsharpMinor
Spell and cooldown system for an Unity game
Viewed 0 times
spellunitysystemcooldowngameforand
Problem
So I have doubts about my code and if it is any good. I have a parent class Spell:
Then one of the child classes:
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
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
You can then write activateSpell like this:
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.
Instead of hard coding in
Variable names should be lowerCamelCase, so
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.