1
\$\begingroup\$

Hey I am in the process of making this game, please could I get any feedback on how to further approach this, as I find it kind of inefficient. 

import pygame import time pygame.init() #Creating the Pygame window X = 500 Y = 500 win = pygame.display.set_mode((X, Y)) pygame.display.set_caption('Last to Pick') width = 65 height = 80 vel = 10 #Colors to Use cool_blue = (20, 50, 70) black = (0,0,0) white = (255, 255, 255) red = (255, 0, 0) green = (0, 255, 0) blue = (0, 0, 128) bright_red = (255,0,0) bright_green = (0,255,0) #1st Row A1 = pygame.Rect(180, 150, width, height) A2 = pygame.Rect(260, 150, width, height) #2nd Row B1 = pygame.Rect(140, 250, width, height) B2 = pygame.Rect(220, 250, width, height) B3 = pygame.Rect(300, 250, width, height) #3rd Row C1 = pygame.Rect(60, 350, width, height) C2 = pygame.Rect(140, 350, width, height) C3 = pygame.Rect(220, 350, width, height) C4 = pygame.Rect(300, 350, width, height) C5 = pygame.Rect(380, 350, width, height) cards = [ [A1, red], [A2, red], [B1, red], [B2, red], [B3, red], [C1, red], [C2, red], [C3, red], [C4, red], [C5, red] ] bot_turn_first = False bot_turn = False player_turn = False player_turn_finsihed = False "GENERAL GAME FUNCTIONS" def text_objects(text, font, color): text_surface = font.render(text, True, color) return text_surface, text_surface.get_rect() def msg_to_display(text): largeTEXT = pygame.font.Font('freesansbold.ttf',55) TextSurf, TextRect = text_objects(text, largeTEXT, black) TextRect.center = ((X/2),(Y/2)) win.blit(TextSurf, TextRect) pygame.display.update() def game_ended(): msg_to_display('Game has Ended, Click Here to Try Again.') def button(mouse,msg,x,y,w,h,ic,ac,action=None): click = pygame.mouse.get_pressed() print(click) if x+w > mouse[0] > x and y+h > mouse[1] > y: pygame.draw.rect(win, ac,(x,y,w,h)) if (click[0] or click[1] or click[2]) == 1 and action != None: action() else: pygame.draw.rect(win, ic,(x,y,w,h)) smallText = pygame.font.Font("freesansbold.ttf",20) textSurf, textRect = text_objects(msg, smallText, white) textRect.center = ( (x+(w/2)), (y+(h/2)) ) win.blit(textSurf, textRect) "GENERAL GAME FUNCTIONS END" def quit_box(): pygame.quit() def intro_screen(): run = True while run: win.fill(cool_blue) for event in pygame.event.get(): if event.type == pygame.QUIT: run = False if event.type == pygame.MOUSEMOTION: mouse = pygame.mouse.get_pos() font = pygame.font.Font('freesansbold.ttf', 50) text = font.render('The Last To Pick', True, green, cool_blue) textRect = text.get_rect() textRect.center = (250, 200) win.blit(text, textRect) button(mouse,'Start', 110, 350, 90, 50, black, green, start_loop) button(mouse,'Options', 220, 350, 90, 50, black, blue) button(mouse,'Quit', 330, 350, 90, 50, black, red, quit_box) pygame.display.update() def bot_playing(): msg_to_display('I get the first move') print('I get the first move') bot_start_loop() def player_playing(): msg_to_display('You get the first move') print('You get the first move') player_start_loop() def checking_player_turn(): global player_turn if player_turn == True: pass#playerturn() def start_loop(): run = True while run: win.fill(cool_blue) #when 'x' is clicked game loop will quit for event in pygame.event.get(): if event.type == pygame.QUIT: run = False if event.type == pygame.MOUSEMOTION: mouse2 = pygame.mouse.get_pos() button(mouse2, 'Bot can start', 300, 0, 200, 50, black, blue, bot_playing) button(mouse2, 'I want to start', 300, 60, 200, 50, black, blue, player_playing) for rect, color in cards: pygame.draw.rect(win, color, rect) pygame.display.update() def bot_start_loop(): cards_bot1 = [ [A1, red], [A2, red], #2nd Row [B1, red], [B2, red], [B3, red], #3rd Row [C1, red], ] run = True while run: win.fill(cool_blue) #when 'x' is clicked game loop will quit for event in pygame.event.get(): if event.type == pygame.QUIT: run = False if event.type == pygame.MOUSEMOTION: mouse3 = pygame.mouse.get_pos() button(mouse3, 'I finshed my turn', 300, 0, 200, 50, black, blue) for rect, color, in cards_bot1: pygame.draw.rect(win, color, rect) pygame.display.update() if event.type == pygame.MOUSEBUTTONUP: pos = pygame.mouse.get_pos() for rect, color in cards: if rect.collidepoint(pos): print('Player has selected card' + str(rect)) selected_cards_turn1 = [] def player_start_loop(): global selected_cards_turn1 run = True while run: win.fill(cool_blue) #when 'x' is clicked game loop will quit for event in pygame.event.get(): if event.type == pygame.QUIT: run = False if event.type == pygame.MOUSEMOTION: mouse3 = pygame.mouse.get_pos() button(mouse3, 'I finshed my turn', 300, 0, 200, 50, black, blue, card_remover) for rect, color in cards: pygame.draw.rect(win, color, rect) pygame.display.update() if event.type == pygame.MOUSEBUTTONUP: pos = pygame.mouse.get_pos() for rect, color in cards: if rect.collidepoint(pos): print('Player has selected card' + str(rect)) nrect = str(rect) selected_cards_turn1.append(nrect) def card_remover(): global selected_cards_turn1 print('Selected cards' + str(selected_cards_turn1)) for element in selected_cards_turn1: if element == '<rect(180, 150, 65, 80)>':#A1 exterminate(element, 0) if element == '<rect(260, 150, 65, 80)>':#A2 exterminate(element, 1) if element == '<rect(140, 250, 65, 80)>':#B1 exterminate(element, 2) if element == '<rect(220, 250, 65, 80)>':#B2 exterminate(element, 3) if element == '<rect(300, 250, 65, 80)>':#B3 exterminate(element, 4) if element == '<rect(60, 350, 65, 80)>':#C1 exterminate(element, 5) if element == '<rect(140, 350, 65, 80)>':#C2 exterminate(element, 6) if element == '<rect(220, 350, 65, 80)>':#C3 exterminate(element, 7) if element == '<rect(300, 350, 65, 80)>':#C4 exterminate(element, 8) if element == '<rect(380, 350, 65, 80)>':#C5 exterminate(element, 9) def new_loop(new_card_list): run = True while run: win.fill(cool_blue) #when 'x' is clicked game loop will quit for event in pygame.event.get(): if event.type == pygame.QUIT: run = False if event.type == pygame.MOUSEMOTION: mouse4 = pygame.mouse.get_pos() button(mouse4, 'I finshed my turn', 300, 0, 200, 50, black, blue, card_remover) for rect, color in new_card_list: pygame.draw.rect(win, color, rect) pygame.display.update() def exterminate(cards1, ele): cards11 = str(cards1) + ', (255, 0, 0)' list(cards11) print(""" """) new_set_of_cards = cards.copy(); del new_set_of_cards[ele] #new_set_of_cards = [ments for ments in cards if ments != cards11] print('Drawing this cards: ' + str(cards11)) print('New Set of Cards' + str(new_set_of_cards)) new_loop(new_set_of_cards) intro_screen() pygame.quit()

