1

I am having trouble generating random fruit for my Snake game. (I am very new to programming and this is my first language).

When I run my code all works fine so far (except from some minor issues). I'm using Visual Studio C++ in an empty project. Here is my full code (I'm not displaying my #includes):

using namespace std;
bool gameOver = false;
int gameScore;
int fruitX;
int fruitY;

string bGameW = "###########";
string bGameL = "#         #\n";

class gameStart
{
public:
void start() 
{   

    cout << bGameW;
    cout << bGameL;
    cout << bGameL;
    cout << bGameL;
    cout << bGameL;
    cout << bGameL;
    cout << bGameL;
    cout << bGameL;
    cout << bGameL;
    cout << bGameW; 

}

void generateFruit() 
{
    srand(time(NULL));
    fruitX = rand() % 21;
    fruitY = rand() % 21;

    bGameW.insert(fruitX, "F");
    bGameL.insert(fruitY, "F");

}

void clearscreen() 
{
    system("cls");
}
private:
};


 int main () 
 {  
 gameStart gameObj;

 gameObj.generateFruit();
 gameObj.clearscreen();
 gameObj.start();

 return 0;
 }

To generate the random the random fruit for the string. I use a string to make the game board, then I create random values for the fruit (X and Y) then I append them into my game board.

But the issue is: I want to make only one fruit with a random X and Y and append it into my game board to display it. But my current code is this:

bGameW.insert(fruitX, "F");
bGameL.insert(fruitY, "F");

This code makes 2 fruits with 1 at a random X and 1 at a random Y. I want to turn these 2 fruits into 1 fruit, with 1 random X and 1 random Y.

halfer
  • 19,824
  • 17
  • 99
  • 186
