Whazzup??

I’m not sure what I’ll do this morning. Look for something that interests me, I guess. We follow our nose. Refactoring for fun and profit.

Ah, here’s something that needs doing. We have no clear scheme for determining the dimensions of the window the game runs in. Let’s see what all we do.

We start with a DungeonLayout, which has a specified number of cells in x and y directions. Our main uses 64x56 because anything larger than 56 is too tall for my screen. That information is not saved in DungeonLayout and probably should be:

class DungeonLayout:
    def __init__(self, max_x=10, max_y=10):
        self.border_map = None
        self.cells = dict()
        for x in range(max_x):
            for y in range(max_y):
                self.cells[(x,y)] = Cell(x, y)
        self.rooms = []

We’ll save those values as members.

class DungeonLayout:
    def __init__(self, max_x=10, max_y=10):
        self.max_x = max_x
        self.max_y = max_y
        self.border_map = None
        self.cells = dict()
        for x in range(max_x):
            for y in range(max_y):
                self.cells[(x,y)] = Cell(x, y)
        self.rooms = []

I don’t love those names but they’ll do for now.

Now we have the file ‘params.py’

# dungeon parameters
screen_width = 1024
screen_height = 1024 - 128
cell_size = 16

I’m not sure who is using that file. The views DungeonView and RoomView refer to cell_size for their dimensioning. Main refers to the width and height values. Using just cell_size, I can remove the width and height ones. Here is that part of main:

def main()
    ...
    dungeon.place_content_at(Cell(29, 30), item_3)
    screen_width = layout.max_x*cell_size
    screen_height = layout.max_y*cell_size
    window = arcade.Window(screen_width, screen_height, 'Caveat Emptor')
    view = DungeonView(dungeon)
    view.setup()
    window.show_view(view)
    arcade.run()

All that stuff after the place_content seems to me to be a matter that should concern the DungeonView, not main. Feature Envy. Let’s change DungeonView to be more helpful.

This is a bit more tricky than I thought, because arcade requires a window before we can create our view. I settle for this:

main.py
    dungeon.place_content_at(Cell(29, 30), item_3)
    screen_width = cell_size*dungeon.max_x
    screen_height = cell_size*dungeon.max_y
    arcade.Window(screen_width, screen_height, 'Caveat Emptor')
    DungeonView(dungeon).run()

Supported thus:

class DungeonView:
    def run(self):
        self.setup()
        self.window.show_view(self)
        arcade.run()

That works. Commit it. My eye was caught by this atrocity:

    def setup_dungeon_camera(self):
        self.dungeon_camera = arcade.Camera2D()
        zoom = 4
        self.dungeon_camera.zoom = zoom
        w = 64
        h = 56
        width = w * cell_size
        height = h * cell_size
        width_in_cells = w // zoom
        height_in_cells = h // zoom
        width_margin = width_in_cells // 2 * cell_size
        height_margin = height_in_cells // 2 * cell_size
        self.dungeon_camera_bounds = arcade.LRBT(
            width_margin,
            width - width_margin,
            height_margin,
            height - height_margin)

I think that we can do better. To begin with, I recently learned that view know the width and height of their window. Let’s assume some properties as well. I do some algebra, well actually a bit of smashing to get this:

    def setup_dungeon_camera(self):
        self.dungeon_camera = arcade.Camera2D()
        zoom = 4
        self.dungeon_camera.zoom = zoom
        # width_in_cells = self.max_x // zoom
        # width_margin = width_in_cells // 2 * cell_size
        width_margin = self.width // zoom // 2
        # height_in_cells = self.max_y // zoom
        # height_margin = height_in_cells // 2 * cell_size
        height_margin = self.height // zoom // 2
        self.dungeon_camera_bounds = arcade.LRBT(
            width_margin,
            self.width - width_margin,
            height_margin,
            self.height - height_margin)

Let’s do the algebra:

width_in_cells = self.max_x // zoom
width_margin = width_in_cells // 2 * cell_size

# substituting 1 into 2
width_margin = self.max_x // zoom // 2 * cell_size

# reordering terms
width_margin = self.max_x * cell_size // zoom // 2

# but self.max_x*cell_size is self.width
width_margin = self.width // zoom // 2

# QED

Also it works. Commit: simplify camera bounds calculation.

I think this is right but wrong. What is this code trying to accomplish? Well, I’m glad you asked. We have 64 cells across in our dungeon, and 1/4 of that or 16 in our zoomed view. When we walk toward the edges, we want to stop scrolling when we get within 8 cells of the edge, because if we don’t do that we get big chunks of black on the screen.

The view margins are in pixels, not cells. (And, worse yet, on my high-res screen, they are fake pixels but we’re not even going there.)

So what we want, I think, is to know how many cells we can display on the screen (16), reserve half that many on each side (8), and convert that to pixels.

Let me see if I can express that in a reasonable way.

    def setup_dungeon_camera(self):
        self.dungeon_camera = arcade.Camera2D()
        zoom = 4
        self.dungeon_camera.zoom = zoom
        cells_across = self.max_x // zoom
        cell_margin_across = cells_across // 2
        width_margin = cell_margin_across * cell_size
        cells_high = self.max_y // zoom
        cell_margin_high = cells_high // 2
        height_margin = cell_margin_high * cell_size
        self.dungeon_camera_bounds = arcade.LRBT(
            width_margin,
            self.width - width_margin,
            height_margin,
            self.height - height_margin)

