Asset Improvement (cd)
Today I propose to change the Border code to return a ‘meaningful’ string rather than those 0-15 integers. I figure I can handle something that tricky, probably.
The core notion that I have in mind is to have a cell’s border described as a string, with the letters ENWS, in that order, if the cell has a border on that side. I chose that order because I count the sides in the conventional angular direction, starting with east. I think we’ll have some visible prefix, perhaps just _, depending on how it seems when we get there.
That will let me recast the texture dictionary, which now looks like this:
texture_dictionary = {
0: arcade.load_texture(path+'Tile (21).png'), # none
1: arcade.load_texture(path+'Floor_Edge_29.png'), # east
2: arcade.load_texture(path+'Floor_Edge_36.png'), # north
...
We’ll have the much more meaningful string keys, which should help us as we decide what tiles to put where.
There may be more to do, because we may want to provide for multiple tiles of a given type, to be chosen randomly. We’ll worry about that later: refactoring is our middle name.
So, to the borders. I think we’ll strip down the border-related classes as we work, if we find that there is still code left in there from the older line-drawing border notion.
The BorderMap just creates borders for all the cells in all the rooms:
class BorderMap:
def __init__(self, layout):
self.borders = {}
for room in layout.rooms:
for cell in room:
borders = BorderList(layout, cell)
self.borders[cell] = borders
def __getitem__(self, cell):
return self.borders[cell]
The BorderList, pertains to a single cell, and it is related somewhat obscurely to our 0-15 value, which is what we want. I think we have no use for the BorderList outside this class. In essence, we want self.borders[cell] to return our string result, ‘EW’ and so on. We’ll continue to see what we have here that relates to what we actually want.
class BorderList:
def __init__(self, layout, cell):
self.borders = []
my_room = layout.get_room(cell)
for side in [(1,0), (-1,0), (0, -1), (0,1)]:
neighbor = layout.at_offset(cell.xy, side)
neighbor_room = layout.get_room(neighbor) if neighbor else None
border = my_room.border_type(neighbor_room)
self.borders.append(Border(side, border))
def border_index(self):
return sum ([border.index() for border in self.borders])
Ah, right. Here we create a Border instance, which we later sum to get our 0-15 values. What is Border now?
@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)),
}
border_indexes: ClassVar[dict] = {
(1,0): 1,
(0,1): 2,
(-1,0): 4,
(0,-1): 8
}
def index(self):
if self.boundary == 'open':
return 0
else:
return self.border_indexes[self.side]
def select_boundary(self, a_dictionary):
...
def line_coordinates(self, cell_xy, cell_size):
...
@staticmethod
def scale_to_size(offset, cell_coordinates, cell_size):
...
Yeah, no. Way too much mechanism here. We have a bunch of tests for all this. Let’s go over there, rip out the tests we no longer need and write one or more new ones.
There were four tests checking correctness for those calculations for the coordinates of the lines that we no longer use. Tests, removed plus the three dotted methods above. Commit: refactoring borders.
Some of the other tests are informative, somewhat to my surprise: I expected we’d find most of them useless. But no, the Borders object identify more than just that there needs to be a border. The Border object records the border type, which can be ‘wall’, ‘open’, ‘partition’ or, in principle, ‘door’.
We aren’t really using that information, and it is in any case computed in Room:
class Room:
def border_type(self, neighbor_room):
if neighbor_room is None:
return 'wall'
elif neighbor_room == self:
return 'open'
else:
return 'partition'
I don’t think we actually use the Border and BorderList much. In actual production code, we only use it to get the 0-15 that we’re trying to replace.
Let’s TDD a new method border_string, and then improve things.
Ah, here’s a test we can repurpose:
def test_border_indexes(self):
borders = []
border = Border((1,0), 'wall')
borders.append(border)
assert border.index() == 1
border = Border((0,1), 'wall')
borders.append(border)
assert border.index() == 2
border = Border((-1,0), 'wall')
borders.append(border)
assert border.index() == 4
border = Border((0,-1), 'wall')
borders.append(border)
assert border.index() == 8
border_list = TestBorderList(borders)
assert border_list.border_index() == 15
We don’t want the index at all, so first let’s change the results:
def test_border_strings(self):
borders = []
border = Border((1,0), 'wall')
borders.append(border)
assert border.index() == 'E'
border = Border((0,1), 'wall')
borders.append(border)
assert border.index() == 'N'
border = Border((-1,0), 'wall')
borders.append(border)
assert border.index() == 'W'
border = Border((0,-1), 'wall')
borders.append(border)
assert border.index() == 'S'
border_list = TestBorderList(borders)
assert border_list.border_index() == 'ENWS'
Fails, of course but we can fix Border.
@dataclass
class Border:
side: tuple
boundary: str
border_indexes: ClassVar[dict] = {
(1,0): 'E',
(0,1): 'N',
(-1,0): 'W',
(0,-1): "S"
}
def index(self):
if self.boundary == 'open':
return ''
else:
return self.border_indexes[self.side]
And in BorderList, we change:
def border_index(self):
return sum ([border.index() for border in self.borders])
To this:
def border_index(self):
strings = [border.index() for border in self.borders]
return ''.join(strings)
Green. Tempted to commit but main is broken. We can fix it quickly.
texture_dictionary = {
'': arcade.load_texture(path+'Tile (21).png'), # none
'E': arcade.load_texture(path+'Floor_Edge_29.png'), # east
'N': arcade.load_texture(path+'Floor_Edge_36.png'), # north
'EN': arcade.load_texture(path+'Wall_81.png'),
'W': arcade.load_texture(path+'Floor_Edge_33.png'), # west
'EW': arcade.load_texture(path+'Wall_67.png'),
'NW': arcade.load_texture(path+'Wall_80.png'),
'ENW': arcade.load_texture(path+'Wall_66.png'),
'S': arcade.load_texture(path+'Floor_Edge_25.png'), # south
'ES': arcade.load_texture(path+'Wall_55.png'),
'NS': arcade.load_texture(path+'Wall_3.png'),
'ENS': arcade.load_texture(path+'Wall_4.png'),
'WS': arcade.load_texture(path+'Wall_53.png'),
'EWS': arcade.load_texture(path+'Wall_69.png'),
'NWS': arcade.load_texture(path+'Wall_1.png'),
'ENWS': arcade.load_texture(path+'Wall_57.png'),
}
That nearly worked but there was a problem here:
class BorderList:
def __init__(self, layout, cell):
self.borders = []
my_room = layout.get_room(cell)
for side in [(1,0), (0,1), (-1, 0), (0,-1)]:
neighbor = layout.at_offset(cell.xy, side)
neighbor_room = layout.get_room(neighbor) if neighbor else None
border = my_room.border_type(neighbor_room)
self.borders.append(Border(side, border))
That list in the loop must be arranged in that order, ENWS, so that the string comes out in the right order. I’ll do two things. First, I’ll put a comment in that method.
class BorderList:
def __init__(self, layout, cell):
self.borders = []
my_room = layout.get_room(cell)
# list below must be in ENWS order!
for side in [(1,0), (0,1), (-1, 0), (0,-1)]:
...
Second I’ll write a test shell and mark it as needing to be done.
@pytest.mark.skip('Needs to be written')
def test_border_list_in_ENWS_order(self):
# set up room surrounded on all sides,
#ensure border comes back ENWS
assert False
Now I’ll get an irritating ignored test until I do that.
Now we need a bit of renaming, index to something better:
class BorderList:
def border_string(self):
strings = [border.border_letter() for border in self.borders]
return ''.join(strings)
class Border:
def border_letter(self):
if self.boundary == 'open':
return ''
else:
return self.border_indexes[self.side]
We’re good. Green except for the newly irritating skipped test, and main works just fine:
I think that map may have all the possible borders. I declare this good. Commit: Borders now report ENWS strings needed by tiling code.
I found a test that shows me the missing one is easier to write than I thought.
def test_border_list_in_ENWS_order(self):
layout = DungeonLayout(10, 10)
cell = Cell(1, 1)
layout.add_room(Room([cell]))
borders = BorderList(layout, cell)
assert borders.border_string() == 'ENWS'
Green, of course. Commit: added test for border order.
Summary
What I think I’d do if I were making tiles for the game, is that I’d name the files in some orderly way, including the ENWS notation for those specialized tiles, so that it would be easy to set up a table of tiles to purpose. In fact, I think I might go to the trouble of creating the missing ones for the tiled room, since I had to use mud tiles for some of the cases, and the colors don’t match well. If this were a real game, there’d be multiple tile sets, I imagine, so some reasonable naming convention would be helpful.
As for what we have here, we still have the special case of the random open tiles, and we’re assuming just one tile of the 15 other types:
def choose_flooring_texture(self, cell):
borders: BorderList = self.layout.get_borders(cell)
border_type = borders.border_string()
if border_type == '':
return random.choices(self.textures, self.weights)[0]
else:
return self.texture_dictionary[border_type]
This is a bit of a bump in the road, but unless we make the textures dictionary support lists and weights, we’re stuck with it. If and when we have more than one texture in any other case, we could address that. As it stands, I don’t think it’s worth enhancing the structure in that direction. Too speculative.
What have we accomplished? Well, if we never create a new texture, this was wasted. However, since the code currently has an incomplete set of tiles, and since I’ve spared no expense ($10.50) to get the complete set, we will be replacing our tile resources. And I think it might be kind of fun to create some of the missing border cases. I plan to do it with Procreate on my iPad.
I wanted to build the code first, so as to set my mind on a good path for naming the textures I’ll be creating. I’ll probably not rename all the existing ones, unless I run across some kind of file renaming app that can do batches. There are something like 83 tiles in the main set and that is more than I want to rename by hand.
Finally (for now), I think we may someday want to create an Assets kind of object, rather than sprinkle arcade.load_texture calls all over. We definitely have some duplication to get rid of with that string appearing around 20 times so far. Could be as simple as deferring the load_string a bit, could call for an object. I’m sure that if we start supporting multiple different tile sets, we’ll need something. And I’m sure that we can sort that out as we discover it.
I was raised to believe that we had to figure out all these design issues before we started to program. First design, then code. At the risk of fighting the previous Method War over again—a risk we old guys are always facing—I’ll just note that if our objects are small and cohesive, we can usually make the design changes we need when we need them.
I suspect that the real Method War wasn’t against Too Much Design Too Soon, but against Not Enough Design Accompanied By Terrible Code. And I suspect that’s still going on, and my point holds there as well. If we can avoid the Terrible Code part, we can adjust the design more readily when it is inevitably needed.
Be that as it may, the program here progresses in capability and its design improves as we go. That’s why I’m here, why I do this. And I’ll keep doing it as long as I can.
See you next time!