2
\$\begingroup\$

I wrote this 2048 game using Python 3 and pygame a few weeks ago. Since I'm getting into CS this summer, I would like to get some review on how I can improve my code in generel. Any tips and improvements are welcome, and highly appreciated.

The code is found below:

import pygame
import random

class Main:
    def __init__(self):
        # SETUP pygame environment
        pygame.init()
        self.screen = pygame.display.set_mode((500,500))
        pygame.display.set_caption('2048')
        self.clock = pygame.time.Clock()
        # Create the Board
        self.board = Board()
        #self.board.add_tile()
        self.run()

    def paint(self):
        # Paint the background
        self.screen.fill((240,240,206))
        # Paint the board object to screen
        self.board.paint(self.screen)
        pygame.display.update()

    # Handling pygame.QUIT event, and
    # KEYDOWNS. Keydowns is parsed to board.move function
    def eventhandler(self):
        for event in pygame.event.get():
            if event.type == pygame.QUIT:
                self.running = False
                pygame.quit()
            if event.type == pygame.KEYDOWN:
                if event.key == pygame.K_LEFT:
                    self.board.move("LEFT")
                if event.key == pygame.K_RIGHT:
                    self.board.move("RIGTH")
                if event.key == pygame.K_UP:
                    self.board.move("UP")
                if event.key == pygame.K_DOWN:
                    self.board.move("DOWN")

    # Update function.
    def update(self):
        self.board.update_tiles()

    # Main loop. 60 FPS
    def run(self):

        self.running = True
        while self.running:
            self.clock.tick(60)
            self.eventhandler()
            self.update()
            self.paint()


class Tile:
    def __init__(self,x,y,stage):
        # SETUP tiles x,y and stage. Stage is the number it represents, 2,4,8 etc.
        self.x = x
        self.y = y
        self.stage = stage
        self.colorlist = [(245,240,255),(237,224,200),(242,177,121),(245,149,99),(246,124,95),(246,94,59),(237,207,113),(237,204,97),(237,200,80),(237,197,63),(238,205,94)]

    # Move the tile to new x,y coordinates. Returns False if it moves into a wall.
    def move_tile(self,x=0,y=0):
        self.x += x
        self.y += y
        if self.x<0 or self.x > 3 or self.y < 0 or self.y > 3:
            self.x -= x
            self.y -=y
            return False
        return True

    # Merge two tiles.
    def merge(self,Tile): 
        if Tile.stage == self.stage:
            self.increasestage()
            return True
        else:
            return False

    def increasestage(self):
        self.stage += 1

    # Draw the tile to Board.
    def draw(self,screen,x,y,font):
        pygame.draw.rect(screen,self.colorlist[self.stage-1],(x,y,87,87))

        # draw the numbers on tiles:

        if self.stage <= 2:
            color = (120,110,101)
        else:
            color = (250,248,239)

        text = font.render(str(2**self.stage),2,color)
        screen.blit(text,(x+(87/2 - text.get_width()/2), y + (87/2 -text.get_height()/2)))