Thank you!!

\$\endgroup\$
3
  • \$\begingroup\$ Actually I'm stuck on the main screen with the three buttons. Mouse clicks don't trigger anything, and I've got just "(0, 0, 0)" spammed in the console. \$\endgroup\$ Commented Feb 12, 2020 at 9:06
  • \$\begingroup\$ I don't know PyGame library, but I've just understand why I can't click on any buttons: you check for clicks only in the MOUSEMOTION event... So, clicking while moving the mouse works. \$\endgroup\$ Commented Feb 12, 2020 at 9:18
  • \$\begingroup\$ @VincentRG Yes on the start screen for the buttons to 'work' you have to be holding down the mouse click on the button. I don't know why it is this way but it is the closest I got to an actual button... \$\endgroup\$ Commented Feb 12, 2020 at 18:29

1 Answer 1

Reset to default
2
\$\begingroup\$

Here are a few things that stand out to me:

Unused import

You import time but don't use it in your code. It should be removed

Style

You should adhere to the PEP 8 style guide. This guide may seem a little superfluous, but having a consistent style makes your code easier to work with should you modify it and having your code follow the same style guide as mostly all python code makes it easier to review.

Constants should be in capitals, so your colors should be called COOL_BLUE, BLACK, etc.

Top level functions should be surrounded by 2 blank lines. In your code, they are surrounded with 1 to 3 blank lines.

