The Repo on GitHub

OK, I forgot that World references Block class. That’s why we read the code before estimating, if we must estimate. Let’s break that connection. Result: small progress, but Bear Bites Man.

Sitting here musing about it, I suspect no one needs the Block class at all. World will just have another Entity, and if and when we elaborate that single class into multiple types, those will be on the world side. And, thus far, all the client side sees is a name in the vision, indicating what it sees. A letter B for Block.

Nope
That’s not as true as it might be. Somewhere below, I stumble on the fact that the notion of the Bot holding a Block adds complexity that I honestly did not need this morning. Chaos ensues. I spare you most of it.

Here’s our reference to it:

class World:
    def add_block(self, x, y, aroma=0):
        entity = WorldEntity.block(x, y, aroma)
        returned_client_entity = Block(x, y)
        return self.add_and_return_client_entity(entity, returned_client_entity)

    def add_and_return_client_entity(self, entity, returned_client_entity):
        self.map.place(entity)
        id = entity.id
        returned_client_entity.id = id
        return returned_client_entity

We can compare that to our new code for Bot, which was almost identical to the above:

class World:
    def add_bot(self, x, y, direction = Direction.EAST):
        entity = WorldEntity.bot(x, y, direction)
        self.map.place(entity)
        return entity.id

We’ll do very similarly for block, of course, and then converge the duplication if it makes sense, which it may not.

We also have a number of uses of the Block class in tests, such as this one, picked because it was the one I clicked on:

    def test_bot_cannot_drop_off_world_north(self):
        world = World(10, 10)
        bot = DirectConnection(world).add_bot(5, 0)
        block = Block(4, 4)
        bot.receive(block)
        bot.direction = Direction.NORTH
        world.drop_forward(bot, block)
        assert bot.has(block), 'drop should not happen'

I think we’d better see what happens with receive, because we’re passing something between the World and the client at that point, so we really should look carefully, lest there be another leak.

class World:
    def take_forward(self, bot):
        is_block = lambda e: e.name == 'B'
        if block := self.map.take_conditionally_at(bot.forward_location(), is_block):
            bot.receive(block)

class Map:
    def take_conditionally_at(self, take_location, condition):
        item = self.at_xy(take_location.x, take_location.y)
        if item and condition(item):
            self.remove(item.id)
            return item
        else:
            return None

But what is the receive on WorldEntity?

class WorldEntity:
    def receive(self, entity):
        self.holding = entity

    @property
    def holding(self):
        try:
            return self._dict['held_entity']
        except KeyError:
            return None

    @holding.setter
    def holding(self, value):
        self._dict['held_entity'] = value

Right and what do we put in the fetch dictionary?

    def as_dictionary(self):
        return self._dict

Right. I think we can write a failing test here.

    def test_connection_does_not_hold_world_entity(self):
        world = World(10, 10)
        bot_id = world.add_bot(5, 5)
        world_bot = world.map.at_id(bot_id)
        world.add_block(6, 5)
        world.take_forward(world_bot)
        assert world_bot.holding
        info = world.fetch(bot_id)
        assert not isinstance(info['held_entity'], WorldEntity)

And, of course, we find a WorldEntity coming back from fetch, which means that the client Bot will be holding on to a WorldEntity.

This is harder than it may seem …

We do need to pass something back to the client bot when she is holding something, and that something probably has to be something more than just an id or letter.

Self Critique
I do wish that past me had been a bit more careful about client-server, even though I know that past me ignored the issue on purpose, so as to put present me into these situations, so that present me could show that we can refactor our way to where we need to be, in small steps, never breaking things, all very nice. Easy for you to say, past me!

Let’s have a glance at the client side. I don’t think we pay much attention to what we are holding.

Arrgh
I just rewound some changes. I left the game not working yesterday, all unknowing, and my idea for dealing with this transfer issue was not a good one. So I rewound the history, fixed that bug, then commit: game uses DirectConnection to create bots.

There is a larger issue here, which is that the game can fail without any tests failing, but the things it does are invasive enough that I am not surprised.

I’ll reinstall that test and then, I guess, ignore it so that I can commit. Commit: new ignored test must run before we can say the split is complete.

The work I just did and threw away as not even worth writing about has taken the wind from my sails. I want to stop and take a break. And I also want not to walk away from the table losing. Let’s try to get the block creation to work while returning an id instead of a client entity. I think this will break things but let’s just make the change and see.

class World:
    def add_block(self, x, y, aroma=0):
        entity = WorldEntity.block(x, y, aroma)
        self.map.place(entity)
        return entity.id

I am somewhat surprised that only one test breaks.

    def test_drop_block_on_open_cell(self):
        world = World(10, 10)
        connection = DirectConnection(world)
        client_bot = connection.add_bot(5, 5)
        bot_id = client_bot.id
        world_bot = world.map.at_id(bot_id)
        client_block = world.add_block(1, 9)
>       world_block = world.map.at_id(client_block.id)
E       AttributeError: 'int' object has no attribute 'id'

No surprise, we are returning the id, not a block. The fix is hideous:

    def test_drop_block_on_open_cell(self):
        world = World(10, 10)
        connection = DirectConnection(world)
        client_bot = connection.add_bot(5, 5)
        bot_id = client_bot.id
        world_bot = world.map.at_id(bot_id)
        block_id = world.add_block(1, 9)
        client_block = Block(1, 9)
        client_block.id = block_id
        world_block = world.map.at_id(block_id)
        world_bot.receive(world_block)
        assert len(world.map.contents.keys()) == 2
        connection.drop(client_bot, client_block)
        assert len(world.map.contents.keys()) == 2
        assert not world_bot.has(world_block)
        assert not world.is_empty(Location(6, 5))

All this messing around with various ids so that we can get ahold of the right things to check! It’s awful.

But it is working. Commit: Fix test to allow returning id from add_block.

Even the above was ragged. I have been thrown off my game. Time to stop.

Summary

Over a period of about two hours, I have made dozens of changes to the program and rolled back most of them. They were so horrid that I couldn’t bear to make you read about them. The real issue, I think, is that the obvious simple path to fixing up the notion of holding “simply” won’t work. It’s going to take a little thinking: we need some kind of structured object that we can send back and forth across the connection, and it appears that just an id won’t do the job.

I tried it, just passing the id across, and things broke. I tried to fix them. They didn’t quite fix. Then I noticed that the game was broken and I couldn’t see how to fix that: it wanted to know something that, it seemed, it couldn’t reach. (I think I was wrong about that. I was probably rattled by then.)

So I backed away in confusion, and since I had failed to commit yesterday, and failed to check this morning, I had to learn to use Local History to revert the morning’s work. It was easy, and a lovely feature, but I was rattled.

Do not program when rattled, that’s my new motto. But sometimes, it seems, we just have to.

Sometimes the bear bites you. Today was one of those times. Tomorrow we’ll try to do better. See you then!