I just want to do a little more this afternoon. Can we move at_offset yet? Yes. Just one more use of current remains.

We were part-way through dealing with Cell.at_offset when a revert became more desirable than slogging through changes that felt sticky. With some other matters now dealt with, let’s try again. Here are the criminal and its collaborators:

class Cell:
    def at_offset(self, dx, dy):
        from dungeonlayout import current
        return current.at_offset(self.xy, (dx, dy))

    def east(self):
        return self.attempt_move(1,0)
    def north(self):
        return self.attempt_move(0, -1)
    def south(self):
        return self.attempt_move(0, 1)
    def west(self):
        return self.attempt_move(-1, 0)

    def attempt_move(self, dx, dy):
        cell = self.at_offset(dx, dy)
        if not cell or cell.is_available:
            return self
        return cell

Those come from Dungeon class, which we haven’t done much with yet:

class Dungeon:
    def move_adventurer_north(self):
        self.place_player_at(self.player_cell.north())

    def move_adventurer_east(self):
        self.place_player_at(self.player_cell.east())

    def move_adventurer_south(self):
        self.place_player_at(self.player_cell.south())

    def move_adventurer_west(self):
        self.place_player_at(self.player_cell.west())

    def place_player_at(self, cell):
        self.player_cell = cell

OK, where’s that coming from? I guess we don’t care. The methods are pretty close to where they belong, and we have a layout in Dungeon.

I think, though, that we might prefer to change this to deal with the cell coordinates rather than the cells themselves, reducing the accesses to Cell methods.

Meh. Did some supposed refactoring, broke things. Tried quick fix, didn’t work. Revert.

Let’s use the Niagara Method, inch by inch, step by step. We’ll defer the calls to player.cell to layout. But we’d really like the cell and the north, etc offset separate. Let’s try this:

class Dungeon:
    def place_player_at_offset(self, cell, offset):
        new_cell = self.layout.at_offset(cell.xy, offset)
        self.place_player_at(new_cell)

    def place_player_at(self, cell):
        self.player_cell = cell

    def move_adventurer_north(self):
        north = (0, -1)
        self.place_player_at_offset(self.player_cell, north)

That works, passes the tests and main works. Repeat the trick, moving the constants up to class members. First this one:

    north = (0, -1)
    def move_adventurer_north(self):
        self.place_player_at_offset(self.player_cell, self.north)

No, let’s not do that, let’s just inline the constants, we have no other need of them.

    def place_player_at_offset(self, cell, offset):
        new_cell = self.layout.at_offset(cell.xy, offset)
        self.place_player_at(new_cell)

    def place_player_at(self, cell):
        self.player_cell = cell

    def move_adventurer_north(self):
        self.place_player_at_offset(self.player_cell, (0, -1))

    def move_adventurer_east(self):
        self.place_player_at_offset(self.player_cell, (1, 0))

    def move_adventurer_south(self):
        self.place_player_at_offset(self.player_cell, (0, 1))

    def move_adventurer_west(self):
        self.place_player_at_offset(self.player_cell, (-1, 0))

This passes the tests but also allows Dot to walk outside the walls. We need to fix that somehow.

How did that work before?

    def attempt_move(self, dx, dy):
        cell = self.at_offset(dx, dy)
        if not cell or cell.is_available:
            return self
        return cell

We need to replicate that, and should have a test for it. This works, because I tried it in main:

    def place_player_at_offset(self, cell, offset):
        new_cell = self.layout.at_offset(cell.xy, offset)
        if new_cell is None or new_cell.is_available:
            new_cell = cell
        self.place_player_at(new_cell)

Now let’s see about the test. Here’s the one we have:

    def test_adventurer_motion(self):
        layout = DungeonLayout(20, 20)
        c55 = layout.at(5, 5)
        round = RoomMaker(layout).round(radius=6, origin=c55)
        layout.add_room(round)
        dungeon = Dungeon(layout)
        dungeon.populate()
        dungeon.place_player_at(c55)
        assert dungeon.player_cell is c55
        dungeon.move_adventurer_north()
        assert dungeon.player_cell is layout.at(5, 4)
        dungeon.move_adventurer_east()
        assert dungeon.player_cell is layout.at(6, 4)
        dungeon.move_adventurer_south()
        assert dungeon.player_cell is layout.at(6, 5)
        dungeon.move_adventurer_west()
        assert dungeon.player_cell is layout.at(5, 5)

That tests legal moves but not much else. Let’s write a new one:

    def test_adventurer_motion_limited(self):
        layout = DungeonLayout(20, 20)
        c55 = layout.at(5, 5)
        room = Room([c55], 'tiny')
        layout.add_room(room)
        dungeon = Dungeon(layout)
        dungeon.populate()
        dungeon.place_player_at(c55)
        dungeon.move_adventurer_north()
        assert dungeon.player_cell is c55
        dungeon.move_adventurer_east()
        assert dungeon.player_cell is c55
        dungeon.move_adventurer_south()
        assert dungeon.player_cell is c55
        dungeon.move_adventurer_west()
        assert dungeon.player_cell is c55

That passes, no surprise. Can’t move out of that tiny one-cell room, as intended. I think we can remove that north etc stuff from Cell now. Green, commit: move player motion to Dungeon class, remove Cell methods.

Let’s sum up.

Summary

I don’t even know what I did wrong the first time that wasn’t working. I think possibly I had defined north as (1, 0) but really don’t know, just reverted when it became clear that whatever the issue was, I didn’t instantly see it.

Tried again, just one of the move methods, and the one new place_player_at_offset method. The unchecked version of that worked, we changed the other three. Then I was smart enough to test walking through the walls and discovered that the new code allowed that — and that we didn’t have a test to show that you can’t do that. Fixed the bug first, because I had it in front of me, then back-filled the test, saw it fail and then work.

We still have the shim in Cell, but less code relies on it. In fact, I think that perhaps no code relies on it. Let’s remove it and see. Pull this out:

class Cell:
    def at_offset(self, dx, dy):
        from dungeonlayout import current
        return current.at_offset(self.xy, (dx, dy))

Nothing breaks. Run main. Works. Are there no references to current any more? No, sorry, one more:

class Cell:
    @property
    def neighbors(self):
        from dungeonlayout import current
        return current.neighbors(self)

Anyone using this? I suspect so. Quick check. Yes, 31 tests break. We’ll look at that next time.

This afternoon, a quick attempt followed by an immediate revert when something went awry, followed by a smaller step, which worked. A bit of good fortune on thinking of the possibility of walking through the walls. That was a good catch, as we had no tests for the case, though the code was in the old scheme and not yet in the new. Code and test both in place now.

Sometimes the bear doesn’t bite you. See you next time!