Help me get my code cleaner/more optimal

I have a very basic (well i think so ) script. That let the player allocate some stats to his character
3111896--235270--Unity_2017-06-17_22-31-40.png

and i also have create a Canvas with button that let met Add (the + sign) and Remove (the - sign) Stats to the Character
3111896--235271--Unity_2017-06-17_22-31-54.png 3111896--235272--Unity_2017-06-17_22-32-02.png

And finnally here is my C# script that assigne those value to the Static GameInformation script (so those variable can be access anywhere in the futur)

    public void SetAttackDamage(int amount)
    {
            if(amount > 0 && GameInformation.StatsPointsToAllocate > 0)
            {
                GameInformation.AttackDamage += amount;
                GameInformation.StatsPointsToAllocate--;
            }
            else if(amount < 0 )//&& GameInformation.AttackDamage > GameInformation.PlayerClass.AttackDamage)//ont peut pas descendre plus bas que la class
            {
                GameInformation.AttackDamage += amount;
                GameInformation.StatsPointsToAllocate++;
            }
            IndividualStatsUpdate(damagetext, GameInformation.AttackDamage);
            StatsToAllocate();
    }
    public void SetAttackSpeed(int amount)
    {
        if(amount > 0 && GameInformation.StatsPointsToAllocate > 0)
        {
            GameInformation.AttackSpeed += amount;
            GameInformation.StatsPointsToAllocate--;
        }

        else if(amount < 0)//&& GameInformation.AttackDamage > GameInformation.PlayerClass.AttackDamage)//ont peut pas descendre plus bas que la class
        {
            GameInformation.AttackSpeed += amount;
            GameInformation.StatsPointsToAllocate++;
        }
        IndividualStatsUpdate(attackSpeedText, GameInformation.AttackSpeed);
        StatsToAllocate();
    }

and so on for all the Character Stats available.