Also, you use random strings ("GENERAL GAME FUNCTIONS") as... comments? Something else? Don't do that, it makes no sense.

Documentation

Your code uses some comments that makes it a bit easier to follow, but doesn't include any docstring. Docstrings should document everything your function does (which should be only one thing), what are the arguments it takes and their purposes, and what it returns, so anyone using it (including yourself in the future) can know what it does without working out the logic of the code.

Code layout

Your code has some executed statements, then function definitions with some executed statements between some of them, then some more executed statements. It makes it hard to read and follow the logic. Put every statement shat should be executed together, preferably at the end, preferably behind an if __name__ = '__main__': guard (except for the constants)

Globals

You use global variables. This is considered to be bad practice, as it makes your logic harder to follow and error prone (it is quite easy to forget that a global was modified in another part of the code). Instead, pass the relevant values as arguments.

Separation of concerns

Your code mixes the game logic and its representation with Pygame. It makes the logic flow hard to follow. Instead, you should ensure your game can run independently, and call it with whatever you use for I/O (Pygame for now, but you may want to switch to something else or debug the game in the terminal).

Encapsulating the game logic in a class is probably the way to go.

Lack of flexibility

Your nim game has 3 rows of respectively 2, 3 and 5 items, each stored in its own, hardcoded variable. However, there are a lot of variants for the game. The first picture on the Wikipedia page shows a game with 4 rows of 1, 3, 5 and 7 items. In fact, you could play the game with any number of rows, containing each any number of item.

A list of list to contain these game objects would be much more flexible, and the number of items per row should be passed as parameters to the game class's __init__() function.

\$\endgroup\$
3
  • \$\begingroup\$ Thanks a lot for the input, will try and adjust it accordingly. A few things to mention, I use global variable cause I could not find a way to pass the selected_cards_turn1p into card_remover(). As card_remover() is being called within a function itself and doesn't work when I give it a parameter. You mention that i should use a class for the cards, how would I make/put this in the code?? \$\endgroup\$ Commented Feb 12, 2020 at 18:33
  • \$\begingroup\$ @RutvikKarupothula I didn't mean you should make a class card. In fact, only the number of cards in each rows is required to represent the game. For graphical representation purposes, you may want to keep track of exactly which cards were removed, in which case a list of booleans would be enough. Either way, a class for cards would be overkill IMO. I meant however that you should encapsulate the game core in its own class, and have the Pygame representation refer to an instance of that class. It also allows you to use attributes in basically the same way as you currently use globals. \$\endgroup\$ Commented Feb 13, 2020 at 8:56
  • \$\begingroup\$ Ahhh that makes sense, I will try to do that. I am quite new to python and stuff so it might not work, i'll let you know when I update it... thx \$\endgroup\$ Commented Feb 13, 2020 at 8:58

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.