program running out of memory?(crash)

[adam2016]

Hello guys I am making a kind of snake game where you hit a piece of fruit your score gets incremented but I still have to implement the snakes body

anyway I have added a new feature which is a time bonus when the score is in multiples of 10 you get 5 seconds to hit a bonus fruit but I used 2 seconds for debugging,anyway when I comment out the printTimer() function the code works fine but when I try to print the timer with the function the program crashes,

is it because it ran out of memory or is it something wrong with my logic,please bare with me guys my codes pretty long and just having fun :)

when I comment this out

if(score == 2 && !timeUp){
        printTimer();  // WHEN COMMENTED OUT CODE WORKS FINE
    }

works fine,

thanks

#include <iostream>
#include <stdlib.h>
#include <conio.h>
#include <ctime>

using namespace std;

enum eDir
{
    STOP = 0,
    UP,
    DOWN,
    LEFT,
    RIGHT
};

const int HEIGHT = 20;
const int WIDTH = 20;
int snakey,snakex,fruitX,fruitY,score,bonusX,bonusY;
bool gameOver;
eDir dir;
bool firstEntered;
bool timeUp;
time_t hour,minute,second;
struct tm* theTime;
time_t tim;

string printTimer(){

 cout << theTime->tm_hour << ":" << theTime->tm_min << ":" << theTime->tm_sec << endl;

}

void timer()
{
    time(&tim);
    theTime = localtime(&tim);

    if(!firstEntered)
    {
        hour = theTime->tm_hour;
        minute = theTime->tm_min;
        second = theTime->tm_sec;
        firstEntered = true;
    }
    system("cls");
    if(theTime->tm_min == minute)
    {
        if(theTime->tm_sec == second+5)
        {

            firstEntered = true;
            timeUp = true;
            return;
        }
        return;
    }
    else if(theTime->tm_min > minute)
    {
        int tempMinute = theTime->tm_min;
        int tempSecond = theTime->tm_sec;

        switch(theTime->tm_min)
        {

        case 56:
            tempMinute = 1;
            if(tempMinute = second+10){
                firstEntered = true;
                timeUp = true;
                return;
            }
            else{
                return;
            }
            break;
        case 57:
            tempMinute = 2;
            if(tempMinute = second+10){
                firstEntered = true;
                timeUp = true;
                return;
            }
            else{
                return;
            }
            break;
        case 58:
            tempMinute = 3;
            if(tempMinute = second+10){
                firstEntered = true;
                timeUp = true;
                return;
            }else{
              return;
            }
            break;
        case 59:
            tempMinute = 4;
            if(tempMinute = second+10){
                firstEntered = true;
                timeUp = true;
                return;
            }else{
                return;
            }
            break;
        }
    }
}

void bonus()
{
    if(!timeUp)
    {
        cout << "Time bonus" << endl;
        timer();
    }
}

void setUp()
{
    srand(time(NULL));
    snakex = WIDTH / 2;
    snakey = HEIGHT / 2;
    fruitX = rand() % HEIGHT;
    fruitY = rand() % WIDTH;
    bonusX = rand() % HEIGHT;
    bonusY = rand() % WIDTH;
    dir = STOP;
    gameOver = false;
    firstEntered = false;
    timeUp = false;
    score = 0;
}

void draw()
{
    system("cls");

    for(int i = 0; i <= WIDTH+1; i++)
    {
        cout << "#";
    }
    cout << endl;
    for(int y = 0; y < HEIGHT; y++)
    {
        for(int x = 0; x <= HEIGHT; x++)
        {

            if(x == 0)
                cout << "#";
            if(x == HEIGHT)
                cout << "#";
            else if(y == snakey && x == snakex)
            {
                cout << "O";
            }
            else if(y == fruitY &&  x == fruitX)
            {

                cout << "F";
            }
            else if(y == bonusY && x == bonusY)
            {

                cout << "B";
            }
            else
            {
                cout << " ";
            }
        }
        cout << endl;
    }
    for(int i = 0; i < WIDTH+2; i++)
    {
        cout << "#";
    }
    cout << endl;
    cout << "SCORE = " << score << endl;
    cout << endl;
    cout << endl;
    cout << endl;
    if(score == 2 && !timeUp){
        printTimer();  // WHEN COMMENTED OUT CODE WORKS FINE
    }
}