Coristat
  • 11
  • 4
  • 3
    You should call `srand` only *once*. Each time you call it you will reset the seed for the `rand` function. On almost all systems, if you call `generateFruit` multiple times each second, you will get the exact same random number. There's some duplicate of this somewhere on Stack Overflow. – Some programmer dude May 04 '20 at 14:18
  • 1
    why did you leave out the includes? If that is all which is missing to make the code you posted complete then you should post them as well. You have either `using namespace std;` or something else, that and which includes you have can seriously affect what you code is doing (not saying that it is the case here, but potentially it can turn something correct in something terribly wrong or vice versa) – 463035818_is_not_an_ai May 04 '20 at 14:18
  • 2
    Besides, C++ have some better [random number functionality](https://en.cppreference.com/w/cpp/numeric/random) than the old `rand` function. – Some programmer dude May 04 '20 at 14:19
  • @Someprogrammerdude I dont understand. I only called the srand function once in the code. – Coristat May 04 '20 at 14:20
  • @idclev463035818 okay i will next time. if you are wondering im using iostream, windows.h, ctime, string, cmath and cstdlib – Coristat May 04 '20 at 14:21
  • why only next time? You can still [edit](https://stackoverflow.com/posts/61594338/edit) your question – 463035818_is_not_an_ai May 04 '20 at 14:22
  • Okay I'm sorry, misread the code. Anyway, please don't call `srand` in some other function, unless it's documented as being called only once. Please do it early in the `main` function instead. – Some programmer dude May 04 '20 at 14:23
  • What do the `W` and `L` stand for in your `bGameW` and `bGameL`? – JohnFilleau May 04 '20 at 14:23
  • I think I made myself unclear. I was wondering if instead of using 2 string appends. I can make one string append with both fruitX and fruitY as parameters. – Coristat May 04 '20 at 14:23
  • @JohnFilleau W stands for width and L stands for Length – Coristat May 04 '20 at 14:23
  • 1
    You need a two-dimensional representation of the board. Right now, you only have two things; a "top-and-bottom" line and an "everything inbetween" line. – molbdnilo May 04 '20 at 14:23
  • I overlooked the `using namespace std;` but it is there. Don't confuse it as a way to not care about includes anymore. It isnt. In fact the presence of `using namespace std;` makes it extremely important what you include and what not. See also [Why is “using namespace std;” considered bad practice?](https://stackoverflow.com/questions/1452721/why-is-using-namespace-std-considered-bad-practice) – 463035818_is_not_an_ai May 04 '20 at 14:25
  • This code should often throw an out of range exception (about 75% of the time) since your insert position of each string is larger than the string's size. Is that happening to you? – JohnFilleau May 04 '20 at 14:28
  • @JohnFilleau Almost everytime I try running the code it gives me an exception now, I cant really run this code anymore. Thinking about starting all over. How would I fix this exception? – Coristat May 04 '20 at 14:30
  • What reference are you using for the standard library functions? I recommend cppreference.com (here's the page for `string::insert` https://en.cppreference.com/w/cpp/string/basic_string/insert) which explains that `pos` must be less than `size()`. Here each of your `string`s is of length 10 and 11 respectively, but you're generating numbers between 0 and 20. It's hard to learn C++. Even harder if you don't have a good resource. – JohnFilleau May 04 '20 at 14:31
  • @JohnFilleau OH yeah, I changed my board size to 10 but forget to change my random numbers to 0-10 aswell. My bad! – Coristat May 04 '20 at 14:33
  • The way I would do this is to have `Board` class that holds a `std::vector` member variable to remember board state. Since you're new and that may be too much to take in, you could have your board represented by a `std::vector` where each index in the `vector` is a row and each index in the composing `string`s is a column. – JohnFilleau May 04 '20 at 14:34
  • Instead of hard-coding numbers like you did (these are called "magic numbers" and are frowned up on because they make maintenance a nightmare especially when your code grows) you can use a defined constant integer to construct each string (call it `constexpr int BOARD_WIDTH = 10;` or something) so you only have to call it once, or you can use `bGameW.size()` to determine the maximum random value you should generate. That way it always matches the valid indices. – JohnFilleau May 04 '20 at 14:36
  • Thank you everyone. I realized my issue and Im changing up my code. Feel free to let me know for any more suggestions if you'd like :) – Coristat May 04 '20 at 14:38

1 Answers1

0

There's a whole host of things worth commenting on. Here goes:

  1. bGameW has no \n
  2. bGameW and bGameL are 10 and 11 characters long (both with be 11 after you add the other \n). Your RNG is generating numbers between 0 and 20... if you ever generate a number > 11 (and you will), Bad Things will happen (and probably have).
  3. Snake games like this let you eat multiple fruit, which is why people brought up the whole "don't call srand more than once" thing, as you'll be calling generate fruit every time someone eats the old one. OTOH, that mostly removes the "calling srand multiple times per second can return the same value" problem too.
  4. Rather than writing out those two lines, bGameW and bGameL, I recommend that you build a character array that holds your entire game display (NOT your game state, just the display of that state). You then clear it and redraw it every move. You'll need a game area, walls, something that tracks where your snake is, and the current fruit. You then 'render' all these things into your character-array-game-display. Don't forget to clear it every time you redraw it or you'll get all kinds of problems.
  5. Rather than redrawing everything, you could use something like the "curses" library to clear and redraw specific character locations on the screen. You'd then face problems 'clearing' and redrawing different spots.

As far as programming style goes, you will find that so called "magic numbers" (like 21 in your case) can lead to bugs. What people generally do instead is to define a const with the appropriate value, and then define things in terms of that const.

// constants are often named in ALL_UPPER_CASE_WITH_UNDERSCORES
// to help you recognize them.
const int PLAY_AREA_HEIGHT = 10;
const int PLAY_AREA_WIDTH = 10;
const char EMPTY_PLAY_SQUARE = '.';
// have you learned about 2d arrays yet?  Arrays at all?  
char playAreaDisplay[PLAY_AREA_HEIGHT][PLAY_AREA_WIDTH];

void wipePlayArea()
{
  for (int heightIdx = 0; heightIdx < PLAY_AREA_HEIGHT; ++heightIdx)
  {
    for (int widthIdx = 0; widthIdx < PLAY_AREA_WIDTH; ++widthIdx)
    {
      playAreaDisplay[heightIdx][widthIdx] = EMPTY_PLAY_SQUARE;
    }
  }
}
Mark Storer
  • 15,672
  • 3
  • 42
  • 80