3

I've been coding a text adventure game in C#. I want to do a scene where you encounter a pokemon. I want to have 3 random pokemon to encounter, and I made a method that returns the name of the pokemon you find. Code is:

public string choosePokemon()  
{  
    Random random = new Random();  
    int pokemonChosen = random.Next();  
    if (pokemonChosen == 1)  
    {  
        string pokemon = "Pidgey";  
        return pokemon;  
    }  
    if (pokemonChosen == 2)   
    {  
        string pokemon = "Charmander";  
        return pokemon;  
    }  
    if (pokemonChosen == 3)  
    {  
        string pokemon = "Geodude";  
        return pokemon;  
    }  
    return "missingno";  
} 

Whenever I execute the code it says "You encountered a missingno!". I want it to return the name and break the method, returning to the method of the scene. I put a return "missingno"; at the end because Studio would give me an error that not all code paths return a value.

4
  • 2
    What numbers do you think random.Next(); can yield? Edit: Well, you edited away the Next() call with no arguments. Oct 4, 2013 at 21:19
  • 1
    In addition to all the answers below that pretty much explain the problem, you probably shouldn't create a new instance of Random each time the method is called
    – HasaniH
    Oct 4, 2013 at 21:23
  • You are recreating the random class every call, this will start with the same seed over and over again. So it will always return the same sequence but only selecting the first one. (see my answer) Oct 4, 2013 at 21:27
  • This method is only called once, since it's only for one scene. However, if the user dies, then it's game over and they restart. Meaning, this method might be called again (random scenarios).
    – Ilan
    Oct 4, 2013 at 21:35

10 Answers 10

10

Random.Next() (with no parameters) will return a random non-negative integer. You probably meant to constrain it to a certain range, like this:

int pokemonChosen = random.Next(1, 4);

Also note, you can make your code a bit cleaner by just returning a constant inside your if-block like this:

if (pokemonChosen == 1)  
{  
    return "Pidgey";  
}  

Or even better, encapsulate your random options in an array. That way you can get rid of all your if-blocks entirely:

var options = new [] { "Pidgey", "Charmander", "Geodude" };
int pokemonChosen = random.Next(options.Length); // array indexes start at 0
return options[pokemonChosen];
0
10

The problem is that your random.Next() call is returning a number anywhere in the range of random - You need an alternate version of random.Next() that returns only 1, 2, or 3, or you need to perform that yourself. Try doing this instead: int pokemonChosen = random.Next(1, 4) (The lower bound is inclusive while the upper bound is exclusive)

9

A good way to do this, as I've found through trials with my own Pokemon based game, is to put the New Random() outside the function. Also, you could instantiate the string outside the function, as well, in case you have different Pokemon options for different parts of the game. This way, you wouldn't have to return anything, just call the function then use the string. Assuming you're doing a windows based with a label for story output:

Random random = new Random();
string pokemon;

    public void choosePokemon()  
    {   
        int pokemonChosen = random.Next(2);  
        if (pokemonChosen == 0) {
        pokemon = "Pidgey";
        }
        if (pokemonChosen == 1) {
        pokemon = "Charmander";
        }
        if (pokemonChosen == 2) {
        pokemon = "Geodude";
        }
    } 

    lblStory.Text += "You encountered a wild " + pokemon + "!";

I did something very similar for a game I made for a friend.

EDIT: Also, something else I did in my game was to shorten pokemon in variable names to pkmn. Shorter to type, and as seen through the main games, commonly accepted. Not that the players will read your code.

8

Lets start with the Random number generator. To be effective, you should only instantiate Random once. To do this, you might make a static instance of it for the class. If you don't, it's possible to get the same number back each time you call random.Next().

public class PokemonChooser
{
    private static Random random = new Random();

    public static string ChoosePokemon() { ... }
}

Then, we need to realize that random.Next() returns any non-negative integer value, not just the ones you want. So, we need to round the result to something that's useful for your case. Since you have 3 items from which you would like to randomly choose, we can round it using the modulus operator (%), which basically gives you the remainder of a division operation (/). For instance: (int)(5 / 2) == 2, and 5 % 2 == 1.

var pokemonChosen = random.Next() % 3;