void input()
{
    while(_kbhit())
    {
        switch(_getch())
        {

        case 'w':
            dir = UP;
            break;
        case 'z':
            dir = DOWN;
            break;
        case 'a':
            dir = LEFT;
            break;
        case 's':
            dir = RIGHT;
            break;
        }
    }
}

void logic()
{

    switch(dir)
    {

    case UP:
        snakey--;
        break;
    case DOWN:
        snakey++;
        break;
    case LEFT:
        snakex--;
        break;
    case RIGHT:
        snakex++;
        break;
    }
    //bounds checking

    if(snakex < 0 || snakex > WIDTH || snakey < 0 || snakey > HEIGHT)
    {

        gameOver = true;
    }

    if(snakex == fruitX && snakey == fruitY)
    {

        fruitX = rand() % HEIGHT;
        fruitY = rand() % WIDTH;
        score++;
    }
    if(score == 2){
            
        bonus();
    }
}

int main()
{
    setUp();

    while(!gameOver)
    {
        draw();
        input();
        logic();
    }
}

**EDIT

note that when I replace the printTimer() function with

if(score == 2 && !timeUp){
        cout << theTime->tm_hour << ":" << theTime->tm_min << ":" << theTime->tm_sec << endl; 
    }

the code runs and the timer is displayed at the bottom but its very very choppy/laggy

also wondering how my printTimer() function is causing a crash

[Thomas1965]

Your printTimer is supposed to return a string but it doesn't.

[adam2016]

yeah I got rid of the printTimer function,

but is that why it crashed?

[woldaker]

As you mentioned, inlining the code from printTimer() to where you call it (you're only calling it from one place anyway) should fix your current runtime crash, but it is not really a thorough solution...

I didn't personally run your code, but instinct tells me that your struct tm *theTime is most likely being dereferenced before it has been initialized.

If you're coming from a C background, think of all global variables in C++ as having an implicit "static" prefix because that's what they're treated as. The compiler is initializing theTime to zero, which is the same thing as setting it to NULL.

What I see is essentially just C code with a few ostream << operators used. If you're writing in C++ then use the functionality in <chrono> rather than <ctime> (unless you specifically need something from ctime). Encapsulate global variables in an object or you'll have to let them be constrained to some local scope.

Bottom line, get rid of your globals, or at least initialize them as part of their declaration.

I must clarify that you can only initialize static variables once and once only per program, whether it's C or C++. The only reason why this should work here is that this program is not split into separate compilation units (i.e. everything's in one file).

[Thomas1965]

but is that why it crashed?

A missing return type is undefined behaviour, so anything can happen.

yeah I got rid of the printTimer function

Why go so far, just make it void should work.

I didn't personally run your code, but instinct tells me that your struct tm *theTime is most likely being dereferenced before it has been initialized.

Good point.

It's always good to check if a pointer is not null.
Try this.

void printTimer()
{
   assert(theTime != nullptr);
   cout << theTime->tm_hour << ":" << theTime->tm_min << ":" << theTime->tm_sec << endl;
}

[JLBorges]

> also wondering how my printTimer() function is causing a crash

Get into the habit of compiling C++ code with

a. specifically demanding standard conformance to the extent possible
(this is particularly important for GNU/GNU compatible compilers)

b. enabling (almost) all warnings
(-Weverything with clang++ and -Wall with Microsoft generate a lot of noise; so you may want to avoid these.)

For instance:

#include <string>

std::string printTimer() {

    // ...
    // no return statement 
    
    //       GNU: *** warning: no return statement in function returning non-void (ok: we are warned)
    //      LLVM: *** warning: control reaches end of non-void function (ok: we are warned)
    // Microsoft: *** error: 'printTimer': must return a value (excellent: generate an error if the code is clearly erroneous) 
}

int main() {
    
}

http://coliru.stacked-crooked.com/a/2276df10bb9c7612
http://rextester.com/LMZG41720

[adam2016]

thanks guys very good points,I edited and tidied it up a little it works fine albeit quite laggy and buggy but its working so its a start,

anyway I noticed every time I call the initTimer() function I'm guessing a new block of memory is assigned to theTime?

