Sign in to follow this  
mihaijulien

C++ 2048 game - movement function issue

Recommended Posts

Hello,
 
I am trying to make a c++ implementation of the following game: http://2048game.com/ 
 
I have the following movement issue which doesn't really work as expected:

void game::goUp()
{

	for (int j = 0; j < 4; j++) //traverse the columns
	{
		for (int i = 3; i > 0; i--) //traverse the rows (start going through the array from the bottom)
		{
				for (int k = 0; k < 4; k++)  //loop purpose: check the same column several times to see if all the swaps were done
				{
					if (n[i - 1][j] == 0)
					{
						n[i - 1][j] = n[i][j];
						n[i][j] = 0;
					}
					else if (n[i - 1][j] == n[i][j])
					{
						n[i - 1][j] *= 2;
						n[i][j] = 0;
					}
				}	
		}
	}
}

Thanks!

Share this post


Link to post
Share on other sites

If you use "column" instead of "j", and "row" instead of "i", things are much more readable.

 

I am guessing your merge is failing, there is no upper limit on the number of merges while moving.

 

Rather than repeatedly moving things 1 position (and thus not being able to track what you last merged), why not move each stone individually all the way?

That way you know what the previous stone did, and you can skip (or not skip) merging of the next stone.

Share this post


Link to post
Share on other sites

Hello,

 

Ok, so the issue is the following: when moving a number up, it goes up to the first row but if there are 2 numbers on the same column, they won't add up. Also, the same algorithm is used for the goDown() function (with the proper changes), but it won't work at all.

 

The down function would be this one:

void game::goDown()
{
	for (int j = 0; j < 4; j++)
	{
		for (int i = 0; i < 3; i++)  //traverse the rows from the top
		{
			for (int k = 0; k < 4; k++) // loop to check that all swaps were done
			{
				if (n[i][j] == 0)
				{
					n[i + 1][j] = n[i][j];
					n[i][j] = 0;
				}
				else if (n[i + 1][j] == n[i][j])
				{
					n[i + 1][j] *= 2;
					n[i][j] = 0;
				}
			}
		}
	}
}

This is the array I'm using to move the numbers:

game::game()
{
	score = 0;
	cout << "Score: " << score << "\n\n";

	int x = rand() % 3;
	int y = rand() % 3;

	for (int j = 0; j < 4; j++)
	{
		cout << "+------+------+------+------+\n";
		for (int i = 0; i < 4; i++)
		{
			if (i == x && j == y)
			{
				n[x][y] = 2;
				cout << "|    " << n[x][y] << " ";
			} else {
				n[i][j] = 0;
				cout << "|    " << n[i][j] << " ";
			}
		}
		cout << "|\n";
	}
	cout << "+------+------+------+------+\n\n";
	cout << "(W)Up (S)Down (A)Left (D)Right\n";
}

What is wrong in the code? Also, I'm not sure if I understand what you're saying by moving each stone individually all the way, Alberth. How would the code be different?

 

Thanks.

Edited by mihaijulien

Share this post


Link to post
Share on other sites

Ah, when trying to debug it, I found the problem (or at least, a problem).

 

In the move code, what is the "k" loop doing?

Inside the loop only i and j are used, and they are not changed (which is recommended btw, don't mess with loop variables if you want to keep your sanity).

So you run the same test and update 4 times. Why?

 

Maybe you should scale down first. Try a single row in a simple array   int stones[4]   first, and only move towards 0.

 

Figuring out the algorithm by hand is also a good way to understand if it works. Write the algorithm down on paper.

"Play" the algorithm by hand. Take another piece of paper, write a start situation at the top, and then mechanically follow what the algorithm says you should do.

Each time the "stones" array changes, write a new line with the new situation under the previous line. That way you can really see how it works.

 

It may seem like a stupid way to write an algorithm, but it really works, as you follow it every single step, so you can immediately see when it makes a wrong decision, and fix it by changing the algorithm, and trying again.

 

Computers are very fast, which is great when things work. However, they thus also make a big mess very fast. Untangling what in the zillion steps it did was not correct is a lot of work. By doing everything manually, you can check each step immediately, and immediately know what it does wrong.

 

 

A more computerized way to do this is by using a debugger, and step through your code, verifying it makes the right checks and makes the right changes. You should probably try both, and decide what works best for you.

 

Also, I'm not sure if I understand what you're saying by moving each stone individually all the way, Alberth.

Right now, you walk over each square, and locally decide for that square what the answer should be (by looking around it). That won't work in its current form. If you have a row "2 2 2 2", then the result should be "4 4", and not "8", right?. I don't see any code that prevents a "4" getting merged with another "4". In other words, you have to distinguish between values that have been merged before (like "4" in the example), and values that have not yet merged (like "2"). To me, that says your strategy of visiting each square and locally decide what to do may not the right solution.

 

Maybe you should instead decide what the result of a destination should be completely before you go to the next destination.

 

In the stones[] array, what would mean

first decide the new value of "stones[0]" completely, possibly updating stones[1], stones[2], and/or stones[3].

Next, decide the new value of stones[1], possibly updating stones[2], and/or stones[3].

Next, decide the new value of stones[2], possibly updating stones[3].

And then you're done (I think), no need to decide a new value for stones[3].

 

By doing each position completely, you avoid problems like double merging.

 

 

Share this post


Link to post
Share on other sites
Skipping over the empty entries of an array is a common algorithm. There is a neat way to do it using a loop with two indices, where you read using one index and write using the other one.

int write_i = 0;
for (int read_i = 0; read_i < n; ++read_i) {
  if (array[read_i] != Empty)
    array[write_i++] = array[read_i];
}

for (; write_i < n; ++write_i)
  array[write_i] = Empty;


A small modification of that code will do what you need for 2048. Also, instead of writing the loop four times, try to parametrize it so the same piece of code can do all four directions.

If you have trouble with it, I can post some code. But give it a try yourself first.

Share this post


Link to post
Share on other sites
Well, I had a bit of time, so I wrote it.

#include <iostream>

void move_row(int *array, int step) {
  int write_i = 0;
  int previous = 0;

  for (int read_i = 0; read_i < 4; ++read_i) {
    int entry = array[step*read_i];
    if (entry == 0)
      continue;
    if (entry == previous) {
      array[step*(write_i - 1)] *= 2;
      previous = 0;
    }
    else {
      array[step*(write_i++)] = entry;
      previous = entry;
    }
  }
  for (; write_i < 4; ++write_i)
    array[step*write_i] = 0;
}

void print_row(int *a) {
  for (int i = 0; i < 4; ++i)
    std::cout << a[i] << ' ';
  std::cout << '\n';
}

int main() {
  int a[4] = {4, 1, 1, 0};

  print_row(a);
  move_row(&a[3], -1);
  print_row(a);
}

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this