Hello, loves!

Looks like there are a couple of methods where our new Cameras object could help out.

Here are all the DungeonView users of the Camera object:

    def on_draw(self):
        self.clear()
        self.scroll_dungeon_camera()
        with self.cameras.dungeon_cam.activate():
            self.keyed_floor_sprites.draw()
            self.draw_contents()
            self.draw_passages()
            self.draw_flood()
        with self.cameras.scroller_cam.activate():
            self.cameras.scroller.draw()

    def scroll_dungeon_camera(self):
        cx, cy = self.dungeon.player_cell.xy
        self.cameras.dungeon_cam.position = (cx * cell_size, cy * cell_size)
        self.cameras.dungeon_cam.view_data.position = (
            arcade.camera.grips.constrain_xy(
            self.cameras.dungeon_cam.view_data, self.cameras.dungeon_camera_bounds
        ))

    def on_update(self, delta_time: float):
        def receive_announcement(msg):
            self.cameras.scroller.append(msg)
        self.dungeon.announce_via(receive_announcement)
        self.cameras.scroller.update()
        self.pub_sub.publish('on_update', 'view', delta_time=delta_time)

Seems to me that the latter two could use a bit of help from the Cameras object. I don’t see much for the on_draw that would be a lot better. Well … we could do a callback sort of thing, but it would basically look the same anyway, just more complicated. So, at least until I get a better idea, we’ll deal with the other two. The scrolling one looks most promising.

Let’s extract a method, passing in cx and cy:

    def scroll_dungeon_camera(self):
        cx, cy = self.dungeon.player_cell.xy
        self.scroll_dungeon_cam(cx, cy)

    def scroll_dungeon_cam(self, cx, cy):
        self.cameras.dungeon_cam.position = (cx * cell_size, cy * cell_size)
        self.cameras.dungeon_cam.view_data.position = (
            arcade.camera.grips.constrain_xy(
                self.cameras.dungeon_cam.view_data, self.cameras.dungeon_camera_bounds
            ))

Move the method over to Cameras and remove all the cameras. I did something wrong but extracted and inlined and waved my hands and the squiggles went away:

class Cameras:
    def scroll_dungeon_cam(self, cx, cy):
        self.dungeon_cam.position = (cx * cell_size, cy * cell_size)
        self.dungeon_cam.position = (
            arcade.camera.grips.constrain_xy(
                self.dungeon_cam.view_data, self.dungeon_camera_bounds
            ))

Now back in DungeonView, we have this:

class DungeonView:
    def scroll_dungeon_camera(self):
        cx, cy = self.dungeon.player_cell.xy
        self.cameras.scroll_dungeon_cam(cx, cy)

Called by this:

    def on_draw(self):
        self.clear()
        self.scroll_dungeon_camera()
        with self.cameras.dungeon_cam.activate():
            self.keyed_floor_sprites.draw()
            self.draw_contents()
            self.draw_passages()
            self.draw_flood()
        with self.cameras.scroller_cam.activate():
            self.cameras.scroller.draw()

So inline the scroll method, giving:

    def on_draw(self):
        self.clear()
        cx, cy = self.dungeon.player_cell.xy
        self.cameras.scroll_dungeon_cam(cx, cy)
        with self.cameras.dungeon_cam.activate():
            self.keyed_floor_sprites.draw()
            self.draw_contents()
            self.draw_passages()
            self.draw_flood()
        with self.cameras.scroller_cam.activate():
            self.cameras.scroller.draw()

Should we pass in the Cell, not the XY? Maybe but first test. Works. Commit: moving method to Cameras.

Now let’s do just pass it the cell:

    def on_draw(self):
        self.clear()
        self.cameras.scroll_dungeon_cam(self.dungeon.player_cell)
        with self.cameras.dungeon_cam.activate():
            self.keyed_floor_sprites.draw()
            self.draw_contents()
            self.draw_passages()
            self.draw_flood()
        with self.cameras.scroller_cam.activate():
            self.cameras.scroller.draw()

class Cameras:
    def scroll_dungeon_cam(self, cell):
        cx, cy = cell.xy
        self.dungeon_cam.position = (cx * cell_size, cy * cell_size)
        self.dungeon_cam.position = (
            arcade.camera.grips.constrain_xy(
                self.dungeon_cam.view_data, self.dungeon_camera_bounds
            ))

Nicer. Commit.

Reflection

I’m not sure what I did wrong to get the squiggles, and I don’t care. Did the edit a third time and it was happy. I think it’s possible that PyCharm was confused but a good workman etc etc. Scrolling the camera inside the object that has the camera makes sense, and DungeonView is now down to 229 lines from its 291 before we started our improvements today. Happy to have done it.

