patternjavaMinor
2048 game clone in Java
Viewed 0 times
javagameclone2048
Problem
I made a 2048 game clone in Java for training, and I am trying to make it more efficient.
The problem I found is the
The game works so far, but, before I proceed further, I am looking for ideas on how to only repaint the rows that get affected. For example, if the player presses ← and only the bottom two rows get affected and the top ones don't change at all, I only want to redraw the bottom rows for efficiency. I had this idea of having an array of 4
```
public class Game_Panel extends JPanel implements KeyListener
{
//instance variables
public TILE panel[][];
public byte current_tiles;
public boolean achieved_goal;
public static final int default_start_A = 2;
public static final int default_start_B = 4;
public static final int HW = 489;
public static final int seperation_length = 4;
public static final int block_width = 119;
public static final int block_center = 119>>1;
public static final int RANDOM = 101;
public static final byte ROWS_COLS = 4;
public static final byte real_end = ROWS_COLS-1;
public static final byte fake_end = 0;
public static final byte left_increment = 1;
public static final byte right_increment = -1;
//keyboard ascii numbers
public static final byte LEF
The problem I found is the
paintComponent() method always resets all the graphics before doing anything, so I am forced to repaint all 4 rows of the game every time I want to repaint. Obviously I didn't want to override paint() and/or the update() method because I don't know how to correctly override them and it's not a good idea anyway.The game works so far, but, before I proceed further, I am looking for ideas on how to only repaint the rows that get affected. For example, if the player presses ← and only the bottom two rows get affected and the top ones don't change at all, I only want to redraw the bottom rows for efficiency. I had this idea of having an array of 4
int elements where each element represents a row. If the is_moved flag becomes true I set that row in that array to equal 0. If that row didn't change, the index in the array for that row will be -1. But still, because of paintComponent() resetting everything beforehand, I have to every time I want to repaint(). I have to redraw all rows, even if some of them don't need to be repainted.```
public class Game_Panel extends JPanel implements KeyListener
{
//instance variables
public TILE panel[][];
public byte current_tiles;
public boolean achieved_goal;
public static final int default_start_A = 2;
public static final int default_start_B = 4;
public static final int HW = 489;
public static final int seperation_length = 4;
public static final int block_width = 119;
public static final int block_center = 119>>1;
public static final int RANDOM = 101;
public static final byte ROWS_COLS = 4;
public static final byte real_end = ROWS_COLS-1;
public static final byte fake_end = 0;
public static final byte left_increment = 1;
public static final byte right_increment = -1;
//keyboard ascii numbers
public static final byte LEF
Solution
Since Simon mentioned that re-writing the output is off-topic here (plus that's not really my area of expertise), I'll just do a general code review.
And FWIW: For a simple project like this, you don't need to do such an optimization.
Don't put everything into one big class. Use an MVC pattern you should have (at least):
Any time you find yourself numbering/lettering variables, or creating any other similar sequence of variables, you are doing something wrong. These should be arrays or lists of some kind.
All constants (
It's completely unnecessary to use bit-shifting for multiplication/division by two here.
I don't get the meaning of these to variable names.
What the? It what way would that be a "waste"? One of the top principles of good programming is DRY (Don't Repeat Yourself). There is absolutely no reason for this.
Don't repeatedly create a
(That's all for now. There are still much more problems. Maybe I'll come back later.)
And FWIW: For a simple project like this, you don't need to do such an optimization.
public class Game_Panel extends JPanel implements KeyListenerDon't put everything into one big class. Use an MVC pattern you should have (at least):
- a class for the game logic completely independent from IO and Swing (Model)
- an extension of JPanel for the output (View)
- a class which basically represents the window/application and which handles raw input and lets the first two classes communicate (Controller with a bit of View)
- possibly a class just consisting of the main function
public static final int default_start_A = 2;
public static final int default_start_B = 4;
public static final Color TWO = new Color(238,228,218);
public static final Color FOUR = new Color(237,224,200);
public static final Color EIGHT = new Color(242,177,121);Any time you find yourself numbering/lettering variables, or creating any other similar sequence of variables, you are doing something wrong. These should be arrays or lists of some kind.
All constants (
static final variables) should have names consisting only of capital letters.public static final int block_center = 119>>1;It's completely unnecessary to use bit-shifting for multiplication/division by two here.
public static final byte real_end = ROWS_COLS-1;
public static final byte fake_end = 0;I don't get the meaning of these to variable names.
//same as generate method, but thought it'd be a waste
//to call it when we're initializing.What the? It what way would that be a "waste"? One of the top principles of good programming is DRY (Don't Repeat Yourself). There is absolutely no reason for this.
Random row_col = new Random();Don't repeatedly create a
Random object each time you need a random number. Create a single one at the start of the program and use that.(That's all for now. There are still much more problems. Maybe I'll come back later.)
Code Snippets
public class Game_Panel extends JPanel implements KeyListenerpublic static final int default_start_A = 2;
public static final int default_start_B = 4;
public static final Color TWO = new Color(238,228,218);
public static final Color FOUR = new Color(237,224,200);
public static final Color EIGHT = new Color(242,177,121);public static final int block_center = 119>>1;public static final byte real_end = ROWS_COLS-1;
public static final byte fake_end = 0;//same as generate method, but thought it'd be a waste
//to call it when we're initializing.Context
StackExchange Code Review Q#51012, answer score: 7
Revisions (0)
No revisions yet.