CODEWITHSUNDEEP
c#windows-phone-7xnaxna-4.0
smohamed
smohamed

Reputation: 3314

XNA Randomly chosen Texture always picks same texture

I made this code to load about 8 different textures to a list of objects with a texture property.

I have a folder with textures named "1.png, 2.png, 3.png,.......,46.png" and I want the 8 different objects to be loaded with randomly chosen textures.

        DockedFruitsList = new List<Fruit>(8);
        for (int i = 0; i < 8; i++)
        {
            Fruit temp = new Fruit();
            temp = new Fruit();
            temp.Position = AvailablePositions[i];
            int random=(new Random().Next(0, 4600) % 46);
            temp.Texture = Content.Load<Texture2D>(@"Fruits/" + random);
            DockedFruitsList.Add(temp);          

        }

The thing is, despite the random always generates a different number, the result of draw is always the same texture, it changes from a run to another run, but it's always the same for all the 8 textures

spriteBatch.Begin();
        for (int i = 0; i < DockedFruitsList.Count; i++)
        {
            spriteBatch.Draw(DockedFruitsList[i].Texture, DockedFruitsList[i].Position, Color.White);
        }
        spriteBatch.End();

Upvotes: 2

Views: 1778

Answers (4)

CodesInChaos
CodesInChaos

Reputation: 108880

Random is seeded by the time(GetTickCount()), the time changes only every few milliseconds. So all instances of random created within that interval will return the same sequence. The correct thing is only creating a single instance of Random outside the loop.

        DockedFruitsList = new List<Fruit>(8);
        Random rand = new Random();
        for (int i = 0; i < 8; i++)
        {
            Fruit temp = new Fruit();
            temp = new Fruit();
            temp.Position = AvailablePositions[i];
            int random=(rand.Next(1, 46));
            temp.Texture = Content.Load<Texture2D>(@"Fruits/" + random);
            DockedFruitsList.Add(temp);          

        }

Check Revisiting randomness for more details.

There is also no reason to use rand.Next(0, 4600)%46. You can use rand.Next(0,46) directly.

Note that this code will still return duplicate entries. If you want to avoid that entirely you can use:

Random rand=new Random();
foreach(int randomFruid in Range(1,46).Shuffle(rand).Take(8))
{
   ...
}

With Shuffle from Is using Random and OrderBy a good shuffle algorithm?

Upvotes: 0

George Duckett
George Duckett

Reputation: 32468

From the msdn:

The random number generation starts from a seed value. If the same seed is used repeatedly, the same series of numbers is generated. By default, the parameterless constructor of the Random class uses the system clock to generate its seed value

System clock it uses only changes its value every few milliseconds, so if you call new Random() very quickly it'll get seeded with the same number, thus the calls to Next... will yield identical results.

Note that if you debugged it, stepping through the code gave the system clock time to advance, thus you won't get the error unless it runs through the loop without you breaking / stepping through.

Create the Random object once, and reuse it (note i'm now converting random to string and i've called a different method on random too):

    var rand = new Random();
    DockedFruitsList = new List<Fruit>(8);
    for (int i = 0; i < 8; i++)
    {
        Fruit temp = new Fruit();
        temp = new Fruit();
        temp.Position = AvailablePositions[i];
        int random=(rand.Next(46) + 1);
        temp.Texture = Content.Load<Texture2D>(@"Fruits/" + random.ToString());
        DockedFruitsList.Add(temp);          

    }

Upvotes: 2

Lorenzo Dematt&#233;
Lorenzo Dematt&#233;

Reputation: 7849

new Random().Next(0, 4600)

means you initialize the rng every time (maybe with the same seed?) Move the new outside the method (static member or similar).

Upvotes: 0

porges
porges

Reputation: 30590

Try this out:

for (int i = 0; i < 100; i++)
    Console.WriteLine(new Random().Next(0, 10));

Odds are you'll get an output like this:

5
5
5
.
.
.

This is because the seed of the Random class is initialized with a time-based value. If you initialize many instances of the Random class near the same point in time, then they will all end up with the same seed, and so the values they will produce will all be the same.

What you really want to do is to create one Random instance and use that for all your random values. The values generated will have no relation to each other:

var r = new Random();
for (int i = 0; i < 100; i++)
    Console.WriteLine(r.Next(0, 10));

Output:

2
8
0
.
.
.

Upvotes: 5

Related Questions