so I try to save myself some memory and delete that block of memory before it is assigned to a new block of memory but when I do this the game crashes,why is this?

also JL that is interesting it makes sense that the compiler would throw an error if a return type was not made I wonder why the compiler I use mingW doesn't implement this as an error as opposed to a warning

I know also it is not a good idea to have so many globals well actually globals at all but I will tidy this up soon

#include <iostream>
#include <stdlib.h>
#include <conio.h>
#include <ctime>
#include <fstream>

using namespace std;

enum eDir
{
    STOP = 0,
    UP,
    DOWN,
    LEFT,
    RIGHT
};

const int HEIGHT = 20;
const int WIDTH = 20;
int snakey,snakex,fruitX,fruitY,score,bonusX,bonusY;
bool gameOver;
eDir dir;
bool firstEntered;
bool timeUp;
time_t hour,minute,second;
struct tm* theTime;
time_t tim;
time_t tempMinute;
time_t tempSecond;
ofstream out;
ifstream in;
int highScore;

void writeToFile()
{

    out.open("snakeHighScores.txt",ofstream::app);
    if(!out.is_open())
    {
        return;
    }
    out << score << endl;
    out.close();
}

void readFromFile()
{

    in.open("snakeHighScores.txt");

    if(!in.is_open())
    {
        return;
    }
    int temp;
    highScore = 0;

    while(in)
    {

        in >> temp;

        if(temp > highScore)
        {

            highScore = temp;
        }
    }
    in.close();
}

void initTimer()
{

    if(theTime != NULL)
    {
        delete theTime; // source of crash
    }
    time(&tim);
    theTime = localtime(&tim);
}

void timer()
{
    initTimer();

    if(!firstEntered)
    {
        hour = theTime->tm_hour;
        minute = theTime->tm_min;
        second = theTime->tm_sec;
        firstEntered = true;
    }
    if(theTime->tm_min == minute) // check if minutes are equal if they are we can compare seconds easily
        // if not we need to do some extra tweaking
    {
        if(theTime->tm_sec == second+5)
        {

            firstEntered = true;
            timeUp = true;
            return;
        }
        return;
    }
    else if(theTime->tm_min > minute)
    {
        switch(second)
        {

        case 56:
            tempSecond = theTime->tm_sec - 5;
            if(tempSecond == -5)
            {
                firstEntered = true;
                timeUp = true;
                return;
            }
            else
            {
                return;
            }
            break;
        case 57:
            tempSecond = theTime->tm_sec - 5;
            if(tempSecond == -4)
            {
                firstEntered = true;
                timeUp = true;
                return;
            }
            else
            {
                return;
            }
            break;
        case 58:
            tempSecond = theTime->tm_sec - 5;
            if(tempSecond == -3)
            {
                firstEntered = true;
                timeUp = true;
                return;
            }
            else
            {
                return;
            }
            break;
        case 59:
            tempSecond = theTime->tm_sec - 5;
            if(tempSecond == -2)
            {
                firstEntered = true;
                timeUp = true;
                return;
            }
            else
            {
                return;
            }
            break;
        }
    }
}

void bonus()
{
    if(!timeUp)
    {
        cout << "Time bonus" << endl;
        timer();
    }
}

void setUp()
{
    srand(time(NULL));
    snakex = WIDTH / 2;
    snakey = HEIGHT / 2;
    fruitX = rand() % HEIGHT;
    fruitY = rand() % WIDTH;
    bonusX = rand() % HEIGHT;
    bonusY = rand() % WIDTH;
    dir = STOP;
    gameOver = false;
    firstEntered = false;
    timeUp = false;
    score = 0;
}

