Sudoku Take 2 Review for Bluenix #7
Conversation
Second commit for review
… cog loading code (lines 383-385)
|
|
||
| def generate_puzzle(self): | ||
| """Generates a new puzzle and solves it.""" | ||
| self.generate_solution(self.grid) |
There was a problem hiding this comment.
You still have to decide! Either you don't pass grid around and use self.grid or you remove self.grid. Don't do both 😄
I think you should turn them into classmethods and keep passing grid around.
There was a problem hiding this comment.
Yeah, passing grid around would probably be easier. Will fix
| await self.start(ctx) | ||
| await self.bot.wait_for(event="message") | ||
|
|
||
| while game: | ||
| await asyncio.sleep(1) | ||
| current_time = time.time() | ||
| time_elapsed = current_time - self.started_at | ||
| formatted_time = self.time_convert(time_elapsed) | ||
| timer_message.description = formatted_time | ||
| await send_timer.edit(embed=timer_message) | ||
|
|
||
| # if coord and digit.isnumeric() and -1 < int(digit) < 10 or digit in "xX": | ||
| # # print(f"{coord=}, {digit=}") | ||
| # await game.update_board(digit, coord) | ||
| # else: | ||
| # raise commands.BadArgument | ||
|
|
||
| # while game is in progress: | ||
| # print(timer_message) |
There was a problem hiding this comment.
You could move this out of the command into a helper method to separate the game running and the actual command.
There was a problem hiding this comment.
What do you mean by "helper method"?
There was a problem hiding this comment.
A method somewhere on the game/cog. The "helper" part just describes its purpose (to help other functionality, on its own it doesn't tell the full story).
| while True: | ||
| x, y = random.randint(0, 5), random.randint(0, 5) | ||
| if game.puzzle[x][y] == 0: |
There was a problem hiding this comment.
Because of chance, this could go on for a long time and cause a lot of CPU usage. Instead, why not find all empty spots beforehand and random.choice() those?
There was a problem hiding this comment.
So what you're suggesting is to make a 2D list of the coordinates of all empty spots (we could call it empty_coords_list for example) and random.choice(empty_coords_list)?
There was a problem hiding this comment.
So like this?
@sudoku.command()
async def hint(self, ctx: commands.Context) -> None:
"""Fill in one empty square on the Sudoku board."""
game = self.games.get(ctx.author.id)
if game:
game.hints.append(time.time())
while True:
x, y = random.randint(0, 5), random.randint(0, 5)
empty_coords_list = []
while game.puzzle[x][y] == 0:
empty_coords_list.append((0, 0))
await game.update_board(digit=random.chance(empty_coords_list), coord=(x, y))
breakThere was a problem hiding this comment.
I don't see how that changed anything. Loop over all cells and remember the ones who's values are 0 (not coordinates are 0). Lastly, randomly pick one of the cell's with a value of zero from earlier.
| @discord.ui.button(style=discord.ButtonStyle.green, label="Hint") | ||
| async def hint_button(self, *_) -> None: | ||
| """Button that fills in one empty square on the Sudoku board.""" | ||
| await self.ctx.invoke(self.ctx.bot.get_command("sudoku hint")) | ||
|
|
||
| @discord.ui.button(style=discord.ButtonStyle.primary, label="Game Info") | ||
| async def info_button(self, *_) -> None: | ||
| """Button that displays information about the current game.""" | ||
| await self.ctx.invoke(self.ctx.bot.get_command("sudoku info")) | ||
|
|
||
| @discord.ui.button(style=discord.ButtonStyle.red, label="End Game") | ||
| async def end_button(self, *_) -> None: | ||
| """Button that ends the current game.""" | ||
| await self.ctx.invoke(self.ctx.bot.get_command("sudoku finish")) |
There was a problem hiding this comment.
This works, but just an idea: why not pass the cog to SudokuView so that you can just call the methods?
I wouldn't recommend doing it like this though, separate the stuff you need to do into methods which this calls. Commands are distinctively somewhat different.
There was a problem hiding this comment.
separate the stuff you need to do into methods which this calls
What do you mean by this?
There was a problem hiding this comment.
If you need to have functionality like "write number", then have a class which only plays the game and has a method like write_number() rather than you implementing this into the view and command.
No description provided.