Hello, loves!

I did a little refactoring on the zoom-scrolling1 code. I think it was the right thing to do. Tout le monde déteste l’IA.

While I do not question that I spent more time on the zoom-scrolling code than was appropriate, I do think it is always worthwhile to take a look at newly committed code after a little while, to let our relieved at last it’s working eyes turn into fresh what have we here eyes that can see possible improvements.

Anyway, I did take a look and the results, I think, are worth noting.

As “finished” yesterday morning:

    def scroll_dungeon_cam(self, cell):
        px, py = self.clamped_position(cell)
        self.dungeon_cam.position = (px, py)

    def clamped_position(self, cell):
        effective_zoom = self.dungeon_cam.zoom / cell_size # 1 - 4
        px, py = cell.position
        cells_across_screen = self.max_x/effective_zoom
        margin_x = self.cells_each_side(cells_across_screen)
        px = min(max(px, margin_x), self.max_x - margin_x - 1)

        cells_up_screen = self.max_y/effective_zoom
        margin_y = self.cells_each_side(cells_up_screen)
        py = min(max(py, margin_y), self.max_y - margin_y - 1)

        return px, py

    def cells_each_side(self, cells_across_screen):
        q, r = divmod(int(cells_across_screen), 2)
        return q - 1 if r == 0 else q

As it stands now:

    def scroll_dungeon_cam(self, cell):
        x_confined = self.confine(cell.x, self.max_x)
        y_confined = self.confine(cell.y, self.max_y)
        self.dungeon_cam.position = (x_confined, y_confined)

    def confine(self, coord, size):
        visible_cell_count = min(size, size / (self.dungeon_cam.zoom / cell_size))
        margin_lo = self.cells_visible_on_each_side_of_center(visible_cell_count)
        margin_hi = size - 1 - margin_lo
        return margin_lo if coord < margin_lo \
            else margin_hi if coord > margin_hi \
            else coord

    @staticmethod
    def cells_visible_on_each_side_of_center(cells_across_screen):
        q, r = divmod(int(cells_across_screen), 2)
        return q - 1 if r == 0 else q

Shorter, doesn’t duplicate the working bits, and I think the names are a bit more communicative.

Summary

Despite the different look, that wasn’t a rewrite: just a lot of inlining, extracting, and renaming. PyCharm saw the duplication but didn’t recognize the opportunity to apply confine twice, so I had to do that by hand, if I recall correctly.

Small steps, better code. I’m glad I did it again. See you next time!

  1. I really wish I had thought of the phrase “zoom-scrolling” sooner. ↩