Next, since this result is zero-based, we'll need to account for that in the if statements.

      if (pokemonChosen == 0)
      {
        string pokemon = "Pidgey";
        return pokemon;
      }

      if (pokemonChosen == 1)
      {
        string pokemon = "Charmander";
        return pokemon;
      }

      if (pokemonChosen == 2)
      {
        string pokemon = "Geodude";
        return pokemon;
      }

Since the Pokemon names are pretty obvious, we can get rid of the pokemon variable without losing any readability.

      if (pokemonChosen == 0)
      {
        return "Pidgey";
      }

      if (pokemonChosen == 1)
      {
        return "Charmander";
      }

      if (pokemonChosen == 2)
      {
        return "Geodude";
      }

We could probably make this clearer and shorter by using a case statement instead of multiple if statements.

      switch (pokemonChosen) {
        case (0): return "Pidgey";
        case (1): return "Charmander";
        case (2): return "Geodude";
      }

And since we simply return "missingno" if the we cant find the pokemon by number, we can use a default case:

      switch (pokemonChosen) {
        case (0): return "Pidgey";
        case (1): return "Charmander";
        case (2): return "Geodude";
        default: return "missingno";
      }

All in all, we end up with:

public class PokemonChooser
{
    private static Random random = new Random();

    public static string ChoosePokemon()
    {
      var pokemonChosen = random.Next() % 3;

      switch (pokemonChosen) {
        case (0): return "Pidgey";
        case (1): return "Charmander";
        case (2): return "Geodude";
        default: return "missingno";
      }
    }
}

And we use it like so:

var pokemon = PokemonChooser.ChoosePokemon();

However, you may want to choose a higher number than 3 in random.Next() % 3, otherwise, "missingno" will never be returned.

2
  • Don't use a modulo operation to limit the output of a function returning a random integer, as doing so makes the largest possible number less likely to come up (in this case random.Next() % 3 is less likely to return 2 than 1 and 0). You should instead multiply random.Next() / 2147483647.0 by 3 and then roll the result down using Math.Floor().
    – SE is dead
    Sep 22, 2016 at 21:45
  • Technically, you're correct. However, this answer was primarily to demonstrate the error on the original code being a range error. Adding superfluous math would have distracted from the OP's understanding of the underlying issue, IMO. Were the question about the fairness and accuracy of random numbers, using random.Next() % n would have been a very bad idea.
    – cwharris
    Sep 26, 2016 at 18:35
4

You need to specify the upper bound for random.Next():

int pokemonChosen = random.Next(4);

would return an integer in the range 0 - 3.

To have it return 1-4 you could use:

int pokemonChosen = random.Next(1, 5);
3

Random.next() will return a value between 0 and 2,147,483,647 (= Integer.MaxValue) so the chance of your number being 1,2 or 3 is slim, that's way it usually returns the last value. You need to limit the value:

random.Next(1, 4);
3

You shouldn't recreate the Random class every call. And call it like: _random.Next(3) + 1

Random _random = new Random();  

public string choosePokemon()  
{  
     int pokemonChosen = _random.Next(3) + 1;  
     if (pokemonChosen == 1)  
     {  
         string pokemon = "Pidgey";  
         return pokemon;  
     }  
     if (pokemonChosen == 2)   
     {  
         string pokemon = "Charmander";  
         return pokemon;  
     }  
     if (pokemonChosen == 3)  
     {  
         string pokemon = "Geodude";  
         return pokemon;  
    }  
    return "missingno";  
} 
3

You need to specify the range of the random value:

int pokemonChosen = random.Next(3) + 1;

Otherwise, the range of values is up to Int32.MaxValue, which gives you about a one-in-a-billion chance of being between 1-4.

1
  • I understood my mistake a minute after I posted this, thanks a lot!
    – Ilan
    Oct 4, 2013 at 21:20
2

You are gettting a value other than 1, 2, or 3, debug it and check what the value of pokemonChosen is. You may wish to limit max value of your random val to be 3

int pokemonChosen = random.Next(4);
2

tested, your code works correctly - not sure what code you actually used, the exact method that you have always works correctly. however, you should change it to

int pokemonChosen = random.Next(1, 4);  

Upd: hey, way to go changing that line in your original question, when I opened it you had int pokemonChosen = random.Next(1, 3); in there...

1
  • Yeah, I edited it because it had a mistake then I realized that /was/ the mistake. Sorry, it's back now.
    – Ilan
    Oct 4, 2013 at 21:41

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.