class Board:
    def __init__(self):
        ## self.tiles keep track of the tiles GUI positions.
        self.tiles = [[0,0,0,0] for i in range(4)]
        self.board = pygame.Rect(50,50,400,400)
        self.color = (186,173,160)
        # tilearray stores the tiles as a list. When self.update_tiles is called
        # the tiles in tile_array gets updated in self.tiles (the tiles GUI position)
        self.tilearray = []
        self.add_tile()
        self.add_tile()
        self.font = pygame.font.SysFont('comicsans',61)


    #Draw the board background to screen.
    def paint(self,screen):
        pygame.draw.rect(screen,self.color,self.board)
        self.drawtiles(screen)

    # Draw tiles to screen. If no tile, draw empty square.
    def drawtiles(self,screen):
        for i,array in enumerate(self.tiles):
            for j,tile in enumerate(array):
                if tile == 0:
                    pygame.draw.rect(screen,(204,193,180),(60+i*87+10*i,60+j*87+10*j,87,87))
                else:
                    tile.draw(screen,60+i*87+10*i,60+j*87+10*j,self.font)

    # Returns an arraylist with positions in self.tiles which are empty
    def get_empty_spaces(self):
        empty = []
        for i,array in enumerate(self.tiles):
            for j,tile in enumerate(array):
                if tile==0:
                    empty.append([i,j])

        return empty

    # Add a new tile to the game. Coordinates chosen at random.
    def add_tile(self):
        empty = self.get_empty_spaces()
        chosen = random.choice(empty)

        if random.randrange(1,100) <10:
            stage = 2
        else:
            stage = 1

        t = Tile(chosen[0],chosen[1],stage)

        self.tilearray.append(t)
    # Move all tiles on the board.
    def move(self,key):

        stepstaken = 0

        if key=="LEFT":
            for i, array in enumerate(self.tiles):
                for j, _ in enumerate(array):
                    tile = self.tiles[j][i]
                    if tile!=0:
                        stepstaken += self.move_single_tile(tile,-1,0)
                    self.update_tiles()

        if key =="RIGTH":
            for i,array in enumerate(self.tiles):
                for j,_ in enumerate(array):
                    tile = self.tiles[3-j][3-i]
                    if tile!= 0:
                        stepstaken += self.move_single_tile(tile,1,0)
                    self.update_tiles()

        if key == "UP":
            for i,array in enumerate(self.tiles):
                for j,_ in enumerate(array):
                    tile = self.tiles[i][j]
                    if tile!=0:
                        stepstaken += self.move_single_tile(tile,0,-1)
                    self.update_tiles()

        if key == "DOWN":
            for i, array in enumerate(self.tiles):
                for j,_ in enumerate(array):
                    tile = self.tiles[3-i][3-j]
                    if tile!=0:
                        stepstaken += self.move_single_tile(tile,0,1)
                    self.update_tiles()
        if stepstaken>0:
            self.add_tile()


    # Tiles are stored in self.tilearray. When updating, the tiles from self.tilearray is 
    # stored in the 2d array.

    def move_single_tile(self,tile,vx=0,vy=0):
        steps = 0
        for i in range(0,3):
            if self.position_is_inside_grid(tile.x+vx,tile.y+vy) and self.tile_is_empty(tile.x+vx,tile.y+vy):
                tile.move_tile(vx,vy)
                steps+=1
            else:
                if self.position_is_inside_grid(tile.x+vx,tile.y+vy) and self.tiles[tile.x+vx][tile.y+vy].merge(tile):
                    self.tilearray.remove(tile)
                    steps += 1

        return steps 

    def position_is_inside_grid(self,x,y):
        if x>-1 and x<4 and y>-1 and y<4:
            return True
        else:
            return False

    def tile_is_empty(self,x,y):
        if self.tiles[x][y] == 0:
            return True
        else:
            return False

    def update_tiles(self):
        self.tiles = [[0,0,0,0] for i in range(4)]
        for tile in self.tilearray:
            self.tiles[tile.x][tile.y] = tile

m = Main()

# TO DO
# Refactor move() to loop functions
#
#
#

\$\endgroup\$
3
\$\begingroup\$

The code looks good, but the

if key=="LEFT":
        for i, array in enumerate(self.tiles):
            for j, _ in enumerate(array):
                tile = self.tiles[j][i]
                if tile!=0:
                    stepstaken += self.move_single_tile(tile,-1,0)
                self.update_tiles()

    if key =="RIGTH":
        for i,array in enumerate(self.tiles):
            for j,_ in enumerate(array):
                tile = self.tiles[3-j][3-i]
                if tile!= 0:
                    stepstaken += self.move_single_tile(tile,1,0)
                self.update_tiles()

has some redundant loops. You can move the if statement so the array looping os only written once.

for i, array in enumerate(self.tiles):
for j, _ in enumerate(array):
    if key=="LEFT":
        tile = self.tiles[j][i]
        if tile!=0:
            stepstaken += self.move_single_tile(tile,-1,0)
            self.update_tiles()
    if key =="RIGTH":
        tile = self.tiles[3-j][3-i]
        if tile!= 0:
            stepstaken += self.move_single_tile(tile,1,0)
            self.update_tiles()

I also recommend to use Enums for the directions.

When you replace the

m = Main()

with

if __name__ == "__main__":
   m = Main();

This allows you to import this file for example to use the Board class for another project. Otherwise the whole game would run on import.

\$\endgroup\$

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.