1
\$\begingroup\$

I've recently started game development in C++ and decided to make an ASCII based snake game. To hold the values of the snake tail I decided to use a linked list due to its flexibility when it comes to manipulating it.

Each node holds the x and y coordinates of a segment of the snake's tail.

The linked list is successful in holding data, creating a new node whenever the snake eats the fruit, but I do not know how to implement this data into my draw function.

I am aware of the issue being caused by a logical error in my program due to the while loop breaking my if else sequence. I just don't know how to fix it.

Here is my current draw function:

void draw() {
    system("cls");
    for (int i = 0; i < width+2; i++)
        cout << '#';
    cout << endl;

    for (int i = 0; i < height; i++) {
        for (int j = 0; j < width; j++) {
            if (j == 0)
                cout << '#';
            if (i == posY && j == posX)
                cout << 'O';

            Node *current = head->next;

            while (current != nullptr) {
                if (i == current->y && j == current->x)
                    cout << 'o';
                current = current->next;
            }
            if (i == fruit.y && j == fruit.x)
                cout << 'F';
            else
                cout << ' ';
            if (j == width-1)
                cout << '#';
        }
        if (i != height-1)
            cout << endl;
    }
    cout << endl;

    for (int i = 0; i < width+2; i++)
        cout << '#';
    cout << endl;

    cout << "Score: " << score << " || Tail len: " << tailLen << endl;

}

As you can see I've tried to retrieve the values from the nodes via a while loop. Unfortunately by doing so extra spaces are added and the frames are extremely glitchy. I've been racking my brain over this issue for ages. Any suggestions would be appreciated, thanks.

Screenshot: Game screenshot

\$\endgroup\$
8
  • \$\begingroup\$ So just to clarify: you want to know how to spawn a new node at the end of the snake when the snake eats a fruit? And what's this about the while loop? You've determined that using a while loop causes additional space characters appearing? \$\endgroup\$ – Tyyppi_77 Sep 30 '16 at 8:12
  • 1
    \$\begingroup\$ Could you manage to take a screenshot of the "extra spaces" ? About the glitchy frames, you should probably flush the output only once, at the end of the frame, instead of every line. \$\endgroup\$ – Quentin Sep 30 '16 at 8:13
  • \$\begingroup\$ @Tyyppi_77 As I've stated, I'm able to create nodes and append them. I just don't know how to efficiently display the data through my draw function. \$\endgroup\$ – James Balajan Sep 30 '16 at 8:14
  • 1
    \$\begingroup\$ There's a chance, that the console you are using doesn't use monospace characters, thus the width of a hashtag and a space is not the same \$\endgroup\$ – Bálint Sep 30 '16 at 8:15
  • \$\begingroup\$ So you are asking the extremely broad question "how to improve my draw function?"? \$\endgroup\$ – Tyyppi_77 Sep 30 '16 at 8:15
2
\$\begingroup\$

Let's go trough your code, especially this part:

        while (current != nullptr) {
            if (i == current->y && j == current->x)
                cout << 'o';
            current = current->next;
        }
        if (i == fruit.y && j == fruit.x)
            cout << 'F';
        else
            cout << ' ';
        if (j == width-1)
            cout << '#';

There are couple of logical errors here. Let's take a scenario, where you need to draw a snake tile:

You go trough each nodes of the snake, and you find one at the current position, so you draw an o. Next you check if there's a fruit at that position. If yes, then it'll draw an F there, if not, then a space. See, no matter what, you'll always draw at least 2 characters where a snake is, an o and a space or an o and an F.

The same happens if you're at the edge of the map. You draw a space for an empty tile and a hashtag.

To fix this issue, create a boolean flag wich marks if you found a snake at that tile and only draws a space if you didn't:

       bool found = false;
       while (current != nullptr) {
            if (i == current->y && j == current->x) {
                cout << 'o';
                found = true;
            }
            current = current->next;
        }
        if (!found) {
            if (i == fruit.y && j == fruit.x)
                cout << 'F';
            else
                cout << ' ';
        }
        if (j == width-1)
            cout << '#';
