Those Coordinates
That
make_linesmethod is horrid. Let’s continue cleaning things up a bit, starting there.
The current border-drawing code is this:
def draw_border(self, cell, screen):
borders: BorderList = self.layout.get_borders(cell)
lines = self.make_lines(cell)
for border in borders:
start, finish = lines[border.side]
color = border.select_boundary(self.boundary_colors)
pygame.draw.line(screen, color, start, finish)
def make_lines(self, cell):
cx0 = cell.x * cell_size
cy0 = cell.y * cell_size
cx1 = cx0 + cell_size
cy1 = cy0 + cell_size
bottom = ((cx0, cy1), (cx1, cy1))
top = ((cx0, cy0), (cx1, cy0))
left = ((cx0, cy0), (cx0, cy1))
right = ((cx1, cy0), (cx1, cy1))
lines = {(0, 1): bottom, (0, -1): top, (-1, 0): left, (1, 0): right}
return lines
We’re doing that calculation on every drawing of every cell, and while I’m not really all that concerned about the computation time, that whole method bugs me. I spent some time last night trying to come up with a formula from the sides (1,0) for east, and so on, to the start and finish offsets for drawing the border lines. I even drew pictures:
I even wrote some Juno Python code to see if there was a simple algorithm. There sort of is, but it is entirely ad hoc and cryptic. I’ve decided just to store the base offsets we need, multiply in the scale, and add in the cell’s own offset.
I think we can sort of refactor the code we have to do what we need. First extract a variable for the border side:
def draw_border(self, cell, screen):
borders: BorderList = self.layout.get_borders(cell)
lines = self.make_lines(cell)
for border in borders:
side = border.side
start, finish = lines[side]
color = border.select_boundary(self.boundary_colors)
pygame.draw.line(screen, color, start, finish)
No, belay that. I’d like to be able to commit this code a few times. But we have no real tests for it. Also, looking at the code a little more carefully, it seems that we’ve started making the Border class a bit more intelligent, with the select_boundary code. Let’s do as much of our work as we can on Border, not in RoomView. We can test. I’ll sketch the code that I want here, but then TDD it.
def draw_border(self, cell, screen):
borders: BorderList = self.layout.get_borders(cell)
for border in borders:
start, finish = border.line_coordinates(cell.xy, cell_size)
color = border.select_boundary(self.boundary_colors)
pygame.draw.line(screen, color, start, finish)
Wishful Thinking for a method line_coordinates on Border. Let’s write some tests for that.
def test_line_coordinates(self):
west_border = Border((-1,0), 'partition')
cell_xy = (10, 20)
cell_size = 12
start, finish = west_border.line_coordinates(cell_xy, cell_size)
assert start == (120, 240)
assert finish == (120, 252)
I think that’s right. Western border offset starts at (0,0) and goes to (0,s), where s is the size. Add in the coordinates and that’s what we get. Probably.
Now some code:
@dataclass
class Border:
side: tuple
boundary: str
def _offsets(self):
return {
(-1, 0): ((0,0), (0,1))
}
def line_coordinates(self, cell_xy, cell_size):
so, fo = self._offsets()[self.side]
start = self.adjust(so, cell_xy, cell_size)
finish = self.adjust(fo, cell_xy, cell_size)
return start, finish
def adjust(self, offset, coord, size):
xo, yo = offset
xc, yc = coord
xa = xc*size + xo*size
ya = yc*size + yo*size
return (xa, ya)
The core idea is that we’ll just look up the darn offset, since I’ve not found a way to compute them that is at all clear. Then we “just” do the vector arithmetic in adjust and we have the answer. Life would be better if our Cell coordinates behaved like vectors. The Cell itself does vaguely understand vector_diff, but we don’t want to overburden Cell, nor to pass a Cell into this rather simple object.
We’ll let that ride, and do the test of the test and dictionary. I’ll try to be careful, and I do have that weird picture to look at.
def test_west_line_coordinates(self):
west_border = Border((-1,0), 'partition')
cell_xy = (10, 20)
cell_size = 12
start, finish = west_border.line_coordinates(cell_xy, cell_size)
assert start == (120, 240)
assert finish == (120, 252)
def test_east_line_coordinates(self):
west_border = Border((1,0), 'partition')
cell_xy = (10, 20)
cell_size = 12
start, finish = west_border.line_coordinates(cell_xy, cell_size)
assert start == (132, 240)
assert finish == (132, 252)
def test_north_line_coordinates(self):
west_border = Border((0,-1), 'partition')
cell_xy = (10, 20)
cell_size = 12
start, finish = west_border.line_coordinates(cell_xy, cell_size)
assert start == (120, 240)
assert finish == (132, 240)
def test_south_line_coordinates(self):
west_border = Border((0, 1), 'partition')
cell_xy = (10, 20)
cell_size = 12
start, finish = west_border.line_coordinates(cell_xy, cell_size)
assert start == (120, 252)
assert finish == (132, 252)
And the dictionary:
def _offsets(self):
return {
(-1, 0): ((0,0), (0,1)),
( 1, 0): ((1,0), (1,1)),
( 0, -1): ((0, 0), (1, 0)),
( 0, 1): ((0, 1), (1, 1)),
}
We are green. Does main work? In fact it does.
Commit: Border now computes border coordinates.
We’ve made it work. Now let’s see if we can make it right. We would like to make that dictionary into a class variable, which is possible with dataclass but weird. We’ll try what it says on the box. With that and a bit more tidying, we have this for Border:
@dataclass
class Border:
side: tuple
boundary: str
_offsets: ClassVar[dict] = {
(-1, 0): ((0,0), (0,1)),
( 1, 0): ((1,0), (1,1)),
( 0, -1): ((0, 0), (1, 0)),
( 0, 1): ((0, 1), (1, 1)),
}
def select_boundary(self, a_dictionary):
return a_dictionary[self.boundary]
def line_coordinates(self, cell_xy, cell_size):
so, fo = self._offsets[self.side]
start = self.adjust(so, cell_xy, cell_size)
finish = self.adjust(fo, cell_xy, cell_size)
return start, finish
@staticmethod
def adjust(offset, coord, size):
xo, yo = offset
xc, yc = coord
xa = xc*size + xo*size
ya = yc*size + yo*size
return xa, ya
Not bad at all. Commit: tidying. Would we like better names in adjust? OK, you talked me into it.
This is your fault if it isn’t better. I did kind of go wild:
@dataclass
class Border:
side: tuple
boundary: str
border_offsets: ClassVar[dict] = {
(-1, 0): ((0, 0), (0, 1)),
( 1, 0): ((1, 0), (1, 1)),
( 0, -1): ((0, 0), (1, 0)),
( 0, 1): ((0, 1), (1, 1)),
}
def select_boundary(self, a_dictionary):
return a_dictionary[self.boundary]
def line_coordinates(self, cell_xy, cell_size):
start_offset, finish_offset = self.border_offsets[self.side]
start = self.scale_to_size(start_offset, cell_xy, cell_size)
finish = self.scale_to_size(finish_offset, cell_xy, cell_size)
return start, finish
@staticmethod
def scale_to_size(offset, cell_coordinates, cell_size):
x_cell, y_cell = cell_coordinates
x_offset, y_offset = offset
x_scaled = x_cell * cell_size + x_offset * cell_size
y_scaled = y_cell * cell_size + y_offset * cell_size
return x_scaled, y_scaled
I do think that’s better, and unless we slip a vector class under Cell, it’s about as good as we can get.
Commit. Let’s sum up and get out of here.
Summary
We set out to improve the border calculations a few sessions ago. We now have three little classes, Border, BorderList, and BorderMap that let us store the border information for all the cells in all the rooms. The actual drawing code is now pretty clear:
class RoomView:
def draw_border(self, cell, screen):
borders: BorderList = self.layout.get_borders(cell)
for border in borders:
start, finish = border.line_coordinates(cell.xy, cell_size)
color = border.select_boundary(self.boundary_colors)
pygame.draw.line(screen, color, start, finish)
That was quite nasty before, and now we defer some of the work over to the Border and our code is much simpler here, and pretty simple in Border as well. I have mixed feelings about the border_offsets dictionary, but it is clear enough and probably no slower than a direct calculation would have been, if I had been able to devise a decent one, which I was not.
We might take one more look around next time, but I think we have just about reached the point where we should start experimenting with Arcade, to see whether we’d like to convert to it before making more of a game out of this thing.
Unless I get a more interesting idea. We’ll all find out next time. See you then!