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:

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.

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.

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()