Conversation
Use tk.destroy() function to close down the window at end of a game when you say you don't want to continue. The quit() function was leaving window open and causing error next time program called.
Add flag to choose to set specific number of mines. If set, will generate correct size list of mines as part of setup.
ripexz
left a comment
ripexz
left a comment
There was a problem hiding this comment.
Thanks for your contribution - I thought I had left a review a while back but I never submitted it and just had the comments sitting there 😅
There's a few small tweaks/improvements to be made, but overall I like the approach and looks like indeed there is a bug with closing the window on python3 - I initially wrote this for python 2 so it's most likely popped up since then 😊
| To Do: | ||
| ---------- | ||
| - Have specific number of mines, rather than random | ||
| - ~~Have specific number of mines, rather than random~~ |
There was a problem hiding this comment.
You can just remove this line entirely
| #if fixed number, generate random list of mines | ||
| #for simplicity generate set of random numbers going up to x*y size | ||
| #will then compare with mine number, counting across row then row by row | ||
| mine_list = random.sample(range(SIZE_X*SIZE_Y-1),MINES_TO_SET) |
There was a problem hiding this comment.
Add a space after the comma for consistent formatting, please
| self.restart() | ||
| else: | ||
| self.tk.quit() | ||
| self.tk.destroy() #shut down the window |
There was a problem hiding this comment.
This will throw errors on quit due to updateTimer running in after, I think wrapping this in an after also should resolve it, i.e. self.tk.after(100, self.tk.destroy) although I haven't had the chance to test it yet.
| else: | ||
| self.tk.quit() | ||
| self.tk.destroy() #shut down the window | ||
| #self.tk.quit() |
There was a problem hiding this comment.
no need to keep commented-out lines, please remove
| #count which square number we're on and see if in mine list | ||
| isMine = True | ||
| self.mines += 1 | ||
|
|
There was a problem hiding this comment.
Remove this blank line just so the if-else block is grouped visually
| SIZE_X = 10 | ||
| SIZE_Y = 10 | ||
|
|
||
| FIXED_MINE_NUMBER = True #if True set a fixed number of mines, rather than a random outcome |
There was a problem hiding this comment.
might be nice to add argument parser and set this to False if run with --random 🤔
2 participants
Hello. I don't know if you're still working on / maintaining your minesweeper program?
I found it yesterday and was having a play - thanks for sharing/making public.
I saw you had a ToDo to set a fixed number of mines - so I've added functionality to do just that.
I also fixed a bug - on my system at least - where the window didn't close when the game ends and you say no you don't want to continue -- which then causes and error next time you run.
I'm quite new to Git (and Python) and have never done a pull request before, so hope I'm doing this right....
Ashley