Let’s review the users of cameras again:

class DUngeonView:
    def on_draw(self):
        self.clear()
        self.cameras.scroll_dungeon_cam(self.dungeon.player_cell)
        with self.cameras.dungeon_cam.activate():
            self.keyed_floor_sprites.draw()
            self.draw_contents()
            self.draw_passages()
            self.draw_flood()
        with self.cameras.scroller_cam.activate():
            self.cameras.scroller.draw()

    def on_update(self, delta_time: float):
        def receive_announcement(msg):
            self.cameras.scroller.append(msg)
        self.dungeon.announce_via(receive_announcement)
        self.cameras.scroller.update()
        self.pub_sub.publish('on_update', 'view', delta_time=delta_time)

This is a bit tricky unless we pass dungeon in to cameras. Let’s check announce_via:

class Dungeon:
    def announce_via(self, callback):
        for announcement in self.announcements:
            callback(announcement)
        self.announcements = []

Looks to me that as long as the announcements are in dungeon, we need to honor that. It seems to me that since they appear in the scroller that they really belong closely bound to it. But unwinding that can wait.

Let’s Extract Variable, to force a parameter in the next step:

    def on_update(self, delta_time: float):
        dungeon = self.dungeon
        def receive_announcement(msg):
            self.cameras.scroller.append(msg)

        dungeon.announce_via(receive_announcement)
        self.cameras.scroller.update()
        self.pub_sub.publish('on_update', 'view', delta_time=delta_time)

Then Extract Method:

    def on_update(self, delta_time: float):
        dungeon = self.dungeon
        self.update_scroller(dungeon)
        self.pub_sub.publish('on_update', 'view', delta_time=delta_time)

    def update_scroller(self, dungeon):
        def receive_announcement(msg):
            self.cameras.scroller.append(msg)

        dungeon.announce_via(receive_announcement)
        self.cameras.scroller.update()

Then move the method and call it:

class Cameras:
    def update_scroller(self, dungeon):
        def receive_announcement(msg):
            self.scroller.append(msg)
        dungeon.announce_via(receive_announcement)
        self.scroller.update()

class DungeonView:
    def on_update(self, delta_time: float):
        dungeon = self.dungeon
        self.cameras.update_scroller(dungeon)
        self.pub_sub.publish('on_update', 'view', delta_time=delta_time)

And inline the temp back:

    def on_update(self, delta_time: float):
        self.cameras.update_scroller(self.dungeon)
        self.pub_sub.publish('on_update', 'view', delta_time=delta_time)

Test. Commit.

Reflection

We only saved about three lines from DungeonView this time, but the capability is where it belongs. I wonder what our references to Cameras look like now:

    def on_draw(self):
        self.clear()
        self.cameras.scroll_dungeon_cam(self.dungeon.player_cell)
        with self.cameras.dungeon_cam.activate():
            self.keyed_floor_sprites.draw()
            self.draw_contents()
            self.draw_passages()
            self.draw_flood()
        with self.cameras.scroller_cam.activate():
            self.cameras.scroller.draw()

    def on_update(self, delta_time: float):
        self.cameras.update_scroller(self.dungeon)
        self.pub_sub.publish('on_update', 'view', delta_time=delta_time)

I think we could conceivably abstract all the drawing away from the view, into a DungeonDrawingThing object. Might be worth it. But for now, I think we’ve pushed the Cameras object just about as far as we can.

Summary

A small object, Cameras, improved the code a bit, and opened doors for more code to flow over to that object, improving DungeonView while isolating all the camera fiddling over into Cameras.

DungeonView continues to decrease in size and complexity, in small steps of one or two hours work. I really don’t know where it will wind up, but indications are that it will be a lot less complicated than it currently is.

There are still nearly 30 methods in DungeonView.

I think there may be profit in moving keyboard handling off to another object: there are about 35 lines in there, many of them just text messages.

We have about 25 or 30 lines taken up in subscriptions: there might be something there. Illumination is around 15 lines.

Just initializing and setting up to be ready to go is around 80 lines. That seems like a lot.

So … small improvements make a difference, and it can take a lot of small improvements to bring an overly complex class back into good order. Of course it would be better never to let that happen, but in over six decades of programming, I’ve never seen a real project that didn’t have some nasty bits like this … and often much worse.

In the olden days we asked for time to rewrite things and if that ever worked out well, I don’t recall the occasion. These days, I’d do as we’re doing here, improve things little by little, as time goes on.

And time has gone on this afternoon. See you next time!