2
\$\begingroup\$

I'm trying to recreate minesweeper in python with PyQT5. I have this function that updates the GUI by using nested for loops (one for column, and one for row) to check each cell in a 19x19 area to determine what type of tile said cell is.

The function works as intended, but runs very slow and takes around 4 seconds to complete. I have tried using threading, but this further harms performance.

Here is the code:
(I know it's not the best, it's still a prototype)

def update_display(self):
                if(Debug == 1):
                                print("[Updating Cells...]")
                            
                for column in range(self.cells):
                    #print(column)
                    for row in range(self.cells):
                        #print(row)
                        #if(Debug == 1):
                        #        print("[Changing tile ({},{})]".format(row,column))
                        if self.initialArray[row][column] <= 9:
                            self.button[row, column].setIcon(QtGui.QIcon(dataFolder + 'tile_plain.gif'))
                        elif self.initialArray[row][column] == 10:
                            self.button[row, column].setIcon(QtGui.QIcon(dataFolder + 'tile_clicked.gif'))
                        elif self.initialArray[row][column] == 11:
                            self.button[row, column].setIcon(QtGui.QIcon(dataFolder + 'tile_1.gif'))
                        elif self.initialArray[row][column] == 12:
                            self.button[row, column].setIcon(QtGui.QIcon(dataFolder + 'tile_2.gif'))
                        elif self.initialArray[row][column] == 13:
                            self.button[row, column].setIcon(QtGui.QIcon(dataFolder + 'tile_3.gif'))
                        elif self.initialArray[row][column] == 14:
                            self.button[row, column].setIcon(QtGui.QIcon(dataFolder + 'tile_4.gif'))
                        elif self.initialArray[row][column] == 15:
                            self.button[row, column].setIcon(QtGui.QIcon(dataFolder + 'tile_5.gif'))
                        elif self.initialArray[row][column] == 16:
                            self.button[row, column].setIcon(QtGui.QIcon(dataFolder + 'tile_6.gif'))
                        elif self.initialArray[row][column] == 19:
                            self.button[row, column].setIcon(QtGui.QIcon(dataFolder + 'tile_flag.gif'))
                        elif self.initialArray[row][column] == 29:
                            self.button[row, column].setIcon(QtGui.QIcon(dataFolder + 'tile_mine.gif'))

Is there a faster way to do this? (Preferably < 1 second as it needs to draw each cell)

If it helps any, I am using Python 3.10

\$\endgroup\$
2
  • 2
    \$\begingroup\$ That's a bit better, but "nested for loops" is still somewhat vague and not quite what your code is about. Your problem isn't the nested for loops or the 10 if/else that should be dataFile[ self.initialArray[row][column] ]. Your problem is calling setIcon(QtGui.QIcon(path)) 389 times. I don't know enough about QTGui or your code's setIcon to help further than that. \$\endgroup\$
    – Teepeemm
    Commented Dec 20, 2021 at 16:48
  • \$\begingroup\$ There isn't enough code here to help you optimize the solution. The problem may not be in this function. \$\endgroup\$
    – pacmaninbw
    Commented Dec 21, 2021 at 13:51

1 Answer 1

2
\$\begingroup\$

I have a hard time believing that your loop is taking 4 seconds. If you're playing a reasonable minesweeper game, even at 100x100, that's just far too long.

Obviously, then, you must either be playing an unreasonably-sized game, or there's a performance problem in the Qt parts of your code. ;-)

Assuming the problem lies in the individual cases, the answer is two-fold:

  1. First, stop checking all those cases!

  2. Second, don't repeat work inside the cases.

Stop checking all the cases

Instead, use some kind of case to result mapping that you can look up at O(1). In this case, even with some holes in the indices, I'd recommend a list.

The simplest list (not recommended) would be a mapping of index values to file names:

Tile_files = [ 'tile_plain.gif' ] * 10 \
             + ...

With that you could look up the integer value and get either None or a string result. This would eliminate all the if / then / else redundancy from your code.

Don't repeat work

However, even with using a map to convert from sequential if statements, you're still repeating computations for each cell. At some level, you should be able to stop doing that without causing a problem. I think that level is the the icon image. Your buttons all have to be separate buttons, because you are using the buttons to encode the identity of the cells. But the images associated with the buttons can be shared.

So share them! Do the computation of dataFolder + file one time. Convert the path to an icon one time. Cache the result!

Instead of a mapping of tile # -> file path, let the mapping be from tile # -> icon image.

\$\endgroup\$
0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.