Server and client for pong game

Question

This is a pong style game which is very basic but involves sending and receiving data using TCP. The data is encoded using pickle which seems to be very inefficient yet I am still using it.

The Server:

The server calculates the new positions of the paddles and balls depending on the inputs from each player.

import socket, time, pickle, random

serversocket = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
serversocket.bind(("YOUR_IP", CHOSEN_PORT))
serversocket.listen(2)
arr = [400,400,400,400,0,0]
connection = []
ball_y_speed = 1
ball_x_speed = 1



def process_positions(array, player_1, player_2):
    #info[0] = key_up
    #info[1] = key_down
    global ball_y_speed, ball_x_speed


    '''PADDLE MOVING'''
    if player_1[0] == True:
        array[0]-=1
    else:
        array[0] = array[0]
    if player_1[1] == True:
        array[0]+=1
    else:
        array[0] = array[0]

    if player_2[0] == True:
        array[1]-=1
    else:
        array[1] = array[1]
    if player_2[1] == True:
        array[1]+=1
    else:
        array[1] = array[1]

    if array[0]<0:
        array[0] = 0
    elif array[0] > 540:
        array[0] = 540

    if array[1]<0:
        array[1] = 0
    elif array[1] > 540:
        array[1] = 540

    '''PADDLE MOVING'''

    '''BALL MOVING'''
    array[2] += round(ball_y_speed)
    array[3] += round(ball_x_speed)

    negative_speed = [-0.6, -0.65, -0.7, -0.75, -0.8, -0.85, -0.9, -0.95, -1]
    positive_speed = [-1, -1.05, -1.1, -1.15, -1.2, -1.25, -1.3, -1.35, -1.4, -1.45, -1.5]

    if array[2] > 595:
        if ball_y_speed >= 1:
            ball_y_speed *= random.choice(negative_speed)
        elif ball_y_speed < 1:
            ball_y_speed *= random.choice(positive_speed)
    if array[2] < 0:
        if ball_y_speed >= 1:
            ball_y_speed *= random.choice(negative_speed)
        elif ball_y_speed < 1:
            ball_y_speed *= random.choice(positive_speed)
    if array[3]>795:
        if ball_x_speed >= 1:
            ball_x_speed *= random.choice(negative_speed)
        elif ball_x_speed < 1:
            ball_x_speed *= random.choice(positive_speed)
        array[4] += 1
    if array[3]<0:
        if ball_x_speed >= 1:
            ball_x_speed *= random.choice(negative_speed)
        elif ball_x_speed < 1:
            ball_x_speed *= random.choice(positive_speed)
        array[5] += 1

    '''BALL MOVING'''


    '''PADDLE DETECTION'''
    if array[3]<20 and (array[0]<array[2] and array[0]+60>array[2]):
        ball_x_speed *=-1
    if array[3]>780 and (array[1]<array[2] and array[1]+60>array[2]):
        ball_x_speed *=-1

    #info = [player_1_y, player_2_y, ball_y, ball_x, score_1, score_2]

    return array

def waiting_for_connections():
    while len(connection)<2:
        conn, addr = serversocket.accept()
        connection.append(conn)
        print(conn)
        print(connection)

def recieve_information():
    player_1_info = pickle.loads(connection[0].recv(1024))
    player_2_info = pickle.loads(connection[1].recv(1024))

    return player_1_info, player_2_info


while True:
    waiting_for_connections()

    data_arr = pickle.dumps(arr)
    print(data_arr)
    connection[0].send(data_arr)
    connection[1].send(data_arr)

    player1, player2 = recieve_information()

    arr = process_positions(arr,player1, player2)

The Client:

Displays the game for each player and receives the inputs using pygame of whether you press the up or down arrow key.

import pygame
import socket
import time
import pickle

pygame.init()

RED = (255, 0, 0)
WHITE = (255, 255, 255)
BLUE = (0, 0, 255)
BLACK = (0, 0, 0)
BLUE = (0, 0, 255)
GREEN = (0,255,0)

display_width = 800
display_height = 600

clock = pygame.time.Clock()

gameDisplay = pygame.display.set_mode((display_width, display_height))

clientsocket = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
clientsocket.connect(("YOUR_IP ", CHOSEN_PORT))


def message_display(text,x,y):
    largeText = pygame.font.Font('freesansbold.ttf',45)
    TextSurf, TextRect = text_objects(str(text), largeText, GREEN)
    TextRect.center = ((x),(y))
    gameDisplay.blit(TextSurf, TextRect)

    pygame.display.update()

def text_objects(text, font, colour):
    textSurface = font.render(text, True, colour)
    return textSurface, textSurface.get_rect()


def recieve_data():
    data = clientsocket.recv(1024)
    data = pickle.loads(data)

    return data

def draw_paddles(x,y,p):
    if p == 1:
        pygame.draw.rect(gameDisplay, RED, [x, y, 10, 60])
    if p == 2:
        pygame.draw.rect(gameDisplay, BLUE, [x, y, 10, 60])