\$\endgroup\$
5
  • \$\begingroup\$ Thanks for the quick reply, but you are not answering my question. I am aware of what's causing the issue, I'm just not aware of how to fix it :( \$\endgroup\$ – James Balajan Sep 30 '16 at 8:22
  • 1
    \$\begingroup\$ That is not at all obvious from your question. You definitely should have mentioned clearly that "I know that the issue is caused by me rendering multiple characters at a single position". \$\endgroup\$ – Tyyppi_77 Sep 30 '16 at 8:23
  • \$\begingroup\$ @Tyypii_77 I apologise profusely for that, I'm just a little rushed right now, about to go on a plane flight. \$\endgroup\$ – James Balajan Sep 30 '16 at 8:24
  • \$\begingroup\$ It's all right, I was mainly pointing out that in the future you will know to mention any work you have already done to conquer the issue :) \$\endgroup\$ – Tyyppi_77 Sep 30 '16 at 8:26
  • \$\begingroup\$ @JamesBalajan There you go \$\endgroup\$ – Bálint Sep 30 '16 at 8:28
1
\$\begingroup\$

Your syntax is getting muddled because you're doing too many things at once: traversing the game space, traversing your snake (and fruits, walls, etc), and outputting. As you have seen, you don't have enough information at the innermost level to decide whether you should output one more character or not.

That can be solved by adding more logic as Bàlint's answer did, but it can also be done by splitting your code into more contained consituents. In your case, you must at one point decide on a single character to output in the current tile. That's a prefect use case for a lambda!

Note: the code is untested

void draw() {
    // Bring in some standard identifiers
    using std::cout;
    using std::endl;
    using std::system;

    system("cls");

    // Draw the top line of the playing field
    for (int i = 0; i < width+2; i++)
        cout << '#';
    cout << '\n';

    // Find out which character we should draw at a given position
    auto charAt = [&](int x, int y) -> char {

        // Left/right wall
        if(x == 0 || x == width - 1)
            return '#';

        // Fruit
        if(x == fruit.x && y == fruit.y)
            return 'F';

        // Snake head
        if(x == posX && y == posY)
            return 'O';

        // Snake body
        for(Node const *cur = head->next; cur; cur = cur->next)
            if(x == cur->x && y == cur->y)
                return 'o';

        // Default: empty space
        return ' ';
    };

    for (int i = 0; i < height; i++) {
        for (int j = 0; j < width; j++)
            cout << charAt(j, i);

        // `if` removed along with the redundant line break below
        cout << '\n';
    }
    // cout << endl;

    // Draw the bottom line of the playing field
    for (int i = 0; i < width+2; i++)
        cout << '#';
    cout << '\n';

    cout << "Score: " << score << " || Tail len: " << tailLen << endl;
}

I have replaced the std::endl calls with '\n': std::endl forces a flush, which probably explain a good part of your frame gliching issues. You should use std::endl or std::flush only when you really want to halt the program, flush everything to the console, and then move on.

It looks like you're using using namespace std; at global scope as well. If so, you should stop doing it: it breaks the purpose of the std namespace completely. If you wish to omit std:: for a portion of code, put the using namespace directive in the tightest scope possible, or better yet replace it with precise using-declarations.

There's still things that can be improved in this code:

  • The code for drawing the top and bottom lines of the playing field are perfectly identical. Even if that doesn't hinder logic as much as the central issue of the question, that is a good candidate for refactoring inside a reused function.

  • The loop that traverses the snake body will probably be useful elsewhere, such as when you're checking whether the snake bit its tail. Thus, it's a good candidate to be factored out into a function on its own.

That's all I've got! Have fun with C++ :)

\$\endgroup\$
7
  • \$\begingroup\$ Lambda feels a little miss-used here to me. What's wrong with good old functions? Otherwise great points about flushing and code readability! \$\endgroup\$ – Tyyppi_77 Sep 30 '16 at 9:08
  • \$\begingroup\$ @Tyyppi_77 well, the lambda has no use outside the draw() function, and local functions are not standard C++. Moreover (and even if it is not used here), lambdas can capture the context of the function they're in. So all in all, they're pretty much the perfect tool to factor out code locally :) \$\endgroup\$ – Quentin Sep 30 '16 at 9:21
  • \$\begingroup\$ Sure, but then the one day you also for debug purposes need to output the character of specific position into a log file, a function char getCharAt(int x, int y); will turn out to be a much more useful idea. I really don't see the downside of using a function here. \$\endgroup\$ – Tyyppi_77 Sep 30 '16 at 9:35
  • \$\begingroup\$ @Tyyppi_77 you'll still be able to then take the lambda and move it out into the global scope if you really need to. The usual downsides of functions don't seem to (we don't have the full code) apply here, but what upside do you see to using one at the current point ? \$\endgroup\$ – Quentin Sep 30 '16 at 9:41
  • 1
    \$\begingroup\$ Let us continue this discussion in chat. \$\endgroup\$ – Quentin Sep 30 '16 at 9:51

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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