Language skill tester using Pygame

Question

I made this little game that tests our language skills. It uses one external module, pinyin, which can be installed via the command prompt command

pip install pinyin

Here is how it goes:

enter image description here

As you can see from the GIF, the game starts with a random selection of characters for the top boxes, and a pronunciation for each corresponding character for the bottom boxes, shuffled, of course.

Every time the user connects a character with the right pronunciation, the score will increase by 10, and the matching boxes will get disabled. Every time the user connects a character with the wrong pronunciation, the score will decrease by 5.

When all the boxes are disabled, the game resets with another random sample of characters, and the score remain the value it was before all the boxes got disabled.

Here is my code:

import pygame
from random import sample, shuffle, choice
import pinyin
pygame.init()
pygame.font.init()
wn = pygame.display.set_mode((600, 400))
words = '的是不我一有大在人了中到資要可以這個你會好為上來就學交也用能如文時沒說他看提那問生過下請天們所多麼小想得之還'
class Text:
 def __init__(self, text, borders, font):
 self.font = font
 self.color = (255, 255, 0)
 self.text = self.font.render(text, True, self.color)
 width = self.text.get_width()
 height = self.text.get_height()
 x, y, w, h = borders
 self.pos = x + (w - width) / 2, y + (h - height) / 2
 def draw(self):
 wn.blit(self.text, self.pos)
class Score:
 def __init__(self, pos, font=pygame.font.SysFont("Californian FB", 40)):
 self.pos = pos
 self.font = font
 self.value = 0
 self.color = (255, 255, 255)
 self.up = 10
 self.down = 5
 self.potential = 0
 def score_up(self):
 self.value += 10
 
 def score_down(self):
 self.value -= 5
 self.potential += 10
 def draw(self):
 wn.blit(self.font.render(str(self.value), True, self.color), self.pos)
