Probability Chance

Question

I'm creating a game and I need some properties to work as percent chance to hit/dodge. I implemented a small testing program that takes percent chance and the amount of hitting attempts as input and outputs the successful hits, failed hits, and overall success rate.

My game will have decimal percents (1.5, 54.8..) The way the code works is pretty simple, let's say you have 50 percent critical chance.

1. We take the % chance as input and multiply it by 10 so we can work properly with decimal %'s.

2. We put all the numbers from 1 up to your current % in a list.

3. We generate a random number from 1 - 1000 using RNGCryptoServiceProvider. The range here is 1 - 1000 because we previously multiplied the user % chance by 10 like this we equalize the sides.

4. Than we compare if the random number occurs in the list of our successful hits.

My main concern is if the program's generated numbers are random enough.

The main program:

    private static void Main()
    {
        while (true)
        {
            Console.Write("Percent Chance : ");
            double percentChance = double.Parse(Console.ReadLine())*10;
            Console.Write("Hit count : ");
            double hitCount = int.Parse(Console.ReadLine());
            List<int> criticalHits = new List<int>();
            for (int i = 1; i <= percentChance; i++)
            {
                criticalHits.Add(i);
            }
            RngCrypto rnd = new RngCrypto();
            int succesCritCount = 0;
            int failedCritCount = 0;
            for (int i = 1; i <= hitCount; i++)
            {
                int hitThatIsCritical = rnd.Next(1, 1001);
                if(criticalHits.Any(criticalHit => criticalHit == hitThatIsCritical))
                {
                    Console.WriteLine("Hit #{0} was a critical", i);
                    succesCritCount++;
                }
                else
                {
                    Console.WriteLine("Hit #{0} was not a critical", i);
                    failedCritCount++;
                }
            }
            double succesRate = succesCritCount/hitCount;
            Console.WriteLine("Succesful Hits : {0}",succesCritCount);
            Console.WriteLine("Failed Hits : {0}", failedCritCount);
            Console.WriteLine("Succes Rate : {0}",succesRate);
        }
    }

RngCrypto class:

using System;
using System.Security.Cryptography;
public class RngCrypto
{
    private const int BufferSize = 1024;  // must be a multiple of 4
    private readonly byte[] _randomBuffer;
    private int _bufferOffset;
    private readonly RNGCryptoServiceProvider _rng;
    public RngCrypto()
    {
        _randomBuffer = new byte[BufferSize];
        _rng = new RNGCryptoServiceProvider();
        _bufferOffset = _randomBuffer.Length;
    }
    private void FillBuffer()
    {
        _rng.GetBytes(_randomBuffer);
        _bufferOffset = 0;
    }
    public int Next()
    {
        if (_bufferOffset >= _randomBuffer.Length)
        {
            FillBuffer();
        }
        int val = BitConverter.ToInt32(_randomBuffer, _bufferOffset) & 0x7fffffff;
        _bufferOffset += sizeof(int);
        return val;
    }
    public int Next(int maxValue)
    {
        return Next() % maxValue;
    }
    public int Next(int minValue, int maxValue)
    {
        if (maxValue < minValue)
        {
            throw new ArgumentOutOfRangeException(@"maxValue must be greater than or equal to minValue");
        }
        int range = maxValue - minValue;
        return minValue + Next(range);
    }
    public double NextDouble()
    {
        int val = Next();
        return (double)val / int.MaxValue;
    }
    public void GetBytes(byte[] buff)
    {
        _rng.GetBytes(buff);
    }
}

Answer 1

I don't know much about making numbers random enough, but I have some other suggestions.

Instead of checking if a range of integers contains a number you can just check if the number is >= the beginning of the range and <= the end of the range.

So you can replace this:

if(criticalHits.Any(criticalHit => criticalHit == hitThatIsCritical))

With this:

if(((int)percentChance) <= hitThatIsCritical)

Then you can get rid of this:

List<int> criticalHits = new List<int>();
for (int i = 1; i <= percentChance; i++)
{
    criticalHits.Add(i);
}

In the case where you do need to declare a range of integers, you can do so like this:

List<int> criticalHits = Enumerable.Range(1, percentChance).ToList();

Note that the second parameter is the count instead of the end number, so you'll have to keep that in mind when replacing a for loop.

You probably shouldn't hard code the logic about things being multiplied by 10. Instead you should use a constant for 10. That way it's easier to modify if you want to change what things get multiplied by and it's easier to tell what the code is doing. I'm going to use the _cConstantName naming convention for constants, but you can use another one if you want.

First declare the constant:

private const int _cPrecisionFactor = 10;

Then you can replace the following two lines:

double percentChance = double.Parse(Console.ReadLine())*10;

and:

int hitThatIsCritical = rnd.Next(1, 1001);

with:

double percentChance = double.Parse(Console.ReadLine()) * _cPrecisionFactor;

and:

int hitThatIsCritical = rnd.Next(1, 100 * _cPrecisionFactor + 1);

You should also validate the input by using TryParse() instead of Parse() and looping until you get a valid number.