def draw_ball(x,y):
    pygame.draw.circle(gameDisplay, BLACK, [x,y], 5)

def display():
    game_finished = False
    data = []
    key_up = False
    key_down = False
    while game_finished == False:
        info = recieve_data()
        gameDisplay.fill(WHITE)
        draw_paddles(10, info[0], 1)
        draw_paddles(display_width-20, info[1], 2)
        draw_ball(info[3], info[2])

        pygame.display.update()

        for event in pygame.event.get():
            if event.type == pygame.KEYDOWN:
                if event.key == pygame.K_UP:
                    key_up = True
                if event.key == pygame.K_DOWN:
                    key_down = True
            if event.type == pygame.KEYUP:
                if event.key == pygame.K_UP:
                    key_up = False
                if event.key == pygame.K_DOWN:
                    key_down = False

        arr = [key_up, key_down]
        data_arr = pickle.dumps(arr)
        clientsocket.send(data_arr)
        message_display(info[4], 250, 300)
        message_display(info[5], 550, 300)


    #info = [player_1_y, player_2_y, ball_y, ball_x, score_1, score_2]


display()

Answer 1

Functionally, this looks decent and most of it isn't hard to follow. I have a pile of suggestions below, but the intention is just to improve consistency and maintainability of your code by following conventions (either standard or keeping consistent with your own code) and using naming and logic tweaks to make the code less verbose and more self-documenting.

• Use named values rather than literals. I'd suggest following this rule for basically any value that isn't -1, 0, 1, or in some cases 2. E.g. in your server, it's unclear what exactly 540 and 595 are, so instead perhaps define:

  goal_margin = 5
  right_margin = display_width - goal_margin
  # etc.
  

  This way, you can check if array[2] > right_margin: ...

• Use more descriptive names. Instead of putting both paddles and the ball into a single variable called array, consider have multiple lists, left_paddle, right_paddle, ball_position, ball_speed, and score so it becomes self-descriptive what you're doing. You could keep these in a tuple, e.g. game_state = (left_paddle, ... score) so that you still only have that one variable to pass around.

• There seems to be a lot of cleanup possible around the pile of if/else statements. For example, instead of

  if player_1[0] == True:
      array[0]-=1
  else:
      array[0] = array[0]
  if player_1[1] == True:
      array[0]+=1
  else:
      array[0] = array[0]
  

  you could have just

  array[0] += -int(player_1[0]) + int(player_1[1])
  

  Or at the very least, don't check == True, just use if player_1[0]: ...

  You can also clip the bounds of values more easily. Instead of

  if array[0]<0:
      array[0] = 0
  elif array[0] > 540:
      array[0] = 540
  

  you could use the following one-liner (in which I also suggest naming your values):

  array[0] = min(max(array[0], top_margin), bottom_margin)
  

  For the ball bounce speed you could reduce

  if array[2] > 595:
      if ball_y_speed >= 1:
          ball_y_speed *= random.choice(negative_speed)
      elif ball_y_speed < 1:
          ball_y_speed *= random.choice(positive_speed)
  if array[2] < 0:
      if ball_y_speed >= 1:
          ball_y_speed *= random.choice(negative_speed)
      elif ball_y_speed < 1:
          ball_y_speed *= random.choice(positive_speed)
  

  to just

  if array[2] < 0 or array[2] > 595:
      ball_y_speed *= random.choice(negative_speed if ball_y_speed >= 1 else positive_speed)
  

  and the same for ball_x_speed.

• Follow PEP8 conventions. Regarding spacing: if array[1]<0: becomes if array[1] < 0:, etc. Regarding naming conventions, use snake-case: gameDisplay becomes game_display, etc. (most of your variable names already do this).

• Why are ball_x/y_speed global variables, when everything else about the state of the game is contained in mutable parameters that get passed around? Also, process_positions both mutates the given array and returns the new array value which you then assign into the source array. Instead of game state being modified by globals, in-place mutation, and return assignment, only use one of these methods and use it consistently--I'd suggest using in-place mutation in this case.

• This suggestion is much broader and would require more extensive re-working, but you have a lot of configuration values set throughout your code. I'd recommend centralizing these into a single object that the server then sends to the clients when the game begins. For example, the display dimensions, margins for paddles and goals, paddle width, etc. could all be contained in a dictionary that the server owns and syncs initially with the clients.

  You might also want to specifically separate the purposes of colors from the definitions of those colors. I.e., instead of

  GREEN = (0,255,0)
  ...
  TextSurf, TextRect = text_objects(str(text), largeText, GREEN)
  

  I'd suggest

  GREEN = (0,255,0)
  MSG_COLOR = GREEN
  ...
  TextSurf, TextRect = text_objects(str(text), largeText, MSG_COLOR)
  

  so that, if you want to change the color of messages, there will always be exactly one location where that's defined, and it's abstracted from your palate of possible colors. Likewise, I'd suggest that, specifically for the font name, you abstract that away into a named value as well. These tweaks make future development and maintenance easier.