class Box:
 def __init__(self, bottom, x, y, w, h, word, font=pygame.font.SysFont("microsoftyaheimicrosoftyaheiui", 23)):
 self.rect = pygame.Rect(x, y, w, h)
 self.inner_rect = pygame.Rect(x + w * .125, y + h * .125, w * .75, h * .75)
 self.box_color = (100, 0, 0)
 self.port_color = (50, 0, 0)
 self.active_color = (20, 0, 0)
 self.disabled = False
 self.port = pygame.Rect(x + w / 4, y + h, w / 2, h // 8)
 self.word = word
 if bottom:
 self.port.y -= h + h // 8
 word = pinyin.get(word)
 self.text = Text(word, (x, y, w, h), font)
 def over(self, x, y):
 return self.rect.collidepoint(x, y)
 def disable(self):
 self.disabled = True
 self.box_color = (255, 50, 50)
 def draw(self, active):
 if active:
 pygame.draw.rect(wn, self.active_color, self.rect)
 pygame.draw.rect(wn, self.box_color, self.inner_rect)
 else:
 pygame.draw.rect(wn, self.box_color, self.rect)
 self.text.draw()
 if self.disabled:
 return
 pygame.draw.rect(wn, self.port_color, self.port)
class Game:
 def __init__(self, words, x, y):
 self.w = 70
 self.h = 50
 self.box_x_pad = 30
 self.box_y_pad = 120
 self.words = words
 self.in_boxes = list()
 self.out_boxes = list()
 self.active_in_box = None
 self.active_out_box = None
 for i, word in enumerate(words):
 in_box = Box(False, x + i * (self.w + self.box_x_pad), y, self.w, self.h, word)
 self.in_boxes.append(in_box)
 shuffle(words)
 for i, word in enumerate(words):
 out_box = Box(True, x + i * (self.w + self.box_x_pad), y + self.box_y_pad, self.w, self.h, word)
 self.out_boxes.append(out_box)
 def won(self):
 return all(in_box.disabled for in_box in self.in_boxes)
 def draw_lines(self, event):
 if self.active_in_box:
 if self.active_out_box:
 pygame.draw.line(wn, (255, 255, 0), self.active_in_box.port.center, self.active_out_box.port.center, 7)
 else:
 pygame.draw.line(wn, (255, 255, 0), self.active_in_box.port.center, event.pos, 7)
 
 def draw_boxes(self):
 for in_box in self.in_boxes:
 in_box.draw(in_box == self.active_in_box)
 for out_box in self.out_boxes:
 out_box.draw(out_box == self.active_out_box)
bg_color = (200, 0, 0)
game = Game(sample(words, 5), 60, 120)
score = Score((60, 50))
running = True
while running:
 wn.fill(bg_color)
 for event in pygame.event.get():
 if event.type == pygame.QUIT:
 running = False
 elif event.type == pygame.MOUSEBUTTONDOWN:
 for in_box in game.in_boxes:
 if in_box.over(*event.pos) and not in_box.disabled:
 game.active_in_box = in_box
 break
 elif event.type == pygame.MOUSEMOTION:
 if game.active_in_box:
 for out_box in game.out_boxes:
 if out_box.over(*event.pos) and not out_box.disabled:
 game.active_out_box = out_box
 break
 else:
 game.active_out_box = None
 elif event.type == pygame.MOUSEBUTTONUP:
 if game.active_in_box and game.active_out_box:
 if game.active_out_box.word == game.active_in_box.word:
 game.active_out_box.disable()
 game.active_in_box.disable()
 score.score_up()
 if game.won():
 game = Game(sample(words, 5), 60, 120)
 else:
 score.score_down()
 game.active_out_box = game.active_in_box = None
 game.draw_lines(event)
 game.draw_boxes()
 score.draw()
 pygame.display.update()
pygame.quit()

I want to improve the efficiency of my code, and improve the DRY concept of my code.

• maxb: The first thing to consider before you start optimizing your code: is the code slow? Have you done any benchmarking? How fast do you expect it to run? Of course you can always make your code more efficient, but from looking at the code above I see other issues that need to be addressed.

Answer 1

I'd say that your code looks quite nice, but there are definitely things that could be (and need to be) improved. One thing that does not need to be improved is the efficiency of the code. I did some quick benchmarking, and the frame time is about 2-3ms, which is completely fine. Here are the things I would fix:

1. Lines should not exceed 80 characters in length.
2. You have some global variables at the top, see if you can avoid them.
3. You have a massive while loop which looks complicated, refactor.
4. Your scoping is a bit wonky for some variables, even if it works.

To start with the third issue (the first two issues are minor), see if you can describe in words what each if case handles, and move the code into separate functions/methods. With this in mind, you could refactor to something like this:

while running:
 wn.fill(bg_color)
 for event in pygame.event.get():
 if event.type == pygame.QUIT:
 running = False
 elif event.type == pygame.MOUSEBUTTONDOWN:
 handle_mouse_down()
 elif event.type == pygame.MOUSEMOTION:
 handle_mouse_motion()
 elif event.type == pygame.MOUSEBUTTONUP:
 handle_mouse_up()
 draw_all_and_update(event)

From looking at this, you might also discover that the event variable should only be used in the scope of the for loop, but you use it after the loop when you draw. Structuring your code like this makes it more readable, and easier to debug.

Actually, when looking at your code, it seems that everything from the line bg_color = (200, 0, 0) and onwards can be encapsulated in some sort of window handler. I took the liberty of writing something together:

class WindowHandler:
 def __init__(self, words):
 self.bg_color = (200, 0, 0)
 self.wn = pygame.display.set_mode((600, 400))
 self.game = Game(sample(words, 5), 60, 120)
 self.score = Score((60, 50))
 self.running = True
 def handle_mouse_down(self, event):
 for in_box in self.game.in_boxes:
 if in_box.over(*event.pos) and not in_box.disabled:
 self.game.active_in_box = in_box
 break
 def handle_mouse_move(self, event):
 if self.game.active_in_box:
 for out_box in self.game.out_boxes:
 if out_box.over(*event.pos) and not out_box.disabled:
 self.game.active_out_box = out_box
 break
 else:
 self.game.active_out_box = None
 def handle_mouse_up(self, event):
 if self.game.active_in_box and self.game.active_out_box:
 if self.game.active_out_box.word == self.game.active_in_box.word:
 self.game.active_out_box.disable()
 self.game.active_in_box.disable()
 self.score.score_up()
 if self.game.won():
 self.game = Game(sample(words, 5), 60, 120)
 else:
 self.score.score_down()
 self.game.active_out_box = self.game.active_in_box = None
 def draw_all_and_update(self, event):
 self.game.draw_lines(event, self.wn)
 self.game.draw_boxes(self.wn)
 self.score.draw(self.wn)
 pygame.display.update()
 def main_loop(self):
 while self.running:
 self.wn.fill(self.bg_color)
 for event in pygame.event.get():
 if event.type == pygame.QUIT:
 self.running = False
 elif event.type == pygame.MOUSEBUTTONDOWN:
 self.handle_mouse_down(event)
 elif event.type == pygame.MOUSEMOTION:
 self.handle_mouse_move(event)
 elif event.type == pygame.MOUSEBUTTONUP:
 self.handle_mouse_up(event)
 self.draw_all_and_update(event)
 pygame.quit()
if __name__ == "__main__":
 handler = WindowHandler(words)
 handler.main_loop()

Here, all the code is easy to find, and we know what each method does from its name (though you could have even more descriptive names). Refactoring the code like this means that you have to send the game window by reference into each draw function, but that's a minor increase in complexity compared to the major benefit of this structure.

• Chocolate: Thanks! But I want to know, is the major benefit of this structure only for simplicity understanding the code, or would it benefit the finished result as well, such as efficiency?
• maxb: I'd say that the finished result of any coding project isn't just the result from running the code, but also the code itself. The improvements I've suggested above do not affect the "efficiency" of the code at all, but unless you're going to add more functionality or run your code on a low-powered machine, you don't need it to be more efficient. With that said, if you want to find bottlenecks and see what's slow, I'd scale everything up. Instead of having 5 pairs of boxes, have 100 (or 1000). If your program is slow then, it'll be easier to profile. If there are no issues then, you're good.

Answer 2

The two most important things to fix here are:

Your code is non-reentrant, relying on a global wn surface which prevents coexistence of other surfaces in the same process; this is fairly easily fixed by passing in wn to your classes.

Also, you've hard-coded a font that many (most?) other people will not have, so your code is non-portable. This might not be easy to fix - in runtime you could attempt to inspect the system fonts to see which ones contain the Mandarin glyphs that you use; or (somewhat more easily) hard-code a priority-ordered list of acceptable fonts and load the first one that's present. For me Droid Sans works well.

Also you should consider introducing type hints.

As a first cut (minus any intelligent font handling) this gets us to:

import pinyin
import pygame
from pygame import draw
from pygame.event import Event
from pygame.font import Font, SysFont
from pygame.surface import Surface
from random import sample, shuffle
from typing import List, Tuple, Literal
WORDS = (
 '的是不我一有大在人了中到資要可以這個'
 '你會好為上來就學交也用能如文時沒說他'
 '看提那問生過下請天們所多麼小想得之還'
)
class Text:
 def __init__(self, wn: Surface, text: str, borders: Tuple[int, int, int, int], font: Font):
 self.wn = wn
 self.font = font
 self.color = (255, 255, 0)
 self.text = self.font.render(text, True, self.color)
 width = self.text.get_width()
 height = self.text.get_height()
 x, y, w, h = borders
 self.pos = x + (w - width) / 2, y + (h - height) / 2
 def draw(self):
 self.wn.blit(self.text, self.pos)
class Score:
 def __init__(self, wn: Surface, pos: Tuple[int, int], font: Font):
 self.wn = wn
 self.pos = pos
 self.font = font
 self.value = 0
 self.color = (255, 255, 255)
 self.up = 10
 self.down = 5
 self.potential = 0
 def score_up(self):
 self.value += 10
 def score_down(self):
 self.value -= 5
 self.potential += 10
 def draw(self):
 self.wn.blit(self.font.render(str(self.value), True, self.color), self.pos)
class Box:
 def __init__(
 self,
 wn: Surface,
 bottom: bool,
 x: int,
 y: int,
 w: int,
 h: int,
 word: str,
 font: Font,
 ):
 self.wn = wn
 self.rect = pygame.Rect(x, y, w, h)
 self.inner_rect = pygame.Rect(x + w * .125, y + h * .125, w * .75, h * .75)
 self.box_color = (100, 0, 0)
 self.port_color = (50, 0, 0)
 self.active_color = (20, 0, 0)
 self.disabled = False
 self.port = pygame.Rect(x + w / 4, y + h, w / 2, h // 8)
 self.word = word
 if bottom:
 self.port.y -= h + h // 8
 word = pinyin.get(word)
 self.text = Text(wn, word, (x, y, w, h), font)
 def over(self, x: int, y: int) -> bool:
 ret: Literal[0, 1] = self.rect.collidepoint(x, y)
 return bool(ret)
 def disable(self):
 self.disabled = True
 self.box_color = (255, 50, 50)
 def draw(self, active: bool):
 if active:
 pygame.draw.rect(self.wn, self.active_color, self.rect)
 pygame.draw.rect(self.wn, self.box_color, self.inner_rect)
 else:
 pygame.draw.rect(self.wn, self.box_color, self.rect)
 self.text.draw()
 if not self.disabled:
 pygame.draw.rect(self.wn, self.port_color, self.port)
class Game:
 def __init__(self, wn: Surface, words: List[str], x: int, y: int):
 self.wn = wn
 self.w = 70
 self.h = 50
 self.box_x_pad = 30
 self.box_y_pad = 120
 self.words = words
 self.in_boxes = list()
 self.out_boxes = list()
 self.active_in_box = None
 self.active_out_box = None
 box_font = SysFont("Droid Sans", 23)
 for i, word in enumerate(words):
 in_box = Box(
 wn=wn,
 bottom=False,
 x=x + i * (self.w + self.box_x_pad),
 y=y,
 w=self.w, h=self.h, word=word, font=box_font,
 )
 self.in_boxes.append(in_box)
 shuffle(words)
 for i, word in enumerate(words):
 out_box = Box(
 wn=wn,
 bottom=True,
 x=x + i * (self.w + self.box_x_pad),
 y=y + self.box_y_pad,
 w=self.w, h=self.h, word=word, font=box_font,
 )
 self.out_boxes.append(out_box)
 def won(self):
 return all(in_box.disabled for in_box in self.in_boxes)
 def draw_lines(self, event: Event):
 if self.active_in_box:
 if self.active_out_box:
 draw.line(self.wn, (255, 255, 0), self.active_in_box.port.center, self.active_out_box.port.center, 7)
 else:
 draw.line(self.wn, (255, 255, 0), self.active_in_box.port.center, event.pos, 7)
 def draw_boxes(self):
 for in_box in self.in_boxes:
 in_box.draw(in_box == self.active_in_box)
 for out_box in self.out_boxes:
 out_box.draw(out_box == self.active_out_box)
def main():
 pygame.init()
 pygame.font.init()
 wn: Surface = pygame.display.set_mode((600, 400))
 bg_color = (200, 0, 0)
 game = Game(wn, sample(WORDS, 5), 60, 120)
 score_font = SysFont("Californian FB", 40)
 score = Score(wn, (60, 50), score_font)
 running = True
 while running:
 wn.fill(bg_color)
 for event in pygame.event.get():
 if event.type == pygame.QUIT:
 running = False
 elif event.type == pygame.MOUSEBUTTONDOWN:
 for in_box in game.in_boxes:
 if in_box.over(*event.pos) and not in_box.disabled:
 game.active_in_box = in_box
 break
 elif event.type == pygame.MOUSEMOTION:
 if game.active_in_box:
 for out_box in game.out_boxes:
 if out_box.over(*event.pos) and not out_box.disabled:
 game.active_out_box = out_box
 break
 else:
 game.active_out_box = None
 elif event.type == pygame.MOUSEBUTTONUP:
 if game.active_in_box and game.active_out_box:
 if game.active_out_box.word == game.active_in_box.word:
 game.active_out_box.disable()
 game.active_in_box.disable()
 score.score_up()
 if game.won():
 game = Game(wn, sample(WORDS, 5), 60, 120)
 else:
 score.score_down()
 game.active_out_box = game.active_in_box = None
 game.draw_lines(event)
 game.draw_boxes()
 score.draw()
 pygame.display.update()
 pygame.quit()
if __name__ == '__main__':
 main()

• Peilonrayz: There's a certain irony that you replaced Microsoft YaHei UI, available on Windows 7+, with a font I don't have on Windows. Whilst leaving Californian FB, another Microsoft font which isn't listed as installed on any of Windows 7-10, alone.
• Reinderien: @Peilonrayz Enjoy the irony if you want; the fact is that this is non-portable, and it's only a coincidence that you have a compatible font. The general solution is to avoid hard-coding a font name altogether and inspect the font for glyph presence.
• Peilonrayz: I don't really see why I'd enjoy the irony. I have a compatible font just like 75% of other desktop users will, where your answer has made the code less portable as you have an MS font and a non-MS font so now one font will always be missing. There is a simpler way to fix this and that is just to provide the fonts with any other assets. Such as what is done on most websites, package managers and installers.