Is there a way to make this better ? like a public void SetStats(int amount, string nameoftheStats…

? Thank you !

ps: the code is working great and doing what it is suppose to do … but im pretty sure it can be better
pss: sorry for my bad english i am a french canadien in the learning process ^^

It looks good to me. You might want to check for your “points” being < 0 but other than that, nothing springs to mind.

A couple of options come to mind.

Check out using a dictionary. That way you can just pass in the key and the operation, then let it happen.

Another alternative is to wrap each stat in a stat class. Then you can pass stay objects around.

Either way you can avoid having to write out identical methods for each stat.

i do have a base statsclass that look like that

public class BaseClass
{
    #region Stats que je veux
    private string _name;
    private string _className;
    private string _classDescription;
    private int _healthCurrent;
    private int _healthMax;
    private int _levelCurrent;
    private int _levelMax;
    private int _expCurrent;
    private int _expToNextLevel;
    private int _resistance;
    private int _regeneration;
    private int _attackDamage;
    private int _attackSpeed;
    private int _skillCooldown;
    private int _skillDamage;
    private int _expToGive;
    #endregion


    public string Name { get; set; }
    public string ClassName { get; set; }
    public string ClassDescription { get; set; }
    public int HealthCurrent { get; set; }
    public int HealthMax { get; set; }
    public int LevelCurrent { get; set; }
    public int LevelMax { get; set; }
    public int ExpCurrent { get; set; }
    public int ExpToNextLevel { get; set; }
    public int Resistance { get; set; }
    public int Regeneration { get; set; }
    public int AttackDamage { get; set; }
    public int AttackSpeed { get; set; }
    public int SkillCooldown { get; set; }
    public int SkillDamage { get; set; }
    public int ExpToGive { get; set; }
}

and also some "alternative" class

public class BaseArcherClass : BaseClass
{
    public BaseArcherClass()
    {
        ClassName = "Archer";
        ClassDescription = "A Fierce Arche. Long range expert";
        HealthMax = 15;
        HealthCurrent = HealthMax;
        LevelCurrent = 1;
        LevelMax = 50;
        ExpCurrent = 0;
        Resistance = 0;
        Regeneration = 0;
        AttackDamage = 0;
        AttackSpeed = 0;
        SkillCooldown = 0;
        SkillDamage = 0;
    }
}

I was thinking something more like this.

public class Stat {
    string name;
    int value;
}

public class CharacterStats {
    List<Stat> CharacterStats;
}

Do you mind giving a bigger example ? I actually never used list. Thank you

I think what @Kiwasi means is that the concept of a stat in your case is actually only two variables big: the name of the stat and the value of the stat.
so you could keep a list of stats each stat just having a different name and/or value.
a list is a IEnumerable collection (like an array) only a list is dynamic, it can change size during runtime, so you can add or remove objects.
It wouldn’t be necessary, you might as well use and array (but i prefer lists as well)
for example:

public Stat[] stats;

you could populate the array in the inspector or by code.
Populate list by code example:

public string[] statNames;
public List<Stat> stats = new List<Stat>();

void Start()
{
    for(int i=0; i<statNames.Length; i++)
    {
        Stat stat = new Stat();
        stat.name = statNames[i];
        stat.value = 1;
        stats.Add(stat);
    }
}

this way you fill in the statNames in the inspector and when you press play, your list of stats would be filled with all stats starting with value 1 and having a names from statNames.

1 Like

And how would I access the x value and what about me different classes ? (Warrior have more hp than the archer example) sorry if that sound like a newbie question.
Thank for the time @jister

hmm in so many different ways :slight_smile:
let me give you one of them. Take note that this creates a new clean character every time you press play, so things would change if you have things like save and loading characters (the code in the start function in Character class would no be called there)
Stat class:

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

[System.Serializable]
public class Stat
{
    public string name;
    public int value;
    public Stat(string name, int value)
    {
        this.name = name;
        this.value = value;
    }
}

CharacterBase class:

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

[System.Serializable]
public class CharacterBase
{
    public List<Stat> characterStats = new List<Stat>();
    private string[] baseStats = new string[]{"Health", "Damage", "Stamina", "Strength", "Fitness"};

    public CharacterBase()
    {
        for(int i = 0; i<baseStats.Length; i++)
        {
            Stat s = new Stat(baseStats[i], 1);
            characterStats.Add(s);
        }
    }
    public Stat GetStat(string name)
    {
        return characterStats.Find(x => x.name == name);
    }

    public virtual void DoSomethingAmazing()
    {
        Debug.Log("Base did something amazing");
    }
}
public enum CharacterType
{
    Warrior,
    Monk,
    Mage
}

CharacterBase Child classes:

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

[System.Serializable]
public class Warrior : CharacterBase
{
    public Warrior(): base()
    {
        //add custom Warrior stats or adjust values
        Stat s = GetStat("Damage");
        s.value = 5;
    }
    public override void DoSomethingAmazing()
    {
        Debug.Log("Warrior did something amazing");
    }
}
[System.Serializable]
public class Monk : CharacterBase
{
    public Monk(): base()
    {
        //add custom Monk stats or adjust values
        Stat s = new Stat("Zen", 3);
        characterStats.Add(s);
    }
    public override void DoSomethingAmazing()
    {
        Debug.Log("Monk did something amazing");
    }
}
[System.Serializable]
public class Mage :CharacterBase
{
    public Mage(): base()
    {
        //add custom mage stats
        Stat s = GetStat("Stamina");
        s.value = 0;
        Stat newStat = new Stat("Magic", 4);
        characterStats.Add(newStat);
    }
    public override void DoSomethingAmazing()
    {
        Debug.Log("Mage did something amazing");
    }
}

A MonoBehaviour Character class:

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

public class Character : MonoBehaviour {

    public CharacterType characterType;
    public CharacterBase character;

    // Use this for initialization
    void Start ()
    {
        character = CharacterFactory.CreateCharacter(characterType);
    }
 
    // Update is called once per frame
    void Update ()
    {
        if(Input.GetKeyDown(KeyCode.Space))
            character.DoSomethingAmazing();
    }
}

and a CharacterFactory class

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

public class CharacterFactory
{
    public static CharacterBase CreateCharacter(CharacterType characterType)
    {
        Type t = Type.GetType(characterType.ToString());
        return (CharacterBase)Activator.CreateInstance(t);
    }
}

there’s a package attached to see how to set it up

3114925–235551–CharacterCreator.unitypackage (7.3 KB)

1 Like

Wow !! was not hoping to have this much information !! im really glad !! i will defenetly take a look and play around to take this approch. a for loop will be much faster than writting all variable one by one