DiceRoll system for board game

Unity Engine
, , , ,
1

Hi, Im making board game for 4 players (not online), I created some sort of Dice Roll System but now I am thinking, what would be best solution to make it work with different systems in the future, for example, players use 1 dice for movement, 2 dices for fighting. Enemies want to " roll" dice/dices too in fight scenarious. Do I just create Instance of DiceController class everytime I want something to have and “roll” dices? I would do this but I feel like something is wrong with this solution. Its kinda my first time doing something like it with OOP.

using UnityEngine;
using Random = UnityEngine.Random;

public class DiceController : MonoBehaviour
{
    List<Dice> dices = new List<Dice>();
    int totalvalue;
        
    private class Dice
    {
        int numberOfSides = 6;
        
        public int throwDice()
        {
            return Random.Range(1, numberOfSides + 1);
        }
    }

    public void CreateDices(int numberOfDices)
    {
        dices.Clear();
        for (int i = 0; i < numberOfDices; i++)
        {
            Dice dice = new Dice();
            Debug.Log($"Dice is created, totalDices {dices.Count + 1}");
            dices.Add(dice);
        }
    }
    
    public int throwDices()
    {
        if (dices.Count == 0)
        {
            return 0;
        }
        for (int i = 0; i < dices.Count; i++)
        {
            totalvalue += dices[i].throwDice();
        }
        return totalvalue;

    }

    public void Update()
    {
        if (Input.GetKeyDown(KeyCode.Space))
        {
            var value = 0;
            CreateDices(4);
            
            value = throwDices();
            Debug.Log(value);
        }
    }
}
2

I can safely say “I wouldn’t do it this way” :slight_smile:

I might start by defining a “dice” but I we might be stuck naming it Die since it is singular. I wouldn’t make it a private class at this point. You’ve defined a field named numberOfSides but note that this isn’t really used for anything important. It must be 6 so a const would work but more to the point there are two values that you actually want to define. MinValue and MaxValue are values you actually use.

You would seemingly want the class to retain the “last roll” value in case something needed to check. I wouldn’t name the method throwDice(). The lowercase “t” is atypical and you know it is related to the Dice so public int Roll() seems like a good choice.

It would return the value (as it does) but also assign it to a public int LastRoll property.

You absolutely do not want to be creating the dice in Update each time you roll them.

3

Maybe something like the following:

    using UERandom = UnityEngine.Random;

    public class Die
    {
        private const Int32 minValue = 1;
        private const Int32 maxValue = 6;

        public Int32 Value { get; private set; }

        public Die()
        {
            Value = minValue;
        }

        public Int32 Roll()
        {
            Value = UERandom.Range(minValue, maxValue);
            return Value;
        }
    }
4

I figured I would continue for fun… I call the set “Dice” and would suggest that you create as many unique Dice objects as you would need. So one with a single Die if you just need a single roll and one with 2 dies, or 4 if you need them. It keeps the logic and values separated.

    public class Dice
    {
        private readonly List<Die> _dies;

        public Dice(Int32 dies)
        {
            _dies = new List<Die>();

            for (Int32 i = 0; i < dies; i++)
            {
                _dies.Add(new Die());
            }
        }

        
        public Int32 Roll()
        {
            var result = 0;

            foreach (var die in _dies)
            {
                result += die.Roll();
            }

            return result;
        }


        public Int32 Value()
        {
            var result = 0;

            foreach (var die in _dies)
            {
                result += die.Value;
            }

            return result;
        }
    }
1 Like