5

Possible Duplicate:
Random number generator only generating one random number

I was a bit baffled with this few moments ago. I have the following code:

public blockType generateRandomBlock()
{
    Random random = new Random();
    int makeBlockOfType = random.Next(0, 100);

    blockType t = blockType.normal;
    if (makeBlockOfType <= 80 && makeBlockOfType >= 60)
    {
        t = blockType.blue;
    }
    else if (makeBlockOfType > 80 && makeBlockOfType <= 95)
    {
        t = blockType.orange;
    }
    else if (makeBlockOfType > 95 && makeBlockOfType <= 100)
    {
        t = blockType.green;
    }

    return t;
}

Fairly simple, it return an enum value based on a randomly generated number (based of system time). Unfortunately for some odd reason, I have all blocks either one color or the other even though this runs for every single block being put into the game. However, when I step through this with the debugger and then see the results after some run, I see that the blocks are now multi colored based on the chances provided. I am a little bit confused about why this is happening.

For this I am using MonoGame which uses the Mono compiler instead of the Microsoft one. Could this be the issue? I have tried to put this code inline into the constructor from where it is being called but I am getting the same result (I am guessing the compiler inlines the code anyways).

I have tried to restart Visual Studio it separately rather than letting the run do the build; no changes.

Any suggestions and help are greatly appreciated!

Community
  • 1
  • 1
Serguei Fedorov
  • 7,763
  • 9
  • 63
  • 94
  • 1
    try to move Random creation out of the method, that should help. – Arsen Mkrtchyan Dec 12 '12 at 14:17
  • `new Random()` seeds using the time, which remains constant for several milliseconds. – CodesInChaos Dec 12 '12 at 14:18
  • The reason it works in debug is that there is time between the calls and Random actually gets different seeds. – Archy Dec 12 '12 at 14:50
  • Hmmm, this makes sense. No explanation needed. However because the instance of the object is created and right away the block type is generated, the issue continues. Is it then smart therefore, to move the Random object into a global static class and refer to it every time? – Serguei Fedorov Dec 12 '12 at 15:12

3 Answers3

7

You should instanciate Random only once (set it as a private field and instanciate in the constructor), see the similar question : Random.Next returns always the same values

See the Random documentation :

The random number generation starts from a seed value. If the same seed is used repeatedly, the same series of numbers is generated

In your case, you create a Random instance with the same seed (too close in time) and you take the first value which will be the same for a given seed.

Community
  • 1
  • 1
AlexH
  • 2,650
  • 1
  • 27
  • 35
4

You are recreating your random number generator every time you call your method:

public blockType generateRandomBlock()
{
    Random random = new Random();

As the seed of the random number generator is based on the time this will return the same value for consecutive calls.

Move your creation of the generator outside the routine:

Random random = new Random();
public blockType generateRandomBlock()
{
ChrisF
  • 134,786
  • 31
  • 255
  • 325
3

When you create multiple instances of Random successively in a very brief period of time, they are likely to end up getting initialized with the same time-dependent seed value.

To work around this, you should initialize your Random as an instance field instead:

private readonly Random random = new Random();

public blockType generateRandomBlock()
{
    int makeBlockOfType = random.Next(0, 100);

    // ...
}
Douglas
  • 53,759
  • 13
  • 140
  • 188