Game in Pythonista App

Question

I've written this Wack-a-mole like game on my iPad in the app Pythonista. Keeping that in mind, it uses a different graphic interface from the one I have usually seen used, Tkinter. I'm a beginner and this is my first major project, so any tips on how to code more efficiently and improve this program would be greatly appreciated.

import random
import ui
from time import sleep
import console
import sys
#turn button to "on" state
def turn_on(sender):
 sender.title = 'on'
 sender.tint_color = ('yellow')
#reverts button to off state 
def turn_off(sender):
 sender.title = 'off'
 sender.tint_color = 'light blue'
#briefly turns button to "on" state then reverts button to original state 
def blink(sender):
 turn_on(sender)
 sleep(.5)
 turn_off(sender)
#When the button is tapped, it reverses the current state of the button
def button_tapped(sender):
 turn_off(sender)
#Describes game
def game_Description():
 console.alert("Game objective:", 'Click the button to turn the light off before the next light turns on', "Ready")
#Pops up when user loses game 
def game_Over_Alert():
 play_Again = console.alert('Game Over!','','Play Again?')
 return play_Again
#Checks to see if all lights are off 
def check_Lights():
 if button.title == 'off':
 if button2.title == 'off':
 if button3.title == 'off':
 if button4. title == 'off':
 return True
#Turns off all lights
def all_Off():
 turn_off(button)
 turn_off(button2)
 turn_off(button3)
 turn_off(button4)
#Increase score by 1 and display
def increase_Score(x):
 x += 1
 score.title = 'Score: %d' % x
 return x
#setting UI and buttons 
view = ui.View()
view.name = 'Light Panel'
view.background_color = 'black'
button = ui.Button(title = 'off') 
button.center = (view.width*.2, view.height*.5)
button.flex = 'LRTB'
button.action = button_tapped
view.add_subview(button)
button2 = ui.Button(title = 'off')
button2.center = (view.width*.4, view.height*.5)
button2.flex = 'LRTB'
button2.action = button_tapped
view.add_subview(button2)
button3 = ui.Button(title = 'off')
button3.center = (view.width*.6, view.height*.5)
button3.flex = 'LRTB'
button3.action = button_tapped
view.add_subview(button3)
button4 = ui.Button(title = 'off')
button4.center = (view.width*.8, view.height*.5)
button4.flex = 'LRTB'
button4.action = button_tapped
view.add_subview(button4)
scoreCount = 0
score = ui.Button(title = 'Score: 0')
score.center = (view.width*.5, view.height*.75)
score.flex = 'LRTB'
view.add_subview(score)
#Set up display
view.present('sheet')
#Runs the game and handles the function
def play_Game():
 scoreCount = 0
 speed = 2.55
 game_Description()
 random.seed() 
 while check_Lights():
 x = random.randint(0,3)
 if x == 0:
 turn_on(button)
 elif x == 1:
 turn_on(button2)
 elif x == 2:
 turn_on(button3)
 else:
 turn_on(button4)
 scoreCount = increase_Score(scoreCount)
 sleep(speed)
 if speed >= 1.5:
 speed-= .07
 if game_Over_Alert():
 all_Off()
 play_Game()
play_Game() 
view.close()

• anon: Welcome to Code Review! Great debut. While I don't have much Python experience, I can suggest that you look up PEP8 and make sure you're abiding by that, if you haven't already.

Answer 1

Here are some general tips and guidelines:

• Do read the PEP8 guide – Loads of good information in that one

• Use docstring, not comments for functions – Docstrings will possibly show up in your IDE when you hover or read documentation on functions, so move your function comments to docstrings like this:

  def turn_on(sender):
   """Switch button to 'on' state."""
   ...
  

• Allow vertical space between functions – Between functions it is recommended to add two new lines, which helps separate the functions, and allows for using single new lines within functions to group code statements there.

• Be consistent in naming – You vary your function naming style from button_tapped() to game_Description(). I would strongly suggest to be consistent in naming (and other style aspect as well), and the most pythonic way for functions is snake_case.

• Switch to new style string formatting – Instead of the % operator, you are better of using 'Score: {}'.format(x). The former is depreceated, and might be removed in the future.