I think those tell a better story but the method is now harder to read. Let’s refactor a bit:

    def setup_dungeon_camera(self):
        self.dungeon_camera = arcade.Camera2D()
        zoom = 4
        self.dungeon_camera.zoom = zoom
        cell_count = self.max_x
        cells_across = cell_count // zoom
        cell_margin_across = cells_across // 2
        width_margin = cell_margin_across * cell_size
        cell_count = self.max_y
        cells_high = cell_count // zoom
        cell_margin_high = cells_high // 2
        height_margin = cell_margin_high * cell_size

Now I think these are close enough to identical to have PyCharm help with extract method. Hm, not quite, have to do it partially manually:

    def setup_dungeon_camera(self):
        self.dungeon_camera = arcade.Camera2D()
        zoom = 4
        self.dungeon_camera.zoom = zoom
        cell_count = self.max_x
        width_margin = self.compute_margin(cell_count, zoom)
        cell_count = self.max_y
        height_margin = self.compute_margin(cell_count, zoom)
        self.dungeon_camera_bounds = arcade.LRBT(
            width_margin,
            self.width - width_margin,
            height_margin,
            self.height - height_margin)

Inline twice:

    def setup_dungeon_camera(self):
        self.dungeon_camera = arcade.Camera2D()
        zoom = 4
        self.dungeon_camera.zoom = zoom
        width_margin = self.compute_margin(self.max_x, zoom)
        height_margin = self.compute_margin(self.max_y, zoom)
        self.dungeon_camera_bounds = arcade.LRBT(
            width_margin,
            self.width - width_margin,
            height_margin,
            self.height - height_margin)

And of course this is the extracted method:

    def compute_margin(self, cell_count, zoom):
        cells_across = cell_count // zoom
        cell_margin_across = cells_across // 2
        width_margin = cell_margin_across * cell_size
        return width_margin

We improve the names:

    def compute_margin(self, cell_count, zoom):
        cells_visible = cell_count // zoom
        desired_margin = cells_visible // 2
        pixel_margin = desired_margin * cell_size
        return pixel_margin

Let’s extract another method for the rectangle and reorder the top method a bit:

    def setup_dungeon_camera(self):
        self.dungeon_camera = arcade.Camera2D()
        zoom = 4
        width_margin = self.compute_margin(self.max_x, zoom)
        height_margin = self.compute_margin(self.max_y, zoom)
        self.dungeon_camera_bounds = self.margin_rectangle(width_margin, height_margin)
        self.dungeon_camera.zoom = zoom

    def compute_margin(self, cell_count, zoom):
        cells_visible = cell_count // zoom
        desired_margin = cells_visible // 2
        pixel_margin = desired_margin * cell_size
        return pixel_margin

    def margin_rectangle(self, width_margin, height_margin):
        return arcade.LRBT(
            width_margin,
            self.width - width_margin,
            height_margin,
            self.height - height_margin)

I think I like that and might be able to figure it out later. Commit: refactoring camera setup.

Let’s sum up.

Summary

The trigger for today’s work was the observation that different parts of the program used different ways of knowing how big the dungeon or display are. We resolved that by making max_x and max_y available in the Layout, accessed by properties in Dungeon and DungeonView, so that everyone who wanted to know that can find out. We still have the magic number cell_size, which is kept in a file ‘params.py’. It’s the basic drawing size for everything, and I’m not sure what else would be better. We could provide it to the Layout, but it’s really only interesting to the views, I think.

Once we had the max x and y in place, we cleaned up the program initiation a bit, moving more responsibility out of main and into the DungeonView. That’s still a bit odd, because you have to create a window before a view, but after that you have no use for the window. Maybe there’s a trick that I don’t know.

But then, we stumbled onto the setup_dungeon_camera method, which was 18 hard-to-understand lines. I tried a few refactorings, was able to make it shorter but less clear. Then a moderately interesting series of refactoring steps got us “down” to “only” 21 lines … but they are, I think, substantially more clear. The top level method says what is going on and it calls two other methods, one that it calls twice to compute the x and y margins, and one that just encapsulates a somewhat nasty rectangle creation.

Note to Self

At one point in my attempt to set up the View for main, the program opened two windows. I thought I had read that arcade couldn’t do that,but if it can, it might be convenient to have one window for the dungeon view, and one or more others for the GUI / information screens. Look into this possibility.

Lesson?

I hesitate even to think in terms of lessons, but I have noticed a tendency in myself that I’ve also noticed on real teams. There are things like the camera setup we worked on today, where what one does is find an example on line and smash on it until it does what we want, and then we back away slowly, hands out, saying “Steady, Blue, steady”. We never turn our back, we never run. That will trigger the raptor’s hunting instinct. No, wait, that’s not right, I was confused for a moment.

Anyway, we tend to back away from the code, leaving it working but not well formed or well understood. The next time we encounter it, it confuses us. We hope that we never have to change it, much less explain it. And ugly boilerplate code just proliferates, profanes, and poisons our code.

I enjoy refactoring code like that, when I find time, and I always learn a bit about how things work, often getting a bit of insight and finding a way to reflect that in the code. It keeps my brain alive, helps me with the next thing I do, and keeps the code a bit more alive as well.

I commend the notion to your attention. Advice? No, I wouldn’t dare.

See you next time!