Snipe Last Chance
I can squeeze in one more session before tonight’s demo. Can I add in two features and improve the code? Also: Resist fascism, and take care of one another.
We need to display the first roll to the left of the mark box, and to display either the second roll, or a mark, in the mark box. Let’s see if we can do it.
I could have done the total score with a test. Let’s do these two little features with tests.
def test_mark(self):
s = Scorekeeper()
s.roll(10)
s.roll(6)
s.roll(4)
s.roll(3)
s.roll(2)
assert s.frame(0).mark() == 'X'
assert s.frame(1).mark() == '/'
assert s.frame(2).mark() == '2'
assert s.frame(3).mark() == ''
All this calls for just one new method, mark. I’ll use the try/except approach to deal with missing information:
class Frame:
def mark(self):
try:
return self._mark()
except Exception:
return ''
def _mark(self):
s = self.start
rolls = self.rolls
if rolls[s] == 10:
return 'X'
elif rolls[s] + rolls[s+1] == 10:
return '/'
else:
return str(rolls[s+1])
Green. Commit: Frame can return mark string.
Let’s plug that into our NormalFrameView object.
def draw(self, game):
...
self.draw_frame_total(game)
self.draw_mark(game)
def draw_mark(self, game):
mark = self.frame.mark()
font = pygame.font.Font(game.BASE_FONT, 20)
font.set_bold(True)
text = font.render(mark, True, game.FOREGROUND, game.BACKGROUND)
text_rect = text.get_rect()
frame_leftx = self.f * game.FRAME_SIZE
text_rect.centerx = frame_leftx + game.FRAME_SIZE//6*5
text_rect.centery = game.GAME_Y//6
game.screen.blit(text, text_rect)
That gives me this very pleasing result:
I am not displeased at all.
At this point I decided to check the rules, to see what the area is called where you put the first roll if it is not a strike, and I discovered that I have been working from a mistaken layout for the score sheet. The scorecard shows two boxes for the first nine frames and three for the tenth. We should have no problem fixing that up.
Let’s TDD out the first roll indicator. That should be a number, unless the first roll is ten, in which case we’ll return an X (and then ignore it in the view). I’m guessing that we don’t want to return a blank because the TenthFrameView will handle things differently when we get there.
def first_roll(self):
try:
return self._first_roll()
except Exception:
return ''
def _first_roll(self):
s = self.start
if (r := self.rolls[s]) == 10:
return 'X'
else:
return str(r)
Green. Commit: computing first roll string.
Now to display it:
def draw_first_roll(self, game):
mark = self.frame.first_roll()
font = pygame.font.Font(game.BASE_FONT, 20)
font.set_bold(True)
text = font.render(mark, True, game.FOREGROUND, game.BACKGROUND)
text_rect = text.get_rect()
frame_leftx = self.f * game.FRAME_SIZE
text_rect.centerx = frame_leftx + game.FRAME_SIZE//6*3
text_rect.centery = game.GAME_Y//6
game.screen.blit(text, text_rect)
I basically copied and pasted that from draw_mark, substituting the call to first_roll and adjusting the calculation of centerx to be 3/6 of the way across, not 5. It looks right:
Now we have two things left to do, I think. We have to make the TenthFrameView do the right thing, and, time permitting, we should draw the additional square for the other frames.
Let’s see. I think TenthFrameView needs to be like this:
Oops, got ahead of myself. In the Normal views if first_roll is X we shouldn’t display it.
def draw_first_roll(self, game):
mark = self.frame.first_roll()
if mark == 'X':
return
...
That works. Commit: do not display first roll in first nine frames if it is a strike.
I copy/paste and edit the method from the NormalFrameView. I’ll spare you the listing. Now for the mark, which should go into the second box … ah, not so fast. What should go into the second box is the second roll, showing X if it is ten. Let’s settle down here and TDD those two boxes.
def test_tenth_frame(self):
s = Scorekeeper()
s.roll(1)
s.roll(2)
s.roll(3)
f = s.frame(0)
assert f.tenth_frame(0) == '1'
assert f.tenth_frame(1) == '2'
assert f.tenth_frame(2) == '3'
I’m testing the initial frame but all frame instances are alike, so this is legit and saves me generating a bunch of confusing leading frames. I am planning to have one method handle all thee positions:
I code:
class Frame:
def tenth_frame(self, n):
try:
return self._tenth_frame(n)
except Exception:
return ''
def _tenth_frame(self, n):
index = self.start + n
roll = self.rolls[index]
if roll == 10:
return 'X'
else:
return str(roll)
Test passes. Commit: tenth_frame string method provided.
Now let’s try to use it in TenthFrameView.
I’ve done what I foresaw, but it isn’t quite right:
class Game:
def draw(self, game):
x = 10 * game.FRAME_SIZE
mark_line = game.MARK_LINE
pygame.draw.line(game.screen, game.WHITE,
(x, 0), (x, game.GAME_Y))
pygame.draw.line(game.screen, game.WHITE,
(x, mark_line), (x - 3 * mark_line, mark_line))
pygame.draw.line(game.screen, game.WHITE,
(x - mark_line, mark_line), (x - mark_line, 0))
pygame.draw.line(game.screen, game.WHITE,
(x - 2 * mark_line, mark_line), (x - 2 * mark_line, 0))
pygame.draw.line(game.screen, game.WHITE,
(x - game.FRAME_SIZE, game.GAME_Y), (x, game.GAME_Y))
self.draw_frame_total(game)
self.draw_tenth_frame(game, 0)
self.draw_tenth_frame(game, 1)
self.draw_tenth_frame(game, 2)
def draw_tenth_frame(self, game, n):
mark = self.frame.tenth_frame(n)
step = 2*n + 1
font = pygame.font.Font(game.BASE_FONT, 20)
font.set_bold(True)
text = font.render(mark, True, game.FOREGROUND, game.BACKGROUND)
text_rect = text.get_rect()
frame_leftx = 9 * game.FRAME_SIZE
text_rect.centerx = frame_leftx + game.FRAME_SIZE//6*step
text_rect.centery = game.GAME_Y//6
game.screen.blit(text, text_rect)
This will work well, unless we throw a spare in the tenth frame, in which case we get this:
That means that my tenth_frame method has to be smarter, and it was perhaps a bad idea to try to handle all three rolls in one method.
Let’s see if we can hack it to work.
I do hack it and it doesn’t work, so I write the test that I should have written:
def test_tenth_middle(self):
s = Scorekeeper()
s.roll(7)
s.roll(3)
f = s.frame(0)
assert f.tenth_frame(1) == '/'
And that works. What have I done wrong? I quickly find the defect, and my test did not detect it:
class Frame:
def _tenth_frame(self, n):
index = self.start + n
roll = self.rolls[index]
if roll == 10:
return 'X'
elif n == 1:
if roll + self.rolls[self.start] == 10:
return '/'
else:
return str(roll)
else:
return str(roll)
That check for 10 was using self.rolls[0], not self.rolls[self.start]. Let me write a test that will find the bug.
def test_tenth_middle(self):
s = Scorekeeper()
s.roll(0)
s.roll(0)
s.roll(7)
s.roll(3)
f = s.frame(1)
assert f.tenth_frame(1) == '/'
That finds the bug and we fix it. Commit: game working fully as advertised.
Here is a picture of the alternating game:
Let’s reflect and see if we can recover from what we observe.
Reflection
As discussed last time, this code suffers from having been rushed, and from not having had much attention paid to cleaning it up.
Let’s start where we finished with the tenth_frame method.
class Frame:
def tenth_frame(self, n):
try:
return self._tenth_frame(n)
except Exception:
return ''
def _tenth_frame(self, n):
index = self.start + n
roll = self.rolls[index]
if roll == 10:
return 'X'
elif n == 1:
if roll + self.rolls[self.start] == 10:
return '/'
else:
return str(roll)
else:
return str(roll)
The idea here was to save time and code by writing one function. I thought that the three cells in the tenth frame were all the same, but in fact they are not: the middle one is special. Let’s see if we can improve this in place, by increasing duplication:
class Frame:
def tenth_frame(self, n):
try:
return self._tenth_frame(n)
except Exception:
return ''
def _tenth_frame(self, n):
index = self.start + n
roll = self.rolls[index]
if n == 1:
return self.middle_cell(roll)
else:
return self.outer_cells(roll)
def middle_cell(self, roll):
if self.spare(roll):
return '/'
else:
return str(roll)
def spare(self, roll):
return roll + self.rolls[self.start] == 10
def outer_cells(self, roll):
if self.strike(roll):
return 'X'
else:
return str(roll)
def strike(self, roll):
return roll == 10
PyCharm found places to use the strike method. Let’s see a few of them:
class Frame:
def next_start(self):
if self.start is None:
return None
first_roll = self.rolls[self.start]
desired_rolls = 1 if self.strike(first_roll) else 2
if self.start + desired_rolls < len(self.rolls):
return self.start + desired_rolls
return None
def _frame_score(self):
s = self.start
rolls = self.rolls
first_roll = rolls[s]
if self.strike(first_roll):
return 10 + rolls[s+1] + rolls[s+2]
second_roll = rolls[s+1]
if (two := first_roll + second_roll) != 10:
return two
return two + rolls[s+2]
def _first_roll(self):
s = self.start
if self.strike((r := self.rolls[s])):
return 'X'
else:
return str(r)
I like strike OK, but the spare method isn’t so usable as the strike one. I might return to work on that but for now let’s just rename it a bit:
def middle_cell(self, roll):
if self.tenth_frame_spare(roll):
return '/'
else:
return str(roll)
def tenth_frame_spare(self, roll):
return roll + self.rolls[self.start] == 10
I think that Frame is in decent shape. I am less pleased with the graphics code, which has lots of duplication in it, for example:
class NormalFrameView:
def __init__(self, frame_number, frame: Frame):
self.f = frame_number
self.frame = frame
def draw(self, game):
x_left = self.f * game.FRAME_SIZE
x_right = x_left + game.FRAME_SIZE
mark_line = game.MARK_LINE
pygame.draw.line(game.screen, game.WHITE,
(x_right, 0), (x_right, game.GAME_Y))
pygame.draw.line(game.screen, game.WHITE,
(x_right - mark_line, mark_line), (x_right , mark_line))
pygame.draw.line(game.screen, game.WHITE,
(x_right - mark_line, mark_line), (x_right - mark_line, 0))
pygame.draw.line(game.screen, game.WHITE,
(x_left, game.GAME_Y), (x_right, game.GAME_Y))
self.draw_frame_total(game)
self.draw_first_roll(game)
self.draw_mark(game)
I think there might be a multi-line drawing thing in pygame, could we use that here? Could we at least pull out some of the duplication? How about this:
def draw(self, game):
x_left = self.f * game.FRAME_SIZE
x_right = x_left + game.FRAME_SIZE
mark_line = game.MARK_LINE
self.white_line((x_right, 0), (x_right, game.GAME_Y))
self.white_line((x_right , mark_line), (x_right - mark_line, mark_line) )
self.white_line((x_right - mark_line, mark_line), (x_right - mark_line, 0))
self.white_line((x_left, game.GAME_Y), (x_right, game.GAME_Y))
self.draw_frame_total(game)
self.draw_first_roll(game)
self.draw_mark(game)
def white_line(self, start, end):
pygame.draw.line(game.screen, game.WHITE, start, end)
I think we can do better with this:
def draw(self, game):
x_left = self.f * game.FRAME_SIZE
x_right = x_left + game.FRAME_SIZE
mark_line = game.MARK_LINE
self.white_lines([(x_right, 0), (x_right, game.GAME_Y)])
self.white_lines([(x_right , mark_line), (x_right - mark_line, mark_line), (x_right - mark_line, 0)])
self.white_lines([(x_left, game.GAME_Y), (x_right, game.GAME_Y)])
self.draw_frame_total(game)
self.draw_first_roll(game)
self.draw_mark(game)
def white_lines(self, lines):
pygame.draw.lines(game.screen, game.WHITE, False, lines)
Let’s put that in place elsewhere.
class TenthFrameView:
def draw(self, game):
x = 10 * game.FRAME_SIZE
mark_line = game.MARK_LINE
self.white_lines([(x, 0), (x, game.GAME_Y)])
self.white_lines([(x, mark_line), (x - 3 * mark_line, mark_line)])
self.white_lines([(x - mark_line, mark_line), (x - mark_line, 0)])
self.white_lines([(x - 2 * mark_line, mark_line), (x - 2 * mark_line, 0)])
self.white_lines([(x - game.FRAME_SIZE, game.GAME_Y), (x, game.GAME_Y)])
self.draw_frame_total(game)
self.draw_tenth_frame(game, 0)
self.draw_tenth_frame(game, 1)
self.draw_tenth_frame(game, 2)
def white_lines(self, lines):
pygame.draw.lines(game.screen, game.WHITE, False, lines)
Not as much savings there but a bit of duplication removed. A bit of cleverness might let us batch some of those, but that’ll do for now.
Summary
- Added in Post:
-
I was removing some more duplication and discovered that the tenth frame display may not be just right. I have to do some research to decide what the rules are. Anyway, it’s clearly getting what I asked it to get, and the total score is right.
We’ll skip the extra box: it’s easy enough to do but we’ve done enough overtime.
I think I’ve shown a good-faith effort to clean up a bit. We’ll see what the lads say tonight!
See you next time, and remember: resist fascism, and take care of one another.