• Don't intermix top level code and functions – You have a section of top level code before the play_game() (aka play_Game()), which is kind of hidden. A really good advice is to not have any top level code besides the following at the bottom of the file:

  if __name__ == '__main__':
   main()
  

  That is group most of the top level code in a main() function, and gather the view initialisation in another function. (Not quite sure how this works in Pythonista, but alternatively, you'll have a single call to main() at the top level.)

• Don't use global variables – If possible, it is always better to pass values to functions, instead of using them globally. When restructuring into a view initialisation function, this introduces possibly a new concept of returning multiple values from a function (see code below)
• Feature: Your code always increases score, tapped or not – Your current code always gives you a increase in score, independent on whether you actually tapped anything or not. If however you didn't tap anything, you'll get a false test on the check_lights() test. This is kind of hiding the game logic. I'd move the test around a little, to increase understanding of game logic.

• Indentation errors in presented code – I'm surprised no-one commented on this yet, but the code as presented doesn't work, there is a faulty indentation on the x = random.randint(0, 3) line, and likewise on the speed -= .07 line. Both lines should be indented. A trick to remember for next post here on Code Review is to mark the entire code block (when editing your question/answer, and hit Ctrl+K. (Do not edit the code now, though, as you've gotten some answers)

• Reorganise buttons into a list – The other answers has touched upon this already, but to take it one step further, you could use a list holding all of your buttons, and this would simplify much of your logic.

• Include the 0 before floats – It is not neccessary, and is a minor detail, but I recommend keeping the 0 in front of floats. That is 0.07, instead of .07. (I kept misreading it as 0.7, and didn't quite see the use of increasing the speed twice before stopping the speed increase... )

Implementing all of these changes (and removing the increase score function, as it kind of hid a main concept of the game) we arrive at the following code:

import random
import ui
from time import sleep
import console
import sys
def turn_on(button):
 """Turn button to "on" state."""
 button.title = 'on'
 button.tint_color = ('yellow')
def turn_off(button):
 """Reverts button to off state."""
 button.title = 'off'
 button.tint_color = 'light blue'
def blink(button):
 """Briefly turns switches button state "on" and then "off"."""
 turn_on(button)
 sleep(.5)
 turn_off(button)
def button_tapped(button):
 """When the button is tapped, it turns the button "off"."""
 turn_off(button)
def game_description():
 """Describe game objective in alert box."""
 console.alert("Game objective:", 
 "Click the button to turn the light off before the next light turns on", 
 "Ready")
def game_over_alert():
 """Alert box stating game over, and asking for a new game."""
 return console.alert('Game Over!','','Play Again?')
def check_lights(buttons):
 """Check if all buttons are turned off."""
 return all(button.title == 'off' for button in buttons)
def all_off(buttons):
 """Turn off all buttons."""
 for button in buttons:
 turn_off(button)
def add_button(view, width_percentage, height_percentage,
 button_action=None, title='off'):
 """Adds a button positioned by percentages, with given title."""
 button = ui.Button(title)
 button.center = (view.width * width_percentage, view.height * height_percentage) 
 button.flex = 'LRTB'
 button.action = button_action
 view.add_subview(button)
 return button
def initialise_view():
 """Initialise view, and return view and buttons."""
 view = ui.View()
 # Default settings for view
 view = ui.View()
 view.name = 'Light Panel'
 view.background_color = 'black'
 # Create "mole" buttons
 mole_buttons = [add_button(view, 0.2 * (i + 1), 0.5,
 button_action=button_tapped) for i in range(4)]
 score_button = add_button(view, 0.5, 0.75, 'Score: 0')
 return view, mole_buttons, score_button
def play_game(view, mole_buttons, score_button):
 """Main game loop. Handles game, and restarting of game."""
 score_count = 0
 speed = 2.55
 game_description()
 random.seed() 
 while True:
 turn_on(mole_buttons[random.randint(0, 3)])
 # Allow player to wack a mole 
 sleep(speed)
 # Check if they hit the mole, that is check all lights are off
 if check_lights(mole_buttons):
 score_count += 1
 score_button.title = 'Score: {}'.format(score_count)
 else:
 # They failed hitting the mole
 break
 # Change speed a little
 if speed >= 1.5:
 speed-= 0.07
 if game_over_alert():
 all_off()
 play_game(view, mole_buttons, score_button)
def main():
 view, mole_buttons, score_button = initialise_view()
 view.present('sheet')
 play_game(view, mole_buttons, score_button)
 view.close()
if __name__ == '__main__':
 main()

I haven't installed Pythonista, yet, so I'm not able to test this code, but hopefully it should work nicely.

• Jamerack: Thank you very much! This helps quite a lot. I actually looked back over my code the next day and integrated the changes the others suggested as while as some of my own. I did put the buttons in a list, but the way I did it is much less efficient than your way. I knew that the fact that my score increased regardless of whether the played won or lost the round was a problem, but I couldn't think up a way to fix it with out recoding much of much of the program (and I was tired so I wasn't able to attempt it at the time). Again, this all helps very much, and I am working on improving my coding.

Answer 2

Please follow PEP8, the Python style guide.

---

This function returns either True or None:

def check_Lights():
 if button.title == 'off':
 if button2.title == 'off':
 if button3.title == 'off':
 if button4. title == 'off':
 return True

Make it a proper boolean. And, instead of the arrow shaped writing style, you can rewrite this as a single boolean expression, for example:

return button1.title == 'off' and button2.title == 'off' and ...

As @Caridorc pointed out in a comment, you can do even better:

return all(b.title == 'off' for b in (button, button2, button3, button4))

---

Don't repeat yourself. The way you create and add new buttons is very repetitive. Create a utility function to reduce the duplication, for example:

def add_new_button(wcoeff, hcoeff):
 button = ui.Button(title = 'off') 
 button.center = (view.width * wcoeff, view.height * hcoeff)
 button.flex = 'LRTB'
 button.action = button_tapped
 view.add_subview(button)
 return button

• Caridorc: Even less duplication: return all(b.title == 'off' for b in (button, button2 ...))
• Jamerack: I've been teaching myself Python, so I've never been told exactly how to style it, so I'll learn to follow PEP8, thanks!

Answer 3

random.choice

 x = random.randint(0,3)
 if x == 0:
 turn_on(button)
 elif x == 1:
 turn_on(button2)
 elif x == 2:
 turn_on(button3)
 else:
 turn_on(button4)

Becomes:

 turn_on( random.choice( (button, button2, button3, button4) ) )

The second is more coincise and readable.