void draw()
{
    system("cls");

    for(int i = 0; i <= WIDTH+1; i++)
    {
        cout << "#";
    }
    cout << endl;
    for(int y = 0; y < HEIGHT; y++)
    {
        for(int x = 0; x <= HEIGHT; x++)
        {
            if(x == 0)
                cout << "#";
            if(x == HEIGHT)
                cout << "#";
            else if(y == snakey && x == snakex)
            {
                cout << "O";
            }
            else if(y == fruitY &&  x == fruitX)
            {
                cout << "F";
            }
            else if((y == bonusY && x == bonusX) && (score == 2 && !timeUp))
            {
                cout << "B";
            }
            else
            {
                cout << " ";
            }
        }
        cout << endl;
    }
    for(int i = 0; i < WIDTH+2; i++)
    {
        cout << "#";
    }
    cout << endl;
    cout << "SCORE = " << score << endl;
    cout << endl;
    if(score == 2 && !timeUp)
    {
        cout << theTime->tm_hour << ":" << theTime->tm_min << ":" << theTime->tm_sec << endl;
    }
    if(theTime != NULL)
    {
        if(theTime->tm_sec >= 56 && theTime->tm_sec <= 59 )
        {
            cout << "now ********************" << endl; // used for debugging
        }
    }
    cout << endl;
    cout << tempMinute << ": " << tempSecond << endl; // used for debugging
}

void input()
{
    while(_kbhit())
    {
        switch(_getch())
        {

        case 'w':
            dir = UP;
            break;
        case 'z':
            dir = DOWN;
            break;
        case 'a':
            dir = LEFT;
            break;
        case 's':
            dir = RIGHT;
            break;
        }
    }
}

void logic()
{

    switch(dir)
    {

    case UP:
        snakey--;
        break;
    case DOWN:
        snakey++;
        break;
    case LEFT:
        snakex--;
        break;
    case RIGHT:
        snakex++;
        break;
    }
    //bounds checking

    if(snakex < 0 || snakex > WIDTH || snakey < 0 || snakey > HEIGHT)
    {
        gameOver = true;
    }

    if(snakex == fruitX && snakey == fruitY)
    {
        fruitX = rand() % HEIGHT;
        fruitY = rand() % WIDTH;
        score++;
    }
    if(score == 2)
    {

        bonus();
    }
    if(snakex == bonusX && snakey == bonusY)
    {
        score+=5;
        bonusX = rand() % HEIGHT;
        bonusY = rand() % WIDTH;
    }
}

int main()
{
    setUp();

    while(!gameOver)
    {
        initTimer();
        draw();
        input();
        logic();
    }
    if(gameOver)
    {
        writeToFile();
        readFromFile();
        cout << "GAME OVER!!!!" << endl;
        cout << "High Score : " << highScore << endl;
        cout << "your Score : " << score << endl;
    }
}

[Cubbi]

adam2016 wrote:

I noticed every time I call the initTimer() function I'm guessing a new block of memory is assigned to theTime?

No, that's not what localtime does. It returns, to quote http://en.cppreference.com/w/cpp/chrono/c/localtime , a pointer to a static internal std::tm object on success, or null pointer otherwise. That pointer is what you're assigning to theTime

when I do this the game crashes,why is this?
delete theTime; // source of crash

You cannot delete what you didn't new

[Thomas1965]

You are not supposed to delete the pointer returned from localtime. It probably points to some static variable. NEVER EVER delete a pointer until
a) you allocated it
or
b) the documentation of the function tells you to do it.

BTW Why don't you learn about classes? This type of game easily leads to an object-oriented design.

[adam2016]

thanks Cubbi

I thought localtime() returned a object because we declare tm* theTime as a pointer so I thought we would have implemented it the same way as if we called the new keyword

and thanks Thomas very good point,I actually know a bit about OOP coming from a Java background and use it sometimes but for small projects I prefer to use a more functional style(probably not good practice)

[JLBorges]

> I wonder why the compiler I use mingW doesn't implement this as an error as opposed to a warning

Well, the standard does not require that undefined behaviour must be diagnosed.

With the GNU compiler, this 'program' (the return type is int) works as expected by the programmer who wrote it.

#include <iostream>

int foo() { std::cout << "hello world!\n" ; }

int main()
{
    foo() ;
}

http://coliru.stacked-crooked.com/a/54987ac583d79c8f

Though with the LLVM compiler, it does crash at run-time.

I suppose the GNU crowd thought that it would not be politically expedient to generate an error for undefined behaviour such as this, and immediately make a massive chunk of open source code un-compilable.

[adam2016]

I suppose the GNU crowd thought that it would not be politically expedient to generate an error for undefined behaviour such as this, and immediately make a massive chunk of open source code un-compilable.

very good point