Object-oriented model of a machine gun using Pygame

Question

I just started to use classes in Python, so I don't know all concepts, rules and conventions about it. I made a little program of a machine gun which simulates firing by user's action. bullets is somehow the stock, and magazine, bullets ready to fire, in the gun. When magazine is at 0, you reload it with bullets. I'm using python 3.7. Here it is:

import random, pygame, sys
class MachineGun(object):
 bullets = 200
 def __init__(self):
 self._magazine = 50
 def _get_magazine(self):
 return self._magazine
 def _set_magazine(self, val):
 self._magazine = val
 magazine = property(_get_magazine, _set_magazine)
 def fire(self):
 pygame.mixer.Sound("fire.wav").play()
 while pygame.mixer.get_busy():
 pass
 fired = random.randint(1, 21)
 self.magazine = self.magazine-fired
 if self.magazine <= 0 :
 print("Bullets remaining : 0")
 else :
 print("Bullets remaining : {}".format(self.magazine))
 def reload(self):
 if self.magazine == 50 :
 print("Full")
 else :
 if MachineGun.bullets >= 50 :
 if self.magazine == 0 :
 self.magazine = 50
 MachineGun.bullets = MachineGun.bullets-50
 else :
 tmp = 50-self.magazine
 self.magazine = self.magazine+tmp
 MachineGun.bullets = MachineGun.bullets-tmp
 else :
 if self.magazine == 0 :
 self.magazine = MachineGun.bullets
 MachineGun.bullets = 0
 else :
 if self.magazine+MachineGun.bullets > 50 :
 tmp = 50 - self.magazine
 self.magazine = self.magazine+tmp
 MachineGun.bullets = MachineGun.bullets-tmp
 else :
 self.magazine += MachineGun.bullets
 MachineGun.bullets = 0
 pygame.mixer.Sound("reload.wav").play()
 while pygame.mixer.get_busy():
 pass
 def action(self):
 a = "x"
 while a != "e" :
 if a == "" :
 if self.magazine > 0 :
 self.fire()
 elif self.magazine <= 0 and MachineGun.bullets > 0 :
 self.magazine = 0
 print("Reload")
 else :
 print("End")
 sys.exit()
 elif a == "r" :
 self.reload()
 a = input()
M = MachineGun()
M.action()

This is somehow my first "complete" implementation using classes. So I don't know if the form is correct. For exemple, I modified the attribute bullets without using a class method. Should I ? Like properties for object attributs ? Is it very necessary to use properties ?

Answer 1

UX

When I run the code, it just hangs in my shell. The code has this line:

a = input()

This means that the code is waiting for user input, but there is no indication of that. You should add text to the prompt which provides instructions for the user. For example:

a = input('Enter an option. To exit hit e, to reload hit r, to show remaining hit Enter ')

Also, a is not a very descriptive name for a variable in this context. action would be more appropriate. Since the function is already named action, I recommend renaming the function as run_game.

Import

It is a good practice to split each import onto its own line. Change:

import random, pygame, sys

to:

import random
import pygame
import sys

OOP

Using object is no longer needed:

class MachineGun(object):

It is simpler as:

class MachineGun():

You should structure the class such that you don't use the class name itself within the class. For example, it would be better if you did not use MachineGun in the line below:

if MachineGun.bullets >= 50 :

Generally, you should reference a class variable with self.

f-string

The following line:

print("Bullets remaining : {}".format(self.magazine))

is simpler with an f-string:

print(f"Bullets remaining : {self.magazine}")

Documentation

The PEP 8 style guide recommends adding doctrings to describe your class and functions. It would be nice to add a doctrings to summarize the purpose of the code as well. This header docstring would also contain instructions for the user as well as notifying the user that input .wav files are needed to run the code.

Whitespace

The code uses inconsistent whitespace. The black program can be used to automatically format the code consistently.

Assignment

This line:

MachineGun.bullets = MachineGun.bullets-50

can be simplified using a special assignment operator:

MachineGun.bullets -= 50

There are other instances of this. In fact, you already used it here:

self.magazine += MachineGun.bullets

Answer 2

Your reload function can be improved a bit:

def is_reload_required(self):
 return not self.magazine == 50
def reload(self):
 if not self.is_reload_required():
 print("Magazine is already full")
 # reload is not needed, so return from here
 return
 bullets_remaining = min(MachineGun.bullets, 50)
 bullets_needed_for_reload = bullets_remaining - self.magazine
 self.magazine += bullets_needed_for_reload
 MachineGun.bullets -= bullets_needed_for_reload
 pygame.mixer.Sound("reload.wav").play()
 while pygame.mixer.get_busy():
 pass

Also, you should not use magic numbers, for example, 50 for magazine capacity. Define named variables instead, like MAGAZINE_CAPACITY and assign it to 50. There also may be a way to make such variables as read-only constants (see documentation for @property: https://docs.python.org/3/library/functions.html#property).