CODEWITHSUNDEEP
c#
Reed
Reed

Reputation: 1642

C# random seed generator of multiple instances

I'm making a simple game to improve my C# skills. I read about the Random() class and how it generates a seed off of datetime and found some results of how to get a pseudorandom generator from one instance.

Player Class:

    Random dice = new Random();
    public int RollDice()
    {
        int dice1 = dice.Next(1, 7);
        int dice2 = dice.Next(1, 7);
        int sum = dice1 + dice2;
        Console.WriteLine("D1: " + dice1 + " D2: " + dice2 + " SUM: " + sum);
        return sum;
    }

Main:

//infiniteloop{
      player1.RollDice();
      Console.ReadKey();
      player2.RollDice();
  }

Upon output, no longer how long I wait to press the key and re-call the player2.DiceRoll() it still will roll the same numbers. If I only have one player, it works perfectly. How can I improve this?

Upvotes: 1

Views: 1486

Answers (4)

paul
paul

Reputation: 22001

As the other answers point out, your problem is that you are using two (identically seeded) random number generators that will each generate the same sequence. There are a few ways around this:

Option One: Seed each Random differently in the player constructor

Player(int seed) {
    this.dice = new Random(seed);
}

then in your main.cs

var player1 = new Player(1);
var player2 = new Player(2);

Option 2: Create a single Random and pass it in to your constructor (or your calls to RollDice)

This method has been detailed by @sdgfsdh

Option 3: Make the Random in Player static

Random dice = new Random();

changes to

static Random dice = new Random();

This means that however many Players you create, they will all use the same Random, avoiding your original problem.

Upvotes: 0

poke
poke

Reputation: 387697

Pseudorandom number generators are seeded once when they are created. After being seeded, they follow a fixed cycle through the numbers (based on their seed) to generate the next number.

When using the Random constructor without an argument, the current time is used as the seed. In all subsequent calls where a number is actually generated, the current time is no longer being used, so it does not matter how long you wait between those calls.

The problem you are seeing is that every player has a Random object of its own and those are created at the same time. See the following example:

// these are created pretty much at the same time
var r1 = new Random();
var r2 = new Random();


Console.WriteLine(r1.Next(1, 7));
Console.WriteLine(r2.Next(1, 7));

Console.WriteLine(r1.Next(1, 7));
Console.WriteLine(r2.Next(1, 7));

Console.WriteLine(r1.Next(1, 7));
Console.WriteLine(r2.Next(1, 7));

Console.WriteLine(r1.Next(1, 7));
Console.WriteLine(r2.Next(1, 7));

If you run that code, you will see that the numbers from the two random generators are always the same. This is because they are seeded with the same time.

In order to fix this, you would have to seed those generators differently, or make sure that one is actually being created after the other.

A much better solution however would be to introduce a single object that is responsible for creating random die throws. Your Player objects would then use the same die generator which only has a single random number generator. So the random numbers would come from the same generator, preventing them from being the same. Something like this:

public class DieGenerator
{
    private Random rand = new Random();

    public int Roll()
    {
        return rand.Next(1, 7);
    }
}

You would then create one object of this and pass it to the Player so they can use it to roll the dice instead of relying on their own random number generator.

Upvotes: 3

sdgfsdh
sdgfsdh

Reputation: 37045

Most likely dice is constructed at the same time for both players, which gives them both the same seed. I would recommended creating one dice for both players: 

// Removed "Random dice = new Random(); "

public int RollDice(Random dice)
{
    int dice1 = dice.Next(1, 7);
    int dice2 = dice.Next(1, 7);
    int sum = dice1 + dice2;
    Console.WriteLine("D1: " + dice1 + " D2: " + dice2 + " SUM: " + sum);
    return sum;
}


// Main
var dice = new Random();
while (true) 
{
    player1.RollDice(dice);
    Console.ReadKey();
    player2.RollDice(dice);
}

Upvotes: 0

InBetween
InBetween

Reputation: 32760

Your random generator dice is not created at the point where player2.RollDice() is called. Its created when you create the player2 instance which, I assume, is created at the same time player1 is and thats why you are seeing the same rolls; both dice have the same seed.

To fix this, one way is to create one unique random generator and inject it in Player instances.

For example, injecting via constructor would look like this:

public class Player
{
     private readonly Random dice;

     public Player(Random dice)
     {
         Debug.Assert(dice != null);
         this.dice = dice;
     }

     public int RollDice() => dice.Next(1, 7);
}

And you'd use it like this:

var dice = new Random();
var player1 = new Player(dice);
var player2 = new Player(dice);

Upvotes: 0

Related Questions